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

2016-12-28 Thread Aihua Xu 

---
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

2016-12-12 Thread Illya Yalovyy 

---
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

2016-12-12 Thread Illya Yalovyy 


> On Dec. 8, 2016, 3:55 p.m., Aihua Xu wrote:
> > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java, line 2426
> > 
> >
> > 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
> > 
> >
> > 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

2016-12-09 Thread Aihua Xu 


> On Dec. 8, 2016, 3:55 p.m., Aihua Xu wrote:
> > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java, line 2426
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > You may need to change message since it's expected that the user is not 
> > in some groups. Probably change

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

2016-12-08 Thread Illya Yalovyy 

---
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

2016-12-08 Thread Illya Yalovyy 


> On Dec. 8, 2016, 3:55 p.m., Aihua Xu wrote:
> > service/src/test/org/apache/hive/service/auth/TestLdapAuthenticationProviderImpl.java,
> >  line 265
> > 
> >
> > 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

2016-12-08 Thread Illya Yalovyy 


> On Dec. 8, 2016, 3:55 p.m., Aihua Xu wrote:
> > service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java, 
> > line 145
> > 
> >
> > 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

2016-12-08 Thread Illya Yalovyy 


> On Dec. 8, 2016, 3:55 p.m., Aihua Xu wrote:
> > service/src/java/org/apache/hive/service/auth/ldap/QueryFactory.java, line 
> > 138
> > 
> >
> > 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
> > 
> >
> > 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

2016-12-08 Thread Illya Yalovyy 


> On Dec. 8, 2016, 3:55 p.m., Aihua Xu wrote:
> > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java, line 2426
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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

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

2016-12-08 Thread Aihua Xu 

---
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)


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)


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)


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)


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)


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)


Seems this could be a info level message.



service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java 
(line 145)


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)


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)


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:

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

2016-12-07 Thread Illya Yalovyy 

---
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

2016-11-28 Thread Illya Yalovyy 

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




service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java 
(line 90)


This is a DEBUG level logging. It will not happen in production 
environment. The purpose of this log message is to help troubleshoot problems. 
Without name and group it makes little sense. I would prefer to keep it as is.



service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java 
(line 124)


I will remove sensitive information from all error messages and exceptions. 
Thank you for suggesting it.

I will move them to a debug logs instead. It will keep security and 
maintainability at high level.



service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java 
(line 138)


This is a DEBUG level logging. It will not happen in production 
environment. The purpose of this log message is to help troubleshoot problems. 
Without name and group it makes little sense. I would prefer to keep it as is.



service/src/test/org/apache/hive/service/auth/TestLdapAuthenticationProviderImpl.java
 (line 261)


I don't think it is the best practice to add javadoc to unit test methods. 
Usually meaningful method names are used instead. If you look at hive code 
base, it is not very common as well. Only few unit tests have javadoc, some of 
them already obsolete and misleading. I would prefer keep it as is.



service/src/test/org/apache/hive/service/auth/ldap/TestGroupFilter.java (line 
32)


Hamcrest module is a part of JUnit. This module is designed to be used in 
conjunction with junit/mockito. I see no reason to break this best practice.


- Illya Yalovyy


On Oct. 28, 2016, 12:59 p.m., Illya Yalovyy wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53204/
> ---
> 
> (Updated Oct. 28, 2016, 12:59 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

2016-11-28 Thread Naveen Gangam 

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




service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java 
(line 90)


Its not a good idea to log the name of the group the user belongs to 
whether it succeeds or fails. So perhaps just log the fact that authentication 
succeeded based on group filter?



service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java 
(line 124)


Remove group name from the log record.



service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java 
(line 130)


anonymize the log record, should have no indication of the configuration



service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java 
(line 138)


Same as above. Remove the group name from the log record.



service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java 
(line 143)


same here



service/src/test/org/apache/hive/service/auth/TestLdapAuthenticationProviderImpl.java
 (line 261)


Please add javadoc comments as to what this test is supposed to test and 
how its being accomplished.



service/src/test/org/apache/hive/service/auth/TestLdapAuthenticationProviderImpl.java
 (line 282)


add javadoc comments for the test



service/src/test/org/apache/hive/service/auth/TestLdapAuthenticationProviderImpl.java
 (line 299)


Add javadoc comments for the test



service/src/test/org/apache/hive/service/auth/ldap/TestGroupFilter.java (line 
32)


can the usage of hamcrest APIs be replaced by JDK regex ?



service/src/test/org/apache/hive/service/auth/ldap/TestGroupFilter.java (line 
108)


Add javadocs



service/src/test/org/apache/hive/service/auth/ldap/TestGroupFilter.java (line 
119)


Add javadoc comments for the test



service/src/test/org/apache/hive/service/auth/ldap/TestGroupFilter.java (line 
134)


Add javadoc comments for the test



service/src/test/org/apache/hive/service/auth/ldap/TestGroupFilter.java (line 
149)


Add javadoc comments for the test.



service/src/test/org/apache/hive/service/auth/ldap/TestLdapSearch.java (line 
212)


Add javadoc comments for the test.



service/src/test/org/apache/hive/service/auth/ldap/TestLdapSearch.java (line 
225)


Add javadoc comments for the test.



service/src/test/org/apache/hive/service/auth/ldap/TestLdapSearch.java (line 
235)


Add javadoc comments for the test



service/src/test/org/apache/hive/service/auth/ldap/TestLdapSearch.java (line 
246)


Add javadoc comments for the test.



service/src/test/org/apache/hive/service/auth/ldap/TestLdapSearch.java (line 
264)


Add javadoc comments for the test.



service/src/test/org/apache/hive/service/auth/ldap/TestLdapSearch.java (line 
279)


Add javadoc comments for the test.



service/src/test/org/apache/hive/service/auth/ldap/TestQueryFactory.java (line 
81)


Add javadoc comments for the test.



service/src/test/org/apache/hive/service/auth/ldap/TestQueryFactory.java (line 
90)


Add javadoc comments for the test.



service/src/test/org/apache/hive/service/auth/ldap/TestQueryFactory.java (line 
96)


Add javadoc comments for the test.


- Naveen Gangam


On Oct. 28, 2016, 12:59 p.m., Illya Yalovyy wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53204/
> ---
> 
> (Updated Oct. 28, 2016, 12:59 p.m.)
> 
> 
> Review request for hive, Aihua Xu, Ashutosh Chauhan, Chaoyu Tang, and Szehon 
> Ho.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-15076 Improve scalability of

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

2016-10-28 Thread Illya Yalovyy 

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

(Updated Oct. 28, 2016, 12:59 p.m.)


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


Changes
---

1. Removed trailing spaces.


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

2016-10-27 Thread Illya Yalovyy 

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

(Updated Oct. 27, 2016, 1:45 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 (updated)
---

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

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

2016-10-27 Thread Illya Yalovyy 

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

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
---

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