Skip to content

Commit

Permalink
[JENKINS-50437] Filter out environment variables coming from computer
Browse files Browse the repository at this point in the history
but keep everything else.

notation KEY+FOO=bar seems to be prepended in my setup and in CI so
inverting the order in the assertion
  • Loading branch information
Vlatombe committed Mar 28, 2018
1 parent 1ad50c7 commit ccf4634
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
import hudson.Launcher;
import hudson.LauncherDecorator;
import hudson.Proc;
import hudson.model.Computer;
import hudson.model.Node;
import io.fabric8.kubernetes.api.model.ContainerStatus;
import io.fabric8.kubernetes.api.model.Pod;
Expand Down Expand Up @@ -212,28 +213,28 @@ public Launcher decorate(final Launcher launcher, final Node node) {
@Override
public Proc launch(ProcStarter starter) throws IOException {
LOGGER.log(Level.FINEST, "Launch proc with environment: {0}", Arrays.toString(starter.envs()));
boolean quiet = starter.quiet();
FilePath pwd = starter.pwd();
List<String> procStarter = Arrays.asList(starter.envs());
List<String> cmdEnvs = new ArrayList<String>();
// One issue that cropped up was that when executing sh commands, we would get the jnlp agent's injected
// environment variables as well, causing obvious problems such as JAVA_HOME being overwritten. The
// unsatisfying answer was to check for the presence of JENKINS_HOME in the cmdenvs and skip if present.
// check if the cmd is sourced from Jenkins, rather than another plugin;
// Currently, build level properties will be provided by the Run Context anyways.
boolean javaHome_detected = false;
for (String env : procStarter) {
if (env.equalsIgnoreCase("JAVA_HOME")) {
LOGGER.log(Level.FINEST, "Detected JAVA_HOME in {0}", env);
javaHome_detected = true;
break;
String[] envVars = starter.envs();
if (node != null) { // It seems this is possible despite the method javadoc saying it is non-null
final Computer computer = node.toComputer();
if (computer != null) {
List<String> resultEnvVar = new ArrayList<>();
try {
EnvVars environment = computer.getEnvironment();
String[] envs = starter.envs();
for (String keyValue : envs) {
String[] split = keyValue.split("=", 2);
if (!split[1].equals(environment.get(split[0]))) {
// Only keep environment variables that differ from Computer's environment
resultEnvVar.add(keyValue);
}
}
envVars = resultEnvVar.toArray(new String[resultEnvVar.size()]);
} catch (InterruptedException e) {
throw new IOException("Unable to retrieve environment variables", e);
}
}
}
if (!javaHome_detected) {
cmdEnvs = procStarter;
}
String[] commands = getCommands(starter);
return doLaunch(quiet, cmdEnvs.toArray(new String[cmdEnvs.size()]), starter.stdout(), pwd, commands);
return doLaunch(starter.quiet(), envVars, starter.stdout(), starter.pwd(), getCommands(starter));
}

private Proc doLaunch(boolean quiet, String [] cmdEnvs, OutputStream outputForCaller, FilePath pwd, String... commands) throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ public void runWithEnvVariablesInContext() throws Exception {
r.assertLogContains("The value of AFTER_ESCAPED_QUOTE is AFTER_ESCAPED_QUOTE\\\"", b);
r.assertLogContains("The value of SINGLE_QUOTE is BEFORE'AFTER", b);
r.assertLogContains("The value of WITH_NEWLINE is before newline\nafter newline", b);
r.assertLogContains("The value of POD_ENV_VAR is pod-env-var-value:/bin/mvn", b);
r.assertLogContains("The value of POD_ENV_VAR is /bin/mvn:pod-env-var-value", b);
r.assertLogContains("The value of WILL.NOT is ", b);
}

Expand Down

1 comment on commit ccf4634

@MattLud
Copy link
Contributor

@MattLud MattLud commented on ccf4634 Mar 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for contributing this! It was not easy figuring out the jenkins internal parts to how it works with computer envVars

Please sign in to comment.