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

extend WasmPlugin's traffic selector to select paths #50638

Open
ramaraochavali opened this issue Apr 23, 2024 · 15 comments
Open

extend WasmPlugin's traffic selector to select paths #50638

ramaraochavali opened this issue Apr 23, 2024 · 15 comments

Comments

@ramaraochavali
Copy link
Contributor

Currently WASMPlugin's Traffic Selector injects WASM based on port or inbound/outbound. We would like to apply WASM filter only to specific paths/ or based on some header value.

Can we extend traffic selector to include path? This would need to enable WASM filter based on Envoy's Composite filter and select path at run time

@ramaraochavali
Copy link
Contributor Author

@kyessenov WDYT?

@howardjohn
Copy link
Member

howardjohn commented Apr 23, 2024

There has been a general desire to allow a targetRef=HTTPRoute on these types of resources. So I think that would be the path we go around, not another matching mechanism? Does that work for your use case or is the path orthogonal from routes?

@spacewander
Copy link
Contributor

If we want to select path in the wasm plugin, do we implement it in the control plane or data plane?

If we want to implement it in the control plane, the current Wasm Filter hasn't supported RDS-level configuration yet. It seems that a per-route Wasm VM will be expensive.

If we want to implement it in the data plane, will it be like Higress's WasmPlugin CRD, which provides a SDK to wrap the original proxy wasm SDK and do the route match?

@ramaraochavali
Copy link
Contributor Author

If we want to implement it in the control plane, the current Wasm Filter hasn't supported RDS-level configuration yet. It seems that a per-route Wasm VM will be expensive.

We want to support this controlplane but using Composite Filter + WASM. We do not need RDS level configuration for WASM for that.

@ramaraochavali
Copy link
Contributor Author

There has been a general desire to allow a targetRef=HTTPRoute on these types of resources.

Does it mean it works for only Gateway API routes? We would like for paths using simple composite filter action. We do not have to have a separate HTTPRoute/Virtual Service route defined for that path.

@kyessenov
Copy link
Contributor

I concur that it is better to make WasmPlugin self contained for traffic selection, rather than binding to a Gateway resource in a non-standard way. For inbound especially, we need this applied for passthrough traffic.

@ramaraochavali
Copy link
Contributor Author

@howardjohn Are you OK with adding this capability to WASM Plugin? If yes, I would like start this as we have many use cases coming up for this.

@howardjohn
Copy link
Member

I'm very concerned about us adding different matching mechanisms into each API.

Can you help explain your use case more around why you want to match path but cannot have a VS for that path?

@ramaraochavali
Copy link
Contributor Author

One example use case is path based Authz on the inbound path .The WASM filter would need to be deployed with different conditions or a different WASM filter needs to added for some paths. We do not generally define a VS for inbound paths. Without this capability the path processing logic gets in to each of the WASMs and has to skip paths that not relevant to it which is very cumbersome.

Even if you define VS for paths, how would we refer here unless we move to Gateway API - May be I am not understanding some thing here

@howardjohn
Copy link
Member

Ah sidecar inbound, that is is quite different ... FWIW linkerd does this by having HTTPRoutes apply to inbound (with no backendRef allowed, so its just a matcher). Just one option.

For VS vs Gateway API, I think we can make it work with both probably. But that was more for outbound

@ramaraochavali
Copy link
Contributor Author

Are you OK with adding path matching support for Inbound?

@howardjohn
Copy link
Member

We discussed this a bit in the WG meeting yesterday. The general consensus was we want to build out a generic matching representation for policies. For outbound, this is fairly obvious - bind to Route; inbound is slightly less clear. In the meantime this can be done via envoyfilter and/or matching in the WASM logic itself. There was some concerns about adding additional ad-hoc matching to another API.

@ramaraochavali
Copy link
Contributor Author

In the meantime this can be done via envoyfilter and/or matching in the WASM logic itself.

Adding that logic WASM filter it self not scalable - we do not know the paths upfront and every WASM filter needs to be changed to know about that path and act on it. Doing it via Envoy filter is not possible especially for WASM with image fetching involved - Istio relies on top level filter being WASM. If we change code and allow composite filter type of config also it works. IMO when we design the matching API, we should also consider inbound - without that it is not complete. In the mean time, should we change Istio to also work with ExtensionWithMatcher some thing like this #45889? If we generate WASM filter with extension with config - I think it would allow users to do it per path while we design the API

@kyessenov
Copy link
Contributor

@ramaraochavali are you fetching the Wasm from HTTP? I think Tetrate folks wanted to fix the native fetch support for Wasm in Envoy so it would not be an issue to use EnvoyFilter directly.

@ramaraochavali
Copy link
Contributor Author

ramaraochavali commented May 4, 2024 via email

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

No branches or pull requests

4 participants