diff --git a/src/api/artifact/controller.go b/src/api/artifact/controller.go index feb41ce38..c9f9bb7a6 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,16 +69,10 @@ 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) - // CreateTag creates a tag - CreateTag(ctx context.Context, 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) @@ -98,11 +89,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 +105,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 +123,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 +180,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 +227,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 +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.tagMgr.DeleteOfArtifact(ctx, id); 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 } } @@ -448,35 +405,8 @@ func (c *controller) copyDeeply(ctx context.Context, srcRepo, reference, dstRepo return id, nil } -func (c *controller) CreateTag(ctx context.Context, tag *Tag) (int64, error) { - // TODO fire event - return c.tagMgr.Create(ctx, &(tag.Tag)) -} -func (c *controller) ListTags(ctx context.Context, query *q.Query, option *TagOption) ([]*Tag, error) { - tgs, err := c.tagMgr.List(ctx, query) - 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) -} - 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 +416,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 +455,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..e8e3daca0 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,39 @@ 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{ - { - 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.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.Require().Nil(err) - c.Equal(int64(1), id) -} - -func (c *controllerTestSuite) TestDeleteTag() { - c.tagMgr.On("Delete").Return(nil) - err := c.ctl.DeleteTag(nil, 1) - c.Require().Nil(err) - c.tagMgr.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..4733043a1 --- /dev/null +++ b/src/api/tag/controller.go @@ -0,0 +1,229 @@ +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, ids []int64) (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, + }) + 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 + } + // 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) + } + // 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 + } + + return c.assembleTag(ctx, &tag.Tag, option), 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, ids []int64) (err error) { + // in order to leverage the signature and immutable status check + for _, id := range ids { + if err := c.Delete(ctx, 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..ea987a1d9 --- /dev/null +++ b/src/api/tag/controller_test.go @@ -0,0 +1,218 @@ +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) + ids := []int64{1, 2, 3, 4} + err := c.ctl.DeleteTags(nil, ids) + 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.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 b1e8fcef9..deaa17fc3 100644 --- a/src/server/registry/tag_test.go +++ b/src/server/registry/tag_test.go @@ -16,12 +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" - "github.com/goharbor/harbor/src/pkg/tag/model/tag" - arttesting "github.com/goharbor/harbor/src/testing/api/artifact" + model_tag "github.com/goharbor/harbor/src/pkg/tag/model/tag" 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" @@ -32,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() { @@ -53,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() { @@ -64,15 +64,15 @@ func (c *tagTestSuite) TestListTag() { RepositoryID: 1, Name: "library/hello-world", }, nil) - c.artCtl.On("ListTags").Return([]*artifact.Tag{ + c.tagCtl.On("List").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 +99,15 @@ func (c *tagTestSuite) TestListTagPagination1() { RepositoryID: 1, Name: "hello-world", }, nil) - c.artCtl.On("ListTags").Return([]*artifact.Tag{ + c.tagCtl.On("List").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 +135,15 @@ func (c *tagTestSuite) TestListTagPagination2() { RepositoryID: 1, Name: "hello-world", }, nil) - c.artCtl.On("ListTags").Return([]*artifact.Tag{ + c.tagCtl.On("List").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 +171,15 @@ func (c *tagTestSuite) TestListTagPagination3() { RepositoryID: 1, Name: "hello-world", }, nil) - c.artCtl.On("ListTags").Return([]*artifact.Tag{ + c.tagCtl.On("List").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..93d4cbdef 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" @@ -50,6 +51,7 @@ func newArtifactAPI() *artifactAPI { proMgr: project.Mgr, repoCtl: repository.Ctl, scanCtl: scan.DefaultController, + tagCtl: tag.Ctl, } } @@ -59,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 { @@ -232,12 +235,12 @@ 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 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? @@ -268,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() @@ -340,7 +343,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..a7e390d3c 100644 --- a/src/testing/api/artifact/controller.go +++ b/src/testing/api/artifact/controller.go @@ -82,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 *artifact.TagOption) ([]*artifact.Tag, error) { - args := f.Called() - var tags []*artifact.Tag - if args.Get(0) != nil { - tags = args.Get(0).([]*artifact.Tag) - } - return tags, args.Error(1) -} - -// CreateTag ... -func (f *FakeController) CreateTag(ctx context.Context, tag *artifact.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 new file mode 100644 index 000000000..d74afeee2 --- /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, ids []int64) (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)