Repository: containous/traefik
Pull request #5147 (2019-07-23 08:06:09)

Do not follow redirects for the health check URLs
### What does this PR do?

This PR avoids that the health checker follows HTTP redirects.


### Motivation

The default behavior of the go HTTP client is to follow redirects and stop after 10 consecutive requests ([see](https://golang.org/src/net/http/client.go?s=1928:3975#L46)). I wouldn't expect this from a health checker. A redirect could point to a completely different host which is broken. The result of this would be that traefik removes a healthy backend.

### More

- [x] Added/updated tests
- [x] Added/updated documentation

### Additional Notes


jbdoumenjou: Hello @coder-hugo,

We think that not following the redirect for the health check makes sense but we can't integrate it like that without risking breaking existing users relying on the current behavior.

WDYT about integrating it as an health check option?

Patch:
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.<service_name>.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.<service_name>.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.<name-of-your-choice>.loadbalancer.pa
     - "traefik.http.services.myservice.loadbalancer.healthcheck.timeout=10"
     ```
 
+??? info "`traefik.http.services.<service_name>.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.<service_name>.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.<service_name>.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.<service_name>.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.<service_name>.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.<service_name>.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/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/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,
 	}
 }
 


Test patch:
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_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",