From eeec4a8aa8c60f80b45472e55b1caf5201ed775d Mon Sep 17 00:00:00 2001 From: aman Date: Mon, 19 Oct 2026 13:03:01 +0530 Subject: [PATCH 1/2] feat(store): project delete keeps the row The project row now gets deleted_at instead of being removed. The project.deleted audit record is written in the same transaction, with the row's delete time. Deleting a project that is already deleted, or does not exist, returns not found. An e2e test checks that everyone who had access loses it and that every policy on the project is marked deleted. --- internal/store/postgres/project_repository.go | 13 +- .../store/postgres/project_repository_test.go | 31 +++- test/e2e/regression/api_test.go | 162 ++++++++++++++++++ 3 files changed, 192 insertions(+), 14 deletions(-) diff --git a/internal/store/postgres/project_repository.go b/internal/store/postgres/project_repository.go index 5c2dbf833..d1f4bc3bc 100644 --- a/internal/store/postgres/project_repository.go +++ b/internal/store/postgres/project_repository.go @@ -381,7 +381,7 @@ func (r ProjectRepository) SetState(ctx context.Context, id string, state projec } func (r ProjectRepository) Delete(ctx context.Context, id string) error { - query, params, err := dialect.Delete(TABLE_PROJECTS).Where( + query, params, err := softDelete(TABLE_PROJECTS).Where( goqu.Ex{ "id": id, }, @@ -392,17 +392,12 @@ func (r ProjectRepository) Delete(ctx context.Context, id string) error { if err = r.dbc.WithTxn(ctx, sql.TxOptions{}, func(tx *sqlx.Tx) error { var result projectWithOrgName - err := r.dbc.WithTimeout(ctx, TABLE_PROJECTS, "Delete", func(ctx context.Context) error { + if err := r.dbc.WithTimeout(ctx, TABLE_PROJECTS, "Delete", func(ctx context.Context) error { return tx.QueryRowxContext(ctx, query, params...).StructScan(&result) - }) - if errors.Is(err, sql.ErrNoRows) { - // already gone: nothing deleted, nothing to audit - return nil - } - if err != nil { + }); err != nil { return err } - return InsertAuditRecordInTx(ctx, tx, buildProjectAuditRecord(ctx, auditrecord.ProjectDeletedEvent, result, time.Now().UTC())) + return InsertAuditRecordInTx(ctx, tx, buildProjectAuditRecord(ctx, auditrecord.ProjectDeletedEvent, result, result.DeletedAt.Time)) }); err != nil { err = checkPostgresError(err) switch { diff --git a/internal/store/postgres/project_repository_test.go b/internal/store/postgres/project_repository_test.go index 575ab2176..be0138b09 100644 --- a/internal/store/postgres/project_repository_test.go +++ b/internal/store/postgres/project_repository_test.go @@ -309,15 +309,36 @@ func (s *ProjectRepositoryTestSuite) TestCreate() { } func (s *ProjectRepositoryTestSuite) TestDelete() { - s.Run("should delete a project and audit it", func() { - s.Require().NoError(s.repository.Delete(s.ctx, s.projects[1].ID)) - s.Equal(1, s.auditCount(pkgAuditRecord.ProjectDeletedEvent, s.projects[1].ID)) + deleted := s.projects[1] + + s.Run("keeps the row with deleted_at set, hides it, and audits the delete", func() { + s.Require().NoError(s.repository.Delete(s.ctx, deleted.ID)) + s.Equal(1, s.auditCount(pkgAuditRecord.ProjectDeletedEvent, deleted.ID)) + + var deletedRows int + s.Require().NoError(s.client.QueryRowxContext(s.ctx, + "SELECT count(*) FROM projects WHERE id = $1 AND deleted_at IS NOT NULL", deleted.ID).Scan(&deletedRows)) + s.Equal(1, deletedRows) + + _, err := s.repository.GetByID(s.ctx, deleted.ID) + s.ErrorIs(err, project.ErrNotExist) }) - s.Run("should skip a project that does not exist without auditing", func() { + s.Run("a second delete returns not found and writes no second audit record", func() { + s.ErrorIs(s.repository.Delete(s.ctx, deleted.ID), project.ErrNotExist) + s.Equal(1, s.auditCount(pkgAuditRecord.ProjectDeletedEvent, deleted.ID)) + }) + s.Run("a project that does not exist returns not found without an audit record", func() { id := uuid.NewString() - s.Require().NoError(s.repository.Delete(s.ctx, id)) + s.ErrorIs(s.repository.Delete(s.ctx, id), project.ErrNotExist) s.Equal(0, s.auditCount(pkgAuditRecord.ProjectDeletedEvent, id)) }) + s.Run("a new project can take the deleted project's name", func() { + _, err := s.repository.Create(s.ctx, project.Project{ + Name: deleted.Name, + Organization: deleted.Organization, + }) + s.NoError(err) + }) } func (s *ProjectRepositoryTestSuite) TestList() { diff --git a/test/e2e/regression/api_test.go b/test/e2e/regression/api_test.go index 4a46ebced..f8a930bb0 100644 --- a/test/e2e/regression/api_test.go +++ b/test/e2e/regression/api_test.go @@ -3783,6 +3783,168 @@ func (s *APIRegressionTestSuite) TestOrganizationDeleteAlsoDeletesDisabledProjec }) } +func (s *APIRegressionTestSuite) TestProjectDeleteKeepsTheRowAndRemovesAccess() { + ctxOrgAdminAuth := testbench.ContextWithAuth(context.Background(), s.adminCookie) + + createOrgResp, err := s.testBench.Client.CreateOrganization(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateOrganizationRequest{ + Body: &frontierv1beta1.OrganizationRequestBody{ + Title: "org project soft delete", + Name: "org-project-soft-delete", + }, + })) + s.Require().NoError(err) + orgID := createOrgResp.Msg.GetOrganization().GetId() + + createProjectResp, err := s.testBench.Client.CreateProject(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateProjectRequest{ + Body: &frontierv1beta1.ProjectRequestBody{ + Name: "project-soft-delete", + Title: "project soft delete", + OrgId: orgID, + }, + })) + s.Require().NoError(err) + projectID := createProjectResp.Msg.GetProject().GetId() + projectResource := schema.JoinNamespaceAndResourceID(schema.ProjectNamespace, projectID) + + createUser := func(name string) string { + resp, err := s.testBench.Client.CreateUser(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateUserRequest{ + Body: &frontierv1beta1.UserRequestBody{ + Email: name + "@raystack.org", + Name: name, + }, + })) + s.Require().NoError(err) + return resp.Msg.GetUser().GetId() + } + ownerID := createUser("project-soft-delete-owner") + viewerID := createUser("project-soft-delete-viewer") + groupMemberID := createUser("project-soft-delete-group-member") + + addOrgMembersResp, err := s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ + OrgId: orgID, + Members: []*frontierv1beta1.OrgMemberEntry{ + {UserId: ownerID, RoleId: s.orgOwnerRole}, + {UserId: groupMemberID, RoleId: s.orgViewerRole}, + }, + })) + requireAddOrgMembersSuccess(s.T(), addOrgMembersResp, err) + + createGroupResp, err := s.testBench.Client.CreateGroup(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateGroupRequest{ + OrgId: orgID, + Body: &frontierv1beta1.GroupRequestBody{Name: "project-soft-delete-group"}, + })) + s.Require().NoError(err) + groupID := createGroupResp.Msg.GetGroup().GetId() + _, err = s.testBench.Client.SetGroupMemberRole(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.SetGroupMemberRoleRequest{ + GroupId: groupID, + PrincipalId: groupMemberID, + PrincipalType: schema.UserPrincipal, + RoleId: s.groupMemberRole, + })) + s.Require().NoError(err) + + createServiceUserResp, err := s.testBench.Client.CreateServiceUser(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateServiceUserRequest{ + OrgId: orgID, + })) + s.Require().NoError(err) + serviceUserID := createServiceUserResp.Msg.GetServiceuser().GetId() + + var createdPolicyIDs []string + for _, principal := range []string{ + schema.JoinNamespaceAndResourceID(schema.UserPrincipal, viewerID), + schema.JoinNamespaceAndResourceID(schema.GroupPrincipal, groupID), + schema.JoinNamespaceAndResourceID(schema.ServiceUserPrincipal, serviceUserID), + } { + resp, err := s.testBench.Client.CreatePolicy(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreatePolicyRequest{ + Body: &frontierv1beta1.PolicyRequestBody{ + RoleId: schema.RoleProjectViewer, + Resource: projectResource, + Principal: principal, + }, + })) + s.Require().NoError(err) + createdPolicyIDs = append(createdPolicyIDs, resp.Msg.GetPolicy().GetId()) + } + + subjects := map[string]string{ + "org owner": schema.JoinNamespaceAndResourceID(schema.UserPrincipal, ownerID), + "user": schema.JoinNamespaceAndResourceID(schema.UserPrincipal, viewerID), + "group member": schema.JoinNamespaceAndResourceID(schema.UserPrincipal, groupMemberID), + "service user": schema.JoinNamespaceAndResourceID(schema.ServiceUserPrincipal, serviceUserID), + } + canGetProject := func(subject string) bool { + resp, err := s.testBench.AdminClient.CheckFederatedResourcePermission(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CheckFederatedResourcePermissionRequest{ + Resource: projectResource, + Permission: schema.GetPermission, + Subject: subject, + })) + s.Require().NoError(err) + return resp.Msg.GetStatus() + } + for name, subject := range subjects { + s.Require().Truef(canGetProject(subject), "%s before the delete", name) + } + + var policyIDs []string + for _, p := range livePoliciesMatching(s.T(), ctxOrgAdminAuth, s.testBench.Client, &frontierv1beta1.ListPoliciesRequest{ProjectId: projectID}) { + policyIDs = append(policyIDs, p.GetId()) + } + s.Require().Subset(policyIDs, createdPolicyIDs) + + _, err = s.testBench.Client.DeleteProject(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.DeleteProjectRequest{Id: projectID})) + s.Require().NoError(err) + + s.Run("nobody who had access can get the project", func() { + for name, subject := range subjects { + s.Assert().Falsef(canGetProject(subject), "%s after the delete", name) + } + }) + + s.Run("every policy on the project is marked deleted", func() { + for _, id := range policyIDs { + _, err := s.testBench.Client.GetPolicy(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.GetPolicyRequest{Id: id})) + s.Require().Error(err) + s.Assert().Equal(connect.CodeNotFound, connect.CodeOf(err)) + + var deletedRows int + s.Require().NoError(s.dbc.GetContext(context.Background(), &deletedRows, + "SELECT count(*) FROM policies WHERE id = $1 AND deleted_at IS NOT NULL", id)) + s.Assert().Equal(1, deletedRows) + } + + var liveRows int + s.Require().NoError(s.dbc.GetContext(context.Background(), &liveRows, + "SELECT count(*) FROM policies WHERE resource_type = $1 AND resource_id = $2 AND deleted_at IS NULL", + schema.ProjectNamespace, projectID)) + s.Assert().Equal(0, liveRows) + }) + + s.Run("the project row stays in the database marked deleted", func() { + var deletedRows int + s.Require().NoError(s.dbc.GetContext(context.Background(), &deletedRows, + "SELECT count(*) FROM projects WHERE id = $1 AND deleted_at IS NOT NULL", projectID)) + s.Assert().Equal(1, deletedRows) + }) + + s.Run("a second delete is refused", func() { + _, err := s.testBench.Client.DeleteProject(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.DeleteProjectRequest{Id: projectID})) + s.Require().Error(err) + s.Assert().Contains([]connect.Code{connect.CodeNotFound, connect.CodePermissionDenied}, connect.CodeOf(err)) + }) + + s.Run("a new project can take the name", func() { + recreateResp, err := s.testBench.Client.CreateProject(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateProjectRequest{ + Body: &frontierv1beta1.ProjectRequestBody{ + Name: "project-soft-delete", + Title: "project soft delete again", + OrgId: orgID, + }, + })) + s.Require().NoError(err) + s.Assert().NotEqual(projectID, recreateResp.Msg.GetProject().GetId()) + }) +} + func TestEndToEndAPIRegressionTestSuite(t *testing.T) { suite.Run(t, new(APIRegressionTestSuite)) } From 5f4dcb00980bbb73ffe6c2f06d991e7b7aaf8400 Mon Sep 17 00:00:00 2001 From: aman Date: Tue, 20 Oct 2026 16:22:56 +0530 Subject: [PATCH 2/2] fix(deleter): a project row that is already gone counts as deleted DeleteProject ignores project.ErrNotExist from DeleteModel, so a second delete, and a project deleted during an org delete, succeed as before. A test now pins the order: policies, then resources, then the project row. The comments that said the project row is removed for good are updated. --- core/deleter/service.go | 9 ++++--- core/deleter/service_test.go | 27 ++++++++++++++++--- core/resource/service.go | 2 +- .../store/postgres/resource_repository.go | 5 ++-- 4 files changed, 31 insertions(+), 12 deletions(-) diff --git a/core/deleter/service.go b/core/deleter/service.go index 75ce2a4df..9f6478b11 100644 --- a/core/deleter/service.go +++ b/core/deleter/service.go @@ -223,9 +223,7 @@ func (d Service) DeleteProject(ctx context.Context, id string) error { } } - // the project row is removed for good below, so every resource row of the - // project, deleted ones included, has to go with it - // TODO(fix): soft-delete the live resources instead once project delete is soft + // TODO(fix): soft-delete the live resources instead of purging every row resources, err := d.resService.List(ctx, resource.Filter{ ProjectID: id, IncludeDeleted: true, @@ -239,7 +237,10 @@ func (d Service) DeleteProject(ctx context.Context, id string) error { } } - return d.projService.DeleteModel(ctx, id) + if err := d.projService.DeleteModel(ctx, id); err != nil && !errors.Is(err, project.ErrNotExist) { + return err + } + return nil } // DeleteGroup orchestrates teardown of a single group: clears every member's diff --git a/core/deleter/service_test.go b/core/deleter/service_test.go index 36dba4e3b..e272b02d1 100644 --- a/core/deleter/service_test.go +++ b/core/deleter/service_test.go @@ -113,17 +113,36 @@ func (m deleterMocks) build() *deleter.Service { func TestDeleteProject(t *testing.T) { t.Run("deletes policies, resources, then project model", func(t *testing.T) { m := newMocks(t) + var steps []string m.polSvc.EXPECT().List(mock.Anything, policy.Filter{ProjectID: "proj-1"}). Return([]policy.Policy{{ID: "pol-1"}, {ID: "pol-2"}}, nil) - m.polSvc.EXPECT().Delete(mock.Anything, "pol-1").Return(nil) - m.polSvc.EXPECT().Delete(mock.Anything, "pol-2").Return(nil) + m.polSvc.EXPECT().Delete(mock.Anything, "pol-1"). + Run(func(_ context.Context, id string) { steps = append(steps, "policy:"+id) }).Return(nil) + m.polSvc.EXPECT().Delete(mock.Anything, "pol-2"). + Run(func(_ context.Context, id string) { steps = append(steps, "policy:"+id) }).Return(nil) m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: "proj-1", IncludeDeleted: true}). Return([]resource.Resource{{ID: "res-1", NamespaceID: "ns-1", Name: "r1"}}, nil) - m.resSvc.EXPECT().Purge(mock.Anything, "ns-1", "res-1").Return(nil) + m.resSvc.EXPECT().Purge(mock.Anything, "ns-1", "res-1"). + Run(func(_ context.Context, _ string, id string) { steps = append(steps, "resource:"+id) }).Return(nil) - m.projSvc.EXPECT().DeleteModel(mock.Anything, "proj-1").Return(nil) + m.projSvc.EXPECT().DeleteModel(mock.Anything, "proj-1"). + Run(func(_ context.Context, id string) { steps = append(steps, "project:"+id) }).Return(nil) + + err := m.build().DeleteProject(context.Background(), "proj-1") + assert.NoError(t, err) + assert.Equal(t, []string{"policy:pol-1", "policy:pol-2", "resource:res-1", "project:proj-1"}, steps) + }) + + t.Run("a project that is already gone counts as deleted", func(t *testing.T) { + m := newMocks(t) + + m.polSvc.EXPECT().List(mock.Anything, policy.Filter{ProjectID: "proj-1"}). + Return([]policy.Policy{}, nil) + m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: "proj-1", IncludeDeleted: true}). + Return([]resource.Resource{}, nil) + m.projSvc.EXPECT().DeleteModel(mock.Anything, "proj-1").Return(project.ErrNotExist) err := m.build().DeleteProject(context.Background(), "proj-1") assert.NoError(t, err) diff --git a/core/resource/service.go b/core/resource/service.go index 99cf4fabc..d169e4cc9 100644 --- a/core/resource/service.go +++ b/core/resource/service.go @@ -504,7 +504,7 @@ func (s Service) Delete(ctx context.Context, namespaceID, id string) error { // Purge removes the resource and its SpiceDB tuples for good. Only the project // delete cascade uses it; the API uses Delete, which keeps the row. -// TODO(fix): remove once project delete is soft +// TODO(fix): remove once the project delete cascade soft-deletes resources func (s Service) Purge(ctx context.Context, namespaceID, id string) error { if err := s.relationService.Delete(ctx, relation.Relation{ Object: relation.Object{ diff --git a/internal/store/postgres/resource_repository.go b/internal/store/postgres/resource_repository.go index 76e214285..e0925ba6b 100644 --- a/internal/store/postgres/resource_repository.go +++ b/internal/store/postgres/resource_repository.go @@ -286,9 +286,8 @@ func (r ResourceRepository) Delete(ctx context.Context, id string) error { return nil } -// Purge removes the row for good. The project delete cascade uses it, since a -// resource row cannot outlive its project row. -// TODO(fix): remove once project delete is soft +// Purge removes the row for good. Only the project delete cascade uses it. +// TODO(fix): remove once the project delete cascade soft-deletes resources func (r ResourceRepository) Purge(ctx context.Context, id string) error { query, params, err := dialect.Delete(TABLE_RESOURCES).Where( goqu.Ex{