Skip to content

fix(apigateway): prevent deletion of stages during deployment replacement #13736

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
cloutierMat merged 4 commits into from
Feb 12, 2026
Merged

fix(apigateway): prevent deletion of stages during deployment replacement #13736

cloutierMat merged 4 commits into from
Feb 12, 2026
+337 −0

Conversation

@shubhiscoding
Copy link
Contributor

@shubhiscoding shubhiscoding commented Feb 10, 2026

Motivation

When the Serverless Framework redeploys, it generates a new deployment logical ID each time (e.g., ApiGatewayDeployment1708076354025 → ApiGatewayDeployment1708076563720). CloudFormation treats this as Add(new) + Remove(old).

The old deployment's CloudFormation delete handler calls get_stages(deploymentId=old_id) to find associated stages. However, the underlying implementation does not filter by deploymentId — it returns all stages regardless. The delete handler then blindly deletes all returned stages, including the stage that was already reassigned to the new deployment.

This causes all subsequent API requests (including CORS OPTIONS) to return 404 after a redeploy.

Changes

  • Added a deploymentId ownership check in the ApiGatewayDeploymentProvider.delete() handler: before deleting a stage, verify that stage.deploymentId matches the deployment being deleted. Stages that have been reassigned to a newer deployment are skipped.

Tests

  • Added test_serverless_like_deployment_stage_survives_update regression test that:
    1. Creates a CF stack with a REST API, MOCK integration methods (GET + OPTIONS/CORS), and a deployment (Deploy1000000001) with stage local
    2. Updates the stack with the same template but a different deployment logical ID (Deploy2000000002), simulating the Serverless Framework redeploy pattern
    3. Asserts the stage survives the deployment replacement and only one deployment remains
  • Uses MOCK integrations (no Lambda dependency) to isolate the CF deployment replacement behavior
  • Test validated against AWS (see test_apigateway.validation.json)

Related

Fixes #13667

@cloutierMat cloutierMat self-assigned this Feb 11, 2026
@cloutierMat cloutierMat added aws:apigateway Amazon API Gateway semver: patch Non-breaking changes which can be included in patch releases docs: skip Pull request does not require documentation changes notes: needed Pull request should be mentioned in the release notes labels Feb 11, 2026
cloutierMat
cloutierMat requested changes Feb 11, 2026
View reviewed changes
Copy link
Member

@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.

Thanks @shubhiscoding for the contribution and taking the time to add an aws validated test.

I am requesting a few changes on the issue before I can merge as I would rather see a fix to getStages filtering over a patch in the CloudFormation resource providers.

Also in addition to the CFN test you added I would like to see a small test for the filter in tests.aws.services.apigateway.test_apigateway_api. There are currently no tests for the deployment and I don't expect the full CRUD test suite either, but at least a small regression test for the filtering behavior.

Let me know if you need extra guidance or have any questions.

@shubhiscoding
Copy link
Contributor Author

shubhiscoding commented Feb 11, 2026

Also in addition to the CFN test you added I would like to see a small test for the filter in tests.aws.services.apigateway.test_apigateway_api. There are currently no tests for the deployment and I don't expect the full CRUD test suite either, but at least a small regression test for the filtering behavior.

Forgot to address this..., looking into it...!

@shubhiscoding
Copy link
Contributor Author

shubhiscoding commented Feb 11, 2026

Hi @cloutierMat , thanks for the review! I've pushed all the requested changes:

  • Moved the deploymentId filtering to get_stages in the provider instead of the CF delete handler
  • Switched to deploy_cfn_template with template_mapping and an external template file
  • Removed the unnecessary skip_snapshot_verify
  • Added a direct API-level regression test (test_get_stages_filters_by_deployment_id) for the filtering behavior in test_apigateway_api.py

Both tests are validated against AWS!
Please have a look and let me know if any other change required from my side!

cloutierMat
cloutierMat requested changes Feb 12, 2026
View reviewed changes
Copy link
Member

@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.

Awesome, thank you for the quick response. I only see a couple of minor changes needed to the snapshot transformers. I will be happy to merged once solved.

Btw, no need to worry about the lambda test failure, this is an unrelated flake.

tests/aws/services/apigateway/test_apigateway_api.py

class TestApiGatewayStage:
@markers.aws.validated
def test_get_stages_filters_by_deployment_id(self, aws_client, apigw_create_rest_api, snapshot):
Copy link
Member

@cloutierMat cloutierMat Feb 12, 2026

Choose a reason for hiding this comment

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

Nice test addition, thank you for adding our first stage CRUD test 🥳

cloutierMat
cloutierMat approved these changes Feb 12, 2026
View reviewed changes
Copy link
Member

@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.

@shubhiscoding In order to save us a day of back and forth (looks like we are in different TZ), I took the liberty to apply the requested changes since they were only minor snapshot transformers adjustments.

Thank you again for you contributions and for quickly resolving the previous comments I made. I will approve and merge this pr and your fix will be available in our next image build.

@cloutierMat cloutierMat merged commit 4f9792f into localstack:main Feb 12, 2026
30 checks passed
@shubhiscoding
Copy link
Contributor Author

shubhiscoding commented Feb 12, 2026

@shubhiscoding In order to save us a day of back and forth (looks like we are in different TZ), I took the liberty to apply the requested changes since they were only minor snapshot transformers adjustments.

Thank you again for you contributions and for quickly resolving the previous comments I made. I will approve and merge this pr and your fix will be available in our next image build.

Thanks @cloutierMat! TZ wasn’t the issue — I just had some office work to handle and was planning to look into it a bit later. thanks for taking care of the changes and for the review too🫡!

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

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

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

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

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

Assignees

@cloutierMat cloutierMat

Labels

aws:apigateway Amazon API Gateway docs: skip Pull request does not require documentation changes notes: needed Pull request should be mentioned in the release notes 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.

bug: After redeploy a cloudformation with lambda with CORS, localstack not found CORS

2 participants

@shubhiscoding @cloutierMat