From 900c095f57da023127a8fc8e59e5472ea9f55f0d Mon Sep 17 00:00:00 2001 From: stonezdj Date: Mon, 18 Sep 2023 14:31:19 +0800 Subject: [PATCH] Delete tag retention rule and tag immutable rule when deleting project fixes #18250 Signed-off-by: stonezdj --- src/controller/event/handler/init.go | 7 +- .../event/handler/internal/artifact.go | 28 ++-- .../event/handler/internal/artifact_test.go | 4 +- .../event/handler/internal/project.go | 64 +++++++++ .../event/handler/internal/project_test.go | 124 ++++++++++++++++++ src/controller/immutable/controller.go | 16 +++ src/controller/retention/controller.go | 17 +++ src/pkg/retention/dao/retention.go | 14 ++ src/pkg/retention/dao/retention_test.go | 14 +- src/pkg/retention/launcher_test.go | 4 + src/pkg/retention/manager.go | 16 +++ .../controller/retention/controller.go | 14 ++ 12 files changed, 299 insertions(+), 23 deletions(-) create mode 100644 src/controller/event/handler/internal/project.go create mode 100644 src/controller/event/handler/internal/project_test.go diff --git a/src/controller/event/handler/init.go b/src/controller/event/handler/init.go index 2510b08e4c12..aa28103da2b0 100644 --- a/src/controller/event/handler/init.go +++ b/src/controller/event/handler/init.go @@ -67,9 +67,10 @@ func init() { _ = notifier.Subscribe(event.TopicDeleteTag, &auditlog.Handler{}) // internal - _ = notifier.Subscribe(event.TopicPullArtifact, &internal.Handler{}) - _ = notifier.Subscribe(event.TopicPushArtifact, &internal.Handler{}) - _ = notifier.Subscribe(event.TopicDeleteArtifact, &internal.Handler{}) + _ = notifier.Subscribe(event.TopicPullArtifact, &internal.ArtifactEventHandler{}) + _ = notifier.Subscribe(event.TopicPushArtifact, &internal.ArtifactEventHandler{}) + _ = notifier.Subscribe(event.TopicDeleteArtifact, &internal.ArtifactEventHandler{}) + _ = notifier.Subscribe(event.TopicDeleteProject, &internal.ProjectEventHandler{}) _ = task.RegisterTaskStatusChangePostFunc(job.ReplicationVendorType, func(ctx context.Context, taskID int64, status string) error { notification.AddEvent(ctx, &metadata.ReplicationMetaData{ diff --git a/src/controller/event/handler/internal/artifact.go b/src/controller/event/handler/internal/artifact.go index fc89eb6e02d7..5cd3e233e644 100644 --- a/src/controller/event/handler/internal/artifact.go +++ b/src/controller/event/handler/internal/artifact.go @@ -66,8 +66,8 @@ func init() { } } -// Handler preprocess artifact event data -type Handler struct { +// ArtifactEventHandler preprocess artifact event data +type ArtifactEventHandler struct { // execMgr for managing executions execMgr task.ExecutionManager // reportMgr for managing scan reports @@ -89,12 +89,12 @@ type Handler struct { } // Name ... -func (a *Handler) Name() string { +func (a *ArtifactEventHandler) Name() string { return "InternalArtifact" } // Handle ... -func (a *Handler) Handle(ctx context.Context, value interface{}) error { +func (a *ArtifactEventHandler) Handle(ctx context.Context, value interface{}) error { switch v := value.(type) { case *event.PullArtifactEvent: return a.onPull(ctx, v.ArtifactEvent) @@ -109,11 +109,11 @@ func (a *Handler) Handle(ctx context.Context, value interface{}) error { } // IsStateful ... -func (a *Handler) IsStateful() bool { +func (a *ArtifactEventHandler) IsStateful() bool { return false } -func (a *Handler) onPull(ctx context.Context, event *event.ArtifactEvent) error { +func (a *ArtifactEventHandler) onPull(ctx context.Context, event *event.ArtifactEvent) error { if config.ScannerSkipUpdatePullTime(ctx) && isScannerUser(ctx, event) { return nil } @@ -159,7 +159,7 @@ func (a *Handler) onPull(ctx context.Context, event *event.ArtifactEvent) error return nil } -func (a *Handler) updatePullTimeInCache(ctx context.Context, event *event.ArtifactEvent) { +func (a *ArtifactEventHandler) updatePullTimeInCache(ctx context.Context, event *event.ArtifactEvent) { var tagName string if len(event.Tags) != 0 { tagName = event.Tags[0] @@ -173,14 +173,14 @@ func (a *Handler) updatePullTimeInCache(ctx context.Context, event *event.Artifa a.pullTimeStore[key] = time.Now() } -func (a *Handler) addPullCountInCache(ctx context.Context, event *event.ArtifactEvent) { +func (a *ArtifactEventHandler) addPullCountInCache(ctx context.Context, event *event.ArtifactEvent) { a.pullCountLock.Lock() defer a.pullCountLock.Unlock() a.pullCountStore[event.Artifact.RepositoryID] = a.pullCountStore[event.Artifact.RepositoryID] + 1 } -func (a *Handler) syncFlushPullTime(ctx context.Context, artifactID int64, tagName string, time time.Time) { +func (a *ArtifactEventHandler) syncFlushPullTime(ctx context.Context, artifactID int64, tagName string, time time.Time) { var tagID int64 if tagName != "" { @@ -203,13 +203,13 @@ func (a *Handler) syncFlushPullTime(ctx context.Context, artifactID int64, tagNa } } -func (a *Handler) syncFlushPullCount(ctx context.Context, repositoryID int64, count uint64) { +func (a *ArtifactEventHandler) syncFlushPullCount(ctx context.Context, repositoryID int64, count uint64) { if err := repository.Ctl.AddPullCount(ctx, repositoryID, count); err != nil { log.Warningf("failed to add pull count repository %d, %v", repositoryID, err) } } -func (a *Handler) asyncFlushPullTime(ctx context.Context) { +func (a *ArtifactEventHandler) asyncFlushPullTime(ctx context.Context) { for { <-time.After(asyncFlushDuration) a.pullTimeLock.Lock() @@ -235,7 +235,7 @@ func (a *Handler) asyncFlushPullTime(ctx context.Context) { } } -func (a *Handler) asyncFlushPullCount(ctx context.Context) { +func (a *ArtifactEventHandler) asyncFlushPullCount(ctx context.Context) { for { <-time.After(asyncFlushDuration) a.pullCountLock.Lock() @@ -249,7 +249,7 @@ func (a *Handler) asyncFlushPullCount(ctx context.Context) { } } -func (a *Handler) onPush(ctx context.Context, event *event.ArtifactEvent) error { +func (a *ArtifactEventHandler) onPush(ctx context.Context, event *event.ArtifactEvent) error { go func() { if event.Operator != "" { ctx = context.WithValue(ctx, operator.ContextKey{}, event.Operator) @@ -263,7 +263,7 @@ func (a *Handler) onPush(ctx context.Context, event *event.ArtifactEvent) error return nil } -func (a *Handler) onDelete(ctx context.Context, event *event.ArtifactEvent) error { +func (a *ArtifactEventHandler) onDelete(ctx context.Context, event *event.ArtifactEvent) error { execMgr := task.ExecMgr reportMgr := report.Mgr artMgr := pkg.ArtifactMgr diff --git a/src/controller/event/handler/internal/artifact_test.go b/src/controller/event/handler/internal/artifact_test.go index 23584a55f1d0..67b191bf614a 100644 --- a/src/controller/event/handler/internal/artifact_test.go +++ b/src/controller/event/handler/internal/artifact_test.go @@ -47,7 +47,7 @@ type ArtifactHandlerTestSuite struct { suite.Suite ctx context.Context - handler *Handler + handler *ArtifactEventHandler projectManager project.Manager scannerCtl scanner.Controller reportMgr *reportMock.Manager @@ -70,7 +70,7 @@ func (suite *ArtifactHandlerTestSuite) SetupSuite() { suite.execMgr = &taskMock.ExecutionManager{} suite.reportMgr = &reportMock.Manager{} suite.artMgr = &artMock.Manager{} - suite.handler = &Handler{execMgr: suite.execMgr, reportMgr: suite.reportMgr, artMgr: suite.artMgr} + suite.handler = &ArtifactEventHandler{execMgr: suite.execMgr, reportMgr: suite.reportMgr, artMgr: suite.artMgr} // mock artifact _, err := pkg.ArtifactMgr.Create(suite.ctx, &artifact.Artifact{ID: 1, RepositoryID: 1}) diff --git a/src/controller/event/handler/internal/project.go b/src/controller/event/handler/internal/project.go new file mode 100644 index 000000000000..06dd6810c7d8 --- /dev/null +++ b/src/controller/event/handler/internal/project.go @@ -0,0 +1,64 @@ +// Copyright Project Harbor Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package internal + +import ( + "context" + + "github.com/goharbor/harbor/src/controller/event" + "github.com/goharbor/harbor/src/controller/immutable" + "github.com/goharbor/harbor/src/controller/retention" + "github.com/goharbor/harbor/src/lib/log" +) + +// ProjectEventHandler process project event data +type ProjectEventHandler struct { +} + +// Name return the name of this handler +func (a *ProjectEventHandler) Name() string { + return "InternalProject" +} + +// IsStateful return false +func (a *ProjectEventHandler) IsStateful() bool { + return false +} + +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 { + log.Errorf("failed to delete immutable rule, error %v", err) + } + // delete tag retention + err = retention.Ctl.DeleteRetentionByProject(ctx, event.ProjectID) + if err != nil { + log.Errorf("failed to delete retention rule, error %v", err) + } + return nil +} + +// Handle handle project event +func (a *ProjectEventHandler) Handle(ctx context.Context, value interface{}) error { + switch v := value.(type) { + case *event.DeleteProjectEvent: + return a.onProjectDelete(ctx, v) + default: + log.Errorf("Can not handler this event type! %#v", v) + } + return nil +} diff --git a/src/controller/event/handler/internal/project_test.go b/src/controller/event/handler/internal/project_test.go new file mode 100644 index 000000000000..a0f99930258a --- /dev/null +++ b/src/controller/event/handler/internal/project_test.go @@ -0,0 +1,124 @@ +// Copyright Project Harbor Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package internal + +import ( + "context" + "testing" + + beegoorm "github.com/beego/beego/v2/client/orm" + "github.com/stretchr/testify/suite" + + common_dao "github.com/goharbor/harbor/src/common/dao" + "github.com/goharbor/harbor/src/controller/event" + "github.com/goharbor/harbor/src/controller/immutable" + "github.com/goharbor/harbor/src/lib/config" + "github.com/goharbor/harbor/src/lib/orm" + "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/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" +) + +// ProjectHandlerTestSuite is test suite for artifact handler. +type ProjectHandlerTestSuite struct { + suite.Suite + + ctx context.Context + handler *ProjectEventHandler +} + +// SetupSuite prepares for running ArtifactHandlerTestSuite. +func (suite *ProjectHandlerTestSuite) SetupSuite() { + common_dao.PrepareTestForPostgresSQL() + config.Init() + suite.ctx = orm.NewContext(context.TODO(), beegoorm.NewOrm()) + suite.handler = &ProjectEventHandler{} + + // mock artifact + _, err := pkg.ArtifactMgr.Create(suite.ctx, &artifact.Artifact{ID: 1, RepositoryID: 1}) + suite.Nil(err) + // mock repository + _, err = pkg.RepositoryMgr.Create(suite.ctx, &model.RepoRecord{RepositoryID: 1}) + suite.Nil(err) + // mock tag + _, err = tag.Mgr.Create(suite.ctx, &tagmodel.Tag{ID: 1, RepositoryID: 1, ArtifactID: 1, Name: "latest"}) + suite.Nil(err) +} + +// TearDownSuite cleans environment. +func (suite *ProjectHandlerTestSuite) TearDownSuite() { + // delete tag + err := tag.Mgr.Delete(suite.ctx, 1) + suite.Nil(err) + // delete artifact + err = pkg.ArtifactMgr.Delete(suite.ctx, 1) + suite.Nil(err) + // delete repository + err = pkg.RepositoryMgr.Delete(suite.ctx, 1) + suite.Nil(err) + +} + +func (suite *ProjectHandlerTestSuite) TestOnProjectDelete() { + // create project + projID, err := project.New().Create(suite.ctx, &models.Project{Name: "test-project", OwnerID: 1}) + suite.Nil(err) + + defer project.New().Delete(suite.ctx, projID) + immutableRule := &immutableModel.Metadata{ + ProjectID: projID, + Priority: 1, + Action: "immutable", + Template: "immutable_template", + TagSelectors: []*immutableModel.Selector{ + { + Kind: "doublestar", + Decoration: "matches", + Pattern: "release-**", + }, + }, + ScopeSelectors: map[string][]*immutableModel.Selector{ + "repository": { + { + Kind: "doublestar", + Decoration: "repoMatches", + Pattern: "redis", + }, + }, + }, + } + // create immutable rule + immutableID, err := immutable.Ctr.CreateImmutableRule(suite.ctx, immutableRule) + suite.Nil(err) + + // emit delete project event + event := &event.DeleteProjectEvent{ProjectID: projID} + err = suite.handler.onProjectDelete(suite.ctx, event) + suite.Nil(err) + + // check if immutable rule is deleted + _, err = immutable.Ctr.GetImmutableRule(suite.ctx, immutableID) + suite.NotNil(err) +} + +// TestArtifactHandler tests ArtifactHandler. +func TestProjectEventHandler(t *testing.T) { + suite.Run(t, &ProjectHandlerTestSuite{}) +} diff --git a/src/controller/immutable/controller.go b/src/controller/immutable/controller.go index 186fea48070c..98d2b945fe46 100644 --- a/src/controller/immutable/controller.go +++ b/src/controller/immutable/controller.go @@ -47,6 +47,9 @@ type Controller interface { // Count count the immutable rules Count(ctx context.Context, query *q.Query) (int64, error) + + // DeleteImmutableRuleByProject delete immuatable rules with project id + DeleteImmutableRuleByProject(ctx context.Context, projectID int64) error } // DefaultAPIController ... @@ -54,6 +57,19 @@ type DefaultAPIController struct { manager immutable.Manager } +func (r *DefaultAPIController) DeleteImmutableRuleByProject(ctx context.Context, projectID int64) error { + rules, err := r.ListImmutableRules(ctx, q.New(q.KeyWords{"ProjectID": projectID})) + if err != nil { + return err + } + for _, rule := range rules { + if err = r.DeleteImmutableRule(ctx, rule.ID); err != nil { + return err + } + } + return nil +} + // GetImmutableRule ... func (r *DefaultAPIController) GetImmutableRule(ctx context.Context, id int64) (*model.Metadata, error) { return r.manager.GetImmutableRule(ctx, id) diff --git a/src/controller/retention/controller.go b/src/controller/retention/controller.go index ef79c5b6a6b0..986268e58cbe 100644 --- a/src/controller/retention/controller.go +++ b/src/controller/retention/controller.go @@ -62,6 +62,8 @@ type Controller interface { GetRetentionExecTaskLog(ctx context.Context, taskID int64) ([]byte, error) GetRetentionExecTask(ctx context.Context, taskID int64) (*retention.Task, error) + // DeleteRetentionByProject delete retetion rule by project id + DeleteRetentionByProject(ctx context.Context, projectID int64) error } var ( @@ -405,6 +407,21 @@ func (r *defaultController) UpdateTaskInfo(ctx context.Context, taskID int64, to return r.taskMgr.UpdateExtraAttrs(ctx, taskID, t.ExtraAttrs) } +func (r *defaultController) DeleteRetentionByProject(ctx context.Context, projectID int64) error { + policyIDs, err := r.manager.ListPolicyIDs(ctx, + q.New(q.KeyWords{"scope_level": "project", + "scope_reference": fmt.Sprintf("%d", projectID)})) + if err != nil { + return err + } + for _, policyID := range policyIDs { + if err := r.DeleteRetention(ctx, policyID); err != nil { + return err + } + } + return nil +} + // NewController ... func NewController() Controller { retentionMgr := retention.NewManager() diff --git a/src/pkg/retention/dao/retention.go b/src/pkg/retention/dao/retention.go index fca8fd992ece..7106aa17f6b8 100644 --- a/src/pkg/retention/dao/retention.go +++ b/src/pkg/retention/dao/retention.go @@ -18,6 +18,7 @@ import ( "context" "github.com/goharbor/harbor/src/lib/orm" + "github.com/goharbor/harbor/src/lib/q" "github.com/goharbor/harbor/src/pkg/retention/dao/models" ) @@ -67,3 +68,16 @@ func GetPolicy(ctx context.Context, id int64) (*models.RetentionPolicy, error) { } return p, nil } + +// ListPolicy list retention policy by query +func ListPolicy(ctx context.Context, query *q.Query) ([]*models.RetentionPolicy, error) { + plcs := []*models.RetentionPolicy{} + qs, err := orm.QuerySetter(ctx, &models.RetentionPolicy{}, query) + if err != nil { + return nil, err + } + if _, err = qs.All(&plcs); err != nil { + return nil, err + } + return plcs, nil +} diff --git a/src/pkg/retention/dao/retention_test.go b/src/pkg/retention/dao/retention_test.go index 6ed92c8e6c12..318d2db048ca 100644 --- a/src/pkg/retention/dao/retention_test.go +++ b/src/pkg/retention/dao/retention_test.go @@ -11,6 +11,7 @@ import ( "github.com/goharbor/harbor/src/common/dao" "github.com/goharbor/harbor/src/lib/orm" + "github.com/goharbor/harbor/src/lib/q" "github.com/goharbor/harbor/src/pkg/retention/dao/models" "github.com/goharbor/harbor/src/pkg/retention/policy" "github.com/goharbor/harbor/src/pkg/retention/policy/rule" @@ -63,10 +64,11 @@ func TestPolicy(t *testing.T) { }, } p1 := &models.RetentionPolicy{ - ScopeLevel: p.Scope.Level, - TriggerKind: p.Trigger.Kind, - CreateTime: time.Now(), - UpdateTime: time.Now(), + ScopeLevel: p.Scope.Level, + ScopeReference: 1, + TriggerKind: p.Trigger.Kind, + CreateTime: time.Now(), + UpdateTime: time.Now(), } data, _ := json.Marshal(p) p1.Data = string(data) @@ -81,6 +83,10 @@ func TestPolicy(t *testing.T) { assert.EqualValues(t, "project", p1.ScopeLevel) assert.True(t, p1.ID > 0) + plcs, err := ListPolicy(ctx, q.New(q.KeyWords{"scope_level": "project", "scope_reference": "1"})) + assert.Nil(t, err) + assert.True(t, len(plcs) > 0) + p1.ScopeLevel = "test" err = UpdatePolicy(ctx, p1) assert.Nil(t, err) diff --git a/src/pkg/retention/launcher_test.go b/src/pkg/retention/launcher_test.go index d715f9d76ae6..92ab1a5fc472 100644 --- a/src/pkg/retention/launcher_test.go +++ b/src/pkg/retention/launcher_test.go @@ -24,6 +24,7 @@ import ( "github.com/goharbor/harbor/src/common/job" "github.com/goharbor/harbor/src/lib/orm" + pq "github.com/goharbor/harbor/src/lib/q" _ "github.com/goharbor/harbor/src/lib/selector/selectors/doublestar" "github.com/goharbor/harbor/src/pkg/project" proModels "github.com/goharbor/harbor/src/pkg/project/models" @@ -40,6 +41,9 @@ import ( type fakeRetentionManager struct{} +func (f *fakeRetentionManager) ListPolicyIDs(ctx context.Context, query *pq.Query) ([]int64, error) { + return nil, nil +} func (f *fakeRetentionManager) GetTotalOfRetentionExecs(policyID int64) (int64, error) { return 0, nil } diff --git a/src/pkg/retention/manager.go b/src/pkg/retention/manager.go index a04ac8674bb8..82c7023ca549 100644 --- a/src/pkg/retention/manager.go +++ b/src/pkg/retention/manager.go @@ -24,6 +24,7 @@ import ( "github.com/go-openapi/strfmt" "github.com/goharbor/harbor/src/common/utils" + "github.com/goharbor/harbor/src/lib/q" "github.com/goharbor/harbor/src/pkg/retention/dao" "github.com/goharbor/harbor/src/pkg/retention/dao/models" "github.com/goharbor/harbor/src/pkg/retention/policy" @@ -41,6 +42,8 @@ type Manager interface { DeletePolicy(ctx context.Context, id int64) error // Get the specified policy GetPolicy(ctx context.Context, id int64) (*policy.Metadata, error) + // List the retention policy with query conditions + ListPolicyIDs(ctx context.Context, query *q.Query) ([]int64, error) } // DefaultManager ... @@ -103,6 +106,19 @@ func (d *DefaultManager) GetPolicy(ctx context.Context, id int64) (*policy.Metad return p, nil } +// ListPolicyIDs list policy id by query +func (d *DefaultManager) ListPolicyIDs(ctx context.Context, query *q.Query) ([]int64, error) { + policyIDs := make([]int64, 0) + plcs, err := dao.ListPolicy(ctx, query) + if err != nil { + return nil, err + } + for _, p := range plcs { + policyIDs = append(policyIDs, p.ID) + } + return policyIDs, nil +} + // NewManager ... func NewManager() Manager { return &DefaultManager{} diff --git a/src/testing/controller/retention/controller.go b/src/testing/controller/retention/controller.go index 14c089ceee14..5f158ffae1bf 100644 --- a/src/testing/controller/retention/controller.go +++ b/src/testing/controller/retention/controller.go @@ -56,6 +56,20 @@ func (_m *Controller) DeleteRetention(ctx context.Context, id int64) error { return r0 } +// DeleteRetentionByProject provides a mock function with given fields: ctx, projectID +func (_m *Controller) DeleteRetentionByProject(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 +} + // GetRetention provides a mock function with given fields: ctx, id func (_m *Controller) GetRetention(ctx context.Context, id int64) (*policy.Metadata, error) { ret := _m.Called(ctx, id)