Skip to content

fix: align NVIDIA Docker file#137

Merged
rollandf merged 1 commit intoMellanox:masterfrom
rollandf:align-docker
Jan 21, 2026
Merged

fix: align NVIDIA Docker file#137
rollandf merged 1 commit intoMellanox:masterfrom
rollandf:align-docker

Conversation

@rollandf
Copy link
Member

No description provided.

@github-actions
Copy link

Thanks for your PR,
To run vendors CIs, Maintainers can use one of:

  • /test-all: To run all tests for all vendors.
  • /test-e2e-all: To run all E2E tests for all vendors.
  • /test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs, Maintainers can use one of:

  • /skip-all: To skip all tests for all vendors.
  • /skip-e2e-all: To skip all E2E tests for all vendors.
  • /skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
    Best regards.

@greptile-apps
Copy link

greptile-apps bot commented Jan 20, 2026

Greptile Summary

This PR upgrades the NVIDIA DOCA base image from version 3.1.0 to 3.2.1 in the SR-IOV network config daemon Dockerfile and improves tar extraction safety.

Key Changes:

  • Upgrades DOCA base image to version 3.2.1 to align with beta releases
  • Adds GPG verification bypass flags (--allow-unauthenticated and Acquire::AllowInsecureRepositories=true) due to DOCA 3.2.1 repository signing issues
  • Adds --no-same-owner flag to tar extraction for improved container build safety
  • Includes explanatory comments documenting the GPG key issue

Context: Previous review comments identified the security vulnerability introduced by bypassing GPG verification. The developer's approach acknowledges this is temporary alignment with the beta3 state, with plans to address it in beta.4. However, this represents a security compromise that allows unsigned/unverified packages to be installed.

Confidence Score: 2/5

  • This PR introduces a known security vulnerability by bypassing GPG package verification in container builds, though it maintains functional alignment with beta3 releases.
  • The low confidence score (2/5) reflects the deliberate introduction of a security compromise that bypasses package signature verification. While this aligns with previous beta releases and the developer was aware of the issue (evident from previous review comments), allowing unsigned packages to be installed from repositories is a significant security risk. The change does improve tar extraction safety with --no-same-owner, but this cannot offset the security concern. The PR title "align NVIDIA Docker file" suggests this is intentional alignment rather than a security improvement.
  • Dockerfile.sriov-network-config-daemon.nvidia requires careful attention - the GPG verification bypass is a critical security concern that should be addressed before merging, even though it aligns with beta3 state.

Important Files Changed

Filename Overview
Dockerfile.sriov-network-config-daemon.nvidia DOCA base image upgraded from 3.1.0 to 3.2.1 with GPG verification bypass for package installation and improved tar extraction. The security vulnerability from bypassing GPG verification was noted in previous reviews and flagged by hadolint. The --allow-unauthenticated and Acquire::AllowInsecureRepositories=true flags compromise package integrity verification. The --no-same-owner tar flag addition is a good improvement for container builds.

Sequence Diagram

sequenceDiagram
    participant Dev as Developer
    participant DockerBuild as Docker Build
    participant GoBuilder as Go Builder Stage
    participant APT as APT Package Manager
    participant DOCA as DOCA Repository (GPG Issue)
    participant MFT as MFT Download
    participant RuntimeImage as Runtime Image

    Dev->>DockerBuild: Build with Dockerfile.sriov-network-config-daemon.nvidia
    DockerBuild->>GoBuilder: Stage 1: Build Go binary
    GoBuilder->>GoBuilder: make _build-sriov-network-config-daemon
    GoBuilder-->>DockerBuild: Binary ready

    DockerBuild->>RuntimeImage: Stage 2: FROM DOCA 3.2.1-base-rt-host
    RuntimeImage->>APT: apt-get update -o Acquire::AllowInsecureRepositories=true
    APT->>DOCA: Request package metadata (GPG key missing)
    DOCA-->>APT: Repository metadata without valid signature
    APT->>RuntimeImage: Allow insecure repos (bypass GPG verification)

    RuntimeImage->>APT: apt-get install --allow-unauthenticated packages
    APT->>DOCA: Request hwdata, pciutils, curl, mstflint
    DOCA-->>APT: Unsigned packages (GPG verification skipped)
    APT->>RuntimeImage: Install packages without signature verification

    RuntimeImage->>MFT: curl -fsSL MFT tarball
    MFT-->>RuntimeImage: MFT tarball
    RuntimeImage->>RuntimeImage: tar -xz --no-same-owner (ownership preserved safely)
    RuntimeImage->>RuntimeImage: Install MFT, symlink mstconfig/mstfwreset

    DockerBuild->>RuntimeImage: Copy binary and sources
    RuntimeImage-->>DockerBuild: Container image ready

Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

2 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

Comment on lines 33 to 35
RUN apt-get update -o Acquire::AllowInsecureRepositories=true || true && \
apt-get install -y --allow-unauthenticated hwdata pciutils curl mstflint && \
apt-get clean && rm -rf /var/lib/apt/lists/*
Copy link

Choose a reason for hiding this comment

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

Using --allow-unauthenticated and -o Acquire::AllowInsecureRepositories=true introduces a security vulnerability by bypassing package signature verification. This allows packages from potentially compromised repositories to be installed without validation.

Consider:

  1. Adding the missing GPG key properly: apt-key adv --keyserver keyserver.ubuntu.com --recv-keys A024F6F0E6D6A281
  2. Contacting NVIDIA to resolve the GPG signing issue in their DOCA 3.2.1 repositories
  3. Temporarily pinning to DOCA 3.1.0 until the GPG issue is resolved

RUN apt-get update && apt-get install -y hwdata pciutils curl mstflint && apt-get clean && rm -rf /var/lib/apt/lists/*
# DOCA repositories have a GPG issue, so we need to allow insecure repositories.
# GPG error: https://linux.mellanox.com/public/repo/doca/3.2.1/ubuntu22.04/x86_64 ./ Release: The following signatures couldn't be verified because the public key is not available: NO_PUBKEY A024F6F0E6D6A281
RUN apt-get update -o Acquire::AllowInsecureRepositories=true || true && \
Copy link

Choose a reason for hiding this comment

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

The || true here will hide any actual errors from apt-get update, not just the GPG warning. If the update fails for legitimate reasons (network issues, repository unavailable), the build will continue silently and potentially install outdated packages.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@rollandf did you see this comment?

Copy link
Member Author

Choose a reason for hiding this comment

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

Currently this is aligned with what we had in beta3.
Do you want me to change it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

If this might hide future errors while building image, it might be good to remove || true. Lets add this in beta.4

Copy link
Member Author

Choose a reason for hiding this comment

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

done

Signed-off-by: Fred Rolland <frolland@nvidia.com>
@rollandf rollandf merged commit a3b6376 into Mellanox:master Jan 21, 2026
12 of 16 checks passed
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.

3 participants