Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

fix(agent): disable dev container integration inside sub agents#18781

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes fromall commits
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 13 additions & 17 deletionsagent/agent.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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...)
Comment on lines +339 to +348
Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

We're going to create the API by default. This is fine as we onlyInit and thenStart at a later stage. This allows us to assumecontainerAPI will always be anon-nil value.

mafredri reacted with thumbs up emoji

a.reconnectingPTYServer = reconnectingpty.NewServer(
a.logger.Named("reconnecting-pty"),
Expand DownExpand Up@@ -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
Expand DownExpand Up@@ -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.
Expand DownExpand Up@@ -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
Expand Down
7 changes: 4 additions & 3 deletionsagent/agent_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Copy link
ContributorAuthor

@DanielleMaywoodDanielleMaywoodJul 7, 2025•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

It appears the test allowed the regression to occur as it had Dev Containersdisabled by default. This means it didn't properly test the flow we'd expect.

mafredri reacted with thumbs up emoji
})

// Query the containers API endpoint. This should fail because
Expand All@@ -2453,8 +2454,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 containersfeatureis experimental andnotenabled 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 Containerfeature notsupported.")
require.Contains(t, err.Error(), "Dev Container integration inside other Dev Containers is explicitly not supported.")
Comment on lines +2457 to +2458
Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Reminder to runCODER_TEST_USE_DOCKER=1 go test ./coderd ./agent ./agent/agentcontainers if you want to verify the "integration" tests.

}

func TestAgent_Dial(t *testing.T) {
Expand Down
12 changes: 10 additions & 2 deletionsagent/api.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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"
Expand DownExpand Up@@ -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 other Dev Containers is 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 containersfeatureis experimental andnot enabled by default.",
Message: "Dev Containerfeature not enabled.",
Detail: "To enable this feature, set CODER_AGENT_DEVCONTAINERS_ENABLE=true in your template.",
})
})
Expand Down
2 changes: 1 addition & 1 deletioncli/ssh_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 containersfeatureis experimental andnot enabled by default.")
require.ErrorContains(t, err, "Dev Containerfeature not enabled.")
})
}

Expand Down
Loading
[8]ページ先頭
©2009-2025 Movatter.jp