CNTRLPLANE-2560: Add async Python script for Konflux task version lookups - #7537
CNTRLPLANE-2560: Add async Python script for Konflux task version lookups#7537celebdor wants to merge 1 commit into
Conversation
|
@celebdor: This pull request references CNTRLPLANE-2560 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Skipping CI for Draft Pull Request. |
|
@celebdor: This pull request references CNTRLPLANE-2560 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
WalkthroughReplaces skopeo-based digest-to-version mapping with a Python aiohttp-based registry lookup; adds a new script to parse Enterprise Contract logs and resolve Tekton task versions via quay.io; updates the update-konflux-tasks command doc to target a single common pipeline file and reflect Python tooling and outputs. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes ✨ Finishing touches
Comment |
2feeb36 to
946ce29
Compare
|
@celebdor: This pull request references CNTRLPLANE-2560 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
1 similar comment
|
@celebdor: This pull request references CNTRLPLANE-2560 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
The original /update-konflux-tasks skill used skopeo to look up task versions but had issues with Docker Registry API pagination, causing incorrect version lookups (e.g., 0.7 → 0.2 "downgrade"). This adds a new Python script that: - Uses async HTTP requests (aiohttp) for parallel registry queries - Properly handles pagination via Link headers to get all semver tags - Parses EC logs using STEP-REPORT-JSON delimiter instead of regex - Prioritizes EC-recommended versions from tasks.unsupported violations - Handles trusted_task.trusted violations for untrusted tasks - Looks up correct digests when using EC-recommended or highest-available The skill documentation is updated to reference the new script and point to the common pipeline file only. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
946ce29 to
cb9cd72
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@hack/tools/scripts/konflux_task_version_lookup.py`:
- Around line 145-149: The env token path currently returns the raw token from
get_auth_token_from_env() which can be interpreted as Basic auth; change the
handling so that when token is truthy you normalize it to a Bearer token (if it
doesn't already start with "Bearer ") before returning it (the return at the
token assignment around get_auth_token_from_env()); apply the same normalization
logic to the other auth branch referenced in the review (the code around lines
184-194) so both paths consistently return a value prefixed with "Bearer ".
- Around line 49-50: The script creates an aiohttp.ClientSession for registry
calls without a timeout, so add a session-level timeout by constructing an
aiohttp.ClientTimeout (e.g., with connect and total/sock_read limits) and pass
it to aiohttp.ClientSession(timeout=...) where the session is instantiated
(refer to the existing aiohttp.ClientSession usage); also ensure any per-request
timeouts aren't overwritten and keep MAX_CONCURRENT_REQUESTS logic intact so the
concurrency limit remains unchanged.
🧹 Nitpick comments (1)
hack/tools/scripts/konflux_task_version_lookup.py (1)
475-544: Avoid order-dependence when applying EC-recommended versions.If a
tasks.unsupportedviolation appears after thetrusted_task.currentwarning (e.g., in a later component),recommended_versionnever gets applied to already-collected tasks. Consider backfilling after all components are parsed.🔁 Proposed fix
- if task_data['recommended_version']: - logger.info(f"Task {friendly_name}: EC recommends version {task_data['recommended_version']}") + if task_data['recommended_version']: + logger.info(f"Task {friendly_name}: EC recommends version {task_data['recommended_version']}") + + # Backfill recommendations discovered after warnings were parsed + for task_data in tasks_by_bundle.values(): + if not task_data.get('recommended_version'): + rec = unsupported_recommendations.get(task_data['task_name']) + if rec: + task_data['recommended_version'] = rec + logger.info(f"Task {task_data['friendly_name']}: EC recommends version {rec}")
| MAX_CONCURRENT_REQUESTS = 20 | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd hack/tools/scripts && wc -l konflux_task_version_lookup.pyRepository: openshift/hypershift
Length of output: 98
🏁 Script executed:
cd hack/tools/scripts && cat -n konflux_task_version_lookup.py | head -70Repository: openshift/hypershift
Length of output: 2405
🏁 Script executed:
cd hack/tools/scripts && cat -n konflux_task_version_lookup.py | sed -n '400,430p'Repository: openshift/hypershift
Length of output: 1601
🏁 Script executed:
cd hack/tools/scripts && grep -n "ClientSession" konflux_task_version_lookup.pyRepository: openshift/hypershift
Length of output: 187
🏁 Script executed:
cd hack/tools/scripts && grep -n -i "timeout\|ClientTimeout" konflux_task_version_lookup.pyRepository: openshift/hypershift
Length of output: 46
🏁 Script executed:
cd hack/tools/scripts && grep -n "self.session" konflux_task_version_lookup.pyRepository: openshift/hypershift
Length of output: 404
🏁 Script executed:
cd hack/tools/scripts && sed -n '200,250p' konflux_task_version_lookup.pyRepository: openshift/hypershift
Length of output: 2309
🏁 Script executed:
cd hack/tools/scripts && sed -n '240,300p' konflux_task_version_lookup.pyRepository: openshift/hypershift
Length of output: 3084
Add explicit HTTP timeouts for registry calls.
The aiohttp.ClientSession created at line 416 lacks a timeout, allowing async registry requests to hang indefinitely. Add a session-level timeout to keep the script responsive during network stalls.
Proposed fix
# Concurrency limit
MAX_CONCURRENT_REQUESTS = 20
+HTTP_TIMEOUT_SECONDS = 30
# Configure logging
logger = logging.getLogger(__name__) async def process_all_tasks(self, tasks: List[dict]) -> List[dict]:
"""Process all tasks in parallel."""
+ timeout = aiohttp.ClientTimeout(total=HTTP_TIMEOUT_SECONDS)
- async with aiohttp.ClientSession() as session:
+ async with aiohttp.ClientSession(timeout=timeout) as session:
self.session = session
results = await asyncio.gather(*[self.process_task(task) for task in tasks])🤖 Prompt for AI Agents
In `@hack/tools/scripts/konflux_task_version_lookup.py` around lines 49 - 50, The
script creates an aiohttp.ClientSession for registry calls without a timeout, so
add a session-level timeout by constructing an aiohttp.ClientTimeout (e.g., with
connect and total/sock_read limits) and pass it to
aiohttp.ClientSession(timeout=...) where the session is instantiated (refer to
the existing aiohttp.ClientSession usage); also ensure any per-request timeouts
aren't overwritten and keep MAX_CONCURRENT_REQUESTS logic intact so the
concurrency limit remains unchanged.
| # 1. Environment variable (already a bearer token) | ||
| token = get_auth_token_from_env() | ||
| if token: | ||
| return token, "QUAY_REGISTRY_TOKEN environment variable" | ||
|
|
There was a problem hiding this comment.
Treat QUAY_REGISTRY_TOKEN as a bearer token, not Basic auth.
Right now a raw token from Line 145 is interpreted as Basic auth unless it already includes Bearer , which breaks the “env token” path for private registries. Consider normalizing the env token to a Bearer token at the source.
🔧 Proposed fix
- token = get_auth_token_from_env()
- if token:
- return token, "QUAY_REGISTRY_TOKEN environment variable"
+ token = get_auth_token_from_env()
+ if token:
+ bearer = token if token.startswith("Bearer ") else f"Bearer {token}"
+ return bearer, "QUAY_REGISTRY_TOKEN environment variable"Also applies to: 184-194
🤖 Prompt for AI Agents
In `@hack/tools/scripts/konflux_task_version_lookup.py` around lines 145 - 149,
The env token path currently returns the raw token from
get_auth_token_from_env() which can be interpreted as Basic auth; change the
handling so that when token is truthy you normalize it to a Bearer token (if it
doesn't already start with "Bearer ") before returning it (the return at the
token assignment around get_auth_token_from_env()); apply the same normalization
logic to the other auth branch referenced in the review (the code around lines
184-194) so both paths consistently return a value prefixed with "Bearer ".
|
/retest-required |
|
Live test on #7551 and looks it's working perfectly fine. It not uses Skopeo, jq and so on, just the Python script. |
|
/retest-required |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: celebdor, jparrill The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/hold Tasks it's failing on the sample PR |
|
@celebdor: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
What this PR does / why we need it:
Adds a new async Python script (
hack/tools/scripts/konflux_task_version_lookup.py) for efficiently querying the quay.io registry API to resolve Konflux Tekton task versions. The original/update-konflux-tasksskill usedskopeovia bash but had issues with Docker Registry API pagination and incomplete EC log parsing.Key improvements:
Pagination handling: Properly follows
Linkheaders in the Docker Registry API to get all semver tags (fixes cases like 0.7 → 0.2 "downgrade" when 0.7 was actually the latest)Structured log parsing: Parses EC logs using the
STEP-REPORT-JSONdelimiter instead of regexHandles three types of EC messages:
trusted_task.currentwarnings - outdated but still trusted taskstasks.unsupportedviolations - task version no longer supportedtrusted_task.trustedviolations - task digest not in trusted list (NEW)EC recommendation priority: Prioritizes EC-recommended versions from violations over digest matching
Correct digest lookup: When using EC-recommended or highest-available versions, looks up the actual digest for that version from the registry
Async performance: Uses aiohttp for parallel HTTP requests to the registry
Files changed:
hack/tools/scripts/konflux_task_version_lookup.py- New async Python script.claude/commands/update-konflux-tasks.md- Updated skill to use the new scriptWhich issue(s) this PR fixes:
Fixes CNTRLPLANE-2560
Special notes for your reviewer:
Requires Python 3.8+ with aiohttp (
pip install aiohttp).Example output showing untrusted task detection:
Checklist:
🤖 Generated with Claude Code via
/update-konflux-tasks