From 890855bb8c655ea02e0014a68f0f83c69d6d3152 Mon Sep 17 00:00:00 2001 From: himanikh Date: Thu, 26 Dec 2024 15:59:10 -0800 Subject: [PATCH] suppressing Project id vs project number diff in forwardingRule.target (#12606) --- mmv1/products/compute/ForwardingRule.yaml | 2 +- .../tpgresource/common_diff_suppress.go.tmpl | 18 +++++- .../tpgresource/common_diff_suppress_test.go | 47 ++++++++++++++ .../tpgresource/self_link_helpers.go | 19 +++++- .../tpgresource/self_link_helpers_test.go | 64 +++++++++++++++++++ 5 files changed, 145 insertions(+), 5 deletions(-) diff --git a/mmv1/products/compute/ForwardingRule.yaml b/mmv1/products/compute/ForwardingRule.yaml index ec3add76aa4a..9d5f5e0325e9 100644 --- a/mmv1/products/compute/ForwardingRule.yaml +++ b/mmv1/products/compute/ForwardingRule.yaml @@ -485,7 +485,7 @@ properties: For Private Service Connect forwarding rules that forward traffic to managed services, the target must be a service attachment. update_url: 'projects/{{project}}/regions/{{region}}/forwardingRules/{{name}}/setTarget' update_verb: 'POST' - diff_suppress_func: 'tpgresource.CompareSelfLinkRelativePaths' + diff_suppress_func: 'tpgresource.CompareSelfLinkRelativePathsIgnoreProjectId' custom_expand: 'templates/terraform/custom_expand/self_link_from_name.tmpl' - name: 'labelFingerprint' type: Fingerprint diff --git a/mmv1/third_party/terraform/tpgresource/common_diff_suppress.go.tmpl b/mmv1/third_party/terraform/tpgresource/common_diff_suppress.go.tmpl index 70763fdd58da..d7ed7e3926d2 100644 --- a/mmv1/third_party/terraform/tpgresource/common_diff_suppress.go.tmpl +++ b/mmv1/third_party/terraform/tpgresource/common_diff_suppress.go.tmpl @@ -93,11 +93,23 @@ func DurationDiffSuppress(k, old, new string, d *schema.ResourceData) bool { // has the project number instead of the project name func ProjectNumberDiffSuppress(_, old, new string, _ *schema.ResourceData) bool { var a2, b2 string - reN := regexp.MustCompile("projects/\\d+") + re := regexp.MustCompile("projects/\\d+") + reN := regexp.MustCompile("projects/[^/]+") + replacement := []byte("projects/equal") + a2 = string(re.ReplaceAll([]byte(old), replacement)) + b2 = string(reN.ReplaceAll([]byte(new), replacement)) + return a2 == b2 +} + +// Suppress diffs when the value read from api +// has the project ID instead of the project number +func ProjectIDDiffSuppress(_, old, new string, _ *schema.ResourceData) bool { + var a2, b2 string re := regexp.MustCompile("projects/[^/]+") + reN := regexp.MustCompile("projects/\\d+") replacement := []byte("projects/equal") - a2 = string(reN.ReplaceAll([]byte(old), replacement)) - b2 = string(re.ReplaceAll([]byte(new), replacement)) + a2 = string(re.ReplaceAll([]byte(old), replacement)) + b2 = string(reN.ReplaceAll([]byte(new), replacement)) return a2 == b2 } diff --git a/mmv1/third_party/terraform/tpgresource/common_diff_suppress_test.go b/mmv1/third_party/terraform/tpgresource/common_diff_suppress_test.go index 3cb2ef2d21c6..90be0b54264f 100644 --- a/mmv1/third_party/terraform/tpgresource/common_diff_suppress_test.go +++ b/mmv1/third_party/terraform/tpgresource/common_diff_suppress_test.go @@ -67,6 +67,53 @@ func TestDurationDiffSuppress(t *testing.T) { } } +func TestProjectNumberDiffSuppress(t *testing.T) { + cases := map[string]struct { + Old, New string + ExpectDiffSuppress bool + }{ + "different project identifiers": { + Old: "projects/1234/locations/abc/serviceAttachments/xyz", + New: "projects/ten-tp/locations/abc/serviceAttachments/xyz", + ExpectDiffSuppress: true, + }, + "different resources": { + Old: "projects/1234/locations/abc/serviceAttachments/jkl", + New: "projects/ten-tp/locations/abc/serviceAttachments/xyz", + ExpectDiffSuppress: false, + }, + } + + for tn, tc := range cases { + if ProjectNumberDiffSuppress("diffSuppress", tc.Old, tc.New, nil) != tc.ExpectDiffSuppress { + t.Fatalf("bad: %s, '%s' => '%s' expect %t", tn, tc.Old, tc.New, tc.ExpectDiffSuppress) + } + } +} + +func TestProjectIDDiffSuppress(t *testing.T) { + cases := map[string]struct { + Old, New string + ExpectDiffSuppress bool + }{ + "different project identifiers": { + Old: "projects/ten-tp/locations/abc/serviceAttachments/xyz", + New: "projects/1234/locations/abc/serviceAttachments/xyz", + ExpectDiffSuppress: true, + }, + "different resources": { + Old: "projects/ten-tp/locations/abc/serviceAttachments/xyz", + New: "projects/1234/locations/abc/serviceAttachments/jkl", + ExpectDiffSuppress: false, + }, + } + + for tn, tc := range cases { + if ProjectIDDiffSuppress("diffSuppress", tc.Old, tc.New, nil) != tc.ExpectDiffSuppress { + t.Fatalf("bad: %s, '%s' => '%s' expect %t", tn, tc.Old, tc.New, tc.ExpectDiffSuppress) + } + } +} func TestEmptyOrUnsetBlockDiffSuppress(t *testing.T) { cases := map[string]struct { Key, Old, New string diff --git a/mmv1/third_party/terraform/tpgresource/self_link_helpers.go b/mmv1/third_party/terraform/tpgresource/self_link_helpers.go index 5672ab1557f6..2e1089fb36aa 100644 --- a/mmv1/third_party/terraform/tpgresource/self_link_helpers.go +++ b/mmv1/third_party/terraform/tpgresource/self_link_helpers.go @@ -17,7 +17,7 @@ func CompareResourceNames(_, old, new string, _ *schema.ResourceData) bool { } // Compare only the relative path of two self links. -func CompareSelfLinkRelativePaths(_, old, new string, _ *schema.ResourceData) bool { +func CompareSelfLinkRelativePathsIgnoreProjectId(unused1, old, new string, unused2 *schema.ResourceData) bool { oldStripped, err := GetRelativePath(old) if err != nil { return false @@ -31,7 +31,24 @@ func CompareSelfLinkRelativePaths(_, old, new string, _ *schema.ResourceData) bo if oldStripped == newStripped { return true } + return ProjectIDDiffSuppress(unused1, oldStripped, newStripped, unused2) +} + +// Compare only the relative path of two self links. +func CompareSelfLinkRelativePaths(_, old, new string, _ *schema.ResourceData) bool { + oldStripped, err := GetRelativePath(old) + if err != nil { + return false + } + newStripped, err := GetRelativePath(new) + if err != nil { + return false + } + + if oldStripped == newStripped { + return true + } return false } diff --git a/mmv1/third_party/terraform/tpgresource/self_link_helpers_test.go b/mmv1/third_party/terraform/tpgresource/self_link_helpers_test.go index 36871db8e937..4dc0c84e0381 100644 --- a/mmv1/third_party/terraform/tpgresource/self_link_helpers_test.go +++ b/mmv1/third_party/terraform/tpgresource/self_link_helpers_test.go @@ -66,6 +66,70 @@ func TestCompareSelfLinkOrResourceName(t *testing.T) { } } +func TestCompareSelfLinkRelativePathsIgnoreProjectId(t *testing.T) { + cases := map[string]struct { + Old, New string + Expect bool + }{ + "full path, project number": { + Old: "https://www.googleapis.com/compute/v1/projects/your-project/global/networks/a-network", + New: "https://www.googleapis.com/compute/v1/projects/1234/global/networks/a-network", + Expect: true, + }, + "partial path, project number": { + Old: "https://www.googleapis.com/compute/v1/projects/your-project/global/networks/a-network", + New: "projects/1234/global/networks/a-network", + Expect: true, + }, + "partial path, same": { + Old: "https://www.googleapis.com/compute/v1/projects/your-project/global/networks/a-network", + New: "projects/your-project/global/networks/a-network", + Expect: true, + }, + "partial path, different name": { + Old: "https://www.googleapis.com/compute/v1/projects/your-project/global/networks/a-network", + New: "projects/your-project/global/networks/another-network", + Expect: false, + }, + "partial path, different project": { + Old: "https://www.googleapis.com/compute/v1/projects/your-project/global/networks/a-network", + New: "projects/another-project/global/networks/a-network", + Expect: false, + }, + "full path, different name": { + Old: "https://www.googleapis.com/compute/v1/projects/your-project/global/networks/a-network", + New: "https://www.googleapis.com/compute/v1/projects/your-project/global/networks/another-network", + Expect: false, + }, + "full path, different project": { + Old: "https://www.googleapis.com/compute/v1/projects/your-project/global/networks/a-network", + New: "https://www.googleapis.com/compute/v1/projects/another-project/global/networks/a-network", + Expect: false, + }, + "beta full path, same": { + Old: "https://www.googleapis.com/compute/v1/projects/your-project/global/networks/a-network", + New: "https://www.googleapis.com/compute/beta/projects/your-project/global/networks/a-network", + Expect: true, + }, + "beta full path, different name": { + Old: "https://www.googleapis.com/compute/v1/projects/your-project/global/networks/a-network", + New: "https://www.googleapis.com/compute/beta/projects/your-project/global/networks/another-network", + Expect: false, + }, + "beta full path, different project": { + Old: "https://www.googleapis.com/compute/v1/projects/your-project/global/networks/a-network", + New: "https://www.googleapis.com/compute/beta/projects/another-project/global/networks/a-network", + Expect: false, + }, + } + + for tn, tc := range cases { + if CompareSelfLinkRelativePathsIgnoreProjectId("", tc.Old, tc.New, nil) != tc.Expect { + t.Errorf("bad: %s, expected %t for old = %q and new = %q", tn, tc.Expect, tc.Old, tc.New) + } + } +} + func TestGetResourceNameFromSelfLink(t *testing.T) { cases := map[string]struct { SelfLink, ExpectedName string