open-telemetry / opentelemetry-collector

OpenTelemetry Collector
https://opentelemetry.io
Apache License 2.0
4.46k stars 1.47k forks source link

[Discussion] External secrets integration #2469

Closed gramidt closed 1 month ago

gramidt commented 3 years ago

Is your feature request related to a problem? Please describe. The Collector does not provide a mechanism to retrieve secrets from external systems or processes. The current mechanisms are to either hardcode the secret in the corresponding config or use environment variable expansion. While the latter is better than the former, it still has significant room for improvement, since environment variables are prone to being accidentally exposed.

Key problems to think about:

This problem is currently affecting multiple components that rely on secrets. If any component would like to adopt a more secure mechanism for secret management, they would have to implement them independently. The below list is just to reference a type of issue that currently exists, and is not to be considered a comprehensive list:

Describe the solution you'd like I would love a mechanism that's similar to the current environment variable expansion but allows for integration into secret managers, HSMs, and other options such as files. This approach would enable all components to support secure secrets without having to do anything initially. The tricky part is determining how to support secret rotations without restarting the collector.

Describe alternatives you've considered

gramidt commented 3 years ago

One idea could be to use something like gomplate (MIT License), since it has many of the needed integrations already built in ( see https://github.com/hairyhenderson/gomplate/tree/master/data ). It could be used within config/config.go and would follow a similar pattern to the current environment variable expansion ( https://github.com/open-telemetry/opentelemetry-collector/blob/9de6dd77eeaef5fd2557a5080df14e9b083de938/config/config.go#L683 ), and be additive, so that the current method of variable expansion will continue to work. If implemented, we'll just need to ensure that the templating does not clash with Helm.

gramidt commented 3 years ago

@alolita @jpkrohling - While not critical at this moment, could you provide your thoughts on this when you have some spare cycles?

jpkrohling commented 3 years ago

I like the proposal and I could have made use of that for the PR that introduced the PerRPCCredentials option, where we did touch the topic of a generic mechanism to get conf value from external files.

I would make this opt-in with the possibility for components to register a callback, so that they can reload whatever needs to be reloaded when the config value changes.

tigrannajaryan commented 3 years ago

@pjanotti is looking into this.

gramidt commented 3 years ago

@pjanotti - Let me know if you would like assistance in your research and initial PRs.

pjanotti commented 3 years ago

Hi @gramidt, @jpkrohling, @alolita and @tigrannajaryan here is a doc with a design based on some prototyping. I would love to get some quick feedback so we can make this available soon.

jpkrohling commented 3 years ago

Thanks for the proposal, @pjanotti! I left a couple of comments there.

gramidt commented 3 years ago

Thank you for the proposal, @pjanotti! I have added it to my queue to review this afternoon.

gramidt commented 3 years ago

@pjanotti - Passing this document around to get further feedback from our teams. I'll be sure to share the feedback in the near future.

gramidt commented 3 years ago

@pjanotti - I apologize for the delay. Our team overall loved the proposal. It went above what our expectation were and couldn't be more excited to see this take form. Let me know if you need anything at all.

BjarteHaram commented 2 years ago

Hi! My company recently enabled Kyverno to identify Kubernetes vulnerabilities and reported a vulnerability since we inject an API key into an environment variable. Does anyone have an update on the resolution to this issue?