diff --git a/agent/agentcontainers/api_test.go b/agent/agentcontainers/api_test.go index a66cb1199b..1429de7d85 100644 --- a/agent/agentcontainers/api_test.go +++ b/agent/agentcontainers/api_test.go @@ -2862,6 +2862,126 @@ func TestAPI(t *testing.T) { "rebuilt agent should include updated display apps") }) + // Verify that when a terraform-managed subagent is injected into + // a devcontainer, the Directory field sent to Create reflects + // the container-internal workspaceFolder from devcontainer + // read-configuration, not the host-side workspace_folder from + // the terraform resource. This is the scenario described in + // https://linear.app/codercom/issue/PRODUCT-259: + // 1. Non-terraform subagent → directory = /workspaces/foo (correct) + // 2. Terraform subagent → directory was stuck on host path (bug) + t.Run("TerraformDefinedSubAgentUsesContainerInternalDirectory", 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)") + } + + var ( + ctx = testutil.Context(t, testutil.WaitMedium) + logger = slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) + mCtrl = gomock.NewController(t) + + terraformAgentID = uuid.New() + containerID = "test-container-id" + + // Given: A container with a host-side workspace folder. + terraformContainer = codersdk.WorkspaceAgentContainer{ + ID: containerID, + FriendlyName: "test-container", + Image: "test-image", + Running: true, + CreatedAt: time.Now(), + Labels: map[string]string{ + agentcontainers.DevcontainerLocalFolderLabel: "/home/coder/project", + agentcontainers.DevcontainerConfigFileLabel: "/home/coder/project/.devcontainer/devcontainer.json", + }, + } + + // Given: A terraform-defined devcontainer whose + // workspace_folder is the HOST-side path (set by provisioner). + terraformDevcontainer = codersdk.WorkspaceAgentDevcontainer{ + ID: uuid.New(), + Name: "terraform-devcontainer", + WorkspaceFolder: "/home/coder/project", + ConfigPath: "/home/coder/project/.devcontainer/devcontainer.json", + SubagentID: uuid.NullUUID{UUID: terraformAgentID, Valid: true}, + } + + fCCLI = &fakeContainerCLI{ + containers: codersdk.WorkspaceAgentListContainersResponse{ + Containers: []codersdk.WorkspaceAgentContainer{terraformContainer}, + }, + arch: runtime.GOARCH, + } + + // Given: devcontainer read-configuration returns the + // CONTAINER-INTERNAL workspace folder. + fDCCLI = &fakeDevcontainerCLI{ + upID: containerID, + readConfig: agentcontainers.DevcontainerConfig{ + Workspace: agentcontainers.DevcontainerWorkspace{ + WorkspaceFolder: "/workspaces/project", + }, + MergedConfiguration: agentcontainers.DevcontainerMergedConfiguration{ + Customizations: agentcontainers.DevcontainerMergedCustomizations{ + Coder: []agentcontainers.CoderCustomization{{}}, + }, + }, + }, + } + + mSAC = acmock.NewMockSubAgentClient(mCtrl) + createCalls = make(chan agentcontainers.SubAgent, 1) + closed bool + ) + + mSAC.EXPECT().List(gomock.Any()).Return([]agentcontainers.SubAgent{}, nil).AnyTimes() + + mSAC.EXPECT().Create(gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, agent agentcontainers.SubAgent) (agentcontainers.SubAgent, error) { + agent.AuthToken = uuid.New() + createCalls <- agent + return agent, nil + }, + ).Times(1) + + mSAC.EXPECT().Delete(gomock.Any(), gomock.Any()).DoAndReturn(func(_ context.Context, _ uuid.UUID) error { + assert.True(t, closed, "Delete should only be called after Close") + return nil + }).AnyTimes() + + api := agentcontainers.NewAPI(logger, + agentcontainers.WithContainerCLI(fCCLI), + agentcontainers.WithDevcontainerCLI(fDCCLI), + agentcontainers.WithDevcontainers( + []codersdk.WorkspaceAgentDevcontainer{terraformDevcontainer}, + []codersdk.WorkspaceAgentScript{{ID: terraformDevcontainer.ID, LogSourceID: uuid.New()}}, + ), + agentcontainers.WithSubAgentClient(mSAC), + agentcontainers.WithSubAgentURL("test-subagent-url"), + agentcontainers.WithWatcher(watcher.NewNoop()), + ) + api.Start() + defer func() { + closed = true + api.Close() + }() + + // When: The devcontainer is created (triggering injection). + err := api.CreateDevcontainer(terraformDevcontainer.WorkspaceFolder, terraformDevcontainer.ConfigPath) + require.NoError(t, err) + + // Then: The subagent sent to Create has the correct + // container-internal directory, not the host path. + createdAgent := testutil.RequireReceive(ctx, t, createCalls) + assert.Equal(t, terraformAgentID, createdAgent.ID, + "agent should use terraform-defined ID") + assert.Equal(t, "/workspaces/project", createdAgent.Directory, + "directory should be the container-internal path from devcontainer "+ + "read-configuration, not the host-side workspace_folder") + }) + t.Run("Error", func(t *testing.T) { t.Parallel() diff --git a/coderd/agentapi/subagent.go b/coderd/agentapi/subagent.go index 2aee5ce36c..dc739545cc 100644 --- a/coderd/agentapi/subagent.go +++ b/coderd/agentapi/subagent.go @@ -71,7 +71,7 @@ func (a *SubAgentAPI) CreateSubAgent(ctx context.Context, req *agentproto.Create // An ID is only given in the request when it is a terraform-defined devcontainer // that has attached resources. These subagents are pre-provisioned by terraform // (the agent record already exists), so we update configurable fields like - // display_apps rather than creating a new agent. + // display_apps and directory rather than creating a new agent. if req.Id != nil { id, err := uuid.FromBytes(req.Id) if err != nil { @@ -97,6 +97,16 @@ func (a *SubAgentAPI) CreateSubAgent(ctx context.Context, req *agentproto.Create return nil, xerrors.Errorf("update workspace agent display apps: %w", err) } + if req.Directory != "" { + if err := a.Database.UpdateWorkspaceAgentDirectoryByID(ctx, database.UpdateWorkspaceAgentDirectoryByIDParams{ + ID: id, + Directory: req.Directory, + UpdatedAt: createdAt, + }); err != nil { + return nil, xerrors.Errorf("update workspace agent directory: %w", err) + } + } + return &agentproto.CreateSubAgentResponse{ Agent: &agentproto.SubAgent{ Name: subAgent.Name, diff --git a/coderd/agentapi/subagent_test.go b/coderd/agentapi/subagent_test.go index e5e6ce73e5..78219aabe7 100644 --- a/coderd/agentapi/subagent_test.go +++ b/coderd/agentapi/subagent_test.go @@ -1267,11 +1267,11 @@ func TestSubAgentAPI(t *testing.T) { agentID, err := uuid.FromBytes(resp.Agent.Id) require.NoError(t, err) - // And: The database agent's other fields are unchanged. + // And: The database agent's name, architecture, and OS are unchanged. updatedAgent, err := db.GetWorkspaceAgentByID(dbauthz.AsSystemRestricted(ctx), agentID) require.NoError(t, err) require.Equal(t, baseChildAgent.Name, updatedAgent.Name) - require.Equal(t, baseChildAgent.Directory, updatedAgent.Directory) + require.Equal(t, "/different/path", updatedAgent.Directory) require.Equal(t, baseChildAgent.Architecture, updatedAgent.Architecture) require.Equal(t, baseChildAgent.OperatingSystem, updatedAgent.OperatingSystem) @@ -1280,6 +1280,42 @@ func TestSubAgentAPI(t *testing.T) { require.Equal(t, database.DisplayAppWebTerminal, updatedAgent.DisplayApps[0]) }, }, + { + name: "OK_DirectoryUpdated", + setup: func(t *testing.T, db database.Store, agent database.WorkspaceAgent) *proto.CreateSubAgentRequest { + // Given: An existing child agent with a stale host-side + // directory (as set by the provisioner at build time). + childAgent := dbgen.WorkspaceAgent(t, db, database.WorkspaceAgent{ + ParentID: uuid.NullUUID{Valid: true, UUID: agent.ID}, + ResourceID: agent.ResourceID, + Name: baseChildAgent.Name, + Directory: "/home/coder/project", + Architecture: baseChildAgent.Architecture, + OperatingSystem: baseChildAgent.OperatingSystem, + DisplayApps: baseChildAgent.DisplayApps, + }) + + // When: Agent injection sends the correct + // container-internal path. + return &proto.CreateSubAgentRequest{ + Id: childAgent.ID[:], + Directory: "/workspaces/project", + DisplayApps: []proto.CreateSubAgentRequest_DisplayApp{ + proto.CreateSubAgentRequest_WEB_TERMINAL, + }, + } + }, + check: func(t *testing.T, ctx context.Context, db database.Store, resp *proto.CreateSubAgentResponse, agent database.WorkspaceAgent) { + agentID, err := uuid.FromBytes(resp.Agent.Id) + require.NoError(t, err) + + // Then: Directory is updated to the container-internal + // path. + updatedAgent, err := db.GetWorkspaceAgentByID(dbauthz.AsSystemRestricted(ctx), agentID) + require.NoError(t, err) + require.Equal(t, "/workspaces/project", updatedAgent.Directory) + }, + }, { name: "Error/MalformedID", setup: func(t *testing.T, db database.Store, agent database.WorkspaceAgent) *proto.CreateSubAgentRequest { diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 82df9d3553..64376e87b0 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -6783,6 +6783,19 @@ func (q *querier) UpdateWorkspaceAgentConnectionByID(ctx context.Context, arg da return q.db.UpdateWorkspaceAgentConnectionByID(ctx, arg) } +func (q *querier) UpdateWorkspaceAgentDirectoryByID(ctx context.Context, arg database.UpdateWorkspaceAgentDirectoryByIDParams) error { + workspace, err := q.db.GetWorkspaceByAgentID(ctx, arg.ID) + if err != nil { + return err + } + + if err := q.authorizeContext(ctx, policy.ActionUpdateAgent, workspace); err != nil { + return err + } + + return q.db.UpdateWorkspaceAgentDirectoryByID(ctx, arg) +} + func (q *querier) UpdateWorkspaceAgentDisplayAppsByID(ctx context.Context, arg database.UpdateWorkspaceAgentDisplayAppsByIDParams) error { workspace, err := q.db.GetWorkspaceByAgentID(ctx, arg.ID) if err != nil { diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 4c9810b0f1..97e0942736 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -2935,6 +2935,17 @@ func (s *MethodTestSuite) TestWorkspace() { dbm.EXPECT().UpdateWorkspaceAgentStartupByID(gomock.Any(), arg).Return(nil).AnyTimes() check.Args(arg).Asserts(w, policy.ActionUpdate).Returns() })) + s.Run("UpdateWorkspaceAgentDirectoryByID", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { + w := testutil.Fake(s.T(), faker, database.Workspace{}) + agt := testutil.Fake(s.T(), faker, database.WorkspaceAgent{}) + arg := database.UpdateWorkspaceAgentDirectoryByIDParams{ + ID: agt.ID, + Directory: "/workspaces/project", + } + dbm.EXPECT().GetWorkspaceByAgentID(gomock.Any(), agt.ID).Return(w, nil).AnyTimes() + dbm.EXPECT().UpdateWorkspaceAgentDirectoryByID(gomock.Any(), arg).Return(nil).AnyTimes() + check.Args(arg).Asserts(w, policy.ActionUpdateAgent).Returns() + })) s.Run("UpdateWorkspaceAgentDisplayAppsByID", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { w := testutil.Fake(s.T(), faker, database.Workspace{}) agt := testutil.Fake(s.T(), faker, database.WorkspaceAgent{}) diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index a867b464ba..506defebaa 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -4840,6 +4840,14 @@ func (m queryMetricsStore) UpdateWorkspaceAgentConnectionByID(ctx context.Contex return r0 } +func (m queryMetricsStore) UpdateWorkspaceAgentDirectoryByID(ctx context.Context, arg database.UpdateWorkspaceAgentDirectoryByIDParams) error { + start := time.Now() + r0 := m.s.UpdateWorkspaceAgentDirectoryByID(ctx, arg) + m.queryLatencies.WithLabelValues("UpdateWorkspaceAgentDirectoryByID").Observe(time.Since(start).Seconds()) + m.queryCounts.WithLabelValues(httpmw.ExtractHTTPRoute(ctx), httpmw.ExtractHTTPMethod(ctx), "UpdateWorkspaceAgentDirectoryByID").Inc() + return r0 +} + func (m queryMetricsStore) UpdateWorkspaceAgentDisplayAppsByID(ctx context.Context, arg database.UpdateWorkspaceAgentDisplayAppsByIDParams) error { start := time.Now() r0 := m.s.UpdateWorkspaceAgentDisplayAppsByID(ctx, arg) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index 9399712000..e99ba8a929 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -9120,6 +9120,20 @@ func (mr *MockStoreMockRecorder) UpdateWorkspaceAgentConnectionByID(ctx, arg any return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateWorkspaceAgentConnectionByID", reflect.TypeOf((*MockStore)(nil).UpdateWorkspaceAgentConnectionByID), ctx, arg) } +// UpdateWorkspaceAgentDirectoryByID mocks base method. +func (m *MockStore) UpdateWorkspaceAgentDirectoryByID(ctx context.Context, arg database.UpdateWorkspaceAgentDirectoryByIDParams) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpdateWorkspaceAgentDirectoryByID", ctx, arg) + ret0, _ := ret[0].(error) + return ret0 +} + +// UpdateWorkspaceAgentDirectoryByID indicates an expected call of UpdateWorkspaceAgentDirectoryByID. +func (mr *MockStoreMockRecorder) UpdateWorkspaceAgentDirectoryByID(ctx, arg any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateWorkspaceAgentDirectoryByID", reflect.TypeOf((*MockStore)(nil).UpdateWorkspaceAgentDirectoryByID), ctx, arg) +} + // UpdateWorkspaceAgentDisplayAppsByID mocks base method. func (m *MockStore) UpdateWorkspaceAgentDisplayAppsByID(ctx context.Context, arg database.UpdateWorkspaceAgentDisplayAppsByIDParams) error { m.ctrl.T.Helper() diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 86e598ba04..d8802d6dba 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -1011,6 +1011,7 @@ type sqlcQuerier interface { UpdateWorkspace(ctx context.Context, arg UpdateWorkspaceParams) (WorkspaceTable, error) UpdateWorkspaceACLByID(ctx context.Context, arg UpdateWorkspaceACLByIDParams) error UpdateWorkspaceAgentConnectionByID(ctx context.Context, arg UpdateWorkspaceAgentConnectionByIDParams) error + UpdateWorkspaceAgentDirectoryByID(ctx context.Context, arg UpdateWorkspaceAgentDirectoryByIDParams) error UpdateWorkspaceAgentDisplayAppsByID(ctx context.Context, arg UpdateWorkspaceAgentDisplayAppsByIDParams) error UpdateWorkspaceAgentLifecycleStateByID(ctx context.Context, arg UpdateWorkspaceAgentLifecycleStateByIDParams) error UpdateWorkspaceAgentLogOverflowByID(ctx context.Context, arg UpdateWorkspaceAgentLogOverflowByIDParams) error diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 61d1160f7f..98138e5fac 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -26816,6 +26816,26 @@ func (q *sqlQuerier) UpdateWorkspaceAgentConnectionByID(ctx context.Context, arg return err } +const updateWorkspaceAgentDirectoryByID = `-- name: UpdateWorkspaceAgentDirectoryByID :exec +UPDATE + workspace_agents +SET + directory = $2, updated_at = $3 +WHERE + id = $1 +` + +type UpdateWorkspaceAgentDirectoryByIDParams struct { + ID uuid.UUID `db:"id" json:"id"` + Directory string `db:"directory" json:"directory"` + UpdatedAt time.Time `db:"updated_at" json:"updated_at"` +} + +func (q *sqlQuerier) UpdateWorkspaceAgentDirectoryByID(ctx context.Context, arg UpdateWorkspaceAgentDirectoryByIDParams) error { + _, err := q.db.ExecContext(ctx, updateWorkspaceAgentDirectoryByID, arg.ID, arg.Directory, arg.UpdatedAt) + return err +} + const updateWorkspaceAgentDisplayAppsByID = `-- name: UpdateWorkspaceAgentDisplayAppsByID :exec UPDATE workspace_agents diff --git a/coderd/database/queries/workspaceagents.sql b/coderd/database/queries/workspaceagents.sql index 7f8b53696a..830ab57835 100644 --- a/coderd/database/queries/workspaceagents.sql +++ b/coderd/database/queries/workspaceagents.sql @@ -190,6 +190,14 @@ SET WHERE id = $1; +-- name: UpdateWorkspaceAgentDirectoryByID :exec +UPDATE + workspace_agents +SET + directory = $2, updated_at = $3 +WHERE + id = $1; + -- name: GetWorkspaceAgentLogsAfter :many SELECT *