feat(deleter): project delete refuses while resources block it

core/deleter/deleter.go

@@ -41,14 +41,24 @@ type Blocker struct {
 // caller gets one checklist instead of discovering blockers one retry at
 // a time.
 type BlockedError struct {
-	OrgID    string
-	Blockers []Blocker
+	OrgID     string
+	ProjectID string
+	Blockers  []Blocker
 }
 
 func (e *BlockedError) Error() string {
-	msgs := make([]string, 0, len(e.Blockers))
+	target := "organization"
+	if e.ProjectID != "" {
+		target = "project"
+	}
+	var msgs []string
 	for _, b := range e.Blockers {
-		msgs = append(msgs, b.Message)
+		if b.Message != "" {
+			msgs = append(msgs, b.Message)
+		}
+	}
+	if len(msgs) == 0 {
+		return target + " cannot be deleted yet"
 	}
-	return "organization cannot be deleted yet: " + strings.Join(msgs, "; ")
+	return target + " cannot be deleted yet: " + strings.Join(msgs, "; ")
 }

core/deleter/service.go

@@ -44,6 +44,7 @@ import (
 	"github.com/raystack/frontier/core/serviceuser"
 	"github.com/raystack/frontier/core/user"
 	"github.com/raystack/frontier/pkg/mailer"
+	"github.com/raystack/frontier/pkg/utils"
 	"github.com/raystack/salt/rql"
 )
 
@@ -219,22 +220,43 @@ func NewCascadeDeleter(orgService OrganizationService, projService ProjectServic
 }
 
 func (d Service) DeleteProject(ctx context.Context, id string) error {
+	if !utils.IsValidUUID(id) {
+		return project.ErrInvalidUUID
+	}
+	projects, err := d.projService.List(ctx, project.Filter{ProjectIDs: []string{id}, IncludeDisabled: true})
+	if err != nil {
+		return err
+	}
+	if len(projects) == 0 {
+		return nil
+	}
+	blockers, err := d.ProjectResourceBlockers(ctx, id)
+	if err != nil {
+		return err
+	}
+	if len(blockers) > 0 {
+		return &BlockedError{ProjectID: id, Blockers: blockers}
+	}
+	return d.deleteProject(ctx, projects[0])
+}
+
+func (d Service) deleteProject(ctx context.Context, prj project.Project) error {
 	// delete all project-level policies (and their rolebinding relations)
 	policies, err := d.policyService.List(ctx, policy.Filter{
-		ProjectID: id,
+		ProjectID: prj.ID,
 	})
 	if err != nil {
 		return err
 	}
 	for _, p := range policies {
-		if err = d.policyService.Delete(ctx, p.ID); err != nil {
+		if err = d.policyService.Delete(ctx, p.ID); err != nil && !errors.Is(err, policy.ErrNotExist) {
 			return fmt.Errorf("failed to delete project while deleting a policy[%s]: %w", p.ID, err)
 		}
 	}
 
 	// TODO(fix): soft-delete the live resources instead of purging every row
 	resources, err := d.resService.List(ctx, resource.Filter{
-		ProjectID:      id,
+		ProjectID:      prj.ID,
 		IncludeDeleted: true,
 	})
 	if err != nil {
@@ -246,7 +268,7 @@ func (d Service) DeleteProject(ctx context.Context, id string) error {
 		}
 	}
 
-	if err := d.projService.DeleteModel(ctx, id); err != nil && !errors.Is(err, project.ErrNotExist) {
+	if err := d.projService.DeleteModel(ctx, prj.ID); err != nil && !errors.Is(err, project.ErrNotExist) {
 		return err
 	}
 	return nil
@@ -319,7 +341,7 @@ func (d Service) DeleteOrganization(ctx context.Context, id string) error {
 		return err
 	}
 	for _, p := range projects {
-		if err = d.DeleteProject(ctx, p.ID); err != nil {
+		if err = d.deleteProject(ctx, p); err != nil {
 			return fmt.Errorf("failed to delete org while deleting a project[%s]: %w", p.Name, err)
 		}
 	}

core/deleter/service_test.go

@@ -124,76 +124,135 @@ func (m deleterMocks) buildWithNotice(notice billing.OrgDeleteNoticeConfig) *del
 }
 
 func TestDeleteProject(t *testing.T) {
+	projectID := uuid.NewString()
+	expectProject := func(m deleterMocks) {
+		m.projSvc.EXPECT().List(mock.Anything, project.Filter{ProjectIDs: []string{projectID}, IncludeDisabled: true}).
+			Return([]project.Project{{ID: projectID, Name: "p1"}}, nil)
+	}
+
 	t.Run("deletes policies, resources, then project model", func(t *testing.T) {
 		m := newMocks(t)
 		var steps []string
+		expectProject(m)
 
-		m.polSvc.EXPECT().List(mock.Anything, policy.Filter{ProjectID: "proj-1"}).
+		m.polSvc.EXPECT().List(mock.Anything, policy.Filter{ProjectID: projectID}).
 			Return([]policy.Policy{{ID: "pol-1"}, {ID: "pol-2"}}, 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}).
+		m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: projectID, IncludeDeleted: true}).
 			Return([]resource.Resource{{ID: "res-1", NamespaceID: "ns-1", Name: "r1"}}, 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").
+		m.projSvc.EXPECT().DeleteModel(mock.Anything, projectID).
 			Run(func(_ context.Context, id string) { steps = append(steps, "project:"+id) }).Return(nil)
 
-		err := m.build().DeleteProject(context.Background(), "proj-1")
+		err := m.build().DeleteProject(context.Background(), projectID)
 		assert.NoError(t, err)
-		assert.Equal(t, []string{"policy:pol-1", "policy:pol-2", "resource:res-1", "project:proj-1"}, steps)
+		assert.Equal(t, []string{"policy:pol-1", "policy:pol-2", "resource:res-1", "project:" + projectID}, steps)
 	})
 
 	t.Run("a project that is already gone counts as deleted", func(t *testing.T) {
 		m := newMocks(t)
+		expectProject(m)
 
-		m.polSvc.EXPECT().List(mock.Anything, policy.Filter{ProjectID: "proj-1"}).
+		m.polSvc.EXPECT().List(mock.Anything, policy.Filter{ProjectID: projectID}).
 			Return([]policy.Policy{}, nil)
-		m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: "proj-1", IncludeDeleted: true}).
+		m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: projectID, IncludeDeleted: true}).
 			Return([]resource.Resource{}, nil)
-		m.projSvc.EXPECT().DeleteModel(mock.Anything, "proj-1").Return(project.ErrNotExist)
+		m.projSvc.EXPECT().DeleteModel(mock.Anything, projectID).Return(project.ErrNotExist)
 
-		err := m.build().DeleteProject(context.Background(), "proj-1")
+		err := m.build().DeleteProject(context.Background(), projectID)
 		assert.NoError(t, err)
 	})
 
 	t.Run("returns error when policy list fails", func(t *testing.T) {
 		m := newMocks(t)
+		expectProject(m)
 
-		m.polSvc.EXPECT().List(mock.Anything, policy.Filter{ProjectID: "proj-1"}).
+		m.polSvc.EXPECT().List(mock.Anything, policy.Filter{ProjectID: projectID}).
 			Return(nil, errors.New("db error"))
 
-		err := m.build().DeleteProject(context.Background(), "proj-1")
+		err := m.build().DeleteProject(context.Background(), projectID)
 		assert.ErrorContains(t, err, "db error")
 	})
 
 	t.Run("returns error when policy delete fails", func(t *testing.T) {
 		m := newMocks(t)
+		expectProject(m)
 
-		m.polSvc.EXPECT().List(mock.Anything, policy.Filter{ProjectID: "proj-1"}).
+		m.polSvc.EXPECT().List(mock.Anything, policy.Filter{ProjectID: projectID}).
 			Return([]policy.Policy{{ID: "pol-fail"}}, nil)
 		m.polSvc.EXPECT().Delete(mock.Anything, "pol-fail").Return(errors.New("delete error"))
 
-		err := m.build().DeleteProject(context.Background(), "proj-1")
+		err := m.build().DeleteProject(context.Background(), projectID)
 		assert.ErrorContains(t, err, "pol-fail")
 	})
 
+	t.Run("a policy that is already gone does not stop the delete", func(t *testing.T) {
+		m := newMocks(t)
+		expectProject(m)
+
+		m.polSvc.EXPECT().List(mock.Anything, policy.Filter{ProjectID: projectID}).
+			Return([]policy.Policy{{ID: "pol-gone"}}, nil)
+		m.polSvc.EXPECT().Delete(mock.Anything, "pol-gone").Return(policy.ErrNotExist)
+		m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: projectID, IncludeDeleted: true}).
+			Return([]resource.Resource{}, nil)
+		m.projSvc.EXPECT().DeleteModel(mock.Anything, projectID).Return(nil)
+
+		err := m.build().DeleteProject(context.Background(), projectID)
+		assert.NoError(t, err)
+	})
+
 	t.Run("no policies — still deletes resources and project", func(t *testing.T) {
 		m := newMocks(t)
+		expectProject(m)
 
-		m.polSvc.EXPECT().List(mock.Anything, policy.Filter{ProjectID: "proj-1"}).
+		m.polSvc.EXPECT().List(mock.Anything, policy.Filter{ProjectID: projectID}).
 			Return([]policy.Policy{}, nil)
-		m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: "proj-1", IncludeDeleted: true}).
+		m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: projectID, IncludeDeleted: true}).
 			Return([]resource.Resource{}, nil)
-		m.projSvc.EXPECT().DeleteModel(mock.Anything, "proj-1").Return(nil)
+		m.projSvc.EXPECT().DeleteModel(mock.Anything, projectID).Return(nil)
 
-		err := m.build().DeleteProject(context.Background(), "proj-1")
+		err := m.build().DeleteProject(context.Background(), projectID)
 		assert.NoError(t, err)
 	})
+
+	t.Run("refuses while resources block and removes nothing", func(t *testing.T) {
+		m := newMocks(t)
+		m.deletion = blockerConfig()
+		expectProject(m)
+		m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: projectID}).
+			Return([]resource.Resource{{ID: "m1", NamespaceID: "compute/machine"}}, nil)
+
+		err := m.build().DeleteProject(context.Background(), projectID)
+
+		var blocked *deleter.BlockedError
+		if assert.ErrorAs(t, err, &blocked) {
+			assert.Equal(t, projectID, blocked.ProjectID)
+			assert.Equal(t, []deleter.Blocker{resourcesExist("compute/machine", 1)}, blocked.Blockers)
+		}
+		assert.EqualError(t, err, "project cannot be deleted yet")
+	})
+
+	t.Run("succeeds and removes nothing when no live project has the id", func(t *testing.T) {
+		m := newMocks(t)
+		m.projSvc.EXPECT().List(mock.Anything, project.Filter{ProjectIDs: []string{projectID}, IncludeDisabled: true}).
+			Return([]project.Project{}, nil)
+
+		err := m.build().DeleteProject(context.Background(), projectID)
+		assert.NoError(t, err)
+	})
+
+	t.Run("rejects an id that is not a uuid", func(t *testing.T) {
+		m := newMocks(t)
+
+		err := m.build().DeleteProject(context.Background(), "p1")
+		assert.ErrorIs(t, err, project.ErrInvalidUUID)
+	})
 }
 
 func expectEmptyOrgTail(m deleterMocks, steps *[]string) {
@@ -369,6 +428,36 @@ func TestDeleteOrganization(t *testing.T) {
 		}, steps)
 	})
 
+	t.Run("a project resource that blocks a project delete does not stop the org delete", func(t *testing.T) {
+		m := newBareMocks(t)
+		m.deletion = blockerConfig()
+		var steps []string
+
+		m.orgSvc.EXPECT().GetRaw(mock.Anything, "org-1").
+			Return(organization.Organization{ID: "org-1"}, nil)
+		m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}).
+			Return([]customer.Customer{}, nil)
+		m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", IncludeDisabled: true}).
+			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", IncludeDeleted: true}).
+			Return([]resource.Resource{{ID: "m1", NamespaceID: "compute/machine", Name: "machine-1"}}, nil)
+		m.resSvc.EXPECT().Purge(mock.Anything, "compute/machine", "m1").Return(nil)
+		m.projSvc.EXPECT().DeleteModel(mock.Anything, "proj-1").
+			Run(func(_ context.Context, id string) { steps = append(steps, "project:"+id) }).Return(nil)
+		m.resSvc.EXPECT().List(mock.Anything, resource.Filter{OrgID: "org-1"}).
+			Return([]resource.Resource{}, nil)
+		m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1", IncludeDisabled: true}).
+			Return([]group.Group{}, nil)
+
+		expectEmptyOrgTail(m, &steps)
+
+		err := m.build().DeleteOrganization(context.Background(), "org-1")
+		assert.NoError(t, err)
+		assert.Equal(t, []string{"project:proj-1", "org"}, steps)
+	})
+
 	t.Run("an org with only enabled children lists nothing extra to delete", func(t *testing.T) {
 		m := newBareMocks(t)
 		var steps []string

internal/api/v1beta1connect/deleter.go

@@ -9,12 +9,20 @@ import (
 	"connectrpc.com/connect"
 	"github.com/raystack/frontier/core/deleter"
 	"github.com/raystack/frontier/core/organization"
+	"github.com/raystack/frontier/core/project"
 	frontierv1beta1 "github.com/raystack/frontier/proto/v1beta1"
 	"google.golang.org/genproto/googleapis/rpc/errdetails"
 )
 
 func (h *ConnectHandler) DeleteProject(ctx context.Context, request *connect.Request[frontierv1beta1.DeleteProjectRequest]) (*connect.Response[frontierv1beta1.DeleteProjectResponse], error) {
 	if err := h.deleterService.DeleteProject(ctx, request.Msg.GetId()); err != nil {
+		var blocked *deleter.BlockedError
+		if errors.As(err, &blocked) {
+			return nil, deleteBlockedError(ctx, blocked)
+		}
+		if errors.Is(err, project.ErrNotExist) || errors.Is(err, project.ErrInvalidUUID) {
+			return nil, connect.NewError(connect.CodeNotFound, ErrProjectNotFound)
+		}
 		return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("DeleteProject.DeleteProject: project_id=%s: %w", request.Msg.GetId(), err))
 	}
 	return connect.NewResponse(&frontierv1beta1.DeleteProjectResponse{}), nil
@@ -73,10 +81,10 @@ func deleteBlockedError(ctx context.Context, blocked *deleter.BlockedError) *con
 		})
 	}
 	if detail, err := connect.NewErrorDetail(failure); err != nil {
-		slog.WarnContext(ctx, "failed to attach precondition failure details", "error", err, "org_id", blocked.OrgID)
+		slog.WarnContext(ctx, "failed to attach precondition failure details", "error", err, "org_id", blocked.OrgID, "project_id", blocked.ProjectID)
 	} else {
 		connectErr.AddDetail(detail)
 	}
-	slog.WarnContext(ctx, "organization delete blocked", "org_id", blocked.OrgID, "blockers", len(blocked.Blockers))
+	slog.WarnContext(ctx, "delete blocked", "org_id", blocked.OrgID, "project_id", blocked.ProjectID, "blockers", len(blocked.Blockers))
 	return connectErr
 }