diff --git a/integration/access_log_test.go b/integration/access_log_test.go index b5e4ec1ef..e1c4003e9 100644 --- a/integration/access_log_test.go +++ b/integration/access_log_test.go @@ -606,10 +606,8 @@ func (s *AccessLogSuite) TestAccessLogPreflightHeadersMiddleware() { func (s *AccessLogSuite) TestAccessLogDisabledForInternals() { ensureWorkingDirectoryIsClean() - file := s.adaptFile("fixtures/access_log/access_log_ping.toml", struct{}{}) - // Start Traefik. - s.traefikCmd(withConfigFile(file)) + s.traefikCmd(withConfigFile("fixtures/access_log/access_log_base.toml")) defer func() { traefikLog, err := os.ReadFile(traefikTestLogFile) @@ -619,7 +617,7 @@ func (s *AccessLogSuite) TestAccessLogDisabledForInternals() { // waitForTraefik makes at least one call to the rawdata api endpoint, // but the logs for this endpoint are ignored in checkAccessLogOutput. - s.waitForTraefik("customPing") + s.waitForTraefik("service3") s.checkStatsForLogFile() @@ -636,8 +634,9 @@ func (s *AccessLogSuite) TestAccessLogDisabledForInternals() { require.NoError(s.T(), err) // Make some requests on the custom ping router. - req, err = http.NewRequest(http.MethodGet, "http://127.0.0.1:8000/ping", nil) + req, err = http.NewRequest(http.MethodGet, "http://127.0.0.1:8010/ping", nil) require.NoError(s.T(), err) + req.Host = "ping.docker.local" err = try.Request(req, 500*time.Millisecond, try.StatusCodeIs(http.StatusOK), try.HasBody()) require.NoError(s.T(), err) @@ -649,6 +648,25 @@ func (s *AccessLogSuite) TestAccessLogDisabledForInternals() { require.Equal(s.T(), 0, count) + // Make some requests on the custom ping router in error. + req, err = http.NewRequest(http.MethodGet, "http://127.0.0.1:8010/ping-error", nil) + require.NoError(s.T(), err) + req.Host = "ping-error.docker.local" + + err = try.Request(req, 500*time.Millisecond, try.StatusCodeIs(http.StatusUnauthorized), try.BodyContains("X-Forwarded-Host: ping-error.docker.local")) + require.NoError(s.T(), err) + err = try.Request(req, 500*time.Millisecond, try.StatusCodeIs(http.StatusUnauthorized), try.BodyContains("X-Forwarded-Host: ping-error.docker.local")) + require.NoError(s.T(), err) + + // Here we verify that the remove of observability doesn't break the metrics for the error page service. + req, err = http.NewRequest(http.MethodGet, "http://127.0.0.1:8080/metrics", nil) + require.NoError(s.T(), err) + + err = try.Request(req, 500*time.Millisecond, try.StatusCodeIs(http.StatusOK), try.BodyContains("service3")) + require.NoError(s.T(), err) + err = try.Request(req, 500*time.Millisecond, try.StatusCodeIs(http.StatusOK), try.BodyNotContains("service=\"ping")) + require.NoError(s.T(), err) + // Verify no other Traefik problems. s.checkNoOtherTraefikProblems() } diff --git a/integration/fixtures/access_log/access_log_base.toml b/integration/fixtures/access_log/access_log_base.toml index 8d954450e..5a88f75e9 100644 --- a/integration/fixtures/access_log/access_log_base.toml +++ b/integration/fixtures/access_log/access_log_base.toml @@ -22,10 +22,17 @@ address = ":8008" [entryPoints.preflight] address = ":8009" + [entryPoints.ping] + address = ":8010" [api] insecure = true +[ping] + +[metrics] + [metrics.prometheus] + [providers] [providers.docker] exposedByDefault = false diff --git a/integration/fixtures/access_log/access_log_ping.toml b/integration/fixtures/access_log/access_log_ping.toml deleted file mode 100644 index 4e85b93bc..000000000 --- a/integration/fixtures/access_log/access_log_ping.toml +++ /dev/null @@ -1,30 +0,0 @@ -[global] - checkNewVersion = false - sendAnonymousUsage = false - -[log] - level = "ERROR" - filePath = "traefik.log" - -[accessLog] - filePath = "access.log" - -[entryPoints] - [entryPoints.web] - address = ":8000" - -[api] - insecure = true - -[ping] - -[providers] - [providers.file] - filename = "{{ .SelfFilename }}" - -## dynamic configuration ## -[http.routers] - [http.routers.customPing] - entryPoints = ["web"] - rule = "PathPrefix(`/ping`)" - service = "ping@internal" diff --git a/integration/resources/compose/access_log.yml b/integration/resources/compose/access_log.yml index c9cf06db7..c5be8f9e4 100644 --- a/integration/resources/compose/access_log.yml +++ b/integration/resources/compose/access_log.yml @@ -94,3 +94,21 @@ services: traefik.http.routers.rt-preflightCORS.middlewares: preflightCORS traefik.http.middlewares.preflightCORS.headers.accessControlAllowMethods: OPTIONS, GET traefik.http.services.preflightCORS.loadbalancer.server.port: 80 + + ping: + image: traefik/whoami + labels: + traefik.enable: true + traefik.http.routers.ping.entryPoints: ping + traefik.http.routers.ping.rule: PathPrefix(`/ping`) + traefik.http.routers.ping.service: ping@internal + + traefik.http.routers.ping-error.entryPoints: ping + traefik.http.routers.ping-error.rule: PathPrefix(`/ping-error`) + traefik.http.routers.ping-error.middlewares: errors, basicauth + traefik.http.routers.ping-error.service: ping@internal + traefik.http.middlewares.basicauth.basicauth.users: "test:$apr1$H6uskkkW$IgXLP6ewTrSuBkTrqE8wj/" + traefik.http.middlewares.errors.errors.status: 401 + traefik.http.middlewares.errors.errors.service: service3 + traefik.http.middlewares.errors.errors.query: / + traefik.http.services.service3.loadbalancer.server.port: 80 diff --git a/pkg/middlewares/capture/capture.go b/pkg/middlewares/capture/capture.go index b7e182117..04e411a20 100644 --- a/pkg/middlewares/capture/capture.go +++ b/pkg/middlewares/capture/capture.go @@ -43,9 +43,21 @@ const capturedData key = "capturedData" // It satisfies the alice.Constructor type. func Wrap(next http.Handler) (http.Handler, error) { return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { - c := &Capture{} - newRW, newReq := c.renew(rw, req) - next.ServeHTTP(newRW, newReq) + capt, err := FromContext(req.Context()) + if err != nil { + c := &Capture{} + newRW, newReq := c.renew(rw, req) + next.ServeHTTP(newRW, newReq) + return + } + + if capt.NeedsReset(rw) { + newRW, newReq := capt.renew(rw, req) + next.ServeHTTP(newRW, newReq) + return + } + + next.ServeHTTP(rw, req) }), nil } diff --git a/pkg/server/service/service.go b/pkg/server/service/service.go index e77fda54c..5003273d9 100644 --- a/pkg/server/service/service.go +++ b/pkg/server/service/service.go @@ -21,6 +21,7 @@ import ( "github.com/traefik/traefik/v3/pkg/healthcheck" "github.com/traefik/traefik/v3/pkg/logs" "github.com/traefik/traefik/v3/pkg/middlewares/accesslog" + "github.com/traefik/traefik/v3/pkg/middlewares/capture" metricsMiddle "github.com/traefik/traefik/v3/pkg/middlewares/metrics" "github.com/traefik/traefik/v3/pkg/middlewares/observability" "github.com/traefik/traefik/v3/pkg/safe" @@ -342,6 +343,17 @@ func (m *Manager) getLoadBalancerServiceHandler(ctx context.Context, serviceName proxy = observability.NewService(ctx, serviceName, proxy) } + if m.observabilityMgr.ShouldAddAccessLogs(qualifiedSvcName) || m.observabilityMgr.ShouldAddMetrics(qualifiedSvcName) { + // Some piece of middleware, like the ErrorPage, are relying on this serviceBuilder to get the handler for a given service, + // to re-target the request to it. + // Those pieces of middleware can be configured on routes that expose a Traefik internal service. + // In such a case, observability for internals being optional, the capture probe could be absent from context (no wrap via the entrypoint). + // But if the service targeted by this piece of middleware is not an internal one, + // and requires observability, we still want the capture probe to be present in the request context. + // Makes sure a capture probe is in the request context. + proxy, _ = capture.Wrap(proxy) + } + lb.Add(proxyName, proxy, server.Weight) // servers are considered UP by default.