diff --git a/storage/storage_reference.go b/storage/storage_reference.go index 53271694..a5ac362f 100644 --- a/storage/storage_reference.go +++ b/storage/storage_reference.go @@ -123,6 +123,8 @@ func (s storageReference) DockerReference() reference.Named { return canonical } } + // FIXME: This should never happen, ParseStoreReference sets name = reference.TagNameOnly(name) if s.digest == "" + // FIXME: This also violates the DockerReference contract. return s.name } diff --git a/storage/storage_reference_test.go b/storage/storage_reference_test.go index a4b133e5..1b292d70 100644 --- a/storage/storage_reference_test.go +++ b/storage/storage_reference_test.go @@ -40,12 +40,32 @@ var validReferenceTestCases = []struct { }, { "busybox@" + sha256digestHex, "docker.io/library/busybox:latest", "docker.io/library/busybox:latest@" + sha256digestHex, + // FIXME: This should start with …/busybox:latest []string{"docker.io/library/busybox", "docker.io/library", "docker.io"}, }, { "busybox@sha256:" + sha256digestHex, "docker.io/library/busybox@sha256:" + sha256digestHex, "docker.io/library/busybox@sha256:" + sha256digestHex, []string{"docker.io/library/busybox", "docker.io/library", "docker.io"}, }, + { + "busybox:notlatest@" + sha256digestHex, "docker.io/library/busybox:notlatest", "docker.io/library/busybox:notlatest@" + sha256digestHex, + // FIXME: This should start with …/busybox:notlatest + []string{"docker.io/library/busybox", "docker.io/library", "docker.io"}, + }, + { + "busybox:notlatest@sha256:" + sha256digestHex, "docker.io/library/busybox:notlatest@sha256:" + sha256digestHex, "docker.io/library/busybox:notlatest@sha256:" + sha256digestHex, + []string{"docker.io/library/busybox", "docker.io/library", "docker.io"}, + }, + { + "busybox@" + sha256Digest2 + "@" + sha256digestHex, "docker.io/library/busybox@" + sha256Digest2, "docker.io/library/busybox@" + sha256Digest2 + "@" + sha256digestHex, + // FIXME: This should start with …/busybox@digest + []string{"docker.io/library/busybox", "docker.io/library", "docker.io"}, + }, + { + "busybox:notlatest@" + sha256Digest2 + "@" + sha256digestHex, "docker.io/library/busybox:notlatest@" + sha256Digest2, "docker.io/library/busybox:notlatest@" + sha256Digest2 + "@" + sha256digestHex, + // FIXME: This should start with …/busybox:tag@digest, then :tag, only then plain repo name …/busybox + []string{"docker.io/library/busybox", "docker.io/library", "docker.io"}, + }, } func TestStorageReferenceDockerReference(t *testing.T) { @@ -104,7 +124,7 @@ func TestStorageReferencePolicyConfigurationNamespaces(t *testing.T) { } expectedNS = append(expectedNS, storeSpec) expectedNS = append(expectedNS, fmt.Sprintf("[%s]", store.GraphRoot())) - assert.Equal(t, expectedNS, ref.PolicyConfigurationNamespaces()) + assert.Equal(t, expectedNS, ref.PolicyConfigurationNamespaces(), c.input) } } diff --git a/storage/storage_transport.go b/storage/storage_transport.go index f6ebcdc4..1177b642 100644 --- a/storage/storage_transport.go +++ b/storage/storage_transport.go @@ -193,7 +193,7 @@ func (s storageTransport) ParseStoreReference(store storage.Store, ref string) ( return nil, errors.Wrapf(err, "error parsing named reference %q", ref) } } - if name == nil && sum == "" && id == "" { + if name == nil && sum == "" && id == "" { // Coverage: This could happen only on empty input, which is refused at the very top of the method. return nil, errors.Errorf("error parsing reference") } diff --git a/storage/storage_transport_test.go b/storage/storage_transport_test.go index 2c5d782e..857f8a82 100644 --- a/storage/storage_transport_test.go +++ b/storage/storage_transport_test.go @@ -13,6 +13,7 @@ import ( const ( sha256digestHex = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + sha256Digest2 = "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" ) func TestTransportName(t *testing.T) { @@ -20,6 +21,8 @@ func TestTransportName(t *testing.T) { } func TestTransportParseStoreReference(t *testing.T) { + const digest3 = "sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + store := newStore(t) Transport.SetStore(nil) @@ -30,22 +33,40 @@ func TestTransportParseStoreReference(t *testing.T) { {"[unterminated", "", ""}, // Unterminated store specifier {"[garbage]busybox", "docker.io/library/busybox:latest", ""}, // Store specifier is overridden by the store we pass to ParseStoreReference - {"UPPERCASEISINVALID", "", ""}, // Invalid single-component name - {"sha256:" + sha256digestHex, "docker.io/library/sha256:" + sha256digestHex, ""}, // Valid single-component name; the hex part is not an ID unless it has a "@" prefix, so it looks like a tag - {sha256digestHex, "", ""}, // Invalid single-component ID; not an ID without a "@" prefix, so it's parsed as a name, but names aren't allowed to look like IDs - {"@" + sha256digestHex, "", sha256digestHex}, // Valid single-component ID + {"UPPERCASEISINVALID", "", ""}, // Invalid single-component name + {"sha256:" + sha256digestHex, "docker.io/library/sha256:" + sha256digestHex, ""}, // Valid single-component name; the hex part is not an ID unless it has a "@" prefix, so it looks like a tag + // FIXME: This test is now incorrect, this should not fail _if the image ID matches_ + {sha256digestHex, "", ""}, // Invalid single-component ID; not an ID without a "@" prefix, so it's parsed as a name, but names aren't allowed to look like IDs + {"@" + sha256digestHex, "", sha256digestHex}, // Valid single-component ID + // FIXME: This should fail it seems, everything else uses ref.digest only if ref.name != nil + // {"@sha256:" + sha256digestHex, "", ""}, // Valid single-component digest + // "aaaa", either a valid image ID prefix, or a short form of docker.io/library/aaaa, untested {"sha256:ab", "docker.io/library/sha256:ab", ""}, // Valid single-component name, explicit tag {"busybox", "docker.io/library/busybox:latest", ""}, // Valid single-component name, implicit tag {"busybox:notlatest", "docker.io/library/busybox:notlatest", ""}, // Valid single-component name, explicit tag {"docker.io/library/busybox:notlatest", "docker.io/library/busybox:notlatest", ""}, // Valid single-component name, everything explicit - {"UPPERCASEISINVALID@" + sha256digestHex, "", ""}, // Invalid name in name@ID - {"busybox@ab", "", ""}, // Invalid ID in name@ID - {"busybox@", "", ""}, // Empty ID in name@ID - {"busybox@sha256:" + sha256digestHex, "docker.io/library/busybox@sha256:" + sha256digestHex, ""}, // Valid two-component name, with a digest and no tag - {"busybox@" + sha256digestHex, "docker.io/library/busybox:latest", sha256digestHex}, // Valid two-component name, implicit tag - {"busybox:notlatest@" + sha256digestHex, "docker.io/library/busybox:notlatest", sha256digestHex}, // Valid two-component name, explicit tag - {"docker.io/library/busybox:notlatest@" + sha256digestHex, "docker.io/library/busybox:notlatest", sha256digestHex}, // Valid two-component name, everything explicit + {"UPPERCASEISINVALID@" + sha256digestHex, "", ""}, // Invalid name in name@digestOrID + {"busybox@ab", "", ""}, // Invalid ID in name@digestOrID + {"busybox@", "", ""}, // Empty ID in name@digestOrID + {"busybox@sha256:ab", "", ""}, // Invalid digest in name@digestOrID + {"busybox@sha256:" + sha256digestHex, "docker.io/library/busybox@sha256:" + sha256digestHex, ""}, // Valid name@digest, no tag + {"busybox@" + sha256digestHex, "docker.io/library/busybox:latest", sha256digestHex}, // Valid name@ID, implicit tag + // "busybox@aaaa", a valid image ID prefix, untested + {"busybox:notlatest@" + sha256digestHex, "docker.io/library/busybox:notlatest", sha256digestHex}, // Valid name@ID, explicit tag + {"docker.io/library/busybox:notlatest@" + sha256digestHex, "docker.io/library/busybox:notlatest", sha256digestHex}, // Valid name@ID, everything explicit + {"docker.io/library/busybox:notlatest@" + sha256Digest2, "docker.io/library/busybox:notlatest@" + sha256Digest2, ""}, // Valid name:tag@digest, everything explicit + + {"busybox@sha256:" + sha256digestHex + "@ab", "", ""}, // Invalid ID in name@digest@ID + {"busybox@ab@" + sha256digestHex, "", ""}, // Invalid digest in name@digest@ID + {"busybox@@" + sha256digestHex, "", ""}, // Invalid digest in name@digest@ID + {"busybox@" + sha256Digest2 + "@" + sha256digestHex, "docker.io/library/busybox@" + sha256Digest2, sha256digestHex}, // name@digest@ID + {"docker.io/library/busybox@" + sha256Digest2 + "@" + sha256digestHex, "docker.io/library/busybox@" + sha256Digest2, sha256digestHex}, // name@digest@ID, everything explicit + {"docker.io/library/busybox:notlatest@sha256:" + sha256digestHex + "@" + sha256digestHex, "docker.io/library/busybox:notlatest@sha256:" + sha256digestHex, sha256digestHex}, // name:tag@digest@ID, everything explicit + // FIXME: Is this supposed to work? the validation of idOrDigest seems to make this impossible. + // "busybox@sha256:"+sha256digestHex+"@aaaa", a valid image ID prefix, untested + // FIXME FIXME: two digests + {"busybox:notlatest@" + sha256Digest2 + "@" + digest3 + "@" + sha256digestHex, "docker.io/library/busybox:notlatest@" + digest3, sha256digestHex}, // name@digest@ID, with name containing a digest } { storageRef, err := Transport.ParseStoreReference(store, c.input) if c.expectedRef == "" && c.expectedID == "" { @@ -74,13 +95,16 @@ func TestTransportParseReference(t *testing.T) { root := store.GraphRoot() for _, c := range []struct{ prefix, expectedDriver, expectedRoot, expectedRunRoot string }{ - {"", driver, root, ""}, // Implicit store location prefix - {"[unterminated", "", "", ""}, // Unterminated store specifier - {"[]", "", "", ""}, // Empty store specifier - {"[relative/path]", "", "", ""}, // Non-absolute graph root path - {"[" + driver + "@relative/path]", "", "", ""}, // Non-absolute graph root path - {"[thisisunknown@" + root + "suffix2]", "", "", ""}, // Unknown graph driver - {"[" + root + "suffix1]", "", root + "suffix1", ""}, // A valid root path, but no run dir + {"", driver, root, ""}, // Implicit store location prefix + {"[unterminated", "", "", ""}, // Unterminated store specifier + {"[]", "", "", ""}, // Empty store specifier + {"[relative/path]", "", "", ""}, // Non-absolute graph root path + {"[" + driver + "@relative/path]", "", "", ""}, // Non-absolute graph root path + {"[@" + root + "suffix2]", "", "", ""}, // Empty graph driver + {"[" + driver + "@]", "", "", ""}, // Empty root path + {"[thisisunknown@" + root + "suffix2]", "", "", ""}, // Unknown graph driver + {"[" + root + "suffix1]", "", "", ""}, // A valid root path, but no run dir + {"[" + driver + "@" + root + "suffix3+relative/path]", "", "", ""}, // Non-absolute run dir {"[" + driver + "@" + root + "suffix3+" + root + "suffix4]", driver, root + "suffix3",