Skip to content
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.

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

Make the PF provider configuration code process region selflinks in same way as the SDK provider #9063

Merged
merged 4 commits into from
Oct 3, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
17 changes: 16 additions & 1 deletion mmv1/third_party/terraform/fwtransport/framework_config.go.erb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"net/http"
"os"
"regexp"
"strconv"
"time"

Expand All @@ -20,6 +21,7 @@ import (
"github.com/hashicorp/terraform-plugin-framework/attr"
"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/hashicorp/terraform-plugin-framework/types/basetypes"
"github.com/hashicorp/terraform-plugin-log/tflog"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/logging"

Expand Down Expand Up @@ -106,7 +108,7 @@ func (p *FrameworkProviderConfig) LoadAndValidateFramework(ctx context.Context,
p.Context = ctx
p.BillingProject = data.BillingProject
p.Project = data.Project
p.Region = data.Region
p.Region = GetRegionFromRegionSelfLink(data.Region)
p.Scopes = data.Scopes
p.Zone = data.Zone
p.UserProjectOverride = data.UserProjectOverride
Expand Down Expand Up @@ -781,3 +783,16 @@ func GetBatchingConfig(ctx context.Context, data types.List, diags *diag.Diagnos

return bc
}

func GetRegionFromRegionSelfLink(selfLink basetypes.StringValue) basetypes.StringValue {
re := regexp.MustCompile("/compute/[a-zA-Z0-9]*/projects/[a-zA-Z0-9-]*/regions/([a-zA-Z0-9-]*)")
value := selfLink.String()
switch {
case re.MatchString(value):
if res := re.FindStringSubmatch(value); len(res) == 2 && res[1] != "" {
region := res[1]
return types.StringValue(region)
}
}
return selfLink
}
Original file line number Diff line number Diff line change
Expand Up @@ -523,15 +523,13 @@ func TestFrameworkProvider_LoadAndValidateFramework_region(t *testing.T) {
ExpectedDataModelValue: types.StringValue("region-from-config"),
ExpectedConfigStructValue: types.StringValue("region-from-config"),
},
// This test currently fails - PF code doesn't behave like SDK code
// TODO(SarahFrench) - address https://github.com/hashicorp/terraform-provider-google/issues/15714
// "region values can be supplied as a self link": {
// ConfigValues: fwmodels.ProviderModel{
// Region: types.StringValue("https://www.googleapis.com/compute/v1/projects/my-project/regions/us-central1"),
// },
// ExpectedDataModelValue: types.StringValue("https://www.googleapis.com/compute/v1/projects/my-project/regions/us-central1"),
// ExpectedConfigStructValue: types.StringValue("us-central1"),
// },
"region values can be supplied as a self link": {
ConfigValues: fwmodels.ProviderModel{
Region: types.StringValue("https://www.googleapis.com/compute/v1/projects/my-project/regions/us-central1"),
},
ExpectedDataModelValue: types.StringValue("https://www.googleapis.com/compute/v1/projects/my-project/regions/us-central1"),
ExpectedConfigStructValue: types.StringValue("us-central1"),
},
"region value can be set by environment variable: GOOGLE_REGION is used": {
ConfigValues: fwmodels.ProviderModel{
Region: types.StringNull(),
Expand Down Expand Up @@ -1719,3 +1717,34 @@ func TestFrameworkProvider_LoadAndValidateFramework_batching(t *testing.T) {
}
}


func TestGetRegionFromRegionSelfLink(t *testing.T) {
cases := map[string]struct {
Input basetypes.StringValue
ExpectedOutput basetypes.StringValue
}{
"A short region name is returned unchanged": {
Input: types.StringValue("us-central1"),
ExpectedOutput: types.StringValue("us-central1"),
},
"A selflink is shortened to a region name": {
Input: types.StringValue("https://www.googleapis.com/compute/v1/projects/my-project/regions/us-central1"),
ExpectedOutput: types.StringValue("us-central1"),
},
"Logic is specific to region selflinks; zone selflinks are not shortened": {
Input: types.StringValue("https://www.googleapis.com/compute/v1/projects/my-project/zones/asia-east1-a"),
ExpectedOutput: types.StringValue("https://www.googleapis.com/compute/v1/projects/my-project/zones/asia-east1-a"),
},
}

for tn, tc := range cases {
t.Run(tn, func(t *testing.T) {

region := fwtransport.GetRegionFromRegionSelfLink(tc.Input)

if region != tc.ExpectedOutput {
t.Fatalf("want %s, got %s", region, tc.ExpectedOutput)
}
})
}
}
31 changes: 31 additions & 0 deletions mmv1/third_party/terraform/transport/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -700,3 +700,34 @@ func TestRemoveBasePathVersion(t *testing.T) {
}
}
}

func TestGetRegionFromRegionSelfLink(t *testing.T) {
cases := map[string]struct {
Input string
ExpectedOutput string
}{
"A short region name is returned unchanged": {
Input: "us-central1",
ExpectedOutput: "us-central1",
},
"A selflink is shortened to a region name": {
Input: "https://www.googleapis.com/compute/v1/projects/my-project/regions/us-central1",
ExpectedOutput: "us-central1",
},
"Logic is specific to region selflinks; zone selflinks are not shortened": {
Input: "https://www.googleapis.com/compute/v1/projects/my-project/zones/asia-east1-a",
ExpectedOutput: "https://www.googleapis.com/compute/v1/projects/my-project/zones/asia-east1-a",
},
}

for tn, tc := range cases {
t.Run(tn, func(t *testing.T) {

region := transport_tpg.GetRegionFromRegionSelfLink(tc.Input)

if region != tc.ExpectedOutput {
t.Fatalf("want %s, got %s", region, tc.ExpectedOutput)
}
})
}
}