reverseproxy: Implement health_uri, deprecate health_path, supports query (#4050)

* reverseproxy: Implement health_uri, replaces health_path, supports query

Also fixes a bug with `health_status` Caddyfile parsing , it would always only take the first character of the status code even if it didn't end with "xx".

* reverseproxy: Rename to URI, named logger, warn in Provision (for JSON)

caddytest/integration/caddyfile_adapt/reverse_proxy_health_path_query.txt

@@ -0,0 +1,75 @@
+# Health with query in the uri
+:8443 {
+	reverse_proxy localhost:54321 {
+		health_uri /health?ready=1
+		health_status 2xx
+	}
+}
+
+# Health without query in the uri
+:8444 {
+	reverse_proxy localhost:54321 {
+		health_uri /health
+		health_status 200
+	}
+}
+
+----------
+{
+	"apps": {
+		"http": {
+			"servers": {
+				"srv0": {
+					"listen": [
+						":8443"
+					],
+					"routes": [
+						{
+							"handle": [
+								{
+									"handler": "reverse_proxy",
+									"health_checks": {
+										"active": {
+											"expect_status": 2,
+											"uri": "/health?ready=1"
+										}
+									},
+									"upstreams": [
+										{
+											"dial": "localhost:54321"
+										}
+									]
+								}
+							]
+						}
+					]
+				},
+				"srv1": {
+					"listen": [
+						":8444"
+					],
+					"routes": [
+						{
+							"handle": [
+								{
+									"handler": "reverse_proxy",
+									"health_checks": {
+										"active": {
+											"expect_status": 200,
+											"uri": "/health"
+										}
+									},
+									"upstreams": [
+										{
+											"dial": "localhost:54321"
+										}
+									]
+								}
+							]
+						}
+					]
+				}
+			}
+		}
+	}
+}

modules/caddyhttp/reverseproxy/caddyfile.go

@@ -288,6 +288,18 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 				}
 				h.LoadBalancing.TryInterval = caddy.Duration(dur)
 
+			case "health_uri":
+				if !d.NextArg() {
+					return d.ArgErr()
+				}
+				if h.HealthChecks == nil {
+					h.HealthChecks = new(HealthChecks)
+				}
+				if h.HealthChecks.Active == nil {
+					h.HealthChecks.Active = new(ActiveHealthChecks)
+				}
+				h.HealthChecks.Active.URI = d.Val()
+
 			case "health_path":
 				if !d.NextArg() {
 					return d.ArgErr()
@@ -299,6 +311,7 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 					h.HealthChecks.Active = new(ActiveHealthChecks)
 				}
 				h.HealthChecks.Active.Path = d.Val()
+				caddy.Log().Named("config.adapter.caddyfile").Warn("the 'health_path' subdirective is deprecated, please use 'health_uri' instead!")
 
 			case "health_port":
 				if !d.NextArg() {
@@ -382,7 +395,7 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 				if len(val) == 3 && strings.HasSuffix(val, "xx") {
 					val = val[:1]
 				}
-				statusNum, err := strconv.Atoi(val[:1])
+				statusNum, err := strconv.Atoi(val)
 				if err != nil {
 					return d.Errf("bad status value '%s': %v", d.Val(), err)
 				}
@@ -463,7 +476,7 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 					if len(arg) == 3 && strings.HasSuffix(arg, "xx") {
 						arg = arg[:1]
 					}
-					statusNum, err := strconv.Atoi(arg[:1])
+					statusNum, err := strconv.Atoi(arg)
 					if err != nil {
 						return d.Errf("bad status value '%s': %v", d.Val(), err)
 					}

modules/caddyhttp/reverseproxy/healthchecks.go

@@ -51,9 +51,13 @@ type HealthChecks struct {
 // health checks (that is, health checks which occur in a
 // background goroutine independently).
 type ActiveHealthChecks struct {
-	// The URI path to use for health checks.
+	// The path to use for health checks.
+	// DEPRECATED: Use 'uri' instead.
 	Path string `json:"path,omitempty"`
 
+	// The URI (path and query) to use for health checks
+	URI string `json:"uri,omitempty"`
+
 	// The port to use (if different from the upstream's dial
 	// address) for health checks.
 	Port int `json:"port,omitempty"`
@@ -79,6 +83,7 @@ type ActiveHealthChecks struct {
 	// body of a healthy backend.
 	ExpectBody string `json:"expect_body,omitempty"`
 
+	uri        *url.URL
 	httpClient *http.Client
 	bodyRegexp *regexp.Regexp
 	logger     *zap.Logger
@@ -218,7 +223,15 @@ func (h *Handler) doActiveHealthCheck(dialInfo DialInfo, hostAddr string, host H
 	u := &url.URL{
 		Scheme: scheme,
 		Host:   hostAddr,
-		Path:   h.HealthChecks.Active.Path,
+	}
+
+	// if we have a provisioned uri, use that, otherwise use
+	// the deprecated Path option
+	if h.HealthChecks.Active.uri != nil {
+		u.Path = h.HealthChecks.Active.uri.Path
+		u.RawQuery = h.HealthChecks.Active.uri.RawQuery
+	} else {
+		u.Path = h.HealthChecks.Active.Path
 	}
 
 	// adjust the port, if configured to be different

modules/caddyhttp/reverseproxy/reverseproxy.go

@@ -23,6 +23,7 @@ import (
 	"io"
 	"net"
 	"net/http"
+	"net/url"
 	"regexp"
 	"strconv"
 	"strings"
@@ -273,8 +274,10 @@ 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.Port != 0) {
+		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)
@@ -282,6 +285,19 @@ func (h *Handler) Provision(ctx caddy.Context) error {
 				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,