diff --git a/pom.xml b/pom.xml index 3d5a1367..51bb7f4a 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.jenkins-ci.plugins plugin - 2.28 + 2.35 envinject @@ -100,6 +100,21 @@ true + + + org.jenkins-ci.plugins + ant + 1.4 + test + + + + org.jenkins-ci + annotation-indexer + + + + org.mockito mockito-core diff --git a/src/main/java/org/jenkinsci/plugins/envinject/EnvInjectListener.java b/src/main/java/org/jenkinsci/plugins/envinject/EnvInjectListener.java index 6ca864f3..502c154f 100644 --- a/src/main/java/org/jenkinsci/plugins/envinject/EnvInjectListener.java +++ b/src/main/java/org/jenkinsci/plugins/envinject/EnvInjectListener.java @@ -10,6 +10,7 @@ import hudson.model.listeners.RunListener; import hudson.tasks.BuildWrapper; import hudson.tasks.BuildWrapperDescriptor; + import org.jenkinsci.lib.envinject.EnvInjectException; import org.jenkinsci.lib.envinject.EnvInjectLogger; import org.jenkinsci.plugins.envinject.model.EnvInjectJobPropertyContributor; @@ -49,7 +50,6 @@ public Environment setUpEnvironment(@Nonnull AbstractBuild build, @Nonnull Launc } else { return setUpEnvironmentWithoutJobPropertyObject(build, launcher, listener); } - } catch (Run.RunnerAbortedException rre) { logger.info("Fail the build."); throw new Run.RunnerAbortedException(); @@ -88,8 +88,7 @@ private void loadEnvironmentVariablesNode(@Nonnull Run build, @Nonnull Nod throw new EnvInjectException(ie); } } - - + private boolean isEnvInjectJobPropertyActive(@Nonnull Run run) { EnvInjectJobProperty envInjectJobProperty = RunHelper.getEnvInjectJobProperty(run); return envInjectJobProperty != null; diff --git a/src/main/java/org/jenkinsci/plugins/envinject/EnvInjectPluginAction.java b/src/main/java/org/jenkinsci/plugins/envinject/EnvInjectPluginAction.java index 9d623c5d..3315a714 100644 --- a/src/main/java/org/jenkinsci/plugins/envinject/EnvInjectPluginAction.java +++ b/src/main/java/org/jenkinsci/plugins/envinject/EnvInjectPluginAction.java @@ -4,20 +4,45 @@ import hudson.EnvVars; import hudson.model.AbstractBuild; import hudson.model.EnvironmentContributingAction; + +import java.io.IOException; import java.util.Collections; + +import hudson.model.ParametersAction; +import hudson.model.Run; import org.jenkinsci.lib.envinject.EnvInjectAction; +import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Set; +import java.util.logging.Level; +import java.util.logging.Logger; import javax.annotation.CheckForNull; import javax.annotation.Nonnull; +import javax.annotation.concurrent.GuardedBy; + import jenkins.model.RunAction2; +import org.jenkinsci.plugins.envinject.util.RunHelper; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.NoExternalUse; /** * @author Gregory Boissinot */ public class EnvInjectPluginAction extends EnvInjectAction implements EnvironmentContributingAction { + private static final Logger LOGGER = Logger.getLogger(EnvInjectPluginAction.class.getName()); + + /** + * Cache of resolved parameters, which is stored within this action. + * This cache assumes that the parameters never change after the creation of the action. + * It is technically possible via API, but there is no realistic use-case for that. + * Famous last words(c) + */ + @GuardedBy("this") + private transient EnvVars resolvedParameterEnvVars = null; + /** * Constructor. * @deprecated This is a {@link RunAction2} instance, not need to pass build explicitly. @@ -76,11 +101,92 @@ public String transformEntry(String key, String value) { })); } + @CheckForNull + private synchronized EnvVars getParameterEnvVars() { + final Run run = getOwner(); + if (resolvedParameterEnvVars == null && run instanceof AbstractBuild) { + AbstractBuild build = (AbstractBuild)run; + EnvVars resolvedParameters = new EnvVars(); + + List actions = build.getActions(ParametersAction.class); + for (ParametersAction params : actions) { + params.buildEnvVars(build, resolvedParameters); + } + resolvedParameterEnvVars = resolvedParameters; + } + return resolvedParameterEnvVars; + } + + // The method is synchronized, because it modifies the internal cache @Override - public void buildEnvVars(@Nonnull AbstractBuild build, @Nonnull EnvVars env) { + public synchronized void buildEnvVars(@Nonnull AbstractBuild build, @Nonnull EnvVars env) { + assert build == getOwner() : "Trying to resolve environment for build, which is not an owner of this action"; + final Map currentEnvMap = getEnvMap(); - if (currentEnvMap != null) { - env.putAll(currentEnvMap); + if (currentEnvMap == null) { + return; // Nothing to inject + } + + // Other extension points may contribute other variable values + // before contributing actions is invoked. See AbstractBuild#getEnvironment() + // We take the externally updated variables as a source of truth and just override the missing ones + Map overrides = null; + for (Map.Entry storedVar : currentEnvMap.entrySet()) { + final String varName = storedVar.getKey(); + final String storedValue = storedVar.getValue(); + final String envValue = env.get(varName); + if (envValue == null) { + LOGGER.log(Level.CONFIG, "Build {0}: Variable {1} is missing, overriding it by value stored in the action", + new Object[] {build, varName}); + env.put(varName, storedValue); + } else if (!envValue.equals(storedValue)) { + // If the value is defined by the Parameters, we actually override them + // See org.jenkinsci.plugins.envinject.EnvInjectJobPropertyTest#shouldOverrideBuildParametersIfEnabled() + final EnvVars parameterEnvVars = getParameterEnvVars(); + boolean usedExternalValue = true; + if (parameterEnvVars != null) { + String parameterValue = parameterEnvVars.get(varName); + if (envValue.equals(parameterValue)) { // defined by parameter and not already overridden + final EnvInjectJobProperty prop = RunHelper.getEnvInjectJobProperty(build); + if (prop != null && prop.isOverrideBuildParameters()) { + LOGGER.log(Level.CONFIG, "Build {0}: Overriding value of {1} defined by the parameter value", + new Object[] {build, varName}); + env.put(varName, storedValue); + usedExternalValue = false; + } else { + LOGGER.log(Level.CONFIG, "Build {0}: Build variable {1} will not be overridden, overriding value stored in the action", + new Object[] {build, varName}); + if (overrides == null) { + overrides = new HashMap<>(); + } + overrides.put(varName, envValue); + } + } + } + + if (usedExternalValue) { // The value was overridden, let's update the cache + LOGGER.log(Level.CONFIG, "Build {0}: Variable {1} is defined externally, overriding value stored in the action", + new Object[] {build, varName}); + if (overrides == null) { + overrides = new HashMap<>(); + } + overrides.put(varName, envValue); + } + } + } + + if (overrides != null) { + LOGGER.log(Level.FINER, "Build {0}: Overriding {1} variables, which have been changed since the previous run", + new Object[] {build, overrides.size()}); + overrideAll(RunHelper.getSensitiveBuildVariables(build), overrides); + // TODO: We do not save the action at this point, + // it should be persisted by the AbstractBuild later when the build completes + // Should we? + // try { + // getOwner().save(); + // } catch (IOException ex) { + // LOGGER.log(Level.WARNING, "Failed to persist EnvInject variable overrides", ex); + // } } } } diff --git a/src/test/java/org/jenkinsci/plugins/envinject/BuildCauseRetrieverTest.java b/src/test/java/org/jenkinsci/plugins/envinject/BuildCauseRetrieverTest.java index e74434f9..d9aa53ef 100644 --- a/src/test/java/org/jenkinsci/plugins/envinject/BuildCauseRetrieverTest.java +++ b/src/test/java/org/jenkinsci/plugins/envinject/BuildCauseRetrieverTest.java @@ -5,6 +5,7 @@ import hudson.model.FreeStyleBuild; import hudson.model.FreeStyleProject; import hudson.model.Run; + import hudson.triggers.SCMTrigger; import hudson.triggers.TimerTrigger; import org.junit.ClassRule; diff --git a/src/test/java/org/jenkinsci/plugins/envinject/EnvInjectActionTest.java b/src/test/java/org/jenkinsci/plugins/envinject/EnvInjectActionTest.java new file mode 100644 index 00000000..ef1617e1 --- /dev/null +++ b/src/test/java/org/jenkinsci/plugins/envinject/EnvInjectActionTest.java @@ -0,0 +1,219 @@ +/* + * The MIT License + * + * Copyright (c) 2015 Red Hat, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ +package org.jenkinsci.plugins.envinject; + +import static org.junit.Assert.assertEquals; +import hudson.EnvVars; +import hudson.Extension; +import hudson.Launcher; +import hudson.model.BuildListener; +import hudson.model.InvisibleAction; +import hudson.model.TaskListener; +import hudson.model.AbstractBuild; +import hudson.model.EnvironmentContributor; +import hudson.model.FreeStyleProject; +import hudson.model.Run; +import hudson.slaves.DumbSlave; +import hudson.tasks.BuildWrapper; + +import java.io.IOException; +import java.util.Map; + +import jenkins.model.RunAction2; +import org.junit.Before; +import org.junit.Ignore; +import org.junit.Rule; +import org.junit.Test; +import org.jvnet.hudson.test.CaptureEnvironmentBuilder; +import org.jvnet.hudson.test.Issue; +import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.TestBuilder; +import org.jvnet.hudson.test.TestExtension; + +public class EnvInjectActionTest { + + @Rule + public JenkinsRule j = new JenkinsRule(); + + @Test + @Issue("JENKINS-26583") + public void doNotOverrideWrapperEnvVar() throws Exception { + FreeStyleProject p = setupProjectWithDefaultEnvValue(); + + p.getBuildWrappersList().add(new ContributingWrapper("DISPLAY", "BUILD_VAL")); + + assertValueInjected(p); + } + + //TODO: Fails, create a follow-up issue for that + @Test + @Ignore + public void doNotOverrideContributorEnvVar() throws Exception { + FreeStyleProject p = setupProjectWithDefaultEnvValue(); + + p.getBuildersList().add(new ContributingBuilder("DISPLAY", "BUILD_VAL")); + + assertValueInjected(p); + } + + //TODO: Fails, create a follow-up issue for that + @Test + @Ignore + public void doNotOverrideWithBuildStep() throws Exception { + FreeStyleProject p = setupProjectWithDefaultEnvValue(); + p.getBuildersList().add(new EnvInjectBuilder(null, "IRRELEVANT_VAR=true")); + + p.getBuildWrappersList().add(new ContributingWrapper("DISPLAY", "BUILD_VAL")); + + assertValueInjected(p); + } + + @Test + public void doNotOverrideWithBuildWrapper() throws Exception { + FreeStyleProject p = setupProjectWithDefaultEnvValue(); + final EnvInjectBuildWrapper wrapper = new EnvInjectBuildWrapper(); + p.getBuildWrappersList().add(wrapper); + wrapper.setInfo(new EnvInjectJobPropertyInfo( + null, "IRRELEVANT_VAR=true", null, null, null, false)); + + p.getBuildWrappersList().add(new ContributingWrapper("DISPLAY", "BUILD_VAL")); + + assertValueInjected(p); + } + + @Test + public void doNotOverrideWithPasswordWrapper() throws Exception { + FreeStyleProject p = setupProjectWithDefaultEnvValue(); + final EnvInjectPasswordWrapper wrapper = new EnvInjectPasswordWrapper(); + wrapper.setPasswordEntries(new EnvInjectPasswordEntry[] { + new EnvInjectPasswordEntry("IRRELEVANT", "value") + }); + p.getBuildWrappersList().add(wrapper); + + p.getBuildWrappersList().add(new ContributingWrapper("DISPLAY", "BUILD_VAL")); + + assertValueInjected(p); + } + + private void assertValueInjected(FreeStyleProject p) throws Exception { + CaptureEnvironmentBuilder capture = new CaptureEnvironmentBuilder(); + p.getBuildersList().add(capture); + + p.scheduleBuild2(0).get(); + assertEquals("BUILD_VAL", capture.getEnvVars().get("DISPLAY")); + } + + private FreeStyleProject setupProjectWithDefaultEnvValue()throws Exception, IOException { + DumbSlave slave = slaveContributing("DISPLAY", "SLAVE_VAL"); + FreeStyleProject p = j.jenkins.createProject(FreeStyleProject.class, "project"); + p.setAssignedNode(slave); + return p; + } + + private DumbSlave slaveContributing(String key, String value) throws Exception { + return j.createOnlineSlave(null, new EnvVars(key, value)); + } + + private static final class ContributingWrapper extends BuildWrapper { + private final String value; + private final String key; + + private ContributingWrapper(String key, String value) { + this.value = value; + this.key = key; + } + + @Override + public Environment setUp( + AbstractBuild build, Launcher launcher, BuildListener listener + ) throws IOException, InterruptedException { + return new Environment() { + @Override + public void buildEnvVars(Map env) { + env.put(key, value); + } + }; + } + + @Extension + public static class Descriptor extends hudson.model.Descriptor { + @Override + public String getDisplayName() { + return null; + } + } + } + + private static final class ContributingBuilder extends TestBuilder { + private final String value; + private final String key; + + private ContributingBuilder(String key, String value) { + this.value = value; + this.key = key; + } + + @Override + public boolean perform( + AbstractBuild build, Launcher launcher, BuildListener listener + ) throws InterruptedException, IOException { + // Start serving envvar from EnvironmentContributor + build.addAction(new ContributorAction(key, value)); + return true; + } + } + + public static class ContributorAction extends InvisibleAction implements RunAction2 { + private String value = null; + private String key = null; + + public ContributorAction(String k, String v) { + value = v; + key = k; + } + + @Override + public void onAttached(Run r) { + // Do not care + } + + @Override + public void onLoad(Run r) { + // Do not care + } + } + + @TestExtension + public static class Contributor extends EnvironmentContributor { + + @SuppressWarnings("rawtypes") + @Override + public void buildEnvironmentFor(Run r, EnvVars envs, TaskListener listener) throws IOException, InterruptedException { + ContributorAction a = r.getAction(ContributorAction.class); + if (a != null) { + envs.put(a.key, a.value); + } + } + } +} diff --git a/src/test/java/org/jenkinsci/plugins/envinject/EnvInjectJobPropertyTest.java b/src/test/java/org/jenkinsci/plugins/envinject/EnvInjectJobPropertyTest.java index 836fb7e9..c28d33b1 100644 --- a/src/test/java/org/jenkinsci/plugins/envinject/EnvInjectJobPropertyTest.java +++ b/src/test/java/org/jenkinsci/plugins/envinject/EnvInjectJobPropertyTest.java @@ -1,5 +1,6 @@ package org.jenkinsci.plugins.envinject; +import hudson.EnvVars; import hudson.model.Cause; import hudson.model.FreeStyleBuild; import hudson.model.FreeStyleProject; @@ -104,6 +105,13 @@ public void shouldNotOverrideBuildParametersByDefault() throws Exception { jenkinsRule.assertBuildStatusSuccess(build); assertEquals("The variable has been overridden in the environment", "ValueFromParameter", envCapture.getEnvVars().get("PARAM")); assertEquals("The variable has been overridden in the API", "ValueFromParameter", build.getEnvironment(TaskListener.NULL).get("PARAM")); + + // Ensure that Parameters action contains the correct value + EnvInjectPluginAction a = build.getAction(EnvInjectPluginAction.class); + assertNotNull("EnvInjectPluginAction has not been added to the build", a); + EnvVars vars = new EnvVars(); + a.buildEnvVars(build, vars); + assertEquals("The variable has been overridden in the stored action", "ValueFromParameter", vars.get("PARAM")); } @Test @@ -118,6 +126,15 @@ public void shouldOverrideBuildParametersIfEnabled() throws Exception { FreeStyleBuild build = scheduled.get(); jenkinsRule.assertBuildStatusSuccess(build); assertEquals("The build parameter value has not been overridden", "Overridden", build.getEnvironment(TaskListener.NULL).get("PARAM")); + + // Ensure that Parameters action contains the correct value + EnvInjectPluginAction a = build.getAction(EnvInjectPluginAction.class); + assertNotNull("EnvInjectPluginAction has not been added to the build", a); + EnvVars vars = new EnvVars(); + a.buildEnvVars(build, vars); + assertEquals("The build parameter value has not been overridden in EnvInjectPluginAction", + "Overridden", vars.get("PARAM")); + } @Test