Skip to content

APIGW: add support for new endpoint configuration #12826

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
Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

APIGW: add support for new endpoint configuration #12826

wants to merge 9 commits into from

Conversation

ArthurAkh
Copy link
Contributor

@ArthurAkh ArthurAkh commented Jul 2, 2025 •
edited
Loading

Motivation

AWS recently introduced the ipAddressType field for API Gateway Rest APIs, defaulting to "ipv4".
LocalStack needs to mimic this behavior to maintain parity.

Related docs:

Changes

  • Added support for returning endpointConfiguration.ipAddressType in CreateRestApi.
  • Added handling of the replace operation for types and ipAddressTypes in UpdateRestApi.
  • Added validation logic.
  • Updated relevant snapshot tests.

TODO

What's left to do:

  • Currently a skip "$..endpointConfiguration.ipAddressType" was added in the test_create_execute_api_vpc_endpoint, because regenerating its snapshot wasn't working.

@ArthurAkh ArthurAkh self-assigned this Jul 2, 2025
@ArthurAkh ArthurAkh added the semver: patch Non-breaking changes which can be included in patch releases label Jul 2, 2025
@ArthurAkh ArthurAkh added this to the 4.7 milestone Jul 2, 2025
@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 2, 2025 •
edited
Loading

Test Results - Preflight, Unit

21 854 tests  +1   20 197 ✅ +1   6m 31s ⏱️ -22s
     1 suites ±0    1 657 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit 446e081. ± Comparison against base commit dbad558.

♻️ This comment has been updated with latest results.

@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 2, 2025 •
edited
Loading

LocalStack Community integration with Pro

    2 files  ±    0      2 suites  ±0   19m 46s ⏱️ - 1h 22m 34s
1 155 tests  - 3 761  1 088 ✅  - 3 052  67 💤  - 709  0 ❌ ±0 
1 157 runs   - 3 761  1 088 ✅  - 3 052  69 💤  - 709  0 ❌ ±0 

Results for commit 446e081. ± Comparison against base commit dbad558.

This pull request removes 3766 and adds 5 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_api.TestApiGatewayApiRestApi ‑ test_create_rest_api_private_type
tests.aws.services.apigateway.test_apigateway_api.TestApiGatewayApiRestApi ‑ test_create_rest_api_verify_defaults
tests.aws.services.apigateway.test_apigateway_api.TestApiGatewayApiRestApi ‑ test_create_rest_api_with_endpoint_configuration
tests.aws.services.apigateway.test_apigateway_api.TestApiGatewayApiRestApi ‑ test_update_rest_api_concatenation_of_errors
tests.aws.services.apigateway.test_apigateway_api.TestApiGatewayApiRestApi ‑ test_update_rest_api_ip_address_type
This pull request removes 710 skipped tests and adds 1 skipped test. Note that renamed tests count towards both. 
tests.aws.scenario.kinesis_firehose.test_kinesis_firehose.TestKinesisFirehoseScenario ‑ test_kinesis_firehose_s3
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input4-FAILED]
tests.aws.scenario.mythical_mysfits.test_mythical_misfits.TestMythicalMisfitsScenario ‑ test_deployed_infra_state
tests.aws.scenario.mythical_mysfits.test_mythical_misfits.TestMythicalMisfitsScenario ‑ test_populate_data
tests.aws.scenario.mythical_mysfits.test_mythical_misfits.TestMythicalMisfitsScenario ‑ test_user_clicks_are_stored
tests.aws.services.cloudcontrol.test_cloudcontrol_api.TestCloudControlResourceApi ‑ test_api_exceptions
tests.aws.services.cloudcontrol.test_cloudcontrol_api.TestCloudControlResourceApi ‑ test_create_exceptions
tests.aws.services.cloudcontrol.test_cloudcontrol_api.TestCloudControlResourceApi ‑ test_create_invalid_desiredstate
tests.aws.services.cloudcontrol.test_cloudcontrol_api.TestCloudControlResourceApi ‑ test_double_create_with_client_token
tests.aws.services.cloudcontrol.test_cloudcontrol_api.TestCloudControlResourceApi ‑ test_lifecycle
…

tests.aws.services.apigateway.test_apigateway_api.TestApiGatewayApiRestApi ‑ test_update_rest_api_concatenation_of_errors

♻️ This comment has been updated with latest results.

@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 3, 2025 •
edited
Loading

Test Results (amd64) - Acceptance

7 tests  ±0   5 ✅ ±0   3m 8s ⏱️ -1s
1 suites ±0   2 💤 ±0 
1 files   ±0   0 ❌ ±0 

Results for commit 446e081. ± Comparison against base commit dbad558.

♻️ This comment has been updated with latest results.

@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 3, 2025 •
edited
Loading

Test Results (amd64) - Integration, Bootstrap

    5 files      5 suites   35m 44s ⏱️
1 179 tests 1 112 ✅ 67 💤 0 ❌
1 185 runs  1 112 ✅ 73 💤 0 ❌

Results for commit 446e081.

♻️ This comment has been updated with latest results.

@ArthurAkh ArthurAkh removed this from the 4.7 milestone Jul 7, 2025
@ArthurAkh ArthurAkh force-pushed the apigw-support-new-endpoint-configuration-field branch from 95ec3ea to e5b7fd4 Compare July 7, 2025 15:22
@ArthurAkh ArthurAkh changed the title WOP: APIGW add ipAddressType field in endpointConfiguration APIGW add ipAddressType field in endpointConfiguration Jul 10, 2025
@ArthurAkh ArthurAkh changed the title APIGW add ipAddressType field in endpointConfiguration APIGW: add ipAddressType field in endpointConfiguration Jul 10, 2025
@ArthurAkh ArthurAkh changed the title APIGW: add ipAddressType field in endpointConfiguration APIGW: add support for new endpoint configuration Jul 10, 2025
@ArthurAkh ArthurAkh marked this pull request as ready for review July 10, 2025 13:59
bentsku
bentsku reviewed Jul 11, 2025
View reviewed changes
Copy link
Contributor

@bentsku bentsku left a comment

Choose a reason for hiding this comment

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

Wow, this is a great PR! 🚀 you really went the extra mile for the validation, this is looking really good. The test cases are very, very thorough and bring us a lot of confidence in previously untested areas. Congrats for the work, I'm really happy about it!

I just have minor comments regarding simplifying the code a little bit, and making use of the generated APIs for hardcoded string values. But once this is addressed, this is good to go! Once again, great work 🚀

localstack-core/localstack/services/apigateway/legacy/provider.py Outdated
Comment on lines 393 to 397
if patch_op["op"] not in ("add", "remove", "move", "test", "replace", "copy"):
raise CommonServiceException(
code="ValidationException",
message=f"1 validation error detected: Value '{patch_op['op']}' at 'updateRestApiInput.patchOperations.1.member.op' failed to satisfy constraint: Member must satisfy enum value set: [add, remove, move, test, replace, copy]",
)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I believe this validation is for all types, and could be move outside of the if patch_op_path check, and be done very early, almost after the for patch_op in patch_operations: line. What do you think?

Copy link
Contributor Author

@ArthurAkh ArthurAkh Jul 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Ah yes, of course! Perfect, I moved this check just after the for patch_op in patch_operations: line. I even considered putting it into a separate loop, to save doing unnecessary patch operations, if it happens that one of the operations is invalid and we end up raising the exception anyways. What do you think?

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

@bentsku bentsku bentsku left review comments

@cloutierMat cloutierMat Awaiting requested review from cloutierMat cloutierMat is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

@ArthurAkh ArthurAkh

Labels
semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Milestone
No milestone
Development

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