Merge branch 'stable-1.21'

jenkinsci · Dec 20, 2019 · 4cf175e · 4cf175e
2 parents 8ca1719 + e99a00c
commit 4cf175e
Show file tree

Hide file tree

Showing 5 changed files with 103 additions and 54 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
@@ -3,6 +3,7 @@
 import static org.apache.commons.lang.StringUtils.isEmpty;
 
 import java.io.IOException;
+import java.io.StringReader;
 import java.net.ConnectException;
 import java.net.MalformedURLException;
 import java.net.SocketTimeoutException;
@@ -28,6 +29,7 @@
 import javax.annotation.Nonnull;
 import javax.servlet.ServletException;
 
+import hudson.util.XStream2;
 import io.fabric8.openshift.client.OpenShiftClient;
 import org.apache.commons.codec.binary.Base64;
 import org.apache.commons.lang.StringUtils;
@@ -96,6 +98,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 +124,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<String, String> labels;
@@ -149,24 +155,11 @@ public KubernetesCloud(String name) {
      */
     public KubernetesCloud(@NonNull String name, @NonNull KubernetesCloud source) {
         super(name);
-        this.defaultsProviderTemplate = source.defaultsProviderTemplate;
+        XStream2 xs = new XStream2();
+        xs.omitField(Cloud.class, "name");
+        xs.omitField(KubernetesCloud.class, "templates"); // TODO PodTemplate and fields needs to implement equals
+        xs.unmarshal(XStream2.getDefaultDriver().createReader(new StringReader(xs.toXML(source))), this);
         this.templates.addAll(source.templates);
-        this.serverUrl = source.serverUrl;
-        this.skipTlsVerify = source.skipTlsVerify;
-        this.addMasterProxyEnvVars = source.addMasterProxyEnvVars;
-        this.namespace = source.namespace;
-        this.directConnection = source.directConnection;
-        this.jenkinsUrl = source.jenkinsUrl;
-        this.jenkinsTunnel = source.jenkinsTunnel;
-        this.credentialsId = source.credentialsId;
-        this.containerCap = source.containerCap;
-        this.retentionTimeout = source.retentionTimeout;
-        this.connectTimeout = source.connectTimeout;
-        this.usageRestricted = source.usageRestricted;
-        this.maxRequestsPerHost = source.maxRequestsPerHost;
-        this.podRetention = source.podRetention;
-        this.waitForPodSec = source.waitForPodSec;
-        setPodLabels(source.podLabels);
     }
 
     @Deprecated
@@ -412,7 +405,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 +479,7 @@ public int getMaxRequestsPerHost() {
 
     @DataBoundSetter
     public void setConnectTimeout(int connectTimeout) {
-        this.connectTimeout = connectTimeout;
+        this.connectTimeout = Math.max(DEFAULT_CONNECT_TIMEOUT_SECONDS, connectTimeout);
     }
 
     /**
@@ -761,6 +754,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 +784,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,15 +808,30 @@ 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);
-                return FormValidation.ok();
-            } catch (NumberFormatException e) {
-                return FormValidation.error("Please supply an integer");
-            }
+            return FormValidation.validatePositiveInteger(value);
+        }
+
+        @RequirePOST
+        @SuppressWarnings("unused") // used by jelly
+        public FormValidation doCheckConnectTimeout(@QueryParameter String value) {
+            return FormValidation.validateIntegerInRange(value, DEFAULT_CONNECT_TIMEOUT_SECONDS, Integer.MAX_VALUE);
+        }
+
+        @RequirePOST
+        @SuppressWarnings("unused") // used by jelly
+        public FormValidation doCheckReadTimeout(@QueryParameter String value) {
+            return FormValidation.validateIntegerInRange(value, DEFAULT_READ_TIMEOUT_SECONDS, Integer.MAX_VALUE);
         }
 
+        @RequirePOST
+        @SuppressWarnings("unused") // used by jelly
+        public FormValidation doCheckRetentionTimeout(@QueryParameter String value) {
+            return FormValidation.validateIntegerInRange(value, DEFAULT_RETENTION_TIMEOUT_MINUTES, Integer.MAX_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 +855,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 +865,7 @@ public FormValidation doCheckJenkinsUrl(@QueryParameter String value, @QueryPara
             return FormValidation.ok();
         }
 
+        @SuppressWarnings("unused") // used by jelly
         public List<Descriptor<PodRetention>> getAllowedPodRetentions() {
             Jenkins jenkins = Jenkins.getInstanceOrNull();
             if (jenkins == null) {
@@ -862,7 +874,7 @@ public List<Descriptor<PodRetention>> 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 +883,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 +944,8 @@ private Object readResolve() {
         if (podRetention == null) {
             podRetention = PodRetention.getKubernetesCloudDefault();
         }
+        setConnectTimeout(connectTimeout);
+        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
@@ -40,15 +40,15 @@
     </f:entry>
 
     <f:entry title="${%Connection Timeout (seconds)}" field="connectTimeout">
-        <f:textbox default="5"/>
+        <f:number min="${descriptor.defaultConnectTimeout}" default="${descriptor.defaultConnectTimeout}" checkMethod="post"/>
     </f:entry>
 
     <f:entry title="${%Read Timeout (seconds)}" field="readTimeout">
-        <f:textbox default="15"/>
+        <f:number min="${descriptor.defaultReadTimeout}" default="${descriptor.defaultReadTimeout}" checkMethod="post"/>
     </f:entry>
 
     <f:entry title="${%Concurrency Limit}" field="containerCapStr">
-        <f:textbox default="10"/>
+        <f:number default="10"/>
     </f:entry>
 
     <f:entry title="${%Pod Labels}" field="podLabels">
@@ -60,16 +60,16 @@
         descriptors="${descriptor.allowedPodRetentions}" default="${descriptor.defaultPodRetention}" /> 
 
     <f:entry title="${%Max connections to Kubernetes API}" field="maxRequestsPerHostStr">
-        <f:textbox default="32" checkMethod="post"/>
+        <f:number default="32" checkMethod="post"/>
     </f:entry>
 
     <f:entry title="Seconds to wait for pod to be running" field="waitForPodSec">
-        <f:number clazz="required number" min="0" step="1" default="600"/>
+        <f:number clazz="required number" min="0" step="1" default="${descriptor.defaultWaitForPod}"/>
     </f:entry>
 
     <f:advanced>
       <f:entry title="${%Container Cleanup Timeout (minutes)}" field="retentionTimeout">
-        <f:textbox default="5"/>
+        <f:number min="${descriptor.defaultRetentionTimeout}" default="${descriptor.defaultRetentionTimeout}" checkMethod="post"/>
       </f:entry>
 
       <f:entry title="${%Transfer proxy related environment variables from master to agent}" field="addMasterProxyEnvVars">

diff --git a/...esources/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud/help-connectTimeout.html b/...esources/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud/help-connectTimeout.html
@@ -1,3 +1,3 @@
 <div>
-    The connection timeout in seconds for connections to Kubernetes API. A value of 0 means no timeout.
-</div>
+    The connection timeout in seconds for connections to Kubernetes API. Minimum value is 5.
+</div>
diff --git a/...n/resources/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud/help-readTimeout.html b/...n/resources/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud/help-readTimeout.html
@@ -1,3 +1,3 @@
 <div>
-    The read timeout in seconds for connections to Kubernetes API. A value of 0 means no timeout.
-</div>
+    The read timeout in seconds for connections to Kubernetes API. Minimum value is 15.
+</div>
diff --git 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,31 +279,42 @@ 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<String> objectProperties = Lists.newArrayList("templates", "podRetention", "podLabels", "labels", "serverCertificate");
+        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.setServerCertificate("-----BEGIN CERTIFICATE-----");
         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);
-
+        assertEquals("copy", copy.name);
         assertEquals("Expected cloud from copy constructor to be equal to the source except for name", cloud, copy);
     }