Skip to content

Commit ae1c165

Browse files
sknot-rhscholzj
authored andcommitted
Replace findbugs by spotbugs (strimzi#1761)
* Replace findbugs by spotbugs * comments * rebase+fixup * spotbugs bamboozling * disable java11 :( * spobugs workaround * comment * fix? * rebase! * rebase
1 parent 21f76a1 commit ae1c165

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

43 files changed

+283
-200
lines changed

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

+3-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
<?xml version="1.0" encoding="UTF-8"?>
22
<FindBugsFilter>
3+
<Match>
4+
<Bug pattern="DMI_HARDCODED_ABSOLUTE_FILENAME"/>
5+
</Match>
36
<Match>
47
<Bug pattern="EI_EXPOSE_REP,EI_EXPOSE_REP2"/>
58
<Package name="io.strimzi.certs"/>
@@ -16,5 +19,4 @@
1619
<Match>
1720
<Class name="~io\.fabric8\.kubernetes\.api\.model\.(Doneable).+(\$.*)?" />
1821
</Match>
19-
2022
</FindBugsFilter>

.travis/build.sh

+1-3
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,7 @@ export DOCKER_REGISTRY=${DOCKER_REGISTRY:-docker.io}
2020
export DOCKER_TAG=$COMMIT
2121

2222
make docu_check
23-
if [ "${MAIN_BUILD}" = "TRUE" ] ; then
24-
make findbugs
25-
fi
23+
make spotbugs
2624

2725
make crd_install
2826
make helm_install

Makefile

+2-2
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ docu_htmlnoheader: docu_htmlnoheaderclean docu_versions docu_check
114114
docu_check:
115115
./.travis/check_docs.sh
116116

117-
findbugs: $(SUBDIRS)
117+
spotbugs: $(SUBDIRS)
118118

119119
docu_pushtowebsite: docu_htmlnoheader docu_html
120120
./.travis/docu-push-to-website.sh
@@ -145,4 +145,4 @@ crd_install: install
145145
$(SUBDIRS):
146146
$(MAKE) -C $@ $(MAKECMDGOALS)
147147

148-
.PHONY: all $(SUBDIRS) $(DOCKER_TARGETS) systemtests docu_versions findbugs docu_check
148+
.PHONY: all $(SUBDIRS) $(DOCKER_TARGETS) systemtests docu_versions spotbugs docu_check

Makefile.maven

+2-2
Original file line numberDiff line numberDiff line change
@@ -18,5 +18,5 @@ java_clean:
1818
echo "Cleaning Maven build ..."
1919
mvn clean
2020

21-
findbugs:
22-
mvn $(MVN_ARGS) org.codehaus.mojo:findbugs-maven-plugin:check
21+
spotbugs:
22+
mvn $(MVN_ARGS) spotbugs:check

cluster-operator/pom.xml

+4-6
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,6 @@
1717
</licenses>
1818

1919
<dependencies>
20-
<dependency>
21-
<groupId>com.google.code.findbugs</groupId>
22-
<artifactId>findbugs-annotations</artifactId>
23-
<version>3.0.1</version>
24-
<scope>provided</scope>
25-
</dependency>
2620
<dependency>
2721
<groupId>io.strimzi</groupId>
2822
<artifactId>api</artifactId>
@@ -121,6 +115,10 @@
121115
<type>test-jar</type>
122116
<scope>test</scope>
123117
</dependency>
118+
<dependency>
119+
<groupId>com.github.spotbugs</groupId>
120+
<artifactId>spotbugs-annotations</artifactId>
121+
</dependency>
124122
</dependencies>
125123

126124
<build>

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

+2
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
*/
55
package io.strimzi.operator.cluster;
66

7+
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
78
import io.fabric8.kubernetes.api.model.rbac.ClusterRole;
89
import io.fabric8.kubernetes.client.DefaultKubernetesClient;
910
import io.fabric8.kubernetes.client.KubernetesClient;
@@ -33,6 +34,7 @@
3334
import java.util.Map;
3435
import java.util.stream.Collectors;
3536

37+
@SuppressFBWarnings("DM_EXIT")
3638
public class Main {
3739
private static final Logger log = LogManager.getLogger(Main.class.getName());
3840

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

+6-2
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
*/
55
package io.strimzi.operator.cluster.model;
66

7-
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
87
import io.fabric8.kubernetes.api.model.Affinity;
98
import io.fabric8.kubernetes.api.model.ConfigMap;
109
import io.fabric8.kubernetes.api.model.ConfigMapBuilder;
@@ -291,7 +290,6 @@ protected OrderedProperties getDefaultLogConfig() {
291290
* @param configFileName The filename
292291
* @return The OrderedProperties
293292
*/
294-
@SuppressFBWarnings("OBL_UNSATISFIED_OBLIGATION") // InputStream is closed by properties.addStringPairs
295293
public static OrderedProperties getOrderedProperties(String configFileName) {
296294
OrderedProperties properties = new OrderedProperties();
297295
if (configFileName != null && !configFileName.isEmpty()) {
@@ -303,6 +301,12 @@ public static OrderedProperties getOrderedProperties(String configFileName) {
303301
properties.addStringPairs(is);
304302
} catch (IOException e) {
305303
log.warn("Unable to read default log config from '{}'", configFileName);
304+
} finally {
305+
try {
306+
is.close();
307+
} catch (IOException e) {
308+
log.error("Failed to close stream. Reason: " + e.getMessage());
309+
}
306310
}
307311
}
308312
}

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -1152,7 +1152,7 @@ Future<ReconciliationState> zkRollingUpdate() {
11521152
*/
11531153
Future<ReconciliationState> zkScaleUpStep() {
11541154
Future<StatefulSet> futss = zkSetOperations.getAsync(namespace, ZookeeperCluster.zookeeperClusterName(name));
1155-
return withVoid(futss.map(ss -> ss == null ? 0 : ss.getSpec().getReplicas())
1155+
return withVoid(futss.map(ss -> ss == null ? Integer.valueOf(0) : ss.getSpec().getReplicas())
11561156
.compose(currentReplicas -> {
11571157
if (currentReplicas > 0 && zkCluster.getReplicas() > currentReplicas) {
11581158
zkCluster.setReplicas(currentReplicas + 1);

docker-images/Makefile

+1-1
Original file line numberDiff line numberDiff line change
@@ -19,5 +19,5 @@ docker_push:
1919

2020
all: docker_build docker_push
2121

22-
.PHONY: build clean release all $(SUBDIRS) $(DOCKER_TARGETS) findbugs
22+
.PHONY: build clean release all $(SUBDIRS) $(DOCKER_TARGETS) spotbugs
2323

examples/Makefile

+1-1
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,4 @@ release:
1515
$(CP) -r ./user $(RELEASE_PATH)/
1616
$(CP) -r ./topic $(RELEASE_PATH)/
1717

18-
.PHONY: all build clean docker_build docker_push docker_tag findbugs
18+
.PHONY: all build clean docker_build docker_push docker_tag spotbugs

helm-charts/Makefile

+1-1
Original file line numberDiff line numberDiff line change
@@ -62,4 +62,4 @@ docker_push:
6262
all: docker_build
6363
clean: helm_clean
6464

65-
.PHONY: build clean release findbugs
65+
.PHONY: build clean release spotbugs

install/Makefile

+1-1
Original file line numberDiff line numberDiff line change
@@ -43,4 +43,4 @@ release:
4343
$(CP) -r ./topic-operator $(RELEASE_PATH)/
4444
$(CP) -r ./strimzi-admin $(RELEASE_PATH)/
4545

46-
.PHONY: all build clean docker_build docker_push docker_tag findbugs
46+
.PHONY: all build clean docker_build docker_push docker_tag spotbugs

metrics/Makefile

+1-1
Original file line numberDiff line numberDiff line change
@@ -17,4 +17,4 @@ release:
1717
$(CP) -r ./examples/prometheus/additional-properties/*.yaml $(RELEASE_PATH)/prometheus-additional-properties/
1818
$(CP) -r ./examples/prometheus/alertmanager-config/*.yaml $(RELEASE_PATH)/prometheus-alertmanager-config/
1919

20-
.PHONY: all build clean docker_build docker_push docker_tag findbugs
20+
.PHONY: all build clean docker_build docker_push docker_tag spotbugs

pom.xml

+17-15
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@
6464
<maven.dependency.version>3.1.1</maven.dependency.version>
6565
<maven.gpg.version>1.6</maven.gpg.version>
6666
<maven.checkstyle.version>3.0.0</maven.checkstyle.version>
67-
<findbugs.version>3.0.5</findbugs.version>
67+
<spotbugs.version>3.1.12</spotbugs.version>
6868
<sonatype.nexus.staging>1.6.3</sonatype.nexus.staging>
6969
<jacoco.version>0.7.9</jacoco.version>
7070
<license.maven.version>2.11</license.maven.version>
@@ -263,10 +263,6 @@
263263
<groupId>org.slf4j</groupId>
264264
<artifactId>slf4j-log4j12</artifactId>
265265
</exclusion>
266-
<exclusion>
267-
<groupId>com.github.spotbugs</groupId>
268-
<artifactId>spotbugs-annotations</artifactId>
269-
</exclusion>
270266
</exclusions>
271267
</dependency>
272268
<dependency>
@@ -384,6 +380,11 @@
384380
<version>${gson.version}</version>
385381
<scope>test</scope>
386382
</dependency>
383+
<dependency>
384+
<groupId>com.github.spotbugs</groupId>
385+
<artifactId>spotbugs-annotations</artifactId>
386+
<version>${spotbugs.version}</version>
387+
</dependency>
387388
</dependencies>
388389
</dependencyManagement>
389390

@@ -588,24 +589,25 @@
588589
</configuration>
589590
</plugin>
590591
<plugin>
591-
<groupId>org.codehaus.mojo</groupId>
592-
<artifactId>findbugs-maven-plugin</artifactId>
593-
<version>${findbugs.version}</version>
592+
<groupId>com.github.spotbugs</groupId>
593+
<artifactId>spotbugs-maven-plugin</artifactId>
594+
<version>3.1.12</version><dependencies>
595+
<!-- overwrite dependency on spotbugs if you want to specify the version of˓→spotbugs -->
596+
<dependency>
597+
<groupId>com.github.spotbugs</groupId>
598+
<artifactId>spotbugs</artifactId>
599+
<version>${spotbugs.version}</version>
600+
</dependency></dependencies>
594601
<configuration>
595-
<!--
596-
Enables analysis which takes more memory but finds more bugs.
597-
If you run out of memory, changes the value of the effort element
598-
to 'Low'.
599-
-->
600602
<effort>Max</effort>
601603
<!-- Reports all bugs (other values are medium and max) -->
602604
<threshold>Low</threshold>
603605
<!-- Produces XML report -->
604606
<xmlOutput>true</xmlOutput>
605607
<!-- Configures the directory in which the XML report is created -->
606-
<findbugsXmlOutputDirectory>${project.build.directory}/findbugs</findbugsXmlOutputDirectory>
608+
<spotbugsXmlOutputDirectory>${project.build.directory}/spotbugs</spotbugsXmlOutputDirectory>
607609
<!-- Configures the file for excluding warnings -->
608-
<excludeFilterFile>${project.basedir}/../.findbugs/findbugs-exclude.xml</excludeFilterFile>
610+
<excludeFilterFile>${project.basedir}/../.spotbugs/spotbugs-exclude.xml</excludeFilterFile>
609611
</configuration>
610612
</plugin>
611613
<plugin>

systemtest/pom.xml

+10
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,16 @@
144144
<version>${vertx.version}</version>
145145
<scope>compile</scope>
146146
</dependency>
147+
148+
<dependency>
149+
<groupId>com.squareup.okhttp3</groupId>
150+
<artifactId>okhttp</artifactId>
151+
</dependency>
152+
153+
<dependency>
154+
<groupId>com.github.spotbugs</groupId>
155+
<artifactId>spotbugs-annotations</artifactId>
156+
</dependency>
147157
</dependencies>
148158

149159
<build>

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

+8-1
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import io.fabric8.kubernetes.api.model.rbac.RoleBinding;
2424
import io.fabric8.kubernetes.api.model.rbac.RoleBindingList;
2525
import io.fabric8.kubernetes.client.BaseClient;
26+
import io.fabric8.kubernetes.client.KubernetesClient;
2627
import io.fabric8.kubernetes.client.dsl.MixedOperation;
2728
import io.fabric8.kubernetes.client.dsl.Resource;
2829
import io.fabric8.kubernetes.client.dsl.internal.CustomResourceOperationContext;
@@ -50,6 +51,7 @@
5051
import io.strimzi.api.kafka.model.KafkaTopic;
5152
import io.strimzi.api.kafka.model.KafkaUser;
5253
import io.strimzi.test.k8s.KubeClient;
54+
import okhttp3.OkHttpClient;
5355

5456
abstract class AbstractResources {
5557

@@ -71,7 +73,12 @@ public MixedOperation<Kafka, KafkaList, DoneableKafka, Resource<Kafka, DoneableK
7173
// This logic is necessary only for the deletion of resources with `cascading: true`
7274
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) {
7375

74-
CustomResourceOperationContext croc = new CustomResourceOperationContext().withOkhttpClient(((BaseClient) client().getClient()).getHttpClient()).withConfig(client().getClient().getConfiguration())
76+
OkHttpClient httpClient = null;
77+
KubernetesClient kubernetesClient = client().getClient();
78+
if (kubernetesClient instanceof BaseClient) {
79+
httpClient = ((BaseClient) kubernetesClient).getHttpClient();
80+
}
81+
CustomResourceOperationContext croc = new CustomResourceOperationContext().withOkhttpClient(httpClient).withConfig(client().getClient().getConfiguration())
7582
.withApiGroupName(Crds.kafka().getSpec().getGroup())
7683
.withApiGroupVersion(Crds.kafka().getSpec().getVersion())
7784
.withNamespace(client().getNamespace())

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

+16-4
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,8 @@ public abstract class AbstractST extends BaseITST implements TestSeparator {
113113
public static final String TEST_LOG_DIR = Environment.TEST_LOG_DIR;
114114

115115
protected Resources testMethodResources;
116-
protected static Resources testClassResources;
117-
protected static String operationID;
116+
private static Resources testClassResources;
117+
private static String operationID;
118118
Random rng = new Random();
119119

120120
protected HelmClient helmClient() {
@@ -437,10 +437,22 @@ public Resources testMethodResources() {
437437
return testMethodResources;
438438
}
439439

440-
public Resources testClassResources() {
440+
public static String getOperationID() {
441+
return operationID;
442+
}
443+
444+
public static void setOperationID(String operationID) {
445+
AbstractST.operationID = operationID;
446+
}
447+
448+
public static Resources getTestClassResources() {
441449
return testClassResources;
442450
}
443451

452+
public static void setTestClassResources(Resources testClassResources) {
453+
AbstractST.testClassResources = testClassResources;
454+
}
455+
444456
String startTimeMeasuring(Operation operation) {
445457
TimeMeasuringSystem.setTestName(testClass, testName);
446458
return TimeMeasuringSystem.startOperation(operation);
@@ -688,7 +700,7 @@ protected void recreateTestEnv(String coNamespace, List<String> bindingsNamespac
688700
createNamespaces(coNamespace, bindingsNamespaces);
689701
applyClusterOperatorInstallFiles();
690702

691-
testClassResources = new Resources(kubeClient());
703+
setTestClassResources(new Resources(kubeClient()));
692704

693705
applyRoleBindings(coNamespace, bindingsNamespaces);
694706
// 050-Deployment

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ protected void deployBridgeNodePortService() throws InterruptedException {
179179
.withType("NodePort")
180180
.withSelector(map)
181181
.endSpec().build();
182-
testClassResources.createServiceResource(service, getBridgeNamespace()).done();
182+
getTestClassResources().createServiceResource(service, getBridgeNamespace()).done();
183183
StUtils.waitForNodePortService(bridgeExternalService);
184184
}
185185

@@ -220,6 +220,6 @@ void deployClusterOperator() {
220220
createTestClassResources();
221221
applyRoleBindings(getBridgeNamespace());
222222
// 050-Deployment
223-
testClassResources.clusterOperator(getBridgeNamespace()).done();
223+
getTestClassResources().clusterOperator(getBridgeNamespace()).done();
224224
}
225225
}

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

+2-1
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@
7272

7373
import java.net.MalformedURLException;
7474
import java.net.URL;
75+
import java.nio.charset.Charset;
7576
import java.util.ArrayList;
7677
import java.util.Base64;
7778
import java.util.Collections;
@@ -1123,7 +1124,7 @@ String clusterCaCertSecretName(String cluster) {
11231124
String saslConfigs(KafkaUser kafkaUser) {
11241125
Secret secret = client().getSecret(kafkaUser.getMetadata().getName());
11251126

1126-
String password = new String(Base64.getDecoder().decode(secret.getData().get("password")));
1127+
String password = new String(Base64.getDecoder().decode(secret.getData().get("password")), Charset.forName("UTF-8"));
11271128
if (password.isEmpty()) {
11281129
LOGGER.info("Secret {}:\n{}", kafkaUser.getMetadata().getName(), toYamlString(secret));
11291130
throw new RuntimeException("The Secret " + kafkaUser.getMetadata().getName() + " lacks the 'password' key");

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

+1
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ protected void setAllowedArguments(ClientType clientType) {
108108
allowedArguments.add(ClientArgument.VALUE_PREFIX);
109109
allowedArguments.add(ClientArgument.REPEATING_KEYS);
110110
allowedArguments.add(ClientArgument.USER);
111+
break;
111112
case CLI_KAFKA_VERIFIABLE_CONSUMER:
112113
allowedArguments.add(ClientArgument.BROKER_LIST);
113114
allowedArguments.add(ClientArgument.TOPIC);

0 commit comments

Comments
 (0)