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"
 )
 
 type ProjectService interface {
@@ -213,9 +214,32 @@ 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 project.ErrNotExist
+	}
+	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])
+}
+
+// deleteProject skips the blocker check, so the org delete can use it: what
+// blocks a project delete must not block an org delete.
+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
@@ -228,7 +252,7 @@ func (d Service) DeleteProject(ctx context.Context, id string) error {
 
 	// 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 {
@@ -240,7 +264,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
@@ -313,7 +337,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

@@ -112,76 +112,120 @@ func (m deleterMocks) build() *deleter.Service {
 }
 
 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("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("returns not found when the project is gone", 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.ErrorIs(t, err, project.ErrNotExist)
+	})
+
+	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) {
@@ -358,6 +402,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
 }

internal/api/v1beta1connect/deleter_test.go

@@ -8,6 +8,7 @@ import (
 	"connectrpc.com/connect"
 	"github.com/raystack/frontier/core/deleter"
 	"github.com/raystack/frontier/core/organization"
+	"github.com/raystack/frontier/core/project"
 	"github.com/raystack/frontier/internal/api/v1beta1connect/mocks"
 	"github.com/raystack/frontier/pkg/errors"
 	frontierv1beta1 "github.com/raystack/frontier/proto/v1beta1"
@@ -35,6 +36,28 @@ func TestHandler_DeleteProject(t *testing.T) {
 			want:    connect.NewResponse(&frontierv1beta1.DeleteProjectResponse{}),
 			wantErr: nil,
 		},
+		{
+			name: "should return not found when the project is gone",
+			setup: func(as *mocks.CascadeDeleter) {
+				as.EXPECT().DeleteProject(mock.Anything, "some-id").Return(project.ErrNotExist)
+			},
+			request: connect.NewRequest(&frontierv1beta1.DeleteProjectRequest{
+				Id: "some-id",
+			}),
+			want:    nil,
+			wantErr: connect.NewError(connect.CodeNotFound, ErrProjectNotFound),
+		},
+		{
+			name: "should return not found for an id that is not a uuid",
+			setup: func(as *mocks.CascadeDeleter) {
+				as.EXPECT().DeleteProject(mock.Anything, "some-name").Return(project.ErrInvalidUUID)
+			},
+			request: connect.NewRequest(&frontierv1beta1.DeleteProjectRequest{
+				Id: "some-name",
+			}),
+			want:    nil,
+			wantErr: connect.NewError(connect.CodeNotFound, ErrProjectNotFound),
+		},
 		{
 			name: "should return error if deleter service encounters an error",
 			setup: func(as *mocks.CascadeDeleter) {
@@ -59,6 +82,38 @@ func TestHandler_DeleteProject(t *testing.T) {
 			assert.Equal(t, tt.wantErr, err)
 		})
 	}
+
+	t.Run("should return failed precondition with one violation per namespace when resources block", func(t *testing.T) {
+		blocked := &deleter.BlockedError{
+			ProjectID: "some-id",
+			Blockers: []deleter.Blocker{
+				{Type: deleter.BlockerResourcesExist, Subject: "compute/order", SubjectType: deleter.SubjectResourceNamespace, Count: 2},
+			},
+		}
+		mockDeleteSrv := new(mocks.CascadeDeleter)
+		mockDeleteSrv.EXPECT().DeleteProject(mock.Anything, "some-id").Return(blocked)
+		mockDel := &ConnectHandler{deleterService: mockDeleteSrv}
+
+		resp, err := mockDel.DeleteProject(context.Background(), connect.NewRequest(&frontierv1beta1.DeleteProjectRequest{
+			Id: "some-id",
+		}))
+		assert.Nil(t, resp)
+
+		var connectErr *connect.Error
+		assert.ErrorAs(t, err, &connectErr)
+		assert.Equal(t, connect.CodeFailedPrecondition, connectErr.Code())
+		assert.Equal(t, "project cannot be deleted yet", connectErr.Message())
+
+		assert.Len(t, connectErr.Details(), 1)
+		detail, detailErr := connectErr.Details()[0].Value()
+		assert.NoError(t, detailErr)
+		failure, ok := detail.(*errdetails.PreconditionFailure)
+		assert.True(t, ok)
+		assert.Len(t, failure.GetViolations(), 1)
+		assert.Equal(t, deleter.BlockerResourcesExist, failure.GetViolations()[0].GetType())
+		assert.Equal(t, "compute/order", failure.GetViolations()[0].GetSubject())
+		assert.Empty(t, failure.GetViolations()[0].GetDescription())
+	})
 }
 
 func TestHandler_DeleteOrganization(t *testing.T) {