From 9d1312381200d469175bd18a93126952a27d19cd Mon Sep 17 00:00:00 2001 From: Kevin DeJong Date: Tue, 21 Jan 2025 11:13:53 -0800 Subject: [PATCH] More cleanup and integration testing --- scripts/update_schemas_format.py | 4 + scripts/update_snapshot_results.sh | 1 + .../all/aws_ec2_securitygroup/format.json | 5 + .../aws-emrserverless-application.json | 1 + .../aws-emrserverless-application.json | 1 + .../aws-emrserverless-application.json | 1 + .../aws-emrserverless-application.json | 1 + .../aws-emrserverless-application.json | 1 + .../aws-emrserverless-application.json | 1 + .../aws-emrserverless-application.json | 1 + .../aws-emrserverless-application.json | 1 + .../aws-emrserverless-application.json | 1 + .../aws-emrserverless-application.json | 1 + .../aws-emrserverless-application.json | 1 + .../aws-emrserverless-application.json | 1 + .../aws-emrserverless-application.json | 1 + .../aws-emrserverless-application.json | 1 + .../aws-emrserverless-application.json | 1 + .../aws-emrserverless-application.json | 1 + .../aws-emrserverless-application.json | 1 + .../us_east_1/aws-ec2-securitygroup.json | 1 + .../aws-emrserverless-application.json | 1 + .../aws-emrserverless-application.json | 1 + .../aws-emrserverless-application.json | 1 + src/cfnlint/rules/functions/Ref.py | 45 ++++----- src/cfnlint/rules/functions/RefFormat.py | 18 ++-- .../parameters/DynamicReferenceSecret.py | 5 +- src/cfnlint/rules/parameters/NoEcho.py | 13 +-- .../rules/resources/properties/ImageId.py | 7 +- .../results/integration/getatt-types.json | 75 ++++++++++----- .../results/integration/ref-types.json | 31 +++++++ .../templates/integration/getatt-types.yaml | 50 ++++++++++ .../templates/integration/ref-types.yaml | 91 +++++++++++++++++++ .../integration/test_integration_templates.py | 5 + test/unit/rules/functions/test_ref_format.py | 8 +- .../test_dynamic_reference_secret.py | 8 +- test/unit/rules/parameters/test_no_echo.py | 20 ++-- .../resources/properties/test_image_id.py | 6 +- 38 files changed, 329 insertions(+), 84 deletions(-) create mode 100644 test/fixtures/results/integration/ref-types.json create mode 100644 test/fixtures/templates/integration/ref-types.yaml diff --git a/scripts/update_schemas_format.py b/scripts/update_schemas_format.py index 9e2c61f171..b05f74eb2c 100755 --- a/scripts/update_schemas_format.py +++ b/scripts/update_schemas_format.py @@ -157,6 +157,10 @@ def _create_patch(value: dict[str, str], ref: Sequence[str], resolver: RefResolv values={"format": "AWS::EC2::SecurityGroup.GroupName"}, path="/properties/GroupName", ), + Patch( + values={"format": "AWS::EC2::SecurityGroup.GroupId"}, + path="/properties/Id", + ), ], "AWS::EC2::SecurityGroupIngress": [ Patch( diff --git a/scripts/update_snapshot_results.sh b/scripts/update_snapshot_results.sh index 98c0f00a69..fbc1866105 100755 --- a/scripts/update_snapshot_results.sh +++ b/scripts/update_snapshot_results.sh @@ -6,6 +6,7 @@ cfn-lint test/fixtures/templates/integration/resources-cloudformation-init.yaml cfn-lint test/fixtures/templates/integration/ref-no-value.yaml -e -c I --format json > test/fixtures/results/integration/ref-no-value.json cfn-lint test/fixtures/templates/integration/availability-zones.yaml -e -c I --format json > test/fixtures/results/integration/availability-zones.json cfn-lint test/fixtures/templates/integration/getatt-types.yaml -e -c I --format json > test/fixtures/results/integration/getatt-types.json +cfn-lint test/fixtures/templates/integration/ref-types.yaml -e -c I --format json > test/fixtures/results/integration/ref-types.json cfn-lint test/fixtures/templates/integration/aws-ec2-networkinterface.yaml -e -c I --format json > test/fixtures/results/integration/aws-ec2-networkinterface.json cfn-lint test/fixtures/templates/integration/aws-ec2-instance.yaml -e -c I --format json > test/fixtures/results/integration/aws-ec2-instance.json cfn-lint test/fixtures/templates/integration/aws-ec2-launchtemplate.yaml -e -c I --format json > test/fixtures/results/integration/aws-ec2-launchtemplate.json diff --git a/src/cfnlint/data/schemas/patches/extensions/all/aws_ec2_securitygroup/format.json b/src/cfnlint/data/schemas/patches/extensions/all/aws_ec2_securitygroup/format.json index fb4ecb5172..5e2c3f2734 100644 --- a/src/cfnlint/data/schemas/patches/extensions/all/aws_ec2_securitygroup/format.json +++ b/src/cfnlint/data/schemas/patches/extensions/all/aws_ec2_securitygroup/format.json @@ -9,6 +9,11 @@ "path": "/properties/GroupName/format", "value": "AWS::EC2::SecurityGroup.GroupName" }, + { + "op": "add", + "path": "/properties/Id/format", + "value": "AWS::EC2::SecurityGroup.GroupId" + }, { "op": "add", "path": "/properties/VpcId/format", diff --git a/src/cfnlint/data/schemas/providers/af_south_1/aws-emrserverless-application.json b/src/cfnlint/data/schemas/providers/af_south_1/aws-emrserverless-application.json index 97193bce96..11abc7cde7 100644 --- a/src/cfnlint/data/schemas/providers/af_south_1/aws-emrserverless-application.json +++ b/src/cfnlint/data/schemas/providers/af_south_1/aws-emrserverless-application.json @@ -216,6 +216,7 @@ "type": "object" }, "LogGroupName": { + "format": "AWS::Logs::LogGroup.Name", "maxLength": 512, "minLength": 1, "pattern": "^[\\.\\-_/#A-Za-z0-9]+$", diff --git a/src/cfnlint/data/schemas/providers/ap_east_1/aws-emrserverless-application.json b/src/cfnlint/data/schemas/providers/ap_east_1/aws-emrserverless-application.json index 97193bce96..11abc7cde7 100644 --- a/src/cfnlint/data/schemas/providers/ap_east_1/aws-emrserverless-application.json +++ b/src/cfnlint/data/schemas/providers/ap_east_1/aws-emrserverless-application.json @@ -216,6 +216,7 @@ "type": "object" }, "LogGroupName": { + "format": "AWS::Logs::LogGroup.Name", "maxLength": 512, "minLength": 1, "pattern": "^[\\.\\-_/#A-Za-z0-9]+$", diff --git a/src/cfnlint/data/schemas/providers/ap_northeast_3/aws-emrserverless-application.json b/src/cfnlint/data/schemas/providers/ap_northeast_3/aws-emrserverless-application.json index 97193bce96..11abc7cde7 100644 --- a/src/cfnlint/data/schemas/providers/ap_northeast_3/aws-emrserverless-application.json +++ b/src/cfnlint/data/schemas/providers/ap_northeast_3/aws-emrserverless-application.json @@ -216,6 +216,7 @@ "type": "object" }, "LogGroupName": { + "format": "AWS::Logs::LogGroup.Name", "maxLength": 512, "minLength": 1, "pattern": "^[\\.\\-_/#A-Za-z0-9]+$", diff --git a/src/cfnlint/data/schemas/providers/ap_south_1/aws-emrserverless-application.json b/src/cfnlint/data/schemas/providers/ap_south_1/aws-emrserverless-application.json index 97193bce96..11abc7cde7 100644 --- a/src/cfnlint/data/schemas/providers/ap_south_1/aws-emrserverless-application.json +++ b/src/cfnlint/data/schemas/providers/ap_south_1/aws-emrserverless-application.json @@ -216,6 +216,7 @@ "type": "object" }, "LogGroupName": { + "format": "AWS::Logs::LogGroup.Name", "maxLength": 512, "minLength": 1, "pattern": "^[\\.\\-_/#A-Za-z0-9]+$", diff --git a/src/cfnlint/data/schemas/providers/ap_southeast_1/aws-emrserverless-application.json b/src/cfnlint/data/schemas/providers/ap_southeast_1/aws-emrserverless-application.json index 97193bce96..11abc7cde7 100644 --- a/src/cfnlint/data/schemas/providers/ap_southeast_1/aws-emrserverless-application.json +++ b/src/cfnlint/data/schemas/providers/ap_southeast_1/aws-emrserverless-application.json @@ -216,6 +216,7 @@ "type": "object" }, "LogGroupName": { + "format": "AWS::Logs::LogGroup.Name", "maxLength": 512, "minLength": 1, "pattern": "^[\\.\\-_/#A-Za-z0-9]+$", diff --git a/src/cfnlint/data/schemas/providers/ap_southeast_2/aws-emrserverless-application.json b/src/cfnlint/data/schemas/providers/ap_southeast_2/aws-emrserverless-application.json index 97193bce96..11abc7cde7 100644 --- a/src/cfnlint/data/schemas/providers/ap_southeast_2/aws-emrserverless-application.json +++ b/src/cfnlint/data/schemas/providers/ap_southeast_2/aws-emrserverless-application.json @@ -216,6 +216,7 @@ "type": "object" }, "LogGroupName": { + "format": "AWS::Logs::LogGroup.Name", "maxLength": 512, "minLength": 1, "pattern": "^[\\.\\-_/#A-Za-z0-9]+$", diff --git a/src/cfnlint/data/schemas/providers/ap_southeast_3/aws-emrserverless-application.json b/src/cfnlint/data/schemas/providers/ap_southeast_3/aws-emrserverless-application.json index 97193bce96..11abc7cde7 100644 --- a/src/cfnlint/data/schemas/providers/ap_southeast_3/aws-emrserverless-application.json +++ b/src/cfnlint/data/schemas/providers/ap_southeast_3/aws-emrserverless-application.json @@ -216,6 +216,7 @@ "type": "object" }, "LogGroupName": { + "format": "AWS::Logs::LogGroup.Name", "maxLength": 512, "minLength": 1, "pattern": "^[\\.\\-_/#A-Za-z0-9]+$", diff --git a/src/cfnlint/data/schemas/providers/ca_central_1/aws-emrserverless-application.json b/src/cfnlint/data/schemas/providers/ca_central_1/aws-emrserverless-application.json index 97193bce96..11abc7cde7 100644 --- a/src/cfnlint/data/schemas/providers/ca_central_1/aws-emrserverless-application.json +++ b/src/cfnlint/data/schemas/providers/ca_central_1/aws-emrserverless-application.json @@ -216,6 +216,7 @@ "type": "object" }, "LogGroupName": { + "format": "AWS::Logs::LogGroup.Name", "maxLength": 512, "minLength": 1, "pattern": "^[\\.\\-_/#A-Za-z0-9]+$", diff --git a/src/cfnlint/data/schemas/providers/cn_northwest_1/aws-emrserverless-application.json b/src/cfnlint/data/schemas/providers/cn_northwest_1/aws-emrserverless-application.json index 97193bce96..11abc7cde7 100644 --- a/src/cfnlint/data/schemas/providers/cn_northwest_1/aws-emrserverless-application.json +++ b/src/cfnlint/data/schemas/providers/cn_northwest_1/aws-emrserverless-application.json @@ -216,6 +216,7 @@ "type": "object" }, "LogGroupName": { + "format": "AWS::Logs::LogGroup.Name", "maxLength": 512, "minLength": 1, "pattern": "^[\\.\\-_/#A-Za-z0-9]+$", diff --git a/src/cfnlint/data/schemas/providers/eu_north_1/aws-emrserverless-application.json b/src/cfnlint/data/schemas/providers/eu_north_1/aws-emrserverless-application.json index 97193bce96..11abc7cde7 100644 --- a/src/cfnlint/data/schemas/providers/eu_north_1/aws-emrserverless-application.json +++ b/src/cfnlint/data/schemas/providers/eu_north_1/aws-emrserverless-application.json @@ -216,6 +216,7 @@ "type": "object" }, "LogGroupName": { + "format": "AWS::Logs::LogGroup.Name", "maxLength": 512, "minLength": 1, "pattern": "^[\\.\\-_/#A-Za-z0-9]+$", diff --git a/src/cfnlint/data/schemas/providers/eu_south_2/aws-emrserverless-application.json b/src/cfnlint/data/schemas/providers/eu_south_2/aws-emrserverless-application.json index 97193bce96..11abc7cde7 100644 --- a/src/cfnlint/data/schemas/providers/eu_south_2/aws-emrserverless-application.json +++ b/src/cfnlint/data/schemas/providers/eu_south_2/aws-emrserverless-application.json @@ -216,6 +216,7 @@ "type": "object" }, "LogGroupName": { + "format": "AWS::Logs::LogGroup.Name", "maxLength": 512, "minLength": 1, "pattern": "^[\\.\\-_/#A-Za-z0-9]+$", diff --git a/src/cfnlint/data/schemas/providers/eu_west_1/aws-emrserverless-application.json b/src/cfnlint/data/schemas/providers/eu_west_1/aws-emrserverless-application.json index 97193bce96..11abc7cde7 100644 --- a/src/cfnlint/data/schemas/providers/eu_west_1/aws-emrserverless-application.json +++ b/src/cfnlint/data/schemas/providers/eu_west_1/aws-emrserverless-application.json @@ -216,6 +216,7 @@ "type": "object" }, "LogGroupName": { + "format": "AWS::Logs::LogGroup.Name", "maxLength": 512, "minLength": 1, "pattern": "^[\\.\\-_/#A-Za-z0-9]+$", diff --git a/src/cfnlint/data/schemas/providers/eu_west_2/aws-emrserverless-application.json b/src/cfnlint/data/schemas/providers/eu_west_2/aws-emrserverless-application.json index 97193bce96..11abc7cde7 100644 --- a/src/cfnlint/data/schemas/providers/eu_west_2/aws-emrserverless-application.json +++ b/src/cfnlint/data/schemas/providers/eu_west_2/aws-emrserverless-application.json @@ -216,6 +216,7 @@ "type": "object" }, "LogGroupName": { + "format": "AWS::Logs::LogGroup.Name", "maxLength": 512, "minLength": 1, "pattern": "^[\\.\\-_/#A-Za-z0-9]+$", diff --git a/src/cfnlint/data/schemas/providers/eu_west_3/aws-emrserverless-application.json b/src/cfnlint/data/schemas/providers/eu_west_3/aws-emrserverless-application.json index 97193bce96..11abc7cde7 100644 --- a/src/cfnlint/data/schemas/providers/eu_west_3/aws-emrserverless-application.json +++ b/src/cfnlint/data/schemas/providers/eu_west_3/aws-emrserverless-application.json @@ -216,6 +216,7 @@ "type": "object" }, "LogGroupName": { + "format": "AWS::Logs::LogGroup.Name", "maxLength": 512, "minLength": 1, "pattern": "^[\\.\\-_/#A-Za-z0-9]+$", diff --git a/src/cfnlint/data/schemas/providers/me_central_1/aws-emrserverless-application.json b/src/cfnlint/data/schemas/providers/me_central_1/aws-emrserverless-application.json index 97193bce96..11abc7cde7 100644 --- a/src/cfnlint/data/schemas/providers/me_central_1/aws-emrserverless-application.json +++ b/src/cfnlint/data/schemas/providers/me_central_1/aws-emrserverless-application.json @@ -216,6 +216,7 @@ "type": "object" }, "LogGroupName": { + "format": "AWS::Logs::LogGroup.Name", "maxLength": 512, "minLength": 1, "pattern": "^[\\.\\-_/#A-Za-z0-9]+$", diff --git a/src/cfnlint/data/schemas/providers/me_south_1/aws-emrserverless-application.json b/src/cfnlint/data/schemas/providers/me_south_1/aws-emrserverless-application.json index 97193bce96..11abc7cde7 100644 --- a/src/cfnlint/data/schemas/providers/me_south_1/aws-emrserverless-application.json +++ b/src/cfnlint/data/schemas/providers/me_south_1/aws-emrserverless-application.json @@ -216,6 +216,7 @@ "type": "object" }, "LogGroupName": { + "format": "AWS::Logs::LogGroup.Name", "maxLength": 512, "minLength": 1, "pattern": "^[\\.\\-_/#A-Za-z0-9]+$", diff --git a/src/cfnlint/data/schemas/providers/sa_east_1/aws-emrserverless-application.json b/src/cfnlint/data/schemas/providers/sa_east_1/aws-emrserverless-application.json index 97193bce96..11abc7cde7 100644 --- a/src/cfnlint/data/schemas/providers/sa_east_1/aws-emrserverless-application.json +++ b/src/cfnlint/data/schemas/providers/sa_east_1/aws-emrserverless-application.json @@ -216,6 +216,7 @@ "type": "object" }, "LogGroupName": { + "format": "AWS::Logs::LogGroup.Name", "maxLength": 512, "minLength": 1, "pattern": "^[\\.\\-_/#A-Za-z0-9]+$", diff --git a/src/cfnlint/data/schemas/providers/us_east_1/aws-ec2-securitygroup.json b/src/cfnlint/data/schemas/providers/us_east_1/aws-ec2-securitygroup.json index 3de009dd36..d5e795063c 100644 --- a/src/cfnlint/data/schemas/providers/us_east_1/aws-ec2-securitygroup.json +++ b/src/cfnlint/data/schemas/providers/us_east_1/aws-ec2-securitygroup.json @@ -140,6 +140,7 @@ "type": "string" }, "Id": { + "format": "AWS::EC2::SecurityGroup.GroupId", "type": "string" }, "SecurityGroupEgress": { diff --git a/src/cfnlint/data/schemas/providers/us_east_2/aws-emrserverless-application.json b/src/cfnlint/data/schemas/providers/us_east_2/aws-emrserverless-application.json index 97193bce96..11abc7cde7 100644 --- a/src/cfnlint/data/schemas/providers/us_east_2/aws-emrserverless-application.json +++ b/src/cfnlint/data/schemas/providers/us_east_2/aws-emrserverless-application.json @@ -216,6 +216,7 @@ "type": "object" }, "LogGroupName": { + "format": "AWS::Logs::LogGroup.Name", "maxLength": 512, "minLength": 1, "pattern": "^[\\.\\-_/#A-Za-z0-9]+$", diff --git a/src/cfnlint/data/schemas/providers/us_west_1/aws-emrserverless-application.json b/src/cfnlint/data/schemas/providers/us_west_1/aws-emrserverless-application.json index 97193bce96..11abc7cde7 100644 --- a/src/cfnlint/data/schemas/providers/us_west_1/aws-emrserverless-application.json +++ b/src/cfnlint/data/schemas/providers/us_west_1/aws-emrserverless-application.json @@ -216,6 +216,7 @@ "type": "object" }, "LogGroupName": { + "format": "AWS::Logs::LogGroup.Name", "maxLength": 512, "minLength": 1, "pattern": "^[\\.\\-_/#A-Za-z0-9]+$", diff --git a/src/cfnlint/data/schemas/providers/us_west_2/aws-emrserverless-application.json b/src/cfnlint/data/schemas/providers/us_west_2/aws-emrserverless-application.json index 97193bce96..11abc7cde7 100644 --- a/src/cfnlint/data/schemas/providers/us_west_2/aws-emrserverless-application.json +++ b/src/cfnlint/data/schemas/providers/us_west_2/aws-emrserverless-application.json @@ -216,6 +216,7 @@ "type": "object" }, "LogGroupName": { + "format": "AWS::Logs::LogGroup.Name", "maxLength": 512, "minLength": 1, "pattern": "^[\\.\\-_/#A-Za-z0-9]+$", diff --git a/src/cfnlint/rules/functions/Ref.py b/src/cfnlint/rules/functions/Ref.py index 452682ebfe..5b2109f6c7 100644 --- a/src/cfnlint/rules/functions/Ref.py +++ b/src/cfnlint/rules/functions/Ref.py @@ -63,32 +63,33 @@ def ref(self, validator, subschema, instance, schema): if not validator.is_type(value, "string"): return - if value not in validator.context.parameters: - return - - parameter_type = validator.context.parameters[value].type - schema_types = self.resolve_type(validator, subschema) - if not schema_types: - return - reprs = ", ".join(repr(type) for type in schema_types) - - if all( - st not in ["string", "boolean", "integer", "number"] for st in schema_types - ): - if parameter_type not in VALID_PARAMETER_TYPES_LIST: - yield ValidationError(f"{instance!r} is not of type {reprs}") - return - elif all(st not in ["array"] for st in schema_types): - if parameter_type not in [ - x for x in VALID_PARAMETER_TYPES if x not in VALID_PARAMETER_TYPES_LIST - ]: - yield ValidationError(f"{instance!r} is not of type {reprs}") + if value in validator.context.parameters: + parameter_type = validator.context.parameters[value].type + schema_types = self.resolve_type(validator, subschema) + if not schema_types: return + reprs = ", ".join(repr(type) for type in schema_types) + + if all( + st not in ["string", "boolean", "integer", "number"] + for st in schema_types + ): + if parameter_type not in VALID_PARAMETER_TYPES_LIST: + yield ValidationError(f"{instance!r} is not of type {reprs}") + return + elif all(st not in ["array"] for st in schema_types): + if parameter_type not in [ + x + for x in VALID_PARAMETER_TYPES + if x not in VALID_PARAMETER_TYPES_LIST + ]: + yield ValidationError(f"{instance!r} is not of type {reprs}") + return for rule_id in self._all_refs: rule = self.child_rules.get(rule_id) if rule: - yield from rule.validate(validator, {}, instance, schema) + yield from rule.validate(validator, {}, value, subschema) keyword = validator.context.path.cfn_path_string for rule in self.child_rules.values(): @@ -97,4 +98,4 @@ def ref(self, validator, subschema, instance, schema): if not hasattr(rule, "keywords"): continue if keyword in rule.keywords or "*" in rule.keywords: - yield from rule.validate(validator, keyword, instance, schema) + yield from rule.validate(validator, keyword, value, subschema) diff --git a/src/cfnlint/rules/functions/RefFormat.py b/src/cfnlint/rules/functions/RefFormat.py index 384d3d72bf..a241e9aada 100644 --- a/src/cfnlint/rules/functions/RefFormat.py +++ b/src/cfnlint/rules/functions/RefFormat.py @@ -5,7 +5,6 @@ from typing import Any -from cfnlint.helpers import is_function from cfnlint.jsonschema import ValidationError, ValidationResult, Validator from cfnlint.rules.jsonschema import CfnLintKeyword from cfnlint.schema import PROVIDER_SCHEMA_MANAGER @@ -27,14 +26,16 @@ def __init__(self): def validate( self, validator: Validator, _, instance: Any, schema: Any ) -> ValidationResult: + if instance in validator.context.parameters: + return + fmt = schema.get("format") if not fmt: return - _, resource = is_function(instance) - if resource not in validator.context.resources: + if instance not in validator.context.resources: return - t = validator.context.resources[resource].type + t = validator.context.resources[instance].type for ( regions, resource_schema, @@ -43,19 +44,22 @@ def validate( ): region = regions[0] - ref_schema = validator.context.resources[resource].ref(region) + ref_schema = validator.context.resources[instance].ref(region) ref_fmt = ref_schema.get("format") if ref_fmt != fmt: if ref_fmt is None: yield ValidationError( - f"{instance!r} does not match destination format of {fmt!r}", + ( + f"{{'Ref': {instance!r}}} does not match " + "destination format of {fmt!r}" + ), rule=self, ) else: yield ValidationError( ( - f"{instance!r} with format {ref_fmt!r} does not " + f"{{'Ref': {instance!r}}} with format {ref_fmt!r} does not " f"match destination format of {fmt!r}" ), rule=self, diff --git a/src/cfnlint/rules/parameters/DynamicReferenceSecret.py b/src/cfnlint/rules/parameters/DynamicReferenceSecret.py index 1bcd4924bc..e94878b208 100644 --- a/src/cfnlint/rules/parameters/DynamicReferenceSecret.py +++ b/src/cfnlint/rules/parameters/DynamicReferenceSecret.py @@ -48,12 +48,11 @@ def validate(self, validator: Validator, _, instance: Any, schema: Any): functions = set(FUNCTIONS) - set(["Fn::If"]) if any(p in functions for p in validator.context.path.path): return - value = instance.get("Ref") - if not validator.is_type(value, "string"): + if not validator.is_type(instance, "string"): return - if value in validator.context.parameters: + if instance in validator.context.parameters: yield ValidationError( "Use dynamic references over parameters for secrets", rule=self ) diff --git a/src/cfnlint/rules/parameters/NoEcho.py b/src/cfnlint/rules/parameters/NoEcho.py index dcd26b2004..c98eb9afe1 100644 --- a/src/cfnlint/rules/parameters/NoEcho.py +++ b/src/cfnlint/rules/parameters/NoEcho.py @@ -21,12 +21,10 @@ class NoEcho(CloudFormationLintRule): tags = ["functions", "dynamic reference", "ref"] def validate(self, validator: Validator, _, instance: Any, schema: Any): - value = instance.get("Ref") - - if not validator.is_type(value, "string"): + if not validator.is_type(instance, "string"): return - parameter = validator.context.parameters.get(value) + parameter = validator.context.parameters.get(instance) if not parameter: return @@ -37,7 +35,10 @@ def validate(self, validator: Validator, _, instance: Any, schema: Any): and validator.context.path.path[2] == "Metadata" ): yield ValidationError( - f"Don't use 'NoEcho' parameter {value!r} in resource metadata", + ( + f"Don't use 'NoEcho' parameter {instance!r} " + "in resource metadata" + ), rule=self, path=deque(["Ref"]), ) @@ -46,7 +47,7 @@ def validate(self, validator: Validator, _, instance: Any, schema: Any): if validator.context.path.path[0] in ["Metadata", "Outputs"]: yield ValidationError( ( - f"Don't use 'NoEcho' parameter {value!r} " + f"Don't use 'NoEcho' parameter {instance!r} " f"in {validator.context.path.path[0]!r}" ), rule=self, diff --git a/src/cfnlint/rules/resources/properties/ImageId.py b/src/cfnlint/rules/resources/properties/ImageId.py index 6780ccaa20..6bf7083f27 100644 --- a/src/cfnlint/rules/resources/properties/ImageId.py +++ b/src/cfnlint/rules/resources/properties/ImageId.py @@ -40,11 +40,10 @@ def validate(self, validator: Validator, _, instance: Any, schema: Any): if any(fn in validator.context.path.path for fn in FUNCTIONS): return - value = instance.get("Ref") - if value not in validator.context.parameters: + if instance not in validator.context.parameters: return - parameter_type = validator.context.parameters[value].type + parameter_type = validator.context.parameters[instance].type for err in validator.descend( instance=parameter_type, schema={ @@ -55,5 +54,5 @@ def validate(self, validator: Validator, _, instance: Any, schema: Any): }, ): err.rule = self - err.path_override = deque(["Parameters", value, "Type"]) + err.path_override = deque(["Parameters", instance, "Type"]) yield err diff --git a/test/fixtures/results/integration/getatt-types.json b/test/fixtures/results/integration/getatt-types.json index 2d15e311cd..e9fd274dc9 100644 --- a/test/fixtures/results/integration/getatt-types.json +++ b/test/fixtures/results/integration/getatt-types.json @@ -31,12 +31,45 @@ }, { "Filename": "test/fixtures/templates/integration/getatt-types.yaml", - "Id": "f9f8618f-c9a0-d78f-84f6-a2fb809cc514", + "Id": "16620b40-6f3e-69eb-874f-34b5f51cedee", + "Level": "Error", + "Location": { + "End": { + "ColumnNumber": 28, + "LineNumber": 62 + }, + "Path": [ + "Resources", + "TestTaskDefinitionWithGetAtt", + "Properties", + "ContainerDefinitions", + 0, + "LogConfiguration", + "Options", + "awslogs-group" + ], + "Start": { + "ColumnNumber": 15, + "LineNumber": 62 + } + }, + "Message": "{'Fn::GetAtt': ['TestLogGroup', 'Arn']} does not match destination format of 'AWS::Logs::LogGroup.Name'", + "ParentId": null, + "Rule": { + "Description": "Validate that if source and destination format exists that they match", + "Id": "E1040", + "ShortDescription": "Check if GetAtt matches destination format", + "Source": "https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/best-practices.html#parmtypes" + } + }, + { + "Filename": "test/fixtures/templates/integration/getatt-types.yaml", + "Id": "536e9277-ad5b-fed7-821e-34fb182c3ecc", "Level": "Error", "Location": { "End": { "ColumnNumber": 69, - "LineNumber": 35 + "LineNumber": 85 }, "Path": [ "Outputs", @@ -49,7 +82,7 @@ ], "Start": { "ColumnNumber": 57, - "LineNumber": 35 + "LineNumber": 85 } }, "Message": "{'Fn::GetAtt': ['CapacityReservation', 'InstanceCount']} is not of type 'string'", @@ -63,12 +96,12 @@ }, { "Filename": "test/fixtures/templates/integration/getatt-types.yaml", - "Id": "518fa518-bd82-b365-8ec2-c48ae8581f3e", + "Id": "36c89a62-589a-e0db-d49b-3dae6791806c", "Level": "Error", "Location": { "End": { "ColumnNumber": 10, - "LineNumber": 37 + "LineNumber": 87 }, "Path": [ "Outputs", @@ -78,7 +111,7 @@ ], "Start": { "ColumnNumber": 5, - "LineNumber": 37 + "LineNumber": 87 } }, "Message": "{'Fn::GetAtt': ['CapacityReservation', 'InstanceCount']} is not of type 'string'", @@ -92,12 +125,12 @@ }, { "Filename": "test/fixtures/templates/integration/getatt-types.yaml", - "Id": "3ff33f0b-1651-bc3b-710e-e603779c3c5f", + "Id": "d66d8ee3-bb6c-8635-52ea-622393dd61b0", "Level": "Error", "Location": { "End": { "ColumnNumber": 10, - "LineNumber": 39 + "LineNumber": 89 }, "Path": [ "Outputs", @@ -107,7 +140,7 @@ ], "Start": { "ColumnNumber": 5, - "LineNumber": 39 + "LineNumber": 89 } }, "Message": "'CapacityReservation.InstanceCount' is not of type 'string'", @@ -121,12 +154,12 @@ }, { "Filename": "test/fixtures/templates/integration/getatt-types.yaml", - "Id": "c9b79105-67dc-dbe7-3757-b4a9d47c5366", + "Id": "e188f184-e315-36dc-231e-6cfd557932bf", "Level": "Informational", "Location": { "End": { "ColumnNumber": 22, - "LineNumber": 41 + "LineNumber": 91 }, "Path": [ "Outputs", @@ -137,7 +170,7 @@ ], "Start": { "ColumnNumber": 20, - "LineNumber": 41 + "LineNumber": 91 } }, "Message": "Prefer using Fn::Sub over Fn::Join with an empty delimiter", @@ -151,12 +184,12 @@ }, { "Filename": "test/fixtures/templates/integration/getatt-types.yaml", - "Id": "e9db8f22-319e-6556-6eef-9dfb16e9aa1b", + "Id": "78f2b914-89f3-f2a3-7013-d8d2d3dc3439", "Level": "Error", "Location": { "End": { "ColumnNumber": 10, - "LineNumber": 43 + "LineNumber": 93 }, "Path": [ "Outputs", @@ -169,7 +202,7 @@ ], "Start": { "ColumnNumber": 5, - "LineNumber": 43 + "LineNumber": 93 } }, "Message": "{'Fn::GetAtt': ['CapacityReservation', 'InstanceCount']} is not of type 'string'", @@ -183,12 +216,12 @@ }, { "Filename": "test/fixtures/templates/integration/getatt-types.yaml", - "Id": "a777b7aa-5a61-f44b-0e11-7f8fad0117c0", + "Id": "fb048506-abb7-1e8f-b3c4-16b5f8c63cae", "Level": "Informational", "Location": { "End": { "ColumnNumber": 22, - "LineNumber": 43 + "LineNumber": 93 }, "Path": [ "Outputs", @@ -199,7 +232,7 @@ ], "Start": { "ColumnNumber": 20, - "LineNumber": 43 + "LineNumber": 93 } }, "Message": "Prefer using Fn::Sub over Fn::Join with an empty delimiter", @@ -213,12 +246,12 @@ }, { "Filename": "test/fixtures/templates/integration/getatt-types.yaml", - "Id": "f968510f-4410-ab60-e14d-76661faf7d3e", + "Id": "37606842-eb43-dd6c-36a2-d69c367b389d", "Level": "Error", "Location": { "End": { "ColumnNumber": 10, - "LineNumber": 47 + "LineNumber": 97 }, "Path": [ "Outputs", @@ -228,7 +261,7 @@ ], "Start": { "ColumnNumber": 5, - "LineNumber": 47 + "LineNumber": 97 } }, "Message": "{'Fn::GetAtt': ['CapacityReservation', 'EphemeralStorage']} is not of type 'string'", diff --git a/test/fixtures/results/integration/ref-types.json b/test/fixtures/results/integration/ref-types.json new file mode 100644 index 0000000000..f440d37f6d --- /dev/null +++ b/test/fixtures/results/integration/ref-types.json @@ -0,0 +1,31 @@ +[ + { + "Filename": "test/fixtures/templates/integration/ref-types.yaml", + "Id": "2f03486c-dde0-ea5e-49a6-634f3a935841", + "Level": "Error", + "Location": { + "End": { + "ColumnNumber": 12, + "LineNumber": 45 + }, + "Path": [ + "Resources", + "Subnet2", + "Properties", + "VpcId" + ], + "Start": { + "ColumnNumber": 7, + "LineNumber": 45 + } + }, + "Message": "{'Ref': 'FargateTaskRole'} does not match destination format of 'AWS::EC2::VPC.Id'", + "ParentId": null, + "Rule": { + "Description": "When source and destination format exists validate that they match in a Ref", + "Id": "E1041", + "ShortDescription": "Check if Ref matches destination format", + "Source": "https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/best-practices.html#parmtypes" + } + } +] diff --git a/test/fixtures/templates/integration/getatt-types.yaml b/test/fixtures/templates/integration/getatt-types.yaml index 4a4d886723..6727d01add 100644 --- a/test/fixtures/templates/integration/getatt-types.yaml +++ b/test/fixtures/templates/integration/getatt-types.yaml @@ -22,6 +22,56 @@ Resources: Type: AWS::DocDB::DBCluster Properties: BackupRetentionPeriod: 1 + TestCluster: + Type: AWS::ECS::Cluster + TestFargateExecutionRole: + Type: AWS::IAM::Role + Properties: + AssumeRolePolicyDocument: + Statement: + - Action: + - sts:AssumeRole + Effect: Allow + Principal: + Service: ecs-tasks.amazonaws.com + ManagedPolicyArns: + - arn:aws:iam::aws:policy/service-role/AmazonECSTaskExecutionRolePolicy + + TestFargateTaskRole: + Type: AWS::IAM::Role + Properties: + AssumeRolePolicyDocument: + Statement: + - Action: + - sts:AssumeRole + Effect: Allow + Principal: + Service: ecs-tasks.amazonaws.com + TestLogGroup: + Type: AWS::Logs::LogGroup + DeletionPolicy: Delete + UpdateReplacePolicy: Delete + TestTaskDefinitionWithGetAtt: + Type: AWS::ECS::TaskDefinition + Properties: + ContainerDefinitions: + - Image: nginx:1.27.3-alpine3.20 + LogConfiguration: + LogDriver: awslogs + Options: + awslogs-group: !GetAtt 'TestLogGroup.Arn' + awslogs-region: !Ref 'AWS::Region' + awslogs-stream-prefix: test-service + Name: TestContainerDefinition + PortMappings: + - ContainerPort: 80 + Cpu: '256' + ExecutionRoleArn: !GetAtt 'TestFargateExecutionRole.Arn' + Memory: '512' + NetworkMode: awsvpc + RequiresCompatibilities: + - FARGATE + TaskRoleArn: !GetAtt 'TestFargateTaskRole.Arn' Outputs: Value: Value: 1 # OK diff --git a/test/fixtures/templates/integration/ref-types.yaml b/test/fixtures/templates/integration/ref-types.yaml new file mode 100644 index 0000000000..d9a5fd4cf3 --- /dev/null +++ b/test/fixtures/templates/integration/ref-types.yaml @@ -0,0 +1,91 @@ +AWSTemplateFormatVersion: '2010-09-09' +Parameters: + AString: + Type: String + Default: default +Resources: + Cluster: + Type: AWS::ECS::Cluster + FargateExecutionRole: + Type: AWS::IAM::Role + Properties: + AssumeRolePolicyDocument: + Statement: + - Action: + - sts:AssumeRole + Effect: Allow + Principal: + Service: ecs-tasks.amazonaws.com + ManagedPolicyArns: + - arn:aws:iam::aws:policy/service-role/AmazonECSTaskExecutionRolePolicy + + FargateTaskRole: + Type: AWS::IAM::Role + Properties: + AssumeRolePolicyDocument: + Statement: + - Action: + - sts:AssumeRole + Effect: Allow + Principal: + Service: ecs-tasks.amazonaws.com + Vpc: + Type: AWS::EC2::VPC + Properties: + CidrBlock: 10.0.0.0/16 + Subnet1: + Type: AWS::EC2::Subnet + Properties: + CidrBlock: 10.0.1.0/24 + VpcId: !Ref Vpc + Subnet2: + Type: AWS::EC2::Subnet + Properties: + CidrBlock: 10.0.2.0/24 + VpcId: !Ref FargateTaskRole # Wrong type + LogGroup: + Type: AWS::Logs::LogGroup + DeletionPolicy: Delete + UpdateReplacePolicy: Delete + TaskDefinitionWithRefToResource: + Type: AWS::ECS::TaskDefinition + Properties: + ContainerDefinitions: + - Image: nginx:1.27.3-alpine3.20 + LogConfiguration: + LogDriver: awslogs + Options: + awslogs-group: !Ref LogGroup + awslogs-region: !Ref 'AWS::Region' + awslogs-stream-prefix: test-service + Name: TestContainerDefinition + PortMappings: + - ContainerPort: 80 + Cpu: '256' + ExecutionRoleArn: !GetAtt 'FargateExecutionRole.Arn' + Memory: '512' + NetworkMode: awsvpc + RequiresCompatibilities: + - FARGATE + TaskRoleArn: !GetAtt 'FargateTaskRole.Arn' + TaskDefinitionWithRefToParameter: + Type: AWS::ECS::TaskDefinition + Properties: + ContainerDefinitions: + - Image: nginx:1.27.3-alpine3.20 + LogConfiguration: + LogDriver: awslogs + Options: + awslogs-group: !Ref AString + awslogs-region: !Ref 'AWS::Region' + awslogs-stream-prefix: test-service + Name: TestContainerDefinition + PortMappings: + - ContainerPort: 80 + Cpu: '256' + ExecutionRoleArn: !GetAtt 'FargateExecutionRole.Arn' + Memory: '512' + NetworkMode: awsvpc + RequiresCompatibilities: + - FARGATE + TaskRoleArn: !GetAtt 'FargateTaskRole.Arn' diff --git a/test/integration/test_integration_templates.py b/test/integration/test_integration_templates.py index 4b92d0d4a1..b0a40d60b1 100644 --- a/test/integration/test_integration_templates.py +++ b/test/integration/test_integration_templates.py @@ -52,6 +52,11 @@ class TestQuickStartTemplates(BaseCliTestCase): "results_filename": ("test/fixtures/results/integration/getatt-types.json"), "exit_code": 10, }, + { + "filename": ("test/fixtures/templates/integration/ref-types.yaml"), + "results_filename": ("test/fixtures/results/integration/ref-types.json"), + "exit_code": 2, + }, { "filename": ( "test/fixtures/templates/integration/aws-ec2-networkinterface.yaml" diff --git a/test/unit/rules/functions/test_ref_format.py b/test/unit/rules/functions/test_ref_format.py index c136d2238d..d0601a2b03 100644 --- a/test/unit/rules/functions/test_ref_format.py +++ b/test/unit/rules/functions/test_ref_format.py @@ -30,13 +30,13 @@ def template(): [ ( "Valid Ref with a good format", - {"Ref": "MyVpc"}, + "MyVpc", {"format": "AWS::EC2::VPC.Id"}, [], ), ( "Invalid Ref with a bad format", - {"Ref": "MyVpc"}, + "MyVpc", {"format": "AWS::EC2::Image.Id"}, [ ValidationError( @@ -51,7 +51,7 @@ def template(): ), ( "Invalid Ref with a resource with no format", - {"Ref": "MyBucket"}, + "MyBucket", {"format": "AWS::EC2::Image.Id"}, [ ValidationError( @@ -65,7 +65,7 @@ def template(): ), ( "Invalid Ref to non existent resource", - {"Ref": "DNE"}, + "DNE", {"format": "AWS::EC2::Image.Id"}, [], ), diff --git a/test/unit/rules/parameters/test_dynamic_reference_secret.py b/test/unit/rules/parameters/test_dynamic_reference_secret.py index fe6c663ed4..fc559a55ce 100644 --- a/test/unit/rules/parameters/test_dynamic_reference_secret.py +++ b/test/unit/rules/parameters/test_dynamic_reference_secret.py @@ -45,19 +45,19 @@ def context(cfn): [ ( "REFing a parameter without a string", - {"Ref": []}, + [], deque([]), [], ), ( "REFing a resource=", - {"Ref": "MyResource"}, + "MyResource", deque([]), [], ), ( "REFing a parameter", - {"Ref": "MyParameter"}, + "MyParameter", deque([]), [ ValidationError( @@ -68,7 +68,7 @@ def context(cfn): ), ( "REFing a parameter in a sub", - {"Ref": "MyParameter"}, + "MyParameter", deque(["Fn::Sub"]), [], ), diff --git a/test/unit/rules/parameters/test_no_echo.py b/test/unit/rules/parameters/test_no_echo.py index 471dedca45..178a0e1d1d 100644 --- a/test/unit/rules/parameters/test_no_echo.py +++ b/test/unit/rules/parameters/test_no_echo.py @@ -49,25 +49,25 @@ def context(cfn): [ ( "An Invalid Ref", - {"Ref": []}, + [], deque(["Metadata"]), [], ), ( "A ref to something that isn't a parameter", - {"Ref": "AWS::Region"}, + "AWS::Region", deque(["Metadata"]), [], ), ( "Using NoEcho in Resources Properties", - {"Ref": "Echo"}, + "Echo", deque(["Resources", "MyResource", "Properties", "Name"]), [], ), ( "Using NoEcho in Resource Metadata", - {"Ref": "Echo"}, + "Echo", deque(["Resources", "MyResource", "Metadata"]), [ ValidationError( @@ -79,7 +79,7 @@ def context(cfn): ), ( "Using NoEcho in Metadata", - {"Ref": "Echo"}, + "Echo", deque(["Metadata"]), [ ValidationError( @@ -91,7 +91,7 @@ def context(cfn): ), ( "Using NoEcho in Metadata", - {"Ref": "EchoTrue"}, + "EchoTrue", deque(["Metadata"]), [ ValidationError( @@ -103,25 +103,25 @@ def context(cfn): ), ( "Using Non NoEcho in Metadata", - {"Ref": "NoEcho"}, + "NoEcho", deque(["Metadata"]), [], ), ( "Using a parameter in Metadata", - {"Ref": "MyParameter"}, + "MyParameter", deque(["Metadata"]), [], ), ( "Short list for path", - {"Ref": "MyParameter"}, + "MyParameter", deque([]), [], ), ( "Using NoEcho in Outputs", - {"Ref": "Echo"}, + "Echo", deque(["Outputs", "Name", "Value"]), [ ValidationError( diff --git a/test/unit/rules/resources/properties/test_image_id.py b/test/unit/rules/resources/properties/test_image_id.py index c2fbb2385b..c06eeaf5b6 100644 --- a/test/unit/rules/resources/properties/test_image_id.py +++ b/test/unit/rules/resources/properties/test_image_id.py @@ -37,17 +37,17 @@ def template(): [ ( "Valid Ref to a paraemter", - {"Ref": "MyImageId"}, + "MyImageId", [], ), ( "Valid Ref to a Pseudo-Parameter", - {"Ref": "AWS::Region"}, + "AWS::Region", [], ), ( "Invalid Ref to a parameter of the wrong type", - {"Ref": "MyString"}, + "MyString", [ ValidationError( (