From 2182270a2cb2861f5358bb41b81caee49bb7d0dd Mon Sep 17 00:00:00 2001 From: Trea Hauet Date: Thu, 16 Mar 2023 11:42:16 -0600 Subject: reverseproxy: Reset Content-Length to prevent FastCGI from hanging (#5435) Fixes: https://github.com/caddyserver/caddy/issues/5420 --- modules/caddyhttp/reverseproxy/reverseproxy.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'modules/caddyhttp/reverseproxy/reverseproxy.go') diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go index 1449785..8fd24fe 100644 --- a/modules/caddyhttp/reverseproxy/reverseproxy.go +++ b/modules/caddyhttp/reverseproxy/reverseproxy.go @@ -646,7 +646,8 @@ func (h Handler) prepareRequest(req *http.Request, repl *caddy.Replacer) (*http. // feature if absolutely required, if read timeouts are // set, and if body size is limited if h.RequestBuffers != 0 && req.Body != nil { - req.Body, _ = h.bufferedBody(req.Body, h.RequestBuffers) + req.Body, req.ContentLength = h.bufferedBody(req.Body, h.RequestBuffers) + req.Header.Set("Content-Length", strconv.FormatInt(req.ContentLength, 10)) } if req.ContentLength == 0 { -- cgit v1.2.3 From b6fe5d4b41d07e70a502ed58d40e8b0e75067db5 Mon Sep 17 00:00:00 2001 From: Corin Langosch Date: Fri, 31 Mar 2023 23:44:53 +0200 Subject: proxyprotocol: Add PROXY protocol support to `reverse_proxy`, add HTTP listener wrapper (#5424) Co-authored-by: WeidiDeng Co-authored-by: Francis Lavoie --- modules/caddyhttp/reverseproxy/reverseproxy.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) (limited to 'modules/caddyhttp/reverseproxy/reverseproxy.go') diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go index 8fd24fe..ff22d49 100644 --- a/modules/caddyhttp/reverseproxy/reverseproxy.go +++ b/modules/caddyhttp/reverseproxy/reverseproxy.go @@ -688,8 +688,18 @@ func (h Handler) prepareRequest(req *http.Request, repl *caddy.Replacer) (*http. req.Header.Set("Upgrade", reqUpType) } + // Set up the PROXY protocol info + address := caddyhttp.GetVar(req.Context(), caddyhttp.ClientIPVarKey).(string) + addrPort, err := netip.ParseAddrPort(address) + if err != nil { + // OK; probably didn't have a port + addrPort, _ = netip.ParseAddrPort(address + ":0") + } + proxyProtocolInfo := ProxyProtocolInfo{AddrPort: addrPort} + caddyhttp.SetVar(req.Context(), proxyProtocolInfoVarKey, proxyProtocolInfo) + // Add the supported X-Forwarded-* headers - err := h.addForwardedHeaders(req) + err = h.addForwardedHeaders(req) if err != nil { return nil, err } -- cgit v1.2.3 From 4636109ce17e6ba5f46e73b7b1f3ae82d076a625 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Mon, 10 Apr 2023 16:08:40 -0400 Subject: reverseproxy: Remove deprecated `lookup_srv` (#5396) --- modules/caddyhttp/reverseproxy/reverseproxy.go | 14 -------------- 1 file changed, 14 deletions(-) (limited to 'modules/caddyhttp/reverseproxy/reverseproxy.go') diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go index ff22d49..367b8a2 100644 --- a/modules/caddyhttp/reverseproxy/reverseproxy.go +++ b/modules/caddyhttp/reverseproxy/reverseproxy.go @@ -243,20 +243,6 @@ func (h *Handler) Provision(ctx caddy.Context) error { h.logger.Warn("UNLIMITED BUFFERING: buffering is enabled without any cap on buffer size, which can result in OOM crashes") } - // verify SRV compatibility - TODO: LookupSRV deprecated; will be removed - for i, v := range h.Upstreams { - if v.LookupSRV == "" { - continue - } - h.logger.Warn("DEPRECATED: lookup_srv: will be removed in a near-future version of Caddy; use the http.reverse_proxy.upstreams.srv module instead") - if h.HealthChecks != nil && h.HealthChecks.Active != nil { - return fmt.Errorf(`upstream: lookup_srv is incompatible with active health checks: %d: {"dial": %q, "lookup_srv": %q}`, i, v.Dial, v.LookupSRV) - } - if v.Dial != "" { - return fmt.Errorf(`upstream: specifying dial address is incompatible with lookup_srv: %d: {"dial": %q, "lookup_srv": %q}`, i, v.Dial, v.LookupSRV) - } - } - // start by loading modules if h.TransportRaw != nil { mod, err := ctx.LoadModule(h, "TransportRaw") -- cgit v1.2.3 From 335cd2e8a4f2a91cb2c55a6c2e624a4c4ccddb0c Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Fri, 5 May 2023 17:19:22 -0400 Subject: reverseproxy: Fix active health check header canonicalization, refactor (#5446) --- modules/caddyhttp/reverseproxy/reverseproxy.go | 54 +++----------------------- 1 file changed, 6 insertions(+), 48 deletions(-) (limited to 'modules/caddyhttp/reverseproxy/reverseproxy.go') 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() } } -- cgit v1.2.3 From 2ddb7171440c1045dececb9d7102a8bc28d8708d Mon Sep 17 00:00:00 2001 From: Corin Langosch Date: Mon, 12 Jun 2023 17:35:22 +0200 Subject: reverseproxy: Fix parsing of source IP in case it's an ipv6 address (#5569) --- modules/caddyhttp/reverseproxy/reverseproxy.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'modules/caddyhttp/reverseproxy/reverseproxy.go') diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go index d89d0ac..839c0cc 100644 --- a/modules/caddyhttp/reverseproxy/reverseproxy.go +++ b/modules/caddyhttp/reverseproxy/reverseproxy.go @@ -637,7 +637,13 @@ func (h Handler) prepareRequest(req *http.Request, repl *caddy.Replacer) (*http. addrPort, err := netip.ParseAddrPort(address) if err != nil { // OK; probably didn't have a port - addrPort, _ = netip.ParseAddrPort(address + ":0") + addr, err := netip.ParseAddr(address) + if err != nil { + // Doesn't seem like a valid ip address at all + } else { + // Ok, only the port was missing + addrPort = netip.AddrPortFrom(addr, 0) + } } proxyProtocolInfo := ProxyProtocolInfo{AddrPort: addrPort} caddyhttp.SetVar(req.Context(), proxyProtocolInfoVarKey, proxyProtocolInfo) -- cgit v1.2.3 From 424ae0f420f478e1b38189fd6632d29e13df7eee Mon Sep 17 00:00:00 2001 From: mmm444 Date: Mon, 19 Jun 2023 23:54:43 +0200 Subject: reverseproxy: Experimental streaming timeouts (#5567) * reverseproxy: WIP streaming timeouts * More verbose logging by using the child logger * reverseproxy: Implement streaming timeouts * reverseproxy: Refactor cleanup * reverseproxy: Avoid **time.Timer --------- Co-authored-by: Francis Lavoie --- modules/caddyhttp/reverseproxy/reverseproxy.go | 44 ++++++++++++-------------- 1 file changed, 20 insertions(+), 24 deletions(-) (limited to 'modules/caddyhttp/reverseproxy/reverseproxy.go') diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go index 839c0cc..2fd0aae 100644 --- a/modules/caddyhttp/reverseproxy/reverseproxy.go +++ b/modules/caddyhttp/reverseproxy/reverseproxy.go @@ -157,6 +157,19 @@ type Handler struct { // could be useful if the backend has tighter memory constraints. ResponseBuffers int64 `json:"response_buffers,omitempty"` + // If nonzero, streaming requests such as WebSockets will be + // forcibly closed at the end of the timeout. Default: no timeout. + StreamTimeout caddy.Duration `json:"stream_timeout,omitempty"` + + // If nonzero, streaming requests such as WebSockets will not be + // closed when the proxy config is unloaded, and instead the stream + // will remain open until the delay is complete. In other words, + // enabling this prevents streams from closing when Caddy's config + // is reloaded. Enabling this may be a good idea to avoid a thundering + // herd of reconnecting clients which had their connections closed + // by the previous config closing. Default: no delay. + StreamCloseDelay caddy.Duration `json:"stream_close_delay,omitempty"` + // If configured, rewrites the copy of the upstream request. // Allows changing the request method and URI (path and query). // Since the rewrite is applied to the copy, it does not persist @@ -198,8 +211,9 @@ type Handler struct { handleResponseSegments []*caddyfile.Dispenser // Stores upgraded requests (hijacked connections) for proper cleanup - connections map[io.ReadWriteCloser]openConnection - connectionsMu *sync.Mutex + connections map[io.ReadWriteCloser]openConnection + connectionsCloseTimer *time.Timer + connectionsMu *sync.Mutex ctx caddy.Context logger *zap.Logger @@ -382,25 +396,7 @@ func (h *Handler) Provision(ctx caddy.Context) error { // Cleanup cleans up the resources made by h. func (h *Handler) Cleanup() error { - // close hijacked connections (both to client and backend) - var err error - h.connectionsMu.Lock() - for _, oc := range h.connections { - if oc.gracefulClose != nil { - // this is potentially blocking while we have the lock on the connections - // map, but that should be OK since the server has in theory shut down - // and we are no longer using the connections map - gracefulErr := oc.gracefulClose() - if gracefulErr != nil && err == nil { - err = gracefulErr - } - } - closeErr := oc.conn.Close() - if closeErr != nil && err == nil { - err = closeErr - } - } - h.connectionsMu.Unlock() + err := h.cleanupConnections() // remove hosts from our config from the pool for _, upstream := range h.Upstreams { @@ -872,7 +868,7 @@ func (h *Handler) reverseProxy(rw http.ResponseWriter, req *http.Request, origRe repl.Set("http.reverse_proxy.status_code", res.StatusCode) repl.Set("http.reverse_proxy.status_text", res.Status) - h.logger.Debug("handling response", zap.Int("handler", i)) + logger.Debug("handling response", zap.Int("handler", i)) // we make some data available via request context to child routes // so that they may inherit some options and functions from the @@ -917,7 +913,7 @@ func (h *Handler) reverseProxy(rw http.ResponseWriter, req *http.Request, origRe } // finalizeResponse prepares and copies the response. -func (h Handler) finalizeResponse( +func (h *Handler) finalizeResponse( rw http.ResponseWriter, req *http.Request, res *http.Response, @@ -967,7 +963,7 @@ func (h Handler) finalizeResponse( // there's nothing an error handler can do to recover at this point; // the standard lib's proxy panics at this point, but we'll just log // the error and abort the stream here - h.logger.Error("aborting with incomplete response", zap.Error(err)) + logger.Error("aborting with incomplete response", zap.Error(err)) return nil } -- cgit v1.2.3 From f45a6de20dd19e82e58c85b37e03957b2203b544 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Fri, 21 Jul 2023 21:00:48 -0700 Subject: go.mod: Update quic-go to v0.37.0, bump to Go 1.20 minimum (#5644) * update quic-go to v0.37.0 * Bump to Go 1.20 * Bump golangci-lint version, yml syntax consistency * Use skip-pkg-cache workaround * Workaround needed for both? * Seeding weakrand is no longer necessary --------- Co-authored-by: Matt Holt Co-authored-by: Francis Lavoie --- modules/caddyhttp/reverseproxy/reverseproxy.go | 39 ++++++++++---------------- 1 file changed, 15 insertions(+), 24 deletions(-) (limited to 'modules/caddyhttp/reverseproxy/reverseproxy.go') diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go index 2fd0aae..d1c9352 100644 --- a/modules/caddyhttp/reverseproxy/reverseproxy.go +++ b/modules/caddyhttp/reverseproxy/reverseproxy.go @@ -27,7 +27,6 @@ import ( "net/netip" "net/textproto" "net/url" - "runtime" "strconv" "strings" "sync" @@ -43,13 +42,7 @@ import ( "golang.org/x/net/http/httpguts" ) -var supports1xx bool - func init() { - // Caddy requires at least Go 1.18, but Early Hints requires Go 1.19; thus we can simply check for 1.18 in version string - // TODO: remove this once our minimum Go version is 1.19 - supports1xx = !strings.Contains(runtime.Version(), "go1.18") - caddy.RegisterModule(Handler{}) } @@ -752,25 +745,23 @@ func (h *Handler) reverseProxy(rw http.ResponseWriter, req *http.Request, origRe server := req.Context().Value(caddyhttp.ServerCtxKey).(*caddyhttp.Server) shouldLogCredentials := server.Logs != nil && server.Logs.ShouldLogCredentials - if supports1xx { - // Forward 1xx status codes, backported from https://github.com/golang/go/pull/53164 - trace := &httptrace.ClientTrace{ - Got1xxResponse: func(code int, header textproto.MIMEHeader) error { - h := rw.Header() - copyHeader(h, http.Header(header)) - rw.WriteHeader(code) - - // Clear headers coming from the backend - // (it's not automatically done by ResponseWriter.WriteHeader() for 1xx responses) - for k := range header { - delete(h, k) - } + // Forward 1xx status codes, backported from https://github.com/golang/go/pull/53164 + trace := &httptrace.ClientTrace{ + Got1xxResponse: func(code int, header textproto.MIMEHeader) error { + h := rw.Header() + copyHeader(h, http.Header(header)) + rw.WriteHeader(code) + + // Clear headers coming from the backend + // (it's not automatically done by ResponseWriter.WriteHeader() for 1xx responses) + for k := range header { + delete(h, k) + } - return nil - }, - } - req = req.WithContext(httptrace.WithClientTrace(req.Context(), trace)) + return nil + }, } + req = req.WithContext(httptrace.WithClientTrace(req.Context(), trace)) // if FlushInterval is explicitly configured to -1 (i.e. flush continuously to achieve // low-latency streaming), don't let the transport cancel the request if the client -- cgit v1.2.3 From da23501457e90024814fe0149aaa57dd4384227a Mon Sep 17 00:00:00 2001 From: mmm444 Date: Tue, 1 Aug 2023 16:01:12 +0200 Subject: reverseproxy: Connection termination cleanup (#5663) --- modules/caddyhttp/reverseproxy/reverseproxy.go | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) (limited to 'modules/caddyhttp/reverseproxy/reverseproxy.go') diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go index d1c9352..b331c6b 100644 --- a/modules/caddyhttp/reverseproxy/reverseproxy.go +++ b/modules/caddyhttp/reverseproxy/reverseproxy.go @@ -769,7 +769,7 @@ func (h *Handler) reverseProxy(rw http.ResponseWriter, req *http.Request, origRe // regardless, and we should expect client disconnection in low-latency streaming // scenarios (see issue #4922) if h.FlushInterval == -1 { - req = req.WithContext(ignoreClientGoneContext{req.Context(), h.ctx.Done()}) + req = req.WithContext(ignoreClientGoneContext{req.Context()}) } // do the round-trip; emit debug log with values we know are @@ -1398,15 +1398,28 @@ type handleResponseContext struct { // ignoreClientGoneContext is a special context.Context type // intended for use when doing a RoundTrip where you don't // want a client disconnection to cancel the request during -// the roundtrip. Set its done field to a Done() channel -// of a context that doesn't get canceled when the client -// disconnects, such as caddy.Context.Done() instead. +// the roundtrip. +// This context clears cancellation, error, and deadline methods, +// but still allows values to pass through from its embedded +// context. +// +// TODO: This can be replaced with context.WithoutCancel once +// the minimum required version of Go is 1.21. type ignoreClientGoneContext struct { context.Context - done <-chan struct{} } -func (c ignoreClientGoneContext) Done() <-chan struct{} { return c.done } +func (c ignoreClientGoneContext) Deadline() (deadline time.Time, ok bool) { + return +} + +func (c ignoreClientGoneContext) Done() <-chan struct{} { + return nil +} + +func (c ignoreClientGoneContext) Err() error { + return nil +} // proxyHandleResponseContextCtxKey is the context key for the active proxy handler // so that handle_response routes can inherit some config options -- cgit v1.2.3 From cd486c25d168caf58f4b6fe5d3252df9432901ec Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Wed, 2 Aug 2023 16:03:26 -0400 Subject: caddyhttp: Make use of `http.ResponseController` (#5654) * caddyhttp: Make use of http.ResponseController Also syncs the reverseproxy implementation with stdlib's which now uses ResponseController as well https://github.com/golang/go/commit/2449bbb5e614954ce9e99c8a481ea2ee73d72d61 * Enable full-duplex for HTTP/1.1 * Appease linter * Add warning for builds with Go 1.20, so it's less surprising to users * Improved godoc for EnableFullDuplex, copied text from stdlib * Only wrap in encode if not already wrapped --- modules/caddyhttp/reverseproxy/reverseproxy.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'modules/caddyhttp/reverseproxy/reverseproxy.go') diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go index b331c6b..842d75d 100644 --- a/modules/caddyhttp/reverseproxy/reverseproxy.go +++ b/modules/caddyhttp/reverseproxy/reverseproxy.go @@ -962,9 +962,8 @@ func (h *Handler) finalizeResponse( // Force chunking if we saw a response trailer. // This prevents net/http from calculating the length for short // bodies and adding a Content-Length. - if fl, ok := rw.(http.Flusher); ok { - fl.Flush() - } + //nolint:bodyclose + http.NewResponseController(rw).Flush() } // total duration spent proxying, including writing response body -- cgit v1.2.3 From b32f265ecad60404c3818cc9d42e367a8e4eb7d4 Mon Sep 17 00:00:00 2001 From: Jacob Gadikian Date: Tue, 8 Aug 2023 03:40:31 +0800 Subject: ci: Use gofumpt to format code (#5707) --- modules/caddyhttp/reverseproxy/reverseproxy.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'modules/caddyhttp/reverseproxy/reverseproxy.go') diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go index 842d75d..b65dce7 100644 --- a/modules/caddyhttp/reverseproxy/reverseproxy.go +++ b/modules/caddyhttp/reverseproxy/reverseproxy.go @@ -450,7 +450,8 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request, next caddyht // It returns true when the loop is done and should break; false otherwise. The error value returned should // be assigned to the proxyErr value for the next iteration of the loop (or the error handled after break). func (h *Handler) proxyLoopIteration(r *http.Request, origReq *http.Request, w http.ResponseWriter, proxyErr error, start time.Time, retries int, - repl *caddy.Replacer, reqHeader http.Header, reqHost string, next caddyhttp.Handler) (bool, error) { + repl *caddy.Replacer, reqHeader http.Header, reqHost string, next caddyhttp.Handler, +) (bool, error) { // get the updated list of upstreams upstreams := h.Upstreams if h.DynamicUpstreams != nil { -- cgit v1.2.3 From d6f86cccf5fa5b4eb30141da390cf2439746c5da Mon Sep 17 00:00:00 2001 From: Jacob Gadikian Date: Mon, 14 Aug 2023 23:41:15 +0800 Subject: ci: use gci linter (#5708) * use gofmput to format code * use gci to format imports * reconfigure gci * linter autofixes * rearrange imports a little * export GOOS=windows golangci-lint run ./... --fix --- modules/caddyhttp/reverseproxy/reverseproxy.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'modules/caddyhttp/reverseproxy/reverseproxy.go') diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go index b65dce7..249326d 100644 --- a/modules/caddyhttp/reverseproxy/reverseproxy.go +++ b/modules/caddyhttp/reverseproxy/reverseproxy.go @@ -32,14 +32,15 @@ import ( "sync" "time" + "go.uber.org/zap" + "golang.org/x/net/http/httpguts" + "github.com/caddyserver/caddy/v2" "github.com/caddyserver/caddy/v2/caddyconfig/caddyfile" "github.com/caddyserver/caddy/v2/modules/caddyevents" "github.com/caddyserver/caddy/v2/modules/caddyhttp" "github.com/caddyserver/caddy/v2/modules/caddyhttp/headers" "github.com/caddyserver/caddy/v2/modules/caddyhttp/rewrite" - "go.uber.org/zap" - "golang.org/x/net/http/httpguts" ) func init() { -- cgit v1.2.3 From 936ee918ee00bcdf05d2f44a7fa292c054d64700 Mon Sep 17 00:00:00 2001 From: Matt Holt Date: Thu, 17 Aug 2023 11:33:40 -0600 Subject: reverseproxy: Always return new upstreams (fix #5736) (#5752) * reverseproxy: Always return new upstreams (fix #5736) * Fix healthcheck logger race --- modules/caddyhttp/reverseproxy/reverseproxy.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'modules/caddyhttp/reverseproxy/reverseproxy.go') diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go index 249326d..f11c8e3 100644 --- a/modules/caddyhttp/reverseproxy/reverseproxy.go +++ b/modules/caddyhttp/reverseproxy/reverseproxy.go @@ -356,6 +356,7 @@ func (h *Handler) Provision(ctx caddy.Context) error { if h.HealthChecks != nil { // set defaults on passive health checks, if necessary if h.HealthChecks.Passive != nil { + h.HealthChecks.Passive.logger = h.logger.Named("health_checker.passive") if h.HealthChecks.Passive.FailDuration > 0 && h.HealthChecks.Passive.MaxFails == 0 { h.HealthChecks.Passive.MaxFails = 1 } @@ -1077,12 +1078,11 @@ func (h Handler) provisionUpstream(upstream *Upstream) { // without MaxRequests), copy the value into this upstream, since the // value in the upstream (MaxRequests) is what is used during // availability checks - if h.HealthChecks != nil && h.HealthChecks.Passive != nil { - h.HealthChecks.Passive.logger = h.logger.Named("health_checker.passive") - if h.HealthChecks.Passive.UnhealthyRequestCount > 0 && - upstream.MaxRequests == 0 { - upstream.MaxRequests = h.HealthChecks.Passive.UnhealthyRequestCount - } + if h.HealthChecks != nil && + h.HealthChecks.Passive != nil && + h.HealthChecks.Passive.UnhealthyRequestCount > 0 && + upstream.MaxRequests == 0 { + upstream.MaxRequests = h.HealthChecks.Passive.UnhealthyRequestCount } // upstreams need independent access to the passive -- cgit v1.2.3 From 4feac4d83cd758c95194090d4f3468373ee342ef Mon Sep 17 00:00:00 2001 From: Patrick Koenig Date: Fri, 6 Oct 2023 12:15:26 +0900 Subject: reverseproxy: Allow fallthrough for response handlers without routes (#5780) --- modules/caddyhttp/reverseproxy/reverseproxy.go | 6 ------ 1 file changed, 6 deletions(-) (limited to 'modules/caddyhttp/reverseproxy/reverseproxy.go') diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go index f11c8e3..395530d 100644 --- a/modules/caddyhttp/reverseproxy/reverseproxy.go +++ b/modules/caddyhttp/reverseproxy/reverseproxy.go @@ -848,12 +848,6 @@ func (h *Handler) reverseProxy(rw http.ResponseWriter, req *http.Request, origRe break } - // otherwise, if there are any routes configured, execute those as the - // actual response instead of what we got from the proxy backend - if len(rh.Routes) == 0 { - continue - } - // set up the replacer so that parts of the original response can be // used for routing decisions for field, value := range res.Header { -- cgit v1.2.3 From 2a6859a5e46227473220c653668fa9da9f00b14e Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Tue, 10 Oct 2023 18:07:20 -0400 Subject: reverseproxy: Fix retries on "upstreams unavailable" error (#5841) --- modules/caddyhttp/reverseproxy/reverseproxy.go | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) (limited to 'modules/caddyhttp/reverseproxy/reverseproxy.go') diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go index 395530d..64a7d7a 100644 --- a/modules/caddyhttp/reverseproxy/reverseproxy.go +++ b/modules/caddyhttp/reverseproxy/reverseproxy.go @@ -357,7 +357,7 @@ func (h *Handler) Provision(ctx caddy.Context) error { // set defaults on passive health checks, if necessary if h.HealthChecks.Passive != nil { h.HealthChecks.Passive.logger = h.logger.Named("health_checker.passive") - if h.HealthChecks.Passive.FailDuration > 0 && h.HealthChecks.Passive.MaxFails == 0 { + if h.HealthChecks.Passive.MaxFails == 0 { h.HealthChecks.Passive.MaxFails = 1 } } @@ -480,7 +480,7 @@ func (h *Handler) proxyLoopIteration(r *http.Request, origReq *http.Request, w h upstream := h.LoadBalancing.SelectionPolicy.Select(upstreams, r, w) if upstream == nil { if proxyErr == nil { - proxyErr = caddyhttp.Error(http.StatusServiceUnavailable, fmt.Errorf("no upstreams available")) + proxyErr = caddyhttp.Error(http.StatusServiceUnavailable, noUpstreamsAvailable) } if !h.LoadBalancing.tryAgain(h.ctx, start, retries, proxyErr, r) { return true, proxyErr @@ -1010,17 +1010,23 @@ func (lb LoadBalancing) tryAgain(ctx caddy.Context, start time.Time, retries int // should be safe to retry, since without a connection, no // HTTP request can be transmitted; but if the error is not // specifically a dialer error, we need to be careful - if _, ok := proxyErr.(DialError); proxyErr != nil && !ok { + if proxyErr != nil { + _, isDialError := proxyErr.(DialError) + herr, isHandlerError := proxyErr.(caddyhttp.HandlerError) + // if the error occurred after a connection was established, // we have to assume the upstream received the request, and // retries need to be carefully decided, because some requests // are not idempotent - if lb.RetryMatch == nil && req.Method != "GET" { - // by default, don't retry requests if they aren't GET - return false - } - if !lb.RetryMatch.AnyMatch(req) { - return false + if !isDialError && !(isHandlerError && errors.Is(herr, noUpstreamsAvailable)) { + if lb.RetryMatch == nil && req.Method != "GET" { + // by default, don't retry requests if they aren't GET + return false + } + + if !lb.RetryMatch.AnyMatch(req) { + return false + } } } @@ -1421,6 +1427,8 @@ func (c ignoreClientGoneContext) Err() error { // from the proxy handler. const proxyHandleResponseContextCtxKey caddy.CtxKey = "reverse_proxy_handle_response_context" +var noUpstreamsAvailable = fmt.Errorf("no upstreams available") + // Interface guards var ( _ caddy.Provisioner = (*Handler)(nil) -- cgit v1.2.3 From 3a3182fba3eb20c45dce3efd9eb2856288a5ae04 Mon Sep 17 00:00:00 2001 From: Matt Holt Date: Wed, 11 Oct 2023 13:36:20 -0600 Subject: reverseproxy: Add more debug logs (#5793) * reverseproxy: Add more debug logs This makes debug logging very noisy when reverse proxying, but I guess that's the point. This has shown to be useful in troubleshooting infrastructure issues. * Update modules/caddyhttp/reverseproxy/streaming.go Co-authored-by: Francis Lavoie * Update modules/caddyhttp/reverseproxy/streaming.go Co-authored-by: Francis Lavoie * Add opt-in `trace_logs` option * Rename to VerboseLogs --------- Co-authored-by: Francis Lavoie --- modules/caddyhttp/reverseproxy/reverseproxy.go | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) (limited to 'modules/caddyhttp/reverseproxy/reverseproxy.go') diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go index 64a7d7a..08be40d 100644 --- a/modules/caddyhttp/reverseproxy/reverseproxy.go +++ b/modules/caddyhttp/reverseproxy/reverseproxy.go @@ -191,6 +191,13 @@ type Handler struct { // - `{http.reverse_proxy.header.*}` The headers from the response HandleResponse []caddyhttp.ResponseHandler `json:"handle_response,omitempty"` + // If set, the proxy will write very detailed logs about its + // inner workings. Enable this only when debugging, as it + // will produce a lot of output. + // + // EXPERIMENTAL: This feature is subject to change or removal. + VerboseLogs bool `json:"verbose_logs,omitempty"` + Transport http.RoundTripper `json:"-"` CB CircuitBreaker `json:"-"` DynamicUpstreams UpstreamSource `json:"-"` @@ -943,9 +950,15 @@ func (h *Handler) finalizeResponse( } rw.WriteHeader(res.StatusCode) + if h.VerboseLogs { + logger.Debug("wrote header") + } - err := h.copyResponse(rw, res.Body, h.flushInterval(req, res)) - res.Body.Close() // close now, instead of defer, to populate res.Trailer + err := h.copyResponse(rw, res.Body, h.flushInterval(req, res), logger) + errClose := res.Body.Close() // close now, instead of defer, to populate res.Trailer + if h.VerboseLogs || errClose != nil { + logger.Debug("closed response body from upstream", zap.Error(errClose)) + } if err != nil { // we're streaming the response and we've already written headers, so // there's nothing an error handler can do to recover at this point; @@ -979,6 +992,10 @@ func (h *Handler) finalizeResponse( } } + if h.VerboseLogs { + logger.Debug("response finalized") + } + return nil } -- cgit v1.2.3