added primary id validation

aws-cloudformation · Jul 8, 2020 · eccf0d5 · eccf0d5
1 parent 21f8b29
commit eccf0d5
Show file tree

Hide file tree

Showing 33 changed files with 129 additions and 6 deletions.
diff --git a/src/rpdk/core/data_loaders.py b/src/rpdk/core/data_loaders.py
@@ -91,6 +91,13 @@ def get_file_base_uri(file):
     return path.resolve().as_uri()
 
 
+def _is_in(schema, key):
+    def contains(values):
+        return set(values).issubset(set(schema.get(key, [])))
+
+    return contains
+
+
 def load_resource_spec(resource_spec_file):  # noqa: C901
     """Load a resource provider definition from a file, and validate it."""
     try:
@@ -106,6 +113,16 @@ def load_resource_spec(resource_spec_file):  # noqa: C901
         LOG.debug("Resource spec validation failed", exc_info=True)
         raise SpecValidationError(str(e)) from e
 
+    in_readonly = _is_in(resource_spec, "readOnlyProperties")
+    in_createonly = _is_in(resource_spec, "createOnlyProperties")
+
+    primary_id = resource_spec["primaryIdentifier"]
+    if not in_readonly(primary_id) and not in_createonly(primary_id):
+        raise SpecValidationError(
+            "Property 'primaryIdentifier' must be specified \
+as either readOnly or createOnly"
+        )
+
     # TODO: more general validation framework
     if "remote" in resource_spec:
         raise SpecValidationError(

diff --git a/tests/data/schema/invalid/invalid_additional_items.json b/tests/data/schema/invalid/invalid_additional_items.json
@@ -10,5 +10,8 @@
     "primaryIdentifier": [
         "/properties/property1"
     ],
+    "readOnlyProperties": [
+        "/properties/property1"
+    ],
     "additionalProperties": false
 }
diff --git a/tests/data/schema/invalid/invalid_additional_properties.json b/tests/data/schema/invalid/invalid_additional_properties.json
@@ -9,5 +9,11 @@
             }
         }
     },
+    "primaryIdentifier": [
+        "/properties/property1"
+    ],
+    "readOnlyProperties": [
+        "/properties/property1"
+    ],
     "additionalProperties": false
 }
diff --git a/tests/data/schema/invalid/invalid_additional_properties_true.json b/tests/data/schema/invalid/invalid_additional_properties_true.json
@@ -7,5 +7,11 @@
             "additionalProperties": true
         }
     },
+    "primaryIdentifier": [
+        "/properties/property1"
+    ],
+    "readOnlyProperties": [
+        "/properties/property1"
+    ],
     "additionalProperties": false
 }
diff --git a/tests/data/schema/invalid/invalid_boolean_property.json b/tests/data/schema/invalid/invalid_boolean_property.json
@@ -4,5 +4,11 @@
     "properties": {
         "property1": true
     },
+    "primaryIdentifier": [
+        "/properties/property1"
+    ],
+    "readOnlyProperties": [
+        "/properties/property1"
+    ],
     "additionalProperties": false
 }
diff --git a/tests/data/schema/invalid/invalid_definition_name_invalid.json b/tests/data/schema/invalid/invalid_definition_name_invalid.json
@@ -11,5 +11,11 @@
             "type": "integer"
         }
     },
+    "primaryIdentifier": [
+        "/properties/property1"
+    ],
+    "readOnlyProperties": [
+        "/properties/property1"
+    ],
     "additionalProperties": false
 }
diff --git a/tests/data/schema/invalid/invalid_definition_reference_invalid.json b/tests/data/schema/invalid/invalid_definition_reference_invalid.json
@@ -10,5 +10,8 @@
     "primaryIdentifier": [
         "/properties/property1"
     ],
+    "readOnlyProperties": [
+        "/properties/property1"
+    ],
     "additionalProperties": false
 }
diff --git a/tests/data/schema/invalid/invalid_extraneous_top_level_keys.json b/tests/data/schema/invalid/invalid_extraneous_top_level_keys.json
@@ -14,6 +14,9 @@
     "primaryIdentifier": [
         "/properties/property1"
     ],
+    "readOnlyProperties": [
+        "/properties/property1"
+    ],
     "additionalProperties": false,
     "unsupportedKey": 1
 }
diff --git a/tests/data/schema/invalid/invalid_handler_definitions.json b/tests/data/schema/invalid/invalid_handler_definitions.json
@@ -19,5 +19,8 @@
     "primaryIdentifier": [
         "/properties/property1"
     ],
+    "readOnlyProperties": [
+        "/properties/property1"
+    ],
     "additionalProperties": false
 }
diff --git a/tests/data/schema/invalid/invalid_items_list.json b/tests/data/schema/invalid/invalid_items_list.json
@@ -14,5 +14,8 @@
     "primaryIdentifier": [
         "/properties/property1"
     ],
+    "readOnlyProperties": [
+        "/properties/property1"
+    ],
     "additionalProperties": false
 }
diff --git a/tests/data/schema/invalid/invalid_no_description.json b/tests/data/schema/invalid/invalid_no_description.json
@@ -10,5 +10,8 @@
     "primaryIdentifier": [
         "/properties/property1"
     ],
+    "readOnlyProperties": [
+        "/properties/property1"
+    ],
     "additionalProperties": false
 }
diff --git a/tests/data/schema/invalid/invalid_no_primary_identifier.json b/tests/data/schema/invalid/invalid_no_primary_identifier.json
@@ -7,8 +7,5 @@
             "additionalItems": true
         }
     },
-    "primaryIdentifier": [
-        "/properties/property1"
-    ],
     "additionalProperties": false
 }
diff --git a/tests/data/schema/invalid/invalid_no_properties_defined.json b/tests/data/schema/invalid/invalid_no_properties_defined.json
@@ -5,5 +5,8 @@
     "primaryIdentifier": [
         "/properties/property1"
     ],
+    "readOnlyProperties": [
+        "/properties/property1"
+    ],
     "additionalProperties": false
 }
diff --git a/tests/data/schema/invalid/invalid_no_type_const.json b/tests/data/schema/invalid/invalid_no_type_const.json
@@ -9,5 +9,8 @@
     "primaryIdentifier": [
         "/properties/property1"
     ],
+    "readOnlyProperties": [
+        "/properties/property1"
+    ],
     "additionalProperties": false
 }
diff --git a/tests/data/schema/invalid/invalid_no_type_enum.json b/tests/data/schema/invalid/invalid_no_type_enum.json
@@ -12,5 +12,8 @@
     "primaryIdentifier": [
         "/properties/property1"
     ],
+    "readOnlyProperties": [
+        "/properties/property1"
+    ],
     "additionalProperties": false
 }
diff --git a/tests/data/schema/invalid/invalid_primary_id.json b/tests/data/schema/invalid/invalid_primary_id.json
@@ -0,0 +1,13 @@
+{
+    "typeName": "AWS::Valid::TypeName",
+    "description": "a test schema",
+    "properties": {
+        "property1": {
+            "type": "array"
+        }
+    },
+    "primaryIdentifier": [
+        "/properties/property1"
+    ],
+    "additionalProperties": false
+}
diff --git a/tests/data/schema/invalid/invalid_properties_and_patternproperties.json b/tests/data/schema/invalid/invalid_properties_and_patternproperties.json
@@ -22,5 +22,8 @@
     "primaryIdentifier": [
         "/properties/property1"
     ],
+    "readOnlyProperties": [
+        "/properties/property1"
+    ],
     "additionalProperties": false
 }
diff --git a/tests/data/schema/invalid/invalid_property_name_invalid.json b/tests/data/schema/invalid/invalid_property_name_invalid.json
@@ -8,5 +8,8 @@
     "primaryIdentifier": [
         "/properties/property1"
     ],
+    "readOnlyProperties": [
+        "/properties/property1"
+    ],
     "additionalProperties": false
 }
diff --git a/tests/data/schema/invalid/invalid_property_not_an_object.json b/tests/data/schema/invalid/invalid_property_not_an_object.json
@@ -6,5 +6,8 @@
     "primaryIdentifier": [
         "/properties/property1"
     ],
+    "readOnlyProperties": [
+        "/properties/property1"
+    ],
     "additionalProperties": false
 }
diff --git a/tests/data/schema/invalid/invalid_property_type_invalid.json b/tests/data/schema/invalid/invalid_property_type_invalid.json
@@ -9,5 +9,8 @@
     "primaryIdentifier": [
         "/properties/property1"
     ],
+    "readOnlyProperties": [
+        "/properties/property1"
+    ],
     "additionalProperties": false
 }
diff --git a/tests/data/schema/invalid/invalid_remote_reserved_word.json b/tests/data/schema/invalid/invalid_remote_reserved_word.json
@@ -12,5 +12,8 @@
     "remote": {
         "schema01": {}
     },
+    "readOnlyProperties": [
+        "/properties/property1"
+    ],
     "additionalProperties": false
 }
diff --git a/tests/data/schema/invalid/invalid_typename.json b/tests/data/schema/invalid/invalid_typename.json
@@ -9,5 +9,8 @@
     "primaryIdentifier": [
         "/properties/property1"
     ],
+    "readOnlyProperties": [
+        "/properties/property1"
+    ],
     "additionalProperties": false
 }
diff --git a/tests/data/schema/valid/valid_array_no_items.json b/tests/data/schema/valid/valid_array_no_items.json
@@ -9,5 +9,8 @@
     "primaryIdentifier": [
         "/properties/property1"
     ],
+    "readOnlyProperties": [
+        "/properties/property1"
+    ],
     "additionalProperties": false
 }
diff --git a/tests/data/schema/valid/valid_nested_property_object.json b/tests/data/schema/valid/valid_nested_property_object.json
@@ -14,5 +14,8 @@
     "primaryIdentifier": [
         "/properties/property1"
     ],
+    "readOnlyProperties": [
+        "/properties/property1"
+    ],
     "additionalProperties": false
 }
diff --git a/tests/data/schema/valid/valid_no_properties.json b/tests/data/schema/valid/valid_no_properties.json
@@ -11,7 +11,8 @@
         "/properties/enum1"
     ],
     "readOnlyProperties": [
-        "/properties/str3"
+        "/properties/str3",
+        "/properties/enum1"
     ],
     "additionalProperties": false
 }
diff --git a/tests/data/schema/valid/valid_no_type.json b/tests/data/schema/valid/valid_no_type.json
@@ -19,5 +19,8 @@
     "primaryIdentifier": [
         "/properties/property1"
     ],
+    "readOnlyProperties": [
+        "/properties/property1"
+    ],
     "additionalProperties": false
 }
diff --git a/tests/data/schema/valid/valid_pattern_properties.json b/tests/data/schema/valid/valid_pattern_properties.json
@@ -66,7 +66,8 @@
         "/properties/enum1"
     ],
     "readOnlyProperties": [
-        "/properties/str3"
+        "/properties/str3",
+        "/properties/enum1"
     ],
     "additionalIdentifiers": [
         [

diff --git a/tests/data/schema/valid/valid_type_complex.json b/tests/data/schema/valid/valid_type_complex.json
@@ -46,7 +46,8 @@
         "/properties/enum1"
     ],
     "readOnlyProperties": [
-        "/properties/str3"
+        "/properties/str3",
+        "/properties/enum1"
     ],
     "additionalIdentifiers": [
         [

diff --git a/tests/data/schema/valid/valid_type_composite_primary_identifier.json b/tests/data/schema/valid/valid_type_composite_primary_identifier.json
@@ -13,5 +13,9 @@
         "/properties/property1",
         "/properties/property2"
     ],
+    "readOnlyProperties": [
+        "/properties/property1",
+        "/properties/property2"
+    ],
     "additionalProperties": false
 }
diff --git a/tests/data/schema/valid/valid_type_const.json b/tests/data/schema/valid/valid_type_const.json
@@ -10,5 +10,8 @@
     "primaryIdentifier": [
         "/properties/property1"
     ],
+    "readOnlyProperties": [
+        "/properties/property1"
+    ],
     "additionalProperties": false
 }
diff --git a/tests/data/schema/valid/valid_type_enum.json b/tests/data/schema/valid/valid_type_enum.json
@@ -13,5 +13,8 @@
     "primaryIdentifier": [
         "/properties/property1"
     ],
+    "readOnlyProperties": [
+        "/properties/property1"
+    ],
     "additionalProperties": false
 }
diff --git a/tests/data/schema/valid/valid_with_handlers.json b/tests/data/schema/valid/valid_with_handlers.json
@@ -9,6 +9,9 @@
     "primaryIdentifier": [
         "/properties/property1"
     ],
+    "readOnlyProperties": [
+        "/properties/property1"
+    ],
     "additionalProperties": false,
     "handlers": {
         "create": {

diff --git a/tests/test_data_loaders.py b/tests/test_data_loaders.py
@@ -36,6 +36,7 @@
     "description": "test schema",
     "properties": {"foo": {"type": "string"}},
     "primaryIdentifier": ["/properties/foo"],
+    "readOnlyProperties": ["/properties/foo"],
     "additionalProperties": False,
 }
 
@@ -112,6 +113,7 @@ def test_load_resource_spec_remote_key_is_invalid():
         "description": "test schema",
         "properties": {"foo": {"type": "string"}},
         "primaryIdentifier": ["/properties/foo"],
+        "readOnlyProperties": ["/properties/foo"],
         "remote": {},
     }
     with pytest.raises(SpecValidationError) as excinfo: