From 26b1c6300913ceb82d025cb1802cf83d6fee3bbf Mon Sep 17 00:00:00 2001 From: Ben McCown Date: Wed, 15 Jul 2026 11:17:48 -0600 Subject: [PATCH 1/2] fix(platform-runner): seed NMP_BASE_URL host from config platform.base_url MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit apply_run_environment derived NMP_BASE_URL from the bind host/port (a loopback address in standalone mode) and set it via setdefault. Because get_base_url() prefers the NMP_BASE_URL env var over the config file, an explicit platform.base_url in the config was silently shadowed by the runner-derived loopback value — making the setting a no-op. This broke container-mode agent deployments: the gateway URL injected into a deployed agent's NAT config (llms.*.base_url) resolved to 127.0.0.1, which inside the agent container is the container itself, so the agent's outbound model calls failed. Seed NMP_BASE_URL from the *host* of an explicit platform.base_url in the config file (read from the raw YAML so a file-set value is distinguishable from the schema default), paired with the port the server actually binds. Only the host is honored — a config that hardcodes a port (e.g. :8080) must not point internal in-process clients (and the embedded PDP) at that port when the platform is launched on a different one (which the e2e harness always does, and any 'nemo services run --port' other than 8080 does); using the config port there leaves internal HTTP clients unable to reach the server, so the platform never becomes ready. An externally-provided NMP_BASE_URL (Helm/k8s) still wins via setdefault, preserving deployed-mode behavior. Signed-off-by: Ben McCown --- .../src/nmp/platform_runner/config.py | 56 +++++++++++++++++- .../nmp_platform_runner/tests/test_config.py | 59 ++++++++++++++++++- 2 files changed, 113 insertions(+), 2 deletions(-) diff --git a/packages/nmp_platform_runner/src/nmp/platform_runner/config.py b/packages/nmp_platform_runner/src/nmp/platform_runner/config.py index f7a99acd6cd..090a144a736 100644 --- a/packages/nmp_platform_runner/src/nmp/platform_runner/config.py +++ b/packages/nmp_platform_runner/src/nmp/platform_runner/config.py @@ -9,6 +9,7 @@ from collections.abc import Callable, MutableMapping from dataclasses import dataclass, field from importlib.resources import files +from urllib.parse import urlparse from nmp.common.config import ( NMP_CONFIG_FILE_PATH_ENV_VAR, @@ -153,6 +154,22 @@ def apply_run_environment( Uses ``setdefault`` for NMP_BASE_URL / NMP_SERVICE_HOST / NMP_SERVICE_PORT so that values pre-set by Helm / k8s (deployed mode) are never overwritten. In standalone mode these variables are absent, so setdefault fills them in. + + Precedence for NMP_BASE_URL: an externally-provided value (Helm / k8s) wins, + then an explicit ``platform.base_url`` *host* from the config file combined + with the actual bind port, then a value derived entirely from the bind + host/port. Seeding the host from the config file lets operators set a base + URL reachable from inside deployed agent containers (e.g. the Docker bridge + address) via config alone; without this the bind-derived loopback default + would silently shadow the configured value. + + Only the host (and scheme) of the configured ``platform.base_url`` is + honored — its port is replaced with the port the server actually binds. A + config that hardcodes ``:8080`` must not point internal clients (and the + embedded PDP) at 8080 when the platform is launched on another port (which + the e2e harness always does, and any ``nemo services run --port`` differing + from 8080 does); doing so leaves internal HTTP clients unable to reach the + server and the platform never becomes ready. """ if env is None: env = os.environ @@ -162,7 +179,12 @@ def apply_run_environment( effective_port = env.setdefault("NMP_SERVICE_PORT", str(config.port)) normalized = effective_host.strip("[]") url_host = f"[{normalized}]" if ":" in normalized else normalized - base_url = env.setdefault("NMP_BASE_URL", f"http://{url_host}:{effective_port}") + config_base_url_host = _config_file_base_url_host(config.config_path) + if config_base_url_host is not None: + default_base_url = f"http://{config_base_url_host}:{effective_port}" + else: + default_base_url = f"http://{url_host}:{effective_port}" + base_url = env.setdefault("NMP_BASE_URL", default_base_url) # Embedded PDP is served from the same platform process; keep the auth client # origin aligned with NMP_BASE_URL when services run on a non-default port. env.setdefault("NMP_AUTH_POLICY_DECISION_POINT_BASE_URL", base_url) @@ -179,6 +201,38 @@ def _set_or_clear_env(env: MutableMapping[str, str], name: str, values: set[str] env.pop(name, None) +def _config_file_base_url_host(config_path: str) -> str | None: + """Return the host of an explicit ``platform.base_url`` from the config file. + + Reads the raw YAML rather than the merged config object so a value present + in the file can be told apart from the schema default (the merged config + always carries ``base_url``). Returns the host component (already bracketed + for IPv6 so it can be dropped into an ``http://:`` URL), or + ``None`` when the file is missing, unreadable, does not set + ``platform.base_url``, or the value has no parseable host. + + Only the host is returned — callers pair it with the actual bind port, so a + config that hardcodes a port (e.g. ``:8080``) does not point internal + clients at the wrong port when the platform runs on a different one. + """ + try: + global_settings = Configuration.get_global_settings_from_file(config_path) + except (OSError, ValueError): + return None + platform_settings = global_settings.get("platform") + if not isinstance(platform_settings, dict): + return None + base_url = platform_settings.get("base_url") + if not isinstance(base_url, str) or not base_url: + return None + parsed = urlparse(base_url) + if not parsed.hostname: + return None + # urlparse lowercases and strips IPv6 brackets from hostname; re-bracket so + # the host can be safely composed back into an http://: URL. + return f"[{parsed.hostname}]" if ":" in parsed.hostname else parsed.hostname + + def _connect_host_for_internal_clients(host: str) -> str: """Translate a bind-address into a connectable address. diff --git a/packages/nmp_platform_runner/tests/test_config.py b/packages/nmp_platform_runner/tests/test_config.py index 5296480aa59..833abc285b9 100644 --- a/packages/nmp_platform_runner/tests/test_config.py +++ b/packages/nmp_platform_runner/tests/test_config.py @@ -24,7 +24,7 @@ def _make_config( sidecars: set[str] | None = None, host: str = "0.0.0.0", port: int = 8080, - config_path: str = "/tmp/test.yaml", + config_path: str = "/nonexistent/nmp-test-config.yaml", ) -> ResolvedRunConfiguration: return ResolvedRunConfiguration( services=services if services is not None else {"auth", "entities"}, @@ -210,3 +210,60 @@ def test_derives_base_url_from_effective_host_port(self): } apply_run_environment(_make_config(host="0.0.0.0", port=8080), env=env) assert env["NMP_BASE_URL"] == "http://nemo-platform-api:9090" + + +class TestApplyRunEnvConfigBaseUrl: + """Standalone mode with an explicit platform.base_url in the config file. + + The config value's *host* must seed NMP_BASE_URL (instead of the + bind-derived loopback default) so operators can set a container-reachable + base URL via config alone — but paired with the actual bind port, not the + port hardcoded in the config. An externally-provided NMP_BASE_URL still + wins. + """ + + def _write_config(self, tmp_path, body: str) -> str: + path = tmp_path / "config.yaml" + path.write_text(body, encoding="utf-8") + return str(path) + + def test_seeds_base_url_host_from_config_file(self, tmp_path): + config_path = self._write_config(tmp_path, "platform:\n base_url: http://172.17.0.1:8080\n") + env: dict[str, str] = {} + apply_run_environment(_make_config(host="0.0.0.0", port=8080, config_path=config_path), env=env) + assert env["NMP_BASE_URL"] == "http://172.17.0.1:8080" + # Embedded PDP base URL follows NMP_BASE_URL. + assert env["NMP_AUTH_POLICY_DECISION_POINT_BASE_URL"] == "http://172.17.0.1:8080" + + def test_uses_actual_bind_port_not_config_port(self, tmp_path): + # The config hardcodes :8080 but the platform is launched on 59007 + # (as the e2e harness does). NMP_BASE_URL must carry the real bind port + # so internal in-process clients can reach the server. + config_path = self._write_config(tmp_path, "platform:\n base_url: http://172.17.0.1:8080\n") + env: dict[str, str] = {} + apply_run_environment(_make_config(host="0.0.0.0", port=59007, config_path=config_path), env=env) + assert env["NMP_BASE_URL"] == "http://172.17.0.1:59007" + assert env["NMP_AUTH_POLICY_DECISION_POINT_BASE_URL"] == "http://172.17.0.1:59007" + + def test_config_base_url_without_port_gets_bind_port(self, tmp_path): + config_path = self._write_config(tmp_path, "platform:\n base_url: http://172.17.0.1\n") + env: dict[str, str] = {} + apply_run_environment(_make_config(host="0.0.0.0", port=9090, config_path=config_path), env=env) + assert env["NMP_BASE_URL"] == "http://172.17.0.1:9090" + + def test_external_env_still_wins_over_config_file(self, tmp_path): + config_path = self._write_config(tmp_path, "platform:\n base_url: http://172.17.0.1:8080\n") + env: dict[str, str] = {"NMP_BASE_URL": "http://nemo-platform-api:8080"} + apply_run_environment(_make_config(host="0.0.0.0", port=8080, config_path=config_path), env=env) + assert env["NMP_BASE_URL"] == "http://nemo-platform-api:8080" + + def test_falls_back_to_bind_derived_when_config_omits_base_url(self, tmp_path): + config_path = self._write_config(tmp_path, "platform:\n runtime: docker\n") + env: dict[str, str] = {} + apply_run_environment(_make_config(host="0.0.0.0", port=8080, config_path=config_path), env=env) + assert env["NMP_BASE_URL"] == "http://127.0.0.1:8080" + + def test_falls_back_to_bind_derived_when_config_file_missing(self): + env: dict[str, str] = {} + apply_run_environment(_make_config(host="0.0.0.0", port=8080, config_path="/nonexistent/nmp.yaml"), env=env) + assert env["NMP_BASE_URL"] == "http://127.0.0.1:8080" From ee811b115e4220f2e03bac95978d45a0d2e485cf Mon Sep 17 00:00:00 2001 From: Ben McCown Date: Wed, 15 Jul 2026 15:13:07 -0600 Subject: [PATCH 2/2] fix(platform-runner): preserve scheme, normalize host, guard malformed base_url Address PR review feedback on config-seeded NMP_BASE_URL: - Preserve the configured scheme: a config platform.base_url of https://... is no longer downgraded to http:// (parse and carry scheme + host). - Normalize the config-derived host through the same wildcard -> loopback translation as the bind host, so the bundled local.yaml default (http://0.0.0.0:8080) seeds a connectable http://127.0.0.1: instead of the non-connectable wildcard, keeping internal PDP/readiness clients working. - Guard urlparse against malformed bracketed IPv6 (e.g. http://[::1): fail soft to the bind-derived default instead of aborting startup. - Correct the docstring to match behavior (scheme and host are honored). Tests: add HTTPS scheme preservation, malformed-IPv6 fallback, wildcard normalization, and a bundled default_config_path() regression guard; annotate tmp_path fixtures as pathlib.Path. Signed-off-by: Ben McCown --- .../src/nmp/platform_runner/config.py | 71 ++++++++++++------- .../nmp_platform_runner/tests/test_config.py | 47 ++++++++++-- 2 files changed, 87 insertions(+), 31 deletions(-) diff --git a/packages/nmp_platform_runner/src/nmp/platform_runner/config.py b/packages/nmp_platform_runner/src/nmp/platform_runner/config.py index 090a144a736..6f7932252f1 100644 --- a/packages/nmp_platform_runner/src/nmp/platform_runner/config.py +++ b/packages/nmp_platform_runner/src/nmp/platform_runner/config.py @@ -156,20 +156,23 @@ def apply_run_environment( In standalone mode these variables are absent, so setdefault fills them in. Precedence for NMP_BASE_URL: an externally-provided value (Helm / k8s) wins, - then an explicit ``platform.base_url`` *host* from the config file combined - with the actual bind port, then a value derived entirely from the bind - host/port. Seeding the host from the config file lets operators set a base + then the scheme + host of an explicit ``platform.base_url`` from the config + file combined with the actual bind port, then a value derived entirely from + the bind host/port. Seeding from the config file lets operators set a base URL reachable from inside deployed agent containers (e.g. the Docker bridge address) via config alone; without this the bind-derived loopback default would silently shadow the configured value. - Only the host (and scheme) of the configured ``platform.base_url`` is + Only the scheme and host of the configured ``platform.base_url`` are honored — its port is replaced with the port the server actually binds. A config that hardcodes ``:8080`` must not point internal clients (and the embedded PDP) at 8080 when the platform is launched on another port (which the e2e harness always does, and any ``nemo services run --port`` differing from 8080 does); doing so leaves internal HTTP clients unable to reach the - server and the platform never becomes ready. + server and the platform never becomes ready. The configured host is run + through the same wildcard -> loopback normalization as the bind host, so a + config like ``http://0.0.0.0:8080`` (the bundled ``local.yaml`` default) + still yields a connectable internal base URL. """ if env is None: env = os.environ @@ -177,13 +180,14 @@ def apply_run_environment( connect_host = _connect_host_for_internal_clients(config.host) effective_host = env.setdefault("NMP_SERVICE_HOST", connect_host) effective_port = env.setdefault("NMP_SERVICE_PORT", str(config.port)) - normalized = effective_host.strip("[]") - url_host = f"[{normalized}]" if ":" in normalized else normalized - config_base_url_host = _config_file_base_url_host(config.config_path) - if config_base_url_host is not None: - default_base_url = f"http://{config_base_url_host}:{effective_port}" + config_base_url_parts = _config_file_base_url_parts(config.config_path) + if config_base_url_parts is not None: + scheme, config_host = config_base_url_parts + host_for_url = _bracket_ipv6(_connect_host_for_internal_clients(config_host)) + default_base_url = f"{scheme}://{host_for_url}:{effective_port}" else: - default_base_url = f"http://{url_host}:{effective_port}" + host_for_url = _bracket_ipv6(effective_host) + default_base_url = f"http://{host_for_url}:{effective_port}" base_url = env.setdefault("NMP_BASE_URL", default_base_url) # Embedded PDP is served from the same platform process; keep the auth client # origin aligned with NMP_BASE_URL when services run on a non-default port. @@ -201,19 +205,21 @@ def _set_or_clear_env(env: MutableMapping[str, str], name: str, values: set[str] env.pop(name, None) -def _config_file_base_url_host(config_path: str) -> str | None: - """Return the host of an explicit ``platform.base_url`` from the config file. +def _config_file_base_url_parts(config_path: str) -> tuple[str, str] | None: + """Return the (scheme, host) of an explicit ``platform.base_url`` from config. Reads the raw YAML rather than the merged config object so a value present in the file can be told apart from the schema default (the merged config - always carries ``base_url``). Returns the host component (already bracketed - for IPv6 so it can be dropped into an ``http://:`` URL), or - ``None`` when the file is missing, unreadable, does not set - ``platform.base_url``, or the value has no parseable host. - - Only the host is returned — callers pair it with the actual bind port, so a - config that hardcodes a port (e.g. ``:8080``) does not point internal - clients at the wrong port when the platform runs on a different one. + always carries ``base_url``). Returns the URL scheme (defaulting to + ``"http"`` when the config omits one) and the host component, or ``None`` + when the file is missing, unreadable, does not set ``platform.base_url``, or + the value has no parseable host. + + The host is returned unbracketed (as ``urlparse`` yields it) so callers can + normalize it (e.g. wildcard -> loopback) before composing the final URL. + Only the scheme and host are returned — callers pair them with the actual + bind port, so a config that hardcodes a port (e.g. ``:8080``) does not point + internal clients at the wrong port when the platform runs on a different one. """ try: global_settings = Configuration.get_global_settings_from_file(config_path) @@ -225,12 +231,16 @@ def _config_file_base_url_host(config_path: str) -> str | None: base_url = platform_settings.get("base_url") if not isinstance(base_url, str) or not base_url: return None - parsed = urlparse(base_url) + try: + parsed = urlparse(base_url) + except ValueError: + # Malformed value (e.g. an unterminated bracketed IPv6 like + # ``http://[::1``). Fall back to the bind-derived default rather than + # aborting startup — a bad config value should fail soft here. + return None if not parsed.hostname: return None - # urlparse lowercases and strips IPv6 brackets from hostname; re-bracket so - # the host can be safely composed back into an http://: URL. - return f"[{parsed.hostname}]" if ":" in parsed.hostname else parsed.hostname + return parsed.scheme or "http", parsed.hostname def _connect_host_for_internal_clients(host: str) -> str: @@ -246,3 +256,14 @@ def _connect_host_for_internal_clients(host: str) -> str: if stripped in _IPV6_WILDCARDS: return _IPV6_LOOPBACK return stripped + + +def _bracket_ipv6(host: str) -> str: + """Bracket an IPv6 literal so it can be composed into ``:``. + + Accepts an already-stripped host (no surrounding brackets) and wraps it in + ``[...]`` when it is an IPv6 literal (contains ``:``); returns other hosts + unchanged. + """ + stripped = host.strip("[]") + return f"[{stripped}]" if ":" in stripped else stripped diff --git a/packages/nmp_platform_runner/tests/test_config.py b/packages/nmp_platform_runner/tests/test_config.py index 833abc285b9..7853a23e1b3 100644 --- a/packages/nmp_platform_runner/tests/test_config.py +++ b/packages/nmp_platform_runner/tests/test_config.py @@ -1,6 +1,8 @@ # SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 +from pathlib import Path + import pytest from nmp.platform_runner import registry from nmp.platform_runner.config import ( @@ -222,12 +224,12 @@ class TestApplyRunEnvConfigBaseUrl: wins. """ - def _write_config(self, tmp_path, body: str) -> str: + def _write_config(self, tmp_path: Path, body: str) -> str: path = tmp_path / "config.yaml" path.write_text(body, encoding="utf-8") return str(path) - def test_seeds_base_url_host_from_config_file(self, tmp_path): + def test_seeds_base_url_host_from_config_file(self, tmp_path: Path): config_path = self._write_config(tmp_path, "platform:\n base_url: http://172.17.0.1:8080\n") env: dict[str, str] = {} apply_run_environment(_make_config(host="0.0.0.0", port=8080, config_path=config_path), env=env) @@ -235,7 +237,7 @@ def test_seeds_base_url_host_from_config_file(self, tmp_path): # Embedded PDP base URL follows NMP_BASE_URL. assert env["NMP_AUTH_POLICY_DECISION_POINT_BASE_URL"] == "http://172.17.0.1:8080" - def test_uses_actual_bind_port_not_config_port(self, tmp_path): + def test_uses_actual_bind_port_not_config_port(self, tmp_path: Path): # The config hardcodes :8080 but the platform is launched on 59007 # (as the e2e harness does). NMP_BASE_URL must carry the real bind port # so internal in-process clients can reach the server. @@ -245,19 +247,19 @@ def test_uses_actual_bind_port_not_config_port(self, tmp_path): assert env["NMP_BASE_URL"] == "http://172.17.0.1:59007" assert env["NMP_AUTH_POLICY_DECISION_POINT_BASE_URL"] == "http://172.17.0.1:59007" - def test_config_base_url_without_port_gets_bind_port(self, tmp_path): + def test_config_base_url_without_port_gets_bind_port(self, tmp_path: Path): config_path = self._write_config(tmp_path, "platform:\n base_url: http://172.17.0.1\n") env: dict[str, str] = {} apply_run_environment(_make_config(host="0.0.0.0", port=9090, config_path=config_path), env=env) assert env["NMP_BASE_URL"] == "http://172.17.0.1:9090" - def test_external_env_still_wins_over_config_file(self, tmp_path): + def test_external_env_still_wins_over_config_file(self, tmp_path: Path): config_path = self._write_config(tmp_path, "platform:\n base_url: http://172.17.0.1:8080\n") env: dict[str, str] = {"NMP_BASE_URL": "http://nemo-platform-api:8080"} apply_run_environment(_make_config(host="0.0.0.0", port=8080, config_path=config_path), env=env) assert env["NMP_BASE_URL"] == "http://nemo-platform-api:8080" - def test_falls_back_to_bind_derived_when_config_omits_base_url(self, tmp_path): + def test_falls_back_to_bind_derived_when_config_omits_base_url(self, tmp_path: Path): config_path = self._write_config(tmp_path, "platform:\n runtime: docker\n") env: dict[str, str] = {} apply_run_environment(_make_config(host="0.0.0.0", port=8080, config_path=config_path), env=env) @@ -267,3 +269,36 @@ def test_falls_back_to_bind_derived_when_config_file_missing(self): env: dict[str, str] = {} apply_run_environment(_make_config(host="0.0.0.0", port=8080, config_path="/nonexistent/nmp.yaml"), env=env) assert env["NMP_BASE_URL"] == "http://127.0.0.1:8080" + + def test_preserves_https_scheme_from_config(self, tmp_path: Path): + # A configured https:// base URL must not be downgraded to http://. + config_path = self._write_config(tmp_path, "platform:\n base_url: https://172.17.0.1:8080\n") + env: dict[str, str] = {} + apply_run_environment(_make_config(host="0.0.0.0", port=9090, config_path=config_path), env=env) + assert env["NMP_BASE_URL"] == "https://172.17.0.1:9090" + assert env["NMP_AUTH_POLICY_DECISION_POINT_BASE_URL"] == "https://172.17.0.1:9090" + + def test_malformed_ipv6_config_falls_back_to_bind_derived(self, tmp_path: Path): + # An unterminated bracketed IPv6 makes urlparse raise ValueError; that + # must fail soft to the bind-derived default rather than abort startup. + config_path = self._write_config(tmp_path, "platform:\n base_url: http://[::1\n") + env: dict[str, str] = {} + apply_run_environment(_make_config(host="0.0.0.0", port=8080, config_path=config_path), env=env) + assert env["NMP_BASE_URL"] == "http://127.0.0.1:8080" + + def test_wildcard_host_in_config_normalized_to_loopback(self, tmp_path: Path): + # The bundled local.yaml sets platform.base_url: http://0.0.0.0:8080. + # 0.0.0.0 is a bind-only wildcard, so the seeded internal base URL must + # be normalized to loopback or in-process clients (PDP, readiness) break. + config_path = self._write_config(tmp_path, "platform:\n base_url: http://0.0.0.0:8080\n") + env: dict[str, str] = {} + apply_run_environment(_make_config(host="0.0.0.0", port=8080, config_path=config_path), env=env) + assert env["NMP_BASE_URL"] == "http://127.0.0.1:8080" + + def test_bundled_default_config_seeds_loopback(self): + # Regression guard for the real default path: `nemo services run` with no + # --config uses default_config_path() (bundled local.yaml, which sets + # http://0.0.0.0:8080). The seeded NMP_BASE_URL must be connectable. + env: dict[str, str] = {} + apply_run_environment(_make_config(host="0.0.0.0", port=8080, config_path=default_config_path()), env=env) + assert env["NMP_BASE_URL"] == "http://127.0.0.1:8080"