Skip to content

Commit 0118e75

Browse files
DanielleMaywoodDanielleMaywood
authored
fix(agent): disable dev container integration inside sub agents (#18781)
It appears we accidentally broke this logic in a previous PR. This should now correctly disable the agent api as we'd expect.
1 parent 1195f31 commit 0118e75

File tree

4 files changed

+28
-23
lines changed

4 files changed

+28
-23
lines changed

‎agent/agent.go

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -336,18 +336,16 @@ func (a *agent) init() {
336336
// will not report anywhere.
337337
a.scriptRunner.RegisterMetrics(a.prometheusRegistry)
338338

339-
if a.devcontainers {
340-
containerAPIOpts := []agentcontainers.Option{
341-
agentcontainers.WithExecer(a.execer),
342-
agentcontainers.WithCommandEnv(a.sshServer.CommandEnv),
343-
agentcontainers.WithScriptLogger(func(logSourceID uuid.UUID) agentcontainers.ScriptLogger {
344-
return a.logSender.GetScriptLogger(logSourceID)
345-
}),
346-
}
347-
containerAPIOpts = append(containerAPIOpts, a.containerAPIOptions...)
348-
349-
a.containerAPI = agentcontainers.NewAPI(a.logger.Named("containers"), containerAPIOpts...)
339+
containerAPIOpts := []agentcontainers.Option{
340+
agentcontainers.WithExecer(a.execer),
341+
agentcontainers.WithCommandEnv(a.sshServer.CommandEnv),
342+
agentcontainers.WithScriptLogger(func(logSourceID uuid.UUID) agentcontainers.ScriptLogger {
343+
return a.logSender.GetScriptLogger(logSourceID)
344+
}),
350345
}
346+
containerAPIOpts = append(containerAPIOpts, a.containerAPIOptions...)
347+
348+
a.containerAPI = agentcontainers.NewAPI(a.logger.Named("containers"), containerAPIOpts...)
351349

352350
a.reconnectingPTYServer = reconnectingpty.NewServer(
353351
a.logger.Named("reconnecting-pty"),
@@ -1162,7 +1160,7 @@ func (a *agent) handleManifest(manifestOK *checkpoint) func(ctx context.Context,
11621160
scripts = manifest.Scripts
11631161
devcontainerScripts map[uuid.UUID]codersdk.WorkspaceAgentScript
11641162
)
1165-
if a.containerAPI != nil {
1163+
if a.devcontainers {
11661164
// Init the container API with the manifest and client so that
11671165
// we can start accepting requests. The final start of the API
11681166
// happens after the startup scripts have been executed to
@@ -1197,7 +1195,7 @@ func (a *agent) handleManifest(manifestOK *checkpoint) func(ctx context.Context,
11971195
// autostarted devcontainer will be included in this time.
11981196
err := a.scriptRunner.Execute(a.gracefulCtx, agentscripts.ExecuteStartScripts)
11991197

1200-
if a.containerAPI != nil {
1198+
if a.devcontainers {
12011199
// Start the container API after the startup scripts have
12021200
// been executed to ensure that the required tools can be
12031201
// installed.
@@ -1928,10 +1926,8 @@ func (a *agent) Close() error {
19281926
a.logger.Error(a.hardCtx, "script runner close", slog.Error(err))
19291927
}
19301928

1931-
if a.containerAPI != nil {
1932-
if err := a.containerAPI.Close(); err != nil {
1933-
a.logger.Error(a.hardCtx, "container API close", slog.Error(err))
1934-
}
1929+
if err := a.containerAPI.Close(); err != nil {
1930+
a.logger.Error(a.hardCtx, "container API close", slog.Error(err))
19351931
}
19361932

19371933
// Wait for the graceful shutdown to complete, but don't wait forever so

‎agent/agent_test.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2441,7 +2441,8 @@ func TestAgent_DevcontainersDisabledForSubAgent(t *testing.T) {
24412441

24422442
// Setup the agent with devcontainers enabled initially.
24432443
//nolint:dogsled
2444-
conn, _, _, _, _ := setupAgent(t, manifest, 0, func(*agenttest.Client, *agent.Options) {
2444+
conn, _, _, _, _ := setupAgent(t, manifest, 0, func(_ *agenttest.Client, o *agent.Options) {
2445+
o.Devcontainers = true
24452446
})
24462447

24472448
// Query the containers API endpoint. This should fail because
@@ -2453,8 +2454,8 @@ func TestAgent_DevcontainersDisabledForSubAgent(t *testing.T) {
24532454
require.Error(t, err)
24542455

24552456
// Verify the error message contains the expected text.
2456-
require.Contains(t, err.Error(), "The agent dev containers feature is experimental and not enabled by default.")
2457-
require.Contains(t, err.Error(), "To enable this feature, set CODER_AGENT_DEVCONTAINERS_ENABLE=true in your template.")
2457+
require.Contains(t, err.Error(), "Dev Container feature not supported.")
2458+
require.Contains(t, err.Error(), "Dev Container integration inside other Dev Containers is explicitly not supported.")
24582459
}
24592460

24602461
func TestAgent_Dial(t *testing.T) {

‎agent/api.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"time"
77

88
"github.com/go-chi/chi/v5"
9+
"github.com/google/uuid"
910

1011
"github.com/coder/coder/v2/coderd/httpapi"
1112
"github.com/coder/coder/v2/codersdk"
@@ -36,12 +37,19 @@ func (a *agent) apiHandler() http.Handler {
3637
cacheDuration: cacheDuration,
3738
}
3839

39-
if a.containerAPI != nil {
40+
if a.devcontainers {
4041
r.Mount("/api/v0/containers", a.containerAPI.Routes())
42+
} else if manifest := a.manifest.Load(); manifest != nil && manifest.ParentID != uuid.Nil {
43+
r.HandleFunc("/api/v0/containers", func(w http.ResponseWriter, r *http.Request) {
44+
httpapi.Write(r.Context(), w, http.StatusForbidden, codersdk.Response{
45+
Message: "Dev Container feature not supported.",
46+
Detail: "Dev Container integration inside other Dev Containers is explicitly not supported.",
47+
})
48+
})
4149
} else {
4250
r.HandleFunc("/api/v0/containers", func(w http.ResponseWriter, r *http.Request) {
4351
httpapi.Write(r.Context(), w, http.StatusForbidden, codersdk.Response{
44-
Message: "The agent dev containers feature is experimental and not enabled by default.",
52+
Message: "Dev Container feature not enabled.",
4553
Detail: "To enable this feature, set CODER_AGENT_DEVCONTAINERS_ENABLE=true in your template.",
4654
})
4755
})

‎cli/ssh_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2104,7 +2104,7 @@ func TestSSH_Container(t *testing.T) {
21042104
clitest.SetupConfig(t, client, root)
21052105

21062106
err := inv.WithContext(ctx).Run()
2107-
require.ErrorContains(t, err, "The agent dev containers feature is experimental and not enabled by default.")
2107+
require.ErrorContains(t, err, "Dev Container feature not enabled.")
21082108
})
21092109
}
21102110

0 commit comments

Comments
 (0)