Fix bug with CSEK where the key stored in state might be associated with the wrong disk (#327)

* Fix bug with CSEK where the key stored in state might be associated with the wrong disk

* preserve original order of attached disks

* use the disk index to figure out the raw key

Author: danawillow

google/resource_compute_instance.go

@@ -1,16 +1,19 @@
 package google
 
 import (
+	"crypto/sha256"
+	"encoding/base64"
 	"fmt"
 	"log"
 	"strings"
 
+	"regexp"
+
 	"github.com/hashicorp/terraform/helper/schema"
 	"github.com/hashicorp/terraform/helper/validation"
 	computeBeta "google.golang.org/api/compute/v0.beta"
 	"google.golang.org/api/compute/v1"
 	"google.golang.org/api/googleapi"
-	"regexp"
 )
 
 var InstanceBaseApiVersion = v1
@@ -1083,16 +1086,15 @@ func resourceComputeInstanceRead(d *schema.ResourceData, meta interface{}) error
 		return fmt.Errorf("Expected %d disks, API returned %d", expectedDisks, len(instance.Disks))
 	}
 
-	attachedDiskSources := make(map[string]struct{}, attachedDisksCount)
+	attachedDiskSources := make(map[string]int, attachedDisksCount)
 	for i := 0; i < attachedDisksCount; i++ {
-		attachedDiskSources[d.Get(fmt.Sprintf("attached_disk.%d.source", i)).(string)] = struct{}{}
+		attachedDiskSources[d.Get(fmt.Sprintf("attached_disk.%d.source", i)).(string)] = i
 	}
 
 	dIndex := 0
-	adIndex := 0
 	sIndex := 0
 	disks := make([]map[string]interface{}, 0, disksCount)
-	attachedDisks := make([]map[string]interface{}, 0, attachedDisksCount)
+	attachedDisks := make([]map[string]interface{}, attachedDisksCount)
 	scratchDisks := make([]map[string]interface{}, 0, scratchDisksCount)
 	for _, disk := range instance.Disks {
 		if _, ok := d.GetOk("boot_disk"); ok && disk.Boot {
@@ -1115,24 +1117,29 @@ func resourceComputeInstanceRead(d *schema.ResourceData, meta interface{}) error
 				"device_name":             d.Get(fmt.Sprintf("disk.%d.device_name", dIndex)),
 				"disk_encryption_key_raw": d.Get(fmt.Sprintf("disk.%d.disk_encryption_key_raw", dIndex)),
 			}
-			if disk.DiskEncryptionKey != nil && disk.DiskEncryptionKey.Sha256 != "" {
-				di["disk_encryption_key_sha256"] = disk.DiskEncryptionKey.Sha256
+			if d.Get(fmt.Sprintf("disk.%d.disk_encryption_key_raw", dIndex)) != "" {
+				sha, err := hash256(d.Get(fmt.Sprintf("disk.%d.disk_encryption_key_raw", dIndex)).(string))
+				if err != nil {
+					return err
+				}
+				di["disk_encryption_key_sha256"] = sha
 			}
 			disks = append(disks, di)
 			dIndex++
 		} else {
+			adIndex := attachedDiskSources[disk.Source]
 			di := map[string]interface{}{
-				"source":                  disk.Source,
-				"device_name":             disk.DeviceName,
-				"disk_encryption_key_raw": d.Get(fmt.Sprintf("attached_disk.%d.disk_encryption_key_raw", adIndex)),
+				"source":      disk.Source,
+				"device_name": disk.DeviceName,
 			}
-			if disk.DiskEncryptionKey != nil && disk.DiskEncryptionKey.Sha256 != "" {
-				di["disk_encryption_key_sha256"] = disk.DiskEncryptionKey.Sha256
+			if key := disk.DiskEncryptionKey; key != nil {
+				di["disk_encryption_key_raw"] = d.Get(fmt.Sprintf("attached_disk.%d.disk_encryption_key_raw", adIndex))
+				di["disk_encryption_key_sha256"] = key.Sha256
 			}
-			attachedDisks = append(attachedDisks, di)
-			adIndex++
+			attachedDisks[adIndex] = di
 		}
 	}
+
 	d.Set("disk", disks)
 	d.Set("attached_disk", attachedDisks)
 	d.Set("scratch_disk", scratchDisks)
@@ -1614,6 +1621,15 @@ func getProjectFromSubnetworkLink(subnetwork string) string {
 	return r.FindStringSubmatch(subnetwork)[1]
 }
 
+func hash256(raw string) (string, error) {
+	decoded, err := base64.StdEncoding.DecodeString(raw)
+	if err != nil {
+		return "", err
+	}
+	h := sha256.Sum256(decoded)
+	return base64.StdEncoding.EncodeToString(h[:]), nil
+}
+
 func createAcceleratorPartialUrl(zone, accelerator string) string {
 	return fmt.Sprintf("zones/%s/acceleratorTypes/%s", zone, accelerator)
 }

google/resource_compute_instance_test.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"os"
 	"regexp"
+	"strconv"
 	"strings"
 	"testing"
 
@@ -225,21 +226,34 @@ func TestAccComputeInstance_deprecated_disksWithAutodelete(t *testing.T) {
 func TestAccComputeInstance_diskEncryption(t *testing.T) {
 	var instance compute.Instance
 	var instanceName = fmt.Sprintf("instance-test-%s", acctest.RandString(10))
-	var diskName = fmt.Sprintf("instance-testd-%s", acctest.RandString(10))
+	bootEncryptionKey := "SGVsbG8gZnJvbSBHb29nbGUgQ2xvdWQgUGxhdGZvcm0="
+	bootEncryptionKeyHash := "esTuF7d4eatX4cnc4JsiEiaI+Rff78JgPhA/v1zxX9E="
+	diskNameToEncryptionKey := map[string]*compute.CustomerEncryptionKey{
+		fmt.Sprintf("instance-testd-%s", acctest.RandString(10)): {
+			RawKey: "Ym9vdDU2Nzg5MDEyMzQ1Njc4OTAxMjM0NTY3ODkwMTI=",
+			Sha256: "awJ7p57H+uVZ9axhJjl1D3lfC2MgA/wnt/z88Ltfvss=",
+		},
+		fmt.Sprintf("instance-testd-%s", acctest.RandString(10)): {
+			RawKey: "c2Vjb25kNzg5MDEyMzQ1Njc4OTAxMjM0NTY3ODkwMTI=",
+			Sha256: "7TpIwUdtCOJpq2m+3nt8GFgppu6a2Xsj1t0Gexk13Yc=",
+		},
+		fmt.Sprintf("instance-testd-%s", acctest.RandString(10)): {
+			RawKey: "dGhpcmQ2Nzg5MDEyMzQ1Njc4OTAxMjM0NTY3ODkwMTI=",
+			Sha256: "b3pvaS7BjDbCKeLPPTx7yXBuQtxyMobCHN1QJR43xeM=",
+		},
+	}
 
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
 		Providers:    testAccProviders,
 		CheckDestroy: testAccCheckComputeInstanceDestroy,
 		Steps: []resource.TestStep{
 			resource.TestStep{
-				Config: testAccComputeInstance_disks_encryption(diskName, instanceName),
+				Config: testAccComputeInstance_disks_encryption(bootEncryptionKey, diskNameToEncryptionKey, instanceName),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckComputeInstanceExists(
 						"google_compute_instance.foobar", &instance),
-					testAccCheckComputeInstanceDisk(&instance, instanceName, true, true),
-					testAccCheckComputeInstanceDisk(&instance, diskName, true, false),
-					testAccCheckComputeInstanceDiskEncryptionKey("google_compute_instance.foobar", &instance),
+					testAccCheckComputeInstanceDiskEncryptionKey("google_compute_instance.foobar", &instance, bootEncryptionKeyHash, diskNameToEncryptionKey),
 				),
 			},
 		},
@@ -982,24 +996,66 @@ func testAccCheckComputeInstanceScratchDisk(instance *compute.Instance, interfac
 	}
 }
 
-func testAccCheckComputeInstanceDiskEncryptionKey(n string, instance *compute.Instance) resource.TestCheckFunc {
+func testAccCheckComputeInstanceDiskEncryptionKey(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 {
-			attr := rs.Primary.Attributes[fmt.Sprintf("disk.%d.disk_encryption_key_sha256", i)]
-			if attr == "" && disk.Boot {
-				attr = rs.Primary.Attributes["boot_disk.0.disk_encryption_key_sha256"]
+			if disk.Boot {
+				attr := rs.Primary.Attributes["boot_disk.0.disk_encryption_key_sha256"]
+				if attr == "" {
+					attr = rs.Primary.Attributes[fmt.Sprintf("disk.%d.disk_encryption_key_sha256", i)]
+				}
+				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)
+				}
+				if disk.DiskEncryptionKey != nil && attr != disk.DiskEncryptionKey.Sha256 {
+					return fmt.Errorf("Disk %d has mismatched encryption key.\nTF State: %+v\nGCP State: %+v",
+						i, attr, disk.DiskEncryptionKey.Sha256)
+				}
+			} else {
+				if disk.DiskEncryptionKey != nil {
+					sourceUrl := strings.Split(disk.Source, "/")
+					expectedKey := diskNameToEncryptionKey[sourceUrl[len(sourceUrl)-1]].Sha256
+					if disk.DiskEncryptionKey.Sha256 != expectedKey {
+						return fmt.Errorf("Disk %d has unexpected encryption key in GCP.\nExpected: %s\nActual: %s", i, expectedKey, disk.DiskEncryptionKey.Sha256)
+					}
+				}
 			}
-			if disk.DiskEncryptionKey == nil && attr != "" {
-				return fmt.Errorf("Disk %d has mismatched encryption key.\nTF State: %+v\nGCP State: <empty>", i, attr)
+		}
+
+		numDisks, err := strconv.Atoi(rs.Primary.Attributes["disk.#"])
+		if err != nil {
+			return fmt.Errorf("Error converting value of disk.#")
+		}
+		for i := 0; i < numDisks; i++ {
+			diskName := rs.Primary.Attributes[fmt.Sprintf("disk.%d.disk", i)]
+			encryptionKey := rs.Primary.Attributes[fmt.Sprintf("disk.%d.disk_encryption_key_sha256", i)]
+			expectedEncryptionKey := diskNameToEncryptionKey[diskName].Sha256
+			if encryptionKey != expectedEncryptionKey {
+				return fmt.Errorf("Disk %d has unexpected encryption key in state.\nExpected: %s\nActual: %s", i, expectedEncryptionKey, encryptionKey)
 			}
-			if disk.DiskEncryptionKey != nil && attr != disk.DiskEncryptionKey.Sha256 {
-				return fmt.Errorf("Disk %d has mismatched encryption key.\nTF State: %+v\nGCP State: %+v",
-					i, attr, 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++ {
+			diskSourceUrl := strings.Split(rs.Primary.Attributes[fmt.Sprintf("attached_disk.%d.source", i)], "/")
+			diskName := diskSourceUrl[len(diskSourceUrl)-1]
+			encryptionKey := rs.Primary.Attributes[fmt.Sprintf("attached_disk.%d.disk_encryption_key_sha256", i)]
+			if key, ok := diskNameToEncryptionKey[diskName]; ok {
+				expectedEncryptionKey := key.Sha256
+				if encryptionKey != expectedEncryptionKey {
+					return fmt.Errorf("Attached disk %d has unexpected encryption key in state.\nExpected: %s\nActual: %s", i, expectedEncryptionKey, encryptionKey)
+				}
 			}
 		}
 		return nil
@@ -1452,13 +1508,44 @@ resource "google_compute_instance" "foobar" {
 `, disk, instance, autodelete)
 }
 
-func testAccComputeInstance_disks_encryption(disk, instance string) string {
+func testAccComputeInstance_disks_encryption(bootEncryptionKey string, diskNameToEncryptionKey map[string]*compute.CustomerEncryptionKey, instance string) string {
+	diskNames := []string{}
+	for k, _ := range diskNameToEncryptionKey {
+		diskNames = append(diskNames, k)
+	}
 	return fmt.Sprintf(`
 resource "google_compute_disk" "foobar" {
 	name = "%s"
 	size = 10
 	type = "pd-ssd"
 	zone = "us-central1-a"
+
+	disk_encryption_key_raw = "%s"
+}
+
+resource "google_compute_disk" "foobar2" {
+	name = "%s"
+	size = 10
+	type = "pd-ssd"
+	zone = "us-central1-a"
+
+	disk_encryption_key_raw = "%s"
+}
+
+resource "google_compute_disk" "foobar3" {
+	name = "%s"
+	size = 10
+	type = "pd-ssd"
+	zone = "us-central1-a"
+
+	disk_encryption_key_raw = "%s"
+}
+
+resource "google_compute_disk" "foobar4" {
+	name = "%s"
+	size = 10
+	type = "pd-ssd"
+	zone = "us-central1-a"
 }
 
 resource "google_compute_instance" "foobar" {
@@ -1470,11 +1557,26 @@ resource "google_compute_instance" "foobar" {
 		initialize_params{
 			image = "debian-8-jessie-v20160803"
 		}
-		disk_encryption_key_raw = "SGVsbG8gZnJvbSBHb29nbGUgQ2xvdWQgUGxhdGZvcm0="
+		disk_encryption_key_raw = "%s"
 	}
 
 	disk {
 		disk = "${google_compute_disk.foobar.name}"
+		disk_encryption_key_raw = "%s"
+	}
+
+	attached_disk {
+		source = "${google_compute_disk.foobar2.self_link}"
+		disk_encryption_key_raw = "%s"
+	}
+
+	attached_disk {
+		source = "${google_compute_disk.foobar4.self_link}"
+	}
+
+	attached_disk {
+		source = "${google_compute_disk.foobar3.self_link}"
+		disk_encryption_key_raw = "%s"
 	}
 
 	network_interface {
@@ -1485,7 +1587,12 @@ resource "google_compute_instance" "foobar" {
 		foo = "bar"
 	}
 }
-`, disk, instance)
+`, diskNames[0], diskNameToEncryptionKey[diskNames[0]].RawKey,
+		diskNames[1], diskNameToEncryptionKey[diskNames[1]].RawKey,
+		diskNames[2], diskNameToEncryptionKey[diskNames[2]].RawKey,
+		"instance-testd-"+acctest.RandString(10),
+		instance, bootEncryptionKey,
+		diskNameToEncryptionKey[diskNames[0]].RawKey, diskNameToEncryptionKey[diskNames[1]].RawKey, diskNameToEncryptionKey[diskNames[2]].RawKey)
 }
 
 func testAccComputeInstance_attachedDisk(disk, instance string) string {