Skip to content

Commit ed39a19

Browse files
authored
Allow Ref AWS::NoValue in FindInMap parameters (#3399)
1 parent 5d5e6c8 commit ed39a19

File tree

6 files changed

+142
-40
lines changed

6 files changed

+142
-40
lines changed

src/cfnlint/jsonschema/_filter.py

+14-8
Original file line numberDiff line numberDiff line change
@@ -36,18 +36,18 @@ class FunctionFilter:
3636
init=False,
3737
default_factory=lambda: [
3838
"dependencies",
39-
"if",
40-
"then",
41-
"else",
42-
"required",
43-
"requiredXor",
44-
"requiredAtLeastOne",
4539
"dependentExcluded",
4640
"dependentRequired",
47-
"minItems",
48-
"minProperties",
41+
"else",
42+
"if",
4943
"maxItems",
5044
"maxProperties",
45+
"minItems",
46+
"minProperties",
47+
"required",
48+
"requiredAtLeastOne",
49+
"requiredXor",
50+
"then",
5151
"uniqueItems",
5252
],
5353
)
@@ -116,6 +116,12 @@ def filter(self, validator: Any, instance: Any, schema: Any):
116116
return
117117
return
118118

119+
# if there are no functions then we don't need to worry
120+
# about ref AWS::NoValue or If conditions
121+
if not validator.context.functions:
122+
yield instance, schema
123+
return
124+
119125
# dependencies, required, minProperties, maxProperties
120126
# need to have filtered properties to validate
121127
# because of Ref: AWS::NoValue

src/cfnlint/rules/functions/FindInMap.py

+40-2
Original file line numberDiff line numberDiff line change
@@ -62,24 +62,62 @@ def schema(self, validator: Validator, instance: Any) -> dict[str, Any]:
6262
scalar_schema,
6363
scalar_schema,
6464
{
65+
"functions": [],
6566
"schema": {
67+
"findinmap_parameters": True,
6668
"type": ["object"],
6769
"properties": {
6870
"DefaultValue": {
69-
"type": ("array",) + singular_types,
71+
"default_value": True,
7072
}
7173
},
7274
"additionalProperties": False,
7375
"required": ["DefaultValue"],
74-
}
76+
},
7577
},
7678
]
7779

7880
return schema
7981

82+
def _default_value(
83+
self, validator: Validator, s: Any, instance: Any, schema: Any
84+
) -> ValidationResult:
85+
validator = validator.evolve(
86+
context=validator.context.evolve(
87+
functions=["Ref"],
88+
),
89+
)
90+
yield from validator.descend(
91+
instance,
92+
{
93+
"type": ("array",) + singular_types,
94+
},
95+
)
96+
8097
def fn_findinmap(
8198
self, validator: Validator, s: Any, instance: Any, schema: Any
8299
) -> ValidationResult:
100+
101+
if validator.context.transforms.has_language_extensions_transform():
102+
# we have to use a special validator for this
103+
# as we don't want DefaultValue: !Ref AWS::NoValue
104+
# is valid
105+
mapping_validator = validator.extend(
106+
validators={
107+
"default_value": self._default_value,
108+
},
109+
)(validator.schema)
110+
validator = mapping_validator.evolve(
111+
context=validator.context.evolve(
112+
resources={},
113+
),
114+
cfn=validator.cfn,
115+
function_filter=validator.function_filter,
116+
resolver=validator.resolver,
117+
)
118+
yield from super().validate(validator, s, instance, schema)
119+
return
120+
83121
validator = validator.evolve(
84122
context=validator.context.evolve(
85123
resources={},

test/unit/module/jsonschema/test_filter.py

+42-3
Original file line numberDiff line numberDiff line change
@@ -20,29 +20,68 @@ def filter():
2020

2121

2222
@pytest.mark.parametrize(
23-
"name,instance,schema,path,expected",
23+
"name,instance,schema,path,functions,expected",
2424
[
2525
(
2626
"Don't validate dynamic references inside of function",
2727
"{{resolve:ssm:${AWS::AccountId}/${AWS::Region}/ac}}",
2828
{"enum": "Foo"},
2929
deque(["Foo", "Test", "Fn::Sub"]),
3030
[],
31+
[],
3132
),
3233
(
3334
"Validate dynamic references",
3435
"{{resolve:ssm:secret}}",
3536
{"enum": "Foo"},
3637
deque(["Foo", "Test"]),
38+
[],
3739
[
3840
("{{resolve:ssm:secret}}", {"dynamicReference": {"enum": "Foo"}}),
3941
],
4042
),
43+
(
44+
"Lack of functions returns the schema and instance",
45+
{
46+
"Foo": {"Ref": "AWS::NoValue"},
47+
},
48+
{"required": ["Foo"]},
49+
deque([]),
50+
[],
51+
[
52+
(
53+
{
54+
"Foo": {"Ref": "AWS::NoValue"},
55+
},
56+
{"required": ["Foo"]},
57+
),
58+
],
59+
),
60+
(
61+
"Filtered schemas",
62+
{
63+
"Foo": {"Ref": "AWS::NoValue"},
64+
},
65+
{"required": ["Foo"]},
66+
deque([]),
67+
["Ref", "Fn::If"],
68+
[
69+
(
70+
{},
71+
{"required": ["Foo"]},
72+
),
73+
({"Foo": {"Ref": "AWS::NoValue"}}, {"cfnLint": [""]}),
74+
],
75+
),
4176
],
4277
)
43-
def test_filter(name, instance, schema, path, expected, filter):
78+
def test_filter(name, instance, schema, path, functions, expected, filter):
4479
validator = CfnTemplateValidator(
45-
context=Context(regions=["us-east-1"], path=Path(path)),
80+
context=Context(
81+
regions=["us-east-1"],
82+
path=Path(path),
83+
functions=functions,
84+
),
4685
schema=schema,
4786
)
4887
results = list(filter.filter(validator, instance, schema))

test/unit/rules/functions/test_find_in_map.py

+28-9
Original file line numberDiff line numberDiff line change
@@ -46,15 +46,15 @@ def context(cfn):
4646
{"Fn::FindInMap": ["A", "B", "C"]},
4747
{"type": "string"},
4848
{},
49-
[],
49+
None,
5050
[],
5151
),
5252
(
5353
"Invalid Fn::FindInMap too long",
5454
{"Fn::FindInMap": ["foo", "bar", "key", "key2"]},
5555
{"type": "string"},
5656
{},
57-
[],
57+
None,
5858
[
5959
ValidationError(
6060
"['foo', 'bar', 'key', 'key2'] is too long (3)",
@@ -69,7 +69,7 @@ def context(cfn):
6969
{"Fn::FindInMap": {"foo": "bar"}},
7070
{"type": "string"},
7171
{},
72-
[],
72+
None,
7373
[
7474
ValidationError(
7575
"{'foo': 'bar'} is not of type 'array'",
@@ -84,7 +84,7 @@ def context(cfn):
8484
{"Fn::FindInMap": [{"Fn::GetAtt": "MyResource.Arn"}, "foo", "bar"]},
8585
{"type": "string"},
8686
{},
87-
[],
87+
None,
8888
[
8989
ValidationError(
9090
"{'Fn::GetAtt': 'MyResource.Arn'} is not of type 'string'",
@@ -99,15 +99,15 @@ def context(cfn):
9999
{"Fn::FindInMap": ["A", "B", "C", {"DefaultValue": "D"}]},
100100
{"type": "string"},
101101
{"transforms": Transforms(["AWS::LanguageExtensions"])},
102-
[],
102+
None,
103103
[],
104104
),
105105
(
106106
"Invalid Fn::FindInMap options not of type object",
107107
{"Fn::FindInMap": ["A", "B", "C", []]},
108108
{"type": "string"},
109109
{"transforms": Transforms(["AWS::LanguageExtensions"])},
110-
[],
110+
None,
111111
[
112112
ValidationError(
113113
"[] is not of type 'object'",
@@ -122,7 +122,7 @@ def context(cfn):
122122
{"Fn::FindInMap": ["A", "B", "C", {}]},
123123
{"type": "string"},
124124
{"transforms": Transforms(["AWS::LanguageExtensions"])},
125-
[],
125+
None,
126126
[
127127
ValidationError(
128128
"'DefaultValue' is a required property",
@@ -147,6 +147,21 @@ def context(cfn):
147147
),
148148
],
149149
),
150+
(
151+
"Valid Fn::FindInMap with a Ref to AWS::NoValue",
152+
{
153+
"Fn::FindInMap": [
154+
"A",
155+
"B",
156+
"C",
157+
{"DefaultValue": {"Ref": "AWS::NoValue"}},
158+
]
159+
},
160+
{"type": "string"},
161+
{"transforms": Transforms(["AWS::LanguageExtensions"])},
162+
[],
163+
[],
164+
),
150165
],
151166
)
152167
def test_validate(
@@ -162,10 +177,14 @@ def test_validate(
162177
):
163178
context = context.evolve(**context_evolve)
164179
ref_mock = MagicMock()
165-
ref_mock.return_value = iter(ref_mock_values)
180+
ref_mock.return_value = iter(ref_mock_values or [])
166181
validator = CfnTemplateValidator({}).extend(validators={"ref": ref_mock})(
167182
context=context, cfn=cfn
168183
)
169184
errs = list(rule.fn_findinmap(validator, schema, instance, {}))
170-
assert ref_mock.call_count == len(ref_mock_values)
185+
186+
if ref_mock_values is None:
187+
ref_mock.assert_not_called()
188+
else:
189+
assert ref_mock.call_count == len(ref_mock_values) or 1
171190
assert errs == expected, f"Test {name!r} got {errs!r}"

test/unit/rules/resources/ecs/test_task_definition_essentials.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,8 @@ def rule():
5757
"At least one essential container is required",
5858
rule=TaskDefinitionEssentialContainer(),
5959
path=deque([]),
60-
validator="minItems",
61-
schema_path=deque(["minItems"]),
60+
validator="contains",
61+
schema_path=deque(["contains"]),
6262
)
6363
],
6464
),

test/unit/rules/resources/route53/test_recordsets.py

+16-16
Original file line numberDiff line numberDiff line change
@@ -341,22 +341,6 @@ def rule():
341341
],
342342
},
343343
[
344-
ValidationError(
345-
"['cname1.example.com', 'foo√bar'] is too long (1)",
346-
rule=RecordSet(),
347-
path=deque(["ResourceRecords"]),
348-
validator="maxItems",
349-
schema_path=deque(
350-
[
351-
"allOf",
352-
3,
353-
"then",
354-
"properties",
355-
"ResourceRecords",
356-
"maxItems",
357-
]
358-
),
359-
),
360344
ValidationError(
361345
"'foo√bar' is not valid under any of the given schemas",
362346
rule=RecordSet(),
@@ -398,6 +382,22 @@ def rule():
398382
]
399383
),
400384
),
385+
ValidationError(
386+
"['cname1.example.com', 'foo√bar'] is too long (1)",
387+
rule=RecordSet(),
388+
path=deque(["ResourceRecords"]),
389+
validator="maxItems",
390+
schema_path=deque(
391+
[
392+
"allOf",
393+
3,
394+
"then",
395+
"properties",
396+
"ResourceRecords",
397+
"maxItems",
398+
]
399+
),
400+
),
401401
],
402402
),
403403
(

0 commit comments

Comments
 (0)