Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: index referrers for image manifests #41

Merged
merged 5 commits into from
Nov 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion manifest/ocischema/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ type Builder struct {
// calls to AppendReference.
layers []distribution.Descriptor

// Subject specifies the descriptor of another manifest. This value is
// used by the referrers API.
subject *distribution.Descriptor

// Annotations contains arbitrary metadata relating to the targeted content.
annotations map[string]string

Expand All @@ -32,10 +36,11 @@ type Builder struct {
// NewManifestBuilder is used to build new manifests for the current schema
// version. It takes a BlobService so it can publish the configuration blob
// as part of the Build process, and annotations.
func NewManifestBuilder(bs distribution.BlobService, configJSON []byte, annotations map[string]string) distribution.ManifestBuilder {
func NewManifestBuilder(bs distribution.BlobService, configJSON []byte, subject *distribution.Descriptor, annotations map[string]string) distribution.ManifestBuilder {
mb := &Builder{
bs: bs,
configJSON: make([]byte, len(configJSON)),
subject: subject,
annotations: annotations,
mediaType: v1.MediaTypeImageManifest,
}
Expand Down Expand Up @@ -63,6 +68,7 @@ func (mb *Builder) Build(ctx context.Context) (distribution.Manifest, error) {
MediaType: mb.mediaType,
},
Layers: make([]distribution.Descriptor, len(mb.layers)),
Subject: mb.subject,
Annotations: mb.annotations,
}
copy(m.Layers, mb.layers)
Expand Down
2 changes: 1 addition & 1 deletion manifest/ocischema/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ func TestBuilder(t *testing.T) {
annotations := map[string]string{"hot": "potato"}

bs := &mockBlobService{descriptors: make(map[digest.Digest]distribution.Descriptor)}
builder := NewManifestBuilder(bs, imgJSON, annotations)
builder := NewManifestBuilder(bs, imgJSON, nil, annotations)

for _, d := range descriptors {
if err := builder.AppendReference(d); err != nil {
Expand Down
7 changes: 7 additions & 0 deletions manifest/ocischema/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ type Manifest struct {
// configuration.
Layers []distribution.Descriptor `json:"layers"`

// Subject specifies the descriptor of another manifest. This value is
// used by the referrers API.
Subject *distribution.Descriptor `json:"subject,omitempty"`

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want to also update the Reference() function?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added subject in Reference() and changed the verifyManifest function accordingly.


// Annotations contains arbitrary metadata for the image manifest.
Annotations map[string]string `json:"annotations,omitempty"`
}
Expand All @@ -60,6 +64,9 @@ 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
2 changes: 1 addition & 1 deletion registry/storage/manifeststore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ func testOCIManifestStorage(t *testing.T, testname string, includeMediaTypes boo
// Build a manifest and store it and its layers in the registry

blobStore := env.repository.Blobs(ctx)
builder := ocischema.NewManifestBuilder(blobStore, []byte{}, map[string]string{})
builder := ocischema.NewManifestBuilder(blobStore, []byte{}, nil, map[string]string{})
err = builder.(*ocischema.Builder).SetMediaType(imageMediaType)
if err != nil {
t.Fatal(err)
Expand Down
8 changes: 6 additions & 2 deletions registry/storage/ociartifactmanifesthandler.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,13 @@ func (ms *ociArtifactManifestHandler) indexReferrers(ctx context.Context, dm *oc
// but need to consider the max path length in different os
subjectRevision := dm.Subject.Digest

referrersLinkPath, err := pathFor(referrersLinkPathSpec{name: ms.repository.Named().Name(), revision: revision, subjectRevision: subjectRevision})
return indexWithSubject(ctx, ms.repository.Named().Name(), revision, subjectRevision, ms.storageDriver)
}

func indexWithSubject(ctx context.Context, repo string, revision digest.Digest, subjectRevision digest.Digest, sd driver.StorageDriver) error {
referrersLinkPath, err := pathFor(referrersLinkPathSpec{name: repo, revision: revision, subjectRevision: subjectRevision})
if err != nil {
return fmt.Errorf("failed to generate referrers link path for %v", revision)
}
return ms.storageDriver.PutContent(ctx, referrersLinkPath, []byte(revision.String()))
return sd.PutContent(ctx, referrersLinkPath, []byte(revision.String()))
}
35 changes: 29 additions & 6 deletions registry/storage/ocimanifesthandler.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,21 @@ 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/ocischema"
"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"
)

//ocischemaManifestHandler is a ManifestHandler that covers ocischema manifests.
// ocischemaManifestHandler is a ManifestHandler that covers ocischema manifests.
type ocischemaManifestHandler struct {
repository distribution.Repository
blobStore distribution.BlobStore
ctx context.Context
manifestURLs manifestURLs
repository distribution.Repository
blobStore distribution.BlobStore
ctx context.Context
manifestURLs manifestURLs
storageDriver driver.StorageDriver
}

var _ ManifestHandler = &ocischemaManifestHandler{}
Expand Down Expand Up @@ -56,6 +60,12 @@ func (ms *ocischemaManifestHandler) Put(ctx context.Context, manifest distributi
return "", err
}

err = ms.indexReferrers(ctx, m, revision.Digest)
if err != nil {
dcontext.GetLogger(ctx).Errorf("error indexing referrers: %v", err)
return "", err
}

return revision.Digest, nil
}

Expand Down Expand Up @@ -109,7 +119,7 @@ func (ms *ocischemaManifestHandler) verifyManifest(ctx context.Context, mnfst oc
}
}

case v1.MediaTypeImageManifest:
case v1.MediaTypeImageManifest, v1.MediaTypeArtifactManifest, v1.MediaTypeImageIndex, schema2.MediaTypeManifest, manifestlist.MediaTypeManifestList:
var exists bool
exists, err = manifestService.Exists(ctx, descriptor.Digest)
if err != nil || !exists {
Expand Down Expand Up @@ -141,3 +151,16 @@ func (ms *ocischemaManifestHandler) verifyManifest(ctx context.Context, mnfst oc

return nil
}

// indexReferrers indexes the subject of the given revision in its referrers index store.
func (ms *ocischemaManifestHandler) indexReferrers(ctx context.Context, dm *ocischema.DeserializedManifest, revision digest.Digest) error {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably OK since oras/distribution is for testing purpose. It will be better if it is refactored since it duplicated with this function.

if dm.Subject == nil {
return nil
}

// [TODO] We can use artifact type in the link path to support filtering by artifact type
// but need to consider the max path length in different os
subjectRevision := dm.Subject.Digest

return indexWithSubject(ctx, ms.repository.Named().Name(), revision, subjectRevision, ms.storageDriver)
}
9 changes: 5 additions & 4 deletions registry/storage/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -283,10 +283,11 @@ func (repo *repository) Manifests(ctx context.Context, options ...distribution.M
blobStore: blobStore,
},
ocischemaHandler: &ocischemaManifestHandler{
ctx: ctx,
repository: repo,
blobStore: blobStore,
manifestURLs: repo.registry.manifestURLs,
ctx: ctx,
repository: repo,
blobStore: blobStore,
manifestURLs: repo.registry.manifestURLs,
storageDriver: repo.driver,
},
ociartifactHandler: &ociArtifactManifestHandler{
repository: repo,
Expand Down