-
Notifications
You must be signed in to change notification settings - Fork 53
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
terraform: Terraform module for GCP #2553
Conversation
6bf3ff4
to
8407c58
Compare
3e1e07f
to
2297685
Compare
Let's put this on hold until #2503 is merged. |
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 from a high level. Will give this a more thorough review once the AWS module is merged.
d2c0d68
to
e65e0e5
Compare
Regarding release pipeline changes, @malt3 should review that. |
e65e0e5
to
4773afc
Compare
✅ Deploy Preview for constellation-docs canceled.
|
4773afc
to
790a2d4
Compare
Rebased and incorporated feedback. Would you test it also locally on Mac to see if it works as expected for you @msanft? |
790a2d4
to
f2b89a4
Compare
I don't have a MacOS machine set up for development right now unfortunately |
45882ca
to
a819b86
Compare
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.
TF specific changes LGTM except the comment. Didn't look at release pipeline or Go upgrade. Just ping me once ready for final review
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.
Only reviewed the changes made to the pipeline. They look good to me.
Apologies for the delay in reviewing from my side.
One thing that is broken here, but that will be addressed in #2566: We cannot default to a We should rather let the user set the CLI version through a variable in the |
This is already fixed, Thomas and I decided to inject the version during the release process: constellation/dev-docs/workflows/release.md Lines 34 to 35 in cea6204
We didn't want to expose it as variable to the user, since the user should not need to care about the CLI version and which one is compatible with which TF module. |
a819b86
to
ba9fce4
Compare
Context
This PR builds on top of #2503 to add the module for GCP.
Proposed change(s)
Additional info
Checklist