Author: daryn
Date: Thu Sep 6 14:06:58 2012
New Revision: 1381606
URL: http://svn.apache.org/viewvc?rev=1381606&view=rev
Log:
HDFS-3890. filecontext mkdirs doesn't apply umask as expected (Tom Graves via
daryn)
Added:
hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/TestFcHdfsSetUMask.java
Modified:
hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/fs/Hdfs.java
hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java
Modified: hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
URL:
http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt?rev=1381606&r1=1381605&r2=1381606&view=diff
==============================================================================
--- hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt (original)
+++ hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt Thu Sep 6
14:06:58 2012
@@ -1628,6 +1628,9 @@ Release 0.23.3 - UNRELEASED
HDFS-3852. TestHftpDelegationToken is broken after HADOOP-8225 (daryn)
+ HDFS-3890. filecontext mkdirs doesn't apply umask as expected
+ (Tom Graves via daryn)
+
Release 0.23.2 - UNRELEASED
INCOMPATIBLE CHANGES
Modified:
hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/fs/Hdfs.java
URL:
http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/fs/Hdfs.java?rev=1381606&r1=1381605&r2=1381606&view=diff
==============================================================================
---
hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/fs/Hdfs.java
(original)
+++
hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/fs/Hdfs.java
Thu Sep 6 14:06:58 2012
@@ -321,7 +321,7 @@ public class Hdfs extends AbstractFileSy
@Override
public void mkdir(Path dir, FsPermission permission, boolean createParent)
throws IOException, UnresolvedLinkException {
- dfs.mkdirs(getUriPath(dir), permission, createParent);
+ dfs.primitiveMkdir(getUriPath(dir), permission, createParent);
}
@SuppressWarnings("deprecation")
Modified:
hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java
URL:
http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java?rev=1381606&r1=1381605&r2=1381606&view=diff
==============================================================================
---
hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java
(original)
+++
hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java
Thu Sep 6 14:06:58 2012
@@ -1956,34 +1956,29 @@ public class DFSClient implements java.i
*/
public boolean mkdirs(String src, FsPermission permission,
boolean createParent) throws IOException {
- checkOpen();
if (permission == null) {
permission = FsPermission.getDefault();
}
FsPermission masked = permission.applyUMask(dfsClientConf.uMask);
- if(LOG.isDebugEnabled()) {
- LOG.debug(src + ": masked=" + masked);
- }
- try {
- return namenode.mkdirs(src, masked, createParent);
- } catch(RemoteException re) {
- throw re.unwrapRemoteException(AccessControlException.class,
- InvalidPathException.class,
- FileAlreadyExistsException.class,
- FileNotFoundException.class,
- ParentNotDirectoryException.class,
- SafeModeException.class,
- NSQuotaExceededException.class,
- UnresolvedPathException.class);
- }
+ return primitiveMkdir(src, masked, createParent);
}
-
+
/**
* Same {{@link #mkdirs(String, FsPermission, boolean)} except
* that the permissions has already been masked against umask.
*/
public boolean primitiveMkdir(String src, FsPermission absPermission)
throws IOException {
+ return primitiveMkdir(src, absPermission, true);
+ }
+
+ /**
+ * Same {{@link #mkdirs(String, FsPermission, boolean)} except
+ * that the permissions has already been masked against umask.
+ */
+ public boolean primitiveMkdir(String src, FsPermission absPermission,
+ boolean createParent)
+ throws IOException {
checkOpen();
if (absPermission == null) {
absPermission =
@@ -1994,15 +1989,20 @@ public class DFSClient implements java.i
LOG.debug(src + ": masked=" + absPermission);
}
try {
- return namenode.mkdirs(src, absPermission, true);
+ return namenode.mkdirs(src, absPermission, createParent);
} catch(RemoteException re) {
throw re.unwrapRemoteException(AccessControlException.class,
+ InvalidPathException.class,
+ FileAlreadyExistsException.class,
+ FileNotFoundException.class,
+ ParentNotDirectoryException.class,
+ SafeModeException.class,
NSQuotaExceededException.class,
DSQuotaExceededException.class,
UnresolvedPathException.class);
}
}
-
+
/**
* Get {@link ContentSummary} rooted at the specified directory.
* @param path The string representation of the path
Added:
hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/TestFcHdfsSetUMask.java
URL:
http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/TestFcHdfsSetUMask.java?rev=1381606&view=auto
==============================================================================
---
hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/TestFcHdfsSetUMask.java
(added)
+++
hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/TestFcHdfsSetUMask.java
Thu Sep 6 14:06:58 2012
@@ -0,0 +1,251 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs;
+
+import java.io.IOException;
+import java.net.URISyntaxException;
+
+import javax.security.auth.login.LoginException;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.permission.FsPermission;
+import org.apache.hadoop.fs.CommonConfigurationKeys;
+import org.apache.hadoop.hdfs.HdfsConfiguration;
+import org.apache.hadoop.hdfs.MiniDFSCluster;
+import org.apache.hadoop.security.UserGroupInformation;
+import org.apache.hadoop.util.StringUtils;
+import static org.apache.hadoop.fs.FileContextTestHelper.*;
+import org.apache.commons.logging.impl.Log4JLogger;
+import org.apache.log4j.Level;
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+public class TestFcHdfsSetUMask {
+
+ private static MiniDFSCluster cluster;
+ private static Path defaultWorkingDirectory;
+ private static FileContext fc;
+
+ // rwxrwx---
+ private static final FsPermission USER_GROUP_OPEN_PERMISSIONS = FsPermission
+ .createImmutable((short) 0770);
+
+ private static final FsPermission USER_GROUP_OPEN_FILE_PERMISSIONS =
+ FsPermission.createImmutable((short) 0660);
+
+ private static final FsPermission USER_GROUP_OPEN_TEST_UMASK = FsPermission
+ .createImmutable((short) (0770 ^ 0777));
+
+ // ---------
+ private static final FsPermission BLANK_PERMISSIONS = FsPermission
+ .createImmutable((short) 0000);
+
+ // parent directory permissions when creating a directory with blank (000)
+ // permissions - it always add the -wx------ bits to the parent so that
+ // it can create the child
+ private static final FsPermission PARENT_PERMS_FOR_BLANK_PERMISSIONS =
+ FsPermission.createImmutable((short) 0300);
+
+ private static final FsPermission BLANK_TEST_UMASK = FsPermission
+ .createImmutable((short) (0000 ^ 0777));
+
+ // rwxrwxrwx
+ private static final FsPermission WIDE_OPEN_PERMISSIONS = FsPermission
+ .createImmutable((short) 0777);
+
+ private static final FsPermission WIDE_OPEN_FILE_PERMISSIONS =
+ FsPermission.createImmutable((short) 0666);
+
+ private static final FsPermission WIDE_OPEN_TEST_UMASK = FsPermission
+ .createImmutable((short) (0777 ^ 0777));
+
+ @BeforeClass
+ public static void clusterSetupAtBegining()
+ throws IOException, LoginException, URISyntaxException {
+ Configuration conf = new HdfsConfiguration();
+ // set permissions very restrictive
+ conf.set(CommonConfigurationKeys.FS_PERMISSIONS_UMASK_KEY, "077");
+ cluster = new MiniDFSCluster.Builder(conf).numDataNodes(2).build();
+ fc = FileContext.getFileContext(cluster.getURI(0), conf);
+ defaultWorkingDirectory = fc.makeQualified( new Path("/user/" +
+ UserGroupInformation.getCurrentUser().getShortUserName()));
+ fc.mkdir(defaultWorkingDirectory, FileContext.DEFAULT_PERM, true);
+ }
+
+ @AfterClass
+ public static void ClusterShutdownAtEnd() throws Exception {
+ cluster.shutdown();
+ }
+
+ {
+ try {
+ ((Log4JLogger)FileSystem.LOG).getLogger().setLevel(Level.DEBUG);
+ }
+ catch(Exception e) {
+ System.out.println("Cannot change log level\n"
+ + StringUtils.stringifyException(e));
+ }
+ }
+
+ @Before
+ public void setUp() throws Exception {
+ fc.setUMask(WIDE_OPEN_TEST_UMASK);
+ fc.mkdir(getTestRootPath(fc), FileContext.DEFAULT_PERM, true);
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ fc.delete(getTestRootPath(fc), true);
+ }
+
+ @Test
+ public void testMkdirWithExistingDirClear() throws IOException {
+ testMkdirWithExistingDir(BLANK_TEST_UMASK, BLANK_PERMISSIONS);
+ }
+
+ @Test
+ public void testMkdirWithExistingDirOpen() throws IOException {
+ testMkdirWithExistingDir(WIDE_OPEN_TEST_UMASK, WIDE_OPEN_PERMISSIONS);
+ }
+
+ @Test
+ public void testMkdirWithExistingDirMiddle() throws IOException {
+ testMkdirWithExistingDir(USER_GROUP_OPEN_TEST_UMASK,
+ USER_GROUP_OPEN_PERMISSIONS);
+ }
+
+ @Test
+ public void testMkdirRecursiveWithNonExistingDirClear() throws IOException {
+ // by default parent directories have -wx------ bits set
+ testMkdirRecursiveWithNonExistingDir(BLANK_TEST_UMASK, BLANK_PERMISSIONS,
+ PARENT_PERMS_FOR_BLANK_PERMISSIONS);
+ }
+
+ @Test
+ public void testMkdirRecursiveWithNonExistingDirOpen() throws IOException {
+ testMkdirRecursiveWithNonExistingDir(WIDE_OPEN_TEST_UMASK,
+ WIDE_OPEN_PERMISSIONS, WIDE_OPEN_PERMISSIONS);
+ }
+
+ @Test
+ public void testMkdirRecursiveWithNonExistingDirMiddle() throws IOException {
+ testMkdirRecursiveWithNonExistingDir(USER_GROUP_OPEN_TEST_UMASK,
+ USER_GROUP_OPEN_PERMISSIONS, USER_GROUP_OPEN_PERMISSIONS);
+ }
+
+
+ @Test
+ public void testCreateRecursiveWithExistingDirClear() throws IOException {
+ testCreateRecursiveWithExistingDir(BLANK_TEST_UMASK, BLANK_PERMISSIONS);
+ }
+
+ @Test
+ public void testCreateRecursiveWithExistingDirOpen() throws IOException {
+ testCreateRecursiveWithExistingDir(WIDE_OPEN_TEST_UMASK,
+ WIDE_OPEN_FILE_PERMISSIONS);
+ }
+
+ @Test
+ public void testCreateRecursiveWithExistingDirMiddle() throws IOException {
+ testCreateRecursiveWithExistingDir(USER_GROUP_OPEN_TEST_UMASK,
+ USER_GROUP_OPEN_FILE_PERMISSIONS);
+ }
+
+
+ @Test
+ public void testCreateRecursiveWithNonExistingDirClear() throws IOException {
+ // directory permission inherited from parent so this must match the
@Before
+ // set of umask
+ testCreateRecursiveWithNonExistingDir(BLANK_TEST_UMASK,
+ WIDE_OPEN_PERMISSIONS, BLANK_PERMISSIONS);
+ }
+
+ @Test
+ public void testCreateRecursiveWithNonExistingDirOpen() throws IOException {
+ // directory permission inherited from parent so this must match the
@Before
+ // set of umask
+ testCreateRecursiveWithNonExistingDir(WIDE_OPEN_TEST_UMASK,
+ WIDE_OPEN_PERMISSIONS, WIDE_OPEN_FILE_PERMISSIONS);
+ }
+
+ @Test
+ public void testCreateRecursiveWithNonExistingDirMiddle() throws IOException
{
+ // directory permission inherited from parent so this must match the
@Before
+ // set of umask
+ testCreateRecursiveWithNonExistingDir(USER_GROUP_OPEN_TEST_UMASK,
+ WIDE_OPEN_PERMISSIONS, USER_GROUP_OPEN_FILE_PERMISSIONS);
+ }
+
+
+ public void testMkdirWithExistingDir(FsPermission umask,
+ FsPermission expectedPerms) throws IOException {
+ Path f = getTestRootPath(fc, "aDir");
+ fc.setUMask(umask);
+ fc.mkdir(f, FileContext.DEFAULT_PERM, true);
+ Assert.assertTrue(isDir(fc, f));
+ Assert.assertEquals("permissions on directory are wrong",
+ expectedPerms, fc.getFileStatus(f).getPermission());
+ }
+
+ public void testMkdirRecursiveWithNonExistingDir(FsPermission umask,
+ FsPermission expectedPerms, FsPermission expectedParentPerms)
+ throws IOException {
+ Path f = getTestRootPath(fc, "NonExistant2/aDir");
+ fc.setUMask(umask);
+ fc.mkdir(f, FileContext.DEFAULT_PERM, true);
+ Assert.assertTrue(isDir(fc, f));
+ Assert.assertEquals("permissions on directory are wrong",
+ expectedPerms, fc.getFileStatus(f).getPermission());
+ Path fParent = getTestRootPath(fc, "NonExistant2");
+ Assert.assertEquals("permissions on parent directory are wrong",
+ expectedParentPerms, fc.getFileStatus(fParent).getPermission());
+ }
+
+
+ public void testCreateRecursiveWithExistingDir(FsPermission umask,
+ FsPermission expectedPerms) throws IOException {
+ Path f = getTestRootPath(fc,"foo");
+ fc.setUMask(umask);
+ createFile(fc, f);
+ Assert.assertTrue(isFile(fc, f));
+ Assert.assertEquals("permissions on file are wrong",
+ expectedPerms , fc.getFileStatus(f).getPermission());
+ }
+
+
+ public void testCreateRecursiveWithNonExistingDir(FsPermission umask,
+ FsPermission expectedDirPerms, FsPermission expectedFilePerms)
+ throws IOException {
+ Path f = getTestRootPath(fc,"NonExisting/foo");
+ Path fParent = getTestRootPath(fc, "NonExisting");
+ Assert.assertFalse(exists(fc, fParent));
+ fc.setUMask(umask);
+ createFile(fc, f);
+ Assert.assertTrue(isFile(fc, f));
+ Assert.assertEquals("permissions on file are wrong",
+ expectedFilePerms, fc.getFileStatus(f).getPermission());
+ Assert.assertEquals("permissions on parent directory are wrong",
+ expectedDirPerms, fc.getFileStatus(fParent).getPermission());
+ }
+
+}
