Skip to content

Commit 2099be8

Browse files
committed
fix(controlplane): tolerate orphaned project memberships in ListMembers
ProjectRepo.ListMembers now skips membership rows whose user/group no longer exists instead of returning 404, and decrements totalCount accordingly. memberships.member_id is polymorphic with no FK, so deletes that bypass the app-level cascade can leave dangling rows. Includes a one-shot migration that removes existing orphans. Closes #3082 Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
1 parent b19e267 commit 2099be8

4 files changed

Lines changed: 75 additions & 7 deletions

File tree

app/controlplane/pkg/biz/project_integration_test.go

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
//
2-
// Copyright 2025 The Chainloop Authors.
2+
// Copyright 2025-2026 The Chainloop Authors.
33
//
44
// Licensed under the Apache License, Version 2.0 (the "License");
55
// you may not use this file except in compliance with the License.
@@ -185,6 +185,18 @@ func (s *projectMembersIntegrationTestSuite) TestListMembers() {
185185
s.Error(err)
186186
s.True(biz.IsNotFound(err))
187187
})
188+
189+
s.Run("tolerates orphaned user memberships", func() {
190+
// Bypass the app-level cascade so the project membership row is left dangling.
191+
err := s.Data.DB.User.DeleteOneID(uuid.MustParse(user2.ID)).Exec(ctx)
192+
require.NoError(s.T(), err)
193+
194+
members, count, err := s.Project.ListMembers(ctx, uuid.MustParse(s.org.ID), projectRef, nil)
195+
s.NoError(err)
196+
s.Equal(1, len(members))
197+
s.Equal(1, count, "totalCount should reflect skipped orphan")
198+
s.Equal(user3.ID, members[0].User.ID)
199+
})
188200
}
189201

190202
// Test adding members to projects
@@ -1229,6 +1241,43 @@ func (s *projectGroupMembersIntegrationTestSuite) TestAddGroupToProject() {
12291241
})
12301242
}
12311243

1244+
func (s *projectGroupMembersIntegrationTestSuite) TestListMembersToleratesOrphanedGroups() {
1245+
ctx := context.Background()
1246+
1247+
projectID := s.project.ID
1248+
projectRef := &biz.IdentityReference{ID: &projectID}
1249+
1250+
_, err := s.Project.AddMemberToProject(ctx, uuid.MustParse(s.org.ID), &biz.AddMemberToProjectOpts{
1251+
ProjectReference: projectRef,
1252+
GroupReference: &biz.IdentityReference{ID: &s.group.ID},
1253+
RequesterID: *s.userUUID,
1254+
Role: authz.RoleProjectViewer,
1255+
})
1256+
require.NoError(s.T(), err)
1257+
1258+
survivingGroup, err := s.Group.Create(ctx, uuid.MustParse(s.org.ID), "surviving-group", "kept", s.userUUID)
1259+
require.NoError(s.T(), err)
1260+
1261+
_, err = s.Project.AddMemberToProject(ctx, uuid.MustParse(s.org.ID), &biz.AddMemberToProjectOpts{
1262+
ProjectReference: projectRef,
1263+
GroupReference: &biz.IdentityReference{ID: &survivingGroup.ID},
1264+
RequesterID: *s.userUUID,
1265+
Role: authz.RoleProjectAdmin,
1266+
})
1267+
require.NoError(s.T(), err)
1268+
1269+
// Bypass the app-level cascade in MembershipRepo.Delete so the membership row dangles.
1270+
err = s.Data.DB.Group.DeleteOneID(s.group.ID).Exec(ctx)
1271+
require.NoError(s.T(), err)
1272+
1273+
members, count, err := s.Project.ListMembers(ctx, uuid.MustParse(s.org.ID), projectRef, nil)
1274+
s.NoError(err)
1275+
s.Equal(1, len(members))
1276+
s.Equal(1, count, "totalCount should reflect skipped orphan")
1277+
s.NotNil(members[0].Group)
1278+
s.Equal(survivingGroup.ID, members[0].Group.ID)
1279+
}
1280+
12321281
// Test removing groups from projects
12331282
func (s *projectGroupMembersIntegrationTestSuite) TestRemoveGroupFromProject() {
12341283
ctx := context.Background()
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
-- atlas:txmode none
2+
3+
-- One-shot cleanup of orphaned memberships.
4+
-- memberships.member_id is polymorphic and has no FK to users/groups, so deletes
5+
-- that bypass the app-level cascade leave project/product membership rows pointing
6+
-- at vanished users or groups.
7+
DELETE FROM "memberships"
8+
WHERE "membership_type" = 'user'
9+
AND NOT EXISTS (SELECT 1 FROM "users" u WHERE u."id" = "memberships"."member_id");
10+
11+
DELETE FROM "memberships"
12+
WHERE "membership_type" = 'group'
13+
AND NOT EXISTS (SELECT 1 FROM "groups" g WHERE g."id" = "memberships"."member_id");

app/controlplane/pkg/data/ent/migrate/migrations/atlas.sum

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
h1:Mkh9OlAmAX9pbzA/X2mdwp/bouuIaDWHcTzH8DMHfOw=
1+
h1:qBVt7XbDlg3MT/tOTxZ6AwotFWXacU/xaweG4KC9WXQ=
22
20230706165452_init-schema.sql h1:VvqbNFEQnCvUVyj2iDYVQQxDM0+sSXqocpt/5H64k8M=
33
20230710111950-cas-backend.sql h1:A8iBuSzZIEbdsv9ipBtscZQuaBp3V5/VMw7eZH6GX+g=
44
20230712094107-cas-backends-workflow-runs.sql h1:a5rzxpVGyd56nLRSsKrmCFc9sebg65RWzLghKHh5xvI=
@@ -130,3 +130,4 @@ h1:Mkh9OlAmAX9pbzA/X2mdwp/bouuIaDWHcTzH8DMHfOw=
130130
20260408122048.sql h1:imfswpfmBlpP1l149/wCLN5HkN3/sGIQ3GnxaSnwOZE=
131131
20260416153232.sql h1:xjEfZuMOo1lgZm3VUYGHpNOhpJixncVZuMRg0jiH+7A=
132132
20260418100730.sql h1:lLcPDneBlzyabUAOIEKqCJgtnklHGac4JUnnZAbyD1g=
133+
20260430223706.sql h1:2mpbhdB4JWfMY8w9J8TGUj/O3tIT7dQpWfugTTI1eEQ=

app/controlplane/pkg/data/project.go

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
//
2-
// Copyright 2024 The Chainloop Authors.
2+
// Copyright 2024-2026 The Chainloop Authors.
33
//
44
// Licensed under the Apache License, Version 2.0 (the "License");
55
// you may not use this file except in compliance with the License.
@@ -153,21 +153,26 @@ func (r *ProjectRepo) ListMembers(ctx context.Context, orgID uuid.UUID, projectI
153153

154154
switch m.MembershipType {
155155
case authz.MembershipTypeUser:
156-
// Fetch the user details for user memberships
157156
u, uErr := r.data.DB.User.Get(ctx, m.MemberID)
158157
if uErr != nil {
158+
// Skip orphaned rows whose member_id no longer points at a real user.
159+
// memberships.member_id is polymorphic with no FK, so deletes that bypass
160+
// the app-level cascade can leave dangling rows here.
159161
if ent.IsNotFound(uErr) {
160-
return nil, 0, biz.NewErrNotFound("user")
162+
r.log.Warnf("orphaned project membership %s references missing user %s, skipping", m.ID, m.MemberID)
163+
totalCount--
164+
continue
161165
}
162166
return nil, 0, fmt.Errorf("failed to find user: %w", uErr)
163167
}
164168
mems = entProjectMembershipToBiz(m, u, nil)
165169
case authz.MembershipTypeGroup:
166-
// Fetch the group details for group memberships
167170
g, gErr := r.data.DB.Group.Get(ctx, m.MemberID)
168171
if gErr != nil {
169172
if ent.IsNotFound(gErr) {
170-
return nil, 0, biz.NewErrNotFound("group")
173+
r.log.Warnf("orphaned project membership %s references missing group %s, skipping", m.ID, m.MemberID)
174+
totalCount--
175+
continue
171176
}
172177
return nil, 0, fmt.Errorf("failed to find group: %w", gErr)
173178
}

0 commit comments

Comments
 (0)