add artifact trash manager

1, move the deleted artifact into trash
2, disable GC to delete the untagged manifest

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

make/migrations/postgresql/0030_1.11.0_schema.up.sql

@@ -43,6 +43,17 @@ CREATE TABLE artifact_reference
   CONSTRAINT  unique_reference UNIQUE (parent_id, child_id)
 );
 
+/* artifact_trash records deleted artifact */
+CREATE TABLE artifact_trash
+(
+  id                  SERIAL PRIMARY KEY NOT NULL,
+  media_type          varchar(255) NOT NULL,
+  manifest_media_type varchar(255) NOT NULL,
+  repository_name     varchar(255) NOT NULL,
+  digest              varchar(255) NOT NULL,
+  creation_time       timestamp default CURRENT_TIMESTAMP,
+  CONSTRAINT      unique_artifact_trash UNIQUE (repository_name, digest)
+);
 
 /* TODO upgrade: how about keep the table "harbor_resource_label" only for helm v2 chart and use the new table for artifact label reference? */
 /* label_reference records the labels added to the artifact */
@@ -56,4 +67,5 @@ CREATE TABLE label_reference (
  /* TODO replace artifact_2 after finishing the upgrade work */
  FOREIGN KEY (artifact_id) REFERENCES artifact_2(id),
  CONSTRAINT unique_label_reference UNIQUE (label_id,artifact_id)
- );
+);
+

src/api/artifact/controller.go

@@ -23,6 +23,8 @@ import (
 	"github.com/goharbor/harbor/src/common/utils"
 	"github.com/goharbor/harbor/src/internal"
 	"github.com/goharbor/harbor/src/pkg/art"
+	"github.com/goharbor/harbor/src/pkg/artifactrash"
+	"github.com/goharbor/harbor/src/pkg/artifactrash/model"
 	"github.com/goharbor/harbor/src/pkg/immutabletag/match"
 	"github.com/goharbor/harbor/src/pkg/immutabletag/match/rule"
 	"github.com/goharbor/harbor/src/pkg/label"
@@ -89,6 +91,7 @@ func NewController() Controller {
 	return &controller{
 		repoMgr:      repository.Mgr,
 		artMgr:       artifact.Mgr,
+		artrashMgr:   artifactrash.Mgr,
 		tagMgr:       tag.Mgr,
 		sigMgr:       signature.GetManager(),
 		labelMgr:     label.Mgr,
@@ -102,6 +105,7 @@ func NewController() Controller {
 type controller struct {
 	repoMgr      repository.Manager
 	artMgr       artifact.Manager
+	artrashMgr   artifactrash.Manager
 	tagMgr       tag.Manager
 	sigMgr       signature.Manager
 	labelMgr     label.Manager
@@ -288,6 +292,7 @@ func (c *controller) deleteDeeply(ctx context.Context, id int64, isRoot bool) er
 		}
 		return err
 	}
+
 	// the child artifact is referenced by some tags, skip
 	if !isRoot && len(art.Tags) > 0 {
 		return nil
@@ -342,6 +347,19 @@ func (c *controller) deleteDeeply(ctx context.Context, id int64, isRoot bool) er
 		return err
 	}
 
+	repo, err := c.repoMgr.Get(ctx, art.RepositoryID)
+	if err != nil && !ierror.IsErr(err, ierror.NotFoundCode) {
+		return err
+	}
+	_, err = c.artrashMgr.Create(ctx, &model.ArtifactTrash{
+		MediaType:         art.MediaType,
+		ManifestMediaType: art.ManifestMediaType,
+		RepositoryName:    repo.Name,
+		Digest:            art.Digest,
+	})
+	if err != nil && !ierror.IsErr(err, ierror.ConflictCode) {
+		return err
+	}
 	// TODO fire delete artifact event
 
 	return nil

src/api/artifact/controller_test.go

@@ -25,6 +25,7 @@ import (
 	"github.com/goharbor/harbor/src/pkg/q"
 	"github.com/goharbor/harbor/src/pkg/tag/model/tag"
 	arttesting "github.com/goharbor/harbor/src/testing/pkg/artifact"
+	artrashtesting "github.com/goharbor/harbor/src/testing/pkg/artifactrash"
 	immutesting "github.com/goharbor/harbor/src/testing/pkg/immutabletag"
 	"github.com/goharbor/harbor/src/testing/pkg/label"
 	repotesting "github.com/goharbor/harbor/src/testing/pkg/repository"
@@ -69,6 +70,7 @@ type controllerTestSuite struct {
 	ctl          *controller
 	repoMgr      *repotesting.FakeManager
 	artMgr       *arttesting.FakeManager
+	artrashMgr   *artrashtesting.FakeManager
 	tagMgr       *tagtesting.FakeManager
 	labelMgr     *label.FakeManager
 	abstractor   *fakeAbstractor
@@ -78,6 +80,7 @@ type controllerTestSuite struct {
 func (c *controllerTestSuite) SetupTest() {
 	c.repoMgr = &repotesting.FakeManager{}
 	c.artMgr = &arttesting.FakeManager{}
+	c.artrashMgr = &artrashtesting.FakeManager{}
 	c.tagMgr = &tagtesting.FakeManager{}
 	c.labelMgr = &label.FakeManager{}
 	c.abstractor = &fakeAbstractor{}
@@ -85,6 +88,7 @@ func (c *controllerTestSuite) SetupTest() {
 	c.ctl = &controller{
 		repoMgr:      c.repoMgr,
 		artMgr:       c.artMgr,
+		artrashMgr:   c.artrashMgr,
 		tagMgr:       c.tagMgr,
 		labelMgr:     c.labelMgr,
 		abstractor:   c.abstractor,
@@ -424,12 +428,14 @@ func (c *controllerTestSuite) TestDeleteDeeply() {
 
 	// child artifact and contains tags
 	c.artMgr.On("Get").Return(&artifact.Artifact{ID: 1}, nil)
+	c.artMgr.On("Delete").Return(nil)
 	c.tagMgr.On("List").Return(0, []*tag.Tag{
 		{
 			ID: 1,
 		},
 	}, nil)
 	c.repoMgr.On("Get").Return(&models.RepoRecord{}, nil)
+	c.artrashMgr.On("Create").Return(0, nil)
 	err = c.ctl.deleteDeeply(nil, 1, false)
 	c.Require().Nil(err)
 
@@ -474,6 +480,7 @@ func (c *controllerTestSuite) TestDeleteDeeply() {
 	c.tagMgr.On("DeleteOfArtifact").Return(nil)
 	c.artMgr.On("Delete").Return(nil)
 	c.labelMgr.On("RemoveAllFrom").Return(nil)
+	c.artrashMgr.On("Create").Return(0, nil)
 	err = c.ctl.deleteDeeply(nil, 1, true)
 	c.Require().Nil(err)
 }

src/pkg/artifact/dao/dao.go

@@ -171,6 +171,7 @@ func (d *dao) Delete(ctx context.Context, id int64) error {
 	if n == 0 {
 		return ierror.NotFoundError(nil).WithMessage("artifact %d not found", id)
 	}
+
 	return nil
 }
 func (d *dao) Update(ctx context.Context, artifact *Artifact, props ...string) error {

src/pkg/artifactrash/dao/dao.go

@@ -0,0 +1,78 @@
+package dao
+
+import (
+	"context"
+	ierror "github.com/goharbor/harbor/src/internal/error"
+	"github.com/goharbor/harbor/src/internal/orm"
+	"github.com/goharbor/harbor/src/pkg/artifactrash/model"
+	"time"
+)
+
+// DAO is the data access object interface for artifact trash
+type DAO interface {
+	// Create the artifact trash
+	Create(ctx context.Context, artifactrsh *model.ArtifactTrash) (id int64, err error)
+	// Delete the artifact trash specified by ID
+	Delete(ctx context.Context, id int64) (err error)
+	// Filter lists the artifact that needs to be cleaned
+	Filter(ctx context.Context) (arts []model.ArtifactTrash, err error)
+}
+
+// New returns an instance of the default DAO
+func New() DAO {
+	return &dao{}
+}
+
+type dao struct{}
+
+// Create ...
+func (d *dao) Create(ctx context.Context, artifactrsh *model.ArtifactTrash) (id int64, err error) {
+	ormer, err := orm.FromContext(ctx)
+	if err != nil {
+		return 0, err
+	}
+	artifactrsh.CreationTime = time.Now()
+	id, err = ormer.Insert(artifactrsh)
+	if err != nil {
+		if e := orm.AsConflictError(err, "artifact trash %s already exists under the repository %s",
+			artifactrsh.Digest, artifactrsh.RepositoryName); e != nil {
+			err = e
+		}
+	}
+	return id, err
+}
+
+// Delete ...
+func (d *dao) Delete(ctx context.Context, id int64) (err error) {
+	ormer, err := orm.FromContext(ctx)
+	if err != nil {
+		return err
+	}
+	n, err := ormer.Delete(&model.ArtifactTrash{
+		ID: id,
+	})
+	if err != nil {
+		return err
+	}
+	if n == 0 {
+		return ierror.NotFoundError(nil).WithMessage("artifact trash %d not found", id)
+	}
+	return nil
+}
+
+// Filter ...
+// ToDo replace artifact_2 with artifact
+func (d *dao) Filter(ctx context.Context) (arts []model.ArtifactTrash, err error) {
+	var deletedAfs []model.ArtifactTrash
+	ormer, err := orm.FromContext(ctx)
+	if err != nil {
+		return deletedAfs, err
+	}
+
+	sql := `SELECT * FROM artifact_trash where artifact_trash.digest NOT IN (select digest from artifact_2)`
+
+	if err := ormer.Raw(sql).QueryRow(&deletedAfs); err != nil {
+		return deletedAfs, err
+	}
+	return deletedAfs, nil
+}

src/pkg/artifactrash/dao/dao_test.go

@@ -0,0 +1,91 @@
+package dao
+
+import (
+	"context"
+	"errors"
+	beegoorm "github.com/astaxie/beego/orm"
+	common_dao "github.com/goharbor/harbor/src/common/dao"
+	ierror "github.com/goharbor/harbor/src/internal/error"
+	"github.com/goharbor/harbor/src/internal/orm"
+	artdao "github.com/goharbor/harbor/src/pkg/artifact/dao"
+	"github.com/goharbor/harbor/src/pkg/artifactrash/model"
+	v1 "github.com/opencontainers/image-spec/specs-go/v1"
+	"github.com/stretchr/testify/suite"
+)
+
+type daoTestSuite struct {
+	suite.Suite
+	dao   DAO
+	afDao artdao.DAO
+	id    int64
+	ctx   context.Context
+}
+
+func (d *daoTestSuite) SetupSuite() {
+	d.dao = New()
+	common_dao.PrepareTestForPostgresSQL()
+	d.ctx = orm.NewContext(nil, beegoorm.NewOrm())
+}
+
+func (d *daoTestSuite) SetupTest() {
+	art1 := &artdao.Artifact{
+		Type:              "image",
+		ManifestMediaType: v1.MediaTypeImageManifest,
+		ProjectID:         10,
+		RepositoryID:      10,
+		Digest:            "1234",
+	}
+	id, err := d.afDao.Create(d.ctx, art1)
+	d.Require().Nil(err)
+	err = d.afDao.Delete(d.ctx, id)
+	d.Require().Nil(err)
+	art2 := &artdao.Artifact{
+		Type:              "image",
+		ManifestMediaType: v1.MediaTypeImageManifest,
+		ProjectID:         10,
+		RepositoryID:      10,
+		Digest:            "5678",
+	}
+	_, err = d.afDao.Create(d.ctx, art2)
+	d.Require().Nil(err)
+
+	aft := &model.ArtifactTrash{
+		ManifestMediaType: v1.MediaTypeImageManifest,
+		RepositoryName:    "test/hello-world",
+		Digest:            "1234",
+	}
+	id, err = d.dao.Create(d.ctx, aft)
+	d.Require().Nil(err)
+	d.id = id
+}
+
+func (d *daoTestSuite) TearDownTest() {
+	err := d.dao.Delete(d.ctx, d.id)
+	d.Require().Nil(err)
+}
+
+func (d *daoTestSuite) TestCreate() {
+	// conflict
+	aft := &model.ArtifactTrash{
+		ManifestMediaType: v1.MediaTypeImageManifest,
+		RepositoryName:    "test/hello-world",
+	}
+
+	_, err := d.dao.Create(d.ctx, aft)
+	d.Require().NotNil(err)
+	d.True(ierror.IsErr(err, ierror.ConflictCode))
+}
+
+func (d *daoTestSuite) TestDelete() {
+	err := d.dao.Delete(d.ctx, 100021)
+	d.Require().NotNil(err)
+	var e *ierror.Error
+	d.Require().True(errors.As(err, &e))
+	d.Equal(ierror.NotFoundCode, e.Code)
+}
+
+func (d *daoTestSuite) TestFilter() {
+	afs, err := d.dao.Filter(d.ctx)
+	d.Require().NotNil(err)
+	d.Require().Equal(afs[0].Digest, "1234")
+}

src/pkg/artifactrash/manager.go

@@ -0,0 +1,59 @@
+// 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 artifactrash
+
+import (
+	"context"
+	"github.com/goharbor/harbor/src/pkg/artifactrash/dao"
+	"github.com/goharbor/harbor/src/pkg/artifactrash/model"
+)
+
+var (
+	// Mgr is a global artifact trash manager instance
+	Mgr = NewManager()
+)
+
+// Manager is the only interface of artifact module to provide the management functions for artifacts
+type Manager interface {
+	// Create ...
+	Create(ctx context.Context, artifactrsh *model.ArtifactTrash) (id int64, err error)
+	// Delete ...
+	Delete(ctx context.Context, id int64) (err error)
+	// Filter ...
+	Filter(ctx context.Context) (arts []model.ArtifactTrash, err error)
+}
+
+// NewManager returns an instance of the default manager
+func NewManager() Manager {
+	return &manager{
+		dao.New(),
+	}
+}
+
+var _ Manager = &manager{}
+
+type manager struct {
+	dao dao.DAO
+}
+
+func (m *manager) Create(ctx context.Context, artifactrsh *model.ArtifactTrash) (id int64, err error) {
+	return m.dao.Create(ctx, artifactrsh)
+}
+func (m *manager) Delete(ctx context.Context, id int64) error {
+	return m.dao.Delete(ctx, id)
+}
+func (m *manager) Filter(ctx context.Context) (arts []model.ArtifactTrash, err error) {
+	return m.dao.Filter(ctx)
+}

src/pkg/artifactrash/manager_test.go

@@ -0,0 +1,71 @@
+package artifactrash
+
+import (
+	"context"
+	"github.com/goharbor/harbor/src/pkg/artifactrash/model"
+	v1 "github.com/opencontainers/image-spec/specs-go/v1"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/suite"
+)
+
+type fakeDao struct {
+	mock.Mock
+}
+
+func (f *fakeDao) Create(ctx context.Context, artifactrsh *model.ArtifactTrash) (id int64, err error) {
+	args := f.Called()
+	return int64(args.Int(0)), args.Error(1)
+}
+func (f *fakeDao) Delete(ctx context.Context, id int64) (err error) {
+	args := f.Called()
+	return args.Error(0)
+}
+func (f *fakeDao) Filter(ctx context.Context) (arts []model.ArtifactTrash, err error) {
+	args := f.Called()
+	return args.Get(0).([]model.ArtifactTrash), args.Error(1)
+}
+
+type managerTestSuite struct {
+	suite.Suite
+	mgr *manager
+	dao *fakeDao
+}
+
+func (m *managerTestSuite) SetupTest() {
+	m.dao = &fakeDao{}
+	m.mgr = &manager{
+		dao: m.dao,
+	}
+}
+
+func (m *managerTestSuite) TestCreate() {
+	m.dao.On("Create", mock.Anything).Return(1, nil)
+	id, err := m.mgr.Create(nil, &model.ArtifactTrash{
+		ManifestMediaType: v1.MediaTypeImageManifest,
+		RepositoryName:    "test/hello-world",
+		Digest:            "5678",
+	})
+	m.Require().Nil(err)
+	m.dao.AssertExpectations(m.T())
+	m.Equal(int64(1), id)
+}
+
+func (m *managerTestSuite) TestDelete() {
+	m.dao.On("Delete", mock.Anything).Return(nil)
+	err := m.mgr.Delete(nil, 1)
+	m.Require().Nil(err)
+	m.dao.AssertExpectations(m.T())
+}
+
+func (m *managerTestSuite) TestFilter() {
+	m.dao.On("Filter", mock.Anything).Return([]model.ArtifactTrash{
+		{
+			ManifestMediaType: v1.MediaTypeImageManifest,
+			RepositoryName:    "test/hello-world",
+			Digest:            "5678",
+		},
+	}, nil)
+	arts, err := m.mgr.Filter(nil)
+	m.Require().Nil(err)
+	m.Equal(len(arts), 1)
+}

src/pkg/artifactrash/model/model.go

@@ -0,0 +1,39 @@
+// 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 model
+
+import (
+	"github.com/astaxie/beego/orm"
+	"time"
+)
+
+func init() {
+	orm.RegisterModel(&ArtifactTrash{})
+}
+
+// ArtifactTrash records the deleted artifact
+type ArtifactTrash struct {
+	ID                int64     `orm:"pk;auto;column(id)"`
+	MediaType         string    `orm:"column(media_type)"`
+	ManifestMediaType string    `orm:"column(manifest_media_type)"`
+	RepositoryName    string    `orm:"column(repository_name)"`
+	Digest            string    `orm:"column(digest)"`
+	CreationTime      time.Time `orm:"column(creation_time);auto_now_add" json:"creation_time"`
+}
+
+// TableName for artifact trash
+func (at *ArtifactTrash) TableName() string {
+	return "artifact_trash"
+}

src/registryctl/api/registry.go

@@ -38,7 +38,7 @@ type GCResult struct {
 
 // StartGC ...
 func StartGC(w http.ResponseWriter, r *http.Request) {
-	cmd := exec.Command("/bin/bash", "-c", "registry garbage-collect --delete-untagged=true "+regConf)
+	cmd := exec.Command("/bin/bash", "-c", "registry garbage-collect --delete-untagged=false "+regConf)
 	var outBuf, errBuf bytes.Buffer
 	cmd.Stdout = &outBuf
 	cmd.Stderr = &errBuf

src/testing/pkg/artifactrash/manager.go

@@ -0,0 +1,47 @@
+package artifactrash
+
+import (
+	"github.com/goharbor/harbor/src/pkg/artifactrash/model"
+	"github.com/stretchr/testify/mock"
+)
+
+// 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.
+
+import (
+	"context"
+)
+
+// FakeManager is a fake tag manager that implement the src/pkg/tag.Manager interface
+type FakeManager struct {
+	mock.Mock
+}
+
+// Create ...
+func (f *FakeManager) Create(ctx context.Context, artifactrsh *model.ArtifactTrash) (id int64, err error) {
+	args := f.Called()
+	return int64(args.Int(0)), args.Error(1)
+}
+
+// Delete ...
+func (f *FakeManager) Delete(ctx context.Context, id int64) error {
+	args := f.Called()
+	return args.Error(0)
+}
+
+// Filter ...
+func (f *FakeManager) Filter(ctx context.Context) (arts []model.ArtifactTrash, err error) {
+	args := f.Called()
+	return args.Get(0).([]model.ArtifactTrash), args.Error(1)
+}