Merge pull request 'hermes: restore legacy chat/triage hosts alongside the renamed ones' (#38) from fix/hermes-restore-legacy-chat-triage-hosts-v2 into main

Reviewed-on: atlas/titan-iac#38
Reviewed-by: bstein <bstein@noreply.scm.bstein.dev>

infrastructure/core/coredns-custom.yaml

@@ -19,6 +19,7 @@ data:
         192.168.22.9 cd.bstein.dev
         192.168.22.9 chat.ai.bstein.dev
         192.168.22.9 chat.bstein.dev
+        192.168.22.9 chat.hermes.bstein.dev
         192.168.22.9 ci.bstein.dev
         192.168.22.9 cloud.bstein.dev
         192.168.22.9 health.bstein.dev
@@ -46,6 +47,7 @@ data:
         192.168.22.9 wolf.bstein.dev
         192.168.22.9 tasks.bstein.dev
         192.168.22.9 triage.bstein.dev
+        192.168.22.9 triage.hermes.bstein.dev
         192.168.22.9 vault.bstein.dev
         fallthrough
       }

services/hermes/agent-certificate.yaml

@@ -13,3 +13,7 @@ spec:
     - agent.hermes.bstein.dev
     - chat.bstein.dev
     - triage.bstein.dev
+    # Legacy hosts stay on the certificate until they are retired on purpose;
+    # the rename in #34 must not break links or sessions already in flight.
+    - chat.hermes.bstein.dev
+    - triage.hermes.bstein.dev

services/hermes/agent-ingress.yaml

@@ -90,6 +90,8 @@ spec:
         - agent.hermes.bstein.dev
         - chat.bstein.dev
         - triage.bstein.dev
+        - chat.hermes.bstein.dev
+        - triage.hermes.bstein.dev
       secretName: hermes-sites-tls
   rules:
     - host: chat.bstein.dev
@@ -112,3 +114,26 @@ spec:
                 name: oauth2-proxy-hermes-triage
                 port:
                   name: http
+    # Legacy hosts serve the same backends so the rename is additive. They are
+    # kept until an explicit retirement change, not redirected: oauth2-proxy
+    # cookies are host-bound, so a redirect would silently drop the session.
+    - host: chat.hermes.bstein.dev
+      http:
+        paths:
+          - path: /
+            pathType: Prefix
+            backend:
+              service:
+                name: oauth2-proxy-hermes-chat
+                port:
+                  name: http
+    - host: triage.hermes.bstein.dev
+      http:
+        paths:
+          - path: /
+            pathType: Prefix
+            backend:
+              service:
+                name: oauth2-proxy-hermes-triage
+                port:
+                  name: http

services/keycloak/bootstrap-jobs/hermes-access-oidc-client-job.yaml

@@ -3,7 +3,7 @@
 apiVersion: batch/v1
 kind: Job
 metadata:
-  name: hermes-access-oidc-client-ensure-10
+  name: hermes-access-oidc-client-ensure-11
   namespace: sso
 spec:
   backoffLimit: 3

services/keycloak/scripts/hermes_access_oidc_ensure.sh

@@ -84,10 +84,24 @@ ensure_proxy_client() {
   client_id="$1"
   public_url="$2"
   vault_path="$3"
+  # Optional legacy host kept registered alongside the canonical one during a
+  # hostname rename. Keycloak matches redirect_uri exactly, so dropping the old
+  # entry turns every in-flight login into "Invalid parameter: redirect_uri".
+  legacy_url="${4:-}"
+  if [ -n "${legacy_url}" ]; then
+    origins="$(jq -nc --arg a "${public_url}" --arg b "${legacy_url}" '[$a,$b]')"
+  else
+    origins="$(jq -nc --arg a "${public_url}" '[$a]')"
+  fi
+  redirect_uris="$(printf '%s' "${origins}" | jq -c 'map(. + "/oauth2/callback")')"
+  # Keycloak takes post-logout origins as one "##"-delimited string.
+  post_logout="$(printf '%s' "${origins}" | jq -r 'join("##")')"
   payload="$(jq -nc \
     --arg client_id "${client_id}" \
-    --arg redirect_uri "${public_url}/oauth2/callback" \
+    --argjson redirect_uris "${redirect_uris}" \
+    --argjson web_origins "${origins}" \
     --arg web_origin "${public_url}" \
+    --arg post_logout "${post_logout}" \
     '{
       clientId:$client_id,
       name:$client_id,
@@ -98,13 +112,13 @@ ensure_proxy_client() {
       implicitFlowEnabled:false,
       directAccessGrantsEnabled:false,
       serviceAccountsEnabled:false,
-      redirectUris:[$redirect_uri],
-      webOrigins:[$web_origin],
+      redirectUris:$redirect_uris,
+      webOrigins:$web_origins,
       rootUrl:$web_origin,
       baseUrl:"/",
       attributes:{
         "pkce.code.challenge.method":"S256",
-        "post.logout.redirect.uris":$web_origin,
+        "post.logout.redirect.uris":$post_logout,
         "access.token.lifespan":"1200"
       }
     }')"
@@ -327,8 +341,10 @@ ensure_telegram_config() {
 }
 
 ensure_hermes_owner
-ensure_proxy_client "hermes-chat-proxy" "https://chat.bstein.dev" "hermes/chat-oidc"
+ensure_proxy_client "hermes-chat-proxy" "https://chat.bstein.dev" "hermes/chat-oidc" \
+  "https://chat.hermes.bstein.dev"
 ensure_proxy_client "hermes-agent-proxy" "https://agent.hermes.bstein.dev" "hermes/agent-oidc"
-ensure_proxy_client "hermes-triage-proxy" "https://triage.bstein.dev" "hermes/triage-oidc"
+ensure_proxy_client "hermes-triage-proxy" "https://triage.bstein.dev" "hermes/triage-oidc" \
+  "https://triage.hermes.bstein.dev"
 ensure_service_account_client "hermes-automation" "hermes/developer-keycloak"
 ensure_telegram_config

testing/tests/test_hermes_public_host_continuity.py

@@ -0,0 +1,153 @@
+"""Public chat/triage hostnames stay served on every layer during a rename.
+
+PR #34 renamed the chat and triage public hosts in place: the legacy names left
+the certificate SANs, the Ingress rules, the CoreDNS overrides and the Keycloak
+ensure script in one change. The legacy hosts began answering 404 with Traefik's
+default certificate while the renamed hosts could not finish a login, because
+Keycloak matches ``redirect_uri`` exactly and still held the old callback. Chat
+and triage were unreachable on every hostname at once.
+
+These tests pin the contract that makes that outage impossible to reintroduce
+silently: a public host is either served by all four layers or by none of them.
+"""
+
+from __future__ import annotations
+
+import re
+from pathlib import Path
+
+import pytest
+import yaml
+
+REPO = Path(__file__).parents[2]
+HERMES = REPO / "services/hermes"
+KEYCLOAK = REPO / "services/keycloak"
+COREDNS = REPO / "infrastructure/core/coredns-custom.yaml"
+ENSURE_SCRIPT = KEYCLOAK / "scripts/hermes_access_oidc_ensure.sh"
+
+# Every hostname the chat/triage surfaces must answer on, with the oauth2-proxy
+# backend that serves it. Both the renamed and the legacy names belong here
+# until a deliberate retirement change removes a row from this table.
+PUBLIC_HOSTS = {
+    "chat.bstein.dev": "oauth2-proxy-hermes-chat",
+    "chat.hermes.bstein.dev": "oauth2-proxy-hermes-chat",
+    "triage.bstein.dev": "oauth2-proxy-hermes-triage",
+    "triage.hermes.bstein.dev": "oauth2-proxy-hermes-triage",
+}
+
+# The agent surface was deliberately untouched by the rename.
+AGENT_HOST = "agent.hermes.bstein.dev"
+
+
+def _docs(path: Path) -> list[dict]:
+    return [doc for doc in yaml.safe_load_all(path.read_text()) if doc]
+
+
+def _named(path: Path, kind: str, name: str) -> dict:
+    for doc in _docs(path):
+        if doc.get("kind") == kind and doc["metadata"]["name"] == name:
+            return doc
+    raise AssertionError(f"{kind}/{name} missing from {path}")
+
+
+@pytest.fixture(scope="module")
+def certificate() -> dict:
+    return _named(HERMES / "agent-certificate.yaml", "Certificate", "hermes-sites-tls")
+
+
+@pytest.fixture(scope="module")
+def sites_ingress() -> dict:
+    return _named(HERMES / "agent-ingress.yaml", "Ingress", "hermes-sites")
+
+
+@pytest.fixture(scope="module")
+def coredns_hosts() -> set[str]:
+    block = yaml.safe_load(COREDNS.read_text())["data"]["bstein-dev.server"]
+    return {
+        line.split()[1]
+        for line in block.splitlines()
+        if len(line.split()) == 2 and re.fullmatch(r"[\d.]+", line.split()[0])
+    }
+
+
+@pytest.fixture(scope="module")
+def ensure_script() -> str:
+    return ENSURE_SCRIPT.read_text()
+
+
+@pytest.mark.parametrize("host", sorted(PUBLIC_HOSTS))
+def test_host_is_on_the_shared_certificate(host: str, certificate: dict):
+    """A host without a SAN serves Traefik's default self-signed certificate."""
+    assert host in certificate["spec"]["dnsNames"]
+
+
+@pytest.mark.parametrize("host", sorted(PUBLIC_HOSTS))
+def test_host_resolves_inside_the_cluster(host: str, coredns_hosts: set[str]):
+    assert host in coredns_hosts
+
+
+@pytest.mark.parametrize("host", sorted(PUBLIC_HOSTS))
+def test_host_has_an_ingress_rule_and_tls_entry(host: str, sites_ingress: dict):
+    """A host without a rule answers 404 even though DNS and TLS look healthy."""
+    spec = sites_ingress["spec"]
+    assert host in {name for entry in spec["tls"] for name in entry["hosts"]}
+
+    rule = next((item for item in spec["rules"] if item["host"] == host), None)
+    assert rule is not None, f"no hermes-sites rule serves {host}"
+
+    backends = {
+        path["backend"]["service"]["name"] for path in rule["http"]["paths"]
+    }
+    assert backends == {PUBLIC_HOSTS[host]}
+
+
+@pytest.mark.parametrize("host", sorted(PUBLIC_HOSTS))
+def test_host_is_registered_with_keycloak(host: str, ensure_script: str):
+    """Keycloak matches redirect_uri exactly, so every served host needs one."""
+    assert f"https://{host}" in ensure_script
+
+
+def test_agent_surface_was_not_touched_by_the_rename(
+    certificate: dict, coredns_hosts: set[str], ensure_script: str
+):
+    assert AGENT_HOST in certificate["spec"]["dnsNames"]
+    assert AGENT_HOST in coredns_hosts
+    assert f"https://{AGENT_HOST}" in ensure_script
+
+
+def test_ensure_script_registers_legacy_and_renamed_origins_together(
+    ensure_script: str,
+):
+    """The renamed proxies must carry both origins; the agent proxy only one."""
+    for client, canonical, legacy in (
+        ("hermes-chat-proxy", "chat.bstein.dev", "chat.hermes.bstein.dev"),
+        ("hermes-triage-proxy", "triage.bstein.dev", "triage.hermes.bstein.dev"),
+    ):
+        call = re.search(
+            rf'ensure_proxy_client "{client}".*?(?=\nensure_)',
+            ensure_script,
+            re.DOTALL,
+        )
+        assert call, f"{client} is never ensured"
+        assert f"https://{canonical}" in call.group(0)
+        assert f"https://{legacy}" in call.group(0)
+
+
+def test_ensure_job_is_rerun_whenever_the_script_changes():
+    """The Job is immutable, so a stale name silently skips the rerun."""
+    job = _named(
+        KEYCLOAK / "bootstrap-jobs/hermes-access-oidc-client-job.yaml",
+        "Job",
+        # The suffix moves with every rerun; resolve it from the manifest.
+        _job_name(),
+    )
+    assert job["spec"]["template"]["spec"]["containers"][0]["command"] == [
+        "/scripts/hermes_access_oidc_ensure.sh"
+    ]
+
+
+def _job_name() -> str:
+    path = KEYCLOAK / "bootstrap-jobs/hermes-access-oidc-client-job.yaml"
+    name = yaml.safe_load(path.read_text())["metadata"]["name"]
+    assert re.fullmatch(r"hermes-access-oidc-client-ensure-\d+", name), name
+    return name