From ea867e0792985d45be43933fc2eb2f388da024ab Mon Sep 17 00:00:00 2001 From: Edward Sun Date: Fri, 22 Sep 2023 15:02:09 -0700 Subject: [PATCH 1/3] fix an issue in InternalIpDiffSuppress --- .../tpgresource/common_diff_suppress.go.erb | 23 +++++++- .../tpgresource/common_diff_suppress_test.go | 59 +++++++++++++++++++ 2 files changed, 81 insertions(+), 1 deletion(-) diff --git a/mmv1/third_party/terraform/tpgresource/common_diff_suppress.go.erb b/mmv1/third_party/terraform/tpgresource/common_diff_suppress.go.erb index 86ced35209ab..b4919aa0bd2b 100644 --- a/mmv1/third_party/terraform/tpgresource/common_diff_suppress.go.erb +++ b/mmv1/third_party/terraform/tpgresource/common_diff_suppress.go.erb @@ -13,6 +13,7 @@ import ( "strconv" "strings" "time" + "bytes" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" ) @@ -178,9 +179,29 @@ func TimestampDiffSuppress(format string) schema.SchemaDiffSuppressFunc { } } -// suppress diff when saved is Ipv4 format while new is required a reference +// suppress diff when saved is Ipv4 and Ipv6 format while new is required a reference // this happens for an internal ip for Private Services Connect func InternalIpDiffSuppress(_, old, new string, _ *schema.ResourceData) bool { + olds := strings.Split(old, "/") + news := strings.Split(new, "/") + + if len(olds) == len(news) { + if len(olds) == 2 { + if bytes.Equal(net.ParseIP(olds[0]), net.ParseIP(news[0])) && olds[1] == news[1] { + return true + } else { + return false + } + } + } + + if (net.ParseIP(old) != nil) && (net.ParseIP(new) != nil) { + if bytes.Equal(net.ParseIP(old), net.ParseIP(new)) { + return true + } else { + return false + } + } return (net.ParseIP(old) != nil) && (net.ParseIP(new) == nil) } 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 6315ada0dd6b..fc40b4537e04 100644 --- a/mmv1/third_party/terraform/tpgresource/common_diff_suppress_test.go +++ b/mmv1/third_party/terraform/tpgresource/common_diff_suppress_test.go @@ -285,6 +285,65 @@ func TestDurationDiffSuppress(t *testing.T) { } } +func TestInternalIpDiffSuppress(t *testing.T) { + cases := map[string]struct { + Old, New string + ExpectDiffSuppress bool + }{ + "same1 ipv6s": { + Old: "2600:1900:4020:31cd:8000:0:0:0", + New: "2600:1900:4020:31cd:8000::", + ExpectDiffSuppress: true, + }, + "same2 ipv6s": { + Old: "2600:1900:4020:31cd:8000:0:0:0/96", + New: "2600:1900:4020:31cd:8000::/96", + ExpectDiffSuppress: true, + }, + "different1 ipv6s": { + Old: "2600:1900:4020:31cd:8000:0:0:0", + New: "2600:1900:4020:31cd:8001::", + ExpectDiffSuppress: false, + }, + "different2 ipv6s": { + Old: "2600:1900:4020:31cd:8000:0:0:0", + New: "2600:1900:4020:31cd:8000:0:0:8000", + ExpectDiffSuppress: false, + }, + "different3 ipv6s": { + Old: "2600:1900:4020:31cd:8000:0:0:0/96", + New: "2600:1900:4020:31cd:8001::/96", + ExpectDiffSuppress: false, + }, + "different ipv4s": { + Old: "1.2.3.4", + New: "1.2.3.5", + ExpectDiffSuppress: false, + }, + "same ipv4s": { + Old: "1.2.3.4", + New: "1.2.3.4", + ExpectDiffSuppress: true, + }, + "ipv4 vs id": { + Old: "1.2.3.4", + New: "google_compute_address.my_ipv4_addr.address", + ExpectDiffSuppress: true, + }, + "ipv6 vs id": { + Old: "2600:1900:4020:31cd:8000:0:0:0", + New: "google_compute_address.my_ipv6_addr.address", + ExpectDiffSuppress: true, + }, + } + + for tn, tc := range cases { + if InternalIpDiffSuppress("ipv4/v6_compare", tc.Old, tc.New, nil) != tc.ExpectDiffSuppress { + t.Fatalf("bad: %s, '%s' => '%s' expect %t", tn, tc.Old, tc.New, tc.ExpectDiffSuppress) + } + } +} + func TestLastSlashDiffSuppress(t *testing.T) { cases := map[string]struct { Old, New string From 6e9999c9aa8580fbe32a7752734b491b4027fc48 Mon Sep 17 00:00:00 2001 From: Edward Sun Date: Sun, 24 Sep 2023 08:31:27 -0700 Subject: [PATCH 2/3] added tests --- ...source_compute_forwarding_rule_test.go.erb | 67 +++++++++++++++++++ .../tpgresource/common_diff_suppress.go.erb | 7 +- .../tpgresource/common_diff_suppress_test.go | 5 ++ 3 files changed, 77 insertions(+), 2 deletions(-) diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_forwarding_rule_test.go.erb b/mmv1/third_party/terraform/services/compute/resource_compute_forwarding_rule_test.go.erb index c1445b640f3c..c35ab2f65cf9 100644 --- a/mmv1/third_party/terraform/services/compute/resource_compute_forwarding_rule_test.go.erb +++ b/mmv1/third_party/terraform/services/compute/resource_compute_forwarding_rule_test.go.erb @@ -237,6 +237,31 @@ func TestAccComputeForwardingRule_forwardingRuleRegionalSteeringExampleUpdate(t }) } +func TestAccComputeForwardingRule_forwardingRuleIpAddressIpv6(t *testing.T) { + t.Parallel() + + context := map[string]interface{}{ + "random_suffix": acctest.RandString(t, 10), + } + + acctest.VcrTest(t, resource.TestCase{ + PreCheck: func() { acctest.AccTestPreCheck(t) }, + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + CheckDestroy: testAccCheckComputeForwardingRuleDestroyProducer(t), + Steps: []resource.TestStep{ + { + Config: testAccComputeForwardingRule_forwardingRuleIpAddressIpv6(context), + }, + { + ResourceName: "google_compute_forwarding_rule.external", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"backend_service", "network", "subnetwork", "region"}, + }, + }, + }) +} + func testAccComputeForwardingRule_basic(poolName, ruleName string) string { return fmt.Sprintf(` resource "google_compute_target_pool" "foo-tp" { @@ -697,3 +722,45 @@ resource "google_compute_forwarding_rule" "external" { } `, context) } + +func testAccComputeForwardingRule_forwardingRuleIpAddressIpv6(context map[string]interface{}) string { + return acctest.Nprintf(` +resource "google_compute_address" "basic" { + name = "tf-test-address%{random_suffix}" + region = "us-central1" + + address_type = "EXTERNAL" + ipv6_endpoint_type = "NETLB" + ip_version = "IPV6" + subnetwork = google_compute_subnetwork.subnetwork-ipv6.id +} + +resource "google_compute_region_backend_service" "external" { + name = "tf-test-backend%{random_suffix}" + region = "us-central1" + load_balancing_scheme = "EXTERNAL" +} + +resource "google_compute_forwarding_rule" "external" { + name = "tf-test-forwarding-rule%{random_suffix}" + region = "us-central1" + ip_address = google_compute_address.basic.self_link + backend_service = google_compute_region_backend_service.external.self_link + load_balancing_scheme = "EXTERNAL" +} + +resource "google_compute_subnetwork" "subnetwork-ipv6" { + name = "tf-test-subnetwork%{random_suffix}" + ip_cidr_range = "10.0.0.0/22" + region = "us-central1" + stack_type = "IPV4_IPV6" + ipv6_access_type = "EXTERNAL" + network = google_compute_network.custom-test.id +} + +resource "google_compute_network" "custom-test" { + name = "tf-test-network%{random_suffix}" + auto_create_subnetworks = false +} +`, context) +} \ No newline at end of file diff --git a/mmv1/third_party/terraform/tpgresource/common_diff_suppress.go.erb b/mmv1/third_party/terraform/tpgresource/common_diff_suppress.go.erb index b4919aa0bd2b..af25c2a7fbac 100644 --- a/mmv1/third_party/terraform/tpgresource/common_diff_suppress.go.erb +++ b/mmv1/third_party/terraform/tpgresource/common_diff_suppress.go.erb @@ -185,13 +185,15 @@ func InternalIpDiffSuppress(_, old, new string, _ *schema.ResourceData) bool { olds := strings.Split(old, "/") news := strings.Split(new, "/") - if len(olds) == len(news) { - if len(olds) == 2 { + if len(olds) == 2 { + if len(news) == 2 { if bytes.Equal(net.ParseIP(olds[0]), net.ParseIP(news[0])) && olds[1] == news[1] { return true } else { return false } + } else { + return (net.ParseIP(olds[0]) != nil) && (net.ParseIP(new) == nil) } } @@ -202,6 +204,7 @@ func InternalIpDiffSuppress(_, old, new string, _ *schema.ResourceData) bool { return false } } + return (net.ParseIP(old) != nil) && (net.ParseIP(new) == nil) } 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 fc40b4537e04..990003fd2264 100644 --- a/mmv1/third_party/terraform/tpgresource/common_diff_suppress_test.go +++ b/mmv1/third_party/terraform/tpgresource/common_diff_suppress_test.go @@ -300,6 +300,11 @@ func TestInternalIpDiffSuppress(t *testing.T) { New: "2600:1900:4020:31cd:8000::/96", ExpectDiffSuppress: true, }, + "same3 ipv6s": { + Old: "2600:1900:4020:31cd:8000:0:0:0/96", + New: "https://www.googleapis.com/compute/v1/projects/myproject/regions/us-central1/addresses/myaddress", + ExpectDiffSuppress: true, + }, "different1 ipv6s": { Old: "2600:1900:4020:31cd:8000:0:0:0", New: "2600:1900:4020:31cd:8001::", From 2c4b7f6c7552a79581153f00d32755168641e179 Mon Sep 17 00:00:00 2001 From: Edward Sun Date: Mon, 25 Sep 2023 09:46:25 -0700 Subject: [PATCH 3/3] update a function --- .../tpgresource/common_diff_suppress.go.erb | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/mmv1/third_party/terraform/tpgresource/common_diff_suppress.go.erb b/mmv1/third_party/terraform/tpgresource/common_diff_suppress.go.erb index af25c2a7fbac..35dac06e822f 100644 --- a/mmv1/third_party/terraform/tpgresource/common_diff_suppress.go.erb +++ b/mmv1/third_party/terraform/tpgresource/common_diff_suppress.go.erb @@ -187,22 +187,14 @@ func InternalIpDiffSuppress(_, old, new string, _ *schema.ResourceData) bool { if len(olds) == 2 { if len(news) == 2 { - if bytes.Equal(net.ParseIP(olds[0]), net.ParseIP(news[0])) && olds[1] == news[1] { - return true - } else { - return false - } + return bytes.Equal(net.ParseIP(olds[0]), net.ParseIP(news[0])) && olds[1] == news[1] } else { return (net.ParseIP(olds[0]) != nil) && (net.ParseIP(new) == nil) } } if (net.ParseIP(old) != nil) && (net.ParseIP(new) != nil) { - if bytes.Equal(net.ParseIP(old), net.ParseIP(new)) { - return true - } else { - return false - } + return bytes.Equal(net.ParseIP(old), net.ParseIP(new)) } return (net.ParseIP(old) != nil) && (net.ParseIP(new) == nil)