-
Notifications
You must be signed in to change notification settings - Fork 888
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[file configuration] Escaping environment variable substitution #3914
Comments
FYI @marcalff |
I don't think this will work. Whether the original yaml text contains The To escape variables, a new escape syntax is needed, independent of yaml. |
Updated. Thanks.
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 😆
Then we will be diverging from YAML even more. As literally we would add new escape syntax that is not defined by YAML. |
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 In Go, we could create custom types that would support env var substitution and use them instead of primitives like To sum up, probably most SIGs can indeed implement it. |
From: Pointing to: https://opentelemetry.io/docs/collector/configuration/#environment-variables
To be consistent with the collector, we can escape a single some_yaml_node: "$${NOT_ENV_VAR}" |
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:
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
The text was updated successfully, but these errors were encountered: