Skip to content

fix(ci): Skip OWASP dep check for doc-only changes#27305

Merged
yhwang merged 1 commit intoprestodb:masterfrom
yhwang:fix-doc-only-ci-4-owasp-dep-check
Mar 11, 2026
Merged

fix(ci): Skip OWASP dep check for doc-only changes#27305
yhwang merged 1 commit intoprestodb:masterfrom
yhwang:fix-doc-only-ci-4-owasp-dep-check

Conversation

@yhwang
Copy link
Copy Markdown
Member

@yhwang yhwang commented Mar 10, 2026

Description

Skip OWASP dep check for doc-only changes; exclude the change in presto-docs/pom.xml. We still want to run OWASP dep check if the pom.xml of the presto-docs project changes.

Motivation and Context

Doc-only PRs shall skip the OWASP dep check, which reduces the time and resources for the CI jobs.

Impact

Doc-only PRs skip the OWASP check.

Test Plan

Use the same mechanism to skip the CI job, and I will also check some doc-only PRs after the change is merged.

Contributor checklist

  • Please make sure your submission complies with our contributing guide, in particular code style and commit standards.
  • PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
  • Documented new properties (with its default value), SQL syntax, functions, or other functionality.
  • If release notes are required, they follow the release notes guidelines.
  • Adequate tests were added if applicable.
  • CI passed.
  • If adding new dependencies, verified they have an OpenSSF Scorecard score of 5.0 or higher (or obtained explicit TSC approval for lower scores).

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== NO RELEASE NOTE ==

Summary by Sourcery

Skip the OWASP dependency check workflow for documentation-only pull requests while still running it when presto-docs/pom.xml changes.

CI:

  • Introduce a paths-filter job to detect non-doc or presto-docs/pom.xml changes and expose the result as a job output.
  • Gate all OWASP dependency-check workflow steps on the presence of relevant code changes so the job is effectively skipped for doc-only PRs.

Skip OWASP dep check for doc-only changes, excluding the
change is on the presto-docs/pom.xml. We still want to
run OWASP dep check if the pom.xml of the presto-docs project
changes.

Signed-off-by: Yihong Wang <yh.wang@ibm.com>
@yhwang yhwang requested review from a team, czentgr and unidevel as code owners March 10, 2026 21:14
@prestodb-ci prestodb-ci added the from:IBM PR from IBM label Mar 10, 2026
@prestodb-ci prestodb-ci requested review from a team and auden-woolfson and removed request for a team March 10, 2026 21:15
@sourcery-ai
Copy link
Copy Markdown
Contributor

sourcery-ai bot commented Mar 10, 2026

Reviewer's Guide

Introduces a preparatory GitHub Actions job to detect non-docs changes in pull requests (while treating presto-docs/pom.xml as code), and conditionally runs the OWASP dependency-check workflow only when relevant code changes are present, thereby skipping the check for doc-only PRs.

Flow diagram for skipping OWASP dependency-check on doc-only PRs

flowchart TD
  start["PR opened or updated"] --> paths_filter

  paths_filter["Run paths-filter in job changes\nfilters: codechange\n- !presto-docs/**\n- presto-docs/pom.xml"] --> decision_codechange

  decision_codechange{codechange == true?}

  decision_codechange -->|yes| run_dependency_check
  decision_codechange -->|no| skip_dependency_check

  subgraph run_dependency_check["Job dependency-check (runs)"]
    direction TB
    checkout_pr["Checkout PR branch"] --> merge_base["Find merge base"] --> checkout_base["Checkout base branch"] --> setup_java["Set up Java"] --> get_date["Get date for cache key"] --> cache_restore["Restore OWASP cache"] --> scan_base["Run OWASP on base"] --> cache_save_partial["Save OWASP cache on miss"] --> scan_pr["Run OWASP on PR"] --> compare_cves["Compare CVEs and fail on regression"] --> cache_save_final["Save OWASP cache (final)"] --> upload_reports["Upload reports artifact"]
  end

  subgraph skip_dependency_check["Job dependency-check (skipped)"]
    direction TB
    note_skip["All steps gated by if: needs.changes.outputs.codechange == 'true'\nNo OWASP scanning for doc-only changes"]
  end
Loading

File-Level Changes

Change Details Files
Add a paths-filter-based job to detect whether a PR includes non-docs changes or modifications to presto-docs/pom.xml and expose this as a reusable workflow output.
  • Introduce a new 'changes' job that runs on ubuntu-latest with read-only pull-requests permission.
  • Use dorny/paths-filter@v3 to compute a 'codechange' output based on path filters that ignore presto-docs/** but explicitly include presto-docs/pom.xml.
  • Export the 'codechange' result via job outputs for downstream jobs to consume.
.github/workflows/owasp-dependency-check.yml
Gate the OWASP dependency-check job and its steps on the computed code-change flag so that OWASP scans only run when relevant files change.
  • Declare dependency-check job depends on the 'changes' job via needs: changes.
  • Add an if: needs.changes.outputs.codechange == 'true' condition to all OWASP-related steps, including checkouts, Java setup, cache restore/save, OWASP scans, CVE comparison, and report upload.
  • Ensure cache save and artifact upload steps still respect always() but are additionally conditioned on codechange being true.
.github/workflows/owasp-dependency-check.yml

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue, and left some high level feedback:

  • Consider moving the if: needs.changes.outputs.codechange == 'true' condition to the dependency-check job level instead of repeating it on nearly every step, which reduces duplication and lowers the chance of missing the guard on new steps.
  • The new changes job relies on pull-requests: read and paths-filter, but the workflow triggers (e.g., push, workflow_dispatch) are not constrained here; verify that for non-PR events needs.changes.outputs.codechange has a sensible default so the OWASP check is not unintentionally skipped on those runs.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider moving the `if: needs.changes.outputs.codechange == 'true'` condition to the `dependency-check` job level instead of repeating it on nearly every step, which reduces duplication and lowers the chance of missing the guard on new steps.
- The new `changes` job relies on `pull-requests: read` and `paths-filter`, but the workflow triggers (e.g., `push`, `workflow_dispatch`) are not constrained here; verify that for non-PR events `needs.changes.outputs.codechange` has a sensible default so the OWASP check is not unintentionally skipped on those runs.

## Individual Comments

### Comment 1
<location path=".github/workflows/owasp-dependency-check.yml" line_range="33-34" />
<code_context>
+              - '!presto-docs/**'
+              - 'presto-docs/pom.xml'
+
   dependency-check:
+    needs: changes
     runs-on: ubuntu-latest
     concurrency:
</code_context>
<issue_to_address>
**suggestion (performance):** Consider moving the `codechange` condition to the job level to avoid spinning up a runner when no relevant changes occurred.

Currently the `dependency-check` job always starts and each step is guarded with:
```yaml
if: needs.changes.outputs.codechange == 'true'
```
This still provisions a runner even when `codechange` is `false`. Instead, apply the condition at the job level:
```yaml
dependency-check:
  needs: changes
  if: needs.changes.outputs.codechange == 'true'
  runs-on: ubuntu-latest
  ...
```
This avoids unnecessary runners and lets you drop the repeated `if` conditions from steps that don’t need their own logic.

Suggested implementation:

```
  dependency-check:
    needs: changes
    if: needs.changes.outputs.codechange == 'true'
    runs-on: ubuntu-latest

```

```
    steps:
      # Checkout PR branch first to get access to the composite action
      - name: Checkout PR branch
        uses: actions/checkout@v4

```

There are likely additional steps in the `dependency-check` job that also use `if: needs.changes.outputs.codechange == 'true'`. For consistency and to fully realize the optimization, remove that `if` condition from all steps that do not require their own, different conditional logic, since the job itself will only run when `codechange` is `true`.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@yhwang
Copy link
Copy Markdown
Member Author

yhwang commented Mar 11, 2026

@tdcmeehan, thanks for the review, and reran the failed job. Let me merge the PR and check some new doc-only PRs to see if it works properly.

@yhwang yhwang merged commit 2ebf611 into prestodb:master Mar 11, 2026
85 of 88 checks passed
@yhwang yhwang deleted the fix-doc-only-ci-4-owasp-dep-check branch March 11, 2026 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

from:IBM PR from IBM

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants