Repository navigation
fix(multi-tenant): harden the unknown-host fix and pin it with tests - #397
Merged
Merged
Conversation
… by outcome Nothing executed handleRequest, so putting the throwing locale lookup back in entry.server left every test green. A test now hands it the redirect resolveDocumentLocale returns and expects that response back. The base-domain and look-alike-host tests asserted that a query was not made. They now let the slug lookup answer and assert what the caller gets, which fails on the old code without a spy. findCongregation was compared with a function that delegates to it; it asserts the mapped fields instead. A subdomain is resolved with UNITAE_BASE_URL written with and without its scheme, as the self-hosting guide writes it, and /livez is requested through the router so losing its route registration is caught.
…nguage resolveDocumentLocale answered a failed lookup with the default locale, and entry.server wrote whatever it got to the locale cookie: after one database error, a member of an English congregation read the app in French until their next full page load. A fallback now renders in the locale the visitor's cookie already holds, or the base one, and is not written back. Only a redirect is answered as one. Any other Response thrown by the lookup was handed to the visitor as the page; it is logged and falls back like the other failures. The log line carries the host and the path, so a failure can be tied to the tenant it happened on. The locale is validated where it is resolved, so entry.server no longer checks it again against a second default.
…r it is written The suffix a subdomain is matched against was UNITAE_BASE_URL with its scheme cut off. Written with a port or a trailing slash, it matched no host: every congregation subdomain was served as the platform root, without the check that a session belongs to the congregation in the URL, and with nothing in the logs. Set but left empty, it did the same. The hostname is now parsed out of the value, so a scheme, a port, a trailing slash or capitals all resolve, and an empty value reads as unset. validateEnv warns about a value that names no host. It does not refuse it: an instance that only serves custom domains runs without a base domain. A host with nothing before the base domain's dot no longer yields an empty slug.
…and fix the probe guide Five comments described the missing-congregation branch as a session outliving its congregation, with a 500 on every page. The foreign key forbids an account without its congregation, and once the accounts are gone the guard already redirected: the branch only covers a deletion racing a request. The comments say so now, and findCongregation's doc names its real callers. "As the loaders do" named no loader, and a test file repeated the JSDoc on what React Router does with a throw in entry.server. In the health-monitoring guide, the Compose example probed with curl, which the runtime image does not ship, so the container reported unhealthy forever. It uses Node's fetch, as docker/docker-compose.yml does. The guide also says a liveness restart takes several failed probes, and drops a readiness delay the startup probe already covers.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Follow-up to #393, from a review of it after the merge. Requests that already got the right answer keep it; this pins the fix with tests and closes the gaps the review found.
Tests (first commit, no production code)
handleRequest, so putting the throwing locale lookup back inentry.serverleft everything green. A test now hands it the redirectresolveDocumentLocalereturns and expects that response back.findCongregationwas compared with a function that delegates to it; it asserts the mapped fields.UNITAE_BASE_URLwritten with and without its scheme (the self-hosting guide writes it with one), and/livezis requested through the router in the E2E health spec.Locale lookup
entry.serverwrote that to the locale cookie: after one database error, a member of an English congregation read the app in French until their next full page load. A fallback now renders in the locale the visitor's cookie already holds, or the base one, and is not written back.Locationis answered as a redirect. Any otherResponsethrown by the lookup was handed to the visitor as the page; it is logged and falls back like the other failures.entry.serverno longer checks it against a second default. The failure log carries the host and the path.Base domain
UNITAE_BASE_URLwith its scheme cut off. With a port or a trailing slash it matched no host, and every congregation subdomain was served as the platform root, silently. The hostname is now parsed out of the value: scheme, port, trailing slash and capitals all resolve. An empty value reads as unset, andvalidateEnv()warns about one that names no host.UNITAE_BASE_URLcarries a port or a trailing slash starts resolving its subdomains. Before, none of them did.Comments and docs
docs/self-hosting/health-monitoring.md: the Compose example probed withcurl, which the runtime image does not ship, so the container reported unhealthy forever. It uses Node'sfetch, asdocker/docker-compose.ymldoes. A liveness restart takes several failed probes, not a brief outage, and the readiness delay the startup probe already covers is gone.Verified against a production build in multi-tenant mode, with
UNITAE_BASE_URL=https://unitae.test:8443/andHostheaders:unknown.<domain>/anything/congregation-not-foundunknown.<domain>/congregation-not-found<domain>/anything(base domain)not<domain>/anything(look-alike host)known.<domain>/anythingknown.<domain>/login/livezok,Cache-Control: no-storeWith
UNITAE_BASE_URL="", subdomains resolve against the default domain again. New behaviour was written test-first; unit tests for the touched files, lint and typecheck pass.Type de changement
Checklist
pnpm build:format)Issue liée
None.