diff --git a/cmd/internal/agentcontainer/daemon.go b/cmd/internal/agentcontainer/daemon.go index 04a420ef7..ac6d73212 100644 --- a/cmd/internal/agentcontainer/daemon.go +++ b/cmd/internal/agentcontainer/daemon.go @@ -13,7 +13,6 @@ import ( "syscall" "time" - "github.com/devsy-org/devsy/pkg/agent" config2 "github.com/devsy-org/devsy/pkg/config" agentd "github.com/devsy-org/devsy/pkg/daemon/agent" "github.com/devsy-org/devsy/pkg/devcontainer/config" @@ -25,7 +24,7 @@ import ( ) const ( - RootDir = agent.ContainerDataDir + RootDir = config2.ContainerDataDir DaemonConfigPath = "/var/run/secrets/" + config2.BinaryName + "/daemon_config" ) @@ -68,13 +67,13 @@ func (cmd *DaemonCmd) Run(c *cobra.Command, args []string) error { } if timeoutDuration > 0 { if err := os.WriteFile( - agent.ContainerActivityFile, + config2.ContainerActivityFile, nil, 0o666, ); err != nil { // #nosec G306 return fmt.Errorf("failed to create activity file: %w", err) } - if err := os.Chmod(agent.ContainerActivityFile, 0o666); err != nil { // #nosec G302 + if err := os.Chmod(config2.ContainerActivityFile, 0o666); err != nil { // #nosec G302 return fmt.Errorf("failed to set activity file permissions: %w", err) } } @@ -197,7 +196,7 @@ func runTimeoutMonitor( case <-ctx.Done(): return nil case <-ticker.C: - stat, err := os.Stat(agent.ContainerActivityFile) + stat, err := os.Stat(config2.ContainerActivityFile) if err != nil { continue } diff --git a/cmd/internal/agentworkspace/logs.go b/cmd/internal/agentworkspace/logs.go index 1baa93036..42bddb892 100644 --- a/cmd/internal/agentworkspace/logs.go +++ b/cmd/internal/agentworkspace/logs.go @@ -7,6 +7,7 @@ import ( "github.com/devsy-org/devsy/cmd/flags" "github.com/devsy-org/devsy/pkg/agent" + "github.com/devsy-org/devsy/pkg/config" "github.com/devsy-org/devsy/pkg/devcontainer" "github.com/spf13/cobra" ) @@ -61,8 +62,8 @@ func (cmd *LogsCmd) Run(ctx context.Context) error { // create new runner runner, err := devcontainer.NewRunner( - agent.ContainerDevsyHelperLocation, - agent.DefaultAgentDownloadURL(), + config.ContainerDevsyHelperLocation, + config.DefaultAgentDownloadURL(), workspaceInfo, ) if err != nil { diff --git a/cmd/internal/agentworkspace/up.go b/cmd/internal/agentworkspace/up.go index 572328f5d..28ed2a1b5 100644 --- a/cmd/internal/agentworkspace/up.go +++ b/cmd/internal/agentworkspace/up.go @@ -208,8 +208,8 @@ func CreateRunner( workspaceInfo *provider.AgentWorkspaceInfo, ) (devcontainer.Runner, error) { return devcontainer.NewRunner( - agent.ContainerDevsyHelperLocation, - agent.DefaultAgentDownloadURL(), + config.ContainerDevsyHelperLocation, + config.DefaultAgentDownloadURL(), workspaceInfo, ) } @@ -544,6 +544,7 @@ func prepareWorkspace(params prepareWorkspaceParams) error { params.workspaceInfo.Workspace.Source.LocalFolder != "" { params.workspaceInfo.ContentFolder = agent.GetAgentWorkspaceContentDir( params.workspaceInfo.Origin, + params.workspaceInfo.Workspace.UID, ) } diff --git a/cmd/internal/container_tunnel.go b/cmd/internal/container_tunnel.go index 20f5d8c63..ede6ecdfa 100644 --- a/cmd/internal/container_tunnel.go +++ b/cmd/internal/container_tunnel.go @@ -86,23 +86,21 @@ func (cmd *ContainerTunnelCmd) Run(ctx context.Context) error { os.Exit(0) }() - // create tunnel into container. - err = agent.Tunnel( - ctx, - func(ctx context.Context, user string, command string, stdin io.Reader, stdout io.Writer, stderr io.Writer) error { + return agent.Tunnel(ctx, agent.TunnelOptions{ + Exec: func( + ctx context.Context, + user, command string, + stdin io.Reader, + stdout, stderr io.Writer, + ) error { return runner.Command(ctx, user, command, stdin, stdout, stderr) }, - cmd.User, - os.Stdin, - os.Stdout, - os.Stderr, - workspaceInfo.InjectTimeout, - ) - if err != nil { - return err - } - - return nil + User: cmd.User, + Stdin: os.Stdin, + Stdout: os.Stdout, + Stderr: os.Stderr, + Timeout: workspaceInfo.InjectTimeout, + }) } func startDevContainer( diff --git a/cmd/internal/fleet_server.go b/cmd/internal/fleet_server.go index d390928ef..bdea9cf64 100644 --- a/cmd/internal/fleet_server.go +++ b/cmd/internal/fleet_server.go @@ -8,7 +8,7 @@ import ( "time" "github.com/devsy-org/devsy/cmd/flags" - "github.com/devsy-org/devsy/pkg/agent" + "github.com/devsy-org/devsy/pkg/config" "github.com/spf13/cobra" ) @@ -62,7 +62,7 @@ func (c *FleetServerCmd) Run(cmd *cobra.Command, _ []string) error { // if ouf last occurrence of notify if "Notify ID connected" // we have an active session, so let's keep alive if strings.Contains(connString[len(connString)-1][0], "is connected") { - file, _ := os.Create(agent.ContainerActivityFile) + file, _ := os.Create(config.ContainerActivityFile) _ = file.Close() } case <-cmd.Context().Done(): diff --git a/cmd/internal/ssh_server.go b/cmd/internal/ssh_server.go index 5483ad7f7..4eb3d342a 100644 --- a/cmd/internal/ssh_server.go +++ b/cmd/internal/ssh_server.go @@ -1,13 +1,16 @@ package cmdinternal import ( + "context" "encoding/base64" + "errors" "fmt" + "io/fs" "os" "time" "github.com/devsy-org/devsy/cmd/flags" - "github.com/devsy-org/devsy/pkg/agent" + "github.com/devsy-org/devsy/pkg/config" "github.com/devsy-org/devsy/pkg/log" sshserver "github.com/devsy-org/devsy/pkg/ssh/server" "github.com/devsy-org/devsy/pkg/ssh/server/port" @@ -17,151 +20,234 @@ import ( "github.com/spf13/cobra" ) -// SSHServerCmd holds the ssh server cmd flags. -type SSHServerCmd struct { +const ( + activityHeartbeatInterval = 10 * time.Second + activityFileMode = 0o666 +) + +// shutdownTimeout bounds the grace period for in-flight SSH connections to +// drain on context cancel before the listener is force-closed. +const shutdownTimeout = 5 * time.Second + +// sshServerCmd holds the ssh server cmd flags. +type sshServerCmd struct { *flags.GlobalFlags - Token string - Address string - Stdio bool - TrackActivity bool - ReuseSSHAuthSock string - Workdir string + token string + address string + stdio bool + trackActivity bool + reuseSSHAuthSock string + workdir string } // NewSSHServerCmd creates a new ssh command. -func NewSSHServerCmd(flags *flags.GlobalFlags) *cobra.Command { - cmd := &SSHServerCmd{ - GlobalFlags: flags, - } +func NewSSHServerCmd(globalFlags *flags.GlobalFlags) *cobra.Command { + cmd := &sshServerCmd{GlobalFlags: globalFlags} sshCmd := &cobra.Command{ Use: "ssh-server", Short: "Starts a new SSH server", Args: cobra.NoArgs, - RunE: cmd.Run, - } - - sshCmd.Flags(). - StringVar(&cmd.Address, "address", fmt.Sprintf("0.0.0.0:%d", sshserver.DefaultPort), "Address to listen to") - sshCmd.Flags(). - BoolVar(&cmd.Stdio, "stdio", false, "Will listen on stdout and stdin instead of an address") - sshCmd.Flags(). - BoolVar(&cmd.TrackActivity, "track-activity", false, "If enabled will write the last activity time to a file") - sshCmd.Flags(). - StringVar(&cmd.ReuseSSHAuthSock, "reuse-ssh-auth-sock", "", - "If set, the SSH_AUTH_SOCK is expected to already be available in the workspace "+ - "(under /tmp using the key provided) and the connection reuses this instead of creating a new one") - _ = sshCmd.Flags().MarkHidden("reuse-ssh-auth-sock") - sshCmd.Flags().StringVar(&cmd.Token, "token", "", "Base64 encoded token to use") - sshCmd.Flags(). - StringVar(&cmd.Workdir, "workdir", "", "Directory where commands will run on the host") + RunE: func(cobraCmd *cobra.Command, _ []string) error { + return cmd.run(cobraCmd.Context()) + }, + } + + f := sshCmd.Flags() + f.StringVar(&cmd.address, "address", + fmt.Sprintf("0.0.0.0:%d", sshserver.DefaultPort), + "Address to listen to") + f.BoolVar(&cmd.stdio, "stdio", false, + "Listen on stdin/stdout instead of an address") + f.BoolVar(&cmd.trackActivity, "track-activity", false, + "Touch the activity file every "+activityHeartbeatInterval.String()+" (only with --stdio)") + f.StringVar(&cmd.reuseSSHAuthSock, "reuse-ssh-auth-sock", "", + "If set, reuse a pre-existing SSH_AUTH_SOCK in the workspace under /tmp") + _ = f.MarkHidden("reuse-ssh-auth-sock") + f.StringVar(&cmd.token, "token", "", "Base64 encoded token to use") + f.StringVar(&cmd.workdir, "workdir", "", + "Directory where commands will run on the host") return sshCmd } -// Run runs the command logic. -func (cmd *SSHServerCmd) Run(_ *cobra.Command, _ []string) error { - var ( - keys []ssh.PublicKey - hostKey []byte - err error - ) - if cmd.Token != "" { - // parse token - t, err := token.ParseToken(cmd.Token) - if err != nil { - return fmt.Errorf("parse token: %w", err) - } - - if t.AuthorizedKeys != "" { - keyBytes, err := base64.StdEncoding.DecodeString(t.AuthorizedKeys) - if err != nil { - return fmt.Errorf("seems like the provided encoded string is not base64 encoded") - } - - for len(keyBytes) > 0 { - key, _, _, rest, err := ssh.ParseAuthorizedKey(keyBytes) - if err != nil { - return fmt.Errorf("parse authorized key: %w", err) - } - - keys = append(keys, key) - keyBytes = rest - } - } +func (cmd *sshServerCmd) run(ctx context.Context) error { + if cmd.trackActivity && !cmd.stdio { + return errors.New("--track-activity requires --stdio") + } - if len(t.HostKey) > 0 { - hostKey, err = base64.StdEncoding.DecodeString(t.HostKey) - if err != nil { - return fmt.Errorf("decode host key") - } - } + keys, hostKey, err := parseSSHToken(cmd.token) + if err != nil { + return err } // Sweep stale per-connection agent socket directories left behind by - // predecessors that - // were killed by docker exec / proxy-chain teardown before any internal - // SSH cleanup could run. Liveness is decided via a per-directory flock - // the owning process holds for its lifetime; the kernel releases the - // flock on any process exit (including SIGKILL). + // predecessors killed by docker exec / proxy-chain teardown before + // internal SSH cleanup could run. Liveness is decided via a per-directory + // flock the owning process holds for its lifetime; the kernel releases + // the flock on any process exit (including SIGKILL). sshserver.SweepStaleAgentSockets() - // start the server server, err := sshserver.NewServer( - cmd.Address, - hostKey, - keys, - cmd.Workdir, - cmd.ReuseSSHAuthSock, + cmd.address, hostKey, keys, cmd.workdir, cmd.reuseSSHAuthSock, ) if err != nil { - return err + return fmt.Errorf("create ssh server: %w", err) } - // should we listen on stdout & stdin? - if cmd.Stdio { - if cmd.TrackActivity { - go func() { - _, err = os.Stat(agent.ContainerActivityFile) - if err != nil { - if err := os.WriteFile( - agent.ContainerActivityFile, - nil, - 0o666, - ); err != nil { // #nosec G306 - fmt.Fprintf(os.Stderr, "error writing file: %v\n", err) - return - } - if err := os.Chmod( - agent.ContainerActivityFile, - 0o666, - ); err != nil { // #nosec G302 - fmt.Fprintf(os.Stderr, "error setting file permissions: %v\n", err) - return - } - } - - for { - time.Sleep(time.Second * 10) - file, _ := os.Create(agent.ContainerActivityFile) - _ = file.Close() - } - }() - } + if cmd.stdio { + return cmd.serveStdio(ctx, server) + } + return cmd.serveListener(ctx, server) +} - lis := stdio.NewStdioListener(os.Stdin, os.Stdout, true) - return server.Serve(lis) +func (cmd *sshServerCmd) serveStdio(ctx context.Context, server sshserver.Server) error { + if cmd.trackActivity { + go runActivityHeartbeat(ctx, config.ContainerActivityFile) } + go shutdownOnCancel(ctx, server) // #nosec G118 -- see shutdownOnCancel. + lis := stdio.NewStdioListener(os.Stdin, os.Stdout, true) + return ignoreServerClosed(server.Serve(lis)) +} - // check if ssh is already running at that port - available, err := port.IsAvailable(cmd.Address) +func (cmd *sshServerCmd) serveListener(ctx context.Context, server sshserver.Server) error { + available, err := port.IsAvailable(cmd.address) + if err != nil { + return fmt.Errorf("check port %s: %w", cmd.address, err) + } if !available { + return fmt.Errorf("address %s already in use", cmd.address) + } + go shutdownOnCancel(ctx, server) // #nosec G118 -- see shutdownOnCancel. + return ignoreServerClosed(server.ListenAndServe()) +} + +// shutdownOnCancel drains active SSH connections when ctx is canceled, +// bounded by shutdownTimeout. The shutdown context derives from +// context.Background by design: ctx is already canceled at this point, so +// reusing it would make Shutdown's grace period instantly expire. +func shutdownOnCancel(ctx context.Context, server sshserver.Server) { + <-ctx.Done() + shutdownCtx, cancel := context.WithTimeout(context.Background(), shutdownTimeout) + defer cancel() + if err := server.Shutdown(shutdownCtx); err != nil { + log.Errorf("ssh server shutdown: %v", err) + } +} + +// ignoreServerClosed turns the expected post-Shutdown error into a clean +// return so cobra doesn't surface "ssh: Server closed" as a failure. +func ignoreServerClosed(err error) error { + if err == nil || errors.Is(err, ssh.ErrServerClosed) { + return nil + } + return err +} + +// parseSSHToken decodes the optional base64-encoded token blob. An empty +// string is valid and yields zero values: callers may run without a token. +func parseSSHToken(encoded string) ([]ssh.PublicKey, []byte, error) { + if encoded == "" { + return nil, nil, nil + } + + t, err := token.ParseToken(encoded) + if err != nil { + return nil, nil, fmt.Errorf("parse token: %w", err) + } + + keys, err := decodeAuthorizedKeys(t.AuthorizedKeys) + if err != nil { + return nil, nil, err + } + + hostKey, err := decodeBase64Bytes(t.HostKey, "host key") + if err != nil { + return nil, nil, err + } + + return keys, hostKey, nil +} + +func decodeAuthorizedKeys(encoded string) ([]ssh.PublicKey, error) { + if encoded == "" { + return nil, nil + } + raw, err := base64.StdEncoding.DecodeString(encoded) + if err != nil { + return nil, fmt.Errorf("decode authorized keys: %w", err) + } + var keys []ssh.PublicKey + for len(raw) > 0 { + key, _, _, rest, err := ssh.ParseAuthorizedKey(raw) if err != nil { - return fmt.Errorf("address %s already in use: %w", cmd.Address, err) + return nil, fmt.Errorf("parse authorized key: %w", err) } + keys = append(keys, key) + raw = rest + } + return keys, nil +} - log.Infof("address %s already in use", cmd.Address) - return nil +func decodeBase64Bytes(encoded, label string) ([]byte, error) { + if encoded == "" { + return nil, nil + } + out, err := base64.StdEncoding.DecodeString(encoded) + if err != nil { + return nil, fmt.Errorf("decode %s: %w", label, err) + } + return out, nil +} + +// runActivityHeartbeat periodically updates the activity file's mtime so +// outside watchers can detect liveness. Exits when ctx is canceled. Logs +// only on success/failure transitions so a permanently broken file does +// not spam the log. +func runActivityHeartbeat(ctx context.Context, path string) { + if err := ensureActivityFile(path); err != nil { + log.Errorf("activity heartbeat: ensure file: %v", err) + return } - return server.ListenAndServe() + t := time.NewTicker(activityHeartbeatInterval) + defer t.Stop() + + var lastErr error + for { + select { + case <-ctx.Done(): + return + case now := <-t.C: + err := os.Chtimes(path, now, now) + if (err == nil) != (lastErr == nil) { + if err != nil { + log.Errorf("activity heartbeat: %v", err) + } else { + log.Infof("activity heartbeat recovered") + } + } + lastErr = err + } + } +} + +func ensureActivityFile(path string) error { + _, err := os.Stat(path) + if err == nil { + return nil + } + if !errors.Is(err, fs.ErrNotExist) { + return fmt.Errorf("stat: %w", err) + } + if err := os.WriteFile( + path, + nil, + activityFileMode, + ); err != nil { // #nosec G306 -- intentionally world-writable; multiple users update activity + return fmt.Errorf("create: %w", err) + } + if err := os.Chmod(path, activityFileMode); err != nil { // #nosec G302 -- ditto + return fmt.Errorf("chmod: %w", err) + } + return nil } diff --git a/cmd/internal/ssh_server_test.go b/cmd/internal/ssh_server_test.go new file mode 100644 index 000000000..27aaacc36 --- /dev/null +++ b/cmd/internal/ssh_server_test.go @@ -0,0 +1,186 @@ +package cmdinternal + +import ( + "context" + "encoding/base64" + "encoding/json" + "errors" + "io/fs" + "net" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/devsy-org/devsy/pkg/token" + "github.com/devsy-org/ssh" +) + +func encodeTestToken(t *testing.T, tok token.Token) string { + t.Helper() + raw, err := json.Marshal(&tok) + if err != nil { + t.Fatalf("marshal token: %v", err) + } + return base64.StdEncoding.EncodeToString(raw) +} + +const testEd25519PubKey = "ssh-ed25519 " + + "AAAAC3NzaC1lZDI1NTE5AAAAIDuxlhheJj+ON3HxiToVhg+Tj1+/cqLgkBQ8KkKr2T87 " + + "test@example" + +func TestParseSSHTokenEmpty(t *testing.T) { + keys, hostKey, err := parseSSHToken("") + if err != nil || keys != nil || hostKey != nil { + t.Fatalf("expected zeroes, got keys=%v hostKey=%v err=%v", keys, hostKey, err) + } +} + +func TestParseSSHTokenInvalid(t *testing.T) { + _, _, err := parseSSHToken("not-a-valid-token-blob") + if err == nil || !strings.Contains(err.Error(), "parse token") { + t.Fatalf("want parse error, got %v", err) + } +} + +func TestParseSSHTokenWithKeys(t *testing.T) { + encoded := encodeTestToken(t, token.Token{ + AuthorizedKeys: base64.StdEncoding.EncodeToString([]byte(testEd25519PubKey + "\n")), + HostKey: base64.StdEncoding.EncodeToString([]byte("hostkeybytes")), + }) + keys, hostKey, err := parseSSHToken(encoded) + if err != nil { + t.Fatalf("parseSSHToken: %v", err) + } + if len(keys) != 1 { + t.Errorf("want 1 key, got %d", len(keys)) + } + if string(hostKey) != "hostkeybytes" { + t.Errorf("hostKey = %q", hostKey) + } +} + +func TestDecodeAuthorizedKeysMultiple(t *testing.T) { + blob := testEd25519PubKey + "\n" + testEd25519PubKey + "\n" + keys, err := decodeAuthorizedKeys(base64.StdEncoding.EncodeToString([]byte(blob))) + if err != nil { + t.Fatalf("decodeAuthorizedKeys: %v", err) + } + if len(keys) != 2 { + t.Errorf("want 2 keys, got %d", len(keys)) + } +} + +func TestDecodeAuthorizedKeysBadBase64(t *testing.T) { + _, err := decodeAuthorizedKeys("!!!not-base64!!!") + if err == nil || !strings.Contains(err.Error(), "decode authorized keys") { + t.Fatalf("want wrapped decode error, got %v", err) + } +} + +func TestDecodeBase64BytesEmpty(t *testing.T) { + out, err := decodeBase64Bytes("", "host key") + if err != nil || out != nil { + t.Fatalf("want (nil,nil), got (%v,%v)", out, err) + } +} + +func TestDecodeBase64BytesError(t *testing.T) { + _, err := decodeBase64Bytes("!!!", "host key") + if err == nil || !strings.Contains(err.Error(), "decode host key") { + t.Fatalf("want wrapped error with label, got %v", err) + } +} + +func TestEnsureActivityFileCreatesAndIsIdempotent(t *testing.T) { + path := filepath.Join(t.TempDir(), "activity") + if err := ensureActivityFile(path); err != nil { + t.Fatalf("first call: %v", err) + } + if _, err := os.Stat(path); err != nil { + t.Fatalf("expected file to exist: %v", err) + } + // idempotent + if err := ensureActivityFile(path); err != nil { + t.Fatalf("second call: %v", err) + } +} + +func TestEnsureActivityFileSurfacesNonExistErrors(t *testing.T) { + // Path through a non-directory is ENOTDIR, which is neither nil nor ErrNotExist — + // must surface as-is instead of falling into the create branch. + tmp := t.TempDir() + regular := filepath.Join(tmp, "regular") + if err := os.WriteFile(regular, nil, 0o600); err != nil { + t.Fatal(err) + } + bad := filepath.Join(regular, "child") + err := ensureActivityFile(bad) + if err == nil { + t.Fatal("expected error for path under a regular file") + } + if errors.Is(err, fs.ErrNotExist) { + t.Errorf("want non-ErrNotExist error surfaced, got %v", err) + } +} + +type fakeServer struct { + shutdownCalls int + shutdownErr error +} + +func (*fakeServer) Serve(net.Listener) error { return nil } +func (*fakeServer) ListenAndServe() error { return nil } +func (f *fakeServer) Shutdown(context.Context) error { + f.shutdownCalls++ + return f.shutdownErr +} + +func TestShutdownOnCancelInvokesServer(t *testing.T) { + f := &fakeServer{} + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan struct{}) + go func() { + shutdownOnCancel(ctx, f) + close(done) + }() + cancel() + select { + case <-done: + case <-time.After(2 * time.Second): + t.Fatal("shutdownOnCancel did not return after ctx cancel") + } + if f.shutdownCalls != 1 { + t.Errorf("Shutdown calls = %d, want 1", f.shutdownCalls) + } +} + +func TestIgnoreServerClosed(t *testing.T) { + if err := ignoreServerClosed(nil); err != nil { + t.Errorf("nil should map to nil, got %v", err) + } + if err := ignoreServerClosed(ssh.ErrServerClosed); err != nil { + t.Errorf("ErrServerClosed should map to nil, got %v", err) + } + other := errors.New("boom") + if err := ignoreServerClosed(other); err != other { + t.Errorf("unrelated error should pass through, got %v", err) + } +} + +func TestRunActivityHeartbeatExitsOnContextCancel(t *testing.T) { + path := filepath.Join(t.TempDir(), "activity") + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan struct{}) + go func() { + runActivityHeartbeat(ctx, path) + close(done) + }() + cancel() + select { + case <-done: + case <-time.After(2 * time.Second): + t.Fatal("heartbeat did not exit within 2s of context cancel") + } +} diff --git a/cmd/workspace/ssh.go b/cmd/workspace/ssh.go index 9f310a94b..227654841 100644 --- a/cmd/workspace/ssh.go +++ b/cmd/workspace/ssh.go @@ -16,7 +16,6 @@ import ( "github.com/devsy-org/devsy/cmd/completion" "github.com/devsy-org/devsy/cmd/flags" "github.com/devsy-org/devsy/cmd/machine" - "github.com/devsy-org/devsy/pkg/agent" client2 "github.com/devsy-org/devsy/pkg/client" "github.com/devsy-org/devsy/pkg/client/clientimplementation" "github.com/devsy-org/devsy/pkg/config" @@ -543,7 +542,7 @@ func (cmd *SSHCmd) startTunnel( log.Debugf("Run outer container tunnel") commandArgs := []string{ - agent.ContainerDevsyHelperLocation, + config.ContainerDevsyHelperLocation, "internal", "ssh-server", "--track-activity", @@ -705,7 +704,7 @@ func (cmd *SSHCmd) setupGPGAgent( // Now we forward the agent socket to the remote, and setup remote gpg to use it forwardAgent := []string{ - agent.ContainerDevsyHelperLocation, + config.ContainerDevsyHelperLocation, "internal", "agent", "workspace", diff --git a/pkg/agent/agent.go b/pkg/agent/agent.go index 6f5263417..85fda2fce 100644 --- a/pkg/agent/agent.go +++ b/pkg/agent/agent.go @@ -18,37 +18,10 @@ import ( "github.com/devsy-org/devsy/pkg/config" "github.com/devsy-org/devsy/pkg/log" provider2 "github.com/devsy-org/devsy/pkg/provider" - "github.com/devsy-org/devsy/pkg/version" ) const DefaultInactivityTimeout = time.Minute * 20 -// ContainerDataDir is the base directory for Devsy data inside containers. -const ContainerDataDir = "/var/" + config.BinaryName - -const ContainerDevsyHelperLocation = "/usr/local/bin/" + config.BinaryName - -const RemoteDevsyHelperLocation = "/tmp/" + config.BinaryName - -const ContainerActivityFile = "/tmp/" + config.BinaryName + ".activity" - -var defaultAgentDownloadURL = config.GitHubReleasesURL + "/download/" - -const WorkspaceBusyFile = "workspace.lock" - -func DefaultAgentDownloadURL() string { - devsyAgentURL := os.Getenv(config.EnvAgentURL) - if devsyAgentURL != "" { - return strings.TrimRight(devsyAgentURL, "/") - } - - if version.GetVersion() == version.DevVersion { - return config.GitHubReleasesURL + "/latest/download" - } - - return defaultAgentDownloadURL + version.GetVersion() -} - func DecodeContainerWorkspaceInfo( workspaceInfoRaw string, ) (*provider2.ContainerWorkspaceInfo, string, error) { @@ -198,178 +171,109 @@ func decodeWorkspaceInfoAndWrite( writeInfo bool, deleteWorkspace func(workspaceInfo *provider2.AgentWorkspaceInfo) error, ) (bool, *provider2.AgentWorkspaceInfo, error) { - log.Debugf( - "starting to decode and write workspace info: workspaceEncodedLength=%v, writeInfo=%v", - len(workspaceInfoEncoded), - writeInfo, - ) - workspaceInfo, _, err := DecodeWorkspaceInfo(workspaceInfoEncoded) if err != nil { - log.Errorf( - "failed to decode workspace info: error=%v, workspaceEncodedLength=%v", - err, - len(workspaceInfoEncoded), - ) return false, nil, err } - log.Debugf( - "decoded workspace info: workspaceId=%s, context=%s, driver=%s", - workspaceInfo.Workspace.ID, - workspaceInfo.Workspace.Context, - workspaceInfo.Agent.Driver, - ) - - // check if we need to become root - log.Debug("checking if root privileges are required") - shouldExit, err := rerunAsRoot(workspaceInfo) - if err != nil { - log.Errorf("failed to rerun as root: error=%v", err) + if shouldExit, err := rerunAsRoot(workspaceInfo); err != nil { return false, nil, fmt.Errorf("rerun as root: %w", err) } else if shouldExit { - log.Debug("rerunning as root, exiting current process") return true, nil, nil } - // write to workspace folder - log.Debugf( - "creating agent workspace directory: dataPath=%s, context=%s, workspaceId=%s", - workspaceInfo.Agent.DataPath, - workspaceInfo.Workspace.Context, - workspaceInfo.Workspace.ID, - ) workspaceDir, err := CreateAgentWorkspaceDir( workspaceInfo.Agent.DataPath, workspaceInfo.Workspace.Context, workspaceInfo.Workspace.ID, ) if err != nil { - log.Errorf( - "failed to create agent workspace directory: error=%v, dataPath=%s, context=%s, workspaceId=%s", - err, - workspaceInfo.Agent.DataPath, - workspaceInfo.Workspace.Context, - workspaceInfo.Workspace.ID, - ) - return false, nil, err + return false, nil, fmt.Errorf("create workspace dir: %w", err) } - log.Debugf("using workspace dir: workspaceDir=%s", workspaceDir) - - // check if workspace config already exists workspaceConfig := filepath.Join(workspaceDir, provider2.WorkspaceConfigFile) if deleteWorkspace != nil { - log.Debugf("checking for existing workspace config: configFile=%s", workspaceConfig) - - oldWorkspaceInfo, _ := ParseAgentWorkspaceInfo(workspaceConfig) - if oldWorkspaceInfo != nil && - oldWorkspaceInfo.Workspace.UID != workspaceInfo.Workspace.UID { - // delete the old workspace - log.Infof( - "delete old workspace: workspaceId=%s, oldUid=%s, newUid=%s", - oldWorkspaceInfo.Workspace.ID, - oldWorkspaceInfo.Workspace.UID, - workspaceInfo.Workspace.UID, - ) - - err = deleteWorkspace(oldWorkspaceInfo) - if err != nil { - log.Errorf( - "failed to delete old workspace: error=%v, workspaceId=%s", - err, - oldWorkspaceInfo.Workspace.ID, - ) - return false, nil, fmt.Errorf("delete old workspace: %w", err) - } - - // recreate workspace folder again - log.Debug("recreating workspace directory after deletion") - workspaceDir, err = CreateAgentWorkspaceDir( - workspaceInfo.Agent.DataPath, - workspaceInfo.Workspace.Context, - workspaceInfo.Workspace.ID, - ) - if err != nil { - log.Errorf( - "failed to recreate workspace directory: error=%v, dataPath=%s", - err, - workspaceInfo.Agent.DataPath, - ) - return false, nil, err - } - } - } - - // check content folder for local folder workspace source - // - // We don't want to initialize the content folder with the value of the local workspace folder - // if we're running in proxy mode. - // We only have write access to /var/lib/loft/* by default causing nearly all local folders - // to run into permissions issues - if workspaceInfo.Workspace.Source.LocalFolder != "" && - !workspaceInfo.CLIOptions.Platform.Enabled { - log.Debugf( - "checking local folder workspace source: localFolder=%s, workspaceOrigin=%s", - workspaceInfo.Workspace.Source.LocalFolder, - workspaceInfo.WorkspaceOrigin, + workspaceDir, err = handleStaleWorkspace( + workspaceInfo, workspaceDir, workspaceConfig, deleteWorkspace, ) - - _, err = os.Stat(workspaceInfo.WorkspaceOrigin) - if err == nil { - workspaceInfo.ContentFolder = workspaceInfo.Workspace.Source.LocalFolder - log.Debugf( - "set content folder to local folder: contentFolder=%s", - workspaceInfo.ContentFolder, - ) - } else { - log.Debugf( - "workspace origin not accessible: error=%v, workspaceOrigin=%s", - err, - workspaceInfo.WorkspaceOrigin, - ) + if err != nil { + return false, nil, err } } - // set content folder - if workspaceInfo.ContentFolder == "" { - workspaceInfo.ContentFolder = GetAgentWorkspaceContentDir(workspaceDir) - log.Debugf( - "set content folder to default location: contentFolder=%s", - workspaceInfo.ContentFolder, - ) - } + resolveContentFolder(workspaceInfo, workspaceDir) - // write workspace info if writeInfo { - log.Debugf("writing workspace info to file: configFile=%s", workspaceConfig) - - err = writeWorkspaceInfo(workspaceConfig, workspaceInfo) - if err != nil { - log.Errorf( - "failed to write workspace info: error=%v, configFile=%s", - err, - workspaceConfig, - ) - return false, nil, err + if err := writeWorkspaceInfo(workspaceConfig, workspaceInfo); err != nil { + return false, nil, fmt.Errorf("write workspace info: %w", err) } - - log.Debugf("wrote workspace info: configFile=%s", workspaceConfig) } workspaceInfo.Origin = workspaceDir - log.Debugf( - "processed workspace info: workspaceId=%s, origin=%s, contentFolder=%s", + return false, workspaceInfo, nil +} + +// handleStaleWorkspace deletes a workspace whose persisted UID no longer +// matches the incoming one, then recreates the workspace dir. Returns the +// (possibly new) workspaceDir. +func handleStaleWorkspace( + workspaceInfo *provider2.AgentWorkspaceInfo, + workspaceDir, workspaceConfig string, + deleteWorkspace func(*provider2.AgentWorkspaceInfo) error, +) (string, error) { + oldWorkspaceInfo, _ := ParseAgentWorkspaceInfo(workspaceConfig) + if oldWorkspaceInfo == nil || + oldWorkspaceInfo.Workspace.UID == workspaceInfo.Workspace.UID { + return workspaceDir, nil + } + + log.Infof( + "delete old workspace: workspaceId=%s, oldUid=%s, newUid=%s", + oldWorkspaceInfo.Workspace.ID, + oldWorkspaceInfo.Workspace.UID, + workspaceInfo.Workspace.UID, + ) + if err := deleteWorkspace(oldWorkspaceInfo); err != nil { + return "", fmt.Errorf("delete old workspace: %w", err) + } + + newDir, err := CreateAgentWorkspaceDir( + workspaceInfo.Agent.DataPath, + workspaceInfo.Workspace.Context, workspaceInfo.Workspace.ID, - workspaceInfo.Origin, - workspaceInfo.ContentFolder, ) + if err != nil { + return "", fmt.Errorf("recreate workspace dir: %w", err) + } - return false, workspaceInfo, nil + // Drop the old UID's path so resolveContentFolder picks up the new one. + workspaceInfo.ContentFolder = "" + return newDir, nil +} + +// resolveContentFolder fills in workspaceInfo.ContentFolder, preferring a +// LocalFolder source when accessible. Skipped under Platform.Enabled because +// the host's local folder path is not addressable from the managed runner. +func resolveContentFolder( + workspaceInfo *provider2.AgentWorkspaceInfo, + workspaceDir string, +) { + if workspaceInfo.Workspace.Source.LocalFolder != "" && + !workspaceInfo.CLIOptions.Platform.Enabled { + if _, err := os.Stat(workspaceInfo.WorkspaceOrigin); err == nil { + workspaceInfo.ContentFolder = workspaceInfo.Workspace.Source.LocalFolder + } + } + if workspaceInfo.ContentFolder == "" { + workspaceInfo.ContentFolder = GetAgentWorkspaceContentDir( + workspaceDir, + workspaceInfo.Workspace.UID, + ) + } } func CreateWorkspaceBusyFile(folder string) { - filePath := filepath.Join(folder, WorkspaceBusyFile) + filePath := filepath.Join(folder, config.WorkspaceBusyFile) _, err := os.Stat(filePath) if err == nil { return @@ -379,13 +283,13 @@ func CreateWorkspaceBusyFile(folder string) { } func HasWorkspaceBusyFile(folder string) bool { - filePath := filepath.Join(folder, WorkspaceBusyFile) + filePath := filepath.Join(folder, config.WorkspaceBusyFile) _, err := os.Stat(filePath) return err == nil } func DeleteWorkspaceBusyFile(folder string) { - _ = os.Remove(filepath.Join(folder, WorkspaceBusyFile)) + _ = os.Remove(filepath.Join(folder, config.WorkspaceBusyFile)) } func writeWorkspaceInfo(file string, workspaceInfo *provider2.AgentWorkspaceInfo) error { @@ -475,46 +379,40 @@ type Exec func( stdin io.Reader, stdout io.Writer, stderr io.Writer, ) error -func Tunnel( - ctx context.Context, - exec Exec, - user string, - stdin io.Reader, - stdout io.Writer, - stderr io.Writer, - timeout time.Duration, -) error { - err := InjectAgent(&InjectOptions{ +type TunnelOptions struct { + Exec Exec + User string + Stdin io.Reader + Stdout io.Writer + Stderr io.Writer + Timeout time.Duration +} + +func Tunnel(ctx context.Context, opts TunnelOptions) error { + if err := InjectAgent(&InjectOptions{ Ctx: ctx, Exec: func(ctx context.Context, command string, stdin io.Reader, stdout io.Writer, stderr io.Writer) error { - return exec(ctx, "root", command, stdin, stdout, stderr) + return opts.Exec(ctx, "root", command, stdin, stdout, stderr) }, IsLocal: false, - RemoteAgentPath: ContainerDevsyHelperLocation, - DownloadURL: DefaultAgentDownloadURL(), + RemoteAgentPath: config.ContainerDevsyHelperLocation, + DownloadURL: config.DefaultAgentDownloadURL(), PreferDownloadFromRemoteUrl: Bool(false), - Timeout: timeout, - }) - if err != nil { + Timeout: opts.Timeout, + }); err != nil { return err } - // build command - command := fmt.Sprintf("'%s' internal ssh-server --stdio", ContainerDevsyHelperLocation) + command := fmt.Sprintf("'%s' internal ssh-server --stdio", config.ContainerDevsyHelperLocation) if log.DebugEnabled() { command += " --debug" } + user := opts.User if user == "" { user = "root" } - // create tunnel - err = exec(ctx, user, command, stdin, stdout, stderr) - if err != nil { - return err - } - - return nil + return opts.Exec(ctx, user, command, opts.Stdin, opts.Stdout, opts.Stderr) } func dockerReachable(dockerOverride string, envs map[string]string) (bool, error) { diff --git a/pkg/agent/agent_test.go b/pkg/agent/agent_test.go deleted file mode 100644 index 8120e547f..000000000 --- a/pkg/agent/agent_test.go +++ /dev/null @@ -1,48 +0,0 @@ -package agent - -import ( - "os" - "testing" - - "github.com/devsy-org/devsy/pkg/config" - "github.com/stretchr/testify/suite" -) - -type AgentTestSuite struct { - suite.Suite - originalEnv string -} - -func (s *AgentTestSuite) SetupTest() { - s.originalEnv = os.Getenv(config.EnvAgentURL) -} - -func (s *AgentTestSuite) TearDownTest() { - if s.originalEnv != "" { - _ = os.Setenv(config.EnvAgentURL, s.originalEnv) - } else { - _ = os.Unsetenv(config.EnvAgentURL) - } -} - -func (s *AgentTestSuite) TestDefaultAgentDownloadURL_NoTrailingSlash() { - _ = os.Setenv(config.EnvAgentURL, "https://example.com/releases/latest/download") - result := DefaultAgentDownloadURL() - s.Equal("https://example.com/releases/latest/download", result) -} - -func (s *AgentTestSuite) TestDefaultAgentDownloadURL_SingleTrailingSlash() { - _ = os.Setenv(config.EnvAgentURL, "https://example.com/releases/latest/download/") - result := DefaultAgentDownloadURL() - s.Equal("https://example.com/releases/latest/download", result) -} - -func (s *AgentTestSuite) TestDefaultAgentDownloadURL_MultipleTrailingSlashes() { - _ = os.Setenv(config.EnvAgentURL, "https://example.com/releases/latest/download///") - result := DefaultAgentDownloadURL() - s.Equal("https://example.com/releases/latest/download", result) -} - -func TestAgentSuite(t *testing.T) { - suite.Run(t, new(AgentTestSuite)) -} diff --git a/pkg/agent/delivery/kubernetes.go b/pkg/agent/delivery/kubernetes.go index 9744f92b8..e778bd7bb 100644 --- a/pkg/agent/delivery/kubernetes.go +++ b/pkg/agent/delivery/kubernetes.go @@ -4,7 +4,7 @@ import ( "context" "fmt" - "github.com/devsy-org/devsy/pkg/agent" + pkgconfig "github.com/devsy-org/devsy/pkg/config" "github.com/devsy-org/devsy/pkg/inject" "github.com/devsy-org/devsy/pkg/log" ) @@ -37,7 +37,7 @@ func (d *KubernetesDelivery) DeliverPostStart(ctx context.Context, opts PostStar } defer func() { _ = binary.Close() }() - destPath := agent.ContainerDevsyHelperLocation + destPath := pkgconfig.ContainerDevsyHelperLocation script := fmt.Sprintf( `set -e; t=$(mktemp %s.XXXXXX); cat > "$t" && chmod 755 "$t" && mv "$t" %s || { rm -f "$t"; exit 1; }`, destPath, diff --git a/pkg/agent/delivery/kubernetes_test.go b/pkg/agent/delivery/kubernetes_test.go index cc6c9b56f..28e4152e4 100644 --- a/pkg/agent/delivery/kubernetes_test.go +++ b/pkg/agent/delivery/kubernetes_test.go @@ -8,7 +8,7 @@ import ( "strings" "testing" - "github.com/devsy-org/devsy/pkg/agent" + pkgconfig "github.com/devsy-org/devsy/pkg/config" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -68,7 +68,7 @@ func TestKubernetesDelivery_DeliverPostStart_WritesBinary(t *testing.T) { require.NoError(t, err) - destPath := agent.ContainerDevsyHelperLocation + destPath := pkgconfig.ContainerDevsyHelperLocation assert.Contains(t, capturedCmd, "cat >") assert.Contains(t, capturedCmd, destPath) assert.Contains(t, capturedCmd, "chmod 755") diff --git a/pkg/agent/delivery/legacy_shell.go b/pkg/agent/delivery/legacy_shell.go index edcacf4ab..7e498c30e 100644 --- a/pkg/agent/delivery/legacy_shell.go +++ b/pkg/agent/delivery/legacy_shell.go @@ -6,6 +6,7 @@ import ( "io" "github.com/devsy-org/devsy/pkg/agent" + pkgconfig "github.com/devsy-org/devsy/pkg/config" "github.com/devsy-org/devsy/pkg/inject" "github.com/devsy-org/devsy/pkg/log" ) @@ -38,7 +39,7 @@ func (d *LegacyShellDelivery) DeliverPostStart(ctx context.Context, opts PostSta Ctx: ctx, Exec: d.ExecFunc, IsLocal: false, - RemoteAgentPath: agent.ContainerDevsyHelperLocation, + RemoteAgentPath: pkgconfig.ContainerDevsyHelperLocation, DownloadURL: d.downloadURL(), PreferDownloadFromRemoteUrl: agent.Bool(false), } @@ -67,7 +68,7 @@ func (d *LegacyShellDelivery) downloadURL() string { if d.DownloadURL != "" { return d.DownloadURL } - return agent.DefaultAgentDownloadURL() + return pkgconfig.DefaultAgentDownloadURL() } // ExecFuncFromDriver creates an inject.ExecFunc that routes commands through diff --git a/pkg/agent/delivery/local_docker.go b/pkg/agent/delivery/local_docker.go index fd59522a8..f22607fe3 100644 --- a/pkg/agent/delivery/local_docker.go +++ b/pkg/agent/delivery/local_docker.go @@ -10,7 +10,7 @@ import ( "path/filepath" "strings" - "github.com/devsy-org/devsy/pkg/agent" + pkgconfig "github.com/devsy-org/devsy/pkg/config" "github.com/devsy-org/devsy/pkg/devcontainer/config" "github.com/devsy-org/devsy/pkg/log" "github.com/devsy-org/devsy/pkg/version" @@ -289,5 +289,5 @@ func (d *LocalDockerDelivery) dockerCommand() string { } func binaryName() string { - return agent.ContainerDevsyHelperLocation[len("/usr/local/bin/"):] + return pkgconfig.ContainerDevsyHelperLocation[len("/usr/local/bin/"):] } diff --git a/pkg/agent/delivery/remote_docker.go b/pkg/agent/delivery/remote_docker.go index b646bf068..ac47f7187 100644 --- a/pkg/agent/delivery/remote_docker.go +++ b/pkg/agent/delivery/remote_docker.go @@ -8,7 +8,7 @@ import ( "os/exec" "path" - "github.com/devsy-org/devsy/pkg/agent" + pkgconfig "github.com/devsy-org/devsy/pkg/config" "github.com/devsy-org/devsy/pkg/log" ) @@ -39,7 +39,7 @@ func (d *RemoteDockerDelivery) DeliverPostStart(ctx context.Context, opts PostSt return fmt.Errorf("container ID is required for remote docker delivery") } - destPath := agent.ContainerDevsyHelperLocation + destPath := pkgconfig.ContainerDevsyHelperLocation if err := d.ensureDir(ctx, destPath); err != nil { return fmt.Errorf("ensure target directory: %w", err) diff --git a/pkg/agent/inject.go b/pkg/agent/inject.go index e6824fbe7..3082d55ba 100644 --- a/pkg/agent/inject.go +++ b/pkg/agent/inject.go @@ -43,9 +43,9 @@ type InjectOptions struct { // IsLocal indicates if the injection target is the local machine. IsLocal bool // RemoteAgentPath is the path where the agent binary should be placed on the remote machine. - // Defaults to RemoteDevsyHelperLocation. + // Defaults to config.RemoteDevsyHelperLocation. RemoteAgentPath string - // DownloadURL is the base URL to download the agent binary from. Defaults to DefaultAgentDownloadURL(). + // DownloadURL is the base URL to download the agent binary from. Defaults to config.DefaultAgentDownloadURL(). DownloadURL string // PreferDownloadFromRemoteUrl forces downloading the agent even if a local binary is available. // Defaults to true for release versions, false for dev versions. @@ -93,7 +93,7 @@ func (o *InjectOptions) Validate() error { func (o *InjectOptions) applyPathDefaults() { if o.RemoteAgentPath == "" { - o.RemoteAgentPath = RemoteDevsyHelperLocation + o.RemoteAgentPath = config.RemoteDevsyHelperLocation } if o.Timeout == 0 { o.Timeout = waitForInstanceConnectionTimeout @@ -108,7 +108,7 @@ func (o *InjectOptions) applyPathDefaults() { func (o *InjectOptions) applyURLDefaults() { if o.DownloadURL == "" { - o.DownloadURL = DefaultAgentDownloadURL() + o.DownloadURL = config.DefaultAgentDownloadURL() } if strings.Contains(o.DownloadURL, "github.com") && @@ -133,7 +133,7 @@ func (o *InjectOptions) applyPreferDownloadDefaults() { return } - isDefaultURL := o.DownloadURL == DefaultAgentDownloadURL() + isDefaultURL := o.DownloadURL == config.DefaultAgentDownloadURL() hasCustomAgentURL := os.Getenv(config.EnvAgentURL) != "" || !isDefaultURL preferDownloadEnv := os.Getenv(config.EnvAgentPreferDownload) diff --git a/pkg/agent/workspace.go b/pkg/agent/workspace.go index 630265792..b1b8b5aaf 100644 --- a/pkg/agent/workspace.go +++ b/pkg/agent/workspace.go @@ -32,7 +32,7 @@ var extraSearchLocations = []string{ "/home/devsy/" + config.ConfigDirName + "/agent", "/opt/devsy/agent", "/var/lib/devsy/agent", - ContainerDataDir + "/agent", + config.ContainerDataDir + "/agent", } // EnvAgentInContainer is set to "1" by every SSH command builder that @@ -215,8 +215,11 @@ echo Devsy return true, nil } -func GetAgentWorkspaceContentDir(workspaceDir string) string { - return filepath.Join(workspaceDir, "content") +// GetAgentWorkspaceContentDir returns the bind-mount source for a workspace. +// The UID suffix ensures a delete-then-recreate cycle never reuses the same +// host path, avoiding Docker Desktop's stale /host_mnt inode cache. +func GetAgentWorkspaceContentDir(workspaceDir, uid string) string { + return filepath.Join(workspaceDir, "content-"+uid) } func GetAgentBinariesDirFromWorkspaceDir(workspaceDir string) (string, error) { diff --git a/pkg/config/paths.go b/pkg/config/paths.go index c94a4adfd..ac158f854 100644 --- a/pkg/config/paths.go +++ b/pkg/config/paths.go @@ -19,4 +19,19 @@ const ( // DaemonProcessName is the name used for the fallback background daemon process // PID file and lock file in os.TempDir(). DaemonProcessName = BinaryName + ".daemon" + + // ContainerDataDir is the base directory for Devsy data inside containers. + ContainerDataDir = "/var/" + BinaryName + + // ContainerDevsyHelperLocation is where the Devsy agent binary lives inside containers. + ContainerDevsyHelperLocation = "/usr/local/bin/" + BinaryName + + // RemoteDevsyHelperLocation is the staging path for the Devsy agent on remote hosts. + RemoteDevsyHelperLocation = "/tmp/" + BinaryName + + // ContainerActivityFile is touched by SSH/fleet servers to record container liveness. + ContainerActivityFile = "/tmp/" + BinaryName + ".activity" + + // WorkspaceBusyFile is the per-workspace lock file written under the workspace folder. + WorkspaceBusyFile = "workspace.lock" ) diff --git a/pkg/config/repo.go b/pkg/config/repo.go index f91e0a377..4a821845f 100644 --- a/pkg/config/repo.go +++ b/pkg/config/repo.go @@ -1,5 +1,12 @@ package config +import ( + "os" + "strings" + + "github.com/devsy-org/devsy/pkg/version" +) + const ( RepoOwner = "devsy-org" RepoName = "devsy" @@ -23,4 +30,23 @@ const ( // WebsiteAssetsURL is the base URL for icon/image assets. WebsiteAssetsURL = WebsiteBaseURL + "/assets" + + // AgentDownloadBaseURL is the prefix under which versioned agent binaries are published. + AgentDownloadBaseURL = GitHubReleasesURL + "/download/" + + // AgentLatestDownloadURL points at the floating "latest" agent release. + AgentLatestDownloadURL = GitHubReleasesURL + "/latest/download" ) + +// DefaultAgentDownloadURL returns the URL the host should download the agent +// binary from. Honors the DEVSY_AGENT_URL override; otherwise uses the +// version-pinned release URL, falling back to "latest" in dev builds. +func DefaultAgentDownloadURL() string { + if override := os.Getenv(EnvAgentURL); override != "" { + return strings.TrimRight(override, "/") + } + if version.GetVersion() == version.DevVersion { + return AgentLatestDownloadURL + } + return AgentDownloadBaseURL + version.GetVersion() +} diff --git a/pkg/config/repo_test.go b/pkg/config/repo_test.go new file mode 100644 index 000000000..d337c2d68 --- /dev/null +++ b/pkg/config/repo_test.go @@ -0,0 +1,44 @@ +package config + +import ( + "os" + "testing" + + "github.com/stretchr/testify/suite" +) + +type RepoTestSuite struct { + suite.Suite + originalEnv string +} + +func (s *RepoTestSuite) SetupTest() { + s.originalEnv = os.Getenv(EnvAgentURL) +} + +func (s *RepoTestSuite) TearDownTest() { + if s.originalEnv != "" { + _ = os.Setenv(EnvAgentURL, s.originalEnv) + } else { + _ = os.Unsetenv(EnvAgentURL) + } +} + +func (s *RepoTestSuite) TestDefaultAgentDownloadURL_NoTrailingSlash() { + _ = os.Setenv(EnvAgentURL, "https://example.com/releases/latest/download") + s.Equal("https://example.com/releases/latest/download", DefaultAgentDownloadURL()) +} + +func (s *RepoTestSuite) TestDefaultAgentDownloadURL_SingleTrailingSlash() { + _ = os.Setenv(EnvAgentURL, "https://example.com/releases/latest/download/") + s.Equal("https://example.com/releases/latest/download", DefaultAgentDownloadURL()) +} + +func (s *RepoTestSuite) TestDefaultAgentDownloadURL_MultipleTrailingSlashes() { + _ = os.Setenv(EnvAgentURL, "https://example.com/releases/latest/download///") + s.Equal("https://example.com/releases/latest/download", DefaultAgentDownloadURL()) +} + +func TestRepoSuite(t *testing.T) { + suite.Run(t, new(RepoTestSuite)) +} diff --git a/pkg/devcontainer/setup.go b/pkg/devcontainer/setup.go index f388514a9..a6ec8bf74 100644 --- a/pkg/devcontainer/setup.go +++ b/pkg/devcontainer/setup.go @@ -16,6 +16,7 @@ import ( "github.com/devsy-org/devsy/pkg/agent/delivery" "github.com/devsy-org/devsy/pkg/agent/tunnelserver" "github.com/devsy-org/devsy/pkg/compress" + pkgconfig "github.com/devsy-org/devsy/pkg/config" "github.com/devsy-org/devsy/pkg/devcontainer/config" "github.com/devsy-org/devsy/pkg/devcontainer/crane" "github.com/devsy-org/devsy/pkg/devcontainer/sshtunnel" @@ -123,7 +124,7 @@ func (r *runner) deliverPostStart(ctx context.Context, strategy delivery.AgentDe func (r *runner) newBinarySource() (delivery.BinarySourceFunc, error) { downloadURL := r.AgentDownloadURL if downloadURL == "" { - downloadURL = agent.DefaultAgentDownloadURL() + downloadURL = pkgconfig.DefaultAgentDownloadURL() } mgr, err := agent.NewBinaryManager(downloadURL) if err != nil { @@ -147,8 +148,8 @@ func (r *runner) legacyInject(ctx context.Context, timeout time.Duration) error ) }, IsLocal: false, - RemoteAgentPath: agent.ContainerDevsyHelperLocation, - DownloadURL: agent.DefaultAgentDownloadURL(), + RemoteAgentPath: pkgconfig.ContainerDevsyHelperLocation, + DownloadURL: pkgconfig.DefaultAgentDownloadURL(), PreferDownloadFromRemoteUrl: agent.Bool(false), Timeout: timeout, }) @@ -252,7 +253,7 @@ func (r *runner) compressWorkspaceConfig() (string, error) { func (r *runner) buildSetupCommand(compressed, workspaceConfigCompressed string) string { log.Infof("setting up container") args := []string{ - shellescape.Quote(agent.ContainerDevsyHelperLocation), + shellescape.Quote(pkgconfig.ContainerDevsyHelperLocation), "internal", "agent", "container", "setup", "--setup-info", shellescape.Quote(compressed), "--container-workspace-info", shellescape.Quote(workspaceConfigCompressed), @@ -375,7 +376,7 @@ func (r *runner) executeSetup( func (r *runner) buildSSHTunnelCommand() string { args := []string{ - shellescape.Quote(agent.ContainerDevsyHelperLocation), + shellescape.Quote(pkgconfig.ContainerDevsyHelperLocation), "internal", "ssh-server", "--stdio", } diff --git a/pkg/devcontainer/setup/setup.go b/pkg/devcontainer/setup/setup.go index 3ea0bb5cd..05d5c0021 100644 --- a/pkg/devcontainer/setup/setup.go +++ b/pkg/devcontainer/setup/setup.go @@ -15,7 +15,6 @@ import ( "strings" "github.com/devsy-org/api/pkg/devsy" - "github.com/devsy-org/devsy/pkg/agent" "github.com/devsy-org/devsy/pkg/agent/tunnel" "github.com/devsy-org/devsy/pkg/command" pkgconfig "github.com/devsy-org/devsy/pkg/config" @@ -410,7 +409,7 @@ func shouldSkipKubeConfig(tunnelClient tunnel.TunnelClient) bool { return true } - markerPath := filepath.Join(agent.ContainerDataDir, "setupKubeConfig.marker") + markerPath := filepath.Join(pkgconfig.ContainerDataDir, "setupKubeConfig.marker") info, err := os.Stat(markerPath) if err == nil { if info.Mode().Perm()&0o022 != 0 { @@ -492,7 +491,7 @@ func ensureKubeConfigMaps(config *clientcmdapi.Config) *clientcmdapi.Config { } func markerFileExists(markerName string, markerContent string) (bool, error) { - markerName = filepath.Join(agent.ContainerDataDir, markerName+".marker") + markerName = filepath.Join(pkgconfig.ContainerDataDir, markerName+".marker") t, err := os.ReadFile(markerName) if err != nil && !os.IsNotExist(err) { return false, err diff --git a/pkg/dotfiles/dotfiles.go b/pkg/dotfiles/dotfiles.go index 88bc6b0d1..40e7680c9 100644 --- a/pkg/dotfiles/dotfiles.go +++ b/pkg/dotfiles/dotfiles.go @@ -6,7 +6,6 @@ import ( "slices" "strings" - "github.com/devsy-org/devsy/pkg/agent" client2 "github.com/devsy-org/devsy/pkg/client" "github.com/devsy-org/devsy/pkg/config" config2 "github.com/devsy-org/devsy/pkg/devcontainer/config" @@ -170,7 +169,7 @@ func buildDotCmd(p buildDotCmdParams) (*exec.Cmd, error) { p.client.Workspace(), "--log-output=raw", "--command", - agent.ContainerDevsyHelperLocation+" "+strings.Join(agentArguments, " "), + config.ContainerDevsyHelperLocation+" "+strings.Join(agentArguments, " "), ) execPath, err := os.Executable() if err != nil { diff --git a/pkg/driver/docker/diagnostics.go b/pkg/driver/docker/diagnostics.go new file mode 100644 index 000000000..34231fc54 --- /dev/null +++ b/pkg/driver/docker/diagnostics.go @@ -0,0 +1,103 @@ +package docker + +import ( + "bytes" + "context" + "fmt" + "io" + "os" + "runtime" + "strings" + "sync" + "time" + + "github.com/devsy-org/devsy/pkg/docker" + "github.com/devsy-org/devsy/pkg/log" +) + +const mountFlag = "--mount" + +func extractBindSources(args []string) []string { + var srcs []string + for i, a := range args { + if spec, ok := mountSpec(a, args, i); ok { + srcs = appendBindSrc(srcs, spec) + } + } + return srcs +} + +func mountSpec(arg string, args []string, i int) (string, bool) { + if arg == mountFlag && i+1 < len(args) { + return args[i+1], true + } + if rest, ok := strings.CutPrefix(arg, "--mount="); ok { + return rest, true + } + return "", false +} + +func appendBindSrc(dst []string, spec string) []string { + if !strings.Contains(spec, "type=bind") { + return dst + } + for part := range strings.SplitSeq(spec, ",") { + if v, ok := strings.CutPrefix(part, "src="); ok { + return append(dst, v) + } + if v, ok := strings.CutPrefix(part, "source="); ok { + return append(dst, v) + } + } + return dst +} + +// logBindSources records host-side resolution of each bind source. A +// "host=exists" line followed by docker reporting the path missing is the +// fingerprint of a Docker Desktop file-share cache race. +func logBindSources(args []string) { + for _, src := range extractBindSources(args) { + _, err := os.Lstat(src) + log.Infof("docker bind: src=%s exists=%t", src, err == nil) + } +} + +var hostEnvOnce sync.Once + +func logHostEnvOnce(ctx context.Context, helper *docker.DockerHelper) { + hostEnvOnce.Do(func() { + log.Infof("docker host: %s", collectHostEnv(ctx, helper)) + }) +} + +func collectHostEnv(ctx context.Context, helper *docker.DockerHelper) string { + parts := []string{fmt.Sprintf("os=%s/%s", runtime.GOOS, runtime.GOARCH)} + if helper != nil { + if v := dockerInfoField(ctx, helper, "{{.ServerVersion}}"); v != "" { + parts = append(parts, "server="+v) + } + if v := dockerInfoField(ctx, helper, "{{.Driver}}"); v != "" { + parts = append(parts, "storageDriver="+v) + } + } + return strings.Join(parts, " ") +} + +func dockerInfoField(ctx context.Context, helper *docker.DockerHelper, tmpl string) string { + if ctx == nil { + ctx = context.Background() + } + tctx, cancel := context.WithTimeout(ctx, 3*time.Second) + defer cancel() + var out bytes.Buffer + if err := helper.Run( + tctx, + []string{"info", "--format", tmpl}, + nil, + &out, + io.Discard, + ); err != nil { + return "" + } + return strings.TrimSpace(out.String()) +} diff --git a/pkg/driver/docker/diagnostics_test.go b/pkg/driver/docker/diagnostics_test.go new file mode 100644 index 000000000..42c150ca4 --- /dev/null +++ b/pkg/driver/docker/diagnostics_test.go @@ -0,0 +1,22 @@ +package docker + +import ( + "reflect" + "testing" +) + +func TestExtractBindSources(t *testing.T) { + args := []string{ + "run", "--sig-proxy=false", + mountFlag, "type=bind,source=/a/b,target=/x,consistency=consistent", + mountFlag, "type=bind,src=/c/d,dst=/y", + "--mount=type=bind,src=/e/f,dst=/z", + mountFlag, "type=volume,source=myvol,target=/v", // not a bind, ignored + "alpine", + } + got := extractBindSources(args) + want := []string{"/a/b", "/c/d", "/e/f"} + if !reflect.DeepEqual(got, want) { + t.Errorf("extractBindSources() = %v, want %v", got, want) + } +} diff --git a/pkg/driver/docker/docker.go b/pkg/driver/docker/docker.go index 155433d1b..5526b2fe6 100644 --- a/pkg/driver/docker/docker.go +++ b/pkg/driver/docker/docker.go @@ -807,6 +807,9 @@ func (d *dockerDriver) startContainer( dir, ) + logHostEnvOnce(ctx, d.Docker) + logBindSources(args) + err := d.Docker.RunWithDir(ctx, dir, args, nil, writer, writer) if err != nil { log.Errorf( diff --git a/pkg/ide/jetbrains/generic.go b/pkg/ide/jetbrains/generic.go index 455704822..34b976125 100644 --- a/pkg/ide/jetbrains/generic.go +++ b/pkg/ide/jetbrains/generic.go @@ -11,7 +11,6 @@ import ( "path/filepath" "runtime" - "github.com/devsy-org/devsy/pkg/agent" "github.com/devsy-org/devsy/pkg/command" config2 "github.com/devsy-org/devsy/pkg/config" copy2 "github.com/devsy-org/devsy/pkg/copy" @@ -110,7 +109,7 @@ func (o *GenericJetBrainsServer) GetVolume() string { } func (o *GenericJetBrainsServer) getDownloadFolder() string { - return filepath.Join(agent.ContainerDataDir, o.options.ID) + return filepath.Join(config2.ContainerDataDir, o.options.ID) } func (o *GenericJetBrainsServer) Install(setupInfo *config.Result) error { diff --git a/pkg/ide/rstudio/rstudio.go b/pkg/ide/rstudio/rstudio.go index ae88a719e..b27614647 100644 --- a/pkg/ide/rstudio/rstudio.go +++ b/pkg/ide/rstudio/rstudio.go @@ -14,7 +14,6 @@ import ( "runtime" "strings" - "github.com/devsy-org/devsy/pkg/agent" "github.com/devsy-org/devsy/pkg/command" "github.com/devsy-org/devsy/pkg/config" copypkg "github.com/devsy-org/devsy/pkg/copy" @@ -38,7 +37,7 @@ var Options = ide.Options{ const ( DefaultServerPort = 8787 - downloadFolder = agent.ContainerDataDir + "/rstudio-server" + downloadFolder = config.ContainerDataDir + "/rstudio-server" dataFolder = "/usr/local/share/devsy/rstudio-server/data" // rstudioConfigFolder is where RStudio expects configuration. rstudioConfigFolder = "/etc/rstudio" diff --git a/pkg/options/resolve.go b/pkg/options/resolve.go index c780e99e3..7aa58aa3c 100644 --- a/pkg/options/resolve.go +++ b/pkg/options/resolve.go @@ -8,7 +8,6 @@ import ( "reflect" "strings" - "github.com/devsy-org/devsy/pkg/agent" "github.com/devsy-org/devsy/pkg/config" "github.com/devsy-org/devsy/pkg/options/resolver" "github.com/devsy-org/devsy/pkg/provider" @@ -335,7 +334,7 @@ func resolveAgentPathAndURL( } } if agentConfig.Path == "" { - agentConfig.Path = agent.RemoteDevsyHelperLocation + agentConfig.Path = config.RemoteDevsyHelperLocation } agentConfig.DownloadURL = resolver.ResolveDefaultValue(agentConfig.DownloadURL, options) if agentConfig.DownloadURL == "" { @@ -383,7 +382,7 @@ func resolveAgentDownloadURL(devConfig *config.Config) string { return strings.TrimSuffix(contextAgentOption.Value, "/") + "/" } - return agent.DefaultAgentDownloadURL() + return config.DefaultAgentDownloadURL() } func filterResolvedOptions( diff --git a/pkg/ssh/server/ssh.go b/pkg/ssh/server/ssh.go index 5e61da49b..ab4a5adb3 100644 --- a/pkg/ssh/server/ssh.go +++ b/pkg/ssh/server/ssh.go @@ -1,6 +1,7 @@ package server import ( + "context" "crypto/rand" "crypto/sha256" "encoding/hex" @@ -94,6 +95,10 @@ const ( type Server interface { Serve(listener net.Listener) error ListenAndServe() error + // Shutdown gracefully closes the listener and waits for active + // connections to finish, bounded by ctx. Use Close for an immediate + // termination. + Shutdown(ctx context.Context) error } type server struct { @@ -375,6 +380,10 @@ func (s *server) ListenAndServe() error { return s.sshServer.ListenAndServe() } +func (s *server) Shutdown(ctx context.Context) error { + return s.sshServer.Shutdown(ctx) +} + // connCallback is invoked once per inbound SSH connection. Outside the // explicit reuseSock (openvscode backhaul) mode it stores a lightweight // intent on the ssh.Context and schedules a teardown goroutine. The agent diff --git a/pkg/ssh/server/ssh_container.go b/pkg/ssh/server/ssh_container.go index ffe6350ec..ac5cdaeda 100644 --- a/pkg/ssh/server/ssh_container.go +++ b/pkg/ssh/server/ssh_container.go @@ -1,6 +1,7 @@ package server import ( + "context" "fmt" "net" "os" @@ -73,6 +74,10 @@ func (s *containerServer) Serve(listener net.Listener) error { return s.sshServer.Serve(listener) } +func (s *containerServer) Shutdown(ctx context.Context) error { + return s.sshServer.Shutdown(ctx) +} + func (s *containerServer) ListenAndServe() error { log.Debugf("Start ssh server on %s", s.sshServer.Addr) return s.sshServer.ListenAndServe() diff --git a/pkg/tunnel/services.go b/pkg/tunnel/services.go index 48ceb27a9..afba69008 100644 --- a/pkg/tunnel/services.go +++ b/pkg/tunnel/services.go @@ -147,7 +147,7 @@ func buildCredentialsCommand(opts RunServicesOptions) string { command := fmt.Sprintf( "%s%s internal agent container credentials-server --user %s", agent.ContainerAgentEnvPrefix, - shellescape.Quote(agent.ContainerDevsyHelperLocation), + shellescape.Quote(config.ContainerDevsyHelperLocation), shellescape.Quote(opts.User), ) if opts.ConfigureGitCredentials {