From 68c20c70889217af9d8e4acc74654c2aceeb62df Mon Sep 17 00:00:00 2001 From: Frank van Boven <35613489+fatbasstard@users.noreply.github.com> Date: Wed, 15 May 2019 17:23:36 +0200 Subject: [PATCH] Exclude dynamic references from the allowed value/pattern rule (#900) * Exclude dynamic references from the allowed value rule * Exclude dynamic references from the allowed pattern rule --- .../rules/resources/properties/AllowedPattern.py | 12 ++++++++---- .../rules/resources/properties/AllowedValue.py | 12 ++++++++---- .../templates/bad/properties_sg_ingress.yaml | 9 +++++++++ .../good/resources/properties/allowed_values.yaml | 7 ++++++- 4 files changed, 31 insertions(+), 9 deletions(-) diff --git a/src/cfnlint/rules/resources/properties/AllowedPattern.py b/src/cfnlint/rules/resources/properties/AllowedPattern.py index 0684b6a3ff..4c5cd5fe7a 100644 --- a/src/cfnlint/rules/resources/properties/AllowedPattern.py +++ b/src/cfnlint/rules/resources/properties/AllowedPattern.py @@ -48,11 +48,15 @@ def check_value(self, value, path, property_name, **kwargs): if value_pattern_regex: regex = re.compile(value_pattern_regex) - if not regex.match(value): - full_path = ('/'.join(str(x) for x in path)) - message = '{} contains invalid characters (Pattern: {}) at {}' - matches.append(RuleMatch(path, message.format(property_name, value_pattern, full_path))) + # Ignore values with dynamic references. Simple check to prevent false-positives + # See: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/dynamic-references.html + if '{{resolve:' not in value: + if not regex.match(value): + full_path = ('/'.join(str(x) for x in path)) + + message = '{} contains invalid characters (Pattern: {}) at {}' + matches.append(RuleMatch(path, message.format(property_name, value_pattern, full_path))) return matches diff --git a/src/cfnlint/rules/resources/properties/AllowedValue.py b/src/cfnlint/rules/resources/properties/AllowedValue.py index 791b3e9a21..977def7d4d 100644 --- a/src/cfnlint/rules/resources/properties/AllowedValue.py +++ b/src/cfnlint/rules/resources/properties/AllowedValue.py @@ -42,10 +42,14 @@ def check_value(self, value, path, property_name, **kwargs): allowed_value_specs = kwargs.get('value_specs', {}).get('AllowedValues', {}) if allowed_value_specs: - # Always compare the allowed value as a string, strict typing is not of concern for this rule - if str(value) not in allowed_value_specs: - message = 'You must specify a valid value for {0} ({1}).\nValid values are {2}' - matches.append(RuleMatch(path, message.format(property_name, value, allowed_value_specs))) + + # Ignore values with dynamic references. Simple check to prevent false-positives + # See: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/dynamic-references.html + if '{{resolve:' not in str(value): + # Always compare the allowed value as a string, strict typing is not of concern for this rule + if str(value) not in allowed_value_specs: + message = 'You must specify a valid value for {0} ({1}).\nValid values are {2}' + matches.append(RuleMatch(path, message.format(property_name, value, allowed_value_specs))) return matches diff --git a/test/fixtures/templates/bad/properties_sg_ingress.yaml b/test/fixtures/templates/bad/properties_sg_ingress.yaml index 4b60a658d1..da94728521 100644 --- a/test/fixtures/templates/bad/properties_sg_ingress.yaml +++ b/test/fixtures/templates/bad/properties_sg_ingress.yaml @@ -47,6 +47,15 @@ Resources: - IpProtocol: 1 SourceSecurityGroupId: vpc-1234567 + CidrIp: '10.0.0.0/8' + SecurityGroupRuleSSM: + Type: 'AWS::EC2::SecurityGroupIngress' + Properties: + GroupId: !Ref mySecurityGroupVpc + IpProtocol: tcp + FromPort: 443 + ToPort: 443 + CidrIp: '{{resolve:ssm:mysubnet}}' mySecurityGroupIngress: Type: AWS::EC2::SecurityGroupIngress Properties: diff --git a/test/fixtures/templates/good/resources/properties/allowed_values.yaml b/test/fixtures/templates/good/resources/properties/allowed_values.yaml index cfbcac694a..5d5f603381 100644 --- a/test/fixtures/templates/good/resources/properties/allowed_values.yaml +++ b/test/fixtures/templates/good/resources/properties/allowed_values.yaml @@ -33,7 +33,7 @@ Parameters: Type: Number Resources: LogGroup: - Type: AWS::Logs::LogGroup + Type: "AWS::Logs::LogGroup" Properties: RetentionInDays: !Ref LogsRetentionLength LogGroupName: '/name' @@ -60,6 +60,11 @@ Resources: EngineType: "ACTIVEMQ" # Valid AllowedValue EngineVersion: "5.15.8" # Valid AllowedValue Name: "Configuration" + ApiGatewayAuthorizerSSM: + Type: "AWS::ApiGateway::Authorizer" + Properties: + RestApiId: !Ref "ApiGatewayRestAPI" + Type: "{{resolve:ssm:myparam:1}} xyz" # Excluded AllowedValue ApiGatewayAuthorizer: Type: "AWS::ApiGateway::Authorizer" Properties: