HBase
  1. HBase
  2. HBASE-5305 Improve cross-version compatibility & upgradeability
  3. HBASE-6887

Convert security-related shell commands to use PB-based AccessControlService

    Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.95.2
    • Fix Version/s: 0.95.0
    • Component/s: security
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      The security-related HBase shell commands (grant, revoke, user_permission) are still using the old CoprocessorProtocol-based AccessControllerProtocol endpoint for dynamic RPC. These need to be converted to use the protocol buffer based AccessControlService interface added in HBASE-5448.

        Issue Links

          Activity

          Hide
          stack added a comment -

          Marking closed.

          Show
          stack added a comment - Marking closed.
          Hide
          Hudson added a comment -

          Integrated in HBase-TRUNK-on-Hadoop-2.0.0 #304 (See https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-2.0.0/304/)
          HBASE-6887 Convert security-related shell commands to use PB-based AccessControlService (Revision 1423965)

          Result = FAILURE
          jxiang :
          Files :

          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessControllerProtocol.java
          • /hbase/trunk/hbase-server/src/main/ruby/hbase/security.rb
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessControlFilter.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
          Show
          Hudson added a comment - Integrated in HBase-TRUNK-on-Hadoop-2.0.0 #304 (See https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-2.0.0/304/ ) HBASE-6887 Convert security-related shell commands to use PB-based AccessControlService (Revision 1423965) Result = FAILURE jxiang : Files : /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessControllerProtocol.java /hbase/trunk/hbase-server/src/main/ruby/hbase/security.rb /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessControlFilter.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
          Hide
          Hudson added a comment -

          Integrated in HBase-TRUNK #3639 (See https://builds.apache.org/job/HBase-TRUNK/3639/)
          HBASE-6887 Convert security-related shell commands to use PB-based AccessControlService (Revision 1423965)

          Result = FAILURE
          jxiang :
          Files :

          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessControllerProtocol.java
          • /hbase/trunk/hbase-server/src/main/ruby/hbase/security.rb
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessControlFilter.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
          Show
          Hudson added a comment - Integrated in HBase-TRUNK #3639 (See https://builds.apache.org/job/HBase-TRUNK/3639/ ) HBASE-6887 Convert security-related shell commands to use PB-based AccessControlService (Revision 1423965) Result = FAILURE jxiang : Files : /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessControllerProtocol.java /hbase/trunk/hbase-server/src/main/ruby/hbase/security.rb /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessControlFilter.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
          Hide
          Jimmy Xiang added a comment -

          Integrated into trunk. Thanks all for the review.

          Show
          Jimmy Xiang added a comment - Integrated into trunk. Thanks all for the review.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12561576/trunk-6887_v2.1.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 6 new or modified tests.

          +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          -1 findbugs. The patch appears to introduce 28 new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed these unit tests:
          org.apache.hadoop.hbase.replication.TestReplication

          -1 core zombie tests. There are zombie tests. See build logs for details.

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/3598//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3598//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3598//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3598//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3598//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3598//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3598//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3598//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/3598//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12561576/trunk-6887_v2.1.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 6 new or modified tests. +1 hadoop2.0 . The patch compiles against the hadoop 2.0 profile. +1 javadoc . The javadoc tool did not generate any warning messages. +1 javac . The applied patch does not increase the total number of javac compiler warnings. -1 findbugs . The patch appears to introduce 28 new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 core tests . The patch failed these unit tests: org.apache.hadoop.hbase.replication.TestReplication -1 core zombie tests . There are zombie tests. See build logs for details. Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/3598//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3598//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3598//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3598//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3598//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3598//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3598//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3598//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/3598//console This message is automatically generated.
          Hide
          Jimmy Xiang added a comment -

          I posted a patch on RB: https://reviews.apache.org/r/8652/

          Show
          Jimmy Xiang added a comment - I posted a patch on RB: https://reviews.apache.org/r/8652/
          Hide
          Jimmy Xiang added a comment -

          Also filed HBASE-7373. Currently, from hbase shell, user_permission doesn't work if table is not specified.

          Show
          Jimmy Xiang added a comment - Also filed HBASE-7373 . Currently, from hbase shell, user_permission doesn't work if table is not specified.
          Hide
          Jimmy Xiang added a comment -

          I am going to remove AccessControllerProtocol.java in this patch too.

          Show
          Jimmy Xiang added a comment - I am going to remove AccessControllerProtocol.java in this patch too.
          Hide
          stack added a comment -

          Smile. Thanks Gary.

          Show
          stack added a comment - Smile. Thanks Gary.
          Hide
          Gary Helmling added a comment -

          Stack Yeah, not too much to it. We just need to convert the functions in hbase-server/src/main/ruby/hbase/security.rb from doing CoprocessorProtocol based endpoint calls to the PB service calls.

          So, for example, for grant(), where it currently does:

          meta_table = org.apache.hadoop.hbase.client.HTable.new(@config,
                           org.apache.hadoop.hbase.security.access.AccessControlLists::ACL_TABLE_NAME)
          protocol = meta_table.coprocessorProxy(
                           org.apache.hadoop.hbase.security.access.AccessControllerProtocol.java_class,
                                                 org.apache.hadoop.hbase.HConstants::EMPTY_START_ROW)
          begin
            protocol.grant(user_permission)
          

          this would need to become something like:

          meta_table = org.apache.hadoop.hbase.client.HTable.new(@config,
                          org.apache.hadoop.hbase.security.access.AccessControlLists::ACL_TABLE_NAME)
          channel = meta_table.coprocessorService(org.apache.hadoop.hbase.HConstants::EMPTY_START_ROW)
          service = AccessControlProtos.AccessControlService.newBlockingStub(channel)
          # build the AccessControlProtos.GrantRequest, see TestAccessController.newGrantRequest()
          
          begin
            service.grant(nil, request)
          

          See TestAccessController for sample usage, especially the grant() and newGrantRequest() methods there. Then curse me for converting endpoints to PB services .

          Show
          Gary Helmling added a comment - Stack Yeah, not too much to it. We just need to convert the functions in hbase-server/src/main/ruby/hbase/security.rb from doing CoprocessorProtocol based endpoint calls to the PB service calls. So, for example, for grant() , where it currently does: meta_table = org.apache.hadoop.hbase.client.HTable.new(@config, org.apache.hadoop.hbase.security.access.AccessControlLists::ACL_TABLE_NAME) protocol = meta_table.coprocessorProxy( org.apache.hadoop.hbase.security.access.AccessControllerProtocol.java_class, org.apache.hadoop.hbase.HConstants::EMPTY_START_ROW) begin protocol.grant(user_permission) this would need to become something like: meta_table = org.apache.hadoop.hbase.client.HTable.new(@config, org.apache.hadoop.hbase.security.access.AccessControlLists::ACL_TABLE_NAME) channel = meta_table.coprocessorService(org.apache.hadoop.hbase.HConstants::EMPTY_START_ROW) service = AccessControlProtos.AccessControlService.newBlockingStub(channel) # build the AccessControlProtos.GrantRequest, see TestAccessController.newGrantRequest() begin service.grant(nil, request) See TestAccessController for sample usage, especially the grant() and newGrantRequest() methods there. Then curse me for converting endpoints to PB services .
          Hide
          stack added a comment -

          Gary Helmling Any chance of an outline of what needs to be done so I can give it a go? Good on you.

          Show
          stack added a comment - Gary Helmling Any chance of an outline of what needs to be done so I can give it a go? Good on you.
          Hide
          Gary Helmling added a comment -

          Yes, this would be a blocker. Or rather it would block the removal of CoprocessorProtocol, which would be a blocker. I'll open a separate issue for CoprocessorProtocol removal and link in the relevant subtasks.

          Show
          Gary Helmling added a comment - Yes, this would be a blocker. Or rather it would block the removal of CoprocessorProtocol, which would be a blocker. I'll open a separate issue for CoprocessorProtocol removal and link in the relevant subtasks.
          Hide
          Andrew Purtell added a comment -

          This is going to be a blocker right? Maybe we should reorganize removal of CoprocessorProtocol under an umbrella and make it a blocker?

          Show
          Andrew Purtell added a comment - This is going to be a blocker right? Maybe we should reorganize removal of CoprocessorProtocol under an umbrella and make it a blocker?

            People

            • Assignee:
              Jimmy Xiang
              Reporter:
              Gary Helmling
            • Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development