From 4fa9a2f44392742b83f147a5494429dcf5eb4def Mon Sep 17 00:00:00 2001 From: Yacine FODIL Date: Thu, 2 Oct 2025 16:48:53 +0200 Subject: [PATCH 1/4] fix(ipam): only suppress diffs when IP host part is identical --- internal/services/ipam/helpers.go | 28 +++------ internal/services/ipam/helpers_test.go | 84 ++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 20 deletions(-) create mode 100644 internal/services/ipam/helpers_test.go diff --git a/internal/services/ipam/helpers.go b/internal/services/ipam/helpers.go index 8708acbb8b..9223dcb48e 100644 --- a/internal/services/ipam/helpers.go +++ b/internal/services/ipam/helpers.go @@ -44,29 +44,17 @@ func NewAPIWithRegionAndID(m any, id string) (*ipam.API, scw.Region, string, err } func diffSuppressFuncStandaloneIPandCIDR(_, oldValue, newValue string, _ *schema.ResourceData) bool { - oldIP, oldNet, errOld := net.ParseCIDR(oldValue) - if errOld != nil { - oldIP = net.ParseIP(oldValue) - } - - newIP, newNet, errNew := net.ParseCIDR(newValue) - if errNew != nil { - newIP = net.ParseIP(newValue) - } - - if oldIP != nil && newIP != nil && oldIP.Equal(newIP) { - return true - } - - if oldNet != nil && newIP != nil && oldNet.Contains(newIP) { - return true + parseIPOrCIDR := func(s string) net.IP { + if ip, _, err := net.ParseCIDR(s); err == nil { + return ip + } + return net.ParseIP(s) } - if newNet != nil && oldIP != nil && newNet.Contains(oldIP) { - return true - } + oldIP := parseIPOrCIDR(oldValue) + newIP := parseIPOrCIDR(newValue) - return false + return oldIP != nil && newIP != nil && oldIP.Equal(newIP) } type GetResourcePrivateIPsOptions struct { diff --git a/internal/services/ipam/helpers_test.go b/internal/services/ipam/helpers_test.go new file mode 100644 index 0000000000..35df001f06 --- /dev/null +++ b/internal/services/ipam/helpers_test.go @@ -0,0 +1,84 @@ +package ipam + +import ( + "testing" +) + +func TestDiffSuppress_IPAMIP(t *testing.T) { + tests := []struct { + name string + oldValue string + newValue string + want bool + }{ + { + name: "IP == IP", + oldValue: "172.16.32.7", + newValue: "172.16.32.7", + want: true, + }, + { + name: "IP == IP/CIDR same host", + oldValue: "172.16.32.7/22", + newValue: "172.16.32.7", + want: true, + }, + { + name: "IP/CIDR == IP same host (reversed)", + oldValue: "172.16.32.7", + newValue: "172.16.32.7/22", + want: true, + }, + { + name: "Different host within same CIDR is NOT suppressed", + oldValue: "172.16.32.7/22", + newValue: "172.16.32.8", + want: false, + }, + { + name: "Different host (plain IPs) is NOT suppressed", + oldValue: "172.16.32.7", + newValue: "172.16.32.8", + want: false, + }, + { + name: "Equal but with /32 single-host CIDR", + oldValue: "10.0.0.1/32", + newValue: "10.0.0.1", + want: true, + }, + { + name: "Broader CIDR vs network address should NOT suppress", + oldValue: "10.0.0.1/24", + newValue: "10.0.0.0", + want: false, + }, + { + name: "Invalid old value -> not suppressed", + oldValue: "not-an-ip", + newValue: "10.0.0.1", + want: false, + }, + { + name: "Invalid new value -> not suppressed", + oldValue: "10.0.0.1", + newValue: "bad/32", + want: false, + }, + { + name: "Both invalid -> not suppressed", + oldValue: "nope", + newValue: "nope2", + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := diffSuppressFuncStandaloneIPandCIDR("", tt.oldValue, tt.newValue, nil) + if got != tt.want { + t.Fatalf("diffSuppress(%q, %q) = %v, want %v", tt.oldValue, tt.newValue, got, tt.want) + } + }) + } +} From dd211e03929ce92639542fdfe96ad25275de6592 Mon Sep 17 00:00:00 2001 From: Yacine FODIL Date: Thu, 2 Oct 2025 16:59:01 +0200 Subject: [PATCH 2/4] lint --- internal/services/ipam/helpers.go | 2 +- internal/services/ipam/helpers_test.go | 6 ++++-- internal/services/ipam/ip.go | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/internal/services/ipam/helpers.go b/internal/services/ipam/helpers.go index 9223dcb48e..fcc6012998 100644 --- a/internal/services/ipam/helpers.go +++ b/internal/services/ipam/helpers.go @@ -43,7 +43,7 @@ func NewAPIWithRegionAndID(m any, id string) (*ipam.API, scw.Region, string, err return ipamAPI, region, ID, err } -func diffSuppressFuncStandaloneIPandCIDR(_, oldValue, newValue string, _ *schema.ResourceData) bool { +func DiffSuppressFuncStandaloneIPandCIDR(_, oldValue, newValue string, _ *schema.ResourceData) bool { parseIPOrCIDR := func(s string) net.IP { if ip, _, err := net.ParseCIDR(s); err == nil { return ip diff --git a/internal/services/ipam/helpers_test.go b/internal/services/ipam/helpers_test.go index 35df001f06..b959313098 100644 --- a/internal/services/ipam/helpers_test.go +++ b/internal/services/ipam/helpers_test.go @@ -1,7 +1,9 @@ -package ipam +package ipam_test import ( "testing" + + "github.com/scaleway/terraform-provider-scaleway/v2/internal/services/ipam" ) func TestDiffSuppress_IPAMIP(t *testing.T) { @@ -75,7 +77,7 @@ func TestDiffSuppress_IPAMIP(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := diffSuppressFuncStandaloneIPandCIDR("", tt.oldValue, tt.newValue, nil) + got := ipam.DiffSuppressFuncStandaloneIPandCIDR("", tt.oldValue, tt.newValue, nil) if got != tt.want { t.Fatalf("diffSuppress(%q, %q) = %v, want %v", tt.oldValue, tt.newValue, got, tt.want) } diff --git a/internal/services/ipam/ip.go b/internal/services/ipam/ip.go index af19925ada..cefa3a57f1 100644 --- a/internal/services/ipam/ip.go +++ b/internal/services/ipam/ip.go @@ -38,7 +38,7 @@ func ResourceIP() *schema.Resource { ForceNew: true, Description: "Request a specific IP in the requested source pool", ValidateFunc: validation.IsIPAddress, - DiffSuppressFunc: diffSuppressFuncStandaloneIPandCIDR, + DiffSuppressFunc: DiffSuppressFuncStandaloneIPandCIDR, }, "source": { Type: schema.TypeList, From 2b23fad17dd2b41d00bc27ae573d8153e7c0b41a Mon Sep 17 00:00:00 2001 From: Yacine FODIL Date: Thu, 2 Oct 2025 17:08:18 +0200 Subject: [PATCH 3/4] lint --- internal/services/ipam/helpers.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/services/ipam/helpers.go b/internal/services/ipam/helpers.go index fcc6012998..fbf670d06b 100644 --- a/internal/services/ipam/helpers.go +++ b/internal/services/ipam/helpers.go @@ -48,6 +48,7 @@ func DiffSuppressFuncStandaloneIPandCIDR(_, oldValue, newValue string, _ *schema if ip, _, err := net.ParseCIDR(s); err == nil { return ip } + return net.ParseIP(s) } From 88bd5565113c12532f89f85a032dfd59051dad89 Mon Sep 17 00:00:00 2001 From: Yacine FODIL Date: Thu, 2 Oct 2025 17:39:40 +0200 Subject: [PATCH 4/4] refactor --- internal/dsf/ip.go | 22 +++++++++++++++++++ .../ipam/helpers_test.go => dsf/ip_test.go} | 6 ++--- internal/services/ipam/helpers.go | 16 -------------- internal/services/ipam/ip.go | 2 +- 4 files changed, 26 insertions(+), 20 deletions(-) create mode 100644 internal/dsf/ip.go rename internal/{services/ipam/helpers_test.go => dsf/ip_test.go} (90%) diff --git a/internal/dsf/ip.go b/internal/dsf/ip.go new file mode 100644 index 0000000000..f5f349c0b7 --- /dev/null +++ b/internal/dsf/ip.go @@ -0,0 +1,22 @@ +package dsf + +import ( + "net" + + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +func DiffSuppressFuncStandaloneIPandCIDR(_, oldValue, newValue string, _ *schema.ResourceData) bool { + parseIPOrCIDR := func(s string) net.IP { + if ip, _, err := net.ParseCIDR(s); err == nil { + return ip + } + + return net.ParseIP(s) + } + + oldIP := parseIPOrCIDR(oldValue) + newIP := parseIPOrCIDR(newValue) + + return oldIP != nil && newIP != nil && oldIP.Equal(newIP) +} diff --git a/internal/services/ipam/helpers_test.go b/internal/dsf/ip_test.go similarity index 90% rename from internal/services/ipam/helpers_test.go rename to internal/dsf/ip_test.go index b959313098..6d22adc17c 100644 --- a/internal/services/ipam/helpers_test.go +++ b/internal/dsf/ip_test.go @@ -1,9 +1,9 @@ -package ipam_test +package dsf_test import ( "testing" - "github.com/scaleway/terraform-provider-scaleway/v2/internal/services/ipam" + "github.com/scaleway/terraform-provider-scaleway/v2/internal/dsf" ) func TestDiffSuppress_IPAMIP(t *testing.T) { @@ -77,7 +77,7 @@ func TestDiffSuppress_IPAMIP(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := ipam.DiffSuppressFuncStandaloneIPandCIDR("", tt.oldValue, tt.newValue, nil) + got := dsf.DiffSuppressFuncStandaloneIPandCIDR("", tt.oldValue, tt.newValue, nil) if got != tt.want { t.Fatalf("diffSuppress(%q, %q) = %v, want %v", tt.oldValue, tt.newValue, got, tt.want) } diff --git a/internal/services/ipam/helpers.go b/internal/services/ipam/helpers.go index fbf670d06b..b7c10c5061 100644 --- a/internal/services/ipam/helpers.go +++ b/internal/services/ipam/helpers.go @@ -3,7 +3,6 @@ package ipam import ( "context" "fmt" - "net" "time" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" @@ -43,21 +42,6 @@ func NewAPIWithRegionAndID(m any, id string) (*ipam.API, scw.Region, string, err return ipamAPI, region, ID, err } -func DiffSuppressFuncStandaloneIPandCIDR(_, oldValue, newValue string, _ *schema.ResourceData) bool { - parseIPOrCIDR := func(s string) net.IP { - if ip, _, err := net.ParseCIDR(s); err == nil { - return ip - } - - return net.ParseIP(s) - } - - oldIP := parseIPOrCIDR(oldValue) - newIP := parseIPOrCIDR(newValue) - - return oldIP != nil && newIP != nil && oldIP.Equal(newIP) -} - type GetResourcePrivateIPsOptions struct { ResourceType *ipam.ResourceType ResourceID *string diff --git a/internal/services/ipam/ip.go b/internal/services/ipam/ip.go index cefa3a57f1..b04f457cc0 100644 --- a/internal/services/ipam/ip.go +++ b/internal/services/ipam/ip.go @@ -38,7 +38,7 @@ func ResourceIP() *schema.Resource { ForceNew: true, Description: "Request a specific IP in the requested source pool", ValidateFunc: validation.IsIPAddress, - DiffSuppressFunc: DiffSuppressFuncStandaloneIPandCIDR, + DiffSuppressFunc: dsf.DiffSuppressFuncStandaloneIPandCIDR, }, "source": { Type: schema.TypeList,