WIP: Smoke test HAProxy 3.2.19 RPM#792
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Skipping CI for Draft Pull Request. |
|
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 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe Dockerfile for the OpenShift router removes ChangesHAProxy version upgrade to 3.2.19
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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
The 4.22 base image uses microdnf via ART wrapper which doesn't support installing RPMs directly from URLs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
[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
microdnf (used by the ART yum wrapper in 4.22 base images) does not support installing local RPM files. Use rpm directly to bypass it. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/test ? |
|
/test e2e-metal-ipi-ovn-dualstack |
|
/test ? |
|
/test e2e-metal-ipi-ovn-dualstack |
|
/retest |
|
@gcs278: 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. |
Summary
Smoke test for the HAProxy 3.2.19 RPM build
haproxy32-3.2.19-1.rhaos4.23.el9.x86_64.rpmSummary by CodeRabbit