Skip to content

Comments

[extension/basicauth] Add username_file and password_file config options#46227

Open
Aneurysm9 wants to merge 2 commits intoopen-telemetry:mainfrom
Aneurysm9:feat/basicauth_credential_file
Open

[extension/basicauth] Add username_file and password_file config options#46227
Aneurysm9 wants to merge 2 commits intoopen-telemetry:mainfrom
Aneurysm9:feat/basicauth_credential_file

Conversation

@Aneurysm9
Copy link
Member

Add a shared credentials file library (extension/internal/credentialsfile) that provides a ValueResolver interface for resolving secrets from inline config values or watched files. The library uses fsnotify to watch for file changes, enabling live credential rotation without collector restarts.

Wire the library into basicauthextension's client_auth config with two new fields: username_file and password_file. When set, file-based values take precedence over their inline counterparts. Credentials are read dynamically on each request via the ValueResolver, so file changes are reflected immediately for both HTTP and gRPC clients.

Assisted-by: Kiro (Amazon Q Developer)

@Aneurysm9 Aneurysm9 requested a review from a team as a code owner February 20, 2026 00:08
@github-actions github-actions bot requested a review from frzifus February 20, 2026 00:09
@Aneurysm9 Aneurysm9 force-pushed the feat/basicauth_credential_file branch 4 times, most recently from 4977a05 to 81c6c94 Compare February 20, 2026 01:00
@grelland
Copy link

Hello! I stumbled across this PR and noticed it relates closely to an open PR of mine #45953

The implementation seems to be based on the buggy one, and thus it will likely have the same problem. I suggest reconsidering the implementation of fileWatcher so that it supports the double-symlink strategy that Kubernetes employs for projected volume mounts. My PR includes a unit test to simulate this which might be adaptable for this one.

@Aneurysm9
Copy link
Member Author

Thanks for the heads up @grelland! This is indeed based on the implementation from bearertokenauth and a next step would be to refactor that to use this mechanism, so I'll definitely want to ensure that your fix is applied here.

Aneurysm9 and others added 2 commits February 20, 2026 14:20
Add a shared credentials file library (`extension/internal/credentialsfile`)
that provides a `ValueResolver` interface for resolving secrets from inline
config values or watched files. The library uses fsnotify to watch for
file changes, enabling live credential rotation without collector restarts.

Wire the library into basicauthextension's `client_auth` config with two
new fields: `username_file` and `password_file`. When set, file-based values
take precedence over their inline counterparts. Credentials are read
dynamically on each request via the `ValueResolver`, so file changes are
reflected immediately for both HTTP and gRPC clients.

Assisted-by: Kiro (Amazon Q Developer)
Signed-off-by: Anthony J Mirabella <a9@aneurysm9.com>
…d volumes

Watch the file itself instead of the parent directory. Kubernetes
projected volumes (ConfigMaps, Secrets, serviceAccountTokens) use a
double-symlink chain that is rotated via an atomic rename of an
intermediate .data symlink followed by removal of the old timestamped
directory. Watching the parent directory missed these events because
the filename filter never matched the intermediate symlink names.

The fix watches the file directly (fsnotify follows symlinks to the
underlying inode) and on Remove/Chmod events removes and re-adds the
watcher so it follows the new symlink target, then reloads the value.
This matches the approach in open-telemetry#45953.

Co-authored-by: Halvdan Hoem Grelland <grelland@users.noreply.github.com>
Assisted-by: Kiro (Amazon Q Developer)
Signed-off-by: Anthony J Mirabella <a9@aneurysm9.com>
@Aneurysm9 Aneurysm9 force-pushed the feat/basicauth_credential_file branch from 81c6c94 to 238a0f7 Compare February 20, 2026 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants