Skip to content

Commit

Permalink
feat(conditions) - Add rule W8003 to check if Equals will always be t…
Browse files Browse the repository at this point in the history
…rue or false (aws-cloudformation#2426)

* add rule W8003 to check if Equals will always be true or false
  • Loading branch information
kddejong authored Oct 27, 2022
1 parent b08b8f6 commit d8cac42
Show file tree
Hide file tree
Showing 7 changed files with 111 additions and 3 deletions.
44 changes: 44 additions & 0 deletions src/cfnlint/rules/conditions/EqualsIsUseful.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
"""
Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
SPDX-License-Identifier: MIT-0
"""
import json
from cfnlint.rules import CloudFormationLintRule
from cfnlint.rules import RuleMatch


class EqualsIsUseful(CloudFormationLintRule):
"""Validate that the Equals will return true/false and not always be true or false"""

id = 'W8003'
shortdesc = 'Fn::Equals will always return true or false'
description = 'Validate Fn::Equals to see if its comparing two strings or two equal items. While this works it may not be intended.'
source_url = 'https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/intrinsic-function-reference-conditions.html#intrinsic-function-reference-conditions-equals'
tags = ['functions', 'equals']

function = 'Fn::Equals'

def _check_equal_values(self, values, path):
matches = []

if json.dumps(values[0]) == json.dumps(values[1]):
message = self.function + ' element will alway return true'
matches.append(RuleMatch(path, message))
elif isinstance(values[0], str) and isinstance(values[1], str):
message = self.function + ' element will alway return false'
matches.append(RuleMatch(path, message))
return matches

def match(self, cfn):
matches = []
# Build the list of functions
trees = cfn.search_deep_keys(self.function)

for tree in trees:
# Test when in Conditions
if tree[0] == 'Conditions':
value = tree[-1]
if isinstance(value, list) and len(value) == 2:
matches.extend(self._check_equal_values(value, tree[:-1]))

return matches
28 changes: 28 additions & 0 deletions test/fixtures/results/quickstart/nist_config_rules.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,32 @@
[
{
"Filename": "test/fixtures/templates/quickstart/nist_config_rules.yaml",
"Level": "Warning",
"Location": {
"End": {
"ColumnNumber": 17,
"LineNumber": 5
},
"Path": [
"Conditions",
"cApprovedAMIsRule",
"Fn::Not",
0,
"Fn::Equals"
],
"Start": {
"ColumnNumber": 7,
"LineNumber": 5
}
},
"Message": "Fn::Equals element will alway return true",
"Rule": {
"Description": "Validate Fn::Equals to see if its comparing two strings or two equal items. While this works it may not be intended.",
"Id": "W8003",
"ShortDescription": "Fn::Equals will not always return true or false",
"Source": "https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/intrinsic-function-reference-conditions.html#intrinsic-function-reference-conditions-equals"
}
},
{
"Filename": "test/fixtures/templates/quickstart/nist_config_rules.yaml",
"Level": "Warning",
Expand Down
11 changes: 11 additions & 0 deletions test/fixtures/templates/bad/conditions/equals_not_useful.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
AWSTemplateFormatVersion: "2010-09-09"
Parameters:
Environment:
Type: String
Default: dev
Conditions:
IsTrue: !Equals ['true', 'true']
IsFalse: !Equals ['true', 'false']
IsDevEnvironment: !Equals [!Ref Environment, !Ref Environment]
Resources: {}
2 changes: 1 addition & 1 deletion test/unit/module/cfn_json/test_cfn_json.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def setUp(self):
self.filenames = {
"config_rule": {
"filename": 'test/fixtures/templates/quickstart/config-rules.json',
"failures": 4
"failures": 5
},
"iam": {
"filename": 'test/fixtures/templates/quickstart/iam.json',
Expand Down
2 changes: 1 addition & 1 deletion test/unit/module/cfn_yaml/test_yaml.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def setUp(self):
},
"generic_bad": {
"filename": 'test/fixtures/templates/bad/generic.yaml',
"failures": 29
"failures": 30
}
}

Expand Down
2 changes: 1 addition & 1 deletion test/unit/module/test_rules_collections.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def test_fail_run(self):
filename = 'test/fixtures/templates/bad/generic.yaml'
template = cfnlint.decode.cfn_yaml.load(filename)
cfn = Template(filename, template, ['us-east-1'])
expected_err_count = 32
expected_err_count = 33
matches = []
matches.extend(self.rules.run(filename, cfn))
assert len(matches) == expected_err_count, 'Expected {} failures, got {}'.format(
Expand Down
25 changes: 25 additions & 0 deletions test/unit/rules/conditions/test_equals_is_useful.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
"""
Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
SPDX-License-Identifier: MIT-0
"""
from test.unit.rules import BaseRuleTestCase
from cfnlint.rules.conditions.EqualsIsUseful import EqualsIsUseful # pylint: disable=E0401


class TestEqualsIsUseful(BaseRuleTestCase):
"""Test template mapping configurations"""

def setUp(self):
"""Setup"""
super(TestEqualsIsUseful, self).setUp()
self.collection.register(EqualsIsUseful())

success_templates = []

def test_file_positive(self):
"""Test Positive"""
self.helper_file_positive()

def test_file_negative(self):
"""Test failure"""
self.helper_file_negative('test/fixtures/templates/bad/conditions/equals_not_useful.yaml', 3)

0 comments on commit d8cac42

Please sign in to comment.