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)) })