fix(intrinsics): only resolve the selected Fn::If branch#9134
Open
EPNOS wants to merge 3 commits into
Open
Conversation
Fn::If eagerly resolved both the true and false branches before checking the condition, so an unresolvable value in the branch NOT selected (e.g. Fn::GetAtt to FunctionUrl, which sam local invoke can't resolve) caused the whole intrinsic to be left unresolved even though the condition picked the other, resolvable branch. Fixes aws#4510, Fixes aws#6205
handle_fn_getatt called resolve_symbols without forwarding its own ignore_errors argument, so resolve_symbols always defaulted to False. Any Fn::GetAtt referencing an attribute the local resolver doesn't support (e.g. FunctionUrl) raised even when the caller explicitly asked to ignore errors, instead of falling back to a placeholder.
…le branch Reproduces the aws#4510 scenario end-to-end via sam local invoke: an environment variable that uses Fn::If where the selected branch is a resolvable Ref and the other branch is Fn::GetAtt to a FunctionUrl attribute, which the local resolver can't handle.
github-actions
Bot
added
pr/external
stage/needs-triage
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue(s) does this change fix?
Fixes #4510, Fixes #6205
Why is this change necessary?
Fn::Ifeagerly resolved both the true and false branches before checkingthe condition. If the branch that the condition did NOT select contained a
value the local resolver can't handle (e.g.
Fn::GetAttto aFunctionUrlattribute, or
!Ref AWS::NoValuein aLayerslist), resolution of theentire
Fn::Iffailed even though the condition picked the other,perfectly resolvable branch.
sam deploy/sam buildwere unaffected sinceCloudFormation itself only evaluates the selected branch.
How does it address the issue?
handle_fn_ifnow evaluates the condition first and only recursivelyresolves the branch it selects, instead of resolving both branches
unconditionally.
handle_fn_getattnow forwards its ownignore_errorsargument toresolve_symbols, which it previously dropped. This matters once theFn::Iffix is in place: if the selected branch itself contains anunsupported
Fn::GetAtt,ignore_errors=Trueshould still let itdegrade to a placeholder instead of raising.
What side effects does this change have?
None expected. Both branches were already required to independently
type-check before this change; only the eager resolution of the unselected
branch's value is removed.
Mandatory Checklist
PRs will only be reviewed after checklist is complete
make prpassesmake update-reproducible-reqsif dependencies were changedBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.