Skip to content

Commit

Permalink
Bigtable: Check ForceNew for kms_key_name field in the diff function (G…
Browse files Browse the repository at this point in the history
  • Loading branch information
kevinsi4508 authored and hao-nan-li committed Dec 6, 2022
1 parent abbd3a8 commit 2421ac1
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ func resourceBigtableInstance() *schema.Resource {
"kms_key_name": {
Type: schema.TypeString,
Optional: true,
ForceNew: true,
Computed: true,
Description: `Describes the Cloud KMS encryption key that will be used to protect the destination Bigtable cluster. The requirements for this key are: 1) The Cloud Bigtable service account associated with the project that contains this cluster must be granted the cloudkms.cryptoKeyEncrypterDecrypter role on the CMEK key. 2) Only regional keys can be used and the region of the CMEK key must match the region of the cluster. 3) All clusters within an instance must use the same CMEK key. Values are of the form projects/{project}/locations/{location}/keyRings/{keyring}/cryptoKeys/{key}`,
},
Expand Down Expand Up @@ -561,6 +560,14 @@ func resourceBigtableInstanceClusterReorderTypeList(_ context.Context, diff *sch
return fmt.Errorf("Error setting cluster diff: %s", err)
}
}

oKey, nKey := diff.GetChange(fmt.Sprintf("cluster.%d.kms_key_name", i))
if oKey != nKey {
err := diff.ForceNew(fmt.Sprintf("cluster.%d.kms_key_name", i))
if err != nil {
return fmt.Errorf("Error setting cluster diff: %s", err)
}
}
}

return nil
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,8 @@ func TestAccBigtableInstance_kms(t *testing.T) {
skipIfVcr(t)
t.Parallel()

kms := BootstrapKMSKeyInLocation(t, "us-central1")
kms1 := BootstrapKMSKeyInLocation(t, "us-central1")
kms2 := BootstrapKMSKeyInLocation(t, "us-east1")
pid := getTestProjectFromEnv()
instanceName := fmt.Sprintf("tf-test-%s", randString(t, 10))

Expand All @@ -182,14 +183,20 @@ func TestAccBigtableInstance_kms(t *testing.T) {
CheckDestroy: testAccCheckBigtableInstanceDestroyProducer(t),
Steps: []resource.TestStep{
{
Config: testAccBigtableInstance_kms(pid, instanceName, kms.CryptoKey.Name, 3),
Config: testAccBigtableInstance_kms(pid, instanceName, kms1.CryptoKey.Name, 3),
},
{
ResourceName: "google_bigtable_instance.instance",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"deletion_protection", "instance_type"}, // we don't read instance type back
},
// TODO(kevinsi4508): Verify that the instance can be recreated due to `kms_key_name` change.
{
Config: testAccBigtableInstance_kms(pid, instanceName, kms2.CryptoKey.Name, 3),
PlanOnly: true,
ExpectNonEmptyPlan: true,
},
},
})
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,9 @@ for a `DEVELOPMENT` instance.

* `kms_key_name` - (Optional) Describes the Cloud KMS encryption key that will be used to protect the destination Bigtable cluster. The requirements for this key are: 1) The Cloud Bigtable service account associated with the project that contains this cluster must be granted the `cloudkms.cryptoKeyEncrypterDecrypter` role on the CMEK key. 2) Only regional keys can be used and the region of the CMEK key must match the region of the cluster.

!> **Warning**: Modifying this field will cause Terraform to delete/recreate the entire resource.
-> **Note**: Removing the field entirely from the config will cause the provider to default to the backend value.

-> **Note**: To remove this field once it is set, set the value to an empty string. Removing the field entirely from the config will cause the provider to default to the backend value.

!> **Warning:** Modifying the `storage_type` or `zone` of an existing cluster (by
!> **Warning:** Modifying the `storage_type`, `zone` or `kms_key_name` of an existing cluster (by
`cluster_id`) will cause Terraform to delete/recreate the entire
`google_bigtable_instance` resource. If these values are changing, use a new
`cluster_id`.
Expand Down

0 comments on commit 2421ac1

Please sign in to comment.