From 84e20aa9c318dece00876d1b991b317bf09a39c4 Mon Sep 17 00:00:00 2001 From: Ludovic Fernandez Date: Wed, 12 Feb 2025 10:02:04 +0100 Subject: [PATCH 1/6] chore: update linter --- .github/workflows/validate.yaml | 2 +- .golangci.yml | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/validate.yaml b/.github/workflows/validate.yaml index dcc04732e..dd20daf56 100644 --- a/.github/workflows/validate.yaml +++ b/.github/workflows/validate.yaml @@ -7,7 +7,7 @@ on: env: GO_VERSION: '1.23' - GOLANGCI_LINT_VERSION: v1.63.3 + GOLANGCI_LINT_VERSION: v1.64.2 MISSPELL_VERSION: v0.6.0 jobs: diff --git a/.golangci.yml b/.golangci.yml index 772583dac..c233d4c19 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,5 +1,6 @@ run: timeout: 10m + relative-path-mode: cfg linters-settings: govet: @@ -162,6 +163,7 @@ linters-settings: linters: enable-all: true disable: + - tenv # Deprecated - sqlclosecheck # not relevant (SQL) - rowserrcheck # not relevant (SQL) - cyclop # duplicate of gocyclo @@ -198,7 +200,6 @@ linters: - maintidx # kind of duplicate of gocyclo - nonamedreturns # Too strict - gosmopolitan # not relevant - - exportloopref # Not relevant since go1.22 issues: exclude-use-default: false From eb07a5ca1a554e2a7530ff25aa64edf3da9fe413 Mon Sep 17 00:00:00 2001 From: Kevin Pollet Date: Fri, 14 Feb 2025 11:24:04 +0100 Subject: [PATCH 2/6] Bump github.com/traefik/paerser to v0.2.2 Co-authored-by: Romain --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index f75279b6e..7356e9962 100644 --- a/go.mod +++ b/go.mod @@ -57,7 +57,7 @@ require ( github.com/stretchr/testify v1.10.0 github.com/stvp/go-udp-testing v0.0.0-20191102171040-06b61409b154 // No tag on the repo. github.com/testcontainers/testcontainers-go v0.32.0 - github.com/traefik/paerser v0.2.1 + github.com/traefik/paerser v0.2.2 github.com/traefik/yaegi v0.16.1 github.com/uber/jaeger-client-go v2.30.0+incompatible github.com/uber/jaeger-lib v2.4.1+incompatible diff --git a/go.sum b/go.sum index 9237f3518..46b093195 100644 --- a/go.sum +++ b/go.sum @@ -1308,8 +1308,8 @@ github.com/tklauser/numcpus v0.6.1 h1:ng9scYS7az0Bk4OZLvrNXNSAO2Pxr1XXRAPyjhIx+F github.com/tklauser/numcpus v0.6.1/go.mod h1:1XfjsgE2zo8GVw7POkMbHENHzVg3GzmoZ9fESEdAacY= github.com/tmc/grpc-websocket-proxy v0.0.0-20170815181823-89b8d40f7ca8/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U= github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U= -github.com/traefik/paerser v0.2.1 h1:LFgeak1NmjEHF53c9ENdXdL1UMkF/lD5t+7Evsz4hH4= -github.com/traefik/paerser v0.2.1/go.mod h1:7BBDd4FANoVgaTZG+yh26jI6CA2nds7D/4VTEdIsh24= +github.com/traefik/paerser v0.2.2 h1:cpzW/ZrQrBh3mdwD/jnp6aXASiUFKOVr6ldP+keJTcQ= +github.com/traefik/paerser v0.2.2/go.mod h1:7BBDd4FANoVgaTZG+yh26jI6CA2nds7D/4VTEdIsh24= github.com/traefik/yaegi v0.16.1 h1:f1De3DVJqIDKmnasUF6MwmWv1dSEEat0wcpXhD2On3E= github.com/traefik/yaegi v0.16.1/go.mod h1:4eVhbPb3LnD2VigQjhYbEJ69vDRFdT2HQNrXx8eEwUY= github.com/transip/gotransip/v6 v6.26.0 h1:Aejfvh8rSp8Mj2GX/RpdBjMCv+Iy/DmgfNgczPDP550= From 8e5d4c6ae9a984b0cc420559326594e3e91e8347 Mon Sep 17 00:00:00 2001 From: Ludovic Fernandez Date: Fri, 21 Feb 2025 09:36:04 +0100 Subject: [PATCH 3/6] Bum github.com/go-acme/lego/v4 to v4.22.2 --- docs/content/https/acme.md | 4 +++- go.mod | 4 ++-- go.sum | 8 ++++---- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/docs/content/https/acme.md b/docs/content/https/acme.md index 2d9583fc9..59dafb831 100644 --- a/docs/content/https/acme.md +++ b/docs/content/https/acme.md @@ -316,7 +316,7 @@ For complete details, refer to your provider's _Additional configuration_ link. | Provider Name | Provider Code | Environment Variables | | |------------------------------------------------------------------------|--------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|---------------------------------------------------------------------------------| -| [ACME DNS](https://github.com/joohoi/acme-dns) | `acme-dns` | `ACME_DNS_API_BASE`, `ACME_DNS_STORAGE_PATH` | [Additional configuration](https://go-acme.github.io/lego/dns/acme-dns) | +| [ACME DNS](https://github.com/joohoi/acme-dns) | `acme-dns` | `ACME_DNS_API_BASE`, `ACME_DNS_STORAGE_PATH`, `ACME_DNS_STORAGE_BASE_URL` | [Additional configuration](https://go-acme.github.io/lego/dns/acme-dns) | | [Alibaba Cloud](https://www.alibabacloud.com) | `alidns` | `ALICLOUD_ACCESS_KEY`, `ALICLOUD_SECRET_KEY`, `ALICLOUD_REGION_ID` | [Additional configuration](https://go-acme.github.io/lego/dns/alidns) | | [all-inkl](https://all-inkl.com) | `allinkl` | `ALL_INKL_LOGIN`, `ALL_INKL_PASSWORD` | [Additional configuration](https://go-acme.github.io/lego/dns/allinkl) | | [ArvanCloud](https://www.arvancloud.ir/en) | `arvancloud` | `ARVANCLOUD_API_KEY` | [Additional configuration](https://go-acme.github.io/lego/dns/arvancloud) | @@ -397,6 +397,7 @@ For complete details, refer to your provider's _Additional configuration_ link. | [Metaname](https://metaname.net) | `metaname` | `METANAME_ACCOUNT_REFERENCE`, `METANAME_API_KEY` | [Additional configuration](https://go-acme.github.io/lego/dns/metaname) | | [mijn.host](https://mijn.host/) | `mijnhost` | `MIJNHOST_API_KEY` | [Additional configuration](https://go-acme.github.io/lego/dns/mijnhost) | | [Mittwald](https://www.mittwald.de) | `mittwald` | `MITTWALD_TOKEN` | [Additional configuration](https://go-acme.github.io/lego/dns/mittwald) | +| [myaddr.{tools,dev,io}](https://myaddr.tools/) | `myaddr` | `MYADDR_PRIVATE_KEYS_MAPPING` | [Additional configuration](https://go-acme.github.io/lego/dns/myaddr) | | [MyDNS.jp](https://www.mydns.jp/) | `mydnsjp` | `MYDNSJP_MASTER_ID`, `MYDNSJP_PASSWORD` | [Additional configuration](https://go-acme.github.io/lego/dns/mydnsjp) | | [Mythic Beasts](https://www.mythic-beasts.com) | `mythicbeasts` | `MYTHICBEASTS_USER_NAME`, `MYTHICBEASTS_PASSWORD` | [Additional configuration](https://go-acme.github.io/lego/dns/mythicbeasts) | | [name.com](https://www.name.com/) | `namedotcom` | `NAMECOM_USERNAME`, `NAMECOM_API_TOKEN`, `NAMECOM_SERVER` | [Additional configuration](https://go-acme.github.io/lego/dns/namedotcom) | @@ -434,6 +435,7 @@ For complete details, refer to your provider's _Additional configuration_ link. | [Shellrent](https://www.shellrent.com) | `shellrent` | `SHELLRENT_USERNAME`, `SHELLRENT_TOKEN` | [Additional configuration](https://go-acme.github.io/lego/dns/shellrent) | | [Simply.com](https://www.simply.com/en/domains/) | `simply` | `SIMPLY_ACCOUNT_NAME`, `SIMPLY_API_KEY` | [Additional configuration](https://go-acme.github.io/lego/dns/simply) | | [Sonic](https://www.sonic.com/) | `sonic` | `SONIC_USER_ID`, `SONIC_API_KEY` | [Additional configuration](https://go-acme.github.io/lego/dns/sonic) | +| [Spaceship](https://spaceship.com) | `spaceship` | `SPACESHIP_API_KEY`, `SPACESHIP_API_SECRET` | [Additional configuration](https://go-acme.github.io/lego/dns/spaceship) | | [Stackpath](https://www.stackpath.com/) | `stackpath` | `STACKPATH_CLIENT_ID`, `STACKPATH_CLIENT_SECRET`, `STACKPATH_STACK_ID` | [Additional configuration](https://go-acme.github.io/lego/dns/stackpath) | | [Technitium](https://technitium.com) | `technitium` | `TECHNITIUM_SERVER_BASE_URL`, `TECHNITIUM_API_TOKEN` | [Additional configuration](https://go-acme.github.io/lego/dns/technitium) | | [Tencent Cloud DNS](https://cloud.tencent.com/product/cns) | `tencentcloud` | `TENCENTCLOUD_SECRET_ID`, `TENCENTCLOUD_SECRET_KEY` | [Additional configuration](https://go-acme.github.io/lego/dns/tencentcloud) | diff --git a/go.mod b/go.mod index 7356e9962..97eab6bf8 100644 --- a/go.mod +++ b/go.mod @@ -17,7 +17,7 @@ require ( github.com/fatih/structs v1.1.0 github.com/fsnotify/fsnotify v1.8.0 github.com/gambol99/go-marathon v0.0.0-20180614232016-99a156b96fb2 // No tag on the repo. - github.com/go-acme/lego/v4 v4.21.0 + github.com/go-acme/lego/v4 v4.22.2 github.com/go-kit/kit v0.13.0 github.com/go-kit/log v0.2.1 github.com/golang/protobuf v1.5.4 @@ -151,7 +151,6 @@ require ( github.com/containerd/log v0.1.0 // indirect github.com/containerd/platforms v0.2.1 // indirect github.com/coreos/go-semver v0.3.0 // indirect - github.com/cpu/goacmedns v0.1.1 // indirect github.com/cpuguy83/dockercfg v0.3.1 // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/deepmap/oapi-codegen v1.9.1 // indirect @@ -262,6 +261,7 @@ require ( github.com/nrdcg/desec v0.10.0 // indirect github.com/nrdcg/dnspod-go v0.4.0 // indirect github.com/nrdcg/freemyip v0.3.0 // indirect + github.com/nrdcg/goacmedns v0.2.0 // indirect github.com/nrdcg/goinwx v0.10.0 // indirect github.com/nrdcg/mailinabox v0.2.0 // indirect github.com/nrdcg/namesilo v0.2.1 // indirect diff --git a/go.sum b/go.sum index 46b093195..d7f4c241f 100644 --- a/go.sum +++ b/go.sum @@ -291,8 +291,6 @@ github.com/coreos/go-systemd/v22 v22.5.0 h1:RrqgGjYQKalulkV8NGVIfkXQf6YYmOyiJKk8 github.com/coreos/go-systemd/v22 v22.5.0/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc= github.com/coreos/pkg v0.0.0-20160727233714-3ac0863d7acf/go.mod h1:E3G3o1h8I7cfcXa63jLwjI0eiQQMgzzUDFVpN/nH/eA= github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f/go.mod h1:E3G3o1h8I7cfcXa63jLwjI0eiQQMgzzUDFVpN/nH/eA= -github.com/cpu/goacmedns v0.1.1 h1:DM3H2NiN2oam7QljgGY5ygy4yDXhK5Z4JUnqaugs2C4= -github.com/cpu/goacmedns v0.1.1/go.mod h1:MuaouqEhPAHxsbqjgnck5zeghuwBP1dLnPoobeGqugQ= github.com/cpuguy83/dockercfg v0.3.1 h1:/FpZ+JaygUR/lZP2NlFI2DVfrOEMAIKP5wWEJdoYe9E= github.com/cpuguy83/dockercfg v0.3.1/go.mod h1:sugsbF4//dDlL/i+S+rtpIWp+5h0BHJHfjj5/jFyUJc= github.com/cpuguy83/go-md2man/v2 v2.0.0-20190314233015-f79a8a8ca69d/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsrgA7czyZG/E6dU= @@ -413,8 +411,8 @@ github.com/ghodss/yaml v1.0.0 h1:wQHKEahhL6wmXdzwWG11gIVCkOv05bNOh+Rxn0yngAk= github.com/ghodss/yaml v1.0.0/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04= github.com/gin-contrib/sse v0.1.0/go.mod h1:RHrZQHXnP2xjPF+u1gW/2HnVO7nvIa9PG3Gm+fLHvGI= github.com/gin-gonic/gin v1.7.4/go.mod h1:jD2toBW3GZUr5UMcdrwQA10I7RuaFOl/SGeDjXkfUtY= -github.com/go-acme/lego/v4 v4.21.0 h1:arEW+8o5p7VI8Bk1kr/PDlgD1DrxtTH1gJ4b7mehL8o= -github.com/go-acme/lego/v4 v4.21.0/go.mod h1:HrSWzm3Ckj45Ie3i+p1zKVobbQoMOaGu9m4up0dUeDI= +github.com/go-acme/lego/v4 v4.22.2 h1:ck+HllWrV/rZGeYohsKQ5iKNnU/WAZxwOdiu6cxky+0= +github.com/go-acme/lego/v4 v4.22.2/go.mod h1:E2FndyI3Ekv0usNJt46mFb9LVpV/XBYT+4E3tz02Tzo= github.com/go-chi/chi/v5 v5.0.0/go.mod h1:BBug9lr0cqtdAhsu6R4AAdvufI0/XBzAQSsUqJpoZOs= github.com/go-cmd/cmd v1.0.5/go.mod h1:y8q8qlK5wQibcw63djSl/ntiHUHXHGdCkPk0j4QeW4s= github.com/go-errors/errors v1.0.1 h1:LUHzmkK3GUKUrL/1gfBUxAHzcev3apQlezX/+O7ma6w= @@ -1003,6 +1001,8 @@ github.com/nrdcg/dnspod-go v0.4.0 h1:c/jn1mLZNKF3/osJ6mz3QPxTudvPArXTjpkmYj0uK6U github.com/nrdcg/dnspod-go v0.4.0/go.mod h1:vZSoFSFeQVm2gWLMkyX61LZ8HI3BaqtHZWgPTGKr6KQ= github.com/nrdcg/freemyip v0.3.0 h1:0D2rXgvLwe2RRaVIjyUcQ4S26+cIS2iFwnhzDsEuuwc= github.com/nrdcg/freemyip v0.3.0/go.mod h1:c1PscDvA0ukBF0dwelU/IwOakNKnVxetpAQ863RMJoM= +github.com/nrdcg/goacmedns v0.2.0 h1:ADMbThobzEMnr6kg2ohs4KGa3LFqmgiBA22/6jUWJR0= +github.com/nrdcg/goacmedns v0.2.0/go.mod h1:T5o6+xvSLrQpugmwHvrSNkzWht0UGAwj2ACBMhh73Cg= github.com/nrdcg/goinwx v0.10.0 h1:6W630bjDxQD6OuXKqrFRYVpTt0G/9GXXm3CeOrN0zJM= github.com/nrdcg/goinwx v0.10.0/go.mod h1:mnMSTi7CXBu2io4DzdOBoGFA1XclD0sEPWJaDhNgkA4= github.com/nrdcg/mailinabox v0.2.0 h1:IKq8mfKiVwNW2hQii/ng1dJ4yYMMv3HAP3fMFIq2CFk= From c2a294c872aeae8415bcbb748a4d09de719a70b4 Mon Sep 17 00:00:00 2001 From: Kevin Pollet Date: Fri, 21 Feb 2025 10:52:04 +0100 Subject: [PATCH 4/6] Retry should send headers on Write Co-authored-by: Romain --- pkg/middlewares/retry/retry.go | 3 ++ pkg/middlewares/retry/retry_test.go | 55 +++++++++++++++++++---------- 2 files changed, 40 insertions(+), 18 deletions(-) diff --git a/pkg/middlewares/retry/retry.go b/pkg/middlewares/retry/retry.go index e1eee533d..5c95486d3 100644 --- a/pkg/middlewares/retry/retry.go +++ b/pkg/middlewares/retry/retry.go @@ -196,6 +196,9 @@ func (r *responseWriterWithoutCloseNotify) Write(buf []byte) (int, error) { if r.ShouldRetry() { return len(buf), nil } + if !r.written { + r.WriteHeader(http.StatusOK) + } return r.responseWriter.Write(buf) } diff --git a/pkg/middlewares/retry/retry_test.go b/pkg/middlewares/retry/retry_test.go index be2606ce4..8d493cf11 100644 --- a/pkg/middlewares/retry/retry_test.go +++ b/pkg/middlewares/retry/retry_test.go @@ -169,7 +169,6 @@ func TestRetryListeners(t *testing.T) { func TestMultipleRetriesShouldNotLooseHeaders(t *testing.T) { attempt := 0 - expectedHeaderName := "X-Foo-Test-2" expectedHeaderValue := "bar" next := http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { @@ -180,44 +179,55 @@ func TestMultipleRetriesShouldNotLooseHeaders(t *testing.T) { return } - // Request has been successfully written to backend + // Request has been successfully written to backend. trace := httptrace.ContextClientTrace(req.Context()) trace.WroteHeaders() - // And we decide to answer to client + // And we decide to answer to client. rw.WriteHeader(http.StatusNoContent) }) retry, err := New(context.Background(), next, dynamic.Retry{Attempts: 3}, &countingRetryListener{}, "traefikTest") require.NoError(t, err) - responseRecorder := httptest.NewRecorder() - retry.ServeHTTP(responseRecorder, testhelpers.MustNewRequest(http.MethodGet, "http://test", http.NoBody)) + res := httptest.NewRecorder() + retry.ServeHTTP(res, testhelpers.MustNewRequest(http.MethodGet, "http://test", http.NoBody)) - headerValue := responseRecorder.Header().Get(expectedHeaderName) - - // Validate if we have the correct header - if headerValue != expectedHeaderValue { - t.Errorf("Expected to have %s for header %s, got %s", expectedHeaderValue, expectedHeaderName, headerValue) - } + // The third header attempt is kept. + headerValue := res.Header().Get("X-Foo-Test-2") + assert.Equal(t, expectedHeaderValue, headerValue) // Validate that we don't have headers from previous attempts for i := range attempt { headerName := fmt.Sprintf("X-Foo-Test-%d", i) - headerValue = responseRecorder.Header().Get("headerName") + headerValue = res.Header().Get(headerName) if headerValue != "" { t.Errorf("Expected no value for header %s, got %s", headerName, headerValue) } } } -// countingRetryListener is a Listener implementation to count the times the Retried fn is called. -type countingRetryListener struct { - timesCalled int -} +func TestRetryShouldNotLooseHeadersOnWrite(t *testing.T) { + next := http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + rw.Header().Add("X-Foo-Test", "bar") -func (l *countingRetryListener) Retried(req *http.Request, attempt int) { - l.timesCalled++ + // Request has been successfully written to backend. + trace := httptrace.ContextClientTrace(req.Context()) + trace.WroteHeaders() + + // And we decide to answer to client without calling WriteHeader. + _, err := rw.Write([]byte("bar")) + require.NoError(t, err) + }) + + retry, err := New(context.Background(), next, dynamic.Retry{Attempts: 3}, &countingRetryListener{}, "traefikTest") + require.NoError(t, err) + + res := httptest.NewRecorder() + retry.ServeHTTP(res, testhelpers.MustNewRequest(http.MethodGet, "http://test", http.NoBody)) + + headerValue := res.Header().Get("X-Foo-Test") + assert.Equal(t, "bar", headerValue) } func TestRetryWithFlush(t *testing.T) { @@ -387,3 +397,12 @@ func Test1xxResponses(t *testing.T) { assert.Equal(t, 0, retryListener.timesCalled) } + +// countingRetryListener is a Listener implementation to count the times the Retried fn is called. +type countingRetryListener struct { + timesCalled int +} + +func (l *countingRetryListener) Retried(req *http.Request, attempt int) { + l.timesCalled++ +} From f196de90e1bc029db23ed9b012ceb5007750348e Mon Sep 17 00:00:00 2001 From: Kevin Pollet Date: Fri, 21 Feb 2025 11:36:05 +0100 Subject: [PATCH 5/6] Enable the retry middleware in the proxy Co-authored-by: Romain --- pkg/middlewares/retry/retry.go | 87 ++++++++++++++++++----------- pkg/middlewares/retry/retry_test.go | 63 ++++++++++++--------- pkg/server/service/service.go | 13 +++-- 3 files changed, 99 insertions(+), 64 deletions(-) diff --git a/pkg/middlewares/retry/retry.go b/pkg/middlewares/retry/retry.go index 5c95486d3..3e8167ba9 100644 --- a/pkg/middlewares/retry/retry.go +++ b/pkg/middlewares/retry/retry.go @@ -37,6 +37,48 @@ type Listener interface { // each of them about a retry attempt. type Listeners []Listener +// Retried exists to implement the Listener interface. It calls Retried on each of its slice entries. +func (l Listeners) Retried(req *http.Request, attempt int) { + for _, listener := range l { + listener.Retried(req, attempt) + } +} + +type shouldRetryContextKey struct{} + +// ShouldRetry is a function allowing to enable/disable the retry middleware mechanism. +type ShouldRetry func(shouldRetry bool) + +// ContextShouldRetry returns the ShouldRetry function if it has been set by the Retry middleware in the chain. +func ContextShouldRetry(ctx context.Context) ShouldRetry { + f, _ := ctx.Value(shouldRetryContextKey{}).(ShouldRetry) + return f +} + +// WrapHandler wraps a given http.Handler to inject the httptrace.ClientTrace in the request context when it is needed +// by the retry middleware. +func WrapHandler(next http.Handler) http.Handler { + return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + if shouldRetry := ContextShouldRetry(req.Context()); shouldRetry != nil { + shouldRetry(true) + + trace := &httptrace.ClientTrace{ + WroteHeaders: func() { + shouldRetry(false) + }, + WroteRequest: func(httptrace.WroteRequestInfo) { + shouldRetry(false) + }, + } + newCtx := httptrace.WithClientTrace(req.Context(), trace) + next.ServeHTTP(rw, req.WithContext(newCtx)) + return + } + + next.ServeHTTP(rw, req) + }) +} + // retry is a middleware that retries requests. type retry struct { attempts int @@ -83,19 +125,13 @@ func (r *retry) ServeHTTP(rw http.ResponseWriter, req *http.Request) { attempts := 1 operation := func() error { - shouldRetry := attempts < r.attempts - retryResponseWriter := newResponseWriter(rw, shouldRetry) + remainAttempts := attempts < r.attempts + retryResponseWriter := newResponseWriter(rw) - // Disable retries when the backend already received request data - trace := &httptrace.ClientTrace{ - WroteHeaders: func() { - retryResponseWriter.DisableRetries() - }, - WroteRequest: func(httptrace.WroteRequestInfo) { - retryResponseWriter.DisableRetries() - }, + var shouldRetry ShouldRetry = func(shouldRetry bool) { + retryResponseWriter.SetShouldRetry(remainAttempts && shouldRetry) } - newCtx := httptrace.WithClientTrace(req.Context(), trace) + newCtx := context.WithValue(req.Context(), shouldRetryContextKey{}, shouldRetry) r.next.ServeHTTP(retryResponseWriter, req.Clone(newCtx)) @@ -142,25 +178,17 @@ func (r *retry) newBackOff() backoff.BackOff { return b } -// Retried exists to implement the Listener interface. It calls Retried on each of its slice entries. -func (l Listeners) Retried(req *http.Request, attempt int) { - for _, listener := range l { - listener.Retried(req, attempt) - } -} - type responseWriter interface { http.ResponseWriter http.Flusher ShouldRetry() bool - DisableRetries() + SetShouldRetry(shouldRetry bool) } -func newResponseWriter(rw http.ResponseWriter, shouldRetry bool) responseWriter { +func newResponseWriter(rw http.ResponseWriter) responseWriter { responseWriter := &responseWriterWithoutCloseNotify{ responseWriter: rw, headers: make(http.Header), - shouldRetry: shouldRetry, } if _, ok := rw.(http.CloseNotifier); ok { return &responseWriterWithCloseNotify{ @@ -181,8 +209,8 @@ func (r *responseWriterWithoutCloseNotify) ShouldRetry() bool { return r.shouldRetry } -func (r *responseWriterWithoutCloseNotify) DisableRetries() { - r.shouldRetry = false +func (r *responseWriterWithoutCloseNotify) SetShouldRetry(shouldRetry bool) { + r.shouldRetry = shouldRetry } func (r *responseWriterWithoutCloseNotify) Header() http.Header { @@ -193,7 +221,7 @@ func (r *responseWriterWithoutCloseNotify) Header() http.Header { } func (r *responseWriterWithoutCloseNotify) Write(buf []byte) (int, error) { - if r.ShouldRetry() { + if r.shouldRetry { return len(buf), nil } if !r.written { @@ -203,16 +231,7 @@ func (r *responseWriterWithoutCloseNotify) Write(buf []byte) (int, error) { } func (r *responseWriterWithoutCloseNotify) WriteHeader(code int) { - if r.ShouldRetry() && code == http.StatusServiceUnavailable { - // We get a 503 HTTP Status Code when there is no backend server in the pool - // to which the request could be sent. Also, note that r.ShouldRetry() - // will never return true in case there was a connection established to - // the backend server and so we can be sure that the 503 was produced - // inside Traefik already and we don't have to retry in this cases. - r.DisableRetries() - } - - if r.ShouldRetry() || r.written { + if r.shouldRetry || r.written { return } diff --git a/pkg/middlewares/retry/retry_test.go b/pkg/middlewares/retry/retry_test.go index 8d493cf11..b7332fe1e 100644 --- a/pkg/middlewares/retry/retry_test.go +++ b/pkg/middlewares/retry/retry_test.go @@ -105,12 +105,21 @@ func TestRetry(t *testing.T) { t.Parallel() retryAttempts := 0 - next := http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + next := http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + // This signals that a connection will be established with the backend + // to enable the Retry middleware mechanism. + shouldRetry := ContextShouldRetry(req.Context()) + if shouldRetry != nil { + shouldRetry(true) + } + retryAttempts++ if retryAttempts > test.amountFaultyEndpoints { - // calls WroteHeaders on httptrace. - _ = r.Write(io.Discard) + // This signals that request headers have been sent to the backend. + if shouldRetry != nil { + shouldRetry(false) + } rw.WriteHeader(http.StatusOK) return @@ -152,26 +161,16 @@ func TestRetryEmptyServerList(t *testing.T) { assert.Equal(t, 0, retryListener.timesCalled) } -func TestRetryListeners(t *testing.T) { - req := httptest.NewRequest(http.MethodGet, "/", nil) - retryListeners := Listeners{&countingRetryListener{}, &countingRetryListener{}} - - retryListeners.Retried(req, 1) - retryListeners.Retried(req, 1) - - for _, retryListener := range retryListeners { - listener := retryListener.(*countingRetryListener) - if listener.timesCalled != 2 { - t.Errorf("retry listener was called %d time(s), want %d time(s)", listener.timesCalled, 2) - } - } -} - func TestMultipleRetriesShouldNotLooseHeaders(t *testing.T) { attempt := 0 expectedHeaderValue := "bar" next := http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + shouldRetry := ContextShouldRetry(req.Context()) + if shouldRetry != nil { + shouldRetry(true) + } + headerName := fmt.Sprintf("X-Foo-Test-%d", attempt) rw.Header().Add(headerName, expectedHeaderValue) if attempt < 2 { @@ -179,9 +178,8 @@ func TestMultipleRetriesShouldNotLooseHeaders(t *testing.T) { return } - // Request has been successfully written to backend. - trace := httptrace.ContextClientTrace(req.Context()) - trace.WroteHeaders() + // Request has been successfully written to backend + shouldRetry(false) // And we decide to answer to client. rw.WriteHeader(http.StatusNoContent) @@ -212,9 +210,10 @@ func TestRetryShouldNotLooseHeadersOnWrite(t *testing.T) { rw.Header().Add("X-Foo-Test", "bar") // Request has been successfully written to backend. - trace := httptrace.ContextClientTrace(req.Context()) - trace.WroteHeaders() - + shouldRetry := ContextShouldRetry(req.Context()) + if shouldRetry != nil { + shouldRetry(false) + } // And we decide to answer to client without calling WriteHeader. _, err := rw.Write([]byte("bar")) require.NoError(t, err) @@ -285,12 +284,24 @@ func TestRetryWebsocket(t *testing.T) { t.Parallel() retryAttempts := 0 - next := http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + next := http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + // This signals that a connection will be established with the backend + // to enable the Retry middleware mechanism. + shouldRetry := ContextShouldRetry(req.Context()) + if shouldRetry != nil { + shouldRetry(true) + } + retryAttempts++ if retryAttempts > test.amountFaultyEndpoints { + // This signals that request headers have been sent to the backend. + if shouldRetry != nil { + shouldRetry(false) + } + upgrader := websocket.Upgrader{} - _, err := upgrader.Upgrade(rw, r, nil) + _, err := upgrader.Upgrade(rw, req, nil) if err != nil { http.Error(rw, err.Error(), http.StatusInternalServerError) } diff --git a/pkg/server/service/service.go b/pkg/server/service/service.go index f30770b62..0a13bb521 100644 --- a/pkg/server/service/service.go +++ b/pkg/server/service/service.go @@ -21,6 +21,7 @@ import ( "github.com/traefik/traefik/v2/pkg/middlewares/emptybackendhandler" metricsMiddle "github.com/traefik/traefik/v2/pkg/middlewares/metrics" "github.com/traefik/traefik/v2/pkg/middlewares/pipelining" + "github.com/traefik/traefik/v2/pkg/middlewares/retry" "github.com/traefik/traefik/v2/pkg/safe" "github.com/traefik/traefik/v2/pkg/server/cookie" "github.com/traefik/traefik/v2/pkg/server/provider" @@ -283,16 +284,20 @@ func (m *Manager) getLoadBalancerServiceHandler(ctx context.Context, serviceName if err != nil { return nil, err } + // The retry wrapping must be done just before the proxy handler, + // to make sure that the retry will not be triggered/disabled by + // middlewares in the chain. + fwd = retry.WrapHandler(fwd) - alHandler := func(next http.Handler) (http.Handler, error) { - return accesslog.NewFieldHandler(next, accesslog.ServiceName, serviceName, accesslog.AddServiceFields), nil - } chain := alice.New() if m.metricsRegistry != nil && m.metricsRegistry.IsSvcEnabled() { chain = chain.Append(metricsMiddle.WrapServiceHandler(ctx, m.metricsRegistry, serviceName)) } + chain = chain.Append(func(next http.Handler) (http.Handler, error) { + return accesslog.NewFieldHandler(next, accesslog.ServiceName, serviceName, accesslog.AddServiceFields), nil + }) - handler, err := chain.Append(alHandler).Then(pipelining.New(ctx, fwd, "pipelining")) + handler, err := chain.Then(pipelining.New(ctx, fwd, "pipelining")) if err != nil { return nil, err } From a3fd48472841e5818400549da3cd813d3bed16d6 Mon Sep 17 00:00:00 2001 From: Kevin Pollet Date: Mon, 24 Feb 2025 15:32:06 +0100 Subject: [PATCH 6/6] Prepare release v2.11.21 --- CHANGELOG.md | 9 +++++++++ script/gcg/traefik-bugfix.toml | 6 +++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f5d39ff06..3a48bd088 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,12 @@ +# [v2.11.21](https://github.com/traefik/traefik/tree/v2.11.21) (2025-02-24) +[All Commits](https://github.com/traefik/traefik/compare/v2.11.20...v2.11.21) + +**Bug fixes:** +- **[acme]** Bump github.com/go-acme/lego/v4 to v4.22.2 ([#11537](https://github.com/traefik/traefik/pull/11537) by [ldez](https://github.com/ldez)) +- **[cli]** Bump github.com/traefik/paerser to v0.2.2 ([#11530](https://github.com/traefik/traefik/pull/11530) by [kevinpollet](https://github.com/kevinpollet)) +- **[middleware]** Enable the retry middleware in the proxy ([#11536](https://github.com/traefik/traefik/pull/11536) by [kevinpollet](https://github.com/kevinpollet)) +- **[middleware]** Retry should send headers on Write ([#11534](https://github.com/traefik/traefik/pull/11534) by [kevinpollet](https://github.com/kevinpollet)) + ## [v2.11.20](https://github.com/traefik/traefik/tree/v2.11.20) (2025-01-31) [All Commits](https://github.com/traefik/traefik/compare/v2.11.19...v2.11.20) diff --git a/script/gcg/traefik-bugfix.toml b/script/gcg/traefik-bugfix.toml index 77ae402b1..9f83b4d2e 100644 --- a/script/gcg/traefik-bugfix.toml +++ b/script/gcg/traefik-bugfix.toml @@ -4,11 +4,11 @@ RepositoryName = "traefik" OutputType = "file" FileName = "traefik_changelog.md" -# example new bugfix v2.11.20 +# example new bugfix v2.11.21 CurrentRef = "v2.11" -PreviousRef = "v2.11.19" +PreviousRef = "v2.11.20" BaseBranch = "v2.11" -FutureCurrentRefName = "v2.11.20" +FutureCurrentRefName = "v2.11.21" ThresholdPreviousRef = 10 ThresholdCurrentRef = 10