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`.
This commit is contained in:
Danielle Maywood
2025-06-20 16:34:30 +01:00
committed by GitHub
parent 9c1feffded
commit d61353f468
3 changed files with 20 additions and 51 deletions

View File

@ -1,11 +1,9 @@
package agentcontainers package agentcontainers
import ( import (
"bytes"
"context" "context"
"errors" "errors"
"fmt" "fmt"
"io"
"net/http" "net/http"
"os" "os"
"path" "path"
@ -1114,27 +1112,6 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
if proc.agent.ID == uuid.Nil || maybeRecreateSubAgent { if proc.agent.ID == uuid.Nil || maybeRecreateSubAgent {
subAgentConfig.Architecture = arch 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{ displayAppsMap := map[codersdk.DisplayApp]bool{
// NOTE(DanielleMaywood): // NOTE(DanielleMaywood):
// We use the same defaults here as set in terraform-provider-coder. // 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, codersdk.DisplayAppPortForward: true,
} }
var appsWithPossibleDuplicates []SubAgentApp var (
appsWithPossibleDuplicates []SubAgentApp
workspaceFolder = DevcontainerDefaultContainerWorkspaceFolder
)
if err := func() error { if err := func() error {
var ( var (
@ -1167,6 +1147,8 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
return err return err
} }
workspaceFolder = config.Workspace.WorkspaceFolder
// NOTE(DanielleMaywood): // NOTE(DanielleMaywood):
// We only want to take an agent name specified in the root customization layer. // 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 // 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.DisplayApps = displayApps
subAgentConfig.Apps = apps subAgentConfig.Apps = apps
subAgentConfig.Directory = workspaceFolder
} }
deleteSubAgent := proc.agent.ID != uuid.Nil && maybeRecreateSubAgent && !proc.agent.EqualConfig(subAgentConfig) deleteSubAgent := proc.agent.ID != uuid.Nil && maybeRecreateSubAgent && !proc.agent.EqualConfig(subAgentConfig)

View File

@ -1262,6 +1262,11 @@ func TestAPI(t *testing.T) {
deleteErrC: make(chan error, 1), deleteErrC: make(chan error, 1),
} }
fakeDCCLI = &fakeDevcontainerCLI{ fakeDCCLI = &fakeDevcontainerCLI{
readConfig: agentcontainers.DevcontainerConfig{
Workspace: agentcontainers.DevcontainerWorkspace{
WorkspaceFolder: "/workspaces/coder",
},
},
execErrC: make(chan func(cmd string, args ...string) error, 1), execErrC: make(chan func(cmd string, args ...string) error, 1),
readConfigErrC: make(chan func(envs []string) error, 1), readConfigErrC: make(chan func(envs []string) error, 1),
} }
@ -1273,8 +1278,8 @@ func TestAPI(t *testing.T) {
Running: true, Running: true,
CreatedAt: time.Now(), CreatedAt: time.Now(),
Labels: map[string]string{ Labels: map[string]string{
agentcontainers.DevcontainerLocalFolderLabel: "/workspaces", agentcontainers.DevcontainerLocalFolderLabel: "/home/coder/coder",
agentcontainers.DevcontainerConfigFileLabel: "/workspace/.devcontainer/devcontainer.json", 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. // Allow initial agent creation and injection to succeed.
testutil.RequireSend(ctx, t, fakeSAC.createErrC, nil) 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 { 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_AGENT_NAME=test-container")
assert.Contains(t, envs, "CODER_WORKSPACE_NAME=test-workspace") assert.Contains(t, envs, "CODER_WORKSPACE_NAME=test-workspace")
@ -1350,7 +1350,7 @@ func TestAPI(t *testing.T) {
// Verify agent was created. // Verify agent was created.
require.Len(t, fakeSAC.created, 1) require.Len(t, fakeSAC.created, 1)
assert.Equal(t, "test-container", fakeSAC.created[0].Name) 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) assert.Len(t, fakeSAC.deleted, 0)
t.Log("Agent injected successfully, now testing reinjection into the same container...") 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) testutil.RequireSend(ctx, t, fakeSAC.deleteErrC, nil)
// Expect the agent to be recreated. // Expect the agent to be recreated.
testutil.RequireSend(ctx, t, fakeSAC.createErrC, nil) 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 { 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_AGENT_NAME=test-container")
assert.Contains(t, envs, "CODER_WORKSPACE_NAME=test-workspace") 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{ testContainer = codersdk.WorkspaceAgentContainer{
@ -1861,15 +1855,9 @@ func TestAPI(t *testing.T) {
// Close before api.Close() defer to avoid deadlock after test. // Close before api.Close() defer to avoid deadlock after test.
defer close(fSAC.createErrC) defer close(fSAC.createErrC)
defer close(fDCCLI.execErrC)
// Given: We allow agent creation and injection to succeed. // Given: We allow agent creation and injection to succeed.
testutil.RequireSend(ctx, t, fSAC.createErrC, nil) 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. // Wait until the ticker has been registered.
tickerTrap.MustWait(ctx).MustRelease(ctx) tickerTrap.MustWait(ctx).MustRelease(ctx)
@ -1913,7 +1901,6 @@ func TestAPI(t *testing.T) {
}, },
}, },
readConfigErrC: make(chan func(envs []string) error, 2), readConfigErrC: make(chan func(envs []string) error, 2),
execErrC: make(chan func(cmd string, args ...string) error, 1),
} }
testContainer = codersdk.WorkspaceAgentContainer{ testContainer = codersdk.WorkspaceAgentContainer{
@ -1960,16 +1947,10 @@ func TestAPI(t *testing.T) {
// Close before api.Close() defer to avoid deadlock after test. // Close before api.Close() defer to avoid deadlock after test.
defer close(fSAC.createErrC) defer close(fSAC.createErrC)
defer close(fDCCLI.execErrC)
defer close(fDCCLI.readConfigErrC) defer close(fDCCLI.readConfigErrC)
// Given: We allow agent creation and injection to succeed. // Given: We allow agent creation and injection to succeed.
testutil.RequireSend(ctx, t, fSAC.createErrC, nil) 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 { testutil.RequireSend(ctx, t, fDCCLI.readConfigErrC, func(env []string) error {
// We expect the wrong workspace agent name passed in first. // We expect the wrong workspace agent name passed in first.
assert.Contains(t, env, "CODER_WORKSPACE_AGENT_NAME=test-container") assert.Contains(t, env, "CODER_WORKSPACE_AGENT_NAME=test-container")

View File

@ -22,6 +22,7 @@ import (
type DevcontainerConfig struct { type DevcontainerConfig struct {
MergedConfiguration DevcontainerMergedConfiguration `json:"mergedConfiguration"` MergedConfiguration DevcontainerMergedConfiguration `json:"mergedConfiguration"`
Configuration DevcontainerConfiguration `json:"configuration"` Configuration DevcontainerConfiguration `json:"configuration"`
Workspace DevcontainerWorkspace `json:"workspace"`
} }
type DevcontainerMergedConfiguration struct { type DevcontainerMergedConfiguration struct {
@ -46,6 +47,10 @@ type CoderCustomization struct {
Name string `json:"name,omitempty"` Name string `json:"name,omitempty"`
} }
type DevcontainerWorkspace struct {
WorkspaceFolder string `json:"workspaceFolder"`
}
// DevcontainerCLI is an interface for the devcontainer CLI. // DevcontainerCLI is an interface for the devcontainer CLI.
type DevcontainerCLI interface { type DevcontainerCLI interface {
Up(ctx context.Context, workspaceFolder, configPath string, opts ...DevcontainerCLIUpOptions) (id string, err error) Up(ctx context.Context, workspaceFolder, configPath string, opts ...DevcontainerCLIUpOptions) (id string, err error)