support accessory in either order (#19375)

In certain cases, the OCI client may push the subject artifact and accessory in either order.
Therefore, it is necessary to handle situations where the client pushes the accessory ahead of the subject artifact.

Signed-off-by: wang yan <wangyan@vmware.com>
This commit is contained in:
Wang Yan 2023-09-23 02:42:58 +08:00 committed by GitHub
parent 6fd4a2bd37
commit 3338f27189
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 182 additions and 16 deletions

View File

@ -32,6 +32,8 @@ type DAO interface {
Get(ctx context.Context, id int64) (accessory *Accessory, err error)
// Create the accessory
Create(ctx context.Context, accessory *Accessory) (id int64, err error)
// Update the accessory
Update(ctx context.Context, accessory *Accessory) error
// Delete the accessory specified by ID
Delete(ctx context.Context, id int64) (err error)
// DeleteAccessories deletes accessories by query
@ -103,6 +105,21 @@ func (d *dao) Create(ctx context.Context, acc *Accessory) (int64, error) {
return id, err
}
func (d *dao) Update(ctx context.Context, acc *Accessory) error {
ormer, err := orm.FromContext(ctx)
if err != nil {
return err
}
n, err := ormer.Update(acc, "SubjectArtifactID")
if n == 0 {
if e := orm.AsConflictError(err, "accessory %s already exists", acc.Digest); e != nil {
err = e
}
return err
}
return err
}
func (d *dao) Delete(ctx context.Context, id int64) error {
ormer, err := orm.FromContext(ctx)
if err != nil {

View File

@ -169,6 +169,19 @@ func (d *daoTestSuite) TestCreate() {
d.True(errors.IsErr(err, errors.ConflictCode))
}
func (d *daoTestSuite) TestUpdate() {
acc := &Accessory{
ID: d.accID,
SubjectArtifactID: 333,
}
err := d.dao.Update(d.ctx, acc)
d.Require().Nil(err)
accAfter, err := d.dao.Get(d.ctx, d.accID)
d.Require().Nil(err)
d.Require().Equal(int64(333), accAfter.SubjectArtifactID)
}
func (d *daoTestSuite) TestDelete() {
// happy pass is covered in TearDown

View File

@ -48,6 +48,8 @@ type Manager interface {
List(ctx context.Context, query *q.Query) (accs []model.Accessory, err error)
// Create the accessory and returns the ID
Create(ctx context.Context, accessory model.AccessoryData) (id int64, err error)
// Update the accessory
Update(ctx context.Context, accessory model.AccessoryData) error
// Delete the accessory specified by ID
Delete(ctx context.Context, id int64) (err error)
// DeleteAccessories deletes accessories according to the query
@ -150,6 +152,14 @@ func (m *manager) Create(ctx context.Context, accessory model.AccessoryData) (in
return m.dao.Create(ctx, acc)
}
func (m *manager) Update(ctx context.Context, accessory model.AccessoryData) error {
acc := &dao.Accessory{
ID: accessory.ID,
SubjectArtifactID: accessory.SubArtifactID,
}
return m.dao.Update(ctx, acc)
}
func (m *manager) Delete(ctx context.Context, id int64) error {
return m.dao.Delete(ctx, id)
}

View File

@ -87,6 +87,16 @@ func (m *managerTestSuite) TestCreate() {
m.dao.AssertExpectations(m.T())
}
func (m *managerTestSuite) TestUpdate() {
mock.OnAnything(m.dao, "Update").Return(nil)
err := m.mgr.Update(nil, model.AccessoryData{
ID: 1,
SubArtifactID: 2,
})
m.Require().Nil(err)
m.dao.AssertExpectations(m.T())
}
func (m *managerTestSuite) TestDelete() {
mock.OnAnything(m.dao, "Delete").Return(nil)
err := m.mgr.Delete(nil, 1)

View File

@ -21,6 +21,7 @@ import (
"net/http"
"github.com/docker/distribution/manifest/schema2"
"github.com/opencontainers/go-digest"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/goharbor/harbor/src/controller/artifact"
@ -28,6 +29,7 @@ import (
"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/lib/q"
"github.com/goharbor/harbor/src/pkg/accessory"
"github.com/goharbor/harbor/src/pkg/accessory/model"
"github.com/goharbor/harbor/src/server/middleware"
@ -146,9 +148,38 @@ func Middleware() func(http.Handler) http.Handler {
return err
}
}
// when subject artifact is pushed after accessory artifact, current subject artifact do not exist.
// so we use reference manifest subject digest instead of subjectArt.Digest
w.Header().Set("OCI-Subject", mf.Subject.Digest.String())
} else {
// In certain cases, the OCI client may push the subject artifact and accessory in either order.
// Therefore, it is necessary to handle situations where the client pushes the accessory ahead of the subject artifact.
digest := digest.FromBytes(body)
accs, err := accessory.Mgr.List(ctx, q.New(q.KeyWords{"SubjectArtifactDigest": digest}))
if err != nil {
logger.Errorf("failed to list accessory artifact: %s, error: %v", digest, err)
return err
}
if len(accs) <= 0 {
return nil
}
art, err := artifact.Ctl.GetByReference(ctx, info.Repository, digest.String(), nil)
if err != nil {
logger.Errorf("failed to list artifact: %s, error: %v", digest, err)
return err
}
if art != nil {
for _, acc := range accs {
accData := model.AccessoryData{
ID: acc.GetData().ID,
SubArtifactID: art.ID,
}
if err := accessory.Mgr.Update(ctx, accData); err != nil {
return err
}
}
}
}
return nil

View File

@ -37,7 +37,7 @@ func (suite *MiddlewareTestSuite) SetupTest() {
func (suite *MiddlewareTestSuite) TearDownTest() {
}
func (suite *MiddlewareTestSuite) prepare(name, subject string) (distribution.Manifest, distribution.Descriptor, *http.Request) {
func (suite *MiddlewareTestSuite) prepare(name, digset string, withoutSub ...bool) (distribution.Manifest, distribution.Descriptor, *http.Request) {
body := fmt.Sprintf(`
{
"schemaVersion":2,
@ -61,7 +61,29 @@ func (suite *MiddlewareTestSuite) prepare(name, subject string) (distribution.Ma
"mediaType":"application/vnd.oci.image.manifest.v1+json",
"size":419,
"digest":"%s"
}}`, subject)
}}`, digset)
if len(withoutSub) > 0 && withoutSub[0] {
body = fmt.Sprintf(`
{
"schemaVersion":2,
"mediaType":"application/vnd.oci.image.manifest.v1+json",
"config":{
"mediaType":"application/vnd.example.sbom",
"size":2,
"digest":"%s"
},
"layers":[
{
"mediaType":"application/vnd.example.sbom.text",
"size":37,
"digest":"sha256:45592a729ef6884ea3297e9510d79104f27aeef5f4919b3a921e3abb7f469709"
}
],
"annotations":{
"org.example.sbom.format":"text"
}}`, digset)
}
manifest, descriptor, err := distribution.UnmarshalManifest("application/vnd.oci.image.manifest.v1+json", []byte(body))
suite.Nil(err)
@ -94,19 +116,21 @@ func (suite *MiddlewareTestSuite) addArt(pid, repositoryID int64, repositoryName
return afid
}
func (suite *MiddlewareTestSuite) addArtAcc(pid, repositoryID int64, repositoryName, dgt, accdgt string) int64 {
subaf := &artifact.Artifact{
Type: "Docker-Image",
ProjectID: pid,
RepositoryID: repositoryID,
RepositoryName: repositoryName,
Digest: dgt,
Size: 1024,
PushTime: time.Now(),
PullTime: time.Now(),
func (suite *MiddlewareTestSuite) addArtAcc(pid, repositoryID int64, repositoryName, dgt, accdgt string, createSub ...bool) int64 {
if len(createSub) > 0 && createSub[0] {
subaf := &artifact.Artifact{
Type: "Docker-Image",
ProjectID: pid,
RepositoryID: repositoryID,
RepositoryName: repositoryName,
Digest: dgt,
Size: 1024,
PushTime: time.Now(),
PullTime: time.Now(),
}
_, err := pkg.ArtifactMgr.Create(suite.Context(), subaf)
suite.Nil(err, fmt.Sprintf("Add artifact failed for %d", repositoryID))
}
_, err := pkg.ArtifactMgr.Create(suite.Context(), subaf)
suite.Nil(err, fmt.Sprintf("Add artifact failed for %d", repositoryID))
af := &artifact.Artifact{
Type: "Subject",
@ -124,7 +148,7 @@ func (suite *MiddlewareTestSuite) addArtAcc(pid, repositoryID int64, repositoryN
accid, err := accessory.Mgr.Create(suite.Context(), accessorymodel.AccessoryData{
ID: 1,
ArtifactID: afid,
SubArtifactDigest: subaf.Digest,
SubArtifactDigest: dgt,
Digest: accdgt,
Type: accessorymodel.TypeSubject,
})
@ -165,6 +189,39 @@ func (suite *MiddlewareTestSuite) TestSubject() {
})
}
func (suite *MiddlewareTestSuite) TestSubjectAfterAcc() {
// add acc, with subject digest
// add subject
suite.WithProject(func(projectID int64, projectName string) {
name := fmt.Sprintf("%s/hello-world", projectName)
_, repoId, err := repository.Ctl.Ensure(suite.Context(), name)
_, descriptor, req := suite.prepare(name, suite.DigestString(), true)
suite.Nil(err)
subArtDigest := descriptor.Digest.String()
accArtDigest := suite.DigestString()
accID := suite.addArtAcc(projectID, repoId, name, subArtDigest, accArtDigest)
subArtID := suite.addArt(projectID, repoId, name, subArtDigest)
res := httptest.NewRecorder()
next := suite.NextHandler(http.StatusCreated, map[string]string{"Docker-Content-Digest": subArtDigest})
Middleware()(next).ServeHTTP(res, req)
suite.Equal(http.StatusCreated, res.Code)
accs, err := accessory.Mgr.List(suite.Context(), &q.Query{
Keywords: map[string]interface{}{
"SubjectArtifactDigest": subArtDigest,
},
})
suite.Equal(1, len(accs))
suite.Equal(subArtDigest, accs[0].GetData().SubArtifactDigest)
suite.Equal(subArtID, accs[0].GetData().SubArtifactID)
suite.Equal(accID, accs[0].GetData().ID)
})
}
func (suite *MiddlewareTestSuite) TestSubjectDup() {
suite.WithProject(func(projectID int64, projectName string) {
name := fmt.Sprintf("%s/hello-world", projectName)
@ -174,7 +231,7 @@ func (suite *MiddlewareTestSuite) TestSubjectDup() {
_, descriptor, req := suite.prepare(name, subArtDigest)
suite.Nil(err)
accID := suite.addArtAcc(projectID, repoId, name, subArtDigest, descriptor.Digest.String())
accID := suite.addArtAcc(projectID, repoId, name, subArtDigest, descriptor.Digest.String(), true)
res := httptest.NewRecorder()
next := suite.NextHandler(http.StatusCreated, map[string]string{"Docker-Content-Digest": descriptor.Digest.String()})

View File

@ -154,6 +154,20 @@ func (_m *DAO) List(ctx context.Context, query *q.Query) ([]*dao.Accessory, erro
return r0, r1
}
// Update provides a mock function with given fields: ctx, accessory
func (_m *DAO) Update(ctx context.Context, accessory *dao.Accessory) error {
ret := _m.Called(ctx, accessory)
var r0 error
if rf, ok := ret.Get(0).(func(context.Context, *dao.Accessory) error); ok {
r0 = rf(ctx, accessory)
} else {
r0 = ret.Error(0)
}
return r0
}
type mockConstructorTestingTNewDAO interface {
mock.TestingT
Cleanup(func())

View File

@ -158,6 +158,20 @@ func (_m *Manager) List(ctx context.Context, query *q.Query) ([]model.Accessory,
return r0, r1
}
// Update provides a mock function with given fields: ctx, _a1
func (_m *Manager) Update(ctx context.Context, _a1 model.AccessoryData) error {
ret := _m.Called(ctx, _a1)
var r0 error
if rf, ok := ret.Get(0).(func(context.Context, model.AccessoryData) error); ok {
r0 = rf(ctx, _a1)
} else {
r0 = ret.Error(0)
}
return r0
}
type mockConstructorTestingTNewManager interface {
mock.TestingT
Cleanup(func())