Spoke with@deansheather and he agrees and thinks this is a good way to solve this over nulling out the auth_token fields, so I'm going to spend some time figuring out the root cause of all the test failures - I'm assuming it's just related to the way we do some common test scaffolding.

@deansheather Added the test

I'm concerned that this is fragile becauseGetWorkspaceAgentAndOwnerByAuthToken will still happily return the agent for non-latest builds. I realize that the middleware won't authorize non-latest builds and we use middleware for authentication pretty much everywhere right now, but I think we could make stronger guarantees and leave fewer sharp edges in our code if droppedGetWorkspaceAgentAndOwnerByAuthToken in favor of a single SQL query that takes the token and returns the agent and build (via joins) if it's the latest, and nothing if it's invalid or not the latest. It would also reduce the number of DB roundtrips to boot.

Since I'm not the strongest SQL person, I'd like to propose the query before i go down the rabbit whole of changing all the interfaces and usages. Are you thinking something like this?

-- name: GetLatestWorkspaceAgentAndBuildAndOwnerByAuthToken :oneWITH LatestBuildsAS (SELECTworkspaces.idAS workspace_id,MAX(workspace_builds.build_number)AS latest_build_numberFROM workspacesINNER JOIN workspace_buildsONworkspace_builds.workspace_id=workspaces.idWHEREworkspaces.deleted= FALSEGROUP BYworkspaces.id)SELECTsqlc.embed(workspace_agents),workspaces.idAS workspace_id,users.idAS owner_id,users.usernameAS owner_name,users.statusAS owner_status,workspaces.template_idAS template_id,sqlc.embed(workspace_builds),    array_cat(        array_append(users.rbac_roles,'member'),        array_append(ARRAY[]::text[],'organization-member:'||organization_members.organization_id::text)    )::text[]as owner_roles,    array_agg(COALESCE(group_members.group_id::text,''))::text[]AS owner_groupsFROM usersINNER JOIN workspacesONworkspaces.owner_id=users.idINNER JOIN workspace_buildsONworkspace_builds.workspace_id=workspaces.idINNER JOIN LatestBuildsONworkspaces.id=LatestBuilds.workspace_idANDworkspace_builds.build_number=LatestBuilds.latest_build_numberINNER JOIN workspace_resourcesONworkspace_resources.job_id=workspace_builds.job_idINNER JOIN workspace_agentsONworkspace_agents.resource_id=workspace_resources.idINNER JOIN organization_membersONorganization_members.user_id=users.idLEFT JOIN group_membersONgroup_members.user_id=users.idWHEREworkspace_agents.auth_token= @auth_tokenGROUP BYworkspace_agents.id,workspaces.id,users.id,organization_members.organization_id,workspace_builds.build_numberORDER BYworkspace_builds.build_numberDESCLIMIT1;

This generates the struct I think I would want, a row with both row.WorkspaceAgent and row.WorkspaceBuildTable. I believe this will return no rows when the auth token is not from the latest build. Does this seem like the direction you were thinking?

Yeah, that's the kind of thing I was thinking.

BTW, I think theLIMIT 1 is superfluous if the query is working correctly because we have a unique constraint on workspace_builds (workspace_id, build_number), which implies that exactly one build can be the "latest".

I also think it would read more clearly if you moved theworkspace_builds.build_number = LatestBuilds.latest_build_number clause toWHERE rather than the join condition. This is just style, but to me it's easier to understand that

we're taking the workspace and looking up the latest build number, then only selecting rows where it's the latest build

vs

we're taking the workspace and then joining to a set of rows that is empty when it's not the latest

@spikecurtis I've spoken with@Emyrk on how to accomplish this query, and we've landed on the following changes:

1. GetWorkspaceAgentAndOwnerByAuthToken changed toGetWorkspaceAgentAndLatestBuildByAuthToken - rbac stuff was removed from query to simplify, array_agg was making things a mess. Since this query is only used here it's safe to change.
2. Get rbac info from a call toDB.GetAuthorizationUserRoles(dbauthz.AsSystemRestricted(ctx), row.Workspace.OwnerID) in the middleware now.

Tests are all passing now so I think this is good for review. I've added@Emyrk as a reviewer as well.