[
https://issues.apache.org/jira/browse/HBASE-5732?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13270295#comment-13270295
]
jirapos...@reviews.apache.org commented on HBASE-5732:
------------------------------------------------------
bq. On 2012-05-01 23:02:34, Michael Stack wrote:
bq. > Here's a quick review of this fat patch. Good stuff.
Yeah the patch is fat but most of it is refactoring and moving classes around.
bq. On 2012-05-01 23:02:34, Michael Stack wrote:
bq. >
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseClient.java,
line 67
bq. > <https://reviews.apache.org/r/4953/diff/1/?file=105837#file105837line67>
bq. >
bq. > Should this security stuff be moved down here into ipc package? Is
it only place where security is referenced?
This is not the only class.. there are a bunch of classes present there..
bq. On 2012-05-01 23:02:34, Michael Stack wrote:
bq. > http://svn.apache.org/repos/asf/hbase/trunk/pom.xml, line 1677
bq. >
<https://reviews.apache.org/r/4953/diff/1/?file=105835#file105835line1677>
bq. >
bq. > Now the underlying hadoop must support all the security apis?
bq. >
bq. > If not present, will we compile?
Yes, this will necessitate hadoop-1.0++..
bq. On 2012-05-01 23:02:34, Michael Stack wrote:
bq. >
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseClient.java,
line 81
bq. > <https://reviews.apache.org/r/4953/diff/1/?file=105837#file105837line81>
bq. >
bq. > So, if underlying hadoop does not have these classes, we'll fail the
build?
yes
bq. On 2012-05-01 23:02:34, Michael Stack wrote:
bq. >
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseClient.java,
line 283
bq. > <https://reviews.apache.org/r/4953/diff/1/?file=105837#file105837line283>
bq. >
bq. > Should this be a fail?
This was what was there in the original code (SecureClient.java). I left it as
it was..
bq. On 2012-05-01 23:02:34, Michael Stack wrote:
bq. >
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java,
line 1495
bq. >
<https://reviews.apache.org/r/4953/diff/1/?file=105838#file105838line1495>
bq. >
bq. > Can you not give byte array to pb to parse? Use builder and
mergeFrom? Not important.
Yeah this is not making any additional copy.
bq. On 2012-05-01 23:02:34, Michael Stack wrote:
bq. >
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java,
line 67
bq. > <https://reviews.apache.org/r/4953/diff/1/?file=105840#file105840line67>
bq. >
bq. > Has to be public?
No.. I reverted this change.
bq. On 2012-05-01 23:02:34, Michael Stack wrote:
bq. >
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java,
line 252
bq. > <https://reviews.apache.org/r/4953/diff/1/?file=105840#file105840line252>
bq. >
bq. > This is a pity removing the static?
Reverted.
bq. On 2012-05-01 23:02:34, Michael Stack wrote:
bq. >
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java,
line 343
bq. > <https://reviews.apache.org/r/4953/diff/1/?file=105840#file105840line343>
bq. >
bq. > Where do these get shut down?
These are daemon threads (most of them are the RPC threads that we currently
have). They will get shut down with the process.
bq. On 2012-05-01 23:02:34, Michael Stack wrote:
bq. >
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/security/access/UserPermission.java,
line 138
bq. > <https://reviews.apache.org/r/4953/diff/1/?file=105854#file105854line138>
bq. >
bq. > This stuff is copied over from the /security dir in hbase? Are
there corresponding deletes? What about some tests?
The patch has the deleted files marked as such. An example is:
Index:
security/src/test/java/org/apache/hadoop/hbase/security/token/TestTokenAuthentication.java
(deleted)
I haven't added any tests as part of this patch since this is mostly
refactoring. Existing tests cover the scenarios. Also, I am manually testing
the Kerberos parts.
- Devaraj
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4953/#review7455
-----------------------------------------------------------
On 2012-05-01 20:27:30, 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-01 20:27:30)
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/src/main/java/org/apache/hadoop/hbase/ipc/HBaseClient.java
1332383
bq.
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/ConnectionHeader.java
1332383
bq. http://svn.apache.org/repos/asf/hbase/trunk/pom.xml 1332383
bq.
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java
1332383
bq.
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/Status.java
1332383
bq.
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java
1332383
bq.
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated/RPCProtos.java
1332383
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
1332383
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
1332383
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
1332383
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: rpcengine-merge.3.patch, rpcengine-merge.4.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
