From 95dcc1745b8c4799a05df98766dfde7610ef68b8 Mon Sep 17 00:00:00 2001 From: Irbe Krumina Date: Tue, 27 Feb 2024 15:19:53 +0000 Subject: [PATCH] cmd/k8s-operator: reconcile tailscale Ingresses when their backend Services change. (#11255) This is so that if a backend Service gets created after the Ingress, it gets picked up by the operator. Updates tailscale/tailscale#11251 Signed-off-by: Irbe Krumina Co-authored-by: Anton Tolchanov <1687799+knyar@users.noreply.github.com> --- cmd/k8s-operator/operator.go | 45 +++++++++- cmd/k8s-operator/operator_test.go | 135 ++++++++++++++++++++++++++++++ 2 files changed, 178 insertions(+), 2 deletions(-) diff --git a/cmd/k8s-operator/operator.go b/cmd/k8s-operator/operator.go index c9749ccb4..6993b20fb 100644 --- a/cmd/k8s-operator/operator.go +++ b/cmd/k8s-operator/operator.go @@ -272,12 +272,14 @@ func runReconcilers(zlog *zap.SugaredLogger, s *tsnet.Server, tsNamespace string // If a ProxyClassChanges, enqueue all Ingresses labeled with that // ProxyClass's name. proxyClassFilterForIngress := handler.EnqueueRequestsFromMapFunc(proxyClassHandlerForIngress(mgr.GetClient(), startlog)) + // Enque Ingress if a managed Service or backend Service associated with a tailscale Ingress changes. + svcHandlerForIngress := handler.EnqueueRequestsFromMapFunc(serviceHandlerForIngress(mgr.GetClient(), startlog)) err = builder. ControllerManagedBy(mgr). For(&networkingv1.Ingress{}). Watches(&appsv1.StatefulSet{}, ingressChildFilter). Watches(&corev1.Secret{}, ingressChildFilter). - Watches(&corev1.Service{}, ingressChildFilter). + Watches(&corev1.Service{}, svcHandlerForIngress). Watches(&tsapi.ProxyClass{}, proxyClassFilterForIngress). Complete(&IngressReconciler{ ssr: ssr, @@ -422,6 +424,46 @@ func proxyClassHandlerForConnector(cl client.Client, logger *zap.SugaredLogger) } } +// serviceHandlerForIngress returns a handler for Service events for ingress +// reconciler that ensures that if the Service associated with an event is of +// interest to the reconciler, the associated Ingress(es) gets be reconciled. +// The Services of interest are backend Services for tailscale Ingress and +// managed Services for an StatefulSet for a proxy configured for tailscale +// Ingress +func serviceHandlerForIngress(cl client.Client, logger *zap.SugaredLogger) handler.MapFunc { + return func(ctx context.Context, o client.Object) []reconcile.Request { + if isManagedByType(o, "ingress") { + ingName := parentFromObjectLabels(o) + return []reconcile.Request{{NamespacedName: ingName}} + } + ingList := networkingv1.IngressList{} + if err := cl.List(ctx, &ingList, client.InNamespace(o.GetNamespace())); err != nil { + logger.Debugf("error listing Ingresses: %v", err) + return nil + } + reqs := make([]reconcile.Request, 0) + for _, ing := range ingList.Items { + if ing.Spec.IngressClassName == nil || *ing.Spec.IngressClassName != tailscaleIngressClassName { + return nil + } + if ing.Spec.DefaultBackend != nil && ing.Spec.DefaultBackend.Service != nil && ing.Spec.DefaultBackend.Service.Name == o.GetName() { + reqs = append(reqs, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&ing)}) + } + for _, rule := range ing.Spec.Rules { + if rule.HTTP == nil { + continue + } + for _, path := range rule.HTTP.Paths { + if path.Backend.Service != nil && path.Backend.Service.Name == o.GetName() { + reqs = append(reqs, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&ing)}) + } + } + } + } + return reqs + } +} + func serviceHandler(_ context.Context, o client.Object) []reconcile.Request { if isManagedByType(o, "svc") { // If this is a Service managed by a Service we want to enqueue its parent @@ -440,7 +482,6 @@ func serviceHandler(_ context.Context, o client.Object) []reconcile.Request { }, }, } - } // isMagicDNSName reports whether name is a full tailnet node FQDN (with or diff --git a/cmd/k8s-operator/operator_test.go b/cmd/k8s-operator/operator_test.go index 829c72eaa..3dba7325f 100644 --- a/cmd/k8s-operator/operator_test.go +++ b/cmd/k8s-operator/operator_test.go @@ -6,15 +6,19 @@ package main import ( + "context" "fmt" "testing" + "github.com/google/go-cmp/cmp" "go.uber.org/zap" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/reconcile" tsapi "tailscale.com/k8s-operator/apis/v1alpha1" "tailscale.com/types/ptr" "tailscale.com/util/mak" @@ -1176,3 +1180,134 @@ func Test_isMagicDNSName(t *testing.T) { }) } } + +func Test_serviceHandlerForIngress(t *testing.T) { + fc := fake.NewFakeClient() + zl, err := zap.NewDevelopment() + if err != nil { + t.Fatal(err) + } + + // 1. An event on a headless Service for a tailscale Ingress results in + // the Ingress being reconciled. + mustCreate(t, fc, &networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ing-1", + Namespace: "ns-1", + }, + Spec: networkingv1.IngressSpec{IngressClassName: ptr.To(tailscaleIngressClassName)}, + }) + svc1 := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "headless-1", + Namespace: "tailscale", + Labels: map[string]string{ + LabelManaged: "true", + LabelParentName: "ing-1", + LabelParentNamespace: "ns-1", + LabelParentType: "ingress", + }, + }, + } + mustCreate(t, fc, svc1) + wantReqs := []reconcile.Request{{NamespacedName: types.NamespacedName{Namespace: "ns-1", Name: "ing-1"}}} + gotReqs := serviceHandlerForIngress(fc, zl.Sugar())(context.Background(), svc1) + if diff := cmp.Diff(gotReqs, wantReqs); diff != "" { + t.Fatalf("unexpected reconcile requests (-got +want):\n%s", diff) + } + + // 2. An event on a Service that is the default backend for a tailscale + // Ingress results in the Ingress being reconciled. + mustCreate(t, fc, &networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ing-2", + Namespace: "ns-2", + }, + Spec: networkingv1.IngressSpec{ + DefaultBackend: &networkingv1.IngressBackend{ + Service: &networkingv1.IngressServiceBackend{Name: "def-backend"}, + }, + IngressClassName: ptr.To(tailscaleIngressClassName), + }, + }) + backendSvc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "def-backend", + Namespace: "ns-2", + }, + } + mustCreate(t, fc, backendSvc) + wantReqs = []reconcile.Request{{NamespacedName: types.NamespacedName{Namespace: "ns-2", Name: "ing-2"}}} + gotReqs = serviceHandlerForIngress(fc, zl.Sugar())(context.Background(), backendSvc) + if diff := cmp.Diff(gotReqs, wantReqs); diff != "" { + t.Fatalf("unexpected reconcile requests (-got +want):\n%s", diff) + } + + // 3. An event on a Service that is one of the non-default backends for + // a tailscale Ingress results in the Ingress being reconciled. + mustCreate(t, fc, &networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ing-3", + Namespace: "ns-3", + }, + Spec: networkingv1.IngressSpec{ + IngressClassName: ptr.To(tailscaleIngressClassName), + Rules: []networkingv1.IngressRule{{IngressRuleValue: networkingv1.IngressRuleValue{HTTP: &networkingv1.HTTPIngressRuleValue{ + Paths: []networkingv1.HTTPIngressPath{ + {Backend: networkingv1.IngressBackend{Service: &networkingv1.IngressServiceBackend{Name: "backend"}}}}, + }}}}, + }, + }) + backendSvc2 := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "backend", + Namespace: "ns-3", + }, + } + mustCreate(t, fc, backendSvc2) + wantReqs = []reconcile.Request{{NamespacedName: types.NamespacedName{Namespace: "ns-3", Name: "ing-3"}}} + gotReqs = serviceHandlerForIngress(fc, zl.Sugar())(context.Background(), backendSvc2) + if diff := cmp.Diff(gotReqs, wantReqs); diff != "" { + t.Fatalf("unexpected reconcile requests (-got +want):\n%s", diff) + } + + // 4. An event on a Service that is a backend for an Ingress that is not + // tailscale Ingress does not result in an Ingress reconcile. + mustCreate(t, fc, &networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ing-4", + Namespace: "ns-4", + }, + Spec: networkingv1.IngressSpec{ + Rules: []networkingv1.IngressRule{{IngressRuleValue: networkingv1.IngressRuleValue{HTTP: &networkingv1.HTTPIngressRuleValue{ + Paths: []networkingv1.HTTPIngressPath{ + {Backend: networkingv1.IngressBackend{Service: &networkingv1.IngressServiceBackend{Name: "non-ts-backend"}}}}, + }}}}, + }, + }) + nonTSBackend := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "non-ts-backend", + Namespace: "ns-4", + }, + } + mustCreate(t, fc, nonTSBackend) + gotReqs = serviceHandlerForIngress(fc, zl.Sugar())(context.Background(), nonTSBackend) + if len(gotReqs) > 0 { + t.Errorf("unexpected reconcile request for a Service that does not belong to a Tailscale Ingress: %#+v\n", gotReqs) + } + + // 5. An event on a Service not related to any Ingress does not result + // in an Ingress reconcile. + someSvc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-svc", + Namespace: "ns-4", + }, + } + mustCreate(t, fc, someSvc) + gotReqs = serviceHandlerForIngress(fc, zl.Sugar())(context.Background(), someSvc) + if len(gotReqs) > 0 { + t.Errorf("unexpected reconcile request for a Service that does not belong to any Ingress: %#+v\n", gotReqs) + } +}