-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix: pass context with authorization to agentapi #20959
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
authorization attached to it via the context so that the cache refresh routine can fetch the workspace from the db via GetWorkspaceForAgentID Signed-off-by: Callum Styan <callumstyan@gmail.com>
coderd/workspaceagentsrpc.go
Outdated
| OrganizationID: workspace.OrganizationID, | ||
|
|
||
| Ctx: api.ctx, | ||
| Ctx: ctx, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this context is only used in startCacheRefreshLoop?
If so, we can rename Ctx to like AuthenticatedCtx or something akin to that? So users of it know what it carries?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, it's not passed to any of the sub API's and afaict none of the other functions directly on the API use it either.
Signed-off-by: Callum Styan <callumstyan@gmail.com>
coderd/agentapi/api.go
Outdated
| // This ensures that changes like prebuild claims (which modify owner_id, name, etc.) | ||
| // are eventually reflected in the cache without requiring agent reconnection. | ||
| func (a *API) refreshCachedWorkspace(ctx context.Context) { | ||
| a.opts.Log.Debug(ctx, "refreshing cached workspace", slog.F("test", "test")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Accidental log? test:test
Signed-off-by: Callum Styan <callumstyan@gmail.com>
The agentapi context needs to be a context with some amount of authorization attached to it via the context so that the cache refresh routine can fetch the workspace from the db via GetWorkspaceForAgentID.