You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by ap...@apache.org on 2014/12/16 03:29:11 UTC

[1/4] hbase git commit: HBASE-12686 Failures in split before PONR not clearing the daughter regions from regions in transition during rollback (Vandana Ayyalasomayajula)

Repository: hbase
Updated Branches:
  refs/heads/0.98 4fcde44d2 -> b5f645e02
  refs/heads/branch-1 677153ad6 -> e4ad5581d
  refs/heads/master 065d03b78 -> 110c5f593


HBASE-12686 Failures in split before PONR not clearing the daughter regions from regions in transition during rollback (Vandana Ayyalasomayajula)


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/871444cb
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/871444cb
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/871444cb

Branch: refs/heads/branch-1
Commit: 871444cb0a733b82af843952253b4545a407979a
Parents: 677153a
Author: Andrew Purtell <ap...@apache.org>
Authored: Mon Dec 15 17:31:33 2014 -0800
Committer: Andrew Purtell <ap...@apache.org>
Committed: Mon Dec 15 17:32:05 2014 -0800

----------------------------------------------------------------------
 .../hadoop/hbase/master/AssignmentManager.java  | 38 +++++++++++++++-----
 .../TestSplitTransactionOnCluster.java          | 38 ++++++++++++++++++++
 2 files changed, 68 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/871444cb/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
index 29c824d..e39adc8 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
@@ -29,6 +29,7 @@ import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.NavigableMap;
 import java.util.Set;
 import java.util.TreeMap;
@@ -165,6 +166,9 @@ public class AssignmentManager extends ZooKeeperListener {
   private final Map<String, PairOfSameType<HRegionInfo>> mergingRegions
     = new HashMap<String, PairOfSameType<HRegionInfo>>();
 
+  private final Map<HRegionInfo, PairOfSameType<HRegionInfo>> splitRegions
+  = new HashMap<HRegionInfo, PairOfSameType<HRegionInfo>>();
+
   /**
    * The sleep time for which the assignment will wait before retrying in case of hbase:meta assignment
    * failure due to lack of availability of region plan or bad region plan
@@ -1321,14 +1325,30 @@ public class AssignmentManager extends ZooKeeperListener {
 
             ServerName serverName = rs.getServerName();
             if (serverManager.isServerOnline(serverName)) {
-              if (rs.isOnServer(serverName)
-                  && (rs.isOpened() || rs.isSplitting())) {
-                regionOnline(regionInfo, serverName);
-                if (disabled) {
-                  // if server is offline, no hurt to unassign again
-                  LOG.info("Opened " + regionNameStr
-                    + "but this table is disabled, triggering close of region");
-                  unassign(regionInfo);
+              if (rs.isOnServer(serverName) && (rs.isOpened() || rs.isSplitting())) {
+                synchronized (regionStates) {
+                  regionOnline(regionInfo, serverName);
+                  if (rs.isSplitting() && splitRegions.containsKey(regionInfo)) {
+                    // Check if the daugter regions are still there, if they are present, offline
+                    // as its the case of a rollback.
+                    HRegionInfo hri_a = splitRegions.get(regionInfo).getFirst();
+                    HRegionInfo hri_b = splitRegions.get(regionInfo).getSecond();
+                    if (!regionStates.isRegionInTransition(hri_a.getEncodedName())) {
+                      LOG.warn("Split daughter region not in transition " + hri_a);
+                    }
+                    if (!regionStates.isRegionInTransition(hri_b.getEncodedName())) {
+                      LOG.warn("Split daughter region not in transition" + hri_b);
+                    }
+                    regionOffline(hri_a);
+                    regionOffline(hri_b);
+                    splitRegions.remove(regionInfo);
+                  }
+                  if (disabled) {
+                    // if server is offline, no hurt to unassign again
+                    LOG.info("Opened " + regionNameStr
+                        + "but this table is disabled, triggering close of region");
+                    unassign(regionInfo);
+                  }
                 }
               } else if (rs.isMergingNew()) {
                 synchronized (regionStates) {
@@ -3798,6 +3818,7 @@ public class AssignmentManager extends ZooKeeperListener {
     }
 
     synchronized (regionStates) {
+      splitRegions.put(p, new PairOfSameType<HRegionInfo>(hri_a, hri_b));
       regionStates.updateRegionState(hri_a, State.SPLITTING_NEW, sn);
       regionStates.updateRegionState(hri_b, State.SPLITTING_NEW, sn);
       regionStates.updateRegionState(rt, State.SPLITTING);
@@ -3813,6 +3834,7 @@ public class AssignmentManager extends ZooKeeperListener {
         regionOffline(p, State.SPLIT);
         regionOnline(hri_a, sn);
         regionOnline(hri_b, sn);
+        splitRegions.remove(p);
       }
     }
 

http://git-wip-us.apache.org/repos/asf/hbase/blob/871444cb/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java
index ee9cd40..99de513 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java
@@ -58,6 +58,8 @@ import org.apache.hadoop.hbase.Waiter;
 import org.apache.hadoop.hbase.ZooKeeperConnectionException;
 import org.apache.hadoop.hbase.MetaTableAccessor;
 import org.apache.hadoop.hbase.client.Admin;
+import org.apache.hadoop.hbase.client.Connection;
+import org.apache.hadoop.hbase.client.ConnectionFactory;
 import org.apache.hadoop.hbase.client.Delete;
 import org.apache.hadoop.hbase.client.HBaseAdmin;
 import org.apache.hadoop.hbase.client.HTable;
@@ -1161,6 +1163,42 @@ public class TestSplitTransactionOnCluster {
       TESTING_UTIL.deleteTable(tableName);
     }
   }
+  
+  @Test
+  public void testFailedSplit() throws Exception {
+    TableName tableName = TableName.valueOf("testFailedSplit");
+    byte[] colFamily = Bytes.toBytes("info");
+    TESTING_UTIL.createTable(tableName, colFamily);
+    Connection connection = ConnectionFactory.createConnection(TESTING_UTIL.getConfiguration());
+    HTable table = (HTable) connection.getTable(tableName);
+    try {
+      TESTING_UTIL.loadTable(table, colFamily);
+      List<HRegionInfo> regions = TESTING_UTIL.getHBaseAdmin().getTableRegions(tableName);
+      assertTrue(regions.size() == 1);
+      final HRegion actualRegion = cluster.getRegions(tableName).get(0);
+      actualRegion.getCoprocessorHost().load(FailingSplitRegionObserver.class,
+        Coprocessor.PRIORITY_USER, actualRegion.getBaseConf());
+
+      // The following split would fail.
+      admin.split(tableName);
+      FailingSplitRegionObserver.latch.await();
+      LOG.info("Waiting for region to come out of RIT");
+      TESTING_UTIL.waitFor(60000, 1000, new Waiter.Predicate<Exception>() {
+        @Override
+        public boolean evaluate() throws Exception {
+          RegionStates regionStates = cluster.getMaster().getAssignmentManager().getRegionStates();
+          Map<String, RegionState> rit = regionStates.getRegionsInTransition();
+          return !rit.containsKey(actualRegion.getRegionInfo().getEncodedName());
+        }
+      });
+      regions = TESTING_UTIL.getHBaseAdmin().getTableRegions(tableName);
+      assertTrue(regions.size() == 1);
+    } finally {
+      table.close();
+      connection.close();
+      TESTING_UTIL.deleteTable(tableName);
+    }
+  }
 
     public static class MockedCoordinatedStateManager extends ZkCoordinatedStateManager {
 


[2/4] hbase git commit: HBASE-12348 preModifyColumn and preDeleteColumn in AC denies user to perform its operation though it has required rights

Posted by ap...@apache.org.
HBASE-12348 preModifyColumn and preDeleteColumn in AC denies user to perform its operation though it has required rights

Signed-off-by: Andrew Purtell <ap...@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/110c5f59
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/110c5f59
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/110c5f59

Branch: refs/heads/master
Commit: 110c5f593057366509b8480e396cc750d5fd782b
Parents: 065d03b
Author: Ashish Singhi <as...@huawei.com>
Authored: Mon Dec 15 17:43:19 2014 -0800
Committer: Andrew Purtell <ap...@apache.org>
Committed: Mon Dec 15 17:43:19 2014 -0800

----------------------------------------------------------------------
 .../hbase/security/access/AccessController.java  |  5 +++--
 .../security/access/TestAccessController.java    | 19 +++++++++++++------
 2 files changed, 16 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/110c5f59/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
index c7abb81..ce3d08b 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
@@ -1028,13 +1028,14 @@ public class AccessController extends BaseMasterAndRegionObserver
   @Override
   public void preModifyColumn(ObserverContext<MasterCoprocessorEnvironment> c, TableName tableName,
       HColumnDescriptor descriptor) throws IOException {
-    requirePermission("modifyColumn", tableName, null, null, Action.ADMIN, Action.CREATE);
+    requirePermission("modifyColumn", tableName, descriptor.getName(), null, Action.ADMIN,
+      Action.CREATE);
   }
 
   @Override
   public void preDeleteColumn(ObserverContext<MasterCoprocessorEnvironment> c, TableName tableName,
       byte[] col) throws IOException {
-    requirePermission("deleteColumn", tableName, null, null, Action.ADMIN, Action.CREATE);
+    requirePermission("deleteColumn", tableName, col, null, Action.ADMIN, Action.CREATE);
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/hbase/blob/110c5f59/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
index 28d33d9..11a131b 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
@@ -155,6 +155,8 @@ public class TestAccessController extends SecureTestUtil {
   private static User USER_CREATE;
   // user with no permissions
   private static User USER_NONE;
+  // user with admin rights on the column family
+  private static User USER_ADMIN_CF;
 
   // TODO: convert this test to cover the full matrix in
   // https://hbase.apache.org/book/appendix_acl_matrix.html
@@ -213,6 +215,7 @@ public class TestAccessController extends SecureTestUtil {
     USER_OWNER = User.createUserForTesting(conf, "owner", new String[0]);
     USER_CREATE = User.createUserForTesting(conf, "tbl_create", new String[0]);
     USER_NONE = User.createUserForTesting(conf, "nouser", new String[0]);
+    USER_ADMIN_CF = User.createUserForTesting(conf, "col_family_admin", new String[0]);
   }
 
   @AfterClass
@@ -261,9 +264,13 @@ public class TestAccessController extends SecureTestUtil {
       TEST_TABLE.getTableName(), TEST_FAMILY, null,
       Permission.Action.READ);
 
-    assertEquals(4, AccessControlLists.getTablePermissions(conf, TEST_TABLE.getTableName()).size());
+    grantOnTable(TEST_UTIL, USER_ADMIN_CF.getShortName(),
+      TEST_TABLE.getTableName(), TEST_FAMILY,
+      null, Permission.Action.ADMIN);
+
+    assertEquals(5, AccessControlLists.getTablePermissions(conf, TEST_TABLE.getTableName()).size());
     try {
-      assertEquals(4, AccessControlClient.getUserPermissions(conf, TEST_TABLE.toString()).size());
+      assertEquals(5, AccessControlClient.getUserPermissions(conf, TEST_TABLE.toString()).size());
     } catch (Throwable e) {
       LOG.error("error during call of AccessControlClient.getUserPermissions. ", e);
     }
@@ -378,7 +385,7 @@ public class TestAccessController extends SecureTestUtil {
       }
     };
 
-    verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_CREATE, USER_OWNER);
+    verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_CREATE, USER_OWNER, USER_ADMIN_CF);
     verifyDenied(action, USER_RW, USER_RO, USER_NONE);
   }
 
@@ -393,7 +400,7 @@ public class TestAccessController extends SecureTestUtil {
       }
     };
 
-    verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_CREATE, USER_OWNER);
+    verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_CREATE, USER_OWNER, USER_ADMIN_CF);
     verifyDenied(action, USER_RW, USER_RO, USER_NONE);
   }
 
@@ -2542,8 +2549,8 @@ public class TestAccessController extends SecureTestUtil {
       null, Action.ADMIN);
     List<UserPermission> perms = testUserPerms.runAs(getPrivilegedAction(regex));
     assertNotNull(perms);
-    // USER_ADMIN, USER_CREATE, USER_RW, USER_RO, testUserPerms has row each.
-    assertEquals(5, perms.size());
+    // USER_ADMIN, USER_CREATE, USER_RW, USER_RO, testUserPerms, USER_ADMIN_CF has row each.
+    assertEquals(6, perms.size());
   }
 
   @Test


[3/4] hbase git commit: HBASE-12348 preModifyColumn and preDeleteColumn in AC denies user to perform its operation though it has required rights

Posted by ap...@apache.org.
HBASE-12348 preModifyColumn and preDeleteColumn in AC denies user to perform its operation though it has required rights

Signed-off-by: Andrew Purtell <ap...@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/e4ad5581
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/e4ad5581
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/e4ad5581

Branch: refs/heads/branch-1
Commit: e4ad5581d9380c88df06129f497e744ea43f9801
Parents: 871444c
Author: Ashish Singhi <as...@huawei.com>
Authored: Mon Dec 15 17:43:19 2014 -0800
Committer: Andrew Purtell <ap...@apache.org>
Committed: Mon Dec 15 17:43:28 2014 -0800

----------------------------------------------------------------------
 .../hbase/security/access/AccessController.java  |  5 +++--
 .../security/access/TestAccessController.java    | 19 +++++++++++++------
 2 files changed, 16 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/e4ad5581/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
index 5050afb..c88bf9d 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
@@ -1026,13 +1026,14 @@ public class AccessController extends BaseMasterAndRegionObserver
   @Override
   public void preModifyColumn(ObserverContext<MasterCoprocessorEnvironment> c, TableName tableName,
       HColumnDescriptor descriptor) throws IOException {
-    requirePermission("modifyColumn", tableName, null, null, Action.ADMIN, Action.CREATE);
+    requirePermission("modifyColumn", tableName, descriptor.getName(), null, Action.ADMIN,
+      Action.CREATE);
   }
 
   @Override
   public void preDeleteColumn(ObserverContext<MasterCoprocessorEnvironment> c, TableName tableName,
       byte[] col) throws IOException {
-    requirePermission("deleteColumn", tableName, null, null, Action.ADMIN, Action.CREATE);
+    requirePermission("deleteColumn", tableName, col, null, Action.ADMIN, Action.CREATE);
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/hbase/blob/e4ad5581/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
index e164a6f..4024317 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
@@ -152,6 +152,8 @@ public class TestAccessController extends SecureTestUtil {
   private static User USER_CREATE;
   // user with no permissions
   private static User USER_NONE;
+  // user with admin rights on the column family
+  private static User USER_ADMIN_CF;
 
   // TODO: convert this test to cover the full matrix in
   // https://hbase.apache.org/book/appendix_acl_matrix.html
@@ -210,6 +212,7 @@ public class TestAccessController extends SecureTestUtil {
     USER_OWNER = User.createUserForTesting(conf, "owner", new String[0]);
     USER_CREATE = User.createUserForTesting(conf, "tbl_create", new String[0]);
     USER_NONE = User.createUserForTesting(conf, "nouser", new String[0]);
+    USER_ADMIN_CF = User.createUserForTesting(conf, "col_family_admin", new String[0]);
   }
 
   @AfterClass
@@ -258,9 +261,13 @@ public class TestAccessController extends SecureTestUtil {
       TEST_TABLE.getTableName(), TEST_FAMILY, null,
       Permission.Action.READ);
 
-    assertEquals(4, AccessControlLists.getTablePermissions(conf, TEST_TABLE.getTableName()).size());
+    grantOnTable(TEST_UTIL, USER_ADMIN_CF.getShortName(),
+      TEST_TABLE.getTableName(), TEST_FAMILY,
+      null, Permission.Action.ADMIN);
+
+    assertEquals(5, AccessControlLists.getTablePermissions(conf, TEST_TABLE.getTableName()).size());
     try {
-      assertEquals(4, AccessControlClient.getUserPermissions(conf, TEST_TABLE.toString()).size());
+      assertEquals(5, AccessControlClient.getUserPermissions(conf, TEST_TABLE.toString()).size());
     } catch (Throwable e) {
       LOG.error("error during call of AccessControlClient.getUserPermissions. ", e);
     }
@@ -375,7 +382,7 @@ public class TestAccessController extends SecureTestUtil {
       }
     };
 
-    verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_CREATE, USER_OWNER);
+    verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_CREATE, USER_OWNER, USER_ADMIN_CF);
     verifyDenied(action, USER_RW, USER_RO, USER_NONE);
   }
 
@@ -390,7 +397,7 @@ public class TestAccessController extends SecureTestUtil {
       }
     };
 
-    verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_CREATE, USER_OWNER);
+    verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_CREATE, USER_OWNER, USER_ADMIN_CF);
     verifyDenied(action, USER_RW, USER_RO, USER_NONE);
   }
 
@@ -2415,8 +2422,8 @@ public class TestAccessController extends SecureTestUtil {
       null, Action.ADMIN);
     List<UserPermission> perms = testUserPerms.runAs(getPrivilegedAction(regex));
     assertNotNull(perms);
-    // USER_ADMIN, USER_CREATE, USER_RW, USER_RO, testUserPerms has row each.
-    assertEquals(5, perms.size());
+    // USER_ADMIN, USER_CREATE, USER_RW, USER_RO, testUserPerms, USER_ADMIN_CF has row each.
+    assertEquals(6, perms.size());
   }
 
   @Test


[4/4] hbase git commit: HBASE-12348 preModifyColumn and preDeleteColumn in AC denies user to perform its operation though it has required rights

Posted by ap...@apache.org.
HBASE-12348 preModifyColumn and preDeleteColumn in AC denies user to perform its operation though it has required rights

Signed-off-by: Andrew Purtell <ap...@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/b5f645e0
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/b5f645e0
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/b5f645e0

Branch: refs/heads/0.98
Commit: b5f645e02dc310d59379fcda2519ab82b4403332
Parents: 4fcde44
Author: Ashish Singhi <as...@huawei.com>
Authored: Mon Dec 15 17:43:35 2014 -0800
Committer: Andrew Purtell <ap...@apache.org>
Committed: Mon Dec 15 17:44:09 2014 -0800

----------------------------------------------------------------------
 .../hbase/security/access/AccessController.java      |  5 +++--
 .../hbase/security/access/TestAccessController.java  | 15 +++++++++++----
 2 files changed, 14 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/b5f645e0/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
index 506acee..a28884c 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
@@ -1022,13 +1022,14 @@ public class AccessController extends BaseMasterAndRegionObserver
   @Override
   public void preModifyColumn(ObserverContext<MasterCoprocessorEnvironment> c, TableName tableName,
       HColumnDescriptor descriptor) throws IOException {
-    requirePermission("modifyColumn", tableName, null, null, Action.ADMIN, Action.CREATE);
+    requirePermission("modifyColumn", tableName, descriptor.getName(), null, Action.ADMIN,
+      Action.CREATE);
   }
 
   @Override
   public void preDeleteColumn(ObserverContext<MasterCoprocessorEnvironment> c, TableName tableName,
       byte[] col) throws IOException {
-    requirePermission("deleteColumn", tableName, null, null, Action.ADMIN, Action.CREATE);
+    requirePermission("deleteColumn", tableName, col, null, Action.ADMIN, Action.CREATE);
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/hbase/blob/b5f645e0/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
index 36e3bce..e10e0f1 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java
@@ -147,6 +147,8 @@ public class TestAccessController extends SecureTestUtil {
   private static User USER_CREATE;
   // user with no permissions
   private static User USER_NONE;
+  // user with admin rights on the column family
+  private static User USER_ADMIN_CF;
 
   // TODO: convert this test to cover the full matrix in
   // https://hbase.apache.org/book/appendix_acl_matrix.html
@@ -204,6 +206,7 @@ public class TestAccessController extends SecureTestUtil {
     USER_OWNER = User.createUserForTesting(conf, "owner", new String[0]);
     USER_CREATE = User.createUserForTesting(conf, "tbl_create", new String[0]);
     USER_NONE = User.createUserForTesting(conf, "nouser", new String[0]);
+    USER_ADMIN_CF = User.createUserForTesting(conf, "col_family_admin", new String[0]);
   }
 
   @AfterClass
@@ -252,9 +255,13 @@ public class TestAccessController extends SecureTestUtil {
       TEST_TABLE.getTableName(), TEST_FAMILY, null,
       Permission.Action.READ);
 
-    assertEquals(4, AccessControlLists.getTablePermissions(conf, TEST_TABLE.getTableName()).size());
+    grantOnTable(TEST_UTIL, USER_ADMIN_CF.getShortName(),
+      TEST_TABLE.getTableName(), TEST_FAMILY,
+      null, Permission.Action.ADMIN);
+
+    assertEquals(5, AccessControlLists.getTablePermissions(conf, TEST_TABLE.getTableName()).size());
     try {
-      assertEquals(4, AccessControlClient.getUserPermissions(conf, TEST_TABLE.toString()).size());
+      assertEquals(5, AccessControlClient.getUserPermissions(conf, TEST_TABLE.toString()).size());
     } catch (Throwable e) {
       LOG.error("error during call of AccessControlClient.getUserPermissions. " + e.getStackTrace());
     }
@@ -369,7 +376,7 @@ public class TestAccessController extends SecureTestUtil {
       }
     };
 
-    verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_CREATE, USER_OWNER);
+    verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_CREATE, USER_OWNER, USER_ADMIN_CF);
     verifyDenied(action, USER_RW, USER_RO, USER_NONE);
   }
 
@@ -384,7 +391,7 @@ public class TestAccessController extends SecureTestUtil {
       }
     };
 
-    verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_CREATE, USER_OWNER);
+    verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_CREATE, USER_OWNER, USER_ADMIN_CF);
     verifyDenied(action, USER_RW, USER_RO, USER_NONE);
   }