Handle Wasm Images with wasm media types

[jsturtevant]

fixes: #108 (requires changes in containerd)

This adds the ability to handle oci artifacts as described in the proposal in https://docs.google.com/document/d/11shgC3l6gplBjWF1VJCWvN_9do51otscAm0hBDGSSAc.

Containerd doesn't currently support passing the annotations for the OCI artifact format and needs the following PR: containerd/containerd#8699

This pr relies on containerd/containerd#9142 which has been merged upstream

[jsturtevant]

This should likely go in after #142, will need some refactoring to support the executor. But wanted to get some eyes on the general direction.

[jsturtevant]

This no longer works after #131 since the entrypoint points to the wasm module which is no longer in the container image. Will need to re-think how to handle that scenario. Similar situation to #194

[jsturtevant]

not sure why the kind-oci test is failing, I ran into #362 when trying locally and will revisit tomorrow now that it is fixed

[jprendes]

It would be great to expose the platform in the WasiContext.
We could use the os part (wasip1/wasip2) to choose between modules or components, and os_features might be useful to know what components we need.

Also, that could also be used to choose between wasm container vs native container, right?

[jsturtevant]

Like this idea of passing OS and Arch to the wasicontext. For features, my understanding is we have some flexibility and we should follow up on how we decide to hand those.

[jprendes]

To be clear, I'm not implying we should start using os_features right now.
The details on that would need wider concensus and review.
But if we expose the Platform object (instead of just the OS / Arch strings), we open the door to that option.

[jsturtevant]

agreed!

[jsturtevant]

I've passed this on to the WasiContext, but not sure I like it the current way I did it. input welcome

[Mossaka]

lgtm! Thanks for this awesome work!

[jprendes]

CI is failling with

[ERROR] failed to initialize container process: wasmedge executor can't handle spec

which indicates we are hitting this line.

[jprendes]

Thanks @jsturtevant !
I left some minor comments.

[jprendes]

I am not a big fan of this. We would have to keep maintaining this every time the spec is updated.
Moreover, we are only interested in Wasm layers, why not probe for wasm layers only?
IIUC, this would let things like MediaType::ImageIndex, MediaType::ImageManifest, MediaType::ArtifactManifest to get through.

Could we use something like this instead?

.filter_map(|config| match config.media_type() {
    MediaType::Other(s) if s.starts_with("application/vnd.bytecodealliance.wasm.") => {
        Some(self.read_content(config.digest()).map(|module| WasmLayer {
            config: config.clone(),
            layer: module,
        }))
    }
    _ => None,
})

I can also imagine evolving WasmLayer into an enum that can tell apart different types of wasm layers (modules, components, etc), but not for this PR.

[jsturtevant]

The reason I didn't do just wasm layer types is because runtimes like spin/slight (probably others) will need to pass there configuration files. By leaving this more open ended, initially, we allow for those other types. In the case of spin they also have a media type for a module that doesn't match the application/vnd.bytecodealliance.wasm format.

I would like to tighten this up in the future once we get to the point where "everything is component" approach. The extra config files, static files, etc would just be component that the runtime knows how to use. But feels to early to be restrictive here.

I am not too concerned about this logic change all that often. It is a borrowed, idea from containerd's IsLayerType and that hasn't had a change in 4 years.

[jprendes]

We could add in the Instance trait a method to declare what layers it cares about (with a reasonable default impl).

Something like:

fn supported_layers_types() -> &'static [&'static str] {
    &["application/vnd.bytecodealliance.wasm.component.layer.v0+wasm"]
}

or maybe something like:

fn supports_layer_type(ty: MediaType) -> bool {
    match ty {
        MediaType::Other(s) => s.starts_with("application/vnd.bytecodealliance.wasm."),
        _ => false,
    }
}

If you think the current approach is better I'm happy with that as well, I don't want to block it over this :-)

[Mossaka]

lgtm!

[jsturtevant]

heads up this has the commit from #362

[jsturtevant]

heads up this has the commit from #362

I dropped it, as it shouldn't be needed.

[jprendes]

LGTM