From 0ad667a315910d01f1b6c17d6149c86af0b109ca Mon Sep 17 00:00:00 2001 From: Sunil G Date: Sun, 8 Jan 2017 20:03:57 +0530 Subject: [PATCH] YARN-5219 --- .../yarn/server/nodemanager/ContainerExecutor.java | 10 ++ .../containermanager/launcher/ContainerLaunch.java | 18 +++ .../launcher/TestContainerLaunch.java | 127 +++++++++++++++++++++ 3 files changed, 155 insertions(+) diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/ContainerExecutor.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/ContainerExecutor.java index 158585e..389dc4a 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/ContainerExecutor.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/ContainerExecutor.java @@ -326,6 +326,11 @@ public void writeLaunchEnv(OutputStream out, Map environment, } if (environment != null) { + // Add set -e to validate all env variables. + if(!environment.isEmpty()) { + sb.setEnvValidator(); + } + for (Map.Entry env : environment.entrySet()) { if (!whitelist.contains(env.getKey())) { sb.env(env.getKey(), env.getValue()); @@ -333,6 +338,11 @@ public void writeLaunchEnv(OutputStream out, Map environment, sb.whitelistedEnv(env.getKey(), env.getValue()); } } + + // Add set +e to stop validation as all env variables are validated. + if(!environment.isEmpty()) { + sb.reSetEnvValidator(); + } } if (resources != null) { diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java index 823457f..918d93f 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java @@ -874,6 +874,14 @@ protected final void line(String... command) { sb.append(LINE_SEPARATOR); } + public void setEnvValidator() { + // Dummy implementation + } + + public void reSetEnvValidator() { + // Dummy implementation + } + protected abstract void link(Path src, Path dst) throws IOException; protected abstract void mkdir(Path path) throws IOException; @@ -951,6 +959,16 @@ public void listDebugInformation(Path output) throws IOException { output.toString(), "\""); line("find -L . -maxdepth 5 -type l -ls 1>>\"", output.toString(), "\""); } + + @Override + public void setEnvValidator() { + line("set -e"); + } + + @Override + public void reSetEnvValidator() { + line("set +e"); + } } private static final class WindowsShellScriptBuilder diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/TestContainerLaunch.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/TestContainerLaunch.java index 23b99d9..697d720 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/TestContainerLaunch.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/TestContainerLaunch.java @@ -41,6 +41,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.StringTokenizer; @@ -372,6 +373,132 @@ public void testInvalidEnvSyntaxDiagnostics() throws IOException { } } + /* + * ${foo.version} is substituted to suffix a specific version number + */ + @Test + public void testInvalidEnvVariableSubstitutionType1() throws IOException { + Map env = new HashMap(); + // invalid env + env.put("testVar", "version${foo.version}"); + validateShellExecutorForDifferentEnvs(env); + } + + /* + * Multiple paths are substituted in a path variable + */ + @Test + public void testInvalidEnvVariableSubstitutionType2() throws IOException { + Map env = new HashMap(); + // invalid env + env.put("testPath", "/abc:/${foo.path}:/$bar"); + validateShellExecutorForDifferentEnvs(env); + } + + /* + * Direct assignment of a shell variable. We will fail fast in such cases. + */ + @Test + public void testInvalidEnvVariableSubstitutionType3() throws IOException { + Map env = new HashMap(); + // invalid env + env.put("testVar", "$(foo)"); + validateShellExecutorForDifferentEnvs(env); + } + + /* + * Direct assignment without any curley braces. + */ + @Test + public void testInvalidEnvVariableSubstitutionType4() throws IOException { + Map env = new HashMap(); + // invalid env + env.put("testVar", "$test"); + validateShellExecutorForDifferentEnvs(env); + } + + private void validateShellExecutorForDifferentEnvs(Map env) + throws IOException { + File shellFile = null; + try { + shellFile = Shell.appendScriptExtension(tmpDir, "hello"); + Map> resources = new HashMap>(); + FileOutputStream fos = new FileOutputStream(shellFile); + FileUtil.setExecutable(shellFile, true); + + List commands = new ArrayList(); + new DefaultContainerExecutor().writeLaunchEnv(fos, env, resources, + commands, new Path(localLogDir.getAbsolutePath()), user); + fos.flush(); + fos.close(); + + // It is supposed that LANG is set as C. + Map cmdEnv = new HashMap(); + cmdEnv.put("LANG", "C"); + Shell.ShellCommandExecutor shexc = new Shell.ShellCommandExecutor( + new String[] { shellFile.getAbsolutePath() }, tmpDir, cmdEnv); + String diagnostics = null; + try { + shexc.execute(); + Assert.fail("Should catch exception"); + } catch (ExitCodeException e) { + diagnostics = e.getMessage(); + } + + Assert.assertTrue(shexc.getExitCode() != 0); + } finally { + // cleanup + if (shellFile != null && shellFile.exists()) { + shellFile.delete(); + } + } + } + + @Test + public void testValidEnvVariableSubstitution() throws IOException { + File shellFile = null; + try { + shellFile = Shell.appendScriptExtension(tmpDir, "hello"); + Map> resources = + new HashMap>(); + FileOutputStream fos = new FileOutputStream(shellFile); + FileUtil.setExecutable(shellFile, true); + + Map env = new LinkedHashMap(); + // valid env + env.put( + "foo", "2.4.6" ); + env.put( + "testVar", "version${foo}" ); + List commands = new ArrayList(); + new DefaultContainerExecutor() + .writeLaunchEnv(fos, env, resources, commands, + new Path(localLogDir.getAbsolutePath()), user); + fos.flush(); + fos.close(); + + // It is supposed that LANG is set as C. + Map cmdEnv = new HashMap(); + cmdEnv.put("LANG", "C"); + Shell.ShellCommandExecutor shexc + = new Shell.ShellCommandExecutor(new String[]{shellFile.getAbsolutePath()}, + tmpDir, cmdEnv); + try { + shexc.execute(); + } catch(ExitCodeException e){ + Assert.fail("Should not catch exception"); + } + Assert.assertTrue(shexc.getExitCode() == 0); + } + finally { + // cleanup + if (shellFile != null + && shellFile.exists()) { + shellFile.delete(); + } + } + } + @Test(timeout = 10000) public void testEnvExpansion() throws IOException { Path logPath = new Path("/nm/container/logs"); -- 2.7.4 (Apple Git-66)