Skip to content

Commit 7802398

Browse files
kddejongkddejong
authored
Fix merge for yaml parsing (#4028)
1 parent 4c969de commit 7802398

File tree

7 files changed

+207
-35
lines changed

7 files changed

+207
-35
lines changed

‎src/cfnlint/decode/cfn_yaml.py

Lines changed: 42 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -74,9 +74,15 @@ class NodeConstructor(SafeConstructor):
7474
def __init__(self, filename):
7575
# Call the base class constructor
7676
super().__init__()
77-
7877
self.filename = filename
7978

79+
def flatten_mapping(self, node):
80+
# Rewrote to handle merging and overwriting keys
81+
if any(key_node.tag == "tag:yaml.org,2002:merge" for key_node, _ in node.value):
82+
super().flatten_mapping(node)
83+
setattr(node, "using_merge", True)
84+
super().flatten_mapping(node)
85+
8086
# To support lazy loading, the original constructors first yield
8187
# an empty object, then fill them in when iterated. Due to
8288
# laziness we omit this behaviour (and will only do "deep
@@ -109,46 +115,47 @@ def construct_yaml_map(self, node):
109115
),
110116
],
111117
)
112-
for key_dup in mapping:
113-
if key_dup == key:
114-
if not matches:
115-
matches.extend(
116-
[
117-
build_match(
118-
filename=self.filename,
119-
message=(
120-
f'Duplicate found "{key}" (line'
121-
f" {key_dup.start_mark.line + 1})"
122-
),
123-
line_number=key_dup.start_mark.line,
124-
column_number=key_dup.start_mark.column,
125-
key=key,
126-
),
118+
if not getattr(node, "using_merge", False):
119+
for key_dup in mapping:
120+
if key_dup == key:
121+
if matches:
122+
matches.append(
127123
build_match(
128124
filename=self.filename,
129125
message=(
130-
f'Duplicate found "{key}" (line'
126+
f"Duplicate found {key!r} (line"
131127
f" {key_node.start_mark.line + 1})"
132128
),
133129
line_number=key_node.start_mark.line,
134130
column_number=key_node.start_mark.column,
135131
key=key,
136-
),
137-
],
138-
)
139-
else:
140-
matches.append(
141-
build_match(
142-
filename=self.filename,
143-
message=(
144-
f'Duplicate found "{key}" (line'
145-
f" {key_node.start_mark.line + 1})"
146-
),
147-
line_number=key_node.start_mark.line,
148-
column_number=key_node.start_mark.column,
149-
key=key,
150-
),
151-
)
132+
)
133+
)
134+
else:
135+
matches.extend(
136+
[
137+
build_match(
138+
filename=self.filename,
139+
message=(
140+
f"Duplicate found {key!r} (line"
141+
f" {key_dup.start_mark.line + 1})"
142+
),
143+
line_number=key_dup.start_mark.line,
144+
column_number=key_dup.start_mark.column,
145+
key=key,
146+
),
147+
build_match(
148+
filename=self.filename,
149+
message=(
150+
f"Duplicate found {key!r} (line"
151+
f" {key_node.start_mark.line + 1})"
152+
),
153+
line_number=key_node.start_mark.line,
154+
column_number=key_node.start_mark.column,
155+
key=key,
156+
),
157+
],
158+
)
152159
try:
153160
mapping[key] = value
154161
except Exception as exc:
@@ -176,7 +183,8 @@ def construct_yaml_map(self, node):
176183

177184
(obj,) = SafeConstructor.construct_yaml_map(self, node)
178185

179-
return dict_node(obj, node.start_mark, node.end_mark)
186+
using_merge = False if not hasattr(node, "using_merge") else node.using_merge
187+
return dict_node(obj, node.start_mark, node.end_mark, using_merge)
180188

181189
def construct_yaml_str(self, node):
182190
obj = SafeConstructor.construct_yaml_str(self, node)

‎src/cfnlint/decode/node.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,14 +54,19 @@ class node_class(cls):
5454
"""Node class created based on the input class"""
5555

5656
def __init__(
57-
self, x, start_mark: Mark | None = None, end_mark: Mark | None = None
57+
self,
58+
x,
59+
start_mark: Mark | None = None,
60+
end_mark: Mark | None = None,
61+
using_merge: bool = False,
5862
):
5963
try:
6064
cls.__init__(self, x)
6165
except TypeError:
6266
cls.__init__(self)
6367
self.start_mark = start_mark or Mark()
6468
self.end_mark = end_mark or Mark()
69+
self.using_merge = using_merge
6570

6671
def __deepcopy__(self, memo):
6772
result = dict_node(self, self.start_mark, self.end_mark)

‎src/cfnlint/rules/aws_cli/UsingMerge.py

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
"""
2+
Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
SPDX-License-Identifier: MIT-0
4+
"""
5+
6+
from __future__ import annotations
7+
8+
from typing import Any
9+
10+
from cfnlint.rules import CloudFormationLintRule, RuleMatch
11+
from cfnlint.template import Template
12+
13+
14+
class UsingMerge(CloudFormationLintRule):
15+
id = "W1100"
16+
shortdesc = "Validate if the template is using YAML merge"
17+
description = (
18+
"The CloudFormation service does not support YAML anchors, "
19+
"aliases, or merging. This rule validates if the "
20+
"merge capability is being used"
21+
)
22+
source_url = "https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/template-formats.html"
23+
tags = ["yaml"]
24+
25+
def _nested_obj(self, obj: Any, path: list[str | int]) -> list[RuleMatch]:
26+
matches = []
27+
if isinstance(obj, dict):
28+
if hasattr(obj, "using_merge") and obj.using_merge:
29+
matches.append(
30+
RuleMatch(
31+
path=path,
32+
message=(
33+
"This code is using yaml marge capabilities "
34+
"and can only be deployed using the "
35+
"'package' cli command"
36+
),
37+
)
38+
)
39+
for k, v in obj.items():
40+
matches.extend(self._nested_obj(v, path + [k]))
41+
elif isinstance(obj, list):
42+
for i, e in enumerate(obj):
43+
matches.extend(self._nested_obj(e, path + [i]))
44+
return matches
45+
46+
def match(self, cfn: Template):
47+
48+
return self._nested_obj(cfn.template, [])

‎src/cfnlint/rules/aws_cli/__init__.py

Whitespace-only changes.

‎test/unit/module/cfn_yaml/test_yaml.py

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,3 +75,47 @@ def test_map_failure(self):
7575
cfnlint.decode.cfn_yaml.load,
7676
filename,
7777
)
78+
79+
def test_yaml_merge(self):
80+
raw_template = """
81+
Resources:
82+
Parameter1:
83+
Type: AWS::SSM::Parameter
84+
Properties: &ssm-parameters
85+
Type: String
86+
Value: 1
87+
88+
Parameter2:
89+
Type: AWS::SSM::Parameter
90+
Properties:
91+
<<: *ssm-parameters
92+
Value: 2
93+
"""
94+
95+
result = cfnlint.decode.cfn_yaml.loads(raw_template)
96+
97+
self.assertTrue(
98+
result.get("Resources").get("Parameter2").get("Properties").using_merge
99+
)
100+
101+
self.assertDictEqual(
102+
result,
103+
{
104+
"Resources": {
105+
"Parameter1": {
106+
"Type": "AWS::SSM::Parameter",
107+
"Properties": {
108+
"Type": "String",
109+
"Value": 1,
110+
},
111+
},
112+
"Parameter2": {
113+
"Type": "AWS::SSM::Parameter",
114+
"Properties": {
115+
"Type": "String",
116+
"Value": 2,
117+
},
118+
},
119+
}
120+
},
121+
)

‎test/unit/rules/aws_cli/__init__.py

Whitespace-only changes.

‎test/unit/rules/aws_cli/test_using_merge.py

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
"""
2+
Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
SPDX-License-Identifier: MIT-0
4+
"""
5+
6+
import pytest
7+
8+
from cfnlint.context import create_context_for_template
9+
from cfnlint.decode.cfn_yaml import loads
10+
from cfnlint.rules import RuleMatch
11+
from cfnlint.rules.aws_cli.UsingMerge import UsingMerge
12+
13+
14+
@pytest.fixture(scope="module")
15+
def rule():
16+
rule = UsingMerge()
17+
yield rule
18+
19+
20+
@pytest.fixture(scope="module")
21+
def context(cfn):
22+
return create_context_for_template(cfn)
23+
24+
25+
@pytest.mark.parametrize(
26+
"name,template,expected",
27+
[
28+
(
29+
"A good template",
30+
loads(
31+
"""
32+
One:
33+
A: 1
34+
"""
35+
),
36+
[],
37+
),
38+
(
39+
"A merge template",
40+
loads(
41+
"""
42+
One:
43+
&foo
44+
A: 1
45+
Two:
46+
<<: *foo
47+
A: 2
48+
"""
49+
),
50+
[
51+
RuleMatch(
52+
path=["Two"],
53+
message=(
54+
"This code is using yaml marge capabilities "
55+
"and can only be deployed using the "
56+
"'package' cli command"
57+
),
58+
)
59+
],
60+
),
61+
],
62+
indirect=["template"],
63+
)
64+
def test_validate(name, template, expected, rule, cfn):
65+
errs = list(rule.match(cfn))
66+
67+
assert errs == expected, f"Test {name!r} got {errs!r}"

0 commit comments

Comments
 (0)