-
Notifications
You must be signed in to change notification settings - Fork 61
Add python script to view archived pipelinerun logs #3528
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,147 @@ | ||
| #!/usr/bin/env python3 | ||
| """Fetch archived Tekton TaskRun logs for a PipelineRun from KubeArchive. | ||
|
|
||
| PipelineRuns in Konflux are vacuumed out of the live cluster and into | ||
| KubeArchive, so `oc logs` / `tkn pr logs` stop working once they're gone. | ||
| KubeArchive mirrors the Kubernetes API and serves archived logs via a | ||
| `/log` subresource, selecting a step with `?container=<step-container>`. | ||
|
|
||
| Usage: | ||
| python hack/ka-logs.py ec-main-enterprise-contract-vqbjs | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Correct the documented script path. This command invokes 🤖 Prompt for AI AgentsThere was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [medium] documentation-consistency The docstring usage examples reference a different filename than the actual file: python hack/ka-logs.py vs the file being hack/find-pr-logs.py. This is a leftover from a rename. Suggested fix: Update the usage examples to reference the actual filename: python hack/find-pr-logs.py. |
||
| python hack/ka-logs.py <pipelinerun> -n <namespace> | ||
| python hack/ka-logs.py <pipelinerun> --task verify # only one pipelineTask | ||
|
Comment on lines
+10
to
+12
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 4. Examples invoke nonexistent script All usage examples tell users to run hack/ka-logs.py, but the added file is hack/find-pr-logs.py. Copying the documented commands therefore fails before the utility starts. Agent Prompt
|
||
|
|
||
| Auth and the KubeArchive route are discovered via `oc` (you must be logged | ||
| in). Override with the KUBEARCHIVE_HOST / KUBEARCHIVE_TOKEN env vars. | ||
| """ | ||
|
|
||
| import argparse | ||
| import json | ||
| import os | ||
| import ssl | ||
| import subprocess | ||
| import sys | ||
| import urllib.parse | ||
| import urllib.request | ||
|
|
||
| DEFAULT_NAMESPACE = "rhtap-contract-tenant" | ||
| KUBEARCHIVE_ROUTE_NS = "product-kubearchive" | ||
| KUBEARCHIVE_ROUTE = "kubearchive-api-server" | ||
|
|
||
|
|
||
| def eprint(*args): | ||
| print(*args, file=sys.stderr) | ||
|
|
||
|
|
||
| def oc(*args): | ||
| """Run an oc command and return stripped stdout, or raise on failure.""" | ||
| try: | ||
| out = subprocess.run( | ||
| ["oc", *args], | ||
| check=True, | ||
| capture_output=True, | ||
| text=True, | ||
| ) | ||
| except FileNotFoundError: | ||
| sys.exit("error: `oc` not found on PATH") | ||
| except subprocess.CalledProcessError as e: | ||
| sys.exit(f"error: `oc {' '.join(args)}` failed:\n{e.stderr.strip()}") | ||
| return out.stdout.strip() | ||
|
|
||
|
|
||
| def discover_host(): | ||
| host = os.environ.get("KUBEARCHIVE_HOST") | ||
| if host: | ||
| return host | ||
| return oc( | ||
| "get", "route", KUBEARCHIVE_ROUTE, | ||
| "-n", KUBEARCHIVE_ROUTE_NS, | ||
| "-o", "jsonpath={.spec.host}", | ||
| ) | ||
|
|
||
|
|
||
| def discover_token(): | ||
| return os.environ.get("KUBEARCHIVE_TOKEN") or oc("whoami", "-t") | ||
|
|
||
|
|
||
| class Client: | ||
| def __init__(self, host, token): | ||
| self.host = host | ||
| self.token = token | ||
| # KubeArchive uses a re-encrypt route; skip verification like `oc`'s | ||
| # -k does here, since the CLI is talking to a known cluster route. | ||
| self.ctx = ssl.create_default_context() | ||
| self.ctx.check_hostname = False | ||
| self.ctx.verify_mode = ssl.CERT_NONE | ||
|
Comment on lines
+73
to
+75
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1. Bearer token sent insecurely Client disables certificate and hostname verification before sending the user's bearer token, allowing an intercepted or impersonated route to capture that credential and alter returned logs. The risk also applies to arbitrary hosts supplied through KUBEARCHIVE_HOST. Agent Prompt
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- repository guidance ---'
find /tmp/coderabbit-repo-knowledge/conforma-cli-e0bb623e -maxdepth 2 -type f -name '*.md' -print
printf '%s\n' '--- target file ---'
cat -n hack/find-pr-logs.py | sed -n '1,115p'Repository: conforma/cli Length of output: 5040 🏁 Script executed: #!/bin/bash
set -eu
cat -n /tmp/coderabbit-repo-knowledge/conforma-cli-e0bb623e/conventions/repo-wide.mdRepository: conforma/cli Length of output: 508 Sensitive Data Exposure (CWE-295): Improper Certificate Validation Reachability: External · Exploitability: Moderate Restore TLS verification before sending the bearer token.
🤖 Prompt for AI Agents |
||
|
|
||
| def _get(self, path, params=None): | ||
| url = f"https://{self.host}{path}" | ||
| if params: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [medium] TLS verification disabled SSL certificate verification is unconditionally disabled (check_hostname=False, verify_mode=CERT_NONE). This exposes the Bearer authentication token (sent on line 86) to interception via man-in-the-middle attacks. Unlike oc -k, which requires explicit user opt-in per invocation, this script always disables verification with no way to enable it. Suggested fix: Default to verifying certificates. If the KubeArchive route uses a custom CA, allow the user to supply a CA bundle via an env var (e.g. KUBEARCHIVE_CA_BUNDLE) or add an explicit --insecure / -k CLI flag. |
||
| url += "?" + urllib.parse.urlencode(params) | ||
| req = urllib.request.Request(url, headers={"Authorization": f"Bearer {self.token}"}) | ||
| try: | ||
| with urllib.request.urlopen(req, context=self.ctx) as resp: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: printf '%s\n' '--- applicable repository guidance ---'
find /tmp/coderabbit-repo-knowledge/conforma-cli-e0bb623e -maxdepth 2 -type f -name '*.md' -print
printf '%s\n' '--- target file outline and relevant source ---'
ast-grep outline hack/find-pr-logs.py
sed -n '1,147p' hack/find-pr-logs.py
printf '%s\n' '--- timeout and request error handling references ---'
rg -n -C 3 'urlopen|timeout|URLError|HTTPError|Client\._get|def _get' hack/find-pr-logs.pyRepository: conforma/cli Length of output: 6649 🏁 Script executed: cat /tmp/coderabbit-repo-knowledge/conforma-cli-e0bb623e/conventions/repo-wide.mdRepository: conforma/cli Length of output: 466 🌐 Web query:
💡 Result: In Python's urllib.request.urlopen, the default timeout behavior is determined by the global default timeout setting [1][2]. Specifically, if the optional timeout parameter is not provided, the function uses socket._GLOBAL_DEFAULT_TIMEOUT [3]. Key points regarding this timeout mechanism include: 1. Global Default: When no timeout is explicitly passed to urlopen, it relies on the global timeout set for sockets, which can be configured application-wide using socket.setdefaulttimeout(timeout) [1][3]. 2. Scope: The timeout applies to blocking operations, such as the connection attempt [1][2]. It is important to note that this is a timeout for specific socket-level operations, not a total deadline for the entire request [4]. 3. Applicability: This timeout feature is effective only for HTTP, HTTPS, and FTP connections [1][2]. 4. Exception Handling: While urllib.request.urlopen is designed to wrap socket errors into a urllib.error.URLError, in some scenarios, internal timeouts may bubble up directly as a socket.timeout or TimeoutError [5][4]. Consequently, robust code should be prepared to catch both urllib.error.URLError and relevant socket/TimeoutError exceptions [4]. Citations:
Set a finite request timeout.
🧰 Tools🪛 Ruff (0.16.3)[error] 83-83: Audit URL open for permitted schemes. Allowing use of (S310) 🤖 Prompt for AI Agents |
||
| return resp.status, resp.read().decode("utf-8", "replace") | ||
|
Comment on lines
+83
to
+84
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 2. Archive requests can hang Every archive request calls urlopen without a timeout, so an unresponsive route can block the script indefinitely. Because requests are performed sequentially for every TaskRun and step, any single stalled request prevents the remaining logs from being fetched. Agent Prompt
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] URL path injection User-controlled values (args.namespace, args.pipelinerun, and child-derived tr_name/container) are interpolated directly into URL paths without encoding or validation. Impact is limited because this is a local developer CLI tool. Suggested fix: Use urllib.parse.quote() on path segments before interpolation. |
||
| except urllib.error.HTTPError as e: | ||
| return e.code, e.read().decode("utf-8", "replace") | ||
|
|
||
| def get_json(self, path, params=None): | ||
| status, body = self._get(path, params) | ||
| if status != 200: | ||
| raise RuntimeError(f"HTTP {status} for {path}: {body[:200]}") | ||
| return json.loads(body) | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] edge-case get_json calls json.loads(body) on the response body without handling json.JSONDecodeError. If the server returns a 200 status with non-JSON content, the script will crash with an unhelpful traceback. Suggested fix: Wrap json.loads(body) in a try/except for json.JSONDecodeError and raise a RuntimeError with a snippet of the body. |
||
| def get_log(self, ns, taskrun, container): | ||
| return self._get( | ||
| f"/apis/tekton.dev/v1/namespaces/{ns}/taskruns/{taskrun}/log", | ||
| {"container": container}, | ||
| ) | ||
|
|
||
|
|
||
| def main(): | ||
| ap = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) | ||
| ap.add_argument("pipelinerun", help="PipelineRun name") | ||
| ap.add_argument("-n", "--namespace", default=DEFAULT_NAMESPACE) | ||
| ap.add_argument("--task", help="only dump this pipelineTask (e.g. verify)") | ||
| ap.add_argument("--no-headers", action="store_true", help="raw logs only, no separators") | ||
| args = ap.parse_args() | ||
|
|
||
| client = Client(discover_host(), discover_token()) | ||
| base = f"/apis/tekton.dev/v1/namespaces/{args.namespace}" | ||
|
|
||
| pr = client.get_json(f"{base}/pipelineruns/{args.pipelinerun}") | ||
| children = [ | ||
| c for c in pr.get("status", {}).get("childReferences", []) | ||
| if c.get("kind") == "TaskRun" | ||
| ] | ||
| if args.task: | ||
| children = [c for c in children if c.get("pipelineTaskName") == args.task] | ||
| if not children: | ||
| sys.exit(f"error: no matching TaskRuns for {args.pipelinerun}") | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] error-handling child["name"] uses direct dict indexing while all other API response fields are accessed via .get() with defaults. A missing name field would produce an unhelpful KeyError. Suggested fix: Use child.get("name") with an appropriate skip or error message. |
||
|
|
||
| for child in children: | ||
| tr_name = child["name"] | ||
| task = child.get("pipelineTaskName", "?") | ||
| tr = client.get_json(f"{base}/taskruns/{tr_name}") | ||
| steps = tr.get("status", {}).get("steps", []) | ||
| for step in steps: | ||
| container = step.get("container") | ||
| if not container: | ||
| continue | ||
| status, body = client.get_log(args.namespace, tr_name, container) | ||
| step_name = step.get("name") | ||
| if not args.no_headers: | ||
| term = step.get("terminated", {}) | ||
| exit_code = term.get("exitCode", "?") | ||
| eprint(f"===== task={task} step={step_name} " | ||
| f"container={container} exit={exit_code} http={status} =====") | ||
| if status == 200: | ||
| prefix = f"[{task} : {step_name}] " | ||
| for line in body.splitlines(): | ||
| sys.stdout.write(prefix + line + "\n") | ||
|
Comment on lines
+139
to
+141
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 3. Raw mode still rewrites logs --no-headers is advertised as raw output, but successful log lines are always prefixed with the task and step names and their original line endings are reconstructed. This breaks consumers expecting the archived log body unchanged. Agent Prompt
|
||
| elif not args.no_headers: | ||
| eprint(f" (no log: HTTP {status})") | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| main() | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[high] license-header
Every script in hack/ begins with the Apache 2.0 license header block (Copyright The Conforma Contributors + SPDX-License-Identifier: Apache-2.0) immediately after the shebang. This new file omits it entirely, breaking the universal convention in this directory.
Suggested fix: Add the standard license header as a Python comment block (lines prefixed with #) between the shebang line and the module docstring, matching the format used in all other hack/ scripts.