Skip to content
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

Environment variables specification should require _FILE variations to support reading from files #2739

Open
kriswuollett opened this issue Aug 17, 2022 · 10 comments
Assignees
Labels
sig-issue A specific SIG should look into this before discussing at the spec

Comments

@kriswuollett
Copy link

What are you trying to achieve?

If an SDK supports environment variables, all of the environment variables should have a variation with a _FILE suffix so that the value can be read from a file instead of the variable itself. In some environments this would be extremely helpful where secrets are not allowed to be passed in an environment variable, e.g., for OTEL_EXPORTER_JAEGER_PASSWORD. Instead users should be able to point to a file specified by OTEL_EXPORTER_JAEGER_PASSWORD_FILE.

A use case that would probably be very common is mounting a Kubernetes Secret to a directory in a container. Besides actual secrets, Kubernetes users may access things like hostnames or ports within the same Secret, so supporting _FILE suffix for everything would remove the configuration burden of adding multiple configMapKeyRef and/or secretKeyRef references in Kubernetes workload resources. If all workloads reference the same ConfigMap and/or Secrets with the OTEL configuration values in their Namespace, then it makes it easier to have workloads set up similarly with less toil.

Additional context.

@kriswuollett kriswuollett added the spec:miscellaneous For issues that don't match any other spec label label Aug 17, 2022
@kriswuollett
Copy link
Author

Is this proposed change small, or does would it require making a proposal on https://github.com/open-telemetry/oteps?

@rbailey7210 rbailey7210 added the [label deprecated] triaged-needmoreinfo [label deprecated] The issue is triaged - the OTel community needs more information to decide label Aug 19, 2022
@tigrannajaryan
Copy link
Member

This should be likely covered by the config file.

@kriswuollett
Copy link
Author

@tigrannajaryan, I do not consider it to be the same request as the config file proposal. It suffers from the issue that Kubernetes users would need to create a ConfigMap or Secret with the contents of the config file stored in a data field, and then map it to the container as a volume. It adds toil to the process because some external process whether runtime by an operator process or at deployment time by a script would need to pull secrets and config to generate the file. There also is the issue that devops may not want secrets merged into other config so they'd only be able to use a Secret.

On the other hand, if someone only needs to set a few properties, then having the implicit _FILE variation would be superior for environments that do not allow secrets to be passed directly in environment variables.

@tigrannajaryan
Copy link
Member

What if the Otel config file supported a way to reference another file and fetch the value of a particular config setting from that other file?

@kriswuollett
Copy link
Author

I'd expect that doing something similar with a _file variation for a value would be useful since the config file would mostly likely not be changed much. But I'd probably also ask if the environment variable name is the equivalent of the config file yaml path, then why not have it be generally supported?

Some software packages only support a _FILE suffix for things known to be secrets, but there could be exceptions to the rule like DigitalOcean's Container Registry that uses the auth token for both username and password, so I just wish there was a general convention to read a value from a file. If _FILE is too general such that it may cause naming conflicts, maybe _FROM_FILE.

An alternative, but not quite as convenient, would be just storing the secrets subset in another configuration file since the file would still need to be regenerated whenever the secret changes.

Perhaps there is someone with more experience than me from CNCF Kubernetes or a whitepaper from the same that could point to the best practices (for Kubernetes, or other similar cloud users).

@zeitlinger
Copy link
Member

we've moved this to "not blocking stability", because there is a workaround: mount the config file from a config map secret

@svrnm svrnm added triage:deciding:needs-info Not enough information. It is left open to provide the author with time to add more details triage:deciding:tc-inbox Needs attention from the TC in order to move forward and removed [label deprecated] triaged-needmoreinfo [label deprecated] The issue is triaged - the OTel community needs more information to decide triage:deciding:needs-info Not enough information. It is left open to provide the author with time to add more details labels Sep 9, 2024
@svrnm
Copy link
Member

svrnm commented Sep 9, 2024

@open-telemetry/technical-committee does this belong to the configuration project or should this be handled independently?

@tigrannajaryan
Copy link
Member

This is tracked in Configuration Project Board and is in the purview of Config SIG. We do not need to involve the TC.

@tigrannajaryan tigrannajaryan added the sig-issue A specific SIG should look into this before discussing at the spec label Sep 11, 2024
@tigrannajaryan
Copy link
Member

@svrnm what's the right triaging label to apply to issues that are moved to a SIG?

@trask
Copy link
Member

trask commented Sep 12, 2024

@tigrannajaryan sig-issue label that you applied is correct

@trask trask removed spec:miscellaneous For issues that don't match any other spec label triage:deciding:tc-inbox Needs attention from the TC in order to move forward labels Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig-issue A specific SIG should look into this before discussing at the spec
Projects
Status: Not blocking stability
Development

No branches or pull requests

8 participants