Eliminate the "DockerArchive means pull all refPairs" special case

Instead, encode it explicitly in pullGoal.pullAllPairs.

Should not change behavior (but does not add unit tests for
all of it).

Signed-off-by: Miloslav Trmač <mitr@redhat.com>

Closes: #1176
Approved by: rhatdan

1 files changed, 24 insertions, 6 deletions

diff --git a/libpod/image/pull_test.go b/libpod/image/pull_test.go
index 910319de6..af8bf14a5 100644
--- a/libpod/image/pull_test.go
+++ b/libpod/image/pull_test.go
@@ -81,24 +81,28 @@ func TestGetPullRefName(t *testing.T) {
 func TestPullGoalNamesFromImageReference(t *testing.T) {
 	type expected struct{ image, dstName string }
 	for _, c := range []struct {
-		srcName  string
-		expected []expected
+		srcName              string
+		expected             []expected
+		expectedPullAllPairs bool
 	}{
 		// == docker-archive:
-		{"docker-archive:/dev/this-does-not-exist", nil}, // Input does not exist.
-		{"docker-archive:/dev/null", nil},                // Input exists but does not contain a manifest.
+		{"docker-archive:/dev/this-does-not-exist", nil, false}, // Input does not exist.
+		{"docker-archive:/dev/null", nil, false},                // Input exists but does not contain a manifest.
 		// FIXME: The implementation has extra code for len(manifest) == 0?! That will fail in getImageDigest anyway.
 		{ // RepoTags is empty
 			"docker-archive:testdata/docker-unnamed.tar.xz",
 			[]expected{{"@ec9293436c2e66da44edb9efb8d41f6b13baf62283ebe846468bc992d76d7951", "@ec9293436c2e66da44edb9efb8d41f6b13baf62283ebe846468bc992d76d7951"}},
+			false,
 		},
 		{ // RepoTags is a [docker.io/library/]name:latest, normalized to the short format.
 			"docker-archive:testdata/docker-name-only.tar.xz",
 			[]expected{{"localhost/pretty-empty:latest", "localhost/pretty-empty:latest"}},
+			true,
 		},
 		{ // RepoTags is a registry/name:latest
 			"docker-archive:testdata/docker-registry-name.tar.xz",
 			[]expected{{"example.com/empty:latest", "example.com/empty:latest"}},
+			true,
 		},
 		{ // RepoTags has multiple items for a single image
 			"docker-archive:testdata/docker-two-names.tar.xz",
@@ -106,33 +110,39 @@ func TestPullGoalNamesFromImageReference(t *testing.T) {
 				{"localhost/pretty-empty:latest", "localhost/pretty-empty:latest"},
 				{"example.com/empty:latest", "example.com/empty:latest"},
 			},
+			true,
 		},
 		{ // FIXME: Two images in a single archive - only the "first" one (whichever it is) is returned
 			// (and docker-archive: then refuses to read anything when the manifest has more than 1 item)
 			"docker-archive:testdata/docker-two-images.tar.xz",
 			[]expected{{"example.com/empty:latest", "example.com/empty:latest"}},
 			// "example.com/empty/but:different" exists but is ignored
+			true,
 		},
 
 		// == oci-archive:
-		{"oci-archive:/dev/this-does-not-exist", nil}, // Input does not exist.
-		{"oci-archive:/dev/null", nil},                // Input exists but does not contain a manifest.
+		{"oci-archive:/dev/this-does-not-exist", nil, false}, // Input does not exist.
+		{"oci-archive:/dev/null", nil, false},                // Input exists but does not contain a manifest.
 		// FIXME: The remaining tests are commented out for now, because oci-archive: does not work unprivileged.
 		// { // No name annotation
 		// 	"oci-archive:testdata/oci-unnamed.tar.gz",
 		// 	[]expected{{"@5c8aca8137ac47e84c69ae93ce650ce967917cc001ba7aad5494073fac75b8b6", "@5c8aca8137ac47e84c69ae93ce650ce967917cc001ba7aad5494073fac75b8b6"}},
+		//  false,
 		// },
 		// { // Name is a name:latest (no normalization is defined).
 		// 	"oci-archive:testdata/oci-name-only.tar.gz",
 		// 	[]expected{{"localhost/pretty-empty:latest", "localhost/pretty-empty:latest"}},
+		//  false,
 		// },
 		// { // Name is a registry/name:latest
 		// 	"oci-archive:testdata/oci-registry-name.tar.gz",
 		// 	[]expected{{"example.com/empty:latest", "example.com/empty:latest"}},
+		//  false,
 		// },
 		// { // Name exists, but is an invalid Docker reference; such names are passed through, and will fail when intepreting dstName.
 		// 	"oci-archive:testdata/oci-non-docker-name.tar.gz",
 		// 	[]expected{{"UPPERCASE-IS-INVALID", "UPPERCASE-IS-INVALID"}},
+		//  false,
 		// },
 		// Maybe test support of two images in a single archive? It should be transparently handled by adding a reference to srcRef.
 
@@ -140,16 +150,19 @@ func TestPullGoalNamesFromImageReference(t *testing.T) {
 		{ // Absolute path
 			"dir:/dev/this-does-not-exist",
 			[]expected{{"localhost/dev/this-does-not-exist", "localhost/dev/this-does-not-exist"}},
+			false,
 		},
 		{ // Relative path, single element.
 			// FIXME? Note the :latest difference in .image.  (In .dstName as well, but it has the same semantics in there.)
 			"dir:this-does-not-exist",
 			[]expected{{"localhost/this-does-not-exist:latest", "localhost/this-does-not-exist:latest"}},
+			false,
 		},
 		{ // Relative path, multiple elements.
 			// FIXME: This does not add localhost/, and dstName is parsed as docker.io/testdata.
 			"dir:testdata/this-does-not-exist",
 			[]expected{{"testdata/this-does-not-exist", "testdata/this-does-not-exist"}},
+			false,
 		},
 
 		// == Others, notably:
@@ -157,10 +170,12 @@ func TestPullGoalNamesFromImageReference(t *testing.T) {
 		{ // Fully-specified input
 			"docker://docker.io/library/busybox:latest",
 			[]expected{{"docker://docker.io/library/busybox:latest", "docker.io/library/busybox:latest"}},
+			false,
 		},
 		{ // Minimal form of the input
 			"docker://busybox",
 			[]expected{{"docker://busybox", "docker.io/library/busybox:latest"}},
+			false,
 		},
 
 		// === tarball: (as an example of what happens when ImageReference.DockerReference is nil).
@@ -168,6 +183,7 @@ func TestPullGoalNamesFromImageReference(t *testing.T) {
 			// (This is NOT an API promise that the results will continue to be this way.)
 			"tarball:/dev/null",
 			[]expected{{"tarball:/dev/null", "tarball:/dev/null"}},
+			false,
 		},
 	} {
 		srcRef, err := alltransports.ParseImageName(c.srcName)
@@ -182,6 +198,7 @@ func TestPullGoalNamesFromImageReference(t *testing.T) {
 			for i, e := range c.expected {
 				assert.Equal(t, pullRefName{image: e.image, srcRef: srcRef, dstName: e.dstName}, res.refNames[i], fmt.Sprintf("%s #%d", c.srcName, i))
 			}
+			assert.Equal(t, c.expectedPullAllPairs, res.pullAllPairs, c.srcName)
 		}
 	}
 }
@@ -296,6 +313,7 @@ func TestPullGoalNamesFromPossiblyUnqualifiedName(t *testing.T) {
 				}
 			}
 			assert.Equal(t, c.expected, strings, c.input)
+			assert.False(t, res.pullAllPairs, c.input)
 		}
 	}
 }