From fefd216ab4f81ae38ca3789f6e81f7fb966acb86 Mon Sep 17 00:00:00 2001 From: aman Date: Tue, 20 Oct 2026 17:24:39 +0530 Subject: [PATCH] feat(api): add CheckProjectDelete CheckProjectDelete returns can_delete and the blockers that would refuse DeleteProject, with the number of blocking resources per namespace. It changes nothing. The caller needs delete permission on the project. Both check RPCs now copy the blocker count into the response. --- core/deleter/service.go | 32 +++++++--- core/deleter/service_test.go | 53 +++++++++++++++++ internal/api/v1beta1connect/deleter.go | 26 +++++++- internal/api/v1beta1connect/deleter_test.go | 48 +++++++++++++++ internal/api/v1beta1connect/interfaces.go | 1 + .../v1beta1connect/mocks/cascade_deleter.go | 59 +++++++++++++++++++ .../connect_interceptors/authorization.go | 4 ++ test/e2e/regression/api_test.go | 16 +++++ 8 files changed, 229 insertions(+), 10 deletions(-) diff --git a/core/deleter/service.go b/core/deleter/service.go index f1fad86b9..3939996c3 100644 --- a/core/deleter/service.go +++ b/core/deleter/service.go @@ -219,17 +219,21 @@ func NewCascadeDeleter(orgService OrganizationService, projService ProjectServic } } +func (d Service) CheckProjectDelete(ctx context.Context, id string) ([]Blocker, error) { + if _, err := d.getProject(ctx, id); err != nil { + return nil, err + } + return d.ProjectResourceBlockers(ctx, id) +} + func (d Service) DeleteProject(ctx context.Context, id string) error { - if !utils.IsValidUUID(id) { - return project.ErrInvalidUUID + prj, err := d.getProject(ctx, id) + if errors.Is(err, project.ErrNotExist) { + return nil } - 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 @@ -237,7 +241,7 @@ func (d Service) DeleteProject(ctx context.Context, id string) error { if len(blockers) > 0 { return &BlockedError{ProjectID: id, Blockers: blockers} } - return d.deleteProject(ctx, projects[0]) + return d.deleteProject(ctx, prj) } func (d Service) deleteProject(ctx context.Context, prj project.Project) error { @@ -570,6 +574,20 @@ func (d Service) listOrgProjects(ctx context.Context, orgID string) ([]project.P return d.projService.List(ctx, project.Filter{OrgID: orgID, IncludeDisabled: true}) } +func (d Service) getProject(ctx context.Context, id string) (project.Project, error) { + if !utils.IsValidUUID(id) { + return project.Project{}, project.ErrInvalidUUID + } + projects, err := d.projService.List(ctx, project.Filter{ProjectIDs: []string{id}, IncludeDisabled: true}) + if err != nil { + return project.Project{}, err + } + if len(projects) == 0 { + return project.Project{}, project.ErrNotExist + } + return projects[0], nil +} + func (d Service) listOrgGroups(ctx context.Context, orgID string) ([]group.Group, error) { return d.groupService.List(ctx, group.Filter{OrganizationID: orgID, IncludeDisabled: true}) } diff --git a/core/deleter/service_test.go b/core/deleter/service_test.go index 1ed04f442..219d5b56c 100644 --- a/core/deleter/service_test.go +++ b/core/deleter/service_test.go @@ -255,6 +255,59 @@ func TestDeleteProject(t *testing.T) { }) } +func TestCheckProjectDelete(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("returns the blockers 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"}, + {ID: "m2", NamespaceID: "compute/machine"}, + }, nil) + + blockers, err := m.build().CheckProjectDelete(context.Background(), projectID) + + assert.NoError(t, err) + assert.Equal(t, []deleter.Blocker{resourcesExist("compute/machine", 2)}, blockers) + }) + + t.Run("returns no blockers when nothing blocks", 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: "f1", NamespaceID: "storage/file"}}, nil) + + blockers, err := m.build().CheckProjectDelete(context.Background(), projectID) + + assert.NoError(t, err) + assert.Empty(t, blockers) + }) + + 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().CheckProjectDelete(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().CheckProjectDelete(context.Background(), "p1") + assert.ErrorIs(t, err, project.ErrInvalidUUID) + }) +} + func expectEmptyOrgTail(m deleterMocks, steps *[]string) { m.suSvc.EXPECT().List(mock.Anything, serviceuser.Filter{OrgID: "org-1"}). Return([]serviceuser.ServiceUser{}, nil) diff --git a/internal/api/v1beta1connect/deleter.go b/internal/api/v1beta1connect/deleter.go index c1853335d..42ab12199 100644 --- a/internal/api/v1beta1connect/deleter.go +++ b/internal/api/v1beta1connect/deleter.go @@ -53,18 +53,38 @@ func (h *ConnectHandler) CheckOrganizationDelete(ctx context.Context, request *c } return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("CheckOrganizationDelete: organization_id=%s: %w", request.Msg.GetId(), err)) } - resp := &frontierv1beta1.CheckOrganizationDeleteResponse{ + return connect.NewResponse(&frontierv1beta1.CheckOrganizationDeleteResponse{ CanDelete: len(blockers) == 0, + Blockers: transformBlockersToPB(blockers), + }), nil +} + +func (h *ConnectHandler) CheckProjectDelete(ctx context.Context, request *connect.Request[frontierv1beta1.CheckProjectDeleteRequest]) (*connect.Response[frontierv1beta1.CheckProjectDeleteResponse], error) { + blockers, err := h.deleterService.CheckProjectDelete(ctx, request.Msg.GetId()) + if err != nil { + 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("CheckProjectDelete: project_id=%s: %w", request.Msg.GetId(), err)) } + return connect.NewResponse(&frontierv1beta1.CheckProjectDeleteResponse{ + CanDelete: len(blockers) == 0, + Blockers: transformBlockersToPB(blockers), + }), nil +} + +func transformBlockersToPB(blockers []deleter.Blocker) []*frontierv1beta1.Blocker { + var pb []*frontierv1beta1.Blocker for _, b := range blockers { - resp.Blockers = append(resp.Blockers, &frontierv1beta1.Blocker{ + pb = append(pb, &frontierv1beta1.Blocker{ Type: b.Type, Subject: b.Subject, SubjectType: b.SubjectType, + Count: int32(b.Count), Message: b.Message, }) } - return connect.NewResponse(resp), nil + return pb } // deleteBlockedError turns the delete's blockers into a failed_precondition diff --git a/internal/api/v1beta1connect/deleter_test.go b/internal/api/v1beta1connect/deleter_test.go index bbc63293b..f98e88346 100644 --- a/internal/api/v1beta1connect/deleter_test.go +++ b/internal/api/v1beta1connect/deleter_test.go @@ -254,3 +254,51 @@ func TestHandler_CheckOrganizationDelete(t *testing.T) { assert.Equal(t, connect.NewError(connect.CodeNotFound, organization.ErrNotExist), err) }) } + +func TestHandler_CheckProjectDelete(t *testing.T) { + t.Run("should report the blockers with their count and can_delete false", func(t *testing.T) { + mockDel := new(mocks.CascadeDeleter) + mockDel.EXPECT().CheckProjectDelete(mock.Anything, "some-id").Return([]deleter.Blocker{ + {Type: deleter.BlockerResourcesExist, Subject: "compute/order", SubjectType: deleter.SubjectResourceNamespace, Count: 2}, + }, nil) + mockDep := &ConnectHandler{deleterService: mockDel} + + resp, err := mockDep.CheckProjectDelete(context.Background(), connect.NewRequest(&frontierv1beta1.CheckProjectDeleteRequest{ + Id: "some-id", + })) + assert.NoError(t, err) + assert.False(t, resp.Msg.GetCanDelete()) + assert.Len(t, resp.Msg.GetBlockers(), 1) + assert.Equal(t, deleter.BlockerResourcesExist, resp.Msg.GetBlockers()[0].GetType()) + assert.Equal(t, "compute/order", resp.Msg.GetBlockers()[0].GetSubject()) + assert.Equal(t, deleter.SubjectResourceNamespace, resp.Msg.GetBlockers()[0].GetSubjectType()) + assert.Equal(t, int32(2), resp.Msg.GetBlockers()[0].GetCount()) + }) + + t.Run("should report can_delete true when nothing blocks", func(t *testing.T) { + mockDel := new(mocks.CascadeDeleter) + mockDel.EXPECT().CheckProjectDelete(mock.Anything, "some-id").Return(nil, nil) + mockDep := &ConnectHandler{deleterService: mockDel} + + resp, err := mockDep.CheckProjectDelete(context.Background(), connect.NewRequest(&frontierv1beta1.CheckProjectDeleteRequest{ + Id: "some-id", + })) + assert.NoError(t, err) + assert.True(t, resp.Msg.GetCanDelete()) + assert.Empty(t, resp.Msg.GetBlockers()) + }) + + t.Run("should return not found for a missing project or a name", func(t *testing.T) { + for _, notFound := range []error{project.ErrNotExist, project.ErrInvalidUUID} { + mockDel := new(mocks.CascadeDeleter) + mockDel.EXPECT().CheckProjectDelete(mock.Anything, "some-id").Return(nil, notFound) + mockDep := &ConnectHandler{deleterService: mockDel} + + resp, err := mockDep.CheckProjectDelete(context.Background(), connect.NewRequest(&frontierv1beta1.CheckProjectDeleteRequest{ + Id: "some-id", + })) + assert.Nil(t, resp) + assert.Equal(t, connect.NewError(connect.CodeNotFound, ErrProjectNotFound), err) + } + }) +} diff --git a/internal/api/v1beta1connect/interfaces.go b/internal/api/v1beta1connect/interfaces.go index 98eff4169..1b8ef7191 100644 --- a/internal/api/v1beta1connect/interfaces.go +++ b/internal/api/v1beta1connect/interfaces.go @@ -395,6 +395,7 @@ type NamespaceService interface { type CascadeDeleter interface { DeleteProject(ctx context.Context, id string) error + CheckProjectDelete(ctx context.Context, id string) ([]deleter.Blocker, error) DeleteOrganization(ctx context.Context, id string) error CheckOrganizationDelete(ctx context.Context, id string) ([]deleter.Blocker, error) DeleteGroup(ctx context.Context, id string) error diff --git a/internal/api/v1beta1connect/mocks/cascade_deleter.go b/internal/api/v1beta1connect/mocks/cascade_deleter.go index 5098ef497..f4e871657 100644 --- a/internal/api/v1beta1connect/mocks/cascade_deleter.go +++ b/internal/api/v1beta1connect/mocks/cascade_deleter.go @@ -81,6 +81,65 @@ func (_c *CascadeDeleter_CheckOrganizationDelete_Call) RunAndReturn(run func(con return _c } +// CheckProjectDelete provides a mock function with given fields: ctx, id +func (_m *CascadeDeleter) CheckProjectDelete(ctx context.Context, id string) ([]deleter.Blocker, error) { + ret := _m.Called(ctx, id) + + if len(ret) == 0 { + panic("no return value specified for CheckProjectDelete") + } + + var r0 []deleter.Blocker + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, string) ([]deleter.Blocker, error)); ok { + return rf(ctx, id) + } + if rf, ok := ret.Get(0).(func(context.Context, string) []deleter.Blocker); ok { + r0 = rf(ctx, id) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]deleter.Blocker) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, string) error); ok { + r1 = rf(ctx, id) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// CascadeDeleter_CheckProjectDelete_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'CheckProjectDelete' +type CascadeDeleter_CheckProjectDelete_Call struct { + *mock.Call +} + +// CheckProjectDelete is a helper method to define mock.On call +// - ctx context.Context +// - id string +func (_e *CascadeDeleter_Expecter) CheckProjectDelete(ctx interface{}, id interface{}) *CascadeDeleter_CheckProjectDelete_Call { + return &CascadeDeleter_CheckProjectDelete_Call{Call: _e.mock.On("CheckProjectDelete", ctx, id)} +} + +func (_c *CascadeDeleter_CheckProjectDelete_Call) Run(run func(ctx context.Context, id string)) *CascadeDeleter_CheckProjectDelete_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(string)) + }) + return _c +} + +func (_c *CascadeDeleter_CheckProjectDelete_Call) Return(_a0 []deleter.Blocker, _a1 error) *CascadeDeleter_CheckProjectDelete_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *CascadeDeleter_CheckProjectDelete_Call) RunAndReturn(run func(context.Context, string) ([]deleter.Blocker, error)) *CascadeDeleter_CheckProjectDelete_Call { + _c.Call.Return(run) + return _c +} + // DeleteGroup provides a mock function with given fields: ctx, id func (_m *CascadeDeleter) DeleteGroup(ctx context.Context, id string) error { ret := _m.Called(ctx, id) diff --git a/pkg/server/connect_interceptors/authorization.go b/pkg/server/connect_interceptors/authorization.go index ae1c12b69..6c9b41e5b 100644 --- a/pkg/server/connect_interceptors/authorization.go +++ b/pkg/server/connect_interceptors/authorization.go @@ -545,6 +545,10 @@ var authorizationValidationMap = map[string]func(ctx context.Context, handler *v pbreq := req.(*connect.Request[frontierv1beta1.DeleteProjectRequest]) return handler.IsAuthorized(ctx, relation.Object{Namespace: schema.ProjectNamespace, ID: pbreq.Msg.GetId()}, schema.DeletePermission, req) }, + "/raystack.frontier.v1beta1.FrontierService/CheckProjectDelete": func(ctx context.Context, handler *v1beta1connect.ConnectHandler, req connect.AnyRequest) error { + pbreq := req.(*connect.Request[frontierv1beta1.CheckProjectDeleteRequest]) + return handler.IsAuthorized(ctx, relation.Object{Namespace: schema.ProjectNamespace, ID: pbreq.Msg.GetId()}, schema.DeletePermission, req) + }, "/raystack.frontier.v1beta1.FrontierService/SetProjectMemberRole": func(ctx context.Context, handler *v1beta1connect.ConnectHandler, req connect.AnyRequest) error { pbreq := req.(*connect.Request[frontierv1beta1.SetProjectMemberRoleRequest]) return handler.IsAuthorized(ctx, relation.Object{Namespace: schema.ProjectNamespace, ID: pbreq.Msg.GetProjectId()}, schema.UpdatePermission, req) diff --git a/test/e2e/regression/api_test.go b/test/e2e/regression/api_test.go index dac39a7e3..d9dd6dd9e 100644 --- a/test/e2e/regression/api_test.go +++ b/test/e2e/regression/api_test.go @@ -4081,9 +4081,24 @@ func (s *APIRegressionTestSuite) TestProjectDeleteRefusedWhileResourcesBlock() { _, err := s.testBench.Client.DeleteProject(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.DeleteProjectRequest{Id: id})) return err } + checkProject := func(id string) *frontierv1beta1.CheckProjectDeleteResponse { + resp, err := s.testBench.Client.CheckProjectDelete(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CheckProjectDeleteRequest{Id: id})) + s.Require().NoError(err) + return resp.Msg + } emptiedID, orderID := createProjectWithOrder("project-blockers-emptied") keptID, _ := createProjectWithOrder("project-blockers-kept") + s.Run("the check reports the blocker with its count", func() { + resp := checkProject(emptiedID) + s.Assert().False(resp.GetCanDelete()) + s.Require().Len(resp.GetBlockers(), 1) + s.Assert().Equal(deleter.BlockerResourcesExist, resp.GetBlockers()[0].GetType()) + s.Assert().Equal(computeOrderNamespace, resp.GetBlockers()[0].GetSubject()) + s.Assert().Equal(deleter.SubjectResourceNamespace, resp.GetBlockers()[0].GetSubjectType()) + s.Assert().Equal(int32(1), resp.GetBlockers()[0].GetCount()) + }) + s.Run("a project with a blocking resource is refused with the blocker", func() { err := deleteProject(emptiedID) var connectErr *connect.Error @@ -4110,6 +4125,7 @@ func (s *APIRegressionTestSuite) TestProjectDeleteRefusedWhileResourcesBlock() { Id: orderID, })) s.Require().NoError(err) + s.Assert().True(checkProject(emptiedID).GetCanDelete()) s.Assert().NoError(deleteProject(emptiedID)) })