Repository: oozie Updated Branches: refs/heads/master b6452cfb0 -> 1c1225f8b
OOZIE-3330 [spark-action] Remove double quotes inside plain option values (asalamon74 via andras.piros) Project: http://git-wip-us.apache.org/repos/asf/oozie/repo Commit: http://git-wip-us.apache.org/repos/asf/oozie/commit/1c1225f8 Tree: http://git-wip-us.apache.org/repos/asf/oozie/tree/1c1225f8 Diff: http://git-wip-us.apache.org/repos/asf/oozie/diff/1c1225f8 Branch: refs/heads/master Commit: 1c1225f8b36c578d91ad70a301babdd652f85058 Parents: b6452cf Author: Andras Piros <[email protected]> Authored: Wed Aug 22 17:50:13 2018 +0200 Committer: Andras Piros <[email protected]> Committed: Wed Aug 22 17:50:13 2018 +0200 ---------------------------------------------------------------------- release-log.txt | 1 + .../action/hadoop/SparkOptionsSplitter.java | 10 ++-- .../action/hadoop/TestSparkArgsExtractor.java | 50 +++++++++++++++++++- .../action/hadoop/TestSparkOptionsSplitter.java | 6 +++ 4 files changed, 62 insertions(+), 5 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/oozie/blob/1c1225f8/release-log.txt ---------------------------------------------------------------------- diff --git a/release-log.txt b/release-log.txt index c0db2fc..237ab06 100644 --- a/release-log.txt +++ b/release-log.txt @@ -1,5 +1,6 @@ -- Oozie 5.1.0 release (trunk - unreleased) +OOZIE-3330 [spark-action] Remove double quotes inside plain option values (asalamon74 via andras.piros) OOZIE-3329 [build] test-patch-30-distro improvement (asalamon74 via andras.piros) OOZIE-3324 Cannot compile with findbugs check (asalamon74 via pbacsko) OOZIE-3304 Parsing sharelib timestamps is not threadsafe (dionusos, matijhs via andras.piros) http://git-wip-us.apache.org/repos/asf/oozie/blob/1c1225f8/sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkOptionsSplitter.java ---------------------------------------------------------------------- diff --git a/sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkOptionsSplitter.java b/sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkOptionsSplitter.java index b614322..ac6ee81 100644 --- a/sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkOptionsSplitter.java +++ b/sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkOptionsSplitter.java @@ -18,6 +18,7 @@ package org.apache.oozie.action.hadoop; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import org.apache.commons.lang.StringUtils; import java.util.ArrayList; @@ -49,7 +50,7 @@ class SparkOptionsSplitter { * <li>{@code spark.executor.extraJavaOptions="-XX:HeapDumpPath=/tmp"}</li> * </ul> */ - private static final String VALUE_HAS_QUOTES_AT_ENDS_REGEX = "[a-zA-Z0-9.]+=\".+\""; + private static final String VALUE_HAS_QUOTES_AT_ENDS_REGEX = "([a-zA-Z0-9.]+=)?\".+\""; /** * Matches an option key / value pair, where the value part has quotes in between. @@ -65,7 +66,7 @@ class SparkOptionsSplitter { * </ul> */ private static final String VALUE_HAS_QUOTES_IN_BETWEEN_REGEX = - "[a-zA-Z0-9.]+=.*(\\w\\s+\"\\w+[\\s+\\w]*\"|\"\\w+[\\s+\\w]*\"\\s+\\w)+.*"; + "([a-zA-Z0-9.]+=)?.*(\\w\\s+\"\\w+[\\s+\\w]*\"|\"\\w+[\\s+\\w]*\"\\s+\\w)+.*"; /** * Converts the options to be Spark-compatible. @@ -133,9 +134,10 @@ class SparkOptionsSplitter { * <li>{@code key="value1 value2 value3 value4"}: gets unquoted (has quotes both ends, and no quotes in between)</li> * </ul> * @param maybeEntirelyQuotedValue a {@code String} that is a parameter value but not necessarily quoted - * @return an unquoted version of the input {@String}, when {@code maybeEntirelyQuotedValue} had quotes at both ends, and didn't - * have any quotes in between. Else {@code maybeEntirelyQuotedValue} + * @return an unquoted version of the input {@code String}, when {@code maybeEntirelyQuotedValue} had quotes at both ends, + * and didn't have any quotes in between. Else {@code maybeEntirelyQuotedValue} */ + @SuppressFBWarnings(value = {"REDOS"}, justification = "Complex regular expression") private static String unquoteEntirelyQuotedValue(final String maybeEntirelyQuotedValue) { final boolean hasQuotesAtEnds = maybeEntirelyQuotedValue.matches(VALUE_HAS_QUOTES_AT_ENDS_REGEX); final boolean hasQuotesInBetween = maybeEntirelyQuotedValue.matches(VALUE_HAS_QUOTES_IN_BETWEEN_REGEX); http://git-wip-us.apache.org/repos/asf/oozie/blob/1c1225f8/sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkArgsExtractor.java ---------------------------------------------------------------------- diff --git a/sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkArgsExtractor.java b/sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkArgsExtractor.java index 60ab8b9..7ccd26a 100644 --- a/sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkArgsExtractor.java +++ b/sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkArgsExtractor.java @@ -225,7 +225,7 @@ public class TestSparkArgsExtractor { } @Test - public void testQuotedDriverAndExecutorExtraJavaOptionsParsing() throws Exception { + public void testQuotedConfDriverAndExecutorExtraJavaOptionsParsing() throws Exception { final Configuration actionConf = new Configuration(); actionConf.set(SparkActionExecutor.SPARK_MASTER, "yarn"); actionConf.set(SparkActionExecutor.SPARK_MODE, "client"); @@ -266,6 +266,54 @@ public class TestSparkArgsExtractor { } @Test + public void testQuotedPlainDriverJavaOptions() throws Exception { + final Configuration actionConf = new Configuration(); + actionConf.set(SparkActionExecutor.SPARK_MASTER, "yarn"); + actionConf.set(SparkActionExecutor.SPARK_MODE, "client"); + actionConf.set(SparkActionExecutor.SPARK_CLASS, "org.apache.oozie.example.SparkFileCopy"); + actionConf.set(SparkActionExecutor.SPARK_JOB_NAME, "Spark Copy File"); + actionConf.set(SparkActionExecutor.SPARK_OPTS, "--queue queueName " + + "--driver-memory 1g " + + "--executor-memory 1g " + + "--num-executors 1 " + + "--executor-cores 1 " + + "--driver-java-options \"-DhostName=localhost -DuserName=userName -DpassWord=password -DportNum=portNum\""); + actionConf.set(SparkActionExecutor.SPARK_JAR, "/lib/test.jar"); + + final String[] mainArgs = {"arg0", "arg1"}; + final List<String> sparkArgs = new SparkArgsExtractor(actionConf).extract(mainArgs); + + assertEquals("Spark args mismatch", + Lists.newArrayList("--master", "yarn", + "--deploy-mode", "client", + "--name", "Spark Copy File", + "--class", "org.apache.oozie.example.SparkFileCopy", + "--queue", "queueName", + "--driver-memory", "1g", + "--executor-memory", "1g", + "--num-executors", "1", + "--executor-cores", "1", + "--driver-java-options", "-DhostName=localhost -DuserName=userName -DpassWord=password -DportNum=portNum", + "--conf", "spark.executor.extraClassPath=$PWD/*", + "--conf", "spark.driver.extraClassPath=$PWD/*", + "--conf", "spark.yarn.security.tokens.hadoopfs.enabled=false", + "--conf", "spark.yarn.security.tokens.hive.enabled=false", + "--conf", "spark.yarn.security.tokens.hbase.enabled=false", + "--conf", "spark.yarn.security.credentials.hadoopfs.enabled=false", + "--conf", "spark.yarn.security.credentials.hive.enabled=false", + "--conf", "spark.yarn.security.credentials.hbase.enabled=false", + "--conf", "spark.executor.extraJavaOptions=-Dlog4j.configuration=spark-log4j.properties", + "--conf", "spark.driver.extraJavaOptions=-Dlog4j.configuration=spark-log4j.properties", + "--files", "spark-log4j.properties,hive-site.xml", + "--conf", "spark.yarn.jar=null", + "--verbose", + "/lib/test.jar", + "arg0", + "arg1"), + sparkArgs); + } + + @Test public void testPropertiesFileMerging() throws Exception { final Configuration actionConf = new Configuration(); actionConf.set(SparkActionExecutor.SPARK_MASTER, "yarn"); http://git-wip-us.apache.org/repos/asf/oozie/blob/1c1225f8/sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkOptionsSplitter.java ---------------------------------------------------------------------- diff --git a/sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkOptionsSplitter.java b/sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkOptionsSplitter.java index 4be9777..61ded5e 100644 --- a/sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkOptionsSplitter.java +++ b/sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkOptionsSplitter.java @@ -38,6 +38,10 @@ public class TestSparkOptionsSplitter { Arrays.asList("--option1", "value1")}, {"--option1 value1", Arrays.asList("--option1", "value1")}, + {"--option1 \"value1 value2\"", + Arrays.asList("--option1", "value1 value2")}, + {"--option1 \"value1 \"value2\" value3\"", + Arrays.asList("--option1", "\"value1 \"value2\" value3\"")}, {" --option1 value1 ", Arrays.asList("--option1", "value1")}, {"--conf special=value1", @@ -48,6 +52,8 @@ public class TestSparkOptionsSplitter { Arrays.asList("--conf", "special=value1 value2")}, {" --conf special=\"value1 value2\" ", Arrays.asList("--conf", "special=value1 value2")}, + {"--conf special=value1 \"value2\"", + Arrays.asList("--conf", "special=value1 \"value2\"")}, {"--conf special=value1 value2 --conf value3", Arrays.asList("--conf", "special=value1 value2", "--conf", "value3")}, {"--conf special1=value1 special2=\"value2 value3\" --conf value4",
