From 2f380495bfb3535442eb72e41a59d115043c211c Mon Sep 17 00:00:00 2001 From: Wang Yan Date: Wed, 22 Mar 2023 10:58:30 +0800 Subject: [PATCH] revert subject id in the accessory (#18377) DO not replact id with digest and just add digest into the accessory table Signed-off-by: Wang Yan --- api/v2.0/swagger.yaml | 4 +++ .../postgresql/0110_2.8.0_schema.up.sql | 3 +-- src/controller/artifact/controller.go | 4 +-- src/controller/artifact/controller_test.go | 2 +- src/pkg/accessory/dao/dao.go | 3 --- src/pkg/accessory/dao/model.go | 1 + src/pkg/accessory/manager.go | 8 ++++-- src/pkg/accessory/manager_test.go | 2 +- src/pkg/accessory/model/accessory.go | 3 ++- src/pkg/accessory/model/base/base_test.go | 4 ++- src/server/middleware/cosign/cosign.go | 26 ++++++++++++------- src/server/middleware/subject/subject.go | 26 ++++++++++++------- src/server/v2.0/handler/artifact.go | 2 +- src/server/v2.0/handler/model/accessory.go | 1 + src/testing/pkg/accessory/manager.go | 10 +++---- 15 files changed, 60 insertions(+), 39 deletions(-) diff --git a/api/v2.0/swagger.yaml b/api/v2.0/swagger.yaml index 249325d0f..319b9303e 100644 --- a/api/v2.0/swagger.yaml +++ b/api/v2.0/swagger.yaml @@ -9382,6 +9382,10 @@ definitions: format: int64 description: The artifact id of the accessory x-omitempty: false + subject_artifact_id: + type: integer + format: int64 + description: The subject artifact id of the accessory subject_artifact_digest: type: string description: The subject artifact digest of the accessory diff --git a/make/migrations/postgresql/0110_2.8.0_schema.up.sql b/make/migrations/postgresql/0110_2.8.0_schema.up.sql index 9f75ce8cb..bb5869f9b 100644 --- a/make/migrations/postgresql/0110_2.8.0_schema.up.sql +++ b/make/migrations/postgresql/0110_2.8.0_schema.up.sql @@ -1,7 +1,7 @@ /* remove the redundant data from table artifact_blob */ delete from artifact_blob afb where not exists (select digest from blob b where b.digest = afb.digest_af); -/* replace subject_artifact_id with subject_artifact_digest*/ +/* add subject_artifact_digest*/ alter table artifact_accessory add column subject_artifact_digest varchar(1024); DO $$ @@ -19,7 +19,6 @@ END $$; alter table artifact_accessory drop CONSTRAINT artifact_accessory_subject_artifact_id_fkey; alter table artifact_accessory drop CONSTRAINT unique_artifact_accessory; alter table artifact_accessory add CONSTRAINT unique_artifact_accessory UNIQUE (artifact_id, subject_artifact_digest); -alter table artifact_accessory drop column subject_artifact_id; /* Update the registry and replication policy associated with the chartmuseum */ UPDATE registry diff --git a/src/controller/artifact/controller.go b/src/controller/artifact/controller.go index 51edb55bf..040ef2c80 100644 --- a/src/controller/artifact/controller.go +++ b/src/controller/artifact/controller.go @@ -161,7 +161,7 @@ func (c *controller) Ensure(ctx context.Context, repository, digest string, opti } } for _, acc := range option.Accs { - if err = c.accessoryMgr.Ensure(ctx, artifact.Digest, acc.ArtifactID, acc.Size, acc.Digest, acc.Type); err != nil { + if err = c.accessoryMgr.Ensure(ctx, artifact.Digest, artifact.ID, acc.ArtifactID, acc.Size, acc.Digest, acc.Type); err != nil { return false, 0, err } } @@ -722,7 +722,7 @@ func (c *controller) populateAdditionLinks(ctx context.Context, artifact *Artifa } func (c *controller) populateAccessories(ctx context.Context, art *Artifact) { - accs, err := c.accessoryMgr.List(ctx, q.New(q.KeyWords{"SubjectArtifactDigest": art.Digest})) + accs, err := c.accessoryMgr.List(ctx, q.New(q.KeyWords{"SubjectArtifactID": art.ID})) if err != nil { log.Errorf("failed to list accessories of artifact %d: %v", art.ID, err) return diff --git a/src/controller/artifact/controller_test.go b/src/controller/artifact/controller_test.go index 40c6537d5..b1722db65 100644 --- a/src/controller/artifact/controller_test.go +++ b/src/controller/artifact/controller_test.go @@ -564,7 +564,7 @@ func (c *controllerTestSuite) TestCopy() { c.artMgr.On("Create", mock.Anything, mock.Anything).Return(int64(1), nil) c.regCli.On("Copy", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) c.tagCtl.On("Ensure").Return(nil) - c.accMgr.On("Ensure", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) + c.accMgr.On("Ensure", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) _, err := c.ctl.Copy(orm.NewContext(nil, &ormtesting.FakeOrmer{}), "library/hello-world", "latest", "library/hello-world2") c.Require().Nil(err) } diff --git a/src/pkg/accessory/dao/dao.go b/src/pkg/accessory/dao/dao.go index 85fb1e709..651871f8a 100644 --- a/src/pkg/accessory/dao/dao.go +++ b/src/pkg/accessory/dao/dao.go @@ -98,9 +98,6 @@ func (d *dao) Create(ctx context.Context, acc *Accessory) (int64, error) { if e := orm.AsConflictError(err, "accessory %s already exists under the artifact %s", acc.Digest, acc.SubjectArtifactDigest); e != nil { err = e - } else if e := orm.AsForeignKeyError(err, "the accessory %s tries to attach to a non existing artifact %s", - acc.Digest, acc.SubjectArtifactDigest); e != nil { - err = e } } return id, err diff --git a/src/pkg/accessory/dao/model.go b/src/pkg/accessory/dao/model.go index 0d084064e..e140cbd54 100644 --- a/src/pkg/accessory/dao/model.go +++ b/src/pkg/accessory/dao/model.go @@ -28,6 +28,7 @@ func init() { type Accessory struct { ID int64 `orm:"pk;auto;column(id)" json:"id"` ArtifactID int64 `orm:"column(artifact_id)" json:"artifact_id"` + SubjectArtifactID int64 `orm:"column(subject_artifact_id)" json:"subject_artifact_id"` SubjectArtifactDigest string `orm:"column(subject_artifact_digest)" json:"subject_artifact_digest"` Type string `orm:"column(type)" json:"type"` Size int64 `orm:"column(size)" json:"size"` diff --git a/src/pkg/accessory/manager.go b/src/pkg/accessory/manager.go index d14ec3ae2..6836662be 100644 --- a/src/pkg/accessory/manager.go +++ b/src/pkg/accessory/manager.go @@ -38,7 +38,7 @@ var ( // Manager is the only interface of artifact module to provide the management functions for artifacts type Manager interface { // Ensure ... - Ensure(ctx context.Context, subArtDigest string, artifactID, size int64, digest, accType string) error + Ensure(ctx context.Context, subArtDigest string, subArtID, artifactID, size int64, digest, accType string) error // Get the artifact specified by the ID Get(ctx context.Context, id int64) (accessory model.Accessory, err error) // Count returns the total count of accessory according to the query. @@ -66,7 +66,7 @@ type manager struct { dao dao.DAO } -func (m *manager) Ensure(ctx context.Context, subArtDigest string, artifactID, size int64, digest, accType string) error { +func (m *manager) Ensure(ctx context.Context, subArtDigest string, subArtID, artifactID, size int64, digest, accType string) error { accs, err := m.dao.List(ctx, q.New(q.KeyWords{"ArtifactID": artifactID, "Digest": digest})) if err != nil { return err @@ -77,6 +77,7 @@ func (m *manager) Ensure(ctx context.Context, subArtDigest string, artifactID, s acc := model.AccessoryData{ ArtifactID: artifactID, + SubArtifactID: subArtID, SubArtifactDigest: subArtDigest, Digest: digest, Size: size, @@ -94,6 +95,7 @@ func (m *manager) Get(ctx context.Context, id int64) (model.Accessory, error) { return model.New(acc.Type, model.AccessoryData{ ID: acc.ID, ArtifactID: acc.ArtifactID, + SubArtifactID: acc.SubjectArtifactID, SubArtifactDigest: acc.SubjectArtifactDigest, Size: acc.Size, Digest: acc.Digest, @@ -116,6 +118,7 @@ func (m *manager) List(ctx context.Context, query *q.Query) ([]model.Accessory, acc, err := model.New(accD.Type, model.AccessoryData{ ID: accD.ID, ArtifactID: accD.ArtifactID, + SubArtifactID: accD.SubjectArtifactID, SubArtifactDigest: accD.SubjectArtifactDigest, Size: accD.Size, Digest: accD.Digest, @@ -133,6 +136,7 @@ func (m *manager) List(ctx context.Context, query *q.Query) ([]model.Accessory, func (m *manager) Create(ctx context.Context, accessory model.AccessoryData) (int64, error) { acc := &dao.Accessory{ ArtifactID: accessory.ArtifactID, + SubjectArtifactID: accessory.SubArtifactID, SubjectArtifactDigest: accessory.SubArtifactDigest, Size: accessory.Size, Digest: accessory.Digest, diff --git a/src/pkg/accessory/manager_test.go b/src/pkg/accessory/manager_test.go index 5136acdf1..5f9a930ac 100644 --- a/src/pkg/accessory/manager_test.go +++ b/src/pkg/accessory/manager_test.go @@ -45,7 +45,7 @@ func (m *managerTestSuite) SetupTest() { func (m *managerTestSuite) TestEnsure() { mock.OnAnything(m.dao, "List").Return([]*dao.Accessory{}, nil) mock.OnAnything(m.dao, "Create").Return(int64(1), nil) - err := m.mgr.Ensure(nil, string(""), int64(1), int64(1), "sha256:1234", model.TypeCosignSignature) + err := m.mgr.Ensure(nil, string(""), int64(1), int64(2), int64(1), "sha256:1234", model.TypeCosignSignature) m.Require().Nil(err) } diff --git a/src/pkg/accessory/model/accessory.go b/src/pkg/accessory/model/accessory.go index e60d9c813..9ca31bf5c 100644 --- a/src/pkg/accessory/model/accessory.go +++ b/src/pkg/accessory/model/accessory.go @@ -79,7 +79,8 @@ const ( type AccessoryData struct { ID int64 `json:"id"` ArtifactID int64 `json:"artifact_id"` - SubArtifactDigest string `json:"subject_artifact_id"` + SubArtifactID int64 `json:"subject_artifact_id"` + SubArtifactDigest string `json:"subject_artifact_digest"` Type string `json:"type"` Size int64 `json:"size"` Digest string `json:"digest"` diff --git a/src/pkg/accessory/model/base/base_test.go b/src/pkg/accessory/model/base/base_test.go index ec66a7872..e8779e837 100644 --- a/src/pkg/accessory/model/base/base_test.go +++ b/src/pkg/accessory/model/base/base_test.go @@ -22,6 +22,7 @@ func (suite *BaseTestSuite) SetupSuite() { suite.accessory, _ = model.New(model.TypeNone, model.AccessoryData{ ArtifactID: 1, + SubArtifactID: 2, SubArtifactDigest: suite.subDigest, Size: 1234, Digest: suite.digest, @@ -36,7 +37,8 @@ func (suite *BaseTestSuite) TestGetArtID() { suite.Equal(int64(1), suite.accessory.GetData().ArtifactID) } -func (suite *BaseTestSuite) TestSubGetArtID() { +func (suite *BaseTestSuite) TestSubGetArt() { + suite.Equal(int64(2), suite.accessory.GetData().SubArtifactID) suite.Equal(suite.subDigest, suite.accessory.GetData().SubArtifactDigest) } diff --git a/src/server/middleware/cosign/cosign.go b/src/server/middleware/cosign/cosign.go index e0b5d69cf..7d0563764 100644 --- a/src/server/middleware/cosign/cosign.go +++ b/src/server/middleware/cosign/cosign.go @@ -99,23 +99,29 @@ func SignatureMiddleware() func(http.Handler) http.Handler { if hasSignature { subjectArt, err := artifact.Ctl.GetByReference(ctx, info.Repository, fmt.Sprintf("%s:%s", digest.SHA256, subjectArtDigest), nil) if err != nil { - logger.Errorf("failed to get subject artifact: %s, error: %v", subjectArtDigest, err) - return err + if !errors.IsNotFoundErr(err) { + logger.Errorf("failed to get subject artifact: %s, error: %v", subjectArtDigest, err) + return err + } + log.Debug("the subject of the signature doesn't exist.") } art, err := artifact.Ctl.GetByReference(ctx, info.Repository, desc.Digest.String(), nil) if err != nil { logger.Errorf("failed to get cosign signature artifact: %s, error: %v", desc.Digest.String(), err) return err } - + accData := model.AccessoryData{ + ArtifactID: art.ID, + SubArtifactDigest: fmt.Sprintf("%s:%s", digest.SHA256, subjectArtDigest), + Size: art.Size, + Digest: art.Digest, + Type: model.TypeCosignSignature, + } + if subjectArt != nil { + accData.SubArtifactID = subjectArt.ID + } if err := orm.WithTransaction(func(ctx context.Context) error { - _, err := accessory.Mgr.Create(ctx, model.AccessoryData{ - ArtifactID: art.ID, - SubArtifactDigest: subjectArt.Digest, - Size: desc.Size, - Digest: desc.Digest.String(), - Type: model.TypeCosignSignature, - }) + _, err := accessory.Mgr.Create(ctx, accData) return err })(orm.SetTransactionOpNameToContext(ctx, "tx-create-cosign-accessory")); err != nil { if !errors.IsConflictErr(err) { diff --git a/src/server/middleware/subject/subject.go b/src/server/middleware/subject/subject.go index 522ed5cbe..9cbb5f699 100644 --- a/src/server/middleware/subject/subject.go +++ b/src/server/middleware/subject/subject.go @@ -84,23 +84,29 @@ func Middleware() func(http.Handler) http.Handler { if mf.Subject != nil { subjectArt, err := artifact.Ctl.GetByReference(ctx, info.Repository, mf.Subject.Digest.String(), nil) if err != nil { - logger.Errorf("failed to get subject artifact: %s, error: %v", mf.Subject.Digest, err) - return err + if !errors.IsNotFoundErr(err) { + logger.Errorf("failed to get subject artifact: %s, error: %v", mf.Subject.Digest, err) + return err + } + log.Debug("the subject of the signature doesn't exist.") } art, err := artifact.Ctl.GetByReference(ctx, info.Repository, info.Reference, nil) if err != nil { logger.Errorf("failed to get artifact with subject field: %s, error: %v", info.Reference, err) return err } - + accData := model.AccessoryData{ + ArtifactID: art.ID, + SubArtifactDigest: mf.Subject.Digest.String(), + Size: art.Size, + Digest: art.Digest, + Type: model.TypeSubject, + } + if subjectArt != nil { + accData.SubArtifactID = subjectArt.ID + } if err := orm.WithTransaction(func(ctx context.Context) error { - _, err := accessory.Mgr.Create(ctx, model.AccessoryData{ - ArtifactID: art.ID, - SubArtifactDigest: subjectArt.Digest, - Size: art.Size, - Digest: art.Digest, - Type: model.TypeSubject, - }) + _, err := accessory.Mgr.Create(ctx, accData) return err })(orm.SetTransactionOpNameToContext(ctx, "tx-create-subject-accessory")); err != nil { if !errors.IsConflictErr(err) { diff --git a/src/server/v2.0/handler/artifact.go b/src/server/v2.0/handler/artifact.go index c9391064f..c65cda595 100644 --- a/src/server/v2.0/handler/artifact.go +++ b/src/server/v2.0/handler/artifact.go @@ -372,7 +372,7 @@ func (a *artifactAPI) ListAccessories(ctx context.Context, params operation.List if err != nil { return a.SendError(ctx, err) } - query.Keywords["SubjectArtifactDigest"] = artifact.Digest + query.Keywords["SubjectArtifactID"] = artifact.ID // list accessories according to the query total, err := a.accMgr.Count(ctx, query) diff --git a/src/server/v2.0/handler/model/accessory.go b/src/server/v2.0/handler/model/accessory.go index 3687ea9ef..7c0e15748 100644 --- a/src/server/v2.0/handler/model/accessory.go +++ b/src/server/v2.0/handler/model/accessory.go @@ -17,6 +17,7 @@ func (a *Accessory) ToSwagger() *models.Accessory { return &models.Accessory{ ID: a.ID, ArtifactID: a.ArtifactID, + SubjectArtifactID: a.SubArtifactID, SubjectArtifactDigest: a.SubArtifactDigest, Size: a.Size, Digest: a.Digest, diff --git a/src/testing/pkg/accessory/manager.go b/src/testing/pkg/accessory/manager.go index d911e5440..397d1d5ad 100644 --- a/src/testing/pkg/accessory/manager.go +++ b/src/testing/pkg/accessory/manager.go @@ -92,13 +92,13 @@ func (_m *Manager) DeleteAccessories(ctx context.Context, _a1 *q.Query) error { return r0 } -// Ensure provides a mock function with given fields: ctx, subArtDigest, artifactID, size, digest, accType -func (_m *Manager) Ensure(ctx context.Context, subArtDigest string, artifactID int64, size int64, digest string, accType string) error { - ret := _m.Called(ctx, subArtDigest, artifactID, size, digest, accType) +// Ensure provides a mock function with given fields: ctx, subArtDigest, subArtID, artifactID, size, digest, accType +func (_m *Manager) Ensure(ctx context.Context, subArtDigest string, subArtID int64, artifactID int64, size int64, digest string, accType string) error { + ret := _m.Called(ctx, subArtDigest, subArtID, artifactID, size, digest, accType) var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string, int64, int64, string, string) error); ok { - r0 = rf(ctx, subArtDigest, artifactID, size, digest, accType) + if rf, ok := ret.Get(0).(func(context.Context, string, int64, int64, int64, string, string) error); ok { + r0 = rf(ctx, subArtDigest, subArtID, artifactID, size, digest, accType) } else { r0 = ret.Error(0) }