Make the PF provider configuration code process region selflinks in s…

…ame way as the SDK provider (#9063) (#6432) * Add test for `TestGetRegionFromRegionSelfLink` used in SDK provider * Add plugin framework equivalent for `TestGetRegionFromRegionSelfLink` * Update PF provider config so test case passes - behaviour now equivalent to SDK [upstream:ab3cf39d9609ee5e7e60cd6277132d0236fe0066] Signed-off-by: Modular Magician <[email protected]>
hashicorp · Oct 3, 2023 · 0049bc9 · 0049bc9
1 parent 9a15f9a
commit 0049bc9
Show file tree

Hide file tree

Showing 4 changed files with 88 additions and 10 deletions.
diff --git a/.changelog/9063.txt b/.changelog/9063.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+provider: Fixed an issue where the plugin-framework implementation of the provider handled default region values that were self-links differently to the SDK implementation. This issue is not believed to have affected users because of downstream functions that turn self links into region names.
+```
diff --git a/google-beta/fwtransport/framework_config.go b/google-beta/fwtransport/framework_config.go
@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"net/http"
 	"os"
+	"regexp"
 	"strconv"
 	"time"
 
@@ -20,6 +21,7 @@ import (
 	"github.com/hashicorp/go-cleanhttp"
 	"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"
 
@@ -319,7 +321,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
@@ -1823,3 +1825,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
+}
diff --git a/google-beta/fwtransport/framework_config_test.go b/google-beta/fwtransport/framework_config_test.go
@@ -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(),
@@ -1714,3 +1712,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)
+			}
+		})
+	}
+}
diff --git a/google-beta/transport/config_test.go b/google-beta/transport/config_test.go
@@ -702,3 +702,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)
+			}
+		})
+	}
+}