fix(freesailarr): redirect the bare /qbittorrent prefix in Gateway mode
Validate Helm Charts / Detect changed charts (pull_request) Successful in 5s
Validate Helm Charts / Resource quantities (pull_request) Successful in 15s
Validate Helm Charts / Selector match (pull_request) Successful in 14s
Validate Helm Charts / HTTPRoute match (pull_request) Successful in 14s
Validate Helm Charts / Build freesailarr (pull_request) Successful in 8s
Validate Helm Charts / Trivy config freesailarr (pull_request) Successful in 22s
Install-test / Install-test freesailarr (pull_request) Successful in 4m19s
Validate Helm Charts / Install-test (pull_request) Successful in 4m19s
Publish Helm Chart with patched Application Version / publish-helm-chart (push) Successful in 12s
Publish Freesailarr Helm Chart / Publish Freesailarr Helm Chart (push) Successful in 12s
SonarQube Analysis / SonarQube Analysis (push) Successful in 48s
Validate Helm Charts / Detect changed charts (pull_request) Successful in 5s
Validate Helm Charts / Resource quantities (pull_request) Successful in 15s
Validate Helm Charts / Selector match (pull_request) Successful in 14s
Validate Helm Charts / HTTPRoute match (pull_request) Successful in 14s
Validate Helm Charts / Build freesailarr (pull_request) Successful in 8s
Validate Helm Charts / Trivy config freesailarr (pull_request) Successful in 22s
Install-test / Install-test freesailarr (pull_request) Successful in 4m19s
Validate Helm Charts / Install-test (pull_request) Successful in 4m19s
Publish Helm Chart with patched Application Version / publish-helm-chart (push) Successful in 12s
Publish Freesailarr Helm Chart / Publish Freesailarr Helm Chart (push) Successful in 12s
SonarQube Analysis / SonarQube Analysis (push) Successful in 48s
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
This commit was merged in pull request #119.
This commit is contained in:
6 files changed
+345
-23
No files matched your search
@@ -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 "<path>" (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 -
|
||||
# "<path>", 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 "|---|---|---|"
|
||||
|
||||
Reference in new issue
Block a user