fix(health): send Bearer auth on remote /health/detailed probe (#5418)
Clean rebase of nankingjing's #5424 (rebase-first). Co-authored-by: nankingjing <nankingjing@users.noreply.github.com>
This commit is contained in:
1 parent
d096b5f5d9
commit
4d7fad06e6
2 files changed
+83
-3
No files matched your search
+31
-3
@@ -348,7 +348,26 @@ def _remote_gateway_base_url() -> str | None:
|
|||||||
return None
|
return None
|
||||||
|
|
||||||
|
|
||||||
def _http_probe(url: str, timeout_s: float) -> tuple[bool, int | None, str | None, bytes | None]:
|
def _remote_gateway_api_key() -> str:
|
||||||
|
"""Return the Bearer token for authenticated gateway health probes.
|
||||||
|
|
||||||
|
Mirrors ``api.gateway_chat._gateway_api_key``: WebUI containers in
|
||||||
|
multi-service deployments must present the same key the agent's API server
|
||||||
|
expects on ``/health/detailed`` (#5418).
|
||||||
|
"""
|
||||||
|
return str(
|
||||||
|
os.environ.get("HERMES_WEBUI_GATEWAY_API_KEY")
|
||||||
|
or os.environ.get("API_SERVER_KEY")
|
||||||
|
or ""
|
||||||
|
).strip()
|
||||||
|
|
||||||
|
|
||||||
|
def _http_probe(
|
||||||
|
url: str,
|
||||||
|
timeout_s: float,
|
||||||
|
*,
|
||||||
|
api_key: str | None = None,
|
||||||
|
) -> tuple[bool, int | None, str | None, bytes | None]:
|
||||||
"""GET ``url`` and return (ok, status_code, error_name, body).
|
"""GET ``url`` and return (ok, status_code, error_name, body).
|
||||||
|
|
||||||
``ok`` is True only for a 2xx response. 5xx and network errors are not OK.
|
``ok`` is True only for a 2xx response. 5xx and network errors are not OK.
|
||||||
@@ -356,7 +375,10 @@ def _http_probe(url: str, timeout_s: float) -> tuple[bool, int | None, str | Non
|
|||||||
on this particular path) so the caller can move on to the next path.
|
on this particular path) so the caller can move on to the next path.
|
||||||
``body`` is the raw response bytes for 2xx responses, None otherwise.
|
``body`` is the raw response bytes for 2xx responses, None otherwise.
|
||||||
"""
|
"""
|
||||||
req = urllib_request.Request(url, method="GET")
|
headers: dict[str, str] = {}
|
||||||
|
if api_key:
|
||||||
|
headers["Authorization"] = f"Bearer {api_key}"
|
||||||
|
req = urllib_request.Request(url, method="GET", headers=headers)
|
||||||
try:
|
try:
|
||||||
with urllib_request.urlopen(req, timeout=timeout_s) as resp: # noqa: S310 - trusted env var URL
|
with urllib_request.urlopen(req, timeout=timeout_s) as resp: # noqa: S310 - trusted env var URL
|
||||||
status = getattr(resp, "status", None) or resp.getcode()
|
status = getattr(resp, "status", None) or resp.getcode()
|
||||||
@@ -392,8 +414,14 @@ def _probe_remote_gateway(base_url: str, *, now: float | None = None) -> dict[st
|
|||||||
|
|
||||||
last_status: int | None = None
|
last_status: int | None = None
|
||||||
last_error: str | None = None
|
last_error: str | None = None
|
||||||
|
gateway_api_key = _remote_gateway_api_key()
|
||||||
for path in _REMOTE_PROBE_PATHS:
|
for path in _REMOTE_PROBE_PATHS:
|
||||||
ok, status, err, body = _http_probe(base_url + path, _REMOTE_PROBE_TIMEOUT_S)
|
probe_key = gateway_api_key if path == "/health/detailed" else None
|
||||||
|
ok, status, err, body = _http_probe(
|
||||||
|
base_url + path,
|
||||||
|
_REMOTE_PROBE_TIMEOUT_S,
|
||||||
|
api_key=probe_key,
|
||||||
|
)
|
||||||
if ok:
|
if ok:
|
||||||
details: dict[str, Any] = {
|
details: dict[str, Any] = {
|
||||||
"state": "alive",
|
"state": "alive",
|
||||||
|
|||||||
@@ -141,6 +141,58 @@ def test_probe_order_prefers_health_detailed(monkeypatch):
|
|||||||
assert probed_urls[0] == "http://fake-gateway:8642/health/detailed"
|
assert probed_urls[0] == "http://fake-gateway:8642/health/detailed"
|
||||||
|
|
||||||
|
|
||||||
|
def test_health_detailed_probe_sends_bearer_when_api_key_configured(monkeypatch):
|
||||||
|
"""/health/detailed must include Bearer auth when a gateway API key is set (#5418)."""
|
||||||
|
monkeypatch.setenv("HERMES_API_URL", "http://fake-gateway:8642")
|
||||||
|
monkeypatch.setenv("HERMES_WEBUI_GATEWAY_API_KEY", "probe-secret")
|
||||||
|
captured_headers: list[dict[str, str]] = []
|
||||||
|
|
||||||
|
def fake_urlopen(req, timeout=None):
|
||||||
|
captured_headers.append(dict(req.header_items()))
|
||||||
|
return _FakeResp(200, body=json.dumps({"gateway_state": "running"}).encode())
|
||||||
|
|
||||||
|
with mock.patch.object(agent_health.urllib_request, "urlopen", fake_urlopen):
|
||||||
|
payload = agent_health.build_agent_health_payload()
|
||||||
|
|
||||||
|
assert payload["alive"] is True
|
||||||
|
assert payload["details"]["gateway_state"] == "running"
|
||||||
|
assert captured_headers
|
||||||
|
auth = captured_headers[0].get("Authorization")
|
||||||
|
assert auth == "Bearer probe-secret"
|
||||||
|
|
||||||
|
|
||||||
|
def test_health_detailed_falls_back_to_api_server_key(monkeypatch):
|
||||||
|
monkeypatch.setenv("HERMES_API_URL", "http://fake-gateway:8642")
|
||||||
|
monkeypatch.delenv("HERMES_WEBUI_GATEWAY_API_KEY", raising=False)
|
||||||
|
monkeypatch.setenv("API_SERVER_KEY", "shared-secret")
|
||||||
|
captured_headers: list[dict[str, str]] = []
|
||||||
|
|
||||||
|
def fake_urlopen(req, timeout=None):
|
||||||
|
captured_headers.append(dict(req.header_items()))
|
||||||
|
return _FakeResp(200, body=b'{"gateway_state": "running"}')
|
||||||
|
|
||||||
|
with mock.patch.object(agent_health.urllib_request, "urlopen", fake_urlopen):
|
||||||
|
agent_health.build_agent_health_payload()
|
||||||
|
|
||||||
|
assert captured_headers[0].get("Authorization") == "Bearer shared-secret"
|
||||||
|
|
||||||
|
|
||||||
|
def test_health_probe_does_not_send_bearer_without_api_key(monkeypatch):
|
||||||
|
monkeypatch.setenv("HERMES_API_URL", "http://fake-gateway:8642")
|
||||||
|
monkeypatch.delenv("HERMES_WEBUI_GATEWAY_API_KEY", raising=False)
|
||||||
|
monkeypatch.delenv("API_SERVER_KEY", raising=False)
|
||||||
|
captured_headers: list[dict[str, str]] = []
|
||||||
|
|
||||||
|
def fake_urlopen(req, timeout=None):
|
||||||
|
captured_headers.append(dict(req.header_items()))
|
||||||
|
return _FakeResp(200, body=b'{"gateway_state": "running"}')
|
||||||
|
|
||||||
|
with mock.patch.object(agent_health.urllib_request, "urlopen", fake_urlopen):
|
||||||
|
agent_health.build_agent_health_payload()
|
||||||
|
|
||||||
|
assert "Authorization" not in captured_headers[0]
|
||||||
|
|
||||||
|
|
||||||
def test_gateway_health_url_env_used(monkeypatch):
|
def test_gateway_health_url_env_used(monkeypatch):
|
||||||
"""GATEWAY_HEALTH_URL should be respected by the remote probe."""
|
"""GATEWAY_HEALTH_URL should be respected by the remote probe."""
|
||||||
monkeypatch.delenv("HERMES_API_URL", raising=False)
|
monkeypatch.delenv("HERMES_API_URL", raising=False)
|
||||||
|
|||||||
Reference in new issue
Block a user