diff --git a/README.md b/README.md index 3c59880..8fd7179 100644 --- a/README.md +++ b/README.md @@ -6,8 +6,9 @@ JSON back. It also clones one organization's resources into another. ```bash curl -LsSf https://raw.githubusercontent.com/Zipstack/unstract-cli/main/install.sh | sh -unstract config init -unstract config doctor +export UNSTRACT_PLATFORM_KEY=... +unstract auth whoami # resolves and stores your organisation +unstract docstudio deployment ls # what can I run? ``` The installer fetches `uv` if it is missing and installs the CLI with it; `uv` @@ -16,6 +17,11 @@ brings its own Python, so nothing on the machine has to match. Already have thing. Set `UNSTRACT_CLI_SOURCE` to install a branch or a local checkout instead. +`auth whoami` is the shortest way in: a platform key carries the organisation it +belongs to, so supplying the key is enough to discover `org_id` rather than +reading it out of a web-app URL. `config init` and `config doctor` are still +there for a profile you write by hand. + Or run it without installing: `uvx --from git+https://github.com/Zipstack/unstract-cli unstract --discover groups`. ## Output @@ -70,8 +76,8 @@ search, or `$UNSTRACT_CONFIG`, or `--config`. Every setting resolves **flag > env > profile > built-in default**, and the CLI is fully usable with no config file at all. The flag tier is the connection options on each product group — `unstract docstudio --base-url … --org-id … deployment run …`, and -`--base-url`/`--api-key` on `whisper` — which override the profile for that one -invocation without writing anything. +`--base-url`/`--api-key` on `whisper` and on `auth` — which override the profile +for that one invocation without writing anything. ```toml default_profile = "cloud-us" @@ -85,6 +91,9 @@ base_url = "https://us-central.unstract.com" org_id = "org_ABC123" api_key = "env:UNSTRACT_DEPLOYMENT_KEY" +[profiles.cloud-us.platform] +base_url = "https://us-central.unstract.com" + [profiles.cloud-us.deployments.invoices] api_name = "invoice-parser" ``` @@ -96,9 +105,23 @@ own `api_key` when its deployment has a separate key of its own. Get an LLMWhisperer key from the LLMWhisperer console; a deployment key is shown on the API deployment's own page in the Unstract UI, and an organisation-wide one -under Settings → API Key Manager. `config init` also writes an -`onprem-example` profile as a shape to copy for a self-hosted install — its host -is a placeholder, and only the *active* profile is ever resolved. +under Settings → API Key Manager. A **platform key** is minted by an +organisation admin under Settings → Platform API Keys. `config init` also writes +an `onprem-example` profile as a shape to copy for a self-hosted install — its +host is a placeholder, and only the *active* profile is ever resolved. + +The two Unstract keys are not interchangeable and neither replaces the other. A +deployment key runs deployments and cannot say which organisation it belongs +to; a platform key identifies the organisation and lists what is in it, and +cannot run a deployment. `auth whoami` and `deployment ls` take the platform key; +`deployment run` and `deployment status` take the deployment key; `auth whoami` +and `deployment ls` take the platform key. `org_id` lives on the `docstudio` +block either way — `auth whoami` writes the one it resolves there, because that +is where everything that needs it reads from. + +`config init` deliberately leaves `platform.api_key` out of the block above, so +that a caller who only holds a deployment key is not told a platform key is +missing. Add the line, or set `$UNSTRACT_PLATFORM_KEY`, when you have one. A credential can be written into the file literally, but `env:VAR_NAME` indirection is what `config init` writes and what the examples use: the file @@ -128,7 +151,8 @@ not part of the document-processing path the rest of this CLI wraps, so an agent serving a user request should not reach for it unasked. It talks to two deployments at once, which no single profile describes, so it takes both endpoints as flags and both keys from `UNSTRACT_SRC_PLATFORM_KEY` / -`UNSTRACT_TGT_PLATFORM_KEY`. It exits 0 when nothing failed, which is not the +`UNSTRACT_TGT_PLATFORM_KEY` — two keys for two organisations, so it reads neither +the `platform` profile block nor `$UNSTRACT_PLATFORM_KEY`. It exits 0 when nothing failed, which is not the same as everything having moved: oversize and unsupported documents are skipped by design, and `data.skipped` counts them. diff --git a/src/unstract_cli/app.py b/src/unstract_cli/app.py index 2eb6306..72002af 100644 --- a/src/unstract_cli/app.py +++ b/src/unstract_cli/app.py @@ -16,6 +16,7 @@ from unstract_cli.config import ( DOCSTUDIO, LLMWHISPERER, + PLATFORM, ConfigError, ResolvedConfig, load_config, @@ -84,7 +85,7 @@ def override(self, product: str, values: dict[str, Any]) -> None: def secrets(self) -> list[str]: """Resolved credentials, for scrubbing anything on its way to a stream.""" out: list[str] = [] - for product in (LLMWHISPERER, DOCSTUDIO): + for product in (LLMWHISPERER, DOCSTUDIO, PLATFORM): try: if value := self.config.get(product, "api_key"): out.append(str(value)) @@ -217,8 +218,9 @@ def whisper_group(ctx: Context, **overrides: str | None) -> None: "--transport-timeout", type=float, default=None, - help="Seconds before a stalled connection is given up on. Unset means it " - "is not, which is what the client has always done.", + help="Seconds before a stalled connection is given up on. Unset means no " + "bound for `deployment run` and `status`, and the platform client's own " + "60s default for `deployment ls`.", ) @pass_context def docstudio_group( @@ -234,11 +236,39 @@ def deployment_group() -> None: """Work with a deployed API.""" +@cli.group("auth") +@_connection_options() +@click.option( + "--transport-timeout", + type=float, + default=None, + help="Seconds before a stalled connection is given up on. Unset means the " + "client's own default, which is 60.", +) +@pass_context +def auth_group( + ctx: Context, transport_timeout: float | None, **overrides: str | None +) -> None: + """Identify the credential you are using. + + Its flags configure the platform key, which is the credential that knows + which organisation it belongs to. A deployment key does not: it authenticates + against the deployment it was minted for and never reaches this endpoint. + """ + ctx.transport_timeout = transport_timeout + ctx.override(PLATFORM, overrides) + + cli.add_command(config_group) # Imported for their side effect of registering commands, and imported last # because those modules hang their commands off the groups declared just above. -from unstract_cli.commands import clone_cmd, docstudio_cmd, whisper_cmd # noqa: E402,F401 +from unstract_cli.commands import ( # noqa: E402,F401 + clone_cmd, + docstudio_cmd, + platform_cmd, + whisper_cmd, +) def command_tree() -> dict[str, Any]: diff --git a/src/unstract_cli/commands/config_cmd.py b/src/unstract_cli/commands/config_cmd.py index aa02390..7730613 100644 --- a/src/unstract_cli/commands/config_cmd.py +++ b/src/unstract_cli/commands/config_cmd.py @@ -18,6 +18,7 @@ DOCSTUDIO, KEY_SOURCES, LLMWHISPERER, + PLATFORM, PRODUCTS, ConfigError, ConfigFile, @@ -36,6 +37,7 @@ emit_result, resolve_format, ) +from unstract_cli.core.platform import platform_client #: Keys whose value is never echoed back, even on explicit request: this output #: is as likely to land in a log or a transcript as on a screen. @@ -208,10 +210,11 @@ def _probe(resolved: ResolvedConfig) -> dict[str, Any]: """Check each product's credentials against the service, where that is possible. LLMWhisperer has a read-only usage endpoint, so its key can be verified for - real. A deployment has no side-effect-free endpoint -- the only thing to call - is an execution -- so its entry reports that the settings resolve and says - plainly that nothing was verified. Claiming otherwise would be worse than - not checking. + real, and so does the platform API -- `whoami` reads nothing but the key + itself. A deployment has no side-effect-free endpoint -- the only thing to + call is an execution -- so its entry reports that the settings resolve and + says plainly that nothing was verified. Claiming otherwise would be worse + than not checking. """ out: dict[str, Any] = {} try: @@ -233,6 +236,31 @@ def _probe(resolved: ResolvedConfig) -> dict[str, Any]: "detail": "The key was accepted by the usage endpoint.", } + try: + with translated(endpoint="whoami"): + identity = platform_client(resolved).whoami() + except CLIError as exc: + out[PLATFORM] = { + "checked": True, + "ok": False, + "detail": exc.message, + "exit_code": int(exc.exit_code), + } + except ConfigError as exc: + # Null, not False: a platform key is optional -- a caller holding only a + # deployment key is the common case -- so an absent one is a report + # rather than a failure, and must not decide this command's exit code. + out[PLATFORM] = {"checked": False, "ok": None, "detail": str(exc)} + else: + out[PLATFORM] = { + "checked": True, + "ok": True, + # The organisation is the reason to hold this key, so the probe + # reports which one answered rather than only that one did. + "organization_id": identity.get("organization_id"), + "detail": "The key was accepted, and resolved to an organisation.", + } + resolves = all( resolved.get(DOCSTUDIO, key) for key in ("org_id", "api_key", "base_url") ) diff --git a/src/unstract_cli/commands/platform_cmd.py b/src/unstract_cli/commands/platform_cmd.py new file mode 100644 index 0000000..9f20ed2 --- /dev/null +++ b/src/unstract_cli/commands/platform_cmd.py @@ -0,0 +1,245 @@ +"""`unstract auth whoami` and `unstract docstudio deployment ls`. + +Both authenticate with a platform key rather than a deployment key. The two +credentials are not interchangeable and neither is going away: a deployment key +runs deployments and cannot describe the account, a platform key describes the +account and lists what is in it but cannot run anything. + +No OpenAPI spec is vendored for the platform API, so these declare their flags +by hand rather than through `spec_options`. +""" + +from __future__ import annotations + +from typing import Any + +import click + +from unstract_cli.app import Context, auth_group, deployment_group, pass_context +from unstract_cli.commands.common import finish +from unstract_cli.config import DOCSTUDIO, ConfigError, load_config, save_config +from unstract_cli.core.clients import translated +from unstract_cli.core.errors import CLIError, ExitCode +from unstract_cli.core.output import diagnostic +from unstract_cli.core.platform import organisation, platform_client + +#: The fields a deployment listing shows. The server sends fifteen per row, +#: including run histories; `--output table` wraps rather than truncates, so the +#: whole row is unreadable at a terminal. Narrowed here rather than silently cut +#: off downstream -- `--full` returns the rows as sent. +LISTING_FIELDS = ("api_name", "display_name", "id", "is_active", "api_endpoint") + + +class SaveDeclinedError(Exception): + """The organisation resolved, and storing it was deliberately skipped. + + Distinct from a write that *failed*: there is nothing to retry and nothing + is wrong. The call succeeded, so it exits 0 and reports the identity, with + `meta.saved` false and `reason` saying which rule declined -- the same shape + `--no-save` already produces. + """ + + def __init__(self, reason: str, hint: str) -> None: + super().__init__(reason) + self.reason = reason + self.hint = hint + + +def _store_organisation(ctx: Context, org_id: str) -> dict[str, Any]: + """Write the resolved organisation into the profile the run is using. + + The profile name comes from `ResolvedConfig.active_profile`, which is the + same flag > env > file-default ladder every read uses. Re-deriving it here + is what dropped the `$UNSTRACT_PROFILE` tier, so the organisation was + written into a profile no later command read. + + It lands on the docstudio block because that is where every consumer reads + it from -- deployment URLs and aliases both -- and a second copy under the + platform block would be one more thing to keep in agreement. + """ + cfg = load_config() + if cfg.is_project_local: + # A `.unstract.toml` found by walking up from the working directory is + # very likely committed. Rewriting it would replace a teammate's + # `org_id` with this caller's, drop every comment (the file is + # re-serialised, not patched) and narrow its mode to 0600 -- a dirty, + # mode-changed, semantically different tracked file, from a command + # named `whoami`. The config layer already declines to *trust* this + # file for credentials; declining to *write* it is the same judgement. + raise SaveDeclinedError( + f"the config at {cfg.path} is project-local", + hint="Nothing was written. Rerun with --no-save to silence this, " + "or store it elsewhere: `unstract --config config set " + f"docstudio org_id {org_id}`.", + ) + + selected = ctx.config.active_profile or cfg.default_profile + if selected is None and cfg.exists and cfg.profiles: + # Neither the caller nor the file named one, so the "cloud-us" literal + # below is this function's own invention -- refusing under that name + # would quote a profile the caller never typed, and advising `config + # set` would create a third one that shadows theirs as the new default. + if len(cfg.profiles) == 1: + selected = next(iter(cfg.profiles)) + else: + known = ", ".join(sorted(cfg.profiles)) + raise ConfigError( + f"no profile is selected and {cfg.path} names no default " + f"(known profiles: {known}); rerun with `-p `" + ) + + name = selected or "cloud-us" + if cfg.exists and cfg.profiles and name not in cfg.profiles: + # `setdefault` would create it. That is not a convenience: the profile + # lookup raises "Profile not found" for a typo today, and materialising + # the name silently disarms that check for every later command, which + # then resolves the built-in production defaults instead. + # + # Raised as `ConfigError` so the caller's SAVE_FAILED wrapper carries + # the identity back: the key was resolved, only the note-taking failed. + known = ", ".join(sorted(cfg.profiles)) or "none" + raise ConfigError( + f"profile {name!r} is not in {cfg.path} " + f"(known profiles: {known}); create it with `config set` first" + ) + + cfg.profiles.setdefault(name, {}).setdefault(DOCSTUDIO, {})["org_id"] = org_id + if not cfg.default_profile: + cfg.default_profile = name + return {"profile": name, "path": str(save_config(cfg))} + + +@auth_group.command("whoami") +@click.option( + "--save/--no-save", + default=True, + help="Write the resolved organisation into the active profile.", +) +@pass_context +def whoami(ctx: Context, save: bool) -> None: + """Resolve which organisation your platform key belongs to. + + The organisation is otherwise only discoverable by reading it out of a + web-app URL, and every other command needs it. Resolving it here and storing + it means it is supplied once rather than pasted. + + \b + Examples: + export UNSTRACT_PLATFORM_KEY=... + unstract auth whoami + unstract auth whoami --no-save # validate the key, change nothing + + Exits 3 when the key is rejected, so a setup script can branch on it without + reading the message. + """ + client = platform_client(ctx.config, timeout=getattr(ctx, "transport_timeout", None)) + with translated(endpoint="whoami"): + identity = client.whoami() + + if not save: + finish(ctx, identity, meta={"saved": False, "reason": "--no-save"}) + return + + org_id = identity.get("organization_id") + if not org_id: + # Distinguished from --no-save: the caller asked to store and there was + # nothing to store, which the next command will fail on. + diagnostic( + "warning: the platform API returned no organization_id; nothing was stored.", + quiet=ctx.quiet, + verbosity=ctx.verbosity, + ) + finish(ctx, identity, meta={"saved": False, "reason": "no organization_id"}) + return + + try: + written = _store_organisation(ctx, str(org_id)) + except SaveDeclinedError as exc: + # The identity is what was asked for; the write was a convenience this + # config layout declines. Reporting the whole command as a usage error + # would fail the CLI's documented first command in any checkout holding + # a committed `.unstract.toml`, and throw the identity away with it. + diagnostic( + f"note: org_id was not stored -- {exc.reason}. {exc.hint}", + quiet=ctx.quiet, + verbosity=ctx.verbosity, + ) + finish(ctx, identity, meta={"saved": False, "reason": exc.reason}) + return + except (OSError, ConfigError) as exc: + # The read succeeded; only the convenience write failed. Losing the + # identity to a full disk would report a working key as a total failure, + # and SAVE_FAILED exists for exactly this shape. + raise CLIError( + f"Resolved the organisation but could not write it: {exc}", + ExitCode.SAVE_FAILED, + details=identity, + hint="`details` carries the identity; set $UNSTRACT_ORG_ID or run " + f"`unstract config set docstudio org_id {org_id}`.", + ) from exc + + # `meta` is not rendered by `-o table` or `-o raw`, so a human would + # otherwise see nothing about a file this command just wrote. + diagnostic( + f"wrote org_id={org_id} to profile {written['profile']!r} in {written['path']}", + quiet=ctx.quiet, + verbosity=ctx.verbosity, + ) + finish(ctx, identity, meta={"saved": True, **written}) + + +@deployment_group.command("ls") +@click.option( + "--api-name", + default=None, + help="Return only the deployment with this exact API name.", +) +@click.option( + "--full/--no-full", + default=False, + help=f"Return every field the server sends, not just {', '.join(LISTING_FIELDS)}.", +) +@pass_context +def ls(ctx: Context, api_name: str | None, full: bool) -> None: + """List the API deployments in your organisation. + + Answers what a deployment is called, which is the one thing `deployment run` + needs and the UI is the only other place to find. Authenticates with the + platform key, not the deployment key -- but takes the same `--base-url` as + its sibling commands, since one deployment serves both. + + \b + Examples: + unstract docstudio deployment ls + unstract docstudio deployment ls --api-name invoice-parser + unstract docstudio deployment ls --full + """ + if ctx.config.overrides.get(f"{DOCSTUDIO}.api_key") is not None: + # `--api-key` on the docstudio group means a *deployment* key, and this + # command authenticates with a platform key. Honouring it would send a + # deployment key to the platform API; ignoring it silently and then + # reporting the platform key as missing is what shipped, and reads as a + # broken flag rather than the wrong credential. + raise CLIError( + "`--api-key` on `docstudio` is a deployment key; " + "`deployment ls` authenticates with a platform key.", + ExitCode.USAGE, + hint="Drop the flag and set $UNSTRACT_PLATFORM_KEY, or add " + "`api_key` to the [profiles..platform] block. A deployment " + "key runs a deployment; a platform key describes the account.", + ) + + client = platform_client( + ctx.config, + organisation(ctx.config), + timeout=getattr(ctx, "transport_timeout", None), + ) + with translated(endpoint="api/deployment/"): + rows = client.list_api_deployments(api_name=api_name) + + if not full: + rows = [{field: row.get(field) for field in LISTING_FIELDS} for row in rows] + finish(ctx, {"results": rows}, meta={"count": len(rows)}) + + +__all__ = ["SaveDeclinedError", "ls", "whoami"] diff --git a/src/unstract_cli/config.py b/src/unstract_cli/config.py index 2064808..82be023 100644 --- a/src/unstract_cli/config.py +++ b/src/unstract_cli/config.py @@ -19,6 +19,7 @@ import os import stat import tomllib +from collections.abc import Iterator from copy import deepcopy from dataclasses import dataclass, field from pathlib import Path @@ -30,43 +31,65 @@ LLMWHISPERER = "llmwhisperer" DOCSTUDIO = "docstudio" -PRODUCTS: tuple[str, ...] = (LLMWHISPERER, DOCSTUDIO) +PLATFORM = "platform" +PRODUCTS: tuple[str, ...] = (LLMWHISPERER, DOCSTUDIO, PLATFORM) #: Built-in defaults, lowest precedence. DEFAULT_BASE_URLS: dict[str, str] = { LLMWHISPERER: "https://llmwhisperer-api.us-central.unstract.com/api/v2", DOCSTUDIO: "https://us-central.unstract.com", + # The same host as docstudio: one deployment serves both the platform API + # and the deployments it manages. + PLATFORM: "https://us-central.unstract.com", } #: Environment variables per (product, setting), checked before the config file #: and in the order given. The trailing names are the ones the published clients #: themselves read: an environment already set up for a client must not leave #: the CLI silently on its built-in default, which is production. +#: +#: `platform` deliberately has no `org_id` of its own. A platform key carries +#: its organisation, and `auth whoami` writes the one it resolves to the +#: docstudio block -- the block everything else already reads. Two `org_id` +#: settings would mean two rows in `config doctor` that a user has to keep in +#: agreement by hand. ENV_VARS: dict[tuple[str, str], tuple[str, ...]] = { (LLMWHISPERER, "api_key"): ("LLMWHISPERER_API_KEY",), (LLMWHISPERER, "base_url"): ("LLMWHISPERER_BASE_URL", "LLMWHISPERER_BASE_URL_V2"), (DOCSTUDIO, "api_key"): ("UNSTRACT_DEPLOYMENT_KEY", "UNSTRACT_API_DEPLOYMENT_KEY"), (DOCSTUDIO, "base_url"): ("UNSTRACT_BASE_URL",), (DOCSTUDIO, "org_id"): ("UNSTRACT_ORG_ID",), + (PLATFORM, "api_key"): ("UNSTRACT_PLATFORM_KEY",), + (PLATFORM, "base_url"): ("UNSTRACT_BASE_URL",), + # `clone` takes this as --api-prefix because a self-hosted deployment can + # mount the Platform API somewhere other than api/v1. `OrgEndpoint` already + # defaults to api/v1, which standard installs serve; this is what reaches + # the ones that remount it, where whoami and `deployment ls` are otherwise + # unreachable. + (PLATFORM, "api_prefix"): ("UNSTRACT_API_PREFIX",), } -#: Where the two credentials are minted. Quoted wherever the CLI reports one as -#: missing: knowing a key is unset is no help without knowing where one is made. +#: Where the three credentials are minted. Quoted wherever the CLI reports one +#: as missing: knowing a key is unset is no help without knowing where one is +#: made. KEY_SOURCES = ( "Get an LLMWhisperer key from the LLMWhisperer console; a deployment key is " "shown on the API deployment's own page in the Unstract UI, and a key " "covering every deployment in the organisation is minted under " - "Settings -> API Key Manager." + "Settings -> API Key Manager. A platform key, which identifies the " + "organisation and lists what is in it but cannot run a deployment, is " + "minted by an organisation admin under Settings -> Platform API Keys." ) def settings_for(product: str) -> tuple[str, ...]: """The settings a product actually has. - Products differ: `org_id` is a URL path segment for one and meaningless for - the other, and reporting a setting a user has no way to supply reads as a - misconfiguration they cannot fix. + Products differ: `org_id` is a setting only for `docstudio` -- llmwhisperer + has no organisation, and `platform` reads docstudio's -- and reporting a + setting a user has no way to supply reads as a misconfiguration they cannot + fix. """ return tuple(sorted(key for prod, key in ENV_VARS if prod == product)) @@ -373,17 +396,48 @@ def get(self, product: str, key: str, default: Any = None) -> Any: remember_secret(value) return value - def _resolve(self, product: str, key: str, default: Any = None) -> Any: - if (value := self.overrides.get(f"{product}.{key}")) is not None: - return value - if (value := self.overrides.get(key)) is not None: - return value + def get_explicit(self, product: str, key: str) -> Any: + """Resolve through **flag > env > profile** only, stopping before defaults. - for env_var in ENV_VARS.get((product, key), ()): - if value := os.environ.get(env_var): - return value + `get` cannot answer "did anyone actually name this?" -- it returns + `DEFAULT_BASE_URLS[product]` for an unset `base_url`, so a caller who + deliberately named the default host and one who named nothing come back + as the same string. Anything that must treat those two differently asks + here instead of comparing the answer against the default, which reads + the caller's own choice as silence. + """ + value = self._explicit(product, key) + if key == "api_key": + remember_secret(value) + return value + + def explicit_tiers(self, product: str, key: str) -> Iterator[Any]: + """What each tier says, in order -- flag, env, profile -- unset as `None`. + + For a setting two products share -- one deployment serves both, so + `base_url` is really one question asked twice -- picking a product first + and then walking its tiers inverts the precedence the whole config layer + promises: a profile value on the preferred product beats a *flag* on the + other. Walking tier by tier across both products keeps flag > env > + profile true regardless of which product a value was written under. + + Lazy on purpose: reading the profile block resolves the profile name, + which raises for one that does not exist. A caller answered by an + earlier tier must not be failed by a later one it never consulted. + """ + yield self.overrides.get(f"{product}.{key}", self.overrides.get(key)) + yield next( + (v for e in ENV_VARS.get((product, key), ()) if (v := os.environ.get(e))), + None, + ) + yield _deref(self._product_block(product).get(key)) - if (value := _deref(self._product_block(product).get(key))) is not None: + def _explicit(self, product: str, key: str) -> Any: + """The tiers a human supplied: flag, then environment, then profile.""" + return next((v for v in self.explicit_tiers(product, key) if v is not None), None) + + def _resolve(self, product: str, key: str, default: Any = None) -> Any: + if (value := self._explicit(product, key)) is not None: return value if default is not None: @@ -538,6 +592,11 @@ def starter_profiles() -> dict[str, dict[str, Any]]: "org_id": "", "api_key": "env:UNSTRACT_DEPLOYMENT_KEY", }, + # No `api_key` on purpose. A platform key is optional -- holding + # only a deployment key is the common case -- and an `env:` + # reference to an unset variable is a `config doctor` problem, + # which would exit 1 for every user who does not hold one. + PLATFORM: {"base_url": DEFAULT_BASE_URLS[PLATFORM]}, "deployments": {"example": {"api_name": "your-api-deployment-name"}}, }, "cloud-eu": { @@ -558,6 +617,8 @@ def starter_profiles() -> dict[str, dict[str, Any]]: "org_id": "", "api_key": "env:UNSTRACT_DEPLOYMENT_KEY", }, + # No `api_key` -- see the cloud-us block. + PLATFORM: {"base_url": "https://unstract.internal.example"}, }, } @@ -569,6 +630,7 @@ def starter_profiles() -> dict[str, dict[str, Any]]: "HOME_CONFIG", "KEY_SOURCES", "LLMWHISPERER", + "PLATFORM", "PRODUCTS", "PROJECT_CONFIG_NAME", "UNTRUSTED_PROJECT_KEYS", diff --git a/src/unstract_cli/core/clients.py b/src/unstract_cli/core/clients.py index 439b3d1..d8985fd 100644 --- a/src/unstract_cli/core/clients.py +++ b/src/unstract_cli/core/clients.py @@ -5,9 +5,10 @@ expected, so it is translated here into a ``CLIError`` carrying an exit code, a hint and the response detail. -The two clients report failure differently -- LLMWhisperer raises with a status -code attached, the deployment client returns a dict containing one -- so both -shapes converge here rather than in each command. +The clients report failure differently -- LLMWhisperer raises with a status +code attached, the deployment client returns a dict containing one, the Platform +API client raises with the body attached -- so every shape converges here rather +than in each command. """ from __future__ import annotations @@ -16,11 +17,19 @@ from contextlib import contextmanager from typing import Any -from requests.exceptions import ConnectionError, Timeout +from requests.exceptions import ( + ConnectionError, + InvalidSchema, + InvalidURL, + MissingSchema, + RequestException, + Timeout, +) from unstract.api_deployments.client import ( APIDeploymentsClient, APIDeploymentsClientException, ) +from unstract.clone.exceptions import PlatformAPIError from unstract.llmwhisperer.client_v2 import ( LLMWhispererClientException, LLMWhispererClientV2, @@ -137,6 +146,23 @@ def translated(endpoint: str | None = None) -> Iterator[None]: raise CLIError(message, details=details, endpoint=endpoint) from exc except APIDeploymentsClientException as exc: raise CLIError(str(exc), ExitCode.USAGE, endpoint=endpoint) from exc + except PlatformAPIError as exc: + # `PlatformAPIError.__init__` appends "\n body: " to + # its own message, so `str(exc)` would put up to 2KB of server body into + # `error.message` -- which `emit_error` documents as a one-line summary + # -- and duplicate it into `details`. + message = str(exc).split("\n body:", 1)[0] + # The Platform API client raises rather than returning a status, and + # carries the response body on the exception. Untranslated it would + # reach the entry point as an unexpected crash and print a traceback. + if exc.status_code: + raise error_from_status( + int(exc.status_code), + message, + details=exc.body, + endpoint=endpoint, + ) from exc + raise CLIError(message, details=exc.body, endpoint=endpoint) from exc except Timeout as exc: raise CLIError( str(exc), @@ -160,6 +186,29 @@ def translated(endpoint: str | None = None) -> Iterator[None]: retryable=True, hint="Could not reach the service. Check the base URL and connectivity.", ) from exc + except RequestException as exc: + # Must sit after Timeout and ConnectionError, which are subclasses. + # + # The Platform client is a bare `requests.Session`: unlike the other two + # it wraps nothing itself, so a malformed base URL (`MissingSchema`, + # `InvalidURL`) or a 2xx carrying HTML from a proxy or SPA host + # (`JSONDecodeError`) arrives here raw. Every one of those subclasses + # `OSError`, so untranslated they were caught by the entry point's + # full-disk handler and rendered "Check the path and disk." -- the wrong + # subsystem, on the one message a user with a bad base URL most needs to + # be right. + usage = isinstance(exc, (MissingSchema, InvalidSchema, InvalidURL)) + raise CLIError( + str(exc), + ExitCode.USAGE if usage else ExitCode.SERVER_ERROR, + endpoint=endpoint, + hint=( + "Check `base_url` -- it needs a scheme, e.g. https://host." + if usage + else "The service answered, but not with JSON. Check that " + "`base_url` names the API rather than a proxy or web app." + ), + ) from exc def translating( diff --git a/src/unstract_cli/core/errors.py b/src/unstract_cli/core/errors.py index a36402a..89d8cce 100644 --- a/src/unstract_cli/core/errors.py +++ b/src/unstract_cli/core/errors.py @@ -37,8 +37,11 @@ class ExitCode(IntEnum): 401: ExitCode.AUTH, 403: ExitCode.AUTH, 404: ExitCode.NOT_FOUND, - # Only the deployment status endpoint answers 406; the whisper equivalent - # is a 400 whose body says so, which is prose we do not translate on. + # The deployment status endpoint is the one that answers 406 meaningfully; + # the whisper equivalent is a 400 whose body says so, which is prose we do + # not translate on. A 406 from anywhere else -- DRF returns one on content + # negotiation failure -- lands on this code too, and reads as a one-shot + # read that was already consumed. 406: ExitCode.ALREADY_CONSUMED, 408: ExitCode.TIMEOUT, 409: ExitCode.VALIDATION, @@ -246,8 +249,8 @@ def hint_for(status: int) -> str | None: return ( "The key was rejected. Keys are per-product: `unstract config " "doctor` reports which one resolved and from where. A key that " - "works elsewhere can still be rejected here -- it may not cover " - "this deployment, or may belong to another organisation." + "works elsewhere can still be rejected here -- it may be the " + "wrong kind for this command, or belong to another organisation." ) case 404: return ( diff --git a/src/unstract_cli/core/platform.py b/src/unstract_cli/core/platform.py new file mode 100644 index 0000000..a7dcae2 --- /dev/null +++ b/src/unstract_cli/core/platform.py @@ -0,0 +1,164 @@ +"""The Platform API client, and the one call the published client lacks. + +`PlatformClient` ships in `unstract-client` for the clone subpackage, and its +list endpoints are exactly what `deployment ls` needs. It builds every URL as +``{base_url}/{prefix}/unstract/{organization_id}//``, which is right for +every call that acts inside an organisation and wrong for the one call made +before the organisation is known. + +`whoami` is that call, so it is added here rather than in the published client: +it needs no release to land, and the organisation-less shape is a CLI concern +until something else wants it. +""" + +from __future__ import annotations + +from typing import Any + +from unstract.clone.client import PlatformClient +from unstract.clone.context import OrgEndpoint + +from unstract_cli.config import ( + DOCSTUDIO, + PLATFORM, + ResolvedConfig, +) +from unstract_cli.core.errors import CLIError, ExitCode + +#: The organisation `whoami` is called with. The endpoint carries no +#: organisation segment -- resolving it is the point of the call -- and +#: `OrgEndpoint` requires the field, so it is named rather than left as a bare +#: empty string at the call site. +NO_ORGANISATION = "" + + +class CLIPlatformClient(PlatformClient): + """`PlatformClient` plus the organisation-less identity read.""" + + def whoami(self) -> dict[str, Any]: + """Describe the key: which organisation it belongs to, and its tier. + + The URL is built here instead of through ``_url`` because that method + inserts the organisation this call exists to discover. + """ + base = self.endpoint.base_url.rstrip("/") + prefix = self.endpoint.api_path_prefix.strip("/") + body = self._send("GET", f"{base}/{prefix}/unstract/whoami/", "whoami/") + if not isinstance(body, dict): + # `_send` returns None on a 204 or an empty 2xx body, and whatever + # `resp.json()` decoded otherwise -- a list, for a misrouted host. + # Guarded here, where the shape is known, rather than at each + # consumer: unguarded, `body.get(...)` raises an AttributeError that + # matches no arm in `__main__`, so the caller gets a traceback and + # no envelope at all. + raise CLIError( + "The platform API did not return an identity.", + ExitCode.SERVER_ERROR, + details=body, + endpoint="whoami", + hint="Check that `base_url` names an Unstract deployment that " + "serves /unstract/whoami/.", + ) + return body + + +def platform_base_url(config: ResolvedConfig) -> str: + """Where the platform API lives. + + One deployment serves both the platform API and the deployments it manages, + so a caller who has said where docstudio is has already said where this is. + Resolving `platform.base_url` alone would ignore that: a profile written + before the `platform` block existed, and every `docstudio --base-url`, would + silently fall through to the built-in cloud default and send the key there. + + The two products are walked **tier by tier**, not one product at a time. + Asking `platform` for all three tiers first would let a profile's + `platform.base_url` beat a `docstudio --base-url` flag -- and `config init` + writes `platform.base_url` into every profile it generates, so that would + silently ignore the flag for every generated config, which is the majority + of them. + + Each tier is read explicitly, stopping before the built-in defaults. + Comparing `get`'s answer against the default instead would read a caller who + named the SaaS host as one who named nothing -- the same `config init` + profiles again, from the other direction. + """ + for from_platform, from_docstudio in zip( + config.explicit_tiers(PLATFORM, "base_url"), + config.explicit_tiers(DOCSTUDIO, "base_url"), + strict=True, + ): + # Within one tier the platform block wins: it is the specific answer to + # this question, where docstudio's is the one inherited from the sibling. + if from_platform is not None: + return str(from_platform) + if from_docstudio is not None: + return str(from_docstudio) + return str(config.require(PLATFORM, "base_url")) + + +def platform_client( + config: ResolvedConfig, + org_id: str | None = None, + *, + timeout: float | None = None, +) -> CLIPlatformClient: + """Build a Platform API client from the resolved configuration. + + ``org_id`` is optional because `whoami` runs before one is known. Every + other call needs it, and asks for it explicitly. + """ + endpoint = OrgEndpoint( + base_url=platform_base_url(config), + organization_id=org_id if org_id is not None else NO_ORGANISATION, + platform_key=config.require(PLATFORM, "api_key"), + **( + {"api_path_prefix": prefix} + if (prefix := config.get(PLATFORM, "api_prefix")) is not None + else {} + ), + ) + # `PlatformClient`'s own default is 60s per request, which `_paginate` + # spends per page. An interactive caller who asked for a bound gets it. + if timeout is None: + return CLIPlatformClient(endpoint) + if timeout <= 0: + # urllib3 raises a bare ValueError for a non-positive timeout, which + # matches no arm in `__main__` -- a traceback and no envelope. Rejected + # here, where it is still a usage error about a flag. + raise CLIError( + f"--transport-timeout must be greater than 0, not {timeout:g}.", + ExitCode.USAGE, + hint="Omit the flag to use the client's own 60s default.", + ) + # Passed as the float it was parsed as. `PlatformClient` annotates this + # `int`, but the annotation is not enforced and `requests` takes floats; + # truncating instead would send `--transport-timeout 0.5` as 0, which is + # the ValueError above, and silently round 1.9 down to 1. + return CLIPlatformClient(endpoint, timeout=timeout) + + +def organisation(config: ResolvedConfig) -> str: + """The organisation to act inside, or a usage error naming how to get one. + + It lives on the docstudio block: a platform key resolves it, and everything + that consumes it -- deployment URLs, aliases -- reads it from there. + """ + if org_id := config.get(DOCSTUDIO, "org_id"): + return str(org_id) + raise CLIError( + "No organisation is configured.", + ExitCode.USAGE, + hint=( + "Run `unstract auth whoami` to resolve it from your platform key, " + "or set $UNSTRACT_ORG_ID." + ), + ) + + +__all__ = [ + "CLIPlatformClient", + "organisation", + "platform_base_url", + "platform_client", +] diff --git a/tests/test_commands.py b/tests/test_commands.py index 015a354..7e30077 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -12,6 +12,7 @@ import pytest from requests.exceptions import ConnectionError +from unstract.clone.exceptions import PlatformAPIError from unstract.clone.report import CloneReport, Endpoint, PhaseResult from unstract.llmwhisperer.client_v2 import ( LLMWhispererClientException, @@ -22,8 +23,8 @@ from unstract_cli.__main__ import main from unstract_cli.app import command_tree -from unstract_cli.commands import clone_cmd, docstudio_cmd, whisper_cmd -from unstract_cli.config import LLMWHISPERER +from unstract_cli.commands import clone_cmd, docstudio_cmd, platform_cmd, whisper_cmd +from unstract_cli.config import LLMWHISPERER, PLATFORM from unstract_cli.core.errors import CLIError, ExitCode @@ -117,6 +118,30 @@ def build(_config, _target, transport_timeout=None): return install +@pytest.fixture +def platform_client(monkeypatch): + """Install a fake Platform API client and hand it back to the test.""" + + def install(**replies): + client = FakeWhisper(**replies) + client.built_with = {} + + def build(config, org_id=None, *, timeout=None): + # Resolving the key is what registers it for scrubbing, so the fake + # factory has to do it too or the seam hides a production path. + # The signature tracks the real `platform_client` deliberately: a + # fixture that drifts from it passes while testing nothing. + client.built_with["api_key"] = config.get(PLATFORM, "api_key") + client.built_with["org_id"] = org_id + client.built_with["timeout"] = timeout + return client + + monkeypatch.setattr(platform_cmd, "platform_client", build) + return client + + return install + + # --------------------------------------------------------------------------- # # The command surface # --------------------------------------------------------------------------- # @@ -140,9 +165,11 @@ def test_the_v1_commands_are_registered(): "update", } assert set(tree["docstudio"]["commands"]["deployment"]["commands"]) == { + "ls", "run", "status", } + assert set(tree["auth"]["commands"]) == {"whoami"} # --------------------------------------------------------------------------- # @@ -1119,3 +1146,472 @@ def fake_clone(source, target, options): assert code == int(ExitCode.SUCCESS) assert "adapters" in captured.out assert key not in captured.out and key not in captured.err + + +# --------------------------------------------------------------------------- # +# auth whoami +# --------------------------------------------------------------------------- # + +IDENTITY = { + "organization_id": "org_ABC123", + "organization_name": "Acme", + "permission": "read", + "key_name": "ci", +} + + +def _platform_env(monkeypatch, tmp_path): + """A resolvable platform key, and a config file of our own to write into.""" + monkeypatch.setenv("UNSTRACT_PLATFORM_KEY", "pk-123") + monkeypatch.setenv("UNSTRACT_CONFIG", str(tmp_path / "config.toml")) + + +def test_whoami_reports_the_identity_the_service_returned( + capsys, platform_client, monkeypatch, tmp_path +): + _platform_env(monkeypatch, tmp_path) + platform_client(whoami=IDENTITY) + + code, out, _ = run(capsys, "auth", "whoami") + + assert code == int(ExitCode.SUCCESS) + assert envelope(out)["data"] == IDENTITY + + +def test_whoami_is_called_with_no_organisation( + capsys, platform_client, monkeypatch, tmp_path +): + """Resolving the organisation is the point, so requiring one would be + circular.""" + _platform_env(monkeypatch, tmp_path) + client = platform_client(whoami=IDENTITY) + + run(capsys, "auth", "whoami") + + assert client.built_with["org_id"] is None + assert client.built_with["api_key"] == "pk-123" + + +def test_whoami_stores_the_organisation_where_everything_else_reads_it( + capsys, platform_client, monkeypatch, tmp_path +): + _platform_env(monkeypatch, tmp_path) + platform_client(whoami=IDENTITY) + + _, out, _ = run(capsys, "auth", "whoami") + + assert envelope(out)["meta"]["saved"] is True + # Read back through the CLI rather than out of the file: what matters is + # that the next command resolves it, not where the bytes landed. + _, out, _ = run(capsys, "config", "get", "docstudio", "org_id") + assert envelope(out)["data"]["value"] == "org_ABC123" + + +def test_whoami_can_validate_without_writing_anything( + capsys, platform_client, monkeypatch, tmp_path +): + _platform_env(monkeypatch, tmp_path) + platform_client(whoami=IDENTITY) + + _, out, _ = run(capsys, "auth", "whoami", "--no-save") + + assert envelope(out)["meta"]["saved"] is False + assert not (tmp_path / "config.toml").exists() + + +def test_a_rejected_platform_key_exits_on_the_auth_code( + capsys, platform_client, monkeypatch, tmp_path +): + """A traceback here would mean the Platform API's own exception type never + reached the translator.""" + _platform_env(monkeypatch, tmp_path) + platform_client(whoami=PlatformAPIError("nope", status_code=401, body="{}")) + + code, out, _ = run(capsys, "auth", "whoami") + + assert code == int(ExitCode.AUTH) + assert envelope(out)["ok"] is False + + +def test_whoami_without_a_key_is_a_usage_error(capsys, monkeypatch, tmp_path): + monkeypatch.setenv("UNSTRACT_CONFIG", str(tmp_path / "config.toml")) + code, out, _ = run(capsys, "auth", "whoami") + + assert code == int(ExitCode.USAGE) + assert "UNSTRACT_PLATFORM_KEY" in json.dumps(envelope(out)["error"]) + + +# --------------------------------------------------------------------------- # +# docstudio deployment ls +# --------------------------------------------------------------------------- # + +DEPLOYMENT_ROW = { + "api_name": "invoice-parser", + "display_name": "Invoices", + "id": "dep-1", + "is_active": True, + "api_endpoint": "https://example.com/deployment/api/org/invoice-parser/", + "created_by_email": "someone@example.com", + "last_5_run_statuses": [], +} + + +def _returns(value): + """Queue one reply whose value is itself a list. + + `FakeWhisper` reads a list reply as a queue of replies, so a bare list would + hand back its first row rather than the listing. + """ + return [value] + + +def _listing_env(monkeypatch, tmp_path): + monkeypatch.setenv("UNSTRACT_PLATFORM_KEY", "pk-123") + monkeypatch.setenv("UNSTRACT_ORG_ID", "org_ABC123") + monkeypatch.setenv("UNSTRACT_CONFIG", str(tmp_path / "config.toml")) + + +def test_ls_narrows_the_row_to_what_a_caller_can_read( + capsys, platform_client, monkeypatch, tmp_path +): + _listing_env(monkeypatch, tmp_path) + platform_client(list_api_deployments=_returns([DEPLOYMENT_ROW])) + + _, out, _ = run(capsys, "docstudio", "deployment", "ls") + + (row,) = envelope(out)["data"]["results"] + assert set(row) == set(platform_cmd.LISTING_FIELDS) + assert row["api_name"] == "invoice-parser" + + +def test_ls_can_return_every_field_the_server_sent( + capsys, platform_client, monkeypatch, tmp_path +): + _listing_env(monkeypatch, tmp_path) + platform_client(list_api_deployments=_returns([DEPLOYMENT_ROW])) + + _, out, _ = run(capsys, "docstudio", "deployment", "ls", "--full") + + (row,) = envelope(out)["data"]["results"] + assert row == DEPLOYMENT_ROW + + +def test_ls_passes_the_name_filter_to_the_server( + capsys, platform_client, monkeypatch, tmp_path +): + """Filtering here rather than locally: the server has the exact-match + filter, and a local one would still page the whole organisation.""" + _listing_env(monkeypatch, tmp_path) + client = platform_client(list_api_deployments=_returns([DEPLOYMENT_ROW])) + + run(capsys, "docstudio", "deployment", "ls", "--api-name", "invoice-parser") + + assert client.kwargs_for("list_api_deployments") == {"api_name": "invoice-parser"} + + +def test_ls_runs_inside_the_configured_organisation( + capsys, platform_client, monkeypatch, tmp_path +): + _listing_env(monkeypatch, tmp_path) + client = platform_client(list_api_deployments=_returns([])) + + run(capsys, "docstudio", "deployment", "ls") + + assert client.built_with["org_id"] == "org_ABC123" + + +def test_ls_without_an_organisation_says_how_to_get_one( + capsys, platform_client, monkeypatch, tmp_path +): + monkeypatch.setenv("UNSTRACT_PLATFORM_KEY", "pk-123") + monkeypatch.setenv("UNSTRACT_CONFIG", str(tmp_path / "config.toml")) + platform_client(list_api_deployments=_returns([])) + + code, out, _ = run(capsys, "docstudio", "deployment", "ls") + + assert code == int(ExitCode.USAGE) + assert "whoami" in json.dumps(envelope(out)["error"]) + + +# --------------------------------------------------------------------------- # +# auth whoami — where it writes, and what happens when it cannot +# --------------------------------------------------------------------------- # + + +def _config_with(tmp_path, monkeypatch, text): + path = tmp_path / "config.toml" + path.write_text(text, encoding="utf-8") + monkeypatch.setenv("UNSTRACT_CONFIG", str(path)) + monkeypatch.setenv("UNSTRACT_PLATFORM_KEY", "pk-123") + return path + + +def test_whoami_writes_to_the_profile_the_run_is_actually_using( + capsys, platform_client, monkeypatch, tmp_path +): + """Reads resolve through `active_profile` (flag > env > file default). + Re-deriving that chain here dropped the env tier, so the organisation was + written into a profile no later command reads -- and `deployment ls` then + failed immediately after a `whoami` reporting `saved: true`. + """ + path = _config_with( + tmp_path, + monkeypatch, + 'default_profile = "cloud-us"\n' + '[profiles.cloud-us.docstudio]\norg_id = ""\n' + '[profiles.cloud-eu.docstudio]\norg_id = ""\n', + ) + monkeypatch.setenv("UNSTRACT_PROFILE", "cloud-eu") + platform_client(whoami=IDENTITY) + + _, out, _ = run(capsys, "auth", "whoami") + + assert envelope(out)["meta"]["profile"] == "cloud-eu" + assert 'org_id = "org_ABC123"' in path.read_text().split("[profiles.cloud-eu")[1] + + +def test_whoami_refuses_to_invent_a_profile_that_does_not_exist( + capsys, platform_client, monkeypatch, tmp_path +): + """`setdefault` created it. That silently disarmed the "Profile not found" + guard for every later command, which then resolved the built-in production + defaults instead -- from a single typo, permanently. + """ + _config_with( + tmp_path, + monkeypatch, + 'default_profile = "cloud-us"\n[profiles.cloud-us.docstudio]\norg_id = ""\n', + ) + platform_client(whoami=IDENTITY) + + code, out, _ = run(capsys, "-p", "cloud-uss", "auth", "whoami") + + # SAVE_FAILED, not USAGE: the key resolved and only the note-taking failed, + # so the identity comes back in `details` rather than being discarded. + error = envelope(out)["error"] + assert code == int(ExitCode.SAVE_FAILED) + assert "cloud-uss" in json.dumps(error) + assert error["details"]["organization_id"] == IDENTITY["organization_id"] + + +def test_whoami_writes_to_the_only_profile_when_no_default_is_named( + capsys, platform_client, monkeypatch, tmp_path +): + """The unknown-profile guard fired on the literal "cloud-us" fallback -- a + name the caller never typed -- for any file with profiles and no + `default_profile`, and advised creating a third that would shadow theirs. + """ + path = _config_with( + tmp_path, + monkeypatch, + '[profiles.work.docstudio]\norg_id = ""\n', + ) + platform_client(whoami=IDENTITY) + + code, out, _ = run(capsys, "auth", "whoami") + + assert code == 0 + assert envelope(out)["meta"]["profile"] == "work" + written = path.read_text(encoding="utf-8") + assert f'org_id = "{IDENTITY["organization_id"]}"' in written + assert 'default_profile = "work"' in written + + +def test_whoami_will_not_guess_between_several_unselected_profiles( + capsys, platform_client, monkeypatch, tmp_path +): + """Two profiles and no default: writing into either would be a guess. It + says so and hands the identity back, rather than naming `cloud-us`. + """ + _config_with( + tmp_path, + monkeypatch, + '[profiles.work.docstudio]\norg_id = ""\n[profiles.home.docstudio]\norg_id = ""\n', + ) + platform_client(whoami=IDENTITY) + + code, out, _ = run(capsys, "auth", "whoami") + error = envelope(out)["error"] + + assert code == int(ExitCode.SAVE_FAILED) + assert "cloud-us" not in json.dumps(error) + assert "-p " in json.dumps(error) + assert error["details"]["organization_id"] == IDENTITY["organization_id"] + + +def test_whoami_keeps_the_identity_when_the_write_fails( + capsys, platform_client, monkeypatch, tmp_path +): + """The read succeeded and only the convenience write failed. Losing the + identity to a full disk reported a working key as a total failure, on an + exit code that means "you invoked it wrong". + """ + _config_with( + tmp_path, + monkeypatch, + 'default_profile = "cloud-us"\n[profiles.cloud-us.docstudio]\norg_id = ""\n', + ) + platform_client(whoami=IDENTITY) + monkeypatch.setattr( + platform_cmd, + "save_config", + lambda *a, **k: (_ for _ in ()).throw(OSError(13, "nope")), + ) + + code, out, _ = run(capsys, "auth", "whoami") + error = envelope(out)["error"] + + assert code == int(ExitCode.SAVE_FAILED) + # Not full equality: `redact_value` masks any field whose *name* looks + # secret, and `key_name` matches -- so the identity reaches `details` with + # that one field starred out. The organisation is the part the caller needs + # in order to carry on without the write. + assert error["details"]["organization_id"] == IDENTITY["organization_id"] + assert error["details"]["key_name"] == "***REDACTED***" + + +def test_whoami_does_not_rewrite_a_discovered_project_config( + capsys, platform_client, monkeypatch, tmp_path +): + """A `.unstract.toml` found by walking up is very likely committed. Writing + it replaced a teammate's org_id, dropped every comment and narrowed the mode + -- from a command named `whoami`, with no flag asked for. + """ + project = tmp_path / "repo" + project.mkdir() + (project / ".unstract.toml").write_text( + '# hand written\ndefault_profile = "team"\n[profiles.team.docstudio]\norg_id = "org_TEAM"\n', + encoding="utf-8", + ) + monkeypatch.chdir(project) + monkeypatch.delenv("UNSTRACT_CONFIG", raising=False) + monkeypatch.setenv("UNSTRACT_PLATFORM_KEY", "pk-123") + platform_client(whoami=IDENTITY) + + code, out, envelope_err = run(capsys, "auth", "whoami") + body = envelope(out) + + # Declining the write is not failing the call: README blesses a committed + # `.unstract.toml`, and `auth whoami` is the documented first command, so + # exiting 2 broke the quickstart and discarded the identity with it. + assert code == 0 + assert body["data"]["organization_id"] == IDENTITY["organization_id"] + assert body["meta"]["saved"] is False + assert "project-local" in body["meta"]["reason"] + assert "# hand written" in (project / ".unstract.toml").read_text() + assert "org_TEAM" in (project / ".unstract.toml").read_text() + + +@pytest.mark.parametrize( + ("argv", "expected"), + [ + (["auth", "whoami"], None), + (["auth", "--transport-timeout", "12.5", "whoami"], 12.5), + (["docstudio", "deployment", "ls"], None), + (["docstudio", "--transport-timeout", "12.5", "deployment", "ls"], 12.5), + ], +) +def test_the_transport_timeout_flag_reaches_the_platform_client( + capsys, platform_client, monkeypatch, tmp_path, argv, expected +): + """The flag was accepted on both groups and threaded through the factory, + but nothing asserted the commands passed it: deleting either call site left + the suite green. The fixture recorded the value and no test read it. + """ + _config_with( + tmp_path, + monkeypatch, + 'default_profile = "cloud-us"\n[profiles.cloud-us.docstudio]\norg_id = "org_X"\n', + ) + client = platform_client(whoami=IDENTITY, list_api_deployments=_returns([])) + + code, _, _ = run(capsys, *argv) + + assert code == 0 + assert client.built_with["timeout"] == expected + + +@pytest.mark.parametrize("value", ["0", "0.0", "-1"]) +def test_a_non_positive_transport_timeout_is_a_usage_error_not_a_traceback( + capsys, monkeypatch, tmp_path, value +): + """urllib3 raises a bare `ValueError` for a non-positive timeout, which + matches no arm in `__main__` -- a traceback and no envelope. The flag is + `type=float`, so anything in (0, 1) truncated to that same 0. + """ + _config_with(tmp_path, monkeypatch, "") + + code, out, _ = run(capsys, "auth", "--transport-timeout", value, "whoami") + + assert code == int(ExitCode.USAGE) + assert "transport-timeout" in envelope(out)["error"]["message"] + + +def test_a_deployment_key_flag_is_refused_rather_than_ignored_by_ls( + capsys, platform_client, monkeypatch, tmp_path +): + """`--api-key` on the docstudio group is a *deployment* key and `ls` + authenticates with a platform key. It was accepted, dropped, and the + platform key then reported missing -- which reads as a broken flag rather + than the wrong credential. + """ + _config_with( + tmp_path, + monkeypatch, + 'default_profile = "cloud-us"\n[profiles.cloud-us.docstudio]\norg_id = "org_X"\n', + ) + platform_client(list_api_deployments=_returns([])) + + code, out, _ = run( + capsys, "docstudio", "--api-key", "dk-FROM-FLAG", "deployment", "ls" + ) + error = envelope(out)["error"] + + assert code == int(ExitCode.USAGE) + assert "platform key" in error["message"] + assert "UNSTRACT_PLATFORM_KEY" in error["hint"] + assert "dk-FROM-FLAG" not in json.dumps(envelope(out)) + + +def test_the_platform_key_never_reaches_a_stream( + capsys, platform_client, monkeypatch, tmp_path +): + """`translated()` attaches `PlatformAPIError.body` -- the server's own + response -- as `details`. If the far end echoes the key, that is the path it + would travel to stdout. + """ + _config_with(tmp_path, monkeypatch, 'default_profile = "cloud-us"\n') + monkeypatch.setenv("UNSTRACT_PLATFORM_KEY", "pk-SUPERSECRET-0987654321") + platform_client( + whoami=PlatformAPIError( + "GET whoami/ returned 401", + status_code=401, + body='{"echoed": "pk-SUPERSECRET-0987654321"}', + ) + ) + + _, out, err = run(capsys, "auth", "whoami") + + assert "pk-SUPERSECRET-0987654321" not in out + assert "pk-SUPERSECRET-0987654321" not in err + + +def test_a_rejected_key_keeps_its_message_on_one_line( + capsys, platform_client, monkeypatch, tmp_path +): + """`PlatformAPIError` folds the body into its own string, so `str(exc)` put + up to 2KB of server response into `error.message` -- which `emit_error` + documents as a one-line summary -- and duplicated it into `details`. + """ + _config_with(tmp_path, monkeypatch, 'default_profile = "cloud-us"\n') + platform_client( + whoami=PlatformAPIError( + "GET whoami/ returned 401", status_code=401, body='{"m": "x"}' + ) + ) + + _, out, _ = run(capsys, "auth", "whoami") + error = envelope(out)["error"] + + assert "\n" not in error["message"] + assert error["details"] == '{"m": "x"}' diff --git a/tests/test_discover.py b/tests/test_discover.py index 69c3a04..200475e 100644 --- a/tests/test_discover.py +++ b/tests/test_discover.py @@ -14,6 +14,7 @@ from unstract_cli.__main__ import main from unstract_cli.commands import config_cmd +from unstract_cli.config import PLATFORM from unstract_cli.core.errors import CLIError, ExitCode from unstract_cli.core.output import CONTRACT_VERSION @@ -28,7 +29,12 @@ def test_groups_names_the_products_and_stops_there(capsys): """The cheap question stays cheap: no command list, no flags.""" code, data = run(capsys, "--discover", "groups") assert code == int(ExitCode.SUCCESS) - assert {g["name"] for g in data["groups"]} == {"config", "docstudio", "whisper"} + assert {g["name"] for g in data["groups"]} == { + "auth", + "config", + "docstudio", + "whisper", + } # A leaf listed among the groups is a group a consumer finds empty. assert [c["name"] for c in data["commands"]] == ["clone"] assert all(entry["help"] for entry in [*data["groups"], *data["commands"]]) @@ -165,6 +171,29 @@ def get_usage_info(self): return install +@pytest.fixture +def platform_probe_client(monkeypatch): + def install(reply=None): + class Fake: + def whoami(self): + if isinstance(reply, Exception): + raise reply + return reply or {} + + monkeypatch.setattr( + config_cmd, + "platform_client", + # Resolves the key like its sibling in test_commands, so the probe + # tests exercise the registration that feeds the scrubber. + lambda config, org_id=None, *, timeout=None: ( + config.get(PLATFORM, "api_key"), + Fake(), + )[1], + ) + + return install + + def test_doctor_makes_no_call_without_probe(capsys, probe_client): probe_client(CLIError("must not be called")) code, data = run(capsys, "config", "doctor") @@ -212,3 +241,82 @@ def test_the_deployment_probe_says_it_verified_nothing(capsys, probe_client, mon assert entry["checked"] is False and entry["ok"] is None assert entry["resolved"] is True assert "NOT verified" in entry["detail"] + + +def test_the_platform_probe_verifies_the_key_and_names_the_organisation( + capsys, probe_client, platform_probe_client, monkeypatch +): + """`whoami` reads nothing but the key, so unlike a deployment it can be + checked for real -- and the organisation it resolves is the reason to + hold the key at all. + """ + probe_client({"quota": 1}) + platform_probe_client({"organization_id": "org_ABC123"}) + monkeypatch.setenv("UNSTRACT_PLATFORM_KEY", "pk-123") + + _, data = run(capsys, "config", "doctor", "--probe") + + assert data["probe"]["platform"] == { + "checked": True, + "ok": True, + "organization_id": "org_ABC123", + "detail": "The key was accepted, and resolved to an organisation.", + } + + +def test_an_absent_platform_key_is_reported_not_failed(capsys, probe_client): + """A platform key is optional -- holding only a deployment key is the + common case -- so not having one must not decide the exit code. + """ + probe_client({"quota": 1}) + + code, data = run(capsys, "config", "doctor", "--probe") + + assert code == int(ExitCode.SUCCESS) + entry = data["probe"]["platform"] + assert entry["checked"] is False + assert entry["ok"] is None + + +def test_a_rejected_platform_key_fails_the_probe( + capsys, probe_client, platform_probe_client, monkeypatch +): + """A key that is set and wrong is a real misconfiguration, unlike one that + is simply absent.""" + probe_client({"quota": 1}) + platform_probe_client(CLIError("bad key", ExitCode.AUTH)) + monkeypatch.setenv("UNSTRACT_PLATFORM_KEY", "pk-123") + + code = main(["-o", "json", "config", "doctor", "--probe"]) + report = json.loads(capsys.readouterr().out)["error"]["details"] + + assert code == int(ExitCode.GENERIC) + assert report["probe"]["platform"] == { + "checked": True, + "ok": False, + "detail": "bad key", + "exit_code": int(ExitCode.AUTH), + } + + +def test_config_init_then_doctor_exits_zero_without_a_platform_key( + capsys, monkeypatch, tmp_path +): + """The property `test_an_absent_platform_key_is_reported_not_failed` claims, + checked the way a real user reaches it. + + That test runs with no config file. The starter profile written by + `config init` used to carry `api_key = "env:UNSTRACT_PLATFORM_KEY"`, and an + `env:` reference to an unset variable is a doctor *problem* -- so the test + passed while every user who ran the documented first command got exit 1. + """ + monkeypatch.setenv("UNSTRACT_CONFIG", str(tmp_path / "config.toml")) + monkeypatch.setenv("LLMWHISPERER_API_KEY", "lw-123456789") + monkeypatch.setenv("UNSTRACT_DEPLOYMENT_KEY", "dk-123456789") + monkeypatch.setenv("UNSTRACT_ORG_ID", "acme") + monkeypatch.delenv("UNSTRACT_PLATFORM_KEY", raising=False) + + assert main(["-o", "json", "config", "init"]) == int(ExitCode.SUCCESS) + capsys.readouterr() + + assert main(["-o", "json", "config", "doctor"]) == int(ExitCode.SUCCESS) diff --git a/tests/test_platform.py b/tests/test_platform.py new file mode 100644 index 0000000..1a8d2a8 --- /dev/null +++ b/tests/test_platform.py @@ -0,0 +1,278 @@ +"""The Platform API client itself, exercised rather than stubbed. + +Every command test replaces the client factory, which is the right seam for +asking what a command hands the client -- and it means nothing in the suite ever +constructs `CLIPlatformClient` or runs the URL it builds. Two mutations proved +that: dropping the `/unstract/` segment from `whoami`, and forcing every listing +to run against organisation `""`, both left the suite green. + +These tests close that. No network: `_send` is replaced with a recorder, which +is the boundary between "what we build" and "what requests does with it". +""" + +from __future__ import annotations + +import pytest +from unstract.clone.context import OrgEndpoint + +from unstract_cli.config import ( + DEFAULT_BASE_URLS, + DOCSTUDIO, + PLATFORM, + ConfigFile, + ResolvedConfig, +) +from unstract_cli.core.errors import CLIError, ExitCode +from unstract_cli.core.platform import ( + CLIPlatformClient, + platform_base_url, + platform_client, +) + + +def _client(**kwargs): + return CLIPlatformClient( + OrgEndpoint( + base_url=kwargs.pop("base_url", "https://host.example"), + organization_id=kwargs.pop("organization_id", ""), + platform_key="pk-000000000000", + **kwargs, + ) + ) + + +def _resolved(profiles, **overrides): + return ResolvedConfig( + file=ConfigFile(profiles=profiles, default_profile="p", exists=True), + overrides=overrides, + ) + + +# --- the URL whoami builds ------------------------------------------------ + + +def test_whoami_asks_for_no_organisation() -> None: + """The parent's `_url` injects the organisation this call exists to find, + so `whoami` builds its own URL -- and nothing else checks that it does. + """ + client = _client() + seen = {} + client._send = lambda method, url, label, **kw: ( + seen.update(method=method, url=url) or {"organization_id": "acme"} + ) + + client.whoami() + + assert seen["url"] == "https://host.example/api/v1/unstract/whoami/" + assert seen["method"] == "GET" + + +def test_whoami_url_survives_a_trailing_slash_on_the_base() -> None: + client = _client(base_url="https://host.example/") + seen = {} + client._send = lambda m, url, label, **kw: seen.update(url=url) or {"a": 1} + + client.whoami() + + assert seen["url"] == "https://host.example/api/v1/unstract/whoami/" + + +@pytest.mark.parametrize("body", [None, [], [{"organization_id": "acme"}], "text"]) +def test_whoami_rejects_a_body_that_is_not_an_identity(body) -> None: + """`_send` returns None on a 204 or an empty 2xx, and whatever `resp.json()` + decoded otherwise. Unguarded, `body.get(...)` raises an AttributeError that + matches no arm in `__main__`, so the caller gets a traceback and no envelope + -- the one thing this CLI promises never to do. + """ + client = _client() + client._send = lambda *a, **kw: body + + with pytest.raises(CLIError) as excinfo: + client.whoami() + + assert excinfo.value.exit_code == ExitCode.SERVER_ERROR + + +# --- what the factory puts in the endpoint -------------------------------- + + +def test_the_factory_passes_the_organisation_through() -> None: + """`deployment ls` runs inside an organisation; `whoami` runs before one is + known. Forcing the org-less value for both left every test green. + """ + config = _resolved({"p": {PLATFORM: {"api_key": "pk-000000000000"}}}) + + assert platform_client(config, "org_ABC").endpoint.organization_id == "org_ABC" + assert platform_client(config).endpoint.organization_id == "" + + +def test_the_factory_threads_a_timeout() -> None: + """`--transport-timeout` was accepted on `deployment ls` and ignored: the + parent's own default is 60s, spent per page. + """ + config = _resolved({"p": {PLATFORM: {"api_key": "pk-000000000000"}}}) + + assert platform_client(config, timeout=3).timeout == 3 + + +# --- which host the key is sent to ---------------------------------------- + + +def test_the_platform_host_follows_docstudio_when_unset() -> None: + """A profile written before the `platform` block existed names only + docstudio's host. Resolving `platform.base_url` alone fell through to the + built-in cloud default and sent the key there. + """ + config = _resolved( + {"p": {DOCSTUDIO: {"base_url": "https://onprem.example"}}}, + ) + + assert platform_base_url(config) == "https://onprem.example" + + +def test_a_docstudio_base_url_flag_reaches_the_platform_call() -> None: + """`docstudio --base-url` records `docstudio.base_url`; `deployment ls` + reads the platform block. The flag was accepted and dropped. + """ + config = _resolved({"p": {}}, **{"docstudio.base_url": "https://flag.example"}) + + assert platform_base_url(config) == "https://flag.example" + + +def test_an_explicit_platform_host_still_wins() -> None: + config = _resolved( + { + "p": { + DOCSTUDIO: {"base_url": "https://docstudio.example"}, + PLATFORM: {"base_url": "https://platform.example"}, + } + } + ) + + assert platform_base_url(config) == "https://platform.example" + + +def test_the_saas_default_is_honoured_when_the_caller_names_it() -> None: + """The first fix compared the resolved value against + `DEFAULT_BASE_URLS[PLATFORM]` to tell "unset" from "chosen". Those are the + same string, so a caller who named the SaaS host was read as having named + nothing and silently redirected to docstudio's -- the inverse of the defect + it fixed. `config init` writes that exact host into every profile, so this + is the common shape, not a corner of it. + """ + profile = _resolved( + { + "p": { + DOCSTUDIO: {"base_url": "https://onprem.example"}, + PLATFORM: {"base_url": DEFAULT_BASE_URLS[PLATFORM]}, + } + } + ) + flag = _resolved( + {"p": {DOCSTUDIO: {"base_url": "https://onprem.example"}}}, + **{"platform.base_url": DEFAULT_BASE_URLS[PLATFORM]}, + ) + + assert platform_base_url(profile) == DEFAULT_BASE_URLS[PLATFORM] + assert platform_base_url(flag) == DEFAULT_BASE_URLS[PLATFORM] + + +def test_a_docstudio_flag_beats_a_platform_host_in_the_profile() -> None: + """Greptile, on PR #3. Walking `platform`'s three tiers before docstudio's + let a *profile* value beat a *flag*, inverting the precedence the config + layer promises everywhere else -- and `config init` writes + `platform.base_url` into every profile it generates, so the flag was ignored + for every generated config, not a corner case. + """ + config = _resolved( + {"p": {PLATFORM: {"base_url": DEFAULT_BASE_URLS[PLATFORM]}}}, + **{"docstudio.base_url": "https://flag.example"}, + ) + + assert platform_base_url(config) == "https://flag.example" + + +def test_a_platform_flag_still_beats_a_docstudio_flag() -> None: + """Within one tier the specific product wins; across tiers it does not.""" + config = _resolved( + {"p": {}}, + **{ + "platform.base_url": "https://platform-flag.example", + "docstudio.base_url": "https://docstudio-flag.example", + }, + ) + + assert platform_base_url(config) == "https://platform-flag.example" + + +def test_an_environment_host_beats_a_profile_on_either_product() -> None: + """`$UNSTRACT_BASE_URL` maps to both products, and env outranks profile.""" + import os + + config = _resolved({"p": {PLATFORM: {"base_url": "https://profile.example"}}}) + os.environ["UNSTRACT_BASE_URL"] = "https://env.example" + try: + assert platform_base_url(config) == "https://env.example" + finally: + del os.environ["UNSTRACT_BASE_URL"] + + +def test_the_built_in_default_is_the_last_resort_not_a_veto() -> None: + """Nobody named a host anywhere: the built-in default is still the answer. + `get_explicit` stopping before the defaults must not lose that. + """ + assert platform_base_url(_resolved({"p": {}})) == DEFAULT_BASE_URLS[PLATFORM] + + +@pytest.mark.parametrize("value", [0, 0.0, -1]) +def test_a_non_positive_timeout_is_refused_before_urllib3_sees_it(value) -> None: + """urllib3 raises a bare `ValueError` for a non-positive timeout, which + matches no arm in `__main__`. Truncating the float with `int()` turned every + `--transport-timeout` under 1s into exactly that. + """ + config = _resolved({"p": {PLATFORM: {"api_key": "pk-000000000000"}}}) + + with pytest.raises(CLIError) as caught: + platform_client(config, timeout=value) + + assert caught.value.exit_code == ExitCode.USAGE + + +def test_a_sub_second_timeout_survives_as_a_float() -> None: + """`int(0.5)` is 0, which is the ValueError above; `int(1.9)` is 1, which is + a bound the caller did not ask for. + """ + config = _resolved({"p": {PLATFORM: {"api_key": "pk-000000000000"}}}) + + assert platform_client(config, timeout=0.5).timeout == 0.5 + assert platform_client(config, timeout=1.9).timeout == 1.9 + + +def test_the_api_prefix_env_var_is_wired(monkeypatch) -> None: + """The profile tier reads the block directly without consulting `ENV_VARS`, + so removing the entry left the suite green while `$UNSTRACT_API_PREFIX` did + nothing. + """ + monkeypatch.setenv("UNSTRACT_API_PREFIX", "unstract-api/v1") + config = _resolved({"p": {PLATFORM: {"api_key": "pk-000000000000"}}}) + + assert platform_client(config).endpoint.api_path_prefix == "unstract-api/v1" + + +def test_a_self_hosted_api_prefix_reaches_the_endpoint() -> None: + """`clone` takes --api-prefix because a self-hosted deployment can mount the + Platform API elsewhere. Without threading it, whoami and `deployment ls` + were unreachable on exactly those installs, with no flag, env var or profile + key that could fix it. + """ + config = _resolved( + {"p": {PLATFORM: {"api_key": "pk-000000000000", "api_prefix": "unstract-api/v1"}}} + ) + + client = platform_client(config) + + assert client.endpoint.api_path_prefix == "unstract-api/v1" + seen = {} + client._send = lambda m, url, label, **kw: seen.update(url=url) or {"a": 1} + client.whoami() + assert seen["url"].endswith("/unstract-api/v1/unstract/whoami/")