From 82d5bae4d41c3edcc9e183ff33bf5d175844c144 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 9 May 2025 09:35:47 +0000 Subject: [PATCH 1/4] fix(agent): fix unexpanded devcontainer paths for agentcontainers Updates #16424 --- agent/agent.go | 4 +++- agent/agentcontainers/devcontainer.go | 13 +++++++++++-- agent/agentcontainers/devcontainer_test.go | 3 +-- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/agent/agent.go b/agent/agent.go index 7525ecf051f69..1fe89810fd7df 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -1085,6 +1085,8 @@ func (a *agent) handleManifest(manifestOK *checkpoint) func(ctx context.Context, if err != nil { return xerrors.Errorf("expand directory: %w", err) } + // Normalize all devcontainer paths by making them absolute. + manifest.Devcontainers = agentcontainers.ExpandAllDevcontainerPaths(a.logger, expandPathToAbs, manifest.Devcontainers) subsys, err := agentsdk.ProtoFromSubsystems(a.subsystems) if err != nil { a.logger.Critical(ctx, "failed to convert subsystems", slog.Error(err)) @@ -1127,7 +1129,7 @@ func (a *agent) handleManifest(manifestOK *checkpoint) func(ctx context.Context, ) if a.experimentalDevcontainersEnabled { var dcScripts []codersdk.WorkspaceAgentScript - scripts, dcScripts = agentcontainers.ExtractAndInitializeDevcontainerScripts(a.logger, expandPathToAbs, manifest.Devcontainers, scripts) + scripts, dcScripts = agentcontainers.ExtractAndInitializeDevcontainerScripts(a.logger, manifest.Devcontainers, scripts) // See ExtractAndInitializeDevcontainerScripts for motivation // behind running dcScripts as post start scripts. scriptRunnerOpts = append(scriptRunnerOpts, agentscripts.WithPostStartScripts(dcScripts...)) diff --git a/agent/agentcontainers/devcontainer.go b/agent/agentcontainers/devcontainer.go index cbf42e150d240..903ef21539b3a 100644 --- a/agent/agentcontainers/devcontainer.go +++ b/agent/agentcontainers/devcontainer.go @@ -37,7 +37,6 @@ devcontainer up %s // important if e.g. a Coder module to install @devcontainer/cli is used. func ExtractAndInitializeDevcontainerScripts( logger slog.Logger, - expandPath func(string) (string, error), devcontainers []codersdk.WorkspaceAgentDevcontainer, scripts []codersdk.WorkspaceAgentScript, ) (filteredScripts []codersdk.WorkspaceAgentScript, devcontainerScripts []codersdk.WorkspaceAgentScript) { @@ -47,7 +46,6 @@ ScriptLoop: // The devcontainer scripts match the devcontainer ID for // identification. if script.ID == dc.ID { - dc = expandDevcontainerPaths(logger, expandPath, dc) devcontainerScripts = append(devcontainerScripts, devcontainerStartupScript(dc, script)) continue ScriptLoop } @@ -75,6 +73,17 @@ func devcontainerStartupScript(dc codersdk.WorkspaceAgentDevcontainer, script co return script } +// ExpandAllDevcontainerPaths expands all devcontainer paths in the given +// devcontainers. This is required by the devcontainer CLI, which requires +// absolute paths for the workspace folder and config path. +func ExpandAllDevcontainerPaths(logger slog.Logger, expandPath func(string) (string, error), devcontainers []codersdk.WorkspaceAgentDevcontainer) []codersdk.WorkspaceAgentDevcontainer { + expanded := make([]codersdk.WorkspaceAgentDevcontainer, 0, len(devcontainers)) + for _, dc := range devcontainers { + expanded = append(expanded, expandDevcontainerPaths(logger, expandPath, dc)) + } + return expanded +} + func expandDevcontainerPaths(logger slog.Logger, expandPath func(string) (string, error), dc codersdk.WorkspaceAgentDevcontainer) codersdk.WorkspaceAgentDevcontainer { logger = logger.With(slog.F("devcontainer", dc.Name), slog.F("workspace_folder", dc.WorkspaceFolder), slog.F("config_path", dc.ConfigPath)) diff --git a/agent/agentcontainers/devcontainer_test.go b/agent/agentcontainers/devcontainer_test.go index 5e0f5d8dae7bc..a3a764b34f9a0 100644 --- a/agent/agentcontainers/devcontainer_test.go +++ b/agent/agentcontainers/devcontainer_test.go @@ -243,8 +243,7 @@ func TestExtractAndInitializeDevcontainerScripts(t *testing.T) { } gotFilteredScripts, gotDevcontainerScripts := agentcontainers.ExtractAndInitializeDevcontainerScripts( logger, - tt.args.expandPath, - tt.args.devcontainers, + agentcontainers.ExpandAllDevcontainerPaths(logger, tt.args.expandPath, tt.args.devcontainers), tt.args.scripts, ) From 6ec33aac13fb18f6fa3efe18fcdd25891945fa93 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 9 May 2025 09:45:59 +0000 Subject: [PATCH 2/4] remove unused logger --- agent/agent.go | 2 +- agent/agentcontainers/devcontainer.go | 1 - agent/agentcontainers/devcontainer_test.go | 1 - 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/agent/agent.go b/agent/agent.go index 1fe89810fd7df..d0e668af34d74 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -1129,7 +1129,7 @@ func (a *agent) handleManifest(manifestOK *checkpoint) func(ctx context.Context, ) if a.experimentalDevcontainersEnabled { var dcScripts []codersdk.WorkspaceAgentScript - scripts, dcScripts = agentcontainers.ExtractAndInitializeDevcontainerScripts(a.logger, manifest.Devcontainers, scripts) + scripts, dcScripts = agentcontainers.ExtractAndInitializeDevcontainerScripts(manifest.Devcontainers, scripts) // See ExtractAndInitializeDevcontainerScripts for motivation // behind running dcScripts as post start scripts. scriptRunnerOpts = append(scriptRunnerOpts, agentscripts.WithPostStartScripts(dcScripts...)) diff --git a/agent/agentcontainers/devcontainer.go b/agent/agentcontainers/devcontainer.go index 903ef21539b3a..e04c308934a2c 100644 --- a/agent/agentcontainers/devcontainer.go +++ b/agent/agentcontainers/devcontainer.go @@ -36,7 +36,6 @@ devcontainer up %s // initialize the workspace (e.g. git clone, npm install, etc). This is // important if e.g. a Coder module to install @devcontainer/cli is used. func ExtractAndInitializeDevcontainerScripts( - logger slog.Logger, devcontainers []codersdk.WorkspaceAgentDevcontainer, scripts []codersdk.WorkspaceAgentScript, ) (filteredScripts []codersdk.WorkspaceAgentScript, devcontainerScripts []codersdk.WorkspaceAgentScript) { diff --git a/agent/agentcontainers/devcontainer_test.go b/agent/agentcontainers/devcontainer_test.go index a3a764b34f9a0..b20c943175821 100644 --- a/agent/agentcontainers/devcontainer_test.go +++ b/agent/agentcontainers/devcontainer_test.go @@ -242,7 +242,6 @@ func TestExtractAndInitializeDevcontainerScripts(t *testing.T) { } } gotFilteredScripts, gotDevcontainerScripts := agentcontainers.ExtractAndInitializeDevcontainerScripts( - logger, agentcontainers.ExpandAllDevcontainerPaths(logger, tt.args.expandPath, tt.args.devcontainers), tt.args.scripts, ) From 97532832f986426ac59bb7e3049bd70aae76d341 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 9 May 2025 11:30:02 +0000 Subject: [PATCH 3/4] update test to depend on path expansion --- agent/agent_test.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/agent/agent_test.go b/agent/agent_test.go index 67fa203252ba7..cc36aa75e825a 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -1998,8 +1998,8 @@ func TestAgent_ReconnectingPTYContainer(t *testing.T) { // You can run it manually as follows: // // CODER_TEST_USE_DOCKER=1 go test -count=1 ./agent -run TestAgent_DevcontainerAutostart +// nolint: paralleltest // This test sets an environment variable. func TestAgent_DevcontainerAutostart(t *testing.T) { - t.Parallel() if os.Getenv("CODER_TEST_USE_DOCKER") != "1" { t.Skip("Set CODER_TEST_USE_DOCKER=1 to run this test") } @@ -2012,9 +2012,12 @@ func TestAgent_DevcontainerAutostart(t *testing.T) { // Prepare temporary devcontainer for test (mywork). devcontainerID := uuid.New() - tempWorkspaceFolder := t.TempDir() - tempWorkspaceFolder = filepath.Join(tempWorkspaceFolder, "mywork") + tmpdir := t.TempDir() + t.Setenv("HOME", tmpdir) + tempWorkspaceFolder := filepath.Join(tmpdir, "mywork") + unexpandedWorkspaceFolder := filepath.Join("~", "mywork") t.Logf("Workspace folder: %s", tempWorkspaceFolder) + t.Logf("Unexpanded workspace folder: %s", unexpandedWorkspaceFolder) devcontainerPath := filepath.Join(tempWorkspaceFolder, ".devcontainer") err = os.MkdirAll(devcontainerPath, 0o755) require.NoError(t, err, "create devcontainer directory") @@ -2031,9 +2034,10 @@ func TestAgent_DevcontainerAutostart(t *testing.T) { // is expected to be prepared by the provisioner normally. Devcontainers: []codersdk.WorkspaceAgentDevcontainer{ { - ID: devcontainerID, - Name: "test", - WorkspaceFolder: tempWorkspaceFolder, + ID: devcontainerID, + Name: "test", + // Use an unexpanded path to test the expansion. + WorkspaceFolder: unexpandedWorkspaceFolder, }, }, Scripts: []codersdk.WorkspaceAgentScript{ From 3f3559be2c5365b3cd88f778e685bfc004dd04e9 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 9 May 2025 11:38:36 +0000 Subject: [PATCH 4/4] fix weird nolint formatting --- agent/agent_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/agent/agent_test.go b/agent/agent_test.go index cc36aa75e825a..fe2c99059e9d8 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -1998,7 +1998,8 @@ func TestAgent_ReconnectingPTYContainer(t *testing.T) { // You can run it manually as follows: // // CODER_TEST_USE_DOCKER=1 go test -count=1 ./agent -run TestAgent_DevcontainerAutostart -// nolint: paralleltest // This test sets an environment variable. +// +//nolint:paralleltest // This test sets an environment variable. func TestAgent_DevcontainerAutostart(t *testing.T) { if os.Getenv("CODER_TEST_USE_DOCKER") != "1" { t.Skip("Set CODER_TEST_USE_DOCKER=1 to run this test")