Skip to content

Commit 05c642a

Browse files
kddejongkddejong
committed
Update rule E8003 to look for string based parameters
1 parent bf0bb6b commit 05c642a

File tree

7 files changed

+123
-60
lines changed

7 files changed

+123
-60
lines changed

‎src/cfnlint/helpers.py

+58
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,64 @@
171171
'AWS::Redshift::Cluster'
172172
]
173173

174+
VALID_PARAMETER_TYPES_SINGLE = [
175+
'AWS::EC2::AvailabilityZone::Name',
176+
'AWS::EC2::Image::Id',
177+
'AWS::EC2::Instance::Id',
178+
'AWS::EC2::KeyPair::KeyName',
179+
'AWS::EC2::SecurityGroup::GroupName',
180+
'AWS::EC2::SecurityGroup::Id',
181+
'AWS::EC2::Subnet::Id',
182+
'AWS::EC2::VPC::Id',
183+
'AWS::EC2::Volume::Id',
184+
'AWS::Route53::HostedZone::Id',
185+
'AWS::SSM::Parameter::Name',
186+
'Number',
187+
'String',
188+
'AWS::SSM::Parameter::Value<AWS::EC2::AvailabilityZone::Name>',
189+
'AWS::SSM::Parameter::Value<AWS::EC2::Image::Id>',
190+
'AWS::SSM::Parameter::Value<AWS::EC2::Instance::Id>',
191+
'AWS::SSM::Parameter::Value<AWS::EC2::KeyPair::KeyName>',
192+
'AWS::SSM::Parameter::Value<AWS::EC2::SecurityGroup::GroupName>',
193+
'AWS::SSM::Parameter::Value<AWS::EC2::SecurityGroup::Id>',
194+
'AWS::SSM::Parameter::Value<AWS::EC2::Subnet::Id>',
195+
'AWS::SSM::Parameter::Value<AWS::EC2::VPC::Id>',
196+
'AWS::SSM::Parameter::Value<AWS::EC2::Volume::Id>',
197+
'AWS::SSM::Parameter::Value<AWS::Route53::HostedZone::Id>',
198+
'AWS::SSM::Parameter::Value<AWS::SSM::Parameter::Name>',
199+
'AWS::SSM::Parameter::Value<Number>',
200+
'AWS::SSM::Parameter::Value<String>',
201+
]
202+
203+
VALID_PARAMETER_TYPES_LIST = [
204+
'CommaDelimitedList',
205+
'List<AWS::EC2::AvailabilityZone::Name>',
206+
'List<AWS::EC2::Image::Id>',
207+
'List<AWS::EC2::Instance::Id>',
208+
'List<AWS::EC2::SecurityGroup::GroupName>',
209+
'List<AWS::EC2::SecurityGroup::Id>',
210+
'List<AWS::EC2::Subnet::Id>',
211+
'List<AWS::EC2::VPC::Id>',
212+
'List<AWS::EC2::Volume::Id>',
213+
'List<AWS::Route53::HostedZone::Id>',
214+
'List<Number>',
215+
'List<String>',
216+
'AWS::SSM::Parameter::Value<CommaDelimitedList>',
217+
'AWS::SSM::Parameter::Value<List<AWS::EC2::AvailabilityZone::Name>>',
218+
'AWS::SSM::Parameter::Value<List<AWS::EC2::Image::Id>>',
219+
'AWS::SSM::Parameter::Value<List<AWS::EC2::Instance::Id>>',
220+
'AWS::SSM::Parameter::Value<List<AWS::EC2::SecurityGroup::GroupName>>',
221+
'AWS::SSM::Parameter::Value<List<AWS::EC2::SecurityGroup::Id>>',
222+
'AWS::SSM::Parameter::Value<List<AWS::EC2::Subnet::Id>>',
223+
'AWS::SSM::Parameter::Value<List<AWS::EC2::VPC::Id>>',
224+
'AWS::SSM::Parameter::Value<List<AWS::EC2::Volume::Id>>',
225+
'AWS::SSM::Parameter::Value<List<AWS::Route53::HostedZone::Id>>',
226+
'AWS::SSM::Parameter::Value<List<Number>>',
227+
'AWS::SSM::Parameter::Value<List<String>>',
228+
]
229+
230+
VALID_PARAMETER_TYPES = VALID_PARAMETER_TYPES_SINGLE + VALID_PARAMETER_TYPES_LIST
231+
174232
def get_metadata_filename(url):
175233
"""Returns the filename for a metadata file associated with a remote resource"""
176234
caching_dir = os.path.join(os.path.dirname(__file__), 'data', 'DownloadsMetadata')

‎src/cfnlint/rules/conditions/Equals.py

+41-23
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
SPDX-License-Identifier: MIT-0
44
"""
55
import six
6+
from cfnlint.helpers import VALID_PARAMETER_TYPES_LIST
67
from cfnlint.rules import CloudFormationLintRule
78
from cfnlint.rules import RuleMatch
89

@@ -15,53 +16,70 @@ class Equals(CloudFormationLintRule):
1516
source_url = 'https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/intrinsic-function-reference-conditions.html#intrinsic-function-reference-conditions-equals'
1617
tags = ['functions', 'equals']
1718

19+
allowed_functions = ['Ref', 'Fn::FindInMap',
20+
'Fn::Sub', 'Fn::Join', 'Fn::Select', 'Fn::Split']
21+
function = 'Fn::Equals'
22+
23+
def _check_equal_values(self, cfn, element, path):
24+
matches = []
25+
26+
if len(element) == 1:
27+
for element_key, element_value in element.items():
28+
if element_key not in self.allowed_functions:
29+
message = self.function + \
30+
' element must be a supported function ({0})'
31+
matches.append(RuleMatch(
32+
path[:] + [element_key],
33+
message.format(', '.join(self.allowed_functions))
34+
))
35+
elif element_key == 'Ref':
36+
valid_refs = cfn.get_valid_refs()
37+
if element_value in valid_refs:
38+
if valid_refs[element_value].get('From') == 'Parameters':
39+
if valid_refs[element_value].get('Type') in VALID_PARAMETER_TYPES_LIST:
40+
message = 'Every Fn::Equals object requires a list of 2 string parameters'
41+
matches.append(RuleMatch(
42+
path, message))
43+
else:
44+
message = self.function + ' element must be a supported function ({0})'
45+
matches.append(RuleMatch(
46+
path,
47+
message.format(', '.join(self.allowed_functions))
48+
))
49+
return matches
50+
1851
def match(self, cfn):
19-
function = 'Fn::Equals'
2052
matches = []
2153
# Build the list of functions
22-
trees = cfn.search_deep_keys(function)
23-
24-
allowed_functions = ['Ref', 'Fn::FindInMap',
25-
'Fn::Sub', 'Fn::Join', 'Fn::Select', 'Fn::Split']
54+
trees = cfn.search_deep_keys(self.function)
2655

2756
for tree in trees:
2857
# Test when in Conditions
2958
if tree[0] == 'Conditions':
3059
value = tree[-1]
3160
if not isinstance(value, list):
32-
message = function + ' must be a list of two elements'
61+
message = self.function + ' must be a list of two elements'
3362
matches.append(RuleMatch(
3463
tree[:-1],
3564
message.format()
3665
))
3766
elif len(value) != 2:
38-
message = function + ' must be a list of two elements'
67+
message = self.function + ' must be a list of two elements'
3968
matches.append(RuleMatch(
4069
tree[:-1],
4170
message.format()
4271
))
4372
else:
4473
for index, element in enumerate(value):
4574
if isinstance(element, dict):
46-
if len(element) == 1:
47-
for element_key in element.keys():
48-
if element_key not in allowed_functions:
49-
message = function + ' element must be a supported function ({0})'
50-
matches.append(RuleMatch(
51-
tree[:-1] + [index, element_key],
52-
message.format(', '.join(allowed_functions))
53-
))
54-
else:
55-
message = function + ' element must be a supported function ({0})'
56-
matches.append(RuleMatch(
57-
tree[:-1] + [index],
58-
message.format(', '.join(allowed_functions))
59-
))
75+
matches.extend(self._check_equal_values(
76+
cfn, element, tree[:-1] + [index]))
6077
elif not isinstance(element, (six.string_types, bool, six.integer_types, float)):
61-
message = function + ' element must be a String, Boolean, Number, or supported function ({0})'
78+
message = self.function + \
79+
' element must be a String, Boolean, Number, or supported function ({0})'
6280
matches.append(RuleMatch(
6381
tree[:-1] + [index],
64-
message.format(', '.join(allowed_functions))
82+
message.format(', '.join(self.allowed_functions))
6583
))
6684

6785
return matches

‎src/cfnlint/rules/parameters/Types.py

+7-35
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
33
SPDX-License-Identifier: MIT-0
44
"""
5+
from cfnlint.helpers import VALID_PARAMETER_TYPES
56
from cfnlint.rules import CloudFormationLintRule
67
from cfnlint.rules import RuleMatch
78

@@ -14,34 +15,6 @@ class Types(CloudFormationLintRule):
1415
source_url = 'https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/best-practices.html#parmtypes'
1516
tags = ['parameters']
1617

17-
valid_types = [
18-
'AWS::EC2::AvailabilityZone::Name',
19-
'AWS::EC2::Image::Id',
20-
'AWS::EC2::Instance::Id',
21-
'AWS::EC2::KeyPair::KeyName',
22-
'AWS::EC2::SecurityGroup::GroupName',
23-
'AWS::EC2::SecurityGroup::Id',
24-
'AWS::EC2::Subnet::Id',
25-
'AWS::EC2::VPC::Id',
26-
'AWS::EC2::Volume::Id',
27-
'AWS::Route53::HostedZone::Id',
28-
'AWS::SSM::Parameter::Name',
29-
'CommaDelimitedList',
30-
'List<AWS::EC2::AvailabilityZone::Name>',
31-
'List<AWS::EC2::Image::Id>',
32-
'List<AWS::EC2::Instance::Id>',
33-
'List<AWS::EC2::SecurityGroup::GroupName>',
34-
'List<AWS::EC2::SecurityGroup::Id>',
35-
'List<AWS::EC2::Subnet::Id>',
36-
'List<AWS::EC2::VPC::Id>',
37-
'List<AWS::EC2::Volume::Id>',
38-
'List<AWS::Route53::HostedZone::Id>',
39-
'List<Number>',
40-
'List<String>',
41-
'Number',
42-
'String',
43-
]
44-
4518
def match(self, cfn):
4619
matches = []
4720

@@ -50,12 +23,11 @@ def match(self, cfn):
5023
# this test isn't about missing required properties for a
5124
# parameter.
5225
paramtype = paramvalue.get('Type', 'String')
53-
if paramtype not in self.valid_types:
54-
if not paramtype.startswith('AWS::SSM::Parameter::Value'):
55-
message = 'Parameter {0} has invalid type {1}'
56-
matches.append(RuleMatch(
57-
['Parameters', paramname, 'Type'],
58-
message.format(paramname, paramtype)
59-
))
26+
if paramtype not in VALID_PARAMETER_TYPES:
27+
message = 'Parameter {0} has invalid type {1}'
28+
matches.append(RuleMatch(
29+
['Parameters', paramname, 'Type'],
30+
message.format(paramname, paramtype)
31+
))
6032

6133
return matches

‎test/fixtures/templates/bad/conditions/equals.yaml

+14
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,23 @@
11
---
22
AWSTemplateFormatVersion: "2010-09-09"
3+
Parameters:
4+
Environments:
5+
Type: CommaDelimitedList
6+
Default: dev,test
37
Conditions:
48
TestEqual: !Equals 'Value'
59
TestEqualToShort: !Equals ['1']
610
TestEqualToLong: !Equals ['1', '2', '3']
711
Test: !Equals [[{'Ref': 'AWS::Region'}], {'Bad': 'Value'}]
812
TestWrongType: !Equals [1.234, ['Not a List']]
13+
TagEnvironments: !Not
14+
- !Equals
15+
- !Ref Environments # missed !Join
16+
- ''
17+
ToManyFunctions:
18+
Fn::Equals:
19+
- Ref: AWS::Region
20+
Fn::Select: [Environments, 0]
21+
- "dev"
22+
923
Resources: {}

‎test/fixtures/templates/bad/parameters/configuration.yaml

+1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ Parameters:
2626
# invalid Property
2727
NotType: String
2828
mySsmParam:
29+
# Invalid SSM Parameter type
2930
Type: AWS::SSM::Parameter::Value<Test>
3031
Resources:
3132
# Helps tests map resource types

‎test/unit/rules/conditions/test_equals.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -23,4 +23,4 @@ def test_file_positive(self):
2323

2424
def test_file_negative(self):
2525
"""Test failure"""
26-
self.helper_file_negative('test/fixtures/templates/bad/conditions/equals.yaml', 6)
26+
self.helper_file_negative('test/fixtures/templates/bad/conditions/equals.yaml', 8)

‎test/unit/rules/parameters/test_types.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,4 @@ def test_file_positive(self):
2020

2121
def test_file_negative(self):
2222
"""Test failure"""
23-
self.helper_file_negative('test/fixtures/templates/bad/parameters/configuration.yaml', 1)
23+
self.helper_file_negative('test/fixtures/templates/bad/parameters/configuration.yaml', 2)

0 commit comments

Comments
 (0)