Fix tenant has no permission to execute kill command (#16909)

apache · Dec 21, 2024 · 1318a22 · 1318a22
1 parent c7b25eb
commit 1318a22
Show file tree

Hide file tree

Showing 6 changed files with 65 additions and 46 deletions.
diff --git a/...hinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/FileUtils.java b/...hinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/FileUtils.java
@@ -237,6 +237,10 @@ public static String getFileChecksum(String pathName) throws IOException {
     }
 
     public static void createFileWith755(@NonNull Path path) throws IOException {
+        final Path parent = path.getParent();
+        if (!parent.toFile().exists()) {
+            createDirectoryWith755(parent);
+        }
         if (SystemUtils.IS_OS_WINDOWS) {
             Files.createFile(path);
         } else {

diff --git a/...cheduler-common/src/test/java/org/apache/dolphinscheduler/common/utils/FileUtilsTest.java b/...cheduler-common/src/test/java/org/apache/dolphinscheduler/common/utils/FileUtilsTest.java
@@ -18,24 +18,31 @@
 package org.apache.dolphinscheduler.common.utils;
 
 import java.io.File;
-import java.io.FileInputStream;
-import java.io.FileNotFoundException;
 import java.io.IOException;
+import java.io.InputStream;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 
-import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.extension.ExtendWith;
-import org.mockito.junit.jupiter.MockitoExtension;
+import org.junit.jupiter.api.io.TempDir;
 
 import com.google.common.truth.Truth;
 
-@ExtendWith(MockitoExtension.class)
 public class FileUtilsTest {
 
+    @TempDir
+    public Path folder;
+
+    private String rootPath;
+
+    @BeforeEach
+    public void setUp() throws Exception {
+        rootPath = folder.toString();
+    }
+
     @Test
     public void testGetDownloadFilename() {
         Truth.assertThat(FileUtils.getDownloadFilename("test")).startsWith("/tmp/dolphinscheduler/tmp/");
@@ -74,14 +81,16 @@ public void createDirectoryWith755() throws IOException {
     }
 
     @Test
-    public void testWriteContent2File() throws FileNotFoundException {
+    public void testWriteContent2File() throws IOException {
         // file exists, fmt is invalid
-        String filePath = "test/testFile.txt";
+        String filePath = rootPath + "/testFile.txt";
         String content = "正正正faffdasfasdfas，한국어； 한글……にほんご\nfrançais";
         FileUtils.writeContent2File(content, filePath);
 
-        String fileContent = FileUtils.readFile2Str(new FileInputStream(filePath));
-        Assertions.assertEquals(content, fileContent);
+        try (final InputStream inputStream = Files.newInputStream(Paths.get(filePath))) {
+            final String fileContent = FileUtils.readFile2Str(inputStream);
+            Assertions.assertEquals(content, fileContent);
+        }
     }
 
     @Test
@@ -116,9 +125,9 @@ public void testDirectoryTraversal() {
 
     @Test
     void testGetFileChecksum() throws Exception {
-        String filePath1 = "test/testFile1.txt";
-        String filePath2 = "test/testFile2.txt";
-        String filePath3 = "test/testFile3.txt";
+        String filePath1 = rootPath + "/testFile1.txt";
+        String filePath2 = rootPath + "/testFile2.txt";
+        String filePath3 = rootPath + "/testFile3.txt";
         String content1 = "正正正faffdasfasdfas，한국어； 한글……にほんご\nfrançais";
         String content2 = "正正正faffdasfasdfas，한국어； 한글……にほん\nfrançais";
         FileUtils.writeContent2File(content1, filePath1);
@@ -132,15 +141,17 @@ void testGetFileChecksum() throws Exception {
         Assertions.assertNotEquals(checksum1, checksum2);
         Assertions.assertEquals(checksum1, checksum3);
 
-        String dirPath = "test/";
-
-        Assertions.assertDoesNotThrow(
-                () -> FileUtils.getFileChecksum(dirPath));
+        Assertions.assertDoesNotThrow(() -> FileUtils.getFileChecksum(rootPath));
     }
 
-    @AfterEach
-    public void tearDown() {
-        FileUtils.deleteFile("test");
+    @Test
+    void createFileWith755() throws IOException {
+        final String filePath = folder.toString() + "/test/a/b/test.txt";
+        FileUtils.createFileWith755(Paths.get(filePath));
+
+        final File file = new File(filePath);
+        Assertions.assertTrue(file.canRead());
+        Assertions.assertTrue(file.canWrite());
+        Assertions.assertTrue(file.canExecute());
     }
-
 }
diff --git a/...-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/am/ApplicationManager.java b/...-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/am/ApplicationManager.java
@@ -19,15 +19,15 @@
 
 import org.apache.dolphinscheduler.plugin.task.api.enums.ResourceManagerType;
 
-public interface ApplicationManager {
+public interface ApplicationManager<T extends ApplicationManagerContext> {
 
     /**
      * kill application by application manager context
      *
      * @param applicationManagerContext
      * @return
      */
-    boolean killApplication(ApplicationManagerContext applicationManagerContext);
+    boolean killApplication(T applicationManagerContext);
 
     /**
      * get resource manager type

diff --git a/...ain/java/org/apache/dolphinscheduler/plugin/task/api/am/KubernetesApplicationManager.java b/...ain/java/org/apache/dolphinscheduler/plugin/task/api/am/KubernetesApplicationManager.java
@@ -48,7 +48,7 @@
 
 @Slf4j
 @AutoService(ApplicationManager.class)
-public class KubernetesApplicationManager implements ApplicationManager {
+public class KubernetesApplicationManager implements ApplicationManager<KubernetesApplicationManagerContext> {
 
     private static final String PENDING = "Pending";
     private static final String RUNNING = "Running";
@@ -64,9 +64,7 @@ public class KubernetesApplicationManager implements ApplicationManager {
     private final Map<String, KubernetesClient> cacheClientMap = new ConcurrentHashMap<>();
 
     @Override
-    public boolean killApplication(ApplicationManagerContext applicationManagerContext) throws TaskException {
-        KubernetesApplicationManagerContext kubernetesApplicationManagerContext =
-                (KubernetesApplicationManagerContext) applicationManagerContext;
+    public boolean killApplication(KubernetesApplicationManagerContext kubernetesApplicationManagerContext) throws TaskException {
 
         boolean isKill;
         String labelValue = kubernetesApplicationManagerContext.getLabelValue();

diff --git a/.../src/main/java/org/apache/dolphinscheduler/plugin/task/api/am/YarnApplicationManager.java b/.../src/main/java/org/apache/dolphinscheduler/plugin/task/api/am/YarnApplicationManager.java
@@ -18,13 +18,17 @@
 package org.apache.dolphinscheduler.plugin.task.api.am;
 
 import org.apache.dolphinscheduler.common.constants.Constants;
+import org.apache.dolphinscheduler.common.utils.FileUtils;
 import org.apache.dolphinscheduler.common.utils.PropertyUtils;
 import org.apache.dolphinscheduler.plugin.task.api.TaskConstants;
 import org.apache.dolphinscheduler.plugin.task.api.TaskException;
 import org.apache.dolphinscheduler.plugin.task.api.enums.ResourceManagerType;
 
-import java.io.File;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.nio.file.StandardOpenOption;
 import java.util.List;
 
 import lombok.extern.slf4j.Slf4j;
@@ -33,12 +37,10 @@
 
 @Slf4j
 @AutoService(ApplicationManager.class)
-public class YarnApplicationManager implements ApplicationManager {
+public class YarnApplicationManager implements ApplicationManager<YarnApplicationManagerContext> {
 
     @Override
-    public boolean killApplication(ApplicationManagerContext applicationManagerContext) throws TaskException {
-        YarnApplicationManagerContext yarnApplicationManagerContext =
-                (YarnApplicationManagerContext) applicationManagerContext;
+    public boolean killApplication(YarnApplicationManagerContext yarnApplicationManagerContext) throws TaskException {
         String executePath = yarnApplicationManagerContext.getExecutePath();
         String tenantCode = yarnApplicationManagerContext.getTenantCode();
         List<String> appIds = yarnApplicationManagerContext.getAppIds();
@@ -77,17 +79,19 @@ private void execYarnKillCommand(String tenantCode, String commandFile,
         sb.append("\n\n");
         sb.append(cmd);
 
-        File f = new File(commandFile);
-
-        if (!f.exists()) {
-            org.apache.commons.io.FileUtils.writeStringToFile(new File(commandFile), sb.toString(),
-                    StandardCharsets.UTF_8);
+        final Path killCommandAbsolutePath = Paths.get(commandFile);
+        try {
+            FileUtils.createFileWith755(killCommandAbsolutePath);
+            Files.write(killCommandAbsolutePath, sb.toString().getBytes(StandardCharsets.UTF_8),
+                    StandardOpenOption.APPEND);
+
+            String runCmd = String.format("%s %s", Constants.SH, commandFile);
+            runCmd = org.apache.dolphinscheduler.common.utils.OSUtils.getSudoCmd(tenantCode, runCmd);
+            log.info("kill cmd:{}", runCmd);
+            org.apache.dolphinscheduler.common.utils.OSUtils.exeCmd(runCmd);
+        } finally {
+            Files.deleteIfExists(killCommandAbsolutePath);
         }
-
-        String runCmd = String.format("%s %s", Constants.SH, commandFile);
-        runCmd = org.apache.dolphinscheduler.common.utils.OSUtils.getSudoCmd(tenantCode, runCmd);
-        log.info("kill cmd:{}", runCmd);
-        org.apache.dolphinscheduler.common.utils.OSUtils.exeCmd(runCmd);
     }
 
     /**

diff --git a/...in/java/org/apache/dolphinscheduler/plugin/task/api/am/YarnApplicationManagerContext.java b/...in/java/org/apache/dolphinscheduler/plugin/task/api/am/YarnApplicationManagerContext.java
@@ -19,11 +19,13 @@
 
 import java.util.List;
 
-import lombok.RequiredArgsConstructor;
-import lombok.Value;
+import lombok.AllArgsConstructor;
+import lombok.Getter;
+import lombok.ToString;
 
-@Value
-@RequiredArgsConstructor
+@Getter
+@ToString
+@AllArgsConstructor
 public class YarnApplicationManagerContext implements ApplicationManagerContext {
 
     /**