From fec069df7fa5b655c1fab7d2f23c5f77635cf9b9 Mon Sep 17 00:00:00 2001 From: meiyi Date: Tue, 25 Jun 2019 12:00:39 +0800 Subject: [PATCH] HBASE-22580 Add a table attribute to make user scan snapshot feature configurable for table --- .../apache/hadoop/hbase/HTableDescriptor.java | 10 + .../hadoop/hbase/client/TableDescriptor.java | 7 + .../hbase/client/TableDescriptorBuilder.java | 31 ++++ .../security/access/PermissionStorage.java | 5 + .../SnapshotScannerHDFSAclController.java | 174 +++++++++++------- .../access/SnapshotScannerHDFSAclHelper.java | 74 ++++++-- .../TestSnapshotScannerHDFSAclController.java | 70 ++++++- 7 files changed, 281 insertions(+), 90 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java index 8866eba94fc1..160876cad02a 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java @@ -527,6 +527,16 @@ public boolean hasFamily(final byte [] familyName) { return delegatee.hasColumnFamily(familyName); } + /** + * Check if user scan snapshot enable flag of the table is true. If flag is false then + * SnapshotScannerHDFSAclController won't handle HDFS acls for users with table read permission + * @return true if user scan snapshot is enabled for this table + */ + @Override + public boolean isUserScanSnapshotEnabled() { + return delegatee.isUserScanSnapshotEnabled(); + } + /** * @return Name of this table and then a map of all of the column family * descriptors. diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableDescriptor.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableDescriptor.java index fc5e69e88c4a..9a266e0594d2 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableDescriptor.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableDescriptor.java @@ -345,4 +345,11 @@ default boolean matchReplicationScope(boolean enabled) { } return !enabled; } + + /** + * Check if user scan snapshot enable flag of the table is true. If flag is false then + * SnapshotScannerHDFSAclController won't handle HDFS acls for users with table read permission + * @return true if user scan snapshot is enabled for this table + */ + boolean isUserScanSnapshotEnabled(); } diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableDescriptorBuilder.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableDescriptorBuilder.java index 037a7f860cbf..02a77daf36e8 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableDescriptorBuilder.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableDescriptorBuilder.java @@ -188,6 +188,11 @@ public class TableDescriptorBuilder { private static final Bytes PRIORITY_KEY = new Bytes(Bytes.toBytes(PRIORITY)); + @InterfaceAudience.Private + public static final String USER_SCAN_SNAPSHOT_ENABLED = "USER_SCAN_SNAPSHOT_ENABLED"; + private static final Bytes USER_SCAN_SNAPSHOT_ENABLED_KEY = + new Bytes(Bytes.toBytes(USER_SCAN_SNAPSHOT_ENABLED)); + /** * Relative priority of the table used for rpc scheduling */ @@ -537,6 +542,11 @@ public TableDescriptorBuilder setReplicationScope(int scope) { return this; } + public TableDescriptorBuilder setUserScanSnapshotEnabled(boolean isEnable) { + desc.setUserScanSnapshotEnabled(isEnable); + return this; + } + public TableDescriptor build() { return new ModifyableTableDescriptor(desc); } @@ -1074,6 +1084,27 @@ public boolean hasColumnFamily(final byte[] familyName) { return families.containsKey(familyName); } + /** + * Setting the user scan snapshot enable flag. + * + * @param isEnabled True if enable user scan snapshot. + * @return the modifyable TD + */ + public ModifyableTableDescriptor setUserScanSnapshotEnabled(boolean isEnabled) { + return setValue(USER_SCAN_SNAPSHOT_ENABLED_KEY, Boolean.toString(isEnabled)); + } + + /** + * Check if user scan snapshot enable flag of the table is true. If flag is false then + * SnapshotScannerHDFSAclController won't handle HDFS acls for users with table read permission + * + * @return true if user scan snapshot is enabled for this table + */ + @Override + public boolean isUserScanSnapshotEnabled() { + return getOrDefault(USER_SCAN_SNAPSHOT_ENABLED_KEY, Boolean::valueOf, false); + } + /** * @return Name of this table and then a map of all of the column family descriptors. */ 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..3da8bf01e41a 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 @@ -25,6 +25,7 @@ import java.util.List; import java.util.Optional; import java.util.Set; +import java.util.function.Supplier; import java.util.stream.Collectors; import org.apache.hadoop.conf.Configuration; @@ -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 @@ -151,6 +156,8 @@ public void postStartMaster(ObserverContext c) thr .setColumnFamily(ColumnFamilyDescriptorBuilder .newBuilder(SnapshotScannerHDFSAclStorage.HDFS_ACL_FAMILY).build()); admin.modifyTable(builder.build()); + } else { + aclTableInitialized = true; } } else { throw new TableNotFoundException("Table " + PermissionStorage.ACL_TABLE_NAME @@ -163,7 +170,7 @@ public void postStartMaster(ObserverContext c) thr @Override public void preStopMaster(ObserverContext c) { - if (checkInitialized()) { + if (initialized) { hdfsAclHelper.close(); } } @@ -171,18 +178,13 @@ public void preStopMaster(ObserverContext c) { @Override public void postCompletedCreateTableAction(ObserverContext c, TableDescriptor desc, RegionInfo[] regions) throws IOException { - if (!desc.getTableName().isSystemTable() && checkInitialized()) { + if (checkTable(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); + hdfsAclHelper.addTableAcl(tableName, Sets.newHashSet(owner)); try (Table aclTable = c.getEnvironment().getConnection().getTable(PermissionStorage.ACL_TABLE_NAME)) { SnapshotScannerHDFSAclStorage.addUserTableHdfsAcl(aclTable, owner, tableName); @@ -193,7 +195,7 @@ public void postCompletedCreateTableAction(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 +208,7 @@ public void postCreateNamespace(ObserverContext c, @Override public void postCompletedSnapshotAction(ObserverContext c, SnapshotDescription snapshot, TableDescriptor tableDescriptor) throws IOException { - if (!tableDescriptor.getTableName().isSystemTable() && checkInitialized()) { + if (checkTable(tableDescriptor, () -> "snapshot " + snapshot.getName())) { hdfsAclHelper.snapshotAcl(snapshot); } } @@ -214,7 +216,7 @@ public void postCompletedSnapshotAction(ObserverContext c, TableName tableName) throws IOException { - if (!tableName.isSystemTable() && checkInitialized()) { + if (checkTable(tableName, () -> "truncateTable " + tableName)) { hdfsAclHelper.resetTableAcl(tableName); } } @@ -222,7 +224,7 @@ public void postCompletedTruncateTableAction(ObserverContext ctx, TableName tableName) throws IOException { - if (!tableName.isSystemTable() && checkInitialized()) { + if (checkTable(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,10 +265,36 @@ public void postDeleteTable(ObserverContext ctx, } } + @Override + public void postModifyTable(ObserverContext ctx, + TableName tableName, TableDescriptor oldDescriptor, TableDescriptor currentDescriptor) + throws IOException { + if (checkTable(currentDescriptor, () -> "modifyTable " + tableName) + && !oldDescriptor.isUserScanSnapshotEnabled()) { + hdfsAclHelper.createTableDirectories(tableName); + Set users = new HashSet<>(); + users.addAll(hdfsAclHelper.getUsersWithGlobalReadAction()); + users.addAll(hdfsAclHelper.getUsersWithNamespaceReadAction(tableName.getNamespaceAsString())); + Set userWithTableReadPermission = + hdfsAclHelper.getUsersWithTableReadAction(tableName); + users.addAll(userWithTableReadPermission); + hdfsAclHelper.addTableAcl(tableName, users); + try (Table aclTable = + ctx.getEnvironment().getConnection().getTable(PermissionStorage.ACL_TABLE_NAME)) { + for (String user : userWithTableReadPermission) { + SnapshotScannerHDFSAclStorage.addUserTableHdfsAcl(aclTable, user, tableName); + } + } + } else if (!aclTableInitialized + && Bytes.equals(PermissionStorage.ACL_GLOBAL_NAME, tableName.getName())) { + aclTableInitialized = true; + } + } + @Override public void postDeleteNamespace(ObserverContext ctx, String namespace) throws IOException { - if (checkInitialized()) { + if (checkInitialized(() -> "deleteNamespace " + namespace)) { try (Table aclTable = ctx.getEnvironment().getConnection().getTable(PermissionStorage.ACL_TABLE_NAME)) { SnapshotScannerHDFSAclStorage.deleteNamespaceHdfsAcl(aclTable, namespace); @@ -292,7 +320,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 +331,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 +351,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,23 +365,20 @@ 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; - } - } - if (tPerm != null && containReadPermission(tPerm)) { - if (!isHdfsAclSet(aclTable, userName, tableName)) { - hdfsAclHelper.grantAcl(userPermission, new HashSet<>(0), new HashSet<>(0)); + TablePermission tablePerm = (TablePermission) userPermission.getPermission(); + if (checkTableAndPermission(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); } - 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: @@ -365,7 +391,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 +399,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 +407,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 (checkTableAndPermission(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 +470,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 + .checkTablePermission((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 +527,32 @@ private boolean isHdfsAclSet(Table aclTable, String userName, String namespace, return isSet; } - private boolean checkInitialized() { + @VisibleForTesting + boolean checkInitialized(Supplier 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.get()); + } } + return false; + } + + private boolean checkTableAndPermission(TablePermission tablePermission) throws IOException { + return checkTable(tablePermission.getTableName(), () -> "") + && hdfsAclHelper.checkTablePermission(tablePermission); + } + + private boolean checkTable(TableName tableName, Supplier operation) throws IOException { + return !tableName.isSystemTable() && checkInitialized(operation) + && masterServices.getTableDescriptors().get(tableName).isUserScanSnapshotEnabled(); + } + + private boolean checkTable(TableDescriptor tableDescriptor, Supplier operation) { + TableName tableName = tableDescriptor.getTableName(); + return !tableName.isSystemTable() && checkInitialized(operation) + && tableDescriptor.isUserScanSnapshotEnabled(); } private User getActiveUser(ObserverContext ctx) 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..0be52072121c 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; @@ -260,20 +261,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 +324,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 +352,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(TableDescriptor::isUserScanSnapshotEnabled).map(TableDescriptor::getTableName) + .collect(Collectors.toSet())); } handleTableAcl(tables, users, skipNamespaces, skipTables, operationType); } @@ -400,6 +403,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 +441,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,16 +459,23 @@ 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 + */ + Set getUsersWithGlobalReadAction() throws IOException { + return getUsersWithReadAction(PermissionStorage.getGlobalPermissions(conf)); + } + /** * Return users with namespace read permission * @param namespace the namespace * @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) throws IOException { + return getUsersWithReadAction(PermissionStorage.getNamespacePermissions(conf, namespace)); } /** @@ -465,10 +484,31 @@ private Set getUsersWithNamespaceReadAction(String namespace) throws IOE * @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) throws IOException { + return getUsersWithReadAction(PermissionStorage.getTablePermissions(conf, tableName)); + } + + private Set + getUsersWithReadAction(ListMultimap permissionMultimap) { + return permissionMultimap.entries().stream() + .filter(entry -> checkUserPermission(entry.getValue())).map(entry -> entry.getKey()) + .collect(Collectors.toSet()); + } + + boolean checkUserPermission(UserPermission userPermission) { + boolean result = containReadAction(userPermission); + if (result && userPermission.getPermission() instanceof TablePermission) { + result = checkTablePermission((TablePermission) userPermission.getPermission()); + } + return result; + } + + boolean containReadAction(UserPermission userPermission) { + return userPermission.getPermission().implies(Permission.Action.READ); + } + + boolean checkTablePermission(TablePermission tablePermission) { + return !tablePermission.hasFamily() && !tablePermission.hasQualifier(); } 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..32f9ff6e2070 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).setUserScanSnapshotEnabled(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 { @@ -591,7 +640,7 @@ static Table createTable(HBaseTestingUtility util, TableName tableName) throws I .setColumnFamily(ColumnFamilyDescriptorBuilder.newBuilder(COLUMN1).build()) .setColumnFamily(ColumnFamilyDescriptorBuilder.newBuilder(COLUMN2).build()) .setOwner(User.createUserForTesting(util.getConfiguration(), "owner", new String[] {})) - .build(); + .setUserScanSnapshotEnabled(true).build(); byte[][] splits = new byte[][] { Bytes.toBytes("2"), Bytes.toBytes("4") }; return util.createTable(td, splits); } @@ -604,11 +653,26 @@ 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(); + .setUserScanSnapshotEnabled(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 = TableDescriptorBuilder.newBuilder(tableName) + .setColumnFamily(ColumnFamilyDescriptorBuilder.newBuilder(COLUMN1).build()) + .setColumnFamily(ColumnFamilyDescriptorBuilder.newBuilder(COLUMN2).build()) + .setOwner(User.createUserForTesting(util.getConfiguration(), "owner", new String[] {})) + .build(); + byte[][] splits = new byte[][] { Bytes.toBytes("2"), Bytes.toBytes("4") }; + try (Table t = util.createTable(td, splits)) { + put(t); + } + return td; + } + static void createTableAndPut(HBaseTestingUtility util, TableName tableNam) throws IOException { try (Table t = createTable(util, tableNam)) { put(t);