fermyon / spin

Spin is the open source developer tool for building and running serverless applications powered by WebAssembly.
https://developer.fermyon.com/spin
Apache License 2.0
5.14k stars 248 forks source link

Consider deprioritizing the environment variable provider #2584

Open kate-goldenring opened 2 months ago

kate-goldenring commented 2 months ago

In spin, application variables can be configured from multiple providers; however, the environment variable provider will always take precedence. This is odd given that it is the most static of the providers. It takes precedence because it is always at the top of the list of providers and the resolve_variable function loops through providers until it gets a value, so providers listed higher up in runtime config get higher precedence.

We could simply append the environment variable provider to the end of the list of providers to reduce it's precedence

kate-goldenring commented 2 months ago

This priority of providers is not documented, so i have a PR up to at least describe the current state: https://github.com/fermyon/developer/pull/1322

lann commented 2 months ago

This sounds like the kind of small, technically-breaking-but-probably-low-impact changes that would be good for a 3.0

itowlson commented 2 months ago

My guess is that the thought behind the order was that a user writing SPIN_VARIABLE_DB_ADDR=foo spin up should get the behaviour they (thought they) asked for, even if another provider was in play.

lann commented 2 months ago

That was my intent, but in some sense environment variables are generally going to be "more static" than say vault, so it does make some sense for it to be lower priority.

itowlson commented 2 months ago

I don't really get the "more static" argument. To me the criterion is more like "more explicit." Although I do get that an "ambient" EV is not very explicit, and we can't tell that apart from a "provided on the line with the command" EV... And since --runtime-config-file is always explicit, any "provided on the line" EV is arguably setting itself up for ambiguity anyway!

lann commented 2 months ago

The "staticness" argument is just a pragmatic one: you only get to update env vars when you restart the process, but you can update e.g. vault much more frequently; it is just more useful for the more-frequently-updatable provider to have higher precedence rather than being masked by the the lass-frequently-updatable provider.