Skip to content

Commit 56ebe73

Browse files
roger-zhanggroger-zhangg
authored
Fix(Intrinsic Resolver): should ignore CFN placeholder in parameter (#3824)
1 parent 2d68188 commit 56ebe73

File tree

3 files changed

+176
-6
lines changed

3 files changed

+176
-6
lines changed

‎bin/run_cfn_lint.sh‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,22 @@ if [ ! -d "${VENV}" ]; then
1010
fi
1111

1212
"${VENV}/bin/python" -m pip install cfn-lint --upgrade --quiet
13-
# update cfn schema
14-
"${VENV}/bin/cfn-lint" -u
13+
# update cfn schema with retry logic (can fail due to network issues)
14+
MAX_RETRIES=3
15+
RETRY_COUNT=0
16+
while [ $RETRY_COUNT -lt $MAX_RETRIES ]; do
17+
if "${VENV}/bin/cfn-lint" -u; then
18+
echo "Successfully updated cfn-lint schema"
19+
break
20+
else
21+
RETRY_COUNT=$((RETRY_COUNT + 1))
22+
if [ $RETRY_COUNT -lt $MAX_RETRIES ]; then
23+
echo "cfn-lint schema update failed, retrying... (attempt $RETRY_COUNT of $MAX_RETRIES)"
24+
sleep 2
25+
else
26+
echo "cfn-lint schema update failed after $MAX_RETRIES attempts"
27+
exit 1
28+
fi
29+
fi
30+
done
1531
"${VENV}/bin/cfn-lint" --format parseable

‎samtranslator/intrinsics/actions.py‎

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,31 @@
55
from samtranslator.model.exceptions import InvalidDocumentException, InvalidTemplateException
66

77

8+
def _get_parameter_value(parameters: Dict[str, Any], param_name: str, default: Any = None) -> Any:
9+
"""
10+
Get parameter value from parameters dict, but return default (None) if
11+
- it's a CloudFormation internal placeholder.
12+
- param_name is not in the parameters.
13+
14+
CloudFormation internal placeholders are passed during changeset creation with --include-nested-stacks
15+
when there are cross-references between nested stacks that don't exist yet.
16+
These placeholders should not be resolved by SAM.
17+
18+
:param parameters: Dictionary of parameter values
19+
:param param_name: Name of the parameter to retrieve
20+
:param default: Default value to return if parameter not found or is a placeholder
21+
:return: Parameter value, or default if not found or is a CloudFormation placeholder
22+
"""
23+
value = parameters.get(param_name, default)
24+
25+
# Check if the value is a CloudFormation internal placeholder
26+
# E.g. {{IntrinsicFunction:api-xx/MyStack.Outputs.API/Fn::GetAtt}}
27+
if isinstance(value, str) and value.startswith("{{IntrinsicFunction:"):
28+
return default
29+
30+
return value
31+
32+
833
class Action(ABC):
934
"""
1035
Base class for intrinsic function actions. Each intrinsic function must subclass this,
@@ -103,9 +128,9 @@ def resolve_parameter_refs(self, input_dict: Optional[Any], parameters: Dict[str
103128
if not isinstance(param_name, str):
104129
return input_dict
105130

106-
if param_name in parameters:
107-
return parameters[param_name]
108-
return input_dict
131+
# Use the wrapper function to get parameter value
132+
# It returns the original input unchanged if the parameter is a CloudFormation internal placeholder
133+
return _get_parameter_value(parameters, param_name, input_dict)
109134

110135
def resolve_resource_refs(
111136
self, input_dict: Optional[Any], supported_resource_refs: Dict[str, Any]
@@ -193,7 +218,9 @@ def do_replacement(full_ref: str, prop_name: str) -> Any:
193218
:param prop_name: => logicalId.property
194219
:return: Either the value it resolves to. If not the original reference
195220
"""
196-
return parameters.get(prop_name, full_ref)
221+
# Use the wrapper function to get parameter value
222+
# It returns the original input unchanged if the parameter is a CloudFormation internal placeholder
223+
return _get_parameter_value(parameters, prop_name, full_ref)
197224

198225
return self._handle_sub_action(input_dict, do_replacement)
199226

‎tests/intrinsics/test_resolver.py‎

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,133 @@ def test_short_circuit_on_empty_parameters(self):
119119
self.assertEqual(resolver.resolve_parameter_refs(input), expected)
120120
resolver._try_resolve_parameter_refs.assert_not_called()
121121

122+
def test_cloudformation_internal_placeholder_not_resolved(self):
123+
"""Test that CloudFormation internal placeholders are not resolved"""
124+
parameter_values = {
125+
"UserPoolArn": "{{IntrinsicFunction:debugging-cloudformation-issues4/Cognito.Outputs.UserPoolArn/Fn::GetAtt}}",
126+
"NormalParam": "normal-value",
127+
}
128+
resolver = IntrinsicsResolver(parameter_values)
129+
130+
# CloudFormation placeholder should not be resolved
131+
input1 = {"Ref": "UserPoolArn"}
132+
expected1 = {"Ref": "UserPoolArn"}
133+
output1 = resolver.resolve_parameter_refs(input1)
134+
self.assertEqual(output1, expected1)
135+
136+
# Normal parameter should still be resolved
137+
input2 = {"Ref": "NormalParam"}
138+
expected2 = "normal-value"
139+
output2 = resolver.resolve_parameter_refs(input2)
140+
self.assertEqual(output2, expected2)
141+
142+
def test_cloudformation_placeholders_in_nested_structure(self):
143+
"""Test CloudFormation placeholders in nested structures"""
144+
parameter_values = {
145+
"Placeholder1": "{{IntrinsicFunction:stack/Output1/Fn::GetAtt}}",
146+
"Placeholder2": "{{IntrinsicFunction:stack/Output2/Fn::GetAtt}}",
147+
"NormalParam": "value",
148+
}
149+
resolver = IntrinsicsResolver(parameter_values)
150+
151+
input = {
152+
"Resources": {
153+
"Resource1": {
154+
"Properties": {
155+
"Prop1": {"Ref": "Placeholder1"},
156+
"Prop2": {"Ref": "NormalParam"},
157+
"Prop3": {"Ref": "Placeholder2"},
158+
}
159+
}
160+
}
161+
}
162+
163+
expected = {
164+
"Resources": {
165+
"Resource1": {
166+
"Properties": {
167+
"Prop1": {"Ref": "Placeholder1"}, # Not resolved
168+
"Prop2": "value", # Resolved
169+
"Prop3": {"Ref": "Placeholder2"}, # Not resolved
170+
}
171+
}
172+
}
173+
}
174+
175+
output = resolver.resolve_parameter_refs(input)
176+
self.assertEqual(output, expected)
177+
178+
def test_cloudformation_placeholders_in_lists(self):
179+
"""Test CloudFormation placeholders in list structures"""
180+
parameter_values = {
181+
"VpceId": "{{IntrinsicFunction:stack/VpcEndpoint.Outputs.Id/Fn::GetAtt}}",
182+
"Region": "us-east-1",
183+
}
184+
resolver = IntrinsicsResolver(parameter_values)
185+
186+
input = [{"Ref": "VpceId"}, {"Ref": "Region"}, "static-value", {"Ref": "VpceId"}]
187+
188+
expected = [
189+
{"Ref": "VpceId"}, # Not resolved
190+
"us-east-1", # Resolved
191+
"static-value",
192+
{"Ref": "VpceId"}, # Not resolved
193+
]
194+
195+
output = resolver.resolve_parameter_refs(input)
196+
self.assertEqual(output, expected)
197+
198+
def test_cloudformation_placeholders_with_sub(self):
199+
"""Test that CloudFormation placeholders inside Fn::Sub are not substituted
200+
201+
Similar to Ref, Fn::Sub should not substitute CloudFormation internal placeholders.
202+
This prevents the placeholders from being embedded in strings where they can't be
203+
properly handled by CloudFormation.
204+
"""
205+
parameter_values = {
206+
"Placeholder": "{{IntrinsicFunction:stack/Output/Fn::GetAtt}}",
207+
"NormalParam": "normal-value",
208+
}
209+
resolver = IntrinsicsResolver(parameter_values)
210+
211+
# Sub should not substitute CloudFormation placeholders, but should substitute normal params
212+
input = {"Fn::Sub": "Value is ${Placeholder} and ${NormalParam}"}
213+
expected = {"Fn::Sub": "Value is ${Placeholder} and normal-value"}
214+
215+
output = resolver.resolve_parameter_refs(input)
216+
self.assertEqual(output, expected)
217+
218+
def test_various_cloudformation_placeholder_formats(self):
219+
"""Test various CloudFormation placeholder formats"""
220+
parameter_values = {
221+
"Valid1": "{{IntrinsicFunction:stack/Resource.Outputs.Value/Fn::GetAtt}}",
222+
"Valid2": "{{IntrinsicFunction:name-with-dashes/Out/Fn::GetAtt}}",
223+
"Valid3": "{{IntrinsicFunction:stack123/Resource.Out/Fn::GetAtt}}",
224+
"NotPlaceholder1": "{{SomethingElse}}",
225+
"NotPlaceholder2": "{{intrinsicfunction:lowercase}}",
226+
"NotPlaceholder3": "normal-string",
227+
}
228+
resolver = IntrinsicsResolver(parameter_values)
229+
230+
# Valid placeholders should not be resolved
231+
for param in ["Valid1", "Valid2", "Valid3"]:
232+
input = {"Ref": param}
233+
expected = {"Ref": param}
234+
output = resolver.resolve_parameter_refs(input)
235+
self.assertEqual(output, expected, f"Failed for {param}")
236+
237+
# Non-placeholders should be resolved
238+
test_cases = [
239+
("NotPlaceholder1", "{{SomethingElse}}"),
240+
("NotPlaceholder2", "{{intrinsicfunction:lowercase}}"),
241+
("NotPlaceholder3", "normal-string"),
242+
]
243+
244+
for param, expected_value in test_cases:
245+
input = {"Ref": param}
246+
output = resolver.resolve_parameter_refs(input)
247+
self.assertEqual(output, expected_value, f"Failed for {param}")
248+
122249

123250
class TestResourceReferenceResolution(TestCase):
124251
def setUp(self):

0 commit comments

Comments
 (0)