From 603225694cb21d141ac7fabf5da4e823a0371438 Mon Sep 17 00:00:00 2001 From: Dmitrii Iurco Date: Wed, 10 Jun 2026 22:56:31 -0400 Subject: [PATCH] =?UTF-8?q?feat:=20connectivity=20redesign=20phase=205=20?= =?UTF-8?q?=E2=80=94=20one=20container=20per=20connection=20instance?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit instanceable rendering, per-instance up/down on create/delete, store-service-installed gate, per-instance health Co-Authored-By: Claude Fable 5 --- api/connectivity_manager.py | 242 ++++++++++++++++++++++++- api/managers.py | 3 + api/manifest_validator.py | 17 +- api/service_composer.py | 153 +++++++++++++++- api/service_store_manager.py | 6 +- tests/test_connectivity_connections.py | 162 +++++++++++++++++ tests/test_manifest_validator.py | 16 ++ tests/test_service_composer.py | 97 ++++++++++ 8 files changed, 688 insertions(+), 8 deletions(-) diff --git a/api/connectivity_manager.py b/api/connectivity_manager.py index 08c2cba..ce8d421 100644 --- a/api/connectivity_manager.py +++ b/api/connectivity_manager.py @@ -199,12 +199,15 @@ class ConnectivityManager(BaseServiceManager): CONNECTIVITY_CHAIN = 'PIC_CONNECTIVITY' def __init__(self, config_manager=None, peer_registry=None, - vault_manager=None, + vault_manager=None, service_composer=None, data_dir: str = '/app/data', config_dir: str = '/app/config'): super().__init__('connectivity', data_dir, config_dir) self.config_manager = config_manager self.peer_registry = peer_registry self.vault_manager = vault_manager + # Set after construction in managers.py (composer is built later) — used + # to bring per-connection containers up/down out-of-process. + self.service_composer = service_composer # Serializes connection CRUD + resource allocation across threads. self._conn_lock = threading.RLock() @@ -924,6 +927,206 @@ class ConnectivityManager(BaseServiceManager): def _new_conn_id() -> str: return f"conn_{secrets.token_hex(4)}" + # ── Connectivity v2 — per-instance container lifecycle ──────────────── + # + # Each connection instance runs its own container, named + # cell--, derived from the connection record (NOT from + # the fixed EXIT_CONTAINERS map, which is the legacy single-container view). + # The backing store service must be installed first — its image and raw + # compose template come from the install record. Tor is single-instance. + + # short container slug per type — must match the instanceable + # compose-template container_name prefixes in pic-services. + CONTAINER_SLUGS = { + "wireguard_ext": "wgext", + "openvpn": "ovpn", + "sshuttle": "sshuttle", + "proxy": "proxy", + } + + def instance_container_name(self, conn: Dict[str, Any]) -> Optional[str]: + """Derive the per-instance container name from a connection record. + + Tor keeps its fixed single-instance name (cell-tor). All other types + get cell--. + """ + conn_type = conn.get('type') + if conn_type == 'tor': + return self.EXIT_CONTAINERS.get('tor') + slug = self.CONTAINER_SLUGS.get(conn_type) + if not slug: + return None + return f"cell-{slug}-{self._instance_id(conn)}" + + @staticmethod + def _instance_id(conn: Dict[str, Any]) -> str: + """Short, docker-safe instance id derived from the connection id.""" + return str(conn.get('id', '')).split('_')[-1][:12] + + def _store_record_for(self, conn_type: str) -> Optional[Dict[str, Any]]: + """Return the install record for the store service backing conn_type.""" + svc_id = self.STORE_SERVICE_IDS.get(conn_type) + if not svc_id or self.config_manager is None: + return None + try: + installed = self.config_manager.get_installed_services() + except Exception as e: + logger.warning(f"_store_record_for({conn_type}): {e}") + return None + if not isinstance(installed, dict): + return None + return installed.get(svc_id) + + def _materialize_instance_config(self, conn: Dict[str, Any], + config_dir: str) -> None: + """Write the per-instance config files the container's entrypoint reads. + + Pulls secrets from the vault by the connection's secret_refs and the + non-secret fields from the record's config. Files land in the + per-instance config dir (bind-mounted into the container). + """ + conn_type = conn.get('type') + config = conn.get('config', {}) or {} + secrets_map = self._load_instance_secrets(conn) + os.makedirs(config_dir, exist_ok=True) + + if conn_type == 'wireguard_ext': + conf = secrets_map.get('conf', '') + if conf: + self._write_secure(os.path.join(config_dir, 'wg_ext0.conf'), conf) + elif conn_type == 'openvpn': + conf = secrets_map.get('conf', '') + if conf: + self._write_secure(os.path.join(config_dir, 'client.ovpn'), conf) + elif conn_type == 'sshuttle': + self._materialize_sshuttle_config(conn, config, secrets_map, config_dir) + elif conn_type == 'proxy': + self._write_secure( + os.path.join(config_dir, 'redsocks.conf'), + self._render_redsocks_for_instance(conn, config, secrets_map)) + + def _load_instance_secrets(self, conn: Dict[str, Any]) -> Dict[str, str]: + """Resolve a connection's secret_refs to {field: value} from the vault.""" + out: Dict[str, str] = {} + conn_id = str(conn.get('id', '')) + if self.vault_manager is None: + return out + for ref in conn.get('secret_refs', []): + field = ref[len(conn_id) + 1:] if ref.startswith(conn_id + '_') else ref + try: + value = self.vault_manager.get_secret(ref) + except Exception as e: + logger.warning(f"_load_instance_secrets: read {ref} failed: {e}") + value = None + if value is not None: + out[field] = value + return out + + def _materialize_sshuttle_config(self, conn, config, secrets_map, + config_dir) -> None: + listen_port = conn.get('redirect_port') or self.SSHUTTLE_PORT + conf_lines = [ + f"HOST={config.get('host', '')}", + f"PORT={config.get('port', 22)}", + f"USER={config.get('user', '')}", + f"AUTH={config.get('auth', 'key')}", + f"LISTEN_PORT={listen_port}", + f"EXCLUDE={','.join(config.get('exclude_subnets', []) or [])}", + ] + if 'known_hosts' in secrets_map: + self._write_secure(os.path.join(config_dir, 'known_hosts'), + secrets_map['known_hosts'].rstrip('\n') + '\n') + if config.get('auth') == 'password': + if 'password' in secrets_map: + self._write_secure(os.path.join(config_dir, 'password'), + secrets_map['password'] + '\n') + else: + if 'private_key' in secrets_map: + self._write_secure(os.path.join(config_dir, 'id_pic'), + secrets_map['private_key'].rstrip('\n') + '\n') + self._write_secure(os.path.join(config_dir, 'sshuttle.conf'), + '\n'.join(conf_lines) + '\n') + + def _render_redsocks_for_instance(self, conn, config, secrets_map) -> str: + local_port = conn.get('redirect_port') or self.REDSOCKS_PORT + redsocks_type = 'socks5' if config.get('scheme') == 'socks5' else 'http-connect' + lines = [ + 'base {', + ' log_debug = off;', + ' log_info = on;', + ' log = stderr;', + ' daemon = off;', + ' redirector = iptables;', + '}', + '', + 'redsocks {', + ' local_ip = 0.0.0.0;', + f' local_port = {local_port};', + f" ip = {config.get('host', '')};", + f" port = {config.get('port', '')};", + f' type = {redsocks_type};', + ] + if config.get('user'): + lines.append(f" login = \"{config['user']}\";") + if secrets_map.get('password'): + lines.append(f" password = \"{secrets_map['password']}\";") + lines.append('}') + return '\n'.join(lines) + '\n' + + def up_connection_instance(self, conn: Dict[str, Any]) -> Dict[str, Any]: + """Render config + compose for one connection and bring its container up. + + Requires the backing store service to be installed (its image + raw + compose template come from the install record). Tor is single-instance + and is started through the plain store-service path, not here. + Returns {'ok': bool, ...}; never raises. + """ + conn_type = conn.get('type') + if conn_type in self.SINGLE_INSTANCE_TYPES: + return {'ok': True, 'single_instance': True} + if self.service_composer is None: + return {'ok': False, 'error': 'service_composer unavailable'} + + record = self._store_record_for(conn_type) + if not record: + svc_id = self.STORE_SERVICE_IDS.get(conn_type, conn_type) + return {'ok': False, + 'error': f"store service {svc_id!r} is not installed; " + f'install it before creating a {conn_type} connection'} + template = record.get('compose_template') + manifest = record.get('manifest') or {} + if not template: + return {'ok': False, + 'error': f'store service for {conn_type} has no compose ' + 'template (reinstall required for per-instance support)'} + + svc_id = self.STORE_SERVICE_IDS[conn_type] + instance_id = self._instance_id(conn) + try: + config_dir = self.service_composer.instance_config_dir(svc_id, instance_id) + self._materialize_instance_config(conn, config_dir) + except Exception as e: + logger.error(f"up_connection_instance: config write failed: {e}") + return {'ok': False, 'error': 'failed to write instance config'} + + return self.service_composer.up_instance( + svc_id, instance_id, manifest, template, + redirect_port=conn.get('redirect_port')) + + def down_connection_instance(self, conn: Dict[str, Any], + purge_data: bool = True) -> Dict[str, Any]: + """Bring down + clean up one connection's container, compose and config.""" + conn_type = conn.get('type') + if conn_type in self.SINGLE_INSTANCE_TYPES: + return {'ok': True, 'single_instance': True} + if self.service_composer is None: + return {'ok': False, 'error': 'service_composer unavailable'} + svc_id = self.STORE_SERVICE_IDS.get(conn_type) + if not svc_id: + return {'ok': True} + return self.service_composer.down_instance( + svc_id, self._instance_id(conn), purge_data=purge_data) + # ── Connectivity v2 — connection CRUD ───────────────────────────────── def _compute_state(self, conn_type: str, config: Dict[str, Any], @@ -978,6 +1181,15 @@ class ConnectivityManager(BaseServiceManager): if err: return {'ok': False, 'error': err} + # The backing store service must be installed before a connection of + # its type can run a container. Tor (single instance) installs its own + # container via the store path, so the gate applies to it too. + if not self._store_service_installed(conn_type): + svc_id = self.STORE_SERVICE_IDS.get(conn_type, conn_type) + return {'ok': False, 'error': + f"the {svc_id!r} service must be installed from the Service " + f'Store before creating a {conn_type} connection'} + with self._conn_lock: existing = [] if self.config_manager is not None: @@ -1059,7 +1271,18 @@ class ConnectivityManager(BaseServiceManager): logger.info(f"connectivity: created connection {conn_id} " f"({conn_type}/{name}) mark={hex(mark)} table={table}") - return {'ok': True, 'connection': self._public_record(record)} + + # Bring up this connection's own container (cell--). Tor is + # single-instance and runs via the store path, so up is a no-op there. + # A failure here is non-fatal to the record — the connection still + # exists and can be retried via apply/up — but is surfaced to the + # caller so the UI can show it. + up = self.up_connection_instance(record) + result = {'ok': True, 'connection': self._public_record(record)} + if not up.get('ok') and not up.get('single_instance'): + result['container'] = {'ok': False, + 'error': up.get('error') or up.get('stderr')} + return result def update_connection(self, conn_id: str, name: Optional[str] = None, config: Optional[Dict[str, Any]] = None, @@ -1138,6 +1361,15 @@ class ConnectivityManager(BaseServiceManager): return {'ok': False, 'error': f'connection is in use by {ref}; detach it first'} + # Tear down this connection's container + its per-instance compose + # and config before forgetting the record (best-effort; a stale + # container must not block deletion of the config entry). + try: + self.down_connection_instance(record) + except Exception as e: + logger.warning(f"delete_connection: container teardown failed " + f"(non-fatal): {e}") + for secret_ref in record.get('secret_refs', []): if self.vault_manager is not None: try: @@ -1719,7 +1951,7 @@ class ConnectivityManager(BaseServiceManager): iface = conn.get('iface') if not iface: return 'unknown', 'no interface assigned' - container = self.EXIT_CONTAINERS.get('wireguard_ext') + container = self.instance_container_name(conn) r = self._exec_in_container( container, ['wg', 'show', iface, 'latest-handshakes']) if r is None or r.returncode != 0: @@ -1742,7 +1974,7 @@ class ConnectivityManager(BaseServiceManager): def _probe_openvpn(self, conn: Dict[str, Any]) -> Tuple[str, Optional[str]]: """An OpenVPN exit is working when its tun iface exists and is UP.""" iface = conn.get('iface') - container = self.EXIT_CONTAINERS.get('openvpn') + container = self.instance_container_name(conn) # The tun device lives in the openvpn container's net namespace. r = self._exec_in_container(container, ['ip', 'link', 'show', iface]) \ if iface else None @@ -1781,7 +2013,7 @@ class ConnectivityManager(BaseServiceManager): if not self._tcp_reachable(host, int(port)): return 'down', f'ssh host {host}:{port} unreachable' listen_port = conn.get('redirect_port') - container = self.EXIT_CONTAINERS.get('sshuttle') + container = self.instance_container_name(conn) if isinstance(listen_port, int) and not self._listener_reachable( container, listen_port): return 'down', f'sshuttle listener :{listen_port} down' diff --git a/api/managers.py b/api/managers.py index bcfbb80..104f9fd 100644 --- a/api/managers.py +++ b/api/managers.py @@ -97,6 +97,9 @@ connectivity_manager = ConnectivityManager( ) service_composer = ServiceComposer(config_manager=config_manager, data_dir=DATA_DIR) +# Connectivity brings one container up per connection instance via the composer; +# wire it now that the composer exists (composer is built after connectivity). +connectivity_manager.service_composer = service_composer account_manager = AccountManager( service_registry=service_registry, data_dir=DATA_DIR, diff --git a/api/manifest_validator.py b/api/manifest_validator.py index 8bc0055..047505b 100644 --- a/api/manifest_validator.py +++ b/api/manifest_validator.py @@ -33,6 +33,13 @@ _RESERVED_CONTAINER_NAMES = frozenset({ 'cell-dnsmasq', 'cell-wireguard', 'cell-chrony', }) _CONTAINER_NAME_RE = re.compile(r'^cell-[a-z0-9][a-z0-9-]{0,30}$') +# Instanceable services template their container name with the connection's +# short id, e.g. "cell-wgext-${INSTANCE_ID}". The literal prefix is validated; +# ${INSTANCE_ID} is substituted at up-time with a hex token that itself matches +# the per-instance naming rules. +_INSTANCEABLE_CONTAINER_NAME_RE = re.compile( + r'^cell-[a-z0-9][a-z0-9-]{0,22}-\$\{INSTANCE_ID\}$' +) _ENV_VALUE_RE = re.compile(r'^[A-Za-z0-9._@:/+\-]{0,256}$') _HOOK_BINARY_RE = re.compile(r'^[a-z][a-z0-9_-]{0,31}$') _CAP_NAME_RE = re.compile(r'^[A-Z_]+$') @@ -93,7 +100,15 @@ def validate_manifest(manifest: dict) -> tuple: # container_name structural check cname = manifest.get('container_name') if cname is not None: - if not _CONTAINER_NAME_RE.match(cname): + instanceable = bool(manifest.get('instanceable')) + if instanceable: + if not _INSTANCEABLE_CONTAINER_NAME_RE.match(cname): + errors.append( + 'instanceable container_name must match ' + "^cell-[a-z0-9][a-z0-9-]{0,22}-${INSTANCE_ID}$, " + f'got: {cname!r}' + ) + elif not _CONTAINER_NAME_RE.match(cname): errors.append( f'container_name must match ^cell-[a-z0-9][a-z0-9-]{{0,30}}$, got: {cname!r}' ) diff --git a/api/service_composer.py b/api/service_composer.py index a982d79..8007ca6 100644 --- a/api/service_composer.py +++ b/api/service_composer.py @@ -114,10 +114,17 @@ class ServiceComposer: # ── Template rendering ──────────────────────────────────────────────── def render_template(self, service_id: str, manifest: Dict, - template_content: str) -> str: + template_content: str, + instance_vars: Optional[Dict[str, str]] = None) -> str: """ Substitute all PIC_* variables in a compose-template.yml string. Returns the rendered compose YAML. + + instance_vars optionally supplies per-connection-instance values for + ${INSTANCE_ID} and ${REDIRECT_PORT} so an instanceable connectivity + service can be rendered once per connection without collisions. They + are ignored for non-instanceable services (the placeholders simply + never appear in the template). """ schema = manifest.get('config_schema') or {} saved = self.cm.configs.get(service_id, {}) @@ -141,6 +148,13 @@ class ServiceComposer: result = result.replace('${PIC_SERVICE_ID}', service_id) result = result.replace('${PIC_DATA_DIR}', str(Path(self.data_dir).resolve())) + if instance_vars: + for var in ('INSTANCE_ID', 'REDIRECT_PORT'): + if var in instance_vars and instance_vars[var] is not None: + safe = str(instance_vars[var]).replace('\n', '').replace( + '\r', '').replace('\t', ' ') + result = result.replace(f'${{{var}}}', safe) + # PIC_SECRET_* — generate on first use, reuse on reconfigure for match in _SECRET_RE.finditer(template_content): var_name = match.group(1) @@ -294,6 +308,143 @@ class ServiceComposer: pass return result + # ── Connection-instance lifecycle (one container per connection) ────── + # + # An instanceable connectivity service (wireguard-ext / openvpn-client / + # sshuttle / proxy) backs MANY connections — one container per connection. + # The store service supplies the image + raw compose-template; each + # connection renders that template with its own ${INSTANCE_ID} (short id), + # ${REDIRECT_PORT} and a per-instance config dir, so two connections of the + # same type never collide on container name, config mount, or listen port. + # + # Layout (all under data/services///): + # docker-compose.yml rendered per-instance compose + # config/ per-instance bind-mounted config dir + # Tor is single-instance and keeps using the plain store-service path. + + @staticmethod + def instance_id_for(conn_id: str) -> str: + """Derive a short, docker-safe INSTANCE_ID from a connection id.""" + return conn_id.split('_')[-1][:12] + + def _instance_dir(self, service_id: str, instance_id: str) -> str: + self._validate_service_id(service_id) + if not _SAFE_ID_RE.match(instance_id): + raise ValueError(f'invalid instance_id {instance_id!r}') + candidate = os.path.join(self._svc_dir(service_id), instance_id) + real_base = os.path.realpath(self._svc_dir(service_id)) + real_cand = os.path.realpath(candidate) + if not real_cand.startswith(real_base + os.sep) and real_cand != real_base: + raise ValueError(f'instance_id {instance_id!r} escapes service directory') + return candidate + + def _instance_compose_path(self, service_id: str, instance_id: str) -> str: + return os.path.join(self._instance_dir(service_id, instance_id), + 'docker-compose.yml') + + def instance_config_dir(self, service_id: str, instance_id: str) -> str: + """Per-instance config dir that the compose template bind-mounts.""" + return os.path.join(self._instance_dir(service_id, instance_id), 'config') + + def has_instance_compose(self, service_id: str, instance_id: str) -> bool: + try: + return os.path.exists(self._instance_compose_path(service_id, instance_id)) + except ValueError: + return False + + def write_instance_compose(self, service_id: str, instance_id: str, + manifest: Dict, template_content: str, + redirect_port: Optional[int] = None) -> str: + """Render + atomically write a per-instance compose file. Returns content.""" + inst_dir = self._instance_dir(service_id, instance_id) + os.makedirs(os.path.join(inst_dir, 'config'), exist_ok=True) + + instance_vars = {'INSTANCE_ID': instance_id} + if redirect_port is not None: + instance_vars['REDIRECT_PORT'] = str(redirect_port) + content = self.render_template( + service_id, manifest, template_content, instance_vars=instance_vars) + + allow_host_network = bool(manifest.get('requires_host_network')) + ok, errs = validate_rendered_compose( + content, + allowed_data_dir=str(Path(self.data_dir).resolve()), + allow_host_network=allow_host_network, + ) + if not ok: + raise ValueError( + f'Instance compose failed security validation: {"; ".join(errs)}') + + path = self._instance_compose_path(service_id, instance_id) + tmp = path + '.tmp' + with open(tmp, 'w') as f: + f.write(content) + f.flush() + os.fsync(f.fileno()) + os.replace(tmp, path) + logger.info('ServiceComposer: wrote instance compose %s/%s', + service_id, instance_id) + return content + + def _instance_cmd(self, service_id: str, instance_id: str, *args, + timeout: int = 120) -> Dict: + compose_file = self._instance_compose_path(service_id, instance_id) + if not os.path.exists(compose_file): + return {'ok': False, + 'error': f'No compose file for instance {service_id}/{instance_id}'} + cmd = [ + 'docker', 'compose', + '-f', compose_file, + '--project-name', f'pic-conn-{instance_id}', + *args, + ] + return self._run(cmd, timeout) + + def up_instance(self, service_id: str, instance_id: str, manifest: Dict, + template_content: str, + redirect_port: Optional[int] = None) -> Dict: + """Render + bring up the container for one connection instance.""" + try: + self.write_instance_compose(service_id, instance_id, manifest, + template_content, redirect_port) + except ValueError as e: + return {'ok': False, 'error': str(e)} + return self._instance_cmd(service_id, instance_id, 'up', '-d', + '--remove-orphans', timeout=600) + + def down_instance(self, service_id: str, instance_id: str, + purge_data: bool = False) -> Dict: + """Stop the connection instance's container and remove its compose/dir.""" + result = {'ok': True} + if self.has_instance_compose(service_id, instance_id): + args = ['down'] + if purge_data: + args.append('--volumes') + result = self._instance_cmd(service_id, instance_id, *args) + try: + inst_dir = self._instance_dir(service_id, instance_id) + except ValueError as e: + logger.warning('down_instance: %s', e) + return result + if os.path.isdir(inst_dir): + real_inst = os.path.realpath(inst_dir) + real_base = os.path.realpath(self._svc_dir(service_id)) + if not real_inst.startswith(real_base + os.sep): + logger.error('ServiceComposer: refusing rmtree outside service dir: %s', + inst_dir) + else: + try: + shutil.rmtree(inst_dir) + except OSError as e: + logger.warning('ServiceComposer: could not remove %s: %s', + inst_dir, e) + return result + + def status_instance(self, service_id: str, instance_id: str) -> Dict: + result = self._instance_cmd(service_id, instance_id, 'ps', '--format', 'json') + result['containers'] = self._parse_ps_json(result.get('stdout', '')) + return result + # ── Dependency resolution ───────────────────────────────────────────── def _resolve_requires(self, manifest: Dict, installed_services: Dict) -> Optional[str]: diff --git a/api/service_store_manager.py b/api/service_store_manager.py index aa1f961..bc505b5 100644 --- a/api/service_store_manager.py +++ b/api/service_store_manager.py @@ -329,12 +329,16 @@ class ServiceStoreManager(BaseServiceManager): if not result.get('ok'): return {'ok': False, 'error': result.get('error') or result.get('stderr', 'docker up failed')} - # Persist minimal install record + # Persist minimal install record. For instanceable connectivity + # services the raw compose template is stored so ConnectivityManager + # can render one container per connection instance without re-fetching. record = { 'id': service_id, 'manifest': manifest, 'installed_at': datetime.utcnow().isoformat(), } + if manifest.get('instanceable'): + record['compose_template'] = template_content self.config_manager.set_installed_service(service_id, record) # Regenerate Caddy (registry now drives routes, no caddy_routes list needed) diff --git a/tests/test_connectivity_connections.py b/tests/test_connectivity_connections.py index 126968d..dc12fc0 100644 --- a/tests/test_connectivity_connections.py +++ b/tests/test_connectivity_connections.py @@ -100,6 +100,15 @@ class _Base(unittest.TestCase): # is exercised separately in TestMigration; here we want an empty slate # so the always-configured Tor default doesn't get auto-migrated in. self.cm.configs['connectivity'] = {'version': 2, 'connections': []} + # P5 gate: a connection's backing store service must be installed before + # it can be created. Mark all connectivity store services installed so + # the CRUD tests below exercise allocation/persistence, not the gate + # (the gate itself is tested explicitly in TestStoreServiceGate). The + # per-instance container up/down is a no-op here: the manager has no + # service_composer wired (service_composer=None), so up_connection_instance + # short-circuits without touching Docker. + for _svc in ('wireguard-ext', 'openvpn-client', 'tor', 'sshuttle', 'proxy'): + self.cm.set_installed_service(_svc, {'id': _svc, 'manifest': {}}) self.cm._save_all_configs() def tearDown(self): @@ -469,5 +478,158 @@ class TestMigration(_Base): self.assertIsNotNone(c['redirect_port']) +# --------------------------------------------------------------------------- +# P5 — per-instance container lifecycle wiring +# --------------------------------------------------------------------------- + +_PROXY_TEMPLATE = ( + 'services:\n' + ' proxy:\n' + ' image: git.pic.ngo/roof/svc-redsocks:latest\n' + ' container_name: cell-proxy-${INSTANCE_ID}\n' + ' network_mode: host\n' + ' cap_add:\n' + ' - NET_ADMIN\n' + ' environment:\n' + ' - REDSOCKS_LOCAL_PORT=${REDIRECT_PORT}\n' + ' volumes:\n' + ' - ${PIC_DATA_DIR}/services/proxy/${INSTANCE_ID}/config:/config\n' +) + +_SSHUTTLE_TEMPLATE = ( + 'services:\n' + ' sshuttle:\n' + ' image: git.pic.ngo/roof/svc-sshuttle:latest\n' + ' container_name: cell-sshuttle-${INSTANCE_ID}\n' + ' network_mode: host\n' + ' cap_add:\n' + ' - NET_ADMIN\n' + ' environment:\n' + ' - SSHUTTLE_LISTEN_PORT=${REDIRECT_PORT}\n' + ' volumes:\n' + ' - ${PIC_DATA_DIR}/services/sshuttle/${INSTANCE_ID}/config:/config\n' +) + + +class _ComposerBase(_Base): + """Base that wires a real ServiceComposer (subprocess mocked) + install records + that carry a compose template, so create/delete drive per-instance up/down.""" + + TEMPLATES = { + 'proxy': _PROXY_TEMPLATE, + 'sshuttle': _SSHUTTLE_TEMPLATE, + } + + def setUp(self): + super().setUp() + from service_composer import ServiceComposer + cm = MagicMock() + ident = {'cell_name': 'c', 'domain': 'cell.local', 'domain_mode': 'lan'} + cm.get_identity.return_value = ident + cm.get_effective_domain.return_value = 'cell.local' + cm.configs = {} + self.composer = ServiceComposer(config_manager=cm, data_dir=self.data_dir) + self.mgr.service_composer = self.composer + # Re-record install records with compose templates so up_connection_instance + # finds an image + template (the _Base records had empty manifests). + svc_template = { + 'wireguard-ext': _SSHUTTLE_TEMPLATE, # content irrelevant; render only + 'openvpn-client': _SSHUTTLE_TEMPLATE, + 'sshuttle': _SSHUTTLE_TEMPLATE, + 'proxy': _PROXY_TEMPLATE, + 'tor': None, + } + for svc, tmpl in svc_template.items(): + rec = {'id': svc, 'manifest': {'instanceable': bool(tmpl), + 'requires_host_network': True}} + if tmpl: + rec['compose_template'] = tmpl + self.cm.set_installed_service(svc, rec) + self.cm._save_all_configs() + + +class TestCreateBringsUpInstance(_ComposerBase): + + @patch('service_composer.subprocess.run') + def test_create_proxy_triggers_up_and_writes_compose(self, mock_run): + mock_run.return_value = MagicMock(returncode=0, stdout='', stderr='') + res = self.mgr.create_connection('proxy', 'Work proxy', _proxy_cfg()) + self.assertTrue(res['ok'], res) + cid = res['connection']['id'] + inst = cid.split('_')[-1][:12] + # up_instance ran docker compose up for the per-instance project. + cmds = [c.args[0] for c in mock_run.call_args_list] + self.assertTrue(any('up' in cmd and f'pic-conn-{inst}' in cmd + for cmd in cmds), cmds) + # Per-instance compose written to its own dir, not the type-level path. + self.assertTrue(self.composer.has_instance_compose('proxy', inst)) + compose_path = self.composer._instance_compose_path('proxy', inst) + with open(compose_path) as f: + content = f.read() + self.assertIn(f'cell-proxy-{inst}', content) + self.assertIn(str(res['connection']['redirect_port']), content) + # redsocks.conf materialized in the per-instance config dir. + self.assertTrue(os.path.exists(os.path.join( + self.composer.instance_config_dir('proxy', inst), 'redsocks.conf'))) + + @patch('service_composer.subprocess.run') + def test_two_proxies_distinct_containers_and_ports(self, mock_run): + mock_run.return_value = MagicMock(returncode=0, stdout='', stderr='') + r1 = self.mgr.create_connection('proxy', 'p1', _proxy_cfg()) + r2 = self.mgr.create_connection('proxy', 'p2', _proxy_cfg()) + self.assertTrue(r1['ok'] and r2['ok']) + n1 = self.mgr.instance_container_name(r1['connection']) + n2 = self.mgr.instance_container_name(r2['connection']) + self.assertNotEqual(n1, n2) + self.assertNotEqual(r1['connection']['redirect_port'], + r2['connection']['redirect_port']) + + @patch('service_composer.subprocess.run') + def test_delete_brings_down_and_cleans_instance(self, mock_run): + mock_run.return_value = MagicMock(returncode=0, stdout='', stderr='') + res = self.mgr.create_connection('sshuttle', 'tun', _sshuttle_cfg(), + secrets={'private_key': VALID_KEY}) + cid = res['connection']['id'] + inst = cid.split('_')[-1][:12] + self.assertTrue(self.composer.has_instance_compose('sshuttle', inst)) + inst_dir = self.composer._instance_dir('sshuttle', inst) + mock_run.reset_mock() + d = self.mgr.delete_connection(cid) + self.assertTrue(d['ok'], d) + cmds = [c.args[0] for c in mock_run.call_args_list] + self.assertTrue(any('down' in cmd and f'pic-conn-{inst}' in cmd + for cmd in cmds), cmds) + self.assertFalse(os.path.exists(inst_dir)) + + +class TestStoreServiceGate(_Base): + + def test_create_errors_when_store_service_not_installed(self): + # _Base marks all installed; remove proxy to hit the gate. + self.cm.remove_installed_service('proxy') + res = self.mgr.create_connection('proxy', 'p', _proxy_cfg()) + self.assertFalse(res['ok']) + self.assertIn('Service Store', res['error']) + self.assertEqual(len(self.cm.list_connections()), 0) + + +class TestTorSingleInstance(_ComposerBase): + + @patch('service_composer.subprocess.run') + def test_tor_uses_fixed_container_and_no_instance_up(self, mock_run): + mock_run.return_value = MagicMock(returncode=0, stdout='', stderr='') + res = self.mgr.create_connection('tor', 'Tor') + self.assertTrue(res['ok'], res) + rec = res['connection'] + # Single fixed container name, not cell-tor-. + self.assertEqual(self.mgr.instance_container_name(rec), 'cell-tor') + # No per-instance compose written for tor. + inst = rec['id'].split('_')[-1][:12] + self.assertFalse(self.composer.has_instance_compose('tor', inst)) + # A second tor is rejected (single instance). + res2 = self.mgr.create_connection('tor', 'Tor2') + self.assertFalse(res2['ok']) + + if __name__ == '__main__': unittest.main() diff --git a/tests/test_manifest_validator.py b/tests/test_manifest_validator.py index b725576..bd25902 100644 --- a/tests/test_manifest_validator.py +++ b/tests/test_manifest_validator.py @@ -260,6 +260,22 @@ class TestValidateManifest(unittest.TestCase): ok, errs = validate_manifest(m) self.assertTrue(ok) + def test_instanceable_container_name_with_placeholder_passes(self): + ok, errs = validate_manifest(_minimal_manifest( + instanceable=True, container_name='cell-sshuttle-${INSTANCE_ID}')) + self.assertTrue(ok, errs) + + def test_instanceable_container_name_without_placeholder_rejected(self): + ok, errs = validate_manifest(_minimal_manifest( + instanceable=True, container_name='cell-sshuttle')) + self.assertFalse(ok) + self.assertTrue(any('container_name' in e for e in errs)) + + def test_non_instanceable_with_placeholder_rejected(self): + ok, errs = validate_manifest(_minimal_manifest( + container_name='cell-sshuttle-${INSTANCE_ID}')) + self.assertFalse(ok) + # ── subdomain ──────────────────────────────────────────────────────── def test_subdomain_api_rejected(self): diff --git a/tests/test_service_composer.py b/tests/test_service_composer.py index e3ff5bd..a54dee1 100644 --- a/tests/test_service_composer.py +++ b/tests/test_service_composer.py @@ -676,5 +676,102 @@ class TestServiceComposerDeps(unittest.TestCase): composer.up.assert_not_called() +# ── Per-connection-instance lifecycle (P5) ─────────────────────────────────── + +_INSTANCEABLE_TEMPLATE = ( + 'services:\n' + ' sshuttle:\n' + ' image: git.pic.ngo/roof/svc-sshuttle:latest\n' + ' container_name: cell-sshuttle-${INSTANCE_ID}\n' + ' network_mode: host\n' + ' cap_add:\n' + ' - NET_ADMIN\n' + ' environment:\n' + ' - SSHUTTLE_LISTEN_PORT=${REDIRECT_PORT}\n' + ' volumes:\n' + ' - ${PIC_DATA_DIR}/services/sshuttle/${INSTANCE_ID}/config:/config\n' +) + + +def _instanceable_manifest(): + return { + 'id': 'sshuttle', + 'kind': 'store', + 'instanceable': True, + 'requires_host_network': True, + 'config_schema': {}, + } + + +class TestPerInstanceRender(unittest.TestCase): + + def test_substitutes_instance_id_redirect_port_and_data_dir(self): + with tempfile.TemporaryDirectory() as tmpdir: + composer = ServiceComposer(config_manager=_make_cm(), data_dir=tmpdir) + content = composer.render_template( + 'sshuttle', _instanceable_manifest(), _INSTANCEABLE_TEMPLATE, + instance_vars={'INSTANCE_ID': 'abcd1234', 'REDIRECT_PORT': '9101'}) + self.assertIn('cell-sshuttle-abcd1234', content) + self.assertIn('SSHUTTLE_LISTEN_PORT=9101', content) + self.assertIn( + os.path.join(tmpdir, 'services', 'sshuttle', 'abcd1234', 'config'), + content) + self.assertNotIn('${INSTANCE_ID}', content) + self.assertNotIn('${REDIRECT_PORT}', content) + + def test_two_instances_distinct_names_dirs_ports(self): + with tempfile.TemporaryDirectory() as tmpdir: + composer = ServiceComposer(config_manager=_make_cm(), data_dir=tmpdir) + manifest = _instanceable_manifest() + c1 = composer.write_instance_compose( + 'sshuttle', 'aaaa1111', manifest, _INSTANCEABLE_TEMPLATE, + redirect_port=9101) + c2 = composer.write_instance_compose( + 'sshuttle', 'bbbb2222', manifest, _INSTANCEABLE_TEMPLATE, + redirect_port=9102) + self.assertIn('cell-sshuttle-aaaa1111', c1) + self.assertIn('cell-sshuttle-bbbb2222', c2) + self.assertNotEqual( + composer._instance_compose_path('sshuttle', 'aaaa1111'), + composer._instance_compose_path('sshuttle', 'bbbb2222')) + self.assertIn('9101', c1) + self.assertIn('9102', c2) + self.assertTrue(os.path.exists( + composer._instance_compose_path('sshuttle', 'aaaa1111'))) + self.assertTrue(os.path.exists( + composer.instance_config_dir('sshuttle', 'bbbb2222'))) + + @patch('service_composer.subprocess.run') + def test_up_instance_uses_unique_project_and_compose(self, mock_run): + mock_run.return_value = MagicMock(returncode=0, stdout='', stderr='') + with tempfile.TemporaryDirectory() as tmpdir: + composer = ServiceComposer(config_manager=_make_cm(), data_dir=tmpdir) + res = composer.up_instance( + 'sshuttle', 'abcd1234', _instanceable_manifest(), + _INSTANCEABLE_TEMPLATE, redirect_port=9101) + self.assertTrue(res['ok']) + cmd = mock_run.call_args[0][0] + self.assertIn('--project-name', cmd) + self.assertIn('pic-conn-abcd1234', cmd) + self.assertIn('up', cmd) + + @patch('service_composer.subprocess.run') + def test_down_instance_removes_dir(self, mock_run): + mock_run.return_value = MagicMock(returncode=0, stdout='', stderr='') + with tempfile.TemporaryDirectory() as tmpdir: + composer = ServiceComposer(config_manager=_make_cm(), data_dir=tmpdir) + composer.write_instance_compose( + 'sshuttle', 'abcd1234', _instanceable_manifest(), + _INSTANCEABLE_TEMPLATE, redirect_port=9101) + inst_dir = composer._instance_dir('sshuttle', 'abcd1234') + self.assertTrue(os.path.isdir(inst_dir)) + res = composer.down_instance('sshuttle', 'abcd1234', purge_data=True) + self.assertTrue(res['ok']) + self.assertFalse(os.path.exists(inst_dir)) + + def test_instance_id_for_strips_prefix(self): + self.assertEqual(ServiceComposer.instance_id_for('conn_a1b2c3d4'), 'a1b2c3d4') + + if __name__ == '__main__': unittest.main()