NO-JIRA: Konflux: Move mirrorSetYaml result to workspace to avoid termination message truncation#424
Conversation
|
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:
📝 WalkthroughWalkthroughThe pipeline configuration in 🚥 Pre-merge checks | ✅ 12✅ Passed checks (12 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Review rate limit: 9/10 reviews remaining, refill in 6 minutes. Comment |
|
/retest |
alebedev87
changed the title
alebedev87
force-pushed
the
output-mirror-set
branch
from
6e68b3b to
178f370
Compare
alebedev87
force-pushed
the
output-mirror-set
branch
from
178f370 to
16135ca
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: grzpiotrowski 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 |
openshift-ci
Bot
added
the
approved
|
Formally it's a continuation of #422. Let me try to link it to the same Jira ticket. /retitle OCPBUGS-79591: Konflux: Move mirrorSetYaml result to workspace to avoid termination message truncation |
openshift-ci
Bot
changed the title
openshift-ci-robot
added
jira/severity-important
|
@alebedev87: This pull request references Jira Issue OCPBUGS-79591, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. 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. |
|
Wrong bug, https://redhat.atlassian.net/browse/OCPBUGS-76243 is the the right one but it's verified already. Let's link it to it anyway and then go with /retitle OCPBUGS-76243: Konflux: Move mirrorSetYaml result to workspace to avoid termination message truncation |
openshift-ci
Bot
changed the title
|
@alebedev87: This pull request references Jira Issue OCPBUGS-76243, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. 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. |
|
As explained in #424 (comment): /retitle NO-JIRA: Konflux: Move mirrorSetYaml result to workspace to avoid termination message truncation |
openshift-ci
Bot
changed the title
openshift-ci-robot
removed
jira/severity-important
|
@alebedev87: This pull request explicitly references no jira issue. 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. |
|
/retest |
…n message truncation The `fetch-config-files` task stores mirror set data in Tekton results which are passed via the container termination message (4096 byte limit). The termination message is also used by Tekton for internal metadata (step timing, result type markers), further reducing the usable space to ~3072 bytes. With growing mirror entries, the combined results exceeded this limit, causing truncation and silently breaking downstream task variable resolution. Move `mirrorSetYaml` to the `shared-data` workspace so it is written to a PVC file instead of a Tekton result. The `serializedMirrorSetYaml` and `scorecardConfigImages` results remain as Tekton results since they are consumed by remote step action refs.
alebedev87
force-pushed
the
output-mirror-set
branch
from
16135ca to
6448c9a
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/pipelines/deploy-fbc-operator.yaml:
- Around line 213-214: The current pipeline uses echo -n to write the mirror-set
YAML which can mangle raw YAML; replace the write step that references
mirror_set_yaml and the workspace file (currently using echo -n
"$mirror_set_yaml" > "$(workspaces.shared-data.path)/mirrorSetYaml") with a safe
write using printf '%s' to preserve the payload exactly (i.e., use printf '%s'
"$mirror_set_yaml" > "$(workspaces.shared-data.path)/mirrorSetYaml").
🪄 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 YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: c89cffc5-52e7-4695-9acd-a46b85257497
📒 Files selected for processing (1)
.tekton/pipelines/deploy-fbc-operator.yaml
| echo "Image mirror set: $mirror_set_yaml" | ||
| echo -n "$mirror_set_yaml" > "$(workspaces.shared-data.path)/mirrorSetYaml" |
There was a problem hiding this comment.
Use printf when persisting the mirror-set YAML.
echo -n is shell-dependent and can mangle raw YAML content in edge cases. printf '%s' preserves the payload exactly.
🔧 Safer write
- echo -n "$mirror_set_yaml" > "$(workspaces.shared-data.path)/mirrorSetYaml"
+ printf '%s' "$mirror_set_yaml" > "$(workspaces.shared-data.path)/mirrorSetYaml"📝 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.
| echo "Image mirror set: $mirror_set_yaml" | |
| echo -n "$mirror_set_yaml" > "$(workspaces.shared-data.path)/mirrorSetYaml" | |
| echo "Image mirror set: $mirror_set_yaml" | |
| printf '%s' "$mirror_set_yaml" > "$(workspaces.shared-data.path)/mirrorSetYaml" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.tekton/pipelines/deploy-fbc-operator.yaml around lines 213 - 214, The
current pipeline uses echo -n to write the mirror-set YAML which can mangle raw
YAML; replace the write step that references mirror_set_yaml and the workspace
file (currently using echo -n "$mirror_set_yaml" >
"$(workspaces.shared-data.path)/mirrorSetYaml") with a safe write using printf
'%s' to preserve the payload exactly (i.e., use printf '%s' "$mirror_set_yaml" >
"$(workspaces.shared-data.path)/mirrorSetYaml").
There was a problem hiding this comment.
It's fine to not add the default newline used by echo into the mirrorSetYaml file. I bear in mind this suggestion but it should not be a problem.
There was a problem hiding this comment.
@alebedev87, understood — the concern was primarily about shell portability (echo -n is not POSIX-standard and behaves differently across shells), not just the trailing newline. That said, since the script uses #!/usr/bin/env bash and runs in a known container image, echo -n is consistent there. Happy to defer to your judgment.
|
@alebedev87: 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. |
|
/lgtm |
|
/override "Red Hat Konflux / ext-dns-optr-fbc-v4-14-deploy-operator / external-dns-fbc-container-ext-dns-optr-fbc-v4-14" Hiccups of the cluster provisioning on 4.14. |
|
@alebedev87: Overrode contexts on behalf of alebedev87: Red Hat Konflux / ext-dns-optr-fbc-v4-14-deploy-operator / external-dns-fbc-container-ext-dns-optr-fbc-v4-14 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. |
openshift-merge-bot
Bot
merged commit 91d27f9
into
openshift:main
3 participants
The
fetch-config-filestask stores mirror set data in Tekton results which are passed via the container termination message (4096 byte limit). The termination message is also used by Tekton for internal metadata (step timing, result type markers), further reducing the usable space to ~3072 bytes. With growing mirror entries, the combined results exceeded this limit, causing truncation and silently breaking downstream task variable resolution.Move
mirrorSetYamlto theshared-dataworkspace so it is written to a PVC file instead of a Tekton result. TheserializedMirrorSetYamlandscorecardConfigImagesresults remain as Tekton results since they are consumed by remote step action refs.Follow-up of #422.