Skip to content

fix: avoid TypeError on CommaDelimitedList parameter / logical ID collision#8987

Open
vishwakt wants to merge 3 commits into
aws:developfrom
vishwakt:fix/comma-delimited-list-output-collision
Open

fix: avoid TypeError on CommaDelimitedList parameter / logical ID collision#8987
vishwakt wants to merge 3 commits into
aws:developfrom
vishwakt:fix/comma-delimited-list-output-collision

Conversation

@vishwakt
Copy link
Copy Markdown

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 with TypeError: unhashable type: 'list' when a CloudFormation template contains a CommaDelimitedList parameter whose name collides with a Resource or Output logical ID of the same name.

Minimal repro template (from the issue thread):

AWSTemplateFormatVersion: '2010-09-09'
Transform: AWS::Serverless-2016-10-31

Parameters:
  AppName:
    Type: CommaDelimitedList
    Default: "hello, world"

Resources:
  MyFunction:
    Type: AWS::Serverless::Function
    Properties:
      Runtime: python3.12
      Handler: index.handler
      InlineCode: |
        def handler(event, context):
            return "hi"

Outputs:
  AppName:
    Value: !Ref AppName

How does it address the issue?

In samcli/lib/intrinsic_resolver/intrinsic_property_resolver.py, resolve_attribute() was using get_translation(key) or key to derive the processed key for Resources/Outputs. For a CommaDelimitedList parameter, IntrinsicsSymbolTable.get_translation() deliberately returns a Python list (e.g. ["hello", "world"]). That list was then assigned as a dictionary key on the next line, which is what raised TypeError: 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.DeploymentRestApi via logical_id_translator) — and falls back to the original key when the translation is anything other than a string:

translated_key = self._symbol_resolver.get_translation(key)
processed_key = translated_key if isinstance(translated_key, str) else key

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?

  • No behavior change for templates without a name collision.
  • No behavior change for legitimate logical-ID renaming via logical_id_translator, which always returns a string for that path.
  • Templates that previously crashed now resolve normally: the Output/Resource key is preserved as-is, and !Ref <CommaDelimitedListParam> values continue to expand into a list as expected.

Mandatory Checklist

PRs will only be reviewed after checklist is complete

  • Review the generative AI contribution guidelines
  • Add input/output type hints to new functions/methods (no new functions added)
  • Write design document if needed (Do I need to write a design document?) — not needed for a bug fix
  • Write/update unit tests
  • Write/update integration tests — bug is fully reproducible at the unit level
  • Write/update functional tests if needed
  • make pr passes
  • make update-reproducible-reqs if dependencies were changed — no dependency changes
  • Write documentation — no user-facing API change

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

…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
@vishwakt vishwakt requested a review from a team as a code owner May 13, 2026 07:12
@github-actions github-actions Bot added pr/external stage/needs-triage Automatically applied to new issues and PRs, indicating they haven't been looked at. labels May 13, 2026
@vishwakt
Copy link
Copy Markdown
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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr/external stage/needs-triage Automatically applied to new issues and PRs, indicating they haven't been looked at.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: sam deploy - collisions between parameter name and logical IDs for CommaDelimitedLists

2 participants