proxy: Fix #1574; health check now respects hostname when upstream Host header...

proxy: Fix #1574; health check now respects hostname when upstream Host header is configured (#1577)

* Implement adding Host header to health check

* Fix type problems

* Fix duplicate function, Replace args

* Add debugging

* Add debugging

* Add debugging

* Add debugging

* Attempt to set req.Host instead of the header

* Clean up debugging

* Fix missing newline

* Fix spelling

* Add test, refactoring

* Fix with gofmt

* Add error check on NewRequest

caddyhttp/httpserver/pathcleaner_test.go

@@ -31,7 +31,7 @@ var paths = map[string]map[string]string{
 
 func assertEqual(t *testing.T, expected, received string) {
 	if expected != received {
-		t.Errorf("\tExpected: %s\n\t\t\tRecieved: %s", expected, received)
+		t.Errorf("\tExpected: %s\n\t\t\tReceived: %s", expected, received)
 	}
 }
 

caddyhttp/proxy/proxy_test.go

@@ -163,7 +163,7 @@ func TestReverseProxyMaxConnLimit(t *testing.T) {
 		proxy / `+backend.URL+` {
 			max_conns `+fmt.Sprint(MaxTestConns)+`
 		}
-	`)))
+	`)), "")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -1008,7 +1008,7 @@ func TestReverseProxyRetry(t *testing.T) {
 		try_duration 5s
 		try_interval 250ms
 	}
-	`)))
+	`)), "")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -1057,7 +1057,7 @@ func TestReverseProxyLargeBody(t *testing.T) {
 	}))
 	defer backend.Close()
 
-	su, err := NewStaticUpstreams(caddyfile.NewDispenser("Testfile", strings.NewReader(`proxy / `+backend.URL)))
+	su, err := NewStaticUpstreams(caddyfile.NewDispenser("Testfile", strings.NewReader(`proxy / `+backend.URL)), "")
 	if err != nil {
 		t.Fatal(err)
 	}

caddyhttp/proxy/setup.go

@@ -14,7 +14,7 @@ func init() {
 
 // setup configures a new Proxy middleware instance.
 func setup(c *caddy.Controller) error {
-	upstreams, err := NewStaticUpstreams(c.Dispenser)
+	upstreams, err := NewStaticUpstreams(c.Dispenser, httpserver.GetConfig(c).Host())
 	if err != nil {
 		return err
 	}

caddyhttp/proxy/upstream.go

@@ -41,6 +41,7 @@ type staticUpstream struct {
 		Path     string
 		Interval time.Duration
 		Timeout  time.Duration
+		Host     string
 	}
 	WithoutPathPrefix  string
 	IgnoredSubPaths    []string
@@ -49,8 +50,10 @@ type staticUpstream struct {
 }
 
 // NewStaticUpstreams parses the configuration input and sets up
-// static upstreams for the proxy middleware.
-func NewStaticUpstreams(c caddyfile.Dispenser) ([]Upstream, error) {
+// static upstreams for the proxy middleware. The host string parameter,
+// if not empty, is used for setting the upstream Host header for the
+// health checks if the upstream header config requires it.
+func NewStaticUpstreams(c caddyfile.Dispenser, host string) ([]Upstream, error) {
 	var upstreams []Upstream
 	for c.Next() {
 
@@ -118,6 +121,14 @@ func NewStaticUpstreams(c caddyfile.Dispenser) ([]Upstream, error) {
 					TLSClientConfig: &tls.Config{InsecureSkipVerify: upstream.insecureSkipVerify},
 				},
 			}
+
+			// set up health check upstream host if we have one
+			if host != "" {
+				hostHeader := upstream.upstreamHeaders.Get("Host")
+				if strings.Contains(hostHeader, "{host}") {
+					upstream.HealthCheck.Host = strings.Replace(hostHeader, "{host}", host, -1)
+				}
+			}
 			upstream.wg.Add(1)
 			go func() {
 				defer upstream.wg.Done()
@@ -371,12 +382,25 @@ func (u *staticUpstream) healthCheck() {
 	for _, host := range u.Hosts {
 		hostURL := host.Name + u.HealthCheck.Path
 		var unhealthy bool
-		if r, err := u.HealthCheck.Client.Get(hostURL); err == nil {
-			io.Copy(ioutil.Discard, r.Body)
-			r.Body.Close()
-			unhealthy = r.StatusCode < 200 || r.StatusCode >= 400
-		} else {
+
+		// set up request, needed to be able to modify headers
+		// possible errors are bad HTTP methods or un-parsable urls
+		req, err := http.NewRequest("GET", hostURL, nil)
+		if err != nil {
 			unhealthy = true
+		} else {
+			// set host for request going upstream
+			if u.HealthCheck.Host != "" {
+				req.Host = u.HealthCheck.Host
+			}
+
+			if r, err := u.HealthCheck.Client.Do(req); err == nil {
+				io.Copy(ioutil.Discard, r.Body)
+				r.Body.Close()
+				unhealthy = r.StatusCode < 200 || r.StatusCode >= 400
+			} else {
+				unhealthy = true
+			}
 		}
 		if unhealthy {
 			atomic.StoreInt32(&host.Unhealthy, 1)

caddyhttp/proxy/upstream_test.go

@@ -228,7 +228,7 @@ func TestStop(t *testing.T) {
 
 			defer backend.Close()
 
-			upstreams, err := NewStaticUpstreams(caddyfile.NewDispenser("Testfile", strings.NewReader(fmt.Sprintf(config, backend.URL, test.intervalInMilliseconds))))
+			upstreams, err := NewStaticUpstreams(caddyfile.NewDispenser("Testfile", strings.NewReader(fmt.Sprintf(config, backend.URL, test.intervalInMilliseconds))), "")
 			if err != nil {
 				t.Error("Expected no error. Got:", err.Error())
 			}
@@ -277,7 +277,7 @@ func TestParseBlock(t *testing.T) {
 	}
 
 	for i, test := range tests {
-		upstreams, err := NewStaticUpstreams(caddyfile.NewDispenser("Testfile", strings.NewReader(test.config)))
+		upstreams, err := NewStaticUpstreams(caddyfile.NewDispenser("Testfile", strings.NewReader(test.config)), "")
 		if err != nil {
 			t.Errorf("Expected no error. Got: %s", err.Error())
 		}
@@ -301,7 +301,7 @@ func TestParseBlock(t *testing.T) {
 
 func TestHealthSetUp(t *testing.T) {
 	// tests for insecure skip verify
-	isv_tests := []struct {
+	tests := []struct {
 		config string
 		flag   bool
 	}{
@@ -312,24 +312,65 @@ func TestHealthSetUp(t *testing.T) {
 		{"proxy / localhost:8080 {\n health_check / \n insecure_skip_verify \n}", true},
 	}
 
-	for i, test := range isv_tests {
-		upstreams, err := NewStaticUpstreams(caddyfile.NewDispenser("Testfile", strings.NewReader(test.config)))
+	for i, test := range tests {
+		upstreams, err := NewStaticUpstreams(caddyfile.NewDispenser("Testfile", strings.NewReader(test.config)), "")
 		if err != nil {
 			t.Errorf("Expected no error. Got: %s", err.Error())
 		}
 		for _, upstream := range upstreams {
 			staticUpstream, ok := upstream.(*staticUpstream)
 			if !ok {
-				t.Errorf("type mismatch: %#v", upstream)
+				t.Errorf("Type mismatch: %#v", upstream)
 				continue
 			}
 			transport, ok := staticUpstream.HealthCheck.Client.Transport.(*http.Transport)
 			if !ok {
-				t.Errorf("type mismatch: %#v", staticUpstream.HealthCheck.Client.Transport)
+				t.Errorf("Type mismatch: %#v", staticUpstream.HealthCheck.Client.Transport)
 				continue
 			}
 			if test.flag != transport.TLSClientConfig.InsecureSkipVerify {
-				t.Errorf("test %d: expected transport.TLSClientCnfig.InsecureSkipVerify=%v, got %v", i, test.flag, transport.TLSClientConfig.InsecureSkipVerify)
+				t.Errorf("Test %d: expected transport.TLSClientCnfig.InsecureSkipVerify=%v, got %v", i, test.flag, transport.TLSClientConfig.InsecureSkipVerify)
+			}
+		}
+	}
+}
+
+func TestHealthCheckHost(t *testing.T) {
+	// tests for upstream host on health checks
+	tests := []struct {
+		config string
+		flag   bool
+		host   string
+	}{
+		// Test #1: without upstream header
+		{"proxy / localhost:8080 {\n health_check / \n}", false, "example.com"},
+
+		// Test #2: without upstream header, missing host
+		{"proxy / localhost:8080 {\n health_check / \n}", true, ""},
+
+		// Test #3: with upstream header (via transparent preset)
+		{"proxy / localhost:8080 {\n health_check / \n transparent \n}", true, "foo.example.com"},
+
+		// Test #4: with upstream header (explicit header)
+		{"proxy / localhost:8080 {\n health_check / \n header_upstream Host {host} \n}", true, "example.com"},
+
+		// Test #5: with upstream header, missing host
+		{"proxy / localhost:8080 {\n health_check / \n transparent \n}", true, ""},
+	}
+
+	for i, test := range tests {
+		upstreams, err := NewStaticUpstreams(caddyfile.NewDispenser("Testfile", strings.NewReader(test.config)), test.host)
+		if err != nil {
+			t.Errorf("Expected no error. Got: %s", err.Error())
+		}
+		for _, upstream := range upstreams {
+			staticUpstream, ok := upstream.(*staticUpstream)
+			if !ok {
+				t.Errorf("Type mismatch: %#v", upstream)
+				continue
+			}
+			if test.flag != (staticUpstream.HealthCheck.Host == test.host) {
+				t.Errorf("Test %d: expected staticUpstream.HealthCheck.Host=%v, got %v", i, test.host, staticUpstream.HealthCheck.Host)
 			}
 		}
 	}