Skip to content

Update shared CI#3722

Merged
chmeliik merged 1 commit into
konflux-ci:mainfrom
chmeliik:update-shared-ci
Jul 22, 2026
Merged

Update shared CI#3722
chmeliik merged 1 commit into
konflux-ci:mainfrom
chmeliik:update-shared-ci

Conversation

@chmeliik

Copy link
Copy Markdown
Contributor

Done with: uvx cruft update --skip-apply-ask --allow-untracked-files

And manually resolved rejections in files that we edited separately from upstream.

@chmeliik
chmeliik requested a review from a team as a code owner July 22, 2026 11:15
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 22, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 11:16 AM UTC · Completed 11:37 AM UTC
Commit: 37b10e4 · View workflow run →

@qodo-app-for-konflux-ci

Copy link
Copy Markdown

PR Summary by Qodo

Refresh shared CI cruft template and update GitHub Actions pins

⚙️ Configuration changes 🐞 Bug fix 🕐 20-40 Minutes

Grey Divider

AI Description

• Update the cruft template reference to the latest shared CI commit.
• Bump pinned GitHub Actions and related CI tooling versions across workflows.
• Fix/strengthen CI helper scripts for Tekton task testing and migration validation.
Diagram

graph TD
T{{"Shared CI template"}} --> C[".cruft.json"] --> W["GitHub workflows"]
W --> A{{"Pinned GitHub Actions"}}
W --> S([".github/scripts/*"] ) --> H(["hack/* helpers"])
subgraph Legend
direction LR
_tpl{{"Template/Action"}} ~~~ _file["Workflow/File"] ~~~ _sh(["Shell script"])
end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Automate action pin bumps via Renovate/Dependabot
  • ➕ Reduces the need for template-driven/manual pin refreshes
  • ➕ Provides continuous updates with PR-by-PR changelogs and alerts
  • ➖ May conflict with the shared CI template’s authoritative pins
  • ➖ Adds extra automation/configuration overhead and potential PR noise
2. Centralize pins via reusable workflow(s)
  • ➕ Single place to update action pins; fewer per-repo diffs
  • ➕ Encourages consistent CI behavior across repositories
  • ➖ Requires broader refactor and governance of shared workflows
  • ➖ Harder for repo-specific customization without additional inputs/abstractions

Recommendation: Given this repo already uses a shared cruft template, updating via cruft is the most consistent approach. Consider adding Renovate/Dependabot only if the organization wants faster action pin cadence than template updates, otherwise keep the template as the single source of truth.

Files changed (12) +29 / -37

Bug fix (3) +8 / -6
test_tekton_tasks.shImprove help/empty-input handling for Tekton task tests +1/-1

Improve help/empty-input handling for Tekton task tests

• Adjusts argument parsing so the help path only triggers on explicit -h, and treats the absence of both CLI args and TEST_ITEMS as an error. This makes behavior consistent for local runs vs CI-driven env configuration.

.github/scripts/test_tekton_tasks.sh

generate-ta-tasks.shFix temp dir creation/cleanup and task path computation +4/-3

Fix temp dir creation/cleanup and task path computation

• Uses mktemp -d with a robust EXIT trap to ensure the temp directory exists and is always removed. Also fixes task naming/path generation derived from recipe.yaml locations, making output paths more predictable.

hack/generate-ta-tasks.sh

validate-migration.shPass migration file into cluster validation and emit CI notice +3/-2

Pass migration file into cluster validation and emit CI notice

• Fixes a missing parameter by passing the current migration file into check_apply_in_real_cluster. Replaces an informational log with a GitHub Actions ::notice annotation that points at the migration file when in-cluster validation is skipped due to no modified pipeline files.

hack/validate-migration.sh

Other (9) +21 / -31
.cruft.jsonBump shared CI template commit +2/-2

Bump shared CI template commit

• Updates the tracked cruft template commit hash to a newer konflux-ci/task-repo-shared-ci revision so regenerated CI files match upstream.

.cruft.json

check-kustomize-build.yamlUpdate checkout action pin +1/-1

Update checkout action pin

• Bumps actions/checkout to a newer pinned digest/version for the kustomize build check workflow.

.github/workflows/check-kustomize-build.yaml

check-ta.yamlUpdate checkout and artifact upload pins +2/-2

Update checkout and artifact upload pins

• Bumps actions/checkout and actions/upload-artifact pins to newer versions for the Trusted Artifact variants check workflow.

.github/workflows/check-ta.yaml

check-task-migration.yamlRefresh Kind/Tekton setup and remove flaky retry guidance +4/-14

Refresh Kind/Tekton setup and remove flaky retry guidance

• Updates action pins (checkout, kind-action) and pins tektoncd/actions/setup-tektoncd to a specific commit. Also switches from 'latest' pipeline version to an explicit Tekton pipeline version (v1.14.0), removing the prior continue-on-error + rate-limit workaround block.

.github/workflows/check-task-migration.yaml

checkton.yamlUpdate checkout action pin +1/-1

Update checkout action pin

• Bumps actions/checkout to a newer pinned digest/version for the Checkton workflow while keeping full-history checkout behavior.

.github/workflows/checkton.yaml

run-task-tests.yamlUpdate pins for checkout/changed-files/kind and konflux-ci ref +5/-5

Update pins for checkout/changed-files/kind and konflux-ci ref

• Bumps several pinned actions (actions/checkout, tj-actions/changed-files, helm/kind-action) and updates the referenced konflux-ci/konflux-ci commit used during task tests.

.github/workflows/run-task-tests.yaml

task-lint.yamlUpdate checkout and yq action pins +2/-2

Update checkout and yq action pins

• Bumps actions/checkout and mikefarah/yq pins to newer versions for the task lint workflow.

.github/workflows/task-lint.yaml

update-shared-ci.yamlUpdate pins for shared CI auto-update workflow +3/-3

Update pins for shared CI auto-update workflow

• Bumps actions/checkout, actions/create-github-app-token, and peter-evans/create-pull-request pins to newer versions for the workflow that opens shared CI update PRs.

.github/workflows/update-shared-ci.yaml

versioning.yamlUpdate checkout action pin +1/-1

Update checkout action pin

• Bumps actions/checkout to a newer pinned digest/version for the versioning workflow.

.github/workflows/versioning.yaml

@qodo-app-for-konflux-ci

qodo-app-for-konflux-ci Bot commented Jul 22, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 9 rules

Grey Divider


Action required

1. Whitespace TEST_ITEMS bypass ✓ Resolved 🐞 Bug ≡ Correctness
Description
The updated no-args guard in test_tekton_tasks.sh only checks that TEST_ITEMS is non-empty as a raw
string, so whitespace-only TEST_ITEMS bypasses the usage error and then splits into an empty array.
The script then runs zero tests and exits successfully, which can produce a false-green CI run.
Code

.github/scripts/test_tekton_tasks.sh[34]

+if [[ ${1:-} == "-h" ]] || [[ $# -eq 0 && -z "${TEST_ITEMS}" ]]; then
Relevance

⭐⭐ Medium

No precedent for trimming TEST_ITEMS; same script accepted robustness changes in PR #2709.

PR-#2709

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new guard only checks -z "${TEST_ITEMS}", but the script later splits the env var into an
array and never validates the array length; if splitting yields no elements, both ITEM loops are
skipped and the script completes successfully.

.github/scripts/test_tekton_tasks.sh[34-60]
.github/scripts/test_tekton_tasks.sh[62-75]

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

### Issue description
`.github/scripts/test_tekton_tasks.sh` now allows running with no CLI args if `TEST_ITEMS` is set, but it only checks `-z "${TEST_ITEMS}"`. If `TEST_ITEMS` contains only whitespace, the guard passes, `read -r -a TEST_ITEMS` produces an empty array, and the subsequent loops execute zero iterations—making the script exit 0 without running any tests.

### Issue Context
This is a regression introduced by changing the initial help/usage condition. The script should fail fast when it ends up with zero test items after parsing.

### Fix Focus Areas
- .github/scripts/test_tekton_tasks.sh[34-72]

### Suggested fix
1) Treat whitespace-only `TEST_ITEMS` as empty in the initial guard, e.g.:
```bash
[[ $# -eq 0 && -z "${TEST_ITEMS//[[:space:]]/}" ]]
```
2) After parsing into the array, add a hard check:
```bash
if (( ${#TEST_ITEMS[@]} == 0 )); then
 echo "Error: No task directories."
 exit 1
fi
```
This ensures no silent success when nothing will be tested.

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



Remediation recommended

2. Unconditional notice in local runs 🐞 Bug ⚙ Maintainability
Description
hack/validate-migration.sh now prints a GitHub Actions workflow command (::notice ...::)
unconditionally when no modified pipelines are found. Since the script explicitly supports local
execution, this produces confusing/noisy output outside GitHub Actions instead of using the script’s
normal info logger.
Code

hack/validate-migration.sh[R346-348]

    if [ -z "$modified_pipeline_files" ]; then
-        info "No modified pipeline file is found. Skipping in-cluster validation."
+        echo "::notice file=${migration_file},line=1::Migration script doesn't modify any of the pipeline files referenced in $BUILD_PIPELINE_CONFIG. Skipping in-cluster validation."
        return 0
Relevance

⭐⭐ Medium

Script uses info logging historically (PR #3609); no history on unconditional ::notice usage.

PR-#3609

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The script documents local usage and defines info() for local-friendly logging, but the modified
branch uses a GitHub Actions annotation string without any environment gating.

hack/validate-migration.sh[3-6]
hack/validate-migration.sh[41-43]
hack/validate-migration.sh[345-349]

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

### Issue description
`hack/validate-migration.sh` emits `::notice ...::` even when run locally. These workflow commands are intended for GitHub Actions log processing, but locally they appear as raw text and break the script’s consistent `info/error` logging.

### Issue Context
The script header explicitly states it can be run locally, and it already has `info()` for human-readable logs.

### Fix Focus Areas
- hack/validate-migration.sh[3-6]
- hack/validate-migration.sh[41-43]
- hack/validate-migration.sh[345-349]

### Suggested fix
Wrap the notice emission:
```bash
if [[ "${GITHUB_ACTIONS:-false}" == "true" ]]; then
 echo "::notice file=${migration_file},line=1::..."
else
 info "Migration script doesn't modify any of the pipeline files referenced in $BUILD_PIPELINE_CONFIG. Skipping in-cluster validation."
fi
```
This preserves annotations in CI while keeping local output readable.

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


Grey Divider

Qodo Logo

Comment thread .github/scripts/test_tekton_tasks.sh Outdated
Comment thread hack/validate-migration.sh
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 22, 2026

Copy link
Copy Markdown

Review

Findings

Low

  • [error-handling] hack/validate-migration.sh:326check_apply_in_real_cluster now requires $1 for migration_file but has no guard against being called without an argument. Under set -o nounset (set at the top of the script), calling without an argument would cause an unbound variable error. The current call site in main() correctly passes the argument.

  • [injection] hack/validate-migration.sh:346 — The ::notice GHA workflow command interpolates ${migration_file} without explicit sanitization for :: sequences. The risk is mitigated by is_migration_file() which validates file paths against /migrations/[0-9.]+\.sh$, constraining the character set. The workflow also runs on pull_request with read-only permissions, limiting the impact of any annotation injection.

Note: This PR modifies 9 files under the protected path .github/. These changes follow the documented shared CI update process via cruft update (per SHARED-CI.md) and are authorized by the template sync mechanism. Human approval is still required for protected-path changes.

Previous run

Review

Findings

Medium

  • [protected-path] .github/ — This PR modifies 10 files under the protected .github/ path: scripts/test_tekton_tasks.sh, workflows/check-kustomize-build.yaml, workflows/check-ta.yaml, workflows/check-task-migration.yaml, workflows/checkton.yaml, workflows/run-task-tests.yaml, workflows/task-lint.yaml, workflows/update-shared-ci.yaml, workflows/versioning.yaml. These are governance/infrastructure files that require human approval. The PR description indicates this is a shared CI template update via cruft update, and .cruft.json authorizes upstream synchronization from konflux-ci/task-repo-shared-ci. No linked issue is present.

Low

  • [GHA workflow command injection] hack/validate-migration.sh:347 — The ::notice workflow command interpolates $migration_file without sanitizing for GHA command injection sequences. While is_migration_file() constrains the filename suffix to /migrations/[0-9.]+\.sh$, directory path components in the migration file path could potentially contain %0A sequences that GHA interprets as newlines, allowing injection of additional workflow commands. Risk is mitigated by the filename regex constraint and the pull_request trigger context (limited secret access).
    Remediation: Sanitize $migration_file before interpolation into the ::notice command.

  • [coherence-api-contract] hack/validate-migration.sh:347 — The output format of check_apply_in_real_cluster() changed from info() (which writes info: ... to stderr) to echo "::notice ..." (GHA annotation to stdout). This modifies the output contract and is used unconditionally — not gated by a GITHUB_ACTIONS check, unlike the emit() function in generate-ta-tasks.sh which conditionally formats output.
    Remediation: Verify no downstream scripts parse the old output format. Consider gating the ::notice format behind a GITHUB_ACTIONS check for local-run compatibility.


Labels: PR updates shared CI template dependencies and GitHub Actions workflow versions

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment dependencies Pull requests that update a dependency file labels Jul 22, 2026

@mmorhun mmorhun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also Qodo's comment about whitespaces input make sense, just to be safer.

Comment thread hack/generate-ta-tasks.sh
TRUSTED_ARTIFACTS="task-generator/trusted-artifacts"

tashdir="$(mktemp --dry-run)"
tashdir="$(mktemp -d)"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

a typo?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@chmeliik

Copy link
Copy Markdown
Contributor Author

Also Qodo's comment about whitespaces input make sense, just to be safer.

Fixing in konflux-ci/task-repo-shared-ci#68

mmorhun
mmorhun previously approved these changes Jul 22, 2026
Done with: uvx cruft update --skip-apply-ask --allow-untracked-files

And manually resolved rejections in files that we edited separately from
upstream.

Signed-off-by: Adam Cmiel <acmiel@redhat.com>
@chmeliik

Copy link
Copy Markdown
Contributor Author

Re-updated after merging konflux-ci/task-repo-shared-ci#68

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 22, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 1:29 PM UTC · Completed 1:46 PM UTC
Commit: 37b10e4 · View workflow run →

@chmeliik
chmeliik added this pull request to the merge queue Jul 22, 2026
Merged via the queue into konflux-ci:main with commit 67a1e80 Jul 22, 2026
23 checks passed
@chmeliik
chmeliik deleted the update-shared-ci branch July 22, 2026 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file requires-manual-review Review requires human judgment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants