catch common resource schema issues (#663)

resource types with LIST API options like MaxResults, NextToken, Filters, etc as properties of the resource itself: aws-cloudformation/aws-cloudformation-resource-providers-licensemanager#3 aws-cloudformation/aws-cloudformation-resource-providers-iotwireless#3 aws-cloudformation/aws-cloudformation-resource-providers-ssm#75 readOnlyProperties overlapping with writeOnlyProperties or createOnlyProperties: aws-cloudformation/aws-cloudformation-resource-providers-auditmanager#2 aws-cloudformation/aws-cloudformation-resource-providers-codeartifact#38 no wildcards in handler permissions: aws-cloudformation/aws-cloudformation-resource-providers-greengrassv2#4 aws-cloudformation/aws-cloudformation-resource-providers-imagebuilder@55fa9bf#r46035310
aws-cloudformation · Jan 20, 2021 · 197b3b1 · 197b3b1
1 parent b1dfe2d
commit 197b3b1
Show file tree

Hide file tree

Showing 6 changed files with 93 additions and 16 deletions.
diff --git a/.pylintrc b/.pylintrc
@@ -21,4 +21,4 @@ good-names=e,ex,f,fp,i,j,k,n,_
 [FORMAT]
 
 indent-string='    '
-max-line-length=88
+max-line-length=160
diff --git a/setup.cfg b/setup.cfg
@@ -14,7 +14,7 @@ exclude =
     .eggs,
     .tox
 max-complexity = 10
-max-line-length = 80
+max-line-length = 160
 select = C,E,F,W,B,B950
 # C812, C815, W503 clash with black
 ignore = E501,C812,C816,C815,W503

diff --git a/src/rpdk/core/data_loaders.py b/src/rpdk/core/data_loaders.py
@@ -123,14 +123,7 @@ def get_file_base_uri(file):
     return path.resolve().as_uri()
 
 
-def _is_in(schema, key):
-    def contains(value):
-        return value in schema.get(key, [])
-
-    return contains
-
-
-def load_resource_spec(resource_spec_file):  # noqa: C901
+def load_resource_spec(resource_spec_file):  # pylint: disable=R0912 # noqa: C901
     """Load a resource provider definition from a file, and validate it."""
     try:
         resource_spec = json.load(resource_spec_file)
@@ -148,6 +141,40 @@ def load_resource_spec(resource_spec_file):  # noqa: C901
         LOG.debug("Resource spec validation failed", exc_info=True)
         raise SpecValidationError(str(e)) from e
 
+    list_options = {
+        "maxresults",
+        "maxrecords",
+        "maxitems",
+        "nexttoken",
+        "nextmarker",
+        "nextpagetoken",
+        "pagetoken",
+        "paginationtoken",
+    } & set(map(str.lower, resource_spec.get("properties", [])))
+    if list_options:
+        LOG.warning(
+            "LIST API inputs like MaxResults, MaxRecords, MaxItems, NextToken, NextMarker, NextPageToken, PageToken, and Filters are not resource properties. \
+%s should not be present in resource schema",
+            list_options,
+        )
+
+    if set(resource_spec.get("readOnlyProperties", [])) & set(
+        resource_spec.get("createOnlyProperties", [])
+    ) or set(resource_spec.get("readOnlyProperties", [])) & set(
+        resource_spec.get("writeOnlyProperties", [])
+    ):
+        LOG.warning(
+            "readOnlyProperties cannot be specified by customers and should not overlap with writeOnlyProperties or createOnlyProperties"
+        )
+
+    for handler in resource_spec.get("handlers", []):
+        for permission in resource_spec.get("handlers", [])[handler]["permissions"]:
+            if "*" in permission:
+                LOG.warning(
+                    "Use specific handler permissions instead of using wildcards: %s",
+                    permission,
+                )
+
     try:
         additional_properties_validator.validate(resource_spec)
     except ValidationError as e:
@@ -158,13 +185,11 @@ def load_resource_spec(resource_spec_file):  # noqa: C901
 in it. Please fix the warnings: %s",
             str(e),
         )
-    in_readonly = _is_in(resource_spec, "readOnlyProperties")
-    in_createonly = _is_in(resource_spec, "createOnlyProperties")
-
-    primary_ids = resource_spec["primaryIdentifier"]
 
-    for primary_id in primary_ids:
-        if not in_readonly(primary_id) and not in_createonly(primary_id):
+    for primary_id in resource_spec["primaryIdentifier"]:
+        if primary_id not in resource_spec.get(
+            "readOnlyProperties", []
+        ) and primary_id not in resource_spec.get("createOnlyProperties", []):
             LOG.warning(
                 "Property 'primaryIdentifier' - %s must be specified \
 as either readOnly or createOnly",

diff --git a/tests/data/schema/valid/invalid_list_api_options_as_properties.json b/tests/data/schema/valid/invalid_list_api_options_as_properties.json
@@ -0,0 +1,14 @@
+{
+  "typeName" : "AWS::Service::Type",
+  "description" : "",
+  "additionalProperties" : false,
+  "properties" : {
+    "NextToken" : {
+      "type" : "string"
+    }
+  },
+  "readOnlyProperties": [
+    "/properties/NextToken"
+  ],
+  "primaryIdentifier" : [ "/properties/NextToken" ]
+}
diff --git a/tests/data/schema/valid/invalid_readOnlyProperties_overlap.json b/tests/data/schema/valid/invalid_readOnlyProperties_overlap.json
@@ -0,0 +1,17 @@
+{
+  "typeName" : "AWS::Service::Type",
+  "description" : "",
+  "additionalProperties" : false,
+  "properties" : {
+    "Property" : {
+      "type" : "string"
+    }
+  },
+  "readOnlyProperties": [
+    "/properties/Property"
+  ],
+  "createOnlyProperties": [
+    "/properties/Property"
+  ],
+  "primaryIdentifier" : [ "/properties/Property" ]
+}
diff --git a/tests/data/schema/valid/invalid_wildcard_handler_permissions.json b/tests/data/schema/valid/invalid_wildcard_handler_permissions.json
@@ -0,0 +1,21 @@
+{
+  "typeName" : "AWS::Service::Type",
+  "description" : "",
+  "additionalProperties" : false,
+  "properties" : {
+    "Property" : {
+      "type" : "string"
+    }
+  },
+  "readOnlyProperties": [
+    "/properties/Property"
+  ],
+  "primaryIdentifier" : [ "/properties/Property" ],
+  "handlers": {
+    "create": {
+      "permissions": [
+        "service:create*"
+      ]
+    }
+  }
+}