From 4da7e9413530c4dd0313959c5bd0a853c5c486ef Mon Sep 17 00:00:00 2001 From: meiyi Date: Fri, 28 Jun 2019 17:32:03 +0800 Subject: [PATCH] HBASE-22580 Add a table attribute to make user scan snapshot feature configurable for table --- .../security/access/PermissionStorage.java | 5 + .../SnapshotScannerHDFSAclController.java | 198 +++++++++++------- .../access/SnapshotScannerHDFSAclHelper.java | 100 +++++++-- .../TestSnapshotScannerHDFSAclController.java | 79 ++++++- 4 files changed, 275 insertions(+), 107 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/PermissionStorage.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/PermissionStorage.java index 6d37fe5bf4f8..2a9b195f0f80 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/PermissionStorage.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/PermissionStorage.java @@ -508,6 +508,11 @@ public static ListMultimap getNamespacePermissions(Confi false); } + public static ListMultimap getGlobalPermissions(Configuration conf) + throws IOException { + return getPermissions(conf, null, null, null, null, null, false); + } + /** * Reads user permission assignments stored in the l: column family of the first * table row in _acl_. diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/SnapshotScannerHDFSAclController.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/SnapshotScannerHDFSAclController.java index c964b67ac5d3..6f495c4007ab 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/SnapshotScannerHDFSAclController.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/SnapshotScannerHDFSAclController.java @@ -38,6 +38,7 @@ import org.apache.hadoop.hbase.TableNotFoundException; import org.apache.hadoop.hbase.client.Admin; import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder; +import org.apache.hadoop.hbase.client.Connection; import org.apache.hadoop.hbase.client.Delete; import org.apache.hadoop.hbase.client.Get; import org.apache.hadoop.hbase.client.Put; @@ -58,7 +59,6 @@ import org.apache.hadoop.hbase.master.MasterServices; import org.apache.hadoop.hbase.security.User; import org.apache.hadoop.hbase.security.UserProvider; -import org.apache.hadoop.hbase.security.access.Permission.Action; import org.apache.hadoop.hbase.security.access.SnapshotScannerHDFSAclHelper.PathHelper; import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.Pair; @@ -66,12 +66,15 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting; +import org.apache.hbase.thirdparty.com.google.common.collect.Sets; + /** * Set HDFS ACLs to hFiles to make HBase granted users have permission to scan snapshot *

* To use this feature, please mask sure HDFS config: *

    - *
  • dfs.permissions.enabled = true
    • + *
  • dfs.namenode.acls.enabled = true
    • *
  • fs.permissions.umask-mode = 027 (or smaller umask than 027)
    • *
*

@@ -103,7 +106,9 @@ public class SnapshotScannerHDFSAclController implements MasterCoprocessor, Mast private SnapshotScannerHDFSAclHelper hdfsAclHelper = null; private PathHelper pathHelper = null; private FileSystem fs = null; + private MasterServices masterServices = null; private volatile boolean initialized = false; + private volatile boolean aclTableInitialized = false; /** Provider for mapping principal names to Users */ private UserProvider userProvider; @@ -121,7 +126,7 @@ public void preMasterInitialization(ObserverContext c) throws IOException { - if (checkInitialized()) { + if (initialized) { try (Admin admin = c.getEnvironment().getConnection().getAdmin()) { if (admin.tableExists(PermissionStorage.ACL_TABLE_NAME)) { // Check if hbase acl table has 'm' CF, if not, add 'm' CF @@ -152,6 +157,7 @@ public void postStartMaster(ObserverContext c) thr .newBuilder(SnapshotScannerHDFSAclStorage.HDFS_ACL_FAMILY).build()); admin.modifyTable(builder.build()); } + aclTableInitialized = true; } else { throw new TableNotFoundException("Table " + PermissionStorage.ACL_TABLE_NAME + " is not created yet. Please check if " + getClass().getName() @@ -163,7 +169,7 @@ public void postStartMaster(ObserverContext c) thr @Override public void preStopMaster(ObserverContext c) { - if (checkInitialized()) { + if (initialized) { hdfsAclHelper.close(); } } @@ -171,29 +177,22 @@ public void preStopMaster(ObserverContext c) { @Override public void postCompletedCreateTableAction(ObserverContext c, TableDescriptor desc, RegionInfo[] regions) throws IOException { - if (!desc.getTableName().isSystemTable() && checkInitialized()) { + if (needHandleTableHdfsAcl(desc, "createTable " + desc.getTableName())) { TableName tableName = desc.getTableName(); - List paths = hdfsAclHelper.getTableRootPaths(tableName, false); - for (Path path : paths) { - if (!fs.exists(path)) { - fs.mkdirs(path); - } - } + hdfsAclHelper.createTableDirectories(tableName); // Add table owner HDFS acls String owner = desc.getOwnerString() == null ? getActiveUser(c).getShortName() : desc.getOwnerString(); - hdfsAclHelper.addTableAcl(tableName, owner); - try (Table aclTable = - c.getEnvironment().getConnection().getTable(PermissionStorage.ACL_TABLE_NAME)) { - SnapshotScannerHDFSAclStorage.addUserTableHdfsAcl(aclTable, owner, tableName); - } + hdfsAclHelper.addTableAcl(tableName, Sets.newHashSet(owner)); + SnapshotScannerHDFSAclStorage.addUserTableHdfsAcl(c.getEnvironment().getConnection(), owner, + tableName); } } @Override public void postCreateNamespace(ObserverContext c, NamespaceDescriptor ns) throws IOException { - if (checkInitialized()) { + if (checkInitialized("createNamespace " + ns.getName())) { List paths = hdfsAclHelper.getNamespaceRootPaths(ns.getName()); for (Path path : paths) { if (!fs.exists(path)) { @@ -206,7 +205,7 @@ public void postCreateNamespace(ObserverContext c, @Override public void postCompletedSnapshotAction(ObserverContext c, SnapshotDescription snapshot, TableDescriptor tableDescriptor) throws IOException { - if (!tableDescriptor.getTableName().isSystemTable() && checkInitialized()) { + if (needHandleTableHdfsAcl(tableDescriptor, "snapshot " + snapshot.getName())) { hdfsAclHelper.snapshotAcl(snapshot); } } @@ -214,7 +213,7 @@ public void postCompletedSnapshotAction(ObserverContext c, TableName tableName) throws IOException { - if (!tableName.isSystemTable() && checkInitialized()) { + if (needHandleTableHdfsAcl(tableName, "truncateTable " + tableName)) { hdfsAclHelper.resetTableAcl(tableName); } } @@ -222,7 +221,7 @@ public void postCompletedTruncateTableAction(ObserverContext ctx, TableName tableName) throws IOException { - if (!tableName.isSystemTable() && checkInitialized()) { + if (needHandleTableHdfsAcl(tableName, "deleteTable " + tableName)) { /* * remove table user access HDFS acl from namespace directory if the user has no permissions * of global, ns of the table or other tables of the ns, eg: Bob has 'ns1:t1' read permission, @@ -263,14 +262,29 @@ public void postDeleteTable(ObserverContext ctx, } } + @Override + public void postModifyTable(ObserverContext ctx, + TableName tableName, TableDescriptor oldDescriptor, TableDescriptor currentDescriptor) + throws IOException { + if (needHandleTableHdfsAcl(currentDescriptor, "modifyTable " + tableName) + && !hdfsAclHelper.isTableUserScanSnapshotEnabled(oldDescriptor)) { + hdfsAclHelper.createTableDirectories(tableName); + Set tableUsers = hdfsAclHelper.getUsersWithTableReadAction(tableName, false, false); + Set users = + hdfsAclHelper.getUsersWithNamespaceReadAction(tableName.getNamespaceAsString(), true); + users.addAll(tableUsers); + hdfsAclHelper.addTableAcl(tableName, users); + SnapshotScannerHDFSAclStorage.addUserTableHdfsAcl(ctx.getEnvironment().getConnection(), + tableUsers, tableName); + } + } + @Override public void postDeleteNamespace(ObserverContext ctx, String namespace) throws IOException { - if (checkInitialized()) { - try (Table aclTable = - ctx.getEnvironment().getConnection().getTable(PermissionStorage.ACL_TABLE_NAME)) { - SnapshotScannerHDFSAclStorage.deleteNamespaceHdfsAcl(aclTable, namespace); - } + if (checkInitialized("deleteNamespace " + namespace)) { + SnapshotScannerHDFSAclStorage.deleteNamespaceHdfsAcl(ctx.getEnvironment().getConnection(), + namespace); /** * Delete namespace tmp directory because it's created by this coprocessor when namespace is * created to make namespace default acl can be inherited by tables. The namespace data @@ -292,7 +306,8 @@ public void postDeleteNamespace(ObserverContext ct @Override public void postGrant(ObserverContext c, UserPermission userPermission, boolean mergeExistingPermissions) throws IOException { - if (!checkInitialized()) { + if (!checkInitialized( + "grant " + userPermission + ", merge existing permissions " + mergeExistingPermissions)) { return; } try (Table aclTable = @@ -302,7 +317,7 @@ public void postGrant(ObserverContext c, switch (userPermission.getAccessScope()) { case GLOBAL: UserPermission perm = getUserGlobalPermission(conf, userName); - if (perm != null && containReadPermission(perm)) { + if (perm != null && hdfsAclHelper.containReadAction(perm)) { if (!isHdfsAclSet(aclTable, userName)) { Pair, Set> namespaceAndTable = SnapshotScannerHDFSAclStorage.getUserNamespaceAndTable(aclTable, userName); @@ -322,7 +337,7 @@ public void postGrant(ObserverContext c, case NAMESPACE: String namespace = ((NamespacePermission) userPermission.getPermission()).getNamespace(); UserPermission nsPerm = getUserNamespacePermission(conf, userName, namespace); - if (nsPerm != null && containReadPermission(nsPerm)) { + if (nsPerm != null && hdfsAclHelper.containReadAction(nsPerm)) { if (!isHdfsAclSet(aclTable, userName, namespace)) { Set skipTables = SnapshotScannerHDFSAclStorage .getUserNamespaceAndTable(aclTable, userName).getSecond(); @@ -336,24 +351,21 @@ public void postGrant(ObserverContext c, } break; case TABLE: - TableName tableName = ((TablePermission) userPermission.getPermission()).getTableName(); - UserPermission tPerm = getUserTablePermission(conf, userName, tableName); - if (tPerm != null) { - TablePermission tablePermission = (TablePermission) tPerm.getPermission(); - if (tablePermission.hasFamily() || tablePermission.hasQualifier()) { - break; + TablePermission tablePerm = (TablePermission) userPermission.getPermission(); + if (needHandleTableHdfsAcl(tablePerm)) { + TableName tableName = tablePerm.getTableName(); + UserPermission tPerm = getUserTablePermission(conf, userName, tableName); + if (tPerm != null && hdfsAclHelper.containReadAction(tPerm)) { + if (!isHdfsAclSet(aclTable, userName, tableName)) { + hdfsAclHelper.grantAcl(userPermission, new HashSet<>(0), new HashSet<>(0)); + } + SnapshotScannerHDFSAclStorage.addUserTableHdfsAcl(aclTable, userName, tableName); + } else { + // The merged user permission doesn't contain READ, so remove user table HDFS acls if + // it's set + removeUserTableHdfsAcl(aclTable, userName, tableName, userPermission); } } - if (tPerm != null && containReadPermission(tPerm)) { - if (!isHdfsAclSet(aclTable, userName, tableName)) { - hdfsAclHelper.grantAcl(userPermission, new HashSet<>(0), new HashSet<>(0)); - } - SnapshotScannerHDFSAclStorage.addUserTableHdfsAcl(aclTable, userName, tableName); - } else { - // The merged user permission doesn't contain READ, so remove user table HDFS acls if - // it's set - removeUserTableHdfsAcl(aclTable, userName, tableName, userPermission); - } break; default: throw new IllegalArgumentException( @@ -365,7 +377,7 @@ public void postGrant(ObserverContext c, @Override public void postRevoke(ObserverContext c, UserPermission userPermission) throws IOException { - if (checkInitialized()) { + if (checkInitialized("revoke " + userPermission)) { try (Table aclTable = c.getEnvironment().getConnection().getTable(PermissionStorage.ACL_TABLE_NAME)) { String userName = userPermission.getUser(); @@ -373,7 +385,7 @@ public void postRevoke(ObserverContext c, switch (userPermission.getAccessScope()) { case GLOBAL: UserPermission userGlobalPerm = getUserGlobalPermission(conf, userName); - if (userGlobalPerm == null || !containReadPermission(userGlobalPerm)) { + if (userGlobalPerm == null || !hdfsAclHelper.containReadAction(userGlobalPerm)) { removeUserGlobalHdfsAcl(aclTable, userName, userPermission); } break; @@ -381,16 +393,18 @@ public void postRevoke(ObserverContext c, NamespacePermission nsPerm = (NamespacePermission) userPermission.getPermission(); UserPermission userNsPerm = getUserNamespacePermission(conf, userName, nsPerm.getNamespace()); - if (userNsPerm == null || !containReadPermission(userNsPerm)) { + if (userNsPerm == null || !hdfsAclHelper.containReadAction(userNsPerm)) { removeUserNamespaceHdfsAcl(aclTable, userName, nsPerm.getNamespace(), userPermission); } break; case TABLE: TablePermission tPerm = (TablePermission) userPermission.getPermission(); - UserPermission userTablePerm = - getUserTablePermission(conf, userName, tPerm.getTableName()); - if (userTablePerm == null || !containReadPermission(userTablePerm)) { - removeUserTableHdfsAcl(aclTable, userName, tPerm.getTableName(), userPermission); + if (needHandleTableHdfsAcl(tPerm)) { + TableName tableName = tPerm.getTableName(); + UserPermission userTablePerm = getUserTablePermission(conf, userName, tableName); + if (userTablePerm == null || !hdfsAclHelper.containReadAction(userTablePerm)) { + removeUserTableHdfsAcl(aclTable, userName, tableName, userPermission); + } } break; default: @@ -442,42 +456,28 @@ private void removeUserTableHdfsAcl(Table aclTable, String userName, TableName t } } - private boolean containReadPermission(UserPermission userPermission) { - if (userPermission != null) { - return Arrays.stream(userPermission.getPermission().getActions()) - .anyMatch(action -> action == Action.READ); - } - return false; - } - private UserPermission getUserGlobalPermission(Configuration conf, String userName) throws IOException { List permissions = PermissionStorage.getUserPermissions(conf, PermissionStorage.ACL_GLOBAL_NAME, null, null, userName, true); - if (permissions != null && permissions.size() > 0) { - return permissions.get(0); - } - return null; + return permissions.size() > 0 ? permissions.get(0) : null; } private UserPermission getUserNamespacePermission(Configuration conf, String userName, String namespace) throws IOException { List permissions = PermissionStorage.getUserNamespacePermissions(conf, namespace, userName, true); - if (permissions != null && permissions.size() > 0) { - return permissions.get(0); - } - return null; + return permissions.size() > 0 ? permissions.get(0) : null; } private UserPermission getUserTablePermission(Configuration conf, String userName, TableName tableName) throws IOException { - List permissions = - PermissionStorage.getUserTablePermissions(conf, tableName, null, null, userName, true); - if (permissions != null && permissions.size() > 0) { - return permissions.get(0); - } - return null; + List permissions = PermissionStorage + .getUserTablePermissions(conf, tableName, null, null, userName, true).stream() + .filter(userPermission -> hdfsAclHelper + .isNotFamilyOrQualifierPermission((TablePermission) userPermission.getPermission())) + .collect(Collectors.toList()); + return permissions.size() > 0 ? permissions.get(0) : null; } private boolean isHdfsAclSet(Table aclTable, String userName) throws IOException { @@ -513,12 +513,32 @@ private boolean isHdfsAclSet(Table aclTable, String userName, String namespace, return isSet; } - private boolean checkInitialized() { + @VisibleForTesting + boolean checkInitialized(String operation) { if (initialized) { - return true; - } else { - return false; + if (aclTableInitialized) { + return true; + } else { + LOG.warn("Skip set HDFS acls because acl table is not initialized when " + operation); + } } + return false; + } + + private boolean needHandleTableHdfsAcl(TablePermission tablePermission) throws IOException { + return needHandleTableHdfsAcl(tablePermission.getTableName(), "") + && hdfsAclHelper.isNotFamilyOrQualifierPermission(tablePermission); + } + + private boolean needHandleTableHdfsAcl(TableName tableName, String operation) throws IOException { + return !tableName.isSystemTable() && checkInitialized(operation) && hdfsAclHelper + .isTableUserScanSnapshotEnabled(masterServices.getTableDescriptors().get(tableName)); + } + + private boolean needHandleTableHdfsAcl(TableDescriptor tableDescriptor, String operation) { + TableName tableName = tableDescriptor.getTableName(); + return !tableName.isSystemTable() && checkInitialized(operation) + && hdfsAclHelper.isTableUserScanSnapshotEnabled(tableDescriptor); } private User getActiveUser(ObserverContext ctx) throws IOException { @@ -554,6 +574,22 @@ static void addUserNamespaceHdfsAcl(Table aclTable, String user, String namespac addUserEntry(aclTable, user, Bytes.toBytes(PermissionStorage.toNamespaceEntry(namespace))); } + static void addUserTableHdfsAcl(Connection connection, Set users, TableName tableName) + throws IOException { + try (Table aclTable = connection.getTable(PermissionStorage.ACL_TABLE_NAME)) { + for (String user : users) { + addUserTableHdfsAcl(aclTable, user, tableName); + } + } + } + + static void addUserTableHdfsAcl(Connection connection, String user, TableName tableName) + throws IOException { + try (Table aclTable = connection.getTable(PermissionStorage.ACL_TABLE_NAME)) { + addUserTableHdfsAcl(aclTable, user, tableName); + } + } + static void addUserTableHdfsAcl(Table aclTable, String user, TableName tableName) throws IOException { addUserEntry(aclTable, user, tableName.getName()); @@ -586,8 +622,10 @@ private static void deleteUserEntry(Table aclTable, String user, byte[] entry) aclTable.delete(delete); } - static void deleteNamespaceHdfsAcl(Table aclTable, String namespace) throws IOException { - deleteEntry(aclTable, Bytes.toBytes(PermissionStorage.toNamespaceEntry(namespace))); + static void deleteNamespaceHdfsAcl(Connection connection, String namespace) throws IOException { + try (Table aclTable = connection.getTable(PermissionStorage.ACL_TABLE_NAME)) { + deleteEntry(aclTable, Bytes.toBytes(PermissionStorage.toNamespaceEntry(namespace))); + } } static void deleteTableHdfsAcl(Table aclTable, TableName tableName) throws IOException { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/SnapshotScannerHDFSAclHelper.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/SnapshotScannerHDFSAclHelper.java index 2a62ff0ab69d..72764b831203 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/SnapshotScannerHDFSAclHelper.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/SnapshotScannerHDFSAclHelper.java @@ -23,7 +23,6 @@ import static org.apache.hadoop.fs.permission.AclEntryType.GROUP; import static org.apache.hadoop.fs.permission.AclEntryType.USER; import static org.apache.hadoop.fs.permission.FsAction.READ_EXECUTE; -import static org.apache.hadoop.hbase.security.access.Permission.Action.READ; import java.io.Closeable; import java.io.FileNotFoundException; @@ -47,6 +46,7 @@ import org.apache.hadoop.fs.permission.FsPermission; import org.apache.hadoop.hbase.AuthUtil; import org.apache.hadoop.hbase.HConstants; +import org.apache.hadoop.hbase.NamespaceDescriptor; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.Admin; import org.apache.hadoop.hbase.client.Connection; @@ -58,6 +58,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.apache.hbase.thirdparty.com.google.common.collect.ListMultimap; import org.apache.hbase.thirdparty.com.google.common.collect.Lists; import org.apache.hbase.thirdparty.com.google.common.collect.Sets; import org.apache.hbase.thirdparty.com.google.common.util.concurrent.ThreadFactoryBuilder; @@ -194,8 +195,7 @@ public boolean snapshotAcl(SnapshotDescription snapshot) { long start = System.currentTimeMillis(); TableName tableName = snapshot.getTableName(); // global user permission can be inherited from default acl automatically - Set userSet = getUsersWithTableReadAction(tableName); - userSet.addAll(getUsersWithNamespaceReadAction(tableName.getNamespaceAsString())); + Set userSet = getUsersWithTableReadAction(tableName, true, false); Path path = pathHelper.getSnapshotDir(snapshot.getName()); handleHDFSAcl(new HDFSAclOperation(fs, path, userSet, HDFSAclOperation.OperationType.MODIFY, true, HDFSAclOperation.AclType.DEFAULT_ADN_ACCESS)).get(); @@ -217,7 +217,7 @@ public boolean resetTableAcl(TableName tableName) { try { long start = System.currentTimeMillis(); // global and namespace user permission can be inherited from default acl automatically - setTableAcl(tableName, getUsersWithTableReadAction(tableName)); + setTableAcl(tableName, getUsersWithTableReadAction(tableName, false, false)); LOG.info("Set HDFS acl when truncate {}, cost {} ms", tableName, System.currentTimeMillis() - start); return true; @@ -260,20 +260,20 @@ public boolean removeNamespaceAcl(TableName tableName, Set removeUsers) } /** - * Set table owner acl when create table + * Set table user acl when create or modify table * @param tableName the table - * @param user the table owner + * @param users the table users with READ permission * @return false if an error occurred, otherwise true */ - public boolean addTableAcl(TableName tableName, String user) { + public boolean addTableAcl(TableName tableName, Set users) { try { long start = System.currentTimeMillis(); - setTableAcl(tableName, Sets.newHashSet(user)); - LOG.info("Set HDFS acl when create table {}, cost {} ms", tableName, + setTableAcl(tableName, users); + LOG.info("Set HDFS acl when create or modify table {}, cost {} ms", tableName, System.currentTimeMillis() - start); return true; } catch (Exception e) { - LOG.error("Set HDFS acl error when create table {}", tableName, e); + LOG.error("Set HDFS acl error when create or modify table {}", tableName, e); return false; } } @@ -323,6 +323,7 @@ private void handleNamespaceAcl(Set namespaces, Set users, HDFSAclOperation.OperationType operationType) throws ExecutionException, InterruptedException, IOException { namespaces.removeAll(skipNamespaces); + namespaces.remove(NamespaceDescriptor.SYSTEM_NAMESPACE_NAME_STR); // handle namespace root directories HDFS acls List hdfsAclOperations = new ArrayList<>(); Set skipTableNamespaces = @@ -350,7 +351,8 @@ private void handleNamespaceAcl(Set namespaces, Set users, Set tables = new HashSet<>(); for (String namespace : namespaces) { tables.addAll(admin.listTableDescriptorsByNamespace(Bytes.toBytes(namespace)).stream() - .map(TableDescriptor::getTableName).collect(Collectors.toSet())); + .filter(this::isTableUserScanSnapshotEnabled).map(TableDescriptor::getTableName) + .collect(Collectors.toSet())); } handleTableAcl(tables, users, skipNamespaces, skipTables, operationType); } @@ -400,6 +402,15 @@ private void setTableAcl(TableName tableName, Set users) operationType); } + void createTableDirectories(TableName tableName) throws IOException { + List paths = getTableRootPaths(tableName, false); + for (Path path : paths) { + if (!fs.exists(path)) { + fs.mkdirs(path); + } + } + } + /** * return paths that user will global permission will visit * @return the path list @@ -429,7 +440,7 @@ List getNamespaceRootPaths(String namespace) { * @return the path list * @throws IOException if an error occurred */ - List getTableRootPaths(TableName tableName, boolean includeSnapshotPath) + private List getTableRootPaths(TableName tableName, boolean includeSnapshotPath) throws IOException { List paths = Lists.newArrayList(pathHelper.getTmpTableDir(tableName), pathHelper.getDataTableDir(tableName), pathHelper.getMobTableDir(tableName), @@ -447,28 +458,77 @@ private List getTableSnapshotPaths(TableName tableName) throws IOException .collect(Collectors.toList()); } + /** + * Return users with global read permission + * @return users with global read permission + * @throws IOException if an error occurred + */ + private Set getUsersWithGlobalReadAction() throws IOException { + return getUsersWithReadAction(PermissionStorage.getGlobalPermissions(conf)); + } + /** * Return users with namespace read permission * @param namespace the namespace + * @param includeGlobal true if include users with global read action * @return users with namespace read permission * @throws IOException if an error occurred */ - private Set getUsersWithNamespaceReadAction(String namespace) throws IOException { - return PermissionStorage.getNamespacePermissions(conf, namespace).entries().stream() - .filter(entry -> entry.getValue().getPermission().implies(READ)) - .map(entry -> entry.getKey()).collect(Collectors.toSet()); + Set getUsersWithNamespaceReadAction(String namespace, boolean includeGlobal) + throws IOException { + Set users = + getUsersWithReadAction(PermissionStorage.getNamespacePermissions(conf, namespace)); + if (includeGlobal) { + users.addAll(getUsersWithGlobalReadAction()); + } + return users; } /** * Return users with table read permission * @param tableName the table + * @param includeNamespace true if include users with namespace read action + * @param includeGlobal true if include users with global read action * @return users with table read permission * @throws IOException if an error occurred */ - private Set getUsersWithTableReadAction(TableName tableName) throws IOException { - return PermissionStorage.getTablePermissions(conf, tableName).entries().stream() - .filter(entry -> entry.getValue().getPermission().implies(READ)) - .map(entry -> entry.getKey()).collect(Collectors.toSet()); + Set getUsersWithTableReadAction(TableName tableName, boolean includeNamespace, + boolean includeGlobal) throws IOException { + Set users = + getUsersWithReadAction(PermissionStorage.getTablePermissions(conf, tableName)); + if (includeNamespace) { + users + .addAll(getUsersWithNamespaceReadAction(tableName.getNamespaceAsString(), includeGlobal)); + } + return users; + } + + private Set + getUsersWithReadAction(ListMultimap permissionMultimap) { + return permissionMultimap.entries().stream() + .filter(entry -> checkUserPermission(entry.getValue())).map(entry -> entry.getKey()) + .collect(Collectors.toSet()); + } + + private boolean checkUserPermission(UserPermission userPermission) { + boolean result = containReadAction(userPermission); + if (result && userPermission.getPermission() instanceof TablePermission) { + result = isNotFamilyOrQualifierPermission((TablePermission) userPermission.getPermission()); + } + return result; + } + + boolean containReadAction(UserPermission userPermission) { + return userPermission.getPermission().implies(Permission.Action.READ); + } + + boolean isNotFamilyOrQualifierPermission(TablePermission tablePermission) { + return !tablePermission.hasFamily() && !tablePermission.hasQualifier(); + } + + boolean isTableUserScanSnapshotEnabled(TableDescriptor tableDescriptor) { + String value = tableDescriptor.getValue(USER_SCAN_SNAPSHOT_ENABLE); + return Boolean.valueOf(value); } PathHelper getPathHelper() { diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSnapshotScannerHDFSAclController.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSnapshotScannerHDFSAclController.java index e35e3a1b1d99..e4191eb1666f 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSnapshotScannerHDFSAclController.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSnapshotScannerHDFSAclController.java @@ -125,7 +125,9 @@ public static void setupBeforeClass() throws Exception { path = path.getParent(); } - TEST_UTIL.waitTableAvailable(PermissionStorage.ACL_TABLE_NAME); + SnapshotScannerHDFSAclController coprocessor = TEST_UTIL.getHBaseCluster().getMaster() + .getMasterCoprocessorHost().findCoprocessor(SnapshotScannerHDFSAclController.class); + TEST_UTIL.waitFor(1200000, () -> coprocessor.checkInitialized("check initialized")); } @AfterClass @@ -562,6 +564,53 @@ public void testGrantMobTable() throws Exception { TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, grantUser, snapshot, 6); } } + + @Test + public void testModifyTable() throws Exception { + String namespace = name.getMethodName(); + TableName table = TableName.valueOf(namespace, "t1"); + String snapshot = namespace + "t1"; + + final String grantUserName = name.getMethodName(); + User grantUser = User.createUserForTesting(conf, grantUserName, new String[] {}); + final String grantUserName2 = grantUserName + "2"; + User grantUser2 = User.createUserForTesting(conf, grantUserName2, new String[] {}); + final String grantUserName3 = grantUserName + "3"; + User grantUser3 = User.createUserForTesting(conf, grantUserName3, new String[] {}); + final String nsGrantUserName = grantUserName + "-ns"; + User nsGrantUser = User.createUserForTesting(conf, nsGrantUserName, new String[] {}); + final String globalGrantUserName = grantUserName + "-global"; + User globalGrantUser = User.createUserForTesting(conf, globalGrantUserName, new String[] {}); + final String globalGrantUserName2 = grantUserName + "-global-2"; + User globalGrantUser2 = User.createUserForTesting(conf, globalGrantUserName2, new String[] {}); + + SecureTestUtil.grantGlobal(TEST_UTIL, globalGrantUserName, READ); + SecureTestUtil.grantOnNamespace(TEST_UTIL, nsGrantUserName, namespace, READ); + TableDescriptor td = TestHDFSAclHelper.createUserScanSnapshotDisabledTable(TEST_UTIL, table); + admin.snapshot(snapshot, table); + SecureTestUtil.grantGlobal(TEST_UTIL, globalGrantUserName2, READ); + TestHDFSAclHelper.grantOnTable(TEST_UTIL, grantUserName, table, READ); + SecureTestUtil.grantOnTable(TEST_UTIL, grantUserName2, table, TestHDFSAclHelper.COLUMN1, null, + READ); + TestHDFSAclHelper.grantOnTable(TEST_UTIL, grantUserName3, table, WRITE); + TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, grantUser, snapshot, -1); + TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, grantUser2, snapshot, -1); + TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, grantUser3, snapshot, -1); + TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, nsGrantUser, snapshot, -1); + // Global permission is set before table is created, the acl is inherited + TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, globalGrantUser, snapshot, 6); + // Global permission is set after table is created, the table dir acl is skip + TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, globalGrantUser2, snapshot, -1); + + // enable user scan snapshot + admin.modifyTable(TableDescriptorBuilder.newBuilder(td) + .setValue(SnapshotScannerHDFSAclHelper.USER_SCAN_SNAPSHOT_ENABLE, "true").build()); + TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, grantUser, snapshot, 6); + TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, grantUser2, snapshot, -1); + TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, grantUser3, snapshot, -1); + TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, nsGrantUser, snapshot, 6); + TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, globalGrantUser, snapshot, 6); + } } final class TestHDFSAclHelper { @@ -587,11 +636,8 @@ private static void createNamespace(HBaseTestingUtility util, String namespace) static Table createTable(HBaseTestingUtility util, TableName tableName) throws IOException { createNamespace(util, tableName.getNamespaceAsString()); - TableDescriptor td = TableDescriptorBuilder.newBuilder(tableName) - .setColumnFamily(ColumnFamilyDescriptorBuilder.newBuilder(COLUMN1).build()) - .setColumnFamily(ColumnFamilyDescriptorBuilder.newBuilder(COLUMN2).build()) - .setOwner(User.createUserForTesting(util.getConfiguration(), "owner", new String[] {})) - .build(); + TableDescriptor td = getTableDescriptorBuilder(util, tableName) + .setValue(SnapshotScannerHDFSAclHelper.USER_SCAN_SNAPSHOT_ENABLE, "true").build(); byte[][] splits = new byte[][] { Bytes.toBytes("2"), Bytes.toBytes("4") }; return util.createTable(td, splits); } @@ -604,11 +650,30 @@ static Table createMobTable(HBaseTestingUtility util, TableName tableName) throw .setColumnFamily(ColumnFamilyDescriptorBuilder.newBuilder(COLUMN2).setMobEnabled(true) .setMobThreshold(0).build()) .setOwner(User.createUserForTesting(util.getConfiguration(), "owner", new String[] {})) - .build(); + .setValue(SnapshotScannerHDFSAclHelper.USER_SCAN_SNAPSHOT_ENABLE, "true").build(); byte[][] splits = new byte[][] { Bytes.toBytes("2"), Bytes.toBytes("4") }; return util.createTable(td, splits); } + static TableDescriptor createUserScanSnapshotDisabledTable(HBaseTestingUtility util, + TableName tableName) throws IOException { + createNamespace(util, tableName.getNamespaceAsString()); + TableDescriptor td = getTableDescriptorBuilder(util, tableName).build(); + byte[][] splits = new byte[][] { Bytes.toBytes("2"), Bytes.toBytes("4") }; + try (Table t = util.createTable(td, splits)) { + put(t); + } + return td; + } + + private static TableDescriptorBuilder getTableDescriptorBuilder(HBaseTestingUtility util, + TableName tableName) { + return TableDescriptorBuilder.newBuilder(tableName) + .setColumnFamily(ColumnFamilyDescriptorBuilder.newBuilder(COLUMN1).build()) + .setColumnFamily(ColumnFamilyDescriptorBuilder.newBuilder(COLUMN2).build()) + .setOwner(User.createUserForTesting(util.getConfiguration(), "owner", new String[] {})); + } + static void createTableAndPut(HBaseTestingUtility util, TableName tableNam) throws IOException { try (Table t = createTable(util, tableNam)) { put(t);