diff --git a/cmd/k8s-operator/svc-for-pg.go b/cmd/k8s-operator/svc-for-pg.go index 08d573f83..395e89eb9 100644 --- a/cmd/k8s-operator/svc-for-pg.go +++ b/cmd/k8s-operator/svc-for-pg.go @@ -829,9 +829,35 @@ func (r *HAServiceReconciler) validateService(ctx context.Context, svc *corev1.S } svcName := nameForService(svc) for _, s := range svcList.Items { - if r.shouldExpose(&s) && nameForService(&s) == svcName && s.UID != svc.UID { - errs = append(errs, fmt.Errorf("found duplicate Service %q for hostname %q - multiple HA Services for the same hostname in the same cluster are not allowed", client.ObjectKeyFromObject(&s), svcName)) + if s.UID == svc.UID { + continue } + // Only check services managed by the ProxyGroup reconciler. Services + // exposed via the single-proxy path in svc.go have their own + // hostname tracking and live in a separate per-proxy tailnet + // namespace; flagging them as duplicates here breaks multi-tailnet + // setups where a single-proxy Service on the primary tailnet shares + // a hostname with a ProxyGroup ingress on a secondary tailnet. + if !r.isTailscaleService(&s) { + continue + } + if nameForService(&s) != svcName { + continue + } + // Two ProxyGroups joined to different tailnets each have their own + // DNS namespace, so a hostname collision across them is not a real + // conflict. Look up the other Service's ProxyGroup and skip the + // duplicate report when the tailnets differ; if the lookup fails + // fall through and flag the collision so a genuine duplicate isn't + // silently allowed. + otherPGName := s.Annotations[AnnotationProxyGroup] + if otherPGName != "" && otherPGName != pg.Name { + otherPG := &tsapi.ProxyGroup{} + if err := r.Get(ctx, client.ObjectKey{Name: otherPGName}, otherPG); err == nil && otherPG.Spec.Tailnet != pg.Spec.Tailnet { + continue + } + } + errs = append(errs, fmt.Errorf("found duplicate Service %q for hostname %q - multiple HA Services for the same hostname on the same tailnet are not allowed", client.ObjectKeyFromObject(&s), svcName)) } return errors.Join(errs...) } diff --git a/cmd/k8s-operator/svc-for-pg_test.go b/cmd/k8s-operator/svc-for-pg_test.go index 455d3363c..9d2296112 100644 --- a/cmd/k8s-operator/svc-for-pg_test.go +++ b/cmd/k8s-operator/svc-for-pg_test.go @@ -258,7 +258,7 @@ func TestValidateService(t *testing.T) { Status: metav1.ConditionFalse, Reason: reasonIngressSvcInvalid, LastTransitionTime: metav1.NewTime(cl.Now().Truncate(time.Second)), - Message: `found duplicate Service "ns-2/my-app2" for hostname "my-app" - multiple HA Services for the same hostname in the same cluster are not allowed`, + Message: `found duplicate Service "ns-2/my-app2" for hostname "my-app" - multiple HA Services for the same hostname on the same tailnet are not allowed`, }, }, }, @@ -270,6 +270,142 @@ func TestValidateService(t *testing.T) { expectEqual(t, lc, wantSvc) } +// Regression test for #20069. The pre-fix duplicate-hostname check scanned +// every Service with shouldExpose=true, which meant a Service exposed on +// one tailnet via the single-proxy path (svc.go) would block a ProxyGroup +// ingress Service for the same hostname on a different tailnet. The +// ProxyGroup path's validateService must skip Services that aren't +// themselves managed by a ProxyGroup. +func TestValidateService_SingleProxyServiceDoesNotCollideWithProxyGroup(t *testing.T) { + pgr, _, lc, _, _ := setupServiceTest(t) + // Service exposed via the single-proxy path: tailscale.com/expose=true + // and no tailscale.com/proxy-group annotation. Its hostname matches + // the ProxyGroup-managed Service below, but it lives in a different + // reconciler entirely and must not be flagged as a duplicate. + singleProxySvc := &corev1.Service{ + TypeMeta: metav1.TypeMeta{Kind: "Service", APIVersion: "v1"}, + ObjectMeta: metav1.ObjectMeta{ + Name: "single-proxy", + Namespace: "ns-1", + UID: types.UID("single-proxy-uid"), + Annotations: map[string]string{ + "tailscale.com/expose": "true", + "tailscale.com/hostname": "my-app", + }, + }, + Spec: corev1.ServiceSpec{ + ClusterIP: "1.2.3.4", + Type: corev1.ServiceTypeClusterIP, + }, + } + // ProxyGroup-managed Service for the same hostname. + pgSvc := &corev1.Service{ + TypeMeta: metav1.TypeMeta{Kind: "Service", APIVersion: "v1"}, + ObjectMeta: metav1.ObjectMeta{ + Name: "pg-svc", + Namespace: "ns-2", + UID: types.UID("pg-svc-uid"), + Annotations: map[string]string{ + "tailscale.com/proxy-group": "test-pg", + "tailscale.com/hostname": "my-app", + }, + }, + Spec: corev1.ServiceSpec{ + ClusterIP: "1.2.3.5", + Type: corev1.ServiceTypeLoadBalancer, + LoadBalancerClass: new("tailscale"), + }, + } + + mustCreate(t, lc, singleProxySvc) + mustCreate(t, lc, pgSvc) + expectReconciled(t, pgr, pgSvc.Namespace, pgSvc.Name) + + got := &corev1.Service{} + if err := lc.Get(context.Background(), client.ObjectKeyFromObject(pgSvc), got); err != nil { + t.Fatalf("get Service: %v", err) + } + for _, c := range got.Status.Conditions { + if c.Type == string(tsapi.IngressSvcValid) && c.Status == metav1.ConditionFalse { + t.Fatalf("ProxyGroup Service flagged invalid by a single-proxy Service on a different tailnet: %s", c.Message) + } + } +} + +// Regression test for #20069. Two ProxyGroup-managed Services with the same +// hostname but joined to different tailnets each have their own DNS +// namespace and must not be flagged as duplicates. The Service being +// reconciled here is on the default tailnet (so it uses the configured +// fake tsclient); the conflicting Service already exists in-cluster on +// a different tailnet and must be skipped by the duplicate check. +func TestValidateService_DifferentTailnetDoesNotCollide(t *testing.T) { + pgr, _, lc, _, _ := setupServiceTest(t) + // Pre-create a ProxyGroup joined to a different tailnet. + secondaryPG := &tsapi.ProxyGroup{ + ObjectMeta: metav1.ObjectMeta{ + Name: "secondary-pg", + Generation: 1, + }, + Spec: tsapi.ProxyGroupSpec{ + Type: tsapi.ProxyGroupTypeIngress, + Tailnet: "secondary", + }, + } + if err := lc.Create(context.Background(), secondaryPG); err != nil { + t.Fatalf("create secondary ProxyGroup: %v", err) + } + // Pre-existing Service on the secondary tailnet with the conflicting hostname. + otherTailnetSvc := &corev1.Service{ + TypeMeta: metav1.TypeMeta{Kind: "Service", APIVersion: "v1"}, + ObjectMeta: metav1.ObjectMeta{ + Name: "other-tailnet-svc", + Namespace: "ns-2", + UID: types.UID("other-tailnet-svc-uid"), + Annotations: map[string]string{ + "tailscale.com/proxy-group": "secondary-pg", + "tailscale.com/hostname": "my-app", + }, + }, + Spec: corev1.ServiceSpec{ + ClusterIP: "1.2.3.5", + Type: corev1.ServiceTypeLoadBalancer, + LoadBalancerClass: new("tailscale"), + }, + } + mustCreate(t, lc, otherTailnetSvc) + + // Service being reconciled: same hostname, default tailnet ProxyGroup. + primarySvc := &corev1.Service{ + TypeMeta: metav1.TypeMeta{Kind: "Service", APIVersion: "v1"}, + ObjectMeta: metav1.ObjectMeta{ + Name: "primary-svc", + Namespace: "ns-1", + UID: types.UID("primary-svc-uid"), + Annotations: map[string]string{ + "tailscale.com/proxy-group": "test-pg", + "tailscale.com/hostname": "my-app", + }, + }, + Spec: corev1.ServiceSpec{ + ClusterIP: "1.2.3.4", + Type: corev1.ServiceTypeLoadBalancer, + LoadBalancerClass: new("tailscale"), + }, + } + mustCreate(t, lc, primarySvc) + expectReconciled(t, pgr, primarySvc.Namespace, primarySvc.Name) + + got := &corev1.Service{} + if err := lc.Get(context.Background(), client.ObjectKeyFromObject(primarySvc), got); err != nil { + t.Fatalf("get Service: %v", err) + } + for _, c := range got.Status.Conditions { + if c.Type == string(tsapi.IngressSvcValid) && c.Status == metav1.ConditionFalse { + t.Fatalf("ProxyGroup Service flagged invalid by a Service on a different tailnet with the same hostname: %s", c.Message) + } + } +} + func TestServicePGReconciler_MultiCluster(t *testing.T) { var ft *fakeTSClient for i := 0; i <= 10; i++ {