Skip to content

Commit

Permalink
Exclude dynamic references from the allowed value/pattern rule (aws-c…
Browse files Browse the repository at this point in the history
…loudformation#900)

* Exclude dynamic references from the allowed value rule
* Exclude dynamic references from the allowed pattern rule
  • Loading branch information
fatbasstard authored and kddejong committed May 15, 2019
1 parent d3b1b02 commit 68c20c7
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 9 deletions.
12 changes: 8 additions & 4 deletions src/cfnlint/rules/resources/properties/AllowedPattern.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
12 changes: 8 additions & 4 deletions src/cfnlint/rules/resources/properties/AllowedValue.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
9 changes: 9 additions & 0 deletions test/fixtures/templates/bad/properties_sg_ingress.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ Parameters:
Type: Number
Resources:
LogGroup:
Type: AWS::Logs::LogGroup
Type: "AWS::Logs::LogGroup"
Properties:
RetentionInDays: !Ref LogsRetentionLength
LogGroupName: '/name'
Expand All @@ -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:
Expand Down

0 comments on commit 68c20c7

Please sign in to comment.