cmd/k8s-operator: default ingress paths to '/' if not specified by user (#15706)

in resource

Fixes #14908

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

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

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