DVO-363: Fix Konflux pipelines for OCP versions 4.12, 4.13, 4.14#639
Conversation
|
@ncaak: This pull request references DVO-363 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.22.0" version, but no target version was set. 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. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ncaak 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 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughPipeline YAMLs updated to use Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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)
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 |
|
@ncaak: This pull request references DVO-363 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.22.0" version, but no target version was set. 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.
🧹 Nitpick comments (1)
.tekton/fbc-ocp4-13-push.yaml (1)
91-94:build-argsdefault:pipelineSpec.paramsdefault forbuild-platformsis not in sync.The
build-argsdefault value itself (line 91) is correct and uses valid YAML flow-sequence syntax. However, thepipelineSpec.params.build-platformsdefault at lines 99–103 still only listslinux/x86_64, while the runtime override at lines 29–31 now addslinux/arm64. Sincespec.paramsalways takes precedence here, there is no functional impact, but the in-spec default is now stale and could mislead anyone reading the embeddedpipelineSpecin isolation. The same applies to all six files in this PR.🔧 Suggested fix — align `pipelineSpec.params.build-platforms` default with runtime value (applies to all six files)
- default: - linux/x86_64 + - linux/arm64 description: List of platforms to build the container images on. The available set of values is determined by the configuration of the multi-platform-controller. name: build-platforms type: array🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.tekton/fbc-ocp4-13-push.yaml around lines 91 - 94, The pipelineSpec embedded params default for build-platforms is stale compared to the runtime override; update the pipelineSpec.params entry named build-platforms so its default list matches the runtime/default used elsewhere (add "linux/arm64" alongside "linux/x86_64") to keep the in-spec documentation consistent; ensure you update this in every occurrence of pipelineSpec.params.build-platforms across the six files so the default array aligns with the build-args/runtime defaults.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.tekton/fbc-ocp4-13-push.yaml:
- Around line 91-94: The pipelineSpec embedded params default for
build-platforms is stale compared to the runtime override; update the
pipelineSpec.params entry named build-platforms so its default list matches the
runtime/default used elsewhere (add "linux/arm64" alongside "linux/x86_64") to
keep the in-spec documentation consistent; ensure you update this in every
occurrence of pipelineSpec.params.build-platforms across the six files so the
default array aligns with the build-args/runtime defaults.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.tekton/fbc-ocp4-12-pull-request.yaml.tekton/fbc-ocp4-12-push.yaml.tekton/fbc-ocp4-13-pull-request.yaml.tekton/fbc-ocp4-13-push.yaml.tekton/fbc-ocp4-14-pull-request.yaml.tekton/fbc-ocp4-14-push.yaml
|
@ncaak: This pull request references DVO-363 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.22.0" version, but no target version was set. 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. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
konflux-ci/fbc/catalog.Dockerfile (2)
14-15:ARG RHEL_V(and the pre-existingARG OCP_V) are unused in the builder stage.Both re-declarations appear after the only builder-stage instructions that consume variables (
ADD $CAT_TYPE/…and theRUNcache-populate). Neither$OCP_Vnor$RHEL_Vis referenced in any builder-stage instruction after lines 14–15. The subsequentFROMat line 18 draws from the global-scope ARGs (lines 1–2), not from these stage-scoped re-declarations.Consider removing them to avoid confusion, unless they are intentionally reserved for future builder-stage use.
♻️ Proposed cleanup
-ARG OCP_V -ARG RHEL_V # The base image is expected to contain🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@konflux-ci/fbc/catalog.Dockerfile` around lines 14 - 15, Remove the unused stage-scoped ARG declarations ARG OCP_V and ARG RHEL_V from the builder stage: they are not referenced by any builder-stage instructions (e.g., the ADD and RUN cache-populate commands) and are shadowed by the global ARGs declared earlier; delete these two ARG lines from the builder stage (or move/restore them only if you plan to actually use $OCP_V/$RHEL_V inside that stage) so the Dockerfile is not confusing about which ARGs are in-scope.
11-11: ReplaceADDwithCOPYfor local directory copy.
ADDhas implicit behaviours (URL fetching, tar auto-extraction) that are not needed here.COPYis the explicit, safe choice for local paths.♻️ Proposed fix
-ADD $CAT_TYPE/catalog /configs +COPY $CAT_TYPE/catalog /configs🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@konflux-ci/fbc/catalog.Dockerfile` at line 11, Replace the Dockerfile instruction ADD with COPY for the local directory copy: change the ADD $CAT_TYPE/catalog /configs line so it uses COPY instead of ADD, ensuring you still reference the same source ($CAT_TYPE/catalog) and destination (/configs); this avoids ADD's implicit behaviors like URL fetching and tar extraction and makes the intent explicit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.tekton/fbc-ocp4-13-pull-request.yaml:
- Around line 91-97: The Dockerfile uses the build-arg RHEL_V to construct the
base image registry.redhat.io/openshift4/ose-operator-registry-$RHEL_V:$OCP_V
which yields a non-existent image; update the Dockerfile to remove the -$RHEL_V
suffix so the base becomes
registry.redhat.io/openshift4/ose-operator-registry:$OCP_V (or alternatively
stop passing RHEL_V into that FROM/pull spec), and ensure any references that
construct that pull spec (search for the string pattern "ose-operator-registry"
or the ARG names RHEL_V and OCP_V in the Dockerfile) are updated accordingly so
buildah pulls the correct image.
---
Nitpick comments:
In `@konflux-ci/fbc/catalog.Dockerfile`:
- Around line 14-15: Remove the unused stage-scoped ARG declarations ARG OCP_V
and ARG RHEL_V from the builder stage: they are not referenced by any
builder-stage instructions (e.g., the ADD and RUN cache-populate commands) and
are shadowed by the global ARGs declared earlier; delete these two ARG lines
from the builder stage (or move/restore them only if you plan to actually use
$OCP_V/$RHEL_V inside that stage) so the Dockerfile is not confusing about which
ARGs are in-scope.
- Line 11: Replace the Dockerfile instruction ADD with COPY for the local
directory copy: change the ADD $CAT_TYPE/catalog /configs line so it uses COPY
instead of ADD, ensuring you still reference the same source ($CAT_TYPE/catalog)
and destination (/configs); this avoids ADD's implicit behaviors like URL
fetching and tar extraction and makes the intent explicit.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.tekton/fbc-ocp4-12-pull-request.yaml.tekton/fbc-ocp4-12-push.yaml.tekton/fbc-ocp4-13-pull-request.yaml.tekton/fbc-ocp4-13-push.yaml.tekton/fbc-ocp4-14-pull-request.yaml.tekton/fbc-ocp4-14-push.yamlkonflux-ci/fbc/catalog.Dockerfile
🚧 Files skipped from review as they are similar to previous changes (4)
- .tekton/fbc-ocp4-14-pull-request.yaml
- .tekton/fbc-ocp4-12-push.yaml
- .tekton/fbc-ocp4-13-push.yaml
- .tekton/fbc-ocp4-12-pull-request.yaml
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #639 +/- ##
=======================================
Coverage 46.16% 46.16%
=======================================
Files 22 22
Lines 1083 1083
=======================================
Hits 500 500
Misses 553 553
Partials 30 30 🚀 New features to boost your workflow:
|
|
@ncaak: This pull request references DVO-363 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.22.0" version, but no target version was set. 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. |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.tekton/fbc-ocp4-13-pull-request.yaml (1)
91-97: Previous concern aboutose-operator-registry-$RHEL_Vimage is resolved.The switch to
catalog.old.Dockerfileeliminates theRHEL_Vbuild-arg and drops the-$RHEL_Vsuffix from the base image pull spec. Thebuild-argsdefault now only carriesOCP_V=v4.13, which is the single ARG consumed by the new Dockerfile.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.tekton/fbc-ocp4-13-pull-request.yaml around lines 91 - 97, Update the Tekton task inputs to match the new Dockerfile: remove all references to RHEL_V (including the ose-operator-registry-$RHEL_V suffix) and set the build-args default to only ["OCP_V=v4.13"] (as shown in the diff and consumed by catalog.old.Dockerfile); also verify the enable-cache-proxy parameter (name: enable-cache-proxy) has the intended default value (add back default: "false" if it was accidentally removed) so the pipeline behavior remains explicit.
🧹 Nitpick comments (3)
konflux-ci/fbc/catalog.old.Dockerfile (3)
8-8: ReplaceADDwithCOPYfor local directory.For items (files, directories) that do not require
ADD's tar auto-extraction capability, you should always useCOPY.bundle.object/catalogis a local directory with no tar/URL semantics.♻️ Proposed fix
-ADD bundle.object/catalog /configs +COPY bundle.object/catalog /configs🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@konflux-ci/fbc/catalog.old.Dockerfile` at line 8, The Dockerfile uses ADD for a local directory in the line adding bundle.object/catalog, which should be COPY; update the Dockerfile by replacing the ADD invocation that references bundle.object/catalog and the destination /configs with COPY so the local directory is copied without invoking ADD's extra behaviors.
1-1:ARG OCP_V=latestuses a mutable tag as the fallback default.All pipeline definitions pass an explicit
OCP_V=v4.XXso there's no build-time risk, but any out-of-pipeline manualdocker buildinvocation without--build-arg OCP_V=...will silently resolve to thelatesttag, which is mutable and can differ between runs.Consider defaulting to a specific pinned version (e.g., the minimum supported version) or leaving it empty to force an explicit value:
♻️ Proposed alternative
-ARG OCP_V=latest +ARG OCP_V🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@konflux-ci/fbc/catalog.old.Dockerfile` at line 1, The Dockerfile defines a mutable default for the build arg (ARG OCP_V=latest) which can lead to non-reproducible builds; change the ARG declaration in the Dockerfile (ARG OCP_V=latest) to either a concrete pinned OpenShift version (e.g., ARG OCP_V=v4.YY) or remove the default entirely (ARG OCP_V) so builds must pass an explicit --build-arg OCP_V=... at build time.
1-25: NoUSERinstruction — container runs as root (Trivy DS-0002).The base
ose-operator-registryimage from Red Hat may already declare a non-root user, which would make this a false positive. If it doesn't, adding aUSERdirective before the finalCMDaligns with container security best practices.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@konflux-ci/fbc/catalog.old.Dockerfile` around lines 1 - 25, The Dockerfile lacks a USER instruction so the final image may run as root; add a non-root user declaration (e.g. USER 1001 or another unprivileged UID that matches the base image) before the ENTRYPOINT/CMD so the container runs unprivileged; ensure any files copied from the builder (COPY --from=builder /configs and /tmp/cache) have ownership/permissions compatible with that UID (chown or adjust in the builder stage if needed) and keep ENTRYPOINT ["/bin/opm"] and CMD ["serve", "/configs", "--cache-dir=/tmp/cache"] unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.tekton/fbc-ocp4-13-pull-request.yaml:
- Around line 91-97: Update the Tekton task inputs to match the new Dockerfile:
remove all references to RHEL_V (including the ose-operator-registry-$RHEL_V
suffix) and set the build-args default to only ["OCP_V=v4.13"] (as shown in the
diff and consumed by catalog.old.Dockerfile); also verify the enable-cache-proxy
parameter (name: enable-cache-proxy) has the intended default value (add back
default: "false" if it was accidentally removed) so the pipeline behavior
remains explicit.
---
Nitpick comments:
In `@konflux-ci/fbc/catalog.old.Dockerfile`:
- Line 8: The Dockerfile uses ADD for a local directory in the line adding
bundle.object/catalog, which should be COPY; update the Dockerfile by replacing
the ADD invocation that references bundle.object/catalog and the destination
/configs with COPY so the local directory is copied without invoking ADD's extra
behaviors.
- Line 1: The Dockerfile defines a mutable default for the build arg (ARG
OCP_V=latest) which can lead to non-reproducible builds; change the ARG
declaration in the Dockerfile (ARG OCP_V=latest) to either a concrete pinned
OpenShift version (e.g., ARG OCP_V=v4.YY) or remove the default entirely (ARG
OCP_V) so builds must pass an explicit --build-arg OCP_V=... at build time.
- Around line 1-25: The Dockerfile lacks a USER instruction so the final image
may run as root; add a non-root user declaration (e.g. USER 1001 or another
unprivileged UID that matches the base image) before the ENTRYPOINT/CMD so the
container runs unprivileged; ensure any files copied from the builder (COPY
--from=builder /configs and /tmp/cache) have ownership/permissions compatible
with that UID (chown or adjust in the builder stage if needed) and keep
ENTRYPOINT ["/bin/opm"] and CMD ["serve", "/configs", "--cache-dir=/tmp/cache"]
unchanged.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.tekton/fbc-ocp4-12-pull-request.yaml.tekton/fbc-ocp4-12-push.yaml.tekton/fbc-ocp4-13-pull-request.yaml.tekton/fbc-ocp4-13-push.yaml.tekton/fbc-ocp4-14-pull-request.yaml.tekton/fbc-ocp4-14-push.yamlkonflux-ci/fbc/catalog.old.Dockerfile
🚧 Files skipped from review as they are similar to previous changes (3)
- .tekton/fbc-ocp4-12-pull-request.yaml
- .tekton/fbc-ocp4-12-push.yaml
- .tekton/fbc-ocp4-14-pull-request.yaml
|
@ncaak: This pull request references DVO-363 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.22.0" version, but no target version was set. 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. |
|
@ncaak: This pull request references DVO-363 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.22.0" version, but no target version was set. 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. |
|
/test deployment-validation-operator-e2e-tests |
3 participants
summary
Fixed the recently added pipelines to have the correct building args and architectures
tracker
https://issues.redhat.com/browse/DVO-363
Summary by CodeRabbit
New Features
Chores