From 1e8115ada637a14321a60939447223bdd322ad01 Mon Sep 17 00:00:00 2001 From: Will Hegedus Date: Mon, 31 Aug 2026 17:02:25 -0400 Subject: [PATCH 1/2] fix(firewalls): skip empty IP family when chunking ACL rules processACL created an empty inbound rule for the IP family with no addresses when the other family exceeded maxIPsPerFirewall, because chunkIPs returns a single chunk for an empty slice. Only chunk and emit rules for a family that actually has addresses. --- cloud/linode/services/firewalls.go | 5 ++++- cloud/linode/services/firewalls_test.go | 20 ++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/cloud/linode/services/firewalls.go b/cloud/linode/services/firewalls.go index 7015dfba..80e26a60 100644 --- a/cloud/linode/services/firewalls.go +++ b/cloud/linode/services/firewalls.go @@ -265,8 +265,11 @@ func chunkIPs(ips []string) [][]string { ipCount := len(ips) // If the number of IPs is less than or equal to maxIPsPerFirewall, - // return a single chunk containing all IPs. + // return a single chunk containing all IPs, or nil if chunking an empty slice. if ipCount <= maxIPsPerFirewall { + if ipCount == 0 { + return nil + } return [][]string{ips} } diff --git a/cloud/linode/services/firewalls_test.go b/cloud/linode/services/firewalls_test.go index bf217c8e..c015844b 100644 --- a/cloud/linode/services/firewalls_test.go +++ b/cloud/linode/services/firewalls_test.go @@ -1,6 +1,7 @@ package services import ( + "fmt" "reflect" "testing" @@ -79,6 +80,25 @@ func TestRuleChanged(t *testing.T) { } } +func TestProcessACLNoEmptyRuleForMissingFamily(t *testing.T) { + ipv4s := make([]string, 256) + for i := range ipv4s { + ipv4s[i] = fmt.Sprintf("10.0.%d.1/32", i) + } + + fwcreateOpts := &linodego.FirewallCreateOptions{} + err := processACL(fwcreateOpts, accept, "test", "svc", "80", linodego.NetworkAddresses{IPv4: ipv4s}) + if err != nil { + t.Fatalf("processACL() error = %v", err) + } + + for _, rule := range fwcreateOpts.Rules.Inbound { + if len(rule.Addresses.IPv4) == 0 && len(rule.Addresses.IPv6) == 0 { + t.Errorf("processACL() created an inbound rule with no addresses: %+v", rule) + } + } +} + func TestParsePorts(t *testing.T) { tests := []struct { name string From 9135a64228437f9fb76f22345b72adb6d5cf4481 Mon Sep 17 00:00:00 2001 From: Will Hegedus Date: Mon, 31 Aug 2026 17:07:51 -0400 Subject: [PATCH 2/2] test(firewalls): assert exact chunk sizes and cover IPv6-only case Strengthen TestProcessACLNoEmptyRuleForMissingFamily to assert the exact expected grouping for 256 addresses (two inbound rules with chunk sizes 255 and 1) instead of only checking for absent empty-address rules. Convert the test to table-driven form and add a symmetric IPv6-only case so the IPv4-empty guard in processACL is directly exercised, not just the IPv6-empty guard covered by the IPv4-only case. --- cloud/linode/services/firewalls_test.go | 75 +++++++++++++++++++++---- 1 file changed, 63 insertions(+), 12 deletions(-) diff --git a/cloud/linode/services/firewalls_test.go b/cloud/linode/services/firewalls_test.go index c015844b..154fb3e4 100644 --- a/cloud/linode/services/firewalls_test.go +++ b/cloud/linode/services/firewalls_test.go @@ -80,22 +80,73 @@ func TestRuleChanged(t *testing.T) { } } -func TestProcessACLNoEmptyRuleForMissingFamily(t *testing.T) { - ipv4s := make([]string, 256) - for i := range ipv4s { - ipv4s[i] = fmt.Sprintf("10.0.%d.1/32", i) +// generateCIDRs builds count distinct CIDRs using the given per-index formatter, +// e.g. an IPv4 /32 or an IPv6 /128 formatter. +func generateCIDRs(count int, format func(i int) string) []string { + ips := make([]string, count) + for i := range ips { + ips[i] = format(i) } + return ips +} + +// TestProcessACLNoEmptyRuleForMissingFamily is a regression test for a bug where +// processACL created an empty inbound rule for the IP family with no addresses +// when the other family exceeded maxIPsPerFirewall (255), because chunkIPs +// returns a single chunk for an empty slice. It exercises both single-family +// cases (IPv4-only and the symmetric IPv6-only case) with 256 addresses, which +// must be split into exactly two chunks: 255 and 1. +func TestProcessACLNoEmptyRuleForMissingFamily(t *testing.T) { + const addrCount = 256 + wantChunkSizes := []int{255, 1} - fwcreateOpts := &linodego.FirewallCreateOptions{} - err := processACL(fwcreateOpts, accept, "test", "svc", "80", linodego.NetworkAddresses{IPv4: ipv4s}) - if err != nil { - t.Fatalf("processACL() error = %v", err) + tests := []struct { + name string + ips linodego.NetworkAddresses + }{ + { + name: "IPv4Only", + ips: linodego.NetworkAddresses{ + IPv4: generateCIDRs(addrCount, func(i int) string { return fmt.Sprintf("10.0.%d.1/32", i) }), + }, + }, + { + name: "IPv6Only", + ips: linodego.NetworkAddresses{ + IPv6: generateCIDRs(addrCount, func(i int) string { return fmt.Sprintf("2001:db8::%x/128", i) }), + }, + }, } - for _, rule := range fwcreateOpts.Rules.Inbound { - if len(rule.Addresses.IPv4) == 0 && len(rule.Addresses.IPv6) == 0 { - t.Errorf("processACL() created an inbound rule with no addresses: %+v", rule) - } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fwcreateOpts := &linodego.FirewallCreateOptions{} + err := processACL(fwcreateOpts, accept, "test", "svc", "80", tt.ips) + if err != nil { + t.Fatalf("processACL() error = %v", err) + } + + if len(fwcreateOpts.Rules.Inbound) != len(wantChunkSizes) { + t.Fatalf("processACL() created %d inbound rules, want %d", len(fwcreateOpts.Rules.Inbound), len(wantChunkSizes)) + } + + for i, rule := range fwcreateOpts.Rules.Inbound { + if len(rule.Addresses.IPv4) == 0 && len(rule.Addresses.IPv6) == 0 { + t.Errorf("processACL() created an inbound rule with no addresses: %+v", rule) + } + + if gotSize := len(rule.Addresses.IPv4) + len(rule.Addresses.IPv6); gotSize != wantChunkSizes[i] { + t.Errorf("rule %d has %d addresses, want %d", i, gotSize, wantChunkSizes[i]) + } + + if len(tt.ips.IPv4) > 0 && len(rule.Addresses.IPv6) != 0 { + t.Errorf("rule %d unexpectedly has IPv6 addresses for an IPv4-only ACL: %+v", i, rule) + } + if len(tt.ips.IPv6) > 0 && len(rule.Addresses.IPv4) != 0 { + t.Errorf("rule %d unexpectedly has IPv4 addresses for an IPv6-only ACL: %+v", i, rule) + } + } + }) } }