[extension/bearertokenauth] Fix the RPC credential updates #35653
+8
−3
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Note that I have in-fact never written golang before so would appreciate any pointers on how this actually should be done. |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
atoulme
reviewed
Nov 9, 2024
| type PerRPCAuth struct { | ||
| metadata map[string]string | ||
| renderMetadata func() map[string]string |
atoulme
reviewed
Nov 9, 2024
| @@ -22,13 +22,15 @@ import ( | |||
| var _ credentials.PerRPCCredentials = (*PerRPCAuth)(nil) | |||
|
|
|||
| // PerRPCAuth is a gRPC credentials.PerRPCCredentials implementation that returns an 'authorization' header. | |||
| // NOTE: Needs a function instead of static values so the data can be refreshed | |||
There was a problem hiding this comment.
Is this note better as an issue?
There was a problem hiding this comment.
I can try and reword the note - this is merely here to help people not trip up on the bug that was originally here as the design does not make it obvious that you actually need a function pointer rather than a static value (that then will not get refreshed)
|
Please add changelog, please add a test. |
- Change spaces to tabs - Reword the note
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes a bug - The gRPC headers not getting updated after collecter startup
Description
Does not appear there were any open/closed issues for this, but somewhere down the line the extension handling code has likely changed so that the
PerRPCCredentialsdoes not get called anymore for each call.To excite the issue - you can start the collector with valid credentials, then inject invalid credentials to the token file.
Expected behavior: The collector starts using the invalid credentials and fails authentication
Old behavior: The collector keeps using the credentials it read at startup (problematic if you need to refresh the token)
This fixes the behavior by moving the evaluation of the authorization token to happen when the metadata is being fetched instead of at a time when the auth object is created.
Testing
Manual testing performed by manipulating the token file on the disk.
The interface to the rest of the application should remain exactly the same.