Make sure KMS key "deletion" disables rotation (#3624)

Signed-off-by: Modular Magician <[email protected]>

Author: modular-magician

google/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 key 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,18 @@ 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())
 
-	err = clearCryptoKeyVersions(cryptoKeyId, config)
-
-	if err != nil {
+	// Delete all versions of the key
+	if err := clearCryptoKeyVersions(cryptoKeyId, config); err != nil {
 		return err
 	}
 
+	// Make sure automatic key rotation is disabled.
+	if err := disableCryptoKeyRotation(cryptoKeyId, config); 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
 }

google/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" {