Skip to content

Commit

Permalink
fix error with missing zonal value for compute attached disk (#9371)
Browse files Browse the repository at this point in the history
  • Loading branch information
NickElliot authored Nov 9, 2023
1 parent 6e6e56e commit 4eb33c5
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
package compute

import (
"context"
"fmt"
"log"
"strings"
Expand All @@ -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"
Expand Down Expand Up @@ -38,7 +40,7 @@ func ResourceComputeAttachedDisk() *schema.Resource {

CustomizeDiff: customdiff.All(
tpgresource.DefaultProviderProject,
tpgresource.DefaultProviderZone,
computeAttachedDiskDefaultProviderZone,
),

Schema: map[string]*schema.Schema{
Expand Down Expand Up @@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
//
Expand Down
56 changes: 56 additions & 0 deletions mmv1/third_party/terraform/tpgresource/field_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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 != "" {
Expand Down

0 comments on commit 4eb33c5

Please sign in to comment.