feat(translator): extproc timeout, failopen and backendRefs (#3206)

* impl extproc timeout, failopen, backendRefs Signed-off-by: Guy Daich <[email protected]> * fix xds translator test Signed-off-by: Guy Daich <[email protected]> * fix xds translator test Signed-off-by: Guy Daich <[email protected]> * fix gen Signed-off-by: Guy Daich <[email protected]> * impl procmode Signed-off-by: Guy Daich <[email protected]> * fix test Signed-off-by: Guy Daich <[email protected]> * fixes Signed-off-by: Guy Daich <[email protected]> * remove unused struct Signed-off-by: Guy Daich <[email protected]> --------- Signed-off-by: Guy Daich <[email protected]>
envoyproxy · Apr 30, 2024 · d38c09f · d38c09f
1 parent 9a33089
commit d38c09f
Show file tree

Hide file tree

Showing 39 changed files with 1,134 additions and 402 deletions.
diff --git a/api/v1alpha1/ext_proc_types.go b/api/v1alpha1/ext_proc_types.go
@@ -45,18 +45,15 @@ type ExtProcProcessingMode struct {
 	Response *ProcessingModeOptions `json:"response,omitempty"`
 }
 
-// +kubebuilder:validation:XValidation:rule="has(self.backendRef) ? (!has(self.backendRef.group) || self.backendRef.group == \"\") : true", message="group is invalid, only the core API group (specified by omitting the group field or setting it to an empty string) is supported"
-// +kubebuilder:validation:XValidation:rule="has(self.backendRef) ? (!has(self.backendRef.kind) || self.backendRef.kind == 'Service') : true", message="kind is invalid, only Service (specified by omitting the kind field or setting it to 'Service') is supported"
-//
 // ExtProc defines the configuration for External Processing filter.
 type ExtProc struct {
-	// BackendRef defines the configuration of the external processing service
-	BackendRef ExtProcBackendRef `json:"backendRef"`
-
 	// BackendRefs defines the configuration of the external processing service
 	//
-	// +optional
-	BackendRefs []BackendRef `json:"backendRefs,omitempty"`
+	// +kubebuilder:validation:MinItems=1
+	// +kubebuilder:validation:MaxItems=1
+	// +kubebuilder:validation:XValidation:message="BackendRefs only supports Service kind.",rule="self.all(f, f.kind == 'Service')"
+	// +kubebuilder:validation:XValidation:message="BackendRefs only supports Core group.",rule="self.all(f, f.group == '')"
+	BackendRefs []BackendRef `json:"backendRefs"`
 
 	// MessageTimeout is the timeout for a response to be returned from the external processor
 	// Default: 200ms
@@ -77,13 +74,3 @@ type ExtProc struct {
 	// +optional
 	ProcessingMode *ExtProcProcessingMode `json:"processingMode,omitempty"`
 }
-
-// ExtProcService defines the gRPC External Processing service using the envoy grpc client
-// The processing request and response messages are defined in
-// https://www.envoyproxy.io/docs/envoy/latest/api-v3/service/ext_proc/v3/external_processor.proto
-type ExtProcBackendRef struct {
-	// BackendObjectReference references a Kubernetes object that represents the
-	// backend server to which the processing requests will be sent.
-	// Only service Kind is supported for now.
-	gwapiv1.BackendObjectReference `json:",inline"`
-}
diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go
diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_envoyextensionpolicies.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_envoyextensionpolicies.yaml
@@ -54,83 +54,6 @@ spec:
                   description: ExtProc defines the configuration for External Processing
                     filter.
                   properties:
-                    backendRef:
-                      description: BackendRef defines the configuration of the external
-                        processing service
-                      properties:
-                        group:
-                          default: ""
-                          description: |-
-                            Group is the group of the referent. For example, "gateway.networking.k8s.io".
-                            When unspecified or empty string, core API group is inferred.
-                          maxLength: 253
-                          pattern: ^$|^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
-                          type: string
-                        kind:
-                          default: Service
-                          description: |-
-                            Kind is the Kubernetes resource kind of the referent. For example
-                            "Service".
-
-
-                            Defaults to "Service" when not specified.
-
-
-                            ExternalName services can refer to CNAME DNS records that may live
-                            outside of the cluster and as such are difficult to reason about in
-                            terms of conformance. They also may not be safe to forward to (see
-                            CVE-2021-25740 for more information). Implementations SHOULD NOT
-                            support ExternalName Services.
-
-
-                            Support: Core (Services with a type other than ExternalName)
-
-
-                            Support: Implementation-specific (Services with type ExternalName)
-                          maxLength: 63
-                          minLength: 1
-                          pattern: ^[a-zA-Z]([-a-zA-Z0-9]*[a-zA-Z0-9])?$
-                          type: string
-                        name:
-                          description: Name is the name of the referent.
-                          maxLength: 253
-                          minLength: 1
-                          type: string
-                        namespace:
-                          description: |-
-                            Namespace is the namespace of the backend. When unspecified, the local
-                            namespace is inferred.
-
-
-                            Note that when a namespace different than the local namespace is specified,
-                            a ReferenceGrant object is required in the referent namespace to allow that
-                            namespace's owner to accept the reference. See the ReferenceGrant
-                            documentation for details.
-
-
-                            Support: Core
-                          maxLength: 63
-                          minLength: 1
-                          pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$
-                          type: string
-                        port:
-                          description: |-
-                            Port specifies the destination port number to use for this resource.
-                            Port is required when the referent is a Kubernetes Service. In this
-                            case, the port number is the service port number, not the target port.
-                            For other resources, destination port might be derived from the referent
-                            resource or this field.
-                          format: int32
-                          maximum: 65535
-                          minimum: 1
-                          type: integer
-                      required:
-                      - name
-                      type: object
-                      x-kubernetes-validations:
-                      - message: Must have port for Service reference
-                        rule: '(size(self.group) == 0 && self.kind == ''Service'')
-                          ? has(self.port) : true'
                     backendRefs:
                       description: BackendRefs defines the configuration of the external
                         processing service
@@ -211,7 +134,14 @@ spec:
                         - message: Must have port for Service reference
                           rule: '(size(self.group) == 0 && self.kind == ''Service'')
                             ? has(self.port) : true'
+                      maxItems: 1
+                      minItems: 1
                       type: array
+                      x-kubernetes-validations:
+                      - message: BackendRefs only supports Service kind.
+                        rule: self.all(f, f.kind == 'Service')
+                      - message: BackendRefs only supports Core group.
+                        rule: self.all(f, f.group == '')
                     failOpen:
                       description: |-
                         FailOpen defines if requests or responses that cannot be processed due to connectivity to the
@@ -257,18 +187,8 @@ spec:
                           type: object
                       type: object
                   required:
-                  - backendRef
+                  - backendRefs
                   type: object
-                  x-kubernetes-validations:
-                  - message: group is invalid, only the core API group (specified
-                      by omitting the group field or setting it to an empty string)
-                      is supported
-                    rule: 'has(self.backendRef) ? (!has(self.backendRef.group) ||
-                      self.backendRef.group == "") : true'
-                  - message: kind is invalid, only Service (specified by omitting
-                      the kind field or setting it to 'Service') is supported
-                    rule: 'has(self.backendRef) ? (!has(self.backendRef.kind) || self.backendRef.kind
-                      == ''Service'') : true'
                 type: array
               targetRef:
                 description: |-

diff --git a/internal/gatewayapi/envoyextensionpolicy.go b/internal/gatewayapi/envoyextensionpolicy.go
@@ -10,13 +10,13 @@ import (
 	"fmt"
 	"sort"
 	"strings"
+	"time"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/utils/ptr"
 
-	gwapiv1 "sigs.k8s.io/gateway-api/apis/v1"
 	gwv1a2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
 	gwv1b1 "sigs.k8s.io/gateway-api/apis/v1beta1"
 
@@ -404,48 +404,79 @@ func (t *Translator) buildExtProc(
 	resources *Resources,
 ) (*ir.ExtProc, error) {
 	var (
-		backendRef *gwapiv1.BackendObjectReference
-		ds         *ir.DestinationSetting
-		authority  string
-		err        error
+		ds        *ir.DestinationSetting
+		authority string
+		err       error
 	)
 
-	backendRef = &extProc.BackendRef.BackendObjectReference
+	var dsl []*ir.DestinationSetting
+	for i := range extProc.BackendRefs {
+		if err = t.validateExtServiceBackendReference(
+			&extProc.BackendRefs[i].BackendObjectReference,
+			policyNamespacedName.Namespace,
+			resources); err != nil {
+			return nil, err
+		}
 
-	if err = t.validateExtServiceBackendReference(
-		backendRef,
-		policyNamespacedName.Namespace,
-		resources); err != nil {
-		return nil, err
-	}
+		ds, err = t.processExtServiceDestination(
+			&extProc.BackendRefs[i].BackendObjectReference,
+			policyNamespacedName,
+			egv1a1.KindEnvoyExtensionPolicy,
+			ir.GRPC,
+			resources)
 
-	authority = fmt.Sprintf(
-		"%s.%s:%d",
-		backendRef.Name,
-		NamespaceDerefOr(backendRef.Namespace, policyNamespacedName.Namespace),
-		*backendRef.Port)
+		if err != nil {
+			return nil, err
+		}
 
-	if ds, err = t.processExtServiceDestination(
-		backendRef,
-		policyNamespacedName,
-		egv1a1.KindEnvoyExtensionPolicy,
-		ir.GRPC,
-		resources); err != nil {
-		return nil, err
+		dsl = append(dsl, ds)
 	}
 
 	rd := ir.RouteDestination{
-		Name: irIndexedExtServiceDestinationName(policyNamespacedName, egv1a1.KindEnvoyExtensionPolicy,
-			string(backendRef.Name), extProcIdx),
-		Settings: []*ir.DestinationSetting{ds},
+		Name:     irIndexedExtServiceDestinationName(policyNamespacedName, egv1a1.KindEnvoyExtensionPolicy, extProcIdx),
+		Settings: dsl,
 	}
 
+	authority = fmt.Sprintf(
+		"%s.%s:%d",
+		extProc.BackendRefs[0].Name,
+		NamespaceDerefOr(extProc.BackendRefs[0].Namespace, policyNamespacedName.Namespace),
+		*extProc.BackendRefs[0].Port)
+
 	extProcIR := &ir.ExtProc{
 		Name:        name,
 		Destination: rd,
 		Authority:   authority,
 	}
 
+	if extProc.MessageTimeout != nil {
+		d, err := time.ParseDuration(string(*extProc.MessageTimeout))
+		if err != nil {
+			return nil, fmt.Errorf("invalid ExtProc MessageTimeout value %v", extProc.MessageTimeout)
+		}
+		extProcIR.MessageTimeout = ptr.To(metav1.Duration{Duration: d})
+	}
+
+	if extProc.FailOpen != nil {
+		extProcIR.FailOpen = extProc.FailOpen
+	}
+
+	if extProc.ProcessingMode != nil {
+		if extProc.ProcessingMode.Request != nil {
+			extProcIR.RequestHeaderProcessing = true
+			if extProc.ProcessingMode.Request.Body != nil {
+				extProcIR.RequestBodyProcessingMode = ptr.To(ir.ExtProcBodyProcessingMode(*extProc.ProcessingMode.Request.Body))
+			}
+		}
+
+		if extProc.ProcessingMode.Response != nil {
+			extProcIR.ResponseHeaderProcessing = true
+			if extProc.ProcessingMode.Response.Body != nil {
+				extProcIR.ResponseBodyProcessingMode = ptr.To(ir.ExtProcBodyProcessingMode(*extProc.ProcessingMode.Response.Body))
+			}
+		}
+	}
+
 	return extProcIR, err
 }
 

diff --git a/internal/gatewayapi/ext_service.go b/internal/gatewayapi/ext_service.go
@@ -94,12 +94,11 @@ func (t *Translator) processExtServiceDestination(
 }
 
 // TODO: also refer to extension type, as WASM may also introduce destinations
-func irIndexedExtServiceDestinationName(policyNamespacedName types.NamespacedName, policyKind, service string, idx int) string {
+func irIndexedExtServiceDestinationName(policyNamespacedName types.NamespacedName, policyKind string, idx int) string {
 	return strings.ToLower(fmt.Sprintf(
-		"%s/%s/%s/%d/%s",
+		"%s/%s/%s/%d",
 		policyKind,
 		policyNamespacedName.Namespace,
 		policyNamespacedName.Name,
-		idx,
-		service))
+		idx))
 }
diff --git a/internal/gatewayapi/testdata/envoyextensionpolicy-invalid-cross-ns-ref.in.yaml b/internal/gatewayapi/testdata/envoyextensionpolicy-invalid-cross-ns-ref.in.yaml
@@ -26,6 +26,6 @@ envoyExtensionPolicies:
       name: gateway-1
       namespace: envoy-gateway
     extProc:
-    - backendRef:
-        name: grpc-backend-4
+    - backendRefs:
+      - name: grpc-backend-4
         port: 4000
diff --git a/internal/gatewayapi/testdata/envoyextensionpolicy-invalid-cross-ns-ref.out.yaml b/internal/gatewayapi/testdata/envoyextensionpolicy-invalid-cross-ns-ref.out.yaml
@@ -7,8 +7,8 @@ envoyExtensionPolicies:
     namespace: default
   spec:
     extProc:
-    - backendRef:
-        name: grpc-backend-4
+    - backendRefs:
+      - name: grpc-backend-4
         port: 4000
     targetRef:
       group: gateway.networking.k8s.io

diff --git a/internal/gatewayapi/testdata/envoyextensionpolicy-override-replace.in.yaml b/internal/gatewayapi/testdata/envoyextensionpolicy-override-replace.in.yaml
@@ -86,8 +86,8 @@ envoyExtensionPolicies:
       name: gateway-1
       namespace: envoy-gateway
     extProc:
-    - backendRef:
-        name: grpc-backend
+    - backendRefs:
+      - name: grpc-backend
         port: 9000
 - apiVersion: gateway.envoyproxy.io/v1alpha1
   kind: EnvoyExtensionPolicy
@@ -101,6 +101,6 @@ envoyExtensionPolicies:
       name: httproute-1
       namespace: default
     extProc:
-    - backendRef:
-        name: grpc-backend-2
+    - backendRefs:
+      - name: grpc-backend-2
         port: 8000