diff --git a/cmd/k8s-operator/ingress-for-pg.go b/cmd/k8s-operator/ingress-for-pg.go index 66d74292b..ea31dbd63 100644 --- a/cmd/k8s-operator/ingress-for-pg.go +++ b/cmd/k8s-operator/ingress-for-pg.go @@ -252,7 +252,7 @@ func (r *HAIngressReconciler) maybeProvision(ctx context.Context, hostname strin return false, fmt.Errorf("error determining DNS name base: %w", err) } dnsName := hostname + "." + tcd - if err := r.ensureCertResources(ctx, pgName, dnsName, ing); err != nil { + if err := r.ensureCertResources(ctx, pg, dnsName, ing); err != nil { return false, fmt.Errorf("error ensuring cert resources: %w", err) } @@ -931,18 +931,31 @@ func ownersAreSetAndEqual(a, b *tailscale.VIPService) bool { // (domain) is a valid Kubernetes resource name. // https://github.com/tailscale/tailscale/blob/8b1e7f646ee4730ad06c9b70c13e7861b964949b/util/dnsname/dnsname.go#L99 // https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-subdomain-names -func (r *HAIngressReconciler) ensureCertResources(ctx context.Context, pgName, domain string, ing *networkingv1.Ingress) error { - secret := certSecret(pgName, r.tsNamespace, domain, ing) - if _, err := createOrUpdate(ctx, r.Client, r.tsNamespace, secret, nil); err != nil { +func (r *HAIngressReconciler) ensureCertResources(ctx context.Context, pg *tsapi.ProxyGroup, domain string, ing *networkingv1.Ingress) error { + secret := certSecret(pg.Name, r.tsNamespace, domain, ing) + if _, err := createOrUpdate(ctx, r.Client, r.tsNamespace, secret, func(s *corev1.Secret) { + // Labels might have changed if the Ingress has been updated to use a + // different ProxyGroup. + s.Labels = secret.Labels + }); err != nil { return fmt.Errorf("failed to create or update Secret %s: %w", secret.Name, err) } - role := certSecretRole(pgName, r.tsNamespace, domain) - if _, err := createOrUpdate(ctx, r.Client, r.tsNamespace, role, nil); err != nil { + role := certSecretRole(pg.Name, r.tsNamespace, domain) + if _, err := createOrUpdate(ctx, r.Client, r.tsNamespace, role, func(r *rbacv1.Role) { + // Labels might have changed if the Ingress has been updated to use a + // different ProxyGroup. + r.Labels = role.Labels + }); err != nil { return fmt.Errorf("failed to create or update Role %s: %w", role.Name, err) } - rb := certSecretRoleBinding(pgName, r.tsNamespace, domain) - if _, err := createOrUpdate(ctx, r.Client, r.tsNamespace, rb, nil); err != nil { - return fmt.Errorf("failed to create or update RoleBinding %s: %w", rb.Name, err) + rolebinding := certSecretRoleBinding(pg.Name, r.tsNamespace, domain) + if _, err := createOrUpdate(ctx, r.Client, r.tsNamespace, rolebinding, func(rb *rbacv1.RoleBinding) { + // Labels and subjects might have changed if the Ingress has been updated to use a + // different ProxyGroup. + rb.Labels = rolebinding.Labels + rb.Subjects = rolebinding.Subjects + }); err != nil { + return fmt.Errorf("failed to create or update RoleBinding %s: %w", rolebinding.Name, err) } return nil } diff --git a/cmd/k8s-operator/ingress-for-pg_test.go b/cmd/k8s-operator/ingress-for-pg_test.go index b487d660c..05f482792 100644 --- a/cmd/k8s-operator/ingress-for-pg_test.go +++ b/cmd/k8s-operator/ingress-for-pg_test.go @@ -69,7 +69,7 @@ func TestIngressPGReconciler(t *testing.T) { expectReconciled(t, ingPGR, "default", "test-ingress") verifyServeConfig(t, fc, "svc:my-svc", false) verifyTailscaleService(t, ft, "svc:my-svc", []string{"tcp:443"}) - verifyTailscaledConfig(t, fc, []string{"svc:my-svc"}) + verifyTailscaledConfig(t, fc, "test-pg", []string{"svc:my-svc"}) // Verify that Role and RoleBinding have been created for the first Ingress. // Do not verify the cert Secret as that was already verified implicitly above. @@ -132,7 +132,7 @@ func TestIngressPGReconciler(t *testing.T) { verifyServeConfig(t, fc, "svc:my-other-svc", false) verifyTailscaleService(t, ft, "svc:my-other-svc", []string{"tcp:443"}) - // Verify that Role and RoleBinding have been created for the first Ingress. + // Verify that Role and RoleBinding have been created for the second Ingress. // Do not verify the cert Secret as that was already verified implicitly above. expectEqual(t, fc, certSecretRole("test-pg", "operator-ns", "my-other-svc.ts.net")) expectEqual(t, fc, certSecretRoleBinding("test-pg", "operator-ns", "my-other-svc.ts.net")) @@ -141,7 +141,7 @@ func TestIngressPGReconciler(t *testing.T) { verifyServeConfig(t, fc, "svc:my-svc", false) verifyTailscaleService(t, ft, "svc:my-svc", []string{"tcp:443"}) - verifyTailscaledConfig(t, fc, []string{"svc:my-svc", "svc:my-other-svc"}) + verifyTailscaledConfig(t, fc, "test-pg", []string{"svc:my-svc", "svc:my-other-svc"}) // Delete second Ingress if err := fc.Delete(context.Background(), ing2); err != nil { @@ -172,11 +172,20 @@ func TestIngressPGReconciler(t *testing.T) { t.Error("second Ingress service config was not cleaned up") } - verifyTailscaledConfig(t, fc, []string{"svc:my-svc"}) + verifyTailscaledConfig(t, fc, "test-pg", []string{"svc:my-svc"}) expectMissing[corev1.Secret](t, fc, "operator-ns", "my-other-svc.ts.net") expectMissing[rbacv1.Role](t, fc, "operator-ns", "my-other-svc.ts.net") expectMissing[rbacv1.RoleBinding](t, fc, "operator-ns", "my-other-svc.ts.net") + // Test Ingress ProxyGroup change + createPGResources(t, fc, "test-pg-second") + mustUpdate(t, fc, "default", "test-ingress", func(ing *networkingv1.Ingress) { + ing.Annotations["tailscale.com/proxy-group"] = "test-pg-second" + }) + expectReconciled(t, ingPGR, "default", "test-ingress") + expectEqual(t, fc, certSecretRole("test-pg-second", "operator-ns", "my-svc.ts.net")) + expectEqual(t, fc, certSecretRoleBinding("test-pg-second", "operator-ns", "my-svc.ts.net")) + // Delete the first Ingress and verify cleanup if err := fc.Delete(context.Background(), ing); err != nil { t.Fatalf("deleting Ingress: %v", err) @@ -187,7 +196,7 @@ func TestIngressPGReconciler(t *testing.T) { // Verify the ConfigMap was cleaned up cm = &corev1.ConfigMap{} if err := fc.Get(context.Background(), types.NamespacedName{ - Name: "test-pg-ingress-config", + Name: "test-pg-second-ingress-config", Namespace: "operator-ns", }, cm); err != nil { t.Fatalf("getting ConfigMap: %v", err) @@ -201,7 +210,7 @@ func TestIngressPGReconciler(t *testing.T) { if len(cfg.Services) > 0 { t.Error("serve config not cleaned up") } - verifyTailscaledConfig(t, fc, nil) + verifyTailscaledConfig(t, fc, "test-pg-second", nil) // Add verification that cert resources were cleaned up expectMissing[corev1.Secret](t, fc, "operator-ns", "my-svc.ts.net") @@ -245,7 +254,7 @@ func TestIngressPGReconciler_UpdateIngressHostname(t *testing.T) { expectReconciled(t, ingPGR, "default", "test-ingress") verifyServeConfig(t, fc, "svc:my-svc", false) verifyTailscaleService(t, ft, "svc:my-svc", []string{"tcp:443"}) - verifyTailscaledConfig(t, fc, []string{"svc:my-svc"}) + verifyTailscaledConfig(t, fc, "test-pg", []string{"svc:my-svc"}) // Update the Ingress hostname and make sure the original Tailscale Service is deleted. mustUpdate(t, fc, "default", "test-ingress", func(ing *networkingv1.Ingress) { @@ -256,7 +265,7 @@ func TestIngressPGReconciler_UpdateIngressHostname(t *testing.T) { expectReconciled(t, ingPGR, "default", "test-ingress") verifyServeConfig(t, fc, "svc:updated-svc", false) verifyTailscaleService(t, ft, "svc:updated-svc", []string{"tcp:443"}) - verifyTailscaledConfig(t, fc, []string{"svc:updated-svc"}) + verifyTailscaledConfig(t, fc, "test-pg", []string{"svc:updated-svc"}) _, err := ft.GetVIPService(context.Background(), tailcfg.ServiceName("svc:my-svc")) if err == nil { @@ -550,6 +559,117 @@ func TestIngressPGReconciler_HTTPEndpoint(t *testing.T) { } } +func TestIngressPGReconciler_MultiCluster(t *testing.T) { + ingPGR, fc, ft := setupIngressTest(t) + ingPGR.operatorID = "operator-1" + + // Create initial Ingress + ing := &networkingv1.Ingress{ + TypeMeta: metav1.TypeMeta{Kind: "Ingress", APIVersion: "networking.k8s.io/v1"}, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ingress", + Namespace: "default", + UID: types.UID("1234-UID"), + Annotations: map[string]string{ + "tailscale.com/proxy-group": "test-pg", + }, + }, + Spec: networkingv1.IngressSpec{ + IngressClassName: ptr.To("tailscale"), + TLS: []networkingv1.IngressTLS{ + {Hosts: []string{"my-svc"}}, + }, + }, + } + mustCreate(t, fc, ing) + + // Simulate existing Tailscale Service from another cluster + existingVIPSvc := &tailscale.VIPService{ + Name: "svc:my-svc", + Annotations: map[string]string{ + ownerAnnotation: `{"ownerrefs":[{"operatorID":"operator-2"}]}`, + }, + } + ft.vipServices = map[tailcfg.ServiceName]*tailscale.VIPService{ + "svc:my-svc": existingVIPSvc, + } + + // Verify reconciliation adds our operator reference + expectReconciled(t, ingPGR, "default", "test-ingress") + + tsSvc, err := ft.GetVIPService(context.Background(), "svc:my-svc") + if err != nil { + t.Fatalf("getting Tailscale Service: %v", err) + } + if tsSvc == nil { + t.Fatal("Tailscale Service not found") + } + + o, err := parseOwnerAnnotation(tsSvc) + if err != nil { + t.Fatalf("parsing owner annotation: %v", err) + } + + wantOwnerRefs := []OwnerRef{ + {OperatorID: "operator-2"}, + {OperatorID: "operator-1"}, + } + if !reflect.DeepEqual(o.OwnerRefs, wantOwnerRefs) { + t.Errorf("incorrect owner refs\ngot: %+v\nwant: %+v", o.OwnerRefs, wantOwnerRefs) + } + + // Delete the Ingress and verify Tailscale Service still exists with one owner ref + if err := fc.Delete(context.Background(), ing); err != nil { + t.Fatalf("deleting Ingress: %v", err) + } + expectRequeue(t, ingPGR, "default", "test-ingress") + + tsSvc, err = ft.GetVIPService(context.Background(), "svc:my-svc") + if err != nil { + t.Fatalf("getting Tailscale Service after deletion: %v", err) + } + if tsSvc == nil { + t.Fatal("Tailscale Service was incorrectly deleted") + } + + o, err = parseOwnerAnnotation(tsSvc) + if err != nil { + t.Fatalf("parsing owner annotation: %v", err) + } + + wantOwnerRefs = []OwnerRef{ + {OperatorID: "operator-2"}, + } + if !reflect.DeepEqual(o.OwnerRefs, wantOwnerRefs) { + t.Errorf("incorrect owner refs after deletion\ngot: %+v\nwant: %+v", o.OwnerRefs, wantOwnerRefs) + } +} + +func populateTLSSecret(ctx context.Context, c client.Client, pgName, domain string) error { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: domain, + Namespace: "operator-ns", + Labels: map[string]string{ + kubetypes.LabelManaged: "true", + labelProxyGroup: pgName, + labelDomain: domain, + kubetypes.LabelSecretType: "certs", + }, + }, + Type: corev1.SecretTypeTLS, + Data: map[string][]byte{ + corev1.TLSCertKey: []byte("fake-cert"), + corev1.TLSPrivateKeyKey: []byte("fake-key"), + }, + } + + _, err := createOrUpdate(ctx, c, "operator-ns", secret, func(s *corev1.Secret) { + s.Data = secret.Data + }) + return err +} + func verifyTailscaleService(t *testing.T, ft *fakeTSClient, serviceName string, wantPorts []string) { t.Helper() tsSvc, err := ft.GetVIPService(context.Background(), tailcfg.ServiceName(serviceName)) @@ -618,7 +738,7 @@ func verifyServeConfig(t *testing.T, fc client.Client, serviceName string, wantH } } -func verifyTailscaledConfig(t *testing.T, fc client.Client, expectedServices []string) { +func verifyTailscaledConfig(t *testing.T, fc client.Client, pgName string, expectedServices []string) { t.Helper() var expected string if expectedServices != nil && len(expectedServices) > 0 { @@ -630,9 +750,9 @@ func verifyTailscaledConfig(t *testing.T, fc client.Client, expectedServices []s } expectEqual(t, fc, &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: pgConfigSecretName("test-pg", 0), + Name: pgConfigSecretName(pgName, 0), Namespace: "operator-ns", - Labels: pgSecretLabels("test-pg", "config"), + Labels: pgSecretLabels(pgName, "config"), }, Data: map[string][]byte{ tsoperator.TailscaledConfigFileName(106): []byte(fmt.Sprintf(`{"Version":""%s}`, expected)), @@ -640,53 +760,44 @@ func verifyTailscaledConfig(t *testing.T, fc client.Client, expectedServices []s }) } -func setupIngressTest(t *testing.T) (*HAIngressReconciler, client.Client, *fakeTSClient) { - tsIngressClass := &networkingv1.IngressClass{ - ObjectMeta: metav1.ObjectMeta{Name: "tailscale"}, - Spec: networkingv1.IngressClassSpec{Controller: "tailscale.com/ts-ingress"}, - } - +func createPGResources(t *testing.T, fc client.Client, pgName string) { + t.Helper() // Pre-create the ProxyGroup pg := &tsapi.ProxyGroup{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-pg", + Name: pgName, Generation: 1, }, Spec: tsapi.ProxyGroupSpec{ Type: tsapi.ProxyGroupTypeIngress, }, } + mustCreate(t, fc, pg) // Pre-create the ConfigMap for the ProxyGroup pgConfigMap := &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-pg-ingress-config", + Name: fmt.Sprintf("%s-ingress-config", pgName), Namespace: "operator-ns", }, BinaryData: map[string][]byte{ "serve-config.json": []byte(`{"Services":{}}`), }, } + mustCreate(t, fc, pgConfigMap) // Pre-create a config Secret for the ProxyGroup pgCfgSecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: pgConfigSecretName("test-pg", 0), + Name: pgConfigSecretName(pgName, 0), Namespace: "operator-ns", - Labels: pgSecretLabels("test-pg", "config"), + Labels: pgSecretLabels(pgName, "config"), }, Data: map[string][]byte{ tsoperator.TailscaledConfigFileName(106): []byte("{}"), }, } - - fc := fake.NewClientBuilder(). - WithScheme(tsapi.GlobalScheme). - WithObjects(pg, pgCfgSecret, pgConfigMap, tsIngressClass). - WithStatusSubresource(pg). - Build() - - // Set ProxyGroup status to ready + mustCreate(t, fc, pgCfgSecret) pg.Status.Conditions = []metav1.Condition{ { Type: string(tsapi.ProxyGroupReady), @@ -697,6 +808,22 @@ func setupIngressTest(t *testing.T) (*HAIngressReconciler, client.Client, *fakeT if err := fc.Status().Update(context.Background(), pg); err != nil { t.Fatal(err) } +} + +func setupIngressTest(t *testing.T) (*HAIngressReconciler, client.Client, *fakeTSClient) { + tsIngressClass := &networkingv1.IngressClass{ + ObjectMeta: metav1.ObjectMeta{Name: "tailscale"}, + Spec: networkingv1.IngressClassSpec{Controller: "tailscale.com/ts-ingress"}, + } + + fc := fake.NewClientBuilder(). + WithScheme(tsapi.GlobalScheme). + WithObjects(tsIngressClass). + WithStatusSubresource(&tsapi.ProxyGroup{}). + Build() + + createPGResources(t, fc, "test-pg") + fakeTsnetServer := &fakeTSNetServer{certDomains: []string{"foo.com"}} ft := &fakeTSClient{} @@ -726,114 +853,3 @@ func setupIngressTest(t *testing.T) (*HAIngressReconciler, client.Client, *fakeT return ingPGR, fc, ft } - -func TestIngressPGReconciler_MultiCluster(t *testing.T) { - ingPGR, fc, ft := setupIngressTest(t) - ingPGR.operatorID = "operator-1" - - // Create initial Ingress - ing := &networkingv1.Ingress{ - TypeMeta: metav1.TypeMeta{Kind: "Ingress", APIVersion: "networking.k8s.io/v1"}, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-ingress", - Namespace: "default", - UID: types.UID("1234-UID"), - Annotations: map[string]string{ - "tailscale.com/proxy-group": "test-pg", - }, - }, - Spec: networkingv1.IngressSpec{ - IngressClassName: ptr.To("tailscale"), - TLS: []networkingv1.IngressTLS{ - {Hosts: []string{"my-svc"}}, - }, - }, - } - mustCreate(t, fc, ing) - - // Simulate existing Tailscale Service from another cluster - existingVIPSvc := &tailscale.VIPService{ - Name: "svc:my-svc", - Annotations: map[string]string{ - ownerAnnotation: `{"ownerrefs":[{"operatorID":"operator-2"}]}`, - }, - } - ft.vipServices = map[tailcfg.ServiceName]*tailscale.VIPService{ - "svc:my-svc": existingVIPSvc, - } - - // Verify reconciliation adds our operator reference - expectReconciled(t, ingPGR, "default", "test-ingress") - - tsSvc, err := ft.GetVIPService(context.Background(), "svc:my-svc") - if err != nil { - t.Fatalf("getting Tailscale Service: %v", err) - } - if tsSvc == nil { - t.Fatal("Tailscale Service not found") - } - - o, err := parseOwnerAnnotation(tsSvc) - if err != nil { - t.Fatalf("parsing owner annotation: %v", err) - } - - wantOwnerRefs := []OwnerRef{ - {OperatorID: "operator-2"}, - {OperatorID: "operator-1"}, - } - if !reflect.DeepEqual(o.OwnerRefs, wantOwnerRefs) { - t.Errorf("incorrect owner refs\ngot: %+v\nwant: %+v", o.OwnerRefs, wantOwnerRefs) - } - - // Delete the Ingress and verify Tailscale Service still exists with one owner ref - if err := fc.Delete(context.Background(), ing); err != nil { - t.Fatalf("deleting Ingress: %v", err) - } - expectRequeue(t, ingPGR, "default", "test-ingress") - - tsSvc, err = ft.GetVIPService(context.Background(), "svc:my-svc") - if err != nil { - t.Fatalf("getting Tailscale Service after deletion: %v", err) - } - if tsSvc == nil { - t.Fatal("Tailscale Service was incorrectly deleted") - } - - o, err = parseOwnerAnnotation(tsSvc) - if err != nil { - t.Fatalf("parsing owner annotation: %v", err) - } - - wantOwnerRefs = []OwnerRef{ - {OperatorID: "operator-2"}, - } - if !reflect.DeepEqual(o.OwnerRefs, wantOwnerRefs) { - t.Errorf("incorrect owner refs after deletion\ngot: %+v\nwant: %+v", o.OwnerRefs, wantOwnerRefs) - } -} - -func populateTLSSecret(ctx context.Context, c client.Client, pgName, domain string) error { - secret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: domain, - Namespace: "operator-ns", - Labels: map[string]string{ - kubetypes.LabelManaged: "true", - labelProxyGroup: pgName, - labelDomain: domain, - kubetypes.LabelSecretType: "certs", - }, - }, - Type: corev1.SecretTypeTLS, - Data: map[string][]byte{ - corev1.TLSCertKey: []byte("fake-cert"), - corev1.TLSPrivateKeyKey: []byte("fake-key"), - }, - } - - _, err := createOrUpdate(ctx, c, "operator-ns", secret, func(s *corev1.Secret) { - s.Data = secret.Data - }) - return err -} diff --git a/cmd/k8s-operator/svc-for-pg_test.go b/cmd/k8s-operator/svc-for-pg_test.go index ecd60af50..5772cd5d6 100644 --- a/cmd/k8s-operator/svc-for-pg_test.go +++ b/cmd/k8s-operator/svc-for-pg_test.go @@ -46,7 +46,7 @@ func TestServicePGReconciler(t *testing.T) { config = append(config, fmt.Sprintf("svc:default-%s", svc.Name)) verifyTailscaleService(t, ft, fmt.Sprintf("svc:default-%s", svc.Name), []string{"do-not-validate"}) - verifyTailscaledConfig(t, fc, config) + verifyTailscaledConfig(t, fc, "test-pg", config) } for i, svc := range svcs { @@ -75,7 +75,7 @@ func TestServicePGReconciler(t *testing.T) { } config = removeEl(config, fmt.Sprintf("svc:default-%s", svc.Name)) - verifyTailscaledConfig(t, fc, config) + verifyTailscaledConfig(t, fc, "test-pg", config) } } @@ -88,7 +88,7 @@ func TestServicePGReconciler_UpdateHostname(t *testing.T) { expectReconciled(t, svcPGR, "default", svc.Name) verifyTailscaleService(t, ft, fmt.Sprintf("svc:default-%s", svc.Name), []string{"do-not-validate"}) - verifyTailscaledConfig(t, fc, []string{fmt.Sprintf("svc:default-%s", svc.Name)}) + verifyTailscaledConfig(t, fc, "test-pg", []string{fmt.Sprintf("svc:default-%s", svc.Name)}) hostname := "foobarbaz" mustUpdate(t, fc, svc.Namespace, svc.Name, func(s *corev1.Service) { @@ -100,7 +100,7 @@ func TestServicePGReconciler_UpdateHostname(t *testing.T) { expectReconciled(t, svcPGR, "default", svc.Name) verifyTailscaleService(t, ft, fmt.Sprintf("svc:%s", hostname), []string{"do-not-validate"}) - verifyTailscaledConfig(t, fc, []string{fmt.Sprintf("svc:%s", hostname)}) + verifyTailscaledConfig(t, fc, "test-pg", []string{fmt.Sprintf("svc:%s", hostname)}) _, err := ft.GetVIPService(context.Background(), tailcfg.ServiceName(fmt.Sprintf("svc:default-%s", svc.Name))) if err == nil { @@ -334,7 +334,7 @@ func TestIgnoreRegularService(t *testing.T) { mustCreate(t, fc, svc) expectReconciled(t, pgr, "default", "test") - verifyTailscaledConfig(t, fc, nil) + verifyTailscaledConfig(t, fc, "test-pg", nil) tsSvcs, err := ft.ListVIPServices(context.Background()) if err == nil {