remove robot accounts when to delete a project (#12789)

The robots associate with the project should be removed after the project is deleted.

Signed-off-by: wang yan <wangyan@vmware.com>

src/pkg/robot/dao/robot.go

@@ -1,9 +1,11 @@
 package dao
 
 import (
+	"context"
 	"fmt"
 	"github.com/astaxie/beego/orm"
 	"github.com/goharbor/harbor/src/common/dao"
+	libOrm "github.com/goharbor/harbor/src/lib/orm"
 	"github.com/goharbor/harbor/src/lib/q"
 	"github.com/goharbor/harbor/src/pkg/robot/model"
 	"strings"
@@ -26,6 +28,9 @@ type RobotAccountDao interface {
 
 	// DeleteRobotAccount ...
 	DeleteRobotAccount(id int64) error
+
+	// DeleteByProjectID ...
+	DeleteByProjectID(ctx context.Context, projectID int64) error
 }
 
 // New creates a default implementation for RobotAccountDao
@@ -103,3 +108,13 @@ func (r *robotAccountDao) DeleteRobotAccount(id int64) error {
 	_, err := dao.GetOrmer().QueryTable(&model.Robot{}).Filter("ID", id).Delete()
 	return err
 }
+
+// DeleteByProjectID ...
+func (r *robotAccountDao) DeleteByProjectID(ctx context.Context, projectID int64) error {
+	qs, err := libOrm.QuerySetter(ctx, &model.Robot{}, q.New(q.KeyWords{"ProjectID": projectID}))
+	if err != nil {
+		return err
+	}
+	_, err = qs.Delete()
+	return err
+}

src/pkg/robot/dao/robot_test.go

@@ -4,6 +4,7 @@ import (
 	"github.com/goharbor/harbor/src/common/dao"
 	"github.com/goharbor/harbor/src/lib/q"
 	"github.com/goharbor/harbor/src/pkg/robot/model"
+	htesting "github.com/goharbor/harbor/src/testing"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
@@ -11,7 +12,7 @@ import (
 )
 
 type robotAccountDaoTestSuite struct {
-	suite.Suite
+	htesting.Suite
 	require *require.Assertions
 	assert  *assert.Assertions
 	dao     RobotAccountDao
@@ -120,6 +121,38 @@ func (t *robotAccountDaoTestSuite) TestDeleteRobotAccount() {
 	t.require.Nil(err)
 }
 
+func (t *robotAccountDaoTestSuite) TestDeleteRobotAccountByPID() {
+	t.WithProject(func(projectID int64, projectName string) {
+		robot := &model.Robot{
+			Name:        t.RandString(5),
+			Description: "TestDeleteRobotAccountByPID description",
+			ProjectID:   projectID,
+		}
+		_, err := t.dao.CreateRobotAccount(robot)
+		t.require.Nil(err)
+		robot = &model.Robot{
+			Name:        t.RandString(5),
+			Description: "TestDeleteRobotAccountByPID description",
+			ProjectID:   projectID,
+		}
+		_, err = t.dao.CreateRobotAccount(robot)
+		t.require.Nil(err)
+
+		// Delete
+		err = t.dao.DeleteByProjectID(t.Context(), projectID)
+		t.require.Nil(err)
+
+		// Get
+		keywords := make(map[string]interface{})
+		keywords["ProjectID"] = projectID
+		robots, err := t.dao.ListRobotAccounts(&q.Query{
+			Keywords: keywords,
+		})
+		t.require.Nil(err)
+		t.require.Equal(0, len(robots))
+	})
+}
+
 // TearDownSuite clears env for test suite
 func (t *robotAccountDaoTestSuite) TearDownSuite() {
 	err := t.dao.DeleteRobotAccount(t.id1)

src/pkg/robot/manager.go

@@ -1,6 +1,7 @@
 package robot
 
 import (
+	"context"
 	"github.com/goharbor/harbor/src/lib/q"
 	"github.com/goharbor/harbor/src/pkg/robot/dao"
 	"github.com/goharbor/harbor/src/pkg/robot/model"
@@ -22,6 +23,9 @@ type Manager interface {
 	// DeleteRobotAccount ...
 	DeleteRobotAccount(id int64) error
 
+	// DeleteByProjectID ...
+	DeleteByProjectID(ctx context.Context, projectID int64) error
+
 	// UpdateRobotAccount ...
 	UpdateRobotAccount(m *model.Robot) error
 
@@ -55,6 +59,11 @@ func (drm *defaultRobotManager) DeleteRobotAccount(id int64) error {
 	return drm.dao.DeleteRobotAccount(id)
 }
 
+// DeleteByProjectID ...
+func (drm *defaultRobotManager) DeleteByProjectID(ctx context.Context, projectID int64) error {
+	return drm.dao.DeleteByProjectID(ctx, projectID)
+}
+
 // UpdateRobotAccount ...
 func (drm *defaultRobotManager) UpdateRobotAccount(r *model.Robot) error {
 	return drm.dao.UpdateRobotAccount(r)

src/pkg/robot/manager_test.go

@@ -3,57 +3,22 @@ package robot
 import (
 	"github.com/goharbor/harbor/src/lib/q"
 	"github.com/goharbor/harbor/src/pkg/robot/model"
+	"github.com/goharbor/harbor/src/testing/mock"
+	"github.com/goharbor/harbor/src/testing/pkg/robot/dao"
 	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 	"os"
 	"testing"
 )
 
-type mockRobotDao struct {
-	mock.Mock
-}
-
-func (m *mockRobotDao) CreateRobotAccount(r *model.Robot) (int64, error) {
-	args := m.Called(r)
-	return int64(args.Int(0)), args.Error(1)
-}
-
-func (m *mockRobotDao) UpdateRobotAccount(r *model.Robot) error {
-	args := m.Called(r)
-	return args.Error(1)
-}
-
-func (m *mockRobotDao) DeleteRobotAccount(id int64) error {
-	args := m.Called(id)
-	return args.Error(1)
-}
-
-func (m *mockRobotDao) GetRobotAccount(id int64) (*model.Robot, error) {
-	args := m.Called(id)
-	var r *model.Robot
-	if args.Get(0) != nil {
-		r = args.Get(0).(*model.Robot)
-	}
-	return r, args.Error(1)
-}
-
-func (m *mockRobotDao) ListRobotAccounts(query *q.Query) ([]*model.Robot, error) {
-	args := m.Called()
-	var rs []*model.Robot
-	if args.Get(0) != nil {
-		rs = args.Get(0).([]*model.Robot)
-	}
-	return rs, args.Error(1)
-}
-
 type managerTestingSuite struct {
 	suite.Suite
 	t            *testing.T
 	assert       *assert.Assertions
 	require      *require.Assertions
-	mockRobotDao *mockRobotDao
+	mockRobotDao *dao.RobotAccountDao
+	mgr          Manager
 }
 
 func (m *managerTestingSuite) SetupSuite() {
@@ -71,8 +36,8 @@ func (m *managerTestingSuite) TearDownSuite() {
 }
 
 func (m *managerTestingSuite) SetupTest() {
-	m.mockRobotDao = &mockRobotDao{}
+	m.mockRobotDao = &dao.RobotAccountDao{}
-	Mgr = &defaultRobotManager{
+	m.mgr = &defaultRobotManager{
 		dao: m.mockRobotDao,
 	}
 }
@@ -82,43 +47,43 @@ func TestManagerTestingSuite(t *testing.T) {
 }
 
 func (m *managerTestingSuite) TestCreateRobotAccount() {
-	m.mockRobotDao.On("CreateRobotAccount", mock.Anything).Return(1, nil)
+	m.mockRobotDao.On("CreateRobotAccount", mock.Anything, mock.Anything).Return(int64(1), nil)
-	id, err := Mgr.CreateRobotAccount(&model.Robot{})
+	id, err := m.mgr.CreateRobotAccount(&model.Robot{})
 	m.mockRobotDao.AssertCalled(m.t, "CreateRobotAccount", mock.Anything)
 	m.require.Nil(err)
 	m.assert.Equal(int64(1), id)
 }
 
 func (m *managerTestingSuite) TestUpdateRobotAccount() {
-	m.mockRobotDao.On("UpdateRobotAccount", mock.Anything).Return(1, nil)
+	m.mockRobotDao.On("UpdateRobotAccount", mock.Anything, mock.Anything).Return(nil)
-	err := Mgr.UpdateRobotAccount(&model.Robot{})
+	err := m.mgr.UpdateRobotAccount(&model.Robot{})
 	m.mockRobotDao.AssertCalled(m.t, "UpdateRobotAccount", mock.Anything)
 	m.require.Nil(err)
 }
 
 func (m *managerTestingSuite) TestDeleteRobotAccount() {
-	m.mockRobotDao.On("DeleteRobotAccount", mock.Anything).Return(1, nil)
+	m.mockRobotDao.On("DeleteRobotAccount", mock.Anything, mock.Anything).Return(nil)
-	err := Mgr.DeleteRobotAccount(int64(1))
+	err := m.mgr.DeleteRobotAccount(int64(1))
 	m.mockRobotDao.AssertCalled(m.t, "DeleteRobotAccount", mock.Anything)
 	m.require.Nil(err)
 }
 
 func (m *managerTestingSuite) TestGetRobotAccount() {
-	m.mockRobotDao.On("GetRobotAccount", mock.Anything).Return(&model.Robot{
+	m.mockRobotDao.On("GetRobotAccount", mock.Anything, mock.Anything).Return(&model.Robot{
 		ID:        1,
 		ProjectID: 1,
 		Disabled:  true,
 		ExpiresAt: 150000,
 	}, nil)
-	ir, err := Mgr.GetRobotAccount(1)
+	ir, err := m.mgr.GetRobotAccount(1)
-	m.mockRobotDao.AssertCalled(m.t, "GetRobotAccount", mock.Anything)
+	m.mockRobotDao.AssertCalled(m.t, "GetRobotAccount", mock.Anything, mock.Anything)
 	m.require.Nil(err)
 	m.require.NotNil(ir)
 	m.assert.Equal(int64(1), ir.ID)
 }
 
 func (m *managerTestingSuite) ListRobotAccount() {
-	m.mockRobotDao.On("ListRobotAccount", mock.Anything).Return([]model.Robot{
+	m.mockRobotDao.On("ListRobotAccount", mock.Anything, mock.Anything).Return([]model.Robot{
 		{
 			ID:        1,
 			ProjectID: 1,
@@ -137,8 +102,8 @@ func (m *managerTestingSuite) ListRobotAccount() {
 	query := &q.Query{
 		Keywords: keywords,
 	}
-	rs, err := Mgr.ListRobotAccount(query)
+	rs, err := m.mgr.ListRobotAccount(query)
-	m.mockRobotDao.AssertCalled(m.t, "ListRobotAccount", mock.Anything)
+	m.mockRobotDao.AssertCalled(m.t, "ListRobotAccount", mock.Anything, mock.Anything)
 	m.require.Nil(err)
 	m.assert.Equal(len(rs), 2)
 	m.assert.Equal(rs[0].Disabled, false)

src/server/v2.0/handler/project.go

@@ -3,6 +3,7 @@ package handler
 import (
 	"context"
 	"fmt"
+	"github.com/goharbor/harbor/src/pkg/robot"
 	"strconv"
 	"strings"
 	"sync"
@@ -46,6 +47,7 @@ func newProjectAPI() *projectAPI {
 		repositoryCtl: repository.Ctl,
 		projectCtl:    project.Ctl,
 		quotaCtl:      quota.Ctl,
+		robotMgr:      robot.Mgr,
 	}
 }
 
@@ -57,6 +59,7 @@ type projectAPI struct {
 	repositoryCtl repository.Controller
 	projectCtl    project.Controller
 	quotaCtl      quota.Controller
+	robotMgr      robot.Manager
 }
 
 func (a *projectAPI) CreateProject(ctx context.Context, params operation.CreateProjectParams) middleware.Responder {
@@ -194,6 +197,11 @@ func (a *projectAPI) DeleteProject(ctx context.Context, params operation.DeleteP
 		return a.SendError(ctx, err)
 	}
 
+	// remove the robot associated with the project
+	if err := a.robotMgr.DeleteByProjectID(ctx, params.ProjectID); err != nil {
+		return a.SendError(ctx, err)
+	}
+
 	referenceID := quota.ReferenceID(params.ProjectID)
 	q, err := a.quotaCtl.GetByRef(ctx, quota.ProjectReference, referenceID)
 	if err != nil {

src/testing/pkg/pkg.go

@@ -26,3 +26,4 @@ package pkg
 //go:generate mockery --case snake --dir ../../pkg/scan/scanner --all --output ./scan/scanner --outpkg scanner
 //go:generate mockery --case snake --dir ../../pkg/scheduler --name Scheduler --output ./scheduler --outpkg scheduler
 //go:generate mockery --case snake --dir ../../pkg/user --name Manager --output ./user --outpkg user
+//go:generate mockery --case snake --dir ../../pkg/robot/dao --name RobotAccountDao --output ./robot/dao --outpkg dao

src/testing/pkg/robot/dao/robot_account_dao.go

@@ -0,0 +1,127 @@
+// Code generated by mockery v2.1.0. DO NOT EDIT.
+
+package dao
+
+import (
+	context "context"
+
+	mock "github.com/stretchr/testify/mock"
+
+	model "github.com/goharbor/harbor/src/pkg/robot/model"
+
+	q "github.com/goharbor/harbor/src/lib/q"
+)
+
+// RobotAccountDao is an autogenerated mock type for the RobotAccountDao type
+type RobotAccountDao struct {
+	mock.Mock
+}
+
+// CreateRobotAccount provides a mock function with given fields: robot
+func (_m *RobotAccountDao) CreateRobotAccount(robot *model.Robot) (int64, error) {
+	ret := _m.Called(robot)
+
+	var r0 int64
+	if rf, ok := ret.Get(0).(func(*model.Robot) int64); ok {
+		r0 = rf(robot)
+	} else {
+		r0 = ret.Get(0).(int64)
+	}
+
+	var r1 error
+	if rf, ok := ret.Get(1).(func(*model.Robot) error); ok {
+		r1 = rf(robot)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
+// DeleteByProjectID provides a mock function with given fields: ctx, projectID
+func (_m *RobotAccountDao) DeleteByProjectID(ctx context.Context, projectID int64) error {
+	ret := _m.Called(ctx, projectID)
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func(context.Context, int64) error); ok {
+		r0 = rf(ctx, projectID)
+	} else {
+		r0 = ret.Error(0)
+	}
+
+	return r0
+}
+
+// DeleteRobotAccount provides a mock function with given fields: id
+func (_m *RobotAccountDao) DeleteRobotAccount(id int64) error {
+	ret := _m.Called(id)
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func(int64) error); ok {
+		r0 = rf(id)
+	} else {
+		r0 = ret.Error(0)
+	}
+
+	return r0
+}
+
+// GetRobotAccount provides a mock function with given fields: id
+func (_m *RobotAccountDao) GetRobotAccount(id int64) (*model.Robot, error) {
+	ret := _m.Called(id)
+
+	var r0 *model.Robot
+	if rf, ok := ret.Get(0).(func(int64) *model.Robot); ok {
+		r0 = rf(id)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(*model.Robot)
+		}
+	}
+
+	var r1 error
+	if rf, ok := ret.Get(1).(func(int64) error); ok {
+		r1 = rf(id)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
+// ListRobotAccounts provides a mock function with given fields: query
+func (_m *RobotAccountDao) ListRobotAccounts(query *q.Query) ([]*model.Robot, error) {
+	ret := _m.Called(query)
+
+	var r0 []*model.Robot
+	if rf, ok := ret.Get(0).(func(*q.Query) []*model.Robot); ok {
+		r0 = rf(query)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).([]*model.Robot)
+		}
+	}
+
+	var r1 error
+	if rf, ok := ret.Get(1).(func(*q.Query) error); ok {
+		r1 = rf(query)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
+// UpdateRobotAccount provides a mock function with given fields: robot
+func (_m *RobotAccountDao) UpdateRobotAccount(robot *model.Robot) error {
+	ret := _m.Called(robot)
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func(*model.Robot) error); ok {
+		r0 = rf(robot)
+	} else {
+		r0 = ret.Error(0)
+	}
+
+	return r0
+}