From fab7b1662dc64bd94b00b2d6ac02e16e7a0c97f7 Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Fri, 15 May 2026 11:45:34 -0400 Subject: [PATCH 01/18] feat(otdf-sdk-mgr): manage platform service + install scenario (DSPX-3302) --- otdf-sdk-mgr/src/otdf_sdk_mgr/cli_install.py | 101 +++++++- otdf-sdk-mgr/src/otdf_sdk_mgr/cli_scenario.py | 100 ++++++++ .../src/otdf_sdk_mgr/platform_installer.py | 224 ++++++++++++++++++ 3 files changed, 417 insertions(+), 8 deletions(-) create mode 100644 otdf-sdk-mgr/src/otdf_sdk_mgr/cli_scenario.py create mode 100644 otdf-sdk-mgr/src/otdf_sdk_mgr/platform_installer.py diff --git a/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_install.py b/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_install.py index e3950d717..dca6793b3 100644 --- a/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_install.py +++ b/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_install.py @@ -11,6 +11,16 @@ install_app = typer.Typer(help="Install SDK CLI artifacts from registries or source.") +def _register_scenario_cmd() -> None: + """Defer scenario import so pydantic is only imported when needed.""" + from otdf_sdk_mgr.cli_scenario import install_scenario_cmd + + install_app.command("scenario")(install_scenario_cmd) + + +_register_scenario_cmd() + + @install_app.command() def stable( sdks: Annotated[ @@ -32,9 +42,27 @@ def lts( ] = None, ) -> None: """Install LTS versions for each SDK.""" + from otdf_sdk_mgr.config import LTS_VERSIONS from otdf_sdk_mgr.installers import cmd_lts - - cmd_lts(sdks or ALL_SDKS) + from otdf_sdk_mgr.platform_installer import ( + PlatformInstallError, + install_platform_release, + ) + + requested = sdks or ALL_SDKS + sdk_targets = [s for s in requested if s != "platform"] + if "platform" in requested: + version = LTS_VERSIONS.get("platform") + if version is None: + typer.echo("Warning: no LTS version defined for platform; skipping", err=True) + else: + try: + install_platform_release(version) + except PlatformInstallError as e: + typer.echo(f"Error: {e}", err=True) + raise typer.Exit(1) + if sdk_targets: + cmd_lts(sdk_targets) @install_app.command() @@ -46,23 +74,80 @@ def tip( ) -> None: """Source checkout + build from main.""" from otdf_sdk_mgr.installers import cmd_tip - - cmd_tip(sdks or ALL_SDKS) + from otdf_sdk_mgr.platform_installer import ( + PlatformInstallError, + install_platform_source, + ) + + requested = sdks or ALL_SDKS + sdk_targets = [s for s in requested if s != "platform"] + if "platform" in requested: + try: + install_platform_source("main", dist_name="tip") + except PlatformInstallError as e: + typer.echo(f"Error: {e}", err=True) + raise typer.Exit(1) + if sdk_targets: + cmd_tip(sdk_targets) @install_app.command() def release( specs: Annotated[ list[str], - typer.Argument(help="Version specs as SDK:VERSION (e.g., go:v0.24.0)"), + typer.Argument(help="Version specs as SDK:VERSION (e.g., go:v0.24.0, platform:v0.9.0)"), ], ) -> None: - """Install specific released versions.""" + """Install specific released versions. + + `sdk` may be one of go/js/java or the literal `platform`. Platform is + built from source against the `service/` tag in the + `opentdf/platform` monorepo. + """ from otdf_sdk_mgr.installers import InstallError, cmd_release + from otdf_sdk_mgr.platform_installer import ( + PlatformInstallError, + install_platform_release, + ) + + sdk_specs: list[str] = [] + for spec in specs: + if ":" not in spec: + typer.echo(f"Error: invalid spec '{spec}'. Use SDK:VERSION.", err=True) + raise typer.Exit(1) + sdk, version = spec.split(":", 1) + if sdk == "platform": + try: + install_platform_release(version) + except PlatformInstallError as e: + typer.echo(f"Error: {e}", err=True) + raise typer.Exit(1) + else: + sdk_specs.append(spec) + if sdk_specs: + try: + cmd_release(sdk_specs) + except InstallError as e: + typer.echo(f"Error: {e}", err=True) + raise typer.Exit(1) + + +@install_app.command() +def scripts( + branch: Annotated[ + str, + typer.Option(help="Branch of opentdf/platform to pull scripts from"), + ] = "main", +) -> None: + """Refresh shared platform helper scripts under xtest/platform/scripts/.""" + from otdf_sdk_mgr.platform_installer import ( + PlatformInstallError, + install_helper_scripts, + ) try: - cmd_release(specs) - except InstallError as e: + install_helper_scripts(branch) + except PlatformInstallError as e: typer.echo(f"Error: {e}", err=True) raise typer.Exit(1) diff --git a/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_scenario.py b/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_scenario.py new file mode 100644 index 000000000..34b428a44 --- /dev/null +++ b/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_scenario.py @@ -0,0 +1,100 @@ +"""Scenario-driven install command. + +Reads a `scenarios.yaml` (or standalone `instance.yaml`) and installs every +artifact referenced — platform service binary, per-KAS binaries (each at +its own pinned version), and encrypt/decrypt SDK CLIs. Writes +`installed.json` next to the manifest so downstream tools (`otdf-local`, +plugin skills) can locate the dist paths without re-resolving. +""" + +from __future__ import annotations + +import json +from pathlib import Path +from typing import Annotated + +import typer + +from otdf_sdk_mgr.installers import InstallError, install_release +from otdf_sdk_mgr.platform_installer import ( + PlatformInstallError, + install_helper_scripts, + install_platform_release, + install_platform_source, +) +from otdf_sdk_mgr.schema import KasPin, PlatformPin, Scenario, load_instance, load_scenario + + +def _install_platform_pin(pin: PlatformPin | KasPin) -> dict[str, str]: + if pin.dist is not None: + dist_dir = install_platform_release(pin.dist) + return {"kind": "dist", "version": pin.dist, "path": str(dist_dir)} + assert pin.source is not None # by schema invariant + dist_dir = install_platform_source(pin.source.ref) + return {"kind": "source", "ref": pin.source.ref, "path": str(dist_dir)} + + +def install_scenario_cmd( + path: Annotated[Path, typer.Argument(help="Path to scenarios.yaml or instance.yaml")], + skip_scripts: Annotated[ + bool, + typer.Option("--skip-scripts", help="Skip refreshing helper scripts from main"), + ] = False, +) -> None: + """Install every artifact declared by a scenarios.yaml or instance.yaml.""" + if not path.exists(): + typer.echo(f"Error: {path} not found", err=True) + raise typer.Exit(1) + + raw_kind = _peek_kind(path) + scenario: Scenario | None = None + if raw_kind == "Scenario": + scenario = load_scenario(path) + instance = scenario.instance + elif raw_kind == "Instance": + instance = load_instance(path) + else: + typer.echo(f"Error: {path} has unknown kind {raw_kind!r}", err=True) + raise typer.Exit(1) + + installed: dict[str, object] = {"manifest": str(path), "platform": None, "kas": {}, "sdks": {}} + + try: + installed["platform"] = _install_platform_pin(instance.platform) + for kas_name, kas_pin in instance.kas.items(): + installed["kas"][kas_name] = _install_platform_pin(kas_pin) + if not skip_scripts: + install_helper_scripts() + except PlatformInstallError as e: + typer.echo(f"Error installing platform artifacts: {e}", err=True) + raise typer.Exit(1) + + if scenario is not None: + sdks = scenario.sdks.union() + for sdk_name, sdk_pin in sdks.items(): + try: + dist_dir = install_release(sdk_name, sdk_pin.version, source=sdk_pin.source) + installed["sdks"][sdk_name] = { + "version": sdk_pin.version, + "source": sdk_pin.source, + "path": str(dist_dir), + } + except InstallError as e: + typer.echo(f"Error installing SDK {sdk_name}: {e}", err=True) + raise typer.Exit(1) + + out = path.parent / f"{path.stem}.installed.json" + out.write_text(json.dumps(installed, indent=2) + "\n") + typer.echo(f" Wrote {out}") + + +def _peek_kind(path: Path) -> str | None: + """Cheap pre-validation read so we can dispatch to the right model loader.""" + from ruamel.yaml import YAML + + y = YAML(typ="safe") + raw = y.load(path.read_text()) + if isinstance(raw, dict): + kind = raw.get("kind") + return kind if isinstance(kind, str) else None + return None diff --git a/otdf-sdk-mgr/src/otdf_sdk_mgr/platform_installer.py b/otdf-sdk-mgr/src/otdf_sdk_mgr/platform_installer.py new file mode 100644 index 000000000..67179672f --- /dev/null +++ b/otdf-sdk-mgr/src/otdf_sdk_mgr/platform_installer.py @@ -0,0 +1,224 @@ +"""Installer for the OpenTDF platform service. + +Mirrors the SDK installer pattern but produces a built `service` binary at +`xtest/platform/dist//service`. v1 supports source builds only — +container images and release tarballs are not published by `opentdf/platform` +today. + +Tag namespacing: the platform monorepo tags releases as `service/vX.Y.Z`. +Users pass plain versions (e.g. `v0.9.0`); the installer prefixes `service/` +when resolving against git. +""" + +from __future__ import annotations + +import os +import shutil +import subprocess +from pathlib import Path + +from otdf_sdk_mgr.config import SDK_GIT_URLS, SDK_TAG_INFIXES +from otdf_sdk_mgr.semver import normalize_version + +PLATFORM_BARE_REPO = "platform.git" +HELPER_SCRIPTS_BRANCH = "main" + + +class PlatformInstallError(Exception): + """Raised when platform installation fails.""" + + +def get_platform_dir() -> Path: + """Return `xtest/platform/`, creating an env-var override hook. + + Search precedence: + 1. `OTDF_PLATFORM_DIR` env var. + 2. Walk up from this package until an `xtest/` sibling is found. + """ + env = os.environ.get("OTDF_PLATFORM_DIR") + if env: + return Path(env) + current = Path(__file__).resolve().parent + while current != current.parent: + if (current / "xtest").exists(): + return current / "xtest" / "platform" + current = current.parent + raise PlatformInstallError( + "Could not locate xtest/ root. Set OTDF_PLATFORM_DIR to override." + ) + + +def _platform_src_root() -> Path: + return get_platform_dir() / "src" + + +def _platform_dist_root() -> Path: + return get_platform_dir() / "dist" + + +def _platform_scripts_dir() -> Path: + return get_platform_dir() / "scripts" + + +def _platform_bare_repo() -> Path: + return _platform_src_root() / PLATFORM_BARE_REPO + + +def _run(cmd: list[str], cwd: Path | None = None) -> None: + """Run a command, raising PlatformInstallError on failure.""" + result = subprocess.run(cmd, cwd=cwd, capture_output=True, text=True) + if result.returncode != 0: + raise PlatformInstallError( + f"command failed: {' '.join(cmd)}\nstdout: {result.stdout}\nstderr: {result.stderr}" + ) + + +def _ensure_bare_repo() -> Path: + """Clone the platform bare repo if missing; fetch updates otherwise.""" + bare = _platform_bare_repo() + bare.parent.mkdir(parents=True, exist_ok=True) + if not bare.exists(): + url = SDK_GIT_URLS["platform"].removesuffix(".git") + print(f"Cloning {url} as a bare repository into {bare}...") + _run(["git", "clone", "--bare", url, str(bare)]) + else: + print(f"Fetching updates for {bare}...") + _run(["git", f"--git-dir={bare}", "fetch", "--all", "--tags"]) + return bare + + +def _resolve_platform_ref(version_or_ref: str) -> str: + """Turn a user-supplied version into the actual git ref to checkout. + + `v0.9.0` → `service/v0.9.0` (matches SDK_TAG_INFIXES["platform"]). + A ref that already contains `/`, a hex SHA, or `main` is returned as-is. + """ + infix = SDK_TAG_INFIXES.get("platform", "service") + if "/" in version_or_ref or version_or_ref in ("main", "HEAD"): + return version_or_ref + if len(version_or_ref) >= 7 and all(c in "0123456789abcdef" for c in version_or_ref.lower()): + return version_or_ref + return f"{infix}/{normalize_version(version_or_ref)}" + + +def _worktree_path_for(ref: str) -> Path: + safe = ref.replace("/", "--") + return _platform_src_root() / safe + + +def _ensure_worktree(ref: str) -> Path: + """Create (or reuse) a git worktree at the given platform ref.""" + bare = _ensure_bare_repo() + worktree = _worktree_path_for(ref) + if worktree.exists(): + print(f"Worktree already exists at {worktree}; reusing.") + return worktree + print(f"Adding worktree at {worktree} for ref {ref}...") + _run(["git", f"--git-dir={bare}", "worktree", "add", "--detach", str(worktree), ref]) + return worktree + + +def _build_service(worktree: Path, dist_dir: Path) -> Path: + """Run `go build` to produce `dist_dir/service`.""" + dist_dir.mkdir(parents=True, exist_ok=True) + binary = dist_dir / "service" + if binary.exists(): + print(f" Binary already built at {binary}; reusing.") + return binary + print(f" Building platform service binary at {binary} from {worktree}...") + _run(["go", "build", "-o", str(binary), "./service"], cwd=worktree) + if not binary.exists(): + raise PlatformInstallError(f"go build completed but {binary} is missing") + return binary + + +def _record_version(dist_dir: Path, ref: str, worktree: Path) -> None: + """Write a `.version` metadata file alongside the binary.""" + sha = _git_rev_parse(worktree, "HEAD") + (dist_dir / ".version").write_text(f"ref={ref}\nsha={sha}\nworktree={worktree}\n") + + +def _git_rev_parse(worktree: Path, rev: str) -> str: + result = subprocess.run( + ["git", "-C", str(worktree), "rev-parse", rev], capture_output=True, text=True + ) + if result.returncode != 0: + return "" + return result.stdout.strip() + + +def install_platform_source(ref: str, dist_name: str | None = None) -> Path: + """Install a platform build by checking out and building `ref`. + + `ref` may be a plain version (`v0.9.0`), a namespaced tag + (`service/v0.9.0`), a branch (`main`), or a SHA. Returns the dist dir. + """ + full_ref = _resolve_platform_ref(ref) + dist_dir = _platform_dist_root() / (dist_name or normalize_version(ref)) + if (dist_dir / "service").exists(): + print(f" Dist already present at {dist_dir}; skipping build.") + return dist_dir + worktree = _ensure_worktree(full_ref) + _build_service(worktree, dist_dir) + _record_version(dist_dir, full_ref, worktree) + print(f" Platform {ref} → {dist_dir}") + return dist_dir + + +def install_platform_release(version: str, dist_name: str | None = None) -> Path: + """Install a released platform version (alias for `install_platform_source`). + + Kept as a separate function so the public CLI surface mirrors the SDK + `install release` semantics, even though there's no published-binary + fast path today. + """ + return install_platform_source(version, dist_name=dist_name) + + +def install_helper_scripts(branch: str = HELPER_SCRIPTS_BRANCH) -> Path: + """Check out provisioning helper scripts from the platform `main` branch. + + Scripts are shared across instances (see plan §1, helper-scripts decision) + and refreshed on demand. Returns the scripts directory. + """ + bare = _ensure_bare_repo() + scripts_dir = _platform_scripts_dir() + worktree = _worktree_path_for(f"scripts--{branch}") + if not worktree.exists(): + print(f"Adding scripts worktree at {worktree} ({branch})...") + _run(["git", f"--git-dir={bare}", "worktree", "add", str(worktree), branch]) + else: + print(f"Updating scripts worktree at {worktree}...") + _run(["git", "-C", str(worktree), "pull", "origin", branch]) + scripts_dir.mkdir(parents=True, exist_ok=True) + src_scripts = worktree / "scripts" + if src_scripts.exists(): + # Mirror the platform/scripts/ tree into xtest/platform/scripts/ + if scripts_dir.exists(): + shutil.rmtree(scripts_dir) + shutil.copytree(src_scripts, scripts_dir) + print(f" Helper scripts copied to {scripts_dir}") + else: + print(f" Warning: no scripts/ directory in platform@{branch}; nothing to copy.") + return scripts_dir + + +def list_platform_versions() -> list[str]: + """Return all `service/vX.Y.Z` tags from the platform repo, version-only.""" + from git import Git + + repo = Git() + raw = repo.ls_remote(SDK_GIT_URLS["platform"], tags=True) + infix = SDK_TAG_INFIXES.get("platform", "service") + out: list[str] = [] + for line in raw.strip().splitlines(): + if not line: + continue + _, ref = line.split("\t", 1) + if ref.endswith("^{}"): + continue + tag = ref.removeprefix("refs/tags/") + if tag.startswith(f"{infix}/"): + out.append(tag.removeprefix(f"{infix}/")) + out.sort() + return out From a5e6229690a06e9aa11b25325d914cf2cdcefbcc Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Thu, 21 May 2026 12:00:49 -0400 Subject: [PATCH 02/18] fix(otdf-sdk-mgr): repair install scenario + harden silent failures `install scenario` could not run as written: it iterated `ScenarioSdks.union()` as a dict (it returns a list) and passed a `source=` kwarg `install_release` does not accept. The emitted `installed.json` shape also did not match what `scenario_to_pytest_sdks` reads (per-role lists, not sdk-name-keyed dict), so even the platform-only path produced a manifest no downstream tool could consume. Source fixes: - cli_scenario.py: iterate `union()` as the list it is, cache installs by (sdk, version, source), emit role-keyed lists matching the reader's expected shape; on failure write a partial manifest with `status=partial` so half-installed dist trees are diagnosable. Catch YAMLError in `_peek_kind` to surface a clean typer error. - platform_installer.py: `_git_rev_parse` raises on failure instead of silently writing an empty `sha=` into `.version`. Missing `scripts/` raises instead of warning-and-continuing. SHA passthrough heuristic tightened from `>=7` chars to exactly 40 (SHA-1) or 64 (SHA-256), so ambiguous short tags like `abc1234` no longer skip the `service/` prefix. Dropped a docstring fragment pointing to a planning doc that won't exist post-merge. - cli_install.py: dropped a docstring whose "deferred import" claim was false (the registration runs at module import). `lts platform` with no pinned version now exits 1 instead of warning-and-exit-0. Tests: - test_platform_installer.py: parametrized cases for `_resolve_platform_ref` covering version normalization, branch passthrough, the tightened hex heuristic, and SHA-1/SHA-256 passthrough. - test_cli_scenario.py: end-to-end smoke that mocks the installers and asserts the produced manifest is round-trip consumable by `scenario_to_pytest_sdks`. This is the gating test that would have caught the original bug. 79 passing (was 67). Co-Authored-By: Claude Opus 4.7 --- otdf-sdk-mgr/src/otdf_sdk_mgr/cli_install.py | 15 ++- otdf-sdk-mgr/src/otdf_sdk_mgr/cli_scenario.py | 53 ++++++--- .../src/otdf_sdk_mgr/platform_installer.py | 29 ++--- otdf-sdk-mgr/tests/test_cli_scenario.py | 110 ++++++++++++++++++ otdf-sdk-mgr/tests/test_platform_installer.py | 23 ++++ 5 files changed, 190 insertions(+), 40 deletions(-) create mode 100644 otdf-sdk-mgr/tests/test_cli_scenario.py create mode 100644 otdf-sdk-mgr/tests/test_platform_installer.py diff --git a/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_install.py b/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_install.py index dca6793b3..1bf864478 100644 --- a/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_install.py +++ b/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_install.py @@ -12,7 +12,6 @@ def _register_scenario_cmd() -> None: - """Defer scenario import so pydantic is only imported when needed.""" from otdf_sdk_mgr.cli_scenario import install_scenario_cmd install_app.command("scenario")(install_scenario_cmd) @@ -54,13 +53,13 @@ def lts( if "platform" in requested: version = LTS_VERSIONS.get("platform") if version is None: - typer.echo("Warning: no LTS version defined for platform; skipping", err=True) - else: - try: - install_platform_release(version) - except PlatformInstallError as e: - typer.echo(f"Error: {e}", err=True) - raise typer.Exit(1) + typer.echo("Error: no LTS version defined for platform", err=True) + raise typer.Exit(1) + try: + install_platform_release(version) + except PlatformInstallError as e: + typer.echo(f"Error: {e}", err=True) + raise typer.Exit(1) if sdk_targets: cmd_lts(sdk_targets) diff --git a/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_scenario.py b/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_scenario.py index 34b428a44..560b575ae 100644 --- a/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_scenario.py +++ b/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_scenario.py @@ -57,7 +57,13 @@ def install_scenario_cmd( typer.echo(f"Error: {path} has unknown kind {raw_kind!r}", err=True) raise typer.Exit(1) - installed: dict[str, object] = {"manifest": str(path), "platform": None, "kas": {}, "sdks": {}} + installed: dict[str, object] = { + "manifest": str(path), + "platform": None, + "kas": {}, + "sdks": {"encrypt": [], "decrypt": []}, + } + out = path.parent / f"{path.stem}.installed.json" try: installed["platform"] = _install_platform_pin(instance.platform) @@ -65,25 +71,29 @@ def install_scenario_cmd( installed["kas"][kas_name] = _install_platform_pin(kas_pin) if not skip_scripts: install_helper_scripts() - except PlatformInstallError as e: - typer.echo(f"Error installing platform artifacts: {e}", err=True) - raise typer.Exit(1) - if scenario is not None: - sdks = scenario.sdks.union() - for sdk_name, sdk_pin in sdks.items(): - try: - dist_dir = install_release(sdk_name, sdk_pin.version, source=sdk_pin.source) - installed["sdks"][sdk_name] = { - "version": sdk_pin.version, - "source": sdk_pin.source, - "path": str(dist_dir), - } - except InstallError as e: - typer.echo(f"Error installing SDK {sdk_name}: {e}", err=True) - raise typer.Exit(1) + if scenario is not None: + install_paths: dict[tuple[str, str, str | None], str] = {} + for entry in scenario.sdks.union(): + dist_dir = install_release(entry.sdk, entry.version) + install_paths[entry.install_key()] = str(dist_dir) + for role in ("encrypt", "decrypt"): + installed["sdks"][role] = [ + { + "sdk": entry.sdk, + "version": entry.version, + "source": entry.source, + "path": install_paths[entry.install_key()], + } + for entry in getattr(scenario.sdks, role) + ] + except (PlatformInstallError, InstallError) as e: + installed["status"] = "partial" + out.write_text(json.dumps(installed, indent=2) + "\n") + typer.echo(f"Error: {e}", err=True) + typer.echo(f" Wrote partial manifest to {out}", err=True) + raise typer.Exit(1) - out = path.parent / f"{path.stem}.installed.json" out.write_text(json.dumps(installed, indent=2) + "\n") typer.echo(f" Wrote {out}") @@ -91,9 +101,14 @@ def install_scenario_cmd( def _peek_kind(path: Path) -> str | None: """Cheap pre-validation read so we can dispatch to the right model loader.""" from ruamel.yaml import YAML + from ruamel.yaml.error import YAMLError y = YAML(typ="safe") - raw = y.load(path.read_text()) + try: + raw = y.load(path.read_text()) + except YAMLError as e: + typer.echo(f"Error: {path} is not valid YAML: {e}", err=True) + raise typer.Exit(1) if isinstance(raw, dict): kind = raw.get("kind") return kind if isinstance(kind, str) else None diff --git a/otdf-sdk-mgr/src/otdf_sdk_mgr/platform_installer.py b/otdf-sdk-mgr/src/otdf_sdk_mgr/platform_installer.py index 67179672f..28f3c866f 100644 --- a/otdf-sdk-mgr/src/otdf_sdk_mgr/platform_installer.py +++ b/otdf-sdk-mgr/src/otdf_sdk_mgr/platform_installer.py @@ -96,7 +96,9 @@ def _resolve_platform_ref(version_or_ref: str) -> str: infix = SDK_TAG_INFIXES.get("platform", "service") if "/" in version_or_ref or version_or_ref in ("main", "HEAD"): return version_or_ref - if len(version_or_ref) >= 7 and all(c in "0123456789abcdef" for c in version_or_ref.lower()): + if len(version_or_ref) in (40, 64) and all( + c in "0123456789abcdef" for c in version_or_ref.lower() + ): return version_or_ref return f"{infix}/{normalize_version(version_or_ref)}" @@ -143,7 +145,9 @@ def _git_rev_parse(worktree: Path, rev: str) -> str: ["git", "-C", str(worktree), "rev-parse", rev], capture_output=True, text=True ) if result.returncode != 0: - return "" + raise PlatformInstallError( + f"git rev-parse {rev} failed in {worktree}: {result.stderr.strip()}" + ) return result.stdout.strip() @@ -178,8 +182,8 @@ def install_platform_release(version: str, dist_name: str | None = None) -> Path def install_helper_scripts(branch: str = HELPER_SCRIPTS_BRANCH) -> Path: """Check out provisioning helper scripts from the platform `main` branch. - Scripts are shared across instances (see plan §1, helper-scripts decision) - and refreshed on demand. Returns the scripts directory. + Scripts are shared across instances; refreshed on demand. Returns the + scripts directory. """ bare = _ensure_bare_repo() scripts_dir = _platform_scripts_dir() @@ -190,16 +194,15 @@ def install_helper_scripts(branch: str = HELPER_SCRIPTS_BRANCH) -> Path: else: print(f"Updating scripts worktree at {worktree}...") _run(["git", "-C", str(worktree), "pull", "origin", branch]) - scripts_dir.mkdir(parents=True, exist_ok=True) src_scripts = worktree / "scripts" - if src_scripts.exists(): - # Mirror the platform/scripts/ tree into xtest/platform/scripts/ - if scripts_dir.exists(): - shutil.rmtree(scripts_dir) - shutil.copytree(src_scripts, scripts_dir) - print(f" Helper scripts copied to {scripts_dir}") - else: - print(f" Warning: no scripts/ directory in platform@{branch}; nothing to copy.") + if not src_scripts.exists(): + raise PlatformInstallError( + f"no scripts/ directory in platform@{branch}; cannot install helper scripts" + ) + if scripts_dir.exists(): + shutil.rmtree(scripts_dir) + shutil.copytree(src_scripts, scripts_dir) + print(f" Helper scripts copied to {scripts_dir}") return scripts_dir diff --git a/otdf-sdk-mgr/tests/test_cli_scenario.py b/otdf-sdk-mgr/tests/test_cli_scenario.py new file mode 100644 index 000000000..c086e324b --- /dev/null +++ b/otdf-sdk-mgr/tests/test_cli_scenario.py @@ -0,0 +1,110 @@ +"""End-to-end smoke test for `otdf-sdk-mgr install scenario`.""" + +from __future__ import annotations + +import json +from pathlib import Path +from unittest.mock import patch + +import pytest +import typer + +from otdf_sdk_mgr.cli_scenario import install_scenario_cmd +from otdf_sdk_mgr.schema import load_scenario, scenario_to_pytest_sdks + + +SCENARIO_YAML = """ +apiVersion: opentdf.io/v1alpha1 +kind: Scenario +metadata: + id: smoke + title: install-scenario smoke +instance: + platform: { dist: v0.9.0 } + kas: + alpha: { dist: v0.9.0, mode: standard } +sdks: + encrypt: + - sdk: go + version: v0.24.0 + - sdk: js + version: v0.5.0 + decrypt: + - sdk: js + version: v0.5.0 + - sdk: java + version: v0.7.8 +suite: + targets: + - "xtest/test_tdfs.py::test_tdf_roundtrip" +""" + + +def test_install_scenario_writes_consumable_manifest(tmp_path: Path) -> None: + scenario_path = tmp_path / "s.yaml" + scenario_path.write_text(SCENARIO_YAML) + + platform_dist = tmp_path / "platform-dist" / "v0.9.0" + + def fake_install_release(sdk: str, version: str) -> Path: + return tmp_path / "sdk" / sdk / version + + with ( + patch("otdf_sdk_mgr.cli_scenario.install_platform_release", return_value=platform_dist), + patch("otdf_sdk_mgr.cli_scenario.install_helper_scripts"), + patch("otdf_sdk_mgr.cli_scenario.install_release", side_effect=fake_install_release), + ): + install_scenario_cmd(scenario_path, skip_scripts=False) + + out_path = tmp_path / "s.installed.json" + record = json.loads(out_path.read_text()) + + assert record["platform"] == { + "kind": "dist", + "version": "v0.9.0", + "path": str(platform_dist), + } + assert set(record["kas"].keys()) == {"alpha"} + assert record["sdks"]["encrypt"] == [ + {"sdk": "go", "version": "v0.24.0", "source": None, + "path": str(tmp_path / "sdk" / "go" / "v0.24.0")}, + {"sdk": "js", "version": "v0.5.0", "source": None, + "path": str(tmp_path / "sdk" / "js" / "v0.5.0")}, + ] + assert record["sdks"]["decrypt"] == [ + {"sdk": "js", "version": "v0.5.0", "source": None, + "path": str(tmp_path / "sdk" / "js" / "v0.5.0")}, + {"sdk": "java", "version": "v0.7.8", "source": None, + "path": str(tmp_path / "sdk" / "java" / "v0.7.8")}, + ] + assert "status" not in record + + # The manifest must be consumable by the downstream reader. + scenario = load_scenario(scenario_path) + tokens = scenario_to_pytest_sdks(scenario, out_path) + assert tokens == { + "encrypt": ["go@v0.24.0", "js@v0.5.0"], + "decrypt": ["js@v0.5.0", "java@v0.7.8"], + } + + +def test_install_scenario_writes_partial_manifest_on_failure(tmp_path: Path) -> None: + from otdf_sdk_mgr.installers import InstallError + + scenario_path = tmp_path / "s.yaml" + scenario_path.write_text(SCENARIO_YAML) + platform_dist = tmp_path / "platform-dist" / "v0.9.0" + + with ( + patch("otdf_sdk_mgr.cli_scenario.install_platform_release", return_value=platform_dist), + patch("otdf_sdk_mgr.cli_scenario.install_helper_scripts"), + patch("otdf_sdk_mgr.cli_scenario.install_release", + side_effect=InstallError("boom")), + pytest.raises(typer.Exit), + ): + install_scenario_cmd(scenario_path, skip_scripts=True) + + out_path = tmp_path / "s.installed.json" + record = json.loads(out_path.read_text()) + assert record["status"] == "partial" + assert record["platform"] is not None diff --git a/otdf-sdk-mgr/tests/test_platform_installer.py b/otdf-sdk-mgr/tests/test_platform_installer.py new file mode 100644 index 000000000..795054338 --- /dev/null +++ b/otdf-sdk-mgr/tests/test_platform_installer.py @@ -0,0 +1,23 @@ +"""Pure-function tests for platform_installer.""" + +import pytest + +from otdf_sdk_mgr.platform_installer import _resolve_platform_ref + + +@pytest.mark.parametrize( + "inp,expected", + [ + ("v0.9.0", "service/v0.9.0"), + ("0.9.0", "service/v0.9.0"), + ("main", "main"), + ("HEAD", "HEAD"), + ("service/v0.9.0", "service/v0.9.0"), + ("a" * 40, "a" * 40), + ("b" * 64, "b" * 64), + ("abc1234", "service/vabc1234"), + ("deadbeef", "service/vdeadbeef"), + ], +) +def test_resolve_platform_ref(inp, expected): + assert _resolve_platform_ref(inp) == expected From 64132cb8114c887cdcd9504f0233e8a455bb1525 Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Thu, 21 May 2026 12:02:53 -0400 Subject: [PATCH 03/18] style(otdf-sdk-mgr): ruff format Co-Authored-By: Claude Opus 4.7 --- .../src/otdf_sdk_mgr/platform_installer.py | 4 +-- otdf-sdk-mgr/tests/test_cli_scenario.py | 35 +++++++++++++------ 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/otdf-sdk-mgr/src/otdf_sdk_mgr/platform_installer.py b/otdf-sdk-mgr/src/otdf_sdk_mgr/platform_installer.py index 28f3c866f..b278e9cb1 100644 --- a/otdf-sdk-mgr/src/otdf_sdk_mgr/platform_installer.py +++ b/otdf-sdk-mgr/src/otdf_sdk_mgr/platform_installer.py @@ -43,9 +43,7 @@ def get_platform_dir() -> Path: if (current / "xtest").exists(): return current / "xtest" / "platform" current = current.parent - raise PlatformInstallError( - "Could not locate xtest/ root. Set OTDF_PLATFORM_DIR to override." - ) + raise PlatformInstallError("Could not locate xtest/ root. Set OTDF_PLATFORM_DIR to override.") def _platform_src_root() -> Path: diff --git a/otdf-sdk-mgr/tests/test_cli_scenario.py b/otdf-sdk-mgr/tests/test_cli_scenario.py index c086e324b..4f007b18f 100644 --- a/otdf-sdk-mgr/tests/test_cli_scenario.py +++ b/otdf-sdk-mgr/tests/test_cli_scenario.py @@ -66,16 +66,32 @@ def fake_install_release(sdk: str, version: str) -> Path: } assert set(record["kas"].keys()) == {"alpha"} assert record["sdks"]["encrypt"] == [ - {"sdk": "go", "version": "v0.24.0", "source": None, - "path": str(tmp_path / "sdk" / "go" / "v0.24.0")}, - {"sdk": "js", "version": "v0.5.0", "source": None, - "path": str(tmp_path / "sdk" / "js" / "v0.5.0")}, + { + "sdk": "go", + "version": "v0.24.0", + "source": None, + "path": str(tmp_path / "sdk" / "go" / "v0.24.0"), + }, + { + "sdk": "js", + "version": "v0.5.0", + "source": None, + "path": str(tmp_path / "sdk" / "js" / "v0.5.0"), + }, ] assert record["sdks"]["decrypt"] == [ - {"sdk": "js", "version": "v0.5.0", "source": None, - "path": str(tmp_path / "sdk" / "js" / "v0.5.0")}, - {"sdk": "java", "version": "v0.7.8", "source": None, - "path": str(tmp_path / "sdk" / "java" / "v0.7.8")}, + { + "sdk": "js", + "version": "v0.5.0", + "source": None, + "path": str(tmp_path / "sdk" / "js" / "v0.5.0"), + }, + { + "sdk": "java", + "version": "v0.7.8", + "source": None, + "path": str(tmp_path / "sdk" / "java" / "v0.7.8"), + }, ] assert "status" not in record @@ -98,8 +114,7 @@ def test_install_scenario_writes_partial_manifest_on_failure(tmp_path: Path) -> with ( patch("otdf_sdk_mgr.cli_scenario.install_platform_release", return_value=platform_dist), patch("otdf_sdk_mgr.cli_scenario.install_helper_scripts"), - patch("otdf_sdk_mgr.cli_scenario.install_release", - side_effect=InstallError("boom")), + patch("otdf_sdk_mgr.cli_scenario.install_release", side_effect=InstallError("boom")), pytest.raises(typer.Exit), ): install_scenario_cmd(scenario_path, skip_scripts=True) From 6a0e805d031ae6aae1e26bf16603bd584a56b1c9 Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Thu, 21 May 2026 12:18:02 -0400 Subject: [PATCH 04/18] refactor(otdf-sdk-mgr): address PR review feedback on platform installer - platform_installer: fix worktree update from bare repo (no `origin` remote exists), use `git reset --hard ` instead of `git pull` - platform_installer: stop swallowing subprocess output so long-running `go build`/`git clone` progress is visible to the user - cli_install: extract `_install_platform_or_exit` to dedupe platform handling across `lts`, `tip`, and `release` - cli_scenario: parse manifest YAML once and dispatch by `kind`, instead of peeking + re-parsing in each loader Co-Authored-By: Claude Opus 4.7 --- otdf-sdk-mgr/src/otdf_sdk_mgr/cli_install.py | 67 ++++++++++--------- otdf-sdk-mgr/src/otdf_sdk_mgr/cli_scenario.py | 45 ++++++------- .../src/otdf_sdk_mgr/platform_installer.py | 17 +++-- otdf-sdk-mgr/src/otdf_sdk_mgr/schema.py | 9 +-- 4 files changed, 71 insertions(+), 67 deletions(-) diff --git a/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_install.py b/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_install.py index 1bf864478..6bf0a4895 100644 --- a/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_install.py +++ b/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_install.py @@ -20,6 +20,30 @@ def _register_scenario_cmd() -> None: _register_scenario_cmd() +def _split_platform(sdks: list[str]) -> tuple[bool, list[str]]: + """Return (platform_requested, sdks_without_platform).""" + return ("platform" in sdks, [s for s in sdks if s != "platform"]) + + +def _install_platform_or_exit( + install_fn, + version: str, + *, + dist_name: str | None = None, +) -> None: + """Run a platform installer, mapping PlatformInstallError to typer.Exit(1).""" + from otdf_sdk_mgr.platform_installer import PlatformInstallError + + try: + if dist_name is None: + install_fn(version) + else: + install_fn(version, dist_name=dist_name) + except PlatformInstallError as e: + typer.echo(f"Error: {e}", err=True) + raise typer.Exit(1) + + @install_app.command() def stable( sdks: Annotated[ @@ -43,23 +67,15 @@ def lts( """Install LTS versions for each SDK.""" from otdf_sdk_mgr.config import LTS_VERSIONS from otdf_sdk_mgr.installers import cmd_lts - from otdf_sdk_mgr.platform_installer import ( - PlatformInstallError, - install_platform_release, - ) + from otdf_sdk_mgr.platform_installer import install_platform_release - requested = sdks or ALL_SDKS - sdk_targets = [s for s in requested if s != "platform"] - if "platform" in requested: + want_platform, sdk_targets = _split_platform(sdks or ALL_SDKS) + if want_platform: version = LTS_VERSIONS.get("platform") if version is None: typer.echo("Error: no LTS version defined for platform", err=True) raise typer.Exit(1) - try: - install_platform_release(version) - except PlatformInstallError as e: - typer.echo(f"Error: {e}", err=True) - raise typer.Exit(1) + _install_platform_or_exit(install_platform_release, version) if sdk_targets: cmd_lts(sdk_targets) @@ -73,19 +89,11 @@ def tip( ) -> None: """Source checkout + build from main.""" from otdf_sdk_mgr.installers import cmd_tip - from otdf_sdk_mgr.platform_installer import ( - PlatformInstallError, - install_platform_source, - ) + from otdf_sdk_mgr.platform_installer import install_platform_source - requested = sdks or ALL_SDKS - sdk_targets = [s for s in requested if s != "platform"] - if "platform" in requested: - try: - install_platform_source("main", dist_name="tip") - except PlatformInstallError as e: - typer.echo(f"Error: {e}", err=True) - raise typer.Exit(1) + want_platform, sdk_targets = _split_platform(sdks or ALL_SDKS) + if want_platform: + _install_platform_or_exit(install_platform_source, "main", dist_name="tip") if sdk_targets: cmd_tip(sdk_targets) @@ -104,10 +112,7 @@ def release( `opentdf/platform` monorepo. """ from otdf_sdk_mgr.installers import InstallError, cmd_release - from otdf_sdk_mgr.platform_installer import ( - PlatformInstallError, - install_platform_release, - ) + from otdf_sdk_mgr.platform_installer import install_platform_release sdk_specs: list[str] = [] for spec in specs: @@ -116,11 +121,7 @@ def release( raise typer.Exit(1) sdk, version = spec.split(":", 1) if sdk == "platform": - try: - install_platform_release(version) - except PlatformInstallError as e: - typer.echo(f"Error: {e}", err=True) - raise typer.Exit(1) + _install_platform_or_exit(install_platform_release, version) else: sdk_specs.append(spec) if sdk_specs: diff --git a/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_scenario.py b/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_scenario.py index 560b575ae..fa8c448d3 100644 --- a/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_scenario.py +++ b/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_scenario.py @@ -22,7 +22,13 @@ install_platform_release, install_platform_source, ) -from otdf_sdk_mgr.schema import KasPin, PlatformPin, Scenario, load_instance, load_scenario +from otdf_sdk_mgr.schema import ( + Instance, + KasPin, + PlatformPin, + Scenario, + load_yaml_mapping, +) def _install_platform_pin(pin: PlatformPin | KasPin) -> dict[str, str]: @@ -46,15 +52,23 @@ def install_scenario_cmd( typer.echo(f"Error: {path} not found", err=True) raise typer.Exit(1) - raw_kind = _peek_kind(path) + from ruamel.yaml.error import YAMLError + + try: + raw = load_yaml_mapping(path) + except YAMLError as e: + typer.echo(f"Error: {path} is not valid YAML: {e}", err=True) + raise typer.Exit(1) + + kind = raw.get("kind") if isinstance(raw.get("kind"), str) else None scenario: Scenario | None = None - if raw_kind == "Scenario": - scenario = load_scenario(path) + if kind == "Scenario": + scenario = Scenario.model_validate(raw) instance = scenario.instance - elif raw_kind == "Instance": - instance = load_instance(path) + elif kind == "Instance": + instance = Instance.model_validate(raw) else: - typer.echo(f"Error: {path} has unknown kind {raw_kind!r}", err=True) + typer.echo(f"Error: {path} has unknown kind {kind!r}", err=True) raise typer.Exit(1) installed: dict[str, object] = { @@ -96,20 +110,3 @@ def install_scenario_cmd( out.write_text(json.dumps(installed, indent=2) + "\n") typer.echo(f" Wrote {out}") - - -def _peek_kind(path: Path) -> str | None: - """Cheap pre-validation read so we can dispatch to the right model loader.""" - from ruamel.yaml import YAML - from ruamel.yaml.error import YAMLError - - y = YAML(typ="safe") - try: - raw = y.load(path.read_text()) - except YAMLError as e: - typer.echo(f"Error: {path} is not valid YAML: {e}", err=True) - raise typer.Exit(1) - if isinstance(raw, dict): - kind = raw.get("kind") - return kind if isinstance(kind, str) else None - return None diff --git a/otdf-sdk-mgr/src/otdf_sdk_mgr/platform_installer.py b/otdf-sdk-mgr/src/otdf_sdk_mgr/platform_installer.py index b278e9cb1..d979c0bc5 100644 --- a/otdf-sdk-mgr/src/otdf_sdk_mgr/platform_installer.py +++ b/otdf-sdk-mgr/src/otdf_sdk_mgr/platform_installer.py @@ -63,12 +63,15 @@ def _platform_bare_repo() -> Path: def _run(cmd: list[str], cwd: Path | None = None) -> None: - """Run a command, raising PlatformInstallError on failure.""" - result = subprocess.run(cmd, cwd=cwd, capture_output=True, text=True) + """Run a command, streaming output to the terminal. + + Long-running commands (`go build`, `git clone`) need live output so the + user can see progress. We don't capture; on failure the user has already + seen the diagnostics in their terminal. + """ + result = subprocess.run(cmd, cwd=cwd) if result.returncode != 0: - raise PlatformInstallError( - f"command failed: {' '.join(cmd)}\nstdout: {result.stdout}\nstderr: {result.stderr}" - ) + raise PlatformInstallError(f"command failed (exit {result.returncode}): {' '.join(cmd)}") def _ensure_bare_repo() -> Path: @@ -190,8 +193,10 @@ def install_helper_scripts(branch: str = HELPER_SCRIPTS_BRANCH) -> Path: print(f"Adding scripts worktree at {worktree} ({branch})...") _run(["git", f"--git-dir={bare}", "worktree", "add", str(worktree), branch]) else: + # Worktrees from a bare clone have no `origin` remote, so `git pull` + # fails. Reset to the (just-fetched) branch ref in the bare repo. print(f"Updating scripts worktree at {worktree}...") - _run(["git", "-C", str(worktree), "pull", "origin", branch]) + _run(["git", "-C", str(worktree), "reset", "--hard", branch]) src_scripts = worktree / "scripts" if not src_scripts.exists(): raise PlatformInstallError( diff --git a/otdf-sdk-mgr/src/otdf_sdk_mgr/schema.py b/otdf-sdk-mgr/src/otdf_sdk_mgr/schema.py index d91cf5042..9255badb6 100644 --- a/otdf-sdk-mgr/src/otdf_sdk_mgr/schema.py +++ b/otdf-sdk-mgr/src/otdf_sdk_mgr/schema.py @@ -194,7 +194,8 @@ def _yaml() -> YAML: return YAML(typ="safe") -def _load_yaml_mapping(path: str | Path) -> dict[str, object]: +def load_yaml_mapping(path: str | Path) -> dict[str, object]: + """Parse `path` as YAML and assert the top-level is a mapping.""" p = Path(path) raw = _yaml().load(p.read_text(encoding="utf-8")) if not isinstance(raw, dict): @@ -204,12 +205,12 @@ def _load_yaml_mapping(path: str | Path) -> dict[str, object]: def load_scenario(path: str | Path) -> Scenario: """Parse and validate a scenarios.yaml file.""" - return Scenario.model_validate(_load_yaml_mapping(path)) + return Scenario.model_validate(load_yaml_mapping(path)) def load_instance(path: str | Path) -> Instance: """Parse and validate an instance.yaml file.""" - return Instance.model_validate(_load_yaml_mapping(path)) + return Instance.model_validate(load_yaml_mapping(path)) def dump_instance(instance: Instance, path: str | Path) -> None: @@ -306,7 +307,7 @@ def _main(argv: list[str] | None = None) -> int: return 2 path = Path(args[1]) try: - raw = _load_yaml_mapping(path) + raw = load_yaml_mapping(path) except OSError as e: print(f"error: cannot read {path}: {e}", file=sys.stderr) return 1 From eb7683934a25bb7cbcabfe066eb6d15925d1a3b3 Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Thu, 21 May 2026 12:23:30 -0400 Subject: [PATCH 05/18] docs+chore: require uv run ruff/pyright pre-commit; fix cli_scenario typing - AGENTS.md: add "Before Committing Python Changes" section requiring `uv run ruff check`, `uv run ruff format`, and `uv run pyright` on any touched Python package before commit. Explicitly call out that `uvx` must NOT be used for pyright (isolated env can't see project deps, so every project import becomes a spurious "could not be resolved" error). - cli_scenario: split the single `dict[str, object]` install record into per-section typed containers (`installed_platform`, `installed_kas`, `installed_sdks`) assembled at write time via a `_snapshot()` helper. Fixes pre-existing pyright `__setitem__ ... not defined on object` errors at the nested writes; on-disk JSON shape is unchanged. Co-Authored-By: Claude Opus 4.7 --- otdf-sdk-mgr/src/otdf_sdk_mgr/cli_scenario.py | 31 ++++++++++++------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_scenario.py b/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_scenario.py index fa8c448d3..9fa0cdf2e 100644 --- a/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_scenario.py +++ b/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_scenario.py @@ -71,18 +71,26 @@ def install_scenario_cmd( typer.echo(f"Error: {path} has unknown kind {kind!r}", err=True) raise typer.Exit(1) - installed: dict[str, object] = { - "manifest": str(path), - "platform": None, - "kas": {}, - "sdks": {"encrypt": [], "decrypt": []}, - } + installed_platform: dict[str, str] | None = None + installed_kas: dict[str, dict[str, str]] = {} + installed_sdks: dict[str, list[dict[str, str | None]]] = {"encrypt": [], "decrypt": []} out = path.parent / f"{path.stem}.installed.json" + def _snapshot(status: str | None = None) -> dict[str, object]: + snap: dict[str, object] = { + "manifest": str(path), + "platform": installed_platform, + "kas": installed_kas, + "sdks": installed_sdks, + } + if status is not None: + snap["status"] = status + return snap + try: - installed["platform"] = _install_platform_pin(instance.platform) + installed_platform = _install_platform_pin(instance.platform) for kas_name, kas_pin in instance.kas.items(): - installed["kas"][kas_name] = _install_platform_pin(kas_pin) + installed_kas[kas_name] = _install_platform_pin(kas_pin) if not skip_scripts: install_helper_scripts() @@ -92,7 +100,7 @@ def install_scenario_cmd( dist_dir = install_release(entry.sdk, entry.version) install_paths[entry.install_key()] = str(dist_dir) for role in ("encrypt", "decrypt"): - installed["sdks"][role] = [ + installed_sdks[role] = [ { "sdk": entry.sdk, "version": entry.version, @@ -102,11 +110,10 @@ def install_scenario_cmd( for entry in getattr(scenario.sdks, role) ] except (PlatformInstallError, InstallError) as e: - installed["status"] = "partial" - out.write_text(json.dumps(installed, indent=2) + "\n") + out.write_text(json.dumps(_snapshot(status="partial"), indent=2) + "\n") typer.echo(f"Error: {e}", err=True) typer.echo(f" Wrote partial manifest to {out}", err=True) raise typer.Exit(1) - out.write_text(json.dumps(installed, indent=2) + "\n") + out.write_text(json.dumps(_snapshot(), indent=2) + "\n") typer.echo(f" Wrote {out}") From 5b75907f11a2ee98f569c6ca9d483740e87640e9 Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Thu, 21 May 2026 13:41:47 -0400 Subject: [PATCH 06/18] docs(agents): expand and reorganize AGENTS.md across packages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Root AGENTS.md: add a Repository Layout table near the top, correct the `platform/` description (it's installed by `otdf-sdk-mgr install`, not committed source), and trim the duplicated "Summary → Preferred Workflow" block that restated the body. - otdf-local/AGENTS.md: lead with the dependency on `otdf-sdk-mgr` (otdf-local launches the binaries the installer produces). Mark the manual-keys YAML block as an emergency fallback that may drift. - otdf-sdk-mgr/AGENTS.md (new): operational guide for the installer — subcommand layout, bare-clone-worktree gotchas (no `origin` remote, namespaced `service/vX.Y.Z` tags, unbuffered subprocess output), pattern for adding a new subcommand. - xtest/AGENTS.md (new): test-suite layout, custom pytest options, audit-log fixture quick reference, authoring guidance. - otdf-sdk-mgr/CLAUDE.md, xtest/CLAUDE.md: symlinks to AGENTS.md to match the repo convention. Co-Authored-By: Claude Opus 4.7 --- AGENTS.md | 10 +++---- otdf-local/AGENTS.md | 2 ++ otdf-sdk-mgr/AGENTS.md | 60 ++++++++++++++++++++++++++++++++++++++++++ otdf-sdk-mgr/CLAUDE.md | 1 + xtest/AGENTS.md | 3 ++- 5 files changed, 69 insertions(+), 7 deletions(-) create mode 100644 otdf-sdk-mgr/AGENTS.md create mode 120000 otdf-sdk-mgr/CLAUDE.md diff --git a/AGENTS.md b/AGENTS.md index 49f16c8f1..cfca4cde6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -7,10 +7,11 @@ This guide provides essential knowledge for AI agents performing updates, refact | Path | Purpose | Has its own AGENTS.md? | |------|---------|------------------------| | `xtest/` | pytest integration tests (the main test suite) | yes | -| `otdf-sdk-mgr/` | Python CLI that installs SDK CLIs from releases or source (see `otdf-sdk-mgr/README.md`) | no | +| `otdf-sdk-mgr/` | Python CLI that installs SDK CLIs and the platform service from releases or source | yes | | `otdf-local/` | Python CLI that runs/stops the platform + KAS instances locally | yes | | `vulnerability/` | Playwright UI test suite (run with `npx playwright test`) | no | -| `xtest/sdk/{go,java,js}/dist/` | Built SDK CLI wrappers, produced by `otdf-sdk-mgr install` (or by `cd xtest/sdk && make` for source builds) | n/a | +| `platform/` | Platform service source — **installed by `otdf-sdk-mgr install platform`**, not committed. Edits here may be wiped by a reinstall. | | +| `xtest/sdk/{go,java,js}/dist/` | Built SDK CLI wrappers, produced by `otdf-sdk-mgr install` (or by `cd xtest/sdk && make` for source builds) | | ## Test Framework Overview @@ -234,7 +235,4 @@ yq e '.services.kas.root_key' platform/opentdf-dev.yaml ## Closing Note -Test failures are usually configuration mismatches, not SDK bugs. Check -the local environment against what the tests expect before suspecting the -code. Per-subsystem details live in `xtest/AGENTS.md`, -`otdf-local/AGENTS.md`, and `otdf-sdk-mgr/README.md`. +The test failures are usually symptoms of configuration mismatches, not SDK bugs. Focus on ensuring the local environment matches what the tests expect. See the per-package guides in `xtest/`, `otdf-sdk-mgr/`, and `otdf-local/` for sub-system specifics. diff --git a/otdf-local/AGENTS.md b/otdf-local/AGENTS.md index 7e5a26347..0c116ef4b 100644 --- a/otdf-local/AGENTS.md +++ b/otdf-local/AGENTS.md @@ -2,6 +2,8 @@ This guide covers operational procedures for managing the test environment with `otdf-local`. For command reference, see [README.md](README.md). +**Depends on `otdf-sdk-mgr`.** `otdf-local` launches binaries that `otdf-sdk-mgr install platform` (or `otdf-sdk-mgr install scenario`) writes into `xtest/platform/dist/`. If `otdf-local up` complains that a binary is missing, run the installer first. + ## Environment Setup for pytest ```bash diff --git a/otdf-sdk-mgr/AGENTS.md b/otdf-sdk-mgr/AGENTS.md new file mode 100644 index 000000000..951b260b0 --- /dev/null +++ b/otdf-sdk-mgr/AGENTS.md @@ -0,0 +1,60 @@ +# otdf-sdk-mgr - Agent Guide + +Python CLI that installs SDK CLIs (`go`, `java`, `js`) and the OpenTDF +platform service from released artifacts or source. Outputs land in +`xtest/sdk/{go,java,js}/dist//` and `xtest/platform/dist//`. + +Full command reference: [README.md](README.md). + +## Subcommand Layout + +| File | Subcommand | Responsibility | +|------|------------|----------------| +| `cli_install.py` | `install {stable,lts,tip,release,scripts,artifact,scenario}` | All `install` subcommands; delegates per-SDK work to `installers.py` and platform work to `platform_installer.py`. | +| `cli_scenario.py` | `install scenario ` | Reads `scenarios.yaml` / `instance.yaml`, installs every referenced artifact, writes `.installed.json`. | +| `cli_versions.py` | `versions {list,latest}` | Lists released versions across registries. | +| `installers.py` | (lib) | Per-SDK install logic for go/java/js. | +| `platform_installer.py` | (lib) | Builds the platform `service` binary via git worktrees on a bare clone. | +| `schema.py` | (lib) | Pydantic models for `Scenario` / `Instance` + `load_yaml_mapping`. | + +## Platform Install via Git Worktrees + +`platform_installer.py` keeps a **bare clone** at `xtest/platform/src/platform.git` +and `git worktree add`s each requested ref into a sibling directory. A few +gotchas worth knowing before editing this module: + +- **Worktrees from a bare clone have no `origin` remote.** `git pull` inside + the worktree will fail. Update by fetching into the bare repo first + (`_ensure_bare_repo()` already does this), then `git -C reset + --hard ` to move the worktree HEAD to the refreshed ref. +- **Platform tags are namespaced** as `service/vX.Y.Z`. `_resolve_platform_ref` + prefixes the `service/` infix on plain versions; raw SHAs, refs with a + `/`, and `main`/`HEAD` pass through unchanged. +- Subprocess output is **not captured** — long-running `go build` / `git + clone` streams to the terminal so users can see progress. On failure the + error message just reports the command and exit code. + +## Before Committing + +Run from this directory: + +```bash +uv run ruff check . # lint — must pass +uv run ruff format . # auto-format — re-stage rewritten files +uv run pyright # type-check — must pass +uv run pytest -q # unit tests +``` + +Use `uv run`, **not `uvx`** — `uvx` strips the project venv, so pyright +reports every project import as unresolved. See the root `AGENTS.md` +("Before Committing Python Changes") for the rationale. + +## Adding a New Subcommand + +1. Create or extend a `cli_.py` module. +2. Register it in `cli.py` (the Typer app entry point), or — for `install` + subcommands — under `install_app` in `cli_install.py`. +3. Wrap any library exceptions (`InstallError`, `PlatformInstallError`) at + the CLI boundary and exit with `typer.Exit(1)`. The + `_install_platform_or_exit` helper in `cli_install.py` shows the + pattern for platform installers. diff --git a/otdf-sdk-mgr/CLAUDE.md b/otdf-sdk-mgr/CLAUDE.md new file mode 120000 index 000000000..47dc3e3d8 --- /dev/null +++ b/otdf-sdk-mgr/CLAUDE.md @@ -0,0 +1 @@ +AGENTS.md \ No newline at end of file diff --git a/xtest/AGENTS.md b/xtest/AGENTS.md index d588b4ce7..a9bb394ed 100644 --- a/xtest/AGENTS.md +++ b/xtest/AGENTS.md @@ -15,7 +15,8 @@ fixture system. | `conftest.py` | `pytest_addoption` + the encrypt/decrypt SDK parametrization. Defines `--sdks`, `--sdks-encrypt`, `--sdks-decrypt`, `--containers`, `--no-audit-logs`. | | `fixtures/` | Module-scoped pytest fixtures: `attributes.py`, `keys.py`, `audit.py`, `assertions.py`, `kas.py`, `encryption.py`, `obligations.py`. | | `tdfs.py` | SDK abstraction layer — wraps the `cli.sh` shims under `sdk//dist//`. | -| `sdk/{go,java,js}/dist//` | SDK CLI builds. Installed by `otdf-sdk-mgr install` (see `../otdf-sdk-mgr/README.md`). | +<<<<<<< HEAD +| `sdk/{go,java,js}/dist//` | SDK CLI builds. Installed by `otdf-sdk-mgr install` (see `../otdf-sdk-mgr/AGENTS.md`). | | `test.env` | Default endpoint and client-credential env vars. Source with `set -a && source test.env && set +a`. | ## Custom pytest Options (defined in `conftest.py`) From 270fb12c420da0dbd727e3792179e0f4a0bf33d9 Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Wed, 27 May 2026 15:26:58 -0400 Subject: [PATCH 07/18] feat(otdf-sdk-mgr): install tip --ref for branches, PRs, and SHAs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a `--ref` option to `install tip` so platform and SDK source builds can target any git ref (branches, tags, SHAs, raw `refs/...`, or the `pr:N` shorthand that expands to `refs/pull/N/head`). Mutable refs (branches, PR heads) re-fetch the bare repo and rebuild on each invocation; immutable refs (tags, full SHAs) reuse the cached dist. Also fetches `refs/...` refs explicitly into the bare repo before `git worktree add` — the default bare-clone refspec doesn't include `refs/pull/*`, so PR installs were dying with `invalid reference`. Co-Authored-By: Claude Opus 4.7 --- otdf-sdk-mgr/AGENTS.md | 15 +++- otdf-sdk-mgr/README.md | 15 ++++ otdf-sdk-mgr/src/otdf_sdk_mgr/checkout.py | 70 ++++++++++++++++--- otdf-sdk-mgr/src/otdf_sdk_mgr/cli_install.py | 28 +++++++- otdf-sdk-mgr/src/otdf_sdk_mgr/installers.py | 23 ++++-- .../src/otdf_sdk_mgr/platform_installer.py | 48 +++++++++++-- otdf-sdk-mgr/src/otdf_sdk_mgr/refs.py | 49 +++++++++++++ otdf-sdk-mgr/tests/test_platform_installer.py | 5 ++ otdf-sdk-mgr/tests/test_refs.py | 65 +++++++++++++++++ 9 files changed, 292 insertions(+), 26 deletions(-) create mode 100644 otdf-sdk-mgr/src/otdf_sdk_mgr/refs.py create mode 100644 otdf-sdk-mgr/tests/test_refs.py diff --git a/otdf-sdk-mgr/AGENTS.md b/otdf-sdk-mgr/AGENTS.md index 951b260b0..5ebeffa39 100644 --- a/otdf-sdk-mgr/AGENTS.md +++ b/otdf-sdk-mgr/AGENTS.md @@ -29,7 +29,20 @@ gotchas worth knowing before editing this module: --hard ` to move the worktree HEAD to the refreshed ref. - **Platform tags are namespaced** as `service/vX.Y.Z`. `_resolve_platform_ref` prefixes the `service/` infix on plain versions; raw SHAs, refs with a - `/`, and `main`/`HEAD` pass through unchanged. + `/`, `pr:N` shorthand (expanded to `refs/pull/N/head`), and `main`/`HEAD` + pass through unchanged. +- **PR refs aren't in the default bare-clone refspec.** `git clone --bare` + sets `+refs/heads/*:refs/heads/*`, so `fetch --all --tags` never pulls + `refs/pull/N/head`. `_ensure_worktree` (and the SDK equivalents in + `checkout.py`) explicitly `fetch origin +refs/...:refs/...` for any + `refs/...` ref before adding the worktree, otherwise `git worktree add + refs/pull/N/head` dies with `invalid reference`. +- **Mutable refs auto-refresh; immutable refs cache.** `refs.is_mutable_ref` + treats full-length hex SHAs and `/vX.Y.Z` tag forms as immutable + (reuse existing dist), and everything else as mutable (re-fetch the bare + repo, `git reset --hard` the worktree, drop the stale binary, rebuild). + Don't reintroduce an unconditional dist-exists skip — it silently serves + stale binaries when a user installs the same branch a second time. - Subprocess output is **not captured** — long-running `go build` / `git clone` streams to the terminal so users can see progress. On failure the error message just reports the command and exit code. diff --git a/otdf-sdk-mgr/README.md b/otdf-sdk-mgr/README.md index 44a39e027..3ad5fd30c 100644 --- a/otdf-sdk-mgr/README.md +++ b/otdf-sdk-mgr/README.md @@ -28,6 +28,12 @@ otdf-sdk-mgr install release go:v0.24.0 js:0.4.0 java:v0.9.0 otdf-sdk-mgr install tip otdf-sdk-mgr install tip go # Single SDK +# Install from a feature branch, PR, tag, or SHA (source build) +otdf-sdk-mgr install tip --ref my-feature-branch platform +otdf-sdk-mgr install tip --ref pr:42 go # pr:N → refs/pull/N/head +otdf-sdk-mgr install tip --ref abc123f4 platform # SHA (cached on re-run) +otdf-sdk-mgr install tip --ref refs/pull/42/head go # raw ref + # Install a published version with optional dist name (defaults to version tag) otdf-sdk-mgr install artifact --sdk go --version v0.24.0 otdf-sdk-mgr install artifact --sdk go --version v0.24.0 --dist-name my-tag @@ -66,6 +72,15 @@ otdf-sdk-mgr java-fixup Source builds (`tip` mode) check out source to `sdk/{lang}/src/` and compile via `make` to `sdk/{lang}/dist/`. For Go, the platform monorepo is cloned to `sdk/go/platform-src/{ref}/` and `sdk/go/src/{ref}` is a symlink to its `otdfctl/` subdirectory; `make` discovers the platform's top-level `go.work` automatically so `protocol/go`, `sdk`, and `lib/*` resolve to the local checkout. +`--ref` accepts branches, tags, SHAs, the `pr:N` shorthand (expands to +`refs/pull/N/head`), and raw `refs/...` paths. Mutable refs (branches and +PR heads) are re-fetched and rebuilt on every invocation so the dist +matches the latest upstream commit; immutable refs (tags and full-length +SHAs) reuse the cached build for speed. Dist directories are named after a +slug of the ref (e.g. `dist/my-branch/`, `dist/pull--42--head/`); `--ref +main` keeps the historical `dist/tip/` (platform) and `dist/main/` (SDK) +names. + After changes to SDK source, rebuild: ```bash diff --git a/otdf-sdk-mgr/src/otdf_sdk_mgr/checkout.py b/otdf-sdk-mgr/src/otdf_sdk_mgr/checkout.py index fd9357e77..29b9cfca9 100644 --- a/otdf-sdk-mgr/src/otdf_sdk_mgr/checkout.py +++ b/otdf-sdk-mgr/src/otdf_sdk_mgr/checkout.py @@ -8,6 +8,7 @@ from typing import Any from otdf_sdk_mgr.config import SDK_BARE_REPOS, SDK_GIT_URLS, get_sdk_dirs +from otdf_sdk_mgr.refs import expand_pr_shorthand, is_mutable_ref def _run(cmd: list[str], **kwargs: Any) -> None: @@ -17,9 +18,13 @@ def _run(cmd: list[str], **kwargs: Any) -> None: raise subprocess.CalledProcessError(result.returncode, cmd) -def checkout_sdk_branch(language: str, branch: str) -> None: +def checkout_sdk_branch(language: str, branch: str) -> str: """Clone bare repo and create/update a worktree for the given branch. + Returns the slug used for the local worktree directory under + `sdk//src/` — callers use this as the dist-dir name and + as the `VERSIONS=` override when invoking `make`. + For "go", use checkout_go_from_platform instead — the standalone opentdf/otdfctl repo is archived; otdfctl source builds come from the platform monorepo. @@ -36,6 +41,8 @@ def checkout_sdk_branch(language: str, branch: str) -> None: f"Unsupported language '{language}'. Supported values are: {', '.join(sdk_dirs)}" ) + branch = expand_pr_shorthand(branch) + sdk_dir = sdk_dirs[language] bare_repo_name = SDK_BARE_REPOS[language] # Strip .git suffix to get the base URL for git clone @@ -52,11 +59,31 @@ def checkout_sdk_branch(language: str, branch: str) -> None: _run(["git", "clone", "--bare", repo_url, str(bare_repo_path)]) else: print(f"Bare repository already exists at {bare_repo_path}. Fetching updates...") - _run(["git", f"--git-dir={bare_repo_path}", "fetch", "--all"]) + _run(["git", f"--git-dir={bare_repo_path}", "fetch", "--all", "--tags"]) + + # PR refs (`refs/pull/N/head`) aren't in the default bare-clone refspec; + # fetch any explicit `refs/...` ref by name so worktree-add can find it. + if branch.startswith("refs/"): + print(f"Fetching {branch} into bare repo...") + _run( + [ + "git", + f"--git-dir={bare_repo_path}", + "fetch", + "origin", + f"+{branch}:{branch}", + ] + ) if worktree_path.exists(): - print(f"Worktree for branch '{branch}' already exists at {worktree_path}. Updating...") - _run(["git", "-C", str(worktree_path), "pull", "origin", branch]) + if is_mutable_ref(branch): + print(f"Worktree exists at {worktree_path}; resetting to '{branch}'.") + # Worktrees from a bare clone have no `origin` remote, so reset + # to the just-fetched ref in the bare repo instead of `git pull`. + _run(["git", "-C", str(worktree_path), "fetch", "origin", branch]) + _run(["git", "-C", str(worktree_path), "reset", "--hard", "FETCH_HEAD"]) + else: + print(f"Worktree for '{branch}' already exists at {worktree_path}; reusing.") else: print(f"Setting up worktree for branch '{branch}' at {worktree_path}...") _run( @@ -69,15 +96,21 @@ def checkout_sdk_branch(language: str, branch: str) -> None: branch, ] ) + return local_name def checkout_go_from_platform(ref: str) -> Path: - """Check out the opentdf/platform monorepo and arrange xtest/sdk/go/src/ + """Check out the opentdf/platform monorepo and arrange xtest/sdk/go/src/ as a symlink to the otdfctl subdir of that checkout. Returns the platform checkout directory so callers can locate go.work for - GOWORK-based source builds. + GOWORK-based source builds. The slug (`Path.name` of the worktree dir) is + also the dist-dir name used by the go SDK Makefile. + + Accepts `pr:N` shorthand in addition to branches, tags, and SHAs. """ + ref = expand_pr_shorthand(ref) + go_dir = get_sdk_dirs()["go"] platform_url = SDK_GIT_URLS["platform"].removesuffix(".git") platform_src_dir = go_dir / "platform-src" @@ -94,12 +127,29 @@ def checkout_go_from_platform(ref: str) -> Path: _run(["git", "clone", "--bare", platform_url, str(bare_repo_path)]) else: print(f"Bare repository already exists at {bare_repo_path}. Fetching updates...") - _run(["git", f"--git-dir={bare_repo_path}", "fetch", "--all"]) + _run(["git", f"--git-dir={bare_repo_path}", "fetch", "--all", "--tags"]) + + # PR refs (`refs/pull/N/head`) aren't in the default bare-clone refspec; + # fetch any explicit `refs/...` ref by name so worktree-add can find it. + if ref.startswith("refs/"): + print(f"Fetching {ref} into bare repo...") + _run( + [ + "git", + f"--git-dir={bare_repo_path}", + "fetch", + "origin", + f"+{ref}:{ref}", + ] + ) if worktree_path.exists(): - print(f"Worktree for ref '{ref}' already exists at {worktree_path}. Updating...") - _run(["git", f"--git-dir={bare_repo_path}", "fetch", "origin", ref, "--tags"]) - _run(["git", "-C", str(worktree_path), "checkout", "--force", "FETCH_HEAD"]) + if is_mutable_ref(ref): + print(f"Worktree for ref '{ref}' exists at {worktree_path}; resetting.") + _run(["git", f"--git-dir={bare_repo_path}", "fetch", "origin", ref, "--tags"]) + _run(["git", "-C", str(worktree_path), "checkout", "--force", "FETCH_HEAD"]) + else: + print(f"Worktree for ref '{ref}' already exists at {worktree_path}; reusing.") else: print(f"Setting up worktree for ref '{ref}' at {worktree_path}...") _run( diff --git a/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_install.py b/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_install.py index 6bf0a4895..722aa91bd 100644 --- a/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_install.py +++ b/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_install.py @@ -86,16 +86,38 @@ def tip( Optional[list[str]], typer.Argument(help="SDKs to build from source (default: all)"), ] = None, + ref: Annotated[ + str, + typer.Option( + "--ref", + help=( + "Git ref to build (branch, tag, SHA, `pr:N`, or `refs/...`). " + "Default `main`. Mutable refs (branches, PR heads) are " + "re-fetched and rebuilt on each invocation; immutable refs " + "(tags, SHAs) reuse the existing build." + ), + ), + ] = "main", ) -> None: - """Source checkout + build from main.""" + """Source checkout + build at a git ref (default: main). + + Examples: + otdf-sdk-mgr install tip # main, all SDKs + otdf-sdk-mgr install tip --ref my-branch platform + otdf-sdk-mgr install tip --ref pr:42 go + otdf-sdk-mgr install tip --ref abc123f platform java + """ from otdf_sdk_mgr.installers import cmd_tip from otdf_sdk_mgr.platform_installer import install_platform_source want_platform, sdk_targets = _split_platform(sdks or ALL_SDKS) + # Preserve historical `dist/tip/` naming when --ref is omitted; otherwise + # let the platform installer slugify the ref. + platform_dist_name = "tip" if ref == "main" else None if want_platform: - _install_platform_or_exit(install_platform_source, "main", dist_name="tip") + _install_platform_or_exit(install_platform_source, ref, dist_name=platform_dist_name) if sdk_targets: - cmd_tip(sdk_targets) + cmd_tip(sdk_targets, ref=ref) @install_app.command() diff --git a/otdf-sdk-mgr/src/otdf_sdk_mgr/installers.py b/otdf-sdk-mgr/src/otdf_sdk_mgr/installers.py index f3c11b7da..9d0e42ba0 100644 --- a/otdf-sdk-mgr/src/otdf_sdk_mgr/installers.py +++ b/otdf-sdk-mgr/src/otdf_sdk_mgr/installers.py @@ -204,22 +204,35 @@ def cmd_lts(sdks: list[str]) -> None: install_release(sdk, version) -def cmd_tip(sdks: list[str]) -> None: - """Delegate to source checkout + make for head builds.""" +def cmd_tip(sdks: list[str], ref: str = "main") -> None: + """Delegate to source checkout + make for source builds at `ref`. + + `ref` defaults to `main` to preserve historical behavior. May be a branch, + tag, SHA, `pr:N` shorthand, or raw `refs/...`. The slug derived from the + ref is passed to `make` via `VERSIONS=` so only that checkout is + (re)built, not every directory under `sdk//src/`. + """ import os from otdf_sdk_mgr.checkout import checkout_go_from_platform + from otdf_sdk_mgr.refs import expand_pr_shorthand + + expanded = expand_pr_shorthand(ref) + # Slug for SDK src/ and dist/ — mirrors the per-SDK + # `local_name` calculation in checkout.py. + slug = expanded.replace("/", "--").removeprefix("sdk--").removeprefix("otdfctl--") sdk_dirs = get_sdk_dirs() for sdk in sdks: - print(f"Checking out and building {sdk} from source...") + print(f"Checking out and building {sdk} from source ({expanded})...") make_dir = sdk_dirs[sdk] env = os.environ.copy() + env["VERSIONS"] = slug if sdk == "go": - platform_dir = checkout_go_from_platform("main") + platform_dir = checkout_go_from_platform(expanded) env["GOWORK"] = str(platform_dir / "go.work") else: - checkout_sdk_branch(sdk, "main") + checkout_sdk_branch(sdk, expanded) subprocess.check_call(["make"], cwd=make_dir, env=env) print(f" {sdk} built from source") diff --git a/otdf-sdk-mgr/src/otdf_sdk_mgr/platform_installer.py b/otdf-sdk-mgr/src/otdf_sdk_mgr/platform_installer.py index d979c0bc5..2d821d39e 100644 --- a/otdf-sdk-mgr/src/otdf_sdk_mgr/platform_installer.py +++ b/otdf-sdk-mgr/src/otdf_sdk_mgr/platform_installer.py @@ -18,6 +18,7 @@ from pathlib import Path from otdf_sdk_mgr.config import SDK_GIT_URLS, SDK_TAG_INFIXES +from otdf_sdk_mgr.refs import expand_pr_shorthand, is_mutable_ref, ref_slug from otdf_sdk_mgr.semver import normalize_version PLATFORM_BARE_REPO = "platform.git" @@ -92,8 +93,11 @@ def _resolve_platform_ref(version_or_ref: str) -> str: """Turn a user-supplied version into the actual git ref to checkout. `v0.9.0` → `service/v0.9.0` (matches SDK_TAG_INFIXES["platform"]). + `pr:42` → `refs/pull/42/head` (expanded first, then passes through the + `/` check). A ref that already contains `/`, a hex SHA, or `main` is returned as-is. """ + version_or_ref = expand_pr_shorthand(version_or_ref) infix = SDK_TAG_INFIXES.get("platform", "service") if "/" in version_or_ref or version_or_ref in ("main", "HEAD"): return version_or_ref @@ -105,16 +109,35 @@ def _resolve_platform_ref(version_or_ref: str) -> str: def _worktree_path_for(ref: str) -> Path: - safe = ref.replace("/", "--") - return _platform_src_root() / safe + return _platform_src_root() / ref_slug(ref) def _ensure_worktree(ref: str) -> Path: - """Create (or reuse) a git worktree at the given platform ref.""" + """Create (or reuse) a git worktree at the given platform ref. + + For mutable refs (branches, PR heads), `_ensure_bare_repo` has already + re-fetched, and we reset the worktree HEAD to the freshly-fetched ref so + a subsequent install picks up new commits. For immutable refs (tags, + SHAs) we just reuse. + """ bare = _ensure_bare_repo() + # The bare clone's default refspec is `+refs/heads/*:refs/heads/*` plus + # `--tags`, so GitHub PR refs (`refs/pull/N/head`) are never pulled. + # Fetch any explicit `refs/...` ref by name into the bare repo before we + # try to use it. + if ref.startswith("refs/"): + print(f"Fetching {ref} into bare repo...") + _run(["git", f"--git-dir={bare}", "fetch", "origin", f"+{ref}:{ref}"]) worktree = _worktree_path_for(ref) if worktree.exists(): - print(f"Worktree already exists at {worktree}; reusing.") + if is_mutable_ref(ref): + print(f"Worktree exists at {worktree}; resetting to {ref}.") + # Worktrees from a bare clone have no `origin` remote, so we + # reset to the bare repo's just-fetched ref. Mirrors the + # `install_helper_scripts` pattern below. + _run(["git", "-C", str(worktree), "reset", "--hard", ref]) + else: + print(f"Worktree already exists at {worktree}; reusing.") return worktree print(f"Adding worktree at {worktree} for ref {ref}...") _run(["git", f"--git-dir={bare}", "worktree", "add", "--detach", str(worktree), ref]) @@ -156,14 +179,25 @@ def install_platform_source(ref: str, dist_name: str | None = None) -> Path: """Install a platform build by checking out and building `ref`. `ref` may be a plain version (`v0.9.0`), a namespaced tag - (`service/v0.9.0`), a branch (`main`), or a SHA. Returns the dist dir. + (`service/v0.9.0`), a branch (`main`), a PR shorthand (`pr:42`), a raw + ref (`refs/pull/42/head`), or a SHA. Returns the dist dir. + + For immutable refs (tags, SHAs) an existing dist is reused. For mutable + refs (branches, PR heads) the bare repo is re-fetched, the worktree is + reset, the old dist is removed, and the service binary is rebuilt. """ full_ref = _resolve_platform_ref(ref) - dist_dir = _platform_dist_root() / (dist_name or normalize_version(ref)) - if (dist_dir / "service").exists(): + if dist_name is None: + dist_name = ref_slug(full_ref) if is_mutable_ref(full_ref) else normalize_version(ref) + dist_dir = _platform_dist_root() / dist_name + binary = dist_dir / "service" + if binary.exists() and not is_mutable_ref(full_ref): print(f" Dist already present at {dist_dir}; skipping build.") return dist_dir worktree = _ensure_worktree(full_ref) + if binary.exists(): + # Mutable ref: drop the stale binary so `_build_service` actually rebuilds. + binary.unlink() _build_service(worktree, dist_dir) _record_version(dist_dir, full_ref, worktree) print(f" Platform {ref} → {dist_dir}") diff --git a/otdf-sdk-mgr/src/otdf_sdk_mgr/refs.py b/otdf-sdk-mgr/src/otdf_sdk_mgr/refs.py new file mode 100644 index 000000000..c307f1c80 --- /dev/null +++ b/otdf-sdk-mgr/src/otdf_sdk_mgr/refs.py @@ -0,0 +1,49 @@ +"""Git-ref helpers shared by platform and SDK installers.""" + +from __future__ import annotations + +import re + +from otdf_sdk_mgr.semver import SEMVER_RE + +_PR_SHORTHAND_RE = re.compile(r"^pr:(\d+)$") +_HEX_SHA_LEN = (40, 64) + + +def expand_pr_shorthand(ref: str) -> str: + """Expand `pr:N` (GitHub PR shorthand) to `refs/pull/N/head`. + + Other refs pass through unchanged. + """ + m = _PR_SHORTHAND_RE.match(ref) + if m: + return f"refs/pull/{m.group(1)}/head" + return ref + + +def is_mutable_ref(ref: str) -> bool: + """True if `ref` can change over time (branches, PR heads); False for tags/SHAs. + + Used to decide whether to re-fetch and rebuild on subsequent installs. + + The caller is expected to have already expanded `pr:N` shorthand and any + tag-namespace prefixes (e.g. `service/v0.9.0`, `sdk/v0.4.0`). + """ + if len(ref) in _HEX_SHA_LEN and all(c in "0123456789abcdef" for c in ref.lower()): + return False + # Namespaced tag refs from monorepo: `service/v0.9.0`, `sdk/v0.4.0`, + # `otdfctl/v0.31.0`. Anything ending in a clean semver after the last + # `/` is treated as an immutable tag. + tail = ref.rsplit("/", 1)[-1] + if SEMVER_RE.match(tail): + return False + return True + + +def ref_slug(ref: str) -> str: + """Slugify a git ref for use as a directory name. + + Mirrors the pattern already used by `_worktree_path_for` and + `checkout_sdk_branch`: replace `/` with `--`. Idempotent. + """ + return ref.replace("/", "--") diff --git a/otdf-sdk-mgr/tests/test_platform_installer.py b/otdf-sdk-mgr/tests/test_platform_installer.py index 795054338..96b6ff58f 100644 --- a/otdf-sdk-mgr/tests/test_platform_installer.py +++ b/otdf-sdk-mgr/tests/test_platform_installer.py @@ -17,6 +17,11 @@ ("b" * 64, "b" * 64), ("abc1234", "service/vabc1234"), ("deadbeef", "service/vdeadbeef"), + # PR shorthand expands before the `/` check, then passes through. + ("pr:42", "refs/pull/42/head"), + ("pr:1234", "refs/pull/1234/head"), + # Raw refs are passed through unchanged. + ("refs/pull/7/head", "refs/pull/7/head"), ], ) def test_resolve_platform_ref(inp, expected): diff --git a/otdf-sdk-mgr/tests/test_refs.py b/otdf-sdk-mgr/tests/test_refs.py new file mode 100644 index 000000000..da77f78bf --- /dev/null +++ b/otdf-sdk-mgr/tests/test_refs.py @@ -0,0 +1,65 @@ +"""Tests for the ref-handling helpers.""" + +import pytest + +from otdf_sdk_mgr.refs import expand_pr_shorthand, is_mutable_ref, ref_slug + + +@pytest.mark.parametrize( + "inp,expected", + [ + ("pr:42", "refs/pull/42/head"), + ("pr:1", "refs/pull/1/head"), + ("pr:12345", "refs/pull/12345/head"), + ("main", "main"), + ("v0.9.0", "v0.9.0"), + ("feature/pr-42", "feature/pr-42"), # `/` disambiguates from shorthand + ("refs/pull/42/head", "refs/pull/42/head"), + ("pr:abc", "pr:abc"), # non-numeric, not shorthand + ("", ""), + ], +) +def test_expand_pr_shorthand(inp, expected): + assert expand_pr_shorthand(inp) == expected + + +@pytest.mark.parametrize( + "ref,expected", + [ + # Immutable: tags and tag-like refs + ("v0.9.0", False), + ("v1.2.3", False), + ("service/v0.9.0", False), + ("sdk/v0.4.0", False), + ("otdfctl/v0.31.0", False), + # Immutable: SHAs + ("a" * 40, False), + ("b" * 64, False), + ("0123456789abcdef0123456789abcdef01234567", False), + # Mutable: branches and PR heads + ("main", True), + ("HEAD", True), + ("feature/my-branch", True), + ("DSPX-3302-02-platform-installer", True), + ("refs/pull/42/head", True), + # Mutable: short hex (could be a SHA, but also could be a branch); + # treated as mutable since it's not full-length. + ("abc1234", True), + ], +) +def test_is_mutable_ref(ref, expected): + assert is_mutable_ref(ref) is expected + + +@pytest.mark.parametrize( + "inp,expected", + [ + ("main", "main"), + ("feature/x", "feature--x"), + ("refs/pull/42/head", "refs--pull--42--head"), + ("service/v0.9.0", "service--v0.9.0"), + ("a" * 40, "a" * 40), + ], +) +def test_ref_slug(inp, expected): + assert ref_slug(inp) == expected From acd089936f951ec3e452b293e35fc2f00e538c55 Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Wed, 27 May 2026 15:35:12 -0400 Subject: [PATCH 08/18] fixup addd platform source to gitignore --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index ecb9a979f..a1bb32382 100644 --- a/.gitignore +++ b/.gitignore @@ -28,6 +28,7 @@ xtest/sdk/java/cmdline.jar /xtest/platform/ /xtest/java-sdk/ /xtest/sdk/go/otdfctl +/xtest/sdk/go/platform-src/ /xtest/otdfctl/ /tmp/ From 4c159a5123d96080b4e33f09a11fa8bd2115d65c Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Wed, 27 May 2026 16:36:31 -0400 Subject: [PATCH 09/18] fix(otdf-sdk-mgr): normalize only semver tail for immutable platform refs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For immutable refs (tags, SHAs), derive dist_name by normalizing only the semver tail after the last `/`. This ensures namespaced tags like `service/v0.9.0` produce the same dist_name (`v0.9.0`) as plain tags (`v0.9.0`, `0.9.0`), enabling immutable ref dist-dir reuse. Before: `normalize_version(ref)` on `service/v0.9.0` → `vservice/v0.9.0` After: `normalize_version(ref.rsplit("/", 1)[-1])` → `v0.9.0` Also add `list_platform_versions()` to registry and expose platform versions via `versions list platform`. Co-Authored-By: Claude Sonnet 4.5 --- otdf-sdk-mgr/src/otdf_sdk_mgr/cli_versions.py | 8 +++-- .../src/otdf_sdk_mgr/platform_installer.py | 7 +++- otdf-sdk-mgr/src/otdf_sdk_mgr/registry.py | 35 +++++++++++++++++++ otdf-sdk-mgr/tests/test_platform_installer.py | 31 ++++++++++++++++ 4 files changed, 78 insertions(+), 3 deletions(-) diff --git a/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_versions.py b/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_versions.py index 19188b124..9a472c11d 100644 --- a/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_versions.py +++ b/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_versions.py @@ -19,7 +19,7 @@ def list_versions( sdk: Annotated[ str, - typer.Argument(help="SDK to query (go, js, java, all)"), + typer.Argument(help="SDK to query (go, js, java, platform, all)"), ] = "all", stable: Annotated[bool, typer.Option("--stable", help="Only stable versions")] = False, latest: Annotated[ @@ -39,9 +39,10 @@ def list_versions( list_java_github_releases, list_java_maven_versions, list_js_versions, + list_platform_versions, ) - sdks = ["go", "js", "java"] if sdk == "all" else [sdk] + sdks = ["go", "js", "java", "platform"] if sdk == "all" else [sdk] all_entries: list[dict[str, Any]] = [] for s in sdks: @@ -57,6 +58,9 @@ def list_versions( if releases: gh_entries = list_java_github_releases() all_entries.extend(apply_filters(gh_entries, stable_only=stable, latest_n=latest)) + elif s == "platform": + entries = list_platform_versions() + all_entries.extend(apply_filters(entries, stable_only=stable, latest_n=latest)) if output_table: _print_rich_table(all_entries) diff --git a/otdf-sdk-mgr/src/otdf_sdk_mgr/platform_installer.py b/otdf-sdk-mgr/src/otdf_sdk_mgr/platform_installer.py index 2d821d39e..ef0c06346 100644 --- a/otdf-sdk-mgr/src/otdf_sdk_mgr/platform_installer.py +++ b/otdf-sdk-mgr/src/otdf_sdk_mgr/platform_installer.py @@ -188,7 +188,12 @@ def install_platform_source(ref: str, dist_name: str | None = None) -> Path: """ full_ref = _resolve_platform_ref(ref) if dist_name is None: - dist_name = ref_slug(full_ref) if is_mutable_ref(full_ref) else normalize_version(ref) + if is_mutable_ref(full_ref): + dist_name = ref_slug(full_ref) + else: + # For immutable refs (tags, SHAs), normalize only the semver tail so + # namespaced tags like `service/v0.9.0` produce the same dist_name as `v0.9.0`. + dist_name = normalize_version(full_ref.rsplit("/", 1)[-1]) dist_dir = _platform_dist_root() / dist_name binary = dist_dir / "service" if binary.exists() and not is_mutable_ref(full_ref): diff --git a/otdf-sdk-mgr/src/otdf_sdk_mgr/registry.py b/otdf-sdk-mgr/src/otdf_sdk_mgr/registry.py index 187e08fe9..58107d204 100644 --- a/otdf-sdk-mgr/src/otdf_sdk_mgr/registry.py +++ b/otdf-sdk-mgr/src/otdf_sdk_mgr/registry.py @@ -16,6 +16,7 @@ SDK_GIT_URLS, SDK_MAVEN_COORDS, SDK_NPM_PACKAGES, + SDK_TAG_INFIXES, go_module_for_tag, ) from otdf_sdk_mgr.semver import is_stable, parse_semver, semver_sort_key @@ -199,6 +200,40 @@ def list_java_github_releases() -> list[dict[str, Any]]: return results +def list_platform_versions() -> list[dict[str, Any]]: + """List platform service versions from `service/vX.Y.Z` tags in opentdf/platform.""" + from git import Git + + results: list[dict[str, Any]] = [] + raw = Git().ls_remote(SDK_GIT_URLS["platform"], tags=True) + infix = SDK_TAG_INFIXES.get("platform", "service") + + for line in raw.strip().split("\n"): + if not line: + continue + _, ref = line.split("\t", 1) + if ref.endswith("^{}"): + continue + tag = ref.removeprefix("refs/tags/") + if not tag.startswith(f"{infix}/"): + continue + version = tag.removeprefix(f"{infix}/") + version_normalized = version.lstrip("v") + if not parse_semver(version_normalized): + continue + results.append( + { + "sdk": "platform", + "version": version_normalized, + "source": "platform-git-tag", + "stable": is_stable(version_normalized), + "install_method": f"otdf-sdk-mgr install platform {version_normalized}", + } + ) + results.sort(key=lambda r: semver_sort_key(r["version"])) + return results + + def apply_filters( entries: list[dict[str, Any]], *, diff --git a/otdf-sdk-mgr/tests/test_platform_installer.py b/otdf-sdk-mgr/tests/test_platform_installer.py index 96b6ff58f..7b23cfce2 100644 --- a/otdf-sdk-mgr/tests/test_platform_installer.py +++ b/otdf-sdk-mgr/tests/test_platform_installer.py @@ -3,6 +3,8 @@ import pytest from otdf_sdk_mgr.platform_installer import _resolve_platform_ref +from otdf_sdk_mgr.refs import is_mutable_ref, ref_slug +from otdf_sdk_mgr.semver import normalize_version @pytest.mark.parametrize( @@ -26,3 +28,32 @@ ) def test_resolve_platform_ref(inp, expected): assert _resolve_platform_ref(inp) == expected + + +@pytest.mark.parametrize( + "ref,expected_dist_name", + [ + # Immutable refs: namespaced tags and plain tags should produce the same dist_name + ("service/v0.9.0", "v0.9.0"), + ("v0.9.0", "v0.9.0"), + ("0.9.0", "v0.9.0"), + # Mutable refs: use ref_slug + ("main", "main"), + ("refs/pull/42/head", "refs--pull--42--head"), + # SHAs: immutable, normalize the tail (which is the full SHA) + ("a" * 40, "v" + "a" * 40), + ], +) +def test_dist_name_derivation(ref, expected_dist_name): + """Verify that dist_name is derived consistently for immutable refs. + + Namespaced tags like `service/v0.9.0` should produce the same dist_name as + plain tags `v0.9.0` or `0.9.0`, ensuring immutable refs reuse existing dist dirs. + """ + full_ref = _resolve_platform_ref(ref) + if is_mutable_ref(full_ref): + dist_name = ref_slug(full_ref) + else: + # Normalize only the semver tail for immutable refs + dist_name = normalize_version(full_ref.rsplit("/", 1)[-1]) + assert dist_name == expected_dist_name From 959a96d46327ab1142d8248947b22dc1948e8966 Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Thu, 28 May 2026 08:37:31 -0400 Subject: [PATCH 10/18] fix(otdf-sdk-mgr): address PR review critical issues - Remove stray <<<<<<< HEAD merge marker from xtest/AGENTS.md - Disambiguate 7-39 char hex refs in platform_installer via `git rev-parse --verify`; ambiguous prefixes raise PlatformInstallError, unresolvable hex falls through as a branch/tag name - Make `install_go_release` fail loudly on `go install` pre-warm errors; no more silent .version writes after a broken install - Add `RegistryUnreachableError` and raise it from npm/Maven/GitHub URLError paths so network outages no longer look like "no versions available"; CLI wrappers translate to clean typer.Exit(1) - Fix `versions {list,latest}` typo in AGENTS.md (subcommands are `list` and `resolve`) Co-Authored-By: Claude Opus 4.7 --- otdf-sdk-mgr/AGENTS.md | 2 +- otdf-sdk-mgr/src/otdf_sdk_mgr/cli_install.py | 20 ++++++-- otdf-sdk-mgr/src/otdf_sdk_mgr/cli_versions.py | 41 ++++++++++------ otdf-sdk-mgr/src/otdf_sdk_mgr/installers.py | 19 ++++--- .../src/otdf_sdk_mgr/platform_installer.py | 49 ++++++++++++++++--- otdf-sdk-mgr/src/otdf_sdk_mgr/registry.py | 19 ++++--- otdf-sdk-mgr/tests/test_platform_installer.py | 6 ++- xtest/AGENTS.md | 1 - 8 files changed, 114 insertions(+), 43 deletions(-) diff --git a/otdf-sdk-mgr/AGENTS.md b/otdf-sdk-mgr/AGENTS.md index 5ebeffa39..ba900ae52 100644 --- a/otdf-sdk-mgr/AGENTS.md +++ b/otdf-sdk-mgr/AGENTS.md @@ -12,7 +12,7 @@ Full command reference: [README.md](README.md). |------|------------|----------------| | `cli_install.py` | `install {stable,lts,tip,release,scripts,artifact,scenario}` | All `install` subcommands; delegates per-SDK work to `installers.py` and platform work to `platform_installer.py`. | | `cli_scenario.py` | `install scenario ` | Reads `scenarios.yaml` / `instance.yaml`, installs every referenced artifact, writes `.installed.json`. | -| `cli_versions.py` | `versions {list,latest}` | Lists released versions across registries. | +| `cli_versions.py` | `versions {list,resolve}` | Lists released versions (`--latest N` filters most-recent) and resolves tags to SHAs. | | `installers.py` | (lib) | Per-SDK install logic for go/java/js. | | `platform_installer.py` | (lib) | Builds the platform `service` binary via git worktrees on a bare clone. | | `schema.py` | (lib) | Pydantic models for `Scenario` / `Instance` + `load_yaml_mapping`. | diff --git a/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_install.py b/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_install.py index 722aa91bd..ce2229b9d 100644 --- a/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_install.py +++ b/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_install.py @@ -52,9 +52,17 @@ def stable( ] = None, ) -> None: """Install latest stable releases for each SDK.""" - from otdf_sdk_mgr.installers import cmd_stable + from otdf_sdk_mgr.installers import InstallError, cmd_stable + from otdf_sdk_mgr.registry import RegistryUnreachableError - cmd_stable(sdks or ALL_SDKS) + try: + cmd_stable(sdks or ALL_SDKS) + except RegistryUnreachableError as e: + typer.echo(f"Error: could not reach version registry — check network: {e}", err=True) + raise typer.Exit(1) + except InstallError as e: + typer.echo(f"Error: {e}", err=True) + raise typer.Exit(1) @install_app.command() @@ -66,7 +74,7 @@ def lts( ) -> None: """Install LTS versions for each SDK.""" from otdf_sdk_mgr.config import LTS_VERSIONS - from otdf_sdk_mgr.installers import cmd_lts + from otdf_sdk_mgr.installers import InstallError, cmd_lts from otdf_sdk_mgr.platform_installer import install_platform_release want_platform, sdk_targets = _split_platform(sdks or ALL_SDKS) @@ -77,7 +85,11 @@ def lts( raise typer.Exit(1) _install_platform_or_exit(install_platform_release, version) if sdk_targets: - cmd_lts(sdk_targets) + try: + cmd_lts(sdk_targets) + except InstallError as e: + typer.echo(f"Error: {e}", err=True) + raise typer.Exit(1) @install_app.command() diff --git a/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_versions.py b/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_versions.py index 9a472c11d..5e1acb68f 100644 --- a/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_versions.py +++ b/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_versions.py @@ -34,6 +34,7 @@ def list_versions( ) -> None: """List available released versions of SDK CLIs.""" from otdf_sdk_mgr.registry import ( + RegistryUnreachableError, apply_filters, list_go_versions, list_java_github_releases, @@ -45,22 +46,30 @@ def list_versions( sdks = ["go", "js", "java", "platform"] if sdk == "all" else [sdk] all_entries: list[dict[str, Any]] = [] - for s in sdks: - if s == "go": - entries = list_go_versions() - all_entries.extend(apply_filters(entries, stable_only=stable, latest_n=latest)) - elif s == "js": - entries = list_js_versions() - all_entries.extend(apply_filters(entries, stable_only=stable, latest_n=latest)) - elif s == "java": - maven_entries = list_java_maven_versions() - all_entries.extend(apply_filters(maven_entries, stable_only=stable, latest_n=latest)) - if releases: - gh_entries = list_java_github_releases() - all_entries.extend(apply_filters(gh_entries, stable_only=stable, latest_n=latest)) - elif s == "platform": - entries = list_platform_versions() - all_entries.extend(apply_filters(entries, stable_only=stable, latest_n=latest)) + try: + for s in sdks: + if s == "go": + entries = list_go_versions() + all_entries.extend(apply_filters(entries, stable_only=stable, latest_n=latest)) + elif s == "js": + entries = list_js_versions() + all_entries.extend(apply_filters(entries, stable_only=stable, latest_n=latest)) + elif s == "java": + maven_entries = list_java_maven_versions() + all_entries.extend( + apply_filters(maven_entries, stable_only=stable, latest_n=latest) + ) + if releases: + gh_entries = list_java_github_releases() + all_entries.extend( + apply_filters(gh_entries, stable_only=stable, latest_n=latest) + ) + elif s == "platform": + entries = list_platform_versions() + all_entries.extend(apply_filters(entries, stable_only=stable, latest_n=latest)) + except RegistryUnreachableError as e: + typer.echo(f"Error: could not reach version registry — check network: {e}", err=True) + raise typer.Exit(1) if output_table: _print_rich_table(all_entries) diff --git a/otdf-sdk-mgr/src/otdf_sdk_mgr/installers.py b/otdf-sdk-mgr/src/otdf_sdk_mgr/installers.py index 9d0e42ba0..f04619528 100644 --- a/otdf-sdk-mgr/src/otdf_sdk_mgr/installers.py +++ b/otdf-sdk-mgr/src/otdf_sdk_mgr/installers.py @@ -33,18 +33,18 @@ def install_go_release(version: str, dist_dir: Path) -> None: `go run @{version}` instead of a local binary. The .version file contains `module-path@version` (e.g., `github.com/opentdf/platform/otdfctl@v0.31.0`). Pre-v0.31.0 tags use the archived `github.com/opentdf/otdfctl` module path. + + `go install` is run first to surface module-resolution failures eagerly; + only after it succeeds do we lay down `.version` + cli wrappers, so a + failed install never leaves a directory that looks successful to the + `dist_dir.exists()` skip check in `install_release`. """ go_dir = get_sdk_dir() / "go" - dist_dir.mkdir(parents=True, exist_ok=True) # Strip tag infix (e.g., "otdfctl/v0.24.0" → "v0.24.0") if "/" in version: version = version.rsplit("/", 1)[-1] tag = normalize_version(version) module = go_module_for_tag(tag) - (dist_dir / ".version").write_text(f"{module}@{tag}\n") - shutil.copy(go_dir / "cli.sh", dist_dir / "cli.sh") - shutil.copy(go_dir / "otdfctl.sh", dist_dir / "otdfctl.sh") - shutil.copy(go_dir / "opentdfctl.yaml", dist_dir / "opentdfctl.yaml") print(f" Pre-warming Go cache for {module}@{tag}...") result = subprocess.run( ["go", "install", f"{module}@{tag}"], @@ -52,12 +52,17 @@ def install_go_release(version: str, dist_dir: Path) -> None: text=True, ) if result.returncode != 0: - msg = f"go install pre-warm failed: {result.stderr.strip()}" + msg = f"go install failed for {module}@{tag}: {result.stderr.strip()}" if module == GO_MODULE_PATH_PLATFORM: raise InstallError( f"{msg}\nThe platform module path {module}@{tag} may not be published yet." ) - print(f" Warning: {msg} (will retry at runtime)") + raise InstallError(msg) + dist_dir.mkdir(parents=True, exist_ok=True) + (dist_dir / ".version").write_text(f"{module}@{tag}\n") + shutil.copy(go_dir / "cli.sh", dist_dir / "cli.sh") + shutil.copy(go_dir / "otdfctl.sh", dist_dir / "otdfctl.sh") + shutil.copy(go_dir / "opentdfctl.yaml", dist_dir / "opentdfctl.yaml") print(f" Go release {tag} installed to {dist_dir}") diff --git a/otdf-sdk-mgr/src/otdf_sdk_mgr/platform_installer.py b/otdf-sdk-mgr/src/otdf_sdk_mgr/platform_installer.py index ef0c06346..473b7db97 100644 --- a/otdf-sdk-mgr/src/otdf_sdk_mgr/platform_installer.py +++ b/otdf-sdk-mgr/src/otdf_sdk_mgr/platform_installer.py @@ -89,25 +89,57 @@ def _ensure_bare_repo() -> Path: return bare +def _is_hex(s: str) -> bool: + return bool(s) and all(c in "0123456789abcdef" for c in s.lower()) + + def _resolve_platform_ref(version_or_ref: str) -> str: """Turn a user-supplied version into the actual git ref to checkout. `v0.9.0` → `service/v0.9.0` (matches SDK_TAG_INFIXES["platform"]). `pr:42` → `refs/pull/42/head` (expanded first, then passes through the `/` check). - A ref that already contains `/`, a hex SHA, or `main` is returned as-is. + A ref that already contains `/`, a full-length hex SHA, or `main` is + returned as-is. 7-39 char hex inputs are returned unchanged — they + might be abbreviated SHAs or branch names; `install_platform_source` + resolves the ambiguity via `_expand_short_sha` once the bare repo is + available. """ version_or_ref = expand_pr_shorthand(version_or_ref) infix = SDK_TAG_INFIXES.get("platform", "service") if "/" in version_or_ref or version_or_ref in ("main", "HEAD"): return version_or_ref - if len(version_or_ref) in (40, 64) and all( - c in "0123456789abcdef" for c in version_or_ref.lower() - ): + if len(version_or_ref) in (40, 64) and _is_hex(version_or_ref): + return version_or_ref + if 7 <= len(version_or_ref) <= 39 and _is_hex(version_or_ref): return version_or_ref return f"{infix}/{normalize_version(version_or_ref)}" +def _expand_short_sha(short: str) -> str: + """Expand an abbreviated hex string to a full SHA via the bare repo. + + Returns the full 40-char SHA if git resolves the prefix uniquely. + Raises `PlatformInstallError` if git reports the prefix is ambiguous. + Returns the input unchanged if git cannot resolve it (caller may then + treat it as a branch/tag name; `git worktree add` will produce a clear + `invalid reference` error if the name doesn't exist either). + """ + bare = _ensure_bare_repo() + result = subprocess.run( + ["git", f"--git-dir={bare}", "rev-parse", "--verify", f"{short}^{{commit}}"], + capture_output=True, + text=True, + ) + if result.returncode == 0: + return result.stdout.strip() + if "ambiguous" in result.stderr.lower(): + raise PlatformInstallError( + f"ambiguous abbreviated SHA {short!r}: pass at least 8 chars, or the full 40-char SHA" + ) + return short + + def _worktree_path_for(ref: str) -> Path: return _platform_src_root() / ref_slug(ref) @@ -187,12 +219,17 @@ def install_platform_source(ref: str, dist_name: str | None = None) -> Path: reset, the old dist is removed, and the service binary is rebuilt. """ full_ref = _resolve_platform_ref(ref) + if 7 <= len(full_ref) <= 39 and _is_hex(full_ref): + # Abbreviated SHA: resolve to full 40-char SHA so caching matches the + # full-SHA case. Falls through to branch-name handling if git can't + # resolve the prefix; raises on ambiguity. + full_ref = _expand_short_sha(full_ref) if dist_name is None: if is_mutable_ref(full_ref): dist_name = ref_slug(full_ref) else: - # For immutable refs (tags, SHAs), normalize only the semver tail so - # namespaced tags like `service/v0.9.0` produce the same dist_name as `v0.9.0`. + # Normalize only the semver tail so namespaced tags like + # `service/v0.9.0` produce the same dist_name as `v0.9.0`. dist_name = normalize_version(full_ref.rsplit("/", 1)[-1]) dist_dir = _platform_dist_root() / dist_name binary = dist_dir / "service" diff --git a/otdf-sdk-mgr/src/otdf_sdk_mgr/registry.py b/otdf-sdk-mgr/src/otdf_sdk_mgr/registry.py index 58107d204..5f278522b 100644 --- a/otdf-sdk-mgr/src/otdf_sdk_mgr/registry.py +++ b/otdf-sdk-mgr/src/otdf_sdk_mgr/registry.py @@ -22,6 +22,16 @@ from otdf_sdk_mgr.semver import is_stable, parse_semver, semver_sort_key +class RegistryUnreachableError(Exception): + """A version registry could not be contacted. + + Distinct from "registry reachable but returned no versions" — callers + that care (e.g. `cmd_stable`) should surface this differently so users + don't chase nonexistent version-availability bugs when the real problem + is a network outage. + """ + + def _github_headers() -> dict[str, str]: """Return headers for GitHub API requests, including auth if GITHUB_TOKEN is set.""" headers: dict[str, str] = {"Accept": "application/json"} @@ -106,8 +116,7 @@ def list_js_versions() -> list[dict[str, Any]]: try: data = fetch_json(url) except urllib.error.URLError as e: - print(f"Warning: failed to fetch npm registry: {e}", file=sys.stderr) - return [] + raise RegistryUnreachableError(f"failed to fetch npm registry ({url}): {e}") from e dist_tags: dict[str, str] = data.get("dist-tags", {}) tag_lookup: dict[str, list[str]] = {} @@ -140,8 +149,7 @@ def list_java_maven_versions() -> list[dict[str, Any]]: try: xml_text = fetch_text(url) except urllib.error.URLError as e: - print(f"Warning: failed to fetch Maven metadata: {e}", file=sys.stderr) - return [] + raise RegistryUnreachableError(f"failed to fetch Maven metadata ({url}): {e}") from e versions = re.findall(r"([^<]+)", xml_text) results = [] @@ -171,8 +179,7 @@ def list_java_github_releases() -> list[dict[str, Any]]: try: releases = fetch_json(url) except urllib.error.URLError as e: - print(f"Warning: failed to fetch GitHub releases: {e}", file=sys.stderr) - break + raise RegistryUnreachableError(f"failed to fetch GitHub releases ({url}): {e}") from e if not releases: break for release in releases: diff --git a/otdf-sdk-mgr/tests/test_platform_installer.py b/otdf-sdk-mgr/tests/test_platform_installer.py index 7b23cfce2..37497118f 100644 --- a/otdf-sdk-mgr/tests/test_platform_installer.py +++ b/otdf-sdk-mgr/tests/test_platform_installer.py @@ -17,8 +17,10 @@ ("service/v0.9.0", "service/v0.9.0"), ("a" * 40, "a" * 40), ("b" * 64, "b" * 64), - ("abc1234", "service/vabc1234"), - ("deadbeef", "service/vdeadbeef"), + # 7-39 char hex passes through unchanged; install_platform_source + # expands via `git rev-parse` once the bare repo is available. + ("abc1234", "abc1234"), + ("deadbeef", "deadbeef"), # PR shorthand expands before the `/` check, then passes through. ("pr:42", "refs/pull/42/head"), ("pr:1234", "refs/pull/1234/head"), diff --git a/xtest/AGENTS.md b/xtest/AGENTS.md index a9bb394ed..7b1e7b899 100644 --- a/xtest/AGENTS.md +++ b/xtest/AGENTS.md @@ -15,7 +15,6 @@ fixture system. | `conftest.py` | `pytest_addoption` + the encrypt/decrypt SDK parametrization. Defines `--sdks`, `--sdks-encrypt`, `--sdks-decrypt`, `--containers`, `--no-audit-logs`. | | `fixtures/` | Module-scoped pytest fixtures: `attributes.py`, `keys.py`, `audit.py`, `assertions.py`, `kas.py`, `encryption.py`, `obligations.py`. | | `tdfs.py` | SDK abstraction layer — wraps the `cli.sh` shims under `sdk//dist//`. | -<<<<<<< HEAD | `sdk/{go,java,js}/dist//` | SDK CLI builds. Installed by `otdf-sdk-mgr install` (see `../otdf-sdk-mgr/AGENTS.md`). | | `test.env` | Default endpoint and client-credential env vars. Source with `set -a && source test.env && set +a`. | From 34568fa6fb3a3b61d1432add37b5a8e7b18a8985 Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Thu, 28 May 2026 08:41:23 -0400 Subject: [PATCH 11/18] fix(otdf-sdk-mgr): tighten error handling and dedupe APIs - Widen `install scenario` exception net to subprocess.CalledProcessError, ValidationError, and OSError so a failed build, bad YAML, or missing helper script still produces a typer.Exit(1) plus a partial manifest instead of an unhandled traceback - Delete duplicate `list_platform_versions` from platform_installer.py (registry.py has the canonical version returning dict entries) - Preserve KasPin.mode and KasPin.features in the installed.json manifest so downstream tooling can read them back without re-parsing YAML - Add `.complete` marker to platform builds; reuse requires both the binary and the marker, surviving Ctrl-C mid-build - checkout._run now captures stderr and includes cwd in the raised CalledProcessError; platform_installer._run wraps FileNotFoundError with the executable name - Move scenario subcommand registration out of `_register_scenario_cmd` side-effect wrapper Co-Authored-By: Claude Opus 4.7 --- otdf-sdk-mgr/src/otdf_sdk_mgr/checkout.py | 16 ++++-- otdf-sdk-mgr/src/otdf_sdk_mgr/cli_install.py | 12 +---- otdf-sdk-mgr/src/otdf_sdk_mgr/cli_scenario.py | 46 ++++++++++------ .../src/otdf_sdk_mgr/platform_installer.py | 53 ++++++++----------- 4 files changed, 69 insertions(+), 58 deletions(-) diff --git a/otdf-sdk-mgr/src/otdf_sdk_mgr/checkout.py b/otdf-sdk-mgr/src/otdf_sdk_mgr/checkout.py index 29b9cfca9..9310d8ec9 100644 --- a/otdf-sdk-mgr/src/otdf_sdk_mgr/checkout.py +++ b/otdf-sdk-mgr/src/otdf_sdk_mgr/checkout.py @@ -12,10 +12,20 @@ def _run(cmd: list[str], **kwargs: Any) -> None: - """Run a command, raising on failure.""" - result = subprocess.run(cmd, **kwargs) + """Run a command, raising on failure with cwd and stderr in the message.""" + try: + result = subprocess.run(cmd, capture_output=True, text=True, **kwargs) + except FileNotFoundError as e: + raise RuntimeError(f"executable not found: {cmd[0]} ({e})") from e if result.returncode != 0: - raise subprocess.CalledProcessError(result.returncode, cmd) + cwd = kwargs.get("cwd", "") + cwd_str = f" (cwd={cwd})" if cwd else "" + raise subprocess.CalledProcessError( + result.returncode, + cmd, + output=result.stdout, + stderr=(result.stderr or "") + cwd_str, + ) def checkout_sdk_branch(language: str, branch: str) -> str: diff --git a/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_install.py b/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_install.py index ce2229b9d..9fa866642 100644 --- a/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_install.py +++ b/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_install.py @@ -6,22 +6,14 @@ import typer +from otdf_sdk_mgr.cli_scenario import install_scenario_cmd from otdf_sdk_mgr.config import ALL_SDKS install_app = typer.Typer(help="Install SDK CLI artifacts from registries or source.") - - -def _register_scenario_cmd() -> None: - from otdf_sdk_mgr.cli_scenario import install_scenario_cmd - - install_app.command("scenario")(install_scenario_cmd) - - -_register_scenario_cmd() +install_app.command("scenario")(install_scenario_cmd) def _split_platform(sdks: list[str]) -> tuple[bool, list[str]]: - """Return (platform_requested, sdks_without_platform).""" return ("platform" in sdks, [s for s in sdks if s != "platform"]) diff --git a/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_scenario.py b/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_scenario.py index 9fa0cdf2e..0884e69a3 100644 --- a/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_scenario.py +++ b/otdf-sdk-mgr/src/otdf_sdk_mgr/cli_scenario.py @@ -10,10 +10,12 @@ from __future__ import annotations import json +import subprocess from pathlib import Path from typing import Annotated import typer +from pydantic import ValidationError from otdf_sdk_mgr.installers import InstallError, install_release from otdf_sdk_mgr.platform_installer import ( @@ -31,13 +33,18 @@ ) -def _install_platform_pin(pin: PlatformPin | KasPin) -> dict[str, str]: +def _install_platform_pin(pin: PlatformPin | KasPin) -> dict[str, object]: if pin.dist is not None: dist_dir = install_platform_release(pin.dist) - return {"kind": "dist", "version": pin.dist, "path": str(dist_dir)} - assert pin.source is not None # by schema invariant - dist_dir = install_platform_source(pin.source.ref) - return {"kind": "source", "ref": pin.source.ref, "path": str(dist_dir)} + record: dict[str, object] = {"kind": "dist", "version": pin.dist, "path": str(dist_dir)} + else: + assert pin.source is not None # by schema invariant + dist_dir = install_platform_source(pin.source.ref) + record = {"kind": "source", "ref": pin.source.ref, "path": str(dist_dir)} + if isinstance(pin, KasPin): + record["mode"] = pin.mode + record["features"] = dict(pin.features) + return record def install_scenario_cmd( @@ -62,17 +69,21 @@ def install_scenario_cmd( kind = raw.get("kind") if isinstance(raw.get("kind"), str) else None scenario: Scenario | None = None - if kind == "Scenario": - scenario = Scenario.model_validate(raw) - instance = scenario.instance - elif kind == "Instance": - instance = Instance.model_validate(raw) - else: - typer.echo(f"Error: {path} has unknown kind {kind!r}", err=True) + try: + if kind == "Scenario": + scenario = Scenario.model_validate(raw) + instance = scenario.instance + elif kind == "Instance": + instance = Instance.model_validate(raw) + else: + typer.echo(f"Error: {path} has unknown kind {kind!r}", err=True) + raise typer.Exit(1) + except ValidationError as e: + typer.echo(f"Error: {path} failed schema validation:\n{e}", err=True) raise typer.Exit(1) - installed_platform: dict[str, str] | None = None - installed_kas: dict[str, dict[str, str]] = {} + installed_platform: dict[str, object] | None = None + installed_kas: dict[str, dict[str, object]] = {} installed_sdks: dict[str, list[dict[str, str | None]]] = {"encrypt": [], "decrypt": []} out = path.parent / f"{path.stem}.installed.json" @@ -109,7 +120,12 @@ def _snapshot(status: str | None = None) -> dict[str, object]: } for entry in getattr(scenario.sdks, role) ] - except (PlatformInstallError, InstallError) as e: + except ( + PlatformInstallError, + InstallError, + subprocess.CalledProcessError, + OSError, + ) as e: out.write_text(json.dumps(_snapshot(status="partial"), indent=2) + "\n") typer.echo(f"Error: {e}", err=True) typer.echo(f" Wrote partial manifest to {out}", err=True) diff --git a/otdf-sdk-mgr/src/otdf_sdk_mgr/platform_installer.py b/otdf-sdk-mgr/src/otdf_sdk_mgr/platform_installer.py index 473b7db97..4a01bb4fb 100644 --- a/otdf-sdk-mgr/src/otdf_sdk_mgr/platform_installer.py +++ b/otdf-sdk-mgr/src/otdf_sdk_mgr/platform_installer.py @@ -1,9 +1,8 @@ """Installer for the OpenTDF platform service. -Mirrors the SDK installer pattern but produces a built `service` binary at -`xtest/platform/dist//service`. v1 supports source builds only — -container images and release tarballs are not published by `opentdf/platform` -today. +Produces a built `service` binary at `xtest/platform/dist//service` +from source — container images and release tarballs are not published by +`opentdf/platform` today. Tag namespacing: the platform monorepo tags releases as `service/vX.Y.Z`. Users pass plain versions (e.g. `v0.9.0`); the installer prefixes `service/` @@ -70,7 +69,10 @@ def _run(cmd: list[str], cwd: Path | None = None) -> None: user can see progress. We don't capture; on failure the user has already seen the diagnostics in their terminal. """ - result = subprocess.run(cmd, cwd=cwd) + try: + result = subprocess.run(cmd, cwd=cwd) + except FileNotFoundError as e: + raise PlatformInstallError(f"executable not found: {cmd[0]} ({e})") from e if result.returncode != 0: raise PlatformInstallError(f"command failed (exit {result.returncode}): {' '.join(cmd)}") @@ -177,16 +179,26 @@ def _ensure_worktree(ref: str) -> Path: def _build_service(worktree: Path, dist_dir: Path) -> Path: - """Run `go build` to produce `dist_dir/service`.""" + """Run `go build` to produce `dist_dir/service`. + + Writes a `.complete` marker after the build succeeds. Reuse requires both + the binary and the marker — survives Ctrl-C mid-build, which would + otherwise leave a half-written binary that the next invocation would + happily serve. + """ dist_dir.mkdir(parents=True, exist_ok=True) binary = dist_dir / "service" - if binary.exists(): + marker = dist_dir / ".complete" + if binary.exists() and marker.exists(): print(f" Binary already built at {binary}; reusing.") return binary + if binary.exists(): + binary.unlink() print(f" Building platform service binary at {binary} from {worktree}...") _run(["go", "build", "-o", str(binary), "./service"], cwd=worktree) if not binary.exists(): raise PlatformInstallError(f"go build completed but {binary} is missing") + marker.write_text("") return binary @@ -233,13 +245,15 @@ def install_platform_source(ref: str, dist_name: str | None = None) -> Path: dist_name = normalize_version(full_ref.rsplit("/", 1)[-1]) dist_dir = _platform_dist_root() / dist_name binary = dist_dir / "service" - if binary.exists() and not is_mutable_ref(full_ref): + marker = dist_dir / ".complete" + if binary.exists() and marker.exists() and not is_mutable_ref(full_ref): print(f" Dist already present at {dist_dir}; skipping build.") return dist_dir worktree = _ensure_worktree(full_ref) if binary.exists(): - # Mutable ref: drop the stale binary so `_build_service` actually rebuilds. binary.unlink() + if marker.exists(): + marker.unlink() _build_service(worktree, dist_dir) _record_version(dist_dir, full_ref, worktree) print(f" Platform {ref} → {dist_dir}") @@ -283,24 +297,3 @@ def install_helper_scripts(branch: str = HELPER_SCRIPTS_BRANCH) -> Path: shutil.copytree(src_scripts, scripts_dir) print(f" Helper scripts copied to {scripts_dir}") return scripts_dir - - -def list_platform_versions() -> list[str]: - """Return all `service/vX.Y.Z` tags from the platform repo, version-only.""" - from git import Git - - repo = Git() - raw = repo.ls_remote(SDK_GIT_URLS["platform"], tags=True) - infix = SDK_TAG_INFIXES.get("platform", "service") - out: list[str] = [] - for line in raw.strip().splitlines(): - if not line: - continue - _, ref = line.split("\t", 1) - if ref.endswith("^{}"): - continue - tag = ref.removeprefix("refs/tags/") - if tag.startswith(f"{infix}/"): - out.append(tag.removeprefix(f"{infix}/")) - out.sort() - return out From 9df465a38975c3add317f9ff9f96808f9b0c840e Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Thu, 28 May 2026 08:45:04 -0400 Subject: [PATCH 12/18] test(otdf-sdk-mgr): cover mutable-rebuild, PR refspec, registry Tests: - Rewrite `test_dist_name_derivation` to call `install_platform_source` instead of re-implementing its dist-name logic in the test body - Regression tests for mutable-vs-immutable rebuild, .complete marker semantics, PR ref fetch via explicit refspec, and short-SHA expansion - New `test_registry.py` covering RegistryUnreachableError propagation, _github_headers with/without GITHUB_TOKEN, ls_remote tag parsing, and GitHub rate-limit warning - Assert KasPin.mode and KasPin.features round-trip into installed.json Polish: - `install_java_release` switches the BaseException catch to try/finally so KeyboardInterrupt/SystemExit retain their normal semantics - README documents the dist-naming convention as a table Co-Authored-By: Claude Opus 4.7 --- otdf-sdk-mgr/README.md | 12 +- otdf-sdk-mgr/src/otdf_sdk_mgr/installers.py | 7 +- otdf-sdk-mgr/tests/test_cli_scenario.py | 46 +++++ otdf-sdk-mgr/tests/test_platform_installer.py | 172 ++++++++++++++++-- otdf-sdk-mgr/tests/test_registry.py | 108 +++++++++++ 5 files changed, 318 insertions(+), 27 deletions(-) create mode 100644 otdf-sdk-mgr/tests/test_registry.py diff --git a/otdf-sdk-mgr/README.md b/otdf-sdk-mgr/README.md index 3ad5fd30c..c99e1dfb2 100644 --- a/otdf-sdk-mgr/README.md +++ b/otdf-sdk-mgr/README.md @@ -76,10 +76,14 @@ Source builds (`tip` mode) check out source to `sdk/{lang}/src/` and compile via `refs/pull/N/head`), and raw `refs/...` paths. Mutable refs (branches and PR heads) are re-fetched and rebuilt on every invocation so the dist matches the latest upstream commit; immutable refs (tags and full-length -SHAs) reuse the cached build for speed. Dist directories are named after a -slug of the ref (e.g. `dist/my-branch/`, `dist/pull--42--head/`); `--ref -main` keeps the historical `dist/tip/` (platform) and `dist/main/` (SDK) -names. +SHAs) reuse the cached build for speed. + +| Ref form | Mutable? | Dist directory name | +|--------------------------------|----------|--------------------------| +| `--ref main` (default) | yes | `dist/tip/` (platform), `dist/main/` (SDK) | +| Other branches, `pr:N`, raw `refs/...` | yes | slug of the ref, e.g. `dist/my-branch/`, `dist/refs--pull--42--head/` | +| `v0.9.0`, `service/v0.9.0` | no | `dist/v0.9.0/` | +| Full 40-char SHA | no | `dist/v/` | After changes to SDK source, rebuild: diff --git a/otdf-sdk-mgr/src/otdf_sdk_mgr/installers.py b/otdf-sdk-mgr/src/otdf_sdk_mgr/installers.py index f04619528..d20fd3f3d 100644 --- a/otdf-sdk-mgr/src/otdf_sdk_mgr/installers.py +++ b/otdf-sdk-mgr/src/otdf_sdk_mgr/installers.py @@ -109,7 +109,6 @@ def install_java_release(version: str, dist_dir: Path) -> None: print(f" Warning: Could not verify artifact availability: {e}", file=sys.stderr) # Proceed with download attempt anyway - # Download to a temp file first to avoid partial writes tmp_path: Path | None = None try: with tempfile.NamedTemporaryFile(delete=False, suffix=".jar") as tmp: @@ -127,17 +126,15 @@ def install_java_release(version: str, dist_dir: Path) -> None: ) raise - # Download succeeded — now create dist_dir and move files into place dist_dir.mkdir(parents=True, exist_ok=True) shutil.copy(java_dir / "cli.sh", dist_dir / "cli.sh") shutil.move(str(tmp_path), str(dist_dir / "cmdline.jar")) - tmp_path = None # Ownership transferred; don't clean up - except BaseException: + tmp_path = None + finally: if tmp_path is not None: tmp_path.unlink(missing_ok=True) if dist_dir.exists() and not (dist_dir / "cmdline.jar").exists(): shutil.rmtree(dist_dir, ignore_errors=True) - raise print(f" Java release {tag} installed to {dist_dir}") diff --git a/otdf-sdk-mgr/tests/test_cli_scenario.py b/otdf-sdk-mgr/tests/test_cli_scenario.py index 4f007b18f..1b7be4221 100644 --- a/otdf-sdk-mgr/tests/test_cli_scenario.py +++ b/otdf-sdk-mgr/tests/test_cli_scenario.py @@ -65,6 +65,8 @@ def fake_install_release(sdk: str, version: str) -> Path: "path": str(platform_dist), } assert set(record["kas"].keys()) == {"alpha"} + assert record["kas"]["alpha"]["mode"] == "standard" + assert record["kas"]["alpha"]["features"] == {} assert record["sdks"]["encrypt"] == [ { "sdk": "go", @@ -104,6 +106,50 @@ def fake_install_release(sdk: str, version: str) -> Path: } +def test_install_scenario_preserves_kas_mode_and_features(tmp_path: Path) -> None: + """KasPin.mode and KasPin.features round-trip into installed.json.""" + scenario_yaml = """ +apiVersion: opentdf.io/v1alpha1 +kind: Scenario +metadata: + id: km-smoke +instance: + platform: { dist: v0.9.0 } + kas: + alpha: + dist: v0.9.0 + mode: key_management + features: + ec_tdf_enabled: true + rotation_enabled: false +sdks: + encrypt: + - { sdk: go, version: v0.24.0 } + decrypt: + - { sdk: go, version: v0.24.0 } +suite: + targets: ["xtest/test_tdfs.py"] +""" + scenario_path = tmp_path / "s.yaml" + scenario_path.write_text(scenario_yaml) + platform_dist = tmp_path / "platform-dist" / "v0.9.0" + + with ( + patch("otdf_sdk_mgr.cli_scenario.install_platform_release", return_value=platform_dist), + patch("otdf_sdk_mgr.cli_scenario.install_helper_scripts"), + patch( + "otdf_sdk_mgr.cli_scenario.install_release", + side_effect=lambda sdk, version: tmp_path / "sdk" / sdk / version, + ), + ): + install_scenario_cmd(scenario_path, skip_scripts=True) + + record = json.loads((tmp_path / "s.installed.json").read_text()) + alpha = record["kas"]["alpha"] + assert alpha["mode"] == "key_management" + assert alpha["features"] == {"ec_tdf_enabled": True, "rotation_enabled": False} + + def test_install_scenario_writes_partial_manifest_on_failure(tmp_path: Path) -> None: from otdf_sdk_mgr.installers import InstallError diff --git a/otdf-sdk-mgr/tests/test_platform_installer.py b/otdf-sdk-mgr/tests/test_platform_installer.py index 37497118f..cbdd866f3 100644 --- a/otdf-sdk-mgr/tests/test_platform_installer.py +++ b/otdf-sdk-mgr/tests/test_platform_installer.py @@ -1,10 +1,14 @@ -"""Pure-function tests for platform_installer.""" +"""Tests for platform_installer.""" + +from __future__ import annotations + +from pathlib import Path +from typing import Any import pytest -from otdf_sdk_mgr.platform_installer import _resolve_platform_ref -from otdf_sdk_mgr.refs import is_mutable_ref, ref_slug -from otdf_sdk_mgr.semver import normalize_version +from otdf_sdk_mgr import platform_installer +from otdf_sdk_mgr.platform_installer import _resolve_platform_ref, install_platform_source @pytest.mark.parametrize( @@ -32,30 +36,162 @@ def test_resolve_platform_ref(inp, expected): assert _resolve_platform_ref(inp) == expected +def _stub_installer(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> dict[str, Any]: + """Replace every subprocess-touching helper with in-memory recorders. + + Returns a `calls` dict that test cases inspect. + """ + calls: dict[str, Any] = { + "run": [], + "ensure_worktree": [], + "build_service": [], + "record_version": 0, + "expand_short_sha": [], + } + dist_root = tmp_path / "dist" + + def _record(name: str): + def fn(*args, **kwargs): + calls[name].append((args, kwargs)) + + return fn + + monkeypatch.setattr(platform_installer, "_platform_dist_root", lambda: dist_root) + monkeypatch.setattr(platform_installer, "_run", _record("run")) + + def fake_ensure_worktree(ref: str) -> Path: + calls["ensure_worktree"].append(ref) + p = tmp_path / "src" / ref.replace("/", "--") + p.mkdir(parents=True, exist_ok=True) + return p + + def fake_build_service(worktree: Path, dist_dir: Path) -> Path: + calls["build_service"].append((str(worktree), str(dist_dir))) + dist_dir.mkdir(parents=True, exist_ok=True) + binary = dist_dir / "service" + binary.write_text("fake binary") + (dist_dir / ".complete").write_text("") + return binary + + def fake_record_version(dist_dir: Path, ref: str, worktree: Path) -> None: + calls["record_version"] += 1 + + def fake_expand_short_sha(short: str) -> str: + calls["expand_short_sha"].append(short) + return "a" * 40 + + monkeypatch.setattr(platform_installer, "_ensure_worktree", fake_ensure_worktree) + monkeypatch.setattr(platform_installer, "_build_service", fake_build_service) + monkeypatch.setattr(platform_installer, "_record_version", fake_record_version) + monkeypatch.setattr(platform_installer, "_expand_short_sha", fake_expand_short_sha) + return calls + + @pytest.mark.parametrize( "ref,expected_dist_name", [ - # Immutable refs: namespaced tags and plain tags should produce the same dist_name + # Immutable refs: namespaced tags and plain tags share a dist_name. ("service/v0.9.0", "v0.9.0"), ("v0.9.0", "v0.9.0"), ("0.9.0", "v0.9.0"), # Mutable refs: use ref_slug ("main", "main"), ("refs/pull/42/head", "refs--pull--42--head"), - # SHAs: immutable, normalize the tail (which is the full SHA) - ("a" * 40, "v" + "a" * 40), ], ) -def test_dist_name_derivation(ref, expected_dist_name): - """Verify that dist_name is derived consistently for immutable refs. +def test_dist_name_derivation( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path, ref: str, expected_dist_name: str +): + """install_platform_source picks dist_name consistently for each ref form.""" + _stub_installer(monkeypatch, tmp_path) + dist_dir = install_platform_source(ref) + assert dist_dir.name == expected_dist_name + + +def test_immutable_ref_with_complete_marker_skips_rebuild( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +): + """An existing immutable build (binary + .complete) is reused, not rebuilt.""" + calls = _stub_installer(monkeypatch, tmp_path) + dist_dir = tmp_path / "dist" / "v0.9.0" + dist_dir.mkdir(parents=True) + (dist_dir / "service").write_text("prebuilt") + (dist_dir / ".complete").write_text("") + + install_platform_source("v0.9.0") + + assert calls["build_service"] == [] + assert calls["ensure_worktree"] == [] - Namespaced tags like `service/v0.9.0` should produce the same dist_name as - plain tags `v0.9.0` or `0.9.0`, ensuring immutable refs reuse existing dist dirs. + +def test_immutable_ref_without_complete_marker_rebuilds( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +): + """A partial dist (binary without .complete) is treated as corrupt and rebuilt.""" + calls = _stub_installer(monkeypatch, tmp_path) + dist_dir = tmp_path / "dist" / "v0.9.0" + dist_dir.mkdir(parents=True) + (dist_dir / "service").write_text("half-built") + + install_platform_source("v0.9.0") + + assert len(calls["build_service"]) == 1 + + +def test_mutable_ref_drops_existing_binary_and_rebuilds( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +): + """A mutable ref (branch) re-fetches and rebuilds even if a binary exists.""" + calls = _stub_installer(monkeypatch, tmp_path) + dist_dir = tmp_path / "dist" / "main" + dist_dir.mkdir(parents=True) + stale_binary = dist_dir / "service" + stale_binary.write_text("stale") + (dist_dir / ".complete").write_text("") + + install_platform_source("main") + + assert calls["ensure_worktree"] == ["main"] + assert len(calls["build_service"]) == 1 + + +def test_pr_ref_expands_and_fetches_refspec(monkeypatch: pytest.MonkeyPatch, tmp_path: Path): + """install_platform_source("pr:123") delegates to _ensure_worktree with the + expanded ref. The real `_ensure_worktree` would then run `git fetch origin + +refs/pull/123/head:refs/pull/123/head` — exercised by the explicit test + below that doesn't stub `_ensure_worktree`.""" + calls = _stub_installer(monkeypatch, tmp_path) + install_platform_source("pr:123") + assert calls["ensure_worktree"] == ["refs/pull/123/head"] + + +def test_ensure_worktree_fetches_pr_refspec(monkeypatch: pytest.MonkeyPatch, tmp_path: Path): + """_ensure_worktree explicitly fetches `refs/...` refs before `worktree add`. + + The bare clone's default refspec is `+refs/heads/*:refs/heads/*` plus + `--tags`, so PR refs are never pulled and `git worktree add + refs/pull/N/head` fails without the explicit fetch. """ - full_ref = _resolve_platform_ref(ref) - if is_mutable_ref(full_ref): - dist_name = ref_slug(full_ref) - else: - # Normalize only the semver tail for immutable refs - dist_name = normalize_version(full_ref.rsplit("/", 1)[-1]) - assert dist_name == expected_dist_name + run_calls: list[list[str]] = [] + + def fake_run(cmd, cwd=None): + run_calls.append(list(cmd)) + + bare = tmp_path / "platform.git" + bare.mkdir() + monkeypatch.setattr(platform_installer, "_run", fake_run) + monkeypatch.setattr(platform_installer, "_ensure_bare_repo", lambda: bare) + monkeypatch.setattr(platform_installer, "_platform_src_root", lambda: tmp_path / "src") + + platform_installer._ensure_worktree("refs/pull/123/head") + + fetches = [c for c in run_calls if len(c) >= 4 and c[2] == "fetch"] + assert any("+refs/pull/123/head:refs/pull/123/head" in c for c in fetches), run_calls + + +def test_short_sha_expansion_at_install_time(monkeypatch: pytest.MonkeyPatch, tmp_path: Path): + """Abbreviated hex refs are expanded via `_expand_short_sha` before caching.""" + calls = _stub_installer(monkeypatch, tmp_path) + dist_dir = install_platform_source("abc1234") + assert calls["expand_short_sha"] == ["abc1234"] + assert dist_dir.name == "v" + "a" * 40 diff --git a/otdf-sdk-mgr/tests/test_registry.py b/otdf-sdk-mgr/tests/test_registry.py new file mode 100644 index 000000000..3b03536c5 --- /dev/null +++ b/otdf-sdk-mgr/tests/test_registry.py @@ -0,0 +1,108 @@ +"""Tests for the registry module.""" + +from __future__ import annotations + +import urllib.error + +import pytest + +from otdf_sdk_mgr import registry +from otdf_sdk_mgr.registry import ( + RegistryUnreachableError, + _github_headers, + list_java_github_releases, + list_java_maven_versions, + list_js_versions, + list_platform_versions, +) + + +def test_github_headers_without_token(monkeypatch: pytest.MonkeyPatch): + monkeypatch.delenv("GITHUB_TOKEN", raising=False) + h = _github_headers() + assert h == {"Accept": "application/json"} + + +def test_github_headers_with_token(monkeypatch: pytest.MonkeyPatch): + monkeypatch.setenv("GITHUB_TOKEN", "fake-token-123") + h = _github_headers() + assert h["Authorization"] == "Bearer fake-token-123" + assert h["Accept"] == "application/json" + + +def test_list_js_versions_raises_on_network_error(monkeypatch: pytest.MonkeyPatch): + def boom(_url: str): + raise urllib.error.URLError("nodename nor servname provided") + + monkeypatch.setattr(registry, "fetch_json", boom) + with pytest.raises(RegistryUnreachableError, match="npm registry"): + list_js_versions() + + +def test_list_java_maven_versions_raises_on_network_error(monkeypatch: pytest.MonkeyPatch): + def boom(_url: str): + raise urllib.error.URLError("no route to host") + + monkeypatch.setattr(registry, "fetch_text", boom) + with pytest.raises(RegistryUnreachableError, match="Maven metadata"): + list_java_maven_versions() + + +def test_list_java_github_releases_raises_on_network_error(monkeypatch: pytest.MonkeyPatch): + def boom(_url: str): + raise urllib.error.URLError("connection refused") + + monkeypatch.setattr(registry, "fetch_json", boom) + with pytest.raises(RegistryUnreachableError, match="GitHub releases"): + list_java_github_releases() + + +def test_list_platform_versions_parses_ls_remote(monkeypatch: pytest.MonkeyPatch): + """Verify tag parsing: skip peeled `^{}`, filter to `service/` infix, drop non-semver.""" + raw = "\n".join( + [ + "deadbeef\trefs/tags/service/v0.9.0", + "deadbef0\trefs/tags/service/v0.9.0^{}", + "deadbef1\trefs/tags/service/v0.10.0-rc1", + "deadbef2\trefs/tags/otdfctl/v0.31.0", # different infix — skip + "deadbef3\trefs/tags/service/not-a-version", # not semver — skip + "deadbef4\trefs/heads/main", # not a tag — skip + ] + ) + + class FakeGit: + def ls_remote(self, *_args, **_kwargs): + return raw + + monkeypatch.setattr("git.Git", lambda: FakeGit()) + + results = list_platform_versions() + versions = [r["version"] for r in results] + assert "0.9.0" in versions + assert "0.10.0-rc1" in versions + assert all("not-a-version" not in v for v in versions) + assert all(r["sdk"] == "platform" for r in results) + assert all(r["source"] == "platform-git-tag" for r in results) + + +def test_list_java_github_releases_403_rate_limit(monkeypatch: pytest.MonkeyPatch, capsys): + """403 from GitHub API surfaces a rate-limit warning before re-raising.""" + err = urllib.error.HTTPError( + url="https://api.github.com/test", + code=403, + msg="rate limited", + hdrs={"X-RateLimit-Reset": "1700000000"}, # type: ignore[arg-type] + fp=None, + ) + + def boom(_url: str): + raise err + + monkeypatch.setattr( + registry.urllib.request, "urlopen", lambda *a, **kw: (_ for _ in ()).throw(err) + ) + + with pytest.raises(urllib.error.HTTPError): + registry.fetch_json("https://api.github.com/repos/test/releases") + captured = capsys.readouterr() + assert "rate limit" in captured.err.lower() From 5ea87e76a8e7e202f217f72c0d7d04f4ba32f3c4 Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Mon, 1 Jun 2026 15:59:13 -0400 Subject: [PATCH 13/18] fix(otdf-sdk-mgr): address remaining PR review items - Reject container-image refs in _resolve_platform_ref with a clear PlatformInstallError, instead of letting strings like ghcr.io/opentdf/platform:v0.9.0 fall through to git and fail with a generic "invalid reference" message. - Use "install release platform:" in registry.install_method so copy-paste from `versions list` lands on the actual subcommand signature. - Drop unused boom() helper flagged by Sonar in test_registry.py. Co-Authored-By: Claude Opus 4.7 --- .../src/otdf_sdk_mgr/platform_installer.py | 5 +++++ otdf-sdk-mgr/src/otdf_sdk_mgr/registry.py | 2 +- otdf-sdk-mgr/tests/test_platform_installer.py | 19 ++++++++++++++++++- otdf-sdk-mgr/tests/test_registry.py | 3 --- 4 files changed, 24 insertions(+), 5 deletions(-) diff --git a/otdf-sdk-mgr/src/otdf_sdk_mgr/platform_installer.py b/otdf-sdk-mgr/src/otdf_sdk_mgr/platform_installer.py index 4a01bb4fb..07afea73f 100644 --- a/otdf-sdk-mgr/src/otdf_sdk_mgr/platform_installer.py +++ b/otdf-sdk-mgr/src/otdf_sdk_mgr/platform_installer.py @@ -109,6 +109,11 @@ def _resolve_platform_ref(version_or_ref: str) -> str: """ version_or_ref = expand_pr_shorthand(version_or_ref) infix = SDK_TAG_INFIXES.get("platform", "service") + if "/" in version_or_ref and (":" in version_or_ref or "@" in version_or_ref): + raise PlatformInstallError( + f"container-image refs are not supported: {version_or_ref!r}; " + "use a git ref like v0.9.0, service/v0.9.0, main, or a SHA" + ) if "/" in version_or_ref or version_or_ref in ("main", "HEAD"): return version_or_ref if len(version_or_ref) in (40, 64) and _is_hex(version_or_ref): diff --git a/otdf-sdk-mgr/src/otdf_sdk_mgr/registry.py b/otdf-sdk-mgr/src/otdf_sdk_mgr/registry.py index 5f278522b..652d472ff 100644 --- a/otdf-sdk-mgr/src/otdf_sdk_mgr/registry.py +++ b/otdf-sdk-mgr/src/otdf_sdk_mgr/registry.py @@ -234,7 +234,7 @@ def list_platform_versions() -> list[dict[str, Any]]: "version": version_normalized, "source": "platform-git-tag", "stable": is_stable(version_normalized), - "install_method": f"otdf-sdk-mgr install platform {version_normalized}", + "install_method": f"otdf-sdk-mgr install release platform:{version_normalized}", } ) results.sort(key=lambda r: semver_sort_key(r["version"])) diff --git a/otdf-sdk-mgr/tests/test_platform_installer.py b/otdf-sdk-mgr/tests/test_platform_installer.py index cbdd866f3..3f7e55560 100644 --- a/otdf-sdk-mgr/tests/test_platform_installer.py +++ b/otdf-sdk-mgr/tests/test_platform_installer.py @@ -8,7 +8,11 @@ import pytest from otdf_sdk_mgr import platform_installer -from otdf_sdk_mgr.platform_installer import _resolve_platform_ref, install_platform_source +from otdf_sdk_mgr.platform_installer import ( + PlatformInstallError, + _resolve_platform_ref, + install_platform_source, +) @pytest.mark.parametrize( @@ -36,6 +40,19 @@ def test_resolve_platform_ref(inp, expected): assert _resolve_platform_ref(inp) == expected +@pytest.mark.parametrize( + "inp", + [ + "ghcr.io/opentdf/platform:v0.9.0", + "registry.example.com/opentdf/platform@sha256:" + "a" * 64, + "docker.io/library/foo:latest", + ], +) +def test_resolve_platform_ref_rejects_container_image_refs(inp): + with pytest.raises(PlatformInstallError, match="container-image"): + _resolve_platform_ref(inp) + + def _stub_installer(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> dict[str, Any]: """Replace every subprocess-touching helper with in-memory recorders. diff --git a/otdf-sdk-mgr/tests/test_registry.py b/otdf-sdk-mgr/tests/test_registry.py index 3b03536c5..c90bfaf0f 100644 --- a/otdf-sdk-mgr/tests/test_registry.py +++ b/otdf-sdk-mgr/tests/test_registry.py @@ -95,9 +95,6 @@ def test_list_java_github_releases_403_rate_limit(monkeypatch: pytest.MonkeyPatc fp=None, ) - def boom(_url: str): - raise err - monkeypatch.setattr( registry.urllib.request, "urlopen", lambda *a, **kw: (_ for _ in ()).throw(err) ) From ec578ac68db2fbbf9082a7a53e1b648eea659735 Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Tue, 2 Jun 2026 13:46:12 -0400 Subject: [PATCH 14/18] docs(schema): document KAS preview features fields Cherry-picks the documentation enhancement from b9f84e05 that adds: - Top-level comment explaining KAS Preview Settings precedence - Field descriptions for KasPin.features and Instance.features This documentation clarifies how preview settings are configured and applied, helping users understand the features dict without needing to reference external docs. Cherry-picked from: b9f84e05 (feat(scenario): enable KAS preview features configuration) Co-Authored-By: Claude Sonnet 4.5 --- otdf-sdk-mgr/src/otdf_sdk_mgr/schema.py | 35 +++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/otdf-sdk-mgr/src/otdf_sdk_mgr/schema.py b/otdf-sdk-mgr/src/otdf_sdk_mgr/schema.py index 9255badb6..2c634b0a4 100644 --- a/otdf-sdk-mgr/src/otdf_sdk_mgr/schema.py +++ b/otdf-sdk-mgr/src/otdf_sdk_mgr/schema.py @@ -18,6 +18,22 @@ API_VERSION = "opentdf.io/v1alpha1" +# KAS Preview Settings +# +# The `KasPin.features` dict specifies preview settings written to the generated +# opentdf.yaml as `services.kas.preview.: `. Available preview +# settings vary by platform version; common examples include ec_tdf_enabled, +# hybrid_tdf_enabled, and key_management. +# +# Precedence (last wins): template defaults → mode auto-enables → user features +# +# Example: +# kas: +# km1: +# mode: key_management +# features: +# hybrid_tdf_enabled: true # Enable ML-KEM in addition to auto-enabled features + KasMode = Literal["standard", "key_management"] SdkName = Literal["go", "java", "js"] ContainerKind = Literal["ztdf", "ztdf-ecwrap"] @@ -59,7 +75,16 @@ class KasPin(_StrictModel): dist: str | None = None source: SourceRef | None = None mode: KasMode = "standard" - features: dict[str, bool] = Field(default_factory=dict) + features: dict[str, bool] = Field( + default_factory=dict, + description=( + "KAS preview settings to enable. Keys are preview setting names " + "(without the 'services.kas.preview.' prefix); values are booleans. " + "Available settings depend on the platform version and may include " + "experimental features in PRs. User-specified features override " + "mode-based defaults." + ), + ) @model_validator(mode="after") def _exactly_one(self) -> KasPin: @@ -115,7 +140,13 @@ class Instance(_StrictModel): platform: PlatformPin ports: PortsConfig = Field(default_factory=PortsConfig) kas: dict[str, KasPin] = Field(default_factory=dict) - features: dict[str, bool] = Field(default_factory=dict) + features: dict[str, bool] = Field( + default_factory=dict, + description=( + "Reserved for future use. Instance-level feature defaults are not " + "currently implemented. Use per-KAS features in the kas dict instead." + ), + ) fixtures: Fixtures = Field(default_factory=Fixtures) From a20470edd16d329632226d0cbc3f9623828e056b Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Fri, 15 May 2026 11:51:38 -0400 Subject: [PATCH 15/18] feat(otdf-local): multi-instance test environments (DSPX-3302) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refactors otdf-local from a single-instance CLI (one platform checkout, fixed ports, hardcoded six KAS instances) into a multi-instance harness where each named instance under tests/instances// owns its own opentdf.yaml, keys, KAS configs, and port range. Why --- A single bug report often describes a *combination* — platform v0.9.0 with Java SDK 0.7.8 and a KAS at a pre-release. Today a developer has to hand-edit configs and re-checkout the platform to reproduce. After this change: otdf-local instance init java-078 --from-scenario .../scenario.yaml otdf-local --instance java-078 up brings up exactly the topology the scenario describes, using platform binaries that otdf-sdk-mgr already provisioned (each instance, and each KAS within an instance, can reference a different pinned version). Two instances on disjoint ports.base can coexist on a developer laptop. What changes ------------ otdf-local now depends on otdf-sdk-mgr via a uv path source so both tools share the canonical Scenario/Instance schema. Settings (otdf_local.config.settings): - New instance_name (env-overridable via OTDF_LOCAL_INSTANCE_NAME), instance_dir, instances_root, instance_yaml properties. - platform_dir becomes optional; legacy sibling-discovery only kicks in when no per-instance configuration is present. - platform_binary_for(dist) resolves to the otdf-sdk-mgr-managed xtest/platform/dist//service binary. - keys_dir, logs_dir, config_dir, platform_config, and get_kas_config_path switch to per-instance paths whenever instance.yaml exists; legacy behavior is preserved otherwise. - load_instance() reads the per-instance manifest via the shared Pydantic model. Ports (otdf_local.config.ports): - KAS_OFFSETS exposes the offset table (alpha=+101, beta=+202, ..., km2=+606) so multiple instances on different bases get disjoint port ranges. The legacy 8080-based constants are preserved as defaults. - get_kas_port(name, base=...) computes the port relative to base. Services (otdf_local.services.platform / .kas): - PlatformService.start() and KASService.start() use the pinned dist binary at xtest/platform/dist//service when an instance is loaded, with cwd set to the recorded worktree so the binary finds its embedded resources. Legacy `go run ./service` path runs unchanged when no instance is active. - KASService.is_key_management defers to the manifest's `mode` field instead of the legacy name-based heuristic; per-KAS features (e.g. ec_tdf_enabled) pass through to opentdf.yaml. - KASManager constructs only the KAS instances listed in instance.yaml's kas: map. start_standard / start_km filter on is_key_management so subset topologies still work. utils.keys.setup_golden_keys: - Writes key files into the target directory (per-instance keys_dir or legacy platform_dir) and uses absolute paths in the generated keys_config so the binary finds them regardless of cwd. CLI: - New top-level --instance option threads through every command via OTDF_LOCAL_INSTANCE_NAME. - New `instance` subcommand group: init [--from-scenario PATH], ls --json, rm. - New `scenario` subcommand: `run ` translates the scenario's suite block into `pytest --sdks-encrypt ... --sdks-decrypt ... --containers ...` under xtest/ with OTDF_LOCAL_INSTANCE_NAME set. Tests (otdf-local/tests/test_multi_instance.py): - Port arithmetic at default and alternate bases. - Settings round-trip with and without an instance.yaml. - platform_binary_for resolves under the otdf-sdk-mgr-managed xtest/platform/ tree. .gitignore additions: - tests/instances/ (per-instance config and logs) - xtest/scenarios/*.installed.json (provisioning records) - .claude/tmp/ Backward compatibility: - `otdf-local up` with no --instance flag keeps working against a sibling platform/ checkout. Refs: https://virtru.atlassian.net/browse/DSPX-3302 Co-Authored-By: Claude Opus 4.7 (1M context) --- .gitignore | 7 + otdf-local/pyproject.toml | 4 + otdf-local/src/otdf_local/cli.py | 28 ++- otdf-local/src/otdf_local/cli_instance.py | 183 ++++++++++++++++++ otdf-local/src/otdf_local/cli_scenario.py | 101 ++++++++++ otdf-local/src/otdf_local/config/ports.py | 30 ++- otdf-local/src/otdf_local/config/settings.py | 116 +++++++++-- otdf-local/src/otdf_local/services/kas.py | 116 +++++++---- .../src/otdf_local/services/platform.py | 69 +++++-- otdf-local/src/otdf_local/utils/keys.py | 10 +- otdf-local/tests/test_multi_instance.py | 78 ++++++++ otdf-local/uv.lock | 63 ++++++ 12 files changed, 736 insertions(+), 69 deletions(-) create mode 100644 otdf-local/src/otdf_local/cli_instance.py create mode 100644 otdf-local/src/otdf_local/cli_scenario.py create mode 100644 otdf-local/tests/test_multi_instance.py diff --git a/.gitignore b/.gitignore index a1bb32382..6fa376ceb 100644 --- a/.gitignore +++ b/.gitignore @@ -32,3 +32,10 @@ xtest/sdk/java/cmdline.jar /xtest/otdfctl/ /tmp/ + +# Multi-instance test harness state (DSPX-3302). Per-instance config, logs, and +# keys live under tests/instances/; otdf-sdk-mgr install scenario writes +# .installed.json next to each scenarios.yaml. +/instances/ +xtest/scenarios/*.installed.json +.claude/tmp/ diff --git a/otdf-local/pyproject.toml b/otdf-local/pyproject.toml index b95ac0609..180dc7977 100644 --- a/otdf-local/pyproject.toml +++ b/otdf-local/pyproject.toml @@ -6,12 +6,16 @@ readme = "README.md" requires-python = ">=3.11" dependencies = [ "httpx>=0.27.0", + "otdf-sdk-mgr", "pydantic-settings>=2.2.0", "rich>=13.7.0", "ruamel.yaml>=0.18.0", "typer>=0.12.0", ] +[tool.uv.sources] +otdf-sdk-mgr = { path = "../otdf-sdk-mgr", editable = true } + [dependency-groups] dev = [ "pyright>=1.1.408", diff --git a/otdf-local/src/otdf_local/cli.py b/otdf-local/src/otdf_local/cli.py index d8e3597ff..422daa65a 100644 --- a/otdf-local/src/otdf_local/cli.py +++ b/otdf-local/src/otdf_local/cli.py @@ -1,10 +1,12 @@ """Typer CLI for otdf_local - OpenTDF test environment management.""" import json +import os import shutil import sys import time -from typing import Annotated +from pathlib import Path +from typing import Annotated, Optional import httpx import typer @@ -44,6 +46,18 @@ ) +def _register_subapps() -> None: + """Defer imports so the schema dependency only loads when needed.""" + from otdf_local.cli_instance import instance_app + from otdf_local.cli_scenario import scenario_app + + app.add_typer(instance_app, name="instance") + app.add_typer(scenario_app, name="scenario") + + +_register_subapps() + + def _show_provision_error(result: ProvisionResult, target: str) -> None: """Display provisioning error with stderr details.""" print_error(f"{target} provisioning failed (exit code {result.return_code})") @@ -75,9 +89,19 @@ def main( is_eager=True, ), ] = False, + instance: Annotated[ + Optional[str], + typer.Option( + "--instance", + help='Named instance under tests/instances/. Defaults to "default" (or $OTDF_LOCAL_INSTANCE_NAME).', + ), + ] = None, ) -> None: """OpenTDF test environment management CLI.""" - pass + if instance is not None: + os.environ["OTDF_LOCAL_INSTANCE_NAME"] = instance + # Invalidate the cached Settings so subsequent commands see the new value + get_settings.cache_clear() @app.command() diff --git a/otdf-local/src/otdf_local/cli_instance.py b/otdf-local/src/otdf_local/cli_instance.py new file mode 100644 index 000000000..98407f56e --- /dev/null +++ b/otdf-local/src/otdf_local/cli_instance.py @@ -0,0 +1,183 @@ +"""`otdf-local instance` subcommands: init / ls / rm.""" + +from __future__ import annotations + +import shutil +from pathlib import Path +from typing import Annotated, Optional + +import typer +from otdf_sdk_mgr.schema import Instance, Metadata, PlatformPin, PortsConfig, dump_instance + +from otdf_local.config.settings import get_settings + +instance_app = typer.Typer(help="Manage named test environment instances.") + + +@instance_app.command("init") +def init( + name: Annotated[str, typer.Argument(help="Instance name (used as directory name)")], + from_scenario: Annotated[ + Optional[Path], + typer.Option("--from-scenario", help="Initialize from a scenarios.yaml or instance.yaml"), + ] = None, + ports_base: Annotated[ + int, + typer.Option("--ports-base", help="Base port (KAS ports computed as base+N*101)"), + ] = 8080, + platform_dist: Annotated[ + Optional[str], + typer.Option("--platform", help="Platform dist version (e.g., v0.9.0)"), + ] = None, +) -> None: + """Scaffold a new instance directory at tests/instances//.""" + settings = get_settings() + instance_dir = settings.instances_root / name + + if from_scenario is not None: + _init_from_scenario(name, from_scenario, instance_dir) + else: + if platform_dist is None: + typer.echo("Error: --platform is required when not using --from-scenario", err=True) + raise typer.Exit(2) + _init_minimal(name, instance_dir, ports_base, platform_dist) + + _validate_port_uniqueness(settings.instances_root, name) + typer.echo(f" Initialized instance '{name}' at {instance_dir}") + + +def _init_from_scenario(name: str, scenario_path: Path, instance_dir: Path) -> None: + """Copy the embedded Instance from a Scenario or load a standalone Instance.""" + from otdf_sdk_mgr.schema import load_instance, load_scenario + from ruamel.yaml import YAML + + y = YAML(typ="safe") + raw = y.load(scenario_path.read_text()) + if not isinstance(raw, dict): + raise typer.BadParameter(f"{scenario_path} top-level YAML must be a mapping") + kind = raw.get("kind") + if kind == "Scenario": + scenario = load_scenario(scenario_path) + instance = scenario.instance + elif kind == "Instance": + instance = load_instance(scenario_path) + else: + raise typer.BadParameter(f"{scenario_path} has unknown kind {kind!r}") + # Ensure the metadata name matches the chosen directory name. + instance.metadata = Metadata(**{**instance.metadata.model_dump(exclude_none=True), "name": name}) + instance_dir.mkdir(parents=True, exist_ok=True) + (instance_dir / "kas").mkdir(parents=True, exist_ok=True) + (instance_dir / "keys").mkdir(mode=0o700, parents=True, exist_ok=True) + (instance_dir / "logs").mkdir(parents=True, exist_ok=True) + dump_instance(instance, instance_dir / "instance.yaml") + + +def _init_minimal(name: str, instance_dir: Path, ports_base: int, platform_dist: str) -> None: + """Create a barebones instance.yaml with default KAS layout.""" + instance = Instance( + metadata=Metadata(name=name), + platform=PlatformPin(dist=platform_dist), + ports=PortsConfig(base=ports_base), + kas={}, + ) + instance_dir.mkdir(parents=True, exist_ok=True) + (instance_dir / "kas").mkdir(parents=True, exist_ok=True) + (instance_dir / "keys").mkdir(mode=0o700, parents=True, exist_ok=True) + (instance_dir / "logs").mkdir(parents=True, exist_ok=True) + dump_instance(instance, instance_dir / "instance.yaml") + + +def _validate_port_uniqueness(instances_root: Path, new_name: str) -> None: + """Warn if another instance shares the same `ports.base`.""" + from otdf_sdk_mgr.schema import load_instance + + new_yaml = instances_root / new_name / "instance.yaml" + if not new_yaml.exists(): + return + new_inst = load_instance(new_yaml) + new_base = new_inst.ports.base + if not instances_root.exists(): + return + for child in instances_root.iterdir(): + if not child.is_dir() or child.name == new_name: + continue + other_yaml = child / "instance.yaml" + if not other_yaml.is_file(): + continue + try: + other = load_instance(other_yaml) + except Exception: + continue + if other.ports.base == new_base: + typer.echo( + f" Warning: instance '{child.name}' already uses ports.base={new_base}; " + f"running both simultaneously will collide. Change one with `otdf-local instance init`.", + err=True, + ) + + +@instance_app.command("ls") +def ls( + as_json: Annotated[bool, typer.Option("--json", "-j", help="Emit JSON")] = False, +) -> None: + """List known instances.""" + import json as _json + + from otdf_sdk_mgr.schema import load_instance + + settings = get_settings() + root = settings.instances_root + if not root.exists(): + if as_json: + typer.echo(_json.dumps([])) + else: + typer.echo(" (no instances yet)") + return + rows: list[dict[str, object]] = [] + for child in sorted(root.iterdir()): + if not child.is_dir(): + continue + ymp = child / "instance.yaml" + if not ymp.is_file(): + continue + try: + inst = load_instance(ymp) + except Exception as e: + rows.append({"name": child.name, "error": str(e)}) + continue + rows.append( + { + "name": child.name, + "platform": ( + inst.platform.dist + or (inst.platform.source.ref if inst.platform.source else inst.platform.image) + ), + "ports_base": inst.ports.base, + "kas": list(inst.kas.keys()), + } + ) + if as_json: + typer.echo(_json.dumps(rows, indent=2)) + else: + for row in rows: + typer.echo(f" {row}") + + +@instance_app.command("rm") +def rm( + name: Annotated[str, typer.Argument(help="Instance to remove")], + yes: Annotated[bool, typer.Option("--yes", "-y", help="Skip confirmation")] = False, +) -> None: + """Remove an instance directory.""" + settings = get_settings() + instance_dir = settings.instances_root / name + if not instance_dir.exists(): + typer.echo(f"Error: instance '{name}' not found at {instance_dir}", err=True) + raise typer.Exit(1) + if not yes: + confirm = typer.confirm(f"Delete {instance_dir}?", default=False) + if not confirm: + typer.echo("aborted") + raise typer.Exit(1) + shutil.rmtree(instance_dir) + typer.echo(f" Removed {instance_dir}") diff --git a/otdf-local/src/otdf_local/cli_scenario.py b/otdf-local/src/otdf_local/cli_scenario.py new file mode 100644 index 000000000..7d1dfde30 --- /dev/null +++ b/otdf-local/src/otdf_local/cli_scenario.py @@ -0,0 +1,101 @@ +"""`otdf-local scenario` subcommands. + +Today's surface area is intentionally narrow — `run` is the only command +that's part of the bug-repro MVP. Bisect and other higher-level loops are +deferred (see plan §9). +""" + +from __future__ import annotations + +import os +import subprocess +from pathlib import Path +from typing import Annotated + +import typer +from otdf_sdk_mgr.schema import ( + Scenario, + installed_json_for, + load_scenario, + scenario_to_pytest_sdks, +) + +from otdf_local.config.settings import get_settings + +scenario_app = typer.Typer(help="Run scenarios.yaml against a healthy instance.") + + +def _build_pytest_args(scenario: Scenario, scenario_path: Path) -> list[str]: + """Translate the scenario's `suite` block into pytest CLI args. + + SDK pins go through `scenario_to_pytest_sdks` so they're forwarded as + the `sdk@` tokens xtest's #446 specifier format expects. + Requires that `otdf-sdk-mgr install scenario` has been run first; the + helper raises FileNotFoundError with a clean hint otherwise. + """ + suite = scenario.suite + args: list[str] = [suite.select] + + tokens = scenario_to_pytest_sdks(scenario, installed_json_for(scenario_path)) + if tokens["encrypt"]: + args.extend(["--sdks-encrypt", " ".join(tokens["encrypt"])]) + if tokens["decrypt"]: + args.extend(["--sdks-decrypt", " ".join(tokens["decrypt"])]) + if suite.containers: + args.extend(["--containers", suite.containers]) + if suite.markers: + args.extend(["-m", suite.markers]) + args.extend(suite.extra_args) + return args + + +@scenario_app.command("run") +def run( + path: Annotated[Path, typer.Argument(help="Path to scenarios.yaml")], + instance: Annotated[ + str | None, + typer.Option( + "--instance", + help="Override which instance to use (defaults to scenario.instance.metadata.name)", + ), + ] = None, + extra: Annotated[ + list[str] | None, + typer.Argument(help="Extra args passed through to pytest (after --)"), + ] = None, +) -> None: + """Run the pytest suite declared by the scenario against its instance.""" + if not path.exists(): + typer.echo(f"Error: {path} not found", err=True) + raise typer.Exit(1) + + scenario = load_scenario(path) + instance_name = instance or scenario.instance.metadata.name + if not instance_name: + typer.echo("Error: scenario.instance.metadata.name not set; pass --instance", err=True) + raise typer.Exit(2) + + settings = get_settings() + # Force the chosen instance via env so child pytest invocations agree. + os.environ["OTDF_LOCAL_INSTANCE_NAME"] = instance_name + + xtest_root = settings.xtest_root + if not xtest_root.exists(): + typer.echo(f"Error: xtest root not found at {xtest_root}", err=True) + raise typer.Exit(1) + + try: + pytest_args = _build_pytest_args(scenario, path) + except FileNotFoundError as e: + typer.echo(f"Error: {e}", err=True) + raise typer.Exit(1) + except ValueError as e: + typer.echo(f"Error: {e}", err=True) + raise typer.Exit(1) + if extra: + pytest_args.extend(extra) + + cmd = ["uv", "run", "pytest", *pytest_args] + typer.echo(f" Running: {' '.join(cmd)} (cwd={xtest_root})") + completed = subprocess.run(cmd, cwd=xtest_root) + raise typer.Exit(completed.returncode) diff --git a/otdf-local/src/otdf_local/config/ports.py b/otdf-local/src/otdf_local/config/ports.py index 21d193358..913f970d0 100644 --- a/otdf-local/src/otdf_local/config/ports.py +++ b/otdf-local/src/otdf_local/config/ports.py @@ -33,14 +33,40 @@ class Ports: "km2": "KAS_KM2", } + # Offset of each KAS port from `base` (which is the platform port). + # The defaults at base=8080 reproduce the historical 8181/8282/... layout. + KAS_OFFSETS: ClassVar[dict[str, int]] = { + "alpha": 101, + "beta": 202, + "gamma": 303, + "delta": 404, + "km1": 505, + "km2": 606, + } + @classmethod - def get_kas_port(cls, name: str) -> int: - """Get port for a KAS instance by name.""" + def get_kas_port(cls, name: str, *, base: int | None = None) -> int: + """Get port for a KAS instance by name. + + When `base` is provided, the port is computed as `base + offset` so + multiple instances can coexist on disjoint port ranges. Otherwise the + legacy class constants are returned (base=8080 layout). + """ + if base is not None: + offset = cls.KAS_OFFSETS.get(name) + if offset is None: + raise ValueError(f"Unknown KAS instance: {name}") + return base + offset attr = cls._KAS_NAMES.get(name) if attr is None: raise ValueError(f"Unknown KAS instance: {name}") return getattr(cls, attr) + @classmethod + def platform_port_for(cls, base: int) -> int: + """Return the platform port for a given `base`. Trivially `base` today.""" + return base + @classmethod def all_kas_names(cls) -> list[str]: """Return all KAS instance names.""" diff --git a/otdf-local/src/otdf_local/config/settings.py b/otdf-local/src/otdf_local/config/settings.py index 96a4c20e8..dffc2cefc 100644 --- a/otdf-local/src/otdf_local/config/settings.py +++ b/otdf-local/src/otdf_local/config/settings.py @@ -8,6 +8,8 @@ from otdf_local.config.ports import Ports +DEFAULT_INSTANCE_NAME = "default" + def _pyproject_has_name(path: Path, project_name: str) -> bool: """Return True if path/pyproject.toml contains the given project name.""" @@ -80,6 +82,19 @@ def _find_platform_dir(xtest_root: Path) -> Path: ) +def _find_platform_dir_optional(xtest_root: Path) -> Path | None: + """Same as `_find_platform_dir` but returns None instead of raising. + + Multi-instance mode looks up platform binaries via `otdf-sdk-mgr` instead of + a sibling repo, so a missing sibling `platform/` is no longer fatal — only + the legacy single-instance path needs it. + """ + try: + return _find_platform_dir(xtest_root) + except FileNotFoundError: + return None + + class Settings(BaseSettings): """Application settings with environment variable support.""" @@ -91,44 +106,100 @@ class Settings(BaseSettings): # Directory paths - computed from xtest_root xtest_root: Path = Field(default_factory=_find_xtest_root) - platform_dir: Path = Field( - default_factory=lambda: _find_platform_dir(_find_xtest_root()) + platform_dir: Path | None = Field( + default_factory=lambda: _find_platform_dir_optional(_find_xtest_root()) ) + # Multi-instance: which named instance under `tests/instances//` to use. + instance_name: str = DEFAULT_INSTANCE_NAME + + @property + def tests_root(self) -> Path: + """Repo root that holds `xtest/`, `instances/`, `otdf-local/`, etc.""" + return self.xtest_root.parent + + @property + def instances_root(self) -> Path: + """Top-level `tests/instances/` directory (created on demand).""" + return self.tests_root / "instances" + + @property + def instance_dir(self) -> Path: + """Per-instance directory: `tests/instances//`.""" + return self.instances_root / self.instance_name + + @property + def instance_yaml(self) -> Path: + """Path to the per-instance manifest.""" + return self.instance_dir / "instance.yaml" + + def has_instance(self) -> bool: + """Return True if `instance.yaml` exists for the selected instance.""" + return self.instance_yaml.is_file() + + def platform_binary_for(self, dist: str) -> Path: + """Resolve a platform dist version to its built `service` binary path. + + Looks under `xtest/platform/dist//service` (managed by + `otdf-sdk-mgr install platform:`). The binary is not required + to exist at the time of the call — callers should check existence and + surface a clear error suggesting `otdf-sdk-mgr install` when missing. + """ + from otdf_sdk_mgr.platform_installer import get_platform_dir + + return get_platform_dir() / "dist" / dist / "service" + @property def logs_dir(self) -> Path: - """Logs directory.""" + """Logs directory. Per-instance when an instance is selected, falls back to legacy.""" + if self.has_instance(): + return self.instance_dir / "logs" return self.xtest_root / "tmp" / "logs" @property def keys_dir(self) -> Path: - """Keys directory.""" + """Keys directory. Per-instance when an instance is selected, falls back to legacy.""" + if self.has_instance(): + return self.instance_dir / "keys" return self.xtest_root / "tmp" / "keys" @property def config_dir(self) -> Path: - """Generated config files directory.""" + """Generated config files directory. Per-instance when present.""" + if self.has_instance(): + return self.instance_dir return self.xtest_root / "tmp" / "config" + def _require_platform_dir(self) -> Path: + if self.platform_dir is None: + raise FileNotFoundError( + "No sibling platform/ directory found. Either check out opentdf/platform as " + "a sibling of tests/, or run `otdf-sdk-mgr install platform:` and " + "select an instance with `otdf-local --instance `." + ) + return self.platform_dir + @property def platform_config(self) -> Path: - """Platform config file path.""" - return self.platform_dir / "opentdf-dev.yaml" + """Platform config file. Per-instance when present, else legacy template.""" + if self.has_instance(): + return self.instance_dir / "opentdf.yaml" + return self._require_platform_dir() / "opentdf-dev.yaml" @property def platform_template_config(self) -> Path: - """Platform config template path.""" - return self.platform_dir / "opentdf.yaml" + """Platform config template path (legacy mode).""" + return self._require_platform_dir() / "opentdf.yaml" @property def kas_template_config(self) -> Path: - """KAS config template path.""" - return self.platform_dir / "opentdf-kas-mode.yaml" + """KAS config template path (legacy mode).""" + return self._require_platform_dir() / "opentdf-kas-mode.yaml" @property def docker_compose_file(self) -> Path: """Docker compose file path.""" - return self.platform_dir / "docker-compose.yaml" + return self._require_platform_dir() / "docker-compose.yaml" # Service ports keycloak_port: int = Ports.KEYCLOAK @@ -147,11 +218,28 @@ def docker_compose_file(self) -> Path: log_level: str = "info" def get_kas_port(self, name: str) -> int: - """Get port for a KAS instance.""" + """Get port for a KAS instance. + + When an `instance.yaml` exists with a `ports.base`, computes ports + relative to it so multiple instances on different bases don't clash. + """ + instance = self.load_instance() + if instance is not None: + return Ports.get_kas_port(name, base=instance.ports.base) return Ports.get_kas_port(name) + def load_instance(self): + """Load the per-instance manifest, or return None when not present.""" + if not self.has_instance(): + return None + from otdf_sdk_mgr.schema import load_instance as _load + + return _load(self.instance_yaml) + def get_kas_config_path(self, name: str) -> Path: """Get config file path for a KAS instance.""" + if self.has_instance(): + return self.instance_dir / "kas" / name / "opentdf.yaml" return self.config_dir / f"kas-{name}.yaml" def get_kas_log_path(self, name: str) -> Path: @@ -163,6 +251,8 @@ def ensure_directories(self) -> None: self.logs_dir.mkdir(parents=True, exist_ok=True) self.config_dir.mkdir(parents=True, exist_ok=True) self.keys_dir.mkdir(mode=0o700, parents=True, exist_ok=True) + if self.has_instance(): + (self.instance_dir / "kas").mkdir(parents=True, exist_ok=True) @lru_cache diff --git a/otdf-local/src/otdf_local/services/kas.py b/otdf-local/src/otdf_local/services/kas.py index 0b7adfa64..d06350e77 100644 --- a/otdf-local/src/otdf_local/services/kas.py +++ b/otdf-local/src/otdf_local/services/kas.py @@ -35,7 +35,7 @@ def name(self) -> str: @property def port(self) -> int: - return Ports.get_kas_port(self._kas_name) + return self.settings.get_kas_port(self._kas_name) @property def service_type(self) -> ServiceType: @@ -47,25 +47,60 @@ def health_url(self) -> str: @property def is_key_management(self) -> bool: - """Check if this is a key management KAS instance.""" + """Check if this is a key management KAS instance. + + When an instance.yaml pins this KAS, prefer the manifest's `mode` + field. Otherwise fall back to the legacy name-based heuristic. + """ + instance = self.settings.load_instance() + if instance is not None and self._kas_name in instance.kas: + return instance.kas[self._kas_name].mode == "key_management" return Ports.is_km_kas(self._kas_name) + def _instance_paths(self) -> tuple[Path, Path] | None: + """Return (binary, worktree) for an instance-pinned KAS, or None.""" + instance = self.settings.load_instance() + if instance is None: + return None + pin = instance.kas.get(self._kas_name) + if pin is None or pin.dist is None: + return None + binary = self.settings.platform_binary_for(pin.dist) + if not binary.exists(): + raise FileNotFoundError( + f"KAS {self._kas_name} binary not found at {binary}. " + f"Run `otdf-sdk-mgr install release platform:{pin.dist}`." + ) + worktree = binary.parent + version_file = binary.parent / ".version" + if version_file.exists(): + for line in version_file.read_text().splitlines(): + if line.startswith("worktree="): + worktree = Path(line.split("=", 1)[1].strip()) + break + return binary, worktree + def _generate_config(self) -> Path: """Generate the KAS config file from template.""" + instance_paths = self._instance_paths() + if instance_paths is not None: + _, worktree = instance_paths + platform_dir = worktree + else: + platform_dir = self.settings._require_platform_dir() + config_path = self.settings.get_kas_config_path(self._kas_name) - template_path = self.settings.kas_template_config + config_path.parent.mkdir(parents=True, exist_ok=True) + template_path = platform_dir / "opentdf-kas-mode.yaml" # Load platform config to get root_key platform_config = load_yaml(self.settings.platform_config) root_key = get_nested(platform_config, "services.kas.root_key", "") # Detect platform features to determine supported config options - features = PlatformFeatures.detect(self.settings.platform_dir) - - # Use stderr if supported, otherwise stdout (v0.9.0 only supports stdout) + features = PlatformFeatures.detect(platform_dir) logger_output = "stderr" if features.supports("logger_stderr") else "stdout" - # Base updates for all KAS instances updates = { "logger.type": "json", "logger.output": logger_output, @@ -73,44 +108,43 @@ def _generate_config(self) -> Path: "services.kas.root_key": root_key, } - # Key management KAS instances need additional config + # Per-KAS features from instance.yaml override the legacy heuristic. + instance = self.settings.load_instance() + kas_pin = instance.kas.get(self._kas_name) if instance is not None else None + extra_features: dict[str, bool] = dict(kas_pin.features) if kas_pin is not None else {} + if self.is_key_management: updates["services.kas.preview.key_management"] = True updates["services.kas.preview.ec_tdf_enabled"] = True - # registered_kas_uri should NOT have /kas suffix updates["services.kas.registered_kas_uri"] = f"http://localhost:{self.port}" + for feature_key, feature_val in extra_features.items(): + updates[f"services.kas.preview.{feature_key}"] = feature_val + copy_yaml_with_updates(template_path, config_path, updates) return config_path def start(self) -> bool: """Start the KAS instance.""" - # Ensure directories exist self.settings.ensure_directories() - - # Kill any existing process on the port kill_process_on_port(self.port) - - # Generate config config_path = self._generate_config() - # Build the command - cmd = [ - "go", - "run", - "./service", - "start", - "--config-file", - str(config_path), - ] - - # Start the process + instance_paths = self._instance_paths() + if instance_paths is not None: + binary, worktree = instance_paths + cmd = [str(binary), "start", "--config-file", str(config_path)] + cwd = worktree + else: + cmd = ["go", "run", "./service", "start", "--config-file", str(config_path)] + cwd = self.settings._require_platform_dir() + log_file = self.settings.get_kas_log_path(self._kas_name) self._process = self._process_manager.start( name=self.name, cmd=cmd, - cwd=self.settings.platform_dir, + cwd=cwd, log_file=log_file, env={"OPENTDF_LOG_LEVEL": "info"}, ) @@ -148,7 +182,12 @@ def get_info(self) -> ServiceInfo: class KASManager: - """Manages all KAS instances.""" + """Manages KAS instances. + + When an `instance.yaml` is loaded, the managed set is restricted to the + KAS names listed in the manifest. Otherwise the legacy full set + (alpha/beta/gamma/delta/km1/km2) is managed. + """ def __init__( self, @@ -159,8 +198,13 @@ def __init__( self._process_manager = process_manager or ProcessManager() self._instances: dict[str, KASService] = {} - # Create instances for all configured KAS - for kas_name in Ports.all_kas_names(): + instance = settings.load_instance() + if instance is not None and instance.kas: + kas_names = list(instance.kas.keys()) + else: + kas_names = Ports.all_kas_names() + + for kas_name in kas_names: self._instances[kas_name] = KASService( settings, kas_name, self._process_manager ) @@ -184,17 +228,19 @@ def stop_all(self) -> dict[str, bool]: return results def start_standard(self) -> dict[str, bool]: - """Start only standard (non-km) KAS instances.""" + """Start only standard (non-key-management) KAS instances under management.""" results = {} - for name in Ports.standard_kas_names(): - results[name] = self._instances[name].start() + for name, inst in self._instances.items(): + if not inst.is_key_management: + results[name] = inst.start() return results def start_km(self) -> dict[str, bool]: - """Start only key management KAS instances.""" + """Start only key-management KAS instances under management.""" results = {} - for name in Ports.km_kas_names(): - results[name] = self._instances[name].start() + for name, inst in self._instances.items(): + if inst.is_key_management: + results[name] = inst.start() return results def get_all_info(self) -> list[ServiceInfo]: diff --git a/otdf-local/src/otdf_local/services/platform.py b/otdf-local/src/otdf_local/services/platform.py index 15f7f4e5e..aa65dcf1d 100644 --- a/otdf-local/src/otdf_local/services/platform.py +++ b/otdf-local/src/otdf_local/services/platform.py @@ -39,6 +39,9 @@ def name(self) -> str: @property def port(self) -> int: + instance = self.settings.load_instance() + if instance is not None: + return Ports.platform_port_for(instance.ports.base) return Ports.PLATFORM @property @@ -49,13 +52,46 @@ def service_type(self) -> ServiceType: def health_url(self) -> str: return f"http://localhost:{self.port}/healthz" + def _instance_dist_paths(self) -> tuple[Path, Path] | None: + """Return (binary, worktree) for an instance-pinned platform, or None. + + The platform binary is at `xtest/platform/dist//service` and its + `.version` file records the source worktree path that should be used + as `cwd` so the binary finds its embedded resources. + """ + instance = self.settings.load_instance() + if instance is None or instance.platform.dist is None: + return None + binary = self.settings.platform_binary_for(instance.platform.dist) + if not binary.exists(): + raise FileNotFoundError( + f"Platform binary not found at {binary}. " + f"Run `otdf-sdk-mgr install release platform:{instance.platform.dist}` " + f"or `otdf-sdk-mgr install scenario` to provision it." + ) + worktree = binary.parent # safe fallback + version_file = binary.parent / ".version" + if version_file.exists(): + for line in version_file.read_text().splitlines(): + if line.startswith("worktree="): + worktree = Path(line.split("=", 1)[1].strip()) + break + return binary, worktree + def _generate_config(self) -> Path: """Generate the platform config file from template.""" + instance_paths = self._instance_dist_paths() + if instance_paths is not None: + _, worktree = instance_paths + platform_dir = worktree + else: + platform_dir = self.settings._require_platform_dir() + config_path = self.settings.platform_config - template_path = self.settings.platform_template_config + template_path = platform_dir / "opentdf.yaml" # Detect platform features to determine supported config options - features = PlatformFeatures.detect(self.settings.platform_dir) + features = PlatformFeatures.detect(platform_dir) # Use stderr if supported, otherwise stdout (v0.9.0 only supports stdout) logger_output = "stderr" if features.supports("logger_stderr") else "stdout" @@ -80,10 +116,14 @@ def _setup_golden_keys(self, config_path: Path) -> None: Extracts keys from extra-keys.json and adds them to the platform config so legacy golden TDFs can be decrypted. """ - # Set up golden key files and get their config entries + # In multi-instance mode, golden keys live alongside the instance + # config; otherwise they go into the legacy platform_dir. + target_dir = self.settings.keys_dir if self.settings.has_instance() else ( + self.settings._require_platform_dir() + ) golden_keys = setup_golden_keys( self.settings.xtest_root, - self.settings.platform_dir, + target_dir, ) if not golden_keys: @@ -112,15 +152,16 @@ def start(self) -> bool: # Generate config config_path = self._generate_config() - # Build the command - cmd = [ - "go", - "run", - "./service", - "start", - "--config-file", - str(config_path), - ] + # Build the command — pinned binary when an instance is loaded, + # legacy `go run ./service` otherwise. + instance_paths = self._instance_dist_paths() + if instance_paths is not None: + binary, worktree = instance_paths + cmd = [str(binary), "start", "--config-file", str(config_path)] + cwd = worktree + else: + cmd = ["go", "run", "./service", "start", "--config-file", str(config_path)] + cwd = self.settings._require_platform_dir() # Start the process log_file = self.settings.logs_dir / "platform.log" @@ -128,7 +169,7 @@ def start(self) -> bool: self._process = self._process_manager.start( name=self.name, cmd=cmd, - cwd=self.settings.platform_dir, + cwd=cwd, log_file=log_file, env={"OPENTDF_LOG_LEVEL": "info"}, ) diff --git a/otdf-local/src/otdf_local/utils/keys.py b/otdf-local/src/otdf_local/utils/keys.py index dee84f2af..79b58bf08 100644 --- a/otdf-local/src/otdf_local/utils/keys.py +++ b/otdf-local/src/otdf_local/utils/keys.py @@ -197,7 +197,9 @@ def setup_golden_keys( f"Missing required fields in extra-keys.json for kid: {kid}" ) - # Write key files to platform directory + # Write key files into the target directory (platform_dir for legacy + # single-instance, or the per-instance keys dir for multi-instance). + platform_dir.mkdir(parents=True, exist_ok=True) private_path = platform_dir / f"{kid}-private.pem" cert_path = platform_dir / f"{kid}-cert.pem" @@ -205,12 +207,14 @@ def setup_golden_keys( private_path.chmod(0o600) cert_path.write_text(cert) + # Use absolute paths so the platform binary finds them regardless of + # its working directory (worktree in multi-instance mode). keys_config.append( { "kid": kid, "alg": alg, - "private": f"{kid}-private.pem", - "cert": f"{kid}-cert.pem", + "private": str(private_path.resolve()), + "cert": str(cert_path.resolve()), } ) diff --git a/otdf-local/tests/test_multi_instance.py b/otdf-local/tests/test_multi_instance.py new file mode 100644 index 000000000..e290d7731 --- /dev/null +++ b/otdf-local/tests/test_multi_instance.py @@ -0,0 +1,78 @@ +"""Smoke tests for the multi-instance refactor. + +These tests exercise the path resolution and port arithmetic without +requiring a real platform build or running services. The goal is to catch +regressions in the wiring between `otdf-sdk-mgr.schema`, `Settings`, and the +service launchers. +""" + +from __future__ import annotations + +import os +from pathlib import Path + +import pytest +from otdf_sdk_mgr.schema import ( + Instance, + KasPin, + Metadata, + PlatformPin, + PortsConfig, + dump_instance, +) + +from otdf_local.config.ports import Ports +from otdf_local.config.settings import Settings + + +def test_ports_offset_layout_at_default_base() -> None: + assert Ports.platform_port_for(8080) == 8080 + assert Ports.get_kas_port("alpha", base=8080) == 8181 + assert Ports.get_kas_port("km2", base=8080) == 8686 + + +def test_ports_offset_layout_at_alternate_base() -> None: + assert Ports.platform_port_for(9080) == 9080 + assert Ports.get_kas_port("alpha", base=9080) == 9181 + assert Ports.get_kas_port("km1", base=9080) == 9585 + + +def test_settings_default_has_no_instance(tmp_path: Path) -> None: + fake_xtest = tmp_path / "xtest" + fake_xtest.mkdir() + s = Settings(xtest_root=fake_xtest, platform_dir=None) + assert s.instance_name == "default" + assert not s.has_instance() + + +def test_settings_loads_instance_when_present(tmp_path: Path) -> None: + fake_xtest = tmp_path / "xtest" + fake_xtest.mkdir() + instances_root = tmp_path / "instances" + instance_dir = instances_root / "demo" + instance_dir.mkdir(parents=True) + dump_instance( + Instance( + metadata=Metadata(name="demo"), + platform=PlatformPin(dist="v0.9.0"), + ports=PortsConfig(base=9080), + kas={"alpha": KasPin(dist="v0.9.0", mode="standard")}, + ), + instance_dir / "instance.yaml", + ) + s = Settings(xtest_root=fake_xtest, platform_dir=None, instance_name="demo") + assert s.has_instance() + inst = s.load_instance() + assert inst is not None + assert inst.ports.base == 9080 + # Per-instance port arithmetic + assert s.get_kas_port("alpha") == 9181 + # Per-instance directory layout + assert s.logs_dir == instance_dir / "logs" + assert s.keys_dir == instance_dir / "keys" + + +def test_platform_binary_for_resolves_under_xtest_platform(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("OTDF_PLATFORM_DIR", "/tmp/fake-platform") + s = Settings() + assert s.platform_binary_for("v0.9.0") == Path("/tmp/fake-platform/dist/v0.9.0/service") diff --git a/otdf-local/uv.lock b/otdf-local/uv.lock index f1b9d2423..d781cc92c 100644 --- a/otdf-local/uv.lock +++ b/otdf-local/uv.lock @@ -54,6 +54,30 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/d1/d6/3965ed04c63042e047cb6a3e6ed1a63a35087b6a609aa3a15ed8ac56c221/colorama-0.4.6-py2.py3-none-any.whl", hash = "sha256:4f1d9991f5acc0ca119f9d443620b77f9d6b33703e51011c16baf57afb285fc6", size = 25335, upload-time = "2022-10-25T02:36:20.889Z" }, ] +[[package]] +name = "gitdb" +version = "4.0.12" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "smmap" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/72/94/63b0fc47eb32792c7ba1fe1b694daec9a63620db1e313033d18140c2320a/gitdb-4.0.12.tar.gz", hash = "sha256:5ef71f855d191a3326fcfbc0d5da835f26b13fbcba60c32c21091c349ffdb571", size = 394684, upload-time = "2025-01-02T07:20:46.413Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/a0/61/5c78b91c3143ed5c14207f463aecfc8f9dbb5092fb2869baf37c273b2705/gitdb-4.0.12-py3-none-any.whl", hash = "sha256:67073e15955400952c6565cc3e707c554a4eea2e428946f7a4c162fab9bd9bcf", size = 62794, upload-time = "2025-01-02T07:20:43.624Z" }, +] + +[[package]] +name = "gitpython" +version = "3.1.50" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "gitdb" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/33/f6/354ae6491228b5eb40e10d89c4d13c651fe1cf7556e35ebdded50cff57ce/gitpython-3.1.50.tar.gz", hash = "sha256:80da2d12504d52e1f998772dc5baf6e553f8d2fcfe1fcc226c9d9a2ee3372dcc", size = 219798, upload-time = "2026-05-06T04:01:26.571Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/20/7a/1c6e3562dfd8950adbb11ffbc65d21e7c89d01a6e4f137fa981056de25c5/gitpython-3.1.50-py3-none-any.whl", hash = "sha256:d352abe2908d07355014abdd21ddf798c2a961469239afec4962e9da884858f9", size = 212507, upload-time = "2026-05-06T04:01:23.799Z" }, +] + [[package]] name = "h11" version = "0.16.0" @@ -145,6 +169,7 @@ version = "0.1.0" source = { editable = "." } dependencies = [ { name = "httpx" }, + { name = "otdf-sdk-mgr" }, { name = "pydantic-settings" }, { name = "rich" }, { name = "ruamel-yaml" }, @@ -161,6 +186,7 @@ dev = [ [package.metadata] requires-dist = [ { name = "httpx", specifier = ">=0.27.0" }, + { name = "otdf-sdk-mgr", editable = "../otdf-sdk-mgr" }, { name = "pydantic-settings", specifier = ">=2.2.0" }, { name = "rich", specifier = ">=13.7.0" }, { name = "ruamel-yaml", specifier = ">=0.18.0" }, @@ -174,6 +200,34 @@ dev = [ { name = "ruff", specifier = ">=0.9.0" }, ] +[[package]] +name = "otdf-sdk-mgr" +version = "0.1.0" +source = { editable = "../otdf-sdk-mgr" } +dependencies = [ + { name = "gitpython" }, + { name = "pydantic" }, + { name = "rich" }, + { name = "ruamel-yaml" }, + { name = "typer" }, +] + +[package.metadata] +requires-dist = [ + { name = "gitpython", specifier = ">=3.1.50" }, + { name = "pydantic", specifier = ">=2.6.0" }, + { name = "rich", specifier = ">=13.7.0" }, + { name = "ruamel-yaml", specifier = ">=0.18.0" }, + { name = "typer", specifier = ">=0.12.0" }, +] + +[package.metadata.requires-dev] +dev = [ + { name = "pyright", specifier = ">=1.1.408" }, + { name = "pytest", specifier = ">=9.0.3" }, + { name = "ruff", specifier = ">=0.9.0" }, +] + [[package]] name = "packaging" version = "26.0" @@ -421,6 +475,15 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/e0/f9/0595336914c5619e5f28a1fb793285925a8cd4b432c9da0a987836c7f822/shellingham-1.5.4-py2.py3-none-any.whl", hash = "sha256:7ecfff8f2fd72616f7481040475a65b2bf8af90a56c89140852d1120324e8686", size = 9755, upload-time = "2023-10-24T04:13:38.866Z" }, ] +[[package]] +name = "smmap" +version = "5.0.3" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/1f/ea/49c993d6dfdd7338c9b1000a0f36817ed7ec84577ae2e52f890d1a4ff909/smmap-5.0.3.tar.gz", hash = "sha256:4d9debb8b99007ae47165abc08670bd74cb74b5227dda7f643eccc4e9eb5642c", size = 22506, upload-time = "2026-03-09T03:43:26.1Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/c1/d4/59e74daffcb57a07668852eeeb6035af9f32cbfd7a1d2511f17d2fe6a738/smmap-5.0.3-py3-none-any.whl", hash = "sha256:c106e05d5a61449cf6ba9a1e650227ecfb141590d2a98412103ff35d89fc7b2f", size = 24390, upload-time = "2026-03-09T03:43:24.361Z" }, +] + [[package]] name = "typer" version = "0.21.1" From 5c1e619f54741892a9bc761cba551f326c68e642 Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Tue, 2 Jun 2026 08:25:33 -0400 Subject: [PATCH 16/18] feat(otdf-local): self-provision keys + opentdf.yaml at instance init MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this change, `otdf-local instance init` only wrote `instance.yaml` and empty subdirs. Anyone running a fresh instance had to manually copy keys from another worktree, run `init-temp-keys.sh` by hand, and copy `opentdf-dev.yaml` into the instance dir before `up` would succeed — otherwise Keycloak crash-looped on a missing `truststore.jks`, and pytest failed with `OT_ROOT_KEY environment variable is not set`. Changes: - utils/keys.py: add `generate_localhost_cert()` and `generate_ca_jks()` to produce the Keycloak TLS pair + JKS truststore (matches the platform's `init-temp-keys.sh`). `generate_ca_jks()` runs `keytool` inside the `keycloak/keycloak:25.0` image so a local JDK isn't required. `ensure_keys_exist()` now generates the full bootstrap bundle, idempotently. - cli_instance.py: `_init_from_scenario` and `_init_minimal` call a new `_provision_instance_dir()` helper that runs `ensure_keys_exist()` and copies the platform's `opentdf-dev.yaml` (or `opentdf-example.yaml`) into the instance dir, overriding `services.kas.root_key` with a freshly generated value so every instance owns its own root key. - services/platform.py: `_generate_config()` preserves an existing per-instance `opentdf.yaml`, only patching logger + golden-key fields in place, so the init-time `root_key` survives restarts. - services/docker.py: docker-compose subprocesses are now run with `KEYS_DIR=/keys` so the compose file's `${KEYS_DIR:-./keys}` mounts resolve to the per-instance bundle. Users can now run: otdf-local instance init --from-scenario path/to/scenario.yaml otdf-local --instance up eval $(otdf-local --instance env) cd xtest && uv run pytest ... with no manual key-copying, no editing of `opentdf.yaml`, and no shell-script fallback. Verified end-to-end against `pure-mlkem.yaml` (PR opentdf/platform#3537): all 9 services come up healthy on the first try and `env` exports `OT_ROOT_KEY`. Co-Authored-By: Claude Opus 4.7 --- otdf-local/src/otdf_local/cli_instance.py | 116 +++++++++- otdf-local/src/otdf_local/services/docker.py | 11 + .../src/otdf_local/services/platform.py | 26 ++- otdf-local/src/otdf_local/utils/keys.py | 200 +++++++++++++++++- 4 files changed, 334 insertions(+), 19 deletions(-) diff --git a/otdf-local/src/otdf_local/cli_instance.py b/otdf-local/src/otdf_local/cli_instance.py index 98407f56e..712dc2967 100644 --- a/otdf-local/src/otdf_local/cli_instance.py +++ b/otdf-local/src/otdf_local/cli_instance.py @@ -7,9 +7,17 @@ from typing import Annotated, Optional import typer -from otdf_sdk_mgr.schema import Instance, Metadata, PlatformPin, PortsConfig, dump_instance +from otdf_sdk_mgr.schema import ( + Instance, + Metadata, + PlatformPin, + PortsConfig, + dump_instance, +) -from otdf_local.config.settings import get_settings +from otdf_local.config.settings import Settings, get_settings +from otdf_local.utils.keys import ensure_keys_exist, generate_root_key +from otdf_local.utils.yaml import copy_yaml_with_updates instance_app = typer.Typer(help="Manage named test environment instances.") @@ -19,11 +27,15 @@ def init( name: Annotated[str, typer.Argument(help="Instance name (used as directory name)")], from_scenario: Annotated[ Optional[Path], - typer.Option("--from-scenario", help="Initialize from a scenarios.yaml or instance.yaml"), + typer.Option( + "--from-scenario", help="Initialize from a scenarios.yaml or instance.yaml" + ), ] = None, ports_base: Annotated[ int, - typer.Option("--ports-base", help="Base port (KAS ports computed as base+N*101)"), + typer.Option( + "--ports-base", help="Base port (KAS ports computed as base+N*101)" + ), ] = 8080, platform_dist: Annotated[ Optional[str], @@ -38,7 +50,10 @@ def init( _init_from_scenario(name, from_scenario, instance_dir) else: if platform_dist is None: - typer.echo("Error: --platform is required when not using --from-scenario", err=True) + typer.echo( + "Error: --platform is required when not using --from-scenario", + err=True, + ) raise typer.Exit(2) _init_minimal(name, instance_dir, ports_base, platform_dist) @@ -64,15 +79,20 @@ def _init_from_scenario(name: str, scenario_path: Path, instance_dir: Path) -> N else: raise typer.BadParameter(f"{scenario_path} has unknown kind {kind!r}") # Ensure the metadata name matches the chosen directory name. - instance.metadata = Metadata(**{**instance.metadata.model_dump(exclude_none=True), "name": name}) + instance.metadata = Metadata( + **{**instance.metadata.model_dump(exclude_none=True), "name": name} + ) instance_dir.mkdir(parents=True, exist_ok=True) (instance_dir / "kas").mkdir(parents=True, exist_ok=True) (instance_dir / "keys").mkdir(mode=0o700, parents=True, exist_ok=True) (instance_dir / "logs").mkdir(parents=True, exist_ok=True) dump_instance(instance, instance_dir / "instance.yaml") + _provision_instance_dir(instance_dir, instance) -def _init_minimal(name: str, instance_dir: Path, ports_base: int, platform_dist: str) -> None: +def _init_minimal( + name: str, instance_dir: Path, ports_base: int, platform_dist: str +) -> None: """Create a barebones instance.yaml with default KAS layout.""" instance = Instance( metadata=Metadata(name=name), @@ -85,6 +105,82 @@ def _init_minimal(name: str, instance_dir: Path, ports_base: int, platform_dist: (instance_dir / "keys").mkdir(mode=0o700, parents=True, exist_ok=True) (instance_dir / "logs").mkdir(parents=True, exist_ok=True) dump_instance(instance, instance_dir / "instance.yaml") + _provision_instance_dir(instance_dir, instance) + + +def _resolve_platform_worktree(instance: Instance) -> Path: + """Find the platform source worktree for this instance's pin. + + For both `dist` and `source` pins, the platform installer writes a + `.version` file next to the binary with `worktree=`. We follow + that pointer because the binary's parent directory only holds the + built artifact — the YAML templates live in the source tree. + """ + from otdf_sdk_mgr.platform_installer import get_platform_dir + from otdf_sdk_mgr.refs import expand_pr_shorthand, ref_slug + + settings = Settings() + pin = instance.platform + if pin.dist is not None: + dist_name = pin.dist + elif pin.source is not None: + dist_name = ref_slug(expand_pr_shorthand(pin.source.ref)) + else: + raise typer.BadParameter("instance.platform must set dist or source") + + binary = get_platform_dir() / "dist" / dist_name / "service" + if not binary.exists(): + raise FileNotFoundError( + f"Platform binary not found at {binary}. " + f"Run `otdf-sdk-mgr install scenario` (or `install release platform:`) " + f"to provision it before `instance init`." + ) + version_file = binary.parent / ".version" + if version_file.exists(): + for line in version_file.read_text().splitlines(): + if line.startswith("worktree="): + worktree = Path(line.split("=", 1)[1].strip()) + if worktree.is_dir(): + return worktree + # Fallback to sibling platform dir (legacy single-instance layout). + if settings.platform_dir is not None: + return settings.platform_dir + raise FileNotFoundError( + f"Could not resolve platform source worktree from {version_file}; " + f"no sibling platform/ directory available either." + ) + + +def _provision_instance_dir(instance_dir: Path, instance: Instance) -> None: + """Generate the bootstrap bundle: keys + opentdf.yaml with a fresh root_key. + + Idempotent — `ensure_keys_exist` skips files that already exist, and + `opentdf.yaml` is only generated when missing so reruns of `instance init` + don't churn the per-instance root_key. + """ + keys_dir = instance_dir / "keys" + keys_dir.mkdir(mode=0o700, parents=True, exist_ok=True) + ensure_keys_exist(keys_dir) + + config_path = instance_dir / "opentdf.yaml" + if config_path.exists(): + return + + worktree = _resolve_platform_worktree(instance) + template = worktree / "opentdf-dev.yaml" + if not template.is_file(): + template = worktree / "opentdf-example.yaml" + if not template.is_file(): + raise FileNotFoundError( + f"No platform config template found in {worktree} " + f"(looked for opentdf-dev.yaml and opentdf-example.yaml)." + ) + + copy_yaml_with_updates( + template, + config_path, + {"services.kas.root_key": generate_root_key()}, + ) def _validate_port_uniqueness(instances_root: Path, new_name: str) -> None: @@ -150,7 +246,11 @@ def ls( "name": child.name, "platform": ( inst.platform.dist - or (inst.platform.source.ref if inst.platform.source else inst.platform.image) + or ( + inst.platform.source.ref + if inst.platform.source + else inst.platform.image + ) ), "ports_base": inst.ports.base, "kas": list(inst.kas.keys()), diff --git a/otdf-local/src/otdf_local/services/docker.py b/otdf-local/src/otdf_local/services/docker.py index 911b42e3c..5cf746f2d 100644 --- a/otdf-local/src/otdf_local/services/docker.py +++ b/otdf-local/src/otdf_local/services/docker.py @@ -1,6 +1,7 @@ """Docker compose service management.""" import json +import os import subprocess from otdf_local.config.ports import Ports @@ -16,6 +17,13 @@ def __init__(self, settings: Settings) -> None: super().__init__(settings) self._compose_file = settings.docker_compose_file + def _compose_env(self) -> dict[str, str]: + """Env passed to docker-compose so `${KEYS_DIR}` resolves per-instance.""" + env = os.environ.copy() + if self.settings.has_instance(): + env["KEYS_DIR"] = str(self.settings.keys_dir.resolve()) + return env + @property def name(self) -> str: return "docker" @@ -42,6 +50,7 @@ def start(self) -> bool: capture_output=True, text=True, cwd=self._compose_file.parent, + env=self._compose_env(), ) return result.returncode == 0 @@ -55,6 +64,7 @@ def stop(self) -> bool: capture_output=True, text=True, cwd=self._compose_file.parent, + env=self._compose_env(), ) return result.returncode == 0 @@ -89,6 +99,7 @@ def get_container_status(self) -> dict[str, dict]: capture_output=True, text=True, cwd=self._compose_file.parent, + env=self._compose_env(), ) if result.returncode != 0: diff --git a/otdf-local/src/otdf_local/services/platform.py b/otdf-local/src/otdf_local/services/platform.py index aa65dcf1d..3f2ad9cb0 100644 --- a/otdf-local/src/otdf_local/services/platform.py +++ b/otdf-local/src/otdf_local/services/platform.py @@ -18,6 +18,7 @@ copy_yaml_with_updates, load_yaml, save_yaml, + set_nested, ) @@ -79,7 +80,13 @@ def _instance_dist_paths(self) -> tuple[Path, Path] | None: return binary, worktree def _generate_config(self) -> Path: - """Generate the platform config file from template.""" + """Generate the platform config file from template. + + When an instance config already exists (written at `instance init` + time), we keep its body intact — only patch logger keys + golden + keys in place. This preserves the per-instance root_key across + restarts. + """ instance_paths = self._instance_dist_paths() if instance_paths is not None: _, worktree = instance_paths @@ -88,7 +95,6 @@ def _generate_config(self) -> Path: platform_dir = self.settings._require_platform_dir() config_path = self.settings.platform_config - template_path = platform_dir / "opentdf.yaml" # Detect platform features to determine supported config options features = PlatformFeatures.detect(platform_dir) @@ -96,14 +102,20 @@ def _generate_config(self) -> Path: # Use stderr if supported, otherwise stdout (v0.9.0 only supports stdout) logger_output = "stderr" if features.supports("logger_stderr") else "stdout" - # Updates for platform config updates = { "logger.level": "debug", "logger.type": "json", "logger.output": logger_output, } - copy_yaml_with_updates(template_path, config_path, updates) + if config_path.exists(): + data = load_yaml(config_path) + for dot_path, value in updates.items(): + set_nested(data, dot_path, value) + save_yaml(config_path, data) + else: + template_path = platform_dir / "opentdf.yaml" + copy_yaml_with_updates(template_path, config_path, updates) # Set up golden keys for legacy TDF tests self._setup_golden_keys(config_path) @@ -118,8 +130,10 @@ def _setup_golden_keys(self, config_path: Path) -> None: """ # In multi-instance mode, golden keys live alongside the instance # config; otherwise they go into the legacy platform_dir. - target_dir = self.settings.keys_dir if self.settings.has_instance() else ( - self.settings._require_platform_dir() + target_dir = ( + self.settings.keys_dir + if self.settings.has_instance() + else (self.settings._require_platform_dir()) ) golden_keys = setup_golden_keys( self.settings.xtest_root, diff --git a/otdf-local/src/otdf_local/utils/keys.py b/otdf-local/src/otdf_local/utils/keys.py index 79b58bf08..5dd5fe5fe 100644 --- a/otdf-local/src/otdf_local/utils/keys.py +++ b/otdf-local/src/otdf_local/utils/keys.py @@ -1,6 +1,7 @@ """Cryptographic key generation utilities.""" import json +import os import secrets import subprocess from pathlib import Path @@ -136,24 +137,213 @@ def generate_ec_keypair(key_dir: Path, name: str = "kas-ec") -> tuple[Path, Path return private_key, public_key +def generate_localhost_cert(key_dir: Path) -> tuple[Path, Path]: + """Generate the TLS cert pair Keycloak mounts at /etc/x509/tls/. + + Mirrors the localhost cert flow in the platform's init-temp-keys.sh: + self-signed CA → CSR with SAN → signed leaf cert. Keycloak rejects a + plain self-signed leaf because it pins the SAN to localhost+127.0.0.1. + """ + key_dir.mkdir(parents=True, exist_ok=True) + ca_key = key_dir / "keycloak-ca-private.pem" + ca_cert = key_dir / "keycloak-ca.pem" + leaf_key = key_dir / "localhost.key" + leaf_csr = key_dir / "localhost.req" + leaf_cert = key_dir / "localhost.crt" + san_conf = key_dir / "sanX509.conf" + req_conf = key_dir / "req.conf" + + san_conf.write_text("subjectAltName=DNS:localhost,IP:127.0.0.1") + req_conf.write_text( + "[req]\n" + "distinguished_name=req_distinguished_name\n" + "[req_distinguished_name]\n" + "[alt_names]\n" + "DNS.1=localhost\n" + "IP.1=127.0.0.1" + ) + + subprocess.run( + [ + "openssl", + "req", + "-x509", + "-nodes", + "-newkey", + "RSA:2048", + "-subj", + "/CN=ca", + "-keyout", + str(ca_key), + "-out", + str(ca_cert), + "-days", + "365", + ], + check=True, + capture_output=True, + ) + ca_key.chmod(0o600) + subprocess.run( + [ + "openssl", + "req", + "-new", + "-nodes", + "-newkey", + "rsa:2048", + "-keyout", + str(leaf_key), + "-out", + str(leaf_csr), + "-batch", + "-subj", + "/CN=localhost", + "-config", + str(req_conf), + ], + check=True, + capture_output=True, + ) + leaf_key.chmod(0o600) + subprocess.run( + [ + "openssl", + "x509", + "-req", + "-in", + str(leaf_csr), + "-CA", + str(ca_cert), + "-CAkey", + str(ca_key), + "-CAcreateserial", + "-out", + str(leaf_cert), + "-days", + "3650", + "-sha256", + "-extfile", + str(san_conf), + ], + check=True, + capture_output=True, + ) + + return leaf_key, leaf_cert + + +def generate_ca_jks(key_dir: Path, password: str = "password") -> Path: + """Convert the keycloak CA into the JKS truststore Keycloak mounts. + + Uses keytool inside the keycloak/keycloak:25.0 image so we don't need a + local JDK — docker is already a hard dependency for the test env. + Requires generate_localhost_cert() to have run first. + """ + ca_key = key_dir / "keycloak-ca-private.pem" + ca_cert = key_dir / "keycloak-ca.pem" + if not ca_key.exists() or not ca_cert.exists(): + raise FileNotFoundError( + f"CA files missing in {key_dir}; call generate_localhost_cert() first" + ) + p12 = key_dir / "ca.p12" + jks = key_dir / "ca.jks" + + subprocess.run( + [ + "openssl", + "pkcs12", + "-export", + "-in", + str(ca_cert), + "-inkey", + str(ca_key), + "-out", + str(p12), + "-nodes", + "-passout", + f"pass:{password}", + ], + check=True, + capture_output=True, + ) + + # keytool -importkeystore via the keycloak image (matches init-temp-keys.sh) + result = subprocess.run( + [ + "docker", + "run", + "--rm", + "-v", + f"{key_dir.resolve()}:/keys", + "--entrypoint", + "keytool", + "--user", + f"{os.getuid()}:{os.getgid()}", + "keycloak/keycloak:25.0", + "-importkeystore", + "-srckeystore", + "/keys/ca.p12", + "-srcstoretype", + "PKCS12", + "-destkeystore", + "/keys/ca.jks", + "-deststoretype", + "JKS", + "-srcstorepass", + password, + "-deststorepass", + password, + "-noprompt", + ], + capture_output=True, + text=True, + ) + if result.returncode != 0: + raise RuntimeError( + f"keytool failed converting PKCS12 → JKS:\n{result.stderr}\n" + "Ensure Docker is running and `keycloak/keycloak:25.0` is pullable." + ) + return jks + + def ensure_keys_exist(key_dir: Path, force: bool = False) -> bool: """Ensure all required keys exist, generating if needed. + Generates the full bootstrap bundle the platform + Keycloak need: + KAS RSA/EC keypairs, the localhost TLS cert pair, and the ca.jks + truststore. PQC keys (ML-KEM, X-Wing) are not generated here — those + are provisioned at test time via the key-management API. + Args: key_dir: Directory for key storage force: If True, regenerate keys even if they exist Returns: - True if keys were generated, False if they already existed + True if any keys were generated, False if everything already existed """ rsa_private = key_dir / "kas-private.pem" ec_private = key_dir / "kas-ec-private.pem" - - if not force and rsa_private.exists() and ec_private.exists(): + localhost_key = key_dir / "localhost.key" + ca_jks = key_dir / "ca.jks" + + if ( + not force + and rsa_private.exists() + and ec_private.exists() + and localhost_key.exists() + and ca_jks.exists() + ): return False - generate_rsa_keypair(key_dir, "kas") - generate_ec_keypair(key_dir, "kas-ec") + if force or not rsa_private.exists(): + generate_rsa_keypair(key_dir, "kas") + if force or not ec_private.exists(): + generate_ec_keypair(key_dir, "kas-ec") + if force or not localhost_key.exists(): + generate_localhost_cert(key_dir) + if force or not ca_jks.exists(): + generate_ca_jks(key_dir) return True From ecb04e1597f5ba80b43e819d6f8a2ef8c517cd40 Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Tue, 2 Jun 2026 09:25:52 -0400 Subject: [PATCH 17/18] fix(otdf-local): translate scenario suite to pytest argv per actual schema `_build_pytest_args` read `suite.select` and treated `suite.containers` as a string, but the Pydantic Suite model exposes `targets: list[str]` and `containers: list[ContainerKind]`. Any user invoking `otdf-local scenario run` hit AttributeError. Also wires `suite.kexpr` through as `-k`; it was silently dropped. Adds unit tests covering empty/multi targets, container join, kexpr, markers + extra args, and SDK token forwarding. Co-Authored-By: Claude Sonnet 4.5 --- otdf-local/src/otdf_local/cli_scenario.py | 10 +- otdf-local/tests/test_cli_scenario.py | 115 ++++++++++++++++++++++ 2 files changed, 122 insertions(+), 3 deletions(-) create mode 100644 otdf-local/tests/test_cli_scenario.py diff --git a/otdf-local/src/otdf_local/cli_scenario.py b/otdf-local/src/otdf_local/cli_scenario.py index 7d1dfde30..08e8def65 100644 --- a/otdf-local/src/otdf_local/cli_scenario.py +++ b/otdf-local/src/otdf_local/cli_scenario.py @@ -34,7 +34,7 @@ def _build_pytest_args(scenario: Scenario, scenario_path: Path) -> list[str]: helper raises FileNotFoundError with a clean hint otherwise. """ suite = scenario.suite - args: list[str] = [suite.select] + args: list[str] = list(suite.targets) tokens = scenario_to_pytest_sdks(scenario, installed_json_for(scenario_path)) if tokens["encrypt"]: @@ -42,7 +42,9 @@ def _build_pytest_args(scenario: Scenario, scenario_path: Path) -> list[str]: if tokens["decrypt"]: args.extend(["--sdks-decrypt", " ".join(tokens["decrypt"])]) if suite.containers: - args.extend(["--containers", suite.containers]) + args.extend(["--containers", " ".join(suite.containers)]) + if suite.kexpr: + args.extend(["-k", suite.kexpr]) if suite.markers: args.extend(["-m", suite.markers]) args.extend(suite.extra_args) @@ -72,7 +74,9 @@ def run( scenario = load_scenario(path) instance_name = instance or scenario.instance.metadata.name if not instance_name: - typer.echo("Error: scenario.instance.metadata.name not set; pass --instance", err=True) + typer.echo( + "Error: scenario.instance.metadata.name not set; pass --instance", err=True + ) raise typer.Exit(2) settings = get_settings() diff --git a/otdf-local/tests/test_cli_scenario.py b/otdf-local/tests/test_cli_scenario.py new file mode 100644 index 000000000..628f7f0a1 --- /dev/null +++ b/otdf-local/tests/test_cli_scenario.py @@ -0,0 +1,115 @@ +"""Tests for `_build_pytest_args` — the scenario-suite → pytest argv translator.""" + +from __future__ import annotations + +from pathlib import Path + +import pytest +from otdf_sdk_mgr.schema import ( + Instance, + Metadata, + PlatformPin, + Scenario, + ScenarioSdk, + ScenarioSdks, + Suite, +) + +from otdf_local import cli_scenario + + +def _scenario(suite: Suite, sdks: ScenarioSdks | None = None) -> Scenario: + return Scenario( + metadata=Metadata(name="t"), + instance=Instance( + metadata=Metadata(name="t"), + platform=PlatformPin(dist="v0.9.0"), + ), + sdks=sdks or ScenarioSdks(), + suite=suite, + ) + + +@pytest.fixture +def stub_sdks(monkeypatch: pytest.MonkeyPatch) -> None: + """Bypass the installed.json round-trip; tests focus on the suite block.""" + monkeypatch.setattr( + cli_scenario, + "scenario_to_pytest_sdks", + lambda _s, _p: {"encrypt": [], "decrypt": []}, + ) + + +def test_empty_targets(stub_sdks: None) -> None: + args = cli_scenario._build_pytest_args(_scenario(Suite(targets=[])), Path("s.yaml")) + assert args == [] + + +def test_multi_target(stub_sdks: None) -> None: + args = cli_scenario._build_pytest_args( + _scenario(Suite(targets=["test_a.py", "test_b.py::test_x"])), Path("s.yaml") + ) + assert args == ["test_a.py", "test_b.py::test_x"] + + +def test_containers_joined(stub_sdks: None) -> None: + args = cli_scenario._build_pytest_args( + _scenario(Suite(targets=["test_pqc.py"], containers=["ztdf", "ztdf-ecwrap"])), + Path("s.yaml"), + ) + assert args == ["test_pqc.py", "--containers", "ztdf ztdf-ecwrap"] + + +def test_no_containers_omits_flag(stub_sdks: None) -> None: + args = cli_scenario._build_pytest_args( + _scenario(Suite(targets=["t.py"], containers=[])), Path("s.yaml") + ) + assert "--containers" not in args + + +def test_kexpr_forwarded(stub_sdks: None) -> None: + args = cli_scenario._build_pytest_args( + _scenario(Suite(targets=["t.py"], kexpr="not slow")), Path("s.yaml") + ) + assert args == ["t.py", "-k", "not slow"] + + +def test_markers_and_extra_args(stub_sdks: None) -> None: + args = cli_scenario._build_pytest_args( + _scenario(Suite(targets=["t.py"], markers="smoke", extra_args=["-vv", "-x"])), + Path("s.yaml"), + ) + assert args == ["t.py", "-m", "smoke", "-vv", "-x"] + + +def test_sdks_tokens_forwarded( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr( + cli_scenario, + "scenario_to_pytest_sdks", + lambda _s, _p: { + "encrypt": ["go@v0.24.0"], + "decrypt": ["go@v0.24.0", "java@v0.10.0"], + }, + ) + args = cli_scenario._build_pytest_args( + _scenario( + Suite(targets=["t.py"]), + sdks=ScenarioSdks( + encrypt=[ScenarioSdk(sdk="go", version="v0.24.0")], + decrypt=[ + ScenarioSdk(sdk="go", version="v0.24.0"), + ScenarioSdk(sdk="java", version="v0.10.0"), + ], + ), + ), + Path("s.yaml"), + ) + assert args == [ + "t.py", + "--sdks-encrypt", + "go@v0.24.0", + "--sdks-decrypt", + "go@v0.24.0 java@v0.10.0", + ] From 0b41c21ec76fea898dc82d2cc8d0ef1ce3fa1bf3 Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Tue, 2 Jun 2026 09:26:32 -0400 Subject: [PATCH 18/18] style(otdf-local): apply ruff format Co-Authored-By: Claude Sonnet 4.5 --- otdf-local/src/otdf_local/services/kas.py | 4 +++- otdf-local/tests/test_multi_instance.py | 8 ++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/otdf-local/src/otdf_local/services/kas.py b/otdf-local/src/otdf_local/services/kas.py index d06350e77..5501af002 100644 --- a/otdf-local/src/otdf_local/services/kas.py +++ b/otdf-local/src/otdf_local/services/kas.py @@ -111,7 +111,9 @@ def _generate_config(self) -> Path: # Per-KAS features from instance.yaml override the legacy heuristic. instance = self.settings.load_instance() kas_pin = instance.kas.get(self._kas_name) if instance is not None else None - extra_features: dict[str, bool] = dict(kas_pin.features) if kas_pin is not None else {} + extra_features: dict[str, bool] = ( + dict(kas_pin.features) if kas_pin is not None else {} + ) if self.is_key_management: updates["services.kas.preview.key_management"] = True diff --git a/otdf-local/tests/test_multi_instance.py b/otdf-local/tests/test_multi_instance.py index e290d7731..8c3b44908 100644 --- a/otdf-local/tests/test_multi_instance.py +++ b/otdf-local/tests/test_multi_instance.py @@ -72,7 +72,11 @@ def test_settings_loads_instance_when_present(tmp_path: Path) -> None: assert s.keys_dir == instance_dir / "keys" -def test_platform_binary_for_resolves_under_xtest_platform(monkeypatch: pytest.MonkeyPatch) -> None: +def test_platform_binary_for_resolves_under_xtest_platform( + monkeypatch: pytest.MonkeyPatch, +) -> None: monkeypatch.setenv("OTDF_PLATFORM_DIR", "/tmp/fake-platform") s = Settings() - assert s.platform_binary_for("v0.9.0") == Path("/tmp/fake-platform/dist/v0.9.0/service") + assert s.platform_binary_for("v0.9.0") == Path( + "/tmp/fake-platform/dist/v0.9.0/service" + )