cloudflare / foundations

Cloudflare's Rust service foundations library.
https://blog.cloudflare.com/introducing-foundations-our-open-source-rust-service-foundation-library
BSD 3-Clause "New" or "Revised" License
1.3k stars 55 forks source link

Allow setting configuration using environment variables #21

Open ibotty opened 9 months ago

ibotty commented 9 months ago

It would be great to allow setting (at least some) configuration using environment variables. This is established practice in many environments (see twelve factor apps, most containers).

This might be possible with serde-env. Would you consider a PR that implements it using serde-env?

inikulin commented 9 months ago

Yes, I think we can have it under a feature flag. However, it's unclear to me how that would interact with configuration coming from a config file. Should it be a separate Cli flag saying to obtain configuration from certain env var? Something like myservice -e MY_ENV_VAR.

ibotty commented 9 months ago

You are right in pointing that question out. See e.g. https://github.com/clap-rs/clap/discussions/2763 for Clap's discussion about it.

I would have said that the configs should be merged and one has preference (I'd say environment variables).

I don't quiet get what the example myservice -e MY_ENV_VAR should mean though. I envision the environment variables to correspond to members of the settings struct. E.g. for service myservice with struct MySetting { bind_v4: SocketAddrV4 } MYSERVICE_BIND_V4=0.0.0.0:8080 would set that field, similarly to what serde-env does: https://github.com/Xuanwo/serde-env/blob/main/benches/from_env.rs

What do you think?

inikulin commented 9 months ago

I feel hesitant about implicit merging of configuration - there are way too many ways how things can get wrong with it. I would rather not allow that conceptually as a practice. For my example: I meant that config file and configuration via env vars is mutually exclusive, you either start service with myservice -c myconfig.yaml loading configuration from a YAML file or you specify an env variable to load the configuration from: myservice -e MY_CONFIG.

So, for separate field overrides via env variables - I believe we should not have this. Too many risks of things going wrong and it's extremely hard to debug such a setup. It's desirable for configuration to come from a single source that you can refer to in case of any trouble and observe the whole picture.

ibotty commented 9 months ago

[not merging of configuration, but explicit choosing which source: env or config file]

Ah. I see. That's fine for me as well and easier as well.

So, for separate field overrides via env variables - I believe we should not have this.

I think I still did not make myself clear. Taking your input above in consideration, I propose the following.

When the app is invoked as myservice -e MY_CONFIG the settings struct

struct MySettings {
   a: String,
   b: String,
   c: SubSettings
}
struct SubSettings {
   a: String
}

would be configured using the following environment variables.

inikulin commented 9 months ago

So, argument provided with -e will be used as a prefix for env variables each of which configures certain config field? And the name of the variable is an expansion of the setting path, right? And to double check: this option will be mutually exclusive with -c option (YAML config)?

ibotty commented 9 months ago

I think that sounds right.

I would prefer if it would default to pick up an env var prefix (so it's possible to just start it as myservice), but that's no big deal.

inikulin commented 9 months ago

Would like to avoid implicit fallbacks, thus the option. sgtm, PRs are welcome

elasticdotventures commented 8 months ago

fwiw .. mix of environment & config file is super important.

In a majority of systems I frequently want a config file with 99.9% of the settings under version control, but often there is a use case for setting one or more exceptional variable using an environment .. (frequently containing a secret, or some type of localized dynamic content) .. this is absurdly common in k8s. .. the last thing I want is a need to generate a new config file with the dynamic content, then write it someplace, then run the program.

You are correct in that is easy to get messy .. your best bet is to use a standardized mechanism for collapsing any config file path into environment variables. If environment variables are set then always have the environment variables win. Introduce a debug log level which indicates which (if any) config settings were overloaded by an environment variable (not necessarily with the value, which again might contain a secret)

I'm super new to this and currently in evaluation stage, just looking at issues before I dive in.