reverseproxy: Remove deprecated lookup_srv (#5396)

caddytest/integration/reverseproxy_test.go

@@ -40,11 +40,10 @@ func TestSRVReverseProxy(t *testing.T) {
 								"handle": [
 									{
 										"handler": "reverse_proxy",
-										"upstreams": [
+										"dynamic_upstreams": {
-											{
+											"source": "srv",
-												"lookup_srv": "srv.host.service.consul"
+											"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(`

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
+	// it to the list.
-	// treated as a SRV-based upstream, and any port will be
-	// dropped.
 	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})
-		}
 		return nil
 	}
 

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),
 				)

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,24 +97,11 @@ 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
+// context. Note that the returned value is not a pointer.
-// returned address is chosen; otherwise, the upstream's regular dial address
-// field is used. 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, "")
@@ -139,7 +113,6 @@ func (u *Upstream) fillDialInfo(r *http.Request) (DialInfo, error) {
 		return DialInfo{}, fmt.Errorf("upstream %s: dial address must represent precisely one socket: %s represents %d",
 			u.Dial, dial, numPorts)
 	}
-	}
 
 	return DialInfo{
 		Upstream: u,

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")