Grant/Revoke global permissions

Details

Description

HBASE-3025 introduced simple ACLs based on coprocessors. It defines global/table/cf/cq level permissions. However, there is no way to grant/revoke global level permissions, other than the hbase.superuser conf setting.

Gary Helmling
added a comment - 07/Feb/12 05:43 Some of the building blocks for this are already in place. It shouldn't be too difficult to fill in the missing pieces. Would be great to see this completed.

@Matteo, I do not plan to work on this in the near future, feel free to take a shot. As Gary mentioned, there is already the infrastructure to manage and distribute ACL changes to region servers. I think for this, we should just reuse those. For the hbase shell, we just need to make table argument optional, and change the AccessControlProtocol.grant()/revoke() methods to accept Permission objects rather than TablePermission objects.

Enis Soztutar
added a comment - 25/Apr/12 00:48 @Matteo, I do not plan to work on this in the near future, feel free to take a shot. As Gary mentioned, there is already the infrastructure to manage and distribute ACL changes to region servers. I think for this, we should just reuse those. For the hbase shell, we just need to make table argument optional, and change the AccessControlProtocol.grant()/revoke() methods to accept Permission objects rather than TablePermission objects.

I've attached a first draft, if someone want start to reviewing it.
I still have to add and fix unit tests, and add some comments in hbase shell and some other parts of the code.
Another part that is missing is HBASE-5385 that I'm going to implement once this is done.
but any feedback is apreciated.

Matteo Bertozzi
added a comment - 28/Apr/12 19:46 I've attached a first draft, if someone want start to reviewing it.
I still have to add and fix unit tests, and add some comments in hbase shell and some other parts of the code.
Another part that is missing is HBASE-5385 that I'm going to implement once this is done.
but any feedback is apreciated.

Matteo Bertozzi
added a comment - 29/Apr/12 05:36 @Zhihong sorry I forgot to say that I'm coding it against 0.92, since is a bit more stable/testable than trunk. But the patch should apply fine to 0.94 too. I'll port it to trunk once is done.

Matteo Bertozzi
added a comment - 29/Apr/12 11:46 The only difference between 0.92/0.94 and trunk is this commit:
https://github.com/apache/hbase/commit/19167af652aeb14979146c7bf312cf5925717190
So if we backport in 0.92/0.94 the posted patch apply without problem.
+ private void updateGlobalCache(ListMultimap< String ,TablePermission> userPerms) {
I would expect Permission in the method signature above. Can the following method be changed to return ListMultimap<String, Permission> ?
...I know, but I've tried to stay as close as possible with the current implementation. The global permission is just the same as permission on acl table.
Maybe we can open a new jira to refactor Permission/TablePermission/UserPermission.

Andrew Purtell
added a comment - 05/May/12 17:37 The AccessControllerProtocol change is not backwards compatible. You should deprecate
public void grant( byte [] user, TablePermission permission)
and
public void revoke( byte [] user, TablePermission permission)
in 0.92 (and 0.94 if it's released already) and take them out in the next major rev after.
The new 'whoami' command for the shell is nice.
I also see some noise/whitespace refactoring around debug logging. That kind of change is a little annoying, it distracts from the logic changes. Just a suggestion for future changes.

Hadoop QA
added a comment - 06/May/12 15:42 -1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12525773/HBASE-5342-v1.patch
against trunk revision .
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 9 new or modified tests.
-1 patch. The patch command could not apply the patch.
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1783//console
This message is automatically generated.

Hadoop QA
added a comment - 06/May/12 16:50 -1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12525774/HBASE-5342-v1.patch
against trunk revision .
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 9 new or modified tests.
+1 hadoop23. The patch compiles against the hadoop 0.23.x 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 does not introduce any 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.regionserver.wal.TestLogRollingNoCluster
Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1784//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1784//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1784//console
This message is automatically generated.

To maintain compatibility in HBase aka Hadoop RPC as it currently works, you must insure:

The new methods must be placed after all others in the interface. If the order of existing methods in the interface changes, it won't work.

Do not change VERSION in VersionedProtocols (which CoprocessorProtocol inherits from). This doesn't allow backwards compatibility, it tells the client to go away if different. We can use it to fast fail incompatible clients after a deprecation is complete but not during the transition.

Then, if the new methods are not available, on the client side you can catch NoSuchMethodException from the remote and use an alternate API strategy.

As you can imagine, it is a great thing we are migrating all of our RPC protocols to protobufs for 0.96+, it has a cross-version story that avoids kludges like the above. Unfortunately, the above is currently necessary.

Andrew Purtell
added a comment - 08/May/12 18:37 @Matteo,
To maintain compatibility in HBase aka Hadoop RPC as it currently works, you must insure:
The new methods must be placed after all others in the interface. If the order of existing methods in the interface changes, it won't work.
Do not change VERSION in VersionedProtocols (which CoprocessorProtocol inherits from). This doesn't allow backwards compatibility, it tells the client to go away if different. We can use it to fast fail incompatible clients after a deprecation is complete but not during the transition.
Then, if the new methods are not available, on the client side you can catch NoSuchMethodException from the remote and use an alternate API strategy.
As you can imagine, it is a great thing we are migrating all of our RPC protocols to protobufs for 0.96+, it has a cross-version story that avoids kludges like the above. Unfortunately, the above is currently necessary.

Hadoop QA
added a comment - 08/May/12 18:50 -1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12526007/HBASE-5342-v2.patch
against trunk revision .
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 9 new or modified tests.
+1 hadoop23. The patch compiles against the hadoop 0.23.x 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 does not introduce any 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:
Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1797//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1797//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1797//console
This message is automatically generated.

Jonathan Hsieh
added a comment - 09/May/12 01:58 Andrew: I don't think the order matters anymore – I checked this when I added the offline method to 0.90. Rpcs actually send their names across the wire!
See here on HBASE-5589 .
https://issues.apache.org/jira/browse/HBASE-5589?focusedCommentId=13233923&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13233923

Matteo Bertozzi
added a comment - 10/May/12 23:42 Keep the old protocol version
mark "old" grant/revoke as deprecated
catch if server doesn't support new grant/revoke with global and fallback to the old method if necessary.

Hadoop QA
added a comment - 11/May/12 01:01 -1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12526440/HBASE-5342-v3.patch
against trunk revision .
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 9 new or modified tests.
+1 hadoop23. The patch compiles against the hadoop 0.23.x 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 does not introduce any 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.TestRegionRebalancing
org.apache.hadoop.hbase.TestDrainingServer
org.apache.hadoop.hbase.coprocessor.TestClassLoading
Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1841//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1841//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1841//console
This message is automatically generated.

getTablePermissions() returns the permission for the specified table is used by AccessController.updateAcl() to write to zookeeper nodes the table permissions.
We can skip root & meta because they are handled as a special case, mostly in AccessController.permissionGranted(), while the acl table is used to store the global permissions. So instead of return an empty listMap we return the global permissions.

Matteo Bertozzi
added a comment - 11/May/12 18:09 getTablePermissions() returns the permission for the specified table is used by AccessController.updateAcl() to write to zookeeper nodes the table permissions.
We can skip root & meta because they are handled as a special case, mostly in AccessController.permissionGranted(), while the acl table is used to store the global permissions. So instead of return an empty listMap we return the global permissions.

Hadoop QA
added a comment - 11/May/12 19:57 +1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12526549/HBASE-5342-v4.patch
against trunk revision .
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 9 new or modified tests.
+1 hadoop23. The patch compiles against the hadoop 0.23.x 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 does not introduce any 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 passed unit tests in .
Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1849//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1849//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1849//console
This message is automatically generated.

Matteo Bertozzi
added a comment - 12/May/12 11:43 Can we backport this to 0.92 and 0.94? the security/access code is the same and this one doesn't break the compatibility.
I'll attach a patch later for 0.92/0.94.

Hadoop QA
added a comment - 12/May/12 15:42 -1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12526640/HBASE-5342-0.94.patch
against trunk revision .
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 9 new or modified tests.
-1 patch. The patch command could not apply the patch.
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1861//console
This message is automatically generated.

Matteo Bertozzi
added a comment - 17/May/12 17:52 @Andy, @Lars: Can we backport this to 0.92 and 0.94?
I've developed it on 0.92, since the code was the same from 0.92 to trunk.
And since the new functionality doesn't break the compatibility, I think that it is safe to backport. What do you think?

Andrew Purtell
added a comment - 18/May/12 00:11 I think that it is safe to backport. What do you think?
+1, we should keep the AccessController code in sync across trunk, 0.94, and 0.92 until finally there is some incompatible change on trunk; then as close as possible.

Ted Yu
added a comment - 18/May/12 00:40 Integrated to 0.92 and 0.94
Thanks for the patch, Matteo (there was a duplicate copy in whoami.rb of 0.94 patch, I removed the extra one)
Thanks for the review, Andy.