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 56514ceb6a..14b5f4b847 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplate.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplate.java @@ -13,6 +13,7 @@ import javax.annotation.CheckForNull; import javax.annotation.Nonnull; +import com.google.common.annotations.VisibleForTesting; import org.apache.commons.lang.StringUtils; import org.csanchez.jenkins.plugins.kubernetes.model.TemplateEnvVar; import org.csanchez.jenkins.plugins.kubernetes.pod.retention.PodRetention; @@ -123,12 +124,14 @@ public class PodTemplate extends AbstractDescribableImpl implements private PodTemplateToolLocation nodeProperties; /** - * @deprecated Stored as a list of yaml fragments + * Persisted yaml fragment */ - @Deprecated - private transient String yaml; + private String yaml; - private List yamls = new ArrayList<>(); + /** + * List of yaml fragments used for transient pod templates. Never persisted + */ + private transient List yamls; public YamlMergeStrategy getYamlMergeStrategy() { return yamlMergeStrategy; @@ -170,6 +173,7 @@ public PodTemplate(PodTemplate from) { this.setActiveDeadlineSeconds(from.getActiveDeadlineSeconds()); this.setVolumes(from.getVolumes()); this.setWorkspaceVolume(from.getWorkspaceVolume()); + this.yaml = from.yaml; this.setYamls(from.getYamls()); this.setShowRawYaml(from.isShowRawYaml()); this.setNodeProperties(from.getNodeProperties()); @@ -633,31 +637,36 @@ public List getContainers() { } /** - * @return The first yaml fragment for this pod template + * @return The persisted yaml fragment */ @Restricted(NoExternalUse.class) // Tests and UI public String getYaml() { - return yamls == null || yamls.isEmpty() ? null : yamls.get(0); + return yaml; } @DataBoundSetter public void setYaml(String yaml) { - String trimmed = Util.fixEmpty(yaml); - if (trimmed != null) { - this.yamls = Collections.singletonList(yaml); - } else { - this.yamls = Collections.emptyList(); - } + this.yaml = Util.fixEmpty(yaml); } @Nonnull public List getYamls() { - if (yamls ==null) { - return Collections.emptyList(); + if (yamls == null) { + if (yaml != null) { + return Collections.singletonList(yaml); + } else { + return Collections.emptyList(); + } } return yamls; } + @VisibleForTesting + @Restricted(NoExternalUse.class) + List _getYamls() { + return yamls; + } + public void setYamls(List yamls) { if (yamls != null) { List ys = new ArrayList<>(); @@ -716,18 +725,19 @@ protected Object readResolve() { annotations = new ArrayList<>(); } - if (yamls == null) { - yamls = new ArrayList<>(); - } - - if (yaml != null) { - yamls.add(yaml); - yaml = null; - } - - // JENKINS-57116 remove empty items from yamls - if (!yamls.isEmpty() && StringUtils.isBlank(yamls.get(0))) { + // Sanitize empty values + yaml = Util.fixEmpty(yaml); + if (yamls != null) { + // JENKINS-57116 Sanitize empty values setYamls(yamls); + // Migration from storage in yamls field + if (!yamls.isEmpty()) { + if (yamls.size() > 1) { + LOGGER.log(Level.WARNING, "Found several persisted YAML fragments in pod template " + name + ". Only the first fragment will be considered, others will be ignored."); + } + yaml = yamls.get(0); + } + yamls = null; } if (showRawYaml == null) { diff --git a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesTest.java b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesTest.java index d179120bfb..7ab3ad6f60 100644 --- a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesTest.java +++ b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesTest.java @@ -31,6 +31,7 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.logging.Level; import org.csanchez.jenkins.plugins.kubernetes.model.KeyValueEnvVar; import org.csanchez.jenkins.plugins.kubernetes.pod.retention.Default; @@ -44,6 +45,7 @@ import org.junit.Test; import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.LoggerRule; import org.jvnet.hudson.test.recipes.LocalData; import com.cloudbees.plugins.credentials.Credentials; @@ -68,6 +70,9 @@ public class KubernetesTest { @Rule public JenkinsRule r = new JenkinsRule(); + @Rule + public LoggerRule log = new LoggerRule(); + private KubernetesCloud cloud; @Before @@ -85,6 +90,30 @@ public void upgradeFrom_1_17_2() throws Exception { assertThat(cloud.getPodLabelsMap(), hasEntry("biff", "johnson")); } + @Test + @LocalData + public void upgradeFrom_1_15_9() { + List templates = cloud.getTemplates(); + assertPodTemplates(templates); + PodTemplate template = templates.get(0); + assertEquals("blah", template.getYaml()); + assertEquals(Collections.singletonList("blah"), template.getYamls()); + assertNull(template._getYamls()); + } + + @Test + @LocalData + public void upgradeFrom_1_15_9_invalid() { + log.record(PodTemplate.class, Level.WARNING).capture(1); + List templates = cloud.getTemplates(); + assertPodTemplates(templates); + PodTemplate template = templates.get(0); + assertEquals("blah", template.getYaml()); + assertEquals(Collections.singletonList("blah"), template.getYamls()); + assertNull(template._getYamls()); + log.getMessages().stream().anyMatch(msg -> msg.contains("Found several persisted YAML fragments in pod template java")); + } + @Test @LocalData() @Issue("JENKINS-57116") diff --git a/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/KubernetesTest/upgradeFrom_1_15_9/config.xml b/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/KubernetesTest/upgradeFrom_1_15_9/config.xml new file mode 100644 index 0000000000..dc1b676cfa --- /dev/null +++ b/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/KubernetesTest/upgradeFrom_1_15_9/config.xml @@ -0,0 +1,144 @@ + + + + 2.138.4 + DEVELOPMENT + 2 + NORMAL + true + + + false + + ${JENKINS_HOME}/workspace/${ITEM_FULLNAME} + ${ITEM_ROOTDIR}/builds + + + + + + kubernetes + + + + + java + + false + false + false + 2147483647 + 100 + 0 + 0 + + + NORMAL + false + + false + + + + /mnt + false + + + /host + /mnt/host + + + + + jnlp + jenkins/jnlp-slave + false + false + /home/jenkins + + ${computer.jnlpmac} ${computer.name} + false + 500m + 250Mi + 500m + 250Mi + + + a + b + + + c + d + + + + + + 0 + 0 + 0 + 0 + 0 + + + + + + a + b + + + c + d + + + + + aa + bb + + + + + + + + + blah + + + + + https://192.168.64.1 + true + false + false + default + 10 + 5 + 0 + 0 + false + 32 + 600 + + + + 5 + 0 + + + + all + false + false + + + + all + 0 + + + + \ No newline at end of file diff --git a/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/KubernetesTest/upgradeFrom_1_15_9_invalid/config.xml b/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/KubernetesTest/upgradeFrom_1_15_9_invalid/config.xml new file mode 100644 index 0000000000..c39baa4b7b --- /dev/null +++ b/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/KubernetesTest/upgradeFrom_1_15_9_invalid/config.xml @@ -0,0 +1,145 @@ + + + + 2.138.4 + DEVELOPMENT + 2 + NORMAL + true + + + false + + ${JENKINS_HOME}/workspace/${ITEM_FULLNAME} + ${ITEM_ROOTDIR}/builds + + + + + + kubernetes + + + + + java + + false + false + false + 2147483647 + 100 + 0 + 0 + + + NORMAL + false + + false + + + + /mnt + false + + + /host + /mnt/host + + + + + jnlp + jenkins/jnlp-slave + false + false + /home/jenkins + + ${computer.jnlpmac} ${computer.name} + false + 500m + 250Mi + 500m + 250Mi + + + a + b + + + c + d + + + + + + 0 + 0 + 0 + 0 + 0 + + + + + + a + b + + + c + d + + + + + aa + bb + + + + + + + + + blah + foo + + + + + https://192.168.64.1 + true + false + false + default + 10 + 5 + 0 + 0 + false + 32 + 600 + + + + 5 + 0 + + + + all + false + false + + + + all + 0 + + + + \ No newline at end of file