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

Support custom CA for 1-way TLS #139

Merged
merged 3 commits into from
Sep 9, 2023

Conversation

jvansanten
Copy link
Contributor

Some Kafka clusters use TLS as a backdoor load-balancing mechanism, see e.g. strimzi's ingress feature. These are impossible to use with kt if the cluster uses a self-signed certificate, as it uses a default tls.Config in 1-way TLS mode. This PR adds the following features:

  • use ca-certificate if provided even in 1-way TLS mode
  • raise authentication errors in group and topic commands, rather than silently falling back to an unencrypted connection

rather than silently failing
This lets you use kt with unauthenticated clusters exposed via a k8s ingress, e.g. as deployed by strimzi
@fgeller
Copy link
Owner

fgeller commented Sep 9, 2023

👋 @jvansanten thank you for contributing! the changes lgtm -- thanks for boyscouting as well 🙏 do you want to follow up with a readme update?

@@ -271,6 +292,12 @@ func setupAuthTLS(auth authConfig, saramaCfg *sarama.Config) error {
return nil
}

func qualifyPath(argFN string, target *string) {
if *target != "" && !filepath.IsAbs(*target) {
Copy link
Owner

Choose a reason for hiding this comment

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

i think we need to also check if filepath.Dir(*target) == "." -- cf test failures for test-secrets/cert.

@fgeller fgeller merged commit 663b21a into fgeller:main Sep 9, 2023
1 check failed
jvansanten added a commit to jvansanten/kt that referenced this pull request Sep 11, 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