emit a warning when threshold values are configured but synchronous m…

…ode is disabled

CHANGELOG.md

@@ -21,6 +21,7 @@
 
 ### 🐞 Bugs Fixed
 - The settings for the threshold values are now only visible when synchronous mode is enabled. This will hopefully avoid misunderstandings/misconfigurations.
+- A warning is emitted when threshold values are configured but synchronous mode is disabled.
 
 ## v4.3.1 - 2023-04-12
 ### ⚠ Breaking

pom.xml

@@ -46,6 +46,7 @@
         <maven.javadoc.skip>true</maven.javadoc.skip>
         <npm.path>npm</npm.path>
         <npm.suffix />
+        <spotbugs.excludeFilterFile>spotbugs-excludes.xml</spotbugs.excludeFilterFile>
     </properties>
 
     <profiles>

spotbugs-excludes.xml

@@ -0,0 +1,13 @@
+<FindBugsFilter
+    xmlns="https://github.com/spotbugs/filter/4.8.3"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:schemaLocation="https://github.com/spotbugs/filter/4.8.3 https://raw.githubusercontent.com/spotbugs/spotbugs/4.8.3/spotbugs/etc/findbugsfilter.xsd">
+    <Match>
+        <Class name="org.jenkinsci.plugins.DependencyTrack.model.RiskGate" />
+        <Or>
+            <Method name="equals" />
+            <Method name="hashCode" />
+        </Or>
+        <Bug pattern="RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE" />
+    </Match>
+</FindBugsFilter>

src/main/java/org/jenkinsci/plugins/DependencyTrack/DependencyTrackPublisher.java

@@ -312,7 +312,7 @@ public void perform(@NonNull final Run<?, ?> run, @NonNull final FilePath worksp
             throw new AbortException(Messages.Builder_Upload_Failed());
         }
 
-        // add ResultLinkAction even if it may not contain a projectId. but we want to store name version for the future.
+        // add ResultLinkAction even if it may not contain a projectId. but we want to store name and version for the future.
         final ResultLinkAction linkAction = new ResultLinkAction(getEffectiveFrontendUrl(), projectId);
         linkAction.setProjectName(effectiveProjectName);
         linkAction.setProjectVersion(effectiveProjectVersion);
@@ -322,12 +322,19 @@ public void perform(@NonNull final Run<?, ?> run, @NonNull final FilePath worksp
 
         updateProjectProperties(logger, apiClient, effectiveProjectName, effectiveProjectVersion);
 
+        final var thresholds = getThresholds();
         if (synchronous && StringUtils.isNotBlank(uploadResult.getToken())) {
-            publishAnalysisResult(logger, apiClient, uploadResult.getToken(), run, effectiveProjectName, effectiveProjectVersion);
+            final var severityDistribution = publishAnalysisResult(logger, apiClient, uploadResult.getToken(), run, effectiveProjectName, effectiveProjectVersion);
+            if (thresholds.hasValues()) {
+                evaluateRiskGates(run, logger, severityDistribution, thresholds);
+            }
+        }
+        if (!synchronous && thresholds.hasValues()) {
+            logger.log(Messages.Builder_Threshold_NoSync());
         }
     }
 
-    private void publishAnalysisResult(final ConsoleLogger logger, final ApiClient apiClient, final String token, final Run<?, ?> build, final String effectiveProjectName, final String effectiveProjectVersion) throws InterruptedException, ApiClientException, AbortException {
+    private SeverityDistribution publishAnalysisResult(final ConsoleLogger logger, final ApiClient apiClient, final String token, final Run<?, ?> build, final String effectiveProjectName, final String effectiveProjectVersion) throws InterruptedException, ApiClientException, AbortException {
         final long timeout = System.currentTimeMillis() + (60000L * getEffectivePollingTimeout());
         final long interval = 1000L * getEffectivePollingInterval();
         logger.log(Messages.Builder_Polling());
@@ -356,22 +363,21 @@ private void publishAnalysisResult(final ConsoleLogger logger, final ApiClient a
         linkAction.setProjectVersion(effectiveProjectVersion);
         build.addOrReplaceAction(linkAction);
 
+        return severityDistribution;
+    }
+
+    private void evaluateRiskGates(final Run<?, ?> build, final ConsoleLogger logger, final SeverityDistribution currentDistribution, final Thresholds thresholds) throws AbortException {
         // Get previous results and evaluate to thresholds
-        final SeverityDistribution previousSeverityDistribution = Optional.ofNullable(getPreviousBuildWithAnalysisResult(build))
+        final SeverityDistribution previousDistribution = Optional.ofNullable(getPreviousBuildWithAnalysisResult(build))
                 .map(previousBuild -> previousBuild.getAction(ResultAction.class))
                 .map(ResultAction::getSeverityDistribution)
                 .orElse(null);
-
-        evaluateRiskGates(build, logger, severityDistribution, previousSeverityDistribution);
-    }
-
-    private void evaluateRiskGates(final Run<?, ?> build, final ConsoleLogger logger, final SeverityDistribution currentDistribution, final SeverityDistribution previousDistribution) throws AbortException {
         if (previousDistribution != null) {
             logger.log(Messages.Builder_Threshold_ComparingTo(previousDistribution.getBuildNumber()));
         } else {
             logger.log(Messages.Builder_Threshold_NoComparison());
         }
-        final RiskGate riskGate = new RiskGate(getThresholds());
+        final RiskGate riskGate = new RiskGate(thresholds);
         final Result result = riskGate.evaluate(currentDistribution, previousDistribution);
         if (result.isWorseOrEqualTo(Result.UNSTABLE) && result.isCompleteBuild()) {
             logger.log(Messages.Builder_Threshold_Exceed());

src/main/java/org/jenkinsci/plugins/DependencyTrack/model/RiskGate.java

@@ -15,11 +15,11 @@
  */
 package org.jenkinsci.plugins.DependencyTrack.model;
 
+import edu.umd.cs.findbugs.annotations.NonNull;
+import edu.umd.cs.findbugs.annotations.Nullable;
 import hudson.model.Result;
-
 import java.io.Serializable;
 import lombok.EqualsAndHashCode;
-import lombok.NonNull;
 import lombok.RequiredArgsConstructor;
 import lombok.ToString;
 
@@ -40,7 +40,7 @@ public class RiskGate implements Serializable {
      * @param previousDistribution previousDistribution
      * @return a Result
      */
-    public Result evaluate(@NonNull final SeverityDistribution currentDistribution, final SeverityDistribution previousDistribution) {
+    public Result evaluate(@NonNull final SeverityDistribution currentDistribution, @Nullable final SeverityDistribution previousDistribution) {
 
         Result result = Result.SUCCESS;
         if ((thresholds.totalFindings.failedCritical != null && currentDistribution.getCritical() > 0 && currentDistribution.getCritical() >= thresholds.totalFindings.failedCritical)

src/main/java/org/jenkinsci/plugins/DependencyTrack/model/Thresholds.java

@@ -23,27 +23,17 @@
 @ToString
 public class Thresholds implements Serializable {
 
-    public final TotalFindings totalFindings = new TotalFindings();
-    public final NewFindings newFindings = new NewFindings();
+    public final ThresholdValues totalFindings = new ThresholdValues();
+    public final ThresholdValues newFindings = new ThresholdValues();
 
-    @EqualsAndHashCode
-    @ToString
-    public static class TotalFindings implements Serializable {
-        public Integer unstableCritical;
-        public Integer unstableHigh;
-        public Integer unstableMedium;
-        public Integer unstableLow;
-        public Integer unstableUnassigned;
-        public Integer failedCritical;
-        public Integer failedHigh;
-        public Integer failedMedium;
-        public Integer failedLow;
-        public Integer failedUnassigned;
+    public boolean hasValues() {
+        return totalFindings.hasValues() || newFindings.hasValues();
     }
 
     @EqualsAndHashCode
     @ToString
-    public static class NewFindings implements Serializable {
+    public static class ThresholdValues implements Serializable {
+
         public Integer unstableCritical;
         public Integer unstableHigh;
         public Integer unstableMedium;
@@ -54,5 +44,25 @@ public static class NewFindings implements Serializable {
         public Integer failedMedium;
         public Integer failedLow;
         public Integer failedUnassigned;
+
+        public boolean hasValues() {
+            return hasUnstableValues() || hasFailedValues();
+        }
+
+        private boolean hasUnstableValues() {
+            return unstableCritical != null
+                    || unstableHigh != null
+                    || unstableMedium != null
+                    || unstableLow != null
+                    || unstableUnassigned != null;
+        }
+
+        private boolean hasFailedValues() {
+            return failedCritical != null
+                    || failedHigh != null
+                    || failedMedium != null
+                    || failedLow != null
+                    || failedUnassigned != null;
+        }
     }
 }

src/main/resources/org/jenkinsci/plugins/DependencyTrack/Messages.properties

@@ -46,6 +46,7 @@ Builder.Findings.Processing=Processing findings
 Builder.Threshold.Exceed=Findings exceed configured thresholds
 Builder.Threshold.ComparingTo=Evaluating new findings against previous build #{0}
 Builder.Threshold.NoComparison=This is the first build. Findings will not be compared to a previous build.
+Builder.Threshold.NoSync=Warning: You have configured threshold values, but the synchronous publishing mode is disabled! The threshold values are not evaluated!
 Builder.Upload.Failed=Uploading artifact failed
 Builder.Connection.Failed=Could not connect to Dependency-Track. Please check the plugin configuration.
 

src/main/resources/org/jenkinsci/plugins/DependencyTrack/Messages_de.properties

@@ -46,6 +46,7 @@ Builder.Findings.Processing=Verarbeite Ergebnisse
 Builder.Threshold.Exceed=Ergebnisse \u00fcberschreiten konfigurierte Schwellwerte
 Builder.Threshold.ComparingTo=Bewerte neue Ergebnisse im Vergleich zu fr\u00fcherem Lauf #{0}
 Builder.Threshold.NoComparison=Dies ist der erste Lauf. Die Ergebnisse werden nicht mit einem fr\u00fcheren Lauf verglichen.
+Builder.Threshold.NoSync=Achtung: Sie haben Schwellenwerte konfiguriert, aber der synchrone Ver\u00f6ffentlichungsmodus ist deaktiviert! Die Schwellenwerte werden nicht ausgewertet!
 Builder.Upload.Failed=Hochladen des Artefakts fehlgeschlagen
 Builder.Connection.Failed=Es konnte keine Verbindung mit Dependency-Track hergestellt werden! Bitte pr\u00fcfen Sie die Plugin-Konfiguration.
 

src/test/java/org/jenkinsci/plugins/DependencyTrack/DependencyTrackPublisherTest.java

@@ -164,6 +164,7 @@ void testPerformAsync(@TempDir Path tmpWork) throws IOException, InterruptedExce
         uut.setProjectId("uuid-1");
         uut.setDependencyTrackApiKey(apikeyId);
         uut.setProjectProperties(new ProjectProperties());
+        uut.setUnstableTotalCritical(1);
 
         when(client.upload(eq("uuid-1"), isNull(), isNull(), any(FilePath.class), eq(false)))
                 .thenReturn(new UploadResult(true))
@@ -203,6 +204,7 @@ void testPerformSync(@TempDir Path tmpWork) throws IOException, InterruptedExcep
         DependencyTrackPublisher uut = new DependencyTrackPublisher(tmp.getName(), true, clientFactory);
         uut.setProjectId("uuid-1");
         uut.setDependencyTrackApiKey(apikeyId);
+        uut.setUnstableTotalCritical(1);
 
         when(client.upload(eq("uuid-1"), isNull(), isNull(), any(FilePath.class), eq(false))).thenReturn(new UploadResult(true, "token-1"));
         when(client.isTokenBeingProcessed("token-1")).thenReturn(Boolean.TRUE).thenReturn(Boolean.FALSE);
@@ -225,6 +227,29 @@ void testPerformSync(@TempDir Path tmpWork) throws IOException, InterruptedExcep
         verify(buildWithResultAction, times(2)).getAction(ResultAction.class);
     }
 
+    @Test
+    void testPerformSyncNoThresholds(@TempDir Path tmpWork) throws IOException, InterruptedException {
+        File tmp = tmpWork.resolve("bom.xml").toFile();
+        tmp.createNewFile();
+        FilePath workDir = new FilePath(tmpWork.toFile());
+        DependencyTrackPublisher uut = new DependencyTrackPublisher(tmp.getName(), true, clientFactory);
+        uut.setProjectId("uuid-1");
+        uut.setDependencyTrackApiKey(apikeyId);
+
+        when(client.upload(eq("uuid-1"), isNull(), isNull(), any(FilePath.class), eq(false))).thenReturn(new UploadResult(true, "token-1"));
+        when(client.isTokenBeingProcessed("token-1")).thenReturn(Boolean.TRUE).thenReturn(Boolean.FALSE);
+        when(client.getFindings("uuid-1")).thenReturn(List.of());
+
+        Run abortedBuild = mock(Run.class);
+        when(abortedBuild.getResult()).thenReturn(Result.NOT_BUILT);
+        when(build.getPreviousSuccessfulBuild()).thenReturn(abortedBuild);
+
+        assertThatCode(() -> uut.perform(build, workDir, env, launcher, listener)).doesNotThrowAnyException();
+        verify(client, times(2)).isTokenBeingProcessed("token-1");
+        verify(client).getFindings("uuid-1");
+        verify(abortedBuild, never()).getAction(ResultAction.class);
+    }
+
     @Test
     void testPerformSyncWithoutProjectId(@TempDir Path tmpWork) throws IOException, InterruptedException {
         File tmp = tmpWork.resolve("bom.xml").toFile();

src/test/java/org/jenkinsci/plugins/DependencyTrack/model/ThresholdsTest.java

@@ -0,0 +1,44 @@
+/*
+ * Copyright 2024 OWASP.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.jenkinsci.plugins.DependencyTrack.model;
+
+import org.junit.jupiter.api.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+class ThresholdsTest {
+
+    @Test
+    void hasValuesTest() {
+        var uut = new Thresholds();
+        assertThat(uut.hasValues()).isFalse();
+
+        uut.newFindings.failedCritical = 1;
+        assertThat(uut.hasValues()).isTrue();
+
+        uut = new Thresholds();
+        uut.newFindings.unstableUnassigned = 1;
+        assertThat(uut.hasValues()).isTrue();
+
+        uut = new Thresholds();
+        uut.totalFindings.failedHigh = 1;
+        assertThat(uut.hasValues()).isTrue();
+
+        uut = new Thresholds();
+        uut.totalFindings.unstableMedium = 1;
+        assertThat(uut.hasValues()).isTrue();
+    }
+}