From da949bfc3fe27d0ddc5802d52578505119eb1046 Mon Sep 17 00:00:00 2001 From: "stonezdj(Daojun Zhang)" Date: Wed, 8 Nov 2023 20:51:21 +0800 Subject: [PATCH] Delete project member when delete project (#19523) Signed-off-by: stonezdj --- .../event/handler/internal/project.go | 12 ++--- .../event/handler/internal/project_test.go | 22 ++++++++ src/pkg/member/dao/dao.go | 14 +++++ src/pkg/member/dao/dao_test.go | 54 +++++++++++++++++++ src/pkg/member/manager.go | 5 ++ src/testing/pkg/member/fake_member_manager.go | 14 +++++ 6 files changed, 115 insertions(+), 6 deletions(-) diff --git a/src/controller/event/handler/internal/project.go b/src/controller/event/handler/internal/project.go index 327ac5317e7..b6b7bf666b3 100644 --- a/src/controller/event/handler/internal/project.go +++ b/src/controller/event/handler/internal/project.go @@ -21,6 +21,7 @@ import ( "github.com/goharbor/harbor/src/controller/immutable" "github.com/goharbor/harbor/src/controller/retention" "github.com/goharbor/harbor/src/lib/log" + "github.com/goharbor/harbor/src/pkg/member" ) // ProjectEventHandler process project event data @@ -39,16 +40,15 @@ func (a *ProjectEventHandler) IsStateful() bool { func (a *ProjectEventHandler) onProjectDelete(ctx context.Context, event *event.DeleteProjectEvent) error { log.Infof("delete project id: %d", event.ProjectID) - // delete tag immutable - err := immutable.Ctr.DeleteImmutableRuleByProject(ctx, event.ProjectID) - if err != nil { + if err := immutable.Ctr.DeleteImmutableRuleByProject(ctx, event.ProjectID); err != nil { log.Errorf("failed to delete immutable rule, error %v", err) } - // delete tag retention - err = retention.Ctl.DeleteRetentionByProject(ctx, event.ProjectID) - if err != nil { + if err := retention.Ctl.DeleteRetentionByProject(ctx, event.ProjectID); err != nil { log.Errorf("failed to delete retention rule, error %v", err) } + if err := member.Mgr.DeleteMemberByProjectID(ctx, event.ProjectID); err != nil { + log.Errorf("failed to delete project member, error %v", err) + } return nil } diff --git a/src/controller/event/handler/internal/project_test.go b/src/controller/event/handler/internal/project_test.go index b05666efc64..d3a29195ed4 100644 --- a/src/controller/event/handler/internal/project_test.go +++ b/src/controller/event/handler/internal/project_test.go @@ -21,7 +21,9 @@ import ( beegoorm "github.com/beego/beego/v2/client/orm" "github.com/stretchr/testify/suite" + "github.com/goharbor/harbor/src/common" common_dao "github.com/goharbor/harbor/src/common/dao" + commonmodels "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/controller/event" "github.com/goharbor/harbor/src/controller/immutable" "github.com/goharbor/harbor/src/lib/config" @@ -29,11 +31,14 @@ import ( "github.com/goharbor/harbor/src/pkg" "github.com/goharbor/harbor/src/pkg/artifact" immutableModel "github.com/goharbor/harbor/src/pkg/immutable/model" + "github.com/goharbor/harbor/src/pkg/member" + memberModels "github.com/goharbor/harbor/src/pkg/member/models" "github.com/goharbor/harbor/src/pkg/project" "github.com/goharbor/harbor/src/pkg/project/models" "github.com/goharbor/harbor/src/pkg/repository/model" "github.com/goharbor/harbor/src/pkg/tag" tagmodel "github.com/goharbor/harbor/src/pkg/tag/model/tag" + "github.com/goharbor/harbor/src/pkg/user" ) // ProjectHandlerTestSuite is test suite for artifact handler. @@ -81,6 +86,18 @@ func (suite *ProjectHandlerTestSuite) TestOnProjectDelete() { projID, err := project.New().Create(suite.ctx, &models.Project{Name: "test-project", OwnerID: 1}) suite.Nil(err) + userID, err := user.Mgr.Create(suite.ctx, &commonmodels.User{Username: "test-user-event", Email: "test-user-event@example.com"}) + defer user.Mgr.Delete(suite.ctx, userID) + + // create project member + _, err = member.Mgr.AddProjectMember(suite.ctx, memberModels.Member{ProjectID: projID, EntityType: common.UserMember, EntityID: userID, Role: 1}) + suite.Nil(err) + + // verify project member + members, err := member.Mgr.SearchMemberByName(suite.ctx, projID, "test-user-event") + suite.Nil(err) + suite.Equal(1, len(members)) + defer project.New().Delete(suite.ctx, projID) immutableRule := &immutableModel.Metadata{ ProjectID: projID, @@ -116,6 +133,11 @@ func (suite *ProjectHandlerTestSuite) TestOnProjectDelete() { // check if immutable rule is deleted _, err = immutable.Ctr.GetImmutableRule(suite.ctx, immutableID) suite.NotNil(err) + + // check if project member is deleted + mbs, err := member.Mgr.SearchMemberByName(suite.ctx, projID, "test-user-event") + suite.Nil(err) + suite.Equal(0, len(mbs)) } // TestArtifactHandler tests ArtifactHandler. diff --git a/src/pkg/member/dao/dao.go b/src/pkg/member/dao/dao.go index f9ff36af7ad..7133589425b 100644 --- a/src/pkg/member/dao/dao.go +++ b/src/pkg/member/dao/dao.go @@ -44,6 +44,8 @@ type DAO interface { DeleteProjectMemberByID(ctx context.Context, projectID int64, pmid int) error // DeleteProjectMemberByUserID -- Delete project member by user id DeleteProjectMemberByUserID(ctx context.Context, uid int) error + // DeleteProjectMemberByProjectID -- Delete project member by project id + DeleteProjectMemberByProjectID(ctx context.Context, projectID int64) error // SearchMemberByName search members of the project by entity_name SearchMemberByName(ctx context.Context, projectID int64, entityName string) ([]*models.Member, error) // ListRoles lists the roles of user for the specific project @@ -261,3 +263,15 @@ func (d *dao) ListRoles(ctx context.Context, user *models.User, projectID int64) } return roles, nil } + +func (d *dao) DeleteProjectMemberByProjectID(ctx context.Context, projectID int64) error { + o, err := orm.FromContext(ctx) + if err != nil { + return err + } + sql := "delete from project_member where project_id = ?" + if _, err := o.Raw(sql, projectID).Exec(); err != nil { + return err + } + return nil +} diff --git a/src/pkg/member/dao/dao_test.go b/src/pkg/member/dao/dao_test.go index e95c175855b..21bc12ecf98 100644 --- a/src/pkg/member/dao/dao_test.go +++ b/src/pkg/member/dao/dao_test.go @@ -16,6 +16,7 @@ package dao import ( "database/sql" + "fmt" "testing" "github.com/stretchr/testify/suite" @@ -293,6 +294,59 @@ func (s *DaoTestSuite) TestDeleteProjectMemberByUserId() { s.Nil(err) } +func (s *DaoTestSuite) TestDeleteProjectMemberByProjectID() { + s.WithUser(func(userID int64, username string) { + proj2, err := s.projectMgr.Get(s.Context(), "member_test_02") + s.Nil(err) + s.NotNil(proj2) + var addMember = models.Member{ + ProjectID: proj2.ProjectID, + EntityID: int(userID), + EntityType: common.UserMember, + Role: common.RoleDeveloper, + } + pmid, err := s.dao.AddProjectMember(s.Context(), addMember) + s.Nil(err) + s.True(pmid > 0) + + err = s.dao.DeleteProjectMemberByProjectID(s.Context(), proj2.ProjectID) + s.Nil(err) + + queryMember := models.Member{ProjectID: proj2.ProjectID, EntityID: int(userID), EntityType: common.UserMember} + + // not exist + members, err := s.dao.GetProjectMember(s.Context(), queryMember, nil) + s.True(len(members) == 0) + s.Nil(err) + }, "test_project_member_delete") +} + +func (s *DaoTestSuite) WithUser(f func(int64, string), usernames ...string) { + var username string + if len(usernames) > 0 { + username = usernames[0] + } else { + username = s.RandString(5) + } + + o, err := orm.FromContext(orm.Context()) + if err != nil { + s.Fail("got error %v", err) + } + + var userID int64 + + email := fmt.Sprintf("%s@example.com", username) + sql := "INSERT INTO harbor_user (username, realname, email, password) VALUES (?, ?, ?, 'Harbor12345') RETURNING user_id" + s.Nil(o.Raw(sql, username, username, email).QueryRow(&userID)) + + defer func() { + o.Raw("delete from harbor_user WHERE user_id = ?", userID).Exec() + }() + + f(userID, username) +} + func TestDaoTestSuite(t *testing.T) { suite.Run(t, &DaoTestSuite{}) } diff --git a/src/pkg/member/manager.go b/src/pkg/member/manager.go index 67737d18353..d1a748fe626 100644 --- a/src/pkg/member/manager.go +++ b/src/pkg/member/manager.go @@ -44,6 +44,8 @@ type Manager interface { SearchMemberByName(ctx context.Context, projectID int64, entityName string) ([]*models.Member, error) // DeleteMemberByUserID delete project member by user id DeleteMemberByUserID(ctx context.Context, uid int) error + // DeleteMemberByProjectID delete project member by project id + DeleteMemberByProjectID(ctx context.Context, projectID int64) error // GetTotalOfProjectMembers get the total amount of project members GetTotalOfProjectMembers(ctx context.Context, projectID int64, query *q.Query, roles ...int) (int, error) // ListRoles list project roles @@ -101,6 +103,9 @@ func (m *manager) Delete(ctx context.Context, projectID int64, memberID int) err func (m *manager) DeleteMemberByUserID(ctx context.Context, uid int) error { return m.dao.DeleteProjectMemberByUserID(ctx, uid) } +func (m *manager) DeleteMemberByProjectID(ctx context.Context, projectID int64) error { + return m.dao.DeleteProjectMemberByProjectID(ctx, projectID) +} // NewManager ... func NewManager() Manager { diff --git a/src/testing/pkg/member/fake_member_manager.go b/src/testing/pkg/member/fake_member_manager.go index 9f5f587eeb5..af75d174648 100644 --- a/src/testing/pkg/member/fake_member_manager.go +++ b/src/testing/pkg/member/fake_member_manager.go @@ -55,6 +55,20 @@ func (_m *Manager) Delete(ctx context.Context, projectID int64, memberID int) er return r0 } +// DeleteMemberByProjectID provides a mock function with given fields: ctx, projectID +func (_m *Manager) DeleteMemberByProjectID(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 +} + // DeleteMemberByUserID provides a mock function with given fields: ctx, uid func (_m *Manager) DeleteMemberByUserID(ctx context.Context, uid int) error { ret := _m.Called(ctx, uid)