OCPBUGS-87541: Updating openshift-enterprise-tests-container image to be consistent with ART for 5.0#31265
Conversation
openshift-bot
force-pushed
the
art-consistency-openshift-5.0-openshift-enterprise-tests
branch
from
fd6ab91 to
2f47a73
Compare
|
Created by ART pipeline job run https://art-jenkins.apps.prod-stable-spoke1-dc-iad2.itup.redhat.com/job/aos-cd-builds/job/build%252Fsync-ci-images/216 |
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
openshift-ci
Bot
added
the
ready-for-human-review
openshift-bot
changed the title
openshift-ci-robot
added
jira/valid-reference
|
@openshift-bot: This pull request references Jira Issue OCPBUGS-87541, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
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. |
WalkthroughThis PR updates the build toolchain configuration to use Go 1.26 and OpenShift 5.0 base images. The CI operator configuration and test Dockerfile are synchronized to reference the new toolchain versions instead of the previous Go 1.25 and OpenShift 4.22 combination. ChangesBuild Toolchain Update
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 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)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: openshift-bot 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 |
|
@openshift-bot: This pull request references Jira Issue OCPBUGS-87541, which is valid. 3 validation(s) were run on this bug
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. |
![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 (2)
images/tests/Dockerfile.rhel (2)
1-24:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winContainer must run as non-root user.
The Dockerfile violates the container security guideline requiring
USER non-rootand never running as root. NoUSERdirective is specified, so the container runs as root by default, creating a significant security risk.Add a
USERdirective before the final stage's runtime commands to specify a non-root user. As per coding guidelines,Container security (prodsec-skills): USER non-root; never run as root.🔒 Proposed fix to add non-root USER
Add after line 9 (before the RUN command):
FROM registry.ci.openshift.org/ocp/5.0:tools COPY --from=builder /tmp/build/openshift-tests /usr/bin/ +USER 1001 RUN PACKAGES="git gzip util-linux" && \Note: You may need to adjust file permissions or choose a different UID based on the base image's user configuration. Verify the base image documentation for the appropriate non-root user.
🤖 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/tests/Dockerfile.rhel` around lines 1 - 24, The final runtime stage currently runs as root because no USER is set; update the Dockerfile to switch to a non-root user in the final stage (after the COPY --from=builder ... /usr/bin/ and before the RUN that installs PACKAGES) by adding a USER directive referencing a non-root account present in the base image (or create one and fix ownership/permissions of /usr/bin/openshift-tests and any files touched by the RUN installing PACKAGES), and ensure group/owner and any required dirs are writable by that user so the subsequent RUN steps succeed.Sources: Coding guidelines, Linters/SAST tools
8-24:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd HEALTHCHECK for container monitoring.
The Dockerfile is missing a
HEALTHCHECKdirective, which violates the container security guideline. Health checks enable orchestrators to monitor container health and restart failed instances.Add an appropriate
HEALTHCHECKdirective based on how theopenshift-testsbinary exposes health status. As per coding guidelines,Container security (prodsec-skills): HEALTHCHECK defined.💚 Example HEALTHCHECK
LABEL io.k8s.display-name="OpenShift End-to-End Tests" \ io.openshift.release.operator=true \ io.k8s.description="OpenShift is a platform for developing, building, and deploying containerized applications." \ io.openshift.build.versions="kubernetes-tests=1.35.1" \ io.openshift.tags="openshift,tests,e2e" + +HEALTHCHECK --interval=30s --timeout=3s \ + CMD /usr/bin/openshift-tests version || exit 1Adjust the health check command based on the actual capabilities of the
openshift-testsbinary.🤖 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/tests/Dockerfile.rhel` around lines 8 - 24, Add a Docker HEALTHCHECK directive near the end of the Dockerfile to satisfy the container security guideline: choose an appropriate probe command that checks the running openshift-tests binary (e.g., a binary-specific health endpoint or a lightweight process/port check for "openshift-tests"), and add HEALTHCHECK with sensible options such as --interval=30s --timeout=5s --start-period=30s --retries=3 so the container runtime can detect and restart unhealthy containers; update the Dockerfile block that installs/copies the openshift-tests binary (reference: the openshift-tests binary copied from builder and the Dockerfile content surrounding PACKAGES and LABEL entries) and place the HEALTHCHECK directive before or after the LABEL entries per Dockerfile conventions.Source: Coding guidelines
🤖 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 `@images/tests/Dockerfile.rhel`:
- Line 3: The Dockerfile.rhel currently uses a broad COPY . . which pulls the
entire build context into the image; replace that with explicit COPY
instructions for only the files needed to build (e.g., COPY go.mod go.sum ./,
COPY Makefile ./, and COPY the source directories used by your build such as
cmd/, pkg/, internal/ or src/), and ensure any build artifacts or sensitive
files are excluded; update the Dockerfile.rhel to sequence copying dependency
files first (go.mod/go.sum), running go mod download (or the equivalent make
target), then copying source directories before running the build so layer
caching remains effective.
---
Outside diff comments:
In `@images/tests/Dockerfile.rhel`:
- Around line 1-24: The final runtime stage currently runs as root because no
USER is set; update the Dockerfile to switch to a non-root user in the final
stage (after the COPY --from=builder ... /usr/bin/ and before the RUN that
installs PACKAGES) by adding a USER directive referencing a non-root account
present in the base image (or create one and fix ownership/permissions of
/usr/bin/openshift-tests and any files touched by the RUN installing PACKAGES),
and ensure group/owner and any required dirs are writable by that user so the
subsequent RUN steps succeed.
- Around line 8-24: Add a Docker HEALTHCHECK directive near the end of the
Dockerfile to satisfy the container security guideline: choose an appropriate
probe command that checks the running openshift-tests binary (e.g., a
binary-specific health endpoint or a lightweight process/port check for
"openshift-tests"), and add HEALTHCHECK with sensible options such as
--interval=30s --timeout=5s --start-period=30s --retries=3 so the container
runtime can detect and restart unhealthy containers; update the Dockerfile block
that installs/copies the openshift-tests binary (reference: the openshift-tests
binary copied from builder and the Dockerfile content surrounding PACKAGES and
LABEL entries) and place the HEALTHCHECK directive before or after the LABEL
entries per Dockerfile conventions.
🪄 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: 41c32db6-9ded-4071-8061-a4da7fa1236a
📒 Files selected for processing (2)
.ci-operator.yamlimages/tests/Dockerfile.rhel
| 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/origin | ||
| COPY . . |
There was a problem hiding this comment.
Avoid copying entire build context.
Line 3 copies the entire repository context (.) into the image, violating the container security guideline to COPY specific files, not entire context. This increases the image size unnecessarily and potentially includes sensitive files or build artifacts.
Copy only the specific files and directories required for the build (e.g., go.mod, go.sum, source directories). As per coding guidelines, Container security (prodsec-skills): COPY specific files, not entire context.
♻️ Suggested approach
Replace the broad copy with specific paths:
WORKDIR /go/src/github.com/openshift/origin
-COPY . .
+COPY go.mod go.sum ./
+COPY cmd/ cmd/
+COPY pkg/ pkg/
+COPY vendor/ vendor/
+# Add other specific directories needed for the buildAdjust the specific directories based on what the make command actually requires.
🤖 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/tests/Dockerfile.rhel` at line 3, The Dockerfile.rhel currently uses a
broad COPY . . which pulls the entire build context into the image; replace that
with explicit COPY instructions for only the files needed to build (e.g., COPY
go.mod go.sum ./, COPY Makefile ./, and COPY the source directories used by your
build such as cmd/, pkg/, internal/ or src/), and ensure any build artifacts or
sensitive files are excluded; update the Dockerfile.rhel to sequence copying
dependency files first (go.mod/go.sum), running go mod download (or the
equivalent make target), then copying source directories before running the
build so layer caching remains effective.
Source: Coding guidelines
|
Scheduling required tests: |
|
ART wants to connect issue OCPBUGS-87790 to this PR, but found it is currently hooked up to ['OCPBUGS-87541']. Please consult with #forum-ocp-art if it is not clear what there is to do. |
|
/retest-required |
|
@openshift-bot: 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. |
3 participants
Updating openshift-enterprise-tests-container image to be consistent with ART for 5.0
TLDR:
Product builds by ART can be configured for different base and builder images than corresponding CI
builds. This automated PR requests a change to CI configuration to align with ART's configuration;
please take steps to merge it quickly or contact ART to coordinate changes.
The configuration in the following ART component metadata is driving this alignment request:
openshift-enterprise-tests.yml.
Detail:
This repository is out of sync with the downstream product builds for this component. The CI
configuration for at least one image differs from ART's expected product configuration. This should
be addressed to ensure that the component's CI testing accurate reflects what customers will
experience.
Most of these PRs are opened as an ART-driven proposal to migrate base image or builder(s) to a
different version, usually prior to GA. The intent is to effect changes in both configurations
simultaneously without breaking either CI or ART builds, so usually ART builds are configured to
consider CI as canonical and attempt to match CI config until the PR merges to align both. ART may
also configure changes in GA releases with CI remaining canonical for a brief grace period to enable
CI to succeed and the alignment PR to merge. In either case, ART configuration will be made
canonical at some point (typically at branch-cut before GA or release dev-cut after GA), so it is
important to align CI configuration as soon as possible.
PRs are also triggered when CI configuration changes without ART coordination, for instance to
change the number of builder images or to use a different golang version. These changes should be
coordinated with ART; whether ART configuration is canonical or not, preferably it would be updated
first to enable the changes to occur simultaneously in both CI and ART at the same time. This also
gives ART a chance to validate the intended changes first. For instance, ART compiles most
components with the Golang version being used by the control plane for a given OpenShift release.
Exceptions to this convention (i.e. you believe your component must be compiled with a Golang
version independent from the control plane) must be granted by the OpenShift staff engineers and
communicated to the ART team.
Roles & Responsibilities:
tests OR that necessary metadata changes are reported to the ART team
in
#forum-ocp-arton Slack. If necessary, the changes required by this pull request can beintroduced with a separate PR opened by the component team. Once the repository is aligned,
this PR will be closed automatically.
verify-depsis complaining. In that case, please opena new PR with the dependency issues addressed (and base images bumped). ART-9595 for reference.
any required labels to ensure the PR merges once tests are passing. In cases where ART config is
canonical, downstream builds are already being built with these changes, and merging this PR
only improves the fidelity of our CI. In cases where ART config is not canonical, this provides
a grace period for the component team to align their CI with ART's configuration before it becomes
canonical in product builds.
ART has been configured to reconcile your CI build root image (see https://docs.ci.openshift.org/docs/architecture/ci-operator/#build-root-image).
In order for your upstream .ci-operator.yaml configuration to be honored, you must set the following in your openshift/release ci-operator configuration file:
Change behavior of future PRs:
set up automatically. This means that such a PR would merge without human intervention (and awareness!) in the future.
To do so, open a PR to set the
auto_labelattribute in the image configuration. ExampleUPSTREAM: <carry>:. An example.If you have any questions about this pull request, please reach out in the
#forum-ocp-artSlack channel.Depends on PullRequest(title="OCPBUGS-87378: Updating ose-tools-container image to be consistent with ART for 5.0", number=2283) . Allow it to merge and then run
/test allon this PR.Summary by CodeRabbit