[ 
https://issues.apache.org/jira/browse/HBASE-5732?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13271919#comment-13271919
 ] 

jirapos...@reviews.apache.org commented on HBASE-5732:
------------------------------------------------------



bq.  On 2012-05-09 18:02:12, Andrew Purtell wrote:
bq.  > Mostly +1. A couple of minor comments, one question on API annotation.

Thanks, Andrew. 


bq.  On 2012-05-09 18:02:12, Andrew Purtell wrote:
bq.  > 
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseClient.java,
 line 677
bq.  > <https://reviews.apache.org/r/4953/diff/4/?file=107883#file107883line677>
bq.  >
bq.  >     Do we have a testcase for relogin?
bq.  >     
bq.  >     In our production with the older implementation of secure HBase RPC, 
we see swarms of GSS initiaition failure due to missing TGT for 5-10 seconds, 
and we speculate this is a race around relogin.

It's not straightforward to write a testcase for the security APIs, 
unfortunately. I'd propose we open a jira for the relogin issue (over in 
hadoop, iirc, we've fixed some issues to do with relogin races in the recent 
past; we should put the fixes here)...


bq.  On 2012-05-09 18:02:12, Andrew Purtell wrote:
bq.  > 
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java,
 line 1328
bq.  > 
<https://reviews.apache.org/r/4953/diff/4/?file=107884#file107884line1328>
bq.  >
bq.  >     Debug logging should be wrapped in a conditional, no biggie.

Done.


bq.  On 2012-05-09 18:02:12, Andrew Purtell wrote:
bq.  > 
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java,
 line 1937
bq.  > 
<https://reviews.apache.org/r/4953/diff/4/?file=107884#file107884line1937>
bq.  >
bq.  >     This is probably just going to add noise.

Removed.


bq.  On 2012-05-09 18:02:12, Andrew Purtell wrote:
bq.  > 
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/security/HBasePolicyProvider.java,
 line 35
bq.  > <https://reviews.apache.org/r/4953/diff/4/?file=107889#file107889line35>
bq.  >
bq.  >     Maybe should be "security.hbase.*", what do you think?

This is how it is in the current trunk.  But yeah, makes sense to have the key 
as "security.hbase.*".


bq.  On 2012-05-09 18:02:12, Andrew Purtell wrote:
bq.  > 
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/security/HBaseSaslRpcClient.java,
 line 94
bq.  > <https://reviews.apache.org/r/4953/diff/4/?file=107890#file107890line94>
bq.  >
bq.  >     It might be better to say 'Kerberos principal does not have the 
expected format'

Done.


bq.  On 2012-05-09 18:02:12, Andrew Purtell wrote:
bq.  > 
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/security/User.java,
 line 53
bq.  > <https://reviews.apache.org/r/4953/diff/4/?file=107892#file107892line53>
bq.  >
bq.  >     Should this be Public+Evolving?

Done. Although I was wondering whether it makes sense to have it as 
Private+Evolving. But if it is currently used by apps outside the core of 
hbase, it makes sense to have it as Public+Evolving..


bq.  On 2012-05-09 18:02:12, Andrew Purtell wrote:
bq.  > 
http://svn.apache.org/repos/asf/hbase/trunk/src/test/resources/hbase-site.xml, 
line 126
bq.  > <https://reviews.apache.org/r/4953/diff/4/?file=107918#file107918line126>
bq.  >
bq.  >     Why this change? Unrelated junk from other work?

In the hbase-site.xml of security/src/test/resources, this block of 
configuration was there. Now that there is a single profile, I thought I should 
not lose this and added this in the src/test/resources/hbase-site.xml..


- Devaraj


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


On 2012-05-09 23:03:45, Devaraj Das wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/4953/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2012-05-09 23:03:45)
bq.  
bq.  
bq.  Review request for Ted Yu, Michael Stack and Andrew Purtell.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  Reviewboard request for HBASE-5732
bq.  
bq.  
bq.  This addresses bug HBASE-5732.
bq.      https://issues.apache.org/jira/browse/HBASE-5732
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    http://svn.apache.org/repos/asf/hbase/trunk/conf/hbase-policy.xml 
1335370 
bq.    http://svn.apache.org/repos/asf/hbase/trunk/pom.xml 1335370 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/AdminProtocol.java
 1335370 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/ClientProtocol.java
 1335370 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/ConnectionHeader.java
 1335370 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseClient.java
 1335370 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java
 1335370 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/RegionServerStatusProtocol.java
 1335370 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java
 1335370 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated/RPCProtos.java
 1335370 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/security/AccessDeniedException.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/security/HBasePolicyProvider.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/security/HBaseSaslRpcClient.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/security/HBaseSaslRpcServer.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/security/User.java
 1335370 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlFilter.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlLists.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/security/access/AccessControllerProtocol.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/security/access/Permission.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/security/access/TableAuthManager.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/security/access/TablePermission.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/security/access/UserPermission.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/security/access/ZKPermissionWatcher.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationKey.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationProtocol.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenIdentifier.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSelector.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/security/token/TokenProvider.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/security/token/TokenUtil.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/security/token/ZKSecretWatcher.java
 PRE-CREATION 
bq.    http://svn.apache.org/repos/asf/hbase/trunk/src/main/protobuf/RPC.proto 
1335370 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/security/access/SecureTestUtil.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessControlFilter.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/security/access/TestTablePermissions.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/security/access/TestZKPermissionsWatcher.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/security/token/TestTokenAuthentication.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/security/token/TestZKSecretWatcher.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/hbase/trunk/src/test/resources/hbase-site.xml 
1335370 
bq.  
bq.  Diff: https://reviews.apache.org/r/4953/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  All unit tests pass.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Devaraj
bq.  
bq.


                
> Remove the SecureRPCEngine and merge the security-related logic in the core 
> engine
> ----------------------------------------------------------------------------------
>
>                 Key: HBASE-5732
>                 URL: https://issues.apache.org/jira/browse/HBASE-5732
>             Project: HBase
>          Issue Type: Improvement
>            Reporter: Devaraj Das
>            Assignee: Devaraj Das
>         Attachments: 5732-rpcengine-merge.7.patch, rpcengine-merge.3.patch, 
> rpcengine-merge.4.patch, rpcengine-merge.9.patch, rpcengine-merge.patch
>
>
> Remove the SecureRPCEngine and merge the security-related logic in the core 
> engine. Follow up to HBASE-5727.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Reply via email to