From d03813ca32d2bfa278961478ce579f2c5b7b0f9c Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang <103478229+wangxiaoxuan273@users.noreply.github.com> Date: Thu, 3 Nov 2022 16:52:46 +0800 Subject: [PATCH] feat!: implement artifact manifest handler in the storage package (#30) Implemented artifact manifest handler related changes for the storage package, unit tests included. Part 3 of #21 Signed-off-by: wangxiaoxuan273 --- registry/storage/manifeststore.go | 8 ++ registry/storage/manifeststore_test.go | 81 +++++++++++++ .../storage/ociartifactmanifesthandler.go | 114 ++++++++++++++++++ .../ociartifactmanifesthandler_test.go | 112 +++++++++++++++++ registry/storage/registry.go | 5 + 5 files changed, 320 insertions(+) create mode 100644 registry/storage/ociartifactmanifesthandler.go create mode 100644 registry/storage/ociartifactmanifesthandler_test.go diff --git a/registry/storage/manifeststore.go b/registry/storage/manifeststore.go index 0daa92dfe91..f459a33463f 100644 --- a/registry/storage/manifeststore.go +++ b/registry/storage/manifeststore.go @@ -9,6 +9,7 @@ import ( dcontext "github.com/distribution/distribution/v3/context" "github.com/distribution/distribution/v3/manifest" "github.com/distribution/distribution/v3/manifest/manifestlist" + "github.com/distribution/distribution/v3/manifest/ociartifact" "github.com/distribution/distribution/v3/manifest/ocischema" "github.com/distribution/distribution/v3/manifest/schema1" "github.com/distribution/distribution/v3/manifest/schema2" @@ -51,6 +52,7 @@ type manifestStore struct { schema1Handler ManifestHandler schema2Handler ManifestHandler ocischemaHandler ManifestHandler + ociartifactHandler ManifestHandler manifestListHandler ManifestHandler } @@ -94,6 +96,10 @@ func (ms *manifestStore) Get(ctx context.Context, dgst digest.Digest, options .. return nil, err } + if versioned.MediaType == v1.MediaTypeArtifactManifest { + return ms.ociartifactHandler.Unmarshal(ctx, dgst, content) + } + switch versioned.SchemaVersion { case 1: return ms.schema1Handler.Unmarshal(ctx, dgst, content) @@ -136,6 +142,8 @@ func (ms *manifestStore) Put(ctx context.Context, manifest distribution.Manifest return ms.schema2Handler.Put(ctx, manifest, ms.skipDependencyVerification) case *ocischema.DeserializedManifest: return ms.ocischemaHandler.Put(ctx, manifest, ms.skipDependencyVerification) + case *ociartifact.DeserializedManifest: + return ms.ociartifactHandler.Put(ctx, manifest, ms.skipDependencyVerification) case *manifestlist.DeserializedManifestList: return ms.manifestListHandler.Put(ctx, manifest, ms.skipDependencyVerification) } diff --git a/registry/storage/manifeststore_test.go b/registry/storage/manifeststore_test.go index 53ae76d041f..33a39c4ff4e 100644 --- a/registry/storage/manifeststore_test.go +++ b/registry/storage/manifeststore_test.go @@ -3,6 +3,7 @@ package storage import ( "bytes" "context" + "encoding/json" "io" "reflect" "testing" @@ -10,6 +11,7 @@ import ( "github.com/distribution/distribution/v3" "github.com/distribution/distribution/v3/manifest" "github.com/distribution/distribution/v3/manifest/manifestlist" + "github.com/distribution/distribution/v3/manifest/ociartifact" "github.com/distribution/distribution/v3/manifest/ocischema" "github.com/distribution/distribution/v3/manifest/schema1" "github.com/distribution/distribution/v3/reference" @@ -543,6 +545,85 @@ func testOCIManifestStorage(t *testing.T, testname string, includeMediaTypes boo } +func TestOCIArtifactManifestStorage(t *testing.T) { + repoName, _ := reference.WithName("foo/woo/loo/koo") + env := newManifestStoreTestEnv(t, repoName, "ociartifact", + BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider(memory.UnlimitedSize)), + EnableDelete, EnableRedirect) + + ctx := context.Background() + ms, err := env.repository.Manifests(ctx) + if err != nil { + t.Fatal(err) + } + + bs := env.repository.Blobs(ctx) + + // create and push the blob and subject manifests into the storage first + blob, _ := json.Marshal(ociartifact.Manifest{ + MediaType: v1.MediaTypeArtifactManifest, + ArtifactType: "test/blob", + }) + blobDesc, err := bs.Put(ctx, v1.MediaTypeArtifactManifest, blob) // push blob manifest into the storage + if err != nil { + t.Fatal(err) + } + + subjectAM := ociartifact.Manifest{ + MediaType: v1.MediaTypeArtifactManifest, + ArtifactType: "test/subject", + } + subject, err := ociartifact.FromStruct(subjectAM) + if err != nil { + t.Fatal(err) + } + subjectDigest, err := ms.Put(ctx, subject) // push subject manifest into the storage + if err != nil { + t.Fatal(err) + } + + // create the main artifact manifest that has the blob and subject as references + am := ociartifact.Manifest{ + MediaType: v1.MediaTypeArtifactManifest, + ArtifactType: "test/main", + Blobs: []distribution.Descriptor{blobDesc}, + Subject: &distribution.Descriptor{MediaType: v1.MediaTypeArtifactManifest, Digest: subjectDigest}, + } + manifest, err := ociartifact.FromStruct(am) + if err != nil { + t.Fatal(err) + } + + var manifestDigest digest.Digest + if manifestDigest, err = ms.Put(ctx, manifest); err != nil { + t.Fatal(err) + } + + // Now check that we can retrieve the manifest + fromStore, err := ms.Get(ctx, manifestDigest) + if err != nil { + t.Fatalf("unexpected error fetching manifest: %v", err) + } + + fetchedManifest, ok := fromStore.(*ociartifact.DeserializedManifest) + if !ok { + t.Fatalf("unexpected type for fetched manifest") + } + + if fetchedManifest.MediaType != v1.MediaTypeArtifactManifest { + t.Fatalf("unexpected MediaType for result, %s", fetchedManifest.MediaType) + } + + payloadMediaType, _, err := fromStore.Payload() + if err != nil { + t.Fatalf("error getting payload %v", err) + } + + if payloadMediaType != v1.MediaTypeArtifactManifest { + t.Fatalf("unexpected MediaType for manifest payload, %s", payloadMediaType) + } +} + // TestLinkPathFuncs ensures that the link path functions behavior are locked // down and implemented as expected. func TestLinkPathFuncs(t *testing.T) { diff --git a/registry/storage/ociartifactmanifesthandler.go b/registry/storage/ociartifactmanifesthandler.go new file mode 100644 index 00000000000..287d333163a --- /dev/null +++ b/registry/storage/ociartifactmanifesthandler.go @@ -0,0 +1,114 @@ +package storage + +import ( + "context" + "fmt" + + "github.com/distribution/distribution/v3" + dcontext "github.com/distribution/distribution/v3/context" + "github.com/distribution/distribution/v3/manifest/ociartifact" + "github.com/opencontainers/go-digest" + v1 "github.com/opencontainers/image-spec/specs-go/v1" +) + +// ociArtifactManifestHandler is a ManifestHandler that covers oci artifact manifests. +type ociArtifactManifestHandler struct { + repository distribution.Repository + blobStore distribution.BlobStore + ctx context.Context +} + +var _ ManifestHandler = &ociArtifactManifestHandler{} + +func (ms *ociArtifactManifestHandler) Unmarshal(ctx context.Context, dgst digest.Digest, content []byte) (distribution.Manifest, error) { + dcontext.GetLogger(ms.ctx).Debug("(*ociArtifactManifestHandler).Unmarshal") + + m := &ociartifact.DeserializedManifest{} + if err := m.UnmarshalJSON(content); err != nil { + return nil, err + } + + return m, nil +} + +func (ms *ociArtifactManifestHandler) Put(ctx context.Context, manifest distribution.Manifest, skipDependencyVerification bool) (digest.Digest, error) { + dcontext.GetLogger(ms.ctx).Debug("(*ociArtifactManifestHandler).Put") + + m, ok := manifest.(*ociartifact.DeserializedManifest) + if !ok { + return "", fmt.Errorf("non-oci artifact manifest put to ociArtifactManifestHandler: %T", manifest) + } + + if err := ms.verifyArtifactManifest(ms.ctx, m, skipDependencyVerification); err != nil { + return "", err + } + + mt, payload, err := m.Payload() + if err != nil { + return "", err + } + + revision, err := ms.blobStore.Put(ctx, mt, payload) + if err != nil { + dcontext.GetLogger(ctx).Errorf("error putting payload into blobstore: %v", err) + return "", err + } + + return revision.Digest, nil +} + +// verifyArtifactManifest ensures that the manifest content is valid from the +// perspective of the registry. As a policy, the registry only tries to store +// valid content, leaving trust policies of that content up to consumers. +func (ms *ociArtifactManifestHandler) verifyArtifactManifest(ctx context.Context, mnfst *ociartifact.DeserializedManifest, skipDependencyVerification bool) error { + var errs distribution.ErrManifestVerification + + if mnfst.MediaType != v1.MediaTypeArtifactManifest { + return fmt.Errorf("unrecognized manifest media type %s", mnfst.MediaType) + } + + if skipDependencyVerification { + return nil + } + + // validate the subject + if mnfst.Subject != nil { + // check if the digest is valid + err := mnfst.Subject.Digest.Validate() + if err != nil { + errs = append(errs, err, distribution.ErrManifestBlobUnknown{Digest: mnfst.Subject.Digest}) + } else { + // check the presence + manifestService, err := ms.repository.Manifests(ctx) + if err != nil { + return err + } + exists, err := manifestService.Exists(ctx, mnfst.Subject.Digest) + if err != nil || !exists { + errs = append(errs, distribution.ErrManifestBlobUnknown{Digest: mnfst.Subject.Digest}) + } + } + } + + // validate the blobs + blobsService := ms.repository.Blobs(ctx) + for _, descriptor := range mnfst.Blobs { + // check if the digest is valid + err := descriptor.Digest.Validate() + if err != nil { + errs = append(errs, err, distribution.ErrManifestBlobUnknown{Digest: descriptor.Digest}) + continue + } + + _, err = blobsService.Stat(ctx, descriptor.Digest) + if err != nil { + errs = append(errs, distribution.ErrManifestBlobUnknown{Digest: descriptor.Digest}) + } + } + + if len(errs) != 0 { + return errs + } + + return nil +} diff --git a/registry/storage/ociartifactmanifesthandler_test.go b/registry/storage/ociartifactmanifesthandler_test.go new file mode 100644 index 00000000000..4a6024b0139 --- /dev/null +++ b/registry/storage/ociartifactmanifesthandler_test.go @@ -0,0 +1,112 @@ +package storage + +import ( + "context" + "strings" + "testing" + + "github.com/distribution/distribution/v3" + "github.com/distribution/distribution/v3/manifest/ociartifact" + "github.com/distribution/distribution/v3/registry/storage/driver/inmemory" + "github.com/opencontainers/go-digest" + v1 "github.com/opencontainers/image-spec/specs-go/v1" +) + +func TestVerifyOCIArtifactManifestBlobsAndSubject(t *testing.T) { + ctx := context.Background() + inmemoryDriver := inmemory.New() + registry := createRegistry(t, inmemoryDriver) + repo := makeRepository(t, registry, strings.ToLower(t.Name())) + manifestService := makeManifestService(t, repo) + + subject, err := repo.Blobs(ctx).Put(ctx, v1.MediaTypeImageManifest, nil) + if err != nil { + t.Fatal(err) + } + + blob, err := repo.Blobs(ctx).Put(ctx, v1.MediaTypeImageLayer, nil) + if err != nil { + t.Fatal(err) + } + + template := ociartifact.Manifest{ + MediaType: v1.MediaTypeArtifactManifest, + } + + checkFn := func(m ociartifact.Manifest, rerr error) { + dm, err := ociartifact.FromStruct(m) + if err != nil { + t.Error(err) + return + } + _, err = manifestService.Put(ctx, dm) + if verr, ok := err.(distribution.ErrManifestVerification); ok { + // Extract the first error + if len(verr) == 2 { + if _, ok = verr[1].(distribution.ErrManifestBlobUnknown); ok { + err = verr[0] + } + } else if len(verr) == 1 { + err = verr[0] + } + } + if err != rerr { + t.Errorf("%#v: expected %v, got %v", m, rerr, err) + } + } + + type testcase struct { + Desc distribution.Descriptor + Err error + } + + layercases := []testcase{ + // normal blob with media type v1.MediaTypeImageLayer + { + blob, + nil, + }, + // blob with empty media type but valid digest + { + distribution.Descriptor{Digest: blob.Digest}, + nil, + }, + // blob with invalid digest + { + distribution.Descriptor{Digest: digest.Digest("invalid")}, + digest.ErrDigestInvalidFormat, + }, + // empty descriptor + { + distribution.Descriptor{}, + digest.ErrDigestInvalidFormat, + }, + } + + for _, c := range layercases { + m := template + m.Subject = &subject + m.Blobs = []distribution.Descriptor{c.Desc} + checkFn(m, c.Err) + } + + subjectcases := []testcase{ + // normal subject + { + subject, + nil, + }, + // subject with invalid digest + { + distribution.Descriptor{Digest: digest.Digest("invalid")}, + digest.ErrDigestInvalidFormat, + }, + } + + for _, c := range subjectcases { + m := template + m.Subject = &c.Desc + m.Blobs = []distribution.Descriptor{blob} + checkFn(m, c.Err) + } +} diff --git a/registry/storage/registry.go b/registry/storage/registry.go index 45939083f7a..679119bf0ec 100644 --- a/registry/storage/registry.go +++ b/registry/storage/registry.go @@ -288,6 +288,11 @@ func (repo *repository) Manifests(ctx context.Context, options ...distribution.M blobStore: blobStore, manifestURLs: repo.registry.manifestURLs, }, + ociartifactHandler: &ociArtifactManifestHandler{ + repository: repo, + blobStore: blobStore, + ctx: ctx, + }, } // Apply options