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

Allow additionalItems keyword into schema #20

Closed rjlohan closed 5 years ago

rjlohan commented 5 years ago

We originally converted items to disallow mixed tuple validation of lists, and rather expected an array to allow only a single consistent type.

However, we also omitted the additionalItems keyword which can be used in certain validation cases to allow an array to validate a single type with a fixed set of allowed values. An example of this is the AWS::CloudFront::Distribution.DistributionConfig.CacheBehavior.AllowedMethods field which uses a set of array values as a wonky enum validation.

SEE: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-cloudfront-distribution-cachebehavior.html#cfn-cloudfront-distribution-cachebehavior-allowedmethods

This needs to be put into the schema;

"additionalItems": { 
    "type": "boolean"
}
rjlohan commented 5 years ago

Also to demonstrate the schema I want (which is valid and functional JSON Schema) but I cannot have because of htis missing keyword;

{
  "AllowedMethods": {
    "oneOf": [
      {
        "additionalItems": false,
        "items": {
          "enum": [
            "GET",
            "HEAD"
          ],
          "type": "string"
        },
        "minItems": 2,
        "uniqueItems": true
      },
      {
        "additionalItems": false,
        "items": {
          "enum": [
            "GET",
            "HEAD",
            "OPTIONS"
          ],
          "type": "string"
        },
        "minItems": 3,
        "uniqueItems": true
      },
      {
        "additionalItems": false,
        "items": {
          "enum": [
            "GET",
            "HEAD",
            "PUT",
            "POST",
            "PATCH",
            "DELETE"
          ],
          "type": "string"
        },
        "minItems": 6,
        "uniqueItems": true
      }
    ],
    "type": "array"
  }
}
aygold92 commented 5 years ago

I don't understand how this relates to additionalItems? additionalItems is false in each section above. Taking away the additionalItems=false, this schema accomplishes what you are trying to do and is valid in our current schema

rjlohan commented 5 years ago

The intent of this schema is to require that one of those 3 lists-of-strings is specified. There are no alternatives allowed.

Valid inputs against this schema are one of these (with items in any order); "AllowedMethods": [ "GET", "HEAD" ] "AllowedMethods": [ "GET", "HEAD", "OPTIONS" ] "AllowedMethods": [ "GET", "HEAD", "PUT", "POST", "PATCH", "DELETE" ]

With "additionalItems": true (or absent) the following invalid items become allowed;

"AllowedMethods": [ "GET", "GET", "GET", "GET" ] "AllowedMethods": [ "HEAD", "GET", "GET", "HEAD" ]

aygold92 commented 5 years ago

That's not what I gathered from the docs https://json-schema.org/understanding-json-schema/reference/array.html#list-validation

In each oneOf block, you use items as a single schema, and there are multiple notes about this

Note

When items is a single schema, the additionalItems keyword is meaningless, and it should not be used.
Note

additionalItems doesn’t make sense if you’re doing “list validation” (items is an object), and is ignored in the case.

By your schema above, removing additionalItems: false. "AllowedMethods": [ "GET", "GET", "GET", "GET" ] fails each schema because of the "uniqueItems: true" constraint

rjlohan commented 5 years ago

The various tools I've used to validate are not compliant with the docs here, perhaps because of the oneOf block around the different items schemas. uniqueItems is also not validating correctly the way I want, though the additionalItems statement does work.

Besides the point of 'this should be irrelevant' is there any reason to block this schema enhancement if it seems supported by tools?

rjlohan commented 5 years ago

Closing this, as the suggestion from @aygold92 about uniqueItems did actually work when I fiddled a bit.