From de23696983f23dbaa63531952dc1827b4c54f2c3 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Wed, 9 Apr 2025 02:20:26 +0000 Subject: [PATCH 01/13] add test for cors Signed-off-by: Huabing (Robin) Zhao --- conformance/tests/httproute-cors.go | 95 +++++++++++++++++++++++++++ conformance/tests/httproute-cors.yaml | 34 ++++++++++ 2 files changed, 129 insertions(+) create mode 100644 conformance/tests/httproute-cors.go create mode 100644 conformance/tests/httproute-cors.yaml diff --git a/conformance/tests/httproute-cors.go b/conformance/tests/httproute-cors.go new file mode 100644 index 0000000000..e68dbcc0fc --- /dev/null +++ b/conformance/tests/httproute-cors.go @@ -0,0 +1,95 @@ +/* +Copyright 2022 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package tests + +import ( + "testing" + + "k8s.io/apimachinery/pkg/types" + + "sigs.k8s.io/gateway-api/conformance/utils/http" + "sigs.k8s.io/gateway-api/conformance/utils/kubernetes" + "sigs.k8s.io/gateway-api/conformance/utils/suite" + "sigs.k8s.io/gateway-api/pkg/features" +) + +func init() { + ConformanceTests = append(ConformanceTests, HTTPRouteCORS) +} + +var HTTPRouteCORS = suite.ConformanceTest{ + ShortName: "HTTPRouteCORS", + Description: "An HTTPRoute with CORS filter", + Manifests: []string{"tests/httproute-cors.yaml"}, + Features: []features.FeatureName{ + features.SupportGateway, + features.SupportHTTPRoute, + features.SupportHTTPRouteCORS, + }, + Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { + ns := "gateway-conformance-infra" + routeNN := types.NamespacedName{Name: "cors", Namespace: ns} + gwNN := types.NamespacedName{Name: "same-namespace", Namespace: ns} + gwAddr := kubernetes.GatewayAndHTTPRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), routeNN) + kubernetes.HTTPRouteMustHaveResolvedRefsConditionsTrue(t, suite.Client, suite.TimeoutConfig, routeNN, gwNN) + + testCases := []http.ExpectedResponse{ + { + Request: http.Request{ + Path: "/", + Method: "OPTIONS", + Headers: map[string]string{ + "Origin": "https://www.foo.com", + "access-control-request-method": "GET", + "access-control-request-headers": "x-header-1, x-header-2", + }, + }, + // Set the expected request properties and namespace to empty strings. + // This is a workaround to avoid the test failure. + // The response body is empty because the request is a preflight request, + // so we can't get the request properties from the echoserver. + ExpectedRequest: &http.ExpectedRequest{ + Request: http.Request{ + Host: "", + Method: "OPTIONS", + Path: "", + Headers: nil, + }, + }, + Namespace: "", + Response: http.Response{ + StatusCode: 200, + Headers: map[string]string{ + "access-control-allow-origin": "https://www.foo.com", + "access-control-allow-methods": "GET, POST, PUT, PATCH, DELETE, OPTIONS", + "access-control-allow-headers": "x-header-1, x-header-2", + "access-control-expose-headers": "x-header-3, x-header-4", + }, + }, + }, + } + for i := range testCases { + // Declare tc here to avoid loop variable + // reuse issues across parallel tests. + tc := testCases[i] + t.Run(tc.GetTestCaseName(i), func(t *testing.T) { + t.Parallel() + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, tc) + }) + } + }, +} diff --git a/conformance/tests/httproute-cors.yaml b/conformance/tests/httproute-cors.yaml new file mode 100644 index 0000000000..78c9d657c0 --- /dev/null +++ b/conformance/tests/httproute-cors.yaml @@ -0,0 +1,34 @@ +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: cors + namespace: gateway-conformance-infra +spec: + parentRefs: + - name: same-namespace + rules: + - filters: + - type: CORS + cors: + allowOrigins: + - "https://www.foo.com" + - "https://www.bar.com" + - "https://*.foobar.com" + allowMethods: + - GET + - POST + - PUT + - PATCH + - DELETE + - OPTIONS + allowHeaders: + - "x-header-1" + - "x-header-2" + exposeHeaders: + - "x-header-3" + - "x-header-4" + allowCredentials: true + maxAge: 1000 + backendRefs: + - name: infra-backend-v1 + port: 8080 From 6040e97e1f2d2535511717c6807644fffe4b7f78 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Fri, 25 Apr 2025 07:34:03 +0000 Subject: [PATCH 02/13] add more tests Signed-off-by: Huabing (Robin) Zhao --- conformance/tests/httproute-cors.go | 133 +++++++++++++++++++++++++- conformance/tests/httproute-cors.yaml | 6 +- 2 files changed, 130 insertions(+), 9 deletions(-) diff --git a/conformance/tests/httproute-cors.go b/conformance/tests/httproute-cors.go index e68dbcc0fc..45a2e54ba3 100644 --- a/conformance/tests/httproute-cors.go +++ b/conformance/tests/httproute-cors.go @@ -33,7 +33,7 @@ func init() { var HTTPRouteCORS = suite.ConformanceTest{ ShortName: "HTTPRouteCORS", - Description: "An HTTPRoute with CORS filter", + Description: "An HTTPRoute with CORS filter should allow CORS requests from specified origins", Manifests: []string{"tests/httproute-cors.yaml"}, Features: []features.FeatureName{ features.SupportGateway, @@ -49,6 +49,7 @@ var HTTPRouteCORS = suite.ConformanceTest{ testCases := []http.ExpectedResponse{ { + TestCaseName: "CORS preflight request from an exact mactching origin should be allowed", Request: http.Request{ Path: "/", Method: "OPTIONS", @@ -74,10 +75,132 @@ var HTTPRouteCORS = suite.ConformanceTest{ Response: http.Response{ StatusCode: 200, Headers: map[string]string{ - "access-control-allow-origin": "https://www.foo.com", - "access-control-allow-methods": "GET, POST, PUT, PATCH, DELETE, OPTIONS", - "access-control-allow-headers": "x-header-1, x-header-2", - "access-control-expose-headers": "x-header-3, x-header-4", + "access-control-allow-origin": "https://www.foo.com", + "access-control-allow-methods": "GET, POST, PUT, PATCH, OPTIONS", + "access-control-allow-headers": "x-header-1, x-header-2", + "access-control-expose-headers": "x-header-3, x-header-4", + "access-control-max-age": "3600", + "access-control-allow-credentials": "true", + }, + }, + }, + { + TestCaseName: "CORS preflight request from a wildcard matching origin should be allowed", + Request: http.Request{ + Path: "/", + Method: "OPTIONS", + Headers: map[string]string{ + "Origin": "https://www.bar.com", + "access-control-request-method": "GET", + "access-control-request-headers": "x-header-1, x-header-2", + }, + }, + // Set the expected request properties and namespace to empty strings. + // This is a workaround to avoid the test failure. + // The response body is empty because the request is a preflight request, + // so we can't get the request properties from the echoserver. + ExpectedRequest: &http.ExpectedRequest{ + Request: http.Request{ + Host: "", + Method: "OPTIONS", + Path: "", + Headers: nil, + }, + }, + Namespace: "", + Response: http.Response{ + StatusCode: 200, + Headers: map[string]string{ + "access-control-allow-origin": "https://www.bar.com", + "access-control-allow-methods": "GET, POST, PUT, PATCH, OPTIONS", + "access-control-allow-headers": "x-header-1, x-header-2", + "access-control-expose-headers": "x-header-3, x-header-4", + "access-control-max-age": "3600", + "access-control-allow-credentials": "true", + }, + }, + }, + { + TestCaseName: "CORS preflight request from a non-matching origin should not be allowed", + Request: http.Request{ + Path: "/", + Method: "OPTIONS", + Headers: map[string]string{ + "Origin": "https://foobar.com", + "access-control-request-method": "GET", + }, + }, + // Set the expected request properties and namespace to empty strings. + // This is a workaround to avoid the test failure. + // The response body is empty because the request is a preflight request, + // so we can't get the request properties from the echoserver. + ExpectedRequest: &http.ExpectedRequest{ + Request: http.Request{ + Host: "", + Method: "OPTIONS", + Path: "", + Headers: nil, + }, + }, + Namespace: "", + Response: http.Response{ + AbsentHeaders: []string{ + "access-control-allow-origin", + }, + }, + }, + { + TestCaseName: "Simple request from an exact mactching origin should be allowed", + Namespace: ns, + Request: http.Request{ + Path: "/", + Method: "GET", + Headers: map[string]string{ + "Origin": "https://www.foo.com", + "access-control-request-method": "GET", + "access-control-request-headers": "x-header-1, x-header-2", + }, + }, + Response: http.Response{ + StatusCode: 200, + Headers: map[string]string{ + "access-control-allow-origin": "https://www.foo.com", + }, + }, + }, + { + TestCaseName: "Simple request from a wildcard matching origin should be allowed", + Namespace: ns, + Request: http.Request{ + Path: "/", + Method: "GET", + Headers: map[string]string{ + "Origin": "https://www.bar.com", + "access-control-request-method": "GET", + "access-control-request-headers": "x-header-1, x-header-2", + }, + }, + Response: http.Response{ + StatusCode: 200, + Headers: map[string]string{ + "access-control-allow-origin": "https://www.bar.com", + }, + }, + }, + { + TestCaseName: "Simple request from a non-matching origin should not be allowed", + Namespace: ns, + Request: http.Request{ + Path: "/", + Method: "GET", + Headers: map[string]string{ + "Origin": "https://foobar.com", + "access-control-request-method": "GET", + }, + }, + Response: http.Response{ + AbsentHeaders: []string{ + "access-control-allow-origin", }, }, }, diff --git a/conformance/tests/httproute-cors.yaml b/conformance/tests/httproute-cors.yaml index 78c9d657c0..2117fe840d 100644 --- a/conformance/tests/httproute-cors.yaml +++ b/conformance/tests/httproute-cors.yaml @@ -12,14 +12,12 @@ spec: cors: allowOrigins: - "https://www.foo.com" - - "https://www.bar.com" - - "https://*.foobar.com" + - "https://*.bar.com" allowMethods: - GET - POST - PUT - PATCH - - DELETE - OPTIONS allowHeaders: - "x-header-1" @@ -28,7 +26,7 @@ spec: - "x-header-3" - "x-header-4" allowCredentials: true - maxAge: 1000 + maxAge: 3600 backendRefs: - name: infra-backend-v1 port: 8080 From b4427275f1482441efd00c93d61822becc1f54ab Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Wed, 14 May 2025 07:54:38 +0000 Subject: [PATCH 03/13] address comment Signed-off-by: Huabing (Robin) Zhao --- conformance/tests/httproute-cors.go | 94 ++++++++++++++++++++++----- conformance/tests/httproute-cors.yaml | 32 +++++++-- 2 files changed, 106 insertions(+), 20 deletions(-) diff --git a/conformance/tests/httproute-cors.go b/conformance/tests/httproute-cors.go index 45a2e54ba3..c45da0bb4e 100644 --- a/conformance/tests/httproute-cors.go +++ b/conformance/tests/httproute-cors.go @@ -42,16 +42,18 @@ var HTTPRouteCORS = suite.ConformanceTest{ }, Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { ns := "gateway-conformance-infra" - routeNN := types.NamespacedName{Name: "cors", Namespace: ns} + routeNN1 := types.NamespacedName{Name: "cors-1", Namespace: ns} + routeNN2 := types.NamespacedName{Name: "cors-2", Namespace: ns} gwNN := types.NamespacedName{Name: "same-namespace", Namespace: ns} - gwAddr := kubernetes.GatewayAndHTTPRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), routeNN) - kubernetes.HTTPRouteMustHaveResolvedRefsConditionsTrue(t, suite.Client, suite.TimeoutConfig, routeNN, gwNN) + gwAddr := kubernetes.GatewayAndHTTPRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), routeNN1, routeNN2) + kubernetes.HTTPRouteMustHaveResolvedRefsConditionsTrue(t, suite.Client, suite.TimeoutConfig, routeNN1, gwNN) + kubernetes.HTTPRouteMustHaveResolvedRefsConditionsTrue(t, suite.Client, suite.TimeoutConfig, routeNN2, gwNN) testCases := []http.ExpectedResponse{ { TestCaseName: "CORS preflight request from an exact mactching origin should be allowed", Request: http.Request{ - Path: "/", + Path: "/cors-1", Method: "OPTIONS", Headers: map[string]string{ "Origin": "https://www.foo.com", @@ -76,7 +78,7 @@ var HTTPRouteCORS = suite.ConformanceTest{ StatusCode: 200, Headers: map[string]string{ "access-control-allow-origin": "https://www.foo.com", - "access-control-allow-methods": "GET, POST, PUT, PATCH, OPTIONS", + "access-control-allow-methods": "GET, OPTIONS", "access-control-allow-headers": "x-header-1, x-header-2", "access-control-expose-headers": "x-header-3, x-header-4", "access-control-max-age": "3600", @@ -87,7 +89,7 @@ var HTTPRouteCORS = suite.ConformanceTest{ { TestCaseName: "CORS preflight request from a wildcard matching origin should be allowed", Request: http.Request{ - Path: "/", + Path: "/cors-1", Method: "OPTIONS", Headers: map[string]string{ "Origin": "https://www.bar.com", @@ -112,7 +114,7 @@ var HTTPRouteCORS = suite.ConformanceTest{ StatusCode: 200, Headers: map[string]string{ "access-control-allow-origin": "https://www.bar.com", - "access-control-allow-methods": "GET, POST, PUT, PATCH, OPTIONS", + "access-control-allow-methods": "GET, OPTIONS", "access-control-allow-headers": "x-header-1, x-header-2", "access-control-expose-headers": "x-header-3, x-header-4", "access-control-max-age": "3600", @@ -123,7 +125,7 @@ var HTTPRouteCORS = suite.ConformanceTest{ { TestCaseName: "CORS preflight request from a non-matching origin should not be allowed", Request: http.Request{ - Path: "/", + Path: "/cors-1", Method: "OPTIONS", Headers: map[string]string{ "Origin": "https://foobar.com", @@ -151,9 +153,9 @@ var HTTPRouteCORS = suite.ConformanceTest{ }, { TestCaseName: "Simple request from an exact mactching origin should be allowed", - Namespace: ns, + Namespace: ns, Request: http.Request{ - Path: "/", + Path: "/cors-1", Method: "GET", Headers: map[string]string{ "Origin": "https://www.foo.com", @@ -164,15 +166,15 @@ var HTTPRouteCORS = suite.ConformanceTest{ Response: http.Response{ StatusCode: 200, Headers: map[string]string{ - "access-control-allow-origin": "https://www.foo.com", + "access-control-allow-origin": "https://www.foo.com", }, }, }, { TestCaseName: "Simple request from a wildcard matching origin should be allowed", - Namespace: ns, + Namespace: ns, Request: http.Request{ - Path: "/", + Path: "/cors-1", Method: "GET", Headers: map[string]string{ "Origin": "https://www.bar.com", @@ -183,15 +185,15 @@ var HTTPRouteCORS = suite.ConformanceTest{ Response: http.Response{ StatusCode: 200, Headers: map[string]string{ - "access-control-allow-origin": "https://www.bar.com", + "access-control-allow-origin": "https://www.bar.com", }, }, }, { TestCaseName: "Simple request from a non-matching origin should not be allowed", - Namespace: ns, + Namespace: ns, Request: http.Request{ - Path: "/", + Path: "/cors-1", Method: "GET", Headers: map[string]string{ "Origin": "https://foobar.com", @@ -204,6 +206,66 @@ var HTTPRouteCORS = suite.ConformanceTest{ }, }, }, + { + TestCaseName: "CORS preflight request with POST method should be allowed by allowMethods with wildcard", + Request: http.Request{ + Path: "/cors-2", + Method: "OPTIONS", + Headers: map[string]string{ + "Origin": "https://www.foo.com", + "access-control-request-method": "POST", + }, + }, + // Set the expected request properties and namespace to empty strings. + // This is a workaround to avoid the test failure. + // The response body is empty because the request is a preflight request, + // so we can't get the request properties from the echoserver. + ExpectedRequest: &http.ExpectedRequest{ + Request: http.Request{ + Host: "", + Method: "OPTIONS", + Path: "", + Headers: nil, + }, + }, + Namespace: "", + Response: http.Response{ + StatusCode: 200, + Headers: map[string]string{ + "access-control-allow-origin": "https://www.foo.com", + "access-control-allow-methods": "POST", + }, + }, + }, + { + TestCaseName: "CORS preflight request should not receive access-control-allow-credentials header without access-control-allow-credentials set to true", + Request: http.Request{ + Path: "/cors-2", + Method: "OPTIONS", + Headers: map[string]string{ + "Origin": "https://www.foo.com", + "access-control-request-method": "POST", + }, + }, + // Set the expected request properties and namespace to empty strings. + // This is a workaround to avoid the test failure. + // The response body is empty because the request is a preflight request, + // so we can't get the request properties from the echoserver. + ExpectedRequest: &http.ExpectedRequest{ + Request: http.Request{ + Host: "", + Method: "OPTIONS", + Path: "", + Headers: nil, + }, + }, + Namespace: "", + Response: http.Response{ + AbsentHeaders: []string{ + "access-control-allow-credentials", + }, + }, + }, } for i := range testCases { // Declare tc here to avoid loop variable diff --git a/conformance/tests/httproute-cors.yaml b/conformance/tests/httproute-cors.yaml index 2117fe840d..dbc204042b 100644 --- a/conformance/tests/httproute-cors.yaml +++ b/conformance/tests/httproute-cors.yaml @@ -1,7 +1,7 @@ apiVersion: gateway.networking.k8s.io/v1 kind: HTTPRoute metadata: - name: cors + name: cors-1 namespace: gateway-conformance-infra spec: parentRefs: @@ -15,9 +15,6 @@ spec: - "https://*.bar.com" allowMethods: - GET - - POST - - PUT - - PATCH - OPTIONS allowHeaders: - "x-header-1" @@ -27,6 +24,33 @@ spec: - "x-header-4" allowCredentials: true maxAge: 3600 + matches: + - path: + type: PathPrefix + value: /cors-1 + backendRefs: + - name: infra-backend-v1 + port: 8080 +--- +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: cors-2 + namespace: gateway-conformance-infra +spec: + parentRefs: + - name: same-namespace + rules: + - filters: + - type: CORS + cors: + allowOrigins: + - "https://www.foo.com" + allowMethods: ["*"] + matches: + - path: + type: PathPrefix + value: /cors-2 backendRefs: - name: infra-backend-v1 port: 8080 From 9a6b27b35d7b9b0c23f95eb2fb6ed5e83bb67b65 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Wed, 14 May 2025 07:59:51 +0000 Subject: [PATCH 04/13] address comment Signed-off-by: Huabing (Robin) Zhao --- conformance/tests/httproute-cors.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conformance/tests/httproute-cors.yaml b/conformance/tests/httproute-cors.yaml index dbc204042b..fab5cc79f3 100644 --- a/conformance/tests/httproute-cors.yaml +++ b/conformance/tests/httproute-cors.yaml @@ -35,7 +35,7 @@ spec: apiVersion: gateway.networking.k8s.io/v1 kind: HTTPRoute metadata: - name: cors-2 + name: cors-2 # test CORS with allowMethods: ["*"] and without allowCredentials=true namespace: gateway-conformance-infra spec: parentRefs: From e31227e12458fe542fc76b7c8a5987bfb5635a02 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Thu, 15 May 2025 17:33:06 +0800 Subject: [PATCH 05/13] Update conformance/tests/httproute-cors.go Co-authored-by: Norwin Schnyder --- conformance/tests/httproute-cors.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conformance/tests/httproute-cors.go b/conformance/tests/httproute-cors.go index c45da0bb4e..c5fb26eb24 100644 --- a/conformance/tests/httproute-cors.go +++ b/conformance/tests/httproute-cors.go @@ -51,7 +51,7 @@ var HTTPRouteCORS = suite.ConformanceTest{ testCases := []http.ExpectedResponse{ { - TestCaseName: "CORS preflight request from an exact mactching origin should be allowed", + TestCaseName: "CORS preflight request from an exact matching origin should be allowed", Request: http.Request{ Path: "/cors-1", Method: "OPTIONS", From 3412f3932eea1be180110727a8915b28734ba81b Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Thu, 15 May 2025 17:33:17 +0800 Subject: [PATCH 06/13] Update conformance/tests/httproute-cors.go Co-authored-by: Norwin Schnyder --- conformance/tests/httproute-cors.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conformance/tests/httproute-cors.go b/conformance/tests/httproute-cors.go index c5fb26eb24..565164583a 100644 --- a/conformance/tests/httproute-cors.go +++ b/conformance/tests/httproute-cors.go @@ -152,7 +152,7 @@ var HTTPRouteCORS = suite.ConformanceTest{ }, }, { - TestCaseName: "Simple request from an exact mactching origin should be allowed", + TestCaseName: "Simple request from an exact matching origin should be allowed", Namespace: ns, Request: http.Request{ Path: "/cors-1", From 2975eee857a6419eb9647b6b486d91def8954686 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Thu, 15 May 2025 10:55:51 +0000 Subject: [PATCH 07/13] Add option to ignore whitespace in the resposnse header values Signed-off-by: Huabing (Robin) Zhao --- conformance/tests/httproute-cors.go | 6 ++++++ conformance/utils/http/http.go | 13 +++++++++++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/conformance/tests/httproute-cors.go b/conformance/tests/httproute-cors.go index 565164583a..a59d9a1c99 100644 --- a/conformance/tests/httproute-cors.go +++ b/conformance/tests/httproute-cors.go @@ -84,6 +84,9 @@ var HTTPRouteCORS = suite.ConformanceTest{ "access-control-max-age": "3600", "access-control-allow-credentials": "true", }, + // Ignore whitespace when comparing the response headers. This is because some + // implementations add a space after each comma, and some don't. Both are valid. + IgnoreWhitespace: true, }, }, { @@ -120,6 +123,9 @@ var HTTPRouteCORS = suite.ConformanceTest{ "access-control-max-age": "3600", "access-control-allow-credentials": "true", }, + // Ignore whitespace when comparing the response headers. This is because some + // implementations add a space after each comma, and some don't. Both are valid. + IgnoreWhitespace: true, }, }, { diff --git a/conformance/utils/http/http.go b/conformance/utils/http/http.go index 6cd68dd084..33bae5e051 100644 --- a/conformance/utils/http/http.go +++ b/conformance/utils/http/http.go @@ -98,6 +98,9 @@ type Response struct { Headers map[string]string AbsentHeaders []string Protocol string + // IgnoreWhitespace will cause whitespace to be ignored when comparing the respond + // header values. + IgnoreWhitespace bool } type BackendRef struct { @@ -375,8 +378,14 @@ func CompareRoundTrip(t *testing.T, req *roundtripper.Request, cReq *roundtrippe actualVal, ok := cRes.Headers[strings.ToLower(name)] if !ok { return fmt.Errorf("expected %s header to be set, actual headers: %v", name, cRes.Headers) - } else if strings.Join(actualVal, ",") != expectedVal { - return fmt.Errorf("expected %s header to be set to %s, got %s", name, expectedVal, strings.Join(actualVal, ",")) + } + actualValStr := strings.Join(actualVal, ",") + if expected.Response.IgnoreWhitespace { + actualValStr = strings.ReplaceAll(actualValStr, " ", "") + expectedVal = strings.ReplaceAll(expectedVal, " ", "") + } + if actualValStr != expectedVal { + return fmt.Errorf("expected %s header to be set to %s, got %s", name, expectedVal, actualValStr) } } } From edb78ffa93120fe03377b3f792476081e0406897 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Thu, 10 Jul 2025 06:30:10 +0000 Subject: [PATCH 08/13] support multiple status codes Signed-off-by: Huabing (Robin) Zhao --- conformance/tests/httproute-cors.go | 10 ++-- conformance/utils/http/http.go | 83 +++++++++++++++++++---------- 2 files changed, 61 insertions(+), 32 deletions(-) diff --git a/conformance/tests/httproute-cors.go b/conformance/tests/httproute-cors.go index a59d9a1c99..67ac12dd7c 100644 --- a/conformance/tests/httproute-cors.go +++ b/conformance/tests/httproute-cors.go @@ -75,7 +75,7 @@ var HTTPRouteCORS = suite.ConformanceTest{ }, Namespace: "", Response: http.Response{ - StatusCode: 200, + StatusCodes: []int{200, 204}, Headers: map[string]string{ "access-control-allow-origin": "https://www.foo.com", "access-control-allow-methods": "GET, OPTIONS", @@ -114,7 +114,7 @@ var HTTPRouteCORS = suite.ConformanceTest{ }, Namespace: "", Response: http.Response{ - StatusCode: 200, + StatusCodes: []int{200, 204}, Headers: map[string]string{ "access-control-allow-origin": "https://www.bar.com", "access-control-allow-methods": "GET, OPTIONS", @@ -170,7 +170,7 @@ var HTTPRouteCORS = suite.ConformanceTest{ }, }, Response: http.Response{ - StatusCode: 200, + StatusCodes: []int{200, 204}, Headers: map[string]string{ "access-control-allow-origin": "https://www.foo.com", }, @@ -189,7 +189,7 @@ var HTTPRouteCORS = suite.ConformanceTest{ }, }, Response: http.Response{ - StatusCode: 200, + StatusCodes: []int{200, 204}, Headers: map[string]string{ "access-control-allow-origin": "https://www.bar.com", }, @@ -236,7 +236,7 @@ var HTTPRouteCORS = suite.ConformanceTest{ }, Namespace: "", Response: http.Response{ - StatusCode: 200, + StatusCodes: []int{200, 204}, Headers: map[string]string{ "access-control-allow-origin": "https://www.foo.com", "access-control-allow-methods": "POST", diff --git a/conformance/utils/http/http.go b/conformance/utils/http/http.go index 33bae5e051..49ec7b2158 100644 --- a/conformance/utils/http/http.go +++ b/conformance/utils/http/http.go @@ -332,43 +332,72 @@ func CompareRoundTrip(t *testing.T, req *roundtripper.Request, cReq *roundtrippe expected.ExpectedRequest = &ExpectedRequest{Request: expected.Request} } - if expected.ExpectedRequest.Method == "" { - expected.ExpectedRequest.Method = "GET" - } + if cRes.StatusCode == 200 || cRes.StatusCode == 204 { + if cRes.StatusCode == 200 { + // The request expected to arrive at the backend is + // the same as the request made, unless otherwise + // specified. + if expected.ExpectedRequest == nil { + expected.ExpectedRequest = &ExpectedRequest{Request: expected.Request} + } - if expected.ExpectedRequest.Host != "" && expected.ExpectedRequest.Host != cReq.Host { - return fmt.Errorf("expected host to be %s, got %s", expected.ExpectedRequest.Host, cReq.Host) - } + if expected.ExpectedRequest.Method == "" { + expected.ExpectedRequest.Method = "GET" + } - if expected.ExpectedRequest.Path != cReq.Path { - return fmt.Errorf("expected path to be %s, got %s", expected.ExpectedRequest.Path, cReq.Path) - } - if expected.ExpectedRequest.Method != cReq.Method { - return fmt.Errorf("expected method to be %s, got %s", expected.ExpectedRequest.Method, cReq.Method) - } - if expected.Namespace != cReq.Namespace { - return fmt.Errorf("expected namespace to be %s, got %s", expected.Namespace, cReq.Namespace) - } - if expected.ExpectedRequest.Headers != nil { - if cReq.Headers == nil { - return fmt.Errorf("no headers captured, expected %v", len(expected.ExpectedRequest.Headers)) + if expected.ExpectedRequest.Host != "" && expected.ExpectedRequest.Host != cReq.Host { + return fmt.Errorf("expected host to be %s, got %s", expected.ExpectedRequest.Host, cReq.Host) } - for name, val := range cReq.Headers { - cReq.Headers[strings.ToLower(name)] = val + + if expected.ExpectedRequest.Path != cReq.Path { + return fmt.Errorf("expected path to be %s, got %s", expected.ExpectedRequest.Path, cReq.Path) } - for name, expectedVal := range expected.ExpectedRequest.Headers { - actualVal, ok := cReq.Headers[strings.ToLower(name)] - if !ok { - return fmt.Errorf("expected %s header to be set, actual headers: %v", name, cReq.Headers) - } else if strings.Join(actualVal, ",") != expectedVal { - return fmt.Errorf("expected %s header to be set to %s, got %s", name, expectedVal, strings.Join(actualVal, ",")) + if expected.ExpectedRequest.Method != cReq.Method { + return fmt.Errorf("expected method to be %s, got %s", expected.ExpectedRequest.Method, cReq.Method) + } + if expected.Namespace != cReq.Namespace { + return fmt.Errorf("expected namespace to be %s, got %s", expected.Namespace, cReq.Namespace) + } + if expected.ExpectedRequest.Headers != nil { + if cReq.Headers == nil { + return fmt.Errorf("no headers captured, expected %v", len(expected.ExpectedRequest.Headers)) + } + for name, val := range cReq.Headers { + cReq.Headers[strings.ToLower(name)] = val } + for name, expectedVal := range expected.ExpectedRequest.Headers { + actualVal, ok := cReq.Headers[strings.ToLower(name)] + if !ok { + return fmt.Errorf("expected %s header to be set, actual headers: %v", name, cReq.Headers) + } else if strings.Join(actualVal, ",") != expectedVal { + return fmt.Errorf("expected %s header to be set to %s, got %s", name, expectedVal, strings.Join(actualVal, ",")) + } + } + } + + // Verify that headers expected *not* to be present on the + // request are actually not present. + if len(expected.ExpectedRequest.AbsentHeaders) > 0 { + for name, val := range cReq.Headers { + cReq.Headers[strings.ToLower(name)] = val + } + + for _, name := range expected.ExpectedRequest.AbsentHeaders { + val, ok := cReq.Headers[strings.ToLower(name)] + if ok { + return fmt.Errorf("expected %s header to not be set, got %s", name, val) + } + } + } + + if !strings.HasPrefix(cReq.Pod, expected.Backend) { + return fmt.Errorf("expected pod name to start with %s, got %s", expected.Backend, cReq.Pod) } } if expected.Response.Headers != nil { if cRes.Headers == nil { - return fmt.Errorf("no headers captured, expected %v", len(expected.ExpectedRequest.Headers)) + return fmt.Errorf("no headers captured, expected %v", len(expected.Response.Headers)) } for name, val := range cRes.Headers { cRes.Headers[strings.ToLower(name)] = val From 712a7a797176c2ab0536de0c12e98e7e44640e75 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Thu, 10 Jul 2025 07:37:17 +0000 Subject: [PATCH 09/13] address comment Signed-off-by: Huabing (Robin) Zhao --- conformance/tests/httproute-cors.go | 47 ++++++++++++++++++--------- conformance/utils/http/http.go | 50 ++++++++++++++++++++++------- 2 files changed, 70 insertions(+), 27 deletions(-) diff --git a/conformance/tests/httproute-cors.go b/conformance/tests/httproute-cors.go index 67ac12dd7c..f507093fc4 100644 --- a/conformance/tests/httproute-cors.go +++ b/conformance/tests/httproute-cors.go @@ -76,13 +76,22 @@ var HTTPRouteCORS = suite.ConformanceTest{ Namespace: "", Response: http.Response{ StatusCodes: []int{200, 204}, - Headers: map[string]string{ - "access-control-allow-origin": "https://www.foo.com", - "access-control-allow-methods": "GET, OPTIONS", - "access-control-allow-headers": "x-header-1, x-header-2", - "access-control-expose-headers": "x-header-3, x-header-4", - "access-control-max-age": "3600", - "access-control-allow-credentials": "true", + HeadersWithMultipleValues: map[string][]string{ + "access-control-allow-origin": {"https://www.foo.com"}, + "access-control-allow-methods": { + "GET, OPTIONS", + "OPTIONS, GET", + }, + "access-control-allow-headers": { + "x-header-1, x-header-2", + "x-header-2, x-header-1", + }, + "access-control-expose-headers": { + "x-header-3, x-header-4", + "x-header-4, x-header-3", + }, + "access-control-max-age": {"3600"}, + "access-control-allow-credentials": {"true"}, }, // Ignore whitespace when comparing the response headers. This is because some // implementations add a space after each comma, and some don't. Both are valid. @@ -114,14 +123,22 @@ var HTTPRouteCORS = suite.ConformanceTest{ }, Namespace: "", Response: http.Response{ - StatusCodes: []int{200, 204}, - Headers: map[string]string{ - "access-control-allow-origin": "https://www.bar.com", - "access-control-allow-methods": "GET, OPTIONS", - "access-control-allow-headers": "x-header-1, x-header-2", - "access-control-expose-headers": "x-header-3, x-header-4", - "access-control-max-age": "3600", - "access-control-allow-credentials": "true", + StatusCode: 200, + HeadersWithMultipleValues: map[string][]string{ + "access-control-allow-origin": {"https://www.bar.com"}, + "access-control-allow-methods": { + "GET, OPTIONS", + "OPTIONS, GET"}, + "access-control-allow-headers": { + "x-header-1, x-header-2", + "x-header-2, x-header-1", + }, + "access-control-expose-headers": { + "x-header-3, x-header-4", + "x-header-4, x-header-3", + }, + "access-control-max-age": {"3600"}, + "access-control-allow-credentials": {"true"}, }, // Ignore whitespace when comparing the response headers. This is because some // implementations add a space after each comma, and some don't. Both are valid. diff --git a/conformance/utils/http/http.go b/conformance/utils/http/http.go index 49ec7b2158..e93588c8f6 100644 --- a/conformance/utils/http/http.go +++ b/conformance/utils/http/http.go @@ -395,7 +395,7 @@ func CompareRoundTrip(t *testing.T, req *roundtripper.Request, cReq *roundtrippe } } - if expected.Response.Headers != nil { + if expected.Response.Headers != nil || expected.Response.HeadersWithMultipleValues != nil { if cRes.Headers == nil { return fmt.Errorf("no headers captured, expected %v", len(expected.Response.Headers)) } @@ -403,18 +403,44 @@ func CompareRoundTrip(t *testing.T, req *roundtripper.Request, cReq *roundtrippe cRes.Headers[strings.ToLower(name)] = val } - for name, expectedVal := range expected.Response.Headers { - actualVal, ok := cRes.Headers[strings.ToLower(name)] - if !ok { - return fmt.Errorf("expected %s header to be set, actual headers: %v", name, cRes.Headers) - } - actualValStr := strings.Join(actualVal, ",") - if expected.Response.IgnoreWhitespace { - actualValStr = strings.ReplaceAll(actualValStr, " ", "") - expectedVal = strings.ReplaceAll(expectedVal, " ", "") + if expected.Response.HeadersWithMultipleValues != nil { + for name, expectedVals := range expected.Response.HeadersWithMultipleValues { + actualVal, ok := cRes.Headers[strings.ToLower(name)] + if !ok { + return fmt.Errorf("expected %s header to be set, actual headers: %v", name, cRes.Headers) + } + actualValStr := strings.Join(actualVal, ",") + if expected.Response.IgnoreWhitespace { + actualValStr = strings.ReplaceAll(actualValStr, " ", "") + for i := range expectedVals { + expectedVals[i] = strings.ReplaceAll(expectedVals[i], " ", "") + } + } + matched := false + for _, expectedVal := range expectedVals { + if actualValStr == expectedVal { + matched = true + break + } + } + if !matched { + return fmt.Errorf("expected %s header to be set to one of %v, got %s", name, expectedVals, actualValStr) + } } - if actualValStr != expectedVal { - return fmt.Errorf("expected %s header to be set to %s, got %s", name, expectedVal, actualValStr) + } else { + for name, expectedVal := range expected.Response.Headers { + actualVal, ok := cRes.Headers[strings.ToLower(name)] + if !ok { + return fmt.Errorf("expected %s header to be set, actual headers: %v", name, cRes.Headers) + } + actualValStr := strings.Join(actualVal, ",") + if expected.Response.IgnoreWhitespace { + actualValStr = strings.ReplaceAll(actualValStr, " ", "") + expectedVal = strings.ReplaceAll(expectedVal, " ", "") + } + if actualValStr != expectedVal { + return fmt.Errorf("expected %s header to be set to %s, got %s", name, expectedVal, actualValStr) + } } } } From 4a3b2842ee8d5a6a65e0dd1fc2614bf99073a181 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Thu, 10 Jul 2025 07:43:16 +0000 Subject: [PATCH 10/13] address comment Signed-off-by: Huabing (Robin) Zhao --- conformance/tests/httproute-cors.go | 1 + 1 file changed, 1 insertion(+) diff --git a/conformance/tests/httproute-cors.go b/conformance/tests/httproute-cors.go index f507093fc4..602e67c1dd 100644 --- a/conformance/tests/httproute-cors.go +++ b/conformance/tests/httproute-cors.go @@ -169,6 +169,7 @@ var HTTPRouteCORS = suite.ConformanceTest{ }, Namespace: "", Response: http.Response{ + StatusCodes: []int{200, 204, 403}, AbsentHeaders: []string{ "access-control-allow-origin", }, From c3624c8ced4b41d188a069180f43fe7169e5e8e3 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Thu, 10 Jul 2025 10:04:37 +0000 Subject: [PATCH 11/13] fix lint Signed-off-by: Huabing (Robin) Zhao --- conformance/tests/httproute-cors.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/conformance/tests/httproute-cors.go b/conformance/tests/httproute-cors.go index 602e67c1dd..c4b96f89ea 100644 --- a/conformance/tests/httproute-cors.go +++ b/conformance/tests/httproute-cors.go @@ -128,7 +128,8 @@ var HTTPRouteCORS = suite.ConformanceTest{ "access-control-allow-origin": {"https://www.bar.com"}, "access-control-allow-methods": { "GET, OPTIONS", - "OPTIONS, GET"}, + "OPTIONS, GET", + }, "access-control-allow-headers": { "x-header-1, x-header-2", "x-header-2, x-header-1", From 4150618406a85bb5bf12b5ca38247ee0671c01f1 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Sun, 31 Aug 2025 14:22:09 +0000 Subject: [PATCH 12/13] fix test Signed-off-by: Huabing (Robin) Zhao --- conformance/utils/http/http.go | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/conformance/utils/http/http.go b/conformance/utils/http/http.go index e93588c8f6..94002b92c0 100644 --- a/conformance/utils/http/http.go +++ b/conformance/utils/http/http.go @@ -93,11 +93,12 @@ type ExpectedRequest struct { // Response defines expected properties of a response from a backend. type Response struct { // Deprecated: Use StatusCodes instead, which supports matching against multiple status codes. - StatusCode int - StatusCodes []int - Headers map[string]string - AbsentHeaders []string - Protocol string + StatusCode int + StatusCodes []int + Headers map[string]string + HeadersWithMultipleValues map[string][]string + AbsentHeaders []string + Protocol string // IgnoreWhitespace will cause whitespace to be ignored when comparing the respond // header values. IgnoreWhitespace bool @@ -324,14 +325,6 @@ func CompareRoundTrip(t *testing.T, req *roundtripper.Request, cReq *roundtrippe return fmt.Errorf("expected protocol to be %s, got %s", expected.Response.Protocol, cRes.Protocol) } - if cRes.StatusCode == 200 { - // The request expected to arrive at the backend is - // the same as the request made, unless otherwise - // specified. - if expected.ExpectedRequest == nil { - expected.ExpectedRequest = &ExpectedRequest{Request: expected.Request} - } - if cRes.StatusCode == 200 || cRes.StatusCode == 204 { if cRes.StatusCode == 200 { // The request expected to arrive at the backend is From a224d52deb5c167df9e0caf4d18131db514880d1 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Sun, 31 Aug 2025 14:28:11 +0000 Subject: [PATCH 13/13] address comment Signed-off-by: Huabing (Robin) Zhao --- conformance/tests/httproute-cors.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conformance/tests/httproute-cors.go b/conformance/tests/httproute-cors.go index c4b96f89ea..6c2ad8719c 100644 --- a/conformance/tests/httproute-cors.go +++ b/conformance/tests/httproute-cors.go @@ -1,5 +1,5 @@ /* -Copyright 2022 The Kubernetes Authors. +Copyright 2025 The Kubernetes Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License.