Skip to content

Commit 0e9e807

Browse files
kddejongkddejong
authored
Add logic to handle Ref AWS::NoValue in list (#3563)
* Add logic to handle Ref AWS::NoValue in list * use context instead a bunch of params
1 parent b16d6b4 commit 0e9e807

File tree

12 files changed

+410
-29
lines changed

12 files changed

+410
-29
lines changed

‎scripts/update_snapshot_results.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
# integration/
44
cfn-lint test/fixtures/templates/integration/dynamic-references.yaml -e -c I --format json > test/fixtures/results/integration/dynamic-references.json
55
cfn-lint test/fixtures/templates/integration/resources-cloudformation-init.yaml -e -c I --format json > test/fixtures/results/integration/resources-cloudformation-init.json
6+
cfn-lint test/fixtures/templates/integration/ref-no-value.yaml -e -c I --format json > test/fixtures/results/integration/ref-no-value.json
67

78
# public/
89
cfn-lint test/fixtures/templates/public/lambda-poller.yaml -e -c I --format json > test/fixtures/results/public/lambda-poller.json

‎src/cfnlint/context/conditions/_conditions.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
from cfnlint.context.conditions.exceptions import Unsatisfiable
2323

2424
if TYPE_CHECKING:
25-
from cfnlint.context.context import Parameter
25+
from cfnlint.context.context import Context, Parameter
2626

2727

2828
@dataclass(frozen=True)
@@ -127,13 +127,18 @@ def _build_conditions(self, conditions: set[str]) -> Iterator["Conditions"]:
127127
if scenarios_attempted >= self._max_scenarios:
128128
return
129129

130-
def evolve_from_instance(self, instance: Any) -> Iterator[tuple[Any, "Conditions"]]:
130+
def evolve_from_instance(
131+
self, instance: Any, context: "Context"
132+
) -> Iterator[tuple[Any, "Conditions"]]:
131133

132134
conditions = get_conditions_from_property(instance)
133135

134136
for scenario in self._build_conditions(conditions):
135137
yield build_instance_from_scenario(
136-
instance, scenario.status, is_root=True
138+
instance,
139+
scenario.status,
140+
is_root=True,
141+
context=context,
137142
), scenario
138143

139144
@property

‎src/cfnlint/context/conditions/_utils.py

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,14 @@
55

66
from __future__ import annotations
77

8-
from typing import Any
8+
from typing import TYPE_CHECKING, Any
99

1010
from cfnlint.decode.node import Mark, dict_node, list_node
1111
from cfnlint.helpers import is_function
1212

13+
if TYPE_CHECKING:
14+
from cfnlint.context.context import Context
15+
1316

1417
def get_conditions_from_property(instance: Any, is_root: bool = True) -> set[str]:
1518
"""
@@ -50,7 +53,10 @@ def get_conditions_from_property(instance: Any, is_root: bool = True) -> set[str
5053

5154

5255
def build_instance_from_scenario(
53-
instance: Any, scenario: dict[str, bool], is_root: bool = True
56+
instance: Any,
57+
scenario: dict[str, bool],
58+
is_root: bool,
59+
context: "Context",
5460
) -> Any:
5561
"""
5662
Get object values from a provided scenario.
@@ -76,35 +82,48 @@ def build_instance_from_scenario(
7682
getattr(instance, "end_mark", Mark(0, 0)),
7783
)
7884
for v in instance:
79-
new_value = build_instance_from_scenario(v, scenario, is_root=False)
85+
new_value = build_instance_from_scenario(
86+
v,
87+
scenario,
88+
is_root=False,
89+
context=context,
90+
)
8091
if new_value is not None:
8192
new_list.append(new_value)
8293
return new_list
8394

8495
if isinstance(instance, dict):
8596
fn_k, fn_v = is_function(instance)
86-
if fn_k == "Fn::If":
97+
if fn_k == "Fn::If" and "Fn::If" in context.functions:
8798
if isinstance(fn_v, list) and len(fn_v) == 3:
8899
if isinstance(fn_v[0], str):
89100
if_path = scenario.get(fn_v[0], None)
90101
if if_path is not None:
91102
new_value = build_instance_from_scenario(
92-
fn_v[1] if if_path else fn_v[2], scenario, is_root
103+
fn_v[1] if if_path else fn_v[2],
104+
scenario,
105+
is_root,
106+
context=context,
93107
)
94108
if new_value is not None:
95109
return new_value
96110
return None
97111
return instance
98-
if fn_k == "Ref" and fn_v == "AWS::NoValue":
99-
return {} if is_root else None
112+
if fn_k == "Ref" and fn_v == "AWS::NoValue" and "Ref" in context.functions:
113+
return None
100114
if is_root:
101115
new_obj: dict[str, Any] = dict_node(
102116
{},
103117
getattr(instance, "start_mark", Mark(0, 0)),
104118
getattr(instance, "end_mark", Mark(0, 0)),
105119
)
106120
for k, v in instance.items():
107-
new_value = build_instance_from_scenario(v, scenario, is_root=False)
121+
new_value = build_instance_from_scenario(
122+
v,
123+
scenario,
124+
is_root=False,
125+
context=context,
126+
)
108127
if new_value is not None:
109128
new_obj[k] = new_value
110129
return new_obj

‎src/cfnlint/jsonschema/_filter.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,12 @@ def filter(
138138
for (
139139
scenario_instance,
140140
scenario,
141-
) in validator.context.conditions.evolve_from_instance(instance):
141+
) in validator.context.conditions.evolve_from_instance(
142+
instance,
143+
context=validator.context,
144+
):
145+
if scenario_instance is None:
146+
continue
142147
scenario_validator = validator.evolve(
143148
context=validator.context.evolve(conditions=scenario)
144149
)

‎src/cfnlint/rules/resources/properties/Properties.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
from collections import deque
88
from typing import Any
99

10-
from cfnlint.helpers import FUNCTIONS
10+
from cfnlint.helpers import FUNCTIONS, is_function
1111
from cfnlint.jsonschema import ValidationResult, Validator
1212
from cfnlint.rules.jsonschema.CfnLintJsonSchema import CfnLintJsonSchema
1313
from cfnlint.schema.manager import PROVIDER_SCHEMA_MANAGER
@@ -85,6 +85,10 @@ def validate(
8585
return
8686

8787
properties = instance.get("Properties", {})
88+
fn_k, fn_v = is_function(properties)
89+
if fn_k == "Ref" and fn_v == "AWS::NoValue":
90+
properties = {}
91+
8892
for regions, schema in PROVIDER_SCHEMA_MANAGER.get_resource_schemas_by_regions(
8993
t, validator.context.regions
9094
):

‎src/cfnlint/rules/resources/properties/Tagging.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,9 @@ def tagging(self, validator: Validator, t: Any, instance: Any, schema: Any):
3232
validator = validator.evolve(
3333
function_filter=validator.function_filter.evolve(
3434
add_cfn_lint_keyword=False,
35-
)
35+
),
3636
)
37+
3738
for err in validator.descend(
3839
instance=instance,
3940
schema=self._schema,

‎test/fixtures/results/integration/ref-no-value.json

Lines changed: 211 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,211 @@
1+
[
2+
{
3+
"Filename": "test/fixtures/templates/integration/ref-no-value.yaml",
4+
"Id": "bc432adb-6b22-c519-4df7-26cd9aa0c67f",
5+
"Level": "Error",
6+
"Location": {
7+
"End": {
8+
"ColumnNumber": 3,
9+
"LineNumber": 24
10+
},
11+
"Path": [
12+
"Resources",
13+
"IamRole1",
14+
"Properties",
15+
"Tags",
16+
3
17+
],
18+
"Start": {
19+
"ColumnNumber": 11,
20+
"LineNumber": 22
21+
}
22+
},
23+
"Message": "'Key' is a required property",
24+
"ParentId": null,
25+
"Rule": {
26+
"Description": "Make sure that Resources properties that are required exist",
27+
"Id": "E3003",
28+
"ShortDescription": "Required Resource properties are missing",
29+
"Source": "https://github.com/aws-cloudformation/cfn-lint/blob/main/docs/cfn-schema-specification.md#required"
30+
}
31+
},
32+
{
33+
"Filename": "test/fixtures/templates/integration/ref-no-value.yaml",
34+
"Id": "c9effb29-057b-04bc-7889-c95a6d3de51d",
35+
"Level": "Error",
36+
"Location": {
37+
"End": {
38+
"ColumnNumber": 3,
39+
"LineNumber": 24
40+
},
41+
"Path": [
42+
"Resources",
43+
"IamRole1",
44+
"Properties",
45+
"Tags",
46+
3
47+
],
48+
"Start": {
49+
"ColumnNumber": 11,
50+
"LineNumber": 22
51+
}
52+
},
53+
"Message": "'Key' is a required property",
54+
"ParentId": null,
55+
"Rule": {
56+
"Description": "Validates tag values to make sure they have unique keys and they follow pattern requirements",
57+
"Id": "E3024",
58+
"ShortDescription": "Validate tag configuration",
59+
"Source": "https://docs.aws.amazon.com/tag-editor/latest/userguide/tagging.html"
60+
}
61+
},
62+
{
63+
"Filename": "test/fixtures/templates/integration/ref-no-value.yaml",
64+
"Id": "2ed7922b-1cd5-30ae-0c5f-918f69c9b782",
65+
"Level": "Error",
66+
"Location": {
67+
"End": {
68+
"ColumnNumber": 15,
69+
"LineNumber": 26
70+
},
71+
"Path": [
72+
"Resources",
73+
"IamRole2",
74+
"Properties"
75+
],
76+
"Start": {
77+
"ColumnNumber": 5,
78+
"LineNumber": 26
79+
}
80+
},
81+
"Message": "'AssumeRolePolicyDocument' is a required property",
82+
"ParentId": null,
83+
"Rule": {
84+
"Description": "Make sure that Resources properties that are required exist",
85+
"Id": "E3003",
86+
"ShortDescription": "Required Resource properties are missing",
87+
"Source": "https://github.com/aws-cloudformation/cfn-lint/blob/main/docs/cfn-schema-specification.md#required"
88+
}
89+
},
90+
{
91+
"Filename": "test/fixtures/templates/integration/ref-no-value.yaml",
92+
"Id": "edfc8e64-ad37-e697-8fb7-5c89d2ff1735",
93+
"Level": "Error",
94+
"Location": {
95+
"End": {
96+
"ColumnNumber": 15,
97+
"LineNumber": 39
98+
},
99+
"Path": [
100+
"Resources",
101+
"CloudFront1",
102+
"Properties"
103+
],
104+
"Start": {
105+
"ColumnNumber": 5,
106+
"LineNumber": 39
107+
}
108+
},
109+
"Message": "'DistributionConfig' is a required property",
110+
"ParentId": null,
111+
"Rule": {
112+
"Description": "Make sure that Resources properties that are required exist",
113+
"Id": "E3003",
114+
"ShortDescription": "Required Resource properties are missing",
115+
"Source": "https://github.com/aws-cloudformation/cfn-lint/blob/main/docs/cfn-schema-specification.md#required"
116+
}
117+
},
118+
{
119+
"Filename": "test/fixtures/templates/integration/ref-no-value.yaml",
120+
"Id": "aabc1fd4-1a8c-eca6-cedc-2270876f6523",
121+
"Level": "Error",
122+
"Location": {
123+
"End": {
124+
"ColumnNumber": 25,
125+
"LineNumber": 43
126+
},
127+
"Path": [
128+
"Resources",
129+
"CloudFront2",
130+
"Properties",
131+
"DistributionConfig"
132+
],
133+
"Start": {
134+
"ColumnNumber": 7,
135+
"LineNumber": 43
136+
}
137+
},
138+
"Message": "'DefaultCacheBehavior' is a required property",
139+
"ParentId": null,
140+
"Rule": {
141+
"Description": "Make sure that Resources properties that are required exist",
142+
"Id": "E3003",
143+
"ShortDescription": "Required Resource properties are missing",
144+
"Source": "https://github.com/aws-cloudformation/cfn-lint/blob/main/docs/cfn-schema-specification.md#required"
145+
}
146+
},
147+
{
148+
"Filename": "test/fixtures/templates/integration/ref-no-value.yaml",
149+
"Id": "9409bd79-0f38-7a61-1eb7-a4705b6e650a",
150+
"Level": "Error",
151+
"Location": {
152+
"End": {
153+
"ColumnNumber": 15,
154+
"LineNumber": 49
155+
},
156+
"Path": [
157+
"Resources",
158+
"CloudFront2",
159+
"Properties",
160+
"DistributionConfig",
161+
"DefaultCacheBehavior",
162+
"Fn::If",
163+
2
164+
],
165+
"Start": {
166+
"ColumnNumber": 13,
167+
"LineNumber": 49
168+
}
169+
},
170+
"Message": "'TargetOriginId' is a required property",
171+
"ParentId": null,
172+
"Rule": {
173+
"Description": "Make sure that Resources properties that are required exist",
174+
"Id": "E3003",
175+
"ShortDescription": "Required Resource properties are missing",
176+
"Source": "https://github.com/aws-cloudformation/cfn-lint/blob/main/docs/cfn-schema-specification.md#required"
177+
}
178+
},
179+
{
180+
"Filename": "test/fixtures/templates/integration/ref-no-value.yaml",
181+
"Id": "6e5a7510-66ea-63ee-96d4-08a42d6340e4",
182+
"Level": "Error",
183+
"Location": {
184+
"End": {
185+
"ColumnNumber": 15,
186+
"LineNumber": 49
187+
},
188+
"Path": [
189+
"Resources",
190+
"CloudFront2",
191+
"Properties",
192+
"DistributionConfig",
193+
"DefaultCacheBehavior",
194+
"Fn::If",
195+
2
196+
],
197+
"Start": {
198+
"ColumnNumber": 13,
199+
"LineNumber": 49
200+
}
201+
},
202+
"Message": "'ViewerProtocolPolicy' is a required property",
203+
"ParentId": null,
204+
"Rule": {
205+
"Description": "Make sure that Resources properties that are required exist",
206+
"Id": "E3003",
207+
"ShortDescription": "Required Resource properties are missing",
208+
"Source": "https://github.com/aws-cloudformation/cfn-lint/blob/main/docs/cfn-schema-specification.md#required"
209+
}
210+
}
211+
]

0 commit comments

Comments
 (0)