diff --git a/README.md b/README.md index 8d9adf6ff7..cfdd8b82de 100644 --- a/README.md +++ b/README.md @@ -141,6 +141,7 @@ Either way it provides access to the following fields: * **namespace** The namespace of the pod. * **label** The label of the pod. Set a unique value to avoid conflicts across builds * **yaml** [yaml representation of the Pod](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.10/#pod-v1-core), to allow setting any values not supported as fields +* **yamlMergeStrategy** `merge()` or `override()`. Controls whether the yaml definition overrides or is merged with the yaml definition inherited from pod templates declared with `inheritFrom`. Defaults to `override()`. * **containers** The container templates that are use to create the containers of the pod *(see below)*. * **serviceAccount** The service account of the pod. * **nodeSelector** The node selector of the pod. diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplate.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplate.java index 994d7450c5..c329589467 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplate.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplate.java @@ -16,6 +16,7 @@ import org.apache.commons.lang.StringUtils; import org.csanchez.jenkins.plugins.kubernetes.model.TemplateEnvVar; import org.csanchez.jenkins.plugins.kubernetes.pod.retention.PodRetention; +import org.csanchez.jenkins.plugins.kubernetes.pod.yaml.YamlMergeStrategy; import org.csanchez.jenkins.plugins.kubernetes.volumes.PodVolume; import org.csanchez.jenkins.plugins.kubernetes.volumes.workspace.WorkspaceVolume; import org.kohsuke.accmod.Restricted; @@ -129,6 +130,21 @@ public class PodTemplate extends AbstractDescribableImpl implements private List yamls = new ArrayList<>(); + public YamlMergeStrategy getYamlMergeStrategy() { + return yamlMergeStrategy; + } + + @DataBoundSetter + public void setYamlMergeStrategy(YamlMergeStrategy yamlMergeStrategy) { + this.yamlMergeStrategy = yamlMergeStrategy; + } + + private YamlMergeStrategy yamlMergeStrategy = YamlMergeStrategy.defaultStrategy(); + + public Pod getYamlsPod() { + return yamlMergeStrategy.merge(getYamls()); + } + private Boolean showRawYaml; @CheckForNull @@ -718,6 +734,10 @@ protected Object readResolve() { showRawYaml = Boolean.TRUE; } + if (yamlMergeStrategy == null) { + yamlMergeStrategy = YamlMergeStrategy.defaultStrategy(); + } + return this; } @@ -801,7 +821,8 @@ public List getEnvVarsDescriptors() { return DescriptorVisibilityFilter.apply(null, Jenkins.getInstance().getDescriptorList(TemplateEnvVar.class)); } - @SuppressWarnings("rawtypes") + @SuppressWarnings("unused") // Used by jelly + @Restricted(DoNotUse.class) // Used by jelly public Descriptor getDefaultPodRetention() { Jenkins jenkins = Jenkins.getInstanceOrNull(); if (jenkins == null) { @@ -810,6 +831,11 @@ public Descriptor getDefaultPodRetention() { return jenkins.getDescriptor(PodRetention.getPodTemplateDefault().getClass()); } + @SuppressWarnings("unused") // Used by jelly + @Restricted(DoNotUse.class) // Used by jelly + public YamlMergeStrategy getDefaultYamlMergeStrategy() { + return YamlMergeStrategy.defaultStrategy(); + } } @Override diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilder.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilder.java index 23f8558441..b285ea8cef 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilder.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilder.java @@ -198,8 +198,7 @@ public Pod build() { builder.withContainers(containers.values().toArray(new Container[containers.size()])); // merge with the yaml fragments - Pod yamlPods = combine(template.getYamls().stream().map(PodTemplateUtils::parseFromYaml).collect(Collectors.toList())); - Pod pod = combine(yamlPods, builder.endSpec().build()); + Pod pod = combine(template.getYamlsPod(), builder.endSpec().build()); // Apply defaults diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtils.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtils.java index 6a992d67a2..ec4699d1ee 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtils.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtils.java @@ -351,6 +351,7 @@ public static PodTemplate combine(PodTemplate parent, PodTemplate template) { podTemplate.setAnnotations(new ArrayList<>(podAnnotations)); podTemplate.setNodeProperties(toolLocationNodeProperties); podTemplate.setNodeUsageMode(nodeUsageMode); + podTemplate.setYamlMergeStrategy(template.getYamlMergeStrategy()); podTemplate.setInheritFrom(!Strings.isNullOrEmpty(template.getInheritFrom()) ? template.getInheritFrom() : parent.getInheritFrom()); diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStep.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStep.java index 9f85b2f1c2..a21192b8f1 100755 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStep.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStep.java @@ -13,6 +13,7 @@ import org.csanchez.jenkins.plugins.kubernetes.PodTemplate; import org.csanchez.jenkins.plugins.kubernetes.model.TemplateEnvVar; import org.csanchez.jenkins.plugins.kubernetes.pod.retention.PodRetention; +import org.csanchez.jenkins.plugins.kubernetes.pod.yaml.YamlMergeStrategy; import org.csanchez.jenkins.plugins.kubernetes.volumes.PodVolume; import org.csanchez.jenkins.plugins.kubernetes.volumes.workspace.WorkspaceVolume; import org.jenkinsci.plugins.workflow.steps.Step; @@ -58,6 +59,7 @@ public class PodTemplateStep extends Step implements Serializable { private String workingDir = ContainerTemplate.DEFAULT_WORKING_DIR; private String yaml; + private YamlMergeStrategy yamlMergeStrategy = YamlMergeStrategy.defaultStrategy(); private PodRetention podRetention; @DataBoundConstructor @@ -122,6 +124,15 @@ public void setEnvVars(List envVars) { } } + public YamlMergeStrategy getYamlMergeStrategy() { + return yamlMergeStrategy; + } + + @DataBoundSetter + public void setYamlMergeStrategy(YamlMergeStrategy yamlMergeStrategy) { + this.yamlMergeStrategy = yamlMergeStrategy; + } + public List getVolumes() { return volumes; } 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 index d6c7dc252d..4a348ea4f2 100755 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStepExecution.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStepExecution.java @@ -101,6 +101,7 @@ public boolean start() throws Exception { newTemplate.setNodeUsageMode(step.getNodeUsageMode()); newTemplate.setServiceAccount(step.getServiceAccount()); newTemplate.setAnnotations(step.getAnnotations()); + newTemplate.setYamlMergeStrategy(step.getYamlMergeStrategy()); if(run!=null) { newTemplate.getAnnotations().add(new PodAnnotation("buildUrl", ((KubernetesCloud)cloud).getJenkinsUrlOrDie()+run.getUrl())); } diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Merge.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Merge.java new file mode 100644 index 0000000000..0adca46978 --- /dev/null +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Merge.java @@ -0,0 +1,37 @@ +package org.csanchez.jenkins.plugins.kubernetes.pod.yaml; + +import hudson.Extension; +import hudson.model.Descriptor; +import io.fabric8.kubernetes.api.model.Pod; +import org.csanchez.jenkins.plugins.kubernetes.PodTemplateUtils; +import org.jenkinsci.Symbol; +import org.kohsuke.stapler.DataBoundConstructor; + +import javax.annotation.Nonnull; +import java.util.List; +import java.util.stream.Collectors; + +import static org.csanchez.jenkins.plugins.kubernetes.PodTemplateUtils.combine; + +public class Merge extends YamlMergeStrategy { + private static final long serialVersionUID = 6562610892063268131L; + + @DataBoundConstructor + public Merge() { + } + + @Override + public Pod merge(List yamls) { + return combine(yamls.stream().map(PodTemplateUtils::parseFromYaml).collect(Collectors.toList())); + } + + @Extension + @Symbol("merge") + public static class DescriptorImpl extends Descriptor { + @Nonnull + @Override + public String getDisplayName() { + return "Merge"; + } + } +} diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Overrides.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Overrides.java new file mode 100644 index 0000000000..3157271b0f --- /dev/null +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Overrides.java @@ -0,0 +1,39 @@ +package org.csanchez.jenkins.plugins.kubernetes.pod.yaml; + +import hudson.Extension; +import hudson.model.Descriptor; +import io.fabric8.kubernetes.api.model.Pod; +import org.jenkinsci.Symbol; +import org.kohsuke.stapler.DataBoundConstructor; + +import javax.annotation.Nonnull; +import java.util.List; + +import static org.csanchez.jenkins.plugins.kubernetes.PodTemplateUtils.parseFromYaml; + +public class Overrides extends YamlMergeStrategy { + private static final long serialVersionUID = 4510341864619338164L; + + @DataBoundConstructor + public Overrides() { + } + + @Override + public Pod merge(List yamls) { + if (yamls.isEmpty()) { + return null; + } else { + return parseFromYaml(yamls.get(yamls.size()-1)); + } + } + + @Extension + @Symbol("override") + public static class DescriptorImpl extends Descriptor { + @Nonnull + @Override + public String getDisplayName() { + return "Override"; + } + } +} diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/YamlMergeStrategy.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/YamlMergeStrategy.java new file mode 100644 index 0000000000..741a258b09 --- /dev/null +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/YamlMergeStrategy.java @@ -0,0 +1,17 @@ +package org.csanchez.jenkins.plugins.kubernetes.pod.yaml; + +import hudson.ExtensionPoint; +import hudson.model.AbstractDescribableImpl; +import io.fabric8.kubernetes.api.model.Pod; + +import javax.annotation.Nonnull; +import java.io.Serializable; +import java.util.List; + +public abstract class YamlMergeStrategy extends AbstractDescribableImpl implements ExtensionPoint, Serializable { + public static YamlMergeStrategy defaultStrategy() { + return new Overrides(); + } + + public abstract Pod merge(@Nonnull List yamls); +} diff --git a/src/main/resources/org/csanchez/jenkins/plugins/kubernetes/PodTemplate/config.jelly b/src/main/resources/org/csanchez/jenkins/plugins/kubernetes/PodTemplate/config.jelly index 6562fc5871..35d27ba8e0 100644 --- a/src/main/resources/org/csanchez/jenkins/plugins/kubernetes/PodTemplate/config.jelly +++ b/src/main/resources/org/csanchez/jenkins/plugins/kubernetes/PodTemplate/config.jelly @@ -71,6 +71,8 @@ + + diff --git a/src/main/resources/org/csanchez/jenkins/plugins/kubernetes/PodTemplate/help-yamlMergeStrategy.html b/src/main/resources/org/csanchez/jenkins/plugins/kubernetes/PodTemplate/help-yamlMergeStrategy.html new file mode 100644 index 0000000000..3da76ecab4 --- /dev/null +++ b/src/main/resources/org/csanchez/jenkins/plugins/kubernetes/PodTemplate/help-yamlMergeStrategy.html @@ -0,0 +1 @@ +Defines how the raw yaml field gets merged with yaml definitions from inherited pod templates. diff --git a/src/main/resources/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Merge/config.jelly b/src/main/resources/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Merge/config.jelly new file mode 100644 index 0000000000..451886f724 --- /dev/null +++ b/src/main/resources/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Merge/config.jelly @@ -0,0 +1,5 @@ + + + The yaml definitions from inherited pod templates are merged with the current template yaml fragment. + diff --git a/src/main/resources/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Overrides/config.jelly b/src/main/resources/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Overrides/config.jelly new file mode 100644 index 0000000000..3dd1a4de4a --- /dev/null +++ b/src/main/resources/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Overrides/config.jelly @@ -0,0 +1,5 @@ + + + The yaml definitions from inherited pod templates are completely overridden by the current yaml field. + diff --git a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilderTest.java b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilderTest.java index db0584e2d0..08db89359f 100644 --- a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilderTest.java +++ b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilderTest.java @@ -20,6 +20,8 @@ import org.apache.commons.compress.utils.IOUtils; import org.csanchez.jenkins.plugins.kubernetes.model.KeyValueEnvVar; import org.csanchez.jenkins.plugins.kubernetes.model.TemplateEnvVar; +import org.csanchez.jenkins.plugins.kubernetes.pod.yaml.Merge; +import org.csanchez.jenkins.plugins.kubernetes.pod.yaml.YamlMergeStrategy; import org.csanchez.jenkins.plugins.kubernetes.volumes.EmptyDirVolume; import org.csanchez.jenkins.plugins.kubernetes.volumes.HostPathVolume; import org.csanchez.jenkins.plugins.kubernetes.volumes.PodVolume; @@ -331,6 +333,7 @@ public void yamlMergeContainers() throws Exception { " - cat\n" + " tty: true\n" ); + child.setYamlMergeStrategy(merge()); child.setInheritFrom("parent"); setupStubs(); PodTemplate result = combine(parent, child); @@ -372,6 +375,7 @@ public void yamlOverrideContainer() throws Exception { " tty: true\n" ); child.setInheritFrom("parent"); + child.setYamlMergeStrategy(merge()); setupStubs(); PodTemplate result = combine(parent, child); Pod pod = new PodTemplateBuilder(result).withSlave(slave).build(); @@ -396,6 +400,7 @@ public void yamlOverrideContainerEnvvar() throws Exception { " - name: VAR2\n" + " value: \"1\"\n"); PodTemplate child = new PodTemplate(); + child.setYamlMergeStrategy(merge()); child.setYaml("kind: Pod\n" + "spec:\n" + " containers:\n" + @@ -439,6 +444,7 @@ public void yamlOverrideVolume() throws Exception { " path: /host/data2\n" ); child.setInheritFrom("parent"); + child.setYamlMergeStrategy(merge()); setupStubs(); PodTemplate result = combine(parent, child); Pod pod = new PodTemplateBuilder(result).withSlave(slave).build(); @@ -474,6 +480,7 @@ public void yamlMergeVolumes() throws Exception { " path: /host/data2\n" ); child.setInheritFrom("parent"); + child.setYamlMergeStrategy(merge()); setupStubs(); PodTemplate result = combine(parent, child); Pod pod = new PodTemplateBuilder(result).withSlave(slave).build(); @@ -511,4 +518,8 @@ private Map toContainerMap(Pod pod) { private String loadYamlFile(String s) throws IOException { return new String(IOUtils.toByteArray(getClass().getResourceAsStream(s))); } + + private YamlMergeStrategy merge() { + return new Merge(); + } } 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 index 58f9453aac..593e7f2ea2 100644 --- a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesPipelineTest.java +++ b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesPipelineTest.java @@ -345,4 +345,16 @@ public void runInPodWithRetention() throws Exception { r.assertBuildStatusSuccess(r.waitForCompletion(b)); assertTrue(deletePods(cloud.connect(), getLabels(this, name), true)); } + + @Test + @Issue("JENKINS-58405") + public void overrideYaml() throws Exception { + r.assertBuildStatusSuccess(r.waitForCompletion(b)); + } + + @Test + @Issue("JENKINS-58405") + public void mergeYaml() throws Exception { + r.assertBuildStatusSuccess(r.waitForCompletion(b)); + } } diff --git a/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/mergeYaml.groovy b/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/mergeYaml.groovy new file mode 100644 index 0000000000..c59893891c --- /dev/null +++ b/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/mergeYaml.groovy @@ -0,0 +1,22 @@ +podTemplate(label: 'mergeYaml-parent', yaml: """ +spec: + containers: + - name: jnlp + env: + - name: VAR1 + value: 1 +""") { + podTemplate(label: 'mergeYaml-child', + yamlMergeStrategy: merge(), + yaml: """ + containers: + - name: jnlp + env: + - name: VAR2 + value: 1 +""") { + node('mergeYaml-child'){ + sh '["$VAR1" == "$VAR2"]' + } + } +} \ No newline at end of file diff --git a/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/overrideYaml.groovy b/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/overrideYaml.groovy new file mode 100644 index 0000000000..e05dfde37b --- /dev/null +++ b/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/overrideYaml.groovy @@ -0,0 +1,21 @@ +podTemplate(label: 'mergeYaml-parent', yaml: """ +spec: + containers: + - name: jnlp + env: + - name: VAR1 + value: 1 +""") { + podTemplate(label: 'mergeYaml-child', + yaml: """ + containers: + - name: jnlp + env: + - name: VAR2 + value: 1 +""") { + node('mergeYaml-child'){ + sh '["$VAR1" != "$VAR2"]' + } + } +} \ No newline at end of file