Skip to content

Commit

Permalink
Move diff suppress funcs used by a single services into that service'…
Browse files Browse the repository at this point in the history
…s package (#9962) (#18550)

[upstream:167ad5f2228223e948b669d64767dbff650f1fc3]

Signed-off-by: Modular Magician <magic-modules@google.com>
  • Loading branch information
modular-magician authored Jun 25, 2024
1 parent 43805c3 commit d42b1c8
Show file tree
Hide file tree
Showing 32 changed files with 627 additions and 656 deletions.
3 changes: 3 additions & 0 deletions .changelog/9962.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:REPLACEME

```
16 changes: 15 additions & 1 deletion google/services/cloudscheduler/resource_cloud_scheduler_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,20 @@ func validateAuthHeaders(_ context.Context, diff *schema.ResourceDiff, v interfa
return nil
}

// Suppress diffs in below cases
// "https://hello-rehvs75zla-uc.a.run.app/" -> "https://hello-rehvs75zla-uc.a.run.app"
// "https://hello-rehvs75zla-uc.a.run.app" -> "https://hello-rehvs75zla-uc.a.run.app/"
func LastSlashDiffSuppress(_, old, new string, _ *schema.ResourceData) bool {
if last := len(new) - 1; last >= 0 && new[last] == '/' {
new = new[:last]
}

if last := len(old) - 1; last >= 0 && old[last] == '/' {
old = old[:last]
}
return new == old
}

func authHeaderDiffSuppress(k, old, new string, d *schema.ResourceData) bool {
// If generating an `oauth_token` and `scope` is not provided in the configuration,
// the default "https://www.googleapis.com/auth/cloud-platform" scope will be used.
Expand Down Expand Up @@ -242,7 +256,7 @@ send a request to the targeted url`,
"uri": {
Type: schema.TypeString,
Required: true,
DiffSuppressFunc: tpgresource.LastSlashDiffSuppress,
DiffSuppressFunc: LastSlashDiffSuppress,
Description: `The full URI path that the request will be sent to.`,
},
"body": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"github.com/hashicorp/terraform-provider-google/google/acctest"
"github.com/hashicorp/terraform-provider-google/google/services/cloudscheduler"
)

func TestAccCloudSchedulerJob_schedulerPausedExample(t *testing.T) {
Expand Down Expand Up @@ -39,6 +40,45 @@ func TestAccCloudSchedulerJob_schedulerPausedExample(t *testing.T) {
})
}

func TestUnitCloudSchedulerJob_LastSlashDiffSuppress(t *testing.T) {
cases := map[string]struct {
Old, New string
ExpectDiffSuppress bool
}{
"slash to no slash": {
Old: "https://hello-rehvs75zla-uc.a.run.app/",
New: "https://hello-rehvs75zla-uc.a.run.app",
ExpectDiffSuppress: true,
},
"no slash to slash": {
Old: "https://hello-rehvs75zla-uc.a.run.app",
New: "https://hello-rehvs75zla-uc.a.run.app/",
ExpectDiffSuppress: true,
},
"slash to slash": {
Old: "https://hello-rehvs75zla-uc.a.run.app/",
New: "https://hello-rehvs75zla-uc.a.run.app/",
ExpectDiffSuppress: true,
},
"no slash to no slash": {
Old: "https://hello-rehvs75zla-uc.a.run.app",
New: "https://hello-rehvs75zla-uc.a.run.app",
ExpectDiffSuppress: true,
},
"different domains": {
Old: "https://x.a.run.app/",
New: "https://y.a.run.app",
ExpectDiffSuppress: false,
},
}

for tn, tc := range cases {
if cloudscheduler.LastSlashDiffSuppress("uri", tc.Old, tc.New, nil) != tc.ExpectDiffSuppress {
t.Fatalf("bad: %s, '%s' => '%s' expect %t", tn, tc.Old, tc.New, tc.ExpectDiffSuppress)
}
}
}

func testAccCloudSchedulerJob_schedulerPaused(context map[string]interface{}) string {
return acctest.Nprintf(`
resource "google_cloud_scheduler_job" "job" {
Expand Down
57 changes: 55 additions & 2 deletions google/services/compute/resource_compute_forwarding_rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,11 @@
package compute

import (
"bytes"
"context"
"fmt"
"log"
"net"
"net/http"
"reflect"
"strings"
Expand Down Expand Up @@ -51,6 +53,57 @@ func forwardingRuleCustomizeDiff(_ context.Context, diff *schema.ResourceDiff, v
return nil
}

// Port range '80' and '80-80' is equivalent.
// `old` is read from the server and always has the full range format (e.g. '80-80', '1024-2048').
// `new` can be either a single port or a port range.
func PortRangeDiffSuppress(k, old, new string, d *schema.ResourceData) bool {
return old == new+"-"+new
}

// 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 {
addr_equality := false
netmask_equality := false

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 {
// old and new are IP addresses
addr_equality = bytes.Equal(addr_old, addr_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 addr_equality && netmask_equality
}

func ResourceComputeForwardingRule() *schema.Resource {
return &schema.Resource{
Create: resourceComputeForwardingRuleCreate,
Expand Down Expand Up @@ -98,7 +151,7 @@ lowercase letters and numbers and must start with a letter.`,
Computed: true,
Optional: true,
ForceNew: true,
DiffSuppressFunc: tpgresource.InternalIpDiffSuppress,
DiffSuppressFunc: InternalIpDiffSuppress,
Description: `IP address for which this forwarding rule accepts traffic. When a client
sends traffic to this IP address, the forwarding rule directs the traffic
to the referenced 'target' or 'backendService'.
Expand Down Expand Up @@ -305,7 +358,7 @@ networkTier of the Address. Possible values: ["PREMIUM", "STANDARD"]`,
Computed: true,
Optional: true,
ForceNew: true,
DiffSuppressFunc: tpgresource.PortRangeDiffSuppress,
DiffSuppressFunc: PortRangeDiffSuppress,
Description: `The 'ports', 'portRange', and 'allPorts' fields are mutually exclusive.
Only packets addressed to ports in the specified range will be forwarded
to the backends configured with this forwarding rule.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ For Private Service Connect forwarding rules that forward traffic to managed ser
Computed: true,
Optional: true,
ForceNew: true,
DiffSuppressFunc: tpgresource.InternalIpDiffSuppress,
DiffSuppressFunc: InternalIpDiffSuppress,
Description: `IP address for which this forwarding rule accepts traffic. When a client
sends traffic to this IP address, the forwarding rule directs the traffic
to the referenced 'target'.
Expand Down Expand Up @@ -284,7 +284,7 @@ APIs, a network must be provided.`,
Type: schema.TypeString,
Optional: true,
ForceNew: true,
DiffSuppressFunc: tpgresource.PortRangeDiffSuppress,
DiffSuppressFunc: PortRangeDiffSuppress,
Description: `The 'portRange' field has the following limitations:
* It requires that the forwarding rule 'IPProtocol' be TCP, UDP, or SCTP,
and
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"github.com/hashicorp/terraform-provider-google/google/acctest"
"github.com/hashicorp/terraform-provider-google/google/services/compute"
)

func TestAccComputeGlobalForwardingRule_updateTarget(t *testing.T) {
Expand Down Expand Up @@ -87,6 +88,144 @@ func TestAccComputeGlobalForwardingRule_ipv6(t *testing.T) {
})
}

func TestUnitComputeGlobalForwardingRule_PortRangeDiffSuppress(t *testing.T) {
cases := map[string]struct {
Old, New string
ExpectDiffSuppress bool
}{
"different single values": {
Old: "80-80",
New: "443",
ExpectDiffSuppress: false,
},
"different ranges": {
Old: "80-80",
New: "443-444",
ExpectDiffSuppress: false,
},
"same single values": {
Old: "80-80",
New: "80",
ExpectDiffSuppress: true,
},
"same ranges": {
Old: "80-80",
New: "80-80",
ExpectDiffSuppress: false,
},
}

for tn, tc := range cases {
if compute.PortRangeDiffSuppress("ports", tc.Old, tc.New, nil) != tc.ExpectDiffSuppress {
t.Fatalf("bad: %s, '%s' => '%s' expect %t", tn, tc.Old, tc.New, tc.ExpectDiffSuppress)
}
}
}

func TestUnitComputeGlobalForwardingRule_InternalIpDiffSuppress(t *testing.T) {
cases := map[string]struct {
Old, New string
ExpectDiffSuppress bool
}{
"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,
},
"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,
},
"suppress - long ipv6 IP with netmask and short ipv6 IP without netmask": {
Old: "2600:1900:4020:31cd:8000:0:0:0/96",
New: "2600:1900:4020:31cd:8000::",
ExpectDiffSuppress: true,
},
"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:8000::/96",
ExpectDiffSuppress: true,
},
"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: "projects/project_id/regions/region/addresses/address-name",
ExpectDiffSuppress: true,
},
"do not suppress - ipv6 IPs different netmask": {
Old: "2600:1900:4020:31cd:8000:0:0:0/96",
New: "2600:1900:4020:31cd:8000:0:0:0/95",
ExpectDiffSuppress: false,
},
"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,
},
"suppress - ipv4 IPs": {
Old: "1.2.3.4",
New: "1.2.3.4",
ExpectDiffSuppress: true,
},
"suppress - ipv4 IP without netmask and ipv4 IP with netmask": {
Old: "1.2.3.4",
New: "1.2.3.4/24",
ExpectDiffSuppress: true,
},
"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 {
if compute.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 testAccComputeGlobalForwardingRule_httpProxy(fr, targetProxy, proxy, proxy2, backend, hc, urlmap string) string {
return fmt.Sprintf(`
resource "google_compute_global_forwarding_rule" "forwarding_rule" {
Expand Down
26 changes: 25 additions & 1 deletion google/services/compute/resource_compute_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,30 @@ import (
"google.golang.org/api/compute/v1"
)

func IpCidrRangeDiffSuppress(k, old, new string, d *schema.ResourceData) bool {
// The range may be a:
// A) single IP address (e.g. 10.2.3.4)
// B) CIDR format string (e.g. 10.1.2.0/24)
// C) netmask (e.g. /24)
//
// For A) and B), no diff to suppress, they have to match completely.
// For C), The API picks a network IP address and this creates a diff of the form:
// network_interface.0.alias_ip_range.0.ip_cidr_range: "10.128.1.0/24" => "/24"
// We should only compare the mask portion for this case.
if len(new) > 0 && new[0] == '/' {
oldNetmaskStartPos := strings.LastIndex(old, "/")

if oldNetmaskStartPos != -1 {
oldNetmask := old[strings.LastIndex(old, "/"):]
if oldNetmask == new {
return true
}
}
}

return false
}

var (
bootDiskKeys = []string{
"boot_disk.0.auto_delete",
Expand Down Expand Up @@ -392,7 +416,7 @@ func ResourceComputeInstance() *schema.Resource {
"ip_cidr_range": {
Type: schema.TypeString,
Required: true,
DiffSuppressFunc: tpgresource.IpCidrRangeDiffSuppress,
DiffSuppressFunc: IpCidrRangeDiffSuppress,
Description: `The IP CIDR range represented by this alias IP range.`,
},
"subnetwork_range_name": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,7 @@ Google Cloud KMS.`,
Type: schema.TypeString,
Required: true,
ForceNew: true,
DiffSuppressFunc: tpgresource.IpCidrRangeDiffSuppress,
DiffSuppressFunc: IpCidrRangeDiffSuppress,
Description: `The IP CIDR range represented by this alias IP range. This IP CIDR range must belong to the specified subnetwork and cannot contain IP addresses reserved by system or used by other network interfaces. At the time of writing only a netmask (e.g. /24) may be supplied, with a CIDR format resulting in an API error.`,
},
"subnetwork_range_name": {
Expand Down
Loading

0 comments on commit d42b1c8

Please sign in to comment.