Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/20421 )
Change subject: IMPALA-12403: Fix Kerberos authentication when connecting with a proxy user that does not delegate another user ...................................................................... Patch Set 2: Code-Review+1 (4 comments) looks great! just minor comments http://gerrit.cloudera.org:8080/#/c/20421/2/be/src/service/impala-hs2-server.cc File be/src/service/impala-hs2-server.cc: http://gerrit.cloudera.org:8080/#/c/20421/2/be/src/service/impala-hs2-server.cc@404 PS2, Line 404: username The name is a bit ambiguous to me, as we do not change the actual user name in line 415, only the name used in the ldap filter. Can you rename it to something like username_for_ldap_filters? http://gerrit.cloudera.org:8080/#/c/20421/2/be/src/service/impala-hs2-server.cc@406 PS2, Line 406: !connection_context->kerberos_user_principal.empty()) { nit, indentation: -2 for most lines http://gerrit.cloudera.org:8080/#/c/20421/2/be/src/service/impala-hs2-server.cc@410 PS2, Line 410: LOG(INFO) Maybe VLOG(1) is enough for this? If the tests use Impala in this way then logs will be full of these lines. http://gerrit.cloudera.org:8080/#/c/20421/2/fe/src/test/java/org/apache/impala/customcluster/LdapSearchBindImpalaShellTest.java File fe/src/test/java/org/apache/impala/customcluster/LdapSearchBindImpalaShellTest.java: http://gerrit.cloudera.org:8080/#/c/20421/2/fe/src/test/java/org/apache/impala/customcluster/LdapSearchBindImpalaShellTest.java@190 PS2, Line 190: "/"); we use /cliservice in all other tests, maybe it is also needed here? -- To view, visit http://gerrit.cloudera.org:8080/20421 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0141cd341cda84ed40cd72f8a038c8d5a215e5e1 Gerrit-Change-Number: 20421 Gerrit-PatchSet: 2 Gerrit-Owner: Gergely Farkas <gfar...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Comment-Date: Tue, 29 Aug 2023 17:14:55 +0000 Gerrit-HasComments: Yes
