From e1df98295706eb014322a06b9507b0807e86e294 Mon Sep 17 00:00:00 2001 From: Michael Gentry Date: Sat, 9 Jul 2022 15:56:24 -0400 Subject: [PATCH 1/2] #841 add top level property definition validation --- src/rpdk/core/data_loaders.py | 50 +++++++++++++++++++ ...id_additional_identifiers_not_defined.json | 16 ++++++ ...id_create_only_properties_not_defined.json | 16 ++++++ ...lid_deprecated_properties_not_defined.json | 16 ++++++ ...nvalid_primary_identifier_not_defined.json | 13 +++++ ...alid_read_only_properties_not_defined.json | 16 ++++++ .../invalid/invalid_required_not_defined.json | 16 ++++++ ...lid_write_only_properties_not_defined.json | 16 ++++++ tests/test_data_loaders.py | 46 +++++++++++++++++ 9 files changed, 205 insertions(+) create mode 100644 tests/data/schema/invalid/invalid_additional_identifiers_not_defined.json create mode 100644 tests/data/schema/invalid/invalid_create_only_properties_not_defined.json create mode 100644 tests/data/schema/invalid/invalid_deprecated_properties_not_defined.json create mode 100644 tests/data/schema/invalid/invalid_primary_identifier_not_defined.json create mode 100644 tests/data/schema/invalid/invalid_read_only_properties_not_defined.json create mode 100644 tests/data/schema/invalid/invalid_required_not_defined.json create mode 100644 tests/data/schema/invalid/invalid_write_only_properties_not_defined.json diff --git a/src/rpdk/core/data_loaders.py b/src/rpdk/core/data_loaders.py index 069cfada..0948eb70 100644 --- a/src/rpdk/core/data_loaders.py +++ b/src/rpdk/core/data_loaders.py @@ -299,6 +299,8 @@ def load_resource_spec(resource_spec_file): # pylint: disable=R # noqa: C901 read_only_properties_intersection, ) + verify_listed_properties_are_defined(resource_spec) + for handler in resource_spec.get("handlers", []): for permission in resource_spec.get("handlers", [])[handler]["permissions"]: if "*" in permission: @@ -368,6 +370,54 @@ def load_resource_spec(resource_spec_file): # pylint: disable=R # noqa: C901 return inlined +def verify_listed_properties_are_defined(resource_spec): + LOG.debug("verify_listed_properties_are_defined") + + schema_keys = [ + "readOnlyProperties", + "createOnlyProperties", + "primaryIdentifier", + "required", + "additionalIdentifiers", + "deprecatedProperties", + "writeOnlyProperties", + ] + for schema_key in schema_keys: + verify_top_level_properties_are_defined(resource_spec, schema_key) + +def verify_top_level_properties_are_defined(resource_spec, schema_key): + LOG.debug("verify_top_level_properties_are_defined: %s", schema_key) + + properties = get_properties_from_schema_key(resource_spec, schema_key) + + nested_properties = filter(lambda property: "/" in property, properties) + top_level_properties = properties - nested_properties + + if len(top_level_properties): + defined_properties = get_defined_properties(resource_spec) + nondefined_top_level_properties = top_level_properties - defined_properties + + if nondefined_top_level_properties: + raise SpecValidationError( + get_properties_not_defined_error_message(schema_key, nondefined_top_level_properties) + ) + +def get_defined_properties(resource_spec): + return set(resource_spec.get("properties", [])) + +def get_properties_from_schema_key(resource_spec, schema_key): + if schema_key == "additionalIdentifiers": + # additionalIdentifiers is a 2 dimensional list, we need to flatten it + properties = resource_spec.get(schema_key, [[]]) + properties = {prop for identifier in properties for prop in identifier} + else: + properties = resource_spec.get(schema_key, []) + + return set(map(lambda property: property.replace("/properties/", ""), properties)) + +def get_properties_not_defined_error_message(key, properties): + return f"The following properties are listed in '{key}' but not defined in 'properties': {', '.join(properties)}" + def load_hook_spec(hook_spec_file): # pylint: disable=R # noqa: C901 """Load a hook definition from a file, and validate it.""" diff --git a/tests/data/schema/invalid/invalid_additional_identifiers_not_defined.json b/tests/data/schema/invalid/invalid_additional_identifiers_not_defined.json new file mode 100644 index 00000000..f2f01556 --- /dev/null +++ b/tests/data/schema/invalid/invalid_additional_identifiers_not_defined.json @@ -0,0 +1,16 @@ +{ + "typeName": "AWS::Valid::TypeName", + "description": "a test schema", + "properties": { + "property1": { + "type": "string" + } + }, + "primaryIdentifier": [ + "/properties/property1" + ], + "additionalIdentifiers": [ + [ "/properties/property2" ] + ], + "additionalProperties": false +} diff --git a/tests/data/schema/invalid/invalid_create_only_properties_not_defined.json b/tests/data/schema/invalid/invalid_create_only_properties_not_defined.json new file mode 100644 index 00000000..c673294e --- /dev/null +++ b/tests/data/schema/invalid/invalid_create_only_properties_not_defined.json @@ -0,0 +1,16 @@ +{ + "typeName": "AWS::Valid::TypeName", + "description": "a test schema", + "properties": { + "property1": { + "type": "string" + } + }, + "primaryIdentifier": [ + "/properties/property1" + ], + "createOnlyProperties": [ + "/properties/property2" + ], + "additionalProperties": false +} diff --git a/tests/data/schema/invalid/invalid_deprecated_properties_not_defined.json b/tests/data/schema/invalid/invalid_deprecated_properties_not_defined.json new file mode 100644 index 00000000..0077f075 --- /dev/null +++ b/tests/data/schema/invalid/invalid_deprecated_properties_not_defined.json @@ -0,0 +1,16 @@ +{ + "typeName": "AWS::Valid::TypeName", + "description": "a test schema", + "properties": { + "property1": { + "type": "string" + } + }, + "primaryIdentifier": [ + "/properties/property1" + ], + "deprecatedProperties": [ + "/properties/property2" + ], + "additionalProperties": false +} diff --git a/tests/data/schema/invalid/invalid_primary_identifier_not_defined.json b/tests/data/schema/invalid/invalid_primary_identifier_not_defined.json new file mode 100644 index 00000000..e668aba4 --- /dev/null +++ b/tests/data/schema/invalid/invalid_primary_identifier_not_defined.json @@ -0,0 +1,13 @@ +{ + "typeName": "AWS::Valid::TypeName", + "description": "a test schema", + "properties": { + "property1": { + "type": "string" + } + }, + "primaryIdentifier": [ + "/properties/property2" + ], + "additionalProperties": false +} diff --git a/tests/data/schema/invalid/invalid_read_only_properties_not_defined.json b/tests/data/schema/invalid/invalid_read_only_properties_not_defined.json new file mode 100644 index 00000000..38852bed --- /dev/null +++ b/tests/data/schema/invalid/invalid_read_only_properties_not_defined.json @@ -0,0 +1,16 @@ +{ + "typeName": "AWS::Valid::TypeName", + "description": "a test schema", + "properties": { + "property1": { + "type": "string" + } + }, + "primaryIdentifier": [ + "/properties/property1" + ], + "readOnlyProperties": [ + "/properties/property2" + ], + "additionalProperties": false +} diff --git a/tests/data/schema/invalid/invalid_required_not_defined.json b/tests/data/schema/invalid/invalid_required_not_defined.json new file mode 100644 index 00000000..16d573b2 --- /dev/null +++ b/tests/data/schema/invalid/invalid_required_not_defined.json @@ -0,0 +1,16 @@ +{ + "typeName": "AWS::Valid::TypeName", + "description": "a test schema", + "properties": { + "property1": { + "type": "string" + } + }, + "primaryIdentifier": [ + "/properties/property1" + ], + "required": [ + "property2" + ], + "additionalProperties": false +} diff --git a/tests/data/schema/invalid/invalid_write_only_properties_not_defined.json b/tests/data/schema/invalid/invalid_write_only_properties_not_defined.json new file mode 100644 index 00000000..1301f2af --- /dev/null +++ b/tests/data/schema/invalid/invalid_write_only_properties_not_defined.json @@ -0,0 +1,16 @@ +{ + "typeName": "AWS::Valid::TypeName", + "description": "a test schema", + "properties": { + "property1": { + "type": "string" + } + }, + "primaryIdentifier": [ + "/properties/property1" + ], + "writeOnlyProperties": [ + "/properties/property2" + ], + "additionalProperties": false +} diff --git a/tests/test_data_loaders.py b/tests/test_data_loaders.py index 23a2e83c..d44dd711 100644 --- a/tests/test_data_loaders.py +++ b/tests/test_data_loaders.py @@ -211,6 +211,52 @@ def test_load_resource_spec_invalid_snippets(example): with pytest.raises(SpecValidationError): load_resource_spec(f) +@pytest.mark.parametrize( + "test_props", + [ + { + 'schema': "invalid_read_only_properties_not_defined.json", + 'schema_key': "readOnlyProperties", + 'property': "properties/property2", + }, + { + 'schema': "invalid_create_only_properties_not_defined.json", + 'schema_key': "createOnlyProperties", + 'property': "properties/property2", + }, + { + 'schema': "invalid_primary_identifier_not_defined.json", + 'schema_key': "primaryIdentifier", + 'property': "properties/property2", + }, + { + 'schema': "invalid_required_not_defined.json", + 'schema_key': "required", + 'property': "property2", + }, + { + 'schema': "invalid_additional_identifiers_not_defined.json", + 'schema_key': "additionalIdentifiers", + 'property': "/properties/property2", + }, + { + 'schema': "invalid_deprecated_properties_not_defined.json", + 'schema_key': "deprecatedProperties", + 'property': "/properties/property2", + }, + { + 'schema': "invalid_write_only_properties_not_defined.json", + 'schema_key': "writeOnlyProperties", + 'property': "/properties/property2", + }, + ], +) +def test_load_resource_spec_property_not_defined(test_props): + schema = BASEDIR / "data" / "schema" / "invalid" / test_props["schema"] + with schema.open("r", encoding="utf-8") as f: + with pytest.raises(SpecValidationError) as excinfo: + load_resource_spec(f) + assert f"The following properties are listed in '{test_props['schema_key']}' but not defined in '{test_props['property']}': /properties/property2"in str(excinfo.value) def test_load_resource_spec_remote_key_is_invalid(): schema = { From f9c6b71bdc9fcd3cb565f4333f2afc3cb315b980 Mon Sep 17 00:00:00 2001 From: Michael Gentry Date: Sat, 9 Jul 2022 16:00:37 -0400 Subject: [PATCH 2/2] nested_properties should be of data type set --- src/rpdk/core/data_loaders.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rpdk/core/data_loaders.py b/src/rpdk/core/data_loaders.py index 0948eb70..b1082ca9 100644 --- a/src/rpdk/core/data_loaders.py +++ b/src/rpdk/core/data_loaders.py @@ -390,7 +390,7 @@ def verify_top_level_properties_are_defined(resource_spec, schema_key): properties = get_properties_from_schema_key(resource_spec, schema_key) - nested_properties = filter(lambda property: "/" in property, properties) + nested_properties = set(filter(lambda property: "/" in property, properties)) top_level_properties = properties - nested_properties if len(top_level_properties):