Skip to content

Conversation

@srujithpoondla03
Copy link
Contributor

@srujithpoondla03 srujithpoondla03 commented Dec 4, 2020

Description of changes:
Currently, in the provider definition schema we do not restrict the structured property to specify the additionalProperties as false. Structured properties should define the additionalProperties as false otherwise properties with different case or pattern could not be caught during validation.

Tested by running the cfn validate

⋊> ~/c/NrqlAlertCondition on master ⨯ cfn validate                                                                                                                                                       
[Warning] Resource spec validation would fail from next major version. Provider should mark additionalProperties as false if the property is defined as a structured property. Please fix the warnings: 'additionalProperties' is a required property

Failed validating 'required' in schema['properties']['definitions']['patternProperties']['^[A-Za-z0-9]{1,64}$']['allOf'][0]['dependencies']['properties']:
    {'$comment': 'An object cannot have both defined and undefined '
                 'properties; therefore, patternProperties is not allowed '
                 'when properties is specified.',
     'not': {'required': ['patternProperties']},
     'required': ['additionalProperties']}

On instance['definitions']['Term']:

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

…onal properties to handlers during validation
@priyap286
Copy link
Contributor

Do we need to define what a structured property is? Also "additionalProperties" are a property of object schemas and are required only there.

Copy link
Contributor

@ammokhov ammokhov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@ammokhov ammokhov merged commit 6e9fd5c into aws-cloudformation:master Dec 10, 2020
prerna-p pushed a commit to prerna-p/cloudformation-cli that referenced this pull request Dec 29, 2020
prerna-p pushed a commit to prerna-p/cloudformation-cli that referenced this pull request Dec 29, 2020
@PatMyron
Copy link
Contributor

PatMyron commented Jan 7, 2021

seems explicitly documented that additionalProperties should not be used:

"use of additionalProperties is not valid for a resource property"

Should try to check why that was documented and update that documentation if that's no longer the case

@srujithpoondla03
Copy link
Contributor Author

srujithpoondla03 commented Jan 7, 2021

Use of additionalProperties is not valid means provider should not model their schema to accept additionalProperties instead they should define all the properties or use patternProperties.

@PatMyron
Copy link
Contributor

PatMyron commented Jan 7, 2021

@srujithpoondla03 got it, maybe just re-worded to clarify then?

@PatMyron PatMyron added schema Related to the provider meta schema schema processing labels May 5, 2021
kddejong pushed a commit to kddejong/cloudformation-cli that referenced this pull request Oct 24, 2022
kddejong pushed a commit to kddejong/cloudformation-cli that referenced this pull request Oct 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

schema processing schema Related to the provider meta schema

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants