fix: avoid TypeError on CommaDelimitedList parameter / logical ID collision#8987
Open
vishwakt wants to merge 3 commits into
Open
fix: avoid TypeError on CommaDelimitedList parameter / logical ID collision#8987vishwakt wants to merge 3 commits into
vishwakt wants to merge 3 commits into
Conversation
…with Resource/Output logical ID In resolve_attribute(), get_translation() can return a list when a CommaDelimitedList parameter shares its name with a Resource or Output logical ID. Using that list as a dict key raised "TypeError: unhashable type: 'list'" and broke `sam deploy --guided`. Only use the translated key when it resolves to a string; otherwise fall back to the original key. This preserves the existing logical-ID renaming behavior for Resources while preventing the crash on name collisions. Fixes aws#8627
Author
|
@reedham-aws Could you please re-trigger the workflow when you have a moment? Thanks for taking a look. the CI failure was a black --check flagging one line in the new test |
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
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 #8627
Why is this change necessary?
sam deploy --guided(and any other command path that resolves a template) crashes withTypeError: unhashable type: 'list'when a CloudFormation template contains aCommaDelimitedListparameter whose name collides with a Resource or Output logical ID of the same name.Minimal repro template (from the issue thread):
How does it address the issue?
In
samcli/lib/intrinsic_resolver/intrinsic_property_resolver.py,resolve_attribute()was usingget_translation(key) or keyto derive the processed key forResources/Outputs. For aCommaDelimitedListparameter,IntrinsicsSymbolTable.get_translation()deliberately returns a Pythonlist(e.g.["hello", "world"]). That list was then assigned as a dictionary key on the next line, which is what raisedTypeError: unhashable type: 'list'.The fix narrows the use of the translated key to the case it was actually designed for — string renames (e.g.
RestApi.Deployment→RestApivialogical_id_translator) — and falls back to the original key when the translation is anything other than a string:Refs from the existing test suite (
TestIntrinsicAttribteResolution) verify the logical-ID renaming path still works, since those translations resolve to strings.What side effects does this change have?
logical_id_translator, which always returns a string for that path.!Ref <CommaDelimitedListParam>values continue to expand into a list as expected.Mandatory Checklist
PRs will only be reviewed after checklist is complete
make prpassesmake update-reproducible-reqsif dependencies were changed — no dependency changesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.