Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 49 additions & 50 deletions core/deleter/mocks/resource_service.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 7 additions & 4 deletions core/deleter/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ type PolicyService interface {

type ResourceService interface {
List(ctx context.Context, flt resource.Filter) ([]resource.Resource, error)
Delete(ctx context.Context, namespaceID, id string) error
Purge(ctx context.Context, namespaceID, id string) error
}

type GroupService interface {
Expand Down Expand Up @@ -213,15 +213,18 @@ func (d Service) DeleteProject(ctx context.Context, id string) error {
}
}

// delete all related resources
// 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
resources, err := d.resService.List(ctx, resource.Filter{
ProjectID: id,
ProjectID: id,
IncludeDeleted: true,
})
if err != nil {
return err
}
for _, r := range resources {
if err = d.resService.Delete(ctx, r.NamespaceID, r.ID); err != nil {
if err = d.resService.Purge(ctx, r.NamespaceID, r.ID); err != nil {
return fmt.Errorf("failed to delete project while deleting a resource[%s]: %w", r.Name, err)
}
}
Expand Down
8 changes: 4 additions & 4 deletions core/deleter/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,9 @@ func TestDeleteProject(t *testing.T) {
m.polSvc.EXPECT().Delete(mock.Anything, "pol-1").Return(nil)
m.polSvc.EXPECT().Delete(mock.Anything, "pol-2").Return(nil)

m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: "proj-1"}).
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().Delete(mock.Anything, "ns-1", "res-1").Return(nil)
m.resSvc.EXPECT().Purge(mock.Anything, "ns-1", "res-1").Return(nil)

m.projSvc.EXPECT().DeleteModel(mock.Anything, "proj-1").Return(nil)

Expand Down Expand Up @@ -145,7 +145,7 @@ func TestDeleteProject(t *testing.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"}).
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(nil)

Expand Down Expand Up @@ -200,7 +200,7 @@ func TestDeleteOrganization(t *testing.T) {
Return([]project.Project{{ID: "proj-1", Name: "p1"}}, nil)
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"}).
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(nil)

Expand Down
3 changes: 3 additions & 0 deletions core/resource/filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,7 @@ type Filter struct {
UserID string
ServiceUserID string
NamespaceID string
// IncludeDeleted also returns soft-deleted rows.
// TODO(fix): remove once the project delete cascade no longer purges
IncludeDeleted bool
}
47 changes: 47 additions & 0 deletions core/resource/mocks/repository.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions core/resource/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ type Repository interface {
List(ctx context.Context, flt Filter) ([]Resource, error)
Update(ctx context.Context, resource Resource) (Resource, error)
Delete(ctx context.Context, id string) error
Purge(ctx context.Context, id string) error
}

type Resource struct {
Expand Down
31 changes: 30 additions & 1 deletion core/resource/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,35 @@ func (s Service) batchCheckWithScopeFilter(ctx context.Context, relations []rela
}

func (s Service) Delete(ctx context.Context, namespaceID, id string) error {
res, err := s.repository.GetByID(ctx, id)
if err != nil {
return err
}
resourceProject, err := s.projectService.Get(ctx, res.ProjectID)
if err != nil {
return fmt.Errorf("failed to get project: %w", err)
}

if err := s.relationService.Delete(ctx, relation.Relation{
Object: relation.Object{
ID: id,
Namespace: namespaceID,
},
}); err != nil && !errors.Is(err, relation.ErrNotExist) {
return err
}
if err := s.repository.Delete(ctx, id); err != nil {
return err
}

s.createAuditRecord(ctx, pkgauditrecord.ResourceDeletedEvent, res, resourceProject)
return nil
}

// 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
func (s Service) Purge(ctx context.Context, namespaceID, id string) error {
if err := s.relationService.Delete(ctx, relation.Relation{
Object: relation.Object{
ID: id,
Expand All @@ -457,7 +486,7 @@ func (s Service) Delete(ctx context.Context, namespaceID, id string) error {
}); err != nil && !errors.Is(err, relation.ErrNotExist) {
return err
}
return s.repository.Delete(ctx, id)
return s.repository.Purge(ctx, id)
}

// RemovePrincipalAccess deletes every resource-level policy the principal holds
Expand Down
Loading
Loading