From db0993902c08447f6d760362d9898cdb44a55ccb Mon Sep 17 00:00:00 2001 From: David Bond Date: Thu, 5 Nov 2026 17:47:48 +0000 Subject: [PATCH] cmd/k8s-operator: raise the egress ProxyGroup port limit to 20000 This commit widens the egress match port range from [10000, 11000) to [10000, 30000). The old limit of 1000 ports was arbitrary, and users with many egress Services reach it. The new range ends below the Linux ephemeral port range, so match ports do not collide with ports that the proxy itself uses. The port count also no longer includes the health check port, which lowered the real limit by one. A new test covers allocation up to the limit. Fixes #22195 Change-Id: Ie20feb2744b40dce052d3732f9f486b1bff31b32 Signed-off-by: David Bond --- cmd/k8s-operator/egress-services.go | 26 ++++-- cmd/k8s-operator/egress-services_test.go | 113 +++++++++++++++++++++++ 2 files changed, 129 insertions(+), 10 deletions(-) diff --git a/cmd/k8s-operator/egress-services.go b/cmd/k8s-operator/egress-services.go index 8bfccabae..dd99f0dd7 100644 --- a/cmd/k8s-operator/egress-services.go +++ b/cmd/k8s-operator/egress-services.go @@ -53,13 +53,14 @@ const ( labelSvcType = "tailscale.com/svc-type" // ingress or egress typeEgress = "egress" - // maxPorts is the maximum number of ports that can be exposed on a - // container. In practice this will be ports in range [10000 - 11000). The - // high range should make it easier to distinguish container ports from - // the tailnet target ports for debugging purposes (i.e when reading - // netfilter rules). The limit of 1000 is somewhat arbitrary, the - // assumption is that this would not be hit in practice. - maxPorts = 1000 + // minMatchPort and maxPorts define the range of match ports that can be + // exposed on a container: [minMatchPort, minMatchPort+maxPorts). The high + // range should make it easier to distinguish container ports from the + // tailnet target ports for debugging purposes (i.e when reading netfilter + // rules). The range ends below the Linux ephemeral port range (32768+), so + // that match ports do not collide with ports that the proxy itself uses. + minMatchPort = 10000 + maxPorts = 20000 indexEgressProxyGroup = ".metadata.annotations.egress-proxy-group" @@ -480,6 +481,11 @@ func (esr *egressSvcsReconciler) usedPortsForPG(ctx context.Context, pg string) usedPorts := sets.New[int32]() for _, s := range svcList.Items { for _, p := range s.Spec.Ports { + // The health check port targets the proxy's own health check + // endpoint, not a match port. + if p.Name == tsHealthCheckPortName { + continue + } usedPorts.Insert(p.TargetPort.IntVal) } } @@ -642,13 +648,13 @@ func svcNameBase(s string) string { } } -// unusedPort returns a port in range [10000 - 11000). The caller must ensure that -// usedPorts does not contain all ports in range [10000 - 11000). +// unusedPort returns a port in range [minMatchPort, minMatchPort+maxPorts). The +// caller must ensure that usedPorts does not contain all ports in that range. func unusedPort(usedPorts sets.Set[int32]) int32 { foundFreePort := false var suggestPort int32 for !foundFreePort { - suggestPort = rand.Int32N(maxPorts) + 10000 + suggestPort = rand.Int32N(maxPorts) + minMatchPort if !usedPorts.Has(suggestPort) { foundFreePort = true } diff --git a/cmd/k8s-operator/egress-services_test.go b/cmd/k8s-operator/egress-services_test.go index 8953487ac..cea21e879 100644 --- a/cmd/k8s-operator/egress-services_test.go +++ b/cmd/k8s-operator/egress-services_test.go @@ -9,6 +9,7 @@ import ( "context" "encoding/json" "fmt" + "strings" "testing" "github.com/AlekSi/pointer" @@ -22,6 +23,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + "sigs.k8s.io/controller-runtime/pkg/reconcile" tsoperator "tailscale.com/k8s-operator" tsapi "tailscale.com/k8s-operator/apis/v1alpha1" @@ -623,3 +625,114 @@ func TestMissingEndpointSliceFamilies(t *testing.T) { t.Error("expected missing=false once the current ClusterIP Service's IPv4 slice exists") } } + +// TestEgressServicePortAllocation verifies that the reconciler allocates match +// ports until the ProxyGroup uses maxPorts of them, and that the health check +// port does not count towards the limit. +func TestEgressServicePortAllocation(t *testing.T) { + tests := []struct { + name string + usedPorts int + wantErr bool + }{ + {name: "above_old_limit", usedPorts: 1000}, + {name: "one_below_limit", usedPorts: maxPorts - 1}, + {name: "at_limit", usedPorts: maxPorts, wantErr: true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + pg := &tsapi.ProxyGroup{ + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Spec: tsapi.ProxyGroupSpec{Type: tsapi.ProxyGroupTypeEgress}, + } + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: pgEgressCMName("foo"), + Namespace: "operator-ns", + }, + } + // A ClusterIP Service for another egress service on the same + // ProxyGroup that already uses tt.usedPorts match ports. + existing := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "existing", + Namespace: "operator-ns", + Labels: map[string]string{labelProxyGroup: "foo"}, + }, + Spec: corev1.ServiceSpec{Type: corev1.ServiceTypeClusterIP}, + } + for i := range tt.usedPorts { + existing.Spec.Ports = append(existing.Spec.Ports, corev1.ServicePort{ + Name: fmt.Sprintf("port-%d", i), + Protocol: "TCP", + Port: int32(i + 1), + TargetPort: intstr.FromInt32(minMatchPort + int32(i)), + }) + } + existing.Spec.Ports = append(existing.Spec.Ports, corev1.ServicePort{ + Name: tsHealthCheckPortName, + Protocol: "TCP", + Port: defaultLocalAddrPort, + TargetPort: intstr.FromInt(defaultLocalAddrPort), + }) + fc := fake.NewClientBuilder(). + WithScheme(tsapi.GlobalScheme). + WithObjects(pg, cm, existing). + WithStatusSubresource(pg). + WithInterceptorFuncs(interceptor.Funcs{ + Create: clusterIPInterceptor("10.96.0.1"), + }). + Build() + zl, err := zap.NewDevelopment() + if err != nil { + t.Fatal(err) + } + esr := &egressSvcsReconciler{ + Client: fc, + logger: zl.Sugar(), + clock: tstest.NewClock(tstest.ClockOpts{}), + tsNamespace: "operator-ns", + } + svc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + UID: types.UID("1234-UID"), + Annotations: map[string]string{ + AnnotationTailnetTargetFQDN: "foo.bar.ts.net.", + AnnotationProxyGroup: "foo", + }, + }, + Spec: corev1.ServiceSpec{ + ExternalName: "placeholder", + Type: corev1.ServiceTypeExternalName, + Ports: []corev1.ServicePort{{Protocol: "TCP", Port: 80}}, + }, + } + mustCreate(t, fc, svc) + + _, err = esr.Reconcile(t.Context(), reconcile.Request{ + NamespacedName: types.NamespacedName{Namespace: "default", Name: "test"}, + }) + if tt.wantErr { + if err == nil || !strings.Contains(err.Error(), "unable to allocate additional ports") { + t.Fatalf("Reconcile: got error %v, want a port allocation error", err) + } + return + } + if err != nil { + t.Fatalf("Reconcile: unexpected error: %v", err) + } + name := findGenNameForEgressSvcResources(t, fc, svc) + clusterSvc := mustGetClusterIPSvc(t, fc, name) + for _, p := range clusterSvc.Spec.Ports { + if p.Name == tsHealthCheckPortName { + continue + } + if got := p.TargetPort.IntVal; got < minMatchPort+int32(tt.usedPorts) || got >= minMatchPort+maxPorts { + t.Errorf("got match port %d, want a free port in range [%d, %d)", got, minMatchPort+tt.usedPorts, minMatchPort+maxPorts) + } + } + }) + } +}