HADOOP-14845. Azure wasb: getFileStatus not making any auth check. Contributed by Sivaguru Sankaridurg
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/9288206c Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/9288206c Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/9288206c Branch: refs/heads/HDFS-10467 Commit: 9288206cb3c1a39044a8e106436987185ef43ddf Parents: f702c95 Author: Steve Loughran <ste...@apache.org> Authored: Thu Oct 5 15:05:23 2017 +0100 Committer: Steve Loughran <ste...@apache.org> Committed: Thu Oct 5 15:05:23 2017 +0100 ---------------------------------------------------------------------- .../hadoop/fs/azure/NativeAzureFileSystem.java | 79 ++++++++- .../fs/azure/ITestWasbRemoteCallHelper.java | 2 +- .../TestNativeAzureFileSystemAuthorization.java | 177 +++++++++++++++---- .../ITestAzureFileSystemInstrumentation.java | 6 +- 4 files changed, 220 insertions(+), 44 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/9288206c/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/NativeAzureFileSystem.java ---------------------------------------------------------------------- diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/NativeAzureFileSystem.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/NativeAzureFileSystem.java index 5f86f84..2e5241d 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/NativeAzureFileSystem.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/NativeAzureFileSystem.java @@ -287,7 +287,7 @@ public class NativeAzureFileSystem extends FileSystem { * @param fs file system on which a file is written. * @throws IOException Thrown when fail to write file. */ - public void writeFile(FileSystem fs) throws IOException { + public void writeFile(NativeAzureFileSystem fs) throws IOException { Path path = getRenamePendingFilePath(); LOG.debug("Preparing to write atomic rename state to {}", path.toString()); OutputStream output = null; @@ -296,7 +296,7 @@ public class NativeAzureFileSystem extends FileSystem { // Write file. try { - output = fs.create(path); + output = fs.createInternal(path, FsPermission.getFileDefault(), false, null); output.write(contents.getBytes(Charset.forName("UTF-8"))); } catch (IOException e) { throw new IOException("Unable to write RenamePending file for folder rename from " @@ -561,7 +561,7 @@ public class NativeAzureFileSystem extends FileSystem { if (!sourceFolderGone) { // Make sure the target folder exists. Path dst = fullPath(dstKey); - if (!fs.exists(dst)) { + if (!fs.existsInternal(dst)) { fs.mkdirs(dst); } @@ -1655,6 +1655,10 @@ public class NativeAzureFileSystem extends FileSystem { // At this point, we have exclusive access to the source folder // via the lease, so we will not conflict with an active folder // rename operation. + // + // In the secure case, the call to exists will happen in the context + // of the user that initiated the operation. In this case, we should + // do the auth-check against ranger for the path. if (!exists(parent)) { try { @@ -1750,6 +1754,30 @@ public class NativeAzureFileSystem extends FileSystem { performAuthCheck(ancestor, WasbAuthorizationOperations.WRITE, "create", absolutePath); + return createInternal(f, permission, overwrite, parentFolderLease); + } + + + /** + * This is the version of the create call that is meant for internal usage. + * This version is not public facing and does not perform authorization checks. + * It is used by the public facing create call and by FolderRenamePending to + * create the internal -RenamePending.json file. + * @param f the path to a file to be created. + * @param permission for the newly created file. + * @param overwrite specifies if the file should be overwritten. + * @param parentFolderLease lease on the parent folder. + * @return the output stream used to write data into the newly created file . + * @throws IOException if an IO error occurs while attempting to delete the + * path. + * + */ + protected FSDataOutputStream createInternal(Path f, FsPermission permission, + boolean overwrite, + SelfRenewingLease parentFolderLease) + throws FileAlreadyExistsException, IOException { + + Path absolutePath = makeAbsolute(f); String key = pathToKey(absolutePath); FileMetadata existingMetadata = store.retrieveMetadata(key); @@ -2589,6 +2617,49 @@ public class NativeAzureFileSystem extends FileSystem { // Capture the absolute path and the path to key. Path absolutePath = makeAbsolute(f); + + if (!isRenamePendingFile(absolutePath)) { + Path ancestor = getAncestor(absolutePath); + if (ancestor.equals(absolutePath) && !ancestor.equals(new Path("/"))) { + performAuthCheck(ancestor.getParent(), WasbAuthorizationOperations.READ, + "getFileStatus", absolutePath); + } + else { + performAuthCheck(ancestor, WasbAuthorizationOperations.READ, + "getFileStatus", absolutePath); + } + } + + return getFileStatusInternal(f); + } + + /** + * Checks if a given path exists in the filesystem. + * Calls getFileStatusInternal and has the same costs + * as the public facing exists call. + * This internal version of the exists call does not perform + * authorization checks, and is used internally by various filesystem + * operations that need to check if the parent/ancestor/path exist. + * The idea is to avoid having to configure authorization policies for + * these internal calls. + * @param f the path to a file or directory. + * @return true if path exists; otherwise false. + * @throws IOException if an IO error occurs while attempting to check + * for existence of the path. + * + */ + protected boolean existsInternal(Path f) throws IOException { + try { + this.getFileStatusInternal(f); + return true; + } catch (FileNotFoundException fnfe) { + return false; + } + } + + protected FileStatus getFileStatusInternal(Path f) throws FileNotFoundException, IOException { + + Path absolutePath = makeAbsolute(f); String key = pathToKey(absolutePath); if (key.length() == 0) { // root always exists return newDirectory(null, absolutePath); @@ -2654,7 +2725,7 @@ public class NativeAzureFileSystem extends FileSystem { // Check if there is a -RenamePending.json file for this folder, and if so, // redo the rename. Path absoluteRenamePendingFile = renamePendingFilePath(f); - if (exists(absoluteRenamePendingFile)) { + if (existsInternal(absoluteRenamePendingFile)) { FolderRenamePending pending = new FolderRenamePending(absoluteRenamePendingFile, this); pending.redo(); http://git-wip-us.apache.org/repos/asf/hadoop/blob/9288206c/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/ITestWasbRemoteCallHelper.java ---------------------------------------------------------------------- diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/ITestWasbRemoteCallHelper.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/ITestWasbRemoteCallHelper.java index 062bc36..6f7e699 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/ITestWasbRemoteCallHelper.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/ITestWasbRemoteCallHelper.java @@ -357,7 +357,7 @@ public class ITestWasbRemoteCallHelper performop(mockHttpClient); - int expectedNumberOfInvocations = isAuthorizationCachingEnabled ? 1 : 2; + int expectedNumberOfInvocations = isAuthorizationCachingEnabled ? 2 : 3; Mockito.verify(mockHttpClient, times(expectedNumberOfInvocations)).execute(Mockito.argThat(new HttpGetForServiceLocal())); Mockito.verify(mockHttpClient, times(expectedNumberOfInvocations)).execute(Mockito.argThat(new HttpGetForService2())); } http://git-wip-us.apache.org/repos/asf/hadoop/blob/9288206c/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/TestNativeAzureFileSystemAuthorization.java ---------------------------------------------------------------------- diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/TestNativeAzureFileSystemAuthorization.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/TestNativeAzureFileSystemAuthorization.java index 2129b33..4e7b6fb 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/TestNativeAzureFileSystemAuthorization.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/TestNativeAzureFileSystemAuthorization.java @@ -18,6 +18,7 @@ package org.apache.hadoop.fs.azure; +import java.io.FileNotFoundException; import java.security.PrivilegedExceptionAction; import java.io.IOException; @@ -28,6 +29,7 @@ import org.apache.hadoop.fs.Path; import org.apache.hadoop.fs.contract.ContractTestUtils; import org.apache.hadoop.fs.permission.FsPermission; import org.apache.hadoop.security.UserGroupInformation; +import org.apache.hadoop.test.GenericTestUtils; import org.apache.hadoop.util.StringUtils; import org.junit.Assume; @@ -38,6 +40,7 @@ import org.junit.rules.ExpectedException; import com.google.common.annotations.VisibleForTesting; import static org.apache.hadoop.fs.azure.AzureNativeFileSystemStore.KEY_USE_SECURE_MODE; +import static org.apache.hadoop.fs.azure.CachingAuthorizer.KEY_AUTH_SERVICE_CACHING_ENABLE; import static org.junit.Assert.assertEquals; /** @@ -62,6 +65,7 @@ public class TestNativeAzureFileSystemAuthorization conf.set(NativeAzureFileSystem.KEY_AZURE_AUTHORIZATION, "true"); conf.set(RemoteWasbAuthorizerImpl.KEY_REMOTE_AUTH_SERVICE_URLS, "http://localhost/";); conf.set(NativeAzureFileSystem.AZURE_CHOWN_USERLIST_PROPERTY_NAME, "user1 , user2"); + conf.set(KEY_AUTH_SERVICE_CACHING_ENABLE, "false"); return conf; } @@ -131,6 +135,7 @@ public class TestNativeAzureFileSystemAuthorization Path testPath = new Path(parentDir, "test.dat"); authorizer.addAuthRuleForOwner("/", WRITE, true); + authorizer.addAuthRuleForOwner("/", READ, true); fs.updateWasbAuthorizer(authorizer); try { @@ -156,6 +161,7 @@ public class TestNativeAzureFileSystemAuthorization Path testPath = new Path(parentDir, "test.dat"); authorizer.addAuthRuleForOwner("/", WRITE, true); + authorizer.addAuthRuleForOwner(testPath.getParent().toString(), READ, true); fs.updateWasbAuthorizer(authorizer); try { @@ -183,6 +189,7 @@ public class TestNativeAzureFileSystemAuthorization setExpectedFailureMessage("create", testPath); authorizer.addAuthRuleForOwner("/", WRITE, true); + authorizer.addAuthRuleForOwner(testPath.getParent().toString(), READ, true); fs.updateWasbAuthorizer(authorizer); try { @@ -208,6 +215,7 @@ public class TestNativeAzureFileSystemAuthorization authorizer.addAuthRuleForOwner("/", WRITE, true); authorizer.addAuthRuleForOwner(testPath.toString(), WRITE, true); + authorizer.addAuthRuleForOwner(parentDir.toString(), READ, true); fs.updateWasbAuthorizer(authorizer); try { @@ -313,6 +321,7 @@ public class TestNativeAzureFileSystemAuthorization authorizer.addAuthRuleForOwner("/", WRITE, true); /* for rename */ authorizer.addAuthRuleForOwner(parentDir.toString(), WRITE, true); + authorizer.addAuthRuleForOwner(parentDir.toString(), READ, true); fs.updateWasbAuthorizer(authorizer); try { @@ -344,6 +353,7 @@ public class TestNativeAzureFileSystemAuthorization /* to create parent dir */ authorizer.addAuthRuleForOwner("/", WRITE, true); authorizer.addAuthRuleForOwner(parentDir.toString(), WRITE, false); + authorizer.addAuthRuleForOwner(parentDir.toString(), READ, true); fs.updateWasbAuthorizer(authorizer); try { @@ -376,6 +386,8 @@ public class TestNativeAzureFileSystemAuthorization authorizer.addAuthRuleForOwner("/", WRITE, true); /* to create parent dir */ authorizer.addAuthRuleForOwner(parentSrcDir.toString(), WRITE, true); authorizer.addAuthRuleForOwner(parentDstDir.toString(), WRITE, false); + authorizer.addAuthRuleForOwner(parentSrcDir.toString(), READ, true); + authorizer.addAuthRuleForOwner(parentDstDir.toString(), READ, true); fs.updateWasbAuthorizer(authorizer); try { @@ -405,6 +417,8 @@ public class TestNativeAzureFileSystemAuthorization authorizer.addAuthRuleForOwner("/", WRITE, true); /* to create parent dirs */ authorizer.addAuthRuleForOwner(parentSrcDir.toString(), WRITE, true); authorizer.addAuthRuleForOwner(parentDstDir.toString(), WRITE, true); + authorizer.addAuthRuleForOwner(parentSrcDir.toString(), READ, true); + authorizer.addAuthRuleForOwner(parentDstDir.toString(), READ, true); fs.updateWasbAuthorizer(authorizer); try { @@ -512,6 +526,7 @@ public class TestNativeAzureFileSystemAuthorization Path testPath = new Path(parentDir, "test.dat"); authorizer.addAuthRuleForOwner("/", WRITE, true); + authorizer.addAuthRuleForOwner(parentDir.toString(), READ, true); fs.updateWasbAuthorizer(authorizer); try { fs.create(testPath); @@ -536,6 +551,7 @@ public class TestNativeAzureFileSystemAuthorization setExpectedFailureMessage("delete", testPath); authorizer.addAuthRuleForOwner("/", WRITE, true); + authorizer.addAuthRuleForOwner(parentDir.toString(), READ, true); fs.updateWasbAuthorizer(authorizer); try { fs.create(testPath); @@ -545,6 +561,7 @@ public class TestNativeAzureFileSystemAuthorization /* Remove permissions for delete to force failure */ authorizer.deleteAllAuthRules(); authorizer.addAuthRuleForOwner("/", WRITE, false); + authorizer.addAuthRuleForOwner(parentDir.toString(), READ, true); fs.updateWasbAuthorizer(authorizer); fs.delete(testPath, false); @@ -553,6 +570,7 @@ public class TestNativeAzureFileSystemAuthorization /* Restore permissions to force a successful delete */ authorizer.deleteAllAuthRules(); authorizer.addAuthRuleForOwner("/", WRITE, true); + authorizer.addAuthRuleForOwner("/", READ, true); fs.updateWasbAuthorizer(authorizer); fs.delete(testPath, false); @@ -569,9 +587,12 @@ public class TestNativeAzureFileSystemAuthorization public void testFileDeleteAccessWithIntermediateFoldersCheckPositive() throws Throwable { Path parentDir = new Path("/testDeleteIntermediateFolder"); - Path testPath = new Path(parentDir, "1/2/test.dat"); + Path childPath = new Path(parentDir, "1/2"); + Path testPath = new Path(childPath, "test.dat"); authorizer.addAuthRuleForOwner("/", WRITE, true); // for create and delete + authorizer.addAuthRuleForOwner(childPath.toString(), READ, true); + authorizer.addAuthRuleForOwner("/", READ, true); authorizer.addAuthRuleForOwner("/testDeleteIntermediateFolder*", WRITE, true); // for recursive delete fs.updateWasbAuthorizer(authorizer); @@ -596,12 +617,16 @@ public class TestNativeAzureFileSystemAuthorization public void testDeleteAuthCheckFailureLeavesFilesUndeleted() throws Throwable { Path parentDir = new Path("/testDeleteAuthCheckFailureLeavesFilesUndeleted"); - Path testPath1 = new Path(parentDir, "child1/test.dat"); - Path testPath2 = new Path(parentDir, "child2/test.dat"); + Path childPath1 = new Path(parentDir, "child1"); + Path childPath2 = new Path(parentDir, "child2"); + Path testPath1 = new Path(childPath1, "test.dat"); + Path testPath2 = new Path(childPath2, "test.dat"); authorizer.addAuthRuleForOwner("/", WRITE, true); authorizer.addAuthRuleForOwner("/testDeleteAuthCheckFailureLeavesFilesUndeleted*", WRITE, true); + authorizer.addAuthRuleForOwner(childPath1.toString(), READ, true); + authorizer.addAuthRuleForOwner(childPath2.toString(), READ, true); fs.updateWasbAuthorizer(authorizer); try { @@ -613,20 +638,19 @@ public class TestNativeAzureFileSystemAuthorization // revoke write on one of the child folders authorizer.deleteAllAuthRules(); authorizer.addAuthRuleForOwner("/", WRITE, true); - authorizer.addAuthRuleForOwner("/testDeleteAuthCheckFailureLeavesFilesUndeleted", - WRITE, true); - authorizer.addAuthRuleForOwner("/testDeleteAuthCheckFailureLeavesFilesUndeleted/child2", - WRITE, true); - authorizer.addAuthRuleForOwner("/testDeleteAuthCheckFailureLeavesFilesUndeleted/child1", - WRITE, false); + authorizer.addAuthRuleForOwner(parentDir.toString(), WRITE, true); + authorizer.addAuthRuleForOwner(childPath2.toString(), WRITE, true); + authorizer.addAuthRuleForOwner(childPath1.toString(), WRITE, false); + authorizer.addAuthRuleForOwner(childPath1.toString(), READ, true); + authorizer.addAuthRuleForOwner(parentDir.toString(), READ, true); + authorizer.addAuthRuleForOwner("/", READ, true); assertFalse(fs.delete(parentDir, true)); // Assert that only child2 contents are deleted ContractTestUtils.assertPathExists(fs, "child1 is deleted!", testPath1); ContractTestUtils.assertPathDoesNotExist(fs, "child2 exists after deletion!", testPath2); - ContractTestUtils.assertPathDoesNotExist(fs, "child2 exists after deletion!", - new Path("/testDeleteAuthCheckFailureLeavesFilesUndeleted/childPath2")); + ContractTestUtils.assertPathDoesNotExist(fs, "child2 exists after deletion!", childPath2); ContractTestUtils.assertPathExists(fs, "parentDir is deleted!", parentDir); } @@ -647,8 +671,8 @@ public class TestNativeAzureFileSystemAuthorization Path testPath = new Path(parentDir, "test.dat"); authorizer.addAuthRuleForOwner("/", WRITE, true); - authorizer.addAuthRuleForOwner("/testSingleFileDeleteWithStickyBitPositive", - WRITE, true); + authorizer.addAuthRuleForOwner(parentDir.toString(), WRITE, true); + authorizer.addAuthRuleForOwner(parentDir.toString(), READ, true); fs.updateWasbAuthorizer(authorizer); try { @@ -684,8 +708,8 @@ public class TestNativeAzureFileSystemAuthorization parentDir.toString(), testPath.toString())); authorizer.addAuthRuleForOwner("/", WRITE, true); - authorizer.addAuthRuleForOwner("/testSingleFileDeleteWithStickyBitNegative", - WRITE, true); + authorizer.addAuthRuleForOwner(parentDir.toString(), WRITE, true); + authorizer.addAuthRuleForOwner(parentDir.toString(), READ, true); fs.updateWasbAuthorizer(authorizer); try { @@ -700,16 +724,22 @@ public class TestNativeAzureFileSystemAuthorization dummyUser.doAs(new PrivilegedExceptionAction<Void>() { @Override public Void run() throws Exception { - authorizer.addAuthRule(parentDir.toString(), WRITE, - getCurrentUserShortName(), true); - fs.delete(testPath, true); - return null; + try { + authorizer.addAuthRule(parentDir.toString(), WRITE, + getCurrentUserShortName(), true); + authorizer.addAuthRule(parentDir.toString(), READ, + getCurrentUserShortName(), true); + fs.delete(testPath, true); + return null; + } + catch (WasbAuthorizationException wae) { + ContractTestUtils.assertPathExists(fs, "testPath should not be deleted!", testPath); + throw wae; + } } }); } finally { - ContractTestUtils.assertPathExists(fs, "testPath should not be deleted!", testPath); - allowRecursiveDelete(fs, parentDir.toString()); fs.delete(parentDir, true); } @@ -724,12 +754,15 @@ public class TestNativeAzureFileSystemAuthorization public void testRecursiveDeleteSucceedsWithStickybit() throws Throwable { Path parentDir = new Path("/testRecursiveDeleteSucceedsWithStickybit"); - Path testFilePath = new Path(parentDir, "child/test.dat"); - Path testFolderPath = new Path(parentDir, "child/testDirectory"); + Path childDir = new Path(parentDir, "child"); + Path testFilePath = new Path(childDir, "test.dat"); + Path testFolderPath = new Path(childDir, "testDirectory"); authorizer.addAuthRuleForOwner("/", WRITE, true); authorizer.addAuthRuleForOwner("/testRecursiveDeleteSucceedsWithStickybit*", WRITE, true); + authorizer.addAuthRuleForOwner(childDir.toString(), READ, true); + authorizer.addAuthRuleForOwner("/", READ, true); fs.updateWasbAuthorizer(authorizer); try { @@ -759,12 +792,15 @@ public class TestNativeAzureFileSystemAuthorization public void testRecursiveDeleteFailsWithStickybit() throws Throwable { Path parentDir = new Path("/testRecursiveDeleteFailsWithStickybit"); - Path testFilePath = new Path(parentDir, "child/test.dat"); - Path testFolderPath = new Path(parentDir, "child/testDirectory"); + Path childDir = new Path(parentDir, "child"); + Path testFilePath = new Path(childDir, "test.dat"); + Path testFolderPath = new Path(childDir, "testDirectory"); authorizer.addAuthRuleForOwner("/", WRITE, true); authorizer.addAuthRuleForOwner("/testRecursiveDeleteFailsWithStickybit*", WRITE, true); + authorizer.addAuthRuleForOwner(childDir.toString(), READ, true); + authorizer.addAuthRuleForOwner("/", READ, true); fs.updateWasbAuthorizer(authorizer); try { @@ -784,8 +820,7 @@ public class TestNativeAzureFileSystemAuthorization @Override public Void run() throws Exception { // Add auth rules for dummyuser - authorizer.addAuthRule("/", WRITE, - getCurrentUserShortName(), true); + authorizer.addAuthRule("/", WRITE, getCurrentUserShortName(), true); authorizer.addAuthRule("/testRecursiveDeleteFailsWithStickybit*", WRITE, getCurrentUserShortName(), true); @@ -821,6 +856,7 @@ public class TestNativeAzureFileSystemAuthorization authorizer.addAuthRuleForOwner( "/testDeleteSucceedsForOnlyFilesOwnedByUserWithStickybitSet*", WRITE, true); + authorizer.addAuthRuleForOwner(parentDir.toString(), READ, true); fs.updateWasbAuthorizer(authorizer); try { @@ -834,10 +870,12 @@ public class TestNativeAzureFileSystemAuthorization dummyUser.doAs(new PrivilegedExceptionAction<Void>() { @Override public Void run() throws Exception { - authorizer.addAuthRule("/", WRITE, - getCurrentUserShortName(), true); + authorizer.addAuthRule("/", WRITE, getCurrentUserShortName(), true); + authorizer.addAuthRule(parentDir.toString(), READ, getCurrentUserShortName(), true); + authorizer.addAuthRule(testFolderPath.toString(), READ, getCurrentUserShortName(), true); authorizer.addAuthRule("/testDeleteSucceedsForOnlyFilesOwnedByUserWithStickybitSet*", WRITE, getCurrentUserShortName(), true); + authorizer.addAuthRule("/", READ, getCurrentUserShortName(), true); fs.create(testFolderPath); // the folder will have owner as dummyuser ContractTestUtils.assertPathExists(fs, "folder was not created", testFolderPath); @@ -876,6 +914,8 @@ public class TestNativeAzureFileSystemAuthorization authorizer.addAuthRuleForOwner( "/testDeleteSucceedsForParentDirectoryOwnerUserWithStickybit*", WRITE, true); + authorizer.addAuthRuleForOwner(parentDir.toString(), READ, true); + authorizer.addAuthRuleForOwner("/", READ, true); fs.updateWasbAuthorizer(authorizer); try { @@ -889,8 +929,8 @@ public class TestNativeAzureFileSystemAuthorization dummyUser.doAs(new PrivilegedExceptionAction<Void>() { @Override public Void run() throws Exception { - authorizer.addAuthRule("/testDeleteSucceedsForParentDirectoryOwnerUserWithStickybit", - WRITE, getCurrentUserShortName(), true); + authorizer.addAuthRule(parentDir.toString(), WRITE, getCurrentUserShortName(), true); + authorizer.addAuthRule(parentDir.toString(), READ, getCurrentUserShortName(), true); fs.create(testFilePath); ContractTestUtils.assertPathExists(fs, "file was not created", testFilePath); @@ -920,12 +960,18 @@ public class TestNativeAzureFileSystemAuthorization public void testDeleteScenarioForRoot() throws Throwable { Path rootPath = new Path("/"); Path parentDir = new Path("/testDeleteScenarioForRoot"); - Path testPath1 = new Path(parentDir, "child1/test.dat"); - Path testPath2 = new Path(parentDir, "child2/testFolder"); + Path childPath1 = new Path(parentDir, "child1"); + Path childPath2 = new Path(parentDir, "child2"); + Path testPath1 = new Path(childPath1, "test.dat"); + Path testPath2 = new Path(childPath2, "testFolder"); authorizer.addAuthRuleForOwner("/", WRITE, true); authorizer.addAuthRuleForOwner("/testDeleteScenarioForRoot*", WRITE, true); + authorizer.addAuthRuleForOwner(childPath1.toString(), READ, true); + authorizer.addAuthRuleForOwner(childPath2.toString(), READ, true); + authorizer.addAuthRuleForOwner(parentDir.toString(), READ, true); + authorizer.addAuthRuleForOwner("/", READ, true); fs.updateWasbAuthorizer(authorizer); try { @@ -948,13 +994,14 @@ public class TestNativeAzureFileSystemAuthorization } /** - * Positive test for getFileStatus. No permissions are required for getting filestatus. + * Positive test for getFileStatus. * @throws Throwable */ @Test public void testGetFileStatusPositive() throws Throwable { Path testPath = new Path("/"); + authorizer.addAuthRuleForOwner("/", READ, true); ContractTestUtils.assertIsDirectory(fs, testPath); } @@ -968,6 +1015,7 @@ public class TestNativeAzureFileSystemAuthorization Path testPath = new Path("/testMkdirsAccessCheckPositive/1/2/3"); authorizer.addAuthRuleForOwner("/", WRITE, true); + authorizer.addAuthRuleForOwner(testPath.getParent().toString(), READ, true); fs.updateWasbAuthorizer(authorizer); try { @@ -990,6 +1038,7 @@ public class TestNativeAzureFileSystemAuthorization Path testPath = new Path("/testMkdirsWithExistingHierarchyCheckPositive1"); authorizer.addAuthRuleForOwner("/", WRITE, true); + authorizer.addAuthRuleForOwner(testPath.getParent().toString(), READ, true); fs.updateWasbAuthorizer(authorizer); try { @@ -998,6 +1047,7 @@ public class TestNativeAzureFileSystemAuthorization /* Don't need permissions to create a directory that already exists */ authorizer.deleteAllAuthRules(); + authorizer.addAuthRuleForOwner(testPath.getParent().toString(), READ, true); // for assert fs.mkdirs(testPath); ContractTestUtils.assertIsDirectory(fs, testPath); @@ -1022,6 +1072,10 @@ public class TestNativeAzureFileSystemAuthorization authorizer.addAuthRuleForOwner(childPath1.toString(), WRITE, true); + authorizer.addAuthRuleForOwner(childPath1.getParent().toString(), READ, true); + authorizer.addAuthRuleForOwner(testPath.getParent().toString(), READ, true); + authorizer.addAuthRuleForOwner(childPath3.getParent().toString(), READ, true); + fs.updateWasbAuthorizer(authorizer); try { @@ -1094,10 +1148,10 @@ public class TestNativeAzureFileSystemAuthorization Path testPath = new Path(parentDir, "test.data"); authorizer.addAuthRuleForOwner("/", WRITE, true); - authorizer.addAuthRuleForOwner(testPath.toString(), READ, true); authorizer.addAuthRuleForOwner(parentDir.toString(), WRITE, true); // additional rule used for assertPathExists authorizer.addAuthRuleForOwner(parentDir.toString(), READ, true); + authorizer.addAuthRuleForOwner("/", READ, true); fs.updateWasbAuthorizer(authorizer); try { @@ -1178,6 +1232,7 @@ public class TestNativeAzureFileSystemAuthorization Path testPath = new Path("/testSetOwnerNegative"); authorizer.addAuthRuleForOwner("/", WRITE, true); + authorizer.addAuthRuleForOwner(testPath.toString(), READ, true); fs.updateWasbAuthorizer(authorizer); String owner = null; @@ -1212,9 +1267,10 @@ public class TestNativeAzureFileSystemAuthorization Path testPath = new Path("/testSetOwnerPositive"); authorizer.addAuthRuleForOwner("/", WRITE, true); + authorizer.addAuthRuleForOwner(testPath.getParent().toString(), READ, true); fs.updateWasbAuthorizer(authorizer); - String newOwner = "newowner"; + String newOwner = "user2"; String newGroup = "newgroup"; UserGroupInformation authorisedUser = UserGroupInformation.createUserForTesting( @@ -1257,6 +1313,7 @@ public class TestNativeAzureFileSystemAuthorization authorizer.init(conf); authorizer.addAuthRuleForOwner("/", WRITE, true); + authorizer.addAuthRuleForOwner(testPath.getParent().toString(), READ, true); fs.updateWasbAuthorizer(authorizer); String newOwner = "newowner"; @@ -1303,6 +1360,7 @@ public class TestNativeAzureFileSystemAuthorization authorizer.init(conf); authorizer.addAuthRuleForOwner("/", WRITE, true); + authorizer.addAuthRuleForOwner(testPath.getParent().toString(), READ, true); fs.updateWasbAuthorizer(authorizer); String owner = null; @@ -1328,4 +1386,51 @@ public class TestNativeAzureFileSystemAuthorization fs.delete(testPath, false); } } + + /** Test to ensure that the internal RenamePending mechanism + * does not make authorization calls. + */ + @Test + public void testRenamePendingAuthorizationCalls() throws Throwable { + Path testPath = new Path("/testRenamePendingAuthorizationCalls"); + Path srcPath = new Path(testPath, "srcPath"); + Path dstPath = new Path(testPath, "dstPath"); + Path srcFilePath = new Path(srcPath, "file.txt"); + Path dstFilePath = new Path(dstPath, "file.txt"); + + authorizer.addAuthRuleForOwner("/", WRITE, true); + /* Remove nextline after fixing createInternal from FolderRenamePending */ + authorizer.addAuthRuleForOwner(testPath.toString(), WRITE, true); + authorizer.addAuthRuleForOwner(srcPath.getParent().toString(), READ, true); + authorizer.addAuthRuleForOwner(dstFilePath.getParent().toString(), READ, true); + fs.updateWasbAuthorizer(authorizer); + + try { + fs.create(srcFilePath); + + String srcKey = fs.pathToKey(srcPath); + String dstKey = fs.pathToKey(dstPath); + + // Create a -RenamePendingFile + NativeAzureFileSystem.FolderRenamePending renamePending = + new NativeAzureFileSystem.FolderRenamePending(srcKey, dstKey, null, fs); + renamePending.writeFile(fs); + + // Initiate the pending-rename + fs.getFileStatus(srcPath); + } catch (FileNotFoundException fnfe) { + // This is expected because getFileStatus would complete the pending "rename" + // represented by the -RenamePending file. + GenericTestUtils.assertExceptionContains( + srcPath.toString() + ": No such file or directory.", fnfe + ); + + // The pending rename should have completed + ContractTestUtils.assertPathExists(fs, + "dstFilePath does not exist -- pending rename failed", dstFilePath); + } finally { + allowRecursiveDelete(fs, testPath.toString()); + fs.delete(testPath, true); + } + } } http://git-wip-us.apache.org/repos/asf/hadoop/blob/9288206c/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/metrics/ITestAzureFileSystemInstrumentation.java ---------------------------------------------------------------------- diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/metrics/ITestAzureFileSystemInstrumentation.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/metrics/ITestAzureFileSystemInstrumentation.java index 60e24ee..5f08d80 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/metrics/ITestAzureFileSystemInstrumentation.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/metrics/ITestAzureFileSystemInstrumentation.java @@ -343,9 +343,9 @@ public class ITestAzureFileSystemInstrumentation extends AbstractWasbTestBase { assertFalse(getFileSystem().exists(filePath)); // At the time of writing this code it takes 2 requests/responses to // check existence, which seems excessive, plus initial request for - // container check. + // container check, plus 2 ancestor checks only in the secure case. logOpResponseCount("Checking file existence for non-existent file", base); - base = assertWebResponsesInRange(base, 1, 3); + base = assertWebResponsesInRange(base, 1, 5); // Create an empty file assertTrue(getFileSystem().createNewFile(filePath)); @@ -354,7 +354,7 @@ public class ITestAzureFileSystemInstrumentation extends AbstractWasbTestBase { // Check existence again assertTrue(getFileSystem().exists(filePath)); logOpResponseCount("Checking file existence for existent file", base); - base = assertWebResponsesInRange(base, 1, 2); + base = assertWebResponsesInRange(base, 1, 4); // Delete the file assertEquals(0, AzureMetricsTestUtil.getLongCounterValue(getInstrumentation(), WASB_FILES_DELETED)); --------------------------------------------------------------------- To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-commits-h...@hadoop.apache.org
