aws-cloudformation / cloudformation-resource-schema

The CloudFormation Resource Schema defines the shape and semantic for resources provisioned by CloudFormation. It is used by provider developers using the CloudFormation RPDK.
Apache License 2.0
93 stars 38 forks source link

Change identifiers items from jsonPointer to list of jsonPointers #4

Closed johnttompkins closed 5 years ago

johnttompkins commented 5 years ago

Issue #, if available: https://github.com/aws-cloudformation/aws-cloudformation-rpdk/issues/81

Description of changes: This changes the structure for identifiers so that compound identifiers are valid. Previously, only single property identifiers were valid. Before:

{
...
    "identifiers": {
        "#/properties/identifier1"
    }
}

In the above schema, if two resource properties identifier2a and identifier2b together uniquely identify a resource, there is no way of expressing that with a single JSON pointer. The proposed change allows for that and is also in line with what CloudFormation is expecting when pulling down schemas.

Now:

{
...
    "identifiers": {
        ["#/properties/identifier1"],
        ["#/properties/identifier2a", "#/properties/identifier2b"] 
    }
}

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

johnttompkins commented 5 years ago

Is this basically aws-cloudformation/aws-cloudformation-rpdk#81?

Yes, although with this change, something you mentioned in that issue like:

"identifiers": [
    "#/properties/Arn",
    ["#/properties/Foo", "#/properties/Bar"],
    "#/properties/URL"
]

would have to be

"identifiers": [
    ["#/properties/Arn"],
    ["#/properties/Foo", "#/properties/Bar"],
    ["#/properties/URL"]
]

Not sure if this would be a pain point for customer

tobywf commented 5 years ago

I don't think it's a big deal as long as the examples are obvious. Be good to close that issue once this is merged.