From da3a6f13a135e984cbee685ac7a7688733960019 Mon Sep 17 00:00:00 2001 From: Vincent Roseberry Date: Tue, 9 Jan 2018 13:57:02 -0800 Subject: [PATCH] Infers region from zone before using the provider-level region (#938) --- google/field_helpers.go | 31 +++++++++++++++++++++---------- google/field_helpers_test.go | 28 +++++++++++++++++++++++++--- google/utils.go | 22 ++++++---------------- 3 files changed, 52 insertions(+), 29 deletions(-) diff --git a/google/field_helpers.go b/google/field_helpers.go index 55a59688..834cd9bc 100644 --- a/google/field_helpers.go +++ b/google/field_helpers.go @@ -27,11 +27,11 @@ func ParseNetworkFieldValue(network string, d TerraformResourceData, config *Con } func ParseSubnetworkFieldValue(subnetwork string, d TerraformResourceData, config *Config) (*RegionalFieldValue, error) { - return parseRegionalFieldValue("subnetworks", subnetwork, "project", "region", d, config, true) + return parseRegionalFieldValue("subnetworks", subnetwork, "project", "region", "zone", d, config, true) } func ParseSubnetworkFieldValueWithProjectField(subnetwork, projectField string, d TerraformResourceData, config *Config) (*RegionalFieldValue, error) { - return parseRegionalFieldValue("subnetworks", subnetwork, projectField, "region", d, config, true) + return parseRegionalFieldValue("subnetworks", subnetwork, projectField, "region", "zone", d, config, true) } func ParseSslCertificateFieldValue(sslCertificate string, d TerraformResourceData, config *Config) (*GlobalFieldValue, error) { @@ -260,8 +260,8 @@ func (f RegionalFieldValue) RelativeLink() string { // - "" (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 region is not specified, it first tries to get the region from the `regionSchemaField` and then fallback on the default region. -func parseRegionalFieldValue(resourceType, fieldValue, projectSchemaField, regionSchemaField string, d TerraformResourceData, config *Config, isEmptyValid bool) (*RegionalFieldValue, error) { +// If the region is not specified, see function documentation for `getRegionFromSchema`. +func parseRegionalFieldValue(resourceType, fieldValue, projectSchemaField, regionSchemaField, zoneSchemaField string, d TerraformResourceData, config *Config, isEmptyValid bool) (*RegionalFieldValue, error) { if len(fieldValue) == 0 { if isEmptyValid { return &RegionalFieldValue{resourceType: resourceType}, nil @@ -294,7 +294,7 @@ func parseRegionalFieldValue(resourceType, fieldValue, projectSchemaField, regio }, nil } - region, err := getRegionFromSchema(regionSchemaField, d, config) + region, err := getRegionFromSchema(regionSchemaField, zoneSchemaField, d, config) if err != nil { return nil, err } @@ -307,13 +307,24 @@ func parseRegionalFieldValue(resourceType, fieldValue, projectSchemaField, regio }, nil } -func getRegionFromSchema(regionSchemaField string, d TerraformResourceData, config *Config) (string, error) { - res, ok := d.GetOk(regionSchemaField) - if ok && regionSchemaField != "" { - return res.(string), nil +// Infers the region based on the following (in order of priority): +// - `regionSchemaField` in resource schema +// - region extracted from the `zoneSchemaField` in resource schema +// - provider-level region +// - region extracted from the provider-level zone +func getRegionFromSchema(regionSchemaField, zoneSchemaField string, d TerraformResourceData, config *Config) (string, error) { + if v, ok := d.GetOk(regionSchemaField); ok && regionSchemaField != "" { + return v.(string), nil + } + if v, ok := d.GetOk(zoneSchemaField); ok && zoneSchemaField != "" { + return getRegionFromZone(v.(string)), nil } if config.Region != "" { return config.Region, nil } - return "", fmt.Errorf("%s: required field is not set", regionSchemaField) + if config.Zone != "" { + return getRegionFromZone(config.Zone), nil + } + + return "", fmt.Errorf("Cannot determine region: set in this resource, or set provider-level 'region' or 'zone'.") } diff --git a/google/field_helpers_test.go b/google/field_helpers_test.go index 33fd76a2..49847f77 100644 --- a/google/field_helpers_test.go +++ b/google/field_helpers_test.go @@ -239,6 +239,8 @@ func TestParseRegionalFieldValue(t *testing.T) { ProjectSchemaValue string RegionSchemaField string RegionSchemaValue string + ZoneSchemaField string + ZoneSchemaValue string Config *Config }{ "subnetwork is a full self link": { @@ -278,6 +280,23 @@ func TestParseRegionalFieldValue(t *testing.T) { Config: &Config{Project: "default-project", Region: "default-region"}, ExpectedRelativeLink: "projects/default-project/regions/us-east1/subnetworks/my-subnetwork", }, + "subnetwork is the name only and region is extracted from the one field.": { + FieldValue: "my-subnetwork", + ProjectSchemaValue: "schema-project", + RegionSchemaField: "region", + ZoneSchemaField: "zone", + ZoneSchemaValue: "us-central1-a", + Config: &Config{Project: "default-project", Region: "default-region"}, + ExpectedRelativeLink: "projects/default-project/regions/us-central1/subnetworks/my-subnetwork", + }, + "subnetwork is the name only and region is extracted from the provider-level zone.": { + FieldValue: "my-subnetwork", + ProjectSchemaValue: "schema-project", + RegionSchemaField: "region", + ZoneSchemaField: "zone", + Config: &Config{Project: "default-project", Zone: "us-central1-c"}, + ExpectedRelativeLink: "projects/default-project/regions/us-central1/subnetworks/my-subnetwork", + }, "subnetwork is the name only and no region field is specified": { FieldValue: "my-subnetwork", Config: &Config{Project: "default-project", Region: "default-region"}, @@ -305,19 +324,22 @@ func TestParseRegionalFieldValue(t *testing.T) { t.Run(tn, func(t *testing.T) { fieldsInSchema := make(map[string]interface{}) - if len(tc.ProjectSchemaValue) > 0 && len(tc.ProjectSchemaField) > 0 { + if tc.ProjectSchemaValue != "" && tc.ProjectSchemaField != "" { fieldsInSchema[tc.ProjectSchemaField] = tc.ProjectSchemaValue } - if len(tc.RegionSchemaValue) > 0 && len(tc.RegionSchemaField) > 0 { + if tc.RegionSchemaValue != "" && tc.RegionSchemaField != "" { fieldsInSchema[tc.RegionSchemaField] = tc.RegionSchemaValue } + if tc.ZoneSchemaValue != "" && tc.ZoneSchemaField != "" { + fieldsInSchema[tc.ZoneSchemaField] = tc.ZoneSchemaValue + } d := &ResourceDataMock{ FieldsInSchema: fieldsInSchema, } - v, err := parseRegionalFieldValue(resourceType, tc.FieldValue, tc.ProjectSchemaField, tc.RegionSchemaField, d, tc.Config, tc.IsEmptyValid) + v, err := parseRegionalFieldValue(resourceType, tc.FieldValue, tc.ProjectSchemaField, tc.RegionSchemaField, tc.ZoneSchemaField, d, tc.Config, tc.IsEmptyValid) if err != nil { if !tc.ExpectedError { diff --git a/google/utils.go b/google/utils.go index 6ba8cc08..11a5371c 100644 --- a/google/utils.go +++ b/google/utils.go @@ -26,23 +26,13 @@ func getRegionFromZone(zone string) string { return "" } -// getRegion reads the "region" field from the given resource data and falls -// back to the provider's value if not given. If the provider's value is not -// given, checks for "zone" in either the given resource data or provider, -// and extracts region from zone. If "zone" is not provided, returns an -// error. +// Infers the region based on the following (in order of priority): +// - `region` field in resource schema +// - region extracted from the `zone` field in resource schema +// - provider-level region +// - region extracted from the provider-level zone func getRegion(d *schema.ResourceData, config *Config) (string, error) { - res, ok := d.GetOk("region") - if !ok { - if config.Region != "" { - return config.Region, nil - } - if zone, err := getZone(d, config); err == nil && getRegionFromZone(zone) != "" { - return getRegionFromZone(zone), nil - } - return "", fmt.Errorf("Cannot determine region: set in this resource, or set provider-level 'region' or 'zone'.") - } - return res.(string), nil + return getRegionFromSchema("region", "zone", d, config) } // getZone reads the "zone" value from the given resource data and falls back