From d61353f4687f2a71c7eb5bd17b88f0a5a120e1b0 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Fri, 20 Jun 2025 16:34:30 +0100 Subject: [PATCH] fix(agent/agentcontainers): read WorkspaceFolder from config (#18467) Instead of exec'ing `pwd` inside of the container, we instead read `WorkspaceFolder` from the outcome of `read-configuration`. --- agent/agentcontainers/api.go | 31 +++++---------------- agent/agentcontainers/api_test.go | 35 ++++++------------------ agent/agentcontainers/devcontainercli.go | 5 ++++ 3 files changed, 20 insertions(+), 51 deletions(-) diff --git a/agent/agentcontainers/api.go b/agent/agentcontainers/api.go index ddf98e38bd..ef2b7aa7eb 100644 --- a/agent/agentcontainers/api.go +++ b/agent/agentcontainers/api.go @@ -1,11 +1,9 @@ package agentcontainers import ( - "bytes" "context" "errors" "fmt" - "io" "net/http" "os" "path" @@ -1114,27 +1112,6 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c if proc.agent.ID == uuid.Nil || maybeRecreateSubAgent { subAgentConfig.Architecture = arch - // Detect workspace folder by executing `pwd` in the container. - // NOTE(mafredri): This is a quick and dirty way to detect the - // workspace folder inside the container. In the future we will - // rely more on `devcontainer read-configuration`. - var pwdBuf bytes.Buffer - err = api.dccli.Exec(ctx, dc.WorkspaceFolder, dc.ConfigPath, "pwd", []string{}, - WithExecOutput(&pwdBuf, io.Discard), - WithExecContainerID(container.ID), - ) - if err != nil { - return xerrors.Errorf("check workspace folder in container: %w", err) - } - directory := strings.TrimSpace(pwdBuf.String()) - if directory == "" { - logger.Warn(ctx, "detected workspace folder is empty, using default workspace folder", - slog.F("default_workspace_folder", DevcontainerDefaultContainerWorkspaceFolder), - ) - directory = DevcontainerDefaultContainerWorkspaceFolder - } - subAgentConfig.Directory = directory - displayAppsMap := map[codersdk.DisplayApp]bool{ // NOTE(DanielleMaywood): // We use the same defaults here as set in terraform-provider-coder. @@ -1146,7 +1123,10 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c codersdk.DisplayAppPortForward: true, } - var appsWithPossibleDuplicates []SubAgentApp + var ( + appsWithPossibleDuplicates []SubAgentApp + workspaceFolder = DevcontainerDefaultContainerWorkspaceFolder + ) if err := func() error { var ( @@ -1167,6 +1147,8 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c return err } + workspaceFolder = config.Workspace.WorkspaceFolder + // NOTE(DanielleMaywood): // We only want to take an agent name specified in the root customization layer. // This restricts the ability for a feature to specify the agent name. We may revisit @@ -1241,6 +1223,7 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c subAgentConfig.DisplayApps = displayApps subAgentConfig.Apps = apps + subAgentConfig.Directory = workspaceFolder } deleteSubAgent := proc.agent.ID != uuid.Nil && maybeRecreateSubAgent && !proc.agent.EqualConfig(subAgentConfig) diff --git a/agent/agentcontainers/api_test.go b/agent/agentcontainers/api_test.go index d0141ea590..4e3e9e4077 100644 --- a/agent/agentcontainers/api_test.go +++ b/agent/agentcontainers/api_test.go @@ -1262,6 +1262,11 @@ func TestAPI(t *testing.T) { deleteErrC: make(chan error, 1), } fakeDCCLI = &fakeDevcontainerCLI{ + readConfig: agentcontainers.DevcontainerConfig{ + Workspace: agentcontainers.DevcontainerWorkspace{ + WorkspaceFolder: "/workspaces/coder", + }, + }, execErrC: make(chan func(cmd string, args ...string) error, 1), readConfigErrC: make(chan func(envs []string) error, 1), } @@ -1273,8 +1278,8 @@ func TestAPI(t *testing.T) { Running: true, CreatedAt: time.Now(), Labels: map[string]string{ - agentcontainers.DevcontainerLocalFolderLabel: "/workspaces", - agentcontainers.DevcontainerConfigFileLabel: "/workspace/.devcontainer/devcontainer.json", + agentcontainers.DevcontainerLocalFolderLabel: "/home/coder/coder", + agentcontainers.DevcontainerConfigFileLabel: "/home/coder/coder/.devcontainer/devcontainer.json", }, } ) @@ -1320,11 +1325,6 @@ func TestAPI(t *testing.T) { // Allow initial agent creation and injection to succeed. testutil.RequireSend(ctx, t, fakeSAC.createErrC, nil) - testutil.RequireSend(ctx, t, fakeDCCLI.execErrC, func(cmd string, args ...string) error { - assert.Equal(t, "pwd", cmd) - assert.Empty(t, args) - return nil - }) // Exec pwd. testutil.RequireSend(ctx, t, fakeDCCLI.readConfigErrC, func(envs []string) error { assert.Contains(t, envs, "CODER_WORKSPACE_AGENT_NAME=test-container") assert.Contains(t, envs, "CODER_WORKSPACE_NAME=test-workspace") @@ -1350,7 +1350,7 @@ func TestAPI(t *testing.T) { // Verify agent was created. require.Len(t, fakeSAC.created, 1) assert.Equal(t, "test-container", fakeSAC.created[0].Name) - assert.Equal(t, "/workspaces", fakeSAC.created[0].Directory) + assert.Equal(t, "/workspaces/coder", fakeSAC.created[0].Directory) assert.Len(t, fakeSAC.deleted, 0) t.Log("Agent injected successfully, now testing reinjection into the same container...") @@ -1467,11 +1467,6 @@ func TestAPI(t *testing.T) { testutil.RequireSend(ctx, t, fakeSAC.deleteErrC, nil) // Expect the agent to be recreated. testutil.RequireSend(ctx, t, fakeSAC.createErrC, nil) - testutil.RequireSend(ctx, t, fakeDCCLI.execErrC, func(cmd string, args ...string) error { - assert.Equal(t, "pwd", cmd) - assert.Empty(t, args) - return nil - }) // Exec pwd. testutil.RequireSend(ctx, t, fakeDCCLI.readConfigErrC, func(envs []string) error { assert.Contains(t, envs, "CODER_WORKSPACE_AGENT_NAME=test-container") assert.Contains(t, envs, "CODER_WORKSPACE_NAME=test-workspace") @@ -1814,7 +1809,6 @@ func TestAPI(t *testing.T) { }, }, }, - execErrC: make(chan func(cmd string, args ...string) error, 1), } testContainer = codersdk.WorkspaceAgentContainer{ @@ -1861,15 +1855,9 @@ func TestAPI(t *testing.T) { // Close before api.Close() defer to avoid deadlock after test. defer close(fSAC.createErrC) - defer close(fDCCLI.execErrC) // Given: We allow agent creation and injection to succeed. testutil.RequireSend(ctx, t, fSAC.createErrC, nil) - testutil.RequireSend(ctx, t, fDCCLI.execErrC, func(cmd string, args ...string) error { - assert.Equal(t, "pwd", cmd) - assert.Empty(t, args) - return nil - }) // Wait until the ticker has been registered. tickerTrap.MustWait(ctx).MustRelease(ctx) @@ -1913,7 +1901,6 @@ func TestAPI(t *testing.T) { }, }, readConfigErrC: make(chan func(envs []string) error, 2), - execErrC: make(chan func(cmd string, args ...string) error, 1), } testContainer = codersdk.WorkspaceAgentContainer{ @@ -1960,16 +1947,10 @@ func TestAPI(t *testing.T) { // Close before api.Close() defer to avoid deadlock after test. defer close(fSAC.createErrC) - defer close(fDCCLI.execErrC) defer close(fDCCLI.readConfigErrC) // Given: We allow agent creation and injection to succeed. testutil.RequireSend(ctx, t, fSAC.createErrC, nil) - testutil.RequireSend(ctx, t, fDCCLI.execErrC, func(cmd string, args ...string) error { - assert.Equal(t, "pwd", cmd) - assert.Empty(t, args) - return nil - }) testutil.RequireSend(ctx, t, fDCCLI.readConfigErrC, func(env []string) error { // We expect the wrong workspace agent name passed in first. assert.Contains(t, env, "CODER_WORKSPACE_AGENT_NAME=test-container") diff --git a/agent/agentcontainers/devcontainercli.go b/agent/agentcontainers/devcontainercli.go index e302ff07d6..e87c3362c6 100644 --- a/agent/agentcontainers/devcontainercli.go +++ b/agent/agentcontainers/devcontainercli.go @@ -22,6 +22,7 @@ import ( type DevcontainerConfig struct { MergedConfiguration DevcontainerMergedConfiguration `json:"mergedConfiguration"` Configuration DevcontainerConfiguration `json:"configuration"` + Workspace DevcontainerWorkspace `json:"workspace"` } type DevcontainerMergedConfiguration struct { @@ -46,6 +47,10 @@ type CoderCustomization struct { Name string `json:"name,omitempty"` } +type DevcontainerWorkspace struct { + WorkspaceFolder string `json:"workspaceFolder"` +} + // DevcontainerCLI is an interface for the devcontainer CLI. type DevcontainerCLI interface { Up(ctx context.Context, workspaceFolder, configPath string, opts ...DevcontainerCLIUpOptions) (id string, err error)