Repository navigation
fix: strip leading slash from the remote session wstunnel path prefix - #1257
Merged
SalimKayal merged 1 commit intoNov 11, 2026
Merged
Conversation
SalimKayal
force-pushed
the
salimkayal/fix/trim-leading-slash-on-remote-sessions
branch
from
November 11, 2026 11:13
bb994a8 to
b94e8e5
Compare
sgaist
previously approved these changes
Nov 11, 2026
leafty
reviewed
Nov 11, 2026
| // wstunnelPathPrefix returns the -P value for the remote wstunnel client. wstunnel prepends a "/" to | ||
| // it when building the upgrade request, so the prefix must not carry one itself. | ||
| func wstunnelPathPrefix(renkuBaseURLPath string) string { | ||
| return strings.Trim(renkuBaseURLPath, "/") + "/" + v1alpha1.TunnelIngressPathSuffix |
Member
There was a problem hiding this comment.
Why don't we use the path library here?
Collaborator
Author
There was a problem hiding this comment.
witched to path.Join + TrimPrefix: path.Join builds/cleans the URL path but keeps a leading slash, so the trim has to stay.
WSTUNNEL_PATH_PREFIX was built as RENKU_BASE_URL_PATH + "/__amalthea__/tunnel",
and RENKU_BASE_URL_PATH is "/sessions/<id>", so the prefix kept its leading
slash. wstunnel's client prepends another one when building the upgrade request
(format!("/{}/events")), so the HPC side actually asks for
"//sessions/<id>/__amalthea__/tunnel/events".
That works behind ingress-nginx only because nginx merges repeated slashes when
matching locations and still forwards the raw URI. Behind a proxy that matches
paths literally (agentgateway/Envoy among others) the request matches no session
route, falls through to the UI and returns 404, leaving the remote session
frontend unreachable: "failed to do websocket handshake with the server ...
Invalid status code: 404".
Trim both edges of the base path in a small helper so the emitted -P never
starts (or ends) with a slash, and cover it with a test. The remote scratch path
is unaffected: it goes through path.Join.
Reported against renku/amalthea-sessions 0.31.0 (3ac6997) deployed behind
agentgateway; the same code on a nginx-based deployment emits the same doubled
path and is only masked by nginx.
SalimKayal
force-pushed
the
salimkayal/fix/trim-leading-slash-on-remote-sessions
branch
from
November 11, 2026 14:45
b94e8e5 to
7a85f51
Compare
leafty
approved these changes
Nov 11, 2026
sgaist
approved these changes
Nov 11, 2026
SalimKayal
deleted the
salimkayal/fix/trim-leading-slash-on-remote-sessions
branch
November 11, 2026 20:20
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #1256: the remote-session wstunnel path prefix kept the leading slash from
RENKU_BASE_URL_PATH, so the HPC client requested//sessions/<id>/__amalthea__/tunnel/events.nginx tolerates that; a proxy that matches paths literally (agentgateway/Envoy) does not, and
remote sessions 404. This trims both edges of the base path when building
-P.Change
internal/remote/firecrest/controller.go—WSTUNNEL_PATH_PREFIXis built by a newwstunnelPathPrefix()helper that trims/from both ends ofrenkuBaseURLPath(
env["WSTUNNEL_PATH_PREFIX"] = wstunnelPathPrefix(renkuBaseURLPath)).internal/remote/firecrest/controller_test.go—TestWstunnelPathPrefixcovers leading slash,no leading slash, trailing slash and doubled leading slash; all must yield
sessions/<id>/__amalthea__/tunnel.No change in
internal/remote/runai: it forwardsRENKU_BASE_URL_PATHinto the workspace spec andhas no tunnel yet. The remote scratch session path goes through
path.Join, which is alreadyslash-tolerant.
Verification
go test ./internal/remote/firecrest/ -run 'TestWstunnelPathPrefix|TestRenderSessionScript|TestStreamsToFetch|TestBranchRegExp'go build ./...,go vet ./internal/remote/firecrest/-Pissessions/<id>/__amalthea__/tunnel, i.e. the request path becomes/sessions/<id>/__amalthea__/tunnel/events— exactly the path amalthea's own Ingress exposes.Deployments behind nginx accept both forms, so this is not a behaviour change for them.