Re: [PR] ci: enable incremental dependency-check updates (druid)

["FrankChen021 (via GitHub)" <gi...@apache.org>]

FrankChen021 commented on code in PR #20126:
URL: https://github.com/apache/druid/pull/20126#discussion_r4367208883


##########
.github/workflows/cron-job-its.yml:
##########
@@ -49,20 +54,76 @@ jobs:
           distribution: 'zulu'
           cache: maven
 
+      - name: Restore Dependency-Check data
+        id: dependency-check-cache
+        uses: actions/cache/restore@v4
+        with:
+          path: ~/.cache/dependency-check
+          key: dependency-check-data-${{ runner.os }}-odc-${{ env.DEPENDENCY_CHECK_CACHE_VERSION }}-${{ github.run_id }}
+          restore-keys: |
+            dependency-check-data-${{ runner.os }}-odc-${{ env.DEPENDENCY_CHECK_CACHE_VERSION }}-
+
+      - name: Report Dependency-Check cache status
+        run: |
+          echo "Dependency-Check cache-hit=${{ steps.dependency-check-cache.outputs.cache-hit }}"
+          echo "Dependency-Check cache-matched-key=${{ steps.dependency-check-cache.outputs.cache-matched-key }}"
+
       - name: maven build # needed to rebuild in case of maven snapshot resolution fails
         run: ./mvnw clean install -P dist -P skip-static-checks,skip-tests -Dmaven.javadoc.skip=true -Dcyclonedx.skip=true -Dweb.console.skip=true
 
+      - name: Update Dependency-Check data
+        id: dependency-check-update
+        env:
+          NVD_API_KEY: ${{ secrets.NVD_API_KEY }}
+        run: |
+          if [ -n "$NVD_API_KEY" ]; then
+            echo "NVD API key is available to the update"
+          else
+            echo "NVD API key is unavailable to the update"
+          fi
+          ./mvnw -B dependency-check:update-only \
+            -DdataDirectory="$HOME/.cache/dependency-check" \
+            -DnvdValidForHours=24
+
       - name: security vulnerabilities check
+        if: steps.dependency-check-update.outcome == 'success'
         env:
           OSS_INDEX_USERNAME: ${{ secrets.OSS_INDEX_USERNAME }}
           OSS_INDEX_PASSWORD: ${{ secrets.OSS_INDEX_PASSWORD }}
           NVD_API_KEY: ${{ secrets.NVD_API_KEY }}
         run: |
-          ./mvnw -B dependency-check:purge dependency-check:check -DnvdApiKey=$NVD_API_KEY -DossIndexUsername=$OSS_INDEX_USERNAME -DossIndexPassword=$OSS_INDEX_PASSWORD || { echo "
-          The OWASP dependency check has found security vulnerabilities. Please use a newer version
-          of the dependency that does not have vulnerabilities. To see a report run
-          './mvnw dependency-check:check'
-          If the analysis has false positives,
-          they can be suppressed by adding entries to owasp-dependency-check-suppressions.xml (for more
-          information, see https://jeremylong.github.io/DependencyCheck/general/suppression.html).
-          " && false; }
+          if [ -n "$NVD_API_KEY" ]; then
+            echo "NVD API key is available to the scan"
+          else
+            echo "NVD API key is unavailable to the scan"
+          fi
+          # --fail-at-end skips modules that depend on a failed module. Continue through the
+          # entire reactor, then aggregate the module results so CI still fails when findings exist.
+          dependency_check_log="$RUNNER_TEMP/dependency-check.log"
+          set +e
+          ./mvnw -B --fail-never dependency-check:check -DautoUpdate=false -DdataDirectory="$HOME/.cache/dependency-check" -DossIndexUsername="$OSS_INDEX_USERNAME" -DossIndexPassword="$OSS_INDEX_PASSWORD" 2>&1 | tee "$dependency_check_log"
+          pipeline_status=("${PIPESTATUS[@]}")
+          set -e
+          maven_status=${pipeline_status[0]}
+          tee_status=${pipeline_status[1]}
+          if [ "$maven_status" -ne 0 ] || [ "$tee_status" -ne 0 ] \
+             || grep -Eq '^\[INFO\] BUILD FAILURE$|^\[ERROR\] Failed to execute goal org\.owasp:dependency-check-maven:' "$dependency_check_log"; then
+            echo "
+            The OWASP dependency check has found security vulnerabilities or failed in one or more modules. Please use a newer version
+            of the dependency that does not have vulnerabilities. To see a report run
+            ./mvnw dependency-check:check
+            If the analysis has false positives,
+            they can be suppressed by adding entries to owasp-dependency-check-suppressions.xml (for more
+            information, see https://jeremylong.github.io/DependencyCheck/general/suppression.html).
+            "
+            exit 1
+          fi
+
+      - name: Save Dependency-Check data
+        # A pull_request run uses an isolated merge-ref cache that cannot be
+        # reused by branch, scheduled, or workflow_dispatch runs.
+        if: steps.dependency-check-update.outcome == 'success' && github.event_name != 'pull_request'

Review Comment:
   [P1] Cache save is skipped after a failed vulnerability scan
   
   **Finding:** When the update step succeeds but the vulnerability scan exits nonzero, line 125 is evaluated with GitHub Actions' implicit success() guard because it contains no status-check function. The save action is therefore skipped, leaving the refreshed database uncached and causing later scheduled or manual runs to repeatedly update from the older cache instead of advancing the incremental cache.
   
   **Suggestion:** Make the save condition explicitly run after a failed scan while excluding cancellation, while retaining the existing update-success and non-pull-request predicates.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org