From 8d1d45d246248af0b2d20fbf41490a340a4522ca Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Tue, 1 Sep 2026 12:00:56 +0200 Subject: [PATCH] fix(discourse): pin the OIDC issuer for the launcher-built container Nextcloud#7 of run 33445831564 fails on three discourse specs, all on one assertion, all for one reason. The received body is Discourse's own page: "Sorry, there was an error while trying to authorize your account with oidc." The browser leg works. What dies is Discourse's server-side back-channel: "GET /auth/oidc/callback?...code=..." 302 2.125 2.125 "GET /auth/failure?message=openid_connect_request_failed&...&strategy=oidc" 200 Four of those, at 2.125 / 2.015 / 2.063 / 2.014 seconds. That is not the plugin's 10s HTTP timeout; it is Discourse's DNS budget, hard-coded at 2s in lib/final_destination/resolver.rb (connect_timeout arrives nil, so the default applies). openid_connect_request_failed has exactly one emitter -- omniauth_open_id_connect.rb:147-155, rescue Faraday::Error -- and the alternative, SocketError -> LookupFailedError, is not a Faraday::Error and would have produced a 500 rather than the observed 302. The correlation leaves no room: across 18 back-channel requests, all from 192.168.9.3 with UA Faraday v2.14.3, there are exactly two complete token 200 + userinfo 200 pairs -- and exactly two specs passed. Thirteen truncated flows, thirteen failures. No test that got a complete exchange failed; none that failed got one. Why discourse alone: discourse.inspect.json:72 reads "ExtraHosts": null. Grepping the whole rescue bundle for the pin finds nextcloud, nextcloud-proxy, mailu-admin, prometheus and prometheus-sso-proxy. Discourse could not have it: config.yml.j2's docker_args: carried only --network= and --name=. It is built by discourse_docker's launcher, not by a compose file, so the lookup's rendered block has nowhere to go -- hence entries_only, which returns the strings and lets the template write --add-host= lines. The pin itself is NOT re-derived: _sso_pins() and _gateway_address() stay the single place that knows compose from swarm. Its existing allowed_internal_hosts cannot help, because ssrf_detector's lookup_and_filter_ips resolves BEFORE host_bypasses_checks?; the setting only makes a private answer acceptable once it arrives. Rendered through a real Templar both ways: with SSO on, the two --add-host lines land between the networks and --name; with SSO off the output is byte-identical to today, so clearnet and variant-1 deployments render an unchanged app.yml and trigger no launcher rebuild. 24 lookup tests green. Not closed: the biber persona has never passed, here or in run 33317483326. Its failure text and code path match administrator, which did pass, so it is explained -- but a persona-specific second defect is not positively excluded. Deploying discourse variant 0 locally, which already has sso and tor enabled, settles it. Separately worth queueing: discourse's own meta/variants.yml variant 1 sets sso.enabled: false and only variant 1 is scheduled, so the role that owns these specs never runs them. Only partner rounds like Nextcloud#7 catch this class. Co-Authored-By: Claude Opus 5 (1M context) --- TODO.md | 3 +- plugins/lookup/container_extra_hosts.py | 15 ++++++++++ .../web-app-discourse/templates/config.yml.j2 | 3 ++ .../lookup/test_container_extra_hosts.py | 28 +++++++++++++++++++ 4 files changed, 48 insertions(+), 1 deletion(-) diff --git a/TODO.md b/TODO.md index 9f401239e6..48cc42d59f 100644 --- a/TODO.md +++ b/TODO.md @@ -83,7 +83,8 @@ against those runs say so inline. `container_extra_hosts` lookup yet (fix is the same one-line lookup, apply on CI evidence): wordpress (daggerhart plugin) and mediawiki (PluggableAuth) are structurally identical PHP in-app plugins (high - likelihood); discourse, jenkins, jira, confluence, odoo, openwebui, + likelihood); discourse is now confirmed and pinned (run 33445831564, job + Nextcloud#7); jenkins, jira, confluence, odoo, openwebui, pretix, zammad, semaphore, jellyfin, listmonk, xwiki, mastodon consume the issuer server-side too (unverified network path). oauth2-proxy-based apps are NOT affected (proven green: prometheus admin SSO). Two names have left diff --git a/plugins/lookup/container_extra_hosts.py b/plugins/lookup/container_extra_hosts.py index fd96e823ef..4ba17a12fd 100644 --- a/plugins/lookup/container_extra_hosts.py +++ b/plugins/lookup/container_extra_hosts.py @@ -24,6 +24,12 @@ through here, from two sources that are both optional: Entries are ``"name:address"`` strings and are deduplicated in first-seen order. With nothing to emit the lookup returns an empty string. +``entries_only=true`` returns those strings as a list instead of the rendered +block, for a container that is not described by a compose file at all: +``web-app-discourse`` is built by discourse_docker's ``launcher``, whose only +argument surface is ``docker_args:`` in its ``app.yml``, so it needs +``--add-host=`` lines. + The automatic pin follows the effective deploy mode, and in swarm it must NOT be ``host-gateway``: that alias resolves per node, while openresty publishes :80 with ``mode: host`` under ``node.role == manager``, so every replica on a worker @@ -48,6 +54,13 @@ Usage in any service template: {{ lookup('container_extra_hosts', extra_hosts=['host.docker.internal:host-gateway']) | indent(4) }} {{ lookup('container_extra_hosts', application_id='web-app-nextcloud') | indent(4) }} + +Usage from a non-compose call site: + + {% for entry in lookup('container_extra_hosts', + entries_only=True, wantlist=True) %} + - --add-host={{ entry }} + {% endfor %} """ from __future__ import annotations @@ -98,6 +111,8 @@ class LookupModule(LookupBase): ) entries += self._caller_entries(kwargs.get("extra_hosts")) merged = list(dict.fromkeys(entries)) + if _to_bool(self._render(kwargs.get("entries_only", False)), strict=False): + return merged if not merged: return [""] diff --git a/roles/web-app-discourse/templates/config.yml.j2 b/roles/web-app-discourse/templates/config.yml.j2 index f09f995673..c493f5bae1 100644 --- a/roles/web-app-discourse/templates/config.yml.j2 +++ b/roles/web-app-discourse/templates/config.yml.j2 @@ -234,4 +234,7 @@ docker_args: {% if DISCOURSE_LITELLM_ENABLED | bool %} - --network={{ DISCOURSE_LITELLM_NETWORK }} {% endif %} +{% for extra_host in lookup('container_extra_hosts', entries_only=True, wantlist=True) %} + - --add-host={{ extra_host }} +{% endfor %} - --name={{ DISCOURSE_CONTAINER }} diff --git a/tests/unit/python/plugins/lookup/test_container_extra_hosts.py b/tests/unit/python/plugins/lookup/test_container_extra_hosts.py index f914fa7c72..955349fc08 100644 --- a/tests/unit/python/plugins/lookup/test_container_extra_hosts.py +++ b/tests/unit/python/plugins/lookup/test_container_extra_hosts.py @@ -176,6 +176,34 @@ class TestContainerExtraHostsLookup(unittest.TestCase): def test_no_sources_at_all_emit_nothing(self): self.assertEqual(self._run(_sso_enabled=False, extra_hosts=[]), "") + def _run_entries(self, **overrides): + variables = { + "application_id": "web-app-discourse", + "_sso_enabled": True, + "_provider_domain": ONION, + "DEPLOYMENT_MODE": "compose", + } + variables.update(overrides) + return self.lookup.run(None, variables=variables, entries_only=True) + + def test_entries_only_returns_the_raw_mappings(self): + self.assertEqual( + self._run_entries(), + ["host.docker.internal:host-gateway", f"{ONION}:host-gateway"], + ) + + def test_entries_only_follows_the_swarm_address(self): + self.assertEqual( + self._run_entries( + DEPLOYMENT_MODE="swarm", + ansible_facts={"default_ipv4": {"address": "10.0.0.7"}}, + ), + ["host.docker.internal:host-gateway", f"{ONION}:10.0.0.7"], + ) + + def test_entries_only_is_empty_without_sso(self): + self.assertEqual(self._run_entries(_sso_enabled=False), []) + if __name__ == "__main__": unittest.main()