From c7efb0307d425eb533885e314518c370a06763da Mon Sep 17 00:00:00 2001 From: Matt Holt Date: Tue, 13 Oct 2020 10:35:20 -0600 Subject: reverseproxy: Fix dial placeholders, SRV, active health checks (#3780) * reverseproxy: Fix dial placeholders, SRV, active health checks Supercedes #3776 Partially reverts or updates #3756, #3693, and #3695 * reverseproxy: add integration tests Co-authored-by: Mohammed Al Sahaf --- modules/caddyhttp/reverseproxy/healthchecks.go | 43 ++++++++++++++++++------- modules/caddyhttp/reverseproxy/hosts.go | 1 - modules/caddyhttp/reverseproxy/httptransport.go | 3 -- modules/caddyhttp/reverseproxy/reverseproxy.go | 18 +---------- 4 files changed, 32 insertions(+), 33 deletions(-) (limited to 'modules/caddyhttp/reverseproxy') diff --git a/modules/caddyhttp/reverseproxy/healthchecks.go b/modules/caddyhttp/reverseproxy/healthchecks.go index 410b9d4..4e93320 100644 --- a/modules/caddyhttp/reverseproxy/healthchecks.go +++ b/modules/caddyhttp/reverseproxy/healthchecks.go @@ -154,23 +154,42 @@ func (h *Handler) doActiveHealthCheckForAllHosts() { } }() - portStr := strconv.Itoa(upstream.activeHealthCheckPort) - hostAddr := net.JoinHostPort(upstream.networkAddress.Host, portStr) - if upstream.networkAddress.IsUnixNetwork() { + networkAddr, err := caddy.NewReplacer().ReplaceOrErr(upstream.Dial, true, true) + if err != nil { + h.HealthChecks.Active.logger.Error("invalid use of placeholders in dial address for active health checks", + zap.String("address", networkAddr), + zap.Error(err), + ) + return + } + addr, err := caddy.ParseNetworkAddress(networkAddr) + if err != nil { + h.HealthChecks.Active.logger.Error("bad network address", + zap.String("address", networkAddr), + zap.Error(err), + ) + return + } + if hcp := uint(upstream.activeHealthCheckPort); hcp != 0 { + if addr.IsUnixNetwork() { + addr.Network = "tcp" // I guess we just assume TCP since we are using a port?? + } + addr.StartPort, addr.EndPort = hcp, hcp + } + if upstream.LookupSRV == "" && addr.PortRangeSize() != 1 { + h.HealthChecks.Active.logger.Error("multiple addresses (upstream must map to only one address)", + zap.String("address", networkAddr), + ) + return + } + hostAddr := addr.JoinHostPort(0) + if addr.IsUnixNetwork() { // this will be used as the Host portion of a http.Request URL, and // paths to socket files would produce an error when creating URL, // so use a fake Host value instead; unix sockets are usually local hostAddr = "localhost" } - - dialInfo := DialInfo{ - Upstream: upstream, - Network: upstream.networkAddress.Network, - Host: upstream.networkAddress.Host, - Port: portStr, - Address: hostAddr, - } - err := h.doActiveHealthCheck(dialInfo, hostAddr, upstream.Host) + err = h.doActiveHealthCheck(DialInfo{Network: addr.Network, Address: hostAddr}, hostAddr, upstream.Host) if err != nil { h.HealthChecks.Active.logger.Error("active health check failed", zap.String("address", hostAddr), diff --git a/modules/caddyhttp/reverseproxy/hosts.go b/modules/caddyhttp/reverseproxy/hosts.go index b7b8c9b..b9817d2 100644 --- a/modules/caddyhttp/reverseproxy/hosts.go +++ b/modules/caddyhttp/reverseproxy/hosts.go @@ -92,7 +92,6 @@ type Upstream struct { // HeaderAffinity string // IPAffinity string - networkAddress caddy.NetworkAddress activeHealthCheckPort int healthCheckPolicy *PassiveHealthChecks cb CircuitBreaker diff --git a/modules/caddyhttp/reverseproxy/httptransport.go b/modules/caddyhttp/reverseproxy/httptransport.go index 7e3bb69..dce7b9e 100644 --- a/modules/caddyhttp/reverseproxy/httptransport.go +++ b/modules/caddyhttp/reverseproxy/httptransport.go @@ -182,9 +182,6 @@ func (h *HTTPTransport) NewTransport(ctx caddy.Context) (*http.Transport, error) if dialInfo, ok := GetDialInfo(ctx); ok { network = dialInfo.Network address = dialInfo.Address - if dialInfo.Upstream.networkAddress.IsUnixNetwork() { - address = dialInfo.Host - } } conn, err := dialer.DialContext(ctx, network, address) if err != nil { diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go index eac02b6..305a0b3 100644 --- a/modules/caddyhttp/reverseproxy/reverseproxy.go +++ b/modules/caddyhttp/reverseproxy/reverseproxy.go @@ -129,10 +129,8 @@ func (h *Handler) Provision(ctx caddy.Context) error { h.ctx = ctx h.logger = ctx.Logger(h) - // get validation out of the way + // verify SRV compatibility for i, v := range h.Upstreams { - // Having LookupSRV non-empty conflicts with 2 other config parameters: active health checks, and upstream dial address. - // Therefore if LookupSRV is empty, then there are no immediately apparent config conflicts, and the iteration can be skipped. if v.LookupSRV == "" { continue } @@ -219,18 +217,6 @@ func (h *Handler) Provision(ctx caddy.Context) error { // set up upstreams for _, upstream := range h.Upstreams { - if upstream.LookupSRV == "" { - addr, err := caddy.ParseNetworkAddress(upstream.Dial) - if err != nil { - return err - } - - if addr.PortRangeSize() != 1 { - return fmt.Errorf("multiple addresses (upstream must map to only one address): %v", addr) - } - - upstream.networkAddress = addr - } // create or get the host representation for this upstream var host Host = new(upstreamHost) existingHost, loaded := hosts.LoadOrStore(upstream.String(), host) @@ -292,8 +278,6 @@ func (h *Handler) Provision(ctx caddy.Context) error { // then use it, otherwise use the port of upstream. if h.HealthChecks.Active.Port != 0 { upstream.activeHealthCheckPort = h.HealthChecks.Active.Port - } else { - upstream.activeHealthCheckPort = int(upstream.networkAddress.StartPort) } } -- cgit v1.2.3