From bc8f5a773414f8c70276dc9b22f47cb329a0f385 Mon Sep 17 00:00:00 2001 From: David Anderson Date: Mon, 12 Dec 2022 21:00:10 -0800 Subject: [PATCH] cmd/k8s-operator: add a basic unit test. The test verifies one of the successful reconcile paths, where a client requests an exposed service via a LoadBalancer class. Updates #502. Signed-off-by: David Anderson --- cmd/k8s-operator/main.go | 61 ++++--- cmd/k8s-operator/main_test.go | 334 ++++++++++++++++++++++++++++++++++ go.mod | 1 + go.sum | 1 + 4 files changed, 368 insertions(+), 29 deletions(-) create mode 100644 cmd/k8s-operator/main_test.go diff --git a/cmd/k8s-operator/main.go b/cmd/k8s-operator/main.go index 928cda587..31050e2c6 100644 --- a/cmd/k8s-operator/main.go +++ b/cmd/k8s-operator/main.go @@ -41,15 +41,15 @@ import ( "tailscale.com/types/logger" ) -var ( - hostname = defaultEnv("OPERATOR_HOSTNAME", "tailscale-operator") - kubeSecret = defaultEnv("OPERATOR_SECRET", "") - tsNamespace = defaultEnv("OPERATOR_NAMESPACE", "default") - image = defaultEnv("PROXY_IMAGE", "tailscale/tailscale:latest") - tags = defaultEnv("PROXY_TAGS", "tag:k8s") -) - func main() { + var ( + hostname = defaultEnv("OPERATOR_HOSTNAME", "tailscale-operator") + kubeSecret = defaultEnv("OPERATOR_SECRET", "") + tsNamespace = defaultEnv("OPERATOR_NAMESPACE", "default") + image = defaultEnv("PROXY_IMAGE", "tailscale/tailscale:latest") + tags = defaultEnv("PROXY_TAGS", "tag:k8s") + ) + // TODO: use logpolicy tailscale.I_Acknowledge_This_API_Is_Unstable = true logf.SetLogger(zap.New()) @@ -126,8 +126,10 @@ waitOnline: log.Fatalf("getting tailscale client: %v", err) } sr := &ServiceReconciler{ - tsClient: tsClient, - defaultTags: strings.Split(tags, ","), + tsClient: tsClient, + defaultTags: strings.Split(tags, ","), + operatorNamespace: tsNamespace, + proxyImage: image, } reconcileFilter := handler.EnqueueRequestsFromMapFunc(func(o client.Object) []reconcile.Request { ls := o.GetLabels() @@ -177,14 +179,15 @@ const ( // ServiceReconciler is a simple ControllerManagedBy example implementation. type ServiceReconciler struct { client.Client - defaultTags []string - tsClient tsClient + tsClient tsClient + defaultTags []string + operatorNamespace string + proxyImage string } type tsClient interface { - DeleteDevice(ctx context.Context, id string) error - Tailnet() string CreateKey(ctx context.Context, caps tailscale.KeyCapabilities) (string, *tailscale.Key, error) + DeleteDevice(ctx context.Context, id string) error } func childResourceLabels(parent *corev1.Service) map[string]string { @@ -220,7 +223,7 @@ func (a *ServiceReconciler) cleanupIfRequired(ctx context.Context, svc *corev1.S // assuming k8s ordering semantics don't mess with us, that should avoid // tailscale device deletion races where we fail to notice a device that // should be removed. - sts, err := getSingleObject[appsv1.StatefulSet](ctx, a.Client, ml) + sts, err := getSingleObject[appsv1.StatefulSet](ctx, a.Client, a.operatorNamespace, ml) if err != nil { return reconcile.Result{}, fmt.Errorf("getting statefulset: %w", err) } @@ -229,7 +232,7 @@ func (a *ServiceReconciler) cleanupIfRequired(ctx context.Context, svc *corev1.S // Deletion in progress, check again later. return reconcile.Result{RequeueAfter: time.Second}, nil } - err := a.DeleteAllOf(ctx, &appsv1.StatefulSet{}, client.InNamespace(tsNamespace), client.MatchingLabels(ml), client.PropagationPolicy(metav1.DeletePropagationForeground)) + err := a.DeleteAllOf(ctx, &appsv1.StatefulSet{}, client.InNamespace(a.operatorNamespace), client.MatchingLabels(ml), client.PropagationPolicy(metav1.DeletePropagationForeground)) if err != nil { return reconcile.Result{}, fmt.Errorf("deleting statefulset: %w", err) } @@ -253,7 +256,7 @@ func (a *ServiceReconciler) cleanupIfRequired(ctx context.Context, svc *corev1.S &corev1.Secret{}, } for _, typ := range types { - if err := a.DeleteAllOf(ctx, typ, client.InNamespace(tsNamespace), client.MatchingLabels(ml)); err != nil { + if err := a.DeleteAllOf(ctx, typ, client.InNamespace(a.operatorNamespace), client.MatchingLabels(ml)); err != nil { return reconcile.Result{}, err } } @@ -366,7 +369,7 @@ func (a *ServiceReconciler) reconcileHeadlessService(ctx context.Context, svc *c hsvc := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ GenerateName: "ts-" + svc.Name + "-", - Namespace: tsNamespace, + Namespace: a.operatorNamespace, Labels: childResourceLabels(svc), }, Spec: corev1.ServiceSpec{ @@ -376,7 +379,7 @@ func (a *ServiceReconciler) reconcileHeadlessService(ctx context.Context, svc *c }, }, } - return createOrUpdate(ctx, a.Client, hsvc, func(svc *corev1.Service) { svc.Spec = hsvc.Spec }) + return createOrUpdate(ctx, a.Client, a.operatorNamespace, hsvc, func(svc *corev1.Service) { svc.Spec = hsvc.Spec }) } func (a *ServiceReconciler) createOrGetSecret(ctx context.Context, svc, hsvc *corev1.Service, tags []string) (string, error) { @@ -386,7 +389,7 @@ func (a *ServiceReconciler) createOrGetSecret(ctx context.Context, svc, hsvc *co // multiple StatefulSet replicas, we can provision -N for // those. Name: hsvc.Name + "-0", - Namespace: tsNamespace, + Namespace: a.operatorNamespace, Labels: childResourceLabels(svc), }, } @@ -399,7 +402,7 @@ func (a *ServiceReconciler) createOrGetSecret(ctx context.Context, svc, hsvc *co // Secret doesn't exist yet, create one. Initially it contains // only the Tailscale authkey, but once Tailscale starts it'll // also store the daemon state. - sts, err := getSingleObject[appsv1.StatefulSet](ctx, a.Client, childResourceLabels(svc)) + sts, err := getSingleObject[appsv1.StatefulSet](ctx, a.Client, a.operatorNamespace, childResourceLabels(svc)) if err != nil { return "", err } @@ -425,7 +428,7 @@ func (a *ServiceReconciler) createOrGetSecret(ctx context.Context, svc, hsvc *co } func (a *ServiceReconciler) getDeviceInfo(ctx context.Context, svc *corev1.Service) (id, hostname string, err error) { - sec, err := getSingleObject[corev1.Secret](ctx, a.Client, childResourceLabels(svc)) + sec, err := getSingleObject[corev1.Secret](ctx, a.Client, a.operatorNamespace, childResourceLabels(svc)) if err != nil { return "", "", err } @@ -491,7 +494,7 @@ func (a *ServiceReconciler) reconcileSTS(ctx context.Context, parentSvc, headles return nil, fmt.Errorf("failed to unmarshal proxy spec: %w", err) } container := &ss.Spec.Template.Spec.Containers[0] - container.Image = image + container.Image = a.proxyImage container.Env = append(container.Env, corev1.EnvVar{ Name: "TS_DEST_IP", @@ -503,7 +506,7 @@ func (a *ServiceReconciler) reconcileSTS(ctx context.Context, parentSvc, headles }) ss.ObjectMeta = metav1.ObjectMeta{ Name: headlessSvc.Name, - Namespace: tsNamespace, + Namespace: a.operatorNamespace, Labels: childResourceLabels(parentSvc), } ss.Spec.ServiceName = headlessSvc.Name @@ -515,7 +518,7 @@ func (a *ServiceReconciler) reconcileSTS(ctx context.Context, parentSvc, headles ss.Spec.Template.ObjectMeta.Labels = map[string]string{ "app": string(parentSvc.UID), } - return createOrUpdate(ctx, a.Client, &ss, func(s *appsv1.StatefulSet) { s.Spec = ss.Spec }) + return createOrUpdate(ctx, a.Client, a.operatorNamespace, &ss, func(s *appsv1.StatefulSet) { s.Spec = ss.Spec }) } func (a *ServiceReconciler) InjectClient(c client.Client) error { @@ -536,7 +539,7 @@ type ptrObject[T any] interface { // // obj is looked up by its Name and Namespace if Name is set, otherwise it's // looked up by labels. -func createOrUpdate[T any, O ptrObject[T]](ctx context.Context, c client.Client, obj O, update func(O)) (O, error) { +func createOrUpdate[T any, O ptrObject[T]](ctx context.Context, c client.Client, ns string, obj O, update func(O)) (O, error) { var ( existing O err error @@ -547,7 +550,7 @@ func createOrUpdate[T any, O ptrObject[T]](ctx context.Context, c client.Client, existing.SetNamespace(obj.GetNamespace()) err = c.Get(ctx, client.ObjectKeyFromObject(obj), existing) } else { - existing, err = getSingleObject[T, O](ctx, c, obj.GetLabels()) + existing, err = getSingleObject[T, O](ctx, c, ns, obj.GetLabels()) } if err == nil && existing != nil { if update != nil { @@ -571,7 +574,7 @@ func createOrUpdate[T any, O ptrObject[T]](ctx context.Context, c client.Client, // (e.g. corev1.Service) with the given labels, and returns // it. Returns nil if no objects match the labels, and an error if // more than one object matches. -func getSingleObject[T any, O ptrObject[T]](ctx context.Context, c client.Client, labels map[string]string) (O, error) { +func getSingleObject[T any, O ptrObject[T]](ctx context.Context, c client.Client, ns string, labels map[string]string) (O, error) { ret := O(new(T)) kinds, _, err := c.Scheme().ObjectKinds(ret) if err != nil { @@ -587,7 +590,7 @@ func getSingleObject[T any, O ptrObject[T]](ctx context.Context, c client.Client gvk.Kind += "List" lst := unstructured.UnstructuredList{} lst.SetGroupVersionKind(gvk) - if err := c.List(ctx, &lst, client.InNamespace(tsNamespace), client.MatchingLabels(labels)); err != nil { + if err := c.List(ctx, &lst, client.InNamespace(ns), client.MatchingLabels(labels)); err != nil { return nil, err } diff --git a/cmd/k8s-operator/main_test.go b/cmd/k8s-operator/main_test.go new file mode 100644 index 000000000..7c9126e50 --- /dev/null +++ b/cmd/k8s-operator/main_test.go @@ -0,0 +1,334 @@ +// Copyright (c) 2022 Tailscale Inc & AUTHORS All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +import ( + "context" + "strings" + "sync" + "testing" + "time" + + "github.com/google/go-cmp/cmp" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + "tailscale.com/client/tailscale" + "tailscale.com/types/ptr" +) + +func TestController(t *testing.T) { + fc := fake.NewFakeClient() + ft := &fakeTSClient{} + sr := &ServiceReconciler{ + Client: fc, + tsClient: ft, + defaultTags: []string{"tag:k8s"}, + operatorNamespace: "operator-ns", + proxyImage: "tailscale/tailscale", + } + + // Create a service that we should manage, and check that the initial round + // of objects looks right. + mustCreate(t, fc, &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + // The apiserver is supposed to set the UID, but the fake client + // doesn't. So, set it explicitly because other code later depends + // on it being set. + UID: types.UID("1234-UID"), + }, + Spec: corev1.ServiceSpec{ + ClusterIP: "10.20.30.40", + Type: corev1.ServiceTypeLoadBalancer, + LoadBalancerClass: ptr.To("tailscale"), + }, + }) + + expectRequeue(t, sr, "default", "test") + + fullName, shortName := findGenName(t, fc, "default", "test") + + expectEqual(t, fc, &corev1.Secret{ + TypeMeta: metav1.TypeMeta{ + Kind: "Secret", + APIVersion: "v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: fullName, + Namespace: "operator-ns", + ResourceVersion: "1", + Labels: map[string]string{ + "tailscale.com/managed": "true", + "tailscale.com/parent-resource": "test", + "tailscale.com/parent-resource-ns": "default", + "tailscale.com/parent-resource-type": "svc", + }, + }, + StringData: map[string]string{ + "authkey": "secret-authkey", + }, + }) + expectEqual(t, fc, &corev1.Service{ + TypeMeta: metav1.TypeMeta{ + Kind: "Service", + APIVersion: "v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: shortName, + GenerateName: "ts-test-", + Namespace: "operator-ns", + ResourceVersion: "1", + Labels: map[string]string{ + "tailscale.com/managed": "true", + "tailscale.com/parent-resource": "test", + "tailscale.com/parent-resource-ns": "default", + "tailscale.com/parent-resource-type": "svc", + }, + }, + Spec: corev1.ServiceSpec{ + Selector: map[string]string{ + "app": "1234-UID", + }, + ClusterIP: "None", + }, + }) + expectEqual(t, fc, &appsv1.StatefulSet{ + TypeMeta: metav1.TypeMeta{ + Kind: "StatefulSet", + APIVersion: "apps/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: shortName, + Namespace: "operator-ns", + ResourceVersion: "1", + Labels: map[string]string{ + "tailscale.com/managed": "true", + "tailscale.com/parent-resource": "test", + "tailscale.com/parent-resource-ns": "default", + "tailscale.com/parent-resource-type": "svc", + }, + }, + Spec: appsv1.StatefulSetSpec{ + Replicas: ptr.To[int32](1), + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "1234-UID"}, + }, + ServiceName: shortName, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + DeletionGracePeriodSeconds: ptr.To[int64](10), + Labels: map[string]string{"app": "1234-UID"}, + }, + Spec: corev1.PodSpec{ + ServiceAccountName: "proxies", + InitContainers: []corev1.Container{ + { + Name: "sysctler", + Image: "busybox", + Command: []string{"/bin/sh"}, + Args: []string{"-c", "sysctl -w net.ipv4.ip_forward=1 net.ipv6.conf.all.forwarding=1"}, + SecurityContext: &corev1.SecurityContext{ + Privileged: ptr.To(true), + }, + }, + }, + Containers: []v1.Container{ + { + Name: "tailscale", + Image: "tailscale/tailscale", + Env: []v1.EnvVar{ + {Name: "TS_USERSPACE", Value: "false"}, + {Name: "TS_AUTH_ONCE", Value: "true"}, + {Name: "TS_DEST_IP", Value: "10.20.30.40"}, + {Name: "TS_KUBE_SECRET", Value: fullName}, + }, + SecurityContext: &corev1.SecurityContext{ + Capabilities: &corev1.Capabilities{ + Add: []corev1.Capability{"NET_ADMIN"}, + }, + }, + ImagePullPolicy: "Always", + }, + }, + }, + }, + }, + }) + + // Normally the Tailscale proxy pod would come up here and write its info + // into the secret. Simulate that, then verify reconcile again and verify + // that we get to the end. + mustUpdate(t, fc, "operator-ns", fullName, func(s *corev1.Secret) { + if s.Data == nil { + s.Data = map[string][]byte{} + } + s.Data["device_id"] = []byte("ts-id-1234") + s.Data["device_fqdn"] = []byte("tailscale.device.name.") + }) + expectReconciled(t, sr, "default", "test") + expectEqual(t, fc, &corev1.Service{ + TypeMeta: metav1.TypeMeta{ + Kind: "Service", + APIVersion: "v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + ResourceVersion: "4", + Finalizers: []string{"tailscale.com/finalizer"}, + UID: types.UID("1234-UID"), + }, + Spec: corev1.ServiceSpec{ + ClusterIP: "10.20.30.40", + Type: corev1.ServiceTypeLoadBalancer, + LoadBalancerClass: ptr.To("tailscale"), + }, + Status: corev1.ServiceStatus{ + LoadBalancer: corev1.LoadBalancerStatus{ + Ingress: []corev1.LoadBalancerIngress{ + { + Hostname: "tailscale.device.name", + }, + }, + }, + }, + }) +} + +func findGenName(t *testing.T, client client.Client, ns, name string) (full, noSuffix string) { + t.Helper() + labels := map[string]string{ + LabelManaged: "true", + LabelParentName: name, + LabelParentNamespace: ns, + LabelParentType: "svc", + } + s, err := getSingleObject[corev1.Secret](context.Background(), client, "operator-ns", labels) + if err != nil { + t.Fatalf("finding secret for %q: %v", name, err) + } + return s.GetName(), strings.TrimSuffix(s.GetName(), "-0") +} + +func mustCreate(t *testing.T, client client.Client, obj client.Object) { + t.Helper() + if err := client.Create(context.Background(), obj); err != nil { + t.Fatalf("creating %q: %v", obj.GetName(), err) + } +} + +func mustUpdate[T any, O ptrObject[T]](t *testing.T, client client.Client, ns, name string, update func(O)) { + t.Helper() + obj := O(new(T)) + if err := client.Get(context.Background(), types.NamespacedName{ + Name: name, + Namespace: ns, + }, obj); err != nil { + t.Fatalf("getting %q: %v", name, err) + } + update(obj) + if err := client.Update(context.Background(), obj); err != nil { + t.Fatalf("updating %q: %v", name, err) + } +} + +func expectEqual[T any, O ptrObject[T]](t *testing.T, client client.Client, want O) { + t.Helper() + got := O(new(T)) + if err := client.Get(context.Background(), types.NamespacedName{ + Name: want.GetName(), + Namespace: want.GetNamespace(), + }, got); err != nil { + t.Fatalf("getting %q: %v", want.GetName(), err) + } + if diff := cmp.Diff(got, want); diff != "" { + t.Fatalf("unexpected object (-got +want):\n%s", diff) + } +} + +func expectReconciled(t *testing.T, sr *ServiceReconciler, ns, name string) { + t.Helper() + req := reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: name, + Namespace: ns, + }, + } + res, err := sr.Reconcile(context.Background(), req) + if err != nil { + t.Fatalf("Reconcile: unexpected error: %v", err) + } + if res.Requeue { + t.Fatalf("unexpected immediate requeue") + } + if res.RequeueAfter != 0 { + t.Fatalf("unexpected timed requeue") + } +} + +func expectRequeue(t *testing.T, sr *ServiceReconciler, ns, name string) { + t.Helper() + req := reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: name, + Namespace: ns, + }, + } + res, err := sr.Reconcile(context.Background(), req) + if err != nil { + t.Fatalf("Reconcile: unexpected error: %v", err) + } + if res.Requeue { + t.Fatalf("unexpected immediate requeue") + } + if res.RequeueAfter == 0 { + t.Fatalf("expected timed requeue, got success") + } +} + +type fakeTSClient struct { + sync.Mutex + keyRequests []tailscale.KeyCapabilities + deleted []string +} + +func (c *fakeTSClient) CreateKey(ctx context.Context, caps tailscale.KeyCapabilities) (string, *tailscale.Key, error) { + c.Lock() + defer c.Unlock() + c.keyRequests = append(c.keyRequests, caps) + k := &tailscale.Key{ + ID: "key", + Created: time.Now(), + Expires: time.Now().Add(24 * time.Hour), + Capabilities: caps, + } + return "secret-authkey", k, nil +} + +func (c *fakeTSClient) DeleteDevice(ctx context.Context, deviceID string) error { + c.Lock() + defer c.Unlock() + c.deleted = append(c.deleted, deviceID) + return nil +} + +func (c *fakeTSClient) KeyRequests() []tailscale.KeyCapabilities { + c.Lock() + defer c.Unlock() + return c.keyRequests +} + +func (c *fakeTSClient) Deleted() []string { + c.Lock() + defer c.Unlock() + return c.deleted +} diff --git a/go.mod b/go.mod index d2af173c4..e69258390 100644 --- a/go.mod +++ b/go.mod @@ -142,6 +142,7 @@ require ( github.com/emirpasic/gods v1.12.0 // indirect github.com/esimonov/ifshort v1.0.3 // indirect github.com/ettle/strcase v0.1.1 // indirect + github.com/evanphx/json-patch v4.12.0+incompatible // indirect github.com/evanphx/json-patch/v5 v5.6.0 // indirect github.com/fatih/color v1.13.0 // indirect github.com/fatih/structtag v1.2.0 // indirect diff --git a/go.sum b/go.sum index 07f2416bf..f3e51ee60 100644 --- a/go.sum +++ b/go.sum @@ -293,6 +293,7 @@ github.com/ettle/strcase v0.1.1 h1:htFueZyVeE1XNnMEfbqp5r67qAN/4r6ya1ysq8Q+Zcw= github.com/ettle/strcase v0.1.1/go.mod h1:hzDLsPC7/lwKyBOywSHEP89nt2pDgdy+No1NBA9o9VY= github.com/evanphx/json-patch v0.5.2/go.mod h1:ZWS5hhDbVDyob71nXKNL0+PWn6ToqBHMikGIFbs31qQ= github.com/evanphx/json-patch v4.12.0+incompatible h1:4onqiflcdA9EOZ4RxV643DvftH5pOlLGNtQ5lPWQu84= +github.com/evanphx/json-patch v4.12.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk= github.com/evanphx/json-patch/v5 v5.6.0 h1:b91NhWfaz02IuVxO9faSllyAtNXHMPkC5J8sJCLunww= github.com/evanphx/json-patch/v5 v5.6.0/go.mod h1:G79N1coSVB93tBe7j6PhzjmR3/2VvlbKOFpnXhI9Bw4= github.com/evanw/esbuild v0.14.53 h1:9uU73SZUmP1jRQhaC6hPm9aoqFGYlPwfk7OrhG6AhpQ=