From 8f450a994fd0fcccd17ecbb569bb58f8b99b1673 Mon Sep 17 00:00:00 2001 From: Meng Date: Tue, 2 Apr 2024 08:03:43 +0800 Subject: [PATCH] Set host for http health checker explicitly to avoid using the cluster 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 * fix broken tests Signed-off-by: lemonlinger * fix health-check test case in xds translation Signed-off-by: lemonlinger * Simplify code and concise comments Signed-off-by: lemonlinger --------- Signed-off-by: lemonlinger --- internal/gatewayapi/backendtrafficpolicy.go | 5 ++++ ...endtrafficpolicy-with-healthcheck.out.yaml | 2 ++ ...cp-udp-listeners-apply-on-gateway.out.yaml | 1 + ...-tcp-udp-listeners-apply-on-route.out.yaml | 1 + internal/ir/xds.go | 12 +++++++++ internal/ir/xds_test.go | 26 +++++++++++++++++++ internal/xds/translator/cluster.go | 1 + .../testdata/in/xds-ir/health-check.yaml | 2 ++ .../out/xds-ir/health-check.clusters.yaml | 2 ++ 9 files changed, 52 insertions(+) diff --git a/internal/gatewayapi/backendtrafficpolicy.go b/internal/gatewayapi/backendtrafficpolicy.go index d7595cb1b4c..5748caee206 100644 --- 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 index 7194512bbb8..34b5a13021e 100644 --- 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/internal/gatewayapi/testdata/backendtrafficpolicy-with-tcp-udp-listeners-apply-on-gateway.out.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-tcp-udp-listeners-apply-on-gateway.out.yaml index b7eae0602f4..62a0ec8fc75 100644 --- a/internal/gatewayapi/testdata/backendtrafficpolicy-with-tcp-udp-listeners-apply-on-gateway.out.yaml +++ b/internal/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/internal/gatewayapi/testdata/backendtrafficpolicy-with-tcp-udp-listeners-apply-on-route.out.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-tcp-udp-listeners-apply-on-route.out.yaml index 1524afa8734..774e7e0525b 100644 --- a/internal/gatewayapi/testdata/backendtrafficpolicy-with-tcp-udp-listeners-apply-on-route.out.yaml +++ b/internal/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 index 73f23a992db..9fa65379118 100644 --- 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 index c9f2bed7411..2f4d9a46a33 100644 --- 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 index 576c1233a11..0d2593f9cc6 100644 --- 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 index ca5f9c0a1c8..a767bdab208 100644 --- 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 index 8c076fbdb87..b789b876c3c 100644 --- 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==