From b4e6cb5b9a58b51a10108add168aa73a380efa25 Mon Sep 17 00:00:00 2001 From: Modular Magician Date: Wed, 4 Oct 2023 17:28:41 +0000 Subject: [PATCH] fix an issue in InternalIpDiffSuppress (#9062) * fix an issue in InternalIpDiffSuppress * added tests * update a function --------- Co-authored-by: Edward Sun [upstream:bf3557dd1720a2366b9fef23625a9693e3c08902] Signed-off-by: Modular Magician --- .changelog/9062.txt | 3 + .../resource_compute_forwarding_rule_test.go | 67 +++++++++++++++++++ google/tpgresource/common_diff_suppress.go | 18 ++++- .../tpgresource/common_diff_suppress_test.go | 64 ++++++++++++++++++ 4 files changed, 151 insertions(+), 1 deletion(-) create mode 100644 .changelog/9062.txt diff --git a/.changelog/9062.txt b/.changelog/9062.txt new file mode 100644 index 00000000000..2d0808243d2 --- /dev/null +++ b/.changelog/9062.txt @@ -0,0 +1,3 @@ +```release-note:bug +compute: fixed a false permadiff on `ip_address` when it is set to ipv6 on `google_compute_forwarding_rule` +``` diff --git a/google/services/compute/resource_compute_forwarding_rule_test.go b/google/services/compute/resource_compute_forwarding_rule_test.go index 19c2e7e3c1d..744a3295e96 100644 --- a/google/services/compute/resource_compute_forwarding_rule_test.go +++ b/google/services/compute/resource_compute_forwarding_rule_test.go @@ -176,6 +176,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" { @@ -401,3 +426,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) +} diff --git a/google/tpgresource/common_diff_suppress.go b/google/tpgresource/common_diff_suppress.go index 6413a654e0f..fdcf280c459 100644 --- a/google/tpgresource/common_diff_suppress.go +++ b/google/tpgresource/common_diff_suppress.go @@ -5,6 +5,7 @@ package tpgresource import ( + "bytes" "crypto/sha256" "encoding/hex" "log" @@ -179,9 +180,24 @@ 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) == 2 { + if len(news) == 2 { + 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) { + return bytes.Equal(net.ParseIP(old), net.ParseIP(new)) + } + return (net.ParseIP(old) != nil) && (net.ParseIP(new) == nil) } diff --git a/google/tpgresource/common_diff_suppress_test.go b/google/tpgresource/common_diff_suppress_test.go index 82916114128..26427bc31af 100644 --- a/google/tpgresource/common_diff_suppress_test.go +++ b/google/tpgresource/common_diff_suppress_test.go @@ -287,6 +287,70 @@ 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, + }, + "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::", + 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