Replace findbugs by spotbugs (strimzi#1761)

* Replace findbugs by spotbugs

* comments

* rebase+fixup

* spotbugs bamboozling

* disable java11 :(

* spobugs workaround

* comment

* fix?

* rebase!

* rebase

.findbugs/findbugs-exclude.xml → .spotbugs/spotbugs-exclude.xml

@@ -1,5 +1,8 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <FindBugsFilter>
+  <Match>
+    <Bug pattern="DMI_HARDCODED_ABSOLUTE_FILENAME"/>
+  </Match>
   <Match>
     <Bug pattern="EI_EXPOSE_REP,EI_EXPOSE_REP2"/>
     <Package name="io.strimzi.certs"/>
@@ -16,5 +19,4 @@
   <Match>
     <Class name="~io\.fabric8\.kubernetes\.api\.model\.(Doneable).+(\$.*)?" />
   </Match>
-
 </FindBugsFilter>

.travis/build.sh

@@ -20,9 +20,7 @@ export DOCKER_REGISTRY=${DOCKER_REGISTRY:-docker.io}
 export DOCKER_TAG=$COMMIT
 
 make docu_check
-if [ "${MAIN_BUILD}" = "TRUE" ] ; then
-  make findbugs
-fi
+make spotbugs
 
 make crd_install
 make helm_install

Makefile

@@ -114,7 +114,7 @@ docu_htmlnoheader: docu_htmlnoheaderclean docu_versions docu_check
 docu_check:
 	./.travis/check_docs.sh
 
-findbugs: $(SUBDIRS)
+spotbugs: $(SUBDIRS)
 
 docu_pushtowebsite: docu_htmlnoheader docu_html
 	./.travis/docu-push-to-website.sh
@@ -145,4 +145,4 @@ crd_install: install
 $(SUBDIRS):
 	$(MAKE) -C $@ $(MAKECMDGOALS)
 
-.PHONY: all $(SUBDIRS) $(DOCKER_TARGETS) systemtests docu_versions findbugs docu_check
+.PHONY: all $(SUBDIRS) $(DOCKER_TARGETS) systemtests docu_versions spotbugs docu_check

Makefile.maven

@@ -18,5 +18,5 @@ java_clean:
 	echo "Cleaning Maven build ..."
 	mvn clean
 
-findbugs:
-	mvn $(MVN_ARGS) org.codehaus.mojo:findbugs-maven-plugin:check
+spotbugs:
+	mvn $(MVN_ARGS) spotbugs:check

cluster-operator/pom.xml

@@ -17,12 +17,6 @@
     </licenses>
 
     <dependencies>
-        <dependency>
-            <groupId>com.google.code.findbugs</groupId>
-            <artifactId>findbugs-annotations</artifactId>
-            <version>3.0.1</version>
-            <scope>provided</scope>
-        </dependency>
         <dependency>
             <groupId>io.strimzi</groupId>
             <artifactId>api</artifactId>
@@ -121,6 +115,10 @@
             <type>test-jar</type>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>com.github.spotbugs</groupId>
+            <artifactId>spotbugs-annotations</artifactId>
+        </dependency>
     </dependencies>
 
     <build>

cluster-operator/src/main/java/io/strimzi/operator/cluster/Main.java

@@ -4,6 +4,7 @@
  */
 package io.strimzi.operator.cluster;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import io.fabric8.kubernetes.api.model.rbac.ClusterRole;
 import io.fabric8.kubernetes.client.DefaultKubernetesClient;
 import io.fabric8.kubernetes.client.KubernetesClient;
@@ -33,6 +34,7 @@
 import java.util.Map;
 import java.util.stream.Collectors;
 
+@SuppressFBWarnings("DM_EXIT")
 public class Main {
     private static final Logger log = LogManager.getLogger(Main.class.getName());
 

cluster-operator/src/main/java/io/strimzi/operator/cluster/model/AbstractModel.java

@@ -4,7 +4,6 @@
  */
 package io.strimzi.operator.cluster.model;
 
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import io.fabric8.kubernetes.api.model.Affinity;
 import io.fabric8.kubernetes.api.model.ConfigMap;
 import io.fabric8.kubernetes.api.model.ConfigMapBuilder;
@@ -291,7 +290,6 @@ protected OrderedProperties getDefaultLogConfig() {
      * @param configFileName The filename
      * @return The OrderedProperties
      */
-    @SuppressFBWarnings("OBL_UNSATISFIED_OBLIGATION") // InputStream is closed by properties.addStringPairs
     public static OrderedProperties getOrderedProperties(String configFileName) {
         OrderedProperties properties = new OrderedProperties();
         if (configFileName != null && !configFileName.isEmpty()) {
@@ -303,6 +301,12 @@ public static OrderedProperties getOrderedProperties(String configFileName) {
                     properties.addStringPairs(is);
                 } catch (IOException e) {
                     log.warn("Unable to read default log config from '{}'", configFileName);
+                } finally {
+                    try {
+                        is.close();
+                    } catch (IOException e) {
+                        log.error("Failed to close stream. Reason: " + e.getMessage());
+                    }
                 }
             }
         }

cluster-operator/src/main/java/io/strimzi/operator/cluster/operator/assembly/KafkaAssemblyOperator.java

@@ -1152,7 +1152,7 @@ Future<ReconciliationState> zkRollingUpdate() {
          */
         Future<ReconciliationState> zkScaleUpStep() {
             Future<StatefulSet> futss = zkSetOperations.getAsync(namespace, ZookeeperCluster.zookeeperClusterName(name));
-            return withVoid(futss.map(ss -> ss == null ? 0 : ss.getSpec().getReplicas())
+            return withVoid(futss.map(ss -> ss == null ? Integer.valueOf(0) : ss.getSpec().getReplicas())
                     .compose(currentReplicas -> {
                         if (currentReplicas > 0 && zkCluster.getReplicas() > currentReplicas) {
                             zkCluster.setReplicas(currentReplicas + 1);

docker-images/Makefile

@@ -19,5 +19,5 @@ docker_push:
 
 all: docker_build docker_push
 
-.PHONY: build clean release all $(SUBDIRS) $(DOCKER_TARGETS) findbugs
+.PHONY: build clean release all $(SUBDIRS) $(DOCKER_TARGETS) spotbugs
 

examples/Makefile

@@ -15,4 +15,4 @@ release:
 	$(CP) -r ./user $(RELEASE_PATH)/
 	$(CP) -r ./topic $(RELEASE_PATH)/
 
-.PHONY: all build clean docker_build docker_push docker_tag findbugs
+.PHONY: all build clean docker_build docker_push docker_tag spotbugs

helm-charts/Makefile

@@ -62,4 +62,4 @@ docker_push:
 all: docker_build
 clean: helm_clean
 
-.PHONY: build clean release findbugs
+.PHONY: build clean release spotbugs

install/Makefile

@@ -43,4 +43,4 @@ release:
 	$(CP) -r ./topic-operator $(RELEASE_PATH)/
 	$(CP) -r ./strimzi-admin $(RELEASE_PATH)/
 
-.PHONY: all build clean docker_build docker_push docker_tag findbugs
+.PHONY: all build clean docker_build docker_push docker_tag spotbugs

metrics/Makefile

@@ -17,4 +17,4 @@ release:
 	$(CP) -r ./examples/prometheus/additional-properties/*.yaml $(RELEASE_PATH)/prometheus-additional-properties/
 	$(CP) -r ./examples/prometheus/alertmanager-config/*.yaml $(RELEASE_PATH)/prometheus-alertmanager-config/
 
-.PHONY: all build clean docker_build docker_push docker_tag findbugs
+.PHONY: all build clean docker_build docker_push docker_tag spotbugs

pom.xml

@@ -64,7 +64,7 @@
         <maven.dependency.version>3.1.1</maven.dependency.version>
         <maven.gpg.version>1.6</maven.gpg.version>
         <maven.checkstyle.version>3.0.0</maven.checkstyle.version>
-        <findbugs.version>3.0.5</findbugs.version>
+        <spotbugs.version>3.1.12</spotbugs.version>
         <sonatype.nexus.staging>1.6.3</sonatype.nexus.staging>
         <jacoco.version>0.7.9</jacoco.version>
         <license.maven.version>2.11</license.maven.version>
@@ -263,10 +263,6 @@
                         <groupId>org.slf4j</groupId>
                         <artifactId>slf4j-log4j12</artifactId>
                     </exclusion>
-                    <exclusion>
-                        <groupId>com.github.spotbugs</groupId>
-                        <artifactId>spotbugs-annotations</artifactId>
-                    </exclusion>
                 </exclusions>
             </dependency>
             <dependency>
@@ -384,6 +380,11 @@
                 <version>${gson.version}</version>
                 <scope>test</scope>
             </dependency>
+            <dependency>
+                <groupId>com.github.spotbugs</groupId>
+                <artifactId>spotbugs-annotations</artifactId>
+                <version>${spotbugs.version}</version>
+            </dependency>
         </dependencies>
     </dependencyManagement>
 
@@ -588,24 +589,25 @@
                 </configuration>
             </plugin>
             <plugin>
-                <groupId>org.codehaus.mojo</groupId>
-                <artifactId>findbugs-maven-plugin</artifactId>
-                <version>${findbugs.version}</version>
+                <groupId>com.github.spotbugs</groupId>
+                <artifactId>spotbugs-maven-plugin</artifactId>
+                <version>3.1.12</version><dependencies>
+                <!-- overwrite dependency on spotbugs if you want to specify the version of˓→spotbugs -->
+                <dependency>
+                    <groupId>com.github.spotbugs</groupId>
+                    <artifactId>spotbugs</artifactId>
+                    <version>${spotbugs.version}</version>
+                </dependency></dependencies>
                 <configuration>
-                    <!--
-                        Enables analysis which takes more memory but finds more bugs.
-                        If you run out of memory, changes the value of the effort element
-                        to 'Low'.
-                    -->
                     <effort>Max</effort>
                     <!-- Reports all bugs (other values are medium and max) -->
                     <threshold>Low</threshold>
                     <!-- Produces XML report -->
                     <xmlOutput>true</xmlOutput>
                     <!-- Configures the directory in which the XML report is created -->
-                    <findbugsXmlOutputDirectory>${project.build.directory}/findbugs</findbugsXmlOutputDirectory>
+                    <spotbugsXmlOutputDirectory>${project.build.directory}/spotbugs</spotbugsXmlOutputDirectory>
                     <!-- Configures the file for excluding warnings -->
-                    <excludeFilterFile>${project.basedir}/../.findbugs/findbugs-exclude.xml</excludeFilterFile>
+                    <excludeFilterFile>${project.basedir}/../.spotbugs/spotbugs-exclude.xml</excludeFilterFile>
                 </configuration>
             </plugin>
             <plugin>

systemtest/pom.xml

@@ -144,6 +144,16 @@
             <version>${vertx.version}</version>
             <scope>compile</scope>
         </dependency>
+
+        <dependency>
+            <groupId>com.squareup.okhttp3</groupId>
+            <artifactId>okhttp</artifactId>
+        </dependency>
+
+        <dependency>
+            <groupId>com.github.spotbugs</groupId>
+            <artifactId>spotbugs-annotations</artifactId>
+        </dependency>
     </dependencies>
 
     <build>

systemtest/src/main/java/io/strimzi/systemtest/AbstractResources.java

@@ -23,6 +23,7 @@
 import io.fabric8.kubernetes.api.model.rbac.RoleBinding;
 import io.fabric8.kubernetes.api.model.rbac.RoleBindingList;
 import io.fabric8.kubernetes.client.BaseClient;
+import io.fabric8.kubernetes.client.KubernetesClient;
 import io.fabric8.kubernetes.client.dsl.MixedOperation;
 import io.fabric8.kubernetes.client.dsl.Resource;
 import io.fabric8.kubernetes.client.dsl.internal.CustomResourceOperationContext;
@@ -50,6 +51,7 @@
 import io.strimzi.api.kafka.model.KafkaTopic;
 import io.strimzi.api.kafka.model.KafkaUser;
 import io.strimzi.test.k8s.KubeClient;
+import okhttp3.OkHttpClient;
 
 abstract class AbstractResources {
 
@@ -71,7 +73,12 @@ public MixedOperation<Kafka, KafkaList, DoneableKafka, Resource<Kafka, DoneableK
     // This logic is necessary only for the deletion of resources with `cascading: true`
     private <T extends HasMetadata, L extends KubernetesResourceList, D extends Doneable<T>> MixedOperation<T, L, D, Resource<T, D>> customResourcesWithCascading(Class<T> resourceType, Class<L> listClass, Class<D> doneClass) {
 
-        CustomResourceOperationContext croc = new CustomResourceOperationContext().withOkhttpClient(((BaseClient) client().getClient()).getHttpClient()).withConfig(client().getClient().getConfiguration())
+        OkHttpClient httpClient = null;
+        KubernetesClient kubernetesClient = client().getClient();
+        if (kubernetesClient instanceof BaseClient) {
+            httpClient = ((BaseClient) kubernetesClient).getHttpClient();
+        }
+        CustomResourceOperationContext croc = new CustomResourceOperationContext().withOkhttpClient(httpClient).withConfig(client().getClient().getConfiguration())
             .withApiGroupName(Crds.kafka().getSpec().getGroup())
             .withApiGroupVersion(Crds.kafka().getSpec().getVersion())
             .withNamespace(client().getNamespace())

systemtest/src/main/java/io/strimzi/systemtest/AbstractST.java

@@ -113,8 +113,8 @@ public abstract class AbstractST extends BaseITST implements TestSeparator {
     public static final String TEST_LOG_DIR = Environment.TEST_LOG_DIR;
 
     protected Resources testMethodResources;
-    protected static Resources testClassResources;
-    protected static String operationID;
+    private static Resources testClassResources;
+    private static String operationID;
     Random rng = new Random();
 
     protected HelmClient helmClient() {
@@ -437,10 +437,22 @@ public Resources testMethodResources() {
         return testMethodResources;
     }
 
-    public Resources testClassResources() {
+    public static String getOperationID() {
+        return operationID;
+    }
+
+    public static void setOperationID(String operationID) {
+        AbstractST.operationID = operationID;
+    }
+
+    public static Resources getTestClassResources() {
         return testClassResources;
     }
 
+    public static void setTestClassResources(Resources testClassResources) {
+        AbstractST.testClassResources = testClassResources;
+    }
+
     String startTimeMeasuring(Operation operation) {
         TimeMeasuringSystem.setTestName(testClass, testName);
         return TimeMeasuringSystem.startOperation(operation);
@@ -688,7 +700,7 @@ protected void recreateTestEnv(String coNamespace, List<String> bindingsNamespac
         createNamespaces(coNamespace, bindingsNamespaces);
         applyClusterOperatorInstallFiles();
 
-        testClassResources = new Resources(kubeClient());
+        setTestClassResources(new Resources(kubeClient()));
 
         applyRoleBindings(coNamespace, bindingsNamespaces);
         // 050-Deployment

systemtest/src/main/java/io/strimzi/systemtest/HttpBridgeBaseST.java

@@ -179,7 +179,7 @@ protected void deployBridgeNodePortService() throws InterruptedException {
                 .withType("NodePort")
                 .withSelector(map)
                 .endSpec().build();
-        testClassResources.createServiceResource(service, getBridgeNamespace()).done();
+        getTestClassResources().createServiceResource(service, getBridgeNamespace()).done();
         StUtils.waitForNodePortService(bridgeExternalService);
     }
 
@@ -220,6 +220,6 @@ void deployClusterOperator() {
         createTestClassResources();
         applyRoleBindings(getBridgeNamespace());
         // 050-Deployment
-        testClassResources.clusterOperator(getBridgeNamespace()).done();
+        getTestClassResources().clusterOperator(getBridgeNamespace()).done();
     }
 }

systemtest/src/main/java/io/strimzi/systemtest/Resources.java

@@ -72,6 +72,7 @@
 
 import java.net.MalformedURLException;
 import java.net.URL;
+import java.nio.charset.Charset;
 import java.util.ArrayList;
 import java.util.Base64;
 import java.util.Collections;
@@ -1123,7 +1124,7 @@ String clusterCaCertSecretName(String cluster) {
     String saslConfigs(KafkaUser kafkaUser) {
         Secret secret = client().getSecret(kafkaUser.getMetadata().getName());
 
-        String password = new String(Base64.getDecoder().decode(secret.getData().get("password")));
+        String password = new String(Base64.getDecoder().decode(secret.getData().get("password")), Charset.forName("UTF-8"));
         if (password.isEmpty()) {
             LOGGER.info("Secret {}:\n{}", kafkaUser.getMetadata().getName(), toYamlString(secret));
             throw new RuntimeException("The Secret " + kafkaUser.getMetadata().getName() + " lacks the 'password' key");

systemtest/src/main/java/io/strimzi/systemtest/clients/api/VerifiableClient.java

@@ -108,6 +108,7 @@ protected void setAllowedArguments(ClientType clientType) {
                 allowedArguments.add(ClientArgument.VALUE_PREFIX);
                 allowedArguments.add(ClientArgument.REPEATING_KEYS);
                 allowedArguments.add(ClientArgument.USER);
+                break;
             case CLI_KAFKA_VERIFIABLE_CONSUMER:
                 allowedArguments.add(ClientArgument.BROKER_LIST);
                 allowedArguments.add(ClientArgument.TOPIC);