Skip to content

Commit 3bbed59

Browse files
committed
feat(deployments): inject auth-proxy sidecar for docker; document auth fallback
Addresses PR feedback. - Docker backend now injects the auth-proxy sidecar too (build_docker_plan), reusing build_auth_proxy_container. The sidecar shares the primary container's network namespace, so the agent reaches it on localhost:8090 — the same loopback address used in k8s. Previously the sidecar was injected only in the k8s compiler, so auth-on docker agent deployments got no identity and would 401. - The docker sidecar's upstream (NMP_BASE_URL) is rewritten to a docker-reachable host via determine_loopback_override() (e.g. host.docker.internal on macOS) when the platform base URL is a loopback, mirroring the agent-side rewrite from #899. K8s uses the Service DNS verbatim. - Documented platform_auth_enabled()'s fail-to-False behavior: the realistic failure is ImportError (package used outside the platform image); other failures are effectively unreachable in the controller (missing config -> defaults; malformed config would have crashed startup; the read is cached). - Tests: docker auth-proxy injection, auth-off no-op, and loopback upstream rewrite. Signed-off-by: Ben McCown <bmccown@nvidia.com>
1 parent 5af3985 commit 3bbed59

4 files changed

Lines changed: 97 additions & 10 deletions

File tree

‎packages/nemo_platform_plugin/src/nemo_platform_plugin/auth.py‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,22 @@
1818

1919

2020
def platform_auth_enabled() -> bool:
21-
"""Return whether platform authentication is enabled."""
21+
"""Return whether platform authentication is enabled.
22+
23+
Returns ``False`` on any failure to resolve the auth config. The realistic
24+
failure is ``ImportError``: ``nmp_common`` ships only in the platform process
25+
image, so when this package is used standalone (outside the platform) there
26+
is no auth config and "disabled" is the correct answer.
27+
28+
Other failures are effectively unreachable in the context that matters here
29+
(the deployment controller, which runs *inside* the platform image): a
30+
missing config file resolves to defaults (``enabled=False``) rather than
31+
raising, and a malformed/invalid config file would have already crashed the
32+
platform service at startup before any deployment is reconciled. The config
33+
read is cached from that successful startup load. We therefore accept the
34+
narrow, largely theoretical fail-open window rather than propagate and block
35+
deployments on a transient/unexpected error.
36+
"""
2237
try:
2338
from nmp.common.config import get_auth_config
2439

‎plugins/nemo-deployments/src/nemo_deployments_plugin/auth_proxy.py‎

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
from __future__ import annotations
1616

1717
import logging
18+
from urllib.parse import urlsplit
1819

1920
from nemo_deployments_plugin.config import DeploymentsConfig
2021
from nemo_deployments_plugin.entities import (
@@ -26,7 +27,7 @@
2627
RestartPolicy,
2728
)
2829
from nemo_platform_plugin.auth import platform_auth_enabled
29-
from nemo_platform_plugin.config import get_nemo_config
30+
from nemo_platform_plugin.config import LOOPBACK_ADDRESSES, get_nemo_config
3031
from nemo_platform_plugin.jobs.image import get_qualified_image
3132

3233
logger = logging.getLogger(__name__)
@@ -44,18 +45,36 @@ def auth_proxy_port() -> int:
4445
return get_nemo_config(DeploymentsConfig).auth_proxy_port
4546

4647

47-
def _upstream_base_url() -> str:
48-
from nemo_platform_plugin.config import get_platform_config
48+
def _upstream_base_url(*, docker: bool) -> str:
49+
"""Return the platform base URL the sidecar forwards to, reachable from its container.
4950
50-
return get_platform_config().base_url.rstrip("/")
51-
52-
53-
def build_auth_proxy_container(config: DeploymentConfig) -> Container | None:
51+
In docker mode the platform base URL is often a host loopback the container
52+
cannot reach; substitute the docker-reachable host (e.g. host.docker.internal)
53+
the same way jobs do. In k8s the base URL is the in-cluster Service DNS and is
54+
used verbatim.
55+
"""
56+
from nemo_platform_plugin.config import determine_loopback_override, get_platform_config
57+
58+
base_url = get_platform_config().base_url.rstrip("/")
59+
if not docker:
60+
return base_url
61+
override = determine_loopback_override()
62+
if not override:
63+
return base_url
64+
parts = urlsplit(base_url)
65+
if (parts.hostname or "").lower() not in LOOPBACK_ADDRESSES:
66+
return base_url
67+
netloc = override if parts.port is None else f"{override}:{parts.port}"
68+
return parts._replace(netloc=netloc).geturl()
69+
70+
71+
def build_auth_proxy_container(config: DeploymentConfig, *, docker: bool = False) -> Container | None:
5472
"""Return the auth-proxy sidecar Container for *config*, or None.
5573
5674
Returns None when the config does not request the sidecar, or when platform
5775
auth is disabled (the sidecar would be pointless — internal calls are already
58-
trusted).
76+
trusted). Pass ``docker=True`` so the sidecar's upstream is rewritten to a
77+
docker-reachable host.
5978
"""
6079
if not config.auth_proxy_sidecar:
6180
return None
@@ -73,7 +92,7 @@ def build_auth_proxy_container(config: DeploymentConfig) -> Container | None:
7392
image=image,
7493
command=["nemo", "services", "run", "--sidecars", "auth-proxy"],
7594
env=[
76-
EnvVar(name="NMP_BASE_URL", value=_upstream_base_url()),
95+
EnvVar(name="NMP_BASE_URL", value=_upstream_base_url(docker=docker)),
7796
EnvVar(name=_AUTH_PROXY_PRINCIPAL_ENVVAR, value=identity),
7897
EnvVar(name=_AUTH_PROXY_HOST_ENVVAR, value="127.0.0.1"),
7998
EnvVar(name=_AUTH_PROXY_PORT_ENVVAR, value=str(port)),

‎plugins/nemo-deployments/src/nemo_deployments_plugin/backends/docker/containers.py‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
from dataclasses import dataclass, field
99
from typing import Any
1010

11+
from nemo_deployments_plugin.auth_proxy import build_auth_proxy_container
1112
from nemo_deployments_plugin.backends.labels import docker_volume_name
1213
from nemo_deployments_plugin.entities import Container, DeploymentConfig, DockerDeploymentConfig, VolumeMount
1314
from nemo_deployments_plugin.types import RestartPolicy
@@ -58,6 +59,13 @@ def build_docker_plan(config: DeploymentConfig) -> DockerDeploymentPlan:
5859
primary = config.containers[0]
5960
sidecars = list(config.containers[1:])
6061

62+
# Auth-proxy sidecar (no-op unless requested and platform auth is enabled).
63+
# It shares the primary's netns, so the primary reaches it on localhost —
64+
# the same loopback address the workload targets in k8s. Declares no ports.
65+
auth_proxy = build_auth_proxy_container(config, docker=True)
66+
if auth_proxy is not None:
67+
sidecars.append(auth_proxy)
68+
6169
# Sidecars share the primary's netns, so they cannot publish their own host
6270
# ports. (The primary owns the published ports for the whole group.)
6371
for sidecar in sidecars:

‎plugins/nemo-deployments/tests/unit/backends/docker/test_containers.py‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55

66
from __future__ import annotations
77

8+
from unittest.mock import patch
9+
810
import pytest
911
from backends.docker.docker_helpers import lora_config, sample_config
1012
from nemo_deployments_plugin.backends.docker.containers import (
@@ -14,6 +16,8 @@
1416
)
1517
from nemo_deployments_plugin.entities import Container, ContainerPort, DeploymentConfig
1618

19+
_AUTH_PROXY_MOD = "nemo_deployments_plugin.auth_proxy"
20+
1721

1822
def test_single_container_plan_has_no_init_or_sidecars() -> None:
1923
plan = build_docker_plan(sample_config())
@@ -23,6 +27,47 @@ def test_single_container_plan_has_no_init_or_sidecars() -> None:
2327
assert plan.is_multi_container is False
2428

2529

30+
def test_auth_proxy_injected_as_docker_sidecar_when_auth_on() -> None:
31+
config = sample_config()
32+
config = config.model_copy(update={"auth_proxy_sidecar": True, "auth_proxy_sidecar_identity": "agents"})
33+
with (
34+
patch(f"{_AUTH_PROXY_MOD}.platform_auth_enabled", return_value=True),
35+
patch(f"{_AUTH_PROXY_MOD}.get_qualified_image", return_value="my-registry/nmp-api:local"),
36+
patch(f"{_AUTH_PROXY_MOD}._upstream_base_url", return_value="http://host.docker.internal:8080"),
37+
):
38+
plan = build_docker_plan(config)
39+
# Injected as a sidecar (shares primary netns), not the primary.
40+
assert plan.primary.name == "main"
41+
proxy = next(c for c in plan.sidecars if c.name == "auth-proxy")
42+
env = {e.name: e.value for e in proxy.env}
43+
assert env["NMP_AUTH_PROXY_PRINCIPAL"] == "agents"
44+
assert env["NMP_BASE_URL"] == "http://host.docker.internal:8080"
45+
# Sidecar must not declare ports (shares netns).
46+
assert proxy.ports == []
47+
48+
49+
def test_auth_proxy_not_injected_when_auth_off() -> None:
50+
config = sample_config().model_copy(update={"auth_proxy_sidecar": True, "auth_proxy_sidecar_identity": "agents"})
51+
with patch(f"{_AUTH_PROXY_MOD}.platform_auth_enabled", return_value=False):
52+
plan = build_docker_plan(config)
53+
assert plan.sidecars == []
54+
55+
56+
def test_auth_proxy_docker_upstream_rewrites_loopback() -> None:
57+
# docker=True + loopback base_url -> host.docker.internal substitution.
58+
# _upstream_base_url imports these lazily from nemo_platform_plugin.config.
59+
from nemo_deployments_plugin.auth_proxy import _upstream_base_url
60+
61+
with (
62+
patch("nemo_platform_plugin.config.determine_loopback_override", return_value="host.docker.internal"),
63+
patch("nemo_platform_plugin.config.get_platform_config") as get_cfg,
64+
):
65+
get_cfg.return_value.base_url = "http://localhost:8080"
66+
assert _upstream_base_url(docker=True) == "http://host.docker.internal:8080"
67+
# k8s path leaves it verbatim.
68+
assert _upstream_base_url(docker=False) == "http://localhost:8080"
69+
70+
2671
def test_lora_plan_splits_init_primary_and_sidecar() -> None:
2772
plan = build_docker_plan(lora_config())
2873
assert plan.primary.name == "server"

0 commit comments

Comments
 (0)