opentelemetry-specification icon indicating copy to clipboard operation
opentelemetry-specification copied to clipboard

[file configuration] Escaping environment variable substitution

Open pellared opened this issue 11 months ago • 6 comments

What are you trying to achieve?

Reference: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/configuration/file-configuration.md#environment-variable-substitution

I would like to set a string value to ${NOT_ENV_VAR} and make sure it is not escaped.

Additional context.

I think that the user should take advantage of https://yaml.org/spec/1.2.2/#57-escaped-characters and do it e.g. like this:

key: "$\x7bNOT_ENV_VAR\x7d"

See: https://yaml-online-parser.appspot.com/?yaml=key%3A+%22%24%5Cx7bNOT_ENV_VAR%5Cx7d%22%0A&type=json

Even if this looks obvious, I would prefer to have it documented as the specification may also help in validating the implementation (and help in writing unit tests).

CC @jack-berg

pellared avatar Feb 29 '24 08:02 pellared

FYI @marcalff

pellared avatar Feb 29 '24 09:02 pellared

I don't think this will work.

Whether the original yaml text contains \x7b or {, the yaml parser will return a scalar that contains { anyway, so code that inspects a scalar -- after the yaml parser is done -- will see an environment variable.

The $ sign is missing by the way, to make it an environment variable reference.

To escape variables, a new escape syntax is needed, independent of yaml.

marcalff avatar Feb 29 '24 09:02 marcalff

The $ sign is missing by the way, to make it an environment variable reference.

Updated. Thanks.

I don't think this will work. [...] the yaml parser will return a scalar that contains

It is an implementation detail. I think the spec diverges so much from YAML (it is a superset) that for most SIGs it would require a dedicated parser.

Side note: That is one of the reasons why I am currently prejudiced towards having env var substitution for file configuration. For me, it is no longer a YAML. It makes the implementations more exposed to security bugs. It increases the implementation complexity. I am not convinced if the cost of this feature is worth its value. However, my opinions are never rock solid 😆

To escape variables, a new escape syntax is needed, independent of yaml.

Then we will be diverging from YAML even more. As literally we would add new escape syntax that is not defined by YAML.

pellared avatar Feb 29 '24 10:02 pellared

I think the spec diverges so much from YAML (it is a superset) that for most SIGs it would require a dedicated parser.

are you able to do env var substitution after YAML parsing, e.g. https://github.com/open-telemetry/opentelemetry-java/pull/5914?

trask avatar Feb 29 '24 23:02 trask

I think the spec diverges so much from YAML (it is a superset) that for most SIGs it would require a dedicated parser.

are you able to do env var substitution after YAML parsing, e.g. open-telemetry/opentelemetry-java#5914?

If I understand correctly the code that you refer to, it makes the substitution during (not after, not before) the YAML parsing by extending StandardConstructor. It alters the default YAML parsing behavior.

In Go, we could create custom types that would support env var substitution and use them instead of primitives like int, string, float64 .

To sum up, probably most SIGs can indeed implement it.

pellared avatar Mar 01 '24 08:03 pellared

From:

https://github.com/open-telemetry/oteps/blob/main/text/0225-configuration.md#environment-variable-substitution

Pointing to:

https://opentelemetry.io/docs/collector/configuration/#environment-variables

Use $$ to indicate a literal $. For example, representing $DataVisualization would look like the following:

To be consistent with the collector, we can escape a single $ sign with $$ as well.

some_yaml_node: "$${NOT_ENV_VAR}"

marcalff avatar Mar 03 '24 22:03 marcalff

From #3960:

In the Collector we support $${ENV} for this.

Should strive for consistency with collector unless there is a strong reason not to, in order to support #3963.

jack-berg avatar Mar 25 '24 14:03 jack-berg