fix: update opm build toolchain and fulcio - #2034
Conversation
|
Hi @russell-parks. Thanks for your PR. I'm waiting for a operator-framework member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
There was a problem hiding this comment.
Pull request overview
This pull request updates the Operator Registry’s build toolchain and Go module dependencies to pick up security fixes (patched Go stdlib and a Fulcio CVE fix), aligning the Docker-based upstream builder path and the module’s Go version used by CI/GoReleaser.
Changes:
- Bumped the module Go version to Go 1.26.5 (CI uses
go-version-file: go.mod, so this propagates to GitHub Actions). - Pinned
upstream-builder.Dockerfileto golang:1.26.5-alpine for the upstream builder path. - Updated
github.com/sigstore/fulcioto v1.8.6 and incorporated the minimalgo mod tidy-driven dependency checksum updates.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| upstream-builder.Dockerfile | Pins the upstream builder image to Go 1.26.5 to ensure patched stdlib in that build path. |
| go.mod | Updates the module Go version and bumps Fulcio (plus associated indirect deps) to pick up security fixes. |
| go.sum | Updates dependency checksums consistent with the go.mod bumps / tidy result. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2034 +/- ##
=======================================
Coverage 58.81% 58.81%
=======================================
Files 141 141
Lines 13434 13434
=======================================
Hits 7901 7901
Misses 4323 4323
Partials 1210 1210 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
The only failing check is The rest of CI is passing. Could a maintainer please add the |
|
/hold |
|
Thanks for the context. Is the blocker specifically that the OLM/operator-framework release train is currently constrained to the OpenShift 4.23 / ART-supported Go If so, would you prefer that I retarget this PR to only address the Fulcio/dependency findings while leaving the root |
|
Hey @russell-parks. Yes, that's our challenge exactly. But we now have 1.26.5 capability so I think we're safe to proceed. /hold cancel |
|
@grokspawn: The label(s) DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tmshort The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/lgtm |
62c07bd to
31c5afc
Compare
|
New changes are detected. LGTM label has been removed. |
|
/test all |
|
@russell-parks: No presubmit jobs available for operator-framework/operator-registry@master DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (1)
go.mod:3
- PR description says Fulcio is bumped to v1.8.6, but go.mod currently resolves github.com/sigstore/fulcio to v1.8.7 (indirect). Please reconcile this (either update the PR description/Trivy evidence to match the actual dependency version, or pin/tidy back to v1.8.6 if that is the intended target).
go 1.26.5
31c5afc to
8773a24
Compare
📝 WalkthroughWalkthroughThe required Go version in ChangesGo version alignment
Estimated code review effort: 1 (Trivial) | ~2 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Rebased again onto current Local validation passed: @grokspawn when you have a chance, could you please re-apply |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (1)
go.mod:3
- PR description says Fulcio is bumped to v1.8.6, but the current module state references github.com/sigstore/fulcio v1.8.7 (indirect) in go.mod/go.sum. Please align the PR description (and any verification notes) with the actual Fulcio version being shipped, or explicitly pin Fulcio to the intended version if v1.8.6 is required.
go 1.26.5
There was a problem hiding this comment.
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 `@upstream-builder.Dockerfile`:
- Line 1: Update the final runtime stage of upstream-builder.Dockerfile to run
as a non-root user by adding a USER directive after /build, /bin, and
/etc/nsswitch.conf are populated. Ensure the required runtime paths are owned or
writable by that user during image setup, while leaving the build-only builder
stage unchanged.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8240af61-cf23-47d1-9ce1-d8b17fbe47d0
📒 Files selected for processing (2)
go.modupstream-builder.Dockerfile
| @@ -1,4 +1,4 @@ | |||
| FROM golang:1.26-alpine AS builder | |||
| FROM golang:1.26.5-alpine AS builder | |||
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 5 \
'upstream-builder\.Dockerfile|docker build|docker run|podman build|podman run|image:.*builder' .Repository: operator-framework/operator-registry
Length of output: 10999
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== upstream-builder.Dockerfile =="
cat -n upstream-builder.Dockerfile
echo
echo "== Makefile relevant section =="
sed -n '80,100p' Makefile | cat -n
echo
echo "== targeted references =="
rg -n "\bsanity-container\b|upstream-builder\.Dockerfile|-f upstream-builder\.Dockerfile|registry serve" .Repository: operator-framework/operator-registry
Length of output: 2587
Run the runtime image as non-root.
The builder stage is build-only, but the final stage served by make sanity-check runs as the default root user. Add a USER after populating /build, /bin, and /etc/nsswitch.conf, and ensure those paths remain writable/owned correctly during setup if needed.
🧰 Tools
🪛 Trivy (0.72.0)
[error] 1-1: Image user should not be 'root'
Specify at least 1 USER command in Dockerfile with non-root user as argument
Rule: DS-0002
(IaC/Dockerfile)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@upstream-builder.Dockerfile` at line 1, Update the final runtime stage of
upstream-builder.Dockerfile to run as a non-root user by adding a USER directive
after /build, /bin, and /etc/nsswitch.conf are populated. Ensure the required
runtime paths are owned or writable by that user during image setup, while
leaving the build-only builder stage unchanged.
Source: Linters/SAST tools
Summary
Trivy evidence
Before, quay.io/operator-framework/opm:master at sha256:a9f84574354fb46686fd0958b0ab5f6f49d6b2736c5c99f10e05d6d92cca10ea failed Trivy 0.70.0 with HIGH findings in usr/bin/opm:
After, I built a local linux/arm64 release-shaped image using Go 1.26.5, distroless static, /bin/opm, /bin/grpc_health_probe, nsswitch.conf, user 1001, and the opm entrypoint. The image ID was sha256:7a7598e588808493efb14e9b9398939e6d836509983b9cca4cfcc1197dc10a37.
Trivy command run locally, adapted only for the OrbStack Docker socket path:
Result:
Verification
No vulnerability suppressions, ignores, or scan weakening were added.
Summary by CodeRabbit