Skip to content

APIGW NG fix routing with ANY request and remove stage from path #11129

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.

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

Merged
merged 2 commits into from
Jul 4, 2024

Conversation

bentsku
Copy link
Contributor

@bentsku bentsku commented Jul 3, 2024

Motivation

As @cloutierMat reported, when actually using the next gen APIGW provider, we would not properly route requests, because we had the full path containing the stage which didn't map properly to any created Werkzeug Rule. This is how APIGW represents their path, this is only what is after the stage.

Only the $context.path contains the stage and path, so we can get weird snapshot with one field named path containing /your-path and one named requestContext.path containing /stage-name/your-path. 🤷‍♂️

We also had the issue with the ANY method, we would try to pass that when creating a Werkzeug Rule.
We now create a special case where ANY gets mapped to DELETE, GET, HEAD, OPTIONS, PATCH, POST, and PUT (the supported HTTP methods of APIGW).

I was worried about having a rule order issue in Werkzeug, but this proved to not be a problem because we only return the Resource from the match, and we can manually get the right resourceMethod afterwards.

Changes

  • create a new Werkzeug Rule to take into account the ANY method
  • remove the stage from the path in the parsing step
  • add new tests for the ANY method with different order of creation
  • update tests to have the stage in the Request path

@bentsku bentsku added aws:apigateway Amazon API Gateway semver: patch Non-breaking changes which can be included in patch releases labels Jul 3, 2024
@bentsku bentsku self-assigned this Jul 3, 2024
@bentsku bentsku requested a review from cloutierMat as a code owner July 3, 2024 00:21
Copy link

github-actions bot commented Jul 3, 2024

LocalStack Community integration with Pro

  2 files  ±    0    2 suites  ±0   23m 25s ⏱️ - 1h 10m 46s
643 tests  - 2 518  598 ✅  - 2 167  45 💤  - 351  0 ❌ ±0 
645 runs   - 2 518  598 ✅  - 2 167  47 💤  - 351  0 ❌ ±0 

Results for commit 8854284. ± Comparison against base commit faf13f7.

This pull request removes 2518 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]
…

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 fix! Thanks for addressing this. Seems like Werkzeug is still making our life easier! 🥳

Comment on lines +50 to +52
# Make sure Werkzeug's Rule does not add any other methods
# (f.e. the HEAD method even though the rule should only match GET)
self.methods = {method.upper()}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice Catch!

@bentsku bentsku merged commit e06725d into master Jul 4, 2024
39 checks passed
@bentsku bentsku deleted the apigw-ng-fix-routing branch July 4, 2024 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aws:apigateway Amazon API Gateway semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants