Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .claude/commands/update-konflux-tasks.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,9 @@ Automatically update outdated Konflux Tekton tasks based on enterprise contract
- For any tasks with version bumps (not just digest updates), check for migration notes
- If a task matches quay.io/redhat-appstudio-tekton-catalog/ rather than quay.io/konflux-ci/tekton-catalog, we should check if it is available in quay.io/konflux-ci/tekton-catalog and change to use the latter.
- Use URL pattern: `https://github.com/konflux-ci/build-definitions/blob/main/task/{task-name}/{version}/MIGRATION.md`
- If the migration notes reference a migration script, check their availability with the pattern: `https://github.com/konflux-ci/build-definitions/blob/main/task/{task-name}/{version}/migrations/{version}.sh`. If it is available:
- Run the migration script on the identified pipeline files
- Extract any breaking changes, new parameters, or manual steps required
- Ask the user for input in any manual steps are required
- Migration scripts are automatically detected and run by `update_trusted_task_bundles.py` when version bumps occur (requires `pmt`: `pip install git+https://github.com/konflux-ci/pipeline-migration-tool`)
- Extract any breaking changes, new parameters, or manual steps required from MIGRATION.md
- Ask the user for input if any manual steps are required
- Report if "No action required" or list specific migration steps

4. **Update Pipeline Files**:
Expand Down Expand Up @@ -112,6 +111,7 @@ Automatically update outdated Konflux Tekton tasks based on enterprise contract
- `skopeo` must be installed (for container image inspection)
- `jq` must be installed (for JSON parsing)
- `yq` must be installed (for YAML parsing and checking multi-platform builds)
- `pmt` (pipeline-migration-tool) must be installed for running migration scripts on version bumps: `pip install git+https://github.com/konflux-ci/pipeline-migration-tool`
- `pyyaml` Python package must be installed when running without a log file (for update_trusted_task_bundles.py)
- Internet connectivity (to check migration notes and inspect container images)

Expand Down
175 changes: 175 additions & 0 deletions hack/tools/scripts/update_trusted_task_bundles.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@
# Show diff of changes (without applying)
./update_trusted_task_bundles.py .tekton/pipelines/*.yaml --dry-run --diff

# Upgrade to latest versions (migration scripts run automatically)
./update_trusted_task_bundles.py .tekton/**/*.yaml --upgrade-versions

# Use a different data source
./update_trusted_task_bundles.py pipeline.yaml --data-source quay.io/other/data:latest

Expand Down Expand Up @@ -489,6 +492,163 @@ def print_summary(
print()


MIGRATION_SCRIPT_URL = (
"https://raw.githubusercontent.com/konflux-ci/build-definitions/main"
"/task/{task_name}/{version}/migrations/{version}.sh"
)
Comment on lines +495 to +498

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n 'raw\.githubusercontent\.com/konflux-ci/build-definitions/main|curl", "-fsSL", "-o", script_path|bash", script_path' hack/tools/scripts/update_trusted_task_bundles.py

Repository: openshift/hypershift

Length of output: 274


Pin migration scripts to an immutable ref and verify before execution.

The migration source is resolved from the mutable main branch (line 496), and the downloaded script is executed directly with bash (line 610) without verification. This creates a supply-chain risk: any upstream change or compromise becomes arbitrary code execution in the caller's environment. Additionally, fetching from a mutable branch makes the update non-reproducible.

Use an immutable commit hash or release tag instead of /main/, and verify the downloaded content (e.g., checksum or signature) before invoking bash to execute it.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@hack/tools/scripts/update_trusted_task_bundles.py` around lines 495 - 498,
The MIGRATION_SCRIPT_URL currently points at the mutable "main" branch and the
fetched script is executed directly with bash; change MIGRATION_SCRIPT_URL to
reference an immutable ref (commit hash or release tag) via a new constant or
environment variable (e.g., MIGRATION_REF) and update fetch logic to first
download the script to a file, obtain a trusted checksum/signature (from a
pinned metadata URL or bundled allowlist), verify the downloaded file against
that checksum/signature, and only then execute it; also stop piping remote
content straight into bash—use the verified local file path in the code paths
that call the executor (look for MIGRATION_SCRIPT_URL and the logic that invokes
bash) and abort with a clear error if verification fails.



def get_intermediate_versions(current: str, target: str) -> List[str]:
"""Get all intermediate versions between current (exclusive) and target (inclusive).

For example, get_intermediate_versions("0.2", "0.4") returns ["0.3", "0.4"].
"""
try:
current_parts = [int(p) for p in current.split('.')]
target_parts = [int(p) for p in target.split('.')]
except ValueError:
return [target]

# Simple case: single-level versions like 0.2, 0.3, 0.4
if len(current_parts) == 2 and len(target_parts) == 2 and current_parts[0] == target_parts[0]:
return [f"{current_parts[0]}.{minor}" for minor in range(current_parts[1] + 1, target_parts[1] + 1)]

return [target]


def find_migration_scripts(task_name: str, current_version: str, target_version: str) -> List[Tuple[str, str]]:
"""Find available migration scripts for intermediate versions.

Args:
task_name: e.g. "task-init" -> stripped to "init" for the URL
current_version: e.g. "0.2"
target_version: e.g. "0.4"

Returns:
List of (version, script_url) for versions that have migration scripts.
"""
# Strip "task-" prefix for the build-definitions repo path
short_name = task_name.removeprefix("task-")
Comment on lines +530 to +531

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n 'Python 3\.8\+|removeprefix\(' hack/tools/scripts/update_trusted_task_bundles.py

Repository: openshift/hypershift

Length of output: 137


removeprefix() requires Python 3.9, contradicting documented Python 3.8+ support.

Line 531 uses str.removeprefix(), which was added in Python 3.9. This breaks compatibility with Python 3.8, which is documented as supported at line 28. Either bump the minimum requirement to Python 3.9+ or use a 3.8-compatible alternative.

Python 3.8-compatible fix
-    short_name = task_name.removeprefix("task-")
+    short_name = task_name[5:] if task_name.startswith("task-") else task_name
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@hack/tools/scripts/update_trusted_task_bundles.py` around lines 530 - 531,
The code uses task_name.removeprefix("task-") which requires Python 3.9; replace
it with a Python 3.8-compatible slice: set short_name = task_name[len("task-"):
] if task_name.startswith("task-") else task_name so the "task-" prefix is
stripped only when present (locate the short_name assignment that uses
task_name.removeprefix).


versions = get_intermediate_versions(current_version, target_version)
available = []

for version in versions:
url = MIGRATION_SCRIPT_URL.format(task_name=short_name, version=version)
try:
result = subprocess.run(
["curl", "-fsSL", "-o", "/dev/null", "-w", "%{http_code}", url],
capture_output=True, text=True, timeout=10
)
if result.stdout.strip() == "200":
available.append((version, url))
except (subprocess.CalledProcessError, subprocess.TimeoutExpired):
continue
Comment on lines +539 to +546

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Treat migration lookup and execution failures as fatal.

Lines 539-546 currently treat transport problems the same as “no script exists”, and Lines 613-619 only log execution failures. By the time Lines 790-798 are reached, the bundle refs have already been written, so the command can still exit successfully with half-migrated YAML. Distinguish 404 from lookup failures, and abort or roll back the update on any lookup, download, or execution error.

Also applies to: 613-619, 790-798

🧰 Tools
🪛 Ruff (0.15.6)

[error] 539-539: subprocess call: check for execution of untrusted input

(S603)


[error] 540-540: Starting a process with a partial executable path

(S607)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@hack/tools/scripts/update_trusted_task_bundles.py` around lines 539 - 546,
The current curl/http-check block (the subprocess.run call with ["curl",
"-fsSL", "-o", "/dev/null", "-w", "%{http_code}", url] wrapped in try/except for
subprocess.CalledProcessError and subprocess.TimeoutExpired) treats
transport/timeouts like "not found" and swallows execution errors later; change
it to treat non-200 non-404 responses and any exceptions as fatal: explicitly
check result.stdout for "200" (available) and for "404" (treat as not available)
but if result.stdout is any other code or if
subprocess.CalledProcessError/TimeoutExpired occurs, raise/exit with an error so
the update aborts; apply the same policy to the download subprocess.run (the
block that actually fetches the migration script) and the migration execution
subprocess.run (the block that runs the script which currently only logs
failures around lines 613-619 and 790-798), and ensure you don’t write/commit
bundle refs until all lookups, downloads, and executions succeed (or roll back
the write on failure) so partial migrations cannot leave half-updated YAML.


return available


def run_migrations(all_results: Dict[Path, 'AnalysisResult'], quiet: bool = False):
"""Run migration scripts for all version bumps across pipeline files."""
# Check pmt is available
try:
subprocess.run(["pmt", "--help"], capture_output=True, check=True)
except (subprocess.CalledProcessError, FileNotFoundError):
print(
"Error: pmt (pipeline-migration-tool) is required for migrations.\n"
"Install with: pip install git+https://github.com/konflux-ci/pipeline-migration-tool",
file=sys.stderr
)
sys.exit(1)

# Collect unique version bumps (task_name -> (current, target))
version_bumps: Dict[str, Tuple[str, str]] = {}
bump_files: Dict[str, List[Path]] = {}

for filepath, result in all_results.items():
for update in result.updates:
if update.is_version_bump:
key = update.task_name
version_bumps[key] = (update.current_version, update.latest_version)
bump_files.setdefault(key, []).append(filepath)
Comment on lines +564 to +573

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Don't collapse different bumps under task_name.

Lines 565-573 and 624-632 key migrations only by task name. If two files bump the same task from different starting versions, the later entry overwrites the earlier one; and if the same task appears multiple times in one file, that file is added repeatedly. The result is skipped intermediate migrations for some files or duplicate runs for others. Key this by (task_name, current_version, latest_version) and dedupe the file list.

Also applies to: 624-632

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@hack/tools/scripts/update_trusted_task_bundles.py` around lines 564 - 573,
The code collapses distinct version bumps by using update.task_name as the dict
key, causing overwrites and duplicate file entries; change the keys for
version_bumps and bump_files to a tuple of (update.task_name,
update.current_version, update.latest_version) when collecting from all_results
and ensure you deduplicate file paths (e.g., use a set or check existence before
append) so each unique migration tuple maps to a unique list of files; apply the
same tuple-key + dedupe fix to the other collection block that mirrors this
logic (the block using version_bumps and bump_files later in the file).


if not version_bumps:
if not quiet:
print("\nNo version bumps found, no migrations needed.")
return

if not quiet:
print(f"\n🔄 Checking migration scripts for {len(version_bumps)} version bump(s)...")

for task_name, (current, target) in version_bumps.items():
scripts = find_migration_scripts(task_name, current, target)
if not scripts:
if not quiet:
print(f" {task_name}: {current} → {target} — no migration scripts found")
continue

for version, url in scripts:
if not quiet:
print(f" {task_name}: running {version} migration...")

# Download the script
with tempfile.NamedTemporaryFile(mode='w', suffix='.sh', delete=False) as f:
script_path = f.name

try:
subprocess.run(
["curl", "-fsSL", "-o", script_path, url],
check=True, capture_output=True
)
os.chmod(script_path, 0o755)

# Run on each affected file
for filepath in bump_files[task_name]:
if not quiet:
print(f" → {filepath}")
subprocess.run(
["bash", script_path, str(filepath)],
check=True
)
except subprocess.CalledProcessError as e:
print(f" ⚠️ Migration script failed for {task_name} {version}: {e}", file=sys.stderr)
finally:
os.unlink(script_path)

if not quiet:
print("✅ Migrations complete.\n")


def print_pending_migrations(all_results: Dict[Path, 'AnalysisResult']):
"""Print migration scripts that would be run (dry-run mode)."""
version_bumps: Dict[str, Tuple[str, str]] = {}
bump_files: Dict[str, List[Path]] = {}

for filepath, result in all_results.items():
for update in result.updates:
if update.is_version_bump:
key = update.task_name
version_bumps[key] = (update.current_version, update.latest_version)
bump_files.setdefault(key, []).append(filepath)

if not version_bumps:
return

print(f"\n🔄 Migration scripts to run for {len(version_bumps)} version bump(s):")

for task_name, (current, target) in version_bumps.items():
scripts = find_migration_scripts(task_name, current, target)
if not scripts:
print(f" {task_name}: {current} → {target} — no migration scripts found")
else:
for version, url in scripts:
files = ", ".join(str(f) for f in bump_files[task_name])
print(f" {task_name}: {version} migration → {files}")
print(f" {url}")

print("\nℹ️ Run without --dry-run to apply these migrations.\n")


def main():
parser = argparse.ArgumentParser(
description='Update Tekton pipeline task bundles to latest trusted versions',
Expand Down Expand Up @@ -531,6 +691,11 @@ def main():
action='store_true',
help='Only show errors'
)
parser.add_argument(
'--skip-migrations',
action='store_true',
help='Skip running migration scripts for version bumps (not recommended)'
)

args = parser.parse_args()

Expand Down Expand Up @@ -622,6 +787,16 @@ def main():
if not args.quiet:
print(f"✅ Updated {filepath}")

# Run migrations for version bumps (always, unless explicitly skipped)
has_version_bumps = any(
u.is_version_bump for r in all_results.values() for u in r.updates
)
if has_version_bumps and not args.skip_migrations:
if args.dry_run:
print_pending_migrations(all_results)
else:
run_migrations(all_results, quiet=args.quiet)

# Exit with status based on whether updates were found
total_updates = sum(len(r.updates) for r in all_results.values())
if total_updates > 0 and args.dry_run:
Expand Down
Loading