reverseproxy: Reintroduce caddyfile.Unmarshaler

caddyserver · May 1, 2021 · 274f4cc · 274f4cc
1 parent b6a5bcb
commit 274f4cc
Show file tree

Hide file tree

Showing 3 changed files with 107 additions and 73 deletions.
diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go
@@ -38,14 +38,18 @@ func init() {
 
 func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error) {
 	rp := new(Handler)
-	err := rp.ParseCaddyfileReverseProxy(h)
+	err := rp.UnmarshalCaddyfile(h.Dispenser)
+	if err != nil {
+		return nil, err
+	}
+	err = rp.FinalizeUnmarshalCaddyfile(h)
 	if err != nil {
 		return nil, err
 	}
 	return rp, nil
 }
 
-// ParseCaddyfileReverseProxy sets up the handler from Caddyfile tokens. Syntax:
+// UnmarshalCaddyfile sets up the handler from Caddyfile tokens. Syntax:
 //
 //     reverse_proxy [<matcher>] [<upstreams...>] {
 //         # upstreams
@@ -101,9 +105,7 @@ func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error)
 // as `host:port`, or a URL such as `scheme://host:port`. Scheme
 // and port may be inferred from other parts of the address/URL; if
 // either are missing, defaults to HTTP.
-func (h *Handler) ParseCaddyfileReverseProxy(helper httpcaddyfile.Helper) error {
-	d := helper.Dispenser
-
+func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 	// currently, all backends must use the same scheme/protocol (the
 	// underlying JSON does not yet support per-backend transports)
 	var commonScheme string
@@ -113,9 +115,9 @@ func (h *Handler) ParseCaddyfileReverseProxy(helper httpcaddyfile.Helper) error
 	var transport http.RoundTripper
 	var transportModuleName string
 
-	// collect the response matchers defined as subdirectives prefixed with "@"
-	// for use with "handle_response" blocks
-	responseMatchers := make(map[string]caddyhttp.ResponseMatcher)
+	// collect the response matchers defined as subdirectives
+	// prefixed with "@" for use with "handle_response" blocks
+	h.responseMatchers = make(map[string]caddyhttp.ResponseMatcher)
 
 	// TODO: the logic in this function is kind of sensitive, we need
 	// to write tests before making any more changes to it
@@ -245,7 +247,7 @@ func (h *Handler) ParseCaddyfileReverseProxy(helper httpcaddyfile.Helper) error
 			// 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(), responseMatchers)
+				err := caddyhttp.ParseNamedResponseMatcher(d.NewFromNextSegment(), h.responseMatchers)
 				if err != nil {
 					return err
 				}
@@ -643,69 +645,10 @@ func (h *Handler) ParseCaddyfileReverseProxy(helper httpcaddyfile.Helper) error
 				transport = rt
 
 			case "handle_response":
-				var matcher *caddyhttp.ResponseMatcher
-				args := d.RemainingArgs()
-
-				// the first arg should be a matcher (optional)
-				// the second arg should be a status code (optional)
-				// any more than that isn't currently supported
-				if len(args) > 2 {
-					return d.Errf("too many arguments for 'handle_response': %s", args)
-				}
-
-				// the first arg should always be a matcher.
-				// it doesn't really make sense to support status code without a matcher.
-				if len(args) > 0 {
-					if !strings.HasPrefix(args[0], matcherPrefix) {
-						return d.Errf("must use a named response matcher, starting with '@'")
-					}
-
-					foundMatcher, ok := responseMatchers[args[0]]
-					if !ok {
-						return d.Errf("no named response matcher defined with name '%s'", args[0][1:])
-					}
-					matcher = &foundMatcher
-				}
-
-				// a second arg should be a status code, in which case
-				// we skip parsing the block for routes
-				if len(args) == 2 {
-					_, err := strconv.Atoi(args[1])
-					if err != nil {
-						return d.Errf("bad integer value '%s': %v", args[1], err)
-					}
-
-					// make sure there's no block, cause it doesn't make sense
-					if d.NextBlock(1) {
-						return d.Errf("cannot define routes for 'handle_response' when changing the status code")
-					}
-
-					h.HandleResponse = append(
-						h.HandleResponse,
-						caddyhttp.ResponseHandler{
-							Match:      matcher,
-							StatusCode: caddyhttp.WeakString(args[1]),
-						},
-					)
-					continue
-				}
-
-				// parse the block as routes
-				handler, err := httpcaddyfile.ParseSegmentAsSubroute(helper.WithDispenser(d.NewFromNextSegment()))
-				if err != nil {
-					return err
-				}
-				subroute, ok := handler.(*caddyhttp.Subroute)
-				if !ok {
-					return helper.Errf("segment was not parsed as a subroute")
-				}
-				h.HandleResponse = append(
-					h.HandleResponse,
-					caddyhttp.ResponseHandler{
-						Match:  matcher,
-						Routes: subroute.Routes,
-					},
-				)
+				// 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())
 
 			default:
 				return d.Errf("unrecognized subdirective %s", d.Val())
@@ -746,6 +689,81 @@ func (h *Handler) ParseCaddyfileReverseProxy(helper httpcaddyfile.Helper) error
 		}
 	}
 
+	return nil
+}
+
+// FinalizeUnmarshalCaddyfile finalizes the Caddyfile parsing which
+// requires having an httpcaddyfile.Helper to function, to parse subroutes.
+func (h *Handler) FinalizeUnmarshalCaddyfile(helper httpcaddyfile.Helper) error {
+	for _, d := range h.handleResponseSegments {
+		// consume the "handle_response" token
+		d.Next()
+
+		var matcher *caddyhttp.ResponseMatcher
+		args := d.RemainingArgs()
+
+		// the first arg should be a matcher (optional)
+		// the second arg should be a status code (optional)
+		// any more than that isn't currently supported
+		if len(args) > 2 {
+			return d.Errf("too many arguments for 'handle_response': %s", args)
+		}
+
+		// the first arg should always be a matcher.
+		// it doesn't really make sense to support status code without a matcher.
+		if len(args) > 0 {
+			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:])
+			}
+			matcher = &foundMatcher
+		}
+
+		// a second arg should be a status code, in which case
+		// we skip parsing the block for routes
+		if len(args) == 2 {
+			_, err := strconv.Atoi(args[1])
+			if err != nil {
+				return d.Errf("bad integer value '%s': %v", args[1], err)
+			}
+
+			// make sure there's no block, cause it doesn't make sense
+			if d.NextBlock(1) {
+				return d.Errf("cannot define routes for 'handle_response' when changing the status code")
+			}
+
+			h.HandleResponse = append(
+				h.HandleResponse,
+				caddyhttp.ResponseHandler{
+					Match:      matcher,
+					StatusCode: caddyhttp.WeakString(args[1]),
+				},
+			)
+			continue
+		}
+
+		// parse the block as routes
+		handler, err := httpcaddyfile.ParseSegmentAsSubroute(helper.WithDispenser(d.NewFromNextSegment()))
+		if err != nil {
+			return err
+		}
+		subroute, ok := handler.(*caddyhttp.Subroute)
+		if !ok {
+			return helper.Errf("segment was not parsed as a subroute")
+		}
+		h.HandleResponse = append(
+			h.HandleResponse,
+			caddyhttp.ResponseHandler{
+				Match:  matcher,
+				Routes: subroute.Routes,
+			},
+		)
+	}
+
 	// move the handle_response entries without a matcher to the end.
 	// we can't use sort.SliceStable because it will reorder the rest of the
 	// entries which may be undesirable because we don't have a good
@@ -761,6 +779,10 @@ func (h *Handler) ParseCaddyfileReverseProxy(helper httpcaddyfile.Helper) error
 	}
 	h.HandleResponse = append(withMatchers, withoutMatchers...)
 
+	// clean up the bits we only needed for adapting
+	h.handleResponseSegments = nil
+	h.responseMatchers = nil
+
 	return nil
 }
 
@@ -1001,5 +1023,6 @@ const matcherPrefix = "@"
 
 // Interface guards
 var (
+	_ caddyfile.Unmarshaler = (*Handler)(nil)
 	_ caddyfile.Unmarshaler = (*HTTPTransport)(nil)
 )
diff --git a/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go b/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go
@@ -353,7 +353,11 @@ func parsePHPFastCGI(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error
 
 	// the rest of the config is specified by the user
 	// using the reverse_proxy directive syntax
-	err = rpHandler.ParseCaddyfileReverseProxy(h.WithDispenser(dispenser))
+	err = rpHandler.UnmarshalCaddyfile(dispenser)
+	if err != nil {
+		return nil, err
+	}
+	err = rpHandler.FinalizeUnmarshalCaddyfile(h)
 	if err != nil {
 		return nil, err
 	}

diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go
@@ -31,6 +31,7 @@ import (
 	"time"
 
 	"github.com/caddyserver/caddy/v2"
+	"github.com/caddyserver/caddy/v2/caddyconfig/caddyfile"
 	"github.com/caddyserver/caddy/v2/modules/caddyhttp"
 	"github.com/caddyserver/caddy/v2/modules/caddyhttp/headers"
 	"go.uber.org/zap"
@@ -127,6 +128,12 @@ type Handler struct {
 	Transport http.RoundTripper `json:"-"`
 	CB        CircuitBreaker    `json:"-"`
 
+	// Holds the named response matchers from the Caddyfile while adapting
+	responseMatchers map[string]caddyhttp.ResponseMatcher
+
+	// Holds the handle_response Caddyfile tokens while adapting
+	handleResponseSegments []*caddyfile.Dispenser
+
 	ctx    caddy.Context
 	logger *zap.Logger
 }