Skip to content

feat(enterprise/coderd): allow system users to be added to groups #18341

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

SasSwart
Copy link
Contributor

@SasSwart SasSwart commented Jun 12, 2025

closes #18274

This pull request makes system users visible in various group related queries so that they can be added to groups. This allows system user quotas to be configured. System users are still ignored in certain queries, such as when license seat consumption is determined.

@SasSwart SasSwart changed the title feat: allow system users to be added to groups feat(enterprise/coderd): allow system users to be added to groups Jun 12, 2025
@github-actions github-actions bot added the stale This issue is like stale bread. label Jun 20, 2025
@github-actions github-actions bot closed this Jun 23, 2025
@johnstcn johnstcn reopened this Jun 23, 2025
@johnstcn johnstcn removed the stale This issue is like stale bread. label Jun 23, 2025
@github-actions github-actions bot added the stale This issue is like stale bread. label Jul 1, 2025
@github-actions github-actions bot closed this Jul 5, 2025
@SasSwart SasSwart reopened this Jul 14, 2025
@SasSwart SasSwart removed the stale This issue is like stale bread. label Jul 14, 2025
@SasSwart SasSwart requested a review from ssncferreira July 14, 2025 13:55
@SasSwart SasSwart marked this pull request as ready for review July 14, 2025 13:55
Copy link
Contributor

@ssncferreira ssncferreira left a comment

Choose a reason for hiding this comment

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

I'm wondering about this approach. Do we need to add IncludeSystem: true to all these queries just to support the prebuilds system user?
This means someone will need to manually create a group for the prebuilds user and assign it a quota, right?

Wouldn't it be more robust to automatically create a group for the prebuilds system user with a default quota, and allow users to override it if needed? That way, we avoid having to include system users in these queries and reduce the risk of system users leaking into user-facing features.

Comment on lines +92 to +97
-- Filter by system type
AND CASE
WHEN @include_system::bool THEN TRUE
ELSE
is_system = false
END
Copy link
Contributor

Choose a reason for hiding this comment

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

nit for readability:

Suggested change
-- Filter by system type
AND CASE
WHEN @include_system::bool THEN TRUE
ELSE
is_system = false
END
-- Conditionally include system users based on @include_system flag
AND (@include_system OR is_system = false)

@SasSwart
Copy link
Contributor Author

The prebuilds user has already leaked into user facing features. Administrators search for prebuilds by specifying the prebuilds user on the workspace list page. For consistency and clarity, I think we should embrace listing and showing the prebuilds user.

I like the idea of automatically creating a prebuilds group, but we'll need to decide what a sensible default quota would be if there is such a value that makes sense.

@ssncferreira
Copy link
Contributor

ssncferreira commented Jul 16, 2025

The prebuilds user has already leaked into user facing features. Administrators search for prebuilds by specifying the prebuilds user on the workspace list page. For consistency and clarity, I think we should embrace listing and showing the prebuilds user.

It’s true that the prebuilds user is already exposed in some user-facing features, like showing prebuilt workspaces, but that might not be the ideal approach 😕 The issue isn’t just about the prebuilds user, it extends to listing all current and future system users.

I like the idea of automatically creating a prebuilds group, but we'll need to decide what a sensible default quota would be if there is such a value that makes sense.

We already have a precedent with the Everyone group:

-- name: InsertAllUsersGroup :one
INSERT INTO groups (
id,
name,
organization_id
)
VALUES
(sqlc.arg(organization_id), 'Everyone', sqlc.arg(organization_id)) RETURNING *;

We could apply the same pattern here: automatically create the prebuilds group with a default quota of 0, and clearly document this behavior along with instructions on how users can update the quota if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: Workspace prebuilds conform to the site quota, leading many to fail
3 participants