This PR optimizes the agent API by using theworkspace.OwnerUsername field directly instead of making an additional database query to fetch the owner's username. The change removes the need to callGetUserByID in the manifest API and workspace agent RPC endpoints.

An issue arose when the agent token was scoped without access to user data (api_key_scope = "no_user_data"), causing the agent to fail to fetch the manifest due to an RBAC issue.

Change-Id: I3b6e7581134e2374b364ee059e3b18ece3d98b41
Signed-off-by: Thomas Kosiewskitk@coder.com

ThomasK33 self-assigned this

May 20, 2025

Copy link

MemberAuthor

ThomasK33 commentedMay 20, 2025

fix: remove unnecessary user lookup in agent API calls #17934 👈(View in Graphite)
main

This stack of pull requests is managed byGraphite. Learn more aboutstacking.

ThomasK33 requested a review fromdannykopping

May 20, 2025 11:05

ThomasK33 force-pushed thethomask33/05-20-fix_remove_unnecessary_user_lookup_in_agent_api_calls branch frome0f806f tob1f8370Compare

May 20, 2025 11:12

ThomasK33 marked this pull request as ready for review

May 20, 2025 11:35

dannykopping approved these changes

May 20, 2025

View reviewed changes

Copy link

Contributor

dannykopping left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

LGTM, although I'm not sure how far this approach of restricting agent access with RBAC will take us.

Copy link

Member

Emyrk commentedMay 20, 2025

We've had this issue that we need username, template name, workspace name, etc. So we fetch the original object for that 1 or two fields, but the caller might not have access to this related object.

We have created some views likeworkspace_build_with_user to just join in these fields, and not require the RBAC to the original user.

The views are awkward to manage, but solves the same issue you are hitting. Since agent keys can only hit a small subset of routes, I think we can manage its implications.

Copy link

MemberAuthor

ThomasK33 commentedMay 20, 2025•
edited
Loading

Oh, that's a great point. I just checked the SQL queries for the call ofGetWorkspaceByID, and it uses theworkspaces_expanded (view) (query) under the hood.

So the information is already getting joined as you suggested. I guess those follow-up calls were genuinely redundant.

fix: remove unnecessary user lookup in agent API calls

840c0db

Change-Id: I3b6e7581134e2374b364ee059e3b18ece3d98b41Signed-off-by: Thomas Kosiewski <tk@coder.com>

ThomasK33 force-pushed thethomask33/05-20-fix_remove_unnecessary_user_lookup_in_agent_api_calls branch fromb1f8370 to840c0dbCompare

May 20, 2025 14:55

Copy link

Member

Emyrk commentedMay 20, 2025

So the information is already getting joined as you suggested. I guess those follow-up calls were genuinely redundant.

Yup! I imagine there are a few other places that are redundant now with the view

Emyrk approved these changes

May 20, 2025

View reviewed changes

ThomasK33 merged commit93f17bc intomain

May 20, 2025

37 checks passed

Copy link

MemberAuthor