From f202fdc3fc87077195b0816349c064e79fa0f0f2 Mon Sep 17 00:00:00 2001 From: Justin Klein Date: Mon, 12 Oct 2026 20:06:09 +0000 Subject: [PATCH] fix(freesailarr): redirect the bare /qbittorrent prefix in Gateway mode qBittorrent has no base-path setting, so the chart serves it under a prefix and strips that prefix before the request reaches it. For the slashed form that is correct. For the BARE form - /qbittorrent, no trailing slash - the request is rewritten to "/" and qBittorrent answers 200 with its login page, while the browser's document URL keeps the prefixless form. Every relative link in that page therefore resolves against the host root, and on the shared host the root is seerr: the login form comes up unstyled and does nothing while a different application answers its assets and XHRs. Nothing reports an error at any level. The shared-domain HTTPRoute now carries a second rule for qBittorrent: an Exact match on qbittorrent.ingress.path with a RequestRedirect filter that replaces the full path with the slashed form, statusCode 301. The Gateway API ranks an Exact path match above every prefix match, so it wins over both qBittorrent's own PathPrefix rule and seerr's "/" wherever it is emitted - the rules' length-descending order stays a rendering convention. 301 rather than 308 because only 301 and 302 are Core in that API; an Extended value an implementation declines makes it set Accepted=False on the WHOLE route, which would take the four neighbouring apps' routing down with it, and that would also contradict decision 1 of Gitea #114. statusCode is written out because it defaults to 302. The rule is guarded on the app being qbittorrent and its path not being "/", and it reads the path from the value, never a literal. The INGRESS BRANCH IS UNCHANGED, deliberately. The remedy there is an nginx configuration-snippet annotation, and the chart cannot ship it: snippets are ignored unless the controller runs with allow-snippet-annotations=true, off by default since ingress-nginx 1.9 and only switched on by accepting annotation risk level "Critical". A shipped annotation would be a silent no-op on every cluster keeping the default. The working snippet, that price, and why a third Ingress object with pathType: Exact is not a substitute - buildLocation in ingress-nginx controller-v1.14.5 returns a regex location in its enforceRegex branch before reaching the PathTypeExact branch, and enforceRegex is computed once per nginx server over every location of the hostname, one of which carries rewrite-target - are all written out in values.yaml instead. ci/run-httproute-match-test.sh gains a third assertion, per HTTPRoute object: a rule stripping a prefix with ReplacePrefixMatch requires a rule in the same route matching that path Exactly and carrying a RequestRedirect filter. This is what stops the gap reappearing; bulwark is the second rewrite-target chart and meets the same check when group 3 of Gitea #114 lands. 37 assertions over 15 charts, up from 36. Both fail paths were exercised: the rule deleted, and the redirect pointed at a path other than the one stripped. Minor and not major: with the Ingress branch untouched and httpRoute.enabled defaulting to false, no existing release's render changes by a byte and no value becomes required. Shown by an empty helm template diff against origin/main in both Ingress shapes, shared-domain and per-app hosts. Fixes #118 --- README.md | 13 +- ci/run-httproute-match-test.sh | 113 +++++++++++++++++- freesailarr/Chart.yaml | 2 +- freesailarr/templates/NOTES.txt | 46 +++++-- .../templates/freesailarr.httproute.yaml | 112 +++++++++++++++-- freesailarr/values.yaml | 82 ++++++++++++- 6 files changed, 345 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index d42d187..50184b7 100644 --- a/README.md +++ b/README.md @@ -163,22 +163,25 @@ Measured: 30 charts, **5 selectors verified in ~14 s**, no cluster. The fail pat ### HTTPRoute render test (all charts, every PR) -`ci/run-httproute-match-test.sh [chart …]` (no argument = all charts) renders every chart with the same fixtures the install-test uses and checks every `HTTPRoute` in the result against the `Service` objects of that **same** render. Introduced with the Gateway-API pilot (Gitea #114); two failure modes, both silent: +`ci/run-httproute-match-test.sh [chart …]` (no argument = all charts) renders every chart with the same fixtures the install-test uses and checks every `HTTPRoute` in the result against the `Service` objects of that **same** render. Introduced with the Gateway-API pilot (Gitea #114); three failure modes, all silent: | Finding | What it does | Who notices today | |---|---|---| | `parentRefs` empty | the route attaches to **no** Gateway: created, schema-valid, rejected by nobody, and it routes nothing | nobody — no event, no condition, the only symptom is a hostname that does not answer | | `backendRefs` naming a Service or a port that does not exist | the reference is resolved **by name** at runtime and validated by nothing | nobody in CI — `ResolvedRefs: False` lands in a route status that only a Gateway controller writes, and none runs here | +| a prefix stripped with `ReplacePrefixMatch` while the same route carries no `Exact` match on that prefix with a `RequestRedirect` filter (Gitea #118) | the **slashless** form of the path is rewritten to `/` and answered **200** by the app, under a document URL that keeps the prefix off — so every relative link in that page resolves against the host root, which on a shared host is a *different* application | nobody — the app's own assets and XHRs are served by its neighbour, and the symptom is a half-working UI, not an error | -**The real CRDs in the install-test do not close either gap**, which is the whole reason this job exists next to them: the Gateway-API schema happily accepts an empty `parentRefs` list and a `backendRefs` entry pointing at a Service that was never created. The CRDs prove the object is *well-formed*; this test proves it *points somewhere*. The port half is a genuine new exposure rather than a hypothetical: an Ingress backend references its Service port **by name**, while `backendRefs` takes a `PortNumber` and cannot, so the number in the route and the port in the Service are kept in step by hand from now on. +**The real CRDs in the install-test close none of these gaps**, which is the whole reason this job exists next to them: the Gateway-API schema happily accepts an empty `parentRefs` list, a `backendRefs` entry pointing at a Service that was never created, and a prefix-stripping rewrite with no redirect beside it. The CRDs prove the object is *well-formed*; this test proves it *points somewhere*, and *somewhere right*. The port half is a genuine new exposure rather than a hypothetical: an Ingress backend references its Service port **by name**, while `backendRefs` takes a `PortNumber` and cannot, so the number in the route and the port in the Service are kept in step by hand from now on. + +The third check is a **pairing within one HTTPRoute object**, and the scope is deliberate: Gateway-API matching precedence is a property of a single route, so a redirect in a *different* route (a different hostname) says nothing about this one. Precedence also makes the pairing a question of existence rather than order — an `Exact` path match outranks every prefix match, so the redirect rule wins wherever it is emitted, and the rules' length-descending order stays a rendering convention. `freesailarr` is the first chart it applies to; `bulwark`, the second `rewrite-target` chart, meets the same check when group 3 of #114 lands, which is why the rule lives here and not in one chart's template. Three things to know before touching it: - **Helm is the YAML parser**, through `ci/yaml2json`, for the same reason as the selector-match test above — the runners have neither `pyyaml` nor `yq`, and a parser written in `awk` fails by reading nothing and reporting nothing. -- **What is deliberately not checked**, named so the gaps are not mistaken for coverage. *That traffic flows* — no Gateway controller runs in CI and none will, so `Accepted: True` is not obtainable; that claim is simply not made anywhere in this pipeline. *Mutual exclusion with the Ingress* — each chart aborts its own render when `ingress.enabled` and `httpRoute.enabled` are both true, which is where it belongs: this test renders one configuration per chart, and a rule here would have to assume the switch is chart-global, which may not survive the multi-app charts of the sweep. *Cross-namespace and non-Service backends*, and a rule with no `backendRefs` at all (legal for a filter-only rule) — all three are **reported as "not checked", never counted as passing**. +- **What is deliberately not checked**, named so the gaps are not mistaken for coverage. *That traffic flows* — no Gateway controller runs in CI and none will, so `Accepted: True` is not obtainable; that claim is simply not made anywhere in this pipeline. *That any redirect is ever served* — the pairing check proves the rendered **object** carries a `RequestRedirect` rule on the bare prefix and nothing more: whether an implementation honours the `statusCode` or declines it and fails the whole route is outside what a render can see. *Mutual exclusion with the Ingress* — each chart aborts its own render when `ingress.enabled` and `httpRoute.enabled` are both true, which is where it belongs: this test renders one configuration per chart, and a rule here would have to assume the switch is chart-global, which may not survive the multi-app charts of the sweep. *Cross-namespace and non-Service backends*, and a rule with no `backendRefs` at all (legal for a filter-only rule) — all three are **reported as "not checked", never counted as passing**. The redirect rule the pairing check demands is itself backend-less, so a chart that satisfies that check also earns one "not checked" line; both verdicts are correct at once. - **Two anti-vacuum guards**, because the convention ships `httpRoute.enabled: false` and this test would otherwise be green over nothing. `ci/httproute/.yaml` is a render-only overlay applied **last** whose one job is to put a chart into Gateway mode — and a chart that *has* one and still renders no route **fails**. On top of that the run fails if **no assertion at all** held anywhere. `cyberchef` is exercised through its ordinary install-test fixture (which installs in Gateway mode, see below), every other converted chart through its overlay. For a multi-app chart the overlay also has to enter every shape the template has: `ci/httproute/freesailarr.yaml` moves one app onto a host of its own so that the shared-host route *and* the per-app route are both rendered. -Measured in CI (run 719, the pilot): **30 charts, 4 assertions held**, whole job 13 s including checkout and the Helm setup, no cluster. Group 1 took that to 20 assertions over 10 charts and group 2 to **36 over 15** — the sweep adds charts, not checks, so the job stays a render. The fail path was exercised rather than assumed: a Service renamed in the route, a port the Service does not expose, a port left out entirely, an emptied `parentRefs` with the chart's own guard disabled, an overlay that renders no route, and a run with zero assertions — each produced a `FAIL` naming both sides and a non-zero exit. +Measured in CI (run 719, the pilot): **30 charts, 4 assertions held**, whole job 13 s including checkout and the Helm setup, no cluster. Group 1 took that to 20 assertions over 10 charts and group 2 to 36 over 15; the prefix/redirect pairing of Gitea #118 adds the first check since the pilot rather than a chart, which brings it to **37 over 15** — one assertion for `freesailarr`'s `/qbittorrent`, plus one more "not checked" line for the backend-less redirect rule it demands. The fail path was exercised rather than assumed: a Service renamed in the route, a port the Service does not expose, a port left out entirely, an emptied `parentRefs` with the chart's own guard disabled, an overlay that renders no route, a run with zero assertions, and — for the pairing — both the redirect rule deleted outright and the redirect pointed at a path other than the one stripped; each produced a `FAIL` naming both sides and a non-zero exit. ### Install-test (PR pipeline stage 2) @@ -393,7 +396,7 @@ These guidelines are **binding for every current and future chart** in this repo - **CI**: either the chart's install-test fixture installs in Gateway mode, or it gets a render-only overlay at `ci/httproute/.yaml` — otherwise `ci/run-httproute-match-test.sh` is green over nothing for it, which that test fails on. An overlay for a multi-app chart has a second job: it must enter **every** shape the template has, or the branch nobody exercised is the one that breaks. `ci/httproute/freesailarr.yaml` gives one app a host of its own so both the shared-host route and the per-app route render. Reference implementation: `template-chart/templates/xxx.httproute.yaml`. Pilot: `cyberchef`. Group 1 — the charts rendering one Ingress with no behaviour-bearing annotation — followed: `bentopdf`, `garage`, `patchmon`, `rallly`, `searxng`, `sure`, `urlaubsverwaltung`, `voucher-vault`. Group 2 — the charts whose behaviour hangs off an nginx annotation — followed: `calibre-web-automated`, `outline`, `paperless-ngx`, `zipline` (all four `proxy-body-size`) and `freesailarr` (`rewrite-target`). That leaves **seven** charts: the six of group 3 — the special shapes (`homeassistant` is the documented ingress exception, `keycloak` and `bulwark` have two Ingresses each, `matrix-synapse` renders in a loop, `stalwart` renders per node group, `ytdlp-web-player` hangs off the oauth2-proxy subchart) — and `rss-bridge`, which is frozen for changes (#51) and is why #114 cannot be closed while #51 is open. `garage` was one of the named special cases and is done: it ships `ingress.enabled: false`, so both of its modes are off by default and its route is exercised by a render overlay rather than by the install-test, which keeps its Ingress. `paperless-ngx` was the one chart in the repository that still rendered its Ingress **unconditionally**; group 2 gave it an `ingress.enabled` switch defaulting to `true`, because without a second value path there is nothing for the mutual-exclusion abort to name and the `required()` on `clusterIssuer` would have kept demanding a value the Gateway mode does not use. -- **Documented exception — freesailarr, and it extends to the Gateway mode**: the chart bundles several independently exposed apps in one pod, so it has **no** `ingress.domain` — and therefore **no `httpRoute.hostnames`** either, the only chart without that key. A chart-global list of hostnames could not say which of the ten apps it belonged to, so the hostnames stay where they already are, in `ingress.host` and `.ingress.host`; both are read by both modes and neither is an Ingress switch. In Gateway mode the per-app shape renders one `HTTPRoute` per app (`---httproute`) and the shared shape exactly one (`--httproute`) with one rule per app — qBittorrent included, since its prefix-stripping rewrite is a per-rule filter there. It offers both shapes. By default each app carries its own `.ingress.enabled` plus a required `.ingress.host` (required only while that app's ingress is enabled and it is not on the shared domain) and renders its own Ingress object `---ingress` with its own TLS secret `tls----ingress`. Setting the global `ingress.host` switches every app without a host of its own to the shared-domain shape under `.ingress.path`; qBittorrent (no base path, needs a prefix-stripping rewrite) gets the extra object described above, and seerr as well as mediathekarr are pinned to `/` — mediathekarr because its UI links assets with root-absolute paths, which a rewrite cannot fix; with both of them on one shared host the duplicate-path guard fires and one of them needs a host of its own (its ingress is therefore off by default). `clusterIssuer`, `className` and `annotations` stay global defaults under `ingress.` and are overridable per app. +- **Documented exception — freesailarr, and it extends to the Gateway mode**: the chart bundles several independently exposed apps in one pod, so it has **no** `ingress.domain` — and therefore **no `httpRoute.hostnames`** either, the only chart without that key. A chart-global list of hostnames could not say which of the ten apps it belonged to, so the hostnames stay where they already are, in `ingress.host` and `.ingress.host`; both are read by both modes and neither is an Ingress switch. In Gateway mode the per-app shape renders one `HTTPRoute` per app (`---httproute`) and the shared shape exactly one (`--httproute`) with one rule per app — qBittorrent included, since its prefix-stripping rewrite is a per-rule filter there, and it is the one app contributing **two** rules: the rewrite plus a 301 from the bare, slashless prefix to the slashed form (Gitea #118). Without that redirect the slashless path is stripped to `/`, answered 200, and the page's relative links resolve to the root of the shared host — a different app. **The Ingress mode does not render it and cannot**: nginx offers only a `configuration-snippet` annotation, which is ignored unless the controller runs with `allow-snippet-annotations=true` (off by default since ingress-nginx 1.9, and it means accepting annotation risk level `Critical`), so the chart documents the snippet in `values.yaml` instead of shipping a no-op. It is the one place where the Gateway mode closes a gap rather than merely replacing machinery. It offers both shapes. By default each app carries its own `.ingress.enabled` plus a required `.ingress.host` (required only while that app's ingress is enabled and it is not on the shared domain) and renders its own Ingress object `---ingress` with its own TLS secret `tls----ingress`. Setting the global `ingress.host` switches every app without a host of its own to the shared-domain shape under `.ingress.path`; qBittorrent (no base path, needs a prefix-stripping rewrite) gets the extra object described above, and seerr as well as mediathekarr are pinned to `/` — mediathekarr because its UI links assets with root-absolute paths, which a rewrite cannot fix; with both of them on one shared host the duplicate-path guard fires and one of them needs a host of its own (its ingress is therefore off by default). `clusterIssuer`, `className` and `annotations` stay global defaults under `ingress.` and are overridable per app. ### 5. Security hardening diff --git a/ci/run-httproute-match-test.sh b/ci/run-httproute-match-test.sh index 8917e76..431a86f 100755 --- a/ci/run-httproute-match-test.sh +++ b/ci/run-httproute-match-test.sh @@ -22,6 +22,30 @@ # yields a route that resolves to no endpoint. ResolvedRefs goes False in # the route's STATUS, which only exists once a controller has looked at it # - so in CI, where no Gateway controller runs, nothing reports it at all. +# A PREFIX STRIPPED WITHOUT THE BARE PREFIX REDIRECTED (#118) - the third +# finding, and the one that is not about an object pointing nowhere but +# about an object pointing somewhere WRONG. A rule that matches +# PathPrefix "" (anything but "/") and carries a URLRewrite filter +# with ReplacePrefixMatch strips the prefix before the app sees the +# request. For the slashed form that is correct. For the BARE form - +# "", no trailing slash - the request becomes "/" and the app +# answers 200, while the browser's document URL keeps the prefixless form: +# every relative link in that page then resolves against the PARENT of +# that path, which is the host root. On a shared host the root belongs to +# a different application, so the app's own assets and XHRs are answered +# by its neighbour. Nothing in the chart, the render, the schema or the +# install reports any of it; the symptom is a half-working UI and a +# neighbour in the access log, which points nowhere useful. +# +# So: wherever a route strips a prefix, the SAME route must also carry a +# rule matching that path with type Exact and a RequestRedirect filter. +# Per HTTPRoute object, because Gateway-API matching precedence is a +# property of one route - a different route is a different hostname and +# says nothing about this one. Exact outranks every prefix match in that +# precedence, so the pairing is about existence and not about order. +# freesailarr is the first chart this applies to; bulwark is the second +# rewrite-target chart and meets the same check when group 3 of #114 +# lands, which is the point of checking it here rather than in one chart. # # And the install-test cannot close either gap, even with the real CRDs in the # cluster: schema validation accepts an empty parentRefs list and accepts a @@ -69,6 +93,19 @@ # added here"). # - A RULE WITHOUT backendRefs. Legal - a rule carrying only a # RequestRedirect filter needs no backend - so it is reported, not failed. +# The redirect rule the prefix check below demands is exactly such a rule, +# so a chart that satisfies that check also earns one "not checked" line +# here. Both are correct: the pairing is an assertion, the missing backend +# is an honest gap. +# - THAT ANY REDIRECT IS EVER SERVED. The prefix/redirect pairing proves the +# rendered OBJECT carries a RequestRedirect rule on the bare prefix. It +# does not prove that a 301 ever reaches a client, and it cannot: no +# Gateway controller runs in CI, nothing evaluates matching precedence, +# and no route reaches "Accepted: True" - so whether the implementation +# honours the statusCode, or declines it and fails the whole route, is +# outside what this test can see. What is proven is that the rule exists, +# that it matches the right path, and that the chart did not silently stop +# rendering it. # # AND THE ANTI-VACUUM GUARDS, because this test can be green over nothing. The # convention ships with httpRoute.enabled: false, so a chart renders no route @@ -344,6 +381,76 @@ for doc in routes: lines.append("OK|%s: %s resolves to Service %s:%s, created by the same " "render" % (chart, where, target, port)) + # 3. A stripped prefix needs the BARE prefix redirected, in THIS route + # (#118). Collected over all rules of this one object first, because the + # pairing is a property of the route and not of a rule: the Gateway API + # ranks an Exact path match above every prefix match, so the redirect + # rule may sit anywhere in the list and still win. + stripped = {} # prefix -> index of the rule that strips it + redirected = {} # path -> index of the rule that redirects it exactly + for i, rule in enumerate(rules): + if not isinstance(rule, dict): + continue + filter_types = set() + strips_prefix = False + filters = rule.get("filters") + for f in (filters if isinstance(filters, list) else []): + if not isinstance(f, dict): + continue + ftype = str(f.get("type") or "") + filter_types.add(ftype) + if ftype != "URLRewrite": + continue + rewrite = f.get("urlRewrite") + rewrite = rewrite if isinstance(rewrite, dict) else {} + rpath = rewrite.get("path") + rpath = rpath if isinstance(rpath, dict) else {} + if str(rpath.get("type") or "") == "ReplacePrefixMatch": + strips_prefix = True + matches = rule.get("matches") + for m in (matches if isinstance(matches, list) else []): + if not isinstance(m, dict): + continue + mpath = m.get("path") + mpath = mpath if isinstance(mpath, dict) else {} + # The API's own defaults for HTTPPathMatch. + ptype = str(mpath.get("type") or "PathPrefix") + value = str(mpath.get("value") or "/") + if strips_prefix and ptype == "PathPrefix" and value != "/": + stripped.setdefault(value, i) + if ptype == "Exact" and "RequestRedirect" in filter_types: + redirected.setdefault(value, i) + + for prefix in sorted(stripped): + if prefix in redirected: + lines.append("OK|%s: %s strips the prefix %r (rules[%d]) and " + "redirects the bare, slashless %r to the slashed form " + "(rules[%d]) - so the prefixless entry point cannot be " + "answered under a URL whose relative links leave the app" + % (chart, route, prefix, stripped[prefix], prefix, + redirected[prefix])) + else: + lines.append("FAIL|%s: %s rules[%d] strips the prefix %r with a " + "ReplacePrefixMatch rewrite, but NO rule of this route " + "redirects the bare prefix: there is no match of type " + "Exact on %r carrying a RequestRedirect filter. A " + "request for %r WITHOUT the trailing slash is then " + "rewritten to \"/\" and the app answers it 200, so " + "nothing anywhere reports a problem - but the browser's " + "document URL stays prefixless, every relative link in " + "that page resolves against the host ROOT, and on a " + "shared host the root is a DIFFERENT application, which " + "then answers the app's own assets and XHRs. The fix is " + "a rule matching %r exactly with a RequestRedirect to " + "'%s/' and statusCode 301 written out (302 is the API's " + "default, and 308 is Extended - an implementation that " + "declines it sets Accepted=False on the WHOLE route, " + "taking every other app on it down). Exact outranks " + "every prefix match, so where the rule sits in the list " + "does not matter. Gitea #118" + % (chart, route, stripped[prefix], prefix, prefix, + prefix, prefix, prefix)) + for line in lines: print(line) PY @@ -420,7 +527,11 @@ SUMMARY="${GITHUB_STEP_SUMMARY:-/dev/null}" echo "\`backendRefs\` entry naming a Service or a port that does not exist resolves to" echo "nothing — both are schema-valid, so neither the real CRDs in the install-test nor" echo "any render error catches them, and the only report would be a route status that no" - echo "controller writes in CI." + echo "controller writes in CI. A third finding is not about pointing nowhere but about" + echo "pointing somewhere wrong: a rule that strips a prefix with \`ReplacePrefixMatch\`" + echo "while the same route carries no \`Exact\` match on that prefix with a" + echo "\`RequestRedirect\` filter leaves the slashless form answered 200 under a URL whose" + echo "relative links resolve to the host root — a neighbouring app (Gitea #118)." echo echo "| Result | Count | Charts |" echo "|---|---|---|" diff --git a/freesailarr/Chart.yaml b/freesailarr/Chart.yaml index 2860e47..e6c05b8 100644 --- a/freesailarr/Chart.yaml +++ b/freesailarr/Chart.yaml @@ -8,7 +8,7 @@ description: A Helm chart to deploy a Wireguard-tunneled media automation stack type: application -version: 3.2.0+up1.0.0 +version: 3.3.0+up1.0.0 # The chart's own version marker, freely chosen and tied to no upstream. The # stack bundles ten independently versioned projects (nine of them on by # default), so there is no single "app version" to track here. diff --git a/freesailarr/templates/NOTES.txt b/freesailarr/templates/NOTES.txt index 009c5bc..bbf4908 100644 --- a/freesailarr/templates/NOTES.txt +++ b/freesailarr/templates/NOTES.txt @@ -12,6 +12,14 @@ {{- range $shared }}{{ $sharedNames = append $sharedNames .name }}{{ end -}} {{- $httpRoute := .Values.httpRoute | default dict -}} {{- $published := include "freesailarr.published" . -}} +{{- /* NOTHING IN CI READS THIS FILE, so every claim below is unverified by any + pipeline stage. `helm template` does not emit NOTES.txt at all, and no + workflow or script under ci/ reads it (measured for Gitea #118: the + three hits for "NOTES" under ci/ are comments in fixtures, not reads). + The render tests therefore cannot catch a statement here that has gone + stale, and the install-test never sees it either — it is prose checked + by review only. Treat an edit to it with the same care as the templates + it describes, because nothing else will. */ -}} freesailarr — release {{ .Release.Name }} in namespace {{ .Release.Namespace }}. {{ if .Values.mediathekarr.enabled }}Ten{{ else }}Nine{{ end }} containers in ONE pod, sharing gluetun's network namespace. @@ -87,17 +95,39 @@ ACCESS same host, because nginx applies rewrite-target per object and the other apps must not be rewritten. {{- else }} In Gateway mode that second object is gone: a - URLRewrite filter belongs to a RULE, so qBittorrent is one rule of + URLRewrite filter belongs to a RULE, so qBittorrent is a rule of {{ $name }}-httproute with the filter on it and its neighbours are untouched by construction — no extra object, no regex path. {{- end }} - Two things here look exactly - like a login failure and send you looking in the wrong place: its session - cookie is marked "Secure" (over plain HTTP the browser never sends it back - and every click bounces to the login form), and it bans the client IP for a - while after a few failed attempts. If it does not work at all, set - qbittorrent.ingress.host and it is back on a host of its own - no chart - change needed. + + OPEN IT WITH THE TRAILING SLASH: + + https://{{ $sharedHost }}{{ include "freesailarr.appPath" (dict "root" $ "app" "qbittorrent") }}/ + +{{ if .Values.ingress.enabled }} Without the slash the prefix is stripped to "/", qBittorrent answers 200, + and every RELATIVE link in that page resolves against the ROOT of this + host — which is {{ if has "seerr" $sharedNames }}seerr{{ else }}whatever serves "/" here{{ end }}, not qBittorrent. The login form then looks + unstyled and does nothing while another application answers its requests. + This chart does NOT render that redirect in Ingress mode: nginx offers only + a configuration-snippet annotation for it, and snippets are ignored unless + the controller runs with allow-snippet-annotations=true, which has been off + by default since ingress-nginx 1.9 and means accepting annotation risk + level "Critical". The working snippet, and its price, are in values.yaml at + qbittorrent.ingress (Gitea #118). + +{{ else }} The bare, slashless path is redirected 301 to the slashed form by a rule of + {{ $name }}-httproute — an Exact path match, which the Gateway API ranks + above every prefix match, so it wins wherever it sits in the list. Without + it qBittorrent would answer 200 under a URL whose relative links resolve to + the root of this host, which is a different application (Gitea #118). This + is the one thing the Gateway mode does here that the Ingress mode cannot. + +{{ end }} Two further things look exactly like a login failure and send you + looking in the wrong place: its session cookie is marked "Secure" (over + plain HTTP the browser never sends it back and every click bounces to the + login form), and it bans the client IP for a while after a few failed + attempts. If it does not work at all, set qbittorrent.ingress.host and it + is back on a host of its own - no chart change needed. {{- end }} {{- if not (has "seerr" $sharedNames) }} diff --git a/freesailarr/templates/freesailarr.httproute.yaml b/freesailarr/templates/freesailarr.httproute.yaml index cab9fcf..a334da0 100644 --- a/freesailarr/templates/freesailarr.httproute.yaml +++ b/freesailarr/templates/freesailarr.httproute.yaml @@ -60,6 +60,16 @@ equivalent"; in this chart the equivalent is strictly less machinery than the original, and that is the reason this chart was worth converting rather than merely translating. +qBittorrent is the one app that contributes a SECOND rule, and it is not part +of the rewrite: a 301 from the bare, slashless prefix to the slashed form +(Gitea #118). Without it qBittorrent answers 200 under a URL whose relative +links resolve to the host root, which on the shared host is a DIFFERENT +application. In Ingress mode the chart does not render that redirect - it +cannot, as the only mechanism nginx offers for it is a snippet annotation that +most clusters refuse (values.yaml, qbittorrent.ingress, has the full price). +This is therefore the one place where the Gateway mode is not merely less +machinery but closes a gap the Ingress mode leaves open. + WHAT IS NOT BETTER: ANYTHING THAT WAS AN ANNOTATION AND IS NOT A FILTER. The per-app and global ingress.annotations have no counterpart at all here. This chart sets no body-size annotation of its own, so nothing it NEEDS is lost - @@ -136,14 +146,21 @@ switching every app off while leaving ingress.host set produces no object rather than an empty one without rules. qBittorrent is in here with the others, which is the difference to the Ingress: -its rule carries the URLRewrite filter that strips the prefix, and a filter in -the Gateway API belongs to a RULE. No second object, no repeated TLS secret, no -conditional cert-manager annotation, no regex path - see the header. +a filter in the Gateway API belongs to a RULE, so it contributes TWO rules of +its own and the apps around it stay untouched by construction - a PathPrefix +rule with the URLRewrite filter that strips the prefix, and an Exact rule on +the bare, slashless prefix that redirects 301 to the slashed form (Gitea #118; +the long reasoning is on that rule). No second object, no repeated TLS secret, +no conditional cert-manager annotation, no regex path - see the header. -The rules come sorted by path length descending, as the Ingress paths do. The -Gateway API specifies matching precedence by longest path match independent of -order (and so does nginx), so this is about a deterministic, reviewable -rendering, not about matching behaviour. +The rules come sorted by path length descending, as the Ingress paths do, and +qBittorrent's redirect rule follows the rule it corrects. Neither placement +carries meaning: the Gateway API specifies matching precedence by criteria +before order - an Exact match outranks every prefix match, and among prefix +matches the longest wins - and list order is consulted only when all of those +tie. So the ordering here is about a deterministic, reviewable rendering, not +about matching behaviour. That it is ALSO the order nginx would need is a +coincidence of this chart's history and not a property to rely on. */}} --- apiVersion: gateway.networking.k8s.io/v1 @@ -183,6 +200,87 @@ spec: backendRefs: - name: {{ $.Release.Name }}-{{ $.Chart.Name }}-{{ $app.name }}-service port: {{ int $app.port }} + {{- if and (eq $app.name "qbittorrent") (ne $app.path "/") }} + {{- /* THE BARE PREFIX, WITHOUT THE TRAILING SLASH (Gitea #118) + + A request for the bare qbittorrent.ingress.path - "/qbittorrent" + by default, no trailing slash - is matched by the PathPrefix rule + above and the URLRewrite turns it into "/". That is a valid + request to qBittorrent, which answers 200 with its login + page, so nothing anywhere reports a problem. But the browser's + document URL stays prefixless, and a relative link in that page + resolves against the PARENT of that path - which is the host root. + Every asset, every XHR and every form target therefore goes to "/", + and "/" on this shared host is SEERR. The symptom is qBittorrent's + login page that is unstyled and does nothing, while the requests it + makes are answered by a different application entirely - and seerr + answering a qBittorrent request is not a clue anyone follows + quickly. One redirect to the slashed form fixes all of it, because + relative links then resolve against the slashed prefix. + + PRECEDENCE IS NOT POSITION. This rule is matched by the Gateway API + on an Exact path, and the specification ranks "Exact path match" + ABOVE "Prefix path match with largest number of characters", with + list order consulted only when every earlier criterion ties + (gateway-api v1.6.3, apis/v1/httproute_types.go). So this rule wins + over the PathPrefix rule above AND over seerr's PathPrefix "/" no + matter where it is emitted. The length-descending sort of the rules + in this route therefore stays what the header says it is - a + deterministic, reviewable rendering - and emitting this rule + directly after the one it corrects is a readability choice, not a + correctness one. + + 301 AND NOT 308, deliberately, and the reason is not the HTTP + semantics but which values an implementation is obliged to support. + The Gateway API marks 301 and 302 as CORE and everything else - + 308 included - as EXTENDED. An implementation that does not support + the value given here must set the Accepted condition of the ROUTE to + status False with reason UnsupportedValue: not of this rule, of the + whole object. On the shared host that is one route carrying every + app under it, so a 308 that one implementation declines takes + prowlarr, radarr, sonarr and seerr down with qBittorrent - and it + does so on a route that is schema-valid, in a status field no + controller writes in CI. It would also contradict decision 1 of + Gitea #114, which is that this chart renders only the subset + that looks identical on nginx-gateway-fabric, Cilium, Envoy + Gateway and Istio, so that the cluster's implementation can be + swapped without touching the chart. + + What 308 would buy is method and body preservation, and this rule + does not need it: it fires on exactly one path, the prefixless entry + to the WebUI, which a browser reaches with GET. qBittorrent's API + lives under "/api/..." - slashed, longer, and therefore + served by the PathPrefix rule above, which this rule never sees. A + POST to the API is not redirected at all, so there is no body to + lose. + + statusCode MUST BE WRITTEN OUT: it defaults to 302 in the API, and + a temporary redirect here would be re-fetched by every client on + every visit forever, for a prefix that is a permanent property of + this release's layout. + + THE ONE ASYMMETRY TO THE nginx SNIPPET (values.yaml, qbittorrent): + HTTPRequestRedirectFilter has exactly five fields - scheme, + hostname, path, port, statusCode - and says nothing about the query + string, while ReplaceFullPath is specified as replacing the full + PATH. The nginx remedy in Ingress mode appends "$is_args$args" and + so carries the query over explicitly; here the query's fate is left + to the implementation and is not something this chart can state. + It costs nothing on the path this rule actually serves - the UI + entry point takes no query parameters - but it is a real difference + and not a translation. */}} + - matches: + - path: + type: Exact + value: {{ $app.path | quote }} + filters: + - type: RequestRedirect + requestRedirect: + path: + type: ReplaceFullPath + replaceFullPath: {{ printf "%s/" $app.path | quote }} + statusCode: 301 + {{- end }} {{- end }} {{- end }} {{- end }} diff --git a/freesailarr/values.yaml b/freesailarr/values.yaml index 12abb83..a604b2c 100644 --- a/freesailarr/values.yaml +++ b/freesailarr/values.yaml @@ -230,7 +230,12 @@ media: # prefix. (In GATEWAY MODE that second object is gone: the Gateway API's # URLRewrite filter is per RULE, not per object, so qBittorrent is one rule # in the shared route while its neighbours stay unrewritten. The traps -# below are the application's and apply in both modes.) Two traps from the +# below are the application's and apply in both modes.) CALL ITS PATH WITH +# A TRAILING SLASH: the bare /qbittorrent is stripped to "/", answered 200 +# by qBittorrent, and its relative links then resolve to the root of the +# shared host - which is seerr. Gateway mode renders a 301 for this itself; +# Ingress mode cannot, and the nginx remedy plus its price is written out +# at qbittorrent.ingress below (Gitea #118). Two further traps from the # upstream recipe, both of which look like a login failure and therefore # point in the wrong direction: # * qBittorrent marks its session cookie "Secure". Reaching the UI over @@ -277,6 +282,14 @@ ingress: # an annotation YOU added here is lost without a word; a body-size cap for # qBittorrent's torrent-file uploads is the likely candidate, and it then has # to be set at the Gateway or through its implementation's own policy object. + # + # ONE ANNOTATION IS WORTH ADDING HERE IN INGRESS MODE and the chart cannot + # ship it: the configuration-snippet that redirects the bare /qbittorrent to + # /qbittorrent/. It is written out, with its allow-snippet-annotations price, + # at qbittorrent.ingress below (Gitea #118). In Gateway mode it is neither + # needed nor possible - the chart renders that redirect as a rule of the + # route - so an operator who set it here does not lose a behaviour on + # migration, unlike the body-size case above. annotations: {} # Gateway-API alternative to the Ingress above (Gitea #114). Off by default, so @@ -421,6 +434,73 @@ qbittorrent: # path is only read in the shared-domain mode (see the ingress block above); # it defaults to / and is ignored while this app has a host of its own. # (Same block for prowlarr, radarr, sonarr, seerr and bazarr below.) + # + # CALL THIS PATH WITH A TRAILING SLASH IN INGRESS MODE (Gitea #118) + # + # qBittorrent has no base-path setting, so the prefix is stripped before the + # request reaches it. A request for the bare path - https://host/qbittorrent, + # no trailing slash - is therefore stripped to "/", which qBittorrent answers + # 200 with its login page. Nothing reports an error anywhere. But the + # browser's document URL stays prefixless, so every RELATIVE link in that + # page resolves against the host root - and "/" on the shared host is SEERR. + # The result is an unstyled login form whose assets, XHRs and form posts are + # all answered by a different application, which is a symptom that points + # nowhere useful. https://host/qbittorrent/ works correctly. The same applies + # to any custom path set here; it is a property of prefix stripping, not of + # the default value. + # + # IN GATEWAY MODE THE CHART RENDERS THE REDIRECT ITSELF. qBittorrent's rule + # in the shared HTTPRoute is joined by a second rule matching the bare path + # EXACTLY, whose RequestRedirect filter answers 301 to the slashed form. The + # Gateway API ranks an Exact match above every prefix match, so it wins over + # both qBittorrent's own PathPrefix rule and seerr's "/" regardless of + # ordering. 301 rather than 308 because only 301 and 302 are Core in that + # API - an Extended value an implementation declines makes it set + # Accepted=False on the WHOLE route, taking the neighbouring apps' routing + # with it. One asymmetry to the nginx remedy below: the Gateway API's + # redirect filter has no field for the query string (it replaces the full + # PATH), while the nginx snippet carries it over explicitly with + # $is_args$args. It costs nothing on the UI entry path, which takes no query + # parameters, but it is a real difference. + # + # IN INGRESS MODE THE CHART RENDERS NO REDIRECT, and that is a limitation, + # not an oversight. The remedy exists but cannot be shipped: put an nginx + # snippet into the global ingress.annotations (it is merged into both of this + # chart's Ingress objects on the shared host, which is what makes it work + # from there): + # + # ingress: + # annotations: + # nginx.ingress.kubernetes.io/configuration-snippet: | + # if ($uri = "/qbittorrent") { return 301 $scheme://$host/qbittorrent/$is_args$args; } + # + # ITS PRICE, which is why it is documented rather than added: snippet + # annotations are only honoured on a controller started with + # allow-snippet-annotations=true, and since ingress-nginx 1.9 that flag is + # OFF by default. Switching it on also means accepting + # annotations-risk-level: Critical, because a configuration snippet is + # arbitrary nginx configuration injected from a namespaced object. A chart + # that shipped this annotation would silently render a no-op on every cluster + # that keeps the default, which is worse than naming the gap. The decision + # belongs to whoever runs the ingress controller. + # + # AND A THIRD INGRESS OBJECT WITH pathType: Exact IS NOT A SUBSTITUTE here, + # which is the obvious-looking alternative and does not work. In + # ingress-nginx controller-v1.14.5, internal/ingress/controller/template/ + # template.go, buildLocation() returns a plain regex location + # (~* "^") in its enforceRegex branch and RETURNS THERE - before it + # ever reaches the PathTypeExact branch that would emit the = "" + # location nginx gives precedence to. And enforceRegex is computed once per + # nginx SERVER, not per location: rootfs/etc/nginx/template/nginx.tmpl + # evaluates enforceRegexModifier over every location of a hostname, and it + # returns true as soon as one of them needs a rewrite. qBittorrent's own + # object carries rewrite-target on this very hostname, so enforceRegex is + # true for the whole server and an Exact path would render as an ordinary + # regex location. The exact-match precedence the idea depends on never comes + # into being. The unsuitability follows from that short-circuit alone; what + # an unanchored regex location would then do to ORDERING - shadow the app, or + # loop the redirect, depending on emission order - is inferred from the same + # sources and has NOT been observed on a running nginx. ingress: enabled: true host: null