support accessor in either order

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 <[email protected]>
goharbor · Sep 21, 2023 · 9cb3e52 · 9cb3e52
1 parent 4051b2b
commit 9cb3e52
Show file tree

Hide file tree

Showing 8 changed files with 182 additions and 17 deletions.
diff --git a/src/pkg/accessory/dao/dao.go b/src/pkg/accessory/dao/dao.go
@@ -16,7 +16,7 @@ package dao
 
 import (
 	"context"
-
+	
 	"github.com/goharbor/harbor/src/lib/errors"
 	"github.com/goharbor/harbor/src/lib/orm"
 	"github.com/goharbor/harbor/src/lib/q"
@@ -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
@@ -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
@@ -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
@@ -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
@@ -21,13 +21,15 @@ 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"
 	"github.com/goharbor/harbor/src/lib"
 	"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}))
+			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
@@ -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()})

diff --git a/src/testing/pkg/accessory/dao/dao.go b/src/testing/pkg/accessory/dao/dao.go
diff --git a/src/testing/pkg/accessory/manager.go b/src/testing/pkg/accessory/manager.go