added primary id validation #482
Conversation
ammokhov
added
code quality
enhancement
|
Should this validation be added to https://github.com/aws-cloudformation/aws-cloudformation-resource-schema/tree/master/src as well/instead? |
Agree with this we should just schematize this |
| in_createonly = _is_in(resource_spec, "createOnlyProperties") | ||
|
|
||
| primary_id = resource_spec["primaryIdentifier"] | ||
| if not in_readonly(primary_id) and not in_createonly(primary_id): |
There was a problem hiding this comment.
can simplify a bit to make condition primaryIdentifier - (createOnlyProperties | readOnlyProperties) where they are all sets
There was a problem hiding this comment.
used closure to reuse the subset operation but we might use a disjoint of createOnly | readOnly
There was a problem hiding this comment.
as discussed offline, we cant really schematize the condition, so this change should go in both repos. |
ammokhov
force-pushed
the
primary-identifier-validation
branch
from
b12e00b to
cf31c37
Compare
…-cli into primary-identifier-validation
we want to inform a customer that id should be either createonly or readonly but if we start throwing an exception it will block existing resources from registration. warning still informs but make it backward compatible |
| @@ -0,0 +1,13 @@ | |||
| { | |||
| "typeName": "AWS::Valid::TypeName", | |||
There was a problem hiding this comment.
Was this schema meant to test aws-cloudformation/cloudformation-resource-schema#98 as well?
typeName should be renamed and wondering if the directory name is confusing as well. Having trouble putting LOG.warning violating schemas in invalid/ in #663 as well
Issue #, if available: #361
Description of changes:
we do not allow mutable updates on primary identifier, instead it could be updated immutably, which causes the replacement.
However, primary identifier could be specified in two ways:
readOnly- auto generated by handler or apicreateOnly- specified by the user and cant be further mutably updatedSince, we would want to enforce the schema modeling it makes sense to add this to schema validation to provide definition correctness.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.