mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
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.
This commit is contained in:
+13
-17
@@ -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
|
||||
|
||||
+4
-3
@@ -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
|
||||
@@ -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 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 other Dev Containers is explicitly not supported.")
|
||||
}
|
||||
|
||||
func TestAgent_Dial(t *testing.T) {
|
||||
|
||||
+10
-2
@@ -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 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 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.",
|
||||
})
|
||||
})
|
||||
|
||||
+1
-1
@@ -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.")
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user