Skip to content

APIGW: fix AWS_PROXY payload with Custom Domains #12296

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

Jump to bottom
Merged
merged 1 commit into from
Feb 21, 2025
Merged

APIGW: fix AWS_PROXY payload with Custom Domains #12296

merged 1 commit into from
Feb 21, 2025

Conversation

bentsku
Copy link
Contributor

@bentsku bentsku commented Feb 21, 2025 •
edited
Loading

Motivation

As we got a report in #12295, we had an issue if our path handling when working with API Gateway Custom Domains and REST APIs.

This PR is a small fix to manage the Lambda Input payload path value, which is quite frankly weird, even for AWS.

See this comment: #12295 (comment)

  • No mapping, no custom domain
    • path: /your-path -> stage is stripped
    • requestContext.path: /<stage>/your-path
  • Mapping configured with no basePath and no stage:
    • path: /<stage>/your-path -> stage is not stripped
    • requestContext.path: /<stage>/your-path
  • Mapping configured with basePath and no stage
    • path: /<basePath>/<stage>/example/v1 -> nothing is stripped
    • requestContext.path: /<basePath>/<stage>/example/v1
  • Mapping configured with basePath and a stage
    • path: /<basePath>/example/v1 -> note that they strip the stage in that case when the stage is specified
    • requestContext.path: /<basePath>/example/v1 -> the stage is even stripped here? which is not following "the rules"

So this PR implements the behavior that if a basePath is configured and present (meaning you're invoking from a Custom Domain), you need to use the requestContext.path, because our internal context invocation_request["path"] is used for routing, it should only have the resource path values.

I think there might be somewhat of an issue in the lambda payload construction in AWS, and even in the requestContext, I think there might be some bug in their implementation 😅

Changes

  • add a small change in how we handle the path field for the AWS_PROXY payload

@bentsku bentsku added aws:apigateway Amazon API Gateway semver: patch Non-breaking changes which can be included in patch releases labels Feb 21, 2025
@bentsku bentsku added this to the 4.2 milestone Feb 21, 2025
@bentsku bentsku self-assigned this Feb 21, 2025
@bentsku bentsku requested a review from cloutierMat as a code owner February 21, 2025 02:06
@bentsku bentsku changed the title fix AWS_PROXY payload with Custom Domains APIGW: fix AWS_PROXY payload with Custom Domains Feb 21, 2025
@github-actions GitHub Actions
Copy link

LocalStack Community integration with Pro

    2 files  ±    0      2 suites  ±0   28m 55s ⏱️ - 1h 22m 58s
1 069 tests  - 3 030  1 004 ✅  - 2 763  65 💤  - 267  0 ❌ ±0 
1 071 runs   - 3 030  1 004 ✅  - 2 763  67 💤  - 267  0 ❌ ±0 

Results for commit 0901caf. ± Comparison against base commit f269fe2.

This pull request removes 3030 tests. 
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_lambda_dynamodb
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_opensearch_crud
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_search_books
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_setup
tests.aws.scenario.kinesis_firehose.test_kinesis_firehose.TestKinesisFirehoseScenario ‑ test_kinesis_firehose_s3
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_destination_sns
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_infra
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_prefill_dynamodb_table
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input0-SUCCEEDED]
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input1-SUCCEEDED]
…

cloutierMat
cloutierMat approved these changes Feb 21, 2025
View reviewed changes
Copy link
Contributor

@cloutierMat cloutierMat left a comment

Choose a reason for hiding this comment

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

🚀

Nice investigation. I guess it would be too simple if the values were consistent! 🤣

Thanks for the fix

@bentsku bentsku merged commit 5d463b5 into master Feb 21, 2025
52 checks passed
@bentsku bentsku deleted the fix-apigw-v1-custom-domain-stage branch February 21, 2025 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cloutierMat cloutierMat cloutierMat approved these changes

Assignees

@bentsku bentsku

Labels
aws:apigateway Amazon API Gateway semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Milestone
4.2
Development

Successfully merging this pull request may close these issues.
2 participants
@bentsku @cloutierMat