ROSAENG-61450 | ci: add presubmit client image and go-toolset 1.26.5 - #3366
Conversation
openshift-ci
Bot
added
the
approved
|
Warning Review limit reached
Next review available in: 9 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughUpdates the repository to Go 1.26.5 across module declarations and CI-related Dockerfiles, adds a Prow presubmit client image, and configures the OpenShift CI builder root image. Documentation now records CI image responsibilities and Go-version synchronization steps. Previously ignored govulncheck and Snyk findings are removed. Possibly related issues
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (13 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
Dockerfile.clients (1)
6-7: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winMake the
jqinstallation reproducible.
dnf install jqresolves the repository’s current package at build time, allowing the CI toolchain to change between rebuilds. Pin an approved RPM version or document and verify the intended floating-update policy.As per path instructions, unpinned downloads in container builds should be flagged.
🤖 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 `@Dockerfile.clients` around lines 6 - 7, Update the jq installation in the Dockerfile’s dnf command to use an approved, explicitly pinned RPM version rather than resolving the latest repository package. Preserve the existing weak-dependency and documentation options and cleanup step, and ensure the selected version is available from the configured repositories.Source: Path instructions
🤖 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 `@AGENTS.md`:
- Around line 75-80: Update the presubmit statement near the Dockerfile table to
describe rosa-clients execution as intended or future behavior, not as currently
active. Keep the deferred openshift/release wiring consistent with the PR
objective, or remove the statement until the follow-up release configuration
lands.
In `@Dockerfile.clients`:
- Around line 4-11: Update the Dockerfile user configuration so root is used
only for package installation and permission setup, then switch the final image
to the documented non-root UID after the existing mkdir/chgrp/chmod commands.
Ensure the final active USER is non-root and no later instruction restores root
privileges.
- Around line 8-14: Update the Git configuration setup in Dockerfile.clients to
avoid trusting every checkout and ensure it applies to the runtime environment:
replace the global wildcard safe.directory entry with a system-level entry for
/opt/app-root/src, adding other required checkout paths explicitly only if
needed.
---
Nitpick comments:
In `@Dockerfile.clients`:
- Around line 6-7: Update the jq installation in the Dockerfile’s dnf command to
use an approved, explicitly pinned RPM version rather than resolving the latest
repository package. Preserve the existing weak-dependency and documentation
options and cleanup step, and ensure the selected version is available from the
configured repositories.
🪄 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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 2e98f9cf-5546-4e22-bb94-c5684565d693
📒 Files selected for processing (3)
.ci-operator.yamlAGENTS.mdDockerfile.clients
| RUN git config --global --add safe.directory '*' | ||
| RUN mkdir -p /opt/app-root/src /tmp/gomodcache /tmp/.cache && \ | ||
| chgrp -R 0 /opt/app-root/src /tmp/gomodcache /tmp/.cache && \ | ||
| chmod -R g+rwX /opt/app-root/src /tmp/gomodcache /tmp/.cache | ||
| ENV HOME="/opt/app-root/src" \ | ||
| GOMODCACHE="/tmp/gomodcache" \ | ||
| GOLANGCI_LINT_CACHE="/tmp/.cache" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- Dockerfile.clients (numbered excerpt) ---'
cat -n Dockerfile.clients | sed -n '1,120p'
echo
echo '--- safe.directory occurrences ---'
rg -n --hidden --glob '!vendor' --glob '!**/node_modules/**' 'safe\.directory|git config --global --add safe\.directory|git config --system --add safe\.directory' .
echo
echo '--- opt/app-root/src occurrences ---'
rg -n --hidden --glob '!vendor' '/opt/app-root/src|app-root/src' .Repository: openshift/rosa
Length of output: 2084
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- files referencing Dockerfile.clients ---'
rg -n --hidden --glob '!vendor' 'Dockerfile\.clients|clients image|Prow presubmit client image' .
echo
echo '--- ci-operator / prow configs mentioning /opt/app-root/src or workspace paths ---'
rg -n --hidden --glob '!vendor' '/opt/app-root/src|/workspace|safe\.directory|dockerfile.clients|clients' .github ci . 2>/dev/null || true
echo
echo '--- Dockerfile (numbered excerpt) ---'
cat -n Dockerfile | sed -n '1,80p'Repository: openshift/rosa
Length of output: 5516
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- files referencing Dockerfile.clients ---'
rg -n --hidden --glob '!vendor' 'Dockerfile\.clients|clients image|Prow presubmit client image' .
echo
echo '--- ci-operator / prow configs mentioning /opt/app-root/src or workspace paths ---'
rg -n --hidden --glob '!vendor' '/opt/app-root/src|/workspace|safe\.directory|dockerfile\.clients|clients' .github ci . 2>/dev/null || true
echo
echo '--- Dockerfile (numbered excerpt) ---'
cat -n Dockerfile | sed -n '1,80p'Repository: openshift/rosa
Length of output: 5516
Narrow the Git safe-directory setup in Dockerfile.clients.
git config --global --add safe.directory '*' trusts every checkout, and because HOME is set later the config lands under /root instead of the runtime home. Use git config --system --add safe.directory /opt/app-root/src (or move HOME earlier) and add any other checkout paths explicitly if needed.
🤖 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 `@Dockerfile.clients` around lines 8 - 14, Update the Git configuration setup
in Dockerfile.clients to avoid trusting every checkout and ensure it applies to
the runtime environment: replace the global wildcard safe.directory entry with a
system-level entry for /opt/app-root/src, adding other required checkout paths
explicitly only if needed.
Source: Path instructions
There was a problem hiding this comment.
@amandahla The wildcard was fixed to a specific path in the latest push, but --global vs --system still matters. Since --global writes to the build-time user's HOME and the runtime user differs under Prow's arbitrary UID, git config --system --add safe.directory /opt/app-root/src is the robust fix.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n Dockerfile.clientsLength of output: 1207
@olucasfreitas You're right, and the current file confirms it. Line 9 runs git config --global while HOME still defaults to root's home (e.g., /root), but ENV HOME="/opt/app-root/src" is only set afterwards on Line 15. So at runtime, git looks for config in /opt/app-root/src/.gitconfig, which was never populated — the safe.directory entry written during build is effectively orphaned. This is exactly the failure mode you described, and it's made worse under Prow's arbitrary UID, since --system (/etc/gitconfig) is readable regardless of runtime UID/HOME, while a --global entry tied to a specific HOME may not be visible to a different UID at all.
Suggested fix:
RUN git config --system --add safe.directory /opt/app-root/srcThis should be a straightforward, low-risk swap on Line 9.
✏️ Learnings added
Learnt from: olucasfreitas
Repo: openshift/rosa PR: 3366
File: Dockerfile.clients:0-0
Timestamp: 2026-07-14T14:57:46.755Z
Learning: In openshift/rosa's `Dockerfile.clients` (and similar Prow/OpenShift-style container images that run with an arbitrary UID and later override `HOME`), `git config --global --add safe.directory <path>` writes to the build-time HOME's `.gitconfig`, which becomes orphaned once `ENV HOME` is reset to a different path later in the Dockerfile, or is invisible to a different runtime UID. Prefer `git config --system --add safe.directory <path>` (writes to `/etc/gitconfig`), which is readable regardless of runtime UID or HOME.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
@amandahla I saw this comment was resolved, but was this addressed ?
There was a problem hiding this comment.
Outdated.
See RUN git config --global --add safe.directory /opt/app-root/src
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
amandahla
changed the title
4aa03d2 to
397355b
Compare
|
Consider this in favor of #3359 |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
397355b to
868f6c5
Compare
amandahla
changed the title
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
|
security issues are related to bumping Go, not to this PR |
|
/hold |
openshift-ci
Bot
added
the
do-not-merge/hold
Add Dockerfile.clients for Prow presubmits, pin build_root via .ci-operator.yaml, document CI images in AGENTS.md, align product/E2E Dockerfiles to ubi9/go-toolset 1.26.5, and track those Dockerfiles in renovate. Leave go.mod at 1.26.3 until openshift/release switches presubmits to rosa-clients; then bump go.mod to 1.26.5. Signed-off-by: Amanda Hager Lopes de Andrade Katz <amanda.katz@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Amanda Hager Lopes de Andrade Katz <amanda.katz@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
868f6c5 to
ab75238
Compare
|
/unhold |
openshift-ci
Bot
removed
the
do-not-merge/hold
|
/override ci/prow/govulncheck |
|
@amandahla: Overrode contexts on behalf of amandahla: ci/prow/govulncheck, ci/prow/security 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. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: amandahla, olucasfreitas 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 |
|
@amandahla: all tests passed! Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
Depends on openshift/rosa#3366 (Dockerfile.clients + .ci-operator.yaml). Switch rosa ci-operator configs to build_root from_repository, run presubmits from rosa-clients, and regenerate prow jobs. Signed-off-by: Amanda Hager Lopes de Andrade Katz <amanda.katz@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Depends on openshift/rosa#3366 (Dockerfile.clients + .ci-operator.yaml). Switch rosa ci-operator configs to build_root from_repository, run presubmits from rosa-clients, and regenerate prow jobs. Stop skipping jobs when only .ci-operator.yaml changes now that build_root uses from_repository: true. Signed-off-by: Amanda Hager Lopes de Andrade Katz <amanda.katz@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Depends on openshift/rosa#3366 (Dockerfile.clients + .ci-operator.yaml). Switch rosa ci-operator configs to build_root from_repository, run presubmits from rosa-clients, and regenerate prow jobs. Stop skipping jobs when only .ci-operator.yaml changes now that build_root uses from_repository: true. Signed-off-by: Amanda Hager Lopes de Andrade Katz <amanda.katz@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Depends on openshift/rosa#3366 (Dockerfile.clients + .ci-operator.yaml). Switch rosa ci-operator configs to build_root from_repository, run presubmits from rosa-clients, and regenerate prow jobs. Stop skipping jobs when only .ci-operator.yaml changes now that build_root uses from_repository: true. Signed-off-by: Amanda Hager Lopes de Andrade Katz <amanda.katz@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Follow-up to openshift#3366: align go.mod, bingo tool modules, and renovate Go constraint with ubi9/go-toolset 1.26.5 already used by Dockerfiles. Drop deferred stdlib Snyk and govulncheck ignores fixed in Go >= 1.26.4. Depends on openshift/release#81830 (from: rosa-clients) so Prow presubmits compile with go-toolset 1.26.5 instead of ocp/builder 1.26.3. Signed-off-by: Amanda Hager Lopes de Andrade Katz <amanda.katz@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Amanda Hager Lopes de Andrade Katz <amanda.katz@redhat.com>
![openshift-prow[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
Depends on openshift/rosa#3366 (Dockerfile.clients + .ci-operator.yaml). Switch rosa ci-operator configs to build_root from_repository, run presubmits from rosa-clients, and regenerate prow jobs. Stop skipping jobs when only .ci-operator.yaml changes now that build_root uses from_repository: true. Signed-off-by: Amanda Hager Lopes de Andrade Katz <amanda.katz@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
2 participants
PR Summary
Add a Prow presubmit client image (
Dockerfile.clients), pin ci-operatorbuild_rootin.ci-operator.yaml, document CI images inAGENTS.md, align Dockerfiles toubi9/go-toolset:1.26.5, and update Renovate to track all Dockerfiles.go.modstays at 1.26.3 so current Prowfrom: srcjobs (Go 1.26.3,GOTOOLCHAIN=local) keep passing. Bumpgo.modto 1.26.5 in a follow-up afteropenshift/releaseswitches torosa-clients.Detailed Description of the Issue
ROSA presubmits run on
ocp/builderviafrom: src. We need a repo-owned client image (RHCS-style) on go-toolset. ci-operator does not re-clone into custom images fromimages:unlessclone: true, soDockerfile.clientsmust COPY the PR tree (same lesson as terraform-provider-rhcs#1257). Without COPY, the follow-up release PR would fail with missingMakefile.Related Issues and PRs
from: rosa-clients(waits for this PR)from: *-clientsissuego.modto 1.26.5 after the release PR landsType of Change
Previous Behavior
Presubmit jobs use
ocp/builder(from: src). No repo-local presubmit client Dockerfile or.ci-operator.yaml.Behavior After This Change
Dockerfile.clientson go-toolset 1.26.5 withjq, COPY of the repo, and Prow-friendly permissions.ubi9/go-toolset:1.26.5-1783931515..ci-operator.yamlpinsbuild_roottorhel-9-golang-1.26-openshift-4.23.renovate.jsontracks all ROSA Dockerfiles.AGENTS.mddocuments CI images, COPY behavior, and Go bump checklist.go.modremains 1.26.3 until after the release PR.How to Test (Step-by-Step)
Preconditions
Test Steps
podman build -f Dockerfile.clients -t rosa-clients:local .podman run --rm -w /opt/app-root/src rosa-clients:local \ bash -c 'export GOTOOLCHAIN=auto && make rosa'npx --yes --package renovate -- renovate-config-validatorbuild/lint/teststill pass (from: srcuntil release PR).Expected Results
Image contains the repo;
make rosaworks without-vmounts. Bind-mount testing alone is not enough.Proof of the Fix
podman build -f Dockerfile.clientssucceeded with COPY.make rosasucceeded.Makefilemissing (confirms the failure mode).Breaking Changes
Developer Verification Checklist
[JIRA-TICKET] | [TYPE]: <MESSAGE>.make install-hookshas been run in this clone.make testpasses.make lintpasses.make rosapasses.