Conversation
Signed-off-by: Wesley Hayutin <weshayutin@gmail.com>
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
📝 WalkthroughWalkthroughA new multi-stage Dockerfile (konflux.Dockerfile) is introduced for building a Go-based OpenShift application. The builder stage compiles the binary using a Red Hat OpenShift Golang image, while the runtime stage packages it into a minimal UBI image with non-root user configuration and metadata labels. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![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
🧹 Nitpick comments (1)
konflux.Dockerfile (1)
2-9: Reorder COPY instructions to preserve module cache and reduce context sprawl.Line 2 copies the full context before dependency resolution, then Lines 4-9 copy files again. This weakens layer caching and increases unnecessary context exposure.
Proposed 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 cmd/ cmd/ 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 currently copies the entire build context early (COPY . .) which defeats layer caching for Go modules; reorder the steps so that you first set WORKDIR ($APP_ROOT/app/), copy only go.mod and go.sum (COPY go.mod go.sum), run the dependency download (RUN go mod download), and only after that COPY the remaining sources (COPY cmd/main.go, COPY api/, COPY internal/ or a final COPY . .) so module resolution is cached and the large context isn’t leaked into earlier layers.
🤖 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 15: The Dockerfile uses the build-time variable APP_ROOT in multiple
stages (e.g., COPY --from=builder $APP_ROOT/app/manager /manager) but never
declares it as an ARG; declare ARG APP_ROOT in both the builder stage and the
final stage (before referencing it and before any FROM that needs it) so the
variable is available during multi-stage builds, ensuring the COPY lines like
COPY --from=builder $APP_ROOT/app/manager /manager resolve correctly.
- Line 14: Replace the mutable base image reference "FROM
registry.redhat.io/ubi9/ubi:latest" with a digest-pinned immutable reference;
obtain the Manifest List Digest for UBI 9 from the Red Hat Ecosystem Catalog and
update the FROM line to use the digest form (e.g. "FROM
registry.redhat.io/ubi9/ubi:9.7@sha256:<manifest-list-digest>" or at minimum
"FROM registry.redhat.io/ubi9/ubi@sha256:<manifest-list-digest>") so builds are
reproducible.
---
Nitpick comments:
In `@konflux.Dockerfile`:
- Around line 2-9: The Dockerfile currently copies the entire build context
early (COPY . .) which defeats layer caching for Go modules; reorder the steps
so that you first set WORKDIR ($APP_ROOT/app/), copy only go.mod and go.sum
(COPY go.mod go.sum), run the dependency download (RUN go mod download), and
only after that COPY the remaining sources (COPY cmd/main.go, COPY api/, COPY
internal/ or a final COPY . .) so module resolution is cached and the large
context isn’t leaked into earlier layers.
| 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 immutable reference format for registry.redhat.io/ubi9/ubi in Dockerfiles (tag vs digest), and what is the current supported digest to pin for reproducible builds?
💡 Result:
Recommended immutable reference format (tag vs digest)
Red Hat recommends pinning images by digest (not just a tag) for reproducible builds, ideally using the “Manifest List Digest (Recommended)” so the correct architecture is selected automatically. In a Dockerfile that looks like:
FROM registry.redhat.io/ubi9/ubi@sha256:<manifest-list-digest>
# (optionally) FROM registry.redhat.io/ubi9/ubi:9.7@sha256:<manifest-list-digest> # tag for readabilityTags (including latest) are mutable, while digest references are immutable and make builds reproducible. [1][2]
Current digest to pin (for reproducible builds)
If you are building linux/amd64, the current digest corresponding to the current 9.7 / latest UBI 9 image is:
sha256:f09be5a4b3eed653eb88eb4d9f936823522d89947baab4aca6fadd543d857abb
(That is the digest shown for redhat/ubi9:9.7 / redhat/ubi9:latest on Docker Hub, which tracks the same UBI 9 content stream.) [3]
To pin the multi-arch manifest list digest specifically (Red Hat’s preferred “Manifest List Digest (Recommended)”), copy it from the UBI 9 entry in the Red Hat Ecosystem Catalog under Image identifiers → Manifest List Digest (Recommended). [2]
Sources:
[1] Red Hat Developer – How to name, version, and reference container images (digest vs tag mutability/reproducibility)
[2] Red Hat Ecosystem Catalog – UBI 9 image page (recommends “Manifest List Digest (Recommended)”)
[3] Docker Hub – redhat/ubi9:9.7 layer details page (digest value in the image reference)
Pin runtime base image to an immutable reference.
Line 14 uses ubi:latest, which is mutable and makes builds non-reproducible, potentially introducing unreviewed runtime changes. Use a digest-based pinning strategy instead.
Obtain the Manifest List Digest (Recommended) from the Red Hat Ecosystem Catalog entry for UBI 9, then update to:
-FROM registry.redhat.io/ubi9/ubi:latest
+FROM registry.redhat.io/ubi9/ubi@sha256:<manifest-list-digest>Optionally include the tag for readability: FROM registry.redhat.io/ubi9/ubi:9.7@sha256:<manifest-list-digest>
🤖 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 base image reference
"FROM registry.redhat.io/ubi9/ubi:latest" with a digest-pinned immutable
reference; obtain the Manifest List Digest for UBI 9 from the Red Hat Ecosystem
Catalog and update the FROM line to use the digest form (e.g. "FROM
registry.redhat.io/ubi9/ubi:9.7@sha256:<manifest-list-digest>" or at minimum
"FROM registry.redhat.io/ubi9/ubi@sha256:<manifest-list-digest>") so builds are
reproducible.
| 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 | ||
| COPY --from=builder $APP_ROOT/app/manager /manager |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify APP_ROOT usage/definition around stage boundaries.
rg -n -C2 '^(FROM|ARG APP_ROOT|ENV APP_ROOT|WORKDIR|COPY --from=builder .*APP_ROOT)' konflux.DockerfileRepository: migtools/oadp-non-admin
Length of output: 461
🏁 Script executed:
rg -i 'APP_ROOT' --type docker --type sh --type makefile --type yamlRepository: migtools/oadp-non-admin
Length of output: 99
🏁 Script executed:
fd -type f -name "Dockerfile*" -o -name "*.dockerfile" | head -20Repository: migtools/oadp-non-admin
Length of output: 237
🏁 Script executed:
rg 'APP_ROOT' .Repository: migtools/oadp-non-admin
Length of output: 182
🏁 Script executed:
fd -name '*[Dd]ockerfile*'Repository: migtools/oadp-non-admin
Length of output: 297
🏁 Script executed:
fd 'Dockerfile'Repository: migtools/oadp-non-admin
Length of output: 96
🏁 Script executed:
find . -type f \( -name "*.yml" -o -name "*.yaml" \) | head -20Repository: migtools/oadp-non-admin
Length of output: 926
🏁 Script executed:
cat -n konflux.DockerfileRepository: migtools/oadp-non-admin
Length of output: 1067
🏁 Script executed:
cat -n Dockerfile | head -40Repository: migtools/oadp-non-admin
Length of output: 1604
🏁 Script executed:
rg 'build' .github/workflows/ci.yml -A 3 -B 3Repository: migtools/oadp-non-admin
Length of output: 251
Declare APP_ROOT as an ARG in both stages to avoid undefined variable expansion.
Lines 3 and 15 reference $APP_ROOT without declaring it anywhere in the Dockerfile. The builder image may define this internally, but it won't be available in child stages unless explicitly declared with ARG. This can lead to broken paths during build.
Proposed fix
+ARG APP_ROOT=/opt/app-root
FROM brew.registry.redhat.io/rh-osbs/openshift-golang-builder:rhel_9_golang_1.25 AS builder
-COPY . .
-WORKDIR $APP_ROOT/app/
+WORKDIR ${APP_ROOT}/app/
COPY go.mod go.mod
COPY go.sum go.sum
RUN go mod download
COPY cmd/main.go cmd/main.go
COPY api/ api/
COPY internal/ internal/
ENV BUILDTAGS strictfipsruntime
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
+ARG APP_ROOT=/opt/app-root
-COPY --from=builder $APP_ROOT/app/manager /manager
+COPY --from=builder ${APP_ROOT}/app/manager /manager🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@konflux.Dockerfile` at line 15, The Dockerfile uses the build-time variable
APP_ROOT in multiple stages (e.g., COPY --from=builder $APP_ROOT/app/manager
/manager) but never declares it as an ARG; declare ARG APP_ROOT in both the
builder stage and the final stage (before referencing it and before any FROM
that needs it) so the variable is available during multi-stage builds, ensuring
the COPY lines like COPY --from=builder $APP_ROOT/app/manager /manager resolve
correctly.
|
/cherry-pick oadp-1.6 |
|
@weshayutin: once the present PR merges, I will cherry-pick it on top of 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. |
|
/cherry-pick oadp-1.6 |
|
@mpryc: once the present PR merges, I will cherry-pick it on top of 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. |
|
/lgtm |
|
/cherry-pick oadp-1.6 |
|
/lgtm |
|
@mpryc: once the present PR merges, I will cherry-pick it on top of 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. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mpryc, shubham-pampattiwar, 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 |
5 participants
Why the changes were made
template konflux dockerfile, can be cp to oadp-1.6
How to test the changes made
Summary by CodeRabbit