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) --- caddytest/integration/reverseproxy_test.go | 95 ++------------------------ modules/caddyhttp/reverseproxy/caddyfile.go | 19 +----- modules/caddyhttp/reverseproxy/healthchecks.go | 2 +- modules/caddyhttp/reverseproxy/hosts.go | 49 +++---------- modules/caddyhttp/reverseproxy/reverseproxy.go | 14 ---- 5 files changed, 18 insertions(+), 161 deletions(-) diff --git a/caddytest/integration/reverseproxy_test.go b/caddytest/integration/reverseproxy_test.go index 4333d12..4f4261b 100644 --- a/caddytest/integration/reverseproxy_test.go +++ b/caddytest/integration/reverseproxy_test.go @@ -40,11 +40,10 @@ func TestSRVReverseProxy(t *testing.T) { "handle": [ { "handler": "reverse_proxy", - "upstreams": [ - { - "lookup_srv": "srv.host.service.consul" - } - ] + "dynamic_upstreams": { + "source": "srv", + "name": "srv.host.service.consul" + } } ] } @@ -57,47 +56,6 @@ func TestSRVReverseProxy(t *testing.T) { `, "json") } -func TestSRVWithDial(t *testing.T) { - caddytest.AssertLoadError(t, ` - { - "apps": { - "pki": { - "certificate_authorities": { - "local": { - "install_trust": false - } - } - }, - "http": { - "grace_period": 1, - "servers": { - "srv0": { - "listen": [ - ":18080" - ], - "routes": [ - { - "handle": [ - { - "handler": "reverse_proxy", - "upstreams": [ - { - "dial": "tcp/address.to.upstream:80", - "lookup_srv": "srv.host.service.consul" - } - ] - } - ] - } - ] - } - } - } - } - } - `, "json", `upstream: specifying dial address is incompatible with lookup_srv: 0: {\"dial\": \"tcp/address.to.upstream:80\", \"lookup_srv\": \"srv.host.service.consul\"}`) -} - func TestDialWithPlaceholderUnix(t *testing.T) { if runtime.GOOS == "windows" { @@ -369,51 +327,6 @@ func TestReverseProxyWithPlaceholderTCPDialAddress(t *testing.T) { tester.AssertResponse(req, 200, "Hello, World!") } -func TestSRVWithActiveHealthcheck(t *testing.T) { - caddytest.AssertLoadError(t, ` - { - "apps": { - "pki": { - "certificate_authorities" : { - "local" : { - "install_trust": false - } - } - }, - "http": { - "grace_period": 1, - "servers": { - "srv0": { - "listen": [ - ":18080" - ], - "routes": [ - { - "handle": [ - { - "handler": "reverse_proxy", - "health_checks": { - "active": { - "path": "/ok" - } - }, - "upstreams": [ - { - "lookup_srv": "srv.host.service.consul" - } - ] - } - ] - } - ] - } - } - } - } - } - `, "json", `upstream: lookup_srv is incompatible with active health checks: 0: {\"dial\": \"\", \"lookup_srv\": \"srv.host.service.consul\"}`) -} - func TestReverseProxyHealthCheck(t *testing.T) { tester := caddytest.NewTester(t) tester.InitServer(` 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