Merge pull request 'route-adapter: write a body limit as the predecessor's buffering middleware' (#94) from feat/a-route-may-limit-the-body-it-carries into main

modules/route-adapter/README.md

@@ -159,3 +159,15 @@ cd modules/route-adapter && npm test
 They hold it to what ADR 0104 says holds it: one file per contribution, a file removed when its
 contribution goes, every file it did not write left alone — and the two facts a route file has to
 get right, the port the contributor publishes and the address of the machine it is on.
+
+## A body limit
+
+A contribution may say `max-request-body`, in bytes, and the adapter writes it as the predecessor's
+own `buffering` middleware, named after the router so the two halves cannot drift. A route that says
+nothing gets no middleware and the predecessor's default stands.
+
+This is the one thing the file shape *can* say that a policy cannot, which is why it is written
+rather than skipped: the predecessor already served its own registry name this way. A limit that is
+not a whole positive number of bytes takes the route with it — written without the limit, the
+predecessor would carry exactly what the module said not to carry, and this module would report
+success doing it.

modules/route-adapter/adapter.ts

@@ -75,6 +75,15 @@ export interface Route {
   from: string;
   /** Where the predecessor's proxy is to send it. */
   target: string;
+  /**
+   * The largest request body, in bytes, the predecessor may carry to it — the contribution's
+   * `max-request-body`. Absent is whatever the predecessor does by default.
+   *
+   * Unlike a policy, this file shape *can* say it: the predecessor has a buffering middleware, and
+   * its own registry route used exactly this. A registry takes image layers in single requests of
+   * gigabytes, so a route that could not say it would be a name nothing could be pushed to.
+   */
+  maxRequestBody?: number;
 }
 
 /** What one pass changed. */
@@ -176,12 +185,40 @@ export function routesFrom(document: unknown, machine: string): { routes: Route[
     }
     // Where the mesh says that machine is. Empty means this one, and this one is reached from
     // inside the predecessor's container by the machine's own name, not by loopback.
+    // A limit it cannot honour is a route it does not write — skipped and named, like a port that
+    // is not one. Written without the limit instead, the predecessor would carry exactly what the
+    // module said not to carry, and this adapter would report success.
+    const askedLimit = entry.values?.["max-request-body"];
+    const limit = asBodyLimit(askedLimit);
+    if (limit === null) {
+      skipped.push(
+        `${from} asked for route ${name} with a max-request-body of ${JSON.stringify(askedLimit)}, ` +
+          `which is not a whole positive number of bytes`,
+      );
+      continue;
+    }
     const at = typeof entry.at === "string" && entry.at.trim() !== "" ? entry.at.trim() : machine;
-    routes.push({ name, from, target: `http://${at}:${port}` });
+    routes.push({ name, from, target: `http://${at}:${port}`, ...(limit === undefined ? {} : { maxRequestBody: limit }) });
   }
   return { routes, skipped };
 }
 
+/**
+ * The body limit a contribution asked for: a number, `undefined` for silence, `null` for unusable.
+ *
+ * Three answers rather than two, because "said nothing" and "said something wrong" must not become
+ * the same route.
+ */
+function asBodyLimit(value: unknown): number | undefined | null {
+  if (value === undefined) {
+    return undefined;
+  }
+  if (typeof value !== "number" || !Number.isInteger(value) || value < 1) {
+    return null;
+  }
+  return value;
+}
+
 /** The file one route is written to. The prefix is how the mesh recognises its own. */
 export function fileNameFor(name: string): string {
   return `mesh-${name}.yml`;
@@ -201,6 +238,10 @@ export function routerNameFor(name: string): string {
  */
 export function routeFile(route: Route, settings: Settings): string {
   const id = routerNameFor(route.name);
+  // The body limit is a middleware in the predecessor's vocabulary — its `buffering`, with the one
+  // field the predecessor's own registry route set — named after the router so the two halves cannot
+  // drift, and written only when the contribution asked for it.
+  const limited = route.maxRequestBody !== undefined;
   return [
     marker,
     `# ${route.from} contributed this route. It is removed when that contribution goes.`,
@@ -210,10 +251,19 @@ export function routeFile(route: Route, settings: Settings): string {
     `      entryPoints: [${settings.entrypoint}]`,
     `      rule: Host(\`${route.name}\`)`,
     `      service: ${id}`,
+    ...(limited ? [`      middlewares: [${id}-body]`] : []),
     "      tls:",
     `        certResolver: ${settings.resolver}`,
     "        domains:",
     `          - main: ${route.name}`,
+    ...(limited
+      ? [
+          "  middlewares:",
+          `    ${id}-body:`,
+          "      buffering:",
+          `        maxRequestBodyBytes: ${route.maxRequestBody}`,
+        ]
+      : []),
     "  services:",
     `    ${id}:`,
     "      loadBalancer:",

modules/route-adapter/test/adapter.test.ts

@@ -22,7 +22,9 @@ async function predecessor(already: Record<string, string> = {}): Promise<Settin
 }
 
 /** The contributions file the mesh writes, in the shape the mesh's own proxy also reads. */
-function contributed(...given: { from: string; node?: string; at?: string; name: string; port: number }[]) {
+function contributed(
+  ...given: { from: string; node?: string; at?: string; name: string; port: number; limit?: unknown }[]
+) {
   return {
     contributions: 1,
     requirement: "route",
@@ -30,7 +32,7 @@ function contributed(...given: { from: string; node?: string; at?: string; name:
       from: g.from,
       node: g.node ?? "control-node",
       at: g.at ?? "",
-      values: { name: g.name, port: g.port },
+      values: { name: g.name, port: g.port, ...(g.limit === undefined ? {} : { "max-request-body": g.limit }) },
     })),
   };
 }
@@ -232,3 +234,41 @@ test("it refuses when the predecessor's directory is not there, and says why", a
   const settings = { ...defaults, dynamic: join(await mkdtemp(join(tmpdir(), "route-adapter-")), "absent") };
   await assert.rejects(reconcile([], settings), /is not there.*`dynamic` setting.*mounts it/s);
 });
+
+// **A registry is why a route needs to say this.** Image layers arrive as single requests of
+// gigabytes, and the predecessor served its own registry name with a `buffering` middleware for
+// exactly that reason. The contribution carries the limit as `max-request-body`, the adapter writes
+// the middleware the predecessor already understands, named after the router so the two halves
+// cannot drift — and writes nothing of the kind for a route that did not ask.
+test("a body limit is written as the predecessor's buffering middleware", async () => {
+  const settings = await predecessor();
+  const changed = await pass(settings, contributed(
+    { from: "registry", name: "images.example", port: 5001, limit: 21474836480 },
+    { from: "forge", name: "git.example", port: 2999 },
+  ));
+  assert.deepEqual(changed.written, ["mesh-git.example.yml", "mesh-images.example.yml"]);
+
+  const written = await readFile(join(settings.dynamic, "mesh-images.example.yml"), "utf8");
+  assert.match(written, /^ {6}middlewares: \[mesh-images-example-body\]$/m);
+  assert.match(written, /^ {2}middlewares:\n {4}mesh-images-example-body:\n {6}buffering:\n {8}maxRequestBodyBytes: 21474836480$/m);
+
+  // The route that asked for nothing carries no middleware — the predecessor's default stands.
+  const plain = await readFile(join(settings.dynamic, "mesh-git.example.yml"), "utf8");
+  assert.doesNotMatch(plain, /middlewares|buffering/);
+});
+
+// A limit it cannot honour is a route it does not write. Written without it, the predecessor would
+// carry exactly what the module said not to carry, and this module would report success.
+test("a body limit that is not a whole number of bytes is skipped and named", () => {
+  for (const limit of ["20g", 0, -1, 1.5, true, null]) {
+    const { routes, skipped } = routesFrom(
+      contributed({ from: "registry", name: "images.example", port: 5001, limit }), defaults.machine);
+    assert.deepEqual(routes, [], `a limit of ${JSON.stringify(limit)} was served`);
+    assert.equal(skipped.length, 1);
+    assert.match(skipped[0]!, /max-request-body/);
+  }
+  // And a limit the mesh's own proxy would accept is carried through, as a number.
+  const { routes } = routesFrom(
+    contributed({ from: "registry", name: "images.example", port: 5001, limit: 1024 }), defaults.machine);
+  assert.equal(routes[0]?.maxRequestBody, 1024);
+});