fix error with missing zonal value for compute attached disk (#9371) (#16484)

[upstream:4eb33c5ac7d09b08667a54985fdfd015a8ba1b20]

Signed-off-by: Modular Magician <[email protected]>

Author: modular-magician

.changelog/9371.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+compute: fixed issue where `google_compute_attached_disk` would produce an error for certain zone configs
+```

google/services/compute/resource_compute_attached_disk.go

@@ -3,6 +3,7 @@
 package compute
 
 import (
+	"context"
 	"fmt"
 	"log"
 	"strings"
@@ -11,6 +12,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"
@@ -35,7 +37,7 @@ func ResourceComputeAttachedDisk() *schema.Resource {
 
 		CustomizeDiff: customdiff.All(
 			tpgresource.DefaultProviderProject,
-			tpgresource.DefaultProviderZone,
+			computeAttachedDiskDefaultProviderZone,
 		),
 
 		Schema: map[string]*schema.Schema{
@@ -263,3 +265,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
+}

google/services/compute/resource_compute_attached_disk_test.go

@@ -122,6 +122,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
 //

google/tpgresource/field_helpers.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"regexp"
 
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	transport_tpg "github.com/hashicorp/terraform-provider-google/google/transport"
 )
 
@@ -236,6 +237,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 != "" {