From 3b53e237d77e599071a8d568ec20154713b8ac7f Mon Sep 17 00:00:00 2001 From: Zach Berger Date: Tue, 23 Apr 2019 10:47:33 -0700 Subject: [PATCH 1/8] Add support for creating instances with CMEK --- google/data_source_google_compute_instance.go | 1 + google/resource_compute_instance.go | 31 ++++++++ google/resource_compute_instance_test.go | 78 +++++++++++++++++++ 3 files changed, 110 insertions(+) diff --git a/google/data_source_google_compute_instance.go b/google/data_source_google_compute_instance.go index 0caf5bcf5a6..f2f72811575 100644 --- a/google/data_source_google_compute_instance.go +++ b/google/data_source_google_compute_instance.go @@ -101,6 +101,7 @@ func dataSourceGoogleComputeInstanceRead(d *schema.ResourceData, meta interface{ } if key := disk.DiskEncryptionKey; key != nil { di["disk_encryption_key_sha256"] = key.Sha256 + di["kms_key_self_link"] = key.KmsKeyName } attachedDisks = append(attachedDisks, di) } diff --git a/google/resource_compute_instance.go b/google/resource_compute_instance.go index ff3fb62af45..edf7d7cf0ab 100644 --- a/google/resource_compute_instance.go +++ b/google/resource_compute_instance.go @@ -75,6 +75,12 @@ func resourceComputeInstance() *schema.Resource { Computed: true, }, + "kms_key_self_link": { + Type: schema.TypeString, + Optional: true, + ForceNew: true, + }, + "initialize_params": { Type: schema.TypeList, Optional: true, @@ -270,6 +276,11 @@ func resourceComputeInstance() *schema.Resource { Sensitive: true, }, + "kms_key_self_link": { + Type: schema.TypeString, + Optional: true, + }, + "disk_encryption_key_sha256": { Type: schema.TypeString, Computed: true, @@ -355,6 +366,12 @@ func resourceComputeInstance() *schema.Resource { Sensitive: true, }, + "kms_key_self_link": { + Type: schema.TypeString, + Optional: true, + ForceNew: true, + }, + "disk_encryption_key_sha256": { Type: schema.TypeString, Computed: true, @@ -860,6 +877,7 @@ func resourceComputeInstanceRead(d *schema.ResourceData, meta interface{}) error if inConfig { di["disk_encryption_key_raw"] = d.Get(fmt.Sprintf("attached_disk.%d.disk_encryption_key_raw", adIndex)) } + di["kms_key_self_link"] = key.KmsKeyName di["disk_encryption_key_sha256"] = key.Sha256 } // We want the disks to remain in the order we set in the config, so if a disk @@ -1371,6 +1389,12 @@ func expandAttachedDisk(diskConfig map[string]interface{}, d *schema.ResourceDat RawKey: v.(string), } } + + if v, ok := diskConfig["kms_key_self_link"]; ok { + disk.DiskEncryptionKey = &computeBeta.CustomerEncryptionKey{ + KmsKeyName: v.(string), + } + } return disk, nil } @@ -1506,6 +1530,12 @@ func expandBootDisk(d *schema.ResourceData, config *Config, zone *compute.Zone, } } + if v, ok := d.GetOk("boot_disk.0.kms_key_self_link"); ok { + disk.DiskEncryptionKey = &computeBeta.CustomerEncryptionKey{ + KmsKeyName: v.(string), + } + } + if v, ok := d.GetOk("boot_disk.0.source"); ok { source, err := ParseDiskFieldValue(v.(string), d, config) if err != nil { @@ -1576,6 +1606,7 @@ func flattenBootDisk(d *schema.ResourceData, disk *computeBeta.AttachedDisk, con if disk.DiskEncryptionKey != nil { result["disk_encryption_key_sha256"] = disk.DiskEncryptionKey.Sha256 + result["kms_key_self_link"] = disk.DiskEncryptionKey.KmsKeyName } return []map[string]interface{}{result} diff --git a/google/resource_compute_instance_test.go b/google/resource_compute_instance_test.go index 827ebd24f4f..c76706d3644 100644 --- a/google/resource_compute_instance_test.go +++ b/google/resource_compute_instance_test.go @@ -282,6 +282,40 @@ func TestAccComputeInstance_diskEncryption(t *testing.T) { }) } +func TestAccComputeInstance_kmsDiskEncryption(t *testing.T) { + t.Parallel() + + var instance compute.Instance + var instanceName = fmt.Sprintf("instance-test-%s", acctest.RandString(10)) + bootKmsKeyName := "projects/project/locations/us-central1/keyRings/testing-cloud-kms/cryptoKeys/key-0/cryptoKeyVersions/1" + diskNameToEncryptionKey := map[string]*compute.CustomerEncryptionKey{ + fmt.Sprintf("instance-testd-%s", acctest.RandString(10)): { + KmsKeyName: "projects/project/locations/us-central1/keyRings/testing-cloud-kms/cryptoKeys/key-1/cryptoKeyVersions/1", + }, + fmt.Sprintf("instance-testd-%s", acctest.RandString(10)): { + KmsKeyName: "projects/project/locations/us-central1/keyRings/testing-cloud-kms/cryptoKeys/key-2/cryptoKeyVersions/1", + }, + fmt.Sprintf("instance-testd-%s", acctest.RandString(10)): { + KmsKeyName: "projects/project/locations/us-central1/keyRings/testing-cloud-kms/cryptoKeys/key-3/cryptoKeyVersions/1", + }, + } + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckComputeInstanceDestroy, + Steps: []resource.TestStep{ + { + Config: testAccComputeInstance_disks_encryption(bootKmsKeyName, diskNameToEncryptionKey, instanceName), + Check: resource.ComposeTestCheckFunc( + testAccCheckComputeInstanceExists("google_compute_instance.foobar", &instance), + testAccCheckComputeInstanceDiskKmsEncryptionKey("google_compute_instance.foobar", &instance, bootKmsKeyName, diskNameToEncryptionKey), + ), + }, + }, + }) +} + func TestAccComputeInstance_attachedDisk(t *testing.T) { t.Parallel() @@ -1363,6 +1397,50 @@ func testAccCheckComputeInstanceDiskEncryptionKey(n string, instance *compute.In } } +func testAccCheckComputeInstanceDiskKmsEncryptionKey(n string, instance *compute.Instance, bootDiskEncryptionKey string, diskNameToEncryptionKey map[string]*compute.CustomerEncryptionKey) resource.TestCheckFunc { + return func(s *terraform.State) error { + rs, ok := s.RootModule().Resources[n] + if !ok { + return fmt.Errorf("Not found: %s", n) + } + + for i, disk := range instance.Disks { + if disk.Boot { + attr := rs.Primary.Attributes["boot_disk.0.kms_key_self_link"] + if attr != bootDiskEncryptionKey { + return fmt.Errorf("Boot disk has wrong encryption key in state.\nExpected: %s\nActual: %s", bootDiskEncryptionKey, attr) + } + if disk.DiskEncryptionKey == nil && attr != "" { + return fmt.Errorf("Disk %d has mismatched encryption key.\nTF State: %+v\nGCP State: ", i, attr) + } + } else { + if disk.DiskEncryptionKey != nil { + expectedKey := diskNameToEncryptionKey[GetResourceNameFromSelfLink(disk.Source)].KmsKeyName + if disk.DiskEncryptionKey.KmsKeyName != expectedKey { + return fmt.Errorf("Disk %d has unexpected encryption key in GCP.\nExpected: %s\nActual: %s", i, expectedKey, disk.DiskEncryptionKey.Sha256) + } + } + } + } + + numAttachedDisks, err := strconv.Atoi(rs.Primary.Attributes["attached_disk.#"]) + if err != nil { + return fmt.Errorf("Error converting value of attached_disk.#") + } + for i := 0; i < numAttachedDisks; i++ { + diskName := GetResourceNameFromSelfLink(rs.Primary.Attributes[fmt.Sprintf("attached_disk.%d.source", i)]) + kmsKeyName := rs.Primary.Attributes[fmt.Sprintf("attached_disk.%d.kms_key_self_link", i)] + if key, ok := diskNameToEncryptionKey[diskName]; ok { + expectedEncryptionKey := key.KmsKeyName + if kmsKeyName != expectedEncryptionKey { + return fmt.Errorf("Attached disk %d has unexpected encryption key in state.\nExpected: %s\nActual: %s", i, expectedEncryptionKey, kmsKeyName) + } + } + } + return nil + } +} + func testAccCheckComputeInstanceTag(instance *compute.Instance, n string) resource.TestCheckFunc { return func(s *terraform.State) error { if instance.Tags == nil { From 8b71442231bad2e2c3ff8dfdde35b5084a4e2f78 Mon Sep 17 00:00:00 2001 From: Zach Berger Date: Tue, 23 Apr 2019 11:38:15 -0700 Subject: [PATCH 2/8] Add documentation for specifying KMS key. --- website/docs/r/compute_instance.html.markdown | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/website/docs/r/compute_instance.html.markdown b/website/docs/r/compute_instance.html.markdown index 45c1f35ecdb..25fb7cd718c 100644 --- a/website/docs/r/compute_instance.html.markdown +++ b/website/docs/r/compute_instance.html.markdown @@ -149,6 +149,9 @@ The `boot_disk` block supports: encoded in [RFC 4648 base64](https://tools.ietf.org/html/rfc4648#section-4) to encrypt this disk. +* `kms_key_self_link` - (Optional) The self_link of the encryption key that is + stored in Google Cloud KMS to encrypt this disk. + * `initialize_params` - (Optional) Parameters for a new disk that will be created alongside the new instance. Either `initialize_params` or `source` must be set. Structure is documented below. @@ -195,6 +198,9 @@ The `attached_disk` block supports: encoded in [RFC 4648 base64](https://tools.ietf.org/html/rfc4648#section-4) to encrypt this disk. +* `kms_key_self_link` - (Optional) The self_link of the encryption key that is + stored in Google Cloud KMS to encrypt this disk. + The `network_interface` block supports: * `network` - (Optional) The name or self_link of the network to attach this interface to. From c22ebe679cc74d197afb3bedc018cf51a6a202a0 Mon Sep 17 00:00:00 2001 From: Zach Berger Date: Fri, 3 May 2019 15:20:26 -0700 Subject: [PATCH 3/8] Address review comments. --- google/resource_compute_instance.go | 25 ++--- google/resource_compute_instance_test.go | 101 +++++++++++++++++- website/docs/r/compute_instance.html.markdown | 11 +- 3 files changed, 120 insertions(+), 17 deletions(-) diff --git a/google/resource_compute_instance.go b/google/resource_compute_instance.go index edf7d7cf0ab..ca66d137c9b 100644 --- a/google/resource_compute_instance.go +++ b/google/resource_compute_instance.go @@ -3,6 +3,7 @@ package google import ( "crypto/sha256" "encoding/base64" + "errors" "fmt" "log" "strings" @@ -277,8 +278,9 @@ func resourceComputeInstance() *schema.Resource { }, "kms_key_self_link": { - Type: schema.TypeString, - Optional: true, + Type: schema.TypeString, + Optional: true, + ConflictsWith: []string{"boot_disk.0.disk_encryption_key_raw"}, }, "disk_encryption_key_sha256": { @@ -366,12 +368,6 @@ func resourceComputeInstance() *schema.Resource { Sensitive: true, }, - "kms_key_self_link": { - Type: schema.TypeString, - Optional: true, - ForceNew: true, - }, - "disk_encryption_key_sha256": { Type: schema.TypeString, Computed: true, @@ -1384,15 +1380,20 @@ func expandAttachedDisk(diskConfig map[string]interface{}, d *schema.ResourceDat disk.DeviceName = v.(string) } - if v, ok := diskConfig["disk_encryption_key_raw"]; ok { + keyValue, keyOk := diskConfig["disk_encryption_key_raw"] + if keyOk { disk.DiskEncryptionKey = &computeBeta.CustomerEncryptionKey{ - RawKey: v.(string), + RawKey: keyValue.(string), } } - if v, ok := diskConfig["kms_key_self_link"]; ok { + kmsValue, kmsOk := diskConfig["kms_key_self_link"] + if kmsOk { + if keyOk { + return nil, errors.New("Only one of kms_key_self_link and disk_encryption_key_raw can be set") + } disk.DiskEncryptionKey = &computeBeta.CustomerEncryptionKey{ - KmsKeyName: v.(string), + KmsKeyName: kmsValue.(string), } } return disk, nil diff --git a/google/resource_compute_instance_test.go b/google/resource_compute_instance_test.go index c76706d3644..3699d191fcf 100644 --- a/google/resource_compute_instance_test.go +++ b/google/resource_compute_instance_test.go @@ -306,12 +306,13 @@ func TestAccComputeInstance_kmsDiskEncryption(t *testing.T) { CheckDestroy: testAccCheckComputeInstanceDestroy, Steps: []resource.TestStep{ { - Config: testAccComputeInstance_disks_encryption(bootKmsKeyName, diskNameToEncryptionKey, instanceName), + Config: testAccComputeInstance_disks_kms(bootKmsKeyName, diskNameToEncryptionKey, instanceName), Check: resource.ComposeTestCheckFunc( testAccCheckComputeInstanceExists("google_compute_instance.foobar", &instance), testAccCheckComputeInstanceDiskKmsEncryptionKey("google_compute_instance.foobar", &instance, bootKmsKeyName, diskNameToEncryptionKey), ), }, + computeInstanceImportStep("us-central1-a", instanceName, []string{}), }, }) } @@ -2099,6 +2100,104 @@ resource "google_compute_instance" "foobar" { diskNameToEncryptionKey[diskNames[0]].RawKey, diskNameToEncryptionKey[diskNames[1]].RawKey, diskNameToEncryptionKey[diskNames[2]].RawKey) } +func testAccComputeInstance_disks_kms(bootEncryptionKey string, diskNameToEncryptionKey map[string]*compute.CustomerEncryptionKey, instance string) string { + diskNames := []string{} + for k := range diskNameToEncryptionKey { + diskNames = append(diskNames, k) + } + return fmt.Sprintf(` +data "google_compute_image" "my_image" { + family = "debian-9" + project = "debian-cloud" +} + +resource "google_compute_disk" "foobar" { + name = "%s" + size = 10 + type = "pd-ssd" + zone = "us-central1-a" + + disk_encryption_key { + kms_key_self_link = "%s" + } +} + +resource "google_compute_disk" "foobar2" { + name = "%s" + size = 10 + type = "pd-ssd" + zone = "us-central1-a" + + disk_encryption_key { + kms_key_self_link = "%s" + } +} + +resource "google_compute_disk" "foobar3" { + name = "%s" + size = 10 + type = "pd-ssd" + zone = "us-central1-a" + + disk_encryption_key { + kms_key_self_link = "%s" + } +} + +resource "google_compute_disk" "foobar4" { + name = "%s" + size = 10 + type = "pd-ssd" + zone = "us-central1-a" +} + +resource "google_compute_instance" "foobar" { + name = "%s" + machine_type = "n1-standard-1" + zone = "us-central1-a" + + boot_disk { + initialize_params{ + image = "${data.google_compute_image.my_image.self_link}" + } + kms_key_self_link = "%s" + } + + attached_disk { + source = "${google_compute_disk.foobar.self_link}" + kms_key_self_link = "%s" + } + + attached_disk { + source = "${google_compute_disk.foobar2.self_link}" + kms_key_self_link = "%s" + } + + attached_disk { + source = "${google_compute_disk.foobar4.self_link}" + } + + attached_disk { + source = "${google_compute_disk.foobar3.self_link}" + kms_key_self_link = "%s" + } + + network_interface { + network = "default" + } + + metadata = { + foo = "bar" + } +} +`, diskNames[0], diskNameToEncryptionKey[diskNames[0]].KmsKeyName, + diskNames[1], diskNameToEncryptionKey[diskNames[1]].KmsKeyName, + diskNames[2], diskNameToEncryptionKey[diskNames[2]].KmsKeyName, + "instance-testd-"+acctest.RandString(10), + instance, bootEncryptionKey, + diskNameToEncryptionKey[diskNames[0]].KmsKeyName, diskNameToEncryptionKey[diskNames[1]].KmsKeyName, diskNameToEncryptionKey[diskNames[2]].KmsKeyName) +} + func testAccComputeInstance_attachedDisk(disk, instance string) string { return fmt.Sprintf(` data "google_compute_image" "my_image" { diff --git a/website/docs/r/compute_instance.html.markdown b/website/docs/r/compute_instance.html.markdown index 25fb7cd718c..89ac76a292e 100644 --- a/website/docs/r/compute_instance.html.markdown +++ b/website/docs/r/compute_instance.html.markdown @@ -147,10 +147,12 @@ The `boot_disk` block supports: * `disk_encryption_key_raw` - (Optional) A 256-bit [customer-supplied encryption key] (https://cloud.google.com/compute/docs/disks/customer-supplied-encryption), encoded in [RFC 4648 base64](https://tools.ietf.org/html/rfc4648#section-4) - to encrypt this disk. + to encrypt this disk. Only one of `kms_key_self_link` and `disk_encryption_key_raw` + may be set. * `kms_key_self_link` - (Optional) The self_link of the encryption key that is - stored in Google Cloud KMS to encrypt this disk. + stored in Google Cloud KMS to encrypt this disk. Only one of `kms_key_self_link` + and `disk_encryption_key_raw` may be set. * `initialize_params` - (Optional) Parameters for a new disk that will be created alongside the new instance. Either `initialize_params` or `source` must be set. @@ -196,10 +198,11 @@ The `attached_disk` block supports: * `disk_encryption_key_raw` - (Optional) A 256-bit [customer-supplied encryption key] (https://cloud.google.com/compute/docs/disks/customer-supplied-encryption), encoded in [RFC 4648 base64](https://tools.ietf.org/html/rfc4648#section-4) - to encrypt this disk. + to encrypt this disk. Only one of `kms_key_self_link` and `disk_encryption_key_raw` may be set. * `kms_key_self_link` - (Optional) The self_link of the encryption key that is - stored in Google Cloud KMS to encrypt this disk. + stored in Google Cloud KMS to encrypt this disk. Only one of `kms_key_self_link` + and `disk_encryption_key_raw` may be set. The `network_interface` block supports: From d889dacf5a4c18f24c30f82a15e54bb0daa923e7 Mon Sep 17 00:00:00 2001 From: Zach Berger Date: Wed, 8 May 2019 16:01:12 -0700 Subject: [PATCH 4/8] Fix ConflictsWith for kms_key_self_link --- google/resource_compute_instance.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/google/resource_compute_instance.go b/google/resource_compute_instance.go index ca66d137c9b..1836d70ffd5 100644 --- a/google/resource_compute_instance.go +++ b/google/resource_compute_instance.go @@ -77,9 +77,10 @@ func resourceComputeInstance() *schema.Resource { }, "kms_key_self_link": { - Type: schema.TypeString, - Optional: true, - ForceNew: true, + Type: schema.TypeString, + Optional: true, + ForceNew: true, + ConflictsWith: []string{"boot_disk.0.disk_encryption_key_raw"}, }, "initialize_params": { @@ -278,9 +279,8 @@ func resourceComputeInstance() *schema.Resource { }, "kms_key_self_link": { - Type: schema.TypeString, - Optional: true, - ConflictsWith: []string{"boot_disk.0.disk_encryption_key_raw"}, + Type: schema.TypeString, + Optional: true, }, "disk_encryption_key_sha256": { From 819550057e0c1d60f56055e3d8df594797706aa2 Mon Sep 17 00:00:00 2001 From: Zach Berger Date: Wed, 8 May 2019 16:07:13 -0700 Subject: [PATCH 5/8] Guard on disk values being set --- google/resource_compute_instance.go | 47 ++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 14 deletions(-) diff --git a/google/resource_compute_instance.go b/google/resource_compute_instance.go index 1836d70ffd5..2e73edfcf58 100644 --- a/google/resource_compute_instance.go +++ b/google/resource_compute_instance.go @@ -871,10 +871,17 @@ func resourceComputeInstanceRead(d *schema.ResourceData, meta interface{}) error } if key := disk.DiskEncryptionKey; key != nil { if inConfig { - di["disk_encryption_key_raw"] = d.Get(fmt.Sprintf("attached_disk.%d.disk_encryption_key_raw", adIndex)) + rawKey := d.Get(fmt.Sprintf("attached_disk.%d.disk_encryption_key_raw", adIndex)) + if rawKey != "" { + di["disk_encryption_key_raw"] = rawKey + } + } + if key.KmsKeyName != "" { + di["kms_key_self_link"] = key.KmsKeyName + } + if key.Sha256 != "" { + di["disk_encryption_key_sha256"] = key.Sha256 } - di["kms_key_self_link"] = key.KmsKeyName - di["disk_encryption_key_sha256"] = key.Sha256 } // We want the disks to remain in the order we set in the config, so if a disk // is present in the config, make sure it's at the correct index. Otherwise, append it. @@ -1382,18 +1389,22 @@ func expandAttachedDisk(diskConfig map[string]interface{}, d *schema.ResourceDat keyValue, keyOk := diskConfig["disk_encryption_key_raw"] if keyOk { - disk.DiskEncryptionKey = &computeBeta.CustomerEncryptionKey{ - RawKey: keyValue.(string), + if keyValue != "" { + disk.DiskEncryptionKey = &computeBeta.CustomerEncryptionKey{ + RawKey: keyValue.(string), + } } } kmsValue, kmsOk := diskConfig["kms_key_self_link"] if kmsOk { - if keyOk { + if keyOk && keyValue != "" && kmsValue != "" { return nil, errors.New("Only one of kms_key_self_link and disk_encryption_key_raw can be set") } - disk.DiskEncryptionKey = &computeBeta.CustomerEncryptionKey{ - KmsKeyName: kmsValue.(string), + if kmsValue != "" { + disk.DiskEncryptionKey = &computeBeta.CustomerEncryptionKey{ + KmsKeyName: kmsValue.(string), + } } } return disk, nil @@ -1526,14 +1537,18 @@ func expandBootDisk(d *schema.ResourceData, config *Config, zone *compute.Zone, } if v, ok := d.GetOk("boot_disk.0.disk_encryption_key_raw"); ok { - disk.DiskEncryptionKey = &computeBeta.CustomerEncryptionKey{ - RawKey: v.(string), + if v != "" { + disk.DiskEncryptionKey = &computeBeta.CustomerEncryptionKey{ + RawKey: v.(string), + } } } if v, ok := d.GetOk("boot_disk.0.kms_key_self_link"); ok { - disk.DiskEncryptionKey = &computeBeta.CustomerEncryptionKey{ - KmsKeyName: v.(string), + if v != "" { + disk.DiskEncryptionKey = &computeBeta.CustomerEncryptionKey{ + KmsKeyName: v.(string), + } } } @@ -1606,8 +1621,12 @@ func flattenBootDisk(d *schema.ResourceData, disk *computeBeta.AttachedDisk, con } if disk.DiskEncryptionKey != nil { - result["disk_encryption_key_sha256"] = disk.DiskEncryptionKey.Sha256 - result["kms_key_self_link"] = disk.DiskEncryptionKey.KmsKeyName + if disk.DiskEncryptionKey.Sha256 != "" { + result["disk_encryption_key_sha256"] = disk.DiskEncryptionKey.Sha256 + } + if disk.DiskEncryptionKey.KmsKeyName != "" { + result["kms_key_self_link"] = disk.DiskEncryptionKey.KmsKeyName + } } return []map[string]interface{}{result} From 38fb5cf9b1dce9ba41d4b2e41d72702cb61ae8a4 Mon Sep 17 00:00:00 2001 From: Zach Berger Date: Thu, 9 May 2019 10:22:02 -0700 Subject: [PATCH 6/8] Address KmsKey self link issues --- google/resource_compute_instance.go | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/google/resource_compute_instance.go b/google/resource_compute_instance.go index 2e73edfcf58..01eaaafc1e0 100644 --- a/google/resource_compute_instance.go +++ b/google/resource_compute_instance.go @@ -77,10 +77,11 @@ func resourceComputeInstance() *schema.Resource { }, "kms_key_self_link": { - Type: schema.TypeString, - Optional: true, - ForceNew: true, - ConflictsWith: []string{"boot_disk.0.disk_encryption_key_raw"}, + Type: schema.TypeString, + Optional: true, + ForceNew: true, + ConflictsWith: []string{"boot_disk.0.disk_encryption_key_raw"}, + DiffSuppressFunc: compareSelfLinkRelativePaths, }, "initialize_params": { @@ -279,8 +280,9 @@ func resourceComputeInstance() *schema.Resource { }, "kms_key_self_link": { - Type: schema.TypeString, - Optional: true, + Type: schema.TypeString, + Optional: true, + DiffSuppressFunc: compareSelfLinkRelativePaths, }, "disk_encryption_key_sha256": { @@ -877,7 +879,9 @@ func resourceComputeInstanceRead(d *schema.ResourceData, meta interface{}) error } } if key.KmsKeyName != "" { - di["kms_key_self_link"] = key.KmsKeyName + // The response for crypto keys often includes the version of the key which needs to be removed + // format: projects//locations//keyRings//cryptoKeys//cryptoKeyVersions/1 + di["kms_key_self_link"] = strings.Split(disk.DiskEncryptionKey.KmsKeyName, "/cryptoKeyVersions")[0] } if key.Sha256 != "" { di["disk_encryption_key_sha256"] = key.Sha256 @@ -1625,7 +1629,9 @@ func flattenBootDisk(d *schema.ResourceData, disk *computeBeta.AttachedDisk, con result["disk_encryption_key_sha256"] = disk.DiskEncryptionKey.Sha256 } if disk.DiskEncryptionKey.KmsKeyName != "" { - result["kms_key_self_link"] = disk.DiskEncryptionKey.KmsKeyName + // The response for crypto keys often includes the version of the key which needs to be removed + // format: projects//locations//keyRings//cryptoKeys//cryptoKeyVersions/1 + result["kms_key_self_link"] = strings.Split(disk.DiskEncryptionKey.KmsKeyName, "/cryptoKeyVersions")[0] } } From e5295071461c3cf32a70f6be465d12b25ee5faf1 Mon Sep 17 00:00:00 2001 From: Zach Berger Date: Thu, 9 May 2019 10:22:13 -0700 Subject: [PATCH 7/8] Create resources needed for tests. --- google/resource_compute_instance_test.go | 26 +++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/google/resource_compute_instance_test.go b/google/resource_compute_instance_test.go index 3699d191fcf..43cb434e582 100644 --- a/google/resource_compute_instance_test.go +++ b/google/resource_compute_instance_test.go @@ -287,16 +287,18 @@ func TestAccComputeInstance_kmsDiskEncryption(t *testing.T) { var instance compute.Instance var instanceName = fmt.Sprintf("instance-test-%s", acctest.RandString(10)) - bootKmsKeyName := "projects/project/locations/us-central1/keyRings/testing-cloud-kms/cryptoKeys/key-0/cryptoKeyVersions/1" + kms := BootstrapKMSKey(t) + + bootKmsKeyName := kms.CryptoKey.Name diskNameToEncryptionKey := map[string]*compute.CustomerEncryptionKey{ fmt.Sprintf("instance-testd-%s", acctest.RandString(10)): { - KmsKeyName: "projects/project/locations/us-central1/keyRings/testing-cloud-kms/cryptoKeys/key-1/cryptoKeyVersions/1", + KmsKeyName: kms.CryptoKey.Name, }, fmt.Sprintf("instance-testd-%s", acctest.RandString(10)): { - KmsKeyName: "projects/project/locations/us-central1/keyRings/testing-cloud-kms/cryptoKeys/key-2/cryptoKeyVersions/1", + KmsKeyName: kms.CryptoKey.Name, }, fmt.Sprintf("instance-testd-%s", acctest.RandString(10)): { - KmsKeyName: "projects/project/locations/us-central1/keyRings/testing-cloud-kms/cryptoKeys/key-3/cryptoKeyVersions/1", + KmsKeyName: kms.CryptoKey.Name, }, } @@ -306,7 +308,7 @@ func TestAccComputeInstance_kmsDiskEncryption(t *testing.T) { CheckDestroy: testAccCheckComputeInstanceDestroy, Steps: []resource.TestStep{ { - Config: testAccComputeInstance_disks_kms(bootKmsKeyName, diskNameToEncryptionKey, instanceName), + Config: testAccComputeInstance_disks_kms(getTestProjectFromEnv(), bootKmsKeyName, diskNameToEncryptionKey, instanceName), Check: resource.ComposeTestCheckFunc( testAccCheckComputeInstanceExists("google_compute_instance.foobar", &instance), testAccCheckComputeInstanceDiskKmsEncryptionKey("google_compute_instance.foobar", &instance, bootKmsKeyName, diskNameToEncryptionKey), @@ -2100,17 +2102,27 @@ resource "google_compute_instance" "foobar" { diskNameToEncryptionKey[diskNames[0]].RawKey, diskNameToEncryptionKey[diskNames[1]].RawKey, diskNameToEncryptionKey[diskNames[2]].RawKey) } -func testAccComputeInstance_disks_kms(bootEncryptionKey string, diskNameToEncryptionKey map[string]*compute.CustomerEncryptionKey, instance string) string { +func testAccComputeInstance_disks_kms(pid string, bootEncryptionKey string, diskNameToEncryptionKey map[string]*compute.CustomerEncryptionKey, instance string) string { diskNames := []string{} for k := range diskNameToEncryptionKey { diskNames = append(diskNames, k) } return fmt.Sprintf(` +data "google_project" "project" { + project_id = "%s" +} + data "google_compute_image" "my_image" { family = "debian-9" project = "debian-cloud" } +resource "google_project_iam_member" "kms-project-binding" { + project = "${data.google_project.project.project_id}" + role = "roles/cloudkms.cryptoKeyEncrypterDecrypter" + member = "serviceAccount:service-${data.google_project.project.number}@compute-system.iam.gserviceaccount.com" +} + resource "google_compute_disk" "foobar" { name = "%s" size = 10 @@ -2190,7 +2202,7 @@ resource "google_compute_instance" "foobar" { foo = "bar" } } -`, diskNames[0], diskNameToEncryptionKey[diskNames[0]].KmsKeyName, +`, pid, diskNames[0], diskNameToEncryptionKey[diskNames[0]].KmsKeyName, diskNames[1], diskNameToEncryptionKey[diskNames[1]].KmsKeyName, diskNames[2], diskNameToEncryptionKey[diskNames[2]].KmsKeyName, "instance-testd-"+acctest.RandString(10), From c20dd54d1b46e4ec40a357c02e71684a53ce3cf7 Mon Sep 17 00:00:00 2001 From: Zach Berger Date: Thu, 9 May 2019 11:27:55 -0700 Subject: [PATCH 8/8] Fix test comparison --- google/resource_compute_instance_test.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/google/resource_compute_instance_test.go b/google/resource_compute_instance_test.go index 43cb434e582..60bdbef0fb6 100644 --- a/google/resource_compute_instance_test.go +++ b/google/resource_compute_instance_test.go @@ -1419,8 +1419,11 @@ func testAccCheckComputeInstanceDiskKmsEncryptionKey(n string, instance *compute } else { if disk.DiskEncryptionKey != nil { expectedKey := diskNameToEncryptionKey[GetResourceNameFromSelfLink(disk.Source)].KmsKeyName - if disk.DiskEncryptionKey.KmsKeyName != expectedKey { - return fmt.Errorf("Disk %d has unexpected encryption key in GCP.\nExpected: %s\nActual: %s", i, expectedKey, disk.DiskEncryptionKey.Sha256) + // The response for crypto keys often includes the version of the key which needs to be removed + // format: projects//locations//keyRings//cryptoKeys//cryptoKeyVersions/1 + actualKey := strings.Split(disk.DiskEncryptionKey.KmsKeyName, "/cryptoKeyVersions")[0] + if actualKey != expectedKey { + return fmt.Errorf("Disk %d has unexpected encryption key in GCP.\nExpected: %s\nActual: %s", i, expectedKey, actualKey) } } }