Merge pull request #2117 from mtrmac/no-imageParts

RFC: Mostly replace imageParts

1 files changed, 66 insertions, 59 deletions

diff --git a/libpod/image/parts.go b/libpod/image/parts.go
index b2a69f26c..dfdf0b08a 100644
--- a/libpod/image/parts.go
+++ b/libpod/image/parts.go
@@ -1,23 +1,20 @@
 package image
 
 import (
-	"fmt"
 	"strings"
 
 	"github.com/containers/image/docker/reference"
+	"github.com/pkg/errors"
 )
 
-// Parts describes the parts of an image's name
-type Parts struct {
-	transport   string
-	Registry    string
-	name        string
-	Tag         string
-	isTagged    bool
-	hasRegistry bool
+// imageParts describes the parts of an image's name
+type imageParts struct {
+	unnormalizedRef reference.Named // WARNING: Did not go through docker.io[/library] normalization
+	hasRegistry     bool
 }
 
-// Registries must contain a ":" or a "." or be localhost
+// Registries must contain a ":" or a "." or be localhost; this helper exists for users of reference.Parse.
+// For inputs that should use the docker.io[/library] normalization, use reference.ParseNormalizedNamed instead.
 func isRegistry(name string) bool {
 	return strings.ContainsAny(name, ".:") || name == "localhost"
 }
@@ -30,68 +27,78 @@ func GetImageBaseName(input string) (string, error) {
 	if err != nil {
 		return "", err
 	}
-	splitImageName := strings.Split(decomposedImage.name, "/")
+	splitImageName := strings.Split(decomposedImage.unnormalizedRef.Name(), "/")
 	return splitImageName[len(splitImageName)-1], nil
 }
 
-// DecomposeString decomposes a string name into imageParts description. This
-// is a wrapper for decompose
-func DecomposeString(input string) (Parts, error) {
-	return decompose(input)
-}
-
 // decompose breaks an input name into an imageParts description
-func decompose(input string) (Parts, error) {
-	var (
-		parts       Parts
-		hasRegistry bool
-		tag         string
-	)
+func decompose(input string) (imageParts, error) {
 	imgRef, err := reference.Parse(input)
 	if err != nil {
-		return parts, err
+		return imageParts{}, err
 	}
-	ntag, isTagged := imgRef.(reference.NamedTagged)
-	if !isTagged {
-		tag = "latest"
-		if _, hasDigest := imgRef.(reference.Digested); hasDigest {
-			tag = "none"
-		}
-	} else {
-		tag = ntag.Tag()
+	unnormalizedNamed := imgRef.(reference.Named)
+	// ip.unnormalizedRef, because it uses reference.Parse and not reference.ParseNormalizedNamed,
+	// does not use the standard heuristics for domains vs. namespaces/repos, so we need to check
+	// explicitly.
+	hasRegistry := isRegistry(reference.Domain(unnormalizedNamed))
+	return imageParts{
+		unnormalizedRef: unnormalizedNamed,
+		hasRegistry:     hasRegistry,
+	}, nil
+}
+
+// suspiciousRefNameTagValuesForSearch returns a "tag" value used in a previous implementation.
+// This exists only to preserve existing behavior in heuristic code; it’s dubious that that behavior is correct,
+// gespecially for the tag value.
+func (ip *imageParts) suspiciousRefNameTagValuesForSearch() (string, string, string) {
+	registry := reference.Domain(ip.unnormalizedRef)
+	imageName := reference.Path(ip.unnormalizedRef)
+	// ip.unnormalizedRef, because it uses reference.Parse and not reference.ParseNormalizedNamed,
+	// does not use the standard heuristics for domains vs. namespaces/repos.
+	if registry != "" && !isRegistry(registry) {
+		imageName = registry + "/" + imageName
+		registry = ""
 	}
-	registry := reference.Domain(imgRef.(reference.Named))
-	imageName := reference.Path(imgRef.(reference.Named))
-	// Is this a Registry or a repo?
-	if isRegistry(registry) {
-		hasRegistry = true
+
+	var tag string
+	if tagged, isTagged := ip.unnormalizedRef.(reference.NamedTagged); isTagged {
+		tag = tagged.Tag()
+	} else if _, hasDigest := ip.unnormalizedRef.(reference.Digested); hasDigest {
+		tag = "none"
 	} else {
-		if registry != "" {
-			imageName = registry + "/" + imageName
-			registry = ""
-		}
+		tag = "latest"
 	}
-	return Parts{
-		Registry:    registry,
-		hasRegistry: hasRegistry,
-		name:        imageName,
-		Tag:         tag,
-		isTagged:    isTagged,
-		transport:   DefaultTransport,
-	}, nil
+	return registry, imageName, tag
 }
 
-// assemble concatenates an image's parts into a string
-func (ip *Parts) assemble() string {
-	spec := fmt.Sprintf("%s:%s", ip.name, ip.Tag)
-
-	if ip.Registry != "" {
-		spec = fmt.Sprintf("%s/%s", ip.Registry, spec)
+// referenceWithRegistry returns a (normalized) reference.Named composed of ip (with !ip.hasRegistry)
+// qualified with registry.
+func (ip *imageParts) referenceWithRegistry(registry string) (reference.Named, error) {
+	if ip.hasRegistry {
+		return nil, errors.Errorf("internal error: referenceWithRegistry called on imageParts with a registry (%#v)", *ip)
 	}
-	return spec
+	// We could build a reference.WithName+WithTag/WithDigest here, but we need to round-trip via a string
+	// and a ParseNormalizedNamed anyway to get the right normalization of docker.io/library, so
+	// just use a string directly.
+	qualified := registry + "/" + ip.unnormalizedRef.String()
+	ref, err := reference.ParseNormalizedNamed(qualified)
+	if err != nil {
+		return nil, errors.Wrapf(err, "error normalizing registry+unqualified reference %#v", qualified)
+	}
+	return ref, nil
 }
 
-// assemble concatenates an image's parts with transport into a string
-func (ip *Parts) assembleWithTransport() string {
-	return fmt.Sprintf("%s%s", ip.transport, ip.assemble())
+// normalizedReference returns a (normalized) reference for ip (with ip.hasRegistry)
+func (ip *imageParts) normalizedReference() (reference.Named, error) {
+	if !ip.hasRegistry {
+		return nil, errors.Errorf("internal error: normalizedReference called on imageParts without a registry (%#v)", *ip)
+	}
+	// We need to round-trip via a string to get the right normalization of docker.io/library
+	s := ip.unnormalizedRef.String()
+	ref, err := reference.ParseNormalizedNamed(s)
+	if err != nil { // Should never happen
+		return nil, errors.Wrapf(err, "error normalizing qualified reference %#v", s)
+	}
+	return ref, nil
 }