From b74a2e949b58b4da32449d46005067ee95a2f97f Mon Sep 17 00:00:00 2001 From: dprotaso Date: Mon, 26 Feb 2024 17:02:47 -0500 Subject: [PATCH 1/4] HTTPBackendRef RequestMirror Conformance I dropped path logic in filter so we can re-use the same test for the backend --- .../httproute-request-mirror-backend.yaml | 20 ++++++++++ conformance/tests/httproute-request-mirror.go | 39 +++++++------------ .../tests/httproute-request-mirror.yaml | 33 +--------------- pkg/features/features.go | 4 ++ 4 files changed, 38 insertions(+), 58 deletions(-) create mode 100644 conformance/tests/httproute-request-mirror-backend.yaml diff --git a/conformance/tests/httproute-request-mirror-backend.yaml b/conformance/tests/httproute-request-mirror-backend.yaml new file mode 100644 index 0000000000..5ab311a640 --- /dev/null +++ b/conformance/tests/httproute-request-mirror-backend.yaml @@ -0,0 +1,20 @@ +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: request-mirror + namespace: gateway-conformance-infra +spec: + parentRefs: + - name: same-namespace + rules: + - backendRefs: + - name: infra-backend-v1 + namespace: gateway-conformance-infra + port: 8080 + filters: + - type: RequestMirror + requestMirror: + backendRef: + name: infra-backend-v2 + namespace: gateway-conformance-infra + port: 8080 diff --git a/conformance/tests/httproute-request-mirror.go b/conformance/tests/httproute-request-mirror.go index fa4c36b44b..43d53f25ef 100644 --- a/conformance/tests/httproute-request-mirror.go +++ b/conformance/tests/httproute-request-mirror.go @@ -29,6 +29,19 @@ import ( func init() { ConformanceTests = append(ConformanceTests, HTTPRouteRequestMirror) + ConformanceTests = append(ConformanceTests, HTTPRouteRequestMirrorBackend) +} + +var HTTPRouteRequestMirrorBackend = suite.ConformanceTest{ + ShortName: "HTTPRouteRequestMirrorBackend", + Description: "An HTTPRoute with request mirror filter on the backendRef", + Manifests: []string{"tests/httproute-request-mirror-backend.yaml"}, + Features: []suite.SupportedFeature{ + suite.SupportGateway, + suite.SupportHTTPRoute, + suite.SupportHTTPRouteRequestMirrorBackend, + }, + Test: HTTPRouteRequestMirror.Test, } var HTTPRouteRequestMirror = suite.ConformanceTest{ @@ -63,32 +76,6 @@ var HTTPRouteRequestMirror = suite.ConformanceTest{ }}, Namespace: ns, }, - { - Request: http.Request{ - Path: "/mirror-and-modify-headers", - Headers: map[string]string{ - "X-Header-Remove": "remove-val", - "X-Header-Add-Append": "append-val-1", - }, - }, - ExpectedRequest: &http.ExpectedRequest{ - Request: http.Request{ - Path: "/mirror-and-modify-headers", - Headers: map[string]string{ - "X-Header-Add": "header-val-1", - "X-Header-Add-Append": "append-val-1,header-val-2", - "X-Header-Set": "set-overwrites-values", - }, - }, - AbsentHeaders: []string{"X-Header-Remove"}, - }, - Namespace: ns, - Backend: "infra-backend-v1", - MirroredTo: []http.BackendRef{{ - Name: "infra-backend-v2", - Namespace: ns, - }}, - }, } for i := range testCases { // Declare tc here to avoid loop variable diff --git a/conformance/tests/httproute-request-mirror.yaml b/conformance/tests/httproute-request-mirror.yaml index 4420202b9d..18c0f8124d 100644 --- a/conformance/tests/httproute-request-mirror.yaml +++ b/conformance/tests/httproute-request-mirror.yaml @@ -7,38 +7,7 @@ spec: parentRefs: - name: same-namespace rules: - - matches: - - path: - type: PathPrefix - value: /mirror - filters: - - type: RequestMirror - requestMirror: - backendRef: - name: infra-backend-v2 - namespace: gateway-conformance-infra - port: 8080 - backendRefs: - - name: infra-backend-v1 - port: 8080 - namespace: gateway-conformance-infra - - matches: - - path: - type: PathPrefix - value: /mirror-and-modify-headers - filters: - - type: RequestHeaderModifier - requestHeaderModifier: - set: - - name: X-Header-Set - value: set-overwrites-values - add: - - name: X-Header-Add - value: header-val-1 - - name: X-Header-Add-Append - value: header-val-2 - remove: - - X-Header-Remove + - filters: - type: RequestMirror requestMirror: backendRef: diff --git a/pkg/features/features.go b/pkg/features/features.go index fcb6d28019..7dfee384b4 100644 --- a/pkg/features/features.go +++ b/pkg/features/features.go @@ -128,6 +128,9 @@ const ( // This option indicates support for HTTPRoute request mirror (extended conformance). SupportHTTPRouteRequestMirror SupportedFeature = "HTTPRouteRequestMirror" + // This option indicates support for HTTPRoute backend request mirror (extended conformance). + SupportHTTPRouteRequestMirrorBackend SupportedFeature = "HTTPRouteRequestMirrorBackend" + // This option indicates support for multiple RequestMirror filters within the same HTTPRoute rule (extended conformance). SupportHTTPRouteRequestMultipleMirrors SupportedFeature = "HTTPRouteRequestMultipleMirrors" @@ -154,6 +157,7 @@ var HTTPRouteExtendedFeatures = sets.New( SupportHTTPRouteHostRewrite, SupportHTTPRoutePathRewrite, SupportHTTPRouteRequestMirror, + SupportHTTPRouteRequestMirrorBackend, SupportHTTPRouteRequestMultipleMirrors, SupportHTTPRouteRequestTimeout, SupportHTTPRouteBackendTimeout, From 5b4773d527f7de1633e9663e5c300c63fc7a4b73 Mon Sep 17 00:00:00 2001 From: dprotaso Date: Mon, 26 Feb 2024 21:33:35 -0500 Subject: [PATCH 2/4] rename feature --- conformance/tests/httproute-request-mirror.go | 14 ++++++++------ pkg/features/features.go | 4 ++-- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/conformance/tests/httproute-request-mirror.go b/conformance/tests/httproute-request-mirror.go index 43d53f25ef..9f8fd3ad9c 100644 --- a/conformance/tests/httproute-request-mirror.go +++ b/conformance/tests/httproute-request-mirror.go @@ -28,18 +28,20 @@ import ( ) func init() { - ConformanceTests = append(ConformanceTests, HTTPRouteRequestMirror) - ConformanceTests = append(ConformanceTests, HTTPRouteRequestMirrorBackend) + ConformanceTests = append(ConformanceTests, + HTTPRouteRequestMirror, + HTTPRouteBackendRequestMirror, + ) } -var HTTPRouteRequestMirrorBackend = suite.ConformanceTest{ - ShortName: "HTTPRouteRequestMirrorBackend", - Description: "An HTTPRoute with request mirror filter on the backendRef", +var HTTPRouteBackendRequestMirror = suite.ConformanceTest{ + ShortName: "HTTPRouteBackendRequestMirror", + Description: "An HTTPRoute backendRef with request mirror filter", Manifests: []string{"tests/httproute-request-mirror-backend.yaml"}, Features: []suite.SupportedFeature{ suite.SupportGateway, suite.SupportHTTPRoute, - suite.SupportHTTPRouteRequestMirrorBackend, + suite.SupportHTTPRouteBackendRequestMirror, }, Test: HTTPRouteRequestMirror.Test, } diff --git a/pkg/features/features.go b/pkg/features/features.go index 7dfee384b4..aa26fae374 100644 --- a/pkg/features/features.go +++ b/pkg/features/features.go @@ -129,7 +129,7 @@ const ( SupportHTTPRouteRequestMirror SupportedFeature = "HTTPRouteRequestMirror" // This option indicates support for HTTPRoute backend request mirror (extended conformance). - SupportHTTPRouteRequestMirrorBackend SupportedFeature = "HTTPRouteRequestMirrorBackend" + SupportHTTPRouteBackendRequestMirror SupportedFeature = "HTTPRouteBackendRequestMirror" // This option indicates support for multiple RequestMirror filters within the same HTTPRoute rule (extended conformance). SupportHTTPRouteRequestMultipleMirrors SupportedFeature = "HTTPRouteRequestMultipleMirrors" @@ -157,7 +157,7 @@ var HTTPRouteExtendedFeatures = sets.New( SupportHTTPRouteHostRewrite, SupportHTTPRoutePathRewrite, SupportHTTPRouteRequestMirror, - SupportHTTPRouteRequestMirrorBackend, + SupportHTTPRouteBackendRequestMirror, SupportHTTPRouteRequestMultipleMirrors, SupportHTTPRouteRequestTimeout, SupportHTTPRouteBackendTimeout, From 1142d0e8ff8267e5a6f7856b43675bd19c65cc5a Mon Sep 17 00:00:00 2001 From: dprotaso Date: Wed, 3 Apr 2024 16:07:32 -0400 Subject: [PATCH 3/4] Preserve existing test cases for top level filter --- conformance/tests/httproute-request-mirror.go | 100 ++++++++++++------ .../tests/httproute-request-mirror.yaml | 33 +++++- 2 files changed, 99 insertions(+), 34 deletions(-) diff --git a/conformance/tests/httproute-request-mirror.go b/conformance/tests/httproute-request-mirror.go index 9f8fd3ad9c..5bca3d8551 100644 --- a/conformance/tests/httproute-request-mirror.go +++ b/conformance/tests/httproute-request-mirror.go @@ -43,7 +43,9 @@ var HTTPRouteBackendRequestMirror = suite.ConformanceTest{ suite.SupportHTTPRoute, suite.SupportHTTPRouteBackendRequestMirror, }, - Test: HTTPRouteRequestMirror.Test, + Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { + RunHTTPRouteRequestMirrorTest(t, suite, HTTPRouteRequestMirrorTestCases) + }, } var HTTPRouteRequestMirror = suite.ConformanceTest{ @@ -56,38 +58,70 @@ var HTTPRouteRequestMirror = suite.ConformanceTest{ features.SupportHTTPRouteRequestMirror, }, Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { - ns := "gateway-conformance-infra" - routeNN := types.NamespacedName{Name: "request-mirror", Namespace: ns} - gwNN := types.NamespacedName{Name: "same-namespace", Namespace: ns} - gwAddr := kubernetes.GatewayAndHTTPRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), routeNN) + RunHTTPRouteRequestMirrorTest(t, suite, HTTPRouteRequestMirrorTestCases) + RunHTTPRouteRequestMirrorTest(t, suite, HTTPRouteRequestMirrorAndModifyRequestHeaderTestCases) + }, +} - testCases := []http.ExpectedResponse{ - { - Request: http.Request{ - Path: "/mirror", - }, - ExpectedRequest: &http.ExpectedRequest{ - Request: http.Request{ - Path: "/mirror", - }, - }, - Backend: "infra-backend-v1", - MirroredTo: []http.BackendRef{{ - Name: "infra-backend-v2", - Namespace: ns, - }}, - Namespace: ns, +func RunHTTPRouteRequestMirrorTest(t *testing.T, suite *suite.ConformanceTestSuite, testCases []http.ExpectedResponse) { + ns := "gateway-conformance-infra" + routeNN := types.NamespacedName{Name: "request-mirror", Namespace: ns} + gwNN := types.NamespacedName{Name: "same-namespace", Namespace: ns} + gwAddr := kubernetes.GatewayAndHTTPRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), routeNN) + + 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) + http.ExpectMirroredRequest(t, suite.Client, suite.Clientset, tc.MirroredTo, tc.Request.Path) + }) + } + +} + +var HTTPRouteRequestMirrorTestCases = []http.ExpectedResponse{{ + Request: http.Request{ + Path: "/mirror", + }, + ExpectedRequest: &http.ExpectedRequest{ + Request: http.Request{ + Path: "/mirror", + }, + }, + Backend: "infra-backend-v1", + MirroredTo: []http.BackendRef{{ + Name: "infra-backend-v2", + Namespace: "gateway-conformance-infra", + }}, + Namespace: "gateway-conformance-infra", +}} + +var HTTPRouteRequestMirrorAndModifyRequestHeaderTestCases = []http.ExpectedResponse{{ + Request: http.Request{ + Path: "/mirror-and-modify-headers", + Headers: map[string]string{ + "X-Header-Remove": "remove-val", + "X-Header-Add-Append": "append-val-1", + }, + }, + ExpectedRequest: &http.ExpectedRequest{ + Request: http.Request{ + Path: "/mirror-and-modify-headers", + Headers: map[string]string{ + "X-Header-Add": "header-val-1", + "X-Header-Add-Append": "append-val-1,header-val-2", + "X-Header-Set": "set-overwrites-values", }, - } - 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) - http.ExpectMirroredRequest(t, suite.Client, suite.Clientset, tc.MirroredTo, tc.Request.Path) - }) - } + }, + AbsentHeaders: []string{"X-Header-Remove"}, }, -} + Namespace: "gateway-conformance-infra", + Backend: "infra-backend-v1", + MirroredTo: []http.BackendRef{{ + Name: "infra-backend-v2", + Namespace: "gateway-conformance-infra", + }}, +}} diff --git a/conformance/tests/httproute-request-mirror.yaml b/conformance/tests/httproute-request-mirror.yaml index 18c0f8124d..4420202b9d 100644 --- a/conformance/tests/httproute-request-mirror.yaml +++ b/conformance/tests/httproute-request-mirror.yaml @@ -7,7 +7,38 @@ spec: parentRefs: - name: same-namespace rules: - - filters: + - matches: + - path: + type: PathPrefix + value: /mirror + filters: + - type: RequestMirror + requestMirror: + backendRef: + name: infra-backend-v2 + namespace: gateway-conformance-infra + port: 8080 + backendRefs: + - name: infra-backend-v1 + port: 8080 + namespace: gateway-conformance-infra + - matches: + - path: + type: PathPrefix + value: /mirror-and-modify-headers + filters: + - type: RequestHeaderModifier + requestHeaderModifier: + set: + - name: X-Header-Set + value: set-overwrites-values + add: + - name: X-Header-Add + value: header-val-1 + - name: X-Header-Add-Append + value: header-val-2 + remove: + - X-Header-Remove - type: RequestMirror requestMirror: backendRef: From eb2197f993e8698c660b9dfe1db1b2bb4bcc2b02 Mon Sep 17 00:00:00 2001 From: dprotaso Date: Wed, 17 Apr 2024 20:28:35 -0400 Subject: [PATCH 4/4] rebase --- conformance/tests/httproute-request-mirror.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/conformance/tests/httproute-request-mirror.go b/conformance/tests/httproute-request-mirror.go index 5bca3d8551..bdd6eaad93 100644 --- a/conformance/tests/httproute-request-mirror.go +++ b/conformance/tests/httproute-request-mirror.go @@ -38,10 +38,10 @@ var HTTPRouteBackendRequestMirror = suite.ConformanceTest{ ShortName: "HTTPRouteBackendRequestMirror", Description: "An HTTPRoute backendRef with request mirror filter", Manifests: []string{"tests/httproute-request-mirror-backend.yaml"}, - Features: []suite.SupportedFeature{ - suite.SupportGateway, - suite.SupportHTTPRoute, - suite.SupportHTTPRouteBackendRequestMirror, + Features: []features.SupportedFeature{ + features.SupportGateway, + features.SupportHTTPRoute, + features.SupportHTTPRouteBackendRequestMirror, }, Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { RunHTTPRouteRequestMirrorTest(t, suite, HTTPRouteRequestMirrorTestCases) @@ -79,7 +79,6 @@ func RunHTTPRouteRequestMirrorTest(t *testing.T, suite *suite.ConformanceTestSui http.ExpectMirroredRequest(t, suite.Client, suite.Clientset, tc.MirroredTo, tc.Request.Path) }) } - } var HTTPRouteRequestMirrorTestCases = []http.ExpectedResponse{{