Repository: oozie Updated Branches: refs/heads/master bc5395dc9 -> 70f34bdd6
OOZIE-3370 Property filtering is not consistent across job submission (andras.piros) Project: http://git-wip-us.apache.org/repos/asf/oozie/repo Commit: http://git-wip-us.apache.org/repos/asf/oozie/commit/70f34bdd Tree: http://git-wip-us.apache.org/repos/asf/oozie/tree/70f34bdd Diff: http://git-wip-us.apache.org/repos/asf/oozie/diff/70f34bdd Branch: refs/heads/master Commit: 70f34bdd6d2a6732bc6b8b705172c5d9acbe5665 Parents: bc5395d Author: Andras Piros <andras.pi...@cloudera.com> Authored: Wed Oct 17 19:08:48 2018 +0200 Committer: Andras Piros <andras.pi...@cloudera.com> Committed: Wed Oct 17 19:08:48 2018 +0200 ---------------------------------------------------------------------- .../oozie/action/hadoop/JavaActionExecutor.java | 4 +- .../hadoop/LauncherConfigurationInjector.java | 10 +- .../action/oozie/SubWorkflowActionExecutor.java | 9 +- .../command/bundle/BundleStartXCommand.java | 8 ++ .../command/bundle/BundleSubmitXCommand.java | 7 +- .../command/coord/CoordActionStartXCommand.java | 10 +- .../command/coord/CoordSubmitXCommand.java | 7 +- .../command/coord/CoordUpdateXCommand.java | 8 ++ .../apache/oozie/command/wf/ReRunXCommand.java | 8 +- .../oozie/command/wf/SubmitHttpXCommand.java | 10 +- .../apache/oozie/command/wf/SubmitXCommand.java | 8 +- .../org/apache/oozie/servlet/V1JobsServlet.java | 11 ++ .../java/org/apache/oozie/util/ConfigUtils.java | 61 +++++++++++ .../org/apache/oozie/util/PropertiesUtils.java | 7 ++ .../TestLauncherConfigurationInjector.java | 28 ++++- .../TestCoordActionInputCheckXCommand.java | 4 +- .../coord/TestCoordActionStartXCommand.java | 4 +- .../org/apache/oozie/util/TestConfigUtils.java | 106 +++++++++++++++++++ release-log.txt | 1 + 19 files changed, 263 insertions(+), 48 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/oozie/blob/70f34bdd/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java b/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java index 7305a12..3b90268 100644 --- a/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java +++ b/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java @@ -166,7 +166,7 @@ public class JavaActionExecutor extends ActionExecutor { private static final String JAVA_MAIN_CLASS_NAME = "org.apache.oozie.action.hadoop.JavaMain"; private static final String HADOOP_JOB_NAME = "mapred.job.name"; - private static final Set<String> DISALLOWED_PROPERTIES = new HashSet<String>(); + static final Set<String> DISALLOWED_PROPERTIES = new HashSet<>(); private static final String OOZIE_ACTION_NAME = "oozie.action.name"; private final static String ACTION_SHARELIB_FOR = "oozie.action.sharelib.for."; public static final String OOZIE_ACTION_DEPENDENCY_DEDUPLICATE = "oozie.action.dependency.deduplicate"; @@ -210,7 +210,7 @@ public class JavaActionExecutor extends ActionExecutor { public XConfiguration workflowConf = null; static { - DISALLOWED_PROPERTIES.add(HADOOP_USER); + DISALLOWED_PROPERTIES.addAll(PropertiesUtils.DEFAULT_DISALLOWED_PROPERTIES); DISALLOWED_PROPERTIES.add(HADOOP_NAME_NODE); DISALLOWED_PROPERTIES.add(HADOOP_YARN_RM); } http://git-wip-us.apache.org/repos/asf/oozie/blob/70f34bdd/core/src/main/java/org/apache/oozie/action/hadoop/LauncherConfigurationInjector.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/oozie/action/hadoop/LauncherConfigurationInjector.java b/core/src/main/java/org/apache/oozie/action/hadoop/LauncherConfigurationInjector.java index c2c0563..a93f4de 100644 --- a/core/src/main/java/org/apache/oozie/action/hadoop/LauncherConfigurationInjector.java +++ b/core/src/main/java/org/apache/oozie/action/hadoop/LauncherConfigurationInjector.java @@ -23,6 +23,7 @@ import com.google.common.collect.LinkedHashMultimap; import com.google.common.collect.Maps; import com.google.common.collect.Multimap; import org.apache.hadoop.conf.Configuration; +import org.apache.oozie.action.ActionExecutorException; import org.apache.oozie.service.ConfigurationService; import org.apache.oozie.util.XLog; @@ -193,7 +194,7 @@ class LauncherConfigurationInjector { * Inject the overridden and prepended values from {@link #sourceConfiguration} into {@code launcherConf} wherever applicable. * @param launcherConf the target {@code Configuration} of the Launcher AM */ - void inject(final Configuration launcherConf) { + void inject(final Configuration launcherConf) throws ActionExecutorException { LOG.debug("Injecting configuration entries to launcher configuration."); copyToLauncherConf(sourceConfiguration, launcherConf); @@ -345,10 +346,15 @@ class LauncherConfigurationInjector { * @param source * @param target */ - private void copyToLauncherConf(final Configuration source, final Configuration target) { + private void copyToLauncherConf(final Configuration source, final Configuration target) throws ActionExecutorException { for (final Map.Entry<String, String> entry : source) { if (entry.getKey().startsWith(OOZIE_LAUNCHER_PREFIX)) { final String name = entry.getKey().substring(OOZIE_LAUNCHER_PREFIX.length()); + if (JavaActionExecutor.DISALLOWED_PROPERTIES.contains(name)) { + LOG.error("Property [{0}] not allowed in launcher configuration", name); + throw new ActionExecutorException(ActionExecutorException.ErrorType.FAILED, "JA010", + "Property [{0}] not allowed in launcher configuration", name); + } final String value = entry.getValue(); // setting original KEY target.set(entry.getKey(), value); http://git-wip-us.apache.org/repos/asf/oozie/blob/70f34bdd/core/src/main/java/org/apache/oozie/action/oozie/SubWorkflowActionExecutor.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/oozie/action/oozie/SubWorkflowActionExecutor.java b/core/src/main/java/org/apache/oozie/action/oozie/SubWorkflowActionExecutor.java index e33056c..8a2fd71 100644 --- a/core/src/main/java/org/apache/oozie/action/oozie/SubWorkflowActionExecutor.java +++ b/core/src/main/java/org/apache/oozie/action/oozie/SubWorkflowActionExecutor.java @@ -56,7 +56,7 @@ public class SubWorkflowActionExecutor extends ActionExecutor { public static final String SUBWORKFLOW_DEPTH = "oozie.action.subworkflow.depth"; public static final String SUBWORKFLOW_RERUN = "oozie.action.subworkflow.rerun"; - private static final Set<String> DISALLOWED_DEFAULT_PROPERTIES = new HashSet<String>(); + private static final Set<String> DISALLOWED_USER_PROPERTIES = new HashSet<String>(); public XLog LOG = XLog.getLog(getClass()); @@ -66,9 +66,7 @@ public class SubWorkflowActionExecutor extends ActionExecutor { PropertiesUtils.RECORDS, PropertiesUtils.MAP_IN, PropertiesUtils.MAP_OUT, PropertiesUtils.REDUCE_IN, PropertiesUtils.REDUCE_OUT, PropertiesUtils.GROUPS}; - String[] badDefaultProps = {PropertiesUtils.HADOOP_USER}; - PropertiesUtils.createPropertySet(badUserProps, DISALLOWED_DEFAULT_PROPERTIES); - PropertiesUtils.createPropertySet(badDefaultProps, DISALLOWED_DEFAULT_PROPERTIES); + PropertiesUtils.createPropertySet(badUserProps, DISALLOWED_USER_PROPERTIES); } protected SubWorkflowActionExecutor() { @@ -101,7 +99,8 @@ public class SubWorkflowActionExecutor extends ActionExecutor { String strConf = XmlUtils.prettyPrint(eConf).toString(); Configuration conf = new XConfiguration(new StringReader(strConf)); try { - PropertiesUtils.checkDisallowedProperties(conf, DISALLOWED_DEFAULT_PROPERTIES); + PropertiesUtils.checkDisallowedProperties(conf, DISALLOWED_USER_PROPERTIES); + PropertiesUtils.checkDefaultDisallowedProperties(conf); } catch (CommandException ex) { throw convertException(ex); http://git-wip-us.apache.org/repos/asf/oozie/blob/70f34bdd/core/src/main/java/org/apache/oozie/command/bundle/BundleStartXCommand.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/oozie/command/bundle/BundleStartXCommand.java b/core/src/main/java/org/apache/oozie/command/bundle/BundleStartXCommand.java index 88de946..5401746 100644 --- a/core/src/main/java/org/apache/oozie/command/bundle/BundleStartXCommand.java +++ b/core/src/main/java/org/apache/oozie/command/bundle/BundleStartXCommand.java @@ -43,6 +43,7 @@ import org.apache.oozie.executor.jpa.BundleJobQueryExecutor; import org.apache.oozie.executor.jpa.BundleJobQueryExecutor.BundleJobQuery; import org.apache.oozie.executor.jpa.JPAExecutorException; import org.apache.oozie.executor.jpa.BatchQueryExecutor.UpdateEntry; +import org.apache.oozie.util.ConfigUtils; import org.apache.oozie.util.ELUtils; import org.apache.oozie.util.JobUtils; import org.apache.oozie.util.LogUtils; @@ -312,6 +313,13 @@ public class BundleStartXCommand extends StartTransitionXCommand { // copy configuration properties in the coordElem to the runConf XConfiguration.copy(localConf, runConf); + + ConfigUtils.checkAndSetDisallowedProperties(runConf, + bundleJob.getUser(), + new CommandException(ErrorCode.E1303, + String.format("%s=%s", OozieClient.USER_NAME, runConf.get(OozieClient.USER_NAME)), + bundleJob.getUser()), + true); } // Step 3: Extract value of 'app-path' in coordElem, save it as a http://git-wip-us.apache.org/repos/asf/oozie/blob/70f34bdd/core/src/main/java/org/apache/oozie/command/bundle/BundleSubmitXCommand.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/oozie/command/bundle/BundleSubmitXCommand.java b/core/src/main/java/org/apache/oozie/command/bundle/BundleSubmitXCommand.java index c06759e..1ee2237 100644 --- a/core/src/main/java/org/apache/oozie/command/bundle/BundleSubmitXCommand.java +++ b/core/src/main/java/org/apache/oozie/command/bundle/BundleSubmitXCommand.java @@ -91,10 +91,6 @@ public class BundleSubmitXCommand extends SubmitTransitionXCommand { PropertiesUtils.TB, PropertiesUtils.PB, PropertiesUtils.RECORDS, PropertiesUtils.MAP_IN, PropertiesUtils.MAP_OUT, PropertiesUtils.REDUCE_IN, PropertiesUtils.REDUCE_OUT, PropertiesUtils.GROUPS }; PropertiesUtils.createPropertySet(badUserProps, DISALLOWED_USER_PROPERTIES); - - String[] badDefaultProps = { PropertiesUtils.HADOOP_USER}; - PropertiesUtils.createPropertySet(badUserProps, DISALLOWED_DEFAULT_PROPERTIES); - PropertiesUtils.createPropertySet(badDefaultProps, DISALLOWED_DEFAULT_PROPERTIES); } /** @@ -243,7 +239,8 @@ public class BundleSubmitXCommand extends SubmitTransitionXCommand { if (fs.exists(configDefault)) { Configuration defaultConf = new XConfiguration(fs.open(configDefault)); - PropertiesUtils.checkDisallowedProperties(defaultConf, DISALLOWED_DEFAULT_PROPERTIES); + PropertiesUtils.checkDisallowedProperties(defaultConf, DISALLOWED_USER_PROPERTIES); + PropertiesUtils.checkDefaultDisallowedProperties(defaultConf); XConfiguration.injectDefaults(defaultConf, conf); } else { http://git-wip-us.apache.org/repos/asf/oozie/blob/70f34bdd/core/src/main/java/org/apache/oozie/command/coord/CoordActionStartXCommand.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/oozie/command/coord/CoordActionStartXCommand.java b/core/src/main/java/org/apache/oozie/command/coord/CoordActionStartXCommand.java index dbc188a..267943e 100644 --- a/core/src/main/java/org/apache/oozie/command/coord/CoordActionStartXCommand.java +++ b/core/src/main/java/org/apache/oozie/command/coord/CoordActionStartXCommand.java @@ -34,12 +34,13 @@ import org.apache.oozie.service.DagEngineService; import org.apache.oozie.service.EventHandlerService; import org.apache.oozie.service.JPAService; import org.apache.oozie.service.Services; +import org.apache.oozie.util.ConfigUtils; import org.apache.oozie.util.JobUtils; import org.apache.oozie.util.LogUtils; import org.apache.oozie.util.ParamChecker; +import org.apache.oozie.util.XConfiguration; import org.apache.oozie.util.XLog; import org.apache.oozie.util.XmlUtils; -import org.apache.oozie.util.XConfiguration; import org.apache.oozie.util.db.SLADbOperations; import org.apache.oozie.client.SLAEvent.SlaAppType; import org.apache.oozie.client.SLAEvent.Status; @@ -172,6 +173,13 @@ public class CoordActionStartXCommand extends CoordinatorXCommand<Void> { } runConf.unset(CoordRerunXCommand.RERUN_CONF); } + + ConfigUtils.checkAndSetDisallowedProperties(runConf, + this.user, + new CommandException(ErrorCode.E1003, + String.format("%s=%s", OozieClient.USER_NAME, runConf.get(OozieClient.USER_NAME))), + true); + return runConf; } http://git-wip-us.apache.org/repos/asf/oozie/blob/70f34bdd/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java b/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java index 2e25e03..c1c6444 100644 --- a/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java +++ b/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java @@ -155,10 +155,6 @@ public class CoordSubmitXCommand extends SubmitTransitionXCommand { PropertiesUtils.TB, PropertiesUtils.PB, PropertiesUtils.RECORDS, PropertiesUtils.MAP_IN, PropertiesUtils.MAP_OUT, PropertiesUtils.REDUCE_IN, PropertiesUtils.REDUCE_OUT, PropertiesUtils.GROUPS }; PropertiesUtils.createPropertySet(badUserProps, DISALLOWED_USER_PROPERTIES); - - String[] badDefaultProps = { PropertiesUtils.HADOOP_USER}; - PropertiesUtils.createPropertySet(badUserProps, DISALLOWED_DEFAULT_PROPERTIES); - PropertiesUtils.createPropertySet(badDefaultProps, DISALLOWED_DEFAULT_PROPERTIES); } /** @@ -569,7 +565,8 @@ public class CoordSubmitXCommand extends SubmitTransitionXCommand { if (fs.exists(configDefault)) { Configuration defaultConf = new XConfiguration(fs.open(configDefault)); - PropertiesUtils.checkDisallowedProperties(defaultConf, DISALLOWED_DEFAULT_PROPERTIES); + PropertiesUtils.checkDisallowedProperties(defaultConf, DISALLOWED_USER_PROPERTIES); + PropertiesUtils.checkDefaultDisallowedProperties(defaultConf); XConfiguration.injectDefaults(defaultConf, conf); } else { http://git-wip-us.apache.org/repos/asf/oozie/blob/70f34bdd/core/src/main/java/org/apache/oozie/command/coord/CoordUpdateXCommand.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/oozie/command/coord/CoordUpdateXCommand.java b/core/src/main/java/org/apache/oozie/command/coord/CoordUpdateXCommand.java index 9efff12..758c5c2 100644 --- a/core/src/main/java/org/apache/oozie/command/coord/CoordUpdateXCommand.java +++ b/core/src/main/java/org/apache/oozie/command/coord/CoordUpdateXCommand.java @@ -36,6 +36,7 @@ import org.apache.oozie.executor.jpa.JPAExecutorException; import org.apache.oozie.executor.jpa.CoordJobQueryExecutor.CoordJobQuery; import org.apache.oozie.service.JPAService; import org.apache.oozie.service.Services; +import org.apache.oozie.util.ConfigUtils; import org.apache.oozie.util.LogUtils; import org.apache.oozie.util.XConfiguration; import org.apache.oozie.util.XmlUtils; @@ -78,6 +79,13 @@ public class CoordUpdateXCommand extends CoordSubmitXCommand { @Override protected String storeToDB(String xmlElement, Element eJob, CoordinatorJobBean coordJob) throws CommandException { check(oldCoordJob, coordJob); + + ConfigUtils.checkAndSetDisallowedProperties(conf, + this.oldCoordJob.getUser(), + new CommandException(ErrorCode.E1003, + String.format("%s=%s", OozieClient.USER_NAME, conf.get(OozieClient.USER_NAME))), + true); + computeDiff(eJob); oldCoordJob.setAppPath(conf.get(OozieClient.COORDINATOR_APP_PATH)); if (isConfChange) { http://git-wip-us.apache.org/repos/asf/oozie/blob/70f34bdd/core/src/main/java/org/apache/oozie/command/wf/ReRunXCommand.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/oozie/command/wf/ReRunXCommand.java b/core/src/main/java/org/apache/oozie/command/wf/ReRunXCommand.java index f4497c5..58f203f 100644 --- a/core/src/main/java/org/apache/oozie/command/wf/ReRunXCommand.java +++ b/core/src/main/java/org/apache/oozie/command/wf/ReRunXCommand.java @@ -93,7 +93,6 @@ public class ReRunXCommand extends WorkflowXCommand<Void> { private List<UpdateEntry> updateList = new ArrayList<UpdateEntry>(); private List<JsonBean> deleteList = new ArrayList<JsonBean>(); - private static final Set<String> DISALLOWED_DEFAULT_PROPERTIES = new HashSet<String>(); private static final Set<String> DISALLOWED_USER_PROPERTIES = new HashSet<String>(); public static final String DISABLE_CHILD_RERUN = "oozie.wf.rerun.disablechild"; @@ -103,10 +102,6 @@ public class ReRunXCommand extends WorkflowXCommand<Void> { PropertiesUtils.RECORDS, PropertiesUtils.MAP_IN, PropertiesUtils.MAP_OUT, PropertiesUtils.REDUCE_IN, PropertiesUtils.REDUCE_OUT, PropertiesUtils.GROUPS }; PropertiesUtils.createPropertySet(badUserProps, DISALLOWED_USER_PROPERTIES); - - String[] badDefaultProps = { PropertiesUtils.HADOOP_USER}; - PropertiesUtils.createPropertySet(badUserProps, DISALLOWED_DEFAULT_PROPERTIES); - PropertiesUtils.createPropertySet(badDefaultProps, DISALLOWED_DEFAULT_PROPERTIES); } public ReRunXCommand(String jobId, Configuration conf) { @@ -163,7 +158,8 @@ public class ReRunXCommand extends WorkflowXCommand<Void> { if (fs.exists(configDefault)) { Configuration defaultConf = new XConfiguration(fs.open(configDefault)); - PropertiesUtils.checkDisallowedProperties(defaultConf, DISALLOWED_DEFAULT_PROPERTIES); + PropertiesUtils.checkDisallowedProperties(defaultConf, DISALLOWED_USER_PROPERTIES); + PropertiesUtils.checkDefaultDisallowedProperties(defaultConf); XConfiguration.injectDefaults(defaultConf, conf); } http://git-wip-us.apache.org/repos/asf/oozie/blob/70f34bdd/core/src/main/java/org/apache/oozie/command/wf/SubmitHttpXCommand.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/oozie/command/wf/SubmitHttpXCommand.java b/core/src/main/java/org/apache/oozie/command/wf/SubmitHttpXCommand.java index 1762800..9ee8406 100644 --- a/core/src/main/java/org/apache/oozie/command/wf/SubmitHttpXCommand.java +++ b/core/src/main/java/org/apache/oozie/command/wf/SubmitHttpXCommand.java @@ -66,7 +66,6 @@ public abstract class SubmitHttpXCommand extends WorkflowXCommand<String> { this.conf = ParamChecker.notNull(conf, "conf"); } - private static final Set<String> DISALLOWED_DEFAULT_PROPERTIES = new HashSet<String>(); private static final Set<String> DISALLOWED_USER_PROPERTIES = new HashSet<String>(); static { @@ -75,10 +74,6 @@ public abstract class SubmitHttpXCommand extends WorkflowXCommand<String> { PropertiesUtils.RECORDS, PropertiesUtils.MAP_IN, PropertiesUtils.MAP_OUT, PropertiesUtils.REDUCE_IN, PropertiesUtils.REDUCE_OUT, PropertiesUtils.GROUPS }; PropertiesUtils.createPropertySet(badUserProps, DISALLOWED_USER_PROPERTIES); - - String[] badDefaultProps = { PropertiesUtils.HADOOP_USER}; - PropertiesUtils.createPropertySet(badUserProps, DISALLOWED_DEFAULT_PROPERTIES); - PropertiesUtils.createPropertySet(badDefaultProps, DISALLOWED_DEFAULT_PROPERTIES); } abstract protected Element generateSection(Configuration conf, Namespace ns); @@ -189,6 +184,7 @@ public abstract class SubmitHttpXCommand extends WorkflowXCommand<String> { WorkflowLib workflowLib = Services.get().get(WorkflowStoreService.class).getWorkflowLibWithNoDB(); PropertiesUtils.checkDisallowedProperties(conf, DISALLOWED_USER_PROPERTIES); + PropertiesUtils.checkDefaultDisallowedProperties(conf); // Resolving all variables in the job properties. // This ensures the Hadoop Configuration semantics is preserved. @@ -269,7 +265,7 @@ public abstract class SubmitHttpXCommand extends WorkflowXCommand<String> { /** * Add file section in X. * - * @param parent XML element to be appended + * @param X XML element to be appended * @param conf Configuration object * @param ns XML element namespace */ @@ -281,7 +277,7 @@ public abstract class SubmitHttpXCommand extends WorkflowXCommand<String> { /** * Add archive section in X. * - * @param parent XML element to be appended + * @param X XML element to be appended * @param conf Configuration object * @param ns XML element namespace */ http://git-wip-us.apache.org/repos/asf/oozie/blob/70f34bdd/core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java b/core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java index a4d3e2f..1509c38 100644 --- a/core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java +++ b/core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java @@ -113,7 +113,6 @@ public class SubmitXCommand extends WorkflowXCommand<String> { this.dryrun = dryrun; } - private static final Set<String> DISALLOWED_DEFAULT_PROPERTIES = new HashSet<String>(); private static final Set<String> DISALLOWED_USER_PROPERTIES = new HashSet<String>(); static { @@ -122,10 +121,6 @@ public class SubmitXCommand extends WorkflowXCommand<String> { PropertiesUtils.RECORDS, PropertiesUtils.MAP_IN, PropertiesUtils.MAP_OUT, PropertiesUtils.REDUCE_IN, PropertiesUtils.REDUCE_OUT, PropertiesUtils.GROUPS}; PropertiesUtils.createPropertySet(badUserProps, DISALLOWED_USER_PROPERTIES); - - String[] badDefaultProps = {PropertiesUtils.HADOOP_USER}; - PropertiesUtils.createPropertySet(badUserProps, DISALLOWED_DEFAULT_PROPERTIES); - PropertiesUtils.createPropertySet(badDefaultProps, DISALLOWED_DEFAULT_PROPERTIES); } @Override @@ -153,7 +148,8 @@ public class SubmitXCommand extends WorkflowXCommand<String> { if (fs.exists(configDefault)) { try { defaultConf = new XConfiguration(fs.open(configDefault)); - PropertiesUtils.checkDisallowedProperties(defaultConf, DISALLOWED_DEFAULT_PROPERTIES); + PropertiesUtils.checkDisallowedProperties(defaultConf, DISALLOWED_USER_PROPERTIES); + PropertiesUtils.checkDefaultDisallowedProperties(defaultConf); XConfiguration.injectDefaults(defaultConf, conf); } catch (IOException ex) { http://git-wip-us.apache.org/repos/asf/oozie/blob/70f34bdd/core/src/main/java/org/apache/oozie/servlet/V1JobsServlet.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/oozie/servlet/V1JobsServlet.java b/core/src/main/java/org/apache/oozie/servlet/V1JobsServlet.java index 5cfcf57..b7967be 100644 --- a/core/src/main/java/org/apache/oozie/servlet/V1JobsServlet.java +++ b/core/src/main/java/org/apache/oozie/servlet/V1JobsServlet.java @@ -64,6 +64,7 @@ import org.apache.oozie.service.DagEngineService; import org.apache.oozie.service.HadoopAccessorException; import org.apache.oozie.service.HadoopAccessorService; import org.apache.oozie.service.Services; +import org.apache.oozie.util.ConfigUtils; import org.apache.oozie.util.IOUtils; import org.apache.oozie.util.XLog; import org.apache.oozie.util.XmlUtils; @@ -95,6 +96,16 @@ public class V1JobsServlet extends BaseJobsServlet { String jobType = request.getParameter(RestConstants.JOBTYPE_PARAM); + if (!getUser(request).equals(UNDEF)) { + ConfigUtils.checkAndSetDisallowedProperties(conf, + getUser(request), + new XServletException(HttpServletResponse.SC_BAD_REQUEST, + ErrorCode.E0303, + "configuration", + OozieClient.USER_NAME), + false); + } + if (jobType == null) { String wfPath = conf.get(OozieClient.APP_PATH); String coordPath = conf.get(OozieClient.COORDINATOR_APP_PATH); http://git-wip-us.apache.org/repos/asf/oozie/blob/70f34bdd/core/src/main/java/org/apache/oozie/util/ConfigUtils.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/oozie/util/ConfigUtils.java b/core/src/main/java/org/apache/oozie/util/ConfigUtils.java index 792723f..aa19cd3 100644 --- a/core/src/main/java/org/apache/oozie/util/ConfigUtils.java +++ b/core/src/main/java/org/apache/oozie/util/ConfigUtils.java @@ -18,7 +18,10 @@ package org.apache.oozie.util; +import com.google.common.base.Preconditions; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.mapreduce.MRJobConfig; +import org.apache.oozie.client.OozieClient; import org.apache.oozie.service.ConfigurationService; import org.apache.oozie.service.StatusTransitService; import org.apache.oozie.servlet.ServicesLoader; @@ -120,4 +123,62 @@ public class ConfigUtils { public static boolean isBackwardSupportForCoordStatus() { return ConfigurationService.getBoolean(StatusTransitService.CONF_BACKWARD_SUPPORT_FOR_COORD_STATUS); } + + /** + * Check {@link Configuration} whether it contains disallowed properties. Given configuration property values of + * {@code oozie-site.xml#oozie.configuration.check-and-set.DISALLOWED.PROPERTY_KEY}, {@code newValue} and {@code performWrite} + * values, one of the following will happen: + * <ul> + * <li>{@code base} doesn't contain any disallowed property: NOP</li> + * <li>{@code base} contains a disallowed property but its value is the same as {@code newValue}: NOP</li> + * <li>{@code base} contains a conflicting disallowed property and configuration tells we should also set, but + * {@code performWrite=false}: NOP</li> + * <li>{@code base} contains a conflicting disallowed property and configuration tells we should also set, and + * {@code performWrite=true}: {@code base} will be overwritten by {@code key=newValue}</li> + * <li>{@code base} contains a conflicting disallowed property and configuration tells we should not set: + * {@code Exception toThrow} is thrown</li> + * </ul> + * @param base the {@link Configuration} to check + * @param newValue the new value to assign if {@code performWrite=true} and if configuration value + * {@code oozie.configuration.check-and-set.*} is set + * @param toThrow the {@link Exception} to throw when {@code oozie.configuration.check-and-set.*} is unset + * @param performWrite + * @param <E> {@link Exception} type + * @throws E the {@link Exception} to throw when {@code oozie.configuration.check-and-set.*} is unset + */ + public static <E extends Exception> void checkAndSetDisallowedProperties(final Configuration base, + final String newValue, + final E toThrow, + final boolean performWrite) throws E { + Preconditions.checkNotNull(base, "base"); + Preconditions.checkNotNull(base, "newValue"); + Preconditions.checkNotNull(base, "toThrow"); + + for (final String defaultDisallowedProperty : PropertiesUtils.DEFAULT_DISALLOWED_PROPERTIES) { + checkAndSetConfigValue(base, defaultDisallowedProperty, newValue, toThrow, performWrite); + } + } + + private static <E extends Exception> void checkAndSetConfigValue(final Configuration base, + final String key, + final String newValue, + final E toThrow, + final boolean performWrite) throws E { + final boolean shouldCheckAndSet = ConfigurationService.getBoolean("oozie.configuration.check-and-set." + key, false); + final boolean isPresent = base.get(key) != null; + if (isPresent && !base.get(key).equals(newValue)) { + LOG.trace("Base configuration contains config property [{0}={1}] that is different from new value [{2}]", + key, + base.get(key), + newValue); + if (shouldCheckAndSet && performWrite) { + LOG.trace("Setting [{0}] to [{1}]", key, newValue); + base.set(key, newValue); + } + else if (!shouldCheckAndSet) { + LOG.error("Cannot set [{0}] to [{1}]. {2}", key, newValue, toThrow.getMessage()); + throw toThrow; + } + } + } } http://git-wip-us.apache.org/repos/asf/oozie/blob/70f34bdd/core/src/main/java/org/apache/oozie/util/PropertiesUtils.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/oozie/util/PropertiesUtils.java b/core/src/main/java/org/apache/oozie/util/PropertiesUtils.java index 8d32ad0..ec0f687 100644 --- a/core/src/main/java/org/apache/oozie/util/PropertiesUtils.java +++ b/core/src/main/java/org/apache/oozie/util/PropertiesUtils.java @@ -25,8 +25,11 @@ import java.io.IOException; import java.io.StringReader; import java.io.Reader; +import com.google.common.collect.ImmutableSet; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.mapreduce.MRJobConfig; import org.apache.oozie.ErrorCode; +import org.apache.oozie.client.OozieClient; import org.apache.oozie.command.CommandException; public class PropertiesUtils { @@ -51,6 +54,7 @@ public class PropertiesUtils { public static final String REDUCE_IN = "REDUCE_IN"; public static final String REDUCE_OUT = "REDUCE_OUT"; public static final String GROUPS = "GROUPS"; + public static final Set<String> DEFAULT_DISALLOWED_PROPERTIES = ImmutableSet.of(OozieClient.USER_NAME, MRJobConfig.USER_NAME); public static String propertiesToString(Properties props) { ParamChecker.notNull(props, "props"); @@ -113,4 +117,7 @@ public class PropertiesUtils { } } + public static void checkDefaultDisallowedProperties(final Configuration conf) throws CommandException { + checkDisallowedProperties(conf, DEFAULT_DISALLOWED_PROPERTIES); + } } http://git-wip-us.apache.org/repos/asf/oozie/blob/70f34bdd/core/src/test/java/org/apache/oozie/action/hadoop/TestLauncherConfigurationInjector.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/oozie/action/hadoop/TestLauncherConfigurationInjector.java b/core/src/test/java/org/apache/oozie/action/hadoop/TestLauncherConfigurationInjector.java index bcc55bf..9c3721f 100644 --- a/core/src/test/java/org/apache/oozie/action/hadoop/TestLauncherConfigurationInjector.java +++ b/core/src/test/java/org/apache/oozie/action/hadoop/TestLauncherConfigurationInjector.java @@ -19,10 +19,13 @@ package org.apache.oozie.action.hadoop; import org.apache.hadoop.conf.Configuration; +import org.apache.oozie.action.ActionExecutorException; import org.apache.oozie.service.ConfigurationService; import org.apache.oozie.service.Services; import org.apache.oozie.test.XTestCase; +import static org.apache.oozie.action.hadoop.JavaActionExecutor.HADOOP_USER; + public class TestLauncherConfigurationInjector extends XTestCase { @Override @@ -39,7 +42,7 @@ public class TestLauncherConfigurationInjector extends XTestCase { super.tearDown(); } - public void testOverrideSwitchedOffSourceCopiedToTargetWithTwoDifferentKeys() { + public void testOverrideSwitchedOffSourceCopiedToTargetWithTwoDifferentKeys() throws ActionExecutorException { ConfigurationService.setBoolean("oozie.launcher.override", false); final Configuration sourceConf = SourceConfigurationFactory.createOverridingAndLauncherEntries(); @@ -78,7 +81,7 @@ public class TestLauncherConfigurationInjector extends XTestCase { assertTrue("queue", launcherConf.get("queue").contains("default2")); } - public void testLauncherConfigSourceCopiedToTarget() { + public void testLauncherConfigSourceCopiedToTarget() throws ActionExecutorException { final Configuration sourceConf = SourceConfigurationFactory.createLauncherEntries(); final Configuration launcherConf = newConfigurationWithoutDefaults(); @@ -87,7 +90,7 @@ public class TestLauncherConfigurationInjector extends XTestCase { assertLauncherAndDefaultEntries(launcherConf); } - public void testOverridingConfigCopiedToTarget() { + public void testOverridingConfigCopiedToTarget() throws ActionExecutorException { final Configuration sourceConf = SourceConfigurationFactory.createOverridingEntries(); final Configuration launcherConf = newConfigurationWithoutDefaults(); @@ -118,7 +121,7 @@ public class TestLauncherConfigurationInjector extends XTestCase { assertEquals("modify ACL", "modify", launcherConf.get(JavaActionExecutor.LAUNCER_MODIFY_ACL)); } - public void testMultipleOverrideOrder() { + public void testMultipleOverrideOrder() throws ActionExecutorException { final Configuration sourceConf = SourceConfigurationFactory.createMultipleOverridingEntries(); final Configuration launcherConf = newConfigurationWithoutDefaults(); @@ -127,7 +130,7 @@ public class TestLauncherConfigurationInjector extends XTestCase { assertHigherRankingOverridingAndNoDefaultEntries(launcherConf); } - public void testPrependLauncherConfigSourcePrependedToTarget() { + public void testPrependLauncherConfigSourcePrependedToTarget() throws ActionExecutorException { final Configuration sourceConf = SourceConfigurationFactory.createPrependingAndLauncherEntries(); final Configuration launcherConf = newConfigurationWithoutDefaults(); @@ -147,6 +150,21 @@ public class TestLauncherConfigurationInjector extends XTestCase { assertFalse("env", launcherConf.get("env").contains("ENV=env:$ENV")); } + public void testLauncherConfigurationFiltering() { + final Configuration sourceConf = SourceConfigurationFactory.createLauncherEntries(); + sourceConf.set("oozie.launcher." + HADOOP_USER, "should-not-be-present"); + final Configuration launcherConf = newConfigurationWithoutDefaults(); + + try { + new LauncherConfigurationInjector(sourceConf).inject(launcherConf); + fail(String.format("configuration entry %s should be filtered out", HADOOP_USER)); + } + catch (final ActionExecutorException e) { + assertEquals("error code mismatch", "JA010", e.getErrorCode()); + } + + } + private static class SourceConfigurationFactory { private static Configuration createOverridingAndLauncherEntries() { http://git-wip-us.apache.org/repos/asf/oozie/blob/70f34bdd/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionInputCheckXCommand.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionInputCheckXCommand.java b/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionInputCheckXCommand.java index d1d2689..4ead0b6 100644 --- a/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionInputCheckXCommand.java +++ b/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionInputCheckXCommand.java @@ -646,7 +646,7 @@ public class TestCoordActionInputCheckXCommand extends XDataTestCase { coordJob.setStatus(CoordinatorJob.Status.RUNNING); coordJob.setCreatedTime(new Date()); coordJob.setLastModifiedTime(new Date()); - coordJob.setUser("testUser"); + coordJob.setUser("test"); coordJob.setGroup("testGroup"); coordJob.setTimeZone("UTC"); coordJob.setTimeUnit(Timeunit.DAY); @@ -962,7 +962,7 @@ public class TestCoordActionInputCheckXCommand extends XDataTestCase { coordJob.setStatus(CoordinatorJob.Status.RUNNING); coordJob.setCreatedTime(new Date()); coordJob.setLastModifiedTime(new Date()); - coordJob.setUser("testUser"); + coordJob.setUser("test"); coordJob.setGroup("testGroup"); coordJob.setTimeZone("UTC"); coordJob.setTimeUnit(Timeunit.DAY); http://git-wip-us.apache.org/repos/asf/oozie/blob/70f34bdd/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionStartXCommand.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionStartXCommand.java b/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionStartXCommand.java index 261c496..4215a32 100644 --- a/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionStartXCommand.java +++ b/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionStartXCommand.java @@ -87,7 +87,7 @@ public class TestCoordActionStartXCommand extends XDataTestCase { public void testActionStartCommand() throws IOException, JPAExecutorException, CommandException { String actionId = new Date().getTime() + "-COORD-ActionStartCommand-C@1"; addRecordToActionTable(actionId, 1, null); - new CoordActionStartXCommand(actionId, "me", "myapp", "myjob").call(); + new CoordActionStartXCommand(actionId, "test", "myapp", "myjob").call(); checkCoordAction(actionId); } @@ -104,7 +104,7 @@ public class TestCoordActionStartXCommand extends XDataTestCase { String actionId = new Date().getTime() + "-COORD-ActionStartCommand-C@1"; String wfApp = "<start to='${someParam}' />"; addRecordToActionTable(actionId, 1, wfApp); - new CoordActionStartXCommand(actionId, "me", "myapp", "myjob").call(); + new CoordActionStartXCommand(actionId, "test", "myapp", "myjob").call(); final JPAService jpaService = Services.get().get(JPAService.class); CoordinatorActionBean action = jpaService.execute(new CoordActionGetForStartJPAExecutor(actionId)); if (action.getStatus() == CoordinatorAction.Status.SUBMITTED) { http://git-wip-us.apache.org/repos/asf/oozie/blob/70f34bdd/core/src/test/java/org/apache/oozie/util/TestConfigUtils.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/oozie/util/TestConfigUtils.java b/core/src/test/java/org/apache/oozie/util/TestConfigUtils.java index 6904881..6b07e72 100644 --- a/core/src/test/java/org/apache/oozie/util/TestConfigUtils.java +++ b/core/src/test/java/org/apache/oozie/util/TestConfigUtils.java @@ -18,6 +18,12 @@ package org.apache.oozie.util; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.mapreduce.MRJobConfig; +import org.apache.oozie.ErrorCode; +import org.apache.oozie.client.OozieClient; +import org.apache.oozie.command.CommandException; +import org.apache.oozie.service.ConfigurationService; import org.apache.oozie.service.Services; import org.apache.oozie.test.XTestCase; @@ -46,4 +52,104 @@ public class TestConfigUtils extends XTestCase { assertEquals("http://localhost:11000/oozie", ConfigUtils.getOozieURL(false)); assertEquals("https://localhost:11443/oozie", ConfigUtils.getOozieURL(true)); } + + public void testCheckAndSetNonConflictingUserNamesNoChange() throws Exception { + checkAndSetNonConflictingNoChange(OozieClient.USER_NAME); + checkAndSetNonConflictingNoChange(MRJobConfig.USER_NAME); + } + + protected void checkAndSetNonConflictingNoChange(final String key) throws Exception { + final Configuration base = new Configuration(); + base.set(key, getTestUser()); + + ConfigurationService.setBoolean("oozie.configuration.check-and-set." + key, false); + + ConfigUtils.checkAndSetDisallowedProperties(base, getTestUser(), new Exception(), false); + } + + public void testCheckAndSetConflictingUserNameSets() throws Exception { + checkAndSetConflictingSets(OozieClient.USER_NAME); + checkAndSetConflictingSets(MRJobConfig.USER_NAME); + } + + protected void checkAndSetConflictingSets(final String key) throws Exception { + final Configuration base = new Configuration(); + base.set(key, getTestUser()); + + ConfigurationService.setBoolean("oozie.configuration.check-and-set." + OozieClient.USER_NAME, true); + ConfigurationService.setBoolean("oozie.configuration.check-and-set." + MRJobConfig.USER_NAME, true); + + ConfigUtils.checkAndSetDisallowedProperties(base, getTestUser2(), new Exception(), false); + + assertEquals("user.name should be preserved as no write will be performed", + getTestUser(), + base.get(key)); + + ConfigUtils.checkAndSetDisallowedProperties(base, getTestUser2(), new Exception(), true); + + assertEquals("user.name should be set as one write operation will be performed", + getTestUser2(), + base.get(key)); + } + + public void testCheckAndSetConflictingUserNameThrows() { + checkAndSetConflictingThrows(OozieClient.USER_NAME); + checkAndSetConflictingThrows(MRJobConfig.USER_NAME); + } + + protected void checkAndSetConflictingThrows(final String key) { + final Configuration base = new Configuration(); + base.set(key, getTestUser()); + + ConfigurationService.setBoolean("oozie.configuration.check-and-set." + key, false); + + try { + ConfigUtils.checkAndSetDisallowedProperties(base, + getTestUser2(), + new CommandException(ErrorCode.E1303, "test error", "test attribute"), false); + fail("CommandException should have been thrown"); + } + catch (final CommandException e) { + assertEquals("ErrorCode mismatch", ErrorCode.E1303, e.getErrorCode()); + assertEquals("message mismatch", + "E1303: Invalid bundle application attributes [test error], test attribute", + e.getMessage()); + } + } + + public void testCheckAndSetConflictingUserNamesSetsAndThrows() { + final Configuration base = new Configuration(); + base.set(OozieClient.USER_NAME, getTestUser()); + base.set(MRJobConfig.USER_NAME, getTestUser()); + + ConfigurationService.setBoolean("oozie.configuration.check-and-set." + OozieClient.USER_NAME, true); + ConfigurationService.setBoolean("oozie.configuration.check-and-set." + MRJobConfig.USER_NAME, false); + + try { + ConfigUtils.checkAndSetDisallowedProperties(base, getTestUser2(), new Exception("test message"), false); + fail("Exception should have been thrown"); + } + catch (final Exception e) { + assertTrue("message mismatch", e.getMessage().contains("test message")); + } + + assertEquals("user.name should be preserved as no write will be performed", + getTestUser(), + base.get(OozieClient.USER_NAME)); + + try { + ConfigUtils.checkAndSetDisallowedProperties(base, getTestUser2(), new Exception("test message"), true); + } + catch (final Exception e) { + fail("Exception should not have been thrown"); + } + + assertEquals("user.name should be set as one write operation will be performed", + getTestUser2(), + base.get(OozieClient.USER_NAME)); + + assertEquals("mapreduce.job.user.name should be set implicitly by Configuration#set(user.name)", + getTestUser2(), + base.get(MRJobConfig.USER_NAME)); + } } http://git-wip-us.apache.org/repos/asf/oozie/blob/70f34bdd/release-log.txt ---------------------------------------------------------------------- diff --git a/release-log.txt b/release-log.txt index 3fdbc84..b8e781c 100644 --- a/release-log.txt +++ b/release-log.txt @@ -7,6 +7,7 @@ OOZIE-3277 [build] Check for star imports (kmarton via andras.piros) -- Oozie 5.1.0 release +OOZIE-3370 Property filtering is not consistent across job submission (andras.piros) OOZIE-3369 [core] Upgrade guru.nidi:graphviz-java to 0.7.0 (andras.piros) OOZIE-3358 [docs] Check and fix differences between help and command line documentation for Fluent Job API (kmarton via andras.piros) OOZIE-3348 amend [Hive action] Remove dependency hive-contrib (kmarton via andras.piros)