From d7d84c18765a9721968ed26c4a707fa78e22b906 Mon Sep 17 00:00:00 2001 From: Chris Stephens Date: Mon, 15 Oct 2018 16:57:24 -0700 Subject: [PATCH 01/10] adding KMS key encryption to disks Also adding to Image and Snapshot even though those resources aren't generated yet. --- products/compute/api.yaml | 30 +++++++++++++++++++++++++++ products/compute/disk_parameters.yaml | 12 +++++++++++ products/compute/terraform.yaml | 24 +++++++++++++++++++++ 3 files changed, 66 insertions(+) diff --git a/products/compute/api.yaml b/products/compute/api.yaml index 337381d54768..d4da1118d3f8 100644 --- a/products/compute/api.yaml +++ b/products/compute/api.yaml @@ -627,6 +627,12 @@ objects: The RFC 4648 base64 encoded SHA-256 hash of the customer-supplied encryption key that protects this resource. output: true + - !ruby/object:Api::Type::String + # TODO(chrisst) Change to ResourceRef once KMS is in Magic Modules + name: 'kmsKeyName' + min_version: beta + description: | + The name of the encryption key that is stored in Google Cloud KMS. input: true - !ruby/object:Api::Type::String name: 'sourceImageId' @@ -1702,6 +1708,12 @@ objects: The RFC 4648 base64 encoded SHA-256 hash of the customer-supplied encryption key that protects this resource. output: true + - !ruby/object:Api::Type::String + # TODO(chrisst) Change to ResourceRef once KMS is in Magic Modules + name: 'kmsKeyName' + min_version: beta + description: | + The name of the encryption key that is stored in Google Cloud KMS. # TODO(alexstephen): Change to ResourceRef with array support - !ruby/object:Api::Type::Array name: 'licenses' @@ -1768,6 +1780,12 @@ objects: The RFC 4648 base64 encoded SHA-256 hash of the customer-supplied encryption key that protects this resource. output: true + - !ruby/object:Api::Type::String + # TODO(chrisst) Change to ResourceRef once KMS is in Magic Modules + name: 'kmsKeyName' + min_version: beta + description: | + The name of the encryption key that is stored in Google Cloud KMS. - !ruby/object:Api::Type::String name: 'sourceDiskId' description: | @@ -2995,6 +3013,12 @@ objects: The RFC 4648 base64 encoded SHA-256 hash of the customer-supplied encryption key that protects this resource. output: true + - !ruby/object:Api::Type::String + # TODO(chrisst) Change to ResourceRef once KMS is in Magic Modules + name: 'kmsKeyName' + min_version: beta + description: | + The name of the encryption key that is stored in Google Cloud KMS. input: true - !ruby/object:Api::Type::NestedObject name: 'sourceDiskEncryptionKey' @@ -3014,6 +3038,12 @@ objects: The RFC 4648 base64 encoded SHA-256 hash of the customer-supplied encryption key that protects this resource. output: true + - !ruby/object:Api::Type::String + # TODO(chrisst) Change to ResourceRef once KMS is in Magic Modules + name: 'kmsKeyName' + min_version: beta + description: | + The name of the encryption key that is stored in Google Cloud KMS. input: true properties: - !ruby/object:Api::Type::Time diff --git a/products/compute/disk_parameters.yaml b/products/compute/disk_parameters.yaml index 7e7bdc39a31b..3b3fe3190b90 100644 --- a/products/compute/disk_parameters.yaml +++ b/products/compute/disk_parameters.yaml @@ -37,6 +37,12 @@ The RFC 4648 base64 encoded SHA-256 hash of the customer-supplied encryption key that protects this resource. output: true + - !ruby/object:Api::Type::String + # TODO(chrisst) Change to ResourceRef once KMS is in Magic Modules + name: 'kmsKeyName' + min_version: beta + description: | + The name of the encryption key that is stored in Google Cloud KMS. input: true - !ruby/object:Api::Type::ResourceRef name: 'sourceSnapshot' @@ -62,6 +68,12 @@ description: | Specifies a 256-bit customer-supplied encryption key, encoded in RFC 4648 base64 to either encrypt or decrypt this resource. + - !ruby/object:Api::Type::String + # TODO(chrisst) Change to ResourceRef once KMS is in Magic Modules + name: 'kmsKeyName' + min_version: beta + description: | + The name of the encryption key that is stored in Google Cloud KMS. - !ruby/object:Api::Type::String name: 'sha256' description: | diff --git a/products/compute/terraform.yaml b/products/compute/terraform.yaml index 84c7aeef972b..25b49ffdd420 100644 --- a/products/compute/terraform.yaml +++ b/products/compute/terraform.yaml @@ -114,6 +114,30 @@ overrides: !ruby/object:Provider::ResourceOverrides override_order: -1 name: image diff_suppress_func: 'diskImageDiffSuppress' + diskEncryptionKey.kmsKeyName: !ruby/object:Provider::Terraform::PropertyOverride + diff_suppress_func: 'compareSelfLinkRelativePaths' + name: "kms_key_self_link" + description: | + The self link of the encryption key used to encrypt the disk. Also called KmsKeyName + in the cloud console. In order to use this additional + IAM permissions need to be set on the Compute Engine Service Agent. See + https://cloud.google.com/compute/docs/disks/customer-managed-encryption#encrypt_a_new_persistent_disk_with_your_own_keys + sourceSnapshotEncryptionKey.kmsKeyName: !ruby/object:Provider::Terraform::PropertyOverride + diff_suppress_func: 'compareSelfLinkRelativePaths' + name: "kms_key_self_link" + description: | + The self link of the encryption key used to encrypt the disk. Also called KmsKeyName + in the cloud console. In order to use this additional + IAM permissions need to be set on the Compute Engine Service Agent. See + https://cloud.google.com/compute/docs/disks/customer-managed-encryption#encrypt_a_new_persistent_disk_with_your_own_keys + sourceImageEncryptionKey.kmsKeyName: !ruby/object:Provider::Terraform::PropertyOverride + diff_suppress_func: 'compareSelfLinkRelativePaths' + name: "kms_key_self_link" + description: | + The self link of the encryption key used to encrypt the disk. Also called KmsKeyName + in the cloud console. In order to use this additional + IAM permissions need to be set on the Compute Engine Service Agent. See + https://cloud.google.com/compute/docs/disks/customer-managed-encryption#encrypt_a_new_persistent_disk_with_your_own_keys image: !ruby/object:Provider::Terraform::PropertyOverride override_order: 5 description: | From a75afebab63c2ee0b550356be9fc75d88a7664cc Mon Sep 17 00:00:00 2001 From: Chris Stephens Date: Mon, 15 Oct 2018 19:27:54 -0700 Subject: [PATCH 02/10] update custom decoder --- templates/terraform/decoders/disk.erb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/templates/terraform/decoders/disk.erb b/templates/terraform/decoders/disk.erb index eee89387bc96..b3418e82084a 100644 --- a/templates/terraform/decoders/disk.erb +++ b/templates/terraform/decoders/disk.erb @@ -4,6 +4,7 @@ if v, ok := res["diskEncryptionKey"]; ok { // The raw key won't be returned, so we need to use the original. transformed["rawKey"] = d.Get("disk_encryption_key.0.raw_key") transformed["sha256"] = original["sha256"] + transformed["kmsKeyName"] = d.Get("disk_encryption_key.0.kms_key_self_link") res["diskEncryptionKey"] = transformed } @@ -13,6 +14,7 @@ if v, ok := res["sourceImageEncryptionKey"]; ok { // The raw key won't be returned, so we need to use the original. transformed["rawKey"] = d.Get("source_image_encryption_key.0.raw_key") transformed["sha256"] = original["sha256"] + transformed["kmsKeyName"] = d.Get("disk_encryption_key.0.kms_key_self_link") res["sourceImageEncryptionKey"] = transformed } @@ -22,6 +24,7 @@ if v, ok := res["sourceSnapshotEncryptionKey"]; ok { // The raw key won't be returned, so we need to use the original. transformed["rawKey"] = d.Get("source_snapshot_encryption_key.0.raw_key") transformed["sha256"] = original["sha256"] + transformed["kmsKeyName"] = d.Get("disk_encryption_key.0.kms_key_self_link") res["sourceSnapshotEncryptionKey"] = transformed } From c5a47a3953ce03fb1bea6dc5963c1f90df0eccf1 Mon Sep 17 00:00:00 2001 From: Chris Stephens Date: Tue, 16 Oct 2018 11:56:13 -0700 Subject: [PATCH 03/10] fix importing disks with kms keys --- templates/terraform/decoders/disk.erb | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/templates/terraform/decoders/disk.erb b/templates/terraform/decoders/disk.erb index b3418e82084a..647de26d8dad 100644 --- a/templates/terraform/decoders/disk.erb +++ b/templates/terraform/decoders/disk.erb @@ -1,10 +1,14 @@ +// The response for crypto keys often includes the version of the key which needs to be removed +// format: projects//locations//keyRings//cryptoKeys//cryptoKeyVersions/1 +reg := regexp.MustCompile("/cryptoKeyVersions") + if v, ok := res["diskEncryptionKey"]; ok { original := v.(map[string]interface{}) transformed := make(map[string]interface{}) // The raw key won't be returned, so we need to use the original. transformed["rawKey"] = d.Get("disk_encryption_key.0.raw_key") transformed["sha256"] = original["sha256"] - transformed["kmsKeyName"] = d.Get("disk_encryption_key.0.kms_key_self_link") + transformed["kmsKeyName"] = reg.Split(original["kmsKeyName"].(string), 2)[0] res["diskEncryptionKey"] = transformed } @@ -14,7 +18,7 @@ if v, ok := res["sourceImageEncryptionKey"]; ok { // The raw key won't be returned, so we need to use the original. transformed["rawKey"] = d.Get("source_image_encryption_key.0.raw_key") transformed["sha256"] = original["sha256"] - transformed["kmsKeyName"] = d.Get("disk_encryption_key.0.kms_key_self_link") + transformed["kmsKeyName"] = reg.Split(original["kmsKeyName"].(string), 2)[0] res["sourceImageEncryptionKey"] = transformed } @@ -24,7 +28,7 @@ if v, ok := res["sourceSnapshotEncryptionKey"]; ok { // The raw key won't be returned, so we need to use the original. transformed["rawKey"] = d.Get("source_snapshot_encryption_key.0.raw_key") transformed["sha256"] = original["sha256"] - transformed["kmsKeyName"] = d.Get("disk_encryption_key.0.kms_key_self_link") + transformed["kmsKeyName"] = reg.Split(original["kmsKeyName"].(string), 2)[0] res["sourceSnapshotEncryptionKey"] = transformed } From f7d759afbb1cad0b4ef0847dcf93c2d0fb821e2e Mon Sep 17 00:00:00 2001 From: Chris Stephens Date: Tue, 16 Oct 2018 12:08:41 -0700 Subject: [PATCH 04/10] Adding kms disk tests --- .../tests/resource_compute_disk_test.go.erb | 78 +++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/provider/terraform/tests/resource_compute_disk_test.go.erb b/provider/terraform/tests/resource_compute_disk_test.go.erb index d315646e91cc..faee0fe3128b 100644 --- a/provider/terraform/tests/resource_compute_disk_test.go.erb +++ b/provider/terraform/tests/resource_compute_disk_test.go.erb @@ -343,6 +343,39 @@ func TestAccComputeDisk_encryption(t *testing.T) { }) } +func TestAccComputeDisk_encryptionKMS(t *testing.T) { + skipIfEnvNotSet(t, "GOOGLE_PROJECT_NUMBER") + t.Parallel() + + serviceAgent := fmt.Sprintf("service-%s@compute-system.iam.gserviceaccount.com", os.Getenv("GOOGLE_PROJECT_NUMBER")) + diskName := fmt.Sprintf("tf-test-%s", acctest.RandString(10)) + keyRingName := fmt.Sprintf("tf-test-%s", acctest.RandString(10)) + keyName := fmt.Sprintf("tf-test-%s", acctest.RandString(10)) + var disk compute.Disk + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckComputeDiskDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccComputeDisk_encryptionKMS(serviceAgent, getTestProjectFromEnv(), diskName, keyRingName, keyName), + Check: resource.ComposeTestCheckFunc( + testAccCheckComputeDiskExists( + "google_compute_disk.foobar", &disk), + testAccCheckEncryptionKey( + "google_compute_disk.foobar", &disk), + ), + }, + resource.TestStep{ + ResourceName: "google_compute_disk.foobar", + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + func TestAccComputeDisk_deleteDetach(t *testing.T) { t.Parallel() @@ -694,6 +727,51 @@ resource "google_compute_disk" "foobar" { }`, diskName) } +func testAccComputeDisk_encryptionKMS(serviceAgent, project, diskName, keyRingName, keyName string) string { + return fmt.Sprintf(` +data "google_compute_image" "my_image" { + family = "debian-9" + project = "debian-cloud" +} + +resource "google_project_iam_member" "kms-project-binding" { + project = "%s" + role = "roles/cloudkms.cryptoKeyEncrypterDecrypter" + member = "serviceAccount:%s" +} + +resource "google_kms_crypto_key_iam_binding" "kms-key-binding" { + crypto_key_id = "${google_kms_crypto_key.my_crypto_key.self_link}" + role = "roles/cloudkms.cryptoKeyEncrypterDecrypter" + + members = [ + "serviceAccount:%s", + ] +} + +resource "google_kms_key_ring" "my_key_ring" { + name = "%s" + location = "us-central1" +} + +resource "google_kms_crypto_key" "my_crypto_key" { + name = "%s" + key_ring = "${google_kms_key_ring.my_key_ring.self_link}" +} + +resource "google_compute_disk" "foobar" { + name = "%s" + image = "${data.google_compute_image.my_image.self_link}" + size = 10 + type = "pd-ssd" + zone = "us-central1-a" + + disk_encryption_key { + kms_key_self_link = "${google_kms_crypto_key.my_crypto_key.self_link}" + } +}`, project, serviceAgent, serviceAgent, keyRingName, keyName, diskName) +} + func testAccComputeDisk_deleteDetach(instanceName, diskName string) string { return fmt.Sprintf(` data "google_compute_image" "my_image" { From 93a43d3902c81c8847e52f7df58af3896d969e7d Mon Sep 17 00:00:00 2001 From: Chris Stephens Date: Wed, 17 Oct 2018 14:11:36 -0700 Subject: [PATCH 05/10] Set up KMS disk test to use new project --- products/compute/terraform.yaml | 6 +- .../tests/resource_compute_disk_test.go.erb | 87 +++++++++++++------ templates/terraform/decoders/disk.erb | 16 ++-- 3 files changed, 72 insertions(+), 37 deletions(-) diff --git a/products/compute/terraform.yaml b/products/compute/terraform.yaml index 25b49ffdd420..f92a9957394c 100644 --- a/products/compute/terraform.yaml +++ b/products/compute/terraform.yaml @@ -116,7 +116,7 @@ overrides: !ruby/object:Provider::ResourceOverrides diff_suppress_func: 'diskImageDiffSuppress' diskEncryptionKey.kmsKeyName: !ruby/object:Provider::Terraform::PropertyOverride diff_suppress_func: 'compareSelfLinkRelativePaths' - name: "kms_key_self_link" + name: "kmsKeySelfLink" description: | The self link of the encryption key used to encrypt the disk. Also called KmsKeyName in the cloud console. In order to use this additional @@ -124,7 +124,7 @@ overrides: !ruby/object:Provider::ResourceOverrides https://cloud.google.com/compute/docs/disks/customer-managed-encryption#encrypt_a_new_persistent_disk_with_your_own_keys sourceSnapshotEncryptionKey.kmsKeyName: !ruby/object:Provider::Terraform::PropertyOverride diff_suppress_func: 'compareSelfLinkRelativePaths' - name: "kms_key_self_link" + name: "kmsKeySelfLink" description: | The self link of the encryption key used to encrypt the disk. Also called KmsKeyName in the cloud console. In order to use this additional @@ -132,7 +132,7 @@ overrides: !ruby/object:Provider::ResourceOverrides https://cloud.google.com/compute/docs/disks/customer-managed-encryption#encrypt_a_new_persistent_disk_with_your_own_keys sourceImageEncryptionKey.kmsKeyName: !ruby/object:Provider::Terraform::PropertyOverride diff_suppress_func: 'compareSelfLinkRelativePaths' - name: "kms_key_self_link" + name: "kmsKeySelfLink" description: | The self link of the encryption key used to encrypt the disk. Also called KmsKeyName in the cloud console. In order to use this additional diff --git a/provider/terraform/tests/resource_compute_disk_test.go.erb b/provider/terraform/tests/resource_compute_disk_test.go.erb index faee0fe3128b..2c04b3e7c5a1 100644 --- a/provider/terraform/tests/resource_compute_disk_test.go.erb +++ b/provider/terraform/tests/resource_compute_disk_test.go.erb @@ -221,7 +221,7 @@ func TestAccComputeDisk_basic(t *testing.T) { Config: testAccComputeDisk_basic(diskName), Check: resource.ComposeTestCheckFunc( testAccCheckComputeDiskExists( - "google_compute_disk.foobar", &disk), + "google_compute_disk.foobar", getTestProjectFromEnv(), &disk), testAccCheckComputeDiskHasLabel(&disk, "my-label", "my-label-value"), testAccCheckComputeDiskHasLabelFingerprint(&disk, "google_compute_disk.foobar"), ), @@ -265,7 +265,7 @@ func TestAccComputeDisk_update(t *testing.T) { Config: testAccComputeDisk_basic(diskName), Check: resource.ComposeTestCheckFunc( testAccCheckComputeDiskExists( - "google_compute_disk.foobar", &disk), + "google_compute_disk.foobar", getTestProjectFromEnv(), &disk), resource.TestCheckResourceAttr("google_compute_disk.foobar", "size", "50"), testAccCheckComputeDiskHasLabel(&disk, "my-label", "my-label-value"), testAccCheckComputeDiskHasLabelFingerprint(&disk, "google_compute_disk.foobar"), @@ -275,7 +275,7 @@ func TestAccComputeDisk_update(t *testing.T) { Config: testAccComputeDisk_updated(diskName), Check: resource.ComposeTestCheckFunc( testAccCheckComputeDiskExists( - "google_compute_disk.foobar", &disk), + "google_compute_disk.foobar", getTestProjectFromEnv(), &disk), resource.TestCheckResourceAttr("google_compute_disk.foobar", "size", "100"), testAccCheckComputeDiskHasLabel(&disk, "my-label", "my-updated-label-value"), testAccCheckComputeDiskHasLabel(&disk, "a-new-label", "a-new-label-value"), @@ -305,14 +305,14 @@ func TestAccComputeDisk_fromSnapshot(t *testing.T) { Config: testAccComputeDisk_fromSnapshot(projectName, firstDiskName, snapshotName, diskName, "self_link"), Check: resource.ComposeTestCheckFunc( testAccCheckComputeDiskExists( - "google_compute_disk.seconddisk", &disk), + "google_compute_disk.seconddisk", getTestProjectFromEnv(), &disk), ), }, resource.TestStep{ Config: testAccComputeDisk_fromSnapshot(projectName, firstDiskName, snapshotName, diskName, "name"), Check: resource.ComposeTestCheckFunc( testAccCheckComputeDiskExists( - "google_compute_disk.seconddisk", &disk), + "google_compute_disk.seconddisk", getTestProjectFromEnv(), &disk), ), }, }, @@ -334,7 +334,7 @@ func TestAccComputeDisk_encryption(t *testing.T) { Config: testAccComputeDisk_encryption(diskName), Check: resource.ComposeTestCheckFunc( testAccCheckComputeDiskExists( - "google_compute_disk.foobar", &disk), + "google_compute_disk.foobar", getTestProjectFromEnv(), &disk), testAccCheckEncryptionKey( "google_compute_disk.foobar", &disk), ), @@ -344,13 +344,15 @@ func TestAccComputeDisk_encryption(t *testing.T) { } func TestAccComputeDisk_encryptionKMS(t *testing.T) { - skipIfEnvNotSet(t, "GOOGLE_PROJECT_NUMBER") t.Parallel() - serviceAgent := fmt.Sprintf("service-%s@compute-system.iam.gserviceaccount.com", os.Getenv("GOOGLE_PROJECT_NUMBER")) + org := getTestOrgFromEnv(t) + pid := "tf-test-" + acctest.RandString(10) + billingAccount := getTestBillingAccountFromEnv(t) diskName := fmt.Sprintf("tf-test-%s", acctest.RandString(10)) keyRingName := fmt.Sprintf("tf-test-%s", acctest.RandString(10)) keyName := fmt.Sprintf("tf-test-%s", acctest.RandString(10)) + importID := fmt.Sprintf("%s/%s/%s", pid, "us-central1-a", diskName) var disk compute.Disk resource.Test(t, resource.TestCase{ @@ -359,16 +361,17 @@ func TestAccComputeDisk_encryptionKMS(t *testing.T) { CheckDestroy: testAccCheckComputeDiskDestroy, Steps: []resource.TestStep{ resource.TestStep{ - Config: testAccComputeDisk_encryptionKMS(serviceAgent, getTestProjectFromEnv(), diskName, keyRingName, keyName), + Config: testAccComputeDisk_encryptionKMS(pid, pname, org, billingAccount, diskName, keyRingName, keyName), Check: resource.ComposeTestCheckFunc( testAccCheckComputeDiskExists( - "google_compute_disk.foobar", &disk), + "google_compute_disk.foobar", pid, &disk), testAccCheckEncryptionKey( "google_compute_disk.foobar", &disk), ), }, resource.TestStep{ ResourceName: "google_compute_disk.foobar", + ImportStateId: importID, ImportState: true, ImportStateVerify: true, }, @@ -392,7 +395,7 @@ func TestAccComputeDisk_deleteDetach(t *testing.T) { Config: testAccComputeDisk_deleteDetach(instanceName, diskName), Check: resource.ComposeTestCheckFunc( testAccCheckComputeDiskExists( - "google_compute_disk.foo", &disk), + "google_compute_disk.foo", getTestProjectFromEnv(), &disk), ), }, // this needs to be a second step so we refresh and see the instance @@ -403,7 +406,7 @@ func TestAccComputeDisk_deleteDetach(t *testing.T) { Config: testAccComputeDisk_deleteDetach(instanceName, diskName), Check: resource.ComposeTestCheckFunc( testAccCheckComputeDiskExists( - "google_compute_disk.foo", &disk), + "google_compute_disk.foo", getTestProjectFromEnv(), &disk), testAccCheckComputeDiskInstances( "google_compute_disk.foo", &disk), ), @@ -429,7 +432,7 @@ func TestAccComputeDisk_deleteDetachIGM(t *testing.T) { Config: testAccComputeDisk_deleteDetachIGM(diskName, mgrName), Check: resource.ComposeTestCheckFunc( testAccCheckComputeDiskExists( - "google_compute_disk.foo", &disk), + "google_compute_disk.foo", getTestProjectFromEnv(), &disk), ), }, // this needs to be a second step so we refresh and see the instance @@ -440,7 +443,7 @@ func TestAccComputeDisk_deleteDetachIGM(t *testing.T) { Config: testAccComputeDisk_deleteDetachIGM(diskName, mgrName), Check: resource.ComposeTestCheckFunc( testAccCheckComputeDiskExists( - "google_compute_disk.foo", &disk), + "google_compute_disk.foo", getTestProjectFromEnv(), &disk), testAccCheckComputeDiskInstances( "google_compute_disk.foo", &disk), ), @@ -450,7 +453,7 @@ func TestAccComputeDisk_deleteDetachIGM(t *testing.T) { Config: testAccComputeDisk_deleteDetachIGM(diskName2, mgrName), Check: resource.ComposeTestCheckFunc( testAccCheckComputeDiskExists( - "google_compute_disk.foo", &disk), + "google_compute_disk.foo", getTestProjectFromEnv(), &disk), ), }, // Add the extra step like before @@ -458,7 +461,7 @@ func TestAccComputeDisk_deleteDetachIGM(t *testing.T) { Config: testAccComputeDisk_deleteDetachIGM(diskName2, mgrName), Check: resource.ComposeTestCheckFunc( testAccCheckComputeDiskExists( - "google_compute_disk.foo", &disk), + "google_compute_disk.foo", getTestProjectFromEnv(), &disk), testAccCheckComputeDiskInstances( "google_compute_disk.foo", &disk), ), @@ -517,9 +520,8 @@ func testAccCheckComputeDiskDestroy(s *terraform.State) error { return nil } -func testAccCheckComputeDiskExists(n string, disk *compute.Disk) resource.TestCheckFunc { +func testAccCheckComputeDiskExists(n, p string, disk *compute.Disk) resource.TestCheckFunc { return func(s *terraform.State) error { - p := getTestProjectFromEnv() rs, ok := s.RootModule().Resources[n] if !ok { return fmt.Errorf("Not found: %s", n) @@ -727,17 +729,37 @@ resource "google_compute_disk" "foobar" { }`, diskName) } -func testAccComputeDisk_encryptionKMS(serviceAgent, project, diskName, keyRingName, keyName string) string { +// TODO chrisst - exclude this test from the non-beta provider +func testAccComputeDisk_encryptionKMS(pid, pname, org, billing, diskName, keyRingName, keyName string) string { return fmt.Sprintf(` +resource "google_project" "project" { + project_id = "%s" + name = "%s" + org_id = "%s" + billing_account = "%s" +} + data "google_compute_image" "my_image" { family = "debian-9" project = "debian-cloud" } +resource "google_project_services" "apis" { + project = "${google_project.project.project_id}" + services = [ + "oslogin.googleapis.com", + "compute.googleapis.com", + "cloudkms.googleapis.com", + "appengine.googleapis.com" + ] +} + resource "google_project_iam_member" "kms-project-binding" { - project = "%s" + project = "${google_project.project.project_id}" role = "roles/cloudkms.cryptoKeyEncrypterDecrypter" - member = "serviceAccount:%s" + member = "serviceAccount:service-${google_project.project.number}@compute-system.iam.gserviceaccount.com" + + depends_on = ["google_project_services.apis"] } resource "google_kms_crypto_key_iam_binding" "kms-key-binding" { @@ -745,13 +767,18 @@ resource "google_kms_crypto_key_iam_binding" "kms-key-binding" { role = "roles/cloudkms.cryptoKeyEncrypterDecrypter" members = [ - "serviceAccount:%s", - ] + "serviceAccount:service-${google_project.project.number}@compute-system.iam.gserviceaccount.com", + ] + + depends_on = ["google_project_services.apis"] } resource "google_kms_key_ring" "my_key_ring" { - name = "%s" - location = "us-central1" + name = "%s" + project = "${google_project.project.project_id}" + location = "us-central1" + + depends_on = ["google_project_services.apis"] } resource "google_kms_crypto_key" "my_crypto_key" { @@ -765,11 +792,17 @@ resource "google_compute_disk" "foobar" { size = 10 type = "pd-ssd" zone = "us-central1-a" + project = "${google_project.project.project_id}" disk_encryption_key { kms_key_self_link = "${google_kms_crypto_key.my_crypto_key.self_link}" - } -}`, project, serviceAgent, serviceAgent, keyRingName, keyName, diskName) + } + + depends_on = [ + "google_kms_crypto_key_iam_binding.kms-key-binding", + "google_project_iam_member.kms-project-binding" + ] +}`, pid, pname, org, billing, keyRingName, keyName, diskName) } func testAccComputeDisk_deleteDetach(instanceName, diskName string) string { diff --git a/templates/terraform/decoders/disk.erb b/templates/terraform/decoders/disk.erb index 647de26d8dad..1693b550fd57 100644 --- a/templates/terraform/decoders/disk.erb +++ b/templates/terraform/decoders/disk.erb @@ -1,14 +1,12 @@ -// The response for crypto keys often includes the version of the key which needs to be removed -// format: projects//locations//keyRings//cryptoKeys//cryptoKeyVersions/1 -reg := regexp.MustCompile("/cryptoKeyVersions") - if v, ok := res["diskEncryptionKey"]; ok { original := v.(map[string]interface{}) transformed := make(map[string]interface{}) // The raw key won't be returned, so we need to use the original. transformed["rawKey"] = d.Get("disk_encryption_key.0.raw_key") transformed["sha256"] = original["sha256"] - transformed["kmsKeyName"] = reg.Split(original["kmsKeyName"].(string), 2)[0] + // The response for crypto keys often includes the version of the key which needs to be removed + // format: projects//locations//keyRings//cryptoKeys//cryptoKeyVersions/1 + transformed["kmsKeyName"] = strings.Split(original["kmsKeyName"].(string), "/cryptoKeyVersions")[0] res["diskEncryptionKey"] = transformed } @@ -18,7 +16,9 @@ if v, ok := res["sourceImageEncryptionKey"]; ok { // The raw key won't be returned, so we need to use the original. transformed["rawKey"] = d.Get("source_image_encryption_key.0.raw_key") transformed["sha256"] = original["sha256"] - transformed["kmsKeyName"] = reg.Split(original["kmsKeyName"].(string), 2)[0] + // The response for crypto keys often includes the version of the key which needs to be removed + // format: projects//locations//keyRings//cryptoKeys//cryptoKeyVersions/1 + transformed["kmsKeyName"] = strings.Split(original["kmsKeyName"].(string), "/cryptoKeyVersions")[0] res["sourceImageEncryptionKey"] = transformed } @@ -28,7 +28,9 @@ if v, ok := res["sourceSnapshotEncryptionKey"]; ok { // The raw key won't be returned, so we need to use the original. transformed["rawKey"] = d.Get("source_snapshot_encryption_key.0.raw_key") transformed["sha256"] = original["sha256"] - transformed["kmsKeyName"] = reg.Split(original["kmsKeyName"].(string), 2)[0] + // The response for crypto keys often includes the version of the key which needs to be removed + // format: projects//locations//keyRings//cryptoKeys//cryptoKeyVersions/1 + transformed["kmsKeyName"] = strings.Split(original["kmsKeyName"].(string), "/cryptoKeyVersions")[0] res["sourceSnapshotEncryptionKey"] = transformed } From 2b2596aa724f4932aaaa503acc5bdf957c96c5a1 Mon Sep 17 00:00:00 2001 From: Chris Stephens Date: Wed, 17 Oct 2018 15:28:20 -0700 Subject: [PATCH 06/10] fix whitespace and exclude beta code --- .../tests/resource_compute_disk_test.go.erb | 62 ++++++++++--------- templates/terraform/decoders/disk.erb | 6 ++ 2 files changed, 38 insertions(+), 30 deletions(-) diff --git a/provider/terraform/tests/resource_compute_disk_test.go.erb b/provider/terraform/tests/resource_compute_disk_test.go.erb index 2c04b3e7c5a1..f322153f9f53 100644 --- a/provider/terraform/tests/resource_compute_disk_test.go.erb +++ b/provider/terraform/tests/resource_compute_disk_test.go.erb @@ -735,31 +735,32 @@ func testAccComputeDisk_encryptionKMS(pid, pname, org, billing, diskName, keyRin resource "google_project" "project" { project_id = "%s" name = "%s" - org_id = "%s" - billing_account = "%s" + org_id = "%s" + billing_account = "%s" } data "google_compute_image" "my_image" { - family = "debian-9" - project = "debian-cloud" + family = "debian-9" + project = "debian-cloud" } resource "google_project_services" "apis" { project = "${google_project.project.project_id}" + services = [ - "oslogin.googleapis.com", + "oslogin.googleapis.com", "compute.googleapis.com", "cloudkms.googleapis.com", - "appengine.googleapis.com" + "appengine.googleapis.com", ] } resource "google_project_iam_member" "kms-project-binding" { project = "${google_project.project.project_id}" role = "roles/cloudkms.cryptoKeyEncrypterDecrypter" - member = "serviceAccount:service-${google_project.project.number}@compute-system.iam.gserviceaccount.com" - - depends_on = ["google_project_services.apis"] + member = "serviceAccount:service-${google_project.project.number}@compute-system.iam.gserviceaccount.com" + + depends_on = ["google_project_services.apis"] } resource "google_kms_crypto_key_iam_binding" "kms-key-binding" { @@ -768,17 +769,17 @@ resource "google_kms_crypto_key_iam_binding" "kms-key-binding" { members = [ "serviceAccount:service-${google_project.project.number}@compute-system.iam.gserviceaccount.com", - ] + ] - depends_on = ["google_project_services.apis"] + depends_on = ["google_project_services.apis"] } resource "google_kms_key_ring" "my_key_ring" { - name = "%s" - project = "${google_project.project.project_id}" - location = "us-central1" - - depends_on = ["google_project_services.apis"] + name = "%s" + project = "${google_project.project.project_id}" + location = "us-central1" + + depends_on = ["google_project_services.apis"] } resource "google_kms_crypto_key" "my_crypto_key" { @@ -787,22 +788,23 @@ resource "google_kms_crypto_key" "my_crypto_key" { } resource "google_compute_disk" "foobar" { - name = "%s" - image = "${data.google_compute_image.my_image.self_link}" - size = 10 - type = "pd-ssd" - zone = "us-central1-a" - project = "${google_project.project.project_id}" + name = "%s" + image = "${data.google_compute_image.my_image.self_link}" + size = 10 + type = "pd-ssd" + zone = "us-central1-a" + project = "${google_project.project.project_id}" - disk_encryption_key { + disk_encryption_key { kms_key_self_link = "${google_kms_crypto_key.my_crypto_key.self_link}" - } - - depends_on = [ - "google_kms_crypto_key_iam_binding.kms-key-binding", - "google_project_iam_member.kms-project-binding" - ] -}`, pid, pname, org, billing, keyRingName, keyName, diskName) + } + + depends_on = [ + "google_kms_crypto_key_iam_binding.kms-key-binding", + "google_project_iam_member.kms-project-binding", + ] +} +`, pid, pname, org, billing, keyRingName, keyName, diskName) } func testAccComputeDisk_deleteDetach(instanceName, diskName string) string { diff --git a/templates/terraform/decoders/disk.erb b/templates/terraform/decoders/disk.erb index 1693b550fd57..6d0b9ec01bf6 100644 --- a/templates/terraform/decoders/disk.erb +++ b/templates/terraform/decoders/disk.erb @@ -4,9 +4,11 @@ if v, ok := res["diskEncryptionKey"]; ok { // The raw key won't be returned, so we need to use the original. transformed["rawKey"] = d.Get("disk_encryption_key.0.raw_key") transformed["sha256"] = original["sha256"] + <% unless version.nil? || version.name == 'ga' %> // The response for crypto keys often includes the version of the key which needs to be removed // format: projects//locations//keyRings//cryptoKeys//cryptoKeyVersions/1 transformed["kmsKeyName"] = strings.Split(original["kmsKeyName"].(string), "/cryptoKeyVersions")[0] + <% end %> res["diskEncryptionKey"] = transformed } @@ -16,9 +18,11 @@ if v, ok := res["sourceImageEncryptionKey"]; ok { // The raw key won't be returned, so we need to use the original. transformed["rawKey"] = d.Get("source_image_encryption_key.0.raw_key") transformed["sha256"] = original["sha256"] + <% unless version.nil? || version.name == 'ga' %> // The response for crypto keys often includes the version of the key which needs to be removed // format: projects//locations//keyRings//cryptoKeys//cryptoKeyVersions/1 transformed["kmsKeyName"] = strings.Split(original["kmsKeyName"].(string), "/cryptoKeyVersions")[0] + <% end %> res["sourceImageEncryptionKey"] = transformed } @@ -28,9 +32,11 @@ if v, ok := res["sourceSnapshotEncryptionKey"]; ok { // The raw key won't be returned, so we need to use the original. transformed["rawKey"] = d.Get("source_snapshot_encryption_key.0.raw_key") transformed["sha256"] = original["sha256"] + <% unless version.nil? || version.name == 'ga' %> // The response for crypto keys often includes the version of the key which needs to be removed // format: projects//locations//keyRings//cryptoKeys//cryptoKeyVersions/1 transformed["kmsKeyName"] = strings.Split(original["kmsKeyName"].(string), "/cryptoKeyVersions")[0] + <% end %> res["sourceSnapshotEncryptionKey"] = transformed } From a35510256ebb6d73cc300213ec67f45dd19f4475 Mon Sep 17 00:00:00 2001 From: Chris Stephens Date: Fri, 19 Oct 2018 15:12:15 -0700 Subject: [PATCH 07/10] filter beta tests from GA --- provider/terraform/tests/resource_compute_disk_test.go.erb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/provider/terraform/tests/resource_compute_disk_test.go.erb b/provider/terraform/tests/resource_compute_disk_test.go.erb index f322153f9f53..d03cfc85423a 100644 --- a/provider/terraform/tests/resource_compute_disk_test.go.erb +++ b/provider/terraform/tests/resource_compute_disk_test.go.erb @@ -343,6 +343,7 @@ func TestAccComputeDisk_encryption(t *testing.T) { }) } +<% unless version.nil? || version == 'beta' -%> func TestAccComputeDisk_encryptionKMS(t *testing.T) { t.Parallel() @@ -378,6 +379,7 @@ func TestAccComputeDisk_encryptionKMS(t *testing.T) { }, }) } +<% end -%> func TestAccComputeDisk_deleteDetach(t *testing.T) { t.Parallel() @@ -729,7 +731,6 @@ resource "google_compute_disk" "foobar" { }`, diskName) } -// TODO chrisst - exclude this test from the non-beta provider func testAccComputeDisk_encryptionKMS(pid, pname, org, billing, diskName, keyRingName, keyName string) string { return fmt.Sprintf(` resource "google_project" "project" { From a98350a7d0d6f6a69485a00064b9888292334082 Mon Sep 17 00:00:00 2001 From: Chris Stephens Date: Fri, 19 Oct 2018 16:01:28 -0700 Subject: [PATCH 08/10] fix version sniffing --- templates/terraform/decoders/disk.erb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/templates/terraform/decoders/disk.erb b/templates/terraform/decoders/disk.erb index 6d0b9ec01bf6..27e9fc5b79ca 100644 --- a/templates/terraform/decoders/disk.erb +++ b/templates/terraform/decoders/disk.erb @@ -4,7 +4,7 @@ if v, ok := res["diskEncryptionKey"]; ok { // The raw key won't be returned, so we need to use the original. transformed["rawKey"] = d.Get("disk_encryption_key.0.raw_key") transformed["sha256"] = original["sha256"] - <% unless version.nil? || version.name == 'ga' %> + <% if version == 'beta' %> // The response for crypto keys often includes the version of the key which needs to be removed // format: projects//locations//keyRings//cryptoKeys//cryptoKeyVersions/1 transformed["kmsKeyName"] = strings.Split(original["kmsKeyName"].(string), "/cryptoKeyVersions")[0] @@ -18,7 +18,7 @@ if v, ok := res["sourceImageEncryptionKey"]; ok { // The raw key won't be returned, so we need to use the original. transformed["rawKey"] = d.Get("source_image_encryption_key.0.raw_key") transformed["sha256"] = original["sha256"] - <% unless version.nil? || version.name == 'ga' %> + <% if version == 'beta' %> // The response for crypto keys often includes the version of the key which needs to be removed // format: projects//locations//keyRings//cryptoKeys//cryptoKeyVersions/1 transformed["kmsKeyName"] = strings.Split(original["kmsKeyName"].(string), "/cryptoKeyVersions")[0] @@ -32,7 +32,7 @@ if v, ok := res["sourceSnapshotEncryptionKey"]; ok { // The raw key won't be returned, so we need to use the original. transformed["rawKey"] = d.Get("source_snapshot_encryption_key.0.raw_key") transformed["sha256"] = original["sha256"] - <% unless version.nil? || version.name == 'ga' %> + <% if version == 'beta' %> // The response for crypto keys often includes the version of the key which needs to be removed // format: projects//locations//keyRings//cryptoKeys//cryptoKeyVersions/1 transformed["kmsKeyName"] = strings.Split(original["kmsKeyName"].(string), "/cryptoKeyVersions")[0] From d4bc1b41124faf1b1f5a566620bbeeed3ff12235 Mon Sep 17 00:00:00 2001 From: Chris Stephens Date: Fri, 19 Oct 2018 17:33:27 -0700 Subject: [PATCH 09/10] change comparsions to be alpha friendly --- templates/terraform/decoders/disk.erb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/templates/terraform/decoders/disk.erb b/templates/terraform/decoders/disk.erb index 27e9fc5b79ca..8bc1209f49d3 100644 --- a/templates/terraform/decoders/disk.erb +++ b/templates/terraform/decoders/disk.erb @@ -4,7 +4,7 @@ if v, ok := res["diskEncryptionKey"]; ok { // The raw key won't be returned, so we need to use the original. transformed["rawKey"] = d.Get("disk_encryption_key.0.raw_key") transformed["sha256"] = original["sha256"] - <% if version == 'beta' %> + <% unless version.nil? || version == 'ga' %> // The response for crypto keys often includes the version of the key which needs to be removed // format: projects//locations//keyRings//cryptoKeys//cryptoKeyVersions/1 transformed["kmsKeyName"] = strings.Split(original["kmsKeyName"].(string), "/cryptoKeyVersions")[0] @@ -18,7 +18,7 @@ if v, ok := res["sourceImageEncryptionKey"]; ok { // The raw key won't be returned, so we need to use the original. transformed["rawKey"] = d.Get("source_image_encryption_key.0.raw_key") transformed["sha256"] = original["sha256"] - <% if version == 'beta' %> + <% unless version.nil? || version == 'ga' %> // The response for crypto keys often includes the version of the key which needs to be removed // format: projects//locations//keyRings//cryptoKeys//cryptoKeyVersions/1 transformed["kmsKeyName"] = strings.Split(original["kmsKeyName"].(string), "/cryptoKeyVersions")[0] @@ -32,7 +32,7 @@ if v, ok := res["sourceSnapshotEncryptionKey"]; ok { // The raw key won't be returned, so we need to use the original. transformed["rawKey"] = d.Get("source_snapshot_encryption_key.0.raw_key") transformed["sha256"] = original["sha256"] - <% if version == 'beta' %> + <% unless version.nil? || version == 'ga' %> // The response for crypto keys often includes the version of the key which needs to be removed // format: projects//locations//keyRings//cryptoKeys//cryptoKeyVersions/1 transformed["kmsKeyName"] = strings.Split(original["kmsKeyName"].(string), "/cryptoKeyVersions")[0] From 0287d618a8721eb6e07c48a34a954f3e555dfd64 Mon Sep 17 00:00:00 2001 From: Modular Magician Date: Sat, 20 Oct 2018 04:15:13 +0000 Subject: [PATCH 10/10] Update tracked submodules -> HEAD on Sat Oct 20 04:15:13 UTC 2018 Tracked submodules are build/puppet/_bundle build/puppet/auth build/puppet/bigquery build/puppet/compute build/puppet/sql build/puppet/storage build/puppet/spanner build/puppet/container build/puppet/dns build/puppet/pubsub build/puppet/resourcemanager build/chef/_bundle build/chef/auth build/chef/compute build/chef/sql build/chef/storage build/chef/spanner build/chef/container build/chef/dns build/chef/iam build/terraform-beta build/terraform build/ansible build/inspec. --- build/terraform | 2 +- build/terraform-beta | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/build/terraform b/build/terraform index 3ce24ce089e7..afcaa8cb3a8d 160000 --- a/build/terraform +++ b/build/terraform @@ -1 +1 @@ -Subproject commit 3ce24ce089e7c07254c2f56026a0e406b083afbe +Subproject commit afcaa8cb3a8da4191b0aa5fddcfb79150743ce6e diff --git a/build/terraform-beta b/build/terraform-beta index 7fea703fff88..9b068f3c0826 160000 --- a/build/terraform-beta +++ b/build/terraform-beta @@ -1 +1 @@ -Subproject commit 7fea703fff88257cfb144a0a8e97b2fba2bb5828 +Subproject commit 9b068f3c08261c4aec5d3d4d39023faf18963b7a