From bb35c303e2f9fd7a722b4d91187e6a62e3363091 Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Fri, 21 Mar 2025 14:58:21 -0400 Subject: [PATCH] Verify images in manifest list are not dangling and pruned Vendor in latest containers/common Currently if you create an un tagged image and add it to a manifest list, podman image prune will remove the image, and leave you with a broken manifest list. This PR removes the image from dangling in this situation and prevents the pruning. We have seen this trigger issues with RamaLama which is creating manifest lists in this manner, and then users pruning the images. Verifing: https://github.com/containers/common/pull/2360 Signed-off-by: Daniel J Walsh --- Makefile | 2 +- go.mod | 2 + go.sum | 4 +- pkg/domain/infra/abi/images.go | 2 +- test/system/012-manifest.bats | 26 +++ .../containers/common/libimage/copier.go | 33 +--- .../containers/common/libimage/history.go | 2 +- .../containers/common/libimage/image.go | 21 ++- .../containers/common/libimage/image_tree.go | 5 +- .../containers/common/libimage/import.go | 2 +- .../containers/common/libimage/layer_tree.go | 31 +++- .../containers/common/libimage/load.go | 10 +- .../common/libimage/manifest_list.go | 2 +- .../containers/common/libimage/pull.go | 154 ++++++++++-------- .../containers/common/libimage/push.go | 2 +- .../containers/common/libimage/runtime.go | 2 +- .../containers/common/libimage/save.go | 4 +- .../common/libnetwork/internal/util/create.go | 7 - .../libnetwork/internal/util/validate.go | 22 --- .../common/libnetwork/types/define.go | 3 - .../containers/common/pkg/config/default.go | 31 +--- vendor/modules.txt | 3 +- 22 files changed, 176 insertions(+), 194 deletions(-) diff --git a/Makefile b/Makefile index 36be6988ee..d7f55a7ba0 100644 --- a/Makefile +++ b/Makefile @@ -363,7 +363,7 @@ $(IN_CONTAINER): %-in-container: $(PODMANCMD) run --rm --env HOME=/root \ -v $(CURDIR):/src -w /src \ --security-opt label=disable \ - docker.io/library/golang:1.22 \ + docker.io/library/golang:1.23 \ make $(*) diff --git a/go.mod b/go.mod index 8c4a1abb60..3a38634046 100644 --- a/go.mod +++ b/go.mod @@ -235,3 +235,5 @@ require ( gopkg.in/yaml.v2 v2.4.0 // indirect tags.cncf.io/container-device-interface/specs-go v1.0.0 // indirect ) + +replace github.com/containers/common => github.com/rhatdan/common v0.47.1-0.20250318135319-2242b2e1f465 diff --git a/go.sum b/go.sum index bf5f2df2ea..5c0a681909 100644 --- a/go.sum +++ b/go.sum @@ -78,8 +78,6 @@ github.com/containernetworking/plugins v1.6.2 h1:pqP8Mq923TLyef5g97XfJ/xpDeVek4y github.com/containernetworking/plugins v1.6.2/go.mod h1:SP5UG3jDO9LtmfbBJdP+nl3A1atOtbj2MBOYsnaxy64= github.com/containers/buildah v1.39.1-0.20250321123219-bc4d7eb70fe3 h1:F5qpz8HsQ/nxhArveDEgskbyOjFuSsEahevt4JHAePQ= github.com/containers/buildah v1.39.1-0.20250321123219-bc4d7eb70fe3/go.mod h1:kCk5Le5CiMazPfGhF8yg43LQa1YLKqBZNnI4PTq+W/U= -github.com/containers/common v0.62.3-0.20250321171839-dbeb17e40c80 h1:U605lFaEyA0zsy4+gqZxth9V2Dl1UXBfcamA3cnQ33E= -github.com/containers/common v0.62.3-0.20250321171839-dbeb17e40c80/go.mod h1:IW8fUkTIwJkeclyROeASOV5FvFBpHjtQj/XBXffhuBk= 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.4 h1:z7MqcldnXYGaU6uTaKVl7RFxTmbhNsd2UL0CyM3fdBs= @@ -438,6 +436,8 @@ github.com/prometheus/common v0.57.0 h1:Ro/rKjwdq9mZn1K5QPctzh+MA4Lp0BuYk5ZZEVho github.com/prometheus/common v0.57.0/go.mod h1:7uRPFSUTbfZWsJ7MHY56sqt7hLQu3bxXHDnNhl8E9qI= github.com/prometheus/procfs v0.15.1 h1:YagwOFzUgYfKKHX6Dr+sHT7km/hxC76UB0learggepc= github.com/prometheus/procfs v0.15.1/go.mod h1:fB45yRUv8NstnjriLhBQLuOUt+WW4BsoGhij/e3PBqk= +github.com/rhatdan/common v0.47.1-0.20250318135319-2242b2e1f465 h1:aV/PjGzklc+iNdy86HriZ/yQ25h5sctFOFIy8O9inIs= +github.com/rhatdan/common v0.47.1-0.20250318135319-2242b2e1f465/go.mod h1:l7TYE/GilpOcGkrErMCRHfvUnftyhjUGrSL/0gYad0M= github.com/rivo/uniseg v0.2.0/go.mod h1:J6wj4VEh+S6ZtnVlnTBMWIodfgj8LQOQFoIToxlJtxc= github.com/rivo/uniseg v0.4.7 h1:WUdvkW8uEhrYfLC4ZzdpI2ztxP1I582+49Oc5Mq64VQ= github.com/rivo/uniseg v0.4.7/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUcx88= diff --git a/pkg/domain/infra/abi/images.go b/pkg/domain/infra/abi/images.go index f9c37bab74..2fa2c86052 100644 --- a/pkg/domain/infra/abi/images.go +++ b/pkg/domain/infra/abi/images.go @@ -597,7 +597,7 @@ func (ir *ImageEngine) Tree(ctx context.Context, nameOrID string, opts entities. if err != nil { return nil, err } - tree, err := image.Tree(opts.WhatRequires) + tree, err := image.Tree(ctx, opts.WhatRequires) if err != nil { return nil, err } diff --git a/test/system/012-manifest.bats b/test/system/012-manifest.bats index 4b70e82519..3cde11c736 100644 --- a/test/system/012-manifest.bats +++ b/test/system/012-manifest.bats @@ -324,4 +324,30 @@ function manifestListAddArtifactOnce() { manifestListAddArtifactOnce done } + +@test "manifest list images should not be marked as dangling" { + # build image and attach it to a manifest list + mlist=m-$(safename) + run_podman build -q -f - <<< "from scratch" + iid=${output} + run_podman manifest create ${mlist} ${iid} + + # verify image is not dangling, and is not remove via prune + run_podman images --filter dangling=true + assert "$output" != "sha256:${iid}" "Verify the filter dangling does not list the image" + run_podman image prune --force + assert "$output" != "${iid}" "Verify the prune does not remove the non dangling image" + run_podman image exists ${iid} + + # Remove manifes + run_podman manifest rm ${mlist} + + # verify the image is now dangling, and is removed via prune + run_podman images -q --filter dangling=true --no-trunc + assert "$output" == "sha256:${iid}" "Verify the filter dangling does list the image" + run_podman image prune --force + assert "$output" == "${iid}" "Verify that prune does not remove the dangling image" + run_podman 1 image exists ${iid} +} + # vim: filetype=sh diff --git a/vendor/github.com/containers/common/libimage/copier.go b/vendor/github.com/containers/common/libimage/copier.go index 5a15c28288..b7fa7534d5 100644 --- a/vendor/github.com/containers/common/libimage/copier.go +++ b/vendor/github.com/containers/common/libimage/copier.go @@ -21,10 +21,8 @@ import ( "github.com/containers/image/v5/signature" "github.com/containers/image/v5/signature/signer" storageTransport "github.com/containers/image/v5/storage" - "github.com/containers/image/v5/transports" "github.com/containers/image/v5/types" encconfig "github.com/containers/ocicrypt/config" - "github.com/containers/storage" "github.com/sirupsen/logrus" ) @@ -177,8 +175,8 @@ type Copier struct { // newCopier creates a Copier based on a runtime's system context. // Note that fields in options *may* overwrite the counterparts of // the specified system context. Please make sure to call `(*Copier).Close()`. -func (r *Runtime) newCopier(options *CopyOptions) (*Copier, error) { - return NewCopier(options, r.SystemContext(), nil) +func (r *Runtime) newCopier(options *CopyOptions, reportResolvedReference *types.ImageReference) (*Copier, error) { + return NewCopier(options, r.SystemContext(), reportResolvedReference) } // storageAllowedPolicyScopes overrides the policy for local storage @@ -352,12 +350,6 @@ func (c *Copier) Close() error { // Copy the source to the destination. Returns the bytes of the copied // manifest which may be used for digest computation. func (c *Copier) Copy(ctx context.Context, source, destination types.ImageReference) ([]byte, error) { - return c.copyInternal(ctx, source, destination, nil) -} - -// Copy the source to the destination. Returns the bytes of the copied -// manifest which may be used for digest computation. -func (c *Copier) copyInternal(ctx context.Context, source, destination types.ImageReference, reportResolvedReference *types.ImageReference) ([]byte, error) { logrus.Debugf("Copying source image %s to destination image %s", source.StringWithinTransport(), destination.StringWithinTransport()) // Avoid running out of time when running inside a systemd unit by @@ -462,11 +454,6 @@ func (c *Copier) copyInternal(ctx context.Context, source, destination types.Ima var returnManifest []byte f := func() error { opts := c.imageCopyOptions - // This is already set when `newCopier` was called but there is an option - // to override it by callers if needed. - if reportResolvedReference != nil { - opts.ReportResolvedReference = reportResolvedReference - } if sourceInsecure != nil { value := types.NewOptionalBool(*sourceInsecure) opts.SourceCtx.DockerInsecureSkipTLSVerify = value @@ -485,22 +472,6 @@ func (c *Copier) copyInternal(ctx context.Context, source, destination types.Ima return returnManifest, retry.IfNecessary(ctx, f, &c.retryOptions) } -func (c *Copier) copyToStorage(ctx context.Context, source, destination types.ImageReference) (*storage.Image, error) { - var resolvedReference types.ImageReference - _, err := c.copyInternal(ctx, source, destination, &resolvedReference) - if err != nil { - return nil, fmt.Errorf("internal error: unable to copy from source %s: %w", source, err) - } - if resolvedReference == nil { - return nil, fmt.Errorf("internal error: After attempting to copy %s, resolvedReference is nil", source) - } - _, image, err := storageTransport.ResolveReference(resolvedReference) - if err != nil { - return nil, fmt.Errorf("resolving an already-resolved reference %q to the pulled image: %w", transports.ImageName(resolvedReference), err) - } - return image, nil -} - // checkRegistrySourcesAllows checks the $BUILD_REGISTRY_SOURCES environment // variable, if it's set. The contents are expected to be a JSON-encoded // github.com/openshift/api/config/v1.Image, set by an OpenShift build diff --git a/vendor/github.com/containers/common/libimage/history.go b/vendor/github.com/containers/common/libimage/history.go index 845f4088e2..096280fc62 100644 --- a/vendor/github.com/containers/common/libimage/history.go +++ b/vendor/github.com/containers/common/libimage/history.go @@ -25,7 +25,7 @@ func (i *Image) History(ctx context.Context) ([]ImageHistory, error) { return nil, err } - layerTree, err := i.runtime.newFreshLayerTree() + layerTree, err := i.runtime.newFreshLayerTree(ctx) if err != nil { return nil, err } diff --git a/vendor/github.com/containers/common/libimage/image.go b/vendor/github.com/containers/common/libimage/image.go index d680537b16..ea7c0e6fbf 100644 --- a/vendor/github.com/containers/common/libimage/image.go +++ b/vendor/github.com/containers/common/libimage/image.go @@ -191,13 +191,21 @@ func (i *Image) IsReadOnly() bool { } // IsDangling returns true if the image is dangling, that is an untagged image -// without children. +// without children and not used in a manifest list. func (i *Image) IsDangling(ctx context.Context) (bool, error) { - return i.isDangling(ctx, nil) + images, layers, err := i.runtime.getImagesAndLayers() + if err != nil { + return false, err + } + tree, err := i.runtime.newLayerTreeFromData(ctx, images, layers, true) + if err != nil { + return false, err + } + return i.isDangling(ctx, tree) } // isDangling returns true if the image is dangling, that is an untagged image -// without children. If tree is nil, it will created for this invocation only. +// without children and not used in a manifest list. If tree is nil, it will created for this invocation only. func (i *Image) isDangling(ctx context.Context, tree *layerTree) (bool, error) { if len(i.Names()) > 0 { return false, nil @@ -206,7 +214,8 @@ func (i *Image) isDangling(ctx context.Context, tree *layerTree) (bool, error) { if err != nil { return false, err } - return len(children) == 0, nil + _, usedInManfiestList := tree.manifestListDigests[i.Digest()] + return (len(children) == 0 && !usedInManfiestList), nil } // IsIntermediate returns true if the image is an intermediate image, that is @@ -258,7 +267,7 @@ func (i *Image) TopLayer() string { // Parent returns the parent image or nil if there is none func (i *Image) Parent(ctx context.Context) (*Image, error) { - tree, err := i.runtime.newFreshLayerTree() + tree, err := i.runtime.newFreshLayerTree(ctx) if err != nil { return nil, err } @@ -292,7 +301,7 @@ func (i *Image) Children(ctx context.Context) ([]*Image, error) { // created for this invocation only. func (i *Image) getChildren(ctx context.Context, all bool, tree *layerTree) ([]*Image, error) { if tree == nil { - t, err := i.runtime.newFreshLayerTree() + t, err := i.runtime.newFreshLayerTree(ctx) if err != nil { return nil, err } diff --git a/vendor/github.com/containers/common/libimage/image_tree.go b/vendor/github.com/containers/common/libimage/image_tree.go index 9628a319f1..0c30b8a554 100644 --- a/vendor/github.com/containers/common/libimage/image_tree.go +++ b/vendor/github.com/containers/common/libimage/image_tree.go @@ -3,6 +3,7 @@ package libimage import ( + "context" "fmt" "strings" @@ -13,7 +14,7 @@ import ( // Tree generates a tree for the specified image and its layers. Use // `traverseChildren` to traverse the layers of all children. By default, only // layers of the image are printed. -func (i *Image) Tree(traverseChildren bool) (string, error) { +func (i *Image) Tree(ctx context.Context, traverseChildren bool) (string, error) { // NOTE: a string builder prevents us from copying to much data around // and compile the string when and where needed. sb := &strings.Builder{} @@ -37,7 +38,7 @@ func (i *Image) Tree(traverseChildren bool) (string, error) { fmt.Fprintf(sb, "No Image Layers") } - layerTree, err := i.runtime.newFreshLayerTree() + layerTree, err := i.runtime.newFreshLayerTree(ctx) if err != nil { return "", err } diff --git a/vendor/github.com/containers/common/libimage/import.go b/vendor/github.com/containers/common/libimage/import.go index a03f288533..ea366f7759 100644 --- a/vendor/github.com/containers/common/libimage/import.go +++ b/vendor/github.com/containers/common/libimage/import.go @@ -104,7 +104,7 @@ func (r *Runtime) Import(ctx context.Context, path string, options *ImportOption return "", err } - c, err := r.newCopier(&options.CopyOptions) + c, err := r.newCopier(&options.CopyOptions, nil) if err != nil { return "", err } diff --git a/vendor/github.com/containers/common/libimage/layer_tree.go b/vendor/github.com/containers/common/libimage/layer_tree.go index 33741bd083..2c548933c8 100644 --- a/vendor/github.com/containers/common/libimage/layer_tree.go +++ b/vendor/github.com/containers/common/libimage/layer_tree.go @@ -8,6 +8,7 @@ import ( "github.com/containers/storage" storageTypes "github.com/containers/storage/types" + digest "github.com/opencontainers/go-digest" ociv1 "github.com/opencontainers/image-spec/specs-go/v1" "github.com/sirupsen/logrus" ) @@ -22,6 +23,10 @@ type layerTree struct { // emptyImages do not have any top-layer so we cannot create a // *layerNode for them. emptyImages []*Image + // manifestList keep track of images based on their digest. + // Library will use this map when checking if a image is dangling. + // If an image is used in a manifestList it is NOT dangling + manifestListDigests map[digest.Digest]struct{} } // node returns a layerNode for the specified layerID. @@ -90,20 +95,21 @@ func (l *layerNode) repoTags() ([]string, error) { } // newFreshLayerTree extracts a layerTree from consistent layers and images in the local storage. -func (r *Runtime) newFreshLayerTree() (*layerTree, error) { +func (r *Runtime) newFreshLayerTree(ctx context.Context) (*layerTree, error) { images, layers, err := r.getImagesAndLayers() if err != nil { return nil, err } - return r.newLayerTreeFromData(images, layers) + return r.newLayerTreeFromData(ctx, images, layers, false) } // newLayerTreeFromData extracts a layerTree from the given the layers and images. // The caller is responsible for (layers, images) being consistent. -func (r *Runtime) newLayerTreeFromData(images []*Image, layers []storage.Layer) (*layerTree, error) { +func (r *Runtime) newLayerTreeFromData(ctx context.Context, images []*Image, layers []storage.Layer, generateManifestDigestList bool) (*layerTree, error) { tree := layerTree{ - nodes: make(map[string]*layerNode), - ociCache: make(map[string]*ociv1.Image), + nodes: make(map[string]*layerNode), + ociCache: make(map[string]*ociv1.Image), + manifestListDigests: make(map[digest.Digest]struct{}), } // First build a tree purely based on layer information. @@ -124,6 +130,21 @@ func (r *Runtime) newLayerTreeFromData(images []*Image, layers []storage.Layer) topLayer := img.TopLayer() if topLayer == "" { tree.emptyImages = append(tree.emptyImages, img) + // When img is a manifest list, cache the lists of + // digests refereenced in manifest list. Digests can + // be used to check for dangling images. + if !generateManifestDigestList { + continue + } + if manifestList, _ := img.IsManifestList(ctx); manifestList { + mlist, err := img.ToManifestList() + if err != nil { + return nil, err + } + for _, digest := range mlist.list.Instances() { + tree.manifestListDigests[digest] = struct{}{} + } + } continue } node, exists := tree.nodes[topLayer] diff --git a/vendor/github.com/containers/common/libimage/load.go b/vendor/github.com/containers/common/libimage/load.go index 48f255e8b1..0327cbca38 100644 --- a/vendor/github.com/containers/common/libimage/load.go +++ b/vendor/github.com/containers/common/libimage/load.go @@ -30,7 +30,7 @@ func (r *Runtime) doLoadReference(ctx context.Context, ref types.ImageReference, case dockerArchiveTransport.Transport.Name(): images, err = r.loadMultiImageDockerArchive(ctx, ref, &options.CopyOptions) default: - _, images, err = r.copyFromDefault(ctx, ref, &options.CopyOptions) + images, err = r.copyFromDefault(ctx, ref, &options.CopyOptions) } return images, ref.Transport().Name(), err } @@ -49,9 +49,6 @@ func (r *Runtime) LoadReference(ctx context.Context, ref types.ImageReference, o // Load loads one or more images (depending on the transport) from the // specified path. The path may point to an image the following transports: // oci, oci-archive, dir, docker-archive. -// -// Load returns a string slice with names of recently loaded images. -// If images are unnamed in the source, it returns a string slice of image IDs instead. func (r *Runtime) Load(ctx context.Context, path string, options *LoadOptions) ([]string, error) { logrus.Debugf("Loading image from %q", path) @@ -145,8 +142,7 @@ func (r *Runtime) loadMultiImageDockerArchive(ctx context.Context, ref types.Ima // should. path := ref.StringWithinTransport() if err := fileutils.Exists(path); err != nil { - _, names, err := r.copyFromDockerArchive(ctx, ref, options) - return names, err + return r.copyFromDockerArchive(ctx, ref, options) } reader, err := dockerArchiveTransport.NewReader(r.systemContextCopy(), path) @@ -167,7 +163,7 @@ func (r *Runtime) loadMultiImageDockerArchive(ctx context.Context, ref types.Ima var copiedImages []string for _, list := range refLists { for _, listRef := range list { - _, names, err := r.copyFromDockerArchiveReaderReference(ctx, reader, listRef, options) + names, err := r.copyFromDockerArchiveReaderReference(ctx, reader, listRef, options) if err != nil { return nil, err } diff --git a/vendor/github.com/containers/common/libimage/manifest_list.go b/vendor/github.com/containers/common/libimage/manifest_list.go index d9140db7c2..83d3e1a9e6 100644 --- a/vendor/github.com/containers/common/libimage/manifest_list.go +++ b/vendor/github.com/containers/common/libimage/manifest_list.go @@ -794,7 +794,7 @@ func (m *ManifestList) Push(ctx context.Context, destination string, options *Ma // NOTE: we're using the logic in copier to create a proper // types.SystemContext. This prevents us from having an error prone // code duplicate here. - copier, err := m.image.runtime.newCopier(&options.CopyOptions) + copier, err := m.image.runtime.newCopier(&options.CopyOptions, nil) if err != nil { return "", err } diff --git a/vendor/github.com/containers/common/libimage/pull.go b/vendor/github.com/containers/common/libimage/pull.go index 522bebe413..02b6a42338 100644 --- a/vendor/github.com/containers/common/libimage/pull.go +++ b/vendor/github.com/containers/common/libimage/pull.go @@ -21,6 +21,7 @@ import ( ociTransport "github.com/containers/image/v5/oci/layout" "github.com/containers/image/v5/pkg/shortnames" storageTransport "github.com/containers/image/v5/storage" + "github.com/containers/image/v5/transports" "github.com/containers/image/v5/transports/alltransports" "github.com/containers/image/v5/types" "github.com/containers/storage" @@ -51,10 +52,6 @@ type PullOptions struct { // The error is storage.ErrImageUnknown iff the pull policy is set to "never" // and no local image has been found. This allows for an easier integration // into some users of this package (e.g., Buildah). -// -// Pull returns a slice of the pulled images. -// -// WARNING: the Digest field of the returned image might not be a value relevant to the user issuing the pull. func (r *Runtime) Pull(ctx context.Context, name string, pullPolicy config.PullPolicy, options *PullOptions) (_ []*Image, pullError error) { logrus.Debugf("Pulling image %s (policy: %s)", name, pullPolicy) if r.eventChannel != nil { @@ -159,7 +156,7 @@ func (r *Runtime) Pull(ctx context.Context, name string, pullPolicy config.PullP options.Variant = r.systemContext.VariantChoice } - var pulledImages []*Image + var pulledImages []string // Dispatch the copy operation. switch ref.Transport().Name() { @@ -169,18 +166,24 @@ func (r *Runtime) Pull(ctx context.Context, name string, pullPolicy config.PullP // DOCKER ARCHIVE case dockerArchiveTransport.Transport.Name(): - pulledImages, _, err = r.copyFromDockerArchive(ctx, ref, &options.CopyOptions) + pulledImages, err = r.copyFromDockerArchive(ctx, ref, &options.CopyOptions) // ALL OTHER TRANSPORTS default: - pulledImages, _, err = r.copyFromDefault(ctx, ref, &options.CopyOptions) + pulledImages, err = r.copyFromDefault(ctx, ref, &options.CopyOptions) } if err != nil { return nil, err } - for _, image := range pulledImages { + localImages := []*Image{} + for _, iName := range pulledImages { + image, _, err := r.LookupImage(iName, nil) + if err != nil { + return nil, fmt.Errorf("locating pulled image %q name in containers storage: %w", iName, err) + } + // Note that we can ignore the 2nd return value here. Some // images may ship with "wrong" platform, but we already warn // about it. Throwing an error is not (yet) the plan. @@ -203,9 +206,11 @@ func (r *Runtime) Pull(ctx context.Context, name string, pullPolicy config.PullP // Note that we use the input name here to preserve the transport data. r.writeEvent(&Event{ID: image.ID(), Name: name, Time: time.Now(), Type: EventTypeImagePull}) } + + localImages = append(localImages, image) } - return pulledImages, pullError + return localImages, pullError } // nameFromAnnotations returns a reference string to be used as an image name, @@ -224,10 +229,10 @@ func nameFromAnnotations(annotations map[string]string) string { // copyFromDefault is the default copier for a number of transports. Other // transports require some specific dancing, sometimes Yoga. -func (r *Runtime) copyFromDefault(ctx context.Context, ref types.ImageReference, options *CopyOptions) ([]*Image, []string, error) { - c, err := r.newCopier(options) +func (r *Runtime) copyFromDefault(ctx context.Context, ref types.ImageReference, options *CopyOptions) ([]string, error) { + c, err := r.newCopier(options, nil) if err != nil { - return nil, nil, err + return nil, err } defer c.Close() @@ -238,7 +243,7 @@ func (r *Runtime) copyFromDefault(ctx context.Context, ref types.ImageReference, // Normalize to docker.io if needed (see containers/podman/issues/10998). named, err := reference.ParseNormalizedNamed(strings.TrimLeft(ref.StringWithinTransport(), ":/")) if err != nil { - return nil, nil, err + return nil, err } imageName = named.String() storageName = imageName @@ -247,7 +252,7 @@ func (r *Runtime) copyFromDefault(ctx context.Context, ref types.ImageReference, // Normalize to docker.io if needed (see containers/podman/issues/10998). named, err := reference.ParseNormalizedNamed(ref.StringWithinTransport()) if err != nil { - return nil, nil, err + return nil, err } imageName = named.String() storageName = imageName @@ -259,7 +264,7 @@ func (r *Runtime) copyFromDefault(ctx context.Context, ref types.ImageReference, // the path to a directory as the name. storageName, err = getImageID(ctx, ref, nil) if err != nil { - return nil, nil, err + return nil, err } imageName = "sha256:" + storageName[1:] } else { // If the OCI-reference includes an image reference, use it @@ -270,7 +275,7 @@ func (r *Runtime) copyFromDefault(ctx context.Context, ref types.ImageReference, case ociArchiveTransport.Transport.Name(): manifestDescriptor, err := ociArchiveTransport.LoadManifestDescriptorWithContext(r.SystemContext(), ref) if err != nil { - return nil, nil, err + return nil, err } storageName = nameFromAnnotations(manifestDescriptor.Annotations) switch len(storageName) { @@ -278,13 +283,13 @@ func (r *Runtime) copyFromDefault(ctx context.Context, ref types.ImageReference, // If there's no reference name in the annotations, compute an ID. storageName, err = getImageID(ctx, ref, nil) if err != nil { - return nil, nil, err + return nil, err } imageName = "sha256:" + storageName[1:] default: named, err := NormalizeName(storageName) if err != nil { - return nil, nil, err + return nil, err } imageName = named.String() storageName = imageName @@ -294,7 +299,7 @@ func (r *Runtime) copyFromDefault(ctx context.Context, ref types.ImageReference, storageName = ref.StringWithinTransport() named := ref.DockerReference() if named == nil { - return nil, nil, fmt.Errorf("could not get an image name for storage reference %q", ref) + return nil, fmt.Errorf("could not get an image name for storage reference %q", ref) } imageName = named.String() @@ -304,7 +309,7 @@ func (r *Runtime) copyFromDefault(ctx context.Context, ref types.ImageReference, // instead of looking at the StringWithinTransport(). storageName, err = getImageID(ctx, ref, nil) if err != nil { - return nil, nil, err + return nil, err } imageName = "sha256:" + storageName[1:] } @@ -312,14 +317,11 @@ func (r *Runtime) copyFromDefault(ctx context.Context, ref types.ImageReference, // Create a storage reference. destRef, err := storageTransport.Transport.ParseStoreReference(r.store, storageName) if err != nil { - return nil, nil, fmt.Errorf("parsing %q: %w", storageName, err) + return nil, fmt.Errorf("parsing %q: %w", storageName, err) } - image, err := c.copyToStorage(ctx, ref, destRef) - if err != nil { - return nil, nil, fmt.Errorf("unable to perform copy: %w", err) - } - resolvedImage := r.storageToImage(image, nil) - return []*Image{resolvedImage}, []string{imageName}, err + + _, err = c.Copy(ctx, ref, destRef) + return []string{imageName}, err } // storageReferencesFromArchiveReader returns a slice of image references inside the @@ -366,12 +368,12 @@ func (r *Runtime) storageReferencesReferencesFromArchiveReader(ctx context.Conte } // copyFromDockerArchive copies one image from the specified reference. -func (r *Runtime) copyFromDockerArchive(ctx context.Context, ref types.ImageReference, options *CopyOptions) ([]*Image, []string, error) { +func (r *Runtime) copyFromDockerArchive(ctx context.Context, ref types.ImageReference, options *CopyOptions) ([]string, error) { // There may be more than one image inside the docker archive, so we // need a quick glimpse inside. reader, readerRef, err := dockerArchiveTransport.NewReaderForReference(&r.systemContext, ref) if err != nil { - return nil, nil, err + return nil, err } defer func() { if err := reader.Close(); err != nil { @@ -383,38 +385,35 @@ func (r *Runtime) copyFromDockerArchive(ctx context.Context, ref types.ImageRefe } // copyFromDockerArchiveReaderReference copies the specified readerRef from reader. -func (r *Runtime) copyFromDockerArchiveReaderReference(ctx context.Context, reader *dockerArchiveTransport.Reader, readerRef types.ImageReference, options *CopyOptions) ([]*Image, []string, error) { - c, err := r.newCopier(options) +func (r *Runtime) copyFromDockerArchiveReaderReference(ctx context.Context, reader *dockerArchiveTransport.Reader, readerRef types.ImageReference, options *CopyOptions) ([]string, error) { + c, err := r.newCopier(options, nil) if err != nil { - return nil, nil, err + return nil, err } defer c.Close() // Get a slice of storage references we can copy. references, destNames, err := r.storageReferencesReferencesFromArchiveReader(ctx, readerRef, reader) if err != nil { - return nil, nil, err + return nil, err } - images := []*Image{} // Now copy all of the images. Use readerRef for performance. for _, destRef := range references { - image, err := c.copyToStorage(ctx, readerRef, destRef) - if err != nil { - return nil, nil, err + if _, err := c.Copy(ctx, readerRef, destRef); err != nil { + return nil, err } - resolvedImage := r.storageToImage(image, nil) - images = append(images, resolvedImage) } - return images, destNames, nil + return destNames, nil } // copyFromRegistry pulls the specified, possibly unqualified, name from a -// registry. On successful pull it returns slice of the pulled images. +// registry. On successful pull it returns the ID of the image in local +// storage. // // If options.All is set, all tags from the specified registry will be pulled. -func (r *Runtime) copyFromRegistry(ctx context.Context, ref types.ImageReference, inputName string, pullPolicy config.PullPolicy, options *PullOptions) ([]*Image, error) { +func (r *Runtime) copyFromRegistry(ctx context.Context, ref types.ImageReference, inputName string, pullPolicy config.PullPolicy, options *PullOptions) ([]string, error) { // Sanity check. if err := pullPolicy.Validate(); err != nil { return nil, err @@ -425,7 +424,7 @@ func (r *Runtime) copyFromRegistry(ctx context.Context, ref types.ImageReference if err != nil { return nil, err } - return []*Image{pulled}, nil + return []string{pulled}, nil } // Copy all tags @@ -435,7 +434,7 @@ func (r *Runtime) copyFromRegistry(ctx context.Context, ref types.ImageReference return nil, err } - pulledImages := []*Image{} + pulledIDs := []string{} for _, tag := range tags { select { // Let's be gentle with Podman remote. case <-ctx.Done(): @@ -451,18 +450,19 @@ func (r *Runtime) copyFromRegistry(ctx context.Context, ref types.ImageReference if err != nil { return nil, err } - pulledImages = append(pulledImages, pulled) + pulledIDs = append(pulledIDs, pulled) } - return pulledImages, nil + return pulledIDs, nil } // copySingleImageFromRegistry pulls the specified, possibly unqualified, name -// from a registry. On successful pull it returns the Image from the local storage. -func (r *Runtime) copySingleImageFromRegistry(ctx context.Context, imageName string, pullPolicy config.PullPolicy, options *PullOptions) (*Image, error) { //nolint:gocyclo +// 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 // Sanity check. if err := pullPolicy.Validate(); err != nil { - return nil, err + return "", err } var ( @@ -487,7 +487,14 @@ 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) @@ -518,23 +525,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 localImage, nil + return resolvedImageName, nil } logrus.Debugf("Pull policy %q but no local image has been found for %s", pullPolicy, imageName) - return nil, fmt.Errorf("%s: %w", imageName, storage.ErrImageUnknown) + return "", fmt.Errorf("%s: %w", imageName, storage.ErrImageUnknown) } if pullPolicy == config.PullPolicyMissing && localImage != nil { - return localImage, nil + return 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 nil, fmt.Errorf("pull policy is always but image has been referred to by ID (%s)", imageName) + return "", fmt.Errorf("pull policy is always but image has been referred to by ID (%s)", imageName) default: - return localImage, nil + return resolvedImageName, nil } } @@ -559,9 +566,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 localImage, nil + return resolvedImageName, nil } - return nil, err + return "", err } // NOTE: Below we print the description from the short-name resolution. @@ -591,9 +598,10 @@ func (r *Runtime) copySingleImageFromRegistry(ctx context.Context, imageName str if socketPath, ok := os.LookupEnv("NOTIFY_SOCKET"); ok { options.extendTimeoutSocket = socketPath } - c, err := r.newCopier(&options.CopyOptions) + var resolvedReference types.ImageReference + c, err := r.newCopier(&options.CopyOptions, &resolvedReference) if err != nil { - return nil, err + return "", err } defer c.Close() @@ -603,7 +611,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 nil, err + return "", err } if pullPolicy == config.PullPolicyNewer && localImage != nil { @@ -621,19 +629,18 @@ func (r *Runtime) copySingleImageFromRegistry(ctx context.Context, imageName str destRef, err := storageTransport.Transport.ParseStoreReference(r.store, candidate.Value.String()) if err != nil { - return nil, err + return "", err } if err := writeDesc(); err != nil { - return nil, err + return "", err } if options.Writer != nil { if _, err := io.WriteString(options.Writer, fmt.Sprintf("Trying to pull %s...\n", candidateString)); err != nil { - return nil, err + return "", err } } - image, err := c.copyToStorage(ctx, srcRef, destRef) - if err != nil { + if _, err := c.Copy(ctx, srcRef, destRef); err != nil { logrus.Debugf("Error pulling candidate %s: %v", candidateString, err) pullErrors = append(pullErrors, err) continue @@ -644,18 +651,25 @@ func (r *Runtime) copySingleImageFromRegistry(ctx context.Context, imageName str // read-only which can cause issues. logrus.Errorf("Error recording short-name alias %q: %v", candidateString, err) } + logrus.Debugf("Pulled candidate %s successfully", candidateString) - resolvedImage := r.storageToImage(image, nil) - return resolvedImage, err + if resolvedReference == nil { // resolvedReference should always be set for storageTransport destinations + return "", fmt.Errorf("internal error: After pulling %s, resolvedReference is nil", candidateString) + } + _, image, err := storageTransport.ResolveReference(resolvedReference) + if err != nil { + return "", fmt.Errorf("resolving an already-resolved reference %q to the pulled image: %w", transports.ImageName(resolvedReference), err) + } + return image.ID, nil } if localImage != nil && pullPolicy == config.PullPolicyNewer { - return localImage, nil + return resolvedImageName, nil } if len(pullErrors) == 0 { - return nil, fmt.Errorf("internal error: no image pulled (pull policy %s)", pullPolicy) + return "", fmt.Errorf("internal error: no image pulled (pull policy %s)", pullPolicy) } - return nil, resolved.FormatPullErrors(pullErrors) + return "", resolved.FormatPullErrors(pullErrors) } diff --git a/vendor/github.com/containers/common/libimage/push.go b/vendor/github.com/containers/common/libimage/push.go index a4fddd4643..cac8fb6024 100644 --- a/vendor/github.com/containers/common/libimage/push.go +++ b/vendor/github.com/containers/common/libimage/push.go @@ -118,7 +118,7 @@ func (r *Runtime) Push(ctx context.Context, source, destination string, options } } - c, err := r.newCopier(&options.CopyOptions) + c, err := r.newCopier(&options.CopyOptions, nil) if err != nil { return nil, err } diff --git a/vendor/github.com/containers/common/libimage/runtime.go b/vendor/github.com/containers/common/libimage/runtime.go index f7c9b2bf97..1e01a5f0e9 100644 --- a/vendor/github.com/containers/common/libimage/runtime.go +++ b/vendor/github.com/containers/common/libimage/runtime.go @@ -634,7 +634,7 @@ func (r *Runtime) ListImages(ctx context.Context, options *ListImagesOptions) ([ var tree *layerTree if needsLayerTree { - tree, err = r.newLayerTreeFromData(images, snapshot.Layers) + tree, err = r.newLayerTreeFromData(ctx, images, snapshot.Layers, true) if err != nil { return nil, err } diff --git a/vendor/github.com/containers/common/libimage/save.go b/vendor/github.com/containers/common/libimage/save.go index 46529d10f3..c9cc8a5073 100644 --- a/vendor/github.com/containers/common/libimage/save.go +++ b/vendor/github.com/containers/common/libimage/save.go @@ -119,7 +119,7 @@ func (r *Runtime) saveSingleImage(ctx context.Context, name, format, path string return err } - c, err := r.newCopier(&options.CopyOptions) + c, err := r.newCopier(&options.CopyOptions, nil) if err != nil { return err } @@ -204,7 +204,7 @@ func (r *Runtime) saveDockerArchive(ctx context.Context, names []string, path st copyOpts := options.CopyOptions copyOpts.dockerArchiveAdditionalTags = local.tags - c, err := r.newCopier(©Opts) + c, err := r.newCopier(©Opts, nil) if err != nil { return err } diff --git a/vendor/github.com/containers/common/libnetwork/internal/util/create.go b/vendor/github.com/containers/common/libnetwork/internal/util/create.go index 376a57315b..1bd2c32790 100644 --- a/vendor/github.com/containers/common/libnetwork/internal/util/create.go +++ b/vendor/github.com/containers/common/libnetwork/internal/util/create.go @@ -39,13 +39,6 @@ func CommonNetworkCreate(n NetUtil, network *types.Network) error { network.NetworkInterface = name } } - - // Validate interface name if specified - if network.NetworkInterface != "" { - if err := ValidateInterfaceName(network.NetworkInterface); err != nil { - return fmt.Errorf("network interface name %s invalid: %w", network.NetworkInterface, err) - } - } return nil } diff --git a/vendor/github.com/containers/common/libnetwork/internal/util/validate.go b/vendor/github.com/containers/common/libnetwork/internal/util/validate.go index 28e6700045..ddc778a5fe 100644 --- a/vendor/github.com/containers/common/libnetwork/internal/util/validate.go +++ b/vendor/github.com/containers/common/libnetwork/internal/util/validate.go @@ -4,8 +4,6 @@ import ( "errors" "fmt" "net" - "strings" - "unicode" "github.com/containers/common/libnetwork/types" "github.com/containers/common/libnetwork/util" @@ -161,23 +159,3 @@ func validatePerNetworkOpts(network *types.Network, netOpts *types.PerNetworkOpt } return nil } - -// ValidateInterfaceName validates the interface name based on the following rules: -// 1. The name must be less than MaxInterfaceNameLength characters -// 2. The name must not be "." or ".." -// 3. The name must not contain / or : or any whitespace characters -// ref to https://github.com/torvalds/linux/blob/81e4f8d68c66da301bb881862735bd74c6241a19/include/uapi/linux/if.h#L33C18-L33C20 -func ValidateInterfaceName(ifName string) error { - if len(ifName) > types.MaxInterfaceNameLength { - return fmt.Errorf("interface name is too long: interface names must be %d characters or less: %w", types.MaxInterfaceNameLength, types.ErrInvalidArg) - } - if ifName == "." || ifName == ".." { - return fmt.Errorf("interface name is . or ..: %w", types.ErrInvalidArg) - } - if strings.ContainsFunc(ifName, func(r rune) bool { - return r == '/' || r == ':' || unicode.IsSpace(r) - }) { - return fmt.Errorf("interface name contains / or : or whitespace characters: %w", types.ErrInvalidArg) - } - return nil -} diff --git a/vendor/github.com/containers/common/libnetwork/types/define.go b/vendor/github.com/containers/common/libnetwork/types/define.go index 0bc688103e..193377b1a2 100644 --- a/vendor/github.com/containers/common/libnetwork/types/define.go +++ b/vendor/github.com/containers/common/libnetwork/types/define.go @@ -30,7 +30,4 @@ var ( // NotHexRegex is a regular expression to check if a string is // a hexadecimal string. NotHexRegex = regexp.Delayed(`[^0-9a-fA-F]`) - - // MaxInterfaceNameLength is the maximum length of a network interface name - MaxInterfaceNameLength = 15 ) diff --git a/vendor/github.com/containers/common/pkg/config/default.go b/vendor/github.com/containers/common/pkg/config/default.go index ff93e2edba..ab22ae7aa8 100644 --- a/vendor/github.com/containers/common/pkg/config/default.go +++ b/vendor/github.com/containers/common/pkg/config/default.go @@ -8,7 +8,6 @@ import ( "path/filepath" "runtime" "strings" - "sync" "github.com/containers/common/internal/attributedstring" nettypes "github.com/containers/common/libnetwork/types" @@ -37,8 +36,8 @@ const ( defaultInitName = "catatonit" ) -func getMaskedPaths() ([]string, error) { - maskedPaths := []string{ +var ( + DefaultMaskedPaths = []string{ "/proc/acpi", "/proc/kcore", "/proc/keys", @@ -50,34 +49,8 @@ func getMaskedPaths() ([]string, error) { "/sys/devices/virtual/powercap", "/sys/firmware", "/sys/fs/selinux", - "/proc/interrupts", - } - maskedPathsToGlob := []string{ - "/sys/devices/system/cpu/cpu*/thermal_throttle", } - for _, p := range maskedPathsToGlob { - matches, err := filepath.Glob(p) - if err != nil { - return nil, err - } - maskedPaths = append(maskedPaths, matches...) - } - return maskedPaths, nil -} - -var DefaultMaskedPaths = sync.OnceValue(func() []string { - maskedPaths, err := getMaskedPaths() - // this should never happen, the only error possible - // is ErrBadPattern and the patterns that were added must be valid - if err != nil { - panic(err) - } - - return maskedPaths -}) - -var ( DefaultReadOnlyPaths = []string{ "/proc/asound", "/proc/bus", diff --git a/vendor/modules.txt b/vendor/modules.txt index 564f116d77..4411ae6d6d 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -179,7 +179,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.62.3-0.20250321171839-dbeb17e40c80 +# github.com/containers/common v0.62.3-0.20250321171839-dbeb17e40c80 => github.com/rhatdan/common v0.47.1-0.20250318135319-2242b2e1f465 ## explicit; go 1.23.0 github.com/containers/common/internal github.com/containers/common/internal/attributedstring @@ -1423,3 +1423,4 @@ tags.cncf.io/container-device-interface/pkg/parser # tags.cncf.io/container-device-interface/specs-go v1.0.0 ## explicit; go 1.19 tags.cncf.io/container-device-interface/specs-go +# github.com/containers/common => github.com/rhatdan/common v0.47.1-0.20250318135319-2242b2e1f465