[Enhancement] Follow Docker's image nameing convention in `k3d image import` (#653, @cimnine)

cmd/image/imageImport.go

@@ -41,9 +41,21 @@ func NewCmdImageImport() *cobra.Command {
 
 	// create new command
 	cmd := &cobra.Command{
-		Use:     "import [IMAGE | ARCHIVE [IMAGE | ARCHIVE...]]",
-		Short:   "Import image(s) from docker into k3d cluster(s).",
-		Long:    `Import image(s) from docker into k3d cluster(s).`,
+		Use:   "import [IMAGE | ARCHIVE [IMAGE | ARCHIVE...]]",
+		Short: "Import image(s) from docker into k3d cluster(s).",
+		Long: `Import image(s) from docker into k3d cluster(s).
+
+If an IMAGE starts with the prefix 'docker.io/', then this prefix is stripped internally.
+That is, 'docker.io/rancher/k3d-tools:latest' is treated as 'rancher/k3d-tools:latest'.
+
+If an IMAGE starts with the prefix 'library/' (or 'docker.io/library/'), then this prefix is stripped internally.
+That is, 'library/busybox:latest' (or 'docker.io/library/busybox:latest') are treated as 'busybox:latest'.
+
+If an IMAGE does not have a version tag, then ':latest' is assumed.
+That is, 'rancher/k3d-tools' is treated as 'rancher/k3d-tools:latest'.
+
+A file ARCHIVE always takes precedence.
+So if a file './rancher/k3d-tools' exists, k3d will try to import it instead of the IMAGE of the same name.`,
 		Aliases: []string{"images"},
 		Args:    cobra.MinimumNArgs(1),
 		Run: func(cmd *cobra.Command, args []string) {

docs/usage/commands/k3d_image_import.md

@@ -6,6 +6,15 @@ Import image(s) from docker into k3d cluster(s).
 
 Import image(s) from docker into k3d cluster(s).
 
+If an IMAGE starts with the prefix 'docker.io/', then this prefix is stripped internally.
+That is, 'docker.io/rancher/k3d-tools:latest' is treated as 'rancher/k3d-tools:latest'.
+
+If an IMAGE does not have a version tag, then ':latest' is assumed.
+That is, 'rancher/k3d-tools' is treated as 'rancher/k3d-tools:latest'.
+
+A file ARCHIVE always takes precedence.
+So if a file './rancher/k3d-tools' exists, k3d will try to import it instead of the IMAGE of the same name.
+
 ```
 k3d image import [IMAGE | ARCHIVE [IMAGE | ARCHIVE...]] [flags]
 ```

pkg/tools/tools.go

@@ -19,6 +19,7 @@ LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
 OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 THE SOFTWARE.
 */
+
 package tools
 
 import (
@@ -40,40 +41,11 @@ import (
 // ImageImportIntoClusterMulti starts up a k3d tools container for the selected cluster and uses it to export
 // images from the runtime to import them into the nodes of the selected cluster
 func ImageImportIntoClusterMulti(ctx context.Context, runtime runtimes.Runtime, images []string, cluster *k3d.Cluster, loadImageOpts k3d.ImageImportOpts) error {
-
-	var imagesFromRuntime []string
-	var imagesFromTar []string
-
-	runtimeImages, err := runtime.GetImages(ctx)
+	imagesFromRuntime, imagesFromTar, err := findImages(ctx, runtime, images)
 	if err != nil {
-		log.Errorln("Failed to fetch list of existing images from runtime")
 		return err
 	}
 
-	for _, image := range images {
-		found := false
-		// Check if the current element is a file
-		if _, err := os.Stat(image); os.IsNotExist(err) {
-			// not a file? Check if such an image is present in the container runtime
-			for _, runtimeImage := range runtimeImages {
-				if image == runtimeImage {
-					found = true
-					imagesFromRuntime = append(imagesFromRuntime, image)
-					log.Debugf("Selected image '%s' found in runtime", image)
-					break
-				}
-			}
-		} else {
-			// file exists
-			found = true
-			imagesFromTar = append(imagesFromTar, image)
-			log.Debugf("Selected image '%s' is a file", image)
-		}
-		if !found {
-			log.Warnf("Image '%s' is not a file and couldn't be found in the container runtime", image)
-		}
-	}
-
 	// no images found to load -> exit early
 	if len(imagesFromRuntime)+len(imagesFromTar) == 0 {
 		return fmt.Errorf("No valid images specified")
@@ -199,6 +171,101 @@ func ImageImportIntoClusterMulti(ctx context.Context, runtime runtimes.Runtime,
 
 }
 
+func findImages(ctx context.Context, runtime runtimes.Runtime, requestedImages []string) (imagesFromRuntime, imagesFromTar []string, err error) {
+	runtimeImages, err := runtime.GetImages(ctx)
+	if err != nil {
+		log.Errorln("Failed to fetch list of existing images from runtime")
+		return nil, nil, err
+	}
+
+	for _, requestedImage := range requestedImages {
+		if isFile(requestedImage) {
+			imagesFromTar = append(imagesFromTar, requestedImage)
+			log.Debugf("Selected image '%s' is a file", requestedImage)
+			break
+		}
+
+		runtimeImage, found := findRuntimeImage(requestedImage, runtimeImages)
+		if found {
+			imagesFromRuntime = append(imagesFromRuntime, runtimeImage)
+			log.Debugf("Selected image '%s' (found as '%s') in runtime", requestedImage, runtimeImage)
+			break
+		}
+
+		log.Warnf("Image '%s' is not a file and couldn't be found in the container runtime", requestedImage)
+	}
+	return imagesFromRuntime, imagesFromTar, err
+}
+
+func findRuntimeImage(requestedImage string, runtimeImages []string) (string, bool) {
+	for _, runtimeImage := range runtimeImages {
+		if imageNamesEqual(requestedImage, runtimeImage) {
+			return runtimeImage, true
+		}
+	}
+
+	// if not found, check for special Docker image naming
+	for _, runtimeImage := range runtimeImages {
+		if dockerSpecialImageNameEqual(requestedImage, runtimeImage) {
+			return runtimeImage, true
+		}
+	}
+	return "", false
+}
+
+func isFile(image string) bool {
+	file, err := os.Stat(image)
+	if err != nil {
+		return false
+	}
+	return !file.IsDir()
+}
+
+func dockerSpecialImageNameEqual(requestedImageName string, runtimeImageName string) bool {
+	if strings.HasPrefix(requestedImageName, "docker.io/") {
+		return dockerSpecialImageNameEqual(strings.TrimPrefix(requestedImageName, "docker.io/"), runtimeImageName)
+	}
+
+	if strings.HasPrefix(requestedImageName, "library/") {
+		return imageNamesEqual(strings.TrimPrefix(requestedImageName, "library/"), runtimeImageName)
+	}
+
+	return false
+}
+
+func imageNamesEqual(requestedImageName string, runtimeImageName string) bool {
+	// first, compare what the user provided
+	if requestedImageName == runtimeImageName {
+		return true
+	}
+
+	// transform to canonical image name, i.e. ensure `:versionName` part on both ends
+	return canonicalImageName(requestedImageName) == runtimeImageName
+}
+
+// canonicalImageName adds `:latest` suffix if `:anyOtherVersionName` is not present.
+func canonicalImageName(image string) string {
+	if !containsVersionPart(image) {
+		image = fmt.Sprintf("%s:latest", image)
+	}
+	return image
+}
+
+func containsVersionPart(imageTag string) bool {
+	if !strings.Contains(imageTag, ":") {
+		return false
+	}
+
+	if !strings.Contains(imageTag, "/") {
+		// happens if someone refers to a library image by just it's imageName (e.g. `postgres` instead of `library/postgres`)
+		return strings.Contains(imageTag, ":")
+	}
+
+	indexOfSlash := strings.Index(imageTag, "/") // can't be -1 because the existence of a '/' is ensured above
+	substringAfterSlash := imageTag[indexOfSlash:]
+	return strings.Contains(substringAfterSlash, ":")
+}
+
 // startToolsNode will start a new k3d tools container and connect it to the network of the chosen cluster
 func startToolsNode(ctx context.Context, runtime runtimes.Runtime, cluster *k3d.Cluster, network string, volumes []string) (*k3d.Node, error) {
 	labels := map[string]string{}

pkg/tools/tools_test.go

@@ -0,0 +1,172 @@
+/*
+Copyright © 2021 The k3d Author(s)
+
+Permission is hereby granted, free of charge, to any person obtaining a copy
+of this software and associated documentation files (the "Software"), to deal
+in the Software without restriction, including without limitation the rights
+to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+copies of the Software, and to permit persons to whom the Software is
+furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in
+all copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+THE SOFTWARE.
+*/
+
+package tools
+
+import (
+	"testing"
+)
+
+func Test_findRuntimeImage(T *testing.T) {
+	imageRegistry := []string{
+		"busybox:latest",
+		"busybox:version",
+		"registry/one:version",
+		"registry/one:latest",
+		"registry/one/two:version",
+		"registry/one/two:latest",
+		"registry/one/two/three/four:version",
+		"registry/one/two/three/four:latest",
+		"registry:1234/one:version",
+		"registry:1234/one:latest",
+		"registry:1234/one/two:version",
+		"registry:1234/one/two:latest",
+		"registry:1234/one/two/three/four:version",
+		"registry:1234/one/two/three/four:latest",
+	}
+
+	tests := map[string]struct {
+		expectedImageName       string
+		expectedFound           bool
+		givenRequestedImageName string
+	}{
+		"registry image tag": {
+			expectedImageName:       "registry/one/two:latest",
+			expectedFound:           true,
+			givenRequestedImageName: "registry/one/two",
+		},
+		"registry image tag with version": {
+			expectedImageName:       "registry/one/two:version",
+			expectedFound:           true,
+			givenRequestedImageName: "registry/one/two:version",
+		},
+		"registry image tag with short path": {
+			expectedImageName:       "registry/one:latest",
+			expectedFound:           true,
+			givenRequestedImageName: "registry/one",
+		},
+		"registry image tag with short path and specific version": {
+			expectedImageName:       "registry/one:version",
+			expectedFound:           true,
+			givenRequestedImageName: "registry/one:version",
+		},
+		"registry image tag with short path and registry port": {
+			expectedImageName:       "registry:1234/one:latest",
+			expectedFound:           true,
+			givenRequestedImageName: "registry:1234/one",
+		},
+		"registry image tag with short path and specific version, registry port": {
+			expectedImageName:       "registry:1234/one:version",
+			expectedFound:           true,
+			givenRequestedImageName: "registry:1234/one:version",
+		},
+		"registry image tag with long path": {
+			expectedImageName:       "registry/one/two/three/four:latest",
+			expectedFound:           true,
+			givenRequestedImageName: "registry/one/two/three/four",
+		},
+		"registry image tag with long path and specific version": {
+			expectedImageName:       "registry/one/two/three/four:version",
+			expectedFound:           true,
+			givenRequestedImageName: "registry/one/two/three/four:version",
+		},
+		"registry image tag with long path and repository port": {
+			expectedImageName:       "registry:1234/one/two/three/four:latest",
+			expectedFound:           true,
+			givenRequestedImageName: "registry:1234/one/two/three/four",
+		},
+		"registry image tag with long path, specific version and repository port": {
+			expectedImageName:       "registry:1234/one/two/three/four:version",
+			expectedFound:           true,
+			givenRequestedImageName: "registry:1234/one/two/three/four:version",
+		},
+		"plain library image tag": {
+			expectedImageName:       "busybox:latest",
+			expectedFound:           true,
+			givenRequestedImageName: "busybox",
+		},
+		"plain library image tag with version": {
+			expectedImageName:       "busybox:latest",
+			expectedFound:           true,
+			givenRequestedImageName: "busybox:latest",
+		},
+		"library image tag": {
+			expectedImageName:       "busybox:latest",
+			expectedFound:           true,
+			givenRequestedImageName: "library/busybox",
+		},
+		"library image tag with latest version": {
+			expectedImageName:       "busybox:latest",
+			expectedFound:           true,
+			givenRequestedImageName: "library/busybox:latest",
+		},
+		"library image tag with specific version": {
+			expectedImageName:       "busybox:latest",
+			expectedFound:           true,
+			givenRequestedImageName: "library/busybox:latest",
+		},
+		"library image tag with repository": {
+			expectedImageName:       "busybox:latest",
+			expectedFound:           true,
+			givenRequestedImageName: "docker.io/library/busybox",
+		},
+		"library image tag with repository and latest version": {
+			expectedImageName:       "busybox:latest",
+			expectedFound:           true,
+			givenRequestedImageName: "docker.io/library/busybox:latest",
+		},
+		"library image tag with repository and specific version": {
+			expectedImageName:       "busybox:version",
+			expectedFound:           true,
+			givenRequestedImageName: "docker.io/library/busybox:version",
+		},
+		"unknown image": {
+			expectedFound:           false,
+			givenRequestedImageName: "unknown",
+		},
+		"unknown with version": {
+			expectedFound:           false,
+			givenRequestedImageName: "unknown:latest",
+		},
+		"unknown with repository": {
+			expectedFound:           false,
+			givenRequestedImageName: "docker.io/unknown",
+		},
+		"unknown with repository and version": {
+			expectedFound:           false,
+			givenRequestedImageName: "docker.io/unknown:tag",
+		},
+	}
+
+	for name, tt := range tests {
+		T.Run(name, func(t *testing.T) {
+			actualImageName, actualFound := findRuntimeImage(tt.givenRequestedImageName, imageRegistry)
+
+			if tt.expectedFound != actualFound {
+				t.Errorf("The image '%s' should not have been found.", tt.givenRequestedImageName)
+			}
+			if tt.expectedImageName != actualImageName {
+				t.Errorf("The image '%s' was found, but '%s' was expected.", actualImageName, tt.expectedImageName)
+			}
+		})
+	}
+}