Skip to content

Commit

Permalink
address comments and fix
Browse files Browse the repository at this point in the history 
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
  • Loading branch information
@Wwwsylvia
Wwwsylvia committed Sep 30, 2022
1 parent bbfd844 commit 5f39bee
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 49 deletions.
15 changes: 12 additions & 3 deletions content/graph.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,22 @@ type ReadOnlyGraphStorage interface {
// In other words, returns the "children" of the current descriptor.
func Successors(ctx context.Context, fetcher Fetcher, node ocispec.Descriptor) ([]ocispec.Descriptor, error) {
switch node.MediaType {
case docker.MediaTypeManifest, ocispec.MediaTypeImageManifest:
case docker.MediaTypeManifest:
content, err := FetchAll(ctx, fetcher, node)
if err != nil {
return nil, err
}
// OCI manifest schema can be used to marshal docker manifest
var manifest ocispec.Manifest
if err := json.Unmarshal(content, &manifest); err != nil {
return nil, err
}
return append([]ocispec.Descriptor{manifest.Config}, manifest.Layers...), nil
case ocispec.MediaTypeImageManifest:
content, err := FetchAll(ctx, fetcher, node)
if err != nil {
return nil, err
}

// docker manifest and oci manifest are equivalent for successors.
var manifest ocispec.Manifest
if err := json.Unmarshal(content, &manifest); err != nil {
return nil, err
Expand Down
56 changes: 27 additions & 29 deletions extendedcopy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,31 +175,30 @@ func findRoots(ctx context.Context, storage content.ReadOnlyGraphStorage, node o
return roots, nil
}

// FilterAnnotation will configure opts.FindPredecessors to filter the
// predecessors whose annotation matches a given regex pattern. A predecessor is
// kept if the key is in its annotation and matches the regex if present.
// FilterAnnotation configures opts.FindPredecessors to filter the predecessors
// whose annotation matches a given regex pattern. A predecessor is kept
// if key is in its annotations and the annotation value matches regex.
// If regex is nil, predecessors whose annotations contain key will be kept,
// no matter of the annotation value.
// For performance consideration, when using both FilterArtifactType and
// FilterAnnotation, it's recommended to call FilterArtifactType first.
func (opts *ExtendedCopyGraphOptions) FilterAnnotation(key string, regex *regexp.Regexp) {
filter := func(desc ocispec.Descriptor) bool {
if value, ok := desc.Annotations[key]; ok && (regex == nil || regex.MatchString(value)) {
return true
}
return false
keep := func(desc ocispec.Descriptor) bool {
value, ok := desc.Annotations[key]
return ok && (regex == nil || regex.MatchString(value))
}

fp := opts.FindPredecessors
opts.FindPredecessors = func(ctx context.Context, src content.ReadOnlyGraphStorage, desc ocispec.Descriptor) ([]ocispec.Descriptor, error) {
var predecessors []ocispec.Descriptor
var err error

if fp == nil {
if rf, ok := src.(registry.ReferrerFinder); ok {
// if src is a ReferrerFinder, use Referrers() for possible memory saving
if err := rf.Referrers(ctx, desc, "", func(referrers []ocispec.Descriptor) error {
// for each page of the results, filter the referrers
for _, r := range referrers {
if filter(r) {
if keep(r) {
predecessors = append(predecessors, r)
}
}
Expand All @@ -219,7 +218,7 @@ func (opts *ExtendedCopyGraphOptions) FilterAnnotation(key string, regex *regexp

// Predecessor descriptors that are not from Referrers API are not
// guaranteed to include the annotations of the corresponding manifests.
var filtered []ocispec.Descriptor
var kept []ocispec.Descriptor
for _, p := range predecessors {
if p.Annotations == nil {
// If the annotations are not present in the descriptors,
Expand All @@ -235,11 +234,11 @@ func (opts *ExtendedCopyGraphOptions) FilterAnnotation(key string, regex *regexp
p.Annotations = annotations
}
}
if filter(p) {
filtered = append(filtered, p)
if keep(p) {
kept = append(kept, p)
}
}
return filtered, nil
return kept, nil
}
}

Expand All @@ -264,16 +263,17 @@ func fetchAnnotations(ctx context.Context, src content.ReadOnlyGraphStorage, des
return manifest.Annotations, nil
}

// FilterArtifactType will configure opts.FindPredecessors to filter the predecessors
// whose artifact type matches a given regex pattern. When the regex pattern is nil,
// no artifact type filter will be applied. For performance consideration, when using both
// FilterArtifactType and FilterAnnotation, it's recommended to call
// FilterArtifactType first.
// FilterArtifactType configures opts.FindPredecessors to filter the
// predecessors whose artifact type matches a given regex pattern. A predecessor
// is kept if its artifact type matches regex. If regex is nil, all
// predecessors will be kept.
// For performance consideration, when using both FilterArtifactType and
// FilterAnnotation, it's recommended to call FilterArtifactType first.
func (opts *ExtendedCopyGraphOptions) FilterArtifactType(regex *regexp.Regexp) {
if regex == nil {
return
}
filter := func(desc ocispec.Descriptor) bool {
keep := func(desc ocispec.Descriptor) bool {
return regex.MatchString(desc.ArtifactType)
}

Expand All @@ -287,7 +287,7 @@ func (opts *ExtendedCopyGraphOptions) FilterArtifactType(regex *regexp.Regexp) {
if err := rf.Referrers(ctx, desc, "", func(referrers []ocispec.Descriptor) error {
// for each page of the results, filter the referrers
for _, r := range referrers {
if filter(r) {
if keep(r) {
predecessors = append(predecessors, r)
}
}
Expand All @@ -308,27 +308,25 @@ func (opts *ExtendedCopyGraphOptions) FilterArtifactType(regex *regexp.Regexp) {
// predecessor descriptors that are not from Referrers API are not
// guaranteed to include the artifact type of the corresponding
// manifests.
var filtered []ocispec.Descriptor
var kept []ocispec.Descriptor
for _, p := range predecessors {
if p.ArtifactType == "" {
// if the artifact type is not present in the descriptors,
// fetch it from the manifest content.
switch p.MediaType {
case ocispec.MediaTypeArtifactManifest,
ocispec.MediaTypeImageManifest,
docker.MediaTypeManifest:
case ocispec.MediaTypeArtifactManifest, ocispec.MediaTypeImageManifest:
artifactType, err := fetchArtifactType(ctx, src, p)
if err != nil {
return nil, err
}
p.ArtifactType = artifactType
}
}
if filter(p) {
filtered = append(filtered, p)
if keep(p) {
kept = append(kept, p)
}
}
return filtered, nil
return kept, nil
}
}

Expand All @@ -347,7 +345,7 @@ func fetchArtifactType(ctx context.Context, src content.ReadOnlyGraphStorage, de
return "", err
}
return manifest.ArtifactType, nil
case ocispec.MediaTypeImageManifest, docker.MediaTypeManifest:
case ocispec.MediaTypeImageManifest:
var manifest ocispec.Manifest
if err := json.NewDecoder(rc).Decode(&manifest); err != nil {
return "", err
Expand Down
32 changes: 15 additions & 17 deletions extendedcopy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import (
"oras.land/oras-go/v2/content"
"oras.land/oras-go/v2/content/memory"
"oras.land/oras-go/v2/errdef"
"oras.land/oras-go/v2/internal/docker"
"oras.land/oras-go/v2/registry/remote"
)

Expand Down Expand Up @@ -1462,12 +1461,12 @@ func TestExtendedCopyGraph_FilterArtifactTypeAndAnnotationWithMultipleRegex(t *t
}
appendBlob(ocispec.MediaTypeArtifactManifest, manifestJSON)
}
generateImageManifest := func(subject, config ocispec.Descriptor, mediaType string, value string) {
generateImageManifest := func(subject, config ocispec.Descriptor, value string) {
manifest := ocispec.Manifest{
Versioned: specs.Versioned{
SchemaVersion: 2, // historical value. does not pertain to OCI or docker version
},
MediaType: mediaType,
MediaType: ocispec.MediaTypeImageManifest,
Config: config,
Subject: &subject,
Annotations: map[string]string{"rank": value},
Expand All @@ -1479,18 +1478,17 @@ func TestExtendedCopyGraph_FilterArtifactTypeAndAnnotationWithMultipleRegex(t *t
appendBlob(ocispec.MediaTypeImageManifest, manifestJSON)
}

appendBlob(ocispec.MediaTypeImageLayer, []byte("foo")) // descs[0]
generateArtifactManifest(descs[0], "good-bar-yellow", "1st") // descs[1]
generateArtifactManifest(descs[0], "bad-woo-red", "1st") // descs[2]
generateArtifactManifest(descs[0], "bad-bar-blue", "2nd") // descs[3]
generateArtifactManifest(descs[0], "bad-bar-red", "3rd") // descs[4]
appendBlob("good-woo-pink", []byte("bar")) // descs[5]
generateImageManifest(descs[0], descs[5], ocispec.MediaTypeImageManifest, "3rd") // descs[6]
appendBlob("bad-bar-pink", []byte("baz")) // descs[7]
generateImageManifest(descs[0], descs[7], ocispec.MediaTypeImageManifest, "4th") // descs[8]
appendBlob("bad-bar-orange", []byte("config!")) // descs[9]
generateImageManifest(descs[0], descs[9], docker.MediaTypeManifest, "4th") // descs[10]

appendBlob(ocispec.MediaTypeImageLayer, []byte("foo")) // descs[0]
generateArtifactManifest(descs[0], "good-bar-yellow", "1st") // descs[1]
generateArtifactManifest(descs[0], "bad-woo-red", "1st") // descs[2]
generateArtifactManifest(descs[0], "bad-bar-blue", "2nd") // descs[3]
generateArtifactManifest(descs[0], "bad-bar-red", "3rd") // descs[4]
appendBlob("good-woo-pink", []byte("bar")) // descs[5]
generateImageManifest(descs[0], descs[5], "3rd") // descs[6]
appendBlob("bad-bar-pink", []byte("baz")) // descs[7]
generateImageManifest(descs[0], descs[7], "4th") // descs[8]
appendBlob("bad-bar-orange", []byte("config!")) // descs[9]
generateImageManifest(descs[0], descs[9], "5th") // descs[10]
ctx := context.Background()
verifyCopy := func(dst content.Fetcher, copiedIndice []int, uncopiedIndice []int) {
for _, i := range copiedIndice {
Expand Down Expand Up @@ -1537,8 +1535,8 @@ func TestExtendedCopyGraph_FilterArtifactTypeAndAnnotationWithMultipleRegex(t *t
if err := oras.ExtendedCopyGraph(ctx, src, dst, descs[0], opts); err != nil {
t.Errorf("ExtendedCopyGraph() error = %v, wantErr %v", err, false)
}
copiedIndice := []int{0, 3, 7, 8, 9, 10}
uncopiedIndice := []int{1, 2, 4, 5, 6}
copiedIndice := []int{0, 3, 7, 8}
uncopiedIndice := []int{1, 2, 4, 5, 6, 9, 10}
verifyCopy(dst, copiedIndice, uncopiedIndice)
}

Expand Down

0 comments on commit 5f39bee

Please sign in to comment.