-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Add OIDC method to Kafka authentication #41873
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
base: main
Are you sure you want to change the base?
Changes from 55 commits
f8a5c3b
fa2ffeb
0b1125a
bd6fdcf
eca5df4
65393c3
6a6e112
379fb01
57f9113
360412c
31f0a0f
c7f7018
f965e0f
6b14e44
c6c24cd
f0fed8f
e5a79a5
a1fc704
d17757a
ac3f072
462d478
ed1a135
cfb5165
d924d93
8a33c50
5b023a4
99ad70e
4f29b83
811b583
5ae0501
1b30400
16db2ef
a178305
425986a
9dec276
9d39786
b07483c
4009f94
3aa6ab8
8f98841
a31a5e3
6585885
d7f977e
2ab594c
bd388ce
114f1ea
bbd3f72
842cf07
f05b1c3
8a4705d
28ac00d
694ab48
b0686f8
339869c
50a4bd7
ac02343
bb5857e
3478222
f13d0a5
a7d360d
d82cabf
4063b85
2ede506
e366d03
05e3ac5
7c5a5ae
bdd2b46
d91ca1e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| # Use this changelog template to create an entry for release notes. | ||
|
|
||
| # One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' | ||
| change_type: enhancement | ||
|
|
||
| # The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) | ||
| component: kafka | ||
|
|
||
| # A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). | ||
| note: This change adds support for authentication via OIDC to the Kafka client. | ||
|
|
||
| # Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. | ||
| issues: [41872] | ||
|
|
||
| # (Optional) One or more lines of additional information to render under the primary note. | ||
| # These lines will be padded with 2 spaces and then inserted directly into the document. | ||
| # Use pipe (|) for multiline entries. | ||
| subtext: | | ||
| It provides an implementation of the sarama.AccessTokenProvider interface, supporting the | ||
| client_credentials Grant Type, and a background token refresh. | ||
| # If your change doesn't affect end users or the exported elements of any package, | ||
| # you should instead start your pull request title with [chore] or use the "Skip Changelog" label. | ||
| # Optional: The change log or logs in which this entry should be included. | ||
| # e.g. '[user]' or '[user, api]' | ||
| # Include 'user' if the change is relevant to end users. | ||
| # Include 'api' if there is a change to a library API. | ||
| # Default: '[user]' | ||
| change_logs: [] | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,6 +32,7 @@ const ( | |
| SCRAMSHA256 = "SCRAM-SHA-256" | ||
| PLAIN = "PLAIN" | ||
| AWSMSKIAMOAUTHBEARER = "AWS_MSK_IAM_OAUTHBEARER" //nolint:gosec // These aren't credentials. | ||
| OIDCFILE = "OIDCFILE" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this a mechanism name understood by anything else? How is this different to SASL/OAUTHBEARER? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was a early-development mistake - I'll be definitely pulling this out. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is now used, with the conversion of the other logic from sarama to franz-go, so it remains in. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My question was more about the choice of string, "OIDCFILE". I'm not aware of a SASL mechanism with that name, and couldn't find anything in searches. I think if we go with my other suggestion, we could use OAUTHBEARER. |
||
| ) | ||
|
|
||
| // NewFranzSyncProducer creates a new Kafka client using the franz-go library. | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI franz-go will soon be the default for both receiver and exporter, and when all the kinks are ironed out (in the not too distant future) the Sarama client will be removed.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@axw Thanks, and I agree completely with all your points/questions. This whole effort was animated by a request from another team of my employer, and they suggested this implementation to extend the Sarama client mechanism to do OIDC, and notably, to get the client_secret from a file (in usage scenario of using a Kubernetes pod secret file, which is dynamic and rotated frequently). I believe they may be using an older revision that does not have these facilities. I see now that the
oauth2clientextensionalready (very closely) has this support, includingclient_id_fileandclient_secret_file. (Full disclosure: I'm new to this project/repo, and working with OIDC as well.) Also, as the franz-go client migration will occur soon(ish), I'll go ahead and change this code to use that in lieu of Sarama. I think I can excise a lot of this, work through the call-chain from the franz_client.go constructors, and then modifyconfigkafka.SASLConfigand its users to use the oauth2clientextension.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @richscott! That sounds good. I'll move this PR back to draft in the mean time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, @axw - I have substantially reworked the branch, as you suggested:
extension/oauth2clientauthextensionpackage, so the changes are considerably smaller/simpler. I had to change a couple small things in the oauth2clientauthextension package code - make a publicClientCredentialsConfigso it was available to internal/kafka/oidc_client.go, added a TokenSource() constructor, etc, but other than that I have not modified anything inoauth2clientauthextension.replacedirective to the go.mod in each kafka-related component package in the repo, so the compiler would find and use the latest in the repo source, rather than downloading, so it would build.Running the
golintandgotestMake targets are successful, and an external toy integration program I wrote to just verify that the client does indeed connect with a OIDC token to a Kafka instance, do a few basic list/delete/create topic operations, sleep for a bit, then repeat infinitely, does work correctly - it refreshes the token as needed, etc.Probably there will be more work needed on this - I will mark this ready for review again and welcome comment/suggestions from everyone.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@richscott thanks for the changes. I'm glad to see most of the logic now lives in oauth2clientauthextension. I'd like to see if we can go even further.
What I have in mind is that users will configure
oauth2clientauthextensionindependently of the Kafka components, and then reference it via a new config attribute in the Kafka components. Something like this:The "oauthbearer_token_source" config setting would be the ID of an extension that implements a token source interface along these lines:
This way we're not duplicating any oauth2 config, and would be closer to how the oauth2client extension works with HTTP/gRPC-based exporters -- see https://github.com/open-telemetry/opentelemetry-collector/blob/main/config/configauth/README.md