diff --git a/src/controller/artifact/controller.go b/src/controller/artifact/controller.go index c9f326530..5a11a8e1c 100644 --- a/src/controller/artifact/controller.go +++ b/src/controller/artifact/controller.go @@ -154,7 +154,7 @@ func (c *controller) Ensure(ctx context.Context, repository, digest string, opti } if option != nil { for _, tag := range option.Tags { - if err = c.tagCtl.Ensure(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 } } diff --git a/src/controller/artifact/controller_test.go b/src/controller/artifact/controller_test.go index 9dcbc8f9b..7d4c46337 100644 --- a/src/controller/artifact/controller_test.go +++ b/src/controller/artifact/controller_test.go @@ -250,7 +250,7 @@ func (c *controllerTestSuite) TestEnsure() { c.artMgr.On("GetByDigest", mock.Anything, mock.Anything, mock.Anything).Return(nil, errors.NotFoundError(nil)) c.artMgr.On("Create", mock.Anything, mock.Anything).Return(int64(1), nil) c.abstractor.On("AbstractMetadata").Return(nil) - c.tagCtl.On("Ensure").Return(nil) + c.tagCtl.On("Ensure").Return(int64(1), nil) c.accMgr.On("Ensure").Return(nil) _, id, err := c.ctl.Ensure(orm.NewContext(nil, &ormtesting.FakeOrmer{}), "library/hello-world", digest, &ArtOption{ Tags: []string{"latest"}, @@ -563,7 +563,7 @@ func (c *controllerTestSuite) TestCopy() { c.abstractor.On("AbstractMetadata").Return(nil) c.artMgr.On("Create", mock.Anything, mock.Anything).Return(int64(1), nil) c.regCli.On("Copy", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) - c.tagCtl.On("Ensure").Return(nil) + c.tagCtl.On("Ensure").Return(int64(1), nil) c.accMgr.On("Ensure", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) _, err := c.ctl.Copy(orm.NewContext(nil, &ormtesting.FakeOrmer{}), "library/hello-world", "latest", "library/hello-world2") c.Require().Nil(err) diff --git a/src/controller/proxy/controller.go b/src/controller/proxy/controller.go index 517643727..b4b3735cd 100644 --- a/src/controller/proxy/controller.go +++ b/src/controller/proxy/controller.go @@ -35,6 +35,7 @@ import ( "github.com/goharbor/harbor/src/lib/log" "github.com/goharbor/harbor/src/lib/orm" proModels "github.com/goharbor/harbor/src/pkg/project/models" + model_tag "github.com/goharbor/harbor/src/pkg/tag/model/tag" ) const ( @@ -117,7 +118,17 @@ func (c *controller) EnsureTag(ctx context.Context, art lib.ArtifactInfo, tagNam if a == nil { return fmt.Errorf("the artifact is not ready yet, failed to tag it to %v", tagName) } - return tag.Ctl.Ensure(ctx, a.RepositoryID, a.Artifact.ID, tagName) + tagID, err := tag.Ctl.Ensure(ctx, a.RepositoryID, a.Artifact.ID, tagName) + if err != nil { + return err + } + // update the pull time of tag for the first time cache + return tag.Ctl.Update(ctx, &tag.Tag{ + Tag: model_tag.Tag{ + ID: tagID, + PullTime: time.Now(), + }, + }, "PullTime") } func (c *controller) UseLocalBlob(ctx context.Context, art lib.ArtifactInfo) bool { diff --git a/src/controller/tag/controller.go b/src/controller/tag/controller.go index b43cc7534..4fab9fb6b 100644 --- a/src/controller/tag/controller.go +++ b/src/controller/tag/controller.go @@ -41,7 +41,7 @@ var ( // Controller manages the tags type Controller interface { // Ensure - Ensure(ctx context.Context, repositoryID, artifactID int64, name string) error + Ensure(ctx context.Context, repositoryID, artifactID int64, name string) (int64, 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 @@ -74,7 +74,7 @@ type controller struct { } // Ensure ... -func (c *controller) Ensure(ctx context.Context, repositoryID, artifactID int64, name string) error { +func (c *controller) Ensure(ctx context.Context, repositoryID, artifactID int64, name string) (int64, error) { query := &q.Query{ Keywords: map[string]interface{}{ "repository_id": repositoryID, @@ -85,43 +85,44 @@ func (c *controller) Ensure(ctx context.Context, repositoryID, artifactID int64, WithImmutableStatus: true, }) if err != nil { - return err + return 0, 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 + return tag.ID, nil } // existing tag must check the immutable status and signature if tag.Immutable { - return errors.New(nil).WithCode(errors.PreconditionCode). + return 0, errors.New(nil).WithCode(errors.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") + return tag.ID, c.Update(ctx, tag, "ArtifactID", "PushTime") } // the tag doesn't exist under the repository, create it // use orm.WithTransaction here to avoid the issue: // https://www.postgresql.org/message-id/002e01c04da9%24a8f95c20%2425efe6c1%40lasting.ro + tagID := int64(0) if err = orm.WithTransaction(func(ctx context.Context) error { tag := &Tag{} tag.RepositoryID = repositoryID tag.ArtifactID = artifactID tag.Name = name tag.PushTime = time.Now() - _, err = c.Create(ctx, tag) + tagID, err = c.Create(ctx, tag) return err })(orm.SetTransactionOpNameToContext(ctx, "tx-tag-ensure")); err != nil && !errors.IsConflictErr(err) { - return err + return 0, err } - return nil + return tagID, nil } // Count ... diff --git a/src/controller/tag/controller_test.go b/src/controller/tag/controller_test.go index 8fc1484ca..bd8d1e7ec 100644 --- a/src/controller/tag/controller_test.go +++ b/src/controller/tag/controller_test.go @@ -68,7 +68,7 @@ func (c *controllerTestSuite) TestEnsureTag() { ID: 1, }, nil) c.immutableMtr.On("Match").Return(false, nil) - err := c.ctl.Ensure(orm.NewContext(nil, &ormtesting.FakeOrmer{}), 1, 1, "latest") + _, err := c.ctl.Ensure(orm.NewContext(nil, &ormtesting.FakeOrmer{}), 1, 1, "latest") c.Require().Nil(err) c.tagMgr.AssertExpectations(c.T()) @@ -89,7 +89,7 @@ func (c *controllerTestSuite) TestEnsureTag() { ID: 1, }, nil) c.immutableMtr.On("Match").Return(false, nil) - err = c.ctl.Ensure(orm.NewContext(nil, &ormtesting.FakeOrmer{}), 1, 1, "latest") + _, err = c.ctl.Ensure(orm.NewContext(nil, &ormtesting.FakeOrmer{}), 1, 1, "latest") c.Require().Nil(err) c.tagMgr.AssertExpectations(c.T()) @@ -103,7 +103,7 @@ func (c *controllerTestSuite) TestEnsureTag() { ID: 1, }, nil) c.immutableMtr.On("Match").Return(false, nil) - err = c.ctl.Ensure(orm.NewContext(nil, &ormtesting.FakeOrmer{}), 1, 1, "latest") + _, err = c.ctl.Ensure(orm.NewContext(nil, &ormtesting.FakeOrmer{}), 1, 1, "latest") c.Require().Nil(err) c.tagMgr.AssertExpectations(c.T()) } diff --git a/src/testing/controller/tag/controller.go b/src/testing/controller/tag/controller.go index 7e751748c..db7521e7e 100644 --- a/src/testing/controller/tag/controller.go +++ b/src/testing/controller/tag/controller.go @@ -15,9 +15,9 @@ type FakeController struct { } // Ensure ... -func (f *FakeController) Ensure(ctx context.Context, repositoryID, artifactID int64, name string) error { +func (f *FakeController) Ensure(ctx context.Context, repositoryID, artifactID int64, name string) (int64, error) { args := f.Called() - return args.Error(0) + return int64(0), args.Error(1) } // Count ...