From de504993ad6433e31f9c5a6aa974b8ac66875920 Mon Sep 17 00:00:00 2001 From: Wang Yan Date: Thu, 28 May 2020 18:48:36 +0800 Subject: [PATCH 1/4] update blob controller & manager 1, add two more attributes, update_time and status 2, add delete and fresh update time method in blob mgr & ctr. Signed-off-by: wang yan --- .../postgresql/0035_2_1_0_schema.up.sql | 2 + src/common/models/base.go | 1 - src/common/models/blob.go | 34 ---------------- src/controller/blob/controller.go | 11 +++++- src/controller/blob/controller_test.go | 16 ++++++++ .../job/impl/gc/garbage_collection_test.go | 5 ++- src/pkg/blob/dao/dao.go | 36 +++++++++++++++++ src/pkg/blob/dao/dao_test.go | 35 +++++++++++++++++ src/pkg/blob/manager.go | 15 +++++++ src/pkg/blob/manager_test.go | 35 +++++++++++++++++ src/pkg/blob/models/blob.go | 31 +++++++++++++-- src/testing/controller/blob/controller.go | 29 ++++++++++---- src/testing/pkg/blob/manager.go | 39 ++++++++++++++++--- 13 files changed, 234 insertions(+), 55 deletions(-) create mode 100644 make/migrations/postgresql/0035_2_1_0_schema.up.sql delete mode 100644 src/common/models/blob.go diff --git a/make/migrations/postgresql/0035_2_1_0_schema.up.sql b/make/migrations/postgresql/0035_2_1_0_schema.up.sql new file mode 100644 index 000000000..649c3f0f6 --- /dev/null +++ b/make/migrations/postgresql/0035_2_1_0_schema.up.sql @@ -0,0 +1,2 @@ +ALTER TABLE blob ADD COLUMN IF NOT EXISTS update_time timestamp default CURRENT_TIMESTAMP; +ALTER TABLE blob ADD COLUMN IF NOT EXISTS status varchar(255); diff --git a/src/common/models/base.go b/src/common/models/base.go index 4ca2b678d..d60952d5c 100644 --- a/src/common/models/base.go +++ b/src/common/models/base.go @@ -34,7 +34,6 @@ func init() { new(OIDCUser), new(NotificationPolicy), new(NotificationJob), - new(Blob), new(ProjectBlob), new(ArtifactAndBlob), new(CVEWhitelist), diff --git a/src/common/models/blob.go b/src/common/models/blob.go deleted file mode 100644 index 94f0d7db3..000000000 --- a/src/common/models/blob.go +++ /dev/null @@ -1,34 +0,0 @@ -package models - -import ( - "time" - - "github.com/docker/distribution/manifest/schema2" -) - -// Blob holds the details of a blob. -type Blob struct { - ID int64 `orm:"pk;auto;column(id)" json:"id"` - Digest string `orm:"column(digest)" json:"digest"` - ContentType string `orm:"column(content_type)" json:"content_type"` - Size int64 `orm:"column(size)" json:"size"` - CreationTime time.Time `orm:"column(creation_time);auto_now_add" json:"creation_time"` -} - -// TableName ... -func (b *Blob) TableName() string { - return "blob" -} - -// IsForeignLayer returns true if the blob is foreign layer -func (b *Blob) IsForeignLayer() bool { - return b.ContentType == schema2.MediaTypeForeignLayer -} - -// BlobQuery ... -type BlobQuery struct { - Digest string - ContentType string - Digests []string - Pagination -} diff --git a/src/controller/blob/controller.go b/src/controller/blob/controller.go index 2cd4e6b37..efb089237 100644 --- a/src/controller/blob/controller.go +++ b/src/controller/blob/controller.go @@ -20,12 +20,12 @@ import ( "github.com/docker/distribution" "github.com/garyburd/redigo/redis" - "github.com/goharbor/harbor/src/common/models" util "github.com/goharbor/harbor/src/common/utils/redis" "github.com/goharbor/harbor/src/lib/errors" "github.com/goharbor/harbor/src/lib/log" "github.com/goharbor/harbor/src/lib/orm" "github.com/goharbor/harbor/src/pkg/blob" + "time" ) var ( @@ -75,6 +75,9 @@ type Controller interface { // GetAcceptedBlobSize returns the accepted size of stream upload blob. GetAcceptedBlobSize(sessionID string) (int64, error) + + // ReFreshUpdateTime updates the update time for the blob. + ReFreshUpdateTime(ctx context.Context, digest string, time time.Time) (err error) } // NewController creates an instance of the default repository controller @@ -184,7 +187,7 @@ func (c *controller) FindMissingAssociationsForProject(ctx context.Context, proj associated[blob.Digest] = true } - var results []*models.Blob + var results []*blob.Blob for _, blob := range blobs { if !associated[blob.Digest] { results = append(results, blob) @@ -314,3 +317,7 @@ func (c *controller) GetAcceptedBlobSize(sessionID string) (int64, error) { return size, nil } + +func (c *controller) ReFreshUpdateTime(ctx context.Context, digest string, time time.Time) (err error) { + return c.blobMgr.ReFreshUpdateTime(ctx, digest, time) +} diff --git a/src/controller/blob/controller_test.go b/src/controller/blob/controller_test.go index 75ae11ec2..3570d6af9 100644 --- a/src/controller/blob/controller_test.go +++ b/src/controller/blob/controller_test.go @@ -18,6 +18,7 @@ import ( "context" "fmt" "testing" + "time" "github.com/docker/distribution/manifest/schema2" "github.com/goharbor/harbor/src/pkg/blob" @@ -267,6 +268,21 @@ func (suite *ControllerTestSuite) TestGetSetAcceptedBlobSize() { suite.Equal(int64(100), size) } +func (suite *ControllerTestSuite) TestReFreshUpdateTime() { + ctx := suite.Context() + + digest := suite.prepareBlob() + blob, err := Ctl.Get(ctx, digest) + suite.Nil(err) + + now := time.Now() + suite.NotEqual(blob.UpdateTime, now) + + err = Ctl.ReFreshUpdateTime(ctx, blob.Digest, now) + suite.Nil(err) + suite.Equal(blob.UpdateTime.Unix(), now.Unix()) +} + func TestControllerTestSuite(t *testing.T) { suite.Run(t, &ControllerTestSuite{}) } diff --git a/src/jobservice/job/impl/gc/garbage_collection_test.go b/src/jobservice/job/impl/gc/garbage_collection_test.go index 28f1f658d..7f70eca52 100644 --- a/src/jobservice/job/impl/gc/garbage_collection_test.go +++ b/src/jobservice/job/impl/gc/garbage_collection_test.go @@ -7,6 +7,7 @@ import ( "github.com/goharbor/harbor/src/jobservice/job" "github.com/goharbor/harbor/src/pkg/artifact" "github.com/goharbor/harbor/src/pkg/artifactrash/model" + pkg_blob "github.com/goharbor/harbor/src/pkg/blob/models" artifacttesting "github.com/goharbor/harbor/src/testing/controller/artifact" projecttesting "github.com/goharbor/harbor/src/testing/controller/project" mockjobservice "github.com/goharbor/harbor/src/testing/jobservice" @@ -93,7 +94,7 @@ func (suite *gcTestSuite) TestRemoveUntaggedBlobs() { }, }, nil) - mock.OnAnything(suite.blobMgr, "List").Return([]*models.Blob{ + mock.OnAnything(suite.blobMgr, "List").Return([]*pkg_blob.Blob{ { ID: 1234, Digest: "sha256:1234", @@ -203,7 +204,7 @@ func (suite *gcTestSuite) TestRun() { }, }, nil) - mock.OnAnything(suite.blobMgr, "List").Return([]*models.Blob{ + mock.OnAnything(suite.blobMgr, "List").Return([]*pkg_blob.Blob{ { ID: 12345, Digest: "sha256:12345", diff --git a/src/pkg/blob/dao/dao.go b/src/pkg/blob/dao/dao.go index 7108d5d63..465380a1f 100644 --- a/src/pkg/blob/dao/dao.go +++ b/src/pkg/blob/dao/dao.go @@ -17,6 +17,7 @@ package dao import ( "context" "fmt" + "github.com/goharbor/harbor/src/lib/errors" "strings" "time" @@ -24,6 +25,8 @@ import ( "github.com/goharbor/harbor/src/lib/orm" "github.com/goharbor/harbor/src/lib/q" "github.com/goharbor/harbor/src/pkg/blob/models" + + beego_orm "github.com/astaxie/beego/orm" ) // DAO the dao for Blob, ArtifactAndBlob and ProjectBlob @@ -66,6 +69,12 @@ type DAO interface { // ExistProjectBlob returns true when ProjectBlob exist ExistProjectBlob(ctx context.Context, projectID int64, blobDigest string) (bool, error) + + // DeleteBlob delete blob + DeleteBlob(ctx context.Context, id int64) (err error) + + // ReFreshUpdateTime updates the blob update time + ReFreshUpdateTime(ctx context.Context, digest string, time time.Time) error } // New returns an instance of the default DAO @@ -318,3 +327,30 @@ func (d *dao) DeleteProjectBlob(ctx context.Context, projectID int64, blobIDs .. _, err = qs.Delete() return err } + +func (d *dao) DeleteBlob(ctx context.Context, id int64) error { + ormer, err := orm.FromContext(ctx) + if err != nil { + return err + } + n, err := ormer.Delete(&models.Blob{ + ID: id, + }) + if err != nil { + return err + } + if n == 0 { + return errors.NotFoundError(nil).WithMessage("blob %d not found", id) + } + return nil +} + +func (d *dao) ReFreshUpdateTime(ctx context.Context, digest string, time time.Time) error { + qs, err := orm.QuerySetter(ctx, &models.Blob{}, &q.Query{ + Keywords: map[string]interface{}{ + "digest": digest, + }, + }) + _, err = qs.Update(beego_orm.Params{"update_time": time}) + return err +} diff --git a/src/pkg/blob/dao/dao_test.go b/src/pkg/blob/dao/dao_test.go index 06bec85b0..836864030 100644 --- a/src/pkg/blob/dao/dao_test.go +++ b/src/pkg/blob/dao/dao_test.go @@ -15,7 +15,9 @@ package dao import ( + "github.com/goharbor/harbor/src/lib/errors" "testing" + "time" "github.com/goharbor/harbor/src/pkg/blob/models" htesting "github.com/goharbor/harbor/src/testing" @@ -322,6 +324,39 @@ func (suite *DaoTestSuite) TestDeleteProjectBlob() { } } +func (suite *DaoTestSuite) TestDelete() { + ctx := suite.Context() + + err := suite.dao.DeleteBlob(ctx, 100021) + suite.Require().NotNil(err) + suite.True(errors.IsErr(err, errors.NotFoundCode)) + + digest := suite.DigestString() + id, err := suite.dao.CreateBlob(ctx, &models.Blob{Digest: digest}) + suite.Nil(err) + err = suite.dao.DeleteBlob(ctx, id) + suite.Require().Nil(err) +} + +func (suite *DaoTestSuite) TestReFreshUpdateTime() { + ctx := suite.Context() + digest := suite.DigestString() + suite.dao.CreateBlob(ctx, &models.Blob{Digest: digest}) + blob, err := suite.dao.GetBlobByDigest(ctx, digest) + suite.Require().Nil(err) + + time.Sleep(1 * time.Second) + now := time.Now() + suite.NotEqual(blob.UpdateTime, now) + + if suite.Nil(suite.dao.ReFreshUpdateTime(ctx, blob.Digest, now)) { + blob, err := suite.dao.GetBlobByDigest(ctx, digest) + if suite.Nil(err) { + suite.Equal(now.Unix(), blob.UpdateTime.Unix()) + } + } +} + func TestDaoTestSuite(t *testing.T) { suite.Run(t, &DaoTestSuite{}) } diff --git a/src/pkg/blob/manager.go b/src/pkg/blob/manager.go index d2790c143..07a496dfb 100644 --- a/src/pkg/blob/manager.go +++ b/src/pkg/blob/manager.go @@ -19,6 +19,7 @@ import ( "github.com/goharbor/harbor/src/pkg/blob/dao" "github.com/goharbor/harbor/src/pkg/blob/models" + "time" ) // Blob alias `models.Blob` to make it natural to use the Manager @@ -60,6 +61,12 @@ type Manager interface { // List returns blobs by params List(ctx context.Context, params ListParams) ([]*Blob, error) + + // DeleteBlob delete blob + Delete(ctx context.Context, id int64) (err error) + + // ReFreshUpdateTime updates the blob update time + ReFreshUpdateTime(ctx context.Context, digest string, time time.Time) error } type manager struct { @@ -116,6 +123,14 @@ func (m *manager) List(ctx context.Context, params ListParams) ([]*Blob, error) return m.dao.ListBlobs(ctx, params) } +func (m *manager) Delete(ctx context.Context, id int64) error { + return m.dao.DeleteBlob(ctx, id) +} + +func (m *manager) ReFreshUpdateTime(ctx context.Context, digest string, time time.Time) error { + return m.dao.ReFreshUpdateTime(ctx, digest, time) +} + // NewManager returns blob manager func NewManager() Manager { return &manager{dao: dao.New()} diff --git a/src/pkg/blob/manager_test.go b/src/pkg/blob/manager_test.go index da836ee28..f1426be2c 100644 --- a/src/pkg/blob/manager_test.go +++ b/src/pkg/blob/manager_test.go @@ -17,6 +17,7 @@ package blob import ( "context" "testing" + "time" htesting "github.com/goharbor/harbor/src/testing" "github.com/stretchr/testify/suite" @@ -256,6 +257,40 @@ func (suite *ManagerTestSuite) TestListByArtifact() { suite.Len(blobs, 3) } +func (suite *ManagerTestSuite) TestDelete() { + ctx := suite.Context() + digest := suite.DigestString() + blobID, err := Mgr.Create(ctx, digest, "media type", 100) + suite.Nil(err) + + err = Mgr.Delete(ctx, blobID) + suite.Nil(err) +} + +func (suite *ManagerTestSuite) TestReFreshUpdateTime() { + ctx := suite.Context() + + digest := suite.DigestString() + _, err := Mgr.Create(ctx, digest, "media type", 100) + suite.Nil(err) + + time.Sleep(1 * time.Second) + now := time.Now() + + blob, err := Mgr.Get(ctx, digest) + if suite.Nil(err) { + blob.UpdateTime = now + suite.Nil(Mgr.Update(ctx, blob)) + + { + blob, err := Mgr.Get(ctx, digest) + suite.Nil(err) + suite.Equal(digest, blob.Digest) + suite.Equal(now.Unix(), blob.UpdateTime.Unix()) + } + } +} + func TestManagerTestSuite(t *testing.T) { suite.Run(t, &ManagerTestSuite{}) } diff --git a/src/pkg/blob/models/blob.go b/src/pkg/blob/models/blob.go index de8a0dac6..4d340b32b 100644 --- a/src/pkg/blob/models/blob.go +++ b/src/pkg/blob/models/blob.go @@ -15,16 +15,41 @@ package models import ( + "github.com/astaxie/beego/orm" + "github.com/docker/distribution/manifest/schema2" "github.com/goharbor/harbor/src/common/models" + "time" ) -// TODO: move ArtifactAndBlob, Blob and ProjectBlob to here +func init() { + orm.RegisterModel(&Blob{}) +} + +// TODO: move ArtifactAndBlob, ProjectBlob to here // ArtifactAndBlob alias ArtifactAndBlob model type ArtifactAndBlob = models.ArtifactAndBlob -// Blob alias Blob model -type Blob = models.Blob +// Blob holds the details of a blob. +type Blob struct { + ID int64 `orm:"pk;auto;column(id)" json:"id"` + Digest string `orm:"column(digest)" json:"digest"` + ContentType string `orm:"column(content_type)" json:"content_type"` + Size int64 `orm:"column(size)" json:"size"` + Status string `orm:"column(status)" json:"status"` + UpdateTime time.Time `orm:"column(update_time);auto_now_add" json:"update_time"` + CreationTime time.Time `orm:"column(creation_time);auto_now_add" json:"creation_time"` +} + +// TableName ... +func (b *Blob) TableName() string { + return "blob" +} + +// IsForeignLayer returns true if the blob is foreign layer +func (b *Blob) IsForeignLayer() bool { + return b.ContentType == schema2.MediaTypeForeignLayer +} // ProjectBlob alias ProjectBlob model type ProjectBlob = models.ProjectBlob diff --git a/src/testing/controller/blob/controller.go b/src/testing/controller/blob/controller.go index 62d3c1f1e..ca23f8a27 100644 --- a/src/testing/controller/blob/controller.go +++ b/src/testing/controller/blob/controller.go @@ -3,16 +3,17 @@ package blob import ( - blob "github.com/goharbor/harbor/src/controller/blob" - blobmodels "github.com/goharbor/harbor/src/pkg/blob/models" - context "context" + blob "github.com/goharbor/harbor/src/controller/blob" + distribution "github.com/docker/distribution" mock "github.com/stretchr/testify/mock" - models "github.com/goharbor/harbor/src/common/models" + models "github.com/goharbor/harbor/src/pkg/blob/models" + + time "time" ) // Controller is an autogenerated mock type for the Controller type @@ -207,11 +208,11 @@ func (_m *Controller) GetAcceptedBlobSize(sessionID string) (int64, error) { } // List provides a mock function with given fields: ctx, params -func (_m *Controller) List(ctx context.Context, params blobmodels.ListParams) ([]*models.Blob, error) { +func (_m *Controller) List(ctx context.Context, params models.ListParams) ([]*models.Blob, error) { ret := _m.Called(ctx, params) var r0 []*models.Blob - if rf, ok := ret.Get(0).(func(context.Context, blobmodels.ListParams) []*models.Blob); ok { + if rf, ok := ret.Get(0).(func(context.Context, models.ListParams) []*models.Blob); ok { r0 = rf(ctx, params) } else { if ret.Get(0) != nil { @@ -220,7 +221,7 @@ func (_m *Controller) List(ctx context.Context, params blobmodels.ListParams) ([ } var r1 error - if rf, ok := ret.Get(1).(func(context.Context, blobmodels.ListParams) error); ok { + if rf, ok := ret.Get(1).(func(context.Context, models.ListParams) error); ok { r1 = rf(ctx, params) } else { r1 = ret.Error(1) @@ -229,6 +230,20 @@ func (_m *Controller) List(ctx context.Context, params blobmodels.ListParams) ([ return r0, r1 } +// ReFreshUpdateTime provides a mock function with given fields: ctx, digest, _a2 +func (_m *Controller) ReFreshUpdateTime(ctx context.Context, digest string, _a2 time.Time) error { + ret := _m.Called(ctx, digest, _a2) + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string, time.Time) error); ok { + r0 = rf(ctx, digest, _a2) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // SetAcceptedBlobSize provides a mock function with given fields: sessionID, size func (_m *Controller) SetAcceptedBlobSize(sessionID string, size int64) error { ret := _m.Called(sessionID, size) diff --git a/src/testing/pkg/blob/manager.go b/src/testing/pkg/blob/manager.go index a6ac7a962..e2cc96d3c 100644 --- a/src/testing/pkg/blob/manager.go +++ b/src/testing/pkg/blob/manager.go @@ -5,11 +5,10 @@ package blob import ( context "context" - blobmodels "github.com/goharbor/harbor/src/pkg/blob/models" - + models "github.com/goharbor/harbor/src/pkg/blob/models" mock "github.com/stretchr/testify/mock" - models "github.com/goharbor/harbor/src/common/models" + time "time" ) // Manager is an autogenerated mock type for the Manager type @@ -129,6 +128,20 @@ func (_m *Manager) Create(ctx context.Context, digest string, contentType string return r0, r1 } +// Delete provides a mock function with given fields: ctx, id +func (_m *Manager) Delete(ctx context.Context, id int64) error { + ret := _m.Called(ctx, id) + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, int64) error); ok { + r0 = rf(ctx, id) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // Get provides a mock function with given fields: ctx, digest func (_m *Manager) Get(ctx context.Context, digest string) (*models.Blob, error) { ret := _m.Called(ctx, digest) @@ -153,11 +166,11 @@ func (_m *Manager) Get(ctx context.Context, digest string) (*models.Blob, error) } // List provides a mock function with given fields: ctx, params -func (_m *Manager) List(ctx context.Context, params blobmodels.ListParams) ([]*models.Blob, error) { +func (_m *Manager) List(ctx context.Context, params models.ListParams) ([]*models.Blob, error) { ret := _m.Called(ctx, params) var r0 []*models.Blob - if rf, ok := ret.Get(0).(func(context.Context, blobmodels.ListParams) []*models.Blob); ok { + if rf, ok := ret.Get(0).(func(context.Context, models.ListParams) []*models.Blob); ok { r0 = rf(ctx, params) } else { if ret.Get(0) != nil { @@ -166,7 +179,7 @@ func (_m *Manager) List(ctx context.Context, params blobmodels.ListParams) ([]*m } var r1 error - if rf, ok := ret.Get(1).(func(context.Context, blobmodels.ListParams) error); ok { + if rf, ok := ret.Get(1).(func(context.Context, models.ListParams) error); ok { r1 = rf(ctx, params) } else { r1 = ret.Error(1) @@ -175,6 +188,20 @@ func (_m *Manager) List(ctx context.Context, params blobmodels.ListParams) ([]*m return r0, r1 } +// ReFreshUpdateTime provides a mock function with given fields: ctx, digest, _a2 +func (_m *Manager) ReFreshUpdateTime(ctx context.Context, digest string, _a2 time.Time) error { + ret := _m.Called(ctx, digest, _a2) + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string, time.Time) error); ok { + r0 = rf(ctx, digest, _a2) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // Update provides a mock function with given fields: ctx, _a1 func (_m *Manager) Update(ctx context.Context, _a1 *models.Blob) error { ret := _m.Called(ctx, _a1) From c10467eb36891de236d0b38aed4cf16b5af577ef Mon Sep 17 00:00:00 2001 From: wang yan Date: Tue, 2 Jun 2020 12:25:24 +0800 Subject: [PATCH 2/4] continue refactor Signed-off-by: wang yan --- .../postgresql/0035_2_1_0_schema.up.sql | 2 - .../postgresql/0040_2.1.0_schema.up.sql | 6 ++ src/controller/blob/controller.go | 26 ++++-- src/controller/blob/controller_test.go | 33 +++++-- src/pkg/blob/dao/dao.go | 60 +++++++++---- src/pkg/blob/dao/dao_test.go | 67 +++++++++----- src/pkg/blob/manager.go | 16 ++-- src/pkg/blob/manager_test.go | 19 ++-- src/pkg/blob/models/blob.go | 32 +++++++ src/server/middleware/blob/head_blob.go | 56 ++++++++++++ src/server/middleware/blob/head_blob_test.go | 88 +++++++++++++++++++ src/server/middleware/patterns.go | 2 +- src/server/registry/route.go | 5 ++ src/testing/controller/blob/controller.go | 65 ++++++++++---- src/testing/pkg/blob/manager.go | 37 ++++---- 15 files changed, 405 insertions(+), 109 deletions(-) delete mode 100644 make/migrations/postgresql/0035_2_1_0_schema.up.sql create mode 100644 src/server/middleware/blob/head_blob.go create mode 100644 src/server/middleware/blob/head_blob_test.go diff --git a/make/migrations/postgresql/0035_2_1_0_schema.up.sql b/make/migrations/postgresql/0035_2_1_0_schema.up.sql deleted file mode 100644 index 649c3f0f6..000000000 --- a/make/migrations/postgresql/0035_2_1_0_schema.up.sql +++ /dev/null @@ -1,2 +0,0 @@ -ALTER TABLE blob ADD COLUMN IF NOT EXISTS update_time timestamp default CURRENT_TIMESTAMP; -ALTER TABLE blob ADD COLUMN IF NOT EXISTS status varchar(255); diff --git a/make/migrations/postgresql/0040_2.1.0_schema.up.sql b/make/migrations/postgresql/0040_2.1.0_schema.up.sql index 667b9db78..dbe15d503 100644 --- a/make/migrations/postgresql/0040_2.1.0_schema.up.sql +++ b/make/migrations/postgresql/0040_2.1.0_schema.up.sql @@ -29,3 +29,9 @@ CREATE TABLE IF NOT EXISTS task ( end_time timestamp, FOREIGN KEY (execution_id) REFERENCES execution(id) ); + +ALTER TABLE blob ADD COLUMN IF NOT EXISTS update_time timestamp default CURRENT_TIMESTAMP; +ALTER TABLE blob ADD COLUMN IF NOT EXISTS status varchar(255); +ALTER TABLE blob ADD COLUMN IF NOT EXISTS version BIGINT default 0; +CREATE INDEX IF NOT EXISTS idx_status ON blob (status); +CREATE INDEX IF NOT EXISTS idx_version ON blob (version); \ No newline at end of file diff --git a/src/controller/blob/controller.go b/src/controller/blob/controller.go index efb089237..817a2912c 100644 --- a/src/controller/blob/controller.go +++ b/src/controller/blob/controller.go @@ -17,7 +17,6 @@ package blob import ( "context" "fmt" - "github.com/docker/distribution" "github.com/garyburd/redigo/redis" util "github.com/goharbor/harbor/src/common/utils/redis" @@ -25,7 +24,6 @@ import ( "github.com/goharbor/harbor/src/lib/log" "github.com/goharbor/harbor/src/lib/orm" "github.com/goharbor/harbor/src/pkg/blob" - "time" ) var ( @@ -76,8 +74,14 @@ type Controller interface { // GetAcceptedBlobSize returns the accepted size of stream upload blob. GetAcceptedBlobSize(sessionID string) (int64, error) - // ReFreshUpdateTime updates the update time for the blob. - ReFreshUpdateTime(ctx context.Context, digest string, time time.Time) (err error) + // Touch updates the blob status and increase version every time. + Touch(ctx context.Context, blob *blob.Blob) (int64, error) + + // Update updates the blob, it cannot handle blob status transitions. + Update(ctx context.Context, blob *blob.Blob) error + + // Delete deletes the blob by its id + Delete(ctx context.Context, id int64) error } // NewController creates an instance of the default repository controller @@ -263,7 +267,7 @@ func (c *controller) Sync(ctx context.Context, references []distribution.Descrip if len(updating) > 0 { orm.WithTransaction(func(ctx context.Context) error { for _, blob := range updating { - if err := c.blobMgr.Update(ctx, blob); err != nil { + if err := c.Update(ctx, blob); err != nil { log.G(ctx).Warningf("Failed to update blob %s, error: %v", blob.Digest, err) return err } @@ -318,6 +322,14 @@ func (c *controller) GetAcceptedBlobSize(sessionID string) (int64, error) { return size, nil } -func (c *controller) ReFreshUpdateTime(ctx context.Context, digest string, time time.Time) (err error) { - return c.blobMgr.ReFreshUpdateTime(ctx, digest, time) +func (c *controller) Touch(ctx context.Context, blob *blob.Blob) (int64, error) { + return c.blobMgr.UpdateBlobStatus(ctx, blob) +} + +func (c *controller) Update(ctx context.Context, blob *blob.Blob) error { + return c.blobMgr.Update(ctx, blob) +} + +func (c *controller) Delete(ctx context.Context, id int64) error { + return c.blobMgr.Delete(ctx, id) } diff --git a/src/controller/blob/controller_test.go b/src/controller/blob/controller_test.go index 3570d6af9..e33c2c331 100644 --- a/src/controller/blob/controller_test.go +++ b/src/controller/blob/controller_test.go @@ -17,8 +17,8 @@ package blob import ( "context" "fmt" + "github.com/goharbor/harbor/src/pkg/blob/models" "testing" - "time" "github.com/docker/distribution/manifest/schema2" "github.com/goharbor/harbor/src/pkg/blob" @@ -268,19 +268,38 @@ func (suite *ControllerTestSuite) TestGetSetAcceptedBlobSize() { suite.Equal(int64(100), size) } -func (suite *ControllerTestSuite) TestReFreshUpdateTime() { +func (suite *ControllerTestSuite) TestUpdateStatus() { ctx := suite.Context() digest := suite.prepareBlob() blob, err := Ctl.Get(ctx, digest) suite.Nil(err) - now := time.Now() - suite.NotEqual(blob.UpdateTime, now) - - err = Ctl.ReFreshUpdateTime(ctx, blob.Digest, now) + suite.Equal(blob.Status, models.StatusNone) + blob.Status = models.StatusDelete + count, err := Ctl.Touch(ctx, blob) suite.Nil(err) - suite.Equal(blob.UpdateTime.Unix(), now.Unix()) + suite.Equal(blob.Status, models.StatusDelete) + suite.Equal(int64(1), count) +} + +func (suite *ControllerTestSuite) TestDelete() { + ctx := suite.Context() + + digest := suite.DigestString() + _, err := Ctl.Ensure(ctx, digest, "application/octet-stream", 100) + suite.Nil(err) + + blob, err := Ctl.Get(ctx, digest) + suite.Nil(err) + suite.Equal(digest, blob.Digest) + + err = Ctl.Delete(ctx, blob.ID) + suite.Nil(err) + + exist, err := Ctl.Exist(ctx, digest) + suite.Nil(err) + suite.False(exist) } func TestControllerTestSuite(t *testing.T) { diff --git a/src/pkg/blob/dao/dao.go b/src/pkg/blob/dao/dao.go index 465380a1f..b2b5dd6ff 100644 --- a/src/pkg/blob/dao/dao.go +++ b/src/pkg/blob/dao/dao.go @@ -21,12 +21,11 @@ import ( "strings" "time" + beego_orm "github.com/astaxie/beego/orm" "github.com/docker/distribution/manifest/schema2" "github.com/goharbor/harbor/src/lib/orm" "github.com/goharbor/harbor/src/lib/q" "github.com/goharbor/harbor/src/pkg/blob/models" - - beego_orm "github.com/astaxie/beego/orm" ) // DAO the dao for Blob, ArtifactAndBlob and ProjectBlob @@ -52,6 +51,9 @@ type DAO interface { // UpdateBlob update blob UpdateBlob(ctx context.Context, blob *models.Blob) error + // UpdateBlob update blob status + UpdateBlobStatus(ctx context.Context, blob *models.Blob) (int64, error) + // ListBlobs list blobs by query ListBlobs(ctx context.Context, params models.ListParams) ([]*models.Blob, error) @@ -72,9 +74,6 @@ type DAO interface { // DeleteBlob delete blob DeleteBlob(ctx context.Context, id int64) (err error) - - // ReFreshUpdateTime updates the blob update time - ReFreshUpdateTime(ctx context.Context, digest string, time time.Time) error } // New returns an instance of the default DAO @@ -171,13 +170,50 @@ func (d *dao) GetBlobByDigest(ctx context.Context, digest string) (*models.Blob, return blob, nil } +func (d *dao) UpdateBlobStatus(ctx context.Context, blob *models.Blob) (int64, error) { + o, err := orm.FromContext(ctx) + if err != nil { + return -1, err + } + + // each update will auto increase version and update time + data := make(beego_orm.Params) + data["version"] = beego_orm.ColValue(beego_orm.ColAdd, 1) + data["update_time"] = time.Now() + data["status"] = blob.Status + + qt := o.QueryTable(&models.Blob{}) + cond := beego_orm.NewCondition() + var c *beego_orm.Condition + + // In the multiple blob head scenario, if one request success mark the blob from StatusDelete to StatusNone, then version should increase one. + // in the meantime, the other requests tries to do the same thing, use 'where version >= blob.version' can handle it. + if blob.Status == models.StatusNone { + c = cond.And("version__gte", blob.Version) + } else { + c = cond.And("version", blob.Version) + } + + /* + generated simple sql string. + UPDATE "blob" SET "version" = "version" + $1, "update_time" = $2, "status" = $3 + WHERE "id" IN ( SELECT T0."id" FROM "blob" T0 WHERE T0."version" >= $4 AND T0."id" = $5 AND T0."status" IN ('delete', 'deleting') ) + */ + + return qt.SetCond(c).Filter("id", blob.ID). + Filter("status__in", models.StatusMap[blob.Status]). + Update(data) +} + +// UpdateBlob cannot handle the status change. func (d *dao) UpdateBlob(ctx context.Context, blob *models.Blob) error { o, err := orm.FromContext(ctx) if err != nil { return err } - - _, err = o.Update(blob) + blob.Version = blob.Version + 1 + blob.UpdateTime = time.Now() + _, err = o.Update(blob, "size", "content_type", "version", "update_time") return err } @@ -344,13 +380,3 @@ func (d *dao) DeleteBlob(ctx context.Context, id int64) error { } return nil } - -func (d *dao) ReFreshUpdateTime(ctx context.Context, digest string, time time.Time) error { - qs, err := orm.QuerySetter(ctx, &models.Blob{}, &q.Query{ - Keywords: map[string]interface{}{ - "digest": digest, - }, - }) - _, err = qs.Update(beego_orm.Params{"update_time": time}) - return err -} diff --git a/src/pkg/blob/dao/dao_test.go b/src/pkg/blob/dao/dao_test.go index 836864030..023394ab6 100644 --- a/src/pkg/blob/dao/dao_test.go +++ b/src/pkg/blob/dao/dao_test.go @@ -16,12 +16,10 @@ package dao import ( "github.com/goharbor/harbor/src/lib/errors" - "testing" - "time" - "github.com/goharbor/harbor/src/pkg/blob/models" htesting "github.com/goharbor/harbor/src/testing" "github.com/stretchr/testify/suite" + "testing" ) type DaoTestSuite struct { @@ -131,20 +129,60 @@ func (suite *DaoTestSuite) TestUpdateBlob() { digest := suite.DigestString() suite.dao.CreateBlob(ctx, &models.Blob{Digest: digest}) - blob, err := suite.dao.GetBlobByDigest(ctx, digest) if suite.Nil(err) { suite.Equal(int64(0), blob.Size) } blob.Size = 100 - if suite.Nil(suite.dao.UpdateBlob(ctx, blob)) { blob, err := suite.dao.GetBlobByDigest(ctx, digest) if suite.Nil(err) { suite.Equal(int64(100), blob.Size) + suite.Equal(int64(1), blob.Version) } } + + blob.Status = "deleting" + suite.Nil(suite.dao.UpdateBlob(ctx, blob), "cannot be updated.") + blob, err = suite.dao.GetBlobByDigest(ctx, digest) + if suite.Nil(err) { + suite.Equal(int64(2), blob.Version) + suite.Equal(models.StatusNone, blob.Status) + } +} + +func (suite *DaoTestSuite) TestUpdateBlobStatus() { + ctx := suite.Context() + + digest := suite.DigestString() + + suite.dao.CreateBlob(ctx, &models.Blob{Digest: digest}) + blob, err := suite.dao.GetBlobByDigest(ctx, digest) + if suite.Nil(err) { + suite.Equal(int64(0), blob.Size) + } + + // StatusNone cannot be updated to StatusDeleting directly + blob.Status = models.StatusDeleting + count, err := suite.dao.UpdateBlobStatus(ctx, blob) + suite.Nil(err) + suite.Equal(int64(0), count) + blob, err = suite.dao.GetBlobByDigest(ctx, digest) + if suite.Nil(err) { + suite.Equal(int64(0), blob.Version) + suite.Equal(models.StatusNone, blob.Status) + } + + blob.Status = models.StatusDelete + count, err = suite.dao.UpdateBlobStatus(ctx, blob) + suite.Nil(err) + suite.Equal(int64(1), count) + blob, err = suite.dao.GetBlobByDigest(ctx, digest) + if suite.Nil(err) { + suite.Equal(int64(1), blob.Version) + suite.Equal(models.StatusDelete, blob.Status) + } } func (suite *DaoTestSuite) TestListBlobs() { @@ -338,25 +376,6 @@ func (suite *DaoTestSuite) TestDelete() { suite.Require().Nil(err) } -func (suite *DaoTestSuite) TestReFreshUpdateTime() { - ctx := suite.Context() - digest := suite.DigestString() - suite.dao.CreateBlob(ctx, &models.Blob{Digest: digest}) - blob, err := suite.dao.GetBlobByDigest(ctx, digest) - suite.Require().Nil(err) - - time.Sleep(1 * time.Second) - now := time.Now() - suite.NotEqual(blob.UpdateTime, now) - - if suite.Nil(suite.dao.ReFreshUpdateTime(ctx, blob.Digest, now)) { - blob, err := suite.dao.GetBlobByDigest(ctx, digest) - if suite.Nil(err) { - suite.Equal(now.Unix(), blob.UpdateTime.Unix()) - } - } -} - func TestDaoTestSuite(t *testing.T) { suite.Run(t, &DaoTestSuite{}) } diff --git a/src/pkg/blob/manager.go b/src/pkg/blob/manager.go index 07a496dfb..a370c6a8f 100644 --- a/src/pkg/blob/manager.go +++ b/src/pkg/blob/manager.go @@ -16,10 +16,8 @@ package blob import ( "context" - "github.com/goharbor/harbor/src/pkg/blob/dao" "github.com/goharbor/harbor/src/pkg/blob/models" - "time" ) // Blob alias `models.Blob` to make it natural to use the Manager @@ -59,14 +57,14 @@ type Manager interface { // Update the blob Update(ctx context.Context, blob *Blob) error + // Update the blob status + UpdateBlobStatus(ctx context.Context, blob *models.Blob) (int64, error) + // List returns blobs by params List(ctx context.Context, params ListParams) ([]*Blob, error) // DeleteBlob delete blob Delete(ctx context.Context, id int64) (err error) - - // ReFreshUpdateTime updates the blob update time - ReFreshUpdateTime(ctx context.Context, digest string, time time.Time) error } type manager struct { @@ -119,6 +117,10 @@ func (m *manager) Update(ctx context.Context, blob *Blob) error { return m.dao.UpdateBlob(ctx, blob) } +func (m *manager) UpdateBlobStatus(ctx context.Context, blob *models.Blob) (int64, error) { + return m.dao.UpdateBlobStatus(ctx, blob) +} + func (m *manager) List(ctx context.Context, params ListParams) ([]*Blob, error) { return m.dao.ListBlobs(ctx, params) } @@ -127,10 +129,6 @@ func (m *manager) Delete(ctx context.Context, id int64) error { return m.dao.DeleteBlob(ctx, id) } -func (m *manager) ReFreshUpdateTime(ctx context.Context, digest string, time time.Time) error { - return m.dao.ReFreshUpdateTime(ctx, digest, time) -} - // NewManager returns blob manager func NewManager() Manager { return &manager{dao: dao.New()} diff --git a/src/pkg/blob/manager_test.go b/src/pkg/blob/manager_test.go index f1426be2c..5618b1158 100644 --- a/src/pkg/blob/manager_test.go +++ b/src/pkg/blob/manager_test.go @@ -16,11 +16,11 @@ package blob import ( "context" - "testing" - "time" - htesting "github.com/goharbor/harbor/src/testing" "github.com/stretchr/testify/suite" + "testing" + + "github.com/goharbor/harbor/src/pkg/blob/models" ) type ManagerTestSuite struct { @@ -201,6 +201,7 @@ func (suite *ManagerTestSuite) TestUpdate() { suite.Equal(digest, blob.Digest) suite.Equal("media type", blob.ContentType) suite.Equal(int64(1000), blob.Size) + suite.Equal(models.StatusNone, blob.Status) } } } @@ -267,26 +268,24 @@ func (suite *ManagerTestSuite) TestDelete() { suite.Nil(err) } -func (suite *ManagerTestSuite) TestReFreshUpdateTime() { +func (suite *ManagerTestSuite) TestUpdateStatus() { ctx := suite.Context() digest := suite.DigestString() _, err := Mgr.Create(ctx, digest, "media type", 100) suite.Nil(err) - time.Sleep(1 * time.Second) - now := time.Now() - blob, err := Mgr.Get(ctx, digest) if suite.Nil(err) { - blob.UpdateTime = now - suite.Nil(Mgr.Update(ctx, blob)) + blob.Status = models.StatusDelete + _, err := Mgr.UpdateBlobStatus(ctx, blob) + suite.Nil(err) { blob, err := Mgr.Get(ctx, digest) suite.Nil(err) suite.Equal(digest, blob.Digest) - suite.Equal(now.Unix(), blob.UpdateTime.Unix()) + suite.Equal(models.StatusDelete, blob.Status) } } } diff --git a/src/pkg/blob/models/blob.go b/src/pkg/blob/models/blob.go index 4d340b32b..bb9a191db 100644 --- a/src/pkg/blob/models/blob.go +++ b/src/pkg/blob/models/blob.go @@ -30,6 +30,37 @@ func init() { // ArtifactAndBlob alias ArtifactAndBlob model type ArtifactAndBlob = models.ArtifactAndBlob +/* +the status are used for Garbage Collection +StatusNone, the blob is using in Harbor as normal. +StatusDelete, the blob is marked as GC candidate. +StatusDeleting, the blob undergo a GC blob deletion. +StatusDeleteFailed, the blob is failed to delete from the backend storage. + +The status transitions +StatusNone -> StatusDelete : Mark the blob as candidate. +StatusDelete -> StatusDeleting : Select the blob and call the API to delete asset in the backend storage. +StatusDeleting -> Trash : Delete success from the backend storage. +StatusDelete -> StatusNone : Client asks the existence of blob, remove it from the candidate. +StatusDelete -> StatusDeleteFailed : The storage driver returns fail when to delete the real data from the configurated file system. +StatusDeleteFailed -> StatusNone : The delete failed blobs can be pushed again, and back to normal. +StatusDeleteFailed -> StatusDelete : The delete failed blobs should be in the candidate. +*/ +const ( + StatusNone = "" + StatusDelete = "delete" + StatusDeleting = "deleting" + StatusDeleteFailed = "deletefailed" +) + +// StatusMap key is the target status, values are the accept source status. For example, only StatusNone and StatusDeleteFailed can be convert to StatusDelete. +var StatusMap = map[string][]string{ + StatusNone: {StatusNone, StatusDelete, StatusDeleteFailed}, + StatusDelete: {StatusNone, StatusDeleteFailed}, + StatusDeleting: {StatusDelete}, + StatusDeleteFailed: {StatusDeleting}, +} + // Blob holds the details of a blob. type Blob struct { ID int64 `orm:"pk;auto;column(id)" json:"id"` @@ -38,6 +69,7 @@ type Blob struct { Size int64 `orm:"column(size)" json:"size"` Status string `orm:"column(status)" json:"status"` UpdateTime time.Time `orm:"column(update_time);auto_now_add" json:"update_time"` + Version int64 `orm:"column(version)" json:"version"` CreationTime time.Time `orm:"column(creation_time);auto_now_add" json:"creation_time"` } diff --git a/src/server/middleware/blob/head_blob.go b/src/server/middleware/blob/head_blob.go new file mode 100644 index 000000000..50123cf0f --- /dev/null +++ b/src/server/middleware/blob/head_blob.go @@ -0,0 +1,56 @@ +package blob + +import ( + "fmt" + "github.com/goharbor/harbor/src/controller/blob" + "github.com/goharbor/harbor/src/lib" + "github.com/goharbor/harbor/src/lib/errors" + "github.com/goharbor/harbor/src/lib/log" + blob_models "github.com/goharbor/harbor/src/pkg/blob/models" + serror "github.com/goharbor/harbor/src/server/error" + "github.com/goharbor/harbor/src/server/middleware" + "net/http" +) + +// HeadBlobMiddleware intercept the head blob request +func HeadBlobMiddleware() func(http.Handler) http.Handler { + return middleware.New(func(rw http.ResponseWriter, req *http.Request, next http.Handler) { + if err := handleHead(req); err != nil { + serror.SendError(rw, err) + return + } + next.ServeHTTP(rw, req) + }) +} + +// handleHead ... +func handleHead(req *http.Request) error { + none := lib.ArtifactInfo{} + art := lib.GetArtifactInfo(req.Context()) + if art == none { + return errors.New("cannot get the artifact information from request context").WithCode(errors.NotFoundCode) + } + + bb, err := blob.Ctl.Get(req.Context(), art.Digest) + if err != nil { + return err + } + + switch bb.Status { + case blob_models.StatusNone, blob_models.StatusDelete: + bb.Status = blob_models.StatusNone + count, err := blob.Ctl.Touch(req.Context(), bb) + if err != nil { + log.Errorf("failed to update blob: %s status to None, error:%v", art.Digest, err) + return err + } + if count == 0 { + return errors.New("the asking blob is in GC, mark it as non existing").WithCode(errors.NotFoundCode) + } + case blob_models.StatusDeleting, blob_models.StatusDeleteFailed: + return errors.New("the asking blob is in GC, mark it as non existing").WithCode(errors.NotFoundCode) + default: + return errors.New(nil).WithMessage(fmt.Sprintf("wrong blob status, %s", bb.Status)) + } + return nil +} diff --git a/src/server/middleware/blob/head_blob_test.go b/src/server/middleware/blob/head_blob_test.go new file mode 100644 index 000000000..057fe592b --- /dev/null +++ b/src/server/middleware/blob/head_blob_test.go @@ -0,0 +1,88 @@ +package blob + +import ( + "fmt" + beego_orm "github.com/astaxie/beego/orm" + "github.com/goharbor/harbor/src/controller/blob" + "github.com/goharbor/harbor/src/lib" + "github.com/goharbor/harbor/src/lib/orm" + blob_models "github.com/goharbor/harbor/src/pkg/blob/models" + htesting "github.com/goharbor/harbor/src/testing" + "github.com/stretchr/testify/suite" + "net/http" + "net/http/httptest" + "testing" +) + +type HeadBlobUploadMiddlewareTestSuite struct { + htesting.Suite +} + +func (suite *HeadBlobUploadMiddlewareTestSuite) SetupSuite() { + suite.Suite.SetupSuite() + suite.Suite.ClearTables = []string{"blob"} +} + +func (suite *HeadBlobUploadMiddlewareTestSuite) makeRequest(projectName, digest string) *http.Request { + req := httptest.NewRequest("HEAD", fmt.Sprintf("/v2/%s/blobs/%s", projectName, digest), nil) + info := lib.ArtifactInfo{ + Repository: fmt.Sprintf("%s/photon", projectName), + Reference: "2.0", + Tag: "2.0", + Digest: digest, + } + *req = *(req.WithContext(orm.NewContext(req.Context(), beego_orm.NewOrm()))) + *req = *(req.WithContext(lib.WithArtifactInfo(req.Context(), info))) + return req +} + +func (suite *HeadBlobUploadMiddlewareTestSuite) TestHeadBlobStatusNone() { + suite.WithProject(func(projectID int64, projectName string) { + digest := suite.DigestString() + + _, err := blob.Ctl.Ensure(suite.Context(), digest, "application/octet-stream", 512) + suite.Nil(err) + + req := suite.makeRequest(projectName, digest) + res := httptest.NewRecorder() + next := suite.NextHandler(http.StatusOK, map[string]string{"Docker-Content-Digest": digest}) + HeadBlobMiddleware()(next).ServeHTTP(res, req) + suite.Equal(http.StatusOK, res.Code) + + blob, err := blob.Ctl.Get(suite.Context(), digest) + suite.Nil(err) + suite.Equal(digest, blob.Digest) + suite.Equal(blob_models.StatusNone, blob.Status) + }) +} + +func (suite *HeadBlobUploadMiddlewareTestSuite) TestHeadBlobStatusDeleting() { + suite.WithProject(func(projectID int64, projectName string) { + digest := suite.DigestString() + + id, err := blob.Ctl.Ensure(suite.Context(), digest, "application/octet-stream", 512) + suite.Nil(err) + + // status-none -> status-delete -> status-deleting + _, err = blob.Ctl.Touch(suite.Context(), &blob_models.Blob{ID: id, Status: blob_models.StatusDelete}) + suite.Nil(err) + _, err = blob.Ctl.Touch(suite.Context(), &blob_models.Blob{ID: id, Status: blob_models.StatusDeleting, Version: 1}) + suite.Nil(err) + + req := suite.NewRequest(http.MethodHead, fmt.Sprintf("/v2/%s/blobs/%s", projectName, digest), nil) + res := httptest.NewRecorder() + + next := suite.NextHandler(http.StatusOK, map[string]string{"Docker-Content-Digest": digest}) + HeadBlobMiddleware()(next).ServeHTTP(res, req) + suite.Equal(http.StatusNotFound, res.Code) + + blob, err := blob.Ctl.Get(suite.Context(), digest) + suite.Nil(err) + suite.Equal(digest, blob.Digest) + suite.Equal(blob_models.StatusDeleting, blob.Status) + }) +} + +func TestHeadBlobUploadMiddlewareTestSuite(t *testing.T) { + suite.Run(t, &HeadBlobUploadMiddlewareTestSuite{}) +} diff --git a/src/server/middleware/patterns.go b/src/server/middleware/patterns.go index a6a256e3c..c9558c1f5 100644 --- a/src/server/middleware/patterns.go +++ b/src/server/middleware/patterns.go @@ -22,7 +22,7 @@ var ( V2ManifestURLRe = regexp.MustCompile(fmt.Sprintf(`^/v2/(?P<%s>%s)/manifests/(?P<%s>%s|%s)$`, RepositorySubexp, reference.NameRegexp.String(), ReferenceSubexp, reference.TagRegexp.String(), digest.DigestRegexp.String())) // V2TagListURLRe is the regular expression for matching request to v2 handler to list tags V2TagListURLRe = regexp.MustCompile(fmt.Sprintf(`^/v2/(?P<%s>%s)/tags/list`, RepositorySubexp, reference.NameRegexp.String())) - // V2BlobURLRe is the regular expression for matching request to v2 handler to retrieve delete a blob + // V2BlobURLRe is the regular expression for matching request to v2 handler to retrieve head/delete a blob V2BlobURLRe = regexp.MustCompile(fmt.Sprintf(`^/v2/(?P<%s>%s)/blobs/(?P<%s>%s)$`, RepositorySubexp, reference.NameRegexp.String(), DigestSubexp, digest.DigestRegexp.String())) // V2BlobUploadURLRe is the regular expression for matching the request to v2 handler to upload a blob, the upload uuid currently is not put into a group V2BlobUploadURLRe = regexp.MustCompile(fmt.Sprintf(`^/v2/(?P<%s>%s)/blobs/uploads[/a-zA-Z0-9\-_\.=]*$`, RepositorySubexp, reference.NameRegexp.String())) diff --git a/src/server/registry/route.go b/src/server/registry/route.go index e6ccb8d8e..3b40a4048 100644 --- a/src/server/registry/route.go +++ b/src/server/registry/route.go @@ -85,6 +85,11 @@ func RegisterRoutes() { Middleware(quota.PutBlobUploadMiddleware()). Middleware(blob.PutBlobUploadMiddleware()). Handler(proxy) + root.NewRoute(). + Method(http.MethodHead). + Path("/*/blobs/:digest"). + Middleware(blob.HeadBlobMiddleware()). + Handler(proxy) // others root.NewRoute().Path("/*").Handler(proxy) } diff --git a/src/testing/controller/blob/controller.go b/src/testing/controller/blob/controller.go index ca23f8a27..1c16eb32f 100644 --- a/src/testing/controller/blob/controller.go +++ b/src/testing/controller/blob/controller.go @@ -12,8 +12,6 @@ import ( mock "github.com/stretchr/testify/mock" models "github.com/goharbor/harbor/src/pkg/blob/models" - - time "time" ) // Controller is an autogenerated mock type for the Controller type @@ -84,6 +82,20 @@ func (_m *Controller) CalculateTotalSizeByProject(ctx context.Context, projectID return r0, r1 } +// Delete provides a mock function with given fields: ctx, id +func (_m *Controller) Delete(ctx context.Context, id int64) error { + ret := _m.Called(ctx, id) + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, int64) error); ok { + r0 = rf(ctx, id) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // Ensure provides a mock function with given fields: ctx, digest, contentType, size func (_m *Controller) Ensure(ctx context.Context, digest string, contentType string, size int64) (int64, error) { ret := _m.Called(ctx, digest, contentType, size) @@ -230,20 +242,6 @@ func (_m *Controller) List(ctx context.Context, params models.ListParams) ([]*mo return r0, r1 } -// ReFreshUpdateTime provides a mock function with given fields: ctx, digest, _a2 -func (_m *Controller) ReFreshUpdateTime(ctx context.Context, digest string, _a2 time.Time) error { - ret := _m.Called(ctx, digest, _a2) - - var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string, time.Time) error); ok { - r0 = rf(ctx, digest, _a2) - } else { - r0 = ret.Error(0) - } - - return r0 -} - // SetAcceptedBlobSize provides a mock function with given fields: sessionID, size func (_m *Controller) SetAcceptedBlobSize(sessionID string, size int64) error { ret := _m.Called(sessionID, size) @@ -271,3 +269,38 @@ func (_m *Controller) Sync(ctx context.Context, references []distribution.Descri return r0 } + +// Touch provides a mock function with given fields: ctx, _a1 +func (_m *Controller) Touch(ctx context.Context, _a1 *models.Blob) (int64, error) { + ret := _m.Called(ctx, _a1) + + var r0 int64 + if rf, ok := ret.Get(0).(func(context.Context, *models.Blob) int64); ok { + r0 = rf(ctx, _a1) + } else { + r0 = ret.Get(0).(int64) + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, *models.Blob) error); ok { + r1 = rf(ctx, _a1) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// Update provides a mock function with given fields: ctx, _a1 +func (_m *Controller) Update(ctx context.Context, _a1 *models.Blob) error { + ret := _m.Called(ctx, _a1) + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, *models.Blob) error); ok { + r0 = rf(ctx, _a1) + } else { + r0 = ret.Error(0) + } + + return r0 +} diff --git a/src/testing/pkg/blob/manager.go b/src/testing/pkg/blob/manager.go index e2cc96d3c..8b2fac699 100644 --- a/src/testing/pkg/blob/manager.go +++ b/src/testing/pkg/blob/manager.go @@ -7,8 +7,6 @@ import ( models "github.com/goharbor/harbor/src/pkg/blob/models" mock "github.com/stretchr/testify/mock" - - time "time" ) // Manager is an autogenerated mock type for the Manager type @@ -188,20 +186,6 @@ func (_m *Manager) List(ctx context.Context, params models.ListParams) ([]*model return r0, r1 } -// ReFreshUpdateTime provides a mock function with given fields: ctx, digest, _a2 -func (_m *Manager) ReFreshUpdateTime(ctx context.Context, digest string, _a2 time.Time) error { - ret := _m.Called(ctx, digest, _a2) - - var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string, time.Time) error); ok { - r0 = rf(ctx, digest, _a2) - } else { - r0 = ret.Error(0) - } - - return r0 -} - // Update provides a mock function with given fields: ctx, _a1 func (_m *Manager) Update(ctx context.Context, _a1 *models.Blob) error { ret := _m.Called(ctx, _a1) @@ -215,3 +199,24 @@ func (_m *Manager) Update(ctx context.Context, _a1 *models.Blob) error { return r0 } + +// UpdateBlobStatus provides a mock function with given fields: ctx, _a1 +func (_m *Manager) UpdateBlobStatus(ctx context.Context, _a1 *models.Blob) (int64, error) { + ret := _m.Called(ctx, _a1) + + var r0 int64 + if rf, ok := ret.Get(0).(func(context.Context, *models.Blob) int64); ok { + r0 = rf(ctx, _a1) + } else { + r0 = ret.Get(0).(int64) + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, *models.Blob) error); ok { + r1 = rf(ctx, _a1) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} From 446739f967f8f36e012bec62e45f842a04822af8 Mon Sep 17 00:00:00 2001 From: wang yan Date: Mon, 22 Jun 2020 14:52:44 +0800 Subject: [PATCH 3/4] rebase with latest source code Signed-off-by: wang yan --- src/server/middleware/blob/head_blob.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/server/middleware/blob/head_blob.go b/src/server/middleware/blob/head_blob.go index 50123cf0f..ca05725b5 100644 --- a/src/server/middleware/blob/head_blob.go +++ b/src/server/middleware/blob/head_blob.go @@ -5,9 +5,9 @@ import ( "github.com/goharbor/harbor/src/controller/blob" "github.com/goharbor/harbor/src/lib" "github.com/goharbor/harbor/src/lib/errors" + lib_http "github.com/goharbor/harbor/src/lib/http" "github.com/goharbor/harbor/src/lib/log" blob_models "github.com/goharbor/harbor/src/pkg/blob/models" - serror "github.com/goharbor/harbor/src/server/error" "github.com/goharbor/harbor/src/server/middleware" "net/http" ) @@ -16,7 +16,7 @@ import ( func HeadBlobMiddleware() func(http.Handler) http.Handler { return middleware.New(func(rw http.ResponseWriter, req *http.Request, next http.Handler) { if err := handleHead(req); err != nil { - serror.SendError(rw, err) + lib_http.SendError(rw, err) return } next.ServeHTTP(rw, req) From 0e175017aa9c31c6a68f94b0e3e327acf26d2e8a Mon Sep 17 00:00:00 2001 From: wang yan Date: Tue, 23 Jun 2020 13:00:29 +0800 Subject: [PATCH 4/4] continue updating code per review comments Signed-off-by: wang yan --- src/controller/blob/controller.go | 17 +++++++++++---- src/controller/blob/controller_test.go | 21 +++++++++++++------ src/pkg/blob/dao/dao.go | 17 +++++++++++---- src/pkg/blob/dao/dao_test.go | 4 ++-- src/server/middleware/blob/head_blob.go | 22 +++++++++----------- src/server/middleware/blob/head_blob_test.go | 5 +++-- src/testing/controller/blob/controller.go | 17 +++++---------- 7 files changed, 61 insertions(+), 42 deletions(-) diff --git a/src/controller/blob/controller.go b/src/controller/blob/controller.go index 817a2912c..2444cc59a 100644 --- a/src/controller/blob/controller.go +++ b/src/controller/blob/controller.go @@ -24,6 +24,7 @@ import ( "github.com/goharbor/harbor/src/lib/log" "github.com/goharbor/harbor/src/lib/orm" "github.com/goharbor/harbor/src/pkg/blob" + blob_models "github.com/goharbor/harbor/src/pkg/blob/models" ) var ( @@ -74,8 +75,8 @@ type Controller interface { // GetAcceptedBlobSize returns the accepted size of stream upload blob. GetAcceptedBlobSize(sessionID string) (int64, error) - // Touch updates the blob status and increase version every time. - Touch(ctx context.Context, blob *blob.Blob) (int64, error) + // Touch updates the blob status to StatusNone and increase version every time. + Touch(ctx context.Context, blob *blob.Blob) error // Update updates the blob, it cannot handle blob status transitions. Update(ctx context.Context, blob *blob.Blob) error @@ -322,8 +323,16 @@ func (c *controller) GetAcceptedBlobSize(sessionID string) (int64, error) { return size, nil } -func (c *controller) Touch(ctx context.Context, blob *blob.Blob) (int64, error) { - return c.blobMgr.UpdateBlobStatus(ctx, blob) +func (c *controller) Touch(ctx context.Context, blob *blob.Blob) error { + blob.Status = blob_models.StatusNone + count, err := c.blobMgr.UpdateBlobStatus(ctx, blob) + if err != nil { + return err + } + if count == 0 { + return errors.New(nil).WithMessage(fmt.Sprintf("no blob item is updated to StatusNone, id:%d, digest:%s", blob.ID, blob.Digest)).WithCode(errors.NotFoundCode) + } + return nil } func (c *controller) Update(ctx context.Context, blob *blob.Blob) error { diff --git a/src/controller/blob/controller_test.go b/src/controller/blob/controller_test.go index e33c2c331..b647cfec7 100644 --- a/src/controller/blob/controller_test.go +++ b/src/controller/blob/controller_test.go @@ -17,6 +17,7 @@ package blob import ( "context" "fmt" + "github.com/goharbor/harbor/src/lib/errors" "github.com/goharbor/harbor/src/pkg/blob/models" "testing" @@ -28,6 +29,8 @@ import ( blobtesting "github.com/goharbor/harbor/src/testing/pkg/blob" "github.com/google/uuid" "github.com/stretchr/testify/suite" + + pkg_blob "github.com/goharbor/harbor/src/pkg/blob" ) type ControllerTestSuite struct { @@ -268,19 +271,25 @@ func (suite *ControllerTestSuite) TestGetSetAcceptedBlobSize() { suite.Equal(int64(100), size) } -func (suite *ControllerTestSuite) TestUpdateStatus() { +func (suite *ControllerTestSuite) TestTouch() { ctx := suite.Context() + err := Ctl.Touch(ctx, &blob.Blob{ + Status: models.StatusNone, + }) + suite.NotNil(err) + suite.True(errors.IsNotFoundErr(err)) + digest := suite.prepareBlob() blob, err := Ctl.Get(ctx, digest) suite.Nil(err) - - suite.Equal(blob.Status, models.StatusNone) blob.Status = models.StatusDelete - count, err := Ctl.Touch(ctx, blob) + _, err = pkg_blob.Mgr.UpdateBlobStatus(suite.Context(), blob) suite.Nil(err) - suite.Equal(blob.Status, models.StatusDelete) - suite.Equal(int64(1), count) + + err = Ctl.Touch(ctx, blob) + suite.Nil(err) + suite.Equal(blob.Status, models.StatusNone) } func (suite *ControllerTestSuite) TestDelete() { diff --git a/src/pkg/blob/dao/dao.go b/src/pkg/blob/dao/dao.go index b2b5dd6ff..08a50d1f7 100644 --- a/src/pkg/blob/dao/dao.go +++ b/src/pkg/blob/dao/dao.go @@ -18,6 +18,7 @@ import ( "context" "fmt" "github.com/goharbor/harbor/src/lib/errors" + "github.com/goharbor/harbor/src/lib/log" "strings" "time" @@ -200,20 +201,28 @@ func (d *dao) UpdateBlobStatus(ctx context.Context, blob *models.Blob) (int64, e WHERE "id" IN ( SELECT T0."id" FROM "blob" T0 WHERE T0."version" >= $4 AND T0."id" = $5 AND T0."status" IN ('delete', 'deleting') ) */ - return qt.SetCond(c).Filter("id", blob.ID). + count, err := qt.SetCond(c).Filter("id", blob.ID). Filter("status__in", models.StatusMap[blob.Status]). Update(data) + if err != nil { + return count, err + } + if count == 0 { + log.Warningf("no blob is updated according to query condition, id: %d, status_in, %v", blob.ID, models.StatusMap[blob.Status]) + return 0, nil + } + return count, nil } -// UpdateBlob cannot handle the status change. +// UpdateBlob cannot handle the status change and version increase, for handling blob status change, please call +// for the UpdateBlobStatus. func (d *dao) UpdateBlob(ctx context.Context, blob *models.Blob) error { o, err := orm.FromContext(ctx) if err != nil { return err } - blob.Version = blob.Version + 1 blob.UpdateTime = time.Now() - _, err = o.Update(blob, "size", "content_type", "version", "update_time") + _, err = o.Update(blob, "size", "content_type", "update_time") return err } diff --git a/src/pkg/blob/dao/dao_test.go b/src/pkg/blob/dao/dao_test.go index 023394ab6..e2bce29a1 100644 --- a/src/pkg/blob/dao/dao_test.go +++ b/src/pkg/blob/dao/dao_test.go @@ -139,7 +139,7 @@ func (suite *DaoTestSuite) TestUpdateBlob() { blob, err := suite.dao.GetBlobByDigest(ctx, digest) if suite.Nil(err) { suite.Equal(int64(100), blob.Size) - suite.Equal(int64(1), blob.Version) + suite.Equal(int64(0), blob.Version) } } @@ -147,7 +147,7 @@ func (suite *DaoTestSuite) TestUpdateBlob() { suite.Nil(suite.dao.UpdateBlob(ctx, blob), "cannot be updated.") blob, err = suite.dao.GetBlobByDigest(ctx, digest) if suite.Nil(err) { - suite.Equal(int64(2), blob.Version) + suite.Equal(int64(0), blob.Version) suite.Equal(models.StatusNone, blob.Status) } } diff --git a/src/server/middleware/blob/head_blob.go b/src/server/middleware/blob/head_blob.go index ca05725b5..b791a9f4f 100644 --- a/src/server/middleware/blob/head_blob.go +++ b/src/server/middleware/blob/head_blob.go @@ -9,6 +9,7 @@ import ( "github.com/goharbor/harbor/src/lib/log" blob_models "github.com/goharbor/harbor/src/pkg/blob/models" "github.com/goharbor/harbor/src/server/middleware" + "github.com/goharbor/harbor/src/server/middleware/requestid" "net/http" ) @@ -26,29 +27,26 @@ func HeadBlobMiddleware() func(http.Handler) http.Handler { // handleHead ... func handleHead(req *http.Request) error { none := lib.ArtifactInfo{} - art := lib.GetArtifactInfo(req.Context()) - if art == none { - return errors.New("cannot get the artifact information from request context").WithCode(errors.NotFoundCode) + // for head blob, the GetArtifactInfo is actually get the information of blob. + blobInfo := lib.GetArtifactInfo(req.Context()) + if blobInfo == none { + return errors.New("cannot get the blob information from request context").WithCode(errors.NotFoundCode) } - bb, err := blob.Ctl.Get(req.Context(), art.Digest) + bb, err := blob.Ctl.Get(req.Context(), blobInfo.Digest) if err != nil { return err } switch bb.Status { case blob_models.StatusNone, blob_models.StatusDelete: - bb.Status = blob_models.StatusNone - count, err := blob.Ctl.Touch(req.Context(), bb) + err := blob.Ctl.Touch(req.Context(), bb) if err != nil { - log.Errorf("failed to update blob: %s status to None, error:%v", art.Digest, err) - return err - } - if count == 0 { - return errors.New("the asking blob is in GC, mark it as non existing").WithCode(errors.NotFoundCode) + log.Errorf("failed to update blob: %s status to StatusNone, error:%v", blobInfo.Digest, err) + return errors.Wrapf(err, fmt.Sprintf("the request id is: %s", req.Header.Get(requestid.HeaderXRequestID))) } case blob_models.StatusDeleting, blob_models.StatusDeleteFailed: - return errors.New("the asking blob is in GC, mark it as non existing").WithCode(errors.NotFoundCode) + return errors.New(nil).WithMessage(fmt.Sprintf("the asking blob is in GC, mark it as non existing, request id: %s", req.Header.Get(requestid.HeaderXRequestID))).WithCode(errors.NotFoundCode) default: return errors.New(nil).WithMessage(fmt.Sprintf("wrong blob status, %s", bb.Status)) } diff --git a/src/server/middleware/blob/head_blob_test.go b/src/server/middleware/blob/head_blob_test.go index 057fe592b..19787c8f0 100644 --- a/src/server/middleware/blob/head_blob_test.go +++ b/src/server/middleware/blob/head_blob_test.go @@ -6,6 +6,7 @@ import ( "github.com/goharbor/harbor/src/controller/blob" "github.com/goharbor/harbor/src/lib" "github.com/goharbor/harbor/src/lib/orm" + pkg_blob "github.com/goharbor/harbor/src/pkg/blob" blob_models "github.com/goharbor/harbor/src/pkg/blob/models" htesting "github.com/goharbor/harbor/src/testing" "github.com/stretchr/testify/suite" @@ -64,9 +65,9 @@ func (suite *HeadBlobUploadMiddlewareTestSuite) TestHeadBlobStatusDeleting() { suite.Nil(err) // status-none -> status-delete -> status-deleting - _, err = blob.Ctl.Touch(suite.Context(), &blob_models.Blob{ID: id, Status: blob_models.StatusDelete}) + _, err = pkg_blob.Mgr.UpdateBlobStatus(suite.Context(), &blob_models.Blob{ID: id, Status: blob_models.StatusDelete}) suite.Nil(err) - _, err = blob.Ctl.Touch(suite.Context(), &blob_models.Blob{ID: id, Status: blob_models.StatusDeleting, Version: 1}) + _, err = pkg_blob.Mgr.UpdateBlobStatus(suite.Context(), &blob_models.Blob{ID: id, Status: blob_models.StatusDeleting, Version: 1}) suite.Nil(err) req := suite.NewRequest(http.MethodHead, fmt.Sprintf("/v2/%s/blobs/%s", projectName, digest), nil) diff --git a/src/testing/controller/blob/controller.go b/src/testing/controller/blob/controller.go index 1c16eb32f..d0bb69743 100644 --- a/src/testing/controller/blob/controller.go +++ b/src/testing/controller/blob/controller.go @@ -271,24 +271,17 @@ func (_m *Controller) Sync(ctx context.Context, references []distribution.Descri } // Touch provides a mock function with given fields: ctx, _a1 -func (_m *Controller) Touch(ctx context.Context, _a1 *models.Blob) (int64, error) { +func (_m *Controller) Touch(ctx context.Context, _a1 *models.Blob) error { ret := _m.Called(ctx, _a1) - var r0 int64 - if rf, ok := ret.Get(0).(func(context.Context, *models.Blob) int64); ok { + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, *models.Blob) error); ok { r0 = rf(ctx, _a1) } else { - r0 = ret.Get(0).(int64) + r0 = ret.Error(0) } - var r1 error - if rf, ok := ret.Get(1).(func(context.Context, *models.Blob) error); ok { - r1 = rf(ctx, _a1) - } else { - r1 = ret.Error(1) - } - - return r0, r1 + return r0 } // Update provides a mock function with given fields: ctx, _a1