You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by vi...@apache.org on 2014/11/06 09:04:29 UTC
[18/43] git commit: HDFS-7218. FSNamesystem ACL operations should
write to audit log on failure. (clamb via yliu)
HDFS-7218. FSNamesystem ACL operations should write to audit log on failure. (clamb via yliu)
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/73e60125
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/73e60125
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/73e60125
Branch: refs/heads/HDFS-EC
Commit: 73e601259fed0646f115b09112995b51ffef3468
Parents: 73068f6
Author: yliu <yl...@apache.org>
Authored: Wed Nov 5 15:49:37 2014 +0800
Committer: yliu <yl...@apache.org>
Committed: Wed Nov 5 15:49:37 2014 +0800
----------------------------------------------------------------------
hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt | 3 +
.../hdfs/server/namenode/FSNamesystem.java | 26 +++++-
.../hdfs/server/namenode/TestAuditLogger.java | 95 ++++++++++++++++++++
3 files changed, 123 insertions(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hadoop/blob/73e60125/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
index 098d4ba..707929e 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
+++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
@@ -1010,6 +1010,9 @@ Release 2.6.0 - UNRELEASED
fails on Windows, because we cannot deny access to the file owner.
(Chris Nauroth via wheat9)
+ HDFS-7218. FSNamesystem ACL operations should write to audit log on
+ failure. (clamb via yliu)
+
BREAKDOWN OF HDFS-6134 AND HADOOP-10150 SUBTASKS AND RELATED JIRAS
HDFS-6387. HDFS CLI admin tool for creating & deleting an
http://git-wip-us.apache.org/repos/asf/hadoop/blob/73e60125/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
index 2bc4ba0..76c1423 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
@@ -7862,6 +7862,11 @@ public class FSNamesystem implements Namesystem, FSClusterStats,
public FSDirectory getFSDirectory() {
return dir;
}
+ /** Set the FSDirectory. */
+ @VisibleForTesting
+ public void setFSDirectory(FSDirectory dir) {
+ this.dir = dir;
+ }
/** @return the cache manager. */
public CacheManager getCacheManager() {
return cacheManager;
@@ -8728,6 +8733,9 @@ public class FSNamesystem implements Namesystem, FSClusterStats,
List<AclEntry> newAcl = dir.modifyAclEntries(src, aclSpec);
getEditLog().logSetAcl(src, newAcl);
resultingStat = getAuditFileInfo(src, false);
+ } catch (AccessControlException e) {
+ logAuditEvent(false, "modifyAclEntries", srcArg);
+ throw e;
} finally {
writeUnlock();
}
@@ -8752,6 +8760,9 @@ public class FSNamesystem implements Namesystem, FSClusterStats,
List<AclEntry> newAcl = dir.removeAclEntries(src, aclSpec);
getEditLog().logSetAcl(src, newAcl);
resultingStat = getAuditFileInfo(src, false);
+ } catch (AccessControlException e) {
+ logAuditEvent(false, "removeAclEntries", srcArg);
+ throw e;
} finally {
writeUnlock();
}
@@ -8775,6 +8786,9 @@ public class FSNamesystem implements Namesystem, FSClusterStats,
List<AclEntry> newAcl = dir.removeDefaultAcl(src);
getEditLog().logSetAcl(src, newAcl);
resultingStat = getAuditFileInfo(src, false);
+ } catch (AccessControlException e) {
+ logAuditEvent(false, "removeDefaultAcl", srcArg);
+ throw e;
} finally {
writeUnlock();
}
@@ -8798,6 +8812,9 @@ public class FSNamesystem implements Namesystem, FSClusterStats,
dir.removeAcl(src);
getEditLog().logSetAcl(src, AclFeature.EMPTY_ENTRY_LIST);
resultingStat = getAuditFileInfo(src, false);
+ } catch (AccessControlException e) {
+ logAuditEvent(false, "removeAcl", srcArg);
+ throw e;
} finally {
writeUnlock();
}
@@ -8821,6 +8838,9 @@ public class FSNamesystem implements Namesystem, FSClusterStats,
List<AclEntry> newAcl = dir.setAcl(src, aclSpec);
getEditLog().logSetAcl(src, newAcl);
resultingStat = getAuditFileInfo(src, false);
+ } catch (AccessControlException e) {
+ logAuditEvent(false, "setAcl", srcArg);
+ throw e;
} finally {
writeUnlock();
}
@@ -8833,6 +8853,7 @@ public class FSNamesystem implements Namesystem, FSClusterStats,
FSPermissionChecker pc = getPermissionChecker();
checkOperation(OperationCategory.READ);
byte[][] pathComponents = FSDirectory.getPathComponentsForReservedPath(src);
+ boolean success = false;
readLock();
try {
checkOperation(OperationCategory.READ);
@@ -8840,9 +8861,12 @@ public class FSNamesystem implements Namesystem, FSClusterStats,
if (isPermissionEnabled) {
checkPermission(pc, src, false, null, null, null, null);
}
- return dir.getAclStatus(src);
+ final AclStatus ret = dir.getAclStatus(src);
+ success = true;
+ return ret;
} finally {
readUnlock();
+ logAuditEvent(success, "getAclStatus", src);
}
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/73e60125/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAuditLogger.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAuditLogger.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAuditLogger.java
index 29fee68..e1e1c67 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAuditLogger.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAuditLogger.java
@@ -19,30 +19,39 @@
package org.apache.hadoop.hdfs.server.namenode;
import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_AUDIT_LOGGERS_KEY;
+import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_ACLS_ENABLED_KEY;
+import static org.apache.hadoop.test.GenericTestUtils.assertExceptionContains;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
+import static org.mockito.Matchers.anyListOf;
+import static org.mockito.Matchers.anyString;
import java.io.IOException;
import java.net.HttpURLConnection;
import java.net.InetAddress;
import java.net.URI;
import java.net.URISyntaxException;
+import java.util.List;
+import com.google.common.collect.Lists;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.permission.AclEntry;
import org.apache.hadoop.fs.permission.FsPermission;
import org.apache.hadoop.hdfs.HdfsConfiguration;
import org.apache.hadoop.hdfs.MiniDFSCluster;
import org.apache.hadoop.hdfs.web.resources.GetOpParam;
import org.apache.hadoop.ipc.RemoteException;
import org.apache.hadoop.net.NetUtils;
+import org.apache.hadoop.security.AccessControlException;
import org.apache.hadoop.security.authorize.ProxyServers;
import org.apache.hadoop.security.authorize.ProxyUsers;
import org.junit.Before;
import org.junit.Test;
+import org.mockito.Mockito;
/**
* Tests for the {@link AuditLogger} custom audit logging interface.
@@ -166,6 +175,87 @@ public class TestAuditLogger {
}
}
+ @Test
+ public void testAuditLogWithAclFailure() throws Exception {
+ final Configuration conf = new HdfsConfiguration();
+ conf.setBoolean(DFS_NAMENODE_ACLS_ENABLED_KEY, true);
+ conf.set(DFS_NAMENODE_AUDIT_LOGGERS_KEY,
+ DummyAuditLogger.class.getName());
+ final MiniDFSCluster cluster = new MiniDFSCluster.Builder(conf).build();
+ try {
+ cluster.waitClusterUp();
+ final FSDirectory dir = cluster.getNamesystem().getFSDirectory();
+ // Set up mock FSDirectory to test FSN audit logging during failure
+ final FSDirectory mockedDir = Mockito.spy(dir);
+ Mockito.doThrow(new AccessControlException("mock setAcl exception")).
+ when(mockedDir).
+ setAcl(anyString(), anyListOf(AclEntry.class));
+ Mockito.doThrow(new AccessControlException("mock getAclStatus exception")).
+ when(mockedDir).
+ getAclStatus(anyString());
+ Mockito.doThrow(new AccessControlException("mock removeAcl exception")).
+ when(mockedDir).
+ removeAcl(anyString());
+ Mockito.doThrow(new AccessControlException("mock removeDefaultAcl exception")).
+ when(mockedDir).
+ removeDefaultAcl(anyString());
+ Mockito.doThrow(new AccessControlException("mock removeAclEntries exception")).
+ when(mockedDir).
+ removeAclEntries(anyString(), anyListOf(AclEntry.class));
+ Mockito.doThrow(new AccessControlException("mock modifyAclEntries exception")).
+ when(mockedDir).
+ modifyAclEntries(anyString(), anyListOf(AclEntry.class));
+ // Replace the FSD with the mock FSD.
+ cluster.getNamesystem().setFSDirectory(mockedDir);
+ assertTrue(DummyAuditLogger.initialized);
+ DummyAuditLogger.resetLogCount();
+
+ final FileSystem fs = cluster.getFileSystem();
+ final Path p = new Path("/");
+ final List<AclEntry> acls = Lists.newArrayList();
+
+ try {
+ fs.getAclStatus(p);
+ } catch (AccessControlException e) {
+ assertExceptionContains("mock getAclStatus exception", e);
+ }
+
+ try {
+ fs.setAcl(p, acls);
+ } catch (AccessControlException e) {
+ assertExceptionContains("mock setAcl exception", e);
+ }
+
+ try {
+ fs.removeAcl(p);
+ } catch (AccessControlException e) {
+ assertExceptionContains("mock removeAcl exception", e);
+ }
+
+ try {
+ fs.removeDefaultAcl(p);
+ } catch (AccessControlException e) {
+ assertExceptionContains("mock removeDefaultAcl exception", e);
+ }
+
+ try {
+ fs.removeAclEntries(p, acls);
+ } catch (AccessControlException e) {
+ assertExceptionContains("mock removeAclEntries exception", e);
+ }
+
+ try {
+ fs.modifyAclEntries(p, acls);
+ } catch (AccessControlException e) {
+ assertExceptionContains("mock modifyAclEntries exception", e);
+ }
+ assertEquals(6, DummyAuditLogger.logCount);
+ assertEquals(6, DummyAuditLogger.unsuccessfulCount);
+ } finally {
+ cluster.shutdown();
+ }
+ }
+
/**
* Tests that a broken audit logger causes requests to fail.
*/
@@ -194,6 +284,7 @@ public class TestAuditLogger {
static boolean initialized;
static int logCount;
+ static int unsuccessfulCount;
static short foundPermission;
static String remoteAddr;
@@ -203,6 +294,7 @@ public class TestAuditLogger {
public static void resetLogCount() {
logCount = 0;
+ unsuccessfulCount = 0;
}
public void logAuditEvent(boolean succeeded, String userName,
@@ -210,6 +302,9 @@ public class TestAuditLogger {
FileStatus stat) {
remoteAddr = addr.getHostAddress();
logCount++;
+ if (!succeeded) {
+ unsuccessfulCount++;
+ }
if (stat != null) {
foundPermission = stat.getPermission().toShort();
}