More cleanup and integration testing

aws-cloudformation · Jan 21, 2025 · 9d13123 · 9d13123
1 parent c56d080
commit 9d13123
Show file tree

Hide file tree

Showing 38 changed files with 329 additions and 84 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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