-
Notifications
You must be signed in to change notification settings - Fork 249
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
Spin Build Targets Check SIP #2556
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
title = "SIP 0XX - Spin Build Targets Check" | ||
template = "main" | ||
date = "2024-06-13T12:00:00Z" | ||
--- | ||
Summary: Extend `spin build` to validate component targets against trigger types. | ||
|
||
Owner(s): | ||
[[email protected]](mailto:[email protected]) | ||
[[email protected]](mailto:[email protected]) | ||
|
||
Created: June 13, 2024 | ||
|
||
# Background | ||
When associating a Spin component to a specific trigger in the Spin manifest, there is an implied world that the Spin component is targeting (e.g. the http trigger type implies `fermyon:spin/http-trigger`). However, we don't know if the Spin component actually implements that world until instantiation time (i.e. when a request / event comes in). | ||
|
||
As a visual example, if a user mistankenly associates an http trigger with a component that targeted the redis world: | ||
|
||
`examples/hello-world/spin.toml:` | ||
```examples/hello-world/spin.toml | ||
[[trigger.http]] | ||
route = "/hello" | ||
component = "hello" | ||
|
||
[component.hello] | ||
source = "redis-component.wasm" # this is a redis component which is wrong | ||
``` | ||
|
||
When issuing a curl to this endpoint: | ||
``` | ||
❯ curl -i http://127.0.0.1:3000/hello | ||
HTTP/1.1 500 Internal Server Error | ||
content-length: 0 | ||
date: Tue, 04 Jun 2024 21:53:41 GMT | ||
``` | ||
|
||
And in the output of `spin up` we see: | ||
``` | ||
❯ spin up -f examples/hello-world | ||
Logging component stdio to "examples/hello-world/.spin/logs/" | ||
|
||
Serving http://127.0.0.1:3000 | ||
Available Routes: | ||
hello: http://127.0.0.1:3000/hello | ||
A simple component that returns hello. | ||
2024-06-04T21:53:16.160773Z ERROR spin_trigger_http: Error processing request: Expected component to either export `wasi:http/[email protected]` or `fermyon:spin/inbound-http` but it exported neither | ||
``` | ||
|
||
# Proposal | ||
|
||
Validate that each Spin component structurally conforms to (or targets) the expected world implied by the associated trigger type at build time rather than runtime. | ||
|
||
Prior art for performing a structural target of components against a world is [`wasm-tools component targets ...`](https://github.com/bytecodealliance/wasm-tools/blob/9340ed2466a50b4dbc580b13ba18a417dee91433/src/bin/wasm-tools/component.rs#L683) subcommand. We could potentially use [wac target predicate implemtation](https://github.com/bytecodealliance/wac/blob/4c96def294e6e779c52cfc5a93e05ed4c73ee60f/crates/wac-parser/src/resolution.rs#L2708) for a cleaner more actionable user diagnostic. Effectively, spin build could invoke the targets check using the trigger implied world and the source component. | ||
|
||
An "approximate" visual for the spin build experience we could enable: | ||
|
||
``` | ||
❯ spin build -f examples/hello-world | ||
Component "hello" is not a valid http component. | ||
|
||
Error: | ||
Expected component to either export `wasi:http/[email protected]` or `fermyon:spin/inbound-http` but it exported neither | ||
``` | ||
|
||
# Open question: How does this work with custom triggers? | ||
One possible solution is to allow trigger extensions the abilitiy to hook into build time validation via the `TriggerExecutor` trait. For each `spin build`, the `spin-cli` could construct a `build` command to invoke each trigger with allowing the custom trigger an opportunity to enforce validation on the target of each component before `up`. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense to me. Could add a standard e.g. Somewhat of a tangent, but we might want to think about how to avoid future collisions with trigger-specific flags; maybe we should reserve e.g. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure asking the trigger plugin is a good direction to go. It will solve the immediate "does my component match my trigger" question but that's not the real problem we're trying to lay foundations for. To refer back to the long game, suppose a user is running Spin 4.0 with v3.5 of the cron trigger, but specifies they want to the app to run on Spn 3.31 with cron 2.2, and on SpinKube 3.2 (with whatever cron trigger that includes). Asking the local trigger is not going to help us there. Maybe I'm YAGNI-ing though. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. An alternative @lann idea is to be able to query custom triggers for the worlds they support.. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @itowlson I'm not sure I follow your argument entirely. The goal of the feature is to determine at build time whether a certain component conforms to a given versioned world; in your counter-example, if the user specifies a version of the world that the local trigger plugin configured does not support, I would expect the check to fail (and ideally, there would be an error that plugins should be updated)? If is not the responsibility of trigger plugins, then I'm not sure how else you would be able to validate that, given Spin itself would have no knowledge of arbitrary worlds (outside of say HTTP and whatever else might be "default"). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (of course I should have just read the thread until the end and would have seen your detailed comment below...) |
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.
One thing this specific example brings up is that we probably don't one one monolithic "make sure it implements world W or bail" since the http trigger accepts several different worlds. I'm guessing a more flexible "validate this app" feature would work well here, combined with a library that makes it easy to check for targeting individual worlds.
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.
Some pedantry: the http trigger accepts several different guest worlds, but it implements one world (the http trigger world). In other worlds, the host world is a union of the various guest worlds it accepts. One way to think about this issue is: what world does the guest implement, and is that world a sub-type (sub-world?) of the host world?
I'm not sure this has an impact on how we're thinking about this feature in this SIP, but it might have an impact on implementation?
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.
I guess in the terminology of this SIP, "HTTP apps target at least one of several worlds."