From 8e4ac01135e386fb5ed09846c0f7bb9deffdadde Mon Sep 17 00:00:00 2001 From: Tamas Domok Date: Tue, 14 Sep 2021 17:34:32 +0200 Subject: [PATCH] YARN-10901. Permission checking error on an existing directory in LogAggregationFileController#verifyAndCreateRemoteLogDir (#3409) Co-authored-by: Tamas Domok --- .../LogAggregationFileController.java | 13 ++++- .../TestLogAggregationFileController.java | 53 +++++++++++++++++++ 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/logaggregation/filecontroller/LogAggregationFileController.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/logaggregation/filecontroller/LogAggregationFileController.java index cf305babea6..4844ae2b15f 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/logaggregation/filecontroller/LogAggregationFileController.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/logaggregation/filecontroller/LogAggregationFileController.java @@ -18,6 +18,7 @@ package org.apache.hadoop.yarn.logaggregation.filecontroller; +import org.apache.commons.lang3.RandomStringUtils; import org.apache.hadoop.thirdparty.com.google.common.annotations.VisibleForTesting; import java.io.FileNotFoundException; @@ -373,10 +374,13 @@ public abstract class LogAggregationFileController { throw new YarnRuntimeException("Failed to create remoteLogDir [" + remoteRootLogDir + "]", e); } - } else{ + } else { //Check if FS has capability to set/modify permissions + Path permissionCheckFile = new Path(qualified, String.format("%s.permission_check", + RandomStringUtils.randomAlphanumeric(8))); try { - remoteFS.setPermission(qualified, new FsPermission(TLDIR_PERMISSIONS)); + remoteFS.createNewFile(permissionCheckFile); + remoteFS.setPermission(permissionCheckFile, new FsPermission(TLDIR_PERMISSIONS)); } catch (UnsupportedOperationException use) { LOG.info("Unable to set permissions for configured filesystem since" + " it does not support this", remoteFS.getScheme()); @@ -384,6 +388,11 @@ public abstract class LogAggregationFileController { } catch (IOException e) { LOG.warn("Failed to check if FileSystem suppports permissions on " + "remoteLogDir [" + remoteRootLogDir + "]", e); + } finally { + try { + remoteFS.delete(permissionCheckFile, false); + } catch (IOException ignored) { + } } } } diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/logaggregation/filecontroller/TestLogAggregationFileController.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/logaggregation/filecontroller/TestLogAggregationFileController.java index 5ade0faabd8..818e01129fc 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/logaggregation/filecontroller/TestLogAggregationFileController.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/logaggregation/filecontroller/TestLogAggregationFileController.java @@ -19,17 +19,23 @@ package org.apache.hadoop.yarn.logaggregation.filecontroller; 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.FsPermission; import org.apache.hadoop.security.UserGroupInformation; import org.apache.hadoop.yarn.conf.YarnConfiguration; +import org.junit.Assert; import org.junit.Test; +import org.mockito.ArgumentMatcher; import org.mockito.Mockito; import java.io.FileNotFoundException; import java.net.URI; +import static org.apache.hadoop.yarn.logaggregation.filecontroller.LogAggregationFileController.TLDIR_PERMISSIONS; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; @@ -88,4 +94,51 @@ public class TestLogAggregationFileController { verify(fs).setOwner(any(), eq("yarn_user"), eq(testGroupName)); } + + private static class PathContainsString implements ArgumentMatcher { + private final String expected; + + PathContainsString(String expected) { + this.expected = expected; + } + + @Override + public boolean matches(Path path) { + return path.getName().contains(expected); + } + + @Override + public String toString() { + return "Path with name=" + expected; + } + } + + @Test + public void testRemoteDirCreationWithCustomUser() throws Exception { + FileSystem fs = mock(FileSystem.class); + doReturn(new URI("")).when(fs).getUri(); + doReturn(new FileStatus(128, false, 0, 64, System.currentTimeMillis(), + System.currentTimeMillis(), new FsPermission(TLDIR_PERMISSIONS), + "not_yarn_user", "yarn_group", new Path("/tmp/logs"))).when(fs) + .getFileStatus(any(Path.class)); + + Configuration conf = new Configuration(); + LogAggregationFileController controller = mock( + LogAggregationFileController.class, Mockito.CALLS_REAL_METHODS); + controller.fsSupportsChmod = true; + doReturn(fs).when(controller).getFileSystem(any(Configuration.class)); + + UserGroupInformation ugi = UserGroupInformation.createUserForTesting( + "yarn_user", new String[]{"yarn_group", "other_group"}); + UserGroupInformation.setLoginUser(ugi); + + controller.initialize(conf, "TFile"); + controller.verifyAndCreateRemoteLogDir(); + + verify(fs).createNewFile(argThat(new PathContainsString(".permission_check"))); + verify(fs).setPermission(argThat(new PathContainsString(".permission_check")), + eq(new FsPermission(TLDIR_PERMISSIONS))); + verify(fs).delete(argThat(new PathContainsString(".permission_check")), eq(false)); + Assert.assertTrue(controller.fsSupportsChmod); + } }