diff --git a/make/migrations/postgresql/0030_1.11.0_schema.up.sql b/make/migrations/postgresql/0030_1.11.0_schema.up.sql index fadafa44c..406793d7e 100644 --- a/make/migrations/postgresql/0030_1.11.0_schema.up.sql +++ b/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) - ); +); + diff --git a/src/api/artifact/controller.go b/src/api/artifact/controller.go index b6c49877d..5f3ae2d39 100644 --- a/src/api/artifact/controller.go +++ b/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 diff --git a/src/api/artifact/controller_test.go b/src/api/artifact/controller_test.go index 0f462bc9d..ec65f4d13 100644 --- a/src/api/artifact/controller_test.go +++ b/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) } diff --git a/src/pkg/artifact/dao/dao.go b/src/pkg/artifact/dao/dao.go index ada009aa1..35ce27381 100644 --- a/src/pkg/artifact/dao/dao.go +++ b/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 { diff --git a/src/pkg/artifactrash/dao/dao.go b/src/pkg/artifactrash/dao/dao.go new file mode 100644 index 000000000..c3be68660 --- /dev/null +++ b/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 +} diff --git a/src/pkg/artifactrash/dao/dao_test.go b/src/pkg/artifactrash/dao/dao_test.go new file mode 100644 index 000000000..76635864a --- /dev/null +++ b/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") +} diff --git a/src/pkg/artifactrash/manager.go b/src/pkg/artifactrash/manager.go new file mode 100644 index 000000000..ba6808be9 --- /dev/null +++ b/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) +} diff --git a/src/pkg/artifactrash/manager_test.go b/src/pkg/artifactrash/manager_test.go new file mode 100644 index 000000000..53f7e256d --- /dev/null +++ b/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) +} diff --git a/src/pkg/artifactrash/model/model.go b/src/pkg/artifactrash/model/model.go new file mode 100644 index 000000000..c1cd5688d --- /dev/null +++ b/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" +} diff --git a/src/registryctl/api/registry.go b/src/registryctl/api/registry.go index 1cb20d926..e453be6df 100644 --- a/src/registryctl/api/registry.go +++ b/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 diff --git a/src/testing/pkg/artifactrash/manager.go b/src/testing/pkg/artifactrash/manager.go new file mode 100644 index 000000000..210069db4 --- /dev/null +++ b/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) +}