Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

catch common resource schema issues in cfn validate #729

Merged
merged 4 commits into from
Apr 26, 2021
Merged

catch common resource schema issues in cfn validate #729

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3036779
catch common resource schema issues in cfn validate
PatMyron Apr 13, 2021
0cf0a0e
expanding type_specific_keywords
PatMyron Apr 13, 2021
f33c34c
black
PatMyron Apr 13, 2021
753de4c
iterating keyword_mappings to get all type_specific_keywords
PatMyron Apr 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 17 additions & 16 deletions src/rpdk/core/data_loaders.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,18 +145,6 @@ def load_resource_spec(resource_spec_file): # pylint: disable=R # noqa: C901
LOG.debug("Resource spec validation failed", exc_info=True)
raise SpecValidationError(str(e)) from e

min_max_keywords = {
"minimum",
"maximum",
"minLength",
"maxLength",
"minProperties",
"maxProperties",
"minItems",
"maxItems",
"exclusiveMinimum",
"exclusiveMaximum",
}
try: # pylint: disable=R
for _key, schema in JsonSchemaFlattener(resource_spec).flatten_schema().items():
Copy link
Contributor

Choose a reason for hiding this comment

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

if u are not using _key could u change it to _

Copy link
Contributor

Choose a reason for hiding this comment

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

just to clarify - you are using flattened schema to avoid recursive traversal, since all the subschemas are on the same level?

for property_name, property_details in schema.get("properties", {}).items():
Expand All @@ -168,44 +156,57 @@ def load_resource_spec(resource_spec_file): # pylint: disable=R # noqa: C901
try:
property_type = property_details["type"]
property_keywords = property_details.keys()
for types, allowed_keywords in [
keyword_mappings = [
(
{"integer", "number"},
{
"minimum",
"maximum",
"exclusiveMinimum",
"exclusiveMaximum",
"multipleOf",
Copy link
Contributor

Choose a reason for hiding this comment

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

cant leave the comment above but could u take this structure out and create a static _dict_? it is really hard to read this way.

Copy link
Contributor

Choose a reason for hiding this comment

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

I support this idea too. Also can you create type_specific_keywords from the _dict_ you create for this structure so that all keywords specified in the _dict_ will be automatically available in type_specific_keywords

Copy link
Contributor Author

@PatMyron PatMyron Apr 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Started with dictionary, but integer and number types share JSON schema keywords, and lists/sets cannot be used as Python dictionary keys, so I switched

Switched type_specific_keywords to be automatically generated from the mapping :)

},
),
(
{"string"},
{
"minLength",
"maxLength",
"pattern",
},
),
(
{"object"},
{
"minProperties",
"maxProperties",
"additionalProperties",
"patternProperties",
},
),
(
{"array"},
{
"minItems",
"maxItems",
"additionalItems",
"uniqueItems",
},
),
]:
]
type_specific_keywords = set().union(
*(mapping[1] for mapping in keyword_mappings)
)
for types, allowed_keywords in keyword_mappings:
if (
property_type in types
and min_max_keywords - allowed_keywords & property_keywords
and type_specific_keywords - allowed_keywords
& property_keywords
ammokhov marked this conversation as resolved.
Show resolved Hide resolved
):
LOG.warning(
"Incorrect min/max JSON schema keywords for type: %s for property: %s",
"Incorrect JSON schema keyword(s) %s for type: %s for property: %s",
type_specific_keywords - allowed_keywords
& property_keywords,
Comment on lines +208 to +209
Copy link
Contributor

@priyap286 priyap286 Apr 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Can you put this in a variable and use it above and here?

property_type,
property_name,
)
Expand Down