From e11ff284435d3789136e583b82c4e416f6c7f925 Mon Sep 17 00:00:00 2001 From: Irbe Krumina Date: Fri, 14 Feb 2025 18:07:17 +0000 Subject: [PATCH] cmd/k8s-operator: allow to optionally configure an HTTP endpoint for the HA Ingress (#14986) Updates tailscale/corp#24795 Signed-off-by: Irbe Krumina --- cmd/k8s-operator/ingress-for-pg.go | 61 +++- cmd/k8s-operator/ingress-for-pg_test.go | 413 ++++++++++++++++++------ 2 files changed, 358 insertions(+), 116 deletions(-) diff --git a/cmd/k8s-operator/ingress-for-pg.go b/cmd/k8s-operator/ingress-for-pg.go index 5a67a891f..b07882deb 100644 --- a/cmd/k8s-operator/ingress-for-pg.go +++ b/cmd/k8s-operator/ingress-for-pg.go @@ -46,6 +46,9 @@ const ( FinalizerNamePG = "tailscale.com/ingress-pg-finalizer" indexIngressProxyGroup = ".metadata.annotations.ingress-proxy-group" + // annotationHTTPEndpoint can be used to configure the Ingress to expose an HTTP endpoint to tailnet (as + // well as the default HTTPS endpoint). + annotationHTTPEndpoint = "tailscale.com/http-endpoint" ) var gaugePGIngressResources = clientmetric.NewGauge(kubetypes.MetricIngressPGResourceCount) @@ -202,16 +205,16 @@ func (a *IngressPGReconciler) maybeProvision(ctx context.Context, hostname strin // 3. Ensure that the serve config for the ProxyGroup contains the VIPService cm, cfg, err := a.proxyGroupServeConfig(ctx, pgName) if err != nil { - return fmt.Errorf("error getting ingress serve config: %w", err) + return fmt.Errorf("error getting Ingress serve config: %w", err) } if cm == nil { - logger.Infof("no ingress serve config ConfigMap found, unable to update serve config. Ensure that ProxyGroup is healthy.") + logger.Infof("no Ingress serve config ConfigMap found, unable to update serve config. Ensure that ProxyGroup is healthy.") return nil } ep := ipn.HostPort(fmt.Sprintf("%s:443", dnsName)) handlers, err := handlersForIngress(ctx, ing, a.Client, a.recorder, dnsName, logger) if err != nil { - return fmt.Errorf("failed to get handlers for ingress: %w", err) + return fmt.Errorf("failed to get handlers for Ingress: %w", err) } ingCfg := &ipn.ServiceConfig{ TCP: map[uint16]*ipn.TCPPortHandler{ @@ -225,6 +228,19 @@ func (a *IngressPGReconciler) maybeProvision(ctx context.Context, hostname strin }, }, } + + // Add HTTP endpoint if configured. + if isHTTPEndpointEnabled(ing) { + logger.Infof("exposing Ingress over HTTP") + epHTTP := ipn.HostPort(fmt.Sprintf("%s:80", dnsName)) + ingCfg.TCP[80] = &ipn.TCPPortHandler{ + HTTP: true, + } + ingCfg.Web[epHTTP] = &ipn.WebServerConfig{ + Handlers: handlers, + } + } + var gotCfg *ipn.ServiceConfig if cfg != nil && cfg.Services != nil { gotCfg = cfg.Services[serviceName] @@ -248,16 +264,23 @@ func (a *IngressPGReconciler) maybeProvision(ctx context.Context, hostname strin tags = strings.Split(tstr, ",") } + vipPorts := []string{"443"} // always 443 for Ingress + if isHTTPEndpointEnabled(ing) { + vipPorts = append(vipPorts, "80") + } + vipSvc := &VIPService{ Name: serviceName, Tags: tags, - Ports: []string{"443"}, // always 443 for Ingress + Ports: vipPorts, Comment: fmt.Sprintf(VIPSvcOwnerRef, ing.UID), } if existingVIPSvc != nil { vipSvc.Addrs = existingVIPSvc.Addrs } - if existingVIPSvc == nil || !reflect.DeepEqual(vipSvc.Tags, existingVIPSvc.Tags) { + if existingVIPSvc == nil || + !reflect.DeepEqual(vipSvc.Tags, existingVIPSvc.Tags) || + !reflect.DeepEqual(vipSvc.Ports, existingVIPSvc.Ports) { logger.Infof("Ensuring VIPService %q exists and is up to date", hostname) if err := a.tsClient.createOrUpdateVIPService(ctx, vipSvc); err != nil { logger.Infof("error creating VIPService: %v", err) @@ -267,16 +290,22 @@ func (a *IngressPGReconciler) maybeProvision(ctx context.Context, hostname strin // 5. Update Ingress status oldStatus := ing.Status.DeepCopy() - // TODO(irbekrm): once we have ingress ProxyGroup, we can determine if instances are ready to route traffic to the VIPService + ports := []networkingv1.IngressPortStatus{ + { + Protocol: "TCP", + Port: 443, + }, + } + if isHTTPEndpointEnabled(ing) { + ports = append(ports, networkingv1.IngressPortStatus{ + Protocol: "TCP", + Port: 80, + }) + } ing.Status.LoadBalancer.Ingress = []networkingv1.IngressLoadBalancerIngress{ { Hostname: dnsName, - Ports: []networkingv1.IngressPortStatus{ - { - Protocol: "TCP", - Port: 443, - }, - }, + Ports: ports, }, } if apiequality.Semantic.DeepEqual(oldStatus, ing.Status) { @@ -569,3 +598,11 @@ func (a *IngressPGReconciler) deleteVIPServiceIfExists(ctx context.Context, name } return nil } + +// isHTTPEndpointEnabled returns true if the Ingress has been configured to expose an HTTP endpoint to tailnet. +func isHTTPEndpointEnabled(ing *networkingv1.Ingress) bool { + if ing == nil { + return false + } + return ing.Annotations[annotationHTTPEndpoint] == "enabled" +} diff --git a/cmd/k8s-operator/ingress-for-pg_test.go b/cmd/k8s-operator/ingress-for-pg_test.go index 9317a44d4..ee8a94336 100644 --- a/cmd/k8s-operator/ingress-for-pg_test.go +++ b/cmd/k8s-operator/ingress-for-pg_test.go @@ -8,6 +8,8 @@ package main import ( "context" "encoding/json" + "maps" + "reflect" "testing" "slices" @@ -18,81 +20,18 @@ import ( 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/ipn" "tailscale.com/ipn/ipnstate" tsapi "tailscale.com/k8s-operator/apis/v1alpha1" + "tailscale.com/tailcfg" "tailscale.com/types/ptr" ) func TestIngressPGReconciler(t *testing.T) { - tsIngressClass := &networkingv1.IngressClass{ - ObjectMeta: metav1.ObjectMeta{Name: "tailscale"}, - Spec: networkingv1.IngressClassSpec{Controller: "tailscale.com/ts-ingress"}, - } + ingPGR, fc, ft := setupIngressTest(t) - // 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":{}}`), - }, - } - - fc := fake.NewClientBuilder(). - WithScheme(tsapi.GlobalScheme). - WithObjects(pg, pgConfigMap, tsIngressClass). - WithStatusSubresource(pg). - Build() - mustUpdateStatus(t, fc, "", pg.Name, func(pg *tsapi.ProxyGroup) { - pg.Status.Conditions = []metav1.Condition{ - { - Type: string(tsapi.ProxyGroupReady), - Status: metav1.ConditionTrue, - ObservedGeneration: 1, - }, - } - }) - ft := &fakeTSClient{} - fakeTsnetServer := &fakeTSNetServer{certDomains: []string{"foo.com"}} - zl, err := zap.NewDevelopment() - if err != nil { - t.Fatal(err) - } - - lc := &fakeLocalClient{ - status: &ipnstate.Status{ - CurrentTailnet: &ipnstate.TailnetStatus{ - MagicDNSSuffix: "ts.net", - }, - }, - } - ingPGR := &IngressPGReconciler{ - Client: fc, - tsClient: ft, - tsnetServer: fakeTsnetServer, - defaultTags: []string{"tag:k8s"}, - tsNamespace: "operator-ns", - logger: zl.Sugar(), - recorder: record.NewFakeRecorder(10), - lc: lc, - } - - // Test 1: Default tags ing := &networkingv1.Ingress{ TypeMeta: metav1.TypeMeta{Kind: "Ingress", APIVersion: "networking.k8s.io/v1"}, ObjectMeta: metav1.ObjectMeta{ @@ -122,8 +61,74 @@ func TestIngressPGReconciler(t *testing.T) { // Verify initial reconciliation expectReconciled(t, ingPGR, "default", "test-ingress") + verifyServeConfig(t, fc, "svc:my-svc", false) + verifyVIPService(t, ft, "svc:my-svc", []string{"443"}) - // Get and verify the ConfigMap was updated + mustUpdate(t, fc, "default", "test-ingress", func(ing *networkingv1.Ingress) { + ing.Annotations["tailscale.com/tags"] = "tag:custom,tag:test" + }) + expectReconciled(t, ingPGR, "default", "test-ingress") + + // Verify VIPService uses custom tags + 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 created") + } + wantTags := []string{"tag:custom", "tag:test"} // custom tags only + gotTags := slices.Clone(vipSvc.Tags) + slices.Sort(gotTags) + slices.Sort(wantTags) + if !slices.Equal(gotTags, wantTags) { + t.Errorf("incorrect VIPService tags: got %v, want %v", gotTags, wantTags) + } + + // Create second Ingress + ing2 := &networkingv1.Ingress{ + TypeMeta: metav1.TypeMeta{Kind: "Ingress", APIVersion: "networking.k8s.io/v1"}, + ObjectMeta: metav1.ObjectMeta{ + Name: "my-other-ingress", + Namespace: "default", + UID: types.UID("5678-UID"), + Annotations: map[string]string{ + "tailscale.com/proxy-group": "test-pg", + }, + }, + Spec: networkingv1.IngressSpec{ + IngressClassName: ptr.To("tailscale"), + DefaultBackend: &networkingv1.IngressBackend{ + Service: &networkingv1.IngressServiceBackend{ + Name: "test", + Port: networkingv1.ServiceBackendPort{ + Number: 8080, + }, + }, + }, + TLS: []networkingv1.IngressTLS{ + {Hosts: []string{"my-other-svc.tailnetxyz.ts.net"}}, + }, + }, + } + mustCreate(t, fc, ing2) + + // Verify second Ingress reconciliation + expectReconciled(t, ingPGR, "default", "my-other-ingress") + verifyServeConfig(t, fc, "svc:my-other-svc", false) + verifyVIPService(t, ft, "svc:my-other-svc", []string{"443"}) + + // Verify first Ingress is still working + verifyServeConfig(t, fc, "svc:my-svc", false) + verifyVIPService(t, ft, "svc:my-svc", []string{"443"}) + + // Delete second Ingress + if err := fc.Delete(context.Background(), ing2); err != nil { + t.Fatalf("deleting second Ingress: %v", err) + } + expectReconciled(t, ingPGR, "default", "my-other-ingress") + + // Verify second Ingress cleanup cm := &corev1.ConfigMap{} if err := fc.Get(context.Background(), types.NamespacedName{ Name: "test-pg-ingress-config", @@ -137,46 +142,16 @@ func TestIngressPGReconciler(t *testing.T) { t.Fatalf("unmarshaling serve config: %v", err) } + // Verify first Ingress is still configured if cfg.Services["svc:my-svc"] == nil { - t.Error("expected serve config to contain VIPService configuration") + t.Error("first Ingress service config was incorrectly removed") + } + // Verify second Ingress was cleaned up + if cfg.Services["svc:my-other-svc"] != nil { + t.Error("second Ingress service config was not cleaned up") } - // Verify VIPService uses default tags - 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 created") - } - wantTags := []string{"tag:k8s"} // default tags - if !slices.Equal(vipSvc.Tags, wantTags) { - t.Errorf("incorrect VIPService tags: got %v, want %v", vipSvc.Tags, wantTags) - } - - // Test 2: Custom tags - mustUpdate(t, fc, "default", "test-ingress", func(ing *networkingv1.Ingress) { - ing.Annotations["tailscale.com/tags"] = "tag:custom,tag:test" - }) - expectReconciled(t, ingPGR, "default", "test-ingress") - - // Verify VIPService uses custom tags - 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 created") - } - wantTags = []string{"tag:custom", "tag:test"} // custom tags only - gotTags := slices.Clone(vipSvc.Tags) - slices.Sort(gotTags) - slices.Sort(wantTags) - if !slices.Equal(gotTags, wantTags) { - t.Errorf("incorrect VIPService tags: got %v, want %v", gotTags, wantTags) - } - - // Delete the Ingress and verify cleanup + // Delete the first Ingress and verify cleanup if err := fc.Delete(context.Background(), ing); err != nil { t.Fatalf("deleting Ingress: %v", err) } @@ -335,3 +310,233 @@ func TestValidateIngress(t *testing.T) { }) } } + +func TestIngressPGReconciler_HTTPEndpoint(t *testing.T) { + ingPGR, fc, ft := setupIngressTest(t) + + // Create test Ingress with HTTP endpoint enabled + 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", + "tailscale.com/http-endpoint": "enabled", + }, + }, + Spec: networkingv1.IngressSpec{ + IngressClassName: ptr.To("tailscale"), + DefaultBackend: &networkingv1.IngressBackend{ + Service: &networkingv1.IngressServiceBackend{ + Name: "test", + Port: networkingv1.ServiceBackendPort{ + Number: 8080, + }, + }, + }, + TLS: []networkingv1.IngressTLS{ + {Hosts: []string{"my-svc"}}, + }, + }, + } + if err := fc.Create(context.Background(), ing); err != nil { + t.Fatal(err) + } + + // Verify initial reconciliation with HTTP enabled + expectReconciled(t, ingPGR, "default", "test-ingress") + verifyVIPService(t, ft, "svc:my-svc", []string{"80", "443"}) + verifyServeConfig(t, fc, "svc:my-svc", true) + + // Verify Ingress status + ing = &networkingv1.Ingress{} + if err := fc.Get(context.Background(), types.NamespacedName{ + Name: "test-ingress", + Namespace: "default", + }, ing); err != nil { + t.Fatal(err) + } + + wantStatus := []networkingv1.IngressPortStatus{ + {Port: 443, Protocol: "TCP"}, + {Port: 80, Protocol: "TCP"}, + } + if !reflect.DeepEqual(ing.Status.LoadBalancer.Ingress[0].Ports, wantStatus) { + t.Errorf("incorrect status ports: got %v, want %v", + ing.Status.LoadBalancer.Ingress[0].Ports, wantStatus) + } + + // Remove HTTP endpoint annotation + mustUpdate(t, fc, "default", "test-ingress", func(ing *networkingv1.Ingress) { + delete(ing.Annotations, "tailscale.com/http-endpoint") + }) + + // Verify reconciliation after removing HTTP + expectReconciled(t, ingPGR, "default", "test-ingress") + verifyVIPService(t, ft, "svc:my-svc", []string{"443"}) + verifyServeConfig(t, fc, "svc:my-svc", false) + + // Verify Ingress status + ing = &networkingv1.Ingress{} + if err := fc.Get(context.Background(), types.NamespacedName{ + Name: "test-ingress", + Namespace: "default", + }, ing); err != nil { + t.Fatal(err) + } + + wantStatus = []networkingv1.IngressPortStatus{ + {Port: 443, Protocol: "TCP"}, + } + if !reflect.DeepEqual(ing.Status.LoadBalancer.Ingress[0].Ports, wantStatus) { + t.Errorf("incorrect status ports: got %v, want %v", + ing.Status.LoadBalancer.Ingress[0].Ports, wantStatus) + } +} + +func verifyVIPService(t *testing.T, ft *fakeTSClient, serviceName string, wantPorts []string) { + t.Helper() + vipSvc, err := ft.getVIPService(context.Background(), tailcfg.ServiceName(serviceName)) + if err != nil { + t.Fatalf("getting VIPService %q: %v", serviceName, err) + } + if vipSvc == nil { + t.Fatalf("VIPService %q not created", serviceName) + } + gotPorts := slices.Clone(vipSvc.Ports) + slices.Sort(gotPorts) + slices.Sort(wantPorts) + if !slices.Equal(gotPorts, wantPorts) { + t.Errorf("incorrect ports for VIPService %q: got %v, want %v", serviceName, gotPorts, wantPorts) + } +} + +func verifyServeConfig(t *testing.T, fc client.Client, serviceName string, wantHTTP bool) { + t.Helper() + + 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) + } + + cfg := &ipn.ServeConfig{} + if err := json.Unmarshal(cm.BinaryData["serve-config.json"], cfg); err != nil { + t.Fatalf("unmarshaling serve config: %v", err) + } + + t.Logf("Looking for service %q in config: %+v", serviceName, cfg) + + svc := cfg.Services[tailcfg.ServiceName(serviceName)] + if svc == nil { + t.Fatalf("service %q not found in serve config, services: %+v", serviceName, maps.Keys(cfg.Services)) + } + + wantHandlers := 1 + if wantHTTP { + wantHandlers = 2 + } + + // Check TCP handlers + if len(svc.TCP) != wantHandlers { + t.Errorf("incorrect number of TCP handlers for service %q: got %d, want %d", serviceName, len(svc.TCP), wantHandlers) + } + if wantHTTP { + if h, ok := svc.TCP[uint16(80)]; !ok { + t.Errorf("HTTP (port 80) handler not found for service %q", serviceName) + } else if !h.HTTP { + t.Errorf("HTTP not enabled for port 80 handler for service %q", serviceName) + } + } + if h, ok := svc.TCP[uint16(443)]; !ok { + t.Errorf("HTTPS (port 443) handler not found for service %q", serviceName) + } else if !h.HTTPS { + t.Errorf("HTTPS not enabled for port 443 handler for service %q", serviceName) + } + + // Check Web handlers + if len(svc.Web) != wantHandlers { + t.Errorf("incorrect number of Web handlers for service %q: got %d, want %d", serviceName, len(svc.Web), wantHandlers) + } +} + +func setupIngressTest(t *testing.T) (*IngressPGReconciler, client.Client, *fakeTSClient) { + t.Helper() + + tsIngressClass := &networkingv1.IngressClass{ + ObjectMeta: metav1.ObjectMeta{Name: "tailscale"}, + Spec: networkingv1.IngressClassSpec{Controller: "tailscale.com/ts-ingress"}, + } + + // 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":{}}`), + }, + } + + fc := fake.NewClientBuilder(). + WithScheme(tsapi.GlobalScheme). + WithObjects(pg, pgConfigMap, tsIngressClass). + WithStatusSubresource(pg). + 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) + } + + ft := &fakeTSClient{} + fakeTsnetServer := &fakeTSNetServer{certDomains: []string{"foo.com"}} + zl, err := zap.NewDevelopment() + if err != nil { + t.Fatal(err) + } + + lc := &fakeLocalClient{ + status: &ipnstate.Status{ + CurrentTailnet: &ipnstate.TailnetStatus{ + MagicDNSSuffix: "ts.net", + }, + }, + } + + ingPGR := &IngressPGReconciler{ + Client: fc, + tsClient: ft, + tsnetServer: fakeTsnetServer, + defaultTags: []string{"tag:k8s"}, + tsNamespace: "operator-ns", + logger: zl.Sugar(), + recorder: record.NewFakeRecorder(10), + lc: lc, + } + + return ingPGR, fc, ft +}