diff --git a/examples/portfolio/config/global_controller.yaml b/examples/portfolio/config/global_controller.yaml index c1aea194..b60dc910 100644 --- a/examples/portfolio/config/global_controller.yaml +++ b/examples/portfolio/config/global_controller.yaml @@ -108,9 +108,6 @@ env_file: .env poll_interval: 4 logs: true -# Secrets handed to every agent container (AWS_BEARER_TOKEN_BEDROCK for the LLM proxy). -env_file: .env - redis: host: localhost port: 6379 diff --git a/examples/text2sql/config/global_controller.yaml b/examples/text2sql/config/global_controller.yaml index 0feb83db..a8e2e735 100644 --- a/examples/text2sql/config/global_controller.yaml +++ b/examples/text2sql/config/global_controller.yaml @@ -105,4 +105,14 @@ logs: false redis: host: localhost port: 6379 - db: 0 \ No newline at end of file + db: 0 + +# EC2 defaults for the `provider: EC2` replicas above. +ec2: + region: ${EC2_REGION} + ami_id: ${EC2_AMI_ID} + subnet_id: ${EC2_SUBNET_ID} + security_group_ids: + - ${EC2_SECURITY_GROUP_ID} + ssh_user: ${EC2_SSH_USER} + ssh_private_key_path: ${EC2_SSH_PRIVATE_KEY_PATH} diff --git a/packages/cli/canyonos/constants.py b/packages/cli/canyonos/constants.py index 732cd426..edee9698 100644 --- a/packages/cli/canyonos/constants.py +++ b/packages/cli/canyonos/constants.py @@ -86,14 +86,30 @@ def local_redis_port(config_path): """Host port the local node's Redis is published on, or the default.""" try: with open(config_path) as f: - config = yaml.safe_load(f) or {} + config = yaml.safe_load(f) except (OSError, yaml.YAMLError): return DEFAULT_REDIS_PORT + if not isinstance(config, dict): + return DEFAULT_REDIS_PORT - for agent in config.get("agents") or []: - if agent.get("host", "localhost") in ("localhost", "127.0.0.1"): - return agent.get("redis_port", DEFAULT_REDIS_PORT) - return (config.get("redis") or {}).get("port", DEFAULT_REDIS_PORT) + agents = config.get("agents") + for agent in agents if isinstance(agents, list) else []: + if isinstance(agent, dict) and agent.get("host", "localhost") in ( + "localhost", + "127.0.0.1", + ): + return _port_or_default(agent.get("redis_port", DEFAULT_REDIS_PORT)) + redis = config.get("redis") + if not isinstance(redis, dict): + return DEFAULT_REDIS_PORT + return _port_or_default(redis.get("port", DEFAULT_REDIS_PORT)) + + +def _port_or_default(value): + """`value` when it is a usable TCP port, else the default the runtime falls back to.""" + if isinstance(value, bool) or not isinstance(value, int): + return DEFAULT_REDIS_PORT + return value if 1 <= value <= 65535 else DEFAULT_REDIS_PORT def _source_root(config_path): diff --git a/packages/cli/canyonos/doctor.py b/packages/cli/canyonos/doctor.py index 63fee484..75f8e8a8 100644 --- a/packages/cli/canyonos/doctor.py +++ b/packages/cli/canyonos/doctor.py @@ -95,12 +95,12 @@ def _gc_check(state, status): def _redis_check(): """TCP reachability only -- not a real PING, but enough to say something's listening where the local provider and dashboard both expect Redis.""" - port = local_redis_port(default_config_path()) + redis_port = local_redis_port(default_config_path()) return _print_check( "Redis", - port_in_use(int(port)), - f"127.0.0.1:{port}", - f"nothing is listening on {port} -- redeploy, or check `docker ps`", + port_in_use(redis_port), + f"127.0.0.1:{redis_port}", + f"nothing is listening on {redis_port} -- redeploy, or check `docker ps`", ) diff --git a/packages/cli/tests/test_cli_entry.py b/packages/cli/tests/test_cli_entry.py new file mode 100644 index 00000000..c8e900b3 --- /dev/null +++ b/packages/cli/tests/test_cli_entry.py @@ -0,0 +1,18 @@ +import subprocess +import sys + + +def test_the_canyonos_entry_point_imports_in_a_fresh_interpreter(): + """`canyonos` is `cli:main`; if `import cli` fails, every command fails. + + Run in a fresh interpreter: inside the suite another test may already have + imported the modules involved, which is how a broken import once passed. + """ + result = subprocess.run( + [sys.executable, "-c", "import cli; assert callable(cli.main)"], + capture_output=True, + text=True, + timeout=60, + ) + + assert result.returncode == 0, result.stderr diff --git a/packages/cli/tests/test_local_redis_port.py b/packages/cli/tests/test_local_redis_port.py index 47d34da9..425269de 100644 --- a/packages/cli/tests/test_local_redis_port.py +++ b/packages/cli/tests/test_local_redis_port.py @@ -37,6 +37,20 @@ def test_the_top_level_redis_section_is_the_fallback(self): def test_an_unreadable_config_falls_back_to_the_default(self): self.assertEqual(local_redis_port("/nonexistent/global_controller.yaml"), 6379) + def test_a_malformed_config_falls_back_to_the_default(self): + for config in ( + {"agents": [{"name": "A", "redis_port": "abc"}]}, + {"agents": [{"name": "A", "redis_port": "${REDIS_PORT}"}]}, + {"agents": [{"name": "A", "redis_port": None}]}, + {"agents": [{"name": "A", "redis_port": 70000}]}, + {"agents": ["just-a-string"]}, + {"agents": "not-a-list", "redis": {"port": True}}, + {"agents": [], "redis": "not-a-mapping"}, + ["not", "a", "mapping"], + ): + with self.subTest(config=config): + self.assertEqual(local_redis_port(self._config(config)), 6379) + if __name__ == "__main__": unittest.main() diff --git a/packages/core/canyonos_core/cli.py b/packages/core/canyonos_core/cli.py index e9ee503a..ce9bd9b1 100644 --- a/packages/core/canyonos_core/cli.py +++ b/packages/core/canyonos_core/cli.py @@ -16,19 +16,19 @@ import subprocess import sys +from canyonos_core.controller.utils.config_env import load_config from canyonos_core.controller.utils.env_file import resolve_env_file +from canyonos_core.schema import ( + DependencyPinConflict, + check_project, + declarations_by_name, + render_violation, +) logging.basicConfig(level=logging.INFO) logger = logging.getLogger("canyonos_core") -DEFAULT_DOCKER_PLATFORM = "linux/amd64" ARTIFACT_DIR_NAME = ".car" SOURCE_DIR_NAME = "app" -EC2_REQUIRED_CONFIG_KEYS = ( - "ami_id", - "subnet_id", - "security_group_ids", - "region", -) # ------------------------------------------------------------------ # @@ -47,11 +47,14 @@ def _get_package_dir(): def _load_config(config_path): - """Load a YAML config file.""" - import yaml + """Load the config as the schema checked it and the controller will read it. - with open(config_path, "r") as f: - config = yaml.safe_load(f) + The root `.env` is imported and `${VAR}` refs are expanded through the + same helper both of those use. Reading the raw YAML here instead let a + reference the schema had validated in its expanded form reach the build + as the literal `${VAR}`. + """ + config = load_config(config_path) # Everything below here till "return config" is basically just checks to make sure the folder is correct if not isinstance(config, dict): raise RuntimeError(f"Config must contain a YAML mapping: {config_path}") @@ -135,25 +138,82 @@ def _artifact_prefix(root): ) +def _project_layout(): + """(artifact_root, source_root, declarations_dir) for the project in cwd. + + The .car layout keeps the app's own code under `.car/app` and everything + generated beside it; a plain checkout keeps both at the project root. + """ + project_dir = os.path.abspath(os.getcwd()) + prefix = _artifact_prefix(project_dir) + artifact_root = os.path.join(project_dir, prefix) if prefix else project_dir + return ( + artifact_root, + os.path.join(artifact_root, SOURCE_DIR_NAME) if prefix else project_dir, + os.path.join(artifact_root, "config" if prefix else "agents"), + ) + + +def _reject(violations, summary): + """Log each violation on its own line, then exit. + + The host CLI reads the first `ERROR:` line out of this process's output as + the root cause, so a violation is never split across lines. + """ + for violation in violations: + logger.error("%s", render_violation(violation)) + logger.error(summary, len(violations)) + sys.exit(1) + + +def validate_or_exit(config_path, declarations_dir, source_dir=None): + """Reject the config before anything is generated; return the parsed manifest.""" + manifest, violations = check_project(config_path, declarations_dir, source_dir) + if violations: + _reject( + violations, + "Configuration rejected: %d problem(s) found; nothing was built.", + ) + return manifest + + def _normalize_requirements(agent_cfg): - """Return an agent's `requirements` list, or [] if absent/null/malformed.""" - requirements = agent_cfg.get("requirements") or [] - if not isinstance(requirements, list) or not all( - isinstance(r, str) for r in requirements - ): - # The requirements list is bad, assuming file has no requirements and logging error - logger.warning( - "Agent '%s': `requirements` must be a list of strings, got %r; ignoring.", - agent_cfg.get("name"), - requirements, + """Return a service's `requirements` list; the schema already checked its shape.""" + return list(agent_cfg.get("requirements") or []) + + +def _check_dependency_pins(manifest): + """Fail the build when an app pin cannot share a version with a platform pin. + + Every service is checked before the first one is built, so a project with + two bad pins is told about both instead of one per run. + """ + from canyonos_core.stub_generator import _platform_overrides + + violations = [] + for index, service in enumerate(manifest.agents): + try: + _platform_overrides( + getattr(service, "requirements", ()), + service=index, + manifest_path=manifest.path, + lines=getattr(service, "requirement_lines", ()), + ) + except DependencyPinConflict as conflict: + violations.extend(conflict.violations) + + if violations: + _reject( + violations, + "Dependency pins rejected: %d conflict(s) found; nothing was built.", ) - return [] - return requirements def _docker_platform(): """Return the target Docker platform for portable runtime images.""" - return os.environ.get("CANYONOS_DOCKER_PLATFORM", DEFAULT_DOCKER_PLATFORM) + from canyonos_core.stub_generator import target_docker_platform + + return target_docker_platform() def _docker_build_cmd(*args): @@ -229,13 +289,8 @@ def _ensure_grpc_stubs_importable(project_dir): def _preflight_ec2_deploy(config, project_dir): - ec2_cfg = config.get("ec2", {}) - missing = [key for key in EC2_REQUIRED_CONFIG_KEYS if not ec2_cfg.get(key)] - if missing: - raise RuntimeError( - f"EC2 deploy preflight failed: missing ec2 config keys: {', '.join(sorted(missing))}" - ) - + # The required `ec2:` keys are the manifest schema's job, checked before + # anything was built; what is left here is the local toolchain. _require_docker_for_ec2("deploy") _ensure_grpc_stubs_importable(project_dir) @@ -308,37 +363,23 @@ def _run_build(config_path): logger.error("Config file not found: %s", config_path) sys.exit(1) + artifact_root, source_root, declarations_dir = _project_layout() + + # Nothing below this line runs against a config the schema rejects: no + # stubs, no protoc, no Docker context, no image. It comes before the load + # so a file that is not YAML at all is rendered as a violation too, and it + # is handed source_root so a service whose code is missing fails here + # rather than being skipped out of a deploy that then reports success. + manifest = validate_or_exit(config_path, declarations_dir, source_root) + _check_dependency_pins(manifest) + config = _load_config(config_path) agents = config.get("agents", []) - project_dir = os.path.abspath(os.getcwd()) - prefix = _artifact_prefix(project_dir) - artifact_root = os.path.join(project_dir, prefix) if prefix else project_dir - source_root = ( - os.path.join(artifact_root, SOURCE_DIR_NAME) if prefix else project_dir - ) package_dir = _get_package_dir() - missing_sources = [] - for agent in agents: - source_field = ( - "workflow_file" - if agent.get("type", "agent") == "workflow" - else "entrypoint" - ) - source_path = agent.get(source_field) - if not isinstance(source_path, str) or not source_path: - missing_sources.append(f"{agent['name']}: missing `{source_field}`") - elif not os.path.isfile(os.path.join(source_root, source_path)): - missing_sources.append(f"{agent['name']}: {source_path} not found") - if missing_sources: - raise RuntimeError( - "Cannot build configured sources: " + "; ".join(missing_sources) - ) - # -------------------------------------------------------------- # # Step 1: Discover agent YAML files and generate Python stubs # # -------------------------------------------------------------- # - declarations_dir = os.path.join(artifact_root, "config" if prefix else "agents") stubs_dir = os.path.join(artifact_root, "stubs") os.makedirs(stubs_dir, exist_ok=True) @@ -352,15 +393,8 @@ def _run_build(config_path): if not yaml_files: logger.warning("No agent YAML files found in %s", declarations_dir) - import yaml - # Looks up a config entry's YAML and to map stubs to entrypoints. - yaml_by_name = {} - for yaml_path in yaml_files: - with open(yaml_path) as f: - name = yaml.safe_load(f).get("agent", {}).get("name") - if name: - yaml_by_name[name] = yaml_path + yaml_by_name = declarations_by_name(declarations_dir) # Maps each generated stub's basename to its agent's entrypoint path, which # is the single location the stub is written to and copied to. @@ -430,10 +464,7 @@ def _run_build(config_path): # No build: pull the declared image and tag it like any other # agent image so the rest of the deploy pipeline treats it the # same way (EC2 image transfer, etc.) without further changes. - image = agent_cfg.get("image") - if not image: - logger.warning("Skipping database '%s': no image specified", agent_name) - continue + image = agent_cfg["image"] target_image = f"canyonos-{agent_name.lower()}" logger.info("Pulling database image '%s' as '%s'", image, target_image) subprocess.run( @@ -442,20 +473,12 @@ def _run_build(config_path): subprocess.run(["docker", "tag", image, target_image], check=True) continue + # Every key read below is one the schema requires and has checked, + # down to the file being on disk -- a service that cannot be built + # fails the deploy rather than dropping quietly out of it. if agent_type == "workflow": # Workflow container - workflow_file = agent_cfg.get("workflow_file") - if not workflow_file: - logger.warning( - "Skipping workflow '%s': no workflow_file specified", agent_name - ) - continue - - workflow_path = os.path.join(source_root, workflow_file) - if not os.path.isfile(workflow_path): - logger.error("Workflow file not found: %s", workflow_path) - continue - + workflow_path = os.path.join(source_root, agent_cfg["workflow_file"]) docker_context = os.path.join(artifact_root, "docker_container", "Workflow") logger.info("Generating workflow Docker context for '%s'", agent_name) generate_workflow_docker( @@ -473,17 +496,7 @@ def _run_build(config_path): else: # Agent container - entrypoint = agent_cfg.get("entrypoint") - if not entrypoint: - logger.warning( - "Skipping agent '%s': no entrypoint specified", agent_name - ) - continue - - agent_file = os.path.join(source_root, entrypoint) - if not os.path.isfile(agent_file): - logger.error("Agent file not found: %s", agent_file) - continue + agent_file = os.path.join(source_root, agent_cfg["entrypoint"]) # Find matching YAML by agent name matching_yaml = yaml_by_name.get(agent_name) diff --git a/packages/core/canyonos_core/controller/controller_context.py b/packages/core/canyonos_core/controller/controller_context.py index 74e94d1d..64ff0d82 100644 --- a/packages/core/canyonos_core/controller/controller_context.py +++ b/packages/core/canyonos_core/controller/controller_context.py @@ -19,15 +19,13 @@ import re import subprocess -import yaml - +from canyonos_core.controller.utils import config_env from canyonos_core.controller.utils.config_specs import read_config_specs from canyonos_core.controller.utils.env_file import resolve_env_file from canyonos_core.controller.utils.redis_client import RedisClient logger = logging.getLogger(__name__) -_RESERVED_ENV_KEYS = frozenset({"CANYONOS_LLM_STUB_TEXT"}) _REMOTE_COPY_PATH = re.compile( r"(/[A-Za-z0-9_-][A-Za-z0-9_.-]*)+/canyonos\.[A-Za-z0-9]+/env" ) @@ -108,54 +106,16 @@ def __init__(self, config_path): @staticmethod def _load_config(config_path): """Load the YAML config, importing root .env values and expanding ${VAR} refs.""" - project_root = os.path.abspath(os.path.join(os.path.dirname(config_path), "..")) - # Under the .car layout the parent-of-parent lands on .car itself, not the root. - if os.path.basename(project_root) == ".car": - project_root = os.path.dirname(project_root) - ControllerContext._load_dotenv(os.path.join(project_root, ".env")) - with open(config_path, "r") as f: - config = yaml.safe_load(f) + config = config_env.load_config(config_path) if not isinstance(config, dict): raise RuntimeError(f"Config must contain a YAML mapping: {config_path}") - return ControllerContext._expand_env_value(config) - - @staticmethod - def _load_dotenv(path): - """Load simple KEY=VALUE entries without overriding existing environment values.""" - if not os.path.isfile(path): - return - with open(path, "r") as f: - for line in f: - line = line.strip() - if not line or line.startswith("#") or "=" not in line: - continue - key, value = line.split("=", 1) - key = key.strip() - value = value.strip() - if len(value) >= 2 and value[0] == value[-1] and value[0] in {"'", '"'}: - value = value[1:-1] - if key in _RESERVED_ENV_KEYS: - continue - if key and key not in os.environ: - os.environ[key] = value + return config - @staticmethod - def _expand_env_value(value): - """Replace every ${VAR} in the config with its environment value, leaving unset ones as written.""" - if isinstance(value, str): - return re.sub( - r"\$\{([A-Za-z_][A-Za-z0-9_]*)\}", - lambda m: os.environ.get(m.group(1), m.group(0)), - value, - ) - if isinstance(value, dict): - return { - key: ControllerContext._expand_env_value(item) - for key, item in value.items() - } - if isinstance(value, list): - return [ControllerContext._expand_env_value(item) for item in value] - return value + # Kept as aliases for callers that use these helpers directly. The parser + # itself lives in config_env so the controller, reconciler and schema use + # the same dotenv and expansion rules. + _load_dotenv = staticmethod(config_env.load_dotenv) + _expand_env_value = staticmethod(config_env.expand_env_value) def _set_controllers(self, agents): """Set the agent spec list and its by-name index together so they can't drift.""" diff --git a/packages/core/canyonos_core/controller/global_controller.py b/packages/core/canyonos_core/controller/global_controller.py index a678f697..3b0816af 100644 --- a/packages/core/canyonos_core/controller/global_controller.py +++ b/packages/core/canyonos_core/controller/global_controller.py @@ -33,6 +33,7 @@ from canyonos_core.controller.utils.redis_utils import _wait_for_redis from canyonos_core.controller.utils.redis_client import RedisClient from canyonos_core.controller.utils.grpc_options import GRPC_CHANNEL_OPTIONS +from canyonos_core.schema.yaml_lines import load_yaml_lines # Add generated grpc_stubs from the local project to the path. Projects using # the .car artifact layout keep grpc_stubs under .car/; older/plain layouts @@ -239,10 +240,24 @@ def _load_config(config_path): @staticmethod def _assign_new_project_id(config_path): - """Generate a project_id and append it to the config file so it stays stable across reloads/restarts.""" + """Generate a project_id and write it to the config file so it stays stable across reloads/restarts. + + A `project_id:` already in the file with no value is replaced in place; + appending a second one would make the file a duplicate-key error. + """ project_id = str(uuid.uuid4()) - with open(config_path, "a") as f: - f.write(f'project_id: "{project_id}"\n') + entry = f'project_id: "{project_id}"\n' + document = load_yaml_lines(config_path) + with open(config_path, "r") as f: + lines = f.readlines() + if isinstance(document, dict) and "project_id" in document: + lines[document.key_lines["project_id"] - 1] = entry + else: + if lines and not lines[-1].endswith("\n"): + lines[-1] += "\n" + lines.append(entry) + with open(config_path, "w") as f: + f.writelines(lines) return project_id @staticmethod diff --git a/packages/core/canyonos_core/controller/utils/config_env.py b/packages/core/canyonos_core/controller/utils/config_env.py new file mode 100644 index 00000000..666218f5 --- /dev/null +++ b/packages/core/canyonos_core/controller/utils/config_env.py @@ -0,0 +1,74 @@ +"""The `.env` import and `${VAR}` expansion the config goes through before it is read. + +Shared so the manifest schema checks exactly the values the Global Controller +will act on. Expansion is textual: `${API_PORT}` becomes the variable's text +and stays a string, whatever it holds. That is why the schema supports +references in string-typed fields only -- a reference in a numeric one would +pass here and be misread at runtime, where `replicas` of `"3"` starts a single +replica rather than three. +""" + +import os +import re + +# Internal controls a user's .env must never be able to set. +RESERVED_ENV_KEYS = frozenset({"CANYONOS_LLM_STUB_TEXT"}) + +ENV_REF = re.compile(r"\$\{([A-Za-z_][A-Za-z0-9_]*)\}") + + +def load_dotenv(path): + """Load simple KEY=VALUE entries without overriding existing environment values.""" + if not os.path.isfile(path): + return + with open(path, "r") as f: + for line in f: + line = line.strip() + if not line or line.startswith("#") or "=" not in line: + continue + key, value = line.split("=", 1) + key = key.strip() + value = value.strip() + if len(value) >= 2 and value[0] == value[-1] and value[0] in {"'", '"'}: + value = value[1:-1] + if key in RESERVED_ENV_KEYS: + # Reserved internal control -- never honor it from user .env. + continue + if key and key not in os.environ: + os.environ[key] = value + + +def expand_env_value(value): + """Substitute `${VAR}` refs throughout a parsed config, leaving unset ones alone.""" + if isinstance(value, str): + return ENV_REF.sub(lambda m: os.environ.get(m.group(1), m.group(0)), value) + if isinstance(value, dict): + return {key: expand_env_value(item) for key, item in value.items()} + if isinstance(value, list): + return [expand_env_value(item) for item in value] + return value + + +def project_root_for_config(config_path): + """The project root a config file belongs to -- where its `.env` lives.""" + project_root = os.path.abspath(os.path.join(os.path.dirname(config_path), "..")) + # Under the .car layout, config lives at /.car/config, so the + # naive parent-of-parent lands on .car itself -- go up one more level + # to reach the actual project root where .env lives. + if os.path.basename(project_root) == ".car": + project_root = os.path.dirname(project_root) + return project_root + + +def load_root_dotenv(config_path): + """Import the project `.env` that sits next to a config file's project root.""" + load_dotenv(os.path.join(project_root_for_config(config_path), ".env")) + + +def load_config(config_path): + """The config as every reader acts on it: root `.env` imported, `${VAR}` expanded.""" + import yaml + + load_root_dotenv(config_path) + with open(config_path, "r") as f: + return expand_env_value(yaml.safe_load(f)) diff --git a/packages/core/canyonos_core/otlp_exporter/otel_exporter.py b/packages/core/canyonos_core/otlp_exporter/otel_exporter.py index 89bb2f67..20f20b4b 100644 --- a/packages/core/canyonos_core/otlp_exporter/otel_exporter.py +++ b/packages/core/canyonos_core/otlp_exporter/otel_exporter.py @@ -2,7 +2,6 @@ import json import logging -import math import os import signal import sqlite3 @@ -45,6 +44,12 @@ import log_convert import otel_reader from canyonos_core.controller.utils.schema import DB_PATH, init_db +from canyonos_core.schema.otel_destinations import ( + destination_problems, + destinations_problem, + duplicate_name_message, + normalize_destination, +) logging.basicConfig(level=logging.INFO) logger = logging.getLogger(__name__) @@ -69,7 +74,6 @@ TRACE_RETENTION_SECONDS = 30 * 60 METRIC_RETENTION_SECONDS = 10 * 60 LOG_RETENTION_SECONDS = 30 * 60 -SUPPORTED_PROTOCOLS = ("grpc", "http", "http/protobuf") # Separate empty-queue trackers per signal: {"consecutive": int, "warned_at": int|None}. _trace_empty_queue = {"consecutive": 0, "warned_at": None} _metric_empty_queue = {"consecutive": 0, "warned_at": None} @@ -99,54 +103,12 @@ class Destination(TypedDict): def _validate_destination(destination, index) -> Destination: if not isinstance(destination, dict): raise ValueError(f"destination {index} must be an object") - - name = destination.get("name") - if not isinstance(name, str) or not name.strip(): - raise ValueError(f"destination {index} name must be a non-empty string") - - protocol = destination.get("protocol") - protocol = protocol.lower() if isinstance(protocol, str) else protocol - if protocol not in SUPPORTED_PROTOCOLS: - raise ValueError( - f"destination {name!r} protocol must be one of " - f"{list(SUPPORTED_PROTOCOLS)}; got {protocol!r}" - ) - endpoint = destination.get("endpoint") - if not isinstance(endpoint, str) or not endpoint.strip(): - raise ValueError(f"destination {name!r} endpoint must be a non-empty string") - - headers = destination.get("headers") - if headers is not None: - if not isinstance(headers, dict): - raise ValueError(f"destination {name!r} headers must be an object") - if any( - not isinstance(key, str) or not key.strip() or not isinstance(value, str) - for key, value in headers.items() - ): - raise ValueError( - f"destination {name!r} headers must map non-empty strings to strings" - ) - headers = dict(headers) - - insecure = destination.get("insecure") - if insecure is not None and not isinstance(insecure, bool): - raise ValueError(f"destination {name!r} insecure must be a boolean") - - timeout = destination.get("timeout") - if timeout is not None: - if isinstance(timeout, bool) or not isinstance(timeout, (int, float)): - raise ValueError(f"destination {name!r} timeout must be a positive number") - if not math.isfinite(timeout) or timeout <= 0: - raise ValueError(f"destination {name!r} timeout must be a positive number") - - return { - "name": name.strip(), - "protocol": cast(DestinationProtocol, protocol), - "endpoint": endpoint.strip(), - "headers": headers, - "insecure": insecure, - "timeout": float(timeout) if timeout is not None else None, - } + problems = list(destination_problems(destination)) + if problems: + name = destination.get("name") + label = repr(name) if isinstance(name, str) and name.strip() else index + raise ValueError(f"destination {label} {problems[0][1]}") + return cast(Destination, normalize_destination(destination)) def _configured_destinations(raw): @@ -157,8 +119,9 @@ def _configured_destinations(raw): destinations = json.loads(raw) except (TypeError, json.JSONDecodeError) as exc: raise ValueError(f"{DESTINATIONS_KEY} must contain a JSON list") from exc - if not isinstance(destinations, list) or not destinations: - raise ValueError(f"{DESTINATIONS_KEY} must contain a non-empty JSON list") + problem = destinations_problem(destinations) + if problem: + raise ValueError(f"{DESTINATIONS_KEY} {problem}") validated = [] names = set() @@ -166,7 +129,7 @@ def _configured_destinations(raw): validated_destination = _validate_destination(destination, index) name = validated_destination["name"] if name in names: - raise ValueError(f"destination names must be unique; duplicate {name!r}") + raise ValueError(duplicate_name_message(name)) names.add(name) validated.append(validated_destination) return validated diff --git a/packages/core/canyonos_core/schema/__init__.py b/packages/core/canyonos_core/schema/__init__.py new file mode 100644 index 00000000..3affcd92 --- /dev/null +++ b/packages/core/canyonos_core/schema/__init__.py @@ -0,0 +1,214 @@ +"""Schema for the files a CanyonOS project declares itself with. + +`validate_project` is what the in-container deploy calls before it generates a +single stub: a manifest key that does not exist, a port written as a string or +an argument annotated with a type the stub cannot import is a failure with a +file and a line, not a warning followed by a build. +""" + +import glob +import os + +import yaml + +from canyonos_core.schema.agent_yaml import ( + BUILTIN_TYPE_NAMES, + AgentDeclaration, + ArgumentDecl, + FunctionDecl, + ReturnsDecl, + load_agent_declaration, +) +from canyonos_core.schema.errors import ( + DependencyPinConflict, + SchemaError, + SchemaViolation, + render_violation, +) +from canyonos_core.schema.manifest import ( + AgentService, + DatabaseService, + Ec2Spec, + Manifest, + OtelDestination, + OtelSpec, + RedisSpec, + Resources, + WorkflowService, + load_manifest, +) + +__all__ = [ + "AgentDeclaration", + "AgentService", + "ArgumentDecl", + "BUILTIN_TYPE_NAMES", + "DatabaseService", + "DependencyPinConflict", + "Ec2Spec", + "FunctionDecl", + "Manifest", + "OtelDestination", + "OtelSpec", + "RedisSpec", + "Resources", + "ReturnsDecl", + "SchemaError", + "SchemaViolation", + "WorkflowService", + "check_project", + "declarations_by_name", + "load_agent_declaration", + "load_manifest", + "render_violation", + "validate_project", +] + + +def _declared_name(path): + """The `agent.name` a YAML in the declarations directory claims, if any. + + Returns None for a file that is not an agent declaration at all: the + directory also holds the manifest itself under the .car layout, and the + build skips those the same way. + """ + try: + with open(path, "r", encoding="utf-8") as f: + document = yaml.safe_load(f) + except (OSError, yaml.YAMLError): + # A file that will not parse is reported by load_agent_declaration. + return "" + if not isinstance(document, dict) or "agent" not in document: + return None + block = document["agent"] + name = block.get("name") if isinstance(block, dict) else None + return name if isinstance(name, str) else "" + + +def _as_typed(path): + """A path as the reader would write it: relative to where the build runs. + + The build runs from the project root, so that is the form the manifest and + the rest of these violations are written in. A path outside it has no + shorter honest form and is left absolute. + """ + try: + relative = os.path.relpath(path) + except ValueError: # a different drive on Windows + return path + return path if relative.startswith(os.pardir) else relative + + +def _missing_sources(manifest, manifest_path, source_dir): + """Violations for the code a manifest points at but the project does not hold. + + The build used to log a line and carry on, leaving a deploy that exited 0 + with the service quietly absent from it. + """ + if not os.path.isdir(source_dir): + return [ + SchemaViolation( + manifest_path, + 0, + "agents", + f"the project source directory {_as_typed(source_dir)} does not " + "exist, so no service's code can be found", + ) + ] + + violations = [] + real_source_dir = os.path.realpath(source_dir) + for index, service in enumerate(manifest.agents): + key = {"agent": "entrypoint", "workflow": "workflow_file"}.get(service.type) + if key is None: + continue + source = os.path.join(source_dir, getattr(service, key)) + if not os.path.isfile(source): + problem = f"{_as_typed(source)} does not exist" + elif ( + os.path.commonpath([real_source_dir, os.path.realpath(source)]) + != real_source_dir + ): + problem = f"{_as_typed(source)} resolves outside the project" + else: + continue + violations.append( + SchemaViolation(manifest_path, 0, f"agents[{index}].{key}", problem) + ) + return violations + + +def _scan_declarations(declarations_dir): + """`({agent.name: path}, names claimed, violations)` for a declarations directory.""" + by_name = {} + claimed = set() + violations = [] + for path in sorted(glob.glob(os.path.join(declarations_dir, "*.yaml"))): + name = _declared_name(path) + if name is None: + continue + # A declaration that fails its own checks still claims its name, so the + # agent it belongs to is not also reported as having none. + claimed.add(name) + try: + declaration = load_agent_declaration(path) + except SchemaError as error: + violations.extend(error.violations) + continue + claimed.add(declaration.name) + by_name[declaration.name] = path + return by_name, claimed, violations + + +def declarations_by_name(declarations_dir): + """Each valid agent declaration's path, keyed by the `agent.name` it sets.""" + return _scan_declarations(declarations_dir)[0] + + +def check_project(manifest_path, declarations_dir, source_dir=None): + """`(manifest, violations)` for a project's manifest and agent declarations. + + Never raises: the caller decides how to report. The manifest is None when + it did not parse. The manifest and the + declarations are checked independently so one broken file does not hide + the others; only the last step, binding each agent to the declaration that + names it, needs a manifest that parsed. + + `source_dir` is the project root the manifest's paths are relative to. + Given one, the entrypoint and workflow file each service declares must + exist under it; left out, nothing on disk is checked. + """ + manifest = None + violations = [] + try: + manifest = load_manifest(manifest_path) + except SchemaError as error: + violations.extend(error.violations) + + _, declared, declaration_violations = _scan_declarations(declarations_dir) + violations.extend(declaration_violations) + + if manifest is None: + return None, tuple(violations) + + for index, service in enumerate(manifest.agents): + if service.type != "agent" or service.name in declared: + continue + violations.append( + SchemaViolation( + manifest_path, + 0, + f"agents[{index}].name", + f"no agent declaration in {declarations_dir} sets " + f"agent.name: {service.name}; the stub cannot be generated", + ) + ) + + if source_dir is not None: + violations.extend(_missing_sources(manifest, manifest_path, source_dir)) + return manifest, tuple(violations) + + +def validate_project(manifest_path, declarations_dir, source_dir=None): + """Every violation in a project's manifest and agent declarations.""" + return check_project(manifest_path, declarations_dir, source_dir)[1] diff --git a/packages/core/canyonos_core/schema/_checks.py b/packages/core/canyonos_core/schema/_checks.py new file mode 100644 index 00000000..5b805b04 --- /dev/null +++ b/packages/core/canyonos_core/schema/_checks.py @@ -0,0 +1,295 @@ +"""Field readers shared by the manifest and the agent declaration schemas. + +Each reader takes the mapping a key lives in, records a violation when the +value is not what the schema declares, and hands back the declared default so +checking carries on and the caller sees every problem at once. + +`${VAR}` references are expanded on the way through, but only string-typed +fields support them: the Global Controller's expansion produces text, so a +reference in a numeric or boolean field would reach the runtime as a string +and be silently misread (a `replicas` of `"3"` starts one replica). Those are +rejected here instead. +""" + +import difflib +import math + +from canyonos_core.controller.utils.config_env import ENV_REF, expand_env_value +from canyonos_core.schema.errors import SchemaViolation +from canyonos_core.schema.yaml_lines import line_of + +_ENV_REF_HINT = " (environment references are only supported in string fields)" + + +class _Collector: + """Accumulates every violation in one file instead of stopping at the first.""" + + def __init__(self, path): + self.path = path + self.violations = [] + + def add(self, node, key, field_name, message): + self.violations.append( + SchemaViolation(self.path, line_of(node, key), field_name, message) + ) + + +def _field(prefix, key): + return f"{prefix}.{key}" if prefix else str(key) + + +def _describe(value): + """Quote a value and name its kind, so the message shows what was written.""" + if value is None: + return "nothing" + if isinstance(value, bool): + return f"the boolean {str(value).lower()}" + if isinstance(value, (int, float)): + return f"the number {value!r}" + if isinstance(value, str): + return f"the string {value!r}" + if isinstance(value, list): + return f"the list {value!r}" + if isinstance(value, dict): + return "a mapping" + return f"{value!r}" + + +def _resolve(raw): + """Expand `${VAR}` refs in a scalar. An unset variable is left literal, + which is what the Global Controller does with it too.""" + if not isinstance(raw, str): + return raw + return expand_env_value(raw) + + +def _describe_rejected(raw, value): + """How to name a rejected value, as the reference it was written as if it is one. + + An expansion can hide what the author typed -- an empty variable reads back + as `''` -- so the message quotes the reference instead. + """ + if isinstance(raw, str) and ENV_REF.search(raw): + return f"{raw!r}{_ENV_REF_HINT}" + return _describe(value) + + +def _unknown_key_message(key, allowed): + message = f"unknown key {key!r}" + close = difflib.get_close_matches(str(key), sorted(allowed), n=1) + if close: + message += f" (did you mean {close[0]!r}?)" + return message + + +def _check_keys(collector, node, prefix, allowed): + """Report every key in `node` the schema does not declare.""" + for key in node: + if key in allowed: + continue + collector.add( + node, key, _field(prefix, key), _unknown_key_message(key, allowed) + ) + + +def _mapping(collector, node, key, prefix, allowed): + """Return the mapping at `key`, or None when it is absent or not a mapping.""" + if key not in node or node[key] is None: + return None + value = node[key] + if not isinstance(value, dict): + collector.add( + node, + key, + _field(prefix, key), + f"expected a mapping, got {_describe(value)}", + ) + return None + _check_keys(collector, value, _field(prefix, key), allowed) + return value + + +def _integer(collector, node, key, prefix, default, minimum=None, maximum=None): + if key not in node: + return default + raw = node[key] + value = _resolve(raw) + if minimum is not None and maximum is not None: + bound = f" between {minimum} and {maximum}" + elif minimum is not None: + bound = f" >= {minimum}" + else: + bound = "" + wrong_type = isinstance(value, bool) or not isinstance(value, int) + out_of_range = not wrong_type and ( + (minimum is not None and value < minimum) + or (maximum is not None and value > maximum) + ) + if wrong_type or out_of_range: + collector.add( + node, + key, + _field(prefix, key), + f"expected an integer{bound}, got {_describe_rejected(raw, value)}", + ) + return default + return value + + +def _port(collector, node, key, prefix, default): + """A TCP port: an integer from 1 to 65535.""" + return _integer(collector, node, key, prefix, default, 1, 65535) + + +def _as_finite_float(value): + """`value` as a finite float, or None. + + YAML spells infinity and NaN as `.inf` and `.nan`, and an integer too large + for a float raises OverflowError on conversion; none of them is a number + any field here can use. + """ + if isinstance(value, bool) or not isinstance(value, (int, float)): + return None + try: + number = float(value) + except OverflowError: + return None + return number if math.isfinite(number) else None + + +def _number(collector, node, key, prefix, default, minimum=None): + """A field that accepts a fraction, unlike the integer counts and ports.""" + if key not in node: + return default + raw = node[key] + value = _resolve(raw) + bound = f" > {minimum}" if minimum is not None else "" + number = _as_finite_float(value) + if number is None or (minimum is not None and number <= minimum): + collector.add( + node, + key, + _field(prefix, key), + f"expected a finite number{bound}, got {_describe_rejected(raw, value)}", + ) + return default + return number + + +def _unset_reference(value): + """True when expansion left a `${VAR}` in place because VAR is not set.""" + return isinstance(value, str) and ENV_REF.search(value) is not None + + +def _unset_message(raw): + return f"{raw!r} names an environment variable that is not set" + + +def _string(collector, node, key, prefix, default=None, required=False): + if key not in node or node[key] is None: + if required: + collector.add(node, key, _field(prefix, key), "is required but missing") + return default + raw = node[key] + value = _resolve(raw) + if required and _unset_reference(value): + collector.add(node, key, _field(prefix, key), _unset_message(raw)) + return default + if not isinstance(value, str) or not value.strip(): + collector.add( + node, + key, + _field(prefix, key), + f"expected a non-empty string, got {_describe_rejected(raw, value)}", + ) + return default + return value + + +def _boolean(collector, node, key, prefix, default): + if key not in node: + return default + raw = node[key] + value = _resolve(raw) + if not isinstance(value, bool): + collector.add( + node, + key, + _field(prefix, key), + f"expected a boolean, got {_describe_rejected(raw, value)}", + ) + return default + return value + + +def _string_list(collector, node, key, prefix, required=False): + if key not in node or node[key] is None: + if required: + collector.add(node, key, _field(prefix, key), "is required but missing") + return () + value = node[key] + if not isinstance(value, list): + collector.add( + node, + key, + _field(prefix, key), + f"expected a list of strings, got {_describe(value)}", + ) + return () + if required and not value: + # A required list is required to say something; `[]` is as missing as + # no key at all. + collector.add( + node, key, _field(prefix, key), "is required and must not be empty" + ) + return () + items = [_resolve(item) for item in value] + for index, (raw, item) in enumerate(zip(value, items)): + if required and _unset_reference(item): + collector.add(value, index, _field(prefix, key), _unset_message(raw)) + return () + if isinstance(item, str) and item.strip(): + continue + collector.add( + value, + index, + _field(prefix, key), + f"expected a list of strings, got {_describe_rejected(raw, item)}", + ) + return () + return tuple(items) + + +def _string_mapping(collector, node, key, prefix): + if key not in node or node[key] is None: + return {} + value = node[key] + if not isinstance(value, dict): + collector.add( + node, + key, + _field(prefix, key), + f"expected a mapping, got {_describe(value)}", + ) + return {} + resolved = {} + for name, raw in value.items(): + item = _resolve(raw) + if not isinstance(name, str): + collector.add( + value, + name, + _field(prefix, key), + f"expected string keys, got the key {_describe(name)}", + ) + return {} + if not isinstance(item, (str, int, float)): + collector.add( + value, + name, + _field(_field(prefix, key), name), + f"expected a string, got {_describe_rejected(raw, item)}", + ) + return {} + resolved[name] = str(item) + return resolved diff --git a/packages/core/canyonos_core/schema/agent_yaml.py b/packages/core/canyonos_core/schema/agent_yaml.py new file mode 100644 index 00000000..314d820f --- /dev/null +++ b/packages/core/canyonos_core/schema/agent_yaml.py @@ -0,0 +1,316 @@ +"""The agent declaration schema: the YAML a stub is generated from. + +The stub generator builds from what `load_agent_declaration` returns, so every +name here becomes Python source: the agent is a class, each function a method +and each argument a parameter. An argument's `type` becomes its annotation, +and the stub imports nothing, so a type may only be built from builtins. +""" + +import ast +import builtins +import keyword +from dataclasses import dataclass + +import yaml + +from canyonos_core.schema._checks import ( + _check_keys, + _Collector, + _describe, + _field, + _string, +) +from canyonos_core.schema.errors import SchemaError, SchemaViolation +from canyonos_core.schema.yaml_lines import line_of, load_yaml_lines, parse_failure + +# Names the generated stub can annotate with, given it imports nothing. +BUILTIN_TYPE_NAMES = frozenset( + { + "bool", + "bytearray", + "bytes", + "complex", + "dict", + "float", + "frozenset", + "int", + "list", + "object", + "set", + "str", + "tuple", + } +) + +_BUILTIN_TYPES = {name: getattr(builtins, name) for name in BUILTIN_TYPE_NAMES} + +# Nodes an annotation may be built from: `list[str]`, `dict[str, int]`, +# `tuple[int, ...]`, `int | None`. +_ANNOTATION_NODES = ( + ast.Expression, + ast.Name, + ast.Load, + ast.Subscript, + ast.Tuple, + ast.BinOp, + ast.BitOr, +) + +# Module-level names every generated method body reads; an agent class or an +# argument by one of these names would shadow it. +STUB_GLOBAL_NAMES = frozenset({"Future", "inspect", "isinstance"}) + +_DOCUMENT_KEYS = frozenset({"agent"}) +_DECLARATION_KEYS = frozenset({"name", "functions"}) +_FUNCTION_KEYS = frozenset({"name", "description", "arguments", "returns"}) +_ARGUMENT_KEYS = frozenset({"name", "type"}) +_RETURNS_KEYS = frozenset({"type"}) + + +@dataclass(frozen=True) +class ArgumentDecl: + name: str + type: str | None = None + + +@dataclass(frozen=True) +class ReturnsDecl: + type: str | None = None + + +@dataclass(frozen=True) +class FunctionDecl: + name: str + description: str = "" + arguments: tuple = () + returns: ReturnsDecl | None = None + + +@dataclass(frozen=True) +class AgentDeclaration: + name: str + functions: tuple = () + path: str = "" + + +def is_builtin_annotation(type_name): + """True when `type_name` evaluates with nothing imported.""" + try: + tree = ast.parse(type_name, mode="eval") + except SyntaxError: + return False + for node in ast.walk(tree): + if isinstance(node, ast.Constant): + if node.value is not None and node.value is not Ellipsis: + return False + elif not isinstance(node, _ANNOTATION_NODES): + return False + elif isinstance(node, ast.Name) and node.id not in BUILTIN_TYPE_NAMES: + return False + # Only builtin type names reach here, so evaluating is safe; it rejects + # what parses but fails when the stub's `def` runs, such as `int[str]`. + try: + eval(compile(tree, "", "eval"), {"__builtins__": _BUILTIN_TYPES}) + except TypeError: + return False + return True + + +def _identifier(collector, node, key, prefix, required=False, reserved=()): + """A name the generated stub writes as Python source.""" + value = _string(collector, node, key, prefix, required=required) + if value is None: + return None + if not value.isidentifier() or keyword.iskeyword(value): + collector.add( + node, + key, + _field(prefix, key), + f"{value!r} is not a valid Python identifier; the generated stub " + "uses it as a name", + ) + return None + if value in reserved: + collector.add( + node, + key, + _field(prefix, key), + f"{value!r} would shadow the name the generated stub uses for " + "itself; choose another", + ) + return None + return value + + +def _returns(collector, node, prefix): + if node.get("returns") is None: + return None + block = node["returns"] + if not isinstance(block, dict): + collector.add( + node, + "returns", + _field(prefix, "returns"), + f"expected a mapping, got {_describe(block)}", + ) + return None + _check_keys(collector, block, _field(prefix, "returns"), _RETURNS_KEYS) + return ReturnsDecl( + type=_string(collector, block, "type", _field(prefix, "returns")) + ) + + +def _arguments(collector, node, prefix): + raw = node.get("arguments") + if raw is None: + return () + if not isinstance(raw, list): + collector.add( + node, + "arguments", + _field(prefix, "arguments"), + f"expected a list of arguments, got {_describe(raw)}", + ) + return () + + arguments = [] + seen = set() + for index, entry in enumerate(raw): + argument_prefix = f"{prefix}.arguments[{index}]" + if not isinstance(entry, dict): + collector.add( + raw, + index, + argument_prefix, + f"expected a mapping, got {_describe(entry)}", + ) + continue + _check_keys(collector, entry, argument_prefix, _ARGUMENT_KEYS) + name = _identifier( + collector, + entry, + "name", + argument_prefix, + required=True, + reserved=STUB_GLOBAL_NAMES, + ) + if name == "self" or name in seen: + collector.add( + entry, + "name", + _field(argument_prefix, "name"), + f"{name!r} is already a parameter of the generated method", + ) + name = None + type_name = _string(collector, entry, "type", argument_prefix) + if type_name is not None and not is_builtin_annotation(type_name): + collector.add( + entry, + "type", + _field(argument_prefix, "type"), + f"{type_name!r} is not built from builtin types; the generated stub " + "imports nothing, so an argument type may only use " + f"{', '.join(sorted(BUILTIN_TYPE_NAMES))} and None", + ) + type_name = None + if name: + seen.add(name) + arguments.append(ArgumentDecl(name=name, type=type_name)) + return tuple(arguments) + + +def _functions(collector, node): + raw = node.get("functions") + if raw is None: + return () + if not isinstance(raw, list): + collector.add( + node, + "functions", + "agent.functions", + f"expected a list of functions, got {_describe(raw)}", + ) + return () + + functions = [] + for index, entry in enumerate(raw): + prefix = f"agent.functions[{index}]" + if not isinstance(entry, dict): + collector.add( + raw, index, prefix, f"expected a mapping, got {_describe(entry)}" + ) + continue + _check_keys(collector, entry, prefix, _FUNCTION_KEYS) + name = _identifier(collector, entry, "name", prefix, required=True) + description = entry.get("description") + if description is not None and not isinstance(description, str): + collector.add( + entry, + "description", + _field(prefix, "description"), + f"expected a string, got {_describe(description)}", + ) + description = None + arguments = _arguments(collector, entry, prefix) + returns = _returns(collector, entry, prefix) + if name: + functions.append( + FunctionDecl( + name=name, + description=description or "", + arguments=arguments, + returns=returns, + ) + ) + return tuple(functions) + + +def load_agent_declaration(path): + """Parse and check one agent YAML, reporting every problem at once. + + Raises: + SchemaError: the declaration is unusable; `.violations` holds them all. + """ + collector = _Collector(path) + try: + document = load_yaml_lines(path) + except OSError as exc: + raise SchemaError( + [SchemaViolation(path, 0, "", f"cannot be read: {exc}")] + ) from exc + except yaml.YAMLError as exc: + line, detail = parse_failure(exc) + raise SchemaError( + [SchemaViolation(path, line, "", f"is not valid YAML: {detail}")] + ) from exc + + if not isinstance(document, dict): + raise SchemaError( + [ + SchemaViolation( + path, 0, "", f"expected a mapping, got {_describe(document)}" + ) + ] + ) + + _check_keys(collector, document, "", _DOCUMENT_KEYS) + block = document.get("agent") + if not isinstance(block, dict): + collector.violations.append( + SchemaViolation( + path, + line_of(document, "agent"), + "agent", + f"is required and must be a mapping, got {_describe(block)}", + ) + ) + raise SchemaError(collector.violations) + + _check_keys(collector, block, "agent", _DECLARATION_KEYS) + name = _identifier( + collector, block, "name", "agent", required=True, reserved=STUB_GLOBAL_NAMES + ) + functions = _functions(collector, block) + if collector.violations: + raise SchemaError(collector.violations) + return AgentDeclaration(name=name or "", functions=functions, path=path) diff --git a/packages/core/canyonos_core/schema/errors.py b/packages/core/canyonos_core/schema/errors.py new file mode 100644 index 00000000..bcfde605 --- /dev/null +++ b/packages/core/canyonos_core/schema/errors.py @@ -0,0 +1,43 @@ +"""Violations the schema reports, and the one-line form the deploy log prints. + +The host CLI scrapes the in-container deploy's output and treats the *first* +line of a fatal message as the root cause, so every violation has to render as +a single self-contained line. +""" + +from typing import NamedTuple + + +class SchemaViolation(NamedTuple): + """One rejected field: where it is written and what is wrong with it.""" + + path: str + line: int + field: str + message: str + + +def render_violation(violation): + """Render a violation as `path:line: field: message`, on one line. + + The line is left out when it is not known, and so is the field when the + problem is the file as a whole rather than one key in it. + """ + location = violation.path + if location and violation.line: + location = f"{location}:{violation.line}" + parts = [part for part in (location, violation.field) if part] + parts.append(violation.message) + return ": ".join(parts) + + +class SchemaError(Exception): + """Every violation found in one file, raised once instead of one at a time.""" + + def __init__(self, violations): + self.violations = tuple(violations) + super().__init__("; ".join(render_violation(v) for v in self.violations)) + + +class DependencyPinConflict(SchemaError): + """An app pinned a package below the version the platform image is built on.""" diff --git a/packages/core/canyonos_core/schema/manifest.py b/packages/core/canyonos_core/schema/manifest.py new file mode 100644 index 00000000..4fad05b8 --- /dev/null +++ b/packages/core/canyonos_core/schema/manifest.py @@ -0,0 +1,659 @@ +"""The manifest schema: every key `global_controller.yaml` may carry. + +Anything not declared here is rejected, so a typo fails the deploy with a line +number instead of being silently ignored, and a wrong type fails here rather +than deep inside the instance manager once containers are already up. +""" + +import ntpath +import posixpath +from dataclasses import dataclass, field +from typing import ClassVar + +import yaml +from packaging.requirements import InvalidRequirement, Requirement + +from canyonos_core.controller.utils.config_env import ( + ENV_REF, + expand_env_value, + load_root_dotenv, +) +from canyonos_core.schema._checks import ( + _ENV_REF_HINT, + _boolean, + _check_keys, + _Collector, + _describe, + _field, + _integer, + _mapping, + _number, + _port, + _resolve, + _string, + _string_list, + _string_mapping, + _unknown_key_message, +) +from canyonos_core.schema.errors import SchemaError, SchemaViolation +from canyonos_core.schema.otel_destinations import ( + DESTINATION_KEYS, + destination_problems, + destinations_problem, + duplicate_name_message, + normalize_destination, +) +from canyonos_core.schema.yaml_lines import load_yaml_lines, parse_failure + +SERVICE_TYPES = ("agent", "workflow", "database") +PROVIDERS = ("local", "EC2") + +# Keys every service entry may carry, whatever its type. +_COMMON_SERVICE_KEYS = frozenset( + { + "name", + "type", + "provider", + "replicas", + "redis_port", + "resources", + "stateful", + "instance_type", + "env", + "host", + "port", + "host_port", + "user", + } +) +_AGENT_KEYS = _COMMON_SERVICE_KEYS | {"entrypoint", "requirements"} +_WORKFLOW_KEYS = _COMMON_SERVICE_KEYS | { + "workflow_file", + "requirements", + "api_port", + "dashboard_port", +} +_DATABASE_KEYS = _COMMON_SERVICE_KEYS | {"image", "db_port", "volume_path"} +_ALL_SERVICE_KEYS = _AGENT_KEYS | _WORKFLOW_KEYS | _DATABASE_KEYS +_SERVICE_KEYS_BY_TYPE = { + "agent": _AGENT_KEYS, + "workflow": _WORKFLOW_KEYS, + "database": _DATABASE_KEYS, +} + +_MANIFEST_KEYS = frozenset( + { + "agents", + "poll_interval", + "cleanup_interval", + "project_id", + "redis", + "env_file", + "logs", + "otel", + "ec2", + } +) +_REDIS_KEYS = frozenset({"host", "port", "db"}) +# Keys that used to mean something and now do nothing, each with the message +# that tells a user carrying one what to do instead. +_RETIRED_KEYS = { + "database": "is no longer used; telemetry is configured under otel: -- remove it", +} +_OTEL_KEYS = frozenset({"destinations"}) +_EC2_KEYS = frozenset( + { + "region", + "ami_id", + "subnet_id", + "security_group_ids", + "ssh_user", + "ssh_private_key_path", + "public_ip_timeout", + "controller_health_timeout", + } +) +_EC2_REQUIRED_KEYS = ("region", "ami_id", "subnet_id", "security_group_ids", "ssh_user") +_RESOURCE_KEYS = frozenset({"cpu", "memory", "gpu"}) + + +# ------------------------------------------------------------------ # +# Parsed shapes # +# ------------------------------------------------------------------ # + + +@dataclass(frozen=True) +class Resources: + cpu: float = 1 + memory: float = 512 + gpu: float | None = None + + +@dataclass(frozen=True) +class RedisSpec: + host: str = "localhost" + port: int = 6379 + db: int = 0 + + +@dataclass(frozen=True) +class OtelDestination: + name: str + protocol: str + endpoint: str + headers: dict = field(default_factory=dict) + insecure: bool = False + timeout: float | None = None + + +@dataclass(frozen=True) +class OtelSpec: + destinations: tuple = () + + +@dataclass(frozen=True) +class Ec2Spec: + region: str + ami_id: str + subnet_id: str + security_group_ids: tuple + ssh_user: str + ssh_private_key_path: str = "~/.ssh/ventis_ec2" + public_ip_timeout: int = 120 + controller_health_timeout: int = 180 + + +@dataclass(frozen=True) +class _Service: + """Fields shared by every service entry in `agents`.""" + + name: str + provider: str = "local" + replicas: int = 1 + redis_port: int = 6379 + resources: Resources = field(default_factory=Resources) + stateful: bool = False + instance_type: str | None = None + env: dict = field(default_factory=dict) + host: str | None = None + port: int | None = None + host_port: int | None = None + user: str | None = None + + type: ClassVar[str] = "agent" + + +@dataclass(frozen=True) +class AgentService(_Service): + entrypoint: str = "" + requirements: tuple = () + requirement_lines: tuple = field(default=(), compare=False, repr=False) + + type: ClassVar[str] = "agent" + + +@dataclass(frozen=True) +class WorkflowService(_Service): + workflow_file: str = "" + requirements: tuple = () + requirement_lines: tuple = field(default=(), compare=False, repr=False) + api_port: int = 8080 + dashboard_port: int = 8081 + + type: ClassVar[str] = "workflow" + + +@dataclass(frozen=True) +class DatabaseService(_Service): + image: str = "" + db_port: int = 5432 + volume_path: str | None = None + + type: ClassVar[str] = "database" + + +@dataclass(frozen=True) +class Manifest: + agents: tuple + poll_interval: float = 5 + cleanup_interval: float = 10 + project_id: str | None = None + redis: RedisSpec = field(default_factory=RedisSpec) + env_file: str | None = None + # Streams failure and log detail into each future; read as + # `config.get("logs", True)` by both runtimes, hence the default. + logs: bool = True + otel: OtelSpec | None = None + ec2: Ec2Spec | None = None + path: str = "" + + +# ------------------------------------------------------------------ # +# Reading and checking # +# ------------------------------------------------------------------ # + + +def _is_rooted(path): + """True for a path anchored anywhere but the project, on either OS. + + `posixpath` alone takes `\\outside\\agent.py`, `C:\\x` and + `\\\\server\\share` for relative names. The leading-backslash test is + explicit because newer Pythons no longer call a drive-relative `\\x` + absolute. + """ + return ( + posixpath.isabs(path) + or ntpath.isabs(path) + or path.startswith("\\") + or (len(path) > 1 and path[1] == ":") + ) + + +def _project_relative_py(collector, node, key, prefix, required): + """A source path inside the project: relative, no `..`, ending in `.py`.""" + value = _string(collector, node, key, prefix, required=required) + if value is None: + return "" + field_name = _field(prefix, key) + if _is_rooted(value): + collector.add( + node, key, field_name, f"must be relative to the project, got {value!r}" + ) + return "" + if ".." in value.replace("\\", "/").split("/"): + collector.add( + node, + key, + field_name, + f"must not escape the project with '..', got {value!r}", + ) + return "" + if not value.endswith(".py"): + collector.add(node, key, field_name, f"must name a .py file, got {value!r}") + return "" + return value + + +def _item_lines(node, key): + return tuple(getattr(node.get(key), "item_lines", ())) + + +def _requirements(collector, node, prefix): + """Each entry must be one PEP 508 requirement, the form the pin check reads. + + requirements.txt takes the entries verbatim, so an option line such as + `-r deps.txt` or two requirements in one entry installed packages the + platform pin check never saw. + """ + requirements = _string_list(collector, node, "requirements", prefix) + for index, requirement in enumerate(requirements): + try: + Requirement(requirement) + except InvalidRequirement: + collector.add( + node["requirements"], + index, + f"{_field(prefix, 'requirements')}[{index}]", + f"{requirement!r} is not a single PEP 508 requirement; write one " + "package per entry, and a URL as `name @ url`", + ) + return () + return requirements + + +def _resources(collector, node, prefix): + block = _mapping(collector, node, "resources", prefix, _RESOURCE_KEYS) + if block is None: + return Resources() + prefix = _field(prefix, "resources") + gpu = None + if "gpu" in block: + gpu = _number(collector, block, "gpu", prefix, None, 0) + return Resources( + cpu=_number(collector, block, "cpu", prefix, 1, 0), + memory=_number(collector, block, "memory", prefix, 512, 0), + gpu=gpu, + ) + + +def _service_type(collector, node, prefix): + if "type" not in node: + return "agent" + value = _resolve(node["type"]) + if value not in SERVICE_TYPES: + collector.add( + node, + "type", + _field(prefix, "type"), + f"expected one of {list(SERVICE_TYPES)}, got {_describe(value)}", + ) + return None + return value + + +def _provider(collector, node, prefix): + """The provider in any casing, normalized to the spelling the runtimes compare. + + cli._load_config accepts `LOCAL` or `ec2` and rewrites them the same way, so + the gate in front of it has to as well. + """ + if "provider" not in node: + return "local" + value = _resolve(node["provider"]) + canonical = {p.casefold(): p for p in PROVIDERS} + if not isinstance(value, str) or value.casefold() not in canonical: + collector.add( + node, + "provider", + _field(prefix, "provider"), + f"expected one of {list(PROVIDERS)}, got {_describe(value)}", + ) + return "local" + return canonical[value.casefold()] + + +def _service(collector, entries, index): + """Parse one entry of `agents`, or None when it cannot be identified. + + A type or name that is wrong still leaves the rest of the entry checked, + so every problem in it is reported at once. + """ + node = entries[index] + prefix = f"agents[{index}]" + if not isinstance(node, dict): + collector.add( + entries, index, prefix, f"expected a mapping, got {_describe(node)}" + ) + return None + + service_type = _service_type(collector, node, prefix) + _check_service_keys(collector, node, prefix, service_type) + + name = _string(collector, node, "name", prefix, required=True) + common = { + "name": name, + "provider": _provider(collector, node, prefix), + "replicas": _integer(collector, node, "replicas", prefix, 1, 1), + "redis_port": _port(collector, node, "redis_port", prefix, 6379), + "resources": _resources(collector, node, prefix), + "stateful": _boolean(collector, node, "stateful", prefix, False), + "instance_type": _string(collector, node, "instance_type", prefix), + "env": _string_mapping(collector, node, "env", prefix), + "host": _string(collector, node, "host", prefix), + "port": _port(collector, node, "port", prefix, None), + "host_port": _port(collector, node, "host_port", prefix, None), + "user": _string(collector, node, "user", prefix), + } + + if common["provider"] == "EC2" and not common["instance_type"]: + collector.add( + node, + "instance_type", + _field(prefix, "instance_type"), + "is required when provider is 'EC2'", + ) + + if service_type == "workflow": + if common["provider"] == "local" and common["replicas"] > 1: + collector.add( + node, + "replicas", + _field(prefix, "replicas"), + "a local workflow runs as a single replica: every replica " + "would publish the same api_port", + ) + service_class = WorkflowService + fields = { + "workflow_file": _project_relative_py( + collector, node, "workflow_file", prefix, required=True + ), + "requirements": _requirements(collector, node, prefix), + "requirement_lines": _item_lines(node, "requirements"), + "api_port": _port(collector, node, "api_port", prefix, 8080), + "dashboard_port": _port(collector, node, "dashboard_port", prefix, 8081), + } + elif service_type == "database": + if common["replicas"] != 1: + collector.add( + node, + "replicas", + _field(prefix, "replicas"), + f"a database runs as a single container, got {_describe(node['replicas'])}", + ) + service_class = DatabaseService + fields = { + "image": _string(collector, node, "image", prefix, "", required=True) or "", + "db_port": _port(collector, node, "db_port", prefix, 5432), + "volume_path": _string(collector, node, "volume_path", prefix), + } + elif service_type == "agent": + service_class = AgentService + fields = { + "entrypoint": _project_relative_py( + collector, node, "entrypoint", prefix, required=True + ), + "requirements": _requirements(collector, node, prefix), + "requirement_lines": _item_lines(node, "requirements"), + } + else: + return None + + if name is None: + return None + return service_class(**common, **fields) + + +def _check_service_keys(collector, node, prefix, service_type): + allowed = _SERVICE_KEYS_BY_TYPE.get(service_type, _ALL_SERVICE_KEYS) + for key in node: + if key in allowed: + continue + if key in _ALL_SERVICE_KEYS: + message = f"key {key!r} is not valid for type {service_type!r}" + else: + message = _unknown_key_message(key, allowed) + collector.add(node, key, _field(prefix, key), message) + + +def _services(collector, node): + if "agents" not in node or node["agents"] is None: + collector.add(node, "agents", "agents", "is required but missing") + return () + raw = node["agents"] + if not isinstance(raw, list): + collector.add( + node, + "agents", + "agents", + f"expected a list of services, got {_describe(raw)}", + ) + return () + + parsed = [] + for index, entry in enumerate(raw): + service = _service(collector, raw, index) + if service is not None: + parsed.append((index, entry, service)) + + # Two services whose names differ only in case collide: the image tag and + # the container name are both built from `name.lower()`. + seen = {} + for index, entry, service in parsed: + key = service.name.lower() + if key in seen: + collector.add( + entry, + "name", + f"agents[{index}].name", + f"duplicate service name {service.name!r}: agents[{seen[key]}] " + "already claims it (names are lowercased into one image tag)", + ) + else: + seen[key] = index + return tuple(service for _, _, service in parsed) + + +def _otel(collector, node): + block = _mapping(collector, node, "otel", "", _OTEL_KEYS) + if block is None: + return None + raw = block.get("destinations") + if raw is None: + return OtelSpec() + problem = destinations_problem(raw) + if problem: + collector.add(block, "destinations", "otel.destinations", problem) + return OtelSpec() + + destinations = [] + names = set() + for index, entry in enumerate(raw): + prefix = f"otel.destinations[{index}]" + if not isinstance(entry, dict): + collector.add( + raw, index, prefix, f"expected a mapping, got {_describe(entry)}" + ) + continue + _check_keys(collector, entry, prefix, DESTINATION_KEYS) + expanded = expand_env_value(entry) + problems = list(destination_problems(expanded)) + for key, message in problems: + raw_value = entry.get(key) + if isinstance(raw_value, str) and ENV_REF.search(raw_value): + message += _ENV_REF_HINT + collector.add(entry, key, _field(prefix, key), message) + if problems: + continue + destination = normalize_destination(expanded) + if destination["name"] in names: + collector.add( + entry, + "name", + _field(prefix, "name"), + duplicate_name_message(destination["name"]), + ) + continue + names.add(destination["name"]) + destinations.append( + OtelDestination( + name=destination["name"], + protocol=destination["protocol"], + endpoint=destination["endpoint"], + headers=destination["headers"] or {}, + insecure=bool(destination["insecure"]), + timeout=destination["timeout"], + ) + ) + return OtelSpec(destinations=tuple(destinations)) + + +def _ec2(collector, node, services): + block = _mapping(collector, node, "ec2", "", _EC2_KEYS) + if not any(service.provider == "EC2" for service in services): + # Only an EC2 deploy reads the block; a local project may carry one + # whose variables are unset. + return None + if block is None: + # No `ec2:` to point at, so the violation names the file only. + collector.violations.append( + SchemaViolation( + collector.path, + 0, + "ec2", + "is required because a service declares provider 'EC2'; it " + f"must set {', '.join(_EC2_REQUIRED_KEYS)}", + ) + ) + return None + + region = _string(collector, block, "region", "ec2", required=True) + ami_id = _string(collector, block, "ami_id", "ec2", required=True) + subnet_id = _string(collector, block, "subnet_id", "ec2", required=True) + ssh_user = _string(collector, block, "ssh_user", "ec2", required=True) + security_group_ids = _string_list( + collector, block, "security_group_ids", "ec2", required=True + ) + ssh_private_key_path = ( + _string(collector, block, "ssh_private_key_path", "ec2", "~/.ssh/ventis_ec2") + or "~/.ssh/ventis_ec2" + ) + public_ip_timeout = _integer(collector, block, "public_ip_timeout", "ec2", 120, 1) + controller_health_timeout = _integer( + collector, block, "controller_health_timeout", "ec2", 180, 1 + ) + if not (region and ami_id and subnet_id and ssh_user and security_group_ids): + return None + return Ec2Spec( + region=region, + ami_id=ami_id, + subnet_id=subnet_id, + security_group_ids=security_group_ids, + ssh_user=ssh_user, + ssh_private_key_path=ssh_private_key_path, + public_ip_timeout=public_ip_timeout, + controller_health_timeout=controller_health_timeout, + ) + + +def load_manifest(path): + """Parse and check `global_controller.yaml`, reporting every problem at once. + + Raises: + SchemaError: the manifest is unusable; `.violations` holds them all. + """ + load_root_dotenv(path) + collector = _Collector(path) + try: + document = load_yaml_lines(path) + except OSError as exc: + raise SchemaError( + [SchemaViolation(path, 0, "", f"cannot be read: {exc}")] + ) from exc + except yaml.YAMLError as exc: + line, detail = parse_failure(exc) + raise SchemaError( + [SchemaViolation(path, line, "", f"is not valid YAML: {detail}")] + ) from exc + + if document is None: + raise SchemaError([SchemaViolation(path, 0, "", "is empty")]) + if not isinstance(document, dict): + raise SchemaError( + [ + SchemaViolation( + path, 0, "", f"expected a mapping, got {_describe(document)}" + ) + ] + ) + + for key, message in _RETIRED_KEYS.items(): + if key in document: + collector.add(document, key, key, message) + _check_keys(collector, document, "", _MANIFEST_KEYS | _RETIRED_KEYS.keys()) + services = _services(collector, document) + manifest = Manifest( + agents=services, + poll_interval=_number(collector, document, "poll_interval", "", 5, 0), + cleanup_interval=_number(collector, document, "cleanup_interval", "", 10, 0), + project_id=_string(collector, document, "project_id", ""), + redis=_redis(collector, document), + env_file=_string(collector, document, "env_file", ""), + logs=_boolean(collector, document, "logs", "", True), + otel=_otel(collector, document), + ec2=_ec2(collector, document, services), + path=path, + ) + if collector.violations: + raise SchemaError(collector.violations) + return manifest + + +def _redis(collector, node): + block = _mapping(collector, node, "redis", "", _REDIS_KEYS) + if block is None: + return RedisSpec() + return RedisSpec( + host=_string(collector, block, "host", "redis", "localhost") or "localhost", + port=_port(collector, block, "port", "redis", 6379), + db=_integer(collector, block, "db", "redis", 0, 0), + ) diff --git a/packages/core/canyonos_core/schema/otel_destinations.py b/packages/core/canyonos_core/schema/otel_destinations.py new file mode 100644 index 00000000..7425ef79 --- /dev/null +++ b/packages/core/canyonos_core/schema/otel_destinations.py @@ -0,0 +1,92 @@ +"""The `otel.destinations` contract, shared by the manifest schema and the exporter. + +The Global Controller hands the list to the exporter as written, after `${VAR}` +expansion, so the schema that gates a deploy and the exporter that reads the +list at runtime check it with these same rules. +""" + +import math + +SUPPORTED_PROTOCOLS = ("grpc", "http", "http/protobuf") +DESTINATION_KEYS = frozenset( + {"name", "protocol", "endpoint", "headers", "insecure", "timeout"} +) + + +def normalize_protocol(protocol): + """Protocols are matched in any casing.""" + return protocol.lower() if isinstance(protocol, str) else protocol + + +def _non_empty_string(value): + return isinstance(value, str) and bool(value.strip()) + + +def destinations_problem(destinations): + """Why the destinations value as a whole is unusable, or None.""" + if not isinstance(destinations, list) or not destinations: + return "must be a non-empty list" + return None + + +def destination_problems(destination): + """Yield `(key, message)` for every rule one destination mapping breaks.""" + if not _non_empty_string(destination.get("name")): + yield "name", "name must be a non-empty string" + + protocol = destination.get("protocol") + if normalize_protocol(protocol) not in SUPPORTED_PROTOCOLS: + yield ( + "protocol", + f"protocol must be one of {list(SUPPORTED_PROTOCOLS)}; got {protocol!r}", + ) + + if not _non_empty_string(destination.get("endpoint")): + yield "endpoint", "endpoint must be a non-empty string" + + headers = destination.get("headers") + if headers is not None: + if not isinstance(headers, dict): + yield "headers", "headers must be a mapping" + elif any( + not _non_empty_string(key) or not isinstance(value, str) + for key, value in headers.items() + ): + yield "headers", "headers must map non-empty strings to strings" + + insecure = destination.get("insecure") + if insecure is not None and not isinstance(insecure, bool): + yield "insecure", "insecure must be a boolean" + + timeout = destination.get("timeout") + if timeout is not None and _positive_number(timeout) is None: + yield "timeout", "timeout must be a positive number" + + +def _positive_number(value): + """`value` as a finite float above zero, or None.""" + if isinstance(value, bool) or not isinstance(value, (int, float)): + return None + try: + number = float(value) + except OverflowError: + return None + return number if math.isfinite(number) and number > 0 else None + + +def normalize_destination(destination): + """A destination that passed `destination_problems`, in the form the exporter uses.""" + headers = destination.get("headers") + timeout = destination.get("timeout") + return { + "name": destination["name"].strip(), + "protocol": normalize_protocol(destination["protocol"]), + "endpoint": destination["endpoint"].strip(), + "headers": dict(headers) if headers is not None else None, + "insecure": destination.get("insecure"), + "timeout": float(timeout) if timeout is not None else None, + } + + +def duplicate_name_message(name): + return f"destination names must be unique; duplicate {name!r}" diff --git a/packages/core/canyonos_core/schema/yaml_lines.py b/packages/core/canyonos_core/schema/yaml_lines.py new file mode 100644 index 00000000..078401e3 --- /dev/null +++ b/packages/core/canyonos_core/schema/yaml_lines.py @@ -0,0 +1,115 @@ +"""YAML loading that remembers which line every mapping key was written on. + +A violation that cannot point at a line is much harder to act on than one that +can, and PyYAML drops the marks as soon as it builds the plain dict -- so the +mapping constructor is replaced with one that keeps them. +""" + +import yaml +import yaml.constructor +import yaml.resolver + + +class LineDict(dict): + """A mapping that remembers where it and each of its keys were written.""" + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.line = 0 + self.key_lines = {} + + +class LineList(list): + """A sequence that remembers where it and each of its items were written.""" + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.line = 0 + self.item_lines = [] + + +class LineLoader(yaml.SafeLoader): + pass + + +def _reject_duplicate_keys(node): + """Refuse a mapping that sets the same key twice. + + PyYAML keeps the last value without a word, so `replicas:` written twice + in one service deploys whichever came second. Only scalar keys are + compared, by tag and text, so `1` and `"1"` stay distinct. + """ + seen = {} + for key, _ in node.value: + if not isinstance(key, yaml.ScalarNode): + continue + identity = (key.tag, key.value) + if identity in seen: + raise yaml.constructor.ConstructorError( + None, + None, + f"found duplicate key {key.value!r} (first set on line {seen[identity]})", + key.start_mark, + ) + seen[identity] = key.start_mark.line + 1 + + +def _construct_mapping(loader, node): + _reject_duplicate_keys(node) + data = LineDict() + yield data + data.update(loader.construct_mapping(node, deep=False)) + data.line = node.start_mark.line + 1 + data.key_lines = { + key.value: key.start_mark.line + 1 + for key, _ in node.value + if isinstance(key, yaml.ScalarNode) + } + + +def _construct_sequence(loader, node): + data = LineList() + yield data + data.extend(loader.construct_sequence(node, deep=False)) + data.line = node.start_mark.line + 1 + data.item_lines = [item.start_mark.line + 1 for item in node.value] + + +LineLoader.add_constructor( + yaml.resolver.BaseResolver.DEFAULT_MAPPING_TAG, _construct_mapping +) +LineLoader.add_constructor( + yaml.resolver.BaseResolver.DEFAULT_SEQUENCE_TAG, _construct_sequence +) + + +def line_of(node, key=None): + """The 1-based line of `key` inside `node`, or of `node` itself. + + `key` is a mapping key, or an index into a sequence. + """ + if isinstance(node, LineList): + if isinstance(key, int) and 0 <= key < len(node.item_lines): + return node.item_lines[key] + return node.line + if not isinstance(node, LineDict): + return 0 + if key is not None: + return node.key_lines.get(key, node.line) + return node.line + + +def load_yaml_lines(path): + """Parse `path` into line-aware mappings.""" + with open(path, "r", encoding="utf-8") as f: + return yaml.load(f, Loader=LineLoader) + + +def parse_failure(exc): + """A YAMLError as `(line, message)`, collapsed onto one line. + + PyYAML writes its errors over four lines; a violation has to stay on one + for the host CLI to report it whole. + """ + mark = getattr(exc, "problem_mark", None) + return (mark.line + 1 if mark else 0), " ".join(str(exc).split()) diff --git a/packages/core/canyonos_core/stub_generator.py b/packages/core/canyonos_core/stub_generator.py index 771d5012..876cdb58 100644 --- a/packages/core/canyonos_core/stub_generator.py +++ b/packages/core/canyonos_core/stub_generator.py @@ -13,11 +13,18 @@ import argparse import ast import os +import re import shutil -import yaml -from packaging.requirements import InvalidRequirement, Requirement +from packaging.requirements import Requirement +from packaging.utils import canonicalize_name from packaging.version import Version +from canyonos_core.schema import ( + DependencyPinConflict, + SchemaViolation, + load_agent_declaration, +) + # Packages every agent container needs regardless of its specific business logic. # # protobuf and grpcio-tools move together: grpcio-tools carries the only upper @@ -43,6 +50,25 @@ # (telemetry and session state moved to Redis/OTLP, so no SQL driver is required). BASE_WORKFLOW_REQUIREMENTS = BASE_AGENT_REQUIREMENTS + [] +IMAGE_PYTHON_VERSION = "3.11" +DEFAULT_DOCKER_PLATFORM = "linux/amd64" +_MACHINE_BY_DOCKER_ARCH = {"amd64": "x86_64", "arm64": "aarch64"} +_MARKER_VARIABLES = frozenset( + { + "implementation_name", + "implementation_version", + "os_name", + "platform_machine", + "platform_python_implementation", + "platform_release", + "platform_system", + "platform_version", + "python_full_version", + "python_version", + "sys_platform", + } +) + # Packages the image's own code is built against, so an app cannot be left to # pick them alone. _FORCED_FROM_BASE = ("protobuf", "grpcio", "grpcio-tools", "requests", "boto3") @@ -63,7 +89,7 @@ def _build_import_nodes(): ] -def _build_stub_method(func_config, agent_name): +def _build_stub_method(function, agent_name): """ Build an AST node for a single stub method. @@ -83,16 +109,16 @@ def get_stock_price(self, ticker: str) -> Future: return Future(parent=inspect.stack()[1].filename, service="FinanceAgent", method="get_stock_price", args=args, grpc_stub=self.stub) """ - func_name = func_config["name"] - description = func_config.get("description", "") - arguments = func_config.get("arguments", []) + func_name = function.name + description = function.description + arguments = function.arguments # Build argument nodes: self + declared args with type annotations args_list = [ast.arg(arg="self")] for arg in arguments: arg_node = ast.arg( - arg=arg["name"], - annotation=ast.Name(id=arg["type"]) if "type" in arg else None, + arg=arg.name, + annotation=ast.parse(arg.type, mode="eval").body if arg.type else None, ) args_list.append(arg_node) @@ -115,7 +141,7 @@ def get_stock_price(self, ticker: str) -> Future: # Build the args dict with Future replacement: # args = {"ticker": ticker.id if isinstance(ticker, Future) else ticker, ...} - arg_dict_keys = [ast.Constant(value=a["name"]) for a in arguments] + arg_dict_keys = [ast.Constant(value=a.name) for a in arguments] arg_dict_values = [] for a in arguments: # value.id if isinstance(value, Future) else value @@ -123,11 +149,11 @@ def get_stock_price(self, ticker: str) -> Future: ast.IfExp( test=ast.Call( func=ast.Name(id="isinstance"), - args=[ast.Name(id=a["name"]), ast.Name(id="Future")], + args=[ast.Name(id=a.name), ast.Name(id="Future")], keywords=[], ), - body=ast.Attribute(value=ast.Name(id=a["name"]), attr="id"), - orelse=ast.Name(id=a["name"]), + body=ast.Attribute(value=ast.Name(id=a.name), attr="id"), + orelse=ast.Name(id=a.name), ) ) @@ -193,7 +219,7 @@ def get_stock_price(self, ticker: str) -> Future: return func_def -def _build_stub_class(agent_config): +def _build_stub_class(declaration): """ Build an AST node for the entire stub class. @@ -203,8 +229,8 @@ def __init__(self): pass ...stub methods... """ - class_name = agent_config["name"] - functions = agent_config.get("functions", []) + class_name = declaration.name + functions = declaration.functions # __init__ method: simple pass, no gRPC setup needed. # Future handles its own gRPC connections via env vars. @@ -226,8 +252,8 @@ def __init__(self): # Build all stub methods methods = [init_method] - for func_config in functions: - methods.append(_build_stub_method(func_config, agent_config["name"])) + for function in functions: + methods.append(_build_stub_method(function, declaration.name)) class_def = ast.ClassDef( name=class_name, @@ -243,13 +269,11 @@ def __init__(self): def generate_stub(yaml_path, output_path): """ Read a YAML agent definition and generate an importable Python stub file. - """ - with open(yaml_path, "r") as f: - config = yaml.safe_load(f) - agent_config = config["agent"] - - class_def = _build_stub_class(agent_config) + Raises: + SchemaError: the declaration fails the agent schema. + """ + class_def = _build_stub_class(load_agent_declaration(yaml_path)) # Build the full module AST module = ast.Module( @@ -547,38 +571,125 @@ def _copy_files(output_dir, files_to_copy): shutil.copy2(src, dest_path) -def _platform_overrides(requirements): +def target_docker_platform(): + """The platform every image is built for.""" + return os.environ.get("CANYONOS_DOCKER_PLATFORM", DEFAULT_DOCKER_PLATFORM) + + +def _image_marker_environment(): + """The marker values the image fixes; the rest are unknown until it runs.""" + environment = { + "python_version": IMAGE_PYTHON_VERSION, + "sys_platform": "linux", + "platform_system": "Linux", + "os_name": "posix", + "implementation_name": "cpython", + "platform_python_implementation": "CPython", + } + arch = target_docker_platform().partition("/")[2].partition("/")[0] + if arch in _MACHINE_BY_DOCKER_ARCH: + environment["platform_machine"] = _MACHINE_BY_DOCKER_ARCH[arch] + return environment + + +def _applies_in_image(marker): + """False only when the marker is known to be false inside the image. + + A marker reading a value the image does not fix, such as the Python patch + version, is checked rather than guessed from the machine running the build. + """ + environment = _image_marker_environment() + unquoted = re.sub(r"'[^']*'|\"[^\"]*\"", "", str(marker)) + used = _MARKER_VARIABLES.intersection(re.findall(r"[a-z_]+", unquoted)) + if used - environment.keys(): + return True + return marker.evaluate(environment) + + +def _only_newer_than(spec, pinned): + """True when `spec` rules `pinned` out only by demanding something newer.""" + if spec.operator not in (">=", ">", "==", "~="): + return False + bound = Version(spec.version.rstrip(".*")) + # `>PIN` excludes the pin itself and nothing older, so every version it + # allows is newer; the other operators need a version past the pin for that. + return bound >= pinned if spec.operator == ">" else bound > pinned + + +def _platform_overrides(requirements, *, service=None, manifest_path=None, lines=None): """Take the higher of each platform pin and what the app asked for. uv replaces a requirement rather than intersecting it, so the comparison cannot be left to the resolver. + + `service` is the manifest index of the service these requirements belong + to, and with `manifest_path` and `lines` (each requirement's line) it is + only there to point a conflict at the line the user has to edit. + + Raises: + DependencyPinConflict: the app pinned a package *below* the version the + image's own code is built against. Forcing the platform pin over it + produced an image that installed cleanly and then failed at import, + so the build stops here instead. """ declared = {} - for requirement in requirements: - try: - parsed = Requirement(requirement) - except InvalidRequirement: + first_lines = {} + for index, requirement in enumerate(requirements): + parsed = Requirement(requirement) + if parsed.marker and not _applies_in_image(parsed.marker): continue - declared[parsed.name.lower()] = parsed + # PEP 503 names: `grpcio_tools`, `Grpcio-Tools` and `grpcio.tools` + # are all the package pinned as `grpcio-tools`. A package asked for + # more than once is asked for once with every bound, since only the + # intersection can be installed -- keeping just the last line made the + # answer depend on the order they were written in. + key = canonicalize_name(parsed.name) + if lines and key not in first_lines: + first_lines[key] = lines[index] + if key in declared: + first_name, specifier = declared[key] + declared[key] = (first_name, specifier & parsed.specifier) + else: + declared[key] = (parsed.name, parsed.specifier) overrides = [] + conflicts = [] for pin in PLATFORM_PINS: name, pinned = pin.split("==") - asked = declared.get(name) - if asked is None or asked.specifier.contains(Version(pinned)): + pinned_version = Version(pinned) + asked = declared.get(canonicalize_name(name)) + if asked is None or asked[1].contains(pinned_version): overrides.append(pin) continue - wanted = f"{asked.name}{asked.specifier}" - if any( - spec.operator in (">=", ">", "==", "~=") - and Version(spec.version.rstrip(".*")) > Version(pinned) - for spec in asked.specifier + asked_name, specifier = asked + wanted = f"{asked_name}{specifier}" + # Newer only if a lower bound above the pin rules it out and nothing + # else does but an exclusion; one upper bound below it and nothing + # newer can satisfy both. + ruling = [spec for spec in specifier if not spec.contains(pinned_version)] + if any(_only_newer_than(spec, pinned_version) for spec in ruling) and all( + _only_newer_than(spec, pinned_version) or spec.operator == "!=" + for spec in ruling ): overrides.append(wanted) print(f" Note: '{wanted}' outranks the platform pin {pin}") else: overrides.append(pin) - print(f" Warning: the platform pin {pin} breaks '{wanted}'") + field = ( + "requirements" if service is None else f"agents[{service}].requirements" + ) + conflicts.append( + SchemaViolation( + manifest_path or "", + first_lines.get(canonicalize_name(name), 0), + field, + f"'{wanted}' conflicts with the platform pin {pin}, which the " + f"agent image is built against: relax the bound or pin " + f"{name} at or above {pinned}", + ) + ) + if conflicts: + raise DependencyPinConflict(conflicts) return overrides @@ -620,10 +731,7 @@ def generate_docker( stub_entrypoints: Optional {stub_basename: entrypoint} map for exact stub placement. requirements: Optional list of extra pip packages this agent needs. """ - with open(yaml_path, "r") as f: - config = yaml.safe_load(f) - - agent_name = config["agent"]["name"] + agent_name = load_agent_declaration(yaml_path).name script_dir = os.path.dirname(os.path.abspath(__file__)) project_root = os.path.join(script_dir, "..") @@ -723,7 +831,7 @@ def generate_docker( # ---- Dockerfile ------------------------------------------------------ agent_basename = os.path.basename(agent_file) dockerfile = f"""# syntax=docker/dockerfile:1 -FROM python:3.11-slim +FROM python:{IMAGE_PYTHON_VERSION}-slim COPY --from=ghcr.io/astral-sh/uv:latest /uv /uvx /bin/ WORKDIR /app @@ -906,7 +1014,7 @@ def mark_ready_when_serving(): # ---- Dockerfile ------------------------------------------------------ dockerfile = f"""# syntax=docker/dockerfile:1 -FROM python:3.11-slim +FROM python:{IMAGE_PYTHON_VERSION}-slim COPY --from=ghcr.io/astral-sh/uv:latest /uv /uvx /bin/ WORKDIR /app diff --git a/packages/core/tests/test_agent_declaration_schema.py b/packages/core/tests/test_agent_declaration_schema.py new file mode 100644 index 00000000..1199f7e3 --- /dev/null +++ b/packages/core/tests/test_agent_declaration_schema.py @@ -0,0 +1,459 @@ +"""The agent declaration schema decides what a stub can be generated from. + +An argument's `type` is pasted verbatim into the generated stub as an +annotation and the stub imports nothing, so `List[str]` or a project's own +model class produced a module that only failed once the container tried to +import it. +""" + +import glob +import os +import sys +import tempfile +import unittest +from unittest.mock import patch +from pathlib import Path + +import yaml +from test_manifest_schema import example_environment + +sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), ".."))) + +from canyonos_core.schema import ( + BUILTIN_TYPE_NAMES, + SchemaError, + load_agent_declaration, + render_violation, + validate_project, +) + +REPO_ROOT = Path(__file__).resolve().parents[3] + + +class _DeclarationCase(unittest.TestCase): + def load(self, document, filename="example_agent.yaml"): + with tempfile.TemporaryDirectory() as tmpdir: + path = os.path.join(tmpdir, filename) + with open(path, "w") as f: + yaml.safe_dump(document, f, sort_keys=False) + return load_agent_declaration(path) + + def violations(self, document): + with self.assertRaises(SchemaError) as raised: + self.load(document) + return raised.exception.violations + + def one(self, document): + violations = self.violations(document) + self.assertEqual(len(violations), 1, [render_violation(v) for v in violations]) + return violations[0] + + +class ExampleDeclarationTests(unittest.TestCase): + def test_every_example_declaration_loads(self): + declarations = sorted(glob.glob(str(REPO_ROOT / "examples/*/agents/*.yaml"))) + self.assertTrue(declarations, "no example declarations found") + for path in declarations: + with self.subTest(declaration=os.path.relpath(path, REPO_ROOT)): + declaration = load_agent_declaration(path) + self.assertTrue(declaration.name) + + +class DeclarationShapeTests(_DeclarationCase): + def test_a_declaration_without_an_agent_block_is_rejected(self): + violations = self.violations({"agents": [{"name": "ExampleAgent"}]}) + + self.assertEqual([v.field for v in violations], ["agents", "agent"]) + self.assertIn("did you mean 'agent'?", violations[0].message) + self.assertIn("is required and must be a mapping", violations[1].message) + + def test_an_agent_without_a_name_is_rejected(self): + violation = self.one({"agent": {"functions": []}}) + + self.assertEqual(violation.field, "agent.name") + self.assertEqual(violation.message, "is required but missing") + + def test_a_key_set_twice_is_rejected_at_the_second(self): + # Two `name:` keys would otherwise generate a stub for whichever came + # last, with no word about the other. + with tempfile.TemporaryDirectory() as tmpdir: + path = os.path.join(tmpdir, "example_agent.yaml") + Path(path).write_text("agent:\n name: ExampleAgent\n name: OtherAgent\n") + with self.assertRaises(SchemaError) as raised: + load_agent_declaration(path) + + (violation,) = raised.exception.violations + self.assertEqual(violation.line, 3) + self.assertIn("found duplicate key 'name'", violation.message) + self.assertNotIn("\n", render_violation(violation)) + + def test_a_declaration_with_no_functions_is_fine(self): + declaration = self.load({"agent": {"name": "ExampleAgent"}}) + + self.assertEqual(declaration.name, "ExampleAgent") + self.assertEqual(declaration.functions, ()) + + def test_a_function_needs_a_name(self): + violation = self.one( + {"agent": {"name": "ExampleAgent", "functions": [{"description": "hi"}]}} + ) + + self.assertEqual(violation.field, "agent.functions[0].name") + + def test_returns_is_optional(self): + declaration = self.load( + { + "agent": { + "name": "ExampleAgent", + "functions": [{"name": "hello", "arguments": []}], + } + } + ) + + (function,) = declaration.functions + self.assertIsNone(function.returns) + self.assertEqual(function.description, "") + self.assertEqual(function.arguments, ()) + + +class ArgumentTypeTests(_DeclarationCase): + def _declaration(self, type_name): + return { + "agent": { + "name": "ExampleAgent", + "functions": [ + { + "name": "hello", + "arguments": [{"name": "value", "type": type_name}], + } + ], + } + } + + def test_every_builtin_type_is_accepted(self): + for type_name in sorted(BUILTIN_TYPE_NAMES): + with self.subTest(type=type_name): + declaration = self.load(self._declaration(type_name)) + self.assertEqual(declaration.functions[0].arguments[0].type, type_name) + + def test_a_type_the_stub_cannot_import_is_rejected(self): + for type_name in ( + "List[str]", + "MyModel", + "Str", + "typing.Any", + "list[MyModel]", + "'str'", + "str(", + "int[str]", + "list[int][str]", + "None | None", + ): + with self.subTest(type=type_name): + violation = self.one(self._declaration(type_name)) + self.assertEqual( + violation.field, "agent.functions[0].arguments[0].type" + ) + self.assertIn(repr(type_name), violation.message) + self.assertIn("not built from builtin types", violation.message) + + def test_a_type_built_from_builtins_is_accepted(self): + for type_name in ( + "list[str]", + "dict[str, int]", + "tuple[int, ...]", + "int | None", + "None", + ): + with self.subTest(type=type_name): + declaration = self.load(self._declaration(type_name)) + self.assertEqual(declaration.functions[0].arguments[0].type, type_name) + + def test_the_rejection_names_the_file_and_the_field(self): + with tempfile.TemporaryDirectory() as tmpdir: + path = os.path.join(tmpdir, "example_agent.yaml") + with open(path, "w") as f: + yaml.safe_dump(self._declaration("List[str]"), f, sort_keys=False) + with self.assertRaises(SchemaError) as raised: + load_agent_declaration(path) + + (violation,) = raised.exception.violations + rendered = render_violation(violation) + self.assertTrue(rendered.startswith(f"{path}:")) + self.assertIn("agent.functions[0].arguments[0].type", rendered) + self.assertNotIn("\n", rendered) + + def test_an_argument_type_is_optional(self): + declaration = self.load( + { + "agent": { + "name": "ExampleAgent", + "functions": [{"name": "hello", "arguments": [{"name": "value"}]}], + } + } + ) + + self.assertIsNone(declaration.functions[0].arguments[0].type) + + +class GeneratedNameTests(_DeclarationCase): + """Every name in a declaration is written into the stub as Python source.""" + + def _declaration(self, agent="ExampleAgent", function="hello", arguments=()): + return { + "agent": { + "name": agent, + "functions": [{"name": function, "arguments": list(arguments)}], + } + } + + def test_a_name_that_is_not_an_identifier_is_rejected(self): + cases = [ + ("agent.name", self._declaration(agent="Bad-Agent")), + ("agent.functions[0].name", self._declaration(function="bad-name")), + ("agent.functions[0].name", self._declaration(function="class")), + ( + "agent.functions[0].arguments[0].name", + self._declaration(arguments=[{"name": "from"}]), + ), + ( + "agent.functions[0].arguments[0].name", + self._declaration(arguments=[{"name": "my-arg"}]), + ), + ] + for field, document in cases: + with self.subTest(field=field, document=document): + violation = self.one(document) + self.assertEqual(violation.field, field) + self.assertIn("not a valid Python identifier", violation.message) + + def test_a_name_the_stub_itself_uses_is_rejected(self): + for name in ("Future", "inspect", "isinstance"): + for field, document in ( + ("agent.name", self._declaration(agent=name)), + ( + "agent.functions[0].arguments[0].name", + self._declaration(arguments=[{"name": name}]), + ), + ): + with self.subTest(name=name, field=field): + violation = self.one(document) + self.assertEqual(violation.field, field) + self.assertIn("would shadow", violation.message) + + def test_an_argument_named_self_is_rejected(self): + violation = self.one(self._declaration(arguments=[{"name": "self"}])) + + self.assertEqual(violation.field, "agent.functions[0].arguments[0].name") + + def test_a_repeated_argument_name_is_rejected(self): + violation = self.one( + self._declaration(arguments=[{"name": "a"}, {"name": "a"}]) + ) + + self.assertEqual(violation.field, "agent.functions[0].arguments[1].name") + + +class ValidateProjectTests(unittest.TestCase): + def _project(self, tmpdir, declaration_name="ExampleAgent"): + project = Path(tmpdir) + (project / "config").mkdir() + (project / "agents").mkdir() + (project / "config" / "global_controller.yaml").write_text( + yaml.safe_dump( + { + "agents": [ + { + "name": "ExampleAgent", + "entrypoint": "agents/example_agent.py", + } + ] + } + ) + ) + (project / "agents" / "example_agent.yaml").write_text( + yaml.safe_dump({"agent": {"name": declaration_name}}) + ) + return ( + str(project / "config" / "global_controller.yaml"), + str(project / "agents"), + ) + + def test_a_valid_project_reports_nothing(self): + with tempfile.TemporaryDirectory() as tmpdir: + manifest, declarations = self._project(tmpdir) + self.assertEqual(validate_project(manifest, declarations), ()) + + def test_a_declaration_naming_a_different_agent_is_reported(self): + with tempfile.TemporaryDirectory() as tmpdir: + manifest, declarations = self._project(tmpdir, declaration_name="Typo") + violations = validate_project(manifest, declarations) + + (violation,) = violations + self.assertEqual(violation.field, "agents[0].name") + self.assertIn("agent.name: ExampleAgent", violation.message) + + def test_a_broken_manifest_does_not_hide_a_broken_declaration(self): + # Both files are checked independently, so one run finds both problems + # instead of costing the user a second deploy to see the next one. + with tempfile.TemporaryDirectory() as tmpdir: + manifest, declarations = self._project(tmpdir) + Path(manifest).write_text(yaml.safe_dump({"agents": [{"name": "X"}]})) + Path(declarations, "example_agent.yaml").write_text( + yaml.safe_dump( + { + "agent": { + "name": "ExampleAgent", + "functions": [ + { + "name": "hello", + "arguments": [{"name": "v", "type": "MyModel"}], + } + ], + } + } + ) + ) + violations = validate_project(manifest, declarations) + + self.assertEqual( + [v.field for v in violations], + ["agents[0].entrypoint", "agent.functions[0].arguments[0].type"], + ) + self.assertEqual( + {os.path.basename(v.path) for v in violations}, + {"global_controller.yaml", "example_agent.yaml"}, + ) + + def test_a_declaration_violation_is_reported_with_the_manifest_intact(self): + with tempfile.TemporaryDirectory() as tmpdir: + manifest, declarations = self._project(tmpdir) + Path(declarations, "example_agent.yaml").write_text( + yaml.safe_dump( + { + "agent": { + "name": "ExampleAgent", + "functions": [ + { + "name": "hello", + "arguments": [{"name": "v", "type": "List[str]"}], + } + ], + } + } + ) + ) + violations = validate_project(manifest, declarations) + + (violation,) = violations + self.assertEqual(violation.field, "agent.functions[0].arguments[0].type") + + def test_a_yaml_that_is_not_a_declaration_is_left_alone(self): + # The .car layout keeps the manifest and policy.yaml in the same + # directory as the declarations; the build skips them the same way. + with tempfile.TemporaryDirectory() as tmpdir: + manifest, declarations = self._project(tmpdir) + Path(declarations, "policy.yaml").write_text( + yaml.safe_dump({"rules": [{"service": "ExampleAgent"}]}) + ) + self.assertEqual(validate_project(manifest, declarations), ()) + + def test_source_dir_is_optional_and_off_by_default(self): + # 2c calls validate_project without one; nothing on disk is checked. + with tempfile.TemporaryDirectory() as tmpdir: + manifest, declarations = self._project(tmpdir) + self.assertEqual(validate_project(manifest, declarations), ()) + + def test_a_declared_entrypoint_must_exist_under_the_source_dir(self): + with tempfile.TemporaryDirectory() as tmpdir: + manifest, declarations = self._project(tmpdir) + violations = validate_project(manifest, declarations, tmpdir) + + (violation,) = violations + self.assertEqual(violation.field, "agents[0].entrypoint") + self.assertEqual( + violation.message, + f"{os.path.join(tmpdir, 'agents', 'example_agent.py')} does not exist", + ) + + def test_the_missing_file_is_named_the_way_the_manifest_writes_it(self): + # The build runs from the project root, so that is the form the reader + # is looking at; an absolute /tmp/... path makes them translate. + with tempfile.TemporaryDirectory() as tmpdir: + manifest, declarations = self._project(tmpdir) + cwd = os.getcwd() + os.chdir(tmpdir) + try: + violations = validate_project(manifest, declarations, ".") + finally: + os.chdir(cwd) + + (violation,) = violations + self.assertEqual( + violation.message, + f"{os.path.join('agents', 'example_agent.py')} does not exist", + ) + + def test_an_entrypoint_that_is_there_passes(self): + with tempfile.TemporaryDirectory() as tmpdir: + manifest, declarations = self._project(tmpdir) + Path(tmpdir, "agents", "example_agent.py").write_text("print('ok')\n") + self.assertEqual(validate_project(manifest, declarations, tmpdir), ()) + + def test_an_entrypoint_that_resolves_outside_the_project_is_rejected(self): + with ( + tempfile.TemporaryDirectory() as tmpdir, + tempfile.TemporaryDirectory() as outside, + ): + manifest, declarations = self._project(tmpdir) + Path(outside, "secret.py").write_text("SECRET = 1\n") + os.symlink( + os.path.join(outside, "secret.py"), + os.path.join(tmpdir, "agents", "example_agent.py"), + ) + violations = validate_project(manifest, declarations, tmpdir) + + (violation,) = violations + self.assertEqual(violation.field, "agents[0].entrypoint") + self.assertIn("resolves outside the project", violation.message) + + def test_a_symlink_inside_the_project_passes(self): + with tempfile.TemporaryDirectory() as tmpdir: + manifest, declarations = self._project(tmpdir) + Path(tmpdir, "real_agent.py").write_text("print('ok')\n") + os.symlink( + os.path.join(tmpdir, "real_agent.py"), + os.path.join(tmpdir, "agents", "example_agent.py"), + ) + self.assertEqual(validate_project(manifest, declarations, tmpdir), ()) + + def test_a_missing_source_dir_is_one_violation_naming_it(self): + with tempfile.TemporaryDirectory() as tmpdir: + manifest, declarations = self._project(tmpdir) + source_dir = os.path.join(tmpdir, "app") + violations = validate_project(manifest, declarations, source_dir) + + (violation,) = violations + self.assertEqual(violation.field, "agents") + self.assertIn(source_dir, violation.message) + self.assertIn("does not exist", violation.message) + + def test_every_example_project_validates(self): + for manifest in sorted( + glob.glob(str(REPO_ROOT / "examples/*/config/global_controller.yaml")) + ): + declarations = os.path.join( + os.path.dirname(os.path.dirname(manifest)), "agents" + ) + with ( + self.subTest(project=os.path.relpath(manifest, REPO_ROOT)), + patch.dict(os.environ, example_environment(manifest)), + ): + violations = validate_project(manifest, declarations) + self.assertEqual( + violations, (), [render_violation(v) for v in violations] + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/packages/core/tests/test_cli.py b/packages/core/tests/test_cli.py index e2ca6dd8..a6433c13 100644 --- a/packages/core/tests/test_cli.py +++ b/packages/core/tests/test_cli.py @@ -43,6 +43,7 @@ def test_deploy_skips_ec2_preflight_for_local_config( patch("canyonos_core.cli.os.path.isfile", return_value=True), patch("canyonos_core.cli._load_config", return_value=config), patch("canyonos_core.cli.resolve_env_file", return_value=None), + patch("canyonos_core.cli.validate_or_exit"), patch.dict( sys.modules, {"canyonos_core.controller.global_controller": controller_module}, @@ -77,6 +78,7 @@ def test_deploy_runs_ec2_preflight_for_ec2_config( patch("canyonos_core.cli.os.path.isfile", return_value=True), patch("canyonos_core.cli._load_config", return_value=config), patch("canyonos_core.cli.resolve_env_file", return_value=None), + patch("canyonos_core.cli.validate_or_exit"), patch.dict( sys.modules, {"canyonos_core.controller.global_controller": controller_module}, @@ -105,6 +107,7 @@ def test_deploy_uses_car_when_present( patch("canyonos_core.cli.os.path.isfile", return_value=True), patch("canyonos_core.cli._load_config", return_value={"agents": []}), patch("canyonos_core.cli.resolve_env_file", return_value=None), + patch("canyonos_core.cli.validate_or_exit"), patch.dict( sys.modules, {"canyonos_core.controller.global_controller": controller_module}, @@ -140,6 +143,37 @@ def test_preflight_does_not_require_ssh_fields(self, require_docker, ensure_grpc require_docker.assert_called_once_with("deploy") ensure_grpc.assert_called_once_with(os.getcwd()) + def test_deploy_rejects_a_bad_config_before_it_builds(self): + with tempfile.TemporaryDirectory() as tmpdir: + project_dir = Path(tmpdir) + (project_dir / "config").mkdir() + (project_dir / "agents").mkdir() + config_path = project_dir / "config" / "global_controller.yaml" + config_path.write_text( + yaml.safe_dump( + {"agents": [{"name": "ExampleAgent", "entrypoint": "/abs.py"}]} + ) + ) + + with ( + patch("canyonos_core.stub_generator.generate_stub") as generate_stub, + patch("canyonos_core.cli.subprocess.run") as subprocess_run, + ): + cwd = os.getcwd() + os.chdir(project_dir) + try: + with self.assertLogs("canyonos_core", level="ERROR") as log: + with self.assertRaises(SystemExit) as raised: + cli.cmd_deploy(SimpleNamespace(config=str(config_path))) + finally: + os.chdir(cwd) + + self.assertEqual(raised.exception.code, 1) + generate_stub.assert_not_called() + subprocess_run.assert_not_called() + self.assertIn("agents[0].entrypoint", log.output[0]) + self.assertNotIn("\n", log.output[0]) + class CliBuildTests(unittest.TestCase): def _run_build( @@ -352,8 +386,13 @@ def test_build_fails_when_stub_cannot_be_generated(self): ) ) - with self.assertRaisesRegex(RuntimeError, "missing `entrypoint`"): - self._run_build(project_dir, [], buildx_available=True) + # The schema gate reports it before the build starts, as one line. + with self.assertLogs("canyonos_core", level="ERROR") as log: + with self.assertRaises(SystemExit) as raised: + self._run_build(project_dir, [], buildx_available=True) + + self.assertEqual(raised.exception.code, 1) + self.assertIn("agents[0].entrypoint: is required but missing", log.output[0]) def _write_requirements_config(self, project_dir): """Scaffold one plain agent, one agent with `requirements`, one workflow with `requirements`.""" @@ -423,7 +462,64 @@ def test_build_passes_per_agent_requirements_to_generators(self): ["sqlalchemy-utils"], ) - def test_build_ignores_non_list_requirements(self): + def test_build_uses_the_expanded_values_the_schema_validated(self): + # The schema checks `${AGENT_FILE}` in its expanded form; the build used + # to re-read the raw YAML and hand the literal to the generators, which + # then failed on a file called `${AGENT_FILE}`. + with tempfile.TemporaryDirectory() as tmpdir: + project_dir = Path(tmpdir) + (project_dir / "config").mkdir() + (project_dir / "agents").mkdir() + (project_dir / "agents" / "example_agent.py").write_text("print('ok')\n") + example_yaml = project_dir / "agents" / "example_agent.yaml" + example_yaml.write_text("agent:\n name: ExampleAgent\n") + (project_dir / "config" / "global_controller.yaml").write_text( + "agents:\n" + " - name: ExampleAgent\n" + " entrypoint: ${CANYONOS_TEST_AGENT_FILE}\n" + " requirements: ['${CANYONOS_TEST_EXTRA}']\n" + ) + + with ( + patch.dict( + os.environ, + { + "CANYONOS_TEST_AGENT_FILE": "agents/example_agent.py", + "CANYONOS_TEST_EXTRA": "yfinance", + }, + ), + patch( + "canyonos_core.cli._get_package_dir", + return_value=str(project_dir / "package"), + ), + patch("canyonos_core.stub_generator.generate_stub") as generate_stub, + patch( + "canyonos_core.stub_generator.generate_docker" + ) as generate_docker, + patch("canyonos_core.cli.subprocess.run"), + patch("canyonos_core.cli._docker_available", return_value=False), + ): + cwd = os.getcwd() + os.chdir(project_dir) + try: + cli._run_build( + str(project_dir / "config" / "global_controller.yaml") + ) + finally: + os.chdir(cwd) + + (stub_call,) = generate_stub.call_args_list + self.assertTrue( + stub_call.args[1].endswith(os.path.join("agents", "example_agent.py")) + ) + docker_call = generate_docker.call_args + self.assertEqual( + os.path.realpath(docker_call.args[1]), + os.path.realpath(project_dir / "agents" / "example_agent.py"), + ) + self.assertEqual(docker_call.kwargs["requirements"], ["yfinance"]) + + def test_build_rejects_non_list_requirements(self): with tempfile.TemporaryDirectory() as tmpdir: project_dir = Path(tmpdir) (project_dir / "config").mkdir() @@ -447,13 +543,221 @@ def test_build_ignores_non_list_requirements(self): ) ) - with self.assertLogs("canyonos_core", level="WARNING") as log: - _, generate_docker, _ = self._run_build( - project_dir, [str(example_yaml)], buildx_available=True + with self.assertLogs("canyonos_core", level="ERROR") as log: + with self.assertRaises(SystemExit): + self._run_build( + project_dir, [str(example_yaml)], buildx_available=True + ) + + self.assertIn("agents[0].requirements", log.output[0]) + self.assertIn("expected a list of strings", log.output[0]) + + def test_build_rejects_a_dependency_pin_the_platform_cannot_meet(self): + with tempfile.TemporaryDirectory() as tmpdir: + project_dir = Path(tmpdir) + (project_dir / "config").mkdir() + (project_dir / "agents").mkdir() + (project_dir / "agents" / "example_agent.py").write_text("print('ok')\n") + (project_dir / "agents" / "other_agent.py").write_text("print('ok')\n") + example_yaml = project_dir / "agents" / "example_agent.yaml" + example_yaml.write_text("agent:\n name: ExampleAgent\n") + other_yaml = project_dir / "agents" / "other_agent.yaml" + other_yaml.write_text("agent:\n name: OtherAgent\n") + config_path = project_dir / "config" / "global_controller.yaml" + config_path.write_text( + yaml.safe_dump( + { + "agents": [ + { + "name": "ExampleAgent", + "entrypoint": "agents/example_agent.py", + "requirements": ["protobuf<5"], + }, + { + "name": "OtherAgent", + "entrypoint": "agents/other_agent.py", + "requirements": ["grpcio<1.0"], + }, + ] + } ) + ) + + with self.assertLogs("canyonos_core", level="ERROR") as log: + with self.assertRaises(SystemExit): + self._run_build( + project_dir, + [str(example_yaml), str(other_yaml)], + buildx_available=True, + ) - self.assertIn("requirements", log.output[0]) - self.assertEqual(generate_docker.call_args.kwargs["requirements"], []) + # Both services are reported, so two bad pins take one run to find. + self.assertIn("agents[0].requirements", log.output[0]) + self.assertIn("protobuf<5", log.output[0]) + self.assertIn("agents[1].requirements", log.output[1]) + + +class BuildStopsBeforeGeneratingAnythingTests(unittest.TestCase): + """A rejected config must cost nothing: no stub, no protoc, no Docker.""" + + def _scaffold(self, project_dir, manifest, declaration, write_source=True): + (project_dir / "config").mkdir() + (project_dir / "agents").mkdir() + if write_source: + (project_dir / "agents" / "example_agent.py").write_text("print('ok')\n") + (project_dir / "agents" / "example_agent.yaml").write_text( + yaml.safe_dump(declaration) + ) + (project_dir / "config" / "global_controller.yaml").write_text( + yaml.safe_dump(manifest) + ) + return project_dir / "config" / "global_controller.yaml" + + def _build(self, manifest, declaration, write_source=True): + with tempfile.TemporaryDirectory() as tmpdir: + project_dir = Path(tmpdir) + config_path = self._scaffold( + project_dir, manifest, declaration, write_source + ) + + with ( + patch("canyonos_core.stub_generator.generate_stub") as generate_stub, + patch("canyonos_core.cli.subprocess.run") as subprocess_run, + ): + cwd = os.getcwd() + os.chdir(project_dir) + try: + with self.assertLogs("canyonos_core", level="ERROR") as self.log: + with self.assertRaises(SystemExit) as raised: + cli._run_build(str(config_path)) + finally: + os.chdir(cwd) + + return raised.exception, generate_stub, subprocess_run + + def test_an_invalid_manifest_stops_the_build(self): + exit_error, generate_stub, subprocess_run = self._build( + { + "agents": [ + { + "name": "ExampleAgent", + "entrypoint": "agents/example_agent.py", + "replicas": "2", + } + ] + }, + {"agent": {"name": "ExampleAgent"}}, + ) + + self.assertEqual(exit_error.code, 1) + generate_stub.assert_not_called() + subprocess_run.assert_not_called() + + def test_an_invalid_declaration_stops_the_build(self): + exit_error, generate_stub, subprocess_run = self._build( + { + "agents": [ + {"name": "ExampleAgent", "entrypoint": "agents/example_agent.py"} + ] + }, + { + "agent": { + "name": "ExampleAgent", + "functions": [ + {"name": "hello", "arguments": [{"name": "v", "type": "List"}]} + ], + } + }, + ) + + self.assertEqual(exit_error.code, 1) + generate_stub.assert_not_called() + subprocess_run.assert_not_called() + + def test_an_entrypoint_with_no_file_behind_it_stops_the_build(self): + # This used to log "Agent file not found", skip the service and let the + # deploy exit 0 without it. + exit_error, generate_stub, subprocess_run = self._build( + { + "agents": [ + {"name": "ExampleAgent", "entrypoint": "agents/example_agent.py"} + ] + }, + {"agent": {"name": "ExampleAgent"}}, + write_source=False, + ) + + self.assertEqual(exit_error.code, 1) + generate_stub.assert_not_called() + subprocess_run.assert_not_called() + self.assertIn("agents[0].entrypoint", self.log.output[0]) + self.assertIn("example_agent.py does not exist", self.log.output[0]) + self.assertNotIn("\n", self.log.output[0]) + + def test_a_workflow_file_with_nothing_behind_it_stops_the_build(self): + exit_error, generate_stub, subprocess_run = self._build( + { + "agents": [ + { + "name": "Workflow", + "type": "workflow", + "workflow_file": "workflows/example_workflow.py", + } + ] + }, + {"agent": {"name": "ExampleAgent"}}, + ) + + self.assertEqual(exit_error.code, 1) + generate_stub.assert_not_called() + subprocess_run.assert_not_called() + self.assertIn("agents[0].workflow_file", self.log.output[0]) + self.assertIn("example_workflow.py does not exist", self.log.output[0]) + + def test_a_missing_source_root_stops_the_build_once(self): + # Under the .car layout the app's code lives in .car/app; without it + # the build found nothing to do and said "No Docker images to build." + with tempfile.TemporaryDirectory() as tmpdir: + project_dir = Path(tmpdir) + artifact_root = project_dir / ".car" + (artifact_root / "config").mkdir(parents=True) + (artifact_root / "config" / "example_agent.yaml").write_text( + yaml.safe_dump({"agent": {"name": "ExampleAgent"}}) + ) + config_path = artifact_root / "config" / "global_controller.yaml" + config_path.write_text( + yaml.safe_dump( + { + "agents": [ + { + "name": "ExampleAgent", + "entrypoint": "agents/example_agent.py", + } + ] + } + ) + ) + + with ( + patch("canyonos_core.stub_generator.generate_stub") as generate_stub, + patch("canyonos_core.cli.subprocess.run") as subprocess_run, + ): + cwd = os.getcwd() + os.chdir(project_dir) + try: + with self.assertLogs("canyonos_core", level="ERROR") as log: + with self.assertRaises(SystemExit) as raised: + cli._run_build(str(config_path)) + finally: + os.chdir(cwd) + + self.assertEqual(raised.exception.code, 1) + generate_stub.assert_not_called() + subprocess_run.assert_not_called() + # One violation naming the directory, not one per service. + self.assertEqual(len(log.output), 2) + self.assertIn("agents: the project source directory", log.output[0]) + self.assertIn(os.path.join(".car", "app"), log.output[0]) class CliCleanTests(unittest.TestCase): @@ -475,3 +779,24 @@ def test_clean_uses_car_when_present(self): if __name__ == "__main__": unittest.main() + + +class ValidateOrExitTests(unittest.TestCase): + def test_the_manifest_is_parsed_once(self): + from canyonos_core import schema + from canyonos_core.cli import validate_or_exit + + with tempfile.TemporaryDirectory() as tmpdir: + manifest_path = os.path.join(tmpdir, "global_controller.yaml") + with open(manifest_path, "w") as f: + f.write( + "agents:\n - name: Workflow\n type: workflow\n" + " workflow_file: workflow.py\n" + ) + with patch.object( + schema, "load_manifest", wraps=schema.load_manifest + ) as load_manifest: + manifest = validate_or_exit(manifest_path, tmpdir) + + self.assertEqual(load_manifest.call_count, 1) + self.assertEqual(manifest.agents[0].name, "Workflow") diff --git a/packages/core/tests/test_controller_context_config.py b/packages/core/tests/test_controller_context_config.py index 2e42de80..e8b81cc5 100644 --- a/packages/core/tests/test_controller_context_config.py +++ b/packages/core/tests/test_controller_context_config.py @@ -12,6 +12,11 @@ from canyonos_core.controller.controller_context import ControllerContext from canyonos_core.controller.global_controller import GlobalController +from canyonos_core.controller.utils.config_specs import ( + read_config_specs, + write_config_specs, +) +from fakes import _FakeRedis _CONFIG = """\ poll_interval: 5 @@ -62,6 +67,15 @@ def test_an_unset_ref_is_left_alone_rather_than_emptied(self): self.assertEqual(config["ec2"]["ami_id"], "${TEST_EC2_AMI_ID}") + def test_expanded_agents_are_published_without_reparsing_the_manifest(self): + with patch.dict(os.environ, self.env): + config = ControllerContext._load_config(self.config_path) + + redis = _FakeRedis() + write_config_specs(config["agents"], redis) + + self.assertEqual(read_config_specs(redis)[0]["instance_type"], "t3.small") + class ReconcilerContextParityTests(unittest.TestCase): """The reconciler builds a bare ControllerContext, so it must match the GC on these.""" diff --git a/packages/core/tests/test_global_controller_project_id.py b/packages/core/tests/test_global_controller_project_id.py index be67e8b1..f2fff410 100644 --- a/packages/core/tests/test_global_controller_project_id.py +++ b/packages/core/tests/test_global_controller_project_id.py @@ -51,6 +51,26 @@ def test_reload_reuses_the_persisted_project_id_instead_of_minting_a_new_one(sel finally: os.unlink(config_path) + def test_a_null_project_id_is_replaced_instead_of_duplicated(self): + for body in ( + "agents: []\nproject_id:\npoll_interval: 5\n", + "project_id: null\n", + ): + with self.subTest(body=body): + config_path = _write_config(body) + try: + config = GlobalController._load_config(config_path) + + with open(config_path) as f: + contents = f.read() + self.assertEqual(contents.count("project_id"), 1) + self.assertEqual( + yaml.safe_load(contents)["project_id"], config["project_id"] + ) + self.assertTrue(UUID_HEX_RE.match(config["project_id"])) + finally: + os.unlink(config_path) + def test_existing_project_id_is_left_untouched(self): config_path = _write_config( 'agents: []\nproject_id: "11111111-1111-1111-1111-111111111111"\n' diff --git a/packages/core/tests/test_manifest_schema.py b/packages/core/tests/test_manifest_schema.py new file mode 100644 index 00000000..677a8c3e --- /dev/null +++ b/packages/core/tests/test_manifest_schema.py @@ -0,0 +1,1120 @@ +"""The manifest schema decides what `global_controller.yaml` may say. + +Before it existed, an unknown key was ignored, `replicas: "2"` blew up inside +the instance manager once containers were already being launched, and a +workflow with no `workflow_file` was warned about and then skipped while the +deploy reported success. Every test here is one of those turned into a +rejection with a file, a line and a field. +""" + +import glob +import os +import sys +import tempfile +import unittest +from pathlib import Path +from unittest.mock import patch + +import yaml + +sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), ".."))) + +from canyonos_core.controller.utils.config_env import ENV_REF +from canyonos_core.schema import ( + AgentService, + DatabaseService, + SchemaError, + SchemaViolation, + WorkflowService, + load_manifest, + render_violation, +) + +REPO_ROOT = Path(__file__).resolve().parents[3] + + +def example_environment(manifest_path): + """A placeholder for every `${VAR}` an example manifest reads, as its `.env` would set.""" + names = ENV_REF.findall(Path(manifest_path).read_text()) + return {name: f"example-{name.lower()}" for name in names} + + +def _agent(**overrides): + entry = {"name": "ExampleAgent", "entrypoint": "agents/example_agent.py"} + entry.update(overrides) + return entry + + +class _ManifestCase(unittest.TestCase): + def load(self, config, filename="global_controller.yaml"): + """Write a manifest to a scratch project and load it.""" + with tempfile.TemporaryDirectory() as tmpdir: + path = os.path.join(tmpdir, filename) + with open(path, "w") as f: + yaml.safe_dump(config, f, sort_keys=False) + return load_manifest(path) + + def violations(self, config): + with self.assertRaises(SchemaError) as raised: + self.load(config) + return raised.exception.violations + + def one(self, config): + violations = self.violations(config) + self.assertEqual(len(violations), 1, [render_violation(v) for v in violations]) + return violations[0] + + def fields(self, config): + return [violation.field for violation in self.violations(config)] + + +class ExampleManifestTests(unittest.TestCase): + """Every manifest shipped in examples/ has to satisfy the schema it documents.""" + + def test_every_example_manifest_loads(self): + manifests = sorted( + glob.glob(str(REPO_ROOT / "examples/*/config/global_controller*.yaml")) + ) + self.assertTrue(manifests, "no example manifests found") + for path in manifests: + with ( + self.subTest(manifest=os.path.relpath(path, REPO_ROOT)), + patch.dict(os.environ, example_environment(path)), + ): + manifest = load_manifest(path) + self.assertTrue(manifest.agents) + + +class UnknownKeyTests(_ManifestCase): + def test_an_unknown_top_level_key_is_rejected_with_a_line(self): + violation = self.one({"agents": [_agent()], "registry": {"url": "example"}}) + + self.assertEqual(violation.field, "registry") + self.assertIn("unknown key 'registry'", violation.message) + self.assertGreater(violation.line, 0) + self.assertTrue(violation.path.endswith("global_controller.yaml")) + + def test_a_near_miss_is_offered_the_key_it_meant(self): + violation = self.one({"agents": [_agent(replias=2)]}) + + self.assertEqual(violation.field, "agents[0].replias") + self.assertEqual( + violation.message, "unknown key 'replias' (did you mean 'replicas'?)" + ) + + def test_a_key_valid_for_another_type_names_this_one(self): + violation = self.one( + { + "agents": [ + { + "name": "Workflow", + "type": "workflow", + "workflow_file": "workflow/example_workflow.py", + "entrypoint": "agents/example_agent.py", + } + ] + } + ) + + self.assertEqual(violation.field, "agents[0].entrypoint") + self.assertEqual( + violation.message, "key 'entrypoint' is not valid for type 'workflow'" + ) + + def test_an_unknown_nested_key_is_rejected(self): + self.assertEqual( + self.fields({"agents": [_agent()], "redis": {"hostname": "localhost"}}), + ["redis.hostname"], + ) + + +class ReplicaTests(_ManifestCase): + def test_a_replica_count_that_is_not_a_positive_integer_is_rejected(self): + for replicas, quoted in ( + ("2", "the string '2'"), + (0, "the number 0"), + (-1, "the number -1"), + (1.5, "the number 1.5"), + (True, "the boolean true"), + ([{"host": "a"}], "the list [{'host': 'a'}]"), + ): + with self.subTest(replicas=replicas): + violation = self.one({"agents": [_agent(replicas=replicas)]}) + self.assertEqual(violation.field, "agents[0].replicas") + self.assertEqual( + violation.message, f"expected an integer >= 1, got {quoted}" + ) + + +class ServiceIdentityTests(_ManifestCase): + def test_a_service_without_a_name_is_rejected(self): + violation = self.one({"agents": [{"entrypoint": "agents/a.py"}]}) + + self.assertEqual(violation.field, "agents[0].name") + self.assertEqual(violation.message, "is required but missing") + + def test_two_services_cannot_share_a_name(self): + violation = self.one( + {"agents": [_agent(), _agent(entrypoint="agents/other.py")]} + ) + + self.assertEqual(violation.field, "agents[1].name") + self.assertIn("duplicate service name 'ExampleAgent'", violation.message) + + def test_names_that_differ_only_in_case_collide(self): + # The image tag and the container name are both name.lower(). + violation = self.one( + { + "agents": [ + _agent(), + _agent(name="exampleagent", entrypoint="agents/other.py"), + ] + } + ) + + self.assertEqual(violation.field, "agents[1].name") + self.assertIn("image tag", violation.message) + + +class LoadConfigParityTests(_ManifestCase): + """The gate rejects everything cli._load_config's own checks reject. + + _run_build validates before it loads, so a manifest the schema passed but + _load_config then refused would escape as a RuntimeError traceback instead + of a one-line violation. + """ + + def _workflow(self, **overrides): + entry = { + "name": "Workflow", + "type": "workflow", + "workflow_file": "workflow/example_workflow.py", + } + entry.update(overrides) + return {"agents": [entry]} + + def test_a_port_outside_the_tcp_range_is_rejected(self): + for field, value in ( + ("api_port", 0), + ("redis_port", 65536), + ("host_port", 70000), + ("dashboard_port", 65536), + ): + with self.subTest(field=field, value=value): + violation = self.one(self._workflow(**{field: value})) + self.assertEqual(violation.field, f"agents[0].{field}") + self.assertIn("between 1 and 65535", violation.message) + + def test_the_highest_port_is_accepted(self): + manifest = self.load(self._workflow(api_port=65535)) + + self.assertEqual(manifest.agents[0].api_port, 65535) + + def test_a_local_workflow_cannot_be_replicated(self): + violation = self.one(self._workflow(replicas=2)) + + self.assertEqual(violation.field, "agents[0].replicas") + self.assertIn("same api_port", violation.message) + + def test_an_ec2_workflow_can_be_replicated(self): + manifest = self.load( + { + **self._workflow(replicas=2, provider="EC2", instance_type="t3.micro"), + "ec2": _EC2_BLOCK, + } + ) + + self.assertEqual(manifest.agents[0].replicas, 2) + + def test_resources_are_positive_numbers(self): + manifest = self.load({"agents": [_agent(resources={"cpu": 0.5})]}) + self.assertEqual(manifest.agents[0].resources.cpu, 0.5) + + for field, value in (("gpu", 0), ("cpu", -1), ("memory", "512"), ("cpu", True)): + with self.subTest(field=field, value=value): + violation = self.one({"agents": [_agent(resources={field: value})]}) + self.assertEqual(violation.field, f"agents[0].resources.{field}") + self.assertIn("expected a finite number > 0", violation.message) + + +class LogsFlagTests(_ManifestCase): + def test_logs_defaults_on_as_the_runtimes_read_it(self): + self.assertTrue(self.load({"agents": [_agent()]}).logs) + + def test_logs_can_be_turned_off(self): + self.assertFalse(self.load({"agents": [_agent()], "logs": False}).logs) + + def test_logs_must_be_a_real_boolean(self): + # The runtimes pass it through bool(), so the string "false" is on. + violation = self.one({"agents": [_agent()], "logs": "false"}) + + self.assertEqual(violation.field, "logs") + self.assertEqual( + violation.message, "expected a boolean, got the string 'false'" + ) + + +class EntrypointTests(_ManifestCase): + def test_an_agent_without_an_entrypoint_is_rejected(self): + violation = self.one({"agents": [{"name": "ExampleAgent"}]}) + + self.assertEqual(violation.field, "agents[0].entrypoint") + self.assertEqual(violation.message, "is required but missing") + + def test_a_workflow_without_a_workflow_file_is_rejected(self): + violation = self.one({"agents": [{"name": "Workflow", "type": "workflow"}]}) + + self.assertEqual(violation.field, "agents[0].workflow_file") + self.assertEqual(violation.message, "is required but missing") + + def test_a_windows_rooted_entrypoint_is_rejected(self): + # posixpath reads all three as relative names inside the project. + for entrypoint in ( + "\\outside\\agent.py", + "C:\\x\\agent.py", + "\\\\server\\share\\agent.py", + ): + with self.subTest(entrypoint=entrypoint): + violation = self.one({"agents": [_agent(entrypoint=entrypoint)]}) + self.assertEqual(violation.field, "agents[0].entrypoint") + self.assertIn("must be relative to the project", violation.message) + + def test_a_windows_rooted_workflow_file_is_rejected(self): + violation = self.one( + { + "agents": [ + { + "name": "Workflow", + "type": "workflow", + "workflow_file": "C:\\flows\\workflow.py", + } + ] + } + ) + + self.assertEqual(violation.field, "agents[0].workflow_file") + self.assertIn("must be relative to the project", violation.message) + + def test_an_entrypoint_outside_the_project_is_rejected(self): + for entrypoint, expected in ( + ("/etc/passwd.py", "must be relative to the project"), + ("../elsewhere/agent.py", "must not escape the project with '..'"), + ("agents/example_agent", "must name a .py file"), + ): + with self.subTest(entrypoint=entrypoint): + violation = self.one({"agents": [_agent(entrypoint=entrypoint)]}) + self.assertEqual(violation.field, "agents[0].entrypoint") + self.assertIn(expected, violation.message) + + +class DatabaseServiceTests(_ManifestCase): + def _database(self, **overrides): + entry = {"name": "StateDB", "type": "database", "image": "postgres:16-alpine"} + entry.update(overrides) + return {"agents": [entry]} + + def test_a_database_without_an_image_is_rejected(self): + violation = self.one({"agents": [{"name": "StateDB", "type": "database"}]}) + + self.assertEqual(violation.field, "agents[0].image") + self.assertEqual(violation.message, "is required but missing") + + def test_a_database_cannot_be_replicated(self): + violation = self.one(self._database(replicas=2)) + + self.assertEqual(violation.field, "agents[0].replicas") + self.assertIn("single container", violation.message) + + def test_a_database_env_must_be_a_mapping(self): + violation = self.one(self._database(env=["POSTGRES_USER=bob"])) + + self.assertEqual(violation.field, "agents[0].env") + self.assertIn("expected a mapping", violation.message) + + def test_a_database_parses_with_its_ports_and_volume(self): + manifest = self.load( + self._database(db_port=5433, volume_path="/var/lib/postgresql/data") + ) + + (service,) = manifest.agents + self.assertIsInstance(service, DatabaseService) + self.assertEqual(service.db_port, 5433) + self.assertEqual(service.volume_path, "/var/lib/postgresql/data") + + +class ProviderTests(_ManifestCase): + def test_a_provider_in_any_casing_is_normalized(self): + # cli._load_config accepts these and rewrites them to the spelling the + # runtimes compare against; the gate in front of it does the same. + for provider, normalized in (("LOCAL", "local"), ("Local", "local")): + with self.subTest(provider=provider): + manifest = self.load({"agents": [_agent(provider=provider)]}) + self.assertEqual(manifest.agents[0].provider, normalized) + + for provider in ("Ec2", "ec2"): + with self.subTest(provider=provider): + manifest = self.load( + { + "agents": [_agent(provider=provider, instance_type="t3.micro")], + "ec2": _EC2_BLOCK, + } + ) + self.assertEqual(manifest.agents[0].provider, "EC2") + + def test_a_lowercase_ec2_still_needs_the_ec2_block(self): + violation = self.one( + {"agents": [_agent(provider="ec2", instance_type="t3.micro")]} + ) + + self.assertEqual(violation.field, "ec2") + + def test_a_misspelled_provider_is_rejected(self): + for provider in ("locale", "EC2 ", "aws"): + with self.subTest(provider=provider): + violation = self.one({"agents": [_agent(provider=provider)]}) + self.assertEqual(violation.field, "agents[0].provider") + self.assertEqual( + violation.message, + f"expected one of ['local', 'EC2'], got the string {provider!r}", + ) + + def test_an_ec2_service_needs_an_instance_type(self): + self.assertIn( + "agents[0].instance_type", + self.fields( + { + "agents": [_agent(provider="EC2")], + "ec2": _EC2_BLOCK, + } + ), + ) + + def test_an_ec2_service_needs_the_ec2_block(self): + violation = self.one( + {"agents": [_agent(provider="EC2", instance_type="t3.micro")]} + ) + + self.assertEqual(violation.field, "ec2") + self.assertIn("provider 'EC2'", violation.message) + self.assertEqual(violation.line, 0) + + def test_the_ec2_block_must_be_complete(self): + block = dict(_EC2_BLOCK) + del block["ssh_user"] + + self.assertEqual( + self.fields( + { + "agents": [_agent(provider="EC2", instance_type="t3.micro")], + "ec2": block, + } + ), + ["ec2.ssh_user"], + ) + + def test_an_empty_security_group_list_is_rejected(self): + # It used to pass the required check, after which the block was + # dropped without a violation: an EC2 service with Manifest.ec2 None. + violation = self.one( + { + "agents": [_agent(provider="EC2", instance_type="t3.micro")], + "ec2": {**_EC2_BLOCK, "security_group_ids": []}, + } + ) + + self.assertEqual(violation.field, "ec2.security_group_ids") + self.assertEqual(violation.message, "is required and must not be empty") + + def test_an_empty_requirements_list_is_still_fine(self): + manifest = self.load({"agents": [_agent(requirements=[])]}) + + self.assertEqual(manifest.agents[0].requirements, ()) + + def test_the_ec2_block_carries_its_own_defaults(self): + manifest = self.load( + { + "agents": [_agent(provider="EC2", instance_type="t3.micro")], + "ec2": _EC2_BLOCK, + } + ) + + self.assertEqual(manifest.ec2.ssh_private_key_path, "~/.ssh/ventis_ec2") + self.assertEqual(manifest.ec2.public_ip_timeout, 120) + self.assertEqual(manifest.ec2.controller_health_timeout, 180) + + +_EC2_BLOCK = { + "region": "us-east-1", + "ami_id": "ami-0123456789abcdef0", + "subnet_id": "subnet-0123456789abcdef0", + "security_group_ids": ["sg-0123456789abcdef0"], + "ssh_user": "ubuntu", +} + + +class DefaultsTests(_ManifestCase): + def test_every_default_materializes(self): + manifest = self.load( + { + "agents": [ + _agent(), + { + "name": "Workflow", + "type": "workflow", + "workflow_file": "workflow/example_workflow.py", + }, + { + "name": "StateDB", + "type": "database", + "image": "postgres:16-alpine", + }, + ] + } + ) + + agent, workflow, database = manifest.agents + self.assertEqual(manifest.poll_interval, 5) + self.assertEqual(manifest.cleanup_interval, 10) + self.assertEqual(manifest.redis.host, "localhost") + self.assertEqual(manifest.redis.port, 6379) + self.assertEqual(manifest.redis.db, 0) + self.assertIsNone(manifest.otel) + self.assertIsNone(manifest.ec2) + + self.assertIsInstance(agent, AgentService) + self.assertEqual(agent.provider, "local") + self.assertEqual(agent.replicas, 1) + self.assertEqual(agent.redis_port, 6379) + self.assertEqual(agent.resources.cpu, 1) + self.assertEqual(agent.resources.memory, 512) + self.assertIsNone(agent.resources.gpu) + self.assertFalse(agent.stateful) + self.assertEqual(agent.requirements, ()) + + self.assertIsInstance(workflow, WorkflowService) + self.assertEqual(workflow.api_port, 8080) + self.assertEqual(workflow.dashboard_port, 8081) + + self.assertIsInstance(database, DatabaseService) + self.assertEqual(database.db_port, 5432) + + +class StringMappingTests(_ManifestCase): + def test_a_bad_value_is_named_with_its_key(self): + for value, described in (([1], "the list [1]"), ({"a": 1}, "a mapping")): + with self.subTest(value=value): + violation = self.one( + {"agents": [_agent(env={"OK": "1", "BAD": value})]} + ) + self.assertEqual(violation.field, "agents[0].env.BAD") + self.assertEqual( + violation.message, f"expected a string, got {described}" + ) + + def test_numbers_are_still_accepted_as_their_text(self): + manifest = self.load({"agents": [_agent(env={"PORT": 8080, "RATIO": 0.5})]}) + + self.assertEqual(manifest.agents[0].env, {"PORT": "8080", "RATIO": "0.5"}) + + +class RequirementTests(_ManifestCase): + def test_an_entry_the_pin_check_cannot_read_is_rejected(self): + for requirement in ("-r deps.txt", "protobuf<5\nyfinance", "git+https://x/y"): + with self.subTest(requirement=requirement): + violation = self.one( + {"agents": [_agent(requirements=["requests", requirement])]} + ) + self.assertEqual(violation.field, "agents[0].requirements[1]") + self.assertIn("not a single PEP 508 requirement", violation.message) + + def test_a_pep_508_requirement_is_accepted(self): + requirements = [ + "requests>=2", + "pkg @ https://example.com/pkg.whl", + "yfinance; python_version >= '3.8'", + ] + manifest = self.load({"agents": [_agent(requirements=requirements)]}) + + self.assertEqual(manifest.agents[0].requirements, tuple(requirements)) + + +class IntervalTests(_ManifestCase): + def test_an_interval_may_be_fractional(self): + manifest = self.load( + {"agents": [_agent()], "poll_interval": 0.5, "cleanup_interval": 2.5} + ) + + self.assertEqual(manifest.poll_interval, 0.5) + self.assertEqual(manifest.cleanup_interval, 2.5) + + def test_an_interval_must_be_positive(self): + for value in (0, -1, "5", True): + with self.subTest(value=value): + violation = self.one({"agents": [_agent()], "poll_interval": value}) + self.assertEqual(violation.field, "poll_interval") + self.assertIn("expected a finite number > 0", violation.message) + + +class OtelTests(_ManifestCase): + def _otel(self, destination): + return {"agents": [_agent()], "otel": {"destinations": [destination]}} + + def test_an_unsupported_protocol_is_caught_at_load(self): + violation = self.one( + self._otel( + {"name": "local", "protocol": "smoke-signal", "endpoint": "http://x"} + ) + ) + + self.assertEqual(violation.field, "otel.destinations[0].protocol") + self.assertIn("'grpc', 'http', 'http/protobuf'", violation.message) + + def test_a_destination_without_an_endpoint_is_caught_at_load(self): + violation = self.one(self._otel({"name": "local", "protocol": "http"})) + + self.assertEqual(violation.field, "otel.destinations[0].endpoint") + self.assertEqual(violation.message, "endpoint must be a non-empty string") + + def test_a_timeout_may_be_fractional(self): + # The exporter takes a float; a timeout is a duration, not a count. + manifest = self.load( + self._otel( + { + "name": "local", + "protocol": "grpc", + "endpoint": "http://x", + "timeout": 2.5, + } + ) + ) + + (destination,) = manifest.otel.destinations + self.assertEqual(destination.timeout, 2.5) + + def test_a_timeout_that_is_not_a_number_is_rejected(self): + for timeout in ("2", 0): + with self.subTest(timeout=timeout): + violation = self.one( + self._otel( + { + "name": "local", + "protocol": "grpc", + "endpoint": "http://x", + "timeout": timeout, + } + ) + ) + self.assertEqual(violation.field, "otel.destinations[0].timeout") + self.assertEqual(violation.message, "timeout must be a positive number") + + def test_a_complete_destination_parses(self): + manifest = self.load( + self._otel( + { + "name": "local", + "protocol": "http", + "endpoint": "http://host.docker.internal:3000/v1/traces", + "headers": {"x-key": "value"}, + } + ) + ) + + (destination,) = manifest.otel.destinations + self.assertEqual(destination.protocol, "http") + self.assertEqual(destination.headers, {"x-key": "value"}) + self.assertFalse(destination.insecure) + + def test_every_protocol_the_exporter_supports_is_accepted_in_any_case(self): + for given, expected in ( + ("GRPC", "grpc"), + ("HTTP", "http"), + ("http/protobuf", "http/protobuf"), + ("Http/Protobuf", "http/protobuf"), + ): + with self.subTest(protocol=given): + manifest = self.load( + self._otel({"name": "d", "protocol": given, "endpoint": "h:1"}) + ) + self.assertEqual(manifest.otel.destinations[0].protocol, expected) + + def test_header_values_must_be_strings(self): + for headers in ({"x-retries": 3}, {"x-flag": True}): + with self.subTest(headers=headers): + violation = self.one( + self._otel( + { + "name": "d", + "protocol": "grpc", + "endpoint": "h:1", + "headers": headers, + } + ) + ) + self.assertEqual(violation.field, "otel.destinations[0].headers") + self.assertIn("strings to strings", violation.message) + + def test_an_empty_destination_list_is_rejected(self): + violation = self.one({"agents": [_agent()], "otel": {"destinations": []}}) + + self.assertEqual(violation.field, "otel.destinations") + self.assertEqual(violation.message, "must be a non-empty list") + + def test_null_destinations_mean_otel_is_not_configured(self): + manifest = self.load({"agents": [_agent()], "otel": {"destinations": None}}) + + self.assertEqual(manifest.otel.destinations, ()) + + def test_names_that_collide_once_trimmed_are_rejected(self): + destination = {"protocol": "grpc", "endpoint": "h:1"} + violation = self.one( + { + "agents": [_agent()], + "otel": { + "destinations": [ + {"name": "d", **destination}, + {"name": " d ", **destination}, + ] + }, + } + ) + + self.assertEqual(violation.field, "otel.destinations[1].name") + self.assertIn("duplicate 'd'", violation.message) + + +class RetiredKeyTests(unittest.TestCase): + """`database:` configured telemetry until #104 moved it under `otel:`. + + Nothing reads it now, so a manifest still carrying one is told so, instead + of being left to believe its runs are being recorded there. + """ + + _MESSAGE = "is no longer used; telemetry is configured under otel: -- remove it" + + def _violations(self, text): + with tempfile.TemporaryDirectory() as tmpdir: + path = os.path.join(tmpdir, "global_controller.yaml") + Path(path).write_text( + "agents:\n" + " - name: ExampleAgent\n" + " entrypoint: agents/example_agent.py\n" + text + ) + with self.assertRaises(SchemaError) as raised: + load_manifest(path) + return raised.exception.violations + + def test_a_database_block_is_rejected_as_retired(self): + (violation,) = self._violations("database:\n url: sqlite:///runtime.db\n") + + self.assertEqual(violation.field, "database") + self.assertEqual(violation.line, 4) + self.assertEqual(violation.message, self._MESSAGE) + + def test_an_empty_database_block_is_rejected_too(self): + (violation,) = self._violations("database:\n") + + self.assertEqual(violation.field, "database") + self.assertEqual(violation.message, self._MESSAGE) + + +class EnvExpansionTests(_ManifestCase): + """`${VAR}` is supported in string fields only. + + Expansion is textual on both sides, so a reference in a numeric field + reaches the Global Controller as a string -- `replicas` of "3" starts one + replica, not three. Rejecting it here is the only way that stays true. + """ + + def test_an_env_ref_in_a_numeric_field_is_rejected_even_when_it_is_set(self): + config = { + "agents": [ + { + "name": "Workflow", + "type": "workflow", + "workflow_file": "workflow/example_workflow.py", + "api_port": "${CANYONOS_TEST_API_PORT}", + } + ] + } + + with patch.dict(os.environ, {"CANYONOS_TEST_API_PORT": "9000"}): + violation = self.one(config) + + self.assertEqual(violation.field, "agents[0].api_port") + self.assertEqual( + violation.message, + "expected an integer between 1 and 65535, got '${CANYONOS_TEST_API_PORT}' " + "(environment references are only supported in string fields)", + ) + + def test_an_env_ref_in_a_boolean_field_is_rejected(self): + config = {"agents": [_agent(stateful="${CANYONOS_TEST_STATEFUL}")]} + + with patch.dict(os.environ, {"CANYONOS_TEST_STATEFUL": "true"}): + violation = self.one(config) + + self.assertEqual(violation.field, "agents[0].stateful") + self.assertIn("only supported in string fields", violation.message) + + def test_a_string_field_takes_the_variable_s_text(self): + config = { + "agents": [_agent(provider="EC2", instance_type="t3.micro")], + "ec2": {**_EC2_BLOCK, "region": "${CANYONOS_TEST_REGION}"}, + } + + with patch.dict(os.environ, {"CANYONOS_TEST_REGION": "eu-west-2"}): + manifest = self.load(config) + + self.assertEqual(manifest.ec2.region, "eu-west-2") + + def test_a_string_list_takes_the_variable_s_text(self): + config = { + "agents": [_agent(provider="EC2", instance_type="t3.micro")], + "ec2": {**_EC2_BLOCK, "security_group_ids": ["${CANYONOS_TEST_SG}"]}, + } + + with patch.dict(os.environ, {"CANYONOS_TEST_SG": "sg-abc"}): + manifest = self.load(config) + + self.assertEqual(manifest.ec2.security_group_ids, ("sg-abc",)) + + def test_a_ref_that_resolves_to_nothing_still_names_the_variable(self): + # The expansion is '', which on its own tells the reader nothing about + # which variable they have to go and set. + config = {"agents": [_agent()], "redis": {"host": "${CANYONOS_TEST_HOST}"}} + + with patch.dict(os.environ, {"CANYONOS_TEST_HOST": ""}): + violation = self.one(config) + + self.assertEqual(violation.field, "redis.host") + self.assertIn("'${CANYONOS_TEST_HOST}'", violation.message) + + def test_a_ref_that_resolves_to_nothing_in_a_list_names_the_variable(self): + config = { + "agents": [_agent(provider="EC2", instance_type="t3.micro")], + "ec2": {**_EC2_BLOCK, "security_group_ids": ["${CANYONOS_TEST_SG}"]}, + } + + with patch.dict(os.environ, {"CANYONOS_TEST_SG": " "}): + violation = self.one(config) + + self.assertEqual(violation.field, "ec2.security_group_ids") + self.assertIn("'${CANYONOS_TEST_SG}'", violation.message) + + def test_an_unset_ref_in_an_optional_field_is_left_literal_as_the_controller_leaves_it( + self, + ): + config = {"agents": [_agent(host="${CANYONOS_TEST_HOST}")]} + + environ = {k: v for k, v in os.environ.items() if k != "CANYONOS_TEST_HOST"} + with patch.dict(os.environ, environ, clear=True): + manifest = self.load(config) + + self.assertEqual(manifest.agents[0].host, "${CANYONOS_TEST_HOST}") + + def test_an_unset_ref_in_a_required_field_counts_as_missing(self): + for key, value in ( + ("region", "${CANYONOS_TEST_REGION}"), + ("security_group_ids", ["${CANYONOS_TEST_REGION}"]), + ): + with self.subTest(key=key): + config = { + "agents": [_agent(provider="EC2", instance_type="t3.micro")], + "ec2": {**_EC2_BLOCK, key: value}, + } + environ = { + k: v for k, v in os.environ.items() if k != "CANYONOS_TEST_REGION" + } + with patch.dict(os.environ, environ, clear=True): + violation = self.one(config) + + self.assertEqual(violation.field, f"ec2.{key}") + self.assertEqual( + violation.message, + "'${CANYONOS_TEST_REGION}' names an environment variable that " + "is not set", + ) + + def test_the_ec2_block_is_only_checked_when_a_service_uses_ec2(self): + environ = {k: v for k, v in os.environ.items() if k != "CANYONOS_TEST_REGION"} + for block in ( + {"region": "${CANYONOS_TEST_REGION}", "ami_id": "ami-1"}, + {**_EC2_BLOCK, "region": ""}, + ): + with self.subTest(block=block): + with patch.dict(os.environ, environ, clear=True): + manifest = self.load({"agents": [_agent()], "ec2": block}) + self.assertIsNone(manifest.ec2) + + def test_an_unknown_ec2_key_is_still_rejected_in_a_local_project(self): + violation = self.one({"agents": [_agent()], "ec2": {"regoin": "us-east-1"}}) + + self.assertEqual(violation.field, "ec2.regoin") + + def test_an_unset_ref_in_a_numeric_field_is_reported_the_same_way(self): + config = {"agents": [_agent(replicas="${CANYONOS_TEST_REPLICAS}")]} + + environ = {k: v for k, v in os.environ.items() if k != "CANYONOS_TEST_REPLICAS"} + with patch.dict(os.environ, environ, clear=True): + violation = self.one(config) + + self.assertEqual(violation.field, "agents[0].replicas") + self.assertEqual( + violation.message, + "expected an integer >= 1, got '${CANYONOS_TEST_REPLICAS}' " + "(environment references are only supported in string fields)", + ) + + +class EveryProblemInAnEntryTests(_ManifestCase): + """One wrong field used to hide every other problem in the same entry.""" + + def test_an_unknown_type_does_not_hide_the_entrys_other_keys(self): + fields = self.fields({"agents": [{"name": "A", "type": "typo", "replias": 0}]}) + + self.assertEqual(fields, ["agents[0].type", "agents[0].replias"]) + + def test_a_missing_name_does_not_hide_the_entrys_other_fields(self): + fields = self.fields( + {"agents": [{"name": None, "entrypoint": 7, "replicas": "many"}]} + ) + + self.assertEqual( + sorted(fields), + ["agents[0].entrypoint", "agents[0].name", "agents[0].replicas"], + ) + + def test_a_missing_required_ec2_key_does_not_hide_the_optional_ones(self): + block = {**_EC2_BLOCK, "public_ip_timeout": "soon"} + del block["region"] + fields = self.fields( + { + "agents": [_agent(provider="EC2", instance_type="t3.micro")], + "ec2": block, + } + ) + + self.assertEqual(sorted(fields), ["ec2.public_ip_timeout", "ec2.region"]) + + +class ListItemLineTests(unittest.TestCase): + """A problem with one list item points at that item, not at the list's key.""" + + def _violations(self, text): + with tempfile.TemporaryDirectory() as tmpdir: + path = os.path.join(tmpdir, "global_controller.yaml") + Path(path).write_text(text) + with self.assertRaises(SchemaError) as raised: + load_manifest(path) + return raised.exception.violations + + def test_a_service_that_is_not_a_mapping_points_at_its_line(self): + (violation,) = self._violations( + "agents:\n" + " - name: ExampleAgent\n" + " entrypoint: agents/example_agent.py\n" + " - not-a-mapping\n" + ) + + self.assertEqual((violation.field, violation.line), ("agents[1]", 4)) + + def test_a_destination_that_is_not_a_mapping_points_at_its_line(self): + (violation,) = self._violations( + "agents:\n" + " - name: ExampleAgent\n" + " entrypoint: agents/example_agent.py\n" + "otel:\n" + " destinations:\n" + " - just-a-string\n" + ) + + self.assertEqual((violation.field, violation.line), ("otel.destinations[0]", 6)) + + def test_a_bad_requirement_points_at_its_line(self): + (violation,) = self._violations( + "agents:\n" + " - name: ExampleAgent\n" + " entrypoint: agents/example_agent.py\n" + " requirements:\n" + " - requests\n" + " - -r deps.txt\n" + ) + + self.assertEqual( + (violation.field, violation.line), ("agents[0].requirements[1]", 6) + ) + + +class NonFiniteNumberTests(unittest.TestCase): + """YAML's `.inf` and `.nan` are floats, and a long enough integer overflows + one: all three used to pass a numeric field, the last as a traceback.""" + + def _violations(self, field_text): + with tempfile.TemporaryDirectory() as tmpdir: + path = os.path.join(tmpdir, "global_controller.yaml") + Path(path).write_text( + "agents:\n" + " - name: ExampleAgent\n" + " entrypoint: agents/example_agent.py\n" + field_text + ) + with self.assertRaises(SchemaError) as raised: + load_manifest(path) + return raised.exception.violations + + def test_a_non_finite_resource_is_rejected(self): + for literal in (".nan", ".inf", "-.inf"): + with self.subTest(literal=literal): + (violation,) = self._violations( + f" resources:\n cpu: {literal}\n" + ) + self.assertEqual(violation.field, "agents[0].resources.cpu") + self.assertIn("expected a finite number > 0", violation.message) + + def test_a_non_finite_otel_timeout_is_rejected(self): + (violation,) = self._violations( + "otel:\n" + " destinations:\n" + " - name: local\n" + " protocol: grpc\n" + " endpoint: http://x\n" + " timeout: .inf\n" + ) + + self.assertEqual(violation.field, "otel.destinations[0].timeout") + + def test_an_integer_too_large_for_a_float_is_a_violation_not_a_traceback(self): + (violation,) = self._violations(f" resources:\n memory: {'9' * 400}\n") + + self.assertEqual(violation.field, "agents[0].resources.memory") + self.assertIn("expected a finite number > 0", violation.message) + + def test_a_non_finite_value_in_an_integer_field_is_rejected(self): + for literal in (".nan", ".inf", "-.inf"): + with self.subTest(literal=literal): + (violation,) = self._violations(f" replicas: {literal}\n") + self.assertEqual(violation.field, "agents[0].replicas") + self.assertIn("expected an integer >= 1", violation.message) + + +class UnparseableFileTests(unittest.TestCase): + """The gate runs before the plain load, so a broken file is a violation too.""" + + def _load(self, text): + with tempfile.TemporaryDirectory() as tmpdir: + path = os.path.join(tmpdir, "global_controller.yaml") + Path(path).write_text(text) + with self.assertRaises(SchemaError) as raised: + load_manifest(path) + return raised.exception.violations + + def test_a_file_that_is_not_yaml_is_reported_on_one_line(self): + (violation,) = self._load("agents: [one,\n two: three\n") + + rendered = render_violation(violation) + self.assertNotIn("\n", rendered) + self.assertIn("is not valid YAML", rendered) + self.assertGreater(violation.line, 0) + # No field to name, so the location is followed by the message itself. + self.assertNotIn(": : ", rendered) + + def test_a_key_set_twice_in_a_service_is_rejected_at_the_second(self): + # PyYAML keeps the last value silently; the first `replicas` would + # simply vanish from the deploy. + (violation,) = self._load( + "agents:\n" + " - name: ExampleAgent\n" + " entrypoint: agents/example_agent.py\n" + " replicas: 1\n" + " replicas: 3\n" + ) + + self.assertEqual(violation.line, 5) + self.assertIn("found duplicate key 'replicas'", violation.message) + self.assertIn("first set on line 4", violation.message) + self.assertNotIn("\n", render_violation(violation)) + + def test_a_top_level_key_set_twice_is_rejected(self): + (violation,) = self._load( + "agents:\n" + " - name: ExampleAgent\n" + " entrypoint: agents/example_agent.py\n" + "agents:\n" + " - name: OtherAgent\n" + " entrypoint: agents/other_agent.py\n" + ) + + self.assertEqual(violation.line, 4) + self.assertIn("found duplicate key 'agents'", violation.message) + + def test_keys_that_only_look_alike_are_not_duplicates(self): + # `1` is an int and `"1"` a string: two different keys to YAML. + with tempfile.TemporaryDirectory() as tmpdir: + path = os.path.join(tmpdir, "global_controller.yaml") + Path(path).write_text( + "agents:\n" + " - name: ExampleAgent\n" + " entrypoint: agents/example_agent.py\n" + " env:\n" + " 1: a\n" + " '1': b\n" + ) + with self.assertRaises(SchemaError) as raised: + load_manifest(path) + + # Rejected, but for the int key in a string mapping -- not as a duplicate. + (violation,) = raised.exception.violations + self.assertEqual(violation.field, "agents[0].env") + self.assertNotIn("duplicate", violation.message) + self.assertEqual( + violation.message, "expected string keys, got the key the number 1" + ) + self.assertEqual(violation.line, 5) + + def test_an_empty_file_is_reported(self): + (violation,) = self._load("# nothing here\n") + + self.assertEqual(violation.message, "is empty") + + def test_a_file_that_is_not_a_mapping_is_reported(self): + (violation,) = self._load("- one\n- two\n") + + self.assertIn("expected a mapping", violation.message) + + +class RenderingTests(_ManifestCase): + def test_a_violation_with_no_path_does_not_render_a_stray_line_number(self): + self.assertEqual( + render_violation(SchemaViolation("", 5, "agents[0].name", "is wrong")), + "agents[0].name: is wrong", + ) + + def test_a_violation_with_no_field_leaves_the_field_out(self): + self.assertEqual( + render_violation(SchemaViolation("m.yaml", 5, "", "is empty")), + "m.yaml:5: is empty", + ) + + def test_a_violation_renders_as_one_line_with_its_location(self): + violation = self.one({"agents": [_agent()], "registry": {"url": "example"}}) + + rendered = render_violation(violation) + self.assertNotIn("\n", rendered) + self.assertIn(f"{violation.path}:{violation.line}: ", rendered) + self.assertIn("registry: ", rendered) + self.assertTrue(rendered.endswith(violation.message)) + + def test_every_violation_in_a_file_is_reported_at_once(self): + fields = self.fields( + { + "agents": [_agent(replicas="2"), {"name": "Nameless"}], + "poll_interval": "soon", + } + ) + + self.assertEqual( + fields, ["agents[0].replicas", "agents[1].entrypoint", "poll_interval"] + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/packages/core/tests/test_otel_exporter_fanout.py b/packages/core/tests/test_otel_exporter_fanout.py index 257f423e..c94b497b 100644 --- a/packages/core/tests/test_otel_exporter_fanout.py +++ b/packages/core/tests/test_otel_exporter_fanout.py @@ -23,6 +23,7 @@ import trace_convert # noqa: E402 from canyonos_core.controller.utils import otel_writer, schema # noqa: E402 import otel_exporter # noqa: E402 +from canyonos_core.schema.otel_destinations import SUPPORTED_PROTOCOLS # noqa: E402 # The generated local-controller protobuf modules are build artifacts and are @@ -603,7 +604,7 @@ def test_protocol_case_is_normalized(self): self.assertEqual(parsed[0]["protocol"], expected) def test_supported_protocols_are_accepted(self): - for protocol in otel_exporter.SUPPORTED_PROTOCOLS: + for protocol in SUPPORTED_PROTOCOLS: with self.subTest(protocol=protocol): parsed = otel_exporter._configured_destinations( json.dumps([{"name": "a", "protocol": protocol, "endpoint": "h:1"}]) diff --git a/packages/core/tests/test_stub_generator.py b/packages/core/tests/test_stub_generator.py index b9f2bc48..40fc7b6f 100644 --- a/packages/core/tests/test_stub_generator.py +++ b/packages/core/tests/test_stub_generator.py @@ -3,6 +3,7 @@ import sys import tempfile import unittest +import unittest.mock from contextlib import redirect_stdout from pathlib import Path @@ -12,10 +13,12 @@ sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), ".."))) from canyonos_core import stub_generator +from canyonos_core.schema import DependencyPinConflict, render_violation from canyonos_core.stub_generator import ( BASE_AGENT_REQUIREMENTS, BASE_WORKFLOW_REQUIREMENTS, PLATFORM_PINS, + _platform_overrides, _stub_destination, _sweep_project_files, generate_docker, @@ -568,13 +571,177 @@ def test_an_app_asking_for_newer_wins(self): notes, ["Note: 'protobuf>=7' outranks the platform pin protobuf==6.33.5"] ) - def test_an_app_asking_for_older_loses_and_is_told(self): - overrides, notes = self._context(["protobuf<5"]) - self.assertIn("protobuf==6.33.5", overrides) + def test_an_app_asking_for_older_fails_the_build(self): + # Forcing the platform pin over the app's bound used to produce an image + # that installed cleanly and then failed at import, so it is fatal now. + with self.assertRaises(DependencyPinConflict) as raised: + self._context(["protobuf<5"]) + + (violation,) = raised.exception.violations + self.assertEqual(violation.field, "requirements") + self.assertIn("protobuf<5", violation.message) + self.assertIn("protobuf==6.33.5", violation.message) + self.assertIn("at or above 6.33.5", violation.message) + + def test_the_conflict_points_at_the_manifest_entry_to_edit(self): + with self.assertRaises(DependencyPinConflict) as raised: + _platform_overrides( + ["boto3<1"], + service=2, + manifest_path="config/global_controller.yaml", + ) + + (violation,) = raised.exception.violations + rendered = render_violation(violation) + self.assertTrue(rendered.startswith("config/global_controller.yaml: ")) + self.assertIn("agents[2].requirements", rendered) + self.assertNotIn("\n", rendered) + + def test_a_conflict_is_caught_however_the_package_name_is_spelled(self): + # pip treats these as one package; matching on .lower() alone missed + # the underscore and forced the pin over the app's bound instead. + for requirement in ("grpcio_tools<1", "Grpcio-Tools<1", "grpcio.tools<1"): + with self.subTest(requirement=requirement): + with self.assertRaises(DependencyPinConflict) as raised: + _platform_overrides([requirement], service=0) + (violation,) = raised.exception.violations + self.assertIn("grpcio-tools==1.76.0", violation.message) + + def test_a_newer_ask_wins_however_the_package_name_is_spelled(self): + overrides = _platform_overrides(["grpcio_tools>=2"]) + + self.assertIn("grpcio_tools>=2", overrides) + self.assertNotIn("grpcio-tools==1.76.0", overrides) + + def test_a_package_asked_for_twice_conflicts_whatever_the_order(self): + # Only the last line used to count, so `protobuf>=7` written second + # hid the `<5` bound and the build went ahead. + messages = [] + for requirements in ( + ["protobuf<5", "protobuf>=7"], + ["protobuf>=7", "protobuf<5"], + ): + with self.subTest(requirements=requirements): + with self.assertRaises(DependencyPinConflict) as raised: + _platform_overrides(requirements, service=0) + (violation,) = raised.exception.violations + messages.append(violation.message) + + self.assertEqual(messages[0], messages[1]) + self.assertIn("'protobuf<5,>=7'", messages[0]) + + def test_strictly_greater_than_the_pin_is_a_newer_ask(self): + # Every version `>6.33.5` allows is newer than the pin, but it used to + # be reported as a conflict because its bound was not past the pin. + overrides, notes = self._context(["protobuf>6.33.5"]) + + self.assertIn("protobuf>6.33.5", overrides) + self.assertNotIn("protobuf==6.33.5", overrides) self.assertEqual( - notes, ["Warning: the platform pin protobuf==6.33.5 breaks 'protobuf<5'"] + notes, + ["Note: 'protobuf>6.33.5' outranks the platform pin protobuf==6.33.5"], + ) + + def test_at_or_equal_to_the_pin_keeps_the_pin_quietly(self): + for requirement in ("protobuf>=6.33.5", "protobuf==6.33.5"): + with self.subTest(requirement=requirement): + overrides, notes = self._context([requirement]) + self.assertEqual(overrides, list(PLATFORM_PINS)) + self.assertEqual(notes, []) + + def test_an_exclusion_beside_a_newer_bound_is_still_a_newer_ask(self): + for requirement in ("protobuf>=7,!=6.33.5", "requests>=3,!=2.34.2"): + with self.subTest(requirement=requirement): + with redirect_stdout(io.StringIO()): + overrides = _platform_overrides([requirement]) + name = requirement.split(">=")[0] + self.assertFalse( + [pin for pin in overrides if pin.startswith(f"{name}==")] + ) + + def test_excluding_the_pin_alone_is_still_a_conflict(self): + with self.assertRaises(DependencyPinConflict): + _platform_overrides(["protobuf!=6.33.5"], service=0) + + def test_a_requirement_whose_marker_is_false_in_the_image_is_ignored(self): + overrides = _platform_overrides( + [ + 'grpcio<0.1; sys_platform == "win32"', + "grpcio>=1.60; python_version >= '3.8'", + "grpcio<1.50; python_version < '3.8'", + ] ) + self.assertIn("grpcio==1.83.1", overrides) + + def test_a_requirement_whose_marker_is_true_in_the_image_is_checked(self): + with self.assertRaises(DependencyPinConflict): + _platform_overrides(['grpcio<0.1; sys_platform == "linux"'], service=0) + + def test_a_marker_is_evaluated_for_the_image_architecture(self): + requirement = 'grpcio<0.1; platform_machine == "aarch64"' + with unittest.mock.patch.dict( + os.environ, {"CANYONOS_DOCKER_PLATFORM": "linux/amd64"} + ): + self.assertIn("grpcio==1.83.1", _platform_overrides([requirement])) + with unittest.mock.patch.dict( + os.environ, {"CANYONOS_DOCKER_PLATFORM": "linux/arm64"} + ): + with self.assertRaises(DependencyPinConflict): + _platform_overrides([requirement], service=0) + + def test_a_marker_on_a_value_the_image_does_not_fix_is_still_checked(self): + with self.assertRaises(DependencyPinConflict): + _platform_overrides( + ['grpcio<0.1; python_full_version >= "3.11.99"'], service=0 + ) + + def test_a_conflict_points_at_the_requirements_line(self): + with self.assertRaises(DependencyPinConflict) as raised: + _platform_overrides( + ["requests", "grpcio_tools<1", "grpcio-tools<0.5"], + service=0, + manifest_path="config/global_controller.yaml", + lines=[7, 8, 9], + ) + + (violation,) = raised.exception.violations + self.assertEqual(violation.line, 8) + self.assertTrue( + render_violation(violation).startswith("config/global_controller.yaml:8: ") + ) + + def test_two_newer_asks_for_one_package_still_win(self): + with redirect_stdout(io.StringIO()): + overrides = _platform_overrides(["protobuf>=7", "Protobuf>=7.1"]) + + self.assertIn("protobuf>=7,>=7.1", overrides) + self.assertNotIn("protobuf==6.33.5", overrides) + + def test_a_repeated_package_is_still_written_line_for_line(self): + # Combining the bounds is only for the comparison; requirements.txt + # keeps exactly what the app asked for. + with tempfile.TemporaryDirectory() as tmpdir: + project = Path(tmpdir) + yaml_path = project / "ExampleAgent.yaml" + yaml_path.write_text(yaml.safe_dump({"agent": {"name": "ExampleAgent"}})) + agent_file = _write(project / "agent.py", "print('ok')\n") + output_dir = os.path.join(tmpdir, "out") + with redirect_stdout(io.StringIO()): + generate_docker( + str(yaml_path), + str(agent_file), + output_dir=output_dir, + requirements=["protobuf>=7", "Protobuf>=7.1"], + ) + requirements = _read_requirements(output_dir) + + self.assertEqual(requirements[-2:], ["protobuf>=7", "Protobuf>=7.1"]) + + def test_the_workflow_context_fails_on_a_conflict_too(self): + with self.assertRaises(DependencyPinConflict): + self._context(["protobuf<5"], workflow=True) + def test_the_workflow_context_decides_the_same_way(self): overrides, notes = self._context(["protobuf>=7"], workflow=True) self.assertIn("protobuf>=7", overrides) @@ -629,3 +796,46 @@ def test_both_dockerfiles_install_with_the_overrides_and_report(self): if __name__ == "__main__": unittest.main() + + +class GenerateStubTests(unittest.TestCase): + """The stub is generated from the declaration the schema checked.""" + + def _generate(self, text, env=None): + with tempfile.TemporaryDirectory() as tmpdir: + yaml_path = Path(tmpdir) / "hello.yaml" + yaml_path.write_text(text) + output_path = Path(tmpdir) / "stubs" / "hello.py" + with ( + unittest.mock.patch.dict(os.environ, env or {}), + redirect_stdout(io.StringIO()), + ): + source = stub_generator.generate_stub(str(yaml_path), str(output_path)) + compile(source, "hello.py", "exec") + return source + + def test_a_blank_list_or_type_is_generated_as_absent(self): + for text in ( + "agent:\n name: Hello\n functions:\n", + "agent:\n name: Hello\n functions:\n - name: hello\n arguments:\n", + "agent:\n name: Hello\n functions:\n - name: hello\n" + " arguments:\n - name: a\n type:\n", + ): + with self.subTest(text=text): + self.assertIn("class Hello(object):", self._generate(text)) + + def test_env_references_are_expanded_into_the_stub(self): + source = self._generate( + "agent:\n name: Hello\n functions:\n - name: ${STUB_FN_NAME}\n", + env={"STUB_FN_NAME": "hello"}, + ) + + self.assertIn("def hello(self)", source) + + def test_a_type_built_from_builtins_is_written_as_its_annotation(self): + source = self._generate( + "agent:\n name: Hello\n functions:\n - name: hello\n" + " arguments:\n - name: a\n type: dict[str, int] | None\n" + ) + + self.assertIn("def hello(self, a: dict[str, int] | None)", source)