Set host for http health checker explicitly to avoid using the cluste…

…r name as host header for http health checking request. (#3057) * Set host for http health checker explictly to avoid using the cluster name as host header for http health checking request Signed-off-by: lemonlinger <[email protected]> * fix broken tests Signed-off-by: lemonlinger <[email protected]> * fix health-check test case in xds translation Signed-off-by: lemonlinger <[email protected]> * Simplify code and concise comments Signed-off-by: lemonlinger <[email protected]> --------- Signed-off-by: lemonlinger <[email protected]>
envoyproxy · Apr 2, 2024 · 8f450a9 · 8f450a9
1 parent 36d7141
commit 8f450a9
Show file tree

Hide file tree

Showing 9 changed files with 52 additions and 0 deletions.
diff --git a/internal/gatewayapi/backendtrafficpolicy.go b/internal/gatewayapi/backendtrafficpolicy.go
@@ -375,6 +375,8 @@ func (t *Translator) translateBackendTrafficPolicyForRoute(policy *egv1a1.Backen
 					r.LoadBalancer = lb
 					r.ProxyProtocol = pp
 					r.HealthCheck = hc
+					// Update the Host field in HealthCheck, now that we have access to the Route Hostname.
+					r.HealthCheck.SetHTTPHostIfAbsent(r.Hostname)
 					r.CircuitBreaker = cb
 					r.FaultInjection = fi
 					r.TCPKeepalive = ka
@@ -532,7 +534,10 @@ func (t *Translator) translateBackendTrafficPolicyForGateway(policy *egv1a1.Back
 			}
 			if r.HealthCheck == nil {
 				r.HealthCheck = hc
+				// Update the Host field in HealthCheck, now that we have access to the Route Hostname.
+				r.HealthCheck.SetHTTPHostIfAbsent(r.Hostname)
 			}
+
 			if r.CircuitBreaker == nil {
 				r.CircuitBreaker = cb
 			}

diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-healthcheck.out.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-healthcheck.out.yaml
@@ -496,6 +496,7 @@ xdsIR:
               expectedStatuses:
               - 200
               - 300
+              host: '*'
               method: GET
               path: /healthz
             interval: 3s
@@ -624,6 +625,7 @@ xdsIR:
               expectedStatuses:
               - 200
               - 201
+              host: gateway.envoyproxy.io
               method: GET
               path: /healthz
             interval: 5s

diff --git a/...gatewayapi/testdata/backendtrafficpolicy-with-tcp-udp-listeners-apply-on-gateway.out.yaml b/...gatewayapi/testdata/backendtrafficpolicy-with-tcp-udp-listeners-apply-on-gateway.out.yaml
@@ -258,6 +258,7 @@ xdsIR:
             expectedStatuses:
             - 200
             - 300
+            host: ""
             method: GET
             path: /healthz
           interval: 3s

diff --git a/...l/gatewayapi/testdata/backendtrafficpolicy-with-tcp-udp-listeners-apply-on-route.out.yaml b/...l/gatewayapi/testdata/backendtrafficpolicy-with-tcp-udp-listeners-apply-on-route.out.yaml
@@ -333,6 +333,7 @@ xdsIR:
             expectedStatuses:
             - 200
             - 300
+            host: ""
             method: GET
             path: /healthz
           interval: 3s

diff --git a/internal/ir/xds.go b/internal/ir/xds.go
@@ -54,6 +54,7 @@ var (
 	ErrHealthCheckUnhealthyThresholdInvalid    = errors.New("field HealthCheck.UnhealthyThreshold should be greater than 0")
 	ErrHealthCheckHealthyThresholdInvalid      = errors.New("field HealthCheck.HealthyThreshold should be greater than 0")
 	ErrHealthCheckerInvalid                    = errors.New("health checker setting is invalid, only one health checker can be set")
+	ErrHCHTTPHostInvalid                       = errors.New("field HTTPHealthChecker.Host should be specified")
 	ErrHCHTTPPathInvalid                       = errors.New("field HTTPHealthChecker.Path should be specified")
 	ErrHCHTTPMethodInvalid                     = errors.New("only one of the GET, HEAD, POST, DELETE, OPTIONS, TRACE, PATCH of HTTPHealthChecker.Method could be set")
 	ErrHCHTTPExpectedStatusesInvalid           = errors.New("field HTTPHealthChecker.ExpectedStatuses should be specified")
@@ -1551,6 +1552,12 @@ type ActiveHealthCheck struct {
 	TCP *TCPHealthChecker `json:"tcp,omitempty" yaml:"tcp,omitempty"`
 }
 
+func (h *HealthCheck) SetHTTPHostIfAbsent(host string) {
+	if h != nil && h.Active != nil && h.Active.HTTP != nil && h.Active.HTTP.Host == "" {
+		h.Active.HTTP.Host = host
+	}
+}
+
 // Validate the fields within the HealthCheck structure.
 func (h *HealthCheck) Validate() error {
 	var errs error
@@ -1607,6 +1614,8 @@ func (h *HealthCheck) Validate() error {
 // HTTPHealthChecker defines the settings of http health check.
 // +k8s:deepcopy-gen=true
 type HTTPHealthChecker struct {
+	// Host defines the value of the host header in the HTTP health check request.
+	Host string `json:"host" yaml:"host"`
 	// Path defines the HTTP path that will be requested during health checking.
 	Path string `json:"path" yaml:"path"`
 	// Method defines the HTTP method used for health checking.
@@ -1620,6 +1629,9 @@ type HTTPHealthChecker struct {
 // Validate the fields within the HTTPHealthChecker structure.
 func (c *HTTPHealthChecker) Validate() error {
 	var errs error
+	if c.Host == "" {
+		errs = errors.Join(errs, ErrHCHTTPHostInvalid)
+	}
 	if c.Path == "" {
 		errs = errors.Join(errs, ErrHCHTTPPathInvalid)
 	}

diff --git a/internal/ir/xds_test.go b/internal/ir/xds_test.go
@@ -1257,6 +1257,7 @@ func TestValidateHealthCheck(t *testing.T) {
 				UnhealthyThreshold: ptr.To[uint32](3),
 				HealthyThreshold:   ptr.To[uint32](3),
 				HTTP: &HTTPHealthChecker{
+					Host:             "*",
 					Path:             "/healthz",
 					ExpectedStatuses: []HTTPStatus{200, 400},
 				},
@@ -1273,6 +1274,7 @@ func TestValidateHealthCheck(t *testing.T) {
 				UnhealthyThreshold: ptr.To[uint32](3),
 				HealthyThreshold:   ptr.To[uint32](3),
 				HTTP: &HTTPHealthChecker{
+					Host:             "*",
 					Path:             "/healthz",
 					Method:           ptr.To(http.MethodGet),
 					ExpectedStatuses: []HTTPStatus{200, 400},
@@ -1290,6 +1292,7 @@ func TestValidateHealthCheck(t *testing.T) {
 				UnhealthyThreshold: ptr.To[uint32](0),
 				HealthyThreshold:   ptr.To[uint32](3),
 				HTTP: &HTTPHealthChecker{
+					Host:             "*",
 					Path:             "/healthz",
 					Method:           ptr.To(http.MethodPatch),
 					ExpectedStatuses: []HTTPStatus{200, 400},
@@ -1307,6 +1310,7 @@ func TestValidateHealthCheck(t *testing.T) {
 				UnhealthyThreshold: ptr.To[uint32](3),
 				HealthyThreshold:   ptr.To[uint32](0),
 				HTTP: &HTTPHealthChecker{
+					Host:             "*",
 					Path:             "/healthz",
 					Method:           ptr.To(http.MethodPost),
 					ExpectedStatuses: []HTTPStatus{200, 400},
@@ -1316,6 +1320,23 @@ func TestValidateHealthCheck(t *testing.T) {
 			},
 			want: ErrHealthCheckHealthyThresholdInvalid,
 		},
+		{
+			name: "http-health-check: invalid host",
+			input: HealthCheck{&ActiveHealthCheck{
+				Timeout:            &metav1.Duration{Duration: time.Second},
+				Interval:           &metav1.Duration{Duration: time.Second},
+				UnhealthyThreshold: ptr.To[uint32](3),
+				HealthyThreshold:   ptr.To[uint32](3),
+				HTTP: &HTTPHealthChecker{
+					Path:             "/healthz",
+					Method:           ptr.To(http.MethodPut),
+					ExpectedStatuses: []HTTPStatus{200, 400},
+				},
+			},
+				&OutlierDetection{},
+			},
+			want: ErrHCHTTPHostInvalid,
+		},
 		{
 			name: "http-health-check: invalid path",
 			input: HealthCheck{&ActiveHealthCheck{
@@ -1324,6 +1345,7 @@ func TestValidateHealthCheck(t *testing.T) {
 				UnhealthyThreshold: ptr.To[uint32](3),
 				HealthyThreshold:   ptr.To[uint32](3),
 				HTTP: &HTTPHealthChecker{
+					Host:             "*",
 					Path:             "",
 					Method:           ptr.To(http.MethodPut),
 					ExpectedStatuses: []HTTPStatus{200, 400},
@@ -1341,6 +1363,7 @@ func TestValidateHealthCheck(t *testing.T) {
 				UnhealthyThreshold: ptr.To(uint32(3)),
 				HealthyThreshold:   ptr.To(uint32(3)),
 				HTTP: &HTTPHealthChecker{
+					Host:             "*",
 					Path:             "/healthz",
 					Method:           ptr.To(http.MethodConnect),
 					ExpectedStatuses: []HTTPStatus{200, 400},
@@ -1358,6 +1381,7 @@ func TestValidateHealthCheck(t *testing.T) {
 				UnhealthyThreshold: ptr.To(uint32(3)),
 				HealthyThreshold:   ptr.To(uint32(3)),
 				HTTP: &HTTPHealthChecker{
+					Host:             "*",
 					Path:             "/healthz",
 					Method:           ptr.To(http.MethodDelete),
 					ExpectedStatuses: []HTTPStatus{},
@@ -1375,6 +1399,7 @@ func TestValidateHealthCheck(t *testing.T) {
 				UnhealthyThreshold: ptr.To(uint32(3)),
 				HealthyThreshold:   ptr.To(uint32(3)),
 				HTTP: &HTTPHealthChecker{
+					Host:             "*",
 					Path:             "/healthz",
 					Method:           ptr.To(http.MethodHead),
 					ExpectedStatuses: []HTTPStatus{100, 600},
@@ -1392,6 +1417,7 @@ func TestValidateHealthCheck(t *testing.T) {
 				UnhealthyThreshold: ptr.To(uint32(3)),
 				HealthyThreshold:   ptr.To(uint32(3)),
 				HTTP: &HTTPHealthChecker{
+					Host:             "*",
 					Path:             "/healthz",
 					Method:           ptr.To(http.MethodOptions),
 					ExpectedStatuses: []HTTPStatus{200, 300},

diff --git a/internal/xds/translator/cluster.go b/internal/xds/translator/cluster.go
@@ -206,6 +206,7 @@ func buildXdsHealthCheck(healthcheck *ir.ActiveHealthCheck) []*corev3.HealthChec
 	}
 	if healthcheck.HTTP != nil {
 		httpChecker := &corev3.HealthCheck_HttpHealthCheck{
+			Host: healthcheck.HTTP.Host,
 			Path: healthcheck.HTTP.Path,
 		}
 		if healthcheck.HTTP.Method != nil {

diff --git a/internal/xds/translator/testdata/in/xds-ir/health-check.yaml b/internal/xds/translator/testdata/in/xds-ir/health-check.yaml
@@ -17,6 +17,7 @@ http:
         unhealthyThreshold: 3
         healthyThreshold: 1
         http:
+          host: "*"
           path: "/healthz"
           expectedResponse:
             text: "ok"
@@ -46,6 +47,7 @@ http:
         unhealthyThreshold: 3
         healthyThreshold: 3
         http:
+          host: "*"
           path: "/healthz"
           expectedResponse:
             binary: "cG9uZw=="

diff --git a/internal/xds/translator/testdata/out/xds-ir/health-check.clusters.yaml b/internal/xds/translator/testdata/out/xds-ir/health-check.clusters.yaml
@@ -18,6 +18,7 @@
         start: "200"
       - end: "301"
         start: "300"
+      host: '*'
       path: /healthz
       receive:
       - text: 6f6b
@@ -53,6 +54,7 @@
       expectedStatuses:
       - end: "202"
         start: "200"
+      host: '*'
       path: /healthz
       receive:
       - binary: cG9uZw==