fix an issue in InternalIpDiffSuppress (GoogleCloudPlatform#9062)

* fix an issue in InternalIpDiffSuppress * added tests * update a function --------- Co-authored-by: Edward Sun <sunedward@google.com>
trodge · Oct 5, 2023 · 187fc8a · 187fc8a
1 parent 10a721e
commit 187fc8a
Show file tree

Hide file tree

Showing 3 changed files with 148 additions and 1 deletion.
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
@@ -239,6 +239,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" {
@@ -699,3 +724,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/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,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/mmv1/third_party/terraform/tpgresource/common_diff_suppress_test.go b/mmv1/third_party/terraform/tpgresource/common_diff_suppress_test.go
@@ -285,6 +285,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