WIP: Smoke test HAProxy 3.2.19 RPM#792
Conversation
|
Skipping CI for Draft Pull Request. |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
gcs278
changed the title
|
FYI this is a "vanilla" 3.2 smoke test PR - just to get some green CI runs before integrating HAProxy 3.2 with the sidecar architecture. /assign @jcmoraisjr |
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
gcs278
changed the title
WalkthroughThe Dockerfile for the OpenShift router updates the HAProxy package installed during image build from ChangesHAProxy package update
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Suggested labels
🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/router/haproxy/Dockerfile.ocp`:
- Line 3: The RUN instruction that installs
haproxy32-3.2.19-1.rhaos4.23.el9.x86_64.rpm must perform package cache cleanup
in the same layer; update the RUN that currently calls "yum install -y
https://.../haproxy32-3.2.19-1.rhaos4.23.el9.x86_64.rpm" to chain the install
with cache cleanup (e.g., run yum clean all and remove yum caches) in the same
command so no package manager metadata remains in the final image.
- Line 3: The RUN line in Dockerfile.ocp that installs the RPM from a mutable
GitHub branch (the existing RUN yum install ... refs/heads/master URL) must be
replaced with an immutable, verified artifact: point to a pinned artifact URL
(digest or internal Brew/registry artifact) and verify its integrity before
install (e.g., fetch the RPM by digest/checksum or GPG signature, validate
checksum/signature, then run yum install from the verified local file). Ensure
the RPM source is not a branch URL and include checksum or signature validation
steps so the container build only consumes a pinned, trusted package.
🪄 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: d5e60100-d267-4eaa-9a00-e268af5564b5
📒 Files selected for processing (1)
images/router/haproxy/Dockerfile.ocp
| FROM registry.ci.openshift.org/ocp/4.22:haproxy-router-base | ||
| RUN INSTALL_PKGS="socat haproxy28 rsyslog procps-ng util-linux" && \ | ||
|
|
||
| RUN yum install -y https://github.com/gcs278/openshift-hacks/raw/refs/heads/master/haproxy-builds/haproxy32-3.2.19-1.rhaos4.23.el9.x86_64.rpm |
There was a problem hiding this comment.
Clean yum metadata in the same layer as the RPM install
The first yum install (Line 3) does not run yum clean all in the same RUN layer, so cache/metadata remain in that layer and bloat the final image. Move cleanup into the same command chain.
As per coding guidelines: "No package manager cache in final layer".
🧰 Tools
🪛 Trivy (0.69.3)
[error] 3-3: 'yum clean all' missing
'yum clean all' is missed: yum install -y https://github.com/gcs278/openshift-hacks/raw/refs/heads/master/haproxy-builds/haproxy32-3.2.19-1.rhaos4.23.el9.x86_64.rpm
Rule: DS-0015
(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 `@images/router/haproxy/Dockerfile.ocp` at line 3, The RUN instruction that
installs haproxy32-3.2.19-1.rhaos4.23.el9.x86_64.rpm must perform package cache
cleanup in the same layer; update the RUN that currently calls "yum install -y
https://.../haproxy32-3.2.19-1.rhaos4.23.el9.x86_64.rpm" to chain the install
with cache cleanup (e.g., run yum clean all and remove yum caches) in the same
command so no package manager metadata remains in the final image.
Sources: Coding guidelines, Linters/SAST tools
Use an immutable, verified RPM source instead of a mutable GitHub branch URL
Line 3 installs an RPM from refs/heads/master, which is mutable and not integrity-checked. If that artifact changes upstream, the build can silently consume untrusted content (including RPM scriptlets executed during install). Use an immutable source (e.g., internal Brew/registry artifact) and verify checksum/signature before install.
As per coding guidelines, container artifacts should follow immutable pinning principles: "non-RH images: pin by digest".
Suggested hardening change
-RUN yum install -y https://github.com/gcs278/openshift-hacks/raw/refs/heads/master/haproxy-builds/haproxy32-3.2.19-1.rhaos4.23.el9.x86_64.rpm
+RUN curl -fsSL -o /tmp/haproxy32.rpm https://<immutable-artifact-url>/haproxy32-3.2.19-1.rhaos4.23.el9.x86_64.rpm && \
+ echo "<sha256> /tmp/haproxy32.rpm" | sha256sum -c - && \
+ yum install -y /tmp/haproxy32.rpm && \
+ rm -f /tmp/haproxy32.rpm && \
+ yum clean all🧰 Tools
🪛 Trivy (0.69.3)
[error] 3-3: 'yum clean all' missing
'yum clean all' is missed: yum install -y https://github.com/gcs278/openshift-hacks/raw/refs/heads/master/haproxy-builds/haproxy32-3.2.19-1.rhaos4.23.el9.x86_64.rpm
Rule: DS-0015
(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 `@images/router/haproxy/Dockerfile.ocp` at line 3, The RUN line in
Dockerfile.ocp that installs the RPM from a mutable GitHub branch (the existing
RUN yum install ... refs/heads/master URL) must be replaced with an immutable,
verified artifact: point to a pinned artifact URL (digest or internal
Brew/registry artifact) and verify its integrity before install (e.g., fetch the
RPM by digest/checksum or GPG signature, validate checksum/signature, then run
yum install from the verified local file). Ensure the RPM source is not a branch
URL and include checksum or signature validation steps so the container build
only consumes a pinned, trusted package.
Source: Coding guidelines
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
There was a problem hiding this comment.
♻️ Duplicate comments (2)
images/router/haproxy/Dockerfile.ocp (2)
3-5:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftUse an immutable, verified RPM source instead of a mutable GitHub branch URL.
Line 3 installs an RPM from
refs/heads/master, which is mutable and not integrity-checked. If that artifact changes upstream, the build can silently consume untrusted content, including RPM scriptlets executed during install.🔒 Recommended hardening change
-RUN curl -fLo /tmp/haproxy32.rpm https://github.com/gcs278/openshift-hacks/raw/refs/heads/master/haproxy-builds/haproxy32-3.2.19-1.rhaos4.23.el9.x86_64.rpm && \ - yum install -y /tmp/haproxy32.rpm && \ - rm -f /tmp/haproxy32.rpm +RUN curl -fsSL -o /tmp/haproxy32.rpm https://<immutable-artifact-url>/haproxy32-3.2.19-1.rhaos4.23.el9.x86_64.rpm && \ + echo "<sha256> /tmp/haproxy32.rpm" | sha256sum -c - && \ + yum install -y /tmp/haproxy32.rpm && \ + rm -f /tmp/haproxy32.rpm && \ + yum clean allAs per coding guidelines, container artifacts should follow immutable pinning principles: "non-RH images: pin by digest".
🤖 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/router/haproxy/Dockerfile.ocp` around lines 3 - 5, The RUN step that curls an RPM from a mutable GitHub branch (the line starting with "RUN curl -fLo /tmp/haproxy32.rpm ... refs/heads/master") must be replaced with an immutable, verified artifact fetch: point the download to a pinned release URL (tag or commit SHA) or a trusted registry with a digest, and verify integrity before install (e.g., check a SHA256 checksum or GPG signature of haproxy32.rpm) instead of relying on refs/heads/master; update the RUN command to download the pinned URL, validate the file, then install and remove it.Source: Coding guidelines
3-5:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClean yum metadata in the same layer as the RPM install.
The
yum installdoes not runyum clean allin the same RUN layer, so cache and metadata remain in that layer and bloat the final image.🧹 Proposed fix
RUN curl -fLo /tmp/haproxy32.rpm https://github.com/gcs278/openshift-hacks/raw/refs/heads/master/haproxy-builds/haproxy32-3.2.19-1.rhaos4.23.el9.x86_64.rpm && \ yum install -y /tmp/haproxy32.rpm && \ - rm -f /tmp/haproxy32.rpm + rm -f /tmp/haproxy32.rpm && \ + yum clean allAs per coding guidelines: "No package manager cache in final layer".
🤖 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/router/haproxy/Dockerfile.ocp` around lines 3 - 5, The RUN layer that downloads and installs /tmp/haproxy32.rpm leaves yum metadata behind; modify the RUN that currently runs curl && yum install -y /tmp/haproxy32.rpm && rm -f /tmp/haproxy32.rpm to also run the yum cleanup (e.g., yum clean all && rm -rf /var/cache/yum or equivalent) in the same RUN chain so package manager cache/metadata are removed before the layer is committed; target the RUN instruction that handles /tmp/haproxy32.rpm in Dockerfile.ocp to implement this change.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.
Duplicate comments:
In `@images/router/haproxy/Dockerfile.ocp`:
- Around line 3-5: The RUN step that curls an RPM from a mutable GitHub branch
(the line starting with "RUN curl -fLo /tmp/haproxy32.rpm ...
refs/heads/master") must be replaced with an immutable, verified artifact fetch:
point the download to a pinned release URL (tag or commit SHA) or a trusted
registry with a digest, and verify integrity before install (e.g., check a
SHA256 checksum or GPG signature of haproxy32.rpm) instead of relying on
refs/heads/master; update the RUN command to download the pinned URL, validate
the file, then install and remove it.
- Around line 3-5: The RUN layer that downloads and installs /tmp/haproxy32.rpm
leaves yum metadata behind; modify the RUN that currently runs curl && yum
install -y /tmp/haproxy32.rpm && rm -f /tmp/haproxy32.rpm to also run the yum
cleanup (e.g., yum clean all && rm -rf /var/cache/yum or equivalent) in the same
RUN chain so package manager cache/metadata are removed before the layer is
committed; target the RUN instruction that handles /tmp/haproxy32.rpm in
Dockerfile.ocp to implement this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c88616bc-de9b-422c-a972-793668d3b076
📒 Files selected for processing (1)
images/router/haproxy/Dockerfile.ocp
|
/test ? |
|
/test e2e-metal-ipi-ovn-dualstack |
|
/test ? |
|
/test e2e-metal-ipi-ovn-dualstack |
|
/retest |
image approach (no sidecar yet) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
gcs278
force-pushed
the
smoke-test-haproxy32-3.2.19
branch
from
b83f1e1 to
2268fb3
Compare
|
We've typically done some aggregate E2E to expose any flakes in the past with new HAProxy versions: /payload-aggregate periodic-ci-openshift-release-master-ci-5.0-e2e-aws-ovn 5 |
|
@gcs278: trigger 0 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command |
|
@gcs278: 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. |
2 participants
Summary
Smoke test for HAProxy 3.2.19 in the monolithic (non-sidecar) router model.
The
haproxy32-3.2.19RPM is already available in the yum repo but is currently unused. This PR switches the monolithic router image to installhaproxy32instead ofhaproxy28, allowing us to smoke test HAProxy 3.2 in CI before it is formally adopted.