From 238cec347831ec11d8b2ff89b0d4d5e0c3d32047 Mon Sep 17 00:00:00 2001 From: marvin-tigera Date: Thu, 29 Jan 2026 11:32:58 +0000 Subject: [PATCH 1/2] Merge pull request #11736 from nelljerram/nftables-nat-port-range Fix rendering of NatPortRange in nftables mode (cherry picked from commit 38568836d2ac17d5a56c0dd74482bd370e3e1aeb) --- felix/fv/nat_outgoing_test.go | 44 ++++++++++++++++++++++++++++++++++ felix/iptables/actions_test.go | 2 ++ felix/nftables/actions.go | 2 +- felix/nftables/actions_test.go | 2 ++ 4 files changed, 49 insertions(+), 1 deletion(-) diff --git a/felix/fv/nat_outgoing_test.go b/felix/fv/nat_outgoing_test.go index ebd5352549a..ef88f3a633b 100644 --- a/felix/fv/nat_outgoing_test.go +++ b/felix/fv/nat_outgoing_test.go @@ -109,3 +109,47 @@ var _ = infrastructure.DatastoreDescribe("NATOutgoing rule rendering test", []ap } }) }) + +var _ = infrastructure.DatastoreDescribe("NATPortRange rendering test", []apiconfig.DatastoreType{apiconfig.EtcdV3, apiconfig.Kubernetes}, func(getInfra infrastructure.InfraFactory) { + var ( + infra infrastructure.DatastoreInfra + tc infrastructure.TopologyContainers + client client.Interface + ) + + BeforeEach(func() { + var err error + infra = getInfra() + + opts := infrastructure.DefaultTopologyOptions() + opts.IPIPMode = api.IPIPModeNever + opts.EnableIPv6 = true + + opts.ExtraEnvVars = map[string]string{ + "FELIX_NATPortRange": "32768:65535", + } + tc, client = infrastructure.StartSingleNodeTopology(opts, infra) + + ctx := context.Background() + ippool := api.NewIPPool() + ippool.Name = "nat-pool" + ippool.Spec.CIDR = "10.244.255.0/24" + ippool.Spec.NATOutgoing = true + ippool, err = client.IPPools().Create(ctx, ippool, options.SetOptions{}) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should have expected rendering", func() { + if NFTMode() { + Eventually(func() string { + output, _ := tc.Felixes[0].ExecOutput("nft", "list", "chain", "ip", "calico", "nat-cali-nat-outgoing") + return output + }, 5*time.Second, 100*time.Millisecond).Should(ContainSubstring("32768-65535")) + } else { + Eventually(func() string { + output, _ := tc.Felixes[0].ExecOutput("iptables-save", "-t", "nat") + return output + }, 5*time.Second, 100*time.Millisecond).Should(ContainSubstring("32768-65535")) + } + }) +}) diff --git a/felix/iptables/actions_test.go b/felix/iptables/actions_test.go index 28c2e8b2fda..a905fca128e 100644 --- a/felix/iptables/actions_test.go +++ b/felix/iptables/actions_test.go @@ -37,7 +37,9 @@ var _ = DescribeTable("Actions", Entry("SNATAction", environment.Features{}, SNATAction{ToAddr: "10.0.0.1"}, "--jump SNAT --to-source 10.0.0.1"), Entry("SNATAction fully random", environment.Features{SNATFullyRandom: true}, SNATAction{ToAddr: "10.0.0.1"}, "--jump SNAT --to-source 10.0.0.1 --random-fully"), Entry("MasqAction", environment.Features{}, MasqAction{}, "--jump MASQUERADE"), + Entry("MasqAction", environment.Features{}, MasqAction{ToPorts: "32768-65535"}, "--jump MASQUERADE --to-ports 32768-65535"), Entry("MasqAction", environment.Features{MASQFullyRandom: true}, MasqAction{}, "--jump MASQUERADE --random-fully"), + Entry("MasqAction", environment.Features{MASQFullyRandom: true}, MasqAction{ToPorts: "32768-65535"}, "--jump MASQUERADE --to-ports 32768-65535 --random-fully"), Entry("ClearMarkAction", environment.Features{}, ClearMarkAction{Mark: 0x1000}, "--jump MARK --set-mark 0/0x1000"), Entry("SetMarkAction", environment.Features{}, SetMarkAction{Mark: 0x1000}, "--jump MARK --set-mark 0x1000/0x1000"), Entry("SetMaskedMarkAction", environment.Features{}, SetMaskedMarkAction{ diff --git a/felix/nftables/actions.go b/felix/nftables/actions.go index 80941fa04bd..7cf81a16413 100644 --- a/felix/nftables/actions.go +++ b/felix/nftables/actions.go @@ -329,7 +329,7 @@ func (g MasqAction) ToFragment(features *environment.Features) string { } if g.ToPorts != "" { // e.g., masquerade to :1024-65535 - return fmt.Sprintf("masquerade to %s"+fullyRand, g.ToPorts) + return fmt.Sprintf("masquerade to :%s"+fullyRand, g.ToPorts) } return "masquerade" + fullyRand } diff --git a/felix/nftables/actions_test.go b/felix/nftables/actions_test.go index e6e0df2f864..5d27d60f2cd 100644 --- a/felix/nftables/actions_test.go +++ b/felix/nftables/actions_test.go @@ -37,7 +37,9 @@ var _ = DescribeTable("Actions", Entry("SNATAction", environment.Features{}, SNATAction{ToAddr: "10.0.0.1"}, "snat to 10.0.0.1"), Entry("SNATAction fully random", environment.Features{SNATFullyRandom: true}, SNATAction{ToAddr: "10.0.0.1"}, "snat to 10.0.0.1 fully-random"), Entry("MasqAction", environment.Features{}, MasqAction{}, "masquerade"), + Entry("MasqAction", environment.Features{}, MasqAction{ToPorts: "32768-65535"}, "masquerade to :32768-65535"), Entry("MasqAction", environment.Features{MASQFullyRandom: true}, MasqAction{}, "masquerade fully-random"), + Entry("MasqAction", environment.Features{MASQFullyRandom: true}, MasqAction{ToPorts: "32768-65535"}, "masquerade to :32768-65535 fully-random"), Entry("ClearMarkAction", environment.Features{}, ClearMarkAction{Mark: 0x1000}, "meta mark set mark & 0xffffefff"), Entry("SetMarkAction", environment.Features{}, SetMarkAction{Mark: 0x1000}, "meta mark set mark or 0x1000"), Entry("SetMaskedMarkAction", environment.Features{}, SetMaskedMarkAction{Mark: 0x1000, Mask: 0xf000}, "meta mark set mark & 0xffff0fff ^ 0x1000"), From e7b65bd86fcef45ee91ef07b5cb4cf8a171de2dc Mon Sep 17 00:00:00 2001 From: Nell Jerram Date: Thu, 29 Jan 2026 11:53:56 +0000 Subject: [PATCH 2/2] Extra boilerplate for FV teardown needed on this branch --- felix/fv/nat_outgoing_test.go | 33 ++++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/felix/fv/nat_outgoing_test.go b/felix/fv/nat_outgoing_test.go index ef88f3a633b..79194c4c5b9 100644 --- a/felix/fv/nat_outgoing_test.go +++ b/felix/fv/nat_outgoing_test.go @@ -112,15 +112,17 @@ var _ = infrastructure.DatastoreDescribe("NATOutgoing rule rendering test", []ap var _ = infrastructure.DatastoreDescribe("NATPortRange rendering test", []apiconfig.DatastoreType{apiconfig.EtcdV3, apiconfig.Kubernetes}, func(getInfra infrastructure.InfraFactory) { var ( - infra infrastructure.DatastoreInfra - tc infrastructure.TopologyContainers - client client.Interface + infra infrastructure.DatastoreInfra + tc infrastructure.TopologyContainers + client client.Interface + dumpedDiags bool ) BeforeEach(func() { var err error infra = getInfra() + dumpedDiags = false opts := infrastructure.DefaultTopologyOptions() opts.IPIPMode = api.IPIPModeNever opts.EnableIPv6 = true @@ -139,6 +141,31 @@ var _ = infrastructure.DatastoreDescribe("NATPortRange rendering test", []apicon Expect(err).NotTo(HaveOccurred()) }) + // Utility function to dump diags if the test failed. Should be called in the inner-most + // AfterEach() to dump diags before the test is torn down. Only the first call for a given + // test has any effect. + dumpDiags := func() { + if !CurrentGinkgoTestDescription().Failed || dumpedDiags { + return + } + if NFTMode() { + logNFTDiags(tc.Felixes[0]) + } else { + iptSave, err := tc.Felixes[0].ExecOutput("iptables-save", "-c") + if err == nil { + log.Info("iptables-save:\n" + iptSave) + } + } + dumpedDiags = true + infra.DumpErrorData() + } + + AfterEach(func() { + dumpDiags() + tc.Stop() + infra.Stop() + }) + It("should have expected rendering", func() { if NFTMode() { Eventually(func() string {