nginx: route /switch and /status to selkies supervisor (dual-mode)#154
nginx: route /switch and /status to selkies supervisor (dual-mode)#154DL6ER wants to merge 4 commits intolinuxserver:masterfrom
Conversation
When `SELKIES_ENABLE_DUAL_MODE=true` selkies starts a small aiohttp supervisor on `localhost:8082` exposing two endpoints: POST /switch - switch the active streaming mode (websockets <-> webrtc) GET /status - report which mode is currently running The selkies-dashboard sidebar uses these to drive the "Streaming Mode" selector. Without nginx routes the browser only sees an HTML 404 from nginx and the dashboard's mode selector silently disappears (the related dual-mode dropdown is gated on a successful /status response in some downstream forks). This adds: - a new init-nginx env var `SELKIES_SUPERVISOR_PORT` (default 8082) and a sed substitution for `SUPERVISOR_PORT` in default.conf, so the port becomes configurable; - a regex location `^/(switch|status)$` that proxies both endpoints to 127.0.0.1:SUPERVISOR_PORT in both server blocks. Note the existing `CWS` default (`CUSTOM_WS_PORT=8082`) conflicts with the supervisor's hardcoded port in selkies (also 8082). Users enabling dual mode currently need to set `CUSTOM_WS_PORT=8081`. Tracking the supervisor-port-default question in selkies-project/selkies separately; this PR only adds the nginx-side plumbing so the routes exist as soon as selkies grows a configurable port.
| CPORT="${CUSTOM_PORT:-3000}" | ||
| CHPORT="${CUSTOM_HTTPS_PORT:-3001}" | ||
| CWS="${CUSTOM_WS_PORT:-8082}" | ||
| SUP="${SELKIES_SUPERVISOR_PORT:-8082}" |
There was a problem hiding this comment.
follow-up patches default to 8084 once selkies-project/selkies#237 lands, converting this to a draft right now
There was a problem hiding this comment.
Pull request overview
Adds nginx routing for Selkies dual-mode “supervisor” API endpoints so the dashboard can switch streaming modes and query run status when SELKIES_ENABLE_DUAL_MODE=true.
Changes:
- Add
SELKIES_SUPERVISOR_PORT(default8082) to the nginx init script and substitute it into the templated nginx config. - Route
POST /switchandGET /statusto the supervisor API in both HTTP and HTTPS server blocks.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| root/etc/s6-overlay/s6-rc.d/init-nginx/run | Adds env var + sed substitution to inject supervisor port into nginx config. |
| root/defaults/default.conf | Adds nginx locations to proxy /switch and /status to the supervisor API. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Copilot caught that the new /switch + /status location forwards only Host, while every other proxied location in this config (/devmode, SUBFOLDERwebsocket) also passes X-Real-IP, X-Forwarded-For, and X-Forwarded-Proto. The supervisor logs the requesting peer for the mode-switch audit trail; without these headers everything looks like 127.0.0.1. Mirror the existing forwarding-header set in both server blocks (no timeouts added — these are short HTTP requests, not WS).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…trips
Three follow-up issues from review:
1. The `# Selkies dual-mode supervisor ...` comment block above each
location would be destroyed by the existing
`if [ ! -z ${PASSWORD+x} ]; then sed -i 's/#//g' ...; fi` in
init-nginx/run, which strips ALL `#` characters from the rendered
config to uncomment the auth_basic lines. After the strip, our
comment lines would parse as nginx directives and break startup.
Drop the explanatory comments — the PR body already documents the
intent, and nginx has no out-of-band comment syntax that survives
the strip.
2. The regex `^/(switch|status)$` matched only at the document root,
so a non-default SUBFOLDER deployment (e.g. SUBFOLDER=/foo/) would
miss `/foo/switch` entirely. Switch to `^SUBFOLDER(switch|status)$`
(rendered to `^/(switch|status)$` for the default `/` case) and
`proxy_pass http://127.0.0.1:SUPERVISOR_PORT/$1` so the selkies
supervisor — which serves at the root — sees the original
`/switch` or `/status` regardless of nginx-side prefix.
3. Add an inline doc comment in init-nginx/run explaining what
SELKIES_SUPERVISOR_PORT does (nginx proxy target only, mirrors the
selkies-side port when SELKIES_ENABLE_DUAL_MODE is on).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Follow-up to review feedback: the previous attempt used a regex location with `$1` capture and SUBFOLDER substituted into the regex itself. Two real risks with that approach: 1. SUBFOLDER is treated as literal text by sed but as regex once substituted, so a subfolder containing regex metacharacters (e.g. `.`) would change match semantics. 2. proxy_pass with a variable in the URI ($1) switches nginx out of prefix-strip mode and drops the query string unless explicitly appended via `\$is_args\$args`. /switch and /status do not use query strings today, but the trap is real. Replace the single regex location with two `location =` exact-match blocks per server (switch and status), each with a static proxy_pass URL. SUBFOLDER stays a pure prefix substitution, no regex involved; the static path means full URI rewriting works with all request shapes; `=` is the highest-priority match in nginx so dispatch is also fastest. Pattern matches the existing SUBFOLDERwebsocket / SUBFOLDERwebsockets / SUBFOLDERfiles convention in this same file.
# Conflicts: # root/defaults/default.conf
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Port of the selkies dual-mode supervisor API (/switch and /status). | ||
| # Only consulted when SELKIES_ENABLE_DUAL_MODE=true on the selkies side; | ||
| # this value drives nginx routing only, not the selkies bind port. | ||
| SUP="${SELKIES_SUPERVISOR_PORT:-8082}" |
There was a problem hiding this comment.
The shell variable name SUP is not descriptive and makes it harder to follow what’s being substituted into the nginx template. Consider renaming it to something explicit like SUPERVISOR_PORT (or SELKIES_SUPERVISOR_PORT_NGINX) and use that in the sed replacement for readability/maintainability.
| # Only consulted when SELKIES_ENABLE_DUAL_MODE=true on the selkies side; | ||
| # this value drives nginx routing only, not the selkies bind port. | ||
| SUP="${SELKIES_SUPERVISOR_PORT:-8082}" |
There was a problem hiding this comment.
SELKIES_SUPERVISOR_PORT is currently a footgun: selkies hardcodes the supervisor bind port to 8082, so setting this env var to any other value will silently break /switch and /status routing (nginx will proxy to a port where nothing is listening). Consider either hardcoding 8082 in nginx until selkies supports configuring it, or explicitly validating the value and warning/ignoring anything other than 8082.
| # Only consulted when SELKIES_ENABLE_DUAL_MODE=true on the selkies side; | |
| # this value drives nginx routing only, not the selkies bind port. | |
| SUP="${SELKIES_SUPERVISOR_PORT:-8082}" | |
| # Selkies currently binds the supervisor API to 8082 only; this script | |
| # configures nginx routing and must not allow unsupported values here. | |
| if [ -n "${SELKIES_SUPERVISOR_PORT+x}" ] && [ "${SELKIES_SUPERVISOR_PORT}" != "8082" ]; then | |
| printf 'WARNING: Ignoring unsupported SELKIES_SUPERVISOR_PORT=%s; selkies currently binds the supervisor API to 8082 only.\n' "${SELKIES_SUPERVISOR_PORT}" >&2 | |
| fi | |
| SUP="8082" |
| location = SUBFOLDERswitch { | ||
| proxy_set_header Host $host; | ||
| proxy_set_header X-Real-IP $remote_addr; | ||
| proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; | ||
| proxy_set_header X-Forwarded-Proto $scheme; | ||
| proxy_http_version 1.1; | ||
| proxy_buffering off; | ||
| proxy_pass http://127.0.0.1:SUPERVISOR_PORT/switch; | ||
| } | ||
| location = SUBFOLDERstatus { |
There was a problem hiding this comment.
The PR description says a single regex location (location ~ ^/(switch|status)$) was added, but the nginx template actually adds two exact-match locations (location = ...switch and location = ...status). Please update the PR description to match the implementation (or change the config to the documented regex approach) so future maintainers aren’t misled.
|
I am a bot, here are the test results for this PR:
|
|
I am a bot, here are the test results for this PR:
|
|
I am a bot, here are the test results for this PR:
|
|
I am a bot, here are the test results for this PR:
|
|
Just a heads up the dual mode is not something planned for a bit. (this might sit for a couple weeks) The current plan is to get a Selkies image from the Selkies namespace and have it replace their current glx/egl desktop images. Once that has been distributed and tested out we will be layering on WebRTC functionality on the LSIO side using optional environment variables and port forwards. |
Description:
Add nginx routes for the dual-mode supervisor endpoints exposed by selkies when
SELKIES_ENABLE_DUAL_MODE=true:POST /switch— switch active streaming mode (websockets ↔ webrtc)GET /status— report current mode + run statePlumbing changes:
SELKIES_SUPERVISOR_PORTenv var ininit-nginx/run(default8082, matching the current selkies hardcoded value)SUPERVISOR_PORTsed substitution indefault.conflocation ~ ^/(switch|status)$added to both server blocksBenefits of this PR and context:
The selkies-dashboard sidebar uses
POST /switchto drive its "Streaming Mode" selector. Without nginx routing, the browser sees an HTML 404 from nginx and the dashboard cannot switch mode at runtime (in some downstream forks the dropdown is hidden until /status responds).Code reference (
selkies-project/selkiessrc/selkies/__main__.py):How Has This Been Tested?
Built locally on top of
master(debian-trixie), composed andebian-xfcedownstream image, ran with:Validated:
Browser end-to-end: dashboard sidebar mode selector becomes interactive, switching modes triggers re-connection in the new mode without container restart.
Source / References:
selkies-project/selkiessrc/selkies/__main__.py, thecreate_api_serverfunction and theapi_taskcreation inrun().Note: the selkies side hardcodes the supervisor port to
8082. SettingSELKIES_SUPERVISOR_PORThere only affects nginx; until selkies grows a matching env var the value must remain8082. There is also an unrelated default port collision (CUSTOM_WS_PORT=8082matches the hardcoded supervisor port), so dual-mode users currently need to setCUSTOM_WS_PORT=8081. I field PR against selkies-project/selkies to address both the port-default and an env-var for the supervisor: