Skip to content

HDFS-14856. Fetch file ACLs while mounting external store. #1478

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Oct 14, 2019
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,9 @@ public class DFSConfigKeys extends CommonConfigurationKeys {
public static final String DFS_PROVIDED_ALIASMAP_TEXT_WRITE_DIR_DEFAULT = "file:///tmp/";

public static final String DFS_PROVIDED_ALIASMAP_LEVELDB_PATH = "dfs.provided.aliasmap.leveldb.path";
public static final String DFS_PROVIDED_ACLS_IMPORT_ENABLED =
"dfs.provided.acls.import.enabled";
public static final boolean DFS_PROVIDED_ACLS_IMPORT_ENABLED_DEFAULT = false;

public static final String DFS_LIST_LIMIT = "dfs.ls.limit";
public static final int DFS_LIST_LIMIT_DEFAULT = 1000;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5357,6 +5357,17 @@
</description>
</property>

<property>
<name>dfs.provided.acls.import.enabled</name>
<value>false</value>
<description>
Set to true to inherit ACLs (Access Control Lists) from remote stores
during mount. Disabled by default, i.e., ACLs are not inherited from
remote stores. Note had HDFS ACLs have to be enabled
(dfs.namenode.acls.enabled must be set to true) for this to take effect.
</description>
</property>

<property>
<name>dfs.provided.aliasmap.load.retries</name>
<value>0</value>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,14 @@
import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.fs.permission.AclStatus;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_ACLS_ENABLED_DEFAULT;
import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_ACLS_ENABLED_KEY;
import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_PROVIDED_ACLS_IMPORT_ENABLED;
import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_PROVIDED_ACLS_IMPORT_ENABLED_DEFAULT;

/**
* Traversal of an external FileSystem.
Expand All @@ -37,12 +45,28 @@
@InterfaceStability.Unstable
public class FSTreeWalk extends TreeWalk {

public static final Logger LOG =
LoggerFactory.getLogger(FSTreeWalk.class);

private final Path root;
private final FileSystem fs;
private final boolean enableACLs;

public FSTreeWalk(Path root, Configuration conf) throws IOException {
this.root = root;
fs = root.getFileSystem(conf);

boolean mountACLsEnabled = conf.getBoolean(DFS_PROVIDED_ACLS_IMPORT_ENABLED,
DFS_PROVIDED_ACLS_IMPORT_ENABLED_DEFAULT);
boolean localACLsEnabled = conf.getBoolean(DFS_NAMENODE_ACLS_ENABLED_KEY,
DFS_NAMENODE_ACLS_ENABLED_DEFAULT);
if (!localACLsEnabled && mountACLsEnabled) {
LOG.warn("Mount ACLs have been enabled but HDFS ACLs are not. " +
"Disabling ACLs on the mount {}", root);
this.enableACLs = false;
} else {
this.enableACLs = mountACLsEnabled;
}
}

@Override
Expand All @@ -55,7 +79,8 @@ protected Iterable<TreePath> getChildren(TreePath path, long id,
try {
ArrayList<TreePath> ret = new ArrayList<>();
for (FileStatus s : fs.listStatus(path.getFileStatus().getPath())) {
ret.add(new TreePath(s, id, i, fs));
AclStatus aclStatus = getAclStatus(fs, s.getPath());
ret.add(new TreePath(s, id, i, fs, aclStatus));
}
return ret;
} catch (FileNotFoundException e) {
Expand All @@ -71,20 +96,19 @@ private FSTreeIterator() {
}

FSTreeIterator(TreePath p) {
getPendingQueue().addFirst(
new TreePath(p.getFileStatus(), p.getParentId(), this, fs));
this(p.getFileStatus(), p.getParentId());
}

FSTreeIterator(Path p) throws IOException {
FSTreeIterator(FileStatus fileStatus, long parentId) {
Path path = fileStatus.getPath();
AclStatus acls;
try {
FileStatus s = fs.getFileStatus(root);
getPendingQueue().addFirst(new TreePath(s, -1L, this, fs));
} catch (FileNotFoundException e) {
if (p.equals(root)) {
throw e;
}
throw new ConcurrentModificationException("FS modified");
acls = getAclStatus(fs, path);
} catch (IOException e) {
throw new RuntimeException(e);
}
TreePath treePath = new TreePath(fileStatus, parentId, this, fs, acls);
getPendingQueue().addFirst(treePath);
}

@Override
Expand All @@ -97,10 +121,16 @@ public TreeIterator fork() {

}

private AclStatus getAclStatus(FileSystem fileSystem, Path path)
throws IOException {
return enableACLs ? fileSystem.getAclStatus(path) : null;
}

@Override
public TreeIterator iterator() {
try {
return new FSTreeIterator(root);
FileStatus s = fs.getFileStatus(root);
return new FSTreeIterator(s, -1L);
} catch (IOException e) {
throw new RuntimeException(e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import org.apache.hadoop.classification.InterfaceStability;
import org.apache.hadoop.conf.Configurable;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.security.UserGroupInformation;

/**
Expand Down Expand Up @@ -73,12 +72,12 @@ public Configuration getConf() {
}

@Override
public String user(FileStatus s) {
public String user(String s) {
return user;
}

@Override
public String group(FileStatus s) {
public String group(String s) {
return group;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import java.io.IOException;

import com.google.common.annotations.VisibleForTesting;
import com.google.protobuf.ByteString;

import org.apache.hadoop.classification.InterfaceAudience;
Expand All @@ -27,6 +28,7 @@
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Options;
import org.apache.hadoop.fs.PathHandle;
import org.apache.hadoop.fs.permission.AclStatus;
import org.apache.hadoop.hdfs.protocol.HdfsConstants;
import org.apache.hadoop.hdfs.protocol.proto.HdfsProtos.BlockProto;
import org.apache.hadoop.hdfs.server.common.FileRegion;
Expand All @@ -52,32 +54,47 @@ public class TreePath {
private final FileStatus stat;
private final TreeWalk.TreeIterator i;
private final FileSystem fs;
private final AclStatus acls;

protected TreePath(FileStatus stat, long parentId, TreeWalk.TreeIterator i,
FileSystem fs) {
@VisibleForTesting
public TreePath(FileStatus stat, long parentId, TreeWalk.TreeIterator i) {
this(stat, parentId, i, null, null);
}

public TreePath(FileStatus stat, long parentId, TreeWalk.TreeIterator i,
FileSystem fs, AclStatus acls) {
this.i = i;
this.stat = stat;
this.parentId = parentId;
this.fs = fs;
this.acls = acls;
}

public FileStatus getFileStatus() {
return stat;
}

public AclStatus getAclStatus() {
return acls;
}

public long getParentId() {
return parentId;
}

public TreeWalk.TreeIterator getIterator() {
return i;
}

public long getId() {
if (id < 0) {
throw new IllegalStateException();
}
return id;
}

void accept(long id) {
this.id = id;
public void accept(long pathId) {
this.id = pathId;
i.onAccept(this, id);
}

Expand Down Expand Up @@ -121,14 +138,14 @@ void writeBlock(long blockId, long offset, long length, long genStamp,
INode toFile(UGIResolver ugi, BlockResolver blk,
BlockAliasMap.Writer<FileRegion> out) throws IOException {
final FileStatus s = getFileStatus();
ugi.addUser(s.getOwner());
ugi.addGroup(s.getGroup());
final AclStatus aclStatus = getAclStatus();
long permissions = ugi.getPermissionsProto(s, aclStatus);
INodeFile.Builder b = INodeFile.newBuilder()
.setReplication(blk.getReplication(s))
.setModificationTime(s.getModificationTime())
.setAccessTime(s.getAccessTime())
.setPreferredBlockSize(blk.preferredBlockSize(s))
.setPermission(ugi.resolve(s))
.setPermission(permissions)
.setStoragePolicyID(HdfsConstants.PROVIDED_STORAGE_POLICY_ID);

// pathhandle allows match as long as the file matches exactly.
Expand All @@ -141,7 +158,11 @@ INode toFile(UGIResolver ugi, BlockResolver blk,
"Exact path handle not supported by filesystem " + fs.toString());
}
}
// TODO: storage policy should be configurable per path; use BlockResolver
if (aclStatus != null) {
throw new UnsupportedOperationException(
"ACLs not supported by ImageWriter");
}
//TODO: storage policy should be configurable per path; use BlockResolver
long off = 0L;
for (BlockProto block : blk.resolve(s)) {
b.addBlocks(block);
Expand All @@ -159,13 +180,17 @@ INode toFile(UGIResolver ugi, BlockResolver blk,

INode toDirectory(UGIResolver ugi) {
final FileStatus s = getFileStatus();
ugi.addUser(s.getOwner());
ugi.addGroup(s.getGroup());
final AclStatus aclStatus = getAclStatus();
long permissions = ugi.getPermissionsProto(s, aclStatus);
INodeDirectory.Builder b = INodeDirectory.newBuilder()
.setModificationTime(s.getModificationTime())
.setNsQuota(DEFAULT_NAMESPACE_QUOTA)
.setDsQuota(DEFAULT_STORAGE_SPACE_QUOTA)
.setPermission(ugi.resolve(s));
.setPermission(permissions);
if (aclStatus != null) {
throw new UnsupportedOperationException(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, this is not currently written to the image?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I am proposing to fix the scanner in the PR which is independent of the ImageWriter. The consumer of the scanner can choose to ignore the ACL.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you make this explicit (e.g,, add javadoc for this method)?

"ACLs not supported by ImageWriter");
}
INode.Builder ib = INode.newBuilder()
.setType(INode.Type.DIRECTORY)
.setId(id)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public abstract class TreeIterator implements Iterator<TreePath> {

private final Deque<TreePath> pending;

TreeIterator() {
public TreeIterator() {
this(new ArrayDeque<TreePath>());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@
import org.apache.hadoop.classification.InterfaceAudience;
import org.apache.hadoop.classification.InterfaceStability;
import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.permission.AclEntry;
import org.apache.hadoop.fs.permission.AclEntryType;
import org.apache.hadoop.fs.permission.AclStatus;
import org.apache.hadoop.fs.permission.FsPermission;

/**
Expand All @@ -34,9 +37,9 @@
@InterfaceStability.Unstable
public abstract class UGIResolver {

static final int USER_STRID_OFFSET = 40;
static final int GROUP_STRID_OFFSET = 16;
static final long USER_GROUP_STRID_MASK = (1 << 24) - 1;
public static final int USER_STRID_OFFSET = 40;
public static final int GROUP_STRID_OFFSET = 16;
public static final long USER_GROUP_STRID_MASK = (1 << 24) - 1;

/**
* Permission is serialized as a 64-bit long. [0:24):[25:48):[48:64) (in Big
Expand Down Expand Up @@ -117,19 +120,77 @@ protected void resetUGInfo() {
}

public long resolve(FileStatus s) {
return buildPermissionStatus(user(s), group(s), permission(s).toShort());
String resolvedGroup = group(s.getGroup());
String resolvedOwner = user(s.getOwner());
FsPermission resolvedPermission = permission(s.getPermission());
return buildPermissionStatus(
resolvedOwner, resolvedGroup, resolvedPermission.toShort());
}

public String user(FileStatus s) {
return s.getOwner();
private long resolve(AclStatus aclStatus) {
String resolvedOwner = user(aclStatus.getOwner());
String resolvedGroup = group(aclStatus.getGroup());
FsPermission resolvedPermision = permission(aclStatus.getPermission());
return buildPermissionStatus(
resolvedOwner, resolvedGroup, resolvedPermision.toShort());
}

public String group(FileStatus s) {
return s.getGroup();
protected String user(String s) {
return s;
}

public FsPermission permission(FileStatus s) {
return s.getPermission();
protected String group(String s) {
return s;
}

public FsPermission permission(FsPermission s) {
return s;
}

/**
* Get the serialized, local permissions for the external
* {@link FileStatus} or {@link AclStatus}. {@code remoteAcl} is used when it
* is not null, otherwise {@code remoteStatus} is used.
*
* @param remoteStatus FileStatus on remote store.
* @param remoteAcl AclStatus on external store.
* @return serialized, local permissions the FileStatus or AclStatus map to.
*/
public long getPermissionsProto(FileStatus remoteStatus,
AclStatus remoteAcl) {
addUGI(remoteStatus, remoteAcl);
if (remoteAcl == null) {
return resolve(remoteStatus);
} else {
return resolve(remoteAcl);
}
}

/**
* Add the users and groups specified by the given {@link FileStatus} and
* {@link AclStatus}.
*
* @param remoteStatus
* @param remoteAcl
*/
private void addUGI(FileStatus remoteStatus, AclStatus remoteAcl) {
if (remoteAcl != null) {
addUser(remoteAcl.getOwner());
addGroup(remoteAcl.getGroup());
for (AclEntry entry : remoteAcl.getEntries()) {
// add the users and groups in this acl entry to ugi
String name = entry.getName();
if (name != null) {
if (entry.getType() == AclEntryType.USER) {
addUser(name);
} else if (entry.getType() == AclEntryType.GROUP) {
addGroup(name);
}
}
}
} else {
addUser(remoteStatus.getOwner());
addGroup(remoteStatus.getGroup());
}
}
}
Loading