diff --git a/setup.py b/setup.py index f0e9abb18b..09bad6f468 100644 --- a/setup.py +++ b/setup.py @@ -45,7 +45,7 @@ def get_version(filename): 'pyyaml>5.4', 'aws-sam-translator>=1.49.0', 'jsonpatch', - 'jsonschema~=3.0', + 'jsonschema~=3.2', 'importlib_resources>=1.4,<4;python_version<"3.7"', 'networkx~=2.4', 'junit-xml~=1.9', diff --git a/src/cfnlint/data/ResourceSchemas/AWS::DynamoDB::Table.json b/src/cfnlint/data/ResourceSchemas/AWS::DynamoDB::Table.json new file mode 100644 index 0000000000..4fa4587fea --- /dev/null +++ b/src/cfnlint/data/ResourceSchemas/AWS::DynamoDB::Table.json @@ -0,0 +1,377 @@ +{ + "typeName" : "AWS::DynamoDB::Table", + "description" : "Version: None. Resource Type definition for AWS::DynamoDB::Table", + "additionalProperties" : false, + "properties" : { + "Arn" : { + "type" : "string" + }, + "StreamArn" : { + "type" : "string" + }, + "AttributeDefinitions" : { + "type" : "array", + "uniqueItems" : true, + "items" : { + "$ref" : "#/definitions/AttributeDefinition" + } + }, + "BillingMode" : { + "type" : "string" + }, + "GlobalSecondaryIndexes" : { + "type" : "array", + "uniqueItems" : false, + "items" : { + "$ref" : "#/definitions/GlobalSecondaryIndex" + } + }, + "KeySchema" : { + "oneOf": [ + { + "type" : "array", + "uniqueItems" : true, + "items" : { + "$ref" : "#/definitions/KeySchema" + } + }, + { + "type": "object" + } + ] + }, + "LocalSecondaryIndexes" : { + "type" : "array", + "uniqueItems" : false, + "items" : { + "$ref" : "#/definitions/LocalSecondaryIndex" + } + }, + "PointInTimeRecoverySpecification" : { + "$ref" : "#/definitions/PointInTimeRecoverySpecification" + }, + "TableClass" : { + "type" : "string" + }, + "ProvisionedThroughput" : { + "$ref" : "#/definitions/ProvisionedThroughput" + }, + "SSESpecification" : { + "$ref" : "#/definitions/SSESpecification" + }, + "StreamSpecification" : { + "$ref" : "#/definitions/StreamSpecification" + }, + "TableName" : { + "type" : "string" + }, + "Tags" : { + "type" : "array", + "uniqueItems" : false, + "items" : { + "$ref" : "#/definitions/Tag" + } + }, + "TimeToLiveSpecification" : { + "$ref" : "#/definitions/TimeToLiveSpecification" + }, + "ContributorInsightsSpecification": { + "$ref": "#/definitions/ContributorInsightsSpecification" + }, + "KinesisStreamSpecification": { + "$ref": "#/definitions/KinesisStreamSpecification" + } + }, + "propertyTransform": { + "/properties/SSESpecification/KMSMasterKeyId": "$join([\"arn:(aws)[-]{0,1}[a-z]{0,2}[-]{0,1}[a-z]{0,3}:kms:[a-z]{2}[-]{1}[a-z]{4,10}[-]{1}[1-3]{1}:[0-9]{12}[:]{1}key\\/\", SSESpecification.KMSMasterKeyId])" + }, + "definitions" : { + "StreamSpecification" : { + "type" : "object", + "additionalProperties" : false, + "properties" : { + "StreamViewType" : { + "type" : "string" + } + }, + "required" : [ "StreamViewType" ] + }, + "DeprecatedKeySchema" : { + "type" : "object", + "additionalProperties" : false, + "properties" : { + "HashKeyElement" : { + "$ref" : "#/definitions/DeprecatedHashKeyElement" + } + }, + "required" : [ "HashKeyElement" ] + }, + "DeprecatedHashKeyElement" : { + "type" : "object", + "additionalProperties" : false, + "properties" : { + "AttributeType" : { + "type" : "string" + }, + "AttributeName" : { + "type" : "string" + } + }, + "required" : [ "AttributeType", "AttributeName" ] + }, + "KeySchema" : { + "type" : "object", + "additionalProperties" : false, + "properties" : { + "AttributeName" : { + "type" : "string" + }, + "KeyType" : { + "type" : "string" + } + }, + "required" : [ "KeyType", "AttributeName" ] + }, + "PointInTimeRecoverySpecification" : { + "type" : "object", + "additionalProperties" : false, + "properties" : { + "PointInTimeRecoveryEnabled" : { + "type" : "boolean" + } + } + }, + "KinesisStreamSpecification" : { + "type" : "object", + "additionalProperties" : false, + "properties" : { + "StreamArn" : { + "type" : "string" + } + }, + "required" : [ "StreamArn" ] + }, + "TimeToLiveSpecification" : { + "type" : "object", + "additionalProperties" : false, + "properties" : { + "AttributeName" : { + "type" : "string" + }, + "Enabled" : { + "type" : "boolean" + } + }, + "required" : [ "Enabled", "AttributeName" ] + }, + "LocalSecondaryIndex" : { + "type" : "object", + "additionalProperties" : false, + "properties" : { + "IndexName" : { + "type" : "string" + }, + "KeySchema" : { + "type" : "array", + "uniqueItems" : true, + "items" : { + "$ref" : "#/definitions/KeySchema" + } + }, + "Projection" : { + "$ref" : "#/definitions/Projection" + } + }, + "required" : [ "IndexName", "Projection", "KeySchema" ] + }, + "GlobalSecondaryIndex" : { + "type" : "object", + "additionalProperties" : false, + "properties" : { + "IndexName" : { + "type" : "string" + }, + "KeySchema" : { + "type" : "array", + "uniqueItems" : true, + "items" : { + "$ref" : "#/definitions/KeySchema" + } + }, + "Projection" : { + "$ref" : "#/definitions/Projection" + }, + "ProvisionedThroughput" : { + "$ref" : "#/definitions/ProvisionedThroughput" + }, + "ContributorInsightsSpecification": { + "$ref": "#/definitions/ContributorInsightsSpecification" + } + }, + "required" : [ "IndexName", "Projection", "KeySchema" ] + }, + "SSESpecification" : { + "type" : "object", + "additionalProperties" : false, + "properties" : { + "KMSMasterKeyId" : { + "type" : "string" + }, + "SSEEnabled" : { + "type" : "boolean" + }, + "SSEType" : { + "type" : "string" + } + }, + "required" : [ "SSEEnabled" ] + }, + "AttributeDefinition" : { + "type" : "object", + "additionalProperties" : false, + "properties" : { + "AttributeName" : { + "type" : "string" + }, + "AttributeType" : { + "type" : "string" + } + }, + "required" : [ "AttributeName", "AttributeType" ] + }, + "ProvisionedThroughput" : { + "type" : "object", + "additionalProperties" : false, + "properties" : { + "ReadCapacityUnits" : { + "type" : "integer" + }, + "WriteCapacityUnits" : { + "type" : "integer" + } + }, + "required" : [ "WriteCapacityUnits", "ReadCapacityUnits" ] + }, + "Tag" : { + "type" : "object", + "additionalProperties" : false, + "properties" : { + "Key" : { + "type" : "string" + }, + "Value" : { + "type" : "string" + } + }, + "required" : [ "Value", "Key" ] + }, + "Projection" : { + "type" : "object", + "additionalProperties" : false, + "properties" : { + "NonKeyAttributes" : { + "type" : "array", + "uniqueItems" : false, + "items" : { + "type" : "string" + } + }, + "ProjectionType" : { + "type" : "string" + } + } + }, + "ContributorInsightsSpecification": { + "type": "object", + "additionalProperties": false, + "properties": { + "Enabled": { + "type": "boolean" + } + }, + "required" : [ "Enabled" ] + } + }, + "tagging": { + "taggable": true, + "tagOnCreate": true, + "tagUpdatable": true, + "cloudFormationSystemTags": false, + "tagProperty": "/properties/Tags" + }, + "required" : [ "KeySchema" ], + "readOnlyProperties" : [ "/properties/Arn", "/properties/StreamArn" ], + "createOnlyProperties" : [ "/properties/TableName" ], + "primaryIdentifier" : [ "/properties/TableName" ], + "handlers": { + "create": { + "permissions": [ + "dynamodb:CreateTable", + "dynamodb:DescribeTable", + "dynamodb:DescribeTimeToLive", + "dynamodb:UpdateTimeToLive", + "dynamodb:UpdateContributorInsights", + "dynamodb:UpdateContinuousBackups", + "dynamodb:DescribeContinuousBackups", + "dynamodb:DescribeContributorInsights", + "dynamodb:EnableKinesisStreamingDestination", + "dynamodb:DisableKinesisStreamingDestination", + "dynamodb:DescribeKinesisStreamingDestination", + "dynamodb:ListTagsOfResource", + "dynamodb:TagResource", + "kinesis:DescribeStream", + "kinesis:PutRecords", + "iam:CreateServiceLinkedRole", + "kms:CreateGrant", + "kms:Describe*", + "kms:Get*", + "kms:List*", + "kms:RevokeGrant" + ] + }, + "read": { + "permissions": [ + "dynamodb:DescribeTable", + "dynamodb:DescribeContinuousBackups", + "dynamodb:DescribeContributorInsights" + ] + }, + "update": { + "permissions": [ + "dynamodb:UpdateTable", + "dynamodb:DescribeTable", + "dynamodb:DescribeTimeToLive", + "dynamodb:UpdateTimeToLive", + "dynamodb:UpdateContinuousBackups", + "dynamodb:UpdateContributorInsights", + "dynamodb:DescribeContinuousBackups", + "dynamodb:DescribeKinesisStreamingDestination", + "dynamodb:ListTagsOfResource", + "dynamodb:TagResource", + "dynamodb:UntagResource", + "dynamodb:DescribeContributorInsights", + "dynamodb:EnableKinesisStreamingDestination", + "dynamodb:DisableKinesisStreamingDestination", + "kinesis:DescribeStream", + "kinesis:PutRecords", + "iam:CreateServiceLinkedRole", + "kms:CreateGrant", + "kms:Describe*", + "kms:Get*", + "kms:List*", + "kms:RevokeGrant" + ] + }, + "delete": { + "permissions": [ + "dynamodb:DeleteTable", + "dynamodb:DescribeTable" + ] + }, + "list": { + "permissions": [ + "dynamodb:ListTables" + ] + } + } +} diff --git a/src/cfnlint/data/ResourceSchemas/__init__.py b/src/cfnlint/data/ResourceSchemas/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/cfnlint/helpers.py b/src/cfnlint/helpers.py index 74b7d22bf2..9cb8938fb7 100644 --- a/src/cfnlint/helpers.py +++ b/src/cfnlint/helpers.py @@ -22,7 +22,7 @@ import importlib_resources as pkg_resources import importlib from cfnlint.decode.node import dict_node, list_node, str_node -from cfnlint.data import CloudSpecs +from cfnlint.data import CloudSpecs, ResourceSchemas try: from urllib.request import urlopen, Request except ImportError: @@ -336,6 +336,7 @@ def load_resource(package, filename='us-east-1.json'): RESOURCE_SPECS = {} REGISTRY_SCHEMAS = [] +REGISTRY_SCHEMAS_AWS = {} def merge_spec(source, destination): """ Recursive merge spec dict """ @@ -420,9 +421,12 @@ def initialize_specs(): for reg in REGIONS: RESOURCE_SPECS[reg] = load_resource(CloudSpecs, filename=('%s.json' % reg)) +def initialize_registry_schemas_aws(): + for f in ['AWS::DynamoDB::Table']: + REGISTRY_SCHEMAS_AWS[f] = load_resource(ResourceSchemas, filename=(f'{f}.json')) initialize_specs() - +initialize_registry_schemas_aws() def format_json_string(json_string): """ Format the given JSON string""" diff --git a/src/cfnlint/rules/__init__.py b/src/cfnlint/rules/__init__.py index 4a50e25938..1cdadba933 100644 --- a/src/cfnlint/rules/__init__.py +++ b/src/cfnlint/rules/__init__.py @@ -144,17 +144,20 @@ def wrapper(self, filename, cfn, *args, **kwargs): if results: for result in results: + error_rule = self + if hasattr(result, 'rule'): + error_rule = result.rule linenumbers = cfn.get_location_yaml(cfn.template, result.path) if linenumbers: matches.append(Match( linenumbers[0] + 1, linenumbers[1] + 1, linenumbers[2] + 1, linenumbers[3] + 1, - filename, self, result.message, result)) + filename, error_rule, result.message, result)) else: matches.append(Match( 1, 1, 1, 1, - filename, self, result.message, result)) + filename, error_rule, result.message, result)) return matches return wrapper diff --git a/src/cfnlint/rules/resources/properties/JsonSchema.py b/src/cfnlint/rules/resources/properties/JsonSchema.py new file mode 100644 index 0000000000..a4518e8b48 --- /dev/null +++ b/src/cfnlint/rules/resources/properties/JsonSchema.py @@ -0,0 +1,269 @@ +""" +Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +SPDX-License-Identifier: MIT-0 +""" +import re +from copy import deepcopy +import jsonschema +from cfnlint.rules import CloudFormationLintRule +from cfnlint.rules import RuleMatch +from cfnlint.template import Template +import cfnlint.helpers + + +class Rules(object): + + def __init__(self): + setattr(self, 'additionalProperties', Rules.JsonSchemaAdditionalProperties) + setattr(self, 'required', Rules.JsonSchemaRequired) + setattr(self, 'enum', Rules.JsonSchemaRequired) + setattr(self, 'type', Rules.JsonSchemaType) + setattr(self, 'minLength', Rules.JsonSchemaLength) + setattr(self, 'maxLength', Rules.JsonSchemaLength) + + class JsonSchemaAdditionalProperties(CloudFormationLintRule): + id = 'E3002New' + + class JsonSchemaRequired(CloudFormationLintRule): + id = 'E3003New' + + class JsonSchemaType(CloudFormationLintRule): + id = 'E3002New' + + class JsonSchemaLength(CloudFormationLintRule): + id = 'E3033New' + + +class JsonSchema(CloudFormationLintRule): + """Check Base Resource Configuration""" + id = 'E3XXX' + shortdesc = 'Parent rule for doing JSON schema validation' + description = 'Making sure that resources properties comply with their JSON schema' + source_url = 'https://github.com/aws-cloudformation/cfn-python-lint/blob/main/docs/cfn-resource-specification.md#properties' + tags = ['resources'] + validator = None + + def __init__(self): + """Init""" + super(JsonSchema, self).__init__() + self.cfn = {} + custom_draft7_validator = jsonschema.validators.extend( + jsonschema.Draft7Validator, + validators={ + 'required': self.validate_required, + 'type': self.validate_type, + 'additionalProperties': self.validate_additional_properties, + }, + type_checker=jsonschema.Draft7Validator.TYPE_CHECKER.redefine_many( + {'object': is_object, 'intrinsic': is_intrinsic} + ), + ) + self.rules = Rules() + + class CfnValidator(custom_draft7_validator): + def descend(self, instance, schema, path=None, schema_path=None): + if isinstance(instance, dict): + if len(instance) == 1: + for k, v in instance.items(): + if k == 'Fn::If': + if isinstance(v, list) and len(v) == 3: + for error in self.descend(v[1], schema, path, schema_path): + if path is not None: + error.path.append('Fn::If') + error.path.append(1) + if schema_path is not None: + error.schema_path.appendleft(schema_path) + yield error + for error in self.descend(v[2], schema, path, schema_path): + if path is not None: + error.path.append('Fn::If') + error.path.append(2) + if schema_path is not None: + error.schema_path.appendleft(schema_path) + yield error + return + for error in self.iter_errors(instance, schema): + if path is not None: + error.path.appendleft(path) + if schema_path is not None: + error.schema_path.appendleft(schema_path) + yield error + + self.validator = CfnValidator + + + def validate_type(self, validator, types, instance, schema): + types = ensure_list(types) + if isinstance(instance, dict): + if len(instance) == 1: + for k, v in instance.items(): + if k == 'Fn::If': + if isinstance(v, list) and len(v) == 3: + for error in self.validate_type(validator, types, v[1], schema): + yield error + for error in self.validate_type(validator, types, v[2], schema): + yield error + return + if not any(validator.is_type(instance, type) for type in types): + if (isinstance(instance, dict)): + if len(instance) == 1: + for k, v in instance.items(): + if k == 'Fn::Sub': + for t in types: + if t in ['string']: # sub is a string, number + return + reprs = ', '.join(repr(type) for type in types) + yield jsonschema.ValidationError(f'{instance!r} is not of type {reprs}') + + #pylint: disable=unused-argument + def validate_required(self, validator, required, instance, schema): + if not validator.is_type(instance, 'object'): + return + for p in required: + if p not in instance: + yield jsonschema.ValidationError(f'{p!r} is a required property') + + def validate_additional_properties(self, validator, aP, instance, schema): + if not validator.is_type(instance, 'object'): + return + + extras = set(find_additional_properties(instance, schema)) + + if validator.is_type(aP, 'object'): + for extra in extras: + yield from validator.descend(instance[extra], aP, path=extra) + elif not aP and extras: + if 'patternProperties' in schema: + if len(extras) == 1: + verb = 'does' + else: + verb = 'do' + + joined = ', '.join(repr(each) for each in sorted(extras)) + patterns = ', '.join( + repr(each) for each in sorted(schema['patternProperties']) + ) + error = f'{joined} {verb} not match any of the regexes: {patterns}' + yield jsonschema.ValidationError(error) + else: + for extra in extras: + error = 'Additional properties are not allowed (%s unexpected)' + yield jsonschema.ValidationError(error % extra, path=[extra]) + + def json_schema_validate(self, validator, properties, path): + matches = [] + errors = validator.iter_errors(properties) + for e in errors: + matches.append( + RuleMatch(path + list(e.path), e.message, rule=getattr(self.rules, e.validator))) + + return matches + + + def clense_schema(self, schema): + """Taking a valid schema we will modify it for additional CloudFormation type things""" + for ro_prop in schema.get('readOnlyProperties', []): + sub_schema = schema + for p in ro_prop.split('/')[1:-1]: + sub_schema = sub_schema.get(p) + del sub_schema[ro_prop.split('/')[-1]] + + return schema + + + def match(self, cfn: Template): + """Check CloudFormation Properties""" + matches = [] + self.cfn = cfn + + resourcespecs = cfnlint.helpers.REGISTRY_SCHEMAS_AWS + for n, values in cfn.get_resources().items(): + p = values.get('Properties', {}) + t = values.get('Type', None) + if p and t: + if t in resourcespecs: + schema = self.clense_schema(deepcopy(resourcespecs[t])) + cfn_validator = self.validator(schema) + path = ['Resources', n, 'Properties'] + for scenario in cfn.get_object_without_nested_conditions(p, path): + matches.extend(self.json_schema_validate( + cfn_validator, scenario.get('Object'), path)) + return matches + + +#pylint: disable=unused-argument +def is_object(checker, instance): + """ + 'object' type definition + Overloaded to exclude intrinsic functions + Parameters + ---------- + checker : dict + Checker + instance : element + Template element + Returns + ------- + boolean + True if an object, False otherwise + """ + return isinstance(instance, dict) and not has_intrinsic_attr(instance) + +#pylint: disable=unused-argument +def is_intrinsic(checker, instance): + """ + 'intrinsic' type definition + Parameters + ---------- + checker : dict + [description] + instance : [type] + [description] + Returns + ------- + [type] + [description] + """ + return isinstance(instance, dict) and has_intrinsic_attr(instance) + +def has_intrinsic_attr(instance): + """ + Returns a value indicating whether the instance has an intrinsic attribute + Only one attribute which must be one of the intrinsics + Parameters + ---------- + instance : dict + Dictionary + Returns + ------- + boolean + True if only has one intrinsic attribute, False otherwise + """ + return len(instance) == 1 and next(iter(instance)) in cfnlint.helpers.FUNCTIONS + +def find_additional_properties(instance, schema): + """ + Return the set of additional properties for the given ``instance``. + Weeds out properties that should have been validated by ``properties`` and + / or ``patternProperties``. + Assumes ``instance`` is dict-like already. + """ + + properties = schema.get('properties', {}) + patterns = '|'.join(schema.get('patternProperties', {})) + for p in instance: + if p not in properties: + if patterns and re.search(patterns, p): + continue + yield p + + +def ensure_list(thing): + """ + Wrap ``thing`` in a list if it's a single str. + Otherwise, return it unchanged. + """ + + if isinstance(thing, str): + return [thing] + return thing diff --git a/src/cfnlint/template.py b/src/cfnlint/template.py index 91b3cc4564..90a6ab4373 100644 --- a/src/cfnlint/template.py +++ b/src/cfnlint/template.py @@ -12,6 +12,7 @@ LOGGER = logging.getLogger(__name__) + class Template(object): # pylint: disable=R0904,too-many-lines """Class for a CloudFormation template""" @@ -727,7 +728,9 @@ def get_value(value, scenario): # pylint: disable=R0911 new_object = {} for k, v in value.items(): - new_object[k] = get_value(v, scenario) + new_v = get_value(v, scenario) + if new_v is not None: + new_object[k] = get_value(v, scenario) return new_object if isinstance(value, list): new_list = [] diff --git a/test/unit/rules/resources/properties/test_json_schema.py b/test/unit/rules/resources/properties/test_json_schema.py new file mode 100644 index 0000000000..9173a95625 --- /dev/null +++ b/test/unit/rules/resources/properties/test_json_schema.py @@ -0,0 +1,101 @@ +""" +Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +SPDX-License-Identifier: MIT-0 +""" +from copy import deepcopy +from test.unit.rules import BaseRuleTestCase +from cfnlint.rules.resources.properties.JsonSchema import JsonSchema, Rules # pylint: disable=E0401 +import cfnlint.helpers +from cfnlint.rules import RuleMatch + + +class TestJsonSchema(BaseRuleTestCase): + """Test Json Size""" + + def setUp(self): + """Setup""" + super(TestJsonSchema, self).setUp() + self.rule = JsonSchema() + self.table = DynamoDBTable() + + schema = deepcopy(cfnlint.helpers.REGISTRY_SCHEMAS_AWS["AWS::DynamoDB::Table"]) + self.schema = self.rule.clense_schema(schema) + + def test_required_empty(self): + """Remove conditions and still validate""" + + path = ["Resources", "Table", "Properties"] + matches = self.rule.json_schema_validate( + self.rule.validator(self.schema), {}, path) + expected = [ + RuleMatch(path, + "'KeySchema' is a required property", rule=Rules.JsonSchemaRequired) + ] + + self.assertListEqual(list(map(vars, expected)), list(map(vars, matches))) + + def test_required_nested(self): + """Remove conditions and still validate""" + + path = ["Resources", "Table", "Properties"] + matches = self.rule.json_schema_validate( + self.rule.validator(self.schema), self.table.get_object_required(), path) + expected = [ + RuleMatch(path + ["TimeToLiveSpecification"], + "'Enabled' is a required property", rule=Rules.JsonSchemaRequired) + ] + + self.assertListEqual(list(map(vars, expected)), list(map(vars, matches))) + + def test_additional_properties(self): + """Test additional properties""" + + path = ["Resources", "Table", "Properties"] + matches = self.rule.json_schema_validate( + self.rule.validator(self.schema), self.table.get_additional_properties(), path) + expected = [ + RuleMatch(path + ["Arn"], + "Additional properties are not allowed (Arn unexpected)", rule=Rules.JsonSchemaAdditionalProperties), + RuleMatch(path + ["StreamSpecification", "Extra"], + "Additional properties are not allowed (Extra unexpected)", rule=Rules.JsonSchemaAdditionalProperties), + ] + + self.assertListEqual(list(map(vars, expected)), list(map(vars, matches))) + + +class DynamoDBTable(object): + _table = { + "KeySchema": [ + { + "AttributeName": "pk", + "KeyType": "HASH" + } + ], + "TimeToLiveSpecification": { + "AttributeName": "TTL", + "Enabled": True, + }, + "AttributeDefinitions": [ + { + "AttributeName": "pk", + "AttributeType": "S" + } + ] + } + + def get_table(self): + return self._table + + def get_object_required(self): + table = deepcopy(self._table) + del table["TimeToLiveSpecification"]["Enabled"] + return table + + def get_additional_properties(self): + table = deepcopy(self._table) + table["Arn"] = "arn" # Read only + table["StreamSpecification"] = { + "StreamViewType": "KEYS_ONLY", + "Extra": False, + } + return table