From 3fe18bcedbb2858a60b696cd30855a9acd5727cb Mon Sep 17 00:00:00 2001 From: Manuel Raynaud Date: Thu, 13 Aug 2026 16:18:54 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8(yhub)=20add=20custom=20probes=20more?= =?UTF-8?q?=20efficient?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The probes used before was using the only one public available endpoint. This endpoint is the jwks endpoint but it is not an appropriated one. For the readyness we check that it is possible to connect to postgres and to redis. for the liveness we made a ping pong just checking the http connection. --- src/helm/impress/README.md | 138 +++++++++++++++++------------------ src/helm/impress/values.yaml | 25 +++++-- src/yhub-server/README.md | 19 ++++- src/yhub-server/server.js | 97 +++++++++++++++++++++--- 4 files changed, 191 insertions(+), 88 deletions(-) diff --git a/src/helm/impress/README.md b/src/helm/impress/README.md index 568e971ea..1bf5fb7c9 100644 --- a/src/helm/impress/README.md +++ b/src/helm/impress/README.md @@ -339,75 +339,75 @@ ### yhub -| Name | Description | Value | -| -------------------------------------------------- | ------------------------------------------------------------------------------------------------------------- | ------------------------ | -| `yhub.enabled` | Enable the yhub collaboration server, its service and its init-db job | `true` | -| `yhub.image.repository` | Repository to use to pull the yhub container image | `lasuite/impress-yhub` | -| `yhub.image.tag` | yhub container tag | `latest` | -| `yhub.image.pullPolicy` | yhub container image pull policy | `IfNotPresent` | -| `yhub.command` | Override the yhub container command | `[]` | -| `yhub.args` | Override the yhub container args | `[]` | -| `yhub.replicas` | Amount of yhub replicas | `3` | -| `yhub.shareProcessNamespace` | Enable share process namespace between containers | `false` | -| `yhub.sidecars` | Add sidecars containers to yhub deployment | `[]` | -| `yhub.terminationGracePeriodSeconds` | Grace period given to a yhub pod to drain before it is killed | `60` | -| `yhub.securityContext.allowPrivilegeEscalation` | Whether to allow privilege escalation for the yhub container | `false` | -| `yhub.securityContext.capabilities.drop` | List of capabilities to drop for the yhub container | `["ALL"]` | -| `yhub.securityContext.runAsNonRoot` | Whether to run the yhub container as a non-root user | `true` | -| `yhub.securityContext.runAsUser` | User the yhub container runs as | `1000` | -| `yhub.securityContext.runAsGroup` | Group the yhub container runs as | `1000` | -| `yhub.securityContext.seccompProfile.type` | Seccomp profile type for the yhub container | `RuntimeDefault` | -| `yhub.envVars` | Configure yhub container environment variables | `undefined` | -| `yhub.envVars.REDIS` | Required, redis/valkey url holding the live document state (e.g. redis://valkey:6379/0) | | -| `yhub.envVars.POSTGRES` | Required, url of the yhub database, created by the init-db job (e.g. postgres://user:pass@postgres:5432/yhub) | | -| `yhub.envVars.REDIS_PREFIX` | Namespace of the redis keys, when the instance is shared (default: yhub) | | -| `yhub.envVars.COLLABORATION_BACKEND_BASE_URL` | Base url of the Docs backend, which yhub asks about users and document access rights | | -| `yhub.envVars.COLLABORATION_SERVER_ORIGIN` | Comma separated list of the origins allowed to open a websocket | | -| `yhub.envVars.YHUB_JWT_PRIVATE_KEY_FILE` | Path to the RSA private key (PEM) yhub signs its calls to the backend with, mounted from a secret | | -| `yhub.envVars.SOFT_MIGRATION` | Set to "true" to seed rooms from the legacy Django/S3 document store on first access | | -| `yhub.envVars.BY_VALUE` | Example environment variable by setting value directly | | -| `yhub.envVars.FROM_CONFIGMAP.configMapKeyRef.name` | Name of a ConfigMap when configuring env vars from a ConfigMap | | -| `yhub.envVars.FROM_CONFIGMAP.configMapKeyRef.key` | Key within a ConfigMap when configuring env vars from a ConfigMap | | -| `yhub.envVars.FROM_SECRET.secretKeyRef.name` | Name of a Secret when configuring env vars from a Secret | | -| `yhub.envVars.FROM_SECRET.secretKeyRef.key` | Key within a Secret when configuring env vars from a Secret | | -| `yhub.podAnnotations` | Annotations to add to the yhub Pod | `{}` | -| `yhub.dpAnnotations` | Annotations to add to the yhub Deployment | `{}` | -| `yhub.initDbJobAnnotations` | Annotations for the yhub init-db job | `{}` | -| `yhub.jobs.ttlSecondsAfterFinished` | Period to wait before removing the init-db job | `30` | -| `yhub.jobs.backoffLimit` | Numbers of init-db job retries | `2` | -| `yhub.initDb.enabled` | Run the job creating and upgrading the yhub schema | `true` | -| `yhub.initDb.command` | Override the command creating and upgrading the yhub schema | `[]` | -| `yhub.initDb.retries` | How many times the schema script is retried while the postgres server does not answer | `60` | -| `yhub.initDb.retryDelaySeconds` | Seconds between two attempts | `5` | -| `yhub.initDb.restartPolicy` | Restart policy of the init-db job | `Never` | -| `yhub.service.type` | yhub Service type | `ClusterIP` | -| `yhub.service.port` | yhub Service listening port | `443` | -| `yhub.service.targetPort` | yhub container listening port | `3002` | -| `yhub.service.annotations` | Annotations to add to the yhub Service | `{}` | -| `yhub.probes.liveness.path` | Configure path for yhub HTTP liveness probe | `/collaboration/jwks/v1` | -| `yhub.probes.liveness.initialDelaySeconds` | Configure initial delay for yhub liveness probe | `10` | -| `yhub.probes.readiness.path` | Configure path for yhub HTTP readiness probe | `/collaboration/jwks/v1` | -| `yhub.probes.readiness.initialDelaySeconds` | Configure initial delay for yhub readiness probe | `5` | -| `yhub.probes.liveness.targetPort` | Configure port for yhub HTTP liveness probe | | -| `yhub.probes.liveness.timeoutSeconds` | Configure timeout for yhub liveness probe | | -| `yhub.probes.readiness.targetPort` | Configure port for yhub HTTP readiness probe | | -| `yhub.probes.readiness.timeoutSeconds` | Configure timeout for yhub readiness probe | | -| `yhub.probes.startup.path` | Configure path for yhub HTTP startup probe | | -| `yhub.probes.startup.targetPort` | Configure port for yhub HTTP startup probe | | -| `yhub.probes.startup.initialDelaySeconds` | Configure initial delay for yhub startup probe | | -| `yhub.probes.startup.timeoutSeconds` | Configure timeout for yhub startup probe | | -| `yhub.resources` | Resource requirements for the yhub container | `{}` | -| `yhub.nodeSelector` | Node selector for the yhub Pod | `{}` | -| `yhub.tolerations` | Tolerations for the yhub Pod | `[]` | -| `yhub.affinity` | Affinity for the yhub Pod | `{}` | -| `yhub.persistence` | Additional volumes to create and mount on the yhub. Used for debugging purposes | `{}` | -| `yhub.persistence.volume-name.size` | Size of the additional volume | | -| `yhub.persistence.volume-name.type` | Type of the additional volume, persistentVolumeClaim or emptyDir | | -| `yhub.persistence.volume-name.mountPath` | Path where the volume should be mounted to | | -| `yhub.extraVolumeMounts` | Additional volumes to mount on the yhub. Mounted on the init-db job too | `[]` | -| `yhub.extraVolumes` | Additional volumes to mount on the yhub. Mounted on the init-db job too | `[]` | -| `yhub.pdb.enabled` | Enable pdb on yhub | `true` | -| `yhub.serviceAccountName` | Optional service account name to use for yhub pods | `nil` | +| Name | Description | Value | +| -------------------------------------------------- | ------------------------------------------------------------------------------------------------------------- | ------------------------- | +| `yhub.enabled` | Enable the yhub collaboration server, its service and its init-db job | `true` | +| `yhub.image.repository` | Repository to use to pull the yhub container image | `lasuite/impress-yhub` | +| `yhub.image.tag` | yhub container tag | `latest` | +| `yhub.image.pullPolicy` | yhub container image pull policy | `IfNotPresent` | +| `yhub.command` | Override the yhub container command | `[]` | +| `yhub.args` | Override the yhub container args | `[]` | +| `yhub.replicas` | Amount of yhub replicas | `3` | +| `yhub.shareProcessNamespace` | Enable share process namespace between containers | `false` | +| `yhub.sidecars` | Add sidecars containers to yhub deployment | `[]` | +| `yhub.terminationGracePeriodSeconds` | Grace period given to a yhub pod to drain before it is killed | `60` | +| `yhub.securityContext.allowPrivilegeEscalation` | Whether to allow privilege escalation for the yhub container | `false` | +| `yhub.securityContext.capabilities.drop` | List of capabilities to drop for the yhub container | `["ALL"]` | +| `yhub.securityContext.runAsNonRoot` | Whether to run the yhub container as a non-root user | `true` | +| `yhub.securityContext.runAsUser` | User the yhub container runs as | `1000` | +| `yhub.securityContext.runAsGroup` | Group the yhub container runs as | `1000` | +| `yhub.securityContext.seccompProfile.type` | Seccomp profile type for the yhub container | `RuntimeDefault` | +| `yhub.envVars` | Configure yhub container environment variables | `undefined` | +| `yhub.envVars.REDIS` | Required, redis/valkey url holding the live document state (e.g. redis://valkey:6379/0) | | +| `yhub.envVars.POSTGRES` | Required, url of the yhub database, created by the init-db job (e.g. postgres://user:pass@postgres:5432/yhub) | | +| `yhub.envVars.REDIS_PREFIX` | Namespace of the redis keys, when the instance is shared (default: yhub) | | +| `yhub.envVars.COLLABORATION_BACKEND_BASE_URL` | Base url of the Docs backend, which yhub asks about users and document access rights | | +| `yhub.envVars.COLLABORATION_SERVER_ORIGIN` | Comma separated list of the origins allowed to open a websocket | | +| `yhub.envVars.YHUB_JWT_PRIVATE_KEY_FILE` | Path to the RSA private key (PEM) yhub signs its calls to the backend with, mounted from a secret | | +| `yhub.envVars.SOFT_MIGRATION` | Set to "true" to seed rooms from the legacy Django/S3 document store on first access | | +| `yhub.envVars.BY_VALUE` | Example environment variable by setting value directly | | +| `yhub.envVars.FROM_CONFIGMAP.configMapKeyRef.name` | Name of a ConfigMap when configuring env vars from a ConfigMap | | +| `yhub.envVars.FROM_CONFIGMAP.configMapKeyRef.key` | Key within a ConfigMap when configuring env vars from a ConfigMap | | +| `yhub.envVars.FROM_SECRET.secretKeyRef.name` | Name of a Secret when configuring env vars from a Secret | | +| `yhub.envVars.FROM_SECRET.secretKeyRef.key` | Key within a Secret when configuring env vars from a Secret | | +| `yhub.podAnnotations` | Annotations to add to the yhub Pod | `{}` | +| `yhub.dpAnnotations` | Annotations to add to the yhub Deployment | `{}` | +| `yhub.initDbJobAnnotations` | Annotations for the yhub init-db job | `{}` | +| `yhub.jobs.ttlSecondsAfterFinished` | Period to wait before removing the init-db job | `30` | +| `yhub.jobs.backoffLimit` | Numbers of init-db job retries | `2` | +| `yhub.initDb.enabled` | Run the job creating and upgrading the yhub schema | `true` | +| `yhub.initDb.command` | Override the command creating and upgrading the yhub schema | `[]` | +| `yhub.initDb.retries` | How many times the schema script is retried while the postgres server does not answer | `60` | +| `yhub.initDb.retryDelaySeconds` | Seconds between two attempts | `5` | +| `yhub.initDb.restartPolicy` | Restart policy of the init-db job | `Never` | +| `yhub.service.type` | yhub Service type | `ClusterIP` | +| `yhub.service.port` | yhub Service listening port | `443` | +| `yhub.service.targetPort` | yhub container listening port | `3002` | +| `yhub.service.annotations` | Annotations to add to the yhub Service | `{}` | +| `yhub.probes.liveness.path` | Configure path for yhub HTTP liveness probe | `/collaboration/ping/v1` | +| `yhub.probes.liveness.initialDelaySeconds` | Configure initial delay for yhub liveness probe | `10` | +| `yhub.probes.liveness.timeoutSeconds` | Configure timeout for yhub liveness probe | `2` | +| `yhub.probes.readiness.path` | Configure path for yhub HTTP readiness probe | `/collaboration/ready/v1` | +| `yhub.probes.readiness.initialDelaySeconds` | Configure initial delay for yhub readiness probe | `5` | +| `yhub.probes.readiness.timeoutSeconds` | Configure timeout for yhub readiness probe | `3` | +| `yhub.probes.liveness.targetPort` | Configure port for yhub HTTP liveness probe | | +| `yhub.probes.readiness.targetPort` | Configure port for yhub HTTP readiness probe | | +| `yhub.probes.startup.path` | Configure path for yhub HTTP startup probe | | +| `yhub.probes.startup.targetPort` | Configure port for yhub HTTP startup probe | | +| `yhub.probes.startup.initialDelaySeconds` | Configure initial delay for yhub startup probe | | +| `yhub.probes.startup.timeoutSeconds` | Configure timeout for yhub startup probe | | +| `yhub.resources` | Resource requirements for the yhub container | `{}` | +| `yhub.nodeSelector` | Node selector for the yhub Pod | `{}` | +| `yhub.tolerations` | Tolerations for the yhub Pod | `[]` | +| `yhub.affinity` | Affinity for the yhub Pod | `{}` | +| `yhub.persistence` | Additional volumes to create and mount on the yhub. Used for debugging purposes | `{}` | +| `yhub.persistence.volume-name.size` | Size of the additional volume | | +| `yhub.persistence.volume-name.type` | Type of the additional volume, persistentVolumeClaim or emptyDir | | +| `yhub.persistence.volume-name.mountPath` | Path where the volume should be mounted to | | +| `yhub.extraVolumeMounts` | Additional volumes to mount on the yhub. Mounted on the init-db job too | `[]` | +| `yhub.extraVolumes` | Additional volumes to mount on the yhub. Mounted on the init-db job too | `[]` | +| `yhub.pdb.enabled` | Enable pdb on yhub | `true` | +| `yhub.serviceAccountName` | Optional service account name to use for yhub pods | `nil` | ### docSpec diff --git a/src/helm/impress/values.yaml b/src/helm/impress/values.yaml index 95671541b..35b33708a 100644 --- a/src/helm/impress/values.yaml +++ b/src/helm/impress/values.yaml @@ -1036,27 +1036,38 @@ yhub: ## @param yhub.probes.liveness.path Configure path for yhub HTTP liveness probe ## @param yhub.probes.liveness.initialDelaySeconds Configure initial delay for yhub liveness probe + ## @param yhub.probes.liveness.timeoutSeconds Configure timeout for yhub liveness probe ## @param yhub.probes.readiness.path Configure path for yhub HTTP readiness probe ## @param yhub.probes.readiness.initialDelaySeconds Configure initial delay for yhub readiness probe + ## @param yhub.probes.readiness.timeoutSeconds Configure timeout for yhub readiness probe ## @extra yhub.probes.liveness.targetPort Configure port for yhub HTTP liveness probe - ## @extra yhub.probes.liveness.timeoutSeconds Configure timeout for yhub liveness probe ## @extra yhub.probes.readiness.targetPort Configure port for yhub HTTP readiness probe - ## @extra yhub.probes.readiness.timeoutSeconds Configure timeout for yhub readiness probe ## @extra yhub.probes.startup.path Configure path for yhub HTTP startup probe ## @extra yhub.probes.startup.targetPort Configure port for yhub HTTP startup probe ## @extra yhub.probes.startup.initialDelaySeconds Configure initial delay for yhub startup probe ## @extra yhub.probes.startup.timeoutSeconds Configure timeout for yhub startup probe ## - ## The JWKS route is the only one yhub serves unauthenticated: it answers 200 - ## with the public keys it signs its calls to the backend with (an empty set - ## when no key is configured), and reads neither redis nor postgres. + ## Two routes yhub serves unauthenticated, and they answer different + ## questions on purpose: + ## + ## - `ping` returns 200 without touching anything. Being answered at all is + ## the proof the http channel and the event loop are alive, which is as far + ## as a liveness probe should ever go: restarting a server over a store it + ## does not reach would drop the websockets it is happily serving. + ## - `ready` asks postgres and redis whether they answer, and returns 503 + ## when either does not. That takes the pod out of the service endpoints + ## and leaves its siblings serving, which is what readiness is for. Its + ## timeout is above the two seconds the server itself gives each store, so + ## an unreachable one is reported rather than cut off. probes: liveness: - path: /collaboration/jwks/v1 + path: /collaboration/ping/v1 initialDelaySeconds: 10 + timeoutSeconds: 2 readiness: - path: /collaboration/jwks/v1 + path: /collaboration/ready/v1 initialDelaySeconds: 5 + timeoutSeconds: 3 ## @param yhub.resources Resource requirements for the yhub container resources: {} diff --git a/src/yhub-server/README.md b/src/yhub-server/README.md index 849816738..324ff0a2b 100644 --- a/src/yhub-server/README.md +++ b/src/yhub-server/README.md @@ -60,6 +60,20 @@ It is not a fork of yhub — it is a thin wrapper: its own tokens: neither side is configured with a copy of the key of the other, so either can roll its key on its own. Served unauthenticated, as any JWKS is, +- answers two probes, unauthenticated like the JWKS and deliberately asking + different questions: + - `GET /collaboration/ping/v1` → `200 {"status":"pong"}` without touching + anything. Being answered at all proves the http channel and the event loop + are alive, which is as far as a **liveness** check should go: restarting a + server over a store it cannot reach would drop the websockets it is + serving perfectly well, + - `GET /collaboration/ready/v1` → `200 {"status":"ready","checks":{…}}`, or + `503` with the offending store marked `unreachable`, after asking postgres + (`SELECT 1`) and redis (`PING`) in parallel, each with a two second + budget. A **readiness** failure takes the pod out of the service endpoints + and leaves its siblings serving. The body names the store but never the + error: the route is public, and a postgres client will happily put its + connection string in the message it raises — that goes to the log instead, - mirrors the environment conventions used elsewhere in this repository (`*_FILE` secret indirection, `COLLABORATION_SERVER_ORIGIN` allowlist, …). @@ -70,7 +84,10 @@ authorization and are meant to be reachable by browsers, as is `/collaboration/jwks/v1`, which carries public keys and nothing else. The one exception is `/collaboration/reset-connections/`, `/collaboration/migrate/`, `/collaboration/restore-ydoc/` and `/collaboration/reset-ydoc/`, which are -backend-internal and should not be routed through the public ingress. +backend-internal and should not be routed through the public ingress. The two +probes are not worth publishing either — kubelet calls them from inside — and +the helm chart's ingress lists what it routes rather than what it hides, so +they stay in-cluster on their own. ## Container image diff --git a/src/yhub-server/server.js b/src/yhub-server/server.js index f2cfc9a39..d63a4e1f9 100644 --- a/src/yhub-server/server.js +++ b/src/yhub-server/server.js @@ -43,9 +43,20 @@ const ORG = process.env.YHUB_ORG || 'docs'; // URL scheme Docs already routes to the collaboration server. Hardcoded like // the audiences: the backend builds its urls with the same prefix. const API_PREFIX = 'collaboration'; -// Path the JWKS endpoint declared in `api` is mounted at. readAuthInfo reads -// the raw request, without any route context, hence the duplication. -const JWKS_PATH = `/${API_PREFIX}/jwks/v1`; +// Paths of the routes declared in `api` that are served to anyone: the JWKS, +// which carries public keys and which the backend must read before it can +// authenticate anything we send it, and the two probes, which kubernetes calls +// with no cookie and no token. readAuthInfo reads the raw request, without any +// route context, hence the duplication of the paths here. +const PUBLIC_PATHS = new Set([ + `/${API_PREFIX}/jwks/v1`, + `/${API_PREFIX}/ping/v1`, + `/${API_PREFIX}/ready/v1`, +]); +// What the readiness check gives a store before reporting it unreachable. Short +// on purpose: the point of the probe is to answer, and answering "not ready" +// early is more useful than holding the connection until kubelet times out. +const READINESS_TIMEOUT_MS = 2000; // Requiring this audience stops a valid admin JWT that Django issued for // another service (today: the y-converter token in converter_services.py, // which is handed to the converter process) from being replayed against yhub. @@ -207,12 +218,10 @@ const auth = createAuthPlugin({ const cookie = req.getHeader('cookie'); const origin = req.getHeader('origin'); const gcOff = req.getQuery('gc') === 'false'; - // The JWKS holds public keys and nothing else, and the backend must be - // able to fetch it before it can authenticate anything we send it — so it - // is served to anyone, as the backend serves its own. This identity is - // granted the 'jwks' purpose and nothing else (getGlobalAccessType), and - // the check is on the exact path of that one route. - if (url === JWKS_PATH) { + // The JWKS and the probes are served to anyone (see PUBLIC_PATHS). This + // identity is granted their purposes and nothing else + // (getGlobalAccessType), and the check is on their exact paths. + if (PUBLIC_PATHS.has(url)) { return { userid: 'anonymous' }; } if (authorization !== '') { @@ -284,9 +293,12 @@ const auth = createAuthPlugin({ return { userid: `anon:${anon}`, cookie, origin }; } }, - // Authorizes the global-scoped endpoints, of which the JWKS is the only one. + // Authorizes the global-scoped endpoints: the JWKS and the two probes, all + // of them read-only and public. Anything else is refused here. async getGlobalAccessType(authInfo, purpose) { - return purpose === 'jwks' ? 'r' : null; + return purpose === 'jwks' || purpose === 'ping' || purpose === 'ready' + ? 'r' + : null; }, async getAccessType(authInfo, { org, docid, branch }, purpose) { if (authInfo.admin === true) { @@ -388,7 +400,70 @@ const eraseContent = async (yhub, room, by) => { await yhub.persistence.deleteTombstone(room); }; +const readyLog = logger.child({ module: 'readiness' }); + +// One readiness check: is that store answering? The error never leaves the +// server — the route is unauthenticated, and a postgres client is happy to put +// its connection string, password included, in the message it raises. +const checkStore = async (name, probe) => { + let timer; + try { + await Promise.race([ + probe(), + new Promise((_, reject) => { + timer = setTimeout( + () => reject(new Error(`no answer in ${READINESS_TIMEOUT_MS}ms`)), + READINESS_TIMEOUT_MS, + ); + }), + ]); + return [name, 'ok']; + } catch (err) { + readyLog.warn({ store: name, err: err?.message }, 'store is unreachable'); + return [name, 'unreachable']; + } finally { + clearTimeout(timer); + } +}; + const api = [ + // GET /collaboration/ping/v1 — liveness. It answers, therefore the http + // channel and the event loop are alive, which is all a liveness probe should + // ever conclude: touching redis or postgres here would restart a server that + // holds perfectly good websocket connections every time a store blinks. + createApiEndpoint('ping', { + scope: 'global', + accessPurpose: 'ping', + get: { + handler: () => jsonResponse(200, { status: 'pong' }), + }, + }), + // GET /collaboration/ready/v1 — readiness. The two stores this server cannot + // serve a single document without: the postgres holding the persisted state + // and the redis carrying the updates between replicas. Answering 503 takes + // this pod out of the service endpoints and leaves the others serving, which + // is the whole difference with the liveness probe above. + createApiEndpoint('ready', { + scope: 'global', + accessPurpose: 'ready', + get: { + handler: async (req) => { + // both at once: a probe is not the place to add the latency of one + // store to the latency of the other + const checks = Object.fromEntries( + await Promise.all([ + checkStore('postgres', () => req.yhub.persistence.sql`SELECT 1`), + checkStore('redis', () => req.yhub.stream.redis.ping()), + ]), + ); + const ready = Object.values(checks).every((state) => state === 'ok'); + return jsonResponse(ready ? 200 : 503, { + status: ready ? 'ready' : 'unready', + checks, + }); + }, + }, + }), // GET /collaboration/jwks/v1 — the public keys verifying the tokens we sign // to call the backend, in the JSON Web Key Set format (RFC 7517). Global // scope: it is about this server, not about a document, so the route carries