Skip to content

Unignore empty values location logic 5.0.0 #9015

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
SarahFrench wants to merge 2 commits into from
Closed

Unignore empty values location logic 5.0.0 #9015

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
721795c
Stop ignoring empty strings when determining location/region/zone val…
SarahFrench Sep 19, 2023
58286c5
Update PF-specific location logic tests to reflect how `""` isn't ign…
SarahFrench Sep 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions mmv1/third_party/terraform/fwresource/framework_location.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,23 +31,23 @@ type LocationDescription struct {

func (ld *LocationDescription) GetLocation() (types.String, error) {
// Location from resource config
if !ld.ResourceLocation.IsNull() && !ld.ResourceLocation.IsUnknown() && !ld.ResourceLocation.Equal(types.StringValue("")) {
if !ld.ResourceLocation.IsNull() && !ld.ResourceLocation.IsUnknown() {
return ld.ResourceLocation, nil
}

// Location from region in resource config
if !ld.ResourceRegion.IsNull() && !ld.ResourceRegion.IsUnknown() && !ld.ResourceRegion.Equal(types.StringValue("")) {
if !ld.ResourceRegion.IsNull() && !ld.ResourceRegion.IsUnknown() {
return ld.ResourceRegion, nil
}

// Location from zone in resource config
if !ld.ResourceZone.IsNull() && !ld.ResourceZone.IsUnknown() && !ld.ResourceZone.Equal(types.StringValue("")) {
if !ld.ResourceZone.IsNull() && !ld.ResourceZone.IsUnknown() {
location := tpgresource.GetResourceNameFromSelfLink(ld.ResourceZone.ValueString()) // Zone could be a self link
return types.StringValue(location), nil
}

// Location from zone in provider config
if !ld.ProviderZone.IsNull() && !ld.ProviderZone.IsUnknown() && !ld.ProviderZone.Equal(types.StringValue("")) {
if !ld.ProviderZone.IsNull() && !ld.ProviderZone.IsUnknown() {
return ld.ProviderZone, nil
}

Expand All @@ -65,21 +65,21 @@ func (ld *LocationDescription) GetRegion() (types.String, error) {
// For all checks in this function body

// Region from resource config
if !ld.ResourceRegion.IsNull() && !ld.ResourceRegion.IsUnknown() && !ld.ResourceRegion.Equal(types.StringValue("")) {
if !ld.ResourceRegion.IsNull() && !ld.ResourceRegion.IsUnknown() {
region := tpgresource.GetResourceNameFromSelfLink(ld.ResourceRegion.ValueString()) // Region could be a self link
return types.StringValue(region), nil
}
// Region from zone in resource config
if !ld.ResourceZone.IsNull() && !ld.ResourceZone.IsUnknown() && !ld.ResourceZone.Equal(types.StringValue("")) {
if !ld.ResourceZone.IsNull() && !ld.ResourceZone.IsUnknown() {
region := tpgresource.GetRegionFromZone(ld.ResourceZone.ValueString())
return types.StringValue(region), nil
}
// Region from provider config
if !ld.ProviderRegion.IsNull() && !ld.ProviderRegion.IsUnknown() && !ld.ProviderRegion.Equal(types.StringValue("")) {
if !ld.ProviderRegion.IsNull() && !ld.ProviderRegion.IsUnknown() {
return ld.ProviderRegion, nil
}
// Region from zone in provider config
if !ld.ProviderZone.IsNull() && !ld.ProviderZone.IsUnknown() && !ld.ProviderZone.Equal(types.StringValue("")) {
if !ld.ProviderZone.IsNull() && !ld.ProviderZone.IsUnknown() {
region := tpgresource.GetRegionFromZone(ld.ProviderZone.ValueString())
return types.StringValue(region), nil
}
Expand All @@ -97,12 +97,12 @@ func (ld *LocationDescription) GetZone() (types.String, error) {
// TODO(SarahFrench): Make empty strings not ignored, see https://github.com/hashicorp/terraform-provider-google/issues/14447
// For all checks in this function body

if !ld.ResourceZone.IsNull() && !ld.ResourceZone.IsUnknown() && !ld.ResourceZone.Equal(types.StringValue("")) {
if !ld.ResourceZone.IsNull() && !ld.ResourceZone.IsUnknown() {
// Zone could be a self link
zone := tpgresource.GetResourceNameFromSelfLink(ld.ResourceZone.ValueString())
return types.StringValue(zone), nil
}
if !ld.ProviderZone.IsNull() && !ld.ProviderZone.IsUnknown() && !ld.ProviderZone.Equal(types.StringValue("")) {
if !ld.ProviderZone.IsNull() && !ld.ProviderZone.IsUnknown() {
return ld.ProviderZone, nil
}

Expand Down
73 changes: 25 additions & 48 deletions mmv1/third_party/terraform/fwresource/framework_location_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,19 @@ func TestLocationDescription_GetZone(t *testing.T) {
ExpectedZone: types.StringValue("provider-zone-a"),
},
// Handling of empty strings
"returns the value of the zone field in provider config when zone is set to an empty string in resource config": {
"returns an empty string when zone is set to empty string in a resource config, instead of falling back to other values": {
ld: LocationDescription{
ResourceZone: types.StringValue(""),
ProviderZone: types.StringValue("provider-zone-a"),
},
ExpectedZone: types.StringValue("provider-zone-a"),
ExpectedZone: types.StringValue(""),
},
"returns an empty string if zone set as empty strings in both resource and provider configs": {
ld: LocationDescription{
ResourceZone: types.StringValue(""),
ProviderZone: types.StringValue(""),
},
ExpectedZone: types.StringValue(""),
},
// Error states
"returns an error when a zone value can't be found": {
Expand All @@ -55,14 +62,7 @@ func TestLocationDescription_GetZone(t *testing.T) {
},
ExpectedError: true,
},
"returns an error if zone is set as an empty string in both resource and provider configs": {
ld: LocationDescription{
ResourceZone: types.StringValue(""),
ProviderZone: types.StringValue(""),
},
ExpectedError: true,
},
"returns an error that mention non-standard schema field names when a zone value can't be found": {
"returns an error that mentions non-standard schema field names when a zone value can't be found": {
ld: LocationDescription{
ZoneSchemaField: types.StringValue("foobar"),
},
Expand Down Expand Up @@ -146,20 +146,23 @@ func TestLocationDescription_GetRegion(t *testing.T) {
ExpectedRegion: types.StringValue("provider-zone"), // is truncated
},
// Handling of empty strings
"returns a region derived from the zone field in resource config when region is set as an empty string": {
"returns an empty string when region is set to empty string in a resource config, instead of falling back to other values": {
ld: LocationDescription{
ResourceRegion: types.StringValue(""),
ResourceZone: types.StringValue("provider-zone-a"),
ResourceZone: types.StringValue("resource-zone-a"),
ProviderRegion: types.StringValue("provider-region"),
ProviderZone: types.StringValue("provider-zone-a"),
},
ExpectedRegion: types.StringValue("provider-zone"), // is truncated
ExpectedRegion: types.StringValue(""),
},
"returns the value of the region field in provider config when region/zone set as an empty string in resource config": {
"returns an empty string if region and zone set as empty strings in both resource and provider configs": {
ld: LocationDescription{
ResourceRegion: types.StringValue(""),
ResourceZone: types.StringValue(""),
ProviderRegion: types.StringValue("provider-region"),
ProviderRegion: types.StringValue(""),
ProviderZone: types.StringValue(""),
},
ExpectedRegion: types.StringValue("provider-region"),
ExpectedRegion: types.StringValue(""),
},
// Error states
"returns an error when region/zone values can't be found (location is ignored)": {
Expand All @@ -168,15 +171,6 @@ func TestLocationDescription_GetRegion(t *testing.T) {
},
ExpectedError: true,
},
"returns an error if region and zone set as empty strings in both resource and provider configs": {
ld: LocationDescription{
ResourceRegion: types.StringValue(""),
ResourceZone: types.StringValue(""),
ProviderRegion: types.StringValue(""),
ProviderZone: types.StringValue(""),
},
ExpectedError: true,
},
"returns an error that mention non-standard schema field names when region value can't be found": {
ld: LocationDescription{
RegionSchemaField: types.StringValue("foobar"),
Expand Down Expand Up @@ -273,29 +267,22 @@ func TestLocationDescription_GetLocation(t *testing.T) {
ExpectedLocation: types.StringValue("https://www.googleapis.com/compute/v1/projects/my-project/zones/provider-zone-a"),
},
// Handling of empty strings
"returns the region value set in the resource config when location is an empty string": {
"returns an empty string when location is set to empty string in a resource config, instead of falling back to other values": {
ld: LocationDescription{
ResourceLocation: types.StringValue(""),
ResourceRegion: types.StringValue("resource-region"),
},
ExpectedLocation: types.StringValue("resource-region"),
},
"returns the zone value set in the resource config when both location or region are empty strings": {
ld: LocationDescription{
ResourceLocation: types.StringValue(""),
ResourceRegion: types.StringValue(""),
ResourceZone: types.StringValue("resource-zone-a"),
},
ExpectedLocation: types.StringValue("resource-zone-a"),
ExpectedLocation: types.StringValue(""),
},
"returns the zone value from the provider config when all of location/region/zone are set as empty strings in the resource config": {
"returns an empty string if location/region/zone set as empty strings in both resource and provider configs": {
ld: LocationDescription{
ResourceLocation: types.StringValue(""),
ResourceRegion: types.StringValue(""),
ResourceZone: types.StringValue(""),
ProviderZone: types.StringValue("provider-zone-a"),
ProviderRegion: types.StringValue(""),
ProviderZone: types.StringValue(""),
},
ExpectedLocation: types.StringValue("provider-zone-a"),
ExpectedLocation: types.StringValue(""),
},
// Error states
"does not use the region value set in the provider config": {
Expand All @@ -307,16 +294,6 @@ func TestLocationDescription_GetLocation(t *testing.T) {
"returns an error when none of location/region/zone are set on the resource, and neither region or zone is set on the provider": {
ExpectedError: true,
},
"returns an error if location/region/zone are set as empty strings in both resource and provider configs": {
ld: LocationDescription{
ResourceLocation: types.StringValue(""),
ResourceRegion: types.StringValue(""),
ResourceZone: types.StringValue(""),
ProviderRegion: types.StringValue(""),
ProviderZone: types.StringValue(""),
},
ExpectedError: true,
},
"returns an error that mention non-standard schema field names when location value can't be found": {
ld: LocationDescription{
LocationSchemaField: types.StringValue("foobar"),
Expand Down