feat(coderd/agentapi): implement sub agent api #17823

DanielleMaywood · 2025-05-14T09:14:30Z

Implement the coderd side of the AgentAPI for the upcoming dev-container agents work.

agent/agenttest/client.go is left unimplemented for a future PR working to implement the agent side of this feature.

…osne

This definitely isn't an ideal test. It feels quite large. Maybe I'll try to extract the different things being tested and move them into their own tests.

Refactor the One Big Test into multiple separate unit tests

Begin validation of the agent name. This uses the same logic (with the exception of duplicate agent name detection) as the provisioner. A future commit will aim to rectify the duplicate agent name issue.

Copilot

Pull Request Overview

This PR implements a new Dev Container Agent API for managing workspace agents used by development containers in the system. Key changes include:

Adding a new subject type and authorization support for dev container agents.
Introducing SQL queries, querier methods, in-memory and mock DB implementations for workspace agent deletion and retrieval by parent ID.
Creating API endpoints and corresponding proto definitions/methods for creating, deleting, and listing dev container agents.

Reviewed Changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
coderd/rbac/authz.go	Introduces a new subject type for dev container agent API authorization.
coderd/database/queries/workspaceagents.sql	Adds SQL queries for workspace agent deletion and retrieval by parent ID.
coderd/database/queries.sql.go	Adds new SQL constants and functions for workspace agent operations.
coderd/database/querier.go	Updates the querier interface with new workspace agent methods.
coderd/database/dbmock/dbmock.go	Updates mock implementations to support new workspace agent methods.
coderd/database/dbmetrics/querymetrics.go	Adds metrics wrappers for the new workspace agent queries.
coderd/database/dbmem/dbmem.go	Implements fake methods for deleting and retrieving workspace agents.
coderd/database/dbauthz/dbauthz_test.go	Adds tests for workspace agent authorization actions.
coderd/database/dbauthz/dbauthz.go	Implements authorization logic and API methods for workspace agent deletion/retrieval.
coderd/agentapi/devcontainer_agent_test.go	Provides tests for creating, deleting, and listing dev container agents.
coderd/agentapi/devcontainer_agent.go	Implements the Dev Container Agent API endpoints.
coderd/agentapi/api.go	Integrates the Dev Container Agent API into the main API composition.
agent/proto/agent_drpc.pb.go	Updates generated proto code with new DRPC client/server methods.
agent/proto/agent.proto	Defines new RPC messages and endpoints for dev container agent operations.
agent/agenttest/client.go	Adds stub implementations for new dev container agent methods in the fake client.

coderd/database/dbmem/dbmem.go

mafredri

This largely looks good to me, nice work!

Just so I'm understanding this correctly. This permission model works because the agentapi is unique for each agent talking to it and you can't fake being another agent. Hence the simple queries which only filter on id/parent_id, etc. are sufficient. And on top of this, dbauthz verifies that agent is allowed to modify the workspace.

agent/proto/agent.proto

coderd/agentapi/devcontainer_agent.go

mafredri · 2025-05-22T12:22:08Z

coderd/agentapi/devcontainer_agent.go

+		AuthToken:                uuid.New(),
+		AuthInstanceID:           parentAgent.AuthInstanceID,
+		Architecture:             req.Architecture,
+		EnvironmentVariables:     pqtype.NullRawMessage{},


Note: I think this is correct, at least for now. If customers request inheritance we can look at it then, but ideally any envs needed are defined in devcontainer.json

Yeah we should definitely keep this in mind, or at least make sure to document that these variables aren't inherited.

coderd/agentapi/devcontainer_agent.go

coderd/agentapi/devcontainer_agent_test.go

coderd/database/queries/workspaceagents.sql

Copilot

Pull Request Overview

This PR implements the devcontainer agents API to support creating, deleting, and listing dev container agents while updating the corresponding database queries, authorization logic, and RPC interfaces.

Introduces new RPC endpoints and protobuf definitions for managing dev container agents.
Adds SQL queries, database functions, and mocks, as well as updates to RBAC, logging middleware, and tests to support the new API.

Reviewed Changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
coderd/workspaceagentsrpc.go	Adds OrganizationID to the RPC payload for workspace agents.
coderd/rbac/authz.go	Introduces a new subject type for the dev container agent API.
coderd/httpmw/loggermw/logger.go	Updates logger middleware to accommodate the new subject type.
coderd/database/queries/*.sql	Adds new SQL queries for fetching agents by parent and deleting agents.
coderd/database/queries.sql.go	Implements new constants and methods for the workspace agent queries.
coderd/database/querier.go	Adds new querier interface methods for dev container agent operations.
coderd/database/dbmock/dbmock.go	Adds mocks for the new database methods.
coderd/database/dbmetrics/querymetrics.go	Includes metrics wrappers for the new database operations.
coderd/database/dbmem/dbmem.go	Implements in-memory versions of the new database methods.
coderd/database/dbauthz/dbauthz_test.go	Introduces tests to verify authorization for the new API methods.
coderd/database/dbauthz/dbauthz.go	Adds authorization logic and a helper for dev container agent API.
coderd/agentapi/devcontainer_agent.go	Implements the new dev container agent API endpoints.
coderd/agentapi/api.go	Integrates DevContainerAgentAPI into the main API.
agent/proto/agent.proto, agent_drpc*.go	Updates prototype and DRPC definitions to add the new RPC endpoints.
agent/agenttest/client.go	Adds stub (unimplemented) methods for the new dev container API.

Comments suppressed due to low confidence (2)

coderd/database/queries.sql.go:14583

[nitpick] Consider renaming the parameter 'dollar_1' to a more descriptive name (e.g., 'parentID') to improve clarity.

func (q *sqlQuerier) GetWorkspaceAgentsByParentID(ctx context.Context, dollar_1 uuid.UUID) ([]WorkspaceAgent, error) {

coderd/database/querier.go:416

[nitpick] Consider renaming the parameter 'dollar_1' to a more meaningful name (e.g., 'parentID') for better readability and consistency.

GetWorkspaceAgentsByParentID(ctx context.Context, dollar_1 uuid.UUID) ([]WorkspaceAgent, error)

agent/proto/agent_drpc_old.go

mafredri

This looks largely good IMO. I suggest adding a bit more paranoia to the agent deletion query, but otherwise only minor suggestions and discussion points.

Pre-approving but deferring final approval to @spikecurtis and @johnstcn.

agent/proto/agent_drpc_old.go

coderd/agentapi/api.go

mafredri · 2025-05-28T18:01:28Z

coderd/agentapi/devcontainer_agent.go

+	for i, agent := range workspaceAgents {
+		agents[i] = &agentproto.ListDevContainerAgentsResponse_DevContainerAgent{
+			Name: agent.Name,
+			Id:   agent.ID[:],


Discussion: Since we don't output a token here, if the agent ever crashes and is restarted, I suppose it will have to start by querying this list, deleting each agent, and then create new ones. I think that's probably best, but thought I'd raise this anyway.

mafredri · 2025-05-28T18:02:49Z

coderd/database/dbauthz/dbauthz.go

@@ -319,6 +319,28 @@ var (
 		Scope: rbac.ScopeAll,
 	}.WithCachedASTValue()

+	subjectDevContainerAgentAPI = func(userID uuid.UUID, orgID uuid.UUID) rbac.Subject {


Suggested change

subjectDevContainerAgentAPI = func(userID uuid.UUID, orgID uuid.UUID) rbac.Subject {

subjectDevContainerAgentAPI = func(orgID uuid.UUID, userID uuid.UUID) rbac.Subject {

I think re-ordering here would make sense (argument of larger scope, followed by smaller scope).

Out of scope of this PR, but there's an argument to be made for something like this:

type OrganizationID uuid.UUID type UserID uuid.UUID

You could also have something like:

type UserOrgID struct { OrgID uuid.UUID UserID uuid.UUID } subjectDevContainerAgentAPI = func(UserOrgID) rbac.Subject { ... }

For the purposes of AsDevcontainerAgentAPI, you could maybe do something like the below, but it feels like overkill:

AsDevcontainerAgentAPI(ctx).InOrg(orgID).ForUser(userID)

We'll just need to be careful not to mix up OrgID and UserID.

I've re-ordered it. It would definitely be nice if we could have some type-safety on IDs.

coderd/database/queries/workspaceagents.sql

coderd/database/dbmem/dbmem.go

agent/proto/agent_drpc_old.go

coderd/agentapi/api.go

johnstcn · 2025-05-28T19:13:34Z

coderd/database/dbauthz/dbauthz.go

@@ -319,6 +319,28 @@ var (
 		Scope: rbac.ScopeAll,
 	}.WithCachedASTValue()

+	subjectDevContainerAgentAPI = func(userID uuid.UUID, orgID uuid.UUID) rbac.Subject {


Out of scope of this PR, but there's an argument to be made for something like this:

type OrganizationID uuid.UUID type UserID uuid.UUID

You could also have something like:

type UserOrgID struct { OrgID uuid.UUID UserID uuid.UUID } subjectDevContainerAgentAPI = func(UserOrgID) rbac.Subject { ... }

For the purposes of AsDevcontainerAgentAPI, you could maybe do something like the below, but it feels like overkill:

AsDevcontainerAgentAPI(ctx).InOrg(orgID).ForUser(userID)

We'll just need to be careful not to mix up OrgID and UserID.

coderd/agentapi/devcontainer_agent_test.go

johnstcn · 2025-05-28T19:24:23Z

coderd/database/queries/workspaceagents.sql

@@ -330,3 +330,9 @@ INNER JOIN workspace_resources ON workspace_resources.id = workspace_agents.reso
 INNER JOIN workspace_builds ON workspace_builds.job_id = workspace_resources.job_id
 WHERE workspace_builds.id = $1
 ORDER BY workspace_agent_script_timings.script_id, workspace_agent_script_timings.started_at;
+
+-- name: GetWorkspaceAgentsByParentID :many
+SELECT * FROM workspace_agents WHERE parent_id = @parent_id::uuid;


More paranoia: given that parent_id is nullable, what happens if someone calls this with uuid.Nil? Should we do a similar check for AND parent_id IS NOT NULL?

If parent_id was NULL then it should return nothing (assuming my Postgres knowledge is correct).

I've added a test to verify this behavior is what we'd want.

Replace all naming of Dev Container Agent to Sub Agent.

Refactor the DeleteWorkspaceAgentByID query to be DeleteWorkspaceSubAgentByID and add a filter that only allows it to delete an agent that has a non-NULL parent id.

agent/proto/agent.proto

agent/proto/agent_drpc_old.go

coderd/database/dbmem/dbmem.go

Accidentally left some unused code in dbmem.go

github-actions bot assigned DanielleMaywood May 14, 2025

DanielleMaywood mentioned this pull request May 14, 2025

chore: introduce ResourceWorkspaceAgent RBAC resource #17824

Closed

DanielleMaywood force-pushed the dm-devcontainer-agents-coderd branch from 71493fe to 3a69abb Compare May 21, 2025 09:32

DanielleMaywood changed the title ~~draft: devcontainer agents api~~ feat(coderd/agentapi): implement devcontainer agents api May 21, 2025

DanielleMaywood added 12 commits May 21, 2025 15:18

chore: begin impl of dev container agent's /coderd/agenetapi

dea341d

chore: add WorkspaceAgent rbac policy

5c6d5fe

chore: add Architecture and OperatingSystem to agent.proto

62ebd44

chore: remove todo

0857054

chore: appease linter

993b671

chore: add system context for test

5895d55

chore: remove outdated code

c4af28b

chore: replace google.protobuf.Empty with DeleteDevContainerAgentResp…

c2deb25

…osne

chore: make test larger

1fe6fe3

This definitely isn't an ideal test. It feels quite large. Maybe I'll try to extract the different things being tested and move them into their own tests.

refactor: tests

7fd59db

Refactor the One Big Test into multiple separate unit tests

chore: appease linter

b1f8e42

chore: begin validating agent name

eb881ea

Begin validation of the agent name. This uses the same logic (with the exception of duplicate agent name detection) as the provisioner. A future commit will aim to rectify the duplicate agent name issue.

DanielleMaywood force-pushed the dm-devcontainer-agents-coderd branch from c18f28a to eb881ea Compare May 21, 2025 15:19

DanielleMaywood requested a review from Copilot May 21, 2025 15:19

Copilot AI reviewed May 21, 2025

View reviewed changes

coderd/database/dbmem/dbmem.go Outdated Show resolved Hide resolved

chore: listen to feedback

924699c

DanielleMaywood requested a review from mafredri May 22, 2025 12:13

mafredri reviewed May 22, 2025

View reviewed changes

DanielleMaywood added 8 commits May 22, 2025 12:56

chore: rename GetWorkspaceAgentsWithParentID

b60d42d

chore: ensure createdAt and updatedAt match

505c0cc

chore: rbac

1af33bc

chore: appease formatter

1731e5f

tests: refactor and add some adversarial tests

7b06e6f

test: behavior is correct with no child agents

0c37a92

chore: appease linter

ef5772d

Merge branch 'main' into dm-devcontainer-agents-coderd

cb89e03

chore: update DRPCAgentClient25 interface

94a2d59

DanielleMaywood requested a review from Copilot May 22, 2025 17:22

Copilot AI reviewed May 22, 2025

View reviewed changes

chore: rename sql parameter

ec4dda9

DanielleMaywood force-pushed the dm-devcontainer-agents-coderd branch from b8675f2 to ec4dda9 Compare May 22, 2025 17:25

DanielleMaywood added 3 commits May 27, 2025 09:10

Merge branch 'main' into dm-devcontainer-agents-coderd

0c8be91

Merge branch 'main' into dm-devcontainer-agents-coderd

34d870a

chore: remove comment as recent PR resolves issue

cb965ea

DanielleMaywood commented May 28, 2025

View reviewed changes

agent/proto/agent_drpc_old.go Outdated Show resolved Hide resolved

DanielleMaywood marked this pull request as ready for review May 28, 2025 13:40

DanielleMaywood requested review from spikecurtis and johnstcn as code owners May 28, 2025 13:40

DanielleMaywood requested a review from mafredri May 28, 2025 13:40

mafredri approved these changes May 28, 2025

View reviewed changes

johnstcn reviewed May 28, 2025

View reviewed changes

DanielleMaywood added 5 commits May 28, 2025 20:40

refactor: devcontainer agent -> subagent

979f4ef

Replace all naming of Dev Container Agent to Sub Agent.

refactor: deleteworkspaceagent to deleteworkspacesubagent

2ffc0e3

Refactor the DeleteWorkspaceAgentByID query to be DeleteWorkspaceSubAgentByID and add a filter that only allows it to delete an agent that has a non-NULL parent id.

fix: appease linter

f1a4da3

test: paranoia

036436b

Merge branch 'main' into dm-devcontainer-agents-coderd

c6ded1f

spikecurtis reviewed May 29, 2025

View reviewed changes

agent/proto/agent.proto Outdated Show resolved Hide resolved

agent/proto/agent.proto Outdated Show resolved Hide resolved

DanielleMaywood added 2 commits May 29, 2025 09:14

chore: apply feedback from spike

0871c82

test: check err is a not authorized error

b7e64ef

johnstcn approved these changes May 29, 2025

View reviewed changes

agent/proto/agent_drpc_old.go Show resolved Hide resolved

coderd/database/dbmem/dbmem.go Outdated Show resolved Hide resolved

fix: remove unused code

1c28667

Accidentally left some unused code in dbmem.go

DanielleMaywood changed the title ~~feat(coderd/agentapi): implement devcontainer agents api~~ feat(coderd/agentapi): implement sub agent api May 29, 2025

DanielleMaywood merged commit b712d0b into main May 29, 2025
36 checks passed

DanielleMaywood deleted the dm-devcontainer-agents-coderd branch May 29, 2025 11:15

github-actions bot locked and limited conversation to collaborators May 29, 2025

	subjectDevContainerAgentAPI = func(userID uuid.UUID, orgID uuid.UUID) rbac.Subject {
	subjectDevContainerAgentAPI = func(orgID uuid.UUID, userID uuid.UUID) rbac.Subject {

feat(coderd/agentapi): implement sub agent api #17823

feat(coderd/agentapi): implement sub agent api #17823

Uh oh!

Conversation

DanielleMaywood commented May 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

mafredri left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

mafredri left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DanielleMaywood commented May 14, 2025 •

edited

Loading