Skip to content

fix APIGW CFN Stage with Stage Variables casing #11344

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 5 commits into from
Aug 12, 2024
Merged

Conversation

bentsku
Copy link
Contributor

@bentsku bentsku commented Aug 12, 2024

Motivation

As reported by a user, we had an issue with stage variables when declared via CloudFormation.

Setting a stage variable to be TestCasing would save it as testCasing. As stage variables are case sensitive, this is quite an issue.

I've tracked down the issue to be because we would call keys_to_lower on the parameters, which would change the casing.

In the meantime, I've added more tests around stage variables and their casing without CFN, and more validation about what the user was encoutering (the stage variable wouldn't be found, which would create an HTTP integration URI to not have a host).

I've also found a small issue with how we would import OpenAPI/Swagger files for the method used for the integration for HTTP/HTTP_PROXY.

Resources:

Changes

  • fix the CloudFormation Stage resource for APIGW, picking up the stage variables before the keys_to_lower call and using deep copy because of the nested dicts
  • update a validated test to check the fix and casing
  • update another test related to Stage Variables but not CFN, just to check behavior (also add a negative test for NextGen only)
  • add better exception handler for HTTP & HTTP_PROXY NextGen integrations, can still be improved with negative testing, we just need to find how to trigger them

@bentsku bentsku added aws:apigateway Amazon API Gateway semver: patch Non-breaking changes which can be included in patch releases labels Aug 12, 2024
@bentsku bentsku self-assigned this Aug 12, 2024
Copy link

github-actions bot commented Aug 12, 2024

LocalStack Community integration with Pro

    2 files  ±0      2 suites  ±0   1h 35m 50s ⏱️ +32s
3 312 tests ±0  2 917 ✅ ±0  395 💤 ±0  0 ❌ ±0 
3 314 runs  ±0  2 917 ✅ ±0  397 💤 ±0  0 ❌ ±0 

Results for commit 1dd7d36. ± Comparison against base commit 15fec75.

♻️ This comment has been updated with latest results.

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 investigation with a clean fix!

Great job, I just left one nit, but not blocking! I am happy either way! 🚀

Comment on lines -1260 to -1265
case "AWS":
case _:
integration_method = (
method_integration.get("httpMethod") or method_name
).upper()
case _:
integration_method = method_name
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 cacac3a into master Aug 12, 2024
34 checks passed
@bentsku bentsku deleted the fix-apigw-stage-vars branch August 12, 2024 21:06
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