Skip to content

APIGW: validate REST API custom id tag #12539

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 22, 2025
Merged

APIGW: validate REST API custom id tag #12539

merged 1 commit into from
Apr 22, 2025

Conversation

bentsku
Copy link
Contributor

@bentsku bentsku commented Apr 19, 2025

Motivation

When setting a custom id for a REST API and using the _custom_id_ tag, using an hyphen will make the API fail because the routing won't work, as the regex explicitly doesn't match hyphens: <regex('[^-]+'):api_id>, to avoid matching on the -vpce part.
It would fail silently, because it wouldn't be able to find the API.

We now validate the custom id to not allow - character in it.

Note: maybe another solution would be to adapt the regex to allow hyphens, but didn't want to take the risk to change it, and this has been the behavior since this has been introduced.

Changes

  • add validation and adapt existing test

@bentsku bentsku added aws:apigateway Amazon API Gateway semver: patch Non-breaking changes which can be included in patch releases labels Apr 19, 2025
@bentsku bentsku self-assigned this Apr 19, 2025
@github-actions GitHub Actions
Copy link

LocalStack Community integration with Pro

    2 files  ±    0      2 suites  ±0   31m 36s ⏱️ - 1h 21m 34s
1 106 tests  - 3 267  1 041 ✅  - 2 975  65 💤  - 292  0 ❌ ±0 
1 108 runs   - 3 267  1 041 ✅  - 2 975  67 💤  - 292  0 ❌ ±0 

Results for commit 7dd4321. ± Comparison against base commit 5e8dc09.

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

@bentsku bentsku added this to the 4.4 milestone Apr 19, 2025
@bentsku bentsku marked this pull request as ready for review April 19, 2025 01:09
@bentsku bentsku requested a review from cloutierMat as a code owner April 19, 2025 01:09
cloutierMat
cloutierMat approved these changes Apr 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.

I totally agree with the decision to to not accept apigw ids with - in it. 👍

Thanks for adding the tests also!

@bentsku bentsku merged commit 7e1e899 into master Apr 22, 2025
36 checks passed
@bentsku bentsku deleted the apigw-validate-custom-id branch April 22, 2025 16:28
@bentsku bentsku mentioned this pull request Apr 23, 2025
Merged
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.4
Development

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