Skip to content

Commit

Permalink
fix: remove subject existence check, add media type check (#52)
Browse files Browse the repository at this point in the history
Resolves #42 
Resolves #50 
Resolves #49

Signed-off-by: wangxiaoxuan273 <wangxiaoxuan119@gmail.com>
  • Loading branch information
wangxiaoxuan273 authored Jan 5, 2023
1 parent 2ba5caa commit 474bc48
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 22 deletions.
4 changes: 4 additions & 0 deletions errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ var ErrUnsupported = errors.New("operation unsupported")
// manifest but the registry is configured to reject it
var ErrSchemaV1Unsupported = errors.New("manifest schema v1 unsupported")

// ErrInvalidSubjectMediaType is returned when a manifest has a subject that is
// not a manifest
var ErrInvalidSubjectMediaType = errors.New("subject is not a manifest")

// ErrTagUnknown is returned if the given tag is not known by the tag service
type ErrTagUnknown struct {
Tag string
Expand Down
3 changes: 0 additions & 3 deletions manifest/ociartifact/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,6 @@ type Manifest struct {
func (m Manifest) References() []distribution.Descriptor {
var references []distribution.Descriptor
references = append(references, m.Blobs...)
if m.Subject != nil {
references = append(references, *m.Subject)
}
return references
}

Expand Down
2 changes: 1 addition & 1 deletion manifest/ociartifact/manifest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func TestArtifactManifest(t *testing.T) {

// Test DeserializedManifest.References()
references := deserialized.References()
if len(references) != 2 {
if len(references) != 1 {
t.Fatalf("unexpected number of references: %d", len(references))
}
}
3 changes: 0 additions & 3 deletions manifest/ocischema/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,6 @@ func (m Manifest) References() []distribution.Descriptor {
references := make([]distribution.Descriptor, 0, 1+len(m.Layers))
references = append(references, m.Config)
references = append(references, m.Layers...)
if m.Subject != nil {
references = append(references, *m.Subject)
}
return references
}

Expand Down
23 changes: 12 additions & 11 deletions registry/storage/ociartifactmanifesthandler.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import (

"github.com/distribution/distribution/v3"
dcontext "github.com/distribution/distribution/v3/context"
"github.com/distribution/distribution/v3/manifest/manifestlist"
"github.com/distribution/distribution/v3/manifest/ociartifact"
"github.com/distribution/distribution/v3/manifest/schema2"
"github.com/distribution/distribution/v3/registry/storage/driver"
"github.com/opencontainers/go-digest"
v1 "github.com/opencontainers/image-spec/specs-go/v1"
Expand Down Expand Up @@ -78,22 +80,21 @@ func (ms *ociArtifactManifestHandler) verifyArtifactManifest(ctx context.Context
return nil
}

// validate the subject
// For subject, we need to verify that:
// First, its digest is valid. Second, it is a manifest.
// No need to check its existence.
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})
}
}
// check the media type of subject
switch mnfst.Subject.MediaType {
case v1.MediaTypeImageManifest, v1.MediaTypeArtifactManifest, v1.MediaTypeImageIndex, schema2.MediaTypeManifest, manifestlist.MediaTypeManifestList:
// no operations for known manifest media types
default:
errs = append(errs, distribution.ErrInvalidSubjectMediaType)
}
}

Expand Down
19 changes: 15 additions & 4 deletions registry/storage/ociartifactmanifesthandler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ func TestVerifyOCIArtifactManifestBlobsAndSubject(t *testing.T) {
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)
subject := distribution.Descriptor{
Digest: digest.Digest("sha256:1a9ec845ee94c202b2d5da74a24f0ed2058318bfa9879fa541efaecba272e86b"),
MediaType: v1.MediaTypeImageManifest,
}

blob, err := repo.Blobs(ctx).Put(ctx, v1.MediaTypeImageLayer, nil)
Expand Down Expand Up @@ -98,9 +98,20 @@ func TestVerifyOCIArtifactManifestBlobsAndSubject(t *testing.T) {
},
// subject with invalid digest
{
distribution.Descriptor{Digest: digest.Digest("invalid")},
distribution.Descriptor{
Digest: digest.Digest("invalid"),
MediaType: v1.MediaTypeImageManifest,
},
digest.ErrDigestInvalidFormat,
},
// subject with a non-manifest mediatype
{
distribution.Descriptor{
Digest: digest.Digest("sha256:1a9ec845ee94c202b2d5da74a24f0ed2058318bfa9879fa541efaecba272e86b"),
MediaType: v1.MediaTypeImageConfig,
},
distribution.ErrInvalidSubjectMediaType,
},
}

for _, c := range subjectcases {
Expand Down
18 changes: 18 additions & 0 deletions registry/storage/ocimanifesthandler.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,24 @@ func (ms *ocischemaManifestHandler) verifyManifest(ctx context.Context, mnfst oc
return err
}

// For subject, we need to verify that:
// First, its digest is valid. Second, it is a manifest.
// No need to check its existence.
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})
}
// check the media type of subject
switch mnfst.Subject.MediaType {
case v1.MediaTypeImageManifest, v1.MediaTypeArtifactManifest, v1.MediaTypeImageIndex, schema2.MediaTypeManifest, manifestlist.MediaTypeManifestList:
// no operations for known manifest media types
default:
errs = append(errs, distribution.ErrInvalidSubjectMediaType)
}
}

blobsService := ms.repository.Blobs(ctx)

for _, descriptor := range mnfst.References() {
Expand Down

0 comments on commit 474bc48

Please sign in to comment.