Add docs on setting up a new project #340
Conversation
|
We can test this out on #207 |
|
Thanks @damianavila! Fixed :) |
| your user account. | ||
|
|
||
| 4. Commit the change, make a PR to the repo with it, and merge it. This is | ||
| preferable to pushing to the repo directly. You can self-merge your PR. |
There was a problem hiding this comment.
Are you sure you want to allow self-merge when the PR has the potential to destroy things? (the second point you wrote above)
There was a problem hiding this comment.
The danger is in the terraform apply step, which is why it doesn't run automatically. Merging doesn't actually trigger anything.
There was a problem hiding this comment.
OK, thanks for the clarification, so how do we make sure others can see/review that "state" before being applied?
There was a problem hiding this comment.
Great question! What kinda workflow do you think would work here? I don't want to run terraform apply in CI, so not sure how to approach review.
There was a problem hiding this comment.
I have not worked with the terraform stuff yet (although I have some experience with AWS Cloudformation), but looking quickly at the terraform docs, it seems you have a terraform plan command. That should surface any problem (or deletion you are trying to prevent), right? Then, you could run that command by the CI and detect any "bad" message. And others can review that plan.
Additionally, it seems you can run that command with a -out parameter and get some file that you can eventually run manually later with terraform apply command (modulo you still do not want to run apply within the CI which could be an event happening on the merge event after a successful CI run and approval from others, but I totally get it is frightening to give the CI such destructive power): https://www.terraform.io/docs/cli/commands/plan.html
Finally, I may be saying stupids things (as I did in the other PR) because I do not know yet the deep stuff, so be patient with me 😉
There was a problem hiding this comment.
I'm a little bit confused now as step 3 is "Check the project has been created and you have access to it" and step 4 is "open the PR" which implies terraform apply was run before opening the PR?
There was a problem hiding this comment.
Ah, my brain just caught up with me! Yes, I think we should approve/merge the PR before running terraform apply - always good to have a second pair of eyes on code/configs before actioning anything I think.
There was a problem hiding this comment.
- Someone else approves PR
This is where terrraform plan (with sensitive on to prevent leaking info/secret) plays a role, IMHO.
Otherwise, the person reviewing the PR does not have any info about how the plan looks like.
There was a problem hiding this comment.
@damianavila @sgibson91 I updated the instructions based on the conversations here. LMK what you think? I wanna balance code review with appropriate load on our small team.
| 2. Run terraform (with instructions in the org-ops repo README). This will give you a | ||
| detailed plan on what exactly terraform will do. [prevent_destroy](https://www.terraform.io/docs/language/meta-arguments/lifecycle.html#prevent_destroy) | ||
| is set to most sensitive resources to prevent accidental destruction - however, | ||
| you should still scrutinize the plan produced by `terraform apply` carefully before applying it. |
There was a problem hiding this comment.
| you should still scrutinize the plan produced by `terraform apply` carefully before applying it. | |
| you should still scrutinize the plan produced by `terraform plan` carefully before applying it. |
Is this supposed to be terraform plan or do you mean before accepting the Continue with apply? prompt?
There was a problem hiding this comment.
LGTM, the only remaining thing would be to mark the secrets as sensitive... but looking at https://github.com/2i2c-org/org-ops/blob/main/projects.tfvars, you already have "visible" data there, so I think this is ready as-is.
|
Thanks, @damianavila! Wanna hit merge? :D |
|
Done 🎉 😉 |
Ref #335