From 22dbe72e6af21fbf0a898111ff900812b1d77603 Mon Sep 17 00:00:00 2001 From: Emily Ye Date: Tue, 14 May 2019 16:00:09 -0700 Subject: [PATCH] disable autorotation for KMS key deletion --- .../resources/resource_kms_crypto_key.go | 32 +++++++++++--- .../tests/resource_kms_crypto_key_test.go | 44 +++++++++++++------ 2 files changed, 57 insertions(+), 19 deletions(-) diff --git a/third_party/terraform/resources/resource_kms_crypto_key.go b/third_party/terraform/resources/resource_kms_crypto_key.go index 2a4a852cb400..109c8b0c0967 100644 --- a/third_party/terraform/resources/resource_kms_crypto_key.go +++ b/third_party/terraform/resources/resource_kms_crypto_key.go @@ -224,12 +224,24 @@ func clearCryptoKeyVersions(cryptoKeyId *kmsCryptoKeyId, config *Config) error { return nil } -/* - Because KMS CryptoKey resources cannot be deleted on GCP, we are only going to remove it from state - and destroy all its versions, rendering the key useless for encryption and decryption of data. - Re-creation of this resource through Terraform will produce an error. -*/ +func disableCryptoKeyRotation(cryptoKeyId *kmsCryptoKeyId, config *Config) error { + keyClient := config.clientKms.Projects.Locations.KeyRings.CryptoKeys + _, err := keyClient.Patch(cryptoKeyId.cryptoKeyId(), &cloudkms.CryptoKey{ + NullFields: []string{"rotationPeriod", "nextRotationTime"}, + }). + UpdateMask("rotationPeriod,nextRotationTime").Do() + + return err +} +// Because KMS CryptoKey keys cannot be deleted (in GCP proper), we "delete" +// the key ring by +// a) marking all key versions for destruction (24hr soft-delete) +// b) disabling rotation of the key +// c) removing it from state +// This disables all usage of previous versions of the ky and makes it +// generally useless for encryption and decryption of data. +// Re-creation of this resource through Terraform will produce an error. func resourceKmsCryptoKeyDelete(d *schema.ResourceData, meta interface{}) error { config := meta.(*Config) @@ -242,12 +254,20 @@ func resourceKmsCryptoKeyDelete(d *schema.ResourceData, meta interface{}) error [WARNING] KMS CryptoKey resources cannot be deleted from GCP. The CryptoKey %s will be removed from Terraform state, and all its CryptoKeyVersions will be destroyed, but it will still be present on the server.`, cryptoKeyId.cryptoKeyId()) + // Delete all versions of the key err = clearCryptoKeyVersions(cryptoKeyId, config) - if err != nil { return err } + // Make sure automatic key rotation is disabled. + err = disableCryptoKeyRotation(cryptoKeyId, config) + if err != nil { + return fmt.Errorf( + "While cryptoKeyVersions were cleared, Terraform was unable to disable automatic rotation of key due to an error: %s."+ + "Please retry or manually disable automatic rotation to prevent creation of a new version of this key.", err) + } + d.SetId("") return nil } diff --git a/third_party/terraform/tests/resource_kms_crypto_key_test.go b/third_party/terraform/tests/resource_kms_crypto_key_test.go index dbf8f7968858..f6eed4e35c9c 100644 --- a/third_party/terraform/tests/resource_kms_crypto_key_test.go +++ b/third_party/terraform/tests/resource_kms_crypto_key_test.go @@ -137,6 +137,7 @@ func TestAccKmsCryptoKey_basic(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckGoogleKmsCryptoKeyWasRemovedFromState("google_kms_crypto_key.crypto_key"), testAccCheckGoogleKmsCryptoKeyVersionsDestroyed(projectId, location, keyRingName, cryptoKeyName), + testAccCheckGoogleKmsCryptoKeyRotationDisabled(projectId, location, keyRingName, cryptoKeyName), ), }, }, @@ -189,16 +190,15 @@ func TestAccKmsCryptoKey_rotation(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckGoogleKmsCryptoKeyWasRemovedFromState("google_kms_crypto_key.crypto_key"), testAccCheckGoogleKmsCryptoKeyVersionsDestroyed(projectId, location, keyRingName, cryptoKeyName), + testAccCheckGoogleKmsCryptoKeyRotationDisabled(projectId, location, keyRingName, cryptoKeyName), ), }, }, }) } -/* - KMS KeyRings cannot be deleted. This ensures that the CryptoKey resource was removed from state, - even though the server-side resource was not removed. -*/ +// KMS KeyRings cannot be deleted. This ensures that the CryptoKey resource was removed from state, +// even though the server-side resource was not removed. func testAccCheckGoogleKmsCryptoKeyWasRemovedFromState(resourceName string) resource.TestCheckFunc { return func(s *terraform.State) error { _, ok := s.RootModule().Resources[resourceName] @@ -211,11 +211,8 @@ func testAccCheckGoogleKmsCryptoKeyWasRemovedFromState(resourceName string) reso } } -/* - KMS KeyRings cannot be deleted. This ensures that the CryptoKey resource's CryptoKeyVersion - sub-resources were scheduled to be destroyed, rendering the key itself inoperable. -*/ - +// KMS KeyRings cannot be deleted. This ensures that the CryptoKey resource's CryptoKeyVersion +// sub-resources were scheduled to be destroyed, rendering the key itself inoperable. func testAccCheckGoogleKmsCryptoKeyVersionsDestroyed(projectId, location, keyRingName, cryptoKeyName string) resource.TestCheckFunc { return func(_ *terraform.State) error { config := testAccProvider.Meta().(*Config) @@ -239,10 +236,31 @@ func testAccCheckGoogleKmsCryptoKeyVersionsDestroyed(projectId, location, keyRin } } -/* - This test runs in its own project, otherwise the test project would start to get filled - with undeletable resources -*/ +// KMS KeyRings cannot be deleted. This ensures that the CryptoKey autorotation +// was disabled to prevent more versions of the key from being created. +func testAccCheckGoogleKmsCryptoKeyRotationDisabled(projectId, location, keyRingName, cryptoKeyName string) resource.TestCheckFunc { + return func(_ *terraform.State) error { + config := testAccProvider.Meta().(*Config) + gcpResourceUri := fmt.Sprintf("projects/%s/locations/%s/keyRings/%s/cryptoKeys/%s", projectId, location, keyRingName, cryptoKeyName) + + response, err := config.clientKms.Projects.Locations.KeyRings.CryptoKeys.Get(gcpResourceUri).Do() + if err != nil { + return fmt.Errorf("Unexpected failure while verifying 'deleted' crypto key: %s", err) + } + + if response.NextRotationTime != "" { + return fmt.Errorf("Expected empty nextRotationTime for 'deleted' crypto key, got %s", response.NextRotationTime) + } + if response.RotationPeriod != "" { + return fmt.Errorf("Expected empty RotationPeriod for 'deleted' crypto key, got %s", response.RotationPeriod) + } + + return nil + } +} + +// This test runs in its own project, otherwise the test project would start to get filled +// with undeletable resources func testGoogleKmsCryptoKey_basic(projectId, projectOrg, projectBillingAccount, keyRingName, cryptoKeyName string) string { return fmt.Sprintf(` resource "google_project" "acceptance" {