Gergely Farkas has posted comments on this change. ( http://gerrit.cloudera.org:8080/19561 )
Change subject: IMPALA-11726: Allow LDAP user and group filter when Kerberos is enabled ...................................................................... Patch Set 7: (4 comments) http://gerrit.cloudera.org:8080/#/c/19561/7/be/src/rpc/authentication-test.cc File be/src/rpc/authentication-test.cc: http://gerrit.cloudera.org:8080/#/c/19561/7/be/src/rpc/authentication-test.cc@27 PS7, Line 27: #include "util/kudu-status-util.h" : #include "kudu/security/test/mini_kdc.h" > includes are generally sorted ¬alphabetically in Impala Done http://gerrit.cloudera.org:8080/#/c/19561/7/fe/src/test/java/org/apache/impala/customcluster/KerberosKdcEnvironment.java File fe/src/test/java/org/apache/impala/customcluster/KerberosKdcEnvironment.java: http://gerrit.cloudera.org:8080/#/c/19561/7/fe/src/test/java/org/apache/impala/customcluster/KerberosKdcEnvironment.java@102 PS7, Line 102: > nit: extra line Done http://gerrit.cloudera.org:8080/#/c/19561/7/fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTest.java File fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTest.java: http://gerrit.cloudera.org:8080/#/c/19561/7/fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTest.java@273 PS7, Line 273: TEST_USER_1 > The proxy and the delegated user are the same here, which seems like a spec I did not see that this case special, I thought that from the test point of view it doesn't matter who the delegated user is. To avoid confusion, I defined a new LDAP user and used it as the delegated user. http://gerrit.cloudera.org:8080/#/c/19561/7/fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTest.java@502 PS7, Line 502: enable_ldap_auth > This seems to be exactly the same as the ldap flags in testShellKerberosAut You are right! I removed the code duplication and created two new helper functions: getLdapSimpleBindFlags() and getCustomLdapSimpleBindSearchFilterFlags(). -- To view, visit http://gerrit.cloudera.org:8080/19561 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If3ca9c4ff8a17167e5233afabdd14c948edb46de Gerrit-Change-Number: 19561 Gerrit-PatchSet: 7 Gerrit-Owner: Gergely Farkas <gfar...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Gergely Farkas <gfar...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Tamas Mate <tma...@apache.org> Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com> Gerrit-Comment-Date: Thu, 09 Mar 2023 21:30:52 +0000 Gerrit-HasComments: Yes