diff --git a/README.md b/README.md index 5236328..81d944f 100644 --- a/README.md +++ b/README.md @@ -71,6 +71,15 @@ cp -r ../examples/* ./ #### Step 1: Configure the Global Controller Edit `config/global_controller.yaml` in your project directory to list the agents you want to deploy, their `provider`, `replicas`, and resource limits. Add a per-agent `requirements: [pkg, ...]` list for any extra pip packages that agent's code imports — only a small base list (grpc, redis, pyyaml, psutil, etc.) is installed by default. +#### Step 1.1: Passing secrets to agents (optional) + +Agents that need API keys read them from environment variables. Point `env_file` at a `.env` file to have Ventis inject it into every agent container: + +```yaml +# config/global_controller.yaml +env_file: .env +``` + #### Step 2: Build the project ```bash ventis build diff --git a/ventis/cli.py b/ventis/cli.py index 4b5c95e..340f6dd 100644 --- a/ventis/cli.py +++ b/ventis/cli.py @@ -16,6 +16,8 @@ import subprocess import sys +from ventis.controller.utils.env_file import resolve_env_file + logging.basicConfig(level=logging.INFO) logger = logging.getLogger("ventis") DEFAULT_DOCKER_PLATFORM = "linux/amd64" @@ -409,6 +411,14 @@ def cmd_deploy(args): config = _load_config(config_path) project_dir = os.getcwd() + # Fail here rather than after a fleet of containers is already up without + # the API keys they need. + try: + resolve_env_file(config, base_dir=project_dir) + except ValueError as e: + logger.error("%s", e) + sys.exit(1) + _ensure_grpc_stubs_importable(project_dir) if any( diff --git a/ventis/controller/cloud_provider_logic/EC2/_runtime.py b/ventis/controller/cloud_provider_logic/EC2/_runtime.py index 4d5f766..9955fa2 100644 --- a/ventis/controller/cloud_provider_logic/EC2/_runtime.py +++ b/ventis/controller/cloud_provider_logic/EC2/_runtime.py @@ -23,6 +23,7 @@ import boto3 +from ventis.controller.utils.env_file import env_file_args from ventis.controller.utils.redis_utils import _wait_for_redis from ventis.utils.redis_client import RedisClient @@ -285,8 +286,15 @@ def _bootstrap_instance(host, spec, replica_index, cfg, redis_host, redis_port, cmd.extend(["-e", f"VENTIS_DATABASE_URL={db_url}"]) if project_id: cmd.extend(["-e", f"VENTIS_PROJECT_ID={project_id}"]) - cmd.append(image) - result = _controller._run_cmd(cmd, host, user=ssh_user) + + # User secrets from `env_file`. Explicit -e flags above still win over + # anything in the file. + with env_file_args( + _controller, host, ssh_user, container_name, is_local=False + ) as env_args: + cmd.extend(env_args) + cmd.append(image) + result = _controller._run_cmd(cmd, host, user=ssh_user) if result.returncode != 0: raise RuntimeError( f"SSH bootstrap failed on {host}: {(result.stderr or result.stdout or '').strip()}" diff --git a/ventis/controller/cloud_provider_logic/Local/_runtime.py b/ventis/controller/cloud_provider_logic/Local/_runtime.py index 963eef3..a387f7b 100644 --- a/ventis/controller/cloud_provider_logic/Local/_runtime.py +++ b/ventis/controller/cloud_provider_logic/Local/_runtime.py @@ -8,6 +8,8 @@ import logging +from ventis.controller.utils.env_file import env_file_args + logger = logging.getLogger(__name__) DEFAULT_HOST = "localhost" @@ -110,9 +112,15 @@ def bootstrap_instance(provisioned, spec, replica_index, agent_id): cmd.extend(["--memory", f"{resources['memory']}m"]) if resources.get("gpu"): cmd.extend(["--gpus", str(resources["gpu"])]) - cmd.append(image) - result = _require_controller()._run_cmd(cmd, host, user) + # User secrets from `env_file`. Explicit -e flags above still win, so a + # stray VENTIS_* line in someone's .env cannot break agent wiring. + with env_file_args( + _require_controller(), host, user, runtime_id, _is_local_host(host) + ) as env_args: + cmd.extend(env_args) + cmd.append(image) + result = _require_controller()._run_cmd(cmd, host, user) if result.returncode != 0: raise RuntimeError(f"Failed to launch {runtime_id}") diff --git a/ventis/controller/global_controller.py b/ventis/controller/global_controller.py index 1e24f10..0b30307 100644 --- a/ventis/controller/global_controller.py +++ b/ventis/controller/global_controller.py @@ -4,6 +4,7 @@ import atexit import logging +import shlex import signal import subprocess import threading @@ -16,6 +17,7 @@ import yaml from ventis.controller.instance_manager import InstanceManager from ventis.controller.utils.agent_specs import write_agent_specs +from ventis.controller.utils.env_file import resolve_env_file from ventis.controller.utils.redis_utils import _wait_for_redis from ventis.controller.utils.telemetry_logging import ( assign_project_id, @@ -64,6 +66,9 @@ class GlobalController(object): def __init__(self, config_path): self.config_path = config_path self.config = self._load_config(config_path) + # Validate before launching anything: an agent that boots without its + # API keys fails deep inside a container, where it is expensive to debug. + self.env_file_path = resolve_env_file(self.config) redis_cfg = self.config.get("redis", {}) self.redis = RedisClient( @@ -174,6 +179,7 @@ def reload_config(self): """Reload the config file and rebuild the routing table.""" logger.info("Reloading config from %s", self.config_path) self.config = self._load_config(self.config_path) + self.env_file_path = resolve_env_file(self.config) self.controllers = self.config.get("agents", []) self.poll_interval = self.config.get("poll_interval", 5) assign_project_id(self.config.get("project_id", 0)) @@ -642,6 +648,28 @@ def _send(instance): # Runtime launching # # ------------------------------------------------------------------ # + def _ssh_args(self, host, user=None): + """Return the `ssh ... target` prefix used to reach a remote host.""" + ssh_key_path = os.path.expanduser( + self.config.get("ec2", {}).get("ssh_private_key_path", "~/.ssh/ventis_ec2") + ) + return [ + "ssh", + "-o", + "StrictHostKeyChecking=no", + "-o", + "IdentitiesOnly=yes", + "-o", + "ConnectTimeout=10", + "-o", + "ServerAliveInterval=10", + "-o", + "ServerAliveCountMax=3", + "-i", + ssh_key_path, + f"{user}@{host}" if user else host, + ] + def _run_cmd(self, cmd, host, user=None): """ Run a command locally or on a remote host via SSH. @@ -656,41 +684,52 @@ def _run_cmd(self, cmd, host, user=None): """ is_local = _is_local_host(host) if is_local: - return subprocess.run( - cmd, capture_output=True, text=True, timeout=180 - ) - else: - ssh_key_path = os.path.expanduser( - self.config.get("ec2", {}).get( - "ssh_private_key_path", "~/.ssh/ventis_ec2" - ) - ) - ssh_target = f"{user}@{host}" if user else host - remote_cmd = " ".join(cmd) - if cmd and cmd[0] == "docker": - remote_cmd = f"sudo {remote_cmd}" - return subprocess.run( - [ - "ssh", - "-o", - "StrictHostKeyChecking=no", - "-o", - "IdentitiesOnly=yes", - "-o", - "ConnectTimeout=10", - "-o", - "ServerAliveInterval=10", - "-o", - "ServerAliveCountMax=3", - "-i", - ssh_key_path, - ssh_target, - remote_cmd, - ], + return subprocess.run(cmd, capture_output=True, text=True, timeout=180) + + remote_cmd = " ".join(cmd) + if cmd and cmd[0] == "docker": + remote_cmd = f"sudo {remote_cmd}" + return subprocess.run( + self._ssh_args(host, user) + [remote_cmd], + capture_output=True, + text=True, + timeout=180, + ) + + def _push_file(self, local_path, remote_path, host, user=None): + """ + Copy a local file to a remote host over SSH. + + Streams the bytes through `cat` under `umask 077` rather than using + `scp`, so a secrets file is never briefly world-readable on the far + side. + + Anything already sitting at the destination is removed first: `umask` + only governs files the shell creates, and `>` follows symlinks. Without + the `rm`, a local user on the remote host could pre-create the path + world-readable, or point it at a file of their own, and collect + whatever we write there. + + Returns: + subprocess.CompletedProcess + """ + quoted = shlex.quote(remote_path) + remote_cmd = f"umask 077; rm -f {quoted}; cat > {quoted}" + with open(local_path, "rb") as f: + result = subprocess.run( + self._ssh_args(host, user) + [remote_cmd], + stdin=f, capture_output=True, text=True, timeout=180, + check=False, + ) + if result.returncode != 0: + raise RuntimeError( + f"Failed to copy {local_path} to {host}:{remote_path}: " + f"{(result.stderr or result.stdout or '').strip()}" ) + return result def launch_docker_agents(self): """Launch all configured runtimes through InstanceManager.""" diff --git a/ventis/controller/utils/env_file.py b/ventis/controller/utils/env_file.py new file mode 100644 index 0000000..b1f9381 --- /dev/null +++ b/ventis/controller/utils/env_file.py @@ -0,0 +1,159 @@ +""" +Pass user secrets (API keys and friends) into agent containers. + +Two scopes, never mixed: `env_file` in the config (self-hosted), and +`DEFAULT_SECRETS_FILE` (managed -- the project's `.env` was never uploaded +there). Below `resolve_env_file` both are just a path: containers on this +machine read it directly, remote ones get a short-lived 0600 copy, and either +way it reaches Docker as `--env-file`. +""" + +import logging +import os +import re +from contextlib import contextmanager + +logger = logging.getLogger(__name__) + +REMOTE_ENV_DIR = "/tmp" +_UNSAFE_PATH_CHARS = re.compile(r"[^A-Za-z0-9_.-]") + +# Where a managed deployment leaves the user's secrets. /var/run is tmpfs, so +# the file dies with the host instead of persisting on disk. +DEFAULT_SECRETS_FILE = "/var/run/ventis/secrets.env" + + +def platform_secrets_file(): + """ + The path a managed deployment leaves the user's secrets at. + + `VENTIS_SECRETS_FILE` overrides it for tests and for deployments that + cannot write under /var/run. Nothing sets it in normal operation. + """ + return os.environ.get("VENTIS_SECRETS_FILE", DEFAULT_SECRETS_FILE) + + +def resolve_env_file(config, base_dir=None): + """ + Return the absolute path of the env file to hand containers, or None. + + A file at `platform_secrets_file()` wins over `env_file`. A convention + beating an explicit setting only makes sense because the two describe + different machines: `env_file: .env` describes the user's own, and + carrying that config to a managed deployment does not make the `.env` + exist there. So it warns rather than raising. + + The platform must create that file unconditionally, empty when no secrets + are configured -- its presence means "managed", not "has secrets". + + Raises: + ValueError: the file is configured but unusable. Deploy should fail + here rather than start a fleet of agents with no API keys. + """ + platform_path = platform_secrets_file() + if os.path.isfile(platform_path): + if not os.access(platform_path, os.R_OK): + raise ValueError(f"platform secrets file is not readable: {platform_path}") + if config.get("env_file"): + logger.warning( + "env_file %r is ignored in a managed deployment: secrets come " + "from the platform. Configure them on the platform instead.", + config["env_file"], + ) + return platform_path + + return _resolve_project_env_file(config, base_dir) + + +def _resolve_project_env_file(config, base_dir): + """ + Resolve the `env_file` a self-hosted deployment points at, or None. + + Relative paths resolve against `base_dir` (default: the current working + directory), matching how `entrypoint` and `workflow_file` are resolved. + """ + raw = config.get("env_file") + if not raw: + return None + + path = os.path.expanduser(str(raw)) + if not os.path.isabs(path): + path = os.path.join(base_dir or os.getcwd(), path) + path = os.path.abspath(path) + + if not os.path.exists(path): + raise ValueError(f"env_file does not exist: {path} (from env_file: {raw})") + if not os.path.isfile(path): + raise ValueError(f"env_file is not a file: {path} (from env_file: {raw})") + if not os.access(path, os.R_OK): + raise ValueError(f"env_file is not readable: {path}") + return path + + +def remote_env_path(container_name): + """ + Where a remote host holds this container's copy of the env file. + + The name is scrubbed down to a shell-safe alphabet. This path is + interpolated into remote commands that `_run_cmd` joins with spaces and + hands to a shell unquoted, so a container name carrying a space would + split the cleanup `rm` into two harmless arguments -- it would exit 0 + while the secrets stayed on the host, with nothing in the log to say so. + """ + safe_name = _UNSAFE_PATH_CHARS.sub("-", container_name) + return f"{REMOTE_ENV_DIR}/ventis-env-{safe_name}" + + +@contextmanager +def env_file_args(controller, host, user, container_name, is_local): + """ + Yield the `docker run` flags that hand the user's env file to a container. + + A container on this machine reads the original file. A container on a + remote host gets a 0600 copy, deleted as soon as the `with` body ends -- + success or failure, since by then the container holds the variables + itself. Keep that body tight around `docker run` so the copy is never + on the host longer than it has to be. + + Yields an empty list when no env file is configured, and when the one + configured is empty. + """ + env_file_path = getattr(controller, "env_file_path", None) + if not env_file_path or _has_no_variables(env_file_path): + yield [] + return + + if is_local: + yield ["--env-file", env_file_path] + return + + remote_path = remote_env_path(container_name) + controller._push_file(env_file_path, remote_path, host, user=user) + try: + yield ["--env-file", remote_path] + finally: + _remove_remote_copy(controller, remote_path, host, user) + + +def _has_no_variables(path): + """ + An empty platform file is routine, so skip it and spare a remote host a + copy pushed and deleted per container. A file that vanished since + resolution is not "empty" -- let it through so Docker reports it. + """ + try: + return os.path.getsize(path) == 0 + except OSError: + return False + + +def _remove_remote_copy(controller, remote_path, host, user): + """Delete a remote copy. Best effort -- never masks the caller's error.""" + try: + result = controller._run_cmd(["rm", "-f", remote_path], host, user=user) + if getattr(result, "returncode", 0) != 0: + logger.warning("Failed to delete env file copy %s on %s", remote_path, host) + except Exception as e: + logger.warning( + "Failed to delete env file copy %s on %s: %s", remote_path, host, e + )