From 75f797debdd6c4294497edba9889c6251a8542e7 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Mon, 29 Mar 2021 20:36:40 -0400 Subject: 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) --- .../reverse_proxy_health_path_query.txt | 75 ++++++++++++++++++++++ modules/caddyhttp/reverseproxy/caddyfile.go | 17 ++++- modules/caddyhttp/reverseproxy/healthchecks.go | 17 ++++- modules/caddyhttp/reverseproxy/reverseproxy.go | 20 +++++- 4 files changed, 123 insertions(+), 6 deletions(-) create mode 100644 caddytest/integration/caddyfile_adapt/reverse_proxy_health_path_query.txt diff --git a/caddytest/integration/caddyfile_adapt/reverse_proxy_health_path_query.txt b/caddytest/integration/caddyfile_adapt/reverse_proxy_health_path_query.txt new file mode 100644 index 0000000..80ac2de --- /dev/null +++ b/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" + } + ] + } + ] + } + ] + } + } + } + } +} diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go index da7450d..dbadef6 100644 --- a/modules/caddyhttp/reverseproxy/caddyfile.go +++ b/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) } diff --git a/modules/caddyhttp/reverseproxy/healthchecks.go b/modules/caddyhttp/reverseproxy/healthchecks.go index 42003b6..6f65866 100644 --- a/modules/caddyhttp/reverseproxy/healthchecks.go +++ b/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 diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go index 9da509f..b552d96 100644 --- a/modules/caddyhttp/reverseproxy/reverseproxy.go +++ b/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, -- cgit v1.2.3