Skip to content

fix(intrinsics): only resolve the selected Fn::If branch #9134

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
EPNOS wants to merge 6 commits into
base: develop
Choose a base branch
from
Open

fix(intrinsics): only resolve the selected Fn::If branch #9134

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d4c07ac
fix(intrinsics): resolve only the selected Fn::If branch
EPNOS Jul 19, 2026
16c70d0
fix(intrinsics): forward ignore_errors to Fn::GetAtt symbol resolution
EPNOS Jul 20, 2026
e16ec5b
test(intrinsics): add integration coverage for Fn::If with unresolvab…
EPNOS Jul 20, 2026
0a7b27d
Merge branch 'develop' into fix/4510-lazy-fn-if-resolution
EPNOS Aug 2, 2026
88ffaea
fix(intrinsics): stop forwarding ignore_errors into Fn::GetAtt symbol…
EPNOS Aug 3, 2026
57b4fbd
Merge branch 'fix/4510-lazy-fn-if-resolution' of https://github.com/E…
EPNOS Aug 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 5 additions & 14 deletions samcli/lib/intrinsic_resolver/intrinsic_property_resolver.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -716,24 +716,14 @@ def handle_fn_if(self, intrinsic_value, ignore_errors):
-------
This will return value_if_true and value_if_false depending on how the condition is evaluated
"""
arguments = self.intrinsic_property_resolver(
intrinsic_value, ignore_errors, parent_function=IntrinsicResolver.FN_IF
)
verify_intrinsic_type_list(arguments, IntrinsicResolver.FN_IF)
verify_number_arguments(arguments, IntrinsicResolver.FN_IF, num=3)
verify_intrinsic_type_list(intrinsic_value, IntrinsicResolver.FN_IF)
verify_number_arguments(intrinsic_value, IntrinsicResolver.FN_IF, num=3)

condition_name = self.intrinsic_property_resolver(
arguments[0], ignore_errors, parent_function=IntrinsicResolver.FN_IF
intrinsic_value[0], ignore_errors, parent_function=IntrinsicResolver.FN_IF
)
verify_intrinsic_type_str(condition_name, IntrinsicResolver.FN_IF)

value_if_true = self.intrinsic_property_resolver(
arguments[1], ignore_errors, parent_function=IntrinsicResolver.FN_IF
)
value_if_false = self.intrinsic_property_resolver(
arguments[2], ignore_errors, parent_function=IntrinsicResolver.FN_IF
)

condition = self._conditions.get(condition_name)
verify_intrinsic_type_dict(
condition,
Expand All @@ -750,7 +740,8 @@ def handle_fn_if(self, intrinsic_value, ignore_errors):
message="The result of {} must evaluate to bool".format(IntrinsicResolver.FN_IF),
)

return value_if_true if condition_evaluated else value_if_false
selected_value = intrinsic_value[1] if condition_evaluated else intrinsic_value[2]
return self.intrinsic_property_resolver(selected_value, ignore_errors, parent_function=IntrinsicResolver.FN_IF)

@aws-sam-tooling-bot aws-sam-tooling-bot Bot Aug 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[BUG] handle_fn_if can now return None, which it could never do before, and at least one downstream consumer is not null-safe.

Before this change, a branch containing !Ref AWS::NoValue was pre-resolved to None by the outer intrinsic_property_resolver(intrinsic_value, ...) call, and the subsequent resolve(arguments[1|2]) hit the if intrinsic is None: raise InvalidIntrinsicException guard at the top of intrinsic_property_resolver. So Fn::If always raised, and with ignore_errors=True the enclosing dict loop left the property as the raw {"Fn::If": [...]} dict.

Now the selected branch is resolved directly, so !Ref AWS::NoValue reaches IntrinsicsSymbolTable.handle_pseudo_no_value() and None is returned and assigned as the property value (the generic dict branch does sanitized_dict[sanitized_key] = sanitized_val with no None filtering).

Concrete failure — the common "conditionally omit a property" idiom:

Properties:
 Layers: !If [UseLayers, [!Ref MyLayer], !Ref "AWS::NoValue"]

When UseLayers is false, Properties["Layers"] becomes None. In samcli/lib/providers/sam_function_provider.py:265, resource_properties.get("Layers", []) returns None (the default only applies when the key is absent), and _parse_layer_info then does for layer in list_of_layers → TypeError: 'NoneType' object is not iterable, an unhandled traceback instead of a domain error.

Note the element-level form Layers: [!If [Cond, !Ref MyLayer, !Ref "AWS::NoValue"]] is fine — it yields [None] and _parse_layer_info skips unrecognized entries. Only the whole-property form breaks, and that is exactly one of the scenarios this PR sets out to fix, so it is worth closing here rather than leaving it as a newly reachable crash.

Two options:

  • Make the resolver match CloudFormation's AWS::NoValue semantics by dropping keys whose resolved value is None in the generic dict branch of intrinsic_property_resolver. This is the semantically correct fix but has wider blast radius, so it needs its own tests.
  • Harden the consumer, e.g. resource_properties.get("Layers") or [] in both call sites in sam_function_provider.py.

Either way, please add a unit test covering an Fn::If whose selected branch is !Ref AWS::NoValue — the four new tests only cover unresolvable Fn::GetAtt in the unselected branch, so this path is currently untested.

Note on the previous review comment: the handle_fn_getatt change that forwarded ignore_errors into resolve_symbols is no longer in the diff, and test_template_ignore_errors_leaves_unresolvable_layer_getatt_as_dict was added as a guard for the nested-stack layer behavior. That finding is resolved and I did not re-raise it. The PR description still describes the handle_fn_getatt change, so it is now out of date.


def handle_fn_equals(self, intrinsic_value, ignore_errors):
"""
Expand Down
21 changes: 21 additions & 0 deletions tests/integration/local/invoke/test_integrations_cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,27 @@ def test_invoke_with_env_using_parameters(self):
self.assertEqual(environ["MyRuntimeVersion"], "v0")
self.assertEqual(environ["EmptyDefaultParameter"], "")

@pytest.mark.flaky(reruns=3)
def test_invoke_with_env_using_fn_if_ignores_unresolvable_branch(self):
command_list = InvokeIntegBase.get_command_list(
"EchoEnvWithFnIf",
template_path=self.template_path,
event_path=self.event_path,
)

process = Popen(command_list, stdout=PIPE)
try:
stdout, _ = process.communicate(timeout=TIMEOUT)
except TimeoutExpired:
process.kill()
raise

self.assertEqual(process.returncode, 0)
process_stdout = stdout.strip()
environ = json.loads(process_stdout.decode("utf-8"))

self.assertEqual(environ["FunctionUrl"], "https://custom.example.com/")

@pytest.mark.flaky(reruns=3)
def test_invoke_multi_tenant_function(self):
command_list = InvokeIntegBase.get_command_list(
Expand Down
35 changes: 35 additions & 0 deletions tests/integration/testdata/invoke/template.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,17 @@ Parameters:
Type: String
Default: "2"

UseCustomFunctionUrl:
Type: String
Default: "true"

CustomFunctionUrl:
Type: String
Default: "https://custom.example.com/"

Conditions:
ShouldUseCustomFunctionUrl: !Equals [!Ref UseCustomFunctionUrl, "true"]

Mappings:
common:
LambdaFunction:
Expand Down Expand Up @@ -202,6 +213,30 @@ Resources:
MyRuntimeVersion: !Ref MyRuntimeVersion
EmptyDefaultParameter: !Ref EmptyDefaultParameter

FunctionWithUrlConfig:
Type: AWS::Serverless::Function
Properties:
Handler: main.handler
Runtime: python3.9
CodeUri: .
Timeout: 600
FunctionUrlConfig:
AuthType: NONE

EchoEnvWithFnIf:
Type: AWS::Serverless::Function
Properties:
Handler: main.env_var_echo_hanler
Runtime: python3.9
CodeUri: .
Timeout: 600
Environment:
Variables:
FunctionUrl: !If
- ShouldUseCustomFunctionUrl
- !Ref CustomFunctionUrl
- !GetAtt FunctionWithUrlConfigUrl.FunctionUrl

TimeoutFunctionWithStringParameter:
Type: AWS::Serverless::Function
Properties:
Expand Down
50 changes: 49 additions & 1 deletion tests/unit/lib/intrinsic_resolver/test_intrinsic_resolver.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from copy import deepcopy
from pathlib import Path
from unittest import TestCase
from unittest.mock import patch
from unittest.mock import MagicMock, patch

from parameterized import parameterized

Expand Down Expand Up @@ -906,6 +906,38 @@ def test_fn_if_condition_not_bool_fail(self):
with self.assertRaises(InvalidIntrinsicException, msg="Invalid Condition"):
self.resolver.intrinsic_property_resolver({"Fn::If": ["InvalidCondition", "test", "test"]}, True)

def test_fn_if_selects_resolvable_true_branch_ignoring_unresolvable_false_branch(self):
intrinsic = {"Fn::If": ["TestCondition", "resolved-value", {"Fn::GetAtt": ["Function2Url", "FunctionUrl"]}]}

result = self.resolver.intrinsic_property_resolver(intrinsic, False)
self.assertEqual(result, "resolved-value")

def test_fn_if_selects_resolvable_false_branch_ignoring_unresolvable_true_branch(self):
intrinsic = {"Fn::If": ["NotTestCondition", {"Fn::GetAtt": ["Function2Url", "FunctionUrl"]}, "resolved-value"]}

result = self.resolver.intrinsic_property_resolver(intrinsic, False)
self.assertEqual(result, "resolved-value")

def test_fn_if_does_not_evaluate_unselected_false_branch(self):
mock_handle_fn_getatt = MagicMock()
self.resolver.intrinsic_key_function_map[IntrinsicResolver.FN_GET_ATT] = mock_handle_fn_getatt
intrinsic = {"Fn::If": ["TestCondition", "resolved-value", {"Fn::GetAtt": ["Function2Url", "FunctionUrl"]}]}

result = self.resolver.intrinsic_property_resolver(intrinsic, False)

self.assertEqual(result, "resolved-value")
mock_handle_fn_getatt.assert_not_called()

def test_fn_if_does_not_evaluate_unselected_true_branch(self):
mock_handle_fn_getatt = MagicMock()
self.resolver.intrinsic_key_function_map[IntrinsicResolver.FN_GET_ATT] = mock_handle_fn_getatt
intrinsic = {"Fn::If": ["NotTestCondition", {"Fn::GetAtt": ["Function2Url", "FunctionUrl"]}, "resolved-value"]}

result = self.resolver.intrinsic_property_resolver(intrinsic, False)

self.assertEqual(result, "resolved-value")
mock_handle_fn_getatt.assert_not_called()


class TestIntrinsicAttribteResolution(TestCase):
def setUp(self):
Expand Down Expand Up @@ -1013,6 +1045,22 @@ def test_template_ignore_errors(self):
}
self.assertEqual(expected_template, dict(result))

def test_template_ignore_errors_leaves_unresolvable_layer_getatt_as_dict(self):
resources = deepcopy(self.resources)
resources["ReferenceLambdaLayerVersionLambdaFunction"]["Properties"]["Layers"] = [
{"Fn::GetAtt": ["NestedStack", "Outputs.MyDepLayer"]}
]
template = {"Mappings": self.mappings, "Conditions": self.conditions, "Resources": resources}
symbol_resolver = IntrinsicsSymbolTable(template=template, logical_id_translator=self.logical_id_translator)
resolver = IntrinsicResolver(template=template, symbol_resolver=symbol_resolver)

result = resolver.resolve_attribute(resources, ignore_errors=True)

self.assertEqual(
result["ReferenceLambdaLayerVersionLambdaFunction"]["Properties"]["Layers"],
[{"Fn::GetAtt": ["NestedStack", "Outputs.MyDepLayer"]}],
)


class TestResolveTemplate(TestCase):
def test_parameter_not_resolved(self):
Expand Down