From 3603d42adc2267bade2878142f58ebda75f17b34 Mon Sep 17 00:00:00 2001 From: Jack Ding Date: Tue, 28 Jul 2026 12:19:56 -0400 Subject: [PATCH] Fix tls-scanner-run cleanup crash and Failed pod handling Four issues in tls-scanner-run-commands.sh: 1. OWNS_NAMESPACE unbound variable: The cleanup trap references NAMESPACE and OWNS_NAMESPACE which are local variables inside run_tls_scan(). When the trap fires after the function returns, locals are out of scope. Fix: snapshot both locals into global variables (_CLEANUP_NAMESPACE, _CLEANUP_OWNS_NAMESPACE) before registering the trap. 2. Scanner exit code 1 treated as infrastructure failure: When the scanner finds TLS compliance issues, it exits with code 1 and the pod phase is "Failed". The script called exit 1 immediately, wasting the 10-minute oc-wait timeout. Replace with artifact-aware check: if JUnit exists, log findings; if not, describe the pod. 3. Failed pod branch did not exit non-zero: After logging, the Failed branch fell through silently. Add exit with the pod's exit code. 4. Fallback oc-wait only waited for Succeeded: If the pod reached Failed during the wait, it would timeout for 10 minutes. Replace oc-wait with a polling loop that terminates on either Succeeded or Failed, then route Failed through the same artifact-aware handler. Co-Authored-By: Claude Opus 4.6 (1M context) Signed-off-by: Jack Ding --- .../scanner/run/tls-scanner-run-commands.sh | 67 +++++++++++++++---- 1 file changed, 53 insertions(+), 14 deletions(-) diff --git a/ci-operator/step-registry/tls/scanner/run/tls-scanner-run-commands.sh b/ci-operator/step-registry/tls/scanner/run/tls-scanner-run-commands.sh index 3d7a3131a92fe..cc2b625defcfd 100644 --- a/ci-operator/step-registry/tls/scanner/run/tls-scanner-run-commands.sh +++ b/ci-operator/step-registry/tls/scanner/run/tls-scanner-run-commands.sh @@ -100,13 +100,16 @@ run_tls_scan() { oc delete pod/tls-scanner -n "${NAMESPACE}" --ignore-not-found --wait=true --timeout=60s || true fi - # Cleanup on exit + # Snapshot locals into global variables so the EXIT trap can reference them + # after run_tls_scan returns and locals go out of scope. + _CLEANUP_NAMESPACE="${NAMESPACE}" + _CLEANUP_OWNS_NAMESPACE="${OWNS_NAMESPACE}" cleanup() { echo "Cleaning up..." - if [[ "${OWNS_NAMESPACE}" == "true" ]]; then - oc delete namespace "${NAMESPACE}" --ignore-not-found --wait=false || true + if [[ "${_CLEANUP_OWNS_NAMESPACE}" == "true" ]]; then + oc delete namespace "${_CLEANUP_NAMESPACE}" --ignore-not-found --wait=false || true else - oc delete pod/tls-scanner -n "${NAMESPACE}" --ignore-not-found --wait=false || true + oc delete pod/tls-scanner -n "${_CLEANUP_NAMESPACE}" --ignore-not-found --wait=false || true fi } trap cleanup EXIT @@ -251,18 +254,54 @@ EOF wait $LOGS_PID 2>/dev/null || true - if [[ "$(oc get pod/tls-scanner -n "${NAMESPACE}" -o jsonpath='{.status.phase}' 2>/dev/null)" == "Failed" ]]; then - echo "Scanner pod failed" - oc describe pod/tls-scanner -n "${NAMESPACE}" - exit 1 + # Check final pod status. The scanner exits non-zero when TLS findings are + # detected — this is expected and not an infrastructure failure. Propagate + # the scanner's exit code so CI marks the job accordingly, but do not treat + # it as a timeout or describe the pod as failed when artifacts were collected. + local pod_phase + pod_phase="$(oc get pod/tls-scanner -n "${NAMESPACE}" -o jsonpath='{.status.phase}' 2>/dev/null || echo "Unknown")" + local pod_exit_code + pod_exit_code="$(oc get pod/tls-scanner -n "${NAMESPACE}" -o jsonpath='{.status.containerStatuses[0].state.terminated.exitCode}' 2>/dev/null || echo "")" + + if [[ "${pod_phase}" == "Failed" ]]; then + echo "Scanner pod exited with phase=${pod_phase} code=${pod_exit_code}" + if [[ -f "${SCANNER_ARTIFACT_DIR}/junit_tls_scan.xml" ]]; then + echo "Artifacts were collected — scanner found TLS compliance issues." + else + echo "No artifacts found — scanner may have crashed." + oc describe pod/tls-scanner -n "${NAMESPACE}" + fi + exit "${pod_exit_code:-1}" + elif [[ "${pod_phase}" != "Succeeded" ]]; then + local poll_timeout=600 + local poll_interval=10 + echo "Scanner pod in unexpected phase: ${pod_phase} — polling until terminal (up to $((poll_timeout/60))m)" + local wait_elapsed=0 + while (( wait_elapsed < poll_timeout )); do + pod_phase="$(oc get pod/tls-scanner -n "${NAMESPACE}" -o jsonpath='{.status.phase}' 2>/dev/null || echo "Unknown")" + if [[ "${pod_phase}" == "Succeeded" || "${pod_phase}" == "Failed" ]]; then + break + fi + sleep "${poll_interval}" + (( wait_elapsed += poll_interval )) || true + done + if [[ "${pod_phase}" == "Failed" ]]; then + pod_exit_code="$(oc get pod/tls-scanner -n "${NAMESPACE}" -o jsonpath='{.status.containerStatuses[0].state.terminated.exitCode}' 2>/dev/null || echo "")" + echo "Scanner pod exited with phase=${pod_phase} code=${pod_exit_code}" + if [[ -f "${SCANNER_ARTIFACT_DIR}/junit_tls_scan.xml" ]]; then + echo "Artifacts were collected — scanner found TLS compliance issues." + else + echo "No artifacts found — scanner may have crashed." + oc describe pod/tls-scanner -n "${NAMESPACE}" + fi + exit "${pod_exit_code:-1}" + elif [[ "${pod_phase}" != "Succeeded" ]]; then + echo "Scanner did not complete successfully - timeout exceeded (phase: ${pod_phase})" + oc describe pod/tls-scanner -n "${NAMESPACE}" + exit 1 + fi fi - oc wait --for=jsonpath='{.status.phase}'=Succeeded pod/tls-scanner -n "${NAMESPACE}" --timeout=10m || { - echo "Scanner did not complete successfully - timeout exceeded" - oc describe pod/tls-scanner -n "${NAMESPACE}" - exit 1 - } - echo "=== TLS Scanner Complete ===" echo "Artifacts saved to: ${SCANNER_ARTIFACT_DIR}" ls -la "${SCANNER_ARTIFACT_DIR}" || true