OCPBUGS-87336: Updating ose-route-controller-manager-container image to be consistent with ART for 5.0#98
Conversation
Signed-off-by: Aswin Suryanarayanan <asuryana@redhat.com>
openshift-ci-robot
added
jira/severity-moderate
|
@aswinsuryan: This pull request references Jira Issue OCPBUGS-87336, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: The bug has been updated to refer to the pull request using the external bug tracker. 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 openshift-eng/jira-lifecycle-plugin repository. |
WalkthroughUpdates the Go toolchain directive in ChangesGo 1.26 / OpenShift 5.0 upgrade
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Dockerfile (1)
6-11: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winSet a non-root runtime user in the final stage.
Line 6 onward defines the runtime image, but no
USERis set, so the container runs as root by default.Suggested fix
FROM registry.ci.openshift.org/ocp/5.0:base-rhel9 COPY --from=builder /go/src/github.com/openshift/route-controller-manager/route-controller-manager /usr/bin/ +USER 1001 LABEL io.k8s.display-name="OpenShift Route Controller Manager Command" \ io.k8s.description="OpenShift is a platform for developing, building, and deploying containerized applications." \ io.openshift.tags="openshift,route-controller-manager"As per path instructions, "USER non-root; never run as root" is required for Dockerfile paths.
🤖 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` around lines 6 - 11, The final runtime stage in the Dockerfile (starting with the FROM registry.ci.openshift.org/ocp/5.0:base-rhel9 instruction) lacks a USER directive, causing the container to run as root. Add a USER directive after the LABEL statements to specify a non-root user for runtime execution. Use a numeric user ID (such as 1000) or a non-privileged user account defined in the base image to comply with security requirements.Sources: Path instructions, Linters/SAST tools
🤖 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 `@Dockerfile`:
- Line 3: Replace the COPY . . command in the builder stage with specific COPY
statements that only copy the files and directories required for the build.
Instead of copying the entire context, identify which files are necessary (such
as package.json, source code directories, configuration files, etc.) and use
individual COPY commands for each required item. This reduces the image size,
improves build security by excluding unnecessary files, and minimizes the attack
surface.
---
Outside diff comments:
In `@Dockerfile`:
- Around line 6-11: The final runtime stage in the Dockerfile (starting with the
FROM registry.ci.openshift.org/ocp/5.0:base-rhel9 instruction) lacks a USER
directive, causing the container to run as root. Add a USER directive after the
LABEL statements to specify a non-root user for runtime execution. Use a numeric
user ID (such as 1000) or a non-privileged user account defined in the base
image to comply with security requirements.
🪄 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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 15a8bce2-3b6b-46f6-b515-5049a6c3b13a
📒 Files selected for processing (3)
.ci-operator.yamlDockerfilego.mod
| FROM registry.ci.openshift.org/ocp/builder:rhel-9-golang-1.25-openshift-4.22 AS builder | ||
| FROM registry.ci.openshift.org/ocp/builder:rhel-9-golang-1.26-openshift-5.0 AS builder | ||
| WORKDIR /go/src/github.com/openshift/route-controller-manager | ||
| COPY . . |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Avoid COPY . . in the builder stage.
Line 3 copies the full context, which can unintentionally include sensitive or irrelevant files and expands attack surface. Copy only required files/directories.
Suggested fix
WORKDIR /go/src/github.com/openshift/route-controller-manager
-COPY . .
+COPY go.mod go.sum ./
+COPY cmd/ ./cmd/
+COPY pkg/ ./pkg/
+COPY Makefile ./
RUN make build --warn-undefined-variablesAs per path instructions, Dockerfiles should "COPY specific files, not entire context."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| COPY . . | |
| WORKDIR /go/src/github.com/openshift/route-controller-manager | |
| COPY go.mod go.sum ./ | |
| COPY cmd/ ./cmd/ | |
| COPY pkg/ ./pkg/ | |
| COPY Makefile ./ | |
| RUN make build --warn-undefined-variables |
🤖 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` at line 3, Replace the COPY . . command in the builder stage with
specific COPY statements that only copy the files and directories required for
the build. Instead of copying the entire context, identify which files are
necessary (such as package.json, source code directories, configuration files,
etc.) and use individual COPY commands for each required item. This reduces the
image size, improves build security by excluding unnecessary files, and
minimizes the attack surface.
Source: Path instructions
There was a problem hiding this comment.
@CodeRabbit this is outside the scope of this PR. We can address in a followup.
There was a problem hiding this comment.
@candita, understood — that's a reasonable call for a targeted version-bump PR. Would you like me to open a GitHub issue to track the COPY . . improvement as a follow-up task?
|
@aswinsuryan: 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. |
I did not see where codeRabbit asked for the go.mod update in #95, and wondering if 1.26.3 is too new and it should be 1.26.0. However, as we discussed today, the builder will use the go version it selects. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aswinsuryan, candita 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 |
openshift-ci
Bot
added
the
approved
Ya that is right the coderabbit , complained about it in openshift/coredns#187 (review). I saw the same pattern here and hence updated go.mod. |
3 participants
Description:
Updates for OpenShift 5.0 ART alignment:
Update base image: registry.ci.openshift.org/ocp/4.22:base-rhel9 → registry.ci.openshift.org/ocp/5.0:base-rhel9
Update builder image: rhel-9-golang-1.25-openshift-4.22 → rhel-9-golang-1.26-openshift-5.0
Align go.mod: 1.25.0 → 1.26.3
Updates: #95
Summary by CodeRabbit