From 79cf21f82fc0ba74d74814a7bc326567a931dcbc Mon Sep 17 00:00:00 2001 From: wang yan Date: Thu, 27 Feb 2020 17:02:11 +0800 Subject: [PATCH 1/2] add tag controller use the tag controller to handle CRUD of tags, especially the delete scenario, it could validate the immutable and signature. And move the code of tag handling from artifact controller to tag controller Signed-off-by: wang yan --- src/api/artifact/controller.go | 140 ++--------- src/api/artifact/controller_test.go | 219 +++++++----------- src/api/artifact/model.go | 21 +- src/api/tag/controller.go | 242 ++++++++++++++++++++ src/api/tag/controller_test.go | 226 ++++++++++++++++++ src/api/tag/model.go | 20 ++ src/pkg/retention/dep/client_test.go | 7 +- src/server/middleware/immutable/deletemf.go | 3 +- src/server/middleware/immutable/pushmf.go | 3 +- src/server/registry/tag_test.go | 27 +-- src/server/v2.0/handler/artifact.go | 5 +- src/testing/api/artifact/controller.go | 9 +- src/testing/api/tag/controller.go | 69 ++++++ tests/apitests/python/test_sign_image.py | 5 +- 14 files changed, 698 insertions(+), 298 deletions(-) create mode 100644 src/api/tag/controller.go create mode 100644 src/api/tag/controller_test.go create mode 100644 src/api/tag/model.go create mode 100644 src/testing/api/tag/controller.go diff --git a/src/api/artifact/controller.go b/src/api/artifact/controller.go index feb41ce38..7cb3ff07c 100644 --- a/src/api/artifact/controller.go +++ b/src/api/artifact/controller.go @@ -20,9 +20,8 @@ import ( "github.com/goharbor/harbor/src/api/artifact/abstractor" "github.com/goharbor/harbor/src/api/artifact/abstractor/resolver" "github.com/goharbor/harbor/src/api/artifact/descriptor" - "github.com/goharbor/harbor/src/common/utils" + "github.com/goharbor/harbor/src/api/tag" "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/blob" @@ -45,8 +44,6 @@ import ( "github.com/goharbor/harbor/src/pkg/artifact" "github.com/goharbor/harbor/src/pkg/q" "github.com/goharbor/harbor/src/pkg/repository" - "github.com/goharbor/harbor/src/pkg/tag" - tm "github.com/goharbor/harbor/src/pkg/tag/model/tag" ) var ( @@ -72,14 +69,14 @@ type Controller interface { // Get the artifact specified by repository name and reference, the reference can be tag or digest, // specify the properties returned with option GetByReference(ctx context.Context, repository, reference string, option *Option) (artifact *Artifact, err error) - // Delete the artifact specified by ID. All tags attached to the artifact are deleted as well + // Delete the artifact specified by artifact ID Delete(ctx context.Context, id int64) (err error) // Copy the artifact specified by "srcRepo" and "reference" into the repository specified by "dstRepo" Copy(ctx context.Context, srcRepo, reference, dstRepo string) (id int64, err error) // ListTags lists the tags according to the query, specify the properties returned with option - ListTags(ctx context.Context, query *q.Query, option *TagOption) (tags []*Tag, err error) + ListTags(ctx context.Context, query *q.Query, option *tag.Option) (tags []*tag.Tag, err error) // CreateTag creates a tag - CreateTag(ctx context.Context, tag *Tag) (id int64, err error) + CreateTag(ctx context.Context, tag *tag.Tag) (id int64, err error) // DeleteTag deletes the tag specified by tagID DeleteTag(ctx context.Context, tagID int64) (err error) // UpdatePullTime updates the pull time for the artifact. If the tagID is provides, update the pull @@ -98,11 +95,11 @@ type Controller interface { // NewController creates an instance of the default artifact controller func NewController() Controller { return &controller{ + tagCtl: tag.Ctl, repoMgr: repository.Mgr, artMgr: artifact.Mgr, artrashMgr: artifactrash.Mgr, blobMgr: blob.Mgr, - tagMgr: tag.Mgr, sigMgr: signature.GetManager(), labelMgr: label.Mgr, abstractor: abstractor.NewAbstractor(), @@ -114,11 +111,11 @@ func NewController() Controller { // TODO concurrency summary type controller struct { + tagCtl tag.Controller repoMgr repository.Manager artMgr artifact.Manager artrashMgr artifactrash.Manager blobMgr blob.Manager - tagMgr tag.Manager sigMgr signature.Manager labelMgr label.Manager abstractor abstractor.Abstractor @@ -132,7 +129,7 @@ func (c *controller) Ensure(ctx context.Context, repository, digest string, tags return false, 0, err } for _, tag := range tags { - if err = c.ensureTag(ctx, artifact.RepositoryID, artifact.ID, tag); err != nil { + if err = c.tagCtl.Ensure(ctx, artifact.RepositoryID, artifact.ID, tag); err != nil { return false, 0, err } } @@ -189,44 +186,6 @@ func (c *controller) ensureArtifact(ctx context.Context, repository, digest stri return true, artifact, nil } -func (c *controller) ensureTag(ctx context.Context, repositoryID, artifactID int64, name string) error { - query := &q.Query{ - Keywords: map[string]interface{}{ - "repository_id": repositoryID, - "name": name, - }, - } - tags, err := c.tagMgr.List(ctx, query) - if err != nil { - return err - } - // the tag already exists under the repository - if len(tags) > 0 { - tag := tags[0] - // the tag already exists under the repository and is attached to the artifact, return directly - if tag.ArtifactID == artifactID { - return nil - } - // the tag exists under the repository, but it is attached to other artifact - // update it to point to the provided artifact - tag.ArtifactID = artifactID - tag.PushTime = time.Now() - return c.tagMgr.Update(ctx, tag, "ArtifactID", "PushTime") - } - // the tag doesn't exist under the repository, create it - _, err = c.tagMgr.Create(ctx, &tm.Tag{ - RepositoryID: repositoryID, - ArtifactID: artifactID, - Name: name, - PushTime: time.Now(), - }) - // ignore the conflict error - if err != nil && ierror.IsConflictErr(err) { - return nil - } - return err -} - func (c *controller) Count(ctx context.Context, query *q.Query) (int64, error) { return c.artMgr.Count(ctx, query) } @@ -274,12 +233,12 @@ func (c *controller) getByTag(ctx context.Context, repository, tag string, optio if err != nil { return nil, err } - tags, err := c.tagMgr.List(ctx, &q.Query{ + tags, err := c.tagCtl.List(ctx, &q.Query{ Keywords: map[string]interface{}{ "RepositoryID": repo.RepositoryID, "Name": tag, }, - }) + }, nil) if err != nil { return nil, err } @@ -341,7 +300,7 @@ func (c *controller) deleteDeeply(ctx context.Context, id int64, isRoot bool) er // delete all tags that attached to the root artifact if isRoot { - if err = c.tagMgr.DeleteOfArtifact(ctx, id); err != nil { + if err = c.tagCtl.DeleteTags(ctx, art.Tags); err != nil { return err } } @@ -448,35 +407,25 @@ func (c *controller) copyDeeply(ctx context.Context, srcRepo, reference, dstRepo return id, nil } -func (c *controller) CreateTag(ctx context.Context, tag *Tag) (int64, error) { +func (c *controller) CreateTag(ctx context.Context, tag *tag.Tag) (int64, error) { // TODO fire event - return c.tagMgr.Create(ctx, &(tag.Tag)) + return c.tagCtl.Create(ctx, tag) } -func (c *controller) ListTags(ctx context.Context, query *q.Query, option *TagOption) ([]*Tag, error) { - tgs, err := c.tagMgr.List(ctx, query) +func (c *controller) ListTags(ctx context.Context, query *q.Query, option *tag.Option) ([]*tag.Tag, error) { + tags, err := c.tagCtl.List(ctx, query, option) if err != nil { return nil, err } - var tags []*Tag - for _, tg := range tgs { - art, err := c.artMgr.Get(ctx, tg.ArtifactID) - if err != nil { - return nil, err - } - tags = append(tags, c.assembleTag(ctx, art, tg, option)) - } return tags, nil } func (c *controller) DeleteTag(ctx context.Context, tagID int64) error { - // Immutable checking is covered in middleware - // TODO check signature // TODO fire delete tag event - return c.tagMgr.Delete(ctx, tagID) + return c.tagCtl.Delete(ctx, tagID) } func (c *controller) UpdatePullTime(ctx context.Context, artifactID int64, tagID int64, time time.Time) error { - tag, err := c.tagMgr.Get(ctx, tagID) + tag, err := c.tagCtl.Get(ctx, tagID, nil) if err != nil { return err } @@ -486,9 +435,7 @@ func (c *controller) UpdatePullTime(ctx context.Context, artifactID int64, tagID if err := c.artMgr.UpdatePullTime(ctx, artifactID, time); err != nil { return err } - return c.tagMgr.Update(ctx, &tm.Tag{ - ID: tagID, - }, "PullTime") + return c.tagCtl.Update(ctx, tag, "PullTime") } func (c *controller) GetAddition(ctx context.Context, artifactID int64, addition string) (*resolver.Addition, error) { @@ -527,62 +474,17 @@ func (c *controller) assembleArtifact(ctx context.Context, art *artifact.Artifac return artifact } -func (c *controller) populateTags(ctx context.Context, art *Artifact, option *TagOption) { - tags, err := c.tagMgr.List(ctx, &q.Query{ +func (c *controller) populateTags(ctx context.Context, art *Artifact, option *tag.Option) { + tags, err := c.tagCtl.List(ctx, &q.Query{ Keywords: map[string]interface{}{ "artifact_id": art.ID, }, - }) + }, option) if err != nil { log.Errorf("failed to list tag of artifact %d: %v", art.ID, err) return } - for _, tag := range tags { - art.Tags = append(art.Tags, c.assembleTag(ctx, &art.Artifact, tag, option)) - } -} - -// assemble several part into a single tag -func (c *controller) assembleTag(ctx context.Context, art *artifact.Artifact, tag *tm.Tag, option *TagOption) *Tag { - t := &Tag{ - Tag: *tag, - } - if option == nil { - return t - } - if option.WithImmutableStatus { - c.populateImmutableStatus(ctx, art, t) - } - if option.WithSignature { - c.populateTagSignature(ctx, art, t, option) - } - return t -} - -func (c *controller) populateImmutableStatus(ctx context.Context, artifact *artifact.Artifact, tag *Tag) { - _, repoName := utils.ParseRepository(artifact.RepositoryName) - matched, err := c.immutableMtr.Match(artifact.ProjectID, art.Candidate{ - Repository: repoName, - Tags: []string{tag.Name}, - NamespaceID: artifact.ProjectID, - }) - if err != nil { - log.Error(err) - return - } - tag.Immutable = matched -} - -func (c *controller) populateTagSignature(ctx context.Context, artifact *artifact.Artifact, tag *Tag, option *TagOption) { - if option.SignatureChecker == nil { - chk, err := signature.GetManager().GetCheckerByRepo(ctx, artifact.RepositoryName) - if err != nil { - log.Error(err) - return - } - option.SignatureChecker = chk - } - tag.Signed = option.SignatureChecker.IsTagSigned(tag.Name, artifact.Digest) + art.Tags = tags } func (c *controller) populateLabels(ctx context.Context, art *Artifact) { diff --git a/src/api/artifact/controller_test.go b/src/api/artifact/controller_test.go index d2af82043..88b6d53b1 100644 --- a/src/api/artifact/controller_test.go +++ b/src/api/artifact/controller_test.go @@ -16,17 +16,16 @@ package artifact import ( "context" - "testing" - "time" - "github.com/goharbor/harbor/src/api/artifact/abstractor/resolver" "github.com/goharbor/harbor/src/api/artifact/descriptor" + "github.com/goharbor/harbor/src/api/tag" "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/internal" ierror "github.com/goharbor/harbor/src/internal/error" "github.com/goharbor/harbor/src/pkg/artifact" "github.com/goharbor/harbor/src/pkg/q" - "github.com/goharbor/harbor/src/pkg/tag/model/tag" + model_tag "github.com/goharbor/harbor/src/pkg/tag/model/tag" + tagtesting "github.com/goharbor/harbor/src/testing/api/tag" arttesting "github.com/goharbor/harbor/src/testing/pkg/artifact" artrashtesting "github.com/goharbor/harbor/src/testing/pkg/artifactrash" "github.com/goharbor/harbor/src/testing/pkg/blob" @@ -34,9 +33,10 @@ import ( "github.com/goharbor/harbor/src/testing/pkg/label" "github.com/goharbor/harbor/src/testing/pkg/registry" repotesting "github.com/goharbor/harbor/src/testing/pkg/repository" - tagtesting "github.com/goharbor/harbor/src/testing/pkg/tag" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" + "testing" + "time" ) // TODO find another way to test artifact controller, it's hard to maintain currently @@ -77,7 +77,7 @@ type controllerTestSuite struct { artMgr *arttesting.FakeManager artrashMgr *artrashtesting.FakeManager blobMgr *blob.Manager - tagMgr *tagtesting.FakeManager + tagCtl *tagtesting.FakeController labelMgr *label.FakeManager abstractor *fakeAbstractor immutableMtr *immutesting.FakeMatcher @@ -89,7 +89,7 @@ func (c *controllerTestSuite) SetupTest() { c.artMgr = &arttesting.FakeManager{} c.artrashMgr = &artrashtesting.FakeManager{} c.blobMgr = &blob.Manager{} - c.tagMgr = &tagtesting.FakeManager{} + c.tagCtl = &tagtesting.FakeController{} c.labelMgr = &label.FakeManager{} c.abstractor = &fakeAbstractor{} c.immutableMtr = &immutesting.FakeMatcher{} @@ -99,7 +99,7 @@ func (c *controllerTestSuite) SetupTest() { artMgr: c.artMgr, artrashMgr: c.artrashMgr, blobMgr: c.blobMgr, - tagMgr: c.tagMgr, + tagCtl: c.tagCtl, labelMgr: c.labelMgr, abstractor: c.abstractor, immutableMtr: c.immutableMtr, @@ -108,34 +108,6 @@ func (c *controllerTestSuite) SetupTest() { descriptor.Register(&fakeDescriptor{}, "") } -func (c *controllerTestSuite) TestAssembleTag() { - art := &artifact.Artifact{ - ID: 1, - ProjectID: 1, - RepositoryID: 1, - RepositoryName: "library/hello-world", - Digest: "sha256:418fb88ec412e340cdbef913b8ca1bbe8f9e8dc705f9617414c1f2c8db980180", - } - tg := &tag.Tag{ - ID: 1, - RepositoryID: 1, - ArtifactID: 1, - Name: "latest", - PushTime: time.Now(), - PullTime: time.Now(), - } - option := &TagOption{ - WithImmutableStatus: true, - } - - c.immutableMtr.On("Match").Return(true, nil) - tag := c.ctl.assembleTag(nil, art, tg, option) - c.Require().NotNil(tag) - c.Equal(tag.ID, tg.ID) - c.Equal(true, tag.Immutable) - // TODO check other fields of option -} - func (c *controllerTestSuite) TestAssembleArtifact() { art := &artifact.Artifact{ ID: 1, @@ -144,20 +116,22 @@ func (c *controllerTestSuite) TestAssembleArtifact() { } option := &Option{ WithTag: true, - TagOption: &TagOption{ + TagOption: &tag.Option{ WithImmutableStatus: false, }, WithLabel: true, } tg := &tag.Tag{ - ID: 1, - RepositoryID: 1, - ArtifactID: 1, - Name: "latest", - PushTime: time.Now(), - PullTime: time.Now(), + Tag: model_tag.Tag{ + ID: 1, + RepositoryID: 1, + ArtifactID: 1, + Name: "latest", + PushTime: time.Now(), + PullTime: time.Now(), + }, } - c.tagMgr.On("List").Return([]*tag.Tag{tg}, nil) + c.tagCtl.On("List").Return([]*tag.Tag{tg}, nil) ctx := internal.SetAPIVersion(nil, "2.0") lb := &models.Label{ ID: 1, @@ -169,7 +143,7 @@ func (c *controllerTestSuite) TestAssembleArtifact() { artifact := c.ctl.assembleArtifact(ctx, art, option) c.Require().NotNil(artifact) c.Equal(art.ID, artifact.ID) - c.Contains(artifact.Tags, &Tag{Tag: *tg}) + c.Contains(artifact.Tags, tg) c.Require().NotNil(artifact.AdditionLinks) c.Require().NotNil(artifact.AdditionLinks["build_history"]) c.False(artifact.AdditionLinks["build_history"].Absolute) @@ -207,48 +181,6 @@ func (c *controllerTestSuite) TestEnsureArtifact() { c.Equal(int64(1), art.ID) } -func (c *controllerTestSuite) TestEnsureTag() { - // the tag already exists under the repository and is attached to the artifact - c.tagMgr.On("List").Return([]*tag.Tag{ - { - ID: 1, - RepositoryID: 1, - ArtifactID: 1, - Name: "latest", - }, - }, nil) - err := c.ctl.ensureTag(nil, 1, 1, "latest") - c.Require().Nil(err) - c.tagMgr.AssertExpectations(c.T()) - - // reset the mock - c.SetupTest() - - // the tag exists under the repository, but it is attached to other artifact - c.tagMgr.On("List").Return([]*tag.Tag{ - { - ID: 1, - RepositoryID: 1, - ArtifactID: 2, - Name: "latest", - }, - }, nil) - c.tagMgr.On("Update").Return(nil) - err = c.ctl.ensureTag(nil, 1, 1, "latest") - c.Require().Nil(err) - c.tagMgr.AssertExpectations(c.T()) - - // reset the mock - c.SetupTest() - - // the tag doesn't exist under the repository, create it - c.tagMgr.On("List").Return([]*tag.Tag{}, nil) - c.tagMgr.On("Create").Return(1, nil) - err = c.ctl.ensureTag(nil, 1, 1, "latest") - c.Require().Nil(err) - c.tagMgr.AssertExpectations(c.T()) -} - func (c *controllerTestSuite) TestEnsure() { digest := "sha256:418fb88ec412e340cdbef913b8ca1bbe8f9e8dc705f9617414c1f2c8db980180" @@ -258,14 +190,13 @@ func (c *controllerTestSuite) TestEnsure() { }, nil) c.artMgr.On("GetByDigest").Return(nil, ierror.NotFoundError(nil)) c.artMgr.On("Create").Return(1, nil) - c.tagMgr.On("List").Return([]*tag.Tag{}, nil) - c.tagMgr.On("Create").Return(1, nil) c.abstractor.On("AbstractMetadata").Return(nil) + c.tagCtl.On("Ensure").Return(nil) _, id, err := c.ctl.Ensure(nil, "library/hello-world", digest, "latest") c.Require().Nil(err) c.repoMgr.AssertExpectations(c.T()) c.artMgr.AssertExpectations(c.T()) - c.tagMgr.AssertExpectations(c.T()) + c.tagCtl.AssertExpectations(c.T()) c.abstractor.AssertExpectations(c.T()) c.Equal(int64(1), id) } @@ -288,12 +219,14 @@ func (c *controllerTestSuite) TestList() { RepositoryID: 1, }, }, nil) - c.tagMgr.On("List").Return([]*tag.Tag{ + c.tagCtl.On("List").Return([]*tag.Tag{ { - ID: 1, - RepositoryID: 1, - ArtifactID: 1, - Name: "latest", + Tag: model_tag.Tag{ + ID: 1, + RepositoryID: 1, + ArtifactID: 1, + Name: "latest", + }, }, }, nil) c.repoMgr.On("Get").Return(&models.RepoRecord{ @@ -357,7 +290,7 @@ func (c *controllerTestSuite) TestGetByTag() { c.repoMgr.On("GetByName").Return(&models.RepoRecord{ RepositoryID: 1, }, nil) - c.tagMgr.On("List").Return(nil, nil) + c.tagCtl.On("List").Return(nil, nil) art, err := c.ctl.getByTag(nil, "library/hello-world", "latest", nil) c.Require().NotNil(err) c.True(ierror.IsErr(err, ierror.NotFoundCode)) @@ -369,12 +302,14 @@ func (c *controllerTestSuite) TestGetByTag() { c.repoMgr.On("GetByName").Return(&models.RepoRecord{ RepositoryID: 1, }, nil) - c.tagMgr.On("List").Return([]*tag.Tag{ + c.tagCtl.On("List").Return([]*tag.Tag{ { - ID: 1, - RepositoryID: 1, - Name: "latest", - ArtifactID: 1, + Tag: model_tag.Tag{ + ID: 1, + RepositoryID: 1, + Name: "latest", + ArtifactID: 1, + }, }, }, nil) c.artMgr.On("Get").Return(&artifact.Artifact{ @@ -410,12 +345,14 @@ func (c *controllerTestSuite) TestGetByReference() { c.repoMgr.On("GetByName").Return(&models.RepoRecord{ RepositoryID: 1, }, nil) - c.tagMgr.On("List").Return([]*tag.Tag{ + c.tagCtl.On("List").Return([]*tag.Tag{ { - ID: 1, - RepositoryID: 1, - Name: "latest", - ArtifactID: 1, + Tag: model_tag.Tag{ + ID: 1, + RepositoryID: 1, + Name: "latest", + ArtifactID: 1, + }, }, }, nil) c.artMgr.On("Get").Return(&artifact.Artifact{ @@ -449,9 +386,11 @@ 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([]*tag.Tag{ + c.tagCtl.On("List").Return([]*tag.Tag{ { - ID: 1, + Tag: model_tag.Tag{ + ID: 1, + }, }, }, nil) c.repoMgr.On("Get").Return(&models.RepoRecord{}, nil) @@ -464,7 +403,7 @@ func (c *controllerTestSuite) TestDeleteDeeply() { // root artifact is referenced by other artifacts c.artMgr.On("Get").Return(&artifact.Artifact{ID: 1}, nil) - c.tagMgr.On("List").Return(nil, nil) + c.tagCtl.On("List").Return(nil, nil) c.repoMgr.On("Get").Return(&models.RepoRecord{}, nil) c.artMgr.On("ListReferences").Return([]*artifact.Reference{ { @@ -479,7 +418,7 @@ func (c *controllerTestSuite) TestDeleteDeeply() { // child artifact contains no tag but referenced by other artifacts c.artMgr.On("Get").Return(&artifact.Artifact{ID: 1}, nil) - c.tagMgr.On("List").Return(nil, nil) + c.tagCtl.On("List").Return(nil, nil) c.repoMgr.On("Get").Return(&models.RepoRecord{}, nil) c.artMgr.On("ListReferences").Return([]*artifact.Reference{ { @@ -494,15 +433,16 @@ func (c *controllerTestSuite) TestDeleteDeeply() { // root artifact is referenced by other artifacts c.artMgr.On("Get").Return(&artifact.Artifact{ID: 1}, nil) - c.tagMgr.On("List").Return(nil, nil) + c.tagCtl.On("List").Return(nil, nil) c.blobMgr.On("List", nil, mock.AnythingOfType("models.ListParams")).Return(nil, nil).Once() c.blobMgr.On("CleanupAssociationsForProject", nil, int64(0), mock.AnythingOfType("[]*models.Blob")).Return(nil).Once() c.repoMgr.On("Get").Return(&models.RepoRecord{}, nil) c.artMgr.On("ListReferences").Return(nil, nil) - c.tagMgr.On("DeleteOfArtifact").Return(nil) + c.tagCtl.On("DeleteOfArtifact").Return(nil) c.artMgr.On("Delete").Return(nil) c.labelMgr.On("RemoveAllFrom").Return(nil) c.artrashMgr.On("Create").Return(0, nil) + c.tagCtl.On("DeleteTags").Return(nil) err = c.ctl.deleteDeeply(nil, 1, true) c.Require().Nil(err) } @@ -517,13 +457,15 @@ func (c *controllerTestSuite) TestCopy() { Name: "library/hello-world", }, nil) c.artMgr.On("GetByDigest").Return(nil, ierror.NotFoundError(nil)) - c.tagMgr.On("List").Return([]*tag.Tag{ + c.tagCtl.On("List").Return([]*tag.Tag{ { - ID: 1, - Name: "latest", + Tag: model_tag.Tag{ + ID: 1, + Name: "latest", + }, }, }, nil) - c.tagMgr.On("Update").Return(nil) + c.tagCtl.On("Update").Return(nil) c.repoMgr.On("Get").Return(&models.RepoRecord{ RepositoryID: 1, Name: "library/hello-world", @@ -531,66 +473,73 @@ func (c *controllerTestSuite) TestCopy() { c.abstractor.On("AbstractMetadata").Return(nil) c.artMgr.On("Create").Return(1, nil) c.regCli.On("Copy").Return(nil) + c.tagCtl.On("Ensure").Return(nil) _, err := c.ctl.Copy(nil, "library/hello-world", "latest", "library/hello-world2") c.Require().Nil(err) } func (c *controllerTestSuite) TestListTags() { - c.tagMgr.On("List").Return([]*tag.Tag{ + c.tagCtl.On("List").Return([]*tag.Tag{ { - ID: 1, - RepositoryID: 1, - Name: "latest", - ArtifactID: 1, + Tag: model_tag.Tag{ + ID: 1, + RepositoryID: 1, + Name: "latest", + ArtifactID: 1, + }, }, }, nil) c.artMgr.On("Get").Return(&artifact.Artifact{}, nil) tags, err := c.ctl.ListTags(nil, nil, nil) c.Require().Nil(err) c.Len(tags, 1) - c.tagMgr.AssertExpectations(c.T()) + c.tagCtl.AssertExpectations(c.T()) c.Equal(tags[0].Immutable, false) // TODO check other properties: label, etc } func (c *controllerTestSuite) TestCreateTag() { - c.tagMgr.On("Create").Return(1, nil) - id, err := c.ctl.CreateTag(nil, &Tag{}) + c.tagCtl.On("Create").Return(1, nil) + id, err := c.ctl.CreateTag(nil, &tag.Tag{}) c.Require().Nil(err) c.Equal(int64(1), id) } func (c *controllerTestSuite) TestDeleteTag() { - c.tagMgr.On("Delete").Return(nil) + c.tagCtl.On("Delete").Return(nil) err := c.ctl.DeleteTag(nil, 1) c.Require().Nil(err) - c.tagMgr.AssertExpectations(c.T()) + c.tagCtl.AssertExpectations(c.T()) } func (c *controllerTestSuite) TestUpdatePullTime() { // artifact ID and tag ID matches - c.tagMgr.On("Get").Return(&tag.Tag{ - ID: 1, - ArtifactID: 1, + c.tagCtl.On("Get").Return(&tag.Tag{ + Tag: model_tag.Tag{ + ID: 1, + ArtifactID: 1, + }, }, nil) c.artMgr.On("UpdatePullTime").Return(nil) - c.tagMgr.On("Update").Return(nil) + c.tagCtl.On("Update").Return(nil) err := c.ctl.UpdatePullTime(nil, 1, 1, time.Now()) c.Require().Nil(err) c.artMgr.AssertExpectations(c.T()) - c.tagMgr.AssertExpectations(c.T()) + c.tagCtl.AssertExpectations(c.T()) // reset the mock c.SetupTest() // artifact ID and tag ID doesn't match - c.tagMgr.On("Get").Return(&tag.Tag{ - ID: 1, - ArtifactID: 2, + c.tagCtl.On("Get").Return(&tag.Tag{ + Tag: model_tag.Tag{ + ID: 1, + ArtifactID: 2, + }, }, nil) err = c.ctl.UpdatePullTime(nil, 1, 1, time.Now()) c.Require().NotNil(err) - c.tagMgr.AssertExpectations(c.T()) + c.tagCtl.AssertExpectations(c.T()) } diff --git a/src/api/artifact/model.go b/src/api/artifact/model.go index 2f2bee128..c91113dd3 100644 --- a/src/api/artifact/model.go +++ b/src/api/artifact/model.go @@ -17,17 +17,16 @@ package artifact import ( "fmt" + "github.com/goharbor/harbor/src/api/tag" cmodels "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/common/utils" "github.com/goharbor/harbor/src/pkg/artifact" - "github.com/goharbor/harbor/src/pkg/signature" - "github.com/goharbor/harbor/src/pkg/tag/model/tag" ) // Artifact is the overall view of artifact type Artifact struct { artifact.Artifact - Tags []*Tag `json:"tags"` // the list of tags that attached to the artifact + Tags []*tag.Tag `json:"tags"` // the list of tags that attached to the artifact AdditionLinks map[string]*AdditionLink `json:"addition_links"` // the resource link for build history(image), values.yaml(chart), dependency(chart), etc Labels []*cmodels.Label `json:"labels"` } @@ -44,13 +43,6 @@ func (artifact *Artifact) SetAdditionLink(addition, version string) { artifact.AdditionLinks[addition] = &AdditionLink{HREF: href, Absolute: false} } -// Tag is the overall view of tag -type Tag struct { - tag.Tag - Immutable bool `json:"immutable"` - Signed bool `json:"signed"` -} - // AdditionLink is a link via that the addition can be fetched type AdditionLink struct { HREF string `json:"href"` @@ -60,13 +52,6 @@ type AdditionLink struct { // Option is used to specify the properties returned when listing/getting artifacts type Option struct { WithTag bool - TagOption *TagOption // only works when WithTag is set to true + TagOption *tag.Option // only works when WithTag is set to true WithLabel bool } - -// TagOption is used to specify the properties returned when listing/getting tags -type TagOption struct { - WithImmutableStatus bool - WithSignature bool - SignatureChecker *signature.Checker -} diff --git a/src/api/tag/controller.go b/src/api/tag/controller.go new file mode 100644 index 000000000..9d5e243a7 --- /dev/null +++ b/src/api/tag/controller.go @@ -0,0 +1,242 @@ +package tag + +import ( + "context" + "github.com/goharbor/harbor/src/common/utils" + "github.com/goharbor/harbor/src/common/utils/log" + ierror "github.com/goharbor/harbor/src/internal/error" + "github.com/goharbor/harbor/src/pkg/art" + "github.com/goharbor/harbor/src/pkg/artifact" + "github.com/goharbor/harbor/src/pkg/immutabletag/match" + "github.com/goharbor/harbor/src/pkg/immutabletag/match/rule" + "github.com/goharbor/harbor/src/pkg/q" + "github.com/goharbor/harbor/src/pkg/signature" + "github.com/goharbor/harbor/src/pkg/tag" + model_tag "github.com/goharbor/harbor/src/pkg/tag/model/tag" + "time" +) + +var ( + // Ctl is a global tag controller instance + Ctl = NewController() +) + +// Controller manages the tags +type Controller interface { + // Ensure + Ensure(ctx context.Context, repositoryID, artifactID int64, name string) error + // Count returns the total count of tags according to the query. + Count(ctx context.Context, query *q.Query) (total int64, err error) + // List tags according to the query + List(ctx context.Context, query *q.Query, option *Option) (tags []*Tag, err error) + // Get the tag specified by ID + Get(ctx context.Context, id int64, option *Option) (tag *Tag, err error) + // Create the tag and returns the ID + Create(ctx context.Context, tag *Tag) (id int64, err error) + // Update the tag. Only the properties specified by "props" will be updated if it is set + Update(ctx context.Context, tag *Tag, props ...string) (err error) + // Delete the tag specified by ID with limitation check + Delete(ctx context.Context, id int64) (err error) + // DeleteTags deletes all tags + DeleteTags(ctx context.Context, tags []*Tag) (err error) +} + +// NewController creates an instance of the default repository controller +func NewController() Controller { + return &controller{ + tagMgr: tag.Mgr, + artMgr: artifact.Mgr, + immutableMtr: rule.NewRuleMatcher(), + } +} + +type controller struct { + tagMgr tag.Manager + artMgr artifact.Manager + immutableMtr match.ImmutableTagMatcher +} + +// Ensure ... +func (c *controller) Ensure(ctx context.Context, repositoryID, artifactID int64, name string) error { + query := &q.Query{ + Keywords: map[string]interface{}{ + "repository_id": repositoryID, + "name": name, + }, + } + tags, err := c.List(ctx, query, &Option{ + WithImmutableStatus: true, + WithSignature: true, + }) + if err != nil { + return err + } + // the tag already exists under the repository + if len(tags) > 0 { + tag := tags[0] + // existing tag must check the immutable status and signature + if tag.Immutable { + return ierror.New(nil).WithCode(ierror.PreconditionCode). + WithMessage("the tag %s configured as immutable, cannot be updated", tag.Name) + } + if tag.Signed { + return ierror.New(nil).WithCode(ierror.PreconditionCode). + WithMessage("the tag %s with signature cannot be updated", tag.Name) + } + // the tag already exists under the repository and is attached to the artifact, return directly + if tag.ArtifactID == artifactID { + return nil + } + // the tag exists under the repository, but it is attached to other artifact + // update it to point to the provided artifact + tag.ArtifactID = artifactID + tag.PushTime = time.Now() + return c.Update(ctx, tag, "ArtifactID", "PushTime") + } + // the tag doesn't exist under the repository, create it + tag := &Tag{} + tag.RepositoryID = repositoryID + tag.ArtifactID = artifactID + tag.Name = name + tag.PushTime = time.Now() + _, err = c.Create(ctx, tag) + // ignore the conflict error + if err != nil && ierror.IsConflictErr(err) { + return nil + } + return err +} + +// Count ... +func (c *controller) Count(ctx context.Context, query *q.Query) (total int64, err error) { + return c.tagMgr.Count(ctx, query) +} + +// List ... +func (c *controller) List(ctx context.Context, query *q.Query, option *Option) ([]*Tag, error) { + tgs, err := c.tagMgr.List(ctx, query) + if err != nil { + return nil, err + } + var tags []*Tag + for _, tg := range tgs { + tags = append(tags, c.assembleTag(ctx, tg, option)) + } + return tags, nil +} + +// Get ... +func (c *controller) Get(ctx context.Context, id int64, option *Option) (tag *Tag, err error) { + tag = &Tag{} + daoTag, err := c.tagMgr.Get(ctx, id) + if err != nil { + return nil, err + } + tag.Tag = *daoTag + + if option == nil { + return tag, nil + } + + if option.WithImmutableStatus { + c.populateImmutableStatus(ctx, tag) + } + + if option.WithSignature { + c.populateTagSignature(ctx, tag, option) + } + + return tag, nil +} + +// Create ... +func (c *controller) Create(ctx context.Context, tag *Tag) (id int64, err error) { + return c.tagMgr.Create(ctx, &(tag.Tag)) +} + +// Update ... +func (c *controller) Update(ctx context.Context, tag *Tag, props ...string) (err error) { + return c.tagMgr.Update(ctx, &tag.Tag, props...) +} + +// Delete needs to check the signature and immutable status +func (c *controller) Delete(ctx context.Context, id int64) (err error) { + option := &Option{ + WithImmutableStatus: true, + WithSignature: true, + } + tag, err := c.Get(ctx, id, option) + if err != nil { + return err + } + if tag.Immutable { + return ierror.New(nil).WithCode(ierror.PreconditionCode). + WithMessage("the tag %s configured as immutable, cannot be deleted", tag.Name) + } + if tag.Signed { + return ierror.New(nil).WithCode(ierror.PreconditionCode). + WithMessage("the tag %s with signature cannot be deleted", tag.Name) + } + return c.tagMgr.Delete(ctx, id) +} + +// DeleteTags ... +func (c *controller) DeleteTags(ctx context.Context, tags []*Tag) (err error) { + // in order to leverage the signature and immutable status check + for _, tag := range tags { + if err := c.Delete(ctx, tag.ID); err != nil { + return err + } + } + return nil +} + +// assemble several part into a single tag +func (c *controller) assembleTag(ctx context.Context, tag *model_tag.Tag, option *Option) *Tag { + t := &Tag{ + Tag: *tag, + } + if option == nil { + return t + } + if option.WithImmutableStatus { + c.populateImmutableStatus(ctx, t) + } + if option.WithSignature { + c.populateTagSignature(ctx, t, option) + } + return t +} + +func (c *controller) populateImmutableStatus(ctx context.Context, tag *Tag) { + artifact, err := c.artMgr.Get(ctx, tag.ArtifactID) + if err != nil { + return + } + _, repoName := utils.ParseRepository(artifact.RepositoryName) + matched, err := c.immutableMtr.Match(artifact.ProjectID, art.Candidate{ + Repository: repoName, + Tags: []string{tag.Name}, + NamespaceID: artifact.ProjectID, + }) + if err != nil { + return + } + tag.Immutable = matched +} + +func (c *controller) populateTagSignature(ctx context.Context, tag *Tag, option *Option) { + artifact, err := c.artMgr.Get(ctx, tag.ArtifactID) + if err != nil { + return + } + if option.SignatureChecker == nil { + chk, err := signature.GetManager().GetCheckerByRepo(ctx, artifact.RepositoryName) + if err != nil { + log.Error(err) + return + } + option.SignatureChecker = chk + } + tag.Signed = option.SignatureChecker.IsTagSigned(tag.Name, artifact.Digest) +} diff --git a/src/api/tag/controller_test.go b/src/api/tag/controller_test.go new file mode 100644 index 000000000..3c8255b32 --- /dev/null +++ b/src/api/tag/controller_test.go @@ -0,0 +1,226 @@ +package tag + +import ( + "github.com/goharbor/harbor/src/common" + coreConfig "github.com/goharbor/harbor/src/core/config" + ierror "github.com/goharbor/harbor/src/internal/error" + pkg_artifact "github.com/goharbor/harbor/src/pkg/artifact" + "github.com/goharbor/harbor/src/pkg/tag/model/tag" + "github.com/goharbor/harbor/src/testing/pkg/artifact" + immutesting "github.com/goharbor/harbor/src/testing/pkg/immutabletag" + "github.com/goharbor/harbor/src/testing/pkg/repository" + tagtesting "github.com/goharbor/harbor/src/testing/pkg/tag" + "github.com/stretchr/testify/suite" + "testing" + "time" +) + +type controllerTestSuite struct { + suite.Suite + ctl *controller + repoMgr *repository.FakeManager + artMgr *artifact.FakeManager + tagMgr *tagtesting.FakeManager + immutableMtr *immutesting.FakeMatcher +} + +func (c *controllerTestSuite) SetupTest() { + c.repoMgr = &repository.FakeManager{} + c.artMgr = &artifact.FakeManager{} + c.tagMgr = &tagtesting.FakeManager{} + c.immutableMtr = &immutesting.FakeMatcher{} + c.ctl = &controller{ + tagMgr: c.tagMgr, + artMgr: c.artMgr, + immutableMtr: c.immutableMtr, + } + + var tagCtlTestConfig = map[string]interface{}{ + common.WithNotary: false, + } + coreConfig.InitWithSettings(tagCtlTestConfig) +} + +func (c *controllerTestSuite) TestEnsureTag() { + // the tag already exists under the repository and is attached to the artifact + c.tagMgr.On("List").Return([]*tag.Tag{ + { + ID: 1, + RepositoryID: 1, + ArtifactID: 1, + Name: "latest", + }, + }, nil) + c.artMgr.On("Get").Return(&pkg_artifact.Artifact{ + ID: 1, + }, nil) + c.immutableMtr.On("Match").Return(false, nil) + err := c.ctl.Ensure(nil, 1, 1, "latest") + c.Require().Nil(err) + c.tagMgr.AssertExpectations(c.T()) + + // reset the mock + c.SetupTest() + + // the tag exists under the repository, but it is attached to other artifact + c.tagMgr.On("List").Return([]*tag.Tag{ + { + ID: 1, + RepositoryID: 1, + ArtifactID: 2, + Name: "latest", + }, + }, nil) + c.tagMgr.On("Update").Return(nil) + c.artMgr.On("Get").Return(&pkg_artifact.Artifact{ + ID: 1, + }, nil) + c.immutableMtr.On("Match").Return(false, nil) + err = c.ctl.Ensure(nil, 1, 1, "latest") + c.Require().Nil(err) + c.tagMgr.AssertExpectations(c.T()) + + // reset the mock + c.SetupTest() + + // the tag doesn't exist under the repository, create it + c.tagMgr.On("List").Return([]*tag.Tag{}, nil) + c.tagMgr.On("Create").Return(1, nil) + c.artMgr.On("Get").Return(&pkg_artifact.Artifact{ + ID: 1, + }, nil) + c.immutableMtr.On("Match").Return(false, nil) + err = c.ctl.Ensure(nil, 1, 1, "latest") + c.Require().Nil(err) + c.tagMgr.AssertExpectations(c.T()) +} + +func (c *controllerTestSuite) TestCount() { + c.tagMgr.On("Count").Return(1, nil) + total, err := c.ctl.Count(nil, nil) + c.Require().Nil(err) + c.Equal(int64(1), total) +} + +func (c *controllerTestSuite) TestList() { + c.tagMgr.On("List").Return([]*tag.Tag{ + { + RepositoryID: 1, + Name: "testlist", + }, + }, nil) + tags, err := c.ctl.List(nil, nil, nil) + c.Require().Nil(err) + c.Require().Len(tags, 1) + c.Equal(int64(1), tags[0].RepositoryID) + c.Equal("testlist", tags[0].Name) +} + +func (c *controllerTestSuite) TestGet() { + getTest := &tag.Tag{} + getTest.RepositoryID = 1 + getTest.Name = "testget" + + c.tagMgr.On("Get").Return(getTest, nil) + tag, err := c.ctl.Get(nil, 1, nil) + c.Require().Nil(err) + c.tagMgr.AssertExpectations(c.T()) + c.Equal(int64(1), tag.RepositoryID) + c.Equal(false, tag.Immutable) +} + +func (c *controllerTestSuite) TestDelete() { + c.tagMgr.On("Get").Return(&tag.Tag{ + RepositoryID: 1, + Name: "test", + }, nil) + c.artMgr.On("Get").Return(&pkg_artifact.Artifact{ + ID: 1, + }, nil) + c.immutableMtr.On("Match").Return(false, nil) + c.tagMgr.On("Delete").Return(nil) + err := c.ctl.Delete(nil, 1) + c.Require().Nil(err) +} + +func (c *controllerTestSuite) TestDeleteImmutable() { + c.tagMgr.On("Get").Return(&tag.Tag{ + RepositoryID: 1, + Name: "test", + }, nil) + c.artMgr.On("Get").Return(&pkg_artifact.Artifact{ + ID: 1, + }, nil) + c.immutableMtr.On("Match").Return(true, nil) + c.tagMgr.On("Delete").Return(nil) + err := c.ctl.Delete(nil, 1) + c.Require().NotNil(err) + c.True(ierror.IsErr(err, ierror.PreconditionCode)) +} + +func (c *controllerTestSuite) TestUpdate() { + c.tagMgr.On("Update").Return(nil) + err := c.ctl.Update(nil, &Tag{ + Tag: tag.Tag{ + RepositoryID: 1, + Name: "test", + }, + Immutable: true, + }, "ArtifactID") + c.Require().Nil(err) +} + +func (c *controllerTestSuite) TestDeleteTags() { + c.tagMgr.On("Get").Return(&tag.Tag{ + RepositoryID: 1, + }, nil) + c.artMgr.On("Get").Return(&pkg_artifact.Artifact{ + ID: 1, + }, nil) + c.immutableMtr.On("Match").Return(false, nil) + c.tagMgr.On("Delete").Return(nil) + tags := []*Tag{ + { + Tag: tag.Tag{ + RepositoryID: 10, + Name: "test2", + }, + Signed: true, + }, + } + err := c.ctl.DeleteTags(nil, tags) + c.Require().Nil(err) +} + +func (c *controllerTestSuite) TestAssembleTag() { + art := &pkg_artifact.Artifact{ + ID: 1, + ProjectID: 1, + RepositoryID: 1, + RepositoryName: "library/hello-world", + Digest: "sha256:418fb88ec412e340cdbef913b8ca1bbe8f9e8dc705f9617414c1f2c8db980180", + } + tg := &tag.Tag{ + ID: 1, + RepositoryID: 1, + ArtifactID: 1, + Name: "latest", + PushTime: time.Now(), + PullTime: time.Now(), + } + option := &Option{ + WithImmutableStatus: true, + } + + c.artMgr.On("Get").Return(art, nil) + c.immutableMtr.On("Match").Return(true, nil) + tag := c.ctl.assembleTag(nil, tg, option) + c.Require().NotNil(tag) + c.Equal(tag.ID, tg.ID) + c.Equal(true, tag.Immutable) + // TODO check signature +} + +func TestControllerTestSuite(t *testing.T) { + suite.Run(t, &controllerTestSuite{}) +} diff --git a/src/api/tag/model.go b/src/api/tag/model.go new file mode 100644 index 000000000..cc541f369 --- /dev/null +++ b/src/api/tag/model.go @@ -0,0 +1,20 @@ +package tag + +import ( + "github.com/goharbor/harbor/src/pkg/signature" + "github.com/goharbor/harbor/src/pkg/tag/model/tag" +) + +// Tag is the overall view of tag +type Tag struct { + tag.Tag + Immutable bool `json:"immutable"` + Signed bool `json:"signed"` +} + +// Option is used to specify the properties returned when listing/getting tags +type Option struct { + WithImmutableStatus bool + WithSignature bool + SignatureChecker *signature.Checker +} diff --git a/src/pkg/retention/dep/client_test.go b/src/pkg/retention/dep/client_test.go index 6e4635a77..095bad87a 100644 --- a/src/pkg/retention/dep/client_test.go +++ b/src/pkg/retention/dep/client_test.go @@ -16,7 +16,8 @@ package dep import ( modelsv2 "github.com/goharbor/harbor/src/api/artifact" - "github.com/goharbor/harbor/src/pkg/tag/model/tag" + "github.com/goharbor/harbor/src/api/tag" + model_tag "github.com/goharbor/harbor/src/pkg/tag/model/tag" "testing" "github.com/goharbor/harbor/src/chartserver" @@ -38,9 +39,9 @@ type fakeCoreClient struct { func (f *fakeCoreClient) ListAllArtifacts(project, repository string) ([]*modelsv2.Artifact, error) { image := &modelsv2.Artifact{} image.Digest = "sha256:123456" - image.Tags = []*modelsv2.Tag{ + image.Tags = []*tag.Tag{ { - Tag: tag.Tag{ + Tag: model_tag.Tag{ Name: "latest", }, }, diff --git a/src/server/middleware/immutable/deletemf.go b/src/server/middleware/immutable/deletemf.go index 04a99ec0e..7e6b4bc3d 100644 --- a/src/server/middleware/immutable/deletemf.go +++ b/src/server/middleware/immutable/deletemf.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "github.com/goharbor/harbor/src/api/artifact" + "github.com/goharbor/harbor/src/api/tag" common_util "github.com/goharbor/harbor/src/common/utils" internal_errors "github.com/goharbor/harbor/src/internal/error" serror "github.com/goharbor/harbor/src/server/error" @@ -40,7 +41,7 @@ func handleDelete(req *http.Request) error { af, err := artifact.Ctl.GetByReference(req.Context(), art.Repository, art.Digest, &artifact.Option{ WithTag: true, - TagOption: &artifact.TagOption{WithImmutableStatus: true}, + TagOption: &tag.Option{WithImmutableStatus: true}, }) if err != nil { if internal_errors.IsErr(err, internal_errors.NotFoundCode) { diff --git a/src/server/middleware/immutable/pushmf.go b/src/server/middleware/immutable/pushmf.go index e7e35b93e..0b1ea14d5 100644 --- a/src/server/middleware/immutable/pushmf.go +++ b/src/server/middleware/immutable/pushmf.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "github.com/goharbor/harbor/src/api/artifact" + "github.com/goharbor/harbor/src/api/tag" common_util "github.com/goharbor/harbor/src/common/utils" internal_errors "github.com/goharbor/harbor/src/internal/error" serror "github.com/goharbor/harbor/src/server/error" @@ -42,7 +43,7 @@ func handlePush(req *http.Request) error { af, err := artifact.Ctl.GetByReference(req.Context(), art.Repository, art.Tag, &artifact.Option{ WithTag: true, - TagOption: &artifact.TagOption{WithImmutableStatus: true}, + TagOption: &tag.Option{WithImmutableStatus: true}, }) if err != nil { if internal_errors.IsErr(err, internal_errors.NotFoundCode) { diff --git a/src/server/registry/tag_test.go b/src/server/registry/tag_test.go index b1e8fcef9..8cdf921a3 100644 --- a/src/server/registry/tag_test.go +++ b/src/server/registry/tag_test.go @@ -18,8 +18,9 @@ import ( "encoding/json" "github.com/goharbor/harbor/src/api/artifact" "github.com/goharbor/harbor/src/api/repository" + "github.com/goharbor/harbor/src/api/tag" "github.com/goharbor/harbor/src/common/models" - "github.com/goharbor/harbor/src/pkg/tag/model/tag" + model_tag "github.com/goharbor/harbor/src/pkg/tag/model/tag" arttesting "github.com/goharbor/harbor/src/testing/api/artifact" repotesting "github.com/goharbor/harbor/src/testing/api/repository" "github.com/stretchr/testify/suite" @@ -64,15 +65,15 @@ func (c *tagTestSuite) TestListTag() { RepositoryID: 1, Name: "library/hello-world", }, nil) - c.artCtl.On("ListTags").Return([]*artifact.Tag{ + c.artCtl.On("ListTags").Return([]*tag.Tag{ { - Tag: tag.Tag{ + Tag: model_tag.Tag{ RepositoryID: 1, Name: "v1", }, }, { - Tag: tag.Tag{ + Tag: model_tag.Tag{ RepositoryID: 1, Name: "v2", }, @@ -99,15 +100,15 @@ func (c *tagTestSuite) TestListTagPagination1() { RepositoryID: 1, Name: "hello-world", }, nil) - c.artCtl.On("ListTags").Return([]*artifact.Tag{ + c.artCtl.On("ListTags").Return([]*tag.Tag{ { - Tag: tag.Tag{ + Tag: model_tag.Tag{ RepositoryID: 1, Name: "v1", }, }, { - Tag: tag.Tag{ + Tag: model_tag.Tag{ RepositoryID: 1, Name: "v2", }, @@ -135,15 +136,15 @@ func (c *tagTestSuite) TestListTagPagination2() { RepositoryID: 1, Name: "hello-world", }, nil) - c.artCtl.On("ListTags").Return([]*artifact.Tag{ + c.artCtl.On("ListTags").Return([]*tag.Tag{ { - Tag: tag.Tag{ + Tag: model_tag.Tag{ RepositoryID: 1, Name: "v1", }, }, { - Tag: tag.Tag{ + Tag: model_tag.Tag{ RepositoryID: 1, Name: "v2", }, @@ -171,15 +172,15 @@ func (c *tagTestSuite) TestListTagPagination3() { RepositoryID: 1, Name: "hello-world", }, nil) - c.artCtl.On("ListTags").Return([]*artifact.Tag{ + c.artCtl.On("ListTags").Return([]*tag.Tag{ { - Tag: tag.Tag{ + Tag: model_tag.Tag{ RepositoryID: 1, Name: "v1", }, }, { - Tag: tag.Tag{ + Tag: model_tag.Tag{ RepositoryID: 1, Name: "v2", }, diff --git a/src/server/v2.0/handler/artifact.go b/src/server/v2.0/handler/artifact.go index 6a8596a45..e488284f2 100644 --- a/src/server/v2.0/handler/artifact.go +++ b/src/server/v2.0/handler/artifact.go @@ -28,6 +28,7 @@ import ( "github.com/goharbor/harbor/src/api/artifact/abstractor/resolver" "github.com/goharbor/harbor/src/api/repository" "github.com/goharbor/harbor/src/api/scan" + "github.com/goharbor/harbor/src/api/tag" "github.com/goharbor/harbor/src/common/rbac" "github.com/goharbor/harbor/src/common/utils" ierror "github.com/goharbor/harbor/src/internal/error" @@ -232,7 +233,7 @@ func (a *artifactAPI) CreateTag(ctx context.Context, params operation.CreateTagP if err != nil { return a.SendError(ctx, err) } - tag := &artifact.Tag{} + tag := &tag.Tag{} tag.RepositoryID = art.RepositoryID tag.ArtifactID = art.ID tag.Name = params.Tag.Name @@ -340,7 +341,7 @@ func option(withTag, withImmutableStatus, withLabel, withSignature *bool) *artif } if option.WithTag { - option.TagOption = &artifact.TagOption{ + option.TagOption = &tag.Option{ WithImmutableStatus: boolValue(withImmutableStatus), WithSignature: boolValue(withSignature), } diff --git a/src/testing/api/artifact/controller.go b/src/testing/api/artifact/controller.go index 22ebc1698..97fa1f698 100644 --- a/src/testing/api/artifact/controller.go +++ b/src/testing/api/artifact/controller.go @@ -18,6 +18,7 @@ import ( "context" "github.com/goharbor/harbor/src/api/artifact" "github.com/goharbor/harbor/src/api/artifact/abstractor/resolver" + "github.com/goharbor/harbor/src/api/tag" "github.com/goharbor/harbor/src/pkg/q" "github.com/stretchr/testify/mock" "time" @@ -83,17 +84,17 @@ func (f *FakeController) Copy(ctx context.Context, srcRepo, ref, dstRepo string) } // ListTags ... -func (f *FakeController) ListTags(ctx context.Context, query *q.Query, option *artifact.TagOption) ([]*artifact.Tag, error) { +func (f *FakeController) ListTags(ctx context.Context, query *q.Query, option *tag.Option) ([]*tag.Tag, error) { args := f.Called() - var tags []*artifact.Tag + var tags []*tag.Tag if args.Get(0) != nil { - tags = args.Get(0).([]*artifact.Tag) + tags = args.Get(0).([]*tag.Tag) } return tags, args.Error(1) } // CreateTag ... -func (f *FakeController) CreateTag(ctx context.Context, tag *artifact.Tag) (int64, error) { +func (f *FakeController) CreateTag(ctx context.Context, tag *tag.Tag) (int64, error) { args := f.Called() return int64(args.Int(0)), args.Error(1) } diff --git a/src/testing/api/tag/controller.go b/src/testing/api/tag/controller.go new file mode 100644 index 000000000..c68ab7e2e --- /dev/null +++ b/src/testing/api/tag/controller.go @@ -0,0 +1,69 @@ +package tag + +import ( + "context" + "github.com/goharbor/harbor/src/api/tag" + "github.com/goharbor/harbor/src/pkg/q" + "github.com/stretchr/testify/mock" +) + +// FakeController is a fake artifact controller that implement src/api/tag.Controller interface +type FakeController struct { + mock.Mock +} + +// Ensure ... +func (f *FakeController) Ensure(ctx context.Context, repositoryID, artifactID int64, name string) error { + args := f.Called() + return args.Error(0) +} + +// Count ... +func (f *FakeController) Count(ctx context.Context, query *q.Query) (total int64, err error) { + args := f.Called() + return int64(args.Int(0)), args.Error(1) +} + +// List ... +func (f *FakeController) List(ctx context.Context, query *q.Query, option *tag.Option) ([]*tag.Tag, error) { + args := f.Called() + var tags []*tag.Tag + if args.Get(0) != nil { + tags = args.Get(0).([]*tag.Tag) + } + return tags, args.Error(1) +} + +// Get ... +func (f *FakeController) Get(ctx context.Context, id int64, option *tag.Option) (*tag.Tag, error) { + args := f.Called() + var tg *tag.Tag + if args.Get(0) != nil { + tg = args.Get(0).(*tag.Tag) + } + return tg, args.Error(1) +} + +// Create ... +func (f *FakeController) Create(ctx context.Context, tag *tag.Tag) (id int64, err error) { + args := f.Called() + return int64(args.Int(0)), args.Error(1) +} + +// Update ... +func (f *FakeController) Update(ctx context.Context, tag *tag.Tag, props ...string) (err error) { + args := f.Called() + return args.Error(0) +} + +// Delete ... +func (f *FakeController) Delete(ctx context.Context, id int64) (err error) { + args := f.Called() + return args.Error(0) +} + +// DeleteTags ... +func (f *FakeController) DeleteTags(ctx context.Context, tags []*tag.Tag) (err error) { + args := f.Called() + return args.Error(0) +} diff --git a/tests/apitests/python/test_sign_image.py b/tests/apitests/python/test_sign_image.py index 879722e01..2186c7235 100644 --- a/tests/apitests/python/test_sign_image.py +++ b/tests/apitests/python/test_sign_image.py @@ -25,11 +25,12 @@ class TestProjects(unittest.TestCase): @unittest.skipIf(TEARDOWN == False, "Test data won't be erased.") def test_ClearData(self): + # remove the deletion as the signed image cannot be deleted. #1. Delete repository(RA) by user(UA); - self.repo.delete_repoitory(TestProjects.project_sign_image_name, TestProjects.repo_name.split('/')[1], **TestProjects.USER_sign_image_CLIENT) + #self.repo.delete_repoitory(TestProjects.project_sign_image_name, TestProjects.repo_name.split('/')[1], **TestProjects.USER_sign_image_CLIENT) #2. Delete project(PA); - self.project.delete_project(TestProjects.project_sign_image_id, **TestProjects.USER_sign_image_CLIENT) + #self.project.delete_project(TestProjects.project_sign_image_id, **TestProjects.USER_sign_image_CLIENT) #3. Delete user(UA); self.user.delete_user(TestProjects.user_sign_image_id, **ADMIN_CLIENT) From 2d4fc0c4daa627f56ee52a45180dcbc6f8b8b08d Mon Sep 17 00:00:00 2001 From: wang yan Date: Fri, 28 Feb 2020 15:20:47 +0800 Subject: [PATCH 2/2] move out the tags methods of artifact ctl 1, move the tag methods out of artifact ctl, let api to call tag ctr 2, update the ensure sequence for existing tag Signed-off-by: wang yan --- src/api/artifact/controller.go | 29 ++++------------------ src/api/artifact/controller_test.go | 34 -------------------------- src/api/tag/controller.go | 31 +++++++---------------- src/api/tag/controller_test.go | 12 ++------- src/server/registry/tag.go | 8 +++--- src/server/registry/tag_test.go | 23 +++++++++-------- src/server/v2.0/handler/artifact.go | 6 +++-- src/testing/api/artifact/controller.go | 23 ----------------- src/testing/api/tag/controller.go | 2 +- 9 files changed, 36 insertions(+), 132 deletions(-) diff --git a/src/api/artifact/controller.go b/src/api/artifact/controller.go index 7cb3ff07c..c9f9bb7a6 100644 --- a/src/api/artifact/controller.go +++ b/src/api/artifact/controller.go @@ -73,12 +73,6 @@ type Controller interface { Delete(ctx context.Context, id int64) (err error) // Copy the artifact specified by "srcRepo" and "reference" into the repository specified by "dstRepo" Copy(ctx context.Context, srcRepo, reference, dstRepo string) (id int64, err error) - // ListTags lists the tags according to the query, specify the properties returned with option - ListTags(ctx context.Context, query *q.Query, option *tag.Option) (tags []*tag.Tag, err error) - // CreateTag creates a tag - CreateTag(ctx context.Context, tag *tag.Tag) (id int64, err error) - // DeleteTag deletes the tag specified by tagID - DeleteTag(ctx context.Context, tagID int64) (err error) // UpdatePullTime updates the pull time for the artifact. If the tagID is provides, update the pull // time of the tag as well UpdatePullTime(ctx context.Context, artifactID int64, tagID int64, time time.Time) (err error) @@ -300,7 +294,11 @@ func (c *controller) deleteDeeply(ctx context.Context, id int64, isRoot bool) er // delete all tags that attached to the root artifact if isRoot { - if err = c.tagCtl.DeleteTags(ctx, art.Tags); err != nil { + var ids []int64 + for _, tag := range art.Tags { + ids = append(ids, tag.ID) + } + if err = c.tagCtl.DeleteTags(ctx, ids); err != nil { return err } } @@ -407,23 +405,6 @@ func (c *controller) copyDeeply(ctx context.Context, srcRepo, reference, dstRepo return id, nil } -func (c *controller) CreateTag(ctx context.Context, tag *tag.Tag) (int64, error) { - // TODO fire event - return c.tagCtl.Create(ctx, tag) -} -func (c *controller) ListTags(ctx context.Context, query *q.Query, option *tag.Option) ([]*tag.Tag, error) { - tags, err := c.tagCtl.List(ctx, query, option) - if err != nil { - return nil, err - } - return tags, nil -} - -func (c *controller) DeleteTag(ctx context.Context, tagID int64) error { - // TODO fire delete tag event - return c.tagCtl.Delete(ctx, tagID) -} - func (c *controller) UpdatePullTime(ctx context.Context, artifactID int64, tagID int64, time time.Time) error { tag, err := c.tagCtl.Get(ctx, tagID, nil) if err != nil { diff --git a/src/api/artifact/controller_test.go b/src/api/artifact/controller_test.go index 88b6d53b1..e8e3daca0 100644 --- a/src/api/artifact/controller_test.go +++ b/src/api/artifact/controller_test.go @@ -478,40 +478,6 @@ func (c *controllerTestSuite) TestCopy() { c.Require().Nil(err) } -func (c *controllerTestSuite) TestListTags() { - c.tagCtl.On("List").Return([]*tag.Tag{ - { - Tag: model_tag.Tag{ - ID: 1, - RepositoryID: 1, - Name: "latest", - ArtifactID: 1, - }, - }, - }, nil) - c.artMgr.On("Get").Return(&artifact.Artifact{}, nil) - tags, err := c.ctl.ListTags(nil, nil, nil) - c.Require().Nil(err) - c.Len(tags, 1) - c.tagCtl.AssertExpectations(c.T()) - c.Equal(tags[0].Immutable, false) - // TODO check other properties: label, etc -} - -func (c *controllerTestSuite) TestCreateTag() { - c.tagCtl.On("Create").Return(1, nil) - id, err := c.ctl.CreateTag(nil, &tag.Tag{}) - c.Require().Nil(err) - c.Equal(int64(1), id) -} - -func (c *controllerTestSuite) TestDeleteTag() { - c.tagCtl.On("Delete").Return(nil) - err := c.ctl.DeleteTag(nil, 1) - c.Require().Nil(err) - c.tagCtl.AssertExpectations(c.T()) -} - func (c *controllerTestSuite) TestUpdatePullTime() { // artifact ID and tag ID matches c.tagCtl.On("Get").Return(&tag.Tag{ diff --git a/src/api/tag/controller.go b/src/api/tag/controller.go index 9d5e243a7..4733043a1 100644 --- a/src/api/tag/controller.go +++ b/src/api/tag/controller.go @@ -38,7 +38,7 @@ type Controller interface { // Delete the tag specified by ID with limitation check Delete(ctx context.Context, id int64) (err error) // DeleteTags deletes all tags - DeleteTags(ctx context.Context, tags []*Tag) (err error) + DeleteTags(ctx context.Context, ids []int64) (err error) } // NewController creates an instance of the default repository controller @@ -66,7 +66,6 @@ func (c *controller) Ensure(ctx context.Context, repositoryID, artifactID int64, } tags, err := c.List(ctx, query, &Option{ WithImmutableStatus: true, - WithSignature: true, }) if err != nil { return err @@ -74,19 +73,15 @@ func (c *controller) Ensure(ctx context.Context, repositoryID, artifactID int64, // the tag already exists under the repository if len(tags) > 0 { tag := tags[0] + // the tag already exists under the repository and is attached to the artifact, return directly + if tag.ArtifactID == artifactID { + return nil + } // existing tag must check the immutable status and signature if tag.Immutable { return ierror.New(nil).WithCode(ierror.PreconditionCode). WithMessage("the tag %s configured as immutable, cannot be updated", tag.Name) } - if tag.Signed { - return ierror.New(nil).WithCode(ierror.PreconditionCode). - WithMessage("the tag %s with signature cannot be updated", tag.Name) - } - // the tag already exists under the repository and is attached to the artifact, return directly - if tag.ArtifactID == artifactID { - return nil - } // the tag exists under the repository, but it is attached to other artifact // update it to point to the provided artifact tag.ArtifactID = artifactID @@ -138,15 +133,7 @@ func (c *controller) Get(ctx context.Context, id int64, option *Option) (tag *Ta return tag, nil } - if option.WithImmutableStatus { - c.populateImmutableStatus(ctx, tag) - } - - if option.WithSignature { - c.populateTagSignature(ctx, tag, option) - } - - return tag, nil + return c.assembleTag(ctx, &tag.Tag, option), nil } // Create ... @@ -181,10 +168,10 @@ func (c *controller) Delete(ctx context.Context, id int64) (err error) { } // DeleteTags ... -func (c *controller) DeleteTags(ctx context.Context, tags []*Tag) (err error) { +func (c *controller) DeleteTags(ctx context.Context, ids []int64) (err error) { // in order to leverage the signature and immutable status check - for _, tag := range tags { - if err := c.Delete(ctx, tag.ID); err != nil { + for _, id := range ids { + if err := c.Delete(ctx, id); err != nil { return err } } diff --git a/src/api/tag/controller_test.go b/src/api/tag/controller_test.go index 3c8255b32..ea987a1d9 100644 --- a/src/api/tag/controller_test.go +++ b/src/api/tag/controller_test.go @@ -179,16 +179,8 @@ func (c *controllerTestSuite) TestDeleteTags() { }, nil) c.immutableMtr.On("Match").Return(false, nil) c.tagMgr.On("Delete").Return(nil) - tags := []*Tag{ - { - Tag: tag.Tag{ - RepositoryID: 10, - Name: "test2", - }, - Signed: true, - }, - } - err := c.ctl.DeleteTags(nil, tags) + ids := []int64{1, 2, 3, 4} + err := c.ctl.DeleteTags(nil, ids) c.Require().Nil(err) } diff --git a/src/server/registry/tag.go b/src/server/registry/tag.go index 7be46a1b1..7285f0a04 100644 --- a/src/server/registry/tag.go +++ b/src/server/registry/tag.go @@ -17,8 +17,8 @@ package registry import ( "encoding/json" "fmt" - "github.com/goharbor/harbor/src/api/artifact" "github.com/goharbor/harbor/src/api/repository" + "github.com/goharbor/harbor/src/api/tag" ierror "github.com/goharbor/harbor/src/internal/error" "github.com/goharbor/harbor/src/pkg/q" serror "github.com/goharbor/harbor/src/server/error" @@ -32,13 +32,13 @@ import ( func newTagHandler() http.Handler { return &tagHandler{ repoCtl: repository.Ctl, - artCtl: artifact.Ctl, + tagCtl: tag.Ctl, } } type tagHandler struct { repoCtl repository.Controller - artCtl artifact.Controller + tagCtl tag.Controller repositoryName string } @@ -80,7 +80,7 @@ func (t *tagHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { } // get tags ... - tags, err := t.artCtl.ListTags(req.Context(), &q.Query{ + tags, err := t.tagCtl.List(req.Context(), &q.Query{ Keywords: map[string]interface{}{ "RepositoryID": repository.RepositoryID, }}, nil) diff --git a/src/server/registry/tag_test.go b/src/server/registry/tag_test.go index 8cdf921a3..deaa17fc3 100644 --- a/src/server/registry/tag_test.go +++ b/src/server/registry/tag_test.go @@ -16,13 +16,12 @@ package registry import ( "encoding/json" - "github.com/goharbor/harbor/src/api/artifact" "github.com/goharbor/harbor/src/api/repository" "github.com/goharbor/harbor/src/api/tag" "github.com/goharbor/harbor/src/common/models" model_tag "github.com/goharbor/harbor/src/pkg/tag/model/tag" - arttesting "github.com/goharbor/harbor/src/testing/api/artifact" repotesting "github.com/goharbor/harbor/src/testing/api/repository" + tagtesting "github.com/goharbor/harbor/src/testing/api/tag" "github.com/stretchr/testify/suite" "net/http" "net/http/httptest" @@ -33,20 +32,20 @@ type tagTestSuite struct { suite.Suite originalRepoCtl repository.Controller repoCtl *repotesting.FakeController - originalArtCtl artifact.Controller - artCtl *arttesting.FakeController + originalTagCtl tag.Controller + tagCtl *tagtesting.FakeController } func (c *tagTestSuite) SetupSuite() { - c.originalArtCtl = artifact.Ctl c.originalRepoCtl = repository.Ctl + c.originalTagCtl = tag.Ctl } func (c *tagTestSuite) SetupTest() { - c.artCtl = &arttesting.FakeController{} - artifact.Ctl = c.artCtl c.repoCtl = &repotesting.FakeController{} repository.Ctl = c.repoCtl + c.tagCtl = &tagtesting.FakeController{} + tag.Ctl = c.tagCtl } func (c *tagTestSuite) TearDownTest() { @@ -54,7 +53,7 @@ func (c *tagTestSuite) TearDownTest() { func (c *tagTestSuite) TearDownSuite() { repository.Ctl = c.originalRepoCtl - artifact.Ctl = c.originalArtCtl + tag.Ctl = c.originalTagCtl } func (c *tagTestSuite) TestListTag() { @@ -65,7 +64,7 @@ func (c *tagTestSuite) TestListTag() { RepositoryID: 1, Name: "library/hello-world", }, nil) - c.artCtl.On("ListTags").Return([]*tag.Tag{ + c.tagCtl.On("List").Return([]*tag.Tag{ { Tag: model_tag.Tag{ RepositoryID: 1, @@ -100,7 +99,7 @@ func (c *tagTestSuite) TestListTagPagination1() { RepositoryID: 1, Name: "hello-world", }, nil) - c.artCtl.On("ListTags").Return([]*tag.Tag{ + c.tagCtl.On("List").Return([]*tag.Tag{ { Tag: model_tag.Tag{ RepositoryID: 1, @@ -136,7 +135,7 @@ func (c *tagTestSuite) TestListTagPagination2() { RepositoryID: 1, Name: "hello-world", }, nil) - c.artCtl.On("ListTags").Return([]*tag.Tag{ + c.tagCtl.On("List").Return([]*tag.Tag{ { Tag: model_tag.Tag{ RepositoryID: 1, @@ -172,7 +171,7 @@ func (c *tagTestSuite) TestListTagPagination3() { RepositoryID: 1, Name: "hello-world", }, nil) - c.artCtl.On("ListTags").Return([]*tag.Tag{ + c.tagCtl.On("List").Return([]*tag.Tag{ { Tag: model_tag.Tag{ RepositoryID: 1, diff --git a/src/server/v2.0/handler/artifact.go b/src/server/v2.0/handler/artifact.go index e488284f2..93d4cbdef 100644 --- a/src/server/v2.0/handler/artifact.go +++ b/src/server/v2.0/handler/artifact.go @@ -51,6 +51,7 @@ func newArtifactAPI() *artifactAPI { proMgr: project.Mgr, repoCtl: repository.Ctl, scanCtl: scan.DefaultController, + tagCtl: tag.Ctl, } } @@ -60,6 +61,7 @@ type artifactAPI struct { proMgr project.Manager repoCtl repository.Controller scanCtl scan.Controller + tagCtl tag.Controller } func (a *artifactAPI) ListArtifacts(ctx context.Context, params operation.ListArtifactsParams) middleware.Responder { @@ -238,7 +240,7 @@ func (a *artifactAPI) CreateTag(ctx context.Context, params operation.CreateTagP tag.ArtifactID = art.ID tag.Name = params.Tag.Name tag.PushTime = time.Now() - if _, err = a.artCtl.CreateTag(ctx, tag); err != nil { + if _, err = a.tagCtl.Create(ctx, tag); err != nil { return a.SendError(ctx, err) } // TODO set location header? @@ -269,7 +271,7 @@ func (a *artifactAPI) DeleteTag(ctx context.Context, params operation.DeleteTagP "tag %s attached to artifact %d not found", params.TagName, artifact.ID) return a.SendError(ctx, err) } - if err = a.artCtl.DeleteTag(ctx, id); err != nil { + if err = a.tagCtl.Delete(ctx, id); err != nil { return a.SendError(ctx, err) } return operation.NewDeleteTagOK() diff --git a/src/testing/api/artifact/controller.go b/src/testing/api/artifact/controller.go index 97fa1f698..a7e390d3c 100644 --- a/src/testing/api/artifact/controller.go +++ b/src/testing/api/artifact/controller.go @@ -18,7 +18,6 @@ import ( "context" "github.com/goharbor/harbor/src/api/artifact" "github.com/goharbor/harbor/src/api/artifact/abstractor/resolver" - "github.com/goharbor/harbor/src/api/tag" "github.com/goharbor/harbor/src/pkg/q" "github.com/stretchr/testify/mock" "time" @@ -83,28 +82,6 @@ func (f *FakeController) Copy(ctx context.Context, srcRepo, ref, dstRepo string) return int64(args.Int(0)), args.Error(1) } -// ListTags ... -func (f *FakeController) ListTags(ctx context.Context, query *q.Query, option *tag.Option) ([]*tag.Tag, error) { - args := f.Called() - var tags []*tag.Tag - if args.Get(0) != nil { - tags = args.Get(0).([]*tag.Tag) - } - return tags, args.Error(1) -} - -// CreateTag ... -func (f *FakeController) CreateTag(ctx context.Context, tag *tag.Tag) (int64, error) { - args := f.Called() - return int64(args.Int(0)), args.Error(1) -} - -// DeleteTag ... -func (f *FakeController) DeleteTag(ctx context.Context, tagID int64) (err error) { - args := f.Called() - return args.Error(0) -} - // UpdatePullTime ... func (f *FakeController) UpdatePullTime(ctx context.Context, artifactID int64, tagID int64, time time.Time) error { args := f.Called() diff --git a/src/testing/api/tag/controller.go b/src/testing/api/tag/controller.go index c68ab7e2e..d74afeee2 100644 --- a/src/testing/api/tag/controller.go +++ b/src/testing/api/tag/controller.go @@ -63,7 +63,7 @@ func (f *FakeController) Delete(ctx context.Context, id int64) (err error) { } // DeleteTags ... -func (f *FakeController) DeleteTags(ctx context.Context, tags []*tag.Tag) (err error) { +func (f *FakeController) DeleteTags(ctx context.Context, ids []int64) (err error) { args := f.Called() return args.Error(0) }