Skip to content

Commit

Permalink
fix: Allow to delete images when names of images are short digest ids…
Browse files Browse the repository at this point in the history
… of another images

Signed-off-by: Hayato Kiwata <[email protected]>
  • Loading branch information
haytok committed Oct 17, 2024
1 parent a1e9890 commit 5b7ccd7
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 29 deletions.
55 changes: 55 additions & 0 deletions cmd/nerdctl/image/image_remove_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@ package image

import (
"errors"
"strings"
"testing"

"gotest.tools/v3/assert"

"github.com/containerd/nerdctl/v2/pkg/imgutil"
"github.com/containerd/nerdctl/v2/pkg/testutil"
"github.com/containerd/nerdctl/v2/pkg/testutil/nerdtest"
Expand Down Expand Up @@ -302,3 +305,55 @@ func TestRemove(t *testing.T) {

testCase.Run(t)
}

// TestIssue3016 tests https://github.com/containerd/nerdctl/issues/3016
func TestIssue3016(t *testing.T) {
testCase := nerdtest.Setup()

const (
alpineImageName = "alpine"
busyboxImageName = "busybox"
tagIDKey = "tagID"
)

testCase.SubTests = []*test.Case{
{
Description: "Issue #3016 - Tags created using the short digest ids of container images cannot be deleted using the nerdctl rmi command.",
Setup: func(data test.Data, helpers test.Helpers) {
helpers.Ensure("pull", alpineImageName)
helpers.Ensure("pull", busyboxImageName)

img := nerdtest.InspectImage(helpers, busyboxImageName)
tagID := strings.TrimPrefix(img.RepoDigests[0], "busybox@sha256:")[0:8]
assert.Equal(t, len(tagID), 8)

helpers.Ensure("tag", alpineImageName, tagID)

data.Set(tagIDKey, tagID)
},
Cleanup: func(data test.Data, helpers test.Helpers) {
helpers.Anyhow("rmi", alpineImageName)
helpers.Anyhow("rmi", busyboxImageName)
},
Command: func(data test.Data, helpers test.Helpers) test.TestableCommand {
return helpers.Command("rmi", data.Get(tagIDKey))
},
Expected: func(data test.Data, helpers test.Helpers) *test.Expected {
return &test.Expected{
ExitCode: 0,
Errors: []error{},
Output: func(stdout string, info string, t *testing.T) {
helpers.Command("images", data.Get(tagIDKey)).Run(&test.Expected{
ExitCode: 0,
Output: func(stdout string, info string, t *testing.T) {
assert.Equal(t, len(strings.Split(stdout, "\n")), 2)
},
})
},
}
},
},
}

testCase.Run(t)
}
15 changes: 11 additions & 4 deletions pkg/cmd/image/remove.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,18 @@ func Remove(ctx context.Context, client *containerd.Client, args []string, optio
walker := &imagewalker.ImageWalker{
Client: client,
OnFound: func(ctx context.Context, found imagewalker.Found) error {
// if found multiple images, return error unless in force-mode and
// there is only 1 unique image.
if found.MatchCount > 1 && !(options.Force && found.UniqueImages == 1) {
return fmt.Errorf("multiple IDs found with provided prefix: %s", found.Req)
if found.NameMatchIndex == -1 {
// if found multiple images, return error unless in force-mode and
// there is only 1 unique image.
if found.MatchCount > 1 && !(options.Force && found.UniqueImages == 1) {
return fmt.Errorf("multiple IDs found with provided prefix: %s", found.Req)
}
} else if found.NameMatchIndex != found.MatchIndex {
// when there is an image with a name matching the argument but the argument is a digest short id,
// the deletion process is not performed.
return nil
}

if cid, ok := runningImages[found.Image.Name]; ok {
return fmt.Errorf("conflict: unable to delete %s (cannot be forced) - image is being used by running container %s", found.Req, cid)
}
Expand Down
49 changes: 24 additions & 25 deletions pkg/idutil/imagewalker/imagewalker.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,12 @@ import (
)

type Found struct {
Image images.Image
Req string // The raw request string. name, short ID, or long ID.
MatchIndex int // Begins with 0, up to MatchCount - 1.
MatchCount int // 1 on exact match. > 1 on ambiguous match. Never be <= 0.
UniqueImages int // Number of unique images in all found images.
Image images.Image
Req string // The raw request string. name, short ID, or long ID.
MatchIndex int // Begins with 0, up to MatchCount - 1.
MatchCount int // 1 on exact match. > 1 on ambiguous match. Never be <= 0.
UniqueImages int // Number of unique images in all found images.
NameMatchIndex int // Image index with a name matching the argument for `nerdctl rmi`.
}

type OnFound func(ctx context.Context, found Found) error
Expand All @@ -50,8 +51,12 @@ type ImageWalker struct {
// Returns the number of the found entries.
func (w *ImageWalker) Walk(ctx context.Context, req string) (int, error) {
var filters []string
if parsedReference, err := referenceutil.Parse(req); err == nil {
filters = append(filters, fmt.Sprintf("name==%s", parsedReference.String()))
var parsedReferenceStr string

parsedReference, err := referenceutil.Parse(req)
if err == nil {
parsedReferenceStr = parsedReference.String()
filters = append(filters, fmt.Sprintf("name==%s", parsedReferenceStr))
}
filters = append(filters,
fmt.Sprintf("name==%s", req),
Expand All @@ -64,33 +69,27 @@ func (w *ImageWalker) Walk(ctx context.Context, req string) (int, error) {
return -1, err
}

matchCount := len(images)
// to handle the `rmi -f` case where returned images are different but
// have the same short prefix.
uniqueImages := make(map[digest.Digest]bool)
for _, image := range images {
nameMatchIndex := -1
for i, image := range images {
uniqueImages[image.Target.Digest] = true
}

// Allow to nerdctl rmi <short digest ids of another images> to remove images.
if len(uniqueImages) > 1 {
imageIDPrefix := fmt.Sprintf("sha256:%s", req)
for i := len(images) - 1; i >= 0; i-- {
if strings.HasPrefix(images[i].Target.Digest.String(), imageIDPrefix) {
delete(uniqueImages, images[i].Target.Digest)
images = append(images[:i], images[i+1:]...)
}
// to get target image index for `nerdctl rmi <short digest ids of another images>`.
if (parsedReferenceStr != "" && image.Name == parsedReferenceStr) || image.Name == req {
nameMatchIndex = i
}
}

matchCount := len(images)

for i, img := range images {
f := Found{
Image: img,
Req: req,
MatchIndex: i,
MatchCount: matchCount,
UniqueImages: len(uniqueImages),
Image: img,
Req: req,
MatchIndex: i,
MatchCount: matchCount,
UniqueImages: len(uniqueImages),
NameMatchIndex: nameMatchIndex,
}
if e := w.OnFound(ctx, f); e != nil {
return -1, e
Expand Down

0 comments on commit 5b7ccd7

Please sign in to comment.