[pkg/kafka] Add validation for AWS MSK SASL config and fix error message - #50936
Open
Rucha0901 wants to merge 1 commit into
Open
[pkg/kafka] Add validation for AWS MSK SASL config and fix error message#50936Rucha0901 wants to merge 1 commit into
Rucha0901 wants to merge 1 commit into
Conversation
Rucha0901
requested review from
a team,
MovieStoreGuy and
paulojmdias
as code owners
September 11, 2026 19:11
Contributor
|
Welcome, contributor! Thank you for your contribution to opentelemetry-collector-contrib. Important reminders:
|
singhvibhanshu
left a comment
singhvibhanshu
left a comment
Member
There was a problem hiding this comment.
Please add a changelog entry.
Signed-off-by: Rucha0901 <imt_2025071@iiitm.ac.in>
Rucha0901
force-pushed
the
fix-aws-msk-sasl-validation
branch
from
September 12, 2026 06:47
0d52f4a to
c1a17c7
Compare
Member
|
/workflow-approve |
singhvibhanshu
left a comment
singhvibhanshu
left a comment
Member
There was a problem hiding this comment.
Should this be breaking instead of bug_fix? Let's wait for the code-owners to have a look at it first 👍
|
|
||
| # One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' | ||
| change_type: breaking | ||
| change_type: 'bug_fix' |
Member
There was a problem hiding this comment.
Suggested change
| change_type: 'bug_fix' | |
| change_type: breaking |
paulojmdias
reviewed
Sep 12, 2026
paulojmdias
left a comment
paulojmdias
left a comment
Member
There was a problem hiding this comment.
Please add to test data client config, a test with a valid region.
| # Include 'api' if there is a change to a library API. | ||
| # Default: '[user]' | ||
| change_logs: [user] | ||
| change_logs: ['user'] |
Member
There was a problem hiding this comment.
Suggested change
| change_logs: ['user'] | |
| change_logs: [user] |
| }, | ||
| "sasl_invalid_mechanism": { | ||
| expectedErr: "auth::sasl: mechanism should be one of 'PLAIN', 'AWS_MSK_IAM_OAUTHBEARER', 'SCRAM-SHA-256' or 'SCRAM-SHA-512'. configured value FANCY", | ||
| expectedErr: "auth::sasl: mechanism should be one of 'PLAIN', 'AWS_MSK_IAM_OAUTHBEARER', 'OAUTHBEARER', 'SCRAM-SHA-256' or 'SCRAM-SHA-512'. configured value FANCY", |
Member
There was a problem hiding this comment.
Suggested change
| expectedErr: "auth::sasl: mechanism should be one of 'PLAIN', 'AWS_MSK_IAM_OAUTHBEARER', 'OAUTHBEARER', 'SCRAM-SHA-256' or 'SCRAM-SHA-512'. configured value FANCY", | |
| expectedErr: "auth::sasl::mechanism should be one of 'PLAIN', 'AWS_MSK_IAM_OAUTHBEARER', 'OAUTHBEARER', 'SCRAM-SHA-256' or 'SCRAM-SHA-512'. configured value FANCY", |
| expectedErr: "auth::sasl: mechanism should be one of 'PLAIN', 'AWS_MSK_IAM_OAUTHBEARER', 'OAUTHBEARER', 'SCRAM-SHA-256' or 'SCRAM-SHA-512'. configured value FANCY", | ||
| }, | ||
| "sasl_aws_msk_iam_oauthbearer_missing_region": { | ||
| expectedErr: "auth::sasl: region is required for AWS_MSK_IAM_OAUTHBEARER mechanism", |
Member
There was a problem hiding this comment.
Suggested change
| expectedErr: "auth::sasl: region is required for AWS_MSK_IAM_OAUTHBEARER mechanism", | |
| expectedErr: "auth::sasl::region is required for AWS_MSK_IAM_OAUTHBEARER mechanism", |
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No unresolved approval-blocking issues were identified.
Pull request overview
Adds AWS MSK SASL region validation and updates the unsupported-mechanism error message.
Changes:
- Requires a region for
AWS_MSK_IAM_OAUTHBEARER. - Includes
OAUTHBEARERin the error message. - Updates tests, fixtures, and changelog documentation.
File summaries
| File | Summary |
|---|---|
pkg/kafka/configkafka/testdata/client_config.yaml |
Adds a missing-region test fixture. |
pkg/kafka/configkafka/config.go |
Adds validation and updates the error message. |
pkg/kafka/configkafka/config_test.go |
Tests validation and revised errors. |
.chloggen/fix-aws-msk-sasl-validation.yaml |
Documents the bug fix. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or 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
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.
Description:
Missing AWS MSK SASL validation in the kafka package config and incomplete error message.
Link to tracking issue:
Fixes #50934
Testing:
Tests updated in config_test.go and verified locally.
Documentation:
None
Authorship: