From a7784c5be3a949e9ac01577598b5c4148c2a8ae8 Mon Sep 17 00:00:00 2001 From: Arko Dasgupta Date: Tue, 1 Aug 2023 22:51:44 -0700 Subject: [PATCH] e2e & misc fixes for EnvoyPatchPolicy (#1738) * Add E2E for EnvoyPatchPolicy * Use LocalReplyConfig to return a custom status code `406` when there is no valid route match Signed-off-by: Arko Dasgupta --- api/v1alpha1/envoypatchpolicy_types.go | 2 +- ...eway.envoyproxy.io_envoypatchpolicies.yaml | 1 - docs/latest/user/envoy-patch-policy.md | 24 ++++---- examples/redis/redis.yaml | 2 + internal/provider/kubernetes/controller.go | 23 ++++--- internal/status/envoypatchpolicy.go | 3 + test/e2e/testdata/envoy-patch-policy.yaml | 48 +++++++++++++++ test/e2e/tests/envoy-patch-policy.go | 61 +++++++++++++++++++ 8 files changed, 140 insertions(+), 24 deletions(-) create mode 100644 test/e2e/testdata/envoy-patch-policy.yaml create mode 100644 test/e2e/tests/envoy-patch-policy.go diff --git a/api/v1alpha1/envoypatchpolicy_types.go b/api/v1alpha1/envoypatchpolicy_types.go index 8234ead224e..012ac42a989 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 c336131fcca..7296a9e5127 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/docs/latest/user/envoy-patch-policy.md b/docs/latest/user/envoy-patch-policy.md index a06e5b416c1..fc0f63fb31c 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 ``` diff --git a/examples/redis/redis.yaml b/examples/redis/redis.yaml index c0b0992199f..74f6f6788de 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 diff --git a/internal/provider/kubernetes/controller.go b/internal/provider/kubernetes/controller.go index 25255cd0c45..1e26fd0bde3 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 diff --git a/internal/status/envoypatchpolicy.go b/internal/status/envoypatchpolicy.go index 950ce1f58ae..05deb89d154 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 new file mode 100644 index 00000000000..fa63ead7c80 --- /dev/null +++ b/test/e2e/testdata/envoy-patch-policy.yaml @@ -0,0 +1,48 @@ +--- +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: gateway-conformance-infra +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" + name: "gateway-conformance-infra/same-namespace/http" + operation: + op: add + path: "/default_filter_chain/filters/0/typed_config/local_reply_config" + value: + mappers: + - filter: + status_code_filter: + comparison: + op: EQ + value: + default_value: 404 + runtime_key: key_b + status_code: 406 + body: + inline_string: "not acceptable" diff --git a/test/e2e/tests/envoy-patch-policy.go b/test/e2e/tests/envoy-patch-policy.go new file mode 100644 index 00000000000..2486518ac31 --- /dev/null +++ b/test/e2e/tests/envoy-patch-policy.go @@ -0,0 +1,61 @@ +// 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/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) + 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 successful response + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, OkResp) + + customResp := http.ExpectedResponse{ + Request: http.Request{ + Path: "/bar", + }, + Response: http.Response{ + StatusCode: 406, + }, + Namespace: ns, + } + + // Send a request to an invalid path and expect a custom response + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, customResp) + }) + }, +}