From cd65c4e97f1625d68cd2db0af1e620478fb63d31 Mon Sep 17 00:00:00 2001 From: Riley Karson Date: Tue, 3 Sep 2019 12:54:58 -0700 Subject: [PATCH 1/4] Add validation for scratch disks --- .../resource_compute_instance_template.go | 23 +++++++- ...resource_compute_instance_template_test.go | 52 +++++++++++++++++++ 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/third_party/terraform/resources/resource_compute_instance_template.go b/third_party/terraform/resources/resource_compute_instance_template.go index aa332f2d9c31..23185a352f35 100644 --- a/third_party/terraform/resources/resource_compute_instance_template.go +++ b/third_party/terraform/resources/resource_compute_instance_template.go @@ -5,9 +5,11 @@ import ( "reflect" "github.com/hashicorp/errwrap" + "github.com/hashicorp/terraform-plugin-sdk/helper/customdiff" "github.com/hashicorp/terraform-plugin-sdk/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/helper/validation" + computeBeta "google.golang.org/api/compute/v0.beta" ) @@ -20,8 +22,11 @@ func resourceComputeInstanceTemplate() *schema.Resource { State: schema.ImportStatePassthrough, }, SchemaVersion: 1, - CustomizeDiff: resourceComputeInstanceTemplateSourceImageCustomizeDiff, - MigrateState: resourceComputeInstanceTemplateMigrateState, + CustomizeDiff: customdiff.All( + resourceComputeInstanceTemplateSourceImageCustomizeDiff, + resourceComputeInstanceTemplateScratchDiskCustomizeDiff, + ), + MigrateState: resourceComputeInstanceTemplateMigrateState, // A compute instance template is more or less a subset of a compute // instance. Please attempt to maintain consistency with the @@ -515,6 +520,20 @@ func resourceComputeInstanceTemplateSourceImageCustomizeDiff(diff *schema.Resour return nil } +func resourceComputeInstanceTemplateScratchDiskCustomizeDiff(diff *schema.ResourceDiff, meta interface{}) error { + numDisks := diff.Get("disk.#").(int) + for i := 0; i < numDisks; i++ { + // misspelled on purpose, type is a special symbol + typee := diff.Get(fmt.Sprintf("disk.%d.type", i)).(string) + diskType := diff.Get(fmt.Sprintf("disk.%d.disk_type", i)).(string) + if typee == "SCRATCH" && diskType != "local-ssd" { + return fmt.Errorf("SCRATCH disks must have a disk_type of local-ssd. disk %d has disk_type %s", i, diskType) + } + } + + return nil +} + func buildDisks(d *schema.ResourceData, config *Config) ([]*computeBeta.AttachedDisk, error) { project, err := getProject(d, config) if err != nil { diff --git a/third_party/terraform/tests/resource_compute_instance_template_test.go b/third_party/terraform/tests/resource_compute_instance_template_test.go index 0e3dae6e0756..a41508322aa1 100644 --- a/third_party/terraform/tests/resource_compute_instance_template_test.go +++ b/third_party/terraform/tests/resource_compute_instance_template_test.go @@ -736,6 +736,21 @@ func TestAccComputeInstanceTemplate_shieldedVmConfig2(t *testing.T) { }) } +func TestAccComputeInstanceTemplate_invalidDiskType(t *testing.T) { + t.Parallel() + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccComputeInstanceTemplate_invalidDiskType(), + ExpectError: regexp.MustCompile("SCRATCH disks must have a disk_type of local-ssd"), + }, + }, + }) +} + func testAccCheckComputeInstanceTemplateDestroy(s *terraform.State) error { config := testAccProvider.Meta().(*Config) @@ -1868,3 +1883,40 @@ resource "google_compute_instance_template" "foobar" { } }`, acctest.RandString(10), enableSecureBoot, enableVtpm, enableIntegrityMonitoring) } + +func testAccComputeInstanceTemplate_invalidDiskType() string { + return fmt.Sprintf(` +data "google_compute_image" "my_image" { + family = "centos-7" + project = "gce-uefi-images" +} + +resource "google_compute_instance_template" "foobar" { + name = "instancet-test-%s" + machine_type = "n1-standard-1" + can_ip_forward = false + + disk { + source_image = "${data.google_compute_image.my_image.self_link}" + auto_delete = true + boot = true + } + + disk { + source_image = "${data.google_compute_image.my_image.self_link}" + auto_delete = true + type = "SCRATCH" + disk_type = "local-ssd" + } + + disk { + source_image = "${data.google_compute_image.my_image.self_link}" + auto_delete = true + type = "SCRATCH" + } + + network_interface { + network = "default" + } +}`, acctest.RandString(10)) +} From 76807f9d4b5953265d37fd8055d67f43c7324472 Mon Sep 17 00:00:00 2001 From: Riley Karson Date: Tue, 3 Sep 2019 14:25:30 -0700 Subject: [PATCH 2/4] Remove source from scratch disk --- .../terraform/tests/resource_compute_instance_template_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/third_party/terraform/tests/resource_compute_instance_template_test.go b/third_party/terraform/tests/resource_compute_instance_template_test.go index a41508322aa1..982fed838d52 100644 --- a/third_party/terraform/tests/resource_compute_instance_template_test.go +++ b/third_party/terraform/tests/resource_compute_instance_template_test.go @@ -1903,7 +1903,6 @@ resource "google_compute_instance_template" "foobar" { } disk { - source_image = "${data.google_compute_image.my_image.self_link}" auto_delete = true type = "SCRATCH" disk_type = "local-ssd" From a1f4784d4b63b0f2581fd2faec40f6eeaaaba2fc Mon Sep 17 00:00:00 2001 From: Riley Karson Date: Mon, 16 Sep 2019 13:49:48 -0700 Subject: [PATCH 3/4] Use hardcoded image --- .../resource_compute_instance_template_test.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/third_party/terraform/tests/resource_compute_instance_template_test.go b/third_party/terraform/tests/resource_compute_instance_template_test.go index 982fed838d52..5b5fd2f58153 100644 --- a/third_party/terraform/tests/resource_compute_instance_template_test.go +++ b/third_party/terraform/tests/resource_compute_instance_template_test.go @@ -1886,10 +1886,12 @@ resource "google_compute_instance_template" "foobar" { func testAccComputeInstanceTemplate_invalidDiskType() string { return fmt.Sprintf(` -data "google_compute_image" "my_image" { - family = "centos-7" - project = "gce-uefi-images" -} +# Use this datasource insead of hardcoded values when https://github.com/hashicorp/terraform/issues/22679 +# is resolved. +# data "google_compute_image" "my_image" { +# family = "centos-7" +# project = "gce-uefi-images" +# } resource "google_compute_instance_template" "foobar" { name = "instancet-test-%s" @@ -1897,7 +1899,7 @@ resource "google_compute_instance_template" "foobar" { can_ip_forward = false disk { - source_image = "${data.google_compute_image.my_image.self_link}" + source_image = "https://www.googleapis.com/compute/v1/projects/gce-uefi-images/global/images/centos-7-v20190729" auto_delete = true boot = true } @@ -1909,7 +1911,7 @@ resource "google_compute_instance_template" "foobar" { } disk { - source_image = "${data.google_compute_image.my_image.self_link}" + source_image = "https://www.googleapis.com/compute/v1/projects/gce-uefi-images/global/images/centos-7-v20190729" auto_delete = true type = "SCRATCH" } From 08e40c14848501b8cb747ce75f3466366a924908 Mon Sep 17 00:00:00 2001 From: Riley Karson Date: Mon, 16 Sep 2019 15:13:56 -0700 Subject: [PATCH 4/4] Add reverse logic --- .../terraform/resources/resource_compute_instance_template.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/third_party/terraform/resources/resource_compute_instance_template.go b/third_party/terraform/resources/resource_compute_instance_template.go index 23185a352f35..a882251209e7 100644 --- a/third_party/terraform/resources/resource_compute_instance_template.go +++ b/third_party/terraform/resources/resource_compute_instance_template.go @@ -529,6 +529,10 @@ func resourceComputeInstanceTemplateScratchDiskCustomizeDiff(diff *schema.Resour if typee == "SCRATCH" && diskType != "local-ssd" { return fmt.Errorf("SCRATCH disks must have a disk_type of local-ssd. disk %d has disk_type %s", i, diskType) } + + if diskType == "local-ssd" && typee != "SCRATCH" { + return fmt.Errorf("disks with a disk_type of local-ssd must be SCRATCH disks. disk %d is a %s disk", i, typee) + } } return nil