You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Illya Yalovyy <ya...@amazon.com> on 2016/12/08 00:45:46 UTC

Re: Review Request 53204: HIVE-15076 Improve scalability of LDAP authentication provider group filter

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53204/
-----------------------------------------------------------

(Updated Dec. 8, 2016, 12:45 a.m.)


Review request for hive, Aihua Xu, Ashutosh Chauhan, Chaoyu Tang, and Szehon Ho.


Changes
-------

Sensitive information was moved to DEBUG log.


Repository: hive-git


Description
-------

HIVE-15076 Improve scalability of LDAP authentication provider group filter

https://issues.apache.org/jira/browse/HIVE-15076


Diffs (updated)
-----

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 5ea9751 
  service/src/java/org/apache/hive/service/auth/ldap/DirSearch.java 33b6088 
  service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java 152c4b2 
  service/src/java/org/apache/hive/service/auth/ldap/LdapSearch.java 65076ea 
  service/src/java/org/apache/hive/service/auth/ldap/Query.java b8bf938 
  service/src/java/org/apache/hive/service/auth/ldap/QueryFactory.java e9172d3 
  service/src/test/org/apache/hive/service/auth/TestLdapAtnProviderWithMiniDS.java cd62935 
  service/src/test/org/apache/hive/service/auth/TestLdapAuthenticationProviderImpl.java 4fad755 
  service/src/test/org/apache/hive/service/auth/ldap/LdapAuthenticationTestCase.java acde8c1 
  service/src/test/org/apache/hive/service/auth/ldap/TestGroupFilter.java 0cc2ead 
  service/src/test/org/apache/hive/service/auth/ldap/TestLdapSearch.java 499b624 
  service/src/test/org/apache/hive/service/auth/ldap/TestQueryFactory.java 3054e33 
  service/src/test/resources/ldap/ad.example.com.ldif PRE-CREATION 
  service/src/test/resources/ldap/microsoft.schema.ldif PRE-CREATION 

Diff: https://reviews.apache.org/r/53204/diff/


Testing
-------

Build succeeded.

Test results:

Tests run: 149, Failures: 0, Errors: 0, Skipped: 0

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 03:14 min
[INFO] Finished at: 2016-10-26T13:53:15-07:00
[INFO] Final Memory: 36M/1091M
[INFO] ------------------------------------------------------------------------


Thanks,

Illya Yalovyy


Re: Review Request 53204: HIVE-15076 Improve scalability of LDAP authentication provider group filter

Posted by Aihua Xu <ax...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53204/#review160229
-----------------------------------------------------------


Ship it!




Ship It!

- Aihua Xu


On Dec. 12, 2016, 5:51 p.m., Illya Yalovyy wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53204/
> -----------------------------------------------------------
> 
> (Updated Dec. 12, 2016, 5:51 p.m.)
> 
> 
> Review request for hive, Aihua Xu, Ashutosh Chauhan, Chaoyu Tang, and Szehon Ho.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-15076 Improve scalability of LDAP authentication provider group filter
> 
> https://issues.apache.org/jira/browse/HIVE-15076
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 5ea9751 
>   service/src/java/org/apache/hive/service/auth/ldap/DirSearch.java 33b6088 
>   service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java 152c4b2 
>   service/src/java/org/apache/hive/service/auth/ldap/LdapSearch.java 65076ea 
>   service/src/java/org/apache/hive/service/auth/ldap/Query.java b8bf938 
>   service/src/java/org/apache/hive/service/auth/ldap/QueryFactory.java e9172d3 
>   service/src/test/org/apache/hive/service/auth/TestLdapAtnProviderWithMiniDS.java cd62935 
>   service/src/test/org/apache/hive/service/auth/TestLdapAuthenticationProviderImpl.java 4fad755 
>   service/src/test/org/apache/hive/service/auth/ldap/LdapAuthenticationTestCase.java acde8c1 
>   service/src/test/org/apache/hive/service/auth/ldap/TestGroupFilter.java 0cc2ead 
>   service/src/test/org/apache/hive/service/auth/ldap/TestLdapSearch.java 499b624 
>   service/src/test/org/apache/hive/service/auth/ldap/TestQueryFactory.java 3054e33 
>   service/src/test/resources/ldap/ad.example.com.ldif PRE-CREATION 
>   service/src/test/resources/ldap/microsoft.schema.ldif PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53204/diff/
> 
> 
> Testing
> -------
> 
> Build succeeded.
> 
> Test results:
> 
> Tests run: 149, Failures: 0, Errors: 0, Skipped: 0
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 03:14 min
> [INFO] Finished at: 2016-10-26T13:53:15-07:00
> [INFO] Final Memory: 36M/1091M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Illya Yalovyy
> 
>


Re: Review Request 53204: HIVE-15076 Improve scalability of LDAP authentication provider group filter

Posted by Illya Yalovyy <ya...@amazon.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53204/
-----------------------------------------------------------

(Updated Dec. 12, 2016, 5:51 p.m.)


Review request for hive, Aihua Xu, Ashutosh Chauhan, Chaoyu Tang, and Szehon Ho.


Changes
-------

1. added INFO log record for successful authentication
2. brought back WARN log records for unexpected LDAP exceptions


Repository: hive-git


Description
-------

HIVE-15076 Improve scalability of LDAP authentication provider group filter

https://issues.apache.org/jira/browse/HIVE-15076


Diffs (updated)
-----

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 5ea9751 
  service/src/java/org/apache/hive/service/auth/ldap/DirSearch.java 33b6088 
  service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java 152c4b2 
  service/src/java/org/apache/hive/service/auth/ldap/LdapSearch.java 65076ea 
  service/src/java/org/apache/hive/service/auth/ldap/Query.java b8bf938 
  service/src/java/org/apache/hive/service/auth/ldap/QueryFactory.java e9172d3 
  service/src/test/org/apache/hive/service/auth/TestLdapAtnProviderWithMiniDS.java cd62935 
  service/src/test/org/apache/hive/service/auth/TestLdapAuthenticationProviderImpl.java 4fad755 
  service/src/test/org/apache/hive/service/auth/ldap/LdapAuthenticationTestCase.java acde8c1 
  service/src/test/org/apache/hive/service/auth/ldap/TestGroupFilter.java 0cc2ead 
  service/src/test/org/apache/hive/service/auth/ldap/TestLdapSearch.java 499b624 
  service/src/test/org/apache/hive/service/auth/ldap/TestQueryFactory.java 3054e33 
  service/src/test/resources/ldap/ad.example.com.ldif PRE-CREATION 
  service/src/test/resources/ldap/microsoft.schema.ldif PRE-CREATION 

Diff: https://reviews.apache.org/r/53204/diff/


Testing
-------

Build succeeded.

Test results:

Tests run: 149, Failures: 0, Errors: 0, Skipped: 0

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 03:14 min
[INFO] Finished at: 2016-10-26T13:53:15-07:00
[INFO] Final Memory: 36M/1091M
[INFO] ------------------------------------------------------------------------


Thanks,

Illya Yalovyy


Re: Review Request 53204: HIVE-15076 Improve scalability of LDAP authentication provider group filter

Posted by Illya Yalovyy <ya...@amazon.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53204/
-----------------------------------------------------------

(Updated Dec. 9, 2016, 1:03 a.m.)


Review request for hive, Aihua Xu, Ashutosh Chauhan, Chaoyu Tang, and Szehon Ho.


Changes
-------

1. Updated logging
2. Added exception to error messages
3. Trivial style correction
4. Test methods renamed according to the actual filter implementation names


Repository: hive-git


Description
-------

HIVE-15076 Improve scalability of LDAP authentication provider group filter

https://issues.apache.org/jira/browse/HIVE-15076


Diffs (updated)
-----

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 5ea9751 
  service/src/java/org/apache/hive/service/auth/ldap/DirSearch.java 33b6088 
  service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java 152c4b2 
  service/src/java/org/apache/hive/service/auth/ldap/LdapSearch.java 65076ea 
  service/src/java/org/apache/hive/service/auth/ldap/Query.java b8bf938 
  service/src/java/org/apache/hive/service/auth/ldap/QueryFactory.java e9172d3 
  service/src/test/org/apache/hive/service/auth/TestLdapAtnProviderWithMiniDS.java cd62935 
  service/src/test/org/apache/hive/service/auth/TestLdapAuthenticationProviderImpl.java 4fad755 
  service/src/test/org/apache/hive/service/auth/ldap/LdapAuthenticationTestCase.java acde8c1 
  service/src/test/org/apache/hive/service/auth/ldap/TestGroupFilter.java 0cc2ead 
  service/src/test/org/apache/hive/service/auth/ldap/TestLdapSearch.java 499b624 
  service/src/test/org/apache/hive/service/auth/ldap/TestQueryFactory.java 3054e33 
  service/src/test/resources/ldap/ad.example.com.ldif PRE-CREATION 
  service/src/test/resources/ldap/microsoft.schema.ldif PRE-CREATION 

Diff: https://reviews.apache.org/r/53204/diff/


Testing
-------

Build succeeded.

Test results:

Tests run: 149, Failures: 0, Errors: 0, Skipped: 0

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 03:14 min
[INFO] Finished at: 2016-10-26T13:53:15-07:00
[INFO] Final Memory: 36M/1091M
[INFO] ------------------------------------------------------------------------


Thanks,

Illya Yalovyy


Re: Review Request 53204: HIVE-15076 Improve scalability of LDAP authentication provider group filter

Posted by Illya Yalovyy <ya...@amazon.com>.

> On Dec. 8, 2016, 3:55 p.m., Aihua Xu wrote:
> > service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java, line 145
> > <https://reviews.apache.org/r/53204/diff/3/?file=1579541#file1579541line145>
> >
> >     You may need to change message since it's expected that the user is not in some groups. Probably change to "Cannot match user ... and group ..." since "Failed to" seems to be an error.
> 
> Illya Yalovyy wrote:
>     I will update the message.

Usually it should just return true or false. If it fails with exception then something is wrong. That was reflected in the message. I noticed that I'm hiding the exception, which is a very bad practice. Will fix it as well. May be even WARN log message with exception details is required here. What you think? Again it should not happen usually, if it does - something wrong.


- Illya


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53204/#review158532
-----------------------------------------------------------


On Dec. 8, 2016, 12:45 a.m., Illya Yalovyy wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53204/
> -----------------------------------------------------------
> 
> (Updated Dec. 8, 2016, 12:45 a.m.)
> 
> 
> Review request for hive, Aihua Xu, Ashutosh Chauhan, Chaoyu Tang, and Szehon Ho.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-15076 Improve scalability of LDAP authentication provider group filter
> 
> https://issues.apache.org/jira/browse/HIVE-15076
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 5ea9751 
>   service/src/java/org/apache/hive/service/auth/ldap/DirSearch.java 33b6088 
>   service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java 152c4b2 
>   service/src/java/org/apache/hive/service/auth/ldap/LdapSearch.java 65076ea 
>   service/src/java/org/apache/hive/service/auth/ldap/Query.java b8bf938 
>   service/src/java/org/apache/hive/service/auth/ldap/QueryFactory.java e9172d3 
>   service/src/test/org/apache/hive/service/auth/TestLdapAtnProviderWithMiniDS.java cd62935 
>   service/src/test/org/apache/hive/service/auth/TestLdapAuthenticationProviderImpl.java 4fad755 
>   service/src/test/org/apache/hive/service/auth/ldap/LdapAuthenticationTestCase.java acde8c1 
>   service/src/test/org/apache/hive/service/auth/ldap/TestGroupFilter.java 0cc2ead 
>   service/src/test/org/apache/hive/service/auth/ldap/TestLdapSearch.java 499b624 
>   service/src/test/org/apache/hive/service/auth/ldap/TestQueryFactory.java 3054e33 
>   service/src/test/resources/ldap/ad.example.com.ldif PRE-CREATION 
>   service/src/test/resources/ldap/microsoft.schema.ldif PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53204/diff/
> 
> 
> Testing
> -------
> 
> Build succeeded.
> 
> Test results:
> 
> Tests run: 149, Failures: 0, Errors: 0, Skipped: 0
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 03:14 min
> [INFO] Finished at: 2016-10-26T13:53:15-07:00
> [INFO] Final Memory: 36M/1091M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Illya Yalovyy
> 
>


Re: Review Request 53204: HIVE-15076 Improve scalability of LDAP authentication provider group filter

Posted by Illya Yalovyy <ya...@amazon.com>.

> On Dec. 8, 2016, 3:55 p.m., Aihua Xu wrote:
> > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java, line 2426
> > <https://reviews.apache.org/r/53204/diff/3/?file=1579539#file1579539line2426>
> >
> >     Just curious why we don't just put the constant string "hive.server2.authentication.ldap.userMembershipKey" here like most of other entries?
> 
> Illya Yalovyy wrote:
>     Because it uses in several places. In particular in documentation. Putting a string in documentation is not maintainable, because later someone can change the string and forget to update in in all places. Documentation would become stale. In such a big project in will be a problem. JavaDoc has means to prevent that from happening by using string constants in documentation sections.
> 
> Aihua Xu wrote:
>     I got what you mean. But you can define as HIVE_SERVER2_PLAIN_LDAP_USERMEMBERSHIP_KEY("the string", null) and then refer the string as HiveConf.ConfVars.HIVE_SERVER2_PLAIN_LDAP_USERMEMBERSHIP_KEY.varname. Is that what you want?

Technically not a constant and one cannot reference it in that context.


> On Dec. 8, 2016, 3:55 p.m., Aihua Xu wrote:
> > service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java, line 115
> > <https://reviews.apache.org/r/53204/diff/3/?file=1579541#file1579541line115>
> >
> >     This should be info level which will be consistent with GroupMembershipKeyFilter class.
> 
> Illya Yalovyy wrote:
>     Ok. I'll generate 2 log entries then: 1. INFO without group information; 2. DEBUG with full information. 
>     
>     Does it make sense?
>     
>     See Naveen's comments for more details.
> 
> Illya Yalovyy wrote:
>     Actually this is a bit different. I'll change it to INFO.
> 
> Aihua Xu wrote:
>     Now I'm convinced for having 2 entries, 1. INFO without group information; 2. DEBUG with full information. 
>     
>     What do you mean it's different?

This particular record doesn't contain sensitive information.


- Illya


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53204/#review158532
-----------------------------------------------------------


On Dec. 9, 2016, 1:03 a.m., Illya Yalovyy wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53204/
> -----------------------------------------------------------
> 
> (Updated Dec. 9, 2016, 1:03 a.m.)
> 
> 
> Review request for hive, Aihua Xu, Ashutosh Chauhan, Chaoyu Tang, and Szehon Ho.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-15076 Improve scalability of LDAP authentication provider group filter
> 
> https://issues.apache.org/jira/browse/HIVE-15076
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 5ea9751 
>   service/src/java/org/apache/hive/service/auth/ldap/DirSearch.java 33b6088 
>   service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java 152c4b2 
>   service/src/java/org/apache/hive/service/auth/ldap/LdapSearch.java 65076ea 
>   service/src/java/org/apache/hive/service/auth/ldap/Query.java b8bf938 
>   service/src/java/org/apache/hive/service/auth/ldap/QueryFactory.java e9172d3 
>   service/src/test/org/apache/hive/service/auth/TestLdapAtnProviderWithMiniDS.java cd62935 
>   service/src/test/org/apache/hive/service/auth/TestLdapAuthenticationProviderImpl.java 4fad755 
>   service/src/test/org/apache/hive/service/auth/ldap/LdapAuthenticationTestCase.java acde8c1 
>   service/src/test/org/apache/hive/service/auth/ldap/TestGroupFilter.java 0cc2ead 
>   service/src/test/org/apache/hive/service/auth/ldap/TestLdapSearch.java 499b624 
>   service/src/test/org/apache/hive/service/auth/ldap/TestQueryFactory.java 3054e33 
>   service/src/test/resources/ldap/ad.example.com.ldif PRE-CREATION 
>   service/src/test/resources/ldap/microsoft.schema.ldif PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53204/diff/
> 
> 
> Testing
> -------
> 
> Build succeeded.
> 
> Test results:
> 
> Tests run: 149, Failures: 0, Errors: 0, Skipped: 0
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 03:14 min
> [INFO] Finished at: 2016-10-26T13:53:15-07:00
> [INFO] Final Memory: 36M/1091M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Illya Yalovyy
> 
>


Re: Review Request 53204: HIVE-15076 Improve scalability of LDAP authentication provider group filter

Posted by Illya Yalovyy <ya...@amazon.com>.

> On Dec. 8, 2016, 3:55 p.m., Aihua Xu wrote:
> > service/src/java/org/apache/hive/service/auth/ldap/QueryFactory.java, line 138
> > <https://reviews.apache.org/r/53204/diff/3/?file=1579544#file1579544line138>
> >
> >     Looks like we won't handle NPE so NPE may cause some problems. 
> >     
> >     If userMembershipAttr is null, will we still check userMememberOfGroup or not? If not, maybe we should handle such exception here. How about groupMembershipAttr above? Seems we will have such issue as well.
> 
> Illya Yalovyy wrote:
>     I think it should not happen, but I'll double check.

The filter will not be created for a case when 'hive.server2.authentication.ldap.userMembershipKey' is not set (NULL). It means we don't have to handle null in this code. If this NPE happens, it means there is a bug in the code.


> On Dec. 8, 2016, 3:55 p.m., Aihua Xu wrote:
> > service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java, line 115
> > <https://reviews.apache.org/r/53204/diff/3/?file=1579541#file1579541line115>
> >
> >     This should be info level which will be consistent with GroupMembershipKeyFilter class.
> 
> Illya Yalovyy wrote:
>     Ok. I'll generate 2 log entries then: 1. INFO without group information; 2. DEBUG with full information. 
>     
>     Does it make sense?
>     
>     See Naveen's comments for more details.

Actually this is a bit different. I'll change it to INFO.


- Illya


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53204/#review158532
-----------------------------------------------------------


On Dec. 8, 2016, 12:45 a.m., Illya Yalovyy wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53204/
> -----------------------------------------------------------
> 
> (Updated Dec. 8, 2016, 12:45 a.m.)
> 
> 
> Review request for hive, Aihua Xu, Ashutosh Chauhan, Chaoyu Tang, and Szehon Ho.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-15076 Improve scalability of LDAP authentication provider group filter
> 
> https://issues.apache.org/jira/browse/HIVE-15076
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 5ea9751 
>   service/src/java/org/apache/hive/service/auth/ldap/DirSearch.java 33b6088 
>   service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java 152c4b2 
>   service/src/java/org/apache/hive/service/auth/ldap/LdapSearch.java 65076ea 
>   service/src/java/org/apache/hive/service/auth/ldap/Query.java b8bf938 
>   service/src/java/org/apache/hive/service/auth/ldap/QueryFactory.java e9172d3 
>   service/src/test/org/apache/hive/service/auth/TestLdapAtnProviderWithMiniDS.java cd62935 
>   service/src/test/org/apache/hive/service/auth/TestLdapAuthenticationProviderImpl.java 4fad755 
>   service/src/test/org/apache/hive/service/auth/ldap/LdapAuthenticationTestCase.java acde8c1 
>   service/src/test/org/apache/hive/service/auth/ldap/TestGroupFilter.java 0cc2ead 
>   service/src/test/org/apache/hive/service/auth/ldap/TestLdapSearch.java 499b624 
>   service/src/test/org/apache/hive/service/auth/ldap/TestQueryFactory.java 3054e33 
>   service/src/test/resources/ldap/ad.example.com.ldif PRE-CREATION 
>   service/src/test/resources/ldap/microsoft.schema.ldif PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53204/diff/
> 
> 
> Testing
> -------
> 
> Build succeeded.
> 
> Test results:
> 
> Tests run: 149, Failures: 0, Errors: 0, Skipped: 0
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 03:14 min
> [INFO] Finished at: 2016-10-26T13:53:15-07:00
> [INFO] Final Memory: 36M/1091M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Illya Yalovyy
> 
>


Re: Review Request 53204: HIVE-15076 Improve scalability of LDAP authentication provider group filter

Posted by Illya Yalovyy <ya...@amazon.com>.

> On Dec. 8, 2016, 3:55 p.m., Aihua Xu wrote:
> > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java, line 2426
> > <https://reviews.apache.org/r/53204/diff/3/?file=1579539#file1579539line2426>
> >
> >     Just curious why we don't just put the constant string "hive.server2.authentication.ldap.userMembershipKey" here like most of other entries?

Because it uses in several places. In particular in documentation. Putting a string in documentation is not maintainable, because later someone can change the string and forget to update in in all places. Documentation would become stale. In such a big project in will be a problem. JavaDoc has means to prevent that from happening by using string constants in documentation sections.


> On Dec. 8, 2016, 3:55 p.m., Aihua Xu wrote:
> > service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java, line 90
> > <https://reviews.apache.org/r/53204/diff/3/?file=1579541#file1579541line90>
> >
> >     This seems to be a useful info that will help in diagnostics. Wondering why changes from info to debug level?

I totally agree, but Naveen doesn't want to expose group names in logs. It is a questionable concern, but moving it to DEBUG may be a good compromise.


> On Dec. 8, 2016, 3:55 p.m., Aihua Xu wrote:
> > service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java, line 115
> > <https://reviews.apache.org/r/53204/diff/3/?file=1579541#file1579541line115>
> >
> >     This should be info level which will be consistent with GroupMembershipKeyFilter class.

Ok. I'll generate 2 log entries then: 1. INFO without group information; 2. DEBUG with full information. 

Does it make sense?

See Naveen's comments for more details.


> On Dec. 8, 2016, 3:55 p.m., Aihua Xu wrote:
> > service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java, line 124
> > <https://reviews.apache.org/r/53204/diff/3/?file=1579541#file1579541line124>
> >
> >     Seems 'warn' is not necessary since that could be expected in the for loop, right?

It means we have a group in configuration that doesn't exist... Would you recommend log it at DEBUG level?


> On Dec. 8, 2016, 3:55 p.m., Aihua Xu wrote:
> > service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java, line 132
> > <https://reviews.apache.org/r/53204/diff/3/?file=1579541#file1579541line132>
> >
> >     Since we are throwing the exception, I guess such debug may be redundant. We should display such exception in the caller somewhere.

Exception message has a different (less descriptive) message. Please see Naveen's comments for more details.


> On Dec. 8, 2016, 3:55 p.m., Aihua Xu wrote:
> > service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java, line 139
> > <https://reviews.apache.org/r/53204/diff/3/?file=1579541#file1579541line139>
> >
> >     Seems this could be a info level message.

Same here.


> On Dec. 8, 2016, 3:55 p.m., Aihua Xu wrote:
> > service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java, line 145
> > <https://reviews.apache.org/r/53204/diff/3/?file=1579541#file1579541line145>
> >
> >     You may need to change message since it's expected that the user is not in some groups. Probably change to "Cannot match user ... and group ..." since "Failed to" seems to be an error.

I will update the message.


> On Dec. 8, 2016, 3:55 p.m., Aihua Xu wrote:
> > service/src/test/org/apache/hive/service/auth/TestLdapAuthenticationProviderImpl.java, line 265
> > <https://reviews.apache.org/r/53204/diff/3/?file=1579546#file1579546line265>
> >
> >     You may need to add some tests for the default configuraiton which is null for HIVE_SERVER2_PLAIN_LDAP_USERMEMBERSHIP_KEY.

If HIVE_SERVER2_PLAIN_LDAP_USERMEMBERSHIP_KEY is NULL this filter will not be used. I think we have enough test for this case. Did I get you correctly? Could you please provide more details about the test case you have in mind?


> On Dec. 8, 2016, 3:55 p.m., Aihua Xu wrote:
> > service/src/java/org/apache/hive/service/auth/ldap/QueryFactory.java, line 138
> > <https://reviews.apache.org/r/53204/diff/3/?file=1579544#file1579544line138>
> >
> >     Looks like we won't handle NPE so NPE may cause some problems. 
> >     
> >     If userMembershipAttr is null, will we still check userMememberOfGroup or not? If not, maybe we should handle such exception here. How about groupMembershipAttr above? Seems we will have such issue as well.

I think it should not happen, but I'll double check.


- Illya


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53204/#review158532
-----------------------------------------------------------


On Dec. 8, 2016, 12:45 a.m., Illya Yalovyy wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53204/
> -----------------------------------------------------------
> 
> (Updated Dec. 8, 2016, 12:45 a.m.)
> 
> 
> Review request for hive, Aihua Xu, Ashutosh Chauhan, Chaoyu Tang, and Szehon Ho.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-15076 Improve scalability of LDAP authentication provider group filter
> 
> https://issues.apache.org/jira/browse/HIVE-15076
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 5ea9751 
>   service/src/java/org/apache/hive/service/auth/ldap/DirSearch.java 33b6088 
>   service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java 152c4b2 
>   service/src/java/org/apache/hive/service/auth/ldap/LdapSearch.java 65076ea 
>   service/src/java/org/apache/hive/service/auth/ldap/Query.java b8bf938 
>   service/src/java/org/apache/hive/service/auth/ldap/QueryFactory.java e9172d3 
>   service/src/test/org/apache/hive/service/auth/TestLdapAtnProviderWithMiniDS.java cd62935 
>   service/src/test/org/apache/hive/service/auth/TestLdapAuthenticationProviderImpl.java 4fad755 
>   service/src/test/org/apache/hive/service/auth/ldap/LdapAuthenticationTestCase.java acde8c1 
>   service/src/test/org/apache/hive/service/auth/ldap/TestGroupFilter.java 0cc2ead 
>   service/src/test/org/apache/hive/service/auth/ldap/TestLdapSearch.java 499b624 
>   service/src/test/org/apache/hive/service/auth/ldap/TestQueryFactory.java 3054e33 
>   service/src/test/resources/ldap/ad.example.com.ldif PRE-CREATION 
>   service/src/test/resources/ldap/microsoft.schema.ldif PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53204/diff/
> 
> 
> Testing
> -------
> 
> Build succeeded.
> 
> Test results:
> 
> Tests run: 149, Failures: 0, Errors: 0, Skipped: 0
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 03:14 min
> [INFO] Finished at: 2016-10-26T13:53:15-07:00
> [INFO] Final Memory: 36M/1091M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Illya Yalovyy
> 
>


Re: Review Request 53204: HIVE-15076 Improve scalability of LDAP authentication provider group filter

Posted by Illya Yalovyy <ya...@amazon.com>.

> On Dec. 8, 2016, 3:55 p.m., Aihua Xu wrote:
> > service/src/test/org/apache/hive/service/auth/TestLdapAuthenticationProviderImpl.java, line 265
> > <https://reviews.apache.org/r/53204/diff/3/?file=1579546#file1579546line265>
> >
> >     You may need to add some tests for the default configuraiton which is null for HIVE_SERVER2_PLAIN_LDAP_USERMEMBERSHIP_KEY.
> 
> Illya Yalovyy wrote:
>     If HIVE_SERVER2_PLAIN_LDAP_USERMEMBERSHIP_KEY is NULL this filter will not be used. I think we have enough test for this case. Did I get you correctly? Could you please provide more details about the test case you have in mind?

I think this use case is tested in #testAuthenticateWhenGroupFilterPasses(). Probably I should rename other tests to make it clear.


- Illya


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53204/#review158532
-----------------------------------------------------------


On Dec. 8, 2016, 12:45 a.m., Illya Yalovyy wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53204/
> -----------------------------------------------------------
> 
> (Updated Dec. 8, 2016, 12:45 a.m.)
> 
> 
> Review request for hive, Aihua Xu, Ashutosh Chauhan, Chaoyu Tang, and Szehon Ho.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-15076 Improve scalability of LDAP authentication provider group filter
> 
> https://issues.apache.org/jira/browse/HIVE-15076
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 5ea9751 
>   service/src/java/org/apache/hive/service/auth/ldap/DirSearch.java 33b6088 
>   service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java 152c4b2 
>   service/src/java/org/apache/hive/service/auth/ldap/LdapSearch.java 65076ea 
>   service/src/java/org/apache/hive/service/auth/ldap/Query.java b8bf938 
>   service/src/java/org/apache/hive/service/auth/ldap/QueryFactory.java e9172d3 
>   service/src/test/org/apache/hive/service/auth/TestLdapAtnProviderWithMiniDS.java cd62935 
>   service/src/test/org/apache/hive/service/auth/TestLdapAuthenticationProviderImpl.java 4fad755 
>   service/src/test/org/apache/hive/service/auth/ldap/LdapAuthenticationTestCase.java acde8c1 
>   service/src/test/org/apache/hive/service/auth/ldap/TestGroupFilter.java 0cc2ead 
>   service/src/test/org/apache/hive/service/auth/ldap/TestLdapSearch.java 499b624 
>   service/src/test/org/apache/hive/service/auth/ldap/TestQueryFactory.java 3054e33 
>   service/src/test/resources/ldap/ad.example.com.ldif PRE-CREATION 
>   service/src/test/resources/ldap/microsoft.schema.ldif PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53204/diff/
> 
> 
> Testing
> -------
> 
> Build succeeded.
> 
> Test results:
> 
> Tests run: 149, Failures: 0, Errors: 0, Skipped: 0
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 03:14 min
> [INFO] Finished at: 2016-10-26T13:53:15-07:00
> [INFO] Final Memory: 36M/1091M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Illya Yalovyy
> 
>


Re: Review Request 53204: HIVE-15076 Improve scalability of LDAP authentication provider group filter

Posted by Aihua Xu <ax...@cloudera.com>.

> On Dec. 8, 2016, 3:55 p.m., Aihua Xu wrote:
> > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java, line 2426
> > <https://reviews.apache.org/r/53204/diff/3/?file=1579539#file1579539line2426>
> >
> >     Just curious why we don't just put the constant string "hive.server2.authentication.ldap.userMembershipKey" here like most of other entries?
> 
> Illya Yalovyy wrote:
>     Because it uses in several places. In particular in documentation. Putting a string in documentation is not maintainable, because later someone can change the string and forget to update in in all places. Documentation would become stale. In such a big project in will be a problem. JavaDoc has means to prevent that from happening by using string constants in documentation sections.

I got what you mean. But you can define as HIVE_SERVER2_PLAIN_LDAP_USERMEMBERSHIP_KEY("the string", null) and then refer the string as HiveConf.ConfVars.HIVE_SERVER2_PLAIN_LDAP_USERMEMBERSHIP_KEY.varname. Is that what you want?


> On Dec. 8, 2016, 3:55 p.m., Aihua Xu wrote:
> > service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java, line 90
> > <https://reviews.apache.org/r/53204/diff/3/?file=1579541#file1579541line90>
> >
> >     This seems to be a useful info that will help in diagnostics. Wondering why changes from info to debug level?
> 
> Illya Yalovyy wrote:
>     I totally agree, but Naveen doesn't want to expose group names in logs. It is a questionable concern, but moving it to DEBUG may be a good compromise.

I see. That makes sense. Then maybe we can have both, so sensitive group only printed during debug level and we will still see authetication succuss message during info level? How do you think? 

LOG.debug("GroupMembershipKeyFilter passes: user '{}' is a member of '{}' group",...
LOG.info("Authentication succeeded based on group membership");


> On Dec. 8, 2016, 3:55 p.m., Aihua Xu wrote:
> > service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java, line 115
> > <https://reviews.apache.org/r/53204/diff/3/?file=1579541#file1579541line115>
> >
> >     This should be info level which will be consistent with GroupMembershipKeyFilter class.
> 
> Illya Yalovyy wrote:
>     Ok. I'll generate 2 log entries then: 1. INFO without group information; 2. DEBUG with full information. 
>     
>     Does it make sense?
>     
>     See Naveen's comments for more details.
> 
> Illya Yalovyy wrote:
>     Actually this is a bit different. I'll change it to INFO.

Now I'm convinced for having 2 entries, 1. INFO without group information; 2. DEBUG with full information. 

What do you mean it's different?


> On Dec. 8, 2016, 3:55 p.m., Aihua Xu wrote:
> > service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java, line 124
> > <https://reviews.apache.org/r/53204/diff/3/?file=1579541#file1579541line124>
> >
> >     Seems 'warn' is not necessary since that could be expected in the for loop, right?
> 
> Illya Yalovyy wrote:
>     It means we have a group in configuration that doesn't exist... Would you recommend log it at DEBUG level?

I see. So we configure several groups in the configuration and it's possible that we configure some incorrectly which are not in LDAP. The normal logic is to except no NamingException, right? Actually based on that, "warn" is correct level. Sorry, I was misunderstanding the logic. 

      for (String groupId : groupFilter) {
        try {
          String groupDn = ldap.findGroupDn(groupId);
          groupDns.add(groupDn);
        } catch (NamingException e) {
          LOG.debug("Cannot find DN for group " + groupId, e);
        }
      }


> On Dec. 8, 2016, 3:55 p.m., Aihua Xu wrote:
> > service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java, line 132
> > <https://reviews.apache.org/r/53204/diff/3/?file=1579541#file1579541line132>
> >
> >     Since we are throwing the exception, I guess such debug may be redundant. We should display such exception in the caller somewhere.
> 
> Illya Yalovyy wrote:
>     Exception message has a different (less descriptive) message. Please see Naveen's comments for more details.

Got it.


> On Dec. 8, 2016, 3:55 p.m., Aihua Xu wrote:
> > service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java, line 139
> > <https://reviews.apache.org/r/53204/diff/3/?file=1579541#file1579541line139>
> >
> >     Seems this could be a info level message.
> 
> Illya Yalovyy wrote:
>     Same here.

Same as previous comment. Maybe print info level without sensitive info and debug with?


> On Dec. 8, 2016, 3:55 p.m., Aihua Xu wrote:
> > service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java, line 145
> > <https://reviews.apache.org/r/53204/diff/3/?file=1579541#file1579541line145>
> >
> >     You may need to change message since it's expected that the user is not in some groups. Probably change to "Cannot match user ... and group ..." since "Failed to" seems to be an error.
> 
> Illya Yalovyy wrote:
>     I will update the message.
> 
> Illya Yalovyy wrote:
>     Usually it should just return true or false. If it fails with exception then something is wrong. That was reflected in the message. I noticed that I'm hiding the exception, which is a very bad practice. Will fix it as well. May be even WARN log message with exception details is required here. What you think? Again it should not happen usually, if it does - something wrong.

Seems I misunderstood here again. yeah. You are right. WARN with details looks good to me.


> On Dec. 8, 2016, 3:55 p.m., Aihua Xu wrote:
> > service/src/test/org/apache/hive/service/auth/TestLdapAuthenticationProviderImpl.java, line 265
> > <https://reviews.apache.org/r/53204/diff/3/?file=1579546#file1579546line265>
> >
> >     You may need to add some tests for the default configuraiton which is null for HIVE_SERVER2_PLAIN_LDAP_USERMEMBERSHIP_KEY.
> 
> Illya Yalovyy wrote:
>     If HIVE_SERVER2_PLAIN_LDAP_USERMEMBERSHIP_KEY is NULL this filter will not be used. I think we have enough test for this case. Did I get you correctly? Could you please provide more details about the test case you have in mind?
> 
> Illya Yalovyy wrote:
>     I think this use case is tested in #testAuthenticateWhenGroupFilterPasses(). Probably I should rename other tests to make it clear.

Looks good. :)


- Aihua


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53204/#review158532
-----------------------------------------------------------


On Dec. 9, 2016, 1:03 a.m., Illya Yalovyy wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53204/
> -----------------------------------------------------------
> 
> (Updated Dec. 9, 2016, 1:03 a.m.)
> 
> 
> Review request for hive, Aihua Xu, Ashutosh Chauhan, Chaoyu Tang, and Szehon Ho.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-15076 Improve scalability of LDAP authentication provider group filter
> 
> https://issues.apache.org/jira/browse/HIVE-15076
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 5ea9751 
>   service/src/java/org/apache/hive/service/auth/ldap/DirSearch.java 33b6088 
>   service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java 152c4b2 
>   service/src/java/org/apache/hive/service/auth/ldap/LdapSearch.java 65076ea 
>   service/src/java/org/apache/hive/service/auth/ldap/Query.java b8bf938 
>   service/src/java/org/apache/hive/service/auth/ldap/QueryFactory.java e9172d3 
>   service/src/test/org/apache/hive/service/auth/TestLdapAtnProviderWithMiniDS.java cd62935 
>   service/src/test/org/apache/hive/service/auth/TestLdapAuthenticationProviderImpl.java 4fad755 
>   service/src/test/org/apache/hive/service/auth/ldap/LdapAuthenticationTestCase.java acde8c1 
>   service/src/test/org/apache/hive/service/auth/ldap/TestGroupFilter.java 0cc2ead 
>   service/src/test/org/apache/hive/service/auth/ldap/TestLdapSearch.java 499b624 
>   service/src/test/org/apache/hive/service/auth/ldap/TestQueryFactory.java 3054e33 
>   service/src/test/resources/ldap/ad.example.com.ldif PRE-CREATION 
>   service/src/test/resources/ldap/microsoft.schema.ldif PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53204/diff/
> 
> 
> Testing
> -------
> 
> Build succeeded.
> 
> Test results:
> 
> Tests run: 149, Failures: 0, Errors: 0, Skipped: 0
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 03:14 min
> [INFO] Finished at: 2016-10-26T13:53:15-07:00
> [INFO] Final Memory: 36M/1091M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Illya Yalovyy
> 
>


Re: Review Request 53204: HIVE-15076 Improve scalability of LDAP authentication provider group filter

Posted by Aihua Xu <ax...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53204/#review158532
-----------------------------------------------------------




common/src/java/org/apache/hadoop/hive/conf/HiveConf.java (line 2425)
<https://reviews.apache.org/r/53204/#comment229295>

    Just curious why we don't just put the constant string "hive.server2.authentication.ldap.userMembershipKey" here like most of other entries?



service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java (line 90)
<https://reviews.apache.org/r/53204/#comment229296>

    This seems to be a useful info that will help in diagnostics. Wondering why changes from info to debug level?



service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java (line 115)
<https://reviews.apache.org/r/53204/#comment229297>

    This should be info level which will be consistent with GroupMembershipKeyFilter class.



service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java (line 124)
<https://reviews.apache.org/r/53204/#comment229298>

    Seems 'warn' is not necessary since that could be expected in the for loop, right?



service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java (line 132)
<https://reviews.apache.org/r/53204/#comment229299>

    Since we are throwing the exception, I guess such debug may be redundant. We should display such exception in the caller somewhere.



service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java (line 139)
<https://reviews.apache.org/r/53204/#comment229300>

    Seems this could be a info level message.



service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java (line 145)
<https://reviews.apache.org/r/53204/#comment229301>

    You may need to change message since it's expected that the user is not in some groups. Probably change to "Cannot match user ... and group ..." since "Failed to" seems to be an error.



service/src/java/org/apache/hive/service/auth/ldap/QueryFactory.java (line 138)
<https://reviews.apache.org/r/53204/#comment229302>

    Looks like we won't handle NPE so NPE may cause some problems. 
    
    If userMembershipAttr is null, will we still check userMememberOfGroup or not? If not, maybe we should handle such exception here. How about groupMembershipAttr above? Seems we will have such issue as well.



service/src/test/org/apache/hive/service/auth/TestLdapAuthenticationProviderImpl.java (line 265)
<https://reviews.apache.org/r/53204/#comment229303>

    You may need to add some tests for the default configuraiton which is null for HIVE_SERVER2_PLAIN_LDAP_USERMEMBERSHIP_KEY.


- Aihua Xu


On Dec. 8, 2016, 12:45 a.m., Illya Yalovyy wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53204/
> -----------------------------------------------------------
> 
> (Updated Dec. 8, 2016, 12:45 a.m.)
> 
> 
> Review request for hive, Aihua Xu, Ashutosh Chauhan, Chaoyu Tang, and Szehon Ho.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-15076 Improve scalability of LDAP authentication provider group filter
> 
> https://issues.apache.org/jira/browse/HIVE-15076
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 5ea9751 
>   service/src/java/org/apache/hive/service/auth/ldap/DirSearch.java 33b6088 
>   service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java 152c4b2 
>   service/src/java/org/apache/hive/service/auth/ldap/LdapSearch.java 65076ea 
>   service/src/java/org/apache/hive/service/auth/ldap/Query.java b8bf938 
>   service/src/java/org/apache/hive/service/auth/ldap/QueryFactory.java e9172d3 
>   service/src/test/org/apache/hive/service/auth/TestLdapAtnProviderWithMiniDS.java cd62935 
>   service/src/test/org/apache/hive/service/auth/TestLdapAuthenticationProviderImpl.java 4fad755 
>   service/src/test/org/apache/hive/service/auth/ldap/LdapAuthenticationTestCase.java acde8c1 
>   service/src/test/org/apache/hive/service/auth/ldap/TestGroupFilter.java 0cc2ead 
>   service/src/test/org/apache/hive/service/auth/ldap/TestLdapSearch.java 499b624 
>   service/src/test/org/apache/hive/service/auth/ldap/TestQueryFactory.java 3054e33 
>   service/src/test/resources/ldap/ad.example.com.ldif PRE-CREATION 
>   service/src/test/resources/ldap/microsoft.schema.ldif PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53204/diff/
> 
> 
> Testing
> -------
> 
> Build succeeded.
> 
> Test results:
> 
> Tests run: 149, Failures: 0, Errors: 0, Skipped: 0
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 03:14 min
> [INFO] Finished at: 2016-10-26T13:53:15-07:00
> [INFO] Final Memory: 36M/1091M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Illya Yalovyy
> 
>