JENKINS-47376: Fix global env variables

src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerExecDecorator.java

@@ -74,7 +74,9 @@ public class ContainerExecDecorator extends LauncherDecorator implements Seriali
     private static final String CONTAINER_READY_TIMEOUT_SYSTEM_PROPERTY = ContainerExecDecorator.class.getName() + ".containerReadyTimeout";
     private static final long CONTAINER_READY_TIMEOUT = containerReadyTimeout();
     private static final String COOKIE_VAR = "JENKINS_SERVER_COOKIE";
-    private static final String JENKINS_HOME = "JENKINS_HOME=";
+    private static final String[] BUILT_IN_ENV_VARS = new String[] { "BUILD_NUMBER", "BUILD_ID", "BUILD_URL",
+            "NODE_NAME", "JOB_NAME", "JENKINS_URL", "BUILD_TAG", "GIT_COMMIT", "GIT_URL", "GIT_BRANCH" };
+
     private static final Logger LOGGER = Logger.getLogger(ContainerExecDecorator.class.getName());
 
     private transient KubernetesClient client;
@@ -85,13 +87,17 @@ public class ContainerExecDecorator extends LauncherDecorator implements Seriali
     @SuppressFBWarnings(value = "SE_TRANSIENT_FIELD_NOT_RESTORED", justification = "not needed on deserialization")
     private transient Map<Integer, ContainerExecProc> processes = new HashMap<Integer, ContainerExecProc>();
 
-    private final String podName;
-    private final String namespace;
-    private final String containerName;
-    private final EnvironmentExpander environmentExpander;
+    private String podName;
+    private String namespace;
+    private String containerName;
+    private EnvironmentExpander environmentExpander;
+    private EnvVars globalVars;
+    private FilePath ws;
 
-    private final FilePath ws;
+    public ContainerExecDecorator() {
+    }
 
+    @Deprecated
     public ContainerExecDecorator(KubernetesClient client, String podName, String containerName, String namespace, EnvironmentExpander environmentExpander, FilePath ws) {
         this.client = client;
         this.podName = podName;
@@ -126,26 +132,100 @@ public ContainerExecDecorator(KubernetesClient client, String podName, String co
         this(client, podName, containerName, (String) null, null, null);
     }
 
+    public KubernetesClient getClient() {
+        return client;
+    }
+
+    public void setClient(KubernetesClient client) {
+        this.client = client;
+    }
+
+    public String getPodName() {
+        return podName;
+    }
+
+    public void setPodName(String podName) {
+        this.podName = podName;
+    }
+
+    public String getNamespace() {
+        return namespace;
+    }
+
+    public void setNamespace(String namespace) {
+        this.namespace = namespace;
+    }
+
+    public String getContainerName() {
+        return containerName;
+    }
+
+    public void setContainerName(String containerName) {
+        this.containerName = containerName;
+    }
+
+    public EnvironmentExpander getEnvironmentExpander() {
+        return environmentExpander;
+    }
+
+    public void setEnvironmentExpander(EnvironmentExpander environmentExpander) {
+        this.environmentExpander = environmentExpander;
+    }
+
+    public EnvVars getGlobalVars() {
+        return globalVars;
+    }
+
+    public void setGlobalVars(EnvVars globalVars) {
+        this.globalVars = globalVars;
+    }
+
+    public FilePath getWs() {
+        return ws;
+    }
+
+    public void setWs(FilePath ws) {
+        this.ws = ws;
+    }
+
     @Override
     public Launcher decorate(final Launcher launcher, final Node node) {
         return new Launcher.DecoratedLauncher(launcher) {
             @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();
 
-                String [] cmdEnvs = starter.envs();
 
-                //check if the cmd is sourced from Jenkins, rather than another plugin; if so, skip cmdEnvs as we are getting other environment variables
-                for (String cmd : cmdEnvs) {
-                    if (cmd.startsWith(JENKINS_HOME)) {
-                        cmdEnvs = new String[0];
-                        LOGGER.info("Skipping injection of procstarter cmdenvs due to JENKINS_HOME present");
+                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; if so, skip cmdEnvs except for
+                // built-in ones.
+                boolean javaHome_detected = false;
+                for (String env : procStarter) {
+                    if (env.contains("JAVA_HOME")) {
+                        LOGGER.log(Level.FINEST, "Detected JAVA_HOME in {0}", env);
+                        javaHome_detected = true;
                         break;
                     }
+                    for (String builtEnvVar : BUILT_IN_ENV_VARS) {
+                        if (env.contains(builtEnvVar)) {
+                            LOGGER.log(Level.FINEST, "Found built-in env var {0} in {1}",
+                                    new String[] { builtEnvVar, env });
+                            cmdEnvs.add(env);
+                        }
+                    }
                 }
-                String [] commands = getCommands(starter);
-                return doLaunch(quiet, cmdEnvs, starter.stdout(), pwd,  commands);
+                if (!javaHome_detected) {
+                    cmdEnvs = procStarter;
+                }
+                String[] commands = getCommands(starter);
+                return doLaunch(quiet, cmdEnvs.toArray(new String[cmdEnvs.size()]), starter.stdout(), pwd, commands);
             }
 
             private Proc doLaunch(boolean quiet, String [] cmdEnvs,  OutputStream outputForCaller, FilePath pwd, String... commands) throws IOException {
@@ -314,6 +394,10 @@ public void onClose(int i, String s) {
                                 String.format("cd \"%s\"%s", pwd, NEWLINE).getBytes(StandardCharsets.UTF_8));
 
                     }
+                    //get global vars here, run the export first as they'll get overwritten.
+                    if (globalVars != null) {
+                            this.setupEnvironmentVariable(globalVars, watch);
+                    }
 
                     EnvVars envVars = new EnvVars();
                     if (environmentExpander != null) {
@@ -322,6 +406,7 @@ public void onClose(int i, String s) {
 
                     //setup specific command envs passed into cmd
                     if (cmdEnvs != null) {
+                        LOGGER.log(Level.FINEST, "Launching with env vars: {0}", Arrays.toString(cmdEnvs));
                         for (String cmdEnv : cmdEnvs) {
                             envVars.addLine(cmdEnv);
                         }

src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerStepExecution.java

@@ -1,24 +1,30 @@
 package org.csanchez.jenkins.plugins.kubernetes.pipeline;
 
+import static org.csanchez.jenkins.plugins.kubernetes.pipeline.Resources.*;
+
 import java.io.Closeable;
+import java.util.List;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
-import hudson.FilePath;
-import org.jenkinsci.plugins.workflow.steps.AbstractStepExecutionImpl;
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+import javax.annotation.Nonnull;
+
 import org.jenkinsci.plugins.workflow.steps.BodyExecutionCallback;
 import org.jenkinsci.plugins.workflow.steps.BodyInvoker;
 import org.jenkinsci.plugins.workflow.steps.EnvironmentExpander;
 import org.jenkinsci.plugins.workflow.steps.StepContext;
+import org.jenkinsci.plugins.workflow.steps.StepExecution;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+import hudson.EnvVars;
+import hudson.FilePath;
 import hudson.LauncherDecorator;
+import hudson.slaves.EnvironmentVariablesNodeProperty;
+import hudson.slaves.NodeProperty;
+import hudson.slaves.NodePropertyDescriptor;
+import hudson.util.DescribableList;
 import io.fabric8.kubernetes.client.KubernetesClient;
-import org.jenkinsci.plugins.workflow.steps.StepExecution;
-
-import javax.annotation.Nonnull;
-
-import static org.csanchez.jenkins.plugins.kubernetes.pipeline.Resources.closeQuietly;
+import jenkins.model.Jenkins;
 
 public class ContainerStepExecution extends StepExecution {
 
@@ -60,7 +66,23 @@ public boolean start() throws Exception {
         client = nodeContext.connectToCloud();
 
         EnvironmentExpander env = getContext().get(EnvironmentExpander.class);
-        decorator = new ContainerExecDecorator(client, nodeContext.getPodName(), containerName, nodeContext.getNamespace(), env, getContext().get(FilePath.class));
+        EnvVars globalVars = null;
+        Jenkins instance = Jenkins.getInstance();
+        DescribableList<NodeProperty<?>, NodePropertyDescriptor> globalNodeProperties = instance
+                .getGlobalNodeProperties();
+        List<EnvironmentVariablesNodeProperty> envVarsNodePropertyList = globalNodeProperties
+                .getAll(EnvironmentVariablesNodeProperty.class);
+        if (envVarsNodePropertyList != null && envVarsNodePropertyList.size() != 0) {
+            globalVars = envVarsNodePropertyList.get(0).getEnvVars();
+        }
+        decorator = new ContainerExecDecorator();
+        decorator.setClient(client);
+        decorator.setPodName(nodeContext.getPodName());
+        decorator.setContainerName(containerName);
+        decorator.setNamespace(nodeContext.getNamespace());
+        decorator.setEnvironmentExpander(env);
+        decorator.setWs(getContext().get(FilePath.class));
+        decorator.setGlobalVars(globalVars);
         getContext().newBodyInvoker()
                 .withContext(BodyInvoker
                         .mergeLauncherDecorators(getContext().get(LauncherDecorator.class), decorator))

src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/AbstractKubernetesPipelineTest.java

@@ -50,6 +50,11 @@
 
 import com.google.common.collect.ImmutableMap;
 
+import hudson.EnvVars;
+import hudson.slaves.EnvironmentVariablesNodeProperty;
+import hudson.slaves.NodeProperty;
+import hudson.slaves.NodePropertyDescriptor;
+import hudson.util.DescribableList;
 import io.fabric8.kubernetes.api.model.Secret;
 import io.fabric8.kubernetes.api.model.SecretBuilder;
 import io.fabric8.kubernetes.client.KubernetesClient;
@@ -61,6 +66,7 @@ public class AbstractKubernetesPipelineTest {
     protected static final String SECRET_KEY = "password";
     protected static final String CONTAINER_ENV_VAR_FROM_SECRET_VALUE = "container-pa55w0rd";
     protected static final String POD_ENV_VAR_FROM_SECRET_VALUE = "pod-pa55w0rd";
+    protected static final String GLOBAL = "GLOBAL";
 
     @ClassRule
     public static BuildWatcher buildWatcher = new BuildWatcher();
@@ -96,6 +102,15 @@ public void configureCloud() throws Exception {
         JenkinsLocationConfiguration.get().setUrl(nonLocalhostUrl.toString());
 
         r.jenkins.clouds.add(cloud);
+
+        DescribableList<NodeProperty<?>, NodePropertyDescriptor> list =  r.jenkins.getGlobalNodeProperties();
+        list.getAll(hudson.slaves.EnvironmentVariablesNodeProperty.class);
+        EnvironmentVariablesNodeProperty newEnvVarsNodeProperty = new hudson.slaves.EnvironmentVariablesNodeProperty();
+        list.add(newEnvVarsNodeProperty);
+        EnvVars envVars = newEnvVarsNodeProperty.getEnvVars();
+        envVars.put("GLOBAL", "GLOBAL");
+        envVars.put("JAVA_HOME_X", "java-home-x");
+        r.jenkins.save();
     }
 
     private PodTemplate buildBusyboxTemplate(String label) {

src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesDeclarativeAgentTest.java

@@ -44,6 +44,7 @@ public void declarative() throws Exception {
         r.assertBuildStatusSuccess(r.waitForCompletion(b));
         r.assertLogContains("Apache Maven 3.3.9", b);
         r.assertLogContains("INSIDE_CONTAINER_ENV_VAR = " + CONTAINER_ENV_VAR_VALUE + "\n", b);
+        r.assertLogContains("OUTSIDE_CONTAINER_ENV_VAR = " + CONTAINER_ENV_VAR_VALUE + "\n", b);
     }
 
 }

src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesPipelineTest.java

@@ -109,12 +109,29 @@ public void runInPodWithExistingTemplate() throws Exception {
 
     @Test
     public void runWithEnvVariables() throws Exception {
-        WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p");
+        WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "runWithEnvVariables");
         p.setDefinition(new CpsFlowDefinition(loadPipelineScript("runWithEnvVars.groovy"), true));
         WorkflowRun b = p.scheduleBuild2(0).waitForStart();
         assertNotNull(b);
         r.assertBuildStatusSuccess(r.waitForCompletion(b));
         assertEnvVars(r, b);
+        r.assertLogContains("OUTSIDE_CONTAINER_BUILD_NUMBER = 1\n", b);
+        r.assertLogContains("INSIDE_CONTAINER_BUILD_NUMBER = 1\n", b);
+        r.assertLogContains("OUTSIDE_CONTAINER_JOB_NAME = runWithEnvVariables\n", b);
+        r.assertLogContains("INSIDE_CONTAINER_JOB_NAME = runWithEnvVariables\n", b);
+
+        // check that we are getting the correct java home
+        r.assertLogContains("INSIDE_JAVA_HOME =\n", b);
+        r.assertLogContains("JNLP_JAVA_HOME = /usr/lib/jvm/java-1.8-openjdk\n", b);
+        r.assertLogContains("JAVA7_HOME = /usr/lib/jvm/java-1.7-openjdk/jre\n", b);
+        r.assertLogContains("JAVA8_HOME = /usr/lib/jvm/java-1.8-openjdk/jre\n", b);
+
+        // check that we are not filtering too much
+        r.assertLogContains("INSIDE_JAVA_HOME_X = java-home-x\n", b);
+        r.assertLogContains("OUTSIDE_JAVA_HOME_X = java-home-x\n", b);
+        r.assertLogContains("JNLP_JAVA_HOME_X = java-home-x\n", b);
+        r.assertLogContains("JAVA7_HOME_X = java-home-x\n", b);
+        r.assertLogContains("JAVA8_HOME_X = java-home-x\n", b);
     }
 
     @Test
@@ -151,12 +168,15 @@ private void assertEnvVars(JenkinsRuleNonLocalhost r2, WorkflowRun b) throws Exc
         r.assertLogContains("INSIDE_CONTAINER_ENV_VAR_FROM_SECRET = " + CONTAINER_ENV_VAR_FROM_SECRET_VALUE + "\n", b);
         r.assertLogContains("INSIDE_POD_ENV_VAR = " + POD_ENV_VAR_VALUE + "\n", b);
         r.assertLogContains("INSIDE_POD_ENV_VAR_FROM_SECRET = " + POD_ENV_VAR_FROM_SECRET_VALUE + "\n", b);
+        r.assertLogContains("INSIDE_GLOBAL = " + GLOBAL + "\n", b);
 
         r.assertLogContains("OUTSIDE_CONTAINER_ENV_VAR =\n", b);
         r.assertLogContains("OUTSIDE_CONTAINER_ENV_VAR_LEGACY =\n", b);
         r.assertLogContains("OUTSIDE_CONTAINER_ENV_VAR_FROM_SECRET =\n", b);
         r.assertLogContains("OUTSIDE_POD_ENV_VAR = " + POD_ENV_VAR_VALUE + "\n", b);
         r.assertLogContains("OUTSIDE_POD_ENV_VAR_FROM_SECRET = " + POD_ENV_VAR_FROM_SECRET_VALUE + "\n", b);
+        r.assertLogContains("OUTSIDE_GLOBAL = " + GLOBAL + "\n", b);
+
     }
 
     @Test

src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/declarative.groovy

@@ -16,6 +16,8 @@ pipeline {
   stages {
     stage('Run maven') {
       steps {
+        sh 'set'
+        sh "echo OUTSIDE_CONTAINER_ENV_VAR = ${CONTAINER_ENV_VAR}"
         container('maven') {
           sh 'echo INSIDE_CONTAINER_ENV_VAR = ${CONTAINER_ENV_VAR}'
           sh 'mvn -version'

src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/runInPodWithExistingTemplate.groovy

@@ -7,6 +7,7 @@ node ('busybox') {
     echo OUTSIDE_CONTAINER_ENV_VAR_FROM_SECRET = \$CONTAINER_ENV_VAR_FROM_SECRET
     echo OUTSIDE_POD_ENV_VAR = \$POD_ENV_VAR
     echo OUTSIDE_POD_ENV_VAR_FROM_SECRET = \$POD_ENV_VAR_FROM_SECRET
+    echo OUTSIDE_GLOBAL = \$GLOBAL
     """
 
     stage('Run busybox') {
@@ -18,6 +19,7 @@ node ('busybox') {
             echo INSIDE_CONTAINER_ENV_VAR_FROM_SECRET = \$CONTAINER_ENV_VAR_FROM_SECRET
             echo INSIDE_POD_ENV_VAR = \$POD_ENV_VAR
             echo INSIDE_POD_ENV_VAR_FROM_SECRET = \$POD_ENV_VAR_FROM_SECRET
+            echo INSIDE_GLOBAL = \$GLOBAL
             """
         }
     }

src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/runWithEnvVars.groovy

@@ -11,25 +11,55 @@ podTemplate(label: 'mypod',
                 secretEnvVar(key: 'CONTAINER_ENV_VAR_FROM_SECRET', secretName: 'container-secret', secretKey: 'password')
             ],
         ),
+        containerTemplate(name: 'java7', image: 'openjdk:7u151-jre-alpine', ttyEnabled: true, command: '/bin/cat'),
+        containerTemplate(name: 'java8', image: 'openjdk:8u151-jre-alpine', ttyEnabled: true, command: '/bin/cat')
     ]) {
 
     node ('mypod') {
+
         sh """
+        echo OUTSIDE_CONTAINER_BUILD_NUMBER = \$BUILD_NUMBER
+        echo OUTSIDE_CONTAINER_JOB_NAME = \$JOB_NAME
         echo OUTSIDE_CONTAINER_ENV_VAR = \$CONTAINER_ENV_VAR
         echo OUTSIDE_CONTAINER_ENV_VAR_LEGACY = \$CONTAINER_ENV_VAR_LEGACY
         echo OUTSIDE_CONTAINER_ENV_VAR_FROM_SECRET = \$CONTAINER_ENV_VAR_FROM_SECRET
         echo OUTSIDE_POD_ENV_VAR = \$POD_ENV_VAR
         echo OUTSIDE_POD_ENV_VAR_FROM_SECRET = \$POD_ENV_VAR_FROM_SECRET
+        echo OUTSIDE_JAVA_HOME_X = \$JAVA_HOME_X
+        echo OUTSIDE_GLOBAL = \$GLOBAL
         """
         stage('Run busybox') {
             container('busybox') {
                 sh 'echo inside container'
                 sh """
+                echo INSIDE_CONTAINER_BUILD_NUMBER = \$BUILD_NUMBER
+                echo INSIDE_CONTAINER_JOB_NAME = \$JOB_NAME
                 echo INSIDE_CONTAINER_ENV_VAR = \$CONTAINER_ENV_VAR
                 echo INSIDE_CONTAINER_ENV_VAR_LEGACY = \$CONTAINER_ENV_VAR_LEGACY
                 echo INSIDE_CONTAINER_ENV_VAR_FROM_SECRET = \$CONTAINER_ENV_VAR_FROM_SECRET
                 echo INSIDE_POD_ENV_VAR = \$POD_ENV_VAR
                 echo INSIDE_POD_ENV_VAR_FROM_SECRET = \$POD_ENV_VAR_FROM_SECRET
+                echo INSIDE_JAVA_HOME_X = \$JAVA_HOME_X
+                echo INSIDE_JAVA_HOME = \$JAVA_HOME
+                echo INSIDE_GLOBAL = \$GLOBAL
+                """
+            }
+            container('jnlp') {
+                sh """
+                echo JNLP_JAVA_HOME = \$JAVA_HOME
+                echo JNLP_JAVA_HOME_X = \$JAVA_HOME_X
+                """
+            }
+            container('java7') {
+                sh """
+                echo JAVA7_HOME = \$JAVA_HOME
+                echo JAVA7_HOME_X = \$JAVA_HOME_X
+                """
+            }
+            container('java8') {
+                sh """
+                echo JAVA8_HOME = \$JAVA_HOME
+                echo JAVA8_HOME_X = \$JAVA_HOME_X
                 """
             }
         }