Skip to content

Fix pipeline scan job - #2873

Open
JunAr7112 wants to merge 1 commit into
NVIDIA:mainfrom
JunAr7112:fix_scan
Open

JunAr7112 wants to merge 1 commit into
NVIDIA:mainfrom
JunAr7112:fix_scan

Conversation

@JunAr7112

@JunAr7112 JunAr7112 commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Description

This PR is created in response to the following breaking pipeline changes:

  1. Pulse Scanners — Removal of --ssa $SSA_TOKEN Flag: Remove --ssa $SSA_TOKEN from any direct CLI invocations. The token is fetched from the environment variable SSA_TOKEN.

This change will use the cli invocation to fix this:
pulse-cli -n "${NSPECT_ID}" scan-image -i "${CONTAINER_IMAGE}" --platform "${PLATFORM}" -p "${CONTAINER_POLICY}" --output-dir="scan-results" -o

  1. Pulse Container Scanner — Docker / DIND capabilities removed: Migrate to registry based scanning which is daemonless. If your job requires Docker CLI commands independent of scanning, add those explicitly to your job definition. Docker login has been replaced with a config.json-based approach. The scanner container now runs as a non-root user. Config is written to $HOME/.docker/config.json (was hardcoded /root/.docker/). File permissions are locked down (chmod 700 ~/.docker, chmod 600 config.json). REGISTRY_AUTH_FILE is exported for skopeo compatibility

This change will replace docker login + docker pull + docker save with registry-based scanning via a $HOME/.docker/config.json and an exported REGISTRY_AUTH_FILE, and disables the inherited docker:dind sidecar with services: []

Checklist

  • [ x] No secrets, sensitive information, or unrelated changes
  • Lint checks passing (make lint)
  • Generated assets in-sync (make validate-generated-assets)
  • Go mod artifacts in-sync (make validate-modules)
  • Test cases are added for new code paths

Testing

Tested on Gitlab. Verified pipeline scan job is running now.

@coderabbitai

coderabbitai Bot commented Sep 9, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

📝 Walkthrough

Walkthrough

The .scan GitLab CI job now scans registry images directly with pulse-cli. It configures registry authentication through REGISTRY_AUTH_FILE, removes Docker-in-Docker usage, passes explicit image, platform, and policy arguments, allows scan failures, and stores files under scan-results/**/*. SSA token setup remains, but the token is no longer passed to the scan command.

Priority: ⬇️ Low

Merge Risk: 🟡 Moderate · up to d6d32

The direct registry scan can fail operationally or on policy checks without blocking image publication, allowing releases to proceed without a completed security scan. Failure handling should be narrowed before merge.


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

coderabbitai[bot]

This comment was marked as resolved.

@devin-ai-integration devin-ai-integration 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.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Devin Review

Comment thread .nvidia-ci.yml
REGISTRY_AUTH="$(printf "%s:%s" "${CI_REGISTRY_USER}" "${CI_REGISTRY_PASSWORD}" | base64 | tr -d '\n')"
- >
jq -n --arg registry "$CI_REGISTRY" --arg auth "$REGISTRY_AUTH"
'{auths: {($registry): {auth: $auth}}}' > "$HOME/.docker/config.json"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Lets do chmod 600 "$HOME/.docker/config.json" after writing it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated to include chmod 600 "$HOME/.docker/config.json"

Signed-off-by: Arjun <agadiyar@nvidia.com>
@rahulait

Copy link
Copy Markdown
Contributor

Overall, the change looks good to me. I am wondering if we should now mark this scan job a hard requirement for pipeline runs than a soft requirement like it was before. Adding it as a hard requirement will mean more steps to manually fix those failures whenever releasing, but just want to double check. @tariq1890 thoughts on this?

@JunAr7112

Copy link
Copy Markdown
Contributor Author

Overall, the change looks good to me. I am wondering if we should now mark this scan job a hard requirement for pipeline runs than a soft requirement like it was before. Adding it as a hard requirement will mean more steps to manually fix those failures whenever releasing, but just want to double check. @tariq1890 thoughts on this?

On most of the other repos the scan job is a hard requirement already. I'm not sure what the initial thinking was for not having that for the gpu operator, but making it a hard req would align with the other repos.

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.

2 participants