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

fix: do not override SASL mechanism for user REST proxy #620

Merged

Conversation

Acuion
Copy link

@Acuion Acuion commented May 16, 2023

About this change - What it does

If sasl_mechanism already defined in the config - do not override it with PLAIN.
Our Kafka installation allows SCRAM authentication only, so current code leads to failed authentication. Preferred mechanism already stated in the config, so let's use it for the user proxy.

Why this way

config["sasl_mechanism"] default value is None, so let's check for it. I'm not sure if we can remove this line completely and rely on that undefined SASL mechanism often means PLAIN. This is the most safe patch - keeping existing behavior.

@Acuion Acuion requested review from a team as code owners May 16, 2023 22:23
Copy link
Contributor

@tvainika tvainika left a comment

Choose a reason for hiding this comment

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

Looks good. This is also safe approach. Karapace configuration uses shared sasl_mechanism and some other configuration values for both REST proxy and schema registry, so it is better to fallback to PLAIN if not configured.

@tvainika tvainika merged commit 56f1139 into Aiven-Open:main May 17, 2023
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.

2 participants