Expand environment variables in configuration files

[ChrisRx]

I ended up implementing a feature to expand environment variables in pgdog.toml/users.toml to make it easier to work with how we are introducing sealed secrets into our Kubernetes tooling that deploys instances of PgDog: https://github.com/pgdogdev/pgdog/compare/main...censys-oss:pgdog:env-toml?expand=1. We could have gone the route of using an initContainer to glue everything together but this ended up being pretty easy and much easier to work with.

I wanted to ask here if this was something that ya'll would be interested in contributing back to PgDog? I'd be very happy to contribute this, but wanted to make an issue before firing off a PR per the contributing guidelines.

Thanks for the consideration and for this awesome project!

[levkk]

Hey! Curious, we already support a lot of the config options through env:

pgdog/pgdog-config/src/general.rs

Lines 1017 to 1045 in 9991e8a

	fn env_or_default<T: std::str::FromStr>(env_var: &str, default: T) -> T {
	env::var(env_var)
	.ok()
	.and_then(|v| v.parse().ok())
	.unwrap_or(default)
	}
	fn env_string_or_default(env_var: &str, default: &str) -> String {
	env::var(env_var).unwrap_or_else(|_| default.to_string())
	}
	fn env_bool_or_default(env_var: &str, default: bool) -> bool {
	env::var(env_var)
	.ok()
	.and_then(|v| match v.to_lowercase().as_str() {
	"true" | "1" | "yes" | "on" => Some(true),
	"false" | "0" | "no" | "off" => Some(false),
	_ => None,
	})
	.unwrap_or(default)
	}
	fn env_option<T: std::str::FromStr>(env_var: &str) -> Option<T> {
	env::var(env_var).ok().and_then(|v| v.parse().ok())
	}
	fn env_option_string(env_var: &str) -> Option<String> {
	env::var(env_var).ok().filter(|s| !s.is_empty())
	}

Definitely not all, especially [[databases]] would be hard to configure this way. Did you know about this and it was insufficient for your needs?

[ChrisRx]

I initially was using some of those, and I definitely could have missed it but the first one I ran into that I couldn't seem to find was for the admin settings (in particular the password), and then the [[databases]] section as well is another one that I needed to configure.

I'm certainly open to any alternatives you might have, or if the change I made could be improved to better align with the design and plans for PgDog. I came upon this idea to use shell-style variable expansion from another Rust project that has a fairly extensive configuration file (Apollo router) and it ended up being pretty straight forward. With that said though, happy to make changes or if ultimately you all don't think it is a good fit in general, that is perfectly fine too!

[levkk]

Your PR looks straightforward. I see no reason why we shouldn't merge it in, it's strictly an improvement and doesn't break anything as far as I can tell?

[ChrisRx]

I tried to be extremely careful with the behavior, so that it wouldn't affect any other workflows 🤞. The shellexpand::env_with_context function usage here will only make a substitution with an environment variable if it actually exists in the environment and passes it through unmodified to be parsed as toml otherwise. The only edge case I could think of is with environments with extremely short variable names set + an extremely unlucky password generation that happens to include a $ and that exact environment variable. That seems exceptionally rare to happen although I don't think it would be difficult to write a simple scanner as a complete replacement for shellexpand that only matches on ${VARIABLE} style variables to be extra cautious.

[ChrisRx]

It ended up being a lot easier than I had imagined, so I went ahead and updated the same fork branch with the replacement for shellexpand that only parses variables like ${VAR}. This should eliminate even the rarest case with shellexpand because it requires a matching closing bracket for a substitution to be made.

[levkk]

Open up a PR, happy to take a look.