Skip to content
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

Kafka: fix context use in AWS IAM auth #3305

Merged
merged 6 commits into from Jan 9, 2024

Conversation

famarting
Copy link
Contributor

Description

The Kafka awsiam authentication type did not actually work since it was making incorrect use of the context that was passed to the Init function

As it can be seen here https://github.com/dapr/dapr/blob/70e3f699793bb138d927079682261999b010e40e/pkg/runtime/processor/processor.go#L381 dapr cancels the context passed to Init once it finishes, making this context not suitable for background tasks such as using the aws sdk to get the kafka credentials at runtime...

Issue reference

We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.

Please reference the issue this PR will close: #[issue number]

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation / Created issue in the https://github.com/dapr/docs/ repo: dapr/docs#[issue number]

Signed-off-by: Fabian Martinez <46371672+famarting@users.noreply.github.com>
@famarting famarting requested review from a team as code owners January 5, 2024 10:44
common/component/kafka/auth.go Outdated Show resolved Hide resolved
famarting and others added 2 commits January 8, 2024 13:48
Signed-off-by: Fabian Martinez <46371672+famarting@users.noreply.github.com>
Signed-off-by: Alessandro (Ale) Segala <43508+ItalyPaleAle@users.noreply.github.com>
@ItalyPaleAle ItalyPaleAle changed the title fix kafka awsiam use of context Kafka: fix context use in AWS IAM auth Jan 8, 2024
ItalyPaleAle
ItalyPaleAle previously approved these changes Jan 8, 2024
@ItalyPaleAle ItalyPaleAle added this to the v1.13 milestone Jan 8, 2024
Copy link
Contributor

@eunicecompra eunicecompra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changes look good. There's a linting error though.

Signed-off-by: Fabian Martinez <46371672+famarting@users.noreply.github.com>
auto-merge was automatically disabled January 9, 2024 08:57

Head branch was pushed to by a user without write access

@ItalyPaleAle ItalyPaleAle added this pull request to the merge queue Jan 9, 2024
Merged via the queue into dapr:main with commit 7d39c46 Jan 9, 2024
171 of 175 checks passed
eunicecompra pushed a commit to eunicecompra/components-contrib that referenced this pull request Jan 9, 2024
Signed-off-by: Fabian Martinez <46371672+famarting@users.noreply.github.com>
Signed-off-by: Alessandro (Ale) Segala <43508+ItalyPaleAle@users.noreply.github.com>
Co-authored-by: Yaron Schneider <schneider.yaron@live.com>
Co-authored-by: Alessandro (Ale) Segala <43508+ItalyPaleAle@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants