-
Notifications
You must be signed in to change notification settings - Fork 29
[CI] Issue: HPCINFRA-3441 move antivirus scan to release job #493
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
This PR relocates the antivirus scanning step from the regular CI matrix pipeline (matrix_job.yaml) to the release-specific pipeline (release_matrix_job.yaml). The change reduces overhead in development CI runs by deferring security scans to the release workflow where they are more appropriate. The antivirus step is removed entirely from the regular CI job, while the kubernetes volume mounts for antivirus dependencies (/auto/BACKUP/logs_of_LOGS and /auto/GLIT/SCRIPTS/HELPERS) are retained in matrix_job.yaml and added to release_matrix_job.yaml. The new antivirus step in the release job is conditionally enabled via ${do_antivirus} and executes .ci/antivirus.sh with ${release_folder} as an argument. This fits into the broader XLIO CI/CD architecture documented in .ci/README.md, where the matrix YAML files define declarative pipeline stages that the shared Jenkins Matrix class executes.
Important Files Changed
| Filename | Score | Overview |
|---|---|---|
.ci/matrix_job.yaml |
4/5 | Removed the Antivirus step from the regular CI pipeline; kept antivirus volume mounts for shared kubernetes pod configuration. |
.ci/pipeline/release_matrix_job.yaml |
1/5 | Added Antivirus step to release pipeline but references non-existent container rhel8.3-mofed-x86_64 and undefined variable ${release_folder}. |
Confidence score: 1/5
- This PR will fail immediately when the antivirus step executes because the container selector does not match any defined container in the release job.
- Score lowered due to: (1) non-existent container reference
rhel8.3-mofed-x86_64when onlyrhel8.6is defined inruns_on_dockers, causing container selection failure; (2) undefined${release_folder}variable will pass empty/null to the antivirus script; (3) mismatch between the removed step'srhel8.3-mofed-x86_64container and the availablerhel8.6container suggests incomplete migration. - Critical attention required for
.ci/pipeline/release_matrix_job.yaml: update thecontainerSelectorto match an existing container definition (likelyrhel8.6-mofed-x86_64or similar) and ensure${release_folder}is properly defined as a pipeline parameter or environment variable.
Sequence Diagram
sequenceDiagram
participant User
participant Jenkins
participant "CI Pipeline" as CI
participant "Release Pipeline" as Release
participant "Docker Container" as Docker
participant "Antivirus Scanner" as AV
participant "Build System" as Build
participant "Artifact Storage" as Storage
User->>Jenkins: "Trigger CI build"
Jenkins->>CI: "Load matrix_job.yaml"
CI->>Docker: "Start build container"
Docker->>Build: "Run autogen.sh"
Docker->>Build: "Execute build steps"
Build-->>Docker: "Build artifacts"
Docker-->>CI: "Build complete"
CI->>Storage: "Archive artifacts"
CI-->>Jenkins: "CI complete (no antivirus)"
User->>Jenkins: "Trigger release job"
Jenkins->>Release: "Load release_matrix_job.yaml"
Release->>Docker: "Start release container"
Docker->>Build: "Build libdpcp"
Docker->>Build: "Run do_release.sh"
Build-->>Docker: "Generate RPM packages"
Docker-->>Release: "Release build complete"
Release->>AV: "Run antivirus.sh on release_folder"
AV->>AV: "Scan RPM artifacts"
AV-->>Release: "Scan results"
Release->>Storage: "Archive RPM and scan logs"
Release->>User: "Send notification email"
Release-->>Jenkins: "Release complete"
2 files reviewed, 2 comments
be63a24 to
cd46aa7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
This PR refactors the CI pipeline by relocating the antivirus scan step from the regular CI builds (triggered on every PR/commit) to the release-only pipeline. The change removes the do_antivirus parameter from opensource_jjb.yaml, eliminates the Antivirus step from matrix_job.yaml, and introduces it into release_matrix_job.yaml with proper volume mounts for the scanning infrastructure. A new do_antivirus boolean parameter (defaulting to true) is added to release_jjb.yaml to control execution. This optimization reduces resource consumption and build times for development iterations while maintaining security scanning for production releases, aligning with the project's CI/CD strategy documented in the .ci/ directory structure.
Important Files Changed
| Filename | Score | Overview |
|---|---|---|
| .ci/opensource_jjb.yaml | 5/5 | Removed unused do_antivirus parameter from opensource CI job template |
| .ci/pipeline/release_jjb.yaml | 5/5 | Added do_antivirus boolean parameter to release pipeline job template |
| .ci/matrix_job.yaml | 4/5 | Removed Antivirus step from regular CI matrix, volume mounts remain unused |
| .ci/pipeline/release_matrix_job.yaml | 4/5 | Added Antivirus step with volume mounts and ${release_folder} parameter usage |
Confidence score: 4/5
- This PR is generally safe to merge with minor clarification needed on parameter plumbing and cleanup opportunities.
- Score reflects well-structured refactoring with clear intent, but points deducted for: (1) the
${release_folder}variable dependency inrelease_matrix_job.yamllacks explicit definition traceability in the changed files (appears to be a pipeline-level parameter but is not shown in these diffs), and (2) unused volume mounts remain inmatrix_job.yamlafter the Antivirus step removal (lines 29-31 should be cleaned up since they're only needed for antivirus scanning). - Pay close attention to
.ci/matrix_job.yamland.ci/pipeline/release_matrix_job.yamlto verify parameter flow and consider removing orphaned volume mount definitions from the regular CI matrix.
Sequence Diagram
sequenceDiagram
participant Developer
participant GitHub
participant Jenkins
participant ReleaseJob as "Release Pipeline"
participant RegularCI as "Regular CI Pipeline"
participant AntivirusService as "Antivirus Scanner"
participant ReleaseFolder as "Release Storage"
Note over Developer,ReleaseFolder: Before this PR
Developer->>GitHub: Push code / Create PR
GitHub->>RegularCI: Trigger CI build
RegularCI->>AntivirusService: Run antivirus scan
AntivirusService-->>RegularCI: Scan results
Note over RegularCI: Every CI build runs antivirus
Note over Developer,ReleaseFolder: After this PR
Developer->>GitHub: Push code / Create PR
GitHub->>RegularCI: Trigger CI build
Note over RegularCI: Antivirus step removed
RegularCI-->>GitHub: Build complete (no antivirus)
Developer->>GitHub: Create release tag
GitHub->>ReleaseJob: Trigger release pipeline
ReleaseJob->>ReleaseJob: Build dpcp
ReleaseJob->>ReleaseJob: Create release packages
ReleaseJob->>AntivirusService: Run antivirus scan
AntivirusService-->>ReleaseJob: Scan results
ReleaseJob->>ReleaseFolder: Copy packages to release folder
ReleaseJob->>Developer: Send notification email
4 files reviewed, no comments
cd46aa7 to
5c85df4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
This PR optimizes the CI pipeline by relocating antivirus scanning from the regular CI matrix job to the release-only pipeline. The do_antivirus parameter is removed from opensource_jjb.yaml (where it defaulted to false) and added to release_jjb.yaml (defaulting to true). The Antivirus step and its associated volume mounts (/auto/BACKUP/logs_of_LOGS, /auto/GLIT/SCRIPTS/HELPERS) are removed from matrix_job.yaml and added to release_matrix_job.yaml. This change prevents expensive security scans from running on every pull request and commit build, deferring them to the less frequent release workflow where they are more appropriate for production artifacts.
Important Files Changed
| Filename | Score | Overview |
|---|---|---|
| .ci/pipeline/release_jjb.yaml | 5/5 | Added do_antivirus boolean parameter (default: true) to enable antivirus scanning in release builds. |
| .ci/opensource_jjb.yaml | 5/5 | Removed do_antivirus parameter from regular CI job to prevent triggering antivirus scans during development. |
| .ci/pipeline/release_matrix_job.yaml | 3/5 | Added Antivirus step with volume mounts; however, ${release_folder} and ${do_antivirus} variables are not defined in this file. |
| .ci/matrix_job.yaml | 4/5 | Removed Antivirus step and volume mounts from regular CI; removed references to non-existent 'rhel8.3-mofed-x86_64' container. |
Confidence score: 3/5
- This PR requires careful review due to undefined variables in the release matrix configuration that may cause the antivirus step to fail.
- Score reflects two unresolved issues from previous reviews:
${release_folder}and${do_antivirus}are referenced inrelease_matrix_job.yamlbut not defined in the env section or as visible pipeline parameters in that file; these variables must be injected by the Jenkins shared library or parent job for the Antivirus step to execute successfully. - Pay close attention to
.ci/pipeline/release_matrix_job.yamlto verify that the undefined variables (${release_folder},${do_antivirus}) are properly resolved at runtime by the CI framework.
Sequence Diagram
sequenceDiagram
participant User
participant Jenkins
participant GitHub
participant Docker
participant K8s Pod
participant Build System
participant Antivirus
participant Release Folder
User->>Jenkins: Trigger release job with release_tag
Jenkins->>Jenkins: Update displayName with release_tag
Jenkins->>GitHub: Checkout code at sha1 (tags/$release_tag)
Jenkins->>Docker: Pull rhel8.6 release image
Jenkins->>K8s Pod: Start privileged pod with volumes mounted
K8s Pod->>GitHub: Clone libdpcp repository
K8s Pod->>Build System: Build and install libdpcp
K8s Pod->>Build System: Execute do_release.sh
Build System->>Build System: Build source RPM
Build System->>Build System: Validate version against Git tag
Build System->>Release Folder: Copy RPM to release folder (if do_release=true)
Build System->>Jenkins: Archive build_pkg.log and RPM artifacts
alt do_antivirus is true
K8s Pod->>Antivirus: Execute antivirus.sh on release_folder
Antivirus->>Antivirus: Locate source RPM
Antivirus->>Antivirus: Run antivirus-scan.sh as swx-jenkins
Antivirus->>Antivirus: Check scan result for "Possibly Infected: 0"
Antivirus->>Jenkins: Archive scan logs
end
Jenkins->>Jenkins: Check build result
Jenkins->>User: Send email notification with build status
4 files reviewed, no comments
5c85df4 to
48f305b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
This PR refactors the CI pipeline to move antivirus scanning from the regular CI matrix job to the release-specific pipeline. The change removes the "Antivirus" step and the do_antivirus parameter from .ci/matrix_job.yaml and .ci/opensource_jjb.yaml, adds the parameter to .ci/pipeline/release_jjb.yaml, and introduces an antivirus step in .ci/pipeline/release_matrix_job.yaml. This optimization prevents time-consuming antivirus scans from running on every development build while maintaining security checks for release artifacts. The change aligns with the CI architecture described in .ci/README.md, where the release pipeline handles artefact packaging and security scanning, while the regular matrix job focuses on build validation and testing.
PR Description Notes:
- Missing checkbox: "How?" section is empty - no description of implementation approach
Important Files Changed
| Filename | Score | Overview |
|---|---|---|
| .ci/pipeline/release_jjb.yaml | 5/5 | Added do_antivirus boolean parameter to release job configuration, defaulting to true |
| .ci/matrix_job.yaml | 5/5 | Removed antivirus step and two associated volume mounts (/auto/BACKUP/logs_of_LOGS, /auto/GLIT/SCRIPTS/HELPERS) |
| .ci/pipeline/release_matrix_job.yaml | 2/5 | Added antivirus step and mount for helper scripts, but contains critical issues with undefined release_folder variable and mismatched containerSelector |
| .ci/opensource_jjb.yaml | 5/5 | Removed do_antivirus parameter from regular CI job template |
Confidence score: 2/5
- This PR cannot be merged as-is due to critical issues in
release_matrix_job.yamlthat will cause pipeline failures - Score reflects two blocking issues previously identified: the
release_foldervariable is undefined in the environment or pipeline parameters, and the containerSelector references 'rhel8.3-mofed-x86_64' which doesn't exist in theruns_on_dockersdefinitions (only 'rhel8.6' is available), plus the step is missing thecontainerSelectorandagentSelectorfields that were present in the original antivirus step .ci/pipeline/release_matrix_job.yamlrequires immediate fixes to definerelease_folder(either as an env variable or pipeline parameter) and correct the containerSelector to 'rhel8.6' before this PR can function
Sequence Diagram
sequenceDiagram
participant Dev as Developer
participant GH as GitHub PR
participant Jenkins as Jenkins CI
participant CI as Regular CI Pipeline
participant Release as Release Pipeline
participant AV as Antivirus Scanner
participant NFS as NFS Release Folder
Note over CI,AV: BEFORE: Antivirus runs on every CI build
Dev->>GH: Create Pull Request
GH->>Jenkins: Trigger via bot:retest
Jenkins->>CI: Execute matrix_job.yaml
CI->>CI: Build, Test, Package steps
CI->>AV: Run Antivirus scan (every build)
AV-->>CI: Scan results
CI-->>Jenkins: Pipeline complete
Note over CI,AV: AFTER: Antivirus only on releases
Dev->>GH: Create Pull Request
GH->>Jenkins: Trigger via bot:retest
Jenkins->>CI: Execute matrix_job.yaml
CI->>CI: Build, Test, Package steps
Note over CI,AV: Antivirus step removed
CI-->>Jenkins: Pipeline complete (no antivirus)
Note over Release,NFS: Release process (unchanged timing)
Dev->>Jenkins: Trigger release job with tag
Jenkins->>Release: Execute release_matrix_job.yaml
Release->>Release: Build-dpcp
Release->>Release: Build release packages
Release->>NFS: Copy RPM to release folder
Release->>AV: Run Antivirus scan
AV-->>Release: Scan results
Release-->>Jenkins: Archive artifacts & logs
Jenkins->>Dev: Send email notification
4 files reviewed, no comments
48f305b to
420c1b8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
This PR reorganizes the CI pipeline to move antivirus scanning from the regular CI job (matrix_job.yaml) to the release-only job (release_matrix_job.yaml). The change removes the antivirus step and its required volume mounts (/auto/BACKUP/logs_of_LOGS, /auto/GLIT/SCRIPTS/HELPERS) from the standard CI matrix, eliminating the do_antivirus parameter from opensource_jjb.yaml. A new do_antivirus boolean parameter is added to release_jjb.yaml (defaulting to true) to control antivirus execution during releases. This refactoring aligns with CI best practices by reserving expensive security scans for release artifacts rather than running them on every PR build, reducing CI execution time while maintaining security for distributed releases.
Important Files Changed
| Filename | Score | Overview |
|---|---|---|
.ci/matrix_job.yaml |
5/5 | Removed antivirus step and two volume mounts from regular CI job |
.ci/opensource_jjb.yaml |
4/5 | Removed do_antivirus parameter from regular CI job template |
.ci/pipeline/release_jjb.yaml |
5/5 | Added do_antivirus parameter to release job configuration |
.ci/pipeline/release_matrix_job.yaml |
0/5 | File content not provided; unable to verify antivirus step addition |
Confidence score: 3/5
- This PR requires review of the missing file changes before merging, as critical implementation details cannot be verified.
- Score reflects that while the visible changes are clean and logically sound, the actual antivirus step implementation in
release_matrix_job.yamlwas not provided. Two previous review comments identified critical issues (undefinedrelease_foldervariable, incorrect container selector) that may exist in the non-visible portions of this PR, and without seeingrelease_matrix_job.yamlchanges, these issues cannot be confirmed as resolved. - Pay close attention to
.ci/pipeline/release_matrix_job.yamlto ensure the antivirus step is correctly configured with proper volume mounts, container selector matching available Docker images, and therelease_foldervariable properly defined or passed as a parameter.
Sequence Diagram
sequenceDiagram
participant User
participant Jenkins
participant Git
participant Docker
participant Kubernetes
participant ReleaseFolder
participant AntivirusScanner
User->>Jenkins: "Trigger Release Job (with release_tag)"
Jenkins->>Jenkins: "Update build displayName with release_tag"
Jenkins->>Git: "Clone repository (sha1/tag)"
Git-->>Jenkins: "Repository contents"
Jenkins->>Kubernetes: "Request privileged pod with rhel8.6 container"
Kubernetes-->>Jenkins: "Pod ready"
Jenkins->>Docker: "Pull/use rhel8.6 release image"
Docker-->>Jenkins: "Container ready"
Jenkins->>Git: "Clone libdpcp repository"
Git-->>Jenkins: "libdpcp source"
Jenkins->>Docker: "Build and install libdpcp (autogen, configure, make install)"
Docker-->>Jenkins: "libdpcp installed"
Jenkins->>Docker: "Execute .ci/do_release.sh"
Docker->>Docker: "Build XLIO release package"
Docker->>ReleaseFolder: "Copy RPM to release folder (if do_release=true)"
ReleaseFolder-->>Docker: "Package copied"
Docker-->>Jenkins: "Release artifacts created"
Jenkins->>Jenkins: "Archive build_pkg.log and RPM artifacts"
alt do_antivirus is true
Jenkins->>Docker: "Execute .ci/antivirus.sh"
Docker->>ReleaseFolder: "Locate source RPM in release folder"
ReleaseFolder-->>Docker: "RPM path"
Docker->>AntivirusScanner: "Run antivirus scan on RPM"
AntivirusScanner-->>Docker: "Scan results"
Docker-->>Jenkins: "Antivirus logs"
Jenkins->>Jenkins: "Archive antivirus logs"
end
Jenkins->>User: "Send email notification (if notification_email provided)"
Jenkins->>User: "Release job complete (SUCCESS/FAIL)"
4 files reviewed, no comments
420c1b8 to
cc03805
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
This PR reorganizes the CI pipeline by moving the antivirus scanning step from the regular matrix job (which runs on every PR/commit) to the release job (which runs only for official releases). The change removes the do_antivirus parameter and the Antivirus step from matrix_job.yaml and opensource_jjb.yaml, and adds them to release_matrix_job.yaml and release_jjb.yaml. Additionally, two volume mounts (/auto/BACKUP/logs_of_LOGS and /auto/GLIT/SCRIPTS/HELPERS) are moved to the release matrix to provide the antivirus script access to internal scanning tools. This optimization prevents resource-intensive antivirus scans from slowing down development feedback loops while maintaining security compliance for released artifacts. The do_antivirus parameter defaults to true in the release job to ensure scanning remains active for production releases.
Important Files Changed
| Filename | Score | Overview |
|---|---|---|
.ci/opensource_jjb.yaml |
5/5 | Removed the do_antivirus boolean parameter from the opensource Jenkins job definition |
.ci/pipeline/release_jjb.yaml |
5/5 | Added the do_antivirus boolean parameter (default: true) to the release job configuration |
.ci/matrix_job.yaml |
5/5 | Removed the Antivirus step and related volume mounts from the regular CI pipeline |
.ci/pipeline/release_matrix_job.yaml |
2/5 | Added Antivirus step and volume mounts to release pipeline; contains critical issues with undefined variables and mismatched container selector |
Confidence score: 2/5
- This PR has critical issues that will prevent the antivirus step from executing correctly in the release pipeline
- Score reflects two unresolved problems: (1) the
antivirus.shscript is called with${release_folder}and${release_tag}variables that are not defined in the environment or passed as parameters inrelease_matrix_job.yaml, which will cause the script to fail or behave incorrectly; (2) the containerSelector referencesrhel8.3-mofed-x86_64but the only defined container inruns_on_dockersisrhel8.6, meaning the step won't find a matching container to execute in - Pay close attention to
.ci/pipeline/release_matrix_job.yaml- you must either definerelease_folderandrelease_tagin the env section, pass them as pipeline parameters, or remove them from the antivirus.sh invocation; you must also fix the containerSelector to match an existing container definition or remove the selector entirely to use the default container
Sequence Diagram
sequenceDiagram
participant User
participant Jenkins
participant Git
participant Docker
participant Antivirus
User->>Jenkins: "Trigger release job with release_tag"
Jenkins->>Git: "Checkout code at sha1 (tags/$release_tag)"
Git-->>Jenkins: "Source code"
Jenkins->>Docker: "Start rhel8.6 container"
Docker-->>Jenkins: "Container ready"
Jenkins->>Git: "Clone libdpcp repository"
Git-->>Jenkins: "libdpcp source"
Jenkins->>Docker: "Build and install libdpcp"
Docker-->>Jenkins: "libdpcp installed"
Jenkins->>Docker: "Run do_release.sh script"
Docker->>Docker: "Build source RPM"
Docker->>Docker: "Validate version against tag"
Docker->>Docker: "Copy RPM to release_folder"
Docker-->>Jenkins: "RPM packages created"
Jenkins->>Jenkins: "Archive build artifacts (*.rpm, build_pkg.log)"
alt do_antivirus is true
Jenkins->>Antivirus: "Run antivirus.sh with release_folder and release_tag"
Antivirus->>Antivirus: "Scan source RPM"
Antivirus-->>Jenkins: "Scan results"
Jenkins->>Jenkins: "Archive antivirus logs"
end
Jenkins->>User: "Send email notification with build status"
4 files reviewed, no comments
cc03805 to
314878a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
This PR reorganizes the CI pipeline by moving the antivirus scanning step from the regular CI matrix job to the release-only pipeline. The do_antivirus parameter and its associated volume mounts (/auto/BACKUP/logs_of_LOGS, /auto/GLIT/SCRIPTS/HELPERS) are removed from opensource_jjb.yaml and matrix_job.yaml, then added to release_jjb.yaml and release_matrix_job.yaml. Additionally, do_release.sh gains a mkdir command to auto-create the release folder. This change prevents expensive antivirus scans from running on every PR/commit build while still ensuring release artifacts are scanned before distribution. The refactor aligns with the existing separation between regular CI workflows (defined in matrix_job.yaml) and release workflows (defined in release_matrix_job.yaml).
Important Files Changed
| Filename | Score | Overview |
|---|---|---|
.ci/pipeline/release_jjb.yaml |
5/5 | Adds do_antivirus boolean parameter (default: true) to enable antivirus scanning in the release pipeline. |
.ci/opensource_jjb.yaml |
5/5 | Removes do_antivirus parameter from regular CI job configuration to prevent scans on every build. |
.ci/do_release.sh |
4/5 | Adds mkdir ${release_folder} to auto-create the release directory; may produce confusing errors if parent paths are missing. |
.ci/matrix_job.yaml |
4/5 | Removes antivirus step and two volume mounts from regular CI matrix; containerSelector mismatch flagged in previous reviews. |
.ci/pipeline/release_matrix_job.yaml |
3/5 | Adds antivirus step and volume mounts to release pipeline; undefined release_folder variable and container selector mismatch remain unresolved. |
Confidence score: 3/5
- This PR requires careful review due to unresolved issues from previous reviews and potential runtime failures in the release pipeline.
- Score reflects two critical unresolved issues: (1) the
antivirus.shscript references${release_folder}and${release_tag}variables that are not defined in the env section or documented as pipeline parameters inrelease_matrix_job.yaml, and (2) the containerSelector in the Antivirus step still references 'rhel8.3-mofed-x86_64' when only 'rhel8.6' is defined in the runs_on_dockers catalog. Themkdiraddition indo_release.shmay also mask permission or parent-path issues. - Pay close attention to
.ci/pipeline/release_matrix_job.yaml(verifyrelease_folderandrelease_tagare properly passed from the pipeline) and.ci/do_release.sh(test behavior when parent directories are missing or permissions are insufficient).
Sequence Diagram
sequenceDiagram
participant User
participant Jenkins
participant ReleaseJob as Release Job
participant Docker as RHEL 8.6 Container
participant DoRelease as do_release.sh
participant Antivirus as antivirus.sh
participant NFS as NFS Release Folder
participant AntivirusSystem as Antivirus Scanner
User->>Jenkins: Trigger release with release_tag
Jenkins->>ReleaseJob: Load release_matrix_job.yaml
ReleaseJob->>Jenkins: Mount volumes (release folder, antivirus paths)
Jenkins->>Docker: Start RHEL 8.6 container
Docker->>DoRelease: Execute do_release.sh
DoRelease->>DoRelease: Run build_pkg.sh to create source RPM
DoRelease->>DoRelease: Validate version from configure.ac matches release_tag
alt do_release is true
DoRelease->>NFS: Copy package to release_folder/release_tag
DoRelease->>NFS: Create symlink to package
end
DoRelease->>Jenkins: Archive build logs and RPMs
alt do_antivirus is true
Jenkins->>Docker: Continue in same container
Docker->>Antivirus: Execute antivirus.sh with release_folder and release_tag
Antivirus->>NFS: Locate source RPM in release folder
Antivirus->>AntivirusSystem: Run antivirus-scan.sh as swx-jenkins
AntivirusSystem->>Antivirus: Return scan results
Antivirus->>Antivirus: Check for "Possibly Infected: ... 0"
Antivirus->>Jenkins: Archive scan logs
end
Jenkins->>User: Send email notification with build status
5 files reviewed, 1 comment
314878a to
373c84d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
This PR optimizes the CI pipeline by relocating antivirus scanning from the regular CI matrix job (which runs on every PR and commit) to the release pipeline. The change removes the Antivirus step and its three associated volume mounts (/auto/BACKUP/logs_of_LOGS, /auto/GLIT/SCRIPTS/HELPERS, and a third mount) from .ci/matrix_job.yaml, removes the do_antivirus parameter from .ci/opensource_jjb.yaml, and adds a new do_antivirus parameter (defaulting to true) to .ci/pipeline/release_jjb.yaml. The antivirus step and required infrastructure are now expected in release_matrix_job.yaml. This aligns with the project's CI architecture where different YAML files manage distinct pipeline types—the regular CI validates code changes rapidly, while the release pipeline handles security/compliance checks and artifact publication.
Important Files Changed
| Filename | Score | Overview |
|---|---|---|
.ci/matrix_job.yaml |
4/5 | Removed antivirus step and three volume mounts needed by antivirus.sh script |
.ci/opensource_jjb.yaml |
5/5 | Removed do_antivirus boolean parameter from regular CI pipeline definition |
.ci/pipeline/release_jjb.yaml |
4/5 | Added do_antivirus parameter (default true) to release job configuration |
.ci/pipeline/release_matrix_job.yaml |
2/5 | Added antivirus step but has critical issues: container selector mismatch, missing release_folder variable, and no volume mounts |
Confidence score:2/5
- This PR has critical configuration issues that will prevent the antivirus scan from functioning correctly in the release pipeline.
- Score lowered due to: (1) container selector referencing non-existent 'rhel8.3-mofed-x86_64' in release_matrix_job.yaml when only 'rhel8.6' is defined, (2) missing
release_foldervariable that the antivirus script requires, (3) absent volume mounts in release_matrix_job.yaml that antivirus.sh needs to access BACKUP logs and GLIT SCRIPTS directories, and (4) potential mkdir failure handling issue. - The release_matrix_job.yaml file requires immediate attention: fix container selector, define release_folder variable, and add the three volume mounts removed from matrix_job.yaml.
Sequence Diagram
sequenceDiagram
participant User
participant Jenkins
participant GitHub
participant Docker
participant BuildEnv
participant ReleaseFolder
participant AntivirusService
User->>Jenkins: Trigger release job with release_tag
Jenkins->>Jenkins: Update displayName with release_tag
Jenkins->>GitHub: Clone libxlio repository (sha1)
Jenkins->>Docker: Pull rhel8.6 release container
Jenkins->>BuildEnv: Start privileged Kubernetes pod
BuildEnv->>GitHub: Clone libdpcp dependency
BuildEnv->>BuildEnv: Build and install libdpcp
BuildEnv->>BuildEnv: Execute do_release.sh script
BuildEnv->>BuildEnv: Build source RPM packages
BuildEnv->>Jenkins: Archive build_pkg.log and RPM artifacts
alt do_antivirus == true
BuildEnv->>AntivirusService: Execute antivirus.sh with release_folder and release_tag
AntivirusService->>BuildEnv: Return scan results
BuildEnv->>Jenkins: Archive antivirus logs
end
alt do_release == true
BuildEnv->>ReleaseFolder: Copy packages to /auto/sw/release/sw_acceleration/xlio
end
Jenkins->>Jenkins: Set build result (SUCCESS/FAILURE)
alt notification_email not empty
Jenkins->>User: Send email with build status and tag info
end
4 files reviewed, no comments
|
Build to verify the antivirus step: |
|
bot:retest |
2 similar comments
|
bot:retest |
|
bot:retest |
373c84d to
67dfe91
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
Successfully relocates antivirus scanning from regular CI builds to release pipeline only.
Key changes:
- Removes antivirus step and
do_antivirusparameter from regular CI (matrix_job.yaml,opensource_jjb.yaml) - Removes antivirus-specific volume mounts from regular CI (BACKUP/logs_of_LOGS, GLIT/SCRIPTS/HELPERS)
- Adds
do_antivirusparameter (default: true) to release pipeline (release_jjb.yaml) - Adds antivirus step to release matrix job with required volume mounts (
release_matrix_job.yaml) - Improvement: New implementation passes both
release_folderandrelease_tagparameters explicitly, whereas old implementation only passedrelease_folderand relied on fallback logic
Previous issues resolved:
- containerSelector mismatch (rhel8.3-mofed-x86_64 vs rhel8.6) is eliminated by removing the step entirely from
matrix_job.yaml - Variable availability concerns are addressed: both
release_folderandrelease_tagare properly defined as pipeline parameters inrelease_jjb.yamland available to the release matrix job
Confidence Score: 5/5
- This PR is safe to merge with no risks identified
- The changes are purely configuration-based CI/CD refactoring with clear intent and correct implementation. All parameters are properly defined in the pipeline, volume mounts are correctly moved, and the antivirus step is cleanly relocated. The new implementation actually improves parameter passing by being explicit. Previous concerns about containerSelector mismatches and variable availability are resolved.
- No files require special attention
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| .ci/matrix_job.yaml | 5/5 | Cleanly removes antivirus step and associated volume mounts from regular CI |
| .ci/opensource_jjb.yaml | 5/5 | Removes do_antivirus parameter from regular CI pipeline |
| .ci/pipeline/release_jjb.yaml | 5/5 | Adds do_antivirus parameter (default true) to release pipeline |
| .ci/pipeline/release_matrix_job.yaml | 5/5 | Adds antivirus step with proper volume mounts, correctly passes both release_folder and release_tag parameters |
Sequence Diagram
sequenceDiagram
participant Dev as Developer
participant Jenkins as Jenkins Release Pipeline
participant Release as Release Matrix Job
participant Docker as RHEL8.6 Container
participant FS as File System
participant AV as Antivirus Scanner
Dev->>Jenkins: Trigger release (release_tag, release_folder)
Jenkins->>Jenkins: Set parameters (do_antivirus=true)
Jenkins->>Release: Start release_matrix_job.yaml
Release->>Docker: Mount volumes (sw_acceleration, logs_of_LOGS, SCRIPTS/HELPERS)
Release->>Docker: Step 1: Build-dpcp
Docker->>Docker: Clone and build libdpcp
Release->>Docker: Step 2: Release
Docker->>Docker: Execute do_release.sh
Docker->>FS: Create ${release_folder}/${release_tag}/
Docker->>FS: Copy libxlio-*.src.rpm to release folder
Release->>Docker: Step 3: Antivirus (if do_antivirus=true)
Docker->>FS: Navigate to ${release_folder}/${release_tag}/
Docker->>AV: Execute antivirus-scan.sh on .src.rpm
AV->>Docker: Return scan results
Docker->>FS: Save logs to logs/ directory
Docker->>Release: Archive logs/ artifacts
Release->>Jenkins: Complete with status
Jenkins->>Dev: Send notification email
4 files reviewed, no comments
67dfe91 to
68cdda6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
Moves antivirus scanning from regular CI to release pipeline to avoid unnecessary scans on every build.
Key Changes:
- Removed antivirus step and
do_antivirusparameter from.ci/matrix_job.yamland.ci/opensource_jjb.yaml - Added antivirus step to
.ci/pipeline/release_matrix_job.yamlwith required/auto/GLIT/SCRIPTS/HELPERSmount - Added
do_antivirusparameter (default: true) to.ci/pipeline/release_jjb.yaml - Refactored
.ci/antivirus.shto:- Use consistent variable names (
release_taginstead ofrelease_version) - Add validation for required variables (
WORKSPACE,release_folder,revision,release_tag) - Improve error handling with safer grep pattern matching
- Simplify exit logic
- Use consistent variable names (
Integration:
Jenkins pipeline parameters (release_tag, revision, release_folder) are automatically exported as environment variables and available to the antivirus script. The script now depends on these being set by the release pipeline configuration in release_jjb.yaml:18-33.
Confidence Score: 5/5
- This PR is safe to merge with no significant risk
- The changes correctly relocate antivirus scanning to the release pipeline without introducing logical errors. All required variables are properly passed through Jenkins parameters, the script refactoring improves error handling, and the removal from regular CI is clean
- No files require special attention
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| .ci/antivirus.sh | 5/5 | Refactored to use consistent variable names and improved error handling with safer grep logic |
| .ci/matrix_job.yaml | 5/5 | Removed antivirus step and related volumes/parameters from regular CI pipeline |
| .ci/pipeline/release_matrix_job.yaml | 4/5 | Added antivirus step and required GLIT mount - previous comment about containerSelector is invalid as step runs on host |
Sequence Diagram
sequenceDiagram
participant JJB as Jenkins Job Builder
participant Pipeline as Release Pipeline
participant Matrix as Matrix Framework
participant Release as do_release.sh
participant AV as antivirus.sh
participant Scan as Antivirus Scanner
JJB->>Pipeline: Trigger with params:<br/>release_tag, revision,<br/>release_folder, do_antivirus
Pipeline->>Matrix: Load release_matrix_job.yaml
Matrix->>Matrix: Mount /auto/GLIT/SCRIPTS/HELPERS
Matrix->>Release: Execute Build-dpcp step
Release-->>Matrix: dpcp installed
Matrix->>Release: Execute Release step
Release->>Release: Build packages
Release->>Release: Copy to ${release_folder}/${release_tag}/
Release-->>Matrix: Package created
alt do_antivirus=true
Matrix->>AV: Execute Antivirus step<br/>with WORKSPACE, release_folder,<br/>release_tag, revision
AV->>AV: Validate variables
AV->>AV: cd ${release_folder}/${release_tag}/
AV->>AV: Locate libxlio-${release_tag}-${revision}.src.rpm
AV->>Scan: sudo antivirus-scan.sh<br/>${release_folder}/${release_tag}/${pkg_name}
Scan-->>AV: Scan output to log
AV->>AV: grep 'Possibly Infected: 0'
alt Clean scan
AV-->>Matrix: exit 0
else Infected
AV-->>Matrix: exit 1 (fail build)
end
Matrix->>Matrix: Archive logs/ artifacts
else do_antivirus=false
Matrix->>Matrix: Skip Antivirus step
end
Matrix-->>Pipeline: Build complete
Pipeline->>Pipeline: Send notification email
5 files reviewed, no comments
68cdda6 to
efc1e1d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
This PR moves the antivirus scanning step from the regular CI pipeline to the release-only pipeline to improve CI performance. The antivirus scan now runs only during release builds rather than on every commit.
Key changes:
- Removed
do_antivirusparameter and antivirus step from.ci/matrix_job.yamland.ci/opensource_jjb.yaml - Added
do_antivirusparameter and antivirus step to.ci/pipeline/release_jjb.yamland.ci/pipeline/release_matrix_job.yaml - Refactored
.ci/antivirus.shto use release-specific variables (release_folder,release_tag,revision) instead of accepting script arguments - Updated volume mounts to support antivirus scanning in release pipeline
Critical issues found:
- Missing
/auto/BACKUP/logs_of_LOGSvolume mount in release matrix (was present in original CI setup) - Log directory creation bug: script creates
logs/in wrong location, will fail when writing to${WORKSPACE}/logs/ - Fragile file lookup using
lswithout error handling will cause cryptic failures
Confidence Score: 1/5
- This PR has critical bugs that will cause the antivirus step to fail in production
- The refactored antivirus script contains a definite bug where logs directory is created in the wrong location, causing the log file write to fail. Additionally, a required volume mount is missing from the release configuration. These are not edge cases - they will cause immediate failure when the antivirus step runs.
.ci/antivirus.shand.ci/pipeline/release_matrix_job.yamlrequire immediate fixes before this can be merged
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| .ci/antivirus.sh | 2/5 | Refactored to use release-specific variables; has critical bugs with log directory creation and fragile file lookup logic |
| .ci/pipeline/release_matrix_job.yaml | 2/5 | Added antivirus step to release pipeline; missing required mount volume for antivirus logs |
| .ci/matrix_job.yaml | 5/5 | Correctly removed antivirus step and associated volume mounts from regular CI |
Sequence Diagram
sequenceDiagram
participant Dev as Developer
participant CI as Regular CI Pipeline
participant Release as Release Pipeline
participant AV as Antivirus Scanner
participant NFS as NFS Release Folder
Note over Dev,CI: Before this PR
Dev->>CI: Push commit
CI->>AV: Run antivirus scan (every build)
AV-->>CI: Scan results
Note over Dev,Release: After this PR
Dev->>CI: Push commit
Note over CI,AV: Antivirus scan removed from CI
Dev->>Release: Trigger release build
Release->>Release: Build packages (do_release.sh)
Release->>NFS: Copy packages to release folder
Release->>AV: Run antivirus scan (release only)
AV->>NFS: Scan package at ${release_folder}/${release_tag}/
AV->>NFS: Write logs to ${WORKSPACE}/logs/
AV-->>Release: Scan results
5 files reviewed, 3 comments
efc1e1d to
7c1f265
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
Moves antivirus scanning from regular CI pipeline to release-only pipeline to optimize CI build times.
Key Changes:
- Removed antivirus step and related volume mounts from
.ci/matrix_job.yamlanddo_antivirusparameter from.ci/opensource_jjb.yaml - Added antivirus step to
.ci/pipeline/release_matrix_job.yamlanddo_antivirusparameter to.ci/pipeline/release_jjb.yaml - Refactored
.ci/antivirus.shto use environment variables (release_folder,release_tag,revision) instead of positional arguments
Issues Found:
- Critical: The refactored
antivirus.shcontains two bugs that will cause the script to fail:- Line 18 uses
lsto find a file that doesn't exist in the current directory - Line 26 creates logs directory in wrong location relative to where the log file will be written
- Line 18 uses
Confidence Score: 2/5
- This PR contains critical bugs in the antivirus script that will cause the release pipeline to fail
- While the overall refactoring approach is sound (moving antivirus to release-only jobs), the implementation has two critical logic errors in
antivirus.shthat will cause immediate script failures. Thelscommand on line 18 attempts to list a file in the wrong directory, and themkdiron line 26 creates a directory that doesn't match where the log file will be written. These must be fixed before the release pipeline can function. .ci/antivirus.shrequires immediate fixes to thelscommand (line 18) andmkdircommand (line 26) before this can be merged
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| .ci/antivirus.sh | 2/5 | Refactored to use environment variables instead of arguments. Contains critical bugs: ls command will fail (line 18), and mkdir path mismatch (line 26) |
| .ci/pipeline/release_matrix_job.yaml | 4/5 | Added antivirus step to release pipeline with required mount. Missing /auto/BACKUP/logs_of_LOGS mount that may be needed by external antivirus-scan.sh script |
Sequence Diagram
sequenceDiagram
participant Jenkins
participant ReleaseJob
participant BuildDpcp
participant DoRelease
participant Antivirus
participant FileSystem
Jenkins->>ReleaseJob: Trigger with release_tag, revision, release_folder, do_antivirus
ReleaseJob->>BuildDpcp: Clone and build libdpcp
BuildDpcp->>FileSystem: Install libdpcp to /usr
ReleaseJob->>DoRelease: Execute .ci/do_release.sh
DoRelease->>DoRelease: Build package with revision
DoRelease->>DoRelease: Validate release_tag matches configure.ac version
alt do_release=true
DoRelease->>FileSystem: Create ${release_folder}/${release_tag}/
DoRelease->>FileSystem: Copy libxlio-${release_tag}-${revision}.src.rpm
DoRelease->>FileSystem: Create symlink in release_folder
end
alt do_antivirus=true
ReleaseJob->>Antivirus: Execute .ci/antivirus.sh
Antivirus->>Antivirus: Validate WORKSPACE, release_folder, release_tag
Antivirus->>Antivirus: Construct pkg_path from variables
Antivirus->>FileSystem: Verify package exists at pkg_path
Antivirus->>FileSystem: Create logs directory
Antivirus->>FileSystem: Execute antivirus-scan.sh on package
Antivirus->>FileSystem: Write scan results to ${WORKSPACE}/logs/
Antivirus->>Antivirus: Check scan results for infections
alt No infections found
Antivirus-->>ReleaseJob: Success
else Infections found
Antivirus-->>ReleaseJob: Failure
end
end
ReleaseJob->>Jenkins: Archive artifacts and send notification
5 files reviewed, 2 comments
|
Build to verify the antivirus step: |
|
bot:retest |
01a8340 to
3bcb635
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
This PR successfully moves the antivirus scan from the regular CI pipeline to the release-specific pipeline to prevent unnecessary scans on every CI build.
Key Changes:
- Refactored
.ci/antivirus.shto use environment variables (WORKSPACE,release_folder,release_tag,revision) instead of positional arguments, with improved validation - Removed antivirus step and related volume mounts from
.ci/matrix_job.yaml - Removed
do_antivirusparameter from.ci/opensource_jjb.yaml - Added
do_antivirusparameter to.ci/pipeline/release_jjb.yaml(defaults to true) - Added antivirus step to
.ci/pipeline/release_matrix_job.yamlwith required/auto/GLIT/SCRIPTS/HELPERSmount - Added
runAsUserandrunAsGroupto release Docker container configuration for proper permissions
Integration:
The antivirus script now integrates with the release pipeline flow: the release job creates packages at ${release_folder}/${release_tag}/, then the antivirus step validates the generated .src.rpm file using the external antivirus scanning service.
Confidence Score: 4/5
- This PR is safe to merge with minor review recommended
- The refactoring is well-structured and achieves the goal of moving antivirus scans to release-only. The antivirus script improvements add better error handling. Previous comments raised valid concerns about variable passing and mounts, but those have been addressed in the implementation. The Docker user/group configuration was added to handle permissions properly.
.ci/antivirus.shand.ci/pipeline/release_matrix_job.yamlshould be verified in a test release build to ensure environment variables are passed correctly
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| .ci/antivirus.sh | 4/5 | Refactored to use environment variables and improved error handling with proper validation checks |
| .ci/pipeline/release_matrix_job.yaml | 4/5 | Added antivirus step with required mounts and updated Docker configuration with user/group settings |
Sequence Diagram
sequenceDiagram
participant Jenkins
participant ReleaseJob
participant ReleaseMatrix
participant Docker
participant AntivirusScript
participant AntivirusScan
Jenkins->>ReleaseJob: Trigger with release_tag, revision, release_folder
ReleaseJob->>ReleaseMatrix: Load .ci/pipeline/release_matrix_job.yaml
ReleaseMatrix->>Docker: Start rhel8.6 container with mounts
Docker->>ReleaseMatrix: Container ready (runAsUser: 6213)
ReleaseMatrix->>ReleaseMatrix: Step 1: Build-dpcp
ReleaseMatrix->>ReleaseMatrix: Step 2: Release (.ci/do_release.sh)
Note over ReleaseMatrix: Creates package at ${release_folder}/${release_tag}/
ReleaseMatrix->>AntivirusScript: Step 3: Antivirus (if do_antivirus=true)
AntivirusScript->>AntivirusScript: Validate WORKSPACE, release_folder, release_tag
AntivirusScript->>AntivirusScript: cd ${release_folder}/${release_tag}/
AntivirusScript->>AntivirusScript: Find package: libxlio-${release_tag}-${revision}.src.rpm
AntivirusScript->>AntivirusScan: Call /auto/GLIT/SCRIPTS/HELPERS/antivirus-scan.sh
AntivirusScan-->>AntivirusScript: Scan results
AntivirusScript->>AntivirusScript: Check for "Possibly Infected: 0"
AntivirusScript-->>ReleaseMatrix: Exit 0 (success) or Exit 1 (failure)
ReleaseMatrix->>ReleaseMatrix: Archive logs/
ReleaseMatrix-->>Jenkins: Pipeline complete
5 files reviewed, no comments
3bcb635 to
100a29b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
This PR migrates the antivirus scanning step from the regular CI pipeline to the release pipeline to avoid running antivirus scans on every build.
Key Changes:
- Removed antivirus step and
do_antivirusparameter from.ci/matrix_job.yamland.ci/opensource_jjb.yaml - Added antivirus step and
do_antivirusparameter to.ci/pipeline/release_matrix_job.yamland.ci/pipeline/release_jjb.yaml - Refactored
.ci/antivirus.shto use improved variable validation and removedsudocalls - Updated
.ci/do_release.shto removesudo -E -u swx-jenkinssince the container now runs as user 6213 - Added
runAsUser: 6213andrunAsGroup: 101to release container configuration - Moved volume mounts for antivirus from regular CI to release pipeline
Issues Found:
- Critical logic error in
.ci/antivirus.sh:21wherelscommand will fail because it runs in$WORKSPACEbut the file is in${release_folder}/${release_tag}/ - Missing
/auto/BACKUP/logs_of_LOGSmount that was present in original antivirus configuration (may be needed by the external antivirus-scan.sh script)
Confidence Score: 1/5
- This PR has a critical bug that will cause the antivirus step to fail completely.
- The migration concept is sound, but
.ci/antivirus.sh:21contains a critical bug wherelstries to find the package file in$WORKSPACEinstead of${release_folder}/${release_tag}/. This will cause the antivirus step to fail with "No such file or directory" error. Additionally, the missing/auto/BACKUP/logs_of_LOGSmount may cause issues if the external antivirus-scan.sh script requires it. .ci/antivirus.shrequires immediate fix on line 21. Verify if.ci/pipeline/release_matrix_job.yamlneeds the/auto/BACKUP/logs_of_LOGSmount.
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| .ci/antivirus.sh | 1/5 | Refactored to use consistent variable validation and simplified logic. Critical bug: line 21 ls command will fail as it runs in $WORKSPACE but file is in ${release_folder}/${release_tag}/. |
| .ci/do_release.sh | 4/5 | Removed sudo -E -u swx-jenkins commands since container now runs as user 6213. Minor formatting fixes. No logical issues. |
| .ci/pipeline/release_matrix_job.yaml | 3/5 | Added antivirus step and /auto/GLIT/SCRIPTS/HELPERS mount. Added runAsUser/runAsGroup. Missing /auto/BACKUP/logs_of_LOGS mount that was in original config (may be needed by antivirus-scan.sh). |
Sequence Diagram
sequenceDiagram
participant Release as Release Pipeline
participant Container as Docker Container (rhel8.6)
participant DoRelease as do_release.sh
participant Antivirus as antivirus.sh
participant AntivirusScanner as /auto/GLIT/SCRIPTS/HELPERS/antivirus-scan.sh
participant ReleaseFolder as /auto/sw/release/sw_acceleration
Release->>Container: Start with do_antivirus=true
Container->>Container: Run as user 6213 (swx-jenkins)
Container->>DoRelease: Execute .ci/do_release.sh
DoRelease->>DoRelease: Build package via build_pkg.sh
DoRelease->>DoRelease: Verify tag matches configure.ac version
DoRelease->>ReleaseFolder: mkdir -p ${release_folder}/${release_tag}
DoRelease->>ReleaseFolder: Copy .src.rpm to release folder
DoRelease->>ReleaseFolder: Create symlink
Container->>Antivirus: Execute .ci/antivirus.sh
Antivirus->>Antivirus: Validate WORKSPACE, release_folder, release_tag
Antivirus->>Antivirus: ❌ ls command fails (file not in WORKSPACE)
Antivirus->>ReleaseFolder: Check if package exists
Antivirus->>AntivirusScanner: Scan ${release_folder}/${release_tag}/${pkg_name}
AntivirusScanner-->>Antivirus: Scan results
Antivirus->>Antivirus: Check for "Possibly Infected: 0"
Antivirus-->>Container: Exit with status code
6 files reviewed, 1 comment
100a29b to
4c0e551
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
Moves antivirus scanning from regular CI builds to release-only pipeline to reduce CI overhead.
Key Changes:
- Removed Antivirus step from
.ci/matrix_job.yaml(regular CI) - Added Antivirus step to
.ci/pipeline/release_matrix_job.yaml(release pipeline) - Refactored
antivirus.shto use environment variables instead of positional arguments - Removed
sudocalls from bothantivirus.shanddo_release.sh(now runs as user 6213:101) - Added
do_antivirusparameter to release JJB configuration
Minor Issues:
- Line 22 in
antivirus.shuseslscommand which is redundant since line 25 already validates file existence. Direct variable assignment would be cleaner and avoid potential early exit fromset -e.
Confidence Score: 4/5
- Safe to merge with one minor style improvement recommended
- Well-structured refactoring that successfully moves antivirus scanning to release pipeline. The changes properly handle environment variables and remove unnecessary sudo calls. One minor style issue with redundant
lscommand, but it doesn't affect functionality. All previous critical comments about missing variables appear resolved -release_folder,release_tag, andrevisionare now properly validated as environment variables. .ci/antivirus.sh- consider simplifying line 22 to avoid redundantlscommand
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| .ci/antivirus.sh | 4/5 | Refactored to use environment variables, improved error handling and removed sudo calls. The ls command on line 22 is unnecessary since line 25 already checks file existence. |
| .ci/pipeline/release_matrix_job.yaml | 4/5 | Added Antivirus step to release pipeline with proper user/group configuration. Missing /auto/BACKUP/logs_of_LOGS mount that may be needed by antivirus-scan.sh. |
| .ci/do_release.sh | 5/5 | Removed sudo calls for mkdir, cp, and ln commands. Uses current user (6213:101) for file operations. Clean refactoring with proper quoting. |
Sequence Diagram
sequenceDiagram
participant Jenkins
participant Release Job
participant do_release.sh
participant antivirus.sh
participant antivirus-scan.sh
Jenkins->>Release Job: Trigger with release_tag, revision, do_antivirus
Release Job->>do_release.sh: Build and copy package
do_release.sh->>do_release.sh: Build RPM package
do_release.sh->>do_release.sh: mkdir -p ${release_folder}/${release_tag}
do_release.sh->>do_release.sh: cp package to ${release_folder}/${release_tag}
alt do_antivirus enabled
Release Job->>antivirus.sh: Run with env vars (WORKSPACE, release_folder, release_tag, revision)
antivirus.sh->>antivirus.sh: Validate env vars
antivirus.sh->>antivirus.sh: cd ${release_folder}/${release_tag}
antivirus.sh->>antivirus-scan.sh: Scan ${release_folder}/${release_tag}/${pkg_name}
antivirus-scan.sh-->>antivirus.sh: Scan results
antivirus.sh->>antivirus.sh: Check "Possibly Infected: 0"
antivirus.sh-->>Release Job: Exit 0 (success) or 1 (infected)
end
Release Job-->>Jenkins: Archive logs and artifacts
6 files reviewed, 1 comment
4c0e551 to
4687de1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
This PR moves the antivirus scan from the regular CI pipeline to the release pipeline to prevent it from running on every build, improving CI efficiency.
Major Changes:
- Removed antivirus step and related mounts from
.ci/matrix_job.yamland.ci/opensource_jjb.yaml - Added antivirus step to
.ci/pipeline/release_matrix_job.yamlwithdo_antivirusparameter in.ci/pipeline/release_jjb.yaml - Refactored
.ci/antivirus.shto use standardized variable names (release_taginstead ofrelease_version) and improved error handling
Issues Found:
- Critical bug in
.ci/antivirus.sh:22: redundantlscommand will fail before file existence check on line 25 can execute - Potential missing mount:
/auto/BACKUP/logs_of_LOGSwas present in original CI setup but not added to release matrix - Minor:
mkdirin.ci/do_release.sh:79may fail if parent directories don't exist (pre-existing issue)
Confidence Score: 2/5
- Not safe to merge - critical bug will cause antivirus step to fail on first run
- The redundant
lscommand inantivirus.shline 22 will fail withset -eif the file doesn't exist, preventing the more informative error message on line 25 from ever executing. This makes the antivirus step fragile and likely to fail. Additionally, the missing log mount may cause issues depending on antivirus-scan.sh requirements. .ci/antivirus.shmust be fixed before merge - thelscommand on line 22 should be replaced with direct filename construction
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| .ci/antivirus.sh | 2/5 | Refactored antivirus script with better error handling but contains critical bugs: redundant ls command will fail before file existence check, causing script to exit prematurely |
| .ci/pipeline/release_matrix_job.yaml | 3/5 | Added antivirus step to release pipeline with required mounts; potentially missing /auto/BACKUP/logs_of_LOGS mount that was in original setup |
| .ci/do_release.sh | 4/5 | Whitespace-only formatting changes; existing potential issue with mkdir not creating parent directories |
Sequence Diagram
sequenceDiagram
participant Jenkins
participant ReleaseJob
participant DoRelease
participant Antivirus
participant NFSStorage
Jenkins->>ReleaseJob: Trigger release pipeline<br/>(release_tag, revision, release_folder)
ReleaseJob->>DoRelease: Execute .ci/do_release.sh
DoRelease->>DoRelease: Build packages<br/>(tarball + src.rpm)
DoRelease->>DoRelease: Verify version matches tag
alt do_release=true
DoRelease->>NFSStorage: mkdir -p release_folder/release_tag
DoRelease->>NFSStorage: Copy packages to<br/>release_folder/release_tag/
DoRelease->>NFSStorage: Create symlink
end
DoRelease-->>ReleaseJob: Build complete
alt do_antivirus=true
ReleaseJob->>Antivirus: Execute .ci/antivirus.sh
Antivirus->>Antivirus: Validate env vars<br/>(WORKSPACE, release_folder, release_tag)
Antivirus->>Antivirus: cd to release_folder/release_tag/
Antivirus->>Antivirus: Locate .src.rpm file
Antivirus->>Antivirus: Run /auto/GLIT/SCRIPTS/HELPERS/<br/>antivirus-scan.sh
Antivirus->>Antivirus: Check scan results for infections
alt No infections
Antivirus-->>ReleaseJob: Success (exit 0)
else Infections found
Antivirus-->>ReleaseJob: Failure (exit 1)
end
end
ReleaseJob-->>Jenkins: Pipeline complete
6 files reviewed, no comments
4687de1 to
abed4c5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
This PR successfully moves the antivirus scanning step from the regular CI pipeline to the release pipeline to prevent it from running on every CI build.
Key Changes:
- Removed antivirus step and related volume mounts from
.ci/matrix_job.yaml(regular CI) - Added antivirus step with necessary mounts to
.ci/pipeline/release_matrix_job.yaml(release pipeline) - Moved
do_antivirusparameter fromopensource_jjb.yamltorelease_jjb.yaml - Refactored
antivirus.shto use environment variables consistently and improved error handling - Updated
do_release.shto run commands directly as container user instead of using sudo (since container now runs as UID 6213) - Configured release container with
runAsUser: 6213andrunAsGroup: 101to match swx-jenkins user
Potential Issues:
Several existing comments identify legitimate concerns that should be addressed, particularly around missing mounts and variable handling in the antivirus script.
Confidence Score: 2/5
- PR has multiple logic issues that will likely cause antivirus step to fail
- The PR correctly moves the antivirus configuration from regular CI to release pipeline, but has several implementation issues already identified in previous comments: the antivirus script will fail due to missing environment variables (
release_tagis not defined in the pipeline env), thelscommand on line 22 of antivirus.sh is fragile, and the/auto/BACKUP/logs_of_LOGSmount may be needed. These issues will prevent the antivirus step from working correctly. .ci/pipeline/release_matrix_job.yamlneeds to definerelease_tagin the env section and potentially add the missing/auto/BACKUP/logs_of_LOGSmount..ci/antivirus.shshould construct the filename directly instead of usingls.
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| .ci/antivirus.sh | 3/5 | Refactored script with improved error handling and removed positional arguments; uses environment variables consistently |
| .ci/do_release.sh | 4/5 | Removed sudo commands and added proper quoting; runs commands directly as container user |
| .ci/pipeline/release_matrix_job.yaml | 3/5 | Added antivirus step with proper mounts and user configuration; missing some mounts from original setup |
Sequence Diagram
sequenceDiagram
participant JJB as Jenkins Job (release_jjb.yaml)
participant Pipeline as Release Pipeline
participant Container as Docker Container (rhel8.6)
participant Release as do_release.sh
participant Antivirus as antivirus.sh
participant Scanner as antivirus-scan.sh
JJB->>Pipeline: Trigger with params (release_tag, release_folder, do_antivirus)
Pipeline->>Container: Launch container as user 6213 (swx-jenkins)
Container->>Release: Execute .ci/do_release.sh
Release->>Release: Build src.rpm and tarball
Release->>Release: Copy packages to ${release_folder}/${release_tag}/
alt do_antivirus=true
Container->>Antivirus: Execute .ci/antivirus.sh
Antivirus->>Antivirus: Validate WORKSPACE, release_folder, release_tag
Antivirus->>Antivirus: cd ${release_folder}/${release_tag}/
Antivirus->>Antivirus: Find libxlio-${release_tag}-${revision}.src.rpm
Antivirus->>Scanner: Call /auto/GLIT/SCRIPTS/HELPERS/antivirus-scan.sh
Scanner-->>Antivirus: Scan results
Antivirus->>Antivirus: Check for "Possibly Infected: 0"
Antivirus-->>Container: Exit 0 (pass) or 1 (fail)
end
Container-->>Pipeline: Archive logs and packages
Pipeline-->>JJB: Complete with status
6 files reviewed, no comments
abed4c5 to
fb06be3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
This PR successfully moves antivirus scanning from the regular CI pipeline to the release pipeline, preventing unnecessary scans on every build.
Key Changes:
- Removed antivirus step and related mounts from
.ci/matrix_job.yaml(regular CI) - Added antivirus step to
.ci/pipeline/release_matrix_job.yaml(release pipeline) - Refactored
antivirus.shto use environment variables instead of positional arguments - Enhanced antivirus script to scan both
.src.rpmand.tar.gzfiles (previously only scanned .src.rpm) - Improved error handling with better validation of required variables
- Removed
sudocommands fromdo_release.sh(now runs with correct user viarunAsUser/runAsGroupin container config) - Moved
do_antivirusparameter from opensource JJB to release JJB
Findings:
- One mount (
/auto/BACKUP/logs_of_LOGS) was present in the original antivirus setup but not added to the release config - needs verification that external antivirus script doesn't require it
Confidence Score: 4/5
- This PR is safe to merge with low risk pending verification of one mount requirement
- The changes are well-structured and accomplish the stated goal. The refactored antivirus script has better error handling and validation. The only concern is the potentially missing
/auto/BACKUP/logs_of_LOGSmount that was in the original setup but wasn't migrated - this needs verification that the external antivirus-scan.sh script doesn't require it. - Verify
.ci/pipeline/release_matrix_job.yamlhas all required mounts for the external antivirus script
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| .ci/pipeline/release_matrix_job.yaml | 4/5 | Added antivirus scan step with proper mounts; containerSelector issue mentioned in previous comments doesn't apply as rhel8.6 is now defined |
| .ci/matrix_job.yaml | 5/5 | Removed antivirus step and related mounts from regular CI job as intended |
| .ci/antivirus.sh | 4/5 | Refactored to expect environment variables instead of arguments; improved error handling and now scans both .rpm and .tar.gz files |
Sequence Diagram
sequenceDiagram
participant Jenkins
participant ReleaseJob
participant DockerContainer
participant BuildScript
participant AntivirusScript
participant ExternalAV
Jenkins->>ReleaseJob: Trigger with params (release_tag, revision, release_folder, do_antivirus)
ReleaseJob->>DockerContainer: Start rhel8.6 container
DockerContainer->>BuildScript: Execute do_release.sh
BuildScript->>BuildScript: Build tarball & src.rpm
BuildScript->>BuildScript: Copy to ${release_folder}/${release_tag}/
BuildScript-->>DockerContainer: Packages created
alt do_antivirus == true
DockerContainer->>AntivirusScript: Execute antivirus.sh
AntivirusScript->>AntivirusScript: Validate env vars (release_folder, release_tag, revision)
AntivirusScript->>AntivirusScript: Construct file paths
AntivirusScript->>ExternalAV: Scan .src.rpm file
ExternalAV-->>AntivirusScript: Scan results → rpm_log
AntivirusScript->>ExternalAV: Scan .tar.gz file
ExternalAV-->>AntivirusScript: Scan results → tarball_log
AntivirusScript->>AntivirusScript: Check "Possibly Infected: 0" in both logs
AntivirusScript-->>DockerContainer: Exit 0 (clean) or 1 (infected)
end
DockerContainer-->>ReleaseJob: Build complete
ReleaseJob->>Jenkins: Archive artifacts & send notification
6 files reviewed, 1 comment
fb06be3 to
fd39290
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
This PR moves the antivirus scanning step from regular CI builds to the release pipeline to optimize CI performance.
Key Changes:
- Removed antivirus step and
do_antivirusparameter from regular CI configuration (.ci/matrix_job.yaml,.ci/opensource_jjb.yaml) - Added antivirus step to release pipeline (
.ci/pipeline/release_matrix_job.yaml,.ci/pipeline/release_jjb.yaml) - Refactored
antivirus.shto construct filenames directly instead of usingls, scan both .rpm and .tar.gz files, and improve error handling - Removed
sudo -E -u swx-jenkinscommands from bothantivirus.shanddo_release.sh- now runs as configured user (UID 6213/GID 101) - Added
runAsUser/runAsGroupto release container configuration for non-root execution
Validation:
The antivirus script now validates all required environment variables (WORKSPACE, release_folder, release_tag) are set before execution and scans both package artifacts with proper log separation.
Confidence Score: 4/5
- This PR is safe to merge with minor risk - the refactoring improves the CI workflow architecture
- The changes are well-structured and achieve the stated goal of moving antivirus scanning to release builds. The antivirus script improvements (direct filename construction, better validation, dual file scanning) are solid. However, there's one concern: the previous comments mention a missing
/auto/BACKUP/logs_of_LOGSmount that existed in the original configuration. While the script doesn't appear to directly reference this path, the externalantivirus-scan.shscript may require it. This should be verified before merging. - Verify that
/auto/GLIT/SCRIPTS/HELPERS/antivirus-scan.shdoesn't require the/auto/BACKUP/logs_of_LOGSmount
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| .ci/antivirus.sh | 4/5 | Refactored to construct filenames directly, scan both .rpm and .tar.gz, improved error handling, removed sudo usage |
| .ci/do_release.sh | 5/5 | Minor style improvements: removed sudo usage, added quotes to variables, whitespace cleanup |
| .ci/matrix_job.yaml | 5/5 | Removed antivirus step and related mounts from regular CI builds |
| .ci/opensource_jjb.yaml | 5/5 | Removed do_antivirus parameter from regular CI job definition |
| .ci/pipeline/release_jjb.yaml | 5/5 | Added do_antivirus parameter to release job definition |
| .ci/pipeline/release_matrix_job.yaml | 4/5 | Added antivirus step with mount for antivirus scripts, added runAsUser/runAsGroup for non-root execution |
Sequence Diagram
sequenceDiagram
participant Release Pipeline
participant do_release.sh
participant Build System
participant Release Folder
participant antivirus.sh
participant Antivirus Scanner
Release Pipeline->>do_release.sh: Start with release_tag, release_folder, revision
do_release.sh->>Build System: Execute contrib/build_pkg.sh
Build System-->>do_release.sh: Create .src.rpm & .tar.gz in pkg/packages/
do_release.sh->>do_release.sh: Validate version matches tag
do_release.sh->>Release Folder: mkdir -p ${release_folder}/${release_tag}
do_release.sh->>Release Folder: Copy pkg & tarball to ${release_folder}/${release_tag}/
do_release.sh->>Release Folder: Create symlink
do_release.sh-->>Release Pipeline: Package released
alt do_antivirus is true
Release Pipeline->>antivirus.sh: Start with WORKSPACE, release_folder, release_tag, revision
antivirus.sh->>antivirus.sh: Validate environment variables
antivirus.sh->>antivirus.sh: cd ${release_folder}/${release_tag}/
antivirus.sh->>antivirus.sh: Construct filenames: libxlio-${release_tag}-${revision}.src.rpm
antivirus.sh->>Antivirus Scanner: Scan .src.rpm file
Antivirus Scanner-->>antivirus.sh: Log results to ${WORKSPACE}/logs/
antivirus.sh->>Antivirus Scanner: Scan .tar.gz file
Antivirus Scanner-->>antivirus.sh: Log results to ${WORKSPACE}/logs/
antivirus.sh->>antivirus.sh: Check both logs for "Possibly Infected: 0"
antivirus.sh-->>Release Pipeline: Exit 0 (clean) or Exit 1 (infected)
Release Pipeline->>Release Pipeline: Archive logs/ artifacts
end
6 files reviewed, no comments
fd39290 to
b5d2d6a
Compare
Greptile OverviewGreptile SummaryThis PR successfully moves the antivirus scan from the regular CI pipeline to the release pipeline, preventing unnecessary scans on every build. The refactored Key changes:
The changes are well-structured and align with the stated goal of optimizing CI performance by moving antivirus scans to release builds only. Confidence Score: 4/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant Jenkins
participant ReleaseMatrix as Release Matrix Job
participant BuildDpcp as Build-dpcp Step
participant Release as Release Step
participant DoRelease as do_release.sh
participant Antivirus as Antivirus Step
participant AntivirusScript as antivirus.sh
participant Scanner as antivirus-scan.sh
Jenkins->>ReleaseMatrix: Trigger with parameters (release_tag, release_folder, revision, do_antivirus)
ReleaseMatrix->>BuildDpcp: Clone and build libdpcp
BuildDpcp-->>ReleaseMatrix: Complete
ReleaseMatrix->>Release: Execute do_release.sh
Release->>DoRelease: Run with env vars from Jenkins params
DoRelease->>DoRelease: Build tarball and src.rpm
DoRelease->>DoRelease: Copy to ${release_folder}/${release_tag}/
DoRelease-->>Release: Packages created
Release-->>ReleaseMatrix: Archive artifacts
alt do_antivirus=true
ReleaseMatrix->>Antivirus: Execute antivirus.sh
Antivirus->>AntivirusScript: Run with WORKSPACE, release_folder, release_tag, revision
AntivirusScript->>AntivirusScript: Validate environment variables
AntivirusScript->>AntivirusScript: cd ${release_folder}/${release_tag}/
AntivirusScript->>Scanner: Scan libxlio-${release_tag}-${revision}.src.rpm
Scanner-->>AntivirusScript: Log to ${WORKSPACE}/logs/
AntivirusScript->>Scanner: Scan libxlio-${release_tag}.tar.gz
Scanner-->>AntivirusScript: Log to ${WORKSPACE}/logs/
AntivirusScript->>AntivirusScript: Check for infections in logs
AntivirusScript-->>Antivirus: Exit 0 (clean) or 1 (infected)
Antivirus-->>ReleaseMatrix: Archive logs/
end
ReleaseMatrix->>Jenkins: Send email notification
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
6 files reviewed, no comments
Move the antivirus scan step from the regular matrix file to the release matrix file Signed-off-by: Noam Tsemah <[email protected]>
b5d2d6a to
d55bb99
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
6 files reviewed, no comments
Description
Add antivirus scan to release job and remove from regular CI job.
What
Move the antivirus scan step and do_antivirus parameter from the regular matrix file to the release matrix file.
Add mounts for the antivirus step to the release matrix file.
Why ?
To prevent the antivirus scan from running on every CI build.
How ?
Change type
What kind of change does this PR introduce?
Check list