Skip to content

gather-extra: don't let PLATFORM lookup abort entire gather step #83299

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

Open
kaovilai wants to merge 2 commits into
base: main
Choose a base branch
from
+3 −3
Open

gather-extra: don't let PLATFORM lookup abort entire gather step #83299

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2d622bb
gather-extra: don't let PLATFORM lookup abort entire gather step
kaovilai Aug 11, 2026
8830df5
gather-extra: download jq into ARTIFACT_DIR, not /tmp
kaovilai Aug 12, 2026
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
6 changes: 3 additions & 3 deletions ci-operator/step-registry/gather/extra/gather-extra-commands.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ oc --insecure-skip-tls-verify --request-timeout=5s get pods -l openshift.io/comp

oc --insecure-skip-tls-verify --request-timeout=5s adm inspect clusteroperators --dest-dir ${ARTIFACT_DIR}/inspect || true

PLATFORM=$(oc get infrastructure cluster -o jsonpath="{.status.platform}")
PLATFORM=$(oc --insecure-skip-tls-verify --request-timeout=5s get infrastructure cluster -o jsonpath="{.status.platform}") || true

@coderabbitai coderabbitai Bot Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

rg -n -C 8 '(^|[[:space:]])PLATFORM[[:space:]]*=' \
  ci-operator/step-registry/gather/extra/gather-extra-commands.sh || true

result=$(bash -c 'PLATFORM=previous; PLATFORM=$(false) || true; printf "%s" "$PLATFORM"')
test -z "$result"

rg -n -C 8 'CAPI_PLATFORM|PLATFORM' \
  ci-operator/step-registry/opendatahub-io/hypershift/gather-extra/opendatahub-io-hypershift-gather-extra-commands.sh || true

Repository: openshift/release

Length of output: 5760

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

printf '%s\n' '--- gather-extra-commands.sh ---'
sed -n '1,70p' ci-operator/step-registry/gather/extra/gather-extra-commands.sh

printf '%s\n' '--- all PLATFORM references in the target script ---'
rg -n -C 4 '\bPLATFORM\b|CAPI_PLATFORM' \
  ci-operator/step-registry/gather/extra/gather-extra-commands.sh

printf '%s\n' '--- referenced HyperShift consumer ---'
sed -n '55,90p' ci-operator/step-registry/opendatahub-io/hypershift/gather-extra/opendatahub-io-hypershift-gather-extra-commands.sh

printf '%s\n' '--- shell assignment behavior ---'
bash -c 'unset PLATFORM; PLATFORM=$(false) || true; declare -p PLATFORM'
bash -c 'PLATFORM=previous; PLATFORM=$(false) || true; printf "existing=%q\n" "$PLATFORM"'

Repository: openshift/release

Length of output: 11793

Handle a failed platform lookup before queuing CAPI artifacts.

This script has no existing PLATFORM fallback. When oc fails without output, PLATFORM and CAPI_PLATFORM become empty. Lines 101–106 then construct invalid CAPI resource names. Skip these queues when the lookup fails or use a validated fallback.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ci-operator/step-registry/gather/extra/gather-extra-commands.sh` at line 45,
Update the platform lookup and CAPI artifact queuing flow around PLATFORM and
the logic at lines 101–106 so a failed or empty oc infrastructure lookup cannot
produce empty CAPI_PLATFORM resource names. Detect and handle lookup failure
before queuing, either by skipping the CAPI queues or by assigning a validated
fallback platform value, while preserving normal queuing for valid platform
results.

CAPI_PLATFORM=$(echo "$PLATFORM" | tr '[:upper:]' '[:lower:]')

if [[ "${CAPI_PLATFORM}" == "baremetal" ]]; then
Expand Down Expand Up @@ -768,12 +768,12 @@ else
source "${SHARED_DIR}/unset-proxy.sh"
fi
# This is a temporary conversion of cluster operator status to JSON matching the upgrade - may be moved to code in the future
curl -sL https://github.com/stedolan/jq/releases/download/jq-1.6/jq-linux64 >/tmp/jq && chmod ug+x /tmp/jq
curl -sL https://github.com/stedolan/jq/releases/download/jq-1.6/jq-linux64 >${ARTIFACT_DIR}/jq && chmod ug+x ${ARTIFACT_DIR}/jq

@coderabbitai coderabbitai Bot Aug 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail
shellcheck -x ci-operator/step-registry/gather/extra/gather-extra-commands.sh

Repository: openshift/release

Length of output: 50373

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

file='ci-operator/step-registry/gather/extra/gather-extra-commands.sh'
printf '%s\n' '--- relevant source ---'
sed -n '755,785p' "$file"

printf '%s\n' '--- ARTIFACT_DIR assignments and relevant uses ---'
rg -n 'ARTIFACT_DIR|clusteroperators\.json|junit_install_status' "$file" | tail -30

printf '%s\n' '--- shellcheck diagnostics for the relevant lines only ---'
shellcheck -x -f gcc "$file" 2>/dev/null | awk -F: '$2 == 771 || $2 == 776'

Repository: openshift/release

Length of output: 8838

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT
mkdir -p "$tmp/artifacts with space/junit"
touch "$tmp/artifacts with space/clusteroperators.json"
touch "$tmp/artifacts with space/jq"

cat >"$tmp/probe.sh" <<'EOF'
#!/usr/bin/env bash
set -euo pipefail
ARTIFACT_DIR="$1"
set -- ${ARTIFACT_DIR}/clusteroperators.json ${ARTIFACT_DIR}/jq
printf 'unquoted argc=%s\n' "$#"
printf 'unquoted arg=%q\n' "$@"
set -- "${ARTIFACT_DIR}/clusteroperators.json" "${ARTIFACT_DIR}/jq"
printf 'quoted argc=%s\n' "$#"
printf 'quoted arg=%q\n' "$@"
EOF
chmod +x "$tmp/probe.sh"

bash "$tmp/probe.sh" "$tmp/artifacts with space"

printf '%s\n' '--- ARTIFACT_DIR definitions in repository metadata and scripts ---'
rg -n --hidden --glob '!*.md' --glob '!vendor/**' \
  'ARTIFACT_DIR[[:space:]]*(=|:)|export[[:space:]]+ARTIFACT_DIR|artifacts?[-_]dir' . \
  | head -120

Repository: openshift/release

Length of output: 19517

Quote all ARTIFACT_DIR expansions.

At lines 771 and 776, unquoted paths split when ARTIFACT_DIR contains whitespace and expand glob characters. Quote the jq path, input path, output path, and chmod operand.

🧰 Tools
🪛 Shellcheck (0.11.0)

[info] 771-771: Double quote to prevent globbing and word splitting.

(SC2086)

[info] 771-771: Double quote to prevent globbing and word splitting.

(SC2086)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ci-operator/step-registry/gather/extra/gather-extra-commands.sh` at line 771,
Update the jq download and related commands around the jq installation flow to
quote every ARTIFACT_DIR-based path, including the jq output path, input and
output paths, and chmod operand, preventing whitespace splitting and glob
expansion.

Source: Linters/SAST tools

if test -f "${SHARED_DIR}/proxy-conf.sh"; then
# shellcheck disable=SC1090
source "${SHARED_DIR}/proxy-conf.sh"
fi
<${ARTIFACT_DIR}/clusteroperators.json /tmp/jq -r 'def one(condition; t): t as $t | first([.[] | select(condition)] | map(.type=t)[]) // null; def msg: "Operator \(.type) (\(.reason)): \(.message)"; def xmlfailure: if .failure then "<failure message=\"\(.failure | @html)\">\(.failure | @html)</failure>" else "" end; def xmltest: "<testcase name=\"\(.name | @html)\">\( xmlfailure )</testcase>"; def withconditions: map({name: "operator conditions \(.metadata.name)"} + ((.status.conditions // [{type:"Available",status: "False",message:"operator is not reporting conditions"}]) | (one(.type=="Available" and .status!="True"; "unavailable") // one(.type=="Degraded" and .status=="True"; "degraded") // one(.type=="Progressing" and .status=="True"; "progressing") // null) | if . then {failure: .|msg} else null end)); .items | withconditions | "<testsuite name=\"Operator results\" tests=\"\( length )\" failures=\"\( [.[] | select(.failure)] | length )\">\n\( [.[] | xmltest] | join("\n"))\n</testsuite>"' >${ARTIFACT_DIR}/junit/junit_install_status.xml
<${ARTIFACT_DIR}/clusteroperators.json ${ARTIFACT_DIR}/jq -r 'def one(condition; t): t as $t | first([.[] | select(condition)] | map(.type=t)[]) // null; def msg: "Operator \(.type) (\(.reason)): \(.message)"; def xmlfailure: if .failure then "<failure message=\"\(.failure | @html)\">\(.failure | @html)</failure>" else "" end; def xmltest: "<testcase name=\"\(.name | @html)\">\( xmlfailure )</testcase>"; def withconditions: map({name: "operator conditions \(.metadata.name)"} + ((.status.conditions // [{type:"Available",status: "False",message:"operator is not reporting conditions"}]) | (one(.type=="Available" and .status!="True"; "unavailable") // one(.type=="Degraded" and .status=="True"; "degraded") // one(.type=="Progressing" and .status=="True"; "progressing") // null) | if . then {failure: .|msg} else null end)); .items | withconditions | "<testsuite name=\"Operator results\" tests=\"\( length )\" failures=\"\( [.[] | select(.failure)] | length )\">\n\( [.[] | xmltest] | join("\n"))\n</testsuite>"' >${ARTIFACT_DIR}/junit/junit_install_status.xml
fi

# This is an experimental wiring of autogenerated failure detection.
Expand Down