Skip to content

APIGW: fix Host regex to allow hyphen and remove restriction #12549

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
Apr 23, 2025
Merged

APIGW: fix Host regex to allow hyphen and remove restriction #12549

merged 1 commit into from
Apr 23, 2025

Conversation

bentsku
Copy link
Contributor

@bentsku bentsku commented Apr 23, 2025

Motivation

Follow-up from #12539, sorry for back and forth on this.

A failure on our upstream pipeline put into light the fact that the - character would still work when using "path based routing" for the API.
In order to not break existing users' setup, I'm removing the previous restriction, and instead updating the regex to allow for dashes in the REST API id, as this is actually a simple fix (making the quantifier be lazy to not match the next regex group).

Changes

  • add a only_localstack test showing that the routing works with hyphen
  • remove previous restriction

@bentsku bentsku added aws:apigateway Amazon API Gateway semver: patch Non-breaking changes which can be included in patch releases review: merge when ready Signals to the reviewer that a PR can be merged if accepted labels Apr 23, 2025
@bentsku bentsku added this to the 4.4 milestone Apr 23, 2025
@bentsku bentsku self-assigned this Apr 23, 2025
@github-actions GitHub Actions
Copy link

LocalStack Community integration with Pro

    2 files  ±    0      2 suites  ±0   30m 30s ⏱️ - 1h 23m 36s
1 106 tests  - 3 269  1 041 ✅  - 2 977  65 💤  - 292  0 ❌ ±0 
1 108 runs   - 3 269  1 041 ✅  - 2 977  67 💤  - 292  0 ❌ ±0 

Results for commit 72abe93. ± Comparison against base commit 77f30dc.

This pull request removes 3270 and adds 1 tests. Note that renamed tests count towards both. 
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]
…

tests.aws.services.apigateway.test_apigateway_common.TestApigatewayRouting ‑ test_routing_with_custom_api_id

@bentsku bentsku marked this pull request as ready for review April 23, 2025 10:52
@bentsku bentsku requested a review from cloutierMat as a code owner April 23, 2025 10:52
cloutierMat
cloutierMat approved these changes Apr 23, 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.

Gotta love those simple fix when they finally appear! Thanks for the follow-up and the added test

@cloutierMat cloutierMat merged commit 560b4cb into master Apr 23, 2025
38 checks passed
@cloutierMat cloutierMat deleted the fix-apigw-host-regex branch April 23, 2025 15:53
@cloutierMat
Copy link
Contributor

image
🥳

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 review: merge when ready Signals to the reviewer that a PR can be merged if accepted semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Milestone
4.4
Development

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