Skip to content

Commit

Permalink
Delete tag retention rule and tag immutable rule when deleting project
Browse files Browse the repository at this point in the history
fixes #18250

Signed-off-by: stonezdj <daojunz@vmware.com>
  • Loading branch information
stonezdj committed Sep 23, 2023
1 parent 560e6cd commit feda341
Show file tree
Hide file tree
Showing 10 changed files with 156 additions and 4 deletions.
1 change: 1 addition & 0 deletions src/controller/event/handler/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ func init() {
_ = notifier.Subscribe(event.TopicPullArtifact, &internal.Handler{})
_ = notifier.Subscribe(event.TopicPushArtifact, &internal.Handler{})
_ = notifier.Subscribe(event.TopicDeleteArtifact, &internal.Handler{})
_ = notifier.Subscribe(event.TopicDeleteProject, &internal.Handler{})

_ = task.RegisterTaskStatusChangePostFunc(job.ReplicationVendorType, func(ctx context.Context, taskID int64, status string) error {
notification.AddEvent(ctx, &metadata.ReplicationMetaData{
Expand Down
19 changes: 19 additions & 0 deletions src/controller/event/handler/internal/artifact.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ import (
"github.com/goharbor/harbor/src/controller/artifact"
"github.com/goharbor/harbor/src/controller/event"
"github.com/goharbor/harbor/src/controller/event/operator"
"github.com/goharbor/harbor/src/controller/immutable"
"github.com/goharbor/harbor/src/controller/repository"
"github.com/goharbor/harbor/src/controller/retention"
"github.com/goharbor/harbor/src/controller/tag"
"github.com/goharbor/harbor/src/jobservice/job"
"github.com/goharbor/harbor/src/lib/config"
Expand Down Expand Up @@ -102,6 +104,8 @@ func (a *Handler) Handle(ctx context.Context, value interface{}) error {
return a.onPush(ctx, v.ArtifactEvent)
case *event.DeleteArtifactEvent:
return a.onDelete(ctx, v.ArtifactEvent)
case *event.DeleteProjectEvent:
return a.onProjectDelete(ctx, v)

Check warning on line 108 in src/controller/event/handler/internal/artifact.go

View check run for this annotation

Codecov / codecov/patch

src/controller/event/handler/internal/artifact.go#L107-L108

Added lines #L107 - L108 were not covered by tests
default:
log.Errorf("Can not handler this event type! %#v", v)
}
Expand Down Expand Up @@ -317,6 +321,21 @@ func (a *Handler) onDelete(ctx context.Context, event *event.ArtifactEvent) erro
return nil
}

func (a *Handler) 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)
}

Check warning on line 330 in src/controller/event/handler/internal/artifact.go

View check run for this annotation

Codecov / codecov/patch

src/controller/event/handler/internal/artifact.go#L329-L330

Added lines #L329 - L330 were not covered by tests
// delete tag retention
err = retention.Ctl.DeleteRetentionByProject(ctx, event.ProjectID)
if err != nil {
log.Errorf("failed to delete retention rule, error %v", err)
}

Check warning on line 335 in src/controller/event/handler/internal/artifact.go

View check run for this annotation

Codecov / codecov/patch

src/controller/event/handler/internal/artifact.go#L334-L335

Added lines #L334 - L335 were not covered by tests
return nil
}

// isScannerUser check if the current user is a scanner user by its prefix
// usually a scanner user should be named like `robot$<projectName>+<Scanner UUID (8byte)>-<Scanner Name>-<UUID>`
// verify it by the prefix `robot$<projectName>+<Scanner UUID (8byte)>`
Expand Down
45 changes: 45 additions & 0 deletions src/controller/event/handler/internal/artifact_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,16 @@ import (

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/controller/scanner"
"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"
_ "github.com/goharbor/harbor/src/pkg/config/db"
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"
Expand Down Expand Up @@ -162,6 +165,48 @@ func (suite *ArtifactHandlerTestSuite) TestOnPull() {
}, 3*asyncFlushDuration, asyncFlushDuration/2, "wait for pull_count async update")
}

func (suite *ArtifactHandlerTestSuite) 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)
}

func (suite *ArtifactHandlerTestSuite) TestOnDelete() {
evt := &event.ArtifactEvent{Artifact: &artifact.Artifact{ID: 1, RepositoryID: 1, Digest: "mock-digest", References: []*artifact.Reference{{ChildDigest: "ref-1", ChildID: 2}, {ChildDigest: "ref-2", ChildID: 3}}}}
suite.execMgr.On("DeleteByVendor", suite.ctx, "IMAGE_SCAN", int64(1)).Return(nil).Times(1)
Expand Down
16 changes: 16 additions & 0 deletions src/controller/immutable/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,29 @@ 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 ...
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
}

Check warning on line 64 in src/controller/immutable/controller.go

View check run for this annotation

Codecov / codecov/patch

src/controller/immutable/controller.go#L63-L64

Added lines #L63 - L64 were not covered by tests
for _, rule := range rules {
if err = r.DeleteImmutableRule(ctx, rule.ID); err != nil {
return err
}

Check warning on line 68 in src/controller/immutable/controller.go

View check run for this annotation

Codecov / codecov/patch

src/controller/immutable/controller.go#L67-L68

Added lines #L67 - L68 were not covered by tests
}
return nil
}

// GetImmutableRule ...
func (r *DefaultAPIController) GetImmutableRule(ctx context.Context, id int64) (*model.Metadata, error) {
return r.manager.GetImmutableRule(ctx, id)
Expand Down
17 changes: 17 additions & 0 deletions src/controller/retention/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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
}

Check warning on line 416 in src/controller/retention/controller.go

View check run for this annotation

Codecov / codecov/patch

src/controller/retention/controller.go#L415-L416

Added lines #L415 - L416 were not covered by tests
for _, policyID := range policyIDs {
if err := r.DeleteRetention(ctx, policyID); err != nil {
return err
}

Check warning on line 420 in src/controller/retention/controller.go

View check run for this annotation

Codecov / codecov/patch

src/controller/retention/controller.go#L418-L420

Added lines #L418 - L420 were not covered by tests
}
return nil
}

// NewController ...
func NewController() Controller {
retentionMgr := retention.NewManager()
Expand Down
14 changes: 14 additions & 0 deletions src/pkg/retention/dao/retention.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
}

Check warning on line 78 in src/pkg/retention/dao/retention.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/retention/dao/retention.go#L77-L78

Added lines #L77 - L78 were not covered by tests
if _, err = qs.All(&plcs); err != nil {
return nil, err
}

Check warning on line 81 in src/pkg/retention/dao/retention.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/retention/dao/retention.go#L80-L81

Added lines #L80 - L81 were not covered by tests
return plcs, nil
}
14 changes: 10 additions & 4 deletions src/pkg/retention/dao/retention_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions src/pkg/retention/launcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
}
Expand Down
16 changes: 16 additions & 0 deletions src/pkg/retention/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 ...
Expand Down Expand Up @@ -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

Check warning on line 119 in src/pkg/retention/manager.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/retention/manager.go#L110-L119

Added lines #L110 - L119 were not covered by tests
}

// NewManager ...
func NewManager() Manager {
return &DefaultManager{}
Expand Down
14 changes: 14 additions & 0 deletions src/testing/controller/retention/controller.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit feda341

Please sign in to comment.