Skip to content

Commit 6415813

Browse files
committed
fix(ssh): harden keepalive and session lifecycle UX
1 parent db73143 commit 6415813

10 files changed

Lines changed: 258 additions & 26 deletions

File tree

infra/sidecar/entrypoint.sh

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,37 @@ if [ -z "$DEV_PID" ]; then
3737
exit 1
3838
fi
3939
40-
exec nsenter --target "$DEV_PID" --mount --uts --ipc --pid -- /bin/sh -l
40+
# Ensure current GID exists in /etc/group inside the target to suppress
41+
# "groups: cannot find name for group ID ..." warnings from login shells.
42+
CURRENT_GID="$(id -g 2>/dev/null || true)"
43+
if [ -n "$CURRENT_GID" ]; then
44+
nsenter --target "$DEV_PID" --mount -- sh -c "
45+
grep -q \":${CURRENT_GID}:\" /etc/group 2>/dev/null || \
46+
echo \"okdev:x:${CURRENT_GID}:\" >> /etc/group 2>/dev/null || true
47+
"
48+
fi
49+
50+
# If a remote command was requested, execute it inside the dev container.
51+
if [ -n "${SSH_ORIGINAL_COMMAND:-}" ]; then
52+
if nsenter --target "$DEV_PID" --mount -- test -x /bin/bash 2>/dev/null; then
53+
exec nsenter --target "$DEV_PID" --mount --uts --ipc --pid -- /bin/bash -lc "$SSH_ORIGINAL_COMMAND"
54+
else
55+
exec nsenter --target "$DEV_PID" --mount --uts --ipc --pid -- /bin/sh -lc "$SSH_ORIGINAL_COMMAND"
56+
fi
57+
fi
58+
59+
# If there is no TTY and no remote command (e.g. SSH master/control session),
60+
# keep the connection open so forwarding channels stay alive.
61+
if [ ! -t 0 ]; then
62+
exec nsenter --target "$DEV_PID" --mount --uts --ipc --pid -- /bin/sh -lc "while :; do sleep 3600; done"
63+
fi
64+
65+
# Interactive login shell in dev container.
66+
if nsenter --target "$DEV_PID" --mount -- test -x /bin/bash 2>/dev/null; then
67+
exec nsenter --target "$DEV_PID" --mount --uts --ipc --pid -- /bin/bash -l
68+
else
69+
exec nsenter --target "$DEV_PID" --mount --uts --ipc --pid -- /bin/sh -l
70+
fi
4171
SCRIPT
4272
chmod +x /usr/local/bin/nsenter-dev.sh
4373

internal/cli/common.go

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"os/exec"
1111
"path/filepath"
1212
"regexp"
13+
"sort"
1314
"strings"
1415
"time"
1516

@@ -54,6 +55,68 @@ func resolveSessionName(opts *Options, cfg *config.DevEnvironment) (string, erro
5455
return session.Resolve(opts.Session, cfg.Spec.Session.DefaultNameTemplate)
5556
}
5657

58+
func resolveSessionNameForUpDown(opts *Options, cfg *config.DevEnvironment, namespace string) (string, error) {
59+
if strings.TrimSpace(opts.Session) != "" {
60+
return resolveSessionName(opts, cfg)
61+
}
62+
active, err := session.LoadActiveSession()
63+
if err != nil {
64+
return "", err
65+
}
66+
if strings.TrimSpace(active) != "" {
67+
return session.Resolve(active, cfg.Spec.Session.DefaultNameTemplate)
68+
}
69+
inferred, err := inferExistingSessionForRepo(opts, cfg, namespace)
70+
if err != nil {
71+
return "", err
72+
}
73+
if strings.TrimSpace(inferred) != "" {
74+
return inferred, nil
75+
}
76+
return session.Resolve("", cfg.Spec.Session.DefaultNameTemplate)
77+
}
78+
79+
func inferExistingSessionForRepo(opts *Options, cfg *config.DevEnvironment, namespace string) (string, error) {
80+
root, err := session.RepoRoot()
81+
if err != nil || strings.TrimSpace(root) == "" {
82+
return "", nil
83+
}
84+
repo := filepath.Base(root)
85+
if strings.TrimSpace(repo) == "" {
86+
return "", nil
87+
}
88+
label := []string{
89+
"okdev.io/managed=true",
90+
ownerLabelSelector(opts),
91+
"okdev.io/repo=" + repo,
92+
}
93+
if strings.TrimSpace(cfg.Metadata.Name) != "" {
94+
label = append(label, "okdev.io/name="+cfg.Metadata.Name)
95+
}
96+
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
97+
defer cancel()
98+
pods, err := newKubeClient(opts).ListPods(ctx, namespace, false, strings.Join(label, ","))
99+
if err != nil {
100+
return "", err
101+
}
102+
if len(pods) == 0 {
103+
return "", nil
104+
}
105+
sort.Slice(pods, func(i, j int) bool {
106+
return pods[i].CreatedAt.After(pods[j].CreatedAt)
107+
})
108+
for _, p := range pods {
109+
sn := strings.TrimSpace(p.Labels["okdev.io/session"])
110+
if sn != "" {
111+
return sn, nil
112+
}
113+
if strings.HasPrefix(p.Name, "okdev-") {
114+
return strings.TrimPrefix(p.Name, "okdev-"), nil
115+
}
116+
}
117+
return "", nil
118+
}
119+
57120
func labelsForSession(opts *Options, cfg *config.DevEnvironment, sessionName string) map[string]string {
58121
owner := currentOwner(opts)
59122
repo := "unknown"

internal/cli/down.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@ import (
66
"path/filepath"
77
"strings"
88

9+
"github.com/acmore/okdev/internal/session"
910
"github.com/spf13/cobra"
11+
apierrors "k8s.io/apimachinery/pkg/api/errors"
1012
)
1113

1214
func newDownCmd(opts *Options) *cobra.Command {
@@ -21,7 +23,7 @@ func newDownCmd(opts *Options) *cobra.Command {
2123
if err != nil {
2224
return err
2325
}
24-
sn, err := resolveSessionName(opts, cfg)
26+
sn, err := resolveSessionNameForUpDown(opts, cfg, ns)
2527
if err != nil {
2628
return err
2729
}
@@ -42,17 +44,20 @@ func newDownCmd(opts *Options) *cobra.Command {
4244
return nil
4345
}
4446

45-
if err := k.Delete(ctx, ns, "pod", podName(sn), true); err != nil {
47+
if err := k.Delete(ctx, ns, "pod", podName(sn), true); err != nil && !apierrors.IsNotFound(err) {
4648
return fmt.Errorf("delete session pod: %w", err)
4749
}
4850
if deletePVC && cfg.Spec.Workspace.PVC.ClaimName == "" {
49-
if err := k.Delete(ctx, ns, "pvc", pvcName(cfg, sn), true); err != nil {
51+
if err := k.Delete(ctx, ns, "pvc", pvcName(cfg, sn), true); err != nil && !apierrors.IsNotFound(err) {
5052
return fmt.Errorf("delete workspace pvc: %w", err)
5153
}
5254
}
5355
alias := sshHostAlias(sn)
5456
_ = stopManagedSSHForward(alias)
5557
_ = removeSSHConfigEntry(alias)
58+
if active, err := session.LoadActiveSession(); err == nil && active == sn {
59+
_ = session.ClearActiveSession()
60+
}
5661
fmt.Fprintf(cmd.OutOrStdout(), "Session stopped: %s\n", sn)
5762
if !deletePVC && cfg.Spec.Workspace.PVC.ClaimName == "" {
5863
fmt.Fprintf(cmd.OutOrStdout(), "Workspace PVC retained: %s (use --delete-pvc to remove)\n", pvcName(cfg, sn))

internal/cli/list.go

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@ package cli
22

33
import (
44
"sort"
5+
"strings"
56

67
"github.com/acmore/okdev/internal/config"
8+
"github.com/acmore/okdev/internal/kube"
79
"github.com/acmore/okdev/internal/output"
810
"github.com/acmore/okdev/internal/session"
911
"github.com/spf13/cobra"
@@ -55,7 +57,7 @@ func newListCmd(opts *Options) *cobra.Command {
5557
}
5658
rows := make([]listRow, 0, len(pods))
5759
for _, p := range pods {
58-
sn := p.Labels["okdev.io/session"]
60+
sn := sessionNameFromPodSummary(p)
5961
rows = append(rows, listRow{
6062
Namespace: p.Namespace,
6163
Session: sn,
@@ -69,7 +71,7 @@ func newListCmd(opts *Options) *cobra.Command {
6971
}
7072
rows := make([][]string, 0, len(pods))
7173
for _, p := range pods {
72-
sn := p.Labels["okdev.io/session"]
74+
sn := sessionNameFromPodSummary(p)
7375
if sn != "" && sn == activeSession {
7476
sn = "*" + sn
7577
}
@@ -90,3 +92,16 @@ func newListCmd(opts *Options) *cobra.Command {
9092
cmd.Flags().BoolVar(&allUsers, "all-users", false, "List sessions for all owners")
9193
return cmd
9294
}
95+
96+
func sessionNameFromPodSummary(p kube.PodSummary) string {
97+
if p.Labels != nil {
98+
if sn := strings.TrimSpace(p.Labels["okdev.io/session"]); sn != "" {
99+
return sn
100+
}
101+
}
102+
name := strings.TrimSpace(p.Name)
103+
if strings.HasPrefix(name, "okdev-") {
104+
return strings.TrimPrefix(name, "okdev-")
105+
}
106+
return name
107+
}

internal/cli/ssh.go

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"path/filepath"
1212
"strings"
1313
"sync"
14+
"syscall"
1415
"time"
1516

1617
"github.com/acmore/okdev/internal/config"
@@ -228,6 +229,7 @@ func ensureSSHConfigEntry(hostAlias, sessionName, namespace, user string, remote
228229
" ServerAliveInterval 30",
229230
" ServerAliveCountMax 10",
230231
" TCPKeepAlive yes",
232+
" LogLevel ERROR",
231233
end,
232234
)
233235
block := strings.Join(blockLines, "\n") + "\n"
@@ -305,7 +307,7 @@ func newSSHProxyCmd(opts *Options) *cobra.Command {
305307
var copyErr error
306308
var once sync.Once
307309
setErr := func(err error) {
308-
if err == nil || errors.Is(err, io.EOF) {
310+
if err == nil || errors.Is(err, io.EOF) || errors.Is(err, net.ErrClosed) || errors.Is(err, syscall.EPIPE) || isIgnorableProxyIOError(err) {
309311
return
310312
}
311313
once.Do(func() { copyErr = err })
@@ -315,9 +317,6 @@ func newSSHProxyCmd(opts *Options) *cobra.Command {
315317
defer wg.Done()
316318
_, err := io.Copy(conn, os.Stdin)
317319
setErr(err)
318-
if cw, ok := conn.(interface{ CloseWrite() error }); ok {
319-
_ = cw.CloseWrite()
320-
}
321320
}()
322321
go func() {
323322
defer wg.Done()
@@ -332,6 +331,16 @@ func newSSHProxyCmd(opts *Options) *cobra.Command {
332331
return cmd
333332
}
334333

334+
func isIgnorableProxyIOError(err error) bool {
335+
if err == nil {
336+
return true
337+
}
338+
msg := strings.ToLower(err.Error())
339+
return strings.Contains(msg, "broken pipe") ||
340+
strings.Contains(msg, "use of closed network connection") ||
341+
strings.Contains(msg, "connection reset by peer")
342+
}
343+
335344
func waitDialLocal(localPort int, timeout time.Duration) (net.Conn, error) {
336345
deadline := time.Now().Add(timeout)
337346
addr := fmt.Sprintf("127.0.0.1:%d", localPort)
@@ -377,10 +386,11 @@ func startManagedSSHForward(hostAlias string) error {
377386
"-M",
378387
"-S", socketPath,
379388
"-o", "ControlPersist=600",
380-
"-o", "ExitOnForwardFailure=yes",
389+
"-o", "ExitOnForwardFailure=no",
381390
"-o", "ServerAliveInterval=30",
382391
"-o", "ServerAliveCountMax=10",
383392
"-o", "TCPKeepAlive=yes",
393+
"-o", "LogLevel=ERROR",
384394
hostAlias,
385395
)
386396
if out, err := cmd.CombinedOutput(); err != nil {

internal/cli/sync.go

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"strconv"
99
"strings"
1010
"syscall"
11+
"time"
1112

1213
syncengine "github.com/acmore/okdev/internal/sync"
1314
"github.com/spf13/cobra"
@@ -98,7 +99,7 @@ func startDetachedSyncthingSync(opts *Options, mode, sessionName string) (string
9899
return "", false, err
99100
}
100101
if pid, ok := readSyncthingPID(pidPath); ok {
101-
if processAlive(pid) {
102+
if processAlive(pid) && processLooksLikeSyncthingSync(pid) {
102103
return logPath, false, nil
103104
}
104105
_ = os.Remove(pidPath)
@@ -131,6 +132,7 @@ func startDetachedSyncthingSync(opts *Options, mode, sessionName string) (string
131132
cmd.Stdout = logFile
132133
cmd.Stderr = logFile
133134
cmd.Stdin = nil
135+
cmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true}
134136
if err := cmd.Start(); err != nil {
135137
_ = logFile.Close()
136138
return "", false, err
@@ -139,6 +141,12 @@ func startDetachedSyncthingSync(opts *Options, mode, sessionName string) (string
139141
_ = logFile.Close()
140142
return "", false, err
141143
}
144+
// Guard against immediate child exit (common when sync init fails).
145+
time.Sleep(300 * time.Millisecond)
146+
if !processAlive(cmd.Process.Pid) || !processLooksLikeSyncthingSync(cmd.Process.Pid) {
147+
_ = logFile.Close()
148+
return "", false, fmt.Errorf("syncthing background process exited early; check logs: %s", logPath)
149+
}
142150
_ = cmd.Process.Release()
143151
_ = logFile.Close()
144152
return logPath, true, nil
@@ -173,10 +181,31 @@ func readSyncthingPID(path string) (int, bool) {
173181
}
174182

175183
func processAlive(pid int) bool {
184+
if pid <= 0 {
185+
return false
186+
}
187+
if statOut, err := exec.Command("ps", "-p", strconv.Itoa(pid), "-o", "stat=").CombinedOutput(); err == nil {
188+
stat := strings.TrimSpace(string(statOut))
189+
if stat == "" || strings.Contains(stat, "Z") {
190+
return false
191+
}
192+
}
176193
p, err := os.FindProcess(pid)
177194
if err != nil {
178195
return false
179196
}
180197
err = p.Signal(syscall.Signal(0))
181198
return err == nil
182199
}
200+
201+
func processLooksLikeSyncthingSync(pid int) bool {
202+
if pid <= 0 {
203+
return false
204+
}
205+
out, err := exec.Command("ps", "-p", strconv.Itoa(pid), "-o", "command=").CombinedOutput()
206+
if err != nil {
207+
return false
208+
}
209+
cmdline := string(out)
210+
return strings.Contains(cmdline, "okdev") && strings.Contains(cmdline, "sync")
211+
}

0 commit comments

Comments
 (0)