From ddbcbbbe528c3a15a34e9791fb67e23f664b0a1a Mon Sep 17 00:00:00 2001 From: chaosinthecrd Date: Mon, 12 May 2025 17:41:40 +0100 Subject: [PATCH] k8s-operator: adding tests (still need multicluster and backwards compat w/ svc Signed-off-by: chaosinthecrd --- cmd/k8s-operator/ingress-for-pg_test.go | 2 +- cmd/k8s-operator/svc-for-pg.go | 9 +- cmd/k8s-operator/svc-for-pg_test.go | 394 ++++++++++++++++++++++++ cmd/k8s-operator/testutils_test.go | 9 + 4 files changed, 411 insertions(+), 3 deletions(-) create mode 100644 cmd/k8s-operator/svc-for-pg_test.go diff --git a/cmd/k8s-operator/ingress-for-pg_test.go b/cmd/k8s-operator/ingress-for-pg_test.go index 0ad424bd6..4e89f3048 100644 --- a/cmd/k8s-operator/ingress-for-pg_test.go +++ b/cmd/k8s-operator/ingress-for-pg_test.go @@ -624,7 +624,7 @@ func verifyServeConfig(t *testing.T, fc client.Client, serviceName string, wantH func verifyTailscaledConfig(t *testing.T, fc client.Client, expectedServices []string) { t.Helper() var expected string - if expectedServices != nil { + if len(expectedServices) > 0 { expectedServicesJSON, err := json.Marshal(expectedServices) if err != nil { t.Fatalf("marshaling expected services: %v", err) diff --git a/cmd/k8s-operator/svc-for-pg.go b/cmd/k8s-operator/svc-for-pg.go index 3707e17ee..73e97fef0 100644 --- a/cmd/k8s-operator/svc-for-pg.go +++ b/cmd/k8s-operator/svc-for-pg.go @@ -530,7 +530,7 @@ func (r *HAServiceReconciler) deleteFinalizer(ctx context.Context, svc *corev1.S svc.Finalizers = slices.DeleteFunc(svc.Finalizers, func(f string) bool { return f == finalizerName }) - logger.Debug("ensure %q finalizer is removed", finalizerName) + logger.Debugf("ensure %q finalizer is removed", finalizerName) if err := r.Update(ctx, svc); err != nil { return fmt.Errorf("failed to remove finalizer %q: %w", finalizerName, err) @@ -652,7 +652,12 @@ func (a *HAServiceReconciler) backendRoutesSetup(ctx context.Context, serviceNam return false, nil } if !reflect.DeepEqual(gotCfgs.Configs.GetConfig(serviceName), wantsCfg) { - logger.Debugf("Pod %q has config %q, but wants %q", pod.Name, gotCfgs.Configs.GetConfig(serviceName), wantsCfg) + wantj, err := json.Marshal(wantsCfg) + if err != nil { + return false, fmt.Errorf("error marshalling ingress config: %w", err) + } + + logger.Debugf("Pod %q has config %q, but wants %q", pod.Name, string(gotCfgB), wantj) return false, nil } return true, nil diff --git a/cmd/k8s-operator/svc-for-pg_test.go b/cmd/k8s-operator/svc-for-pg_test.go new file mode 100644 index 000000000..239b56ff7 --- /dev/null +++ b/cmd/k8s-operator/svc-for-pg_test.go @@ -0,0 +1,394 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +//go:build !plan9 + +package main + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "math/rand/v2" + "net/http" + "net/netip" + "testing" + + "go.uber.org/zap" + corev1 "k8s.io/api/core/v1" + discoveryv1 "k8s.io/api/discovery/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/record" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "tailscale.com/internal/client/tailscale" + "tailscale.com/ipn/ipnstate" + tsoperator "tailscale.com/k8s-operator" + tsapi "tailscale.com/k8s-operator/apis/v1alpha1" + "tailscale.com/kube/ingressservices" + "tailscale.com/tstest" + "tailscale.com/types/ptr" + "tailscale.com/util/mak" + + "tailscale.com/tailcfg" +) + +func TestServicePGReconciler(t *testing.T) { + svcPGR, stateSecret, fc, ft := setupServiceTest(t) + svcs := []*corev1.Service{} + config := []string{} + for i := range 4 { + svc, _ := setupTestService(t, fmt.Sprintf("test-svc-%d", i), "", fmt.Sprintf("1.2.3.%d", i), fc, stateSecret) + svcs = append(svcs, svc) + + // Verify initial reconciliation + expectReconciled(t, svcPGR, "default", svc.Name) + + config = append(config, fmt.Sprintf("svc:default-%s", svc.Name)) + verifyVIPService(t, ft, fmt.Sprintf("svc:default-%s", svc.Name), []string{"do-not-validate"}) + verifyTailscaledConfig(t, fc, config) + } + + for i, svc := range svcs { + if err := fc.Delete(context.Background(), svc); err != nil { + t.Fatalf("deleting Service: %v", err) + } + + expectReconciled(t, svcPGR, "default", svc.Name) + + // Verify the ConfigMap was cleaned up + cm := &corev1.ConfigMap{} + if err := fc.Get(context.Background(), types.NamespacedName{ + Name: "test-pg-ingress-config", + Namespace: "operator-ns", + }, cm); err != nil { + t.Fatalf("getting ConfigMap: %v", err) + } + + cfgs := ingressservices.Configs{} + if err := json.Unmarshal(cm.BinaryData[ingressservices.IngressConfigKey], &cfgs); err != nil { + t.Fatalf("unmarshaling serve config: %v", err) + } + + if len(cfgs) > len(svcs)-(i+1) { + t.Error("serve config not cleaned up") + } + + config = removeEl(config, fmt.Sprintf("svc:default-%s", svc.Name)) + verifyTailscaledConfig(t, fc, config) + } +} + +func TestServicePGReconciler_UpdateHostname(t *testing.T) { + svcPGR, stateSecret, fc, ft := setupServiceTest(t) + + cip := "4.1.6.7" + svc, _ := setupTestService(t, "test-service", "", cip, fc, stateSecret) + + expectReconciled(t, svcPGR, "default", svc.Name) + + verifyVIPService(t, ft, fmt.Sprintf("svc:default-%s", svc.Name), []string{"do-not-validate"}) + verifyTailscaledConfig(t, fc, []string{fmt.Sprintf("svc:default-%s", svc.Name)}) + + hostname := "foobarbaz" + mustUpdate(t, fc, svc.Namespace, svc.Name, func(s *corev1.Service) { + mak.Set(&s.Annotations, AnnotationHostname, hostname) + }) + + // NOTE: we need to update hte ingress config secret because there is no containerboot in the fake proxy pod + updateIngressConfigSecret(t, fc, stateSecret, hostname, cip) + expectReconciled(t, svcPGR, "default", svc.Name) + + verifyVIPService(t, ft, fmt.Sprintf("svc:%s", hostname), []string{"do-not-validate"}) + verifyTailscaledConfig(t, fc, []string{fmt.Sprintf("svc:%s", hostname)}) + + _, err := ft.GetVIPService(context.Background(), tailcfg.ServiceName(fmt.Sprintf("svc:default-%s", svc.Name))) + if err == nil { + t.Fatalf("svc:my-svc not cleaned up") + } + var errResp *tailscale.ErrResponse + if !errors.As(err, &errResp) || errResp.Status != http.StatusNotFound { + t.Fatalf("unexpected error: %v", err) + } +} + +func setupServiceTest(t *testing.T) (*HAServiceReconciler, *corev1.Secret, client.Client, *fakeTSClient) { + // Pre-create the ProxyGroup + pg := &tsapi.ProxyGroup{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pg", + Generation: 1, + }, + Spec: tsapi.ProxyGroupSpec{ + Type: tsapi.ProxyGroupTypeIngress, + }, + } + + // Pre-create the ConfigMap for the ProxyGroup + pgConfigMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pg-ingress-config", + Namespace: "operator-ns", + }, + BinaryData: map[string][]byte{ + "serve-config.json": []byte(`{"Services":{}}`), + }, + } + + // Pre-create a config Secret for the ProxyGroup + pgCfgSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: pgConfigSecretName("test-pg", 0), + Namespace: "operator-ns", + Labels: pgSecretLabels("test-pg", "config"), + }, + Data: map[string][]byte{ + tsoperator.TailscaledConfigFileName(106): []byte("{}"), + }, + } + + pgStateSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pg-0", + Namespace: "operator-ns", + }, + Data: map[string][]byte{}, + } + + pgPod := &corev1.Pod{ + TypeMeta: metav1.TypeMeta{Kind: "Pod", APIVersion: "v1"}, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pg-0", + Namespace: "operator-ns", + }, + Status: corev1.PodStatus{ + PodIPs: []corev1.PodIP{ + { + IP: "4.3.2.1", + }, + }, + }, + } + + // NOTE: not sure if we need to setup the state secret here... + + fc := fake.NewClientBuilder(). + WithScheme(tsapi.GlobalScheme). + WithObjects(pg, pgCfgSecret, pgConfigMap, pgPod, pgStateSecret). + WithStatusSubresource(pg). + WithIndex(new(corev1.Service), indexIngressProxyGroup, indexPGIngresses). + Build() + + // Set ProxyGroup status to ready + pg.Status.Conditions = []metav1.Condition{ + { + Type: string(tsapi.ProxyGroupReady), + Status: metav1.ConditionTrue, + ObservedGeneration: 1, + }, + } + if err := fc.Status().Update(context.Background(), pg); err != nil { + t.Fatal(err) + } + fakeTsnetServer := &fakeTSNetServer{certDomains: []string{"foo.com"}} + + ft := &fakeTSClient{} + zl, err := zap.NewDevelopment() + if err != nil { + t.Fatal(err) + } + + lc := &fakeLocalClient{ + status: &ipnstate.Status{ + CurrentTailnet: &ipnstate.TailnetStatus{ + MagicDNSSuffix: "ts.net", + }, + }, + } + + cl := tstest.NewClock(tstest.ClockOpts{}) + svcPGR := &HAServiceReconciler{ + Client: fc, + tsClient: ft, + clock: cl, + defaultTags: []string{"tag:k8s"}, + tsNamespace: "operator-ns", + tsnetServer: fakeTsnetServer, + logger: zl.Sugar(), + recorder: record.NewFakeRecorder(10), + lc: lc, + } + + return svcPGR, pgStateSecret, fc, ft +} + +func setupTestService(t *testing.T, svcName string, hostname string, clusterIP string, fc client.Client, stateSecret *corev1.Secret) (svc *corev1.Service, eps *discoveryv1.EndpointSlice) { + uid := rand.IntN(100) + svc = &corev1.Service{ + TypeMeta: metav1.TypeMeta{Kind: "Service", APIVersion: "v1"}, + ObjectMeta: metav1.ObjectMeta{ + Name: svcName, + Namespace: "default", + UID: types.UID(fmt.Sprintf("%d-UID", uid)), + Annotations: map[string]string{ + "tailscale.com/proxy-group": "test-pg", + }, + }, + Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeLoadBalancer, + LoadBalancerClass: ptr.To("tailscale"), + ClusterIP: clusterIP, + ClusterIPs: []string{clusterIP}, + }, + } + + eps = &discoveryv1.EndpointSlice{ + TypeMeta: metav1.TypeMeta{Kind: "EndpointSlice", APIVersion: "v1"}, + ObjectMeta: metav1.ObjectMeta{ + Name: svcName, + Namespace: "default", + Labels: map[string]string{ + discoveryv1.LabelServiceName: svcName, + }, + }, + AddressType: discoveryv1.AddressTypeIPv4, + Endpoints: []discoveryv1.Endpoint{ + { + Addresses: []string{"4.3.2.1"}, + Conditions: discoveryv1.EndpointConditions{ + Ready: ptr.To(true), + }, + }, + }, + } + + updateIngressConfigSecret(t, fc, stateSecret, fmt.Sprintf("default-%s", svcName), clusterIP) + + mustCreate(t, fc, svc) + mustCreate(t, fc, eps) + + return svc, eps +} + +// func TestServicePGReconciler_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 VIPService 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") +// +// vipSvc, err := ft.GetVIPService(context.Background(), "svc:my-svc") +// if err != nil { +// t.Fatalf("getting VIPService: %v", err) +// } +// if vipSvc == nil { +// t.Fatal("VIPService not found") +// } +// +// o, err := parseOwnerAnnotation(vipSvc) +// 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 VIPService 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") +// +// vipSvc, err = ft.GetVIPService(context.Background(), "svc:my-svc") +// if err != nil { +// t.Fatalf("getting VIPService after deletion: %v", err) +// } +// if vipSvc == nil { +// t.Fatal("VIPService was incorrectly deleted") +// } +// +// o, err = parseOwnerAnnotation(vipSvc) +// 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 removeEl(s []string, value string) []string { + result := s[:0] + for _, v := range s { + if v != value { + result = append(result, v) + } + } + return result +} + +func updateIngressConfigSecret(t *testing.T, fc client.Client, stateSecret *corev1.Secret, serviceName string, clusterIP string) { + ingressConfig := ingressservices.Configs{ + fmt.Sprintf("svc:%s", serviceName): ingressservices.Config{ + IPv4Mapping: &ingressservices.Mapping{ + VIPServiceIP: netip.MustParseAddr(vipTestIP), + ClusterIP: netip.MustParseAddr(clusterIP), + }, + }, + } + + ingressStatus := ingressservices.Status{ + Configs: ingressConfig, + PodIPv4: "4.3.2.1", + } + + icJson, err := json.Marshal(ingressStatus) + if err != nil { + t.Fatal("failed to json marshal ingress config") + } + + mustUpdate(t, fc, stateSecret.Namespace, stateSecret.Name, func(sec *corev1.Secret) { + mak.Set(&sec.Data, ingressservices.IngressConfigKey, icJson) + }) +} diff --git a/cmd/k8s-operator/testutils_test.go b/cmd/k8s-operator/testutils_test.go index f47f96e44..209864011 100644 --- a/cmd/k8s-operator/testutils_test.go +++ b/cmd/k8s-operator/testutils_test.go @@ -38,6 +38,10 @@ import ( "tailscale.com/util/mak" ) +const ( + vipTestIP = "5.6.7.8" +) + // confgOpts contains configuration options for creating cluster resources for // Tailscale proxies. type configOpts struct { @@ -895,6 +899,11 @@ func (c *fakeTSClient) CreateOrUpdateVIPService(ctx context.Context, svc *tailsc if c.vipServices == nil { c.vipServices = make(map[tailcfg.ServiceName]*tailscale.VIPService) } + + if svc.Addrs == nil { + svc.Addrs = []string{vipTestIP} + } + c.vipServices[svc.Name] = svc return nil }