Skip to content

Commit daf2a73

Browse files
committed
fix: tighten session resolution for ssh workflows
1 parent d747a08 commit daf2a73

10 files changed

Lines changed: 217 additions & 26 deletions

File tree

docs/quickstart.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,9 @@ okdev resolves the target cluster in this order:
141141
## SSH Details
142142

143143
- SSH connects to the `dev` container via `okdev-sshd` on port 2222.
144-
- Tmux is enabled by default with a built-in okdev profile (history, mouse, vi-copy, status bar) using the standard `Ctrl-b` prefix.
145-
- Use `okdev ssh --no-tmux` to bypass tmux for a single connection, or set `spec.ssh.persistentSession: false` to disable it globally.
144+
- `okdev ssh` uses tmux by default with a built-in okdev profile (history, mouse, vi-copy, status bar) using the standard `Ctrl-b` prefix.
145+
- The generated `ssh okdev-<session>` host entry bypasses tmux by default for a plain shell.
146+
- Use `okdev ssh --no-tmux` to bypass tmux for a single `okdev ssh` connection, or set `spec.ssh.persistentSession: false` to disable it globally.
146147
- The managed SSH host entry uses tight proxy keepalive settings so hung sessions fail fast instead of freezing.
147148
- Tune keepalive with `spec.ssh.keepAliveIntervalSeconds` (default `30`) and `spec.ssh.keepAliveTimeoutSeconds` (default `90`, must be >= interval).
148149
- Proxy diagnostics are written to `~/.okdev/logs/okdev.log`, not the SSH session.

docs/troubleshooting.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,9 @@
2929
- Run `okdev ssh --setup-key` at least once per key pair.
3030
- If local bind conflicts occur, use `okdev ssh --local-port <port>`.
3131
- Verify the managed entry exists in `~/.ssh/config`: `Host okdev-<session>`.
32-
- For tmux-enabled sessions, use interactive TTY mode: `okdev ssh` or `ssh -tt okdev-<session>`.
33-
- To bypass tmux for one connection, use `okdev ssh --no-tmux`.
32+
- For tmux-enabled sessions, use interactive TTY mode with `okdev ssh`.
33+
- `ssh okdev-<session>` opens a plain shell by default.
34+
- To bypass tmux for one `okdev ssh` connection, use `okdev ssh --no-tmux`.
3435
- For unstable links, increase `spec.ssh.keepAliveIntervalSeconds` and `spec.ssh.keepAliveTimeoutSeconds`.
3536
- If `ssh okdev-<session>` exits after a long stall, inspect `~/.okdev/logs/okdev.log` for proxy health events such as port-forward degradation or idle watchdog disconnects.
3637
- Managed proxy sessions are designed to fail closed and return control to the local terminal rather than hang indefinitely on a dead port-forward.

internal/cli/common.go

Lines changed: 69 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -209,31 +209,75 @@ func isTerminalFD(v any) bool {
209209
return term.IsTerminal(int(f.Fd()))
210210
}
211211

212-
func resolveSessionName(opts *Options, cfg *config.DevEnvironment) (string, error) {
213-
return session.Resolve(opts.Session, cfg.Spec.Session.DefaultNameTemplate)
212+
// applySessionArg uses the first positional argument as the session name
213+
// when --session was not explicitly provided.
214+
func applySessionArg(opts *Options, args []string) {
215+
if len(args) > 0 && strings.TrimSpace(args[0]) != "" && strings.TrimSpace(opts.Session) == "" {
216+
opts.Session = args[0]
217+
}
218+
}
219+
220+
func resolveSessionName(opts *Options, cfg *config.DevEnvironment, namespace string) (string, error) {
221+
return resolveSessionNameWithState(opts, cfg, namespace, true)
214222
}
215223

216224
func resolveSessionNameForUpDown(opts *Options, cfg *config.DevEnvironment, namespace string) (string, error) {
225+
return resolveSessionNameWithState(opts, cfg, namespace, true)
226+
}
227+
228+
func resolveSessionNameWithState(opts *Options, cfg *config.DevEnvironment, namespace string, inferExisting bool) (string, error) {
229+
return resolveSessionNameWithReader(opts, cfg, namespace, inferExisting, newKubeClient(opts))
230+
}
231+
232+
func resolveSessionNameWithReader(opts *Options, cfg *config.DevEnvironment, namespace string, inferExisting bool, reader sessionAccessReader) (string, error) {
217233
if strings.TrimSpace(opts.Session) != "" {
218-
return resolveSessionName(opts, cfg)
234+
return session.Resolve(opts.Session, cfg.Spec.Session.DefaultNameTemplate)
219235
}
220236
active, err := session.LoadActiveSession()
221237
if err != nil {
222238
return "", err
223239
}
224240
if strings.TrimSpace(active) != "" {
225-
return session.Resolve(active, cfg.Spec.Session.DefaultNameTemplate)
226-
}
227-
inferred, err := inferExistingSessionForRepo(opts, cfg, namespace)
228-
if err != nil {
229-
return "", err
241+
resolvedActive, err := session.Resolve(active, cfg.Spec.Session.DefaultNameTemplate)
242+
if err != nil {
243+
return "", err
244+
}
245+
exists, existsErr := sessionPodExists(reader, namespace, resolvedActive)
246+
if existsErr == nil {
247+
if exists {
248+
return resolvedActive, nil
249+
}
250+
_ = session.ClearActiveSession()
251+
} else {
252+
slog.Debug("failed to verify active session pod", "session", resolvedActive, "namespace", namespace, "error", existsErr)
253+
return resolvedActive, nil
254+
}
230255
}
231-
if strings.TrimSpace(inferred) != "" {
232-
return inferred, nil
256+
if inferExisting {
257+
inferred, err := inferExistingSessionForRepo(opts, cfg, namespace)
258+
if err != nil {
259+
return "", err
260+
}
261+
if strings.TrimSpace(inferred) != "" {
262+
return inferred, nil
263+
}
233264
}
234265
return session.Resolve("", cfg.Spec.Session.DefaultNameTemplate)
235266
}
236267

268+
func sessionPodExists(k sessionAccessReader, namespace, sessionName string) (bool, error) {
269+
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
270+
defer cancel()
271+
_, err := k.GetPodSummary(ctx, namespace, podName(sessionName))
272+
if err == nil {
273+
return true, nil
274+
}
275+
if apierrors.IsNotFound(err) {
276+
return false, nil
277+
}
278+
return false, err
279+
}
280+
237281
func inferExistingSessionForRepo(opts *Options, cfg *config.DevEnvironment, namespace string) (string, error) {
238282
root, err := session.RepoRoot()
239283
if err != nil || strings.TrimSpace(root) == "" {
@@ -417,11 +461,26 @@ func isSessionShareable(p kube.PodSummary) bool {
417461
}
418462

419463
func ensureSessionOwnership(opts *Options, k *kube.Client, namespace, sessionName string, allowShareable bool) error {
464+
return ensureSessionAccess(opts, k, namespace, sessionName, allowShareable, false)
465+
}
466+
467+
func ensureExistingSessionOwnership(opts *Options, k *kube.Client, namespace, sessionName string, allowShareable bool) error {
468+
return ensureSessionAccess(opts, k, namespace, sessionName, allowShareable, true)
469+
}
470+
471+
type sessionAccessReader interface {
472+
GetPodSummary(context.Context, string, string) (*kube.PodSummary, error)
473+
}
474+
475+
func ensureSessionAccess(opts *Options, k sessionAccessReader, namespace, sessionName string, allowShareable bool, requireExisting bool) error {
420476
ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
421477
defer cancel()
422478
pod, err := k.GetPodSummary(ctx, namespace, podName(sessionName))
423479
if err != nil {
424480
if apierrors.IsNotFound(err) {
481+
if requireExisting {
482+
return fmt.Errorf("session %q does not exist in namespace %q", sessionName, namespace)
483+
}
425484
return nil
426485
}
427486
return err

internal/cli/connect.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,19 +12,21 @@ func newConnectCmd(opts *Options) *cobra.Command {
1212
var noTTY bool
1313

1414
cmd := &cobra.Command{
15-
Use: "connect",
15+
Use: "connect [session]",
1616
Short: "Open shell or run command in session pod",
17+
Args: cobra.MaximumNArgs(1),
1718
RunE: func(cmd *cobra.Command, args []string) error {
19+
applySessionArg(opts, args)
1820
cfg, ns, err := loadConfigAndNamespace(opts)
1921
if err != nil {
2022
return err
2123
}
22-
sn, err := resolveSessionName(opts, cfg)
24+
sn, err := resolveSessionName(opts, cfg, ns)
2325
if err != nil {
2426
return err
2527
}
2628
k := newKubeClient(opts)
27-
if err := ensureSessionOwnership(opts, k, ns, sn, true); err != nil {
29+
if err := ensureExistingSessionOwnership(opts, k, ns, sn, true); err != nil {
2830
return err
2931
}
3032
stopMaintenance := startSessionMaintenance(opts, cfg, ns, sn, cmd.OutOrStdout(), true, true)

internal/cli/ports.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ func newPortsCmd(opts *Options) *cobra.Command {
2727
return err
2828
}
2929
k := newKubeClient(opts)
30-
if err := ensureSessionOwnership(opts, k, ns, sn, true); err != nil {
30+
if err := ensureExistingSessionOwnership(opts, k, ns, sn, true); err != nil {
3131
return err
3232
}
3333
effectiveSSHPort := sshPort
Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
package cli
2+
3+
import (
4+
"context"
5+
"strings"
6+
"testing"
7+
8+
"github.com/acmore/okdev/internal/config"
9+
"github.com/acmore/okdev/internal/kube"
10+
"github.com/acmore/okdev/internal/session"
11+
apierrors "k8s.io/apimachinery/pkg/api/errors"
12+
"k8s.io/apimachinery/pkg/runtime/schema"
13+
)
14+
15+
type fakeSessionAccessReader struct {
16+
pod *kube.PodSummary
17+
err error
18+
}
19+
20+
func (f fakeSessionAccessReader) GetPodSummary(_ context.Context, _, _ string) (*kube.PodSummary, error) {
21+
if f.err != nil {
22+
return nil, f.err
23+
}
24+
return f.pod, nil
25+
}
26+
27+
func TestEnsureSessionAccessRequiresExistingSessionWhenRequested(t *testing.T) {
28+
err := ensureSessionAccess(&Options{}, fakeSessionAccessReader{
29+
err: apierrors.NewNotFound(schema.GroupResource{Group: "", Resource: "pods"}, "okdev-old"),
30+
}, "default", "old", true, true)
31+
if err == nil || !strings.Contains(err.Error(), `session "old" does not exist in namespace "default"`) {
32+
t.Fatalf("expected missing session error, got %v", err)
33+
}
34+
}
35+
36+
func TestEnsureSessionAccessAllowsMissingSessionWhenNotRequired(t *testing.T) {
37+
err := ensureSessionAccess(&Options{}, fakeSessionAccessReader{
38+
err: apierrors.NewNotFound(schema.GroupResource{Group: "", Resource: "pods"}, "okdev-old"),
39+
}, "default", "old", true, false)
40+
if err != nil {
41+
t.Fatalf("expected missing session to be allowed, got %v", err)
42+
}
43+
}
44+
45+
func TestEnsureSessionAccessRejectsOtherOwner(t *testing.T) {
46+
t.Setenv("USER", "alice")
47+
err := ensureSessionAccess(&Options{}, fakeSessionAccessReader{
48+
pod: &kube.PodSummary{
49+
Labels: map[string]string{
50+
"okdev.io/owner": "bob",
51+
"okdev.io/shareable": "false",
52+
},
53+
Annotations: map[string]string{},
54+
},
55+
}, "default", "team", false, true)
56+
if err == nil || !strings.Contains(err.Error(), `session "team" is owned by "bob"`) {
57+
t.Fatalf("expected owner mismatch error, got %v", err)
58+
}
59+
}
60+
61+
func TestEnsureSessionAccessAllowsShareableOtherOwner(t *testing.T) {
62+
t.Setenv("USER", "alice")
63+
err := ensureSessionAccess(&Options{}, fakeSessionAccessReader{
64+
pod: &kube.PodSummary{
65+
Labels: map[string]string{
66+
"okdev.io/owner": "bob",
67+
"okdev.io/shareable": "true",
68+
},
69+
Annotations: map[string]string{
70+
"okdev.io/shareable": "true",
71+
},
72+
},
73+
}, "default", "team", true, true)
74+
if err != nil {
75+
t.Fatalf("expected shareable session to be allowed, got %v", err)
76+
}
77+
}
78+
79+
func TestEnsureSessionAccessAllowsCurrentOwner(t *testing.T) {
80+
t.Setenv("USER", "alice")
81+
err := ensureSessionAccess(&Options{}, fakeSessionAccessReader{
82+
pod: &kube.PodSummary{
83+
Labels: map[string]string{
84+
"okdev.io/owner": "alice",
85+
},
86+
Annotations: map[string]string{},
87+
},
88+
}, "default", "team", false, true)
89+
if err != nil {
90+
t.Fatalf("expected current owner to be allowed, got %v", err)
91+
}
92+
}
93+
94+
func TestResolveSessionNameWithReaderIgnoresStaleActiveSession(t *testing.T) {
95+
t.Setenv("HOME", t.TempDir())
96+
if err := session.SaveActiveSession("old-session"); err != nil {
97+
t.Fatalf("SaveActiveSession: %v", err)
98+
}
99+
100+
cfg := &config.DevEnvironment{}
101+
cfg.Spec.Session.DefaultNameTemplate = "fresh-session"
102+
103+
got, err := resolveSessionNameWithReader(&Options{}, cfg, "default", false, fakeSessionAccessReader{
104+
err: apierrors.NewNotFound(schema.GroupResource{Group: "", Resource: "pods"}, "okdev-old-session"),
105+
})
106+
if err != nil {
107+
t.Fatalf("resolveSessionNameWithReader: %v", err)
108+
}
109+
if got != "fresh-session" {
110+
t.Fatalf("expected stale active session to be ignored, got %q", got)
111+
}
112+
113+
active, err := session.LoadActiveSession()
114+
if err != nil {
115+
t.Fatalf("LoadActiveSession: %v", err)
116+
}
117+
if active != "" {
118+
t.Fatalf("expected stale active session to be cleared, got %q", active)
119+
}
120+
}

internal/cli/ssh.go

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,24 +34,26 @@ func newSSHCmd(opts *Options) *cobra.Command {
3434
var noTmux bool
3535

3636
cmd := &cobra.Command{
37-
Use: "ssh",
37+
Use: "ssh [session]",
3838
Short: "Connect to session pod over SSH",
39+
Args: cobra.MaximumNArgs(1),
3940
RunE: func(cmd *cobra.Command, args []string) error {
4041
return withQuietConfigAnnounce(func() error {
4142
errOut := cmd.ErrOrStderr()
43+
applySessionArg(opts, args)
4244
cfg, ns, err := loadConfigAndNamespace(opts)
4345
if err != nil {
4446
return err
4547
}
4648
stopResolve := startTransientStatus(errOut, "Resolving SSH session")
47-
sn, err := resolveSessionName(opts, cfg)
49+
sn, err := resolveSessionName(opts, cfg, ns)
4850
stopResolve()
4951
if err != nil {
5052
return err
5153
}
5254
k := newKubeClient(opts)
5355
stopOwnership := startTransientStatus(errOut, "Verifying session access")
54-
if err := ensureSessionOwnership(opts, k, ns, sn, true); err != nil {
56+
if err := ensureExistingSessionOwnership(opts, k, ns, sn, true); err != nil {
5557
stopOwnership()
5658
return err
5759
}
@@ -416,6 +418,7 @@ func ensureSSHConfigEntry(hostAlias, sessionName, namespace, user string, remote
416418
" IdentityFile " + keyPath,
417419
" StrictHostKeyChecking no",
418420
" UserKnownHostsFile /dev/null",
421+
" SetEnv OKDEV_NO_TMUX=1",
419422
" ProxyCommand " + proxyCmd,
420423
}
421424
// Hardcoded keepalive values for the proxy path — sshSpec values are
@@ -509,12 +512,12 @@ func newSSHProxyCmd(opts *Options) *cobra.Command {
509512
if err != nil {
510513
return err
511514
}
512-
sn, err := resolveSessionName(opts, cfg)
515+
sn, err := resolveSessionName(opts, cfg, ns)
513516
if err != nil {
514517
return err
515518
}
516519
k := newKubeClient(opts)
517-
if err := ensureSessionOwnership(opts, k, ns, sn, true); err != nil {
520+
if err := ensureExistingSessionOwnership(opts, k, ns, sn, true); err != nil {
518521
return err
519522
}
520523
if remotePort == 0 {

internal/cli/ssh_config_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,9 @@ func TestEnsureSSHConfigEntryIncludesNamespaceInProxyCommand(t *testing.T) {
4242
if !strings.Contains(text, "Host okdev-test") {
4343
t.Fatalf("missing host block: %s", text)
4444
}
45+
if !strings.Contains(text, "SetEnv OKDEV_NO_TMUX=1") {
46+
t.Fatalf("expected managed ssh host to disable tmux by default: %s", text)
47+
}
4548
if !strings.Contains(text, "--session") || !strings.Contains(text, "test-session") || !strings.Contains(text, " -n ") || !strings.Contains(text, "dev-ns") || !strings.Contains(text, "ssh-proxy --remote-port 2222") {
4649
t.Fatalf("proxy command missing namespace: %s", text)
4750
}

internal/cli/status.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,11 @@ func newStatusCmd(opts *Options) *cobra.Command {
1313
var allUsers bool
1414

1515
cmd := &cobra.Command{
16-
Use: "status",
16+
Use: "status [session]",
1717
Short: "Show session status",
18+
Args: cobra.MaximumNArgs(1),
1819
RunE: func(cmd *cobra.Command, args []string) error {
20+
applySessionArg(opts, args)
1921
cfg, ns, err := loadConfigAndNamespace(opts)
2022
if err != nil {
2123
return err
@@ -25,7 +27,7 @@ func newStatusCmd(opts *Options) *cobra.Command {
2527
label = label + "," + ownerLabelSelector(opts)
2628
}
2729
if !all {
28-
sn, err := resolveSessionName(opts, cfg)
30+
sn, err := resolveSessionName(opts, cfg, ns)
2931
if err != nil {
3032
return err
3133
}

internal/cli/sync.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ func newSyncCmd(opts *Options) *cobra.Command {
3838
return err
3939
}
4040
k := newKubeClient(opts)
41-
if err := ensureSessionOwnership(opts, k, ns, sn, true); err != nil {
41+
if err := ensureExistingSessionOwnership(opts, k, ns, sn, true); err != nil {
4242
return err
4343
}
4444
engine := cfg.Spec.Sync.Engine

0 commit comments

Comments
 (0)