Skip to content

NO-JIRA: Konflux: Move mirrorSetYaml result to workspace to avoid termination message truncation #424

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
openshift-merge-bot merged 1 commit into from
Apr 29, 2026
Merged

NO-JIRA: Konflux: Move mirrorSetYaml result to workspace to avoid termination message truncation #424

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions .tekton/pipelines/deploy-fbc-operator.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ spec:
The deploy-fbc-operator pipeline automates the provisioning of a EaaS Hypershift cluster and the deployment of an operator from a given FBC (File-Based Catalog) fragment.
It automates the process of retrieving an unreleased bundle from the FBC, selecting an appropriate OpenShift version and architecture for cluster provisioning,
installing the operator, and gathering cluster artifacts.
workspaces:
- name: shared-data
params:
- description: |
Spec section of an ApplicationSnapshot resource. Not all fields of the
Expand Down Expand Up @@ -117,10 +119,12 @@ spec:
- input: $(tasks.parse-metadata.results.test-event-type)
operator: in
values: ["pull_request", "ok-to-test-comment", "retest-all-comment"]
workspaces:
- name: shared-data
taskSpec:
workspaces:
- name: shared-data
results:
- name: mirrorSetYaml
description: "Contents of the image mirror set yaml file"
- name: serializedMirrorSetYaml
description: "A serialized string of an ImageDigestMirrorSet YAML, where each mirror entry is formatted with escaped newlines to preserve structure in a single-line format"
- name: scorecardConfigImages
Expand Down Expand Up @@ -206,7 +210,8 @@ spec:
exit 1
fi

echo -n "$mirror_set_yaml" > "$(results.mirrorSetYaml.path)"
echo "Image mirror set: $mirror_set_yaml"
echo -n "$mirror_set_yaml" > "$(workspaces.shared-data.path)/mirrorSetYaml"
Comment on lines +213 to +214
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 29, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

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.

Suggested change
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").

Copy link
Copy Markdown
Contributor Author

@alebedev87 alebedev87 Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.


serialized_mirror_set=$(serialize_image_mirrors_yaml "${mirror_set_yaml}")
echo "Serialized image mirror set: $serialized_mirror_set"
Expand Down Expand Up @@ -239,7 +244,11 @@ spec:
- input: $(tasks.parse-metadata.results.test-event-type)
operator: in
values: ["pull_request", "ok-to-test-comment", "retest-all-comment"]
workspaces:
- name: shared-data
taskSpec:
workspaces:
- name: shared-data
results:
- name: unreleasedBundle
description: "Unreleased bundle image (may be mirrored)"
Expand Down Expand Up @@ -282,13 +291,13 @@ spec:
env:
- name: BUNDLE_IMAGE
value: "$(steps.get-unreleased-bundle.results.unreleasedBundle)"
- name: MIRROR_SET_YAML
value: "$(tasks.fetch-config-files.results.mirrorSetYaml)"
script: |
#!/usr/bin/env bash
set -euo pipefail
. /utils.sh

MIRROR_SET_YAML=$(cat "$(workspaces.shared-data.path)/mirrorSetYaml")

replaced_image=""

if [[ -z "${BUNDLE_IMAGE}" || "${BUNDLE_IMAGE}" == "null" ]]; then
Expand Down