Add support for creating instances with CMEK (#1749)

Merged PR #1749.
GoogleCloudPlatform · May 13, 2019 · d89c256 · d89c256
1 parent 8367233
commit d89c256
Show file tree

Hide file tree

Showing 6 changed files with 271 additions and 12 deletions.
diff --git a/build/terraform b/build/terraform
diff --git a/build/terraform-beta b/build/terraform-beta
diff --git a/third_party/terraform/data_sources/data_source_google_compute_instance.go b/third_party/terraform/data_sources/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/third_party/terraform/resources/resource_compute_instance.go b/third_party/terraform/resources/resource_compute_instance.go
@@ -3,6 +3,7 @@ package google
 import (
 	"crypto/sha256"
 	"encoding/base64"
+	"errors"
 	"fmt"
 	"log"
 	"strings"
@@ -74,6 +75,14 @@ func resourceComputeInstance() *schema.Resource {
 							Computed: true,
 						},
 
+						"kms_key_self_link": {
+							Type:             schema.TypeString,
+							Optional:         true,
+							ForceNew:         true,
+							ConflictsWith:    []string{"boot_disk.0.disk_encryption_key_raw"},
+							DiffSuppressFunc: compareSelfLinkRelativePaths,
+						},
+
 						"initialize_params": {
 							Type:     schema.TypeList,
 							Optional: true,
@@ -269,6 +278,12 @@ func resourceComputeInstance() *schema.Resource {
 							Sensitive: true,
 						},
 
+						"kms_key_self_link": {
+							Type:             schema.TypeString,
+							Optional:         true,
+							DiffSuppressFunc: compareSelfLinkRelativePaths,
+						},
+
 						"disk_encryption_key_sha256": {
 							Type:     schema.TypeString,
 							Computed: true,
@@ -852,9 +867,19 @@ 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 != "" {
+					// The response for crypto keys often includes the version of the key which needs to be removed
+					// format: projects/<project>/locations/<region>/keyRings/<keyring>/cryptoKeys/<key>/cryptoKeyVersions/1
+					di["kms_key_self_link"] = strings.Split(disk.DiskEncryptionKey.KmsKeyName, "/cryptoKeyVersions")[0]
+				}
+				if key.Sha256 != "" {
+					di["disk_encryption_key_sha256"] = key.Sha256
 				}
-				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.
@@ -1378,9 +1403,24 @@ func expandAttachedDisk(diskConfig map[string]interface{}, d *schema.ResourceDat
 		disk.DeviceName = v.(string)
 	}
 
-	if v, ok := diskConfig["disk_encryption_key_raw"]; ok {
-		disk.DiskEncryptionKey = &computeBeta.CustomerEncryptionKey{
-			RawKey: v.(string),
+	keyValue, keyOk := diskConfig["disk_encryption_key_raw"]
+	if keyOk {
+		if keyValue != "" {
+			disk.DiskEncryptionKey = &computeBeta.CustomerEncryptionKey{
+				RawKey: keyValue.(string),
+			}
+		}
+	}
+
+	kmsValue, kmsOk := diskConfig["kms_key_self_link"]
+	if kmsOk {
+		if keyOk && keyValue != "" && kmsValue != "" {
+			return nil, errors.New("Only one of kms_key_self_link and disk_encryption_key_raw can be set")
+		}
+		if kmsValue != "" {
+			disk.DiskEncryptionKey = &computeBeta.CustomerEncryptionKey{
+				KmsKeyName: kmsValue.(string),
+			}
 		}
 	}
 	return disk, nil
@@ -1513,8 +1553,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 {
+		if v != "" {
+			disk.DiskEncryptionKey = &computeBeta.CustomerEncryptionKey{
+				KmsKeyName: v.(string),
+			}
 		}
 	}
 
@@ -1587,7 +1637,14 @@ func flattenBootDisk(d *schema.ResourceData, disk *computeBeta.AttachedDisk, con
 	}
 
 	if disk.DiskEncryptionKey != nil {
-		result["disk_encryption_key_sha256"] = disk.DiskEncryptionKey.Sha256
+		if disk.DiskEncryptionKey.Sha256 != "" {
+			result["disk_encryption_key_sha256"] = disk.DiskEncryptionKey.Sha256
+		}
+		if disk.DiskEncryptionKey.KmsKeyName != "" {
+			// The response for crypto keys often includes the version of the key which needs to be removed
+			// format: projects/<project>/locations/<region>/keyRings/<keyring>/cryptoKeys/<key>/cryptoKeyVersions/1
+			result["kms_key_self_link"] = strings.Split(disk.DiskEncryptionKey.KmsKeyName, "/cryptoKeyVersions")[0]
+		}
 	}
 
 	return []map[string]interface{}{result}

diff --git a/third_party/terraform/tests/resource_compute_instance_test.go b/third_party/terraform/tests/resource_compute_instance_test.go
@@ -282,6 +282,43 @@ 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))
+	kms := BootstrapKMSKey(t)
+
+	bootKmsKeyName := kms.CryptoKey.Name
+	diskNameToEncryptionKey := map[string]*compute.CustomerEncryptionKey{
+		fmt.Sprintf("instance-testd-%s", acctest.RandString(10)): {
+			KmsKeyName: kms.CryptoKey.Name,
+		},
+		fmt.Sprintf("instance-testd-%s", acctest.RandString(10)): {
+			KmsKeyName: kms.CryptoKey.Name,
+		},
+		fmt.Sprintf("instance-testd-%s", acctest.RandString(10)): {
+			KmsKeyName: kms.CryptoKey.Name,
+		},
+	}
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckComputeInstanceDestroy,
+		Steps: []resource.TestStep{
+			{
+				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),
+				),
+			},
+			computeInstanceImportStep("us-central1-a", instanceName, []string{}),
+		},
+	})
+}
+
 func TestAccComputeInstance_attachedDisk(t *testing.T) {
 	t.Parallel()
 
@@ -1395,6 +1432,53 @@ 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: <empty>", i, attr)
+				}
+			} else {
+				if disk.DiskEncryptionKey != nil {
+					expectedKey := diskNameToEncryptionKey[GetResourceNameFromSelfLink(disk.Source)].KmsKeyName
+					// The response for crypto keys often includes the version of the key which needs to be removed
+					// format: projects/<project>/locations/<region>/keyRings/<keyring>/cryptoKeys/<key>/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)
+					}
+				}
+			}
+		}
+
+		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 {
@@ -2053,6 +2137,114 @@ resource "google_compute_instance" "foobar" {
 		diskNameToEncryptionKey[diskNames[0]].RawKey, diskNameToEncryptionKey[diskNames[1]].RawKey, diskNameToEncryptionKey[diskNames[2]].RawKey)
 }
 
+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
+	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"
+	}
+}
+`, pid, 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/third_party/terraform/website/docs/r/compute_instance.html.markdown b/third_party/terraform/website/docs/r/compute_instance.html.markdown
@@ -147,7 +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. 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.
@@ -194,7 +199,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. Only one of `kms_key_self_link`
+    and `disk_encryption_key_raw` may be set.
 
 The `network_interface` block supports: