Skip to content

wasi-blobstore draft support #3060

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

itowlson
Copy link
Collaborator

This is lurching towards readiness, although it still needs a lot of tidying. E.g.

  • A bunch of stuff needs renaming! Oh goodness does it need renaming. It's embarrassing.
  • A bunch of stuff would probably benefit from splitting into modules.
  • I'm not sure how to factor-test it. I can't follow the KV tests because InstanceBuilder::build() doesn't actually produce a dispatcher object - instead the factor setup does some terrifying shenanigans to try to hook our streams up to Spin's WASI implementation, which means the dispatcher is produced at link time rather than build time.
  • Somebody needs to look at those WASI shenanigans and tell me gently "it doesn't need to be like this Ivan."
  • I'm not sure how to integration-test it. I can integration-test the files provider no worries, but not sure about Azure and S3.
  • There are several to-dos (e.g. in the file system one we need to prevent paths escaping from the "container" root path, error messages that helpfully say "oh no").

There are some underspecification issues in the current draft of wasi-blobstore: I think I've made my implementations at least consistent, but I probably need to go back over and check that too.

The implementations in this PR are:

  • Azure blob storage
  • S3-compatible, although currently it uses the Amazon client so likely bound only to the S3 implementation
  • File system, primarily for k8s persistent volumes or other attached storage

I've tested all of them but not tested them hard.

Anyway, I'm sorry this is such a beast and I'm sorry it's still messy - I wanted to do enough to have reasonable confidence in the internal provider API, and I think I'm there bar the renaming and tidying, but it means there's a lot of code, and there's a lot of scar tissue!

@itowlson itowlson force-pushed the wasi-blobstore branch 2 times, most recently from 020f2a8 to 430093e Compare March 25, 2025 03:13
@itowlson
Copy link
Collaborator Author

Okay there is some silliness going on where it has upgraded an unrelated dependency that now requires Rust 1.83, but that makes one of the examples fling a strange new lint. I'll see if I can roll things back. Faugh.

@itowlson itowlson force-pushed the wasi-blobstore branch 3 times, most recently from ee1a99d to 2630a65 Compare March 25, 2025 03:48
}

pub struct BlobStoreDispatch<'a> {
allowed_containers: HashSet<String>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not entirely sure that it can be but if this can be borrowed it would save a clone

Suggested change
allowed_containers: HashSet<String>,
allowed_containers: &'a HashSet<String>,

Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually if this one works then probably all of the Arcs below could just be borrows. You'd still need the locks for interior mutability of course.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@lann I tried it and it hasn't segfaulted yet - thanks for the suggestion!

@itowlson itowlson force-pushed the wasi-blobstore branch 2 times, most recently from 6045e54 to 74449eb Compare March 31, 2025 19:20
Signed-off-by: itowlson <[email protected]>
@itowlson itowlson linked an issue Apr 30, 2025 that may be closed by this pull request
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.

Support WASI-blobstore
2 participants