diff --git a/docs/architecture.md b/docs/architecture.md index 93503fd..dad3df9 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -68,11 +68,14 @@ the core settings knowing any provider exists. Not every provider supports every feature, and the host contract must not grow provider-shaped warts. Optional features are capability -mix-ins: `HttpProxyCapability` declares the http-proxy surface, the -exe provider implements it, and the http-proxies routes refuse with a -clear error when the active provider doesn't. New provider-specific -features should follow this pattern rather than widening `VMProvider` -or the host schema. +mix-ins: `HttpProxyCapability` declares the http-proxy surface and the +exe provider implements it. `resolve_capability` narrows a specific +provider instance to a capability — the default provider for +account-bound operations, the host's own provider for host-bound ones +— and raises the shared `CapabilityUnsupportedError` when that +provider doesn't implement it, which the routes surface as a clear +error. New provider-specific features should follow this pattern +rather than widening `VMProvider` or the host schema. The review question that guards the whole design: *does this change leak a provider into the contract?* diff --git a/src/http_proxies/deps.py b/src/http_proxies/deps.py index e188fa2..95f7ad7 100644 --- a/src/http_proxies/deps.py +++ b/src/http_proxies/deps.py @@ -4,18 +4,10 @@ from sqlalchemy.ext.asyncio import AsyncSession from core.database import get_session -from http_proxies.exceptions import HTTPProxyUnsupportedError from http_proxies.service import HTTPProxyService -from providers.exceptions import UnknownProviderError async def get_http_proxy_service( session: Annotated[AsyncSession, Depends(get_session)], ) -> HTTPProxyService: - # The active provider may not speak the http-proxy capability (only exe - # does today). Translate that into a clean 501 instead of letting the - # raw UnknownProviderError surface as a 500. - try: - return HTTPProxyService(session) - except UnknownProviderError as exc: - raise HTTPProxyUnsupportedError(str(exc)) from exc + return HTTPProxyService(session) diff --git a/src/http_proxies/service.py b/src/http_proxies/service.py index 000f3f4..34645c4 100644 --- a/src/http_proxies/service.py +++ b/src/http_proxies/service.py @@ -7,14 +7,22 @@ from hosts.exceptions import HostStateError from hosts.models import HostStatus from hosts.service import HostService -from http_proxies.exceptions import HTTPProxyError, HTTPProxyExistsError, HTTPProxyNotFoundError -from providers.capabilities import HttpProxyCapability, get_default_http_proxy_capability +from http_proxies.exceptions import ( + HTTPProxyError, + HTTPProxyExistsError, + HTTPProxyNotFoundError, + HTTPProxyUnsupportedError, +) +from providers.capabilities import HttpProxyCapability, resolve_capability from providers.exceptions import ( + CapabilityUnsupportedError, ProviderError, ProviderHttpProxyExistsError, ProviderHttpProxyNotFoundError, ProviderTargetVMNotFoundError, + UnknownProviderError, ) +from providers.registry import get_vm_provider ATTACHABLE_HOST_STATUSES = frozenset( { @@ -29,13 +37,19 @@ def __init__( self, session: AsyncSession, settings: Settings | None = None, - *, - http_proxy: HttpProxyCapability | None = None, ) -> None: self.session = session self.settings = settings or get_settings() self.host_service = HostService(session, settings=self.settings) - self.http_proxy = http_proxy or get_default_http_proxy_capability() + + def _http_proxy_capability(self, provider_name: str | None = None) -> HttpProxyCapability: + # Account-bound operations (create/delete) resolve the default + # provider; host-bound operations (attach/detach) resolve the provider + # that owns the host's VM. + try: + return resolve_capability(get_vm_provider(provider_name), HttpProxyCapability) + except (UnknownProviderError, CapabilityUnsupportedError) as exc: + raise HTTPProxyUnsupportedError(str(exc)) from exc async def create_http_proxy( self, @@ -44,16 +58,20 @@ async def create_http_proxy( target: str, headers: dict[str, str], ) -> None: + http_proxy = self._http_proxy_capability() + try: - await self.http_proxy.create_http_proxy(name=name, target=target, headers=headers) + await http_proxy.create_http_proxy(name=name, target=target, headers=headers) except ProviderHttpProxyExistsError as exc: raise HTTPProxyExistsError("http proxy already exists") from exc except ProviderError as exc: raise HTTPProxyError("http proxy could not be created") from exc async def delete_http_proxy(self, name: str) -> None: + http_proxy = self._http_proxy_capability() + try: - await self.http_proxy.delete_http_proxy(name) + await http_proxy.delete_http_proxy(name) except ProviderHttpProxyNotFoundError as exc: raise HTTPProxyNotFoundError("http proxy not found") from exc except ProviderError as exc: @@ -68,8 +86,10 @@ async def attach_http_proxy(self, name: str, host_id: uuid.UUID) -> None: if host.status not in ATTACHABLE_HOST_STATUSES: raise HostStateError("host does not have a backing VM") + http_proxy = self._http_proxy_capability(host.provider) + try: - await self.http_proxy.attach_http_proxy(name, attach_vm=host.name) + await http_proxy.attach_http_proxy(name, attach_vm=host.name) except ProviderTargetVMNotFoundError as exc: raise HostStateError("host does not have a backing VM") from exc except ProviderHttpProxyNotFoundError as exc: @@ -86,8 +106,10 @@ async def detach_http_proxy(self, name: str, host_id: uuid.UUID) -> None: if host.status not in ATTACHABLE_HOST_STATUSES: raise HostStateError("host does not have a backing VM") + http_proxy = self._http_proxy_capability(host.provider) + try: - await self.http_proxy.detach_http_proxy(name, attach_vm=host.name) + await http_proxy.detach_http_proxy(name, attach_vm=host.name) except ProviderTargetVMNotFoundError as exc: raise HostStateError("host does not have a backing VM") from exc except ProviderHttpProxyNotFoundError as exc: diff --git a/src/http_proxies/tests/test_api.py b/src/http_proxies/tests/test_api.py index 3b17094..99d4f37 100644 --- a/src/http_proxies/tests/test_api.py +++ b/src/http_proxies/tests/test_api.py @@ -4,6 +4,7 @@ from uuid6 import uuid7 from core.database import async_session_factory +from core.settings import get_settings from hosts.models import Host, HostStatus from hosts.service import utc_now from providers.exe.settings import ExeSettings @@ -32,13 +33,8 @@ async def test_create_http_proxy_returns_created(client, monkeypatch): ) -async def test_http_proxy_returns_501_when_provider_lacks_capability(client, monkeypatch): - from providers.exceptions import UnknownProviderError - - def _unsupported(): - raise UnknownProviderError("default VM provider 'docker' does not support http proxies") - - monkeypatch.setattr("http_proxies.service.get_default_http_proxy_capability", _unsupported) +async def test_http_proxy_returns_501_when_default_provider_lacks_capability(client, monkeypatch): + monkeypatch.setattr(get_settings(), "default_host_provider", "docker") response = await client.post( "/http-proxies", @@ -54,6 +50,22 @@ def _unsupported(): assert response.json()["error_code"] == "HTTP_PROXY_UNSUPPORTED" +async def test_attach_returns_501_when_host_provider_lacks_capability(client): + host = await create_host_record( + name="lb-sandbox-test", + status=HostStatus.ACTIVE.value, + provider="docker", + ) + + response = await client.post( + f"/http-proxies/gmail-mcp/hosts/{host.id}", + headers={"Authorization": "Bearer service-token"}, + ) + + assert response.status_code == 501 + assert response.json()["error_code"] == "HTTP_PROXY_UNSUPPORTED" + + async def test_http_proxy_rejects_option_like_names(client, monkeypatch): # Leading-hyphen names could be parsed as exe.dev CLI options, so they must be # rejected (422) at every boundary before reaching the provider. @@ -242,6 +254,7 @@ async def create_host_record( id: uuid.UUID | None = None, name: str, status: str, + provider: str = "exe", tailscale_device_id: str | None = None, ) -> Host: now = utc_now() @@ -249,7 +262,7 @@ async def create_host_record( id=id or uuid7(), name=name, status=status, - provider="exe", + provider=provider, image=ExeSettings().default_image, # pyright: ignore[reportCallIssue] env={}, internal_ssh_host=f"{name}.example.ts.net", diff --git a/src/http_proxies/tests/test_service.py b/src/http_proxies/tests/test_service.py index 591f38b..f971972 100644 --- a/src/http_proxies/tests/test_service.py +++ b/src/http_proxies/tests/test_service.py @@ -5,10 +5,15 @@ from uuid6 import uuid7 from core.database import async_session_factory +from core.settings import get_settings from hosts.exceptions import HostStateError from hosts.models import Host, HostStatus from hosts.service import HostService, utc_now -from http_proxies.exceptions import HTTPProxyExistsError, HTTPProxyNotFoundError +from http_proxies.exceptions import ( + HTTPProxyExistsError, + HTTPProxyNotFoundError, + HTTPProxyUnsupportedError, +) from http_proxies.service import HTTPProxyService from providers.exceptions import ( ProviderHttpProxyExistsError, @@ -54,6 +59,21 @@ async def test_create_http_proxy_maps_already_exists(monkeypatch): ) +async def test_create_http_proxy_unsupported_on_default_provider(monkeypatch): + """Create refuses with the unsupported error when the default provider lacks the capability.""" + monkeypatch.setattr(get_settings(), "default_host_provider", "docker") + + async with async_session_factory() as session: + service = HTTPProxyService(session) + + with pytest.raises(HTTPProxyUnsupportedError, match="docker"): + await service.create_http_proxy( + name="gmail-mcp", + target="https://gmailmcp.googleapis.com", + headers={"Authorization": "Bearer token"}, + ) + + async def test_attach_http_proxy_uses_host_vm_name(monkeypatch): mocked_attach = AsyncMock() monkeypatch.setattr("providers.exe.provider.ExeProvider.attach_http_proxy", mocked_attach) @@ -78,6 +98,39 @@ async def test_detach_http_proxy_uses_host_vm_name(monkeypatch): mocked_detach.assert_awaited_once_with("gmail-mcp", attach_vm="lb-sandbox-test") +async def test_attach_http_proxy_resolves_hosts_provider_not_default(monkeypatch): + """Attach refuses when the host's provider lacks the capability, even if the default has it.""" + mocked_attach = AsyncMock() + monkeypatch.setattr("providers.exe.provider.ExeProvider.attach_http_proxy", mocked_attach) + host = await create_host_record( + name="lb-sandbox-test", + status=HostStatus.ACTIVE.value, + provider="docker", + ) + + async with async_session_factory() as session: + service = HTTPProxyService(session) + + with pytest.raises(HTTPProxyUnsupportedError, match="docker"): + await service.attach_http_proxy("gmail-mcp", host.id) + + mocked_attach.assert_not_awaited() + + +async def test_attach_http_proxy_on_non_default_provider_with_capability(monkeypatch): + """Attach reaches the host's provider when only that non-default provider has the capability.""" + monkeypatch.setattr(get_settings(), "default_host_provider", "docker") + mocked_attach = AsyncMock() + monkeypatch.setattr("providers.exe.provider.ExeProvider.attach_http_proxy", mocked_attach) + host = await create_host_record(name="lb-sandbox-test", status=HostStatus.ACTIVE.value) + + async with async_session_factory() as session: + service = HTTPProxyService(session) + await service.attach_http_proxy("gmail-mcp", host.id) + + mocked_attach.assert_awaited_once_with("gmail-mcp", attach_vm="lb-sandbox-test") + + async def test_detach_http_proxy_maps_not_found(monkeypatch): monkeypatch.setattr( "providers.exe.provider.ExeProvider.detach_http_proxy", @@ -175,6 +228,7 @@ async def create_host_record( id: uuid.UUID | None = None, name: str, status: str, + provider: str = "exe", tailscale_device_id: str | None = None, ) -> Host: now = utc_now() @@ -182,7 +236,7 @@ async def create_host_record( id=id or uuid7(), name=name, status=status, - provider="exe", + provider=provider, image=ExeSettings().default_image, # pyright: ignore[reportCallIssue] env={}, internal_ssh_host=f"{name}.example.ts.net", diff --git a/src/providers/capabilities.py b/src/providers/capabilities.py index 0ec9657..9ab4554 100644 --- a/src/providers/capabilities.py +++ b/src/providers/capabilities.py @@ -1,8 +1,18 @@ import abc +from typing import TypeVar from providers.base import VMProvider -from providers.exceptions import UnknownProviderError -from providers.registry import get_default_vm_provider +from providers.exceptions import CapabilityUnsupportedError + +CapabilityT = TypeVar("CapabilityT") + + +def resolve_capability(provider: VMProvider, capability: type[CapabilityT]) -> CapabilityT: + if not isinstance(provider, capability): + raise CapabilityUnsupportedError( + f"VM provider '{provider.name}' does not support {capability.__name__}", + ) + return provider class HttpProxyCapability(abc.ABC): @@ -31,13 +41,3 @@ async def attach_http_proxy(self, name: str, *, attach_vm: str) -> None: ... @abc.abstractmethod async def detach_http_proxy(self, name: str, *, attach_vm: str) -> None: ... - - -def get_default_http_proxy_capability() -> HttpProxyCapability: - provider: VMProvider = get_default_vm_provider() - - if not isinstance(provider, HttpProxyCapability): - raise UnknownProviderError( - f"default VM provider '{provider.name}' does not support http proxies", - ) - return provider diff --git a/src/providers/exceptions.py b/src/providers/exceptions.py index 20718f2..8074404 100644 --- a/src/providers/exceptions.py +++ b/src/providers/exceptions.py @@ -32,3 +32,7 @@ class ProviderHttpProxyNotFoundError(ProviderNotFoundError): class UnknownProviderError(ProviderError): pass + + +class CapabilityUnsupportedError(ProviderError): + pass diff --git a/src/providers/tests/test_capabilities.py b/src/providers/tests/test_capabilities.py new file mode 100644 index 0000000..145ed29 --- /dev/null +++ b/src/providers/tests/test_capabilities.py @@ -0,0 +1,17 @@ +import pytest + +from providers.capabilities import HttpProxyCapability, resolve_capability +from providers.exceptions import CapabilityUnsupportedError +from providers.registry import get_vm_provider + + +def test_resolve_capability_returns_implementing_provider() -> None: + """A provider that inherits the capability mix-in resolves to itself.""" + provider = get_vm_provider("exe") + assert resolve_capability(provider, HttpProxyCapability) is provider + + +def test_resolve_capability_refuses_provider_without_capability() -> None: + """A provider without the mix-in raises the shared unsupported error.""" + with pytest.raises(CapabilityUnsupportedError, match="'docker' does not support"): + resolve_capability(get_vm_provider("docker"), HttpProxyCapability)