Run a Wasm file as an application without a manifest by itowlson · Pull Request #2479 · spinframework/spin

itowlson · 2024-04-30T00:31:30Z

Fixes #2477. Or at least it explores what a solution might look like - there are definitely other paths to consider.

For example, this goes the route of constructing an in-memory spin.toml, because the route from spin.toml to locked app to trigger is well-exercised. But we could bypass that and construct a lockfile directly - a lot of spin.toml concerns are not (yet) relevant to manifestless applications.

Or we could start planning more extensively for Future Things - this POC doesn't really provide any support in terms of the OCI proposal.

One thing we discussed internally was whether to put this on spin up or on a new command such as spin serve (by analogy with wasmtime serve). For the sake of this POC I put it on spin up and inferred "bare file or manifest" based on the file extension. This is not intended as a proposal, just a thing to dangle the actual loader code off.

Anyway hopefully provides us with something to argue about grin

fibonacci1729 · 2024-04-30T00:40:22Z

src/commands/up/app_source.rs

        match spin_common::paths::resolve_manifest_file_path(path.into()) {
-            Ok(file) => Self::File(file),
+            Ok(file) => {
+                if file.extension().and_then(std::ffi::OsStr::to_str) == Some("wasm") {


Might be nice to support *.wat as well. If we want to be really precise we can use wasmparser to peek the special bytes to determine if the file is Wasm.

we can use wasmparser to peek the special bytes

...or bytes.starts_with(b"\x00asm") 🙂

we can use wasmparser to peek the special bytes

...or bytes.starts_with(b"\x00asm") 🙂

...or https://github.com/bytecodealliance/wasm-tools/blob/755abb691d9d401dad542b8f238d19e6f3624e16/crates/wat/src/lib.rs#L228 😸

This is absolutely world class bikeshedding, keep it up

To add to the bikeshedding — do we actually expect users to try this with .wat?

Most tools in this space accept either wat or wasm interchangeably, so yes...

I mean no, I don't think we expect users to use .wat, but also it isn't that hard to support apart from this slight inconvenience in detecting file type.

For now I have checked for the .wasm and .wat extensions, and isolated the "is it a Wasm file" logic into a function that we can evolve with magic numbers and wasmparsers and other such wild excesses if and when people feel strongly about it. And by "we" I mean you Brian

itowlson · 2024-05-20T23:03:07Z

Folks, do we have any thoughts on whether to take this forward in its current form? (Ignoring the detail of how we identify Wasm files.)

lann · 2024-05-21T14:48:57Z

I remain in favor of my proposal 🙂

tschneidereit · 2024-05-21T14:57:32Z

IIUC this would not provide a way to do any IO, because that would require having anything allow-listed, right? If so, I think that's a perfectly valid starting point, but it might make sense to see if we could at least provide slightly better error messages? Such as telling folks that if they want to do outbound http, they have to use a manifest?

itowlson · 2024-05-21T20:11:03Z

@lann Which proposal do you mean?

lann · 2024-05-21T20:13:40Z

@lann Which proposal do you mean?

#2477

itowlson · 2024-05-21T20:17:45Z

@tschneidereit That would require either threading "did the app come from a loose Wasm file" all the way through Spin, or majorly reworking our error reporting architecture.

This would be a case where the Target Worlds proposal would be helpful, because we can give manifestless apps an implicit target world that doesn't include outbound HTTP, KV stores, etc. etc. Target world validation would, I'm pretty sure, happen at a time and level where we are still threading the origin info through, so it would be easy to customise the world validation failure to talk about supplying a manifest.

If that's correct, then we could either punt this until after Target Worlds, or go ahead with this accepting that the errors will be suboptimal for now, with a view to sorting them out in the TW timeframe.

tschneidereit · 2024-05-23T09:46:21Z

@itowlson that makes sense, and I agree that if getting to better error messages right now is too hard, then that's okay because as you say we have other things in flight that should get us there :)

itowlson · 2024-05-23T19:39:46Z

Next discussion topic is do we put this on spin up or create a new spin serve / spin run / spin something-else command (to avoid any confusion likely to arise from flags being likely different across the manifestful and manifestless apps).

lann · 2024-05-23T21:14:59Z

I think spin up is fine. "Manifestless" really means "we create a secret manifest for you", right?

lann

A few non-blocking suggestions; looks great!

lann · 2024-05-24T12:48:50Z

crates/loader/src/lib.rs


+/// Load a Spin locked app from a standalone Wasm file.
+pub async fn from_wasm_file(wasm_path: impl AsRef<Path>) -> Result<LockedApp> {
+    let app_root = std::env::current_dir()?;


I would feel slightly better if we used a tempdir here. It shouldn't make any difference as the feature currently works, but as features are added I wouldn't want to accidentally expose content from the cwd.

Actually, I believe you already have working_dir where this function is called; could probably just pass that through.

This is the directory relative to which to resolve the Wasm file reference (and, perhaps, in future, other paths). If we use working_dir then we have to copy the Wasm file first (before copying it again), and any file-related errors will show unexpected locations.

The alternative is for app_root to be the directory containing the Wasm file, and to override wasm_path to be only the name of the Wasm file (so that the relative reference resolves correctly), but I'm not sure that's going to be helpful with hypothetical future features where the user will expect paths entered on the CLI to be resolved relative to the current directory. Of course it's hard to know because we haven't defined those features yet!

We could also do a more invasive change that threads multiple "app roots" through the loader, or redefines the app root, or absolutises everything earlier. Or we can have a separate loader path for much longer than this PR does it, e.g. doing any copies needed then jumping straight to building the lockfile instead of leaning on the existing manifestful path.

Sounds complicated. I retract my feelings.

lann · 2024-05-24T12:52:10Z

crates/loader/src/lib.rs

    Direct,
 }
+
+fn single_file_manifest(wasm_path: impl AsRef<Path>) -> spin_manifest::schema::v2::AppManifest {


I think you could get away with using the toml! macro here, which would be much less and code and easier to read.

lann · 2024-05-24T12:56:30Z

src/commands/up/app_source.rs

 pub enum AppSource {
    File(PathBuf),
    OciRegistry(String),
+    BareWasm(PathBuf),


Should we add an explicit CLI arg (e.g. --from-wasm) for this path like we have for the other app sources?

Yes, I had wondered about this. I'm concerned it would make the docs a bit confusing, given that --from-file (in the current proposal) also accepts Wasm files (because my lawyers inform me that Wasm files are files within the meaning of the Act). I suppose we could deprecate --from-file and define a new --from-manifest flag that forces the file to be interpreted as TOML, but this feels excessive. Although maybe I'll feel differently if I understand what #2529 is about.

Maybe it would be better to instead have an orthogonal option to differentiate between manifestful and manifestless sources. Anyway, that ought to be backward compatible with this so definitely not a blocker here.

Signed-off-by: itowlson <ivan.towlson@fermyon.com>

fibonacci1729 reviewed Apr 30, 2024

View reviewed changes

itowlson force-pushed the manifestless branch from fbe926f to f298c08 Compare April 30, 2024 00:51

lann mentioned this pull request Apr 30, 2024

Proposal: "Manifest-less" Single Component Apps #2477

Closed

itowlson force-pushed the manifestless branch from f298c08 to d1a5b70 Compare May 23, 2024 23:01

itowlson marked this pull request as ready for review May 23, 2024 23:05

itowlson force-pushed the manifestless branch from d1a5b70 to d017687 Compare May 24, 2024 02:47

lann approved these changes May 24, 2024

View reviewed changes

Run Wasm file without application manifest

3d562c4

Signed-off-by: itowlson <ivan.towlson@fermyon.com>

itowlson force-pushed the manifestless branch from d017687 to 3d562c4 Compare May 26, 2024 22:58

itowlson merged commit c14a222 into spinframework:main May 26, 2024

mikkelhegn mentioned this pull request Jun 19, 2024

Spin 2.6 release #2570

Closed

6 tasks

lann mentioned this pull request Jun 20, 2024

Add spin up --trigger-type flag #2585

Open

github-actions bot mentioned this pull request Feb 27, 2026

🌱 CNCF mission generation 2026-02-27 kubestellar/console-kb#11

Merged

Conversation

itowlson commented Apr 30, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fibonacci1729 May 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

itowlson commented May 20, 2024

Uh oh!

lann commented May 21, 2024

Uh oh!

tschneidereit commented May 21, 2024

Uh oh!

itowlson commented May 21, 2024

Uh oh!

lann commented May 21, 2024

Uh oh!

itowlson commented May 21, 2024

Uh oh!

tschneidereit commented May 23, 2024

Uh oh!

itowlson commented May 23, 2024

Uh oh!

lann commented May 23, 2024

Uh oh!

lann left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

fibonacci1729 May 1, 2024 •

edited

Loading