Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions .env.dev
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ POD_NAME="dev-router-pod"

# Public URLs
BASEURL="dev.vendeuvre.lan"
BASE_URL="dev.vendeuvre.lan"
PUBLIC_BASE_URL="https://dev.vendeuvre.lan/llm-routing"
PUBLIC_BASE_URL="https://dev.vendeuvre.lan"
PROXY_BASE_URL="https://litellm.dev.vendeuvre.lan"
NEXTAUTH_URL="https://langfuse.dev.vendeuvre.lan"

# Dev port assignments (+10 offset from production)
ROUTER_PORT="5010"
Expand Down
22 changes: 8 additions & 14 deletions router/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -3546,21 +3546,15 @@ def resolve_external_urls(request: Request) -> tuple[str, str, str]:
is_valid_external = external_host == domain or external_host.endswith("." + domain)
is_valid_base = request.base_url.hostname == domain or (request.base_url.hostname or "").endswith("." + domain)

if is_valid_external:
# Centralized base URL path under subdomain/reverse proxy
if is_valid_external or is_valid_base:
host_val = external_host if is_valid_external else (request.base_url.hostname or "vendeuvre.lan")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (bug_risk): Hardcoded "vendeuvre.lan" fallback may be surprising and environment-specific.

This hardcoded fallback in host_val risks misrouting in non-vendeuvre environments if request.base_url.hostname is unset or misconfigured. Consider deriving the fallback from configuration (e.g., ROUTING_DOMAIN), or failing fast/logging when neither external_host nor a valid base hostname is available, instead of silently defaulting to a specific LAN domain.

Suggested implementation:

    if is_valid_external or is_valid_base:
        # Fallback to configured `domain` instead of an environment-specific hardcoded hostname
        host_val = external_host if is_valid_external else (request.base_url.hostname or domain)
        host_base = host_val.replace("dashboard.", "")

If your routing domain is configurable via a dedicated setting (e.g. ROUTING_DOMAIN), you may want to:

  1. Initialize domain from that configuration, if it is not already.
  2. Optionally add logging where domain is determined to surface misconfigurations (e.g. when request.base_url.hostname is unexpectedly None and the code falls back to domain).

host_base = host_val.replace("dashboard.", "")
if host_base.startswith("litellm.") or host_base.startswith("langfuse.") or host_base.startswith("llama."):
host_base = re.sub(r"^(litellm|langfuse|llama)\.", "", host_base)
return (
f"{external_scheme}://{external_netloc}/llm-routing/langfuse",
f"{external_scheme}://{external_netloc}/llm-routing/litellm/ui",
f"{external_scheme}://{external_netloc}/llm-routing/llama/"
)
elif is_valid_base:
parsed_netloc = urlparse(f"{external_scheme}://{request.url.netloc}")
netloc = request.url.netloc if parsed_netloc.hostname else "localhost"
base = f"{external_scheme}://{netloc}"
return (
f"{base}/llm-routing/langfuse",
f"{base}/llm-routing/litellm/ui",
f"{base}/llm-routing/llama/"
f"{external_scheme}://langfuse.{host_base}",
f"{external_scheme}://litellm.{host_base}/ui/",
f"{external_scheme}://llama.{host_base}/"
Comment on lines +3549 to +3557

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Update the resolver tests to the new subdomain contract.

router/tests/test_resolve_external_urls.py:41-51 still expects /llm-routing/... paths. With this implementation, the expected values for https://app.vendeuvre.lan are https://langfuse.app.vendeuvre.lan, https://litellm.app.vendeuvre.lan/ui/, and https://llama.app.vendeuvre.lan/.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@router/main.py` around lines 3549 - 3557, The resolver tests in
test_resolve_external_urls.py should be updated to match the new subdomain
contract implemented by the external URL resolver: for
https://app.vendeuvre.lan, expect Langfuse at
https://langfuse.app.vendeuvre.lan, LiteLLM at
https://litellm.app.vendeuvre.lan/ui/, and Llama at
https://llama.app.vendeuvre.lan/. Replace the outdated /llm-routing/ path
expectations without changing resolver behavior.

Comment on lines 3554 to +3557

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Preserve an explicit public port when constructing subdomain URLs.

external_host is hostname-only, so a base such as https://app.vendeuvre.lan:8443 produces links on default port 443. start-stack.sh preserves the port through parsed_pub.netloc, creating inconsistent rendered and runtime URLs.

Carry the explicit port from the configured/request netloc into all three returned URLs.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@router/main.py` around lines 3554 - 3557, Update the URL construction in the
surrounding function to retain the explicit port from the configured/request
netloc, rather than using the hostname-only external_host base. Build the
Langfuse, LiteLLM, and Llama URLs from a shared host-and-port base so all three
preserve ports such as :8443 while retaining their existing paths.

)
else:
# Local development fallback: derive schemes, ports, and paths dynamically from configuration constants
Expand Down
19 changes: 15 additions & 4 deletions start-stack.sh
Original file line number Diff line number Diff line change
Expand Up @@ -698,13 +698,24 @@ text = text.replace("MINIO_PASSWORD_PLACEHOLDER", yaml_scalar(os.environ["MINIO_
text = text.replace("LANGFUSE_INIT_USER_PASSWORD_PLACEHOLDER", yaml_scalar(os.environ["LANGFUSE_INIT_USER_PASSWORD"]))
text = text.replace("REDIS_AUTH_PLACEHOLDER", yaml_scalar(os.environ["REDIS_AUTH"]))
text = text.replace("CLICKHOUSE_PASSWORD_PLACEHOLDER", yaml_scalar(os.environ["CLICKHOUSE_PASSWORD"]))
# Derive PROXY_BASE_URL from PUBLIC_BASE_URL
# Derive PROXY_BASE_URL and NEXTAUTH_URL (favoring explicit env or subdomain formatting)
public_base_url = os.environ["PUBLIC_BASE_URL"].rstrip("/")
proxy_base_url = f"{public_base_url}/litellm"
parsed_pub = urllib.parse.urlparse(public_base_url)
scheme = parsed_pub.scheme or "https"
host = parsed_pub.netloc or parsed_pub.path.split("/")[0] or "vendeuvre.lan"

if "PROXY_BASE_URL" in os.environ:
proxy_base_url = os.environ["PROXY_BASE_URL"]
else:
proxy_base_url = f"{scheme}://litellm.{host}"

if "NEXTAUTH_URL" in os.environ:
nextauth_url = os.environ["NEXTAUTH_URL"]
else:
nextauth_url = f"{scheme}://langfuse.{host}"

text = text.replace("PROXY_BASE_URL_PLACEHOLDER", yaml_scalar(proxy_base_url))
text = text.replace("PUBLIC_BASE_URL_PLACEHOLDER", yaml_scalar(os.environ["PUBLIC_BASE_URL"]))
# Derive NEXTAUTH_URL from PUBLIC_BASE_URL (Langfuse needs the public URL for OAuth redirects)
nextauth_url = f"{public_base_url}/langfuse"
text = text.replace("NEXTAUTH_URL_PLACEHOLDER", yaml_scalar(nextauth_url))
text = text.replace("ROUTING_DOMAIN_PLACEHOLDER", yaml_scalar(os.environ["ROUTING_DOMAIN"]))
# Raw replacements (no quoting — used for pod name, data root, and integer port values)
Expand Down
Loading