Skip to content

fix: handle intrinsic functions in Lambda FunctionName for GetAtt resolution#8820

Open
hoangsetup wants to merge 7 commits intoaws:developfrom
hoangsetup:fix/joinfn_getatt
Open

fix: handle intrinsic functions in Lambda FunctionName for GetAtt resolution#8820
hoangsetup wants to merge 7 commits intoaws:developfrom
hoangsetup:fix/joinfn_getatt

Conversation

@hoangsetup
Copy link
Copy Markdown
Contributor

Which issue(s) does this change fix?

Fixes #6868, #5916, #4597

Fixes Lambda function mounting issue where Fn::Join containing Fn::GetAtt for Lambda Integration URIs in API Gateway Method resources shows "Mounting None" instead of the actual function name.

Affected Use Case:

Uri: !Join
  - ''
  - - 'arn:'
    - !Ref 'AWS::Partition'
    - ':apigateway:'
    - !Ref 'AWS::Region'
    - ':lambda:path/2015-03-31/functions/'
    - !GetAtt HelloWorldFunction.Arn
    - '/invocations'

Previously showed: Mounting None
Now shows: Mounting HelloWorldFunction at http://127.0.0.1:3000/hello [GET]

Why is this change necessary?

When developers use Fn::Join with Fn::GetAtt to construct Lambda Integration URIs (a common CloudFormation pattern), SAM CLI failed to mount the Lambda function correctly, showing:

Routes found:
  Path: /hello, Methods: ['GET'], Function: None
  ERROR: function_name is None!

This prevented developers from:

  1. Testing their API Gateway integrations locally
  2. Using standard CloudFormation intrinsic function patterns
  3. Having feature parity between Fn::Join and Fn::Sub (which worked correctly)

The bug was particularly confusing because Fn::Sub worked fine, leading developers to believe their template was wrong when it was actually a SAM CLI bug.

How does it address the issue?

What side effects does this change have?

Positive effects:

  • Fn::Join with Fn::GetAtt now works correctly for Lambda Integration URIs
  • Function names properly extracted even when FunctionName contains intrinsic functions
  • Feature parity between Fn::Join and Fn::Sub approaches
  • Supports CloudFormation best practices and common patterns
  • Better error handling (graceful fallback to logical ID)

No negative effects:

  • All 319 existing unit tests pass (no regression)
  • Backward compatible (existing templates continue to work)
  • No performance impact
  • No breaking changes to API or behavior

Mandatory Checklist

PRs will only be reviewed after checklist is complete

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

@hoangsetup hoangsetup requested a review from a team as a code owner March 18, 2026 04:01
@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 Mar 18, 2026
@vicheey
Copy link
Copy Markdown
Contributor

vicheey commented Mar 26, 2026

Thank you @hoangsetup for your fix!
So the main fix seems correct. I see you implement the isinstance guard in arn_resolver() to correctly prevents the TypeError: unhashable type crash when FunctionName contains an intrinsic function. Kudos to that!

However, there's dead code in the PR. I see you added the intrinsic_resolver parameter to _get_function_name() but never wires it up from the only caller. The intrinsic resolution path is unreachable. Do you mind cleaning this section?

@vicheey vicheey added need-customer-response Waiting for customer response and removed stage/needs-triage Automatically applied to new issues and PRs, indicating they haven't been looked at. labels Mar 26, 2026
@@ -270,12 +270,19 @@ def arn_resolver(self, logical_id, service_name="lambda"):
partition_name = self.handle_pseudo_partition()
if service_name == "lambda":
resource_name = self._get_function_name(logical_id)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why do we add intrinsic_resolver to __get_function_name()` function if we are not using it here?

@hoangsetup
Copy link
Copy Markdown
Contributor Author

@vicheey You’re absolutely right — intrinsic_resolver was dead code and has been removed. The real fix is the isinstance guard in arn_resolver(), which prevents crashes by falling back to the logical ID when FunctionName includes intrinsic functions.

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

Labels

need-customer-response Waiting for customer response pr/external

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Fn::GetAtt fails to resolve correctly

2 participants