Skip to content

Commit 3ee0933

Browse files
eufrictionjoamaki
eufriction
authored and
joamaki
committed
loadbalancer: prefer exact port match in ForPort over wildcard
ProxyRedirects.ForPort prefers an exact port match over a wildcard (empty Ports) fallback. A single pass returns immediately on an exact match and tracks the first wildcard to return at the end. Signed-off-by: Martin Lindberg <github@eufriction.com>
1 parent 157c826 commit 3ee0933

4 files changed

Lines changed: 381 additions & 1 deletion

File tree

‎pkg/ciliumenvoyconfig/testdata/multi-port-redirect.txtar‎

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
# Test handling of CiliumEnvoyConfig with multiple proxy redirects to different
2+
# ports on the same service. This validates the fix for the bug where the second
3+
# ServiceListener entry for the same service would overwrite the first, leaving
4+
# one port without an L7 redirect in the BPF LB map.
5+
#
6+
# Scenario: A Gateway with two listeners (HTTP on port 80, HTTPS on port 443)
7+
# targeting the same backend service. Each listener should produce a separate
8+
# ProxyRedirect entry, and both frontends should get L7 proxy redirection.
9+
10+
hive start
11+
12+
# Set up the service and endpoints
13+
k8s/add service.yaml endpointslice.yaml
14+
15+
# Validate initial state (no proxy redirect yet)
16+
db/cmp services services.table
17+
db/cmp frontends frontends.table
18+
19+
# Add the CiliumEnvoyConfig with two service entries for the same service,
20+
# each targeting a different port.
21+
k8s/add cec.yaml
22+
db/cmp ciliumenvoyconfigs cec.table
23+
24+
# Both ports should now have proxy redirects.
25+
db/cmp services services_redirected.table
26+
27+
# Check BPF maps. Port 80 gets proxy port 1000, port 443 gets proxy port 2000.
28+
lb/maps-dump lbmaps.out
29+
* grep '10.96.50.104:80/TCP.*L7Proxy=1000' lbmaps.out
30+
* grep '10.96.50.104:443/TCP.*L7Proxy=2000' lbmaps.out
31+
32+
# Remove the CEC and verify redirects are cleared.
33+
k8s/delete cec.yaml
34+
db/cmp services services.table
35+
36+
# ---------------------------------------------
37+
38+
-- services.table --
39+
Name Flags
40+
test/gateway
41+
42+
-- services_redirected.table --
43+
Name Flags
44+
test/gateway ProxyRedirects=[1000 (ports: [80]), 2000 (ports: [443])]
45+
46+
-- frontends.table --
47+
Address Type ServiceName PortName Status Backends
48+
10.96.50.104:80/TCP ClusterIP test/gateway http Done 10.244.1.1:8080/TCP
49+
10.96.50.104:443/TCP ClusterIP test/gateway https Done 10.244.1.1:8443/TCP
50+
51+
-- cec.table --
52+
Name Services
53+
test/gateway-cec test/gateway, test/gateway
54+
55+
-- cec.yaml --
56+
apiVersion: cilium.io/v2
57+
kind: CiliumEnvoyConfig
58+
metadata:
59+
name: gateway-cec
60+
namespace: test
61+
spec:
62+
services:
63+
- name: gateway
64+
namespace: test
65+
listener: http-listener
66+
ports:
67+
- 80
68+
- name: gateway
69+
namespace: test
70+
listener: https-listener
71+
ports:
72+
- 443
73+
resources:
74+
- "@type": type.googleapis.com/envoy.config.listener.v3.Listener
75+
name: http-listener
76+
address:
77+
socket_address:
78+
address: 127.0.0.1
79+
port_value: 1000
80+
- "@type": type.googleapis.com/envoy.config.listener.v3.Listener
81+
name: https-listener
82+
address:
83+
socket_address:
84+
address: 127.0.0.1
85+
port_value: 2000
86+
87+
-- service.yaml --
88+
apiVersion: v1
89+
kind: Service
90+
metadata:
91+
name: gateway
92+
namespace: test
93+
uid: b59fe99c-3564-4754-acc4-780f2331a49b
94+
spec:
95+
clusterIP: 10.96.50.104
96+
clusterIPs:
97+
- 10.96.50.104
98+
ports:
99+
- name: http
100+
port: 80
101+
protocol: TCP
102+
targetPort: 8080
103+
- name: https
104+
port: 443
105+
protocol: TCP
106+
targetPort: 8443
107+
selector:
108+
app: gateway
109+
type: ClusterIP
110+
status: {}
111+
112+
-- endpointslice.yaml --
113+
apiVersion: discovery.k8s.io/v1
114+
kind: EndpointSlice
115+
metadata:
116+
labels:
117+
kubernetes.io/service-name: gateway
118+
name: gateway-eps1
119+
namespace: test
120+
uid: e1f517f6-ab88-4c76-9bd0-4906a17cdd75
121+
addressType: IPv4
122+
endpoints:
123+
- addresses:
124+
- 10.244.1.1
125+
conditions:
126+
ready: true
127+
serving: true
128+
terminating: false
129+
nodeName: testnode
130+
ports:
131+
- name: http
132+
port: 8080
133+
protocol: TCP
134+
- name: https
135+
port: 8443
136+
protocol: TCP

‎pkg/loadbalancer/proxy_redirects_test.go‎

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
// Copyright Authors of Cilium
3+
4+
package loadbalancer
5+
6+
import (
7+
"testing"
8+
9+
"github.com/stretchr/testify/assert"
10+
)
11+
12+
func TestProxyRedirects_ForPort(t *testing.T) {
13+
tests := []struct {
14+
name string
15+
redirects ProxyRedirects
16+
port uint16
17+
want *ProxyRedirect
18+
}{
19+
{
20+
name: "nil redirects",
21+
redirects: nil,
22+
port: 80,
23+
want: nil,
24+
},
25+
{
26+
name: "empty redirects",
27+
redirects: ProxyRedirects{},
28+
port: 80,
29+
want: nil,
30+
},
31+
{
32+
name: "exact port match",
33+
redirects: ProxyRedirects{
34+
{ProxyPort: 1000, Ports: []uint16{80}},
35+
{ProxyPort: 2000, Ports: []uint16{443}},
36+
},
37+
port: 443,
38+
want: &ProxyRedirect{ProxyPort: 2000, Ports: []uint16{443}},
39+
},
40+
{
41+
name: "wildcard match when no exact match",
42+
redirects: ProxyRedirects{
43+
{ProxyPort: 1000},
44+
},
45+
port: 8080,
46+
want: &ProxyRedirect{ProxyPort: 1000},
47+
},
48+
{
49+
name: "exact match preferred over wildcard (wildcard first)",
50+
redirects: ProxyRedirects{
51+
{ProxyPort: 1000}, // wildcard
52+
{ProxyPort: 2000, Ports: []uint16{80}},
53+
},
54+
port: 80,
55+
want: &ProxyRedirect{ProxyPort: 2000, Ports: []uint16{80}},
56+
},
57+
{
58+
name: "exact match preferred over wildcard (exact first)",
59+
redirects: ProxyRedirects{
60+
{ProxyPort: 2000, Ports: []uint16{80}},
61+
{ProxyPort: 1000}, // wildcard
62+
},
63+
port: 80,
64+
want: &ProxyRedirect{ProxyPort: 2000, Ports: []uint16{80}},
65+
},
66+
{
67+
name: "wildcard fallback when port not in any exact set",
68+
redirects: ProxyRedirects{
69+
{ProxyPort: 1000}, // wildcard
70+
{ProxyPort: 2000, Ports: []uint16{443}},
71+
},
72+
port: 8080,
73+
want: &ProxyRedirect{ProxyPort: 1000},
74+
},
75+
{
76+
name: "no match when port not in any set and no wildcard",
77+
redirects: ProxyRedirects{
78+
{ProxyPort: 1000, Ports: []uint16{80}},
79+
{ProxyPort: 2000, Ports: []uint16{443}},
80+
},
81+
port: 8080,
82+
want: nil,
83+
},
84+
{
85+
name: "multi-port redirect matches any listed port",
86+
redirects: ProxyRedirects{
87+
{ProxyPort: 1000, Ports: []uint16{80, 443}},
88+
},
89+
port: 443,
90+
want: &ProxyRedirect{ProxyPort: 1000, Ports: []uint16{80, 443}},
91+
},
92+
{
93+
name: "first wildcard wins when multiple wildcards exist",
94+
redirects: ProxyRedirects{
95+
{ProxyPort: 1000},
96+
{ProxyPort: 2000},
97+
},
98+
port: 80,
99+
want: &ProxyRedirect{ProxyPort: 1000},
100+
},
101+
{
102+
name: "multiple redirects different ports",
103+
redirects: ProxyRedirects{
104+
{ProxyPort: 1000, Ports: []uint16{80}},
105+
{ProxyPort: 2000, Ports: []uint16{443}},
106+
{ProxyPort: 3000, Ports: []uint16{8080}},
107+
},
108+
port: 8080,
109+
want: &ProxyRedirect{ProxyPort: 3000, Ports: []uint16{8080}},
110+
},
111+
}
112+
113+
for _, tt := range tests {
114+
t.Run(tt.name, func(t *testing.T) {
115+
got := tt.redirects.ForPort(tt.port)
116+
if tt.want == nil {
117+
assert.Nil(t, got)
118+
} else {
119+
assert.Equal(t, tt.want, got)
120+
}
121+
})
122+
}
123+
}
124+
125+
func TestProxyRedirects_Redirects(t *testing.T) {
126+
redirects := ProxyRedirects{
127+
{ProxyPort: 1000, Ports: []uint16{80}},
128+
{ProxyPort: 2000, Ports: []uint16{443}},
129+
}
130+
131+
assert.True(t, redirects.Redirects(80))
132+
assert.True(t, redirects.Redirects(443))
133+
assert.False(t, redirects.Redirects(8080))
134+
}
135+
136+
func TestProxyRedirects_Empty(t *testing.T) {
137+
assert.True(t, ProxyRedirects(nil).Empty())
138+
assert.True(t, ProxyRedirects{}.Empty())
139+
assert.False(t, ProxyRedirects{{ProxyPort: 1000}}.Empty())
140+
}
141+
142+
func TestProxyRedirects_Equal(t *testing.T) {
143+
a := ProxyRedirects{
144+
{ProxyPort: 1000, Ports: []uint16{80}},
145+
{ProxyPort: 2000, Ports: []uint16{443}},
146+
}
147+
b := ProxyRedirects{
148+
{ProxyPort: 1000, Ports: []uint16{80}},
149+
{ProxyPort: 2000, Ports: []uint16{443}},
150+
}
151+
c := ProxyRedirects{
152+
{ProxyPort: 1000, Ports: []uint16{80}},
153+
}
154+
155+
assert.True(t, a.Equal(b))
156+
assert.False(t, a.Equal(c))
157+
assert.True(t, ProxyRedirects(nil).Equal(nil))
158+
}
159+
160+
func TestProxyRedirects_String(t *testing.T) {
161+
assert.Empty(t, ProxyRedirects(nil).String())
162+
assert.Equal(t, "1000 (ports: [80])", ProxyRedirects{{ProxyPort: 1000, Ports: []uint16{80}}}.String())
163+
assert.Equal(t, "[1000 (ports: [80]), 2000 (ports: [443])]",
164+
ProxyRedirects{
165+
{ProxyPort: 1000, Ports: []uint16{80}},
166+
{ProxyPort: 2000, Ports: []uint16{443}},
167+
}.String())
168+
}

‎pkg/loadbalancer/reconciler/bpf_reconciler_test.go‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -624,6 +624,81 @@ var proxyTestCases = []testCase{
624624
nil,
625625
false,
626626
),
627+
newTestCase(
628+
"L7Proxy_distinct_port80",
629+
630+
func(svc *loadbalancer.Service, fe *loadbalancer.Frontend) (delete bool, bes []loadbalancer.Backend) {
631+
fe.Type = ClusterIP
632+
fe.Address = autoAddr
633+
fe.ServicePort = 80
634+
635+
svc.ProxyRedirects = loadbalancer.ProxyRedirects{
636+
{ProxyPort: 0x0a0a, Ports: []uint16{80}},
637+
{ProxyPort: 0x0b0b, Ports: []uint16{443}},
638+
}
639+
return false, []loadbalancer.Backend{baseBackend}
640+
},
641+
[]maps.MapDump{
642+
"BE: ID=2 ADDR=10.1.0.1:80/TCP STATE=active",
643+
"REV: ID=2 ADDR=<auto>",
644+
"SVC: ID=0 ADDR=<auto>/ANY SLOT=0 LBALG=undef AFFTimeout=0 COUNT=0 QCOUNT=0 FLAGS=ClusterIP+non-routable",
645+
"SVC: ID=2 ADDR=<auto>/TCP SLOT=0 L7Proxy=2570 COUNT=1 QCOUNT=0 FLAGS=ClusterIP+Local+InternalLocal+non-routable+l7-load-balancer",
646+
"SVC: ID=2 ADDR=<auto>/TCP SLOT=1 BEID=2 COUNT=0 QCOUNT=0 FLAGS=ClusterIP+Local+InternalLocal+non-routable+l7-load-balancer",
647+
},
648+
nil,
649+
false,
650+
),
651+
newTestCase(
652+
"L7Proxy_distinct_port443",
653+
654+
func(svc *loadbalancer.Service, fe *loadbalancer.Frontend) (delete bool, bes []loadbalancer.Backend) {
655+
fe.Type = ClusterIP
656+
fe.Address = loadbalancer.NewL3n4Addr(loadbalancer.TCP, types.MustParseAddrCluster("10.0.0.2"), 443, loadbalancer.ScopeExternal)
657+
fe.ServicePort = 443
658+
659+
svc.ProxyRedirects = loadbalancer.ProxyRedirects{
660+
{ProxyPort: 0x0a0a, Ports: []uint16{80}},
661+
{ProxyPort: 0x0b0b, Ports: []uint16{443}},
662+
}
663+
return false, []loadbalancer.Backend{baseBackend}
664+
},
665+
[]maps.MapDump{
666+
"BE: ID=2 ADDR=10.1.0.1:80/TCP STATE=active",
667+
"REV: ID=2 ADDR=<auto>",
668+
"REV: ID=3 ADDR=10.0.0.2:443",
669+
"SVC: ID=0 ADDR=10.0.0.2:0/ANY SLOT=0 LBALG=undef AFFTimeout=0 COUNT=0 QCOUNT=0 FLAGS=ClusterIP+non-routable",
670+
"SVC: ID=0 ADDR=<auto>/ANY SLOT=0 LBALG=undef AFFTimeout=0 COUNT=0 QCOUNT=0 FLAGS=ClusterIP+non-routable",
671+
"SVC: ID=2 ADDR=<auto>/TCP SLOT=0 L7Proxy=2570 COUNT=1 QCOUNT=0 FLAGS=ClusterIP+Local+InternalLocal+non-routable+l7-load-balancer",
672+
"SVC: ID=2 ADDR=<auto>/TCP SLOT=1 BEID=2 COUNT=0 QCOUNT=0 FLAGS=ClusterIP+Local+InternalLocal+non-routable+l7-load-balancer",
673+
"SVC: ID=3 ADDR=10.0.0.2:443/TCP SLOT=0 L7Proxy=2827 COUNT=1 QCOUNT=0 FLAGS=ClusterIP+Local+InternalLocal+non-routable+l7-load-balancer",
674+
"SVC: ID=3 ADDR=10.0.0.2:443/TCP SLOT=1 BEID=2 COUNT=0 QCOUNT=0 FLAGS=ClusterIP+Local+InternalLocal+non-routable+l7-load-balancer",
675+
},
676+
nil,
677+
false,
678+
),
679+
newTestCase(
680+
"L7Proxy_distinct_cleanup",
681+
deleteFrontend(autoAddr, ClusterIP),
682+
[]maps.MapDump{
683+
"BE: ID=2 ADDR=10.1.0.1:80/TCP STATE=active",
684+
"REV: ID=3 ADDR=10.0.0.2:443",
685+
"SVC: ID=0 ADDR=10.0.0.2:0/ANY SLOT=0 LBALG=undef AFFTimeout=0 COUNT=0 QCOUNT=0 FLAGS=ClusterIP+non-routable",
686+
"SVC: ID=3 ADDR=10.0.0.2:443/TCP SLOT=0 L7Proxy=2827 COUNT=1 QCOUNT=0 FLAGS=ClusterIP+Local+InternalLocal+non-routable+l7-load-balancer",
687+
"SVC: ID=3 ADDR=10.0.0.2:443/TCP SLOT=1 BEID=2 COUNT=0 QCOUNT=0 FLAGS=ClusterIP+Local+InternalLocal+non-routable+l7-load-balancer",
688+
},
689+
nil,
690+
false,
691+
),
692+
newTestCase(
693+
"L7Proxy_distinct_cleanup_2",
694+
deleteFrontend(
695+
loadbalancer.NewL3n4Addr(loadbalancer.TCP, types.MustParseAddrCluster("10.0.0.2"), 443, loadbalancer.ScopeExternal),
696+
ClusterIP,
697+
),
698+
[]maps.MapDump{},
699+
nil,
700+
false,
701+
),
627702
}
628703

629704
var extraFrontendInternal = loadbalancer.NewL3n4Addr(

‎pkg/loadbalancer/service.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,8 @@ func (p ProxyRedirects) Equal(other ProxyRedirects) bool {
215215
}
216216

217217
// ForPort finds the ProxyRedirect that matches the given frontend port.
218-
// Returns nil if no match is found.
218+
// It prefers an exact port match over a wildcard (Ports is empty) match.
219+
// Returns nil if no match is found. Nil entries are not expected.
219220
func (p ProxyRedirects) ForPort(port uint16) *ProxyRedirect {
220221
var wildcard *ProxyRedirect
221222
for i := range p {

0 commit comments

Comments
 (0)