Add additional checks on structured properties to not leak the additi…

…onal properties to handlers during validation (#642)

src/rpdk/core/data_loaders.py

@@ -74,6 +74,38 @@ def make_resource_validator():
     return make_validator(schema)
 
 
+def make_resource_validator_with_additional_properties_check():
+    schema = resource_json(__name__, "data/schema/provider.definition.schema.v1.json")
+    dependencies = schema["definitions"]["validations"]["dependencies"]
+    properties_check = {
+        "properties": {
+            "$comment": "An object cannot have both defined and undefined \
+properties; therefore, patternProperties is not allowed when properties is specified.\
+ Provider should mark additionalProperties as false if the \
+property is of object type and has properties defined \
+in it.",
+            "not": {"required": ["patternProperties"]},
+            "required": ["additionalProperties"],
+        }
+    }
+    pattern_properties_check = {
+        "patternProperties": {
+            "$comment": "An object cannot have both defined and undefined \
+properties; therefore, properties is not allowed when patternProperties is specified. \
+Provider should mark additionalProperties as false if the property is of object type \
+and has patternProperties defined in it.",
+            "not": {"required": ["properties"]},
+            "required": ["additionalProperties"],
+        }
+    }
+    schema["definitions"]["validations"]["dependencies"] = {
+        **dependencies,
+        **properties_check,
+        **pattern_properties_check,
+    }
+    return make_validator(schema)
+
+
 def get_file_base_uri(file):
     try:
         name = file.name
@@ -107,12 +139,25 @@ def load_resource_spec(resource_spec_file):  # noqa: C901
         raise SpecValidationError(str(e)) from e
 
     validator = make_resource_validator()
+    additional_properties_validator = (
+        make_resource_validator_with_additional_properties_check()
+    )
     try:
         validator.validate(resource_spec)
     except ValidationError as e:
         LOG.debug("Resource spec validation failed", exc_info=True)
         raise SpecValidationError(str(e)) from e
 
+    try:
+        additional_properties_validator.validate(resource_spec)
+    except ValidationError as e:
+        LOG.warning(
+            "[Warning] Resource spec validation would fail from next \
+major version. Provider should mark additionalProperties as false if the \
+property is of object type and has properties or patternProperties defined \
+in it. Please fix the warnings: %s",
+            str(e),
+        )
     in_readonly = _is_in(resource_spec, "readOnlyProperties")
     in_createonly = _is_in(resource_spec, "createOnlyProperties")
 

tests/data/schema/invalid/invalid_nested_property_object_additionalProperties_true_warning.json

@@ -0,0 +1,22 @@
+{
+    "typeName": "AWS::Valid::TypeName",
+    "description": "a test schema",
+    "properties": {
+        "property1": {
+            "type": "object",
+            "properties": {
+                "property1": {
+                    "type": "integer"
+                }
+            },
+            "additionalProperties": true
+        }
+    },
+    "primaryIdentifier": [
+        "/properties/property1"
+    ],
+    "readOnlyProperties": [
+        "/properties/property1"
+    ],
+    "additionalProperties": false
+}

tests/data/schema/invalid/invalid_pattern_properties_additionalProperties_true_warning.json

@@ -0,0 +1,29 @@
+{
+    "typeName": "AWS::Valid::TypeName",
+    "description": "a test schema",
+    "definitions": {
+        "obj2def": {
+            "type": "object",
+            "patternProperties": {
+                ".*": {
+                    "type": "string"
+                }
+            },
+            "additionalProperties": true
+        }
+    },
+    "properties": {
+        "obj2": {
+            "type": "object",
+            "description": "",
+            "$ref": "#/definitions/obj2def"
+        },
+        "enum1": {
+            "type": "string"
+        }
+    },
+    "primaryIdentifier": [
+        "/properties/enum1"
+    ],
+    "additionalProperties": false
+}

tests/data/schema/valid/valid_nested_property_object.json

@@ -8,7 +8,8 @@
                 "property1": {
                     "type": "integer"
                 }
-            }
+            },
+            "additionalProperties": false
         }
     },
     "primaryIdentifier": [

tests/data/schema/valid/valid_nested_property_object_no_additionalProperties_warning.json

@@ -0,0 +1,21 @@
+{
+    "typeName": "AWS::Valid::TypeName",
+    "description": "a test schema",
+    "properties": {
+        "property1": {
+            "type": "object",
+            "properties": {
+                "property1": {
+                    "type": "integer"
+                }
+            }
+        }
+    },
+    "primaryIdentifier": [
+        "/properties/property1"
+    ],
+    "readOnlyProperties": [
+        "/properties/property1"
+    ],
+    "additionalProperties": false
+}

tests/data/schema/valid/valid_pattern_properties_no_additionalProperties_warning.json

@@ -0,0 +1,28 @@
+{
+    "typeName": "AWS::Valid::TypeName",
+    "description": "a test schema",
+    "definitions": {
+        "obj2def": {
+            "type": "object",
+            "patternProperties": {
+                ".*": {
+                    "type": "string"
+                }
+            }
+        }
+    },
+    "properties": {
+        "obj2": {
+            "type": "object",
+            "description": "",
+            "$ref": "#/definitions/obj2def"
+        },
+        "enum1": {
+            "type": "string"
+        }
+    },
+    "primaryIdentifier": [
+        "/properties/enum1"
+    ],
+    "additionalProperties": false
+}

tests/test_data_loaders.py

@@ -98,6 +98,48 @@ def test_load_resource_spec_valid_snippets(example):
         assert load_resource_spec(f)
 
 
+@pytest.mark.parametrize(
+    "schema",
+    [
+        "valid_nested_property_object_no_additionalProperties_warning.json",
+        "valid_pattern_properties_no_additionalProperties_warning.json",
+    ],
+)
+def test_load_resource_spec_object_property_missing_additional_properties(
+    schema, caplog
+):
+    schema = BASEDIR / "data" / "schema" / "valid" / schema
+    with schema.open("r", encoding="utf-8") as f:
+        assert load_resource_spec(f)
+    assert "Resource spec validation would fail from next major version" in caplog.text
+
+
+def test_load_resource_spec_unmodeled_object_property_missing_additional_properties(
+    caplog,
+):
+    schema = BASEDIR / "data" / "schema" / "valid" / "valid_no_properties.json"
+    with schema.open("r", encoding="utf-8") as f:
+        assert load_resource_spec(f)
+    assert (
+        "Resource spec validation would fail from next major version" not in caplog.text
+    )
+
+
+@pytest.mark.parametrize(
+    "schema",
+    [
+        "invalid_nested_property_object_additionalProperties_true_warning.json",
+        "invalid_pattern_properties_additionalProperties_true_warning.json",
+    ],
+)
+def test_load_resource_spec_object_property_additional_properties_true(schema):
+    schema = BASEDIR / "data" / "schema" / "invalid" / schema
+    with schema.open("r", encoding="utf-8") as f:
+        with pytest.raises(SpecValidationError) as excinfo:
+            load_resource_spec(f)
+    assert "False was expected" in str(excinfo.value)
+
+
 @pytest.mark.parametrize(
     "example", json_files_params(BASEDIR / "data" / "schema" / "invalid")
 )