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 <arko@tetrate.io>
envoyproxy · Aug 2, 2023 · a7784c5 · a7784c5
1 parent b4d4e2f
commit a7784c5
Show file tree

Hide file tree

Showing 8 changed files with 140 additions and 24 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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)
+		})
+	},
+}