diff --git a/src/pkg/accessory/dao/dao.go b/src/pkg/accessory/dao/dao.go index 651871f8a55..96e63e05ba8 100644 --- a/src/pkg/accessory/dao/dao.go +++ b/src/pkg/accessory/dao/dao.go @@ -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 { diff --git a/src/pkg/accessory/dao/dao_test.go b/src/pkg/accessory/dao/dao_test.go index 4a23a7b3046..06e9ae2cccf 100644 --- a/src/pkg/accessory/dao/dao_test.go +++ b/src/pkg/accessory/dao/dao_test.go @@ -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 diff --git a/src/pkg/accessory/manager.go b/src/pkg/accessory/manager.go index 6c1d68d276d..0c02908d571 100644 --- a/src/pkg/accessory/manager.go +++ b/src/pkg/accessory/manager.go @@ -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) } diff --git a/src/pkg/accessory/manager_test.go b/src/pkg/accessory/manager_test.go index 7e436ce46f4..a06bb5ccd01 100644 --- a/src/pkg/accessory/manager_test.go +++ b/src/pkg/accessory/manager_test.go @@ -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) diff --git a/src/server/middleware/subject/subject.go b/src/server/middleware/subject/subject.go index b285891e70a..6a3c447fc54 100644 --- a/src/server/middleware/subject/subject.go +++ b/src/server/middleware/subject/subject.go @@ -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" @@ -147,6 +149,34 @@ func Middleware() func(http.Handler) http.Handler { } } w.Header().Set("OCI-Subject", subjectArt.Digest) + } 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, "SubjectArtifactRepo": info.Repository})) + 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 diff --git a/src/server/middleware/subject/subject_test.go b/src/server/middleware/subject/subject_test.go index cd8a585ac63..907fb649a3a 100644 --- a/src/server/middleware/subject/subject_test.go +++ b/src/server/middleware/subject/subject_test.go @@ -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,8 @@ 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, + SubArtifactRepo: repositoryName, Digest: accdgt, Type: accessorymodel.TypeSubject, }) @@ -165,6 +190,40 @@ 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, + "SubjectArtifactRepo": name, + }, + }) + 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 +233,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()}) diff --git a/src/testing/pkg/accessory/dao/dao.go b/src/testing/pkg/accessory/dao/dao.go index a59eb8a011e..ad5bc393724 100644 --- a/src/testing/pkg/accessory/dao/dao.go +++ b/src/testing/pkg/accessory/dao/dao.go @@ -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()) diff --git a/src/testing/pkg/accessory/manager.go b/src/testing/pkg/accessory/manager.go index fd9ebb99f9e..8d16162f78a 100644 --- a/src/testing/pkg/accessory/manager.go +++ b/src/testing/pkg/accessory/manager.go @@ -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())