Commented environment variables are still attempted to be interpolated

[ghost]

When Vector is launched with this config

[sources.stdin]
  type = "stdin"
  # commented = "${UNDEFINED_VARIABLE}"

[sinks.console]
  type = "console"
  inputs = ["stdin"]

then, assuming that UNDEFINED_VARIABLE is not defined, there is a warning

Aug 25 13:48:36.037  WARN vector::topology::config::vars: unknown env var in config: "UNDEFINED_VARIABLE"

[binarylogic]

Thanks @a-rodin. I'm wondering how we can streamline this. Currently, we interpolate env vars before passing the config through the TOML parser. I can't think of a better way to solve this than to move env var interpolation to post TOML parsing. This means we'd have to walk all string values.

[ghost]

I think there's nothing bad in walking over string values, it shouldn't be much slower than deserialization itself. Maybe it can be implemented by deserializing TOML not directly to vector::topology::Config type, but first to toml::Value, recursively traversing the resulting toml::Value to do interpolations in all strings, and then using toml::Value::try_into<'_, vector::topology::Config> to convert the updated toml::Value to vector::topology::Config.

[ghost]

Although with that approach it would not be possible to use interpolations for non-string values, such as integers and booleans. So probably stripping out the comments and doing the interpolation after that before parsing would still be necessary to use vars for non-string values.

[lukesteensen]

Although with that approach it would not be possible to use interpolations for non-string values, such as integers and booleans.

This is the reason it works the way it does currently. It's definitely a little rough, but very simple.

Stripping out comments would work, but I'm not sure how to do it cleanly. I'd rather not go down a path of writing our own almost-TOML parser. The tricky part is that the pre-interpolation file doesn't necessarily have to be valid TOML itself (which ties into the non-string values problem).

[jszwedko]

We also have users using the environment variable interpolation to interpolate full blocks of configuration like:

[sources.stdin]
  type = "stdin"
  ${STIDN_CONFIG}

Which, if we did the interpolation later, would prohibit this use-case. This might be OK (maybe it isn't something we want to support), but just noting it for consideration.

[jszwedko]

One way we could do this is with an intermediate deserialization step that deserializes into JSON, does the interpolation, and then deserializes into Vector config.