Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JENKINS-60551] Copy constructor test should check all fields #668

Merged
merged 2 commits into from
Dec 20, 2019
Merged

[JENKINS-60551] Copy constructor test should check all fields #668

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
368ec74
[JENKINS-60551] Copy constructor test should check all fields
Dohbedoh Dec 20, 2019
0a4ed30
Reviews from Jesse
Vlatombe Dec 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 39 additions & 5 deletions src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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<String, String> labels;
Expand Down Expand Up @@ -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;
Vlatombe marked this conversation as resolved.
Show resolved Hide resolved
this.skipTlsVerify = source.skipTlsVerify;
this.addMasterProxyEnvVars = source.addMasterProxyEnvVars;
this.namespace = source.namespace;
Expand All @@ -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);
}

Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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);
Vlatombe marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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() //
Expand All @@ -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);
Expand All @@ -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(
Expand All @@ -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);
Expand All @@ -854,6 +866,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) {
Expand All @@ -862,7 +875,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) {
Expand All @@ -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
Expand Down Expand Up @@ -913,6 +945,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;
Expand Down
8 changes: 4 additions & 4 deletions src/main/resources/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud/config.jelly
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,11 @@
</f:entry>

<f:entry title="${%Connection Timeout (seconds)}" field="connectTimeout">
<f:textbox default="5"/>
<f:textbox default="${descriptor.defaultConnectTimeout}"/>
</f:entry>

<f:entry title="${%Read Timeout (seconds)}" field="readTimeout">
<f:textbox default="15"/>
<f:textbox default="${descriptor.defaultReadTimeout}"/>
</f:entry>

<f:entry title="${%Concurrency Limit}" field="containerCapStr">
Expand All @@ -64,12 +64,12 @@
</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:textbox default="${descriptor.defaultRetentionTimeout}"/>
Vlatombe marked this conversation as resolved.
Show resolved Hide resolved
</f:entry>

<f:entry title="${%Transfer proxy related environment variables from master to agent}" field="addMasterProxyEnvVars">
Expand Down
45 changes: 30 additions & 15 deletions src/test/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloudTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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<String> objectProperties = Lists.newArrayList("templates", "podRetention", "podLabels", "labels");
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This hunk should prevent any further unhandled field in the future.

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);

Expand Down