feat(agent/agentcontainers): implement ignore customization for devcontainers (#18530)

Fixes coder/internal#737

agent/agentcontainers/api.go

@@ -83,6 +83,7 @@ type API struct {
 	recreateErrorTimes       map[string]time.Time                           // By workspace folder.
 	injectedSubAgentProcs    map[string]subAgentProcess                     // By workspace folder.
 	usingWorkspaceFolderName map[string]bool                                // By workspace folder.
+	ignoredDevcontainers     map[string]bool                                // By workspace folder. Tracks three states (true, false and not checked).
 	asyncWg                  sync.WaitGroup
 
 	devcontainerLogSourceIDs map[string]uuid.UUID // By workspace folder.
@@ -276,6 +277,7 @@ func NewAPI(logger slog.Logger, options ...Option) *API {
 		devcontainerNames:           make(map[string]bool),
 		knownDevcontainers:          make(map[string]codersdk.WorkspaceAgentDevcontainer),
 		configFileModifiedTimes:     make(map[string]time.Time),
+		ignoredDevcontainers:        make(map[string]bool),
 		recreateSuccessTimes:        make(map[string]time.Time),
 		recreateErrorTimes:          make(map[string]time.Time),
 		scriptLogger:                func(uuid.UUID) ScriptLogger { return noopScriptLogger{} },
@@ -804,6 +806,10 @@ func (api *API) getContainers() (codersdk.WorkspaceAgentListContainersResponse,
 	if len(api.knownDevcontainers) > 0 {
 		devcontainers = make([]codersdk.WorkspaceAgentDevcontainer, 0, len(api.knownDevcontainers))
 		for _, dc := range api.knownDevcontainers {
+			if api.ignoredDevcontainers[dc.WorkspaceFolder] {
+				continue
+			}
+
 			// Include the agent if it's running (we're iterating over
 			// copies, so mutating is fine).
 			if proc := api.injectedSubAgentProcs[dc.WorkspaceFolder]; proc.agent.ID != uuid.Nil {
@@ -1036,6 +1042,10 @@ func (api *API) markDevcontainerDirty(configPath string, modifiedAt time.Time) {
 			logger.Info(api.ctx, "marking devcontainer as dirty")
 			dc.Dirty = true
 		}
+		if _, ok := api.ignoredDevcontainers[dc.WorkspaceFolder]; ok {
+			logger.Debug(api.ctx, "clearing devcontainer ignored state")
+			delete(api.ignoredDevcontainers, dc.WorkspaceFolder) // Allow re-reading config.
+		}
 
 		api.knownDevcontainers[dc.WorkspaceFolder] = dc
 	}
@@ -1092,6 +1102,10 @@ func (api *API) cleanupSubAgents(ctx context.Context) error {
 // This method uses an internal timeout to prevent blocking indefinitely
 // if something goes wrong with the injection.
 func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc codersdk.WorkspaceAgentDevcontainer) (err error) {
+	if api.ignoredDevcontainers[dc.WorkspaceFolder] {
+		return nil
+	}
+
 	ctx, cancel := context.WithTimeout(ctx, defaultOperationTimeout)
 	defer cancel()
 
@@ -1113,6 +1127,42 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
 	maybeRecreateSubAgent := false
 	proc, injected := api.injectedSubAgentProcs[dc.WorkspaceFolder]
 	if injected {
+		if _, ignoreChecked := api.ignoredDevcontainers[dc.WorkspaceFolder]; !ignoreChecked {
+			// If ignore status has not yet been checked, or cleared by
+			// modifications to the devcontainer.json, we must read it
+			// to determine the current status. This can happen while
+			// the  devcontainer subagent is already running or before
+			// we've had a chance to inject it.
+			//
+			// Note, for simplicity, we do not try to optimize to reduce
+			// ReadConfig calls here.
+			config, err := api.dccli.ReadConfig(ctx, dc.WorkspaceFolder, dc.ConfigPath, nil)
+			if err != nil {
+				return xerrors.Errorf("read devcontainer config: %w", err)
+			}
+
+			dcIgnored := config.Configuration.Customizations.Coder.Ignore
+			if dcIgnored {
+				proc.stop()
+				if proc.agent.ID != uuid.Nil {
+					// Unlock while doing the delete operation.
+					api.mu.Unlock()
+					client := *api.subAgentClient.Load()
+					if err := client.Delete(ctx, proc.agent.ID); err != nil {
+						api.mu.Lock()
+						return xerrors.Errorf("delete subagent: %w", err)
+					}
+					api.mu.Lock()
+				}
+				// Reset agent and containerID to force config re-reading if ignore is toggled.
+				proc.agent = SubAgent{}
+				proc.containerID = ""
+				api.injectedSubAgentProcs[dc.WorkspaceFolder] = proc
+				api.ignoredDevcontainers[dc.WorkspaceFolder] = dcIgnored
+				return nil
+			}
+		}
+
 		if proc.containerID == container.ID && proc.ctx.Err() == nil {
 			// Same container and running, no need to reinject.
 			return nil
@@ -1131,7 +1181,8 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
 		// Container ID changed or the subagent process is not running,
 		// stop the existing subagent context to replace it.
 		proc.stop()
-	} else {
+	}
+	if proc.agent.OperatingSystem == "" {
 		// Set SubAgent defaults.
 		proc.agent.OperatingSystem = "linux" // Assuming Linux for devcontainers.
 	}
@@ -1150,7 +1201,11 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
 	ranSubAgent := false
 
 	// Clean up if injection fails.
+	var dcIgnored, setDCIgnored bool
 	defer func() {
+		if setDCIgnored {
+			api.ignoredDevcontainers[dc.WorkspaceFolder] = dcIgnored
+		}
 		if !ranSubAgent {
 			proc.stop()
 			if !api.closed {
@@ -1188,48 +1243,6 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
 		proc.agent.Architecture = arch
 	}
 
-	agentBinaryPath, err := os.Executable()
-	if err != nil {
-		return xerrors.Errorf("get agent binary path: %w", err)
-	}
-	agentBinaryPath, err = filepath.EvalSymlinks(agentBinaryPath)
-	if err != nil {
-		return xerrors.Errorf("resolve agent binary path: %w", err)
-	}
-
-	// If we scripted this as a `/bin/sh` script, we could reduce these
-	// steps to one instruction, speeding up the injection process.
-	//
-	// Note: We use `path` instead of `filepath` here because we are
-	// working with Unix-style paths inside the container.
-	if _, err := api.ccli.ExecAs(ctx, container.ID, "root", "mkdir", "-p", path.Dir(coderPathInsideContainer)); err != nil {
-		return xerrors.Errorf("create agent directory in container: %w", err)
-	}
-
-	if err := api.ccli.Copy(ctx, container.ID, agentBinaryPath, coderPathInsideContainer); err != nil {
-		return xerrors.Errorf("copy agent binary: %w", err)
-	}
-
-	logger.Info(ctx, "copied agent binary to container")
-
-	// Make sure the agent binary is executable so we can run it (the
-	// user doesn't matter since we're making it executable for all).
-	if _, err := api.ccli.ExecAs(ctx, container.ID, "root", "chmod", "0755", path.Dir(coderPathInsideContainer), coderPathInsideContainer); err != nil {
-		return xerrors.Errorf("set agent binary executable: %w", err)
-	}
-
-	// Attempt to add CAP_NET_ADMIN to the binary to improve network
-	// performance (optional, allow to fail). See `bootstrap_linux.sh`.
-	// TODO(mafredri): Disable for now until we can figure out why this
-	// causes the following error on some images:
-	//
-	//	Image: mcr.microsoft.com/devcontainers/base:ubuntu
-	// 	Error: /.coder-agent/coder: Operation not permitted
-	//
-	// if _, err := api.ccli.ExecAs(ctx, container.ID, "root", "setcap", "cap_net_admin+ep", coderPathInsideContainer); err != nil {
-	// 	logger.Warn(ctx, "set CAP_NET_ADMIN on agent binary failed", slog.Error(err))
-	// }
-
 	subAgentConfig := proc.agent.CloneConfig(dc)
 	if proc.agent.ID == uuid.Nil || maybeRecreateSubAgent {
 		subAgentConfig.Architecture = arch
@@ -1269,6 +1282,13 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
 				return err
 			}
 
+			// We only allow ignore to be set in the root customization layer to
+			// prevent weird interactions with devcontainer features.
+			dcIgnored, setDCIgnored = config.Configuration.Customizations.Coder.Ignore, true
+			if dcIgnored {
+				return nil
+			}
+
 			workspaceFolder = config.Workspace.WorkspaceFolder
 
 			// NOTE(DanielleMaywood):
@@ -1317,6 +1337,22 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
 			api.logger.Error(ctx, "unable to read devcontainer config", slog.Error(err))
 		}
 
+		if dcIgnored {
+			proc.stop()
+			if proc.agent.ID != uuid.Nil {
+				// If we stop the subagent, we also need to delete it.
+				client := *api.subAgentClient.Load()
+				if err := client.Delete(ctx, proc.agent.ID); err != nil {
+					return xerrors.Errorf("delete subagent: %w", err)
+				}
+			}
+			// Reset agent and containerID to force config re-reading if
+			// ignore is toggled.
+			proc.agent = SubAgent{}
+			proc.containerID = ""
+			return nil
+		}
+
 		displayApps := make([]codersdk.DisplayApp, 0, len(displayAppsMap))
 		for app, enabled := range displayAppsMap {
 			if enabled {
@@ -1349,6 +1385,48 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
 		subAgentConfig.Directory = workspaceFolder
 	}
 
+	agentBinaryPath, err := os.Executable()
+	if err != nil {
+		return xerrors.Errorf("get agent binary path: %w", err)
+	}
+	agentBinaryPath, err = filepath.EvalSymlinks(agentBinaryPath)
+	if err != nil {
+		return xerrors.Errorf("resolve agent binary path: %w", err)
+	}
+
+	// If we scripted this as a `/bin/sh` script, we could reduce these
+	// steps to one instruction, speeding up the injection process.
+	//
+	// Note: We use `path` instead of `filepath` here because we are
+	// working with Unix-style paths inside the container.
+	if _, err := api.ccli.ExecAs(ctx, container.ID, "root", "mkdir", "-p", path.Dir(coderPathInsideContainer)); err != nil {
+		return xerrors.Errorf("create agent directory in container: %w", err)
+	}
+
+	if err := api.ccli.Copy(ctx, container.ID, agentBinaryPath, coderPathInsideContainer); err != nil {
+		return xerrors.Errorf("copy agent binary: %w", err)
+	}
+
+	logger.Info(ctx, "copied agent binary to container")
+
+	// Make sure the agent binary is executable so we can run it (the
+	// user doesn't matter since we're making it executable for all).
+	if _, err := api.ccli.ExecAs(ctx, container.ID, "root", "chmod", "0755", path.Dir(coderPathInsideContainer), coderPathInsideContainer); err != nil {
+		return xerrors.Errorf("set agent binary executable: %w", err)
+	}
+
+	// Attempt to add CAP_NET_ADMIN to the binary to improve network
+	// performance (optional, allow to fail). See `bootstrap_linux.sh`.
+	// TODO(mafredri): Disable for now until we can figure out why this
+	// causes the following error on some images:
+	//
+	//	Image: mcr.microsoft.com/devcontainers/base:ubuntu
+	// 	Error: /.coder-agent/coder: Operation not permitted
+	//
+	// if _, err := api.ccli.ExecAs(ctx, container.ID, "root", "setcap", "cap_net_admin+ep", coderPathInsideContainer); err != nil {
+	// 	logger.Warn(ctx, "set CAP_NET_ADMIN on agent binary failed", slog.Error(err))
+	// }
+
 	deleteSubAgent := proc.agent.ID != uuid.Nil && maybeRecreateSubAgent && !proc.agent.EqualConfig(subAgentConfig)
 	if deleteSubAgent {
 		logger.Debug(ctx, "deleting existing subagent for recreation", slog.F("agent_id", proc.agent.ID))

agent/agentcontainers/api_test.go

@@ -2070,6 +2070,177 @@ func TestAPI(t *testing.T) {
 		require.Equal(t, testDir, lastCmd.Dir, "custom directory should be used")
 		require.Equal(t, testEnv, lastCmd.Env, "custom environment should be used")
 	})
+
+	t.Run("IgnoreCustomization", func(t *testing.T) {
+		t.Parallel()
+
+		if runtime.GOOS == "windows" {
+			t.Skip("Dev Container tests are not supported on Windows (this test uses mocks but fails due to Windows paths)")
+		}
+
+		ctx := testutil.Context(t, testutil.WaitShort)
+
+		startTime := time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC)
+		configPath := "/workspace/project/.devcontainer/devcontainer.json"
+
+		container := codersdk.WorkspaceAgentContainer{
+			ID:           "container-id",
+			FriendlyName: "container-name",
+			Running:      true,
+			CreatedAt:    startTime.Add(-1 * time.Hour),
+			Labels: map[string]string{
+				agentcontainers.DevcontainerLocalFolderLabel: "/workspace/project",
+				agentcontainers.DevcontainerConfigFileLabel:  configPath,
+			},
+		}
+
+		fLister := &fakeContainerCLI{
+			containers: codersdk.WorkspaceAgentListContainersResponse{
+				Containers: []codersdk.WorkspaceAgentContainer{container},
+			},
+			arch: runtime.GOARCH,
+		}
+
+		// Start with ignore=true
+		fDCCLI := &fakeDevcontainerCLI{
+			execErrC: make(chan func(string, ...string) error, 1),
+			readConfig: agentcontainers.DevcontainerConfig{
+				Configuration: agentcontainers.DevcontainerConfiguration{
+					Customizations: agentcontainers.DevcontainerCustomizations{
+						Coder: agentcontainers.CoderCustomization{Ignore: true},
+					},
+				},
+				Workspace: agentcontainers.DevcontainerWorkspace{WorkspaceFolder: "/workspace/project"},
+			},
+		}
+
+		fakeSAC := &fakeSubAgentClient{
+			logger:     slogtest.Make(t, nil).Named("fakeSubAgentClient"),
+			agents:     make(map[uuid.UUID]agentcontainers.SubAgent),
+			createErrC: make(chan error, 1),
+			deleteErrC: make(chan error, 1),
+		}
+
+		mClock := quartz.NewMock(t)
+		mClock.Set(startTime)
+		fWatcher := newFakeWatcher(t)
+
+		logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug)
+		api := agentcontainers.NewAPI(
+			logger,
+			agentcontainers.WithDevcontainerCLI(fDCCLI),
+			agentcontainers.WithContainerCLI(fLister),
+			agentcontainers.WithSubAgentClient(fakeSAC),
+			agentcontainers.WithWatcher(fWatcher),
+			agentcontainers.WithClock(mClock),
+		)
+		defer func() {
+			close(fakeSAC.createErrC)
+			close(fakeSAC.deleteErrC)
+			api.Close()
+		}()
+
+		r := chi.NewRouter()
+		r.Mount("/", api.Routes())
+
+		t.Log("Phase 1: Test ignore=true filters out devcontainer")
+		req := httptest.NewRequest(http.MethodGet, "/", nil).WithContext(ctx)
+		rec := httptest.NewRecorder()
+		r.ServeHTTP(rec, req)
+		require.Equal(t, http.StatusOK, rec.Code)
+
+		var response codersdk.WorkspaceAgentListContainersResponse
+		err := json.NewDecoder(rec.Body).Decode(&response)
+		require.NoError(t, err)
+
+		assert.Empty(t, response.Devcontainers, "ignored devcontainer should not be in response when ignore=true")
+		assert.Len(t, response.Containers, 1, "regular container should still be listed")
+
+		t.Log("Phase 2: Change to ignore=false")
+		fDCCLI.readConfig.Configuration.Customizations.Coder.Ignore = false
+		var (
+			exitSubAgent     = make(chan struct{})
+			subAgentExited   = make(chan struct{})
+			exitSubAgentOnce sync.Once
+		)
+		defer func() {
+			exitSubAgentOnce.Do(func() {
+				close(exitSubAgent)
+			})
+		}()
+		execSubAgent := func(cmd string, args ...string) error {
+			if len(args) != 1 || args[0] != "agent" {
+				t.Log("execSubAgent called with unexpected arguments", cmd, args)
+				return nil
+			}
+			defer close(subAgentExited)
+			select {
+			case <-exitSubAgent:
+			case <-ctx.Done():
+				return ctx.Err()
+			}
+			return nil
+		}
+		testutil.RequireSend(ctx, t, fDCCLI.execErrC, execSubAgent)
+		testutil.RequireSend(ctx, t, fakeSAC.createErrC, nil)
+
+		fWatcher.sendEventWaitNextCalled(ctx, fsnotify.Event{
+			Name: configPath,
+			Op:   fsnotify.Write,
+		})
+
+		err = api.RefreshContainers(ctx)
+		require.NoError(t, err)
+
+		t.Log("Phase 2: Cont, waiting for sub agent to exit")
+		exitSubAgentOnce.Do(func() {
+			close(exitSubAgent)
+		})
+		select {
+		case <-subAgentExited:
+		case <-ctx.Done():
+			t.Fatal("timeout waiting for sub agent to exit")
+		}
+
+		req = httptest.NewRequest(http.MethodGet, "/", nil).WithContext(ctx)
+		rec = httptest.NewRecorder()
+		r.ServeHTTP(rec, req)
+		require.Equal(t, http.StatusOK, rec.Code)
+
+		err = json.NewDecoder(rec.Body).Decode(&response)
+		require.NoError(t, err)
+
+		assert.Len(t, response.Devcontainers, 1, "devcontainer should be in response when ignore=false")
+		assert.Len(t, response.Containers, 1, "regular container should still be listed")
+		assert.Equal(t, "/workspace/project", response.Devcontainers[0].WorkspaceFolder)
+		require.Len(t, fakeSAC.created, 1, "sub agent should be created when ignore=false")
+		createdAgentID := fakeSAC.created[0].ID
+
+		t.Log("Phase 3: Change back to ignore=true and test sub agent deletion")
+		fDCCLI.readConfig.Configuration.Customizations.Coder.Ignore = true
+		testutil.RequireSend(ctx, t, fakeSAC.deleteErrC, nil)
+
+		fWatcher.sendEventWaitNextCalled(ctx, fsnotify.Event{
+			Name: configPath,
+			Op:   fsnotify.Write,
+		})
+
+		err = api.RefreshContainers(ctx)
+		require.NoError(t, err)
+
+		req = httptest.NewRequest(http.MethodGet, "/", nil).WithContext(ctx)
+		rec = httptest.NewRecorder()
+		r.ServeHTTP(rec, req)
+		require.Equal(t, http.StatusOK, rec.Code)
+
+		err = json.NewDecoder(rec.Body).Decode(&response)
+		require.NoError(t, err)
+
+		assert.Empty(t, response.Devcontainers, "devcontainer should be filtered out when ignore=true again")
+		assert.Len(t, response.Containers, 1, "regular container should still be listed")
+		require.Len(t, fakeSAC.deleted, 1, "sub agent should be deleted when ignore=true")
+		assert.Equal(t, createdAgentID, fakeSAC.deleted[0], "the same sub agent that was created should be deleted")
+	})
 }
 
 // mustFindDevcontainerByPath returns the devcontainer with the given workspace

agent/agentcontainers/devcontainercli.go

@@ -44,6 +44,7 @@ type CoderCustomization struct {
 	DisplayApps map[codersdk.DisplayApp]bool `json:"displayApps,omitempty"`
 	Apps        []SubAgentApp                `json:"apps,omitempty"`
 	Name        string                       `json:"name,omitempty"`
+	Ignore      bool                         `json:"ignore,omitempty"`
 }
 
 type DevcontainerWorkspace struct {

coderd/workspaceagents_test.go

@@ -1432,6 +1432,7 @@ func TestWorkspaceAgentRecreateDevcontainer(t *testing.T) {
 					}, nil).AnyTimes()
 					// DetectArchitecture always returns "<none>" for this test to disable agent injection.
 					mccli.EXPECT().DetectArchitecture(gomock.Any(), devContainer.ID).Return("<none>", nil).AnyTimes()
+					mdccli.EXPECT().ReadConfig(gomock.Any(), workspaceFolder, configFile, gomock.Any()).Return(agentcontainers.DevcontainerConfig{}, nil).Times(1)
 					mdccli.EXPECT().Up(gomock.Any(), workspaceFolder, configFile, gomock.Any()).Return("someid", nil).Times(1)
 					return 0
 				},