Compile using Java 11 (linkedin#1217)

* Migrate from findbugs to spotbugs (findbugs won't work with java 11). This required separation of spotBugs gradle task in a separate run in CircleCI to avoid out of memory issues * Fix checkstyle config files discovery * Fix on prio 1 bug VO_VOLATILE_INCREMENT reported in AnomalyDetector.java * Upgrade org.eclipse.jetty.security and fixed tests to provide a non-null Credential
barthy1 · Jun 3, 2020 · 77f5e28 · 77f5e28
1 parent 595ad7d
commit 77f5e28
Show file tree

Hide file tree

Showing 14 changed files with 90 additions and 40 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -4,16 +4,21 @@ jobs:
 
   build:
     environment:
-      _JAVA_OPTIONS: "-Xms512m -Xmx512m"
+      _JAVA_OPTIONS: "-Xms512m -Xmx1g"
     working_directory: ~/workspace
     docker:
-      - image: circleci/openjdk:8-jdk
+      - image: circleci/openjdk:11-jdk
     steps:
       - checkout
       - restore_cache:
           key: dependency-cache-{{ checksum "build.gradle" }}
       - run:
-          command: ./gradlew --no-daemon -PmaxParallelForks=1 clean javadoc build
+          command: ./gradlew --no-daemon clean javadoc
+      - run:
+          # run static analysis tasks standalone to avoid OOME in CircleCI
+          command: ./gradlew --max-workers=1 --no-daemon analyze
+      - run:
+          command: ./gradlew --no-daemon -PmaxParallelForks=1 build
       - save_cache:
           key: dependency-cache-{{ checksum "build.gradle" }}
           paths:
@@ -32,7 +37,7 @@ jobs:
   publish:
     working_directory: ~/workspace
     docker:
-      - image: circleci/openjdk:8-jdk
+      - image: circleci/openjdk:11-jdk
     steps:
       - checkout
       - restore_cache:
@@ -55,4 +60,4 @@ workflows:
             branches:
               ignore: /.*/
             tags:
-              only: /^[0-9]+\.[0-9]+\.[0-9]+$/
+              only: /^[0-9]+\.[0-9]+\.[0-9]+$/
diff --git a/build.gradle b/build.gradle
@@ -8,6 +8,7 @@ plugins {
   id "idea"
   id "jacoco" // Java Code Coverage plugin
   id "com.github.ben-manes.versions" version "0.28.0"
+  id "com.github.spotbugs" version "4.2.4" apply false
 }
 
 group = 'com.linkedin.cruisecontrol'
@@ -76,7 +77,7 @@ subprojects {
 
   apply plugin: 'java'
   apply plugin: 'checkstyle'
-  apply plugin: 'findbugs'
+  apply plugin: "com.github.spotbugs"
   apply plugin: 'jacoco'
 
   task sourcesJar(type: Jar, dependsOn: classes) {
@@ -91,25 +92,38 @@ subprojects {
 
   //code quality and inspections
   checkstyle {
-    toolVersion = '8.1'
+    project.ext.checkstyleVersion = '8.20'
     ignoreFailures = false
-    configFile = rootProject.file('checkstyle/checkstyle.xml')
+    configDir = file("$rootDir/checkstyle")
   }
 
-  findbugs {
-    toolVersion = "3.0.1"
+  spotbugs {
+    toolVersion = '4.0.3'
     excludeFilter = file("$rootDir/gradle/findbugs-exclude.xml")
     ignoreFailures = false
+    jvmArgs = [ '-Xms512m' ]
+    maxHeapSize = '768m'
+    showProgress = true
   }
-
-  test.dependsOn('checkstyleMain', 'checkstyleTest', 'findbugsMain', 'findbugsTest')
-
-  tasks.withType(FindBugs) {
+  spotbugsMain {
+    reports {
+      xml.enabled = (project.hasProperty('xmlFindBugsReport'))
+      html.enabled = (!project.hasProperty('xmlFindBugsReport'))
+    }
+  }
+  spotbugsTest {
     reports {
-      xml.enabled (project.hasProperty('xmlFindBugsReport'))
-      html.enabled (!project.hasProperty('xmlFindBugsReport'))
+      xml.enabled = (project.hasProperty('xmlFindBugsReport'))
+      html.enabled = (!project.hasProperty('xmlFindBugsReport'))
     }
   }
+  // aggregated task for checkstyle and spotbugs static analyzers
+  task('analyze') {
+    dependsOn('checkstyleMain', 'checkstyleTest', 'spotbugsMain', 'spotbugsTest')
+    doLast {}
+  }
+
+  test.dependsOn('checkstyleMain', 'checkstyleTest', 'spotbugsMain', 'spotbugsTest')
 
   jar {
     from "$rootDir/LICENSE"
@@ -140,8 +154,8 @@ project(':cruise-control-core') {
     compile "org.slf4j:slf4j-api:1.7.30"
     compile 'junit:junit:4.13'
     compile 'org.apache.commons:commons-math3:3.6.1'
-    compile 'org.eclipse.jetty:jetty-servlet:9.4.26.v20200117'
-
+    compile 'org.eclipse.jetty:jetty-servlet:9.4.29.v20200521'
+    implementation group: 'com.google.code.findbugs', name: 'jsr305', version: '3.0.2'
     testOutput sourceSets.test.output
   }
 
@@ -220,7 +234,7 @@ project(':cruise-control') {
     compile 'org.apache.commons:commons-math3:3.6.1'
     compile 'org.apache.httpcomponents:httpclient:4.5.11'
     compile 'com.google.code.gson:gson:2.8.6'
-    compile 'org.eclipse.jetty:jetty-server:9.4.26.v20200117'
+    compile 'org.eclipse.jetty:jetty-server:9.4.29.v20200521'
     compile 'io.dropwizard.metrics:metrics-core:3.2.6'
     compile 'com.nimbusds:nimbus-jose-jwt:8.5'
     compile 'com.101tec:zkclient:0.11'
@@ -237,6 +251,8 @@ project(':cruise-control') {
     testCompile 'org.bouncycastle:bcpkix-jdk15on:1.64'
     testCompile 'org.apache.kerby:kerb-simplekdc:2.0.0'
 
+    implementation group: 'com.google.code.findbugs', name: 'jsr305', version: '3.0.2'
+
   }
 
   publishing {
@@ -341,6 +357,8 @@ project(':cruise-control-metrics-reporter') {
     testCompile "org.apache.kafka:kafka-clients:$kafkaVersion"
     testCompile 'commons-io:commons-io:2.6'
     testOutput sourceSets.test.output
+
+    implementation group: 'com.google.code.findbugs', name: 'jsr305', version: '3.0.2'
   }
 
   publishing {

diff --git a/checkstyle/checkstyle.xml b/checkstyle/checkstyle.xml
@@ -11,7 +11,7 @@ LinkedIn Java style.
 -->
 <module name="Checker">
     <module name="SuppressionFilter">
-        <property name="file" value="checkstyle/suppressions.xml"/>
+        <property name="file" value="${config_loc}/suppressions.xml"/>
     </module>
 
     <!-- header -->
@@ -22,7 +22,6 @@ LinkedIn Java style.
     <property name="severity" value="${checkstyle.severity}" default="error"/>
 
     <module name="TreeWalker">
-        <module name="FileContentsHolder"/>
 
         <!-- ANNOTATIONS -->
 

diff --git a/...kedin/kafka/cruisecontrol/metricsreporter/utils/CCKafkaClientsIntegrationTestHarness.java b/...kedin/kafka/cruisecontrol/metricsreporter/utils/CCKafkaClientsIntegrationTestHarness.java
@@ -24,6 +24,7 @@ public void setUp() {
     super.setUp();
   }
 
+  @javax.annotation.Nonnull
   protected Producer<String, String> createProducer(Properties overrides) {
     Properties props = getProducerProperties(overrides);
     return new KafkaProducer<>(props);

diff --git a/...ontrol/src/main/java/com/linkedin/kafka/cruisecontrol/config/constants/MonitorConfig.java b/...ontrol/src/main/java/com/linkedin/kafka/cruisecontrol/config/constants/MonitorConfig.java
@@ -197,6 +197,7 @@ private MonitorConfig() {
    * @deprecated (i.e. cannot be configured to a value other than 1).
    * <code>num.metric.fetchers</code>
    */
+  @Deprecated
   public static final String NUM_METRIC_FETCHERS_CONFIG = "num.metric.fetchers";
   public static final int DEFAULT_NUM_METRIC_FETCHERS = 1;
   public static final String NUM_METRIC_FETCHERS_DOC = "The number of metric fetchers to fetch from the Kafka cluster.";

diff --git a/cruise-control/src/main/java/com/linkedin/kafka/cruisecontrol/detector/AnomalyDetector.java b/cruise-control/src/main/java/com/linkedin/kafka/cruisecontrol/detector/AnomalyDetector.java
@@ -24,6 +24,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Random;
+import java.util.concurrent.atomic.AtomicLong;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.PriorityBlockingQueue;
@@ -74,7 +75,7 @@ public class AnomalyDetector {
   private final List<String> _selfHealingGoals;
   private final ExecutorService _anomalyLoggerExecutor;
   private volatile Anomaly _anomalyInProgress;
-  private volatile long _numCheckedWithDelay;
+  private AtomicLong _numCheckedWithDelay;
   private final Object _shutdownLock;
 
   public AnomalyDetector(KafkaCruiseControl kafkaCruiseControl,
@@ -118,7 +119,7 @@ public AnomalyDetector(KafkaCruiseControl kafkaCruiseControl,
     _anomalyLoggerExecutor =
         Executors.newSingleThreadScheduledExecutor(new KafkaCruiseControlThreadFactory("AnomalyLogger", true, null));
     _anomalyInProgress = null;
-    _numCheckedWithDelay = 0L;
+    _numCheckedWithDelay = new AtomicLong();
     _shutdownLock = new Object();
     // Register gauge sensors.
     registerGaugeSensors(dropwizardMetricRegistry);
@@ -162,7 +163,7 @@ public AnomalyDetector(KafkaCruiseControl kafkaCruiseControl,
     _anomalyLoggerExecutor =
         Executors.newSingleThreadScheduledExecutor(new KafkaCruiseControlThreadFactory("AnomalyLogger", true, null));
     _anomalyInProgress = null;
-    _numCheckedWithDelay = 0L;
+    _numCheckedWithDelay = new AtomicLong();
     _shutdownLock = new Object();
     // Add anomaly detector state
     _anomalyDetectorState = new AnomalyDetectorState(new SystemTime(), new HashMap<>(KafkaAnomalyType.cachedValues().size()), 10, null);
@@ -302,7 +303,7 @@ public boolean setSelfHealingFor(AnomalyType anomalyType, boolean isSelfHealingE
    * @return Number of anomalies checked with delay.
    */
   public long numCheckedWithDelay() {
-    return _numCheckedWithDelay;
+    return _numCheckedWithDelay.get();
   }
 
   /**
@@ -458,7 +459,7 @@ private void postProcessAnomalyInProgress(long delayMs) {
             LOG.debug("Skip delayed checking anomaly {}, because anomaly detector is shutting down.", _anomalyInProgress);
           } else {
             LOG.debug("Scheduling broker failure detection with delay of {} ms", delayMs);
-            _numCheckedWithDelay++;
+            _numCheckedWithDelay.incrementAndGet();
             _detectorScheduler.schedule(() -> _brokerFailureDetector.detectBrokerFailures(false), delayMs, TimeUnit.MILLISECONDS);
             _anomalyDetectorState.onAnomalyHandle(_anomalyInProgress, AnomalyState.Status.CHECK_WITH_DELAY);
           }

diff --git a/cruise-control/src/main/java/com/linkedin/kafka/cruisecontrol/detector/DiskFailures.java b/cruise-control/src/main/java/com/linkedin/kafka/cruisecontrol/detector/DiskFailures.java
@@ -64,7 +64,7 @@ public String toString() {
     StringBuilder sb = new StringBuilder().append("{\n");
     _failedDisksByBroker.forEach((brokerId, failures) -> {
       failures.forEach((logdir, eventTime) -> {
-        sb.append(String.format("\tDisk %s on broker %d failed at %s\n ", logdir, brokerId, toDateString(eventTime)));
+        sb.append(String.format("\tDisk %s on broker %d failed at %s%n ", logdir, brokerId, toDateString(eventTime)));
       });
     });
     sb.append("}");
@@ -93,4 +93,4 @@ public void configure(Map<String, ?> configs) {
                                                                  _anomalyId.toString(),
                                                                  reasonSupplier());
   }
-}
+}
diff --git a/...ava/com/linkedin/kafka/cruisecontrol/monitor/sampling/MetricSamplerPartitionAssignor.java b/...ava/com/linkedin/kafka/cruisecontrol/monitor/sampling/MetricSamplerPartitionAssignor.java
@@ -24,6 +24,7 @@ public interface MetricSamplerPartitionAssignor extends CruiseControlConfigurabl
    * @param numFetchers    The number of metric fetchers.
    * @return A List of partition assignment for each of the fetchers.
    */
+  @Deprecated
   List<Set<TopicPartition>> assignPartitions(Cluster cluster, int numFetchers);
 
   /**

diff --git a/...ontrol/src/main/java/com/linkedin/kafka/cruisecontrol/servlet/security/SecurityUtils.java b/...ontrol/src/main/java/com/linkedin/kafka/cruisecontrol/servlet/security/SecurityUtils.java
@@ -0,0 +1,19 @@
+/*
+ * Copyright 2020 LinkedIn Corp. Licensed under the BSD 2-Clause License (the "License"). See License in the project root for license information.
+ */
+
+package com.linkedin.kafka.cruisecontrol.servlet.security;
+
+import org.eclipse.jetty.util.security.Credential;
+
+public final class SecurityUtils {
+    public static final Credential NO_CREDENTIAL = new Credential() {
+        @Override
+        public boolean check(Object credentials) {
+            return false;
+        }
+    };
+
+    private SecurityUtils() {
+    }
+}
diff --git a/...n/kafka/cruisecontrol/servlet/security/trustedproxy/TrustedProxyAuthorizationService.java b/...n/kafka/cruisecontrol/servlet/security/trustedproxy/TrustedProxyAuthorizationService.java
@@ -5,6 +5,7 @@
 package com.linkedin.kafka.cruisecontrol.servlet.security.trustedproxy;
 
 import com.linkedin.kafka.cruisecontrol.servlet.security.DefaultRoleSecurityProvider;
+import com.linkedin.kafka.cruisecontrol.servlet.security.SecurityUtils;
 import org.eclipse.jetty.security.UserStore;
 import org.eclipse.jetty.security.authentication.AuthorizationService;
 import org.eclipse.jetty.server.UserIdentity;
@@ -25,7 +26,7 @@ public class TrustedProxyAuthorizationService extends AbstractLifeCycle implemen
 
   TrustedProxyAuthorizationService(List<String> userNames, String trustedProxyIpPattern) {
     _adminUserStore = new UserStore();
-    userNames.forEach(u -> _adminUserStore.addUser(u, null, new String[] { DefaultRoleSecurityProvider.ADMIN }));
+    userNames.forEach(u -> _adminUserStore.addUser(u, SecurityUtils.NO_CREDENTIAL, new String[] { DefaultRoleSecurityProvider.ADMIN }));
     if (trustedProxyIpPattern != null) {
       _trustedProxyIpPattern = Pattern.compile(trustedProxyIpPattern);
     } else {

diff --git a/...test/java/com/linkedin/kafka/cruisecontrol/servlet/security/jwt/JwtAuthenticatorTest.java b/...test/java/com/linkedin/kafka/cruisecontrol/servlet/security/jwt/JwtAuthenticatorTest.java
@@ -3,6 +3,7 @@
  */
 package com.linkedin.kafka.cruisecontrol.servlet.security.jwt;
 
+import com.linkedin.kafka.cruisecontrol.servlet.security.SecurityUtils;
 import com.linkedin.kafka.cruisecontrol.servlet.security.UserStoreAuthorizationService;
 import org.eclipse.jetty.http.HttpHeader;
 import org.eclipse.jetty.http.HttpMethod;
@@ -112,7 +113,7 @@ public void testRedirect() throws IOException, ServerAuthException {
   @Test
   public void testSuccessfulLogin() throws Exception {
     UserStore testUserStore = new UserStore();
-    testUserStore.addUser(TEST_USER, null, new String[] {USER_ROLE});
+    testUserStore.addUser(TEST_USER, SecurityUtils.NO_CREDENTIAL, new String[]{USER_ROLE});
     TokenGenerator.TokenAndKeys tokenAndKeys = TokenGenerator.generateToken(TEST_USER);
     JwtLoginService loginService = new JwtLoginService(new UserStoreAuthorizationService(testUserStore), tokenAndKeys.publicKey(), null);
 
@@ -147,7 +148,7 @@ public void testSuccessfulLogin() throws Exception {
   @Test
   public void testFailedLoginWithUserNotFound() throws Exception {
     UserStore testUserStore = new UserStore();
-    testUserStore.addUser(TEST_USER_2, null, new String[] {USER_ROLE});
+    testUserStore.addUser(TEST_USER_2, SecurityUtils.NO_CREDENTIAL, new String[] {USER_ROLE});
     TokenGenerator.TokenAndKeys tokenAndKeys = TokenGenerator.generateToken(TEST_USER);
     JwtLoginService loginService = new JwtLoginService(new UserStoreAuthorizationService(testUserStore), tokenAndKeys.publicKey(), null);
 
@@ -181,7 +182,7 @@ public void testFailedLoginWithUserNotFound() throws Exception {
   @Test
   public void testFailedLoginWithInvalidToken() throws Exception {
     UserStore testUserStore = new UserStore();
-    testUserStore.addUser(TEST_USER_2, null, new String[] {USER_ROLE});
+    testUserStore.addUser(TEST_USER_2, SecurityUtils.NO_CREDENTIAL, new String[] {USER_ROLE});
     TokenGenerator.TokenAndKeys tokenAndKeys = TokenGenerator.generateToken(TEST_USER);
     TokenGenerator.TokenAndKeys tokenAndKeys2 = TokenGenerator.generateToken(TEST_USER);
     JwtLoginService loginService = new JwtLoginService(new UserStoreAuthorizationService(testUserStore), tokenAndKeys.publicKey(), null);

diff --git a/.../test/java/com/linkedin/kafka/cruisecontrol/servlet/security/jwt/JwtLoginServiceTest.java b/.../test/java/com/linkedin/kafka/cruisecontrol/servlet/security/jwt/JwtLoginServiceTest.java
@@ -4,6 +4,7 @@
 
 package com.linkedin.kafka.cruisecontrol.servlet.security.jwt;
 
+import com.linkedin.kafka.cruisecontrol.servlet.security.SecurityUtils;
 import com.linkedin.kafka.cruisecontrol.servlet.security.UserStoreAuthorizationService;
 import com.nimbusds.jwt.SignedJWT;
 import org.eclipse.jetty.security.UserStore;
@@ -34,7 +35,7 @@ public class JwtLoginServiceTest {
   @Test
   public void testValidateTokenSuccessfully() throws Exception {
     UserStore testUserStore = new UserStore();
-    testUserStore.addUser(TEST_USER, null, new String[] {"USER"});
+    testUserStore.addUser(TEST_USER, SecurityUtils.NO_CREDENTIAL, new String[] {"USER"});
     TokenGenerator.TokenAndKeys tokenAndKeys = TokenGenerator.generateToken(TEST_USER);
     JwtLoginService loginService = new JwtLoginService(new UserStoreAuthorizationService(testUserStore), tokenAndKeys.publicKey(), null);
 
@@ -52,7 +53,7 @@ public void testValidateTokenSuccessfully() throws Exception {
   @Test
   public void testFailSignatureValidation() throws Exception {
     UserStore testUserStore = new UserStore();
-    testUserStore.addUser(TEST_USER, null, new String[] {"USER"});
+    testUserStore.addUser(TEST_USER, SecurityUtils.NO_CREDENTIAL, new String[] {"USER"});
     TokenGenerator.TokenAndKeys tokenAndKeys = TokenGenerator.generateToken(TEST_USER);
     TokenGenerator.TokenAndKeys tokenAndKeys2 = TokenGenerator.generateToken(TEST_USER); // this will be signed with a different key
     JwtLoginService loginService = new JwtLoginService(new UserStoreAuthorizationService(testUserStore), tokenAndKeys2.publicKey(), null);
@@ -67,7 +68,7 @@ public void testFailSignatureValidation() throws Exception {
   @Test
   public void testFailAudienceValidation() throws Exception {
     UserStore testUserStore = new UserStore();
-    testUserStore.addUser(TEST_USER, null, new String[] {"USER"});
+    testUserStore.addUser(TEST_USER, SecurityUtils.NO_CREDENTIAL, new String[] {"USER"});
     TokenGenerator.TokenAndKeys tokenAndKeys = TokenGenerator.generateToken(TEST_USER, Arrays.asList("A", "B"));
     JwtLoginService loginService = new JwtLoginService(
         new UserStoreAuthorizationService(testUserStore), tokenAndKeys.publicKey(), Arrays.asList("C", "D"));
@@ -82,7 +83,7 @@ public void testFailAudienceValidation() throws Exception {
   @Test
   public void testFailExpirationValidation() throws Exception {
     UserStore testUserStore = new UserStore();
-    testUserStore.addUser(TEST_USER, null, new String[] {"USER"});
+    testUserStore.addUser(TEST_USER, SecurityUtils.NO_CREDENTIAL, new String[] {"USER"});
     TokenGenerator.TokenAndKeys tokenAndKeys = TokenGenerator.generateToken(TEST_USER, 1L);
     JwtLoginService loginService = new JwtLoginService(new UserStoreAuthorizationService(testUserStore), tokenAndKeys.publicKey(), null);
 
@@ -96,7 +97,7 @@ public void testFailExpirationValidation() throws Exception {
   @Test
   public void testRevalidateTokenPasses() throws Exception {
     UserStore testUserStore = new UserStore();
-    testUserStore.addUser(TEST_USER, null, new String[] {"USER"});
+    testUserStore.addUser(TEST_USER, SecurityUtils.NO_CREDENTIAL, new String[] {"USER"});
     TokenGenerator.TokenAndKeys tokenAndKeys = TokenGenerator.generateToken(TEST_USER);
     JwtLoginService loginService = new JwtLoginService(new UserStoreAuthorizationService(testUserStore), tokenAndKeys.publicKey(), null);
 
@@ -115,7 +116,7 @@ public void testRevalidateTokenPasses() throws Exception {
   @Test
   public void testRevalidateTokenFails() throws Exception {
     UserStore testUserStore = new UserStore();
-    testUserStore.addUser(TEST_USER, null, new String[] {"USER"});
+    testUserStore.addUser(TEST_USER, SecurityUtils.NO_CREDENTIAL, new String[] {"USER"});
     Instant now = Instant.now();
     TokenGenerator.TokenAndKeys tokenAndKeys = TokenGenerator.generateToken(TEST_USER, now.plusSeconds(10).toEpochMilli());
     Clock fixedClock = Clock.fixed(now, ZoneOffset.UTC);