fix: handle health checks in admin not proxy handler (#1448)

A fix in 0.12.0 made empty hosts wildcard matchers which triggered a bug
forwarding health check probes to user applications.
The smart health check detection logic in the proxy handler was replaced
by having the admin handler respond to health check requests.
In theory there is a behavioral change as only headers were used before
to detect health check probes, we now rely on the configured /healthz
and /readyz endpoints.

Signed-off-by: Vincent Link <[email protected]>

Author: linkvt

CHANGELOG.md

@@ -37,7 +37,7 @@ This changelog keeps track of work items that have been completed and are ready
 
 ### Fixes
 
-- **General**: TODO ([#TODO](https://github.com/kedacore/http-add-on/issues/TODO))
+- **Interceptor**: Fix health probes failing when HTTPScaledObject has no hosts configured ([#1447](https://github.com/kedacore/http-add-on/issues/1447))
 
 ### Deprecations
 

config/interceptor/deployment.yaml

@@ -64,11 +64,11 @@ spec:
         livenessProbe:
           httpGet:
             path: /livez
-            port: proxy
+            port: admin
         readinessProbe:
           httpGet:
             path: /readyz
-            port: proxy
+            port: admin
         # TODO(pedrotorres): set better default values avoiding overcommitment
         resources:
           requests:

interceptor/admin.go

@@ -0,0 +1,25 @@
+package main
+
+import (
+	"net/http"
+
+	"github.com/go-logr/logr"
+
+	"github.com/kedacore/http-add-on/pkg/queue"
+)
+
+// BuildAdminHandler creates the handler for the admin endpoint.
+func BuildAdminHandler(logger logr.Logger, counter queue.Counter, probeHandler http.Handler) http.Handler {
+	mux := http.NewServeMux()
+
+	mux.Handle("/readyz", probeHandler)
+	mux.Handle("/livez", probeHandler)
+
+	queue.AddCountsRoute(
+		logger,
+		mux,
+		counter,
+	)
+
+	return mux
+}

interceptor/admin_test.go

@@ -0,0 +1,56 @@
+package main
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/go-logr/logr"
+)
+
+func TestAdminHandler(t *testing.T) {
+	tests := map[string]struct {
+		path            string
+		wantProbeCalled bool
+		wantStatus      int
+	}{
+		"livez": {
+			path:            "/livez",
+			wantProbeCalled: true,
+			wantStatus:      http.StatusOK,
+		},
+		"readyz": {
+			path:            "/readyz",
+			wantProbeCalled: true,
+			wantStatus:      http.StatusOK,
+		},
+		"other": {
+			path:            "/other",
+			wantProbeCalled: false,
+			wantStatus:      http.StatusNotFound,
+		},
+	}
+
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			probeCalled := false
+			probeHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				probeCalled = true
+				w.WriteHeader(http.StatusOK)
+			})
+
+			handler := BuildAdminHandler(logr.Discard(), nil, probeHandler)
+
+			req := httptest.NewRequest(http.MethodGet, tt.path, nil)
+			rec := httptest.NewRecorder()
+			handler.ServeHTTP(rec, req)
+
+			if probeCalled != tt.wantProbeCalled {
+				t.Error("expected probe to be called")
+			}
+			if rec.Code != tt.wantStatus {
+				t.Errorf("got status = %d, want %d", rec.Code, tt.wantStatus)
+			}
+		})
+	}
+}

interceptor/handler/probe.go

@@ -14,7 +14,7 @@ type Probe struct {
 	healthy        atomic.Bool
 }
 
-func NewProbe(healthChecks []util.HealthChecker) *Probe {
+func NewProbe(healthChecks ...util.HealthChecker) *Probe {
 	return &Probe{
 		healthCheckers: healthChecks,
 	}

interceptor/handler/probe_test.go

@@ -24,13 +24,11 @@ var _ = Describe("ProbeHandler", func() {
 			)
 
 			var b bool
-			healthCheckers := []util.HealthChecker{
-				util.HealthCheckerFunc(func(_ context.Context) error {
-					b = true
+			healthCheckers := util.HealthCheckerFunc(func(_ context.Context) error {
+				b = true
 
-					return ret
-				}),
-			}
+				return ret
+			})
 
 			ph := NewProbe(healthCheckers)
 			Expect(ph).NotTo(BeNil())

interceptor/main.go

@@ -189,7 +189,7 @@ func main() {
 	eg.Go(func() error {
 		setupLog.Info("starting the admin server", "port", adminPort)
 
-		if err := runAdminServer(ctx, ctrl.Log, queues, adminPort); !util.IsIgnoredErr(err) {
+		if err := runAdminServer(ctx, ctrl.Log, adminPort, queues, routingTable); !util.IsIgnoredErr(err) {
 			setupLog.Error(err, "admin server failed")
 			return err
 		}
@@ -271,20 +271,20 @@ func main() {
 func runAdminServer(
 	ctx context.Context,
 	lggr logr.Logger,
-	q queue.Counter,
 	port int,
+	q queue.Counter,
+	routingTable routing.Table,
 ) error {
 	lggr = lggr.WithName("runAdminServer")
-	adminServer := http.NewServeMux()
-	queue.AddCountsRoute(
-		lggr,
-		adminServer,
-		q,
-	)
+
+	probeHandler := handler.NewProbe(routingTable)
+	go probeHandler.Start(ctx)
+
+	adminHandler := BuildAdminHandler(lggr, q, probeHandler)
 
 	addr := fmt.Sprintf("0.0.0.0:%d", port)
 	lggr.Info("admin server starting", "address", addr)
-	return kedahttp.ServeContext(ctx, addr, adminServer, nil)
+	return kedahttp.ServeContext(ctx, addr, adminHandler, nil)
 }
 
 func runMetricsServer(
@@ -310,18 +310,12 @@ func runProxyServer(
 	tlsCfg *tls.Config,
 	tracingConfig config.Tracing,
 ) error {
-	probeHandler := handler.NewProbe([]util.HealthChecker{
-		routingTable,
-	})
-	go probeHandler.Start(ctx)
-
 	// Build handler chain using the shared builder
 	rootHandler := BuildProxyHandler(&ProxyHandlerConfig{
 		Logger:       logger,
 		Queue:        q,
 		WaitFunc:     waitFunc,
 		RoutingTable: routingTable,
-		ProbeHandler: probeHandler,
 		Reader:       reader,
 		Timeouts:     timeouts,
 		Serving:      serving,

interceptor/middleware/routing.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"net/http"
 	"net/url"
-	"regexp"
 
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -17,24 +16,16 @@ import (
 	"github.com/kedacore/http-add-on/pkg/util"
 )
 
-var (
-	kubernetesProbeUserAgent = regexp.MustCompile(`(^|\s)kube-probe/`)
-	googleHCUserAgent        = regexp.MustCompile(`(^|\s)GoogleHC/`)
-	awsELBserAgent           = regexp.MustCompile(`(^|\s)ELB-HealthChecker/`)
-)
-
 type Routing struct {
 	routingTable    routing.Table
-	probeHandler    http.Handler
 	upstreamHandler http.Handler
 	reader          client.Reader
 	tlsEnabled      bool
 }
 
-func NewRouting(routingTable routing.Table, probeHandler http.Handler, upstreamHandler http.Handler, reader client.Reader, tlsEnabled bool) *Routing {
+func NewRouting(routingTable routing.Table, upstreamHandler http.Handler, reader client.Reader, tlsEnabled bool) *Routing {
 	return &Routing{
 		routingTable:    routingTable,
-		probeHandler:    probeHandler,
 		upstreamHandler: upstreamHandler,
 		reader:          reader,
 		tlsEnabled:      tlsEnabled,
@@ -46,11 +37,6 @@ var _ http.Handler = (*Routing)(nil)
 func (rm *Routing) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	httpso := rm.routingTable.Route(r)
 	if httpso == nil {
-		if rm.isProbe(r) {
-			rm.probeHandler.ServeHTTP(w, r)
-			return
-		}
-
 		sh := handler.NewStatic(http.StatusNotFound, nil)
 		sh.ServeHTTP(w, r)
 
@@ -130,9 +116,3 @@ func (rm *Routing) streamFromHTTPSO(ctx context.Context, httpso *httpv1alpha1.HT
 		Host:   fmt.Sprintf("%s.%s:%d", reference.GetServiceName(), httpso.GetNamespace(), port),
 	}, nil
 }
-
-func (rm *Routing) isProbe(r *http.Request) bool {
-	ua := r.UserAgent()
-
-	return kubernetesProbeUserAgent.Match([]byte(ua)) || googleHCUserAgent.Match([]byte(ua)) || awsELBserAgent.Match([]byte(ua))
-}