From d4e8c09593b8edc4d0494d64ce19ac78c6f31277 Mon Sep 17 00:00:00 2001 From: Allen Wittenauer Date: Tue, 14 Apr 2015 08:20:13 +0200 Subject: [PATCH] HADOOP-9642. Configuration to resolve environment variables via ${env.VARIABLE} references (Kengo Seki via aw) --- .../hadoop-common/CHANGES.txt | 6 +- .../org/apache/hadoop/conf/Configuration.java | 55 ++++++++++++- .../apache/hadoop/conf/TestConfiguration.java | 77 +++++++++++++------ 3 files changed, 110 insertions(+), 28 deletions(-) diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index 0fb6e9288c1..68913bc3d74 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/hadoop-common-project/hadoop-common/CHANGES.txt @@ -25,7 +25,8 @@ Trunk (Unreleased) NEW FEATURES - HADOOP-6590. Add a username check for hadoop sub-commands (John Smith via aw) + HADOOP-6590. Add a username check for hadoop sub-commands (John Smith via + aw) HADOOP-11353. Add support for .hadooprc (aw) @@ -41,6 +42,9 @@ Trunk (Unreleased) HADOOP-11565. Add --slaves shell option (aw) + HADOOP-9642. Configuration to resolve environment variables via + ${env.VARIABLE} references (Kengo Seki via aw) + IMPROVEMENTS HADOOP-8017. Configure hadoop-main pom to get rid of M2E plugin execution diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java index 8a312ffda81..7c25e6caaab 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java @@ -146,6 +146,8 @@ import com.google.common.base.Preconditions; * available properties are:
    *
  1. Other properties defined in this Configuration; and, if a name is * undefined here,
    2. + *
  2. Environment variables in {@link System#getenv()} if a name starts with + * "env.", or
    3. *
  3. Properties in {@link System#getProperties()}.
    4. *
* @@ -160,13 +162,25 @@ import com.google.common.base.Preconditions; * <property> * <name>tempdir</name> * <value>${basedir}/tmp</value> - * </property> + * </property> * - * When conf.get("tempdir") is called, then ${basedir} + * <property> + * <name>otherdir</name> + * <value>${env.BASE_DIR}/other</value> + * </property> + * + * + *

When conf.get("tempdir") is called, then ${basedir} * will be resolved to another property in this Configuration, while * ${user.name} would then ordinarily be resolved to the value * of the System property with that name. - * By default, warnings will be given to any deprecated configuration + *

When conf.get("otherdir") is called, then ${env.BASE_DIR} + * will be resolved to the value of the ${BASE_DIR} environment variable. + * It supports ${env.NAME:-default} and ${env.NAME-default} notations. + * The former is resolved to “default” if ${NAME} environment variable is undefined + * or its value is empty. + * The latter behaves the same way only if ${NAME} is undefined. + *

By default, warnings will be given to any deprecated configuration * parameters and these are suppressible by configuring * log4j.logger.org.apache.hadoop.conf.Configuration.deprecation in * log4j.properties file. @@ -915,6 +929,7 @@ public class Configuration implements Iterable>, * Attempts to repeatedly expand the value {@code expr} by replacing the * left-most substring of the form "${var}" in the following precedence order *

    + *
  1. by the value of the environment variable "var" if defined
    2. *
  2. by the value of the Java system property "var" if defined
    3. *
  3. by the value of the configuration key "var" if defined
    4. *
@@ -947,7 +962,31 @@ public class Configuration implements Iterable>, varBounds[SUB_END_IDX]); String val = null; try { - val = System.getProperty(var); + if (var.startsWith("env.") && 4 < var.length()) { + String v = var.substring(4); + int i = 0; + for (; i < v.length(); i++) { + char c = v.charAt(i); + if (c == ':' && i < v.length() - 1 && v.charAt(i + 1) == '-') { + val = getenv(v.substring(0, i)); + if (val == null || val.length() == 0) { + val = v.substring(i + 2); + } + break; + } else if (c == '-') { + val = getenv(v.substring(0, i)); + if (val == null) { + val = v.substring(i + 1); + } + break; + } + } + if (i == v.length()) { + val = getenv(v); + } + } else { + val = getProperty(var); + } } catch(SecurityException se) { LOG.warn("Unexpected SecurityException in Configuration", se); } @@ -979,6 +1018,14 @@ public class Configuration implements Iterable>, + MAX_SUBST + " " + expr); } + String getenv(String name) { + return System.getenv(name); + } + + String getProperty(String key) { + return System.getProperty(key); + } + /** * Get the value of the name property, null if * no such property exists. If the key is deprecated, it returns the value of diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfiguration.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfiguration.java index a3675537bf0..ec6c964bb9e 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfiguration.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfiguration.java @@ -50,6 +50,7 @@ import org.apache.hadoop.io.IOUtils; import org.apache.hadoop.net.NetUtils; import static org.apache.hadoop.util.PlatformName.IBM_JAVA; import org.codehaus.jackson.map.ObjectMapper; +import org.mockito.Mockito; public class TestConfiguration extends TestCase { @@ -148,46 +149,77 @@ public class TestConfiguration extends TestCase { } public void testVariableSubstitution() throws IOException { + // stubbing only environment dependent functions + Configuration mock = Mockito.spy(conf); + Mockito.when(mock.getProperty("user.name")).thenReturn("hadoop_user"); + Mockito.when(mock.getenv("FILE_NAME")).thenReturn("hello"); + out=new BufferedWriter(new FileWriter(CONFIG)); startConfig(); declareProperty("my.int", "${intvar}", "42"); declareProperty("intvar", "42", "42"); - declareProperty("my.base", "/tmp/${user.name}", UNSPEC); - declareProperty("my.file", "hello", "hello"); + declareProperty("my.base", "/tmp/${user.name}", "/tmp/hadoop_user"); + declareProperty("my.file", "${env.FILE_NAME}", "hello"); declareProperty("my.suffix", ".txt", ".txt"); declareProperty("my.relfile", "${my.file}${my.suffix}", "hello.txt"); - declareProperty("my.fullfile", "${my.base}/${my.file}${my.suffix}", UNSPEC); + declareProperty("my.fullfile", "${my.base}/${my.file}${my.suffix}", "/tmp/hadoop_user/hello.txt"); // check that undefined variables are returned as-is declareProperty("my.failsexpand", "a${my.undefvar}b", "a${my.undefvar}b"); endConfig(); Path fileResource = new Path(CONFIG); - conf.addResource(fileResource); + mock.addResource(fileResource); for (Prop p : props) { System.out.println("p=" + p.name); - String gotVal = conf.get(p.name); - String gotRawVal = conf.getRaw(p.name); + String gotVal = mock.get(p.name); + String gotRawVal = mock.getRaw(p.name); assertEq(p.val, gotRawVal); - if (p.expectEval == UNSPEC) { - // expansion is system-dependent (uses System properties) - // can't do exact match so just check that all variables got expanded - assertTrue(gotVal != null && -1 == gotVal.indexOf("${")); - } else { - assertEq(p.expectEval, gotVal); - } + assertEq(p.expectEval, gotVal); } // check that expansion also occurs for getInt() - assertTrue(conf.getInt("intvar", -1) == 42); - assertTrue(conf.getInt("my.int", -1) == 42); + assertTrue(mock.getInt("intvar", -1) == 42); + assertTrue(mock.getInt("my.int", -1) == 42); + } - Map results = conf.getValByRegex("^my.*file$"); - assertTrue(results.keySet().contains("my.relfile")); - assertTrue(results.keySet().contains("my.fullfile")); - assertTrue(results.keySet().contains("my.file")); - assertEquals(-1, results.get("my.relfile").indexOf("${")); - assertEquals(-1, results.get("my.fullfile").indexOf("${")); - assertEquals(-1, results.get("my.file").indexOf("${")); + public void testEnvDefault() throws IOException { + Configuration mock = Mockito.spy(conf); + Mockito.when(mock.getenv("NULL_VALUE")).thenReturn(null); + Mockito.when(mock.getenv("EMPTY_VALUE")).thenReturn(""); + Mockito.when(mock.getenv("SOME_VALUE")).thenReturn("some value"); + + out=new BufferedWriter(new FileWriter(CONFIG)); + startConfig(); + + // if var is unbound, literal ${var} is returned + declareProperty("null1", "${env.NULL_VALUE}", "${env.NULL_VALUE}"); + declareProperty("null2", "${env.NULL_VALUE-a}", "a"); + declareProperty("null3", "${env.NULL_VALUE:-b}", "b"); + declareProperty("empty1", "${env.EMPTY_VALUE}", ""); + declareProperty("empty2", "${env.EMPTY_VALUE-c}", ""); + declareProperty("empty3", "${env.EMPTY_VALUE:-d}", "d"); + declareProperty("some1", "${env.SOME_VALUE}", "some value"); + declareProperty("some2", "${env.SOME_VALUE-e}", "some value"); + declareProperty("some3", "${env.SOME_VALUE:-f}", "some value"); + + // some edge cases + declareProperty("edge1", "${env.NULL_VALUE-g-h}", "g-h"); + declareProperty("edge2", "${env.NULL_VALUE:-i:-j}", "i:-j"); + declareProperty("edge3", "${env.NULL_VALUE-}", ""); + declareProperty("edge4", "${env.NULL_VALUE:-}", ""); + declareProperty("edge5", "${env.NULL_VALUE:}", "${env.NULL_VALUE:}"); + + endConfig(); + Path fileResource = new Path(CONFIG); + mock.addResource(fileResource); + + for (Prop p : props) { + System.out.println("p=" + p.name); + String gotVal = mock.get(p.name); + String gotRawVal = mock.getRaw(p.name); + assertEq(p.val, gotRawVal); + assertEq(p.expectEval, gotVal); + } } public void testFinalParam() throws IOException { @@ -247,7 +279,6 @@ public class TestConfiguration extends TestCase { String expectEval; } - final String UNSPEC = null; ArrayList props = new ArrayList(); void declareProperty(String name, String val, String expectEval)