Skip to content

fix(bazel): repin distroless_static and make the nvsnap base multi-arch - #580

Merged
balajinvda merged 5 commits into
mainfrom
fix/repin-distroless-static
Jul 30, 2026
Merged

fix(bazel): repin distroless_static and make the nvsnap base multi-arch#580
balajinvda merged 5 commits into
mainfrom
fix/repin-distroless-static

Conversation

@balajinvda

@balajinvda balajinvda commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Why

The Black Duck scan of this repository has been failing, and the cause is two
defects here rather than anything in the scanner. Both were found by reproducing
its command locally:

bazel cquery --noimplicit_deps 'kind(j.*import, deps(//...))' --output build

1. A dead base image digest. The root module pinned a
gcr.io/distroless/static digest that upstream had garbage-collected:

WARNING: Download from https://gcr.io/v2/distroless/static/manifests/
  sha256:dd7614b5... failed: GET returned 404 Not Found
ERROR: nvsnap_server_image_pre_transitioned depends on
  @@rules_oci++oci+distroless_static which failed to fetch

The tag still resolves; the manifest behind that digest does not. Digests are
immutable, but registries garbage-collect untagged manifests, so a pin rots
without anyone editing it.

2. A base/macro platform mismatch, hidden behind the first. With the digest
repinned, analysis failed one layer deeper:

ERROR: configurable attribute "actual" in @@rules_oci++oci+distroless_static
  doesn't match this configuration: could not find an image matching the
  target platform.
ERROR: Analysis of target nvsnap_agent_image_multi_arch failed

go_oci_image always emits a _multi_arch target over DEFAULT_PLATFORMS
(arm64 and x86_64), but @distroless_static was pulled linux/amd64 only, so
the arm64 branch of the transition had no image. nvsnap was the only service
whose base disagreed with the shared macro.

Why CI never caught either

bazel build //... skips these targets because they are tagged manual.
cquery 'deps(//...)' ignores manual and analyses them, so an external scan
hits what our own wildcard build never touches.

The dead pin was additionally masked because CI restores the Bazel repository
cache, so the fetch was served from cache and the pin looked healthy. That was
live risk, not theory: the Actions cache is over quota and evicting
continuously, so this was one eviction away from breaking main with an error
that reads like a network blip.

What changed

distroless_static is repinned to a current digest and pulled for both
architectures, matching the macro and every other service. Its use_repo gains
the _linux_arm64_v8 entry.

tools/ci/check-oci-pins verifies that every oci.pull digest still resolves,
across all 23 pins in the repository. It is meant to run on a schedule rather
than per change, since a pin rots with no commit touching it.

Testing

The scanner's exact command now succeeds on Linux against //...:

EXIT=0
java_import targets found: 347

Previously it produced no output at all. nvsnap_agent_image_multi_arch also
analyses cleanly.

check-oci-pins reports all 23 pins resolving. It ships with a self-test
(--self-test) covering its reference handling, because while investigating
this I twice reported healthy pins as dead: appending @digest to a reference
that already carries a tag yields an invalid reference that skopeo rejects. The
check also retries before declaring a pin dead, so a registry timeout is not
mistaken for a missing image, and it fails if it parses zero pins rather than
reporting success over an empty set.

Notes

Worth knowing for the SBOM effort: a scan of //... from the repository root
covers only the root Bazel module. The ~20 nested modules each have their own
MODULE.bazel and are not in that target universe, so this unblocks the scan
but does not by itself make it complete. Our CI already enumerates those
modules and their workdirs.

References

None

Dependencies

None.

Summary by CodeRabbit

  • Bug Fixes

    • Updated the Distroless base image pin to support both linux/amd64 and linux/arm64/v8.
    • Repinned the base image digest for correctness.
  • Chores

    • Replaced the OCI pin verification with a Python-based checker that validates image/digest pins and declared platform support.
    • Improved parsing (ignores comment text), adds inspection timeouts with retries, enhanced per-pin status reporting, and includes a self-test mode.

balajinvda and others added 2 commits July 30, 2026 11:21
`bazel build //...` fails at analysis on any checkout without a warm Bazel
repository cache:

    ERROR: src/compute-plane-services/nvsnap/BUILD.bazel:66:13:
      nvsnap_server_image_pre_transitioned depends on
      @@rules_oci++oci+distroless_static which failed to fetch
    WARNING: Download from https://gcr.io/v2/distroless/static/manifests/
      sha256:dd7614b5... failed: GET returned 404 Not Found

The pinned digest was garbage-collected upstream. gcr.io answers it with
"manifest unknown" while the tag itself still resolves, so this is not
authentication or network. Digests are immutable but the manifests behind them
are not retained forever once untagged.

CI never noticed because it restores the Bazel repository cache, so the fetch
was served from cache and a dead pin looked healthy. That is fragile in a
specific, already-live way: the Actions cache is over its quota and evicting
continuously, so this was one eviction away from breaking main with an error
that reads like a network blip. It had already blocked an external Black Duck
scan, which runs cold and hit it immediately.

check-oci-pins verifies every oci.pull digest still resolves. It is a scheduled
check rather than a per-change one, because a pin rots without anyone editing
it.

The check carries a self-test for its reference handling. While investigating
this I twice reported healthy pins as dead, because appending @digest to an
image reference that already carries a tag produces an invalid reference that
skopeo rejects. It also retries before declaring a pin dead, so a registry
timeout is not mistaken for a missing image, and it fails if it parses no pins
at all rather than reporting success over an empty set.

Co-authored-by: Balaji Ganesan <bganesan@nvidia.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Repinning the digest exposed a second failure one layer down:

    ERROR: configurable attribute "actual" in @@rules_oci++oci+distroless_static
      doesn't match this configuration: could not find an image matching the
      target platform.
    ERROR: Analysis of target
      //src/compute-plane-services/nvsnap:nvsnap_agent_image_multi_arch failed

go_oci_image always emits a _multi_arch target over DEFAULT_PLATFORMS, which is
arm64 and x86_64. @distroless_static was pulled for linux/amd64 only, so the
arm64 branch of the transition had no matching image. nvsnap was the only
service whose base disagreed with the macro.

`bazel build //...` never showed this because those targets are tagged manual
and wildcard builds skip them. `cquery deps(//...)` ignores manual, so an
external scan hit it immediately while CI stayed green.

With both fixes the exact Black Duck command now succeeds on Linux against
//..., returning 347 java_import targets where it previously produced none.

Co-authored-by: Balaji Ganesan <bganesan@nvidia.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@balajinvda
balajinvda requested review from a team as code owners July 30, 2026 18:26
@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 8028c8f2-5efb-4e0b-89e2-1a2e4168f3d7

📥 Commits

Reviewing files that changed from the base of the PR and between 3a8c0cc and a90ccf1.

📒 Files selected for processing (1)
  • tools/ci/check-oci-pins

📝 Walkthrough

Walkthrough

The OCI module configuration repins the distroless static image and adds arm64 support. A Python CI utility parses OCI pins, validates digest resolution and declared platform coverage with skopeo, retries transient failures, and reports invalid pins.

Changes

OCI pin management

Layer / File(s) Summary
Enable multi-architecture distroless image
MODULE.bazel
The distroless static image uses a new digest, supports linux/amd64 and linux/arm64/v8, and exposes the arm64 repository.
Parse pinned images and platforms
tools/ci/check-oci-pins
The utility parses oci.pull declarations, strips comments safely, and checks exact declared platform and variant coverage in OCI manifests.
Validate pinned OCI digests
tools/ci/check-oci-pins
Manifest inspection uses timeouts and retries, self-tests cover parsing and platform matching, and the main command reports DEAD or NOPLAT pins with corresponding exit status.

Estimated code review effort: 4 (Complex) | ~45 minutes

Suggested reviewers: kristinapathak

Sequence Diagram(s)

sequenceDiagram
  participant check_oci_pins
  participant MODULE_bazel
  participant skopeo
  participant OCIRegistry
  check_oci_pins->>MODULE_bazel: parse OCI image, digest, and platforms
  check_oci_pins->>skopeo: inspect image@digest
  skopeo->>OCIRegistry: request raw manifest
  OCIRegistry-->>skopeo: return manifest or resolution error
  skopeo-->>check_oci_pins: return result after retries
  check_oci_pins->>check_oci_pins: report DEAD, NOPLAT, or ok
Loading
🚥 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 follows Conventional Commits and matches the main change: repinning distroless_static and fixing the nvsnap base image for multi-arch.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/repin-distroless-static

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

Pulling distroless_static for linux/arm64/v8 in addition to linux/amd64 changes
module resolution, so the committed lockfile has to move with it. Leaving it
stale makes Bazel either rewrite it mid-build or fail outright, depending on
lockfile mode.

Co-authored-by: Balaji Ganesan <bganesan@nvidia.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.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.

Actionable comments posted: 2

🤖 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 `@tools/ci/check-oci-pins`:
- Around line 24-28: Move the SKOPEO command-availability guard below the
--self-test option branch in the script. Ensure --self-test reaches its
strip_tag-only execution without requiring skopeo, while normal pin verification
still validates SKOPEO before proceeding.
- Around line 81-92: Extend the OCI pin check’s Python extraction in
check-oci-pins to parse each oci.pull platforms list alongside image and digest,
then inspect the resolved OCI index manifests and verify every requested
OS/architecture/variant descriptor is present, including linux/arm64/v8
matching. Add a focused self-test demonstrating that a missing arm64/v8 entry
fails while existing digest validation remains intact.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: faa25c78-e1c6-4974-a693-3d0693617170

📥 Commits

Reviewing files that changed from the base of the PR and between 620d47e and f335116.

📒 Files selected for processing (2)
  • MODULE.bazel
  • tools/ci/check-oci-pins

Comment thread tools/ci/check-oci-pins Outdated
Comment thread tools/ci/check-oci-pins Outdated
Verifying that a digest resolves would not have caught the second defect this
pull request fixes. nvsnap's base resolved perfectly; it simply lacked the
arm64 entry the shared macro asks for, and that fails later at analysis with
"could not find an image matching the target platform". The check now parses
each pin's platforms list and asserts the index advertises every one, matching
os/arch/variant exactly because rules_oci compares the platform string exactly.

Rewritten in Python: this is manifest JSON work, and the matching logic needs
to be unit-testable without a registry.

The self-test covers the case that matters, an index missing arm64/v8, plus
arm64 not matching arm64/v8, single-image manifests satisfying nothing, and the
reference handling that produced earlier false positives. It runs before the
skopeo dependency check so it works on a machine without skopeo.

Comment stripping is in there for a reason worth recording: function-autoscaler
documents why it uses arm64/v8 by quoting the wrong value, "linux/arm64", in a
comment inside the platforms list. Scanning for quoted strings without removing
comments read that as a declaration and reported a correct pin as broken. The
first run of this check produced exactly that false positive, so the fix ships
with a regression test in its shape.

Co-authored-by: Balaji Ganesan <bganesan@nvidia.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@balajinvda
balajinvda enabled auto-merge July 30, 2026 19:11

@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: 1

🤖 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 `@tools/ci/check-oci-pins`:
- Around line 149-174: Update subprocess.run in fetch_manifest to enforce a
per-call timeout for each skopeo inspect attempt, using the existing retry flow
to handle timeout failures and continue or return according to the established
behavior. Ensure a stalled registry cannot block the retry loop indefinitely.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 6936cb1d-868e-4e3f-9da0-74d1e8057227

📥 Commits

Reviewing files that changed from the base of the PR and between f335116 and 3a8c0cc.

⛔ Files ignored due to path filters (1)
  • MODULE.bazel.lock is excluded by !**/*.lock, !**/MODULE.bazel.lock
📒 Files selected for processing (1)
  • tools/ci/check-oci-pins

Comment thread tools/ci/check-oci-pins
subprocess.run without a timeout blocks indefinitely, so a stalled registry or
proxy would hang the retry loop and take the whole scheduled sweep with it,
losing every result rather than one pin's.

Each inspect is now capped at 60s. A timeout is treated like any other
transient failure: retried, and if it persists reported as unresolved rather
than as a dead pin, since a hung connection is not evidence that an image is
gone.

Co-authored-by: Balaji Ganesan <bganesan@nvidia.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@balajinvda
balajinvda added this pull request to the merge queue Jul 30, 2026
Merged via the queue into main with commit 6ec8f0d Jul 30, 2026
18 checks passed
@balajinvda
balajinvda deleted the fix/repin-distroless-static branch July 30, 2026 20:18
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