From cc43edbaf3487c512de436b2df4ed2c5b46faaa9 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 4 Nov 2024 16:26:50 +0100 Subject: [PATCH] DNM: vendor c/common test PR revert https://github.com/containers/common/pull/2227 Signed-off-by: Paul Holzinger --- go.mod | 2 + go.sum | 4 +- .../containers/common/libimage/pull.go | 129 +++++++++--------- vendor/modules.txt | 3 +- 4 files changed, 72 insertions(+), 66 deletions(-) diff --git a/go.mod b/go.mod index 343cdbfaf5..2aea9982e4 100644 --- a/go.mod +++ b/go.mod @@ -229,3 +229,5 @@ require ( gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 // indirect tags.cncf.io/container-device-interface/specs-go v0.8.0 // indirect ) + +replace github.com/containers/common => github.com/Luap99/common v0.20.3-0.20241104152339-f0f87a93a145 diff --git a/go.sum b/go.sum index 03f1e5a1c8..6cd668f684 100644 --- a/go.sum +++ b/go.sum @@ -10,6 +10,8 @@ github.com/Azure/go-ansiterm v0.0.0-20230124172434-306776ec8161/go.mod h1:xomTg6 github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/BurntSushi/toml v1.4.0 h1:kuoIxZQy2WRRk1pttg9asf+WVv6tWQuBNVmK8+nqPr0= github.com/BurntSushi/toml v1.4.0/go.mod h1:ukJfTF/6rtPPRCnwkur4qwRxa8vTRFBF0uk2lLoLwho= +github.com/Luap99/common v0.20.3-0.20241104152339-f0f87a93a145 h1:XkBLGr/7QH5E2Kv7wHb7B+X5t00+VnuAYchh9zZqC+k= +github.com/Luap99/common v0.20.3-0.20241104152339-f0f87a93a145/go.mod h1:t79zzhVKgiLNv0KpBYPKXO/H0NFxbBkmyduIH9AWAlM= github.com/Microsoft/go-winio v0.6.2 h1:F2VQgta7ecxGYO8k3ZZz3RS8fVIXVxONVUPlNERoyfY= github.com/Microsoft/go-winio v0.6.2/go.mod h1:yd8OoFMLzJbo9gZq8j5qaps8bJ9aShtEA8Ipt1oGCvU= github.com/Microsoft/hcsshim v0.12.9 h1:2zJy5KA+l0loz1HzEGqyNnjd3fyZA31ZBCGKacp6lLg= @@ -81,8 +83,6 @@ github.com/containernetworking/plugins v1.5.1 h1:T5ji+LPYjjgW0QM+KyrigZbLsZ8jaX+ github.com/containernetworking/plugins v1.5.1/go.mod h1:MIQfgMayGuHYs0XdNudf31cLLAC+i242hNm6KuDGqCM= github.com/containers/buildah v1.37.1-0.20241030165353-3c433224196c h1:J8c2DjTvQSMkYngtsOAIo9xCn0hSHrOb97lsMhyrJBM= github.com/containers/buildah v1.37.1-0.20241030165353-3c433224196c/go.mod h1:iEAf1rpQW/C+l87KLtDMVUvvyT64Lz5QJP7HmCWQ6Os= -github.com/containers/common v0.60.1-0.20241101112026-fb1a5d5980ab h1:D5b9cJaUdUMoV/FHMRidZjV332yKwVElS/ii5MlITXQ= -github.com/containers/common v0.60.1-0.20241101112026-fb1a5d5980ab/go.mod h1:t79zzhVKgiLNv0KpBYPKXO/H0NFxbBkmyduIH9AWAlM= github.com/containers/conmon v2.0.20+incompatible h1:YbCVSFSCqFjjVwHTPINGdMX1F6JXHGTUje2ZYobNrkg= github.com/containers/conmon v2.0.20+incompatible/go.mod h1:hgwZ2mtuDrppv78a/cOBNiCm6O0UMWGx1mu7P00nu5I= github.com/containers/gvisor-tap-vsock v0.8.0 h1:Z8ZEWb+Lio0d+lXexONdUWT4rm9lF91vH0g3ARnMy7o= diff --git a/vendor/github.com/containers/common/libimage/pull.go b/vendor/github.com/containers/common/libimage/pull.go index ad4699c606..c4ad5df0c7 100644 --- a/vendor/github.com/containers/common/libimage/pull.go +++ b/vendor/github.com/containers/common/libimage/pull.go @@ -25,6 +25,7 @@ import ( "github.com/containers/image/v5/transports/alltransports" "github.com/containers/image/v5/types" "github.com/containers/storage" + digest "github.com/opencontainers/go-digest" ociSpec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/sirupsen/logrus" ) @@ -420,11 +421,7 @@ func (r *Runtime) copyFromRegistry(ctx context.Context, ref types.ImageReference } if !options.AllTags { - pulled, err := r.copySingleImageFromRegistry(ctx, inputName, pullPolicy, options) - if err != nil { - return nil, err - } - return []string{pulled}, nil + return r.copySingleImageFromRegistry(ctx, inputName, pullPolicy, options) } // Copy all tags @@ -450,53 +447,68 @@ func (r *Runtime) copyFromRegistry(ctx context.Context, ref types.ImageReference if err != nil { return nil, err } - pulledIDs = append(pulledIDs, pulled) + pulledIDs = append(pulledIDs, pulled...) } return pulledIDs, nil } -// imageIDForPulledImage makes a best-effort guess at an image ID for -// a just-pulled image written to destName, where the pull returned manifestBytes -func (r *Runtime) imageIDForPulledImage(destName reference.Named, manifestBytes []byte) (string, error) { - // The caller, copySingleImageFromRegistry, never triggers a multi-platform copy, so manifestBytes - // is always a single-platform manifest instance. - manifestDigest, err := manifest.Digest(manifestBytes) - if err != nil { - return "", err +// imageIDsForManifest() parses the manifest of the copied image and then looks +// up the IDs of the matching image. There's a small slice of time, between +// when we copy the image into local storage and when we go to look for it +// using the name that we gave it when we copied it, when the name we wanted to +// assign to the image could have been moved, but the image's ID will remain +// the same until it is deleted. +func (r *Runtime) imagesIDsForManifest(manifestBytes []byte, sys *types.SystemContext) ([]string, error) { + var imageDigest digest.Digest + manifestType := manifest.GuessMIMEType(manifestBytes) + if manifest.MIMETypeIsMultiImage(manifestType) { + list, err := manifest.ListFromBlob(manifestBytes, manifestType) + if err != nil { + return nil, fmt.Errorf("parsing manifest list: %w", err) + } + d, err := list.ChooseInstance(sys) + if err != nil { + return nil, fmt.Errorf("choosing instance from manifest list: %w", err) + } + imageDigest = d + } else { + d, err := manifest.Digest(manifestBytes) + if err != nil { + return nil, errors.New("digesting manifest") + } + imageDigest = d } - destDigestedName, err := reference.WithDigest(reference.TrimNamed(destName), manifestDigest) + images, err := r.store.ImagesByDigest(imageDigest) if err != nil { - return "", err + return nil, fmt.Errorf("listing images by manifest digest: %w", err) } - storeRef, err := storageTransport.Transport.NewStoreReference(r.store, destDigestedName, "") - if err != nil { - return "", err + + // If you have additionStores defined and the same image stored in + // both storage and additional store, it can be output twice. + // Fixes github.com/containers/podman/issues/18647 + results := []string{} + imageMap := map[string]bool{} + for _, image := range images { + if imageMap[image.ID] { + continue + } + imageMap[image.ID] = true + results = append(results, image.ID) } - // With zstd:chunked partial pulls, the same image can have several - // different IDs, depending on which layers of the image were pulled using the - // partial pull (are identified by TOC, not by uncompressed digest). - // - // At this point, from just the manifest digest, we can’t tell which image - // is the one that was actually pulled. (They should all have the same contents - // unless the image author is malicious.) - // - // FIXME: To return an accurate value, c/image would need to return the image ID, - // not just manifestBytes. - _, image, err := storageTransport.ResolveReference(storeRef) - if err != nil { - return "", fmt.Errorf("looking up a just-pulled image: %w", err) + if len(results) == 0 { + return nil, fmt.Errorf("identifying new image by manifest digest: %w", storage.ErrImageUnknown) } - return image.ID, nil + return results, nil } // copySingleImageFromRegistry pulls the specified, possibly unqualified, name // from a registry. On successful pull it returns the ID of the image in local -// storage (or, FIXME, a name/ID? that could be resolved in local storage) -func (r *Runtime) copySingleImageFromRegistry(ctx context.Context, imageName string, pullPolicy config.PullPolicy, options *PullOptions) (string, error) { //nolint:gocyclo +// storage. +func (r *Runtime) copySingleImageFromRegistry(ctx context.Context, imageName string, pullPolicy config.PullPolicy, options *PullOptions) ([]string, error) { //nolint:gocyclo // Sanity check. if err := pullPolicy.Validate(); err != nil { - return "", err + return nil, err } var ( @@ -521,14 +533,6 @@ func (r *Runtime) copySingleImageFromRegistry(ctx context.Context, imageName str if options.OS != runtime.GOOS { lookupImageOptions.OS = options.OS } - // FIXME: We sometimes return resolvedImageName from this function. - // The function documentation says this returns an image ID, resolvedImageName is frequently not an image ID. - // - // Ultimately Runtime.Pull looks up the returned name... again, possibly finding some other match - // than we did. - // - // This should be restructured so that the image we found here is returned to the caller of Pull - // directly, without another image -> name -> image round-trip and possible inconsistency. localImage, resolvedImageName, err = r.LookupImage(imageName, lookupImageOptions) if err != nil && !errors.Is(err, storage.ErrImageUnknown) { logrus.Errorf("Looking up %s in local storage: %v", imageName, err) @@ -559,23 +563,23 @@ func (r *Runtime) copySingleImageFromRegistry(ctx context.Context, imageName str if pullPolicy == config.PullPolicyNever { if localImage != nil { logrus.Debugf("Pull policy %q and %s resolved to local image %s", pullPolicy, imageName, resolvedImageName) - return resolvedImageName, nil + return []string{resolvedImageName}, nil } logrus.Debugf("Pull policy %q but no local image has been found for %s", pullPolicy, imageName) - return "", fmt.Errorf("%s: %w", imageName, storage.ErrImageUnknown) + return nil, fmt.Errorf("%s: %w", imageName, storage.ErrImageUnknown) } if pullPolicy == config.PullPolicyMissing && localImage != nil { - return resolvedImageName, nil + return []string{resolvedImageName}, nil } // If we looked up the image by ID, we cannot really pull from anywhere. if localImage != nil && strings.HasPrefix(localImage.ID(), imageName) { switch pullPolicy { case config.PullPolicyAlways: - return "", fmt.Errorf("pull policy is always but image has been referred to by ID (%s)", imageName) + return nil, fmt.Errorf("pull policy is always but image has been referred to by ID (%s)", imageName) default: - return resolvedImageName, nil + return []string{resolvedImageName}, nil } } @@ -600,9 +604,9 @@ func (r *Runtime) copySingleImageFromRegistry(ctx context.Context, imageName str resolved, err := shortnames.Resolve(sys, imageName) if err != nil { if localImage != nil && pullPolicy == config.PullPolicyNewer { - return resolvedImageName, nil + return []string{resolvedImageName}, nil } - return "", err + return nil, err } // NOTE: Below we print the description from the short-name resolution. @@ -634,7 +638,7 @@ func (r *Runtime) copySingleImageFromRegistry(ctx context.Context, imageName str } c, err := r.newCopier(&options.CopyOptions) if err != nil { - return "", err + return nil, err } defer c.Close() @@ -644,7 +648,7 @@ func (r *Runtime) copySingleImageFromRegistry(ctx context.Context, imageName str logrus.Debugf("Attempting to pull candidate %s for %s", candidateString, imageName) srcRef, err := registryTransport.NewReference(candidate.Value) if err != nil { - return "", err + return nil, err } if pullPolicy == config.PullPolicyNewer && localImage != nil { @@ -662,15 +666,15 @@ func (r *Runtime) copySingleImageFromRegistry(ctx context.Context, imageName str destRef, err := storageTransport.Transport.ParseStoreReference(r.store, candidate.Value.String()) if err != nil { - return "", err + return nil, err } if err := writeDesc(); err != nil { - return "", err + return nil, err } if options.Writer != nil { if _, err := io.WriteString(options.Writer, fmt.Sprintf("Trying to pull %s...\n", candidateString)); err != nil { - return "", err + return nil, err } } var manifestBytes []byte @@ -687,20 +691,19 @@ func (r *Runtime) copySingleImageFromRegistry(ctx context.Context, imageName str } logrus.Debugf("Pulled candidate %s successfully", candidateString) - ids, err := r.imageIDForPulledImage(candidate.Value, manifestBytes) - if err != nil { - return "", err + if ids, err := r.imagesIDsForManifest(manifestBytes, sys); err == nil { + return ids, nil } - return ids, nil + return []string{candidate.Value.String()}, nil } if localImage != nil && pullPolicy == config.PullPolicyNewer { - return resolvedImageName, nil + return []string{resolvedImageName}, nil } if len(pullErrors) == 0 { - return "", fmt.Errorf("internal error: no image pulled (pull policy %s)", pullPolicy) + return nil, fmt.Errorf("internal error: no image pulled (pull policy %s)", pullPolicy) } - return "", resolved.FormatPullErrors(pullErrors) + return nil, resolved.FormatPullErrors(pullErrors) } diff --git a/vendor/modules.txt b/vendor/modules.txt index 4965ed8d55..cbe679b737 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -174,7 +174,7 @@ github.com/containers/buildah/pkg/sshagent github.com/containers/buildah/pkg/util github.com/containers/buildah/pkg/volumes github.com/containers/buildah/util -# github.com/containers/common v0.60.1-0.20241101112026-fb1a5d5980ab +# github.com/containers/common v0.60.1-0.20241101112026-fb1a5d5980ab => github.com/Luap99/common v0.20.3-0.20241104152339-f0f87a93a145 ## explicit; go 1.22.0 github.com/containers/common/internal github.com/containers/common/internal/attributedstring @@ -1391,3 +1391,4 @@ tags.cncf.io/container-device-interface/pkg/parser # tags.cncf.io/container-device-interface/specs-go v0.8.0 ## explicit; go 1.19 tags.cncf.io/container-device-interface/specs-go +# github.com/containers/common => github.com/Luap99/common v0.20.3-0.20241104152339-f0f87a93a145