Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for creating instances with CMEK #3481

Merged
merged 8 commits into from
May 13, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions google/data_source_google_compute_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
73 changes: 65 additions & 8 deletions google/resource_compute_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package google
import (
"crypto/sha256"
"encoding/base64"
"errors"
"fmt"
"log"
"strings"
Expand Down Expand Up @@ -75,6 +76,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,
Expand Down Expand Up @@ -270,6 +279,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,
Expand Down Expand Up @@ -858,9 +873,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.
Expand Down Expand Up @@ -1366,9 +1391,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
Expand Down Expand Up @@ -1501,8 +1541,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 != "" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, did you run into any cases where d.GetOk returned true, but the value was empty?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I ask because that should be impossible, and so the code could be a tiny bit cleaner without the extra != "" checks)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without the check the API was complaining that I was trying to create a disk with a customer-supplied encryption key without specifying a key.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm that's surprising. I don't think it's worth either of our time to really dig in though since it seems to work just fine with the checks. Thanks!

disk.DiskEncryptionKey = &computeBeta.CustomerEncryptionKey{
RawKey: v.(string),
}
}
}

if v, ok := d.GetOk("boot_disk.0.kms_key_self_link"); ok {
zachberger marked this conversation as resolved.
Show resolved Hide resolved
if v != "" {
disk.DiskEncryptionKey = &computeBeta.CustomerEncryptionKey{
KmsKeyName: v.(string),
}
}
}

Expand Down Expand Up @@ -1575,7 +1625,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}
Expand Down
192 changes: 192 additions & 0 deletions google/resource_compute_instance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
zachberger marked this conversation as resolved.
Show resolved Hide resolved
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{}),
},
zachberger marked this conversation as resolved.
Show resolved Hide resolved
})
}

func TestAccComputeInstance_attachedDisk(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -1363,6 +1400,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 {
Expand Down Expand Up @@ -2021,6 +2105,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" {
Expand Down
13 changes: 11 additions & 2 deletions website/docs/r/compute_instance.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -193,7 +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. Only one of `kms_key_self_link`
and `disk_encryption_key_raw` may be set.

The `network_interface` block supports:

Expand Down