Implement wasi-config

[itowlson]

Fixes #2868.

This has the same considerations as our wasi-observe and wasi-keyvalue implementations in terms of versioning the Spin worlds - I mashed it into the existing Spin 2 worlds for development and testing but know that is not sustainable. (At this point, I'm assuming all this lot will be punted into a new Spin 3 world.)

I'm publishing this mostly to:

1. see if it works
2. give us the chance to look at implementation considerations and decide if we need to offer feedback to upstream proposal

(Regarding 2, it felt like we have some errors that don't map nicely to the proposal. I ended up using the io error variant for anything that wasn't a provider error, which feels potentially misleading. @kate-goldenring I'd value your thoughts on this!)

[rylev]

I think we'll want to consider whether we want to make this configurable for users of the factor - i.e., do we want to force all runtimes that use the VariablesFactor to link all three interfaces? We could instead make this linking conditional based on some configuration provided when the factor is constructed.

[lann]

Crate feature flag?

[rylev]

Seems like the feature is light-weight enough that we could just do a dynamic check instead of a compile time feature flag, but I don't feel strongly either way.

[itowlson]

We'll need to make a decision on this (and a similar consideration for wasi-keyvalue) fairly soon if we hope to land this in Spin 3.

[rylev]

My suggestion is just an API concern for those using this factor as library (in alternate runtimes). Whether Spin CLI turns this feature on, puts it behind a feature flag, or something else, is unrelated to whether the factor allows embedder to turn on wasi config or not.

That all being said, I'm highly in favor of allowing embedders to control what gets linked. I have less strong feelings about how that gets accomplished, but I think I prefer a dynamic check (i.e., a bool, enum, or maybe even bitflags that the embedder passes to the factor to indicate which interfaces should be linked in).

[itowlson]

It sounds like you're expressing a general principle about allowing embedders to control which interfaces get linked, rather than anything specific to wasi-config, or am I misunderstanding?

[rylev]

We should consider adding a conformance test for this. The runtime tests here are meant to be Spin CLI specific but this test seems like it should be applied to all Spin compliant runtimes.

[itowlson]

I can't do that as part of this PR but I've raised fermyon/conformance-tests#40 to remind us to do so once this merges.

[rylev]

I am slightly worried about performance - at some point we might want to see if adding some sort of caching mechanism to providers is worth it.

[lann]

We should probably push this down into the provider as e.g. (untested):

trait Provider {
    async fn get(&self, key: &Key) -> anyhow::Result<Option<String>>;

    async fn get_many(&self, keys: impl IntoIterator<Item = &Key>) -> anyhow::Result<impl IntoIterator<Option<String>>> {
        try_join_all(keys.into_iter().map(|key| self.get(key)).await
    }
}

[itowlson]

I don't think any of our current providers can optimise this beyond running the futures concurrently - all appear to support retrieving only one secret at once - so I propose we defer this.

[kate-goldenring]

@itowlson instead of adding this function, could we use lann's suggestion from get_all

[itowlson]

@kate-goldenring re #2869 (comment) can you clarify "no it doesn't" please? I didn't follow how the linked WIT comment applied - are you thinking of this as "we can't possibly find a key for an invalid name so should return Ok(None)" kind of thing?

[kate-goldenring]

@kate-goldenring re #2869 (comment) can you clarify "no it doesn't" please? I didn't follow how the linked WIT comment applied - are you thinking of this as "we can't possibly find a key for an invalid name so should return Ok(None)" kind of thing?

@itowlson I was aiming to point out that this is a weak spot of the WASI config interface and that we should propose updating it to either (A) be opinionated about string formatting or (B) support and invalid key error. My preference is that latter; however, the former would help achieve @rylev's point of enabling more portability across host implementatations.

[itowlson]

Updated this to sit over the world3 proposal (#2887). The wasi-config stuff is only the final commit - the first three are the world3 proposal baseline.

[kate-goldenring]

Thank you for adding this and doing all the work to assemble a world for Spin 3.0!

[kate-goldenring]

@itowlson instead of adding this function, could we use lann's suggestion from get_all

[itowlson]

@kate-goldenring I'm sceptical about pushing get_all down to the provider level because:

1. We would still need to do a try_join_all at the top level, because there could be multiple providers in play, even if each provider was batching.
2. If we sent a get_all off to each provider, the code for "rejoining" them would become more complicated, because two providers might return values for the same key, so we would need to take account of precedence (which resolve_template already does for us).
3. There appears to be no performance benefit for any current provider because none of the underlying implementations can batch requests.

Or maybe I'm misunderstanding the suggestion. Wouldn't be the first time...

If we're looking to optimise then I think the most practical thing we can do is retain a cache for the duration of the get-all request so we can go "oh I need to resolve key blah, but I've resolved that already in this cycle, so I'll just use the same value." Given timescales, my preference would be that we don't block on that and instead consider it as a non-breaking change in a subsequent PR, either before or after 3.0 depending on how we go. But I can take a look if folks prefer.