WIP: Update images for 5.0#2286
Conversation
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
WalkthroughBuild tooling and container base images were updated to Go 1.26 and OpenShift 5.0 across CI configuration and Dockerfiles. ChangesBuild Infrastructure Upgrade
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: tchap The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
images/cli/Dockerfile.rhel (1)
7-16:⚠️ Potential issue | 🟠 Major | ⚖️ Poor tradeoffAdd USER directive to run as non-root.
The Dockerfile does not specify a USER directive, causing the container to run as root by default. This violates the container security guidelines which require "USER non-root; never run as root." As per coding guidelines, all containers should run with a non-root user for security hardening.
🛡️ Proposed fix to add non-root user
Add a USER directive before the final CMD/LABEL declarations:
RUN for i in kubectl openshift-deploy openshift-docker-build openshift-sti-build openshift-git-clone openshift-manage-dockerfile openshift-extract-image-content openshift-recycle; do ln -sf /usr/bin/oc /usr/bin/$i; done +USER 1001 LABEL io.k8s.display-name="OpenShift Client" \Note: Verify that UID 1001 is appropriate for your security context, or use a different non-root UID as required by your organization's standards.
🤖 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 `@images/cli/Dockerfile.rhel` around lines 7 - 16, The image currently leaves no USER set (after the COPY and RUN lines and before the LABEL block), so update the Dockerfile to create a non-root user and switch to it: add commands to create a non-root user/group (e.g., UID 1001), chown the installed binaries copied by the COPY --from=builder lines (/usr/bin/oc and /usr/bin/oc-tests-ext.gz) and any other paths written during build, and then add a USER directive (e.g., USER 1001) before the LABEL/CMD so the container runs non-root; ensure no subsequent RUN steps require root or adjust ownership accordingly.Sources: Coding guidelines, Linters/SAST tools
images/tools/Dockerfile (1)
6-63:⚠️ Potential issue | 🟠 Major | ⚖️ Poor tradeoffAdd USER directive to run as non-root.
The Dockerfile does not specify a USER directive, causing the container to run as root by default. This violates the container security guidelines which require "USER non-root; never run as root." As per coding guidelines, all containers should run with a non-root user for security hardening, even for debugging and diagnostic tools.
🛡️ Proposed fix to add non-root user
Add a USER directive before CMD:
yum -y install $INSTALL_PKGS && rpm -V --nogroup --nosize --nofiledigest --nomtime --nomode $INSTALL_PKGS && yum clean all && rm -rf /var/cache/* # Disabled until they are buildable on s390x # numactl \ # numactl-devel \ +USER 1001 CMD ["/usr/bin/bash"]Note: Verify that UID 1001 is appropriate for your security context, or use a different non-root UID as required by your organization's standards.
🤖 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 `@images/tools/Dockerfile` around lines 6 - 63, The Dockerfile currently has no USER directive so containers run as root; create a non-root user (e.g., UID 1001) and group, chown any runtime-needed files/directories (for example /usr/bin/oc and any files under /etc/sos referenced in the Dockerfile) during the build, and add a USER <non-root> line before the CMD to run the container as that user; ensure the chosen UID/GID is consistent with your security policy and that no privileged operations in the RUN steps require root at container runtime.Sources: Coding guidelines, 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.
Outside diff comments:
In `@images/cli/Dockerfile.rhel`:
- Around line 7-16: The image currently leaves no USER set (after the COPY and
RUN lines and before the LABEL block), so update the Dockerfile to create a
non-root user and switch to it: add commands to create a non-root user/group
(e.g., UID 1001), chown the installed binaries copied by the COPY --from=builder
lines (/usr/bin/oc and /usr/bin/oc-tests-ext.gz) and any other paths written
during build, and then add a USER directive (e.g., USER 1001) before the
LABEL/CMD so the container runs non-root; ensure no subsequent RUN steps require
root or adjust ownership accordingly.
In `@images/tools/Dockerfile`:
- Around line 6-63: The Dockerfile currently has no USER directive so containers
run as root; create a non-root user (e.g., UID 1001) and group, chown any
runtime-needed files/directories (for example /usr/bin/oc and any files under
/etc/sos referenced in the Dockerfile) during the build, and add a USER
<non-root> line before the CMD to run the container as that user; ensure the
chosen UID/GID is consistent with your security policy and that no privileged
operations in the RUN steps require root at container runtime.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 29eb633b-64e7-4f56-9840-db72a4dce342
📒 Files selected for processing (4)
.ci-operator.yamlimages/cli-artifacts/Dockerfile.rhelimages/cli/Dockerfile.rhelimages/tools/Dockerfile
|
We need to wait for the rebase after all... /close |
|
@tchap: Closed this PR. 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. |
|
/reopen |
|
@tchap: Reopened this PR. 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. |
|
/retest |
1 similar comment
|
/retest |
|
@tchap: The following tests failed, say
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. |
1 participant
Cherrypicking the following PRs since the change needs to occur at once:
Summary by CodeRabbit