[ https://issues.apache.org/jira/browse/HBASE-5732?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13271642#comment-13271642 ]
jirapos...@reviews.apache.org commented on HBASE-5732: ------------------------------------------------------ ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4953/#review7738 ----------------------------------------------------------- Mostly +1. A couple of minor comments, one question on API annotation. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseClient.java <https://reviews.apache.org/r/4953/#comment17034> Do we have a testcase for relogin? 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. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java <https://reviews.apache.org/r/4953/#comment17035> Debug logging should be wrapped in a conditional, no biggie. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java <https://reviews.apache.org/r/4953/#comment17033> This is probably just going to add noise. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/security/HBasePolicyProvider.java <https://reviews.apache.org/r/4953/#comment17036> Maybe should be "security.hbase.*", what do you think? http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/security/HBaseSaslRpcClient.java <https://reviews.apache.org/r/4953/#comment17037> It might be better to say 'Kerberos principal does not have the expected format' http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/security/User.java <https://reviews.apache.org/r/4953/#comment17038> Should this be Public+Evolving? http://svn.apache.org/repos/asf/hbase/trunk/src/test/resources/hbase-site.xml <https://reviews.apache.org/r/4953/#comment17040> Why this change? Unrelated junk from other work? - Andrew On 2012-05-08 21:48:09, 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-08 21:48:09) 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/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