Skip to content

feat(antivirusCI): Add bridge to swx-jenkins-lib's antivirus var#149

Open
orbalayla-nvidia wants to merge 3 commits into
Mellanox:masterfrom
orbalayla-nvidia:feat/antivirus
Open

feat(antivirusCI): Add bridge to swx-jenkins-lib's antivirus var#149
orbalayla-nvidia wants to merge 3 commits into
Mellanox:masterfrom
orbalayla-nvidia:feat/antivirus

Conversation

@orbalayla-nvidia

@orbalayla-nvidia orbalayla-nvidia commented May 14, 2026

Copy link
Copy Markdown

Matrix-side bridge to the antivirus var in Mellanox-lab/swx-jenkins-lib#4. Shape mirrors slurmCI: argument template resolution, optional ref to pin a library version, single supported op (scan).

Usage from a matrix:

- name: Antivirus scan
  shell: action
  module: antivirusCI
  run: scan
  containerSelector: '{name:"some_container"}'
  args:
    path: '/abs/path'
    skipIfMissing: true   # optional
  archiveArtifacts: 'logs/antivirus_*.log'

The example under .ci/examples/job_matrix_antivirus.yaml shows it standalone. First real caller: https://github.com/Mellanox/cloudaix/pull/562.

Summary by CodeRabbit

  • New Features
    • Integrated antivirus scanning into CI with configurable scan paths, argument templating, environment merging, validation, and failure reporting.
  • Chores
    • Added an example CI job matrix for antivirus scans with runner configuration, log archiving, non-fail-fast behavior, and a 30-minute timeout.

@coderabbitai

coderabbitai Bot commented May 14, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a Jenkins shared-library entrypoint (antivirusCI) that invokes swx-jenkins-lib's antivirus.scan with bounded String ${VAR} resolution and env merging, plus an example CI job matrix demonstrating the scan on a mounted /auto/GLIT hostPath using ubuntu:22.04.

Changes

Antivirus scanning CI integration

Layer / File(s) Summary
antivirusCI Groovy bridge and library integration
vars/antivirusCI.groovy
Implements the call(ctx, oneStep, config) entrypoint that optionally loads swx-jenkins-lib@<ref> (or a fallback fork), validates and prunes args, performs bounded String-only ${VAR} template substitution (≤20 iterations), merges config.env and oneStep.env, runs antivirus.scan(args) inside withEnv, and returns integer status with failure reporting.
Antivirus example job matrix configuration
.ci/examples/job_matrix_antivirus.yaml
Provides an example job: failFast: false, timeout_minutes: 30, targets swx-k8s with amd64 node selector, requires hostPath mount /auto/GLIT, defines a ubuntu:22.04 runner (av_runner), runs antivirusCI with path: ${RELEASE_PATH} and skipIfMissing: true, and archives logs/antivirus_*.log.

Sequence Diagram(s)

sequenceDiagram
  participant Job as CI Job Matrix
  participant antivirusCI as antivirusCI (Groovy)
  participant swxLib as swx-jenkins-lib
  participant Container as ubuntu:22.04 Container
  
  Job->>antivirusCI: call(ctx, oneStep, config)
  antivirusCI->>antivirusCI: validate args & resolve ${VAR}
  antivirusCI->>antivirusCI: merge env vars
  antivirusCI->>swxLib: antivirus.scan(args)
  swxLib->>Container: run scan against mounted /auto/GLIT
  Container-->>swxLib: logs/artifacts
  swxLib-->>antivirusCI: return/throw
  antivirusCI-->>Job: return 0/1 and report
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

A rabbit hops where scanners gleam,
Templates untangle in an env stream,
Mounts bind GLIT and runners hum,
antivirus.scan() runs, results come,
Logs tucked safe — hop, scan, and drum! 🐇

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding a bridge module (antivirusCI) that connects to swx-jenkins-lib's antivirus functionality.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.ci/examples/job_matrix_antivirus.yaml:
- Around line 31-32: The example currently uses path: '${RELEASE_PATH}' with
skipIfMissing: true which allows the scan to silently skip when RELEASE_PATH
isn't set; update the example to be self-contained by defining a default
RELEASE_PATH in the YAML (or adding an env block/example export) so RELEASE_PATH
is always set for the example, and keep or adjust skipIfMissing accordingly;
reference the RELEASE_PATH variable and the path/skipIfMissing keys when making
this change so readers see the default value and avoid silent skips.

In `@vars/antivirusCI.groovy`:
- Around line 29-31: The current guard if (!args || args.size() < 1) in
antivirusCI.groovy lets invalid payloads like args = [skipIfMissing:true] pass;
update the validation to explicitly require a scan target by checking that args
contains either a non-empty "path" or "paths" key (e.g. assert args.path ||
args.paths) and if neither is present call ctx.reportFail(oneStep.name, 'fatal:
antivirusCI expects "path" or "paths" in args') and return 1; apply this check
before any later use of args so the function fails fast on malformed input.
- Line 57: The println currently logs the full args payload (args) which can
leak secrets; change the call that prints before invoking antivirus.scan to log
only the argument keys or a masked version: iterate the args Map (args) and
either log args.keySet() or build a maskedMap that replaces values for known
sensitive keys (e.g., "password","token","secret","apiKey") with "***" and log
that instead, and replace the plain println with the safer masked/keys-only log
prior to calling antivirus.scan.
- Around line 67-69: The current catch(Throwable t) in the antivirus.scan
handling can swallow FlowInterruptedException and prevent pipeline aborts;
update the handler so FlowInterruptedException (and InterruptedException if
desired) is rethrown immediately and only other exceptions are reported.
Concretely, replace the broad catch with either catch(Exception e) or keep the
catch but add: if (t instanceof
org.jenkinsci.plugins.workflow.steps.FlowInterruptedException) throw t; then
call ctx.reportFail(oneStep.name, "antivirus.scan failed: ${t.message}") and set
rc = 1 for non-interrupting errors only.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Enterprise

Run ID: d54f1720-ad1b-405d-ab5c-3bfc57c1ae32

📥 Commits

Reviewing files that changed from the base of the PR and between dd4da82 and e237060.

📒 Files selected for processing (2)
  • .ci/examples/job_matrix_antivirus.yaml
  • vars/antivirusCI.groovy

Comment thread .ci/examples/job_matrix_antivirus.yaml
Comment thread vars/antivirusCI.groovy Outdated
Comment thread vars/antivirusCI.groovy Outdated
Comment thread vars/antivirusCI.groovy Outdated
@orbalayla-nvidia orbalayla-nvidia marked this pull request as draft May 14, 2026 13:35
Loads Mellanox-lab/swx-jenkins-lib and calls antivirus.scan(args) from a
matrix step (shell: action, module: antivirusCI, run: scan). Shape
mirrors slurmCI: same arg-template resolution, optional `ref` to pin a
library version. Only the `scan` operation is exposed.

Example under .ci/examples/job_matrix_antivirus.yaml.

Signed-off-by: Or Balayla <obalayla@nvidia.com>
Points at orbalayla-nvidia/swx-jenkins-lib@feat/antivirus so this PR can
be exercised end-to-end before its dependency
(Mellanox-lab/swx-jenkins-lib#4) merges.

Revert before merging.

Signed-off-by: Or Balayla <obalayla@nvidia.com>
@orbalayla-nvidia orbalayla-nvidia marked this pull request as ready for review May 18, 2026 14:20
- Require an explicit 'path' or 'paths' arg instead of any non-empty
  map, so payloads like {skipIfMissing: true} fail fast.
- Rethrow FlowInterruptedException so pipeline aborts and manual
  interruptions propagate instead of being reported as scan failures.
- Make the example matrix self-contained with a default RELEASE_PATH so
  the scan does not silently skip when the variable is unset.

Signed-off-by: Or Balayla <obalayla@nvidia.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
vars/antivirusCI.groovy (1)

19-20: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Require non-empty scan targets, not just key presence.

Line 19 still allows malformed payloads (path: '', path: null, paths: []) to pass the guard and fail later inside antivirus.scan. Validate non-empty target values in this precheck.

Proposed fix
-    if (!(args?.containsKey('path') || args?.containsKey('paths'))) {
+    def hasPath = (args?.path instanceof String) && args.path.trim()
+    def hasPaths = (args?.paths instanceof Collection) && !args.paths.isEmpty()
+    if (!(hasPath || hasPaths)) {
         ctx.reportFail(oneStep.name, "antivirusCI: 'path' or 'paths' arg required")
         return 1
     }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@vars/antivirusCI.groovy` around lines 19 - 20, The guard only checks for
presence of keys but should also validate non-empty target values; update the
precheck in vars/antivirusCI.groovy to ensure args.path is a non-empty string
(not null/empty) or args.paths is a non-empty collection containing at least one
non-empty string, and if neither condition is true call
ctx.reportFail(oneStep.name, "antivirusCI: 'path' or 'paths' arg required and
must be non-empty"); reference the existing args, ctx.reportFail and
oneStep.name symbols when implementing the checks so malformed payloads like
path: '' or paths: [] are rejected before calling antivirus.scan.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@vars/antivirusCI.groovy`:
- Around line 19-20: The guard only checks for presence of keys but should also
validate non-empty target values; update the precheck in vars/antivirusCI.groovy
to ensure args.path is a non-empty string (not null/empty) or args.paths is a
non-empty collection containing at least one non-empty string, and if neither
condition is true call ctx.reportFail(oneStep.name, "antivirusCI: 'path' or
'paths' arg required and must be non-empty"); reference the existing args,
ctx.reportFail and oneStep.name symbols when implementing the checks so
malformed payloads like path: '' or paths: [] are rejected before calling
antivirus.scan.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Enterprise

Run ID: 4695c08c-800b-404d-b949-76645dea97c7

📥 Commits

Reviewing files that changed from the base of the PR and between f21597d and eaafa7b.

📒 Files selected for processing (2)
  • .ci/examples/job_matrix_antivirus.yaml
  • vars/antivirusCI.groovy
✅ Files skipped from review due to trivial changes (1)
  • .ci/examples/job_matrix_antivirus.yaml

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant