Conversation
Signed-off-by: Wesley Hayutin <weshayutin@gmail.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: weshayutin 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 |
📝 WalkthroughWalkthroughA new multi-stage Dockerfile (konflux.Dockerfile) is introduced for building and containerizing a Go application. The build stage compiles the "manager" binary using Red Hat's OpenShift Golang builder image with CGO enabled and FIPS-strict runtime flags, while the runtime stage packages the binary into a minimal Red Hat UBI container running as user 65532:65532. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can scan for known vulnerabilities in your dependencies using OSV Scanner.OSV Scanner will automatically detect and report security vulnerabilities in your project's dependencies. No additional configuration is required. |
|
oops cp to oadp-1.6 :) |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
konflux.Dockerfile (1)
2-9: Reorder copy steps to recover build cache efficiency.Line 2 copies the full context too early, then Lines 4-9 copy files again. This invalidates
go mod downloadcaching on most source changes and adds unnecessary layer churn.♻️ Suggested Dockerfile refactor
FROM brew.registry.redhat.io/rh-osbs/openshift-golang-builder:rhel_9_golang_1.25 AS builder -COPY . . WORKDIR $APP_ROOT/app/ -COPY go.mod go.mod -COPY go.sum go.sum +COPY go.mod go.sum ./ RUN go mod download COPY cmd/main.go cmd/main.go COPY api/ api/ COPY internal/ internal/🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@konflux.Dockerfile` around lines 2 - 9, The Dockerfile copies the entire context too early (COPY . .) which invalidates caching for the go mod download layer; reorder steps to first set WORKDIR ($APP_ROOT/app/), copy only go.mod and go.sum, run RUN go mod download, then copy the application source (COPY cmd/main.go, COPY api/, COPY internal/, or a single COPY ./ ./) so that dependency downloading is cached across most source changes and layer churn is reduced; ensure APP_ROOT is used consistently and keep the dependency download step immediately after copying only go.mod/go.sum and before copying project sources.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@konflux.Dockerfile`:
- Line 14: Replace the mutable tag in the Dockerfile FROM line (currently "FROM
registry.redhat.io/ubi9/ubi:latest") with an immutable image digest (sha256) to
ensure reproducible builds; obtain the correct digest for the desired UBI9 image
from the registry (or via docker pull + inspect) and update the FROM instruction
to use the pinned form "registry.redhat.io/ubi9/ubi@sha256:<digest>" so the
build always uses the exact same image.
---
Nitpick comments:
In `@konflux.Dockerfile`:
- Around line 2-9: The Dockerfile copies the entire context too early (COPY . .)
which invalidates caching for the go mod download layer; reorder steps to first
set WORKDIR ($APP_ROOT/app/), copy only go.mod and go.sum, run RUN go mod
download, then copy the application source (COPY cmd/main.go, COPY api/, COPY
internal/, or a single COPY ./ ./) so that dependency downloading is cached
across most source changes and layer churn is reduced; ensure APP_ROOT is used
consistently and keep the dependency download step immediately after copying
only go.mod/go.sum and before copying project sources.
| ENV GOEXPERIMENT strictfipsruntime | ||
| RUN CGO_ENABLED=1 GOOS=linux go build -tags "$BUILDTAGS" -mod=mod -a -o manager cmd/main.go | ||
|
|
||
| FROM registry.redhat.io/ubi9/ubi:latest |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is Red Hat’s recommended production practice for pinning registry.redhat.io/ubi9/ubi(mutable tags vs immutable digest), and which current UBI9 digest should be used instead of:latest?
💡 Result:
Recommended production practice (mutable tags vs. immutable digest)
- Don’t pin production builds/deployments to mutable tags like
:latest(or even:9.7, etc.) if you need repeatability—tags can be reassigned to newer image builds over time. Red Hat’s guidance is to use immutable digests when you need full control/reproducibility in production. [1] - Red Hat also recommends using fully-qualified image references (registry/namespace/name, plus an explicit tag or digest) rather than relying on defaults like
:latest. [3]
Current UBI9 digest to use instead of registry.redhat.io/ubi9/ubi:latest
Use:
registry.redhat.io/ubi9/ubi@sha256:20f695d2a91352d4eaa25107535126727b5945bff38ed36a3e59590f495046f0 [2]
Example:
FROM registry.redhat.io/ubi9/ubi@sha256:20f695d2a91352d4eaa25107535126727b5945bff38ed36a3e59590f495046f0Sources: Red Hat Developer guidance on tags vs digests and production use [1]; Red Hat documentation example listing this UBI9 digest [2].
[1] (developers.redhat.com)
[2] (docs.redhat.com)
[3] (docs.redhat.com)
Citations:
- 1: https://developers.redhat.com/articles/2025/01/28/how-name-version-and-reference-container-images
- 2: https://docs.redhat.com/ko-kr/documentation/openshift_container_platform/4.17/pdf/disconnected_environments/OpenShift_Container_Platform-4.17-Disconnected_environments-ko-KR.pdf?utm_source=openai
- 3: https://docs.redhat.com/en/documentation/red_hat_enterprise_linux/9/html/building_running_and_managing_containers/working-with-container-registries_building-running-and-managing-containers
Replace mutable :latest tag with immutable digest for reproducible builds.
Using a mutable tag like :latest makes builds non-reproducible and can introduce unreviewed runtime changes. Red Hat's production guidance recommends pinning to an immutable digest instead.
🔒 Suggested hardening
-FROM registry.redhat.io/ubi9/ubi:latest
+FROM registry.redhat.io/ubi9/ubi@sha256:20f695d2a91352d4eaa25107535126727b5945bff38ed36a3e59590f495046f0📝 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.
| FROM registry.redhat.io/ubi9/ubi:latest | |
| FROM registry.redhat.io/ubi9/ubi@sha256:20f695d2a91352d4eaa25107535126727b5945bff38ed36a3e59590f495046f0 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@konflux.Dockerfile` at line 14, Replace the mutable tag in the Dockerfile
FROM line (currently "FROM registry.redhat.io/ubi9/ubi:latest") with an
immutable image digest (sha256) to ensure reproducible builds; obtain the
correct digest for the desired UBI9 image from the registry (or via docker pull
+ inspect) and update the FROM instruction to use the pinned form
"registry.redhat.io/ubi9/ubi@sha256:<digest>" so the build always uses the exact
same image.
|
@weshayutin: The following test 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. |
Summary by CodeRabbit