Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

api: wasm extension #2877

Merged
merged 26 commits into from
Mar 29, 2024
Merged

api: wasm extension #2877

merged 26 commits into from
Mar 29, 2024

Conversation

zhaohuabing
Copy link
Member

@zhaohuabing zhaohuabing commented Mar 11, 2024

This PR proposes an API for supporting Wasm plugins:

apiVersion: gateway.networking.k8s.io/v1alpha2
kind: EnvoyExtensionPolicy
metadata:
  name: wasm-extensions
spec:
  targetRef:
    kind: HTTPRoute
    name: backend
  wasm:
    - name: my_plugin_1
      code:
        image:  # the oci image that contains wasm code
          url: <oci-registry>.foo.bar.com/wasm-to-oci:v1 
          pullSecret: wasmPullSecret
        # http: https://foo.bar.com/example.wasm  # the HTTP URL containing the wasm code.
      sha256: xxxxxxx # SHA256 checksum that will be used to verify the wasm code.
      failOpen: true
      config:
        foo:
           foo1: value1
           foo2: value2
        bar: value3
      priority: 450   
    - name: my_plugin_2
      ...

@zhaohuabing zhaohuabing requested a review from a team as a code owner March 11, 2024 12:20
@zhaohuabing zhaohuabing force-pushed the wasm-extensions branch 3 times, most recently from 1e38c46 to 23b312f Compare March 11, 2024 12:30
@guydc
Copy link
Contributor

guydc commented Mar 11, 2024

Hi @zhaohuabing . We had some initial discussion here: #2546 on extensibility. The discussion was leaning towards a more common interface for all data plane extensions (golang, wasm, lua, ext-proc).

@Xunzhuo
Copy link
Member

Xunzhuo commented Mar 11, 2024

Also there is a proposal for containing all extensions by #2570, how do we deal with the relationship with that?

BTW, I prefer wasm for a high level API, since it is frequently used for extensions.

@guydc
Copy link
Contributor

guydc commented Mar 11, 2024

Which persona will be using this API? IMHO it's a bit dangerous to let application owners inject code into envoy, even if executed in isolation. If we see this as a policy for Gateway admins, should we support attachment to routes?

@zhaohuabing
Copy link
Member Author

zhaohuabing commented Mar 11, 2024

My apologies @guydc , I haven't noticed the existing discussion and the draft PR and didn't mean to add a competing PR. :-)

Before proposing the API, I considered two approaches to support data plane extensions (WASM, Lua, Golang, External Process, etc.)

  • A fat EnvoyExtensionPolicy containing all the extensions.
  • A set of EnvoyXExtensionPolicy, each supporting a specific extension.

The main problem of the first approach is that it would be difficult to support multiple extensions of the same type, e.g. users may need to define multiple WASM filters for different purposes for a single HTTPRoute. If we do support a list of each type of extensions in the fat EnvoyExtensionPolicy, that policy would become very large.

@zhaohuabing
Copy link
Member Author

Which persona will be using this API? IMHO it's a bit dangerous to let application owners inject code into envoy, even if executed in isolation. If we see this as a policy for Gateway admins, should we support attachment to routes?

It's common that there are multiple applications behind the gateway, and they may need different wasm extensions, just like the other policies.

@guydc
Copy link
Contributor

guydc commented Mar 11, 2024

My apologies @guydc , I haven't noticed the existing discussion and the draft PR and didn't mean to add a competing PR. :-)

All good, I'm very happy that this topic is getting more attention and I hope that we can make this happen for v1.1.0.

Before proposing the API, I considered two approaches to support data plane extensions (WASM, Lua, Golang, External Process, etc.)

I agree that different extensions do have different structures, interfaces, concerns and use patterns. Some support per-route config while other don't, some are more modular and may repeat multiple time in the FC, while others are external and more likely to only be called-out once in the filter chain.

I'm very much open to different extension types being supported for different use cases if other maintainers also support this approach.

@zhaohuabing
Copy link
Member Author

I agree that different extensions do have different structures, interfaces, concerns and use patterns. Some support per-route config while other don't, some are more modular and may repeat multiple time in the FC, while others are external and more likely to only be called-out once in the filter chain.

I'm very much open to different extension types being supported for different use cases if other maintainers also support this approach.

Let's bring this to the EG meeting and discuss further. Even we option to have multiple extension types, these types do need to address the common concerns raised in #2570 to ensure cohensive approach.

  • 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)

@zirain zirain added this to the Backlog milestone Mar 12, 2024
@zhaohuabing zhaohuabing self-assigned this Mar 12, 2024
@zhaohuabing zhaohuabing marked this pull request as draft March 15, 2024 01:53
Signed-off-by: huabing zhao <zhaohuabing@gmail.com>
Signed-off-by: huabing zhao <zhaohuabing@gmail.com>
Signed-off-by: huabing zhao <zhaohuabing@gmail.com>
@zhaohuabing zhaohuabing force-pushed the wasm-extensions branch 3 times, most recently from c5f4203 to 071ecc5 Compare March 21, 2024 03:44
Signed-off-by: huabing zhao <zhaohuabing@gmail.com>
Signed-off-by: huabing zhao <zhaohuabing@gmail.com>
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
Signed-off-by: huabing zhao <zhaohuabing@gmail.com>
Copy link

codecov bot commented Mar 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.51%. Comparing base (f699edf) to head (54b0eab).
Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2877   +/-   ##
=======================================
  Coverage   64.50%   64.51%           
=======================================
  Files         121      121           
  Lines       21381    21388    +7     
=======================================
+ Hits        13792    13798    +6     
- Misses       6718     6720    +2     
+ Partials      871      870    -1     

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

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>
@zhaohuabing zhaohuabing requested a review from arkodg March 28, 2024 00:16
Comment on lines 81 to 82
// +optional
SHA256 *string `json:"sha256,omitempty"`
// SHA256 *string `json:"sha256,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should remove it instead of comment.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I leave this because it's likely to be used when implementing wasm extension.

Of course, I'll remove it if I find out it's not necessary when implementing it.

api/v1alpha1/wasm_types.go Outdated Show resolved Hide resolved
api/v1alpha1/wasm_types.go Outdated Show resolved Hide resolved
api/v1alpha1/wasm_types.go Outdated Show resolved Hide resolved
Signed-off-by: huabing zhao <zhaohuabing@gmail.com>
Signed-off-by: huabing zhao <zhaohuabing@gmail.com>
Copy link
Contributor

@arkodg arkodg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks !

@arkodg arkodg requested review from zirain and a team March 28, 2024 07:05
@zirain zirain merged commit 50d10b5 into envoyproxy:main Mar 29, 2024
22 checks passed
@arkodg
Copy link
Contributor

arkodg commented Mar 29, 2024

hey @zhaohuabing lets start off with HTTP support, and temporarily pause on the design for the Image feature based on the discussions from envoyproxy/envoy#33212

ShyunnY pushed a commit to ShyunnY/gateway that referenced this pull request Apr 1, 2024
* API for wasm extensions

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

* fix lint

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

* ues envoyextenstionpolicy

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

* any.Any

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

* order

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

* change config to json string

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

* change config to json string

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

* fix lint

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

* add sha256 checksum

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

* fix gen

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

* address comments

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

* address comments

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

* fix test

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

* revert kube.mk

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* remove wasmextensionpolicies.yaml

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

* address comments

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

* remove sha256 for now

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

* address comments

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

* address comments

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

---------

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants