diff --git a/.github/scripts/assert_wheel_contents.sh b/.github/scripts/assert_wheel_contents.sh index 0751376d..f8118094 100755 --- a/.github/scripts/assert_wheel_contents.sh +++ b/.github/scripts/assert_wheel_contents.sh @@ -22,6 +22,7 @@ listing=$(unzip -l "$wheel") required=( 'holoscan_cli/logging\.json$' 'holoscan_cli/py\.typed$' + 'holoscan_cli/command_plan\.schema\.json$' 'holoscan_cli/metadata/.+\.schema\.json$' 'holoscan_cli/setup_scripts/.+' 'holoscan_cli/setup_scripts/requirements\.template\.txt$' diff --git a/README.md b/README.md index ab539672..eda32b26 100644 --- a/README.md +++ b/README.md @@ -25,6 +25,40 @@ Per-repo wrappers install this package and delegate to `holoscan`, layering on t Common env vars: `HOLOSCAN_CLI_ROOT` (repo root), `HOLOSCAN_CLI_SEARCH_PATH` (subdirs to scan for `metadata.json`), `HOLOSCAN_CLI_PATH_PREFIX` (placeholder prefix in metadata templates), `HOLOSCAN_CLI_REPO_PREFIX` (container image name prefix). The legacy `HOLOHUB_*` spelling is no longer honored since holoscan v4.3.0 — set the `HOLOSCAN_CLI_*` names directly. `holoscan env-info` lists every env var the CLI reads in the current shell. +### Structured dry-run plans + +The core `holoscan build-container` command can resolve its host-dependent +Docker command without running the build and return either structured JSON or +one editable Bash script: + +```bash +holoscan build-container my_app --dryrun --json +holoscan build-container my_app --dryrun --shell > build-container.sh +``` + +The JSON `steps` include read-only host probes as well as the ordered action +commands. The Bash replay contains the resolved action commands; review it +before execution. Both formats reflect the current host and inherited +environment rather than forming a portable or hermetic build description. + +The JSON scope is `current_cli_process`. Bootstrap or provisioning performed +by a repository wrapper before it starts `holoscan` is not included. Use the +installed `holoscan` entry point for machine-readable output until a given +wrapper documents that it preserves structured stdout; wrapper diagnostics +must go to stderr. + +To override an input, set it on the planning invocation and regenerate the +plan, or prefer the equivalent typed CLI option when one exists: + +```bash +env HOLOSCAN_CLI_BASE_SDK_VERSION=4.5.0 \ + holoscan build-container my_app --dryrun --json +``` + +Plain `--dryrun` retains its human-readable output. Structured planning is +being enabled one command at a time; unsupported action commands reject these +formats instead of returning a partial plan. + ## Source layout ```text diff --git a/pyproject.toml b/pyproject.toml index 023834c6..cd543ff5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -76,6 +76,7 @@ Documentation = "https://docs.nvidia.com/holoscan/sdk-user-guide/index.html" requires-poetry = ">=2.0" packages = [{ include = "holoscan_cli", from = "src" }] include = [ + { path = "src/holoscan_cli/command_plan.schema.json", format = ["sdist", "wheel"] }, { path = "src/holoscan_cli/metadata/*.schema.json", format = ["sdist", "wheel"] }, { path = "src/holoscan_cli/setup_scripts/*", format = ["sdist", "wheel"] }, { path = "src/holoscan_cli/testing/**/*", format = ["sdist", "wheel"] }, diff --git a/src/holoscan_cli/cli.py b/src/holoscan_cli/cli.py index 7dc0373a..54460d6c 100755 --- a/src/holoscan_cli/cli.py +++ b/src/holoscan_cli/cli.py @@ -33,10 +33,13 @@ import argparse import functools import os +from contextlib import redirect_stdout +from io import StringIO from pathlib import Path from typing import List, Optional import holoscan_cli.metadata.gather_metadata as metadata_util +from holoscan_cli.command_plan import CommandPlanError, PlanRecorder from holoscan_cli.commands import registry as commands_registry from holoscan_cli.container import HoloscanContainer from holoscan_cli.container.parsers import get_build_argparse, get_run_argparse @@ -300,47 +303,26 @@ def get_effective_build_config( if "depends" in build_config: if config["with_operators"]: - mode_deps = [dep.strip() for dep in build_config["depends"] if dep.strip()] - msg = f"CLI args --build-with='{config['with_operators']}' " - msg += f"overrides mode depends: {', '.join(mode_deps)}" - warn(msg) + warn("CLI --build-with overrides mode build.depends") else: mode_deps = [dep.strip() for dep in build_config["depends"] if dep.strip()] config["with_operators"] = ";".join(mode_deps) if mode_deps else "" if "docker_build_args" in build_config: if config["build_args"]: - mode_args = normalize_args_str(build_config["docker_build_args"]) - msg = f"CLI args --build-args='{config['build_args']}' " - msg += f"overrides mode --build-args: {mode_args}" - warn(msg) + warn("CLI --build-args overrides mode build.docker_build_args") else: config["build_args"] = normalize_args_str(build_config["docker_build_args"]) if "cmake_options" in build_config: if config["configure_args"]: - mode_opts = ( - " ".join(build_config["cmake_options"]) - if isinstance(build_config["cmake_options"], list) - else build_config["cmake_options"] - ) - cli_opts = ( - " ".join(config["configure_args"]) - if isinstance(config["configure_args"], list) - else config["configure_args"] - ) - msg = f"CLI args --configure-args='{cli_opts}' " - msg += f"overrides mode --configure-args: {mode_opts}" - warn(msg) + warn("CLI --configure-args overrides mode build.cmake_options") else: config["configure_args"] = build_config["cmake_options"] if "run" in mode_config and "docker_run_args" in mode_config["run"]: if getattr(args, "docker_opts", ""): - mode_opts = normalize_args_str(mode_config["run"]["docker_run_args"]) - msg = f"CLI args --docker-opts='{getattr(args, 'docker_opts', '')}' " - msg += f"overrides mode --docker-opts: {mode_opts}" - warn(msg) + warn("CLI --docker-opts overrides mode run.docker_run_args") else: config["docker_opts"] = normalize_args_str(mode_config["run"]["docker_run_args"]) @@ -366,20 +348,11 @@ def get_effective_run_config( config["workdir"] = run_config["workdir"] if "command" in run_config and getattr(args, "run_args", ""): - msg = ( - f"CLI args --run-args='{getattr(args, 'run_args', '')}' " - f"will be appended to mode command" - ) - warn(msg) + warn("CLI --run-args will be appended to mode run.command") if "docker_run_args" in run_config: if getattr(args, "docker_opts", ""): - mode_opts = normalize_args_str(run_config["docker_run_args"]) - msg = ( - f"CLI args --docker-opts='{getattr(args, 'docker_opts', '')}' " - f"overrides mode --docker-opts: {mode_opts}" - ) - warn(msg) + warn("CLI --docker-opts overrides mode run.docker_run_args") else: config["docker_opts"] = normalize_args_str(run_config["docker_run_args"]) return config @@ -471,12 +444,45 @@ def run(self, argv: Optional[List[str]] = None) -> None: print(file=sys.stderr) sys.exit(1) raise - if hasattr(args, "func"): + plan_format = getattr(args, "plan_format", None) + if plan_format is not None: + if not getattr(args, "_supports_plan", False): + self.subparsers[args.command].error( + "structured command plans are not supported for this command" + ) + if not getattr(args, "dryrun", False): + self.subparsers[args.command].error(f"--{plan_format} requires --dryrun") + self._run_structured_plan(args, plan_format) + elif hasattr(args, "func"): args.func(args) else: self.parser.print_help() sys.exit(1) + @staticmethod + def _run_structured_plan(args: argparse.Namespace, plan_format: str) -> None: + """Run one audited dry-run and write its artifact atomically to stdout.""" + + recorder = PlanRecorder() + legacy_stdout = StringIO() + try: + try: + with recorder.activate(), redirect_stdout(legacy_stdout): + args.func(args) + finally: + notices = legacy_stdout.getvalue() + if notices: + sys.stderr.write(notices) + + output = recorder.json_text() if plan_format == "json" else recorder.shell_text() + except CommandPlanError as exc: + print(f"Command planning failed: {exc}", file=sys.stderr) + raise SystemExit(1) from exc + + if plan_format == "json": + output += "\n" + sys.stdout.write(output) + def main(argv: Optional[List[str]] = None): script_name = None diff --git a/src/holoscan_cli/command_plan.py b/src/holoscan_cli/command_plan.py new file mode 100644 index 00000000..bbaa6c17 --- /dev/null +++ b/src/holoscan_cli/command_plan.py @@ -0,0 +1,441 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Invocation-scoped structured command plans. + +The first public slice records process steps for ``build-container``. The +recorder is deliberately private and active only for ``--dryrun --json`` or +``--dryrun --shell`` so the normal execution path and human dry-run output stay +unchanged. +""" + +from __future__ import annotations + +import argparse +import json +import os +import re +import shlex +from contextlib import contextmanager +from contextvars import ContextVar +from dataclasses import dataclass, field +from pathlib import Path +from typing import Iterator, Mapping, Optional, Sequence + + +class CommandPlanError(RuntimeError): + """Raised when an invocation cannot produce a complete v1 plan.""" + + +_ACTIVE_RECORDER: ContextVar[Optional["PlanRecorder"]] = ContextVar( + "holoscan_cli_command_plan", default=None +) + +_ENV_NAME = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$") +_SENSITIVE_EXACT_NAMES = { + "API_KEY", + "AWS_ACCESS_KEY_ID", + "AWS_SECRET_ACCESS_KEY", + "NGC_API_KEY", + "NGC_CLI_API_KEY", + "PASSWORD", + "SECRET", + "TOKEN", +} +_SENSITIVE_SUFFIXES = ("_API_KEY", "_PASSWORD", "_SECRET", "_TOKEN") +_DOCKER_VALUE_OPTIONS = {"-e", "--env", "--build-arg"} + + +def add_plan_output_arguments(parser: argparse.ArgumentParser) -> None: + """Add the command-plan output formats to one audited action parser.""" + + output = parser.add_mutually_exclusive_group() + output.add_argument( + "--json", + dest="plan_format", + action="store_const", + const="json", + help="With --dryrun, print a machine-readable command plan", + ) + output.add_argument( + "--shell", + dest="plan_format", + action="store_const", + const="shell", + help="With --dryrun, print a copyable Bash command plan", + ) + parser.set_defaults(plan_format=None) + + +def get_active_recorder() -> Optional["PlanRecorder"]: + """Return the recorder for the current invocation, if any.""" + + return _ACTIVE_RECORDER.get() + + +def command_plan_active() -> bool: + """Return whether structured planning is active in this context.""" + + return get_active_recorder() is not None + + +def record_probe_fallback(message: str) -> None: + """Attach a warning when the real resolver uses a documented fallback.""" + + recorder = get_active_recorder() + if recorder is not None: + recorder.add_warning("probe_fallback_used", message) + + +def _normalize_env_name(name: str) -> str: + return re.sub(r"[^A-Za-z0-9]+", "_", name).strip("_").upper() + + +def _is_sensitive_env_name(name: str) -> bool: + normalized = _normalize_env_name(name) + return normalized in _SENSITIVE_EXACT_NAMES or normalized.endswith(_SENSITIVE_SUFFIXES) + + +def _redact_assignment(assignment: str) -> tuple[str, bool, Optional[str]]: + """Return public assignment, redaction state, and named-env requirement.""" + + if "=" not in assignment: + name = assignment + required = name if _ENV_NAME.fullmatch(name) else None + return assignment, False, required + + name, _value = assignment.split("=", 1) + if not _is_sensitive_env_name(name): + return assignment, False, None + return f"{name}=", True, None + + +def _public_argv(argv: Sequence[str]) -> tuple[list[str], bool, list[str]]: + """Redact recognized literal credentials in Docker option assignments.""" + + public = [str(token) for token in argv] + redacted = False + required: set[str] = set() + index = 0 + + while index < len(public): + token = public[index] + if token in _DOCKER_VALUE_OPTIONS and index + 1 < len(public): + replacement, changed, required_name = _redact_assignment(public[index + 1]) + public[index + 1] = replacement + redacted = redacted or changed + if required_name: + required.add(required_name) + index += 2 + continue + + for prefix in ("--env=", "-e=", "--build-arg="): + if token.startswith(prefix): + replacement, changed, required_name = _redact_assignment(token[len(prefix) :]) + public[index] = f"{prefix}{replacement}" + redacted = redacted or changed + if required_name: + required.add(required_name) + break + index += 1 + + return public, redacted, sorted(required) + + +def _environment_delta( + baseline: Mapping[str, str], + effective: Mapping[str, str], + explicit_set: Optional[Mapping[str, str]] = None, +) -> tuple[dict[str, str], list[str], bool]: + changed: dict[str, str] = {} + redacted = False + explicit_names = set(explicit_set or {}) + for name in sorted(effective): + value = str(effective[name]) + if baseline.get(name) == value and name not in explicit_names: + continue + if _is_sensitive_env_name(name): + changed[name] = "" + redacted = True + else: + changed[name] = value + unset = sorted(name for name in baseline if name not in effective) + return changed, unset, redacted + + +def _shell_argv_groups(argv: Sequence[str]) -> list[list[str]]: + """Group argv tokens into readable lines without changing shell semantics.""" + + tokens = [str(token) for token in argv] + if not tokens: + return [] + + head_length = 2 if len(tokens) > 1 and not tokens[1].startswith("-") else 1 + return [tokens[:head_length], *[[token] for token in tokens[head_length:]]] + + +def _shell_group_lines( + groups: Sequence[Sequence[str]], *, first_indent: str, continuation_indent: str +) -> list[str]: + """Quote grouped argv and add Bash line continuations.""" + + lines = [] + for index, group in enumerate(groups): + indent = first_indent if index == 0 else continuation_indent + suffix = " \\" if index + 1 < len(groups) else "" + lines.append(f"{indent}{shlex.join([str(token) for token in group])}{suffix}") + return lines + + +def _process_shell( + argv: Sequence[str], + cwd: str, + environment_set: Mapping[str, str], + environment_unset: Sequence[str], +) -> str: + command_groups = _shell_argv_groups(argv) + lines = ["(", f" {shlex.join(['cd', '--', cwd])} && \\"] + if environment_set or environment_unset: + lines.append(" env \\") + env_groups = [["-u", name] for name in environment_unset] + env_groups.extend([[f"{name}={value}"] for name, value in environment_set.items()]) + for group in env_groups: + lines.append(f" {shlex.join(group)} \\") + lines.extend( + _shell_group_lines( + command_groups, + first_indent=" ", + continuation_indent=" ", + ) + ) + else: + lines.extend( + _shell_group_lines( + command_groups, + first_indent=" ", + continuation_indent=" ", + ) + ) + lines.append(")") + return "\n".join(lines) + + +@dataclass +class ProcessStep: + """A process invocation plus its private parity data.""" + + id: str + role: str + argv: list[str] + private_argv: list[str] = field(repr=False) + shell: str + cwd: str + environment: dict + check: bool + privilege: str + redacted: bool + destructive: bool = False + + def public_dict(self) -> dict: + return { + "id": self.id, + "kind": "process", + "role": self.role, + "argv": self.argv, + "shell": self.shell, + "cwd": self.cwd, + "environment": self.environment, + "check": self.check, + "privilege": self.privilege, + "redacted": self.redacted, + "destructive": self.destructive, + } + + +class PlanRecorder: + """Record a complete, ordered command plan for one CLI invocation.""" + + def __init__(self) -> None: + self._environment = dict(os.environ) + self.steps: list[ProcessStep] = [] + self.limitations: list[dict] = [] + self.warnings: list[dict] = [] + + @contextmanager + def activate(self) -> Iterator["PlanRecorder"]: + if get_active_recorder() is not None: + raise CommandPlanError("nested command-plan recorders are not supported") + token = _ACTIVE_RECORDER.set(self) + try: + yield self + finally: + _ACTIVE_RECORDER.reset(token) + + def add_warning(self, code: str, message: str, step_id: Optional[str] = None) -> None: + warning = {"code": code, "message": message} + if step_id is not None: + warning["step_id"] = step_id + if warning not in self.warnings: + self.warnings.append(warning) + + def record_process( + self, + argv: Sequence[str], + *, + role: str, + cwd: Optional[os.PathLike[str] | str] = None, + env: Optional[Mapping[str, str]] = None, + explicit_env: Optional[Mapping[str, str]] = None, + check: bool, + privilege: str = "user", + destructive: bool = False, + ) -> ProcessStep: + if role not in {"probe", "action", "cleanup"}: + raise CommandPlanError(f"unsupported process role: {role}") + + private_argv = [str(token) for token in argv] + if not private_argv: + raise CommandPlanError("process argv must contain at least one token") + public_argv, argv_redacted, required = _public_argv(private_argv) + if env is not None: + if not explicit_env: + raise CommandPlanError( + "replacement subprocess environments are not supported in v1 plans" + ) + expected_env = dict(self._environment) + expected_env.update({str(name): str(value) for name, value in explicit_env.items()}) + if dict(env) != expected_env: + raise CommandPlanError( + "replacement subprocess environments are not supported in v1 plans" + ) + effective_env = os.environ if env is None else env + missing_required = [name for name in required if name not in effective_env] + if missing_required: + names = ", ".join(missing_required) + raise CommandPlanError( + f"Docker environment reference is unset during planning: {names}" + ) + environment_set, environment_unset, env_redacted = _environment_delta( + self._environment, effective_env, explicit_env + ) + external_required = [name for name in required if name not in environment_set] + resolved_cwd = str(Path.cwd() if cwd is None else Path(cwd).resolve()) + step_id = f"step-{len(self.steps) + 1:03d}" + redacted = argv_redacted or env_redacted + environment = { + "inherit": True, + "set": environment_set, + "unset": environment_unset, + "required": external_required, + } + step = ProcessStep( + id=step_id, + role=role, + argv=public_argv, + private_argv=private_argv, + shell=_process_shell(public_argv, resolved_cwd, environment_set, environment_unset), + cwd=resolved_cwd, + environment=environment, + check=check, + privilege=privilege, + redacted=redacted, + destructive=destructive, + ) + self.steps.append(step) + if redacted: + self.add_warning( + "redacted_value", + "A credential-like literal was redacted from the public command plan.", + step_id, + ) + return step + + def _replay(self) -> dict: + self._ensure_complete() + replay_steps = [step for step in self.steps if step.role != "probe"] + required = sorted( + {name for step in replay_steps for name in step.environment.get("required", [])} + ) + + unavailable_reason = None + if any(step.redacted for step in replay_steps): + unavailable_reason = "redacted_literal" + elif any(not step.check for step in replay_steps): + unavailable_reason = "nonfatal_exit_handling" + + if unavailable_reason is not None: + return { + "format": "bash", + "script": None, + "required_environment": required, + "unavailable_reason": unavailable_reason, + } + + lines = ["#!/usr/bin/env bash", "set -e", ""] + for name in required: + lines.append(f': "${{{name}?Set {name} before running this script}}"') + if required: + lines.append("") + for index, step in enumerate(replay_steps): + summary = shlex.join(step.argv[:2]).replace("\r", "\\r").replace("\n", "\\n") + lines.append(f"# {step.id} ({step.role}): {summary}") + lines.append(step.shell) + if index + 1 < len(replay_steps): + lines.append("") + return { + "format": "bash", + "script": "\n".join(lines) + "\n", + "required_environment": required, + "unavailable_reason": None, + } + + def _ensure_complete(self) -> None: + if not any(step.role in {"action", "cleanup"} for step in self.steps): + raise CommandPlanError("the resolved branch contains no action steps") + + def payload(self) -> dict: + replay = self._replay() + warnings = list(self.warnings) + if replay["script"] is None: + warnings.append( + { + "code": "shell_replay_unavailable", + "message": f"Bash replay unavailable: {replay['unavailable_reason']}", + } + ) + return { + "schema_version": 1, + "status": "dryrun", + "scope": "current_cli_process", + "host_resolved": True, + "complete": True, + "steps": [step.public_dict() for step in self.steps], + "replay": replay, + "limitations": list(self.limitations), + "warnings": warnings, + } + + def json_text(self) -> str: + """Render the complete JSON artifact without writing stdout.""" + + return json.dumps(self.payload(), indent=2) + + def shell_text(self) -> str: + """Render Bash or fail before anything is written to stdout.""" + + replay = self._replay() + if replay["script"] is None: + raise CommandPlanError(f"Bash replay unavailable: {replay['unavailable_reason']}") + return str(replay["script"]) diff --git a/src/holoscan_cli/command_plan.schema.json b/src/holoscan_cli/command_plan.schema.json new file mode 100644 index 00000000..5f36641d --- /dev/null +++ b/src/holoscan_cli/command_plan.schema.json @@ -0,0 +1,136 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://github.com/nvidia-holoscan/holoscan-cli/command_plan.schema.json", + "title": "Holoscan CLI dry-run command plan", + "type": "object", + "required": [ + "schema_version", + "status", + "scope", + "host_resolved", + "complete", + "steps", + "replay", + "limitations", + "warnings" + ], + "properties": { + "schema_version": {"const": 1}, + "status": {"const": "dryrun"}, + "scope": {"const": "current_cli_process"}, + "host_resolved": {"const": true}, + "complete": {"const": true}, + "steps": { + "type": "array", + "minItems": 1, + "items": {"$ref": "#/$defs/process_step"} + }, + "replay": {"$ref": "#/$defs/replay"}, + "limitations": { + "type": "array", + "items": {"$ref": "#/$defs/notice"} + }, + "warnings": { + "type": "array", + "items": {"$ref": "#/$defs/notice"} + } + }, + "additionalProperties": true, + "$defs": { + "environment": { + "type": "object", + "required": ["inherit", "set", "unset", "required"], + "properties": { + "inherit": {"const": true}, + "set": { + "type": "object", + "additionalProperties": {"type": "string"} + }, + "unset": { + "type": "array", + "items": {"type": "string"}, + "uniqueItems": true + }, + "required": { + "type": "array", + "items": {"type": "string"}, + "uniqueItems": true + } + }, + "additionalProperties": true + }, + "process_step": { + "type": "object", + "required": [ + "id", + "kind", + "role", + "argv", + "shell", + "cwd", + "environment", + "check", + "privilege", + "redacted", + "destructive" + ], + "properties": { + "id": {"type": "string", "pattern": "^step-[0-9]{3,}$"}, + "kind": {"const": "process"}, + "role": {"enum": ["probe", "action", "cleanup"]}, + "argv": { + "type": "array", + "minItems": 1, + "items": {"type": "string"} + }, + "shell": {"type": "string"}, + "cwd": {"type": "string"}, + "environment": {"$ref": "#/$defs/environment"}, + "check": {"type": "boolean"}, + "privilege": {"enum": ["user", "system"]}, + "redacted": {"type": "boolean"}, + "destructive": {"type": "boolean"} + }, + "additionalProperties": true + }, + "replay": { + "type": "object", + "required": ["format", "script", "required_environment", "unavailable_reason"], + "properties": { + "format": {"const": "bash"}, + "script": {"type": ["string", "null"]}, + "required_environment": { + "type": "array", + "items": {"type": "string"}, + "uniqueItems": true + }, + "unavailable_reason": {"type": ["string", "null"]} + }, + "oneOf": [ + { + "properties": { + "script": {"type": "string", "minLength": 1}, + "unavailable_reason": {"type": "null"} + } + }, + { + "properties": { + "script": {"type": "null"}, + "unavailable_reason": {"type": "string", "minLength": 1} + } + } + ], + "additionalProperties": true + }, + "notice": { + "type": "object", + "required": ["code", "message"], + "properties": { + "code": {"type": "string"}, + "message": {"type": "string"}, + "step_id": {"type": "string"} + }, + "additionalProperties": true + } + } +} diff --git a/src/holoscan_cli/commands/registry.py b/src/holoscan_cli/commands/registry.py index 66e09f86..68a783f5 100644 --- a/src/holoscan_cli/commands/registry.py +++ b/src/holoscan_cli/commands/registry.py @@ -63,6 +63,7 @@ class CommandSpec: short_help: str help: str group: str # "project" | "container" | "info" | "workspace" + supports_plan: bool = False # Ordered for predictable iteration; ``holoscan --help`` re-sorts alphabetically. @@ -98,6 +99,7 @@ class CommandSpec: short_help="build a source-project development container", help="Build the development container", group="container", + supports_plan=True, ), CommandSpec( "run-container", @@ -239,6 +241,7 @@ def register_all( # Imported lazily so simple ``from holoscan_cli.commands import registry`` # consumers (e.g. ``__main__.py``) don't pull in every command module # just to read :data:`PROJECT_COMMANDS`. + from holoscan_cli.command_plan import add_plan_output_arguments from holoscan_cli.commands import ( build, clear_cache, @@ -256,7 +259,12 @@ def register_all( registered: dict[str, argparse.ArgumentParser] = {} def add(register_fn, name: str, **kwargs) -> None: - registered[name] = register_fn(cli, subparsers, **kwargs) + parser = register_fn(cli, subparsers, **kwargs) + supports_plan = PROJECT_COMMANDS_BY_NAME[name].supports_plan + if supports_plan: + add_plan_output_arguments(parser) + parser.set_defaults(_supports_plan=supports_plan) + registered[name] = parser # Workspace-touching commands. add(create.register_create_parser, "create") diff --git a/src/holoscan_cli/container/core.py b/src/holoscan_cli/container/core.py index c7f0d52e..e66490ec 100644 --- a/src/holoscan_cli/container/core.py +++ b/src/holoscan_cli/container/core.py @@ -27,6 +27,7 @@ from pathlib import Path from typing import Any, List, Optional, Union +from holoscan_cli.command_plan import command_plan_active from holoscan_cli.metadata.utils import list_normalized_languages from ..utils.docker import get_image_pythonpath @@ -40,7 +41,7 @@ get_sccache_dir, replace_placeholders, ) -from ..utils.io import fatal, info, run_command, warn +from ..utils.io import fatal, info, run_command, run_probe, warn from ..utils.sdk import ( check_nvidia_ctk, find_hsdk_build_rel_dir, @@ -447,18 +448,24 @@ def build( self.cuda_version if self.cuda_version is not None else get_default_cuda_version() ) - # Check if buildx exists - if not self.dryrun: + # Structured planning runs this read-only probe because the real action + # requires buildx. Plain human dry-run keeps its historical no-probe behavior. + if not self.dryrun or command_plan_active(): try: - run_command([self.DOCKER_EXE, "buildx", "version"], check=True, capture_output=True) - except subprocess.CalledProcessError: + run_probe( + [self.DOCKER_EXE, "buildx", "version"], + check=True, + echo=True, + capture_output=True, + text=True, + ) + except (subprocess.CalledProcessError, OSError): fatal( "docker buildx plugin is missing. Please install docker-buildx-plugin:\n" "https://docs.docker.com/engine/install/ubuntu/#install-using-the-repository" ) - # Set DOCKER_BUILDKIT environment variable - os.environ["DOCKER_BUILDKIT"] = "1" + build_env = {"DOCKER_BUILDKIT": "1"} cmd = [ self.DOCKER_EXE, @@ -498,7 +505,7 @@ def build( cmd.extend(["-t", f"{tag_name}-base"]) cmd.append(str(HoloscanContainer.HOLOHUB_ROOT)) - run_command(cmd, dry_run=self.dryrun) + run_command(cmd, dry_run=self.dryrun, env_updates=build_env) if extra_scripts: setup_scripts_dir = get_holohub_setup_scripts_dir() @@ -529,7 +536,7 @@ def build( for tag_name in tags: # We override the default tag so we can add the next scripts on top of this. cmd.extend(["-t", f"{tag_name}-{script}", "-t", f"{tag_name}"]) - run_command(cmd, dry_run=self.dryrun) + run_command(cmd, dry_run=self.dryrun, env_updates=build_env) def run( self, diff --git a/src/holoscan_cli/utils/holohub.py b/src/holoscan_cli/utils/holohub.py index 3575f239..c49106cb 100644 --- a/src/holoscan_cli/utils/holohub.py +++ b/src/holoscan_cli/utils/holohub.py @@ -33,6 +33,7 @@ from pathlib import Path from typing import Optional, Tuple +from holoscan_cli.command_plan import record_probe_fallback from holoscan_cli.utils.io import format_cmd, info, run_info_command, warn from holoscan_cli.utils.text import _slugify, get_env_bool @@ -365,10 +366,14 @@ def get_git_short_sha(length: int = 12) -> str: sha = run_info_command( ["git", "rev-parse", f"--short={length}", "HEAD"], cwd=str(HOLOHUB_ROOT) ) - return sha or DEFAULT_GIT_REF + if sha: + return sha except Exception: warn(f"Failed to get current git sha, defaulting to {DEFAULT_GIT_REF}") - return DEFAULT_GIT_REF + record_probe_fallback( + f"Git SHA detection failed; using the image-tag fallback '{DEFAULT_GIT_REF}'." + ) + return DEFAULT_GIT_REF def get_current_branch_slug() -> str: @@ -381,9 +386,17 @@ def get_current_branch_slug() -> str: branch = run_info_command( ["git", "rev-parse", "--abbrev-ref", "HEAD"], cwd=str(HOLOHUB_ROOT) ) - if not branch or branch in ["HEAD", "(no branch)"] or branch.startswith("(HEAD detached"): - return DEFAULT_GIT_REF - return _slugify(branch) or DEFAULT_GIT_REF + if ( + branch + and branch not in ["HEAD", "(no branch)"] + and not branch.startswith("(HEAD detached") + ): + slug = _slugify(branch) + if slug: + return slug except Exception: warn(f"Failed to get current branch, defaulting to {DEFAULT_GIT_REF}") - return DEFAULT_GIT_REF + record_probe_fallback( + f"Git branch detection failed or was detached; using '{DEFAULT_GIT_REF}'." + ) + return DEFAULT_GIT_REF diff --git a/src/holoscan_cli/utils/io.py b/src/holoscan_cli/utils/io.py index ff058e40..a04e73e2 100644 --- a/src/holoscan_cli/utils/io.py +++ b/src/holoscan_cli/utils/io.py @@ -25,7 +25,9 @@ import traceback from datetime import datetime, timezone from pathlib import Path -from typing import Iterable, List, Optional, Union +from typing import Iterable, List, Mapping, Optional, Union + +from holoscan_cli.command_plan import CommandPlanError, get_active_recorder def resolve(path) -> Path: @@ -208,6 +210,7 @@ def run_command( check: bool = True, as_root: bool = False, preserve_env: Optional[Iterable[str]] = None, + env_updates: Optional[Mapping[str, str]] = None, **kwargs, ) -> subprocess.CompletedProcess: """Run a command, optionally elevated with ``sudo``. @@ -219,11 +222,21 @@ def run_command( re-applied via ``/usr/bin/env`` (sudo and ld.so scrub them); all other names go through ``--preserve-env`` so their values — possibly secrets — stay out of the world-readable /proc//cmdline. Missing sudo fails - clearly rather than running unprivileged. + clearly rather than running unprivileged. ``env_updates`` applies an + invocation-owned overlay without mutating :data:`os.environ`; structured + plans retain that overlay even when it equals the invocation's initial + value. """ if preserve_env is not None and not as_root: raise ValueError("preserve_env requires as_root=True") + explicit_env = {str(name): str(value) for name, value in (env_updates or {}).items()} + if explicit_env: + base_env = kwargs.get("env") + effective_env = dict(os.environ if base_env is None else base_env) + effective_env.update(explicit_env) + kwargs["env"] = effective_env + elevate = as_root and os.geteuid() != 0 sudo_prefix: List[str] = [] sudo_display_prefix: List[str] = [] @@ -275,6 +288,23 @@ def run_command( quoted = [f'"{x}"' if " " in x else x for x in display_argv] display_cmd = format_long_command(quoted) if dry_run else " ".join(quoted) + recorder = get_active_recorder() + if recorder is not None: + if isinstance(exec_cmd, str): + raise CommandPlanError( + "string subprocess commands are not supported in structured plans" + ) + recorder.record_process( + exec_cmd, + role="action", + cwd=kwargs.get("cwd"), + env=kwargs.get("env"), + explicit_env=explicit_env, + check=check, + privilege="system" if as_root else "user", + ) + return subprocess.CompletedProcess(exec_cmd, 0) + if elevate: print(Color.yellow("[system] elevating with sudo")) if dry_run: @@ -292,6 +322,40 @@ def run_command( sys.exit(e.returncode) +def run_probe( + cmd: List[str], + *, + check: bool = False, + echo: bool = False, + **kwargs, +) -> subprocess.CompletedProcess: + """Execute a declared read-only probe, recording it during planning. + + Unlike normal action commands, probes execute while a structured dry-run + is active so host-dependent argv can be resolved. Probe output should be + captured by the caller. + """ + + argv = [str(token) for token in cmd] + recorder = get_active_recorder() + if recorder is not None: + recorder.record_process( + argv, + role="probe", + cwd=kwargs.get("cwd"), + env=kwargs.get("env"), + check=check, + ) + if not kwargs.get("capture_output", False): + if kwargs.get("stdout") is None: + kwargs["stdout"] = subprocess.PIPE + if kwargs.get("stderr") is None: + kwargs["stderr"] = subprocess.PIPE + elif echo: + print(format_cmd(shlex.join(argv))) + return subprocess.run(argv, check=check, **kwargs) + + def write_system_file( path: Union[str, "os.PathLike[str]"], content: Union[str, bytes], @@ -317,6 +381,16 @@ def write_system_file( def run_info_command(cmd: List[str], cwd: Optional[str] = None) -> Optional[str]: """Run a command for information gathering and return stripped output or None if failed""" try: - return subprocess.check_output(cmd, text=True, stderr=subprocess.DEVNULL, cwd=cwd).strip() - except (subprocess.CalledProcessError, FileNotFoundError): + result = run_probe( + cmd, + cwd=cwd, + check=False, + stdout=subprocess.PIPE, + stderr=subprocess.DEVNULL, + text=True, + ) + if result.returncode != 0: + return None + return result.stdout.strip() + except OSError: return None diff --git a/src/holoscan_cli/utils/sdk.py b/src/holoscan_cli/utils/sdk.py index ccbb4f38..6fd122f6 100644 --- a/src/holoscan_cli/utils/sdk.py +++ b/src/holoscan_cli/utils/sdk.py @@ -31,7 +31,8 @@ from pathlib import Path from typing import Optional, Union -from holoscan_cli.utils.io import fatal, run_info_command, warn +from holoscan_cli.command_plan import record_probe_fallback +from holoscan_cli.utils.io import fatal, run_info_command, run_probe, warn from holoscan_cli.utils.text import parse_semantic_version @@ -72,13 +73,17 @@ def get_gpu_name() -> Optional[str]: if not shutil.which("nvidia-smi"): return None try: - output = subprocess.check_output( + result = run_probe( ["nvidia-smi", "--query-gpu=name", "--format=csv,noheader"], + check=False, + stdout=subprocess.PIPE, text=True, stderr=subprocess.DEVNULL, ) - return output.strip() if output else None - except (subprocess.CalledProcessError, FileNotFoundError): + if result.returncode != 0: + return None + return result.stdout.strip() if result.stdout else None + except OSError: return None @@ -87,6 +92,7 @@ def get_host_gpu() -> str: """Determine if running on dGPU or iGPU""" gpu_name = get_gpu_name() if gpu_name is None: + record_probe_fallback("GPU detection failed; using the resolver fallback 'dgpu'.") print( "Could not find any GPU drivers on host. Defaulting build to target dGPU/CPU stack.", file=sys.stderr, @@ -123,6 +129,9 @@ def get_default_cuda_version() -> str: - "12" if driver version < 580 """ if not shutil.which("nvidia-smi"): + record_probe_fallback( + "NVIDIA driver detection was unavailable; using the CUDA major fallback '13'." + ) warn("nvidia-smi not found, default CUDA version is 13") return "13" @@ -131,11 +140,15 @@ def get_default_cuda_version() -> str: ) if not driver_version: + record_probe_fallback("NVIDIA driver detection failed; using the CUDA major fallback '13'.") warn("Unable to detect NVIDIA driver version, default CUDA version is 13") return "13" result = cuda_major_from_driver(driver_version) if result is None: + record_probe_fallback( + "The NVIDIA driver version was not parseable; using the CUDA major fallback '13'." + ) warn(f"Unable to parse driver version '{driver_version}', default CUDA version is 13") return "13" return result @@ -295,14 +308,24 @@ def get_compute_capacity() -> str: """Get GPU compute capacity""" nvidia_smi = shutil.which("nvidia-smi") if not nvidia_smi: + record_probe_fallback( + "GPU compute-capability detection was unavailable; using the fallback '0.0'." + ) return "0.0" try: - output = subprocess.check_output( - [nvidia_smi, "--query-gpu=compute_cap", "--format=csv,noheader"] + result = run_probe( + [nvidia_smi, "--query-gpu=compute_cap", "--format=csv,noheader"], + check=False, + stdout=subprocess.PIPE, + stderr=subprocess.DEVNULL, + text=True, ) - return output.decode().strip().split("\n")[0] - except (subprocess.CalledProcessError, OSError): - return "0.0" + if result.returncode == 0: + return result.stdout.strip().split("\n")[0] + except OSError: + pass + record_probe_fallback("GPU compute-capability detection failed; using the fallback '0.0'.") + return "0.0" def get_cuda_runtime_version() -> Optional[str]: diff --git a/tests/unit/test_cli_behaviors.py b/tests/unit/test_cli_behaviors.py index 8f548b10..3717ab18 100644 --- a/tests/unit/test_cli_behaviors.py +++ b/tests/unit/test_cli_behaviors.py @@ -193,6 +193,36 @@ def test_effective_mode_config_preserves_cli_overrides(capsys): assert "overrides mode" in capsys.readouterr().err +def test_mode_override_diagnostics_do_not_echo_argument_values(capsys): + cli = object.__new__(project_cli.HoloscanCLI) + args = Namespace( + with_operators="cli-operator", + build_args="--build-arg SERVICE_TOKEN=cli-secret", + configure_args=["-DSERVICE_PASSWORD=cli-secret"], + docker_opts="--env SERVICE_TOKEN=cli-secret", + run_args="--password cli-secret", + ) + mode_config = { + "build": { + "depends": ["mode-operator"], + "docker_build_args": "--build-arg SERVICE_TOKEN=mode-secret", + "cmake_options": ["-DSERVICE_PASSWORD=mode-secret"], + }, + "run": { + "command": "python app.py", + "docker_run_args": "--env SERVICE_TOKEN=mode-secret", + }, + } + + cli.get_effective_build_config(args, mode_config) + cli.get_effective_run_config(args, mode_config) + + diagnostics = capsys.readouterr().err + assert "overrides mode" in diagnostics + assert "cli-secret" not in diagnostics + assert "mode-secret" not in diagnostics + + def test_run_preserves_container_command_after_separator(monkeypatch, tmp_path): monkeypatch.setattr(project_cli.HoloscanCLI, "HOLOHUB_ROOT", tmp_path) with patch.object(project_cli.metadata_util, "gather_metadata", return_value=[]): diff --git a/tests/unit/test_cli_parser.py b/tests/unit/test_cli_parser.py index 7d903499..59e4ce90 100644 --- a/tests/unit/test_cli_parser.py +++ b/tests/unit/test_cli_parser.py @@ -111,6 +111,37 @@ def test_package_accepts_no_docker_build_flag(cli): assert args.no_docker_build is False +@pytest.mark.parametrize("flag,expected", [("--json", "json"), ("--shell", "shell")]) +def test_build_container_accepts_structured_dryrun_formats(cli, flag, expected): + args = cli.parser.parse_args(["build-container", "--dryrun", flag]) + assert args.dryrun is True + assert args.plan_format == expected + assert args._supports_plan is True + + +def test_build_container_plan_formats_are_mutually_exclusive(cli): + with pytest.raises(SystemExit) as exc_info: + cli.parser.parse_args(["build-container", "--dryrun", "--json", "--shell"]) + assert exc_info.value.code == 2 + + +@pytest.mark.parametrize("flag", ["--json", "--shell"]) +def test_build_container_plan_format_requires_dryrun(cli, flag, capsys): + with pytest.raises(SystemExit) as exc_info: + cli.run(["holoscan", "build-container", flag]) + captured = capsys.readouterr() + assert exc_info.value.code == 2 + assert captured.out == "" + assert f"{flag} requires --dryrun" in captured.err + + +@pytest.mark.parametrize("command", ["run-container", "build", "run", "package"]) +def test_unaudited_actions_do_not_accept_plan_json(cli, command): + with pytest.raises(SystemExit) as exc_info: + cli.parser.parse_args([command, "--dryrun", "--json"]) + assert exc_info.value.code == 2 + + def _subparser_help_strings(parser): """Return ``{command_name: help}`` recorded on the parser's subparsers action. diff --git a/tests/unit/test_command_plan.py b/tests/unit/test_command_plan.py new file mode 100644 index 00000000..ffe6add9 --- /dev/null +++ b/tests/unit/test_command_plan.py @@ -0,0 +1,387 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +from __future__ import annotations + +import importlib.resources +import json +import os +import subprocess +import sys + +import pytest +from jsonschema import Draft202012Validator + +from holoscan_cli.command_plan import CommandPlanError, PlanRecorder +from holoscan_cli.utils import holohub, io, sdk + + +@pytest.fixture(autouse=True) +def _clear_host_probe_caches(): + cached = (sdk.get_gpu_name, sdk.get_host_gpu, sdk.get_default_cuda_version) + for function in cached: + function.cache_clear() + yield + for function in cached: + function.cache_clear() + + +def _schema() -> dict: + schema_path = importlib.resources.files("holoscan_cli").joinpath("command_plan.schema.json") + return json.loads(schema_path.read_text(encoding="utf-8")) + + +def _validate(payload: dict) -> None: + Draft202012Validator(_schema()).validate(payload) + + +def test_process_plan_has_stable_steps_env_delta_and_replay(monkeypatch, tmp_path): + monkeypatch.chdir(tmp_path) + recorder = PlanRecorder() + effective_env = dict(os.environ) + effective_env["DOCKER_BUILDKIT"] = "1" + + recorder.record_process(["docker", "buildx", "version"], role="probe", check=True) + action = recorder.record_process( + ["docker", "build", "-t", "example:latest", "."], + role="action", + env=effective_env, + explicit_env={"DOCKER_BUILDKIT": "1"}, + check=True, + ) + + payload = recorder.payload() + _validate(payload) + assert [step["id"] for step in payload["steps"]] == ["step-001", "step-002"] + assert [step["role"] for step in payload["steps"]] == ["probe", "action"] + assert action.private_argv == ["docker", "build", "-t", "example:latest", "."] + assert payload["steps"][1]["environment"] == { + "inherit": True, + "set": {"DOCKER_BUILDKIT": "1"}, + "unset": [], + "required": [], + } + assert "buildx version" not in payload["replay"]["script"] + assert "DOCKER_BUILDKIT=1" in payload["replay"]["script"] + checked = subprocess.run( + ["bash", "-n"], input=payload["replay"]["script"], text=True, capture_output=True + ) + assert checked.returncode == 0, checked.stderr + + +def test_shell_replay_is_multiline_editable_and_preserves_execution(monkeypatch, tmp_path): + working_dir = tmp_path / "working directory" + working_dir.mkdir() + monkeypatch.setenv("PLAN_REMOVE", "remove-me") + recorder = PlanRecorder() + monkeypatch.delenv("PLAN_REMOVE") + monkeypatch.setenv("PLAN_VALUE", "value with spaces") + + recorder.record_process( + [ + sys.executable, + "-c", + ( + "import os; " + "print(os.getcwd()); " + "print(os.environ['PLAN_VALUE']); " + "print('PLAN_REMOVE' in os.environ)" + ), + ], + role="action", + cwd=working_dir, + check=True, + ) + + script = recorder.shell_text() + assert script.startswith("#!/usr/bin/env bash\nset -e\n\n# step-001 (action):") + assert f" cd -- '{working_dir}' && \\\n" in script + assert " env \\\n -u PLAN_REMOVE \\\n" in script + assert " 'PLAN_VALUE=value with spaces' \\\n" in script + assert " \\\n" in script + + replay = subprocess.run(["bash"], input=script, text=True, capture_output=True, check=True) + assert replay.stdout.splitlines() == [str(working_dir), "value with spaces", "False"] + + +def test_shell_replay_maps_multiple_actions_and_cleanup_to_json_steps(): + recorder = PlanRecorder() + recorder.record_process(["docker", "inspect", "example:latest"], role="probe", check=True) + recorder.record_process(["docker", "build", "."], role="action", check=True) + recorder.record_process(["docker", "run", "example:latest"], role="action", check=True) + recorder.record_process(["rm", "-f", "container.cid"], role="cleanup", check=True) + + script = recorder.shell_text() + + assert "docker inspect" not in script + assert "# step-002 (action): docker build\n" in script + assert "\n\n# step-003 (action): docker run\n" in script + assert "\n\n# step-004 (cleanup): rm -f\n" in script + + +def test_shell_replay_comment_escapes_newlines_in_argv(): + recorder = PlanRecorder() + recorder.record_process(["printf", "safe\nnot-a-command"], role="action", check=True) + + script = recorder.shell_text() + + assert "# step-001 (action): printf 'safe\\nnot-a-command'\n" in script + checked = subprocess.run(["bash", "-n"], input=script, text=True, capture_output=True) + assert checked.returncode == 0, checked.stderr + + +def test_schema_rejects_contradictory_replay_states(): + recorder = PlanRecorder() + recorder.record_process(["docker", "build", "."], role="action", check=True) + payload = recorder.payload() + validator = Draft202012Validator(_schema()) + + payload["replay"]["unavailable_reason"] = "contradictory" + assert not validator.is_valid(payload) + + payload["replay"]["script"] = None + payload["replay"]["unavailable_reason"] = None + assert not validator.is_valid(payload) + + +def test_sensitive_docker_literal_is_redacted_but_xauthority_is_not(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + recorder = PlanRecorder() + step = recorder.record_process( + [ + "docker", + "run", + "--env", + "SERVICE_API_TOKEN=sentinel-secret", + "--env", + "XAUTHORITY=/tmp/xauth", + "example:latest", + ], + role="action", + check=True, + ) + + serialized = recorder.json_text() + payload = json.loads(serialized) + _validate(payload) + assert "sentinel-secret" in step.private_argv[3] + assert "sentinel-secret" not in serialized + assert "SERVICE_API_TOKEN=" in payload["steps"][0]["argv"] + assert "XAUTHORITY=/tmp/xauth" in payload["steps"][0]["argv"] + assert payload["replay"]["script"] is None + assert payload["replay"]["unavailable_reason"] == "redacted_literal" + + +def test_bare_docker_environment_references_are_declared_without_leaking_values( + tmp_path, monkeypatch +): + monkeypatch.chdir(tmp_path) + monkeypatch.setenv("PLAN_DEMO", "ordinary-value") + monkeypatch.setenv("NGC_API_KEY", "sentinel-secret") + recorder = PlanRecorder() + recorder.record_process( + [ + "docker", + "build", + "--build-arg", + "PLAN_DEMO", + "--build-arg", + "NGC_API_KEY", + ".", + ], + role="action", + check=True, + ) + + serialized = recorder.json_text() + payload = json.loads(serialized) + assert payload["steps"][0]["environment"]["required"] == [ + "NGC_API_KEY", + "PLAN_DEMO", + ] + assert payload["replay"]["required_environment"] == ["NGC_API_KEY", "PLAN_DEMO"] + assert "ordinary-value" not in serialized + assert "sentinel-secret" not in serialized + assert "Set NGC_API_KEY" in payload["replay"]["script"] + + +def test_unset_bare_docker_environment_reference_fails_closed(monkeypatch): + monkeypatch.delenv("PLAN_DEMO", raising=False) + recorder = PlanRecorder() + + with pytest.raises(CommandPlanError, match="PLAN_DEMO"): + recorder.record_process( + ["docker", "build", "--build-arg", "PLAN_DEMO", "."], + role="action", + check=True, + ) + + +def test_owned_environment_overlay_satisfies_bare_docker_reference(monkeypatch): + monkeypatch.delenv("PLAN_DEMO", raising=False) + recorder = PlanRecorder() + effective_env = dict(os.environ) + effective_env["PLAN_DEMO"] = "owned-value" + recorder.record_process( + ["docker", "build", "--build-arg", "PLAN_DEMO", "."], + role="action", + env=effective_env, + explicit_env={"PLAN_DEMO": "owned-value"}, + check=True, + ) + + payload = recorder.payload() + assert payload["steps"][0]["environment"]["set"] == {"PLAN_DEMO": "owned-value"} + assert payload["steps"][0]["environment"]["required"] == [] + assert "Set PLAN_DEMO" not in payload["replay"]["script"] + assert "PLAN_DEMO=owned-value" in payload["replay"]["script"] + + +def test_empty_bare_docker_environment_reference_is_replayable(monkeypatch): + monkeypatch.setenv("PLAN_DEMO", "") + recorder = PlanRecorder() + recorder.record_process( + ["docker", "build", "--build-arg", "PLAN_DEMO", "."], + role="action", + check=True, + ) + + script = recorder.payload()["replay"]["script"] + assert "${PLAN_DEMO?Set PLAN_DEMO" in script + assert "${PLAN_DEMO:?" not in script + + +def test_run_command_records_action_without_printing_or_executing(monkeypatch, capsys): + recorder = PlanRecorder() + + def unexpected_run(*args, **kwargs): + raise AssertionError("an action subprocess executed during planning") + + monkeypatch.setattr(io.subprocess, "run", unexpected_run) + with recorder.activate(): + result = io.run_command( + ["docker", "build", "."], + dry_run=True, + env_updates={"DOCKER_BUILDKIT": "1"}, + ) + + assert result.returncode == 0 + assert capsys.readouterr().out == "" + assert recorder.steps[0].role == "action" + assert recorder.steps[0].environment["set"]["DOCKER_BUILDKIT"] == "1" + + +def test_run_probe_executes_and_is_recorded(monkeypatch): + recorder = PlanRecorder() + calls = [] + + def fake_run(cmd, check=False, **kwargs): + calls.append((cmd, check, kwargs)) + return subprocess.CompletedProcess(cmd, 0, stdout="ok\n") + + monkeypatch.setattr(io.subprocess, "run", fake_run) + with recorder.activate(): + result = io.run_probe( + ["docker", "buildx", "version"], check=True, capture_output=True, text=True + ) + + assert result.stdout == "ok\n" + assert calls[0][0] == ["docker", "buildx", "version"] + assert recorder.steps[0].role == "probe" + assert recorder.steps[0].check is True + + +def test_unconfigured_probe_output_is_captured_during_planning(monkeypatch): + recorder = PlanRecorder() + seen = {} + + def fake_run(cmd, check=False, **kwargs): + seen.update(kwargs) + return subprocess.CompletedProcess(cmd, 0, stdout=b"probe output", stderr=b"") + + monkeypatch.setattr(io.subprocess, "run", fake_run) + with recorder.activate(): + io.run_probe(["host-probe"]) + + assert seen["stdout"] is subprocess.PIPE + assert seen["stderr"] is subprocess.PIPE + + +def test_shell_renderer_rejects_probe_only_plan(): + recorder = PlanRecorder() + recorder.record_process(["docker", "buildx", "version"], role="probe", check=True) + + with pytest.raises(CommandPlanError, match="no action steps"): + recorder.shell_text() + + +def test_recorder_rejects_empty_process_argv(): + recorder = PlanRecorder() + + with pytest.raises(CommandPlanError, match="at least one token"): + recorder.record_process([], role="action", check=True) + + +def test_replacement_subprocess_environment_fails_closed(monkeypatch): + recorder = PlanRecorder() + + with recorder.activate(), pytest.raises(CommandPlanError, match="replacement subprocess"): + io.run_command( + ["docker", "build", "."], + dry_run=True, + env={"PATH": os.environ.get("PATH", "")}, + ) + + +def test_host_resolvers_record_the_probes_that_select_build_arguments(monkeypatch, tmp_path): + monkeypatch.setattr(sdk.shutil, "which", lambda name: f"/usr/bin/{name}") + monkeypatch.setattr(holohub, "HOLOHUB_ROOT", tmp_path) + + outputs = { + ("nvidia-smi", "--query-gpu=name", "--format=csv,noheader"): "NVIDIA H100\n", + ( + "/usr/bin/nvidia-smi", + "--query-gpu=compute_cap", + "--format=csv,noheader", + ): "9.0\n", + ( + "nvidia-smi", + "--query-gpu=driver_version", + "--format=csv,noheader", + ): "580.126.20\n", + ("git", "rev-parse", "--short=12", "HEAD"): "deadbeef1234\n", + ("git", "rev-parse", "--abbrev-ref", "HEAD"): "feature/plan\n", + } + + def fake_run(cmd, check=False, **kwargs): + return subprocess.CompletedProcess(cmd, 0, stdout=outputs[tuple(cmd)], stderr="") + + monkeypatch.setattr(io.subprocess, "run", fake_run) + recorder = PlanRecorder() + with recorder.activate(): + assert sdk.get_host_gpu() == "dgpu" + assert sdk.get_compute_capacity() == "9.0" + assert sdk.get_default_cuda_version() == "13" + assert holohub.get_git_short_sha() == "deadbeef1234" + assert holohub.get_current_branch_slug() == "feature-plan" + + assert [step.role for step in recorder.steps] == ["probe"] * 5 + assert [step.private_argv for step in recorder.steps] == [list(command) for command in outputs] + assert recorder.steps[-1].cwd == str(tmp_path) + + +def test_host_resolver_fallbacks_are_explicit_plan_warnings(monkeypatch): + monkeypatch.setattr(sdk.shutil, "which", lambda _name: None) + recorder = PlanRecorder() + + with recorder.activate(): + assert sdk.get_host_gpu() == "dgpu" + assert sdk.get_compute_capacity() == "0.0" + assert sdk.get_default_cuda_version() == "13" + + assert recorder.steps == [] + assert [warning["code"] for warning in recorder.warnings] == [ + "probe_fallback_used", + "probe_fallback_used", + "probe_fallback_used", + ] diff --git a/tests/unit/test_command_plan_cli.py b/tests/unit/test_command_plan_cli.py new file mode 100644 index 00000000..bdb3cf88 --- /dev/null +++ b/tests/unit/test_command_plan_cli.py @@ -0,0 +1,222 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +from __future__ import annotations + +import json +import subprocess + +import pytest + +from holoscan_cli import cli as project_cli +from holoscan_cli.container import core as container_core +from holoscan_cli.utils import holohub, io, sdk + +_REAL_SUBPROCESS_RUN = subprocess.run + + +@pytest.fixture() +def plan_cli(tmp_path, monkeypatch): + dockerfile = tmp_path / "Dockerfile" + dockerfile.write_text("FROM scratch\n", encoding="utf-8") + monkeypatch.chdir(tmp_path) + monkeypatch.setattr(project_cli.HoloscanCLI, "HOLOHUB_ROOT", tmp_path) + monkeypatch.setattr(container_core.HoloscanContainer, "HOLOHUB_ROOT", tmp_path) + monkeypatch.setattr(container_core.HoloscanContainer, "DEFAULT_DOCKERFILE", dockerfile) + monkeypatch.setattr(container_core.HoloscanContainer, "BASE_SDK_VERSION", None) + monkeypatch.setattr(container_core.HoloscanContainer, "DEFAULT_DOCKER_BUILD_ARGS", "") + monkeypatch.setattr(container_core.HoloscanContainer, "DOCKER_EXE", "docker") + monkeypatch.delenv("HOLOSCAN_CLI_SOURCE", raising=False) + monkeypatch.setattr(container_core, "get_host_gpu", lambda: "dgpu") + monkeypatch.setattr(container_core, "get_compute_capacity", lambda: "90") + monkeypatch.setattr(container_core, "get_default_cuda_version", lambda: "13") + + probe_calls = [] + + def fake_subprocess_run(cmd, check=False, **kwargs): + probe_calls.append(list(cmd)) + return subprocess.CompletedProcess(cmd, 0, stdout="buildx 0.20.0\n", stderr="") + + monkeypatch.setattr(io.subprocess, "run", fake_subprocess_run) + cli = project_cli.HoloscanCLI(script_name="holoscan") + return cli, dockerfile, probe_calls + + +def _argv(dockerfile, output_format): + return [ + "holoscan", + "build-container", + "--docker-file", + str(dockerfile), + "--base-img", + "example.com/base:latest", + "--img", + "example:plan", + "--cuda", + "13", + "--dryrun", + output_format, + ] + + +def test_build_container_json_is_pure_and_actions_do_not_execute(plan_cli, capsys): + cli, dockerfile, probe_calls = plan_cli + + cli.run(_argv(dockerfile, "--json")) + + captured = capsys.readouterr() + payload = json.loads(captured.out) + assert payload["schema_version"] == 1 + assert payload["scope"] == "current_cli_process" + assert payload["complete"] is True + assert [step["role"] for step in payload["steps"]] == ["probe", "action"] + assert payload["steps"][0]["argv"] == ["docker", "buildx", "version"] + action = payload["steps"][1] + assert action["argv"][:2] == ["docker", "build"] + assert "BASE_IMAGE=example.com/base:latest" in action["argv"] + assert "GPU_TYPE=dgpu" in action["argv"] + assert "COMPUTE_CAPACITY=90" in action["argv"] + assert action["environment"]["set"] == {"DOCKER_BUILDKIT": "1"} + assert probe_calls == [["docker", "buildx", "version"]] + assert "No project provided" in captured.err + assert "[dryrun]" not in captured.out + + +def test_build_container_plan_integrates_host_resolution_and_wrapper_defaults( + tmp_path, monkeypatch, capsys +): + dockerfile = tmp_path / "Dockerfile" + dockerfile.write_text("FROM scratch\n", encoding="utf-8") + monkeypatch.chdir(tmp_path) + monkeypatch.setattr(project_cli.HoloscanCLI, "HOLOHUB_ROOT", tmp_path) + monkeypatch.setattr(container_core.HoloscanContainer, "HOLOHUB_ROOT", tmp_path) + monkeypatch.setattr(container_core.HoloscanContainer, "DEFAULT_DOCKERFILE", dockerfile) + monkeypatch.setattr(container_core.HoloscanContainer, "BASE_SDK_VERSION", "4.5.0") + monkeypatch.setattr( + container_core.HoloscanContainer, + "BASE_IMAGE_NAME", + container_core.HoloscanContainer.DEFAULT_BASE_IMAGE_NAME, + ) + monkeypatch.setattr(container_core.HoloscanContainer, "BASE_IMAGE_FORMAT", None) + monkeypatch.setattr(container_core.HoloscanContainer, "DEFAULT_IMAGE_FORMAT", None) + monkeypatch.setattr(container_core.HoloscanContainer, "CONTAINER_PREFIX", "wrapper") + monkeypatch.setattr( + container_core.HoloscanContainer, + "DEFAULT_DOCKER_BUILD_ARGS", + "--build-arg WRAPPER_FEATURE=enabled --progress=plain", + ) + monkeypatch.setattr(container_core.HoloscanContainer, "DOCKER_EXE", "docker") + monkeypatch.setattr(holohub, "HOLOHUB_ROOT", tmp_path) + monkeypatch.setattr(sdk.shutil, "which", lambda name: f"/usr/bin/{name}") + monkeypatch.delenv("HOLOSCAN_CLI_SOURCE", raising=False) + + outputs = { + ( + "nvidia-smi", + "--query-gpu=driver_version", + "--format=csv,noheader", + ): "580.126.20\n", + ("git", "rev-parse", "--short=12", "HEAD"): "deadbeef1234\n", + ("git", "rev-parse", "--abbrev-ref", "HEAD"): "feature/plan\n", + ("nvidia-smi", "--query-gpu=name", "--format=csv,noheader"): "NVIDIA H100\n", + ( + "/usr/bin/nvidia-smi", + "--query-gpu=compute_cap", + "--format=csv,noheader", + ): "9.0\n", + ("docker", "buildx", "version"): "buildx 0.20.0\n", + } + probe_calls = [] + + def fake_subprocess_run(cmd, check=False, **kwargs): + probe_calls.append(list(cmd)) + return subprocess.CompletedProcess(cmd, 0, stdout=outputs[tuple(cmd)], stderr="") + + monkeypatch.setattr(io.subprocess, "run", fake_subprocess_run) + cached = (sdk.get_gpu_name, sdk.get_host_gpu, sdk.get_default_cuda_version) + for function in cached: + function.cache_clear() + try: + project_cli.HoloscanCLI(script_name="holoscan").run( + ["holoscan", "build-container", "--dryrun", "--json"] + ) + finally: + for function in cached: + function.cache_clear() + + payload = json.loads(capsys.readouterr().out) + assert probe_calls == [list(command) for command in outputs] + assert [step["role"] for step in payload["steps"]] == ["probe"] * 6 + ["action"] + action = payload["steps"][-1] + assert "BASE_IMAGE=nvcr.io/nvidia/clara-holoscan/holoscan:v4.5.0-cuda13" in action["argv"] + assert "GPU_TYPE=dgpu" in action["argv"] + assert "COMPUTE_CAPACITY=9.0" in action["argv"] + assert "CUDA_MAJOR=13" in action["argv"] + assert "WRAPPER_FEATURE=enabled" in action["argv"] + assert "--progress=plain" in action["argv"] + assert "wrapper:feature-plan" in action["argv"] + assert "wrapper:deadbeef1234" in action["argv"] + assert payload["warnings"] == [] + + +def test_shell_output_matches_json_replay(plan_cli, capsys): + cli, dockerfile, _probe_calls = plan_cli + cli.run(_argv(dockerfile, "--json")) + payload = json.loads(capsys.readouterr().out) + + cli.run(_argv(dockerfile, "--shell")) + shell = capsys.readouterr().out + + assert shell == payload["replay"]["script"] + action_id = next(step["id"] for step in payload["steps"] if step["role"] == "action") + assert f"# {action_id} (action): docker build\n" in shell + assert "docker build \\\n" in shell + checked = _REAL_SUBPROCESS_RUN(["bash", "-n"], input=shell, text=True, capture_output=True) + assert checked.returncode == 0, checked.stderr + + +def test_buildx_failure_leaves_stdout_empty(plan_cli, monkeypatch, capsys): + cli, dockerfile, _probe_calls = plan_cli + + def fail_buildx(cmd, check=False, **kwargs): + raise subprocess.CalledProcessError(1, cmd) + + monkeypatch.setattr(io.subprocess, "run", fail_buildx) + with pytest.raises(SystemExit) as exc_info: + cli.run(_argv(dockerfile, "--json")) + + captured = capsys.readouterr() + assert exc_info.value.code == 1 + assert captured.out == "" + assert "docker buildx plugin is missing" in captured.err + + +def test_late_extra_script_failure_does_not_emit_partial_plan(plan_cli, capsys): + cli, dockerfile, _probe_calls = plan_cli + argv = _argv(dockerfile, "--json") + argv[-1:-1] = ["--extra-scripts", "definitely-not-a-script"] + + with pytest.raises(SystemExit) as exc_info: + cli.run(argv) + + captured = capsys.readouterr() + assert exc_info.value.code == 1 + assert captured.out == "" + assert "definitely-not-a-script.sh not found" in captured.err + + +def test_unreplayable_shell_plan_leaves_stdout_empty(plan_cli, capsys): + cli, dockerfile, _probe_calls = plan_cli + argv = _argv(dockerfile, "--shell") + argv[-1:-1] = [ + "--build-args=--build-arg SERVICE_API_TOKEN=sentinel-secret", + ] + + with pytest.raises(SystemExit) as exc_info: + cli.run(argv) + + captured = capsys.readouterr() + assert exc_info.value.code == 1 + assert captured.out == "" + assert "redacted_literal" in captured.err + assert "sentinel-secret" not in captured.err diff --git a/tests/unit/test_command_registry.py b/tests/unit/test_command_registry.py index 04d192b1..42c7ba70 100644 --- a/tests/unit/test_command_registry.py +++ b/tests/unit/test_command_registry.py @@ -72,6 +72,11 @@ def test_project_commands_by_name_keys_match_specs(): assert set(by_name) == {spec.name for spec in registry.PROJECT_COMMANDS} +def test_only_audited_commands_advertise_structured_plan_support(): + supported = {spec.name for spec in registry.PROJECT_COMMANDS if spec.supports_plan} + assert supported == {"build-container"} + + def test_autocompletion_command_list_comes_from_registry(capsys): cli = SimpleNamespace( projects=[ diff --git a/tests/unit/test_container_core.py b/tests/unit/test_container_core.py index eec69d3b..6732e690 100644 --- a/tests/unit/test_container_core.py +++ b/tests/unit/test_container_core.py @@ -422,6 +422,39 @@ def test_build_dryrun_emits_base_and_extra_script_layers(tmp_path, monkeypatch): assert "holohub-my_app:feature-x-coverage" in layer +def test_build_uses_explicit_buildkit_env_without_mutating_process_env(tmp_path, monkeypatch): + dockerfile = tmp_path / "Dockerfile" + dockerfile.write_text("FROM scratch\n", encoding="utf-8") + calls = [] + monkeypatch.setenv("DOCKER_BUILDKIT", "0") + monkeypatch.setattr(container_core, "get_host_gpu", lambda: "dgpu") + monkeypatch.setattr(container_core, "get_compute_capacity", lambda: "90") + monkeypatch.setattr(container_core, "get_default_cuda_version", lambda: "13") + monkeypatch.setattr( + container_core, + "run_command", + lambda cmd, **kwargs: calls.append((cmd, kwargs)), + ) + + def unexpected_probe(*args, **kwargs): + raise AssertionError("plain dry-run must not execute buildx") + + monkeypatch.setattr(container_core, "run_probe", unexpected_probe) + + container = _stub_container(tmp_path) + container.dryrun = True + container.build( + docker_file=str(dockerfile), + base_img="example.com/base:latest", + img="example:plan", + cuda_version="13", + ) + + assert len(calls) == 1 + assert calls[0][1]["env_updates"] == {"DOCKER_BUILDKIT": "1"} + assert container_core.os.environ["DOCKER_BUILDKIT"] == "0" + + def test_build_dryrun_allows_bundled_extra_script_dir(tmp_path, monkeypatch): """Bundled setup scripts live outside the source project but can still serve as the Docker build context for extra-script layers.""" diff --git a/tests/unit/test_io.py b/tests/unit/test_io.py index ca299666..246481d7 100644 --- a/tests/unit/test_io.py +++ b/tests/unit/test_io.py @@ -63,3 +63,25 @@ def fake_run(cmd, check=True, **kwargs): assert seen["env"] is app_env assert all("not-on-the-command-line" not in arg for arg in seen["cmd"]) assert "PATH=" in capsys.readouterr().out + + +def test_run_command_applies_owned_env_overlay_without_mutating_process_env(monkeypatch): + monkeypatch.setenv("DOCKER_BUILDKIT", "0") + monkeypatch.delenv("PLAN_ONLY", raising=False) + seen = {} + + def fake_run(cmd, check=True, **kwargs): + seen["env"] = kwargs["env"] + return subprocess.CompletedProcess(cmd, 0) + + monkeypatch.setattr(io.subprocess, "run", fake_run) + + io.run_command( + ["docker", "build", "."], + env_updates={"DOCKER_BUILDKIT": "1", "PLAN_ONLY": "enabled"}, + ) + + assert seen["env"]["DOCKER_BUILDKIT"] == "1" + assert seen["env"]["PLAN_ONLY"] == "enabled" + assert io.os.environ["DOCKER_BUILDKIT"] == "0" + assert "PLAN_ONLY" not in io.os.environ diff --git a/tests/unit/test_main.py b/tests/unit/test_main.py index 3ea89e40..8fef8079 100644 --- a/tests/unit/test_main.py +++ b/tests/unit/test_main.py @@ -211,6 +211,11 @@ class TestMain: ["holoscan", "run", "some-image:tag", "--driver"], None, ), + ( + ["holoscan", "build-container", "--dryrun", "--json"], + ["holoscan", "build-container", "--dryrun", "--json"], + None, + ), ( ["holoscan", "--log-level", "debug", "list"], ["holoscan", "list"], diff --git a/tests/unit/test_package_data.py b/tests/unit/test_package_data.py index 74c037f4..a6162833 100644 --- a/tests/unit/test_package_data.py +++ b/tests/unit/test_package_data.py @@ -105,6 +105,11 @@ def test_logging_config_is_shipped(): assert logging_config.is_file() +def test_command_plan_schema_is_shipped(): + schema = importlib.resources.files("holoscan_cli").joinpath("command_plan.schema.json") + assert schema.is_file() + + def test_all_metadata_schemas_are_packaged(): schemas = { path.name diff --git a/tests/unit/test_sdk_utils.py b/tests/unit/test_sdk_utils.py index bf6e8878..4a564e39 100644 --- a/tests/unit/test_sdk_utils.py +++ b/tests/unit/test_sdk_utils.py @@ -46,9 +46,9 @@ def test_cuda_major_from_driver(driver, expected): def test_get_gpu_name_returns_first_nvidia_smi_result(monkeypatch): monkeypatch.setattr(sdk.shutil, "which", lambda name: "/usr/bin/nvidia-smi") monkeypatch.setattr( - sdk.subprocess, - "check_output", - lambda cmd, **kwargs: "NVIDIA H100\n", + sdk, + "run_probe", + lambda cmd, **kwargs: subprocess.CompletedProcess(cmd, 0, stdout="NVIDIA H100\n"), ) assert sdk.get_gpu_name() == "NVIDIA H100" @@ -158,9 +158,9 @@ def test_find_hsdk_build_rel_dir_prefers_install_then_build(tmp_path, monkeypatc def test_get_compute_capacity_from_nvidia_smi(monkeypatch): monkeypatch.setattr(sdk.shutil, "which", lambda name: "/usr/bin/nvidia-smi") monkeypatch.setattr( - sdk.subprocess, - "check_output", - lambda cmd: b"9.0\n8.9\n", + sdk, + "run_probe", + lambda cmd, **kwargs: subprocess.CompletedProcess(cmd, 0, stdout="9.0\n8.9\n"), ) assert sdk.get_compute_capacity() == "9.0" @@ -207,11 +207,11 @@ def test_check_nvidia_ctk_accepts_new_tool(monkeypatch): def test_get_gpu_name_and_compute_capacity_handle_subprocess_failures(monkeypatch): monkeypatch.setattr(sdk.shutil, "which", lambda name: "/usr/bin/nvidia-smi") - - def fail(*args, **kwargs): - raise subprocess.CalledProcessError(1, args[0]) - - monkeypatch.setattr(sdk.subprocess, "check_output", fail) + monkeypatch.setattr( + sdk, + "run_probe", + lambda cmd, **kwargs: subprocess.CompletedProcess(cmd, 1, stdout=""), + ) assert sdk.get_gpu_name() is None assert sdk.get_compute_capacity() == "0.0"