-
Notifications
You must be signed in to change notification settings - Fork 1k
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
use aws_api_gateway_stage instead of stage_name from aws_api_gateway_deployment for Terraform #1839
Conversation
…deployment for Terraform
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm, thanks! Still needs to wait on a current amazon employee/maintainer for actual approval/merge...
One question would be understanding how these changes affect an extant deployment with the previous terraform?
Looks good to me, just want to double check backwards compat for existing chalice apps using terraform deployments. |
A couple questions, when I try this on a brand new app, I'm getting an error about the deployment ID not existing:
For reference here's the: app.py file
generated chalice.tf.json template
I also noticed that when I try to deploy an existing Chalice app that was using the previous tf template generation, I see this in the output:
The deployment fails with:
but assuming it was able to deploy successfully, wouldn't this mean that the rest API would be replaced and therefore have a new Rest API url? |
Interesting, I'll have to do some more digging into that As for existing apps it looks like that would be a pretty big breaking change for anyone using Chalice that has already deployed an API, since Terraform is unaware of the API stages that were deployed before this change. Seems like there would be two options at that point:
|
Here's the full logs when trying to do the initial deployment: deployment output
We'd need to come up with something so that users with existing chalice apps deployed with terraform can upgrade Chalice without the new template replacing any resources, and without any additional actions on their part (so any existing CI/CD setups continue to work). Worst case scenario, we may need to toggle this updated tf template generation behind a config file option until we get to a 2.0 release, but ideally we can come up with a way to do this in a backwards compatible way. |
Closing this PR for now as there is more work needed with regards to testing and the ability to have some sort of feature flag to avoid disrupting existing Chalice projects |
Resolves #1838
Also related to #1816 as a workaround for associating WAF ACL with Chalice APIGW Stages
Bump AWS provider version used for Terraform with fixes for API Gateway and follow latest recommendation using
aws_api_gateway_stage
instead ofstage_name
fromaws_api_gateway_deployment
ie. hashicorp/terraform-provider-aws#11344 (comment)
This is covered in the latest docs for the Terraform AWS provider:
https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/api_gateway_deployment
In addition to switching to
aws_api_gateway_stage
for Terraform deployment I've also added a Terraform outputRestAPIStageArn
which can be used to further enhance Chalice deployments of API Gateway (ie. associating a WAF ACL)By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.