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.

Sign up for GitHub

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

Jump to bottom

fix: Add support for ocischema.DeserializedImageIndex manifest type #618

Merged
merged 2 commits into from
Sep 21, 2023
Merged

fix: Add support for ocischema.DeserializedImageIndex manifest type #618

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4c72f73
fix: Add support for ocischema.DeserializedImageIndex manifest type
jessebye Sep 1, 2023
cc4a195
more informative error logging
jessebye Sep 1, 2023
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
2 changes: 1 addition & 1 deletion pkg/argocd/update_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1739,7 +1739,7 @@ func Test_GetGitCreds(t *testing.T) {
},
},
Spec: v1alpha1.ApplicationSpec{
Source: v1alpha1.ApplicationSource{
Source: &v1alpha1.ApplicationSource{
RepoURL: "https://example-helm-repo.com/example",
TargetRevision: "main",
},
Expand Down
136 changes: 82 additions & 54 deletions pkg/registry/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,6 @@ func (client *registryClient) TagMetadata(manifest distribution.Manifest, opts *

case *manifestlist.DeserializedManifestList:
var list manifestlist.DeserializedManifestList = *deserialized
var ml []distribution.Descriptor
platforms := []string{}

// List must contain at least one image manifest
if len(list.Manifests) == 0 {
Expand All @@ -262,63 +260,27 @@ func (client *registryClient) TagMetadata(manifest distribution.Manifest, opts *

logCtx.Tracef("SHA256 of manifest parent is %v", ti.EncodedDigest())

for _, ref := range list.References() {
platforms = append(platforms, ref.Platform.OS+"/"+ref.Platform.Architecture)
logCtx.Tracef("Found %s", options.PlatformKey(ref.Platform.OS, ref.Platform.Architecture, ref.Platform.Variant))
if !opts.WantsPlatform(ref.Platform.OS, ref.Platform.Architecture, ref.Platform.Variant) {
logCtx.Tracef("Ignoring referenced manifest %v because platform %s does not match any of: %s",
ref.Digest,
options.PlatformKey(ref.Platform.OS, ref.Platform.Architecture, ref.Platform.Variant),
strings.Join(opts.Platforms(), ","))
continue
}
ml = append(ml, ref)
}
return TagInfoFromReferences(client, opts, logCtx, ti, list.References())

// We need at least one reference that matches requested plaforms
if len(ml) == 0 {
logCtx.Debugf("Manifest list did not contain any usable reference. Platforms requested: (%s), platforms included: (%s)",
strings.Join(opts.Platforms(), ","), strings.Join(platforms, ","))
return nil, nil
}
case *ocischema.DeserializedImageIndex:
var index ocischema.DeserializedImageIndex = *deserialized

// For some strategies, we do not need to fetch metadata for further
// processing.
if !opts.WantsMetadata() {
return ti, nil
// Index must contain at least one image manifest
if len(index.Manifests) == 0 {
return nil, fmt.Errorf("empty index not supported")
}

// Loop through all referenced manifests to get their metadata. We only
// consider manifests for platforms we are interested in.
for _, ref := range ml {
logCtx.Tracef("Inspecting metadata of reference: %v", ref.Digest)

man, err := client.ManifestForDigest(ref.Digest)
if err != nil {
return nil, fmt.Errorf("could not fetch manifest %v: %v", ref.Digest, err)
}

cti, err := client.TagMetadata(man, opts)
if err != nil {
return nil, fmt.Errorf("could not fetch metadata for manifest %v: %v", ref.Digest, err)
}

// We save the timestamp of the most recent pushed manifest for any
// given reference, if the metadata for the tag was correctly
// retrieved. This is important for the latest update strategy to
// be able to handle multi-arch images. The latest strategy will
// consider the most recent reference from a manifest list.
if cti != nil {
if cti.CreatedAt.After(ti.CreatedAt) {
ti.CreatedAt = cti.CreatedAt
}
} else {
logCtx.Warnf("returned metadata for manifest %v is nil, this should not happen.", ref.Digest)
continue
}
// We use the SHA from the manifest index to let the container engine
// decide which image to pull, in case of multi-arch clusters.
_, mBytes, err := index.Payload()
if err != nil {
return nil, fmt.Errorf("could not retrieve index payload: %v", err)
}
ti.Digest = sha256.Sum256(mBytes)

return ti, nil
logCtx.Tracef("SHA256 of manifest parent is %v", ti.EncodedDigest())

return TagInfoFromReferences(client, opts, logCtx, ti, index.References())

case *schema2.DeserializedManifest:
var man schema2.Manifest = deserialized.Manifest
Expand Down Expand Up @@ -387,8 +349,74 @@ func (client *registryClient) TagMetadata(manifest distribution.Manifest, opts *

return ti, nil
default:
return nil, fmt.Errorf("invalid manifest type")
return nil, fmt.Errorf("invalid manifest type %T", manifest)
}
}

// TagInfoFromReferences is a helper method to retrieve metadata for a given
// list of references. It will return the most recent pushed manifest from the
// list of references.
func TagInfoFromReferences(client *registryClient, opts *options.ManifestOptions, logCtx *log.LogContext, ti *tag.TagInfo, references []distribution.Descriptor) (*tag.TagInfo, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is merely an internal helper, I think it could be better prototyped as:

func (client *registryClient) tagInfoFromReferences(...

to make it a method of registryClient and make it non-exported.

var ml []distribution.Descriptor
platforms := []string{}

for _, ref := range references {
platforms = append(platforms, ref.Platform.OS+"/"+ref.Platform.Architecture)
logCtx.Tracef("Found %s", options.PlatformKey(ref.Platform.OS, ref.Platform.Architecture, ref.Platform.Variant))
if !opts.WantsPlatform(ref.Platform.OS, ref.Platform.Architecture, ref.Platform.Variant) {
logCtx.Tracef("Ignoring referenced manifest %v because platform %s does not match any of: %s",
ref.Digest,
options.PlatformKey(ref.Platform.OS, ref.Platform.Architecture, ref.Platform.Variant),
strings.Join(opts.Platforms(), ","))
continue
}
ml = append(ml, ref)
}

// We need at least one reference that matches requested plaforms
if len(ml) == 0 {
logCtx.Debugf("Manifest list did not contain any usable reference. Platforms requested: (%s), platforms included: (%s)",
strings.Join(opts.Platforms(), ","), strings.Join(platforms, ","))
return nil, nil
}

// For some strategies, we do not need to fetch metadata for further
// processing.
if !opts.WantsMetadata() {
return ti, nil
}

// Loop through all referenced manifests to get their metadata. We only
// consider manifests for platforms we are interested in.
for _, ref := range ml {
logCtx.Tracef("Inspecting metadata of reference: %v", ref.Digest)

man, err := client.ManifestForDigest(ref.Digest)
if err != nil {
return nil, fmt.Errorf("could not fetch manifest %v: %v", ref.Digest, err)
}

cti, err := client.TagMetadata(man, opts)
if err != nil {
return nil, fmt.Errorf("could not fetch metadata for manifest %v: %v", ref.Digest, err)
}

// We save the timestamp of the most recent pushed manifest for any
// given reference, if the metadata for the tag was correctly
// retrieved. This is important for the latest update strategy to
// be able to handle multi-arch images. The latest strategy will
// consider the most recent reference from an image index.
if cti != nil {
if cti.CreatedAt.After(ti.CreatedAt) {
ti.CreatedAt = cti.CreatedAt
}
} else {
logCtx.Warnf("returned metadata for manifest %v is nil, this should not happen.", ref.Digest)
continue
}
}

return ti, nil
}

// Implementation of ping method to intialize the challenge list
Expand Down
Loading