From 37718560c1bf8e330d18edb2e663e64a624dee8e Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Wed, 12 May 2021 00:26:16 -0400 Subject: ci: Run CI on PRs targeting minor version branches (#4164) We decided that we'll use branches like `2.4` as the target for any changes that we might want to release in a `2.4.x` version like `2.4.1`, so that we can continue to merge changes targeting the next minor release (e.g. `2.5.0`) on master. Our CI config wasn't set up for this to work properly though, since it was only running checks on PRs targeting master. This should fix it. I couldn't find a way to do a pattern to only match digits for the branch names from Github's docs, it just looks like a pretty generic glob syntax. But this should do until we get to 3.0 --- .github/workflows/ci.yml | 2 ++ .github/workflows/cross-build.yml | 2 ++ .github/workflows/lint.yml | 2 ++ 3 files changed, 6 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 22e7d7e..9a46b80 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,9 +6,11 @@ on: push: branches: - master + - 2.* pull_request: branches: - master + - 2.* jobs: test: diff --git a/.github/workflows/cross-build.yml b/.github/workflows/cross-build.yml index 4da0a25..dc6b8e3 100644 --- a/.github/workflows/cross-build.yml +++ b/.github/workflows/cross-build.yml @@ -4,9 +4,11 @@ on: push: branches: - master + - 2.* pull_request: branches: - master + - 2.* jobs: cross-build-test: diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index ef69150..4b4ed03 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -4,9 +4,11 @@ on: push: branches: - master + - 2.* pull_request: branches: - master + - 2.* jobs: # From https://github.com/golangci/golangci-lint-action -- cgit v1.2.3 From aef8d4decceba3371d6722a81e17cd5f94162116 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Wed, 12 May 2021 16:19:08 -0400 Subject: reverseproxy: Set the headers in the replacer before `handle_response` (#4165) Turns out this was an oversight, we assumed we could use `{http.response.header.*}` but that doesn't work because those are grabbed from the response writer, and we haven't copied any headers into the response writer yet. So the fix is to set all the response headers into the replacer at a new namespace before running the handlers. This adds the `{http.reverse_proxy.header.*}` replacer. See https://caddy.community/t/empty-http-response-header-x-accel-redirect/12447 --- modules/caddyhttp/reverseproxy/reverseproxy.go | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go index ddb3ca9..671f42a 100644 --- a/modules/caddyhttp/reverseproxy/reverseproxy.go +++ b/modules/caddyhttp/reverseproxy/reverseproxy.go @@ -120,9 +120,10 @@ type Handler struct { // handler chain will not affect the health status of the // backend. // - // Two new placeholders are available in this handler chain: - // - `{http.reverse_proxy.status_code}` The status code - // - `{http.reverse_proxy.status_text}` The status text + // Three new placeholders are available in this handler chain: + // - `{http.reverse_proxy.status_code}` The status code from the response + // - `{http.reverse_proxy.status_text}` The status text from the response + // - `{http.reverse_proxy.header.*}` The headers from the response HandleResponse []caddyhttp.ResponseHandler `json:"handle_response,omitempty"` Transport http.RoundTripper `json:"-"` @@ -631,9 +632,17 @@ func (h *Handler) reverseProxy(rw http.ResponseWriter, req *http.Request, repl * if len(rh.Routes) == 0 { continue } + res.Body.Close() + + // set up the replacer so that parts of the original response can be + // used for routing decisions + for field, value := range res.Header { + repl.Set("http.reverse_proxy.header."+field, strings.Join(value, ",")) + } 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)) if routeErr := rh.Routes.Compile(next).ServeHTTP(rw, req); routeErr != nil { // wrap error in roundtripSucceeded so caller knows that -- cgit v1.2.3 From b82db994f39c6991d673a7ae9836f5454b1f9668 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Wed, 12 May 2021 18:18:44 -0400 Subject: caddyfile: Add parse error on site address with trailing `{` (#4163) * caddyfile: Add parse error on site address in `{` This is an incredibly common mistake made by users, so we should catch it earlier in the parser and give a more friendly message. Often it ends up adapting but with mistakes, or erroring out later due to other site addresses being read as directives. There's not really ever a situation where a lone '{' is valid at the end of a site address (but I suppose there are edgecases where the user wants to use a path matcher where it ends specifically in `{`, but... why?), so this should be fine. * Update caddyconfig/caddyfile/parse.go --- caddyconfig/caddyfile/parse.go | 5 +++++ caddyconfig/caddyfile/parse_test.go | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/caddyconfig/caddyfile/parse.go b/caddyconfig/caddyfile/parse.go index 96491bb..d870765 100755 --- a/caddyconfig/caddyfile/parse.go +++ b/caddyconfig/caddyfile/parse.go @@ -214,6 +214,11 @@ func (p *parser) addresses() error { break } + // Users commonly forget to place a space between the address and the '{' + if strings.HasSuffix(tkn, "{") { + return p.Errf("Site addresses cannot end with a curly brace: '%s' - put a space between the token and the brace", tkn) + } + if tkn != "" { // empty token possible if user typed "" // Trailing comma indicates another address will follow, which // may possibly be on the next line diff --git a/caddyconfig/caddyfile/parse_test.go b/caddyconfig/caddyfile/parse_test.go index 3c7db79..12c7139 100755 --- a/caddyconfig/caddyfile/parse_test.go +++ b/caddyconfig/caddyfile/parse_test.go @@ -160,6 +160,10 @@ func TestParseOneAndImport(t *testing.T) { "localhost", }, []int{}}, + {`localhost{ + dir1 + }`, true, []string{}, []int{}}, + {`localhost dir1 { nested { -- cgit v1.2.3