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

Add Support for SASL/SCRAM Authentication in README #711

Merged
merged 7 commits into from
Jan 1, 2025

Conversation

dyeaster
Copy link
Contributor

Add Support for SASL/SCRAM Authentication in README

Description

This pull request adds detailed instructions to the README for configuring and running Kafdrop with a Kafka cluster that has SASL/SCRAM authentication enabled. The changes include:

  • Instructions for creating a kafka.properties file with the necessary SASL/SCRAM configuration.
  • A command to run Kafdrop with the JAR file and the kafka.properties file.

Changes

  • Added a section in the README titled "Running from JAR with SASL/SCRAM Authentication".
  • Provided a sample kafka.properties file with the required configuration.
  • Included a command to start Kafdrop with the JAR file and the kafka.properties file.

Testing

  • Tested the configuration and command locally to ensure Kafdrop connects successfully to a Kafka cluster with SASL/SCRAM authentication.

Example

Create kafka.properties file
echo "sasl.mechanism=SCRAM-SHA-256 security.protocol=SASL_PLAINTEXT sasl.jaas.config=org.apache.kafka.common.security.scram.ScramLoginModule required username="your_username" password="your_password";" > kafka.properties
Run Kafdrop with the JAR file
java --add-opens=java.base/sun.nio.ch=ALL-UNNAMED -Dkafka.properties.file=kafka.properties -jar target/kafdrop-<version>.jar --kafka.brokerConnect=host:port,host:port

Conclusion

These changes will help users who are working with Kafka clusters that have SASL/SCRAM authentication enabled to configure and run Kafdrop more easily.

Please review and let me know if any further changes are needed.

Thank you

Copy link
Collaborator

@Bert-R Bert-R left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!
I've added a few comments to make the new example align more closely with the existing ones.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@dyeaster dyeaster requested a review from Bert-R December 19, 2024 02:23
Copy link
Collaborator

@Bert-R Bert-R left a comment

Choose a reason for hiding this comment

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

You marked all comments as revolved and requested a new review but forgot to push your update

dyeaster and others added 6 commits December 31, 2024 11:05
Co-authored-by: Bert Roos <[email protected]>
Co-authored-by: Bert Roos <[email protected]>
Co-authored-by: Bert Roos <[email protected]>
Co-authored-by: Bert Roos <[email protected]>
Co-authored-by: Bert Roos <[email protected]>
Co-authored-by: Bert Roos <[email protected]>
Copy link
Collaborator

@Bert-R Bert-R 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 now. Thanks!

@Bert-R Bert-R merged commit 5250711 into obsidiandynamics:master Jan 1, 2025
1 check passed
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