[release/v0.5] Cherry-pick fixes needed for v0.5 (#1749)

* refactor: set defaults in Deployment, else k8s sets them for you, creating infinite reconciliation loop (#1594) * fix: envoy proxy resource apply bug. Signed-off-by: qicz <qiczzhu@gmail.com> * update pointer. Signed-off-by: qicz <qiczzhu@gmail.com> * add comment Signed-off-by: qicz <qiczzhu@gmail.com> * update cm cmp logic. Signed-off-by: qicz <qiczzhu@gmail.com> * fix lint Signed-off-by: qicz <qiczzhu@gmail.com> * add probe field default value. Signed-off-by: qicz <qiczzhu@gmail.com> * fix uts Signed-off-by: qicz <qiczzhu@gmail.com> * align probe Signed-off-by: qicz <qiczzhu@gmail.com> * optimize deploy compare logic Signed-off-by: qicz <qiczzhu@gmail.com> * add compare deploy uts Signed-off-by: qicz <qiczzhu@gmail.com> * rm cm binarydata cmp Signed-off-by: qicz <qiczzhu@gmail.com> * rm deploy cmp logic Signed-off-by: qicz <qiczzhu@gmail.com> * fix ut Signed-off-by: qicz <qiczzhu@gmail.com> * fix lint Signed-off-by: qicz <qiczzhu@gmail.com> --------- Signed-off-by: qicz <qiczzhu@gmail.com> Signed-off-by: qi <qiczzhu@gmail.com> (cherry picked from commit 9ba9103) * DeepCopy resources that require status updates (#1723) * Was seeing constant churn between provider runner publishing resources and gateway-api runner receiving them. * Tried to debug it by printing the o/p of `cmp.Diff` between current and previous values ``` diff --git a/internal/gatewayapi/runner/runner.go b/internal/gatewayapi/runner/runner.go index 050394ba..50d09f6f 100644 --- a/internal/gatewayapi/runner/runner.go +++ b/internal/gatewayapi/runner/runner.go @@ -8,6 +8,7 @@ package runner import ( "context" + "github.com/google/go-cmp/cmp" "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/gateway-api/apis/v1beta1" "sigs.k8s.io/yaml" @@ -49,6 +50,7 @@ func (r *Runner) Start(ctx context.Context) error { } func (r *Runner) subscribeAndTranslate(ctx context.Context) { + prev := &gatewayapi.Resources{} message.HandleSubscription(r.ProviderResources.GatewayAPIResources.Subscribe(ctx), func(update message.Update[string, *gatewayapi.Resources]) { val := update.Value @@ -56,6 +58,9 @@ func (r *Runner) subscribeAndTranslate(ctx context.Context) { if update.Delete || val == nil { return } + diff := cmp.Diff(prev, val) + r.Logger.WithValues("output", "diff").Info(diff) + prev = val.DeepCopy() // Translate and publish IRs. t := &gatewayapi.Translator{ ``` Here's the o/p and its empty ``` 2023-07-27T23:55:29.795Z INFO gateway-api runner/runner.go:62 {"runner": "gateway-api", "output": "diff"} ``` * Using a DeepCopy for resources that were updating the `Status` subresource seems to have solved the issue, which implies that watchable doesnt like clients to mutate the value, even though they are meant to be a `DeepCopy` Fixes: #1715 Signed-off-by: Arko Dasgupta <arko@tetrate.io> (cherry picked from commit 5b72451) * observability: add container port for metrics (#1736) container port Signed-off-by: zirain <zirain2009@gmail.com> (cherry picked from commit 4bba03a) * docs: Add user docs for EnvoyPatchPolicy (#1733) * Add user docs for EnvoyPatchPolicy Relates to #24 Signed-off-by: Arko Dasgupta <arko@tetrate.io> * nits Signed-off-by: Arko Dasgupta <arko@tetrate.io> * wrap up Signed-off-by: Arko Dasgupta <arko@tetrate.io> * lint Signed-off-by: Arko Dasgupta <arko@tetrate.io> * address comments && fix config Signed-off-by: Arko Dasgupta <arko@tetrate.io> --------- Signed-off-by: Arko Dasgupta <arko@tetrate.io> (cherry picked from commit 27b0939) * 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> (cherry picked from commit a7784c5) --------- Signed-off-by: Arko Dasgupta <arko@tetrate.io> Co-authored-by: qi <qiczzhu@gmail.com> Co-authored-by: zirain <zirain2009@gmail.com>
envoyproxy · Aug 2, 2023 · a498365 · a498365
1 parent c303835
commit a498365
Show file tree

Hide file tree

Showing 40 changed files with 499 additions and 29 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
@@ -0,0 +1,200 @@
+# Envoy Patch Policy
+
+This guide explains the usage of the [EnvoyPatchPolicy][] API.
+__Note:__ This API is meant for users extremely familiar with Envoy [xDS][] semantics.
+Also before considering this API for production use cases, please be aware that this API
+is unstable and the outcome may change across versions. Use at your own risk.
+
+## Introduction
+
+The [EnvoyPatchPolicy][] API allows user to modify the output [xDS][]
+configuration generated by Envoy Gateway intended for EnvoyProxy, 
+using [JSON Patch][] semantics.
+
+## Motivation
+
+This API was introduced to allow advanced users to be able to leverage Envoy Proxy functionality
+not exposed by Envoy Gateway APIs today.
+
+## Quickstart
+
+### Prerequistes
+
+* Follow the steps from the [Quickstart](quickstart.md) guide to install Envoy Gateway and the example manifest.
+Before proceeding, you should be able to query the example backend using HTTP.
+
+### Enable EnvoyPatchPolicy
+
+* By default EnvoyPatchPolicy][] is disabled. Lets enable it in the [EnvoyGateway][] startup configuration
+
+* The default installation of Envoy Gateway installs a default [EnvoyGateway][] configuration and attaches it
+using a `ConfigMap`. In the next step, we will update this resource to enable EnvoyPatchPolicy. 
+
+
+```shell
+cat <<EOF | kubectl apply -f -
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: envoy-gateway-config
+  namespace: envoy-gateway-system
+data:
+  envoy-gateway.yaml: |
+    apiVersion: config.gateway.envoyproxy.io/v1alpha1
+    kind: EnvoyGateway
+    provider:
+      type: Kubernetes
+    gateway:
+      controllerName: gateway.envoyproxy.io/gatewayclass-controller
+    extensionApis:
+      enableEnvoyPatchPolicy: true
+EOF
+```
+
+* After updating the `ConfigMap`, you will need to restart the `envoy-gateway` deployment so the configuration kicks in
+
+```shell
+kubectl rollout restart deployment envoy-gateway -n envoy-gateway-system
+```
+
+## Testing
+
+### Customize Response
+
+* Lets use EnvoyProxy's [Local Reply Modification][] feature to return a custom response back to the client when
+the status code is `404`
+
+* Lets apply the configuration
+
+```shell
+cat <<EOF | kubectl apply -f -
+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: eg
+    namespace: default
+  type: JSONPatch
+  jsonPatches:
+    - type: "type.googleapis.com/envoy.config.listener.v3.Listener"
+      # The listener name is of the form <GatewayNamespace>/<GatewayName>/<GatewayListenerName>
+      name: default/eg/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: "could not find what you are looking for"
+EOF
+```
+
+* Lets edit the HTTPRoute resource from the Quickstart to only match on paths with value `/get`
+
+```
+kubectl patch httproute backend --type=json --patch '[{
+   "op": "add",
+   "path": "/spec/rules/0/matches/0/path/value",
+   "value": "/get",
+}]'
+```
+
+* Lets test it out by specifying a path apart from `/get`
+
+```
+$ curl --header "Host: www.example.com" http://localhost:8888/find
+Handling connection for 8888
+could not find what you are looking for
+```
+
+## Debugging
+
+### Runtime
+
+* The `Status` subresource should have information about the status of the resource. Make sure
+`Accepted=True` and `Programmed=True` conditions are set to ensure that the policy has been
+applied to Envoy Proxy.
+
+```
+apiVersion: gateway.envoyproxy.io/v1alpha1
+kind: EnvoyPatchPolicy
+metadata:
+  annotations:
+    kubectl.kubernetes.io/last-applied-configuration: |
+      {"apiVersion":"gateway.envoyproxy.io/v1alpha1","kind":"EnvoyPatchPolicy","metadata":{"annotations":{},"name":"custom-response-patch-policy","namespace":"default"},"spec":{"jsonPatches":[{"name":"default/eg/http","operation":{"op":"add","path":"/default_filter_chain/filters/0/typed_config/local_reply_config","value":{"mappers":[{"body":{"inline_string":"could not find what you are looking for"},"filter":{"status_code_filter":{"comparison":{"op":"EQ","value":{"default_value":404}}}}}]}},"type":"type.googleapis.com/envoy.config.listener.v3.Listener"}],"priority":0,"targetRef":{"group":"gateway.networking.k8s.io","kind":"Gateway","name":"eg","namespace":"default"},"type":"JSONPatch"}}
+  creationTimestamp: "2023-07-31T21:47:53Z"
+  generation: 1
+  name: custom-response-patch-policy
+  namespace: default
+  resourceVersion: "10265"
+  uid: a35bda6e-a0cc-46d7-a63a-cee765174bc3
+spec:
+  jsonPatches:
+  - name: default/eg/http
+    operation:
+      op: add
+      path: /default_filter_chain/filters/0/typed_config/local_reply_config
+      value:
+        mappers:
+        - body:
+            inline_string: could not find what you are looking for
+          filter:
+            status_code_filter:
+              comparison:
+                op: EQ
+                value:
+                  default_value: 404
+    type: type.googleapis.com/envoy.config.listener.v3.Listener
+  priority: 0
+  targetRef:
+    group: gateway.networking.k8s.io
+    kind: Gateway
+    name: eg
+    namespace: default
+  type: JSONPatch
+status:
+  conditions:
+  - lastTransitionTime: "2023-07-31T21:48:19Z"
+    message: EnvoyPatchPolicy has been accepted.
+    observedGeneration: 1
+    reason: Accepted
+    status: "True"
+    type: Accepted
+  - lastTransitionTime: "2023-07-31T21:48:19Z"
+    message: successfully applied patches.
+    reason: Programmed
+    status: "True"
+    type: Programmed
+```
+
+### Offline
+
+* You can use [egctl x translate][] to validate the translated xds output.
+
+## Caveats
+
+This API will always be an unstable API and the same outcome cannot be garunteed
+across versions for these reasons
+* The Envoy Proxy API might deprecate and remove API fields
+* Envoy Gateway might alter the xDS translation creating a different xDS output
+such as changing the `name` field of resources.
+
+[EnvoyPatchPolicy]: https://gateway.envoyproxy.io/latest/api/extension_types.html#envoypatchpolicy
+[EnvoyGateway]: https://gateway.envoyproxy.io/latest/api/config_types.html#envoygateway
+[JSON Patch]: https://datatracker.ietf.org/doc/html/rfc6902
+[xDS]: https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/operations/dynamic_configuration
+[Local Reply Modification]: https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_conn_man/local_reply
+[egctl x translate]: https://gateway.envoyproxy.io/latest/user/egctl.html#egctl-experimental-translate
diff --git a/docs/latest/user_docs.rst b/docs/latest/user_docs.rst
@@ -23,6 +23,7 @@ Learn how to deploy, use, and operate Envoy Gateway.
   user/grpc-routing
   user/authn
   user/rate-limit
+  user/envoy-patch-policy
   user/egctl
   user/customize-envoyproxy
   user/deployment-mode

diff --git a/examples/kubernetes/metric/pod-monitor.yaml b/examples/kubernetes/metric/pod-monitor.yaml
@@ -0,0 +1,17 @@
+apiVersion: monitoring.coreos.com/v1
+kind: PodMonitor
+metadata:
+  name: envoy-gateway-proxy-monitoring
+  namespace: envoy-gateway-system
+spec:
+  selector:
+    matchLabels:
+      app.kubernetes.io/name: envoy
+      app.kubernetes.io/component: proxy
+  namespaceSelector:
+    any: true
+  jobLabel: proxy-stats
+  podMetricsEndpoints:
+    - path: /stats/prometheus
+      interval: 15s
+      port: metrics
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/gatewayapi/envoypatchpolicy.go b/internal/gatewayapi/envoypatchpolicy.go
@@ -25,7 +25,7 @@ func (t *Translator) ProcessEnvoyPatchPolicies(envoyPatchPolicies []*egv1a1.Envo
 	})
 
 	for _, policy := range envoyPatchPolicies {
-
+		policy := policy.DeepCopy()
 		targetNs := policy.Spec.TargetRef.Namespace
 		if targetNs == nil {
 			// This status condition will not get updated in the resource because

diff --git a/internal/gatewayapi/route.go b/internal/gatewayapi/route.go
@@ -39,7 +39,7 @@ func (t *Translator) ProcessHTTPRoutes(httpRoutes []*v1beta1.HTTPRoute, gateways
 		}
 		httpRoute := &HTTPRouteContext{
 			GatewayControllerName: t.GatewayControllerName,
-			HTTPRoute:             h,
+			HTTPRoute:             h.DeepCopy(),
 		}
 
 		// Find out if this route attaches to one of our Gateway's listeners,
@@ -67,7 +67,7 @@ func (t *Translator) ProcessGRPCRoutes(grpcRoutes []*v1alpha2.GRPCRoute, gateway
 		}
 		grpcRoute := &GRPCRouteContext{
 			GatewayControllerName: t.GatewayControllerName,
-			GRPCRoute:             g,
+			GRPCRoute:             g.DeepCopy(),
 		}
 
 		// Find out if this route attaches to one of our Gateway's listeners,
@@ -563,7 +563,7 @@ func (t *Translator) ProcessTLSRoutes(tlsRoutes []*v1alpha2.TLSRoute, gateways [
 		}
 		tlsRoute := &TLSRouteContext{
 			GatewayControllerName: t.GatewayControllerName,
-			TLSRoute:              tls,
+			TLSRoute:              tls.DeepCopy(),
 		}
 
 		// Find out if this route attaches to one of our Gateway's listeners,
@@ -685,7 +685,7 @@ func (t *Translator) ProcessUDPRoutes(udpRoutes []*v1alpha2.UDPRoute, gateways [
 		}
 		udpRoute := &UDPRouteContext{
 			GatewayControllerName: t.GatewayControllerName,
-			UDPRoute:              u,
+			UDPRoute:              u.DeepCopy(),
 		}
 
 		// Find out if this route attaches to one of our Gateway's listeners,
@@ -817,7 +817,7 @@ func (t *Translator) ProcessTCPRoutes(tcpRoutes []*v1alpha2.TCPRoute, gateways [
 		}
 		tcpRoute := &TCPRouteContext{
 			GatewayControllerName: t.GatewayControllerName,
-			TCPRoute:              tcp,
+			TCPRoute:              tcp.DeepCopy(),
 		}
 
 		// Find out if this route attaches to one of our Gateway's listeners,

diff --git a/internal/gatewayapi/runner/runner.go b/internal/gatewayapi/runner/runner.go
@@ -51,6 +51,8 @@ func (r *Runner) Start(ctx context.Context) error {
 func (r *Runner) subscribeAndTranslate(ctx context.Context) {
 	message.HandleSubscription(r.ProviderResources.GatewayAPIResources.Subscribe(ctx),
 		func(update message.Update[string, *gatewayapi.Resources]) {
+			r.Logger.Info("received an update")
+
 			val := update.Value
 
 			if update.Delete || val == nil {

diff --git a/internal/infrastructure/kubernetes/proxy/resource.go b/internal/infrastructure/kubernetes/proxy/resource.go
@@ -117,6 +117,14 @@ func expectedProxyContainers(infra *ir.ProxyInfra, deploymentConfig *egcfgv1a1.K
 		proxyMetrics = infra.Config.Spec.Telemetry.Metrics
 	}
 
+	if proxyMetrics != nil && proxyMetrics.Prometheus != nil {
+		ports = append(ports, corev1.ContainerPort{
+			Name:          "metrics",
+			ContainerPort: bootstrap.EnvoyReadinessPort, // TODO: make this configurable
+			Protocol:      corev1.ProtocolTCP,
+		})
+	}
+
 	var bootstrapConfigurations string
 	// Get Bootstrap from EnvoyProxy API if set by the user
 	// The config should have been validated already
@@ -163,10 +171,15 @@ func expectedProxyContainers(infra *ir.ProxyInfra, deploymentConfig *egcfgv1a1.K
 			ReadinessProbe: &corev1.Probe{
 				ProbeHandler: corev1.ProbeHandler{
 					HTTPGet: &corev1.HTTPGetAction{
-						Path: bootstrap.EnvoyReadinessPath,
-						Port: intstr.IntOrString{Type: intstr.Int, IntVal: bootstrap.EnvoyReadinessPort},
+						Path:   bootstrap.EnvoyReadinessPath,
+						Port:   intstr.IntOrString{Type: intstr.Int, IntVal: bootstrap.EnvoyReadinessPort},
+						Scheme: corev1.URISchemeHTTP,
 					},
 				},
+				TimeoutSeconds:   1,
+				PeriodSeconds:    10,
+				SuccessThreshold: 1,
+				FailureThreshold: 3,
 			},
 		},
 	}
@@ -222,7 +235,8 @@ func expectedDeploymentVolumes(name string, deploymentSpec *egcfgv1a1.Kubernetes
 			Name: "certs",
 			VolumeSource: corev1.VolumeSource{
 				Secret: &corev1.SecretVolumeSource{
-					SecretName: "envoy",
+					SecretName:  "envoy",
+					DefaultMode: pointer.Int32(420),
 				},
 			},
 		},
@@ -243,7 +257,7 @@ func expectedDeploymentVolumes(name string, deploymentSpec *egcfgv1a1.Kubernetes
 							Path: SdsCertFilename,
 						},
 					},
-					DefaultMode: pointer.Int32(int32(420)),
+					DefaultMode: pointer.Int32(420),
 					Optional:    pointer.Bool(false),
 				},
 			},

diff --git a/internal/infrastructure/kubernetes/proxy/resource_provider.go b/internal/infrastructure/kubernetes/proxy/resource_provider.go
@@ -224,6 +224,8 @@ func (r *ResourceRender) Deployment() (*appsv1.Deployment, error) {
 					Volumes:                       expectedDeploymentVolumes(r.infra.Name, deploymentConfig),
 				},
 			},
+			RevisionHistoryLimit:    pointer.Int32(10),
+			ProgressDeadlineSeconds: pointer.Int32(600),
 		},
 	}
 

diff --git a/internal/infrastructure/kubernetes/proxy/resource_provider_test.go b/internal/infrastructure/kubernetes/proxy/resource_provider_test.go
@@ -208,7 +208,8 @@ func TestDeployment(t *testing.T) {
 							Name: "certs",
 							VolumeSource: corev1.VolumeSource{
 								Secret: &corev1.SecretVolumeSource{
-									SecretName: "custom-envoy-cert",
+									SecretName:  "custom-envoy-cert",
+									DefaultMode: pointer.Int32(420),
 								},
 							},
 						},