API: HTTP Filter ordering #2993

zhaohuabing · 2024-03-21T06:09:27Z

API for HCM HTTP filter ordering. This is a global-level config for all the HTTP listeners in Envoy proxy. If the Listener/HTTPRoute level filter ordering is required in the future, we can define a fine-grained API targeting Listener/HTTPRoute.

This is a blocker for #2877

Related issue: #2571

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

codecov · 2024-03-21T06:13:34Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.55%. Comparing base (29946b0) to head (0d76bfc).
Report is 53 commits behind head on main.

❗ Current head 0d76bfc differs from pull request most recent head 7673385. Consider uploading reports for the commit 7673385 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2993      +/-   ##
==========================================
- Coverage   66.51%   64.55%   -1.96%     
==========================================
  Files         161      121      -40     
  Lines       22673    21235    -1438     
==========================================
- Hits        15080    13709    -1371     
+ Misses       6720     6672      -48     
+ Partials      873      854      -19

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

api/v1alpha1/envoyproxy_types.go

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

guydc · 2024-03-21T15:17:24Z

api/v1alpha1/envoyproxy_types.go

+// FilterOrder defines the order of filters in the Envoy proxy's HTTP filter chain.
+// If unspecified, the default order of filters is applied.
+// Default order of filters:
+// - envoy.filters.http.cors


maybe we can expose a more high-level filter name?

I used the Envoy filter name so these const variables can be directly used in the code for comparison.
But they could be simplified, maybe just the last part, eg, cors, ext_authz, local_ratelimit, oauth2, etc.

WDYT?

api/v1alpha1/envoyproxy_types.go

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

api/v1alpha1/envoyproxy_types.go

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

zhaohuabing · 2024-03-26T02:10:03Z

defer to @arkodg

api/v1alpha1/envoyproxy_types.go

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

api/v1alpha1/envoyproxy_types.go

arkodg · 2024-03-26T05:05:10Z

I really like how simple this API is, great stuff @zhaohuabing !
my only q is naming - FIlterOrder or FilterPriority :)

arkodg · 2024-03-26T10:05:20Z

api/v1alpha1/envoyproxy_types.go

+	FilterPriority []FilterPriority `json:"filterPriority,omitempty"`
+}
+
+// FilterPriority defines the order of filters in the Envoy proxy's HTTP filter chain.


the API looks good, but lets take a little more time to make sure if we

agree on this default order

should we leave a gap for any upcoming feature like rbac or compression filter or is the 100 gap enough to handle these future cases ?

we cannot change these default priority values going forward

should we leave a gap for any upcoming feature like rbac or compression filter or is the 100 gap enough to handle these future cases ?

We could list all the filters that EG is likely to use.

default order:

The default order of EG: cors ext_authz basic_authn oauth2 jwt_authn fault local_ratelimit ratelimit
The order of envoy filters in Isito: ext_auth jwt_authn rbac fault cors

I think it makes sense to put cors at the last since users probably need to apply fault injection and rate limit to preflight requests as well.

zhaohuabing · 2024-03-27T01:53:24Z

After discussing with @arkodg and @guydc during this week's EG meeting, let's hold off on this for now to gather more feedback.

@envoyproxy/gateway-maintainers your input on the default orders of the HTTP filters and comments on this API would be greatly appreciated. Thanks!

zirain · 2024-03-27T02:03:55Z

After discussing with @arkodg and @guydc during this week's EG meeting, let's hold off on this for now to gather more feedback.

@envoyproxy/gateway-maintainers your input on the default orders of the HTTP filters and comments on this API would be greatly appreciated. Thanks!

I do think this's a blocker of #2877, a wasm filter can be inserted anywhere in the filter chain.

zhaohuabing · 2024-03-27T03:00:32Z

After discussing with @arkodg and @guydc during this week's EG meeting, let's hold off on this for now to gather more feedback.
@envoyproxy/gateway-maintainers your input on the default orders of the HTTP filters and comments on this API would be greatly appreciated. Thanks!

I do think this's a blocker of #2877, a wasm filter can be inserted anywhere in the filter chain.

Yah, that's correct. But we need more input to make sure we do the API right. In the first iteration of wasm/extproc impl, the priority will be removed to make this none-blocking. We will revisit it right after the wasm extension implementation.

ZackButcher · 2024-03-28T16:35:49Z

api/v1alpha1/envoyproxy_types.go

+	// FilterPriority defines the order of filters in the Envoy proxy's HTTP filter chain.
+	//
+	// +optional
+	FilterPriority []FilterPriority `json:"filterPriority,omitempty"`


We need to clarify the API: as I read it, it's all-or-nothing (you either set a filter order, in which case you need to set every filter you want executed, or you get the default) -- is that correct?

I think that's a reasonable API, but it makes simple things ("add a WASM filter before authn, please") as hard as hard things ("totally re-order how policy is applied, including my WASM, ext_proc, ext_authz, and rate limiting"). It's also very copy+paste heavy: you look up the official order and copy+paste it to start editing, and next time you copy+paste your edited version for the new one, and so on. This winds up making it more expensive to iterate on APIs as a project.

Instead it might be easier to have a split API, along the lines of:

Suggested change

FilterPriority []FilterPriority `json:"filterPriority,omitempty"`

type FilterOrder struct {

Add []FilterPriority `json:"add,omitempty"`

Custom []FilterPriority `json:"custom,omitempty"`

}

FilterOrder FilterOrder `json:"filterOrder,omitempty"`

Then for small changes a user can add, or for total control they can use custom:

filterOrder: add: - filter: envoy.filters.http.wasm priority: 10

filterOrder: custom: - filter: envoy.filters.http.cors priority: 901 - filter: envoy.filters.http.ext_authz priority: 801 - filter: envoy.filters.http.basic_authn priority: 701 - filter: envoy.filters.http.oauth2 priority: 601 - filter: envoy.filters.http.jwt_authn priority: 501 - filter: envoy.filters.http.fault priority: 401 - filter: envoy.filters.http.local_ratelimit priority: 301 - filter: envoy.filters.http.ratelimit priority: 201

And of course we'd expect the two to work together as well:

# these are the same filterOrder: add: - filter: envoy.filters.http.wasm priority: 10 custom: - filter: envoy.filters.http.cors priority: 901 - filter: envoy.filters.http.ext_authz priority: 801 - filter: envoy.filters.http.basic_authn priority: 701 - filter: envoy.filters.http.oauth2 priority: 601 - filter: envoy.filters.http.jwt_authn priority: 501 - filter: envoy.filters.http.fault priority: 401 - filter: envoy.filters.http.local_ratelimit priority: 301 - filter: envoy.filters.http.ratelimit priority: 201 --- filterOrder: custom: - filter: envoy.filters.http.cors priority: 901 - filter: envoy.filters.http.ext_authz priority: 801 - filter: envoy.filters.http.basic_authn priority: 701 - filter: envoy.filters.http.oauth2 priority: 601 - filter: envoy.filters.http.jwt_authn priority: 501 - filter: envoy.filters.http.fault priority: 401 - filter: envoy.filters.http.local_ratelimit priority: 301 - filter: envoy.filters.http.ratelimit priority: 201 - filter: envoy.filters.http.wasm priority: 10

@ZackButcher It doesn't have to be a full list.

Each filter comes with a default priority, as the comment shows:
// - envoy.filters.http.cors 0
//
// - envoy.filters.http.ext_authz 100
//
// - envoy.filters.http.basic_authn 200
//
// - envoy.filters.http.oauth2 300
//
// - envoy.filters.http.jwt_authn 400
//
// - envoy.filters.http.fault 500
//
// - envoy.filters.http.local_ratelimit 600
//
// - envoy.filters.http.ratelimit 700

If we need to adjust the oauth2 filter's priority to sit between cors and ext_auth, we can do so with this configuration:

filterPriority: envoy.filters.http.oauth2: 50

For custom extensions like wasm, lua, and extproc, etc., we're planning to introduce a priority field in their specs since there can be more than one filter per type for an HTTPRoute. For example:

apiVersion: gateway.networking.k8s.io/v1alpha2 kind: EnvoyExtensionPolicy metadata: name: app-extensions spec: targetRef: kind: HTTPRoute name: backend wasm: - name: my_plugin_1 priority: 450 code: image: # the oci image that contains wasm code url: <oci-registry>.foo.bar.com/my_plugin_1:v1 pullSecret: wasmPullSecret ... - name: my_plugin_2 priority: 250 code: image: # the oci image that contains wasm code url: <oci-registry>.foo.bar.com/my_plugin_2:v1 pullSecret: wasmPullSecret ...

This means my_plugin_1 will be positioned between jwt_authn and fault, and my_plugin_2 will come between basic_authn and jwt_authn.

So, the final order of these filters will be:
cors - 0
oauth2 - 50
basic_authn - 200
wasm_my_plugin_2 - 250
jwt_authn - 400
wasm_my_plugin_1 - 450
fault - 500
local_ratelimit - 600
ratelimit - 700

arkodg · 2024-03-29T11:04:49Z

blocked on #3046

arkodg · 2024-04-12T06:17:14Z

hey @zhaohuabing , we brought this up in the community meeting yesterday and @davidalger had a great alternate suggestion - to use before and after keywords e.g.

filterOrder:
- name: envoy.filters.http.local_ratelimit
   before: envoy.filters.http.ext_authz
- name: envoy.filters.http.cors
   after: envoy.filters.http.local_ratelimit

this does improve readability of intent. Flattening this intent out, may result in a loop, which needs to be avoided and flagged in the status (but thats an internal detail)

this API can look similar inside the WASM API

spec:
.......
  filterOrder:
    before: envoy.filters.http.ext_authz

this will require us to rely on priority field in EEP to create order across EEPs

zhaohuabing · 2024-04-12T07:57:10Z

hey @zhaohuabing , we brought this up in the community meeting yesterday and @davidalger had a great alternate suggestion - to use before and after keywords e.g.
filterOrder:
- name: envoy.filters.http.local_ratelimit
   before: envoy.filters.http.ext_authz
- name: envoy.filters.http.cors
   after: envoy.filters.http.local_ratelimit
this does improve readability of intent. Flattening this intent out, may result in a loop, which needs to be avoided and flagged in the status (but thats an internal detail)

this API can look similar inside the WASM API
spec:
.......
  filterOrder:
    before: envoy.filters.http.ext_authz
this will require us to rely on priority field in EEP to create order across EEPs

LGreatTM!

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

api/v1alpha1/envoyproxy_types.go

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

arkodg

LGTM thanks !

guydc

LGTM

zhaohuabing added 2 commits March 21, 2024 13:23

add filter ordering API

6d7b9dc

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

envoy HTTP filter ordering API

e4e11cc

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

zhaohuabing requested a review from a team as a code owner March 21, 2024 06:09

zhaohuabing requested review from arkodg and guydc March 21, 2024 06:10

zhaohuabing mentioned this pull request Mar 21, 2024

api: wasm extension #2877

Merged

zhaohuabing commented Mar 21, 2024

View reviewed changes

api/v1alpha1/envoyproxy_types.go Show resolved Hide resolved

zhaohuabing force-pushed the filter-order branch 2 times, most recently from f304b14 to 75d7042 Compare March 21, 2024 07:40

add validation

fd26a09

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

zhaohuabing force-pushed the filter-order branch from 75d7042 to fd26a09 Compare March 21, 2024 07:42

guydc reviewed Mar 21, 2024

View reviewed changes

remove router

93e88d0

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

guydc reviewed Mar 23, 2024

View reviewed changes

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

zhaohuabing added 2 commits March 25, 2024 09:08

remove maxItem

59b4cc4

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

address comments

913a159

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

zhaohuabing requested a review from guydc March 25, 2024 01:30

zhaohuabing added 4 commits March 25, 2024 09:33

Merge branch 'main' into filter-order

ccc5180

fix json tag

dcc1063

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

fix gen

5ff23ef

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

Merge branch 'main' into filter-order

50caf0a

guydc previously approved these changes Mar 25, 2024

View reviewed changes

Merge branch 'main' into filter-order

8afa438

arkodg reviewed Mar 26, 2024

View reviewed changes

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

fix gen

0956189

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

zhaohuabing dismissed guydc’s stale review via 0956189 March 26, 2024 05:01

arkodg reviewed Mar 26, 2024

View reviewed changes

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

arkodg reviewed Mar 26, 2024

View reviewed changes

Merge branch 'main' into filter-order

0d76bfc

zhaohuabing added the hold do not merge label Mar 27, 2024

ZackButcher reviewed Mar 28, 2024

View reviewed changes

zhaohuabing requested a review from ZackButcher March 29, 2024 09:45

zhaohuabing added 3 commits April 12, 2024 16:17

address comments

b2ba887

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

Merge branch 'main' into filter-order

9a12f9f

kube gen

ade4707

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

zhaohuabing requested a review from arkodg April 16, 2024 05:52

zhaohuabing removed the hold do not merge label Apr 16, 2024

Merge remote-tracking branch 'upstream/main' into filter-order

e8291a0

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

arkodg reviewed Apr 16, 2024

View reviewed changes

api/v1alpha1/envoyproxy_types.go Show resolved Hide resolved

arkodg reviewed Apr 16, 2024

View reviewed changes

api/v1alpha1/envoyproxy_types.go Show resolved Hide resolved

arkodg reviewed Apr 16, 2024

View reviewed changes

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

zhaohuabing added 2 commits April 16, 2024 14:28

address comments

d68ee22

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

address comments

321b5ab

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

arkodg approved these changes Apr 17, 2024

View reviewed changes

arkodg requested review from a team and removed request for ZackButcher April 17, 2024 04:49

Merge branch 'main' into filter-order

7673385

guydc approved these changes Apr 17, 2024

View reviewed changes

guydc merged commit a7b4b55 into envoyproxy:main Apr 17, 2024
20 checks passed

zhaohuabing mentioned this pull request Apr 25, 2024

feat: support custom HTTP filter ordering #3273

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

API: HTTP Filter ordering #2993

API: HTTP Filter ordering #2993

zhaohuabing commented Mar 21, 2024 •

edited

Loading

codecov bot commented Mar 21, 2024 •

edited

Loading

guydc Mar 21, 2024

zhaohuabing Mar 21, 2024 •

edited

Loading

zhaohuabing commented Mar 26, 2024

arkodg commented Mar 26, 2024

arkodg Mar 26, 2024 •

edited

Loading

zhaohuabing Mar 26, 2024 •

edited

Loading

zhaohuabing commented Mar 27, 2024

zirain commented Mar 27, 2024

zhaohuabing commented Mar 27, 2024 •

edited

Loading

ZackButcher Mar 28, 2024 •

edited

Loading

zhaohuabing Mar 29, 2024 •

edited

Loading

arkodg commented Mar 29, 2024

arkodg commented Apr 12, 2024

zhaohuabing commented Apr 12, 2024

arkodg left a comment

guydc left a comment

-	FilterPriority []FilterPriority `json:"filterPriority,omitempty"`
+        type FilterOrder struct {
+                Add    []FilterPriority `json:"add,omitempty"`
+                Custom []FilterPriority `json:"custom,omitempty"`
+        }
+        FilterOrder FilterOrder `json:"filterOrder,omitempty"`

API: HTTP Filter ordering #2993

API: HTTP Filter ordering #2993

Conversation

zhaohuabing commented Mar 21, 2024 • edited Loading

codecov bot commented Mar 21, 2024 • edited Loading

Codecov Report

guydc Mar 21, 2024

Choose a reason for hiding this comment

zhaohuabing Mar 21, 2024 • edited Loading

Choose a reason for hiding this comment

zhaohuabing commented Mar 26, 2024

arkodg commented Mar 26, 2024

arkodg Mar 26, 2024 • edited Loading

Choose a reason for hiding this comment

zhaohuabing Mar 26, 2024 • edited Loading

Choose a reason for hiding this comment

zhaohuabing commented Mar 27, 2024

zirain commented Mar 27, 2024

zhaohuabing commented Mar 27, 2024 • edited Loading

ZackButcher Mar 28, 2024 • edited Loading

Choose a reason for hiding this comment

zhaohuabing Mar 29, 2024 • edited Loading

Choose a reason for hiding this comment

arkodg commented Mar 29, 2024

arkodg commented Apr 12, 2024

zhaohuabing commented Apr 12, 2024

arkodg left a comment

Choose a reason for hiding this comment

guydc left a comment

Choose a reason for hiding this comment

zhaohuabing commented Mar 21, 2024 •

edited

Loading

codecov bot commented Mar 21, 2024 •

edited

Loading

zhaohuabing Mar 21, 2024 •

edited

Loading

arkodg Mar 26, 2024 •

edited

Loading

zhaohuabing Mar 26, 2024 •

edited

Loading

zhaohuabing commented Mar 27, 2024 •

edited

Loading

ZackButcher Mar 28, 2024 •

edited

Loading

zhaohuabing Mar 29, 2024 •

edited

Loading