Skip to content

OCPBUGS-64697: [release-4.20] Referencing pod named ports within a service results in bad DNAT rules containing tcp/0 target port #2844

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

Merged
openshift-merge-bot merged 5 commits into from
Nov 14, 2025
Merged

OCPBUGS-64697: [release-4.20] Referencing pod named ports within a service results in bad DNAT rules containing tcp/0 target port #2844

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8c50a8d
Fix named port handling in externalTrafficPolicy Local services
andreaskaris Sep 23, 2025
9bab81f
Extends unit test coverage for named port handling in ETP local
andreaskaris Sep 23, 2025
87b946b
E2E service: Move checkNumberOf.. to util.go
andreaskaris Sep 26, 2025
b2b17a2
E2E service: Fix potential flake in conn to an ext IP using src port
andreaskaris Sep 26, 2025
a3a4827
E2E service: Add test for named port handling with ETP=Local services
andreaskaris Sep 23, 2025
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
29 changes: 22 additions & 7 deletions go-controller/pkg/node/gateway_iptables.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,15 +183,30 @@ func computeProbability(n, i int) string {
return fmt.Sprintf("%0.10f", 1.0/float64(n-i+1))
}

func generateIPTRulesForLoadBalancersWithoutNodePorts(svcPort corev1.ServicePort, externalIP string, localEndpoints []string) []nodeipt.Rule {
iptRules := make([]nodeipt.Rule, 0, len(localEndpoints))
// generateIPTRulesForLoadBalancersWithoutNodePorts generates iptables DNAT rules for load balancer services
// without NodePort allocation. It performs statistical load balancing between endpoints via iptables.
func generateIPTRulesForLoadBalancersWithoutNodePorts(svcPort corev1.ServicePort, externalIP string, localEndpoints util.PortToLBEndpoints) []nodeipt.Rule {
if len(localEndpoints) == 0 {
// either its smart nic mode; etp&itp not implemented, OR
// fetching endpointSlices error-ed out prior to reaching here so nothing to do
return iptRules
return []nodeipt.Rule{}
}
numLocalEndpoints := len(localEndpoints)
for i, ip := range localEndpoints {

// Get the endpoints for the port key.
// svcPortKey is of format e.g. "TCP/my-port-name" or "TCP/" if name is empty
// (is the case when only a single ServicePort is defined on this service).
svcPortKey := util.GetServicePortKey(svcPort.Protocol, svcPort.Name)
lbEndpoints := localEndpoints[svcPortKey]

// Get IPv4 or IPv6 IPs, depending on the type of the service's external IP.
destinations := lbEndpoints.GetV4Destinations()
if utilnet.IsIPv6String(externalIP) {
destinations = lbEndpoints.GetV6Destinations()
}

numLocalEndpoints := len(destinations)
iptRules := make([]nodeipt.Rule, 0, numLocalEndpoints)
for i, destination := range destinations {
iptRules = append([]nodeipt.Rule{
{
Table: "nat",
Expand All @@ -201,7 +216,7 @@ func generateIPTRulesForLoadBalancersWithoutNodePorts(svcPort corev1.ServicePort
"-d", externalIP,
"--dport", fmt.Sprintf("%v", svcPort.Port),
"-j", "DNAT",
"--to-destination", util.JoinHostPortInt32(ip, int32(svcPort.TargetPort.IntValue())),
"--to-destination", util.JoinHostPortInt32(destination.IP, destination.Port),
"-m", "statistic",
"--mode", "random",
"--probability", computeProbability(numLocalEndpoints, i+1),
Expand Down Expand Up @@ -476,7 +491,7 @@ func recreateIPTRules(table, chain string, keepIPTRules []nodeipt.Rule) error {
// case3: if svcHasLocalHostNetEndPnt and svcTypeIsITPLocal, rule that redirects clusterIP traffic to host targetPort is added.
//
// if !svcHasLocalHostNetEndPnt and svcTypeIsITPLocal, rule that marks clusterIP traffic to steer it to ovn-k8s-mp0 is added.
func getGatewayIPTRules(service *corev1.Service, localEndpoints []string, svcHasLocalHostNetEndPnt bool) []nodeipt.Rule {
func getGatewayIPTRules(service *corev1.Service, localEndpoints util.PortToLBEndpoints, svcHasLocalHostNetEndPnt bool) []nodeipt.Rule {
rules := make([]nodeipt.Rule, 0)
clusterIPs := util.GetClusterIPs(service)
svcTypeIsETPLocal := util.ServiceExternalTrafficPolicyLocal(service)
Expand Down
196 changes: 181 additions & 15 deletions go-controller/pkg/node/gateway_localnet_linux_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,7 @@ var _ = Describe("Node Operations", func() {
config.Gateway.Mode = config.GatewayModeLocal
epPortName := "https"
epPortValue := int32(443)
epPortProtocol := corev1.ProtocolTCP
service := *newService("service1", "namespace1", "10.129.0.2",
[]corev1.ServicePort{
{
Expand All @@ -579,8 +580,9 @@ var _ = Describe("Node Operations", func() {
Addresses: []string{"10.244.0.3"},
}
epPort1 := discovery.EndpointPort{
Name: &epPortName,
Port: &epPortValue,
Name: &epPortName,
Port: &epPortValue,
Protocol: &epPortProtocol,
}
// endpointSlice.Endpoints is ovn-networked so this will
// come under !hasLocalHostNetEp case
Expand Down Expand Up @@ -878,9 +880,11 @@ var _ = Describe("Node Operations", func() {
Cmd: "ovs-ofctl show ",
Err: fmt.Errorf("deliberate error to fall back to output:LOCAL"),
})
svcPortName := "http"
service := *newServiceWithoutNodePortAllocation("service1", "namespace1", "10.129.0.2",
[]corev1.ServicePort{
{
Name: svcPortName,
Protocol: corev1.ProtocolTCP,
Port: int32(80),
TargetPort: intstr.FromInt(int(int32(8080))),
Expand Down Expand Up @@ -910,11 +914,12 @@ var _ = Describe("Node Operations", func() {
Addresses: []string{"10.244.0.4"},
NodeName: &fakeNodeName,
}
epPortName := "http"
epPortValue := int32(8080)
epPortProtocol := corev1.ProtocolTCP
epPort1 := discovery.EndpointPort{
Name: &epPortName,
Port: &epPortValue,
Name: &svcPortName,
Port: &epPortValue,
Protocol: &epPortProtocol,
}
// endpointSlice.Endpoints is ovn-networked so this will
// come under !hasLocalHostNetEp case
Expand Down Expand Up @@ -994,6 +999,155 @@ var _ = Describe("Node Operations", func() {
Expect(app.Run([]string{app.Name})).To(Succeed())
})

It("inits iptables rules and openflows with named port and AllocateLoadBalancerNodePorts=False, ETP=local, LGW mode", func() {
app.Action = func(*cli.Context) error {
minNFakeCommands := nInitialFakeCommands + 1
fExec.AddRepeatedFakeCmd(&ovntest.ExpectedCmd{
Cmd: "ovs-ofctl show ",
}, minNFakeCommands)

config.Gateway.Mode = config.GatewayModeLocal
svcPortName := "https-port"
svcPortValue := int32(8080)
svcProtocol := corev1.ProtocolTCP
svcTargetPortName := "https-target"
svcAllocateLoadBalancerNodePorts := false
svcStatusIP := "192.168.0.10"
svcStatusIPMode := corev1.LoadBalancerIPModeVIP

epPortValue := int32(443)
epPortProtocol := corev1.ProtocolTCP

nodeName := "node"

service := *newService("service1", "namespace1", "10.129.0.2",
[]corev1.ServicePort{
{
Name: svcPortName,
Port: svcPortValue,
Protocol: svcProtocol,
TargetPort: intstr.FromString(svcTargetPortName),
},
},
corev1.ServiceTypeLoadBalancer,
nil,
corev1.ServiceStatus{
LoadBalancer: corev1.LoadBalancerStatus{
Ingress: []corev1.LoadBalancerIngress{
{
IP: svcStatusIP,
IPMode: &svcStatusIPMode,
},
},
},
},
true, false,
)
service.Spec.AllocateLoadBalancerNodePorts = &svcAllocateLoadBalancerNodePorts
ep1 := discovery.Endpoint{
Addresses: []string{"10.244.0.3"},
NodeName: &nodeName,
}
ep2 := discovery.Endpoint{
Addresses: []string{"10.244.0.4"},
NodeName: &nodeName,
}
epPort1 := discovery.EndpointPort{
Name: &svcPortName,
Port: &epPortValue,
Protocol: &epPortProtocol,
}
// endpointSlice.Endpoints is ovn-networked so this will
// come under !hasLocalHostNetEp case
endpointSlice := *newEndpointSlice(
"service1",
"namespace1",
[]discovery.Endpoint{ep1, ep2},
[]discovery.EndpointPort{epPort1},
)

stopChan := make(chan struct{})
fakeClient := util.GetOVNClientset(&service, &endpointSlice).GetNodeClientset()
wf, err := factory.NewNodeWatchFactory(fakeClient, nodeName)
Expect(err).ToNot(HaveOccurred())
Expect(wf.Start()).To(Succeed())
defer func() {
close(stopChan)
wf.Shutdown()
}()

fNPW.watchFactory = wf
Expect(startNodePortWatcher(fNPW, fakeClient)).To(Succeed())

expectedTables := map[string]util.FakeTable{
"nat": {
"PREROUTING": []string{
"-j OVN-KUBE-ETP",
"-j OVN-KUBE-EXTERNALIP",
"-j OVN-KUBE-NODEPORT",
},
"OUTPUT": []string{
"-j OVN-KUBE-EXTERNALIP",
"-j OVN-KUBE-NODEPORT",
"-j OVN-KUBE-ITP",
},
"OVN-KUBE-NODEPORT": []string{},
"OVN-KUBE-EXTERNALIP": []string{
fmt.Sprintf("-p %s -d %s --dport %d -j DNAT --to-destination %s:%v",
service.Spec.Ports[0].Protocol,
service.Status.LoadBalancer.Ingress[0].IP,
service.Spec.Ports[0].Port,
service.Spec.ClusterIP,
service.Spec.Ports[0].Port),
},
"OVN-KUBE-ETP": []string{
fmt.Sprintf("-p %s -d %s --dport %d -j DNAT --to-destination %s:%d -m statistic --mode random --probability 0.5000000000",
service.Spec.Ports[0].Protocol,
service.Status.LoadBalancer.Ingress[0].IP,
service.Spec.Ports[0].Port,
endpointSlice.Endpoints[0].Addresses[0],
*endpointSlice.Ports[0].Port),
fmt.Sprintf("-p %s -d %s --dport %d -j DNAT --to-destination %s:%d -m statistic --mode random --probability 1.0000000000",
service.Spec.Ports[0].Protocol,
service.Status.LoadBalancer.Ingress[0].IP,
service.Spec.Ports[0].Port,
endpointSlice.Endpoints[1].Addresses[0],
*endpointSlice.Ports[0].Port),
},
"OVN-KUBE-ITP": []string{},
},
"filter": {},
"mangle": {
"OUTPUT": []string{
"-j OVN-KUBE-ITP",
},
"OVN-KUBE-ITP": []string{},
},
}

f4 := iptV4.(*util.FakeIPTables)
err = f4.MatchState(expectedTables, nil)
Expect(err).NotTo(HaveOccurred())

expectedNFT := getBaseNFTRules(types.K8sMgmtIntfName)
expectedNFT += fmt.Sprintf("add element inet ovn-kubernetes mgmtport-no-snat-services-v4 { %s . tcp . %v }\n"+
"add element inet ovn-kubernetes mgmtport-no-snat-services-v4 { %s . tcp . %v }\n",
endpointSlice.Endpoints[1].Addresses[0],
*endpointSlice.Ports[0].Port,
endpointSlice.Endpoints[0].Addresses[0],
*endpointSlice.Ports[0].Port)
err = nodenft.MatchNFTRules(expectedNFT, nft.Dump())
Expect(err).NotTo(HaveOccurred())

flows := fNPW.ofm.getFlowsByKey("NodePort_namespace1_service1_tcp_31111")
Expect(flows).To(BeNil())

return nil
}
err := app.Run([]string{app.Name})
Expect(err).NotTo(HaveOccurred())
})

It("inits iptables rules and openflows with LoadBalancer where ETP=cluster, LGW mode", func() {
app.Action = func(*cli.Context) error {
externalIP := "1.1.1.1"
Expand Down Expand Up @@ -2092,6 +2246,7 @@ var _ = Describe("Node Operations", func() {
config.Gateway.Mode = config.GatewayModeLocal
epPortName := "https"
epPortValue := int32(443)
epPortProtocol := corev1.ProtocolTCP
service := *newService("service1", "namespace1", "10.129.0.2",
[]corev1.ServicePort{
{
Expand All @@ -2109,8 +2264,9 @@ var _ = Describe("Node Operations", func() {
Addresses: []string{"10.244.0.3"},
}
epPort1 := discovery.EndpointPort{
Name: &epPortName,
Port: &epPortValue,
Name: &epPortName,
Port: &epPortValue,
Protocol: &epPortProtocol,
}
// endpointSlice.Endpoints is ovn-networked so this will come
// under !hasLocalHostNetEp case
Expand Down Expand Up @@ -2231,6 +2387,7 @@ var _ = Describe("Node Operations", func() {
config.Gateway.Mode = config.GatewayModeShared
epPortName := "https"
epPortValue := int32(443)
epPortProtocol := corev1.ProtocolTCP
service := *newService("service1", "namespace1", "10.129.0.2",
[]corev1.ServicePort{
{
Expand All @@ -2249,8 +2406,9 @@ var _ = Describe("Node Operations", func() {
Addresses: []string{"10.244.0.3"},
}
epPort1 := discovery.EndpointPort{
Name: &epPortName,
Port: &epPortValue,
Name: &epPortName,
Port: &epPortValue,
Protocol: &epPortProtocol,
}
// endpointSlice.Endpoints is ovn-networked so this will come
// under !hasLocalHostNetEp case
Expand Down Expand Up @@ -2376,9 +2534,11 @@ var _ = Describe("Node Operations", func() {
outport := int32(443)
epPortName := "https"
epPortValue := int32(443)
epPortProtocol := corev1.ProtocolTCP
service := *newService("service1", "namespace1", "10.129.0.2",
[]corev1.ServicePort{
{
Name: epPortName,
NodePort: int32(31111),
Protocol: corev1.ProtocolTCP,
Port: int32(8080),
Expand All @@ -2396,8 +2556,9 @@ var _ = Describe("Node Operations", func() {
NodeName: &fakeNodeName,
}
epPort1 := discovery.EndpointPort{
Name: &epPortName,
Port: &epPortValue,
Name: &epPortName,
Port: &epPortValue,
Protocol: &epPortProtocol,
}
// endpointSlice.Endpoints is ovn-networked so this will
// come under !hasLocalHostNetEp case
Expand Down Expand Up @@ -2522,6 +2683,7 @@ var _ = Describe("Node Operations", func() {
config.Gateway.Mode = config.GatewayModeShared
epPortName := "https"
epPortValue := int32(443)
epPortProtocol := corev1.ProtocolTCP
service := *newService("service1", "namespace1", "10.129.0.2",
[]corev1.ServicePort{
{
Expand All @@ -2539,8 +2701,9 @@ var _ = Describe("Node Operations", func() {
Addresses: []string{"10.244.0.3"},
}
epPort1 := discovery.EndpointPort{
Name: &epPortName,
Port: &epPortValue,
Name: &epPortName,
Port: &epPortValue,
Protocol: &epPortProtocol,
}
// endpointSlice.Endpoints is ovn-networked so this will
// come under !hasLocalHostNetEp case
Expand Down Expand Up @@ -2665,9 +2828,11 @@ var _ = Describe("Node Operations", func() {
config.Gateway.Mode = config.GatewayModeLocal
epPortName := "https"
outport := int32(443)
epPortProtocol := corev1.ProtocolTCP
service := *newService("service1", "namespace1", "10.129.0.2",
[]corev1.ServicePort{
{
Name: epPortName,
NodePort: int32(31111),
Protocol: corev1.ProtocolTCP,
Port: int32(8080),
Expand All @@ -2684,8 +2849,9 @@ var _ = Describe("Node Operations", func() {
NodeName: &fakeNodeName,
}
epPort1 := discovery.EndpointPort{
Name: &epPortName,
Port: &outport,
Name: &epPortName,
Port: &outport,
Protocol: &epPortProtocol,
}
// endpointSlice.Endpoints is host-networked so this will
// come under hasLocalHostNetEp case
Expand Down
Loading