Marton Greber has posted comments on this change. ( http://gerrit.cloudera.org:8080/24511 )
Change subject: KUDU-3787 Make tls_debug support Kerberos ...................................................................... Patch Set 1: (1 comment) No tests cover the new tls_only mode. Two options: 1. Unit test in negotiation-test.cc: set tls_only(true), negotiate against a GSSAPI+TLS server, verify it returns OK with a TlsSocket. 2. Tool test in kudu-tool-test.cc: run `kudu diagnose tls_debug` against a Kerberized cluster, check it prints TLS info. Otherwise looks good to me. Thanks! http://gerrit.cloudera.org:8080/#/c/24511/1/src/kudu/rpc/client_negotiation.cc File src/kudu/rpc/client_negotiation.cc: http://gerrit.cloudera.org:8080/#/c/24511/1/src/kudu/rpc/client_negotiation.cc@225 PS1, Line 225: if (tls_only_) { Question/observation: If `tls_only_` is true but TLS was not negotiated (e.g. the server didn't advertise TLS, or encryption was explicitly set to DISABLED), this returns OK without a TLS socket. The caller in `tool_action_diagnose.cc` does handle the `tls_socket == nullptr` case gracefully (prints "cleartext connection"), but is it intentional that tls_only mode succeeds silently even if no TLS handshake happened? It might be more helpful for the diagnostic tool to warn or error early here rather than proceed to print "cleartext" - since the whole purpose is to debug TLS. -- To view, visit http://gerrit.cloudera.org:8080/24511 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic6c6c9ac1cebeab8cb48bd22aea852c93c0252a4 Gerrit-Change-Number: 24511 Gerrit-PatchSet: 1 Gerrit-Owner: Attila Bukor <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Comment-Date: Thu, 25 Jun 2026 17:07:18 +0000 Gerrit-HasComments: Yes
