From ba8ed5b0aef794c0b501980179d499518df0688a Mon Sep 17 00:00:00 2001 From: Steve Loughran Date: Fri, 7 Oct 2016 17:30:25 +0100 Subject: [PATCH 1/4] HADOOP-12774 use UGI.currentUser for user and group of s3a objects --- .../org/apache/hadoop/fs/s3a/Listing.java | 5 ++-- .../apache/hadoop/fs/s3a/S3AFileStatus.java | 21 ++++++++++++-- .../apache/hadoop/fs/s3a/S3AFileSystem.java | 29 ++++++++++++++----- .../org/apache/hadoop/fs/s3a/S3AUtils.java | 8 +++-- 4 files changed, 47 insertions(+), 16 deletions(-) diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Listing.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Listing.java index 4120b20c1a1f4..5d3dba12bd5e0 100644 --- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Listing.java +++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Listing.java @@ -298,7 +298,7 @@ private boolean buildNextStatusBatch(ObjectListing objects) { // Skip over keys that are ourselves and old S3N _$folder$ files if (acceptor.accept(keyPath, summary) && filter.accept(keyPath)) { FileStatus status = createFileStatus(keyPath, summary, - owner.getDefaultBlockSize(keyPath)); + owner.getDefaultBlockSize(keyPath), owner.getUsername()); LOG.debug("Adding: {}", status); stats.add(status); added++; @@ -312,7 +312,8 @@ private boolean buildNextStatusBatch(ObjectListing objects) { for (String prefix : objects.getCommonPrefixes()) { Path keyPath = owner.keyToQualifiedPath(prefix); if (acceptor.accept(keyPath, prefix) && filter.accept(keyPath)) { - FileStatus status = new S3AFileStatus(true, false, keyPath); + FileStatus status = new S3AFileStatus(true, false, keyPath, + owner.getUsername()); LOG.debug("Adding directory: {}", status); added++; stats.add(status); diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileStatus.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileStatus.java index 75a650073b3e8..24b997ff27029 100644 --- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileStatus.java +++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileStatus.java @@ -32,18 +32,24 @@ @InterfaceStability.Evolving public class S3AFileStatus extends FileStatus { private boolean isEmptyDirectory; + private final String owner; // Directories - public S3AFileStatus(boolean isdir, boolean isemptydir, Path path) { + public S3AFileStatus(boolean isdir, + boolean isemptydir, + Path path, + String owner) { super(0, isdir, 1, 0, 0, path); isEmptyDirectory = isemptydir; + this.owner = owner; } // Files public S3AFileStatus(long length, long modification_time, Path path, - long blockSize) { + long blockSize, String owner) { super(length, false, 1, blockSize, modification_time, path); isEmptyDirectory = false; + this.owner = owner; } public boolean isEmptyDirectory() { @@ -52,7 +58,16 @@ public boolean isEmptyDirectory() { @Override public String getOwner() { - return System.getProperty("user.name"); + return owner; + } + + /** + * The group of an S3A entry is the same as the owner + * @return the owner. + */ + @Override + public String getGroup() { + return owner; } /** Compare if this object is equal to another object. diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java index 0e899e3e7051e..4dd00e06418a5 100644 --- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java +++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java @@ -86,6 +86,7 @@ import org.apache.hadoop.fs.StorageStatistics; import org.apache.hadoop.fs.permission.FsPermission; import org.apache.hadoop.fs.s3native.S3xLoginHelper; +import org.apache.hadoop.security.UserGroupInformation; import org.apache.hadoop.util.Progressable; import org.apache.hadoop.util.ReflectionUtils; @@ -119,6 +120,7 @@ public class S3AFileSystem extends FileSystem { public static final int DEFAULT_BLOCKSIZE = 32 * 1024 * 1024; private URI uri; private Path workingDir; + private String username; private AmazonS3 s3; private String bucket; private int maxKeys; @@ -159,7 +161,9 @@ public void initialize(URI name, Configuration conf) throws IOException { instrumentation = new S3AInstrumentation(name); uri = S3xLoginHelper.buildFSURI(name); - workingDir = new Path("/user", System.getProperty("user.name")) + // Username is the current user at the time the FS was instantiated. + username = UserGroupInformation.getCurrentUser().getShortUserName(); + workingDir = new Path("/user", username) .makeQualified(this.uri, this.getWorkingDirectory()); bucket = name.getHost(); @@ -1386,6 +1390,14 @@ public Path getWorkingDirectory() { return workingDir; } + /** + * Get the username of the FS + * @return the short name of the user who instantiated the FS + */ + public String getUsername() { + return username; + } + /** * * Make the given path and all non-existent parents into @@ -1477,14 +1489,14 @@ public S3AFileStatus getFileStatus(final Path f) throws IOException { if (objectRepresentsDirectory(key, meta.getContentLength())) { LOG.debug("Found exact file: fake directory"); - return new S3AFileStatus(true, true, - path); + return new S3AFileStatus(true, true, path, username); } else { LOG.debug("Found exact file: normal file"); return new S3AFileStatus(meta.getContentLength(), dateToLong(meta.getLastModified()), path, - getDefaultBlockSize(path)); + getDefaultBlockSize(path), + username); } } catch (AmazonServiceException e) { if (e.getStatusCode() != 404) { @@ -1502,7 +1514,7 @@ public S3AFileStatus getFileStatus(final Path f) throws IOException { if (objectRepresentsDirectory(newKey, meta.getContentLength())) { LOG.debug("Found file (with /): fake directory"); - return new S3AFileStatus(true, true, path); + return new S3AFileStatus(true, true, path, username); } else { LOG.warn("Found file (with /): real file? should not happen: {}", key); @@ -1510,7 +1522,8 @@ public S3AFileStatus getFileStatus(final Path f) throws IOException { return new S3AFileStatus(meta.getContentLength(), dateToLong(meta.getLastModified()), path, - getDefaultBlockSize(path)); + getDefaultBlockSize(path), + username); } } catch (AmazonServiceException e) { if (e.getStatusCode() != 404) { @@ -1547,10 +1560,10 @@ public S3AFileStatus getFileStatus(final Path f) throws IOException { } } - return new S3AFileStatus(true, false, path); + return new S3AFileStatus(true, false, path, username); } else if (key.isEmpty()) { LOG.debug("Found root directory"); - return new S3AFileStatus(true, true, path); + return new S3AFileStatus(true, true, path, username); } } catch (AmazonServiceException e) { if (e.getStatusCode() != 404) { diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AUtils.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AUtils.java index c89f6904cdaa6..9cf4eb886d5b8 100644 --- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AUtils.java +++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AUtils.java @@ -236,17 +236,19 @@ public static String stringify(AmazonS3Exception e) { * @param keyPath path to entry * @param summary summary from AWS * @param blockSize block size to declare. + * @param owner owner of the file * @return a status entry */ public static S3AFileStatus createFileStatus(Path keyPath, S3ObjectSummary summary, - long blockSize) { + long blockSize, + String owner) { if (objectRepresentsDirectory(summary.getKey(), summary.getSize())) { - return new S3AFileStatus(true, true, keyPath); + return new S3AFileStatus(true, true, keyPath, owner); } else { return new S3AFileStatus(summary.getSize(), dateToLong(summary.getLastModified()), keyPath, - blockSize); + blockSize, owner); } } From 3a92100fcea6d24dfaa259a63de8bde942fde988 Mon Sep 17 00:00:00 2001 From: Steve Loughran Date: Mon, 10 Oct 2016 12:47:55 +0100 Subject: [PATCH 2/4] Patch 002: create a fake user, create an FS and verify that the user and owner on the root path is that username --- .../hadoop/fs/s3a/ITestS3AConfiguration.java | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AConfiguration.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AConfiguration.java index 30d4bf66baf53..302c15f9465e9 100644 --- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AConfiguration.java +++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AConfiguration.java @@ -44,8 +44,11 @@ import java.io.File; import java.net.URI; +import java.security.PrivilegedAction; +import java.security.PrivilegedExceptionAction; import org.apache.hadoop.security.ProviderUtils; +import org.apache.hadoop.security.UserGroupInformation; import org.apache.hadoop.security.alias.CredentialProvider; import org.apache.hadoop.security.alias.CredentialProviderFactory; import org.apache.hadoop.util.VersionInfo; @@ -436,7 +439,7 @@ public void testDirectoryAllocatorRR() throws Throwable { dir1.mkdirs(); dir2.mkdirs(); conf = new Configuration(); - conf.set(Constants.BUFFER_DIR, dir1 +", " + dir2); + conf.set(Constants.BUFFER_DIR, dir1 + ", " + dir2); fs = S3ATestUtils.createTestFileSystem(conf); File tmp1 = fs.createTmpFileForWrite("out-", 1024, conf); tmp1.delete(); @@ -446,6 +449,25 @@ public void testDirectoryAllocatorRR() throws Throwable { tmp1.getParent(), tmp2.getParent()); } + @Test + public void testUsernameFromUGI() throws Throwable { + final String alice = "alice"; + UserGroupInformation fakeUser = + UserGroupInformation.createUserForTesting(alice, + new String[]{"users", "administrators"}); + conf = new Configuration(); + fs = fakeUser.doAs(new PrivilegedExceptionAction() { + @Override + public S3AFileSystem run() throws Exception{ + return S3ATestUtils.createTestFileSystem(conf); + } + }); + assertEquals("username", alice, fs.getUsername()); + S3AFileStatus status = fs.getFileStatus(new Path("/")); + assertEquals("owner in " + status, alice, status.getOwner()); + assertEquals("group in " + status, alice, status.getGroup()); + } + /** * Reads and returns a field from an object using reflection. If the field * cannot be found, is null, or is not the expected type, then this method From f9dae32f8d0bcc5052f232d4b6858f633bb991a7 Mon Sep 17 00:00:00 2001 From: Steve Loughran Date: Wed, 12 Oct 2016 15:05:11 +0100 Subject: [PATCH 3/4] HADOOP-12774: Address Chris's comments about using parent class fields. Also, given that the first contructor for S3AFileStatus is only for directories, the isdir parameter is always true, hence superflous. Cut and add javadocs for both constructors. --- .../org/apache/hadoop/fs/s3a/Listing.java | 2 +- .../apache/hadoop/fs/s3a/S3AFileStatus.java | 42 +++++++++---------- .../apache/hadoop/fs/s3a/S3AFileSystem.java | 8 ++-- .../org/apache/hadoop/fs/s3a/S3AUtils.java | 2 +- 4 files changed, 26 insertions(+), 28 deletions(-) diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Listing.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Listing.java index 5d3dba12bd5e0..30d8e6f37d031 100644 --- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Listing.java +++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Listing.java @@ -312,7 +312,7 @@ private boolean buildNextStatusBatch(ObjectListing objects) { for (String prefix : objects.getCommonPrefixes()) { Path keyPath = owner.keyToQualifiedPath(prefix); if (acceptor.accept(keyPath, prefix) && filter.accept(keyPath)) { - FileStatus status = new S3AFileStatus(true, false, keyPath, + FileStatus status = new S3AFileStatus(false, keyPath, owner.getUsername()); LOG.debug("Adding directory: {}", status); added++; diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileStatus.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileStatus.java index 24b997ff27029..b0f08e32eff41 100644 --- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileStatus.java +++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileStatus.java @@ -32,44 +32,42 @@ @InterfaceStability.Evolving public class S3AFileStatus extends FileStatus { private boolean isEmptyDirectory; - private final String owner; - // Directories - public S3AFileStatus(boolean isdir, - boolean isemptydir, + /** + * Create a directory status. + * @param isemptydir is this an empty directory? + * @param path the path + * @param owner the owner + */ + public S3AFileStatus(boolean isemptydir, Path path, String owner) { - super(0, isdir, 1, 0, 0, path); + super(0, true, 1, 0, 0, path); isEmptyDirectory = isemptydir; - this.owner = owner; + setOwner(owner); + setGroup(owner); } - // Files + /** + * A simple file. + * @param length file length + * @param modification_time mod time + * @param path path + * @param blockSize block size + * @param owner owner + */ public S3AFileStatus(long length, long modification_time, Path path, long blockSize, String owner) { super(length, false, 1, blockSize, modification_time, path); isEmptyDirectory = false; - this.owner = owner; + setOwner(owner); + setGroup(owner); } public boolean isEmptyDirectory() { return isEmptyDirectory; } - @Override - public String getOwner() { - return owner; - } - - /** - * The group of an S3A entry is the same as the owner - * @return the owner. - */ - @Override - public String getGroup() { - return owner; - } - /** Compare if this object is equal to another object. * @param o the object to be compared. * @return true if two file status has the same path name; false if not. diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java index 4dd00e06418a5..c428610a8deac 100644 --- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java +++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java @@ -1489,7 +1489,7 @@ public S3AFileStatus getFileStatus(final Path f) throws IOException { if (objectRepresentsDirectory(key, meta.getContentLength())) { LOG.debug("Found exact file: fake directory"); - return new S3AFileStatus(true, true, path, username); + return new S3AFileStatus(true, path, username); } else { LOG.debug("Found exact file: normal file"); return new S3AFileStatus(meta.getContentLength(), @@ -1514,7 +1514,7 @@ public S3AFileStatus getFileStatus(final Path f) throws IOException { if (objectRepresentsDirectory(newKey, meta.getContentLength())) { LOG.debug("Found file (with /): fake directory"); - return new S3AFileStatus(true, true, path, username); + return new S3AFileStatus(true, path, username); } else { LOG.warn("Found file (with /): real file? should not happen: {}", key); @@ -1560,10 +1560,10 @@ public S3AFileStatus getFileStatus(final Path f) throws IOException { } } - return new S3AFileStatus(true, false, path, username); + return new S3AFileStatus(false, path, username); } else if (key.isEmpty()) { LOG.debug("Found root directory"); - return new S3AFileStatus(true, true, path, username); + return new S3AFileStatus(true, path, username); } } catch (AmazonServiceException e) { if (e.getStatusCode() != 404) { diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AUtils.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AUtils.java index 9cf4eb886d5b8..6386769d65ad8 100644 --- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AUtils.java +++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AUtils.java @@ -244,7 +244,7 @@ public static S3AFileStatus createFileStatus(Path keyPath, long blockSize, String owner) { if (objectRepresentsDirectory(summary.getKey(), summary.getSize())) { - return new S3AFileStatus(true, true, keyPath, owner); + return new S3AFileStatus(true, keyPath, owner); } else { return new S3AFileStatus(summary.getSize(), dateToLong(summary.getLastModified()), keyPath, From fae8ee7c93129b33b46a5a12b4a0567d0211d724 Mon Sep 17 00:00:00 2001 From: Steve Loughran Date: Thu, 13 Oct 2016 15:53:08 +0100 Subject: [PATCH 4/4] HADOOP-12774: Address checkstyle complaints --- .../src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java | 2 +- .../java/org/apache/hadoop/fs/s3a/ITestS3AConfiguration.java | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java index c428610a8deac..a82fc93663da5 100644 --- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java +++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java @@ -1391,7 +1391,7 @@ public Path getWorkingDirectory() { } /** - * Get the username of the FS + * Get the username of the FS. * @return the short name of the user who instantiated the FS */ public String getUsername() { diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AConfiguration.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AConfiguration.java index 302c15f9465e9..04057a969a17f 100644 --- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AConfiguration.java +++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AConfiguration.java @@ -44,7 +44,6 @@ import java.io.File; import java.net.URI; -import java.security.PrivilegedAction; import java.security.PrivilegedExceptionAction; import org.apache.hadoop.security.ProviderUtils;