YARN-9125. Fixed Carriage Return detection in Docker container launch command.

Contributed by Billie Rinaldi

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/docker/DockerClient.java

@@ -105,13 +105,13 @@ public final class DockerClient {
                 "'=' found in entry for docker command file, key = " + entry
                     .getKey() + "; value = " + entry.getValue());
           }
-          if (entry.getValue().contains("\n")) {
+          String value = StringUtils.join(",", entry.getValue());
+          if (value.contains("\n")) {
             throw new ContainerExecutionException(
                 "'\\n' found in entry for docker command file, key = " + entry
-                    .getKey() + "; value = " + entry.getValue());
+                    .getKey() + "; value = " + value);
           }
-          printWriter.println("  " + entry.getKey() + "=" + StringUtils
+          printWriter.println("  " + entry.getKey() + "=" + value);
-              .join(",", entry.getValue()));
         }
         if (cmd instanceof DockerRunCommand) {
           DockerRunCommand runCommand = (DockerRunCommand) cmd;

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/docker/TestDockerClient.java

@@ -27,13 +27,16 @@ import org.apache.hadoop.yarn.api.records.ContainerId;
 import org.apache.hadoop.yarn.conf.YarnConfiguration;
 import org.apache.hadoop.yarn.server.nodemanager.Context;
 import org.apache.hadoop.yarn.server.nodemanager.LocalDirsHandlerService;
+import org.apache.hadoop.yarn.server.nodemanager.containermanager.runtime.ContainerExecutionException;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 
 import java.io.File;
+import java.util.Arrays;
 
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
 
@@ -77,4 +80,47 @@ public class TestDockerClient {
     File tmpFile = new File(tmpPath);
     assertTrue(tmpFile + " was not created", tmpFile.exists());
   }
+
+  @Test
+  public void testCommandValidation() throws Exception {
+    String absRoot = TEST_ROOT_DIR.getAbsolutePath();
+    ApplicationId appId = ApplicationId.newInstance(1, 1);
+    ApplicationAttemptId attemptId = ApplicationAttemptId.newInstance(appId, 1);
+    ContainerId cid = ContainerId.newContainerId(attemptId, 1);
+    Configuration conf = new Configuration();
+    conf.set("hadoop.tmp.dir", absRoot);
+    conf.set(YarnConfiguration.NM_LOCAL_DIRS, absRoot);
+    conf.set(YarnConfiguration.NM_LOG_DIRS, absRoot);
+    LocalDirsHandlerService dirsHandler = new LocalDirsHandlerService();
+    Context mockContext = mock(Context.class);
+    doReturn(conf).when(mockContext).getConf();
+    doReturn(dirsHandler).when(mockContext).getLocalDirsHandler();
+
+    DockerClient dockerClient = new DockerClient();
+    dirsHandler.init(conf);
+    dirsHandler.start();
+    try {
+      DockerRunCommand dockerCmd = new DockerRunCommand(cid.toString(), "user",
+          "image");
+      dockerCmd.addCommandArguments("prop=bad", "val");
+      dockerClient.writeCommandToTempFile(dockerCmd, cid,
+          mockContext);
+      fail("Expected exception writing command file");
+    } catch (ContainerExecutionException e) {
+      assertTrue("Expected key validation error", e.getMessage()
+          .contains("'=' found in entry for docker command file"));
+    }
+
+    try {
+      DockerRunCommand dockerCmd = new DockerRunCommand(cid.toString(), "user",
+          "image");
+      dockerCmd.setOverrideCommandWithArgs(Arrays.asList("sleep", "1000\n"));
+      dockerClient.writeCommandToTempFile(dockerCmd, cid, mockContext);
+      fail("Expected exception writing command file");
+    } catch (ContainerExecutionException e) {
+      assertTrue("Expected value validation error", e.getMessage()
+          .contains("'\\n' found in entry for docker command file"));
+    }
+    dirsHandler.stop();
+  }
 }