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 8 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 4 commits
Commits
Show all changes
8 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
b63826a
fix(providers): treat None Layers as empty list when parsing function…
EPNOS Aug 5, 2026
ace7482
fix(intrinsics): drop properties that resolve to AWS::NoValue via Fn::If
EPNOS Aug 5, 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
21 changes: 6 additions & 15 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 @@ -615,7 +615,7 @@ def handle_fn_getatt(self, intrinsic_value, ignore_errors):
verify_intrinsic_type_str(logical_id, IntrinsicResolver.FN_GET_ATT)
verify_intrinsic_type_str(resource_type, IntrinsicResolver.FN_GET_ATT)

return self._symbol_resolver.resolve_symbols(logical_id, resource_type)
return self._symbol_resolver.resolve_symbols(logical_id, resource_type, ignore_errors)

@aws-sam-tooling-bot aws-sam-tooling-bot Bot Aug 2, 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] Forwarding ignore_errors into resolve_symbols changes behavior for every unresolvable Fn::GetAtt in the template, not just ones inside a selected Fn::If branch, and it regresses nested-stack layer handling.

SamBaseProvider.get_template / get_resolved_template_dict both call resolver.resolve_template(ignore_errors=True), so this is the only mode sam build and sam local ever run in. Previously an unresolvable GetAtt raised InvalidSymbolException, which the dict-recursion handler in intrinsic_property_resolver (and resolve_attribute) caught and used to leave the property as the raw intrinsic dict. Now it silently becomes the string "$LogicalId.Attribute", and downstream code that distinguishes "unresolved intrinsic dict" from "resolved string" takes the wrong path.

Concretely, for a layer referencing a nested stack output — the exact shape nested_stack_manager.py emits and that SamFunctionProvider._locate_layer_from_nested documents:

Layers:
  - !GetAtt AwsSamAutoDependencyLayerNestedStack.Outputs.MyDepLayer

resolve_symbols("AwsSamAutoDependencyLayerNestedStack", "Outputs.MyDepLayer") has no match in logical_id_translator, parameters, default_type_resolver, or common_attribute_resolver (which only handles Ref and Arn). Before this change the entry stayed a dict and _parse_layer_info fell into its else branch, logging layer "..." is not recognizable ... Skipping. After this change the entry is the string "$AwsSamAutoDependencyLayerNestedStack.Outputs.MyDepLayer", so isinstance(layer, str) is true, locate_layer_nested is False for build and local invoke, and it is passed to LayerVersion(layer, None, ...). _compute_layer_name / _compute_layer_version then fail to rsplit an ARN out of it and raise InvalidLayerVersionArn — a hard failure where the layer used to be skipped.

This change also isn't required for the Fn::If fix. If the selected branch contains an unsupported Fn::GetAtt, the exception already propagates out of handle_fn_if to the enclosing dict recursion, which catches it under ignore_errors=True and preserves the original value. Suggest reverting this line to keep the blast radius limited to handle_fn_if:

return self._symbol_resolver.resolve_symbols(logical_id, resource_type)

If the placeholder degradation is genuinely wanted, it needs to be scoped so it can't turn an unresolved layer reference into something _parse_layer_info mistakes for a literal ARN, plus a test covering Layers: [!GetAtt Stack.Outputs.Layer]. Note that tests/unit/.../test_intrinsic_resolver.py:471 currently locks in the new placeholder behavior, so this would need updating too.

@EPNOS EPNOS Aug 3, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed
88ffaea


def handle_fn_ref(self, intrinsic_value, ignore_errors):
"""
Expand Down 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.

@EPNOS EPNOS Aug 5, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed
b63826a

@aws-sam-tooling-bot aws-sam-tooling-bot Bot Aug 5, 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, and the or [] patches in sam_function_provider.py only cover two of the affected call sites.

When the selected branch is {"Ref": "AWS::NoValue"}, resolve_symbols routes to IntrinsicsSymbolTable.handle_pseudo_no_value(), which returns Python None. Before this change that None hit the if intrinsic is None: raise InvalidIntrinsicException guard, so handle_fn_if always raised and the enclosing dict branch (with ignore_errors=True) left the raw {"Fn::If": [...]} in place. Now None is returned and stored as the property value:

# intrinsic_property_resolver, dict branch
sanitized_val = self.intrinsic_property_resolver(val, ignore_errors, parent_function=parent_function)
...
sanitized_dict[sanitized_key] = sanitized_val   # key kept, value is None

The key is retained rather than dropped, which does not match CloudFormation's AWS::NoValue semantics (the property is removed). A concrete crash path is SamApiProvider, which reads Events off the intrinsic-resolved stack.resources:

Events:
 ApiEvent: !If [UseApi, {Type: Api, Properties: {Path: /, Method: get}}, !Ref "AWS::NoValue"]

With UseApi false this resolves to {"ApiEvent": None}, and sam local start-api then hits samcli/lib/providers/sam_api_provider.py:469:

for , event in serverlessfunction_events.items():
   event_type = event.get(self._EVENT_TYPE)   # AttributeError: 'NoneType' object has no attribute 'get'

Previously that entry stayed as the raw Fn::If dict and was silently skipped. The same shape applies to properties.get("Events", {}) / properties.get(AUTHORIZER_TYPE, "").lower() style lookups elsewhere — a .get(key, default) default never fires when the key is present with a None value, so patching individual consumers with or [] will keep surfacing new failures.

Normalizing at the resolver would fix the whole class at once, and there is already precedent for exactly this in the codebase — CfnLanguageExtensionsApi._remove_no_value in samcli/lib/cfn_language_extensions/api.py:482, whose docstring notes "When Fn::If returns AWS::NoValue, the resolver returns None" and which skips such keys and list items (while deliberately preserving AWS::NoValue inside intrinsic-function arguments).

@EPNOS EPNOS Aug 5, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed
ace7482


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
41 changes: 40 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 @@ -468,6 +468,13 @@ def test_fn_getatt_second_arguments_invalid(self, name, intrinsic):
with self.assertRaises(InvalidIntrinsicException, msg=name):
self.resolver.intrinsic_property_resolver({"Fn::GetAtt": ["some logical Id", intrinsic]}, True)

def test_fn_getatt_ignore_errors_forwarded_for_unsupported_attribute(self):
intrinsic = {"Fn::GetAtt": ["UnknownResource", "UnknownAttribute"]}

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

self.assertEqual(result, "$UnknownResource.UnknownAttribute")


class TestIntrinsicFnSubResolver(TestCase):
def setUp(self):
Expand Down Expand Up @@ -906,6 +913,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