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 98d45f4..971b141 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 @@ -33,6 +33,8 @@ import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock.ReadLock; import java.util.concurrent.locks.ReentrantReadWriteLock.WriteLock; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import com.google.common.annotations.VisibleForTesting; import org.apache.commons.io.FileUtils; @@ -270,9 +272,14 @@ public void writeLaunchEnv(OutputStream out, whitelist.add(ApplicationConstants.Environment.HADOOP_CONF_DIR.name()); whitelist.add(ApplicationConstants.Environment.JAVA_HOME.name()); if (environment != null) { - for (Map.Entry env : environment.entrySet()) { + sb.reportError(); + sb.validatorRoutine(); + for (Map.Entry env : environment.entrySet()) { if (!whitelist.contains(env.getKey())) { sb.env(env.getKey().toString(), env.getValue().toString()); + + // Validate all shell variables explicitly passed for null check + extractAndValidateShellVariables(sb, env.getKey().toString(), env.getValue().toString()); } else { sb.whitelistedEnv(env.getKey().toString(), env.getValue().toString()); } @@ -318,6 +325,19 @@ public void writeLaunchEnv(OutputStream out, } } + private void extractAndValidateShellVariables( + ContainerLaunch.ShellScriptBuilder sb, String key, String value) { + + // Pattern matcher to find valid shell variables from a string. + // $ is the main delimeter here, and also considering {} () too. + Pattern p = Pattern.compile("\\$[a-zA-Z0-9{(_.]+[)}]*"); + Matcher m = p.matcher(value); + + while (m.find()) { + sb.validateShellVariable(key, m.group(0)); + } + } + public enum ExitCode { SUCCESS(0), FORCE_KILLED(137), 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 7e9030c..fbbdab6 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 @@ -824,6 +824,18 @@ protected final void line(String... command) { sb.append(LINE_SEPARATOR); } + public void validatorRoutine() { + // Dummy implementation. + } + + public void reportError() { + // Dummy implementation. + } + + public void validateShellVariable(String key, String value) { + // Dummy implementation. + } + protected abstract void link(Path src, Path dst) throws IOException; protected abstract void mkdir(Path path) throws IOException; @@ -839,6 +851,26 @@ private void errorCheck() { line("fi"); } + @Override + public void reportError() { + line("err_report() {"); + line(" echo \"Error on line ${1} while substituting for ${2}\""); + line("}"); + } + + @Override + public void validatorRoutine() { + line("verify_shell_variable() {"); + line(" echo \"Variable ${1} has a substitution named ${2:? `err_report ${LINENO} ${1}`}\""); + line("}"); + } + + @Override + public void validateShellVariable(String key, String value) { + line("toValidate=", value); + line("verify_shell_variable ", key, " ${toValidate}"); + } + public UnixShellScriptBuilder(){ line("#!/bin/bash"); line(); 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 a558338..3f1a869 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 @@ -36,6 +36,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; @@ -327,6 +328,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())); + 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())); + 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)