From 801585a5dc08fed03c0ad9434434de19b24905f9 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Mon, 7 Jul 2025 21:45:57 +0000 Subject: [PATCH 1/4] fix(agent): ensure dev container integration disabled inside of sub agent It appears we accidentally broke this logic in a previous PR. This should now correctly disable the agent api as we'd expect. --- agent/agent.go | 30 +++++++++++++----------------- agent/agent_test.go | 4 ++-- agent/api.go | 12 ++++++++++-- 3 files changed, 25 insertions(+), 21 deletions(-) diff --git a/agent/agent.go b/agent/agent.go index 3c02b5f2790f0..75117769d8e2d 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -336,18 +336,16 @@ func (a *agent) init() { // will not report anywhere. a.scriptRunner.RegisterMetrics(a.prometheusRegistry) - if a.devcontainers { - containerAPIOpts := []agentcontainers.Option{ - agentcontainers.WithExecer(a.execer), - agentcontainers.WithCommandEnv(a.sshServer.CommandEnv), - agentcontainers.WithScriptLogger(func(logSourceID uuid.UUID) agentcontainers.ScriptLogger { - return a.logSender.GetScriptLogger(logSourceID) - }), - } - containerAPIOpts = append(containerAPIOpts, a.containerAPIOptions...) - - a.containerAPI = agentcontainers.NewAPI(a.logger.Named("containers"), containerAPIOpts...) + containerAPIOpts := []agentcontainers.Option{ + agentcontainers.WithExecer(a.execer), + agentcontainers.WithCommandEnv(a.sshServer.CommandEnv), + agentcontainers.WithScriptLogger(func(logSourceID uuid.UUID) agentcontainers.ScriptLogger { + return a.logSender.GetScriptLogger(logSourceID) + }), } + containerAPIOpts = append(containerAPIOpts, a.containerAPIOptions...) + + a.containerAPI = agentcontainers.NewAPI(a.logger.Named("containers"), containerAPIOpts...) a.reconnectingPTYServer = reconnectingpty.NewServer( a.logger.Named("reconnecting-pty"), @@ -1162,7 +1160,7 @@ func (a *agent) handleManifest(manifestOK *checkpoint) func(ctx context.Context, scripts = manifest.Scripts devcontainerScripts map[uuid.UUID]codersdk.WorkspaceAgentScript ) - if a.containerAPI != nil { + if a.devcontainers { // Init the container API with the manifest and client so that // we can start accepting requests. The final start of the API // happens after the startup scripts have been executed to @@ -1197,7 +1195,7 @@ func (a *agent) handleManifest(manifestOK *checkpoint) func(ctx context.Context, // autostarted devcontainer will be included in this time. err := a.scriptRunner.Execute(a.gracefulCtx, agentscripts.ExecuteStartScripts) - if a.containerAPI != nil { + if a.devcontainers { // Start the container API after the startup scripts have // been executed to ensure that the required tools can be // installed. @@ -1928,10 +1926,8 @@ func (a *agent) Close() error { a.logger.Error(a.hardCtx, "script runner close", slog.Error(err)) } - if a.containerAPI != nil { - if err := a.containerAPI.Close(); err != nil { - a.logger.Error(a.hardCtx, "container API close", slog.Error(err)) - } + if err := a.containerAPI.Close(); err != nil { + a.logger.Error(a.hardCtx, "container API close", slog.Error(err)) } // Wait for the graceful shutdown to complete, but don't wait forever so diff --git a/agent/agent_test.go b/agent/agent_test.go index 4a9141bd37f9e..626f03a0a47b4 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -2453,8 +2453,8 @@ func TestAgent_DevcontainersDisabledForSubAgent(t *testing.T) { require.Error(t, err) // Verify the error message contains the expected text. - require.Contains(t, err.Error(), "The agent dev containers feature is experimental and not enabled by default.") - require.Contains(t, err.Error(), "To enable this feature, set CODER_AGENT_DEVCONTAINERS_ENABLE=true in your template.") + require.Contains(t, err.Error(), "Dev Container feature not supported.") + require.Contains(t, err.Error(), "Dev Container integration inside of Dev Containers are explicitly not supported.") } func TestAgent_Dial(t *testing.T) { diff --git a/agent/api.go b/agent/api.go index 0458df7c58e1f..2733e5ee744bd 100644 --- a/agent/api.go +++ b/agent/api.go @@ -6,6 +6,7 @@ import ( "time" "github.com/go-chi/chi/v5" + "github.com/google/uuid" "github.com/coder/coder/v2/coderd/httpapi" "github.com/coder/coder/v2/codersdk" @@ -36,12 +37,19 @@ func (a *agent) apiHandler() http.Handler { cacheDuration: cacheDuration, } - if a.containerAPI != nil { + if a.devcontainers { r.Mount("/api/v0/containers", a.containerAPI.Routes()) + } else if manifest := a.manifest.Load(); manifest != nil && manifest.ParentID != uuid.Nil { + r.HandleFunc("/api/v0/containers", func(w http.ResponseWriter, r *http.Request) { + httpapi.Write(r.Context(), w, http.StatusForbidden, codersdk.Response{ + Message: "Dev Container feature not supported.", + Detail: "Dev Container integration inside of Dev Containers are explicitly not supported.", + }) + }) } else { r.HandleFunc("/api/v0/containers", func(w http.ResponseWriter, r *http.Request) { httpapi.Write(r.Context(), w, http.StatusForbidden, codersdk.Response{ - Message: "The agent dev containers feature is experimental and not enabled by default.", + Message: "Dev Container feature not enabled.", Detail: "To enable this feature, set CODER_AGENT_DEVCONTAINERS_ENABLE=true in your template.", }) }) From 12f3dc7b116059325c9d4ebdc4e19205eb557d0a Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Mon, 7 Jul 2025 21:54:08 +0000 Subject: [PATCH 2/4] test: ensure devcontainers = true for better testing --- 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 626f03a0a47b4..2bba9f12e256f 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -2441,7 +2441,8 @@ func TestAgent_DevcontainersDisabledForSubAgent(t *testing.T) { // Setup the agent with devcontainers enabled initially. //nolint:dogsled - conn, _, _, _, _ := setupAgent(t, manifest, 0, func(*agenttest.Client, *agent.Options) { + conn, _, _, _, _ := setupAgent(t, manifest, 0, func(_ *agenttest.Client, o *agent.Options) { + o.Devcontainers = true }) // Query the containers API endpoint. This should fail because From 36c276fb9b7f720cedd9432b7cbe3e2da60cf913 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Mon, 7 Jul 2025 22:02:39 +0000 Subject: [PATCH 3/4] chore: listen to copilot --- agent/agent_test.go | 2 +- agent/api.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/agent/agent_test.go b/agent/agent_test.go index 2bba9f12e256f..d87148be9ad15 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -2455,7 +2455,7 @@ func TestAgent_DevcontainersDisabledForSubAgent(t *testing.T) { // Verify the error message contains the expected text. require.Contains(t, err.Error(), "Dev Container feature not supported.") - require.Contains(t, err.Error(), "Dev Container integration inside of Dev Containers are explicitly not supported.") + require.Contains(t, err.Error(), "Dev Container integration inside other Dev Containers is explicitly not supported.") } func TestAgent_Dial(t *testing.T) { diff --git a/agent/api.go b/agent/api.go index 2733e5ee744bd..ca0760e130ffe 100644 --- a/agent/api.go +++ b/agent/api.go @@ -43,7 +43,7 @@ func (a *agent) apiHandler() http.Handler { r.HandleFunc("/api/v0/containers", func(w http.ResponseWriter, r *http.Request) { httpapi.Write(r.Context(), w, http.StatusForbidden, codersdk.Response{ Message: "Dev Container feature not supported.", - Detail: "Dev Container integration inside of Dev Containers are explicitly not supported.", + Detail: "Dev Container integration inside other Dev Containers is explicitly not supported.", }) }) } else { From bb48815cbaca53248371527a3b7d4ea7f0de146d Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Tue, 8 Jul 2025 08:56:33 +0000 Subject: [PATCH 4/4] fix: update error check --- cli/ssh_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/ssh_test.go b/cli/ssh_test.go index 582f8a3fdf691..7a91cfa3ce365 100644 --- a/cli/ssh_test.go +++ b/cli/ssh_test.go @@ -2104,7 +2104,7 @@ func TestSSH_Container(t *testing.T) { clitest.SetupConfig(t, client, root) err := inv.WithContext(ctx).Run() - require.ErrorContains(t, err, "The agent dev containers feature is experimental and not enabled by default.") + require.ErrorContains(t, err, "Dev Container feature not enabled.") }) }