From 9666c2e7002160cba936b181fb4ee7f5590f6cce Mon Sep 17 00:00:00 2001 From: Tom Meadows Date: Thu, 17 Apr 2025 16:14:34 +0100 Subject: [PATCH] cmd/k8s-operator: default ingress paths to '/' if not specified by user (#15706) in resource Fixes #14908 Signed-off-by: chaosinthecrd --- cmd/k8s-operator/ingress.go | 6 ++ cmd/k8s-operator/ingress_test.go | 180 +++++++++++++++++++++++++++++-- 2 files changed, 178 insertions(+), 8 deletions(-) diff --git a/cmd/k8s-operator/ingress.go b/cmd/k8s-operator/ingress.go index 8c19a5e05..6c50e10b2 100644 --- a/cmd/k8s-operator/ingress.go +++ b/cmd/k8s-operator/ingress.go @@ -292,9 +292,15 @@ func validateIngressClass(ctx context.Context, cl client.Client) error { func handlersForIngress(ctx context.Context, ing *networkingv1.Ingress, cl client.Client, rec record.EventRecorder, tlsHost string, logger *zap.SugaredLogger) (handlers map[string]*ipn.HTTPHandler, err error) { addIngressBackend := func(b *networkingv1.IngressBackend, path string) { + if path == "" { + path = "/" + rec.Eventf(ing, corev1.EventTypeNormal, "PathUndefined", "configured backend is missing a path, defaulting to '/'") + } + if b == nil { return } + if b.Service == nil { rec.Eventf(ing, corev1.EventTypeWarning, "InvalidIngressBackend", "backend for path %q is missing service", path) return diff --git a/cmd/k8s-operator/ingress_test.go b/cmd/k8s-operator/ingress_test.go index f9623850c..a975fec7a 100644 --- a/cmd/k8s-operator/ingress_test.go +++ b/cmd/k8s-operator/ingress_test.go @@ -16,6 +16,7 @@ import ( apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/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/ipn" @@ -487,6 +488,138 @@ func TestIngressLetsEncryptStaging(t *testing.T) { } } +func TestEmptyPath(t *testing.T) { + testCases := []struct { + name string + paths []networkingv1.HTTPIngressPath + expectedEvents []string + }{ + { + name: "empty_path_with_prefix_type", + paths: []networkingv1.HTTPIngressPath{ + { + PathType: ptrPathType(networkingv1.PathTypePrefix), + Path: "", + Backend: *backend(), + }, + }, + expectedEvents: []string{ + "Normal PathUndefined configured backend is missing a path, defaulting to '/'", + }, + }, + { + name: "empty_path_with_implementation_specific_type", + paths: []networkingv1.HTTPIngressPath{ + { + PathType: ptrPathType(networkingv1.PathTypeImplementationSpecific), + Path: "", + Backend: *backend(), + }, + }, + expectedEvents: []string{ + "Normal PathUndefined configured backend is missing a path, defaulting to '/'", + }, + }, + { + name: "empty_path_with_exact_type", + paths: []networkingv1.HTTPIngressPath{ + { + PathType: ptrPathType(networkingv1.PathTypeExact), + Path: "", + Backend: *backend(), + }, + }, + expectedEvents: []string{ + "Warning UnsupportedPathTypeExact Exact path type strict matching is currently not supported and requests will be routed as for Prefix path type. This behaviour might change in the future.", + "Normal PathUndefined configured backend is missing a path, defaulting to '/'", + }, + }, + { + name: "two_competing_but_not_identical_paths_including_one_empty", + paths: []networkingv1.HTTPIngressPath{ + { + PathType: ptrPathType(networkingv1.PathTypeImplementationSpecific), + Path: "", + Backend: *backend(), + }, + { + PathType: ptrPathType(networkingv1.PathTypeImplementationSpecific), + Path: "/", + Backend: *backend(), + }, + }, + expectedEvents: []string{ + "Normal PathUndefined configured backend is missing a path, defaulting to '/'", + }, + }, + } + + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + fc := fake.NewFakeClient(ingressClass()) + ft := &fakeTSClient{} + fr := record.NewFakeRecorder(3) // bump this if you expect a test case to throw more events + fakeTsnetServer := &fakeTSNetServer{certDomains: []string{"foo.com"}} + zl, err := zap.NewDevelopment() + if err != nil { + t.Fatal(err) + } + ingR := &IngressReconciler{ + recorder: fr, + Client: fc, + ssr: &tailscaleSTSReconciler{ + Client: fc, + tsClient: ft, + tsnetServer: fakeTsnetServer, + defaultTags: []string{"tag:k8s"}, + operatorNamespace: "operator-ns", + proxyImage: "tailscale/tailscale", + }, + logger: zl.Sugar(), + } + + // 1. Resources get created for regular Ingress + mustCreate(t, fc, ingressWithPaths(tt.paths)) + mustCreate(t, fc, service()) + + expectReconciled(t, ingR, "default", "test") + + fullName, shortName := findGenName(t, fc, "default", "test", "ingress") + mustCreate(t, fc, &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: fullName, + Namespace: "operator-ns", + UID: "test-uid", + }, + }) + opts := configOpts{ + stsName: shortName, + secretName: fullName, + namespace: "default", + parentType: "ingress", + hostname: "foo", + app: kubetypes.AppIngressResource, + } + serveConfig := &ipn.ServeConfig{ + TCP: map[uint16]*ipn.TCPPortHandler{443: {HTTPS: true}}, + Web: map[ipn.HostPort]*ipn.WebServerConfig{"${TS_CERT_DOMAIN}:443": {Handlers: map[string]*ipn.HTTPHandler{"/": {Proxy: "http://1.2.3.4:8080/"}}}}, + } + opts.serveConfig = serveConfig + + expectEqual(t, fc, expectedSecret(t, fc, opts)) + expectEqual(t, fc, expectedHeadlessService(shortName, "ingress")) + expectEqual(t, fc, expectedSTSUserspace(t, fc, opts), removeHashAnnotation, removeResourceReqs) + + expectEvents(t, fr, tt.expectedEvents) + }) + } +} + +// ptrPathType is a helper function to return a pointer to the pathtype string (required for TestEmptyPath) +func ptrPathType(p networkingv1.PathType) *networkingv1.PathType { + return &p +} + func ingressClass() *networkingv1.IngressClass { return &networkingv1.IngressClass{ ObjectMeta: metav1.ObjectMeta{Name: "tailscale"}, @@ -520,17 +653,48 @@ func ingress() *networkingv1.Ingress { }, Spec: networkingv1.IngressSpec{ IngressClassName: ptr.To("tailscale"), - DefaultBackend: &networkingv1.IngressBackend{ - Service: &networkingv1.IngressServiceBackend{ - Name: "test", - Port: networkingv1.ServiceBackendPort{ - Number: 8080, - }, - }, - }, + DefaultBackend: backend(), TLS: []networkingv1.IngressTLS{ {Hosts: []string{"default-test"}}, }, }, } } + +func ingressWithPaths(paths []networkingv1.HTTPIngressPath) *networkingv1.Ingress { + return &networkingv1.Ingress{ + TypeMeta: metav1.TypeMeta{Kind: "Ingress", APIVersion: "networking.k8s.io/v1"}, + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + UID: types.UID("1234-UID"), + }, + Spec: networkingv1.IngressSpec{ + IngressClassName: ptr.To("tailscale"), + Rules: []networkingv1.IngressRule{ + { + Host: "foo.tailnetxyz.ts.net", + IngressRuleValue: networkingv1.IngressRuleValue{ + HTTP: &networkingv1.HTTPIngressRuleValue{ + Paths: paths, + }, + }, + }, + }, + TLS: []networkingv1.IngressTLS{ + {Hosts: []string{"foo.tailnetxyz.ts.net"}}, + }, + }, + } +} + +func backend() *networkingv1.IngressBackend { + return &networkingv1.IngressBackend{ + Service: &networkingv1.IngressServiceBackend{ + Name: "test", + Port: networkingv1.ServiceBackendPort{ + Number: 8080, + }, + }, + } +}