diff --git a/python/CHANGELOG.md b/python/CHANGELOG.md index c2fcd53af..c4ffdb74c 100644 --- a/python/CHANGELOG.md +++ b/python/CHANGELOG.md @@ -7,6 +7,32 @@ This project adheres to [Semantic Versioning](http://semver.org/). ### What's New +#### Credentials from sift-cli profiles + +`SiftClient` now reads the same `sift.toml` profiles that `sift-cli --profile` uses. An environment that you configure once for the CLI works from Python with no arguments. + +```python +client = SiftClient() # the CLI's default profile +client = SiftClient(profile="staging") # a named profile +client = SiftClient.from_profile("staging") +``` + +Precedence, highest first: + +1. Arguments. +2. A profile that you name in code. +3. `SIFT_API_KEY`, `SIFT_GRPC_URI`, `SIFT_REST_URI`, and `SIFT_APP_URL`. +4. The profile that `SIFT_PROFILE` names. +5. The config file's default profile. + +`client.credential_sources` reports the layer that supplied each value. See [Credentials and profiles](guides/credentials.md). + +The pytest plugin gains `--sift-profile`, the `sift_profile` ini key, and `SIFT_PROFILE`. In the plugin, the existing settings surfaces outrank the profile. The profile supplies only the values that they leave unset, so a key that CI injects stays in effect. + +An `http://` URL now connects without TLS instead of failing, because transport security follows the scheme of the gRPC URL. An `https://` URL and a bare host name behave as before. + +In the config file, the canonical spelling of the API key is `api_key`, which matches the rest of the Sift API. The older `apikey` spelling is still valid, so you do not need to migrate. If a profile holds both keys, the client uses `api_key`. + #### List and get data imports New in `client.data_import`: `list_` and `get` (plus `find`), and a `run.data_imports` property. diff --git a/python/docs/guides/credentials.md b/python/docs/guides/credentials.md new file mode 100644 index 000000000..af2524a1b --- /dev/null +++ b/python/docs/guides/credentials.md @@ -0,0 +1,157 @@ +# Credentials and profiles + +`SiftClient` finds its credentials the same way `sift-cli` does. If you already +run `sift-cli`, the Python client works with no arguments: + +```python +from sift_client import SiftClient + +client = SiftClient() +``` + +To use a named environment, give the profile name that you pass to +`sift-cli --profile`: + +```python +client = SiftClient(profile="staging") + +# The same thing, and easier to find in the docs: +client = SiftClient.from_profile("staging") +``` + +## The config file + +`sift-cli` keeps one or more profiles in a `sift.toml` file in your user config +directory. Use the CLI to create and edit that file: + +```bash +sift-cli config create +sift-cli config update --profile staging +sift-cli config where # prints the path +``` + +The top-level table is the default profile. Each named table is one more +profile: + +```toml +grpc_uri = "https://api.siftstack.com" +rest_uri = "https://api.siftstack.com" +app_uri = "https://app.siftstack.com" +api_key = "..." + +[staging] +grpc_uri = "https://api.staging.siftstack.com" +rest_uri = "https://api.staging.siftstack.com" +app_uri = "https://app.staging.siftstack.com" +api_key = "..." + +[localdev] +grpc_uri = "http://localhost:50051" +rest_uri = "http://localhost:8080" +api_key = "local" +``` + +A profile does not inherit from the default profile. If `[staging]` has no +`api_key`, the client reports an error. It does not fall back to the default +profile's key, because that key can point your tests at a different +environment. + +Use an `http://` scheme for a plaintext endpoint, as `[localdev]` does above. +The client reads the scheme to select TLS or plaintext. + +Older files spell the key `apikey`. That spelling is still valid, so you do not +need to migrate. `api_key` is canonical, matches the rest of the Sift API, and +is what `sift-cli config update` writes. If a profile holds both keys, the +client uses `api_key`. + +## Resolution order + +Highest precedence first: + +1. Arguments that you pass to `SiftClient`, one field at a time. +2. The fields of a profile that you name with `profile=`. +3. The environment variables `SIFT_API_KEY`, `SIFT_GRPC_URI`, `SIFT_REST_URI`, + and `SIFT_APP_URL`. +4. The fields of the profile that `SIFT_PROFILE` names. +5. The default (top-level) table of the config file. + +A profile that you name in code outranks the environment. +`SiftClient(profile="prod")` therefore reaches production even in a shell that +points somewhere else. `SIFT_PROFILE` does not outrank the environment, so CI +can select a profile for its endpoints and still inject the API key: + +```bash +export SIFT_PROFILE=staging # endpoints from the staging profile +export SIFT_API_KEY="$CI_SECRET" # key from the secret store +pytest +``` + +The client reads one profile at most. If you set both `profile=` and +`SIFT_PROFILE`, the client uses `profile=` and ignores the other profile. + +## Where the client looks for the file + +1. If `SIFT_CONFIG_FILE` is set, the client uses that path. +2. If not, the client uses your user config directory: + `$XDG_CONFIG_HOME/sift.toml` (or `~/.config/sift.toml`) on Linux, + `~/Library/Application Support/sift.toml` on macOS, and + `%APPDATA%\sift.toml` on Windows. + +The client does not search the current working directory. A `sift.toml` file in +a repository that you cloned therefore cannot supply an API key. + +## Check what a client resolved + +`credential_sources` reports the layer that supplied each value: + +```python +client = SiftClient(profile="staging") +client.profile # 'staging' +client.credential_sources # {'grpc_url': 'profile:staging', 'api_key': 'env', ...} +``` + +Each value is `arg`, `profile:`, `env`, `default`, or `unset`. Both +properties are `None` if you build the client from an explicit +`connection_config`, because that path does not resolve credentials. + +## Pass credentials directly + +Explicit arguments and `connection_config` work as before. Use them if the +credentials come from a source that the resolver does not read, such as a +secrets manager: + +```python +client = SiftClient( + api_key="...", + grpc_url="https://api.siftstack.com", + rest_url="https://api.siftstack.com", +) +``` + +## Errors + +If `SiftClient` cannot resolve the API key or either URL, it raises +`SiftCredentialsError`, a subclass of `ValueError`. The message names the +missing variables and the file and profile that the client read. It also lists +the profiles in that file and gives the `sift-cli` command that sets the +missing values. + +## Use with pytest + +The pytest plugin reads the same profiles. For the plugin's own settings, see +[Configuration & Defaults](pytest_plugin/configuration.md). + +The plugin uses a different precedence order. Its environment variables, +`--sift-*` flags, and `sift_grpc_uri` / `sift_rest_uri` ini keys all outrank the +profile. The profile supplies only the values that they leave unset, so a key +that CI injects stays in effect. + +```bash +pytest --sift-profile staging +``` + +```toml +# pyproject.toml +[tool.pytest.ini_options] +sift_profile = "staging" +``` diff --git a/python/docs/guides/index.md b/python/docs/guides/index.md index 105f0bb25..4181bad3f 100644 --- a/python/docs/guides/index.md +++ b/python/docs/guides/index.md @@ -6,6 +6,9 @@ works and how to configure it. For runnable, end-to-end walkthroughs see the ## Available guides +- [Credentials and profiles](credentials.md): how `SiftClient` resolves its API + key and endpoints from arguments, environment variables, and the `sift.toml` + profiles that `sift-cli` manages. - [Pytest Plugin](pytest_plugin/index.md): turn a pytest run into a `TestReport` in Sift. Each test becomes a `TestStep`, measurements are recorded as rows, and failures propagate up through nested substeps to the report. diff --git a/python/docs/guides/pytest_plugin/configuration.md b/python/docs/guides/pytest_plugin/configuration.md index c949b597d..fc4d5e88f 100644 --- a/python/docs/guides/pytest_plugin/configuration.md +++ b/python/docs/guides/pytest_plugin/configuration.md @@ -161,12 +161,13 @@ suggestion, so typos like `SIFT_REPORT_SERIALNUM` surface immediately. ### Connection -| Setting | Ini (`[tool.pytest.ini_options]`) | Env var | -|---|---|---| -| Sift API key (secret, env-only). | — | `SIFT_API_KEY` | -| Sift gRPC endpoint URI. | `sift_grpc_uri` | `SIFT_GRPC_URI` | -| Sift REST endpoint URI. | `sift_rest_uri` | `SIFT_REST_URI` | -| Sift web-app origin for the report link in the terminal footer (e.g. https://app.siftstack.com). When unset, the link is derived from the REST URI for known Sift hosts. | `sift_app_url` | `SIFT_APP_URL` | +| Setting | CLI flag | Ini (`[tool.pytest.ini_options]`) | Env var | +|---|---|---|---| +| Name of a sift.toml profile that supplies credentials, as with `sift-cli --profile`. | `--sift-profile` | `sift_profile` | `SIFT_PROFILE` | +| Sift API key (secret, env-only). | — | — | `SIFT_API_KEY` | +| Sift gRPC endpoint URI. | — | `sift_grpc_uri` | `SIFT_GRPC_URI` | +| Sift REST endpoint URI. | — | `sift_rest_uri` | `SIFT_REST_URI` | +| Sift web-app origin for the report link in the terminal footer (e.g. https://app.siftstack.com). When unset, the link is derived from the REST URI for known Sift hosts. | — | `sift_app_url` | `SIFT_APP_URL` | ### Report content @@ -179,6 +180,7 @@ suggestion, so typos like `SIFT_REPORT_SERIALNUM` surface immediately. | Serial number of the unit under test. | `[tool.sift.pytest.report] serial_number` | `SIFT_REPORT_SERIAL_NUMBER` | | Part number of the unit under test. | `[tool.sift.pytest.report] part_number` | `SIFT_REPORT_PART_NUMBER` | | Free-form report metadata, as a TOML table of scalar values. For dynamic per-run keys, override the sift_report_metadata fixture in conftest. | `[tool.sift.pytest.report.metadata]` (table) | — | + ### Quick-start examples diff --git a/python/lib/sift_client/_internal/credentials.py b/python/lib/sift_client/_internal/credentials.py new file mode 100644 index 000000000..e8c24d949 --- /dev/null +++ b/python/lib/sift_client/_internal/credentials.py @@ -0,0 +1,383 @@ +"""Resolution of Sift credentials from arguments, environment, and ``sift.toml``. + +``sift-cli`` stores one or more named profiles in a ``sift.toml`` file in the +user's config directory, and selects between them with ``--profile``. This +module reads that same file. A developer who runs ``sift-cli --profile staging`` +gets the same endpoints from ``SiftClient(profile="staging")`` and does not +restate them. + +The resolution order, highest precedence first: + +1. Inline arguments (``SiftClient(api_key=...)``), per field. +2. The fields of a profile named explicitly via ``profile=``. +3. Per-field environment variables (``SIFT_API_KEY``, ``SIFT_GRPC_URI``, + ``SIFT_REST_URI``, ``SIFT_APP_URL``). +4. The fields of the profile named by ``SIFT_PROFILE``. +5. The default (top-level) table of the config file. + +A profile that the caller names outranks the environment variables, so an +argument wins over a shell that points somewhere else. ``SIFT_PROFILE`` does +not outrank them, so a per-field environment override still works in CI. + +The resolver reads one profile table at most. A named profile does not inherit +missing fields from the top-level table, which matches ``sift-cli``. If a +profile omits ``api_key``, the resolver reports an error. It does not fall back +to the default profile's key. +""" + +from __future__ import annotations + +import os +import sys +import warnings +from dataclasses import dataclass +from pathlib import Path +from typing import Any, Mapping +from urllib.parse import urlparse + +# The ``[tool.sift]`` loader declares the 3.8-3.10 ``tomli`` fallback. Import it +# from there so only one module declares it. +from sift_client._internal.pyproject_config import tomllib +from sift_client.errors import SiftCredentialsError, SiftWarning + +CONFIG_FILE_NAME = "sift.toml" + +ENV_PROFILE = "SIFT_PROFILE" +ENV_CONFIG_FILE = "SIFT_CONFIG_FILE" +ENV_API_KEY = "SIFT_API_KEY" +ENV_GRPC_URI = "SIFT_GRPC_URI" +ENV_REST_URI = "SIFT_REST_URI" +ENV_APP_URL = "SIFT_APP_URL" + +# Accepted TOML keys -> (public field name, environment variable). The first +# TOML key is canonical. The rest are older spellings, which stay valid so that +# files from earlier releases still work. The environment names are the ones the +# pytest plugin ships. They differ from the TOML keys for the app URL, and both +# names stay. +_FIELDS = ( + (("grpc_uri",), "grpc_url", ENV_GRPC_URI), + (("rest_uri",), "rest_url", ENV_REST_URI), + (("app_uri",), "app_url", ENV_APP_URL), + (("api_key", "apikey"), "api_key", ENV_API_KEY), +) + +#: Every ``SIFT_*`` variable that this module reads. The pytest plugin adds this +#: list to its own registry, so its unknown-variable warning does not report one +#: of these names as a typo. +CREDENTIAL_ENV_VARS = (ENV_PROFILE, ENV_CONFIG_FILE, *(env for _, _, env in _FIELDS)) + +_REQUIRED = ("grpc_url", "rest_url", "api_key") + +_CLI_NAME = "sift-cli" + + +@dataclass(frozen=True) +class ResolvedCredentials: + """Credentials resolved from arguments, environment, and config file. + + ``sources`` maps each field name to the layer that supplied it: ``"arg"``, + ``"profile:"``, ``"env"``, ``"default"``, or ``"unset"``. Use it to + find which environment the client connects to. You then do not have to + work through the precedence order by hand. + """ + + api_key: str + grpc_url: str + rest_url: str + app_url: str | None + use_ssl: bool + profile: str | None + sources: Mapping[str, str] + + +def user_config_dir(env: Mapping[str, str] | None = None) -> Path | None: + """The directory ``sift-cli`` stores ``sift.toml`` in. + + This function matches Rust's ``dirs::config_dir()``, which ``sift-cli`` + uses. That directory is ``%APPDATA%`` on Windows and + ``~/Library/Application Support`` on macOS. Elsewhere it is + ``$XDG_CONFIG_HOME`` when that path is absolute, and ``~/.config`` if not. + Returns ``None`` if this function cannot find the home directory. + + Note: ``platformdirs`` is not used here. It appends an application name by + default, which puts the file in a directory that ``sift-cli`` never + reads. + """ + environ = os.environ if env is None else env + + if sys.platform == "win32": + appdata = environ.get("APPDATA") + return Path(appdata) if appdata else None + + if sys.platform == "darwin": + home = _home(environ) + return home / "Library" / "Application Support" if home else None + + xdg = environ.get("XDG_CONFIG_HOME") + if xdg and os.path.isabs(xdg): + return Path(xdg) + home = _home(environ) + return home / ".config" if home else None + + +def _home(environ: Mapping[str, str]) -> Path | None: + home = environ.get("HOME") or environ.get("USERPROFILE") + if home: + return Path(home) + try: + return Path.home() + except (RuntimeError, OSError): + return None + + +def config_file_path( + config_path: str | None = None, + env: Mapping[str, str] | None = None, +) -> Path | None: + """Where to look for ``sift.toml``. + + An explicit path takes precedence, then ``SIFT_CONFIG_FILE``, then the user + config directory. + + Note: this function does not search the current working directory. A + ``sift.toml`` file in a checkout can supply an API key, so that behavior + needs its own decision before it ships. + """ + if config_path is not None: + return Path(config_path) + environ = os.environ if env is None else env + from_env = environ.get(ENV_CONFIG_FILE) + if from_env: + return Path(from_env) + base = user_config_dir(environ) + return base / CONFIG_FILE_NAME if base else None + + +def _load_config(path: Path | None) -> dict[str, Any]: + """Parse the config file, or return ``{}`` if there is no file. + + A missing file is not an error on its own, because arguments or environment + variables can supply every field. A file that exists but that this function + cannot read or parse does raise. If it did not raise, the caller would later + see a confusing "credentials missing" error instead of the syntax error. + """ + if path is None: + return {} + try: + with path.open("rb") as fh: + return tomllib.load(fh) + except FileNotFoundError: + return {} + except OSError as exc: + raise SiftCredentialsError(f"Failed to read Sift config file '{path}': {exc}") from exc + except tomllib.TOMLDecodeError as exc: + raise SiftCredentialsError( + f"Sift config file '{path}' is not valid TOML: {exc}. " + f"Run `{_CLI_NAME} config show` to inspect it." + ) from exc + + +def _profile_names(config: Mapping[str, Any]) -> list[str]: + return sorted(key for key, value in config.items() if isinstance(value, dict)) + + +def _profile_table( + config: Mapping[str, Any], + name: str, + path: Path | None, +) -> Mapping[str, Any]: + table = config.get(name) + if isinstance(table, dict): + return table + + location = f"'{path}'" if path else "the Sift config file" + if not config: + raise SiftCredentialsError( + f"Profile '{name}' was requested but no Sift config file was found at {location}. " + f"Create one with `{_CLI_NAME} config create`, then " + f"`{_CLI_NAME} config update --profile {name}`." + ) + available = _profile_names(config) + known = ", ".join(available) if available else "none" + raise SiftCredentialsError( + f"Profile '{name}' was not found in {location}. Profiles defined there: {known}. " + f"Add it with `{_CLI_NAME} config update --profile {name}`." + ) + + +def _first_present(table: Mapping[str, Any], keys: tuple[str, ...]) -> Any: + """The value of the first key in ``keys`` that ``table`` sets.""" + for key in keys: + value = table.get(key) + if _str_or_none(value) is not None: + return value + return None + + +def _str_or_none(value: Any) -> str | None: + """Return a layer's value. Empty and non-string values count as absent.""" + if isinstance(value, str) and value: + return value + return None + + +def _derive_use_ssl(grpc_url: str, rest_url: str) -> bool: + """Infer transport security from the gRPC URL's scheme. + + ``sift_py`` removes the scheme from the URI. It then selects plaintext or + TLS from ``use_ssl`` alone. Without this function, a profile that holds + ``http://localhost:50051`` gets a TLS connection to a plaintext port, and + the connection fails. A bare host with no scheme keeps the TLS default. + """ + grpc_scheme = urlparse(grpc_url).scheme + rest_scheme = urlparse(rest_url).scheme + use_ssl = grpc_scheme != "http" + + if grpc_scheme and rest_scheme and grpc_scheme != rest_scheme: + warnings.warn( + f"Sift gRPC URL uses '{grpc_scheme}://' but the REST URL uses " + f"'{rest_scheme}://'. Both connections will use " + f"{'TLS' if use_ssl else 'plaintext'}, following the gRPC URL.", + SiftWarning, + stacklevel=3, + ) + return use_ssl + + +def _select_profile( + profile: str | None, + environ: Mapping[str, str], +) -> tuple[str | None, bool]: + """The profile to read, and whether the caller named it.""" + if profile: + return profile, True + return environ.get(ENV_PROFILE) or None, False + + +def resolve_credentials( + api_key: str | None = None, + grpc_url: str | None = None, + rest_url: str | None = None, + app_url: str | None = None, + profile: str | None = None, + config_path: str | None = None, + env: Mapping[str, str] | None = None, + require: bool = True, +) -> ResolvedCredentials: + """Resolve Sift credentials across arguments, environment, and ``sift.toml``. + + Args: + api_key: Explicit API key, overriding every other layer. + grpc_url: Explicit gRPC endpoint, overriding every other layer. + rest_url: Explicit REST endpoint, overriding every other layer. + app_url: Explicit Sift web-app origin, overriding every other layer. + profile: Name of a profile in the config file. It outranks the + per-field environment variables. See the module docstring. + config_path: Path to a specific config file, bypassing discovery. + env: Environment mapping to read, defaulting to ``os.environ``. + require: If ``True``, raise when the API key or either URL is still + missing. Pass ``False`` to resolve every field that is available + and leave the rest empty. The pytest plugin uses ``False`` for its + offline mode. + + Returns: + The resolved credentials, including which layer supplied each field. + + Raises: + SiftCredentialsError: This function cannot read or parse the config + file, the named profile does not exist, or ``require`` is ``True`` + and a required field is still missing. + """ + environ = os.environ if env is None else env + profile_name, profile_is_explicit = _select_profile(profile, environ) + + path = config_file_path(config_path, environ) + config = _load_config(path) + + if profile_name is not None: + table: Mapping[str, Any] = _profile_table(config, profile_name, path) + file_source = f"profile:{profile_name}" + else: + table = config + file_source = "default" + + # Each layer uses the field name as its key. The choice of a value therefore + # does not depend on the layer that holds it. + arg_layer = {"grpc_url": grpc_url, "rest_url": rest_url, "app_url": app_url, "api_key": api_key} + env_layer = {field: environ.get(env_key) for _, field, env_key in _FIELDS} + file_layer = {field: _first_present(table, toml_keys) for toml_keys, field, _ in _FIELDS} + + # Highest precedence first. A profile that the caller names outranks the + # environment. A profile that SIFT_PROFILE names does not. + if profile_is_explicit: + layers = [("arg", arg_layer), (file_source, file_layer), ("env", env_layer)] + else: + layers = [("arg", arg_layer), ("env", env_layer), (file_source, file_layer)] + + resolved: dict[str, str] = {} + sources: dict[str, str] = {} + for _, field_name, _ in _FIELDS: + for source, layer in layers: + value = _str_or_none(layer.get(field_name)) + if value is not None: + resolved[field_name] = value + sources[field_name] = source + break + else: + resolved[field_name] = "" + sources[field_name] = "unset" + + if require: + missing = [name for name in _REQUIRED if not resolved[name]] + if missing: + raise SiftCredentialsError( + _missing_message(missing, profile_name, profile_is_explicit, path, config) + ) + + return ResolvedCredentials( + api_key=resolved["api_key"], + grpc_url=resolved["grpc_url"], + rest_url=resolved["rest_url"], + app_url=resolved["app_url"] or None, + use_ssl=_derive_use_ssl(resolved["grpc_url"], resolved["rest_url"]), + profile=profile_name, + sources=sources, + ) + + +def _missing_message( + missing: list[str], + profile_name: str | None, + profile_is_explicit: bool, + path: Path | None, + config: Mapping[str, Any], +) -> str: + """Explain what is missing, where the resolver looked, and how to supply it.""" + env_names = {field_name: env_key for _, field_name, env_key in _FIELDS} + wanted = ", ".join(env_names[name] for name in missing) + + if profile_name is not None: + origin = "--profile/profile=" if profile_is_explicit else ENV_PROFILE + looked = f"profile '{profile_name}' (from {origin}) in '{path}'" + fix = f"`{_CLI_NAME} config update --profile {profile_name}`" + elif path is not None: + looked = f"the default profile in '{path}'" + fix = f"`{_CLI_NAME} config update`" + else: + looked = "the environment (no config file location could be determined)" + fix = f"`{_CLI_NAME} config create`" + + lines = [ + f"Sift credentials incomplete. Missing: {wanted}.", + f"Looked in: {looked}, then the environment.", + ] + if profile_name is None and (available := _profile_names(config)): + lines.append( + f"Named profiles in that file: {', '.join(available)}. " + "Select one with profile= or SIFT_PROFILE=." + ) + lines.append( + f"Set them with {fix}, export {wanted}, " + "or pass api_key/grpc_url/rest_url to SiftClient directly." + ) + return " ".join(lines) diff --git a/python/lib/sift_client/_internal/pytest_plugin/options.py b/python/lib/sift_client/_internal/pytest_plugin/options.py index 461c0c93b..6514866f1 100644 --- a/python/lib/sift_client/_internal/pytest_plugin/options.py +++ b/python/lib/sift_client/_internal/pytest_plugin/options.py @@ -20,6 +20,14 @@ logger = logging.getLogger(__name__) +from sift_client._internal.credentials import ( + CREDENTIAL_ENV_VARS, + ENV_API_KEY, + ENV_APP_URL, + ENV_GRPC_URI, + ENV_PROFILE, + ENV_REST_URI, +) from sift_client._internal.pyproject_config import load_tool_sift # Settings-reference categories. Each maps to a docs subsection and, in the @@ -55,7 +63,7 @@ class Option: A setting may come from an env var, a CLI flag, a pytest ini key, or a ``[tool.sift...]`` TOML path. :meth:`resolve` walks the declared surfaces in - env > cli > ini > toml order; ``metadata`` (``merge=True``) is the one + ``surfaces`` order; ``metadata`` (``merge=True``) is the one free-form table, resolved by :meth:`resolve_merged`. The single ``PLUGIN_OPTIONS`` registry of these drives ``pytest_addoption``, the resolvers, the docs settings-reference table, and the typo detector. @@ -67,6 +75,9 @@ class Option: - ``toml``: tuple path under ``[tool.sift...]``, e.g. ``("pytest", "report", "name")`` -> ``tool.sift.pytest.report.name``. - ``env``: full env var name, e.g. ``"SIFT_API_KEY"``. + - ``surfaces``: the precedence order. The default puts env before cli. + Override it if a flag that the user types must outrank an environment + variable, as ``profile`` does. ``category`` groups the option in the docs reference (one of ``CATEGORIES``). """ @@ -82,6 +93,7 @@ class Option: toml: tuple[str, ...] | None = None env: str | None = None merge: bool = False + surfaces: tuple[str, ...] = ("env", "cli", "ini", "toml") @property def cli_dest(self) -> str: @@ -109,12 +121,16 @@ def __post_init__(self) -> None: raise ValueError(f"Option({self.name!r}): declares no surfaces") if self.category not in CATEGORIES: raise ValueError(f"Option({self.name!r}): category must be one of {CATEGORIES}") + if set(self.surfaces) != {"env", "cli", "ini", "toml"}: + raise ValueError( + f"Option({self.name!r}): surfaces must be a permutation of " + "('env', 'cli', 'ini', 'toml')" + ) def resolve(self, config: pytest.Config | None) -> Any: """First set value from declared surfaces; ``None`` when unset everywhere. - Walk order is env > cli > ini > toml. No current option declares both - env and cli, so the chain isn't ambiguous in practice. + The walk order is :attr:`surfaces`, which puts env before cli by default. ``getini`` returns the typed default for unset bool/list keys, so this only returns ini values for booleans (always meaningful), non-empty strings, and non-empty lists. @@ -125,35 +141,43 @@ def resolve_with_source(self, config: pytest.Config | None) -> tuple[Any, str]: """Like :meth:`resolve`, but also reports which surface set the value. Returns ``(value, source)`` where ``source`` is one of - ``env``/``cli``/``ini``/``toml``, or ``default`` when nothing set it - (``value`` is then ``None``). Used by the audit log's settings snapshot. + ``env``/``cli``/``ini``/``toml``, or ``default`` if nothing set it + (``value`` is then ``None``). The audit log's settings snapshot calls + this method, so it always reports the surface that the run used. """ - if self.env: + for surface in self.surfaces: + value = self._read_surface(surface, config) + if value is not None: + return value, surface + return None, "default" + + def _read_surface(self, surface: str, config: pytest.Config | None) -> Any: + """This option's value from one surface, or ``None`` when unset there.""" + if surface == "env": + if not self.env: + return None env_value = os.getenv(self.env) - if env_value not in (None, ""): - return env_value, "env" + return env_value if env_value else None if config is None: - return None, "default" - if self.cli: - cli_value = config.getoption(self.cli_dest, default=None) - if cli_value is not None: - return cli_value, "cli" - if self.ini: + return None + if surface == "cli": + return config.getoption(self.cli_dest, default=None) if self.cli else None + if surface == "ini": + if not self.ini: + return None try: ini_value = config.getini(self.ini) except (KeyError, ValueError): - ini_value = None + return None if isinstance(ini_value, bool): - return ini_value, "ini" - if isinstance(ini_value, str) and ini_value: - return ini_value, "ini" - if isinstance(ini_value, list) and ini_value: - return ini_value, "ini" - if self.toml: - toml_value = _walk_toml(tool_sift(config), self.toml) - if toml_value not in (None, ""): - return toml_value, "toml" - return None, "default" + return ini_value + if isinstance(ini_value, (str, list)) and ini_value: + return ini_value + return None + if not self.toml: + return None + toml_value = _walk_toml(tool_sift(config), self.toml) + return toml_value if toml_value not in (None, "") else None def resolve_merged(self, config: pytest.Config | None) -> dict[str, str | float | bool]: """For ``merge=True`` dict-shape settings: the free-form TOML table. @@ -335,25 +359,36 @@ def _walk_toml(data: dict[str, Any], path: tuple[str, ...]) -> Any: ini_default=True, ) -# Credentials. The API key is env-only; the URIs accept env + ini. +# Credentials. The API key has no ini key. The URIs accept env and ini. A +# profile supplies the values that the other three leave unset, from the same +# sift.toml that `sift-cli --profile` reads. +PROFILE_OPTION = Option( + name="profile", + category=CAT_CONNECTION, + help="Name of a sift.toml profile that supplies credentials, as with `sift-cli --profile`.", + cli="--sift-profile", + env=ENV_PROFILE, + ini="sift_profile", + surfaces=("cli", "env", "ini", "toml"), +) API_KEY_OPTION = Option( name="api_key", category=CAT_CONNECTION, help="Sift API key (secret, env-only).", - env="SIFT_API_KEY", + env=ENV_API_KEY, ) GRPC_URI_OPTION = Option( name="grpc_uri", category=CAT_CONNECTION, help="Sift gRPC endpoint URI.", - env="SIFT_GRPC_URI", + env=ENV_GRPC_URI, ini="sift_grpc_uri", ) REST_URI_OPTION = Option( name="rest_uri", category=CAT_CONNECTION, help="Sift REST endpoint URI.", - env="SIFT_REST_URI", + env=ENV_REST_URI, ini="sift_rest_uri", ) APP_URL_OPTION = Option( @@ -362,7 +397,7 @@ def _walk_toml(data: dict[str, Any], path: tuple[str, ...]) -> Any: help="Sift web-app origin for the report link in the terminal footer (e.g. " "https://app.siftstack.com). When unset, the link is derived from the REST URI " "for known Sift hosts.", - env="SIFT_APP_URL", + env=ENV_APP_URL, ini="sift_app_url", ) @@ -432,6 +467,7 @@ def _walk_toml(data: dict[str, Any], path: tuple[str, ...]) -> Any: MODULE_STEP_OPTION, CLASS_STEP_OPTION, PARAMETRIZE_NESTING_OPTION, + PROFILE_OPTION, API_KEY_OPTION, GRPC_URI_OPTION, REST_URI_OPTION, @@ -524,6 +560,7 @@ def _env_cell(opt: Option) -> str: ("Ini (`[tool.pytest.ini_options]`)", _ini_cell), ], CAT_CONNECTION: [ + ("CLI flag", _cli_cell), ("Ini (`[tool.pytest.ini_options]`)", _ini_cell), ("Env var", _env_cell), ], @@ -559,16 +596,18 @@ def _escape(cell: str) -> str: def warn_on_unknown_env_vars() -> None: - """Emit a warning for any ``SIFT_*`` env var not declared in the registry. + """Emit a warning for any ``SIFT_*`` env var that this plugin does not read. - The registry declares each env var by its full name (``opt.env``); a - ``SIFT_*`` var that matches none of them is almost always a typo. + The known names are the registry's (``opt.env``) and the credential + resolver's ``CREDENTIAL_ENV_VARS``. The resolver reads some names, such as + ``SIFT_CONFIG_FILE``, that the registry does not declare. A ``SIFT_*`` + variable in neither list is almost always a typo. """ import difflib from sift_client.pytest_plugin import SiftPytestPluginWarning - known_full = {opt.env for opt in PLUGIN_OPTIONS if opt.env} + known_full = {opt.env for opt in PLUGIN_OPTIONS if opt.env} | set(CREDENTIAL_ENV_VARS) suggestion_pool = sorted(known_full) for name in sorted(os.environ): if not name.startswith("SIFT_"): diff --git a/python/lib/sift_client/_tests/conftest.py b/python/lib/sift_client/_tests/conftest.py index 2272cc299..23f2dd7a6 100644 --- a/python/lib/sift_client/_tests/conftest.py +++ b/python/lib/sift_client/_tests/conftest.py @@ -33,6 +33,22 @@ def _isolate_default_disk_cache_path(monkeypatch, tmp_path): ) +@pytest.fixture(autouse=True) +def _isolate_sift_config_file(monkeypatch, tmp_path_factory): + """Point credential resolution at a config file that does not exist. + + ``SiftClient()`` without a ``connection_config`` reads ``sift.toml`` from + the user config directory. Without this fixture, a developer's real + ``~/.config/sift.toml`` supplies credentials that a test never set, and it + can point a test at a live backend. The plugin suite needs this fixture too, + because its inner sessions inherit this environment through + ``runpytest_subprocess``. + """ + absent = tmp_path_factory.mktemp("sift-config") / "absent.toml" + monkeypatch.setenv("SIFT_CONFIG_FILE", str(absent)) + monkeypatch.delenv("SIFT_PROFILE", raising=False) + + @pytest.fixture(scope="session") def sift_client() -> SiftClient: """Create a SiftClient instance for testing. diff --git a/python/lib/sift_client/_tests/pytest_plugin/conftest.py b/python/lib/sift_client/_tests/pytest_plugin/conftest.py index 62569d0c4..96d396634 100644 --- a/python/lib/sift_client/_tests/pytest_plugin/conftest.py +++ b/python/lib/sift_client/_tests/pytest_plugin/conftest.py @@ -29,7 +29,15 @@ import pytest -_SIFT_ENV_VARS = ("SIFT_API_KEY", "SIFT_GRPC_URI", "SIFT_REST_URI", "SIFT_DISABLED", "SIFT_APP_URL") +_SIFT_ENV_VARS = ( + "SIFT_API_KEY", + "SIFT_GRPC_URI", + "SIFT_REST_URI", + "SIFT_DISABLED", + "SIFT_APP_URL", + "SIFT_PROFILE", + "SIFT_CONFIG_FILE", +) def pytest_collection_modifyitems(items: list[pytest.Item]) -> None: diff --git a/python/lib/sift_client/_tests/pytest_plugin/test_credentials.py b/python/lib/sift_client/_tests/pytest_plugin/test_credentials.py index 3f6d22a6e..3a3436784 100644 --- a/python/lib/sift_client/_tests/pytest_plugin/test_credentials.py +++ b/python/lib/sift_client/_tests/pytest_plugin/test_credentials.py @@ -6,10 +6,9 @@ from __future__ import annotations -from typing import TYPE_CHECKING, Callable +from typing import Callable -if TYPE_CHECKING: - import pytest +import pytest class TestCredentials: @@ -115,3 +114,119 @@ def test_missing_credentials_named_in_error( combined = "\n".join(result.outlines + result.errlines) for name in ("SIFT_API_KEY", "SIFT_GRPC_URI", "SIFT_REST_URI"): assert name in combined, combined + + +_PROFILE_CONFIG = """\ +grpc_uri = "https://grpc.default.example" +rest_uri = "https://rest.default.example" +apikey = "default-key" + +[staging] +grpc_uri = "https://grpc.staging.example" +rest_uri = "https://rest.staging.example" +apikey = "staging-key" + +[other] +grpc_uri = "https://grpc.other.example" +rest_uri = "https://rest.other.example" +apikey = "other-key" +""" + + +class TestProfiles: + """The fixture's use of ``sift.toml`` profiles via ``--sift-profile``.""" + + @staticmethod + def _write_config(pytester: pytest.Pytester, monkeypatch: pytest.MonkeyPatch) -> None: + config = pytester.path / "sift.toml" + config.write_text(_PROFILE_CONFIG) + monkeypatch.setenv("SIFT_CONFIG_FILE", str(config)) + for name in ("SIFT_API_KEY", "SIFT_GRPC_URI", "SIFT_REST_URI", "SIFT_PROFILE"): + monkeypatch.delenv(name, raising=False) + + @pytest.mark.parametrize( + ("extra_env", "cli_args", "expected_key"), + [ + pytest.param({}, ("--sift-profile", "staging"), "staging-key", id="named-profile"), + pytest.param({}, (), "default-key", id="default-profile"), + pytest.param( + {"SIFT_API_KEY": "ci-key"}, + ("--sift-profile", "staging"), + "ci-key", + id="env-beats-profile", + ), + pytest.param( + {"SIFT_PROFILE": "staging"}, + ("--sift-profile", "other"), + "other-key", + id="cli-beats-env-profile", + ), + ], + ) + def test_profile_selection( + self, + pytester: pytest.Pytester, + monkeypatch: pytest.MonkeyPatch, + write_plugin_conftest: Callable[[], None], + extra_env: dict[str, str], + cli_args: tuple[str, ...], + expected_key: str, + ) -> None: + """Which profile supplies the API key, across the surfaces that can name one. + + ``env-beats-profile`` is the plugin's deliberate difference from + ``SiftClient``: a key injected by CI outranks the profile, so a profile + on the runner can never silently replace it. + """ + self._write_config(pytester, monkeypatch) + for name, value in extra_env.items(): + monkeypatch.setenv(name, value) + write_plugin_conftest() + pytester.makepyfile( + f""" + def test_key(sift_client): + assert sift_client.grpc_client._config.api_key == {expected_key!r} + """ + ) + result = pytester.runpytest_subprocess(*cli_args, "--sift-offline") + result.assert_outcomes(passed=1) + + def test_profile_from_ini_key( + self, + pytester: pytest.Pytester, + monkeypatch: pytest.MonkeyPatch, + write_plugin_conftest: Callable[[], None], + ) -> None: + """`sift_profile` in pyproject selects the profile without a CLI flag.""" + self._write_config(pytester, monkeypatch) + write_plugin_conftest() + pytester.makepyprojecttoml( + """ + [tool.pytest.ini_options] + sift_profile = "staging" + sift_offline = true + """ + ) + pytester.makepyfile( + """ + def test_from_ini_profile(sift_client): + assert sift_client.grpc_client._config.api_key == "staging-key" + """ + ) + result = pytester.runpytest_subprocess() + result.assert_outcomes(passed=1) + + def test_unknown_profile_is_a_usage_error( + self, + pytester: pytest.Pytester, + monkeypatch: pytest.MonkeyPatch, + write_plugin_conftest: Callable[[], None], + ) -> None: + """A named profile that the file does not define aborts and lists the ones it does.""" + self._write_config(pytester, monkeypatch) + write_plugin_conftest() + pytester.makepyfile("def test_should_not_run(): pass") + result = pytester.runpytest_subprocess("--sift-profile", "nope", "--sift-offline") + assert result.ret != 0 + combined = "\n".join(result.outlines + result.errlines) + assert "staging" in combined, combined diff --git a/python/lib/sift_client/_tests/pytest_plugin/test_typo_detector.py b/python/lib/sift_client/_tests/pytest_plugin/test_typo_detector.py index 435170ed5..b5bfa7fbb 100644 --- a/python/lib/sift_client/_tests/pytest_plugin/test_typo_detector.py +++ b/python/lib/sift_client/_tests/pytest_plugin/test_typo_detector.py @@ -111,3 +111,23 @@ def test_metadata_subtree_keys_are_user_defined( result = pytester.runpytest_subprocess("--sift-disabled") combined = "\n".join(result.outlines + result.errlines) assert "Unknown sift config key" not in combined, combined + + def test_credential_resolver_env_vars_are_known( + self, + pytester: pytest.Pytester, + clear_sift_env: None, + monkeypatch: pytest.MonkeyPatch, + write_plugin_conftest: Callable[[], None], + ) -> None: + """``SIFT_CONFIG_FILE`` is honored by the resolver, so it must not read as a typo. + + The registry does not declare it. The known set therefore includes the + credential resolver's own variables, so the warning cannot tell a user + that the run ignored a variable that it read. + """ + monkeypatch.setenv("SIFT_CONFIG_FILE", str(pytester.path / "absent.toml")) + write_plugin_conftest() + pytester.makepyfile("def test_runs(): pass") + result = pytester.runpytest_subprocess("--sift-disabled") + combined = "\n".join(result.outlines + result.errlines) + assert "SIFT_CONFIG_FILE" not in combined, combined diff --git a/python/lib/sift_client/_tests/test_credentials.py b/python/lib/sift_client/_tests/test_credentials.py new file mode 100644 index 000000000..1c7196014 --- /dev/null +++ b/python/lib/sift_client/_tests/test_credentials.py @@ -0,0 +1,286 @@ +"""Tests for credential resolution across arguments, environment, and sift.toml.""" + +from __future__ import annotations + +import sys + +import pytest + +from sift_client._internal.credentials import config_file_path, user_config_dir +from sift_client.credentials import resolve_credentials +from sift_client.errors import SiftCredentialsError + +CONFIG = """\ +grpc_uri = "https://grpc.default.example" +rest_uri = "https://rest.default.example" +app_uri = "https://app.default.example" +apikey = "default-key" + +[staging] +grpc_uri = "https://grpc.staging.example" +rest_uri = "https://rest.staging.example" +app_uri = "https://app.staging.example" +apikey = "staging-key" + +[localdev] +grpc_uri = "http://localhost:50051" +rest_uri = "http://localhost:8080" +apikey = "local-key" + +[partial] +grpc_uri = "https://grpc.partial.example" +""" + + +@pytest.fixture +def config_file(tmp_path): + path = tmp_path / "sift.toml" + path.write_text(CONFIG) + return str(path) + + +def resolve(config_file=None, env=None, **kwargs): + """Resolve against an isolated environment so the host's config never leaks in.""" + return resolve_credentials(config_path=config_file, env=env or {}, **kwargs) + + +class TestProfileSelection: + def test_default_profile_is_top_level_table(self, config_file): + creds = resolve(config_file) + assert creds.grpc_url == "https://grpc.default.example" + assert creds.api_key == "default-key" + assert creds.profile is None + assert creds.sources["api_key"] == "default" + + def test_named_profile(self, config_file): + creds = resolve(config_file, profile="staging") + assert creds.grpc_url == "https://grpc.staging.example" + assert creds.api_key == "staging-key" + assert creds.profile == "staging" + assert creds.sources["api_key"] == "profile:staging" + + def test_profile_from_env(self, config_file): + creds = resolve(config_file, env={"SIFT_PROFILE": "staging"}) + assert creds.api_key == "staging-key" + assert creds.profile == "staging" + + def test_profile_argument_beats_env_profile(self, config_file): + creds = resolve(config_file, profile="localdev", env={"SIFT_PROFILE": "staging"}) + assert creds.profile == "localdev" + assert creds.api_key == "local-key" + + def test_named_profile_does_not_inherit_from_default(self, config_file): + """A profile missing a key is an error, not a silent fall back to the default's.""" + with pytest.raises(SiftCredentialsError) as exc: + resolve(config_file, profile="partial") + assert "SIFT_REST_URI" in str(exc.value) + assert "SIFT_API_KEY" in str(exc.value) + + def test_unknown_profile_lists_the_known_ones(self, config_file): + with pytest.raises(SiftCredentialsError) as exc: + resolve(config_file, profile="nope") + message = str(exc.value) + assert "'nope' was not found" in message + assert "localdev, partial, staging" in message + + def test_unknown_profile_without_config_file_says_so(self, tmp_path): + with pytest.raises(SiftCredentialsError) as exc: + resolve(str(tmp_path / "absent.toml"), profile="staging") + assert "no Sift config file was found" in str(exc.value) + + +class TestPrecedence: + def test_arguments_win_over_everything(self, config_file): + creds = resolve( + config_file, + profile="staging", + env={"SIFT_API_KEY": "env-key"}, + api_key="arg-key", + ) + assert creds.api_key == "arg-key" + assert creds.sources["api_key"] == "arg" + + def test_named_profile_beats_environment(self, config_file): + """Naming a profile is explicit, so it outranks an ambient env var.""" + creds = resolve( + config_file, + profile="staging", + env={"SIFT_GRPC_URI": "https://grpc.env.example"}, + ) + assert creds.grpc_url == "https://grpc.staging.example" + assert creds.sources["grpc_url"] == "profile:staging" + + def test_environment_beats_env_named_profile(self, config_file): + """SIFT_PROFILE is ambient too, so per-field env vars still override it.""" + creds = resolve( + config_file, + env={"SIFT_PROFILE": "staging", "SIFT_GRPC_URI": "https://grpc.env.example"}, + ) + assert creds.grpc_url == "https://grpc.env.example" + assert creds.sources["grpc_url"] == "env" + assert creds.api_key == "staging-key" + assert creds.sources["api_key"] == "profile:staging" + + def test_environment_beats_default_profile(self, config_file): + creds = resolve(config_file, env={"SIFT_API_KEY": "env-key"}) + assert creds.api_key == "env-key" + assert creds.sources["api_key"] == "env" + assert creds.grpc_url == "https://grpc.default.example" + + def test_partial_environment_override_keeps_profile_fields(self, config_file): + """The CI case: profile endpoints, key injected from a secret store.""" + creds = resolve(config_file, profile="staging", env={"SIFT_API_KEY": "ci-key"}) + assert creds.grpc_url == "https://grpc.staging.example" + assert creds.api_key == "staging-key" + + creds = resolve(config_file, env={"SIFT_PROFILE": "staging", "SIFT_API_KEY": "ci-key"}) + assert creds.grpc_url == "https://grpc.staging.example" + assert creds.api_key == "ci-key" + + def test_app_url_environment_name_differs_from_toml_key(self, config_file): + creds = resolve(config_file, env={"SIFT_APP_URL": "https://app.env.example"}) + assert creds.app_url == "https://app.env.example" + creds = resolve(config_file) + assert creds.app_url == "https://app.default.example" + + def test_empty_values_are_treated_as_absent(self, config_file): + creds = resolve(config_file, api_key="", env={"SIFT_API_KEY": ""}) + assert creds.api_key == "default-key" + assert creds.sources["api_key"] == "default" + + +class TestApiKeySpelling: + """``api_key`` is canonical; ``apikey`` stays accepted for existing files.""" + + def test_canonical_api_key(self, tmp_path): + path = tmp_path / "sift.toml" + path.write_text( + 'grpc_uri = "https://g.example"\n' + 'rest_uri = "https://r.example"\n' + 'api_key = "canonical"\n' + ) + assert resolve(str(path)).api_key == "canonical" + + def test_legacy_apikey_still_accepted(self, config_file): + """The shared fixture uses `apikey`, so this is the migration path.""" + assert resolve(config_file).api_key == "default-key" + + def test_canonical_wins_when_a_file_carries_both(self, tmp_path): + """A file that someone edits by hand can hold both keys. + + `sift-cli config update` never writes both. If both are present, the + canonical spelling decides. + """ + path = tmp_path / "sift.toml" + path.write_text( + 'grpc_uri = "https://g.example"\n' + 'rest_uri = "https://r.example"\n' + 'api_key = "canonical"\n' + 'apikey = "legacy"\n' + ) + assert resolve(str(path)).api_key == "canonical" + + def test_legacy_key_in_a_named_profile(self, config_file): + assert resolve(config_file, profile="staging").api_key == "staging-key" + + +class TestUseSsl: + def test_https_profile_uses_tls(self, config_file): + assert resolve(config_file, profile="staging").use_ssl is True + + def test_http_profile_disables_tls(self, config_file): + """Without this the transport would dial a plaintext port over TLS.""" + creds = resolve(config_file, profile="localdev") + assert creds.use_ssl is False + + def test_bare_host_keeps_the_tls_default(self, config_file): + creds = resolve(config_file, grpc_url="grpc.example:443", rest_url="rest.example") + assert creds.use_ssl is True + + def test_mismatched_schemes_warn_and_follow_grpc(self, config_file): + from sift_client.errors import SiftWarning + + with pytest.warns(SiftWarning, match="REST URL"): + creds = resolve( + config_file, + grpc_url="http://localhost:50051", + rest_url="https://rest.example", + ) + assert creds.use_ssl is False + + +class TestMissingAndMalformed: + def test_missing_everything_names_the_variables_and_the_fix(self, tmp_path): + with pytest.raises(SiftCredentialsError) as exc: + resolve(str(tmp_path / "absent.toml")) + message = str(exc.value) + assert "SIFT_GRPC_URI" in message + assert "SIFT_REST_URI" in message + assert "SIFT_API_KEY" in message + assert "sift-cli config update" in message + + def test_missing_message_lists_available_profiles(self, config_file, tmp_path): + stripped = tmp_path / "profiles-only.toml" + stripped.write_text('[staging]\ngrpc_uri = "https://g.example"\n') + with pytest.raises(SiftCredentialsError) as exc: + resolve(str(stripped)) + assert "Named profiles in that file: staging" in str(exc.value) + + def test_require_false_leaves_fields_empty(self, tmp_path): + creds = resolve(str(tmp_path / "absent.toml"), require=False) + assert creds.api_key == "" + assert creds.grpc_url == "" + assert creds.sources["api_key"] == "unset" + + def test_malformed_toml_raises_rather_than_falling_through(self, tmp_path): + bad = tmp_path / "sift.toml" + bad.write_text("grpc_uri = \nnot valid") + with pytest.raises(SiftCredentialsError, match="not valid TOML"): + resolve(str(bad), env={"SIFT_API_KEY": "k"}) + + def test_missing_file_is_not_an_error_when_env_supplies_everything(self, tmp_path): + creds = resolve( + str(tmp_path / "absent.toml"), + env={ + "SIFT_API_KEY": "k", + "SIFT_GRPC_URI": "https://g.example", + "SIFT_REST_URI": "https://r.example", + }, + ) + assert creds.api_key == "k" + assert creds.sources["grpc_url"] == "env" + + +class TestConfigDiscovery: + def test_explicit_path_wins(self, config_file): + assert str(config_file_path(config_file, {})) == config_file + + def test_env_var_overrides_the_config_directory(self, tmp_path): + target = tmp_path / "elsewhere.toml" + found = config_file_path(None, {"SIFT_CONFIG_FILE": str(target)}) + assert found == target + + def test_cwd_is_not_searched(self, tmp_path, monkeypatch): + """A sift.toml in a checkout must not be able to supply an API key.""" + monkeypatch.chdir(tmp_path) + (tmp_path / "sift.toml").write_text('apikey = "from-cwd"\n') + found = config_file_path(None, {"HOME": str(tmp_path / "home")}) + assert found != tmp_path / "sift.toml" + + @pytest.mark.skipif(sys.platform != "linux", reason="XDG layout is Linux-only") + def test_linux_uses_xdg_config_home_when_absolute(self, tmp_path): + assert user_config_dir({"XDG_CONFIG_HOME": str(tmp_path)}) == tmp_path + + @pytest.mark.skipif(sys.platform != "linux", reason="XDG layout is Linux-only") + def test_linux_ignores_relative_xdg_config_home(self, tmp_path): + found = user_config_dir({"XDG_CONFIG_HOME": "relative/path", "HOME": str(tmp_path)}) + assert found == tmp_path / ".config" + + @pytest.mark.skipif(sys.platform != "darwin", reason="macOS layout") + def test_macos_uses_application_support(self, tmp_path): + found = user_config_dir({"HOME": str(tmp_path)}) + assert found == tmp_path / "Library" / "Application Support" + + @pytest.mark.skipif(sys.platform != "win32", reason="Windows layout") + def test_windows_uses_appdata(self, tmp_path): + assert user_config_dir({"APPDATA": str(tmp_path)}) == tmp_path diff --git a/python/lib/sift_client/client.py b/python/lib/sift_client/client.py index b7c68b7a2..5c1c49551 100644 --- a/python/lib/sift_client/client.py +++ b/python/lib/sift_client/client.py @@ -2,10 +2,11 @@ import logging import warnings -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Mapping from sift_client._internal.disk_cache_config import DiskCacheConfig from sift_client._internal.urls import frontend_origin_for_api +from sift_client.credentials import ResolvedCredentials, resolve_credentials from sift_client.errors import SiftWarning from sift_client.resources import ( AssetsAPI, @@ -45,9 +46,7 @@ from sift_client.resources.access_control import AccessControlAPI, AccessControlAPIAsync from sift_client.transport import ( GrpcClient, - GrpcConfig, RestClient, - RestConfig, SiftConnectionConfig, WithGrpcClient, WithRestClient, @@ -73,6 +72,12 @@ class SiftClient( from sift_client import SiftClient from datetime import datetime + # Use the same credentials as sift-cli, from its default profile + client = SiftClient() + + # Or a named profile from sift.toml, like `sift-cli --profile staging` + client = SiftClient(profile="staging") + # Initialize with individual parameters client = SiftClient( api_key="your-api-key", @@ -154,44 +159,66 @@ def __init__( rest_url: str | None = None, connection_config: SiftConnectionConfig | None = None, app_url: str | None = None, + profile: str | None = None, ): """Initialize the SiftClient with specific connection parameters or a connection_config. + For any argument that you leave unset, the client reads the environment + and the ``sift.toml`` profiles that ``sift-cli`` manages. ``SiftClient()`` + therefore connects to the same endpoints as ``sift-cli``. See + :func:`sift_client.credentials.resolve_credentials` for the full + precedence order. + Args: api_key: The Sift API key for authentication. grpc_url: The Sift gRPC API URL. rest_url: The Sift REST API URL. connection_config: A SiftConnectionConfig object to configure the connection behavior of the SiftClient. + The client uses it unchanged and resolves no credentials. app_url: The Sift web-app origin (e.g. ``https://app.siftstack.com``). Set this for on-prem or custom deployments whose API host can't be mapped to a frontend automatically; see the ``app_url`` property. A value here takes precedence over ``connection_config.app_url``. + profile: Name of a ``sift.toml`` profile that supplies credentials, + as with ``sift-cli --profile``. The client ignores this argument + if you also pass ``connection_config``. + + Raises: + SiftCredentialsError: You passed no ``connection_config``, and the + client cannot resolve the API key or either URL. """ - if not (api_key and grpc_url and rest_url) and not connection_config: - raise ValueError( - "Either api_key, grpc_url and rest_url or connection_config must be provided to establish a connection." + self._credentials: ResolvedCredentials | None = None + + if connection_config is None: + creds = resolve_credentials( + api_key=api_key, + grpc_url=grpc_url, + rest_url=rest_url, + app_url=app_url, + profile=profile, ) - - if connection_config: - grpc_client = GrpcClient(connection_config.get_grpc_config()) - rest_client = RestClient(connection_config.get_rest_config()) - elif api_key and grpc_url and rest_url: - grpc_client = GrpcClient(GrpcConfig(grpc_url, api_key)) - rest_client = RestClient(RestConfig(rest_url, api_key)) - else: - raise ValueError( - "Invalid connection configuration. Please provide api_key, grpc_uri and rest_uri or a connection_config." + self._credentials = creds + # ``use_ssl`` comes from the scheme of the gRPC URL. The transport + # removes that scheme, so without this it connects to an ``http://`` + # endpoint over TLS. + connection_config = SiftConnectionConfig( + grpc_url=creds.grpc_url, + rest_url=creds.rest_url, + api_key=creds.api_key, + use_ssl=creds.use_ssl, + app_url=creds.app_url, ) + grpc_client = GrpcClient(connection_config.get_grpc_config()) + rest_client = RestClient(connection_config.get_rest_config()) + WithGrpcClient.__init__(self, grpc_client=grpc_client) WithRestClient.__init__(self, rest_client=rest_client) # Explicit web-app origin override; falls back to the connection config's # value, then to host-based derivation in the ``app_url`` property. - self._app_url: str | None = app_url or ( - connection_config.app_url if connection_config else None - ) + self._app_url: str | None = app_url or connection_config.app_url # When set, test-results writes return synthesized responses without # contacting Sift. Read by `TestResultsAPIAsync._simulate`. Used by the @@ -248,6 +275,40 @@ def __init__( data_import=DataImportAPIAsync(self), ) + @classmethod + def from_profile(cls, profile: str, **kwargs) -> SiftClient: + """Build a client from a named ``sift.toml`` profile. + + This method calls ``SiftClient(profile=...)``. It passes every keyword + argument through, and those arguments still outrank the profile's + values. + + Args: + profile: Profile name, as used by ``sift-cli --profile``. + **kwargs: Any other :class:`SiftClient` argument. + + Returns: + A client connected to the endpoints that profile names. + """ + return cls(profile=profile, **kwargs) + + @property + def credential_sources(self) -> Mapping[str, str] | None: + """The layer that supplied each credential. Use it to diagnose a connection. + + Maps ``api_key`` / ``grpc_url`` / ``rest_url`` / ``app_url`` to + ``"arg"``, ``"profile:"``, ``"env"``, ``"default"``, or + ``"unset"``. This property is ``None`` if you build the client from an + explicit ``connection_config``, because that path resolves no + credentials. + """ + return self._credentials.sources if self._credentials else None + + @property + def profile(self) -> str | None: + """The ``sift.toml`` profile that supplied this client's credentials.""" + return self._credentials.profile if self._credentials else None + @property def grpc_client(self) -> GrpcClient: """The gRPC client used by the SiftClient for making gRPC API calls.""" diff --git a/python/lib/sift_client/credentials.py b/python/lib/sift_client/credentials.py new file mode 100644 index 000000000..c5a2ba4a4 --- /dev/null +++ b/python/lib/sift_client/credentials.py @@ -0,0 +1,20 @@ +"""Credential resolution shared by ``SiftClient`` and the pytest plugin. + +This module reads the same ``sift.toml`` profiles that ``sift-cli --profile`` +uses. An environment that you configure once for the CLI is then available to +the Python client, and you do not restate its endpoints. See +:func:`resolve_credentials` for the precedence order. The Credentials and +profiles guide describes the config file. + +This module is the public surface. The implementation is in +``sift_client._internal.credentials``. +""" + +from __future__ import annotations + +from sift_client._internal.credentials import ResolvedCredentials, resolve_credentials + +__all__ = [ + "ResolvedCredentials", + "resolve_credentials", +] diff --git a/python/lib/sift_client/errors.py b/python/lib/sift_client/errors.py index 34ffb6677..657c6a948 100644 --- a/python/lib/sift_client/errors.py +++ b/python/lib/sift_client/errors.py @@ -11,6 +11,14 @@ class SiftExperimentalWarning(SiftWarning): """Warning for experimental features.""" +class SiftCredentialsError(ValueError): + """Raised when Sift credentials cannot be resolved. + + Subclasses ``ValueError`` because that is what ``SiftClient`` raised for + unusable connection arguments before credential resolution existed. + """ + + def _sift_stream_bindings_import_error(original_error: ImportError) -> NoReturn: # Returns NoReturn to satisfy pyright raise ImportError( diff --git a/python/lib/sift_client/pytest_plugin.py b/python/lib/sift_client/pytest_plugin.py index 3e2eda6d6..a143064ba 100644 --- a/python/lib/sift_client/pytest_plugin.py +++ b/python/lib/sift_client/pytest_plugin.py @@ -46,6 +46,7 @@ LOG_FILE_OPTION, OPEN_OPTION, OUTPUT_DIR_OPTION, + PROFILE_OPTION, REST_URI_OPTION, register_options, resolved_settings, @@ -80,7 +81,8 @@ write_disabled_summary, write_report_summary, ) -from sift_client.errors import SiftWarning +from sift_client.credentials import resolve_credentials +from sift_client.errors import SiftCredentialsError, SiftWarning from sift_client.sift_types.test_report import TestStatus from sift_client.util.test_results import ReportContext from sift_client.util.test_results.context_manager import NewStep @@ -183,36 +185,64 @@ def abort(reason: str, returncode: int | None = None) -> NoReturn: @pytest.fixture(scope="session") def sift_client(pytestconfig: pytest.Config) -> SiftClient: - """Default ``SiftClient`` resolved from environment variables and ini keys. - - Each credential is read from its environment variable first. The URIs - (``SIFT_GRPC_URI``, ``SIFT_REST_URI``) also fall back to the - ``sift_grpc_uri`` / ``sift_rest_uri`` ini keys, since they are stable - per-org values that are safe to commit. ``SIFT_API_KEY`` is intentionally - env-only; use ``pytest-dotenv`` (already a project dependency) to load - it from a ``.env`` file kept out of version control. - - Projects that need custom construction (TLS toggles, custom timeouts, - etc.) can override this fixture by defining their own ``sift_client`` - in their ``conftest.py``; pytest fixture resolution prefers the local - definition. - - In ``--sift-offline`` mode the missing-credential check is relaxed: - real env vars and ini values still win when set (so the client is - constructible against a real backend even though no calls are made), but - anything still missing is filled with a placeholder. In ``--sift-disabled`` - mode the credential resolution is skipped entirely and placeholders are - always used. + """Default ``SiftClient`` resolved from env vars, ini keys, and sift.toml profiles. + + This fixture reads each credential from its environment variable first. The + URIs (``SIFT_GRPC_URI``, ``SIFT_REST_URI``) also fall back to the + ``sift_grpc_uri`` / ``sift_rest_uri`` ini keys, because those are stable + per-org values that are safe to commit. ``SIFT_API_KEY`` has no ini key, by + design. Use ``pytest-dotenv``, which is already a project dependency, to + load it from a ``.env`` file that you keep out of version control. + + A ``sift.toml`` profile supplies the values that those surfaces leave unset. + It is the same file that ``sift-cli --profile`` reads. Name a profile with + ``--sift-profile``, the ``sift_profile`` ini key, or ``SIFT_PROFILE``. If you + name none, the fixture uses the file's default profile. + + Note: the precedence here differs from :class:`~sift_client.SiftClient`. The + profile ranks below the environment variables, not above them, so a profile + on the runner cannot replace a key that CI injects. + + A project that needs custom construction, such as a TLS toggle or a custom + timeout, can override this fixture. Declare your own ``sift_client`` in + ``conftest.py``, because pytest prefers the local definition. + + ``--sift-offline`` mode relaxes the missing-credential check. A real + environment variable or ini value still takes precedence when it is set, so + the client can point at a real backend even though it makes no calls. A + placeholder fills every field that is still missing. ``--sift-disabled`` + mode resolves no credentials and always uses placeholders. """ if is_disabled(pytestconfig): return build_disabled_client() + + offline = is_offline(pytestconfig) + # Every value that the plugin's own surfaces resolved goes in as an explicit + # argument, so it outranks the profile. The profile then supplies only the + # values that they left unset, which keeps a key from CI in effect. + try: + creds = resolve_credentials( + api_key=API_KEY_OPTION.resolve(pytestconfig), + grpc_url=GRPC_URI_OPTION.resolve(pytestconfig), + rest_url=REST_URI_OPTION.resolve(pytestconfig), + app_url=APP_URL_OPTION.resolve(pytestconfig), + profile=PROFILE_OPTION.resolve(pytestconfig), + require=False, + ) + except SiftCredentialsError as exc: + # A named profile that does not exist, or a config file that the resolver + # cannot read, is a usage error even offline, because the run asked for + # something specific. + log_event(logger, logging.ERROR, "credentials", error=type(exc).__name__) + raise pytest.UsageError(str(exc)) from exc + resolved = { - "SIFT_API_KEY": API_KEY_OPTION.resolve(pytestconfig), - "SIFT_GRPC_URI": GRPC_URI_OPTION.resolve(pytestconfig), - "SIFT_REST_URI": REST_URI_OPTION.resolve(pytestconfig), + "SIFT_API_KEY": creds.api_key, + "SIFT_GRPC_URI": creds.grpc_url, + "SIFT_REST_URI": creds.rest_url, } missing = [env for env, value in resolved.items() if not value] - if missing and not is_offline(pytestconfig): + if missing and not offline: log_event(logger, logging.ERROR, "credentials", missing=",".join(missing)) raise pytest.UsageError( "Sift credentials missing: " @@ -220,21 +250,21 @@ def sift_client(pytestconfig: pytest.Config) -> SiftClient: + ". Set the environment variable(s) (pytest-dotenv loads them " "from a `.env` file automatically), or set the URIs under " "`sift_grpc_uri` / `sift_rest_uri` in `[tool.pytest.ini_options]` " - "in pyproject.toml, or override the sift_client fixture in your " - "conftest.py, or pass --sift-offline / --sift-disabled to run " - "without contacting Sift." + "in pyproject.toml, or name a sift.toml profile with " + "`--sift-profile` / `sift_profile` / SIFT_PROFILE, or override the " + "sift_client fixture in your conftest.py, or pass --sift-offline / " + "--sift-disabled to run without contacting Sift." ) for env in missing: resolved[env] = OFFLINE_DEFAULTS[env] - # Web-app origin for the report link: the SIFT_APP_URL env var wins, then the - # sift_app_url ini key, else host-based derivation in SiftClient.app_url. - app_url = APP_URL_OPTION.resolve(pytestconfig) + return SiftClient( connection_config=SiftConnectionConfig( - api_key=resolved["SIFT_API_KEY"] or "", - grpc_url=resolved["SIFT_GRPC_URI"] or "", - rest_url=resolved["SIFT_REST_URI"] or "", - app_url=app_url or None, + api_key=resolved["SIFT_API_KEY"], + grpc_url=resolved["SIFT_GRPC_URI"], + rest_url=resolved["SIFT_REST_URI"], + app_url=creds.app_url, + use_ssl=creds.use_ssl, ) ) diff --git a/python/mkdocs.yml b/python/mkdocs.yml index 32699b635..589c7d58d 100644 --- a/python/mkdocs.yml +++ b/python/mkdocs.yml @@ -66,6 +66,7 @@ nav: - Pytest Plugin Quickstart: examples/pytest_plugin_quickstart.md - Guides: - guides/index.md + - Credentials and profiles: guides/credentials.md - Pytest Plugin: - Overview: guides/pytest_plugin/index.md - Configuration & Defaults: guides/pytest_plugin/configuration.md diff --git a/rust/crates/sift_cli/src/cmd/config/mod.rs b/rust/crates/sift_cli/src/cmd/config/mod.rs index 616b8fdff..378fb917c 100644 --- a/rust/crates/sift_cli/src/cmd/config/mod.rs +++ b/rust/crates/sift_cli/src/cmd/config/mod.rs @@ -14,6 +14,7 @@ use toml::{Table, Value}; use crate::{ cli::ConfigUpdateArgs, + cmd::{API_KEY_KEY, API_KEY_KEY_LEGACY}, util::{ app_uri::{infer_app_uri, normalize_app_uri}, tty::{Output, PromptUser}, @@ -245,7 +246,10 @@ fn apply_profile_updates( target.insert(String::from("rest_uri"), Value::String(uri)); } if let Some(token) = api_key { - target.insert(String::from("apikey"), Value::String(token)); + // Write the canonical spelling and remove the older one, so a profile + // never holds both keys and no reader has to choose between them. + target.remove(API_KEY_KEY_LEGACY); + target.insert(String::from(API_KEY_KEY), Value::String(token)); } if let Some(uri) = app_uri.as_deref().and_then(normalize_app_uri) { target.insert(String::from("app_uri"), Value::String(uri.to_string())); diff --git a/rust/crates/sift_cli/src/cmd/config/tests.rs b/rust/crates/sift_cli/src/cmd/config/tests.rs index 6bd8d2ae8..797885e1d 100644 --- a/rust/crates/sift_cli/src/cmd/config/tests.rs +++ b/rust/crates/sift_cli/src/cmd/config/tests.rs @@ -195,3 +195,57 @@ app_uri = "https://sift.example.net" ); } } + +mod api_key_key { + use super::super::apply_profile_updates; + use crate::cmd::{API_KEY_KEY, API_KEY_KEY_LEGACY}; + use toml::Table; + + fn config(input: &str) -> Table { + input.parse().unwrap() + } + + fn update_key(config: &mut Table, profile: Option<&str>, key: &str) { + apply_profile_updates( + config, + profile.map(String::from), + None, + None, + Some(key.to_string()), + None, + ) + .unwrap(); + } + + #[test] + fn writes_the_canonical_key() { + let mut config = config(""); + update_key(&mut config, None, "fresh"); + assert_eq!(config[API_KEY_KEY].as_str(), Some("fresh")); + assert!(!config.contains_key(API_KEY_KEY_LEGACY)); + } + + #[test] + fn migrates_a_legacy_key_instead_of_leaving_both() { + let mut config = config("apikey = \"old\"\n"); + update_key(&mut config, None, "new"); + assert_eq!(config[API_KEY_KEY].as_str(), Some("new")); + assert!( + !config.contains_key(API_KEY_KEY_LEGACY), + "the older key must go, so that no reader sees two spellings" + ); + } + + #[test] + fn migrates_within_a_named_profile_only() { + let mut config = config("apikey = \"top\"\n\n[mission]\napikey = \"old\"\n"); + update_key(&mut config, Some("mission"), "new"); + + let mission = config["mission"].as_table().unwrap(); + assert_eq!(mission[API_KEY_KEY].as_str(), Some("new")); + assert!(!mission.contains_key(API_KEY_KEY_LEGACY)); + + // The default profile does not change, and keeps the older spelling. + assert_eq!(config[API_KEY_KEY_LEGACY].as_str(), Some("top")); + } +} diff --git a/rust/crates/sift_cli/src/cmd/mod.rs b/rust/crates/sift_cli/src/cmd/mod.rs index c897e2eba..c11e8b055 100644 --- a/rust/crates/sift_cli/src/cmd/mod.rs +++ b/rust/crates/sift_cli/src/cmd/mod.rs @@ -25,6 +25,22 @@ pub struct Context { pub app_uri: Option, } +/// The canonical TOML key for the API key. It matches the `api_key` spelling +/// that the Sift API uses. +pub(super) const API_KEY_KEY: &str = "api_key"; + +/// The older spelling. It stays valid so that a config from an earlier release +/// still works. The CLI never writes it. +pub(super) const API_KEY_KEY_LEGACY: &str = "apikey"; + +/// The profile's API key under either spelling, canonical first. +fn profile_api_key(profile: &Table) -> Option { + profile + .get(API_KEY_KEY) + .or_else(|| profile.get(API_KEY_KEY_LEGACY)) + .cloned() +} + impl Context { pub fn new(profile: Option, disable_tls: bool) -> Result { let config_path = config::get_config_file_path()?; @@ -100,16 +116,16 @@ impl Context { .and_then(normalize_app_uri) .map(str::to_string); - let Some(Value::String(api_key)) = target_profile.get("apikey").cloned() else { + let Some(Value::String(api_key)) = profile_api_key(target_profile) else { return Err(anyhow!( "Expected value of '{}' to be a string", - "apikey".yellow() + API_KEY_KEY.yellow() )); }; if api_key.is_empty() { return Err(anyhow!( "Expected value of '{}' to be present", - "apikey".yellow() + API_KEY_KEY.yellow() )); } @@ -157,7 +173,7 @@ mod tests { grpc_uri = "https://grpc-api.siftstack.com" rest_uri = "https://api.siftstack.com" app_uri = "https://app.siftstack.com" -apikey = "default-key" +api_key = "default-key" [mission] grpc_uri = "https://grpc.example.net" @@ -211,7 +227,7 @@ apikey = "key" "#, ), ( - "apikey", + "api_key", r#" grpc_uri = "https://grpc-api.siftstack.com" rest_uri = "https://api.siftstack.com" @@ -224,6 +240,30 @@ app_uri = "https://app.siftstack.com" } } + #[test] + fn accepts_either_api_key_spelling() { + // COMPLETE_CONFIG uses `api_key` at the top level and the older + // `apikey` under [mission], so one load covers both spellings. + assert_eq!( + context(COMPLETE_CONFIG, None).unwrap().api_key, + "default-key" + ); + assert_eq!( + context(COMPLETE_CONFIG, Some("mission")).unwrap().api_key, + "mission-key" + ); + + // `config update` never writes both keys. A file that someone edits by + // hand can hold both, and then the canonical spelling decides. + let both = r#" +grpc_uri = "https://grpc-api.siftstack.com" +rest_uri = "https://api.siftstack.com" +api_key = "canonical" +apikey = "legacy" +"#; + assert_eq!(context(both, None).unwrap().api_key, "canonical"); + } + #[test] fn incomplete_app_uri_remains_loadable_for_recovery() { for app_uri in [ diff --git a/rust/crates/sift_connect/README.md b/rust/crates/sift_connect/README.md index 5fc1cdb92..58bae060e 100644 --- a/rust/crates/sift_connect/README.md +++ b/rust/crates/sift_connect/README.md @@ -30,11 +30,11 @@ The following is an example of a valid `sift.toml` file: ```toml uri = "http://example-sift-api.com" -apikey = "example-sift-api-key" +api_key = "example-sift-api-key" [mission] uri = "http://example-sift-api.com" -apikey = "my-other-sift-api-key" +api_key = "my-other-sift-api-key" ``` The top-level TOML table is considered the default profile, with the `mission` table being a diff --git a/rust/crates/sift_connect/src/grpc/config.rs b/rust/crates/sift_connect/src/grpc/config.rs index a488d23e3..9faeb59c1 100644 --- a/rust/crates/sift_connect/src/grpc/config.rs +++ b/rust/crates/sift_connect/src/grpc/config.rs @@ -20,13 +20,16 @@ pub const SIFT_CONFIG_NAME: &str = "sift.toml"; /// /// ```toml /// uri = "https://api.siftstack.com" -/// apikey = "default-api-key" +/// api_key = "default-api-key" /// /// [production] /// uri = "https://api.siftstack.com" -/// apikey = "production-api-key" +/// api_key = "production-api-key" /// ``` /// +/// The older `apikey` spelling is still valid, so a file from an earlier release +/// still works. If a table holds both keys, the loader uses `api_key`. +/// /// # Direct Credentials /// /// The `Config` variant allows you to provide credentials directly without @@ -102,56 +105,55 @@ impl TryFrom for SiftChannelConfig { .with_context(|| format!("failed to parse {}", config.display())) .help("ensure that the config file is properly formated")?; - match profile { + let (table, location) = match &profile { Some(p) => { - let Some(Value::Table(sub_table)) = config_toml.get(&p) else { + let Some(Value::Table(sub_table)) = config_toml.get(p) else { return Err(Error::new_msg( ErrorKind::ConfigError, format!("expected a '{p}' sub-table in '{}'", config.display()), )); }; - - let Some(Value::String(uri)) = sub_table.get("uri") else { - return Err(Error::new_msg( - ErrorKind::ConfigError, - format!("expected '{p}' to contain 'uri' entry"), - )); - }; - - let Some(Value::String(apikey)) = sub_table.get("apikey") else { - return Err(Error::new_msg( - ErrorKind::ConfigError, - format!("expected '{p}' to contain 'apikey' entry"), - )); - }; - - Ok(SiftChannelConfig::new(uri, apikey)) + (sub_table, format!("'{p}'")) } - None => { - let Some(Value::String(uri)) = config_toml.get("uri") else { - return Err(Error::new_msg( - ErrorKind::ConfigError, - format!( - "expected '{}' to contain a top-level 'uri' entry", - config.display() - ), - )); - }; + None => ( + &config_toml, + format!("a top-level entry in '{}'", config.display()), + ), + }; - let Some(Value::String(apikey)) = config_toml.get("apikey") else { - return Err(Error::new_msg( - ErrorKind::ConfigError, - format!( - "expected '{}' to contain a top-level 'apikey' entry", - config.display() - ), - )); - }; + let Some(uri) = lookup(table, &[URI_KEY]) else { + return Err(Error::new_msg( + ErrorKind::ConfigError, + format!("expected {location} to contain '{URI_KEY}'"), + )); + }; - Ok(SiftChannelConfig::new(uri, apikey)) - } - } + let Some(apikey) = lookup(table, API_KEY_KEYS) else { + return Err(Error::new_msg( + ErrorKind::ConfigError, + format!("expected {location} to contain '{}'", API_KEY_KEYS[0]), + )); + }; + + Ok(SiftChannelConfig::new(uri, apikey)) } } } } + +/// The accepted TOML keys for the API key, canonical first. `api_key` matches +/// the spelling that the Sift API and `sift-cli` use. `apikey` is the older +/// spelling that earlier releases wrote. A lookup accepts either key. Nothing +/// writes the older one. +pub const API_KEY_KEYS: &[&str] = &["api_key", "apikey"]; + +/// The TOML key naming the gRPC endpoint. +pub const URI_KEY: &str = "uri"; + +/// The value of the first key in `keys` that `table` sets, or `None`. +fn lookup<'a>(table: &'a Table, keys: &[&str]) -> Option<&'a str> { + keys.iter().find_map(|key| match table.get(*key) { + Some(Value::String(value)) if !value.is_empty() => Some(value.as_str()), + _ => None, + }) +} diff --git a/rust/crates/sift_connect/src/lib.rs b/rust/crates/sift_connect/src/lib.rs index 544590675..f079e553a 100644 --- a/rust/crates/sift_connect/src/lib.rs +++ b/rust/crates/sift_connect/src/lib.rs @@ -25,11 +25,11 @@ //! //! ```text //! uri = "http://example-sift-api.com" -//! apikey = "example-sift-api-key" +//! api_key = "example-sift-api-key" //! //! [mission] //! uri = "http://example-sift-api.com" -//! apikey = "my-other-sift-api-key" +//! api_key = "my-other-sift-api-key" //! ``` //! //! The top-level TOML table is considered the default profile, with the `mission` table being a