Skip to content

Prod signing#2335

Closed
ralphbean wants to merge 62 commits into
konflux-ci:developmentfrom
ralphbean:prod-signing
Closed

Prod signing#2335
ralphbean wants to merge 62 commits into
konflux-ci:developmentfrom
ralphbean:prod-signing

Conversation

@ralphbean

Copy link
Copy Markdown
Member

No description provided.

brunoapimentel and others added 30 commits June 24, 2026 16:17
This pipeline will be used in Konflux for the releasing of the Java
artifacts. The necessary tasks, however, will be hosted in the dedicated
Slan-Cuan repository, since they have a tight coupling with the CLI tool
that is also built from this repository.

Assisted-by: Claude Sonnet 4.5
Signed-off-by: Bruno Pimentel <bpimente@redhat.com>
…via collect-data

Replace static pipeline params with dynamic extraction from RPA data blob.
Uses collect-data and collect-task-params tasks resolved from catalogGitUrl
to enable Trusted Artifacts handoff between pipeline stages.

Assisted-by: Claude Code (Sonnet 4.6)
Assisted-by: Claude Code (Sonnet 4.6)
Pass the `RADAS_CONFIG_PATH` as env secret

Signed-off-by: Jonathan Gangi <jgangi@redhat.com>
Signed-off-by: Bruno Pimentel <bpimente@redhat.com>
Fix path to tekton tasks in the slan-cuan pipeline
Signed-off-by: Luiz Carvalho <lucarval@redhat.com>
Add Conforma task to slan-cuan pipeline
Signed-off-by: Luiz Carvalho <lucarval@redhat.com>
Signed-off-by: Luiz Carvalho <lucarval@redhat.com>
Remove invalid env definition
Remove pnc-image, signing-key, trustify-api-url, and sso-token-url as
required pipeline params. pnc-image is now extracted from the snapshot
component image via a second collect-task-params step (collect-snapshot-params).
signing-key and Trustify URLs are extracted from RPA data alongside the
existing Pulp config.

Assisted-by: Claude Code (Sonnet 4.6)
slan-cuan-release: Derive all runtime params from snapshot and RPA data
Without a default, the release service cannot create the PipelineRun
since it has no mechanism to pass taskGitRevision. Default to main.

Assisted-by: Claude Code (Sonnet 4.6)
…or artifact passing

Switch slan-cuan domain tasks from shared-workspace to Trusted Artifacts.
extract creates a TA of the extracted artifact directory; sign, register,
and publish each restore it. This removes the dependency on a shared PVC,
which the release service cannot provision. Also pins taskGitUrl to
arewm/slan-cuan@509fb97 and removes the shared-workspace pipeline declaration.

Assisted-by: Claude Code (Sonnet 4.6)
Signed-off-by: Luiz Carvalho <lucarval@redhat.com>
Syncing in changes from andrew's fork
Signed-off-by: Luiz Carvalho <lucarval@redhat.com>
Add missing enterpriseContractTimeout pipeline param
Signed-off-by: Luiz Carvalho <lucarval@redhat.com>
Use Andrew's fork of slan-cuan tasks
Signed-off-by: Luiz Carvalho <lucarval@redhat.com>
Use slan-cuan tasks from Andrew's fork
Signed-off-by: Jonathan Gangi <jgangi@redhat.com>
brunoapimentel and others added 18 commits June 26, 2026 16:22
Uploading of the SBOM is out of scope for now

Signed-off-by: Luiz Carvalho <lucarval@redhat.com>
Remove slan-cuan register task from pipeline
Signed-off-by: Luiz Carvalho <lucarval@redhat.com>
Disable references to results not populated by task
Signed-off-by: Luiz Carvalho <lucarval@redhat.com>
Signed-off-by: Jonathan Gangi <jgangi@redhat.com>
slan-cuan: pass the secrets for pulp to the task
Signed-off-by: Jonathan Gangi <jgangi@redhat.com>
Signed-off-by: Jonathan Gangi <jgangi@redhat.com>
This reverts commit 9a589d4.
Due to a bug in the Python script, reused IIB builds cause
missing iibLog result, failing PipelineRuns with
CouldntGetPipelineResult.

Signed-off-by: Filip Nikolovski <fnikolov@redhat.com>
The jira_ci.py script no longer works after the Jira
migration. Replace it with a call to a Jira Automation
webhook that handles ticket updates (label swaps,
comments, transitions) via an automation rule.

The parsed tickets payload format has changed to group
PRs by ticket, as Jira Automation's smart values cannot
correlate individual entries back to the current issue
being processed.

Requires JIRA_AUTOMATION_WEBHOOK_URL and
JIRA_AUTOMATION_WEBHOOK_TOKEN repository secrets.

Assisted-by: Cursor
Signed-off-by: Lubomir Gallovic <lgallovi@redhat.com>
The verify-conforma task was running in parallel with sign but not
gating it. Add verify-conforma to sign's runAfter list so that the
Conforma policy check must pass before signing begins.

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
…s-sign

Revert: make verify-conforma block sign task
@qodo-app-for-konflux-ci

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Warning

/review is deprecated. Use /agentic_review instead (removal date not yet scheduled).

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 PR contains tests
🔒 Security concerns

Sensitive information exposure:
In tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml, the step script runs with set -x and builds JSON containing overwrite_from_index_token (username:token). Depending on how bash xtrace renders the heredoc/redirections and subsequent commands, this may leak tokens into Task logs. Consider disabling xtrace around JSON construction and curl invocation that includes secrets, and avoid echoing/printing files containing credentials.

⚡ Recommended focus areas for review

Secret Leakage

The script enables xtrace and later writes sensitive data (overwrite token and potentially registry auth) into JSON / commands; ensure logs cannot expose credentials and that debug tracing is disabled around any secret material creation/usage.

      # disabling debug to not leak the token
      set +x
      jq -n --arg authName "${authName}" \
            --arg token "$(base64 -w 0 < <(printf %s "${PUBLISHING_CREDENTIAL}"))" \
            '.auths[$authName].auth = $token' > "${HOME}/.config/containers/auth.json"
      set -x
    }

    # performs kerberos authentication.
    base64 -d /mnt/service-account-secret/keytab > "/tmp/keytab"

    KRB5_TEMP_CONF=$(mktemp)
    KRB5_PRINCIPAL=$(cat /mnt/service-account-secret/principal)

    echo "${KRB5_CONF_CONTENT}" > "${KRB5_TEMP_CONF}"
    export KRB5_CONFIG="${KRB5_TEMP_CONF}"
    export KRB5_TRACE=/dev/stderr

    retry 5 kinit -V "${KRB5_PRINCIPAL}" -k -t "/tmp/keytab"

    set -x

    FROM_INDEX="$(params.fromIndex)"

    # Validate that required environment variables are set
    if [ -z "${FBC_FRAGMENTS:-}" ]; then
        echo "Error: FBC_FRAGMENTS environment variable is required"
        exit 1
    fi
    if [ -z "${FROM_INDEX:-}" ]; then
        echo "Error: FROM_INDEX environment variable is required"
        exit 1
    fi

    # validate fbcFragments parameter is valid JSON array
    if ! echo "${FBC_FRAGMENTS}" | jq -e . >/dev/null 2>&1; then
      echo "Error: fbcFragments parameter must be a valid JSON array"
      jq -n '{ "state": "failed", "state_reason": "Invalid fbcFragments parameter" }' \
        | tee "$(results.buildState.path)"
      echo -n 1 > "$(results.exitCode.path)"
      exit 1
    fi

    # check if fbcFragments array is empty
    if [ "$(echo "${FBC_FRAGMENTS}" | jq 'length')" -eq 0 ]; then
      echo "Error: fbcFragments array is empty - no fragments to process"
      jq -n '{ "state": "failed", "state_reason": "Empty fbcFragments array" }' | tee "$(results.buildState.path)"
      echo -n 1 > "$(results.exitCode.path)"
      exit 1
    fi

    # Use pre-determined publishing decisions from prepare-fbc-parameters
    echo "Processing fragments: ${FBC_FRAGMENTS}"
    mustOverwriteFromIndexImage="$(params.mustOverwriteFromIndexImage)"
    mustPublishIndexImage="$(params.mustPublishIndexImage)"

    echo "Using pre-determined publishing decisions from prepare-fbc-parameters:"
    echo "             \`mustOverwriteFromIndexImage==${mustOverwriteFromIndexImage}\`"
    echo "             \`mustPublishIndexImage==${mustPublishIndexImage}\`"

    # Sort fragments once for consistent use throughout the script
    sorted_fbc_fragments=$(echo "${FBC_FRAGMENTS}" | jq 'sort')

    # if it finds a build which is completed or in progress, it should exit this step and jump to
    # the next step `s-wait-for-build-state` which will watch the build until it is completed.
    # Pass build_tags to enable PLR filtering for in-progress builds
    # shellcheck disable=SC2016 # Tekton substitutes params before bash runs; single quotes preserve JSON
    build_tags_param='$(params.buildTags)'
    build=$(check_previous_build "${KRB5_PRINCIPAL}" "${FROM_INDEX}" \
              "${sorted_fbc_fragments}" "${build_tags_param}")
    if [ -n "${build}" ]; then
      echo "=== A previous build for this fragment was found ==="
      echo "${build}" | gzip -c | base64 -w0 | tee "$(results.jsonBuildInfo.path)"
      exit 0
    fi

    # adds the json request parameters to a file to be used as input data
    # for curl and preventing shell expansion.
    json_input="/tmp/$$.tmp"
    json_raw_input="/tmp/$$_raw.tmp"

    cat > "$json_raw_input" <<JSON
    {
      "fbc_fragments": ${sorted_fbc_fragments},
      "from_index": "${FROM_INDEX}",
      "build_tags": $(params.buildTags),
      "add_arches": $(params.addArches),
      "overwrite_from_index": ${mustOverwriteFromIndexImage},
      "overwrite_from_index_token": "${IIB_OVERWRITE_FROM_INDEX_USERNAME}:${IIB_OVERWRITE_FROM_INDEX_TOKEN}"
    }
    JSON

    # filtering out empty params
    jq -r '
      if .overwrite_from_index == false then del(( .overwrite_from_index, .overwrite_from_index_token)) else . end |
      if(.add_arches | length) == 0 then del(.add_arches) else . end |
      if(.build_tags | length) == 0 then del(.build_tags) else . end' "${json_raw_input}" > "${json_input}"

    jq -n '{ "state": "in_progress", "state_reason": "Calling IIB endpoint" }' | tee "$(results.buildState.path)"
    # adds image to the index.
    iib_response=$(curl -u : --negotiate -s -X POST \
      -H "Content-Type: application/json" -d@"${json_input}" --insecure \
      "${IIB_SERVICE_URL}/builds/fbc-operations")

    # checks if the previous call returned an error and exits if so
    if jq -e -r ".error | select( . != null )" <<< "${iib_response}" >/dev/null; then
        jq -n '{ "state": "failed", "state_reason": "IIB service error" }' | tee "$(results.buildState.path)"
        echo -n 1 > "$(results.exitCode.path)"
        exit 1
    fi

    # compress and store the response only after error check passes
    echo "${iib_response}" | gzip -c | base64 -w0 > "$(results.jsonBuildInfo.path)"
- name: update-fbc-catalog-wait-for-iib-build-step
Fragile Mock

The mocked curl implementation relies on parsing "$*" and hard-coded positional arguments, which can break when curl invocations change slightly; consider more robust argument parsing and safer pattern matching.

function curl() {
  params="$*"
  # Debug: always print the task name and curl params for troubleshooting
  echo "DEBUG: TaskRun name: $(context.taskRun.name)" >&2
  echo "DEBUG: curl params: $params" >&2

  if [[ "$params" =~ "--negotiate -u: https://pyxis.engineering.redhat.com/v1/repositories/registry/quay.io/repository/repo/image -o"* ]]; then
    tempfile="$5"
    echo -e '{ "fbc_opt_in": true }' > "$tempfile"

  elif [[ "$params" == *"-s https://fakeiib.host/builds"* ]] && [[ "$params" == *"from_index="* ]] && [[ "$params" == *"state="* ]] && [[ "$params" != *"/builds/1"* ]] && [[ "$params" != *"/api/v1/builds"* ]]; then
    # Check params directly for registry-proxy pattern (most reliable)
    is_registry_proxy_test=false
    if [[ "$params" =~ registry-proxy.engineering.redhat.com/rh-osbs/iib-pub ]]; then
      is_registry_proxy_test=true
    fi
    # Extract from_index from params to help with debugging
    # Handle both URL-encoded and non-encoded versions
    extracted_from_index=""
    # Try multiple patterns to extract from_index
    # Expected formats in params:
    #   - Non-encoded: "from_index=registry-proxy.engineering.redhat.com/rh-osbs/iib-pub:v4.12"
    #   - URL-encoded: "from_index%3Dregistry-proxy.engineering.redhat.com%2Frh-osbs%2Fiib-pub%3Av4.12"
    #   (where %3D is '=', %2F is '/', %3A is ':')
    if [[ "$params" =~ from_index=([^&\ ]+) ]]; then
      extracted_from_index="${BASH_REMATCH[1]}"
    elif [[ "$params" =~ from_index%3D([^&\ ]+) ]]; then
      # URL-encoded equals sign (%3D = '=')
      extracted_from_index="${BASH_REMATCH[1]}"
    fi
    # URL decode if needed (simple case for common characters)
    if [[ -n "$extracted_from_index" ]]; then
      extracted_from_index="${extracted_from_index//%3A/:}"
      extracted_from_index="${extracted_from_index//%2F//}"
      extracted_from_index="${extracted_from_index//%40/@}"
      # If from_index contains registry-proxy, we should be in auth-failure test
      if [[ "$extracted_from_index" =~ registry-proxy.engineering.redhat.com/rh-osbs/iib-pub ]]; then
        is_registry_proxy_test=true
      fi
    fi
    build="${buildSeed}"
    taskrun_name="$(context.taskRun.name)"

    # Check if this is the auth-failure test by from_index value FIRST (more reliable)
    # If from_index contains registry-proxy, treat it as auth-failure regardless of taskrun name
    # For auth-failure test: return an OLD build (>24 hours) that should be rejected
    if [[ "$is_registry_proxy_test" = "true" ]] || \
       ([[ -n "$extracted_from_index" ]] && [[ "$extracted_from_index" =~ registry-proxy.engineering.redhat.com/rh-osbs/iib-pub ]]); then
      # Set updated timestamp to 25 hours ago (old build that should be rejected)
      old_timestamp=$(date -u -d '25 hours ago' --iso-8601=seconds 2>/dev/null || date -u -v-25H --iso-8601=seconds 2>/dev/null || echo "$(date -u --iso-8601=seconds)")
      jq_expr="$(get_auth_failure_jq_expr "${old_timestamp}")"
      build=$(jq -rc "$jq_expr" <<< "${build}")
    else
      # Fall back to taskrun name matching
      case "${taskrun_name}" in
        *stage-resume-inprogress*)
          # For stage-resume-inprogress test: validate KONFLUX-12016 fix
          # Return in-progress stage build only for in_progress queries, empty for complete queries
          echo "DEBUG: Setting stage-resume-inprogress case" >&2
          if [[ "$params" == *"state=in_progress"* ]]; then
            # Return the in-progress stage build - this should be resumed
            build=$(jq -rc '.items[0].distribution_scope = "stage"' <<< "${build}")
            build=$(jq -rc '.items[0].state = "in_progress"' <<< "${build}")
            build=$(jq -rc '.items[0].from_index = "quay.io/scoheb/fbc-index-testing:latest"' <<< "${build}")
            build=$(jq -rc '.items[0].index_image = "quay.io/scoheb/fbc-index-testing:latest"' <<< "${build}")
          else
            # For state=complete queries, return empty (no completed build to resume)
            build='{"items": []}'
          fi
        ;;
        *auth-failure*)
          # For auth-failure test, return an OLD build (>24 hours) that should be rejected
          # Check this BEFORE *complete* to avoid pattern collision
          old_timestamp=$(date -u -d '25 hours ago' --iso-8601=seconds 2>/dev/null || date -u -v-25H --iso-8601=seconds 2>/dev/null || echo "$(date -u --iso-8601=seconds)")
          jq_expr="$(get_auth_failure_jq_expr "${old_timestamp}")"
          build=$(jq -rc "$jq_expr" <<< "${build}")
        ;;
        *complete*|*multiple-fragments-retry*|*multiple-fragments*)
          # For complete and multiple-fragments-retry tests, use "retry-complete" as the mock case
          # Added *multiple-fragments* to catch truncated names
          taskrun_name="retry-complete"
          echo "DEBUG: Setting retry-complete case" >&2
          # For multiple fragments retry test, set the correct fragments array to match what the task expects
          if [[ "$(context.taskRun.name)" =~ "multiple-fragments-retry" ]]; then
            # Retry scenario with 2 fragments, ensure from_index and index_image match task parameters
            build=$(jq -rc '.items[0].fbc_fragments = ["registry.io/image0@sha256:0000", "registry.io/image1@sha256:1111"] | .items[0].from_index = "quay.io/scoheb/fbc-index-testing:latest" | .items[0].index_image = "quay.io/scoheb/fbc-index-testing:latest"' <<< "${build}")
          elif [[ "$(context.taskRun.name)" =~ "multiple-fragments" ]]; then
            # Basic multiple fragments test with 3 fragments, ensure from_index and index_image match task parameters
            build=$(jq -rc '.items[0].fbc_fragments = ["registry.io/image0@sha256:0000", "registry.io/image1@sha256:1111", "registry.io/image2@sha256:2222"] | .items[0].from_index = "quay.io/scoheb/fbc-index-testing:latest" | .items[0].index_image = "quay.io/scoheb/fbc-index-testing:latest"' <<< "${build}")
          fi
          build=$(jq -rc --arg taskrun_name "$taskrun_name" '.items[0].mock_case = $taskrun_name' <<< "${build}")
          build=$(jq -rc '.items[0].state = "complete"' <<< "${build}")
          build=$(jq -rc '.items[0].state_reason = "The FBC fragment was successfully added in the index image"' <<< "${build}")
        ;;
        *outdated*)
          # For outdated tests, set state to complete but don't set mock_case (triggers new build)
          echo "DEBUG: Setting outdated case" >&2
          build=$(jq -rc '.items[0].state = "complete"' <<< "${build}")
          build=$(jq -rc '.items[0].state_reason = "The FBC fragment was successfully added in the index image"' <<< "${build}")
        ;;
        *"retry-in-progress"*)
          echo "DEBUG: Setting retry-in-progress case" >&2
          build=$(jq -rc '.items[0].mock_case = "retry-in-progress"' <<< "${buildSeed}")
        ;;
        *empty-fragments*)
          # For empty fragments test, the task should exit early before reaching this point
          # But if it does reach here, return empty build list
          echo "DEBUG: Setting empty-fragments case" >&2
          build='{"items": []}'
        ;;
        *index-mismatch*)
          # For index-mismatch test, return a completed build with wrong index_image
          echo "DEBUG: Setting index-mismatch case" >&2
          build=$(jq -rc '.items[0].fbc_fragments = ["registry.io/image0@sha256:0000"] | .items[0].from_index = "quay.io/fbc/catalog:mismatch" | .items[0].index_image = "quay.io/scoheb/fbc-index-testing:latest"' <<< "${build}")
          build=$(jq -rc '.items[0].state = "complete"' <<< "${build}")
          build=$(jq -rc '.items[0].state_reason = "The FBC fragment was successfully added in the index image"' <<< "${build}")
        ;;
        *)
          # Fallback: if from_index contains registry-proxy and we didn't match any case,
          # treat it as auth-failure test (return old build)
          if [[ -n "$extracted_from_index" ]] && [[ "$extracted_from_index" =~ registry-proxy.engineering.redhat.com/rh-osbs/iib-pub ]]; then
            old_timestamp=$(date -u -d '25 hours ago' --iso-8601=seconds 2>/dev/null || date -u -v-25H --iso-8601=seconds 2>/dev/null || echo "$(date -u --iso-8601=seconds)")
            jq_expr="$(get_auth_failure_jq_expr "${old_timestamp}")"
            build=$(jq -rc "$jq_expr" <<< "${build}")
          fi
        ;;
      esac
    fi

    # Final check: ALWAYS check params directly for registry-proxy pattern and fix if needed
    # This is the most reliable check - just look for the pattern in the params string
    current_from_index=$(jq -r '.items[0].from_index // empty' <<< "${build}")
    if [[ "$params" == *"registry-proxy.engineering.redhat.com/rh-osbs/iib-pub"* ]]; then
      if [[ "$current_from_index" != "registry-proxy.engineering.redhat.com/rh-osbs/iib-pub:v4.12" ]]; then
        old_timestamp=$(date -u -d '25 hours ago' --iso-8601=seconds 2>/dev/null || date -u -v-25H --iso-8601=seconds 2>/dev/null || echo "$(date -u --iso-8601=seconds)")
        jq_expr="$(get_auth_failure_jq_expr "${old_timestamp}")"
        build=$(jq -rc "$jq_expr" <<< "${build}")
      fi
    fi
    echo -en "${build}"

  elif [[ "$params" == "-s https://fakeiib.host/builds/1" ]] || [[ "$params" == "-s https://fakeiib.host/builds/2" ]]; then
    set -x
    echo "$*" >> mock_build_progress_calls
    if [[ "$(context.taskRun.name)" =~ "test-update-fbc-catalog-error"* ]]; then
        mock_error="true"
    fi

    # Decompress the jsonBuildInfo since task now uses compression
    buildJson="$(base64 -d < $(results.jsonBuildInfo.path) | gunzip)"

    # For index-mismatch test, keep default index_image to trigger validation failure
    # (no action needed - default index_image won't match fromIndex)

    mock_build_progress "$(awk 'END{ print NR }' mock_build_progress_calls)" "$(base64 <<< "${buildJson}")" "$mock_error" | tee build_json
    export -n buildJson
    buildJson=$(cat build_json)
    export buildJson

  elif [[ "$params" == "-s https://fakeiib.host/api/v1/builds/1/logs" ]] || [[ "$params" == "-s https://fakeiib.host/api/v1/builds/2/logs" ]]; then
    echo "Logs are for weaks"

  elif [[ "$params" =~ "-u : --negotiate -s -X POST -H Content-Type: application/json -d@".*" --insecure https://fakeiib.host/builds/fbc-operations" ]]; then
    # For POST requests, use the buildSeed template as the base
    buildJson=$(jq -cr '.items[0]' <<< "${buildSeed}")

    # Check if this is stage-resume-inprogress test
    # If we reach here in the stage-resume-inprogress test, it means the resume failed
    # (should not happen with the fix, but good to track for debugging)
    if [[ "$(context.taskRun.name)" == *"stage-resume-inprogress"* ]]; then
      echo "DEBUG: WARNING - POST request in stage-resume-inprogress test means build was NOT resumed!" >&2
      # Set up stage build for this test case
      buildJson=$(jq -c '.distribution_scope = "stage" | .from_index = "quay.io/scoheb/fbc-index-testing:latest" | .index_image = "quay.io/scoheb/fbc-index-testing:latest"' <<< "${buildJson}")
    fi

    # Check if this is auth-failure test by taskrun name
    if [[ "$(context.taskRun.name)" == *"auth-failure"* ]]; then
      # For auth-failure test, set correct from_index and assign a NEW id (2) since old build (1) was rejected
      buildJson=$(jq -c '.id = 2 | .from_index = "registry-proxy.engineering.redhat.com/rh-osbs/iib-pub:v4.12" | .index_image = "registry-proxy.engineering.redhat.com/rh-osbs/iib-pub:v4.12" | .logs.url = "https://fakeiib.host/api/v1/builds/2/logs"' <<< "${buildJson}")
    fi

    # For multiple fragments tests, update the buildJson to include the appropriate fbc_fragments array
    case "$(context.taskRun.name)" in
        *multiple-fragments*)
          if [[ "$(context.taskRun.name)" =~ "multiple-fragments-retry" ]]; then
            # For retry scenario with 2 fragments
            buildJson=$(jq -c '.fbc_fragments = ["registry.io/image0@sha256:0000", "registry.io/image1@sha256:1111"]' <<< "${buildJson}")
          else
            # For basic multiple fragments test with 3 fragments, set correct index_image for production build validation
            buildJson=$(jq -c '.fbc_fragments = ["registry.io/image0@sha256:0000", "registry.io/image1@sha256:1111", "registry.io/image2@sha256:2222"] | .index_image = "quay.io/scoheb/fbc-index-testing:latest"' <<< "${buildJson}")
          fi
        ;;
        *empty-fragments*)
          # For empty array test - this should not be reached since task exits early
          buildJson=$(jq -c '.fbc_fragments = []' <<< "${buildJson}")
        ;;
        *invalid-fragments*)
          # For invalid JSON test - this shouldn't reach here due to early validation failure
          # But if it does, return an error response
          echo '{"error": "Invalid fbc_fragments parameter"}'
          exit
        ;;
        *index-mismatch*)
          # For index-mismatch test, set up mismatched from_index and index_image
          buildJson=$(jq -c '.fbc_fragments = ["registry.io/image0@sha256:0000"] | .from_index = "quay.io/fbc/catalog:mismatch" | .index_image = "quay.io/scoheb/fbc-index-testing:latest"' <<< "${buildJson}")
        ;;
        *error*)
          # For error test - return successful API response, but build will fail in step 2
          buildJson=$(jq -c '.fbc_fragments = ["registry.io/image0@sha256:0000"]' <<< "${buildJson}")
        ;;
    esac
    # Export the updated buildJson for use in subsequent calls
    export buildJson
    # Return uncompressed JSON - the task will handle compression
    echo "${buildJson}"
  else
    # Catch-all: if no pattern matched, check if it looks like a check_previous_build call
    if [[ "$params" == *"-s https://fakeiib.host/builds"* ]] && [[ "$params" == *"from_index="* ]] && [[ "$params" == *"state="* ]] && [[ "$params" != *"/builds/1"* ]] && [[ "$params" != *"/api/v1/builds"* ]]; then
      # This should have been caught by the earlier pattern, but if not, handle it here
      build="${buildSeed}"
      if [[ "$params" == *"registry-proxy.engineering.redhat.com/rh-osbs/iib-pub"* ]]; then
        old_timestamp=$(date -u -d '25 hours ago' --iso-8601=seconds 2>/dev/null || date -u -v-25H --iso-8601=seconds 2>/dev/null || echo "$(date -u --iso-8601=seconds)")
        jq_expr="$(get_auth_failure_jq_expr "${old_timestamp}")"
        build=$(jq -rc "$jq_expr" <<< "${build}")
      fi
      echo -en "${build}"
    else
      echo ""
    fi
  fi
}
📚 Focus areas based on broader codebase context

SecurityContext

The new update-fbc-catalog-wait-for-iib-build-step step lacks an explicit securityContext.runAsUser, while the first step sets runAsUser: 1001. Align the wait step with the same non-root runAsUser to avoid running as root depending on the image/user defaults and to keep step behavior consistent. (Ref 1, Ref 4)

- name: update-fbc-catalog-wait-for-iib-build-step
  image: >-
    quay.io/konflux-ci/release-service-utils@sha256:b524cefd4f00d386fd50fcc4892b761f09c8c5ca49242700c5ee129aae1408d3
  computeResources:
    limits:
      memory: 256Mi
    requests:
      memory: 256Mi
      cpu: 250m
  volumeMounts:

Reference reasoning: The existing managed task YAMLs consistently apply securityContext.runAsUser: 1001 via stepTemplate, ensuring all steps run as the same non-root UID. Mirroring that pattern here avoids per-step drift and reduces the risk of permission differences between steps.

📄 References
  1. konflux-ci/release-service-catalog/tasks/managed/rh-sign-image/rh-sign-image.yaml [96-130]
  2. konflux-ci/release-service-catalog/tasks/managed/rh-sign-image-cosign/rh-sign-image-cosign.yaml [81-116]
  3. konflux-ci/release-service-catalog/tasks/managed/sign-image-cosign-keyless/sign-image-cosign-keyless.yaml [79-115]
  4. konflux-ci/release-service-catalog/tasks/managed/rh-sign-rpm/rh-sign-rpm.yaml [156-174]
  5. konflux-ci/release-service-catalog/tasks/managed/rh-sign-image/tests/test-rh-sign-image-single-component-plr-alreadysigned.yaml [28-51]
  6. konflux-ci/release-service-catalog/tasks/managed/rh-sign-image/tests/test-rh-sign-image-multiple-components-already-signed.yaml [32-55]
  7. konflux-ci/release-service-catalog/tasks/managed/rh-sign-image/tests/test-rh-sign-image-helm-chart.yaml [28-51]
  8. konflux-ci/release-service-catalog/tasks/managed/rh-sign-rpm/tests/test-rh-sign-rpm.yaml [28-51]

@qodo-app-for-konflux-ci

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (11) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 25 rules

Grey Divider


Action required

1. Result written with echo 📘 Rule violation ≡ Correctness
Description
The task writes the iibLog Tekton result using echo without -n, which appends a trailing
newline into the result value. This can cause downstream parsing/validation issues when consumers
expect an exact URL string.
Code

tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[R495-496]

+                url="$(jq -r ".logs.url" <<< "${build_info}")"
+                echo IIB log url is: "${url}" > "$(results.iibLog.path)"
Relevance

⭐⭐⭐ High

Repo commonly uses echo -n for Tekton results; newline-free result writing appears expected (e.g.,
PR #2266).

PR-#2266

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 1259 requires echo -n (or equivalent) when writing Tekton results; the script writes to
$(results.iibLog.path) using echo ... > without -n.

Rule 1259: Use echo -n when writing Tekton task results to avoid trailing newlines
tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[495-496]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A Tekton result file is written using `echo` without `-n`, introducing a trailing newline.

## Issue Context
Checklist requires `echo -n` (or `printf %s`) when writing Tekton results.

## Fix Focus Areas
- tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[495-496]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Empty digests still succeed 🐞 Bug ≡ Correctness
Description
In update-fbc-catalog-task’s wait step, when indexImageDigests is empty it writes exitCode=1 to
results but still exits with 0 (BUILDEXIT), so Tekton reports the TaskRun as successful while
emitting failure-like results.
Code

tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[R552-589]

+        if [ ${BUILDEXIT} -eq 0 ]; then
+            echo -n 0 > "$(results.exitCode.path)"
+
+            # get the manifest digests
+            indexImageCopy=$(base64 -d < "$(results.jsonBuildInfo.path)" | gunzip | \
+              jq -cr .internal_index_image_copy)
+            # Use this to obtain the manifest digests for each arch in manifest list
+            indexImageDigestsRaw=$(skopeo inspect --retry-times 3 --raw "docker://${indexImageCopy}")
+            # according the IIB team,
+            #  "all index images will always be multi-arch with a manifest list"
+            #
+            indexImageDigests=$(echo "${indexImageDigestsRaw}" | \
+               jq -r \
+               '.manifests[]? | select(.mediaType=="application/vnd.docker.distribution.manifest.v2+json") | .digest' \
+               | tr '\n' ' ' | sed 's/ $//')  # make sure the result is on one line and remove trailing space
+            echo -n "${indexImageDigests}" > "$(results.indexImageDigests.path)"
+            if [ -z "${indexImageDigests}" ] ; then
+              echo "Index image produced is not multi-arch with a manifest list"
+              echo -n 1 > "$(results.exitCode.path)"
+            fi
+        else
+            if [ ${BUILDEXIT} -eq 124 ]; then
+                echo "Timeout while waiting for the build to finish"
+                jq -n '{ "state": "failed", "state_reason": "Build timeout" }' | tee "$(results.buildState.path)"
+            else
+                echo "Build failed with exit code ${BUILDEXIT}"
+                jq -n --arg exit_code "$BUILDEXIT" \
+                  '{ "state": "failed", "state_reason": ("Build failed with exit code " + $exit_code) }' \
+                  | tee "$(results.buildState.path)"
+            fi
+            echo -n "" > "$(results.indexImageDigests.path)"
+            echo -n "$BUILDEXIT" > "$(results.exitCode.path)"
+        fi
+        # We don't put the log in a result because tekton results are too limited for what we can put
+        # to be useful, but still print it for debugging
+        curl -s "$(awk '{print $NF}' < "$(results.iibLog.path)")"
+
+        exit "${BUILDEXIT}"
Relevance

⭐⭐⭐ High

Team tends to ensure real failures propagate via non-zero exits, not just results (see PRs #1860,
#2021).

PR-#1860
PR-#2021

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The wait step explicitly flags empty digests by writing exitCode=1 but then exits with
${BUILDEXIT} (0 in the success path). Downstream, add-fbc-contribution assumes indexImageDigests
exists to build its structured results.

tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[551-589]
tasks/managed/add-fbc-contribution/add-fbc-contribution.yaml[488-503]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The wait step detects an empty `indexImageDigests` result, but only updates `$(results.exitCode.path)` and does not fail the step process (it still `exit "${BUILDEXIT}"` where `BUILDEXIT` is 0). This creates an inconsistent state: TaskRun success with error results.

### Issue Context
Downstream logic (e.g., add-fbc-contribution) consumes `indexImageDigests` to populate result payloads. If digests are missing, the task should fail consistently (non-zero exit + failed buildState), not succeed.

### Fix Focus Areas
- When `indexImageDigests` is empty:
 - write a failed `buildState` (or at least keep consistency with exitCode)
 - set a non-zero exit status (e.g., set `BUILDEXIT=1` and/or `exit 1` immediately)
 - ensure `$(results.indexImageDigests.path)` is set appropriately (likely empty) and the TaskRun fails.

#### Code locations
- tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[552-571]
- tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[589-589]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Unquoted SCRIPT_DIR substitution 📘 Rule violation ≡ Correctness
Description
SCRIPT_DIR=$(...) is not quoted and $SCRIPT_DIR is expanded without brace syntax when building
the yq expression. This can break on whitespace/special characters and violates the quoting/brace
requirements.
Code

tasks/internal/update-fbc-catalog-task/tests/pre-apply-task-hook.sh[R23-25]

+SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
+yq -i '.spec.steps[0].script = load_str("'$SCRIPT_DIR'/mocks.sh") + .spec.steps[0].script' "$TASK_PATH"
+yq -i '.spec.steps[1].script = load_str("'$SCRIPT_DIR'/mocks.sh") + .spec.steps[1].script' "$TASK_PATH"
Relevance

⭐⭐ Medium

Quoting/brace-style feedback exists but often rejected or inconclusive across tasks/tests.

PR-#1861
PR-#2118

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 1263 requires quoting command substitutions, and rule 1264 requires brace syntax for variable
expansion; the added lines use SCRIPT_DIR=$(...) and embed $SCRIPT_DIR without braces in the
yq expression.

Rule 1263: Always quote shell variables and command substitutions
Rule 1264: Use brace syntax for shell variable expansion
tasks/internal/update-fbc-catalog-task/tests/pre-apply-task-hook.sh[23-25]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The added `SCRIPT_DIR` assignment is unquoted and later expanded without `${...}` braces.

## Issue Context
Checklist requires quoting variables/command substitutions and using brace syntax for variable expansion.

## Fix Focus Areas
- tasks/internal/update-fbc-catalog-task/tests/pre-apply-task-hook.sh[23-25]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View more (6)
4. mocks.sh missing strict flags 📘 Rule violation ☼ Reliability
Description
The new standalone script mocks.sh enables xtrace (set -x) but does not enable strict error
handling (set -euo pipefail). This can hide failures and make test behavior non-deterministic.
Code

tasks/internal/update-fbc-catalog-task/tests/mocks.sh[R1-4]

+#!/usr/bin/env bash
+set -x
+
+echo "CLAUDE_DEBUGGING: mocks.sh file loaded successfully with fix attempt $(date)" >&2
Relevance

⭐⭐ Medium

Strict-mode additions in task/test scripts appear in reviews but outcomes are inconsistent/mostly
undetermined.

PR-#2213
PR-#1505

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 1261 requires strict shell flags in standalone scripts; mocks.sh has a bash shebang and uses
set -x but omits set -euo pipefail.

Rule 1261: Shell scripts must enable strict error handling flags
tasks/internal/update-fbc-catalog-task/tests/mocks.sh[1-4]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
New standalone bash script lacks strict error handling flags.

## Issue Context
Checklist requires `set -euo pipefail` near the top of standalone scripts.

## Fix Focus Areas
- tasks/internal/update-fbc-catalog-task/tests/mocks.sh[1-4]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. Tekton scripts missing pipefail 📘 Rule violation ☼ Reliability
Description
Tekton step scripts in update-fbc-catalog-task were added/updated without set -eo pipefail (or
stronger like set -euo pipefail) as the first executable line, which reduces safety by allowing
steps to continue after failures. As a result, failures in commands like base64/kinit/skopeo/date/jq
can be ignored and the task may proceed with invalid state or incorrect build reuse decisions.
Code

tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[R120-125]

+      script: |
+        #!/usr/bin/env bash
+
+        IIB_SERVICE_URL="$(cat /mnt/iib-services-config/url)"
+        KRB5_CONF_CONTENT="$(cat /mnt/iib-services-config/krb5.conf)"
+
Relevance

⭐⭐ Medium

Repo discusses adding pipefail/strict mode in Tekton scripts, but some reviewers caution against
behavior changes.

PR-#1505
PR-#891

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 1268 requires Tekton Task script: blocks to begin with set -eo pipefail (or stronger) as
the first non-comment, non-empty executable line, but the cited script: blocks start by running
commands (or enabling set -x) without first turning on -e and pipefail. The scripts then
invoke tools such as base64, kinit, skopeo, date, and jq without fail-fast semantics, meaning errors
from these commands can be silently ignored; this contrasts with other tasks in the repository that
consistently start with set -euo pipefail to ensure TaskRuns fail immediately on command failures.

Rule 1268: Shell scripts in Tekton tasks must start with set -eo pipefail
tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[120-127]
tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[436-444]
tasks/internal/update-fbc-catalog-task/tests/test-update-fbc-catalog-error.yaml[59-62]
tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[120-333]
tasks/managed/extract-py-artifacts/extract-py-artifacts.yaml[124-141]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Tekton `script:` blocks in `update-fbc-catalog-task` do not enable fail-fast behavior (`set -eo pipefail` or stronger like `set -euo pipefail`) as the first executable line, which can allow the task to continue after failures and produce incorrect results (e.g., invalid state or incorrect build reuse decisions).

## Issue Context
A checklist/rule (Rule 1268) requires that the first non-comment, non-empty line in Tekton shell scripts include `set -eo pipefail` (or stronger). The affected step scripts currently start executing commands (and/or `set -x`) before enabling `-e`/`pipefail`, even though other tasks in this repo commonly start scripts with `set -euo pipefail` to ensure command failures fail the TaskRun. These scripts rely on commands such as `base64`, `kinit`, `skopeo`, `date`, and `jq`, so missing fail-fast can let authentication/inspection/conversion failures be ignored.

## Fix Focus Areas
- tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[120-334]
- tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[436-549]
- tasks/internal/update-fbc-catalog-task/tests/test-update-fbc-catalog-error.yaml[59-62]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


6. mktemp not cleaned up 📘 Rule violation ☼ Reliability
Description
The Tekton task script creates a temp file via mktemp but does not register an EXIT trap to remove
it. This can leak temporary files into the workspace/container filesystem and complicate debugging
and reuse.
Code

tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[R322-323]

+        KRB5_TEMP_CONF=$(mktemp)
+        KRB5_PRINCIPAL=$(cat /mnt/service-account-secret/principal)
Relevance

⭐⭐ Medium

mktemp cleanup via EXIT trap has been suggested repeatedly but not clearly accepted as a standard.

PR-#2127
PR-#2036

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 1267 requires an EXIT trap that removes mktemp-created paths; the script sets
KRB5_TEMP_CONF=$(mktemp) but no cleanup trap is present nearby or later in the script.

Rule 1267: Clean up mktemp-created temporary files with a trap on EXIT
tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[319-327]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`mktemp` is used without an EXIT trap cleanup.

## Issue Context
Checklist requires mktemp-created temp paths to be removed via a `trap ... EXIT` handler.

## Fix Focus Areas
- tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[319-327]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


7. IIB curl missing retries 📘 Rule violation ☼ Reliability
Description
The Tekton task uses operational curl calls to IIB without --retry 3 and without
--fail-with-body. This increases flakiness on transient failures and can mask HTTP errors as
successful execution.
Code

tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[R411-413]

+        iib_response=$(curl -u : --negotiate -s -X POST \
+          -H "Content-Type: application/json" -d@"${json_input}" --insecure \
+          "${IIB_SERVICE_URL}/builds/fbc-operations")
Relevance

⭐⭐ Medium

Mixed history: curl hardening with --fail-with-body was rejected in PR #2236; retry usage varies.

PR-#2236

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 1253 requires --retry 3 and rule 1254 requires --fail-with-body for operational curl usage;
the added IIB calls use curl -s ... (and curl ... -s -X POST) without those flags.

Rule 1253: Include curl retry flag
Rule 1254: Include --fail-with-body in curl commands for robust error handling
tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[146-148]
tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[411-413]
tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[459-463]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Operational `curl` calls in the task lack required robustness flags (`--retry 3` and `--fail-with-body`).

## Issue Context
Checklist requires retry handling and fail-with-body for curl commands expected to succeed.

## Fix Focus Areas
- tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[146-153]
- tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[411-413]
- tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[459-463]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


8. JSON built via heredoc 📘 Rule violation ≡ Correctness
Description
The task constructs a JSON payload using a heredoc with shell interpolation rather than building
JSON via jq arguments (--arg/--argjson). This can lead to invalid JSON or injection bugs if any
interpolated values contain quotes or special characters.
Code

tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[R392-401]

+        cat > "$json_raw_input" <<JSON
+        {
+          "fbc_fragments": ${sorted_fbc_fragments},
+          "from_index": "${FROM_INDEX}",
+          "build_tags": $(params.buildTags),
+          "add_arches": $(params.addArches),
+          "overwrite_from_index": ${mustOverwriteFromIndexImage},
+          "overwrite_from_index_token": "${IIB_OVERWRITE_FROM_INDEX_USERNAME}:${IIB_OVERWRITE_FROM_INDEX_TOKEN}"
+        }
+        JSON
Relevance

⭐⭐ Medium

Historical feedback suggests using jq to build JSON instead of heredocs, but acceptance evidence is
unclear.

PR-#2118

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 1271 requires using jq --arg/--argjson (or jq -n) to construct JSON from shell variables;
the payload is created using cat <<JSON ... with direct interpolations like
${sorted_fbc_fragments} and ${FROM_INDEX}.

Rule 1271: Construct JSON with jq arguments instead of string concatenation
tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[392-401]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
JSON request payload is built via a heredoc with interpolated shell variables.

## Issue Context
Checklist requires constructing JSON with `jq -n` and `--arg/--argjson` instead of string concatenation/interpolation.

## Fix Focus Areas
- tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[392-401]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


9. Tekton script exits non-zero 📘 Rule violation ≡ Correctness
Description
The Tekton task scripts use exit 1/propagate non-zero exit codes for business logic and failures.
This violates the requirement to always exit 0 and instead communicate status via Tekton results.
Code

tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[R335-352]

+        # Validate that required environment variables are set
+        if [ -z "${FBC_FRAGMENTS:-}" ]; then
+            echo "Error: FBC_FRAGMENTS environment variable is required"
+            exit 1
+        fi
+        if [ -z "${FROM_INDEX:-}" ]; then
+            echo "Error: FROM_INDEX environment variable is required"
+            exit 1
+        fi
+
+        # validate fbcFragments parameter is valid JSON array
+        if ! echo "${FBC_FRAGMENTS}" | jq -e . >/dev/null 2>&1; then
+          echo "Error: fbcFragments parameter must be a valid JSON array"
+          jq -n '{ "state": "failed", "state_reason": "Invalid fbcFragments parameter" }' \
+            | tee "$(results.buildState.path)"
+          echo -n 1 > "$(results.exitCode.path)"
+          exit 1
+        fi
Relevance

⭐⭐ Medium

Non-zero exits in Tekton scripts flagged in prior reviews, but no clear accepted precedent enforcing
“always exit 0”.

PR-#2191
PR-#2213

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 1256 forbids non-zero exits for business logic in Tekton scripts; the added script explicitly
uses exit 1 on validation failures and propagates failures via exit "${BUILDEXIT}".

Rule 1256: Tekton task scripts must always exit 0 and use results for status
tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[335-352]
tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[587-589]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Task scripts exit non-zero (`exit 1`) instead of always exiting 0 and using Tekton results to signal failure.

## Issue Context
Checklist requires writing status to `$(results.*.path)` and ending the script with `exit 0` regardless of logical success/failure.

## Fix Focus Areas
- tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[335-360]
- tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[572-589]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

10. taskGit params ignored 🐞 Bug ⚙ Maintainability
Description
slan-cuan-release defines taskGitUrl/taskGitRevision params but extract/sign/publish taskRefs
hardcode the slan-cuan repo URL and revision, so overriding these params has no effect and the
pipeline cannot be reliably pinned/configured.
Code

pipelines/managed/slan-cuan-release/slan-cuan-release.yaml[R311-320]

+    - name: extract
+      taskRef:
+        resolver: "git"
+        params:
+          - name: url
+            value: https://github.com/konflux-lightwell/slan-cuan.git
+          - name: revision
+            value: main
+          - name: pathInRepo
+            value: tekton/tasks/slan-cuan-extract.yaml
Relevance

⭐⭐ Medium

Mixed history: reviewers sometimes reject “unused param” cleanup; no clear prior on hardcoded
git-resolver vs params.

PR-#2117
PR-#2271

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The pipeline declares taskGitUrl/taskGitRevision parameters but the taskRefs for the slan-cuan tasks
use literal values instead of these params; the accompanying README documents different defaults,
reinforcing that these are intended to be configurable.

pipelines/managed/slan-cuan-release/slan-cuan-release.yaml[77-87]
pipelines/managed/slan-cuan-release/slan-cuan-release.yaml[311-432]
pipelines/managed/slan-cuan-release/README.md[26-29]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The pipeline exposes `taskGitUrl`/`taskGitRevision` parameters, but the tasks that should use them (`extract`, `sign`, `publish`) are hardcoded to `https://github.com/konflux-lightwell/slan-cuan.git` and `main`. This makes the parameters effectively dead and reduces reproducibility.

### Issue Context
The README also documents defaults for these params, indicating they are intended to be configurable/pinnable.

### Fix Focus Areas
- Replace hardcoded `taskRef.resolver.git.params.url` and `revision` for `extract`, `sign`, and `publish` with `$(params.taskGitUrl)` and `$(params.taskGitRevision)`.
- (Optional) Align README defaults with the actual Pipeline defaults, or update Pipeline defaults to match the intended pinned revisions.

#### Code locations
- pipelines/managed/slan-cuan-release/slan-cuan-release.yaml[77-87]
- pipelines/managed/slan-cuan-release/slan-cuan-release.yaml[311-432]
- pipelines/managed/slan-cuan-release/README.md[26-29]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

11. Added pipelines README.md 📘 Rule violation ⚙ Maintainability
Description
pipelines/managed/slan-cuan-release/README.md was added/changed in this PR, but READMEs under
pipelines/ are auto-generated and must not be edited directly. This can cause generator drift and
future regenerations to overwrite manual changes.
Code

pipelines/managed/slan-cuan-release/README.md[R1-42]

+# slan-cuan-release pipeline
+
+Release pipeline for Lightwell Java artifacts (slan-cuan).
+
+This pipeline orchestrates the complete release workflow for Java artifacts built by
+PNC (Project Newcastle). All per-release configuration (image reference, signing key,
+Trustify URLs, Pulp target) is derived from the Snapshot and ReleasePlanAdmission data
+via collect-data, so no pipeline parameters need to be supplied at invocation time.
+
+## Parameters
+
+| Name                    | Description                                                                                                                                                                     | Optional | Default value                                        |
+|-------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------|------------------------------------------------------|
+| release                 | The namespaced name (namespace/name) of the Release                                                                                                                             | No       | -                                                    |
+| releasePlan             | The namespaced name (namespace/name) of the ReleasePlan                                                                                                                         | No       | -                                                    |
+| releasePlanAdmission    | The namespaced name (namespace/name) of the ReleasePlanAdmission                                                                                                                | No       | -                                                    |
+| releaseServiceConfig    | The namespaced name (namespace/name) of the ReleaseServiceConfig                                                                                                                | No       | -                                                    |
+| snapshot                | The namespaced name (namespace/name) of the Snapshot                                                                                                                            | No       | -                                                    |
+| ociStorage              | The OCI repository where Trusted Artifacts are stored                                                                                                                           | Yes      | empty                                                |
+| ociArtifactExpiresAfter | Expiration date for trusted artifacts created in the OCI repository                                                                                                             | Yes      | 1d                                                   |
+| trustedArtifactsDebug   | Flag to enable debug logging in trusted artifacts                                                                                                                               | Yes      | ""                                                   |
+| orasOptions             | oras options to pass to Trusted Artifacts calls                                                                                                                                 | Yes      | ""                                                   |
+| dataDir                 | The location where data will be stored                                                                                                                                          | Yes      | /var/workdir/release                                 |
+| caTrustConfigMapName    | The name of the ConfigMap to read CA bundle data from                                                                                                                           | Yes      | trusted-ca                                           |
+| caTrustConfigMapKey     | The name of the key in the ConfigMap that contains the CA bundle data                                                                                                           | Yes      | ca-bundle.crt                                        |
+| catalogGitUrl           | The url to the git repo where release-service-catalog tasks are stored                                                                                                          | Yes      | https://github.com/arewm/release-service-catalog.git |
+| catalogGitRevision      | The revision in the catalogGitUrl repo to be used                                                                                                                               | Yes      | lightwell-pipeline                                   |
+| taskGitUrl              | The url to the git repo where the slan-cuan tasks are stored                                                                                                                    | Yes      | https://github.com/arewm/slan-cuan.git               |
+| taskGitRevision         | The revision in the taskGitUrl repo to be used                                                                                                                                  | Yes      | 509fb97bb20fd8c3c021c6965e6f000134d396e5             |
+| registry-auth-secret    | Kubernetes Secret name for registry authentication (.dockerconfigjson format). Points to a Docker/Podman auth config for accessing private registries                           | Yes      | registry-auth                                        |
+| force-extract           | Overwrite existing output directory if it exists. Without this flag, the extract task refuses to overwrite existing directories                                                 | Yes      | false                                                |
+| radas-config-secret     | Kubernetes Secret name containing RADAS configuration JSON. The secret must have a `config.json` key with RADAS API URL and credentials                                         | Yes      | radas-config                                         |
+| requester-id            | Requester identity for signing operations. Used for audit trails and RADAS access control. Typically an email address                                                           | Yes      | slan-cuan@org.com                                    |
+| zip-root-path           | Root of the Maven repository tree inside the ZIP archive submitted to RADAS. The ZIP file structure is <ZIP_ROOT_PATH>/<maven-layout>                                           | Yes      | repository                                           |
+| product-key             | Product key for metadata tagging. Identifies the product in RADAS records and signing logs                                                                                      | Yes      | slan-cuan                                            |
+| ignore-patterns         | Comma-separated regex patterns to exclude files from signing. Example: ".*-sources\\.jar$,.*-javadoc\\.jar$" excludes source and javadoc JARs                                   | Yes      | ""                                                   |
+| sso-secret-name         | Kubernetes Secret name with OIDC credentials. The secret must have `client-id` and `client-secret` keys for OAuth2 client credentials flow                                      | Yes      | trustify-sso                                         |
+| register-insecure       | Disable TLS verification for Trustify API calls. Set to "true" to skip certificate validation (not recommended for production)                                                  | Yes      | false                                                |
+| register-retries        | Number of retry attempts for Trustify API calls. The task will retry failed API calls this many times before giving up                                                          | Yes      | 3                                                    |
+| register-ca-cert-secret | Kubernetes Secret name for custom CA certificate (optional). The secret must have a `ca.crt` key containing the PEM-encoded CA certificate. Leave empty to use system CA bundle | Yes      | ""                                                   |
+| publish-insecure        | Disable TLS verification for Pulp API calls. Set to "true" to skip certificate validation (not recommended for production)                                                      | Yes      | false                                                |
+| publish-ca-cert-secret  | Kubernetes Secret name for custom CA certificate (optional). The secret must have a `ca.crt` key containing the PEM-encoded CA certificate. Leave empty to use system CA bundle | Yes      | ""                                                   |
Relevance

⭐ Low

Repo frequently updates pipeline READMEs; reviewers even request regenerating them to stay in sync.

PR-#2303

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The checklist explicitly forbids editing auto-generated README files under pipelines/, and this PR
adds pipelines/managed/slan-cuan-release/README.md.

Rule 1260: Do not manually edit auto-generated README files under tasks/ and pipelines/
pipelines/managed/slan-cuan-release/README.md[1-42]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A README under `pipelines/` was modified/added, but these files are auto-generated and must not be edited directly.

## Issue Context
Compliance requires that `pipelines/**/README.md` not be edited in PRs.

## Fix Focus Areas
- pipelines/managed/slan-cuan-release/README.md[1-42]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


12. curl missing --fail-with-body 📘 Rule violation ☼ Reliability
Description
The new Jira webhook curl call does not include --fail-with-body, so HTTP 4xx/5xx may not be
treated as failures. This can allow the workflow to continue while silently failing to update Jira
tickets.
Code

.github/workflows/promote_branch.yaml[R226-233]

+          set +e
+          HTTP_CODE=$(curl -s -o /dev/null -w "%{http_code}" \
+            --retry 3 --max-time 30 \
+            -X POST "${JIRA_WEBHOOK_URL}" \
+            -H "Content-Type: application/json" \
+            -H "X-Automation-Webhook-Token: ${JIRA_WEBHOOK_TOKEN}" \
+            -d "${PAYLOAD}")
+          CURL_RC=$?
Relevance

⭐ Low

Similar --fail-with-body curl hardening suggestion was rejected in push-rpms-to-pulp review
discussions.

PR-#2236

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 1254 requires --fail-with-body on non-trivial curl usage; the added webhook call uses `curl
-s ... --retry 3 but omits --fail-with-body`.

Rule 1254: Include --fail-with-body in curl commands for robust error handling
.github/workflows/promote_branch.yaml[226-233]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The Jira webhook `curl` invocation is missing `--fail-with-body`, reducing robustness on HTTP errors.

## Issue Context
Checklist requires `--fail-with-body` for operational curl usage.

## Fix Focus Areas
- .github/workflows/promote_branch.yaml[226-233]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


13. pre-apply-task-hook.sh uses cd 📘 Rule violation ⚙ Maintainability
Description
The added SCRIPT_DIR computation uses cd for a temporary directory change instead of
pushd/popd (or an alternative that avoids directory switching). This violates the shell
directory-change requirement.
Code

tasks/internal/update-fbc-catalog-task/tests/pre-apply-task-hook.sh[23]

+SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
Relevance

⭐ Low

Common pattern uses cd-in-subshell for SCRIPT_DIR; no evidence this team enforces pushd/popd here.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 1269 requires pushd/popd instead of temporary cd; the new SCRIPT_DIR assignment uses cd
inside command substitution.

Rule 1269: Use pushd/popd instead of cd in shell scripts
tasks/internal/update-fbc-catalog-task/tests/pre-apply-task-hook.sh[23-23]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The script uses `cd` for a temporary operation; the checklist requires using `pushd/popd` (or avoiding directory changes).

## Issue Context
This was added while computing `SCRIPT_DIR`.

## Fix Focus Areas
- tasks/internal/update-fbc-catalog-task/tests/pre-apply-task-hook.sh[23-23]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment on lines +23 to +25
SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
yq -i '.spec.steps[0].script = load_str("'$SCRIPT_DIR'/mocks.sh") + .spec.steps[0].script' "$TASK_PATH"
yq -i '.spec.steps[1].script = load_str("'$SCRIPT_DIR'/mocks.sh") + .spec.steps[1].script' "$TASK_PATH"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

4. Unquoted script_dir substitution 📘 Rule violation ≡ Correctness

SCRIPT_DIR=$(...) is not quoted and $SCRIPT_DIR is expanded without brace syntax when building
the yq expression. This can break on whitespace/special characters and violates the quoting/brace
requirements.
Agent Prompt
## Issue description
The added `SCRIPT_DIR` assignment is unquoted and later expanded without `${...}` braces.

## Issue Context
Checklist requires quoting variables/command substitutions and using brace syntax for variable expansion.

## Fix Focus Areas
- tasks/internal/update-fbc-catalog-task/tests/pre-apply-task-hook.sh[23-25]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +1 to +4
#!/usr/bin/env bash
set -x

echo "CLAUDE_DEBUGGING: mocks.sh file loaded successfully with fix attempt $(date)" >&2

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

5. mocks.sh missing strict flags 📘 Rule violation ☼ Reliability

The new standalone script mocks.sh enables xtrace (set -x) but does not enable strict error
handling (set -euo pipefail). This can hide failures and make test behavior non-deterministic.
Agent Prompt
## Issue description
New standalone bash script lacks strict error handling flags.

## Issue Context
Checklist requires `set -euo pipefail` near the top of standalone scripts.

## Fix Focus Areas
- tasks/internal/update-fbc-catalog-task/tests/mocks.sh[1-4]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +120 to +125
script: |
#!/usr/bin/env bash

IIB_SERVICE_URL="$(cat /mnt/iib-services-config/url)"
KRB5_CONF_CONTENT="$(cat /mnt/iib-services-config/krb5.conf)"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

6. Tekton scripts missing pipefail 📘 Rule violation ☼ Reliability

Tekton step scripts in update-fbc-catalog-task were added/updated without set -eo pipefail (or
stronger like set -euo pipefail) as the first executable line, which reduces safety by allowing
steps to continue after failures. As a result, failures in commands like base64/kinit/skopeo/date/jq
can be ignored and the task may proceed with invalid state or incorrect build reuse decisions.
Agent Prompt
## Issue description
Tekton `script:` blocks in `update-fbc-catalog-task` do not enable fail-fast behavior (`set -eo pipefail` or stronger like `set -euo pipefail`) as the first executable line, which can allow the task to continue after failures and produce incorrect results (e.g., invalid state or incorrect build reuse decisions).

## Issue Context
A checklist/rule (Rule 1268) requires that the first non-comment, non-empty line in Tekton shell scripts include `set -eo pipefail` (or stronger). The affected step scripts currently start executing commands (and/or `set -x`) before enabling `-e`/`pipefail`, even though other tasks in this repo commonly start scripts with `set -euo pipefail` to ensure command failures fail the TaskRun. These scripts rely on commands such as `base64`, `kinit`, `skopeo`, `date`, and `jq`, so missing fail-fast can let authentication/inspection/conversion failures be ignored.

## Fix Focus Areas
- tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[120-334]
- tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[436-549]
- tasks/internal/update-fbc-catalog-task/tests/test-update-fbc-catalog-error.yaml[59-62]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +322 to +323
KRB5_TEMP_CONF=$(mktemp)
KRB5_PRINCIPAL=$(cat /mnt/service-account-secret/principal)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

7. mktemp not cleaned up 📘 Rule violation ☼ Reliability

The Tekton task script creates a temp file via mktemp but does not register an EXIT trap to remove
it. This can leak temporary files into the workspace/container filesystem and complicate debugging
and reuse.
Agent Prompt
## Issue description
`mktemp` is used without an EXIT trap cleanup.

## Issue Context
Checklist requires mktemp-created temp paths to be removed via a `trap ... EXIT` handler.

## Fix Focus Areas
- tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[319-327]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +411 to +413
iib_response=$(curl -u : --negotiate -s -X POST \
-H "Content-Type: application/json" -d@"${json_input}" --insecure \
"${IIB_SERVICE_URL}/builds/fbc-operations")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

8. Iib curl missing retries 📘 Rule violation ☼ Reliability

The Tekton task uses operational curl calls to IIB without --retry 3 and without
--fail-with-body. This increases flakiness on transient failures and can mask HTTP errors as
successful execution.
Agent Prompt
## Issue description
Operational `curl` calls in the task lack required robustness flags (`--retry 3` and `--fail-with-body`).

## Issue Context
Checklist requires retry handling and fail-with-body for curl commands expected to succeed.

## Fix Focus Areas
- tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[146-153]
- tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[411-413]
- tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[459-463]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +392 to +401
cat > "$json_raw_input" <<JSON
{
"fbc_fragments": ${sorted_fbc_fragments},
"from_index": "${FROM_INDEX}",
"build_tags": $(params.buildTags),
"add_arches": $(params.addArches),
"overwrite_from_index": ${mustOverwriteFromIndexImage},
"overwrite_from_index_token": "${IIB_OVERWRITE_FROM_INDEX_USERNAME}:${IIB_OVERWRITE_FROM_INDEX_TOKEN}"
}
JSON

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

9. Json built via heredoc 📘 Rule violation ≡ Correctness

The task constructs a JSON payload using a heredoc with shell interpolation rather than building
JSON via jq arguments (--arg/--argjson). This can lead to invalid JSON or injection bugs if any
interpolated values contain quotes or special characters.
Agent Prompt
## Issue description
JSON request payload is built via a heredoc with interpolated shell variables.

## Issue Context
Checklist requires constructing JSON with `jq -n` and `--arg/--argjson` instead of string concatenation/interpolation.

## Fix Focus Areas
- tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[392-401]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +335 to +352
# Validate that required environment variables are set
if [ -z "${FBC_FRAGMENTS:-}" ]; then
echo "Error: FBC_FRAGMENTS environment variable is required"
exit 1
fi
if [ -z "${FROM_INDEX:-}" ]; then
echo "Error: FROM_INDEX environment variable is required"
exit 1
fi

# validate fbcFragments parameter is valid JSON array
if ! echo "${FBC_FRAGMENTS}" | jq -e . >/dev/null 2>&1; then
echo "Error: fbcFragments parameter must be a valid JSON array"
jq -n '{ "state": "failed", "state_reason": "Invalid fbcFragments parameter" }' \
| tee "$(results.buildState.path)"
echo -n 1 > "$(results.exitCode.path)"
exit 1
fi

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

10. Tekton script exits non-zero 📘 Rule violation ≡ Correctness

The Tekton task scripts use exit 1/propagate non-zero exit codes for business logic and failures.
This violates the requirement to always exit 0 and instead communicate status via Tekton results.
Agent Prompt
## Issue description
Task scripts exit non-zero (`exit 1`) instead of always exiting 0 and using Tekton results to signal failure.

## Issue Context
Checklist requires writing status to `$(results.*.path)` and ending the script with `exit 0` regardless of logical success/failure.

## Fix Focus Areas
- tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[335-360]
- tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[572-589]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +495 to +496
url="$(jq -r ".logs.url" <<< "${build_info}")"
echo IIB log url is: "${url}" > "$(results.iibLog.path)"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

11. Result written with echo 📘 Rule violation ≡ Correctness

The task writes the iibLog Tekton result using echo without -n, which appends a trailing
newline into the result value. This can cause downstream parsing/validation issues when consumers
expect an exact URL string.
Agent Prompt
## Issue description
A Tekton result file is written using `echo` without `-n`, introducing a trailing newline.

## Issue Context
Checklist requires `echo -n` (or `printf %s`) when writing Tekton results.

## Fix Focus Areas
- tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[495-496]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +552 to +589
if [ ${BUILDEXIT} -eq 0 ]; then
echo -n 0 > "$(results.exitCode.path)"

# get the manifest digests
indexImageCopy=$(base64 -d < "$(results.jsonBuildInfo.path)" | gunzip | \
jq -cr .internal_index_image_copy)
# Use this to obtain the manifest digests for each arch in manifest list
indexImageDigestsRaw=$(skopeo inspect --retry-times 3 --raw "docker://${indexImageCopy}")
# according the IIB team,
# "all index images will always be multi-arch with a manifest list"
#
indexImageDigests=$(echo "${indexImageDigestsRaw}" | \
jq -r \
'.manifests[]? | select(.mediaType=="application/vnd.docker.distribution.manifest.v2+json") | .digest' \
| tr '\n' ' ' | sed 's/ $//') # make sure the result is on one line and remove trailing space
echo -n "${indexImageDigests}" > "$(results.indexImageDigests.path)"
if [ -z "${indexImageDigests}" ] ; then
echo "Index image produced is not multi-arch with a manifest list"
echo -n 1 > "$(results.exitCode.path)"
fi
else
if [ ${BUILDEXIT} -eq 124 ]; then
echo "Timeout while waiting for the build to finish"
jq -n '{ "state": "failed", "state_reason": "Build timeout" }' | tee "$(results.buildState.path)"
else
echo "Build failed with exit code ${BUILDEXIT}"
jq -n --arg exit_code "$BUILDEXIT" \
'{ "state": "failed", "state_reason": ("Build failed with exit code " + $exit_code) }' \
| tee "$(results.buildState.path)"
fi
echo -n "" > "$(results.indexImageDigests.path)"
echo -n "$BUILDEXIT" > "$(results.exitCode.path)"
fi
# We don't put the log in a result because tekton results are too limited for what we can put
# to be useful, but still print it for debugging
curl -s "$(awk '{print $NF}' < "$(results.iibLog.path)")"

exit "${BUILDEXIT}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

12. Empty digests still succeed 🐞 Bug ≡ Correctness

In update-fbc-catalog-task’s wait step, when indexImageDigests is empty it writes exitCode=1 to
results but still exits with 0 (BUILDEXIT), so Tekton reports the TaskRun as successful while
emitting failure-like results.
Agent Prompt
### Issue description
The wait step detects an empty `indexImageDigests` result, but only updates `$(results.exitCode.path)` and does not fail the step process (it still `exit "${BUILDEXIT}"` where `BUILDEXIT` is 0). This creates an inconsistent state: TaskRun success with error results.

### Issue Context
Downstream logic (e.g., add-fbc-contribution) consumes `indexImageDigests` to populate result payloads. If digests are missing, the task should fail consistently (non-zero exit + failed buildState), not succeed.

### Fix Focus Areas
- When `indexImageDigests` is empty:
  - write a failed `buildState` (or at least keep consistency with exitCode)
  - set a non-zero exit status (e.g., set `BUILDEXIT=1` and/or `exit 1` immediately)
  - ensure `$(results.indexImageDigests.path)` is set appropriately (likely empty) and the TaskRun fails.

#### Code locations
- tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[552-571]
- tasks/internal/update-fbc-catalog-task/update-fbc-catalog-task.yaml[589-589]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants