Merge cf8d6f3866 into b195d15b93

helm/litellm/templates/ingress.yaml

@@ -122,7 +122,9 @@
   at "/" Prefix would swallow the whole backend management API) instead of
   adding to it.
 */}}
-{{- $builtinPathKeys := list "/test|Exact" "/debug/memory/summary|Exact" "/|Prefix" -}}
+{{- $backendCatchAllPath := ternary "/*" "/" (eq $controller "alb") -}}
+{{- $backendCatchAllType := ternary "ImplementationSpecific" "Prefix" (eq $controller "alb") -}}
+{{- $builtinPathKeys := list "/test|Exact" "/debug/memory/summary|Exact" (printf "%s|%s" $backendCatchAllPath $backendCatchAllType) -}}
 {{- if $hasComponentHosts }}
 apiVersion: networking.k8s.io/v1
 kind: Ingress
@@ -265,10 +267,10 @@ spec:
             precedence over a default, and before the backend catch-all.
             Position only decides the match on controllers that honour manifest
             order: the AWS Load Balancer Controller (ingress.controller=alb)
-            sorts Exact paths first and Prefix paths longest-first, but keeps
-            ImplementationSpecific paths in manifest order, which is what the
-            /*.txt rule above already depends on. ingress-nginx ignores order
-            and serves the longest matching location.
+            sorts all Prefix paths ahead of ImplementationSpecific paths. Both
+            /*.txt and the ALB catch-all use ImplementationSpecific so their
+            manifest order is preserved. ingress-nginx serves the longest
+            matching location.
           */}}
           {{- range $idx, $extra := .Values.ingress.extraPaths }}
           {{- if not (kindIs "map" $extra) }}
@@ -311,7 +313,7 @@ spec:
           {{- end }}
           {{- $pathType := include "litellm.ingress.pathType" (dict "controller" $controller "path" $extra.path "pathType" $requestedPathType) }}
           {{- if eq $extra.path "/" }}
-          {{- fail (printf "ingress.extraPaths[%d]: path / is already routed in both directions, Exact to ui and Prefix to backend, so no pathType leaves a request for an entry here to capture" $idx) }}
+          {{- fail (printf "ingress.extraPaths[%d]: path / is already routed to ui, with unmatched paths routed to backend" $idx) }}
           {{- end }}
           {{- if has (printf "%s|%s" $extra.path $pathType) $builtinPathKeys }}
           {{- fail (printf "ingress.extraPaths[%d]: path %s with pathType %s is already routed by this chart, and a duplicate would take it over rather than add to it" $idx $extra.path $pathType) }}
@@ -325,8 +327,8 @@ spec:
                   {{- toYaml $port | nindent 18 }}
           {{- end }}
           # --- Catch-all → backend (management API: /key/*, /user/*, /team/*, ...) ---
-          - path: /
-            pathType: Prefix
+          - path: {{ $backendCatchAllPath | quote }}
+            pathType: {{ $backendCatchAllType }}
             backend:
               service:
                 name: {{ $backendName }}

helm/litellm/tests/ingress_controller_tests.yaml

@@ -38,6 +38,33 @@ tests:
                 name: RELEASE-NAME-litellm-ui
                 port:
                   number: 3000
+      - equal:
+          path: spec.rules[0].http.paths[-1]
+          value:
+            path: /*
+            pathType: ImplementationSpecific
+            backend:
+              service:
+                name: RELEASE-NAME-litellm-backend
+                port:
+                  number: 4001
+      - notContains:
+          path: spec.rules[0].http.paths
+          content:
+            path: /
+            pathType: Prefix
+          any: true
+
+  - it: rejects an extra wildcard that would take over the ALB backend catch-all
+    set:
+      ingress.enabled: true
+      ingress.extraPaths:
+        - path: /*
+          pathType: ImplementationSpecific
+          service: ui
+    asserts:
+      - failedTemplate:
+          errorMessage: "ingress.extraPaths[0]: path /* with pathType ImplementationSpecific is already routed by this chart, and a duplicate would take it over rather than add to it"
 
   - it: renders no dotted Exact or Prefix path for ingress-nginx, whose admission webhook rejects them
     set:

helm/litellm/tests/ingress_extra_paths_tests.yaml

@@ -11,8 +11,8 @@ tests:
       - equal:
           path: spec.rules[0].http.paths[-1]
           value:
-            path: /
-            pathType: Prefix
+            path: /*
+            pathType: ImplementationSpecific
             backend:
               service:
                 name: RELEASE-NAME-litellm-backend
@@ -48,8 +48,8 @@ tests:
       - equal:
           path: spec.rules[0].http.paths[-1]
           value:
-            path: /
-            pathType: Prefix
+            path: /*
+            pathType: ImplementationSpecific
             backend:
               service:
                 name: RELEASE-NAME-litellm-backend
@@ -218,7 +218,7 @@ tests:
           service: gateway
     asserts:
       - failedTemplate:
-          errorMessage: "ingress.extraPaths[0]: path / is already routed in both directions, Exact to ui and Prefix to backend, so no pathType leaves a request for an entry here to capture"
+          errorMessage: "ingress.extraPaths[0]: path / is already routed to ui, with unmatched paths routed to backend"
 
   - it: rejects a root entry that would take over the UI root
     set:
@@ -229,7 +229,7 @@ tests:
           service: gateway
     asserts:
       - failedTemplate:
-          errorMessage: "ingress.extraPaths[0]: path / is already routed in both directions, Exact to ui and Prefix to backend, so no pathType leaves a request for an entry here to capture"
+          errorMessage: "ingress.extraPaths[0]: path / is already routed to ui, with unmatched paths routed to backend"
 
   # A root ImplementationSpecific entry duplicates no built-in pair, so the
   # duplicate check alone would admit it. It is still dead: the built-in
@@ -244,7 +244,7 @@ tests:
           service: gateway
     asserts:
       - failedTemplate:
-          errorMessage: "ingress.extraPaths[0]: path / is already routed in both directions, Exact to ui and Prefix to backend, so no pathType leaves a request for an entry here to capture"
+          errorMessage: "ingress.extraPaths[0]: path / is already routed to ui, with unmatched paths routed to backend"
 
   - it: rejects an entry that would take over a UI prefix
     set:

tests/integration/database/image/test_ui_image_serves_offline.py

@@ -23,6 +23,7 @@ import subprocess
 import time
 import uuid
 from collections.abc import Iterator
+from typing import Final
 
 import pytest
 
@@ -86,11 +87,13 @@ def _is_running(container: str) -> bool:
     )
 
 
-def _probe(network: str, container: str, path: str) -> "subprocess.CompletedProcess[str]":
+def _probe(
+    network: str, container: str, path: str, write_out: str = "%{http_code}"
+) -> "subprocess.CompletedProcess[str]":
     return _docker(
         "run", "--rm", "--network", network, CURL_IMAGE,
         "--silent", "--show-error", "--max-time", "10",
-        "--output", "/dev/null", "--write-out", "%{http_code}",
+        "--output", "/dev/null", "--write-out", write_out,
         f"http://{container}:{UI_PORT}{path}",
         check=False,
     )
@@ -124,9 +127,19 @@ def test_ui_serves_as_arbitrary_uid_read_only(ui_container: tuple[str, str]) ->
         f"{ARBITRARY_UID}.\n{_container_logs(container)}"
     )
 
-    for path in ("/", "/ui", "/ui/login"):
+    for path in ("/ui", "/ui/", "/ui/login"):
         page = _probe(network, container, path)
         assert page.stdout.strip() == "200", (
             f"GET {path} returned {page.stdout.strip()!r} as uid {ARBITRARY_UID}.\n"
             f"{_container_logs(container)}"
         )
+
+    root: Final = _probe(network, container, "/?page=logs&limit=10", "%{http_code} %header{location}")
+    assert root.stdout.strip() == "307 /ui/?page=logs&limit=10", root.stderr
+
+    for path in ("/index.txt", "/ui.txt", "/ui/index.txt", "/ui/home/index.txt", "/ui/login/index.txt"):
+        payload: Final = _probe(network, container, f"{path}?_rsc=regression", "%{http_code} %{content_type}")
+        assert payload.stdout.strip() == "200 text/plain", f"GET {path}: {payload.stdout!r}\n{payload.stderr}"
+
+    missing: Final = _probe(network, container, "/ui/missing-route/index.txt")
+    assert missing.stdout.strip() == "404", missing.stderr

ui/nginx.conf

@@ -91,6 +91,12 @@ http {
     # /ui/<page>.txt would otherwise be rewritten to HTML and break RSC
     # for nested routes. A genuinely missing payload must 404 (the
     # router degrades to a hard navigation); never fall back to HTML.
+    location = /ui.txt {
+      try_files /index.txt =404;
+    }
+    location ~ ^/ui/(.+\.txt)$ {
+      try_files /$1 =404;
+    }
     location ~ \.txt$ {
       try_files $uri =404;
     }
@@ -107,8 +113,10 @@ http {
       try_files /$1.html /$1/index.html /index.html =404;
     }
 
-    # `/` is handy for direct-debug port-forwards.
-    location = / { try_files /index.html =404; }
+    location = / {
+      absolute_redirect off;
+      return 307 /ui/$is_args$args;
+    }
 
     # Anything else (API calls etc.) returns 404 from the UI's
     # perspective. A reverse proxy in front of this image routes the