diff --git a/.github/workflows/runtime-image.yml b/.github/workflows/runtime-image.yml index 83ea54682..a812ff459 100644 --- a/.github/workflows/runtime-image.yml +++ b/.github/workflows/runtime-image.yml @@ -93,9 +93,25 @@ jobs: cache-to: type=gha,mode=max,scope=${{ matrix.arch }} - name: Export the digest + # The `sha256:` prefix is stripped because the digest becomes a **filename**, and + # `upload-artifact@v4` rejects a path containing a colon. Left on, the build pushes both + # architectures successfully and then fails on the upload, which skips the `manifest` job + # that creates the tag — so the registry ends up holding the layers with `"tags": null` + # and every `factory contained` command dies on ImagePullBackOff for an image that was + # in fact built. `Assemble the manifest list` puts the prefix back. + # + # Through `env:` rather than interpolated into the script, which is this workflow's rule + # for every value that reaches a shell. + env: + DIGEST: ${{ steps.build.outputs.digest }} run: | + set -eu + case "$DIGEST" in + sha256:[0-9a-f]*) ;; + *) echo "::error::unexpected digest format: $DIGEST"; exit 1 ;; + esac mkdir -p /tmp/digests - touch "/tmp/digests/${{ steps.build.outputs.digest }}" + touch "/tmp/digests/${DIGEST#sha256:}" - uses: actions/upload-artifact@v4 with: diff --git a/CLAUDE.md b/CLAUDE.md index 3ab0120f1..ae4ab4a83 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -320,10 +320,13 @@ Six things are load-bearing and fail quietly if broken: - **Provenance.** A run always starts from the files on this machine, uncommitted changes included — never `HEAD`, never a fresh clone. The workspace is a git worktree with the working tree rsynced over the top, because a HEAD checkout silently drops the gitignored `.factory/` the whole experiment history lives in. Five assertions then run between provisioning and the first agent call (`factory/contained/provenance.py`); a failure aborts naming the file and the likely cause, and leaves the runtime up for inspection. - **Identity.** A bind mount carries ownership through unchanged, so a container whose UID does not own the tree gets a *silently read-only* workspace. The rule differs between rootless, rootful and macOS, so `factory/contained/identity.py` **probes** rather than deciding: a throwaway container reports the mount's owner as the kernel inside sees it, and the run matches. The runtime image is built for arbitrary UIDs (group 0, `chmod g=u`), which is also what OpenShift's restricted SCC needs. - **PID 1.** The factory spawns agent subprocesses and is not a well-behaved init, so the container runs `--init` around `sleep infinity` and the run itself lives in tmux. The runtime persists after the run — a failed run is exactly when its state is worth reading. -- **Credentials cross the boundary, by design.** There is no gateway. The policy is `FACTORY_` by default, plus exactly what `--forward` names, plus the backend variables the resolved shape requires (`factory/contained/credentials.py`) — nothing implicit. `verify` reports credential *shape*, never material, and secret-looking values are redacted anywhere a command is printed. On k8s the credentials come from a namespace Secret the user creates; the factory references it by name and never handles the material. +- **Credentials cross the boundary, by design.** There is no gateway. The policy is `FACTORY_` by default, plus exactly what `--forward` names, plus the backend variables the resolved shape requires (`factory/contained/credentials.py`) — nothing implicit. `verify` reports credential *shape*, never material, and secret-looking values are redacted anywhere a command is printed. On k8s the credentials live in a namespace Secret. `factory/contained/k8s_credentials.py` will *create* one for you as step 3 of the cluster wizard — backend picker, then per value: typed (masked), an environment variable you name, or a file (whose required fields are printed before the question and validated after). Four rules there are load-bearing: the material never enters an **argv** (`--from-literal` is visible in `ps` and in shell history — the manifest goes to `oc apply -f -` on **stdin**), the manifest is **JSON** not YAML (a key containing `:` or a newline is ordinary here), nothing is echoed (shape-only confirmation, redacted command, scrubbed stderr), and nothing is logged but key names and value lengths. With nobody at the keyboard the step is **skipped**, not defaulted — `--yes` means "do not stop to ask me", not "choose a credential for me". - **Both divisions reach outward, and that is the point.** Builds cannot happen inside either boundary, so `--division` is opt-in and separately named. Locally it starts an **unauthenticated** `podman-mcp-server` on `0.0.0.0:8430` — every interface, because the tool has no bind flag and the container reaches the host through a gateway address rather than loopback — detached into its own process group, because the run outlives the launch, and stopped by `factory contained rm`. On the cluster it goes through OpenShift `Build` objects behind a sidecar container that is the only holder of `oc` and the ServiceAccount token; that separation is a boundary only while the Role excludes `pods/exec`, which `verify` asserts via a **SubjectAccessReview API object** — `oc auth can-i --as` collapses `pods/exec` onto `pods` and answers "yes" where RBAC says no. The sidecar runs a **different image** (`FACTORY_CONTAINED_SIDECAR_IMAGE`, an `oc` image) from the agent's; one image for both silently collapses the boundary. - **Interactive prompts stall an unattended run.** A fresh `~/.claude` makes Claude Code ask about folder trust, project MCP servers, and Bypass Permissions mode — all interactive-only, so headless agents never hit them and the interactive CEO does, and the run then sits at a menu nobody is watching. `factory/contained/claude_state.py` pre-records those answers, which the invocation already implies. - **All podman knowledge lives in `factory/podman.py` and all cluster knowledge in `factory/contained/k8s.py`.** Both **compose** commands and do not execute them, which is what makes `FACTORY_CONTAINED_DRY_RUN=1` print the same argv the real path runs rather than a separate rendering that drifts. +- **Never wait on a pod with a flag.** `oc wait --for=... --timeout=Ns` can only report that a condition did not hold, so an unpullable image cost the full timeout and was then reported as a mystery ("the probe produced no output"). `k8s.classify_pod` reads the pod document and `k8s.poll_pod` acts on it: states the kubelet has already given up on (`ImagePullBackOff` — *BackOff* is the word it uses **after** retrying — `CreateContainerConfigError`, `Unschedulable`, …) return at once carrying the kubelet's own message; genuinely retryable ones (`ErrImagePull`) get 30s. `ContainerCreating` is deliberately **not** capped: a first pull legitimately runs for minutes, and capping it trades a hang for a false failure on every cold node. Both the inference probe and `wait_for_container` on the real run path go through it. +- **A step that takes minutes must say what it is waiting for.** `style.activity()` is a transient status line: silent for 5s, then one line rewritten in place (`\r\x1b[2K`) carrying the pod's state and a clock, erased on exit so the caller's result lands in its place. Gated on `style.can_rewrite()`, **not** `enabled()` — colour and motion are different questions, and honouring `FORCE_COLOR` as permission to emit carriage returns fills a CI log with fragments. Off a TTY it degrades to one plain line per changed text; `FACTORY_NO_PROGRESS=1` disables it. +- **Runtime flags parse on either side of a subcommand.** `contained verify --target k8s` and `contained --target k8s verify` are the same command. The flags are declared once in `contained_args._RUNTIME_FLAGS` and fed to both the real parser and a tail parser built with `default=argparse.SUPPRESS`; suppressed defaults are the whole mechanism, since a tail parser that applied its own defaults would report `--target local` for a command line that never said `--target` and silently overwrite the left-hand side. Repeatable flags merge across the subcommand; the same flag on both sides with different values is an **error**, because choosing one silently is how a bundle reaches the wrong namespace. Nothing after `--` is touched. The runtime image (`containers/factory/Containerfile`) is UBI9 + the factory wheel + the agent CLIs + tmux, published multi-arch by CI (`.github/workflows/runtime-image.yml`) — amd64 for cluster nodes, arm64 for a Mac laptop. It publishes on pushes to `main` (`:latest`), on **published releases** (`:`, plus `:latest` unless the release is a prerelease — nightlies are, so they never move `:latest`), and on dispatch. The release trigger is load-bearing: `factory contained setup` pulls and does not build, so a release whose image was never built breaks every new user's first command. Release and dispatch tag names reach the shell through `env:` and are validated against the legal image-tag character set before use. diff --git a/docs/contained/index.md b/docs/contained/index.md index 4d7c2524b..216217126 100644 --- a/docs/contained/index.md +++ b/docs/contained/index.md @@ -109,8 +109,19 @@ factory contained {ls|attach|rm|sync|setup|verify|bundle|help} [name] | `--storage-class SC` | cluster default | Workspace PVC | A flag used against the wrong target fails at parse time naming the target it belongs to — never -silently ignored. Runtime flags go **before** the subcommand; anything flag-shaped after it is an -error rather than a name. +silently ignored. + +Runtime flags go on **either side** of a subcommand. These are the same command: + +```bash +factory contained --target k8s verify +factory contained verify --target k8s +``` + +Both orders are what people type, so both work. Give the same flag on both sides with two different +values and it stops and says so rather than picking one — `--namespace a verify --namespace b` has +no obviously right reading, and choosing wrong applies RBAC to somebody else's namespace. Nothing +after `--` is interpreted at all: that belongs to the factory inside the runtime. --- @@ -411,7 +422,7 @@ credentials**: ```console $ factory contained --target k8s setup -━━ 1/3 Cluster and namespace ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +━━ 1/4 Cluster and namespace ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ Clusters in your kubeconfig: @@ -438,7 +449,7 @@ Create namespace 'factory-contained' now? [y]es [n]o (y/N): y Created factory-contained. `oc new-project` also made it your current project. -━━ 2/3 Review and apply ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +━━ 2/4 Review and apply ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ Comparing 5 object(s) against namespace 'factory-contained' on 'https://api.my-cluster.example.com:443': @@ -477,12 +488,36 @@ Apply this? (2 of 4) [y]es [n]o [a]ll remaining [q]uit (Enter or Esc = skip rolebinding.rbac.authorization.k8s.io/factory-scc created persistentvolumeclaim/factory-workspace created -The credentials Secret is yours to create — the factory never handles the material: - oc create secret generic factory-credentials -n factory-contained \ - --from-literal=ANTHROPIC_API_KEY=... +━━ 3/4 Credentials ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + secret/factory-credentials is missing from factory-contained + The pod reads this Secret as its environment. It stays in the namespace; the + factory sends the material once, here, and never reads it back. + + [1] Anthropic API key + [2] Vertex AI (Google Cloud) + [3] copy what this shell is configured for (anthropic) + [s] skip — print the command and let me do it + +Which inference backend should the pod use? [1/2/3/s]: 1 + + [t] type it now (hidden as you type) + [e] read it from an environment variable in this shell + [f] read it from a file on this machine + [q] cancel + +Where does the Anthropic API key come from? [t/e/f/q]: t +Anthropic API key: *********************************************** + ANTHROPIC_API_KEY: 108 characters, starts 'sk-ant-a', ends '9f2c' (typed) +Use this? [y]es [n]o (Y/n): y + + About to create 'secret/factory-credentials' in 'factory-contained' with: + ANTHROPIC_API_KEY: 108 characters, starts 'sk-ant-a', ends '9f2c' +Create it now? [y]es [n]o (Y/n): y + secret/factory-credentials created ``` -Six details that are deliberate. +Seven details that are deliberate. The **cluster** is asked too, not just the namespace. A kubeconfig usually holds several, and `oc config use-context` is the only way most people know to move between them — so picking the wrong @@ -555,6 +590,34 @@ There is no second, blanket "are you sure?": every object was confirmed a moment prompt on top of that is the friction that teaches people to hit `y` without reading. `--yes` applies everything pending without walking, for automation. +The **credentials Secret is offered as a step**, not left as a closing reminder. It used to be +printed as an `oc create secret` line and nothing more, which meant every freshly prepared namespace +ended one check short of an answer — the inference probe needs that Secret to authenticate, so it +could only be skipped. Pick a backend, pick where each value comes from, and it is created. + +Four rules govern the material, and each one exists because the obvious implementation breaks it: + +- **It never reaches an argv.** `oc create secret --from-literal=KEY=value` puts the value in the + process table for every user on the machine, and in the shell history of anyone who copies the + line. The Secret is composed as a manifest and fed to `oc apply -f -` on **stdin**. +- **The manifest is JSON, not YAML.** A key containing `:`, a newline or a leading `%` is ordinary + here and is a quoting bug waiting to happen in hand-built YAML. +- **Nothing is echoed.** Typing is masked; a value is confirmed by *shape only* + (`108 characters, starts 'sk-ant-a', ends '9f2c'`), which is enough to catch a paste that grabbed + the surrounding quotes and not enough to reuse. The command printed afterwards is redacted, and + anything that turns up in `oc`'s stderr is scrubbed before you see it. +- **Nothing is logged.** Key names and value lengths; never a value. + +A value can be **typed**, read from an **environment variable** you name, or read from a **file** — +and for the Google credential the required fields are printed *before* you are asked, then the file +is parsed and any missing field named. Checking it here is the point: an unusable credential is +accepted into a Secret without complaint and would otherwise surface as an authentication failure +inside an agent call, minutes later, looking like a model outage. + +If a usable Secret is already there, the step says which backend it carries and asks nothing. With +nobody at the keyboard it is **skipped** and the manual command printed — `--yes` means "do not stop +to ask me", not "choose a credential for me", and there is no safe default for that question. + Then `verify` checks every object, every verb the ServiceAccount needs, the Secret's **keys** (never its values), and that inference is reachable from a pod *inside* the namespace. Results print **as each one lands**, not at the end — several are a cluster round trip and the in-cluster inference @@ -582,8 +645,35 @@ The inference check is the slow one: it creates a short-lived pod, with the same a real run uses, and asks it to make one request — because a host-side check proves nothing about the *pod's* egress. It is announced before it starts, and **skipped entirely when the credentials Secret is missing**, since the probe pod mounts that Secret and could only spend its 180-second -timeout rediscovering what the check above already said. That is the state a freshly prepared -namespace is in, because creating the Secret is deliberately left to you. +timeout rediscovering what the check above already said. + +While it waits it **says what it is waiting for**. Anything that takes more than five seconds grows +a status line that rewrites itself in place, carrying the pod's own state and a clock: + +```console +⠹ inference_from_cluster — probe pod: ContainerCreating (0:47) +``` + +Nothing is drawn below five seconds, so a fast check looks exactly as it always did, and the line +erases itself when the result lands. In a pipe or a CI log it degrades to one plain line per change +rather than thousands of redraws; `FACTORY_NO_PROGRESS=1` turns it off entirely. + +It also **stops early when the pod cannot start**. The wait used to be `oc wait --timeout=180s`, +which is blind by construction — it can only report that a condition did not hold, so an image the +cluster could not pull cost the full three minutes and was then described as `the probe produced no +output`, naming neither the cause nor where to look. Now each poll reads the pod, and a state the +kubelet has already given up on (`ImagePullBackOff`, `CreateContainerConfigError`, a pod nothing +will schedule) returns immediately with the kubelet's own words: + +```console +[FAIL] inference_from_cluster: a pod in this namespace could NOT reach inference: the probe pod + could not run: ImagePullBackOff — Back-off pulling image "ghcr.io/…/factory-runtime:latest" +``` + +A first-ever image pull is *not* treated that way — a cold `ContainerCreating` legitimately runs for +minutes, and capping it would trade a hang for a false failure on every new node. Only states that +are already an error get the 30-second ceiling. The same reading now guards the workspace and +factory containers on the real run path, which had the identical problem with a 300-second timeout. Before setup, the same command lists what is missing with the command that restores each — e.g. `factory contained --namespace factory-contained bundle | oc apply -f -`. diff --git a/factory/cli/contained.py b/factory/cli/contained.py index 21fb808e5..cc27b2cc9 100644 --- a/factory/cli/contained.py +++ b/factory/cli/contained.py @@ -17,6 +17,7 @@ from factory.cli.contained_args import ( HELP_EPILOG, HELP_SUBCOMMAND, + add_runtime_flags, interpret, target_given, ) @@ -58,23 +59,10 @@ def build_contained_parser(sub: argparse._SubParsersAction) -> argparse.Argument # One REMAINDER for everything positional, split afterwards by `interpret`. A declarative split # is not expressible: an optional positional carrying `choices` would try to match the first # word of the payload and reject it as an invalid choice. - # Every flag is SUPPRESSed from argparse's own listing and described in the epilog instead: - # a flat list hides which target each flag belongs to, and printing both lists each flag twice. p.add_argument("rest", nargs=argparse.REMAINDER, help=argparse.SUPPRESS) - p.add_argument("--target", choices=["local", "k8s"], default="local", help=argparse.SUPPRESS) - p.add_argument("--division", action="store_true", default=False, help=argparse.SUPPRESS) - p.add_argument("--name", default=None, help=argparse.SUPPRESS) - p.add_argument("--env", action="append", default=[], metavar="KEY=VALUE", dest="extra_env", - help=argparse.SUPPRESS) - p.add_argument("--forward", action="append", default=[], metavar="VAR", help=argparse.SUPPRESS) - p.add_argument("--mount", action="append", default=[], metavar="PATH", help=argparse.SUPPRESS) - p.add_argument("--namespace", default=None, help=argparse.SUPPRESS) - p.add_argument("--storage-class", default=None, dest="storage_class", help=argparse.SUPPRESS) - p.add_argument("--context", default=None, help=argparse.SUPPRESS) - p.add_argument("--image", default=None, help=argparse.SUPPRESS) - # `rm` prompts before deleting an active runtime and the cluster upload prompts on a secret-scan - # finding; `--yes` skips both, for automation. - p.add_argument("--yes", action="store_true", default=False, help=argparse.SUPPRESS) + # From the one table `interpret` also parses the tail against, so a flag can never be accepted + # before the subcommand and rejected after it. + add_runtime_flags(p) _PARSER = p return p diff --git a/factory/cli/contained_args.py b/factory/cli/contained_args.py index 722fbfa07..2c793c0dc 100644 --- a/factory/cli/contained_args.py +++ b/factory/cli/contained_args.py @@ -17,6 +17,7 @@ import os import sys from pathlib import Path +from typing import Any, NoReturn import structlog @@ -26,6 +27,73 @@ LIFECYCLE_SUBCOMMANDS = ("ls", "attach", "rm", "sync", "setup", "verify", "bundle") +# Every runtime flag, declared once. Both the real parser and the tail parser below are built from +# this, because the two must accept exactly the same set: a flag added to one and forgotten in the +# other is a flag that works on one side of the subcommand and errors on the other, which is the +# defect this table exists to make impossible rather than merely unlikely. +_RUNTIME_FLAGS: tuple[tuple[str, dict[str, Any]], ...] = ( + ("--target", {"choices": ["local", "k8s"], "default": "local"}), + ("--division", {"action": "store_true", "default": False}), + ("--name", {"default": None}), + ("--env", {"action": "append", "default": [], "metavar": "KEY=VALUE", "dest": "extra_env"}), + ("--forward", {"action": "append", "default": [], "metavar": "VAR"}), + ("--mount", {"action": "append", "default": [], "metavar": "PATH"}), + ("--namespace", {"default": None}), + ("--storage-class", {"default": None, "dest": "storage_class"}), + ("--context", {"default": None}), + ("--image", {"default": None}), + # `rm` prompts before deleting an active runtime and the cluster upload prompts on a secret-scan + # finding; `--yes` skips both, for automation. + ("--yes", {"action": "store_true", "default": False}), +) + + +def _dest_of(flag: str, options: dict[str, Any]) -> str: + return str(options.get("dest") or flag.lstrip("-").replace("-", "_")) + + +_FLAG_DEFAULTS = {_dest_of(flag, opts): opts["default"] for flag, opts in _RUNTIME_FLAGS} + +# Repeatable flags. Given on both sides of the subcommand they merge rather than conflict, which is +# what "repeatable" already means everywhere else on the command line. +_REPEATABLE_DESTS = frozenset( + _dest_of(flag, opts) for flag, opts in _RUNTIME_FLAGS if opts.get("action") == "append" +) + + +def add_runtime_flags(parser: argparse.ArgumentParser, *, keep_defaults: bool = True) -> None: + """Add every runtime flag to `parser`. + + With `keep_defaults=False` an absent flag is left out of the namespace entirely + (`argparse.SUPPRESS`) rather than filled in with its default. That distinction is the whole + mechanism behind accepting flags on either side of the subcommand: a tail parser whose defaults + were applied would report `--target local` for a command line that never mentioned `--target`, + and silently overwrite the `--target k8s` typed before the subcommand. + + Every flag is hidden from argparse's own listing and described in `HELP_EPILOG` instead: a flat + list hides which target each flag belongs to, and printing both lists each flag twice. + """ + for flag, options in _RUNTIME_FLAGS: + settings = dict(options, help=argparse.SUPPRESS) + if not keep_defaults: + settings["default"] = argparse.SUPPRESS + parser.add_argument(flag, **settings) + + +class _TailError(Exception): + """A bad flag *value* after the subcommand — reported through the real parser, not by exiting.""" + + +class _TailParser(argparse.ArgumentParser): + """A parser for the tail that raises instead of exiting. + + Left to itself argparse would print its own usage — which describes this internal parser rather + than `factory contained` — and call `sys.exit`. Raising lets the real parser own the message. + """ + + def error(self, message: str) -> NoReturn: + raise _TailError(message) + # `help` is not a lifecycle subcommand — it provisions nothing and acts on no runtime — but it is # what people type, and without it the word falls through to the passthrough path and fails with # "no existing directory found in ['help']", a message about materializing workspaces for what is a @@ -62,6 +130,10 @@ bundle Print the cluster prerequisites as YAML (k8s) help Print this text (same as --help) +Runtime flags go on either side of a subcommand — `contained --target k8s verify` +and `contained verify --target k8s` are the same command. After `--` nothing is +interpreted: it belongs to the factory inside the runtime. + Both targets: --target local|k8s Which runtime (default: local) --division Let the agent build container images @@ -145,30 +217,77 @@ def _split_positional(parser: argparse.ArgumentParser, args: argparse.Namespace) def _read_lifecycle_tail( parser: argparse.ArgumentParser, args: argparse.Namespace, tail: list[str] ) -> None: - """What may follow a lifecycle subcommand: a runtime name, and `--yes`. Nothing else.""" - # `--yes` is the one trailing flag accepted here, because `rm --yes` is the order - # people type it. It is documented as the exception; every other flag in this position is - # rejected below rather than silently dropped. - if "--yes" in tail: - args.yes = True - tail = [token for token in tail if token != "--yes"] - # Everything else that looks like a flag here is a mistake worth naming, not swallowing. - # The REMAINDER split means `--target k8s` typed *after* the subcommand never reaches - # `args.target` — it lands here as a plain string instead, so a silent absorption would - # leave `args.target` at its default ("local") while the user believes they asked for k8s, - # and would hand a lifecycle command a name like "--target" to resolve. - flag_like = [token for token in tail if token.startswith("-")] - if flag_like: + """What may follow a lifecycle subcommand: a runtime name and any runtime flag. + + Both orders work — `contained --target k8s verify` and `contained verify --target k8s` — because + both are what people type, and a tool that accepts only one of them is teaching an ordering rule + that serves nobody. argparse cannot do this itself: the REMAINDER that carries the verbatim + payload swallows the tail whole, so a flag typed here never reaches `args` unless it is parsed + back out, which is what happens below. + """ + tail_parser = _TailParser(add_help=False, allow_abbrev=False) + add_runtime_flags(tail_parser, keep_defaults=False) + try: + typed, leftover = tail_parser.parse_known_args(tail) + except _TailError as exc: + # A bad *value* — `--target nope`. Reported through the real parser so the usage line the + # user sees is `factory contained`'s, not this internal parser's. + parser.error(f"after `factory contained {args.subcommand}`: {exc}") + + _merge_tail_flags(parser, args, typed) + + # What is left is either a runtime name or a mistake. A flag reaching here is genuinely + # unrecognized now that every real one has been parsed, so it is named rather than swallowed: + # absorbing it silently would hand a lifecycle command a name like "--targt" to resolve. + unknown = [token for token in leftover if token.startswith("-")] + if unknown: + parser.error( + f"unrecognized flag {unknown[0]!r} after `factory contained {args.subcommand}`. " + f"`factory contained help` lists every flag; they may go on either side of the " + f"subcommand." + ) + names = [token for token in leftover if not token.startswith("-")] + if len(names) > 1: parser.error( - f"unrecognized flag {flag_like[0]!r} after `factory contained " - f"{args.subcommand}`. Runtime flags (--target, --namespace, --name, ...) go before " - f"the subcommand, for example:\n" - f" factory contained --target k8s {args.subcommand}" + f"`factory contained {args.subcommand}` takes one runtime name, but was given " + f"{len(names)}: {', '.join(repr(n) for n in names)}. Try `factory contained ls`." ) # Only the positional overrides `--name` here, and only when one was actually given — # `ls` takes no name. - if tail: - args.name = tail[0] + if names: + args.name = names[0] + + +def _merge_tail_flags( + parser: argparse.ArgumentParser, args: argparse.Namespace, typed: argparse.Namespace +) -> None: + """Fold flags parsed from the tail into `args`, refusing to guess when the two sides disagree. + + Only flags actually typed after the subcommand appear in `typed` — that is what suppressed + defaults buy — so nothing here can overwrite a left-side flag with a default. + + A flag given on *both* sides with different values is an error rather than a silent win for one + of them. `--namespace a verify --namespace b` has no reading that is obviously right, and the + cost of choosing wrong is applying RBAC to somebody else's namespace. + + One gap, named because it is invisible otherwise: a left-side flag set to exactly its own + default cannot be told apart from one that was never typed, so `--target local verify --target + k8s` is accepted as `k8s` rather than reported as a conflict. Both sides agreeing on a value is + also not a conflict, which is the common case when a script and a user both pass `--yes`. + """ + for dest, value in vars(typed).items(): + current = getattr(args, dest, None) + if dest in _REPEATABLE_DESTS: + setattr(args, dest, list(current or []) + list(value)) + continue + default = _FLAG_DEFAULTS[dest] + if current != default and current != value: + flag = f"--{dest.replace('_', '-')}" + parser.error( + f"{flag} was given twice with different values ({current!r} before " + f"`{args.subcommand}`, {value!r} after). Pass it once." + ) + setattr(args, dest, value) def _reject_out_of_scope_flags( diff --git a/factory/cli/contained_k8s.py b/factory/cli/contained_k8s.py index 924d67530..d16b96269 100644 --- a/factory/cli/contained_k8s.py +++ b/factory/cli/contained_k8s.py @@ -26,6 +26,7 @@ import structlog +from factory.contained import style from factory.contained.credentials import resolve_credentials, vertex_model_warning from factory.contained.env import CONTAINED_ENV_POLICY from factory.contained.errors import ContainedError @@ -267,20 +268,34 @@ def _filter(entry: tarfile.TarInfo) -> tarfile.TarInfo | None: def _provision(plan: PodPlan, tarball: Path) -> None: - """Create the claim and the pod, then stream the workspace into the waiting loader.""" + """Create the claim and the pod, then stream the workspace into the waiting loader. + + Both waits report into a status line. Between them they are the longest silence in a cluster + run — the first cold pull of the runtime image happens inside them — and a wait that says + nothing is read as a hang whether or not it is one. + """ apply_manifest(render_pvc(plan.namespace, plan.storage_class), plan.namespace) apply_manifest(render_pod(plan), plan.namespace) # The identifier first, before any long-running work: a run whose name the user cannot see is a # run they cannot manage. print(plan.name) - state = wait_for_container(plan.name, plan.namespace, LOADER_CONTAINER) - if state == "running": - stream_workspace(tarball, plan.name, plan.namespace) - else: - # Already unpacked for *this* run — the pod restarted after a successful upload. The marker - # is per-run, so this can never mean "a previous run's files are already here". - log.debug("contained_workspace_already_present", pod=plan.name) - wait_for_container(plan.name, plan.namespace, FACTORY_CONTAINER) + with style.activity("workspace", "waiting for the loader container") as act: + state = wait_for_container( + plan.name, plan.namespace, LOADER_CONTAINER, + on_progress=lambda p: act.update(f"loader: {p.describe()}"), + ) + if state == "running": + act.update("streaming the workspace into the pod") + stream_workspace(tarball, plan.name, plan.namespace) + else: + # Already unpacked for *this* run — the pod restarted after a successful upload. The + # marker is per-run, so this can never mean "a previous run's files are already here". + log.debug("contained_workspace_already_present", pod=plan.name) + with style.activity("pod", "waiting for the factory container") as act: + wait_for_container( + plan.name, plan.namespace, FACTORY_CONTAINER, + on_progress=lambda p: act.update(f"factory container: {p.describe()}"), + ) def _start(plan: PodPlan, ws: Workspace, project: Path) -> int: diff --git a/factory/contained/k8s.py b/factory/contained/k8s.py index 6c5065e2d..cab5179f5 100644 --- a/factory/contained/k8s.py +++ b/factory/contained/k8s.py @@ -26,6 +26,8 @@ import shutil import subprocess import sys +import time +from collections.abc import Callable from dataclasses import dataclass, field from pathlib import Path from typing import Any @@ -124,6 +126,60 @@ def cli_binary() -> str: ) +# What the API server and the CLIs say when the kubeconfig has a context but no usable credential. +# An expired token is the ordinary end of a working session — `oc login` issues one that lasts a +# day — so this is not an exotic state, and it must never be mistaken for "the object is not there". +_AUTH_ERROR_MARKERS = ( + "you must be logged in", + "asked for the client to provide credentials", + "unauthorized", + "invalid bearer token", + "token has expired", + "no credentials", +) + +# What `NotFound` looks like on stderr. This is the *only* failure that means an object is absent; +# everything else means the question could not be answered. +_NOT_FOUND_MARKERS = ("not found", "notfound") + + +def is_auth_error(text: str) -> bool: + """Whether this failure means "you are logged out", rather than anything about the object.""" + lowered = (text or "").lower() + return any(marker in lowered for marker in _AUTH_ERROR_MARKERS) + + +def is_not_found(text: str) -> bool: + """Whether this failure means the object genuinely is not there. + + Everything else — Unauthorized, Forbidden, a DNS failure, a proxy — is a *question that could + not be answered*, and reporting one of those as "absent" is how a review comes to offer to + create five objects that already exist. + """ + lowered = (text or "").lower() + return any(marker in lowered for marker in _NOT_FOUND_MARKERS) + + +def login_status(binary: str) -> tuple[bool, str]: + """One authenticated round trip: is the selected context actually usable? Never raises. + + `config current-context` cannot answer this — it reads the kubeconfig on disk and succeeds + happily against a context whose token expired an hour ago. That is exactly the state where + every later read fails, and where the failures are subtle enough to be misread as answers. + """ + argv = ( + cli(binary, "whoami") if binary == "oc" + else cli(binary, "auth", "can-i", "get", "pods") + ) + result = _run(argv, timeout=30) + if result is None: + return False, f"`{' '.join(argv)}` could not be run" + if result.returncode == 0: + return True, (result.stdout or "").strip().splitlines()[0] if result.stdout.strip() else "" + detail = (result.stderr or result.stdout or "").strip().splitlines() + return False, detail[0][:200] if detail else f"exit {result.returncode}" + + def current_namespace() -> str | None: """The namespace from the current context. Never hardcoded.""" result = _run(cli(cli_binary(), "config", "view", "--minify", "-o", "jsonpath={..namespace}")) @@ -862,59 +918,258 @@ def apply_manifest(manifest: str, namespace: str) -> None: log.debug("k8s_applied", namespace=namespace, output=result.stdout.strip()[:200]) -def wait_for_container(name: str, namespace: str, container: str, *, timeout: int = 300) -> str: - """Block until `container` is running or has finished. Returns `"running"` or `"terminated"`. +# ------------------------------------------------------------------------------------------------ +# Waiting on a pod, without waiting on one that is never going to start +# ------------------------------------------------------------------------------------------------ - Both are answers, and conflating them hangs: an initContainer that already did its work on an - earlier pod for this run terminates before the host ever looks, and a wait that only accepts - "running" then times out against a container that succeeded. +# Waiting reasons the kubelet reports only once it has already given up, or that no amount of time +# can resolve. Waiting out any of these buys nothing: the answer will not change, and the minutes +# spent are minutes the user spends looking at a blank screen before being told something the very +# first poll knew. `ImagePullBackOff` belongs here and `ErrImagePull` does not — *BackOff* is the +# word the kubelet uses after it has already retried. +DOOMED_WAITING_REASONS = frozenset({ + "ImagePullBackOff", + "ErrImageNeverPull", + "InvalidImageName", + "CreateContainerConfigError", + "CreateContainerError", + "RunContainerError", + "CrashLoopBackOff", +}) + +# Reasons that *might* still resolve — a registry blip, a node under load — but that are an error +# rather than progress. Tolerated for `stuck_after` seconds and then treated as doomed. +RETRYABLE_WAITING_REASONS = frozenset({"ErrImagePull", "ImageInspectError"}) + +# How long an error-ish state is given to clear itself before the wait gives up. Deliberately not +# applied to `ContainerCreating`, which is what a first-ever image pull looks like and legitimately +# runs for minutes; capping that would trade a hang for a false failure on every cold cache. +STUCK_AFTER_SECONDS = 30 + +RUNNING, SUCCEEDED, WAITING, DOOMED = "running", "succeeded", "waiting", "doomed" + + +@dataclass(frozen=True) +class PodProgress: + """What a pod is doing, and whether waiting longer could change it. - Polled rather than `oc wait`ed: the condition here is per-container ("the loader is up"), and - `oc wait --for=condition=Ready` is per-pod and is never satisfied while an initContainer is - still running — which is precisely the window the upload needs. + `verdict` is one of `running`, `succeeded`, `waiting`, `doomed`. The split that matters is the + last one: a pod whose image cannot be pulled and a pod that is still pulling look identical + from a `--timeout` flag, and only one of them is worth waiting for. """ - import time + verdict: str + phase: str = "" + reason: str = "" + message: str = "" + + def describe(self) -> str: + """One human-readable line — the kubelet's own words wherever it supplied any.""" + head = self.reason or self.phase or self.verdict + detail = " ".join(self.message.split())[:200] + return f"{head} — {detail}" if detail else head + + +def classify_pod(pod: dict[str, Any], *, container: str | None = None) -> PodProgress: + """Read one `oc get pod -o json` into a verdict. Pure: no cluster calls, no clock. + + `container` narrows the question to one container of the pod, which is what the workspace + loader needs — an initContainer that is *running* is the window the upload has to hit, and a + pod-level readiness condition is never true during it. + + Anything unrecognized is `waiting`, never `doomed`: this decides whether to stop waiting, and + the cost of being wrong in that direction is a run aborted for a state that would have cleared. + """ + # `or {}` as well as the type check: a key that is present and explicitly null is not the same + # as an absent one, and `.get(k, {})` returns the null. A half-written pod document has to + # classify as "keep waiting", never raise inside a poll loop. + status = (pod.get("status") or {}) if isinstance(pod, dict) else {} + if not isinstance(status, dict): + return PodProgress(WAITING, "", "Pending", "") + phase = str(status.get("phase", "") or "") + + for entry in _container_statuses(status, container): + verdict = _classify_container_state(entry.get("state", {}) or {}, phase) + if verdict is not None: + return verdict + + # No container has a state yet — the answer is at pod level, where an unschedulable pod lives. + return _classify_pod_level(status, phase) + + +def _container_statuses(status: dict[str, Any], container: str | None) -> list[dict[str, Any]]: + """The container statuses this question is about — init and app alike, in that order. + + initContainers come first because they run first: the loader's window is an initContainer that + is still running, and an app container's state says nothing about it. + """ + entries = list(status.get("initContainerStatuses", []) or []) + list( + status.get("containerStatuses", []) or [] + ) + return [ + entry + for entry in entries + if isinstance(entry, dict) and (container is None or entry.get("name") == container) + ] + + +def _classify_container_state(state: dict[str, Any], phase: str) -> PodProgress | None: + """What one container's state means, or `None` when it does not speak yet. + + `None` is not "fine" — it means this entry carries no state at all, so the caller moves on to + the next container and ultimately to the pod-level question. + """ + terminated = state.get("terminated") + if isinstance(terminated, dict): + code = terminated.get("exitCode") + if code == 0: + return PodProgress( + SUCCEEDED, + phase, + str(terminated.get("reason") or "Completed"), + str(terminated.get("message") or ""), + ) + return PodProgress( + DOOMED, + phase, + str(terminated.get("reason") or "Error"), + str(terminated.get("message") or f"exit code {code}"), + ) + if "running" in state: + return PodProgress(RUNNING, phase, "Running", "") + waiting = state.get("waiting") + if isinstance(waiting, dict): + reason = str(waiting.get("reason") or "") + message = str(waiting.get("message") or "") + if reason in DOOMED_WAITING_REASONS: + return PodProgress(DOOMED, phase, reason, message) + return PodProgress(WAITING, phase, reason or "Waiting", message) + return None + + +def _classify_pod_level(status: dict[str, Any], phase: str) -> PodProgress: + """The verdict when no container has spoken — including the pod nothing will ever schedule.""" + unschedulable = _unschedulable(status) + if unschedulable is not None: + return PodProgress(DOOMED, phase, "Unschedulable", unschedulable) + if phase == "Succeeded": + return PodProgress(SUCCEEDED, phase, "Succeeded", "") + if phase == "Failed": + return PodProgress( + DOOMED, phase, str(status.get("reason") or "Failed"), str(status.get("message") or "") + ) + return PodProgress(WAITING, phase, phase or "Pending", "") + + +def _unschedulable(status: dict[str, Any]) -> str | None: + """The scheduler's explanation, when it declined to place the pod at all. + + A pod nothing will ever schedule — no node with the requested resources, a PVC in a zone the + nodes are not in — sits in `Pending` with no container status whatsoever, which is + indistinguishable from "starting" unless the conditions are read. + """ + for condition in status.get("conditions", []) or []: + if not isinstance(condition, dict): + continue + if (condition.get("type") == "PodScheduled" and condition.get("status") == "False" + and condition.get("reason") == "Unschedulable"): + return str(condition.get("message") or "no node can accept this pod") + return None + + +def read_pod(name: str, namespace: str) -> dict[str, Any] | None: + """One pod as a dict, or None when it could not be read. Never raises.""" + result = _run(cli(cli_binary(), "get", "pod", name, "-n", namespace, "-o", "json"), timeout=30) + if result is None or result.returncode != 0: + return None + try: + pod = json.loads(result.stdout or "{}") + except json.JSONDecodeError: + return None + return pod if isinstance(pod, dict) else None + + +def poll_pod( + name: str, + namespace: str, + *, + container: str | None = None, + until: tuple[str, ...] = (RUNNING, SUCCEEDED), + timeout: int = 300, + stuck_after: int = STUCK_AFTER_SECONDS, + interval: float = 2.0, + on_progress: Callable[[PodProgress], None] | None = None, +) -> PodProgress: + """Wait for a pod to reach one of `until`, or to prove it never will. + + Replaces `oc wait --for=... --timeout=Ns`, which is blind by construction: it reports only that + the condition did not hold, so an image that cannot be pulled costs the full timeout and is then + described as a mystery. Here the pod is read each round and `classify_pod` decides — a doomed + state returns immediately carrying the kubelet's own message, and an error state that might + still clear is given `stuck_after` seconds to do so. + + `on_progress` is called with every changed state, which is how a caller drives a status line. + Returns the last `PodProgress`; a timeout is returned as `doomed` with reason `Timeout` rather + than raised, because both callers want to add their own context to it. + """ deadline = time.monotonic() + timeout - last = "" + error_since: float | None = None + last = PodProgress(WAITING, "", "Pending", "") + reported = "" while time.monotonic() < deadline: - result = _run(cli(cli_binary(), "get", "pod", name, "-n", namespace, "-o", "json")) - if result is not None and result.returncode == 0: - try: - pod = json.loads(result.stdout or "{}") - except json.JSONDecodeError: - pod = {} - statuses = pod.get("status", {}).get("initContainerStatuses", []) + pod.get( - "status", {} - ).get("containerStatuses", []) - for status in statuses: - if status.get("name") != container: - continue - state = status.get("state", {}) - if "running" in state: - return "running" - terminated = state.get("terminated") - if isinstance(terminated, dict): - if terminated.get("exitCode") == 0: - return "terminated" - raise ClusterError( - f"container {container} in pod {name} exited " - f"{terminated.get('exitCode')} ({terminated.get('reason')}). " - f"`{cli_binary()} logs {name} -c {container} -n {namespace}` has why." + pod = read_pod(name, namespace) + if pod is not None: + last = classify_pod(pod, container=container) + if on_progress is not None and last.describe() != reported: + reported = last.describe() + on_progress(last) + if last.verdict in until: + return last + if last.verdict == DOOMED: + return last + if last.reason in RETRYABLE_WAITING_REASONS: + error_since = error_since or time.monotonic() + if time.monotonic() - error_since >= stuck_after: + return PodProgress( + DOOMED, last.phase, last.reason, + f"{last.message} (unchanged for {stuck_after}s)".strip(), ) - last = json.dumps(state)[:200] - phase = pod.get("status", {}).get("phase", "") - if phase in ("Failed", "Succeeded") and not last: - raise ClusterError( - f"pod {name} reached {phase} before {container} ran. " - f"`{cli_binary()} describe pod {name} -n {namespace}` has the reason." - ) - time.sleep(2) + else: + error_since = None + time.sleep(interval) + return PodProgress(DOOMED, last.phase, "Timeout", + f"still {last.describe()} after {timeout}s") + + +def wait_for_container( + name: str, namespace: str, container: str, *, timeout: int = 300, + on_progress: Callable[[PodProgress], None] | None = None, +) -> str: + """Block until `container` is running or has finished. Returns `"running"` or `"terminated"`. + + Both are answers, and conflating them hangs: an initContainer that already did its work on an + earlier pod for this run terminates before the host ever looks, and a wait that only accepts + "running" then times out against a container that succeeded. + + Polled rather than `oc wait`ed, for two reasons. The condition is per-container ("the loader is + up") and `oc wait --for=condition=Ready` is per-pod, never satisfied while an initContainer is + still running — precisely the window the upload needs. And polling is what makes it possible to + stop early: this used to spend its full five minutes against an `ImagePullBackOff` before + reporting a timeout whose cause the very first poll already knew. + """ + progress = poll_pod( + name, namespace, container=container, timeout=timeout, on_progress=on_progress, + ) + if progress.verdict == RUNNING: + return "running" + if progress.verdict == SUCCEEDED: + return "terminated" raise ClusterError( - f"timed out after {timeout}s waiting for container {container} in pod {name} to run" - + (f" (last state: {last})" if last else "") - + f". `{cli_binary()} describe pod {name} -n {namespace}` has the reason — an unschedulable " - "pod and an unpullable image both look like this from here." + f"container {container} in pod {name} did not start: {progress.describe()}. " + f"`{cli_binary()} describe pod {name} -n {namespace}` has the full story" + + (f", and `{cli_binary()} logs {name} -c {container} -n {namespace}` has its output" + if progress.phase in ("Running", "Failed", "Succeeded") else "") + + "." ) diff --git a/factory/contained/k8s_credentials.py b/factory/contained/k8s_credentials.py new file mode 100644 index 000000000..0fbd19e22 --- /dev/null +++ b/factory/contained/k8s_credentials.py @@ -0,0 +1,610 @@ +"""The namespace's credentials Secret — checking it, and creating it without mishandling it. + +`setup` used to print an `oc create secret` line and stop, which left every freshly prepared +namespace failing `verify` on the one step that decides whether it can do any work at all. This +module closes that gap while keeping the material's exposure as small as the job allows. + +Four rules hold everywhere below, and each exists because the obvious implementation breaks it: + +- **Never in an argv.** `oc create secret --from-literal=KEY=value` puts the value in the process + table for every user on the machine and into the shell history of anyone who copies the line. The + Secret is composed as a manifest and fed to `oc apply -f -` on **stdin** instead. +- **Never in YAML.** The manifest is JSON. A key containing `:`, a newline or a leading `%` is + ordinary in this domain and is a quoting bug waiting to happen in hand-built YAML; JSON has one + escaping rule and `oc apply` reads it natively. +- **Never echoed.** Typed input is masked, the command printed afterwards is redacted, and any + value that somehow appears in a subprocess's stderr is scrubbed before it is shown. +- **Never logged.** structlog records key names and value *lengths*. A log line is a file, and a + file is the thing this whole module is trying to keep the credential out of. + +The step is skipped entirely when nobody is at the keyboard. `--yes` means "do not stop to ask me", +not "invent a credential", and there is no safe default for this question. +""" + +from __future__ import annotations + +import json +import os +import subprocess +from dataclasses import dataclass, field +from pathlib import Path + +import structlog + +from factory.contained import style +from factory.contained.credentials import ( + ADC_DIR, + ADC_FILE, + VERTEX_PINNED_ENV, + resolve_credentials, +) +from factory.contained.k8s import ADC_SECRET_KEY, LABEL_CONTAINED, SECRET_NAME, cli +from factory.contained.prereq import Check + +log = structlog.get_logger() + +# The keys a credentials Secret must carry for at least one supported backend. +ANTHROPIC_KEYS = ("ANTHROPIC_API_KEY",) +# The three configuration variables *and* the credential file. The credential is the point: the +# first three only say which endpoint to talk to, so a Secret carrying just those was reported as +# "carries the Vertex configuration" while holding nothing that could authenticate. +VERTEX_KEYS = ( + "CLAUDE_CODE_USE_VERTEX", "CLOUD_ML_REGION", "ANTHROPIC_VERTEX_PROJECT_ID", ADC_SECRET_KEY, +) + +# What a Google Application Default Credentials file has to contain, by its own `type`. Checked +# before the file is uploaded because the alternative is finding out from inside a pod, where the +# failure surfaces as an authentication error several minutes into an agent call. +ADC_REQUIRED_FIELDS = { + "authorized_user": ("client_id", "client_secret", "refresh_token"), + "service_account": ("project_id", "private_key", "client_email"), +} + +ADC_TEMPLATE = """\ +{ + "type": "authorized_user", + "client_id": "....apps.googleusercontent.com", + "client_secret": "...", + "refresh_token": "...", + "quota_project_id": "your-project" // optional +} + +A service account key is also accepted; it needs "type": "service_account" plus +"project_id", "private_key" and "client_email". The usual way to produce the first +form is `gcloud auth application-default login`, which writes exactly this file to +~/.config/gcloud/application_default_credentials.json.""" + + +# ------------------------------------------------------------------------------------------------ +# Reading what is there +# ------------------------------------------------------------------------------------------------ + + +def _run(argv: list[str], *, timeout: int = 60) -> subprocess.CompletedProcess[str] | None: + try: + return subprocess.run(argv, capture_output=True, text=True, timeout=timeout) + except (FileNotFoundError, PermissionError, OSError, subprocess.TimeoutExpired): + return None + + +def create_secret_command(binary: str, namespace: str) -> str: + """The manual route, for the fix line and for anyone who would rather not be walked through it. + + Shown with `...` where the material goes. It is a template, and a user who fills it in has + chosen to put a key in their shell history; the guided path exists so that is not the only + option. + """ + return ( + f"factory contained --target k8s --namespace {namespace} setup # walks you through it\n" + f" or by hand:\n" + f" {binary} create secret generic {SECRET_NAME} -n {namespace} \\\n" + f" --from-literal=ANTHROPIC_API_KEY=...\n" + f" or, for Vertex:\n" + f" {binary} create secret generic {SECRET_NAME} -n {namespace} \\\n" + f" --from-literal=CLAUDE_CODE_USE_VERTEX=1 \\\n" + f" --from-literal=CLOUD_ML_REGION= \\\n" + f" --from-literal=ANTHROPIC_VERTEX_PROJECT_ID= \\\n" + f" --from-file={ADC_SECRET_KEY}=$HOME/.config/gcloud/" + f"application_default_credentials.json" + ) + + +def secret_check(binary: str, namespace: str) -> Check: + """The Secret must exist and carry a usable backend's keys — its *keys*, never its values.""" + result = _run(cli(binary, "get", "secret", SECRET_NAME, "-n", namespace, + "-o", "jsonpath={.data}")) + if result is None or result.returncode != 0: + return Check( + name="credentials_secret", + ok=False, + detail=f"secret/{SECRET_NAME} is missing from {namespace}", + fix=create_secret_command(binary, namespace), + ) + keys = _keys_of(result.stdout) + if set(ANTHROPIC_KEYS) <= keys: + return Check(name="credentials_secret", ok=True, + detail=f"secret/{SECRET_NAME} carries the Anthropic API key") + if set(VERTEX_KEYS) <= keys: + return Check(name="credentials_secret", ok=True, + detail=f"secret/{SECRET_NAME} carries the Vertex configuration") + return Check( + name="credentials_secret", + ok=False, + detail=( + f"secret/{SECRET_NAME} exists but carries none of the supported backends' keys " + f"(has: {', '.join(sorted(keys)) or 'nothing'})" + ), + fix=create_secret_command(binary, namespace), + ) + + +def _keys_of(raw: str) -> set[str]: + try: + data = json.loads(raw or "{}") + except json.JSONDecodeError: + return set() + return set(data) if isinstance(data, dict) else set() + + +def secret_exists(binary: str, namespace: str) -> bool: + result = _run(cli(binary, "get", "secret", SECRET_NAME, "-n", namespace, "-o", "name")) + return result is not None and result.returncode == 0 + + +# ------------------------------------------------------------------------------------------------ +# Describing a value without disclosing it +# ------------------------------------------------------------------------------------------------ + +# Below this, an excerpt would be most of the value. Short credentials are described by length only. +_EXCERPT_FLOOR = 16 + + +def describe_value(value: str) -> str: + """A value's shape: enough to recognise a paste that went wrong, not enough to reuse. + + A masked prompt tells you *something* arrived; it cannot tell you *what*. The common mistake + this catches is a copy that grabbed the surrounding quotes, or an environment variable holding + the name of a key rather than a key. + """ + length = len(value) + if length < _EXCERPT_FLOOR: + return f"{length} characters" + return f"{length} characters, starts {value[:8]!r}, ends {value[-4:]!r}" + + +def redact(text: str, values: tuple[str, ...]) -> str: + """Scrub known material out of text that is about to be shown. + + Applied to subprocess stderr. `oc` does not normally echo a Secret's contents back, but "does + not normally" is not a property worth betting a credential on, and a malformed manifest is + exactly the case where a parser quotes the input it choked on. + """ + for value in values: + if value and len(value) >= 4: + text = text.replace(value, "***") + return text + + +# ------------------------------------------------------------------------------------------------ +# Composing and applying +# ------------------------------------------------------------------------------------------------ + + +def build_secret_manifest(namespace: str, data: dict[str, str]) -> str: + """The Secret, as JSON. + + `stringData` rather than `data`, so the API server does the base64 and nothing here has to. + JSON rather than YAML for the escaping reason in the module docstring — every value in here is + attacker-shaped by accident: long, random, and full of characters YAML gives meaning to. + """ + return json.dumps( + { + "apiVersion": "v1", + "kind": "Secret", + "metadata": { + "name": SECRET_NAME, + "namespace": namespace, + "labels": {LABEL_CONTAINED: "true"}, + }, + "type": "Opaque", + "stringData": data, + }, + indent=2, + ) + + +def redacted_command(binary: str, namespace: str, data: dict[str, str]) -> str: + """What was done, in a form that is readable and deliberately not runnable-with-secret.""" + literals = " \\\n ".join( + f"--from-literal={key}={'***' if _is_material(key) else value}" + for key, value in data.items() + ) + return ( + f"{binary} create secret generic {SECRET_NAME} -n {namespace} \\\n {literals}" + ) + + +# Keys whose values are credentials. The rest of a backend's shape — which region, which project, +# which flag — is configuration, and printing it is how a user confirms they configured the right +# thing. +def _is_material(key: str) -> bool: + return key == ADC_SECRET_KEY or "KEY" in key or "TOKEN" in key or "SECRET" in key + + +def apply_secret(binary: str, namespace: str, data: dict[str, str]) -> tuple[bool, str]: + """Create or replace the Secret from stdin. Never raises, never echoes the material.""" + manifest = build_secret_manifest(namespace, data) + values = tuple(data.values()) + try: + result = subprocess.run( + cli(binary, "apply", "-n", namespace, "-f", "-"), + input=manifest, capture_output=True, text=True, timeout=120, + ) + except (FileNotFoundError, PermissionError, OSError, subprocess.TimeoutExpired) as exc: + return False, redact(f"{type(exc).__name__}: {exc}", values) + log.info( + "contained_secret_applied", + namespace=namespace, + ok=result.returncode == 0, + # Names and sizes. The values are the one thing that must not reach a log file. + keys={key: len(value) for key, value in data.items()}, + ) + if result.returncode == 0: + return True, redact((result.stdout or "").strip(), values) + detail = redact((result.stderr or "").strip(), values).splitlines() + return False, detail[0][:200] if detail else "no detail given" + + +# ------------------------------------------------------------------------------------------------ +# Asking +# ------------------------------------------------------------------------------------------------ + + +@dataclass(frozen=True) +class Field: + """One key of the Secret, and where its value may come from.""" + + key: str + question: str + material: bool = False + fixed: str | None = None # not asked at all + default_env: tuple[str, ...] = () # variables to offer as the source + default_value: str = "" + from_file: bool = False + file_default: Path | None = None + json_template: str = "" + validate_json: bool = False + + +ANTHROPIC_FIELDS = ( + Field( + key="ANTHROPIC_API_KEY", + question="Anthropic API key", + material=True, + default_env=("ANTHROPIC_API_KEY",), + ), +) + +VERTEX_FIELDS = ( + Field(key="CLAUDE_CODE_USE_VERTEX", question="", fixed="1"), + Field(key="CLOUD_ML_REGION", question="Vertex region", default_env=("CLOUD_ML_REGION",)), + Field( + key="ANTHROPIC_VERTEX_PROJECT_ID", + question="Google Cloud project ID", + default_env=("ANTHROPIC_VERTEX_PROJECT_ID",), + ), + Field( + key=ADC_SECRET_KEY, + question="Application Default Credentials file", + material=True, + from_file=True, + file_default=ADC_DIR / ADC_FILE, + json_template=ADC_TEMPLATE, + validate_json=True, + ), +) + + +@dataclass +class _Readers: + """The three input functions, in one place so tests can supply their own. + + `tests/conftest.py` forces raw terminal reads off, and a prompt reached under pytest blocks on + a keypress that never comes. Injecting is the only way to exercise this flow at all. + """ + + line: object = field(default=None) + secret: object = field(default=None) + select: object = field(default=None) + + def read_line(self, question: str, default: str | None = None) -> str | None: + reader = self.line or style.read_line + return reader(question, default) # type: ignore[operator] + + def read_secret(self, question: str) -> str | None: + reader = self.secret or style.read_secret + return reader(question) # type: ignore[operator] + + def read_select(self, question: str, options: list[tuple[str, str]]) -> str | None: + reader = self.select or style.select + return reader(question, options) # type: ignore[operator] + + +def _collect_field(field_spec: Field, readers: _Readers) -> str | None: + """One key's value, from whichever source the user picks. `None` means they backed out.""" + if field_spec.fixed is not None: + return field_spec.fixed + if not field_spec.material and not field_spec.from_file: + default = next( + (os.environ[name] for name in field_spec.default_env if os.environ.get(name)), "" + ) or field_spec.default_value + answer = readers.read_line(field_spec.question, default or None) + if answer is None: + return None + return answer.strip() or default + + options = [("t", "type it now (hidden as you type)")] + if field_spec.default_env or not field_spec.from_file: + options.append(("e", "read it from an environment variable in this shell")) + options.append(("f", "read it from a file on this machine")) + options.append(("q", "cancel")) + + while True: + source = readers.read_select(f"Where does the {field_spec.question} come from?", options) + if source is None or source == "q": + return None + value = _read_from_source(source, field_spec, readers) + if value is not None: + return value + # A source that could not supply a value returns here rather than aborting: choosing the + # wrong variable name is a slip, not a decision to stop. + + +def _read_from_source(source: str, field_spec: Field, readers: _Readers) -> str | None: + if source == "t": + if field_spec.json_template: + print(style.note("Paste the file's contents, or press Escape and choose the file " + "instead — which is easier for anything multi-line.")) + typed = readers.read_secret(field_spec.question + ":") + if not typed: + return None + return _confirm_value(field_spec, typed, readers) + if source == "e": + return _from_environment(field_spec, readers) + return _from_file(field_spec, readers) + + +def _from_environment(field_spec: Field, readers: _Readers) -> str | None: + suggestion = next((name for name in field_spec.default_env), None) + name = readers.read_line("Which environment variable?", suggestion) + if name is None: + return None + name = (name.strip() or suggestion or "").strip() + if not name: + return None + value = os.environ.get(name, "") + if not value.strip(): + print(style.line(style.paint( + f"{name} is not set in this shell (or is empty). Nothing was read.", "yellow" + ))) + return None + return _confirm_value(field_spec, value.strip(), readers, source=f"${name}") + + +def _from_file(field_spec: Field, readers: _Readers) -> str | None: + if field_spec.json_template: + # Before the question, not after a rejection: a template shown only once the answer is + # wrong is a template shown to somebody who has already gone and found the wrong file. + print(style.note("This file must contain:")) + for chunk in field_spec.json_template.splitlines(): + print(style.line(style.dim(chunk))) + default = str(field_spec.file_default) if field_spec.file_default else None + typed = readers.read_line("Path to the file", default) + if typed is None: + return None + path = Path((typed.strip() or default or "")).expanduser() + if not str(path): # pragma: no cover - Path("").expanduser() is ".", so this never fires + return None + try: + content = path.read_text() + except OSError as exc: + print(style.line(style.paint(f"Could not read {path}: {exc.strerror or exc}", "yellow"))) + return None + if field_spec.validate_json: + problem = validate_adc(content) + if problem is not None: + print(style.line(style.paint(f"{path} is not usable: {problem}", "yellow"))) + return None + return _confirm_value(field_spec, content, readers, source=str(path)) + + +def validate_adc(content: str) -> str | None: + """`None` when the text is a usable ADC document, else why it is not. + + Checked here rather than in the cluster because the cluster cannot check it: an unusable + credential is accepted into a Secret without complaint and surfaces as an authentication error + inside an agent call, minutes later, indistinguishable from a model outage. + """ + try: + data = json.loads(content) + except json.JSONDecodeError as exc: + return f"it is not valid JSON ({exc.msg} at line {exc.lineno})" + if not isinstance(data, dict): + return "it is JSON, but not an object" + kind = str(data.get("type", "")) + if kind not in ADC_REQUIRED_FIELDS: + return ( + f"its \"type\" is {kind or 'missing'}; expected one of " + f"{', '.join(sorted(ADC_REQUIRED_FIELDS))}" + ) + missing = [name for name in ADC_REQUIRED_FIELDS[kind] if not str(data.get(name, "")).strip()] + if missing: + return f"a {kind} document is missing: {', '.join(missing)}" + return None + + +def _confirm_value( + field_spec: Field, value: str, readers: _Readers, source: str = "typed" +) -> str | None: + """Show the value's shape and have it confirmed. Shape only — never the value.""" + if not field_spec.material: + return value + print(style.line( + f"{style.bold(field_spec.key)}: {describe_value(value)} {style.dim(f'({source})')}" + )) + answer = style.confirm("Use this?", default=True) + return value if answer else None + + +# ------------------------------------------------------------------------------------------------ +# The step +# ------------------------------------------------------------------------------------------------ + + +def run_credentials_step( + binary: str, + namespace: str, + *, + interactive: bool, + assume_yes: bool = False, + readers: _Readers | None = None, +) -> bool: + """Leave the namespace holding usable credentials, or say exactly how to add them. + + Returns whether a usable Secret is now in place. Never raises: every failure here is a thing to + report and carry on from, since `verify` runs immediately afterwards and will say so again. + """ + readers = readers or _Readers() + existing = secret_check(binary, namespace) + if existing.ok: + print(style.line(style.paint(existing.detail, "green"))) + print(style.note("Nothing to do. Delete it and re-run setup if you want to change it.")) + return True + + if not interactive: + # `--yes` is deliberately not enough. It means "do not stop to ask me", and there is no + # answer to "which credential" that can be assumed on a user's behalf. + print(style.line(style.paint(existing.detail, "yellow"))) + print(style.note( + "Nobody is at the keyboard, so this step is being skipped — a credential is never " + "chosen on your behalf. Create it with:" + )) + _print_create_command(binary, namespace) + return False + + print(style.line(style.paint(existing.detail, "yellow"))) + if secret_exists(binary, namespace): + print(style.note( + f"A secret/{SECRET_NAME} is already there but carries no backend this factory " + "understands. Continuing replaces it." + )) + print(style.note( + "The pod reads this Secret as its environment. It stays in the namespace; the factory " + "sends the material once, here, and never reads it back." + )) + + data = _choose_backend(readers) + if data is None: + print(style.line("Skipped. Create it yourself with:")) + _print_create_command(binary, namespace) + return False + + return _confirm_and_create(binary, namespace, data) + + +def _print_create_command(binary: str, namespace: str) -> None: + """The command that does by hand what this step offers to do, dimmed and ready to copy.""" + for chunk in create_secret_command(binary, namespace).splitlines(): + print(style.line(style.dim(chunk))) + + +def _confirm_and_create(binary: str, namespace: str, data: dict[str, str]) -> bool: + """Show the shape of what is about to be sent, then send it. Returns whether it took. + + The confirmation prints *shape* — key names and value lengths — never material, so a user + reading it over someone's shoulder learns nothing they could reuse. + """ + print() + print(style.line(f"About to create {style.value(f'secret/{SECRET_NAME}')} in " + f"{style.value(namespace)} with:")) + for key, value in data.items(): + shown = describe_value(value) if _is_material(key) else style.value(value) + print(style.field(key, shown, pad=34)) + if style.confirm("Create it now?", default=True) is not True: + print(style.line("Nothing was created.")) + return False + + created, detail = apply_secret(binary, namespace, data) + if not created: + print(style.line(style.paint(f"Could not create the Secret: {detail}", "red"))) + print(style.note("This is usually a permissions problem. Whoever owns the namespace can " + "create it with the command above.")) + return False + print(style.line(style.paint(detail or f"secret/{SECRET_NAME} created.", "green"))) + print(style.note("For the record, redacted — the material was sent on stdin, never in an " + "argument:")) + for chunk in redacted_command(binary, namespace, data).splitlines(): + print(style.line(style.dim(chunk))) + return secret_check(binary, namespace).ok + + +def _choose_backend(readers: _Readers) -> dict[str, str] | None: + """Which backend, then its values. `None` means the user chose to do it themselves.""" + local = resolve_credentials() + options = [ + ("1", "Anthropic API key"), + ("2", "Vertex AI (Google Cloud)"), + ] + if local.ok and local.backend in ("anthropic", "vertex"): + options.append(("3", f"copy what this shell is configured for ({local.backend})")) + options.append(("s", "skip — print the command and let me do it")) + + picked = readers.read_select("Which inference backend should the pod use?", options) + if picked is None or picked == "s": + return None + if picked == "3": + return _copy_from_shell(local.backend) + fields = ANTHROPIC_FIELDS if picked == "1" else VERTEX_FIELDS + collected: dict[str, str] = {} + for field_spec in fields: + value = _collect_field(field_spec, readers) + if value is None: + return None + collected[field_spec.key] = value + if fields is VERTEX_FIELDS: + # Not a credential, but the pod has no other route to it and the run behaves differently + # without it — the local target pins the same value for the same reason. + collected.update(VERTEX_PINNED_ENV) + return collected + + +def _copy_from_shell(backend: str) -> dict[str, str] | None: + """Rebuild the shape this shell already resolves, reading the ADC file where one is needed.""" + if backend == "anthropic": + key = os.environ.get("ANTHROPIC_API_KEY", "").strip() + return {"ANTHROPIC_API_KEY": key} if key else None + collected = { + name: os.environ.get(name, "").strip() + for name in ("CLAUDE_CODE_USE_VERTEX", "CLOUD_ML_REGION", "ANTHROPIC_VERTEX_PROJECT_ID") + } + if not all(collected.values()): + print(style.line(style.paint( + "This shell's Vertex configuration is incomplete; answer the questions instead.", + "yellow", + ))) + return None + try: + content = (ADC_DIR / ADC_FILE).read_text() + except OSError as exc: + print(style.line(style.paint( + f"Could not read {ADC_DIR / ADC_FILE}: {exc.strerror or exc}. Run " + "`gcloud auth application-default login` first.", "yellow", + ))) + return None + problem = validate_adc(content) + if problem is not None: + print(style.line(style.paint(f"{ADC_DIR / ADC_FILE} is not usable: {problem}", "yellow"))) + return None + collected[ADC_SECRET_KEY] = content + collected.update(VERTEX_PINNED_ENV) + return collected diff --git a/factory/contained/k8s_review.py b/factory/contained/k8s_review.py index b346659ec..61e1a35f7 100644 --- a/factory/contained/k8s_review.py +++ b/factory/contained/k8s_review.py @@ -29,7 +29,7 @@ from factory.contained import style from factory.contained.bundle import BundleObject -from factory.contained.k8s import cli +from factory.contained.k8s import cli, is_auth_error, is_not_found log = structlog.get_logger() @@ -67,10 +67,21 @@ def _run(argv: list[str], *, stdin: str | None = None, def inspect_objects( - objects: list[BundleObject], namespace: str, binary: str + objects: list[BundleObject], namespace: str, binary: str, + on_object: Callable[[BundleObject], None] | None = None, ) -> list[ObjectState]: - """Compare each object against the cluster. Never raises; an unreadable object is `unknown`.""" - return [_inspect_one(obj, namespace, binary) for obj in objects] + """Compare each object against the cluster. Never raises; an unreadable object is `unknown`. + + `on_object` is called before each comparison, so a caller can say which one is being read. Two + cluster round trips per object means a six-object bundle is comfortably long enough to look + stopped. + """ + states = [] + for obj in objects: + if on_object is not None: + on_object(obj) + states.append(_inspect_one(obj, namespace, binary)) + return states def _inspect_one(obj: BundleObject, namespace: str, binary: str) -> ObjectState: @@ -79,7 +90,26 @@ def _inspect_one(obj: BundleObject, namespace: str, binary: str) -> ObjectState: if present is None: return ObjectState(obj, UNKNOWN, detail=f"could not reach the cluster to check {obj.ref}") if present.returncode != 0: - return ObjectState(obj, ABSENT, detail="not in this namespace — it would be created") + # **Only `NotFound` means absent.** Treating every non-zero exit as "not there" made an + # expired login look like an empty namespace: all five objects were offered for creation + # against a namespace that already had them, and the one honest line on screen — "could not + # confirm whether the namespace exists" — was contradicted by the five under it. + stderr = (present.stderr or "").strip() + if is_not_found(stderr): + return ObjectState(obj, ABSENT, detail="not in this namespace — it would be created") + if is_auth_error(stderr): + return ObjectState( + obj, UNKNOWN, + detail=f"could not be checked — not logged in to this cluster ({binary} login ...)", + ) + first = stderr.splitlines() + return ObjectState( + obj, UNKNOWN, + detail=( + f"could not be checked: {first[0][:140]}" if first + else f"could not be checked (exit {present.returncode})" + ), + ) # `diff` exits 0 for no change and 1 for a change; anything higher is a real error, and so is 1 # with nothing on stdout (some builds report a failure that way). diff --git a/factory/contained/k8s_setup.py b/factory/contained/k8s_setup.py index d2b4765c7..a9d0659b6 100644 --- a/factory/contained/k8s_setup.py +++ b/factory/contained/k8s_setup.py @@ -15,8 +15,9 @@ missing, the object that failed is named and the walk carries on — `verify` then reports exactly what is absent, so a partial apply is never dressed up as success. -The credentials Secret stays outside that flow. `setup` prints the `oc create secret` command and -never handles the material. +The credentials Secret is settled as its own step, in `k8s_credentials`. It used to be left +entirely to the user, which meant every freshly prepared namespace ended one check short of an +answer — the inference probe needs that Secret to authenticate, so it could only be skipped. """ from __future__ import annotations @@ -25,19 +26,21 @@ import json import subprocess import sys -from collections.abc import Callable +from collections.abc import Callable, Iterator import structlog from factory.contained import style from factory.contained.bundle import BundleObject, bundle_objects, render_bundle from factory.contained.k8s import ( - ADC_SECRET_KEY, + DOOMED, LABEL_CONTAINED, SECRET_NAME, SERVICE_ACCOUNT, + SUCCEEDED, ClusterContext, ClusterError, + PodProgress, access_review, build_api_resources_argv, cli, @@ -45,11 +48,21 @@ active_context, cluster_context, current_namespace, + is_auth_error, + is_not_found, list_contexts, + login_status, + poll_pod, resolve_namespace, set_active_context, use_context, ) +from factory.contained.k8s_credentials import ( + ANTHROPIC_KEYS, + VERTEX_KEYS, + run_credentials_step, + secret_check, +) from factory.contained.k8s_review import inspect_objects, render_summary, walk from factory.contained.prereq import Check, format_check, summary_line from factory.contained.secrets import gitleaks_available @@ -57,19 +70,15 @@ log = structlog.get_logger() -# The cluster half of `setup`: choose a namespace, review-and-apply object by object, verify. -# Three rather than four because applying is no longer a step of its own — each object is applied -# at the moment it is accepted, so there is nothing left to batch afterwards. -_K8S_STEPS = 3 - -# The keys a credentials Secret must carry for at least one supported backend. -ANTHROPIC_KEYS = ("ANTHROPIC_API_KEY",) -# The three configuration variables *and* the credential file. The credential is the point: the -# first three only say which endpoint to talk to, so a Secret carrying just those was reported as -# "carries the Vertex configuration" while holding nothing that could authenticate. -VERTEX_KEYS = ( - "CLAUDE_CODE_USE_VERTEX", "CLOUD_ML_REGION", "ANTHROPIC_VERTEX_PROJECT_ID", ADC_SECRET_KEY, -) +# The cluster half of `setup`: choose a namespace, review-and-apply object by object, settle the +# credentials, verify. Applying is not a step of its own — each object is applied at the moment it +# is accepted, so there is nothing left to batch afterwards. +_K8S_STEPS = 4 + +# Re-exported: `ANTHROPIC_KEYS`, `VERTEX_KEYS` and the Secret checks now live in +# `k8s_credentials`, beside the code that creates one. Imported here so the names this module has +# always exposed keep resolving. +__all__ = ["ANTHROPIC_KEYS", "VERTEX_KEYS", "setup_k8s", "verify_k8s"] # The verbs the pod's ServiceAccount needs. Checked as the ServiceAccount, not as the user: a # namespace where *you* can create pods but the pod cannot read its own logs fails on the agent's @@ -140,13 +149,24 @@ def record(*new: Check) -> None: ) return checks - context = _context_check(binary) + with style.activity("cluster_cli", "reading the current context"): + context = _context_check(binary) record(context) if not context.ok: # Everything below needs a reachable cluster. Reporting eight further failures that all mean # "no context" buries the one that matters. return checks + with style.activity("cluster_login", "checking the credential actually works"): + login = _login_check(binary) + record(login) + if not login.ok: + # Same reason, one level deeper. A context with an expired token passes every *local* check + # and fails every remote one, and the remote failures do not say "log in" — they say the + # namespace is unreadable and the objects are missing, which is nine lines of plausible + # fiction. Stopping here is the only way the user is told the one thing that is true. + return checks + try: target = resolve_namespace(namespace) except ClusterError as exc: @@ -155,18 +175,49 @@ def record(*new: Check) -> None: ) return checks - record(_namespace_check(binary, target)) - record(*_object_checks(binary, target, division)) - record(*_verb_checks(target, division)) - secret = _secret_check(binary, target) - record(secret) - record(_image_check()) + for check in _namespace_inspection( + binary, target, division=division, probe_inference=probe_inference, + streaming=on_check is not None, + ): + record(check) + return checks + + +def _namespace_inspection( + binary: str, + target: str, + *, + division: bool, + probe_inference: bool, + streaming: bool, +) -> Iterator[Check]: + """Everything worth asking once a reachable cluster and a usable namespace are established. + + A generator rather than a list so the caller reports each result the moment it is known. These + are the slow checks — an access review per verb is a round trip each, and the inference probe + launches a pod — and a caller that could only print at the end would show a blank screen for + minutes, which reads as a hang. + """ + with style.activity("namespace", f"looking up {target}"): + namespace_check = _namespace_check(binary, target) + yield namespace_check + with style.activity("bundle", f"comparing each object against {target}"): + object_checks = _object_checks(binary, target, division) + yield from object_checks + with style.activity("permissions", "posting an access review per verb the run needs"): + verb_checks = _verb_checks(target, division) + yield from verb_checks + with style.activity("credentials_secret", f"reading secret/{SECRET_NAME}"): + secret = secret_check(binary, target) + yield secret + yield _image_check() if probe_inference: - record(_inference_result(binary, target, secret, announce=on_check is not None)) - record(_gitleaks_check()) + yield _inference_result(binary, target, secret, announce=streaming) + yield _gitleaks_check() if division: - record(*_division_checks(target)) - return checks + with style.activity("build_api", "asking the cluster which APIs it serves"): + division_checks = _division_checks(target) + yield from division_checks def _context_check(binary: str) -> Check: @@ -192,6 +243,36 @@ def _context_check(binary: str) -> Check: ) +def _login_check(binary: str) -> Check: + """Is the selected context's credential still valid? One authenticated round trip. + + Separate from `_context_check` because the two answer different questions and only one of them + used to be asked. A kubeconfig entry is a local file; a *session* is a token with an expiry, and + `oc login` issues one that lasts about a day. Between those two facts sits the state this + exists for: everything local reports fine, every cluster read fails, and none of the failures + say "log in" — `get namespace` becomes "could not confirm", `get serviceaccount` becomes "not in + this namespace". A namespace that was fully prepared reads as an empty one. + """ + ok, detail = login_status(binary) + if ok: + return Check( + name="cluster_login", + ok=True, + detail=f"authenticated{f' as {detail}' if detail else ''}", + ) + return Check( + name="cluster_login", + ok=False, + detail=( + f"the selected context has no working credential: {detail}" + if not is_auth_error(detail) + else "your session for this context has expired — every cluster read would fail, and " + "the failures look like missing objects rather than like a login problem" + ), + fix=f"{binary} login --web # or `{binary} login --token=...`", + ) + + def _inference_result(binary: str, namespace: str, secret: Check, *, announce: bool) -> Check: """The in-cluster probe, or the reason it was not worth running.""" if not secret.ok: @@ -210,12 +291,14 @@ def _inference_result(binary: str, namespace: str, secret: Check, *, announce: b ) if announce: # Announced rather than merely slow: this one creates a pod and waits on it, and - # "nothing on screen for three minutes" is the report people read as a crash. + # "nothing on screen for three minutes" is the report people read as a crash. The status + # line below then keeps saying what the pod is doing; this says what is about to happen. print(style.note( "Checking inference from inside the namespace — this launches a short-lived pod " "and waits for it, up to three minutes." )) - return _inference_check(binary, namespace, resolve_image()) + with style.activity("inference_from_cluster", "creating the probe pod") as act: + return _inference_check(binary, namespace, resolve_image(), act=act) def _namespace_check(binary: str, namespace: str) -> Check: @@ -239,19 +322,32 @@ def _object_checks(binary: str, namespace: str, division: bool) -> list[Check]: objects while `setup` applies five, and the missing one is only found by a run that fails. """ checks = [] + apply_fix = ( + f"factory contained --namespace {namespace}" + f"{' --division' if division else ''} bundle | {binary} apply -f -" + ) for obj in bundle_objects(namespace=namespace, division=division): kind, name = obj.kind, obj.name result = _run(cli(binary, "get", kind, name, "-n", namespace, "-o", "name")) ok = result is not None and result.returncode == 0 + # A failed read is not evidence of absence. Only `NotFound` says the object is missing; + # anything else means the question went unanswered, and reporting that as "is missing" + # sends the user to apply a bundle over objects that are already there. + stderr = (result.stderr or "").strip() if result is not None else "" + answered = ok or is_not_found(stderr) checks.append( Check( name=f"bundle:{kind}/{name}", ok=ok, - detail=f"{kind}/{name} present" if ok else f"{kind}/{name} is missing", + detail=( + f"{kind}/{name} present" if ok + else f"{kind}/{name} is missing" if answered + else f"{kind}/{name} could not be checked: " + f"{stderr.splitlines()[0][:120] if stderr else 'the cluster did not answer'}" + ), fix=( - None if ok else - f"factory contained --namespace {namespace}" - f"{' --division' if division else ''} bundle | {binary} apply -f -" + None if ok else apply_fix if answered else + f"{binary} login --web # the read failed, so this is not known to be missing" ), ) ) @@ -341,57 +437,14 @@ def _no_exec_check(namespace: str) -> Check: ) -def _secret_check(binary: str, namespace: str) -> Check: - """The Secret must exist and carry a usable backend's keys — its *keys*, never its values.""" - result = _run(cli(binary, "get", "secret", SECRET_NAME, "-n", namespace, - "-o", "jsonpath={.data}")) - create_line = ( - f"{binary} create secret generic {SECRET_NAME} -n {namespace} \\\n" - f" --from-literal=ANTHROPIC_API_KEY=...\n" - f" or, for Vertex:\n" - f" {binary} create secret generic {SECRET_NAME} -n {namespace} \\\n" - f" --from-literal=CLAUDE_CODE_USE_VERTEX=1 \\\n" - f" --from-literal=CLOUD_ML_REGION= \\\n" - f" --from-literal=ANTHROPIC_VERTEX_PROJECT_ID= \\\n" - f" --from-file={ADC_SECRET_KEY}=$HOME/.config/gcloud/" - f"application_default_credentials.json" - ) - if result is None or result.returncode != 0: - return Check( - name="credentials_secret", - ok=False, - detail=f"secret/{SECRET_NAME} is missing from {namespace}", - fix=create_line, - ) - keys = _keys_of(result.stdout) - if set(ANTHROPIC_KEYS) <= keys: - return Check(name="credentials_secret", ok=True, - detail=f"secret/{SECRET_NAME} carries the Anthropic API key") - if set(VERTEX_KEYS) <= keys: - return Check(name="credentials_secret", ok=True, - detail=f"secret/{SECRET_NAME} carries the Vertex configuration") - return Check( - name="credentials_secret", - ok=False, - detail=( - f"secret/{SECRET_NAME} exists but carries none of the supported backends' keys " - f"(has: {', '.join(sorted(keys)) or 'nothing'})" - ), - fix=create_line, - ) - - -def _keys_of(raw: str) -> set[str]: - import json - - try: - data = json.loads(raw or "{}") - except json.JSONDecodeError: - return set() - return set(data) if isinstance(data, dict) else set() +# The probe's own ceiling, once it is actually running. A pod that cannot start never reaches it — +# `poll_pod` returns the moment the kubelet says so — so this now bounds only the request itself. +PROBE_TIMEOUT_SECONDS = 180 -def _inference_check(binary: str, namespace: str, image: str) -> Check: +def _inference_check( + binary: str, namespace: str, image: str, act: style.Activity | None = None +) -> Check: """Can a pod in this namespace actually reach inference? (spec.0 check 6) **From inside the cluster, not from here.** A host-side check proves nothing about the pod's @@ -403,15 +456,25 @@ def _inference_check(binary: str, namespace: str, image: str) -> Check: the design makes deliberately: a credentials problem found here fails at launch with a named cause, and found any other way it fails inside an agent call, minutes in, looking like a model outage. + + `act` is the status line. Every wait below reports into it, because this check was reported as a + hang: it is the only one that can legitimately take minutes, and it used to say nothing at all + while doing so. """ + def say(detail: str) -> None: + if act is not None: + act.update(detail) + # A hash rather than a slice of the namespace: a truncated name can end in a hyphen, which # RFC 1123 rejects and which the API server reports as an invalid *value* rather than as a # naming mistake. Hashing also keeps two namespaces' probes from colliding. pod = f"factory-inference-probe-{hashlib.sha1(namespace.encode()).hexdigest()[:8]}" manifest = _probe_pod_manifest(pod, namespace, image) try: + say("removing any probe pod left behind by an earlier run") subprocess.run(cli(binary, "delete", "pod", pod, "-n", namespace, "--ignore-not-found"), capture_output=True, text=True, timeout=60) + say("creating the probe pod") created = subprocess.run(cli(binary, "apply", "-n", namespace, "-f", "-"), input=manifest, capture_output=True, text=True, timeout=60) if created.returncode != 0: @@ -421,23 +484,26 @@ def _inference_check(binary: str, namespace: str, image: str) -> Check: detail=f"the probe pod could not be created: {created.stderr.strip()[:160]}", fix=f"factory contained --namespace {namespace} bundle | {binary} apply -f -", ) - waited = subprocess.run( - cli(binary, "wait", f"pod/{pod}", "-n", namespace, - "--for=jsonpath={.status.phase}=Succeeded", "--timeout=180s"), - capture_output=True, text=True, timeout=240, + # Polled rather than `oc wait --for=...Succeeded --timeout=180s`. That flag is blind: it can + # only report that the condition did not hold, so an image the cluster cannot pull cost the + # full three minutes and was then described as "the probe produced no output" — a sentence + # that names neither the cause nor where to look for it. + progress = poll_pod( + pod, namespace, until=(SUCCEEDED,), timeout=PROBE_TIMEOUT_SECONDS, + on_progress=lambda state: say(f"probe pod: {state.describe()}"), ) + say("reading the probe pod's output") logs = subprocess.run(cli(binary, "logs", pod, "-n", namespace), capture_output=True, text=True, timeout=60) output = (logs.stdout or "").strip() - ok = waited.returncode == 0 and "PROBE_OK" in output + ok = progress.verdict == SUCCEEDED and "PROBE_OK" in output return Check( name="inference_from_cluster", ok=ok, detail=( - "a pod in this namespace reached the configured inference backend" - if ok - else "a pod in this namespace could NOT reach inference: " - + (output.splitlines()[-1][:200] if output else "the probe produced no output") + "a pod in this namespace reached the configured inference backend" if ok + else f"a pod in this namespace could NOT reach inference: " + f"{_probe_failure(progress, output)}" ), fix=( None if ok else @@ -457,6 +523,20 @@ def _inference_check(binary: str, namespace: str, image: str) -> Check: "--wait=false"), capture_output=True, text=True, timeout=60) +def _probe_failure(progress: PodProgress, output: str) -> str: + """Why the probe failed, preferring whichever source actually knows. + + A pod that never started has no logs, and reporting "no output" for it describes the symptom of + the previous sentence rather than the cause. The kubelet's reason is the answer in that case; + the probe's own last line is the answer once it has run. + """ + if progress.verdict == DOOMED and not output: + return f"the probe pod could not run: {progress.describe()}" + if output: + return output.splitlines()[-1][:200] + return f"the probe produced no output ({progress.describe()})" + + def _probe_pod_manifest(name: str, namespace: str, image: str) -> str: """One pod, one request, no workspace, no PVC — it must not depend on anything under test. @@ -600,11 +680,17 @@ def setup_k8s( # Say the outcome before printing 80 lines of YAML that would otherwise bury it — and check the # blocker the user actually has. With no cluster reachable, nothing could be applied whatever # they answer, and "About to apply..." would be untrue. - reachable = _run(cli(binary, "config", "current-context")) - if reachable is None or reachable.returncode != 0 or not reachable.stdout.strip(): + # + # This asks the cluster, not the kubeconfig. It used to run `config current-context`, which + # reads a local file and answers "yes, a context is selected" for a context whose token expired + # hours ago — so the gate passed, the walk ran, and every object in an already-prepared + # namespace was offered for creation because every `get` had failed. + authenticated, why = login_status(binary) + if not authenticated: print( - f"No cluster is selected, so nothing can be applied to namespace {target} from here.\n" - f"Log in first (`{binary} login ...`), then re-run. The manifest you will need is " + f"This context has no working credential, so nothing can be applied to namespace " + f"{target} from here: {why}\n" + f"Log in first (`{binary} login --web`), then re-run. The manifest you will need is " "below; you can also hand it to whoever owns the namespace:\n" f"{apply_line}\n", file=sys.stderr, @@ -617,7 +703,12 @@ def setup_k8s( # decide is, per object that is not already right, what it is for and what would change. print(style.section("Review and apply", step=2, total=_K8S_STEPS)) objects = bundle_objects(namespace=target, division=division) - states = inspect_objects(objects, target, binary) + # One server-side `oc diff` per object, so this grows with the bundle and is a round trip each. + with style.activity("review", f"comparing {len(objects)} objects against {target}") as act: + states = inspect_objects( + objects, target, binary, + on_object=lambda obj: act.update(f"comparing {obj.kind}/{obj.name}"), + ) print(render_summary(states, target, cluster_context().server)) # There is no separate apply step: each object is applied the moment it is accepted. Batching @@ -658,7 +749,7 @@ def setup_k8s( )) return 1 - return _finish(binary, target, division, interactive) + return _finish(binary, target, division, interactive, assume_yes) def _apply_object(obj: BundleObject, namespace: str, binary: str) -> tuple[bool, str]: @@ -680,17 +771,20 @@ def _apply_object(obj: BundleObject, namespace: str, binary: str) -> tuple[bool, return False, detail[0][:200] if detail else "no detail given" -def _finish(binary: str, target: str, division: bool, interactive: bool = False) -> int: - """The Secret reminder and the verify pass — reached whether or not anything was applied. +def _finish( + binary: str, target: str, division: bool, interactive: bool = False, assume_yes: bool = False +) -> int: + """The credentials step and the verify pass — reached whether or not anything was applied. A run where every object was already correct still has to end in `verify`'s two states, because "nothing to apply" is not the same claim as "this namespace is ready". + + Credentials come before verify rather than after it for one reason: without them the inference + probe cannot run, so a setup that ends without asking always ends one check short of an answer. """ - print( - f"\nThe credentials Secret is yours to create — the factory never handles the material:\n" - f" {binary} create secret generic {SECRET_NAME} -n {target} " - "--from-literal=ANTHROPIC_API_KEY=...\n" - ) + print(style.section("Credentials", step=3, total=_K8S_STEPS)) + run_credentials_step(binary, target, interactive=interactive, assume_yes=assume_yes) + print(style.section("Verify", step=_K8S_STEPS, total=_K8S_STEPS)) # Streamed, not collected: the access reviews and the in-cluster inference probe take minutes # between them, and a step that prints nothing until they all finish is read as a hang — which diff --git a/factory/contained/style.py b/factory/contained/style.py index 9f7c667dc..7e402f3d0 100644 --- a/factory/contained/style.py +++ b/factory/contained/style.py @@ -11,6 +11,13 @@ Precedence follows the conventions people already have configured: `NO_COLOR` (any value, https://no-color.org) beats `FORCE_COLOR`, which beats TTY detection. + +Two things here are about *time* rather than appearance, and they answer complaints of the same +shape — "I could not tell whether it was working". `activity()` gives a slow step a line that says +what it is waiting for, rewritten in place; `read_secret()` gives a masked prompt the same +keystroke feedback an ordinary one has. Both degrade where the terminal cannot support them, and +`can_rewrite()` — not `enabled()` — is what decides that, because colour and motion are different +questions. """ from __future__ import annotations @@ -19,6 +26,9 @@ import shutil import sys import textwrap +import threading +import time +from types import TracebackType from typing import Any, TextIO _RESET = "\033[0m" @@ -157,6 +167,157 @@ def line(text: str) -> str: return f" {text}" +# ------------------------------------------------------------------------------------------------ +# Saying what a slow step is waiting for +# ------------------------------------------------------------------------------------------------ + +# Nothing is drawn before this. Every fast operation therefore produces byte-for-byte the output it +# produced before this existed, and a spinner that flashes for a third of a second — which reads as +# a glitch rather than as progress — is impossible by construction. +ACTIVITY_THRESHOLD_SECONDS = 5.0 + +# Fast enough that the elapsed counter looks live, slow enough to be free. +_ACTIVITY_FRAME_SECONDS = 0.1 + +_SPINNER_FRAMES = "⠋⠙⠹⠸⠼⠴⠦⠧⠇⠏" + +# Return to column zero and clear to end of line. `\r` alone leaves the tail of a longer previous +# frame on screen, which is how a status line comes to read `waiting for the podd pod`. +_ERASE_LINE = "\r\033[2K" + + +def can_rewrite(stream: TextIO | None = None) -> bool: + """Whether the last line on `stream` can be erased and redrawn in place. + + Deliberately *not* `enabled()`. Colour and motion are different questions with different + answers: `FORCE_COLOR` in a CI job asks for colour in a log file, and honouring it as permission + to emit carriage returns fills that log with thousands of half-lines. `NO_COLOR` likewise says + nothing about redrawing. Only a real terminal gets rewritten; `FACTORY_NO_PROGRESS=1` opts out + of even that. + """ + target = stream if stream is not None else sys.stdout + if os.environ.get("FACTORY_NO_PROGRESS"): + return False + if os.environ.get("TERM", "").strip().lower() == "dumb": + return False + try: + return bool(target.isatty()) + except (AttributeError, ValueError): + return False + + +def _elapsed(seconds: float) -> str: + return f"{int(seconds) // 60}:{int(seconds) % 60:02d}" + + +class Activity: + """A status line for one slow operation: what is running, and what it is waiting for. + + Use it through `activity()`. While one is live, nothing else may write to the same stream — + another writer's output lands in the middle of a frame and is then erased by the next one. + + Off a terminal it degrades to plain lines rather than to silence, because the case that + prompted this — a check that takes three minutes — is just as unreadable in a CI log as it is on + a laptop. There it prints one line per *changed* description, so a log gains a progress trail + instead of a thousand redraws. + """ + + def __init__(self, label: str, detail: str = "", *, stream: TextIO | None = None, + threshold: float = ACTIVITY_THRESHOLD_SECONDS) -> None: + self._label = label + self._detail = detail + self._stream = stream if stream is not None else sys.stdout + self._threshold = threshold + self._rewrites = can_rewrite(self._stream) + self._lock = threading.Lock() + self._done = threading.Event() + self._thread: threading.Thread | None = None + self._started = 0.0 + self._frame = 0 + self._drawn = False + self._announced: str | None = None + + def update(self, detail: str) -> None: + """Say what is being waited for now. Safe to call as often as a poll loop likes.""" + with self._lock: + if detail == self._detail: + return + self._detail = detail + if self._rewrites: + return + # The plain path has no thread to notice the threshold passing, so the decision is made + # here: quiet until the operation has proved slow, then one line per change. + if time.monotonic() - self._started >= self._threshold and detail != self._announced: + self._announced = detail + self._write(f" ... {self._compose_text()}\n") + + def _compose_text(self) -> str: + return f"{self._label} — {self._detail}" if self._detail else self._label + + def _write(self, text: str) -> None: + try: + self._stream.write(text) + self._stream.flush() + except (OSError, ValueError): + # A closed stream must not take down the operation being reported on. + self._rewrites = False + + def __enter__(self) -> Activity: + self._started = time.monotonic() + if self._rewrites: + self._thread = threading.Thread(target=self._spin, daemon=True) + self._thread.start() + return self + + def __exit__(self, exc_type: type[BaseException] | None, exc: BaseException | None, + tb: TracebackType | None) -> None: + self._done.set() + if self._thread is not None: + self._thread.join(timeout=1.0) + with self._lock: + if self._drawn: + # The caller's own result line goes where the spinner was. Erasing here rather than + # leaving the frame up is what keeps a completed check looking like it always did. + self._write(_ERASE_LINE) + self._drawn = False + + def _spin(self) -> None: + while not self._done.wait(_ACTIVITY_FRAME_SECONDS): + if time.monotonic() - self._started < self._threshold: + continue + with self._lock: + if self._done.is_set(): + return + self._draw() + + def _draw(self) -> None: + spinner = _SPINNER_FRAMES[self._frame % len(_SPINNER_FRAMES)] + self._frame += 1 + clock = _elapsed(time.monotonic() - self._started) + text = f"{spinner} {self._compose_text()} ({clock})" + # Truncated to the terminal, because a frame that wraps occupies two lines and `\r` only + # ever returns to the start of the last one — leaving the first half on screen forever. + width = max(shutil.get_terminal_size(fallback=(80, 24)).columns - 1, 20) + if len(text) > width: + text = text[: width - 1] + "…" + self._write(_ERASE_LINE + paint(text, "cyan", stream=self._stream)) + self._drawn = True + + +def activity(label: str, detail: str = "", *, stream: TextIO | None = None, + threshold: float = ACTIVITY_THRESHOLD_SECONDS) -> Activity: + """A status line for a slow operation, silent unless it turns out to be slow. + + with style.activity("inference_from_cluster", "creating the probe pod") as act: + ... + act.update("pod is Pending — ContainerCreating") + + It draws nothing for the first `threshold` seconds, redraws one line in place after that, and + erases itself on the way out so the caller's result line lands in its place. + """ + return Activity(label, detail, stream=stream, threshold=threshold) + + ESCAPE = "\x1b" """What `read_key` returns for a bare Escape, and what a text prompt looks for in a typed line.""" @@ -191,7 +352,7 @@ def _raw_session(target: TextIO) -> tuple[int, Any] | None: return None try: import termios - except ImportError: # non-POSIX + except ImportError: # pragma: no cover - non-POSIX only; termios always imports on the test platforms return None try: descriptor = sys.stdin.fileno() @@ -296,13 +457,63 @@ def read_line( target.flush() -def _edit_line(descriptor: int, target: TextIO) -> str | None: +def read_secret( + question: str, *, mask: str = "*", stream: TextIO | None = None +) -> str | None: + """Read a value that must not appear on screen. `None` means cancelled. + + Echoes one `mask` character per keystroke rather than nothing at all: a prompt that shows no + response to typing is indistinguishable from one that is not receiving the keys, and the value + being entered here is routinely a hundred-character paste. The length is the only thing the + mask discloses. + + Falls back to `getpass`, which suppresses echo entirely, where raw reading is impossible. The + returned value is stripped — a trailing newline or space from a copy-paste is never part of a + key, and one that survives fails authentication in a way nothing reports usefully. + + The caller owns the value from here. It must not be logged, echoed, or put in an argv. + """ + target = stream if stream is not None else sys.stdout + rendered = f"{bold(question, stream=target)} " + session = _raw_session(target) + if session is None: + import getpass + + try: + typed = getpass.getpass(rendered) + except (EOFError, OSError): + print() + return None + return None if is_escape(typed) else typed.strip() + + descriptor, original = session + + import termios + import tty + + target.write(rendered) + target.flush() + try: + tty.setcbreak(descriptor) + return _edit_line(descriptor, target, mask=mask) + except (OSError, ValueError): + return None + finally: + termios.tcsetattr(descriptor, termios.TCSADRAIN, original) + target.write("\n") + target.flush() + + +def _edit_line(descriptor: int, target: TextIO, *, mask: str | None = None) -> str | None: """The line editor itself, on a terminal already in cbreak mode. `None` means cancelled. Small on purpose, and it echoes as it goes: cbreak turns off the line discipline that normally provides echo and Backspace, so anything it does not handle here is a key that appears to do nothing. The caller owns putting the terminal into cbreak and restoring it — this function only reads, and must not be called on a terminal that is still line-buffered. + + `mask` replaces each character on screen, for a value that must not be readable over a + shoulder. The buffer is unaffected; only the echo changes. """ typed_chars: list[str] = [] while True: @@ -326,7 +537,7 @@ def _edit_line(descriptor: int, target: TextIO) -> str | None: continue if char.isprintable(): typed_chars.append(char) - target.write(char) + target.write(mask if mask is not None else char) target.flush() @@ -377,6 +588,45 @@ def confirm(question: str, *, default: bool = False, stream: TextIO | None = Non return False +def select( + question: str, options: list[tuple[str, str]], *, stream: TextIO | None = None +) -> str | None: + """A one-keypress choice between named options. `None` means the user backed out. + + Each option is a `(key, label)` pair, printed one per line as `[k] label` — the same shape as + `confirm`'s legend, for the same reason: a menu whose keys are only listed as `[a/b/c]` makes + the reader hold the mapping in their head while deciding. + + Rejects an unknown key by asking again rather than by choosing something, because every caller + of this is about to do something to a cluster. + """ + target = stream if stream is not None else sys.stdout + keys = {key.lower() for key, _ in options} + print(file=target) + for key, label in options: + print(line(choice(key, f" {label}", stream=target)), file=target) + print(file=target) + legend = f"{bold(question, stream=target)} [{'/'.join(key for key, _ in options)}]: " + while True: + pressed = read_key(legend, stream=target) + if pressed is None: + try: + raw = input(legend) + except (EOFError, OSError): + print() + return None + if is_escape(raw): + return None + answer = raw.strip().lower() + if answer in keys: + return answer + continue + if pressed == ESCAPE: + return None + if pressed.lower() in keys: + return pressed.lower() + + def prompt(question: str, default: str | None = None, stream: TextIO | None = None) -> str: """A question, with its default rendered so it is obvious what Enter does.""" if default is None: diff --git a/tests/test_contained.py b/tests/test_contained.py index 542f7bf9b..89ee59d8d 100644 --- a/tests/test_contained.py +++ b/tests/test_contained.py @@ -85,9 +85,58 @@ def test_trailing_yes_reaches_the_namespace() -> None: assert args.yes is True -def test_flag_after_lifecycle_subcommand_is_an_error_not_a_name() -> None: +def test_runtime_flags_work_on_either_side_of_the_subcommand() -> None: + """`contained verify --target k8s` is what people type; it used to be rejected. + + The two orders have to produce the same namespace, not merely both be accepted — a `--target` + that parses but does not reach `args.target` sends a cluster command to the local runtime. + """ + before = interpret(["--target", "k8s", "--namespace", "ns", "verify"]) + after = interpret(["verify", "--target", "k8s", "--namespace", "ns"]) + assert (before.subcommand, before.target, before.namespace) == ("verify", "k8s", "ns") + assert (after.subcommand, after.target, after.namespace) == ("verify", "k8s", "ns") + + +def test_a_name_and_flags_can_be_mixed_after_the_subcommand() -> None: + args = interpret(["rm", "rta-abc123", "--target", "k8s", "--namespace", "ns"]) + assert (args.subcommand, args.name, args.target, args.namespace) == ( + "rm", "rta-abc123", "k8s", "ns" + ) + + +def test_a_repeatable_flag_merges_across_the_subcommand() -> None: + """Repeatable means repeatable; splitting one across the subcommand is not a conflict.""" + args = interpret(["--env", "A=1", "ls", "--env", "B=2"]) + assert args.extra_env == ["A=1", "B=2"] + + +def test_the_same_flag_on_both_sides_with_different_values_is_refused() -> None: + """Silently letting one side win is how a bundle reaches the wrong namespace.""" + with pytest.raises(SystemExit): + interpret(["--namespace", "a", "verify", "--namespace", "b"]) + + +def test_an_unknown_flag_after_the_subcommand_is_still_named() -> None: + with pytest.raises(SystemExit): + interpret(["ls", "--targt", "k8s"]) + + +def test_a_bad_flag_value_after_the_subcommand_is_reported_not_exited_silently() -> None: with pytest.raises(SystemExit): - interpret(["ls", "--target", "k8s"]) + interpret(["verify", "--target", "nope"]) + + +def test_two_names_after_a_lifecycle_subcommand_are_refused() -> None: + """The second used to be dropped without a word, leaving the wrong runtime targeted.""" + with pytest.raises(SystemExit): + interpret(["attach", "one", "two"]) + + +def test_the_payload_after_the_separator_is_never_parsed_as_a_runtime_flag() -> None: + """The whole passthrough contract: `--target` in the payload belongs to the inner factory.""" + args = interpret(["--", "ceo", "/tmp", "--target", "k8s"]) + assert args.target == "local" + assert args.factory_args == ["ceo", "/tmp", "--target", "k8s"] def test_local_only_flag_against_k8s_fails_at_parse_time() -> None: diff --git a/tests/test_contained_bundle_coverage.py b/tests/test_contained_bundle_coverage.py new file mode 100644 index 000000000..44b55cd9f --- /dev/null +++ b/tests/test_contained_bundle_coverage.py @@ -0,0 +1,18 @@ +"""The one branch in `bundle` that only fires when the cluster is unreachable. + +`render_bundle` has to produce YAML with no cluster in reach — that is the whole point of `bundle`, +which a user hands to someone who owns a namespace they cannot touch. So the current-namespace +lookup swallows everything and falls back to None, and that swallow is the branch under test. +""" + +from __future__ import annotations + +from unittest.mock import patch + +from factory.contained import bundle + + +def test_current_namespace_lookup_swallows_every_failure() -> None: + """A broken or absent CLI must degrade to "no namespace known", never raise into rendering.""" + with patch("factory.contained.k8s.current_namespace", side_effect=RuntimeError("no cli")): + assert bundle._safe_current_namespace() is None diff --git a/tests/test_contained_cli_coverage.py b/tests/test_contained_cli_coverage.py new file mode 100644 index 000000000..ec13cddd9 --- /dev/null +++ b/tests/test_contained_cli_coverage.py @@ -0,0 +1,806 @@ +"""Coverage-completing tests for the `factory contained` CLI front door and the local runtime. + +The three modules under test are pure orchestration: `contained.py` reads one interpreted namespace +and hands it to a peer, `contained_args.py` reads the command line, and `contained_local.py` composes +and runs one podman container. None of them should ever launch podman during a test, so every seam — +podman helpers, workspace materialization, credential/identity probes, the division server — is +mocked. What is asserted is the routing and the composition, not the side effects. + +The existing `tests/test_contained.py` and `tests/test_contained_lifecycle.py` already cover the happy +paths reachable through dry-run; this file fills in the branches those cannot reach without either +launching podman or standing up a cluster. +""" + +from __future__ import annotations + +import argparse +import subprocess +from pathlib import Path +from types import SimpleNamespace +from unittest.mock import patch + +import pytest + +from factory.cli import contained as cli +from factory.cli import contained_args +from factory.cli import contained_local +from factory.contained.credentials import CredentialShape +from factory.contained.errors import ContainedError +from factory.contained.provenance import Probe +from factory.contained.workspace import Workspace, WorkspaceError +from factory.podman import CONTAINER_HOME, LABEL_CONTAINED, LABEL_PROJECT, ContainerPlan, Mount, Step + + +# -------------------------------------------------------------------------------------------- +# Shared helpers — a parser built the way the real CLI builds it, plus a minimal plan. +# -------------------------------------------------------------------------------------------- + + +def parse(argv: list[str]) -> argparse.Namespace: + parser = argparse.ArgumentParser(prog="factory") + sub = parser.add_subparsers(dest="command") + cli.build_contained_parser(sub) + return parser.parse_args(["contained", *argv]) + + +def interpret(argv: list[str]) -> argparse.Namespace: + args = parse(argv) + cli.interpret(cli._PARSER, args) + return args + + +def _completed( + returncode: int = 0, stdout: str = "", stderr: str = "" +) -> subprocess.CompletedProcess[str]: + return subprocess.CompletedProcess([], returncode, stdout, stderr) + + +def _plan(tmp_path: Path) -> ContainerPlan: + workspace = tmp_path / "rta" + workspace.mkdir(exist_ok=True) + return ContainerPlan( + name="rta-abc123", + image="example/runtime:latest", + workdir=str(workspace), + env={"FACTORY_CONTAINED": "1", "HOME": CONTAINER_HOME}, + labels={LABEL_CONTAINED: "true", LABEL_PROJECT: "deadbeef"}, + mounts=(Mount(workspace, str(workspace)),), + run_command=f"cd {workspace} && factory study {workspace}", + user="501:0", + ) + + +class _FakeDivision: + """Stand-in for `division.Division` — records whether the run kept or stopped the endpoint.""" + + def __init__(self, plan: ContainerPlan) -> None: + self.plan = plan + self.kept = False + self.stopped = False + + def keep(self) -> None: + self.kept = True + + def stop(self) -> None: + self.stopped = True + + +# ============================================================================================ +# contained.py — the front door: dispatch routing, the Ctrl-C handler, and `_verify`. +# ============================================================================================ + + +def test_keyboard_interrupt_is_caught_and_reported_as_130( + capsys: pytest.CaptureFixture[str], +) -> None: + """Backing out of a wizard is ordinary, so an interrupt is a message and 130, not a traceback.""" + with patch.object(cli, "_dispatch", side_effect=KeyboardInterrupt): + code = cli.cmd_contained(argparse.Namespace()) + assert code == 130 + assert "Stopped." in capsys.readouterr().err + + +def test_context_is_pinned_once_when_given() -> None: + """`--context` is applied globally so no downstream cluster command has to remember it.""" + args = interpret(["--target", "k8s", "--context", "ctx", "verify"]) + with patch("factory.contained.k8s.set_active_context") as pin, \ + patch("factory.contained.k8s_setup.verify_k8s", return_value=[SimpleNamespace(ok=True)]), \ + patch("factory.contained.prereq.format_check", return_value=""), \ + patch("factory.contained.prereq.summary_line", return_value="ok"): + assert cli.cmd_contained(args) == 0 + pin.assert_called_once_with("ctx") + + +def test_verify_k8s_streams_and_returns_zero_when_all_pass( + capsys: pytest.CaptureFixture[str], +) -> None: + args = interpret(["--target", "k8s", "--namespace", "ns", "verify"]) + with patch( + "factory.contained.k8s_setup.verify_k8s", return_value=[SimpleNamespace(ok=True)] + ) as vk, \ + patch("factory.contained.prereq.format_check", return_value="line"), \ + patch("factory.contained.prereq.summary_line", return_value="summary"): + assert cli.cmd_contained(args) == 0 + assert vk.call_args.kwargs["namespace"] == "ns" + assert "summary" in capsys.readouterr().out + + +def test_verify_k8s_returns_one_when_a_check_fails() -> None: + args = interpret(["--target", "k8s", "verify"]) + with patch("factory.contained.k8s_setup.verify_k8s", return_value=[SimpleNamespace(ok=False)]), \ + patch("factory.contained.prereq.format_check", return_value=""), \ + patch("factory.contained.prereq.summary_line", return_value=""): + assert cli.cmd_contained(args) == 1 + + +def test_verify_local_returns_zero_when_all_pass(capsys: pytest.CaptureFixture[str]) -> None: + # `local_checks`/`render_checks` are imported into `contained.py` at module load, so they must + # be patched there rather than at their source module. + args = interpret(["verify"]) + with patch("factory.cli.contained.local_checks", return_value=[SimpleNamespace(ok=True)]), \ + patch("factory.cli.contained.render_checks", return_value="rendered"): + assert cli.cmd_contained(args) == 0 + assert "rendered" in capsys.readouterr().out + + +def test_verify_local_returns_one_when_a_check_fails() -> None: + args = interpret(["verify"]) + with patch("factory.cli.contained.local_checks", return_value=[SimpleNamespace(ok=False)]), \ + patch("factory.cli.contained.render_checks", return_value=""): + assert cli.cmd_contained(args) == 1 + + +def test_setup_is_dispatched_with_the_interactive_and_target_context() -> None: + args = interpret(["setup"]) + with patch("factory.cli.contained.run_setup", return_value=0) as run_setup, \ + patch("factory.cli.contained.sys.stdin.isatty", return_value=False): + assert cli.cmd_contained(args) == 0 + # `--target` was never typed, so setup is asked to decide the target itself (None). + assert run_setup.call_args.args[0] is None + assert run_setup.call_args.kwargs["interactive"] is False + + +def test_bundle_is_dispatched_and_implies_the_cluster_target( + capsys: pytest.CaptureFixture[str], +) -> None: + args = interpret(["bundle"]) + assert args.target == "k8s" # `interpret` promotes bundle to the cluster target + with patch("factory.contained.bundle.render_bundle", return_value="MANIFEST") as render, \ + patch("factory.podman.resolve_image", return_value="img:latest"): + assert cli.cmd_contained(args) == 0 + assert render.call_args.kwargs["image"] == "img:latest" + assert "MANIFEST" in capsys.readouterr().out + + +def test_bundle_uses_an_explicit_image_when_given() -> None: + # Runtime flags go before the subcommand on this parser. + args = interpret(["--image", "custom:tag", "bundle"]) + with patch("factory.contained.bundle.render_bundle", return_value="M") as render, \ + patch("factory.podman.resolve_image") as resolve: + assert cli.cmd_contained(args) == 0 + resolve.assert_not_called() + assert render.call_args.kwargs["image"] == "custom:tag" + + +def test_a_lifecycle_subcommand_is_handed_to_dispatch_lifecycle() -> None: + args = interpret(["ls"]) + with patch("factory.cli.contained.dispatch_lifecycle", return_value=0) as dispatch: + assert cli.cmd_contained(args) == 0 + dispatch.assert_called_once() + + +def test_a_payload_run_against_k8s_routes_to_run_k8s() -> None: + args = interpret(["--target", "k8s", "--", "ceo", "/tmp"]) + with patch("factory.cli.contained_k8s.run_k8s", return_value=0) as run_k8s: + assert cli.cmd_contained(args) == 0 + run_k8s.assert_called_once_with(args) + + +def test_a_payload_run_against_local_routes_to_run_local() -> None: + # `run_local` is imported into `contained.py` at module load, so patch it there. + args = interpret(["--", "study", "/tmp"]) + with patch("factory.cli.contained.run_local", return_value=0) as run_local: + assert cli.cmd_contained(args) == 0 + run_local.assert_called_once_with(args) + + +def test_help_subcommand_prints_the_help_and_returns_zero( + capsys: pytest.CaptureFixture[str], +) -> None: + args = interpret(["help"]) + assert cli.cmd_contained(args) == 0 + assert "Targets:" in capsys.readouterr().out + + +# ============================================================================================ +# contained_args.py — the remaining command-line reading branches. +# ============================================================================================ + + +def test_within_one_edit_covers_every_shape() -> None: + we = contained_args._within_one_edit + assert we("ls", "ls") is True # identical + assert we("abcd", "xy") is False # length gap > 1 + assert we("ls", "lx") is True # one substitution + assert we("ls", "xy") is False # two substitutions + assert we("ls", "lst") is True # one insertion/deletion + assert we("ab", "cde") is False # same length gap of 1, but nothing matches + + +def test_a_close_typo_of_a_subcommand_is_named() -> None: + """`lst` is one edit from `ls`, so it is caught before the passthrough path.""" + parser = argparse.ArgumentParser() + with pytest.raises(SystemExit): + contained_args._reject_subcommand_typo(parser, ["lst"]) + + +def test_a_leading_flag_is_left_for_the_real_parser() -> None: + """A token beginning with `-` is not a subcommand typo; the check returns without complaint.""" + parser = argparse.ArgumentParser() + contained_args._reject_subcommand_typo(parser, ["--something"]) # no raise + + +def test_an_existing_directory_is_not_treated_as_a_typo(tmp_path: Path) -> None: + parser = argparse.ArgumentParser() + contained_args._reject_subcommand_typo(parser, [str(tmp_path)]) # no raise + + +def test_a_word_far_from_any_subcommand_is_left_alone() -> None: + """A free-text first token that resembles no subcommand falls through to the passthrough.""" + parser = argparse.ArgumentParser() + contained_args._reject_subcommand_typo(parser, ["totallyunrelated"]) # no raise + + +def test_an_empty_remainder_is_a_no_op() -> None: + parser = argparse.ArgumentParser() + contained_args._reject_subcommand_typo(parser, []) # no raise + + +def test_target_given_recognizes_both_the_space_and_equals_forms() -> None: + with patch.object(contained_args.sys, "argv", ["factory", "contained", "--target", "k8s"]): + assert contained_args.target_given(argparse.Namespace()) is True + with patch.object(contained_args.sys, "argv", ["factory", "contained", "--target=local"]): + assert contained_args.target_given(argparse.Namespace()) is True + with patch.object(contained_args.sys, "argv", ["factory", "contained", "setup"]): + assert contained_args.target_given(argparse.Namespace()) is False + + +def test_bundle_interpret_promotes_the_target_to_k8s() -> None: + """Directly exercises the `subcommand == "bundle"` promotion line inside `interpret`.""" + args = interpret(["--namespace", "ns", "bundle"]) + assert (args.subcommand, args.target, args.namespace) == ("bundle", "k8s", "ns") + + +def test_forwarding_an_unset_variable_raises() -> None: + args = argparse.Namespace(extra_env=[], forward=["DEFINITELY_NOT_SET_XYZ"]) + with patch.dict(contained_args.os.environ, {}, clear=True): + with pytest.raises(ContainedError, match="not set in this environment"): + contained_args.validate_env_args(args) + + +def test_forwarding_a_set_variable_returns_its_value() -> None: + args = argparse.Namespace(extra_env=["A=1"], forward=["PRESENT_XYZ"]) + with patch.dict(contained_args.os.environ, {"PRESENT_XYZ": "here"}, clear=False): + extra, forwarded = contained_args.validate_env_args(args) + assert extra == {"A": "1"} + assert forwarded == {"PRESENT_XYZ": "here"} + + +def test_parse_extra_env_rejects_a_pair_without_an_equals() -> None: + with pytest.raises(ContainedError, match="not KEY=VALUE"): + contained_args.parse_extra_env(["NOEQUALS"]) + + +def test_parse_extra_env_rejects_a_blank_key() -> None: + with pytest.raises(ContainedError, match="not KEY=VALUE"): + contained_args.parse_extra_env(["=value"]) + + +def test_resolve_project_returns_the_first_existing_directory(tmp_path: Path) -> None: + project = tmp_path / "proj" + project.mkdir() + assert contained_args.resolve_project(["ceo", str(project), "--loop"]) == project.resolve() + + +def test_resolve_project_raises_when_no_directory_is_named() -> None: + with pytest.raises(ContainedError, match="no existing directory"): + contained_args.resolve_project(["ceo", "--focus", "x"]) + + +# ============================================================================================ +# contained_local.py — helpers. +# ============================================================================================ + + +def test_macos_share_warning_is_silent_off_darwin() -> None: + with patch.object(contained_local.platform, "system", return_value="Linux"): + assert contained_local._macos_share_warning([Mount(Path("/x"), "/x")]) is None + + +def test_macos_share_warning_is_silent_when_no_shared_paths_are_known() -> None: + with patch.object(contained_local.platform, "system", return_value="Darwin"), \ + patch.object(contained_local, "_machine_shared_paths", return_value=[]): + assert contained_local._macos_share_warning([Mount(Path("/x"), "/x")]) is None + + +def test_macos_share_warning_is_silent_when_every_mount_is_inside_a_shared_path() -> None: + with patch.object(contained_local.platform, "system", return_value="Darwin"), \ + patch.object(contained_local, "_machine_shared_paths", return_value=[Path("/shared")]): + assert contained_local._macos_share_warning([Mount(Path("/shared/proj"), "/x")]) is None + + +def test_macos_share_warning_names_a_mount_outside_the_shared_paths() -> None: + with patch.object(contained_local.platform, "system", return_value="Darwin"), \ + patch.object(contained_local, "_machine_shared_paths", return_value=[Path("/shared")]): + warning = contained_local._macos_share_warning([Mount(Path("/elsewhere/proj"), "/x")]) + assert warning is not None and "/elsewhere/proj" in warning and "/shared" in warning + + +def test_machine_shared_paths_returns_empty_when_podman_is_absent() -> None: + with patch.object(contained_local.subprocess, "run", side_effect=FileNotFoundError): + assert contained_local._machine_shared_paths() == [] + + +def test_machine_shared_paths_returns_empty_on_a_nonzero_exit() -> None: + with patch.object(contained_local.subprocess, "run", return_value=_completed(returncode=1)): + assert contained_local._machine_shared_paths() == [] + + +def test_machine_shared_paths_keeps_only_absolute_paths() -> None: + out = "/abs/one\nrelative\n \n/abs/two\n" + with patch.object(contained_local.subprocess, "run", return_value=_completed(stdout=out)): + assert contained_local._machine_shared_paths() == [Path("/abs/one"), Path("/abs/two")] + + +def test_handle_create_failure_ignores_a_non_create_step(tmp_path: Path) -> None: + plan = _plan(tmp_path) + result = _completed(returncode=1, stderr="already in use") + out, hint = contained_local._handle_create_failure(Step("run", ["x"]), result, plan) + assert out is result and hint is None + + +def test_handle_create_failure_ignores_an_unrelated_create_error(tmp_path: Path) -> None: + plan = _plan(tmp_path) + result = _completed(returncode=1, stderr="disk full") + out, hint = contained_local._handle_create_failure(Step("create", ["x"]), result, plan) + assert out is result and hint is None + + +def test_handle_create_failure_reaps_and_retries_successfully(tmp_path: Path) -> None: + plan = _plan(tmp_path) + first = _completed(returncode=1, stderr="name already in use") + retry = _completed(returncode=0) + with patch.object(contained_local, "reap_stale", return_value=(True, "was exited")), \ + patch.object(contained_local, "_run_step", return_value=retry): + out, hint = contained_local._handle_create_failure(Step("create", ["x"]), first, plan) + assert out is retry and hint is None + + +def test_handle_create_failure_reaps_but_the_retry_still_fails(tmp_path: Path) -> None: + plan = _plan(tmp_path) + first = _completed(returncode=1, stderr="already exists") + retry = _completed(returncode=1, stderr="still broken") + with patch.object(contained_local, "reap_stale", return_value=(True, "was exited")), \ + patch.object(contained_local, "_run_step", return_value=retry): + out, hint = contained_local._handle_create_failure(Step("create", ["x"]), first, plan) + assert out is retry and hint is not None and "already exists" in hint + + +def test_handle_create_failure_declines_to_reap_a_live_container(tmp_path: Path) -> None: + plan = _plan(tmp_path) + first = _completed(returncode=1, stderr="name already in use") + with patch.object(contained_local, "reap_stale", return_value=(False, "still active")), \ + patch.object(contained_local, "_run_step") as run_step: + out, hint = contained_local._handle_create_failure(Step("create", ["x"]), first, plan) + run_step.assert_not_called() + assert out is first and hint is not None and "still active" in hint + + +def test_run_step_uses_a_longer_timeout_for_create() -> None: + with patch.object(contained_local.subprocess, "run", return_value=_completed()) as run: + contained_local._run_step(Step("create", ["podman", "run"])) + assert run.call_args.kwargs["timeout"] == 300 + with patch.object(contained_local.subprocess, "run", return_value=_completed()) as run: + contained_local._run_step(Step("run", ["podman", "exec"])) + assert run.call_args.kwargs["timeout"] == 120 + + +def test_roll_back_is_a_no_op_without_a_workspace() -> None: + contained_local._roll_back(None) # no raise + + +def test_roll_back_is_a_no_op_when_the_copy_is_already_gone(tmp_path: Path) -> None: + ws = Workspace(source=tmp_path, path=tmp_path / "gone", kind="copy") + contained_local._roll_back(ws) # no raise + + +def test_roll_back_releases_the_copy_and_removes_an_empty_run_dir(tmp_path: Path) -> None: + import shutil + + run_dir = tmp_path / "run" + copy = run_dir / "rta" + copy.mkdir(parents=True) + ws = Workspace(source=tmp_path / "src", path=copy, kind="worktree", branch="b") + + def _release(workspace: Workspace, *, delete_branch: bool) -> None: + shutil.rmtree(workspace.path) + + with patch("factory.contained.workspace.release", side_effect=_release): + contained_local._roll_back(ws) + assert not run_dir.exists(), "the emptied run directory should be removed too" + + +def test_roll_back_leaves_a_run_dir_that_still_holds_other_work(tmp_path: Path) -> None: + """When the run directory is not empty after release, it is left in place, not removed.""" + import shutil + + run_dir = tmp_path / "run" + copy = run_dir / "rta" + copy.mkdir(parents=True) + (run_dir / "sibling").mkdir() # keeps the run dir non-empty after the copy is released + ws = Workspace(source=tmp_path / "src", path=copy, kind="worktree", branch="b") + + def _release(workspace: Workspace, *, delete_branch: bool) -> None: + shutil.rmtree(workspace.path) + + with patch("factory.contained.workspace.release", side_effect=_release): + contained_local._roll_back(ws) + assert run_dir.exists() and (run_dir / "sibling").exists() + + +def test_roll_back_reports_rather_than_masks_a_cleanup_failure( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + copy = tmp_path / "run" / "rta" + copy.mkdir(parents=True) + ws = Workspace(source=tmp_path / "src", path=copy, kind="worktree", branch="b") + with patch("factory.contained.workspace.release", side_effect=WorkspaceError("locked")), \ + patch("factory.contained.workspace.cleanup_hint", return_value="do this by hand"): + contained_local._roll_back(ws) + err = capsys.readouterr().err + assert "could not clean up" in err and "do this by hand" in err + + +def test_settle_workspace_keeps_the_copy_on_success(tmp_path: Path) -> None: + ws = Workspace(source=tmp_path, path=tmp_path, kind="copy") + with patch.object(contained_local, "_roll_back") as roll_back: + contained_local._settle_workspace(ws, code=0, created=True) + roll_back.assert_not_called() + + +def test_settle_workspace_rolls_back_when_nothing_was_created(tmp_path: Path) -> None: + ws = Workspace(source=tmp_path, path=tmp_path, kind="copy") + with patch.object(contained_local, "_roll_back") as roll_back: + contained_local._settle_workspace(ws, code=1, created=False) + roll_back.assert_called_once_with(ws) + + +def test_settle_workspace_keeps_a_created_runtime_for_inspection( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + ws = Workspace(source=tmp_path, path=tmp_path, kind="copy") + with patch("factory.contained.workspace.cleanup_hint", return_value="inspect me"): + contained_local._settle_workspace(ws, code=1, created=True) + assert "inspect me" in capsys.readouterr().err + + +def test_announce_prints_the_lifecycle_commands( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + contained_local._announce(_plan(tmp_path)) + out = capsys.readouterr().out + assert "Starting rta-abc123" in out + assert "attach:" in out and "sync" in out and "rm" in out + + +def test_execute_runs_every_step_and_reports_a_created_container(tmp_path: Path) -> None: + plan = _plan(tmp_path) + steps = [Step("create", ["c"]), Step("run", ["r"])] + with patch.object(contained_local, "_run_step", return_value=_completed()): + code, created = contained_local._execute(plan, steps, []) + assert code == 0 and created is True + + +def test_execute_returns_130_on_interrupt( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + plan = _plan(tmp_path) + with patch.object(contained_local, "_run_step", side_effect=KeyboardInterrupt): + code, created = contained_local._execute(plan, [Step("create", ["c"])], []) + assert code == 130 and created is False + assert "may still be running" in capsys.readouterr().err + + +def test_execute_reports_a_failure_before_the_container_exists( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + plan = _plan(tmp_path) + failed = _completed(returncode=1, stderr="boom") + with patch.object(contained_local, "_run_step", return_value=failed), \ + patch.object(contained_local, "_handle_create_failure", return_value=(failed, "a hint")): + code, created = contained_local._execute(plan, [Step("create", ["c"])], []) + assert code == 1 and created is False + err = capsys.readouterr().err + assert "step 'create' failed" in err and "a hint" in err + + +def test_execute_keeps_a_created_container_after_a_later_step_fails( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + plan = _plan(tmp_path) + steps = [Step("create", ["c"]), Step("assert:git", ["g"])] + probes = [Probe(name="git", argv=["g"], hint="mount hint")] + with patch.object( + contained_local, "_run_step", + side_effect=[_completed(), _completed(returncode=1, stderr="assertion failed")], + ): + code, created = contained_local._execute(plan, steps, probes) + assert code == 1 and created is True + err = capsys.readouterr().err + assert "still there for inspection" in err and "mount hint" in err + + +def test_probes_for_projects_from_the_source_in_dry_run( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + project = tmp_path / "proj" + project.mkdir() + ws = Workspace(source=project, path=tmp_path / "copy", kind="copy") + with patch.object(contained_local, "content_probe", return_value=("a.txt", "deadbeef")): + probes = contained_local._probes_for(ws, project, dry_run=True) + assert probes # a real probe list is still produced + assert "projection from the source tree" in capsys.readouterr().err + + +def test_probes_for_is_silent_when_the_source_has_no_content_probe(tmp_path: Path) -> None: + project = tmp_path / "proj" + project.mkdir() + ws = Workspace(source=project, path=tmp_path / "copy", kind="copy") + with patch.object(contained_local, "content_probe", return_value=None): + probes = contained_local._probes_for(ws, project, dry_run=True) + assert probes + + +def test_probes_for_measures_the_copy_when_not_a_dry_run(tmp_path: Path) -> None: + project = tmp_path / "proj" + project.mkdir() + copy = tmp_path / "copy" + copy.mkdir() + ws = Workspace(source=project, path=copy, kind="copy") + with patch.object(contained_local, "content_probe", return_value=None) as content: + contained_local._probes_for(ws, project, dry_run=False) + content.assert_called_once_with(copy) + + +# -------------------------------------------------------------------------------------------- +# contained_local.py — `_build_plan` composition branches. +# -------------------------------------------------------------------------------------------- + + +def _plan_args(tmp_path: Path, **overrides: object) -> argparse.Namespace: + project = tmp_path / "src" + project.mkdir(exist_ok=True) + fields: dict[str, object] = dict( + image="img:pinned", extra_env=[], forward=[], mount=[], + factory_args=["study", str(project)], name=None, + ) + fields.update(overrides) + return argparse.Namespace(**fields) + + +def test_build_plan_warns_when_no_credentials_and_home_is_absent(tmp_path: Path) -> None: + """A non-worktree copy, no `~/.factory`, no credentials: the missing-inference warning fires.""" + home = tmp_path / "empty-home" + home.mkdir() + (tmp_path / "src").mkdir(exist_ok=True) + (tmp_path / "copy").mkdir(exist_ok=True) + ws = Workspace(source=tmp_path / "src", path=tmp_path / "copy", kind="copy") + args = _plan_args(tmp_path) + with patch.dict(contained_local.os.environ, {"HOME": str(home)}, clear=False), \ + patch.object( + contained_local, "resolve_credentials", + return_value=CredentialShape(backend="none", ok=False, detail=""), + ): + plan = contained_local._build_plan(args, ws, dry_run=True) + assert any("no inference credentials" in w for w in plan.warnings) + # No `~/.factory` mount was added, and a plain copy adds no git-common mount. + assert not any(m.target.endswith(".factory") for m in plan.mounts) + assert plan.image == "img:pinned" + + +def test_build_plan_appends_a_macos_share_warning(tmp_path: Path) -> None: + (tmp_path / "src").mkdir(exist_ok=True) + (tmp_path / "copy").mkdir(exist_ok=True) + ws = Workspace(source=tmp_path / "src", path=tmp_path / "copy", kind="copy") + args = _plan_args(tmp_path) + with patch.object( + contained_local, "resolve_credentials", + return_value=CredentialShape(backend="none", ok=False, detail=""), + ), \ + patch.object(contained_local, "_macos_share_warning", return_value="share warning"): + plan = contained_local._build_plan(args, ws, dry_run=True) + assert "share warning" in plan.warnings + + +def test_build_plan_rejects_an_extra_mount_that_does_not_exist(tmp_path: Path) -> None: + (tmp_path / "src").mkdir(exist_ok=True) + (tmp_path / "copy").mkdir(exist_ok=True) + ws = Workspace(source=tmp_path / "src", path=tmp_path / "copy", kind="copy") + args = _plan_args(tmp_path, mount=[str(tmp_path / "nope")]) + with patch.object( + contained_local, "resolve_credentials", + return_value=CredentialShape(backend="none", ok=False, detail=""), + ): + with pytest.raises(ContainedError, match="no such path"): + contained_local._build_plan(args, ws, dry_run=True) + + +def test_build_plan_mounts_an_existing_extra_path(tmp_path: Path) -> None: + (tmp_path / "src").mkdir(exist_ok=True) + (tmp_path / "copy").mkdir(exist_ok=True) + ws = Workspace(source=tmp_path / "src", path=tmp_path / "copy", kind="copy") + extra = tmp_path / "extra" + extra.mkdir() + args = _plan_args(tmp_path, mount=[str(extra)]) + with patch.object( + contained_local, "resolve_credentials", + return_value=CredentialShape(backend="none", ok=False, detail=""), + ): + plan = contained_local._build_plan(args, ws, dry_run=True) + assert any(m.source == extra.resolve() for m in plan.mounts) + + +def test_build_plan_skips_the_git_common_mount_when_there_is_none(tmp_path: Path) -> None: + """A worktree whose common dir cannot be resolved simply adds no git mount.""" + (tmp_path / "src").mkdir(exist_ok=True) + (tmp_path / "copy").mkdir(exist_ok=True) + ws = Workspace(source=tmp_path / "src", path=tmp_path / "copy", kind="worktree", branch="b") + args = _plan_args(tmp_path) + with patch.object( + contained_local, "resolve_credentials", + return_value=CredentialShape(backend="none", ok=False, detail=""), + ), \ + patch.object(contained_local, "git_common_dir", return_value=None): + plan = contained_local._build_plan(args, ws, dry_run=True) + assert not any(m.target.endswith(".git") for m in plan.mounts) + + +def test_build_plan_mounts_the_worktree_common_dir_when_present(tmp_path: Path) -> None: + common = tmp_path / "src" / ".git" + common.mkdir(parents=True) + (tmp_path / "copy").mkdir(exist_ok=True) + ws = Workspace(source=tmp_path / "src", path=tmp_path / "copy", kind="worktree", branch="b") + args = _plan_args(tmp_path) + with patch.object( + contained_local, "resolve_credentials", + return_value=CredentialShape(backend="none", ok=False, detail=""), + ), \ + patch.object(contained_local, "git_common_dir", return_value=common): + plan = contained_local._build_plan(args, ws, dry_run=True) + assert any(m.source == common for m in plan.mounts) + + +# -------------------------------------------------------------------------------------------- +# contained_local.py — `run_local`, the non-dry-run execution path. +# -------------------------------------------------------------------------------------------- + + +def _run_local_args(tmp_path: Path, **overrides: object) -> argparse.Namespace: + project = tmp_path / "src" + project.mkdir(exist_ok=True) + fields: dict[str, object] = dict( + image=None, extra_env=[], forward=[], mount=[], division=False, + factory_args=["study", str(project)], name="rta-run", + ) + fields.update(overrides) + return argparse.Namespace(**fields) + + +def test_run_local_executes_and_settles_on_success(tmp_path: Path) -> None: + args = _run_local_args(tmp_path) + ws = Workspace(source=tmp_path / "src", path=tmp_path / "copy", kind="copy") + plan = _plan(tmp_path) + with patch.object(contained_local, "dry_run_enabled", return_value=False), \ + patch.object(contained_local, "materialize", return_value=ws), \ + patch.object(contained_local, "_build_plan", return_value=plan), \ + patch.object(contained_local, "_probes_for", return_value=[]), \ + patch.object(contained_local, "plan_steps", return_value=[Step("run", ["r"])]), \ + patch.object(contained_local, "growth_context_warning", return_value=None), \ + patch.object(contained_local.shutil, "which", return_value="/usr/bin/podman"), \ + patch("factory.contained.usage.record_target") as record, \ + patch.object(contained_local, "_execute", return_value=(0, True)), \ + patch.object(contained_local, "_settle_workspace") as settle: + assert contained_local.run_local(args) == 0 + record.assert_called_once_with("local") + settle.assert_called_once() + + +def test_run_local_errors_when_podman_is_missing( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + args = _run_local_args(tmp_path) + ws = Workspace(source=tmp_path / "src", path=tmp_path / "gone", kind="copy") + plan = _plan(tmp_path) + with patch.object(contained_local, "dry_run_enabled", return_value=False), \ + patch.object(contained_local, "materialize", return_value=ws), \ + patch.object(contained_local, "_build_plan", return_value=plan), \ + patch.object(contained_local, "_probes_for", return_value=[]), \ + patch.object(contained_local, "plan_steps", return_value=[Step("run", ["r"])]), \ + patch.object(contained_local, "growth_context_warning", return_value=None), \ + patch.object(contained_local.shutil, "which", return_value=None): + assert contained_local.run_local(args) == 1 + assert "`podman` is not installed" in capsys.readouterr().err + + +def test_run_local_reports_a_provisioning_error_and_returns_two( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + args = _run_local_args(tmp_path) + with patch.object(contained_local, "dry_run_enabled", return_value=False), \ + patch.object(contained_local, "materialize", side_effect=WorkspaceError("no copy")): + assert contained_local.run_local(args) == 2 + assert "Error: no copy" in capsys.readouterr().err + + +def test_run_local_prints_warnings_but_keeps_the_exit_code( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + args = _run_local_args(tmp_path) + ws = Workspace(source=tmp_path / "src", path=tmp_path / "copy", kind="copy") + plan = ContainerPlan( + name="rta-warn", image="i", workdir=str(tmp_path), env={}, labels={}, + mounts=(), run_command="x", warnings=("credentials missing",), + ) + with patch.object(contained_local, "dry_run_enabled", return_value=False), \ + patch.object(contained_local, "materialize", return_value=ws), \ + patch.object(contained_local, "_build_plan", return_value=plan), \ + patch.object(contained_local, "_probes_for", return_value=[]), \ + patch.object(contained_local, "plan_steps", return_value=[Step("run", ["r"])]), \ + patch.object(contained_local, "growth_context_warning", return_value="a growth warning"), \ + patch.object(contained_local.shutil, "which", return_value="/usr/bin/podman"), \ + patch("factory.contained.usage.record_target"), \ + patch.object(contained_local, "_execute", return_value=(0, True)), \ + patch.object(contained_local, "_settle_workspace"): + assert contained_local.run_local(args) == 0 + err = capsys.readouterr().err + assert "a growth warning" in err and "credentials missing" in err + + +def test_run_local_starts_and_keeps_the_division_on_success(tmp_path: Path) -> None: + args = _run_local_args(tmp_path, division=True) + ws = Workspace(source=tmp_path / "src", path=tmp_path / "copy", kind="copy") + plan = _plan(tmp_path) + division = _FakeDivision(plan) + with patch.object(contained_local, "dry_run_enabled", return_value=False), \ + patch.object(contained_local, "materialize", return_value=ws), \ + patch.object(contained_local, "_build_plan", return_value=plan), \ + patch.object(contained_local, "_probes_for", return_value=[]), \ + patch.object(contained_local, "plan_steps", return_value=[Step("run", ["r"])]), \ + patch.object(contained_local, "growth_context_warning", return_value=None), \ + patch.object(contained_local.shutil, "which", return_value="/usr/bin/podman"), \ + patch("factory.contained.division.start_local_division", return_value=division), \ + patch("factory.contained.usage.record_target"), \ + patch.object(contained_local, "_execute", return_value=(0, True)), \ + patch.object(contained_local, "_settle_workspace"): + assert contained_local.run_local(args) == 0 + assert division.kept is True and division.stopped is False + + +def test_run_local_stops_the_division_when_the_run_fails(tmp_path: Path) -> None: + args = _run_local_args(tmp_path, division=True) + ws = Workspace(source=tmp_path / "src", path=tmp_path / "copy", kind="copy") + plan = _plan(tmp_path) + division = _FakeDivision(plan) + with patch.object(contained_local, "dry_run_enabled", return_value=False), \ + patch.object(contained_local, "materialize", return_value=ws), \ + patch.object(contained_local, "_build_plan", return_value=plan), \ + patch.object(contained_local, "_probes_for", return_value=[]), \ + patch.object(contained_local, "plan_steps", return_value=[Step("run", ["r"])]), \ + patch.object(contained_local, "growth_context_warning", return_value=None), \ + patch.object(contained_local.shutil, "which", return_value="/usr/bin/podman"), \ + patch("factory.contained.division.start_local_division", return_value=division), \ + patch("factory.contained.usage.record_target"), \ + patch.object(contained_local, "_execute", return_value=(1, True)), \ + patch.object(contained_local, "_settle_workspace"): + assert contained_local.run_local(args) == 1 + assert division.kept is False and division.stopped is True diff --git a/tests/test_contained_k8s.py b/tests/test_contained_k8s.py index 3fe1f2cba..897cadd75 100644 --- a/tests/test_contained_k8s.py +++ b/tests/test_contained_k8s.py @@ -15,7 +15,7 @@ from factory.cli import contained as cli from factory.cli.contained_k8s import PACK_EXCLUDES, _build_pod_plan, _pack -from factory.contained import k8s, k8s_setup, secrets +from factory.contained import k8s, k8s_credentials, k8s_setup, secrets from factory.contained.bundle import SCC_ROLEBINDING, render_bundle from factory.contained.k8s import ( FACTORY_CONTAINER, @@ -74,7 +74,13 @@ def _no_real_kubeconfig(): patch("factory.contained.k8s_setup.current_namespace", return_value=None), \ patch("factory.contained.k8s_setup._namespace_status", return_value=k8s_setup.PRESENT), \ patch("factory.contained.k8s._run", return_value=_completed("true")), \ + patch("factory.contained.k8s_credentials._run", return_value=_completed("{}")), \ + patch("factory.contained.k8s_setup.run_credentials_step", return_value=False), \ patch("factory.contained.k8s_setup.access_review", return_value=True): + # The credentials step is stubbed for the same two reasons as the rest: it reads the + # cluster, and it is a conversation. A `setup_k8s` test that let it run would consume the + # mocked `input()` the object walk is asserting on, and would block on a prompt that never + # receives a valid key. `tests/test_contained_k8s_credentials.py` exercises it directly. # `access_review` is stubbed under the name *k8s_setup* imported, not on `k8s` itself: it # shells out with `subprocess.run` directly, so nothing else here catches it, and the test # that exercises the real function reaches it through `k8s.access_review`, untouched. @@ -394,7 +400,11 @@ def fake_run(argv, **kwargs): if "current-context" in argv: return _completed("ctx") if "rolebinding" in argv and SCC_ROLEBINDING in argv: - return _completed(returncode=1) + # `NotFound` specifically: any non-zero used to count as missing, which turned an + # expired login into a namespace that appeared to hold nothing. + missing = _completed(returncode=1) + missing.stderr = f'Error from server (NotFound): rolebindings "{SCC_ROLEBINDING}" not found' + return missing return _completed("ok") with patch("factory.contained.k8s_setup.cli_binary", return_value="oc"), \ @@ -463,8 +473,8 @@ def test_pods_exec_being_granted_is_itself_a_failure() -> None: def test_a_secret_with_the_wrong_keys_is_reported_by_key_never_by_value() -> None: payload = json.dumps({"SOME_OTHER_KEY": "c2VjcmV0"}) - with patch("factory.contained.k8s_setup._run", return_value=_completed(payload)): - check = k8s_setup._secret_check("oc", "ns") + with patch("factory.contained.k8s_credentials._run", return_value=_completed(payload)): + check = k8s_credentials.secret_check("oc", "ns") assert not check.ok assert "SOME_OTHER_KEY" in check.detail assert "c2VjcmV0" not in check.detail @@ -473,8 +483,8 @@ def test_a_secret_with_the_wrong_keys_is_reported_by_key_never_by_value() -> Non def test_a_vertex_secret_is_accepted() -> None: payload = json.dumps({k: "x" for k in k8s_setup.VERTEX_KEYS}) - with patch("factory.contained.k8s_setup._run", return_value=_completed(payload)): - assert k8s_setup._secret_check("oc", "ns").ok + with patch("factory.contained.k8s_credentials._run", return_value=_completed(payload)): + assert k8s_credentials.secret_check("oc", "ns").ok def test_setup_reports_the_current_state_before_asking( @@ -755,12 +765,12 @@ def test_vertex_configuration_without_a_credential_is_not_enough() -> None: k: "x" for k in ("CLAUDE_CODE_USE_VERTEX", "CLOUD_ML_REGION", "ANTHROPIC_VERTEX_PROJECT_ID") }) - with patch("factory.contained.k8s_setup._run", return_value=_completed(config_only)): - assert not k8s_setup._secret_check("oc", "ns").ok + with patch("factory.contained.k8s_credentials._run", return_value=_completed(config_only)): + assert not k8s_credentials.secret_check("oc", "ns").ok # With the credential file, it passes. complete = json.dumps({k: "x" for k in k8s_setup.VERTEX_KEYS}) - with patch("factory.contained.k8s_setup._run", return_value=_completed(complete)): - assert k8s_setup._secret_check("oc", "ns").ok + with patch("factory.contained.k8s_credentials._run", return_value=_completed(complete)): + assert k8s_credentials.secret_check("oc", "ns").ok def test_secret_keys_reads_names_and_never_values() -> None: @@ -804,7 +814,7 @@ def test_the_inference_probe_is_skipped_when_the_secret_is_missing() -> None: """The probe pod mounts that Secret; without it the wait is 180s to learn what we know.""" with patch("factory.contained.k8s_setup.cli_binary", return_value="oc"), \ patch("factory.contained.k8s_setup._run", return_value=_completed("ctx")), \ - patch("factory.contained.k8s_setup._secret_check", + patch("factory.contained.k8s_setup.secret_check", return_value=Check("credentials_secret", False, "missing", fix="oc create secret")), \ patch("factory.contained.k8s_setup._inference_check") as probe: checks = k8s_setup.verify_k8s(namespace="ns") @@ -818,7 +828,7 @@ def test_the_inference_probe_is_skipped_when_the_secret_is_missing() -> None: def test_the_inference_probe_still_runs_when_the_secret_is_there() -> None: with patch("factory.contained.k8s_setup.cli_binary", return_value="oc"), \ patch("factory.contained.k8s_setup._run", return_value=_completed("ctx")), \ - patch("factory.contained.k8s_setup._secret_check", + patch("factory.contained.k8s_setup.secret_check", return_value=Check("credentials_secret", True, "present")), \ patch("factory.contained.k8s_setup._inference_check", return_value=Check("inference_from_cluster", True, "reached")) as probe: @@ -968,16 +978,23 @@ def test_setup_applies_nothing_without_confirmation(capsys: pytest.CaptureFixtur def test_setup_says_so_when_no_cluster_is_selected(capsys: pytest.CaptureFixture[str]) -> None: - """"About to apply ... with your own credentials" is untrue when there are none.""" + """"About to apply ... with your own credentials" is untrue when there are none. + + The gate asks the *cluster*, not the kubeconfig: `config current-context` reads a local file + and passes happily for a context whose token expired hours ago, which is precisely the state + where every apply below it would fail. + """ with patch("factory.contained.k8s_setup.cli_binary", return_value="oc"), \ patch("factory.contained.k8s_setup.resolve_namespace", return_value="ns"), \ patch("factory.contained.k8s_setup._run", return_value=_completed("", returncode=1)), \ + patch("factory.contained.k8s_setup.login_status", + return_value=(False, "You must be logged in to the server (Unauthorized)")), \ patch("factory.contained.k8s_setup.subprocess.run") as run: code = k8s_setup.setup_k8s(namespace="ns", division=False, interactive=True, assume_yes=True) run.assert_not_called() assert code == 1 - assert "No cluster is selected" in capsys.readouterr().err + assert "no working credential" in capsys.readouterr().err def test_setup_degrades_to_printing_when_apply_is_refused( @@ -1016,3 +1033,10 @@ def test_a_sweep_that_deleted_something_reports_a_count( return_value=_completed('pod "a" deleted\npod "b" deleted')): k8s.remove_cluster_runtime("rta-test", namespace="ns") assert "swept 2 pod(s)" in capsys.readouterr().out + + +def test_a_plan_without_a_vertex_model_warning_adds_no_warning(tmp_path: Path) -> None: + """The common path: a non-Vertex backend produces no model warning, so none is appended.""" + with patch("factory.cli.contained_k8s.vertex_model_warning", return_value=None): + plan = _plan(tmp_path) + assert not any("--model" in w for w in plan.warnings) diff --git a/tests/test_contained_k8s_coverage.py b/tests/test_contained_k8s_coverage.py new file mode 100644 index 000000000..9e74aa032 --- /dev/null +++ b/tests/test_contained_k8s_coverage.py @@ -0,0 +1,521 @@ +"""Coverage-completing tests for `factory.contained.k8s`. + +The existing suites cover the happy paths and the manifest shapes; this file targets the failure +directions those leave behind — every "the CLI could not be run", "the JSON was junk", "the token +expired", and "the pod never started" branch. Each of these is a place the module deliberately +degrades rather than raising, and a branch that degrades wrongly is exactly the kind of bug that +only shows up on a real cluster having a bad day, so it is worth pinning here. + +Nothing in this file may touch a real cluster: everything shells out through `k8s._run` / +`k8s.subprocess.run` / `shutil.which`, and every test patches those. `wait_for_container` polls on +a clock, so its tests patch `time.monotonic` (to drive the deadline instantly) and `time.sleep` +(to a no-op) — the module imports `time` inside the function, so those are patched on the real +`time` module rather than on the k8s namespace. +""" + +from __future__ import annotations + +import json +import subprocess +from pathlib import Path +from unittest.mock import patch + +import pytest + +from factory.contained import k8s +from factory.contained.runtimes import LifecycleError + + +def _completed(stdout: str = "", returncode: int = 0, stderr: str = "") -> subprocess.CompletedProcess[str]: + return subprocess.CompletedProcess([], returncode, stdout, stderr) + + +# -------------------------------------------------------------------------------------------- +# cli_binary — the "neither tool is installed" path +# -------------------------------------------------------------------------------------------- + + +def test_cli_binary_prefers_oc_then_kubectl() -> None: + with patch("factory.contained.k8s.shutil.which", side_effect=lambda c: c == "oc"): + assert k8s.cli_binary() == "oc" + # oc absent, kubectl present — exercises the loop advancing past the first candidate. + with patch("factory.contained.k8s.shutil.which", side_effect=lambda c: c == "kubectl"): + assert k8s.cli_binary() == "kubectl" + + +def test_cli_binary_raises_when_neither_is_on_path() -> None: + with patch("factory.contained.k8s.shutil.which", return_value=None): + with pytest.raises(k8s.ClusterError, match="neither"): + k8s.cli_binary() + + +# -------------------------------------------------------------------------------------------- +# current_namespace +# -------------------------------------------------------------------------------------------- + + +def test_current_namespace_reads_the_context_namespace() -> None: + with patch("factory.contained.k8s.cli_binary", return_value="oc"), \ + patch("factory.contained.k8s._run", return_value=_completed("team-ns\n")): + assert k8s.current_namespace() == "team-ns" + + +def test_current_namespace_is_none_when_the_read_fails_or_is_empty() -> None: + with patch("factory.contained.k8s.cli_binary", return_value="oc"), \ + patch("factory.contained.k8s._run", return_value=None): + assert k8s.current_namespace() is None + with patch("factory.contained.k8s.cli_binary", return_value="oc"), \ + patch("factory.contained.k8s._run", return_value=_completed(" ")): + assert k8s.current_namespace() is None + + +# -------------------------------------------------------------------------------------------- +# _kubeconfig_json / list_contexts / _first_section / cluster_context +# -------------------------------------------------------------------------------------------- + + +def test_kubeconfig_json_degrades_to_empty_on_a_failed_read() -> None: + with patch("factory.contained.k8s._run", return_value=None): + assert k8s._kubeconfig_json(["oc", "config", "view"]) == {} + with patch("factory.contained.k8s._run", return_value=_completed("{}", 1)): + assert k8s._kubeconfig_json(["oc", "config", "view"]) == {} + + +def test_kubeconfig_json_degrades_when_the_top_level_is_not_an_object() -> None: + with patch("factory.contained.k8s._run", return_value=_completed("[1, 2, 3]")): + assert k8s._kubeconfig_json(["oc", "config", "view"]) == {} + + +def test_list_contexts_is_empty_when_no_cli_is_installed() -> None: + with patch("factory.contained.k8s.cli_binary", side_effect=k8s.ClusterError("no cli")): + assert k8s.list_contexts() == [] + + +def test_list_contexts_skips_non_dict_entries() -> None: + payload = json.dumps({ + "contexts": ["junk", {"name": "dev", "context": {"cluster": "c1"}}], + "clusters": [{"name": "c1", "cluster": {"server": "https://x"}}], + }) + with patch("factory.contained.k8s.cli_binary", return_value="oc"), \ + patch("factory.contained.k8s._run", return_value=_completed(payload)): + contexts = k8s.list_contexts() + assert [c.context for c in contexts] == ["dev"] + assert contexts[0].server == "https://x" + + +def test_first_section_returns_empty_when_the_nested_value_is_not_a_dict() -> None: + # entries[0][inner] is present but not a dict — the 303->305 fall-through. + data = {"contexts": [{"context": "not-a-dict"}]} + assert k8s._first_section(data, "contexts", "context") == {} + # And when the list is empty. + assert k8s._first_section({"contexts": []}, "contexts", "context") == {} + + +def test_cluster_context_is_empty_when_no_cli_is_installed() -> None: + with patch("factory.contained.k8s.cli_binary", side_effect=k8s.ClusterError("no cli")): + assert k8s.cluster_context() == k8s.ClusterContext() + + +# -------------------------------------------------------------------------------------------- +# secret_keys — the failure directions the value-safe reader must not raise on +# -------------------------------------------------------------------------------------------- + + +def test_secret_keys_is_empty_when_no_cli_is_installed() -> None: + with patch("factory.contained.k8s.cli_binary", side_effect=k8s.ClusterError("no cli")): + assert k8s.secret_keys(k8s.SECRET_NAME, "ns") == set() + + +def test_secret_keys_is_empty_when_the_json_is_malformed_but_object_shaped() -> None: + # Starts with `{` so it passes the shape guard, then fails to parse — the 275-276 branch. + with patch("factory.contained.k8s.cli_binary", return_value="oc"), \ + patch("factory.contained.k8s._run", return_value=_completed("{not valid json")): + assert k8s.secret_keys(k8s.SECRET_NAME, "ns") == set() + + +# -------------------------------------------------------------------------------------------- +# use_context +# -------------------------------------------------------------------------------------------- + + +def test_use_context_reports_when_no_cli_is_installed() -> None: + with patch("factory.contained.k8s.cli_binary", side_effect=k8s.ClusterError("no cli")): + ok, detail = k8s.use_context("dev") + assert ok is False + assert "no cli" in detail + + +def test_use_context_reports_when_the_cli_could_not_be_run() -> None: + with patch("factory.contained.k8s.cli_binary", return_value="oc"), \ + patch("factory.contained.k8s._run", return_value=None): + ok, detail = k8s.use_context("dev") + assert ok is False + assert "config use-context dev" in detail + + +def test_use_context_succeeds_and_returns_the_output() -> None: + with patch("factory.contained.k8s.cli_binary", return_value="oc"), \ + patch("factory.contained.k8s._run", return_value=_completed('Switched to "dev".\n')): + ok, detail = k8s.use_context("dev") + assert ok is True + assert detail == 'Switched to "dev".' + + +def test_use_context_reports_the_failure_detail_or_a_placeholder() -> None: + with patch("factory.contained.k8s.cli_binary", return_value="oc"), \ + patch("factory.contained.k8s._run", + return_value=_completed("", 1, stderr="no context exists with the name")): + ok, detail = k8s.use_context("dev") + assert ok is False + assert "no context exists" in detail + # Non-zero but with no stderr at all falls back to the placeholder. + with patch("factory.contained.k8s.cli_binary", return_value="oc"), \ + patch("factory.contained.k8s._run", return_value=_completed("", 1)): + ok, detail = k8s.use_context("dev") + assert ok is False + assert detail == "no detail given" + + +# -------------------------------------------------------------------------------------------- +# has_cluster_context +# -------------------------------------------------------------------------------------------- + + +def test_has_cluster_context_reflects_whether_a_context_is_set() -> None: + with patch("factory.contained.k8s.cli_binary", return_value="oc"), \ + patch("factory.contained.k8s._run", return_value=_completed("dev\n")): + assert k8s.has_cluster_context() is True + with patch("factory.contained.k8s.cli_binary", return_value="oc"), \ + patch("factory.contained.k8s._run", return_value=None): + assert k8s.has_cluster_context() is False + + +# -------------------------------------------------------------------------------------------- +# resolve_namespace — the two distinct "no usable namespace" messages +# -------------------------------------------------------------------------------------------- + + +def test_resolve_namespace_blames_the_flag_only_when_a_flag_was_given() -> None: + with patch("factory.contained.k8s.current_namespace", return_value=None): + # An explicit-but-empty value points the finger at the flag, not the user. + with pytest.raises(k8s.ClusterError, match="--namespace was given"): + k8s.resolve_namespace("") + # No flag at all gets the "pass --namespace" guidance instead. + with pytest.raises(k8s.ClusterError, match="no namespace given"): + k8s.resolve_namespace(None) + + +def test_resolve_namespace_prefers_the_explicit_value() -> None: + assert k8s.resolve_namespace("mine") == "mine" + + +# -------------------------------------------------------------------------------------------- +# _run — the subprocess-failed swallow +# -------------------------------------------------------------------------------------------- + + +def test_run_returns_none_when_the_subprocess_cannot_be_launched() -> None: + with patch("factory.contained.k8s.subprocess.run", side_effect=FileNotFoundError): + assert k8s._run(["oc", "get", "pods"]) is None + with patch("factory.contained.k8s.subprocess.run", + side_effect=subprocess.TimeoutExpired(cmd="oc", timeout=1)): + assert k8s._run(["oc", "get", "pods"]) is None + + +def test_run_returns_the_completed_process_on_success() -> None: + with patch("factory.contained.k8s.subprocess.run", return_value=_completed("ok")): + result = k8s._run(["oc", "version"]) + assert result is not None and result.stdout == "ok" + + +# -------------------------------------------------------------------------------------------- +# Command composition — the pure argv builders +# -------------------------------------------------------------------------------------------- + + +def test_build_apply_argv_targets_stdin_in_the_namespace() -> None: + with patch("factory.contained.k8s.cli_binary", return_value="oc"): + assert k8s.build_apply_argv("ns") == ["oc", "apply", "-n", "ns", "-f", "-"] + + +def test_build_pod_exec_argv_uses_a_bare_i_without_a_tty() -> None: + with patch("factory.contained.k8s.cli_binary", return_value="oc"): + argv = k8s.build_pod_exec_argv("pod", "ns", ["ls"], tty=False) + assert "-i" in argv + assert "-t" not in argv + assert argv[-2:] == ["--", "ls"] + + +def test_render_access_review_carries_an_explicit_api_group() -> None: + review = json.loads( + k8s.render_access_review("create", "builds", "ns", group="build.openshift.io") + ) + assert review["spec"]["resourceAttributes"]["group"] == "build.openshift.io" + + +# -------------------------------------------------------------------------------------------- +# access_review — the "denied vs could-not-find-out" split +# -------------------------------------------------------------------------------------------- + + +def test_access_review_is_none_when_the_review_command_fails() -> None: + with patch("factory.contained.k8s.subprocess.run", + return_value=_completed("", 1, stderr="boom")): + assert k8s.access_review("create", "pods", "ns") is None + + +# -------------------------------------------------------------------------------------------- +# namespace_fs_group — reading the OpenShift supplemental-groups range +# -------------------------------------------------------------------------------------------- + + +def test_namespace_fs_group_parses_the_range_start() -> None: + with patch("factory.contained.k8s.cli_binary", return_value="oc"), \ + patch("factory.contained.k8s._run", return_value=_completed("1000700000/10000\n")): + assert k8s.namespace_fs_group("ns") == 1000700000 + + +def test_namespace_fs_group_is_none_when_the_annotation_is_absent_or_junk() -> None: + with patch("factory.contained.k8s.cli_binary", return_value="oc"), \ + patch("factory.contained.k8s._run", return_value=None): + assert k8s.namespace_fs_group("ns") is None + with patch("factory.contained.k8s.cli_binary", return_value="oc"), \ + patch("factory.contained.k8s._run", return_value=_completed("", 1)): + assert k8s.namespace_fs_group("ns") is None + # Present but not an integer — plain Kubernetes has no such annotation. + with patch("factory.contained.k8s.cli_binary", return_value="oc"), \ + patch("factory.contained.k8s._run", return_value=_completed("not-a-number\n")): + assert k8s.namespace_fs_group("ns") is None + + +# -------------------------------------------------------------------------------------------- +# apply_manifest +# -------------------------------------------------------------------------------------------- + + +def test_apply_manifest_succeeds_quietly() -> None: + with patch("factory.contained.k8s.cli_binary", return_value="oc"), \ + patch("factory.contained.k8s.subprocess.run", return_value=_completed("configured")): + k8s.apply_manifest("kind: Pod\n", "ns") # no raise + + +def test_apply_manifest_raises_when_the_cli_cannot_be_run() -> None: + with patch("factory.contained.k8s.cli_binary", return_value="oc"), \ + patch("factory.contained.k8s.subprocess.run", side_effect=FileNotFoundError("oc")): + with pytest.raises(k8s.ClusterError, match="applying the manifest failed"): + k8s.apply_manifest("kind: Pod\n", "ns") + + +def test_apply_manifest_raises_on_a_nonzero_exit() -> None: + with patch("factory.contained.k8s.cli_binary", return_value="oc"), \ + patch("factory.contained.k8s.subprocess.run", + return_value=_completed("", 1, stderr="forbidden")): + with pytest.raises(k8s.ClusterError, match="forbidden"): + k8s.apply_manifest("kind: Pod\n", "ns") + + +# -------------------------------------------------------------------------------------------- +# wait_for_container — the polled loop, driven instantly by a patched clock +# -------------------------------------------------------------------------------------------- + + +def _pod_json(*, name: str = "factory", state: dict | None = None, phase: str = "Pending") -> str: + status: dict = {"phase": phase} + if state is not None: + status["containerStatuses"] = [{"name": name, "state": state}] + return json.dumps({"status": status}) + + +def test_wait_for_container_returns_running_when_the_container_is_up() -> None: + with patch("factory.contained.k8s.cli_binary", return_value="oc"), \ + patch("factory.contained.k8s._run", + return_value=_completed(_pod_json(state={"running": {}}))), \ + patch("time.sleep"), patch("time.monotonic", side_effect=[0, 1]): + assert k8s.wait_for_container("p", "ns", "factory") == "running" + + +def test_wait_for_container_returns_terminated_on_a_clean_exit() -> None: + with patch("factory.contained.k8s.cli_binary", return_value="oc"), \ + patch("factory.contained.k8s._run", + return_value=_completed(_pod_json(state={"terminated": {"exitCode": 0}}))), \ + patch("time.sleep"), patch("time.monotonic", side_effect=[0, 1]): + assert k8s.wait_for_container("p", "ns", "factory") == "terminated" + + +def test_wait_for_container_tolerates_a_missing_read_and_junk_json_then_succeeds() -> None: + """A dropped read and a half-written document are both "keep polling", not failures.""" + running = _completed(_pod_json(state={"running": {}})) + with patch("factory.contained.k8s.cli_binary", return_value="oc"), \ + patch("factory.contained.k8s._run", + side_effect=[None, _completed("not json"), running]), \ + patch("time.sleep") as slept, \ + patch("time.monotonic", side_effect=[0, 1, 2, 3]): + assert k8s.wait_for_container("p", "ns", "factory", timeout=100) == "running" + assert slept.call_count == 2 # once for the dropped read, once for the junk + + +def test_wait_for_container_times_out_without_a_last_state() -> None: + """When the deadline is already past, there is no last state to report — the 886 falsy branch.""" + with patch("factory.contained.k8s.cli_binary", return_value="oc"), \ + patch("time.sleep"), patch("time.monotonic", side_effect=[0, 0]): + with pytest.raises(k8s.ClusterError) as raised: + k8s.wait_for_container("p", "ns", "factory", timeout=0) + assert "last state" not in str(raised.value) + + +# -------------------------------------------------------------------------------------------- +# stream_workspace / fetch_workspace — the tarball transport +# -------------------------------------------------------------------------------------------- + + +def test_stream_workspace_succeeds(tmp_path: Path) -> None: + tarball = tmp_path / "ws.tar.gz" + tarball.write_bytes(b"payload") + with patch("factory.contained.k8s.cli_binary", return_value="oc"), \ + patch("factory.contained.k8s.subprocess.run", return_value=_completed("done")): + k8s.stream_workspace(tarball, "pod", "ns") # no raise + + +def test_stream_workspace_raises_with_a_retry_hint_on_failure(tmp_path: Path) -> None: + tarball = tmp_path / "ws.tar.gz" + tarball.write_bytes(b"payload") + with patch("factory.contained.k8s.cli_binary", return_value="oc"), \ + patch("factory.contained.k8s.subprocess.run", + return_value=_completed("", 1, stderr="broken pipe")): + with pytest.raises(k8s.ClusterError, match="retrying is safe"): + k8s.stream_workspace(tarball, "pod", "ns") + + +def test_fetch_workspace_succeeds(tmp_path: Path) -> None: + destination = tmp_path / "out.tar.gz" + with patch("factory.contained.k8s.cli_binary", return_value="oc"), \ + patch("factory.contained.k8s.subprocess.run", + return_value=subprocess.CompletedProcess([], 0, b"", b"")): + k8s.fetch_workspace("pod", "ns", destination) # no raise + assert destination.exists() + + +def test_fetch_workspace_raises_on_failure(tmp_path: Path) -> None: + destination = tmp_path / "out.tar.gz" + # stderr is bytes here — the call runs without text=True — so the error path must .decode() it. + with patch("factory.contained.k8s.cli_binary", return_value="oc"), \ + patch("factory.contained.k8s.subprocess.run", + return_value=subprocess.CompletedProcess([], 1, b"", b"no such pod")): + with pytest.raises(k8s.ClusterError, match="no such pod"): + k8s.fetch_workspace("pod", "ns", destination) + + +# -------------------------------------------------------------------------------------------- +# _summarize +# -------------------------------------------------------------------------------------------- + + +def test_summarize_returns_the_last_meaningful_line() -> None: + stderr = "E0812 noise line\nUnhandled Error in something\nerror: the real problem\n" + assert k8s._summarize(stderr) == "the real problem" + + +def test_summarize_reports_a_placeholder_when_there_is_nothing_useful() -> None: + assert k8s._summarize("E0812 only noise\n\n") == "no details given" + + +# -------------------------------------------------------------------------------------------- +# cluster_runtimes — the error surface +# -------------------------------------------------------------------------------------------- + + +def test_cluster_runtimes_raises_lifecycle_error_when_no_namespace_resolves() -> None: + with patch("factory.contained.k8s.current_namespace", return_value=None): + with pytest.raises(LifecycleError): + k8s.cluster_runtimes(None) + + +def test_cluster_runtimes_raises_when_the_cluster_does_not_answer() -> None: + with patch("factory.contained.k8s.cli_binary", return_value="oc"), \ + patch("factory.contained.k8s._run", return_value=None): + with pytest.raises(LifecycleError, match="did not answer"): + k8s.cluster_runtimes("ns") + + +def test_cluster_runtimes_summarizes_an_unreachable_cluster() -> None: + with patch("factory.contained.k8s.cli_binary", return_value="oc"), \ + patch("factory.contained.k8s._run", + return_value=_completed("", 1, stderr="error: Unauthorized")): + with pytest.raises(LifecycleError, match="cannot reach the cluster"): + k8s.cluster_runtimes("ns") + + +def test_cluster_runtimes_raises_on_non_json_output() -> None: + with patch("factory.contained.k8s.cli_binary", return_value="oc"), \ + patch("factory.contained.k8s._run", return_value=_completed("not json")): + with pytest.raises(LifecycleError, match="isn't JSON"): + k8s.cluster_runtimes("ns") + + +def test_cluster_runtimes_tolerates_missing_and_malformed_timestamps() -> None: + payload = json.dumps({"items": [ + {"metadata": {"name": "a", "labels": {}}, "status": {"phase": "Running"}}, + {"metadata": {"name": "b", "creationTimestamp": "not-a-date"}, + "status": {"phase": "Pending"}}, + ]}) + with patch("factory.contained.k8s.cli_binary", return_value="oc"), \ + patch("factory.contained.k8s._run", return_value=_completed(payload)): + runtimes = k8s.cluster_runtimes("ns") + assert [r.name for r in runtimes] == ["a", "b"] + assert all(r.created is None for r in runtimes) + + +# -------------------------------------------------------------------------------------------- +# remove_cluster_runtime — the failure paths +# -------------------------------------------------------------------------------------------- + + +def test_remove_cluster_runtime_reports_a_failed_delete( + capsys: pytest.CaptureFixture[str], +) -> None: + # Sweep could not be run (None), so the sweep-report block is skipped; the delete then fails. + with patch("factory.contained.k8s.cli_binary", return_value="oc"), \ + patch("factory.contained.k8s._run", + side_effect=[None, _completed("", 1, stderr="forbidden")]): + code = k8s.remove_cluster_runtime("rta-test", namespace="ns") + assert code == 1 + assert "deleting pod rta-test failed" in capsys.readouterr().err + + +def test_remove_cluster_runtime_reports_when_the_delete_cli_could_not_run( + capsys: pytest.CaptureFixture[str], +) -> None: + with patch("factory.contained.k8s.cli_binary", return_value="oc"), \ + patch("factory.contained.k8s._run", side_effect=[None, None]): + code = k8s.remove_cluster_runtime("rta-test", namespace="ns") + assert code == 1 + assert "the CLI could not be run" in capsys.readouterr().err + + +# -------------------------------------------------------------------------------------------- +# sync_cluster_runtime +# -------------------------------------------------------------------------------------------- + + +def test_sync_cluster_runtime_fetches_and_reports_where_it_landed( + tmp_path: Path, capsys: pytest.CaptureFixture[str], +) -> None: + with patch("factory.contained.workspace.contained_home", return_value=tmp_path), \ + patch("factory.contained.k8s.fetch_workspace") as fetch: + code = k8s.sync_cluster_runtime("rta-test", namespace="ns") + assert code == 0 + fetch.assert_called_once() + out = capsys.readouterr().out + assert "workspace fetched to" in out + assert "Nothing is merged automatically." in out + + +def test_sync_cluster_runtime_reports_a_fetch_failure( + tmp_path: Path, capsys: pytest.CaptureFixture[str], +) -> None: + with patch("factory.contained.workspace.contained_home", return_value=tmp_path), \ + patch("factory.contained.k8s.fetch_workspace", + side_effect=k8s.ClusterError("no such pod")): + code = k8s.sync_cluster_runtime("rta-test", namespace="ns") + assert code == 1 + assert "no such pod" in capsys.readouterr().err diff --git a/tests/test_contained_k8s_credentials.py b/tests/test_contained_k8s_credentials.py new file mode 100644 index 000000000..4cea1b6a5 --- /dev/null +++ b/tests/test_contained_k8s_credentials.py @@ -0,0 +1,290 @@ +"""The guided credentials Secret: what it composes, what it refuses, and what it never discloses. + +The disclosure tests are the ones worth having. A wizard that collects an API key has exactly one +way to be dangerous, and it is not "the wrong key ends up in the Secret" — it is the key ending up +somewhere nobody was looking: an argv, a log line, a printed command, an error message. +""" + +from __future__ import annotations + +import json +import subprocess +from pathlib import Path +from unittest.mock import patch + +import pytest + +from factory.contained import k8s_credentials as creds +from factory.contained.k8s import ADC_SECRET_KEY, SECRET_NAME + +SECRET = "sk-ant-api03-averylongsecretvaluethatmustnotleak-9f2c" + + +def _completed(stdout: str = "", returncode: int = 0, stderr: str = ""): + return subprocess.CompletedProcess([], returncode, stdout, stderr) + + +class _Answers: + """A scripted stand-in for the three readers, so the flow can run without a terminal. + + `tests/conftest.py` forces raw reads off and pytest's stdin raises on `input()`, so a prompt + reached for real here would either block or abort. Injection is the only way in. + """ + + def __init__(self, *, selects: list[str], lines: list[str] | None = None, + secrets: list[str] | None = None) -> None: + self.selects = list(selects) + self.lines = list(lines or []) + self.secrets = list(secrets or []) + + def readers(self) -> creds._Readers: + return creds._Readers( + line=lambda question, default=None: self.lines.pop(0) if self.lines else default, + secret=lambda question: self.secrets.pop(0) if self.secrets else None, + select=lambda question, options: self.selects.pop(0) if self.selects else None, + ) + + +# -------------------------------------------------------------------------------------------- +# The manifest +# -------------------------------------------------------------------------------------------- + + +def test_the_manifest_is_json_so_no_value_can_break_it() -> None: + """A key with a colon, a quote or a newline in it is ordinary here and fatal to hand-built YAML.""" + hostile = 'a: "b"\nc: {d}\n%e\n\t--- ' + manifest = creds.build_secret_manifest("ns", {"ANTHROPIC_API_KEY": hostile}) + parsed = json.loads(manifest) + assert parsed["stringData"]["ANTHROPIC_API_KEY"] == hostile + assert parsed["kind"] == "Secret" + assert parsed["metadata"]["name"] == SECRET_NAME + assert parsed["metadata"]["namespace"] == "ns" + + +def test_the_manifest_uses_string_data_so_nothing_has_to_base64_by_hand() -> None: + parsed = json.loads(creds.build_secret_manifest("ns", {"ANTHROPIC_API_KEY": SECRET})) + assert "data" not in parsed + assert parsed["stringData"]["ANTHROPIC_API_KEY"] == SECRET + + +def test_the_material_never_reaches_an_argv() -> None: + """`--from-literal` puts a key in the process table and in the caller's shell history.""" + seen: dict[str, object] = {} + + def fake_run(argv, **kwargs): + seen["argv"] = argv + seen["stdin"] = kwargs.get("input", "") + return _completed("secret/factory-credentials created") + + with patch("factory.contained.k8s_credentials.subprocess.run", side_effect=fake_run): + ok, detail = creds.apply_secret("oc", "ns", {"ANTHROPIC_API_KEY": SECRET}) + + assert ok + assert SECRET not in " ".join(seen["argv"]) # type: ignore[arg-type] + assert not any("from-literal" in token for token in seen["argv"]) # type: ignore[union-attr] + assert SECRET in seen["stdin"] # type: ignore[operator] + + +def test_the_echoed_command_is_redacted() -> None: + line = creds.redacted_command("oc", "ns", {"ANTHROPIC_API_KEY": SECRET}) + assert SECRET not in line + assert "***" in line + + +def test_configuration_stays_readable_while_material_is_hidden() -> None: + """A user confirms they configured the right region; hiding that helps nobody.""" + line = creds.redacted_command("oc", "ns", { + "CLOUD_ML_REGION": "us-east5", + ADC_SECRET_KEY: '{"type": "authorized_user"}', + }) + assert "us-east5" in line + assert "authorized_user" not in line + + +def test_a_failure_message_cannot_carry_the_value_back_out() -> None: + """A parser quoting the input it choked on is exactly how a key reaches a terminal.""" + with patch("factory.contained.k8s_credentials.subprocess.run", + return_value=_completed(returncode=1, stderr=f"error validating {SECRET}")): + ok, detail = creds.apply_secret("oc", "ns", {"ANTHROPIC_API_KEY": SECRET}) + assert not ok + assert SECRET not in detail + assert "***" in detail + + +def test_nothing_but_names_and_lengths_is_logged() -> None: + recorded: list[dict] = [] + with patch("factory.contained.k8s_credentials.subprocess.run", + return_value=_completed("created")), \ + patch.object(creds.log, "info", lambda event, **kw: recorded.append(kw)): + creds.apply_secret("oc", "ns", {"ANTHROPIC_API_KEY": SECRET}) + assert recorded + assert SECRET not in json.dumps(recorded) + assert recorded[0]["keys"] == {"ANTHROPIC_API_KEY": len(SECRET)} + + +# -------------------------------------------------------------------------------------------- +# Describing a value without disclosing it +# -------------------------------------------------------------------------------------------- + + +def test_a_value_is_described_by_shape_not_by_content() -> None: + described = creds.describe_value(SECRET) + assert str(len(SECRET)) in described + assert SECRET not in described + # Enough to catch a paste that grabbed the quotes, and no more. + assert "sk-ant-a" in described + + +def test_a_short_value_gets_no_excerpt_at_all() -> None: + """An excerpt of a twelve-character secret is most of the secret.""" + described = creds.describe_value("short-key-1") + assert described == "11 characters" + + +# -------------------------------------------------------------------------------------------- +# The ADC file +# -------------------------------------------------------------------------------------------- + + +def test_an_authorized_user_document_is_accepted() -> None: + assert creds.validate_adc(json.dumps({ + "type": "authorized_user", "client_id": "a", "client_secret": "b", "refresh_token": "c", + })) is None + + +def test_a_service_account_key_is_accepted_too() -> None: + assert creds.validate_adc(json.dumps({ + "type": "service_account", "project_id": "p", "private_key": "k", "client_email": "e", + })) is None + + +def test_a_document_missing_a_required_field_names_the_field() -> None: + problem = creds.validate_adc(json.dumps({"type": "authorized_user", "client_id": "a"})) + assert problem is not None + assert "client_secret" in problem and "refresh_token" in problem + + +def test_a_non_json_file_is_refused_before_it_is_uploaded() -> None: + """Otherwise the failure surfaces inside an agent call and reads as a model outage.""" + problem = creds.validate_adc("not json at all") + assert problem is not None and "not valid JSON" in problem + + +def test_a_json_document_of_an_unknown_type_is_refused() -> None: + problem = creds.validate_adc(json.dumps({"type": "something_else"})) + assert problem is not None and "authorized_user" in problem + + +# -------------------------------------------------------------------------------------------- +# The step +# -------------------------------------------------------------------------------------------- + + +def test_an_existing_usable_secret_asks_nothing(capsys: pytest.CaptureFixture[str]) -> None: + payload = json.dumps({"ANTHROPIC_API_KEY": "x"}) + with patch("factory.contained.k8s_credentials._run", return_value=_completed(payload)): + # No readers supplied: reaching a prompt at all would raise here. + assert creds.run_credentials_step("oc", "ns", interactive=True) is True + assert "Nothing to do" in capsys.readouterr().out + + +def test_nobody_at_the_keyboard_means_no_credential_is_chosen( + capsys: pytest.CaptureFixture[str], +) -> None: + """`--yes` means "do not stop to ask me", not "pick something for me".""" + with patch("factory.contained.k8s_credentials._run", return_value=_completed(returncode=1)): + created = creds.run_credentials_step( + "oc", "ns", interactive=False, assume_yes=True, readers=creds._Readers() + ) + assert created is False + printed = capsys.readouterr().out + assert "skipped" in printed + assert "oc create secret generic" in printed + + +def test_the_anthropic_path_types_a_key_and_applies_it() -> None: + answers = _Answers(selects=["1", "t"], secrets=[SECRET]) + applied: dict[str, dict[str, str]] = {} + + def fake_apply(binary, namespace, data): + applied["data"] = data + return True, "secret/factory-credentials created" + + with patch("factory.contained.k8s_credentials._run", return_value=_completed(returncode=1)), \ + patch("factory.contained.k8s_credentials.apply_secret", side_effect=fake_apply), \ + patch("factory.contained.k8s_credentials.secret_exists", return_value=False), \ + patch("factory.contained.k8s_credentials.style.confirm", return_value=True), \ + patch("factory.contained.k8s_credentials.secret_check") as check: + check.side_effect = [ + creds.Check("credentials_secret", False, "missing", fix="..."), + creds.Check("credentials_secret", True, "present"), + ] + assert creds.run_credentials_step( + "oc", "ns", interactive=True, readers=answers.readers() + ) is True + assert applied["data"] == {"ANTHROPIC_API_KEY": SECRET} + + +def test_a_key_can_come_from_an_environment_variable_by_name(monkeypatch) -> None: + monkeypatch.setenv("MY_OWN_KEY", SECRET) + answers = _Answers(selects=["1", "e"], lines=["MY_OWN_KEY"]) + with patch("factory.contained.k8s_credentials.style.confirm", return_value=True): + data = creds._choose_backend(answers.readers()) + assert data == {"ANTHROPIC_API_KEY": SECRET} + + +def test_an_unset_environment_variable_reads_nothing_and_asks_again(monkeypatch) -> None: + """Naming the wrong variable is a slip, not a decision to stop.""" + monkeypatch.delenv("NOT_SET_ANYWHERE", raising=False) + monkeypatch.setenv("THE_RIGHT_ONE", SECRET) + answers = _Answers(selects=["1", "e", "e"], lines=["NOT_SET_ANYWHERE", "THE_RIGHT_ONE"]) + with patch("factory.contained.k8s_credentials.style.confirm", return_value=True): + data = creds._choose_backend(answers.readers()) + assert data == {"ANTHROPIC_API_KEY": SECRET} + + +def test_the_vertex_path_reads_the_credential_from_a_file(tmp_path: Path, monkeypatch) -> None: + adc = tmp_path / "adc.json" + document = json.dumps({ + "type": "authorized_user", "client_id": "a", "client_secret": "b", "refresh_token": "c", + }) + adc.write_text(document) + monkeypatch.delenv("CLOUD_ML_REGION", raising=False) + monkeypatch.delenv("ANTHROPIC_VERTEX_PROJECT_ID", raising=False) + answers = _Answers(selects=["2", "f"], lines=["us-east5", "my-project", str(adc)]) + with patch("factory.contained.k8s_credentials.style.confirm", return_value=True): + data = creds._choose_backend(answers.readers()) + assert data is not None + assert data["CLAUDE_CODE_USE_VERTEX"] == "1" + assert data["CLOUD_ML_REGION"] == "us-east5" + assert data["ANTHROPIC_VERTEX_PROJECT_ID"] == "my-project" + assert data[ADC_SECRET_KEY] == document + # The pinned setting the local target applies too; without it the cluster run behaves + # differently from the same run on this machine. + assert data["MAX_THINKING_TOKENS"] == "0" + + +def test_a_vertex_secret_built_here_satisfies_the_check_that_reads_it() -> None: + """The two halves must agree: a Secret this composes has to be one `secret_check` accepts.""" + monkey = { + "CLAUDE_CODE_USE_VERTEX": "1", "CLOUD_ML_REGION": "us-east5", + "ANTHROPIC_VERTEX_PROJECT_ID": "p", ADC_SECRET_KEY: "{}", + } + payload = json.dumps({key: "x" for key in monkey}) + with patch("factory.contained.k8s_credentials._run", return_value=_completed(payload)): + assert creds.secret_check("oc", "ns").ok + + +def test_backing_out_creates_nothing_and_prints_the_manual_route( + capsys: pytest.CaptureFixture[str], +) -> None: + answers = _Answers(selects=["s"]) + with patch("factory.contained.k8s_credentials._run", return_value=_completed(returncode=1)), \ + patch("factory.contained.k8s_credentials.secret_exists", return_value=False), \ + patch("factory.contained.k8s_credentials.apply_secret") as apply: + created = creds.run_credentials_step( + "oc", "ns", interactive=True, readers=answers.readers() + ) + assert created is False + apply.assert_not_called() + assert "oc create secret generic" in capsys.readouterr().out diff --git a/tests/test_contained_k8s_credentials_coverage.py b/tests/test_contained_k8s_credentials_coverage.py new file mode 100644 index 000000000..a9fef837d --- /dev/null +++ b/tests/test_contained_k8s_credentials_coverage.py @@ -0,0 +1,390 @@ +"""Exhaustive-branch coverage for the guided credentials Secret. + +`tests/test_contained_k8s_credentials.py` covers the disclosure guarantees — the tests worth having. +This file exists for a narrower, mechanical reason: every remaining statement and branch in +`factory.contained.k8s_credentials`. That means the shell-out failure arms, the "wrong source" +re-ask loops, each `validate_adc` verdict, and the whole `_copy_from_shell` shape reconstruction — +paths a real user hits rarely and a regression would hide in. + +The interactive surface is driven exactly as the sibling file drives it: injected `_Readers` and a +patched `style.confirm`, never a terminal. No real secret-looking value appears; dummy strings only. +""" + +from __future__ import annotations + +import json +import subprocess +from pathlib import Path +from unittest.mock import patch + +import pytest + +from factory.contained import k8s_credentials as creds +from factory.contained.credentials import CredentialShape +from factory.contained.k8s import ADC_SECRET_KEY, SECRET_NAME + +# Long enough to trip the excerpt floor and the redaction length guard, but obviously not a key. +DUMMY = "dummy-value-1234567890-abcdefgh" + + +def _completed(stdout: str = "", returncode: int = 0, stderr: str = ""): + return subprocess.CompletedProcess([], returncode, stdout, stderr) + + +class _Answers: + """A scripted stand-in for the three readers — copied from the sibling test file. + + `tests/conftest.py` forces raw reads off and pytest's stdin raises on `input()`, so a prompt + reached for real here would either block or abort. Injection is the only way in. + """ + + def __init__(self, *, selects: list[str], lines: list[str] | None = None, + secrets: list[str] | None = None) -> None: + self.selects = list(selects) + self.lines = list(lines or []) + self.secrets = list(secrets or []) + + def readers(self) -> creds._Readers: + return creds._Readers( + line=lambda question, default=None: self.lines.pop(0) if self.lines else default, + secret=lambda question: self.secrets.pop(0) if self.secrets else None, + select=lambda question, options: self.selects.pop(0) if self.selects else None, + ) + + +# -------------------------------------------------------------------------------------------- +# _run — the shell-out that swallows a missing binary +# -------------------------------------------------------------------------------------------- + + +def test_run_returns_none_when_the_binary_is_absent() -> None: + """A missing `oc` must read as "could not check", not crash the whole setup wizard.""" + with patch("factory.contained.k8s_credentials.subprocess.run", + side_effect=FileNotFoundError("no oc")): + assert creds._run(["oc", "get"]) is None + + +# -------------------------------------------------------------------------------------------- +# Reading what is there +# -------------------------------------------------------------------------------------------- + + +def test_a_secret_with_no_recognised_backend_is_reported_as_such() -> None: + """It exists, but nothing in it can authenticate — the check has to say which state that is.""" + payload = json.dumps({"SOMETHING_ELSE": "x", "ANOTHER": "y"}) + with patch("factory.contained.k8s_credentials._run", return_value=_completed(payload)): + check = creds.secret_check("oc", "ns") + assert check.ok is False + assert "none of the supported backends" in check.detail + assert "ANOTHER" in check.detail # the keys it does carry are named + + +def test_keys_of_treats_unparseable_and_non_object_json_as_empty() -> None: + assert creds._keys_of("not json at all") == set() # JSONDecodeError arm + assert creds._keys_of("[1, 2, 3]") == set() # valid JSON, but not a dict + assert creds._keys_of('{"A": 1}') == {"A"} + + +def test_secret_exists_mirrors_the_get_return_code() -> None: + with patch("factory.contained.k8s_credentials._run", return_value=_completed(returncode=0)): + assert creds.secret_exists("oc", "ns") is True + with patch("factory.contained.k8s_credentials._run", return_value=_completed(returncode=1)): + assert creds.secret_exists("oc", "ns") is False + with patch("factory.contained.k8s_credentials._run", return_value=None): + assert creds.secret_exists("oc", "ns") is False + + +# -------------------------------------------------------------------------------------------- +# Describing and redacting +# -------------------------------------------------------------------------------------------- + + +def test_a_long_value_is_described_with_both_ends() -> None: + described = creds.describe_value(DUMMY) + assert str(len(DUMMY)) in described + assert "starts" in described and "ends" in described + assert DUMMY not in described # only the sanctioned excerpts, not the whole thing + + +def test_redact_skips_empty_and_too_short_values() -> None: + """The length guard exists so a two-character value cannot turn every 'ab' into '***'.""" + text = "keep ab but scrub the-longer-value here" + scrubbed = creds.redact(text, ("", "ab", "the-longer-value")) + assert "ab but" in scrubbed # short value left alone (the `>= 4` false branch) + assert "the-longer-value" not in scrubbed + assert "***" in scrubbed + + +# -------------------------------------------------------------------------------------------- +# apply_secret — the exception arm +# -------------------------------------------------------------------------------------------- + + +def test_apply_secret_never_raises_and_redacts_the_exception() -> None: + """A subprocess failure is a thing to report; the value must not ride out on the message.""" + with patch("factory.contained.k8s_credentials.subprocess.run", + side_effect=OSError(f"boom near {DUMMY}")): + ok, detail = creds.apply_secret("oc", "ns", {"ANTHROPIC_API_KEY": DUMMY}) + assert ok is False + assert DUMMY not in detail + assert "OSError" in detail + + +# -------------------------------------------------------------------------------------------- +# _collect_field — the back-out arms +# -------------------------------------------------------------------------------------------- + + +def test_a_plain_field_backs_out_when_the_line_reader_returns_none() -> None: + field = creds.Field(key="CLOUD_ML_REGION", question="region") + readers = creds._Readers(line=lambda question, default=None: None) + assert creds._collect_field(field, readers) is None + + +def test_a_material_field_backs_out_when_the_source_menu_is_cancelled() -> None: + readers = _Answers(selects=["q"]).readers() + assert creds._collect_field(creds.ANTHROPIC_FIELDS[0], readers) is None + + +# -------------------------------------------------------------------------------------------- +# _read_from_source / _from_environment / _from_file +# -------------------------------------------------------------------------------------------- + + +def test_typing_a_json_field_prints_the_paste_hint_and_backs_out_on_empty( + capsys: pytest.CaptureFixture[str], +) -> None: + """The ADC field's `json_template` triggers the 'paste or choose the file' note before reading.""" + adc_field = creds.VERTEX_FIELDS[-1] + assert adc_field.json_template # guard: this is the field with the hint + readers = _Answers(selects=[], secrets=[""]).readers() # types nothing + assert creds._read_from_source("t", adc_field, readers) is None + assert "Paste the file's contents" in capsys.readouterr().out + + +def test_environment_source_backs_out_when_the_name_reader_returns_none() -> None: + field = creds.ANTHROPIC_FIELDS[0] + readers = creds._Readers(line=lambda question, default=None: None) + assert creds._from_environment(field, readers) is None + + +def test_environment_source_backs_out_when_no_name_is_given() -> None: + """A field with no suggested variable and a blank answer has nothing to read.""" + field = creds.Field(key="CUSTOM", question="custom", material=True) # default_env=() + readers = creds._Readers(line=lambda question, default=None: "") + assert creds._from_environment(field, readers) is None + + +def test_file_source_reads_an_unvalidated_file_for_a_plain_material_field( + tmp_path: Path, +) -> None: + """The API-key field has no `json_template` and no `validate_json` — both false branches.""" + blob = tmp_path / "key.txt" + blob.write_text(DUMMY) + readers = _Answers(selects=[], lines=[str(blob)]).readers() + with patch("factory.contained.k8s_credentials.style.confirm", return_value=True): + value = creds._from_file(creds.ANTHROPIC_FIELDS[0], readers) + assert value == DUMMY + + +def test_file_source_backs_out_when_the_path_reader_returns_none() -> None: + readers = creds._Readers(line=lambda question, default=None: None) + assert creds._from_file(creds.ANTHROPIC_FIELDS[0], readers) is None + + +def test_file_source_reports_an_unreadable_file( + tmp_path: Path, capsys: pytest.CaptureFixture[str], +) -> None: + missing = tmp_path / "does-not-exist.txt" + readers = _Answers(selects=[], lines=[str(missing)]).readers() + assert creds._from_file(creds.ANTHROPIC_FIELDS[0], readers) is None + assert "Could not read" in capsys.readouterr().out + + +def test_file_source_prints_the_template_and_refuses_an_invalid_adc( + tmp_path: Path, capsys: pytest.CaptureFixture[str], +) -> None: + """The ADC field prints its required-shape template, then rejects a bad document before upload.""" + bad = tmp_path / "adc.json" + bad.write_text(json.dumps({"type": "authorized_user"})) # missing fields + readers = _Answers(selects=[], lines=[str(bad)]).readers() + assert creds._from_file(creds.VERTEX_FIELDS[-1], readers) is None + out = capsys.readouterr().out + assert "This file must contain" in out # the template, shown before the question + assert "is not usable" in out # the validation refusal + + +# -------------------------------------------------------------------------------------------- +# validate_adc — every return arm +# -------------------------------------------------------------------------------------------- + + +def test_validate_adc_rejects_non_object_json() -> None: + problem = creds.validate_adc("123") + assert problem is not None and "not an object" in problem + + +# -------------------------------------------------------------------------------------------- +# _confirm_value — the non-material passthrough +# -------------------------------------------------------------------------------------------- + + +def test_confirm_value_returns_configuration_without_asking() -> None: + """A non-material value is configuration; it is returned as-is, no confirmation prompt.""" + field = creds.Field(key="CLOUD_ML_REGION", question="region") # material=False + assert creds._confirm_value(field, "us-east5", creds._Readers()) == "us-east5" + + +def test_confirm_value_honours_a_no_for_material() -> None: + field = creds.ANTHROPIC_FIELDS[0] + with patch("factory.contained.k8s_credentials.style.confirm", return_value=False): + assert creds._confirm_value(field, DUMMY, creds._Readers()) is None + + +# -------------------------------------------------------------------------------------------- +# run_credentials_step — the branches the happy-path tests skip +# -------------------------------------------------------------------------------------------- + + +def test_an_unrecognised_existing_secret_warns_that_it_will_be_replaced( + capsys: pytest.CaptureFixture[str], +) -> None: + """secret_check fails but the Secret object is there — the note has to say it gets replaced.""" + answers = _Answers(selects=["s"]) # then skip, to end the flow quickly + missing = creds.Check("credentials_secret", False, "wrong keys", fix="...") + with patch("factory.contained.k8s_credentials.secret_check", return_value=missing), \ + patch("factory.contained.k8s_credentials.secret_exists", return_value=True): + created = creds.run_credentials_step( + "oc", "ns", interactive=True, readers=answers.readers() + ) + assert created is False + assert "Continuing replaces it" in capsys.readouterr().out + + +def test_declining_the_final_confirm_creates_nothing( + monkeypatch, capsys: pytest.CaptureFixture[str], +) -> None: + """Backend chosen, then 'Create it now?' answered no — apply must not run.""" + monkeypatch.setenv("ANTHROPIC_API_KEY", DUMMY) + shape = CredentialShape(backend="anthropic", ok=True, detail="from key") + missing = creds.Check("credentials_secret", False, "missing", fix="...") + with patch("factory.contained.k8s_credentials.secret_check", return_value=missing), \ + patch("factory.contained.k8s_credentials.secret_exists", return_value=False), \ + patch("factory.contained.k8s_credentials.resolve_credentials", return_value=shape), \ + patch("factory.contained.k8s_credentials.style.confirm", return_value=False), \ + patch("factory.contained.k8s_credentials.apply_secret") as apply: + # select "3": copy from this shell, so no per-value confirm intervenes before the final one. + created = creds.run_credentials_step( + "oc", "ns", interactive=True, readers=_Answers(selects=["3"]).readers() + ) + assert created is False + apply.assert_not_called() + assert "Nothing was created" in capsys.readouterr().out + + +def test_a_failed_apply_is_reported_and_returns_false( + monkeypatch, capsys: pytest.CaptureFixture[str], +) -> None: + monkeypatch.setenv("ANTHROPIC_API_KEY", DUMMY) + shape = CredentialShape(backend="anthropic", ok=True, detail="from key") + missing = creds.Check("credentials_secret", False, "missing", fix="...") + with patch("factory.contained.k8s_credentials.secret_check", return_value=missing), \ + patch("factory.contained.k8s_credentials.secret_exists", return_value=False), \ + patch("factory.contained.k8s_credentials.resolve_credentials", return_value=shape), \ + patch("factory.contained.k8s_credentials.style.confirm", return_value=True), \ + patch("factory.contained.k8s_credentials.apply_secret", + return_value=(False, "forbidden")): + created = creds.run_credentials_step( + "oc", "ns", interactive=True, readers=_Answers(selects=["3"]).readers() + ) + assert created is False + assert "Could not create the Secret: forbidden" in capsys.readouterr().out + + +# -------------------------------------------------------------------------------------------- +# _choose_backend — the copy-from-shell pick and the collect back-out +# -------------------------------------------------------------------------------------------- + + +def test_choose_backend_offers_and_routes_copy_from_shell(monkeypatch) -> None: + """When the shell already resolves a backend, option 3 appears and dispatches to the copy.""" + monkeypatch.setenv("ANTHROPIC_API_KEY", DUMMY) + shape = CredentialShape(backend="anthropic", ok=True, detail="from key") + with patch("factory.contained.k8s_credentials.resolve_credentials", return_value=shape): + data = creds._choose_backend(_Answers(selects=["3"]).readers()) + assert data == {"ANTHROPIC_API_KEY": DUMMY} + + +def test_choose_backend_aborts_when_a_field_is_abandoned() -> None: + """Pick Anthropic, then cancel the key's source menu — the whole choice returns None.""" + answers = _Answers(selects=["1", "q"]) + data = creds._choose_backend(answers.readers()) + assert data is None + + +# -------------------------------------------------------------------------------------------- +# _copy_from_shell — every arm +# -------------------------------------------------------------------------------------------- + + +def test_copy_from_shell_anthropic_present_and_absent(monkeypatch) -> None: + monkeypatch.setenv("ANTHROPIC_API_KEY", DUMMY) + assert creds._copy_from_shell("anthropic") == {"ANTHROPIC_API_KEY": DUMMY} + monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False) + assert creds._copy_from_shell("anthropic") is None + + +def test_copy_from_shell_vertex_incomplete_config_asks_instead( + monkeypatch, capsys: pytest.CaptureFixture[str], +) -> None: + monkeypatch.setenv("CLAUDE_CODE_USE_VERTEX", "1") + monkeypatch.delenv("CLOUD_ML_REGION", raising=False) + monkeypatch.delenv("ANTHROPIC_VERTEX_PROJECT_ID", raising=False) + assert creds._copy_from_shell("vertex") is None + assert "configuration is incomplete" in capsys.readouterr().out + + +def _complete_vertex_env(monkeypatch) -> None: + monkeypatch.setenv("CLAUDE_CODE_USE_VERTEX", "1") + monkeypatch.setenv("CLOUD_ML_REGION", "us-east5") + monkeypatch.setenv("ANTHROPIC_VERTEX_PROJECT_ID", "my-project") + + +def test_copy_from_shell_vertex_unreadable_adc( + monkeypatch, tmp_path: Path, capsys: pytest.CaptureFixture[str], +) -> None: + _complete_vertex_env(monkeypatch) + monkeypatch.setattr(creds, "ADC_DIR", tmp_path) + monkeypatch.setattr(creds, "ADC_FILE", "absent.json") # never created + assert creds._copy_from_shell("vertex") is None + out = capsys.readouterr().out + assert "Could not read" in out and "application-default login" in out + + +def test_copy_from_shell_vertex_invalid_adc( + monkeypatch, tmp_path: Path, capsys: pytest.CaptureFixture[str], +) -> None: + _complete_vertex_env(monkeypatch) + (tmp_path / "adc.json").write_text("not json") + monkeypatch.setattr(creds, "ADC_DIR", tmp_path) + monkeypatch.setattr(creds, "ADC_FILE", "adc.json") + assert creds._copy_from_shell("vertex") is None + assert "is not usable" in capsys.readouterr().out + + +def test_copy_from_shell_vertex_ok_carries_config_credential_and_pinned_env( + monkeypatch, tmp_path: Path, +) -> None: + _complete_vertex_env(monkeypatch) + document = json.dumps({ + "type": "authorized_user", "client_id": "a", "client_secret": "b", "refresh_token": "c", + }) + (tmp_path / "adc.json").write_text(document) + monkeypatch.setattr(creds, "ADC_DIR", tmp_path) + monkeypatch.setattr(creds, "ADC_FILE", "adc.json") + data = creds._copy_from_shell("vertex") + assert data is not None + assert data["CLOUD_ML_REGION"] == "us-east5" + assert data[ADC_SECRET_KEY] == document + assert data["MAX_THINKING_TOKENS"] == "0" # the pinned Vertex setting merged in + assert SECRET_NAME # sanity: import stays used diff --git a/tests/test_contained_k8s_division.py b/tests/test_contained_k8s_division.py index 809e4b858..15043d46a 100644 --- a/tests/test_contained_k8s_division.py +++ b/tests/test_contained_k8s_division.py @@ -32,11 +32,17 @@ def _completed(stdout: str = "", returncode: int = 0) -> subprocess.CompletedPro def _no_cluster_round_trip(): """Building a pod plan must not phone a cluster. - `_build_pod_plan` reads the namespace's allocated `fsGroup` range, which is a live `oc get - namespace`. On a machine logged in to a slow or unreachable cluster that is a 30-second timeout - per test — the difference between this file taking one second and taking two minutes. + `_build_pod_plan` makes *two* live reads, and both have to be stubbed. The namespace's allocated + `fsGroup` range is one; `secret_keys`, which decides whether a Google credential file is + mounted, is the other. Whichever cluster the developer's kubeconfig happens to point at answers + them — and when that cluster is unreachable each call waits out its full timeout, which is the + difference between this file taking one second and hanging for minutes. + + Stubbing both is also what keeps a test run off whatever context is current. A test suite has no + business reaching a cluster at all, least of all one chosen by accident. """ - with patch("factory.cli.contained_k8s.namespace_fs_group", return_value=None): + with patch("factory.cli.contained_k8s.namespace_fs_group", return_value=None), \ + patch("factory.cli.contained_k8s.secret_keys", return_value=set()): yield diff --git a/tests/test_contained_k8s_helpers.py b/tests/test_contained_k8s_helpers.py index 57d3b0b47..3f1fd6ae7 100644 --- a/tests/test_contained_k8s_helpers.py +++ b/tests/test_contained_k8s_helpers.py @@ -10,7 +10,9 @@ import subprocess from unittest.mock import patch -from factory.contained import k8s_review, style +import pytest + +from factory.contained import k8s, k8s_review, style from factory.contained.k8s_division import openshift_available @@ -96,3 +98,314 @@ def test_a_diff_that_cannot_be_run_is_reported_as_unknown_not_as_current() -> No ): state = k8s_review._inspect_one(obj, "ns", "oc") assert state.status == k8s_review.UNKNOWN + + +# -------------------------------------------------------------------------------------------- +# Reading a pod's state well enough to stop waiting on a hopeless one +# -------------------------------------------------------------------------------------------- + + +def _pod(*, phase: str = "Pending", waiting: dict | None = None, running: bool = False, + terminated: dict | None = None, conditions: list | None = None, + name: str = "probe") -> dict: + state: dict = {} + if waiting is not None: + state["waiting"] = waiting + if running: + state["running"] = {"startedAt": "now"} + if terminated is not None: + state["terminated"] = terminated + status: dict = {"phase": phase} + if state: + status["containerStatuses"] = [{"name": name, "state": state}] + if conditions is not None: + status["conditions"] = conditions + return {"status": status} + + +def test_an_unpullable_image_is_doomed_immediately_not_after_the_timeout() -> None: + """The defect this exists for: three minutes of silence, then "the probe produced no output". + + `ImagePullBackOff` is the kubelet saying it has already retried and given up. Waiting past it + buys nothing, and the message it carries is the answer the user actually needs. + """ + progress = k8s.classify_pod(_pod(waiting={ + "reason": "ImagePullBackOff", + "message": 'Back-off pulling image "ghcr.io/akashgit/remote-factory/factory-runtime"', + })) + assert progress.verdict == k8s.DOOMED + assert progress.reason == "ImagePullBackOff" + assert "Back-off pulling image" in progress.describe() + + +def test_a_secret_missing_a_key_is_doomed_immediately() -> None: + progress = k8s.classify_pod(_pod(waiting={ + "reason": "CreateContainerConfigError", "message": "secret 'factory-credentials' not found", + })) + assert progress.verdict == k8s.DOOMED + assert "factory-credentials" in progress.describe() + + +def test_a_first_pull_is_not_mistaken_for_a_failure() -> None: + """A cold `ContainerCreating` legitimately runs for minutes; capping it would break every + first run on a fresh node.""" + progress = k8s.classify_pod(_pod(waiting={"reason": "ContainerCreating", "message": ""})) + assert progress.verdict == k8s.WAITING + assert progress.reason == "ContainerCreating" + + +def test_a_retryable_pull_error_is_not_doomed_on_sight() -> None: + """`ErrImagePull` is the attempt; `ImagePullBackOff` is the verdict. Only the second is final.""" + progress = k8s.classify_pod(_pod(waiting={"reason": "ErrImagePull", "message": "timeout"})) + assert progress.verdict == k8s.WAITING + assert progress.reason in k8s.RETRYABLE_WAITING_REASONS + + +def test_a_pod_no_node_will_accept_is_doomed_with_the_schedulers_words() -> None: + """It sits in Pending with no container status at all, which reads as "starting".""" + progress = k8s.classify_pod(_pod(conditions=[{ + "type": "PodScheduled", "status": "False", "reason": "Unschedulable", + "message": "0/6 nodes are available: insufficient memory", + }])) + assert progress.verdict == k8s.DOOMED + assert "insufficient memory" in progress.describe() + + +def test_a_running_container_is_running_and_a_clean_exit_succeeded() -> None: + assert k8s.classify_pod(_pod(running=True)).verdict == k8s.RUNNING + done = k8s.classify_pod(_pod(phase="Succeeded", terminated={"exitCode": 0, + "reason": "Completed"})) + assert done.verdict == k8s.SUCCEEDED + + +def test_a_nonzero_exit_is_doomed_and_carries_its_code() -> None: + progress = k8s.classify_pod(_pod(phase="Failed", terminated={"exitCode": 7, "reason": "Error"})) + assert progress.verdict == k8s.DOOMED + assert "7" in progress.describe() + + +def test_one_container_can_be_asked_about_by_name() -> None: + """The loader's window is "that initContainer is running", which no pod condition expresses.""" + pod = {"status": {"phase": "Pending", "initContainerStatuses": [ + {"name": "workspace-loader", "state": {"running": {}}}, + ], "containerStatuses": [ + {"name": "factory", "state": {"waiting": {"reason": "PodInitializing"}}}, + ]}} + assert k8s.classify_pod(pod, container="workspace-loader").verdict == k8s.RUNNING + assert k8s.classify_pod(pod, container="factory").verdict == k8s.WAITING + + +def test_an_unrecognized_state_waits_rather_than_giving_up() -> None: + """Being wrong in this direction aborts a run over a state that would have cleared.""" + progress = k8s.classify_pod(_pod(waiting={"reason": "SomethingNewInKubernetes"})) + assert progress.verdict == k8s.WAITING + + +def test_an_empty_or_malformed_pod_never_raises() -> None: + for payload in ({}, {"status": None}, {"status": {"containerStatuses": None}}): + assert k8s.classify_pod(payload).verdict in (k8s.WAITING, k8s.DOOMED) + + +def test_polling_stops_the_moment_a_pod_is_doomed() -> None: + """Not after the timeout: the first poll already knew, and the user waited three minutes.""" + doomed = _pod(waiting={"reason": "ImagePullBackOff", "message": "no such image"}) + with patch("factory.contained.k8s.read_pod", return_value=doomed), \ + patch("factory.contained.k8s.time.sleep") as slept: + progress = k8s.poll_pod("probe", "ns", timeout=180) + assert progress.verdict == k8s.DOOMED + slept.assert_not_called() + + +def test_polling_reports_each_change_once() -> None: + states = [ + _pod(waiting={"reason": "ContainerCreating"}), + _pod(waiting={"reason": "ContainerCreating"}), + _pod(running=True), + ] + seen: list[str] = [] + with patch("factory.contained.k8s.read_pod", side_effect=states), \ + patch("factory.contained.k8s.time.sleep"): + k8s.poll_pod("probe", "ns", timeout=180, on_progress=lambda p: seen.append(p.reason)) + assert seen == ["ContainerCreating", "Running"] + + +def test_a_wait_that_times_out_says_what_it_was_still_waiting_for() -> None: + with patch("factory.contained.k8s.read_pod", + return_value=_pod(waiting={"reason": "ContainerCreating"})), \ + patch("factory.contained.k8s.time.sleep"): + progress = k8s.poll_pod("probe", "ns", timeout=0) + assert progress.verdict == k8s.DOOMED + assert progress.reason == "Timeout" + + +def test_wait_for_container_names_the_reason_rather_than_reporting_a_timeout() -> None: + """It used to spend its full five minutes and then blame the clock.""" + with patch("factory.contained.k8s.read_pod", return_value=_pod( + waiting={"reason": "ImagePullBackOff", "message": "manifest unknown"}, name="factory")), \ + patch("factory.contained.k8s.time.sleep"), \ + patch("factory.contained.k8s.cli_binary", return_value="oc"): + with pytest.raises(k8s.ClusterError) as raised: + k8s.wait_for_container("pod", "ns", "factory", timeout=300) + assert "ImagePullBackOff" in str(raised.value) + assert "manifest unknown" in str(raised.value) + + +# -------------------------------------------------------------------------------------------- +# A failed read is not evidence of absence +# -------------------------------------------------------------------------------------------- + +_UNAUTHORIZED = ( + 'error: You must be logged in to the server (Unauthorized)\n' + 'couldn\'t get current server API group list: the server has asked for the client to ' + 'provide credentials' +) + + +def _obj(): + from factory.contained.bundle import BundleObject + + return BundleObject(kind="serviceaccount", name="factory", + purpose="the identity the pod runs as", manifest="kind: ServiceAccount\n") + + +def test_an_expired_login_is_not_reported_as_a_missing_object() -> None: + """The defect: a fully prepared namespace read as an empty one. + + Every `oc get` failed with Unauthorized, every failure was classified as "not there", and the + review offered to create five objects that already existed — directly contradicting the honest + "could not confirm whether the namespace exists" printed one line above. + """ + with patch("factory.contained.k8s_review._run", + return_value=subprocess.CompletedProcess([], 1, "", _UNAUTHORIZED)): + state = k8s_review._inspect_one(_obj(), "factory-yi", "oc") + assert state.status == k8s_review.UNKNOWN + assert "not logged in" in state.detail + + +def test_a_genuine_notfound_is_still_absent() -> None: + """The distinction has to cut both ways, or a first setup stops offering to create anything.""" + stderr = 'Error from server (NotFound): serviceaccounts "factory" not found' + with patch("factory.contained.k8s_review._run", + return_value=subprocess.CompletedProcess([], 1, "", stderr)): + state = k8s_review._inspect_one(_obj(), "factory-yi", "oc") + assert state.status == k8s_review.ABSENT + + +def test_any_other_read_failure_is_unknown_and_carries_its_reason() -> None: + stderr = "Error from server (Forbidden): serviceaccounts is forbidden" + with patch("factory.contained.k8s_review._run", + return_value=subprocess.CompletedProcess([], 1, "", stderr)): + state = k8s_review._inspect_one(_obj(), "factory-yi", "oc") + assert state.status == k8s_review.UNKNOWN + assert "Forbidden" in state.detail + + +def test_an_auth_error_is_recognized_however_the_cli_words_it() -> None: + for text in ( + "error: You must be logged in to the server (Unauthorized)", + "the server has asked for the client to provide credentials", + "Unauthorized", + "invalid bearer token", + ): + assert k8s.is_auth_error(text), text + assert not k8s.is_auth_error('serviceaccounts "factory" not found') + + +def test_a_login_check_asks_the_cluster_not_the_kubeconfig() -> None: + """`config current-context` reads a local file and passes with an hours-dead token.""" + with patch("factory.contained.k8s._run", + return_value=subprocess.CompletedProcess([], 1, "", _UNAUTHORIZED)): + ok, detail = k8s.login_status("oc") + assert ok is False + assert "logged in" in detail.lower() or "credentials" in detail.lower() + + with patch("factory.contained.k8s._run", + return_value=subprocess.CompletedProcess([], 0, "yizheng@redhat.com\n", "")): + ok, detail = k8s.login_status("oc") + assert ok is True and detail == "yizheng@redhat.com" + + +def test_the_login_probe_is_an_authenticated_round_trip() -> None: + seen = {} + + def fake_run(argv, **kw): + seen["argv"] = argv + return subprocess.CompletedProcess([], 0, "you", "") + + with patch("factory.contained.k8s._run", side_effect=fake_run): + k8s.login_status("oc") + assert "whoami" in seen["argv"] + with patch("factory.contained.k8s._run", side_effect=fake_run): + k8s.login_status("kubectl") + assert "auth" in seen["argv"] and "can-i" in seen["argv"] + + +# -------------------------------------------------------------------------------------------- +# classify_pod / _unschedulable / login_status / poll_pod — the residual edge branches +# -------------------------------------------------------------------------------------------- + + +def test_a_status_that_is_not_a_mapping_is_treated_as_still_waiting() -> None: + """A half-written pod document (`status` a string, not an object) must not raise in a poll.""" + assert k8s.classify_pod({"status": "corrupt"}).verdict == k8s.WAITING + + +def test_a_pod_level_succeeded_with_no_container_status_is_succeeded() -> None: + """The probe pod is gone by the time it Succeeds — only the pod phase remains to read.""" + assert k8s.classify_pod({"status": {"phase": "Succeeded"}}).verdict == k8s.SUCCEEDED + + +def test_a_pod_level_failed_with_no_container_status_is_doomed() -> None: + prog = k8s.classify_pod({"status": {"phase": "Failed", "reason": "Evicted", "message": "oom"}}) + assert prog.verdict == k8s.DOOMED + assert "oom" in prog.describe() + + +def test_a_container_state_matching_nothing_falls_through_to_the_pod_level() -> None: + """An empty container state is neither running, waiting nor terminated — keep waiting.""" + pod = {"status": {"phase": "Pending", "containerStatuses": [{"name": "c", "state": {}}]}} + assert k8s.classify_pod(pod).verdict == k8s.WAITING + + +def test_unschedulable_skips_a_non_dict_condition_and_reads_the_real_one() -> None: + pod = {"status": {"phase": "Pending", "conditions": [ + "not-a-dict", + {"type": "PodScheduled", "status": "False", "reason": "Unschedulable", "message": "no room"}, + ]}} + prog = k8s.classify_pod(pod) + assert prog.verdict == k8s.DOOMED and "no room" in prog.describe() + + +def test_a_non_matching_condition_leaves_the_pod_merely_waiting() -> None: + """A `Ready=False` condition is not `Unschedulable`; the pod is still just starting.""" + pod = {"status": {"phase": "Pending", "conditions": [{"type": "Ready", "status": "False"}]}} + assert k8s.classify_pod(pod).verdict == k8s.WAITING + + +def test_login_status_reports_authenticated_even_when_whoami_prints_nothing() -> None: + """A 0 exit with empty stdout is still success — the detail is just blank.""" + with patch("factory.contained.k8s._run", + return_value=subprocess.CompletedProcess([], 0, "", "")): + ok, detail = k8s.login_status("oc") + assert ok is True and detail == "" + + +def test_polling_gives_up_on_a_retryable_error_that_never_clears() -> None: + """`ErrImagePull` might be a blip; if it is still there after `stuck_after`, stop waiting.""" + stuck = _pod(waiting={"reason": "ErrImagePull", "message": "pull failed"}) + # A monotonic clock that advances past stuck_after between the two readings. Base is non-zero so + # the first timestamp stored in `error_since` is truthy (0.0 would re-trigger the `or`). + clock = iter([1000, 1000, 1000, 1000, 1035, 1035, 1035, 1035]) + with patch("factory.contained.k8s.read_pod", return_value=stuck), \ + patch("factory.contained.k8s.time.sleep"), \ + patch("factory.contained.k8s.time.monotonic", lambda: next(clock)): + prog = k8s.poll_pod("p", "ns", timeout=300, stuck_after=30) + assert prog.verdict == k8s.DOOMED + assert "unchanged for 30s" in prog.describe() + + +def test_login_status_when_the_cli_cannot_be_run_at_all() -> None: + """`_run` returns None when the binary is missing or the OS refuses — reported, not raised.""" + with patch("factory.contained.k8s._run", return_value=None): + ok, detail = k8s.login_status("oc") + assert ok is False and "could not be run" in detail diff --git a/tests/test_contained_k8s_review.py b/tests/test_contained_k8s_review.py index 0145c364c..e1bb54064 100644 --- a/tests/test_contained_k8s_review.py +++ b/tests/test_contained_k8s_review.py @@ -61,7 +61,14 @@ def test_every_object_explains_itself() -> None: def test_a_missing_object_is_absent_and_never_diffed() -> None: - with patch("factory.contained.k8s_review._run", return_value=_completed("", 1)) as run: + """`NotFound` on stderr is what makes it absent — not merely a non-zero exit. + + Any non-zero used to qualify, so an expired login turned a prepared namespace into an empty + one. The `get` has to actually say the object is not there. + """ + not_found = _completed("", 1) + not_found.stderr = 'Error from server (NotFound): serviceaccounts "factory" not found' + with patch("factory.contained.k8s_review._run", return_value=not_found) as run: states = inspect_objects([_obj()], "ns", "oc") assert states[0].status == ABSENT # One call: `get`. Diffing something that does not exist wastes a round trip per object. diff --git a/tests/test_contained_k8s_setup_coverage.py b/tests/test_contained_k8s_setup_coverage.py new file mode 100644 index 000000000..e3550376e --- /dev/null +++ b/tests/test_contained_k8s_setup_coverage.py @@ -0,0 +1,607 @@ +"""Coverage for the failure, degradation and interactive branches of `k8s_setup`. + +Written against the real branch source (the 4-step wizard with the login gate, the `poll_pod`-based +inference probe, and the credentials step). Everything that would reach a cluster or a prompt is +mocked at the module boundary — a test that reached either would hang, since `conftest` forces the +raw-terminal path off and there is no cluster to answer. +""" + +from __future__ import annotations + +import subprocess +from unittest.mock import patch + +import pytest + +from factory.contained import k8s_setup +from factory.contained.k8s import ( + DOOMED, + SUCCEEDED, + WAITING, + ClusterContext, + ClusterError, + PodProgress, +) +from factory.contained.prereq import Check + + +def _completed(stdout: str = "", returncode: int = 0, stderr: str = "") -> subprocess.CompletedProcess: + return subprocess.CompletedProcess([], returncode, stdout, stderr) + + +@pytest.fixture(autouse=True) +def _off_cluster(): + """Safe defaults so nothing reaches a cluster or a prompt; individual tests override as needed. + + Mirrors the intent of `test_contained_k8s.py`'s autouse fixtures, which this file does not + inherit. Also clears the process-global pinned context that `setup_k8s` sets, so one test's + choice does not leak into the next. + """ + k8s_setup.set_active_context(None) + with patch("factory.contained.k8s_setup.cli_binary", return_value="oc"), \ + patch("factory.contained.k8s_setup.list_contexts", return_value=[]), \ + patch("factory.contained.k8s_setup.cluster_context", return_value=ClusterContext()), \ + patch("factory.contained.k8s_setup.current_namespace", return_value=None), \ + patch("factory.contained.k8s_setup.access_review", return_value=True), \ + patch("factory.contained.k8s_setup.gitleaks_available", return_value=True), \ + patch("factory.contained.k8s_setup.resolve_image", return_value="img:latest"): + yield + k8s_setup.set_active_context(None) + + +# --------------------------------------------------------------------------------------------- +# _run +# --------------------------------------------------------------------------------------------- + + +def test_run_swallows_a_launch_failure() -> None: + with patch("factory.contained.k8s_setup.subprocess.run", side_effect=OSError("boom")): + assert k8s_setup._run(["oc", "version"]) is None + + +# --------------------------------------------------------------------------------------------- +# verify_k8s — the early returns and the division branch +# --------------------------------------------------------------------------------------------- + + +def test_verify_stops_when_no_cli_is_installed() -> None: + with patch("factory.contained.k8s_setup.cli_binary", + side_effect=ClusterError("neither oc nor kubectl")): + checks = k8s_setup.verify_k8s(namespace="ns") + assert [c.name for c in checks] == ["cluster_cli"] + assert not checks[0].ok + + +def test_verify_stops_when_the_login_has_expired() -> None: + """A bad credential must halt before the object checks turn Unauthorized into "missing".""" + with patch("factory.contained.k8s_setup._run", return_value=_completed("ctx")), \ + patch("factory.contained.k8s_setup.cluster_context", + return_value=ClusterContext(context="c", server="https://s")), \ + patch("factory.contained.k8s_setup.login_status", + return_value=(False, "Unauthorized")): + checks = k8s_setup.verify_k8s(namespace="ns") + assert [c.name for c in checks] == ["cluster_cli", "cluster_login"] + assert not checks[-1].ok + + +def test_verify_stops_when_the_namespace_cannot_be_resolved() -> None: + with patch("factory.contained.k8s_setup._run", return_value=_completed("ctx")), \ + patch("factory.contained.k8s_setup.cluster_context", + return_value=ClusterContext(context="c")), \ + patch("factory.contained.k8s_setup.login_status", return_value=(True, "me")), \ + patch("factory.contained.k8s_setup.resolve_namespace", + side_effect=ClusterError("no namespace")): + checks = k8s_setup.verify_k8s(namespace=None) + assert checks[-1].name == "namespace" and not checks[-1].ok + + +def test_verify_runs_the_division_check_when_asked() -> None: + with patch("factory.contained.k8s_setup._run", return_value=_completed("ok")), \ + patch("factory.contained.k8s_setup.cluster_context", + return_value=ClusterContext(context="c")), \ + patch("factory.contained.k8s_setup.login_status", return_value=(True, "me")), \ + patch("factory.contained.k8s_setup.resolve_namespace", return_value="ns"), \ + patch("factory.contained.k8s_setup.secret_check", + return_value=Check("credentials_secret", False, "missing", fix="x")), \ + patch("factory.contained.k8s_setup.build_api_resources_argv", return_value=["oc"]): + checks = k8s_setup.verify_k8s(namespace="ns", division=True, probe_inference=False) + assert any(c.name == "build_api" for c in checks) + + +# --------------------------------------------------------------------------------------------- +# _login_check +# --------------------------------------------------------------------------------------------- + + +def test_login_check_authenticated() -> None: + with patch("factory.contained.k8s_setup.login_status", return_value=(True, "me@example.com")): + check = k8s_setup._login_check("oc") + assert check.ok and "me@example.com" in check.detail + + +def test_login_check_expired_session_reads_as_a_login_problem() -> None: + with patch("factory.contained.k8s_setup.login_status", + return_value=(False, "You must be logged in (Unauthorized)")): + check = k8s_setup._login_check("oc") + assert not check.ok and "expired" in check.detail + + +def test_login_check_other_failure_reports_the_detail() -> None: + with patch("factory.contained.k8s_setup.login_status", + return_value=(False, "connection refused")): + check = k8s_setup._login_check("oc") + assert not check.ok and "connection refused" in check.detail + + +# --------------------------------------------------------------------------------------------- +# _object_checks — a failed read is not absence +# --------------------------------------------------------------------------------------------- + + +def test_object_check_present() -> None: + with patch("factory.contained.k8s_setup._run", return_value=_completed("serviceaccount/factory")): + checks = k8s_setup._object_checks("oc", "ns", division=False) + assert all(c.ok for c in checks) + + +def test_object_check_genuinely_missing_points_at_the_bundle() -> None: + not_found = _completed(returncode=1, stderr='Error (NotFound): serviceaccounts "x" not found') + with patch("factory.contained.k8s_setup._run", return_value=not_found): + checks = k8s_setup._object_checks("oc", "ns", division=False) + assert not checks[0].ok and "is missing" in checks[0].detail + assert "bundle |" in (checks[0].fix or "") + + +def test_object_check_auth_error_is_unknown_and_says_log_in() -> None: + with patch("factory.contained.k8s_setup._run", + return_value=_completed(returncode=1, stderr="error: Unauthorized")): + checks = k8s_setup._object_checks("oc", "ns", division=False) + assert not checks[0].ok and "could not be checked" in checks[0].detail + assert "login" in (checks[0].fix or "") + + +def test_object_check_other_error_carries_its_reason() -> None: + with patch("factory.contained.k8s_setup._run", + return_value=_completed(returncode=1, stderr="Error (Forbidden): nope")): + checks = k8s_setup._object_checks("oc", "ns", division=False) + assert not checks[0].ok and "Forbidden" in checks[0].detail + + +def test_object_check_unreadable_when_the_cli_could_not_run() -> None: + with patch("factory.contained.k8s_setup._run", return_value=None): + checks = k8s_setup._object_checks("oc", "ns", division=False) + assert not checks[0].ok and "could not be checked" in checks[0].detail + + +# --------------------------------------------------------------------------------------------- +# _inference_result and the probe +# --------------------------------------------------------------------------------------------- + + +def test_inference_result_skips_the_probe_without_a_secret() -> None: + secret = Check("credentials_secret", False, "missing", fix="make it") + check = k8s_setup._inference_result("oc", "ns", secret, announce=False) + assert not check.ok and "not attempted" in check.detail and check.fix == "make it" + + +def test_inference_result_announces_then_probes() -> None: + secret = Check("credentials_secret", True, "present") + with patch("factory.contained.k8s_setup._inference_check", + return_value=Check("inference_from_cluster", True, "reached")) as probe: + check = k8s_setup._inference_result("oc", "ns", secret, announce=True) + probe.assert_called_once() + assert check.ok + + +def _probe_run(**verdicts): + """A subprocess.run stand-in for the probe: delete/apply/logs keyed off argv.""" + def run(argv, **kw): + if "apply" in argv: + return _completed(verdicts.get("apply_out", "created"), + returncode=verdicts.get("apply_rc", 0), + stderr=verdicts.get("apply_err", "")) + if "logs" in argv: + return _completed(verdicts.get("logs", "")) + return _completed("") # delete, cleanup + return run + + +def test_probe_reports_success_when_the_pod_reaches_inference() -> None: + with patch("factory.contained.k8s_setup.subprocess.run", side_effect=_probe_run(logs="PROBE_OK")), \ + patch("factory.contained.k8s_setup.poll_pod", + return_value=PodProgress(SUCCEEDED, "Succeeded", "Succeeded", "")): + check = k8s_setup._inference_check("oc", "ns", "img") + assert check.ok + + +def test_probe_reports_a_pod_that_could_not_be_created() -> None: + with patch("factory.contained.k8s_setup.subprocess.run", + side_effect=_probe_run(apply_rc=1, apply_err="quota exceeded")): + check = k8s_setup._inference_check("oc", "ns", "img") + assert not check.ok and "could not be created" in check.detail + + +def test_probe_reports_a_doomed_pod_with_the_kubelet_reason() -> None: + with patch("factory.contained.k8s_setup.subprocess.run", side_effect=_probe_run(logs="")), \ + patch("factory.contained.k8s_setup.poll_pod", + return_value=PodProgress(DOOMED, "Pending", "ImagePullBackOff", "no such image")): + check = k8s_setup._inference_check("oc", "ns", "img") + assert not check.ok and "ImagePullBackOff" in check.detail + + +def test_probe_reports_the_pods_last_line_when_it_ran_but_failed() -> None: + with patch("factory.contained.k8s_setup.subprocess.run", + side_effect=_probe_run(logs="probing...\nno response — DNS")), \ + patch("factory.contained.k8s_setup.poll_pod", + return_value=PodProgress(SUCCEEDED, "Succeeded", "Succeeded", "")): + check = k8s_setup._inference_check("oc", "ns", "img") + assert not check.ok and "no response" in check.detail + + +def test_probe_reports_no_output_when_neither_logs_nor_a_doomed_reason() -> None: + with patch("factory.contained.k8s_setup.subprocess.run", side_effect=_probe_run(logs="")), \ + patch("factory.contained.k8s_setup.poll_pod", + return_value=PodProgress(WAITING, "P", "P", "")): + check = k8s_setup._inference_check("oc", "ns", "img") + assert not check.ok and "no output" in check.detail + + +def test_probe_survives_a_subprocess_failure() -> None: + """An error mid-probe is caught; the `finally` cleanup still runs, so it must not itself raise.""" + def run(argv, **kw): + if "apply" in argv: + raise OSError("no oc") + return _completed("") # delete before, and the finally cleanup after + + with patch("factory.contained.k8s_setup.subprocess.run", side_effect=run): + check = k8s_setup._inference_check("oc", "ns", "img") + assert not check.ok and "could not be run" in check.detail + + +def test_probe_manifest_names_the_pod_and_namespace() -> None: + manifest = k8s_setup._probe_pod_manifest("probe-x", "ns", "img:1") + assert "probe-x" in manifest and "ns" in manifest and "img:1" in manifest + + +# --------------------------------------------------------------------------------------------- +# _division_checks +# --------------------------------------------------------------------------------------------- + + +def test_division_present_and_absent() -> None: + with patch("factory.contained.k8s_setup._run", return_value=_completed("builds\n")): + assert k8s_setup._division_checks("ns")[0].ok + with patch("factory.contained.k8s_setup._run", return_value=_completed("")): + assert not k8s_setup._division_checks("ns")[0].ok + + +# --------------------------------------------------------------------------------------------- +# _apply_object +# --------------------------------------------------------------------------------------------- + + +def test_apply_object_success_failure_and_exception() -> None: + from factory.contained.bundle import BundleObject + + obj = BundleObject(kind="role", name="factory", purpose="p", manifest="kind: Role\n") + with patch("factory.contained.k8s_setup.subprocess.run", return_value=_completed("configured")): + assert k8s_setup._apply_object(obj, "ns", "oc") == (True, "configured") + with patch("factory.contained.k8s_setup.subprocess.run", + return_value=_completed(returncode=1, stderr="forbidden")): + ok, detail = k8s_setup._apply_object(obj, "ns", "oc") + assert not ok and "forbidden" in detail + with patch("factory.contained.k8s_setup.subprocess.run", side_effect=OSError("gone")): + ok, detail = k8s_setup._apply_object(obj, "ns", "oc") + assert not ok and "OSError" in detail + + +# --------------------------------------------------------------------------------------------- +# setup_k8s — the top-level flow +# --------------------------------------------------------------------------------------------- + + +def test_setup_returns_2_when_no_cli() -> None: + with patch("factory.contained.k8s_setup.cli_binary", side_effect=ClusterError("none")): + assert k8s_setup.setup_k8s(namespace="ns", division=False, interactive=True) == 2 + + +def test_setup_aborts_when_the_context_chooser_is_escaped() -> None: + with patch("factory.contained.k8s_setup._choose_context", return_value=k8s_setup._ABORT): + assert k8s_setup.setup_k8s(namespace="ns", division=False, interactive=True) == 1 + + +def test_setup_returns_2_when_the_namespace_lookup_errors() -> None: + with patch("factory.contained.k8s_setup._choose_context", return_value="ctx"), \ + patch("factory.contained.k8s_setup._choose_namespace", + side_effect=ClusterError("boom")): + assert k8s_setup.setup_k8s(namespace="ns", division=False, interactive=True) == 2 + + +def test_setup_aborts_when_no_namespace_is_chosen() -> None: + with patch("factory.contained.k8s_setup._choose_context", return_value=None), \ + patch("factory.contained.k8s_setup._choose_namespace", return_value=None): + assert k8s_setup.setup_k8s(namespace=None, division=False, interactive=True) == 1 + + +def test_setup_stops_when_not_logged_in() -> None: + with patch("factory.contained.k8s_setup._choose_context", return_value=None), \ + patch("factory.contained.k8s_setup._choose_namespace", return_value="ns"), \ + patch("factory.contained.k8s_setup.login_status", return_value=(False, "Unauthorized")): + assert k8s_setup.setup_k8s(namespace="ns", division=False, interactive=True) == 1 + + +def test_setup_non_interactive_without_yes_applies_nothing() -> None: + with patch("factory.contained.k8s_setup._choose_context", return_value=None), \ + patch("factory.contained.k8s_setup._choose_namespace", return_value="ns"), \ + patch("factory.contained.k8s_setup.login_status", return_value=(True, "me")), \ + patch("factory.contained.k8s_setup.inspect_objects", return_value=[]): + assert k8s_setup.setup_k8s(namespace="ns", division=False, interactive=False) == 1 + + +def _walk_result(*, failed=False, aborted=False): + class _R: + pass + r = _R() + r.failed, r.aborted = failed, aborted + return r + + +def test_setup_walk_aborted_names_the_verify_command() -> None: + with patch("factory.contained.k8s_setup._choose_context", return_value=None), \ + patch("factory.contained.k8s_setup._choose_namespace", return_value="ns"), \ + patch("factory.contained.k8s_setup.login_status", return_value=(True, "me")), \ + patch("factory.contained.k8s_setup.inspect_objects", return_value=[]), \ + patch("factory.contained.k8s_setup.walk", return_value=_walk_result(aborted=True)): + assert k8s_setup.setup_k8s(namespace="ns", division=False, interactive=True) == 1 + + +def test_setup_full_success_runs_credentials_then_verify() -> None: + ok = [Check("cluster_cli", True, "x")] + with patch("factory.contained.k8s_setup._choose_context", return_value=None), \ + patch("factory.contained.k8s_setup._choose_namespace", return_value="ns"), \ + patch("factory.contained.k8s_setup.login_status", return_value=(True, "me")), \ + patch("factory.contained.k8s_setup.inspect_objects", return_value=[]), \ + patch("factory.contained.k8s_setup.walk", + return_value=_walk_result(failed=True)) as walked, \ + patch("factory.contained.k8s_setup.run_credentials_step", return_value=True) as creds, \ + patch("factory.contained.k8s_setup.verify_k8s", return_value=ok): + code = k8s_setup.setup_k8s(namespace="ns", division=False, interactive=True) + walked.assert_called_once() + creds.assert_called_once() + assert code == 0 + + +# --------------------------------------------------------------------------------------------- +# _finish, and the default-context switch offer +# --------------------------------------------------------------------------------------------- + + +def test_finish_offers_the_default_switch_when_a_context_was_pinned() -> None: + k8s_setup.set_active_context("prepared-ctx") + with patch("factory.contained.k8s_setup.run_credentials_step", return_value=True), \ + patch("factory.contained.k8s_setup.verify_k8s", + return_value=[Check("x", True, "ok")]), \ + patch("factory.contained.k8s_setup._offer_default_switch") as offer: + code = k8s_setup._finish("oc", "ns", division=False, interactive=True) + offer.assert_called_once() + assert code == 0 + + +# --------------------------------------------------------------------------------------------- +# _choose_context / _ask_context +# --------------------------------------------------------------------------------------------- + + +def test_choose_context_returns_none_with_fewer_than_two() -> None: + with patch("factory.contained.k8s_setup.list_contexts", + return_value=[ClusterContext(context="only")]): + assert k8s_setup._choose_context(interactive=True) is None + + +def test_ask_context_accepts_a_number_a_name_and_reprompts_on_junk() -> None: + ctxs = [ClusterContext(context="a", server="s1"), ClusterContext(context="b", server="s2")] + with patch("factory.contained.k8s_setup.list_contexts", return_value=ctxs), \ + patch("factory.contained.k8s_setup.cluster_context", + return_value=ClusterContext(context="a")), \ + patch("factory.contained.k8s_setup.style.read_line", side_effect=["9", "junk", "b"]): + assert k8s_setup._choose_context(interactive=True) == "b" + + +def test_ask_context_aborts_on_escape() -> None: + ctxs = [ClusterContext(context="a"), ClusterContext(context="b")] + with patch("factory.contained.k8s_setup.list_contexts", return_value=ctxs), \ + patch("factory.contained.k8s_setup.style.read_line", return_value=None): + assert k8s_setup._choose_context(interactive=True) is k8s_setup._ABORT + + +# --------------------------------------------------------------------------------------------- +# _offer_default_switch +# --------------------------------------------------------------------------------------------- + + +def test_offer_switch_is_a_noop_when_already_current() -> None: + with patch("factory.contained.k8s_setup.cluster_context", + return_value=ClusterContext(context="c")): + k8s_setup._offer_default_switch("c", interactive=True) # returns early, nothing raised + + +def test_offer_switch_non_interactive_just_prints_the_command() -> None: + with patch("factory.contained.k8s_setup.cluster_context", + return_value=ClusterContext(context="other")): + k8s_setup._offer_default_switch("c", interactive=False) + + +def test_offer_switch_declined() -> None: + with patch("factory.contained.k8s_setup.cluster_context", + return_value=ClusterContext(context="other")), \ + patch("factory.contained.k8s_setup.style.confirm", return_value=False): + k8s_setup._offer_default_switch("c", interactive=True) + + +def test_offer_switch_accepted_success_and_failure() -> None: + with patch("factory.contained.k8s_setup.cluster_context", + return_value=ClusterContext(context="other")), \ + patch("factory.contained.k8s_setup.style.confirm", return_value=True), \ + patch("factory.contained.k8s_setup.use_context", return_value=(True, "now c")): + k8s_setup._offer_default_switch("c", interactive=True) + with patch("factory.contained.k8s_setup.cluster_context", + return_value=ClusterContext(context="other")), \ + patch("factory.contained.k8s_setup.style.confirm", return_value=True), \ + patch("factory.contained.k8s_setup.use_context", return_value=(False, "denied")): + k8s_setup._offer_default_switch("c", interactive=True) + + +# --------------------------------------------------------------------------------------------- +# _print_context field variants +# --------------------------------------------------------------------------------------------- + + +def test_print_context_full_and_empty() -> None: + with patch("factory.contained.k8s_setup.cluster_context", + return_value=ClusterContext(context="c", server="s", user="u")): + k8s_setup._print_context("ns") + with patch("factory.contained.k8s_setup.cluster_context", return_value=ClusterContext()): + k8s_setup._print_context(None) + + +# --------------------------------------------------------------------------------------------- +# _namespace_status / _create_namespace / _resolve_existing / _choose_namespace +# --------------------------------------------------------------------------------------------- + + +def test_namespace_status_present_absent_unreadable() -> None: + with patch("factory.contained.k8s_setup._run", return_value=_completed("namespace/ns")): + assert k8s_setup._namespace_status("ns", "oc") == k8s_setup.PRESENT + with patch("factory.contained.k8s_setup._run", return_value=None): + assert k8s_setup._namespace_status("ns", "kubectl") == k8s_setup.UNREADABLE + with patch("factory.contained.k8s_setup._run", + return_value=_completed(returncode=1, stderr="Error (NotFound): not found")): + assert k8s_setup._namespace_status("ns", "kubectl") == k8s_setup.ABSENT + # oc falls back to `get project`; a Forbidden on both is unreadable, not absent. + with patch("factory.contained.k8s_setup._run", + return_value=_completed(returncode=1, stderr="Forbidden")): + assert k8s_setup._namespace_status("ns", "oc") == k8s_setup.UNREADABLE + + +def test_create_namespace_success_failure_and_unrunnable() -> None: + with patch("factory.contained.k8s_setup._run", return_value=_completed("created")): + assert k8s_setup._create_namespace("ns", "oc")[0] is True + with patch("factory.contained.k8s_setup._run", return_value=None): + ok, detail = k8s_setup._create_namespace("ns", "kubectl") + assert not ok and "could not run" in detail + with patch("factory.contained.k8s_setup._run", + return_value=_completed(returncode=1, stderr="denied")): + ok, detail = k8s_setup._create_namespace("ns", "oc") + assert not ok and "denied" in detail + + +def test_resolve_existing_present_and_unreadable_are_ok() -> None: + with patch("factory.contained.k8s_setup._namespace_status", return_value=k8s_setup.PRESENT): + assert k8s_setup._resolve_existing("ns", "oc", interactive=True, assume_yes=False) == "ok" + with patch("factory.contained.k8s_setup._namespace_status", return_value=k8s_setup.UNREADABLE): + assert k8s_setup._resolve_existing("ns", "oc", interactive=True, assume_yes=False) == "ok" + + +def test_resolve_existing_absent_non_interactive_aborts() -> None: + with patch("factory.contained.k8s_setup._namespace_status", return_value=k8s_setup.ABSENT): + assert k8s_setup._resolve_existing( + "ns", "oc", interactive=False, assume_yes=False) == "abort" + + +def test_resolve_existing_absent_declined_retries_and_escaped_aborts() -> None: + with patch("factory.contained.k8s_setup._namespace_status", return_value=k8s_setup.ABSENT), \ + patch("factory.contained.k8s_setup.style.confirm", return_value=False): + assert k8s_setup._resolve_existing( + "ns", "oc", interactive=True, assume_yes=False) == "retry" + with patch("factory.contained.k8s_setup._namespace_status", return_value=k8s_setup.ABSENT), \ + patch("factory.contained.k8s_setup.style.confirm", return_value=None): + assert k8s_setup._resolve_existing( + "ns", "oc", interactive=True, assume_yes=False) == "abort" + + +def test_resolve_existing_absent_creates_when_confirmed() -> None: + with patch("factory.contained.k8s_setup._namespace_status", return_value=k8s_setup.ABSENT), \ + patch("factory.contained.k8s_setup._create_namespace", return_value=(True, "made")): + assert k8s_setup._resolve_existing( + "ns", "oc", interactive=True, assume_yes=True) == "ok" + + +def test_resolve_existing_create_failure_aborts_or_retries() -> None: + with patch("factory.contained.k8s_setup._namespace_status", return_value=k8s_setup.ABSENT), \ + patch("factory.contained.k8s_setup._create_namespace", return_value=(False, "denied")): + assert k8s_setup._resolve_existing( + "ns", "oc", interactive=False, assume_yes=True) == "abort" + assert k8s_setup._resolve_existing( + "ns", "oc", interactive=True, assume_yes=True) == "retry" + + +def test_choose_namespace_explicit_ok_and_not_ok() -> None: + with patch("factory.contained.k8s_setup._resolve_existing", return_value="ok"): + assert k8s_setup._choose_namespace( + "ns", interactive=True, binary="oc") == "ns" + with patch("factory.contained.k8s_setup._resolve_existing", return_value="abort"): + assert k8s_setup._choose_namespace( + "ns", interactive=True, binary="oc") is None + + +def test_choose_namespace_non_interactive_uses_the_current_context() -> None: + with patch("factory.contained.k8s_setup.resolve_namespace", return_value="ns"), \ + patch("factory.contained.k8s_setup._resolve_existing", return_value="ok"): + assert k8s_setup._choose_namespace( + None, interactive=False, binary="oc") == "ns" + + +def test_choose_namespace_interactive_reprompts_on_empty_then_accepts() -> None: + with patch("factory.contained.k8s_setup.current_namespace", return_value=None), \ + patch("factory.contained.k8s_setup.style.read_line", side_effect=["", "ns"]), \ + patch("factory.contained.k8s_setup._resolve_existing", return_value="ok"): + assert k8s_setup._choose_namespace( + None, interactive=True, binary="oc") == "ns" + + +def test_choose_namespace_interactive_escape_and_abort() -> None: + with patch("factory.contained.k8s_setup.current_namespace", return_value="cur"), \ + patch("factory.contained.k8s_setup.style.read_line", return_value=None): + assert k8s_setup._choose_namespace(None, interactive=True, binary="oc") is None + with patch("factory.contained.k8s_setup.current_namespace", return_value="cur"), \ + patch("factory.contained.k8s_setup.style.read_line", return_value="ns"), \ + patch("factory.contained.k8s_setup._resolve_existing", return_value="abort"): + assert k8s_setup._choose_namespace(None, interactive=True, binary="oc") is None + + +# --------------------------------------------------------------------------------------------- +# _verb_checks — unknown and denied +# --------------------------------------------------------------------------------------------- + + +def test_verb_checks_unknown_when_the_review_cannot_run() -> None: + with patch("factory.contained.k8s_setup.access_review", return_value=None): + checks = k8s_setup._verb_checks("ns", division=False) + perms = next(c for c in checks if c.name == "permissions") + assert not perms.ok and "unknown" in perms.detail + + +def test_verb_checks_names_the_denied_verbs() -> None: + with patch("factory.contained.k8s_setup.access_review", return_value=False): + checks = k8s_setup._verb_checks("ns", division=False) + perms = next(c for c in checks if c.name == "permissions") + assert not perms.ok and "cannot" in perms.detail + + +def test_probe_updates_the_status_line_when_given_one() -> None: + """With an Activity attached, `say()` drives it; without one it is a no-op (other tests).""" + import io + + act = k8s_setup.style.Activity("probe", stream=io.StringIO(), threshold=0.0) + with patch("factory.contained.k8s_setup.subprocess.run", side_effect=_probe_run(logs="PROBE_OK")), \ + patch("factory.contained.k8s_setup.poll_pod", + return_value=PodProgress(SUCCEEDED, "Succeeded", "Succeeded", "")): + check = k8s_setup._inference_check("oc", "ns", "img", act=act) + assert check.ok + + +def test_resolve_existing_create_on_plain_kubernetes_skips_the_oc_note() -> None: + with patch("factory.contained.k8s_setup._namespace_status", return_value=k8s_setup.ABSENT), \ + patch("factory.contained.k8s_setup._create_namespace", return_value=(True, "made")): + assert k8s_setup._resolve_existing( + "ns", "kubectl", interactive=True, assume_yes=True) == "ok" diff --git a/tests/test_contained_style.py b/tests/test_contained_style.py index cb05b2ffc..f237102d8 100644 --- a/tests/test_contained_style.py +++ b/tests/test_contained_style.py @@ -7,8 +7,11 @@ from __future__ import annotations import io +import time from unittest.mock import patch +import pytest + from factory.contained import style ESC = "\033" @@ -159,3 +162,119 @@ def test_confirm_reads_a_single_keypress() -> None: with patch("factory.contained.style.read_key", return_value="y"), \ patch("builtins.input", side_effect=AssertionError("must not need Enter")): assert style.confirm("Create it?") is True + + +def test_select_returns_the_key_that_was_pressed() -> None: + with patch("factory.contained.style.read_key", return_value="2"): + assert style.select("Which?", [("1", "local"), ("2", "k8s")]) == "2" + + +def test_select_backs_out_on_escape() -> None: + with patch("factory.contained.style.read_key", return_value=style.ESCAPE): + assert style.select("Which?", [("1", "local"), ("2", "k8s")]) is None + + +def test_select_spells_every_option_out() -> None: + """A menu rendered as `[1/2]` makes the reader hold the mapping in their head.""" + tty = _Tty() + with patch("factory.contained.style.read_key", return_value="1"): + style.select("Which?", [("1", "a podman container"), ("2", "a cluster pod")], stream=tty) + printed = tty.getvalue() + assert "a podman container" in printed and "a cluster pod" in printed + + +# --------------------------------------------------------------------------------------------- +# Saying what a slow step is waiting for +# --------------------------------------------------------------------------------------------- + + +def test_a_fast_operation_draws_nothing_at_all() -> None: + """Below the threshold the output is byte-for-byte what it was before this existed.""" + tty = _Tty() + with style.activity("check", "working", stream=tty) as act: + act.update("still working") + assert tty.getvalue() == "" + + +def test_a_slow_operation_redraws_in_place_and_erases_itself() -> None: + tty = _Tty() + with style.activity("probe", "creating the pod", stream=tty, threshold=0.01) as act: + time.sleep(0.15) + act.update("waiting for the pod") + time.sleep(0.15) + printed = tty.getvalue() + assert printed.count("\r") > 1 # redrawn, not appended + assert "probe" in printed and "waiting for the pod" in printed + # Erased on the way out, so the caller's result line lands where the spinner was. + assert printed.endswith("\r\033[2K") + + +def test_a_pipe_gets_plain_lines_rather_than_carriage_returns() -> None: + """A CI log needs the progress and must not receive a thousand half-drawn frames.""" + plain = io.StringIO() + with style.activity("probe", "creating the pod", stream=plain, threshold=0.01) as act: + time.sleep(0.05) + act.update("waiting for the pod") + act.update("waiting for the pod") # unchanged: says nothing twice + act.update("reading its output") + printed = plain.getvalue() + assert "\r" not in printed + assert printed.count("waiting for the pod") == 1 + assert "reading its output" in printed + + +def test_progress_can_be_turned_off_entirely() -> None: + tty = _Tty() + with patch.dict("os.environ", {"FACTORY_NO_PROGRESS": "1"}, clear=True): + with style.activity("probe", "creating", stream=tty, threshold=0.01) as act: + time.sleep(0.05) + act.update("waiting") + assert "\r" not in tty.getvalue() + + +def test_a_dumb_terminal_is_never_rewritten() -> None: + tty = _Tty() + with patch.dict("os.environ", {"TERM": "dumb"}, clear=True): + assert style.can_rewrite(tty) is False + + +def test_force_color_does_not_authorize_redrawing_a_log_file() -> None: + """Colour and motion are different questions; conflating them fills CI logs with fragments.""" + with patch.dict("os.environ", {"FORCE_COLOR": "1"}, clear=True): + assert style.can_rewrite(io.StringIO()) is False + + +def test_the_status_line_is_erased_even_when_the_operation_raises() -> None: + tty = _Tty() + with pytest.raises(RuntimeError): + with style.activity("probe", "creating", stream=tty, threshold=0.01): + time.sleep(0.25) # long enough for at least one frame to be drawn + raise RuntimeError("boom") + printed = tty.getvalue() + assert "probe" in printed # it did draw + assert printed.endswith("\r\033[2K") # and cleaned up on the way out + + +# --------------------------------------------------------------------------------------------- +# Reading a value that must not appear on screen +# --------------------------------------------------------------------------------------------- + + +def test_read_secret_falls_back_to_getpass_where_raw_reading_is_impossible() -> None: + with patch("factory.contained.style._raw_session", return_value=None), \ + patch("getpass.getpass", return_value=" sk-ant-secret "): + assert style.read_secret("API key") == "sk-ant-secret" + + +def test_read_secret_cancels_rather_than_returning_an_empty_string() -> None: + """"Nothing was entered" and "the user backed out" call for different behaviour.""" + for failure in (EOFError, OSError): + with patch("factory.contained.style._raw_session", return_value=None), \ + patch("getpass.getpass", side_effect=failure): + assert style.read_secret("API key") is None + + +def test_read_secret_recognizes_escape_from_the_fallback_path() -> None: + with patch("factory.contained.style._raw_session", return_value=None), \ + patch("getpass.getpass", return_value="\x1b"): + assert style.read_secret("API key") is None diff --git a/tests/test_contained_style_raw.py b/tests/test_contained_style_raw.py new file mode 100644 index 000000000..37d40f6e1 --- /dev/null +++ b/tests/test_contained_style_raw.py @@ -0,0 +1,433 @@ +"""The raw-terminal half of `style`, and the display branches the fallback tests never reach. + +`tests/conftest.py` forces `style._raw_session` to return None so no test blocks on a keypress — +which is exactly why the raw paths (`read_key`, `read_line`, `read_secret`, `_edit_line`, and the +keypress arms of `confirm`/`select`) go uncovered. Here we go the other way on purpose: a real pty, +with `_raw_session` restored to the genuine implementation for the duration, so the raw code runs +against a real terminal. + +The one trap worth naming: `sys.stdin` must read a byte straight from the fd (`os.read`), NOT through +a buffered text wrapper. `read_key`'s escape-sequence drain uses `select` on the fd, and a text +wrapper that has already pulled the `[` of an arrow key into its own buffer leaves `select` seeing +nothing on the fd — so an arrow reads as a bare Escape and the drain path never runs. `_RawStdin` +below reads one byte at a time from the kernel, which keeps `select` honest. +""" + +from __future__ import annotations + +import io +import os +import time +from unittest.mock import patch + + +from factory.contained import style + +# Captured at import, BEFORE conftest's autouse fixture replaces the attribute with a None-returning +# stub. This is the genuine function, so the tests below can exercise it and the callers that use it. +_REAL_RAW_SESSION = style._raw_session + + +class _TtyBuf(io.StringIO): + """A capture buffer that claims to be a terminal, for the `target`/echo side of a raw read.""" + + def isatty(self) -> bool: + return True + + +class _RawStdin: + """A stdin backed directly by a pty slave fd — one unbuffered byte per `read`, so `select` works.""" + + def __init__(self, fd: int, *, read_error: bool = False) -> None: + self._fd = fd + self._read_error = read_error + + def isatty(self) -> bool: + return os.isatty(self._fd) + + def fileno(self) -> int: + return self._fd + + def read(self, n: int = 1) -> str: + if self._read_error: + raise OSError("stdin read failed") + return os.read(self._fd, n).decode() + + +class _pty: + """A pty pair with `sys.stdin` pointed at the slave and the real `_raw_session` restored. + + Enter, get `(master_fd, target)`; write keystrokes with `feed()`; the raw `style` functions then + behave as if a person were typing. Everything is torn down on exit. + """ + + def __init__(self, *, read_error: bool = False) -> None: + self._read_error = read_error + + def __enter__(self) -> tuple[int, _TtyBuf]: + import pty + import termios + import tty + + self._master, self._slave = pty.openpty() + # Put the slave in cbreak up front. Bytes fed while the terminal is still in its default + # canonical mode sit in a line buffer that `read(1)` cannot reach until a newline arrives — + # which is a hang, since the code under test reads before we ever send one. cbreak makes each + # byte immediately readable, which is the mode `read_key`/`_edit_line` run in anyway. + tty.setcbreak(self._slave) + self._target = _TtyBuf() + + # `tty.setcbreak` defaults to TCSAFLUSH, which throws away input already queued on the + # terminal. The code under test calls it *after* we have fed our keystrokes, so with the + # default those keystrokes vanish and the read blocks forever. TCSANOW switches mode without + # discarding, which is exactly what a real interactive session does not need but a + # feed-then-read test does. + real_setcbreak = tty.setcbreak + + def _setcbreak_now(fd: int, when: int = termios.TCSANOW) -> None: + real_setcbreak(fd, termios.TCSANOW) + + self._patches = [ + patch.object(tty, "setcbreak", _setcbreak_now), + patch.object(style.sys, "stdin", _RawStdin(self._slave, read_error=self._read_error)), + patch.object(style, "_raw_session", _REAL_RAW_SESSION), + ] + for p in self._patches: + p.start() + return self._master, self._target + + def feed(self, data: bytes) -> None: + os.write(self._master, data) + + def __exit__(self, *exc: object) -> None: + for p in reversed(self._patches): + p.stop() + os.close(self._master) + os.close(self._slave) + + +def _feed(master: int, data: bytes) -> None: + os.write(master, data) + + +def _feed_staged(master: int, chunks: list[bytes], delay: float = 0.09) -> None: + """Write chunks with a gap between them, from a background thread. + + Needed only where an escape sequence is followed by more input: `_drain_escape_sequence` drains + *everything* currently buffered, so a single write of `arrow + text` loses the text. On a real + keyboard the arrow's bytes arrive as one burst and the next keystroke comes later; the gap here + (longer than the 50ms drain window) reproduces that so the drain stops at the arrow. + """ + import threading + + def run() -> None: + for chunk in chunks: + time.sleep(delay) + os.write(master, chunk) + + threading.Thread(target=run, daemon=True).start() + + +# --------------------------------------------------------------------------------------------- +# Display helpers the fallback suite never calls +# --------------------------------------------------------------------------------------------- + + +def test_enabled_and_can_rewrite_swallow_a_stream_that_cannot_answer_isatty() -> None: + """Asking a closed or exotic stream whether it is a terminal must not raise inside output code.""" + class _Broken(io.StringIO): + def isatty(self) -> bool: + raise ValueError("closed") + + with patch.dict("os.environ", {}, clear=True): + assert style.enabled(_Broken()) is False + assert style.can_rewrite(_Broken()) is False + + +def test_the_marks_and_headers_render_in_colour() -> None: + tty = _TtyBuf() + with patch.dict("os.environ", {"FORCE_COLOR": "1"}, clear=True): + assert "\033" in style.ok_mark(stream=tty) + assert "\033" in style.fail_mark(stream=tty) + assert "━" in style.section("Step", step=1, total=3, stream=tty) + assert "─" in style.subsection("Item", step=1, total=3, stream=tty) + + +def test_subsection_and_field_render_plain_too() -> None: + plain = io.StringIO() + assert "1 of 3" in style.subsection("Item", step=1, total=3, stream=plain) + assert "Cluster:" in style.field("Cluster", "x", stream=plain) + + +# --------------------------------------------------------------------------------------------- +# Activity: the spinner thread and the draw internals +# --------------------------------------------------------------------------------------------- + + +def test_activity_write_disables_itself_when_the_stream_breaks() -> None: + """A closed stream mid-run must not take down the operation being reported on.""" + class _Broken(_TtyBuf): + def write(self, s: str) -> int: + raise OSError("gone") + + act = style.Activity("x", stream=_Broken()) + act._write("anything") + assert act._rewrites is False + + +def test_the_spinner_stays_silent_until_the_threshold_passes() -> None: + """A wake before the threshold hits the `continue`, so nothing is drawn for a quick operation.""" + tty = _TtyBuf() + with style.activity("x", "waiting", stream=tty, threshold=0.3): + time.sleep(0.15) # one spinner wake (~0.1s) lands under the threshold + assert tty.getvalue() == "" # never crossed the threshold, so never drew + + +def test_the_spinner_notices_done_after_the_threshold_under_the_lock() -> None: + """If the run finishes just as a frame is due, the drawn-check under the lock bails cleanly.""" + tty = _TtyBuf() + act = style.Activity("x", stream=tty, threshold=0.0) + act.__enter__() + act._lock.acquire() # make the spinner block right where it would draw + time.sleep(0.15) # let it wake, pass the threshold, and wait on the lock + act._done.set() # now finish, so the post-lock check returns + act._lock.release() + act.__exit__(None, None, None) + # Nothing was drawn (the draw was blocked then cancelled), so there is nothing to erase. + assert "\r" not in tty.getvalue() + + +def test_a_long_frame_is_truncated_to_the_terminal_width() -> None: + tty = _TtyBuf() + act = style.Activity("x", "y" * 200, stream=tty, threshold=0.0) + act._started = time.monotonic() + with patch("factory.contained.style.shutil.get_terminal_size", + return_value=os.terminal_size((40, 24))): + act._draw() + drawn = tty.getvalue() + assert "…" in drawn # the overflow was cut, not wrapped + + +# --------------------------------------------------------------------------------------------- +# _raw_session itself +# --------------------------------------------------------------------------------------------- + + +def test_raw_session_returns_the_fd_and_saved_settings_on_a_real_tty() -> None: + import pty + + master, slave = pty.openpty() + try: + with patch.object(style.sys, "stdin", _RawStdin(slave)): + session = _REAL_RAW_SESSION(_TtyBuf()) + assert session is not None + fd, saved = session + assert fd == slave and isinstance(saved, list) + finally: + os.close(master) + os.close(slave) + + +def test_raw_session_declines_when_the_target_is_not_a_terminal() -> None: + import pty + + master, slave = pty.openpty() + try: + with patch.object(style.sys, "stdin", _RawStdin(slave)): + assert _REAL_RAW_SESSION(io.StringIO()) is None # target.isatty() is False + finally: + os.close(master) + os.close(slave) + + +def test_raw_session_declines_when_isatty_raises() -> None: + class _Broken: + def isatty(self) -> bool: + raise ValueError("closed") + + with patch.object(style.sys, "stdin", _Broken()): + assert _REAL_RAW_SESSION(_TtyBuf()) is None + + +def test_raw_session_declines_when_termios_refuses_the_descriptor() -> None: + import pty + import termios + + master, slave = pty.openpty() + try: + with patch.object(style.sys, "stdin", _RawStdin(slave)), \ + patch("termios.tcgetattr", side_effect=termios.error("nope")): + assert _REAL_RAW_SESSION(_TtyBuf()) is None + finally: + os.close(master) + os.close(slave) + + +# --------------------------------------------------------------------------------------------- +# read_key on a real terminal +# --------------------------------------------------------------------------------------------- + + +def test_read_key_returns_a_single_character() -> None: + with _pty() as (master, target): + _feed(master, b"a") + assert style.read_key("? ", stream=target) == "a" + + +def test_read_key_returns_escape_for_a_bare_escape() -> None: + with _pty() as (master, target): + _feed(master, b"\x1b") + assert style.read_key("? ", stream=target) == style.ESCAPE + + +def test_read_key_ignores_an_arrow_key() -> None: + """An arrow arrives as ESC + `[A`; the drain consumes the tail and the key is reported as ''.""" + with _pty() as (master, target): + _feed(master, b"\x1b[A") + assert style.read_key("? ", stream=target) == "" + + +def test_read_key_returns_none_when_the_read_fails() -> None: + with _pty(read_error=True) as (master, target): + assert style.read_key("? ", stream=target) is None + + +# --------------------------------------------------------------------------------------------- +# read_line / _edit_line on a real terminal +# --------------------------------------------------------------------------------------------- + + +def test_read_line_accepts_a_typed_line() -> None: + with _pty() as (master, target): + _feed(master, b"factory-yi\r") + assert style.read_line("Namespace", stream=target) == "factory-yi" + + +def test_read_line_backspace_erases_a_character() -> None: + with _pty() as (master, target): + _feed(master, b"ab\x7fc\r") # a, b, backspace, c -> "ac" + assert style.read_line("?", stream=target) == "ac" + + +def test_read_line_escape_cancels_immediately() -> None: + with _pty() as (master, target): + _feed(master, b"\x1b") + assert style.read_line("?", stream=target) is None + + +def test_read_line_an_arrow_key_is_neither_text_nor_cancel() -> None: + with _pty() as (master, target): + # Staged: the arrow first, then the text after the drain window, so the drain stops at the + # arrow instead of swallowing the "x". + _feed_staged(master, [b"\x1b[A", b"x\r"]) + assert style.read_line("?", stream=target) == "x" + + +def test_read_line_ctrl_d_on_an_empty_line_cancels() -> None: + with _pty() as (master, target): + _feed(master, b"\x04") + assert style.read_line("?", stream=target) is None + + +def test_read_line_ctrl_d_mid_line_is_ignored() -> None: + with _pty() as (master, target): + _feed(master, b"a\x04b\r") # the Ctrl-D between letters does nothing + assert style.read_line("?", stream=target) == "ab" + + +# --------------------------------------------------------------------------------------------- +# read_secret on a real terminal +# --------------------------------------------------------------------------------------------- + + +def test_read_secret_masks_the_echo_but_returns_the_value() -> None: + with _pty() as (master, target): + _feed(master, b"s3kret\r") + value = style.read_secret("API key", stream=target) + assert value == "s3kret" + echo = target.getvalue() + assert "s3kret" not in echo # the value never appears on screen + assert "*" * 6 in echo # one mask per character + + +# --------------------------------------------------------------------------------------------- +# confirm / select — keypress arms and the input() fallback arms +# --------------------------------------------------------------------------------------------- + + +def test_confirm_keypress_enter_takes_the_default_and_n_returns_false() -> None: + with patch("factory.contained.style.read_key", return_value="\r"): + assert style.confirm("?", default=True) is True + with patch("factory.contained.style.read_key", return_value="n"): + assert style.confirm("?") is False + + +def test_confirm_line_fallback_covers_yes_no_and_reask() -> None: + with patch("factory.contained.style.read_key", return_value=None), \ + patch("builtins.input", side_effect=["y"]): + assert style.confirm("?") is True + with patch("factory.contained.style.read_key", return_value=None), \ + patch("builtins.input", side_effect=["no"]): + assert style.confirm("?") is False + # An unrecognised answer re-asks rather than guessing. + with patch("factory.contained.style.read_key", return_value=None), \ + patch("builtins.input", side_effect=["huh?", "yes"]): + assert style.confirm("?") is True + + +def test_select_keypress_reasks_on_an_unknown_key() -> None: + with patch("factory.contained.style.read_key", side_effect=["z", "1"]): + assert style.select("?", [("1", "local"), ("2", "k8s")]) == "1" + + +def test_select_line_fallback_covers_valid_reask_escape_and_eof() -> None: + with patch("factory.contained.style.read_key", return_value=None), \ + patch("builtins.input", side_effect=["nope", "2"]): + assert style.select("?", [("1", "local"), ("2", "k8s")]) == "2" + with patch("factory.contained.style.read_key", return_value=None), \ + patch("builtins.input", return_value="\x1b"): + assert style.select("?", [("1", "local")]) is None + with patch("factory.contained.style.read_key", return_value=None), \ + patch("builtins.input", side_effect=EOFError): + assert style.select("?", [("1", "local")]) is None + + +# --------------------------------------------------------------------------------------------- +# The last few branch edges +# --------------------------------------------------------------------------------------------- + + +def test_activity_plain_update_is_silent_before_the_threshold() -> None: + """Off a tty and still quick: `update` records the text but writes nothing yet.""" + plain = io.StringIO() + act = style.Activity("x", stream=plain, threshold=5.0) + act._started = time.monotonic() + act.update("waiting") + assert plain.getvalue() == "" + + +def test_read_line_returns_none_when_the_raw_read_fails() -> None: + with _pty(read_error=True) as (master, target): + assert style.read_line("?", stream=target) is None + + +def test_read_secret_returns_none_when_the_raw_read_fails() -> None: + with _pty(read_error=True) as (master, target): + assert style.read_secret("?", stream=target) is None + + +def test_edit_line_ignores_backspace_on_an_empty_line() -> None: + with _pty() as (master, target): + _feed(master, b"\x7fab\r") # backspace with nothing to erase, then "ab" + assert style.read_line("?", stream=target) == "ab" + + +def test_edit_line_ignores_an_unhandled_control_character() -> None: + with _pty() as (master, target): + _feed(master, b"\x01a\r") # Ctrl-A: not a key it handles, and not printable + assert style.read_line("?", stream=target) == "a" + + +def test_confirm_keypress_reasks_on_an_unrelated_key() -> None: + with patch("factory.contained.style.read_key", side_effect=["q", "y"]): + assert style.confirm("?") is True