- Notifications
You must be signed in to change notification settings - Fork1.1k
fix(coderd): ensure lifecycle executor has sufficient task permissions#20539
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
4a360c9 to4ad5bdeCompareWe recently made a change to the `wsbuilder` to handle task relatedlogic. Our test coverage for the lifecycle executor didn't handle thisscenario and so we missed the insufficient permissions.This PR adds `Update` and `Read` permissions for `Task`s in thelifecycle executor, as well as an autostart/autostop test tailored totask workspaces to verify the change.
4ad5bde to6cb618cCompare| funcTestExecutorTaskWorkspace(t*testing.T) { | ||
| t.Parallel() | ||
| createTaskTemplate:=func(t*testing.T,client*codersdk.Client,orgID uuid.UUID,ctx context.Context,defaultTTL time.Duration) codersdk.Template { |
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.
Can we avoid duplication here? I'm fine to defer cleanup to later though.
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.
What thoughts do you have here? Is this related to how many similar test utilities we have for creating task templates?
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.
Yup, similar test setups, but let's defer 👍🏻
06dbada intomainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
We recently made a change to the
wsbuilderto handle task related logic. Our test coverage for the lifecycle executor didn't handle this scenario and so we missed that it had insufficient permissions.This PR adds
UpdateandReadpermissions forTasks in the lifecycle executor, as well as an autostart/autostop test tailored to task workspaces to verify the change.Anthropic's Claude Sonnet 4.5 Thinking was involved in writing the tests