API: EnvoyExtensionPolicy #2570

guydc · 2024-02-06T22:40:39Z

What this PR does / why we need it:

This PR introduces a new policy type, EnvoyExtensionPolicy, that can be used to add multiple HTTP and Network extension filters (such as lua, wasm, golang, ext-proc), and define their order of execution.

Relates to #2546, #2025, #2877

codecov · 2024-02-06T22:45:07Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.57%. Comparing base (23fa358) to head (b0dea04).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2570   +/-   ##
=======================================
  Coverage   64.57%   64.57%           
=======================================
  Files         122      122           
  Lines       21115    21115           
=======================================
  Hits        13636    13636           
  Misses       6632     6632           
  Partials      847      847

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

arkodg · 2024-02-06T23:13:20Z

api/v1alpha1/trafficextensionypolicy_types.go

+// +kubebuilder:printcolumn:name="Status",type=string,JSONPath=`.status.conditions[?(@.type=="Accepted")].reason`
+// +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`
+
+// TrafficExtensionPolicy allows the user to configure various traffic extensibility options for the Gateway.


prefer EnvoyExtensionPolicy since these extensions are specific to envoy or ExtensionPolicy if these extensions are common across proxy implementations

guydc · 2024-02-07T20:23:20Z

In yesterday's community meeting, @arkodg raised the following requirements:

Users may need to attach multiple policies in order to configure multiple extension filters (a common use case for WASM, LUA)
Users may need to control the logical phase of filter execution (e.g. before auth, before rate limit, ... )
Users may need to control the order of execution of extension filters amongst themselves (within a specific phase)

Istio's implementation for WASM plugins uses phase and priority to achieve this. Envoy Gateway can follow similar principles. The definition of such phases can have an impact on #2571, for example limiting the level of flexibility that users will have when re-arranging the Envoy Gateway built-in extensions.

@envoyproxy/gateway-maintainers - WDYT? Should we follow the istio pattern and implement phases and priorities for user-defined extensions?

liorokman · 2024-02-12T07:29:27Z

api/v1alpha1/envoyextensionypolicy_types.go

+type EnvoyExtensionPhase string
+
+const (
+	AuthzEnvoyExtensionPhase     EnvoyExtensionPhase = "AUTHZ"


In order to be consistent with other enums defined in the various EnvoyGateway CRDs, the enum values should not be all-caps.

AuthzEnvoyExtensionPhase EnvoyExtensionPhase = "AuthZ" AuthnEnvoyExtensionPhase EnvoyExtensionPhase = "AuthN" RateLimitEnvoyExtensionPhase EnvoyExtensionPhase = "RateLimit"

api/v1alpha1/envoyextensionypolicy_types.go

liorokman · 2024-02-12T08:58:12Z

api/v1alpha1/envoyextensionypolicy_types.go

+type NetworkEnvoyExtension struct {
+}
+
+type ExtensionMetadata map[string]string


Does this really need to be an aliased type?

Similar definitions in the APIs don't define an aliased type for map[string]string. See KubernetesPodSpec for example.

api/v1alpha1/ext_proc_types.go

liorokman · 2024-02-12T09:05:25Z

api/v1alpha1/ext_proc_types.go

+	// Default: no metadata context is sent or received
+	MetadataOptions *ExtProcMetadataOptions `json:"metadataOptions,omitempty"`
+	// The timeout for a response to be returned from the external processor
+	// Default: 200ms


Add a +kubebuilder:default comment for MessageTimeout.

liorokman · 2024-02-12T09:07:45Z

api/v1alpha1/envoyextensionypolicy_types.go

+)
+
+// +kubebuilder:object:root=true
+// +kubebuilder:resource:shortName=sp


This shortName is the same as the shortName defined for the SecurityPolicy resource.

Consider removing the short name, or changing it to ep.

api/v1alpha1/ext_proc_types.go

guydc · 2024-03-14T22:18:01Z

Incorporated points from the discussion in today's community meeting:

Filter Ordering in EnvoyProxy
Disablement of Extensibility by default, enablement in EnvoyGateway
Remove distinction between HTTP/Network in the interface
Allow route attachment

arkodg · 2024-03-14T22:42:04Z

@zhaohuabing lets pause on #2877 until this API is done, and the WASM API can be incorporated as a field within this API

arkodg · 2024-03-14T22:42:34Z

api/v1alpha1/envoyextensionypolicy_types.go

+	Spec EnvoyExtensionPolicySpec `json:"spec"`
+
+	// Status defines the current status of EnvoyExtensionPolicy.
+	Status EnvoyExtensionPolicyStatus `json:"status,omitempty"`


probably PolicyStatus here

Signed-off-by: Guy Daich <guy.daich@sap.com>

arkodg · 2024-03-14T22:45:04Z

api/v1alpha1/envoyproxy_types.go

+	// Fault-Injection, Local-Rate-Limit, Global-Rate-Limit
+	//
+	// +optional
+	FilterOrdering []EnvoyFilterName `json:"filterOrdering,omitempty"`


suggest keep filter ordering out of scope of this API, and solving it separately in #2571

Signed-off-by: Guy Daich <guy.daich@sap.com>

zhaohuabing

Should the spec support defining extensions as a list?

Unlike "normal" filters(Rate Limit, CORS, Basic Auth, OAuth2, JWT, etc), where typically only one filter per HTTPRoute is needed, users often require multiple custom Envoy extensions(wasm, lua, golang, ext auth?, etc.) of the same type for different purposes. For example, they might use a wasm filter for WAF and another for auth on the same HTTPRoute.

The current spec only supports defining a single extension per HTTPRoute, and it would block those use cases.

We may need to make it a list, like the below yaml snippet shows:

apiVersion: gateway.networking.k8s.io/v1alpha2
kind: EnvoyExtensionPolicy
metadata:
  name: my-extensions
spec:
  targetRef:
    kind: HTTPRoute
    name: backend
  wasm:
    - name: wasm_plugin_waf
     ...
    - name: my_plugin_auth
      ...
  ExtProc:
    - name: exproc_plugin_1 
      ....
    - name: exproc_plugin_2     
  Lua:
    - name: lua_plugin_1 
      ....
    - name: lua_plugin_2 
.....

guydc · 2024-03-15T12:53:55Z

Should the spec support defining extensions as a list?

+1

I think that's a reasonable ask, when considering that we allow something similar with envoy patches and it is indeed a very common use case. Maybe we can even avoid dealing with priority and multi-attachment in the first phases for this policy, if we support this + route attachment in the initial iteration.

arkodg · 2024-03-15T17:11:06Z

api/v1alpha1/envoyextensionypolicy_types.go

+	// ExtProc defines the configuration for the external processor extension.
+	//
+	// +optional
+	ExtProc *ExtProc `json:"extProc,omitempty"`


as per huabing's point, should we make this a list ?

Signed-off-by: Guy Daich <guy.daich@sap.com>

guydc · 2024-03-15T18:52:39Z

To make things a bit simpler, I removed ext-proc from this PR. After the top-level resource is added, @zhaohuabing and I can work on ext-proc/wasm in parallel.

arkodg · 2024-03-15T19:26:57Z

api/v1alpha1/envoyextensionypolicy_types.go

+// EnvoyExtensionPolicySpec defines the desired state of EnvoyExtensionPolicy.
+type EnvoyExtensionPolicySpec struct {
+	// +kubebuilder:validation:XValidation:rule="self.group == 'gateway.networking.k8s.io'", message="this policy can only have a targetRef.group of gateway.networking.k8s.io"
+	// +kubebuilder:validation:XValidation:rule="self.kind in ['Gateway']", message="this policy can only have a targetRef.kind of Gateway"


should target Gateway and Route

arkodg · 2024-03-15T19:28:31Z

api/v1alpha1/envoygateway_types.go

@@ -151,6 +151,10 @@ type ExtensionAPISettings struct {
 	// EnableEnvoyPatchPolicy enables Envoy Gateway to
 	// reconcile and implement the EnvoyPatchPolicy resources.
 	EnableEnvoyPatchPolicy bool `json:"enableEnvoyPatchPolicy"`
+
+	// EnableEnvoyExtensionPolicy enables Envoy Gateway to


@envoyproxy/gateway-maintainers @envoyproxy/gateway-reviewers thoughts on this ?

I'm fine to remove all these, it will not happen unless you create a CR.

api/v1alpha1/envoyproxy_types.go

site/content/en/latest/contributions/design/envoy-extension-policy.md

arkodg · 2024-03-15T19:35:28Z

site/content/en/latest/contributions/design/envoy-extension-policy.md

+
+## Overview
+
+This design document introduces the `EnvoyExtensionPolicy` API allowing system administrators to configure traffic


can we incorporate some of these below points in this doc

there are now 2 kinds of extensibility today

extending EG API by configuring xDS directly using EnvoyPatchPolicy & ExtensionManager

the other kind, highlighted in this doc that extends and adds newer functionality on top of what Envoy offers

Also a note around how users have been using EnvoyPatchPolicy & ExtensionManager to directly configure this extensions, but this API provides a safer and more scoped API

Signed-off-by: Guy Daich <guy.daich@sap.com>

guydc · 2024-03-15T21:07:07Z

/retest

arkodg

LGTM thanks !

guydc force-pushed the api-extension-policy branch from 43f07ce to 1389569 Compare February 6, 2024 22:52

arkodg reviewed Feb 6, 2024

View reviewed changes

guydc changed the title ~~[WIP] API: TrafficExtensionPolicy~~ API: TrafficExtensionPolicy Feb 9, 2024

guydc marked this pull request as ready for review February 9, 2024 23:19

guydc requested a review from a team as a code owner February 9, 2024 23:19

liorokman reviewed Feb 12, 2024

View reviewed changes

guydc changed the title ~~API: TrafficExtensionPolicy~~ API: EnvoyExtensionPolicy Feb 12, 2024

guydc marked this pull request as draft February 14, 2024 21:55

Xunzhuo mentioned this pull request Mar 11, 2024

api: wasm extension #2877

Merged

guydc force-pushed the api-extension-policy branch from ac5628c to f34cfbe Compare March 14, 2024 22:15

guydc force-pushed the api-extension-policy branch from f34cfbe to b657fc8 Compare March 14, 2024 22:23

arkodg reviewed Mar 14, 2024

View reviewed changes

api: EnvoyExtensionPolicy

04b7280

Signed-off-by: Guy Daich <guy.daich@sap.com>

guydc force-pushed the api-extension-policy branch from b657fc8 to 04b7280 Compare March 14, 2024 22:43

arkodg reviewed Mar 14, 2024

View reviewed changes

guydc marked this pull request as ready for review March 14, 2024 22:57

guydc added 2 commits March 14, 2024 18:11

review fixes

851def2

Signed-off-by: Guy Daich <guy.daich@sap.com>

fix lint

73f63ee

Signed-off-by: Guy Daich <guy.daich@sap.com>

zhaohuabing reviewed Mar 15, 2024

View reviewed changes

arkodg reviewed Mar 15, 2024

View reviewed changes

guydc added 3 commits March 15, 2024 13:35

remove ext-proc, ordering

81d6c69

Signed-off-by: Guy Daich <guy.daich@sap.com>

Merge branch 'main' into api-extension-policy

8a7c01f

fix gen

4a48185

Signed-off-by: Guy Daich <guy.daich@sap.com>

arkodg reviewed Mar 15, 2024

View reviewed changes

api/v1alpha1/envoyproxy_types.go Outdated Show resolved Hide resolved

arkodg reviewed Mar 15, 2024

View reviewed changes

site/content/en/latest/contributions/design/envoy-extension-policy.md Outdated Show resolved Hide resolved

arkodg reviewed Mar 15, 2024

View reviewed changes

guydc added 2 commits March 15, 2024 15:29

remove ordering, add CEL, fix api and design doc

5475dc2

Signed-off-by: Guy Daich <guy.daich@sap.com>

fix CEL names

b0dea04

Signed-off-by: Guy Daich <guy.daich@sap.com>

arkodg approved these changes Mar 15, 2024

View reviewed changes

arkodg requested review from a team March 15, 2024 22:01

zirain approved these changes Mar 16, 2024

View reviewed changes

zirain merged commit df051fa into envoyproxy:main Mar 16, 2024
22 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

API: EnvoyExtensionPolicy #2570

API: EnvoyExtensionPolicy #2570

guydc commented Feb 6, 2024 •

edited

Loading

codecov bot commented Feb 6, 2024 •

edited

Loading

arkodg Feb 6, 2024

guydc commented Feb 7, 2024

liorokman Feb 12, 2024

liorokman Feb 12, 2024

liorokman Feb 12, 2024

liorokman Feb 12, 2024

guydc commented Mar 14, 2024

arkodg commented Mar 14, 2024

arkodg Mar 14, 2024

arkodg Mar 14, 2024

zhaohuabing left a comment •

edited

Loading

guydc commented Mar 15, 2024

arkodg Mar 15, 2024

guydc commented Mar 15, 2024

arkodg Mar 15, 2024 •

edited

Loading

arkodg Mar 15, 2024

zirain Mar 16, 2024

arkodg Mar 15, 2024

guydc commented Mar 15, 2024

arkodg left a comment


		## Overview

		This design document introduces the `EnvoyExtensionPolicy` API allowing system administrators to configure traffic

API: EnvoyExtensionPolicy #2570

API: EnvoyExtensionPolicy #2570

Conversation

guydc commented Feb 6, 2024 • edited Loading

codecov bot commented Feb 6, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

guydc commented Feb 7, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guydc commented Mar 14, 2024

arkodg commented Mar 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhaohuabing left a comment • edited Loading

Choose a reason for hiding this comment

guydc commented Mar 15, 2024

Choose a reason for hiding this comment

guydc commented Mar 15, 2024

arkodg Mar 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guydc commented Mar 15, 2024

arkodg left a comment

Choose a reason for hiding this comment

guydc commented Feb 6, 2024 •

edited

Loading

codecov bot commented Feb 6, 2024 •

edited

Loading

zhaohuabing left a comment •

edited

Loading

arkodg Mar 15, 2024 •

edited

Loading