From 2c71ee582c60e952124d33d64a3c2b5e1f37f476 Mon Sep 17 00:00:00 2001 From: Arko Dasgupta Date: Tue, 1 Aug 2023 13:48:18 -0700 Subject: [PATCH 1/9] Add E2E for EnvoyPatchPolicy * Use LocalReplyConfig to return a custom status code `418` when there is no valid route match Signed-off-by: Arko Dasgupta --- test/e2e/testdata/envoy-patch-policy.yaml | 50 +++++++++++++++++++++++ test/e2e/tests/envoy-patch-policy.go | 49 ++++++++++++++++++++++ 2 files changed, 99 insertions(+) create mode 100644 test/e2e/testdata/envoy-patch-policy.yaml create mode 100644 test/e2e/tests/envoy-patch-policy.go diff --git a/test/e2e/testdata/envoy-patch-policy.yaml b/test/e2e/testdata/envoy-patch-policy.yaml new file mode 100644 index 0000000000..c30fdf67b3 --- /dev/null +++ b/test/e2e/testdata/envoy-patch-policy.yaml @@ -0,0 +1,50 @@ +--- +apiVersion: gateway.networking.k8s.io/v1beta1 +kind: HTTPRoute +metadata: + name: http-envoy-patch-policy + namespace: gateway-conformance-infra +spec: + parentRefs: + - name: same-namespace + rules: + - backendRefs: + - name: infra-backend-v1 + port: 8080 + matches: + - path: + type: PathPrefix + value: /foo +--- +apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: EnvoyPatchPolicy + metadata: + name: custom-response-patch-policy + namespace: default + spec: + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: same-namespace + namespace: gateway-conformance-infra + type: JSONPatch + jsonPatches: + - type: "type.googleapis.com/envoy.config.listener.v3.Listener" + # The listener name is of the form // + name: gateway-conformance-infra/same-namespace/http + operation: + op: add + path: "/default_filter_chain/filters/0/typed_config" + value: + local_reply_config: + mappers: + - filter: + status_code_filter: + comparison: + op: EQ + value: + default_value: 404 + runtime_key: key_b + status_code: 418 + body: + inline_string: "im a teapot" diff --git a/test/e2e/tests/envoy-patch-policy.go b/test/e2e/tests/envoy-patch-policy.go new file mode 100644 index 0000000000..caace95582 --- /dev/null +++ b/test/e2e/tests/envoy-patch-policy.go @@ -0,0 +1,49 @@ +// Copyright Envoy Gateway Authors +// SPDX-License-Identifier: Apache-2.0 +// The full text of the Apache license is available in the LICENSE file at +// the root of the repo. + +//go:build e2e +// +build e2e + +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/roundtripper" + "sigs.k8s.io/gateway-api/conformance/utils/suite" +) + +func init() { + ConformanceTests = append(ConformanceTests, EnvoyPatchPolicyTest) +} + +var EnvoyPatchPolicyTest = suite.ConformanceTest{ + ShortName: "EnvoyPatchPolicy", + Description: "update xds using EnvoyPatchPolicy", + Manifests: []string{"testdata/envoy-patch-policy.yaml"}, + Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { + t.Run("envoy patch policy", func(t *testing.T) { + ns := "gateway-conformance-infra" + routeNN := types.NamespacedName{Name: "http-envoy-patch-policy", Namespace: ns} + gwNN := types.NamespacedName{Name: "same-namespace", Namespace: ns} + gwAddr := kubernetes.GatewayAndHTTPRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), routeNN) + customResp := http.ExpectedResponse{ + Request: http.Request{ + Path: "/bar", + }, + Response: http.Response{ + StatusCode: 418, + }, + Namespace: ns, + } + + // Send a request to an invalid path and expect a custom response + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, customResp) + }) + }, +} From 0d0d3d37b760a45158ad45c61bee09dd99a37ba2 Mon Sep 17 00:00:00 2001 From: Arko Dasgupta Date: Tue, 1 Aug 2023 13:59:13 -0700 Subject: [PATCH 2/9] lint Signed-off-by: Arko Dasgupta --- test/e2e/testdata/envoy-patch-policy.yaml | 64 +++++++++++------------ test/e2e/tests/envoy-patch-policy.go | 1 - 2 files changed, 32 insertions(+), 33 deletions(-) diff --git a/test/e2e/testdata/envoy-patch-policy.yaml b/test/e2e/testdata/envoy-patch-policy.yaml index c30fdf67b3..3f8e83bb4b 100644 --- a/test/e2e/testdata/envoy-patch-policy.yaml +++ b/test/e2e/testdata/envoy-patch-policy.yaml @@ -2,7 +2,7 @@ apiVersion: gateway.networking.k8s.io/v1beta1 kind: HTTPRoute metadata: - name: http-envoy-patch-policy + name: http-envoy-patch-policy namespace: gateway-conformance-infra spec: parentRefs: @@ -17,34 +17,34 @@ spec: value: /foo --- apiVersion: gateway.envoyproxy.io/v1alpha1 - kind: EnvoyPatchPolicy - metadata: - name: custom-response-patch-policy - namespace: default - spec: - targetRef: - group: gateway.networking.k8s.io - kind: Gateway - name: same-namespace - namespace: gateway-conformance-infra - type: JSONPatch - jsonPatches: - - type: "type.googleapis.com/envoy.config.listener.v3.Listener" - # The listener name is of the form // - name: gateway-conformance-infra/same-namespace/http - operation: - op: add - path: "/default_filter_chain/filters/0/typed_config" - value: - local_reply_config: - mappers: - - filter: - status_code_filter: - comparison: - op: EQ - value: - default_value: 404 - runtime_key: key_b - status_code: 418 - body: - inline_string: "im a teapot" +kind: EnvoyPatchPolicy +metadata: + name: custom-response-patch-policy + namespace: default +spec: + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: same-namespace + namespace: gateway-conformance-infra + type: JSONPatch + jsonPatches: + - type: "type.googleapis.com/envoy.config.listener.v3.Listener" + # The listener name is of the form // + name: gateway-conformance-infra/same-namespace/http + operation: + op: add + path: "/default_filter_chain/filters/0/typed_config" + value: + local_reply_config: + mappers: + - filter: + status_code_filter: + comparison: + op: EQ + value: + default_value: 404 + runtime_key: key_b + status_code: 418 + body: + inline_string: "im a teapot" diff --git a/test/e2e/tests/envoy-patch-policy.go b/test/e2e/tests/envoy-patch-policy.go index caace95582..af0672e37b 100644 --- a/test/e2e/tests/envoy-patch-policy.go +++ b/test/e2e/tests/envoy-patch-policy.go @@ -14,7 +14,6 @@ import ( "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/roundtripper" "sigs.k8s.io/gateway-api/conformance/utils/suite" ) From 850a74437104a1b805a9237630114a710685e56e Mon Sep 17 00:00:00 2001 From: Arko Dasgupta Date: Mon, 31 Jul 2023 17:51:52 -0700 Subject: [PATCH 3/9] fix: misc bug fixes for EnvoyPatchPolicy * Only add EnvoyPatchPolicy to provider resources if enabled in EnvoyGateway API * Add `omitempty` tag to `priority` field to make it optional Signed-off-by: Arko Dasgupta (cherry picked from commit 93c92b93cf2302fd10890b1eb821cf320f346fb9) --- api/v1alpha1/envoypatchpolicy_types.go | 2 +- ...eway.envoyproxy.io_envoypatchpolicies.yaml | 1 - internal/provider/kubernetes/controller.go | 23 +++++++++++-------- 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/api/v1alpha1/envoypatchpolicy_types.go b/api/v1alpha1/envoypatchpolicy_types.go index 8234ead224..012ac42a98 100644 --- a/api/v1alpha1/envoypatchpolicy_types.go +++ b/api/v1alpha1/envoypatchpolicy_types.go @@ -59,7 +59,7 @@ type EnvoyPatchPolicySpec struct { // the priority i.e. int32.min has the highest priority and // int32.max has the lowest priority. // Defaults to 0. - Priority int32 `json:"priority"` + Priority int32 `json:"priority,omitempty"` } // EnvoyPatchType specifies the types of Envoy patching mechanisms. diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_envoypatchpolicies.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_envoypatchpolicies.yaml index c336131fcc..7296a9e512 100644 --- a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_envoypatchpolicies.yaml +++ b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_envoypatchpolicies.yaml @@ -142,7 +142,6 @@ spec: - JSONPatch type: string required: - - priority - targetRef - type type: object diff --git a/internal/provider/kubernetes/controller.go b/internal/provider/kubernetes/controller.go index 25255cd0c4..1e26fd0bde 100644 --- a/internal/provider/kubernetes/controller.go +++ b/internal/provider/kubernetes/controller.go @@ -240,16 +240,19 @@ func (r *gatewayAPIReconciler) Reconcile(ctx context.Context, request reconcile. } // Add all EnvoyPatchPolicies - envoyPatchPolicies := egv1a1.EnvoyPatchPolicyList{} - if err := r.client.List(ctx, &envoyPatchPolicies); err != nil { - return reconcile.Result{}, fmt.Errorf("error listing envoypatchpolicies: %v", err) - } - for _, policy := range envoyPatchPolicies.Items { - policy := policy - // Discard Status to reduce memory consumption in watchable - // It will be recomputed by the gateway-api layer - policy.Status = egv1a1.EnvoyPatchPolicyStatus{} - resourceTree.EnvoyPatchPolicies = append(resourceTree.EnvoyPatchPolicies, &policy) + if r.envoyGateway.ExtensionAPIs != nil && r.envoyGateway.ExtensionAPIs.EnableEnvoyPatchPolicy { + envoyPatchPolicies := egv1a1.EnvoyPatchPolicyList{} + if err := r.client.List(ctx, &envoyPatchPolicies); err != nil { + return reconcile.Result{}, fmt.Errorf("error listing envoypatchpolicies: %v", err) + } + + for _, policy := range envoyPatchPolicies.Items { + policy := policy + // Discard Status to reduce memory consumption in watchable + // It will be recomputed by the gateway-api layer + policy.Status = egv1a1.EnvoyPatchPolicyStatus{} + resourceTree.EnvoyPatchPolicies = append(resourceTree.EnvoyPatchPolicies, &policy) + } } // For this particular Gateway, and all associated objects, check whether the From 0023d92ab6ac9f22203b2cd8bedd41a7abafff5c Mon Sep 17 00:00:00 2001 From: Arko Dasgupta Date: Tue, 1 Aug 2023 15:02:51 -0700 Subject: [PATCH 4/9] enable in envoygateway config Signed-off-by: Arko Dasgupta --- examples/redis/redis.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/examples/redis/redis.yaml b/examples/redis/redis.yaml index c0b0992199..74f6f6788d 100644 --- a/examples/redis/redis.yaml +++ b/examples/redis/redis.yaml @@ -62,6 +62,8 @@ data: type: Kubernetes gateway: controllerName: gateway.envoyproxy.io/gatewayclass-controller + extensionApis: + enableEnvoyPatchPolicy: true rateLimit: backend: type: Redis From eaebbab2ba75bd676de7d04fc61c652dfd0ac978 Mon Sep 17 00:00:00 2001 From: Arko Dasgupta Date: Tue, 1 Aug 2023 15:55:52 -0700 Subject: [PATCH 5/9] use a different status code Signed-off-by: Arko Dasgupta --- test/e2e/testdata/envoy-patch-policy.yaml | 4 ++-- test/e2e/tests/envoy-patch-policy.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/e2e/testdata/envoy-patch-policy.yaml b/test/e2e/testdata/envoy-patch-policy.yaml index 3f8e83bb4b..412dfc432c 100644 --- a/test/e2e/testdata/envoy-patch-policy.yaml +++ b/test/e2e/testdata/envoy-patch-policy.yaml @@ -45,6 +45,6 @@ spec: value: default_value: 404 runtime_key: key_b - status_code: 418 + status_code: 406 body: - inline_string: "im a teapot" + inline_string: "not acceptable" diff --git a/test/e2e/tests/envoy-patch-policy.go b/test/e2e/tests/envoy-patch-policy.go index af0672e37b..e0f055f1c6 100644 --- a/test/e2e/tests/envoy-patch-policy.go +++ b/test/e2e/tests/envoy-patch-policy.go @@ -36,7 +36,7 @@ var EnvoyPatchPolicyTest = suite.ConformanceTest{ Path: "/bar", }, Response: http.Response{ - StatusCode: 418, + StatusCode: 406, }, Namespace: ns, } From f3662d55285f37810ea382320fc8a6a7f3b3c7ca Mon Sep 17 00:00:00 2001 From: Arko Dasgupta Date: Tue, 1 Aug 2023 16:35:17 -0700 Subject: [PATCH 6/9] send a ok response first Signed-off-by: Arko Dasgupta --- test/e2e/tests/envoy-patch-policy.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/e2e/tests/envoy-patch-policy.go b/test/e2e/tests/envoy-patch-policy.go index e0f055f1c6..08f0e43fc4 100644 --- a/test/e2e/tests/envoy-patch-policy.go +++ b/test/e2e/tests/envoy-patch-policy.go @@ -31,6 +31,19 @@ var EnvoyPatchPolicyTest = suite.ConformanceTest{ routeNN := types.NamespacedName{Name: "http-envoy-patch-policy", Namespace: ns} gwNN := types.NamespacedName{Name: "same-namespace", Namespace: ns} gwAddr := kubernetes.GatewayAndHTTPRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), routeNN) + OkResp := http.ExpectedResponse{ + Request: http.Request{ + Path: "/foo", + }, + Response: http.Response{ + StatusCode: 200, + }, + Namespace: ns, + } + + // Send a request to an valid path and expect a sucessful response + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, OkResp) + customResp := http.ExpectedResponse{ Request: http.Request{ Path: "/bar", From c78dcc951f623a5d51e88ac1bb7822ee1a32bc23 Mon Sep 17 00:00:00 2001 From: Arko Dasgupta Date: Tue, 1 Aug 2023 16:48:15 -0700 Subject: [PATCH 7/9] lint Signed-off-by: Arko Dasgupta --- test/e2e/tests/envoy-patch-policy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/tests/envoy-patch-policy.go b/test/e2e/tests/envoy-patch-policy.go index 08f0e43fc4..2486518ac3 100644 --- a/test/e2e/tests/envoy-patch-policy.go +++ b/test/e2e/tests/envoy-patch-policy.go @@ -41,7 +41,7 @@ var EnvoyPatchPolicyTest = suite.ConformanceTest{ Namespace: ns, } - // Send a request to an valid path and expect a sucessful response + // Send a request to an valid path and expect a successful response http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, OkResp) customResp := http.ExpectedResponse{ From 45afb18a15feeb1c64ae440bce3f168f0e1bacce Mon Sep 17 00:00:00 2001 From: Arko Dasgupta Date: Tue, 1 Aug 2023 22:05:54 -0700 Subject: [PATCH 8/9] fix yaml && status Signed-off-by: Arko Dasgupta --- internal/status/envoypatchpolicy.go | 3 +++ test/e2e/testdata/envoy-patch-policy.yaml | 30 +++++++++++------------ 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/internal/status/envoypatchpolicy.go b/internal/status/envoypatchpolicy.go index 950ce1f58a..05deb89d15 100644 --- a/internal/status/envoypatchpolicy.go +++ b/internal/status/envoypatchpolicy.go @@ -25,6 +25,9 @@ func SetEnvoyPatchPolicyProgrammedIfUnset(s *egv1a1.EnvoyPatchPolicyStatus, mess if c.Type == string(egv1a1.PolicyConditionProgrammed) { return } + if c.Type == string(gwv1a2.PolicyConditionAccepted) && c.Status == metav1.ConditionFalse { + return + } } cond := newCondition(string(egv1a1.PolicyConditionProgrammed), metav1.ConditionTrue, string(egv1a1.PolicyReasonProgrammed), message, time.Now(), 0) diff --git a/test/e2e/testdata/envoy-patch-policy.yaml b/test/e2e/testdata/envoy-patch-policy.yaml index 412dfc432c..fa63ead7c8 100644 --- a/test/e2e/testdata/envoy-patch-policy.yaml +++ b/test/e2e/testdata/envoy-patch-policy.yaml @@ -20,7 +20,7 @@ apiVersion: gateway.envoyproxy.io/v1alpha1 kind: EnvoyPatchPolicy metadata: name: custom-response-patch-policy - namespace: default + namespace: gateway-conformance-infra spec: targetRef: group: gateway.networking.k8s.io @@ -30,21 +30,19 @@ spec: type: JSONPatch jsonPatches: - type: "type.googleapis.com/envoy.config.listener.v3.Listener" - # The listener name is of the form // - name: gateway-conformance-infra/same-namespace/http + name: "gateway-conformance-infra/same-namespace/http" operation: op: add - path: "/default_filter_chain/filters/0/typed_config" + path: "/default_filter_chain/filters/0/typed_config/local_reply_config" value: - local_reply_config: - mappers: - - filter: - status_code_filter: - comparison: - op: EQ - value: - default_value: 404 - runtime_key: key_b - status_code: 406 - body: - inline_string: "not acceptable" + mappers: + - filter: + status_code_filter: + comparison: + op: EQ + value: + default_value: 404 + runtime_key: key_b + status_code: 406 + body: + inline_string: "not acceptable" From 0bac03869e9da3ef9d5b8a988e442431b4e29aa4 Mon Sep 17 00:00:00 2001 From: Arko Dasgupta Date: Tue, 1 Aug 2023 22:16:26 -0700 Subject: [PATCH 9/9] fix docs Signed-off-by: Arko Dasgupta --- docs/latest/user/envoy-patch-policy.md | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/docs/latest/user/envoy-patch-policy.md b/docs/latest/user/envoy-patch-policy.md index a06e5b416c..fc0f63fb31 100644 --- a/docs/latest/user/envoy-patch-policy.md +++ b/docs/latest/user/envoy-patch-policy.md @@ -86,19 +86,19 @@ spec: name: default/eg/http operation: op: add - path: "/default_filter_chain/filters/0/typed_config" + path: "/default_filter_chain/filters/0/typed_config/local_reply_config" value: - local_reply_config: - mappers: - - filter: - status_code_filter: - comparison: - op: EQ - value: - default_value: 404 - runtime_key: key_b - body: - inline_string: "could not find what you are looking for" + mappers: + - filter: + status_code_filter: + comparison: + op: EQ + value: + default_value: 404 + runtime_key: key_b + status_code: 406 + body: + inline_string: "could not find what you are looking for" EOF ```