From 18d90ecd96b1501d098a92c815be4911ec45d41c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robin=20M=C3=BCller?= Date: Wed, 26 Feb 2020 17:28:04 +0100 Subject: [PATCH] Do not follow redirects for the health check URLs --- .../dynamic-configuration/docker-labels.yml | 1 + .../reference/dynamic-configuration/file.toml | 1 + .../reference/dynamic-configuration/file.yaml | 1 + .../reference/dynamic-configuration/kv-ref.md | 1 + .../marathon-labels.json | 1 + .../routing/providers/consul-catalog.md | 8 +++ docs/content/routing/providers/docker.md | 8 +++ docs/content/routing/providers/marathon.md | 8 +++ docs/content/routing/providers/rancher.md | 8 +++ docs/content/routing/services/index.md | 1 + pkg/config/dynamic/http_config.go | 13 +++-- pkg/config/label/label_test.go | 4 ++ pkg/healthcheck/healthcheck.go | 27 ++++++---- pkg/healthcheck/healthcheck_test.go | 54 +++++++++++++++++++ pkg/provider/kv/kv_test.go | 14 ++--- pkg/server/service/service.go | 22 +++++--- 16 files changed, 145 insertions(+), 27 deletions(-) diff --git a/docs/content/reference/dynamic-configuration/docker-labels.yml b/docs/content/reference/dynamic-configuration/docker-labels.yml index dc3fd18f0..26be2a93f 100644 --- a/docs/content/reference/dynamic-configuration/docker-labels.yml +++ b/docs/content/reference/dynamic-configuration/docker-labels.yml @@ -142,6 +142,7 @@ - "traefik.http.services.service01.loadbalancer.healthcheck.port=42" - "traefik.http.services.service01.loadbalancer.healthcheck.scheme=foobar" - "traefik.http.services.service01.loadbalancer.healthcheck.timeout=foobar" +- "traefik.http.services.service01.loadbalancer.healthcheck.followredirects=true" - "traefik.http.services.service01.loadbalancer.passhostheader=true" - "traefik.http.services.service01.loadbalancer.responseforwarding.flushinterval=foobar" - "traefik.http.services.service01.loadbalancer.sticky=true" diff --git a/docs/content/reference/dynamic-configuration/file.toml b/docs/content/reference/dynamic-configuration/file.toml index 8cba1cbe1..8fa8c21ee 100644 --- a/docs/content/reference/dynamic-configuration/file.toml +++ b/docs/content/reference/dynamic-configuration/file.toml @@ -56,6 +56,7 @@ interval = "foobar" timeout = "foobar" hostname = "foobar" + followRedirects = true [http.services.Service01.loadBalancer.healthCheck.headers] name0 = "foobar" name1 = "foobar" diff --git a/docs/content/reference/dynamic-configuration/file.yaml b/docs/content/reference/dynamic-configuration/file.yaml index e88da86e6..a7e946551 100644 --- a/docs/content/reference/dynamic-configuration/file.yaml +++ b/docs/content/reference/dynamic-configuration/file.yaml @@ -62,6 +62,7 @@ http: interval: foobar timeout: foobar hostname: foobar + followRedirects: true headers: name0: foobar name1: foobar diff --git a/docs/content/reference/dynamic-configuration/kv-ref.md b/docs/content/reference/dynamic-configuration/kv-ref.md index d7aaba381..318d9cc4d 100644 --- a/docs/content/reference/dynamic-configuration/kv-ref.md +++ b/docs/content/reference/dynamic-configuration/kv-ref.md @@ -156,6 +156,7 @@ | `traefik/http/routers/Router1/tls/domains/1/sans/0` | `foobar` | | `traefik/http/routers/Router1/tls/domains/1/sans/1` | `foobar` | | `traefik/http/routers/Router1/tls/options` | `foobar` | +| `traefik/http/services/Service01/loadBalancer/healthCheck/followRedirects` | `true` | | `traefik/http/services/Service01/loadBalancer/healthCheck/headers/name0` | `foobar` | | `traefik/http/services/Service01/loadBalancer/healthCheck/headers/name1` | `foobar` | | `traefik/http/services/Service01/loadBalancer/healthCheck/hostname` | `foobar` | diff --git a/docs/content/reference/dynamic-configuration/marathon-labels.json b/docs/content/reference/dynamic-configuration/marathon-labels.json index ca11e5a1d..cb465730d 100644 --- a/docs/content/reference/dynamic-configuration/marathon-labels.json +++ b/docs/content/reference/dynamic-configuration/marathon-labels.json @@ -140,6 +140,7 @@ "traefik.http.services.service01.loadbalancer.healthcheck.port": "42", "traefik.http.services.service01.loadbalancer.healthcheck.scheme": "foobar", "traefik.http.services.service01.loadbalancer.healthcheck.timeout": "foobar", +"traefik.http.services.service01.loadbalancer.healthcheck.followredirects": "true", "traefik.http.services.service01.loadbalancer.passhostheader": "true", "traefik.http.services.service01.loadbalancer.responseforwarding.flushinterval": "foobar", "traefik.http.services.service01.loadbalancer.sticky.cookie.httponly": "true", diff --git a/docs/content/routing/providers/consul-catalog.md b/docs/content/routing/providers/consul-catalog.md index 835a8899f..56294f472 100644 --- a/docs/content/routing/providers/consul-catalog.md +++ b/docs/content/routing/providers/consul-catalog.md @@ -193,6 +193,14 @@ you'd add the tag `traefik.http.services.{name-of-your-choice}.loadbalancer.pass traefik.http.services.myservice.loadbalancer.healthcheck.timeout=10 ``` +??? info "`traefik.http.services..loadbalancer.healthcheck.followredirects`" + + See [health check](../services/index.md#health-check) for more information. + + ```yaml + traefik.http.services.myservice.loadbalancer.healthcheck.followredirects=true + ``` + ??? info "`traefik.http.services..loadbalancer.sticky`" See [sticky sessions](../services/index.md#sticky-sessions) for more information. diff --git a/docs/content/routing/providers/docker.md b/docs/content/routing/providers/docker.md index b0ad61431..fffd71a4a 100644 --- a/docs/content/routing/providers/docker.md +++ b/docs/content/routing/providers/docker.md @@ -326,6 +326,14 @@ you'd add the label `traefik.http.services..loadbalancer.pa - "traefik.http.services.myservice.loadbalancer.healthcheck.timeout=10" ``` +??? info "`traefik.http.services..loadbalancer.healthcheck.followredirects`" + + See [health check](../services/index.md#health-check) for more information. + + ```yaml + - "traefik.http.services.myservice.loadbalancer.healthcheck.followredirects=true" + ``` + ??? info "`traefik.http.services..loadbalancer.sticky`" See [sticky sessions](../services/index.md#sticky-sessions) for more information. diff --git a/docs/content/routing/providers/marathon.md b/docs/content/routing/providers/marathon.md index 3c1600a8d..3b158772d 100644 --- a/docs/content/routing/providers/marathon.md +++ b/docs/content/routing/providers/marathon.md @@ -224,6 +224,14 @@ For example, to change the passHostHeader behavior, you'd add the label `"traefi "traefik.http.services.myservice.loadbalancer.healthcheck.timeout": "10" ``` +??? info "`traefik.http.services..loadbalancer.healthcheck.followredirects`" + + See [health check](../services/index.md#health-check) for more information. + + ```json + "traefik.http.services.myservice.loadbalancer.healthcheck.followredirects": "true" + ``` + ??? info "`traefik.http.services..loadbalancer.sticky`" See [sticky sessions](../services/index.md#sticky-sessions) for more information. diff --git a/docs/content/routing/providers/rancher.md b/docs/content/routing/providers/rancher.md index 4b0cbd9bc..88e3d91d5 100644 --- a/docs/content/routing/providers/rancher.md +++ b/docs/content/routing/providers/rancher.md @@ -230,6 +230,14 @@ you'd add the label `traefik.http.services.{name-of-your-choice}.loadbalancer.pa - "traefik.http.services.myservice.loadbalancer.healthcheck.timeout=10" ``` +??? info "`traefik.http.services..loadbalancer.healthcheck.followredirects`" + + See [health check](../services/index.md#health-check) for more information. + + ```yaml + - "traefik.http.services.myservice.loadbalancer.healthcheck.followredirects=true" + ``` + ??? info "`traefik.http.services..loadbalancer.sticky`" See [sticky sessions](../services/index.md#sticky-sessions) for more information. diff --git a/docs/content/routing/services/index.md b/docs/content/routing/services/index.md index 281df7aae..067ff63a5 100644 --- a/docs/content/routing/services/index.md +++ b/docs/content/routing/services/index.md @@ -240,6 +240,7 @@ Below are the available options for the health check mechanism: - `interval` defines the frequency of the health check calls. - `timeout` defines the maximum duration Traefik will wait for a health check request before considering the server failed (unhealthy). - `headers` defines custom headers to be sent to the health check endpoint. +- `followRedirects` defines whether redirects should be followed during the health check calls (default: true). !!! info "Interval & Timeout Format" diff --git a/pkg/config/dynamic/http_config.go b/pkg/config/dynamic/http_config.go index c98e10697..2f743ba55 100644 --- a/pkg/config/dynamic/http_config.go +++ b/pkg/config/dynamic/http_config.go @@ -164,7 +164,14 @@ type HealthCheck struct { // FIXME change string to types.Duration Interval string `json:"interval,omitempty" toml:"interval,omitempty" yaml:"interval,omitempty"` // FIXME change string to types.Duration - Timeout string `json:"timeout,omitempty" toml:"timeout,omitempty" yaml:"timeout,omitempty"` - Hostname string `json:"hostname,omitempty" toml:"hostname,omitempty" yaml:"hostname,omitempty"` - Headers map[string]string `json:"headers,omitempty" toml:"headers,omitempty" yaml:"headers,omitempty"` + Timeout string `json:"timeout,omitempty" toml:"timeout,omitempty" yaml:"timeout,omitempty"` + Hostname string `json:"hostname,omitempty" toml:"hostname,omitempty" yaml:"hostname,omitempty"` + FollowRedirects *bool `json:"followRedirects" toml:"followRedirects" yaml:"followRedirects"` + Headers map[string]string `json:"headers,omitempty" toml:"headers,omitempty" yaml:"headers,omitempty"` +} + +// SetDefaults Default values for a HealthCheck. +func (h *HealthCheck) SetDefaults() { + fr := true + h.FollowRedirects = &fr } diff --git a/pkg/config/label/label_test.go b/pkg/config/label/label_test.go index b4d74d789..56d73a753 100644 --- a/pkg/config/label/label_test.go +++ b/pkg/config/label/label_test.go @@ -143,6 +143,7 @@ func TestDecodeConfiguration(t *testing.T) { "traefik.http.services.Service0.loadbalancer.healthcheck.port": "42", "traefik.http.services.Service0.loadbalancer.healthcheck.scheme": "foobar", "traefik.http.services.Service0.loadbalancer.healthcheck.timeout": "foobar", + "traefik.http.services.Service0.loadbalancer.healthcheck.followredirects": "true", "traefik.http.services.Service0.loadbalancer.passhostheader": "true", "traefik.http.services.Service0.loadbalancer.responseforwarding.flushinterval": "foobar", "traefik.http.services.Service0.loadbalancer.server.scheme": "foobar", @@ -157,6 +158,7 @@ func TestDecodeConfiguration(t *testing.T) { "traefik.http.services.Service1.loadbalancer.healthcheck.port": "42", "traefik.http.services.Service1.loadbalancer.healthcheck.scheme": "foobar", "traefik.http.services.Service1.loadbalancer.healthcheck.timeout": "foobar", + "traefik.http.services.Service1.loadbalancer.healthcheck.followredirects": "true", "traefik.http.services.Service1.loadbalancer.passhostheader": "true", "traefik.http.services.Service1.loadbalancer.responseforwarding.flushinterval": "foobar", "traefik.http.services.Service1.loadbalancer.server.scheme": "foobar", @@ -595,6 +597,7 @@ func TestDecodeConfiguration(t *testing.T) { "name0": "foobar", "name1": "foobar", }, + FollowRedirects: func(v bool) *bool { return &v }(true), }, PassHostHeader: func(v bool) *bool { return &v }(true), ResponseForwarding: &dynamic.ResponseForwarding{ @@ -621,6 +624,7 @@ func TestDecodeConfiguration(t *testing.T) { "name0": "foobar", "name1": "foobar", }, + FollowRedirects: func(v bool) *bool { return &v }(true), }, PassHostHeader: func(v bool) *bool { return &v }(true), ResponseForwarding: &dynamic.ResponseForwarding{ diff --git a/pkg/healthcheck/healthcheck.go b/pkg/healthcheck/healthcheck.go index 95972474d..65db86f00 100644 --- a/pkg/healthcheck/healthcheck.go +++ b/pkg/healthcheck/healthcheck.go @@ -47,19 +47,20 @@ type metricsRegistry interface { // Options are the public health check options. type Options struct { - Headers map[string]string - Hostname string - Scheme string - Path string - Port int - Transport http.RoundTripper - Interval time.Duration - Timeout time.Duration - LB Balancer + Headers map[string]string + Hostname string + Scheme string + Path string + Port int + FollowRedirects bool + Transport http.RoundTripper + Interval time.Duration + Timeout time.Duration + LB Balancer } func (opt Options) String() string { - return fmt.Sprintf("[Hostname: %s Headers: %v Path: %s Port: %d Interval: %s Timeout: %s]", opt.Hostname, opt.Headers, opt.Path, opt.Port, opt.Interval, opt.Timeout) + return fmt.Sprintf("[Hostname: %s Headers: %v Path: %s Port: %d Interval: %s Timeout: %s FollowRedirects: %v]", opt.Hostname, opt.Headers, opt.Path, opt.Port, opt.Interval, opt.Timeout, opt.FollowRedirects) } type backendURL struct { @@ -239,6 +240,12 @@ func checkHealth(serverURL *url.URL, backend *BackendConfig) error { Transport: backend.Options.Transport, } + if !backend.FollowRedirects { + client.CheckRedirect = func(req *http.Request, via []*http.Request) error { + return http.ErrUseLastResponse + } + } + resp, err := client.Do(req) if err != nil { return fmt.Errorf("HTTP request failed: %s", err) diff --git a/pkg/healthcheck/healthcheck_test.go b/pkg/healthcheck/healthcheck_test.go index fc9bbd9c1..e9f6092ad 100644 --- a/pkg/healthcheck/healthcheck_test.go +++ b/pkg/healthcheck/healthcheck_test.go @@ -469,3 +469,57 @@ func TestLBStatusUpdater(t *testing.T) { break } } + +func TestNotFollowingRedirects(t *testing.T) { + redirectServerCalled := false + redirectTestServer := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + redirectServerCalled = true + })) + defer redirectTestServer.Close() + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + rw.Header().Add("location", redirectTestServer.URL) + rw.WriteHeader(http.StatusSeeOther) + cancel() + })) + defer server.Close() + + lb := &testLoadBalancer{ + RWMutex: &sync.RWMutex{}, + servers: []*url.URL{testhelpers.MustParseURL(server.URL)}, + } + + backend := NewBackendConfig(Options{ + Path: "/path", + Interval: healthCheckInterval, + Timeout: healthCheckTimeout, + LB: lb, + FollowRedirects: false, + }, "backendName") + + check := HealthCheck{ + Backends: make(map[string]*BackendConfig), + metrics: testhelpers.NewCollectingHealthCheckMetrics(), + } + + wg := sync.WaitGroup{} + wg.Add(1) + + go func() { + check.execute(ctx, backend) + wg.Done() + }() + + timeout := time.Duration(int(healthCheckInterval) + 500) + select { + case <-time.After(timeout): + t.Fatal("test did not complete in time") + case <-ctx.Done(): + wg.Wait() + } + + assert.False(t, redirectServerCalled, "HTTP redirect must not be followed") +} diff --git a/pkg/provider/kv/kv_test.go b/pkg/provider/kv/kv_test.go index 3f6cc7fca..7f9e013c5 100644 --- a/pkg/provider/kv/kv_test.go +++ b/pkg/provider/kv/kv_test.go @@ -47,6 +47,7 @@ func Test_buildConfiguration(t *testing.T) { "traefik/http/services/Service01/loadBalancer/healthCheck/headers/name0": "foobar", "traefik/http/services/Service01/loadBalancer/healthCheck/headers/name1": "foobar", "traefik/http/services/Service01/loadBalancer/healthCheck/scheme": "foobar", + "traefik/http/services/Service01/loadBalancer/healthCheck/followredirects": "true", "traefik/http/services/Service01/loadBalancer/responseForwarding/flushInterval": "foobar", "traefik/http/services/Service01/loadBalancer/passHostHeader": "true", "traefik/http/services/Service01/loadBalancer/sticky/cookie/name": "foobar", @@ -608,12 +609,13 @@ func Test_buildConfiguration(t *testing.T) { }, }, HealthCheck: &dynamic.HealthCheck{ - Scheme: "foobar", - Path: "foobar", - Port: 42, - Interval: "foobar", - Timeout: "foobar", - Hostname: "foobar", + Scheme: "foobar", + Path: "foobar", + Port: 42, + Interval: "foobar", + Timeout: "foobar", + Hostname: "foobar", + FollowRedirects: func(v bool) *bool { return &v }(true), Headers: map[string]string{ "name0": "foobar", "name1": "foobar", diff --git a/pkg/server/service/service.go b/pkg/server/service/service.go index 4ff4d213b..3182234ae 100644 --- a/pkg/server/service/service.go +++ b/pkg/server/service/service.go @@ -262,15 +262,21 @@ func buildHealthCheckOptions(ctx context.Context, lb healthcheck.Balancer, backe logger.Warnf("Health check timeout for backend '%s' should be lower than the health check interval. Interval set to timeout + 1 second (%s).", backend, interval) } + followRedirects := true + if hc.FollowRedirects != nil { + followRedirects = *hc.FollowRedirects + } + return &healthcheck.Options{ - Scheme: hc.Scheme, - Path: hc.Path, - Port: hc.Port, - Interval: interval, - Timeout: timeout, - LB: lb, - Hostname: hc.Hostname, - Headers: hc.Headers, + Scheme: hc.Scheme, + Path: hc.Path, + Port: hc.Port, + Interval: interval, + Timeout: timeout, + LB: lb, + Hostname: hc.Hostname, + Headers: hc.Headers, + FollowRedirects: followRedirects, } }