[confmap] Decide on desired behavior for ${..} and ${env:..}

[mx-psi]

This writeup is intended to solve #8565 and define what to do with configuration resolution mechanisms before we try to remove the expandconverter.

Current state

Assume you have a Collector instance that has a field with type string and another field with type int. Let's also assume that there are two environment variables:

• UNQUOTED_OCTAL set to the value 0123
• QUOTED_OCTAL set to the value "0123"

Lastly assume that:

• the string field is set to Value is <input>
• the integer field is set to <input>

Where the value of <input> varies according to the table value. What should the configuration resolve to in this situation, if anything? The following table summarizes the current state:

Current behavior (click to expand)

<input>	Interpolated string field value	integer field value
0123	Value is 0123	83
"0123"	Value is "0123"	83
${env:UNQUOTED_OCTAL}	Value is 83	83
${env:QUOTED_OCTAL}	Value is 0123	83
${UNQUOTED_OCTAL}	Value is 0123	83
${QUOTED_OCTAL}	Value is "0123"	error

Discussion of current state

I feel like a desirable property of our configuration system would be the following:

(Property 1) Let str, a, b and c be strings and let <s> be the value of a string s. Assume that str is the concatenation of a, b, and c (i.e. <str> is the same as <a><b><c>). Then a configuration with a field set to <a>${env:B}<c> and an environment variable B set to <b> result in the same configuration value as a field set to <str>.

A second property that seems desirable to me is the following:

(Property 2) Let ENV be an environment variable name. Assume that both ${env:ENV} and ${ENV} both resolve to a scalar configuration value. Then a string field set to ${env:ENV} and a field set to ${ENV} should resolve to the same configuration.

The table above shows that we do not satisfy these naive properties:

1. The difference between Value is 0123 and Value is ${env:UNQUOTED_OCTAL} violates property 1.
2. The difference between Value is "0123" and Value is ${env:QUOTED_OCTAL} violates property 1.
3. The difference between ${env:QUOTED_OCTAL} and ${QUOTED_OCTAL} violates property 2.

I find violation 1 in particular very surprising, so I am going to focus on it.

Why (1) happens

In the envprovider we use yaml.Unmarshal into a structure . When parsing ${env:UNQUOTED_OCTAL}, the 0123 value will get parsed into an integer in octal base, and stored as 83 in the confmap.Retrieved struct. The fact that this was written in octal is then lost.

When we get to unmarshaling, since we set WeaklyTypedInput: true in our mapstructure configuration, the field will be converted to a string in base 10.

If we want to fix this...

I think we can make two changes to the configuration resolution logic that would address these. I am more convinced about change 1 than about change 2

Change 1: Defer scalar value resolution until you get to mapstructure

Instead of yaml.Unmarshaling into an any struct, we can unmarshal into a custom structure that defers evaluating scalar values and stores them as strings in the confmap.Retrieved struct. This seems possible with the current library we use (see a very rough PoC here: https://go.dev/play/p/g8hciqbI921). We leave the actual mapping to int/float/bool to the mapstructure library.

This would resolve violation 1, but not violation 2 or violation 3. The table would look like this (changes highlighted with a †):

Behavior after change 1 (click to expand)

<input>	Interpolated string field value	integer field value
0123	Value is 0123	83
"0123"	Value is "0123"	83
${env:UNQUOTED_OCTAL}	Value is 0123†	83
${env:QUOTED_OCTAL}	Value is 0123	83
${UNQUOTED_OCTAL}	Value is 0123	83
${QUOTED_OCTAL}	Value is "0123"	error

Change 2: Make ${..} parse YAML syntax, but fail if the result is not a scalar value

We only support ${..} when resolving to a scalar value. We can make it also parse YAML instead of passing the raw value (possibly with Change 1 included), and fail if the result is not a scalar value. (We can eventually make it work exactly as ${env:..}, but for now this is a more minimal change).

This would not work the same way as the Configuration WG proposal for the ${..}, which is the main reason I am not entirely convinced about doing this, but the difference would be in edge cases like "0123" (explicit quotes) or !!str 0123 (in the latter case in particular it feels like it's clearly okay to parse as YAML since the user clearly used YAML specific syntax).

Behavior after change 1 + change 2 (click to expand)

<input>	Interpolated string field value	integer field value
0123	Value is 0123	83
"0123"	Value is "0123"	83
${env:UNQUOTED_OCTAL}	Value is 0123†	83
${env:QUOTED_OCTAL}	Value is 0123	83
${UNQUOTED_OCTAL}	Value is 0123	83
${QUOTED_OCTAL}	Value is 0123†	error

Things to decide before confmap 1.0

1. Are property 1 and property 2 things we want?
2. If the answer to (1) is yes, is adhering to this properties something to do before 1.0?
3. If the answer to (2) is yes, should we do change 1?
4. If the answer to (2) is yes, should we do change 2?

[mx-psi]

I think we should decide on #9532 first, since if we do it the proposed change here wouldn't work. It's still valuable to think about the desired behavior here without focusing on the implementation.