From ca1ccaddba538af9e25161448cd4c6a866dc9e03 Mon Sep 17 00:00:00 2001 From: Antoine Neveux Date: Thu, 22 Jun 2023 15:21:15 +0200 Subject: [PATCH 1/4] Add more spotbugs checks and fix existing issues --- pom.xml | 2 ++ .../plugins/kubernetes/ContainerEnvVar.java | 2 ++ .../kubernetes/ContainerLivenessProbe.java | 3 ++ .../plugins/kubernetes/KubernetesCloud.java | 3 ++ .../kubernetes/KubernetesComputer.java | 30 +++++++++++++++---- .../kubernetes/KubernetesFolderProperty.java | 7 +++++ .../kubernetes/KubernetesLauncher.java | 3 +- .../plugins/kubernetes/KubernetesSlave.java | 17 ++++++++--- .../plugins/kubernetes/MetricNames.java | 4 ++- .../OpenShiftTokenCredentialImpl.java | 3 ++ .../plugins/kubernetes/PodTemplate.java | 13 +++++++- .../kubernetes/PodTemplateToolLocation.java | 3 ++ .../plugins/kubernetes/PodVolumes.java | 14 +++++++++ .../plugins/kubernetes/PortMapping.java | 3 ++ .../kubernetes/ServiceAccountCredential.java | 3 +- .../pipeline/ContainerExecDecorator.java | 2 ++ .../pipeline/ContainerLogStepExecution.java | 4 +++ .../pipeline/ContainerStepExecution.java | 4 ++- .../KubernetesAgentErrorCondition.java | 3 ++ .../pipeline/KubernetesDeclarativeAgent.java | 5 +++- .../kubernetes/pipeline/NamespaceAction.java | 3 +- .../pipeline/PodTemplateAction.java | 3 +- .../pipeline/PodTemplateStepExecution.java | 3 +- .../kubernetes/pod/retention/OnFailure.java | 4 ++- .../kubernetes/pod/retention/Reaper.java | 8 ++++- .../kubernetes/volumes/ConfigMapVolume.java | 3 ++ .../kubernetes/volumes/DynamicPVCVolume.java | 3 ++ .../kubernetes/volumes/EmptyDirVolume.java | 3 ++ .../kubernetes/volumes/HostPathVolume.java | 3 ++ .../plugins/kubernetes/volumes/NfsVolume.java | 3 ++ .../volumes/PersistentVolumeClaim.java | 3 ++ .../kubernetes/volumes/SecretVolume.java | 3 ++ .../workspace/DynamicPVCWorkspaceVolume.java | 3 ++ .../workspace/EmptyDirWorkspaceVolume.java | 3 ++ .../workspace/HostPathWorkspaceVolume.java | 3 ++ .../volumes/workspace/NfsWorkspaceVolume.java | 3 ++ .../PersistentVolumeClaimWorkspaceVolume.java | 3 ++ 37 files changed, 161 insertions(+), 24 deletions(-) diff --git a/pom.xml b/pom.xml index 0308e22902..579d836231 100644 --- a/pom.xml +++ b/pom.xml @@ -49,6 +49,8 @@ false true jenkinsci/${project.artifactId}-plugin + Max + Low diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/ContainerEnvVar.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/ContainerEnvVar.java index f7c05919e3..472e15484c 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/ContainerEnvVar.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/ContainerEnvVar.java @@ -16,6 +16,8 @@ @Deprecated public class ContainerEnvVar extends KeyValueEnvVar { + private static final long serialVersionUID = 42L; + @DataBoundConstructor public ContainerEnvVar(String key, String value) { super(key, value); diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/ContainerLivenessProbe.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/ContainerLivenessProbe.java index 172126d7f6..39070d7e1b 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/ContainerLivenessProbe.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/ContainerLivenessProbe.java @@ -12,6 +12,9 @@ * Created by fabricio.leotti on 26/04/17. */ public class ContainerLivenessProbe extends AbstractDescribableImpl implements Serializable { + + private static final long serialVersionUID = 42L; + private String execArgs; private int timeoutSeconds; private int initialDelaySeconds; 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 3dbbae26e7..750765ea42 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java @@ -151,6 +151,9 @@ public KubernetesCloud(String name) { * @param source Source Kubernetes cloud implementation * @since 0.13 */ + @SuppressFBWarnings(value = "MC_OVERRIDABLE_METHOD_CALL_IN_CONSTRUCTOR", justification = "Problem raised for calling unmarshal. Ignoring the " + + "warning cause it leads to too many changes, with " + + "unclear impact.") public KubernetesCloud(@NonNull String name, @NonNull KubernetesCloud source) { super(name); XStream2 xs = new XStream2(); diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesComputer.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesComputer.java index e96c181910..b7882e8c49 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesComputer.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesComputer.java @@ -31,6 +31,8 @@ import java.util.logging.Level; import java.util.logging.Logger; +import edu.umd.cs.findbugs.annotations.NonNull; + /** * @author Carlos Sanchez carlos@apache.org */ @@ -151,14 +153,30 @@ public String toString() { } @Override + @NonNull public ACL getACL() { final ACL base = super.getACL(); - return new ACL() { - @Override - public boolean hasPermission(Authentication a, Permission permission) { - return permission == Computer.CONFIGURE ? false : base.hasPermission(a,permission); - } - }; + return new KubernetesComputerACL(base); + } + + /** + * Simple static inner class to be used by {@link #getACL()}. + * It replaces an anonymous inner class in order to fix + * SIC_INNER_SHOULD_BE_STATIC_ANON. + */ + private static final class KubernetesComputerACL extends ACL { + + private final ACL base; + + public KubernetesComputerACL(final ACL base) { + this.base = base; + } + + @Override + public boolean hasPermission(Authentication a, Permission permission) { + return permission == Computer.CONFIGURE ? false : base.hasPermission(a,permission); + } + } public void setLaunching(boolean launching) { diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesFolderProperty.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesFolderProperty.java index a0f381adc7..addff11b6a 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesFolderProperty.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesFolderProperty.java @@ -10,6 +10,8 @@ import java.util.stream.Collectors; import edu.umd.cs.findbugs.annotations.NonNull; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + import hudson.model.Job; import org.jenkinsci.Symbol; @@ -150,6 +152,7 @@ public UsagePermission(String name, boolean granted, boolean inherited) { this.inherited = inherited; } + @SuppressFBWarnings("UPM_UNCALLED_PRIVATE_METHOD") private void setInherited(boolean inherited) { this.inherited = inherited; } @@ -158,6 +161,7 @@ public boolean isInherited() { return inherited; } + @SuppressFBWarnings("UPM_UNCALLED_PRIVATE_METHOD") private void setGranted(boolean granted) { this.granted = granted; } @@ -167,6 +171,7 @@ public boolean isGranted() { return granted; } + @SuppressFBWarnings("UPM_UNCALLED_PRIVATE_METHOD") private void setName(String name) { this.name = name; } @@ -191,6 +196,8 @@ private static boolean userHasAdministerPermission() { return Jenkins.get().hasPermission(Jenkins.ADMINISTER); } + + @SuppressFBWarnings(value = "UPM_UNCALLED_PRIVATE_METHOD") private static boolean isUsageRestrictedKubernetesCloud(Cloud cloud) { if (cloud instanceof KubernetesCloud) { return ((KubernetesCloud) cloud).isUsageRestricted(); diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java index d8e6b1e529..14215ec0df 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java @@ -86,6 +86,7 @@ public KubernetesLauncher() { } @Override + @SuppressFBWarnings(value = "IS2_INCONSISTENT_SYNC", justification = "Only this accessor is called without synchronized.") public boolean isLaunchSupported() { return !launched; } @@ -112,7 +113,7 @@ public synchronized void launch(SlaveComputer computer, TaskListener listener) { String cloudName = node.getCloudName(); final PodTemplate template = node.getTemplate(); - TaskListener runListener = TaskListener.NULL; + TaskListener runListener; try { KubernetesCloud cloud = node.getKubernetesCloud(); KubernetesClient client = cloud.connect(); diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesSlave.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesSlave.java index 90a5d12369..6f4b253bce 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesSlave.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesSlave.java @@ -1,10 +1,9 @@ package org.csanchez.jenkins.plugins.kubernetes; import edu.umd.cs.findbugs.annotations.NonNull; -import io.fabric8.kubernetes.api.model.StatusDetails; import java.io.IOException; import java.util.HashSet; -import java.util.List; +import java.util.Locale; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -16,7 +15,8 @@ import java.util.logging.Logger; import edu.umd.cs.findbugs.annotations.CheckForNull; -import edu.umd.cs.findbugs.annotations.NonNull; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + import hudson.FilePath; import hudson.Util; import hudson.slaves.SlaveComputer; @@ -182,6 +182,7 @@ public String getPodName() { private String remoteFS; + @SuppressFBWarnings(value = "NM_CONFUSING", justification = "Naming confusion with a getRemoteFs method, but the latter is deprecated.") @Override public String getRemoteFS() { if (remoteFS == null) { @@ -249,7 +250,7 @@ static String getSlaveName(PodTemplate template) { return String.format("%s-%s", DEFAULT_AGENT_PREFIX, randString); } // no spaces - name = name.replaceAll("[ _]", "-").toLowerCase(); + name = name.replaceAll("[ _]", "-").toLowerCase(Locale.getDefault()); // keep it under 63 chars (62 is used to account for the '-') name = name.substring(0, Math.min(name.length(), 62 - randString.length())); String slaveName = String.format("%s-%s", name, randString); @@ -536,8 +537,11 @@ public Builder retentionStrategy(RetentionStrategy retentionStrategy) { return this; } + @SuppressFBWarnings(value = "UWF_FIELD_NOT_INITIALIZED_IN_CONSTRUCTOR", justification = "Warning is for the cloud field, and a null " + + "validation check is done before using it.") private RetentionStrategy determineRetentionStrategy() { if (podTemplate.getIdleMinutes() == 0) { + Validate.notNull(cloud); return new OnceRetentionStrategy(cloud.getRetentionTimeout()); } else { return new CloudRetentionStrategy(podTemplate.getIdleMinutes()); @@ -550,6 +554,8 @@ private RetentionStrategy determineRetentionStrategy() { * @throws IOException * @throws Descriptor.FormException */ + @SuppressFBWarnings(value = "UWF_FIELD_NOT_INITIALIZED_IN_CONSTRUCTOR", justification = "Warning is for the cloud field, and a null " + + "validation check is done before using it.") public KubernetesSlave build() throws IOException, Descriptor.FormException { Validate.notNull(podTemplate); Validate.notNull(cloud); @@ -563,8 +569,11 @@ public KubernetesSlave build() throws IOException, Descriptor.FormException { retentionStrategy == null ? determineRetentionStrategy() : retentionStrategy); } + @SuppressFBWarnings(value = "UWF_FIELD_NOT_INITIALIZED_IN_CONSTRUCTOR", justification = "Warning is for the cloud field, and a null " + + "validation check is done before using it.") private ComputerLauncher decorateLauncher(@NonNull ComputerLauncher launcher) { if (launcher instanceof KubernetesLauncher) { + Validate.notNull(cloud); ((KubernetesLauncher) launcher).setWebSocket(cloud.isWebSocket()); } return launcher; diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/MetricNames.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/MetricNames.java index 5d3c6d7262..5344e638d9 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/MetricNames.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/MetricNames.java @@ -1,5 +1,7 @@ package org.csanchez.jenkins.plugins.kubernetes; +import java.util.Locale; + import hudson.model.Label; public class MetricNames { @@ -17,7 +19,7 @@ public class MetricNames { public static final String PODS_LAUNCHED = PREFIX + ".pods.launched"; public static String metricNameForPodStatus(String status) { - String formattedStatus = status == null ? "null" : status.toLowerCase(); + String formattedStatus = status == null ? "null" : status.toLowerCase(Locale.getDefault()); return PREFIX + ".pods.launch.status." + formattedStatus; } diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/OpenShiftTokenCredentialImpl.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/OpenShiftTokenCredentialImpl.java index f2c419f232..30e7165e6f 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/OpenShiftTokenCredentialImpl.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/OpenShiftTokenCredentialImpl.java @@ -17,6 +17,9 @@ @Deprecated public class OpenShiftTokenCredentialImpl extends BaseStandardCredentials implements TokenProducer { + + private static final long serialVersionUID = 42L; + private final Secret secret; @DataBoundConstructor 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 2c37c9d245..c386dd854e 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplate.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplate.java @@ -18,6 +18,8 @@ import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + import hudson.model.AbstractDescribableImpl; import hudson.model.Descriptor; import hudson.model.DescriptorVisibilityFilter; @@ -25,6 +27,7 @@ import hudson.model.Node; import hudson.model.Saveable; import hudson.model.TaskListener; + import org.apache.commons.lang.StringUtils; import org.csanchez.jenkins.plugins.kubernetes.model.TemplateEnvVar; import org.csanchez.jenkins.plugins.kubernetes.pod.retention.PodRetention; @@ -223,6 +226,9 @@ public PodTemplate(@CheckForNull String id) { recomputeLabelDerivedFields(); } + @SuppressFBWarnings(value = "MC_OVERRIDABLE_METHOD_CALL_IN_CONSTRUCTOR", justification = "Warning raised for the call to unmarshal. Ignoring as" + + " it would require too many changes and uncertain " + + "impact.") public PodTemplate(PodTemplate from) { XStream2 xs = new XStream2(); xs.unmarshal(XStream2.getDefaultDriver().createReader(new StringReader(xs.toXML(from))), this); @@ -236,6 +242,9 @@ public PodTemplate(String image, List volumes) { this(null, image, volumes); } + @SuppressFBWarnings(value = "MC_OVERRIDABLE_METHOD_CALL_IN_CONSTRUCTOR", justification = + "Warning raised for calling the getContainers method. As " + "this current method is deprecated anyway, it is " + + "probably safer to not change it.") @Deprecated PodTemplate(String name, String image, List volumes) { this(name, volumes, Collections.emptyList()); @@ -244,7 +253,8 @@ public PodTemplate(String image, List volumes) { } } - @Restricted(NoExternalUse.class) // testing only + @Restricted(NoExternalUse.class) + // testing only PodTemplate(String name, List volumes, List containers) { this(); this.name = name; @@ -322,6 +332,7 @@ public void setRemoteFs(String remoteFs) { } @Deprecated + @SuppressFBWarnings(value = "NM_CONFUSING", justification = "Naming confusion with a getRemoteFS method, but the current one is deprecated.") public String getRemoteFs() { return getFirstContainer().map(ContainerTemplate::getWorkingDir).orElse(ContainerTemplate.DEFAULT_WORKING_DIR); } diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateToolLocation.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateToolLocation.java index 3227d22d60..0d4be2f434 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateToolLocation.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateToolLocation.java @@ -34,6 +34,9 @@ */ public class PodTemplateToolLocation extends DescribableList,NodePropertyDescriptor> implements Serializable { + + private static final long serialVersionUID = 42L; + public PodTemplateToolLocation() {} diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodVolumes.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodVolumes.java index 2040443c10..722842c45d 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodVolumes.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodVolumes.java @@ -14,6 +14,9 @@ public static abstract class PodVolume extends org.csanchez.jenkins.plugins.kube @Deprecated public static class EmptyDirVolume extends org.csanchez.jenkins.plugins.kubernetes.volumes.EmptyDirVolume { + + private static final long serialVersionUID = 42L; + public EmptyDirVolume(String mountPath, Boolean memory) { super(mountPath, memory); } @@ -27,6 +30,9 @@ protected Object readResolve() { @Deprecated public static class SecretVolume extends org.csanchez.jenkins.plugins.kubernetes.volumes.SecretVolume { + + private static final long serialVersionUID = 42L; + public SecretVolume(String mountPath, String secretName) { super(mountPath, secretName); } @@ -40,6 +46,9 @@ protected Object readResolve() { @Deprecated public static class HostPathVolume extends org.csanchez.jenkins.plugins.kubernetes.volumes.HostPathVolume { + + private static final long serialVersionUID = 42L; + public HostPathVolume(String hostPath, String mountPath, Boolean readOnly) { super(hostPath, mountPath, readOnly); } @@ -53,6 +62,9 @@ protected Object readResolve() { @Deprecated public static class NfsVolume extends org.csanchez.jenkins.plugins.kubernetes.volumes.NfsVolume { + + private static final long serialVersionUID = 42L; + public NfsVolume(String serverAddress, String serverPath, Boolean readOnly, String mountPath) { super(serverAddress, serverPath, readOnly, mountPath); } @@ -66,6 +78,7 @@ protected Object readResolve() { /** * @deprecated Use {@link PodVolume#volumeMountExists(String, List)} instead */ + @Deprecated public static boolean volumeMountExists(String path, List existingMounts) { return PodVolume.volumeMountExists(path, existingMounts); } @@ -73,6 +86,7 @@ public static boolean volumeMountExists(String path, List existingM /** * @deprecated Use {@link PodVolume#podVolumeExists(String,List)} instead */ + @Deprecated public static boolean podVolumeExists(String path, List existingVolumes) { for (PodVolume podVolume : existingVolumes) { if (podVolume.getMountPath().equals(path)) { diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PortMapping.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PortMapping.java index 9912668451..bb2e3ac189 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PortMapping.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PortMapping.java @@ -13,6 +13,9 @@ public class PortMapping extends AbstractDescribableImpl implements Serializable { + + private static final long serialVersionUID = 42L; + private String name; private Integer containerPort; private Integer hostPort; diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/ServiceAccountCredential.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/ServiceAccountCredential.java index 167cf1c439..f2883d12a3 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/ServiceAccountCredential.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/ServiceAccountCredential.java @@ -1,7 +1,6 @@ package org.csanchez.jenkins.plugins.kubernetes; import org.jenkinsci.plugins.kubernetes.credentials.FileSystemServiceAccountCredential; -import org.jenkinsci.plugins.kubernetes.credentials.TokenProducer; import org.kohsuke.stapler.DataBoundConstructor; import com.cloudbees.plugins.credentials.CredentialsScope; @@ -14,7 +13,7 @@ * @author Nicolas De Loof */ @Deprecated -public class ServiceAccountCredential extends FileSystemServiceAccountCredential implements TokenProducer { +public class ServiceAccountCredential extends FileSystemServiceAccountCredential { private static final long serialVersionUID = 2739355565227800401L; diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerExecDecorator.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerExecDecorator.java index cc7112dd92..5e2c5494a3 100755 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerExecDecorator.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerExecDecorator.java @@ -336,6 +336,8 @@ public Proc launch(ProcStarter starter) throws IOException { getCommands(starter, containerWorkingDirFilePathStr, launcher.isUnix())); } + @SuppressFBWarnings(value = "REC_CATCH_EXCEPTION", justification = "Warning raised for catching the Exception e at the end of the " + + "method. Not sure about the possible impacts of changing that bit.") private Proc doLaunch(boolean quiet, String[] cmdEnvs, OutputStream outputForCaller, FilePath pwd, boolean[] masks, String... commands) throws IOException { long startMethod = System.nanoTime(); diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerLogStepExecution.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerLogStepExecution.java index ccff68cf3c..67a84280fb 100755 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerLogStepExecution.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerLogStepExecution.java @@ -29,6 +29,8 @@ import java.util.logging.Level; import java.util.logging.Logger; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + public class ContainerLogStepExecution extends SynchronousNonBlockingStepExecution { private static final long serialVersionUID = 5588861066775717487L; private static final transient Logger LOGGER = Logger.getLogger(ContainerLogStepExecution.class.getName()); @@ -41,6 +43,8 @@ public class ContainerLogStepExecution extends SynchronousNonBlockingStepExecuti this.step = step; } + @SuppressFBWarnings(value = "REC_CATCH_EXCEPTION", justification = "Warning raised for catching the Exception x, not entirely sure about the " + + "possible effects of removing that bit.") private PrintStream logger() { TaskListener l = null; StepContext context = getContext(); diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerStepExecution.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerStepExecution.java index c4ee21b1ed..493ec43cc0 100755 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerStepExecution.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerStepExecution.java @@ -30,7 +30,7 @@ public class ContainerStepExecution extends StepExecution { private static final long serialVersionUID = 7634132798345235774L; - private static final transient Logger LOGGER = Logger.getLogger(ContainerStepExecution.class.getName()); + private static final Logger LOGGER = Logger.getLogger(ContainerStepExecution.class.getName()); @SuppressFBWarnings(value = "SE_TRANSIENT_FIELD_NOT_RESTORED", justification = "not needed on deserialization") private final transient ContainerStep step; @@ -95,6 +95,8 @@ public void stop(@NonNull Throwable cause) throws Exception { closeQuietly(getContext(), decorator); } + @SuppressFBWarnings(value = "SE_BAD_FIELD", justification = "Warning about closeables not being non-transient non-serializable, not entirely " + + "sure what is the desired behavior and what impact could a change have here.") private static class ContainerExecCallback extends BodyExecutionCallback.TailCall { private static final long serialVersionUID = 6385838254761750483L; diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesAgentErrorCondition.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesAgentErrorCondition.java index 5acc1d4c3c..b27e521c83 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesAgentErrorCondition.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesAgentErrorCondition.java @@ -49,6 +49,9 @@ */ public class KubernetesAgentErrorCondition extends ErrorCondition { + + private static final long serialVersionUID = 42L; + private static final Logger LOGGER = Logger.getLogger(KubernetesAgentErrorCondition.class.getName()); private static final Set IGNORED_CONTAINER_TERMINATION_REASONS = new HashSet<>(); diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesDeclarativeAgent.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesDeclarativeAgent.java index 103962e3ea..bf41f37d61 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesDeclarativeAgent.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesDeclarativeAgent.java @@ -33,6 +33,9 @@ public class KubernetesDeclarativeAgent extends RetryableDeclarativeAgent { + + private static final long serialVersionUID = 42L; + private static final Logger LOGGER = Logger.getLogger(KubernetesDeclarativeAgent.class.getName()); @CheckForNull @@ -328,7 +331,7 @@ public Map getAsArgs() { "Ignoring containerTemplate option as containerTemplates is also defined"); } } - if (containerTemplates != null && !containerTemplates.isEmpty()) { + if (!containerTemplates.isEmpty()) { argMap.put("containers", containerTemplates); } diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/NamespaceAction.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/NamespaceAction.java index 43e9ef6b09..3efa952046 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/NamespaceAction.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/NamespaceAction.java @@ -8,13 +8,12 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import hudson.BulkChange; import hudson.model.Run; -import jenkins.model.RunAction2; /** * Use 
getContext().get(PodTemplateContext.class)
instead. */ @Deprecated -public class NamespaceAction extends AbstractInvisibleRunAction2 implements RunAction2 { +public class NamespaceAction extends AbstractInvisibleRunAction2 { private static final Logger LOGGER = Logger.getLogger(NamespaceAction.class.getName()); diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateAction.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateAction.java index b8eede2d0a..062c8cd641 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateAction.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateAction.java @@ -9,13 +9,12 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import hudson.BulkChange; import hudson.model.Run; -import jenkins.model.RunAction2; /** * Use 
getContext().get(PodTemplateContext.class)
instead. */ @Deprecated -public class PodTemplateAction extends AbstractInvisibleRunAction2 implements RunAction2 { +public class PodTemplateAction extends AbstractInvisibleRunAction2 { private static final Logger LOGGER = Logger.getLogger(PodTemplateAction.class.getName()); 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 22a8b81ee6..a7a8671561 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 @@ -43,7 +43,7 @@ public class PodTemplateStepExecution extends AbstractStepExecutionImpl { private static final long serialVersionUID = -6139090518333729333L; - private static final transient String NAME_FORMAT = "%s-%s"; + private static final String NAME_FORMAT = "%s-%s"; private static /* almost final */ boolean VERBOSE = Boolean.parseBoolean(System.getProperty(PodTemplateStepExecution.class.getName() + ".verbose")); @@ -245,6 +245,7 @@ public void onResume() { } } + @SuppressFBWarnings(value = "SE_INNER_CLASS", justification = "Not sure if it is intended or if this inner class should be static.") private class PodTemplateCallback extends BodyExecutionCallback.TailCall { private static final long serialVersionUID = 6043919968776851324L; diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/retention/OnFailure.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/retention/OnFailure.java index 594b572f01..8f3d8c090c 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/retention/OnFailure.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/retention/OnFailure.java @@ -8,6 +8,8 @@ import hudson.Extension; import io.fabric8.kubernetes.api.model.Pod; + +import java.util.Locale; import java.util.function.Supplier; import java.util.logging.Level; import java.util.logging.Logger; @@ -34,7 +36,7 @@ public boolean shouldDeletePod(KubernetesCloud cloud, Supplier podS) { if (pod == null || pod.getStatus() == null) { return false; } - boolean hasErrors = pod.getStatus().getPhase().toLowerCase().matches("(failed|unknown)"); + boolean hasErrors = pod.getStatus().getPhase().toLowerCase(Locale.getDefault()).matches("(failed|unknown)"); return !hasErrors; } diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/retention/Reaper.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/retention/Reaper.java index 33bae4575b..8926cbf086 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/retention/Reaper.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/retention/Reaper.java @@ -44,6 +44,7 @@ import io.fabric8.kubernetes.client.WatcherException; import java.io.IOException; import java.util.ArrayList; +import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -146,6 +147,8 @@ private void activate() { /** * Remove any {@link KubernetesSlave} nodes that reference Pods that don't exist. */ + @SuppressFBWarnings(value = "REC_CATCH_EXCEPTION", justification = "Warning about the Exception x catch at the end. Not sure about the possible" + + " impact of removing it.") private void reapAgents() { Jenkins jenkins = Jenkins.getInstanceOrNull(); if (jenkins != null) { @@ -210,6 +213,8 @@ private void watchClouds() { * is no longer valid. * @param kc kubernetes cloud to watch */ + @SuppressFBWarnings(value = "REC_CATCH_EXCEPTION", justification = "Warning about the Exception x caught at the end, but not sure about the " + + "possible impact of removing it.") private void watchCloud(@NonNull KubernetesCloud kc) { // can't use ConcurrentHashMap#computeIfAbsent because CloudPodWatcher will remove itself from the watchers // map on close. If an error occurs when creating the watch it would create a deadlock situation. @@ -315,7 +320,8 @@ public void eventReceived(Action action, Pod pod) { Listeners.notify(Listener.class, true, listener -> { try { - listener.onEvent(action, optionalNode.get(), pod, terminationReasons.get(optionalNode.get().getNodeName())); + Set terminationReasons = Reaper.this.terminationReasons.get(optionalNode.get().getNodeName()); + listener.onEvent(action, optionalNode.get(), pod, terminationReasons != null ? terminationReasons : Collections.emptySet()); } catch (Exception x) { LOGGER.log(Level.WARNING, "Listener " + listener + " failed for " + ns + "/" + name, x); } diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/ConfigMapVolume.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/ConfigMapVolume.java index 3863895225..c2865cf0f7 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/ConfigMapVolume.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/ConfigMapVolume.java @@ -36,6 +36,9 @@ public class ConfigMapVolume extends PodVolume { + + private static final long serialVersionUID = 42L; + private String mountPath; private String subPath; private String configMapName; diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/DynamicPVCVolume.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/DynamicPVCVolume.java index 48db86a244..ece7540743 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/DynamicPVCVolume.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/DynamicPVCVolume.java @@ -23,6 +23,9 @@ * Implements a dynamic PVC volume, that is created before the agent pod is created, and terminated afterwards. */ public class DynamicPVCVolume extends PodVolume implements DynamicPVC { + + private static final long serialVersionUID = 42L; + private String id; private String storageClassName; private String requestsSize; diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/EmptyDirVolume.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/EmptyDirVolume.java index 86e8e0f2f3..2fbf7af217 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/EmptyDirVolume.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/EmptyDirVolume.java @@ -36,6 +36,9 @@ public class EmptyDirVolume extends PodVolume { + + private static final long serialVersionUID = 42L; + private static final String DEFAULT_MEDIUM = ""; private static final String MEMORY_MEDIUM = "Memory"; diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/HostPathVolume.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/HostPathVolume.java index cd6e39361b..ec97de9b10 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/HostPathVolume.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/HostPathVolume.java @@ -35,6 +35,9 @@ import io.fabric8.kubernetes.api.model.VolumeBuilder; public class HostPathVolume extends PodVolume { + + private static final long serialVersionUID = 42L; + private String mountPath; private String hostPath; diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/NfsVolume.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/NfsVolume.java index 2d6b2488bb..11235eef42 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/NfsVolume.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/NfsVolume.java @@ -35,6 +35,9 @@ import io.fabric8.kubernetes.api.model.VolumeBuilder; public class NfsVolume extends PodVolume { + + private static final long serialVersionUID = 42L; + private String mountPath; private String serverAddress; private String serverPath; diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/PersistentVolumeClaim.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/PersistentVolumeClaim.java index 7c8378e3b2..007932904e 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/PersistentVolumeClaim.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/PersistentVolumeClaim.java @@ -35,6 +35,9 @@ import io.fabric8.kubernetes.api.model.VolumeBuilder; public class PersistentVolumeClaim extends PodVolume { + + private static final long serialVersionUID = 42L; + private String mountPath; private String claimName; @CheckForNull diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/SecretVolume.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/SecretVolume.java index 150eb93f27..691573aa91 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/SecretVolume.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/SecretVolume.java @@ -35,6 +35,9 @@ import io.fabric8.kubernetes.api.model.VolumeBuilder; public class SecretVolume extends PodVolume { + + private static final long serialVersionUID = 42L; + private String mountPath; private String secretName; private String defaultMode; diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/DynamicPVCWorkspaceVolume.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/DynamicPVCWorkspaceVolume.java index e359f9ffbc..fa65ff5b5d 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/DynamicPVCWorkspaceVolume.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/DynamicPVCWorkspaceVolume.java @@ -24,6 +24,9 @@ * @author runzexia */ public class DynamicPVCWorkspaceVolume extends WorkspaceVolume implements DynamicPVC { + + private static final long serialVersionUID = 42L; + private String storageClassName; private String requestsSize; private String accessModes; diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/EmptyDirWorkspaceVolume.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/EmptyDirWorkspaceVolume.java index a7bb544002..05e0d9c34d 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/EmptyDirWorkspaceVolume.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/EmptyDirWorkspaceVolume.java @@ -38,6 +38,9 @@ public class EmptyDirWorkspaceVolume extends WorkspaceVolume { + + private static final long serialVersionUID = 42L; + private static final String DEFAULT_MEDIUM = ""; private static final String MEMORY_MEDIUM = "Memory"; diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/HostPathWorkspaceVolume.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/HostPathWorkspaceVolume.java index 533dfd28b4..b27ec0b25e 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/HostPathWorkspaceVolume.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/HostPathWorkspaceVolume.java @@ -35,6 +35,9 @@ import java.util.Objects; public class HostPathWorkspaceVolume extends WorkspaceVolume { + + private static final long serialVersionUID = 42L; + private String hostPath; @DataBoundConstructor diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/NfsWorkspaceVolume.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/NfsWorkspaceVolume.java index a1a0c09c2e..8598326772 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/NfsWorkspaceVolume.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/NfsWorkspaceVolume.java @@ -37,6 +37,9 @@ import java.util.Objects; public class NfsWorkspaceVolume extends WorkspaceVolume { + + private static final long serialVersionUID = 42L; + private String serverAddress; private String serverPath; @CheckForNull diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/PersistentVolumeClaimWorkspaceVolume.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/PersistentVolumeClaimWorkspaceVolume.java index ce478f5012..3952ef158e 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/PersistentVolumeClaimWorkspaceVolume.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/PersistentVolumeClaimWorkspaceVolume.java @@ -37,6 +37,9 @@ import java.util.Objects; public class PersistentVolumeClaimWorkspaceVolume extends WorkspaceVolume { + + private static final long serialVersionUID = 42L; + private String claimName; @CheckForNull private Boolean readOnly; From f74612f93d345bc8f9c6530d6a6b4527714a916c Mon Sep 17 00:00:00 2001 From: Antoine Neveux Date: Thu, 22 Jun 2023 22:10:48 +0200 Subject: [PATCH 2/4] Applying suggestions from code review --- .../plugins/kubernetes/ContainerEnvVar.java | 4 +-- .../kubernetes/ContainerLivenessProbe.java | 5 ++-- .../kubernetes/KubernetesFolderProperty.java | 15 ---------- .../kubernetes/KubernetesLauncher.java | 3 +- .../plugins/kubernetes/KubernetesSlave.java | 17 ++++------- .../OpenShiftTokenCredentialImpl.java | 6 ++-- .../plugins/kubernetes/PodTemplate.java | 3 +- .../kubernetes/PodTemplateToolLocation.java | 6 ++-- .../plugins/kubernetes/PodVolumes.java | 17 ++++------- .../plugins/kubernetes/PortMapping.java | 5 ++-- .../pipeline/ContainerExecDecorator.java | 5 ++-- .../pipeline/ContainerLogStepExecution.java | 5 ++-- .../pipeline/ContainerStepExecution.java | 10 +++---- .../KubernetesAgentErrorCondition.java | 5 ++-- .../pipeline/KubernetesDeclarativeAgent.java | 6 ++-- .../kubernetes/pod/retention/Reaper.java | 28 ++++++++----------- .../kubernetes/volumes/ConfigMapVolume.java | 6 ++-- .../kubernetes/volumes/DynamicPVCVolume.java | 5 ++-- .../kubernetes/volumes/EmptyDirVolume.java | 5 ++-- .../kubernetes/volumes/HostPathVolume.java | 5 ++-- .../plugins/kubernetes/volumes/NfsVolume.java | 5 ++-- .../volumes/PersistentVolumeClaim.java | 5 ++-- .../kubernetes/volumes/SecretVolume.java | 6 ++-- .../workspace/DynamicPVCWorkspaceVolume.java | 5 ++-- .../workspace/EmptyDirWorkspaceVolume.java | 5 ++-- .../workspace/HostPathWorkspaceVolume.java | 6 ++-- .../volumes/workspace/NfsWorkspaceVolume.java | 5 ++-- .../PersistentVolumeClaimWorkspaceVolume.java | 5 ++-- 28 files changed, 77 insertions(+), 126 deletions(-) diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/ContainerEnvVar.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/ContainerEnvVar.java index 472e15484c..233650b698 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/ContainerEnvVar.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/ContainerEnvVar.java @@ -2,6 +2,7 @@ import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import org.csanchez.jenkins.plugins.kubernetes.model.KeyValueEnvVar; import org.jenkinsci.Symbol; import org.kohsuke.stapler.DataBoundConstructor; @@ -13,11 +14,10 @@ /** * Deprecated, use KeyValueEnvVar */ +@SuppressFBWarnings(value = "SE_NO_SERIALVERSIONID", justification = "Serialization happens exclusively through XStream and not Java Serialization.") @Deprecated public class ContainerEnvVar extends KeyValueEnvVar { - private static final long serialVersionUID = 42L; - @DataBoundConstructor public ContainerEnvVar(String key, String value) { super(key, value); diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/ContainerLivenessProbe.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/ContainerLivenessProbe.java index 39070d7e1b..da545ffe42 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/ContainerLivenessProbe.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/ContainerLivenessProbe.java @@ -8,13 +8,14 @@ import java.io.Serializable; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + /** * Created by fabricio.leotti on 26/04/17. */ +@SuppressFBWarnings(value = "SE_NO_SERIALVERSIONID", justification = "Serialization happens exclusively through XStream and not Java Serialization.") public class ContainerLivenessProbe extends AbstractDescribableImpl implements Serializable { - private static final long serialVersionUID = 42L; - private String execArgs; private int timeoutSeconds; private int initialDelaySeconds; diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesFolderProperty.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesFolderProperty.java index addff11b6a..d1632674c1 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesFolderProperty.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesFolderProperty.java @@ -152,30 +152,15 @@ public UsagePermission(String name, boolean granted, boolean inherited) { this.inherited = inherited; } - @SuppressFBWarnings("UPM_UNCALLED_PRIVATE_METHOD") - private void setInherited(boolean inherited) { - this.inherited = inherited; - } - public boolean isInherited() { return inherited; } - @SuppressFBWarnings("UPM_UNCALLED_PRIVATE_METHOD") - private void setGranted(boolean granted) { - this.granted = granted; - } - @SuppressWarnings("unused") // by stapler/jelly public boolean isGranted() { return granted; } - @SuppressFBWarnings("UPM_UNCALLED_PRIVATE_METHOD") - private void setName(String name) { - this.name = name; - } - /** * Called from Jelly. * diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java index 283842b233..e59ddab1be 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java @@ -87,8 +87,7 @@ public KubernetesLauncher() { } @Override - @SuppressFBWarnings(value = "IS2_INCONSISTENT_SYNC", justification = "Only this accessor is called without synchronized.") - public boolean isLaunchSupported() { + public synchronized boolean isLaunchSupported() { return !launched; } diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesSlave.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesSlave.java index 9ad992c52b..15756469db 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesSlave.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesSlave.java @@ -574,11 +574,8 @@ public Builder retentionStrategy(RetentionStrategy retentionStrategy) { return this; } - @SuppressFBWarnings(value = "UWF_FIELD_NOT_INITIALIZED_IN_CONSTRUCTOR", justification = "Warning is for the cloud field, and a null " - + "validation check is done before using it.") - private RetentionStrategy determineRetentionStrategy() { + private static RetentionStrategy determineRetentionStrategy(@NonNull KubernetesCloud cloud, @NonNull PodTemplate podTemplate) { if (podTemplate.getIdleMinutes() == 0) { - Validate.notNull(cloud); return new OnceRetentionStrategy(cloud.getRetentionTimeout()); } else { return new CloudRetentionStrategy(podTemplate.getIdleMinutes()); @@ -591,8 +588,7 @@ private RetentionStrategy determineRetentionStrategy() { * @throws IOException * @throws Descriptor.FormException */ - @SuppressFBWarnings(value = "UWF_FIELD_NOT_INITIALIZED_IN_CONSTRUCTOR", justification = "Warning is for the cloud field, and a null " - + "validation check is done before using it.") + @SuppressFBWarnings(value = "UWF_FIELD_NOT_INITIALIZED_IN_CONSTRUCTOR", justification = "False positive. https://github.com/spotbugs/spotbugs/issues/567") public KubernetesSlave build() throws IOException, Descriptor.FormException { Validate.notNull(podTemplate); Validate.notNull(cloud); @@ -602,15 +598,12 @@ public KubernetesSlave build() throws IOException, Descriptor.FormException { nodeDescription == null ? podTemplate.getName() : nodeDescription, cloud.name, label == null ? podTemplate.getLabel() : label, - decorateLauncher(computerLauncher == null ? new KubernetesLauncher(cloud.getJenkinsTunnel(), null) : computerLauncher), - retentionStrategy == null ? determineRetentionStrategy() : retentionStrategy); + decorateLauncher(cloud, computerLauncher == null ? new KubernetesLauncher(cloud.getJenkinsTunnel(), null) : computerLauncher), + retentionStrategy == null ? determineRetentionStrategy(cloud, podTemplate) : retentionStrategy); } - @SuppressFBWarnings(value = "UWF_FIELD_NOT_INITIALIZED_IN_CONSTRUCTOR", justification = "Warning is for the cloud field, and a null " - + "validation check is done before using it.") - private ComputerLauncher decorateLauncher(@NonNull ComputerLauncher launcher) { + private ComputerLauncher decorateLauncher(@NonNull KubernetesCloud cloud, @NonNull ComputerLauncher launcher) { if (launcher instanceof KubernetesLauncher) { - Validate.notNull(cloud); ((KubernetesLauncher) launcher).setWebSocket(cloud.isWebSocket()); } return launcher; diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/OpenShiftTokenCredentialImpl.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/OpenShiftTokenCredentialImpl.java index 30e7165e6f..c8f72741ae 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/OpenShiftTokenCredentialImpl.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/OpenShiftTokenCredentialImpl.java @@ -4,6 +4,8 @@ import org.jenkinsci.plugins.plaincredentials.StringCredentials; import org.kohsuke.stapler.DataBoundConstructor; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + import com.cloudbees.plugins.credentials.CredentialsScope; import com.cloudbees.plugins.credentials.impl.BaseStandardCredentials; @@ -14,12 +16,10 @@ * @deprecated Use {@link StringCredentials} * @author Andrew Block */ +@SuppressFBWarnings(value = "SE_NO_SERIALVERSIONID", justification = "Serialization happens exclusively through XStream and not Java Serialization.") @Deprecated public class OpenShiftTokenCredentialImpl extends BaseStandardCredentials implements TokenProducer { - - private static final long serialVersionUID = 42L; - private final Secret secret; @DataBoundConstructor 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 b75d14cb20..a8e915f91f 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplate.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplate.java @@ -253,8 +253,7 @@ public PodTemplate(String image, List volumes) { } } - @Restricted(NoExternalUse.class) - // testing only + @Restricted(NoExternalUse.class) // testing only PodTemplate(String name, List volumes, List containers) { this(); this.name = name; diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateToolLocation.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateToolLocation.java index 0d4be2f434..60e339de6f 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateToolLocation.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateToolLocation.java @@ -24,6 +24,8 @@ import java.io.Serializable; import java.util.Collection; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + /** * Pod Template Tool Location * This class extends Jenkins DescribableList as implemented in Slave Class. Also implements Serializable interface @@ -32,11 +34,9 @@ * * @author Aytunc BEKEN */ +@SuppressFBWarnings(value = "SE_NO_SERIALVERSIONID", justification = "Serialization happens exclusively through XStream and not Java Serialization.") public class PodTemplateToolLocation extends DescribableList,NodePropertyDescriptor> implements Serializable { - - private static final long serialVersionUID = 42L; - public PodTemplateToolLocation() {} diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodVolumes.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodVolumes.java index 722842c45d..0334ee9fb5 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodVolumes.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodVolumes.java @@ -2,6 +2,7 @@ import java.util.List; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import io.fabric8.kubernetes.api.model.VolumeMount; @Deprecated @@ -11,12 +12,10 @@ public class PodVolumes { public static abstract class PodVolume extends org.csanchez.jenkins.plugins.kubernetes.volumes.PodVolume { } + @SuppressFBWarnings(value = "SE_NO_SERIALVERSIONID", justification = "Serialization happens exclusively through XStream and not Java Serialization.") @Deprecated public static class EmptyDirVolume extends org.csanchez.jenkins.plugins.kubernetes.volumes.EmptyDirVolume { - - private static final long serialVersionUID = 42L; - public EmptyDirVolume(String mountPath, Boolean memory) { super(mountPath, memory); } @@ -27,12 +26,10 @@ protected Object readResolve() { } } + @SuppressFBWarnings(value = "SE_NO_SERIALVERSIONID", justification = "Serialization happens exclusively through XStream and not Java Serialization.") @Deprecated public static class SecretVolume extends org.csanchez.jenkins.plugins.kubernetes.volumes.SecretVolume { - - private static final long serialVersionUID = 42L; - public SecretVolume(String mountPath, String secretName) { super(mountPath, secretName); } @@ -43,12 +40,10 @@ protected Object readResolve() { } } + @SuppressFBWarnings(value = "SE_NO_SERIALVERSIONID", justification = "Serialization happens exclusively through XStream and not Java Serialization.") @Deprecated public static class HostPathVolume extends org.csanchez.jenkins.plugins.kubernetes.volumes.HostPathVolume { - - private static final long serialVersionUID = 42L; - public HostPathVolume(String hostPath, String mountPath, Boolean readOnly) { super(hostPath, mountPath, readOnly); } @@ -59,12 +54,10 @@ protected Object readResolve() { } } + @SuppressFBWarnings(value = "SE_NO_SERIALVERSIONID", justification = "Serialization happens exclusively through XStream and not Java Serialization.") @Deprecated public static class NfsVolume extends org.csanchez.jenkins.plugins.kubernetes.volumes.NfsVolume { - - private static final long serialVersionUID = 42L; - public NfsVolume(String serverAddress, String serverPath, Boolean readOnly, String mountPath) { super(serverAddress, serverPath, readOnly, mountPath); } diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PortMapping.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PortMapping.java index bb2e3ac189..87f6f04324 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PortMapping.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PortMapping.java @@ -10,12 +10,11 @@ import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.DataBoundSetter; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +@SuppressFBWarnings(value = "SE_NO_SERIALVERSIONID", justification = "Serialization happens exclusively through XStream and not Java Serialization.") public class PortMapping extends AbstractDescribableImpl implements Serializable { - - private static final long serialVersionUID = 42L; - private String name; private Integer containerPort; private Integer hostPort; diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerExecDecorator.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerExecDecorator.java index 5e2c5494a3..2322591c53 100755 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerExecDecorator.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerExecDecorator.java @@ -336,8 +336,6 @@ public Proc launch(ProcStarter starter) throws IOException { getCommands(starter, containerWorkingDirFilePathStr, launcher.isUnix())); } - @SuppressFBWarnings(value = "REC_CATCH_EXCEPTION", justification = "Warning raised for catching the Exception e at the end of the " - + "method. Not sure about the possible impacts of changing that bit.") private Proc doLaunch(boolean quiet, String[] cmdEnvs, OutputStream outputForCaller, FilePath pwd, boolean[] masks, String... commands) throws IOException { long startMethod = System.nanoTime(); @@ -570,8 +568,9 @@ public void onClose(int i, String s) { closables.add(proc); return proc; } catch (InterruptedException ie) { + closeWatch(watch); throw new InterruptedIOException(ie.getMessage()); - } catch (Exception e) { + } catch (RuntimeException e) { closeWatch(watch); throw e; } diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerLogStepExecution.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerLogStepExecution.java index 67a84280fb..9d32b2e04d 100755 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerLogStepExecution.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerLogStepExecution.java @@ -25,6 +25,7 @@ import org.jenkinsci.plugins.workflow.steps.StepContext; import org.jenkinsci.plugins.workflow.steps.SynchronousNonBlockingStepExecution; +import java.io.IOException; import java.io.PrintStream; import java.util.logging.Level; import java.util.logging.Logger; @@ -43,14 +44,12 @@ public class ContainerLogStepExecution extends SynchronousNonBlockingStepExecuti this.step = step; } - @SuppressFBWarnings(value = "REC_CATCH_EXCEPTION", justification = "Warning raised for catching the Exception x, not entirely sure about the " - + "possible effects of removing that bit.") private PrintStream logger() { TaskListener l = null; StepContext context = getContext(); try { l = context.get(TaskListener.class); - } catch (Exception x) { + } catch (IOException | InterruptedException x) { LOGGER.log(Level.WARNING, "Failed to find TaskListener in context"); } finally { if (l == null) { diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerStepExecution.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerStepExecution.java index 493ec43cc0..7708fd69f2 100755 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerStepExecution.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerStepExecution.java @@ -3,6 +3,7 @@ import static org.csanchez.jenkins.plugins.kubernetes.pipeline.Resources.*; import java.io.Closeable; +import java.io.Serializable; import java.util.Collections; import java.util.List; import java.util.logging.Level; @@ -95,15 +96,14 @@ public void stop(@NonNull Throwable cause) throws Exception { closeQuietly(getContext(), decorator); } - @SuppressFBWarnings(value = "SE_BAD_FIELD", justification = "Warning about closeables not being non-transient non-serializable, not entirely " - + "sure what is the desired behavior and what impact could a change have here.") - private static class ContainerExecCallback extends BodyExecutionCallback.TailCall { + private static class ContainerExecCallback extends BodyExecutionCallback.TailCall { private static final long serialVersionUID = 6385838254761750483L; - private final Closeable[] closeables; + private final T[] closeables; - private ContainerExecCallback(Closeable... closeables) { + @SafeVarargs + private ContainerExecCallback(T... closeables) { this.closeables = closeables; } @Override diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesAgentErrorCondition.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesAgentErrorCondition.java index b27e521c83..6b691da0f3 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesAgentErrorCondition.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesAgentErrorCondition.java @@ -42,16 +42,15 @@ import org.jenkinsci.plugins.workflow.support.steps.AgentErrorCondition; import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.DataBoundSetter; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; /** * Qualifies {@code node} blocks associated with {@link KubernetesSlave} to be retried if the node was deleted. * A more specific version of {@link AgentErrorCondition}. */ +@SuppressFBWarnings(value = "SE_NO_SERIALVERSIONID", justification = "Serialization happens exclusively through XStream and not Java Serialization.") public class KubernetesAgentErrorCondition extends ErrorCondition { - - private static final long serialVersionUID = 42L; - private static final Logger LOGGER = Logger.getLogger(KubernetesAgentErrorCondition.class.getName()); private static final Set IGNORED_CONTAINER_TERMINATION_REASONS = new HashSet<>(); diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesDeclarativeAgent.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesDeclarativeAgent.java index bf41f37d61..6195e948ae 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesDeclarativeAgent.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesDeclarativeAgent.java @@ -2,6 +2,8 @@ import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + import hudson.ExtensionList; import hudson.Util; import hudson.model.Label; @@ -31,11 +33,9 @@ import java.util.stream.Collectors; import org.jenkinsci.plugins.pipeline.modeldefinition.agent.RetryableDeclarativeAgent; +@SuppressFBWarnings(value = "SE_NO_SERIALVERSIONID", justification = "Serialization happens exclusively through XStream and not Java Serialization.") public class KubernetesDeclarativeAgent extends RetryableDeclarativeAgent { - - private static final long serialVersionUID = 42L; - private static final Logger LOGGER = Logger.getLogger(KubernetesDeclarativeAgent.class.getName()); @CheckForNull diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/retention/Reaper.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/retention/Reaper.java index 5e4568ee08..0c5ac4fc1e 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/retention/Reaper.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/retention/Reaper.java @@ -147,8 +147,6 @@ private void activate() { /** * Remove any {@link KubernetesSlave} nodes that reference Pods that don't exist. */ - @SuppressFBWarnings(value = "REC_CATCH_EXCEPTION", justification = "Warning about the Exception x catch at the end. Not sure about the possible" - + " impact of removing it.") private void reapAgents() { Jenkins jenkins = Jenkins.getInstanceOrNull(); if (jenkins != null) { @@ -163,20 +161,21 @@ private void reapAgents() { } String ns = ks.getNamespace(); String name = ks.getPodName(); - try { - // TODO more efficient to do a single (or paged) list request, but tricky since there may be multiple clouds, - // and even within a single cloud an agent pod is permitted to use a nondefault namespace, - // yet we do not want to do an unnamespaced pod list for RBAC reasons. - // Could use a hybrid approach: first list all pods in the configured namespace for all clouds; - // then go back and individually check any unmatched agents with their configured namespace. - KubernetesCloud cloud = ks.getKubernetesCloud(); - if (cloud.connect().pods().inNamespace(ns).withName(name).get() == null) { + + // TODO more efficient to do a single (or paged) list request, but tricky since there may be multiple clouds, + // and even within a single cloud an agent pod is permitted to use a nondefault namespace, + // yet we do not want to do an unnamespaced pod list for RBAC reasons. + // Could use a hybrid approach: first list all pods in the configured namespace for all clouds; + // then go back and individually check any unmatched agents with their configured namespace. + KubernetesCloud cloud = ks.getKubernetesCloud(); + try (KubernetesClient kubernetesClient = cloud.connect()) { + if (kubernetesClient.pods().inNamespace(ns).withName(name).get() == null) { LOGGER.info(() -> ns + "/" + name + " seems to have been deleted, so removing corresponding Jenkins agent"); jenkins.removeNode(ks); } else { LOGGER.fine(() -> ns + "/" + name + " still seems to exist, OK"); } - } catch (Exception x) { + } catch (KubernetesAuthException | IOException | RuntimeException x) { LOGGER.log(Level.WARNING, x, () -> "failed to do initial reap check for " + ns + "/" + name); } } @@ -213,15 +212,12 @@ private void watchClouds() { * is no longer valid. * @param kc kubernetes cloud to watch */ - @SuppressFBWarnings(value = "REC_CATCH_EXCEPTION", justification = "Warning about the Exception x caught at the end, but not sure about the " - + "possible impact of removing it.") private void watchCloud(@NonNull KubernetesCloud kc) { // can't use ConcurrentHashMap#computeIfAbsent because CloudPodWatcher will remove itself from the watchers // map on close. If an error occurs when creating the watch it would create a deadlock situation. CloudPodWatcher watcher = new CloudPodWatcher(kc); if (!isCloudPodWatcherActive(watcher)) { - try { - KubernetesClient client = kc.connect(); + try(KubernetesClient client = kc.connect()) { watcher.watch = client.pods().inNamespace(client.getNamespace()).watch(watcher); CloudPodWatcher old = watchers.put(kc.name, watcher); // if another watch slipped in then make sure it stopped @@ -229,7 +225,7 @@ private void watchCloud(@NonNull KubernetesCloud kc) { old.stop(); } LOGGER.info(() -> "set up watcher on " + kc.getDisplayName()); - } catch (Exception x) { + } catch (KubernetesAuthException | IOException | RuntimeException x) { LOGGER.log(Level.WARNING, x, () -> "failed to set up watcher on " + kc.getDisplayName()); } } diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/ConfigMapVolume.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/ConfigMapVolume.java index c2865cf0f7..3fdc98d4c2 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/ConfigMapVolume.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/ConfigMapVolume.java @@ -33,12 +33,10 @@ import io.fabric8.kubernetes.api.model.Volume; import io.fabric8.kubernetes.api.model.VolumeBuilder; import org.kohsuke.stapler.DataBoundSetter; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; - +@SuppressFBWarnings(value = "SE_NO_SERIALVERSIONID", justification = "Serialization happens exclusively through XStream and not Java Serialization.") public class ConfigMapVolume extends PodVolume { - - private static final long serialVersionUID = 42L; - private String mountPath; private String subPath; private String configMapName; diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/DynamicPVCVolume.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/DynamicPVCVolume.java index ece7540743..aa48b5e6cc 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/DynamicPVCVolume.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/DynamicPVCVolume.java @@ -12,6 +12,7 @@ import java.util.Objects; import java.util.UUID; import edu.umd.cs.findbugs.annotations.NonNull; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import org.jenkinsci.Symbol; import org.kohsuke.accmod.Restricted; import org.kohsuke.accmod.restrictions.DoNotUse; @@ -22,10 +23,8 @@ /** * Implements a dynamic PVC volume, that is created before the agent pod is created, and terminated afterwards. */ +@SuppressFBWarnings(value = "SE_NO_SERIALVERSIONID", justification = "Serialization happens exclusively through XStream and not Java Serialization.") public class DynamicPVCVolume extends PodVolume implements DynamicPVC { - - private static final long serialVersionUID = 42L; - private String id; private String storageClassName; private String requestsSize; diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/EmptyDirVolume.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/EmptyDirVolume.java index 2fbf7af217..79e807a5dc 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/EmptyDirVolume.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/EmptyDirVolume.java @@ -26,6 +26,7 @@ import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import org.jenkinsci.Symbol; import org.kohsuke.stapler.DataBoundConstructor; @@ -34,11 +35,9 @@ import io.fabric8.kubernetes.api.model.Volume; import io.fabric8.kubernetes.api.model.VolumeBuilder; +@SuppressFBWarnings(value = "SE_NO_SERIALVERSIONID", justification = "Serialization happens exclusively through XStream and not Java Serialization.") public class EmptyDirVolume extends PodVolume { - - private static final long serialVersionUID = 42L; - private static final String DEFAULT_MEDIUM = ""; private static final String MEMORY_MEDIUM = "Memory"; diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/HostPathVolume.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/HostPathVolume.java index ec97de9b10..76ef260866 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/HostPathVolume.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/HostPathVolume.java @@ -26,6 +26,7 @@ import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import org.jenkinsci.Symbol; import org.kohsuke.stapler.DataBoundConstructor; @@ -34,10 +35,8 @@ import io.fabric8.kubernetes.api.model.Volume; import io.fabric8.kubernetes.api.model.VolumeBuilder; +@SuppressFBWarnings(value = "SE_NO_SERIALVERSIONID", justification = "Serialization happens exclusively through XStream and not Java Serialization.") public class HostPathVolume extends PodVolume { - - private static final long serialVersionUID = 42L; - private String mountPath; private String hostPath; diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/NfsVolume.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/NfsVolume.java index 11235eef42..dea04db571 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/NfsVolume.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/NfsVolume.java @@ -26,6 +26,7 @@ import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import org.jenkinsci.Symbol; import org.kohsuke.stapler.DataBoundConstructor; @@ -34,10 +35,8 @@ import io.fabric8.kubernetes.api.model.Volume; import io.fabric8.kubernetes.api.model.VolumeBuilder; +@SuppressFBWarnings(value = "SE_NO_SERIALVERSIONID", justification = "Serialization happens exclusively through XStream and not Java Serialization.") public class NfsVolume extends PodVolume { - - private static final long serialVersionUID = 42L; - private String mountPath; private String serverAddress; private String serverPath; diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/PersistentVolumeClaim.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/PersistentVolumeClaim.java index 007932904e..a4a656c27b 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/PersistentVolumeClaim.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/PersistentVolumeClaim.java @@ -26,6 +26,7 @@ import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import org.jenkinsci.Symbol; import org.kohsuke.stapler.DataBoundConstructor; @@ -34,10 +35,8 @@ import io.fabric8.kubernetes.api.model.Volume; import io.fabric8.kubernetes.api.model.VolumeBuilder; +@SuppressFBWarnings(value = "SE_NO_SERIALVERSIONID", justification = "Serialization happens exclusively through XStream and not Java Serialization.") public class PersistentVolumeClaim extends PodVolume { - - private static final long serialVersionUID = 42L; - private String mountPath; private String claimName; @CheckForNull diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/SecretVolume.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/SecretVolume.java index 691573aa91..12b18053a5 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/SecretVolume.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/SecretVolume.java @@ -28,16 +28,16 @@ import org.jenkinsci.Symbol; import org.kohsuke.stapler.DataBoundConstructor; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + import hudson.Extension; import hudson.model.Descriptor; import io.fabric8.kubernetes.api.model.SecretVolumeSource; import io.fabric8.kubernetes.api.model.Volume; import io.fabric8.kubernetes.api.model.VolumeBuilder; +@SuppressFBWarnings(value = "SE_NO_SERIALVERSIONID", justification = "Serialization happens exclusively through XStream and not Java Serialization.") public class SecretVolume extends PodVolume { - - private static final long serialVersionUID = 42L; - private String mountPath; private String secretName; private String defaultMode; diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/DynamicPVCWorkspaceVolume.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/DynamicPVCWorkspaceVolume.java index fa65ff5b5d..e67f0faded 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/DynamicPVCWorkspaceVolume.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/DynamicPVCWorkspaceVolume.java @@ -11,6 +11,7 @@ import io.fabric8.kubernetes.client.KubernetesClient; import java.util.Objects; import edu.umd.cs.findbugs.annotations.NonNull; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import org.csanchez.jenkins.plugins.kubernetes.volumes.DynamicPVC; import org.csanchez.jenkins.plugins.kubernetes.volumes.PVCVolumeUtils; import org.jenkinsci.Symbol; @@ -23,10 +24,8 @@ /** * @author runzexia */ +@SuppressFBWarnings(value = "SE_NO_SERIALVERSIONID", justification = "Serialization happens exclusively through XStream and not Java Serialization.") public class DynamicPVCWorkspaceVolume extends WorkspaceVolume implements DynamicPVC { - - private static final long serialVersionUID = 42L; - private String storageClassName; private String requestsSize; private String accessModes; diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/EmptyDirWorkspaceVolume.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/EmptyDirWorkspaceVolume.java index 05e0d9c34d..a0138e302c 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/EmptyDirWorkspaceVolume.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/EmptyDirWorkspaceVolume.java @@ -26,6 +26,7 @@ import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import org.jenkinsci.Symbol; import org.kohsuke.stapler.DataBoundConstructor; @@ -36,11 +37,9 @@ import java.util.Objects; +@SuppressFBWarnings(value = "SE_NO_SERIALVERSIONID", justification = "Serialization happens exclusively through XStream and not Java Serialization.") public class EmptyDirWorkspaceVolume extends WorkspaceVolume { - - private static final long serialVersionUID = 42L; - private static final String DEFAULT_MEDIUM = ""; private static final String MEMORY_MEDIUM = "Memory"; diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/HostPathWorkspaceVolume.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/HostPathWorkspaceVolume.java index b27ec0b25e..d4ebcc207d 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/HostPathWorkspaceVolume.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/HostPathWorkspaceVolume.java @@ -34,10 +34,10 @@ import java.util.Objects; -public class HostPathWorkspaceVolume extends WorkspaceVolume { - - private static final long serialVersionUID = 42L; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +@SuppressFBWarnings(value = "SE_NO_SERIALVERSIONID", justification = "Serialization happens exclusively through XStream and not Java Serialization.") +public class HostPathWorkspaceVolume extends WorkspaceVolume { private String hostPath; @DataBoundConstructor diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/NfsWorkspaceVolume.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/NfsWorkspaceVolume.java index 8598326772..3137991c88 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/NfsWorkspaceVolume.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/NfsWorkspaceVolume.java @@ -26,6 +26,7 @@ import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import org.jenkinsci.Symbol; import org.kohsuke.stapler.DataBoundConstructor; @@ -36,10 +37,8 @@ import java.util.Objects; +@SuppressFBWarnings(value = "SE_NO_SERIALVERSIONID", justification = "Serialization happens exclusively through XStream and not Java Serialization.") public class NfsWorkspaceVolume extends WorkspaceVolume { - - private static final long serialVersionUID = 42L; - private String serverAddress; private String serverPath; @CheckForNull diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/PersistentVolumeClaimWorkspaceVolume.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/PersistentVolumeClaimWorkspaceVolume.java index 3952ef158e..abea9a0e7f 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/PersistentVolumeClaimWorkspaceVolume.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/volumes/workspace/PersistentVolumeClaimWorkspaceVolume.java @@ -26,6 +26,7 @@ import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import org.jenkinsci.Symbol; import org.kohsuke.stapler.DataBoundConstructor; @@ -36,10 +37,8 @@ import java.util.Objects; +@SuppressFBWarnings(value = "SE_NO_SERIALVERSIONID", justification = "Serialization happens exclusively through XStream and not Java Serialization.") public class PersistentVolumeClaimWorkspaceVolume extends WorkspaceVolume { - - private static final long serialVersionUID = 42L; - private String claimName; @CheckForNull private Boolean readOnly; From ecafc23a38ddf8837325128ab535750052c1f08a Mon Sep 17 00:00:00 2001 From: Antoine Neveux Date: Fri, 23 Jun 2023 20:19:06 +0200 Subject: [PATCH 3/4] Adressing code review and fixing tests --- .../pipeline/PodTemplateStepExecution.java | 18 ++++--- .../kubernetes/pod/retention/Reaper.java | 49 +++++++++---------- 2 files changed, 34 insertions(+), 33 deletions(-) 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 a7a8671561..065a9b4d9d 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 @@ -61,7 +61,7 @@ public class PodTemplateStepExecution extends AbstractStepExecutionImpl { @Override public boolean start() throws Exception { - KubernetesCloud cloud = resolveCloud(); + KubernetesCloud cloud = resolveCloud(cloudName); Run run = getContext().get(Run.class); if (cloud.isUsageRestricted()) { @@ -150,7 +150,8 @@ public boolean start() throws Exception { listener.getLogger().println("Registering template with id=" + newTemplate.getId() + ",label="+ newTemplate.getLabel()); } cloud.addDynamicTemplate(newTemplate); - BodyInvoker invoker = getContext().newBodyInvoker().withContexts(step, new PodTemplateContext(namespace, name)).withCallback(new PodTemplateCallback(newTemplate)); + BodyInvoker invoker = + getContext().newBodyInvoker().withContexts(step, new PodTemplateContext(namespace, name)).withCallback(new PodTemplateCallback(newTemplate, cloudName)); if (step.getLabel() == null) { invoker.withContext(EnvironmentExpander.merge(getContext().get(EnvironmentExpander.class), EnvironmentExpander.constant(Collections.singletonMap("POD_LABEL", label)))); } @@ -160,7 +161,7 @@ public boolean start() throws Exception { } @NonNull - private KubernetesCloud resolveCloud() throws AbortException { + private static KubernetesCloud resolveCloud(final String cloudName) throws AbortException { KubernetesCloud cloud; if (cloudName == null) { cloud = Jenkins.get().clouds.get(KubernetesCloud.class); @@ -230,7 +231,7 @@ private String checkNamespace(KubernetesCloud kubernetesCloud, @CheckForNull Pod @Override public void onResume() { try { - KubernetesCloud cloud = resolveCloud(); + KubernetesCloud cloud = resolveCloud(cloudName); TaskListener listener = getContext().get(TaskListener.class); newTemplate.setListener(listener); LOGGER.log(Level.FINE, "Re-registering template with id=" + newTemplate.getId() + " after resume"); @@ -245,15 +246,16 @@ public void onResume() { } } - @SuppressFBWarnings(value = "SE_INNER_CLASS", justification = "Not sure if it is intended or if this inner class should be static.") - private class PodTemplateCallback extends BodyExecutionCallback.TailCall { + private static class PodTemplateCallback extends BodyExecutionCallback.TailCall { private static final long serialVersionUID = 6043919968776851324L; private final PodTemplate podTemplate; + private final String cloudName; - private PodTemplateCallback(PodTemplate podTemplate) { + private PodTemplateCallback(PodTemplate podTemplate, final String cloudName) { this.podTemplate = podTemplate; + this.cloudName = cloudName; } @Override @@ -262,7 +264,7 @@ private PodTemplateCallback(PodTemplate podTemplate) { */ protected void finished(StepContext context) throws Exception { try { - KubernetesCloud cloud = resolveCloud(); + KubernetesCloud cloud = resolveCloud(cloudName); LOGGER.log(Level.FINE, () -> "Removing pod template " + podTemplate.getName() + " from cloud " + cloud.name); cloud.removeDynamicTemplate(podTemplate); diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/retention/Reaper.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/retention/Reaper.java index 0c5ac4fc1e..1eda0ad0fa 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/retention/Reaper.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/retention/Reaper.java @@ -64,7 +64,6 @@ import org.csanchez.jenkins.plugins.kubernetes.KubernetesCloud; import org.csanchez.jenkins.plugins.kubernetes.KubernetesComputer; import org.csanchez.jenkins.plugins.kubernetes.KubernetesSlave; -import org.csanchez.jenkins.plugins.kubernetes.PodTemplate; import org.csanchez.jenkins.plugins.kubernetes.PodUtils; import org.jenkinsci.plugins.kubernetes.auth.KubernetesAuthException; @@ -78,7 +77,7 @@ */ @Extension public class Reaper extends ComputerListener { - + private static final Logger LOGGER = Logger.getLogger(Reaper.class.getName()); /** @@ -106,8 +105,8 @@ public static Reaper getInstance() { private final Map watchers = new ConcurrentHashMap<>(); private final LoadingCache> terminationReasons = Caffeine.newBuilder(). - expireAfterAccess(1, TimeUnit.DAYS). - build(k -> new ConcurrentSkipListSet<>()); + expireAfterAccess(1, TimeUnit.DAYS). + build(k -> new ConcurrentSkipListSet<>()); @Override public void preLaunch(Computer c, TaskListener taskListener) throws IOException, InterruptedException { @@ -161,15 +160,14 @@ private void reapAgents() { } String ns = ks.getNamespace(); String name = ks.getPodName(); - - // TODO more efficient to do a single (or paged) list request, but tricky since there may be multiple clouds, - // and even within a single cloud an agent pod is permitted to use a nondefault namespace, - // yet we do not want to do an unnamespaced pod list for RBAC reasons. - // Could use a hybrid approach: first list all pods in the configured namespace for all clouds; - // then go back and individually check any unmatched agents with their configured namespace. - KubernetesCloud cloud = ks.getKubernetesCloud(); - try (KubernetesClient kubernetesClient = cloud.connect()) { - if (kubernetesClient.pods().inNamespace(ns).withName(name).get() == null) { + try { + // TODO more efficient to do a single (or paged) list request, but tricky since there may be multiple clouds, + // and even within a single cloud an agent pod is permitted to use a nondefault namespace, + // yet we do not want to do an unnamespaced pod list for RBAC reasons. + // Could use a hybrid approach: first list all pods in the configured namespace for all clouds; + // then go back and individually check any unmatched agents with their configured namespace. + KubernetesCloud cloud = ks.getKubernetesCloud(); + if (cloud.connect().pods().inNamespace(ns).withName(name).get() == null) { LOGGER.info(() -> ns + "/" + name + " seems to have been deleted, so removing corresponding Jenkins agent"); jenkins.removeNode(ks); } else { @@ -198,12 +196,12 @@ private void watchClouds() { // close any cloud watchers that have been removed cloudNames.stream() - .map(this.watchers::get) - .filter(Objects::nonNull) - .forEach(cpw -> { - LOGGER.info(() -> "stopping pod watcher for deleted kubernetes cloud " + cpw.cloudName); - cpw.stop(); - }); + .map(this.watchers::get) + .filter(Objects::nonNull) + .forEach(cpw -> { + LOGGER.info(() -> "stopping pod watcher for deleted kubernetes cloud " + cpw.cloudName); + cpw.stop(); + }); } } @@ -217,7 +215,8 @@ private void watchCloud(@NonNull KubernetesCloud kc) { // map on close. If an error occurs when creating the watch it would create a deadlock situation. CloudPodWatcher watcher = new CloudPodWatcher(kc); if (!isCloudPodWatcherActive(watcher)) { - try(KubernetesClient client = kc.connect()) { + try { + KubernetesClient client = kc.connect(); watcher.watch = client.pods().inNamespace(client.getNamespace()).watch(watcher); CloudPodWatcher old = watchers.put(kc.name, watcher); // if another watch slipped in then make sure it stopped @@ -253,10 +252,10 @@ private boolean isCloudPodWatcherActive(@NonNull CloudPodWatcher watcher) { private static Optional resolveNode(@NonNull Jenkins jenkins, String namespace, String name) { return new ArrayList<>(jenkins.getNodes()).stream() - .filter(KubernetesSlave.class::isInstance) - .map(KubernetesSlave.class::cast) - .filter(ks -> Objects.equals(ks.getNamespace(), namespace) && Objects.equals(ks.getPodName(), name)) - .findFirst(); + .filter(KubernetesSlave.class::isInstance) + .map(KubernetesSlave.class::cast) + .filter(ks -> Objects.equals(ks.getNamespace(), namespace) && Objects.equals(ks.getPodName(), name)) + .findFirst(); } /** @@ -496,4 +495,4 @@ public void onChange(Saveable o, XmlFile file) { } } } -} +} \ No newline at end of file From be1fc33646d52b927e7254c1b59d5c7b8c6b2e50 Mon Sep 17 00:00:00 2001 From: Antoine Neveux Date: Wed, 23 Aug 2023 14:58:01 +0200 Subject: [PATCH 4/4] Suppressing an additional Spotbugs warning --- .../kubernetes/pipeline/ContainerStepExecution.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerStepExecution.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerStepExecution.java index 7708fd69f2..5af3d66929 100755 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerStepExecution.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerStepExecution.java @@ -96,14 +96,14 @@ public void stop(@NonNull Throwable cause) throws Exception { closeQuietly(getContext(), decorator); } - private static class ContainerExecCallback extends BodyExecutionCallback.TailCall { + @SuppressFBWarnings("SE_BAD_FIELD") + private static class ContainerExecCallback extends BodyExecutionCallback.TailCall { private static final long serialVersionUID = 6385838254761750483L; - private final T[] closeables; + private final Closeable[] closeables; - @SafeVarargs - private ContainerExecCallback(T... closeables) { + private ContainerExecCallback(Closeable... closeables) { this.closeables = closeables; } @Override