-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[extension/oauth2clientauth] Enable dynamically reading ClientID and ClientSecret from file #26117
Comments
Pinging code owners:
See Adding Labels via Comments if you do not have permissions to add labels yourself. |
When I first implemented it, I thought about reloading the config from a file, but had to remove it as other folks said that this type of capability should come from the collector. It's been a few years now and the collector still doesn't provide the APIs to do that. I would go ahead and implement this, as it's not the first time this has been requested. My personal preference is to have |
@jpkrohling @pavankrish123 thank you very much for your feedback! I spent these days implementing and testing the |
/label -needs-triage |
…ClientSecret from files (#26310) **Description:** This PR implements the feature described in detail in the issue linked below. In a nutshell, it extends the `oauth2clientauth` extension to read ClientID and/or ClientSecret from files whenever a new token is needed for the OAuth flow. As a result, the extension can use updated credentials (when the old ones expire for example) without the need to restart the OTEL collector, as long as the file contents are in sync. **Link to tracking Issue:** #26117 **Testing:** Apart from the unit testing you can see in the PR, I've tested this feature in two real-life environments: 1. As a systemd service exporting `otlphttp` data 2. A Kubernetes microservice (deployed by an OpenTelemetryCollector CR) exporting `otlphttp` data In both cases, the collectors export the data to a service which sits behind an OIDC authentication proxy. Using the `oauth2clientauth` extension, the `otlphttp` exporter hits the authentication provider to issue tokens for the OIDC client and successfully authenticates to the service. In my cases, the ClientSecret gets rotated quite frequently and there is a stack making sure the ClientID and ClientSecret in the corresponding files are up-to-date. **Documentation:** I have extended the extension's README file. I'm open to more suggestions! cc @jpkrohling @pavankrish123
Closing the issue since we merged #26310 and is part of the 0.85.0 release. Thank you for your help and feedback. |
Component(s)
extension/oauth2clientauth
Is your feature request related to a problem? Please describe.
It's common that the secret of an OIDC client expires and, therefore, it has to get rotated frequently. As expected, after its expiration the OIDC client will get rejected if it uses the same secret to authenticate.
Thus, I have come across the need to refresh the value of the
client_secret
field while the OTEL collector is running without any downtime.More specifically, a service stores the credentials in files somewhere in the filesystem and refreshes these files when they rotate. Then, other services have to read the files to get the up-to-date values.
Describe the solution you'd like
I can propose two solutions for this:
file:/path/to/file
values to the corresponding attributes denoting that the actual value to use should be retrieved from the/path/to/file
file.Important: What if an actual value happens to begin with
file:
?client_id_file
andclient_secret_file
fields in the config.These fields take precedence over the
client_id
andclient_secret
fields respectively.Example config:
file:
URIs*_file
fieldsIt's important to note that existing configurations will continue to work. Any of the above changes is fully backwards compatible. We will just handle the config and the values in a little different way.
The implementation I have been using so far uses the first solution (
file:
URIs) and extends the logic ofclientcredentials.Config
object, used in theclientAuthenticator
object.To extend it, in this case, means to define a custom object (
clientCredentialsConfig
) with the exact same attributes:And, then, define some custom logic during the corresponding
Token()
method, called atopentelemetry-collector-contrib/extension/oauth2clientauthextension/extension.go
Line 74 in 0c12aa3
It eventually builds a
clientcredentials.Config
object using the same values, and replaces theClientID
and/orClientSecret
with the file contents (if they reference a file; i.e., contain afile:
-prefixed value).Following the official API (from https://pkg.go.dev/golang.org/x/oauth2 and https://pkg.go.dev/golang.org/x/oauth2/clientcredentials), we need to define the following objects:
Config > TokenSource > Token
A > B
denotes thatB
is created by a method ofA
.The plan is to make the currently used new Config in the place of the
clientcredentials.Config
with no other change when invoking its methods.The
TokenSource > Token
part is similar to what is already implemented forerrorWrappingTokenSource
.What do you think? Which of the two do you prefer? Do you have any other thoughts or suggestions?
Describe alternatives you've considered
An alternative would be to implement an independent service which watches the file for changes, updates the OTEL config, and restarts the collector.
However, apart from being odd as brutally forced and resulting in downtime, this cannot apply uniformly in any scenario. For example, I have needed this feature when using the collector both in a systemd service as well as in Kubernetes environments.
Additional context
I'm also planning on creating an issue about another, similar, feature regarding passing commands to retrieve the ClientID and/or ClientSecret. I have an implementation for this as well, introducing the extra
[]string
fieldscliend_id_cmd
andclient_secret_cmd
.I won't go into greater detail in this comment. If you want to discuss the command case in parallel to the file one, I can open a new issue or add more comments on this one.
The important part is that although reading from file can happen with a command (so the command could be considered as a superset feature), there are cases in which the collector runs on a minimal Docker image (e.g., the official one) without any external tools that can be used to read files. I have needed both (read from file, read from command output) in the environments I've been working on.
The text was updated successfully, but these errors were encountered: