Skip to content

Commit

Permalink
Replace JSR-305 annotations with spotbugs annotations
Browse files Browse the repository at this point in the history
Annotations for Nonnull, CheckForNull, and several others were proposed
for Java as part of dormant Java specification request JSR-305. The
proposal never became a part of standard Java.

Jenkins plugins should switch from using JSR-305 annotations to use
Spotbugs annotations that provide the same semantics.

The [mailing list discussion](https://groups.google.com/g/jenkinsci-dev/c/uE1wwtVi1W0/m/gLxdEJmlBQAJ)
from James Nord describes the affected annotations and why they should
be replaced with annotations that are actively maintained.

The ["Improve a plugin" tutorial](https://www.jenkins.io/doc/developer/tutorial-improve/replace-jsr-305-annotations/)
provides instructions to perform this change.

An [OpenRewrite recipe](https://docs.openrewrite.org/recipes/jenkins/javaxannotationstospotbugs)
is also available and is even better than the tutorial.

Confirmed that automated tests pass on Linux with Java 21.
  • Loading branch information
MarkEWaite committed May 8, 2024
1 parent e844f40 commit 366b3ef
Show file tree
Hide file tree
Showing 7 changed files with 15 additions and 29 deletions.
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
package org.jenkinsci.plugins.kubernetes.cli;

import javax.annotation.Nonnull;

import com.cloudbees.plugins.credentials.CredentialsMatcher;
import com.cloudbees.plugins.credentials.CredentialsMatchers;
import com.cloudbees.plugins.credentials.common.StandardCertificateCredentials;
import com.cloudbees.plugins.credentials.common.StandardCredentials;
import com.cloudbees.plugins.credentials.common.StandardListBoxModel;
import com.cloudbees.plugins.credentials.common.StandardUsernamePasswordCredentials;
import com.cloudbees.plugins.credentials.domains.URIRequirementBuilder;

import edu.umd.cs.findbugs.annotations.NonNull;
import org.jenkinsci.plugins.kubernetes.credentials.TokenProducer;
import org.jenkinsci.plugins.plaincredentials.FileCredentials;
import org.jenkinsci.plugins.plaincredentials.StringCredentials;
Expand All @@ -31,7 +29,7 @@ public abstract class CredentialsLister {
CredentialsMatchers.instanceOf(StandardCertificateCredentials.class),
CredentialsMatchers.instanceOf(FileCredentials.class));

public static ListBoxModel doFillCredentialsIdItems(@Nonnull @AncestorInPath Item item,
public static ListBoxModel doFillCredentialsIdItems(@NonNull @AncestorInPath Item item,

Check warning

Code scanning / Jenkins Security Scan

Stapler: Missing POST/RequirePOST annotation Warning

Potential CSRF vulnerability: If CredentialsLister#doFillCredentialsIdItems connects to user-specified URLs, modifies state, or is expensive to run, it should be annotated with @POST or @RequirePOST
@QueryParameter String serverUrl, @QueryParameter String credentialsId) {
if (item == null
? !Jenkins.get().hasPermission(Jenkins.ADMINISTER)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,13 @@
import java.util.ArrayList;
import java.util.List;

import javax.annotation.Nonnull;

import org.jenkinsci.plugins.kubernetes.cli.kubeconfig.KubeConfigWriter;
import org.jenkinsci.plugins.kubernetes.cli.kubeconfig.KubeConfigWriterFactory;
import org.jenkinsci.plugins.workflow.steps.AbstractStepExecutionImpl;
import org.jenkinsci.plugins.workflow.steps.BodyExecutionCallback;
import org.jenkinsci.plugins.workflow.steps.EnvironmentExpander;
import org.jenkinsci.plugins.workflow.steps.StepContext;

import edu.umd.cs.findbugs.annotations.NonNull;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import hudson.FilePath;
import hudson.model.TaskListener;
Expand Down Expand Up @@ -78,7 +76,7 @@ public boolean start() throws Exception {
* {@inheritDoc}
*/
@Override
public void stop(@Nonnull Throwable cause) throws Exception {
public void stop(@NonNull Throwable cause) throws Exception {
getContext().onFailure(cause);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
import java.util.List;
import java.util.Set;

import javax.annotation.Nonnull;

import org.jenkinsci.plugins.workflow.steps.Step;
import org.jenkinsci.plugins.workflow.steps.StepContext;
import org.jenkinsci.plugins.workflow.steps.StepDescriptor;
Expand All @@ -15,7 +13,7 @@
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.DataBoundSetter;
import org.kohsuke.stapler.QueryParameter;

import edu.umd.cs.findbugs.annotations.NonNull;
import hudson.Extension;
import hudson.model.Item;
import hudson.util.ListBoxModel;
Expand Down Expand Up @@ -97,7 +95,7 @@ public Set<? extends Class<?>> getRequiredContext() {
return new HashSet<>();
}

public ListBoxModel doFillCredentialsIdItems(@Nonnull @AncestorInPath Item item,
public ListBoxModel doFillCredentialsIdItems(@NonNull @AncestorInPath Item item,

Check warning

Code scanning / Jenkins Security Scan

Stapler: Missing POST/RequirePOST annotation Warning

Potential CSRF vulnerability: If DescriptorImpl#doFillCredentialsIdItems connects to user-specified URLs, modifies state, or is expensive to run, it should be annotated with @POST or @RequirePOST
@QueryParameter String serverUrl, @QueryParameter String credentialsId) {
return CredentialsLister.doFillCredentialsIdItems(item, serverUrl, credentialsId);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,11 @@
import java.util.ArrayList;
import java.util.List;

import javax.annotation.Nonnull;

import org.kohsuke.stapler.AncestorInPath;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.DataBoundSetter;
import org.kohsuke.stapler.QueryParameter;

import edu.umd.cs.findbugs.annotations.NonNull;
import hudson.EnvVars;
import hudson.Extension;
import hudson.FilePath;
Expand Down Expand Up @@ -87,7 +85,7 @@ public String getDisplayName() {
return "Configure Kubernetes CLI (kubectl) (deprecated, use the multi credentials one instead)";
}

public ListBoxModel doFillCredentialsIdItems(@Nonnull @AncestorInPath Item item,
public ListBoxModel doFillCredentialsIdItems(@NonNull @AncestorInPath Item item,

Check warning

Code scanning / Jenkins Security Scan

Stapler: Missing POST/RequirePOST annotation Warning

Potential CSRF vulnerability: If DescriptorImpl#doFillCredentialsIdItems connects to user-specified URLs, modifies state, or is expensive to run, it should be annotated with @POST or @RequirePOST
@QueryParameter String serverUrl, @QueryParameter String credentialsId) {
return CredentialsLister.doFillCredentialsIdItems(item, serverUrl, credentialsId);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
package org.jenkinsci.plugins.kubernetes.cli;

import javax.annotation.Nonnull;

import org.kohsuke.stapler.AncestorInPath;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.DataBoundSetter;
import org.kohsuke.stapler.QueryParameter;

import edu.umd.cs.findbugs.annotations.NonNull;
import hudson.Extension;
import hudson.model.AbstractDescribableImpl;
import hudson.model.Descriptor;
Expand Down Expand Up @@ -47,7 +45,7 @@ public String getDisplayName() {
return "";
}

public ListBoxModel doFillCredentialsIdItems(@Nonnull @AncestorInPath Item item,
public ListBoxModel doFillCredentialsIdItems(@NonNull @AncestorInPath Item item,

Check warning

Code scanning / Jenkins Security Scan

Stapler: Missing POST/RequirePOST annotation Warning

Potential CSRF vulnerability: If DescriptorImpl#doFillCredentialsIdItems connects to user-specified URLs, modifies state, or is expensive to run, it should be annotated with @POST or @RequirePOST
@QueryParameter String serverUrl, @QueryParameter String credentialsId) {
return CredentialsLister.doFillCredentialsIdItems(item, serverUrl, credentialsId);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,9 @@
import java.nio.charset.StandardCharsets;
import java.util.Collections;

import javax.annotation.Nonnull;

import com.cloudbees.plugins.credentials.CredentialsProvider;
import com.cloudbees.plugins.credentials.common.StandardCredentials;

import edu.umd.cs.findbugs.annotations.NonNull;
import org.jenkinsci.plugins.kubernetes.auth.KubernetesAuth;
import org.jenkinsci.plugins.kubernetes.auth.KubernetesAuthConfig;
import org.jenkinsci.plugins.kubernetes.auth.KubernetesAuthException;
Expand Down Expand Up @@ -47,7 +45,7 @@ public class KubeConfigWriter {
private final Launcher launcher;
private final Run<?, ?> build;

public KubeConfigWriter(@Nonnull String serverUrl, String credentialsId,
public KubeConfigWriter(@NonNull String serverUrl, String credentialsId,
String caCertificate, String clusterName, String contextName, String namespace,
Boolean restrictKubeConfigAccess, FilePath workspace, Launcher launcher, Run<?, ?> build) {
this.serverUrl = serverUrl;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@

import java.io.IOException;

import javax.annotation.Nonnull;

import org.jenkinsci.plugins.workflow.steps.StepContext;

import edu.umd.cs.findbugs.annotations.NonNull;
import hudson.FilePath;
import hudson.Launcher;
import hudson.model.Run;
Expand All @@ -14,14 +12,14 @@
* @author Max Laverse
*/
public abstract class KubeConfigWriterFactory {
public static KubeConfigWriter get(@Nonnull String serverUrl, @Nonnull String credentialsId,
public static KubeConfigWriter get(@NonNull String serverUrl, @NonNull String credentialsId,
String caCertificate, String clusterName, String contextName, String namespace,
Boolean restrictKubeConfigAccess, FilePath workspace, Launcher launcher, Run<?, ?> build) {
return new KubeConfigWriter(serverUrl, credentialsId, caCertificate, clusterName, contextName, namespace,
restrictKubeConfigAccess, workspace, launcher, build);
}

public static KubeConfigWriter get(@Nonnull String serverUrl, @Nonnull String credentialsId,
public static KubeConfigWriter get(@NonNull String serverUrl, @NonNull String credentialsId,
String caCertificate, String clusterName, String contextName, String namespace,
Boolean restrictKubeConfigAccess, StepContext context) throws IOException, InterruptedException {
Run<?, ?> run = context.get(Run.class);
Expand Down

0 comments on commit 366b3ef

Please sign in to comment.