diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dfd5787..9db89c7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -5,9 +5,15 @@ on: push: jobs: - test: + test-matrix: + name: test (${{ matrix.python-version }}) runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + python-version: ["3.12", "3.13"] + steps: - name: Check out repository uses: actions/checkout@v4 @@ -15,7 +21,7 @@ jobs: - name: Set up Python uses: actions/setup-python@v5 with: - python-version: "3.12" + python-version: ${{ matrix.python-version }} - name: Set up uv uses: astral-sh/setup-uv@v5 @@ -29,4 +35,20 @@ jobs: run: git diff --check - name: Run tests - run: uv run pytest + run: uv run --python ${{ matrix.python-version }} pytest + + test: + name: test + needs: test-matrix + if: ${{ always() }} + runs-on: ubuntu-latest + + steps: + - name: Require all supported Python tests to pass + env: + MATRIX_RESULT: ${{ needs.test-matrix.result }} + run: | + if [ "$MATRIX_RESULT" != "success" ]; then + echo "The supported-Python test matrix finished with: $MATRIX_RESULT" >&2 + exit 1 + fi diff --git a/README.md b/README.md index 54af38b..81fe884 100644 --- a/README.md +++ b/README.md @@ -60,6 +60,57 @@ API-backed commands commonly accept these options. Explicit CLI options win over | `--config` | YAML/JSON defaults. Many commands accept a single object, a list of objects, or `_defaults` + `configs`. | | `--log-type` | SDK progress logs: `console`, `none`, or `file`. Logs go to stderr or a temp log file so structured stdout stays parseable. | | `TANGLE_VERBOSE=1` | Redacted HTTP request/response diagnostics only. This is separate from normal progress logging. | +| `--ca-bundle` | Global CLI flag: path to a PEM CA bundle used as the TLS trust store for every transport. Overrides `TANGLE_API_CA_BUNDLE`. Place before the subcommand. | +| `--verify-tls` / `--no-verify-tls` | Global CLI flag: enable or disable TLS verification for every transport. Overrides `TANGLE_API_VERIFY_TLS`. `--no-verify-tls` is local-development only. Place before the subcommand. | +| `TANGLE_API_CA_BUNDLE` | Path to a PEM CA bundle used to verify TLS for every transport. Use this to trust a private or corporate CA without disabling verification. | +| `TANGLE_API_VERIFY_TLS` | TLS verification toggle. Values `0`, `false`, or `no` (case/space-insensitive) disable verification; any other nonempty value keeps it on. | + +### TLS verification + +TLS certificate verification is enabled by default for all HTTP transports (schema +fetches, `tangle api` calls, and the programmatic clients). The effective setting is +resolved with the following precedence, highest to lowest: + +1. An explicit `verify=` argument to the Python clients (a `bool` or a path to a CA bundle). +2. The global CLI flags `--ca-bundle` / `--verify-tls` / `--no-verify-tls`. +3. `TANGLE_API_CA_BUNDLE` — verify against the given CA bundle. +4. `TANGLE_API_VERIFY_TLS` — enable or disable verification. +5. The secure default: verification enabled against the system trust store. + +The global CLI flags are true root options that apply to every command — the static +`tangle sdk ...` clients, the dynamic `tangle api ...` commands, and `tangle api refresh`. +Place them **before** the subcommand, for example `tangle --ca-bundle ca.pem api ...` or +`tangle --no-verify-tls sdk ...`. They are honored even by the dynamic OpenAPI schema +discovery that runs before command dispatch. A defaulted (absent) flag does not override +the environment: when a flag is not supplied, the `TANGLE_API_*` variables and the standard +`REQUESTS_CA_BUNDLE` / `CURL_CA_BUNDLE` handling still apply. `--ca-bundle` combined with an +explicit `--no-verify-tls` is contradictory and fails fast before any request; `--ca-bundle` +with `--verify-tls` is redundant but accepted. + +If both env vars are set, `TANGLE_API_CA_BUNDLE` wins and TLS stays verified against the +bundle. Empty values are treated as unset. A `--ca-bundle` or `TANGLE_API_CA_BUNDLE` that +does not point to an existing file fails fast with an actionable error before any request is +made. When no Tangle-specific setting is provided, the standard `REQUESTS_CA_BUNDLE` / +`CURL_CA_BUNDLE` handling and any caller-supplied `requests.Session.verify` are left +untouched. + +For a private CA, prefer `--ca-bundle` / `TANGLE_API_CA_BUNDLE` over disabling verification: +it keeps certificates verified against a trusted root. `--no-verify-tls` / +`TANGLE_API_VERIFY_TLS=0` disables verification entirely and is intended for local +development only — never use it against production endpoints. + +```bash +# Trust a private CA with the global flag (recommended for internal/self-hosted APIs) +uv run tangle --ca-bundle /etc/ssl/private-ca.pem \ + api refresh --base-url https://internal.example + +# Or via environment variable +TANGLE_API_CA_BUNDLE=/etc/ssl/private-ca.pem \ + uv run tangle api refresh --base-url https://internal.example + +# Disable verification (local development only) +uv run tangle --no-verify-tls api refresh --base-url https://localhost:8443 +``` Examples for protected APIs: diff --git a/packages/tangle-cli/src/tangle_cli/api_cli.py b/packages/tangle-cli/src/tangle_cli/api_cli.py index 4a80863..db98df8 100644 --- a/packages/tangle-cli/src/tangle_cli/api_cli.py +++ b/packages/tangle-cli/src/tangle_cli/api_cli.py @@ -640,10 +640,22 @@ def _argv_dispatches_dynamic_command(argv: list[str]) -> bool: def _api_argv_tail(argv: list[str]) -> list[str] | None: - """Return args after the root `api` command, or None for non-API invocations.""" + """Return args after the root `api` command, or None for non-API invocations. + + Global TLS flags (``--ca-bundle``/``--verify-tls``/``--no-verify-tls``) may + precede the subcommand, so they are skipped before locating `api`. + """ args = list(argv[1:]) - for index, arg in enumerate(args): + index = 0 + while index < len(args): + arg = args[index] + if arg == "--ca-bundle" and index + 1 < len(args): + index += 2 + continue + if arg.startswith("--ca-bundle=") or arg in {"--verify-tls", "--no-verify-tls"}: + index += 1 + continue if arg == "--": if index + 1 < len(args) and args[index + 1] == "api": return args[index + 2 :] diff --git a/packages/tangle-cli/src/tangle_cli/api_schema.py b/packages/tangle-cli/src/tangle_cli/api_schema.py index 5475ea8..72ed2cc 100644 --- a/packages/tangle-cli/src/tangle_cli/api_schema.py +++ b/packages/tangle-cli/src/tangle_cli/api_schema.py @@ -18,7 +18,9 @@ _normalize_base_url, _openapi_url, _request_headers, + _VERIFY_UNSET, default_base_url, + httpx_verify, ) SUPPORTED_METHODS = {"get", "post", "put", "patch", "delete"} @@ -122,6 +124,7 @@ def fetch_schema( auth_header: str | None = None, headers: dict[str, str] | None = None, include_env_credentials: bool = True, + verify: Any = _VERIFY_UNSET, ) -> dict[str, Any]: """Fetch ``/openapi.json``, applying bearer and custom auth headers.""" @@ -136,6 +139,7 @@ def fetch_schema( include_env_credentials=include_env_credentials, ), timeout=DEFAULT_TIMEOUT_SECONDS, + verify=httpx_verify(verify), ) response.raise_for_status() payload = response.text @@ -152,6 +156,7 @@ def refresh_schema( auth_header: str | None = None, headers: dict[str, str] | None = None, include_env_credentials: bool = True, + verify: Any = _VERIFY_UNSET, ) -> tuple[dict[str, Any], Path]: """Fetch and cache the latest schema for a backend.""" @@ -163,6 +168,7 @@ def refresh_schema( auth_header, headers, include_env_credentials=include_env_credentials, + verify=verify, ) path = write_cached_schema(schema, base_url) return schema, path @@ -175,6 +181,7 @@ def load_or_fetch_schema( auth_header: str | None = None, headers: dict[str, str] | None = None, include_env_credentials: bool = True, + verify: Any = _VERIFY_UNSET, ) -> dict[str, Any]: """Use a cached schema when available, otherwise fetch once and cache it.""" @@ -188,6 +195,7 @@ def load_or_fetch_schema( auth_header, headers, include_env_credentials=include_env_credentials, + verify=verify, ) return schema diff --git a/packages/tangle-cli/src/tangle_cli/api_transport.py b/packages/tangle-cli/src/tangle_cli/api_transport.py index b128070..42c76f2 100644 --- a/packages/tangle-cli/src/tangle_cli/api_transport.py +++ b/packages/tangle-cli/src/tangle_cli/api_transport.py @@ -5,6 +5,7 @@ import json import os import re +import ssl import sys import urllib.parse from pathlib import Path @@ -16,6 +17,20 @@ DEFAULT_TIMEOUT_SECONDS = 30.0 _HEADER_NAME_RE = re.compile(r"^[!#$%&'*+.^_`|~0-9A-Za-z-]+$") _MISSING = object() + +# Canonical resolved TLS verification value: ``True``/``False`` or a CA bundle +# path. Both requests and httpx transports adapt this single contract. +VerifyValue = bool | str +VerifyArgument = bool | str | os.PathLike[str] | None +_VERIFY_UNSET = object() +_TLS_FALSE_VALUES = frozenset({"0", "false", "no"}) + +# Process-wide TLS override set from global CLI flags (``--ca-bundle`` / +# ``--verify-tls`` / ``--no-verify-tls``). It sits between an explicit +# ``verify=`` argument and the environment variables in :func:`resolve_verify`, +# so a single resolver serves every transport, including the schema discovery +# that runs before normal command dispatch. +_CLI_VERIFY_OVERRIDE: Any = _VERIFY_UNSET _SENSITIVE_HEADER_NAMES = {"authorization", "cloud-auth", "cookie", "x-api-key"} _SENSITIVE_KEY_RE = re.compile( r"(authorization|authentication|(^|[-_])auth($|[-_])|cloud[-_]?auth|cookie|x[-_]?api[-_]?key|token|secret|password|credential|pre[-_]?signed[-_]?url|signed[-_]?url)", @@ -40,6 +55,115 @@ def tangle_verbose_enabled() -> bool: return value.strip().lower() in {"1", "true", "yes", "on"} +def _parse_verify_flag(raw: str) -> bool: + """Interpret ``TANGLE_API_VERIFY_TLS``. + + Only the case/space-insensitive values ``0``, ``false``, and ``no`` disable + verification; any other nonempty value keeps it enabled. + """ + + return raw.strip().lower() not in _TLS_FALSE_VALUES + + +def _validate_ca_bundle(path: str, source: str) -> str: + """Return an existing CA bundle path or fail before any network request.""" + + candidate = Path(path).expanduser() + if not candidate.is_file(): + raise SystemExit( + f"{source} points to a CA bundle that does not exist: {path!r}. " + "Provide a path to an existing PEM file, or unset it to use the " + "system trust store." + ) + return str(candidate) + + +def _coerce_explicit_verify(value: bool | str | os.PathLike[str]) -> VerifyValue: + if isinstance(value, bool): + return value + if isinstance(value, (str, os.PathLike)): + return _validate_ca_bundle(os.fspath(value), "verify") + raise SystemExit("verify must be a bool or a path to a CA bundle file") + + +def configure_cli_verify( + ca_bundle: str | os.PathLike[str] | None = None, + verify_tls: bool | None = None, +) -> None: + """Install (or clear) the process-wide TLS override from global CLI flags. + + ``ca_bundle`` is a path to a PEM trust store; ``verify_tls`` is the tri-state + ``--verify-tls`` / ``--no-verify-tls`` flag where ``None`` means the flag was + not supplied. A ``--ca-bundle`` combined with an explicit + ``--no-verify-tls`` is contradictory and fails fast before any network + request. Passing neither clears any previously installed override. + + The resolved value is validated here so an invalid or missing CA bundle + fails before dynamic schema discovery. It is consulted by + :func:`resolve_verify` for every transport. + """ + + global _CLI_VERIFY_OVERRIDE + if ca_bundle is not None: + if verify_tls is False: + raise SystemExit( + "--ca-bundle cannot be combined with --no-verify-tls: a CA " + "bundle only takes effect when verification is enabled. Drop " + "one of the two flags." + ) + _CLI_VERIFY_OVERRIDE = _validate_ca_bundle(os.fspath(ca_bundle), "--ca-bundle") + return + if verify_tls is not None: + _CLI_VERIFY_OVERRIDE = bool(verify_tls) + return + _CLI_VERIFY_OVERRIDE = _VERIFY_UNSET + + +def resolve_verify(verify: Any = _VERIFY_UNSET) -> Any: + """Resolve the effective TLS verification setting. + + Precedence, highest to lowest: an explicit ``verify`` argument, the global + CLI override (``--ca-bundle`` / ``--verify-tls`` / ``--no-verify-tls``), a + nonempty ``TANGLE_API_CA_BUNDLE``, ``TANGLE_API_VERIFY_TLS``, then a secure + enabled default. When none of these are set, ``_VERIFY_UNSET`` is returned + so callers can preserve library and caller defaults (for example requests' + ``REQUESTS_CA_BUNDLE``/``CURL_CA_BUNDLE`` handling and a caller-supplied + ``Session.verify``). Empty environment values are treated as unset. + """ + + if verify is not _VERIFY_UNSET and verify is not None: + return _coerce_explicit_verify(verify) + if _CLI_VERIFY_OVERRIDE is not _VERIFY_UNSET: + return _CLI_VERIFY_OVERRIDE + ca_bundle = os.environ.get("TANGLE_API_CA_BUNDLE", "") + if ca_bundle.strip(): + return _validate_ca_bundle(ca_bundle.strip(), "TANGLE_API_CA_BUNDLE") + flag = os.environ.get("TANGLE_API_VERIFY_TLS", "") + if flag.strip(): + return _parse_verify_flag(flag) + return _VERIFY_UNSET + + +def resolve_verify_default(verify: Any = _VERIFY_UNSET) -> VerifyValue: + """Like :func:`resolve_verify`, but fall back to the secure ``True`` default.""" + + resolved = resolve_verify(verify) + return True if resolved is _VERIFY_UNSET else resolved + + +def httpx_verify(verify: Any = _VERIFY_UNSET) -> bool | ssl.SSLContext: + """Adapt the resolved verify value to what httpx expects. + + httpx 0.28 deprecates string CA-bundle paths, so a path is turned into an + :class:`ssl.SSLContext`; booleans pass through unchanged. + """ + + resolved = resolve_verify_default(verify) + if isinstance(resolved, bool): + return resolved + return ssl.create_default_context(cafile=resolved) + + def _redact_headers(headers: dict[str, Any] | None) -> dict[str, Any]: redacted: dict[str, Any] = {} for name, value in (headers or {}).items(): @@ -280,6 +404,7 @@ def request_operation( timeout: float = DEFAULT_TIMEOUT_SECONDS, allow_body_file_references: bool = False, include_env_credentials: bool = True, + verify: Any = _VERIFY_UNSET, ) -> httpx.Response: """Dispatch one normalized OpenAPI operation as an HTTP request. @@ -306,6 +431,7 @@ def request_operation( content=content, headers=request_headers, timeout=timeout, + verify=httpx_verify(verify), ) if tangle_verbose_enabled(): log_http_exchange( diff --git a/packages/tangle-cli/src/tangle_cli/artifacts_cli.py b/packages/tangle-cli/src/tangle_cli/artifacts_cli.py index a6d59e1..bb9f496 100644 --- a/packages/tangle-cli/src/tangle_cli/artifacts_cli.py +++ b/packages/tangle-cli/src/tangle_cli/artifacts_cli.py @@ -80,6 +80,7 @@ def artifacts_get( header=args.header, include_env_credentials=include_env_credentials_for_args(args, base_url), command_name="artifact commands", + logger=logger, ) if require_available := getattr(client, "require_available", None): require_available() diff --git a/packages/tangle-cli/src/tangle_cli/cli.py b/packages/tangle-cli/src/tangle_cli/cli.py index e9c1fd4..ad9541c 100644 --- a/packages/tangle-cli/src/tangle_cli/cli.py +++ b/packages/tangle-cli/src/tangle_cli/cli.py @@ -1,4 +1,9 @@ -from cyclopts import App +from __future__ import annotations + +import sys +from typing import Annotated + +from cyclopts import App, Parameter from . import ( __version__, @@ -11,6 +16,8 @@ quickstart, secrets_cli, ) +from .api_transport import configure_cli_verify +from .cli_options import CaBundleOption, VerifyTlsOption def version() -> None: @@ -19,6 +26,41 @@ def version() -> None: print(__version__) +def _configure_tls_from_argv(argv: list[str]) -> None: + """Parse the global TLS flags and install the process-wide override. + + Runs before the `api` app is built so the override is in place for the + dynamic schema discovery that happens during command construction, ahead of + normal Cyclopts dispatch. Only tokens before the first subcommand are + considered; flag validation and conflict detection live in + :func:`configure_cli_verify`. + """ + + ca_bundle: str | None = None + verify_tls: bool | None = None + index = 1 + while index < len(argv): + arg = argv[index] + if arg == "--ca-bundle" and index + 1 < len(argv): + ca_bundle = argv[index + 1] + index += 2 + continue + if arg.startswith("--ca-bundle="): + ca_bundle = arg.split("=", 1)[1] + index += 1 + continue + if arg == "--verify-tls": + verify_tls = True + index += 1 + continue + if arg == "--no-verify-tls": + verify_tls = False + index += 1 + continue + break + configure_cli_verify(ca_bundle, verify_tls) + + def build_sdk_app() -> App: """Build the SDK command group.""" @@ -35,8 +77,15 @@ def build_sdk_app() -> App: return sdk_app -def build_app() -> App: - """Build the root CLI app lazily for the current invocation.""" +def build_app(argv: list[str] | None = None) -> App: + """Build the root CLI app lazily for the current invocation. + + Global TLS flags are parsed from *argv* (defaulting to ``sys.argv``) and + installed before the `api` app is built, because building it can trigger + dynamic OpenAPI schema discovery over the network. + """ + + _configure_tls_from_argv(sys.argv if argv is None else argv) app = App( help="CLI for Tangle, the open-source ML pipeline orchestration platform.", @@ -46,11 +95,23 @@ def build_app() -> App: app.command(quickstart.app) app.command(api_cli.build_app()) app.command(build_sdk_app()) + + @app.meta.default + def launcher( + *tokens: Annotated[str, Parameter(allow_leading_hyphen=True)], + ca_bundle: CaBundleOption = None, + verify_tls: VerifyTlsOption = None, + ) -> None: + """Apply global TLS options, then dispatch the requested command.""" + + configure_cli_verify(ca_bundle, verify_tls) + app(tokens) + return app def main() -> None: - build_app()() + build_app().meta() if __name__ == "__main__": diff --git a/packages/tangle-cli/src/tangle_cli/cli_options.py b/packages/tangle-cli/src/tangle_cli/cli_options.py index f6080ed..5edfbb2 100644 --- a/packages/tangle-cli/src/tangle_cli/cli_options.py +++ b/packages/tangle-cli/src/tangle_cli/cli_options.py @@ -2,6 +2,7 @@ from __future__ import annotations +from pathlib import Path from typing import Annotated from cyclopts import Parameter @@ -50,3 +51,26 @@ str, Parameter(help="Log output: console, none, file."), ] +CaBundleOption = Annotated[ + Path | None, + Parameter( + name="--ca-bundle", + help=( + "Path to a PEM CA bundle used as the TLS trust store for every " + "transport. Overrides TANGLE_API_CA_BUNDLE. Place before the " + "subcommand, e.g. `tangle --ca-bundle ca.pem api ...`." + ), + ), +] +VerifyTlsOption = Annotated[ + bool | None, + Parameter( + name="--verify-tls", + help=( + "Enable (--verify-tls) or disable (--no-verify-tls) TLS " + "certificate verification for every transport. Overrides " + "TANGLE_API_VERIFY_TLS. --no-verify-tls is for local development " + "only. Place before the subcommand." + ), + ), +] diff --git a/packages/tangle-cli/src/tangle_cli/client.py b/packages/tangle-cli/src/tangle_cli/client.py index 060c960..2c15a9b 100644 --- a/packages/tangle-cli/src/tangle_cli/client.py +++ b/packages/tangle-cli/src/tangle_cli/client.py @@ -9,21 +9,24 @@ from __future__ import annotations import time -from collections.abc import Iterable, Mapping +from collections.abc import Iterable, Iterator, Mapping from dataclasses import asdict, is_dataclass from email.utils import parsedate_to_datetime from typing import Any -from urllib.parse import quote, urljoin, urlparse +from urllib.parse import quote, urljoin, urlparse, urlsplit, urlunsplit import requests from .api_transport import ( DEFAULT_TIMEOUT_SECONDS, + VerifyArgument, _join_operation_url, _normalize_base_url, _request_headers, + _VERIFY_UNSET, default_base_url, log_http_exchange, + resolve_verify, tangle_verbose_enabled, ) from tangle_api.generated.operations import GeneratedTangleApiOperations @@ -40,6 +43,69 @@ ) +class _RetryBudget: + """Shared attempt/send/deadline budget for one logical request. + + The transient-5xx, 429 rate-limit, and 401 auth-refresh retry layers all + draw from a single instance so a composed outage cannot multiply their + per-layer limits. Attempts and sends are counted separately because they + bound different things: ``attempts`` is how many logical tries remain (one + per entry into the redirect helper) while ``sends`` is how many physical + ``session.request`` calls remain, every same-origin redirect hop included. + Keeping ``sends`` larger than ``attempts`` is what lets a legal redirect + chain still be followed on a retry, instead of its hops being spent as if + they were tries. ``deadline`` is a ``time.monotonic`` value past which no + further send is admitted. + """ + + __slots__ = ("attempts", "deadline", "sends") + + def __init__(self, max_attempts: int, max_sends: int, deadline: float) -> None: + self.attempts = max_attempts + self.sends = max_sends + self.deadline = deadline + + def start_attempt(self) -> None: + """Charge one logical try; callers gate on :meth:`can_retry` first.""" + + self.attempts -= 1 + + def try_consume_send(self) -> bool: + """Atomically admit one physical send, charging the budget for it. + + Checking and charging together, immediately before the send, is what + stops a request from slipping out after a long ``Retry-After`` or + backoff has already carried the clock past the deadline. + """ + + if self.sends <= 0 or time.monotonic() >= self.deadline: + return False + self.sends -= 1 + return True + + def exhaustion_reason(self) -> str: + """Why :meth:`try_consume_send` refused, for the exhaustion error text. + + Mirrors that method's check order so the reported cause is the one that + actually stopped the send. + """ + + return "send pool exhausted" if self.sends <= 0 else "deadline exceeded" + + def can_retry(self) -> bool: + return self.attempts > 0 and self.sends > 0 and time.monotonic() < self.deadline + + def allows_wait(self, delay: float) -> bool: + """True when sleeping ``delay`` would still leave time to send again. + + Waits are bounded by the remaining deadline rather than truncated to it: + a sleep that would reach the deadline can only be followed by a send the + budget must refuse, so the caller stops immediately instead. + """ + + return delay < self.deadline - time.monotonic() + + class TangleApiClient(GeneratedTangleApiOperations): """Single public API wrapper for Tangle backends. @@ -50,9 +116,45 @@ class TangleApiClient(GeneratedTangleApiOperations): _REDIRECT_STATUSES = {301, 302, 303, 307, 308} _MAX_REDIRECTS = 5 - _MAX_RATE_LIMIT_RETRIES = 3 _RATE_LIMIT_BACKOFF_SECONDS = 1.0 _MAX_RETRY_AFTER_SECONDS = 60.0 + # Opening a log stream retries transient failures (transport-open errors + # and retryable 5xx) with doubling backoff, spent entirely before any line + # is yielded. Connect and response-header reads are bounded; after the final + # response is accepted, only its body socket is reset to unbounded idle. + # An already-open stream that drops is the caller's to handle; never + # re-opening an established stream means lines cannot be duplicated. + _RETRYABLE_STREAM_STATUSES = frozenset({500, 502, 503, 504}) + _MAX_STREAM_OPEN_ATTEMPTS = 7 + _STREAM_OPEN_BACKOFF_SECONDS = 1.0 + _MAX_STREAM_OPEN_BACKOFF_SECONDS = 30.0 + # Requests that cannot be replayed after a transient failure (mutating + # methods and streamed GETs) keep the historical four-attempt 429 allowance + # and its 1/2/4s backoff rather than spending the larger shared GET budget + # on rate limiting alone. + _MAX_RATE_LIMIT_RETRIES = 3 + _RETRYABLE_GET_STATUSES = frozenset({500, 502, 503, 504}) + _MAX_GET_RETRIES = 6 + _GET_RETRY_BACKOFF_SECONDS = 1.0 + _MAX_GET_RETRY_BACKOFF_SECONDS = 30.0 + # A single logical request may issue at most ``_MAX_GET_RETRIES + 1`` + # attempts and ``_MAX_PHYSICAL_SENDS`` physical sends -- counting every + # same-origin redirect hop -- shared across the transient-5xx, 429 + # rate-limit, and 401 auth-refresh layers, and must not spend more than + # ``_MAX_RETRY_ELAPSED_SECONDS`` retrying. One shared budget prevents the + # layers from multiplying into a large physical request count during an + # outage (e.g. interleaved 503/429 responses, a 401 mid-sequence, or a + # redirect chain in front of every retry). + # + # Sends are pooled separately from attempts because one attempt behind a + # redirect chain costs several sends. The pool is the larger of two floors: + # two full ``_MAX_REDIRECTS``-deep chains (12), so such a chain stays + # followable both before and after a 401 refresh, and two sends per logical + # attempt (14), so a one-hop gateway does not halve the effective retry + # count. The second dominates at current constants. The worst case stays far + # below attempts x chain length. + _MAX_PHYSICAL_SENDS = max(2 * (_MAX_REDIRECTS + 1), 2 * (_MAX_GET_RETRIES + 1)) + _MAX_RETRY_ELAPSED_SECONDS = 120.0 def __init__( self, @@ -67,6 +169,7 @@ def __init__( timeout: float = DEFAULT_TIMEOUT_SECONDS, session: requests.Session | None = None, include_env_credentials: bool = True, + verify: VerifyArgument = None, ) -> None: self.base_url = _normalize_base_url(base_url or default_base_url()) env_verbose = tangle_verbose_enabled() @@ -79,6 +182,7 @@ def __init__( self.timeout = timeout self.session = session or requests.Session() self.include_env_credentials = include_env_credentials + self._verify = resolve_verify(verify) def _response_model(self, model_name: str, default: Any) -> Any: """Use CLI-composed models for generated operation deserialization.""" @@ -121,6 +225,12 @@ def _make_request( clean_params = self._clean_mapping(params) request_method = method.upper() + budget = _RetryBudget( + self._MAX_GET_RETRIES + 1, + self._MAX_PHYSICAL_SENDS, + time.monotonic() + self._MAX_RETRY_ELAPSED_SECONDS, + ) + self._refresh_auth() response = self._request_with_rate_limit_retries( request_method, @@ -130,18 +240,34 @@ def _make_request( extra_headers=extra_headers, timeout=timeout, request_kwargs=kwargs, + budget=budget, ) - if response.status_code == 401: + # The auth-refresh retry draws from the same budget, so a 401 late in a + # transient/rate-limit sequence cannot start a fresh round of retries. + if response.status_code == 401 and budget.can_retry(): + # The 401 response is discarded by the auth-refresh retry. For a + # streamed request it is an open streamed connection, so close it + # before refreshing auth and issuing the second request to avoid + # leaking it. ``response.headers`` stays available after close. + if kwargs.get("stream"): + response.close() self._refresh_auth() - response = self._request_with_rate_limit_retries( - request_method, - url, - params=clean_params, - json_data=json_data, - extra_headers=extra_headers, - timeout=timeout, - request_kwargs=kwargs, - ) + try: + response = self._request_with_rate_limit_retries( + request_method, + url, + params=clean_params, + json_data=json_data, + extra_headers=extra_headers, + timeout=timeout, + request_kwargs=kwargs, + budget=budget, + ) + except requests.exceptions.RetryError: + # The retry ran out of sends before any response came back. The + # 401 already in hand is a real backend answer, so report it + # rather than an exhaustion error. + return response return response def _request_with_rate_limit_retries( @@ -152,12 +278,99 @@ def _request_with_rate_limit_retries( params: Mapping[str, Any] | None, json_data: Any, extra_headers: Mapping[str, str] | None, - timeout: float, + timeout: float | tuple[float, float | None], request_kwargs: Mapping[str, Any], + budget: _RetryBudget, ) -> requests.Response: - response: requests.Response | None = None - for attempt in range(self._MAX_RATE_LIMIT_RETRIES + 1): - response = self._request_with_same_origin_redirects( + # Replayable GETs are bounded by the shared budget alone; everything else + # bypasses the transient layer and so keeps its historical 429 cap. + max_rounds = ( + None + if self._is_transient_retryable(method, request_kwargs) + else self._MAX_RATE_LIMIT_RETRIES + ) + rate_limit_round = 0 + last_response: requests.Response | None = None + while True: + try: + response = self._request_with_transient_retries( + method, + url, + params=params, + json_data=json_data, + extra_headers=extra_headers, + timeout=timeout, + request_kwargs=request_kwargs, + budget=budget, + ) + except requests.exceptions.RetryError: + # Sends ran out part-way through a redirect chain. The 429 from + # the previous round is a real backend answer and reports better + # than the exhaustion error. + if last_response is None: + raise + return last_response + if response.status_code != 429: + return response + if max_rounds is not None and rate_limit_round >= max_rounds: + return response + delay = self._rate_limit_delay(response, rate_limit_round) + # A 429 retry re-enters the transient layer, so it must draw from the + # shared budget rather than a per-round allowance. A ``Retry-After`` + # that would outlast the deadline ends the sequence here instead of + # sleeping into a send the budget must then refuse. + if not budget.can_retry() or not budget.allows_wait(delay): + return response + # Release the superseded 429 so its connection is not held for the + # whole wait. Streamed responses are released here too: a 429 is + # never the stream the caller asked for, and nothing has read its + # body. The status stays readable afterwards, so this response is + # still reportable if a later round exhausts the budget mid-chain. + last_response = response + self._release_response(response) + self._sleep_for_rate_limit(delay) + rate_limit_round += 1 + + @staticmethod + def _is_transient_retryable(method: str, request_kwargs: Mapping[str, Any]) -> bool: + """Only non-streamed GETs may be replayed after a transient failure. + + Mutating methods must never be duplicated, and a streamed GET's consumer + owns any stream-open retries. + """ + + return method.upper() == "GET" and not request_kwargs.get("stream") + + def _request_with_transient_retries( + self, + method: str, + url: str, + *, + params: Mapping[str, Any] | None, + json_data: Any, + extra_headers: Mapping[str, str] | None, + timeout: float | tuple[float, float | None], + request_kwargs: Mapping[str, Any], + budget: _RetryBudget, + ) -> requests.Response: + """Retry idempotent GETs on transient 5xx and transport errors. + + Mutating methods are sent once (never duplicated). Streamed GETs bypass + this layer so their consumer owns any stream-open retries. 429s are left + to the rate-limit layer, whose retries re-enter this layer with a fresh + backoff. ``SSLError`` raises immediately: certificate failures are + deterministic, so retrying only delays the report. Every physical send + draws from the shared ``budget`` at the send boundary, so the transient, + rate-limit, and auth-refresh layers cannot multiply into a large request + count. Each doubling sleep is capped at + ``_MAX_GET_RETRY_BACKOFF_SECONDS``, is skipped entirely when it would + outlast the deadline, and is announced through ``self.logger`` (a null + logger on non-verbose clients built without one), so a stalled GET is + bounded. + """ + + if not self._is_transient_retryable(method, request_kwargs): + return self._request_with_same_origin_redirects( method, url, params=params, @@ -165,22 +378,93 @@ def _request_with_rate_limit_retries( extra_headers=extra_headers, timeout=timeout, request_kwargs=request_kwargs, + budget=budget, ) - if response.status_code != 429 or attempt == self._MAX_RATE_LIMIT_RETRIES: - return response - self._sleep_for_rate_limit(response, attempt) - return response + backoff = self._GET_RETRY_BACKOFF_SECONDS + attempt = 0 + last_response: requests.Response | None = None + while True: + attempt += 1 + delay = min(backoff, self._MAX_GET_RETRY_BACKOFF_SECONDS) + backoff *= 2.0 + try: + response = self._request_with_same_origin_redirects( + method, + url, + params=params, + json_data=json_data, + extra_headers=extra_headers, + timeout=timeout, + request_kwargs=request_kwargs, + budget=budget, + ) + except requests.exceptions.RetryError: + # Sends ran out part-way through a redirect chain. A completed + # 5xx from an earlier attempt tells the caller what the backend + # actually said, so ``raise_for_status`` still reports the true + # status instead of an exhaustion error. + if last_response is None: + raise + return last_response + # Transient transport failures (reset/refused, timeout, truncated or + # corrupt body) can succeed on retry; other request errors surface. + except ( + requests.ConnectionError, + requests.Timeout, + requests.exceptions.ChunkedEncodingError, + requests.exceptions.ContentDecodingError, + ) as exc: + # SSLError subclasses ConnectionError but signals a certificate + # or TLS configuration problem that no retry can fix. + if isinstance(exc, requests.exceptions.SSLError): + raise + # Budget exhausted (attempts or deadline), or the backoff alone + # would outlast the deadline: surface the failure. + if not budget.can_retry() or not budget.allows_wait(delay): + raise + self._sleep_for_transient_retry(delay, attempt, type(exc).__name__) + else: + if response.status_code not in self._RETRYABLE_GET_STATUSES: + return response + # Budget exhausted, or the backoff alone would outlast the + # deadline: return the final 5xx for raise_for_status. + if not budget.can_retry() or not budget.allows_wait(delay): + return response + # Release the intermediate response so its connection returns to + # the pool. Its body is already buffered (these GETs are never + # streamed), so it stays reportable if the retry runs out of + # sends mid-chain. + last_response = response + self._release_response(response) + self._sleep_for_transient_retry(delay, attempt, f"HTTP {response.status_code}") + + def _sleep_for_transient_retry(self, delay: float, attempt: int, reason: str) -> None: + self.logger.warn( # noqa: G010 - Logger intentionally exposes warn(). + f"transient {reason} on GET; retrying in {delay:.1f}s " + f"(attempt {attempt + 1}/{self._MAX_GET_RETRIES + 1})" + ) + time.sleep(delay) - def _sleep_for_rate_limit(self, response: requests.Response, attempt: int) -> None: + def _rate_limit_delay(self, response: requests.Response, attempt: int) -> float: retry_after = response.headers.get("Retry-After") delay = self._retry_after_delay(retry_after) if delay is None: delay = self._RATE_LIMIT_BACKOFF_SECONDS * (2 ** attempt) - delay = min(delay, self._MAX_RETRY_AFTER_SECONDS) + return min(delay, self._MAX_RETRY_AFTER_SECONDS) + + def _sleep_for_rate_limit(self, delay: float) -> None: if self.verbose: self.logger.info(f"429 rate limited; retrying in {delay:.1f}s") time.sleep(delay) + def _sleep_for_stream_open_retry(self, backoff: float, next_attempt: int, reason: str) -> None: + delay = min(backoff, self._MAX_STREAM_OPEN_BACKOFF_SECONDS) + self.logger.warn( + f"transient {reason} opening log stream; retrying in {delay:.1f}s " + f"(attempt {next_attempt}/{self._MAX_STREAM_OPEN_ATTEMPTS})" + ) + time.sleep(delay) + @staticmethod def _retry_after_delay(value: str | None) -> float | None: if not value: @@ -205,8 +489,9 @@ def _request_with_same_origin_redirects( params: Mapping[str, Any] | None, json_data: Any, extra_headers: Mapping[str, str] | None, - timeout: float, + timeout: float | tuple[float, float | None], request_kwargs: Mapping[str, Any], + budget: _RetryBudget, ) -> requests.Response: """Send one request, following only same-origin redirects. @@ -214,8 +499,16 @@ def _request_with_same_origin_redirects( ``requests`` does not strip those custom credentials on cross-origin redirects, so redirects are handled manually and constrained to the original origin. + + This helper is entered once per logical attempt, so it charges the + shared budget one attempt on entry and one send per hop immediately + before that hop goes out. Charging hops keeps a redirect in front of + every retry from multiplying the total request count, while charging + them against the send pool rather than the attempt count leaves a legal + chain followable on every attempt. """ + budget.start_attempt() current_method = method current_url = url current_params = params @@ -223,7 +516,15 @@ def _request_with_same_origin_redirects( response: requests.Response | None = None for _ in range(self._MAX_REDIRECTS + 1): + if not budget.try_consume_send(): + raise requests.exceptions.RetryError( + f"Retry budget exhausted ({budget.exhaustion_reason()}) " + f"while sending {current_method} {self._credential_safe_url(current_url)}" + ) request_headers = self._headers(extra_headers) + call_kwargs = dict(request_kwargs) + if self._verify is not _VERIFY_UNSET and "verify" not in call_kwargs: + call_kwargs["verify"] = self._verify response = self.session.request( current_method, current_url, @@ -232,9 +533,19 @@ def _request_with_same_origin_redirects( headers=request_headers, timeout=timeout, allow_redirects=False, - **request_kwargs, + **call_kwargs, ) if self.verbose: + # For streamed responses, reading ``response.text`` would buffer + # the entire body here, defeating callers that stream via + # ``iter_content``/``iter_lines``; a followed container-log + # stream may never terminate. Log a placeholder and leave the + # body unread. + response_body = ( + "" + if request_kwargs.get("stream") + else response.text + ) log_http_exchange( self.logger, method=current_method, @@ -243,7 +554,7 @@ def _request_with_same_origin_redirects( request_body=current_json, response_status=response.status_code, response_headers=dict(response.headers), - response_body=response.text, + response_body=response_body, ) if response.status_code not in self._REDIRECT_STATUSES: return response @@ -254,15 +565,19 @@ def _request_with_same_origin_redirects( next_url = urljoin(response.url, location) if not self._same_origin(response.url, next_url): + # Close before raising so callers that catch this and fall back + # to another route (or a streamed open) do not leak the open + # streamed redirect response and its pooled connection. + try: + response.close() + except Exception: + pass raise requests.HTTPError( f"Refusing to follow cross-origin redirect from {response.url} to {next_url}", response=response, ) - try: - response.close() - except Exception: - pass + self._release_response(response) if response.status_code == 303 or ( response.status_code in {301, 302} and current_method not in {"GET", "HEAD"} ): @@ -276,6 +591,35 @@ def _request_with_same_origin_redirects( response=response, ) + @staticmethod + def _credential_safe_url(url: str) -> str: + """Render a URL for error text with every credential-bearing part removed. + + A same-origin redirect can land on a signed URL whose query, fragment, + or authority userinfo carries credentials, and this rendering flows + into CLI output and logs. Only scheme, host[:port], and path survive. + ``urlsplit`` rejects some malformed authorities (an unclosed IPv6 + bracket, for one), and ``.hostname``/``.port`` reject malformed ports, + so the authority is taken from the raw netloc and parse failures fall + back to a placeholder rather than letting error formatting raise. + """ + + try: + parts = urlsplit(url) + except ValueError: + return "" + netloc = parts.netloc.rpartition("@")[2] + return urlunsplit((parts.scheme, netloc, parts.path, "", "")) or "" + + @staticmethod + def _release_response(response: requests.Response) -> None: + """Return an abandoned response's connection to the pool.""" + + try: + response.close() + except Exception: + pass + @staticmethod def _same_origin(left: str, right: str) -> bool: left_parts = urlparse(left) @@ -357,17 +701,142 @@ def get_execution_details(self, execution_id: str) -> GetExecutionInfoResponse: self._enrich_execution_tree(details) return details + @staticmethod + def _make_stream_body_idle_unbounded(response: requests.Response) -> None: + """Clear the accepted response socket's timeout for quiet body reads. + + ``requests`` has no public API for using a finite response-header timeout + followed by an unbounded streamed-body timeout. Keep the locked + requests 2.34.2 / urllib3 2.7.0 transport access isolated here and + fail closed if their response layout changes. + """ + + raw = response.raw + socket = None + try: + socket = raw._fp.fp.raw._sock # type: ignore[attr-defined] + except AttributeError: + connection = getattr(raw, "connection", None) + socket = getattr(connection, "sock", None) + settimeout = getattr(socket, "settimeout", None) + if not callable(settimeout): + response.close() + raise requests.ConnectionError( + "opened log stream but could not disable the body read timeout" + ) + try: + settimeout(None) + except Exception as exc: + response.close() + raise requests.ConnectionError( + "opened log stream but could not disable the body read timeout" + ) from exc + def stream_execution_container_log(self, execution_id: str) -> requests.Response: - response = self._make_request( - "GET", - self._format_path( - "/api/executions/{id}/stream_container_log", - {"id": execution_id}, - ), - stream=True, + """Open the streaming container-log response for ``execution_id``. + + The endpoint delivers raw log lines over a long-lived chunked HTTP + response; the client streams those lines as-is and does no + event-protocol parsing. + + Establishing the stream (open + status check) follows a transient-error + retry budget: transport-open errors (connection/timeout) and retryable + 5xx responses are retried with exponential backoff before any line is + read. Same-origin redirect protection errors (cross-origin ``HTTPError`` + / ``TooManyRedirects``) are not transport blips and propagate + immediately. Once the stream is open the caller owns the response and + must close it; :meth:`iter_execution_container_log_lines` does that. + + The request uses finite ``(connect, read)`` timeouts of + ``(self.timeout, self.timeout)`` through receipt of the final response + headers. Once that response is accepted, its body socket alone is reset + to an unbounded idle timeout, because a healthy follow stream can stay + silent for as long as the container emits no output. + """ + + path = self._format_path( + "/api/executions/{id}/stream_container_log", + {"id": execution_id}, ) - response.raise_for_status() - return response + backoff = self._STREAM_OPEN_BACKOFF_SECONDS + last_exc: requests.RequestException | None = None + last_error_response: requests.Response | None = None + for attempt in range(1, self._MAX_STREAM_OPEN_ATTEMPTS + 1): + try: + response = self._make_request( + "GET", path, stream=True, timeout=(self.timeout, self.timeout) + ) + except (requests.HTTPError, requests.TooManyRedirects) as exc: + # Same-origin redirect guard errors carry the rejected streamed + # response and are intentionally not retried. No iterator ever + # receives that response, so close it before re-raising. + if exc.response is not None: + exc.response.close() + raise + except (requests.ConnectionError, requests.Timeout) as exc: + last_exc = exc + last_error_response = None + if attempt == self._MAX_STREAM_OPEN_ATTEMPTS: + break + self._sleep_for_stream_open_retry(backoff, attempt + 1, type(exc).__name__) + backoff *= 2.0 + continue + if response.status_code in self._RETRYABLE_STREAM_STATUSES: + response.close() + last_error_response = response + last_exc = None + if attempt == self._MAX_STREAM_OPEN_ATTEMPTS: + break + self._sleep_for_stream_open_retry( + backoff, attempt + 1, f"HTTP {response.status_code}" + ) + backoff *= 2.0 + continue + try: + response.raise_for_status() + except requests.HTTPError: + # Non-retryable status (e.g. 400/403/404): close the open + # streamed response before propagating so it is not leaked. + response.close() + raise + self._make_stream_body_idle_unbounded(response) + return response + if last_exc is not None: + raise last_exc + if last_error_response is not None: + last_error_response.raise_for_status() + # Defensive: every exhausted attempt records either a transport error + # (re-raised above) or a retryable-status response (raise_for_status + # always raises for those), so this cannot be reached. + raise RuntimeError( # pragma: no cover + "log stream open retries exhausted without a failure to re-raise" + ) + + def iter_execution_container_log_lines(self, execution_id: str) -> Iterable[str]: + """Return an iterator of decoded container-log lines for ``execution_id``. + + The stream is opened eagerly, so open failures (HTTP status or transport + errors) raise from this call rather than on first iteration; anything + raised while iterating is a drop of an already-open stream. The + underlying streaming response is always closed when iteration finishes + or the consumer stops early. + """ + + response = self.stream_execution_container_log(execution_id) + + def lines() -> Iterator[str]: + try: + # Decode whole ``bytes`` lines as UTF-8 explicitly rather than + # via ``decode_unicode=True``: requests' charset guessing falls + # back to latin-1 when the response declares no charset and + # would mojibake non-ASCII output. Whole-line decoding also + # reassembles multibyte sequences split across stream chunks. + for raw in response.iter_lines(): + yield raw.decode("utf-8", "replace") + finally: + response.close() + + return lines() def get_component_spec(self, digest: str) -> ComponentSpec: """Return a parsed domain component spec from the generated component endpoint.""" diff --git a/packages/tangle-cli/src/tangle_cli/dynamic_discovery_client.py b/packages/tangle-cli/src/tangle_cli/dynamic_discovery_client.py index d3a5582..3270919 100644 --- a/packages/tangle-cli/src/tangle_cli/dynamic_discovery_client.py +++ b/packages/tangle-cli/src/tangle_cli/dynamic_discovery_client.py @@ -18,6 +18,7 @@ ) from .api_transport import ( DEFAULT_TIMEOUT_SECONDS, + VerifyArgument, _normalize_base_url, default_base_url, request_operation, @@ -43,6 +44,7 @@ def __init__( auth_header: str | None = None, header: list[str] | str | None = None, timeout: float = DEFAULT_TIMEOUT_SECONDS, + verify: VerifyArgument = None, ) -> None: self.schema = schema self.base_url = _normalize_base_url(base_url or default_base_url()) @@ -51,6 +53,7 @@ def __init__( self.auth_header = auth_header self.header = _header_list(header) self.timeout = timeout + self.verify = verify self._operations = operation_map(schema) self._aliases = self._build_alias_map(self._operations) self._groups = self._build_groups(self._operations) @@ -66,6 +69,7 @@ def from_schema( auth_header: str | None = None, header: list[str] | str | None = None, timeout: float = DEFAULT_TIMEOUT_SECONDS, + verify: VerifyArgument = None, ) -> TangleDynamicDiscoveryClient: """Create a client from an already loaded OpenAPI schema.""" @@ -77,6 +81,7 @@ def from_schema( auth_header=auth_header, header=header, timeout=timeout, + verify=verify, ) @classmethod @@ -89,6 +94,7 @@ def from_cache( auth_header: str | None = None, header: list[str] | str | None = None, timeout: float = DEFAULT_TIMEOUT_SECONDS, + verify: VerifyArgument = None, ) -> TangleDynamicDiscoveryClient: """Create a client from the local schema cache without network access.""" @@ -107,6 +113,7 @@ def from_cache( auth_header=auth_header, header=header, timeout=timeout, + verify=verify, ) @classmethod @@ -119,6 +126,7 @@ def from_url( auth_header: str | None = None, header: list[str] | str | None = None, timeout: float = DEFAULT_TIMEOUT_SECONDS, + verify: VerifyArgument = None, ) -> TangleDynamicDiscoveryClient: """Fetch ``/openapi.json`` and create a client without writing the cache.""" @@ -129,6 +137,7 @@ def from_url( header=header, auth_header=auth_header, headers=headers, + verify=verify, ) return cls.from_schema( schema, @@ -138,6 +147,7 @@ def from_url( auth_header=auth_header, header=header, timeout=timeout, + verify=verify, ) @classmethod @@ -150,6 +160,7 @@ def from_cache_or_refresh( auth_header: str | None = None, header: list[str] | str | None = None, timeout: float = DEFAULT_TIMEOUT_SECONDS, + verify: VerifyArgument = None, ) -> TangleDynamicDiscoveryClient: """Create a client from cache, fetching and caching the schema on miss.""" @@ -162,6 +173,7 @@ def from_cache_or_refresh( header=header, auth_header=auth_header, headers=headers, + verify=verify, ) return cls.from_schema( schema, @@ -171,6 +183,7 @@ def from_cache_or_refresh( auth_header=auth_header, header=header, timeout=timeout, + verify=verify, ) @property @@ -197,6 +210,7 @@ def request(self, operation_name: str, **params: Any) -> httpx.Response: headers = {**self.headers, **dict(headers_override or {})} body = params.pop("body", None) timeout = params.pop("timeout", self.timeout) + verify = params.pop("verify", self.verify) return request_operation( operation, params, @@ -207,6 +221,7 @@ def request(self, operation_name: str, **params: Any) -> httpx.Response: headers=headers, body=body, timeout=timeout, + verify=verify, ) def call(self, operation_name: str, **params: Any) -> Any: diff --git a/packages/tangle-cli/src/tangle_cli/pipeline_run_manager.py b/packages/tangle-cli/src/tangle_cli/pipeline_run_manager.py index 24742fa..266082b 100644 --- a/packages/tangle-cli/src/tangle_cli/pipeline_run_manager.py +++ b/packages/tangle-cli/src/tangle_cli/pipeline_run_manager.py @@ -12,15 +12,17 @@ import copy import inspect import json +import math import re import time import uuid -from collections.abc import Callable +from collections.abc import Callable, Iterable from contextlib import AbstractContextManager, nullcontext from dataclasses import dataclass, field from pathlib import Path from typing import Any, Mapping +import requests import yaml from .handler import TangleCliHandler @@ -32,9 +34,42 @@ from .pipeline_run_search import PipelineRunSearch from .utils import dump_yaml -_TERMINAL_STATUSES = ("FAILED", "SYSTEM_ERROR", "CANCELLED", "CANCELED", "SKIPPED", "SUCCEEDED", "INVALID") -_ACTIVE_STATUSES = ("RUNNING", "CANCELLING", "CANCELING", "PENDING", "QUEUED") +# Precedence order for reducing a mixed terminal aggregate to a single status: +# failure terminals must precede the non-failure terminals SKIPPED and SUCCEEDED +# so a mixed aggregate (e.g. {SKIPPED, INVALID}) reduces to the failure instead +# of masking it (a fully-SKIPPED map still reduces to SKIPPED). +_TERMINAL_PRECEDENCE = ("FAILED", "SYSTEM_ERROR", "CANCELLED", "CANCELED", "INVALID", "SKIPPED", "SUCCEEDED") +# Membership set for is_terminal_status. +_TERMINAL_STATUSES = frozenset(_TERMINAL_PRECEDENCE) +# Nonterminal statuses outrank any terminal count when reducing an aggregate; +# WAITING_FOR_UPSTREAM/UNINITIALIZED are backend enum members and nonterminal. +_ACTIVE_STATUSES = ( + "RUNNING", + "CANCELLING", + "CANCELING", + "PENDING", + "QUEUED", + "WAITING_FOR_UPSTREAM", + "UNINITIALIZED", +) _FAILURE_EARLY_EXIT_STATUSES = ("FAILED", "SYSTEM_ERROR") +# The only terminals that are not per-task failures; every other terminal makes +# a root-execution `task-wait` exit non-zero. A single SKIPPED task (e.g. a +# conditional branch) is not a failure, unlike a run whose reduced status is +# SKIPPED at the run level. +_NON_FAILURE_TERMINAL_STATUSES = frozenset({"SKIPPED", "SUCCEEDED"}) +_TASK_FAILURE_STATUSES = _TERMINAL_STATUSES - _NON_FAILURE_TERMINAL_STATUSES +# A child execution's status is unknown until its state row is written; 404 +# means "wrong endpoint type" (container vs graph) and transient 5xx can be +# returned while the orchestrator is still settling. Both fall through to the +# next endpoint / report UNKNOWN so the wait loop polls again. +_RETRYABLE_EXEC_STATE_STATUSES = frozenset({404, 500, 502, 503, 504}) +# An unbounded task-wait (max_wait=None) has no deadline to stop retrying a +# failing root details fetch, so give up after this many consecutive server +# errors instead of hiding a persistent outage behind endless retries. +_MAX_UNBOUNDED_ROOT_FETCH_FAILURES = 3 +_UNKNOWN_TASK_STATUS = "UNKNOWN" +_ROOT_TASK_NAME = "root" _EXECUTION_STATE_TIMINGS_METADATA_KEY = "execution_state_timings" _EXECUTION_STATE_TIMING_MONOTONIC_METADATA_KEY = "_execution_state_timing_monotonic" _SUBMISSION_ID_ANNOTATION_KEY = "tangle-cli/submission-id" @@ -54,6 +89,56 @@ class AmbiguousPipelineRunRecoveryError(PipelineRunError): """Raised when submit recovery finds multiple runs for one submission id.""" +class TransientServerError(PipelineRunError): + """An HTTP 5xx failure that a deadline-bounded polling loop may retry. + + One-shot callers (e.g. ``task-status``) still fail fast on it like any + other PipelineRunError. + """ + + +def _transport_error(context: str, exc: requests.RequestException) -> PipelineRunError: + """Wrap a transport/HTTP failure as a clean CLI-surfaced PipelineRunError.""" + + response = getattr(exc, "response", None) + status_code = getattr(response, "status_code", None) + if status_code is not None: + message = f"{context}: request failed with HTTP {status_code}" + if status_code >= 500: + return TransientServerError(message) + return PipelineRunError(message) + return PipelineRunError(f"{context}: {exc}") + + +class TaskStatusesFailed(PipelineRunError): + """Raised when a root execution reaches terminal state with failed tasks.""" + + def __init__(self, root_execution_id: str, statuses: dict[str, str], failures: dict[str, str]): + self.root_execution_id = root_execution_id + self.statuses = statuses + self.failures = failures + summary = ", ".join(f"{name}={status}" for name, status in sorted(failures.items())) + super().__init__( + f"Root execution {root_execution_id} had {len(failures)} failed task(s): {summary}" + ) + + +def _describe_http_error(exc: requests.HTTPError) -> str: + """Summarize an HTTP status failure with the attempted request target.""" + + response = exc.response + if response is None: + return str(exc) + request = response.request + target = ( + f"{request.method} {request.url}" + if request is not None and request.url + else (response.url or "Tangle API") + ) + reason = f" {response.reason}" if response.reason else "" + return f"HTTP {response.status_code}{reason} for {target}" + + @dataclass class PipelineSubmitPayload: """Prepared submit payload state before calling ``pipeline_runs_create``. @@ -599,6 +684,14 @@ def fetch_logs(self, client: Any, execution_id: str) -> Any: """Hook for alternate TD log providers; OSS uses the Tangle API only.""" return client.executions_container_log(execution_id) + def stream_logs(self, client: Any, execution_id: str) -> Iterable[str]: + """Hook for alternate TD log providers; OSS streams via the Tangle API. + + Failures to open the stream must raise from this call; exceptions raised + while iterating are reported as mid-stream interruptions. + """ + return client.iter_execution_container_log_lines(execution_id) + @dataclass class PipelineRunManager(TangleCliHandler): @@ -739,6 +832,25 @@ def normalize_submit_body_in_place(body: dict[str, Any]) -> dict[str, Any]: def is_terminal_status(status: str | None) -> bool: return bool(status and status.upper() in _TERMINAL_STATUSES) + @staticmethod + def _validate_prepared_body(body: dict[str, Any]) -> dict[str, Any]: + """Fail fast on malformed prepared bodies, returning the componentRef. + + Locator-style bodies (a componentRef with ``name``/``digest`` and no + inline ``spec``) are valid; only the ``root_task``/``componentRef`` + mappings themselves are required. + """ + + root_task = body.get("root_task") + if not isinstance(root_task, dict): + raise PipelineRunError("Prepared submit body must contain a 'root_task' mapping") + component_ref = root_task.get("componentRef") + if not isinstance(component_ref, dict): + raise PipelineRunError( + "Prepared submit body 'root_task' must contain a 'componentRef' mapping" + ) + return component_ref + @staticmethod def status_counts_from_run(run: Mapping[str, Any]) -> dict[str, int]: stats = run.get("execution_status_stats") @@ -817,31 +929,40 @@ def status_from_counts(status_counts: Mapping[str, int]) -> str | None: for status in _ACTIVE_STATUSES: if int(status_counts.get(status, 0) or 0) > 0: return status - for status in _TERMINAL_STATUSES: + for status in _TERMINAL_PRECEDENCE: if int(status_counts.get(status, 0) or 0) > 0: return status return None + @staticmethod + def _status_from_container_state(state: Mapping[str, Any] | Any) -> str | None: + plain = PipelineRunManager.to_plain(state) + if isinstance(plain, Mapping): + status = plain.get("status") + if isinstance(status, str) and status: + return status + return None + + @classmethod + def _status_from_graph_state(cls, state: Mapping[str, Any] | Any) -> str | None: + # A nested graph/subpipeline execution carries no direct ``status``; its + # status is derived from child execution counts the same way run-level + # graph polling does. + direct = cls._status_from_container_state(state) + if direct is not None: + return direct + return cls.status_from_counts(cls.status_counts_from_graph_state(cls.to_plain(state))) + @staticmethod def status_from_run(run: Mapping[str, Any]) -> str | None: summary = run.get("execution_summary") - if isinstance(summary, Mapping) and summary.get("has_ended") is True: - stats = run.get("execution_status_stats") - if isinstance(stats, Mapping): - for status in ("FAILED", "SYSTEM_ERROR", "CANCELLED", "CANCELED"): - if int(stats.get(status, 0) or 0) > 0: - return status - if int(stats.get("SUCCEEDED", 0) or 0) > 0: - return "SUCCEEDED" - return "ENDED" stats = run.get("execution_status_stats") if isinstance(stats, Mapping): - for status in _ACTIVE_STATUSES: - if int(stats.get(status, 0) or 0) > 0: - return status - for status in _TERMINAL_STATUSES: - if int(stats.get(status, 0) or 0) > 0: - return status + status = PipelineRunManager.status_from_counts(stats) + if status is not None: + return status + if isinstance(summary, Mapping) and summary.get("has_ended") is True: + return "ENDED" return None @staticmethod @@ -1122,7 +1243,7 @@ def submit_prepared_body( notify_submit_error: bool = True, ) -> dict[str, Any]: self.normalize_submit_body_in_place(body) - pipeline_spec = body["root_task"]["componentRef"]["spec"] + pipeline_spec = self._validate_prepared_body(body).get("spec") submit_context = context or PipelineRunContext( pipeline_path=pipeline_path, start_time=time.time(), @@ -1261,6 +1382,211 @@ def graph_state_output(self, run_ids: list[str], *, timeout: float = 30.0) -> di def logs(self, execution_id: str) -> dict[str, Any]: return self.to_plain(self.hooks.fetch_logs(self.client, execution_id)) + def _exec_status(self, execution_id: str, *, deadline: float | None = None) -> str | None: + """Resolve a single execution's status, trying container then graph state. + + 404 means the execution is the other kind (container vs graph); a + transient 5xx means the state row is still being written. Both cases + fall through to the next endpoint and, if neither resolves, return + ``None`` so the caller reports UNKNOWN. Once past ``deadline`` (a + ``time.monotonic()`` timestamp) no further requests are issued. A + request already in flight remains subject to the client's transport + timeout and can finish after the wait deadline. + """ + + endpoints = ( + (self.client.executions_container_state, self._status_from_container_state), + (self.client.executions_graph_execution_state, self._status_from_graph_state), + ) + context = f"Resolving status for execution {execution_id}" + last_server_error: int | None = None + for endpoint, extract_status in endpoints: + if deadline is not None and time.monotonic() >= deadline: + return None + try: + state = endpoint(execution_id) + except requests.HTTPError as exc: + response = exc.response + if response is not None and response.status_code in _RETRYABLE_EXEC_STATE_STATUSES: + if response.status_code >= 500: + last_server_error = response.status_code + continue + raise _transport_error(context, exc) from exc + except requests.RequestException as exc: + raise _transport_error(context, exc) from exc + status = extract_status(state) + if status is not None: + return status + if last_server_error is not None: + # Without this a wait that times out reports UNKNOWN with no hint + # that the state API was failing the whole time. + self.logger.warn( + f"Status for execution {execution_id} is UNKNOWN: state endpoints " + f"kept failing with HTTP {last_server_error}" + ) + return None + + def task_statuses( + self, root_execution_id: str, *, deadline: float | None = None + ) -> dict[str, str]: + """Return a ``{task_name: status}`` map for a root execution. + + Walks ``child_task_execution_ids`` from the root execution details and + resolves each child's status. A leaf/root-only execution (no children) + reports a single ``{"root": status}`` entry; unresolved child statuses + are reported as ``UNKNOWN``. A missing root id (404) is a clean + not-found error; other transport/HTTP failures surface cleanly too. + Past ``deadline`` (a ``time.monotonic()`` timestamp), no new requests + are issued and the root is reported as ``UNKNOWN``. A request already + in flight remains subject to the client's transport timeout and can + finish after the wait deadline. + """ + + if deadline is not None and time.monotonic() >= deadline: + return {_ROOT_TASK_NAME: _UNKNOWN_TASK_STATUS} + try: + details = self.to_plain(self.client.executions_details(root_execution_id)) + except requests.HTTPError as exc: + response = exc.response + if response is not None and response.status_code == 404: + raise PipelineRunError(f"Root execution {root_execution_id} not found") from exc + raise _transport_error(f"Fetching root execution {root_execution_id}", exc) from exc + except requests.RequestException as exc: + raise _transport_error(f"Fetching root execution {root_execution_id}", exc) from exc + children = details.get("child_task_execution_ids") if isinstance(details, Mapping) else None + if not isinstance(children, Mapping) or not children: + return { + _ROOT_TASK_NAME: self._exec_status(root_execution_id, deadline=deadline) + or _UNKNOWN_TASK_STATUS + } + return { + str(task_name): self._child_status(execution_id, deadline=deadline) + for task_name, execution_id in children.items() + } + + def _child_status(self, execution_id: Any, *, deadline: float | None = None) -> str: + # A None/empty/whitespace-only child id has no execution to query; + # report UNKNOWN instead of polling a literal blank id forever. + child_id = str(execution_id).strip() if execution_id else "" + if not child_id: + return _UNKNOWN_TASK_STATUS + return self._exec_status(child_id, deadline=deadline) or _UNKNOWN_TASK_STATUS + + def wait_for_task_statuses( + self, + root_execution_id: str, + *, + max_wait: float | None = 1800.0, + poll_interval: float = 5.0, + allow_failure: bool = False, + ) -> dict[str, str]: + """Poll ``task_statuses`` until every task is terminal. + + Returns the final ``{task_name: status}`` map. Raises + ``TaskStatusesFailed`` when any task ends in a failure status unless + ``allow_failure`` is set, and ``PipelineRunError`` on timeout. + ``max_wait=None`` waits indefinitely. + """ + + self._validate_wait_params( + max_wait=max_wait, + poll_interval=poll_interval, + timeout_clock="monotonic", + allow_zero_poll_interval=False, + ) + deadline = None if max_wait is None else time.monotonic() + max_wait + root_fetch_failures = 0 + while True: + try: + statuses = self.task_statuses(root_execution_id, deadline=deadline) + except TransientServerError as exc: + # A 5xx on the root details fetch is tolerated like child-state + # 5xx (retried next poll) so one blip cannot abort a long wait. + # The deadline still bounds the retries; an unbounded wait gives + # up after a few consecutive failures instead of masking a + # persistent outage forever. + root_fetch_failures += 1 + if deadline is not None and time.monotonic() >= deadline: + raise + if deadline is None and root_fetch_failures >= _MAX_UNBOUNDED_ROOT_FETCH_FAILURES: + raise + if root_fetch_failures == 1: + # Warn once per outage, not once per poll, so a long outage + # does not flood the logs. + self.logger.warn(f"{exc}; retrying on the next poll") + self._sleep_before_next_poll(poll_interval, deadline) + continue + root_fetch_failures = 0 + if statuses and all(self.is_terminal_status(status) for status in statuses.values()): + return self._handle_terminal_task_statuses(root_execution_id, statuses, allow_failure) + if deadline is not None and time.monotonic() >= deadline: + pending = ", ".join( + f"{name}={status}" + for name, status in sorted(statuses.items()) + if not self.is_terminal_status(status) + ) + raise PipelineRunError( + f"Root execution {root_execution_id} still has non-terminal tasks " + f"after {max_wait:g}s: {pending}" + ) + self._sleep_before_next_poll(poll_interval, deadline) + + @staticmethod + def _sleep_before_next_poll(poll_interval: float, deadline: float | None) -> None: + if deadline is None: + time.sleep(poll_interval) + return + # Clamp the final sleep to the remaining budget so the loop times out + # at max_wait instead of overshooting by up to a poll interval. + time.sleep(min(poll_interval, max(0.0, deadline - time.monotonic()))) + + def _handle_terminal_task_statuses( + self, + root_execution_id: str, + statuses: dict[str, str], + allow_failure: bool, + ) -> dict[str, str]: + failures = { + name: status + for name, status in statuses.items() + if status.upper() in _TASK_FAILURE_STATUSES + } + if failures and not allow_failure: + raise TaskStatusesFailed(root_execution_id, statuses, failures) + return statuses + + def stream_logs(self, execution_id: str) -> Iterable[str]: + try: + lines = self.hooks.stream_logs(self.client, execution_id) + except requests.HTTPError as exc: + # A definitive non-2xx answer to the stream-open request (404 for a + # missing execution or endpoint, 403, ...): keep the status and the + # attempted target visible, since that is what the caller acts on. + raise PipelineRunError( + f"Failed to open log stream for execution {execution_id}: " + f"{_describe_http_error(exc)}" + ) from exc + except requests.RequestException as exc: + # Non-HTTP transport failures (connection refused, timeout) raise + # from the open call itself; surface them as a clean open failure. + raise PipelineRunError( + f"Failed to open log stream for execution {execution_id}: {exc}" + ) from exc + return self._relabel_stream_drops(lines, execution_id) + + @staticmethod + def _relabel_stream_drops(lines: Iterable[str], execution_id: str) -> Iterable[str]: + try: + yield from lines + except requests.RequestException as exc: + # The stream opened (the hook call succeeded), so a transport + # failure here is a drop of the live follow, possibly before the + # first line arrived. Surface it as an interruption rather than a + # fetch failure, which would wrongly imply the initial open failed. + raise PipelineRunError( + f"Log stream for execution {execution_id} was interrupted: {exc}" + ) from exc + def search_runs( self, *, @@ -1452,6 +1778,36 @@ def _poll_run_status( execution_state_timings=execution_state_timings, ) + @staticmethod + def _validate_wait_params( + *, + max_wait: float | None, + poll_interval: float, + timeout_clock: str, + allow_zero_poll_interval: bool, + ) -> None: + """Validate wait/poll parameters before submission. + + The CLI wait/run commands and the programmatic API share this path, so + error messages name both the parameter and its CLI flag. + """ + + # NaN/inf pass the sign checks below (NaN compares False, inf is + # "positive") and would become a never-firing deadline or a raw + # ValueError from time.sleep; reject them up front. An unbounded wait is + # spelled max_wait=None, not an explicit inf. + if max_wait is not None: + if not math.isfinite(max_wait): + raise PipelineRunError("max_wait (--max-wait) must be a finite number") + if max_wait < 0: + raise PipelineRunError("max_wait (--max-wait) must be non-negative") + if not math.isfinite(poll_interval): + raise PipelineRunError("poll_interval (--poll-interval) must be a finite number") + if poll_interval < 0 or (poll_interval == 0 and not allow_zero_poll_interval): + raise PipelineRunError("poll_interval (--poll-interval) must be positive") + if timeout_clock not in {"monotonic", "wall"}: + raise PipelineRunError("timeout_clock must be 'monotonic' or 'wall'") + def wait_for_completion( self, run_id: str, @@ -1467,12 +1823,12 @@ def wait_for_completion( wait_context = context or PipelineRunContext(run_id=run_id, start_time=time.time()) if exit_on_first_failure: wait_context.metadata["exit_on_first_failure"] = True - if max_wait is not None and max_wait < 0: - raise PipelineRunError("--max-wait must be non-negative") - if poll_interval < 0 or (poll_interval == 0 and not allow_zero_poll_interval): - raise PipelineRunError("--poll-interval must be positive") - if timeout_clock not in {"monotonic", "wall"}: - raise PipelineRunError("timeout_clock must be 'monotonic' or 'wall'") + self._validate_wait_params( + max_wait=max_wait, + poll_interval=poll_interval, + timeout_clock=timeout_clock, + allow_zero_poll_interval=allow_zero_poll_interval, + ) enforce_max_wait = max_wait is not None and self.hooks.should_enforce_max_wait(wait_context) poll_started_at = time.monotonic() deadline_now: Callable[[], float] = time.time if timeout_clock == "wall" else time.monotonic @@ -1751,6 +2107,15 @@ def _run_body_factory( if max_attempts < 1: raise PipelineRunError("max_attempts must be at least 1") + if wait: + # Validate wait/poll params up front so an invalid request never + # submits a run it can't wait on. + self._validate_wait_params( + max_wait=max_wait, + poll_interval=poll_interval, + timeout_clock=timeout_clock, + allow_zero_poll_interval=allow_zero_poll_interval, + ) last_error: Exception | None = None previous_context: PipelineRunContext | None = None attempts: list[PipelineRunContext] = [] @@ -1797,7 +2162,9 @@ def _run_body_factory( context.metadata["submission_id"] = submission_id if metadata_factory is not None: context.metadata.update(metadata_factory(attempt, previous_context, last_error)) - pipeline_spec = body.get("root_task", {}).get("componentRef", {}).get("spec") + # Validate before the tolerant spec extraction so a malformed body + # raises PipelineRunError instead of an AttributeError. + pipeline_spec = self._validate_prepared_body(body).get("spec") context.submit_body = body context.pipeline_spec = pipeline_spec if isinstance(pipeline_spec, dict) else None if context.pipeline_spec is not None: @@ -2076,6 +2443,111 @@ def body_factory( ) +def _build_default_manager(*, logger: Logger | None = None) -> PipelineRunManager: + """Construct a manager backed by the native ``TangleApiClient``. + + Imported lazily so the lightweight top-level package and local-only SDK + commands stay native-free. Credentials/base URL come from the standard + ``TangleApiClient`` environment defaults. + """ + + try: + from .client import TangleApiClient + except ModuleNotFoundError as exc: + # Catch both the top-level package and any missing submodule (e.g. a + # partially-installed ``tangle_api.generated``). + if exc.name is not None and (exc.name == "tangle_api" or exc.name.startswith("tangle_api.")): + raise PipelineRunError( + "Native generated Tangle API bindings are required to submit a prepared " + "body without an explicit client. Install tangle-cli[native], provide a " + "local tangle_api.generated package, or pass client=/manager=." + ) from exc + raise + + hooks = PipelineRunHooks(logger=logger) if logger is not None else PipelineRunHooks() + return PipelineRunManager(client=TangleApiClient(), hooks=hooks, logger=logger or hooks.logger) + + +def submit_and_wait_prepared_body( + body: dict[str, Any], + *, + manager: PipelineRunManager | None = None, + client: Any | None = None, + logger: Logger | None = None, + wait: bool = True, + max_wait: float | None = 600.0, + poll_interval: float = 10.0, + use_graph_state: bool = False, + allow_zero_poll_interval: bool = False, + timeout_clock: str = "monotonic", + exit_on_first_failure: bool = False, + metadata: dict[str, Any] | None = None, + submit_recovery_attempts: int = _DEFAULT_SUBMIT_RECOVERY_ATTEMPTS, +) -> dict[str, Any]: + """Submit an already-prepared API submit body and optionally wait for completion. + + Thin wrapper over :meth:`PipelineRunManager.run_prepared_body` for callers + that already hold a fully formed submit ``body`` (``{"root_task": {...}}``); + it reuses the existing submit/wait/poll and submit-recovery logic and never + mutates ``body``. Provide ``manager`` to reuse a configured manager, + ``client`` to wrap an existing API client, or neither to build a + :class:`tangle_cli.client.TangleApiClient` from the environment (requires + the native extra). ``logger`` applies only when this helper constructs the + manager (the ``client`` and default paths); a supplied ``manager`` keeps + its own configured logger and ``logger`` is ignored. + + Returns a dict with ``response``, ``run_id``, ``root_execution_id``, and + ``wait`` (when ``wait=True``); the result is JSON-serializable whenever the + API responses are. With ``wait=True``, a submit response carrying no run id + raises :class:`PipelineRunError`; use ``wait=False`` to inspect such + responses. + """ + + if manager is not None and client is not None: + raise PipelineRunError("Pass at most one of manager= or client=") + + if manager is None: + if client is not None: + hooks = PipelineRunHooks(logger=logger) if logger is not None else PipelineRunHooks() + manager = PipelineRunManager(client=client, hooks=hooks, logger=logger or hooks.logger) + else: + manager = _build_default_manager(logger=logger) + + raw = manager.run_prepared_body( + body, + wait=wait, + max_wait=max_wait, + poll_interval=poll_interval, + use_graph_state=use_graph_state, + allow_zero_poll_interval=allow_zero_poll_interval, + timeout_clock=timeout_clock, + exit_on_first_failure=exit_on_first_failure, + metadata=metadata, + submit_recovery_attempts=submit_recovery_attempts, + ) + + context: PipelineRunContext | None = raw.get("context") + result: dict[str, Any] = { + "response": raw.get("response"), + "run_id": context.run_id if context is not None else None, + "root_execution_id": context.root_execution_id if context is not None else None, + } + if "wait" in raw: + result["wait"] = raw["wait"] + elif wait: + # The underlying run skips waiting when the submit response carries no + # run id; surface that instead of silently returning without waiting. + raise PipelineRunError( + "Run was submitted but the submit response did not include a run id, " + "so completion cannot be awaited; use wait=False to inspect the response." + ) + return result + + +# Short public alias; ``submit_and_wait_prepared_body`` is the definition. +submit_and_wait = submit_and_wait_prepared_body + + def parse_key_value_entries(entries: list[str] | None) -> dict[str, str]: parsed: dict[str, str] = {} for entry in entries or []: @@ -2100,3 +2572,80 @@ def parse_json_or_key_values( result.update(loaded) result.update(parse_key_value_entries(entries)) return result + + +def secret_argument_value(secret_name: str) -> dict[str, Any]: + """Return the OSS dynamic-data payload for a Tangle secret reference.""" + + return {"dynamicData": {"secret": {"name": secret_name}}} + + +def parse_arg_secret_entries(entries: list[str] | None) -> dict[str, str]: + """Parse ``INPUT=SECRET_NAME`` secret references into a mapping. + + Trimmed input and secret names must be non-empty, and an input may not be + repeated. Raising here keeps validation before any file read or network + call in the submit path. + """ + + parsed: dict[str, str] = {} + for entry in entries or []: + if "=" not in entry: + raise PipelineRunError(f"Expected INPUT=SECRET_NAME for --arg-secret, got {entry!r}") + raw_input, raw_secret = entry.split("=", 1) + input_name = raw_input.strip() + secret_name = raw_secret.strip() + if not input_name or not secret_name: + raise PipelineRunError( + f"--arg-secret requires a non-empty input and secret name, got {entry!r}" + ) + if input_name in parsed: + raise PipelineRunError(f"Duplicate --arg-secret for input {input_name!r}") + parsed[input_name] = secret_name + return parsed + + +def normalize_arg_secret_config(value: Any) -> dict[str, str]: + """Normalize a config ``arg_secrets`` mapping of input -> secret name.""" + + if value is None: + return {} + if not isinstance(value, Mapping): + raise PipelineRunError("arg_secrets config must be a mapping of INPUT to SECRET_NAME") + normalized: dict[str, str] = {} + for raw_input, raw_secret in value.items(): + input_name = str(raw_input).strip() + if not isinstance(raw_secret, str): + raise PipelineRunError( + f"arg_secrets[{raw_input!r}] must be a secret name string, " + f"got {type(raw_secret).__name__}" + ) + secret_name = raw_secret.strip() + if not input_name or not secret_name: + raise PipelineRunError("arg_secrets entries require a non-empty input and secret name") + normalized[input_name] = secret_name + return normalized + + +def merge_secret_run_args( + run_args: dict[str, Any], + secret_names: Mapping[str, str], +) -> dict[str, Any]: + """Merge secret references into run args, rejecting input conflicts. + + An input may be supplied as a plain value (``--arg`` / ``--args-json`` / + config ``args``) or as a secret reference (``--arg-secret`` / config + ``arg_secrets``), never both. Overlap is rejected rather than silently + overwriting one form with the other. + """ + + conflicts = sorted(set(run_args) & set(secret_names)) + if conflicts: + raise PipelineRunError( + "Input(s) given as both a value and a secret reference: " + f"{', '.join(conflicts)}. Use only one of --arg/--args-json or --arg-secret per input." + ) + merged = dict(run_args) + for input_name, secret_name in secret_names.items(): + merged[input_name] = secret_argument_value(secret_name) + return merged diff --git a/packages/tangle-cli/src/tangle_cli/pipeline_runs_cli.py b/packages/tangle-cli/src/tangle_cli/pipeline_runs_cli.py index 2da0eb5..3df4df4 100644 --- a/packages/tangle-cli/src/tangle_cli/pipeline_runs_cli.py +++ b/packages/tangle-cli/src/tangle_cli/pipeline_runs_cli.py @@ -3,7 +3,9 @@ from __future__ import annotations import json +import os import pathlib +import sys from typing import Annotated, Any from cyclopts import App, Parameter @@ -32,6 +34,10 @@ PipelineRunError, PipelineRunHooks, PipelineRunManager, + TaskStatusesFailed, + merge_secret_run_args, + normalize_arg_secret_config, + parse_arg_secret_entries, parse_json_or_key_values, parse_key_value_entries, ) @@ -69,7 +75,9 @@ def _allow_all_hydration_for_args(args: ArgsContainer) -> bool: return bool(config.get("allow_all", False)) -def _api_client(args: ArgsContainer, *, cli_base_url: str | None, command_name: str) -> LazyTangleApiClient: +def _api_client( + args: ArgsContainer, *, cli_base_url: str | None, command_name: str, logger: Logger | None = None +) -> LazyTangleApiClient: return LazyTangleApiClient( base_url=args.base_url, token=args.token, @@ -77,12 +85,15 @@ def _api_client(args: ArgsContainer, *, cli_base_url: str | None, command_name: header=args.header, include_env_credentials=include_env_credentials_for_args(args, cli_base_url), command_name=command_name, + logger=logger, ) def _manager(args: ArgsContainer, *, cli_base_url: str | None, logger: Logger) -> PipelineRunManager: return PipelineRunManager( - client=_api_client(args, cli_base_url=cli_base_url, command_name="pipeline-run commands"), + client=_api_client( + args, cli_base_url=cli_base_url, command_name="pipeline-run commands", logger=logger + ), hooks=PipelineRunHooks( logger=logger, trusted_python_sources=_trusted_sources_for_args(args), @@ -117,7 +128,12 @@ def _run_annotation_action(config: str | None, cli_base_url: str | None, specs: raise SystemExit(str(exc)) from exc try: manager = AnnotationManager( - client=_api_client(args, cli_base_url=cli_base_url, command_name="pipeline-run annotation commands"), + client=_api_client( + args, + cli_base_url=cli_base_url, + command_name="pipeline-run annotation commands", + logger=logger, + ), logger=logger, ) print_json(fn(manager, args)) @@ -134,6 +150,17 @@ def pipeline_runs_submit( Parameter(help="Pipeline argument as KEY=VALUE. Repeat for multiple.", negative_iterable=()), ] = None, args_json: Annotated[str | None, Parameter(help="Pipeline arguments as a JSON object.")] = None, + arg_secret: Annotated[ + list[str] | None, + Parameter( + name="--arg-secret", + help=( + "Pipeline argument bound to a Tangle secret as INPUT=SECRET_NAME. " + "Repeat for multiple." + ), + negative_iterable=(), + ), + ] = None, annotation: Annotated[ list[str] | None, Parameter(help="Run annotation as KEY=VALUE. Repeat for multiple.", negative_iterable=()), @@ -185,6 +212,8 @@ def pipeline_runs_submit( "arg": (arg, None), "args_json": (args_json, None), "args_config": ("args", None, None, True), + "arg_secret": (arg_secret, None), + "arg_secrets_config": ("arg_secrets", None, None, True), "annotation": (annotation, None), "hydrate": (hydrate, True), "dry_run": (dry_run, None), @@ -197,8 +226,11 @@ def pipeline_runs_submit( } def action(manager: PipelineRunManager, args: ArgsContainer) -> dict[str, Any]: + run_args = parse_json_or_key_values(args.args_json or args.args_config, args.arg) + secret_names = normalize_arg_secret_config(args.arg_secrets_config) + secret_names.update(parse_arg_secret_entries(args.arg_secret)) kwargs = { - "run_args": parse_json_or_key_values(args.args_json or args.args_config, args.arg), + "run_args": merge_secret_run_args(run_args, secret_names), "annotations": parse_key_value_entries(args.annotation), "hydrate": bool(args.hydrate), "run_as": args.run_as, @@ -355,10 +387,95 @@ def pipeline_runs_wait( ) +@app.command(name="task-status") +def pipeline_runs_task_status( + root_execution_id: str | None = None, + *, + base_url: BaseUrlOption = None, + token: TokenOption = None, + auth_header: AuthHeaderOption = None, + header: HeaderOption = None, + config: ConfigOption = None, + log_type: LogTypeOption = "console", +) -> None: + """Print the {task_name: status} map for a root execution id, without polling. + + Walks the root execution's child task executions. A leaf/root-only execution + reports a single ``{"root": status}`` entry; unresolved children are UNKNOWN. + """ + specs = { + "root_execution_id": (root_execution_id,), + "log_type": (log_type, "console"), + **api_arg_specs(base_url=base_url, token=token, auth_header=auth_header, header=header), + } + _run_manager_action( + config, + base_url, + specs, + lambda manager, args: manager.task_statuses(args.root_execution_id), + ) + + +@app.command(name="task-wait") +def pipeline_runs_task_wait( + root_execution_id: str | None = None, + *, + max_wait: float = 1800.0, + poll_interval: float = 5.0, + allow_failure: bool = False, + base_url: BaseUrlOption = None, + token: TokenOption = None, + auth_header: AuthHeaderOption = None, + header: HeaderOption = None, + config: ConfigOption = None, + log_type: LogTypeOption = "console", +) -> None: + """Poll a root execution's task-status map until all tasks are terminal. + + Prints the final {task_name: status} map. Exit codes: 0 when every task + ends in a non-failure terminal status, 2 when any task fails and + --allow-failure is not set (the map is still printed), and non-zero on + timeout or API errors. + """ + specs = { + "root_execution_id": (root_execution_id,), + "max_wait": (max_wait, 1800.0), + "poll_interval": (poll_interval, 5.0), + "allow_failure": (allow_failure, False), + "log_type": (log_type, "console"), + **api_arg_specs(base_url=base_url, token=token, auth_header=auth_header, header=header), + } + + def action(manager: PipelineRunManager, args: ArgsContainer) -> dict[str, Any] | None: + try: + return manager.wait_for_task_statuses( + args.root_execution_id, + max_wait=float(args.max_wait), + poll_interval=float(args.poll_interval), + allow_failure=bool(args.allow_failure), + ) + except TaskStatusesFailed as exc: + # Print the full final map (failures retained on the exception drive + # the non-zero exit code) so succeeded/skipped tasks are not dropped. + print(str(exc), file=sys.stderr) + print_json(exc.statuses) + raise SystemExit(2) from exc + + _run_manager_action(config, base_url, specs, action) + + @app.command(name="logs") def pipeline_runs_logs( execution_id: str | None = None, *, + stream: Annotated[ + bool | None, + Parameter( + help="Follow the live log stream instead of fetching a one-shot snapshot. " + "The follow has no read timeout and stays open silently while the " + "container emits no output." + ), + ] = None, base_url: BaseUrlOption = None, token: TokenOption = None, auth_header: AuthHeaderOption = None, @@ -369,11 +486,24 @@ def pipeline_runs_logs( """Print Tangle API container logs for an execution id.""" specs = { "execution_id": (execution_id,), + "stream": (stream, None), "log_type": (log_type, "console"), **api_arg_specs(base_url=base_url, token=token, auth_header=auth_header, header=header), } def action(manager: PipelineRunManager, args: ArgsContainer) -> object: + if args.stream: + try: + for line in manager.stream_logs(args.execution_id): + print(line, flush=True) + except BrokenPipeError: + # The downstream reader closed the pipe (e.g. `... | head`). + # Point stdout at devnull so the interpreter's exit-time flush + # of the closed pipe cannot raise a second BrokenPipeError. + devnull_fd = os.open(os.devnull, os.O_WRONLY) + os.dup2(devnull_fd, sys.stdout.fileno()) + os.close(devnull_fd) + return None result = manager.logs(args.execution_id) if isinstance(result, dict) and isinstance(result.get("log_text"), str): print(result["log_text"], end="" if result["log_text"].endswith("\n") else "\n") diff --git a/packages/tangle-cli/src/tangle_cli/pipelines_cli.py b/packages/tangle-cli/src/tangle_cli/pipelines_cli.py index 3a30e57..4ce9006 100644 --- a/packages/tangle-cli/src/tangle_cli/pipelines_cli.py +++ b/packages/tangle-cli/src/tangle_cli/pipelines_cli.py @@ -232,6 +232,7 @@ def pipelines_hydrate( ), header=_header_entries(header, config_values), include_env_credentials=include_env_credentials, + logger=logger, ), ) except PipelineValidationError as exc: diff --git a/packages/tangle-cli/src/tangle_cli/published_components_cli.py b/packages/tangle-cli/src/tangle_cli/published_components_cli.py index de4f70a..733eb02 100644 --- a/packages/tangle-cli/src/tangle_cli/published_components_cli.py +++ b/packages/tangle-cli/src/tangle_cli/published_components_cli.py @@ -24,7 +24,7 @@ TokenOption, ) from .component_publisher import ComponentPublisher, deprecate_component -from .logger import logger_for_log_type +from .logger import Logger, logger_for_log_type def _client_from_options( @@ -35,6 +35,7 @@ def _client_from_options( header: list[str] | str | None = None, include_env_credentials: bool = True, command_name: str = "published-component commands", + logger: Logger | None = None, ) -> LazyTangleApiClient: """Create a lazy static client proxy for published-component commands. @@ -49,6 +50,7 @@ def _client_from_options( header=header, include_env_credentials=include_env_credentials, command_name=command_name, + logger=logger, ) @@ -97,6 +99,7 @@ def published_components_search( header=args.header, include_env_credentials=include_env_credentials_for_args(args, base_url), command_name="published-component commands", + logger=logger, ) if require_available := getattr(client, "require_available", None): require_available() @@ -163,6 +166,7 @@ def published_components_inspect( header=args.header, include_env_credentials=include_env_credentials_for_args(args, base_url), command_name="published-component commands", + logger=logger, ) if require_available := getattr(client, "require_available", None): require_available() @@ -219,6 +223,7 @@ def published_components_library( header=args.header, include_env_credentials=include_env_credentials_for_args(args, base_url), command_name="published-component commands", + logger=logger, ) if require_available := getattr(client, "require_available", None): require_available() @@ -288,6 +293,7 @@ def published_components_publish( header=args.header, include_env_credentials=include_env_credentials_for_args(args, base_url), command_name="published-component commands", + logger=logger, ) publisher = ComponentPublisher( dry_run=bool(args.dry_run), @@ -358,6 +364,7 @@ def published_components_deprecate( header=args.header, include_env_credentials=include_env_credentials_for_args(args, base_url), command_name="published-component commands", + logger=logger, ) result = deprecate_component( client, diff --git a/packages/tangle-cli/src/tangle_cli/secrets_cli.py b/packages/tangle-cli/src/tangle_cli/secrets_cli.py index d6c2da1..48ba9ff 100644 --- a/packages/tangle-cli/src/tangle_cli/secrets_cli.py +++ b/packages/tangle-cli/src/tangle_cli/secrets_cli.py @@ -58,7 +58,9 @@ app = App(name="secrets", help="Manage Tangle secrets.") -def _client(args: ArgsContainer, *, cli_base_url: str | None, command_name: str) -> LazyTangleApiClient: +def _client( + args: ArgsContainer, *, cli_base_url: str | None, command_name: str, logger: Logger | None = None +) -> LazyTangleApiClient: return LazyTangleApiClient( base_url=args.base_url, token=args.token, @@ -66,6 +68,7 @@ def _client(args: ArgsContainer, *, cli_base_url: str | None, command_name: str) header=args.header, include_env_credentials=include_env_credentials_for_args(args, cli_base_url), command_name=command_name, + logger=logger, ) @@ -79,7 +82,7 @@ def _run_secret_action( for args in load_args_or_exit(config, **specs): logger, finalize_logs = logger_for_log_type(getattr(args, "log_type", "console")) try: - client = _client(args, cli_base_url=cli_base_url, command_name="secret commands") + client = _client(args, cli_base_url=cli_base_url, command_name="secret commands", logger=logger) try: results.append(fn(client, args, logger)) except SecretValueError as exc: diff --git a/tests/test_api_cli.py b/tests/test_api_cli.py index c93a575..8993cc6 100644 --- a/tests/test_api_cli.py +++ b/tests/test_api_cli.py @@ -1,6 +1,7 @@ import importlib import json import sys +from unittest.mock import ANY import httpx import pytest @@ -460,6 +461,7 @@ def fake_client_from_options(**kwargs): "header": ["X-Config: yes"], "include_env_credentials": False, "command_name": "published-component commands", + "logger": ANY, } diff --git a/tests/test_artifacts_cli.py b/tests/test_artifacts_cli.py index b7b3ec1..f635497 100644 --- a/tests/test_artifacts_cli.py +++ b/tests/test_artifacts_cli.py @@ -5,6 +5,7 @@ import json import sys from typing import Any +from unittest.mock import ANY from tangle_cli import artifacts as artifacts_module from tangle_cli import artifacts_cli, cli @@ -80,6 +81,7 @@ def fake_get_artifacts(self, run_id: str, query: dict[str, Any]) -> dict[str, ob "header": ["X-Config: yes"], "include_env_credentials": False, "command_name": "artifact commands", + "logger": ANY, } ] assert get_calls == [ diff --git a/tests/test_client.py b/tests/test_client.py index 5456ac0..faa0322 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -1,13 +1,51 @@ from __future__ import annotations +import io +import json +import threading +import time +from contextlib import contextmanager +from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer from types import SimpleNamespace +from typing import Any from unittest.mock import MagicMock +import pytest +import requests + import tangle_cli.client as client_module from tangle_cli.client import TangleApiClient +from tangle_cli.logger import CaptureLogger from tangle_cli.models import ComponentInfo +def _response(payload: Any = None, status_code: int = 200) -> requests.Response: + r = requests.Response() + r.status_code = status_code + if payload is None: + r._content = b"" + else: + r._content = json.dumps(payload).encode("utf-8") + r.headers["Content-Type"] = "application/json" + r.request = requests.Request("GET", "https://api.test").prepare() + return r + + +class _FakeSession: + def __init__(self, responses: list[requests.Response | Exception] | None = None) -> None: + self.calls: list[dict[str, Any]] = [] + self.responses = responses or [] + + def request(self, method: str, url: str, **kwargs: Any) -> requests.Response: + self.calls.append({"method": method, "url": url, **kwargs}) + if self.responses: + next_response = self.responses.pop(0) + if isinstance(next_response, Exception): + raise next_response + return next_response + return _response({}) + + def test_find_existing_components_matches_exact_names_case_insensitively() -> None: client = TangleApiClient("https://api.test") client.list_published_component_infos = MagicMock( @@ -53,3 +91,1398 @@ def fail_from_dict(*args, **kwargs): assert client.get_run_pipeline_spec("run-1") is task_spec client.executions_details.assert_called_once_with("root-exec-1") + + +class _TimeoutSocket: + def __init__(self) -> None: + self.timeouts: list[float | None] = [] + + def settimeout(self, value: float | None) -> None: + self.timeouts.append(value) + + +def _tracked_stream_response(raw: Any, status_code: int = 200) -> requests.Response: + """A streaming-style response reading from ``raw`` that records ``close()`` in ``_closed``.""" + + if not hasattr(raw, "connection"): + raw.connection = SimpleNamespace(sock=_TimeoutSocket()) + r = requests.Response() + r.status_code = status_code + r.raw = raw + r.headers["Content-Type"] = "text/event-stream" + r.request = requests.Request("GET", "https://api.test").prepare() + r._closed = False + original_close = r.close + + def tracked_close() -> None: + r._closed = True + original_close() + + r.close = tracked_close # type: ignore[method-assign] + return r + + +def _stream_response(lines: list[bytes] | None = None, status_code: int = 200) -> requests.Response: + body = b"\n".join(lines) if lines else b"" + return _tracked_stream_response(io.BytesIO(body), status_code) + + +@contextmanager +def _local_http_server(handler: type[BaseHTTPRequestHandler]): + server = ThreadingHTTPServer(("127.0.0.1", 0), handler) + server.daemon_threads = True + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + yield f"http://127.0.0.1:{server.server_port}" + finally: + server.shutdown() + server.server_close() + thread.join(timeout=1) + + +def test_stream_open_header_stall_is_bounded() -> None: + class HeaderStallHandler(BaseHTTPRequestHandler): + def do_GET(self) -> None: + time.sleep(0.5) + + def log_message(self, _format: str, *args: Any) -> None: + pass + + with _local_http_server(HeaderStallHandler) as base_url: + client = TangleApiClient(base_url, timeout=0.1) + client._MAX_STREAM_OPEN_ATTEMPTS = 1 + started = time.monotonic() + with pytest.raises(requests.ReadTimeout): + client.stream_execution_container_log("exec-1") + elapsed = time.monotonic() - started + + assert 0.05 <= elapsed < 0.4 + + +def test_stream_quiet_body_read_is_unbounded_after_headers() -> None: + class QuietBodyHandler(BaseHTTPRequestHandler): + protocol_version = "HTTP/1.1" + + def do_GET(self) -> None: + self.send_response(200) + self.send_header("Content-Length", "5") + self.end_headers() + self.wfile.flush() + time.sleep(0.25) + self.wfile.write(b"line\n") + self.wfile.flush() + + def log_message(self, _format: str, *args: Any) -> None: + pass + + with _local_http_server(QuietBodyHandler) as base_url: + client = TangleApiClient(base_url, timeout=0.1) + client._MAX_STREAM_OPEN_ATTEMPTS = 1 + lines = list(client.iter_execution_container_log_lines("exec-1")) + + assert lines == ["line"] + + +def test_stream_execution_container_log_yields_lines_and_closes() -> None: + stream = _stream_response([b"line-1", b"line-2", b"line-3"]) + session = _FakeSession([stream]) + client = TangleApiClient("https://api.test", session=session) + + lines = list(client.iter_execution_container_log_lines("exec-1")) + + assert lines == ["line-1", "line-2", "line-3"] + assert stream._closed is True + assert session.calls[0]["url"] == "https://api.test/api/executions/exec-1/stream_container_log" + assert session.calls[0]["stream"] is True + # Opening, including the response headers, remains bounded. Only the + # accepted response socket is changed to unbounded idle for body reads. + assert session.calls[0]["timeout"] == (client.timeout, client.timeout) + assert stream.raw.connection.sock.timeouts == [None] + + +def test_stream_open_closes_when_body_timeout_cannot_be_disabled() -> None: + stream = _tracked_stream_response(io.BytesIO(b"line\n")) + stream.raw.connection = SimpleNamespace(sock=object()) + client = TangleApiClient("https://api.test", session=_FakeSession([stream])) + + with pytest.raises( + requests.ConnectionError, + match="opened log stream but could not disable the body read timeout", + ): + client.stream_execution_container_log("exec-1") + + assert stream._closed is True + + +def test_stream_execution_container_log_closes_on_early_break() -> None: + stream = _stream_response([b"a", b"b", b"c"]) + session = _FakeSession([stream]) + client = TangleApiClient("https://api.test", session=session) + + gen = client.iter_execution_container_log_lines("exec-1") + assert next(iter(gen)) == "a" + gen.close() # type: ignore[union-attr] + + assert stream._closed is True + + +def test_stream_open_retries_transient_status_then_succeeds(monkeypatch) -> None: + sleeps: list[float] = [] + monkeypatch.setattr("tangle_cli.client.time.sleep", sleeps.append) + bad = _stream_response(status_code=503) + ok = _stream_response([b"recovered"]) + session = _FakeSession([bad, ok]) + logger = CaptureLogger() + client = TangleApiClient("https://api.test", session=session, logger=logger) + + lines = list(client.iter_execution_container_log_lines("exec-1")) + + assert lines == ["recovered"] + assert bad._closed is True + assert sleeps == [1.0] + assert len(session.calls) == 2 + # Every stream-open retry sleep is announced through the client logger. + assert "transient HTTP 503 opening log stream; retrying in 1.0s (attempt 2/7)" in ( + logger.get_logs() or "" + ) + + +def test_stream_open_retries_transport_error_then_succeeds(monkeypatch) -> None: + sleeps: list[float] = [] + monkeypatch.setattr("tangle_cli.client.time.sleep", sleeps.append) + ok = _stream_response([b"after-blip"]) + calls = {"n": 0} + + class FlakySession(_FakeSession): + def request(self, method: str, url: str, **kwargs: Any) -> requests.Response: + calls["n"] += 1 + if calls["n"] == 1: + raise requests.ConnectionError("transient transport blip") + return ok + + client = TangleApiClient("https://api.test", session=FlakySession()) + + lines = list(client.iter_execution_container_log_lines("exec-1")) + + assert lines == ["after-blip"] + assert calls["n"] == 2 + assert sleeps == [1.0] + + +def test_stream_open_backoff_doubles_and_is_capped(monkeypatch) -> None: + sleeps: list[float] = [] + monkeypatch.setattr("tangle_cli.client.time.sleep", sleeps.append) + attempts = TangleApiClient._MAX_STREAM_OPEN_ATTEMPTS + session = _FakeSession([_stream_response(status_code=503) for _ in range(attempts)]) + logger = CaptureLogger() + client = TangleApiClient("https://api.test", session=session, logger=logger) + + with pytest.raises(requests.HTTPError): + client.stream_execution_container_log("exec-1") + + assert sleeps == [1.0, 2.0, 4.0, 8.0, 16.0, 30.0] + # The last retry announces the final attempt; the exhausted 7th attempt + # raises without announcing an 8th. + logs = logger.get_logs() or "" + assert "(attempt 7/7)" in logs + assert "8/7" not in logs + + +def test_stream_open_raises_non_retryable_status_immediately(monkeypatch) -> None: + sleeps: list[float] = [] + monkeypatch.setattr("tangle_cli.client.time.sleep", sleeps.append) + bad = _stream_response(status_code=404) + session = _FakeSession([bad]) + client = TangleApiClient("https://api.test", session=session) + + with pytest.raises(requests.HTTPError): + client.stream_execution_container_log("exec-1") + + # The streamed response must be closed before the non-retryable error + # propagates so the open connection is not leaked. + assert bad._closed is True + assert sleeps == [] + assert len(session.calls) == 1 + + +def test_stream_open_exhausts_retries_and_raises_last_status(monkeypatch) -> None: + monkeypatch.setattr("tangle_cli.client.time.sleep", lambda _seconds: None) + attempts = TangleApiClient._MAX_STREAM_OPEN_ATTEMPTS + session = _FakeSession([_stream_response(status_code=502) for _ in range(attempts)]) + client = TangleApiClient("https://api.test", session=session) + + with pytest.raises(requests.HTTPError) as exc_info: + client.stream_execution_container_log("exec-1") + + assert exc_info.value.response.status_code == 502 + assert len(session.calls) == attempts + + +def test_stream_open_exhausts_retries_and_raises_last_transport_error(monkeypatch) -> None: + monkeypatch.setattr("tangle_cli.client.time.sleep", lambda _seconds: None) + + class AlwaysFailingSession(_FakeSession): + def request(self, method: str, url: str, **kwargs: Any) -> requests.Response: + raise requests.ConnectionError("permanent transport failure") + + client = TangleApiClient("https://api.test", session=AlwaysFailingSession()) + + with pytest.raises(requests.ConnectionError, match="permanent transport failure"): + client.stream_execution_container_log("exec-1") + + +def test_stream_open_cross_origin_redirect_is_not_retried(monkeypatch) -> None: + sleeps: list[float] = [] + monkeypatch.setattr("tangle_cli.client.time.sleep", sleeps.append) + redirect = _stream_response(status_code=307) + redirect.url = "https://api.test/api/executions/exec-1/stream_container_log" + redirect.headers["Location"] = "https://attacker.example/leak" + session = _FakeSession([redirect]) + client = TangleApiClient("https://api.test", session=session) + + with pytest.raises(requests.HTTPError, match="cross-origin redirect") as exc_info: + client.stream_execution_container_log("exec-1") + + # Same-origin redirect protection must propagate immediately, not be retried. + assert sleeps == [] + assert len(session.calls) == 1 + # The rejected streamed response is attached to the guard error and no + # iterator ever receives it, so it must be closed before the error + # propagates to avoid leaking the open connection. + assert exc_info.value.response is redirect + assert redirect._closed is True + + +def test_stream_open_too_many_redirects_is_not_retried(monkeypatch) -> None: + sleeps: list[float] = [] + monkeypatch.setattr("tangle_cli.client.time.sleep", sleeps.append) + + def same_origin_redirect() -> requests.Response: + r = _stream_response(status_code=307) + r.url = "https://api.test/api/executions/exec-1/stream_container_log" + r.headers["Location"] = "/api/executions/exec-1/stream_container_log" + return r + + redirect_calls = TangleApiClient._MAX_REDIRECTS + 1 + responses = [same_origin_redirect() for _ in range(redirect_calls)] + session = _FakeSession(list(responses)) + client = TangleApiClient("https://api.test", session=session) + + with pytest.raises(requests.TooManyRedirects) as exc_info: + client.stream_execution_container_log("exec-1") + + # One stream-open attempt that exhausts redirects; no retry of the open. + assert sleeps == [] + assert len(session.calls) == redirect_calls + # Every streamed redirect response must be closed; the final one is + # attached to the guard error and must not leak. + assert all(r._closed is True for r in responses) + assert exc_info.value.response is responses[-1] + + +def test_stream_open_verbose_does_not_read_streamed_body(monkeypatch) -> None: + monkeypatch.setenv("TANGLE_VERBOSE", "1") + stream = _stream_response([b"line-1", b"line-2"]) + text_reads: list[int] = [] + original_text = type(stream).text + + def tracked_text(self: requests.Response) -> str: + text_reads.append(1) + return original_text.fget(self) # type: ignore[attr-defined] + + monkeypatch.setattr(type(stream), "text", property(tracked_text)) + logger = CaptureLogger() + session = _FakeSession([stream]) + client = TangleApiClient("https://api.test", session=session, logger=logger) + + response = client.stream_execution_container_log("exec-1") + + # Verbose logging must not drain the streamed body before the caller can + # iterate it; the log stream stays readable. + assert text_reads == [] + assert response._closed is False + assert list(response.iter_lines()) == [b"line-1", b"line-2"] + logs = logger.get_logs() or "" + assert "" in logs + + +def test_rate_limit_retry_closes_streamed_response_before_sleep(monkeypatch) -> None: + closed_at_sleep: list[bool] = [] + rate_limited = _stream_response(status_code=429) + rate_limited.headers["Retry-After"] = "0" + ok = _stream_response([b"recovered"]) + + def tracking_sleep(_seconds: float) -> None: + # Record whether the 429 stream is already closed when the rate-limit + # sleep runs; it must not be held open during the sleep. + closed_at_sleep.append(rate_limited._closed) + + monkeypatch.setattr("tangle_cli.client.time.sleep", tracking_sleep) + closed_at_retry: list[bool] = [] + + class TrackingSession(_FakeSession): + def request(self, method: str, url: str, **kwargs: Any) -> requests.Response: + # Record whether the prior 429 stream was already closed by the + # time the successful retry is issued. + if self.calls: + closed_at_retry.append(rate_limited._closed) + return super().request(method, url, **kwargs) + + session = TrackingSession([rate_limited, ok]) + client = TangleApiClient("https://api.test", session=session) + + response = client.stream_execution_container_log("exec-1") + + assert response is ok + assert len(session.calls) == 2 + # The intermediate 429 streamed response must be closed before sleeping and + # before the retry is issued. + assert closed_at_sleep == [True] + assert closed_at_retry == [True] + assert rate_limited._closed is True + assert list(response.iter_lines()) == [b"recovered"] + + +def test_auth_refresh_closes_streamed_response_before_retry() -> None: + unauthorized = _stream_response(status_code=401) + ok = _stream_response([b"authorized"]) + closed_at_refresh: list[bool] = [] + closed_at_retry: list[bool] = [] + + class RefreshingClient(TangleApiClient): + def __init__(self, *args: Any, **kwargs: Any) -> None: + super().__init__(*args, **kwargs) + self.refreshes = 0 + + def _refresh_auth(self) -> None: + self.refreshes += 1 + # On the auth-refresh triggered by the 401, the streamed 401 + # response must already be closed (not held open during refresh). + if self.refreshes == 2: + closed_at_refresh.append(unauthorized._closed) + self.headers["Authorization"] = f"Bearer refreshed-{self.refreshes}" + + class TrackingSession(_FakeSession): + def request(self, method: str, url: str, **kwargs: Any) -> requests.Response: + # Record whether the prior 401 stream was already closed by the + # time the successful retry is issued. + if self.calls: + closed_at_retry.append(unauthorized._closed) + return super().request(method, url, **kwargs) + + session = TrackingSession([unauthorized, ok]) + client = RefreshingClient("https://api.test", session=session) + + response = client._make_request("GET", "/api/users/me", stream=True) + + assert response is ok + assert client.refreshes == 2 + assert len(session.calls) == 2 + # The intermediate 401 streamed response must be closed before the auth + # refresh and before the retry is issued. + assert closed_at_refresh == [True] + assert closed_at_retry == [True] + assert unauthorized._closed is True + # The successful retry stream remains open and readable for the caller. + assert response._closed is False + assert list(response.iter_lines()) == [b"authorized"] + + +def test_stream_open_synthetic_http_error_from_make_request_is_not_retried(monkeypatch) -> None: + sleeps: list[float] = [] + monkeypatch.setattr("tangle_cli.client.time.sleep", sleeps.append) + client = TangleApiClient("https://api.test", session=_FakeSession()) + calls = {"n": 0} + + def fake_make_request(*args: Any, **kwargs: Any) -> requests.Response: + calls["n"] += 1 + raise requests.HTTPError("redirect guard tripped") + + monkeypatch.setattr(client, "_make_request", fake_make_request) + + with pytest.raises(requests.HTTPError, match="redirect guard tripped"): + client.stream_execution_container_log("exec-1") + + assert sleeps == [] + assert calls["n"] == 1 + + +class _ScriptedRaw: + """A raw stream whose ``read`` replays scripted byte chunks/exceptions. + + Each ``read`` returns the next queued ``bytes`` chunk verbatim (ignoring the + requested size, so a multi-byte char can be split across reads) or raises a + queued exception, modelling a mid-stream transport failure. + """ + + def __init__(self, chunks: list[bytes | Exception]) -> None: + self._chunks = list(chunks) + + def read(self, _size: int = -1) -> bytes: + if not self._chunks: + return b"" + item = self._chunks.pop(0) + if isinstance(item, Exception): + raise item + return item + + def close(self) -> None: + self._chunks.clear() + + +def _scripted_stream_response(chunks: list[bytes | Exception]) -> requests.Response: + return _tracked_stream_response(_ScriptedRaw(chunks)) + + +def test_stream_decodes_multibyte_char_split_across_chunks() -> None: + # "café" and "日本語" each contain multi-byte UTF-8 sequences; feeding the + # stream one byte at a time splits those sequences across chunk reads. + # Decoding whole lines as UTF-8 must reassemble them rather than yield + # replacement characters or mojibake. + payload = "café\n日本語\n".encode("utf-8") + stream = _scripted_stream_response([payload[i : i + 1] for i in range(len(payload))]) + client = TangleApiClient("https://api.test", session=_FakeSession([stream])) + + lines = list(client.iter_execution_container_log_lines("exec-1")) + + assert lines == ["café", "日本語"] + assert stream._closed is True + + +def test_stream_read_error_mid_iteration_propagates_and_closes() -> None: + # Once the stream is open the retry budget is spent; a transport failure + # during iteration must propagate (not be retried or swallowed) and the + # streamed response must still be closed by the iterator's finally block. + stream = _scripted_stream_response( + [b"line-1\n", requests.exceptions.ChunkedEncodingError("connection broken mid-stream")] + ) + client = TangleApiClient("https://api.test", session=_FakeSession([stream])) + + gen = iter(client.iter_execution_container_log_lines("exec-1")) + assert next(gen) == "line-1" + with pytest.raises(requests.exceptions.ChunkedEncodingError, match="connection broken mid-stream"): + next(gen) + + assert stream._closed is True + + +def test_get_retries_transient_5xx_then_succeeds(monkeypatch) -> None: + sleeps: list[float] = [] + monkeypatch.setattr("tangle_cli.client.time.sleep", sleeps.append) + ok = _response({"ok": True}) + session = _FakeSession([_response(status_code=503), _response(status_code=500), ok]) + client = TangleApiClient("https://api.test", session=session) + + result = client._make_request("GET", "/api/test") + + assert result is ok + assert len(session.calls) == 3 + assert sleeps == [1.0, 2.0] + + +def test_get_closes_intermediate_5xx_responses_before_retrying(monkeypatch) -> None: + events: list[str] = [] + + def tracking_response(status_code: int, marker: str) -> requests.Response: + r = _response(status_code=status_code) + r.close = lambda: events.append(f"close-{marker}") # type: ignore[method-assign] + return r + + monkeypatch.setattr("tangle_cli.client.time.sleep", lambda _delay: events.append("sleep")) + session = _FakeSession( + [tracking_response(503, "1"), tracking_response(500, "2"), tracking_response(200, "3")] + ) + client = TangleApiClient("https://api.test", session=session) + + client._make_request("GET", "/api/test") + + # Intermediate 5xx are closed before each retry; the returned one is left open. + assert events == ["close-1", "sleep", "close-2", "sleep"] + + +def test_get_retries_transport_error_then_succeeds(monkeypatch) -> None: + sleeps: list[float] = [] + monkeypatch.setattr("tangle_cli.client.time.sleep", sleeps.append) + ok = _response({"ok": True}) + session = _FakeSession( + [ + requests.ConnectionError("connection reset"), + requests.Timeout("read timed out"), + requests.exceptions.ChunkedEncodingError("incomplete chunked read"), + requests.exceptions.ContentDecodingError("failed to decode gzip stream"), + ok, + ] + ) + client = TangleApiClient("https://api.test", session=session) + + result = client._make_request("GET", "/api/test") + + assert result is ok + assert len(session.calls) == 5 + assert sleeps == [1.0, 2.0, 4.0, 8.0] + + +def test_get_raises_after_exhausting_transport_retries(monkeypatch) -> None: + monkeypatch.setattr("tangle_cli.client.time.sleep", lambda _delay: None) + budget = TangleApiClient._MAX_GET_RETRIES + final_attempt_error = requests.exceptions.ChunkedEncodingError("final permitted attempt") + surplus = [requests.ConnectionError("never reached") for _ in range(3)] + queued = [requests.ConnectionError("blip") for _ in range(budget)] + [final_attempt_error] + surplus + assert len(queued) > budget + 1 + session = _FakeSession(queued) + client = TangleApiClient("https://api.test", session=session) + + with pytest.raises(requests.exceptions.ChunkedEncodingError) as exc_info: + client._make_request("GET", "/api/test") + + assert exc_info.value is final_attempt_error + assert len(session.calls) == budget + 1 + assert len(session.responses) == len(surplus) + + +def test_get_returns_final_5xx_after_exhausting_status_retries(monkeypatch) -> None: + closed: list[str] = [] + + def tracking_5xx(marker: str) -> requests.Response: + r = _response(status_code=503) + r.close = lambda: closed.append(marker) # type: ignore[method-assign] + return r + + monkeypatch.setattr("tangle_cli.client.time.sleep", lambda _delay: None) + budget = TangleApiClient._MAX_GET_RETRIES + errors = [tracking_5xx(str(i)) for i in range(budget + 1)] + trailing_ok = _response({"ok": True}) + session = _FakeSession([*errors, trailing_ok]) + client = TangleApiClient("https://api.test", session=session) + + result = client._make_request("GET", "/api/test") + + assert result is errors[budget] + assert len(session.calls) == budget + 1 + assert closed == [str(i) for i in range(budget)] + assert session.responses == [trailing_ok] + + +def test_post_is_not_retried_on_transient_5xx(monkeypatch) -> None: + sleeps: list[float] = [] + monkeypatch.setattr("tangle_cli.client.time.sleep", sleeps.append) + server_error = _response(status_code=503) + session = _FakeSession([server_error, _response({"ok": True})]) + client = TangleApiClient("https://api.test", session=session) + + result = client._make_request("POST", "/api/pipeline_runs/", json_data={"a": 1}) + + assert result is server_error + assert len(session.calls) == 1 + assert sleeps == [] + + +def test_streamed_get_bypasses_transient_retry(monkeypatch) -> None: + sleeps: list[float] = [] + monkeypatch.setattr("tangle_cli.client.time.sleep", sleeps.append) + server_error = _response(status_code=503) + session = _FakeSession([server_error, _response({"ok": True})]) + client = TangleApiClient("https://api.test", session=session) + + result = client._make_request("GET", "/api/logs", stream=True) + + assert result is server_error + assert len(session.calls) == 1 + assert sleeps == [] + + +def test_transient_retry_decision_is_method_case_insensitive(monkeypatch) -> None: + monkeypatch.setattr("tangle_cli.client.time.sleep", lambda _delay: None) + + def call(method: str) -> tuple[requests.Response, int]: + ok = _response({"ok": True}) + session = _FakeSession([_response(status_code=503), ok]) + client = TangleApiClient("https://api.test", session=session) + budget = client_module._RetryBudget( + client._MAX_GET_RETRIES + 1, + client._MAX_PHYSICAL_SENDS, + client_module.time.monotonic() + client._MAX_RETRY_ELAPSED_SECONDS, + ) + result = client._request_with_transient_retries( + method, + "https://api.test/api/test", + params=None, + json_data=None, + extra_headers=None, + timeout=client.timeout, + request_kwargs={}, + budget=budget, + ) + return result, len(session.calls) + + get_result, get_calls = call("get") + assert get_result.status_code == 200 + assert get_calls == 2 + + post_result, post_calls = call("post") + assert post_result.status_code == 503 + assert post_calls == 1 + + +def test_get_retries_proxy_errors(monkeypatch) -> None: + sleeps: list[float] = [] + monkeypatch.setattr("tangle_cli.client.time.sleep", sleeps.append) + ok = _response({"ok": True}) + session = _FakeSession( + [ + requests.exceptions.ProxyError("proxy refused"), + ok, + ] + ) + client = TangleApiClient("https://api.test", session=session) + + result = client._make_request("GET", "/api/test") + + assert result is ok + assert len(session.calls) == 2 + assert sleeps == [1.0] + + +def test_get_does_not_retry_ssl_errors(monkeypatch) -> None: + sleeps: list[float] = [] + monkeypatch.setattr("tangle_cli.client.time.sleep", sleeps.append) + error = requests.exceptions.SSLError("certificate verify failed") + session = _FakeSession([error, _response({"ok": True})]) + client = TangleApiClient("https://api.test", session=session) + + with pytest.raises(requests.exceptions.SSLError) as exc_info: + client._make_request("GET", "/api/test") + + assert exc_info.value is error + assert len(session.calls) == 1 + assert sleeps == [] + + +def test_get_transient_and_rate_limit_retry_layers_compose(monkeypatch) -> None: + sleeps: list[float] = [] + monkeypatch.setattr("tangle_cli.client.time.sleep", sleeps.append) + ok = _response({"ok": True}) + session = _FakeSession( + [ + _response(status_code=503), + _response(status_code=429), + _response(status_code=503), + ok, + ] + ) + client = TangleApiClient("https://api.test", session=session) + + result = client._make_request("GET", "/api/test") + + assert result is ok + assert len(session.calls) == 4 + # transient 1.0s, rate-limit 1.0s (no Retry-After), fresh transient 1.0s + assert sleeps == [1.0, 1.0, 1.0] + + +def test_get_retry_sleeps_are_capped_and_announced_without_verbose(monkeypatch) -> None: + sleeps: list[float] = [] + monkeypatch.setattr("tangle_cli.client.time.sleep", sleeps.append) + budget = TangleApiClient._MAX_GET_RETRIES + session = _FakeSession([_response(status_code=503) for _ in range(budget + 1)]) + logger = CaptureLogger() + client = TangleApiClient("https://api.test", session=session, logger=logger) + + result = client._make_request("GET", "/api/test") + + assert result.status_code == 503 + assert sleeps == [1.0, 2.0, 4.0, 8.0, 16.0, 30.0] # final sleep capped, not 32.0 + messages = (logger.get_logs() or "").splitlines() + assert len(messages) == budget + assert all(m.startswith("transient HTTP 503 on GET; retrying in ") for m in messages) + assert messages[-1] == "transient HTTP 503 on GET; retrying in 30.0s (attempt 7/7)" + + +def test_get_retries_are_silent_on_default_non_verbose_client(monkeypatch, capsys) -> None: + # A non-verbose client built without a logger stays silent; callers that + # want retry announcements pass a logger (as the CLI command layer does). + monkeypatch.delenv("TANGLE_VERBOSE", raising=False) + monkeypatch.setattr("tangle_cli.client.time.sleep", lambda _delay: None) + session = _FakeSession([_response(status_code=503), _response({"ok": True})]) + client = TangleApiClient("https://api.test", session=session) + + result = client._make_request("GET", "/api/test") + + assert result.status_code == 200 + captured = capsys.readouterr() + assert captured.out == "" + assert captured.err == "" + + +def test_shared_budget_caps_total_requests_across_transient_and_rate_limit(monkeypatch) -> None: + # Interleaved 503/429 responses must not let the rate-limit layer hand the + # transient layer a fresh budget each round: the total physical request + # count is bounded by the single shared budget, not their product. + monkeypatch.setattr("tangle_cli.client.time.sleep", lambda _delay: None) + budget = TangleApiClient._MAX_GET_RETRIES + # Far more responses than the budget allows, alternating retryable states. + session = _FakeSession([_response(status_code=503 if i % 2 == 0 else 429) for i in range(40)]) + client = TangleApiClient("https://api.test", session=session) + + result = client._make_request("GET", "/api/test") + + # Exactly the advertised budget of physical requests is spent, then the last + # response surfaces for the caller's raise_for_status (no amplification). + assert len(session.calls) == budget + 1 + assert result.status_code in {503, 429} + + +def test_auth_refresh_shares_transient_retry_budget(monkeypatch) -> None: + # A 401 that triggers an auth refresh must continue on the same budget + # rather than starting a fresh transient-retry round. + monkeypatch.setattr("tangle_cli.client.time.sleep", lambda _delay: None) + + class RefreshingClient(TangleApiClient): + def __init__(self, *args: Any, **kwargs: Any) -> None: + super().__init__(*args, **kwargs) + self.refreshes = 0 + + def _refresh_auth(self) -> None: + self.refreshes += 1 + + budget = TangleApiClient._MAX_GET_RETRIES + # One 401 (consumes a request) followed by an unbroken run of 503s. + session = _FakeSession( + [_response(status_code=401)] + [_response(status_code=503) for _ in range(budget + 5)] + ) + client = RefreshingClient("https://api.test", session=session) + + result = client._make_request("GET", "/api/test") + + assert result.status_code == 503 + # Refresh fired once for the 401 (plus the unconditional pre-request refresh). + assert client.refreshes == 2 + # The 401 request plus the post-refresh retries share one budget: the total + # never exceeds the shared cap (a fresh budget would allow budget+1 more). + assert len(session.calls) == budget + 1 + + +def test_shared_budget_caps_total_requests_across_transient_rate_limit_and_auth(monkeypatch) -> None: + # The worst case the reviewer flagged: a 401 auth refresh, 429 rate limits, + # and transient 503s all interleaved for one logical GET. A single shared + # budget must bound the total physical request count instead of letting the + # three layers multiply their per-layer limits together. + monkeypatch.setattr("tangle_cli.client.time.sleep", lambda _delay: None) + + class RefreshingClient(TangleApiClient): + def __init__(self, *args: Any, **kwargs: Any) -> None: + super().__init__(*args, **kwargs) + self.refreshes = 0 + + def _refresh_auth(self) -> None: + self.refreshes += 1 + + budget = TangleApiClient._MAX_GET_RETRIES + # Lead with a 401 (drives one refresh), then alternate 429/503 far past the + # budget so the cap, not the response list, is what stops the retries. + responses = [_response(status_code=401)] + responses += [_response(status_code=429 if i % 2 == 0 else 503) for i in range(40)] + session = _FakeSession(responses) + client = RefreshingClient("https://api.test", session=session) + + result = client._make_request("GET", "/api/test") + + # Exactly the advertised shared budget of physical requests is spent. + assert len(session.calls) == budget + 1 + assert result.status_code in {429, 503} + # Pre-request refresh plus exactly one refresh for the single 401; the 401's + # retry continues on the shared budget rather than opening a fresh round. + assert client.refreshes == 2 + + +def _redirect(url: str, location: str, status_code: int = 307) -> requests.Response: + r = _response(status_code=status_code) + r.url = url + r.headers["Location"] = location + return r + + +def _redirect_chain(hops: int, final: requests.Response) -> list[requests.Response]: + """A ``hops``-deep same-origin 307 chain ending in ``final``.""" + + return [ + _redirect(f"https://api.test/api/hop{hop}", f"/api/hop{hop + 1}") for hop in range(hops) + ] + [final] + + +class _ClockAdvancingSession(_FakeSession): + """Session whose sends are the only thing that advances the fake clock. + + Deadline behaviour is then a pure function of the response sequence, with no + dependence on how fast the test host runs. + """ + + def __init__( + self, + responses: list[requests.Response | Exception], + clock: SimpleNamespace, + step: float, + ) -> None: + super().__init__(responses) + self.clock = clock + self.step = step + self.send_times: list[float] = [] + + def request(self, method: str, url: str, **kwargs: Any) -> requests.Response: + self.send_times.append(self.clock.now) + response = super().request(method, url, **kwargs) + self.clock.now += self.step + return response + + +def test_shared_budget_deadline_halts_composed_retries_without_wallclock(monkeypatch) -> None: + # The shared budget bounds retries by BOTH an attempt count and a + # _MAX_RETRY_ELAPSED_SECONDS wall-time deadline. This pins the deadline + # clause deterministically: a fake monotonic clock advances only when a + # request is sent (never the real wall clock), so the elapsed-time cap, not + # the attempt cap, is what stops the composed auth-refresh / rate-limit / + # transient retry sequence. Without this every other budget test would still + # pass on the attempt cap alone, so the deadline could be removed silently. + clock = SimpleNamespace(now=1_000.0) + step = 50.0 + window = TangleApiClient._MAX_RETRY_ELAPSED_SECONDS + deadline = clock.now + window + monkeypatch.setattr("tangle_cli.client.time.monotonic", lambda: clock.now) + + sleep_times: list[float] = [] + monkeypatch.setattr("tangle_cli.client.time.sleep", lambda _delay: sleep_times.append(clock.now)) + + class RefreshingClient(TangleApiClient): + def __init__(self, *args: Any, **kwargs: Any) -> None: + super().__init__(*args, **kwargs) + self.refreshes = 0 + + def _refresh_auth(self) -> None: + self.refreshes += 1 + + # A 401 (auth-refresh path), a 429 (rate-limit path), then unbroken 503s + # (transient path): all three layers draw on the one shared budget. Far more + # responses are queued than either cap allows, so the cap that fires first is + # what stops the sequence. + responses = [_response(status_code=401), _response(status_code=429)] + responses += [_response(status_code=503) for _ in range(20)] + session = _ClockAdvancingSession(responses, clock, step) + client = RefreshingClient("https://api.test", session=session) + + result = client._make_request("GET", "/api/test") + + # The sequence genuinely retried across layers but stopped short of the + # attempt cap with responses still queued: the deadline, not the attempt + # count or the response list, is what ended it. + attempt_cap = TangleApiClient._MAX_GET_RETRIES + 1 + assert 1 < len(session.calls) < attempt_cap + assert session.responses, "unused responses prove the queue did not stop the retries" + # Time actually crossed the deadline, yet no send or sleep happened at/after + # it: can_retry gates every physical send and every sleep across the + # transient and rate-limit layers. + assert clock.now >= deadline + assert all(t < deadline for t in session.send_times) + assert all(t < deadline for t in sleep_times) + # The composed auth path ran on the shared budget (pre-request refresh plus + # one for the single 401), and the final 5xx surfaces for the caller. + assert client.refreshes == 2 + assert result.status_code == 503 + + +def test_long_retry_after_is_not_slept_when_it_would_cross_the_deadline(monkeypatch) -> None: + # A Retry-After longer than the time left on the shared deadline must end the + # sequence immediately. Sleeping it out would only be followed by a send the + # budget is then obliged to refuse, so the wait is pure dead time. + clock = SimpleNamespace(now=1_000.0) + deadline = clock.now + TangleApiClient._MAX_RETRY_ELAPSED_SECONDS + monkeypatch.setattr("tangle_cli.client.time.monotonic", lambda: clock.now) + sleeps: list[float] = [] + monkeypatch.setattr("tangle_cli.client.time.sleep", sleeps.append) + + rate_limited = _response(status_code=429) + rate_limited.headers["Retry-After"] = "60" + # One send burns most of the window, leaving less than the 60s Retry-After. + session = _ClockAdvancingSession( + [rate_limited] + [_response({"ok": True}) for _ in range(3)], clock, step=80.0 + ) + client = TangleApiClient("https://api.test", session=session) + + result = client._make_request("GET", "/api/test") + + assert result is rate_limited + assert len(session.calls) == 1 + assert sleeps == [] + # Attempts and the deadline itself both still had room: only the fact that the + # wait would have crossed the deadline stopped the retry. + assert clock.now < deadline + assert deadline - clock.now < 60.0 + + +def test_transient_backoff_that_would_cross_the_deadline_stops_retrying(monkeypatch) -> None: + # Same rule on the transient-5xx layer: once the doubling backoff no longer + # fits inside the remaining deadline, the final 5xx surfaces instead of the + # client sleeping into a send it cannot make. + clock = SimpleNamespace(now=1_000.0) + deadline = clock.now + TangleApiClient._MAX_RETRY_ELAPSED_SECONDS + monkeypatch.setattr("tangle_cli.client.time.monotonic", lambda: clock.now) + sleeps: list[float] = [] + monkeypatch.setattr("tangle_cli.client.time.sleep", sleeps.append) + + session = _ClockAdvancingSession( + [_response(status_code=503) for _ in range(20)], clock, step=39.9 + ) + client = TangleApiClient("https://api.test", session=session) + + result = client._make_request("GET", "/api/test") + + assert result.status_code == 503 + # Two backoffs fitted in the window (1s, then 2s); the third would have been + # 4s with only 0.3s left, so the sequence ends there. + assert sleeps == [1.0, 2.0] + assert len(session.calls) == 3 + # The attempt cap was nowhere near reached and the clock had not yet passed + # the deadline, so neither of those is what stopped it. + assert len(session.calls) < TangleApiClient._MAX_GET_RETRIES + 1 + assert clock.now < deadline + assert session.responses, "unused responses prove the queue did not stop the retries" + + +def test_redirect_hops_are_charged_to_the_shared_send_budget(monkeypatch) -> None: + # Every physical send counts, including same-origin redirect hops. Without + # charging them, a 307 in front of each 503 would double the number of + # requests an outage can provoke (and a full 5-hop chain would multiply it + # sixfold). Hops are charged to the send pool rather than to the attempt + # count, so the chain is still followed on every attempt. + monkeypatch.setattr("tangle_cli.client.time.sleep", lambda _delay: None) + + responses: list[requests.Response] = [] + for _ in range(20): + responses.append(_redirect("https://api.test/api/test", "/api/moved")) + responses.append(_response(status_code=503)) + session = _FakeSession(responses) + client = TangleApiClient("https://api.test", session=session) + + result = client._make_request("GET", "/api/test") + + # Seven logical attempts of two sends each spend the pool exactly, and the + # real 503 surfaces instead of a budget-exhaustion error. + assert result.status_code == 503 + assert len(session.calls) == TangleApiClient._MAX_PHYSICAL_SENDS == 14 + assert [call["url"] for call in session.calls] == [ + "https://api.test/api/test", + "https://api.test/api/moved", + ] * 7 + + +@pytest.mark.parametrize("method", ["GET", "POST"]) +@pytest.mark.parametrize("hops", range(TangleApiClient._MAX_REDIRECTS + 1)) +def test_legal_redirect_chain_survives_an_auth_refresh(monkeypatch, hops, method) -> None: + # The client advertises support for chains up to _MAX_REDIRECTS deep, and a + # 401 refresh replays the whole request. Both together are ordinary healthy + # traffic -- no outage -- so every depth must still reach the backend. This + # is why physical sends are pooled separately from logical attempts: charging + # hops against the attempt count would fail depth 3 and beyond outright. + monkeypatch.setattr("tangle_cli.client.time.sleep", lambda _delay: None) + + class RefreshingClient(TangleApiClient): + def __init__(self, *args: Any, **kwargs: Any) -> None: + super().__init__(*args, **kwargs) + self.refreshes = 0 + + def _refresh_auth(self) -> None: + self.refreshes += 1 + + ok = _response({"ok": True}) + responses = _redirect_chain(hops, _response(status_code=401)) + responses += _redirect_chain(hops, ok) + session = _FakeSession(responses) + client = RefreshingClient("https://api.test", session=session) + + result = client._make_request( + method, "/api/test", json_data={"a": 1} if method == "POST" else None + ) + + assert result is ok + # Pre-request refresh plus exactly one for the 401. + assert client.refreshes == 2 + assert len(session.calls) == 2 * (hops + 1) + assert not session.responses + # Two full legal chains is one of the two floors the send pool is sized to. + assert 2 * (hops + 1) <= TangleApiClient._MAX_PHYSICAL_SENDS + + +def test_worst_case_redirect_chain_stays_within_the_physical_send_cap(monkeypatch) -> None: + # The reviewer's amplification concern, at its worst: a full-depth chain in + # front of every retryable 5xx. The send pool, not the chain length, is what + # bounds the total. + monkeypatch.setattr("tangle_cli.client.time.sleep", lambda _delay: None) + hops = TangleApiClient._MAX_REDIRECTS + + responses: list[requests.Response] = [] + for _ in range(10): + responses += _redirect_chain(hops, _response(status_code=503)) + session = _FakeSession(responses) + client = TangleApiClient("https://api.test", session=session) + + result = client._make_request("GET", "/api/test") + + assert len(session.calls) == TangleApiClient._MAX_PHYSICAL_SENDS == 14 + # Far below the attempts x chain-length product an uncharged chain would allow. + assert len(session.calls) < (TangleApiClient._MAX_GET_RETRIES + 1) * (hops + 1) + # The pool runs out part-way through the third chain, but a completed 503 + # from the second is still the truthful answer for the caller. + assert result.status_code == 503 + with pytest.raises(requests.HTTPError): + result.raise_for_status() + + +def test_redirect_then_5xx_reports_the_backend_status_not_budget_exhaustion(monkeypatch) -> None: + # End to end through a public operation: a 307 in front of a 503 must still + # be reported as HTTP 503. Replacing the completed response with a + # RetryError would hide the real backend failure from raise_for_status -- + # and RetryError is not an HTTPError, so the 404 fallbacks would not see it + # either. + monkeypatch.setattr("tangle_cli.client.time.sleep", lambda _delay: None) + + responses: list[requests.Response] = [] + for _ in range(10): + responses += _redirect_chain(TangleApiClient._MAX_REDIRECTS, _response(status_code=503)) + session = _FakeSession(responses) + client = TangleApiClient("https://api.test", session=session) + + with pytest.raises(requests.HTTPError) as exc_info: + client.pipeline_runs_get("run-1") + + assert exc_info.value.response is not None + assert exc_info.value.response.status_code == 503 + + +def test_direct_get_5xx_still_stops_at_the_logical_attempt_cap(monkeypatch) -> None: + # The send pool is deliberately larger than the attempt count so redirect + # chains fit. It must not become extra retries for a request that never + # redirects. + monkeypatch.setattr("tangle_cli.client.time.sleep", lambda _delay: None) + session = _FakeSession([_response(status_code=503) for _ in range(20)]) + client = TangleApiClient("https://api.test", session=session) + + result = client._make_request("GET", "/api/test") + + assert result.status_code == 503 + assert len(session.calls) == TangleApiClient._MAX_GET_RETRIES + 1 == 7 + assert len(session.calls) < TangleApiClient._MAX_PHYSICAL_SENDS + + +def test_deadline_hit_mid_first_redirect_chain_raises_a_clean_retry_error(monkeypatch) -> None: + # Exhaustion inside the very first chain has no completed backend response to + # fall back on, so the exhaustion error itself is the honest answer. It must + # stay a requests-family error and must not be an HTTPError, or the 404 + # fallbacks in the public helpers would inspect a response that is not there. + clock = SimpleNamespace(now=1_000.0) + deadline = clock.now + TangleApiClient._MAX_RETRY_ELAPSED_SECONDS + monkeypatch.setattr("tangle_cli.client.time.monotonic", lambda: clock.now) + monkeypatch.setattr("tangle_cli.client.time.sleep", lambda _delay: None) + + responses = _redirect_chain(TangleApiClient._MAX_REDIRECTS, _response({"ok": True})) + session = _ClockAdvancingSession(responses, clock, step=50.0) + client = TangleApiClient("https://api.test", session=session) + + with pytest.raises(requests.exceptions.RetryError, match="Retry budget exhausted"): + client._make_request("GET", "/api/test") + + # Three hops fitted inside the window; the fourth was refused at the send + # boundary, before the request went out. + assert len(session.calls) == 3 + assert clock.now >= deadline + assert all(t < deadline for t in session.send_times) + assert not issubclass(requests.exceptions.RetryError, requests.HTTPError) + + +def test_post_429_keeps_legacy_four_attempt_cap_and_backoff(monkeypatch) -> None: + # POSTs never enter the replay-on-5xx layer, so they keep the historical + # four-attempt 429 allowance and its 1/2/4s backoff rather than spending the + # larger shared GET budget on rate limiting alone. + sleeps: list[float] = [] + monkeypatch.setattr("tangle_cli.client.time.sleep", sleeps.append) + session = _FakeSession([_response(status_code=429) for _ in range(10)]) + client = TangleApiClient("https://api.test", session=session) + + result = client._make_request("POST", "/api/test", json_data={"a": 1}) + + assert result.status_code == 429 + assert len(session.calls) == TangleApiClient._MAX_RATE_LIMIT_RETRIES + 1 == 4 + assert sleeps == [1.0, 2.0, 4.0] + + +def test_streamed_get_429_keeps_legacy_four_attempt_cap_and_backoff(monkeypatch) -> None: + # A streamed GET is not replayable either (its consumer owns stream-open + # retries), so it gets the same legacy 429 treatment as a POST. + sleeps: list[float] = [] + monkeypatch.setattr("tangle_cli.client.time.sleep", sleeps.append) + session = _FakeSession([_response(status_code=429) for _ in range(10)]) + client = TangleApiClient("https://api.test", session=session) + + result = client._make_request("GET", "/api/test", stream=True) + + assert result.status_code == 429 + assert len(session.calls) == TangleApiClient._MAX_RATE_LIMIT_RETRIES + 1 == 4 + assert sleeps == [1.0, 2.0, 4.0] + + +def test_ordinary_get_429_uses_the_shared_budget_not_the_legacy_cap(monkeypatch) -> None: + # The counterpart to the two tests above: a replayable GET still gets the + # full shared budget, so restoring the legacy cap did not narrow it. Its + # doubling backoff is capped by _MAX_RETRY_AFTER_SECONDS (60s), which 32s + # never reaches, so the worst-case wall time is 63s over seven sends. + sleeps: list[float] = [] + monkeypatch.setattr("tangle_cli.client.time.sleep", sleeps.append) + session = _FakeSession([_response(status_code=429) for _ in range(20)]) + client = TangleApiClient("https://api.test", session=session) + + result = client._make_request("GET", "/api/test") + + assert result.status_code == 429 + assert len(session.calls) == TangleApiClient._MAX_GET_RETRIES + 1 == 7 + assert sleeps == [1.0, 2.0, 4.0, 8.0, 16.0, 32.0] + assert sum(sleeps) == 63.0 + assert max(sleeps) <= TangleApiClient._MAX_RETRY_AFTER_SECONDS + + +def test_ordinary_get_5xx_backoff_is_capped_lower_than_the_429_backoff(monkeypatch) -> None: + # Same seven sends as the 429 case, but the transient layer clamps at + # _MAX_GET_RETRY_BACKOFF_SECONDS (30s), so the final wait is 30s not 32s and + # the worst-case wall time is 61s. Both stay inside the 120s deadline. + sleeps: list[float] = [] + monkeypatch.setattr("tangle_cli.client.time.sleep", sleeps.append) + session = _FakeSession([_response(status_code=503) for _ in range(20)]) + client = TangleApiClient("https://api.test", session=session) + + result = client._make_request("GET", "/api/test") + + assert result.status_code == 503 + assert len(session.calls) == TangleApiClient._MAX_GET_RETRIES + 1 == 7 + assert sleeps == [1.0, 2.0, 4.0, 8.0, 16.0, 30.0] + assert sum(sleeps) == 61.0 + assert max(sleeps) == TangleApiClient._MAX_GET_RETRY_BACKOFF_SECONDS + assert sum(sleeps) < TangleApiClient._MAX_RETRY_ELAPSED_SECONDS + + +@pytest.mark.parametrize("stream", [False, True], ids=["plain", "streamed"]) +def test_intermediate_429_responses_are_closed_but_the_final_one_is_usable( + monkeypatch, stream: bool +) -> None: + # A superseded 429 holds a pooled connection for the whole backoff if it is + # never released, and a streamed one is never read at all. Only the response + # actually handed back to the caller must stay open. + closed: list[str] = [] + + def tracking_response(status_code: int, marker: str, payload: Any = None) -> requests.Response: + r = _response(payload, status_code=status_code) + r.close = lambda: closed.append(marker) # type: ignore[method-assign] + return r + + monkeypatch.setattr("tangle_cli.client.time.sleep", lambda _delay: None) + session = _FakeSession( + [ + tracking_response(429, "429-1"), + tracking_response(429, "429-2"), + tracking_response(200, "final", {"ok": True}), + ] + ) + client = TangleApiClient("https://api.test", session=session) + + result = client._make_request("GET", "/api/test", stream=stream) + + assert closed == ["429-1", "429-2"] + assert result.status_code == 200 + assert result.json() == {"ok": True} + + +def test_a_returned_429_is_not_closed_when_the_rate_limit_rounds_run_out(monkeypatch) -> None: + # The counterpart to the test above on the give-up path: the last 429 is the + # answer, so it must survive for the caller to inspect. + closed: list[str] = [] + + def tracking_response(marker: str) -> requests.Response: + r = _response({"detail": marker}, status_code=429) + r.close = lambda: closed.append(marker) # type: ignore[method-assign] + return r + + monkeypatch.setattr("tangle_cli.client.time.sleep", lambda _delay: None) + session = _FakeSession([tracking_response(f"r{i}") for i in range(4)]) + client = TangleApiClient("https://api.test", session=session) + + result = client._make_request("POST", "/api/test", json_data={"a": 1}) + + assert closed == ["r0", "r1", "r2"] + assert result.status_code == 429 + assert result.json() == {"detail": "r3"} + + +def test_retry_error_names_the_deadline_rather_than_the_send_pool(monkeypatch) -> None: + # Both exhaustion causes stop a send at the same boundary; the message has to + # say which one, or a slow-backend incident reads as a redirect-loop bug. + clock = SimpleNamespace(now=1_000.0) + monkeypatch.setattr("tangle_cli.client.time.monotonic", lambda: clock.now) + monkeypatch.setattr("tangle_cli.client.time.sleep", lambda _delay: None) + + responses = _redirect_chain(TangleApiClient._MAX_REDIRECTS, _response({"ok": True})) + session = _ClockAdvancingSession(responses, clock, step=50.0) + client = TangleApiClient("https://api.test", session=session) + + with pytest.raises(requests.exceptions.RetryError, match="deadline exceeded"): + client._make_request("GET", "/api/test") + + +def test_retry_error_names_the_send_pool_when_the_deadline_is_untouched(monkeypatch) -> None: + # The other branch of the message, with the clock standing still so only the + # pool can be the cause. The real pool is sized to outlast any single chain, + # so it is narrowed here to reach the branch at all. + monkeypatch.setattr("tangle_cli.client.time.monotonic", lambda: 1_000.0) + monkeypatch.setattr("tangle_cli.client.time.sleep", lambda _delay: None) + monkeypatch.setattr(TangleApiClient, "_MAX_PHYSICAL_SENDS", 3) + + responses = _redirect_chain(TangleApiClient._MAX_REDIRECTS, _response({"ok": True})) + session = _FakeSession(responses) + client = TangleApiClient("https://api.test", session=session) + + with pytest.raises(requests.exceptions.RetryError, match="send pool exhausted"): + client._make_request("GET", "/api/test") + + assert len(session.calls) == 3 + + +@pytest.mark.parametrize( + ("redirect_url", "location", "secrets", "expected"), + [ + pytest.param( + "https://api.test/api/test", + "/download?access_token=tok-SECRET", + ["tok-SECRET", "access_token"], + "https://api.test/download", + id="signed-query", + ), + pytest.param( + "https://api.test/api/test", + "/blob?X-Amz-Signature=amz-SECRET&X-Amz-Credential=AKIA-SECRET", + ["amz-SECRET", "AKIA-SECRET", "X-Amz-Signature"], + "https://api.test/blob", + id="aws-sigv4-query", + ), + pytest.param( + "https://api.test/api/test", + "/asset#token=frag-SECRET", + ["frag-SECRET", "#"], + "https://api.test/asset", + id="fragment-credential", + ), + pytest.param( + "https://alice:hunter2@api.test/api/test", + "https://alice:hunter2@api.test/next", + ["alice", "hunter2"], + "https://api.test/next", + id="userinfo", + ), + pytest.param( + "https://api.test:9x9/api/test", + "https://api.test:9x9/next?X-Amz-Signature=port-SECRET", + ["port-SECRET", "X-Amz-Signature"], + "https://api.test:9x9/next", + id="malformed-port", + ), + ], +) +def test_retry_error_after_redirect_keeps_credentials_out_of_the_message( + monkeypatch, redirect_url: str, location: str, secrets: list[str], expected: str +) -> None: + # A same-origin redirect can land on a signed URL, and the exhaustion error + # flows into CLI output and logs. The destination stays nameable through + # scheme/host/path, but query, fragment, and userinfo must not survive -- + # and a malformed port must not turn the error itself into a ValueError. + monkeypatch.setattr("tangle_cli.client.time.sleep", lambda _delay: None) + monkeypatch.setattr(TangleApiClient, "_MAX_PHYSICAL_SENDS", 1) + session = _FakeSession([_redirect(redirect_url, location)]) + client = TangleApiClient("https://api.test", session=session) + + with pytest.raises(requests.exceptions.RetryError, match="send pool exhausted") as exc_info: + client._make_request("GET", "/api/test") + + message = str(exc_info.value) + for secret in secrets: + assert secret not in message + assert expected in message + + +def test_deadline_retry_error_after_redirect_keeps_credentials_out_of_the_message( + monkeypatch, +) -> None: + # Same guarantee on the other exhaustion branch: a backoff-free deadline hit + # right after the redirect hop must not report the signed query either. + clock = SimpleNamespace(now=1_000.0) + monkeypatch.setattr("tangle_cli.client.time.monotonic", lambda: clock.now) + monkeypatch.setattr("tangle_cli.client.time.sleep", lambda _delay: None) + + responses = [_redirect("https://api.test/api/test", "/blob?X-Amz-Signature=amz-SECRET")] + session = _ClockAdvancingSession( + responses, clock, step=TangleApiClient._MAX_RETRY_ELAPSED_SECONDS + 10.0 + ) + client = TangleApiClient("https://api.test", session=session) + + with pytest.raises(requests.exceptions.RetryError, match="deadline exceeded") as exc_info: + client._make_request("GET", "/api/test") + + message = str(exc_info.value) + assert "amz-SECRET" not in message + assert "X-Amz-Signature" not in message + assert "https://api.test/blob" in message + + +def test_credential_safe_url_never_raises_on_authorities_urlsplit_rejects() -> None: + # Error formatting runs while an exhaustion error is already being raised, + # so an authority the parser rejects must degrade to a placeholder. + assert TangleApiClient._credential_safe_url("https://[::1/api") == "" + assert TangleApiClient._credential_safe_url("") == "" + + +def test_post_401_then_429_shares_the_budget_across_auth_and_rate_limit(monkeypatch) -> None: + # Mixed composition on the non-replayable path: the auth refresh restarts the + # rate-limit rounds, so the shared budget is the only thing bounding the + # total number of physical POSTs. + sleeps: list[float] = [] + monkeypatch.setattr("tangle_cli.client.time.sleep", sleeps.append) + + class RefreshingClient(TangleApiClient): + def __init__(self, *args: Any, **kwargs: Any) -> None: + super().__init__(*args, **kwargs) + self.refreshes = 0 + + def _refresh_auth(self) -> None: + self.refreshes += 1 + + session = _FakeSession( + [_response(status_code=401)] + [_response(status_code=429) for _ in range(10)] + ) + client = RefreshingClient("https://api.test", session=session) + + result = client._make_request("POST", "/api/test", json_data={"a": 1}) + + assert result.status_code == 429 + # The 401 send plus one full legacy 429 sequence, still inside the shared cap. + assert len(session.calls) == 1 + TangleApiClient._MAX_RATE_LIMIT_RETRIES + 1 == 5 + assert len(session.calls) <= TangleApiClient._MAX_GET_RETRIES + 1 + assert sleeps == [1.0, 2.0, 4.0] + assert client.refreshes == 2 + + +def test_get_5xx_exhaustion_raises_http_error_from_public_operation(monkeypatch) -> None: + monkeypatch.setattr("tangle_cli.client.time.sleep", lambda _delay: None) + budget = TangleApiClient._MAX_GET_RETRIES + session = _FakeSession([_response(status_code=503) for _ in range(budget + 1)]) + client = TangleApiClient("https://api.test", session=session) + + with pytest.raises(requests.HTTPError) as exc_info: + client.pipeline_runs_get("run-1") + + assert exc_info.value.response is not None + assert exc_info.value.response.status_code == 503 + assert len(session.calls) == budget + 1 diff --git a/tests/test_components_cli.py b/tests/test_components_cli.py index 5764695..c52b5d8 100644 --- a/tests/test_components_cli.py +++ b/tests/test_components_cli.py @@ -138,6 +138,7 @@ def fake_client_from_options(**kwargs: Any) -> object: "header": ["X-Config: yes"], "include_env_credentials": False, "command_name": "published-component commands", + "logger": ANY, } ] assert FakePublisher.instances[0].kwargs["client"] is fake_client @@ -271,6 +272,7 @@ def fake_client_from_options(**kwargs: Any) -> object: "header": None, "include_env_credentials": False, "command_name": "published-component commands", + "logger": ANY, } ] assert [publisher.kwargs for publisher in FakePublisher.instances] == [ @@ -341,6 +343,7 @@ def fake_deprecate_component(client: object, digest: str, **kwargs: Any) -> dict "header": ["X-Test: yes"], "include_env_credentials": False, "command_name": "published-component commands", + "logger": ANY, } ] assert deprecate_calls == [ diff --git a/tests/test_packaging.py b/tests/test_packaging.py index e40e838..59c8b3e 100644 --- a/tests/test_packaging.py +++ b/tests/test_packaging.py @@ -40,7 +40,7 @@ def _write_import_stubs(path: Path) -> None: _write_runtime_stubs(path) (path / "cyclopts.py").write_text( "class App:\n" - " def __init__(self, *args, **kwargs): pass\n" + " def __init__(self, *args, **kwargs): self._meta = None\n" " def command(self, obj=None, **kwargs):\n" " if obj is not None:\n" " return obj\n" @@ -48,6 +48,11 @@ def _write_import_stubs(path: Path) -> None: " return decorator\n" " def __call__(self, *args, **kwargs): pass\n" " def default(self, fn): return fn\n" + " @property\n" + " def meta(self):\n" + " if self._meta is None:\n" + " self._meta = App()\n" + " return self._meta\n" "\n" "def Parameter(*args, **kwargs): return object()\n", encoding="utf-8", diff --git a/tests/test_pipeline_runs_cli.py b/tests/test_pipeline_runs_cli.py index b74b0ad..ae03a19 100644 --- a/tests/test_pipeline_runs_cli.py +++ b/tests/test_pipeline_runs_cli.py @@ -1,17 +1,26 @@ from __future__ import annotations import copy +import io import json +import math +import os +import sys +import time from contextlib import nullcontext from pathlib import Path from types import SimpleNamespace from typing import Any import pytest +import requests import yaml from tangle_cli import cli, pipeline_run_manager, pipeline_runs_cli +from tangle_cli.logger import CaptureLogger from tangle_cli.pipeline_run_manager import ( + _TASK_FAILURE_STATUSES, + _TERMINAL_STATUSES, AmbiguousPipelineRunRecoveryError, PipelineRunContext, PipelineRunError, @@ -19,6 +28,12 @@ PipelineRunManager, PipelineWaitOutcome, PipelineWaitPoll, + TaskStatusesFailed, + TransientServerError, + merge_secret_run_args, + normalize_arg_secret_config, + parse_arg_secret_entries, + secret_argument_value, ) from tangle_cli.pipeline_runner import PipelineRunner, PipelineRunnerHooks @@ -114,6 +129,10 @@ def executions_graph_execution_state(self, id: str) -> dict[str, Any]: def executions_container_log(self, id: str) -> dict[str, Any]: return {"log_text": f"logs for {id}\n"} + def iter_execution_container_log_lines(self, id: str): + yield f"stream {id} line 1" + yield f"stream {id} line 2" + def pipeline_runs_list(self, **kwargs: Any) -> dict[str, Any]: self.list_calls.append(kwargs) return {"pipeline_runs": [{"id": "run-1"}], "next_page_token": None} @@ -2528,6 +2547,25 @@ def test_pipeline_run_status_uses_deterministic_precedence() -> None: assert PipelineRunManager.status_from_run(terminal_run) == "FAILED" +@pytest.mark.parametrize( + "counts, expected", + [ + ({"INVALID": 1, "SUCCEEDED": 1}, "INVALID"), + ({"INVALID": 1}, "INVALID"), + ({"SKIPPED": 1, "INVALID": 1}, "INVALID"), + ({"SUCCEEDED": 1}, "SUCCEEDED"), + ({}, "ENDED"), + ], +) +def test_pipeline_run_ended_summary_uses_terminal_precedence(counts, expected) -> None: + run = { + "execution_summary": {"has_ended": True}, + "execution_status_stats": counts, + } + + assert PipelineRunManager.status_from_run(run) == expected + + def test_pipeline_runs_wait_is_bounded_and_testable(monkeypatch): fake_client = FakeClient() manager = PipelineRunManager(client=fake_client) @@ -2790,3 +2828,1120 @@ def cleanup_prepared_pipeline(self, preparation, *, error=None): # type: ignore assert cleaned == [(temp_effective_path, "Pipeline validation failed:\n - boom")] assert not temp_effective_path.exists() + + +def _http_error(status_code: int) -> requests.HTTPError: + response = requests.Response() + response.status_code = status_code + return requests.HTTPError(response=response) + + +class ExecutionStateClient: + """Configurable fake for the root-execution task-map endpoints. + + ``details`` maps a root execution id to its child_task_execution_ids; each + child/leaf id is resolved against ``container_state`` then + ``graph_execution_state``. Endpoint values may be a status dict, an HTTP + error to raise, or absent (KeyError-style 404). + """ + + def __init__( + self, + *, + details: dict[str, Any], + container_state: dict[str, Any] | None = None, + graph_state: dict[str, Any] | None = None, + ) -> None: + self.base_url = "https://tangle.example" + self._details = details + self._container_state = container_state or {} + self._graph_state = graph_state or {} + self.details_calls: list[str] = [] + self.container_calls: list[str] = [] + self.graph_calls: list[str] = [] + + def executions_details(self, id: str) -> dict[str, Any]: + return self._resolve(self._details, self.details_calls, id) + + def _resolve(self, store: dict[str, Any], calls: list[str], id: str) -> dict[str, Any]: + calls.append(id) + if id not in store: + raise _http_error(404) + value = store[id] + if isinstance(value, requests.RequestException): + raise value + return value + + def executions_container_state(self, id: str, **_: Any) -> dict[str, Any]: + return self._resolve(self._container_state, self.container_calls, id) + + def executions_graph_execution_state(self, id: str) -> dict[str, Any]: + return self._resolve(self._graph_state, self.graph_calls, id) + + +def test_task_statuses_walks_children_via_container_state() -> None: + client = ExecutionStateClient( + details={"root-1": {"child_task_execution_ids": {"a": "exec-a", "b": "exec-b"}}}, + container_state={ + "exec-a": {"status": "SUCCEEDED"}, + "exec-b": {"status": "RUNNING"}, + }, + ) + manager = PipelineRunManager(client=client) + + assert manager.task_statuses("root-1") == {"a": "SUCCEEDED", "b": "RUNNING"} + # Container state resolves first, so graph state is never queried. + assert client.graph_calls == [] + + +def test_task_statuses_falsy_child_id_is_unknown_without_api_call() -> None: + # A None/empty/whitespace-only child execution id maps straight to UNKNOWN; + # it must not be stringified to "None"/blank and queried against the state + # endpoints. + client = ExecutionStateClient( + details={ + "root-1": {"child_task_execution_ids": {"a": None, "b": "", "c": "exec-c", "d": " "}} + }, + container_state={"exec-c": {"status": "RUNNING"}}, + ) + manager = PipelineRunManager(client=client) + + assert manager.task_statuses("root-1") == { + "a": "UNKNOWN", + "b": "UNKNOWN", + "c": "RUNNING", + "d": "UNKNOWN", + } + # Only the real child id reaches the state API. + assert client.container_calls == ["exec-c"] + assert client.graph_calls == [] + + +def test_task_statuses_root_only_execution_reports_root_key() -> None: + client = ExecutionStateClient( + details={"root-1": {"child_task_execution_ids": {}}}, + container_state={"root-1": {"status": "SUCCEEDED"}}, + ) + manager = PipelineRunManager(client=client) + + assert manager.task_statuses("root-1") == {"root": "SUCCEEDED"} + + +def test_task_statuses_falls_back_to_graph_state_on_404() -> None: + client = ExecutionStateClient( + details={"root-1": {"child_task_execution_ids": {"a": "exec-a"}}}, + container_state={}, # exec-a missing -> 404 from container state + graph_state={"exec-a": {"status": "RUNNING"}}, + ) + manager = PipelineRunManager(client=client) + + assert manager.task_statuses("root-1") == {"a": "RUNNING"} + assert client.container_calls == ["exec-a"] + assert client.graph_calls == ["exec-a"] + + +def test_task_statuses_persistent_5xx_reported_as_unknown() -> None: + client = ExecutionStateClient( + details={"root-1": {"child_task_execution_ids": {"a": "exec-a"}}}, + container_state={"exec-a": _http_error(503)}, + graph_state={"exec-a": _http_error(503)}, + ) + logger = CaptureLogger() + manager = PipelineRunManager(client=client, logger=logger) + + assert manager.task_statuses("root-1") == {"a": "UNKNOWN"} + # The persistent 5xx is announced so an UNKNOWN result is diagnosable. + logs = logger.get_logs() or "" + assert "exec-a" in logs + assert "503" in logs + + +def test_task_statuses_past_deadline_reports_unknown_without_fetching() -> None: + # Past the caller's deadline no further state requests are issued, so one + # task-statuses poll cannot overrun the wait budget of task-wait. + client = ExecutionStateClient( + details={"root-1": {"child_task_execution_ids": {"a": "exec-a"}}}, + container_state={"exec-a": {"status": "SUCCEEDED"}}, + ) + manager = PipelineRunManager(client=client) + + assert manager.task_statuses("root-1", deadline=time.monotonic() - 1) == { + "root": "UNKNOWN" + } + assert client.details_calls == [] + assert client.container_calls == [] + assert client.graph_calls == [] + + +def test_wait_for_task_statuses_zero_budget_does_not_start_root_fetch() -> None: + client = ExecutionStateClient( + details={"root-1": {"child_task_execution_ids": {"a": "exec-a"}}}, + container_state={"exec-a": {"status": "SUCCEEDED"}}, + ) + manager = PipelineRunManager(client=client) + + with pytest.raises(PipelineRunError, match="non-terminal"): + manager.wait_for_task_statuses("root-1", max_wait=0, poll_interval=1) + + assert client.details_calls == [] + assert client.container_calls == [] + assert client.graph_calls == [] + + +def test_failure_statuses_are_the_non_skipped_non_succeeded_terminals() -> None: + # _TASK_FAILURE_STATUSES is derived from _TERMINAL_STATUSES; guard the intended + # membership so a new failure terminal can't be silently treated as success. + assert _TASK_FAILURE_STATUSES == {"FAILED", "SYSTEM_ERROR", "CANCELLED", "CANCELED", "INVALID"} + assert _TASK_FAILURE_STATUSES == frozenset(_TERMINAL_STATUSES) - {"SKIPPED", "SUCCEEDED"} + + +def test_task_statuses_wraps_non_retryable_http_error() -> None: + # A non-retryable state-endpoint status (429/403/etc.) becomes a clean + # PipelineRunError instead of a raw requests traceback out of task-status. + client = ExecutionStateClient( + details={"root-1": {"child_task_execution_ids": {"a": "exec-a"}}}, + container_state={"exec-a": _http_error(429)}, + ) + manager = PipelineRunManager(client=client) + + with pytest.raises(PipelineRunError, match="HTTP 429"): + manager.task_statuses("root-1") + + +def test_task_statuses_wraps_connection_error() -> None: + # A transport failure (no HTTP response) is surfaced cleanly rather than + # escaping as a raw requests.ConnectionError. + client = ExecutionStateClient( + details={"root-1": {"child_task_execution_ids": {"a": "exec-a"}}}, + container_state={"exec-a": requests.ConnectionError("connection refused")}, + ) + manager = PipelineRunManager(client=client) + + with pytest.raises(PipelineRunError, match="connection refused"): + manager.task_statuses("root-1") + + +def test_task_statuses_missing_root_is_not_found() -> None: + # A 404 on the root execution details is a clean not-found error, not a + # masked {"root": "UNKNOWN"} map. + client = ExecutionStateClient(details={"root-1": _http_error(404)}) + manager = PipelineRunManager(client=client) + + with pytest.raises(PipelineRunError, match="not found"): + manager.task_statuses("root-1") + + +def test_task_statuses_root_details_transport_error_surfaces_cleanly() -> None: + client = ExecutionStateClient(details={"root-1": requests.ConnectionError("boom")}) + manager = PipelineRunManager(client=client) + + with pytest.raises(PipelineRunError, match="boom"): + manager.task_statuses("root-1") + + +def test_task_statuses_root_details_5xx_is_transient_error() -> None: + # A one-shot task-status still fails fast on a root details 5xx, but the + # error is typed so the task-wait polling loop can retry it. + client = ExecutionStateClient(details={"root-1": _http_error(503)}) + manager = PipelineRunManager(client=client) + + with pytest.raises(TransientServerError, match="HTTP 503"): + manager.task_statuses("root-1") + + +def test_task_statuses_derives_graph_status_from_child_stats() -> None: + # A child that is itself a graph/subpipeline has no direct `status`; derive + # it from child_execution_status_stats counts rather than reporting UNKNOWN. + client = ExecutionStateClient( + details={"root-1": {"child_task_execution_ids": {"sub": "exec-sub"}}}, + container_state={}, # 404 -> fall back to graph state + graph_state={"exec-sub": {"child_execution_status_stats": {"leaf": {"SUCCEEDED": 1}}}}, + ) + manager = PipelineRunManager(client=client) + + assert manager.task_statuses("root-1") == {"sub": "SUCCEEDED"} + + +def test_task_statuses_derives_graph_status_from_status_totals() -> None: + client = ExecutionStateClient( + details={"root-1": {"child_task_execution_ids": {"sub": "exec-sub"}}}, + container_state={}, + graph_state={"exec-sub": {"status_totals": {"RUNNING": 1}}}, + ) + manager = PipelineRunManager(client=client) + + # An active count outranks terminal counts, matching status_from_counts. + assert manager.task_statuses("root-1") == {"sub": "RUNNING"} + + +def test_task_statuses_prefers_direct_graph_status_over_counts() -> None: + client = ExecutionStateClient( + details={"root-1": {"child_task_execution_ids": {"sub": "exec-sub"}}}, + container_state={}, + graph_state={"exec-sub": {"status": "FAILED", "status_totals": {"SUCCEEDED": 1}}}, + ) + manager = PipelineRunManager(client=client) + + assert manager.task_statuses("root-1") == {"sub": "FAILED"} + + +@pytest.mark.parametrize( + "counts, expected", + [ + # An active/nonterminal status must outrank terminal success. + ({"SUCCEEDED": 1, "WAITING_FOR_UPSTREAM": 1}, "WAITING_FOR_UPSTREAM"), + ({"SUCCEEDED": 1, "UNINITIALIZED": 1}, "UNINITIALIZED"), + # Failure/invalid terminal statuses must outrank SUCCEEDED. + ({"SUCCEEDED": 1, "INVALID": 1}, "INVALID"), + ({"SUCCEEDED": 1, "FAILED": 1}, "FAILED"), + # INVALID must also outrank the other non-failure terminal, SKIPPED, so a + # mixed graph aggregate does not mask an invalid child behind SKIPPED. + ({"SKIPPED": 1, "INVALID": 1}, "INVALID"), + ({"SKIPPED": 1, "FAILED": 1}, "FAILED"), + ], +) +def test_status_from_counts_nonterminal_and_failure_outrank_success(counts, expected) -> None: + assert PipelineRunManager.status_from_counts(counts) == expected + + +def test_task_statuses_nested_graph_waiting_is_nonterminal_and_blocks_wait() -> None: + # {"SUCCEEDED": 1, "WAITING_FOR_UPSTREAM": 1} must not collapse to SUCCEEDED + # and must not be treated as terminal by task-wait. + client = ExecutionStateClient( + details={"root-1": {"child_task_execution_ids": {"sub": "exec-sub"}}}, + container_state={}, + graph_state={ + "exec-sub": {"child_execution_status_stats": {"leaf": {"SUCCEEDED": 1, "WAITING_FOR_UPSTREAM": 1}}} + }, + ) + manager = PipelineRunManager(client=client) + + statuses = manager.task_statuses("root-1") + assert statuses == {"sub": "WAITING_FOR_UPSTREAM"} + assert not PipelineRunManager.is_terminal_status(statuses["sub"]) + + +def test_task_statuses_nested_graph_invalid_is_failure(monkeypatch) -> None: + # {"SUCCEEDED": 1, "INVALID": 1} reduces to INVALID, so task-wait fails. + client = ExecutionStateClient( + details={"root-1": {"child_task_execution_ids": {"sub": "exec-sub"}}}, + container_state={}, + graph_state={"exec-sub": {"status_totals": {"SUCCEEDED": 1, "INVALID": 1}}}, + ) + manager = PipelineRunManager(client=client) + monkeypatch.setattr("tangle_cli.pipeline_run_manager.time.sleep", lambda value: None) + + with pytest.raises(TaskStatusesFailed) as exc_info: + manager.wait_for_task_statuses("root-1", poll_interval=1) + assert exc_info.value.statuses == {"sub": "INVALID"} + + +def test_task_statuses_nested_graph_skipped_and_invalid_collapses_to_invalid(monkeypatch) -> None: + # A nested-graph child whose counts mix SKIPPED and INVALID must reduce to + # INVALID (the failure), not SKIPPED, so task-wait surfaces the invalid child. + client = ExecutionStateClient( + details={"root-1": {"child_task_execution_ids": {"sub": "exec-sub"}}}, + container_state={}, + graph_state={"exec-sub": {"status_totals": {"SKIPPED": 1, "INVALID": 1}}}, + ) + manager = PipelineRunManager(client=client) + monkeypatch.setattr("tangle_cli.pipeline_run_manager.time.sleep", lambda value: None) + + assert manager.task_statuses("root-1") == {"sub": "INVALID"} + with pytest.raises(TaskStatusesFailed) as exc_info: + manager.wait_for_task_statuses("root-1", poll_interval=1) + assert exc_info.value.failures == {"sub": "INVALID"} + + +def test_wait_for_task_statuses_skipped_is_successful_terminal() -> None: + # SKIPPED is a non-failure terminal (a skipped task, e.g. a conditional + # branch, is not a failure), so a fully SKIPPED task map returns + # successfully (exit 0), not as a failure. + manager = PipelineRunManager(client=ExecutionStateClient(details={})) + manager.task_statuses = lambda root_id, deadline=None: {"a": "SKIPPED"} # type: ignore[method-assign] + + assert manager.wait_for_task_statuses("root-1") == {"a": "SKIPPED"} + + +def test_wait_for_task_statuses_returns_terminal_map(monkeypatch) -> None: + sleeps: list[float] = [] + monkeypatch.setattr("tangle_cli.pipeline_run_manager.time.sleep", lambda value: sleeps.append(value)) + statuses = iter([{"a": "RUNNING"}, {"a": "SUCCEEDED"}]) + manager = PipelineRunManager(client=ExecutionStateClient(details={})) + monkeypatch.setattr(manager, "task_statuses", lambda root_id, deadline=None: next(statuses)) + + assert manager.wait_for_task_statuses("root-1", poll_interval=1) == {"a": "SUCCEEDED"} + assert sleeps == [1] + + +def test_wait_for_task_statuses_max_wait_none_waits_until_terminal(monkeypatch) -> None: + # max_wait=None waits indefinitely: no deadline is computed and the plain + # poll interval is slept between checks. + sleeps: list[float] = [] + monkeypatch.setattr("tangle_cli.pipeline_run_manager.time.sleep", lambda value: sleeps.append(value)) + statuses = iter([{"a": "RUNNING"}, {"a": "RUNNING"}, {"a": "SUCCEEDED"}]) + manager = PipelineRunManager(client=ExecutionStateClient(details={})) + monkeypatch.setattr(manager, "task_statuses", lambda root_id, deadline=None: next(statuses)) + + result = manager.wait_for_task_statuses("root-1", max_wait=None, poll_interval=3) + + assert result == {"a": "SUCCEEDED"} + assert sleeps == [3, 3] + + +def test_wait_for_task_statuses_raises_on_failure() -> None: + manager = PipelineRunManager(client=ExecutionStateClient(details={})) + manager.task_statuses = lambda root_id, deadline=None: {"a": "SUCCEEDED", "b": "FAILED"} # type: ignore[method-assign] + + with pytest.raises(TaskStatusesFailed) as exc_info: + manager.wait_for_task_statuses("root-1") + # Failures drive the exit code, but the full final map is retained too. + assert exc_info.value.failures == {"b": "FAILED"} + assert exc_info.value.statuses == {"a": "SUCCEEDED", "b": "FAILED"} + + +def test_wait_for_task_statuses_allow_failure_returns_map() -> None: + manager = PipelineRunManager(client=ExecutionStateClient(details={})) + final = {"a": "SUCCEEDED", "b": "CANCELLED"} + manager.task_statuses = lambda root_id, deadline=None: final # type: ignore[method-assign] + + assert manager.wait_for_task_statuses("root-1", allow_failure=True) == final + + +def test_wait_for_task_statuses_timeout(monkeypatch) -> None: + monkeypatch.setattr("tangle_cli.pipeline_run_manager.time.sleep", lambda value: None) + # deadline calc, first check, sleep-clamp read, then second check past deadline. + ticks = iter([0.0, 0.0, 0.0, 100.0]) + monkeypatch.setattr("tangle_cli.pipeline_run_manager.time.monotonic", lambda: next(ticks)) + manager = PipelineRunManager(client=ExecutionStateClient(details={})) + manager.task_statuses = lambda root_id, deadline=None: {"a": "RUNNING"} # type: ignore[method-assign] + + with pytest.raises(PipelineRunError, match="non-terminal"): + manager.wait_for_task_statuses("root-1", max_wait=10, poll_interval=1) + + +def test_wait_for_task_statuses_clamps_final_sleep_to_remaining(monkeypatch) -> None: + # Near the deadline the final sleep must shrink to the remaining budget so the + # loop times out at max_wait instead of overshooting by a full poll interval. + sleeps: list[float] = [] + monkeypatch.setattr("tangle_cli.pipeline_run_manager.time.sleep", lambda value: sleeps.append(value)) + # deadline calc -> 0 (deadline=10); check -> 8 (not past); sleep clamp -> 8 + # (10-8=2 < poll_interval 5); next check -> 10 (timeout). + ticks = iter([0.0, 8.0, 8.0, 10.0]) + monkeypatch.setattr("tangle_cli.pipeline_run_manager.time.monotonic", lambda: next(ticks)) + manager = PipelineRunManager(client=ExecutionStateClient(details={})) + manager.task_statuses = lambda root_id, deadline=None: {"a": "RUNNING"} # type: ignore[method-assign] + + with pytest.raises(PipelineRunError, match="non-terminal"): + manager.wait_for_task_statuses("root-1", max_wait=10, poll_interval=5) + assert sleeps == [2.0] + + +def test_wait_for_task_statuses_retries_transient_root_fetch_5xx(monkeypatch) -> None: + # A single 5xx blip on the per-poll root details fetch must not abort a + # long wait; it is tolerated like child-state 5xx and retried next poll. + sleeps: list[float] = [] + monkeypatch.setattr("tangle_cli.pipeline_run_manager.time.sleep", lambda value: sleeps.append(value)) + outcomes = iter([TransientServerError("Fetching root execution root-1: request failed with HTTP 503"), {"a": "SUCCEEDED"}]) + + def fetch(root_id: str, deadline: float | None = None) -> dict[str, str]: + value = next(outcomes) + if isinstance(value, Exception): + raise value + return value + + logger = CaptureLogger() + manager = PipelineRunManager(client=ExecutionStateClient(details={}), logger=logger) + monkeypatch.setattr(manager, "task_statuses", fetch) + + assert manager.wait_for_task_statuses("root-1", poll_interval=1) == {"a": "SUCCEEDED"} + assert sleeps == [1] + # The tolerated blip is announced so a later timeout is diagnosable. + assert "HTTP 503" in (logger.get_logs() or "") + + +def test_wait_for_task_statuses_persistent_root_fetch_5xx_raises_at_deadline(monkeypatch) -> None: + # Retrying the root fetch is bounded by the wait deadline: a persistent + # outage surfaces as the underlying server error once max_wait elapses, + # and the warning is logged once per outage rather than once per poll. + monkeypatch.setattr("tangle_cli.pipeline_run_manager.time.sleep", lambda value: None) + # deadline calc, first-failure deadline check, sleep-clamp read, then + # second-failure deadline check past the deadline. + ticks = iter([0.0, 0.0, 0.0, 100.0]) + monkeypatch.setattr("tangle_cli.pipeline_run_manager.time.monotonic", lambda: next(ticks)) + logger = CaptureLogger() + manager = PipelineRunManager(client=ExecutionStateClient(details={}), logger=logger) + + def fetch(root_id: str, deadline: float | None = None) -> dict[str, str]: + raise TransientServerError("Fetching root execution root-1: request failed with HTTP 502") + + monkeypatch.setattr(manager, "task_statuses", fetch) + + with pytest.raises(TransientServerError, match="HTTP 502"): + manager.wait_for_task_statuses("root-1", max_wait=10, poll_interval=1) + assert (logger.get_logs() or "").count("HTTP 502") == 1 + + +def test_wait_for_task_statuses_unbounded_wait_caps_root_fetch_retries(monkeypatch) -> None: + # With max_wait=None there is no deadline to stop the retries, so a + # persistent root fetch outage must still surface after a bounded number + # of consecutive failures instead of being retried forever. + monkeypatch.setattr("tangle_cli.pipeline_run_manager.time.sleep", lambda value: None) + calls = 0 + + def fetch(root_id: str, deadline: float | None = None) -> dict[str, str]: + nonlocal calls + calls += 1 + raise TransientServerError("Fetching root execution root-1: request failed with HTTP 500") + + manager = PipelineRunManager(client=ExecutionStateClient(details={})) + monkeypatch.setattr(manager, "task_statuses", fetch) + + with pytest.raises(TransientServerError, match="HTTP 500"): + manager.wait_for_task_statuses("root-1", max_wait=None, poll_interval=1) + assert calls == pipeline_run_manager._MAX_UNBOUNDED_ROOT_FETCH_FAILURES + + +def test_wait_for_task_statuses_rejects_invalid_bounds() -> None: + manager = PipelineRunManager(client=ExecutionStateClient(details={})) + with pytest.raises(PipelineRunError, match="max-wait"): + manager.wait_for_task_statuses("root-1", max_wait=-1) + with pytest.raises(PipelineRunError, match="poll-interval"): + manager.wait_for_task_statuses("root-1", poll_interval=-1) + + +def test_wait_for_task_statuses_rejects_non_finite_bounds() -> None: + # NaN/inf bounds slip past the sign checks; they must reject cleanly instead + # of reaching time.sleep (raw ValueError) or creating an unbounded deadline. + manager = PipelineRunManager(client=ExecutionStateClient(details={})) + for bad in (math.nan, math.inf): + with pytest.raises( + PipelineRunError, match=r"max_wait \(--max-wait\) must be a finite number" + ): + manager.wait_for_task_statuses("root-1", max_wait=bad) + with pytest.raises( + PipelineRunError, + match=r"poll_interval \(--poll-interval\) must be a finite number", + ): + manager.wait_for_task_statuses("root-1", max_wait=1, poll_interval=bad) + + +def test_wait_for_task_statuses_rejects_zero_poll_interval_without_spinning(monkeypatch) -> None: + # A zero interval must be rejected before any polling, not busy-loop. + calls = 0 + + def counting_statuses(root_id: str, deadline: float | None = None) -> dict[str, str]: + nonlocal calls + calls += 1 + return {"a": "RUNNING"} + + manager = PipelineRunManager(client=ExecutionStateClient(details={})) + monkeypatch.setattr(manager, "task_statuses", counting_statuses) + + with pytest.raises( + PipelineRunError, match=r"poll_interval \(--poll-interval\) must be positive" + ): + manager.wait_for_task_statuses("root-1", max_wait=0.02, poll_interval=0) + assert calls == 0 + + +def test_cli_task_status_outputs_json(monkeypatch, capsys) -> None: + client = ExecutionStateClient( + details={"root-1": {"child_task_execution_ids": {"a": "exec-a"}}}, + container_state={"exec-a": {"status": "SUCCEEDED"}}, + ) + monkeypatch.setattr(pipeline_runs_cli, "LazyTangleApiClient", lambda **kwargs: client) + + run_app(cli.build_app(), ["sdk", "pipeline-runs", "task-status", "root-1"]) + + assert json.loads(capsys.readouterr().out) == {"a": "SUCCEEDED"} + + +def test_cli_task_wait_failure_exits_nonzero_and_prints_full_map(monkeypatch, capsys) -> None: + # Mixed outcome: the failure exit code must not drop the succeeded task. + client = ExecutionStateClient( + details={"root-1": {"child_task_execution_ids": {"a": "exec-a", "b": "exec-b"}}}, + container_state={ + "exec-a": {"status": "SUCCEEDED"}, + "exec-b": {"status": "FAILED"}, + }, + ) + monkeypatch.setattr(pipeline_runs_cli, "LazyTangleApiClient", lambda **kwargs: client) + + with pytest.raises(SystemExit) as exc_info: + # All tasks are already terminal, so the first poll returns without sleeping. + run_app(cli.build_app(), ["sdk", "pipeline-runs", "task-wait", "root-1", "--poll-interval", "1"]) + + assert exc_info.value.code == 2 + captured = capsys.readouterr() + assert json.loads(captured.out) == {"a": "SUCCEEDED", "b": "FAILED"} + # A human-readable failure summary goes to stderr; stdout stays pure JSON. + assert "failed task" in captured.err + + +def test_cli_task_wait_success_outputs_map(monkeypatch, capsys) -> None: + client = ExecutionStateClient( + details={"root-1": {"child_task_execution_ids": {"a": "exec-a"}}}, + container_state={"exec-a": {"status": "SUCCEEDED"}}, + ) + monkeypatch.setattr(pipeline_runs_cli, "LazyTangleApiClient", lambda **kwargs: client) + + run_app(cli.build_app(), ["sdk", "pipeline-runs", "task-wait", "root-1", "--poll-interval", "1"]) + + assert json.loads(capsys.readouterr().out) == {"a": "SUCCEEDED"} + + +def test_cli_task_wait_skipped_exits_zero(monkeypatch, capsys) -> None: + # SKIPPED is a non-failure terminal, so task-wait must exit 0 rather than + # raise SystemExit. + client = ExecutionStateClient( + details={"root-1": {"child_task_execution_ids": {"a": "exec-a"}}}, + container_state={"exec-a": {"status": "SKIPPED"}}, + ) + monkeypatch.setattr(pipeline_runs_cli, "LazyTangleApiClient", lambda **kwargs: client) + + run_app(cli.build_app(), ["sdk", "pipeline-runs", "task-wait", "root-1", "--poll-interval", "1"]) + + assert json.loads(capsys.readouterr().out) == {"a": "SKIPPED"} + + +def _write_secret_pipeline(path: Path) -> Path: + path.write_text( + yaml.safe_dump( + { + "name": "Secret Pipeline", + "inputs": [ + {"name": "query", "type": "String", "default": "default"}, + {"name": "api_key", "type": "String"}, + {"name": "db_password", "type": "String", "optional": True}, + {"name": "required", "type": "String"}, + ], + "implementation": {"graph": {"tasks": {}}}, + }, + sort_keys=False, + ), + encoding="utf-8", + ) + return path + + +def test_secret_argument_value_matches_oss_dynamic_data_contract() -> None: + assert secret_argument_value("OPENAI_KEY") == { + "dynamicData": {"secret": {"name": "OPENAI_KEY"}} + } + + +def test_parse_arg_secret_entries_trims_and_maps_inputs() -> None: + assert parse_arg_secret_entries([" api_key = OPENAI_KEY ", "db_password=PG"]) == { + "api_key": "OPENAI_KEY", + "db_password": "PG", + } + + +def test_parse_arg_secret_entries_defaults_to_empty() -> None: + assert parse_arg_secret_entries(None) == {} + + +@pytest.mark.parametrize("entry", ["api_key", "", " ", "=SECRET", "api_key=", "api_key= "]) +def test_parse_arg_secret_entries_rejects_malformed(entry: str) -> None: + with pytest.raises(PipelineRunError): + parse_arg_secret_entries([entry]) + + +def test_parse_arg_secret_entries_rejects_duplicate_input() -> None: + with pytest.raises(PipelineRunError, match="Duplicate --arg-secret for input 'api_key'"): + parse_arg_secret_entries(["api_key=A", "api_key=B"]) + + +def test_normalize_arg_secret_config_accepts_mapping() -> None: + assert normalize_arg_secret_config({"api_key": "OPENAI_KEY", " db ": " PG "}) == { + "api_key": "OPENAI_KEY", + "db": "PG", + } + + +def test_normalize_arg_secret_config_defaults_to_empty() -> None: + assert normalize_arg_secret_config(None) == {} + + +def test_normalize_arg_secret_config_rejects_non_mapping() -> None: + with pytest.raises(PipelineRunError, match="arg_secrets config must be a mapping"): + normalize_arg_secret_config(["api_key=OPENAI_KEY"]) + + +def test_normalize_arg_secret_config_rejects_non_string_secret() -> None: + with pytest.raises(PipelineRunError, match="must be a secret name string"): + normalize_arg_secret_config({"api_key": 123}) + + +def test_merge_secret_run_args_injects_dynamic_data() -> None: + merged = merge_secret_run_args({"required": "value"}, {"api_key": "OPENAI_KEY"}) + assert merged == { + "required": "value", + "api_key": {"dynamicData": {"secret": {"name": "OPENAI_KEY"}}}, + } + + +def test_merge_secret_run_args_rejects_value_and_secret_conflict() -> None: + with pytest.raises(PipelineRunError, match="both a value and a secret reference: api_key"): + merge_secret_run_args({"api_key": "plain"}, {"api_key": "OPENAI_KEY"}) + + +def test_pipeline_runs_submit_arg_secret_encodes_dynamic_data(monkeypatch, tmp_path: Path, capsys): + pipeline_path = _write_secret_pipeline(tmp_path / "pipeline.yaml") + fake_client = FakeClient() + monkeypatch.setattr(pipeline_runs_cli, "LazyTangleApiClient", lambda **kwargs: fake_client) + app = cli.build_app() + + run_app( + app, + [ + "sdk", + "pipeline-runs", + "submit", + str(pipeline_path), + "--no-hydrate", + "--arg", + "required=value", + "--arg-secret", + "api_key=OPENAI_KEY", + ], + ) + + capsys.readouterr() + arguments = fake_client.created[0]["root_task"]["arguments"] + assert arguments["required"] == "value" + assert arguments["api_key"] == {"dynamicData": {"secret": {"name": "OPENAI_KEY"}}} + + +def test_pipeline_runs_submit_multiple_arg_secrets(monkeypatch, tmp_path: Path, capsys): + pipeline_path = _write_secret_pipeline(tmp_path / "pipeline.yaml") + fake_client = FakeClient() + monkeypatch.setattr(pipeline_runs_cli, "LazyTangleApiClient", lambda **kwargs: fake_client) + app = cli.build_app() + + run_app( + app, + [ + "sdk", + "pipeline-runs", + "submit", + str(pipeline_path), + "--no-hydrate", + "--arg", + "required=value", + "--arg-secret", + "api_key=OPENAI_KEY", + "--arg-secret", + "db_password=PG_PASSWORD", + ], + ) + + capsys.readouterr() + arguments = fake_client.created[0]["root_task"]["arguments"] + assert arguments["api_key"] == {"dynamicData": {"secret": {"name": "OPENAI_KEY"}}} + assert arguments["db_password"] == {"dynamicData": {"secret": {"name": "PG_PASSWORD"}}} + + +def test_pipeline_runs_submit_arg_secret_dry_run_no_network(monkeypatch, tmp_path: Path, capsys): + pipeline_path = _write_secret_pipeline(tmp_path / "pipeline.yaml") + fake_client = FakeClient() + monkeypatch.setattr(pipeline_runs_cli, "LazyTangleApiClient", lambda **kwargs: fake_client) + app = cli.build_app() + + run_app( + app, + [ + "sdk", + "pipeline-runs", + "submit", + str(pipeline_path), + "--no-hydrate", + "--dry-run", + "--arg", + "required=value", + "--arg-secret", + "api_key=OPENAI_KEY", + ], + ) + + payload = json.loads(capsys.readouterr().out) + assert fake_client.created == [] + arguments = payload["root_task"]["arguments"] + assert arguments["api_key"] == {"dynamicData": {"secret": {"name": "OPENAI_KEY"}}} + + +def test_pipeline_runs_submit_arg_secret_conflict_rejected_without_submit( + monkeypatch, tmp_path: Path +): + pipeline_path = _write_secret_pipeline(tmp_path / "pipeline.yaml") + fake_client = FakeClient() + monkeypatch.setattr(pipeline_runs_cli, "LazyTangleApiClient", lambda **kwargs: fake_client) + app = cli.build_app() + + with pytest.raises(SystemExit) as excinfo: + app( + [ + "sdk", + "pipeline-runs", + "submit", + str(pipeline_path), + "--no-hydrate", + "--arg", + "api_key=plain", + "--arg-secret", + "api_key=OPENAI_KEY", + ] + ) + + assert "both a value and a secret reference" in str(excinfo.value) + assert fake_client.created == [] + + +def test_pipeline_runs_submit_arg_secret_conflicts_with_args_json(monkeypatch, tmp_path: Path): + pipeline_path = _write_secret_pipeline(tmp_path / "pipeline.yaml") + fake_client = FakeClient() + monkeypatch.setattr(pipeline_runs_cli, "LazyTangleApiClient", lambda **kwargs: fake_client) + app = cli.build_app() + + with pytest.raises(SystemExit) as excinfo: + app( + [ + "sdk", + "pipeline-runs", + "submit", + str(pipeline_path), + "--no-hydrate", + "--args-json", + json.dumps({"api_key": "plain"}), + "--arg-secret", + "api_key=OPENAI_KEY", + ] + ) + + assert "both a value and a secret reference" in str(excinfo.value) + assert fake_client.created == [] + + +def test_pipeline_runs_submit_arg_secret_malformed_skips_file_and_network( + monkeypatch, tmp_path: Path +): + fake_client = FakeClient() + monkeypatch.setattr(pipeline_runs_cli, "LazyTangleApiClient", lambda **kwargs: fake_client) + app = cli.build_app() + missing_path = tmp_path / "does-not-exist.yaml" + + with pytest.raises(SystemExit) as excinfo: + app( + [ + "sdk", + "pipeline-runs", + "submit", + str(missing_path), + "--no-hydrate", + "--arg-secret", + "api_key=", + ] + ) + + message = str(excinfo.value) + assert "non-empty input and secret name" in message + assert not missing_path.exists() + assert fake_client.created == [] + + +def test_pipeline_runs_submit_arg_secret_duplicate_rejected(monkeypatch, tmp_path: Path): + pipeline_path = _write_secret_pipeline(tmp_path / "pipeline.yaml") + fake_client = FakeClient() + monkeypatch.setattr(pipeline_runs_cli, "LazyTangleApiClient", lambda **kwargs: fake_client) + app = cli.build_app() + + with pytest.raises(SystemExit) as excinfo: + app( + [ + "sdk", + "pipeline-runs", + "submit", + str(pipeline_path), + "--no-hydrate", + "--arg-secret", + "api_key=A", + "--arg-secret", + "api_key=B", + ] + ) + + assert "Duplicate --arg-secret for input 'api_key'" in str(excinfo.value) + assert fake_client.created == [] + + +def test_pipeline_runs_submit_arg_secret_from_config(monkeypatch, tmp_path: Path): + pipeline_path = _write_secret_pipeline(tmp_path / "pipeline.yaml") + config = tmp_path / "pipeline.config.yaml" + config.write_text( + yaml.safe_dump( + { + "pipeline_path": str(pipeline_path), + "hydrate": False, + "args": {"required": "value"}, + "arg_secrets": {"api_key": "OPENAI_KEY"}, + }, + sort_keys=False, + ), + encoding="utf-8", + ) + fake_client = FakeClient() + monkeypatch.setattr(pipeline_runs_cli, "LazyTangleApiClient", lambda **kwargs: fake_client) + app = cli.build_app() + + run_app(app, ["sdk", "pipeline-runs", "submit", "--config", str(config)]) + + arguments = fake_client.created[0]["root_task"]["arguments"] + assert arguments["required"] == "value" + assert arguments["api_key"] == {"dynamicData": {"secret": {"name": "OPENAI_KEY"}}} + + +def test_pipeline_runs_submit_cli_arg_secret_overrides_config(monkeypatch, tmp_path: Path): + pipeline_path = _write_secret_pipeline(tmp_path / "pipeline.yaml") + config = tmp_path / "pipeline.config.yaml" + config.write_text( + yaml.safe_dump( + { + "pipeline_path": str(pipeline_path), + "hydrate": False, + "args": {"required": "value"}, + "arg_secrets": {"api_key": "FROM_CONFIG"}, + }, + sort_keys=False, + ), + encoding="utf-8", + ) + fake_client = FakeClient() + monkeypatch.setattr(pipeline_runs_cli, "LazyTangleApiClient", lambda **kwargs: fake_client) + app = cli.build_app() + + run_app( + app, + [ + "sdk", + "pipeline-runs", + "submit", + "--config", + str(config), + "--arg-secret", + "api_key=FROM_CLI", + ], + ) + + arguments = fake_client.created[0]["root_task"]["arguments"] + assert arguments["api_key"] == {"dynamicData": {"secret": {"name": "FROM_CLI"}}} + + +def test_pipeline_runs_logs_stream_prints_lines(monkeypatch, capsys) -> None: + fake_client = FakeClient() + monkeypatch.setattr(pipeline_runs_cli, "LazyTangleApiClient", lambda **kwargs: fake_client) + app = cli.build_app() + + run_app(app, ["sdk", "pipeline-runs", "logs", "exec-7", "--stream"]) + + assert capsys.readouterr().out == "stream exec-7 line 1\nstream exec-7 line 2\n" + + +def test_pipeline_runs_logs_snapshot_is_default_when_stream_absent(monkeypatch, capsys) -> None: + class TrackingClient(FakeClient): + def __init__(self) -> None: + super().__init__() + self.streamed_ids: list[str] = [] + + def iter_execution_container_log_lines(self, id: str): + self.streamed_ids.append(id) + yield from () + + fake_client = TrackingClient() + monkeypatch.setattr(pipeline_runs_cli, "LazyTangleApiClient", lambda **kwargs: fake_client) + app = cli.build_app() + + run_app(app, ["sdk", "pipeline-runs", "logs", "exec-1"]) + + assert capsys.readouterr().out == "logs for exec-1\n" + assert fake_client.streamed_ids == [] + + +def test_stream_logs_open_transport_error_is_clean() -> None: + class RefusingClient(FakeClient): + def iter_execution_container_log_lines(self, id: str): + raise requests.ConnectionError("connection refused") + + manager = pipeline_run_manager.PipelineRunManager(client=RefusingClient()) + + with pytest.raises( + PipelineRunError, match="Failed to open log stream for execution exec-1" + ): + manager.stream_logs("exec-1") + + +def test_stream_logs_body_timeout_reset_error_is_clean() -> None: + class UnconfigurableStreamClient(FakeClient): + def iter_execution_container_log_lines(self, id: str): + raise requests.ConnectionError( + "opened log stream but could not disable the body read timeout" + ) + + manager = pipeline_run_manager.PipelineRunManager(client=UnconfigurableStreamClient()) + + with pytest.raises( + PipelineRunError, match="Failed to open log stream for execution exec-1" + ) as exc_info: + manager.stream_logs("exec-1") + assert "could not disable the body read timeout" in str(exc_info.value) + + +def test_pipeline_runs_logs_stream_open_failure_exits_cleanly(monkeypatch) -> None: + class RefusingClient(FakeClient): + def iter_execution_container_log_lines(self, id: str): + raise requests.ConnectionError("connection refused") + + fake_client = RefusingClient() + monkeypatch.setattr(pipeline_runs_cli, "LazyTangleApiClient", lambda **kwargs: fake_client) + app = cli.build_app() + + with pytest.raises(SystemExit) as exc_info: + app(["sdk", "pipeline-runs", "logs", "exec-1", "--stream"]) + + assert "Failed to open log stream for execution exec-1" in str(exc_info.value) + assert "connection refused" in str(exc_info.value) + + +def test_pipeline_runs_logs_stream_broken_pipe_exits_cleanly(monkeypatch) -> None: + # Simulates `... logs --stream | head`: the downstream reader closes the + # pipe, so the first stdout write raises BrokenPipeError. The command must + # stop following and exit cleanly instead of tracebacking. + fake_client = FakeClient() + monkeypatch.setattr(pipeline_runs_cli, "LazyTangleApiClient", lambda **kwargs: fake_client) + devnull_fd = os.open(os.devnull, os.O_WRONLY) + write_attempts = {"count": 0} + + class ClosedPipeStdout(io.TextIOBase): + def write(self, _text: str) -> int: + write_attempts["count"] += 1 + raise BrokenPipeError + + def fileno(self) -> int: + return devnull_fd + + monkeypatch.setattr(sys, "stdout", ClosedPipeStdout()) + app = cli.build_app() + + try: + run_app(app, ["sdk", "pipeline-runs", "logs", "exec-7", "--stream"]) + finally: + os.close(devnull_fd) + + assert write_attempts["count"] == 1 + + +def test_pipeline_runs_logs_stream_ctrl_c_exits_cleanly(monkeypatch, capsys) -> None: + # An interactive Ctrl-C is the normal way to stop a live follow; it must + # exit with the conventional interrupt code (128 + SIGINT), not a + # KeyboardInterrupt traceback. cyclopts provides this by default + # (suppress_keyboard_interrupt); this locks that contract in for --stream. + class InterruptedStreamClient(FakeClient): + def iter_execution_container_log_lines(self, id: str): + yield "line-1" + raise KeyboardInterrupt + + fake_client = InterruptedStreamClient() + monkeypatch.setattr(pipeline_runs_cli, "LazyTangleApiClient", lambda **kwargs: fake_client) + app = cli.build_app() + + with pytest.raises(SystemExit) as exc_info: + app(["sdk", "pipeline-runs", "logs", "exec-1", "--stream"]) + + assert exc_info.value.code == 130 + assert capsys.readouterr().out == "line-1\n" + + +def test_pipeline_runs_logs_help_documents_stream(capsys) -> None: + app = cli.build_app() + run_app(app, ["sdk", "pipeline-runs", "logs", "--help"]) + assert "--stream" in capsys.readouterr().out + + +def test_pipeline_runs_stream_logs_uses_client_iterator() -> None: + fake_client = FakeClient() + manager = PipelineRunManager(client=fake_client) + + assert list(manager.stream_logs("exec-9")) == ["stream exec-9 line 1", "stream exec-9 line 2"] + + +def test_pipeline_runs_stream_logs_midstream_error_is_interruption() -> None: + # A transport failure after some lines have flowed must surface as a + # mid-stream interruption, not the "failed to open" wording reserved for a + # stream that never opened. + class MidStreamDropClient(FakeClient): + def iter_execution_container_log_lines(self, id: str): + yield "line-1" + raise requests.exceptions.ChunkedEncodingError("connection broken mid-stream") + + manager = PipelineRunManager(client=MidStreamDropClient()) + + gen = iter(manager.stream_logs("exec-9")) + assert next(gen) == "line-1" + with pytest.raises(PipelineRunError, match="interrupted") as exc_info: + next(gen) + assert "Failed to open" not in str(exc_info.value) + + +def test_pipeline_runs_stream_logs_zero_line_drop_is_interruption() -> None: + # A connection that opens successfully but drops before the first line is + # still a mid-stream interruption; only open failures (raised from the + # hook call itself) get the "failed to open" wording. + class ZeroLineDropClient(FakeClient): + def iter_execution_container_log_lines(self, id: str): + raise requests.exceptions.ChunkedEncodingError("Response ended prematurely") + yield # pragma: no cover + + manager = PipelineRunManager(client=ZeroLineDropClient()) + + with pytest.raises(PipelineRunError, match="interrupted") as exc_info: + list(manager.stream_logs("exec-9")) + assert "Failed to open" not in str(exc_info.value) + + +def test_pipeline_runs_logs_stream_honors_log_type_none(monkeypatch, capsys) -> None: + # --log-type controls the CLI's diagnostic logger and is orthogonal to + # --stream; the streamed log content still reaches stdout under any mode. + fake_client = FakeClient() + monkeypatch.setattr(pipeline_runs_cli, "LazyTangleApiClient", lambda **kwargs: fake_client) + app = cli.build_app() + + run_app(app, ["sdk", "pipeline-runs", "logs", "exec-7", "--stream", "--log-type", "none"]) + + assert capsys.readouterr().out == "stream exec-7 line 1\nstream exec-7 line 2\n" + + +def _logs_http_error(status: int, path: str) -> requests.HTTPError: + response = requests.Response() + response.status_code = status + response.request = requests.Request("GET", f"https://api.test{path}").prepare() + return requests.HTTPError(f"{status} Client Error", response=response) + + +def test_pipeline_runs_logs_stream_missing_endpoint_is_clean(monkeypatch) -> None: + class NoStreamClient(FakeClient): + def iter_execution_container_log_lines(self, id: str): + # Like the real client, open failures raise from the call itself + # rather than on first iteration. + raise _logs_http_error(404, f"/api/executions/{id}/stream_container_log") + + fake_client = NoStreamClient() + monkeypatch.setattr(pipeline_runs_cli, "LazyTangleApiClient", lambda **kwargs: fake_client) + app = cli.build_app() + + # A clean SystemExit (not a raw requests.HTTPError) means no traceback. + with pytest.raises(SystemExit) as exc_info: + app(["sdk", "pipeline-runs", "logs", "exec-1", "--stream"]) + + message = str(exc_info.value) + assert "404" in message + assert "/api/executions/exec-1/stream_container_log" in message diff --git a/tests/test_secrets_cli.py b/tests/test_secrets_cli.py index 6072131..f61b17f 100644 --- a/tests/test_secrets_cli.py +++ b/tests/test_secrets_cli.py @@ -6,6 +6,7 @@ import sys from types import SimpleNamespace from typing import Any +from unittest.mock import ANY import pytest @@ -341,6 +342,7 @@ def test_sdk_secrets_config_array_and_config_base_url_credential_isolation( "header": ["X-Config: yes"], "include_env_credentials": False, "command_name": "secret commands", + "logger": ANY, }, { "base_url": "https://config.example", @@ -349,6 +351,7 @@ def test_sdk_secrets_config_array_and_config_base_url_credential_isolation( "header": ["X-Config: yes"], "include_env_credentials": False, "command_name": "secret commands", + "logger": ANY, }, ] assert [instance.calls[0]["secret_name"] for instance in FakeLazyTangleApiClient.instances] == [ diff --git a/tests/test_submit_and_wait_prepared_body.py b/tests/test_submit_and_wait_prepared_body.py new file mode 100644 index 0000000..04877e4 --- /dev/null +++ b/tests/test_submit_and_wait_prepared_body.py @@ -0,0 +1,461 @@ +from __future__ import annotations + +import builtins +import copy +import json +from typing import Any + +import pytest +from tangle_cli.logger import CaptureLogger +from tangle_cli.pipeline_run_manager import ( + PipelineRunError, + PipelineRunManager, + submit_and_wait_prepared_body, +) + +RUN_ID = "run-1" +ROOT_EXECUTION_ID = "exec-1" + +# The submit lifecycle injects a submission-id annotation into the (deep-copied) +# body before submit so post-failure recovery can find the created run. Strip it +# when comparing the submitted body to the caller's original. +_SUBMISSION_ANNOTATION_KEY = "tangle-cli/submission-id" + + +def _without_submission_annotation(body: dict[str, Any]) -> dict[str, Any]: + stripped = copy.deepcopy(body) + annotations = stripped.get("annotations") + if isinstance(annotations, dict): + annotations.pop(_SUBMISSION_ANNOTATION_KEY, None) + if not annotations: + stripped.pop("annotations", None) + return stripped + + +def _prepared_body() -> dict[str, Any]: + return { + "root_task": { + "componentRef": { + "spec": {"name": "Prepared", "implementation": {"graph": {"tasks": {}}}} + }, + "arguments": {"query": "value"}, + } + } + + +class _SubmitClient: + """Minimal API client: submit succeeds, status reports terminal SUCCEEDED.""" + + def __init__(self) -> None: + self.created: list[Any] = [] + self.get_calls: int = 0 + + def pipeline_runs_create(self, body: Any = None) -> dict[str, Any]: + self.created.append(copy.deepcopy(body)) + return {"id": RUN_ID, "root_execution_id": ROOT_EXECUTION_ID} + + def pipeline_runs_get(self, id: str, include_execution_stats: bool | None = None) -> dict[str, Any]: + self.get_calls += 1 + return { + "id": id, + "root_execution_id": ROOT_EXECUTION_ID, + "execution_summary": {"has_ended": True}, + "execution_status_stats": {"SUCCEEDED": 1}, + } + + +def _locator_body() -> dict[str, Any]: + """A fully-formed submit body that references a component by name/digest. + + This shape has no inline ``componentRef.spec``; the helper must submit it + verbatim instead of failing while trying to read a spec. + """ + return { + "root_task": { + "componentRef": {"name": "my-pipeline", "digest": "sha256:abc123"}, + "arguments": {"query": "value"}, + } + } + + +def test_submit_only_returns_run_metadata_without_wait() -> None: + client = _SubmitClient() + result = submit_and_wait_prepared_body(_prepared_body(), client=client, wait=False) + + assert result["run_id"] == RUN_ID + assert result["root_execution_id"] == ROOT_EXECUTION_ID + assert result["response"] == {"id": RUN_ID, "root_execution_id": ROOT_EXECUTION_ID} + assert "wait" not in result + assert client.get_calls == 0 + # No PipelineRunContext / attempts leak into the default output. + assert set(result) == {"response", "run_id", "root_execution_id"} + assert json.dumps(result) + + +def test_submit_and_wait_success_includes_serializable_wait_result() -> None: + client = _SubmitClient() + result = submit_and_wait_prepared_body( + _prepared_body(), client=client, wait=True, poll_interval=0.01 + ) + + assert result["run_id"] == RUN_ID + assert result["wait"]["status"] == "SUCCEEDED" + assert result["wait"]["timed_out"] is False + assert client.get_calls >= 1 + assert json.dumps(result) + + +def test_timeout_metadata_preserved_and_serializable() -> None: + class _NeverTerminalClient(_SubmitClient): + def pipeline_runs_get( + self, id: str, include_execution_stats: bool | None = None + ) -> dict[str, Any]: + self.get_calls += 1 + return { + "id": id, + "root_execution_id": ROOT_EXECUTION_ID, + "execution_summary": {"has_ended": False}, + "execution_status_stats": {"RUNNING": 1}, + } + + result = submit_and_wait_prepared_body( + _prepared_body(), + client=_NeverTerminalClient(), + wait=True, + max_wait=0.0, + poll_interval=0.01, + ) + + assert result["run_id"] == RUN_ID + assert result["wait"]["timed_out"] is True + assert "status_counts" in result["wait"] + assert json.dumps(result) + + +def test_exit_on_first_failure_returns_serializable_early_exit() -> None: + class _FailingGraphClient(_SubmitClient): + # Run stays nonterminal at the top level; graph state reports a FAILED + # child alongside a still-running one, so exit_on_first_failure trips. + def pipeline_runs_get( + self, id: str, include_execution_stats: bool | None = None + ) -> dict[str, Any]: + self.get_calls += 1 + return { + "id": id, + "root_execution_id": ROOT_EXECUTION_ID, + "execution_status_stats": {"RUNNING": 1}, + } + + def executions_graph_execution_state(self, id: str) -> dict[str, Any]: + return {"status_totals": {"RUNNING": 1, "FAILED": 1}} + + result = submit_and_wait_prepared_body( + _prepared_body(), + client=_FailingGraphClient(), + wait=True, + use_graph_state=True, + exit_on_first_failure=True, + poll_interval=0.01, + ) + + assert result["run_id"] == RUN_ID + assert result["wait"]["early_exit"] is True + assert result["wait"]["timed_out"] is False + assert result["wait"]["failed_count"] == 1 + # Default output stays JSON-serializable and free of context/attempts leakage. + assert "context" not in result + assert "attempts" not in result + assert json.dumps(result) + + +def test_invalid_poll_interval_raises_before_submit() -> None: + client = _SubmitClient() + with pytest.raises(PipelineRunError): + submit_and_wait_prepared_body( + _prepared_body(), client=client, wait=True, poll_interval=0 + ) + # The run must never be submitted when the wait params are invalid. + assert client.created == [] + + +def test_negative_max_wait_raises_before_submit() -> None: + client = _SubmitClient() + with pytest.raises(PipelineRunError): + submit_and_wait_prepared_body( + _prepared_body(), client=client, wait=True, max_wait=-1.0 + ) + assert client.created == [] + + +@pytest.mark.parametrize("bad", [float("nan"), float("inf"), float("-inf")]) +def test_non_finite_max_wait_raises_before_submit(bad: float) -> None: + # NaN/inf pass sign checks (NaN compares False, inf is "positive") and + # would become a never-firing deadline; they must be rejected up front. + client = _SubmitClient() + with pytest.raises(PipelineRunError, match=r"max_wait \(--max-wait\) must be a finite number"): + submit_and_wait_prepared_body( + _prepared_body(), client=client, wait=True, max_wait=bad + ) + assert client.created == [] + + +@pytest.mark.parametrize("bad", [float("nan"), float("inf"), float("-inf")]) +def test_non_finite_poll_interval_raises_before_submit(bad: float) -> None: + client = _SubmitClient() + with pytest.raises(PipelineRunError, match=r"poll_interval \(--poll-interval\) must be a finite number"): + submit_and_wait_prepared_body( + _prepared_body(), client=client, wait=True, poll_interval=bad + ) + assert client.created == [] + + +def test_invalid_timeout_clock_raises_before_submit() -> None: + client = _SubmitClient() + with pytest.raises(PipelineRunError): + submit_and_wait_prepared_body( + _prepared_body(), + client=client, + wait=True, + poll_interval=0.01, + timeout_clock="bogus", + ) + assert client.created == [] + + +def test_wait_true_without_run_id_raises_instead_of_silent_no_wait() -> None: + class _NoRunIdClient(_SubmitClient): + def pipeline_runs_create(self, body: Any = None) -> dict[str, Any]: + self.created.append(copy.deepcopy(body)) + return {"root_execution_id": ROOT_EXECUTION_ID} + + client = _NoRunIdClient() + with pytest.raises(PipelineRunError, match="did not include a run id"): + submit_and_wait_prepared_body( + _prepared_body(), client=client, wait=True, poll_interval=0.01 + ) + # The run was submitted; only the wait was refused. + assert len(client.created) == 1 + assert client.get_calls == 0 + + # wait=False keeps the id-less response inspectable. + result = submit_and_wait_prepared_body(_prepared_body(), client=client, wait=False) + assert result["run_id"] is None + assert "wait" not in result + + +@pytest.mark.parametrize( + "body", [{"arguments": {}}, {"root_task": None}, {"root_task": "not-a-mapping"}] +) +def test_body_without_root_task_mapping_fails_before_submit(body: dict) -> None: + client = _SubmitClient() + with pytest.raises(PipelineRunError, match="root_task"): + submit_and_wait_prepared_body(body, client=client, wait=False) + assert client.created == [] + + +@pytest.mark.parametrize( + "root_task", + [{"arguments": {}}, {"componentRef": None}, {"componentRef": ["not-a-mapping"]}], +) +def test_body_without_component_ref_mapping_fails_before_submit(root_task: dict) -> None: + client = _SubmitClient() + with pytest.raises(PipelineRunError, match="componentRef"): + submit_and_wait_prepared_body( + {"root_task": root_task}, client=client, wait=False + ) + assert client.created == [] + + +def test_caller_body_not_mutated() -> None: + body = _prepared_body() + original = copy.deepcopy(body) + submit_and_wait_prepared_body(body, client=_SubmitClient(), wait=True, poll_interval=0.01) + assert body == original + + +def test_manager_and_client_are_mutually_exclusive() -> None: + manager = PipelineRunManager(client=_SubmitClient()) + with pytest.raises(PipelineRunError): + submit_and_wait_prepared_body( + _prepared_body(), manager=manager, client=_SubmitClient() + ) + + +def test_locator_body_without_inline_spec_submits_original_body() -> None: + client = _SubmitClient() + body = _locator_body() + original = copy.deepcopy(body) + + result = submit_and_wait_prepared_body(body, client=client, wait=False) + + # The client receives the original locator body (modulo the submission-id + # annotation the submit lifecycle injects for post-failure recovery). + assert len(client.created) == 1 + assert _without_submission_annotation(client.created[0]) == original + assert result["run_id"] == RUN_ID + assert result["root_execution_id"] == ROOT_EXECUTION_ID + assert body == original + + +def test_manager_submit_prepared_body_accepts_locator_body() -> None: + """Regression for the shipped submit path (not just the new helper): + ``PipelineRunManager.submit_prepared_body`` used to raise ``KeyError`` on a + locator-style body with no inline ``componentRef.spec``; it must now submit + the body verbatim with a spec-less run context.""" + + client = _SubmitClient() + manager = PipelineRunManager(client=client) + body = _locator_body() + original = copy.deepcopy(body) + + response = manager.submit_prepared_body(body) + + # submit_prepared_body normalizes/submits the caller's body directly (no + # submission-id annotation injection, which lives in the run lifecycle). + assert client.created == [original] + assert response == {"id": RUN_ID, "root_execution_id": ROOT_EXECUTION_ID} + assert body == original + + +def test_partial_native_package_raises_actionable_error( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A partially-installed native package (missing ``tangle_api.generated``) + should surface the actionable install hint, not a raw ModuleNotFoundError.""" + + real_import = builtins.__import__ + + def fake_import( + name: str, + globals: Any = None, + locals: Any = None, + fromlist: Any = (), + level: int = 0, + ) -> Any: + # Intercept the lazy ``from .client import TangleApiClient`` and fail as + # if a native submodule were missing rather than the top-level package. + if level == 1 and name == "client" and fromlist and "TangleApiClient" in fromlist: + raise ModuleNotFoundError( + "No module named 'tangle_api.generated'", name="tangle_api.generated" + ) + return real_import(name, globals, locals, fromlist, level) + + monkeypatch.setattr(builtins, "__import__", fake_import) + + # No manager/client -> the default manager path performs the native import. + with pytest.raises(PipelineRunError, match="Native generated Tangle API bindings"): + submit_and_wait_prepared_body(_prepared_body()) + + +def test_explicit_manager_is_reused() -> None: + client = _SubmitClient() + manager = PipelineRunManager(client=client) + result = submit_and_wait_prepared_body( + _prepared_body(), manager=manager, wait=False + ) + assert result["run_id"] == RUN_ID + assert client.created # the provided manager's client handled the submit + + +class _RecoveringClient(_SubmitClient): + """Submit dies without a response although the run was actually created; + the run is then discoverable via the submission-id list lookup.""" + + def __init__(self) -> None: + super().__init__() + self.list_calls: list[dict[str, Any]] = [] + + def pipeline_runs_create(self, body: Any = None) -> dict[str, Any]: + self.created.append(copy.deepcopy(body)) + raise TimeoutError("submit connection dropped") + + def pipeline_runs_list(self, **kwargs: Any) -> dict[str, Any]: + self.list_calls.append(kwargs) + return { + "pipeline_runs": [{"id": RUN_ID, "root_execution_id": ROOT_EXECUTION_ID}] + } + + +def test_submit_failure_adopts_run_recovered_by_submission_id(monkeypatch) -> None: + # Recovery finds the already-created run by the injected submission-id + # annotation and adopts it instead of resubmitting a duplicate. + monkeypatch.setattr("tangle_cli.pipeline_run_manager.time.sleep", lambda _delay: None) + client = _RecoveringClient() + + result = submit_and_wait_prepared_body(_prepared_body(), client=client, wait=False) + + assert result["run_id"] == RUN_ID + assert result["root_execution_id"] == ROOT_EXECUTION_ID + # Exactly one submit was attempted; the run was adopted, not resubmitted. + assert len(client.created) == 1 + submission_id = client.created[0]["annotations"][_SUBMISSION_ANNOTATION_KEY] + assert len(client.list_calls) == 1 + assert submission_id in client.list_calls[0]["filter_query"] + + +def test_submit_recovery_attempts_zero_disables_lookup(monkeypatch) -> None: + class _FailingClient(_SubmitClient): + def __init__(self) -> None: + super().__init__() + self.list_calls: list[dict[str, Any]] = [] + + def pipeline_runs_create(self, body: Any = None) -> dict[str, Any]: + self.created.append(copy.deepcopy(body)) + raise TimeoutError("submit connection dropped") + + def pipeline_runs_list(self, **kwargs: Any) -> dict[str, Any]: + self.list_calls.append(kwargs) + return {"pipeline_runs": []} + + monkeypatch.setattr("tangle_cli.pipeline_run_manager.time.sleep", lambda _delay: None) + client = _FailingClient() + + with pytest.raises(TimeoutError): + submit_and_wait_prepared_body( + _prepared_body(), client=client, wait=False, submit_recovery_attempts=0 + ) + + assert client.list_calls == [] + + +def test_logger_used_when_helper_builds_the_manager(monkeypatch) -> None: + # The recovery path logs through the manager's logger, so it makes the + # logger= wiring observable: with client=, the helper must hand the caller's + # logger to the manager it builds. + monkeypatch.setattr("tangle_cli.pipeline_run_manager.time.sleep", lambda _delay: None) + logger = CaptureLogger() + + result = submit_and_wait_prepared_body( + _prepared_body(), client=_RecoveringClient(), logger=logger, wait=False + ) + + assert result["run_id"] == RUN_ID + assert "Recovered existing pipeline run" in (logger.get_logs() or "") + + +def test_logger_ignored_when_manager_is_supplied(monkeypatch) -> None: + # Documented contract: a supplied manager keeps its own configured logger. + monkeypatch.setattr("tangle_cli.pipeline_run_manager.time.sleep", lambda _delay: None) + manager_logger = CaptureLogger() + ignored_logger = CaptureLogger() + manager = PipelineRunManager(client=_RecoveringClient(), logger=manager_logger) + + result = submit_and_wait_prepared_body( + _prepared_body(), manager=manager, logger=ignored_logger, wait=False + ) + + assert result["run_id"] == RUN_ID + assert "Recovered existing pipeline run" in (manager_logger.get_logs() or "") + assert ignored_logger.get_logs() is None + + +def test_wait_with_max_wait_none_waits_without_deadline() -> None: + client = _SubmitClient() + + result = submit_and_wait_prepared_body( + _prepared_body(), client=client, wait=True, max_wait=None, poll_interval=0.01 + ) + + assert result["wait"]["status"] == "SUCCEEDED" + assert result["wait"]["timed_out"] is False + assert client.get_calls >= 1 diff --git a/tests/test_tls_verification.py b/tests/test_tls_verification.py new file mode 100644 index 0000000..46318f5 --- /dev/null +++ b/tests/test_tls_verification.py @@ -0,0 +1,764 @@ +"""Tests for centralized TLS verification configuration across transports. + +Covers environment parsing, explicit precedence, path handling, preservation of +caller-supplied session settings, and propagation into the requests client, the +httpx schema/operation transport, and the dynamic-discovery client. A real +local HTTPS server with a generated private CA exercises the end-to-end +behavior of the secure default, ``TANGLE_API_CA_BUNDLE``, and +``TANGLE_API_VERIFY_TLS=0``. +""" + +from __future__ import annotations + +import http.server +import json +import os +import shutil +import ssl +import subprocess +import sys +import threading +from pathlib import Path +from types import SimpleNamespace +from typing import Any, Iterator + +import httpx +import pytest +import requests + +from tangle_cli import cli +from tangle_cli.api_cli import _api_argv_tail +from tangle_cli.api_schema import fetch_schema +from tangle_cli.api_transport import ( + _VERIFY_UNSET, + configure_cli_verify, + httpx_verify, + request_operation, + resolve_verify, + resolve_verify_default, +) +from tangle_cli.client import TangleApiClient +from tangle_cli.dynamic_discovery_client import TangleDynamicDiscoveryClient + +_TLS_ENV_VARS = ("TANGLE_API_VERIFY_TLS", "TANGLE_API_CA_BUNDLE") + + +@pytest.fixture(autouse=True) +def _clear_tls_env(monkeypatch: pytest.MonkeyPatch) -> Iterator[None]: + for name in _TLS_ENV_VARS: + monkeypatch.delenv(name, raising=False) + configure_cli_verify() # ensure no leaked process-wide CLI override + yield + configure_cli_verify() + + +# --------------------------------------------------------------------------- # +# Environment parsing and precedence +# --------------------------------------------------------------------------- # + + +def test_resolve_verify_defaults_to_unset() -> None: + assert resolve_verify() is _VERIFY_UNSET + assert resolve_verify_default() is True + assert httpx_verify() is True + + +@pytest.mark.parametrize("value", ["0", "false", "FALSE", " no ", "No", "nO"]) +def test_verify_tls_false_values_disable_verification( + monkeypatch: pytest.MonkeyPatch, value: str +) -> None: + monkeypatch.setenv("TANGLE_API_VERIFY_TLS", value) + assert resolve_verify() is False + assert httpx_verify() is False + + +@pytest.mark.parametrize("value", ["1", "true", "yes", "on", "enabled", "anything"]) +def test_verify_tls_other_nonempty_values_keep_verification( + monkeypatch: pytest.MonkeyPatch, value: str +) -> None: + monkeypatch.setenv("TANGLE_API_VERIFY_TLS", value) + assert resolve_verify() is True + + +def test_empty_verify_tls_is_treated_as_unset(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("TANGLE_API_VERIFY_TLS", " ") + assert resolve_verify() is _VERIFY_UNSET + + +def test_ca_bundle_env_resolves_to_path( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + bundle = tmp_path / "ca.pem" + bundle.write_text("cert", encoding="utf-8") + monkeypatch.setenv("TANGLE_API_CA_BUNDLE", str(bundle)) + assert resolve_verify() == str(bundle) + + +def test_empty_ca_bundle_is_treated_as_unset(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("TANGLE_API_CA_BUNDLE", " ") + assert resolve_verify() is _VERIFY_UNSET + + +def test_missing_ca_bundle_fails_early(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("TANGLE_API_CA_BUNDLE", "/nonexistent/ca.pem") + with pytest.raises(SystemExit, match="TANGLE_API_CA_BUNDLE"): + resolve_verify() + + +def test_ca_bundle_wins_over_verify_tls( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + bundle = tmp_path / "ca.pem" + bundle.write_text("cert", encoding="utf-8") + monkeypatch.setenv("TANGLE_API_CA_BUNDLE", str(bundle)) + monkeypatch.setenv("TANGLE_API_VERIFY_TLS", "0") + # CA bundle wins and TLS stays verified against the bundle. + assert resolve_verify() == str(bundle) + + +def test_explicit_argument_wins_over_environment( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + bundle = tmp_path / "ca.pem" + bundle.write_text("cert", encoding="utf-8") + monkeypatch.setenv("TANGLE_API_CA_BUNDLE", str(bundle)) + assert resolve_verify(False) is False + assert resolve_verify(True) is True + + +def test_explicit_pathlike_argument_is_accepted(tmp_path: Path) -> None: + bundle = tmp_path / "ca.pem" + bundle.write_text("cert", encoding="utf-8") + assert resolve_verify(bundle) == str(bundle) + + +def test_explicit_missing_path_argument_fails_early(tmp_path: Path) -> None: + with pytest.raises(SystemExit, match="verify"): + resolve_verify(str(tmp_path / "missing.pem")) + + +def test_none_argument_falls_through_to_environment( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv("TANGLE_API_VERIFY_TLS", "0") + assert resolve_verify(None) is False + + +# --------------------------------------------------------------------------- # +# httpx adapter +# --------------------------------------------------------------------------- # + + +def test_httpx_verify_converts_path_to_ssl_context(tmp_path: Path) -> None: + ca = _generate_private_ca(tmp_path) + context = httpx_verify(str(ca.ca_pem)) + assert isinstance(context, ssl.SSLContext) + + +def test_httpx_verify_passes_booleans_through() -> None: + assert httpx_verify(True) is True + assert httpx_verify(False) is False + + +# --------------------------------------------------------------------------- # +# Transport propagation (mocked) +# --------------------------------------------------------------------------- # + + +def _operation(path: str, *, method: str = "GET") -> SimpleNamespace: + return SimpleNamespace( + method=method, + path=path, + parameters=[], + group_name="test", + command_name="op", + has_request_body=False, + ) + + +def test_request_operation_passes_verify_to_httpx( + monkeypatch: pytest.MonkeyPatch, +) -> None: + captured: dict[str, Any] = {} + + def fake_request(method: str, url: str, **kwargs: Any) -> httpx.Response: + captured.update(kwargs) + return httpx.Response(200, json={}, request=httpx.Request(method, url)) + + monkeypatch.setattr("tangle_cli.api_transport.httpx.request", fake_request) + request_operation( + _operation("/api/ping"), + {}, + base_url="https://api.test", + verify=False, + ) + assert captured["verify"] is False + + +def test_request_operation_defaults_to_secure_verify( + monkeypatch: pytest.MonkeyPatch, +) -> None: + captured: dict[str, Any] = {} + + def fake_request(method: str, url: str, **kwargs: Any) -> httpx.Response: + captured.update(kwargs) + return httpx.Response(200, json={}, request=httpx.Request(method, url)) + + monkeypatch.setattr("tangle_cli.api_transport.httpx.request", fake_request) + request_operation(_operation("/api/ping"), {}, base_url="https://api.test") + assert captured["verify"] is True + + +def test_fetch_schema_passes_verify_to_httpx(monkeypatch: pytest.MonkeyPatch) -> None: + captured: dict[str, Any] = {} + + def fake_get(url: str, **kwargs: Any) -> httpx.Response: + captured.update(kwargs) + return httpx.Response( + 200, + json={"openapi": "3.1.0", "paths": {}}, + request=httpx.Request("GET", url), + ) + + monkeypatch.setattr("tangle_cli.api_schema.httpx.get", fake_get) + fetch_schema("https://api.test", verify=False) + assert captured["verify"] is False + + +class _RecordingSession: + """Minimal session without a ``.verify`` attribute.""" + + def __init__(self) -> None: + self.calls: list[dict[str, Any]] = [] + + def request(self, method: str, url: str, **kwargs: Any) -> requests.Response: + self.calls.append({"method": method, "url": url, **kwargs}) + r = requests.Response() + r.status_code = 200 + r._content = b"{}" + r.headers["Content-Type"] = "application/json" + r.request = requests.Request(method, url).prepare() + return r + + +def test_static_client_injects_verify_when_configured( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv("TANGLE_API_VERIFY_TLS", "0") + session = _RecordingSession() + client = TangleApiClient("https://api.test", session=session) + + client._make_request("GET", "/api/ping") + + assert session.calls[0]["verify"] is False + + +def test_static_client_preserves_session_when_unset() -> None: + session = _RecordingSession() + client = TangleApiClient("https://api.test", session=session) + + client._make_request("GET", "/api/ping") + + # No Tangle TLS setting: do not pass ``verify`` so requests' own + # session/environment handling (REQUESTS_CA_BUNDLE/CURL_CA_BUNDLE) applies. + assert "verify" not in session.calls[0] + + +def test_static_client_explicit_verify_argument_wins() -> None: + session = _RecordingSession() + client = TangleApiClient("https://api.test", session=session, verify=False) + + client._make_request("GET", "/api/ping") + + assert session.calls[0]["verify"] is False + + +# --------------------------------------------------------------------------- # +# Real local HTTPS end-to-end tests with a generated private CA +# --------------------------------------------------------------------------- # + + +class _GeneratedCa(SimpleNamespace): + ca_pem: Path + server_crt: Path + server_key: Path + + +def _generate_private_ca(directory: Path) -> _GeneratedCa: + """Generate a private CA and a localhost server cert using openssl.""" + + ca_key = directory / "ca.key" + ca_pem = directory / "ca.pem" + server_key = directory / "server.key" + server_csr = directory / "server.csr" + server_crt = directory / "server.crt" + ext = directory / "ext.cnf" + # Python 3.13's TLS stack rejects a leaf missing an Authority Key Identifier + # ("Missing Authority Key Identifier"), so the leaf must carry the full, + # standards-conformant extension set (AKI/SKI, CA:FALSE, key usage, and the + # server-auth EKU) rather than only a SAN. This keeps the CA-bundle success + # cases valid across every supported interpreter (3.10-3.13), not just 3.12. + ext.write_text( + "subjectAltName=DNS:localhost,IP:127.0.0.1\n" + "basicConstraints=critical,CA:FALSE\n" + "keyUsage=critical,digitalSignature,keyEncipherment\n" + "extendedKeyUsage=serverAuth\n" + "subjectKeyIdentifier=hash\n" + "authorityKeyIdentifier=keyid,issuer\n", + encoding="utf-8", + ) + + def _run(args: list[str]) -> None: + subprocess.run( + args, + check=True, + capture_output=True, + ) + + _run([ + "openssl", "req", "-x509", "-newkey", "rsa:2048", "-nodes", + "-keyout", str(ca_key), "-out", str(ca_pem), + "-subj", "/CN=Tangle Test CA", "-days", "2", + "-addext", "basicConstraints=critical,CA:TRUE", + "-addext", "keyUsage=critical,keyCertSign,cRLSign", + # A subject key identifier on the CA lets the leaf's + # ``authorityKeyIdentifier=keyid`` resolve to it. + "-addext", "subjectKeyIdentifier=hash", + ]) + _run([ + "openssl", "req", "-newkey", "rsa:2048", "-nodes", + "-keyout", str(server_key), "-out", str(server_csr), + "-subj", "/CN=localhost", + ]) + _run([ + "openssl", "x509", "-req", "-in", str(server_csr), + "-CA", str(ca_pem), "-CAkey", str(ca_key), "-CAcreateserial", + "-out", str(server_crt), "-days", "2", "-extfile", str(ext), + ]) + return _GeneratedCa(ca_pem=ca_pem, server_crt=server_crt, server_key=server_key) + + +class _SchemaHandler(http.server.BaseHTTPRequestHandler): + def do_GET(self) -> None: # noqa: N802 - required by BaseHTTPRequestHandler + body = json.dumps({"openapi": "3.1.0", "info": {}, "paths": {}}).encode() + self.send_response(200) + self.send_header("Content-Type", "application/json") + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + + def log_message(self, *args: Any) -> None: # silence test server logging + pass + + +@pytest.fixture(scope="module") +def https_server(tmp_path_factory: pytest.TempPathFactory) -> Iterator[dict[str, Any]]: + if shutil.which("openssl") is None: # pragma: no cover - environment guard + pytest.skip("openssl is required for the real HTTPS TLS tests") + + directory = tmp_path_factory.mktemp("tls") + ca = _generate_private_ca(directory) + + context = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) + context.load_cert_chain(certfile=str(ca.server_crt), keyfile=str(ca.server_key)) + + server = http.server.ThreadingHTTPServer(("127.0.0.1", 0), _SchemaHandler) + server.socket = context.wrap_socket(server.socket, server_side=True) + port = server.server_address[1] + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + yield { + "base_url": f"https://localhost:{port}", + "ca_pem": str(ca.ca_pem), + } + finally: + server.shutdown() + thread.join(timeout=5) + server.server_close() + + +def test_httpx_schema_default_verification_fails( + https_server: dict[str, Any], +) -> None: + with pytest.raises(httpx.ConnectError): + fetch_schema(https_server["base_url"]) + + +def test_httpx_schema_ca_bundle_succeeds( + monkeypatch: pytest.MonkeyPatch, https_server: dict[str, Any] +) -> None: + monkeypatch.setenv("TANGLE_API_CA_BUNDLE", https_server["ca_pem"]) + schema = fetch_schema(https_server["base_url"]) + assert schema["paths"] == {} + + +def test_httpx_schema_verify_off_succeeds( + monkeypatch: pytest.MonkeyPatch, https_server: dict[str, Any] +) -> None: + monkeypatch.setenv("TANGLE_API_VERIFY_TLS", "0") + schema = fetch_schema(https_server["base_url"]) + assert schema["paths"] == {} + + +def test_dynamic_client_from_url_uses_ca_bundle( + monkeypatch: pytest.MonkeyPatch, https_server: dict[str, Any] +) -> None: + monkeypatch.setenv("TANGLE_API_CA_BUNDLE", https_server["ca_pem"]) + client = TangleDynamicDiscoveryClient.from_url(https_server["base_url"]) + assert client.operations == () + + +def test_dynamic_client_from_url_default_verification_fails( + https_server: dict[str, Any], +) -> None: + with pytest.raises(httpx.ConnectError): + TangleDynamicDiscoveryClient.from_url(https_server["base_url"]) + + +def test_requests_client_default_verification_fails( + https_server: dict[str, Any], +) -> None: + client = TangleApiClient(https_server["base_url"]) + with pytest.raises(requests.exceptions.SSLError): + client._make_request("GET", "/api/ping") + + +def test_requests_client_ca_bundle_succeeds( + monkeypatch: pytest.MonkeyPatch, https_server: dict[str, Any] +) -> None: + monkeypatch.setenv("TANGLE_API_CA_BUNDLE", https_server["ca_pem"]) + client = TangleApiClient(https_server["base_url"]) + response = client._make_request("GET", "/api/ping") + assert response.status_code == 200 + + +def test_requests_client_verify_off_succeeds( + monkeypatch: pytest.MonkeyPatch, https_server: dict[str, Any] +) -> None: + monkeypatch.setenv("TANGLE_API_VERIFY_TLS", "0") + client = TangleApiClient(https_server["base_url"]) + response = client._make_request("GET", "/api/ping") + assert response.status_code == 200 + + +# --------------------------------------------------------------------------- # +# Global CLI override: configure_cli_verify and resolver precedence +# --------------------------------------------------------------------------- # + + +def test_cli_override_disables_verification() -> None: + configure_cli_verify(verify_tls=False) + assert resolve_verify() is False + + +def test_cli_override_enables_verification() -> None: + configure_cli_verify(verify_tls=True) + assert resolve_verify() is True + + +def test_cli_override_ca_bundle_resolves_to_path(tmp_path: Path) -> None: + bundle = tmp_path / "ca.pem" + bundle.write_text("cert", encoding="utf-8") + configure_cli_verify(ca_bundle=bundle) + assert resolve_verify() == str(bundle) + + +def test_cli_override_wins_over_environment( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv("TANGLE_API_VERIFY_TLS", "1") + configure_cli_verify(verify_tls=False) + assert resolve_verify() is False + + +def test_cli_ca_bundle_wins_over_environment( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + env_bundle = tmp_path / "env.pem" + env_bundle.write_text("cert", encoding="utf-8") + cli_bundle = tmp_path / "cli.pem" + cli_bundle.write_text("cert", encoding="utf-8") + monkeypatch.setenv("TANGLE_API_CA_BUNDLE", str(env_bundle)) + configure_cli_verify(ca_bundle=cli_bundle) + assert resolve_verify() == str(cli_bundle) + + +def test_explicit_python_argument_wins_over_cli_override() -> None: + configure_cli_verify(verify_tls=False) + # A library caller's explicit verify= stays highest precedence. + assert resolve_verify(True) is True + + +def test_cli_override_conflict_ca_bundle_and_no_verify_fails(tmp_path: Path) -> None: + bundle = tmp_path / "ca.pem" + bundle.write_text("cert", encoding="utf-8") + with pytest.raises(SystemExit, match="ca-bundle"): + configure_cli_verify(ca_bundle=bundle, verify_tls=False) + + +def test_cli_override_ca_bundle_and_verify_true_is_accepted(tmp_path: Path) -> None: + bundle = tmp_path / "ca.pem" + bundle.write_text("cert", encoding="utf-8") + configure_cli_verify(ca_bundle=bundle, verify_tls=True) + assert resolve_verify() == str(bundle) + + +def test_cli_override_missing_ca_bundle_fails_early() -> None: + with pytest.raises(SystemExit, match="ca-bundle"): + configure_cli_verify(ca_bundle="/nonexistent/ca.pem") + + +def test_cli_override_clears_back_to_unset(monkeypatch: pytest.MonkeyPatch) -> None: + configure_cli_verify(verify_tls=False) + assert resolve_verify() is False + configure_cli_verify() + assert resolve_verify() is _VERIFY_UNSET + + +# --------------------------------------------------------------------------- # +# argv pre-parse and placement +# --------------------------------------------------------------------------- # + + +@pytest.mark.parametrize( + "argv, expected", + [ + (["tangle", "--no-verify-tls"], False), + (["tangle", "--verify-tls"], True), + ], +) +def test_configure_tls_from_argv_parses_boolean_flags( + argv: list[str], expected: bool +) -> None: + cli._configure_tls_from_argv(argv) + assert resolve_verify() is expected + + +def test_configure_tls_from_argv_parses_ca_bundle(tmp_path: Path) -> None: + bundle = tmp_path / "ca.pem" + bundle.write_text("cert", encoding="utf-8") + cli._configure_tls_from_argv(["tangle", "--ca-bundle", str(bundle), "api", "ping"]) + assert resolve_verify() == str(bundle) + + +def test_configure_tls_from_argv_parses_ca_bundle_equals(tmp_path: Path) -> None: + bundle = tmp_path / "ca.pem" + bundle.write_text("cert", encoding="utf-8") + cli._configure_tls_from_argv(["tangle", f"--ca-bundle={bundle}", "sdk"]) + assert resolve_verify() == str(bundle) + + +def test_configure_tls_from_argv_stops_at_subcommand( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv("TANGLE_API_VERIFY_TLS", "1") + # A --no-verify-tls after the subcommand is not a global flag: it must not + # be consumed here, leaving the env default to apply. + cli._configure_tls_from_argv(["tangle", "api", "ping", "--no-verify-tls"]) + assert resolve_verify() is True + + +@pytest.mark.parametrize( + "argv, expected_tail", + [ + (["tangle", "--no-verify-tls", "api", "ping"], ["ping"]), + (["tangle", "--ca-bundle", "ca.pem", "api", "foo", "bar"], ["foo", "bar"]), + (["tangle", "--ca-bundle=ca.pem", "api", "foo"], ["foo"]), + (["tangle", "--verify-tls", "api"], []), + ], +) +def test_api_argv_tail_skips_global_tls_flags( + argv: list[str], expected_tail: list[str] +) -> None: + assert _api_argv_tail(argv) == expected_tail + + +# --------------------------------------------------------------------------- # +# Global override propagation into every transport (mocked) +# --------------------------------------------------------------------------- # + + +def test_cli_override_propagates_to_static_requests_client() -> None: + configure_cli_verify(verify_tls=False) + session = _RecordingSession() + client = TangleApiClient("https://api.test", session=session) + client._make_request("GET", "/api/ping") + assert session.calls[0]["verify"] is False + + +def test_cli_override_propagates_to_request_operation( + monkeypatch: pytest.MonkeyPatch, +) -> None: + captured: dict[str, Any] = {} + + def fake_request(method: str, url: str, **kwargs: Any) -> httpx.Response: + captured.update(kwargs) + return httpx.Response(200, json={}, request=httpx.Request(method, url)) + + monkeypatch.setattr("tangle_cli.api_transport.httpx.request", fake_request) + configure_cli_verify(verify_tls=False) + request_operation(_operation("/api/ping"), {}, base_url="https://api.test") + assert captured["verify"] is False + + +def test_cli_override_propagates_to_schema_fetch( + monkeypatch: pytest.MonkeyPatch, +) -> None: + captured: dict[str, Any] = {} + + def fake_get(url: str, **kwargs: Any) -> httpx.Response: + captured.update(kwargs) + return httpx.Response( + 200, + json={"openapi": "3.1.0", "paths": {}}, + request=httpx.Request("GET", url), + ) + + monkeypatch.setattr("tangle_cli.api_schema.httpx.get", fake_get) + configure_cli_verify(verify_tls=False) + fetch_schema("https://api.test") + assert captured["verify"] is False + + +# --------------------------------------------------------------------------- # +# Real local HTTPS end-to-end tests driven by the global CLI override +# --------------------------------------------------------------------------- # + + +def test_cli_override_ca_bundle_schema_fetch_succeeds( + https_server: dict[str, Any], +) -> None: + configure_cli_verify(ca_bundle=https_server["ca_pem"]) + schema = fetch_schema(https_server["base_url"]) + assert schema["paths"] == {} + + +def test_cli_override_verify_off_schema_fetch_succeeds( + https_server: dict[str, Any], +) -> None: + configure_cli_verify(verify_tls=False) + schema = fetch_schema(https_server["base_url"]) + assert schema["paths"] == {} + + +def test_cli_override_default_schema_fetch_fails( + https_server: dict[str, Any], +) -> None: + configure_cli_verify() + with pytest.raises(httpx.ConnectError): + fetch_schema(https_server["base_url"]) + + +def test_cli_override_ca_bundle_dynamic_client_succeeds( + https_server: dict[str, Any], +) -> None: + configure_cli_verify(ca_bundle=https_server["ca_pem"]) + client = TangleDynamicDiscoveryClient.from_url(https_server["base_url"]) + assert client.operations == () + + +def test_cli_override_ca_bundle_requests_client_succeeds( + https_server: dict[str, Any], +) -> None: + configure_cli_verify(ca_bundle=https_server["ca_pem"]) + client = TangleApiClient(https_server["base_url"]) + response = client._make_request("GET", "/api/ping") + assert response.status_code == 200 + + +# --------------------------------------------------------------------------- # +# Live CLI subprocess: global flags reach `tangle api refresh` over real HTTPS +# --------------------------------------------------------------------------- # + + +def _run_tangle( + args: list[str], cache_dir: Path, extra_env: dict[str, str] | None = None +) -> subprocess.CompletedProcess[str]: + env = {**os.environ, "TANGLE_CLI_CACHE_DIR": str(cache_dir)} + if extra_env: + env.update(extra_env) + return subprocess.run( + [ + sys.executable, + "-c", + "import sys; from tangle_cli.cli import main; " + "sys.argv = ['tangle', *sys.argv[1:]]; main()", + *args, + ], + capture_output=True, + text=True, + env=env, + ) + + +def test_cli_subprocess_ca_bundle_refresh_succeeds( + https_server: dict[str, Any], tmp_path: Path +) -> None: + result = _run_tangle( + [ + "--ca-bundle", + https_server["ca_pem"], + "api", + "refresh", + "--base-url", + https_server["base_url"], + ], + tmp_path, + ) + assert result.returncode == 0, result.stderr + assert "Cached OpenAPI schema" in result.stdout + + +def test_cli_subprocess_default_refresh_fails( + https_server: dict[str, Any], tmp_path: Path +) -> None: + result = _run_tangle( + ["api", "refresh", "--base-url", https_server["base_url"]], + tmp_path, + ) + assert result.returncode != 0 + assert "Failed to fetch" in result.stderr + + +def test_cli_subprocess_verify_off_refresh_succeeds( + https_server: dict[str, Any], tmp_path: Path +) -> None: + result = _run_tangle( + [ + "--no-verify-tls", + "api", + "refresh", + "--base-url", + https_server["base_url"], + ], + tmp_path, + ) + assert result.returncode == 0, result.stderr + assert "Cached OpenAPI schema" in result.stdout + + +def test_cli_subprocess_ca_bundle_and_no_verify_conflict_fails( + https_server: dict[str, Any], tmp_path: Path +) -> None: + result = _run_tangle( + [ + "--ca-bundle", + https_server["ca_pem"], + "--no-verify-tls", + "api", + "refresh", + "--base-url", + https_server["base_url"], + ], + tmp_path, + ) + assert result.returncode != 0 + assert "ca-bundle" in result.stderr + + +def test_cli_root_help_lists_global_tls_flags(tmp_path: Path) -> None: + result = _run_tangle(["--help"], tmp_path) + assert result.returncode == 0 + assert "--ca-bundle" in result.stdout + assert "--no-verify-tls" in result.stdout