Skip to content

Commit ed1e408

Browse files
committed
Add source on resolution clean errors
1 parent 641d93b commit ed1e408

File tree

6 files changed

+138
-39
lines changed

6 files changed

+138
-39
lines changed

src/cfnlint/match.py

+1-3
Original file line numberDiff line numberDiff line change
@@ -118,14 +118,12 @@ def create(
118118
if linenumberend is None:
119119
linenumberend = linenumber
120120

121-
filename = getattr(rulematch_obj, "filename", filename)
122-
123121
return cls(
124122
linenumber=linenumber,
125123
columnnumber=columnnumber,
126124
linenumberend=linenumberend,
127125
columnnumberend=columnnumberend,
128-
filename=filename,
126+
filename=getattr(rulematch_obj, "filename", filename),
129127
rule=rule,
130128
message=message,
131129
rulematch_obj=rulematch_obj,

src/cfnlint/rules/deployment_files/Parameters.py

+2-1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
from __future__ import annotations
77

8+
from pathlib import Path
89
from typing import Any
910

1011
from cfnlint.jsonschema import Validator
@@ -97,5 +98,5 @@ def validate(self, validator: Validator, _: Any, instance: Any, schema: Any):
9798

9899
for err in super()._iter_errors(cfn_validator, parameter_set.parameters):
99100
if parameter_set.source:
100-
err.extra_args["filename"] = parameter_set.source
101+
err.extra_args["filename"] = str(Path(parameter_set.source))
101102
yield err

src/cfnlint/rules/functions/_BaseFn.py

+27-4
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
from collections import namedtuple
99
from typing import Any, Tuple
1010

11-
from cfnlint.helpers import ToPy, ensure_list, is_types_compatible
11+
from cfnlint.helpers import ToPy, ensure_list, is_function, is_types_compatible
1212
from cfnlint.jsonschema import ValidationError, ValidationResult, Validator
1313
from cfnlint.rules import CloudFormationLintRule
1414

@@ -57,14 +57,35 @@ def validator(self, validator: Validator) -> Validator:
5757
)
5858

5959
def _clean_resolve_errors(
60-
self, err: ValidationError, value: Any, instance: Any
60+
self, err: ValidationError, value: Any, instance: Any, validator: Validator
6161
) -> ValidationError:
6262
err.message = err.message.replace(f"{value!r}", f"{instance!r}")
6363
err.message = f"{err.message} when {self.fn.name!r} is resolved"
64+
if validator.context.parameter_sets:
65+
k, v = is_function(instance)
66+
if k == "Ref":
67+
if v in validator.context.parameters:
68+
deployment_files = []
69+
for parameter_set in validator.context.parameter_sets:
70+
if (
71+
v in parameter_set.parameters
72+
and value == parameter_set.parameters[v]
73+
):
74+
if parameter_set.source:
75+
deployment_files.append(parameter_set.source)
76+
else:
77+
err.message = f"{err.message} to {value!r}"
78+
break
79+
else:
80+
err.message = (
81+
f"{err.message} to {value!r} from {deployment_files!r}"
82+
)
6483
if self.child_rules[self.resolved_rule]:
6584
err.rule = self.child_rules[self.resolved_rule]
6685
for i, err_ctx in enumerate(err.context):
67-
err.context[i] = self._clean_resolve_errors(err_ctx, value, instance)
86+
err.context[i] = self._clean_resolve_errors(
87+
err_ctx, value, instance, validator
88+
)
6889
return err
6990

7091
def resolve(
@@ -103,7 +124,9 @@ def resolve(
103124
return
104125

105126
for err in errs:
106-
all_errs.append(self._clean_resolve_errors(err, value, instance))
127+
all_errs.append(
128+
self._clean_resolve_errors(err, value, instance, validator)
129+
)
107130

108131
yield from iter(all_errs)
109132

src/cfnlint/template/template.py

+17-12
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@
1414

1515
import cfnlint.conditions
1616
import cfnlint.helpers
17-
from cfnlint._typing import CheckValueFn, Path
17+
from cfnlint._typing import CheckValueFn
18+
from cfnlint._typing import Path as CfnPath
1819
from cfnlint.context import Context, ParameterSet, create_context_for_template
1920
from cfnlint.context.conditions.exceptions import Unsatisfiable
2021
from cfnlint.decode.node import dict_node, list_node
@@ -357,7 +358,7 @@ def get_directives(self) -> dict[str, list[str]]:
357358
return results
358359

359360
# pylint: disable=dangerous-default-value
360-
def _search_deep_keys(self, searchText: str | re.Pattern, cfndict, path: Path):
361+
def _search_deep_keys(self, searchText: str | re.Pattern, cfndict, path: CfnPath):
361362
"""Search deep for keys and get their values.
362363
363364
Args:
@@ -371,7 +372,7 @@ def _search_deep_keys(self, searchText: str | re.Pattern, cfndict, path: Path):
371372
keys = []
372373
if isinstance(cfndict, dict):
373374
for key in cfndict:
374-
pathprop: Path = path[:]
375+
pathprop: CfnPath = path[:]
375376
pathprop.append(key)
376377
if isinstance(searchText, str):
377378
if key == searchText:
@@ -511,7 +512,9 @@ def _get_cfn_path(
511512

512513
yield from _get_cfn_path(path, self.template, context)
513514

514-
def get_condition_values(self, template, path: Path | None) -> list[dict[str, Any]]:
515+
def get_condition_values(
516+
self, template, path: CfnPath | None
517+
) -> list[dict[str, Any]]:
515518
"""
516519
Evaluates conditions in the provided CloudFormation template and returns the values.
517520
@@ -568,7 +571,7 @@ def get_condition_values(self, template, path: Path | None) -> list[dict[str, An
568571

569572
return matches
570573

571-
def get_values(self, obj, key, path: Path | None = None):
574+
def get_values(self, obj, key, path: CfnPath | None = None):
572575
"""
573576
Logic for getting the value of a key in the provided object.
574577
@@ -694,7 +697,7 @@ def get_sub_parameters(self, sub_string):
694697

695698
return results
696699

697-
def get_location_yaml(self, text: Any, path: Path):
700+
def get_location_yaml(self, text: Any, path: CfnPath):
698701
"""
699702
Get the location information for the given YAML text and path.
700703
@@ -750,7 +753,7 @@ def check_value(
750753
self,
751754
obj: dict[str, Any],
752755
key: str,
753-
path: Path,
756+
path: CfnPath,
754757
check_value: CheckValueFn | None = None,
755758
check_ref: CheckValueFn | None = None,
756759
check_get_att: CheckValueFn | None = None,
@@ -859,7 +862,9 @@ def check_value(
859862

860863
return matches
861864

862-
def is_resource_available(self, path: Path, resource: str) -> list[dict[str, bool]]:
865+
def is_resource_available(
866+
self, path: CfnPath, resource: str
867+
) -> list[dict[str, bool]]:
863868
"""
864869
Compares a path to a resource to see if it is available.
865870
@@ -911,7 +916,7 @@ def is_resource_available(self, path: Path, resource: str) -> list[dict[str, boo
911916
return results
912917

913918
def get_object_without_nested_conditions(
914-
self, obj: dict | list, path: Path, region: str | None = None
919+
self, obj: dict | list, path: CfnPath, region: str | None = None
915920
):
916921
"""
917922
Get a list of object values without conditions included.
@@ -1136,7 +1141,7 @@ def get_object_without_conditions(self, obj, property_names=None, region=None):
11361141

11371142
def get_condition_scenarios_below_path(
11381143
self,
1139-
path: Path,
1144+
path: CfnPath,
11401145
include_if_in_function: bool = False,
11411146
region: str | None = None,
11421147
) -> list[dict[str, bool]]:
@@ -1238,7 +1243,7 @@ def get_conditions_from_property(value):
12381243
def get_conditions_from_path(
12391244
self,
12401245
text: Any,
1241-
path: Path,
1246+
path: CfnPath,
12421247
include_resource_conditions: bool = True,
12431248
include_if_in_function: bool = True,
12441249
only_last: bool = False,
@@ -1282,7 +1287,7 @@ def get_conditions_from_path(
12821287
def _get_conditions_from_path(
12831288
self,
12841289
text: Any,
1285-
path: Path,
1290+
path: CfnPath,
12861291
include_if_in_function: bool = True,
12871292
only_last: bool = False,
12881293
) -> dict[str, set[bool]]:

test/integration/test_deployment_files.py

+31-16
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
SPDX-License-Identifier: MIT-0
44
"""
55

6+
from pathlib import Path
7+
68
import pytest
79

810
from cfnlint.config import ConfigMixIn
@@ -27,7 +29,7 @@
2729
Match(
2830
message="'ImageId' is a required property",
2931
rule=Parameters(),
30-
filename="test/fixtures/deployment_files/dev.yaml",
32+
filename=str(Path("test/fixtures/deployment_files/dev.yaml")),
3133
linenumber=1,
3234
linenumberend=1,
3335
columnnumber=1,
@@ -39,7 +41,7 @@
3941
"('Environment' was unexpected)"
4042
),
4143
rule=Parameters(),
42-
filename="test/fixtures/deployment_files/dev.yaml",
44+
filename=str(Path("test/fixtures/deployment_files/dev.yaml")),
4345
linenumber=3,
4446
linenumberend=3,
4547
columnnumber=3,
@@ -59,7 +61,7 @@
5961
Match(
6062
message="'ImageId' is a required property",
6163
rule=Parameters(),
62-
filename="test/fixtures/deployment_files/dev.yaml",
64+
filename=str(Path("test/fixtures/deployment_files/dev.yaml")),
6365
linenumber=1,
6466
linenumberend=1,
6567
columnnumber=1,
@@ -71,7 +73,7 @@
7173
"('Environment' was unexpected)"
7274
),
7375
rule=Parameters(),
74-
filename="test/fixtures/deployment_files/dev.yaml",
76+
filename=str(Path("test/fixtures/deployment_files/dev.yaml")),
7577
linenumber=3,
7678
linenumberend=3,
7779
columnnumber=3,
@@ -80,7 +82,7 @@
8082
Match(
8183
message="'host' is not one of ['default', 'dedicated']",
8284
rule=Parameters(),
83-
filename="test/fixtures/deployment_files/stage.yaml",
85+
filename=str(Path("test/fixtures/deployment_files/stage.yaml")),
8486
linenumber=5,
8587
linenumberend=5,
8688
columnnumber=3,
@@ -89,10 +91,16 @@
8991
Match(
9092
message=(
9193
"{'Ref': 'Affinity'} is not one of ['default', 'host'] "
92-
"when 'Ref' is resolved"
94+
"when 'Ref' is resolved to 'dne' from "
95+
"['test/fixtures/deployment_files/test.yaml', "
96+
"'test/fixtures/deployment_files/stage.yaml']"
9397
),
9498
rule=RefResolved(),
95-
filename="test/fixtures/templates/integration/deployment-file-template.yaml",
99+
filename=str(
100+
Path(
101+
"test/fixtures/templates/integration/deployment-file-template.yaml"
102+
)
103+
),
96104
linenumber=34,
97105
linenumberend=34,
98106
columnnumber=7,
@@ -102,10 +110,16 @@
102110
message=(
103111
"{'Ref': 'ImageId'} is not a 'AWS::EC2::Image.Id' with "
104112
"pattern '^ami-([0-9a-z]{8}|[0-9a-z]{17})$' when 'Ref' "
105-
"is resolved"
113+
"is resolved to 'ami-zxyzabc123' from "
114+
"['test/fixtures/deployment_files/test.yaml', "
115+
"'test/fixtures/deployment_files/stage.yaml']"
106116
),
107117
rule=RefResolved(),
108-
filename="test/fixtures/templates/integration/deployment-file-template.yaml",
118+
filename=str(
119+
Path(
120+
"test/fixtures/templates/integration/deployment-file-template.yaml"
121+
)
122+
),
109123
linenumber=35,
110124
linenumberend=35,
111125
columnnumber=7,
@@ -129,12 +143,13 @@ def test_deployment_files(
129143
runner = Runner(config)
130144
results = list(runner.run())
131145

132-
# for result in results:
133-
# print(result.message)
134-
# print(result.rule)
135-
# print(result.linenumber)
136-
# print(result.linenumberend)
137-
# print(result.columnnumber)
138-
# print(result.columnnumberend)
146+
for i, result in enumerate(results):
147+
print(result.message)
148+
print(result.rule)
149+
print(result.filename, expected[i].filename)
150+
print(result.linenumber)
151+
print(result.linenumberend)
152+
print(result.columnnumber)
153+
print(result.columnnumberend)
139154

140155
assert results == expected, f"{name}: {results} != {expected}"

0 commit comments

Comments
 (0)