From db506cbb30c0c5d950baf98bdd00f0900c275fe7 Mon Sep 17 00:00:00 2001 From: Nick Elliot Date: Thu, 9 Nov 2023 09:27:00 -0800 Subject: [PATCH] fix error with missing zonal value for compute attached disk (#9371) --- .../resource_compute_attached_disk.go.erb | 20 ++++++- .../resource_compute_attached_disk_test.go | 31 ++++++++++ .../terraform/tpgresource/field_helpers.go | 56 +++++++++++++++++++ 3 files changed, 106 insertions(+), 1 deletion(-) diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_attached_disk.go.erb b/mmv1/third_party/terraform/services/compute/resource_compute_attached_disk.go.erb index 9725498a1f7a..c03e4399ac3a 100644 --- a/mmv1/third_party/terraform/services/compute/resource_compute_attached_disk.go.erb +++ b/mmv1/third_party/terraform/services/compute/resource_compute_attached_disk.go.erb @@ -2,6 +2,7 @@ package compute import ( + "context" "fmt" "log" "strings" @@ -10,6 +11,7 @@ import ( "github.com/hashicorp/terraform-provider-google/google/tpgresource" transport_tpg "github.com/hashicorp/terraform-provider-google/google/transport" + "github.com/hashicorp/go-cty/cty" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" @@ -38,7 +40,7 @@ func ResourceComputeAttachedDisk() *schema.Resource { CustomizeDiff: customdiff.All( tpgresource.DefaultProviderProject, - tpgresource.DefaultProviderZone, + computeAttachedDiskDefaultProviderZone, ), Schema: map[string]*schema.Schema{ @@ -266,3 +268,19 @@ func FindDiskByName(disks []*compute.AttachedDisk, id string) *compute.AttachedD return nil } + +func computeAttachedDiskDefaultProviderZone(_ context.Context, diff *schema.ResourceDiff, meta interface{}) error { + if diff.GetRawConfig().GetAttr("instance") == cty.UnknownVal(cty.String) { + return nil + } + config := meta.(*transport_tpg.Config) + zv, err := tpgresource.ParseZonalFieldValueDiff("instances", diff.Get("instance").(string), "project", "zone", diff, config, false) + if err != nil { + return err + } + if err := diff.SetNew("zone", zv.Zone); err != nil { + return fmt.Errorf("Failed to retrieve zone: %s", err) + } + + return nil +} diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_attached_disk_test.go b/mmv1/third_party/terraform/services/compute/resource_compute_attached_disk_test.go index 90b0bbd45f5b..f4ba45548e78 100644 --- a/mmv1/third_party/terraform/services/compute/resource_compute_attached_disk_test.go +++ b/mmv1/third_party/terraform/services/compute/resource_compute_attached_disk_test.go @@ -120,6 +120,37 @@ func TestAccComputeAttachedDisk_count(t *testing.T) { } +func TestAccComputeAttachedDisk_zoneless(t *testing.T) { + t.Setenv("GOOGLE_ZONE", "") + + diskName := fmt.Sprintf("tf-test-disk-%d", acctest.RandInt(t)) + instanceName := fmt.Sprintf("tf-test-inst-%d", acctest.RandInt(t)) + importID := fmt.Sprintf("%s/us-central1-a/%s/%s", envvar.GetTestProjectFromEnv(), instanceName, diskName) + + acctest.VcrTest(t, resource.TestCase{ + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + // Check destroy isn't a good test here, see comment on testCheckAttachedDiskIsNowDetached + CheckDestroy: nil, + Steps: []resource.TestStep{ + { + Config: testAttachedDiskResource(diskName, instanceName) + testAttachedDiskResourceAttachment(), + }, + { + ResourceName: "google_compute_attached_disk.test", + ImportStateId: importID, + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAttachedDiskResource(diskName, instanceName), + Check: resource.ComposeTestCheckFunc( + testCheckAttachedDiskIsNowDetached(t, instanceName, diskName), + ), + }, + }, + }) +} + // testCheckAttachedDiskIsNowDetached queries a compute instance and iterates through the attached // disks to confirm that a specific disk is no longer attached to the instance // diff --git a/mmv1/third_party/terraform/tpgresource/field_helpers.go b/mmv1/third_party/terraform/tpgresource/field_helpers.go index a32e2030a8c0..427e492352cb 100644 --- a/mmv1/third_party/terraform/tpgresource/field_helpers.go +++ b/mmv1/third_party/terraform/tpgresource/field_helpers.go @@ -4,6 +4,7 @@ import ( "fmt" "regexp" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" transport_tpg "github.com/hashicorp/terraform-provider-google/google/transport" ) @@ -234,6 +235,61 @@ func ParseZonalFieldValue(resourceType, fieldValue, projectSchemaField, zoneSche }, nil } +// Parses a zonal field supporting 5 different formats: +// - https://www.googleapis.com/compute/ANY_VERSION/projects/{my_project}/zones/{zone}/{resource_type}/{resource_name} +// - projects/{my_project}/zones/{zone}/{resource_type}/{resource_name} +// - zones/{zone}/{resource_type}/{resource_name} +// - resource_name +// - "" (empty string). RelativeLink() returns empty if isEmptyValid is true. +// +// If the project is not specified, it first tries to get the project from the `projectSchemaField` and then fallback on the default project. +// If the zone is not specified, it takes the value of `zoneSchemaField`. +func ParseZonalFieldValueDiff(resourceType, fieldValue, projectSchemaField, zoneSchemaField string, d *schema.ResourceDiff, config *transport_tpg.Config, isEmptyValid bool) (*ZonalFieldValue, error) { + r := regexp.MustCompile(fmt.Sprintf(ZonalLinkBasePattern, resourceType)) + if parts := r.FindStringSubmatch(fieldValue); parts != nil { + return &ZonalFieldValue{ + Project: parts[1], + Zone: parts[2], + Name: parts[3], + ResourceType: resourceType, + }, nil + } + + project, err := GetProjectFromDiff(d, config) + if err != nil { + return nil, err + } + + r = regexp.MustCompile(fmt.Sprintf(ZonalPartialLinkBasePattern, resourceType)) + if parts := r.FindStringSubmatch(fieldValue); parts != nil { + return &ZonalFieldValue{ + Project: project, + Zone: parts[1], + Name: parts[2], + ResourceType: resourceType, + }, nil + } + + if len(zoneSchemaField) == 0 { + return nil, fmt.Errorf("Invalid field format. Got '%s', expected format '%s'", fieldValue, fmt.Sprintf(GlobalLinkTemplate, "{project}", resourceType, "{name}")) + } + + zone, ok := d.GetOk(zoneSchemaField) + if !ok { + zone = config.Zone + if zone == "" { + return nil, fmt.Errorf("A zone must be specified") + } + } + + return &ZonalFieldValue{ + Project: project, + Zone: zone.(string), + Name: GetResourceNameFromSelfLink(fieldValue), + ResourceType: resourceType, + }, nil +} + func GetProjectFromSchema(projectSchemaField string, d TerraformResourceData, config *transport_tpg.Config) (string, error) { res, ok := d.GetOk(projectSchemaField) if ok && projectSchemaField != "" {