Skip to content
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

chore(KMS): cleanup isolated and redundant KMS resources #1271

Merged
merged 13 commits into from
Jun 26, 2024

Conversation

eeaton
Copy link
Collaborator

@eeaton eeaton commented Jun 21, 2024

Address #1248:

  • remove standalone CMEK configuration applied only to cai_monitoring module (tech debt from copying this module from somewhere else)
  • remove prj-$env-$bu-kms projects which are contradictory to best practice guidance on how to manage KMS host keys. Use the pre-existing prj-$env-kms projects instead.

…s KMS project is intended for use resources in the shared services common environment, not for org-level resources
…nces of prj-$env-$bu-kms that will be removed. Only intended to use prj-$env-kms
…d not be used. replace references to it with module.env_kms configured in Stage 2.
…a keyring in environment-wide org projects. Also remove some leftover references to KMS in cai-monitoring
…hecking for env_bu_kms_project that has been removed. env_kms_project is created in stage 2 and tested in envs_test.go)
Copy link
Contributor

@daniel-cit daniel-cit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe regarding CMEK in 0-bootstrapit cloud be made optional since the common KMS project does not exist yet and the user may need to enable CMEK on the Terraform state bucket

0-bootstrap/main.tf Outdated Show resolved Hide resolved
@eeaton
Copy link
Collaborator Author

eeaton commented Jun 24, 2024

You're right Daniel, I was working through some incremental changes and seeing the CI results, it has produced this error when tearing down the bootstrap stage:

Step #34 - "destroy-bootstrap": Resource module.seed_bootstrap.module.kms[0].google_kms_crypto_key.key[0] has
Step #34 - "destroy-bootstrap": lifecycle.prevent_destroy set, but the plan calls for this resource to be
Step #34 - "destroy-bootstrap": destroyed. To avoid this error and continue with the plan, either disable
Step #34 - "destroy-bootstrap": lifecycle.prevent_destroy or reduce the scope of the plan using the -target
Step #34 - "destroy-bootstrap": option.}
Step #34 - "destroy-bootstrap": Test: TestBootstrap

I'll revise this.

And regarding "Maybe regarding CMEK in 0-bootstrapit cloud be made optional since the common KMS project does not exist yet and the user may need to enable CMEK on the Terraform state bucket"...
Right, it was my intent to not modify the CMEK on the bootstrap state bucket yet, but I made a mistake on the last commit. I'll be recommending we convert to KMS Autokey as soon as it's GA, this will simplify the code and operation significantly from the current state. However, we'll need more design work to plan how to implement that at the bootstrap stage.

@eeaton eeaton enabled auto-merge (squash) June 24, 2024 16:17
@eeaton
Copy link
Collaborator Author

eeaton commented Jun 25, 2024

All checks are green... @daniel-cit and @apeabody , can I get approval please?

Copy link
Contributor

@daniel-cit daniel-cit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sleighton2022
Copy link
Collaborator

LGTM

auto-merge was automatically disabled June 26, 2024 15:04

Pull request was closed

@sleighton2022 sleighton2022 reopened this Jun 26, 2024
@sleighton2022 sleighton2022 merged commit df5817e into terraform-google-modules:master Jun 26, 2024
10 checks passed
@eeaton eeaton deleted the cleanup-kms branch June 26, 2024 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants