-
Notifications
You must be signed in to change notification settings - Fork 454
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
Typed backend config #224
Typed backend config #224
Conversation
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.
That's fantastic, thanks for your contribution.
Mid / Long term
I think the backends should follow the same concept as the TerraformProvider
and would be ideally generated from a schema.
Since there are Go schemas for the backends (e.g. S3, I think that the backend classes should be generated from there somehow. This could be similar to what we do for provider Constructs and be a build step in cdktf
.
The benefits I see for doing this:
- No manual work to keep it aligned with Terraform
- No need to test individual backends, as long as the code generator is tested properly
- Inline docs would be generated as well
- Long term maintenance effort significantly less compared to doing this manually
Perhaps the Terraform team could support this by exposing the Go schemas as JSON schema somehow? /cc @anubhavmishra
Short term
I think it'd be good to rework this PR to match the anticipated generated structre, which means:
- Have a base
TerraformBackend
class - Treat backends as a Construct which get synthesised simlar to
Resources
,Data
andProviders
. - Drop support for generic
TerraformSettings
This would mean, we'd be good to go with the manual implementation and could replace this later on by something which is generated from a JSON schema with little to none breaking changes
What are your thoughts @jsteinich / @anubhavmishra
I'm not a big fan of parsing the Go schemas directly, but if the Terraform team is willing to support getting them as JSON in some fashion, then that makes a lot of sense. Making backends |
👍
Yes, this could be validated after synth. For now, I think it'd be sufficient to let Terraform do the validation. |
I think long term it makes sense to automatically generate the backends but for now, there is no way to do it.
We can have a conversation with the Terraform team to see if that is possible.
Backends don't tend to change as much as we should be good with the manual implementation for now. |
I edited the issue that was related to this PR. #33 |
@jsteinich would you be up for refactoring this accordingly? |
I should be able to get the changes done in the next couple of days. |
Awesome, thanks! Looking forward to it. |
I believe this is ready to go, but there are a few points that might require more discussion.
|
Cool, looks good at a first glance. Will look into it more thoroughly later today. |
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.
That looks very good, thank you! Two minor remarks as inline comments.
Other than that, could you adapt the docs about remote backends as well?
Yeah, I think it would be good for consistency reasons
|
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.
Awesome, that looks great!
I'm going to lock this pull request because it has been closed for 30 days. This helps our maintainers find and focus on the active issues. If you've found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
This adds the ability to specify Terraform Settings when creating a
TerraformStack
, including strongly typed backends.I still need to add some test cases, but I believe it is otherwise complete. I think remote state belongs in a separate request.