From bab940e4a086cb1f2d15e7bede9021f22110d6cb Mon Sep 17 00:00:00 2001 From: Allan Burdajewicz Date: Fri, 20 Dec 2019 15:01:45 +1000 Subject: [PATCH] [JENKINS-60551] Copy constructor test should check all fields * Default jelly values from java constants * Apply min values for readTimeout (15 sec) and connectTimeout (5 sec) * Add missing serverCertificate, readTimeout and capOnlyOnAlivePods to copy constructor --- .../plugins/kubernetes/KubernetesCloud.java | 43 +++++++++++++++--- .../kubernetes/KubernetesCloud/config.jelly | 8 ++-- .../kubernetes/KubernetesCloudTest.java | 45 ++++++++++++------- 3 files changed, 72 insertions(+), 24 deletions(-) diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java index 16dd08fa16..abc2f71e15 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java @@ -96,6 +96,10 @@ public class KubernetesCloud extends Cloud { /** Default timeout for idle workers that don't correctly indicate exit. */ public static final int DEFAULT_RETENTION_TIMEOUT_MINUTES = 5; + public static final int DEFAULT_READ_TIMEOUT_SECONDS = 15; + + public static final int DEFAULT_CONNECT_TIMEOUT_SECONDS = 5; + private String defaultsProviderTemplate; @Nonnull @@ -118,8 +122,8 @@ public class KubernetesCloud extends Cloud { private String credentialsId; private int containerCap = Integer.MAX_VALUE; private int retentionTimeout = DEFAULT_RETENTION_TIMEOUT_MINUTES; - private int connectTimeout; - private int readTimeout; + private int connectTimeout = DEFAULT_CONNECT_TIMEOUT_SECONDS; + private int readTimeout = DEFAULT_READ_TIMEOUT_SECONDS; /** @deprecated Stored as a list of PodLabels */ @Deprecated private transient Map labels; @@ -152,6 +156,7 @@ public KubernetesCloud(@NonNull String name, @NonNull KubernetesCloud source) { this.defaultsProviderTemplate = source.defaultsProviderTemplate; this.templates.addAll(source.templates); this.serverUrl = source.serverUrl; + this.serverCertificate = source.serverCertificate; this.skipTlsVerify = source.skipTlsVerify; this.addMasterProxyEnvVars = source.addMasterProxyEnvVars; this.namespace = source.namespace; @@ -162,10 +167,12 @@ public KubernetesCloud(@NonNull String name, @NonNull KubernetesCloud source) { this.containerCap = source.containerCap; this.retentionTimeout = source.retentionTimeout; this.connectTimeout = source.connectTimeout; + this.readTimeout = source.readTimeout; this.usageRestricted = source.usageRestricted; this.maxRequestsPerHost = source.maxRequestsPerHost; this.podRetention = source.podRetention; this.waitForPodSec = source.waitForPodSec; + this.capOnlyOnAlivePods = source.capOnlyOnAlivePods; setPodLabels(source.podLabels); } @@ -412,7 +419,7 @@ public int getReadTimeout() { @DataBoundSetter public void setReadTimeout(int readTimeout) { - this.readTimeout = readTimeout; + this.readTimeout = Math.max(DEFAULT_READ_TIMEOUT_SECONDS, readTimeout); } public int getConnectTimeout() { @@ -486,7 +493,7 @@ public int getMaxRequestsPerHost() { @DataBoundSetter public void setConnectTimeout(int connectTimeout) { - this.connectTimeout = connectTimeout; + this.connectTimeout = Math.max(DEFAULT_CONNECT_TIMEOUT_SECONDS, connectTimeout); } /** @@ -761,6 +768,7 @@ public static void addAliases() { } @RequirePOST + @SuppressWarnings("unused") // used by jelly public FormValidation doTestConnection(@QueryParameter String name, @QueryParameter String serverUrl, @QueryParameter String credentialsId, @QueryParameter String serverCertificate, @QueryParameter boolean skipTlsVerify, @@ -790,6 +798,7 @@ public FormValidation doTestConnection(@QueryParameter String name, @QueryParame } @RequirePOST + @SuppressWarnings("unused") // used by jelly public ListBoxModel doFillCredentialsIdItems(@QueryParameter String serverUrl) { Jenkins.get().checkPermission(Jenkins.ADMINISTER); return new StandardListBoxModel().withEmptySelection() // @@ -813,6 +822,7 @@ public ListBoxModel doFillCredentialsIdItems(@QueryParameter String serverUrl) { } @RequirePOST + @SuppressWarnings("unused") // used by jelly public FormValidation doCheckMaxRequestsPerHostStr(@QueryParameter String value) throws IOException, ServletException { try { Integer.parseInt(value); @@ -822,6 +832,7 @@ public FormValidation doCheckMaxRequestsPerHostStr(@QueryParameter String value) } } + @SuppressWarnings("unused") // used by jelly public FormValidation doCheckDirectConnection(@QueryParameter boolean value, @QueryParameter String jenkinsUrl) throws IOException, ServletException { int slaveAgentPort = Jenkins.get().getSlaveAgentPort(); if(slaveAgentPort == -1) return FormValidation.warning( @@ -845,6 +856,7 @@ public FormValidation doCheckDirectConnection(@QueryParameter boolean value, @Qu return FormValidation.ok(); } + @SuppressWarnings("unused") // used by jelly public FormValidation doCheckJenkinsUrl(@QueryParameter String value, @QueryParameter boolean directConnection) throws IOException, ServletException { try { if(!isEmpty(value)) new URL(value); @@ -854,6 +866,7 @@ public FormValidation doCheckJenkinsUrl(@QueryParameter String value, @QueryPara return FormValidation.ok(); } + @SuppressWarnings("unused") // used by jelly public List> getAllowedPodRetentions() { Jenkins jenkins = Jenkins.getInstanceOrNull(); if (jenkins == null) { @@ -862,7 +875,7 @@ public List> getAllowedPodRetentions() { return DescriptorVisibilityFilter.apply(this, jenkins.getDescriptorList(PodRetention.class)); } - @SuppressWarnings("rawtypes") + @SuppressWarnings({"rawtypes", "unused"}) // used by jelly public Descriptor getDefaultPodRetention() { Jenkins jenkins = Jenkins.getInstanceOrNull(); if (jenkins == null) { @@ -871,6 +884,25 @@ public Descriptor getDefaultPodRetention() { return jenkins.getDescriptor(PodRetention.getKubernetesCloudDefault().getClass()); } + @SuppressWarnings("unused") // used by jelly + public int getDefaultReadTimeout() { + return DEFAULT_READ_TIMEOUT_SECONDS; + } + + @SuppressWarnings("unused") // used by jelly + public int getDefaultConnectTimeout() { + return DEFAULT_CONNECT_TIMEOUT_SECONDS; + } + + @SuppressWarnings("unused") // used by jelly + public int getDefaultRetentionTimeout() { + return DEFAULT_RETENTION_TIMEOUT_MINUTES; + } + + public int getDefaultWaitForPod() { + return DEFAULT_WAIT_FOR_POD_SEC; + } + } @Override @@ -913,6 +945,7 @@ private Object readResolve() { if (podRetention == null) { podRetention = PodRetention.getKubernetesCloudDefault(); } + setReadTimeout(readTimeout); setRetentionTimeout(retentionTimeout); if (waitForPodSec == null) { waitForPodSec = DEFAULT_WAIT_FOR_POD_SEC; diff --git a/src/main/resources/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud/config.jelly b/src/main/resources/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud/config.jelly index 935e8cf58a..f68460b7bf 100644 --- a/src/main/resources/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud/config.jelly +++ b/src/main/resources/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud/config.jelly @@ -40,11 +40,11 @@ - + - + @@ -64,12 +64,12 @@ - + - + diff --git a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloudTest.java b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloudTest.java index c51a3970c1..aa6b127300 100644 --- a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloudTest.java +++ b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloudTest.java @@ -3,6 +3,7 @@ import static org.junit.Assert.*; import java.io.IOException; +import java.lang.reflect.InvocationTargetException; import java.security.KeyStoreException; import java.security.NoSuchAlgorithmException; import java.security.UnrecoverableKeyException; @@ -26,6 +27,10 @@ import com.gargoylesoftware.htmlunit.html.HtmlInput; import com.gargoylesoftware.htmlunit.html.HtmlPage; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Lists; +import org.apache.commons.beanutils.PropertyUtils; +import org.apache.commons.lang3.RandomStringUtils; +import org.apache.commons.lang3.RandomUtils; import org.csanchez.jenkins.plugins.kubernetes.pod.retention.Always; import org.csanchez.jenkins.plugins.kubernetes.pod.retention.PodRetention; import org.csanchez.jenkins.plugins.kubernetes.volumes.EmptyDirVolume; @@ -274,28 +279,38 @@ public void testLabels() { } @Test - public void copyConstructor() { + public void copyConstructor() throws Exception { PodTemplate pt = new PodTemplate(); pt.setName("podTemplate"); KubernetesCloud cloud = new KubernetesCloud("name"); - cloud.setDefaultsProviderTemplate("default"); + ArrayList objectProperties = Lists.newArrayList("templates", "podRetention", "podLabels", "labels"); + for (String property: PropertyUtils.describe(cloud).keySet()) { + if (PropertyUtils.isWriteable(cloud, property)) { + Class propertyType = PropertyUtils.getPropertyType(cloud, property); + if (propertyType == String.class) { + if (property.endsWith("Str")) { + // setContainerCapStr + // setMaxRequestsPerHostStr + PropertyUtils.setProperty(cloud, property, RandomStringUtils.randomNumeric(3)); + } else { + PropertyUtils.setProperty(cloud, property, RandomStringUtils.randomAlphabetic(10)); + } + } else if (propertyType == int.class) { + PropertyUtils.setProperty(cloud, property, RandomUtils.nextInt()); + } else if (propertyType == Integer.class) { + PropertyUtils.setProperty(cloud, property, Integer.valueOf(RandomUtils.nextInt())); + } else if (propertyType == boolean.class) { + PropertyUtils.setProperty(cloud, property, RandomUtils.nextBoolean()); + } else if (!objectProperties.contains(property)) { + fail("Unhandled field in copy constructor: " + property); + } + } + } cloud.setTemplates(Collections.singletonList(pt)); - cloud.setServerUrl("serverUrl"); - cloud.setSkipTlsVerify(true); - cloud.setAddMasterProxyEnvVars(true); - cloud.setNamespace("namespace"); - cloud.setJenkinsUrl("jenkinsUrl"); - cloud.setJenkinsTunnel("tunnel"); - cloud.setCredentialsId("abcd"); - cloud.setContainerCapStr("100"); - cloud.setRetentionTimeout(1000); - cloud.setConnectTimeout(123); - cloud.setUsageRestricted(true); - cloud.setMaxRequestsPerHostStr("42"); cloud.setPodRetention(new Always()); - cloud.setWaitForPodSec(245); cloud.setPodLabels(PodLabel.listOf("foo", "bar", "cat", "dog")); + cloud.setLabels(ImmutableMap.of("foo", "bar")); KubernetesCloud copy = new KubernetesCloud("copy", cloud);