reverseproxy: Fix active health check header canonicalization, refactor (#5446)

2 files changed, 77 insertions, 50 deletions

diff --git a/modules/caddyhttp/reverseproxy/healthchecks.go b/modules/caddyhttp/reverseproxy/healthchecks.go
index cfc7bdf..c969c8c 100644
--- a/modules/caddyhttp/reverseproxy/healthchecks.go
+++ b/modules/caddyhttp/reverseproxy/healthchecks.go
@@ -24,7 +24,6 @@ import (
 	"regexp"
 	"runtime/debug"
 	"strconv"
-	"strings"
 	"time"
 
 	"github.com/caddyserver/caddy/v2"
@@ -106,6 +105,76 @@ type ActiveHealthChecks struct {
 	logger     *zap.Logger
 }
 
+// Provision ensures that a is set up properly before use.
+func (a *ActiveHealthChecks) Provision(ctx caddy.Context, h *Handler) error {
+	if !a.IsEnabled() {
+		return nil
+	}
+
+	// Canonicalize the header keys ahead of time, since
+	// JSON unmarshaled headers may be incorrect
+	cleaned := http.Header{}
+	for key, hdrs := range a.Headers {
+		for _, val := range hdrs {
+			cleaned.Add(key, val)
+		}
+	}
+	a.Headers = cleaned
+
+	h.HealthChecks.Active.logger = h.logger.Named("health_checker.active")
+
+	timeout := time.Duration(a.Timeout)
+	if timeout == 0 {
+		timeout = 5 * time.Second
+	}
+
+	if a.Path != "" {
+		a.logger.Warn("the 'path' option is deprecated, please use 'uri' instead!")
+	}
+
+	// parse the URI string (supports path and query)
+	if a.URI != "" {
+		parsedURI, err := url.Parse(a.URI)
+		if err != nil {
+			return err
+		}
+		a.uri = parsedURI
+	}
+
+	a.httpClient = &http.Client{
+		Timeout:   timeout,
+		Transport: h.Transport,
+	}
+
+	for _, upstream := range h.Upstreams {
+		// if there's an alternative port for health-check provided in the config,
+		// then use it, otherwise use the port of upstream.
+		if a.Port != 0 {
+			upstream.activeHealthCheckPort = a.Port
+		}
+	}
+
+	if a.Interval == 0 {
+		a.Interval = caddy.Duration(30 * time.Second)
+	}
+
+	if a.ExpectBody != "" {
+		var err error
+		a.bodyRegexp, err = regexp.Compile(a.ExpectBody)
+		if err != nil {
+			return fmt.Errorf("expect_body: compiling regular expression: %v", err)
+		}
+	}
+
+	return nil
+}
+
+// IsEnabled checks if the active health checks have
+// the minimum config necessary to be enabled.
+func (a *ActiveHealthChecks) IsEnabled() bool {
+	return a.Path != "" || a.URI != "" || a.Port != 0
+}
+
 // PassiveHealthChecks holds configuration related to passive
 // health checks (that is, health checks which occur during
 // the normal flow of request proxying).
@@ -280,7 +349,7 @@ func (h *Handler) doActiveHealthCheck(dialInfo DialInfo, hostAddr string, upstre
 	ctx = context.WithValue(ctx, caddyhttp.OriginalRequestCtxKey, *req)
 	req = req.WithContext(ctx)
 	for key, hdrs := range h.HealthChecks.Active.Headers {
-		if strings.ToLower(key) == "host" {
+		if key == "Host" {
 			req.Host = h.HealthChecks.Active.Headers.Get(key)
 		} else {
 			req.Header[key] = hdrs
diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go
index 367b8a2..d89d0ac 100644
--- a/modules/caddyhttp/reverseproxy/reverseproxy.go
+++ b/modules/caddyhttp/reverseproxy/reverseproxy.go
@@ -27,7 +27,6 @@ import (
 	"net/netip"
 	"net/textproto"
 	"net/url"
-	"regexp"
 	"runtime"
 	"strconv"
 	"strings"
@@ -355,56 +354,15 @@ func (h *Handler) Provision(ctx caddy.Context) error {
 		}
 
 		// if active health checks are enabled, configure them and start a worker
-		if h.HealthChecks.Active != nil && (h.HealthChecks.Active.Path != "" ||
-			h.HealthChecks.Active.URI != "" ||
-			h.HealthChecks.Active.Port != 0) {
-
-			h.HealthChecks.Active.logger = h.logger.Named("health_checker.active")
-
-			timeout := time.Duration(h.HealthChecks.Active.Timeout)
-			if timeout == 0 {
-				timeout = 5 * time.Second
-			}
-
-			if h.HealthChecks.Active.Path != "" {
-				h.HealthChecks.Active.logger.Warn("the 'path' option is deprecated, please use 'uri' instead!")
-			}
-
-			// parse the URI string (supports path and query)
-			if h.HealthChecks.Active.URI != "" {
-				parsedURI, err := url.Parse(h.HealthChecks.Active.URI)
-				if err != nil {
-					return err
-				}
-				h.HealthChecks.Active.uri = parsedURI
-			}
-
-			h.HealthChecks.Active.httpClient = &http.Client{
-				Timeout:   timeout,
-				Transport: h.Transport,
-			}
-
-			for _, upstream := range h.Upstreams {
-				// if there's an alternative port for health-check provided in the config,
-				// then use it, otherwise use the port of upstream.
-				if h.HealthChecks.Active.Port != 0 {
-					upstream.activeHealthCheckPort = h.HealthChecks.Active.Port
-				}
-			}
-
-			if h.HealthChecks.Active.Interval == 0 {
-				h.HealthChecks.Active.Interval = caddy.Duration(30 * time.Second)
+		if h.HealthChecks.Active != nil {
+			err := h.HealthChecks.Active.Provision(ctx, h)
+			if err != nil {
+				return err
 			}
 
-			if h.HealthChecks.Active.ExpectBody != "" {
-				var err error
-				h.HealthChecks.Active.bodyRegexp, err = regexp.Compile(h.HealthChecks.Active.ExpectBody)
-				if err != nil {
-					return fmt.Errorf("expect_body: compiling regular expression: %v", err)
-				}
+			if h.HealthChecks.Active.IsEnabled() {
+				go h.activeHealthChecker()
 			}
-
-			go h.activeHealthChecker()
 		}
 	}