Uploaded image for project: 'HBase'
  1. HBase
  2. HBASE-19318

MasterRpcServices#getSecurityCapabilities explicitly checks for the HBase AccessController implementation

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.0.0-beta-1
    • Component/s: master, security
    • Labels:
      None
    • Hadoop Flags:
      Reviewed
    • Release Note:
      Fixes an issue with loading customer coprocessor endpoint implementations inside of the HBase Master which breaks Apache Ranger.

      Description

      Sharmadha brought a failure to my attention trying to use Ranger with HBase 2.0 where the grant command was erroring out unexpectedly. The cluster had the Ranger-specific coprocessors deployed, per what was previously working on the HBase 1.1 line.

      After some digging, I found that the the Master is actually making a check explicitly for a Coprocessor that has the name org.apache.hadoop.hbase.security.access.AccessController (short name or full name), instead of looking for a deployed coprocessor which can be assigned to AccessController (which is what Ranger does). We have the CoprocessorHost methods to do the latter already implemented; it strikes me that we just accidentally used the wrong method in MasterRpcServices.

      1. HBASE-19318.001.branch-2.patch
        14 kB
        Josh Elser
      2. HBASE-19318.002.branch-2.patch
        14 kB
        Josh Elser

        Issue Links

          Activity

          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Jenkins build HBase-Trunk_matrix #4129 (See https://builds.apache.org/job/HBase-Trunk_matrix/4129/)
          HBASE-19318 Use the PB service interface as the judge of whether some (elserj: rev 5c1acf4792a7f7b6a5ace11c2fa4d172ede46b4e)

          • (add) hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterCoprocessorServices.java
          • (edit) hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Jenkins build HBase-Trunk_matrix #4129 (See https://builds.apache.org/job/HBase-Trunk_matrix/4129/ ) HBASE-19318 Use the PB service interface as the judge of whether some (elserj: rev 5c1acf4792a7f7b6a5ace11c2fa4d172ede46b4e) (add) hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterCoprocessorServices.java (edit) hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Jenkins build HBase-2.0 #927 (See https://builds.apache.org/job/HBase-2.0/927/)
          HBASE-19318 Use the PB service interface as the judge of whether some (elserj: rev e42d20f8ddd4c27d6138e71ec78d0fbfe59790a4)

          • (edit) hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java
          • (add) hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterCoprocessorServices.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Jenkins build HBase-2.0 #927 (See https://builds.apache.org/job/HBase-2.0/927/ ) HBASE-19318 Use the PB service interface as the judge of whether some (elserj: rev e42d20f8ddd4c27d6138e71ec78d0fbfe59790a4) (edit) hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java (add) hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterCoprocessorServices.java
          Hide
          elserj Josh Elser added a comment -

          Actually, I'm just going to drop the other 1.x fixVersions. I don't have a setup to do testing against those versions and, the fact that no one else has reported this, seems to imply that users of those versions aren't missing this.

          We can consider a backport to those versions if someone can volunteer time/resources to test it.

          Show
          elserj Josh Elser added a comment - Actually, I'm just going to drop the other 1.x fixVersions. I don't have a setup to do testing against those versions and, the fact that no one else has reported this, seems to imply that users of those versions aren't missing this. We can consider a backport to those versions if someone can volunteer time/resources to test it.
          Hide
          elserj Josh Elser added a comment -

          Checkstyle was on import order. Fixed that locally and committing to branch-2 and master.

          Show
          elserj Josh Elser added a comment - Checkstyle was on import order. Fixed that locally and committing to branch-2 and master.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 2m 0s Docker mode activated.
                Prechecks
          0 findbugs 0m 0s Findbugs executables are not available.
          +1 hbaseanti 0m 0s Patch does not have any anti-patterns.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
                branch-2 Compile Tests
          +1 mvninstall 4m 1s branch-2 passed
          +1 compile 0m 43s branch-2 passed
          +1 checkstyle 1m 5s branch-2 passed
          +1 shadedjars 5m 13s branch has no errors when building our shaded downstream artifacts.
          +1 javadoc 0m 29s branch-2 passed
                Patch Compile Tests
          +1 mvninstall 3m 56s the patch passed
          +1 compile 0m 43s the patch passed
          +1 javac 0m 43s the patch passed
          -1 checkstyle 1m 4s hbase-server: The patch generated 2 new + 16 unchanged - 0 fixed = 18 total (was 16)
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 shadedjars 4m 2s patch has no errors when building our shaded downstream artifacts.
          +1 hadoopcheck 42m 49s Patch does not cause any errors with Hadoop 2.6.1 2.6.2 2.6.3 2.6.4 2.6.5 2.7.1 2.7.2 2.7.3 2.7.4 or 3.0.0-alpha4.
          +1 javadoc 0m 27s the patch passed
                Other Tests
          -1 unit 113m 36s hbase-server in the patch failed.
          +1 asflicense 0m 18s The patch does not generate ASF License warnings.
          175m 33s



          Subsystem Report/Notes
          Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hbase:9f2f2db
          JIRA Issue HBASE-19318
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12899190/HBASE-19318.002.branch-2.patch
          Optional Tests asflicense javac javadoc unit findbugs shadedjars hadoopcheck hbaseanti checkstyle compile
          uname Linux 9a59b0d1ccf8 3.13.0-129-generic #178-Ubuntu SMP Fri Aug 11 12:48:20 UTC 2017 x86_64 GNU/Linux
          Build tool maven
          Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build@2/component/dev-support/hbase-personality.sh
          git revision branch-2 / 0406d06533
          maven version: Apache Maven 3.5.2 (138edd61fd100ec658bfa2d307c43b76940a5d7d; 2017-10-18T07:58:13Z)
          Default Java 1.8.0_151
          checkstyle https://builds.apache.org/job/PreCommit-HBASE-Build/10040/artifact/patchprocess/diff-checkstyle-hbase-server.txt
          unit https://builds.apache.org/job/PreCommit-HBASE-Build/10040/artifact/patchprocess/patch-unit-hbase-server.txt
          Test Results https://builds.apache.org/job/PreCommit-HBASE-Build/10040/testReport/
          modules C: hbase-server U: hbase-server
          Console output https://builds.apache.org/job/PreCommit-HBASE-Build/10040/console
          Powered by Apache Yetus 0.6.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 2m 0s Docker mode activated.       Prechecks 0 findbugs 0m 0s Findbugs executables are not available. +1 hbaseanti 0m 0s Patch does not have any anti-patterns. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.       branch-2 Compile Tests +1 mvninstall 4m 1s branch-2 passed +1 compile 0m 43s branch-2 passed +1 checkstyle 1m 5s branch-2 passed +1 shadedjars 5m 13s branch has no errors when building our shaded downstream artifacts. +1 javadoc 0m 29s branch-2 passed       Patch Compile Tests +1 mvninstall 3m 56s the patch passed +1 compile 0m 43s the patch passed +1 javac 0m 43s the patch passed -1 checkstyle 1m 4s hbase-server: The patch generated 2 new + 16 unchanged - 0 fixed = 18 total (was 16) +1 whitespace 0m 0s The patch has no whitespace issues. +1 shadedjars 4m 2s patch has no errors when building our shaded downstream artifacts. +1 hadoopcheck 42m 49s Patch does not cause any errors with Hadoop 2.6.1 2.6.2 2.6.3 2.6.4 2.6.5 2.7.1 2.7.2 2.7.3 2.7.4 or 3.0.0-alpha4. +1 javadoc 0m 27s the patch passed       Other Tests -1 unit 113m 36s hbase-server in the patch failed. +1 asflicense 0m 18s The patch does not generate ASF License warnings. 175m 33s Subsystem Report/Notes Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hbase:9f2f2db JIRA Issue HBASE-19318 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12899190/HBASE-19318.002.branch-2.patch Optional Tests asflicense javac javadoc unit findbugs shadedjars hadoopcheck hbaseanti checkstyle compile uname Linux 9a59b0d1ccf8 3.13.0-129-generic #178-Ubuntu SMP Fri Aug 11 12:48:20 UTC 2017 x86_64 GNU/Linux Build tool maven Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build@2/component/dev-support/hbase-personality.sh git revision branch-2 / 0406d06533 maven version: Apache Maven 3.5.2 (138edd61fd100ec658bfa2d307c43b76940a5d7d; 2017-10-18T07:58:13Z) Default Java 1.8.0_151 checkstyle https://builds.apache.org/job/PreCommit-HBASE-Build/10040/artifact/patchprocess/diff-checkstyle-hbase-server.txt unit https://builds.apache.org/job/PreCommit-HBASE-Build/10040/artifact/patchprocess/patch-unit-hbase-server.txt Test Results https://builds.apache.org/job/PreCommit-HBASE-Build/10040/testReport/ modules C: hbase-server U: hbase-server Console output https://builds.apache.org/job/PreCommit-HBASE-Build/10040/console Powered by Apache Yetus 0.6.0 http://yetus.apache.org This message was automatically generated.
          Hide
          yuzhihong@gmail.com Ted Yu added a comment -

          +1

          Show
          yuzhihong@gmail.com Ted Yu added a comment - +1
          Hide
          elserj Josh Elser added a comment -

          Let me fix the checkstyle issues, and, if I don't hear otherwise, I'll push this come Monday. Thanks for the reviews.

          Just another ping in case people want to give more feedback since QA didn't run automatically. I've kicked it and will wait for that to return.

          Show
          elserj Josh Elser added a comment - Let me fix the checkstyle issues, and, if I don't hear otherwise, I'll push this come Monday. Thanks for the reviews. Just another ping in case people want to give more feedback since QA didn't run automatically. I've kicked it and will wait for that to return.
          Hide
          elserj Josh Elser added a comment -

          .002 Assorted minor improvements:

          • Test classification
          • Javadoc on test class
          • Test renamed
          • Checkstyle import nits
          Show
          elserj Josh Elser added a comment - .002 Assorted minor improvements: Test classification Javadoc on test class Test renamed Checkstyle import nits
          Hide
          elserj Josh Elser added a comment -

          Ya that would be ideal path Josh. Hope u will start that work after 2.0 Will help with that when it comes.

          Thanks, Anoop. I appreciate you working with me here! I'd certainly like to help make this better. I think it will be pretty straightforward, but we'll see

          Let me fix the checkstyle issues, and, if I don't hear otherwise, I'll push this come Monday. Thanks for the reviews.

          Show
          elserj Josh Elser added a comment - Ya that would be ideal path Josh. Hope u will start that work after 2.0 Will help with that when it comes. Thanks, Anoop. I appreciate you working with me here! I'd certainly like to help make this better. I think it will be pretty straightforward, but we'll see Let me fix the checkstyle issues, and, if I don't hear otherwise, I'll push this come Monday. Thanks for the reviews.
          Hide
          anoop.hbase Anoop Sam John added a comment -

          Ya that would be ideal path Josh. Hope u will start that work after 2.0 Will help with that when it comes.
          On the patch itself, as I said above, the change look very much ok. Only questions/concerns were on the general approach by Ranger abt this.. Ya u got the points already. Tks for the explain BTW.

          Show
          anoop.hbase Anoop Sam John added a comment - Ya that would be ideal path Josh. Hope u will start that work after 2.0 Will help with that when it comes. On the patch itself, as I said above, the change look very much ok. Only questions/concerns were on the general approach by Ranger abt this.. Ya u got the points already. Tks for the explain BTW.
          Hide
          elserj Josh Elser added a comment -

          So the intention is to see if the cluster has Accesscontrol services installed and if so Ranger CP will do necessary actions before the ACL CP is invoked, right?

          Right. This is a smell, but something that Ranger has been doing already (I've since learned).

          Is it right to do this on Ranger side?

          No, but I think this is necessary-evil presently.

          To Anoop's point, I think it would be better to separate AccessController from being the coprocessor and the hbase:acl-based authz implementation. With this, we can:

          1. Provide a proper interface for folks to implement custom authz logic
          2. Avoiding unnecessary "system internals" leaking to these impls

          I would guess that we could do this in a backwards compat way (new AC implementation instead of removing the current implementation) and handle it in 2.1.0 rather than waiting for 3.0.

          We should involve Ranger folks though (Velmurugan Periasamy) to make sure that there isn't more that we're missing which they could benefit from. I would guess that they had no idea folks in HBase considered authz to be internal-only

          Show
          elserj Josh Elser added a comment - So the intention is to see if the cluster has Accesscontrol services installed and if so Ranger CP will do necessary actions before the ACL CP is invoked, right? Right. This is a smell, but something that Ranger has been doing already (I've since learned). Is it right to do this on Ranger side? No, but I think this is necessary-evil presently. To Anoop's point, I think it would be better to separate AccessController from being the coprocessor and the hbase:acl-based authz implementation. With this, we can: Provide a proper interface for folks to implement custom authz logic Avoiding unnecessary "system internals" leaking to these impls I would guess that we could do this in a backwards compat way (new AC implementation instead of removing the current implementation) and handle it in 2.1.0 rather than waiting for 3.0. We should involve Ranger folks though ( Velmurugan Periasamy ) to make sure that there isn't more that we're missing which they could benefit from. I would guess that they had no idea folks in HBase considered authz to be internal-only
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          Patch LGTM. So the intention is to see if the cluster has Accesscontrol services installed and if so Ranger CP will do necessary actions before the ACL CP is invoked, right?
          On this

          confirmed for me that hbase:acl does somehow get created with Ranger

          Is it right to do this on Ranger side?

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - Patch LGTM. So the intention is to see if the cluster has Accesscontrol services installed and if so Ranger CP will do necessary actions before the ACL CP is invoked, right? On this confirmed for me that hbase:acl does somehow get created with Ranger Is it right to do this on Ranger side?
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 12s Docker mode activated.
                Prechecks
          0 findbugs 0m 0s Findbugs executables are not available.
          +1 hbaseanti 0m 0s Patch does not have any anti-patterns.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
                branch-2 Compile Tests
          +1 mvninstall 4m 14s branch-2 passed
          +1 compile 0m 48s branch-2 passed
          +1 checkstyle 1m 10s branch-2 passed
          +1 shadedjars 5m 50s branch has no errors when building our shaded downstream artifacts.
          +1 javadoc 0m 30s branch-2 passed
                Patch Compile Tests
          +1 mvninstall 4m 9s the patch passed
          +1 compile 0m 46s the patch passed
          +1 javac 0m 46s the patch passed
          -1 checkstyle 1m 9s hbase-server: The patch generated 2 new + 16 unchanged - 0 fixed = 18 total (was 16)
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 shadedjars 4m 31s patch has no errors when building our shaded downstream artifacts.
          +1 hadoopcheck 46m 26s Patch does not cause any errors with Hadoop 2.6.1 2.6.2 2.6.3 2.6.4 2.6.5 2.7.1 2.7.2 2.7.3 2.7.4 or 3.0.0-alpha4.
          +1 javadoc 0m 32s the patch passed
                Other Tests
          +1 unit 82m 6s hbase-server in the patch passed.
          +1 asflicense 0m 18s The patch does not generate ASF License warnings.
          147m 13s



          Subsystem Report/Notes
          Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hbase:9f2f2db
          JIRA Issue HBASE-19318
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12898946/HBASE-19318.001.branch-2.patch
          Optional Tests asflicense javac javadoc unit findbugs shadedjars hadoopcheck hbaseanti checkstyle compile
          uname Linux 7a6e54b327de 3.13.0-129-generic #178-Ubuntu SMP Fri Aug 11 12:48:20 UTC 2017 x86_64 GNU/Linux
          Build tool maven
          Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/component/dev-support/hbase-personality.sh
          git revision branch-2 / 0ef7a24245
          maven version: Apache Maven 3.5.2 (138edd61fd100ec658bfa2d307c43b76940a5d7d; 2017-10-18T07:58:13Z)
          Default Java 1.8.0_151
          checkstyle https://builds.apache.org/job/PreCommit-HBASE-Build/9981/artifact/patchprocess/diff-checkstyle-hbase-server.txt
          Test Results https://builds.apache.org/job/PreCommit-HBASE-Build/9981/testReport/
          modules C: hbase-server U: hbase-server
          Console output https://builds.apache.org/job/PreCommit-HBASE-Build/9981/console
          Powered by Apache Yetus 0.6.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 12s Docker mode activated.       Prechecks 0 findbugs 0m 0s Findbugs executables are not available. +1 hbaseanti 0m 0s Patch does not have any anti-patterns. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.       branch-2 Compile Tests +1 mvninstall 4m 14s branch-2 passed +1 compile 0m 48s branch-2 passed +1 checkstyle 1m 10s branch-2 passed +1 shadedjars 5m 50s branch has no errors when building our shaded downstream artifacts. +1 javadoc 0m 30s branch-2 passed       Patch Compile Tests +1 mvninstall 4m 9s the patch passed +1 compile 0m 46s the patch passed +1 javac 0m 46s the patch passed -1 checkstyle 1m 9s hbase-server: The patch generated 2 new + 16 unchanged - 0 fixed = 18 total (was 16) +1 whitespace 0m 0s The patch has no whitespace issues. +1 shadedjars 4m 31s patch has no errors when building our shaded downstream artifacts. +1 hadoopcheck 46m 26s Patch does not cause any errors with Hadoop 2.6.1 2.6.2 2.6.3 2.6.4 2.6.5 2.7.1 2.7.2 2.7.3 2.7.4 or 3.0.0-alpha4. +1 javadoc 0m 32s the patch passed       Other Tests +1 unit 82m 6s hbase-server in the patch passed. +1 asflicense 0m 18s The patch does not generate ASF License warnings. 147m 13s Subsystem Report/Notes Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hbase:9f2f2db JIRA Issue HBASE-19318 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12898946/HBASE-19318.001.branch-2.patch Optional Tests asflicense javac javadoc unit findbugs shadedjars hadoopcheck hbaseanti checkstyle compile uname Linux 7a6e54b327de 3.13.0-129-generic #178-Ubuntu SMP Fri Aug 11 12:48:20 UTC 2017 x86_64 GNU/Linux Build tool maven Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/component/dev-support/hbase-personality.sh git revision branch-2 / 0ef7a24245 maven version: Apache Maven 3.5.2 (138edd61fd100ec658bfa2d307c43b76940a5d7d; 2017-10-18T07:58:13Z) Default Java 1.8.0_151 checkstyle https://builds.apache.org/job/PreCommit-HBASE-Build/9981/artifact/patchprocess/diff-checkstyle-hbase-server.txt Test Results https://builds.apache.org/job/PreCommit-HBASE-Build/9981/testReport/ modules C: hbase-server U: hbase-server Console output https://builds.apache.org/job/PreCommit-HBASE-Build/9981/console Powered by Apache Yetus 0.6.0 http://yetus.apache.org This message was automatically generated.
          Hide
          yuzhihong@gmail.com Ted Yu added a comment -

          Please add javadoc and test category to TestMasterRpcServices.
          I think the test class should have better name - it tests coprocessor specific aspects.

          Show
          yuzhihong@gmail.com Ted Yu added a comment - Please add javadoc and test category to TestMasterRpcServices. I think the test class should have better name - it tests coprocessor specific aspects.
          Hide
          stack stack added a comment -

          Anoop Sam John Yeah, we've talked of taking AC internal to hbase. Didn't consider that AC could get outsourced, a facility we want to support.

          Show
          stack stack added a comment - Anoop Sam John Yeah, we've talked of taking AC internal to hbase. Didn't consider that AC could get outsourced, a facility we want to support.
          Hide
          elserj Josh Elser added a comment -

          .001 for branch-2. Haven't tested explicitly on 1.2 or 1.3, but I assume the same issue lives there. However, given that no one has complained yet, maybe no one cares about Ranger integration on 1.2/1.3/1.4 branches..

          Show
          elserj Josh Elser added a comment - .001 for branch-2. Haven't tested explicitly on 1.2 or 1.3, but I assume the same issue lives there. However, given that no one has complained yet, maybe no one cares about Ranger integration on 1.2/1.3/1.4 branches..
          Hide
          elserj Josh Elser added a comment -

          After having to fix two things in the Ranger plugin itself, I was able to verify that my fix addresses the issue. Will rebase and attach shortly.

          Show
          elserj Josh Elser added a comment - After having to fix two things in the Ranger plugin itself, I was able to verify that my fix addresses the issue. Will rebase and attach shortly.
          Hide
          elserj Josh Elser added a comment -

          Well, my current issue is that hbase:acl doesn't exist which means that the region lookup to meta fails to find a RS to send the acl RPC to.

          Looks like this is something that Ranger itself used to do.

          Show
          elserj Josh Elser added a comment - Well, my current issue is that hbase:acl doesn't exist which means that the region lookup to meta fails to find a RS to send the acl RPC to. Looks like this is something that Ranger itself used to do.
          Hide
          elserj Josh Elser added a comment -

          Well, my current issue is that hbase:acl doesn't exist which means that the region lookup to meta fails to find a RS to send the acl RPC to.

          Ted Yu confirmed for me that hbase:acl does somehow get created with Ranger (there's just no data in it). The Ranger CP intercepts all of the calls and sends it to the central Ranger service instead.

          Show
          elserj Josh Elser added a comment - Well, my current issue is that hbase:acl doesn't exist which means that the region lookup to meta fails to find a RS to send the acl RPC to. Ted Yu confirmed for me that hbase:acl does somehow get created with Ranger (there's just no data in it). The Ranger CP intercepts all of the calls and sends it to the central Ranger service instead.
          Hide
          elserj Josh Elser added a comment -

          Ya I dont see we allow plugging in Authorization. AC implemented as CP and so Ranger or any other could do customization.

          Ok, so this is the source of our confusion. I strongly disagree with your assertion. I'm a little worried if this is an opinion that a majority of folks entertains.

          Also I believe the custom ACL impl is dealing with acl table directly

          No, Ranger does not interact with the hbase:acl table at all.

          Sorry am not saying Ranger is unsupported by HBase

          That's the thing though: you are. Your opinions that you're stating here at explicitly stating that you the interface(s) that Ranger is using should not exist.

          They using a non exposed thing now may be because we really not exposing some thing which we must or it is wrong usage from user end. If the former , we have to correct ourselves first. That is the whole point am trying to convey.

          Doing more abstraction at this point to properly enable this "pluggability" is fine (I'm not disagreeing with the goals you are trying to achieve), but we cannot just say "don't do that" when we have no alternative. I don't see that being a trivial change or one we would want to shove into 2.0 at this point.

          Show
          elserj Josh Elser added a comment - Ya I dont see we allow plugging in Authorization. AC implemented as CP and so Ranger or any other could do customization. Ok, so this is the source of our confusion. I strongly disagree with your assertion. I'm a little worried if this is an opinion that a majority of folks entertains. Also I believe the custom ACL impl is dealing with acl table directly No, Ranger does not interact with the hbase:acl table at all. Sorry am not saying Ranger is unsupported by HBase That's the thing though: you are. Your opinions that you're stating here at explicitly stating that you the interface(s) that Ranger is using should not exist. They using a non exposed thing now may be because we really not exposing some thing which we must or it is wrong usage from user end. If the former , we have to correct ourselves first. That is the whole point am trying to convey. Doing more abstraction at this point to properly enable this "pluggability" is fine (I'm not disagreeing with the goals you are trying to achieve), but we cannot just say "don't do that" when we have no alternative. I don't see that being a trivial change or one we would want to shove into 2.0 at this point.
          Hide
          anoop.hbase Anoop Sam John added a comment -

          Ya I dont see we allow plugging in Authorization. AC implemented as CP and so Ranger or any other could do customization. What if tomorrow we remove the CP based authorization and make the impl core to the HBase? (We discussed abt this many a times but as no one could dedicate time for it , never happened). The custom implementation is using the AccessControlService which is not exposed. We can any time change this PB interface no (as per our BC guidelines). Also I believe the custom ACL impl is dealing with acl table directly. So this is like dealing with a system table which is not exposed. Sorry I really dont know the impl details of Ranger. I was just trying to point to a fact of using no exposed things which would create issues for that project later. So you can think and propose how/whether we should be exposing these now for making customization of AC.
          I think ur patch is to use findCoprocessor(Class<T> cls) than findCoprocessor(String className). That looks to be harmless change any way. Sorry am not saying Ranger is unsupported by HBase or any. Just trying to address the bigger picture. We want our users to use exposed things only. They using a non exposed thing now may be because we really not exposing some thing which we must or it is wrong usage from user end. If the former , we have to correct ourselves first. That is the whole point am trying to convey.

          Show
          anoop.hbase Anoop Sam John added a comment - Ya I dont see we allow plugging in Authorization. AC implemented as CP and so Ranger or any other could do customization. What if tomorrow we remove the CP based authorization and make the impl core to the HBase? (We discussed abt this many a times but as no one could dedicate time for it , never happened). The custom implementation is using the AccessControlService which is not exposed. We can any time change this PB interface no (as per our BC guidelines). Also I believe the custom ACL impl is dealing with acl table directly. So this is like dealing with a system table which is not exposed. Sorry I really dont know the impl details of Ranger. I was just trying to point to a fact of using no exposed things which would create issues for that project later. So you can think and propose how/whether we should be exposing these now for making customization of AC. I think ur patch is to use findCoprocessor(Class<T> cls) than findCoprocessor(String className). That looks to be harmless change any way. Sorry am not saying Ranger is unsupported by HBase or any. Just trying to address the bigger picture. We want our users to use exposed things only. They using a non exposed thing now may be because we really not exposing some thing which we must or it is wrong usage from user end. If the former , we have to correct ourselves first. That is the whole point am trying to convey.
          Hide
          elserj Josh Elser added a comment -

          Are we allowing plugging in? Sorry I don't think so. Pls correct me if u find else in some other place. This is the whole point I wanted to bring about.

          I'm not sure how to interpret this other than you saying that you think Ranger is unsupported by HBase. Authz hooks via CP is what Ranger is and Ranger definitely worked with HBase 1.1 (my hunch at this point is that HBASE-14122 broke Ranger integration in all of 1.2 and 1.3.

          I feel like we might be talking past each other. Can you try to clarify what you mean when you say that ayou don't think we allow pluggable authz?

          Show
          elserj Josh Elser added a comment - Are we allowing plugging in? Sorry I don't think so. Pls correct me if u find else in some other place. This is the whole point I wanted to bring about. I'm not sure how to interpret this other than you saying that you think Ranger is unsupported by HBase. Authz hooks via CP is what Ranger is and Ranger definitely worked with HBase 1.1 (my hunch at this point is that HBASE-14122 broke Ranger integration in all of 1.2 and 1.3. I feel like we might be talking past each other. Can you try to clarify what you mean when you say that ayou don't think we allow pluggable authz?
          Hide
          anoop.hbase Anoop Sam John added a comment -

          I agree that some changes broke Ranger.

          I'm really confused by your suggestion that authz is not meant to be pluggable inside of HBase.

          Are we allowing plugging in? Sorry I don't think so. Pls correct me if u find else in some other place. This is the whole point I wanted to bring about.

          Show
          anoop.hbase Anoop Sam John added a comment - I agree that some changes broke Ranger. I'm really confused by your suggestion that authz is not meant to be pluggable inside of HBase. Are we allowing plugging in? Sorry I don't think so. Pls correct me if u find else in some other place. This is the whole point I wanted to bring about.
          Hide
          elserj Josh Elser added a comment -

          Clients will run a check against the master to determine the security capabilities and fails outright because it thinks HBase doesn't have any authorization support (because the AccessController implementation is missing). I'm really confused by your suggestion that authz is not meant to be pluggable inside of HBase.

          Let me try to paint a better picture of how I see this intended to work:

          Clients are ultimately invoking API on AccessControlClient which is sending some RPC to a CP endpoint. HBase shouldn't care what the implementation of that Endpoint is, just that one exists such that the RPC can be handled by it.

          I deployed my patch on a test cluster, but I'm seeing some runtime issues with it. Need to dig further before a patch. Happy to entertain discussion meanwhile though!

          Show
          elserj Josh Elser added a comment - Clients will run a check against the master to determine the security capabilities and fails outright because it thinks HBase doesn't have any authorization support (because the AccessController implementation is missing). I'm really confused by your suggestion that authz is not meant to be pluggable inside of HBase. Let me try to paint a better picture of how I see this intended to work: Clients are ultimately invoking API on AccessControlClient which is sending some RPC to a CP endpoint. HBase shouldn't care what the implementation of that Endpoint is, just that one exists such that the RPC can be handled by it. I deployed my patch on a test cluster, but I'm seeing some runtime issues with it. Need to dig further before a patch. Happy to entertain discussion meanwhile though!
          Hide
          elserj Josh Elser added a comment -

          I'm struggling to follow, Anoop Sam John

          Is that really advisable? ACL is a core part. Just because it is implemented as CP, this is possible now.

          This is nothing new. Ranger has been doing this for years. Why the "possible now" comment? As it stands now, HBase has broken Ranger functionality.

          Clients will run a check against the master to determine the security capabilities and fails outright because it thinks HBase doesn't have any authorization support (because the AccessController implementation is missing). I'm really confused by your suggestion that authz is not meant to be pluggable inside of HBase.

          Show
          elserj Josh Elser added a comment - I'm struggling to follow, Anoop Sam John Is that really advisable? ACL is a core part. Just because it is implemented as CP, this is possible now. This is nothing new. Ranger has been doing this for years. Why the "possible now" comment? As it stands now, HBase has broken Ranger functionality. Clients will run a check against the master to determine the security capabilities and fails outright because it thinks HBase doesn't have any authorization support (because the AccessController implementation is missing). I'm really confused by your suggestion that authz is not meant to be pluggable inside of HBase.
          Hide
          anoop.hbase Anoop Sam John added a comment -

          Is that really advisable? ACL is a core part. Just because it is implemented as CP, this is possible now. So the own implementation working on the system table ACL. Am still not sure is this really correct. Neither this ACL table nor the ACL proto things are exposed to users.

          Show
          anoop.hbase Anoop Sam John added a comment - Is that really advisable? ACL is a core part. Just because it is implemented as CP, this is possible now. So the own implementation working on the system table ACL. Am still not sure is this really correct. Neither this ACL table nor the ACL proto things are exposed to users.
          Hide
          elserj Josh Elser added a comment -

          Ranger is extending our AccessController

          No, they don't. They're just implementing AccessControlService.Interface. I think this is just a simple mistake of assuming that the only implementation of that endpoint would be AccessController

          Should have a patch up within the hour (famous last words though..)

          Show
          elserj Josh Elser added a comment - Ranger is extending our AccessController No, they don't. They're just implementing AccessControlService.Interface . I think this is just a simple mistake of assuming that the only implementation of that endpoint would be AccessController Should have a patch up within the hour (famous last words though..)
          Hide
          anoop.hbase Anoop Sam John added a comment - - edited

          Ranger is extending our AccessController? Is that allowed? !! I dont see AC is exposed to CPs. It is LP but just for Conig. Means we expose the name as such for users to config in the xml.

          Show
          anoop.hbase Anoop Sam John added a comment - - edited Ranger is extending our AccessController? Is that allowed? !! I dont see AC is exposed to CPs. It is LP but just for Conig. Means we expose the name as such for users to config in the xml.

            People

            • Assignee:
              elserj Josh Elser
              Reporter:
              ssainath Sharmadha Sainath
            • Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development