Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[#16400] Fix and simplify InternalIpDiffSuppress function #16550

Merged
merged 1 commit into from
Nov 16, 2023
Merged

[#16400] Fix and simplify InternalIpDiffSuppress function #16550

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/9423.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
Fix (and simplify) InternalIpDiffSuppress function to suppress drifts for IPv6 with and without netmask.
```
45 changes: 34 additions & 11 deletions google/tpgresource/common_diff_suppress.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,25 +180,48 @@ func TimestampDiffSuppress(format string) schema.SchemaDiffSuppressFunc {
}
}

// 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
// Suppresses diff for IPv4 and IPv6 different formats.
// It also suppresses diffs if an IP is changing to a reference.
func InternalIpDiffSuppress(_, old, new string, _ *schema.ResourceData) bool {
olds := strings.Split(old, "/")
news := strings.Split(new, "/")
addr_equality := false
netmask_equality := false

if len(olds) == 2 {
if len(news) == 2 {
return bytes.Equal(net.ParseIP(olds[0]), net.ParseIP(news[0])) && olds[1] == news[1]
addr_netmask_old := strings.Split(old, "/")
addr_netmask_new := strings.Split(new, "/")

// Check if old or new are IPs (with or without netmask)
var addr_old net.IP
if net.ParseIP(addr_netmask_old[0]) == nil {
addr_old = net.ParseIP(old)
} else {
addr_old = net.ParseIP(addr_netmask_old[0])
}
var addr_new net.IP
if net.ParseIP(addr_netmask_new[0]) == nil {
addr_new = net.ParseIP(new)
} else {
addr_new = net.ParseIP(addr_netmask_new[0])
}

if addr_old != nil {
if addr_new == nil {
// old is an IP and new is a reference
addr_equality = true
} else {
return (net.ParseIP(olds[0]) != nil) && (net.ParseIP(new) == nil)
// old and new are IP addresses
addr_equality = bytes.Equal(addr_old, addr_new)
}
}

if (net.ParseIP(old) != nil) && (net.ParseIP(new) != nil) {
return bytes.Equal(net.ParseIP(old), net.ParseIP(new))
// If old and new both have a netmask compare them, otherwise suppress
// This is not technically correct but prevents the permadiff described in https://github.com/hashicorp/terraform-provider-google/issues/16400
if (len(addr_netmask_old)) == 2 && (len(addr_netmask_new) == 2) {
netmask_equality = addr_netmask_old[1] == addr_netmask_new[1]
} else {
netmask_equality = true
}

return (net.ParseIP(old) != nil) && (net.ParseIP(new) == nil)
return addr_equality && netmask_equality
}

// Suppress diffs for duration format. ex "60.0s" and "60s" same
Expand Down
82 changes: 61 additions & 21 deletions google/tpgresource/common_diff_suppress_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,56 +292,96 @@ func TestInternalIpDiffSuppress(t *testing.T) {
Old, New string
ExpectDiffSuppress bool
}{
"same1 ipv6s": {
"suppress - same long and short ipv6 IPs without netmask": {
Old: "2600:1900:4020:31cd:8000:0:0:0",
New: "2600:1900:4020:31cd:8000::",
ExpectDiffSuppress: true,
},
"same2 ipv6s": {
"suppress - long and short ipv6 IPs with netmask": {
Old: "2600:1900:4020:31cd:8000:0:0:0/96",
New: "2600:1900:4020:31cd:8000::/96",
ExpectDiffSuppress: true,
},
"same3 ipv6s": {
"suppress - long ipv6 IP with netmask and short ipv6 IP without netmask": {
Old: "2600:1900:4020:31cd:8000:0:0:0/96",
New: "https://www.googleapis.com/compute/v1/projects/myproject/regions/us-central1/addresses/myaddress",
New: "2600:1900:4020:31cd:8000::",
ExpectDiffSuppress: true,
},
"different1 ipv6s": {
"suppress - long ipv6 IP without netmask and short ipv6 IP with netmask": {
Old: "2600:1900:4020:31cd:8000:0:0:0",
New: "2600:1900:4020:31cd:8001::",
ExpectDiffSuppress: false,
New: "2600:1900:4020:31cd:8000::/96",
ExpectDiffSuppress: true,
},
"different2 ipv6s": {
"suppress - long ipv6 IP with netmask and reference": {
Old: "2600:1900:4020:31cd:8000:0:0:0/96",
New: "projects/project_id/regions/region/addresses/address-name",
ExpectDiffSuppress: true,
},
"suppress - long ipv6 IP without netmask and reference": {
Old: "2600:1900:4020:31cd:8000:0:0:0",
New: "2600:1900:4020:31cd:8000:0:0:8000",
ExpectDiffSuppress: false,
New: "projects/project_id/regions/region/addresses/address-name",
ExpectDiffSuppress: true,
},
"different3 ipv6s": {
"do not suppress - ipv6 IPs different netmask": {
Old: "2600:1900:4020:31cd:8000:0:0:0/96",
New: "2600:1900:4020:31cd:8001::/96",
New: "2600:1900:4020:31cd:8000:0:0:0/95",
ExpectDiffSuppress: false,
},
"different ipv4s": {
Old: "1.2.3.4",
New: "1.2.3.5",
"do not suppress - reference and ipv6 IP with netmask": {
Old: "projects/project_id/regions/region/addresses/address-name",
New: "2600:1900:4020:31cd:8000:0:0:0/96",
ExpectDiffSuppress: false,
},
"do not suppress - ipv6 IPs - 1": {
Old: "2600:1900:4020:31cd:8000:0:0:0",
New: "2600:1900:4020:31cd:8001::",
ExpectDiffSuppress: false,
},
"do not suppress - ipv6 IPs - 2": {
Old: "2600:1900:4020:31cd:8000:0:0:0",
New: "2600:1900:4020:31cd:8000:0:0:8000",
ExpectDiffSuppress: false,
},
"same ipv4s": {
"suppress - ipv4 IPs": {
Old: "1.2.3.4",
New: "1.2.3.4",
ExpectDiffSuppress: true,
},
"ipv4 vs id": {
"suppress - ipv4 IP without netmask and ipv4 IP with netmask": {
Old: "1.2.3.4",
New: "google_compute_address.my_ipv4_addr.address",
New: "1.2.3.4/24",
ExpectDiffSuppress: true,
},
"ipv6 vs id": {
Old: "2600:1900:4020:31cd:8000:0:0:0",
New: "google_compute_address.my_ipv6_addr.address",
"suppress - ipv4 IP without netmask and reference": {
Old: "1.2.3.4",
New: "projects/project_id/regions/region/addresses/address-name",
ExpectDiffSuppress: true,
},
"do not suppress - reference and ipv4 IP without netmask": {
Old: "projects/project_id/regions/region/addresses/address-name",
New: "1.2.3.4",
ExpectDiffSuppress: false,
},
"do not suppress - different ipv4 IPs": {
Old: "1.2.3.4",
New: "1.2.3.5",
ExpectDiffSuppress: false,
},
"do not suppress - ipv4 IPs different netmask": {
Old: "1.2.3.4/24",
New: "1.2.3.5/25",
ExpectDiffSuppress: false,
},
"do not suppress - different references": {
Old: "projects/project_id/regions/region/addresses/address-name",
New: "projects/project_id/regions/region/addresses/address-name-1",
ExpectDiffSuppress: false,
},
"do not suppress - same references": {
Old: "projects/project_id/regions/region/addresses/address-name",
New: "projects/project_id/regions/region/addresses/address-name",
ExpectDiffSuppress: false,
},
}

for tn, tc := range cases {
Expand Down