diff --git a/Lessons-Learned.md b/Lessons-Learned.md index 9d0671a..78e3d82 100644 --- a/Lessons-Learned.md +++ b/Lessons-Learned.md @@ -522,7 +522,7 @@ Nothing about that fails. The pod becomes `Ready`, the UI works, the route is `A Three things worth carrying forward, and the first is the one that nearly got missed: -- **A chart's second routing mode is not a template change, it is a change to what `ingress.enabled` means** — and the readers that are affected are the ones **outside** the Ingress template, where nobody reviewing a routing PR is looking. The grep that finds them is `.Values.ingress.enabled` across the chart, *minus* the Ingress template and `NOTES.txt`. Still open on that count when group 1 landed: [freesailarr](freesailarr) and [ytdlp-web-player](ytdlp-web-player), both of which read it in their helpers. **Group 2 confirmed the prediction on freesailarr, and the symptom was a different one** — see below. `ytdlp-web-player` remains open. +- **A chart's second routing mode is not a template change, it is a change to what `ingress.enabled` means** — and the readers that are affected are the ones **outside** the Ingress template, where nobody reviewing a routing PR is looking. The grep that finds them is `.Values.ingress.enabled` across the chart, *minus* the Ingress template and `NOTES.txt`. Still open on that count when group 1 landed: [freesailarr](freesailarr) and [ytdlp-web-player](ytdlp-web-player), both of which read it in their helpers. **Group 2 confirmed the prediction on freesailarr, and the symptom was a different one; group 3 confirmed it on ytdlp-web-player, and the symptom was worse than either** — both below. The screen is closed at three finds, which was every chart that had the pattern. - **"Sets no behaviour-bearing annotation" is not the same question as "needs none."** Group 1's selection criterion screened on annotations a chart *sets*, measured and correct — and [garage](garage) passed it with `annotations: {}` while its own values comment tells the operator to set `proxy-body-size: "0"`, because ingress-nginx truncates S3 uploads at 1 MB. `proxy-body-size` has no Gateway-API counterpart at all, so garage is a sixth chart that loses an upload limit in Gateway mode where the issue had counted five. A screen over what a chart *declares* misses what a chart *documents*. - **A claim about a value's only reader expires when a second template is added.** garage's Ingress template stated, under the guideline that exists for exactly this, that nothing outside it read `ingress.domain` — true when written, false the moment an `HTTPRoute` read it as its hostname. The same sentence existed in its `values.schema.json`. **A "checked in both directions" note is a measurement with a date on it**, and the work that invalidates it is the work that has to correct it. [calibre-web-automated](calibre-web-automated) carried the identical claim and needed the identical correction in group 2 — the pattern repeats because the sentence is copied. @@ -609,3 +609,36 @@ The investigation produced a **correct search** and a **wrong conclusion**, and - **A search over the chart answers what the chart does; what the running system does is a second measurement.** The chart is one input among several — the cluster repository's values, the controller's flags, an admission webhook, a previous operator's hand edit. Where a finding about a chart is meant to carry a claim about *behaviour*, read the live object or the repository that supplies its values; a correct search over the wrong scope produces a confident wrong answer. - **An operator can close a chart's gap invisibly, and the chart still looks correct.** Where the closure lives in another repository, the chart is incomplete, the system works, and the change that removes the closure contains no trace of it — which is why "this has always worked" is evidence about a deployment and not about a chart. - **A "nothing else reads this" note is a measurement, not a property.** It is true on the day it is written, it is what later decisions rest on, and the change that falsifies it is the change obliged to rewrite it — in every copy, the schema description included. + +## Four of six charts leaned on an annotation the chart never set + +**Where:** group 3 of the Gateway-API sweep (Gitea #114) — [keycloak](keycloak), [matrix-synapse](matrix-synapse), [stalwart](stalwart), [bulwark](bulwark); and not [homeassistant](homeassistant) or [ytdlp-web-player](ytdlp-web-player). + +**What it is:** the generalisation of the entry above, and the reason that one is recorded as a method and not as a qBittorrent bug. At #118 the discovery was singular and late: one chart, one annotation, found *after* shipping, while migrating a host. Group 3 asked the same question **before** writing any Gateway branch, with read-only `kubectl` against the live cluster — and found it was not singular at all. + +| Chart | Set on the live Ingress by the **operator**, never by the chart | +|---|---| +| [keycloak](keycloak) | `proxy-buffer-size: 16k`, `proxy-buffers-number: 4` — on **both** of its Ingress objects | +| [matrix-synapse](matrix-synapse) | `proxy-body-size: 50m`, the `nginx.org/client-max-body-size` twin for the other nginx controller, and a `configuration-snippet` | +| [stalwart](stalwart) | `enable-cors: true` plus `cors-allow-origin`/`-methods`/`-headers` | +| [bulwark](bulwark) | `proxy-body-size: 25m` | + +**None of it is findable from inside the repository.** A `git grep` over those six chart directories for `proxy-buffer`, `app-root`, `more_set_headers`, `enable-cors`, `cors-allow`, `proxy-body-size` and `configuration-snippet` hits **documentation only** — `bulwark/values.yaml` *recommending* `proxy-body-size: "25m"`, `ytdlp-web-player/values.yaml` *recommending* `proxy-buffer-size: "16k"`. No template sets any of them. Every one of the four would have vanished at the mode switch, in a diff containing no trace of it, because the annotation channel does not exist in Gateway mode. + +**So the measurement at the live object is part of the procedure, not a flourish.** Before writing a chart's Gateway branch: read its live routing objects. Reading the chart is not a substitute, and neither is reading the issue that planned the sweep — #114's own measured table listed `proxy-body-size` for five charts and did not include `matrix-synapse`, which has it. + +### And three qualifications, because the measurement is only the first step + +**Read the directive before calling an annotation load-bearing.** matrix-synapse's snippet looks exactly like what Synapse's `x_forwarded` setting depends on — client IP for rate limiting, and federation — and it is the entry in the table that would frighten a reviewer most. It is not load-bearing at all: `more_set_headers` is the headers-more module's **output**-header directive ("Replaces (if any) or adds (if not any) the specified *output* headers"), the request-side directive being `more_set_input_headers`. Those three headers go on the **response to the client**; Synapse never sees them, and what it reads with `x_forwarded: true` is the *request's* `X-Forwarded-For`, which the proxy supplies itself. The chart's notes therefore say in as many words **not** to re-create it at the Gateway and **not** to read its absence as the cause of a rate-limiting problem. An annotation that looks alarming and does nothing costs as much review time as one that matters — and had this gone the other way round, the sweep would have shipped a scary paragraph about a non-problem. + +**"No counterpart" is a claim to check against the pinned CRDs, and once it was false.** stalwart's CORS is the loss that breaks a *second chart of the same group* — the allow-origin names bulwark, the webmail, so the symptom is a browser console error with nothing in the mail server's log. The convenient conclusion would have been "no counterpart, document it". Grepping the CRD set this repository actually pins says otherwise: HTTPRoute's filters carry a full `cors` field (GEP-1767) with `allowOrigins`, `allowMethods`, `allowHeaders`, `allowCredentials` and `maxAge`, and `responseHeaderModifier` sits beside it — **both `Support: Extended`, neither Core**. + +The chart still renders neither, and the three reasons are the transferable part: + +1. **No values to render it from.** The origin and the method list are the operator's, and the chart sets no CORS annotation in Ingress mode either — so a filter would exist in Gateway mode only. That asymmetry is its own defect and its own piece of work, not something to slip into a sweep. +2. **Extended support is not support.** A rejected route is rejected **whole**, which here would take the WebUI and JMAP down together. And the observed failure is quieter than rejection: Cilium accepts `filters.type: CORS` and forwards the browser's preflight upstream anyway — `Accepted: True`, behaviour absent. A chart cannot know which implementation it landed on. +3. **Half a feature is worse than none, and the expressible half was the useless one.** A `responseHeaderModifier` can add the `Access-Control-Allow-*` headers; the **preflight** must be answered by the *gateway* rather than the backend, which a header filter cannot do. And because the allowed headers include `Authorization`, **every** request from the webmail is preflighted — so the half that needs no CORS filter is precisely the half that does not help. A rendered half would have looked configured and failed on the first real call. + +**A guard can matter *more* in the new mode, and that is a reason to look at what it protects.** ytdlp-web-player was the last open chart on the `ingress.enabled`-derived screen, and the find was not an environment variable but a **render guard**: the one refusing to publish the application and its bundled oauth2-proxy on the same host. In Ingress mode ignoring that guard fails loudly — ingress-nginx's admission webhook rejects the second Ingress and the Fleet bundle lands in `ErrApplied`. In Gateway mode **the webhook never fires**: it compares Ingress objects with Ingress objects, and an `HTTPRoute` is a different *kind*. Two valid objects then serve one hostname over two data paths that are not equivalent — the proxy's Ingress through authentication, the chart's route **straight to the application** — and which one a client reaches depends on what DNS resolves to. So a guard whose Ingress-mode job was to pre-empt a noisy webhook becomes, in Gateway mode, the only thing between a values file and an **authentication bypass that comes and goes with a DNS record**. When screening for a derived value, ask what it *protects*, not only what it computes. + +**And a shape read off an issue's summary table is a hypothesis.** Three of #114's four descriptions of the group-3 shapes were wrong — bulwark has one Ingress rather than two and has no `rewrite-target` at all; matrix-synapse renders one object with two host rules rather than a loop over apps; stalwart's routing object is not per node group. Two of the three had already been copied into `README.md` and into the header comment of `ci/run-httproute-match-test.sh`, where they read as settled fact and would have been copied again. The bulwark one has a mechanism worth naming: the table was built by grepping for the string `rewrite-target`, and every occurrence of that string in bulwark is an explanation of **why the annotation does not help it**. A grep counts refutations as occurrences.