From 0398db19b2c4558a9f08ac2700a27752748896fa Mon Sep 17 00:00:00 2001 From: Harsh J Date: Tue, 28 Oct 2014 12:08:26 +0530 Subject: [PATCH] HDFS-6741. Improve permission denied message when FSPermissionChecker#checkOwner fails. Contributed by Stephen Chu and Harsh J. (harsh) --- hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt | 3 +++ .../server/namenode/FSPermissionChecker.java | 4 +++- .../apache/hadoop/hdfs/TestDFSPermission.java | 18 +++++++++++++++--- .../namenode/TestFSPermissionChecker.java | 8 +++++++- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index 456f77b8371..e18b935c314 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -316,6 +316,9 @@ Release 2.7.0 - UNRELEASED BUG FIXES + HDFS-6741. Improve permission denied message when + FSPermissionChecker#checkOwner fails (Stephen Chu and harsh). + HDFS-6538. Comment format error in ShortCircuitRegistry javadoc. (David Luo via harsh). diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java index 5b7804b40e7..2c48051142a 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java @@ -198,7 +198,9 @@ class FSPermissionChecker { if (inode != null && user.equals(inode.getUserName(snapshotId))) { return; } - throw new AccessControlException("Permission denied"); + throw new AccessControlException( + "Permission denied. user=" + + user + " is not the owner of inode=" + inode); } /** Guarded by {@link FSNamesystem#readLock()} */ diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSPermission.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSPermission.java index 68349a2ac67..23ce916ae11 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSPermission.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSPermission.java @@ -443,7 +443,11 @@ public class TestDFSPermission { fs.access(p1, FsAction.WRITE); fail("The access call should have failed."); } catch (AccessControlException e) { - // expected + assertTrue("Permission denied messages must carry the username", + e.getMessage().contains(USER1_NAME)); + assertTrue("Permission denied messages must carry the path parent", + e.getMessage().contains( + p1.getParent().toUri().getPath())); } Path badPath = new Path("/bad/bad"); @@ -473,7 +477,11 @@ public class TestDFSPermission { fs.access(p2, FsAction.EXECUTE); fail("The access call should have failed."); } catch (AccessControlException e) { - // expected + assertTrue("Permission denied messages must carry the username", + e.getMessage().contains(USER1_NAME)); + assertTrue("Permission denied messages must carry the path parent", + e.getMessage().contains( + p2.getParent().toUri().getPath())); } } @@ -494,7 +502,11 @@ public class TestDFSPermission { fs.access(p3, FsAction.READ_WRITE); fail("The access call should have failed."); } catch (AccessControlException e) { - // expected + assertTrue("Permission denied messages must carry the username", + e.getMessage().contains(USER1_NAME)); + assertTrue("Permission denied messages must carry the path parent", + e.getMessage().contains( + p3.getParent().toUri().getPath())); } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSPermissionChecker.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSPermissionChecker.java index 91931aa5924..c0e1e05ff83 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSPermissionChecker.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSPermissionChecker.java @@ -33,6 +33,7 @@ import static org.apache.hadoop.fs.permission.FsAction.WRITE; import static org.apache.hadoop.fs.permission.FsAction.WRITE_EXECUTE; import static org.apache.hadoop.hdfs.server.namenode.AclTestHelpers.aclEntry; import static org.junit.Assert.fail; +import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; @@ -41,6 +42,7 @@ import java.io.IOException; import java.util.Arrays; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.Path; import org.apache.hadoop.fs.permission.AclEntry; import org.apache.hadoop.fs.permission.FsAction; import org.apache.hadoop.fs.permission.FsPermission; @@ -412,7 +414,11 @@ public class TestFSPermissionChecker { fail("expected AccessControlException for user + " + user + ", path = " + path + ", access = " + access); } catch (AccessControlException e) { - // expected + assertTrue("Permission denied messages must carry the username", + e.getMessage().contains(user.getUserName().toString())); + assertTrue("Permission denied messages must carry the path parent", + e.getMessage().contains( + new Path(path).getParent().toUri().getPath())); } }