Skip to content

Commit cf10d98

Browse files
brettkolodnybrettkolodny
authored
fix: improve error message when deleting organization with resources (coder#17049)
Closes [coder/internal#477](coder/internal#477) ![Screenshot 2025-03-21 at 11 25 57 AM](https://github.com/user-attachments/assets/50cc03e9-395d-4fc7-8882-18cb66b1fac9) I'm solving this issue in two parts: 1. Updated the postgres function so that it doesn't omit 0 values in the error 2. Created a new query to fetch the number of resources associated with an organization and using that information to provider a cleaner error message to the frontend > **_NOTE:_** SQL is not my strong suit, and the code was created with the help of AI. So I'd take extra time looking over what I wrote there
1 parent 2c53f7a commit cf10d98

13 files changed

+416
-22
lines changed

‎coderd/database/dbauthz/dbauthz.go

+29
Original file line numberDiff line numberDiff line change
@@ -1989,6 +1989,35 @@ func (q *querier) GetOrganizationIDsByMemberIDs(ctx context.Context, ids []uuid.
19891989
return fetchWithPostFilter(q.auth, policy.ActionRead, q.db.GetOrganizationIDsByMemberIDs)(ctx, ids)
19901990
}
19911991

1992+
func (q *querier) GetOrganizationResourceCountByID(ctx context.Context, organizationID uuid.UUID) (database.GetOrganizationResourceCountByIDRow, error) {
1993+
// Can read org members
1994+
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceOrganizationMember.InOrg(organizationID)); err != nil {
1995+
return database.GetOrganizationResourceCountByIDRow{}, err
1996+
}
1997+
1998+
// Can read org workspaces
1999+
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceWorkspace.InOrg(organizationID)); err != nil {
2000+
return database.GetOrganizationResourceCountByIDRow{}, err
2001+
}
2002+
2003+
// Can read org groups
2004+
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceGroup.InOrg(organizationID)); err != nil {
2005+
return database.GetOrganizationResourceCountByIDRow{}, err
2006+
}
2007+
2008+
// Can read org templates
2009+
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceTemplate.InOrg(organizationID)); err != nil {
2010+
return database.GetOrganizationResourceCountByIDRow{}, err
2011+
}
2012+
2013+
// Can read org provisioner daemons
2014+
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceProvisionerDaemon.InOrg(organizationID)); err != nil {
2015+
return database.GetOrganizationResourceCountByIDRow{}, err
2016+
}
2017+
2018+
return q.db.GetOrganizationResourceCountByID(ctx, organizationID)
2019+
}
2020+
19922021
func (q *querier) GetOrganizations(ctx context.Context, args database.GetOrganizationsParams) ([]database.Organization, error) {
19932022
fetch := func(ctx context.Context, _ interface{}) ([]database.Organization, error) {
19942023
return q.db.GetOrganizations(ctx, args)

‎coderd/database/dbauthz/dbauthz_test.go

+33
Original file line numberDiff line numberDiff line change
@@ -815,6 +815,39 @@ func (s *MethodTestSuite) TestOrganization() {
815815
o := dbgen.Organization(s.T(), db, database.Organization{})
816816
check.Args(o.ID).Asserts(o, policy.ActionRead).Returns(o)
817817
}))
818+
s.Run("GetOrganizationResourceCountByID", s.Subtest(func(db database.Store, check *expects) {
819+
u := dbgen.User(s.T(), db, database.User{})
820+
o := dbgen.Organization(s.T(), db, database.Organization{})
821+
822+
t := dbgen.Template(s.T(), db, database.Template{
823+
CreatedBy: u.ID,
824+
OrganizationID: o.ID,
825+
})
826+
dbgen.Workspace(s.T(), db, database.WorkspaceTable{
827+
OrganizationID: o.ID,
828+
OwnerID: u.ID,
829+
TemplateID: t.ID,
830+
})
831+
dbgen.Group(s.T(), db, database.Group{OrganizationID: o.ID})
832+
dbgen.OrganizationMember(s.T(), db, database.OrganizationMember{
833+
OrganizationID: o.ID,
834+
UserID: u.ID,
835+
})
836+
837+
check.Args(o.ID).Asserts(
838+
rbac.ResourceOrganizationMember.InOrg(o.ID), policy.ActionRead,
839+
rbac.ResourceWorkspace.InOrg(o.ID), policy.ActionRead,
840+
rbac.ResourceGroup.InOrg(o.ID), policy.ActionRead,
841+
rbac.ResourceTemplate.InOrg(o.ID), policy.ActionRead,
842+
rbac.ResourceProvisionerDaemon.InOrg(o.ID), policy.ActionRead,
843+
).Returns(database.GetOrganizationResourceCountByIDRow{
844+
WorkspaceCount: 1,
845+
GroupCount: 1,
846+
TemplateCount: 1,
847+
MemberCount: 1,
848+
ProvisionerKeyCount: 0,
849+
})
850+
}))
818851
s.Run("GetDefaultOrganization", s.Subtest(func(db database.Store, check *expects) {
819852
o, _ := db.GetDefaultOrganization(context.Background())
820853
check.Args().Asserts(o, policy.ActionRead).Returns(o)

‎coderd/database/dbmem/dbmem.go

+48
Original file line numberDiff line numberDiff line change
@@ -4008,6 +4008,54 @@ func (q *FakeQuerier) GetOrganizationIDsByMemberIDs(_ context.Context, ids []uui
40084008
return getOrganizationIDsByMemberIDRows, nil
40094009
}
40104010

4011+
func (q *FakeQuerier) GetOrganizationResourceCountByID(_ context.Context, organizationID uuid.UUID) (database.GetOrganizationResourceCountByIDRow, error) {
4012+
q.mutex.RLock()
4013+
defer q.mutex.RUnlock()
4014+
4015+
workspacesCount := 0
4016+
for _, workspace := range q.workspaces {
4017+
if workspace.OrganizationID == organizationID {
4018+
workspacesCount++
4019+
}
4020+
}
4021+
4022+
groupsCount := 0
4023+
for _, group := range q.groups {
4024+
if group.OrganizationID == organizationID {
4025+
groupsCount++
4026+
}
4027+
}
4028+
4029+
templatesCount := 0
4030+
for _, template := range q.templates {
4031+
if template.OrganizationID == organizationID {
4032+
templatesCount++
4033+
}
4034+
}
4035+
4036+
organizationMembersCount := 0
4037+
for _, organizationMember := range q.organizationMembers {
4038+
if organizationMember.OrganizationID == organizationID {
4039+
organizationMembersCount++
4040+
}
4041+
}
4042+
4043+
provKeyCount := 0
4044+
for _, provKey := range q.provisionerKeys {
4045+
if provKey.OrganizationID == organizationID {
4046+
provKeyCount++
4047+
}
4048+
}
4049+
4050+
return database.GetOrganizationResourceCountByIDRow{
4051+
WorkspaceCount: int64(workspacesCount),
4052+
GroupCount: int64(groupsCount),
4053+
TemplateCount: int64(templatesCount),
4054+
MemberCount: int64(organizationMembersCount),
4055+
ProvisionerKeyCount: int64(provKeyCount),
4056+
}, nil
4057+
}
4058+
40114059
func (q *FakeQuerier) GetOrganizations(_ context.Context, args database.GetOrganizationsParams) ([]database.Organization, error) {
40124060
q.mutex.RLock()
40134061
defer q.mutex.RUnlock()

‎coderd/database/dbmetrics/querymetrics.go

+7
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/dbmock/dbmock.go

+15
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/dump.sql

+38-19
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/migrations/000310_update_protect_deleting_organization_function.down.sql

+77
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
-- Drop trigger that uses this function
2+
DROP TRIGGER IF EXISTS protect_deleting_organizations ON organizations;
3+
4+
-- Revert the function to its original implementation
5+
CREATE OR REPLACE FUNCTION protect_deleting_organizations()
6+
RETURNS TRIGGER AS
7+
$$
8+
DECLARE
9+
workspace_count int;
10+
template_count int;
11+
group_count int;
12+
member_count int;
13+
provisioner_keys_count int;
14+
BEGIN
15+
workspace_count := (
16+
SELECT count(*) as count FROM workspaces
17+
WHERE
18+
workspaces.organization_id = OLD.id
19+
AND workspaces.deleted = false
20+
);
21+
22+
template_count := (
23+
SELECT count(*) as count FROM templates
24+
WHERE
25+
templates.organization_id = OLD.id
26+
AND templates.deleted = false
27+
);
28+
29+
group_count := (
30+
SELECT count(*) as count FROM groups
31+
WHERE
32+
groups.organization_id = OLD.id
33+
);
34+
35+
member_count := (
36+
SELECT count(*) as count FROM organization_members
37+
WHERE
38+
organization_members.organization_id = OLD.id
39+
);
40+
41+
provisioner_keys_count := (
42+
Select count(*) as count FROM provisioner_keys
43+
WHERE
44+
provisioner_keys.organization_id = OLD.id
45+
);
46+
47+
-- Fail the deletion if one of the following:
48+
-- * the organization has 1 or more workspaces
49+
-- * the organization has 1 or more templates
50+
-- * the organization has 1 or more groups other than "Everyone" group
51+
-- * the organization has 1 or more members other than the organization owner
52+
-- * the organization has 1 or more provisioner keys
53+
54+
IF (workspace_count + template_count + provisioner_keys_count) > 0 THEN
55+
RAISE EXCEPTION 'cannot delete organization: organization has % workspaces, % templates, and % provisioner keys that must be deleted first', workspace_count, template_count, provisioner_keys_count;
56+
END IF;
57+
58+
IF (group_count) > 1 THEN
59+
RAISE EXCEPTION 'cannot delete organization: organization has % groups that must be deleted first', group_count - 1;
60+
END IF;
61+
62+
-- Allow 1 member to exist, because you cannot remove yourself. You can
63+
-- remove everyone else. Ideally, we only omit the member that matches
64+
-- the user_id of the caller, however in a trigger, the caller is unknown.
65+
IF (member_count) > 1 THEN
66+
RAISE EXCEPTION 'cannot delete organization: organization has % members that must be deleted first', member_count - 1;
67+
END IF;
68+
69+
RETURN NEW;
70+
END;
71+
$$ LANGUAGE plpgsql;
72+
73+
-- Re-create trigger that uses this function
74+
CREATE TRIGGER protect_deleting_organizations
75+
BEFORE DELETE ON organizations
76+
FOR EACH ROW
77+
EXECUTE FUNCTION protect_deleting_organizations();

0 commit comments

Comments
 (0)