Skip to content

Commit afb54f6

Browse files
johnstcnjohnstcn
authored
chore: revert feat(enterprise/coderd): allow system users to be added to groups (#19254)
This reverts commit b200fc8 (#18341).
1 parent b200fc8 commit afb54f6

File tree

8 files changed

+103
-524
lines changed

8 files changed

+103
-524
lines changed

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -485,16 +485,6 @@ var (
485485
rbac.ResourceFile.Type: {
486486
policy.ActionRead,
487487
},
488-
// Needs to be able to add the prebuilds system user to the "prebuilds" group in each organization that needs prebuilt workspaces
489-
// so that prebuilt workspaces can be scheduled and owned in those organizations.
490-
rbac.ResourceGroup.Type: {
491-
policy.ActionRead,
492-
policy.ActionCreate,
493-
policy.ActionUpdate,
494-
},
495-
rbac.ResourceGroupMember.Type: {
496-
policy.ActionRead,
497-
},
498488
}),
499489
},
500490
}),

‎coderd/database/queries.sql.go

Lines changed: 3 additions & 11 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/organizationmembers.sql

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,6 @@ WHERE
8989
organization_id = @organization_id
9090
ELSE true
9191
END
92-
-- Filter by system type
93-
AND CASE WHEN @include_system::bool THEN TRUE ELSE is_system = false END
9492
ORDER BY
9593
-- Deterministic and consistent ordering of all users. This is to ensure consistent pagination.
9694
LOWER(username) ASC OFFSET @offset_opt

‎coderd/members.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,6 @@ func (api *API) paginatedMembers(rw http.ResponseWriter, r *http.Request) {
203203

204204
paginatedMemberRows, err := api.Database.PaginatedOrganizationMembers(ctx, database.PaginatedOrganizationMembersParams{
205205
OrganizationID: organization.ID,
206-
IncludeSystem: false,
207206
// #nosec G115 - Pagination limits are small and fit in int32
208207
LimitOpt: int32(paginationParams.Limit),
209208
// #nosec G115 - Pagination offsets are small and fit in int32

‎docs/admin/templates/extending-templates/prebuilt-workspaces.md

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -235,18 +235,12 @@ The system always maintains the desired number of prebuilt workspaces for the ac
235235

236236
### Managing resource quotas
237237

238-
To help prevent unexpected infrastructure costs, prebuilt workspaces can be used in conjunction with [resource quotas](../../users/quotas.md).
238+
Prebuilt workspaces can be used in conjunction with [resource quotas](../../users/quotas.md).
239239
Because unclaimed prebuilt workspaces are owned by the `prebuilds` user, you can:
240240

241241
1. Configure quotas for any group that includes this user.
242242
1. Set appropriate limits to balance prebuilt workspace availability with resource constraints.
243243

244-
When prebuilt workspaces are configured for an organization, Coder creates a "prebuilds" group in that organization and adds the prebuilds user to it. This group has a default quota allowance of 0, which you should adjust based on your needs:
245-
246-
- **Set a quota allowance** on the "prebuilds" group to control how many prebuilt workspaces can be provisioned
247-
- **Monitor usage** to ensure the quota is appropriate for your desired number of prebuilt instances
248-
- **Adjust as needed** based on your template costs and desired prebuilt workspace pool size
249-
250244
If a quota is exceeded, the prebuilt workspace will fail provisioning the same way other workspaces do.
251245

252246
### Template configuration best practices

‎enterprise/coderd/prebuilds/membership.go

Lines changed: 21 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,6 @@ import (
1212
"github.com/coder/quartz"
1313
)
1414

15-
const (
16-
PrebuiltWorkspacesGroupName = "coder_prebuilt_workspaces"
17-
PrebuiltWorkspacesGroupDisplayName = "Prebuilt Workspaces"
18-
)
19-
2015
// StoreMembershipReconciler encapsulates the responsibility of ensuring that the prebuilds system user is a member of all
2116
// organizations for which prebuilt workspaces are requested. This is necessary because our data model requires that such
2217
// prebuilt workspaces belong to a member of the organization of their eventual claimant.
@@ -32,16 +27,11 @@ func NewStoreMembershipReconciler(store database.Store, clock quartz.Clock) Stor
3227
}
3328
}
3429

35-
// ReconcileAll compares the current organization and group memberships of a user to the memberships required
36-
// in order to create prebuilt workspaces. If the user in question is not yet a member of an organization that
37-
// needs prebuilt workspaces, ReconcileAll will create the membership required.
30+
// ReconcileAll compares the current membership of a user to the membership required in order to create prebuilt workspaces.
31+
// If the user in question is not yet a member of an organization that needs prebuilt workspaces, ReconcileAll will create
32+
// the membership required.
3833
//
39-
// To facilitate quota management, ReconcileAll will ensure:
40-
// * the existence of a group (defined by PrebuiltWorkspacesGroupName) in each organization that needs prebuilt workspaces
41-
// * that the prebuilds system user belongs to the group in each organization that needs prebuilt workspaces
42-
// * that the group has a quota of 0 by default, which users can adjust based on their needs.
43-
//
44-
// ReconcileAll does not have an opinion on transaction or lock management. These responsibilities are left to the caller.
34+
// This method does not have an opinion on transaction or lock management. These responsibilities are left to the caller.
4535
func (s StoreMembershipReconciler) ReconcileAll(ctx context.Context, userID uuid.UUID, presets []database.GetTemplatePresetsWithPrebuildsRow) error {
4636
organizationMemberships, err := s.store.GetOrganizationsByUserID(ctx, database.GetOrganizationsByUserIDParams{
4737
UserID: userID,
@@ -54,74 +44,37 @@ func (s StoreMembershipReconciler) ReconcileAll(ctx context.Context, userID uuid
5444
return xerrors.Errorf("determine prebuild organization membership: %w", err)
5545
}
5646

57-
orgMemberships := make(map[uuid.UUID]struct{}, 0)
47+
systemUserMemberships := make(map[uuid.UUID]struct{}, 0)
5848
defaultOrg, err := s.store.GetDefaultOrganization(ctx)
5949
if err != nil {
6050
return xerrors.Errorf("get default organization: %w", err)
6151
}
62-
orgMemberships[defaultOrg.ID] = struct{}{}
52+
systemUserMemberships[defaultOrg.ID] = struct{}{}
6353
for _, o := range organizationMemberships {
64-
orgMemberships[o.ID] = struct{}{}
54+
systemUserMemberships[o.ID] = struct{}{}
6555
}
6656

6757
var membershipInsertionErrors error
6858
for _, preset := range presets {
69-
_, alreadyOrgMember := orgMemberships[preset.OrganizationID]
70-
if !alreadyOrgMember {
71-
// Add the organization to our list of memberships regardless of potential failure below
72-
// to avoid a retry that will probably be doomed anyway.
73-
orgMemberships[preset.OrganizationID] = struct{}{}
74-
75-
// Insert the missing membership
76-
_, err = s.store.InsertOrganizationMember(ctx, database.InsertOrganizationMemberParams{
77-
OrganizationID: preset.OrganizationID,
78-
UserID: userID,
79-
CreatedAt: s.clock.Now(),
80-
UpdatedAt: s.clock.Now(),
81-
Roles: []string{},
82-
})
83-
if err != nil {
84-
membershipInsertionErrors = errors.Join(membershipInsertionErrors, xerrors.Errorf("insert membership for prebuilt workspaces: %w", err))
85-
continue
86-
}
59+
_, alreadyMember := systemUserMemberships[preset.OrganizationID]
60+
if alreadyMember {
61+
continue
8762
}
63+
// Add the organization to our list of memberships regardless of potential failure below
64+
// to avoid a retry that will probably be doomed anyway.
65+
systemUserMemberships[preset.OrganizationID] = struct{}{}
8866

89-
// Create a "prebuilds" group in the organization and add the system user to it
90-
// This group will have a quota of 0 by default, which users can adjust based on their needs
91-
prebuildsGroup, err := s.store.InsertGroup(ctx, database.InsertGroupParams{
92-
ID: uuid.New(),
93-
Name: PrebuiltWorkspacesGroupName,
94-
DisplayName: PrebuiltWorkspacesGroupDisplayName,
67+
// Insert the missing membership
68+
_, err = s.store.InsertOrganizationMember(ctx, database.InsertOrganizationMemberParams{
9569
OrganizationID: preset.OrganizationID,
96-
AvatarURL: "",
97-
QuotaAllowance: 0, // Default quota of 0, users should set this based on their needs
98-
})
99-
if err != nil {
100-
// If the group already exists, try to get it
101-
if !database.IsUniqueViolation(err) {
102-
membershipInsertionErrors = errors.Join(membershipInsertionErrors, xerrors.Errorf("create prebuilds group: %w", err))
103-
continue
104-
}
105-
prebuildsGroup, err = s.store.GetGroupByOrgAndName(ctx, database.GetGroupByOrgAndNameParams{
106-
OrganizationID: preset.OrganizationID,
107-
Name: PrebuiltWorkspacesGroupName,
108-
})
109-
if err != nil {
110-
membershipInsertionErrors = errors.Join(membershipInsertionErrors, xerrors.Errorf("get existing prebuilds group: %w", err))
111-
continue
112-
}
113-
}
114-
115-
// Add the system user to the prebuilds group
116-
err = s.store.InsertGroupMember(ctx, database.InsertGroupMemberParams{
117-
GroupID: prebuildsGroup.ID,
118-
UserID: userID,
70+
UserID: userID,
71+
CreatedAt: s.clock.Now(),
72+
UpdatedAt: s.clock.Now(),
73+
Roles: []string{},
11974
})
12075
if err != nil {
121-
// Ignore unique violation errors as the user might already be in the group
122-
if !database.IsUniqueViolation(err) {
123-
membershipInsertionErrors = errors.Join(membershipInsertionErrors, xerrors.Errorf("add system user to prebuilds group: %w", err))
124-
}
76+
membershipInsertionErrors = errors.Join(membershipInsertionErrors, xerrors.Errorf("insert membership for prebuilt workspaces: %w", err))
77+
continue
12578
}
12679
}
12780
return membershipInsertionErrors

0 commit comments

Comments
 (0)