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

Reply via email to