From 5ded580444e9258cb35a9c94192d3c1d63e7b74f Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Thu, 16 Feb 2023 11:14:07 -0500 Subject: cmd: Adjust documentation for commands (#5377) --- modules/caddyhttp/reverseproxy/command.go | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/command.go b/modules/caddyhttp/reverseproxy/command.go index 44f4c22..04fb9b4 100644 --- a/modules/caddyhttp/reverseproxy/command.go +++ b/modules/caddyhttp/reverseproxy/command.go @@ -35,7 +35,7 @@ func init() { caddycmd.RegisterCommand(caddycmd.Command{ Name: "reverse-proxy", Func: cmdReverseProxy, - Usage: "[--from ] [--to ] [--change-host-header]", + Usage: "[--from ] [--to ] [--change-host-header] [--insecure] [--internal-certs] [--disable-redirects]", Short: "A quick and production-ready reverse proxy", Long: ` A simple but production-ready reverse proxy. Useful for quick deployments, @@ -52,16 +52,23 @@ If the --from address has a host or IP, Caddy will attempt to serve the proxy over HTTPS with a certificate (unless overridden by the HTTP scheme or port). -If --change-host-header is set, the Host header on the request will be modified -from its original incoming value to the address of the upstream. (Otherwise, by -default, all incoming headers are passed through unmodified.) +If serving HTTPS: + --disable-redirects can be used to avoid binding to the HTTP port. + --internal-certs can be used to force issuance certs using the internal + CA instead of attempting to issue a public certificate. + +For proxying: + --change-host-header sets the Host header on the request to the address + of the upstream, instead of defaulting to the incoming Host header. + --insecure disables TLS verification with the upstream. WARNING: THIS + DISABLES SECURITY BY NOT VERIFYING THE UPSTREAM'S CERTIFICATE. `, Flags: func() *flag.FlagSet { fs := flag.NewFlagSet("reverse-proxy", flag.ExitOnError) fs.String("from", "localhost", "Address on which to receive traffic") fs.Var(&reverseProxyCmdTo, "to", "Upstream address(es) to which traffic should be sent") fs.Bool("change-host-header", false, "Set upstream Host header to address of upstream") - fs.Bool("insecure", false, "Disable TLS verification (WARNING: DISABLES SECURITY BY NOT VERIFYING SSL CERTIFICATES!)") + fs.Bool("insecure", false, "Disable TLS verification (WARNING: DISABLES SECURITY BY NOT VERIFYING TLS CERTIFICATES!)") fs.Bool("internal-certs", false, "Use internal CA for issuing certs") fs.Bool("debug", false, "Enable verbose debug logs") fs.Bool("disable-redirects", false, "Disable HTTP->HTTPS redirects") -- cgit v1.2.3 From e3909cc385f68a636cc8675dc12f6cd5185d722d Mon Sep 17 00:00:00 2001 From: Mohammed Al Sahaf Date: Fri, 24 Feb 2023 22:54:04 +0300 Subject: reverseproxy: refactor HTTP transport layer (#5369) Co-authored-by: Francis Lavoie Co-authored-by: Weidi Deng --- modules/caddyhttp/reverseproxy/httptransport.go | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/httptransport.go b/modules/caddyhttp/reverseproxy/httptransport.go index ec5d2f2..71d06d5 100644 --- a/modules/caddyhttp/reverseproxy/httptransport.go +++ b/modules/caddyhttp/reverseproxy/httptransport.go @@ -172,12 +172,19 @@ func (h *HTTPTransport) NewTransport(caddyCtx caddy.Context) (*http.Transport, e } } - // Set up the dialer to pull the correct information from the context dialContext := func(ctx context.Context, network, address string) (net.Conn, error) { - // the proper dialing information should be embedded into the request's context + // For unix socket upstreams, we need to recover the dial info from + // the request's context, because the Host on the request's URL + // will have been modified by directing the request, overwriting + // the unix socket filename. + // Also, we need to avoid overwriting the address at this point + // when not necessary, because http.ProxyFromEnvironment may have + // modified the address according to the user's env proxy config. if dialInfo, ok := GetDialInfo(ctx); ok { - network = dialInfo.Network - address = dialInfo.Address + if strings.HasPrefix(dialInfo.Network, "unix") { + network = dialInfo.Network + address = dialInfo.Address + } } conn, err := dialer.DialContext(ctx, network, address) @@ -188,8 +195,8 @@ func (h *HTTPTransport) NewTransport(caddyCtx caddy.Context) (*http.Transport, e return nil, DialError{err} } - // if read/write timeouts are configured and this is a TCP connection, enforce the timeouts - // by wrapping the connection with our own type + // if read/write timeouts are configured and this is a TCP connection, + // enforce the timeouts by wrapping the connection with our own type if tcpConn, ok := conn.(*net.TCPConn); ok && (h.ReadTimeout > 0 || h.WriteTimeout > 0) { conn = &tcpRWTimeoutConn{ TCPConn: tcpConn, @@ -203,6 +210,7 @@ func (h *HTTPTransport) NewTransport(caddyCtx caddy.Context) (*http.Transport, e } rt := &http.Transport{ + Proxy: http.ProxyFromEnvironment, DialContext: dialContext, MaxConnsPerHost: h.MaxConnsPerHost, ResponseHeaderTimeout: time.Duration(h.ResponseHeaderTimeout), -- cgit v1.2.3 From 9e6919550be5689628d0020ec14e90ea6f527716 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Fri, 24 Feb 2023 18:09:12 -0500 Subject: cmd: Expand cobra support, add short flags (#5379) * cmd: Expand cobra support * Convert commands to cobra, add short flags * Fix version command typo Co-authored-by: Emily Lange * Apply suggestions from code review Co-authored-by: Matt Holt --------- Co-authored-by: Emily Lange Co-authored-by: Matt Holt --- modules/caddyhttp/reverseproxy/command.go | 46 +++++++++++++++++-------------- 1 file changed, 25 insertions(+), 21 deletions(-) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/command.go b/modules/caddyhttp/reverseproxy/command.go index 04fb9b4..8c171ec 100644 --- a/modules/caddyhttp/reverseproxy/command.go +++ b/modules/caddyhttp/reverseproxy/command.go @@ -16,7 +16,6 @@ package reverseproxy import ( "encoding/json" - "flag" "fmt" "net/http" "strconv" @@ -28,14 +27,14 @@ import ( "github.com/caddyserver/caddy/v2/modules/caddyhttp" "github.com/caddyserver/caddy/v2/modules/caddyhttp/headers" "github.com/caddyserver/caddy/v2/modules/caddytls" + "github.com/spf13/cobra" "go.uber.org/zap" ) func init() { caddycmd.RegisterCommand(caddycmd.Command{ Name: "reverse-proxy", - Func: cmdReverseProxy, - Usage: "[--from ] [--to ] [--change-host-header] [--insecure] [--internal-certs] [--disable-redirects]", + Usage: "[--from ] [--to ] [--change-host-header] [--insecure] [--internal-certs] [--disable-redirects] [--access-log]", Short: "A quick and production-ready reverse proxy", Long: ` A simple but production-ready reverse proxy. Useful for quick deployments, @@ -63,17 +62,17 @@ For proxying: --insecure disables TLS verification with the upstream. WARNING: THIS DISABLES SECURITY BY NOT VERIFYING THE UPSTREAM'S CERTIFICATE. `, - Flags: func() *flag.FlagSet { - fs := flag.NewFlagSet("reverse-proxy", flag.ExitOnError) - fs.String("from", "localhost", "Address on which to receive traffic") - fs.Var(&reverseProxyCmdTo, "to", "Upstream address(es) to which traffic should be sent") - fs.Bool("change-host-header", false, "Set upstream Host header to address of upstream") - fs.Bool("insecure", false, "Disable TLS verification (WARNING: DISABLES SECURITY BY NOT VERIFYING TLS CERTIFICATES!)") - fs.Bool("internal-certs", false, "Use internal CA for issuing certs") - fs.Bool("debug", false, "Enable verbose debug logs") - fs.Bool("disable-redirects", false, "Disable HTTP->HTTPS redirects") - return fs - }(), + CobraFunc: func(cmd *cobra.Command) { + cmd.Flags().StringP("from", "f", "localhost", "Address on which to receive traffic") + cmd.Flags().StringSliceP("to", "t", []string{}, "Upstream address(es) to which traffic should be sent") + cmd.Flags().BoolP("change-host-header", "c", false, "Set upstream Host header to address of upstream") + cmd.Flags().BoolP("insecure", "", false, "Disable TLS verification (WARNING: DISABLES SECURITY BY NOT VERIFYING TLS CERTIFICATES!)") + cmd.Flags().BoolP("disable-redirects", "r", false, "Disable HTTP->HTTPS redirects") + cmd.Flags().BoolP("internal-certs", "i", false, "Use internal CA for issuing certs") + cmd.Flags().BoolP("access-log", "", false, "Enable the access log") + cmd.Flags().BoolP("debug", "v", false, "Enable verbose debug logs") + cmd.RunE = caddycmd.WrapCommandFuncForCobra(cmdReverseProxy) + }, }) } @@ -83,14 +82,19 @@ func cmdReverseProxy(fs caddycmd.Flags) (int, error) { from := fs.String("from") changeHost := fs.Bool("change-host-header") insecure := fs.Bool("insecure") + disableRedir := fs.Bool("disable-redirects") internalCerts := fs.Bool("internal-certs") + accessLog := fs.Bool("access-log") debug := fs.Bool("debug") - disableRedir := fs.Bool("disable-redirects") httpPort := strconv.Itoa(caddyhttp.DefaultHTTPPort) httpsPort := strconv.Itoa(caddyhttp.DefaultHTTPSPort) - if len(reverseProxyCmdTo) == 0 { + to, err := fs.GetStringSlice("to") + if err != nil { + return caddy.ExitCodeFailedStartup, fmt.Errorf("invalid to flag: %v", err) + } + if len(to) == 0 { return caddy.ExitCodeFailedStartup, fmt.Errorf("--to is required") } @@ -119,9 +123,9 @@ func cmdReverseProxy(fs caddycmd.Flags) (int, error) { // set up the upstream address; assume missing information from given parts // mixing schemes isn't supported, so use first defined (if available) - toAddresses := make([]string, len(reverseProxyCmdTo)) + toAddresses := make([]string, len(to)) var toScheme string - for i, toLoc := range reverseProxyCmdTo { + for i, toLoc := range to { addr, scheme, err := parseUpstreamDialAddress(toLoc) if err != nil { return caddy.ExitCodeFailedStartup, fmt.Errorf("invalid upstream address %s: %v", toLoc, err) @@ -180,6 +184,9 @@ func cmdReverseProxy(fs caddycmd.Flags) (int, error) { Routes: caddyhttp.RouteList{route}, Listen: []string{":" + fromAddr.Port}, } + if accessLog { + server.Logs = &caddyhttp.ServerLogConfig{} + } if fromAddr.Scheme == "http" { server.AutoHTTPS = &caddyhttp.AutoHTTPSConfig{Disabled: true} @@ -238,6 +245,3 @@ func cmdReverseProxy(fs caddycmd.Flags) (int, error) { select {} } - -// reverseProxyCmdTo holds the parsed values from repeated use of the --to flag. -var reverseProxyCmdTo caddycmd.StringSlice -- cgit v1.2.3 From 960150bb034dc9a549ee7289b1a4eb4abafeb30a Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Sat, 25 Feb 2023 19:34:27 -0500 Subject: caddyfile: Implement heredoc support (#5385) --- .../caddyhttp/reverseproxy/fastcgi/caddyfile.go | 42 ++++++---------------- .../reverseproxy/forwardauth/caddyfile.go | 13 +++---- 2 files changed, 15 insertions(+), 40 deletions(-) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go b/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go index 799050e..4687e68 100644 --- a/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go @@ -217,25 +217,18 @@ func parsePHPFastCGI(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error return nil, dispenser.ArgErr() } fcgiTransport.Root = dispenser.Val() - dispenser.Delete() - dispenser.Delete() + dispenser.DeleteN(2) case "split": extensions = dispenser.RemainingArgs() - dispenser.Delete() - for range extensions { - dispenser.Delete() - } + dispenser.DeleteN(len(extensions) + 1) if len(extensions) == 0 { return nil, dispenser.ArgErr() } case "env": args := dispenser.RemainingArgs() - dispenser.Delete() - for range args { - dispenser.Delete() - } + dispenser.DeleteN(len(args) + 1) if len(args) != 2 { return nil, dispenser.ArgErr() } @@ -246,10 +239,7 @@ func parsePHPFastCGI(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error case "index": args := dispenser.RemainingArgs() - dispenser.Delete() - for range args { - dispenser.Delete() - } + dispenser.DeleteN(len(args) + 1) if len(args) != 1 { return nil, dispenser.ArgErr() } @@ -257,10 +247,7 @@ func parsePHPFastCGI(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error case "try_files": args := dispenser.RemainingArgs() - dispenser.Delete() - for range args { - dispenser.Delete() - } + dispenser.DeleteN(len(args) + 1) if len(args) < 1 { return nil, dispenser.ArgErr() } @@ -268,10 +255,7 @@ func parsePHPFastCGI(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error case "resolve_root_symlink": args := dispenser.RemainingArgs() - dispenser.Delete() - for range args { - dispenser.Delete() - } + dispenser.DeleteN(len(args) + 1) fcgiTransport.ResolveRootSymlink = true case "dial_timeout": @@ -283,8 +267,7 @@ func parsePHPFastCGI(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error return nil, dispenser.Errf("bad timeout value %s: %v", dispenser.Val(), err) } fcgiTransport.DialTimeout = caddy.Duration(dur) - dispenser.Delete() - dispenser.Delete() + dispenser.DeleteN(2) case "read_timeout": if !dispenser.NextArg() { @@ -295,8 +278,7 @@ func parsePHPFastCGI(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error return nil, dispenser.Errf("bad timeout value %s: %v", dispenser.Val(), err) } fcgiTransport.ReadTimeout = caddy.Duration(dur) - dispenser.Delete() - dispenser.Delete() + dispenser.DeleteN(2) case "write_timeout": if !dispenser.NextArg() { @@ -307,15 +289,11 @@ func parsePHPFastCGI(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error return nil, dispenser.Errf("bad timeout value %s: %v", dispenser.Val(), err) } fcgiTransport.WriteTimeout = caddy.Duration(dur) - dispenser.Delete() - dispenser.Delete() + dispenser.DeleteN(2) case "capture_stderr": args := dispenser.RemainingArgs() - dispenser.Delete() - for range args { - dispenser.Delete() - } + dispenser.DeleteN(len(args) + 1) fcgiTransport.CaptureStderr = true } } diff --git a/modules/caddyhttp/reverseproxy/forwardauth/caddyfile.go b/modules/caddyhttp/reverseproxy/forwardauth/caddyfile.go index cecc000..dbd2a29 100644 --- a/modules/caddyhttp/reverseproxy/forwardauth/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/forwardauth/caddyfile.go @@ -129,8 +129,7 @@ func parseCaddyfile(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) return nil, dispenser.ArgErr() } rpHandler.Rewrite.URI = dispenser.Val() - dispenser.Delete() - dispenser.Delete() + dispenser.DeleteN(2) case "copy_headers": args := dispenser.RemainingArgs() @@ -140,13 +139,11 @@ func parseCaddyfile(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) args = append(args, dispenser.Val()) } - dispenser.Delete() // directive name + // directive name + args + dispenser.DeleteN(len(args) + 1) if hadBlock { - dispenser.Delete() // opening brace - dispenser.Delete() // closing brace - } - for range args { - dispenser.Delete() + // opening & closing brace + dispenser.DeleteN(2) } for _, headerField := range args { -- cgit v1.2.3 From 941eae5f615aeaf038f62002e673a7bf4886f1c7 Mon Sep 17 00:00:00 2001 From: Emily Lange Date: Mon, 27 Feb 2023 18:23:09 +0100 Subject: reverseproxy: allow specifying ip version for dynamic `a` upstream (#5401) Co-authored-by: Francis Lavoie --- modules/caddyhttp/reverseproxy/caddyfile.go | 25 +++++++++++++++++++- modules/caddyhttp/reverseproxy/upstreams.go | 36 +++++++++++++++++++++++++++-- 2 files changed, 58 insertions(+), 3 deletions(-) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go index 1211188..cf84ba7 100644 --- a/modules/caddyhttp/reverseproxy/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/caddyfile.go @@ -1324,6 +1324,7 @@ func (u *SRVUpstreams) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { // resolvers // dial_timeout // dial_fallback_delay +// versions ipv4|ipv6 // } func (u *AUpstreams) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { for d.Next() { @@ -1397,8 +1398,30 @@ func (u *AUpstreams) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { } u.FallbackDelay = caddy.Duration(dur) + case "versions": + args := d.RemainingArgs() + if len(args) == 0 { + return d.Errf("must specify at least one version") + } + + if u.Versions == nil { + u.Versions = &ipVersions{} + } + + trueBool := true + for _, arg := range args { + switch arg { + case "ipv4": + u.Versions.IPv4 = &trueBool + case "ipv6": + u.Versions.IPv6 = &trueBool + default: + return d.Errf("unsupported version: '%s'", arg) + } + } + default: - return d.Errf("unrecognized srv option '%s'", d.Val()) + return d.Errf("unrecognized a option '%s'", d.Val()) } } } diff --git a/modules/caddyhttp/reverseproxy/upstreams.go b/modules/caddyhttp/reverseproxy/upstreams.go index 7a90016..30bd7b5 100644 --- a/modules/caddyhttp/reverseproxy/upstreams.go +++ b/modules/caddyhttp/reverseproxy/upstreams.go @@ -213,6 +213,11 @@ func (sl srvLookup) isFresh() bool { return time.Since(sl.freshness) < time.Duration(sl.srvUpstreams.Refresh) } +type ipVersions struct { + IPv4 *bool `json:"ipv4,omitempty"` + IPv6 *bool `json:"ipv6,omitempty"` +} + // AUpstreams provides upstreams from A/AAAA lookups. // Results are cached and refreshed at the configured // refresh interval. @@ -240,6 +245,11 @@ type AUpstreams struct { // A negative value disables this. FallbackDelay caddy.Duration `json:"dial_fallback_delay,omitempty"` + // The IP versions to resolve for. By default, both + // "ipv4" and "ipv6" will be enabled, which + // correspond to A and AAAA records respectively. + Versions *ipVersions `json:"versions,omitempty"` + resolver *net.Resolver } @@ -286,7 +296,29 @@ func (au *AUpstreams) Provision(_ caddy.Context) error { func (au AUpstreams) GetUpstreams(r *http.Request) ([]*Upstream, error) { repl := r.Context().Value(caddy.ReplacerCtxKey).(*caddy.Replacer) - auStr := repl.ReplaceAll(au.String(), "") + + resolveIpv4 := au.Versions.IPv4 == nil || *au.Versions.IPv4 + resolveIpv6 := au.Versions.IPv6 == nil || *au.Versions.IPv6 + + // Map ipVersion early, so we can use it as part of the cache-key. + // This should be fairly inexpensive and comes and the upside of + // allowing the same dynamic upstream (name + port combination) + // to be used multiple times with different ip versions. + // + // It also forced a cache-miss if a previously cached dynamic + // upstream changes its ip version, e.g. after a config reload, + // while keeping the cache-invalidation as simple as it currently is. + var ipVersion string + switch { + case resolveIpv4 && !resolveIpv6: + ipVersion = "ip4" + case !resolveIpv4 && resolveIpv6: + ipVersion = "ip6" + default: + ipVersion = "ip" + } + + auStr := repl.ReplaceAll(au.String()+ipVersion, "") // first, use a cheap read-lock to return a cached result quickly aAaaaMu.RLock() @@ -311,7 +343,7 @@ func (au AUpstreams) GetUpstreams(r *http.Request) ([]*Upstream, error) { name := repl.ReplaceAll(au.Name, "") port := repl.ReplaceAll(au.Port, "") - ips, err := au.resolver.LookupIPAddr(r.Context(), name) + ips, err := au.resolver.LookupIP(r.Context(), ipVersion, name) if err != nil { return nil, err } -- cgit v1.2.3 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') 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 10b265d25279aa4aabdf3e390f580492cf0077f7 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Mon, 27 Mar 2023 16:35:31 -0400 Subject: reverseproxy: Header up/down support for CLI command (#5460) --- modules/caddyhttp/reverseproxy/command.go | 64 ++++++++++++++++++++++++++++--- 1 file changed, 59 insertions(+), 5 deletions(-) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/command.go b/modules/caddyhttp/reverseproxy/command.go index 8c171ec..02c921c 100644 --- a/modules/caddyhttp/reverseproxy/command.go +++ b/modules/caddyhttp/reverseproxy/command.go @@ -19,6 +19,7 @@ import ( "fmt" "net/http" "strconv" + "strings" "github.com/caddyserver/caddy/v2" "github.com/caddyserver/caddy/v2/caddyconfig" @@ -34,7 +35,7 @@ import ( func init() { caddycmd.RegisterCommand(caddycmd.Command{ Name: "reverse-proxy", - Usage: "[--from ] [--to ] [--change-host-header] [--insecure] [--internal-certs] [--disable-redirects] [--access-log]", + Usage: `[--from ] [--to ] [--change-host-header] [--insecure] [--internal-certs] [--disable-redirects] [--header-up "Field: value"] [--header-down "Field: value"] [--access-log] [--debug]`, Short: "A quick and production-ready reverse proxy", Long: ` A simple but production-ready reverse proxy. Useful for quick deployments, @@ -57,8 +58,11 @@ If serving HTTPS: CA instead of attempting to issue a public certificate. For proxying: + --header-up can be used to set a request header to send to the upstream. + --header-down can be used to set a response header to send back to the client. --change-host-header sets the Host header on the request to the address of the upstream, instead of defaulting to the incoming Host header. + This is a shortcut for --header-up "Host: {http.reverse_proxy.upstream.hostport}". --insecure disables TLS verification with the upstream. WARNING: THIS DISABLES SECURITY BY NOT VERIFYING THE UPSTREAM'S CERTIFICATE. `, @@ -69,6 +73,8 @@ For proxying: cmd.Flags().BoolP("insecure", "", false, "Disable TLS verification (WARNING: DISABLES SECURITY BY NOT VERIFYING TLS CERTIFICATES!)") cmd.Flags().BoolP("disable-redirects", "r", false, "Disable HTTP->HTTPS redirects") cmd.Flags().BoolP("internal-certs", "i", false, "Use internal CA for issuing certs") + cmd.Flags().StringSliceP("header-up", "H", []string{}, "Set a request header to send to the upstream (format: \"Field: value\")") + cmd.Flags().StringSliceP("header-down", "d", []string{}, "Set a response header to send back to the client (format: \"Field: value\")") cmd.Flags().BoolP("access-log", "", false, "Enable the access log") cmd.Flags().BoolP("debug", "v", false, "Enable verbose debug logs") cmd.RunE = caddycmd.WrapCommandFuncForCobra(cmdReverseProxy) @@ -157,16 +163,64 @@ func cmdReverseProxy(fs caddycmd.Flags) (int, error) { Upstreams: upstreamPool, } - if changeHost { + // set up header_up + headerUp, err := fs.GetStringSlice("header-up") + if err != nil { + return caddy.ExitCodeFailedStartup, fmt.Errorf("invalid header flag: %v", err) + } + if len(headerUp) > 0 { + reqHdr := make(http.Header) + for i, h := range headerUp { + key, val, found := strings.Cut(h, ":") + key, val = strings.TrimSpace(key), strings.TrimSpace(val) + if !found || key == "" || val == "" { + return caddy.ExitCodeFailedStartup, fmt.Errorf("header-up %d: invalid format \"%s\" (expecting \"Field: value\")", i, h) + } + reqHdr.Set(key, val) + } handler.Headers = &headers.Handler{ Request: &headers.HeaderOps{ - Set: http.Header{ - "Host": []string{"{http.reverse_proxy.upstream.hostport}"}, - }, + Set: reqHdr, + }, + } + } + + // set up header_down + headerDown, err := fs.GetStringSlice("header-down") + if err != nil { + return caddy.ExitCodeFailedStartup, fmt.Errorf("invalid header flag: %v", err) + } + if len(headerDown) > 0 { + respHdr := make(http.Header) + for i, h := range headerDown { + key, val, found := strings.Cut(h, ":") + key, val = strings.TrimSpace(key), strings.TrimSpace(val) + if !found || key == "" || val == "" { + return caddy.ExitCodeFailedStartup, fmt.Errorf("header-down %d: invalid format \"%s\" (expecting \"Field: value\")", i, h) + } + respHdr.Set(key, val) + } + if handler.Headers == nil { + handler.Headers = &headers.Handler{} + } + handler.Headers.Response = &headers.RespHeaderOps{ + HeaderOps: &headers.HeaderOps{ + Set: respHdr, }, } } + if changeHost { + if handler.Headers == nil { + handler.Headers = &headers.Handler{ + Request: &headers.HeaderOps{ + Set: http.Header{}, + }, + } + } + handler.Headers.Request.Set.Set("Host", "{http.reverse_proxy.upstream.hostport}") + } + route := caddyhttp.Route{ HandlersRaw: []json.RawMessage{ caddyconfig.JSONModuleObject(handler, "handler", "reverse_proxy", nil), -- cgit v1.2.3 From e16a886814d8cd43d545de38a4d6b98313fb31cb Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Mon, 27 Mar 2023 17:16:22 -0400 Subject: caddytls: Eval replacer on automation policy subjects (#5459) Also renamed the field to SubjectsRaw, which can be considered a breaking change but I don't expect this to affect much. --- modules/caddyhttp/reverseproxy/command.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/command.go b/modules/caddyhttp/reverseproxy/command.go index 02c921c..5e8beb1 100644 --- a/modules/caddyhttp/reverseproxy/command.go +++ b/modules/caddyhttp/reverseproxy/command.go @@ -259,8 +259,8 @@ func cmdReverseProxy(fs caddycmd.Flags) (int, error) { tlsApp := caddytls.TLS{ Automation: &caddytls.AutomationConfig{ Policies: []*caddytls.AutomationPolicy{{ - Subjects: []string{fromAddr.Host}, - IssuersRaw: []json.RawMessage{json.RawMessage(`{"module":"internal"}`)}, + SubjectsRaw: []string{fromAddr.Host}, + IssuersRaw: []json.RawMessage{json.RawMessage(`{"module":"internal"}`)}, }}, }, } -- cgit v1.2.3 From 1aef807c71b1ea8e70e664765e0010734aee468c Mon Sep 17 00:00:00 2001 From: Mohammed Al Sahaf Date: Tue, 28 Mar 2023 00:41:24 +0300 Subject: log: Make sink logs encodable (#5441) * log: make `sink` encodable * deduplicate logger fields * extract common fields into `BaseLog` and embed it into `SinkLog` * amend godoc on `BaseLog` and `SinkLog` * minor style change --------- Co-authored-by: Francis Lavoie --- modules/caddyhttp/reverseproxy/command.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/command.go b/modules/caddyhttp/reverseproxy/command.go index 5e8beb1..bd3efcd 100644 --- a/modules/caddyhttp/reverseproxy/command.go +++ b/modules/caddyhttp/reverseproxy/command.go @@ -280,7 +280,7 @@ func cmdReverseProxy(fs caddycmd.Flags) (int, error) { if debug { cfg.Logging = &caddy.Logging{ Logs: map[string]*caddy.CustomLog{ - "default": {Level: zap.DebugLevel.CapitalString()}, + "default": {BaseLog: caddy.BaseLog{Level: zap.DebugLevel.CapitalString()}}, }, } } -- cgit v1.2.3 From 66e571e687eeddca0aafd5df0e3ab5f7cecbdcfa Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Fri, 31 Mar 2023 15:46:29 -0400 Subject: reverseproxy: Add mention of which half a copyBuffer err comes from (#5472) Co-authored-by: Matt Holt --- modules/caddyhttp/reverseproxy/streaming.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/streaming.go b/modules/caddyhttp/reverseproxy/streaming.go index 1db107a..1f5387e 100644 --- a/modules/caddyhttp/reverseproxy/streaming.go +++ b/modules/caddyhttp/reverseproxy/streaming.go @@ -20,6 +20,7 @@ package reverseproxy import ( "context" + "fmt" "io" weakrand "math/rand" "mime" @@ -215,7 +216,7 @@ func (h Handler) copyBuffer(dst io.Writer, src io.Reader, buf []byte) (int64, er written += int64(nw) } if werr != nil { - return written, werr + return written, fmt.Errorf("writing: %w", werr) } if nr != nw { return written, io.ErrShortWrite @@ -223,9 +224,9 @@ func (h Handler) copyBuffer(dst io.Writer, src io.Reader, buf []byte) (int64, er } if rerr != nil { if rerr == io.EOF { - rerr = nil + return written, nil } - return written, rerr + return written, fmt.Errorf("reading: %w", rerr) } } } -- 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/caddyfile.go | 11 +++++ modules/caddyhttp/reverseproxy/hosts.go | 11 +++++ modules/caddyhttp/reverseproxy/httptransport.go | 65 +++++++++++++++++++++++++ modules/caddyhttp/reverseproxy/reverseproxy.go | 12 ++++- 4 files changed, 98 insertions(+), 1 deletion(-) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go index cf84ba7..fab3099 100644 --- a/modules/caddyhttp/reverseproxy/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/caddyfile.go @@ -918,6 +918,17 @@ func (h *HTTPTransport) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { } h.MaxResponseHeaderSize = int64(size) + case "proxy_protocol": + if !d.NextArg() { + return d.ArgErr() + } + switch proxyProtocol := d.Val(); proxyProtocol { + case "v1", "v2": + h.ProxyProtocol = proxyProtocol + default: + return d.Errf("invalid proxy protocol version '%s'", proxyProtocol) + } + case "dial_timeout": if !d.NextArg() { return d.ArgErr() diff --git a/modules/caddyhttp/reverseproxy/hosts.go b/modules/caddyhttp/reverseproxy/hosts.go index a973ecb..b97c8b4 100644 --- a/modules/caddyhttp/reverseproxy/hosts.go +++ b/modules/caddyhttp/reverseproxy/hosts.go @@ -19,6 +19,7 @@ import ( "fmt" "net" "net/http" + "net/netip" "strconv" "sync/atomic" @@ -259,3 +260,13 @@ var hosts = caddy.NewUsagePool() // dialInfoVarKey is the key used for the variable that holds // the dial info for the upstream connection. const dialInfoVarKey = "reverse_proxy.dial_info" + +// proxyProtocolInfoVarKey is the key used for the variable that holds +// the proxy protocol info for the upstream connection. +const proxyProtocolInfoVarKey = "reverse_proxy.proxy_protocol_info" + +// ProxyProtocolInfo contains information needed to write proxy protocol to a +// connection to an upstream host. +type ProxyProtocolInfo struct { + AddrPort netip.AddrPort +} diff --git a/modules/caddyhttp/reverseproxy/httptransport.go b/modules/caddyhttp/reverseproxy/httptransport.go index 71d06d5..1135862 100644 --- a/modules/caddyhttp/reverseproxy/httptransport.go +++ b/modules/caddyhttp/reverseproxy/httptransport.go @@ -29,7 +29,9 @@ import ( "time" "github.com/caddyserver/caddy/v2" + "github.com/caddyserver/caddy/v2/modules/caddyhttp" "github.com/caddyserver/caddy/v2/modules/caddytls" + "github.com/mastercactapus/proxyprotocol" "go.uber.org/zap" "golang.org/x/net/http2" ) @@ -64,6 +66,10 @@ type HTTPTransport struct { // Maximum number of connections per host. Default: 0 (no limit) MaxConnsPerHost int `json:"max_conns_per_host,omitempty"` + // If non-empty, which PROXY protocol version to send when + // connecting to an upstream. Default: off. + ProxyProtocol string `json:"proxy_protocol,omitempty"` + // How long to wait before timing out trying to connect to // an upstream. Default: `3s`. DialTimeout caddy.Duration `json:"dial_timeout,omitempty"` @@ -195,6 +201,57 @@ func (h *HTTPTransport) NewTransport(caddyCtx caddy.Context) (*http.Transport, e return nil, DialError{err} } + if h.ProxyProtocol != "" { + proxyProtocolInfo, ok := caddyhttp.GetVar(ctx, proxyProtocolInfoVarKey).(ProxyProtocolInfo) + if !ok { + return nil, fmt.Errorf("failed to get proxy protocol info from context") + } + + // The src and dst have to be of the some address family. As we don't know the original + // dst address (it's kind of impossible to know) and this address is generelly of very + // little interest, we just set it to all zeros. + var destIP net.IP + switch { + case proxyProtocolInfo.AddrPort.Addr().Is4(): + destIP = net.IPv4zero + case proxyProtocolInfo.AddrPort.Addr().Is6(): + destIP = net.IPv6zero + default: + return nil, fmt.Errorf("unexpected remote addr type in proxy protocol info") + } + + // TODO: We should probably migrate away from net.IP to use netip.Addr, + // but due to the upstream dependency, we can't do that yet. + switch h.ProxyProtocol { + case "v1": + header := proxyprotocol.HeaderV1{ + SrcIP: net.IP(proxyProtocolInfo.AddrPort.Addr().AsSlice()), + SrcPort: int(proxyProtocolInfo.AddrPort.Port()), + DestIP: destIP, + DestPort: 0, + } + caddyCtx.Logger().Debug("sending proxy protocol header v1", zap.Any("header", header)) + _, err = header.WriteTo(conn) + case "v2": + header := proxyprotocol.HeaderV2{ + Command: proxyprotocol.CmdProxy, + Src: &net.TCPAddr{IP: net.IP(proxyProtocolInfo.AddrPort.Addr().AsSlice()), Port: int(proxyProtocolInfo.AddrPort.Port())}, + Dest: &net.TCPAddr{IP: destIP, Port: 0}, + } + caddyCtx.Logger().Debug("sending proxy protocol header v2", zap.Any("header", header)) + _, err = header.WriteTo(conn) + default: + return nil, fmt.Errorf("unexpected proxy protocol version") + } + + if err != nil { + // identify this error as one that occurred during + // dialing, which can be important when trying to + // decide whether to retry a request + return nil, DialError{err} + } + } + // if read/write timeouts are configured and this is a TCP connection, // enforce the timeouts by wrapping the connection with our own type if tcpConn, ok := conn.(*net.TCPConn); ok && (h.ReadTimeout > 0 || h.WriteTimeout > 0) { @@ -239,6 +296,14 @@ func (h *HTTPTransport) NewTransport(caddyCtx caddy.Context) (*http.Transport, e rt.IdleConnTimeout = time.Duration(h.KeepAlive.IdleConnTimeout) } + // The proxy protocol header can only be sent once right after opening the connection. + // So single connection must not be used for multiple requests, which can potentially + // come from different clients. + if !rt.DisableKeepAlives && h.ProxyProtocol != "" { + caddyCtx.Logger().Warn("disabling keepalives, they are incompatible with using PROXY protocol") + rt.DisableKeepAlives = true + } + if h.Compression != nil { rt.DisableCompression = !*h.Compression } 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 f8b59e77f83c05da87bd5e3780fb7522b863d462 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Mon, 3 Apr 2023 23:31:47 -0400 Subject: reverseproxy: Add `query` and `client_ip_hash` lb policies (#5468) --- .../caddyhttp/reverseproxy/selectionpolicies.go | 85 ++++++++ .../reverseproxy/selectionpolicies_test.go | 215 +++++++++++++++++++++ 2 files changed, 300 insertions(+) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/selectionpolicies.go b/modules/caddyhttp/reverseproxy/selectionpolicies.go index 0b7f50c..4184df5 100644 --- a/modules/caddyhttp/reverseproxy/selectionpolicies.go +++ b/modules/caddyhttp/reverseproxy/selectionpolicies.go @@ -24,11 +24,13 @@ import ( "net" "net/http" "strconv" + "strings" "sync/atomic" "time" "github.com/caddyserver/caddy/v2" "github.com/caddyserver/caddy/v2/caddyconfig/caddyfile" + "github.com/caddyserver/caddy/v2/modules/caddyhttp" ) func init() { @@ -38,7 +40,9 @@ func init() { caddy.RegisterModule(RoundRobinSelection{}) caddy.RegisterModule(FirstSelection{}) caddy.RegisterModule(IPHashSelection{}) + caddy.RegisterModule(ClientIPHashSelection{}) caddy.RegisterModule(URIHashSelection{}) + caddy.RegisterModule(QueryHashSelection{}) caddy.RegisterModule(HeaderHashSelection{}) caddy.RegisterModule(CookieHashSelection{}) @@ -303,6 +307,39 @@ func (r *IPHashSelection) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { return nil } +// ClientIPHashSelection is a policy that selects a host +// based on hashing the client IP of the request, as determined +// by the HTTP app's trusted proxies settings. +type ClientIPHashSelection struct{} + +// CaddyModule returns the Caddy module information. +func (ClientIPHashSelection) CaddyModule() caddy.ModuleInfo { + return caddy.ModuleInfo{ + ID: "http.reverse_proxy.selection_policies.client_ip_hash", + New: func() caddy.Module { return new(ClientIPHashSelection) }, + } +} + +// Select returns an available host, if any. +func (ClientIPHashSelection) Select(pool UpstreamPool, req *http.Request, _ http.ResponseWriter) *Upstream { + address := caddyhttp.GetVar(req.Context(), caddyhttp.ClientIPVarKey).(string) + clientIP, _, err := net.SplitHostPort(address) + if err != nil { + clientIP = address // no port + } + return hostByHashing(pool, clientIP) +} + +// UnmarshalCaddyfile sets up the module from Caddyfile tokens. +func (r *ClientIPHashSelection) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { + for d.Next() { + if d.NextArg() { + return d.ArgErr() + } + } + return nil +} + // URIHashSelection is a policy that selects a // host by hashing the request URI. type URIHashSelection struct{} @@ -330,6 +367,52 @@ func (r *URIHashSelection) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { return nil } +// QueryHashSelection is a policy that selects +// a host based on a given request query parameter. +type QueryHashSelection struct { + // The query key whose value is to be hashed and used for upstream selection. + Key string `json:"key,omitempty"` +} + +// CaddyModule returns the Caddy module information. +func (QueryHashSelection) CaddyModule() caddy.ModuleInfo { + return caddy.ModuleInfo{ + ID: "http.reverse_proxy.selection_policies.query", + New: func() caddy.Module { return new(QueryHashSelection) }, + } +} + +// Select returns an available host, if any. +func (s QueryHashSelection) Select(pool UpstreamPool, req *http.Request, _ http.ResponseWriter) *Upstream { + if s.Key == "" { + return nil + } + + // Since the query may have multiple values for the same key, + // we'll join them to avoid a problem where the user can control + // the upstream that the request goes to by sending multiple values + // for the same key, when the upstream only considers the first value. + // Keep in mind that a client changing the order of the values may + // affect which upstream is selected, but this is a semantically + // different request, because the order of the values is significant. + vals := strings.Join(req.URL.Query()[s.Key], ",") + if vals == "" { + return RandomSelection{}.Select(pool, req, nil) + } + return hostByHashing(pool, vals) +} + +// UnmarshalCaddyfile sets up the module from Caddyfile tokens. +func (s *QueryHashSelection) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { + for d.Next() { + if !d.NextArg() { + return d.ArgErr() + } + s.Key = d.Val() + } + return nil +} + // HeaderHashSelection is a policy that selects // a host based on a given request header. type HeaderHashSelection struct { @@ -552,7 +635,9 @@ var ( _ Selector = (*RoundRobinSelection)(nil) _ Selector = (*FirstSelection)(nil) _ Selector = (*IPHashSelection)(nil) + _ Selector = (*ClientIPHashSelection)(nil) _ Selector = (*URIHashSelection)(nil) + _ Selector = (*QueryHashSelection)(nil) _ Selector = (*HeaderHashSelection)(nil) _ Selector = (*CookieHashSelection)(nil) diff --git a/modules/caddyhttp/reverseproxy/selectionpolicies_test.go b/modules/caddyhttp/reverseproxy/selectionpolicies_test.go index 546a60d..d2b7b3d 100644 --- a/modules/caddyhttp/reverseproxy/selectionpolicies_test.go +++ b/modules/caddyhttp/reverseproxy/selectionpolicies_test.go @@ -15,9 +15,12 @@ package reverseproxy import ( + "context" "net/http" "net/http/httptest" "testing" + + "github.com/caddyserver/caddy/v2/modules/caddyhttp" ) func testPool() UpstreamPool { @@ -229,6 +232,149 @@ func TestIPHashPolicy(t *testing.T) { } } +func TestClientIPHashPolicy(t *testing.T) { + pool := testPool() + ipHash := new(ClientIPHashSelection) + req, _ := http.NewRequest("GET", "/", nil) + req = req.WithContext(context.WithValue(req.Context(), caddyhttp.VarsCtxKey, make(map[string]any))) + + // We should be able to predict where every request is routed. + caddyhttp.SetVar(req.Context(), caddyhttp.ClientIPVarKey, "172.0.0.1:80") + h := ipHash.Select(pool, req, nil) + if h != pool[1] { + t.Error("Expected ip hash policy host to be the second host.") + } + caddyhttp.SetVar(req.Context(), caddyhttp.ClientIPVarKey, "172.0.0.2:80") + h = ipHash.Select(pool, req, nil) + if h != pool[1] { + t.Error("Expected ip hash policy host to be the second host.") + } + caddyhttp.SetVar(req.Context(), caddyhttp.ClientIPVarKey, "172.0.0.3:80") + h = ipHash.Select(pool, req, nil) + if h != pool[1] { + t.Error("Expected ip hash policy host to be the second host.") + } + caddyhttp.SetVar(req.Context(), caddyhttp.ClientIPVarKey, "172.0.0.4:80") + h = ipHash.Select(pool, req, nil) + if h != pool[1] { + t.Error("Expected ip hash policy host to be the second host.") + } + + // we should get the same results without a port + caddyhttp.SetVar(req.Context(), caddyhttp.ClientIPVarKey, "172.0.0.1") + h = ipHash.Select(pool, req, nil) + if h != pool[1] { + t.Error("Expected ip hash policy host to be the second host.") + } + caddyhttp.SetVar(req.Context(), caddyhttp.ClientIPVarKey, "172.0.0.2") + h = ipHash.Select(pool, req, nil) + if h != pool[1] { + t.Error("Expected ip hash policy host to be the second host.") + } + caddyhttp.SetVar(req.Context(), caddyhttp.ClientIPVarKey, "172.0.0.3") + h = ipHash.Select(pool, req, nil) + if h != pool[1] { + t.Error("Expected ip hash policy host to be the second host.") + } + caddyhttp.SetVar(req.Context(), caddyhttp.ClientIPVarKey, "172.0.0.4") + h = ipHash.Select(pool, req, nil) + if h != pool[1] { + t.Error("Expected ip hash policy host to be the second host.") + } + + // we should get a healthy host if the original host is unhealthy and a + // healthy host is available + caddyhttp.SetVar(req.Context(), caddyhttp.ClientIPVarKey, "172.0.0.4") + pool[1].setHealthy(false) + h = ipHash.Select(pool, req, nil) + if h != pool[0] { + t.Error("Expected ip hash policy host to be the first host.") + } + + caddyhttp.SetVar(req.Context(), caddyhttp.ClientIPVarKey, "172.0.0.2") + h = ipHash.Select(pool, req, nil) + if h != pool[0] { + t.Error("Expected ip hash policy host to be the first host.") + } + pool[1].setHealthy(true) + + caddyhttp.SetVar(req.Context(), caddyhttp.ClientIPVarKey, "172.0.0.3") + pool[2].setHealthy(false) + h = ipHash.Select(pool, req, nil) + if h != pool[1] { + t.Error("Expected ip hash policy host to be the second host.") + } + caddyhttp.SetVar(req.Context(), caddyhttp.ClientIPVarKey, "172.0.0.4") + h = ipHash.Select(pool, req, nil) + if h != pool[1] { + t.Error("Expected ip hash policy host to be the second host.") + } + + // We should be able to resize the host pool and still be able to predict + // where a req will be routed with the same IP's used above + pool = UpstreamPool{ + {Host: new(Host), Dial: "0.0.0.2"}, + {Host: new(Host), Dial: "0.0.0.3"}, + } + caddyhttp.SetVar(req.Context(), caddyhttp.ClientIPVarKey, "172.0.0.1:80") + h = ipHash.Select(pool, req, nil) + if h != pool[0] { + t.Error("Expected ip hash policy host to be the first host.") + } + caddyhttp.SetVar(req.Context(), caddyhttp.ClientIPVarKey, "172.0.0.2:80") + h = ipHash.Select(pool, req, nil) + if h != pool[0] { + t.Error("Expected ip hash policy host to be the first host.") + } + caddyhttp.SetVar(req.Context(), caddyhttp.ClientIPVarKey, "172.0.0.3:80") + h = ipHash.Select(pool, req, nil) + if h != pool[0] { + t.Error("Expected ip hash policy host to be the first host.") + } + caddyhttp.SetVar(req.Context(), caddyhttp.ClientIPVarKey, "172.0.0.4:80") + h = ipHash.Select(pool, req, nil) + if h != pool[0] { + t.Error("Expected ip hash policy host to be the first host.") + } + + // We should get nil when there are no healthy hosts + pool[0].setHealthy(false) + pool[1].setHealthy(false) + h = ipHash.Select(pool, req, nil) + if h != nil { + t.Error("Expected ip hash policy host to be nil.") + } + + // Reproduce #4135 + pool = UpstreamPool{ + {Host: new(Host)}, + {Host: new(Host)}, + {Host: new(Host)}, + {Host: new(Host)}, + {Host: new(Host)}, + {Host: new(Host)}, + {Host: new(Host)}, + {Host: new(Host)}, + {Host: new(Host)}, + } + pool[0].setHealthy(false) + pool[1].setHealthy(false) + pool[2].setHealthy(false) + pool[3].setHealthy(false) + pool[4].setHealthy(false) + pool[5].setHealthy(false) + pool[6].setHealthy(false) + pool[7].setHealthy(false) + pool[8].setHealthy(true) + + // We should get a result back when there is one healthy host left. + h = ipHash.Select(pool, req, nil) + if h == nil { + // If it is nil, it means we missed a host even though one is available + t.Error("Expected ip hash policy host to not be nil, but it is nil.") + } +} + func TestFirstPolicy(t *testing.T) { pool := testPool() firstPolicy := new(FirstSelection) @@ -246,6 +392,75 @@ func TestFirstPolicy(t *testing.T) { } } +func TestQueryHashPolicy(t *testing.T) { + pool := testPool() + queryPolicy := QueryHashSelection{Key: "foo"} + + request := httptest.NewRequest(http.MethodGet, "/?foo=1", nil) + h := queryPolicy.Select(pool, request, nil) + if h != pool[0] { + t.Error("Expected query policy host to be the first host.") + } + + request = httptest.NewRequest(http.MethodGet, "/?foo=100000", nil) + h = queryPolicy.Select(pool, request, nil) + if h != pool[0] { + t.Error("Expected query policy host to be the first host.") + } + + request = httptest.NewRequest(http.MethodGet, "/?foo=1", nil) + pool[0].setHealthy(false) + h = queryPolicy.Select(pool, request, nil) + if h != pool[1] { + t.Error("Expected query policy host to be the second host.") + } + + request = httptest.NewRequest(http.MethodGet, "/?foo=100000", nil) + h = queryPolicy.Select(pool, request, nil) + if h != pool[2] { + t.Error("Expected query policy host to be the third host.") + } + + // We should be able to resize the host pool and still be able to predict + // where a request will be routed with the same query used above + pool = UpstreamPool{ + {Host: new(Host)}, + {Host: new(Host)}, + } + + request = httptest.NewRequest(http.MethodGet, "/?foo=1", nil) + h = queryPolicy.Select(pool, request, nil) + if h != pool[0] { + t.Error("Expected query policy host to be the first host.") + } + + pool[0].setHealthy(false) + h = queryPolicy.Select(pool, request, nil) + if h != pool[1] { + t.Error("Expected query policy host to be the second host.") + } + + request = httptest.NewRequest(http.MethodGet, "/?foo=4", nil) + h = queryPolicy.Select(pool, request, nil) + if h != pool[1] { + t.Error("Expected query policy host to be the second host.") + } + + pool[0].setHealthy(false) + pool[1].setHealthy(false) + h = queryPolicy.Select(pool, request, nil) + if h != nil { + t.Error("Expected query policy policy host to be nil.") + } + + request = httptest.NewRequest(http.MethodGet, "/?foo=aa11&foo=bb22", nil) + pool = testPool() + h = queryPolicy.Select(pool, request, nil) + if h != pool[0] { + t.Error("Expected query policy host to be the first host.") + } +} + func TestURIHashPolicy(t *testing.T) { pool := testPool() uriPolicy := new(URIHashSelection) -- 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/caddyfile.go | 19 ++-------- modules/caddyhttp/reverseproxy/healthchecks.go | 2 +- modules/caddyhttp/reverseproxy/hosts.go | 49 ++++++-------------------- modules/caddyhttp/reverseproxy/reverseproxy.go | 14 -------- 4 files changed, 14 insertions(+), 70 deletions(-) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go index fab3099..fc8eed6 100644 --- a/modules/caddyhttp/reverseproxy/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/caddyfile.go @@ -15,7 +15,6 @@ package reverseproxy import ( - "net" "net/http" "reflect" "strconv" @@ -142,15 +141,8 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { h.responseMatchers = make(map[string]caddyhttp.ResponseMatcher) // appendUpstream creates an upstream for address and adds - // it to the list. If the address starts with "srv+" it is - // treated as a SRV-based upstream, and any port will be - // dropped. + // it to the list. appendUpstream := func(address string) error { - isSRV := strings.HasPrefix(address, "srv+") - if isSRV { - address = strings.TrimPrefix(address, "srv+") - } - dialAddr, scheme, err := parseUpstreamDialAddress(address) if err != nil { return d.WrapErr(err) @@ -165,14 +157,7 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { } commonScheme = scheme - if isSRV { - if host, _, err := net.SplitHostPort(dialAddr); err == nil { - dialAddr = host - } - h.Upstreams = append(h.Upstreams, &Upstream{LookupSRV: dialAddr}) - } else { - h.Upstreams = append(h.Upstreams, &Upstream{Dial: dialAddr}) - } + h.Upstreams = append(h.Upstreams, &Upstream{Dial: dialAddr}) return nil } diff --git a/modules/caddyhttp/reverseproxy/healthchecks.go b/modules/caddyhttp/reverseproxy/healthchecks.go index c27b24f..cfc7bdf 100644 --- a/modules/caddyhttp/reverseproxy/healthchecks.go +++ b/modules/caddyhttp/reverseproxy/healthchecks.go @@ -203,7 +203,7 @@ func (h *Handler) doActiveHealthCheckForAllHosts() { } addr.StartPort, addr.EndPort = hcp, hcp } - if upstream.LookupSRV == "" && addr.PortRangeSize() != 1 { + if addr.PortRangeSize() != 1 { h.HealthChecks.Active.logger.Error("multiple addresses (upstream must map to only one address)", zap.String("address", networkAddr), ) diff --git a/modules/caddyhttp/reverseproxy/hosts.go b/modules/caddyhttp/reverseproxy/hosts.go index b97c8b4..298d4f3 100644 --- a/modules/caddyhttp/reverseproxy/hosts.go +++ b/modules/caddyhttp/reverseproxy/hosts.go @@ -17,7 +17,6 @@ package reverseproxy import ( "context" "fmt" - "net" "net/http" "net/netip" "strconv" @@ -48,15 +47,6 @@ type Upstream struct { // backends is down. Also be aware of open proxy vulnerabilities. Dial string `json:"dial,omitempty"` - // DEPRECATED: Use the SRVUpstreams module instead - // (http.reverse_proxy.upstreams.srv). This field will be - // removed in a future version of Caddy. TODO: Remove this field. - // - // If DNS SRV records are used for service discovery with this - // upstream, specify the DNS name for which to look up SRV - // records here, instead of specifying a dial address. - LookupSRV string `json:"lookup_srv,omitempty"` - // The maximum number of simultaneous requests to allow to // this upstream. If set, overrides the global passive health // check UnhealthyRequestCount value. @@ -74,9 +64,6 @@ type Upstream struct { } func (u Upstream) String() string { - if u.LookupSRV != "" { - return u.LookupSRV - } return u.Dial } @@ -110,35 +97,21 @@ func (u *Upstream) Full() bool { } // fillDialInfo returns a filled DialInfo for upstream u, using the request -// context. If the upstream has a SRV lookup configured, that is done and a -// returned address is chosen; otherwise, the upstream's regular dial address -// field is used. Note that the returned value is not a pointer. +// context. Note that the returned value is not a pointer. func (u *Upstream) fillDialInfo(r *http.Request) (DialInfo, error) { repl := r.Context().Value(caddy.ReplacerCtxKey).(*caddy.Replacer) var addr caddy.NetworkAddress - if u.LookupSRV != "" { - // perform DNS lookup for SRV records and choose one - TODO: deprecated - srvName := repl.ReplaceAll(u.LookupSRV, "") - _, records, err := net.DefaultResolver.LookupSRV(r.Context(), "", "", srvName) - if err != nil { - return DialInfo{}, err - } - addr.Network = "tcp" - addr.Host = records[0].Target - addr.StartPort, addr.EndPort = uint(records[0].Port), uint(records[0].Port) - } else { - // use provided dial address - var err error - dial := repl.ReplaceAll(u.Dial, "") - addr, err = caddy.ParseNetworkAddress(dial) - if err != nil { - return DialInfo{}, fmt.Errorf("upstream %s: invalid dial address %s: %v", u.Dial, dial, err) - } - if numPorts := addr.PortRangeSize(); numPorts != 1 { - return DialInfo{}, fmt.Errorf("upstream %s: dial address must represent precisely one socket: %s represents %d", - u.Dial, dial, numPorts) - } + // use provided dial address + var err error + dial := repl.ReplaceAll(u.Dial, "") + addr, err = caddy.ParseNetworkAddress(dial) + if err != nil { + return DialInfo{}, fmt.Errorf("upstream %s: invalid dial address %s: %v", u.Dial, dial, err) + } + if numPorts := addr.PortRangeSize(); numPorts != 1 { + return DialInfo{}, fmt.Errorf("upstream %s: dial address must represent precisely one socket: %s represents %d", + u.Dial, dial, numPorts) } return DialInfo{ 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 53b6fab125f3f2f149d59fcfe13b1e8b1735da56 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Thu, 20 Apr 2023 14:43:51 -0400 Subject: caddyfile: Stricter parsing, error for brace on new line (#5505) --- modules/caddyhttp/reverseproxy/caddyfile.go | 973 ++++++++++----------- .../caddyhttp/reverseproxy/fastcgi/caddyfile.go | 1 + .../reverseproxy/forwardauth/caddyfile.go | 1 + 3 files changed, 488 insertions(+), 487 deletions(-) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go index fc8eed6..728bc2f 100644 --- a/modules/caddyhttp/reverseproxy/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/caddyfile.go @@ -161,556 +161,555 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { return nil } - for d.Next() { - for _, up := range d.RemainingArgs() { - err := appendUpstream(up) + d.Next() // consume the directive name + for _, up := range d.RemainingArgs() { + err := appendUpstream(up) + if err != nil { + return err + } + } + + for nesting := d.Nesting(); d.NextBlock(nesting); { + // if the subdirective has an "@" prefix then we + // parse it as a response matcher for use with "handle_response" + if strings.HasPrefix(d.Val(), matcherPrefix) { + err := caddyhttp.ParseNamedResponseMatcher(d.NewFromNextSegment(), h.responseMatchers) if err != nil { return err } + continue } - for d.NextBlock(0) { - // if the subdirective has an "@" prefix then we - // parse it as a response matcher for use with "handle_response" - if strings.HasPrefix(d.Val(), matcherPrefix) { - err := caddyhttp.ParseNamedResponseMatcher(d.NewFromNextSegment(), h.responseMatchers) + switch d.Val() { + case "to": + args := d.RemainingArgs() + if len(args) == 0 { + return d.ArgErr() + } + for _, up := range args { + err := appendUpstream(up) if err != nil { return err } - continue } - switch d.Val() { - case "to": - args := d.RemainingArgs() - if len(args) == 0 { - return d.ArgErr() - } - for _, up := range args { - err := appendUpstream(up) - if err != nil { - return err - } - } + case "dynamic": + if !d.NextArg() { + return d.ArgErr() + } + if h.DynamicUpstreams != nil { + return d.Err("dynamic upstreams already specified") + } + dynModule := d.Val() + modID := "http.reverse_proxy.upstreams." + dynModule + unm, err := caddyfile.UnmarshalModule(d, modID) + if err != nil { + return err + } + source, ok := unm.(UpstreamSource) + if !ok { + return d.Errf("module %s (%T) is not an UpstreamSource", modID, unm) + } + h.DynamicUpstreamsRaw = caddyconfig.JSONModuleObject(source, "source", dynModule, nil) - case "dynamic": - if !d.NextArg() { - return d.ArgErr() - } - if h.DynamicUpstreams != nil { - return d.Err("dynamic upstreams already specified") - } - dynModule := d.Val() - modID := "http.reverse_proxy.upstreams." + dynModule - unm, err := caddyfile.UnmarshalModule(d, modID) - if err != nil { - return err - } - source, ok := unm.(UpstreamSource) - if !ok { - return d.Errf("module %s (%T) is not an UpstreamSource", modID, unm) - } - h.DynamicUpstreamsRaw = caddyconfig.JSONModuleObject(source, "source", dynModule, nil) + case "lb_policy": + if !d.NextArg() { + return d.ArgErr() + } + if h.LoadBalancing != nil && h.LoadBalancing.SelectionPolicyRaw != nil { + return d.Err("load balancing selection policy already specified") + } + name := d.Val() + modID := "http.reverse_proxy.selection_policies." + name + unm, err := caddyfile.UnmarshalModule(d, modID) + if err != nil { + return err + } + sel, ok := unm.(Selector) + if !ok { + return d.Errf("module %s (%T) is not a reverseproxy.Selector", modID, unm) + } + if h.LoadBalancing == nil { + h.LoadBalancing = new(LoadBalancing) + } + h.LoadBalancing.SelectionPolicyRaw = caddyconfig.JSONModuleObject(sel, "policy", name, nil) - case "lb_policy": - if !d.NextArg() { - return d.ArgErr() - } - if h.LoadBalancing != nil && h.LoadBalancing.SelectionPolicyRaw != nil { - return d.Err("load balancing selection policy already specified") - } - name := d.Val() - modID := "http.reverse_proxy.selection_policies." + name - unm, err := caddyfile.UnmarshalModule(d, modID) - if err != nil { - return err - } - sel, ok := unm.(Selector) - if !ok { - return d.Errf("module %s (%T) is not a reverseproxy.Selector", modID, unm) - } - if h.LoadBalancing == nil { - h.LoadBalancing = new(LoadBalancing) - } - h.LoadBalancing.SelectionPolicyRaw = caddyconfig.JSONModuleObject(sel, "policy", name, nil) + case "lb_retries": + if !d.NextArg() { + return d.ArgErr() + } + tries, err := strconv.Atoi(d.Val()) + if err != nil { + return d.Errf("bad lb_retries number '%s': %v", d.Val(), err) + } + if h.LoadBalancing == nil { + h.LoadBalancing = new(LoadBalancing) + } + h.LoadBalancing.Retries = tries - case "lb_retries": - if !d.NextArg() { - return d.ArgErr() - } - tries, err := strconv.Atoi(d.Val()) - if err != nil { - return d.Errf("bad lb_retries number '%s': %v", d.Val(), err) - } - if h.LoadBalancing == nil { - h.LoadBalancing = new(LoadBalancing) - } - h.LoadBalancing.Retries = tries + case "lb_try_duration": + if !d.NextArg() { + return d.ArgErr() + } + if h.LoadBalancing == nil { + h.LoadBalancing = new(LoadBalancing) + } + dur, err := caddy.ParseDuration(d.Val()) + if err != nil { + return d.Errf("bad duration value %s: %v", d.Val(), err) + } + h.LoadBalancing.TryDuration = caddy.Duration(dur) - case "lb_try_duration": - if !d.NextArg() { - return d.ArgErr() - } - if h.LoadBalancing == nil { - h.LoadBalancing = new(LoadBalancing) - } - dur, err := caddy.ParseDuration(d.Val()) - if err != nil { - return d.Errf("bad duration value %s: %v", d.Val(), err) - } - h.LoadBalancing.TryDuration = caddy.Duration(dur) + case "lb_try_interval": + if !d.NextArg() { + return d.ArgErr() + } + if h.LoadBalancing == nil { + h.LoadBalancing = new(LoadBalancing) + } + dur, err := caddy.ParseDuration(d.Val()) + if err != nil { + return d.Errf("bad interval value '%s': %v", d.Val(), err) + } + h.LoadBalancing.TryInterval = caddy.Duration(dur) - case "lb_try_interval": - if !d.NextArg() { - return d.ArgErr() - } - if h.LoadBalancing == nil { - h.LoadBalancing = new(LoadBalancing) - } - dur, err := caddy.ParseDuration(d.Val()) - if err != nil { - return d.Errf("bad interval value '%s': %v", d.Val(), err) - } - h.LoadBalancing.TryInterval = caddy.Duration(dur) + case "lb_retry_match": + matcherSet, err := caddyhttp.ParseCaddyfileNestedMatcherSet(d) + if err != nil { + return d.Errf("failed to parse lb_retry_match: %v", err) + } + if h.LoadBalancing == nil { + h.LoadBalancing = new(LoadBalancing) + } + h.LoadBalancing.RetryMatchRaw = append(h.LoadBalancing.RetryMatchRaw, matcherSet) - case "lb_retry_match": - matcherSet, err := caddyhttp.ParseCaddyfileNestedMatcherSet(d) - if err != nil { - return d.Errf("failed to parse lb_retry_match: %v", err) - } - if h.LoadBalancing == nil { - h.LoadBalancing = new(LoadBalancing) - } - h.LoadBalancing.RetryMatchRaw = append(h.LoadBalancing.RetryMatchRaw, matcherSet) + 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_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() + } + if h.HealthChecks == nil { + h.HealthChecks = new(HealthChecks) + } + if h.HealthChecks.Active == nil { + 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_path": - 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.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() { + return d.ArgErr() + } + if h.HealthChecks == nil { + h.HealthChecks = new(HealthChecks) + } + if h.HealthChecks.Active == nil { + h.HealthChecks.Active = new(ActiveHealthChecks) + } + portNum, err := strconv.Atoi(d.Val()) + if err != nil { + return d.Errf("bad port number '%s': %v", d.Val(), err) + } + h.HealthChecks.Active.Port = portNum - case "health_port": - if !d.NextArg() { - return d.ArgErr() - } - if h.HealthChecks == nil { - h.HealthChecks = new(HealthChecks) - } - if h.HealthChecks.Active == nil { - h.HealthChecks.Active = new(ActiveHealthChecks) + case "health_headers": + healthHeaders := make(http.Header) + for nesting := d.Nesting(); d.NextBlock(nesting); { + key := d.Val() + values := d.RemainingArgs() + if len(values) == 0 { + values = append(values, "") } - portNum, err := strconv.Atoi(d.Val()) - if err != nil { - return d.Errf("bad port number '%s': %v", d.Val(), err) - } - h.HealthChecks.Active.Port = portNum + healthHeaders[key] = values + } + if h.HealthChecks == nil { + h.HealthChecks = new(HealthChecks) + } + if h.HealthChecks.Active == nil { + h.HealthChecks.Active = new(ActiveHealthChecks) + } + h.HealthChecks.Active.Headers = healthHeaders - case "health_headers": - healthHeaders := make(http.Header) - for nesting := d.Nesting(); d.NextBlock(nesting); { - key := d.Val() - values := d.RemainingArgs() - if len(values) == 0 { - values = append(values, "") - } - healthHeaders[key] = values - } - if h.HealthChecks == nil { - h.HealthChecks = new(HealthChecks) - } - if h.HealthChecks.Active == nil { - h.HealthChecks.Active = new(ActiveHealthChecks) - } - h.HealthChecks.Active.Headers = healthHeaders + case "health_interval": + if !d.NextArg() { + return d.ArgErr() + } + if h.HealthChecks == nil { + h.HealthChecks = new(HealthChecks) + } + if h.HealthChecks.Active == nil { + h.HealthChecks.Active = new(ActiveHealthChecks) + } + dur, err := caddy.ParseDuration(d.Val()) + if err != nil { + return d.Errf("bad interval value %s: %v", d.Val(), err) + } + h.HealthChecks.Active.Interval = caddy.Duration(dur) - case "health_interval": - if !d.NextArg() { - return d.ArgErr() - } - if h.HealthChecks == nil { - h.HealthChecks = new(HealthChecks) - } - if h.HealthChecks.Active == nil { - h.HealthChecks.Active = new(ActiveHealthChecks) - } - dur, err := caddy.ParseDuration(d.Val()) - if err != nil { - return d.Errf("bad interval value %s: %v", d.Val(), err) - } - h.HealthChecks.Active.Interval = caddy.Duration(dur) + case "health_timeout": + if !d.NextArg() { + return d.ArgErr() + } + if h.HealthChecks == nil { + h.HealthChecks = new(HealthChecks) + } + if h.HealthChecks.Active == nil { + h.HealthChecks.Active = new(ActiveHealthChecks) + } + dur, err := caddy.ParseDuration(d.Val()) + if err != nil { + return d.Errf("bad timeout value %s: %v", d.Val(), err) + } + h.HealthChecks.Active.Timeout = caddy.Duration(dur) - case "health_timeout": - if !d.NextArg() { - return d.ArgErr() - } - if h.HealthChecks == nil { - h.HealthChecks = new(HealthChecks) - } - if h.HealthChecks.Active == nil { - h.HealthChecks.Active = new(ActiveHealthChecks) - } - dur, err := caddy.ParseDuration(d.Val()) - if err != nil { - return d.Errf("bad timeout value %s: %v", d.Val(), err) - } - h.HealthChecks.Active.Timeout = caddy.Duration(dur) + case "health_status": + if !d.NextArg() { + return d.ArgErr() + } + if h.HealthChecks == nil { + h.HealthChecks = new(HealthChecks) + } + if h.HealthChecks.Active == nil { + h.HealthChecks.Active = new(ActiveHealthChecks) + } + val := d.Val() + if len(val) == 3 && strings.HasSuffix(val, "xx") { + val = val[:1] + } + statusNum, err := strconv.Atoi(val) + if err != nil { + return d.Errf("bad status value '%s': %v", d.Val(), err) + } + h.HealthChecks.Active.ExpectStatus = statusNum - case "health_status": - if !d.NextArg() { - return d.ArgErr() - } - if h.HealthChecks == nil { - h.HealthChecks = new(HealthChecks) - } - if h.HealthChecks.Active == nil { - h.HealthChecks.Active = new(ActiveHealthChecks) - } - val := d.Val() - if len(val) == 3 && strings.HasSuffix(val, "xx") { - val = val[:1] - } - statusNum, err := strconv.Atoi(val) - if err != nil { - return d.Errf("bad status value '%s': %v", d.Val(), err) - } - h.HealthChecks.Active.ExpectStatus = statusNum + case "health_body": + 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.ExpectBody = d.Val() - case "health_body": - 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.ExpectBody = d.Val() + case "max_fails": + if !d.NextArg() { + return d.ArgErr() + } + if h.HealthChecks == nil { + h.HealthChecks = new(HealthChecks) + } + if h.HealthChecks.Passive == nil { + h.HealthChecks.Passive = new(PassiveHealthChecks) + } + maxFails, err := strconv.Atoi(d.Val()) + if err != nil { + return d.Errf("invalid maximum fail count '%s': %v", d.Val(), err) + } + h.HealthChecks.Passive.MaxFails = maxFails - case "max_fails": - if !d.NextArg() { - return d.ArgErr() - } - if h.HealthChecks == nil { - h.HealthChecks = new(HealthChecks) - } - if h.HealthChecks.Passive == nil { - h.HealthChecks.Passive = new(PassiveHealthChecks) - } - maxFails, err := strconv.Atoi(d.Val()) - if err != nil { - return d.Errf("invalid maximum fail count '%s': %v", d.Val(), err) - } - h.HealthChecks.Passive.MaxFails = maxFails + case "fail_duration": + if !d.NextArg() { + return d.ArgErr() + } + if h.HealthChecks == nil { + h.HealthChecks = new(HealthChecks) + } + if h.HealthChecks.Passive == nil { + h.HealthChecks.Passive = new(PassiveHealthChecks) + } + dur, err := caddy.ParseDuration(d.Val()) + if err != nil { + return d.Errf("bad duration value '%s': %v", d.Val(), err) + } + h.HealthChecks.Passive.FailDuration = caddy.Duration(dur) - case "fail_duration": - if !d.NextArg() { - return d.ArgErr() - } - if h.HealthChecks == nil { - h.HealthChecks = new(HealthChecks) - } - if h.HealthChecks.Passive == nil { - h.HealthChecks.Passive = new(PassiveHealthChecks) - } - dur, err := caddy.ParseDuration(d.Val()) - if err != nil { - return d.Errf("bad duration value '%s': %v", d.Val(), err) - } - h.HealthChecks.Passive.FailDuration = caddy.Duration(dur) + case "unhealthy_request_count": + if !d.NextArg() { + return d.ArgErr() + } + if h.HealthChecks == nil { + h.HealthChecks = new(HealthChecks) + } + if h.HealthChecks.Passive == nil { + h.HealthChecks.Passive = new(PassiveHealthChecks) + } + maxConns, err := strconv.Atoi(d.Val()) + if err != nil { + return d.Errf("invalid maximum connection count '%s': %v", d.Val(), err) + } + h.HealthChecks.Passive.UnhealthyRequestCount = maxConns - case "unhealthy_request_count": - if !d.NextArg() { - return d.ArgErr() - } - if h.HealthChecks == nil { - h.HealthChecks = new(HealthChecks) - } - if h.HealthChecks.Passive == nil { - h.HealthChecks.Passive = new(PassiveHealthChecks) + case "unhealthy_status": + args := d.RemainingArgs() + if len(args) == 0 { + return d.ArgErr() + } + if h.HealthChecks == nil { + h.HealthChecks = new(HealthChecks) + } + if h.HealthChecks.Passive == nil { + h.HealthChecks.Passive = new(PassiveHealthChecks) + } + for _, arg := range args { + if len(arg) == 3 && strings.HasSuffix(arg, "xx") { + arg = arg[:1] } - maxConns, err := strconv.Atoi(d.Val()) + statusNum, err := strconv.Atoi(arg) if err != nil { - return d.Errf("invalid maximum connection count '%s': %v", d.Val(), err) + return d.Errf("bad status value '%s': %v", d.Val(), err) } - h.HealthChecks.Passive.UnhealthyRequestCount = maxConns + h.HealthChecks.Passive.UnhealthyStatus = append(h.HealthChecks.Passive.UnhealthyStatus, statusNum) + } - case "unhealthy_status": - args := d.RemainingArgs() - if len(args) == 0 { - return d.ArgErr() - } - if h.HealthChecks == nil { - h.HealthChecks = new(HealthChecks) - } - if h.HealthChecks.Passive == nil { - h.HealthChecks.Passive = new(PassiveHealthChecks) - } - for _, arg := range args { - if len(arg) == 3 && strings.HasSuffix(arg, "xx") { - arg = arg[:1] - } - statusNum, err := strconv.Atoi(arg) - if err != nil { - return d.Errf("bad status value '%s': %v", d.Val(), err) - } - h.HealthChecks.Passive.UnhealthyStatus = append(h.HealthChecks.Passive.UnhealthyStatus, statusNum) - } + case "unhealthy_latency": + if !d.NextArg() { + return d.ArgErr() + } + if h.HealthChecks == nil { + h.HealthChecks = new(HealthChecks) + } + if h.HealthChecks.Passive == nil { + h.HealthChecks.Passive = new(PassiveHealthChecks) + } + dur, err := caddy.ParseDuration(d.Val()) + if err != nil { + return d.Errf("bad duration value '%s': %v", d.Val(), err) + } + h.HealthChecks.Passive.UnhealthyLatency = caddy.Duration(dur) - case "unhealthy_latency": - if !d.NextArg() { - return d.ArgErr() - } - if h.HealthChecks == nil { - h.HealthChecks = new(HealthChecks) - } - if h.HealthChecks.Passive == nil { - h.HealthChecks.Passive = new(PassiveHealthChecks) - } + case "flush_interval": + if !d.NextArg() { + return d.ArgErr() + } + if fi, err := strconv.Atoi(d.Val()); err == nil { + h.FlushInterval = caddy.Duration(fi) + } else { dur, err := caddy.ParseDuration(d.Val()) if err != nil { return d.Errf("bad duration value '%s': %v", d.Val(), err) } - h.HealthChecks.Passive.UnhealthyLatency = caddy.Duration(dur) + h.FlushInterval = caddy.Duration(dur) + } - case "flush_interval": - if !d.NextArg() { - return d.ArgErr() - } - if fi, err := strconv.Atoi(d.Val()); err == nil { - h.FlushInterval = caddy.Duration(fi) - } else { - dur, err := caddy.ParseDuration(d.Val()) - if err != nil { - return d.Errf("bad duration value '%s': %v", d.Val(), err) - } - h.FlushInterval = caddy.Duration(dur) - } + case "request_buffers", "response_buffers": + subdir := d.Val() + if !d.NextArg() { + return d.ArgErr() + } + size, err := humanize.ParseBytes(d.Val()) + if err != nil { + return d.Errf("invalid byte size '%s': %v", d.Val(), err) + } + if d.NextArg() { + return d.ArgErr() + } + if subdir == "request_buffers" { + h.RequestBuffers = int64(size) + } else if subdir == "response_buffers" { + h.ResponseBuffers = int64(size) - case "request_buffers", "response_buffers": - subdir := d.Val() - if !d.NextArg() { - return d.ArgErr() - } - size, err := humanize.ParseBytes(d.Val()) - if err != nil { - return d.Errf("invalid byte size '%s': %v", d.Val(), err) - } - if d.NextArg() { - return d.ArgErr() - } - if subdir == "request_buffers" { - h.RequestBuffers = int64(size) - } else if subdir == "response_buffers" { - h.ResponseBuffers = int64(size) + } - } + // TODO: These three properties are deprecated; remove them sometime after v2.6.4 + case "buffer_requests": // TODO: deprecated + if d.NextArg() { + return d.ArgErr() + } + caddy.Log().Named("config.adapter.caddyfile").Warn("DEPRECATED: buffer_requests: use request_buffers instead (with a maximum buffer size)") + h.DeprecatedBufferRequests = true + case "buffer_responses": // TODO: deprecated + if d.NextArg() { + return d.ArgErr() + } + caddy.Log().Named("config.adapter.caddyfile").Warn("DEPRECATED: buffer_responses: use response_buffers instead (with a maximum buffer size)") + h.DeprecatedBufferResponses = true + case "max_buffer_size": // TODO: deprecated + if !d.NextArg() { + return d.ArgErr() + } + size, err := humanize.ParseBytes(d.Val()) + if err != nil { + return d.Errf("invalid byte size '%s': %v", d.Val(), err) + } + if d.NextArg() { + return d.ArgErr() + } + caddy.Log().Named("config.adapter.caddyfile").Warn("DEPRECATED: max_buffer_size: use request_buffers and/or response_buffers instead (with maximum buffer sizes)") + h.DeprecatedMaxBufferSize = int64(size) - // TODO: These three properties are deprecated; remove them sometime after v2.6.4 - case "buffer_requests": // TODO: deprecated - if d.NextArg() { - return d.ArgErr() + case "trusted_proxies": + for d.NextArg() { + if d.Val() == "private_ranges" { + h.TrustedProxies = append(h.TrustedProxies, caddyhttp.PrivateRangesCIDR()...) + continue } - caddy.Log().Named("config.adapter.caddyfile").Warn("DEPRECATED: buffer_requests: use request_buffers instead (with a maximum buffer size)") - h.DeprecatedBufferRequests = true - case "buffer_responses": // TODO: deprecated - if d.NextArg() { - return d.ArgErr() - } - caddy.Log().Named("config.adapter.caddyfile").Warn("DEPRECATED: buffer_responses: use response_buffers instead (with a maximum buffer size)") - h.DeprecatedBufferResponses = true - case "max_buffer_size": // TODO: deprecated - if !d.NextArg() { - return d.ArgErr() - } - size, err := humanize.ParseBytes(d.Val()) - if err != nil { - return d.Errf("invalid byte size '%s': %v", d.Val(), err) - } - if d.NextArg() { - return d.ArgErr() - } - caddy.Log().Named("config.adapter.caddyfile").Warn("DEPRECATED: max_buffer_size: use request_buffers and/or response_buffers instead (with maximum buffer sizes)") - h.DeprecatedMaxBufferSize = int64(size) + h.TrustedProxies = append(h.TrustedProxies, d.Val()) + } - case "trusted_proxies": - for d.NextArg() { - if d.Val() == "private_ranges" { - h.TrustedProxies = append(h.TrustedProxies, caddyhttp.PrivateRangesCIDR()...) - continue - } - h.TrustedProxies = append(h.TrustedProxies, d.Val()) - } + case "header_up": + var err error - case "header_up": - var err error + if h.Headers == nil { + h.Headers = new(headers.Handler) + } + if h.Headers.Request == nil { + h.Headers.Request = new(headers.HeaderOps) + } + args := d.RemainingArgs() - if h.Headers == nil { - h.Headers = new(headers.Handler) + switch len(args) { + case 1: + err = headers.CaddyfileHeaderOp(h.Headers.Request, args[0], "", "") + case 2: + // some lint checks, I guess + if strings.EqualFold(args[0], "host") && (args[1] == "{hostport}" || args[1] == "{http.request.hostport}") { + caddy.Log().Named("caddyfile").Warn("Unnecessary header_up Host: the reverse proxy's default behavior is to pass headers to the upstream") } - if h.Headers.Request == nil { - h.Headers.Request = new(headers.HeaderOps) + if strings.EqualFold(args[0], "x-forwarded-for") && (args[1] == "{remote}" || args[1] == "{http.request.remote}" || args[1] == "{remote_host}" || args[1] == "{http.request.remote.host}") { + caddy.Log().Named("caddyfile").Warn("Unnecessary header_up X-Forwarded-For: the reverse proxy's default behavior is to pass headers to the upstream") } - args := d.RemainingArgs() - - switch len(args) { - case 1: - err = headers.CaddyfileHeaderOp(h.Headers.Request, args[0], "", "") - case 2: - // some lint checks, I guess - if strings.EqualFold(args[0], "host") && (args[1] == "{hostport}" || args[1] == "{http.request.hostport}") { - caddy.Log().Named("caddyfile").Warn("Unnecessary header_up Host: the reverse proxy's default behavior is to pass headers to the upstream") - } - if strings.EqualFold(args[0], "x-forwarded-for") && (args[1] == "{remote}" || args[1] == "{http.request.remote}" || args[1] == "{remote_host}" || args[1] == "{http.request.remote.host}") { - caddy.Log().Named("caddyfile").Warn("Unnecessary header_up X-Forwarded-For: the reverse proxy's default behavior is to pass headers to the upstream") - } - if strings.EqualFold(args[0], "x-forwarded-proto") && (args[1] == "{scheme}" || args[1] == "{http.request.scheme}") { - caddy.Log().Named("caddyfile").Warn("Unnecessary header_up X-Forwarded-Proto: the reverse proxy's default behavior is to pass headers to the upstream") - } - if strings.EqualFold(args[0], "x-forwarded-host") && (args[1] == "{host}" || args[1] == "{http.request.host}" || args[1] == "{hostport}" || args[1] == "{http.request.hostport}") { - caddy.Log().Named("caddyfile").Warn("Unnecessary header_up X-Forwarded-Host: the reverse proxy's default behavior is to pass headers to the upstream") - } - err = headers.CaddyfileHeaderOp(h.Headers.Request, args[0], args[1], "") - case 3: - err = headers.CaddyfileHeaderOp(h.Headers.Request, args[0], args[1], args[2]) - default: - return d.ArgErr() + if strings.EqualFold(args[0], "x-forwarded-proto") && (args[1] == "{scheme}" || args[1] == "{http.request.scheme}") { + caddy.Log().Named("caddyfile").Warn("Unnecessary header_up X-Forwarded-Proto: the reverse proxy's default behavior is to pass headers to the upstream") } - - if err != nil { - return d.Err(err.Error()) + if strings.EqualFold(args[0], "x-forwarded-host") && (args[1] == "{host}" || args[1] == "{http.request.host}" || args[1] == "{hostport}" || args[1] == "{http.request.hostport}") { + caddy.Log().Named("caddyfile").Warn("Unnecessary header_up X-Forwarded-Host: the reverse proxy's default behavior is to pass headers to the upstream") } + err = headers.CaddyfileHeaderOp(h.Headers.Request, args[0], args[1], "") + case 3: + err = headers.CaddyfileHeaderOp(h.Headers.Request, args[0], args[1], args[2]) + default: + return d.ArgErr() + } - case "header_down": - var err error - - if h.Headers == nil { - h.Headers = new(headers.Handler) - } - if h.Headers.Response == nil { - h.Headers.Response = &headers.RespHeaderOps{ - HeaderOps: new(headers.HeaderOps), - } - } - args := d.RemainingArgs() - switch len(args) { - case 1: - err = headers.CaddyfileHeaderOp(h.Headers.Response.HeaderOps, args[0], "", "") - case 2: - err = headers.CaddyfileHeaderOp(h.Headers.Response.HeaderOps, args[0], args[1], "") - case 3: - err = headers.CaddyfileHeaderOp(h.Headers.Response.HeaderOps, args[0], args[1], args[2]) - default: - return d.ArgErr() - } + if err != nil { + return d.Err(err.Error()) + } - if err != nil { - return d.Err(err.Error()) - } + case "header_down": + var err error - case "method": - if !d.NextArg() { - return d.ArgErr() - } - if h.Rewrite == nil { - h.Rewrite = &rewrite.Rewrite{} - } - h.Rewrite.Method = d.Val() - if d.NextArg() { - return d.ArgErr() + if h.Headers == nil { + h.Headers = new(headers.Handler) + } + if h.Headers.Response == nil { + h.Headers.Response = &headers.RespHeaderOps{ + HeaderOps: new(headers.HeaderOps), } + } + args := d.RemainingArgs() + switch len(args) { + case 1: + err = headers.CaddyfileHeaderOp(h.Headers.Response.HeaderOps, args[0], "", "") + case 2: + err = headers.CaddyfileHeaderOp(h.Headers.Response.HeaderOps, args[0], args[1], "") + case 3: + err = headers.CaddyfileHeaderOp(h.Headers.Response.HeaderOps, args[0], args[1], args[2]) + default: + return d.ArgErr() + } - case "rewrite": - if !d.NextArg() { - return d.ArgErr() - } - if h.Rewrite == nil { - h.Rewrite = &rewrite.Rewrite{} - } - h.Rewrite.URI = d.Val() - if d.NextArg() { - return d.ArgErr() - } + if err != nil { + return d.Err(err.Error()) + } - case "transport": - if !d.NextArg() { - return d.ArgErr() - } - if h.TransportRaw != nil { - return d.Err("transport already specified") - } - transportModuleName = d.Val() - modID := "http.reverse_proxy.transport." + transportModuleName - unm, err := caddyfile.UnmarshalModule(d, modID) - if err != nil { - return err - } - rt, ok := unm.(http.RoundTripper) - if !ok { - return d.Errf("module %s (%T) is not a RoundTripper", modID, unm) - } - transport = rt + case "method": + if !d.NextArg() { + return d.ArgErr() + } + if h.Rewrite == nil { + h.Rewrite = &rewrite.Rewrite{} + } + h.Rewrite.Method = d.Val() + if d.NextArg() { + return d.ArgErr() + } - case "handle_response": - // delegate the parsing of handle_response to the caller, - // since we need the httpcaddyfile.Helper to parse subroutes. - // See h.FinalizeUnmarshalCaddyfile - h.handleResponseSegments = append(h.handleResponseSegments, d.NewFromNextSegment()) + case "rewrite": + if !d.NextArg() { + return d.ArgErr() + } + if h.Rewrite == nil { + h.Rewrite = &rewrite.Rewrite{} + } + h.Rewrite.URI = d.Val() + if d.NextArg() { + return d.ArgErr() + } - case "replace_status": - args := d.RemainingArgs() - if len(args) != 1 && len(args) != 2 { - return d.Errf("must have one or two arguments: an optional response matcher, and a status code") - } + case "transport": + if !d.NextArg() { + return d.ArgErr() + } + if h.TransportRaw != nil { + return d.Err("transport already specified") + } + transportModuleName = d.Val() + modID := "http.reverse_proxy.transport." + transportModuleName + unm, err := caddyfile.UnmarshalModule(d, modID) + if err != nil { + return err + } + rt, ok := unm.(http.RoundTripper) + if !ok { + return d.Errf("module %s (%T) is not a RoundTripper", modID, unm) + } + transport = rt + + case "handle_response": + // delegate the parsing of handle_response to the caller, + // since we need the httpcaddyfile.Helper to parse subroutes. + // See h.FinalizeUnmarshalCaddyfile + h.handleResponseSegments = append(h.handleResponseSegments, d.NewFromNextSegment()) + + case "replace_status": + args := d.RemainingArgs() + if len(args) != 1 && len(args) != 2 { + return d.Errf("must have one or two arguments: an optional response matcher, and a status code") + } - responseHandler := caddyhttp.ResponseHandler{} + responseHandler := caddyhttp.ResponseHandler{} - if len(args) == 2 { - if !strings.HasPrefix(args[0], matcherPrefix) { - return d.Errf("must use a named response matcher, starting with '@'") - } - foundMatcher, ok := h.responseMatchers[args[0]] - if !ok { - return d.Errf("no named response matcher defined with name '%s'", args[0][1:]) - } - responseHandler.Match = &foundMatcher - responseHandler.StatusCode = caddyhttp.WeakString(args[1]) - } else if len(args) == 1 { - responseHandler.StatusCode = caddyhttp.WeakString(args[0]) + if len(args) == 2 { + if !strings.HasPrefix(args[0], matcherPrefix) { + return d.Errf("must use a named response matcher, starting with '@'") } - - // make sure there's no block, cause it doesn't make sense - if d.NextBlock(1) { - return d.Errf("cannot define routes for 'replace_status', use 'handle_response' instead.") + foundMatcher, ok := h.responseMatchers[args[0]] + if !ok { + return d.Errf("no named response matcher defined with name '%s'", args[0][1:]) } + responseHandler.Match = &foundMatcher + responseHandler.StatusCode = caddyhttp.WeakString(args[1]) + } else if len(args) == 1 { + responseHandler.StatusCode = caddyhttp.WeakString(args[0]) + } - h.HandleResponse = append( - h.HandleResponse, - responseHandler, - ) - - default: - return d.Errf("unrecognized subdirective %s", d.Val()) + // make sure there's no block, cause it doesn't make sense + if d.NextBlock(1) { + return d.Errf("cannot define routes for 'replace_status', use 'handle_response' instead.") } + + h.HandleResponse = append( + h.HandleResponse, + responseHandler, + ) + + default: + return d.Errf("unrecognized subdirective %s", d.Val()) } } diff --git a/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go b/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go index 4687e68..a24a3ed 100644 --- a/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go @@ -373,6 +373,7 @@ func parsePHPFastCGI(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error // the rest of the config is specified by the user // using the reverse_proxy directive syntax + dispenser.Next() // consume the directive name err = rpHandler.UnmarshalCaddyfile(dispenser) if err != nil { return nil, err diff --git a/modules/caddyhttp/reverseproxy/forwardauth/caddyfile.go b/modules/caddyhttp/reverseproxy/forwardauth/caddyfile.go index dbd2a29..8350096 100644 --- a/modules/caddyhttp/reverseproxy/forwardauth/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/forwardauth/caddyfile.go @@ -216,6 +216,7 @@ func parseCaddyfile(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) // the rest of the config is specified by the user // using the reverse_proxy directive syntax + dispenser.Next() // consume the directive name err = rpHandler.UnmarshalCaddyfile(dispenser) if err != nil { return nil, err -- cgit v1.2.3 From 2b04e09fa7830a2d24d863e448ebbdc51d537fbe Mon Sep 17 00:00:00 2001 From: "Y.Horie" Date: Wed, 26 Apr 2023 00:59:26 +0900 Subject: reverseproxy: Fix reinitialize upstream healthy metrics (#5498) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Dávid Szabó --- modules/caddyhttp/reverseproxy/metrics.go | 2 ++ 1 file changed, 2 insertions(+) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/metrics.go b/modules/caddyhttp/reverseproxy/metrics.go index 4272bc4..d3c8ee0 100644 --- a/modules/caddyhttp/reverseproxy/metrics.go +++ b/modules/caddyhttp/reverseproxy/metrics.go @@ -39,6 +39,8 @@ func newMetricsUpstreamsHealthyUpdater(handler *Handler) *metricsUpstreamsHealth initReverseProxyMetrics(handler) }) + reverseProxyMetrics.upstreamsHealthy.Reset() + return &metricsUpstreamsHealthyUpdater{handler} } -- cgit v1.2.3 From f0e39817746903bb24948893348ae1cab67f1e46 Mon Sep 17 00:00:00 2001 From: Dave Henderson Date: Wed, 26 Apr 2023 22:46:41 -0400 Subject: logging: Add traceID field to access logs when tracing is active (#5507) Co-authored-by: Francis Lavoie --- modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go b/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go index ec194e7..5160067 100644 --- a/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go +++ b/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go @@ -254,9 +254,7 @@ func (t Transport) buildEnv(r *http.Request) (envVars, error) { // if we didn't get a split result here. // See https://github.com/caddyserver/caddy/issues/3718 if pathInfo == "" { - if remainder, ok := repl.GetString("http.matchers.file.remainder"); ok { - pathInfo = remainder - } + pathInfo, _ = repl.GetString("http.matchers.file.remainder") } // SCRIPT_FILENAME is the absolute path of SCRIPT_NAME @@ -286,10 +284,7 @@ func (t Transport) buildEnv(r *http.Request) (envVars, error) { reqHost = r.Host } - authUser := "" - if val, ok := repl.Get("http.auth.user.id"); ok { - authUser = val.(string) - } + authUser, _ := repl.GetString("http.auth.user.id") // Some variables are unused but cleared explicitly to prevent // the parent environment from interfering. -- cgit v1.2.3 From 3f20a7c9f348122d5fae7074b9fa17651189bb9a Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Wed, 3 May 2023 13:07:22 -0400 Subject: acmeserver: Configurable `resolvers`, fix smallstep deprecations (#5500) * acmeserver: Configurable `resolvers`, fix smallstep deprecations * Improve default net/port * Update proxy resolvers parsing to use the new function * Update listeners.go Co-authored-by: itsxaos <33079230+itsxaos@users.noreply.github.com> --------- Co-authored-by: itsxaos <33079230+itsxaos@users.noreply.github.com> --- modules/caddyhttp/reverseproxy/upstreams.go | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/upstreams.go b/modules/caddyhttp/reverseproxy/upstreams.go index 30bd7b5..42fedb6 100644 --- a/modules/caddyhttp/reverseproxy/upstreams.go +++ b/modules/caddyhttp/reverseproxy/upstreams.go @@ -8,7 +8,6 @@ import ( "net" "net/http" "strconv" - "strings" "sync" "time" @@ -471,16 +470,9 @@ type UpstreamResolver struct { // and ensures they're ready to be used. func (u *UpstreamResolver) ParseAddresses() error { for _, v := range u.Addresses { - addr, err := caddy.ParseNetworkAddress(v) + addr, err := caddy.ParseNetworkAddressWithDefaults(v, "udp", 53) if err != nil { - // If a port wasn't specified for the resolver, - // try defaulting to 53 and parse again - if strings.Contains(err.Error(), "missing port in address") { - addr, err = caddy.ParseNetworkAddress(v + ":53") - } - if err != nil { - return err - } + return err } if addr.PortRangeSize() != 1 { return fmt.Errorf("resolver address must have exactly one address; cannot call %v", addr) -- cgit v1.2.3 From c8032867b14e52d18c239ed97140be3ae05d9f2f Mon Sep 17 00:00:00 2001 From: eanavitarte <130239684+eanavitarte@users.noreply.github.com> Date: Wed, 3 May 2023 19:40:49 -0500 Subject: fastcgi: Fix `capture_stderr` (#5515) --- modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go | 1 + 1 file changed, 1 insertion(+) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go b/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go index 5160067..f150edc 100644 --- a/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go +++ b/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go @@ -171,6 +171,7 @@ func (t Transport) RoundTrip(r *http.Request) (*http.Response, error) { rwc: conn, reqID: 1, logger: logger, + stderr: t.CaptureStderr, } // read/write timeouts -- cgit v1.2.3 From 48598e1f2a370c2440b38f0b77e4d74748111b9a Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Fri, 5 May 2023 17:08:10 -0400 Subject: reverseproxy: Add `fallback` for some policies, instead of always random (#5488) --- .../caddyhttp/reverseproxy/selectionpolicies.go | 184 +++++++++++++++++---- .../reverseproxy/selectionpolicies_test.go | 92 +++++++++-- 2 files changed, 237 insertions(+), 39 deletions(-) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/selectionpolicies.go b/modules/caddyhttp/reverseproxy/selectionpolicies.go index 4184df5..a2985f1 100644 --- a/modules/caddyhttp/reverseproxy/selectionpolicies.go +++ b/modules/caddyhttp/reverseproxy/selectionpolicies.go @@ -18,6 +18,7 @@ import ( "crypto/hmac" "crypto/sha256" "encoding/hex" + "encoding/json" "fmt" "hash/fnv" weakrand "math/rand" @@ -29,6 +30,7 @@ import ( "time" "github.com/caddyserver/caddy/v2" + "github.com/caddyserver/caddy/v2/caddyconfig" "github.com/caddyserver/caddy/v2/caddyconfig/caddyfile" "github.com/caddyserver/caddy/v2/modules/caddyhttp" ) @@ -372,6 +374,10 @@ func (r *URIHashSelection) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { type QueryHashSelection struct { // The query key whose value is to be hashed and used for upstream selection. Key string `json:"key,omitempty"` + + // The fallback policy to use if the query key is not present. Defaults to `random`. + FallbackRaw json.RawMessage `json:"fallback,omitempty" caddy:"namespace=http.reverse_proxy.selection_policies inline_key=policy"` + fallback Selector } // CaddyModule returns the Caddy module information. @@ -382,12 +388,24 @@ func (QueryHashSelection) CaddyModule() caddy.ModuleInfo { } } -// Select returns an available host, if any. -func (s QueryHashSelection) Select(pool UpstreamPool, req *http.Request, _ http.ResponseWriter) *Upstream { +// Provision sets up the module. +func (s *QueryHashSelection) Provision(ctx caddy.Context) error { if s.Key == "" { - return nil + return fmt.Errorf("query key is required") + } + if s.FallbackRaw == nil { + s.FallbackRaw = caddyconfig.JSONModuleObject(RandomSelection{}, "policy", "random", nil) } + mod, err := ctx.LoadModule(s, "FallbackRaw") + if err != nil { + return fmt.Errorf("loading fallback selection policy: %s", err) + } + s.fallback = mod.(Selector) + return nil +} +// Select returns an available host, if any. +func (s QueryHashSelection) Select(pool UpstreamPool, req *http.Request, _ http.ResponseWriter) *Upstream { // Since the query may have multiple values for the same key, // we'll join them to avoid a problem where the user can control // the upstream that the request goes to by sending multiple values @@ -397,7 +415,7 @@ func (s QueryHashSelection) Select(pool UpstreamPool, req *http.Request, _ http. // different request, because the order of the values is significant. vals := strings.Join(req.URL.Query()[s.Key], ",") if vals == "" { - return RandomSelection{}.Select(pool, req, nil) + return s.fallback.Select(pool, req, nil) } return hostByHashing(pool, vals) } @@ -410,6 +428,24 @@ func (s *QueryHashSelection) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { } s.Key = d.Val() } + for nesting := d.Nesting(); d.NextBlock(nesting); { + switch d.Val() { + case "fallback": + if !d.NextArg() { + return d.ArgErr() + } + if s.FallbackRaw != nil { + return d.Err("fallback selection policy already specified") + } + mod, err := loadFallbackPolicy(d) + if err != nil { + return err + } + s.FallbackRaw = mod + default: + return d.Errf("unrecognized option '%s'", d.Val()) + } + } return nil } @@ -418,6 +454,10 @@ func (s *QueryHashSelection) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { type HeaderHashSelection struct { // The HTTP header field whose value is to be hashed and used for upstream selection. Field string `json:"field,omitempty"` + + // The fallback policy to use if the header is not present. Defaults to `random`. + FallbackRaw json.RawMessage `json:"fallback,omitempty" caddy:"namespace=http.reverse_proxy.selection_policies inline_key=policy"` + fallback Selector } // CaddyModule returns the Caddy module information. @@ -428,12 +468,24 @@ func (HeaderHashSelection) CaddyModule() caddy.ModuleInfo { } } -// Select returns an available host, if any. -func (s HeaderHashSelection) Select(pool UpstreamPool, req *http.Request, _ http.ResponseWriter) *Upstream { +// Provision sets up the module. +func (s *HeaderHashSelection) Provision(ctx caddy.Context) error { if s.Field == "" { - return nil + return fmt.Errorf("header field is required") + } + if s.FallbackRaw == nil { + s.FallbackRaw = caddyconfig.JSONModuleObject(RandomSelection{}, "policy", "random", nil) } + mod, err := ctx.LoadModule(s, "FallbackRaw") + if err != nil { + return fmt.Errorf("loading fallback selection policy: %s", err) + } + s.fallback = mod.(Selector) + return nil +} +// Select returns an available host, if any. +func (s HeaderHashSelection) Select(pool UpstreamPool, req *http.Request, _ http.ResponseWriter) *Upstream { // The Host header should be obtained from the req.Host field // since net/http removes it from the header map. if s.Field == "Host" && req.Host != "" { @@ -442,7 +494,7 @@ func (s HeaderHashSelection) Select(pool UpstreamPool, req *http.Request, _ http val := req.Header.Get(s.Field) if val == "" { - return RandomSelection{}.Select(pool, req, nil) + return s.fallback.Select(pool, req, nil) } return hostByHashing(pool, val) } @@ -455,6 +507,24 @@ func (s *HeaderHashSelection) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { } s.Field = d.Val() } + for nesting := d.Nesting(); d.NextBlock(nesting); { + switch d.Val() { + case "fallback": + if !d.NextArg() { + return d.ArgErr() + } + if s.FallbackRaw != nil { + return d.Err("fallback selection policy already specified") + } + mod, err := loadFallbackPolicy(d) + if err != nil { + return err + } + s.FallbackRaw = mod + default: + return d.Errf("unrecognized option '%s'", d.Val()) + } + } return nil } @@ -465,6 +535,10 @@ type CookieHashSelection struct { Name string `json:"name,omitempty"` // Secret to hash (Hmac256) chosen upstream in cookie Secret string `json:"secret,omitempty"` + + // The fallback policy to use if the cookie is not present. Defaults to `random`. + FallbackRaw json.RawMessage `json:"fallback,omitempty" caddy:"namespace=http.reverse_proxy.selection_policies inline_key=policy"` + fallback Selector } // CaddyModule returns the Caddy module information. @@ -475,15 +549,48 @@ func (CookieHashSelection) CaddyModule() caddy.ModuleInfo { } } -// Select returns an available host, if any. -func (s CookieHashSelection) Select(pool UpstreamPool, req *http.Request, w http.ResponseWriter) *Upstream { +// Provision sets up the module. +func (s *CookieHashSelection) Provision(ctx caddy.Context) error { if s.Name == "" { s.Name = "lb" } + if s.FallbackRaw == nil { + s.FallbackRaw = caddyconfig.JSONModuleObject(RandomSelection{}, "policy", "random", nil) + } + mod, err := ctx.LoadModule(s, "FallbackRaw") + if err != nil { + return fmt.Errorf("loading fallback selection policy: %s", err) + } + s.fallback = mod.(Selector) + return nil +} + +// Select returns an available host, if any. +func (s CookieHashSelection) Select(pool UpstreamPool, req *http.Request, w http.ResponseWriter) *Upstream { + // selects a new Host using the fallback policy (typically random) + // and write a sticky session cookie to the response. + selectNewHost := func() *Upstream { + upstream := s.fallback.Select(pool, req, w) + if upstream == nil { + return nil + } + sha, err := hashCookie(s.Secret, upstream.Dial) + if err != nil { + return upstream + } + http.SetCookie(w, &http.Cookie{ + Name: s.Name, + Value: sha, + Path: "/", + Secure: false, + }) + return upstream + } + cookie, err := req.Cookie(s.Name) - // If there's no cookie, select new random host + // If there's no cookie, select a host using the fallback policy if err != nil || cookie == nil { - return selectNewHostWithCookieHashSelection(pool, w, s.Secret, s.Name) + return selectNewHost() } // If the cookie is present, loop over the available upstreams until we find a match cookieValue := cookie.Value @@ -496,13 +603,15 @@ func (s CookieHashSelection) Select(pool UpstreamPool, req *http.Request, w http return upstream } } - // If there is no matching host, select new random host - return selectNewHostWithCookieHashSelection(pool, w, s.Secret, s.Name) + // If there is no matching host, select a host using the fallback policy + return selectNewHost() } // UnmarshalCaddyfile sets up the module from Caddyfile tokens. Syntax: // -// lb_policy cookie [ []] +// lb_policy cookie [ []] { +// fallback +// } // // By default name is `lb` func (s *CookieHashSelection) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { @@ -517,22 +626,25 @@ func (s *CookieHashSelection) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { default: return d.ArgErr() } - return nil -} - -// Select a new Host randomly and add a sticky session cookie -func selectNewHostWithCookieHashSelection(pool []*Upstream, w http.ResponseWriter, cookieSecret string, cookieName string) *Upstream { - randomHost := selectRandomHost(pool) - - if randomHost != nil { - // Hash (HMAC with some key for privacy) the upstream.Dial string as the cookie value - sha, err := hashCookie(cookieSecret, randomHost.Dial) - if err == nil { - // write the cookie. - http.SetCookie(w, &http.Cookie{Name: cookieName, Value: sha, Path: "/", Secure: false}) + for nesting := d.Nesting(); d.NextBlock(nesting); { + switch d.Val() { + case "fallback": + if !d.NextArg() { + return d.ArgErr() + } + if s.FallbackRaw != nil { + return d.Err("fallback selection policy already specified") + } + mod, err := loadFallbackPolicy(d) + if err != nil { + return err + } + s.FallbackRaw = mod + default: + return d.Errf("unrecognized option '%s'", d.Val()) } } - return randomHost + return nil } // hashCookie hashes (HMAC 256) some data with the secret @@ -627,6 +739,20 @@ func hash(s string) uint32 { return h.Sum32() } +func loadFallbackPolicy(d *caddyfile.Dispenser) (json.RawMessage, error) { + name := d.Val() + modID := "http.reverse_proxy.selection_policies." + name + unm, err := caddyfile.UnmarshalModule(d, modID) + if err != nil { + return nil, err + } + sel, ok := unm.(Selector) + if !ok { + return nil, d.Errf("module %s (%T) is not a reverseproxy.Selector", modID, unm) + } + return caddyconfig.JSONModuleObject(sel, "policy", name, nil), nil +} + // Interface guards var ( _ Selector = (*RandomSelection)(nil) diff --git a/modules/caddyhttp/reverseproxy/selectionpolicies_test.go b/modules/caddyhttp/reverseproxy/selectionpolicies_test.go index d2b7b3d..93dcb77 100644 --- a/modules/caddyhttp/reverseproxy/selectionpolicies_test.go +++ b/modules/caddyhttp/reverseproxy/selectionpolicies_test.go @@ -20,6 +20,8 @@ import ( "net/http/httptest" "testing" + "github.com/caddyserver/caddy/v2" + "github.com/caddyserver/caddy/v2/caddyconfig" "github.com/caddyserver/caddy/v2/modules/caddyhttp" ) @@ -33,7 +35,7 @@ func testPool() UpstreamPool { func TestRoundRobinPolicy(t *testing.T) { pool := testPool() - rrPolicy := new(RoundRobinSelection) + rrPolicy := RoundRobinSelection{} req, _ := http.NewRequest("GET", "/", nil) h := rrPolicy.Select(pool, req, nil) @@ -74,7 +76,7 @@ func TestRoundRobinPolicy(t *testing.T) { func TestLeastConnPolicy(t *testing.T) { pool := testPool() - lcPolicy := new(LeastConnSelection) + lcPolicy := LeastConnSelection{} req, _ := http.NewRequest("GET", "/", nil) pool[0].countRequest(10) @@ -92,7 +94,7 @@ func TestLeastConnPolicy(t *testing.T) { func TestIPHashPolicy(t *testing.T) { pool := testPool() - ipHash := new(IPHashSelection) + ipHash := IPHashSelection{} req, _ := http.NewRequest("GET", "/", nil) // We should be able to predict where every request is routed. @@ -234,7 +236,7 @@ func TestIPHashPolicy(t *testing.T) { func TestClientIPHashPolicy(t *testing.T) { pool := testPool() - ipHash := new(ClientIPHashSelection) + ipHash := ClientIPHashSelection{} req, _ := http.NewRequest("GET", "/", nil) req = req.WithContext(context.WithValue(req.Context(), caddyhttp.VarsCtxKey, make(map[string]any))) @@ -377,7 +379,7 @@ func TestClientIPHashPolicy(t *testing.T) { func TestFirstPolicy(t *testing.T) { pool := testPool() - firstPolicy := new(FirstSelection) + firstPolicy := FirstSelection{} req := httptest.NewRequest(http.MethodGet, "/", nil) h := firstPolicy.Select(pool, req, nil) @@ -393,8 +395,15 @@ func TestFirstPolicy(t *testing.T) { } func TestQueryHashPolicy(t *testing.T) { - pool := testPool() + ctx, cancel := caddy.NewContext(caddy.Context{Context: context.Background()}) + defer cancel() queryPolicy := QueryHashSelection{Key: "foo"} + if err := queryPolicy.Provision(ctx); err != nil { + t.Errorf("Provision error: %v", err) + t.FailNow() + } + + pool := testPool() request := httptest.NewRequest(http.MethodGet, "/?foo=1", nil) h := queryPolicy.Select(pool, request, nil) @@ -463,7 +472,7 @@ func TestQueryHashPolicy(t *testing.T) { func TestURIHashPolicy(t *testing.T) { pool := testPool() - uriPolicy := new(URIHashSelection) + uriPolicy := URIHashSelection{} request := httptest.NewRequest(http.MethodGet, "/test", nil) h := uriPolicy.Select(pool, request, nil) @@ -552,8 +561,7 @@ func TestRandomChoicePolicy(t *testing.T) { pool[2].countRequest(30) request := httptest.NewRequest(http.MethodGet, "/test", nil) - randomChoicePolicy := new(RandomChoiceSelection) - randomChoicePolicy.Choose = 2 + randomChoicePolicy := RandomChoiceSelection{Choose: 2} h := randomChoicePolicy.Select(pool, request, nil) @@ -568,6 +576,14 @@ func TestRandomChoicePolicy(t *testing.T) { } func TestCookieHashPolicy(t *testing.T) { + ctx, cancel := caddy.NewContext(caddy.Context{Context: context.Background()}) + defer cancel() + cookieHashPolicy := CookieHashSelection{} + if err := cookieHashPolicy.Provision(ctx); err != nil { + t.Errorf("Provision error: %v", err) + t.FailNow() + } + pool := testPool() pool[0].Dial = "localhost:8080" pool[1].Dial = "localhost:8081" @@ -577,7 +593,7 @@ func TestCookieHashPolicy(t *testing.T) { pool[2].setHealthy(false) request := httptest.NewRequest(http.MethodGet, "/test", nil) w := httptest.NewRecorder() - cookieHashPolicy := new(CookieHashSelection) + h := cookieHashPolicy.Select(pool, request, w) cookieServer1 := w.Result().Cookies()[0] if cookieServer1 == nil { @@ -614,3 +630,59 @@ func TestCookieHashPolicy(t *testing.T) { t.Error("Expected cookieHashPolicy to set a new cookie.") } } + +func TestCookieHashPolicyWithFirstFallback(t *testing.T) { + ctx, cancel := caddy.NewContext(caddy.Context{Context: context.Background()}) + defer cancel() + cookieHashPolicy := CookieHashSelection{ + FallbackRaw: caddyconfig.JSONModuleObject(FirstSelection{}, "policy", "first", nil), + } + if err := cookieHashPolicy.Provision(ctx); err != nil { + t.Errorf("Provision error: %v", err) + t.FailNow() + } + + pool := testPool() + pool[0].Dial = "localhost:8080" + pool[1].Dial = "localhost:8081" + pool[2].Dial = "localhost:8082" + pool[0].setHealthy(true) + pool[1].setHealthy(true) + pool[2].setHealthy(true) + request := httptest.NewRequest(http.MethodGet, "/test", nil) + w := httptest.NewRecorder() + + h := cookieHashPolicy.Select(pool, request, w) + cookieServer1 := w.Result().Cookies()[0] + if cookieServer1 == nil { + t.Fatal("cookieHashPolicy should set a cookie") + } + if cookieServer1.Name != "lb" { + t.Error("cookieHashPolicy should set a cookie with name lb") + } + if h != pool[0] { + t.Errorf("Expected cookieHashPolicy host to be the first only available host, got %s", h) + } + request = httptest.NewRequest(http.MethodGet, "/test", nil) + w = httptest.NewRecorder() + request.AddCookie(cookieServer1) + h = cookieHashPolicy.Select(pool, request, w) + if h != pool[0] { + t.Errorf("Expected cookieHashPolicy host to stick to the first host (matching cookie), got %s", h) + } + s := w.Result().Cookies() + if len(s) != 0 { + t.Error("Expected cookieHashPolicy to not set a new cookie.") + } + pool[0].setHealthy(false) + request = httptest.NewRequest(http.MethodGet, "/test", nil) + w = httptest.NewRecorder() + request.AddCookie(cookieServer1) + h = cookieHashPolicy.Select(pool, request, w) + if h != pool[1] { + t.Errorf("Expected cookieHashPolicy to select the next first available host, got %s", h) + } + if w.Result().Cookies() == nil { + t.Error("Expected cookieHashPolicy to set a new cookie.") + } +} -- 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/healthchecks.go | 73 +++++++++++++++++++++++++- modules/caddyhttp/reverseproxy/reverseproxy.go | 54 +++---------------- 2 files changed, 77 insertions(+), 50 deletions(-) (limited to 'modules/caddyhttp/reverseproxy') 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() } } -- cgit v1.2.3 From b19946f6af75a00bb1f3d094a903224541a70fcd Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Fri, 5 May 2023 22:53:48 -0400 Subject: reverseproxy: Optimize base case for least_conn and random_choose policies (#5487) When only a single request has the least amount of requests, there's no need to compute a random number, because the modulo of 1 will always be 0 anyways. --- modules/caddyhttp/reverseproxy/selectionpolicies.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/selectionpolicies.go b/modules/caddyhttp/reverseproxy/selectionpolicies.go index a2985f1..35fb143 100644 --- a/modules/caddyhttp/reverseproxy/selectionpolicies.go +++ b/modules/caddyhttp/reverseproxy/selectionpolicies.go @@ -187,7 +187,7 @@ func (LeastConnSelection) Select(pool UpstreamPool, _ *http.Request, _ http.Resp // sample: https://en.wikipedia.org/wiki/Reservoir_sampling if numReqs == leastReqs { count++ - if (weakrand.Int() % count) == 0 { //nolint:gosec + if count > 1 || (weakrand.Int()%count) == 0 { //nolint:gosec bestHost = host } } @@ -707,6 +707,9 @@ func leastRequests(upstreams []*Upstream) *Upstream { if len(best) == 0 { return nil } + if len(best) == 1 { + return best[0] + } return best[weakrand.Intn(len(best))] //nolint:gosec } -- cgit v1.2.3 From 75b690d248c7681dd974f6179c98a363af417a25 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Mon, 15 May 2023 14:14:50 -0400 Subject: reverseproxy: Expand port ranges to multiple upstreams in CLI + Caddyfile (#5494) * reverseproxy: Expand port ranges to multiple upstreams in CLI + Caddyfile * Add clarifying comment --- modules/caddyhttp/reverseproxy/addresses.go | 41 +++++++++++++++++------- modules/caddyhttp/reverseproxy/addresses_test.go | 38 ++++++++++++++++++++++ modules/caddyhttp/reverseproxy/caddyfile.go | 21 +++++++++++- modules/caddyhttp/reverseproxy/command.go | 21 ++++++++++-- 4 files changed, 106 insertions(+), 15 deletions(-) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/addresses.go b/modules/caddyhttp/reverseproxy/addresses.go index 8152108..6078f11 100644 --- a/modules/caddyhttp/reverseproxy/addresses.go +++ b/modules/caddyhttp/reverseproxy/addresses.go @@ -40,7 +40,29 @@ func parseUpstreamDialAddress(upstreamAddr string) (string, string, error) { toURL, err := url.Parse(upstreamAddr) if err != nil { - return "", "", fmt.Errorf("parsing upstream URL: %v", err) + // if the error seems to be due to a port range, + // try to replace the port range with a dummy + // single port so that url.Parse() will succeed + if strings.Contains(err.Error(), "invalid port") && strings.Contains(err.Error(), "-") { + index := strings.LastIndex(upstreamAddr, ":") + if index == -1 { + return "", "", fmt.Errorf("parsing upstream URL: %v", err) + } + portRange := upstreamAddr[index+1:] + if strings.Count(portRange, "-") != 1 { + return "", "", fmt.Errorf("parsing upstream URL: parse \"%v\": port range invalid: %v", upstreamAddr, portRange) + } + toURL, err = url.Parse(strings.ReplaceAll(upstreamAddr, portRange, "0")) + if err != nil { + return "", "", fmt.Errorf("parsing upstream URL: %v", err) + } + port = portRange + } else { + return "", "", fmt.Errorf("parsing upstream URL: %v", err) + } + } + if port == "" { + port = toURL.Port() } // there is currently no way to perform a URL rewrite between choosing @@ -51,30 +73,27 @@ func parseUpstreamDialAddress(upstreamAddr string) (string, string, error) { } // ensure the port and scheme aren't in conflict - urlPort := toURL.Port() - if toURL.Scheme == "http" && urlPort == "443" { + if toURL.Scheme == "http" && port == "443" { return "", "", fmt.Errorf("upstream address has conflicting scheme (http://) and port (:443, the HTTPS port)") } - if toURL.Scheme == "https" && urlPort == "80" { + if toURL.Scheme == "https" && port == "80" { return "", "", fmt.Errorf("upstream address has conflicting scheme (https://) and port (:80, the HTTP port)") } - if toURL.Scheme == "h2c" && urlPort == "443" { + if toURL.Scheme == "h2c" && port == "443" { return "", "", fmt.Errorf("upstream address has conflicting scheme (h2c://) and port (:443, the HTTPS port)") } // if port is missing, attempt to infer from scheme - if toURL.Port() == "" { - var toPort string + if port == "" { switch toURL.Scheme { case "", "http", "h2c": - toPort = "80" + port = "80" case "https": - toPort = "443" + port = "443" } - toURL.Host = net.JoinHostPort(toURL.Hostname(), toPort) } - scheme, host, port = toURL.Scheme, toURL.Hostname(), toURL.Port() + scheme, host = toURL.Scheme, toURL.Hostname() } else { var err error network, host, port, err = caddy.SplitNetworkAddress(upstreamAddr) diff --git a/modules/caddyhttp/reverseproxy/addresses_test.go b/modules/caddyhttp/reverseproxy/addresses_test.go index 6355c75..0c7ad7b 100644 --- a/modules/caddyhttp/reverseproxy/addresses_test.go +++ b/modules/caddyhttp/reverseproxy/addresses_test.go @@ -149,6 +149,24 @@ func TestParseUpstreamDialAddress(t *testing.T) { expectHostPort: "[::1]:1234", expectScheme: "h2c", }, + { + input: "localhost:1001-1009", + expectHostPort: "localhost:1001-1009", + }, + { + input: "{host}:1001-1009", + expectHostPort: "{host}:1001-1009", + }, + { + input: "http://localhost:1001-1009", + expectHostPort: "localhost:1001-1009", + expectScheme: "http", + }, + { + input: "https://localhost:1001-1009", + expectHostPort: "localhost:1001-1009", + expectScheme: "https", + }, { input: "unix//var/php.sock", expectHostPort: "unix//var/php.sock", @@ -196,6 +214,26 @@ func TestParseUpstreamDialAddress(t *testing.T) { input: "http://localhost#fragment", expectErr: true, }, + { + input: "http://localhost:8001-8002-8003", + expectErr: true, + }, + { + input: "http://localhost:8001-8002/foo:bar", + expectErr: true, + }, + { + input: "http://localhost:8001-8002/foo:1", + expectErr: true, + }, + { + input: "http://localhost:8001-8002/foo:1-2", + expectErr: true, + }, + { + input: "http://localhost:8001-8002#foo:1", + expectErr: true, + }, { input: "http://foo:443", expectErr: true, diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go index 728bc2f..a79bd09 100644 --- a/modules/caddyhttp/reverseproxy/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/caddyfile.go @@ -15,6 +15,7 @@ package reverseproxy import ( + "fmt" "net/http" "reflect" "strconv" @@ -157,7 +158,25 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { } commonScheme = scheme - h.Upstreams = append(h.Upstreams, &Upstream{Dial: dialAddr}) + parsedAddr, err := caddy.ParseNetworkAddress(dialAddr) + if err != nil { + return d.WrapErr(err) + } + + if parsedAddr.StartPort == 0 && parsedAddr.EndPort == 0 { + // unix networks don't have ports + h.Upstreams = append(h.Upstreams, &Upstream{ + Dial: dialAddr, + }) + } else { + // expand a port range into multiple upstreams + for i := parsedAddr.StartPort; i <= parsedAddr.EndPort; i++ { + h.Upstreams = append(h.Upstreams, &Upstream{ + Dial: caddy.JoinNetworkAddress("", parsedAddr.Host, fmt.Sprint(i)), + }) + } + } + return nil } diff --git a/modules/caddyhttp/reverseproxy/command.go b/modules/caddyhttp/reverseproxy/command.go index bd3efcd..48fabd5 100644 --- a/modules/caddyhttp/reverseproxy/command.go +++ b/modules/caddyhttp/reverseproxy/command.go @@ -153,9 +153,24 @@ func cmdReverseProxy(fs caddycmd.Flags) (int, error) { upstreamPool := UpstreamPool{} for _, toAddr := range toAddresses { - upstreamPool = append(upstreamPool, &Upstream{ - Dial: toAddr, - }) + parsedAddr, err := caddy.ParseNetworkAddress(toAddr) + if err != nil { + return caddy.ExitCodeFailedStartup, fmt.Errorf("invalid upstream address %s: %v", toAddr, err) + } + + if parsedAddr.StartPort == 0 && parsedAddr.EndPort == 0 { + // unix networks don't have ports + upstreamPool = append(upstreamPool, &Upstream{ + Dial: toAddr, + }) + } else { + // expand a port range into multiple upstreams + for i := parsedAddr.StartPort; i <= parsedAddr.EndPort; i++ { + upstreamPool = append(upstreamPool, &Upstream{ + Dial: caddy.JoinNetworkAddress("", parsedAddr.Host, fmt.Sprint(i)), + }) + } + } } handler := Handler{ -- 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') 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/caddyfile.go | 34 +++++++++- modules/caddyhttp/reverseproxy/reverseproxy.go | 44 ++++++------- modules/caddyhttp/reverseproxy/streaming.go | 89 +++++++++++++++++++++++--- 3 files changed, 133 insertions(+), 34 deletions(-) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go index a79bd09..26dd55c 100644 --- a/modules/caddyhttp/reverseproxy/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/caddyfile.go @@ -83,10 +83,12 @@ func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error) // unhealthy_request_count // // # streaming -// flush_interval +// flush_interval // buffer_requests // buffer_responses -// max_buffer_size +// max_buffer_size +// stream_timeout +// stream_close_delay // // # request manipulation // trusted_proxies [private_ranges] @@ -571,6 +573,34 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { caddy.Log().Named("config.adapter.caddyfile").Warn("DEPRECATED: max_buffer_size: use request_buffers and/or response_buffers instead (with maximum buffer sizes)") h.DeprecatedMaxBufferSize = int64(size) + case "stream_timeout": + if !d.NextArg() { + return d.ArgErr() + } + if fi, err := strconv.Atoi(d.Val()); err == nil { + h.StreamTimeout = caddy.Duration(fi) + } else { + dur, err := caddy.ParseDuration(d.Val()) + if err != nil { + return d.Errf("bad duration value '%s': %v", d.Val(), err) + } + h.StreamTimeout = caddy.Duration(dur) + } + + case "stream_close_delay": + if !d.NextArg() { + return d.ArgErr() + } + if fi, err := strconv.Atoi(d.Val()); err == nil { + h.StreamCloseDelay = caddy.Duration(fi) + } else { + dur, err := caddy.ParseDuration(d.Val()) + if err != nil { + return d.Errf("bad duration value '%s': %v", d.Val(), err) + } + h.StreamCloseDelay = caddy.Duration(dur) + } + case "trusted_proxies": for d.NextArg() { if d.Val() == "private_ranges" { 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 } diff --git a/modules/caddyhttp/reverseproxy/streaming.go b/modules/caddyhttp/reverseproxy/streaming.go index 1f5387e..6c1e44c 100644 --- a/modules/caddyhttp/reverseproxy/streaming.go +++ b/modules/caddyhttp/reverseproxy/streaming.go @@ -33,19 +33,19 @@ import ( "golang.org/x/net/http/httpguts" ) -func (h Handler) handleUpgradeResponse(logger *zap.Logger, rw http.ResponseWriter, req *http.Request, res *http.Response) { +func (h *Handler) handleUpgradeResponse(logger *zap.Logger, rw http.ResponseWriter, req *http.Request, res *http.Response) { reqUpType := upgradeType(req.Header) resUpType := upgradeType(res.Header) // Taken from https://github.com/golang/go/commit/5c489514bc5e61ad9b5b07bd7d8ec65d66a0512a // We know reqUpType is ASCII, it's checked by the caller. if !asciiIsPrint(resUpType) { - h.logger.Debug("backend tried to switch to invalid protocol", + logger.Debug("backend tried to switch to invalid protocol", zap.String("backend_upgrade", resUpType)) return } if !asciiEqualFold(reqUpType, resUpType) { - h.logger.Debug("backend tried to switch to unexpected protocol via Upgrade header", + logger.Debug("backend tried to switch to unexpected protocol via Upgrade header", zap.String("backend_upgrade", resUpType), zap.String("requested_upgrade", reqUpType)) return @@ -53,12 +53,12 @@ func (h Handler) handleUpgradeResponse(logger *zap.Logger, rw http.ResponseWrite hj, ok := rw.(http.Hijacker) if !ok { - h.logger.Sugar().Errorf("can't switch protocols using non-Hijacker ResponseWriter type %T", rw) + logger.Error("can't switch protocols using non-Hijacker ResponseWriter", zap.String("type", fmt.Sprintf("%T", rw))) return } backConn, ok := res.Body.(io.ReadWriteCloser) if !ok { - h.logger.Error("internal error: 101 switching protocols response with non-writable body") + logger.Error("internal error: 101 switching protocols response with non-writable body") return } @@ -83,7 +83,7 @@ func (h Handler) handleUpgradeResponse(logger *zap.Logger, rw http.ResponseWrite logger.Debug("upgrading connection") conn, brw, err := hj.Hijack() if err != nil { - h.logger.Error("hijack failed on protocol switch", zap.Error(err)) + logger.Error("hijack failed on protocol switch", zap.Error(err)) return } @@ -94,7 +94,7 @@ func (h Handler) handleUpgradeResponse(logger *zap.Logger, rw http.ResponseWrite }() if err := brw.Flush(); err != nil { - h.logger.Debug("response flush", zap.Error(err)) + logger.Debug("response flush", zap.Error(err)) return } @@ -120,10 +120,23 @@ func (h Handler) handleUpgradeResponse(logger *zap.Logger, rw http.ResponseWrite spc := switchProtocolCopier{user: conn, backend: backConn} + // setup the timeout if requested + var timeoutc <-chan time.Time + if h.StreamTimeout > 0 { + timer := time.NewTimer(time.Duration(h.StreamTimeout)) + defer timer.Stop() + timeoutc = timer.C + } + errc := make(chan error, 1) go spc.copyToBackend(errc) go spc.copyFromBackend(errc) - <-errc + select { + case err := <-errc: + logger.Debug("streaming error", zap.Error(err)) + case time := <-timeoutc: + logger.Debug("stream timed out", zap.Time("timeout", time)) + } } // flushInterval returns the p.FlushInterval value, conditionally @@ -243,10 +256,70 @@ func (h *Handler) registerConnection(conn io.ReadWriteCloser, gracefulClose func return func() { h.connectionsMu.Lock() delete(h.connections, conn) + // if there is no connection left before the connections close timer fires + if len(h.connections) == 0 && h.connectionsCloseTimer != nil { + // we release the timer that holds the reference to Handler + if (*h.connectionsCloseTimer).Stop() { + h.logger.Debug("stopped streaming connections close timer - all connections are already closed") + } + h.connectionsCloseTimer = nil + } h.connectionsMu.Unlock() } } +// closeConnections immediately closes all hijacked connections (both to client and backend). +func (h *Handler) closeConnections() error { + var err error + h.connectionsMu.Lock() + defer h.connectionsMu.Unlock() + + 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 + } + } + return err +} + +// cleanupConnections closes hijacked connections. +// Depending on the value of StreamCloseDelay it does that either immediately +// or sets up a timer that will do that later. +func (h *Handler) cleanupConnections() error { + if h.StreamCloseDelay == 0 { + return h.closeConnections() + } + + h.connectionsMu.Lock() + defer h.connectionsMu.Unlock() + // the handler is shut down, no new connection can appear, + // so we can skip setting up the timer when there are no connections + if len(h.connections) > 0 { + delay := time.Duration(h.StreamCloseDelay) + h.connectionsCloseTimer = time.AfterFunc(delay, func() { + h.logger.Debug("closing streaming connections after delay", + zap.Duration("delay", delay)) + err := h.closeConnections() + if err != nil { + h.logger.Error("failed to closed connections after delay", + zap.Error(err), + zap.Duration("delay", delay)) + } + }) + } + return nil +} + // writeCloseControl sends a best-effort Close control message to the given // WebSocket connection. Thanks to @pascaldekloe who provided inspiration // from his simple implementation of this I was able to learn from at: -- cgit v1.2.3 From 361946eb0c08791ad16ebc3e82a79512895e650f Mon Sep 17 00:00:00 2001 From: Saber Haj Rabiee Date: Tue, 20 Jun 2023 10:42:58 -0700 Subject: reverseproxy: weighted_round_robin load balancing policy (#5579) * added weighted round robin algorithm to load balancer * added an adapt integration test for wrr and fixed a typo * changed args format to Caddyfile args convention * added provisioner and validator for wrr * simplified the code and improved doc --- .../caddyhttp/reverseproxy/selectionpolicies.go | 91 +++++++++++++++++++++- .../reverseproxy/selectionpolicies_test.go | 57 ++++++++++++++ 2 files changed, 147 insertions(+), 1 deletion(-) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/selectionpolicies.go b/modules/caddyhttp/reverseproxy/selectionpolicies.go index 35fb143..f89c48f 100644 --- a/modules/caddyhttp/reverseproxy/selectionpolicies.go +++ b/modules/caddyhttp/reverseproxy/selectionpolicies.go @@ -40,6 +40,7 @@ func init() { caddy.RegisterModule(RandomChoiceSelection{}) caddy.RegisterModule(LeastConnSelection{}) caddy.RegisterModule(RoundRobinSelection{}) + caddy.RegisterModule(WeightedRoundRobinSelection{}) caddy.RegisterModule(FirstSelection{}) caddy.RegisterModule(IPHashSelection{}) caddy.RegisterModule(ClientIPHashSelection{}) @@ -78,6 +79,90 @@ func (r *RandomSelection) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { return nil } +// WeightedRoundRobinSelection is a policy that selects +// a host based on weighted round-robin ordering. +type WeightedRoundRobinSelection struct { + // The weight of each upstream in order, + // corresponding with the list of upstreams configured. + Weights []int `json:"weights,omitempty"` + index uint32 + totalWeight int +} + +// CaddyModule returns the Caddy module information. +func (WeightedRoundRobinSelection) CaddyModule() caddy.ModuleInfo { + return caddy.ModuleInfo{ + ID: "http.reverse_proxy.selection_policies.weighted_round_robin", + New: func() caddy.Module { + return new(WeightedRoundRobinSelection) + }, + } +} + +// UnmarshalCaddyfile sets up the module from Caddyfile tokens. +func (r *WeightedRoundRobinSelection) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { + for d.Next() { + args := d.RemainingArgs() + if len(args) == 0 { + return d.ArgErr() + } + + for _, weight := range args { + weightInt, err := strconv.Atoi(weight) + if err != nil { + return d.Errf("invalid weight value '%s': %v", weight, err) + } + if weightInt < 1 { + return d.Errf("invalid weight value '%s': weight should be non-zero and positive", weight) + } + r.Weights = append(r.Weights, weightInt) + } + } + return nil +} + +// Provision sets up r. +func (r *WeightedRoundRobinSelection) Provision(ctx caddy.Context) error { + for _, weight := range r.Weights { + r.totalWeight += weight + } + return nil +} + +// Select returns an available host, if any. +func (r *WeightedRoundRobinSelection) Select(pool UpstreamPool, _ *http.Request, _ http.ResponseWriter) *Upstream { + if len(pool) == 0 { + return nil + } + if len(r.Weights) < 2 { + return pool[0] + } + var index, totalWeight int + currentWeight := int(atomic.AddUint32(&r.index, 1)) % r.totalWeight + for i, weight := range r.Weights { + totalWeight += weight + if currentWeight < totalWeight { + index = i + break + } + } + + upstreams := make([]*Upstream, 0, len(r.Weights)) + for _, upstream := range pool { + if !upstream.Available() { + continue + } + upstreams = append(upstreams, upstream) + if len(upstreams) == cap(upstreams) { + break + } + } + if len(upstreams) == 0 { + return nil + } + return upstreams[index%len(upstreams)] +} + // RandomChoiceSelection is a policy that selects // two or more available hosts at random, then // chooses the one with the least load. @@ -762,6 +847,7 @@ var ( _ Selector = (*RandomChoiceSelection)(nil) _ Selector = (*LeastConnSelection)(nil) _ Selector = (*RoundRobinSelection)(nil) + _ Selector = (*WeightedRoundRobinSelection)(nil) _ Selector = (*FirstSelection)(nil) _ Selector = (*IPHashSelection)(nil) _ Selector = (*ClientIPHashSelection)(nil) @@ -770,8 +856,11 @@ var ( _ Selector = (*HeaderHashSelection)(nil) _ Selector = (*CookieHashSelection)(nil) - _ caddy.Validator = (*RandomChoiceSelection)(nil) + _ caddy.Validator = (*RandomChoiceSelection)(nil) + _ caddy.Provisioner = (*RandomChoiceSelection)(nil) + _ caddy.Provisioner = (*WeightedRoundRobinSelection)(nil) _ caddyfile.Unmarshaler = (*RandomChoiceSelection)(nil) + _ caddyfile.Unmarshaler = (*WeightedRoundRobinSelection)(nil) ) diff --git a/modules/caddyhttp/reverseproxy/selectionpolicies_test.go b/modules/caddyhttp/reverseproxy/selectionpolicies_test.go index 93dcb77..dc613a5 100644 --- a/modules/caddyhttp/reverseproxy/selectionpolicies_test.go +++ b/modules/caddyhttp/reverseproxy/selectionpolicies_test.go @@ -74,6 +74,63 @@ func TestRoundRobinPolicy(t *testing.T) { } } +func TestWeightedRoundRobinPolicy(t *testing.T) { + pool := testPool() + wrrPolicy := WeightedRoundRobinSelection{ + Weights: []int{3, 2, 1}, + totalWeight: 6, + } + req, _ := http.NewRequest("GET", "/", nil) + + h := wrrPolicy.Select(pool, req, nil) + if h != pool[0] { + t.Error("Expected first weighted round robin host to be first host in the pool.") + } + h = wrrPolicy.Select(pool, req, nil) + if h != pool[0] { + t.Error("Expected second weighted round robin host to be first host in the pool.") + } + // Third selected host is 1, because counter starts at 0 + // and increments before host is selected + h = wrrPolicy.Select(pool, req, nil) + if h != pool[1] { + t.Error("Expected third weighted round robin host to be second host in the pool.") + } + h = wrrPolicy.Select(pool, req, nil) + if h != pool[1] { + t.Error("Expected fourth weighted round robin host to be second host in the pool.") + } + h = wrrPolicy.Select(pool, req, nil) + if h != pool[2] { + t.Error("Expected fifth weighted round robin host to be third host in the pool.") + } + h = wrrPolicy.Select(pool, req, nil) + if h != pool[0] { + t.Error("Expected sixth weighted round robin host to be first host in the pool.") + } + + // mark host as down + pool[0].setHealthy(false) + h = wrrPolicy.Select(pool, req, nil) + if h != pool[1] { + t.Error("Expected to skip down host.") + } + // mark host as up + pool[0].setHealthy(true) + + h = wrrPolicy.Select(pool, req, nil) + if h != pool[0] { + t.Error("Expected to select first host on availablity.") + } + // mark host as full + pool[1].countRequest(1) + pool[1].MaxRequests = 1 + h = wrrPolicy.Select(pool, req, nil) + if h != pool[2] { + t.Error("Expected to skip full host.") + } +} + func TestLeastConnPolicy(t *testing.T) { pool := testPool() lcPolicy := LeastConnSelection{} -- cgit v1.2.3 From 7a69ae757197660d26095045fba385c613926d77 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Thu, 22 Jun 2023 18:20:30 -0400 Subject: reverseproxy: Honor `tls_except_port` for active health checks (#5591) --- modules/caddyhttp/reverseproxy/healthchecks.go | 41 ++++++++++++++++---------- 1 file changed, 25 insertions(+), 16 deletions(-) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/healthchecks.go b/modules/caddyhttp/reverseproxy/healthchecks.go index c969c8c..80b635a 100644 --- a/modules/caddyhttp/reverseproxy/healthchecks.go +++ b/modules/caddyhttp/reverseproxy/healthchecks.go @@ -306,16 +306,35 @@ func (h *Handler) doActiveHealthCheckForAllHosts() { // the host's health status fails. func (h *Handler) doActiveHealthCheck(dialInfo DialInfo, hostAddr string, upstream *Upstream) error { // create the URL for the request that acts as a health check - scheme := "http" - if ht, ok := h.Transport.(TLSTransport); ok && ht.TLSEnabled() { - // this is kind of a hacky way to know if we should use HTTPS, but whatever - scheme = "https" - } u := &url.URL{ - Scheme: scheme, + Scheme: "http", Host: hostAddr, } + // split the host and port if possible, override the port if configured + host, port, err := net.SplitHostPort(hostAddr) + if err != nil { + host = hostAddr + } + if h.HealthChecks.Active.Port != 0 { + port := strconv.Itoa(h.HealthChecks.Active.Port) + u.Host = net.JoinHostPort(host, port) + } + + // this is kind of a hacky way to know if we should use HTTPS, but whatever + if tt, ok := h.Transport.(TLSTransport); ok && tt.TLSEnabled() { + u.Scheme = "https" + + // if the port is in the except list, flip back to HTTP + if ht, ok := h.Transport.(*HTTPTransport); ok { + for _, exceptPort := range ht.TLS.ExceptPorts { + if exceptPort == port { + u.Scheme = "http" + } + } + } + } + // if we have a provisioned uri, use that, otherwise use // the deprecated Path option if h.HealthChecks.Active.uri != nil { @@ -325,16 +344,6 @@ func (h *Handler) doActiveHealthCheck(dialInfo DialInfo, hostAddr string, upstre u.Path = h.HealthChecks.Active.Path } - // adjust the port, if configured to be different - if h.HealthChecks.Active.Port != 0 { - portStr := strconv.Itoa(h.HealthChecks.Active.Port) - host, _, err := net.SplitHostPort(hostAddr) - if err != nil { - host = hostAddr - } - u.Host = net.JoinHostPort(host, portStr) - } - // attach dialing information to this request, as well as context values that // may be expected by handlers of this request ctx := h.ctx.Context -- cgit v1.2.3 From 5dec11f2a0e920d5466ff224a0749a667193e3ce Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Sat, 8 Jul 2023 13:42:51 -0600 Subject: reverseproxy: Pointer receiver This avoids copying the Upstream, which has an atomically-accessed value in it. --- modules/caddyhttp/reverseproxy/hosts.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/hosts.go b/modules/caddyhttp/reverseproxy/hosts.go index 298d4f3..83a39d8 100644 --- a/modules/caddyhttp/reverseproxy/hosts.go +++ b/modules/caddyhttp/reverseproxy/hosts.go @@ -63,9 +63,10 @@ type Upstream struct { unhealthy int32 // accessed atomically; status from active health checker } -func (u Upstream) String() string { - return u.Dial -} +// (pointer receiver necessary to avoid a race condition, since +// copying the Upstream reads the 'unhealthy' field which is +// accessed atomically) +func (u *Upstream) String() string { return u.Dial } // Available returns true if the remote host // is available to receive requests. This is -- cgit v1.2.3 From 0e2c7e1d35b287fc0e56d6db2951f791e09b5a37 Mon Sep 17 00:00:00 2001 From: Matt Holt Date: Tue, 11 Jul 2023 13:10:58 -0600 Subject: caddytls: Reuse certificate cache through reloads (#5623) * caddytls: Don't purge cert cache on config reload * Update CertMagic This actually avoids reloading managed certs from storage when already in the cache, d'oh. * Fix bug; re-implement HasCertificateForSubject * Update go.mod: CertMagic tag --- modules/caddyhttp/reverseproxy/httptransport.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/httptransport.go b/modules/caddyhttp/reverseproxy/httptransport.go index 1135862..8334f25 100644 --- a/modules/caddyhttp/reverseproxy/httptransport.go +++ b/modules/caddyhttp/reverseproxy/httptransport.go @@ -525,7 +525,7 @@ func (t TLSConfig) MakeTLSClientConfig(ctx caddy.Context) (*tls.Config, error) { return nil, fmt.Errorf("managing client certificate: %v", err) } cfg.GetClientCertificate = func(cri *tls.CertificateRequestInfo) (*tls.Certificate, error) { - certs := tlsApp.AllMatchingCertificates(t.ClientCertificateAutomate) + certs := caddytls.AllMatchingCertificates(t.ClientCertificateAutomate) var err error for _, cert := range certs { err = cri.SupportsCertificate(&cert.Certificate) -- 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 +++++++++------------- .../caddyhttp/reverseproxy/selectionpolicies.go | 3 -- 2 files changed, 15 insertions(+), 27 deletions(-) (limited to 'modules/caddyhttp/reverseproxy') 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 diff --git a/modules/caddyhttp/reverseproxy/selectionpolicies.go b/modules/caddyhttp/reverseproxy/selectionpolicies.go index f89c48f..bc6de35 100644 --- a/modules/caddyhttp/reverseproxy/selectionpolicies.go +++ b/modules/caddyhttp/reverseproxy/selectionpolicies.go @@ -27,7 +27,6 @@ import ( "strconv" "strings" "sync/atomic" - "time" "github.com/caddyserver/caddy/v2" "github.com/caddyserver/caddy/v2/caddyconfig" @@ -48,8 +47,6 @@ func init() { caddy.RegisterModule(QueryHashSelection{}) caddy.RegisterModule(HeaderHashSelection{}) caddy.RegisterModule(CookieHashSelection{}) - - weakrand.Seed(time.Now().UTC().UnixNano()) } // RandomSelection is a policy that selects -- cgit v1.2.3 From d7d16360d41159a3bea772c36e795bab31503897 Mon Sep 17 00:00:00 2001 From: Omar Ramadan Date: Tue, 25 Jul 2023 21:50:21 +0300 Subject: reverseproxy: Export ipVersions type (#5648) allows AUpstreams to be instantiated externally --- modules/caddyhttp/reverseproxy/caddyfile.go | 2 +- modules/caddyhttp/reverseproxy/upstreams.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go index 26dd55c..1d86beb 100644 --- a/modules/caddyhttp/reverseproxy/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/caddyfile.go @@ -1449,7 +1449,7 @@ func (u *AUpstreams) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { } if u.Versions == nil { - u.Versions = &ipVersions{} + u.Versions = &IPVersions{} } trueBool := true diff --git a/modules/caddyhttp/reverseproxy/upstreams.go b/modules/caddyhttp/reverseproxy/upstreams.go index 42fedb6..19e4f3c 100644 --- a/modules/caddyhttp/reverseproxy/upstreams.go +++ b/modules/caddyhttp/reverseproxy/upstreams.go @@ -212,7 +212,7 @@ func (sl srvLookup) isFresh() bool { return time.Since(sl.freshness) < time.Duration(sl.srvUpstreams.Refresh) } -type ipVersions struct { +type IPVersions struct { IPv4 *bool `json:"ipv4,omitempty"` IPv6 *bool `json:"ipv6,omitempty"` } @@ -247,7 +247,7 @@ type AUpstreams struct { // The IP versions to resolve for. By default, both // "ipv4" and "ipv6" will be enabled, which // correspond to A and AAAA records respectively. - Versions *ipVersions `json:"versions,omitempty"` + Versions *IPVersions `json:"versions,omitempty"` resolver *net.Resolver } -- 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') 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 +- modules/caddyhttp/reverseproxy/streaming.go | 60 ++++++++++++------------ modules/caddyhttp/reverseproxy/streaming_test.go | 6 ++- 3 files changed, 38 insertions(+), 33 deletions(-) (limited to 'modules/caddyhttp/reverseproxy') 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 diff --git a/modules/caddyhttp/reverseproxy/streaming.go b/modules/caddyhttp/reverseproxy/streaming.go index 6c1e44c..3f2489d 100644 --- a/modules/caddyhttp/reverseproxy/streaming.go +++ b/modules/caddyhttp/reverseproxy/streaming.go @@ -20,6 +20,7 @@ package reverseproxy import ( "context" + "errors" "fmt" "io" weakrand "math/rand" @@ -51,17 +52,19 @@ func (h *Handler) handleUpgradeResponse(logger *zap.Logger, rw http.ResponseWrit return } - hj, ok := rw.(http.Hijacker) - if !ok { - logger.Error("can't switch protocols using non-Hijacker ResponseWriter", zap.String("type", fmt.Sprintf("%T", rw))) - return - } backConn, ok := res.Body.(io.ReadWriteCloser) if !ok { logger.Error("internal error: 101 switching protocols response with non-writable body") return } + //nolint:bodyclose + conn, brw, hijackErr := http.NewResponseController(rw).Hijack() + if errors.Is(hijackErr, http.ErrNotSupported) { + h.logger.Sugar().Errorf("can't switch protocols using non-Hijacker ResponseWriter type %T", rw) + return + } + // adopted from https://github.com/golang/go/commit/8bcf2834afdf6a1f7937390903a41518715ef6f5 backConnCloseCh := make(chan struct{}) go func() { @@ -81,9 +84,8 @@ func (h *Handler) handleUpgradeResponse(logger *zap.Logger, rw http.ResponseWrit rw.WriteHeader(res.StatusCode) logger.Debug("upgrading connection") - conn, brw, err := hj.Hijack() - if err != nil { - logger.Error("hijack failed on protocol switch", zap.Error(err)) + if hijackErr != nil { + h.logger.Error("hijack failed on protocol switch", zap.Error(hijackErr)) return } @@ -181,26 +183,28 @@ func (h Handler) isBidirectionalStream(req *http.Request, res *http.Response) bo (ae == "identity" || ae == "") } -func (h Handler) copyResponse(dst io.Writer, src io.Reader, flushInterval time.Duration) error { +func (h Handler) copyResponse(dst http.ResponseWriter, src io.Reader, flushInterval time.Duration) error { + var w io.Writer = dst + if flushInterval != 0 { - if wf, ok := dst.(writeFlusher); ok { - mlw := &maxLatencyWriter{ - dst: wf, - latency: flushInterval, - } - defer mlw.stop() + mlw := &maxLatencyWriter{ + dst: dst, + //nolint:bodyclose + flush: http.NewResponseController(dst).Flush, + latency: flushInterval, + } + defer mlw.stop() - // set up initial timer so headers get flushed even if body writes are delayed - mlw.flushPending = true - mlw.t = time.AfterFunc(flushInterval, mlw.delayedFlush) + // set up initial timer so headers get flushed even if body writes are delayed + mlw.flushPending = true + mlw.t = time.AfterFunc(flushInterval, mlw.delayedFlush) - dst = mlw - } + w = mlw } buf := streamingBufPool.Get().(*[]byte) defer streamingBufPool.Put(buf) - _, err := h.copyBuffer(dst, src, *buf) + _, err := h.copyBuffer(w, src, *buf) return err } @@ -439,13 +443,9 @@ type openConnection struct { gracefulClose func() error } -type writeFlusher interface { - io.Writer - http.Flusher -} - type maxLatencyWriter struct { - dst writeFlusher + dst io.Writer + flush func() error latency time.Duration // non-zero; negative means to flush immediately mu sync.Mutex // protects t, flushPending, and dst.Flush @@ -458,7 +458,8 @@ func (m *maxLatencyWriter) Write(p []byte) (n int, err error) { defer m.mu.Unlock() n, err = m.dst.Write(p) if m.latency < 0 { - m.dst.Flush() + //nolint:errcheck + m.flush() return } if m.flushPending { @@ -479,7 +480,8 @@ func (m *maxLatencyWriter) delayedFlush() { if !m.flushPending { // if stop was called but AfterFunc already started this goroutine return } - m.dst.Flush() + //nolint:errcheck + m.flush() m.flushPending = false } diff --git a/modules/caddyhttp/reverseproxy/streaming_test.go b/modules/caddyhttp/reverseproxy/streaming_test.go index 4ed1f1e..919538f 100644 --- a/modules/caddyhttp/reverseproxy/streaming_test.go +++ b/modules/caddyhttp/reverseproxy/streaming_test.go @@ -2,6 +2,7 @@ package reverseproxy import ( "bytes" + "net/http/httptest" "strings" "testing" ) @@ -13,12 +14,15 @@ func TestHandlerCopyResponse(t *testing.T) { strings.Repeat("a", defaultBufferSize), strings.Repeat("123456789 123456789 123456789 12", 3000), } + dst := bytes.NewBuffer(nil) + recorder := httptest.NewRecorder() + recorder.Body = dst for _, d := range testdata { src := bytes.NewBuffer([]byte(d)) dst.Reset() - err := h.copyResponse(dst, src, 0) + err := h.copyResponse(recorder, src, 0) if err != nil { t.Errorf("failed with error: %v", err) } -- cgit v1.2.3 From e2fc08bd34cc17b8cbb6ac364fa1ec41c4c643b9 Mon Sep 17 00:00:00 2001 From: WeidiDeng Date: Thu, 3 Aug 2023 12:08:12 +0800 Subject: reverseproxy: Fix hijack ordering which broke websockets (#5679) --- modules/caddyhttp/reverseproxy/streaming.go | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/streaming.go b/modules/caddyhttp/reverseproxy/streaming.go index 3f2489d..71b9386 100644 --- a/modules/caddyhttp/reverseproxy/streaming.go +++ b/modules/caddyhttp/reverseproxy/streaming.go @@ -58,6 +58,13 @@ func (h *Handler) handleUpgradeResponse(logger *zap.Logger, rw http.ResponseWrit return } + // write header first, response headers should not be counted in size + // like the rest of handler chain. + copyHeader(rw.Header(), res.Header) + rw.WriteHeader(res.StatusCode) + + logger.Debug("upgrading connection") + //nolint:bodyclose conn, brw, hijackErr := http.NewResponseController(rw).Hijack() if errors.Is(hijackErr, http.ErrNotSupported) { @@ -65,6 +72,11 @@ func (h *Handler) handleUpgradeResponse(logger *zap.Logger, rw http.ResponseWrit return } + if hijackErr != nil { + h.logger.Error("hijack failed on protocol switch", zap.Error(hijackErr)) + return + } + // adopted from https://github.com/golang/go/commit/8bcf2834afdf6a1f7937390903a41518715ef6f5 backConnCloseCh := make(chan struct{}) go func() { @@ -78,17 +90,6 @@ func (h *Handler) handleUpgradeResponse(logger *zap.Logger, rw http.ResponseWrit }() defer close(backConnCloseCh) - // write header first, response headers should not be counted in size - // like the rest of handler chain. - copyHeader(rw.Header(), res.Header) - rw.WriteHeader(res.StatusCode) - - logger.Debug("upgrading connection") - if hijackErr != nil { - h.logger.Error("hijack failed on protocol switch", zap.Error(hijackErr)) - return - } - start := time.Now() defer func() { conn.Close() -- cgit v1.2.3 From 65e33fc1ee4798bb3450f6e291bfc88404982636 Mon Sep 17 00:00:00 2001 From: Mohammed Al Sahaf Date: Sat, 5 Aug 2023 23:30:02 +0200 Subject: reverseproxy: do not parse upstream address too early if it contains replaceble parts (#5695) * reverseproxy: do not parse upstream address too early if it contains replaceble parts * remove unused method * cleanup * accommodate partially replaceable port --- modules/caddyhttp/reverseproxy/addresses.go | 67 +++++++++++++++--------- modules/caddyhttp/reverseproxy/addresses_test.go | 10 ++-- modules/caddyhttp/reverseproxy/caddyfile.go | 20 ++++--- modules/caddyhttp/reverseproxy/command.go | 8 +-- 4 files changed, 65 insertions(+), 40 deletions(-) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/addresses.go b/modules/caddyhttp/reverseproxy/addresses.go index 6078f11..0939497 100644 --- a/modules/caddyhttp/reverseproxy/addresses.go +++ b/modules/caddyhttp/reverseproxy/addresses.go @@ -23,11 +23,43 @@ import ( "github.com/caddyserver/caddy/v2" ) +type parsedAddr struct { + network, scheme, host, port string + valid bool +} + +func (p parsedAddr) dialAddr() string { + if !p.valid { + return "" + } + // for simplest possible config, we only need to include + // the network portion if the user specified one + if p.network != "" { + return caddy.JoinNetworkAddress(p.network, p.host, p.port) + } + + // if the host is a placeholder, then we don't want to join with an empty port, + // because that would just append an extra ':' at the end of the address. + if p.port == "" && strings.Contains(p.host, "{") { + return p.host + } + return net.JoinHostPort(p.host, p.port) +} +func (p parsedAddr) rangedPort() bool { + return strings.Contains(p.port, "-") +} +func (p parsedAddr) replaceablePort() bool { + return strings.Contains(p.port, "{") && strings.Contains(p.port, "}") +} +func (p parsedAddr) isUnix() bool { + return caddy.IsUnixNetwork(p.network) +} + // parseUpstreamDialAddress parses configuration inputs for // the dial address, including support for a scheme in front // as a shortcut for the port number, and a network type, // for example 'unix' to dial a unix socket. -func parseUpstreamDialAddress(upstreamAddr string) (string, string, error) { +func parseUpstreamDialAddress(upstreamAddr string) (parsedAddr, error) { var network, scheme, host, port string if strings.Contains(upstreamAddr, "://") { @@ -35,7 +67,7 @@ func parseUpstreamDialAddress(upstreamAddr string) (string, string, error) { // so we return a more user-friendly error message instead // to explain what to do instead if strings.Contains(upstreamAddr, "{") { - return "", "", fmt.Errorf("due to parsing difficulties, placeholders are not allowed when an upstream address contains a scheme") + return parsedAddr{}, fmt.Errorf("due to parsing difficulties, placeholders are not allowed when an upstream address contains a scheme") } toURL, err := url.Parse(upstreamAddr) @@ -46,19 +78,19 @@ func parseUpstreamDialAddress(upstreamAddr string) (string, string, error) { if strings.Contains(err.Error(), "invalid port") && strings.Contains(err.Error(), "-") { index := strings.LastIndex(upstreamAddr, ":") if index == -1 { - return "", "", fmt.Errorf("parsing upstream URL: %v", err) + return parsedAddr{}, fmt.Errorf("parsing upstream URL: %v", err) } portRange := upstreamAddr[index+1:] if strings.Count(portRange, "-") != 1 { - return "", "", fmt.Errorf("parsing upstream URL: parse \"%v\": port range invalid: %v", upstreamAddr, portRange) + return parsedAddr{}, fmt.Errorf("parsing upstream URL: parse \"%v\": port range invalid: %v", upstreamAddr, portRange) } toURL, err = url.Parse(strings.ReplaceAll(upstreamAddr, portRange, "0")) if err != nil { - return "", "", fmt.Errorf("parsing upstream URL: %v", err) + return parsedAddr{}, fmt.Errorf("parsing upstream URL: %v", err) } port = portRange } else { - return "", "", fmt.Errorf("parsing upstream URL: %v", err) + return parsedAddr{}, fmt.Errorf("parsing upstream URL: %v", err) } } if port == "" { @@ -69,18 +101,18 @@ func parseUpstreamDialAddress(upstreamAddr string) (string, string, error) { // a backend and proxying to it, so we cannot allow extra components // in backend URLs if toURL.Path != "" || toURL.RawQuery != "" || toURL.Fragment != "" { - return "", "", fmt.Errorf("for now, URLs for proxy upstreams only support scheme, host, and port components") + return parsedAddr{}, fmt.Errorf("for now, URLs for proxy upstreams only support scheme, host, and port components") } // ensure the port and scheme aren't in conflict if toURL.Scheme == "http" && port == "443" { - return "", "", fmt.Errorf("upstream address has conflicting scheme (http://) and port (:443, the HTTPS port)") + return parsedAddr{}, fmt.Errorf("upstream address has conflicting scheme (http://) and port (:443, the HTTPS port)") } if toURL.Scheme == "https" && port == "80" { - return "", "", fmt.Errorf("upstream address has conflicting scheme (https://) and port (:80, the HTTP port)") + return parsedAddr{}, fmt.Errorf("upstream address has conflicting scheme (https://) and port (:80, the HTTP port)") } if toURL.Scheme == "h2c" && port == "443" { - return "", "", fmt.Errorf("upstream address has conflicting scheme (h2c://) and port (:443, the HTTPS port)") + return parsedAddr{}, fmt.Errorf("upstream address has conflicting scheme (h2c://) and port (:443, the HTTPS port)") } // if port is missing, attempt to infer from scheme @@ -112,18 +144,5 @@ func parseUpstreamDialAddress(upstreamAddr string) (string, string, error) { network = "unix" scheme = "h2c" } - - // for simplest possible config, we only need to include - // the network portion if the user specified one - if network != "" { - return caddy.JoinNetworkAddress(network, host, port), scheme, nil - } - - // if the host is a placeholder, then we don't want to join with an empty port, - // because that would just append an extra ':' at the end of the address. - if port == "" && strings.Contains(host, "{") { - return host, scheme, nil - } - - return net.JoinHostPort(host, port), scheme, nil + return parsedAddr{network, scheme, host, port, true}, nil } diff --git a/modules/caddyhttp/reverseproxy/addresses_test.go b/modules/caddyhttp/reverseproxy/addresses_test.go index 0c7ad7b..0c51419 100644 --- a/modules/caddyhttp/reverseproxy/addresses_test.go +++ b/modules/caddyhttp/reverseproxy/addresses_test.go @@ -265,18 +265,18 @@ func TestParseUpstreamDialAddress(t *testing.T) { expectScheme: "h2c", }, } { - actualHostPort, actualScheme, err := parseUpstreamDialAddress(tc.input) + actualAddr, err := parseUpstreamDialAddress(tc.input) if tc.expectErr && err == nil { t.Errorf("Test %d: Expected error but got %v", i, err) } if !tc.expectErr && err != nil { t.Errorf("Test %d: Expected no error but got %v", i, err) } - if actualHostPort != tc.expectHostPort { - t.Errorf("Test %d: Expected host and port '%s' but got '%s'", i, tc.expectHostPort, actualHostPort) + if actualAddr.dialAddr() != tc.expectHostPort { + t.Errorf("Test %d: input %s: Expected host and port '%s' but got '%s'", i, tc.input, tc.expectHostPort, actualAddr.dialAddr()) } - if actualScheme != tc.expectScheme { - t.Errorf("Test %d: Expected scheme '%s' but got '%s'", i, tc.expectScheme, actualScheme) + if actualAddr.scheme != tc.expectScheme { + t.Errorf("Test %d: Expected scheme '%s' but got '%s'", i, tc.expectScheme, actualAddr.scheme) } } } diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go index 1d86beb..f5eb7a5 100644 --- a/modules/caddyhttp/reverseproxy/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/caddyfile.go @@ -146,7 +146,7 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { // appendUpstream creates an upstream for address and adds // it to the list. appendUpstream := func(address string) error { - dialAddr, scheme, err := parseUpstreamDialAddress(address) + pa, err := parseUpstreamDialAddress(address) if err != nil { return d.WrapErr(err) } @@ -154,21 +154,27 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { // the underlying JSON does not yet support different // transports (protocols or schemes) to each backend, // so we remember the last one we see and compare them - if commonScheme != "" && scheme != commonScheme { + if commonScheme != "" && pa.scheme != commonScheme { return d.Errf("for now, all proxy upstreams must use the same scheme (transport protocol); expecting '%s://' but got '%s://'", - commonScheme, scheme) + commonScheme, pa.scheme) } - commonScheme = scheme + commonScheme = pa.scheme - parsedAddr, err := caddy.ParseNetworkAddress(dialAddr) + // if the port of upstream address contains a placeholder, only wrap it with the `Upstream` struct, + // delaying actual resolution of the address until request time. + if pa.replaceablePort() { + h.Upstreams = append(h.Upstreams, &Upstream{Dial: pa.dialAddr()}) + return nil + } + parsedAddr, err := caddy.ParseNetworkAddress(pa.dialAddr()) if err != nil { return d.WrapErr(err) } - if parsedAddr.StartPort == 0 && parsedAddr.EndPort == 0 { + if pa.isUnix() || !pa.rangedPort() { // unix networks don't have ports h.Upstreams = append(h.Upstreams, &Upstream{ - Dial: dialAddr, + Dial: pa.dialAddr(), }) } else { // expand a port range into multiple upstreams diff --git a/modules/caddyhttp/reverseproxy/command.go b/modules/caddyhttp/reverseproxy/command.go index 48fabd5..8438b72 100644 --- a/modules/caddyhttp/reverseproxy/command.go +++ b/modules/caddyhttp/reverseproxy/command.go @@ -132,14 +132,14 @@ func cmdReverseProxy(fs caddycmd.Flags) (int, error) { toAddresses := make([]string, len(to)) var toScheme string for i, toLoc := range to { - addr, scheme, err := parseUpstreamDialAddress(toLoc) + addr, err := parseUpstreamDialAddress(toLoc) if err != nil { return caddy.ExitCodeFailedStartup, fmt.Errorf("invalid upstream address %s: %v", toLoc, err) } - if scheme != "" && toScheme == "" { - toScheme = scheme + if addr.scheme != "" && toScheme == "" { + toScheme = addr.scheme } - toAddresses[i] = addr + toAddresses[i] = addr.dialAddr() } // proceed to build the handler and server -- 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/addresses.go | 3 +++ modules/caddyhttp/reverseproxy/caddyfile.go | 1 - modules/caddyhttp/reverseproxy/command.go | 3 ++- modules/caddyhttp/reverseproxy/fastcgi/client.go | 3 --- modules/caddyhttp/reverseproxy/reverseproxy.go | 3 ++- modules/caddyhttp/reverseproxy/streaming.go | 6 ++++-- 6 files changed, 11 insertions(+), 8 deletions(-) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/addresses.go b/modules/caddyhttp/reverseproxy/addresses.go index 0939497..82c1c79 100644 --- a/modules/caddyhttp/reverseproxy/addresses.go +++ b/modules/caddyhttp/reverseproxy/addresses.go @@ -45,12 +45,15 @@ func (p parsedAddr) dialAddr() string { } return net.JoinHostPort(p.host, p.port) } + func (p parsedAddr) rangedPort() bool { return strings.Contains(p.port, "-") } + func (p parsedAddr) replaceablePort() bool { return strings.Contains(p.port, "{") && strings.Contains(p.port, "}") } + func (p parsedAddr) isUnix() bool { return caddy.IsUnixNetwork(p.network) } diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go index f5eb7a5..774f889 100644 --- a/modules/caddyhttp/reverseproxy/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/caddyfile.go @@ -549,7 +549,6 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { h.RequestBuffers = int64(size) } else if subdir == "response_buffers" { h.ResponseBuffers = int64(size) - } // TODO: These three properties are deprecated; remove them sometime after v2.6.4 diff --git a/modules/caddyhttp/reverseproxy/command.go b/modules/caddyhttp/reverseproxy/command.go index 8438b72..9359f3d 100644 --- a/modules/caddyhttp/reverseproxy/command.go +++ b/modules/caddyhttp/reverseproxy/command.go @@ -284,7 +284,8 @@ func cmdReverseProxy(fs caddycmd.Flags) (int, error) { var false bool cfg := &caddy.Config{ - Admin: &caddy.AdminConfig{Disabled: true, + Admin: &caddy.AdminConfig{ + Disabled: true, Config: &caddy.ConfigSettings{ Persist: &false, }, diff --git a/modules/caddyhttp/reverseproxy/fastcgi/client.go b/modules/caddyhttp/reverseproxy/fastcgi/client.go index ae36dd8..04513dd 100644 --- a/modules/caddyhttp/reverseproxy/fastcgi/client.go +++ b/modules/caddyhttp/reverseproxy/fastcgi/client.go @@ -251,7 +251,6 @@ func (c *client) Request(p map[string]string, req io.Reader) (resp *http.Respons // Get issues a GET request to the fcgi responder. func (c *client) Get(p map[string]string, body io.Reader, l int64) (resp *http.Response, err error) { - p["REQUEST_METHOD"] = "GET" p["CONTENT_LENGTH"] = strconv.FormatInt(l, 10) @@ -260,7 +259,6 @@ func (c *client) Get(p map[string]string, body io.Reader, l int64) (resp *http.R // Head issues a HEAD request to the fcgi responder. func (c *client) Head(p map[string]string) (resp *http.Response, err error) { - p["REQUEST_METHOD"] = "HEAD" p["CONTENT_LENGTH"] = "0" @@ -269,7 +267,6 @@ func (c *client) Head(p map[string]string) (resp *http.Response, err error) { // Options issues an OPTIONS request to the fcgi responder. func (c *client) Options(p map[string]string) (resp *http.Response, err error) { - p["REQUEST_METHOD"] = "OPTIONS" p["CONTENT_LENGTH"] = "0" 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 { diff --git a/modules/caddyhttp/reverseproxy/streaming.go b/modules/caddyhttp/reverseproxy/streaming.go index 71b9386..c5369d8 100644 --- a/modules/caddyhttp/reverseproxy/streaming.go +++ b/modules/caddyhttp/reverseproxy/streaming.go @@ -522,5 +522,7 @@ var streamingBufPool = sync.Pool{ }, } -const defaultBufferSize = 32 * 1024 -const wordSize = int(unsafe.Sizeof(uintptr(0))) +const ( + defaultBufferSize = 32 * 1024 + wordSize = int(unsafe.Sizeof(uintptr(0))) +) -- 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/caddyfile.go | 3 ++- modules/caddyhttp/reverseproxy/command.go | 8 +++++--- modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go | 6 +++--- modules/caddyhttp/reverseproxy/healthchecks.go | 3 ++- modules/caddyhttp/reverseproxy/httptransport.go | 7 ++++--- modules/caddyhttp/reverseproxy/reverseproxy.go | 5 +++-- modules/caddyhttp/reverseproxy/upstreams.go | 3 ++- 7 files changed, 21 insertions(+), 14 deletions(-) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go index 774f889..a986f89 100644 --- a/modules/caddyhttp/reverseproxy/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/caddyfile.go @@ -21,6 +21,8 @@ import ( "strconv" "strings" + "github.com/dustin/go-humanize" + "github.com/caddyserver/caddy/v2" "github.com/caddyserver/caddy/v2/caddyconfig" "github.com/caddyserver/caddy/v2/caddyconfig/caddyfile" @@ -28,7 +30,6 @@ import ( "github.com/caddyserver/caddy/v2/modules/caddyhttp" "github.com/caddyserver/caddy/v2/modules/caddyhttp/headers" "github.com/caddyserver/caddy/v2/modules/caddyhttp/rewrite" - "github.com/dustin/go-humanize" ) func init() { diff --git a/modules/caddyhttp/reverseproxy/command.go b/modules/caddyhttp/reverseproxy/command.go index 9359f3d..11f935c 100644 --- a/modules/caddyhttp/reverseproxy/command.go +++ b/modules/caddyhttp/reverseproxy/command.go @@ -21,15 +21,17 @@ import ( "strconv" "strings" + "github.com/spf13/cobra" + "go.uber.org/zap" + + caddycmd "github.com/caddyserver/caddy/v2/cmd" + "github.com/caddyserver/caddy/v2" "github.com/caddyserver/caddy/v2/caddyconfig" "github.com/caddyserver/caddy/v2/caddyconfig/httpcaddyfile" - caddycmd "github.com/caddyserver/caddy/v2/cmd" "github.com/caddyserver/caddy/v2/modules/caddyhttp" "github.com/caddyserver/caddy/v2/modules/caddyhttp/headers" "github.com/caddyserver/caddy/v2/modules/caddytls" - "github.com/spf13/cobra" - "go.uber.org/zap" ) func init() { diff --git a/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go b/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go index f150edc..31febdd 100644 --- a/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go +++ b/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go @@ -24,13 +24,13 @@ import ( "strings" "time" - "github.com/caddyserver/caddy/v2/modules/caddyhttp" - "github.com/caddyserver/caddy/v2/modules/caddyhttp/reverseproxy" - "github.com/caddyserver/caddy/v2/modules/caddytls" "go.uber.org/zap" "go.uber.org/zap/zapcore" "github.com/caddyserver/caddy/v2" + "github.com/caddyserver/caddy/v2/modules/caddyhttp" + "github.com/caddyserver/caddy/v2/modules/caddyhttp/reverseproxy" + "github.com/caddyserver/caddy/v2/modules/caddytls" ) var noopLogger = zap.NewNop() diff --git a/modules/caddyhttp/reverseproxy/healthchecks.go b/modules/caddyhttp/reverseproxy/healthchecks.go index 80b635a..1b4f2d0 100644 --- a/modules/caddyhttp/reverseproxy/healthchecks.go +++ b/modules/caddyhttp/reverseproxy/healthchecks.go @@ -26,9 +26,10 @@ import ( "strconv" "time" + "go.uber.org/zap" + "github.com/caddyserver/caddy/v2" "github.com/caddyserver/caddy/v2/modules/caddyhttp" - "go.uber.org/zap" ) // HealthChecks configures active and passive health checks. diff --git a/modules/caddyhttp/reverseproxy/httptransport.go b/modules/caddyhttp/reverseproxy/httptransport.go index 8334f25..bcce67b 100644 --- a/modules/caddyhttp/reverseproxy/httptransport.go +++ b/modules/caddyhttp/reverseproxy/httptransport.go @@ -28,12 +28,13 @@ import ( "strings" "time" - "github.com/caddyserver/caddy/v2" - "github.com/caddyserver/caddy/v2/modules/caddyhttp" - "github.com/caddyserver/caddy/v2/modules/caddytls" "github.com/mastercactapus/proxyprotocol" "go.uber.org/zap" "golang.org/x/net/http2" + + "github.com/caddyserver/caddy/v2" + "github.com/caddyserver/caddy/v2/modules/caddyhttp" + "github.com/caddyserver/caddy/v2/modules/caddytls" ) func init() { 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() { diff --git a/modules/caddyhttp/reverseproxy/upstreams.go b/modules/caddyhttp/reverseproxy/upstreams.go index 19e4f3c..8bdc60d 100644 --- a/modules/caddyhttp/reverseproxy/upstreams.go +++ b/modules/caddyhttp/reverseproxy/upstreams.go @@ -11,8 +11,9 @@ import ( "sync" "time" - "github.com/caddyserver/caddy/v2" "go.uber.org/zap" + + "github.com/caddyserver/caddy/v2" ) 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 ++++----- modules/caddyhttp/reverseproxy/upstreams.go | 34 ++++++++++++++++---------- 2 files changed, 27 insertions(+), 19 deletions(-) (limited to 'modules/caddyhttp/reverseproxy') 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 diff --git a/modules/caddyhttp/reverseproxy/upstreams.go b/modules/caddyhttp/reverseproxy/upstreams.go index 8bdc60d..09efdd5 100644 --- a/modules/caddyhttp/reverseproxy/upstreams.go +++ b/modules/caddyhttp/reverseproxy/upstreams.go @@ -114,7 +114,7 @@ func (su SRVUpstreams) GetUpstreams(r *http.Request) ([]*Upstream, error) { cached := srvs[suAddr] srvsMu.RUnlock() if cached.isFresh() { - return cached.upstreams, nil + return allNew(cached.upstreams), nil } // otherwise, obtain a write-lock to update the cached value @@ -126,7 +126,7 @@ func (su SRVUpstreams) GetUpstreams(r *http.Request) ([]*Upstream, error) { // have refreshed it in the meantime before we re-obtained our lock cached = srvs[suAddr] if cached.isFresh() { - return cached.upstreams, nil + return allNew(cached.upstreams), nil } su.logger.Debug("refreshing SRV upstreams", @@ -145,7 +145,7 @@ func (su SRVUpstreams) GetUpstreams(r *http.Request) ([]*Upstream, error) { su.logger.Warn("SRV records filtered", zap.Error(err)) } - upstreams := make([]*Upstream, len(records)) + upstreams := make([]Upstream, len(records)) for i, rec := range records { su.logger.Debug("discovered SRV record", zap.String("target", rec.Target), @@ -153,7 +153,7 @@ func (su SRVUpstreams) GetUpstreams(r *http.Request) ([]*Upstream, error) { zap.Uint16("priority", rec.Priority), zap.Uint16("weight", rec.Weight)) addr := net.JoinHostPort(rec.Target, strconv.Itoa(int(rec.Port))) - upstreams[i] = &Upstream{Dial: addr} + upstreams[i] = Upstream{Dial: addr} } // before adding a new one to the cache (as opposed to replacing stale one), make room if cache is full @@ -170,7 +170,7 @@ func (su SRVUpstreams) GetUpstreams(r *http.Request) ([]*Upstream, error) { upstreams: upstreams, } - return upstreams, nil + return allNew(upstreams), nil } func (su SRVUpstreams) String() string { @@ -206,7 +206,7 @@ func (SRVUpstreams) formattedAddr(service, proto, name string) string { type srvLookup struct { srvUpstreams SRVUpstreams freshness time.Time - upstreams []*Upstream + upstreams []Upstream } func (sl srvLookup) isFresh() bool { @@ -325,7 +325,7 @@ func (au AUpstreams) GetUpstreams(r *http.Request) ([]*Upstream, error) { cached := aAaaa[auStr] aAaaaMu.RUnlock() if cached.isFresh() { - return cached.upstreams, nil + return allNew(cached.upstreams), nil } // otherwise, obtain a write-lock to update the cached value @@ -337,7 +337,7 @@ func (au AUpstreams) GetUpstreams(r *http.Request) ([]*Upstream, error) { // have refreshed it in the meantime before we re-obtained our lock cached = aAaaa[auStr] if cached.isFresh() { - return cached.upstreams, nil + return allNew(cached.upstreams), nil } name := repl.ReplaceAll(au.Name, "") @@ -348,15 +348,15 @@ func (au AUpstreams) GetUpstreams(r *http.Request) ([]*Upstream, error) { return nil, err } - upstreams := make([]*Upstream, len(ips)) + upstreams := make([]Upstream, len(ips)) for i, ip := range ips { - upstreams[i] = &Upstream{ + upstreams[i] = Upstream{ Dial: net.JoinHostPort(ip.String(), port), } } // before adding a new one to the cache (as opposed to replacing stale one), make room if cache is full - if cached.freshness.IsZero() && len(srvs) >= 100 { + if cached.freshness.IsZero() && len(aAaaa) >= 100 { for randomKey := range aAaaa { delete(aAaaa, randomKey) break @@ -369,7 +369,7 @@ func (au AUpstreams) GetUpstreams(r *http.Request) ([]*Upstream, error) { upstreams: upstreams, } - return upstreams, nil + return allNew(upstreams), nil } func (au AUpstreams) String() string { return net.JoinHostPort(au.Name, au.Port) } @@ -377,7 +377,7 @@ func (au AUpstreams) String() string { return net.JoinHostPort(au.Name, au.Port) type aLookup struct { aUpstreams AUpstreams freshness time.Time - upstreams []*Upstream + upstreams []Upstream } func (al aLookup) isFresh() bool { @@ -483,6 +483,14 @@ func (u *UpstreamResolver) ParseAddresses() error { return nil } +func allNew(upstreams []Upstream) []*Upstream { + results := make([]*Upstream, len(upstreams)) + for i := range upstreams { + results[i] = &Upstream{Dial: upstreams[i].Dial} + } + return results +} + var ( srvs = make(map[string]srvLookup) srvsMu sync.RWMutex -- cgit v1.2.3 From 288216e1fbf25ebe11fcea7c71be526c4cd0dcce Mon Sep 17 00:00:00 2001 From: Karun Agarwal <113603846+singhalkarun@users.noreply.github.com> Date: Sat, 19 Aug 2023 16:58:25 +0530 Subject: httpcaddyfile: Stricter errors for site and upstream address schemes (#5757) Co-authored-by: Mohammed Al Sahaf Co-authored-by: Francis Lavoie --- modules/caddyhttp/reverseproxy/caddyfile.go | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go index a986f89..674776f 100644 --- a/modules/caddyhttp/reverseproxy/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/caddyfile.go @@ -155,6 +155,18 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { // the underlying JSON does not yet support different // transports (protocols or schemes) to each backend, // so we remember the last one we see and compare them + + switch pa.scheme { + case "wss": + return d.Errf("the scheme wss:// is only supported in browsers; use https:// instead") + case "ws": + return d.Errf("the scheme ws:// is only supported in browsers; use http:// instead") + case "https", "http", "h2c", "": + // Do nothing or handle the valid schemes + default: + return d.Errf("unsupported URL scheme %s://", pa.scheme) + } + if commonScheme != "" && pa.scheme != commonScheme { return d.Errf("for now, all proxy upstreams must use the same scheme (transport protocol); expecting '%s://' but got '%s://'", commonScheme, pa.scheme) @@ -193,7 +205,7 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { for _, up := range d.RemainingArgs() { err := appendUpstream(up) if err != nil { - return err + return fmt.Errorf("parsing upstream '%s': %w", up, err) } } @@ -217,7 +229,7 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { for _, up := range args { err := appendUpstream(up) if err != nil { - return err + return fmt.Errorf("parsing upstream '%s': %w", up, err) } } -- cgit v1.2.3 From b377208ededa964893dedd5660734e9616f998f7 Mon Sep 17 00:00:00 2001 From: Matt Holt Date: Wed, 23 Aug 2023 20:47:54 -0600 Subject: chore: Appease gosec linter (#5777) These happen to be harmless memory aliasing but I guess the linter can't know that and we can't really prove it in general. --- modules/caddyhttp/reverseproxy/httptransport.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/httptransport.go b/modules/caddyhttp/reverseproxy/httptransport.go index bcce67b..9290f7e 100644 --- a/modules/caddyhttp/reverseproxy/httptransport.go +++ b/modules/caddyhttp/reverseproxy/httptransport.go @@ -529,7 +529,8 @@ func (t TLSConfig) MakeTLSClientConfig(ctx caddy.Context) (*tls.Config, error) { certs := caddytls.AllMatchingCertificates(t.ClientCertificateAutomate) var err error for _, cert := range certs { - err = cri.SupportsCertificate(&cert.Certificate) + certCertificate := cert.Certificate // avoid taking address of iteration variable (gosec warning) + err = cri.SupportsCertificate(&certCertificate) if err == nil { return &cert.Certificate, nil } -- cgit v1.2.3 From 1e0dea59efc606f0eaec09993649930d12961bb9 Mon Sep 17 00:00:00 2001 From: Pascal Vorwerk Date: Mon, 11 Sep 2023 01:08:02 +0200 Subject: reverseproxy: fix nil pointer dereference in AUpstreams.GetUpstreams (#5811) fix a nil pointer dereference in AUpstreams.GetUpstreams when AUpstreams.Versions is not set (fixes caddyserver#5809) Signed-off-by: Pascal Vorwerk --- modules/caddyhttp/reverseproxy/upstreams.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/upstreams.go b/modules/caddyhttp/reverseproxy/upstreams.go index 09efdd5..528e2c5 100644 --- a/modules/caddyhttp/reverseproxy/upstreams.go +++ b/modules/caddyhttp/reverseproxy/upstreams.go @@ -297,8 +297,8 @@ func (au *AUpstreams) Provision(_ caddy.Context) error { func (au AUpstreams) GetUpstreams(r *http.Request) ([]*Upstream, error) { repl := r.Context().Value(caddy.ReplacerCtxKey).(*caddy.Replacer) - resolveIpv4 := au.Versions.IPv4 == nil || *au.Versions.IPv4 - resolveIpv6 := au.Versions.IPv6 == nil || *au.Versions.IPv6 + resolveIpv4 := au.Versions == nil || au.Versions.IPv4 == nil || *au.Versions.IPv4 + resolveIpv6 := au.Versions == nil || au.Versions.IPv6 == nil || *au.Versions.IPv6 // Map ipVersion early, so we can use it as part of the cache-key. // This should be fairly inexpensive and comes and the upside of -- 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') 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') 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 b245ecd325428966ac4e4c208e268967d0e0cb83 Mon Sep 17 00:00:00 2001 From: Fred Cox Date: Wed, 11 Oct 2023 09:42:40 +0100 Subject: reverseproxy: fix parsing Caddyfile fails for unlimited request/response buffers (#5828) --- modules/caddyhttp/reverseproxy/caddyfile.go | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go index 674776f..533a82e 100644 --- a/modules/caddyhttp/reverseproxy/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/caddyfile.go @@ -551,17 +551,24 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { if !d.NextArg() { return d.ArgErr() } - size, err := humanize.ParseBytes(d.Val()) - if err != nil { - return d.Errf("invalid byte size '%s': %v", d.Val(), err) + val := d.Val() + var size int64 + if val == "unlimited" { + size = -1 + } else { + usize, err := humanize.ParseBytes(val) + if err != nil { + return d.Errf("invalid byte size '%s': %v", val, err) + } + size = int64(usize) } if d.NextArg() { return d.ArgErr() } if subdir == "request_buffers" { - h.RequestBuffers = int64(size) + h.RequestBuffers = size } else if subdir == "response_buffers" { - h.ResponseBuffers = int64(size) + h.ResponseBuffers = size } // TODO: These three properties are deprecated; remove them sometime after v2.6.4 -- cgit v1.2.3 From 05dbe1c171846b0b683dedbe2c4c20683e867ba0 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Wed, 11 Oct 2023 11:50:28 -0400 Subject: reverseproxy: Replace health header placeholders (#5861) --- modules/caddyhttp/reverseproxy/caddyfile.go | 2 +- modules/caddyhttp/reverseproxy/healthchecks.go | 14 ++++++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go index 533a82e..95293f0 100644 --- a/modules/caddyhttp/reverseproxy/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/caddyfile.go @@ -372,7 +372,7 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { if len(values) == 0 { values = append(values, "") } - healthHeaders[key] = values + healthHeaders[key] = append(healthHeaders[key], values...) } if h.HealthChecks == nil { h.HealthChecks = new(HealthChecks) diff --git a/modules/caddyhttp/reverseproxy/healthchecks.go b/modules/caddyhttp/reverseproxy/healthchecks.go index 1b4f2d0..ad21ccb 100644 --- a/modules/caddyhttp/reverseproxy/healthchecks.go +++ b/modules/caddyhttp/reverseproxy/healthchecks.go @@ -358,11 +358,17 @@ 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 { + + // set headers, using a replacer with only globals (env vars, system info, etc.) + repl := caddy.NewReplacer() + for key, vals := range h.HealthChecks.Active.Headers { + key = repl.ReplaceAll(key, "") if key == "Host" { - req.Host = h.HealthChecks.Active.Headers.Get(key) - } else { - req.Header[key] = hdrs + req.Host = repl.ReplaceAll(h.HealthChecks.Active.Headers.Get(key), "") + continue + } + for _, val := range vals { + req.Header.Add(key, repl.ReplaceKnown(val, "")) } } -- cgit v1.2.3 From a8586b05aac81435f2a3d929a762fc994accbfdd Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Wed, 11 Oct 2023 11:50:44 -0400 Subject: reverseproxy: Add logging for dynamic A upstreams (#5857) --- modules/caddyhttp/reverseproxy/upstreams.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/upstreams.go b/modules/caddyhttp/reverseproxy/upstreams.go index 528e2c5..2d21a5c 100644 --- a/modules/caddyhttp/reverseproxy/upstreams.go +++ b/modules/caddyhttp/reverseproxy/upstreams.go @@ -251,6 +251,8 @@ type AUpstreams struct { Versions *IPVersions `json:"versions,omitempty"` resolver *net.Resolver + + logger *zap.Logger } // CaddyModule returns the Caddy module information. @@ -261,7 +263,8 @@ func (AUpstreams) CaddyModule() caddy.ModuleInfo { } } -func (au *AUpstreams) Provision(_ caddy.Context) error { +func (au *AUpstreams) Provision(ctx caddy.Context) error { + au.logger = ctx.Logger() if au.Refresh == 0 { au.Refresh = caddy.Duration(time.Minute) } @@ -343,6 +346,11 @@ func (au AUpstreams) GetUpstreams(r *http.Request) ([]*Upstream, error) { name := repl.ReplaceAll(au.Name, "") port := repl.ReplaceAll(au.Port, "") + au.logger.Debug("refreshing A upstreams", + zap.String("version", ipVersion), + zap.String("name", name), + zap.String("port", port)) + ips, err := au.resolver.LookupIP(r.Context(), ipVersion, name) if err != nil { return nil, err @@ -350,6 +358,8 @@ func (au AUpstreams) GetUpstreams(r *http.Request) ([]*Upstream, error) { upstreams := make([]Upstream, len(ips)) for i, ip := range ips { + au.logger.Debug("discovered A record", + zap.String("ip", ip.String())) upstreams[i] = Upstream{ Dial: net.JoinHostPort(ip.String(), port), } -- cgit v1.2.3 From e8b8d4a8cdf116bf05345443e37a21ea7a37b8bb Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Wed, 11 Oct 2023 12:04:28 -0400 Subject: reverseproxy: Fix `least_conn` policy regression (#5862) --- modules/caddyhttp/reverseproxy/selectionpolicies.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/selectionpolicies.go b/modules/caddyhttp/reverseproxy/selectionpolicies.go index bc6de35..acb069a 100644 --- a/modules/caddyhttp/reverseproxy/selectionpolicies.go +++ b/modules/caddyhttp/reverseproxy/selectionpolicies.go @@ -269,7 +269,7 @@ func (LeastConnSelection) Select(pool UpstreamPool, _ *http.Request, _ http.Resp // sample: https://en.wikipedia.org/wiki/Reservoir_sampling if numReqs == leastReqs { count++ - if count > 1 || (weakrand.Int()%count) == 0 { //nolint:gosec + if count == 1 || (weakrand.Int()%count) == 0 { //nolint:gosec bestHost = host } } -- 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/caddyfile.go | 7 +++++ modules/caddyhttp/reverseproxy/reverseproxy.go | 21 ++++++++++++-- modules/caddyhttp/reverseproxy/streaming.go | 36 ++++++++++++++++++++++-- modules/caddyhttp/reverseproxy/streaming_test.go | 4 ++- 4 files changed, 62 insertions(+), 6 deletions(-) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go index 95293f0..bcbe744 100644 --- a/modules/caddyhttp/reverseproxy/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/caddyfile.go @@ -90,6 +90,7 @@ func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error) // max_buffer_size // stream_timeout // stream_close_delay +// trace_logs // // # request manipulation // trusted_proxies [private_ranges] @@ -782,6 +783,12 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { responseHandler, ) + case "verbose_logs": + if h.VerboseLogs { + return d.Err("verbose_logs already specified") + } + h.VerboseLogs = true + default: return d.Errf("unrecognized subdirective %s", d.Val()) } 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 } diff --git a/modules/caddyhttp/reverseproxy/streaming.go b/modules/caddyhttp/reverseproxy/streaming.go index c5369d8..155a1df 100644 --- a/modules/caddyhttp/reverseproxy/streaming.go +++ b/modules/caddyhttp/reverseproxy/streaming.go @@ -184,15 +184,22 @@ func (h Handler) isBidirectionalStream(req *http.Request, res *http.Response) bo (ae == "identity" || ae == "") } -func (h Handler) copyResponse(dst http.ResponseWriter, src io.Reader, flushInterval time.Duration) error { +func (h Handler) copyResponse(dst http.ResponseWriter, src io.Reader, flushInterval time.Duration, logger *zap.Logger) error { var w io.Writer = dst if flushInterval != 0 { + var mlwLogger *zap.Logger + if h.VerboseLogs { + mlwLogger = logger.Named("max_latency_writer") + } else { + mlwLogger = zap.NewNop() + } mlw := &maxLatencyWriter{ dst: dst, //nolint:bodyclose flush: http.NewResponseController(dst).Flush, latency: flushInterval, + logger: mlwLogger, } defer mlw.stop() @@ -205,19 +212,30 @@ func (h Handler) copyResponse(dst http.ResponseWriter, src io.Reader, flushInter buf := streamingBufPool.Get().(*[]byte) defer streamingBufPool.Put(buf) - _, err := h.copyBuffer(w, src, *buf) + + var copyLogger *zap.Logger + if h.VerboseLogs { + copyLogger = logger + } else { + copyLogger = zap.NewNop() + } + + _, err := h.copyBuffer(w, src, *buf, copyLogger) return err } // copyBuffer returns any write errors or non-EOF read errors, and the amount // of bytes written. -func (h Handler) copyBuffer(dst io.Writer, src io.Reader, buf []byte) (int64, error) { +func (h Handler) copyBuffer(dst io.Writer, src io.Reader, buf []byte, logger *zap.Logger) (int64, error) { if len(buf) == 0 { buf = make([]byte, defaultBufferSize) } var written int64 for { + logger.Debug("waiting to read from upstream") nr, rerr := src.Read(buf) + logger := logger.With(zap.Int("read", nr)) + logger.Debug("read from upstream", zap.Error(rerr)) if rerr != nil && rerr != io.EOF && rerr != context.Canceled { // TODO: this could be useful to know (indeed, it revealed an error in our // fastcgi PoC earlier; but it's this single error report here that necessitates @@ -229,10 +247,15 @@ func (h Handler) copyBuffer(dst io.Writer, src io.Reader, buf []byte) (int64, er h.logger.Error("reading from backend", zap.Error(rerr)) } if nr > 0 { + logger.Debug("writing to downstream") nw, werr := dst.Write(buf[:nr]) if nw > 0 { written += int64(nw) } + logger.Debug("wrote to downstream", + zap.Int("written", nw), + zap.Int64("written_total", written), + zap.Error(werr)) if werr != nil { return written, fmt.Errorf("writing: %w", werr) } @@ -452,18 +475,22 @@ type maxLatencyWriter struct { mu sync.Mutex // protects t, flushPending, and dst.Flush t *time.Timer flushPending bool + logger *zap.Logger } func (m *maxLatencyWriter) Write(p []byte) (n int, err error) { m.mu.Lock() defer m.mu.Unlock() n, err = m.dst.Write(p) + m.logger.Debug("wrote bytes", zap.Int("n", n), zap.Error(err)) if m.latency < 0 { + m.logger.Debug("flushing immediately") //nolint:errcheck m.flush() return } if m.flushPending { + m.logger.Debug("delayed flush already pending") return } if m.t == nil { @@ -471,6 +498,7 @@ func (m *maxLatencyWriter) Write(p []byte) (n int, err error) { } else { m.t.Reset(m.latency) } + m.logger.Debug("timer set for delayed flush", zap.Duration("duration", m.latency)) m.flushPending = true return } @@ -479,8 +507,10 @@ func (m *maxLatencyWriter) delayedFlush() { m.mu.Lock() defer m.mu.Unlock() if !m.flushPending { // if stop was called but AfterFunc already started this goroutine + m.logger.Debug("delayed flush is not pending") return } + m.logger.Debug("delayed flush") //nolint:errcheck m.flush() m.flushPending = false diff --git a/modules/caddyhttp/reverseproxy/streaming_test.go b/modules/caddyhttp/reverseproxy/streaming_test.go index 919538f..3f6da2f 100644 --- a/modules/caddyhttp/reverseproxy/streaming_test.go +++ b/modules/caddyhttp/reverseproxy/streaming_test.go @@ -5,6 +5,8 @@ import ( "net/http/httptest" "strings" "testing" + + "github.com/caddyserver/caddy/v2" ) func TestHandlerCopyResponse(t *testing.T) { @@ -22,7 +24,7 @@ func TestHandlerCopyResponse(t *testing.T) { for _, d := range testdata { src := bytes.NewBuffer([]byte(d)) dst.Reset() - err := h.copyResponse(recorder, src, 0) + err := h.copyResponse(recorder, src, 0, caddy.Log()) if err != nil { t.Errorf("failed with error: %v", err) } -- cgit v1.2.3