[JENKINS-57548] Explicit inheritance should override implicit inherit…

…ance. Declarative k8s template do not inherit from parent pod template by default.
jenkinsci · Jun 2, 2019 · 3ba8352 · 3ba8352
1 parent 1a17ecf
commit 3ba8352
Show file tree

Hide file tree

Showing 8 changed files with 125 additions and 3 deletions.
diff --git a/README.md b/README.md
@@ -313,6 +313,18 @@ slaveTemplates.dockerTemplate {
 }
 ```
 
+There are cases where this implicit inheritance via nested declaration is not wanted or another explicit inheritance is preferred.
+In this case, use `inheritFrom ''` to remove any inheritance, or `inheritFrom 'otherParent'` to override it.
+
+```groovy
+    podTemplate(label: 'docker-linux', containers: [containerTemplate(image: 'docker', name: 'docker-linux', command: 'cat', ttyEnabled: true)]) {
+        // Will run on linux node
+        podTemplate(label: 'maven-windows', inheritFrom: '', nodeSelector: 'os:windows', containers: [containerTemplate(image: 'maven-windows-servercore', name: 'maven-windows', command: 'cat', ttyEnabled: true)]) {
+            // Will run on windows node without merging the docker pod
+        }
+    }
+```
+
 #### Using a different namespace
 
 There might be cases, where you need to have the agent pod run inside a different namespace than the one configured with the cloud definition.

diff --git a/...ain/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesDeclarativeAgent.java b/...ain/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesDeclarativeAgent.java
@@ -246,7 +246,7 @@ public Map<String, Object> getAsArgs() {
         if (idleMinutes != 0) {
             argMap.put("idleMinutes", idleMinutes);
         }
-        if (!StringUtils.isEmpty(inheritFrom)) {
+        if (inheritFrom != null) {
             argMap.put("inheritFrom", inheritFrom);
         }
         if (!StringUtils.isEmpty(serviceAccount)) {

diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStepExecution.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStepExecution.java
@@ -82,7 +82,12 @@ public boolean start() throws Exception {
         newTemplate = new PodTemplate();
         newTemplate.setName(name);
         newTemplate.setNamespace(namespace);
-        newTemplate.setInheritFrom(!Strings.isNullOrEmpty(parentTemplates) ? parentTemplates : step.getInheritFrom());
+
+        if (step.getInheritFrom() == null) {
+            newTemplate.setInheritFrom(Strings.emptyToNull(parentTemplates));
+        } else {
+            newTemplate.setInheritFrom(Strings.emptyToNull(step.getInheritFrom()));
+        }
         newTemplate.setInstanceCap(step.getInstanceCap());
         newTemplate.setIdleMinutes(step.getIdleMinutes());
         newTemplate.setSlaveConnectTimeout(step.getSlaveConnectTimeout());

diff --git a/.../org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesDeclarativeAgentScript.groovy b/.../org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesDeclarativeAgentScript.groovy
@@ -39,6 +39,10 @@ public class KubernetesDeclarativeAgentScript extends DeclarativeAgentScript<Kub
             if ((describable.getYamlFile() != null) && (describable.hasScmContext(script))) {
                 describable.setYaml(script.readTrusted(describable.getYamlFile()))
             }
+            if (describable.getInheritFrom() == null) {
+                // Do not implicitly inherit from parent template context for declarative Kubernetes agent declaration
+                describable.setInheritFrom("")
+            }
             script.podTemplate(describable.asArgs) {
                 script.node(describable.label) {
                     CheckoutScript.doCheckout(script, describable, describable.customWorkspace) {

diff --git a/...java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesDeclarativeAgentTest.java b/...java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesDeclarativeAgentTest.java
@@ -34,7 +34,6 @@
 import org.jenkinsci.plugins.workflow.job.WorkflowRun;
 import org.junit.Rule;
 import org.junit.Test;
-import org.junit.rules.TestName;
 import org.jvnet.hudson.test.Issue;
 
 public class KubernetesDeclarativeAgentTest extends AbstractKubernetesPipelineTest {
@@ -126,4 +125,16 @@ public void declarativeUseCustomWorkspace() throws Exception {
         r.assertLogContains("Apache Maven 3.3.9", b);
         r.assertLogContains("Workspace dir is", b);
     }
+
+    @Issue("JENKINS-57548")
+    @Test
+    public void declarativeWithNestedExplicitInheritance() throws Exception {
+        WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "job with explicit nested inherit");
+        p.setDefinition(new CpsFlowDefinition(loadPipelineScript("declarativeWithNestedExplicitInheritance.groovy"), true));
+        WorkflowRun b = p.scheduleBuild2(0).waitForStart();
+        assertNotNull(b);
+        r.assertBuildStatusSuccess(r.waitForCompletion(b));
+        r.assertLogContains("Apache Maven 3.3.9", b);
+        r.assertLogNotContains("go version go1.6.3", b);
+    }
 }
diff --git a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesPipelineTest.java b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesPipelineTest.java
@@ -47,6 +47,7 @@
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
+import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRuleNonLocalhost;
 
 import hudson.model.Result;
@@ -210,6 +211,16 @@ public void runInPodNested() throws Exception {
         r.assertLogContains("go version go1.6.3", b);
     }
 
+    @Issue("JENKINS-57548")
+    @Test
+    public void runInPodNestedExplicitInherit() throws Exception {
+        r.assertBuildStatusSuccess(r.waitForCompletion(b));
+        r.assertLogContains("[maven] maven:3.3.9-jdk-8-alpine", b);
+        r.assertLogNotContains("[golang] golang:1.6.3-alpine", b);
+        r.assertLogContains("Apache Maven 3.3.9", b);
+        r.assertLogNotContains("go version go1.6.3", b);
+    }
+
     @Test
     public void runInPodWithExistingTemplate() throws Exception {
         r.assertBuildStatusSuccess(r.waitForCompletion(b));

diff --git a/...nchez/jenkins/plugins/kubernetes/pipeline/declarativeWithNestedExplicitInheritance.groovy b/...nchez/jenkins/plugins/kubernetes/pipeline/declarativeWithNestedExplicitInheritance.groovy
@@ -0,0 +1,50 @@
+pipeline {
+  agent {
+    kubernetes {
+      label 'parent-pod'
+      yaml """
+spec:
+  containers:
+  - name: golang
+    image: golang:1.6.3-alpine
+    command:
+    - cat
+    tty: true
+"""
+    }
+  }
+  stages {
+    stage('Run maven') {
+        agent {
+            kubernetes {
+                label 'nested-pod'
+                yaml """
+spec:
+  containers:
+  - name: maven
+    image: maven:3.3.9-jdk-8-alpine
+    command:
+    - cat
+    tty: true
+"""
+            }
+        }
+      steps {
+        container('maven') {
+          sh 'echo MAVEN_CONTAINER_ENV_VAR = ${CONTAINER_ENV_VAR}'
+          sh 'mvn -version'
+        }
+        container('golang') {
+          script {
+            try {
+              sh "go version"
+              error("Should not inherit")
+            } catch (e) {
+              // ignored
+            }
+          }
+        }
+      }
+    }
+  }
+}
diff --git a/...ces/org/csanchez/jenkins/plugins/kubernetes/pipeline/runInPodNestedExplicitInherit.groovy b/...ces/org/csanchez/jenkins/plugins/kubernetes/pipeline/runInPodNestedExplicitInherit.groovy
@@ -0,0 +1,29 @@
+podTemplate(label: 'runInPodNestedExplicitInherit1', containers: [
+		containerTemplate(name: 'golang', image: 'golang:1.6.3-alpine', ttyEnabled: true, command: '/bin/cat'),
+	]) {
+
+	podTemplate(label: 'runInPodNestedExplicitInherit', inheritFrom: '',  containers: [
+		containerTemplate(name: 'maven', image: 'maven:3.3.9-jdk-8-alpine', ttyEnabled: true, command: '/bin/cat'),
+	]) {
+
+		node ('runInPodNestedExplicitInherit') {
+			stage('Nested') {
+				container('maven') {
+					sh "mvn -version"
+				}
+			}
+			stage('Parent') {
+				container('golang') {
+                    script {
+                        try {
+                            sh "go version"
+                            error("Should not inherit")
+                        } catch (e) {
+                            // ignored
+                        }
+                    }
+				}
+			}
+		}
+	}
+}