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) + } + } + }) + } +}