-
Notifications
You must be signed in to change notification settings - Fork 5
update javy plugin #234
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
update javy plugin #234
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! I also prefer the change in kwctl annotate to the policy evaluator option.
I was trying something similar: using wasm-tools to convert the wasm file to a wat file and replacing the import name before converting back to a wasm file. But it makes more sense to handle through kwctl.
6f6d96c to
4c4cb39
Compare
|
I was thinking about this whole situation and I think we should get both fixes:
|
|
JFYI: the e2e tests are broken because we need a new version of kwctl to run them. One that has the patches linked to this PR |
About that, it's too early to cut an RC of kwctl, but we could create a What do you think? |
Fine by me. |
|
Fine by me as well |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for the explanation and the historical commits.
we could create a 1.30-beta1 release that includes all the patches we need.
What do you think?
Fine by me too.
Move to javy-plugin 4.0. This version requires javy plugins to be written for the wasip2 target. That requires to use the WebAssembly component model. This PR introduces WIT definitions for the javy plugin, plus the import function that is required by Kubewarden WASI policies. The WIT definitions make use of the new rich data types provided by the WIT specification. Signed-off-by: Flavio Castelli <[email protected]>
javy-plugin requires to build our plugin using the wasip2 target, which in turns forces the usage of the WebAssembly component model. However, at this time, the javy compiler (v7.0.0) produces wasip1 modules instead of wasm components. That means that inside of policy-evaluator we cannot use wasmtime's `bindgen` feature to generate the host bindings. The code generation works only when dealing with a wasm component. The signatures of WIT functions are pretty elaborated, they are too complex to be implemented manually. Hence, we have to define a WIT interface that relies on simple native WebAssembly types, which is exactly what waPC does. The WIT interface has to be defined using `i32` types only, which is actually what wasip1 uses under the hood when translating fancy Rust types like `*const u8` and `usize`. Moreover, right now there's no reason to have a WIT package for the kubewarden wasip2 policies since we don't have other languages that could make use of it. On top of changing the function signature, this commit moves the `host:call` import straight into the `world` of the plugin. This makes the final name more specific `kubewarden:javy/host:call`. The import name is then renamed by `kwctl annotate` to match what all the other WASI policy use (`host::call`). In this way we don't have to maintain any fancy import statement inside of policy-evaluator. Signed-off-by: Flavio Castelli <[email protected]>
Create a minor release and update rust version Signed-off-by: Flavio Castelli <[email protected]>
Make it clear we're not using the wasip2 target. Signed-off-by: Flavio Castelli <[email protected]>
Signed-off-by: Esosa Ohangbon <[email protected]>
The import of the `policy-action` function is not needed. This is a function that is not imported from the WebAssembly host, but is exposed by our javy plugin to the JS guest. Because of that, it doesn't have to be mentioned inside of the WIT file. Signed-off-by: Flavio Castelli <[email protected]>
Update to latest stable version Signed-off-by: Flavio Castelli <[email protected]>
Consume the 1.30.0-beta1 relase of kwctl. This is required to have the e2e tests work Signed-off-by: Flavio Castelli <[email protected]>
This is required to ensure our javy plugin can be built. Signed-off-by: Flavio Castelli <[email protected]>
ae33edc to
04fcf10
Compare
|
@kubewarden/kubewarden-developers this is ready to be reviewed again, and hopefully merged too. |
update javy plugin Signed-off-by: Esosa Ohangbon <[email protected]>
This PR updates the javy-plugin to its latest version (4.0.0). This update required quite some work and some changes.
Welcome wasip2
By using javy-plugin 4.0.0, the build target must be
wasip2, which means we have to use the component model.This PR introduces WIT definitions for the javy plugin, plus the import function that is required by Kubewarden WASI policies.
Wait, it's not really wasip2
Our plugin is built using the
wasip2target, but later the javy compiler (v7.0.0) takes our wasip2 component, the JavaScript/TypeScript code of the user and produces a.wasmfile.This file is not a WebAssembly component, it's just a regular WebAssembly module (it's actually rebased against wasip1).
This is a problem. That means that inside of policy-evaluator we cannot use wasmtime's
bindgenfeature to generate the host bindings for the function defined inside of the WIT file we wrote. The code generation works only when dealing with a wasm component, but we have a regular WebAssembly module.The signatures of WIT functions can be pretty elaborated (they can support strings, lists,...), but they are too complex to be implemented manually.
Since we cannot use the automatic code generation, we have to define a WIT interface that relies on simple native WebAssembly types so that we can do a manual implementation of that import.
So I went back to use the same signature that we use with the other WASI policies, this uses
i32types only, which is actually what wasip1 uses under the hood when translating fancy Rust types like*const u8andusize.This PR has a bunch of commits inside of it. The first shows how to do WIT in a proper way:
string,list<u8>,result<bool, string>)I wanted to leave the old implementation to leave a track of it. The information about how to write component models are not super clear and it took me some time to figure it out.
The final WIT file has been simplified to define everything inside of a single
world. That's because we don't plan to build any other type of policy using thewasip2target.Host import function name changed
By moving to WIT we lose control over how the final import function will be named.
As a result of that, the final
.wasmmodule produced by the javy compiler will import a function namedkubewarden:javy/host:callinstead of justhost:call. That causes kwctl and policy-server to choke when loading the policy since they cannot satisfy this import.Everything is broken...
Initially I fixed the problem by changing the
policy-evaluatorcode to expose a new function calledkubewarden:javy/host:call. The implementation of this function was of course the same as thehost:callone. I'm going to file a PR with this change, just to leave track of it.However, I'm not particularly happy about this approach. That means we have to keep around this hack inside of policy-evaluator for the time being.
I then realized there's another solution, just rename that import function. We're already rewriting the
.wasmmodule when we annotate it. I then made a small change to kwctl (I'll link the PR) that basically looks for this new import function with the silly name and just rename with the name we want.That means:
kwctl run non-annotated-policyis not going to workkwct annotateto produce a JS policy that will actually workI don't know which approach we should follow, patch
policy-evaluatororkwctl annotate? Let's discuss that here.Linked PRs: