k8s-operator: adding tests (still need multicluster and backwards compat w/ svc

Signed-off-by: chaosinthecrd <tom@tmlabs.co.uk>

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)

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

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)
+	})
+}

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
 }