Image inspect rework

cmd/nerdctl/image_inspect_test.go

@@ -17,10 +17,15 @@
 package main
 
 import (
+	"encoding/json"
+	"runtime"
+	"strings"
 	"testing"
 
-	"github.com/containerd/nerdctl/v2/pkg/testutil"
 	"gotest.tools/v3/assert"
+
+	"github.com/containerd/nerdctl/v2/pkg/inspecttypes/dockercompat"
+	"github.com/containerd/nerdctl/v2/pkg/testutil"
 )
 
 func TestImageInspectContainsSomeStuff(t *testing.T) {
@@ -39,9 +44,134 @@ func TestImageInspectWithFormat(t *testing.T) {
 	base := testutil.NewBase(t)
 
 	base.Cmd("pull", testutil.CommonImage).AssertOK()
+
 	// test RawFormat support
 	base.Cmd("image", "inspect", testutil.CommonImage, "--format", "{{.Id}}").AssertOK()
 
 	// test typedFormat support
 	base.Cmd("image", "inspect", testutil.CommonImage, "--format", "{{.ID}}").AssertOK()
 }
+
+func inspectImageHelper(base *testutil.Base, identifier ...string) []dockercompat.Image {
+	args := append([]string{"image", "inspect"}, identifier...)
+	cmdResult := base.Cmd(args...).Run()
+	assert.Equal(base.T, cmdResult.ExitCode, 0)
+	var dc []dockercompat.Image
+	if err := json.Unmarshal([]byte(cmdResult.Stdout()), &dc); err != nil {
+		base.T.Fatal(err)
+	}
+	return dc
+}
+
+func TestImageInspectDifferentValidReferencesForTheSameImage(t *testing.T) {
+	testutil.DockerIncompatible(t)
+
+	if runtime.GOOS == "windows" {
+		t.Skip("Windows is not supported for this test right now")
+	}
+
+	base := testutil.NewBase(t)
+
+	// Overall, we need a clean slate before doing these lookups.
+	// More specifically, because we trigger https://github.com/containerd/nerdctl/issues/3016
+	// we cannot do selective rmi, so, just nuke everything
+	ids := base.Cmd("image", "list", "-q").Out()
+	allIds := strings.Split(ids, "\n")
+	for _, id := range allIds {
+		id = strings.TrimSpace(id)
+		if id != "" {
+			base.Cmd("rmi", "-f", id).Run()
+		}
+	}
+
+	base.Cmd("pull", "alpine", "--platform", "linux/amd64").AssertOK()
+	base.Cmd("pull", "busybox", "--platform", "linux/amd64").AssertOK()
+	base.Cmd("pull", "busybox:stable", "--platform", "linux/amd64").AssertOK()
+	base.Cmd("pull", "registry-1.docker.io/library/busybox", "--platform", "linux/amd64").AssertOK()
+	base.Cmd("pull", "registry-1.docker.io/library/busybox:stable", "--platform", "linux/amd64").AssertOK()
+
+	tags := []string{
+		"",
+		":latest",
+		":stable",
+	}
+	names := []string{
+		"busybox",
+		"library/busybox",
+		"docker.io/library/busybox",
+		"registry-1.docker.io/library/busybox",
+	}
+
+	// Build reference values for comparison
+	reference := inspectImageHelper(base, "busybox")
+	assert.Equal(base.T, 1, len(reference))
+	// Extract image sha
+	sha := strings.TrimPrefix(reference[0].RepoDigests[0], "busybox@sha256:")
+
+	differentReference := inspectImageHelper(base, "alpine")
+	assert.Equal(base.T, 1, len(differentReference))
+
+	// Testing all name and tags variants
+	for _, name := range names {
+		for _, tag := range tags {
+			t.Logf("Testing %s", name+tag)
+			result := inspectImageHelper(base, name+tag)
+			assert.Equal(base.T, 1, len(result))
+			assert.Equal(base.T, reference[0].ID, result[0].ID)
+		}
+	}
+
+	// Testing all name and tags variants, with a digest
+	for _, name := range names {
+		for _, tag := range tags {
+			t.Logf("Testing %s", name+tag+"@"+sha)
+			result := inspectImageHelper(base, name+tag+"@sha256:"+sha)
+			assert.Equal(base.T, 1, len(result))
+			assert.Equal(base.T, reference[0].ID, result[0].ID)
+		}
+	}
+
+	// Testing repo digest and short digest with or without prefix
+	for _, id := range []string{"sha256:" + sha, sha, sha[0:8], "sha256:" + sha[0:8]} {
+		t.Logf("Testing %s", id)
+		result := inspectImageHelper(base, id)
+		assert.Equal(base.T, 1, len(result))
+		assert.Equal(base.T, reference[0].ID, result[0].ID)
+	}
+
+	// Demonstrate image name precedence over digest lookup
+	// Using the shortened sha should no longer get busybox, but rather the newly tagged Alpine
+	t.Logf("Testing (alpine tagged) %s", sha[0:8])
+	// Tag a different image with the short id
+	base.Cmd("tag", "alpine", sha[0:8]).AssertOK()
+	result := inspectImageHelper(base, sha[0:8])
+	assert.Equal(base.T, 1, len(result))
+	assert.Equal(base.T, differentReference[0].ID, result[0].ID)
+
+	// Prove that wrong references with an existing digest do not get retrieved when asking by digest
+	for _, id := range []string{"doesnotexist", "doesnotexist:either", "busybox:bogustag"} {
+		t.Logf("Testing %s", id+"@"+sha)
+		args := append([]string{"image", "inspect"}, id+"@"+sha)
+		cmdResult := base.Cmd(args...).Run()
+		assert.Equal(base.T, cmdResult.ExitCode, 0)
+		assert.Equal(base.T, cmdResult.Stdout(), "")
+	}
+
+	// Prove that invalid reference return no result without crashing
+	for _, id := range []string{"∞∞∞∞∞∞∞∞∞∞", "busybox:∞∞∞∞∞∞∞∞∞∞"} {
+		t.Logf("Testing %s", id)
+		args := append([]string{"image", "inspect"}, id)
+		cmdResult := base.Cmd(args...).Run()
+		assert.Equal(base.T, cmdResult.ExitCode, 0)
+		assert.Equal(base.T, cmdResult.Stdout(), "")
+	}
+
+	// Retrieving multiple entries at once
+	t.Logf("Testing %s", "busybox busybox busybox:stable")
+	result = inspectImageHelper(base, "busybox", "busybox", "busybox:stable")
+	assert.Equal(base.T, 3, len(result))
+	assert.Equal(base.T, reference[0].ID, result[0].ID)
+	assert.Equal(base.T, reference[0].ID, result[1].ID)
+	assert.Equal(base.T, reference[0].ID, result[2].ID)
+
+}

go.mod

@@ -76,7 +76,7 @@ require (
 	github.com/containerd/ttrpc v1.2.3 // indirect
 	github.com/containerd/typeurl v1.0.3-0.20220422153119-7f6e6d160d67 // indirect
 	github.com/containers/ocicrypt v1.1.10 // indirect
-	github.com/distribution/reference v0.6.0 // indirect
+	github.com/distribution/reference v0.6.0
 	github.com/djherbis/times v1.6.0 // indirect
 	github.com/docker/docker-credential-helpers v0.7.0 // indirect
 	github.com/docker/go-events v0.0.0-20190806004212-e31b211e4f1c // indirect

pkg/cmd/image/inspect.go

@@ -19,58 +19,182 @@ package image
 import (
 	"context"
 	"fmt"
+	"regexp"
+	"strings"
 	"time"
 
 	"github.com/containerd/containerd"
+	"github.com/containerd/containerd/images"
 	"github.com/containerd/log"
 	"github.com/containerd/nerdctl/v2/pkg/api/types"
 	"github.com/containerd/nerdctl/v2/pkg/formatter"
-	"github.com/containerd/nerdctl/v2/pkg/idutil/imagewalker"
 	"github.com/containerd/nerdctl/v2/pkg/imageinspector"
 	"github.com/containerd/nerdctl/v2/pkg/inspecttypes/dockercompat"
+	"github.com/containerd/nerdctl/v2/pkg/referenceutil"
+	"github.com/distribution/reference"
 )
 
+func inspectIdentifier(ctx context.Context, client *containerd.Client, identifier string) ([]images.Image, string, string, error) {
+	// Figure out what we have here - digest, tag, name
+	parsedIdentifier, err := referenceutil.ParseAnyReference(identifier)
+	if err != nil {
+		return nil, "", "", fmt.Errorf("invalid identifier %s: %w", identifier, err)
+	}
+	digest := ""
+	if identifierDigest, hasDigest := parsedIdentifier.(reference.Digested); hasDigest {
+		digest = identifierDigest.Digest().String()
+	}
+	name := ""
+	if identifierName, hasName := parsedIdentifier.(reference.Named); hasName {
+		name = identifierName.Name()
+	}
+	tag := "latest"
+	if identifierTag, hasTag := parsedIdentifier.(reference.Tagged); hasTag && identifierTag.Tag() != "" {
+		tag = identifierTag.Tag()
+	}
+
+	// Initialize filters
+	var filters []string
+	// This will hold the final image list, if any
+	var imageList []images.Image
+
+	// No digest in the request? Then assume it is a name
+	if digest == "" {
+		filters = []string{fmt.Sprintf("name==%s:%s", name, tag)}
+		// Query it
+		imageList, err = client.ImageService().List(ctx, filters...)
+		if err != nil {
+			return nil, "", "", fmt.Errorf("containerd image service failed: %w", err)
+		}
+		// Nothing? Then it could be a short id (aka truncated digest) - we are going to use this
+		if len(imageList) == 0 {
+			digest = fmt.Sprintf("sha256:%s.*", regexp.QuoteMeta(strings.TrimPrefix(identifier, "sha256:")))
+			name = ""
+			tag = ""
+		} else {
+			// Otherwise, we found one by name. Get the digest from it.
+			digest = imageList[0].Target.Digest.String()
+		}
+	}
+
+	// At this point, we DO have a digest (or short id), so, that is what we are retrieving
+	filters = []string{fmt.Sprintf("target.digest~=^%s$", digest)}
+	imageList, err = client.ImageService().List(ctx, filters...)
+	if err != nil {
+		return nil, "", "", fmt.Errorf("containerd image service failed: %w", err)
+	}
+
+	// TODO: docker does allow retrieving images by Id, so implement as a last ditch effort (probably look-up the store)
+
+	// Return the list we found, along with normalized name and tag
+	return imageList, name, tag, nil
+}
+
 // Inspect prints detailed information of each image in `images`.
-func Inspect(ctx context.Context, client *containerd.Client, images []string, options types.ImageInspectOptions) error {
-	f := &imageInspector{
-		mode: options.Mode,
+func Inspect(ctx context.Context, client *containerd.Client, identifiers []string, options types.ImageInspectOptions) error {
+	// Verify we have a valid mode
+	// TODO: move this out of here, to Cobra command line arg validation
+	if options.Mode != "native" && options.Mode != "dockercompat" {
+		return fmt.Errorf("unknown mode %q", options.Mode)
 	}
-	walker := &imagewalker.ImageWalker{
-		Client: client,
-		OnFound: func(ctx context.Context, found imagewalker.Found) error {
-			ctx, cancel := context.WithTimeout(ctx, 5*time.Second)
-			defer cancel()
 
-			n, err := imageinspector.Inspect(ctx, client, found.Image, options.GOptions.Snapshotter)
+	// Set a timeout
+	ctx, cancel := context.WithTimeout(ctx, 5*time.Second)
+	defer cancel()
+
+	// Will hold the final answers
+	var entries []interface{}
+
+	// We have to query per provided identifier, as we need to post-process results for the case name + digest
+	for _, identifier := range identifiers {
+		candidateImageList, requestedName, requestedTag, err := inspectIdentifier(ctx, client, identifier)
+		if err != nil {
+			log.G(ctx).WithError(err).WithField("identifier", identifier).Error("failure calling inspect")
+			continue
+		}
+
+		var validatedImage *dockercompat.Image
+		var repoTags []string
+		var repoDigests []string
+
+		// Go through the candidates
+		for _, candidateImage := range candidateImageList {
+			// Inspect the image
+			candidateNativeImage, err := imageinspector.Inspect(ctx, client, candidateImage, options.GOptions.Snapshotter)
 			if err != nil {
-				return err
+				log.G(ctx).WithError(err).WithField("name", candidateImage.Name).Error("failure inspecting image")
+				continue
 			}
-			switch f.mode {
-			case "native":
-				f.entries = append(f.entries, n)
-			case "dockercompat":
-				d, err := dockercompat.ImageFromNative(n)
+
+			// If native, we just add everything in there and that's it
+			if options.Mode == "native" {
+				entries = append(entries, candidateNativeImage)
+				continue
+			}
+
+			// If dockercompat: does the candidate have a name? Get it if so
+			candidateRef, err := referenceutil.ParseAnyReference(candidateNativeImage.Image.Name)
+			if err != nil {
+				log.G(ctx).WithError(err).WithField("name", candidateNativeImage.Image.Name).Error("the found image has an unparsable name")
+				continue
+			}
+			parsedCandidateNameTag, candidateHasAName := candidateRef.(reference.NamedTagged)
+
+			// If we were ALSO asked for a specific name on top of the digest, we need to make sure we keep only the image with that name
+			if requestedName != "" {
+				// If the candidate did not have a name, then we should ignore this one and continue
+				if !candidateHasAName {
+					continue
+				}
+
+				// Otherwise, the candidate has a name. If it is the one we want, store it and continue, otherwise, fall through
+				candidateTag := parsedCandidateNameTag.Tag()
+				if candidateTag == "" {
+					candidateTag = "latest"
+				}
+				if parsedCandidateNameTag.Name() == requestedName && candidateTag == requestedTag {
+					validatedImage, err = dockercompat.ImageFromNative(candidateNativeImage)
+					if err != nil {
+						log.G(ctx).WithError(err).WithField("name", candidateNativeImage.Image.Name).Error("could not get a docker compat version of the native image")
+					}
+					continue
+				}
+			} else if validatedImage == nil {
+				// Alternatively, we got a request by digest only, so, if we do not know about it already, store it and continue
+				validatedImage, err = dockercompat.ImageFromNative(candidateNativeImage)
 				if err != nil {
-					return err
+					log.G(ctx).WithError(err).WithField("name", candidateNativeImage.Image.Name).Error("could not get a docker compat version of the native image")
 				}
-				f.entries = append(f.entries, d)
-			default:
-				return fmt.Errorf("unknown mode %q", f.mode)
+				continue
+			}
+
+			// Fallthrough cases:
+			// - we got a request by digest, but we already had the image stored
+			// - we got a request by name, and the name of the candidate did not match the requested name
+			// Now, check if the candidate has a name - if it does, populate repoTags and repoDigests
+			if candidateHasAName {
+				repoTags = append(repoTags, fmt.Sprintf("%s:%s", reference.FamiliarName(parsedCandidateNameTag), parsedCandidateNameTag.Tag()))
+				repoDigests = append(repoDigests, fmt.Sprintf("%s@%s", reference.FamiliarName(parsedCandidateNameTag), candidateImage.Target.Digest.String()))
 			}
-			return nil
-		},
+		}
+
+		// Done iterating through candidates. Did we find anything that matches?
+		if validatedImage != nil {
+			// Then slap in the repoTags and repoDigests we found from the other candidates
+			validatedImage.RepoTags = append(validatedImage.RepoTags, repoTags...)
+			validatedImage.RepoDigests = append(validatedImage.RepoDigests, repoDigests...)
+			// Store our image
+			// foundImages[validatedDigest] = validatedImage
+			entries = append(entries, validatedImage)
+		}
 	}
 
-	err := walker.WalkAll(ctx, images, true)
-	if len(f.entries) > 0 {
-		if formatErr := formatter.FormatSlice(options.Format, options.Stdout, f.entries); formatErr != nil {
+	// Display
+	if len(entries) > 0 {
+		if formatErr := formatter.FormatSlice(options.Format, options.Stdout, entries); formatErr != nil {
 			log.G(ctx).Error(formatErr)
 		}
 	}
-	return err
-}
 
-type imageInspector struct {
-	mode    string
-	entries []interface{}
+	return nil
 }