From 8de79af65bdbe90f26a0bbcc20824e0512902851 Mon Sep 17 00:00:00 2001 From: Nadezhda Savchenko Date: Sat, 20 Jun 2026 10:49:58 +0300 Subject: [PATCH] Fixes --- server/pkg/auth.go | 23 +++++++++++++++++++++-- server/pkg/task.go | 40 ++++++++++++++++++++++++++++------------ server/pkg/task_test.go | 13 +++++++++++-- server/pkg/xds.go | 6 ++++++ server/pkg/xds_test.go | 2 ++ 5 files changed, 68 insertions(+), 16 deletions(-) diff --git a/server/pkg/auth.go b/server/pkg/auth.go index 70089d2..7124228 100644 --- a/server/pkg/auth.go +++ b/server/pkg/auth.go @@ -60,8 +60,12 @@ func (s *authServer) Check(ctx context.Context, req *authv3.CheckRequest) (*auth task, err := s.findTaskByRequest(host, headers) if err != nil { defaultMetrics.ObserveAuthFailure(authReasonTaskLookup, nil) - s.logger.Warnf("failed to find task during auth check: %s", err) - return deniedResponse, nil + // Not an authz failure: the task isn't in our tables (yet). This is normal right + // after an operation restart — the new operation must reach "running" and be + // picked up by the next discovery pass. Return 503 (transient) rather than 403, + // so the client retries instead of surfacing a scary "permission denied". + s.logger.Warnf("task not found during auth check (likely starting): %s", err) + return taskNotFoundResponse, nil } // skip auth for UI services for statics; currently it is the case for SPYT UI @@ -267,6 +271,21 @@ var ( }, }, } + // Returned when the requested task is not (yet) in the routing tables — typically a + // notebook/clique whose operation just restarted and hasn't been rediscovered. + // Transient, so 503 (not 403): the client should retry, and it isn't an authz failure. + taskNotFoundResponse = &authv3.CheckResponse{ + Status: &status.Status{ + Code: int32(codes.Unavailable), + Message: "task not found or starting", + }, + HttpResponse: &authv3.CheckResponse_DeniedResponse{ + DeniedResponse: &authv3.DeniedHttpResponse{ + Status: &typev3.HttpStatus{Code: typev3.StatusCode_ServiceUnavailable}, + Body: "task not found or starting, please retry", + }, + }, + } ) func taskHash(operationID, taskName, service string) string { diff --git a/server/pkg/task.go b/server/pkg/task.go index 929c839..ed34786 100644 --- a/server/pkg/task.go +++ b/server/pkg/task.go @@ -28,8 +28,15 @@ type Task struct { jobs []HostPort } +// taskName and service must stay hyphen-free: they are the last two segments of the +// alias subdomain (--) and parsing relies on that. var valueRegexp = regexp.MustCompile(`^[a-z0-9_]{1,30}$`) +// The alias may contain hyphens (strawberry aliases commonly do). Edge hyphens are +// disallowed so the resulting DNS label stays valid; tryParseAliasSubdomain recovers +// the alias by taking everything left of the trailing --. +var aliasRegexp = regexp.MustCompile(`^[a-z0-9_]([a-z0-9_-]{0,28}[a-z0-9_])?$`) + // Identifies task, for sorting and domain hash func (t *Task) ID() string { return t.operationID + t.taskName + t.service @@ -64,24 +71,28 @@ func (t *Task) Validate() error { } // to avoid collisions in alias domains, we should check some fields on regexp for _, f := range []struct { - value string - name string + value string + name string + regexp *regexp.Regexp }{ { - value: t.operationAlias, - name: "operationAlias", + value: t.operationAlias, + name: "operationAlias", + regexp: aliasRegexp, }, { - value: t.taskName, - name: "taskName", + value: t.taskName, + name: "taskName", + regexp: valueRegexp, }, { - value: t.service, - name: "service", + value: t.service, + name: "service", + regexp: valueRegexp, }, } { - if !valueRegexp.MatchString(f.value) { - return fmt.Errorf("field %q value %q does not match regexp %q", f.name, f.value, valueRegexp.String()) + if !f.regexp.MatchString(f.value) { + return fmt.Errorf("field %q value %q does not match regexp %q", f.name, f.value, f.regexp.String()) } } return nil @@ -104,11 +115,16 @@ func getTaskAliasDomain(task Task, baseDomain string) string { } func tryParseAliasSubdomain(subdomain string) (string, string, string, bool) { + // Format: --. The alias may itself contain hyphens, so we + // parse from the right: task and service are the last two (hyphen-free) segments, + // and everything before them is the alias. parts := strings.Split(subdomain, "-") - if len(parts) != 3 { + if len(parts) < 3 { return "", "", "", false } - return parts[0], parts[1], parts[2], true + n := len(parts) + alias := strings.Join(parts[:n-2], "-") + return alias, parts[n-2], parts[n-1], true } func Hash(source []byte) string { diff --git a/server/pkg/task_test.go b/server/pkg/task_test.go index aae4ce8..c3c6bc9 100644 --- a/server/pkg/task_test.go +++ b/server/pkg/task_test.go @@ -23,14 +23,23 @@ func TestValidateTask(t *testing.T) { }, }, { - name: "invalid alias", + name: "alias with hyphen is valid", task: Task{ operationID: "123", operationAlias: "ali-as", taskName: "task", service: "service", }, - err: errors.New("field \"operationAlias\" value \"ali-as\" does not match regexp \"^[a-z0-9_]{1,30}$\""), + }, + { + name: "invalid alias (leading hyphen)", + task: Task{ + operationID: "123", + operationAlias: "-alias", + taskName: "task", + service: "service", + }, + err: errors.New("field \"operationAlias\" value \"-alias\" does not match regexp \"^[a-z0-9_]([a-z0-9_-]{0,28}[a-z0-9_])?$\""), }, { name: "invalid task name", diff --git a/server/pkg/xds.go b/server/pkg/xds.go index 0b5176f..172c9c8 100644 --- a/server/pkg/xds.go +++ b/server/pkg/xds.go @@ -206,6 +206,12 @@ func makeSnapshot(hashToTask map[string]Task, version string, baseDomain string, CodecType: hcmv3.HttpConnectionManager_AUTO, HttpFilters: httpFilters, Http2ProtocolOptions: &corev3.Http2ProtocolOptions{}, + // Allow proxying WebSocket connections (e.g. Jupyter RTC / collaboration + // endpoints). Without this Envoy strips the Upgrade header and forwards the + // request as a plain GET, which the upstream rejects with 400. + UpgradeConfigs: []*hcmv3.HttpConnectionManager_UpgradeConfig{ + {UpgradeType: "websocket"}, + }, } var transportSocket *corev3.TransportSocket diff --git a/server/pkg/xds_test.go b/server/pkg/xds_test.go index 2ecb9d5..2886054 100644 --- a/server/pkg/xds_test.go +++ b/server/pkg/xds_test.go @@ -228,6 +228,8 @@ listener: match: prefix: / statPrefix: ingress_http + upgradeConfigs: + - upgradeType: websocket name: listener_0 `