Skip to content

Commit

Permalink
fix: Add support for ocischema.DeserializedImageIndex manifest type (#…
Browse files Browse the repository at this point in the history
…618)

* fix: Add support for ocischema.DeserializedImageIndex manifest type

Signed-off-by: Jesse Bye <[email protected]>

* more informative error logging

Signed-off-by: Jesse Bye <[email protected]>

---------

Signed-off-by: Jesse Bye <[email protected]>
  • Loading branch information
jessebye authored Sep 21, 2023
1 parent 202f6bf commit 420bd97
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 55 deletions.
2 changes: 1 addition & 1 deletion pkg/argocd/update_test.go
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
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) {
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

1 comment on commit 420bd97

@github-actions
Copy link

Choose a reason for hiding this comment

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

@check-spelling-bot Report

🔴 Please review

See the 📜action log or 📝 job summary for details.

❌ Errors Count
❌ dictionary-not-found 3

See ❌ Event descriptions for more information.

Available 📚 dictionaries could cover words (expected and unrecognized) not in the 📘 dictionary

This includes both expected items (21) from .github/actions/spelling/expect.txt and unrecognized words (0)

Dictionary Entries Covers Uniquely
cspell:typescript/dict/typescript.txt 1098 1 1
cspell:cpp/src/template-strings.txt 8 1
cspell:software-terms/dict/softwareTerms.txt 1288 1

Consider adding them (in .github/workflows/spelling.yml):

      with:
        extra_dictionaries:
          cspell:typescript/dict/typescript.txt
          cspell:cpp/src/template-strings.txt
          cspell:software-terms/dict/softwareTerms.txt

To stop checking additional dictionaries, add (in .github/workflows/spelling.yml):

check_extra_dictionaries: ''

Please sign in to comment.