Enforce that a reference has at least one of "named" and "id"

The code assumes that anyway, and this allows us to move an unreachable
check from ParseStoreReference into a testable one in newReference.

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

storage/storage_reference.go

@@ -16,6 +16,7 @@ import (
 // A storageReference holds an arbitrary name and/or an ID, which is a 32-byte
 // value hex-encoded into a 64-character string, and a reference to a Store
 // where an image is, or would be, kept.
+// Either "named" or "id" must be set.
 type storageReference struct {
 	transport            storageTransport
 	named                reference.Named // may include a tag and/or a digest
@@ -23,7 +24,10 @@ type storageReference struct {
 	breakDockerReference bool // Possibly set by newImageDestination.  FIXME: Figure out another way.
 }
 
-func newReference(transport storageTransport, named reference.Named, id string) *storageReference {
+func newReference(transport storageTransport, named reference.Named, id string) (*storageReference, error) {
+	if named == nil && id == "" {
+		return nil, ErrInvalidReference
+	}
 	// We take a copy of the transport, which contains a pointer to the
 	// store that it used for resolving this reference, so that the
 	// transport that we'll return from Transport() won't be affected by
@@ -32,7 +36,7 @@ func newReference(transport storageTransport, named reference.Named, id string)
 		transport: transport,
 		named:     named,
 		id:        id,
-	}
+	}, nil
 }
 
 // imageMatchesRepo returns true iff image.Names contains an element with the same repo as ref

storage/storage_reference_test.go

@@ -11,6 +11,15 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+func TestNewReference(t *testing.T) {
+	newStore(t)
+	st, ok := Transport.(*storageTransport)
+	require.True(t, ok)
+	// Success is tested throughout; test only the failure
+	_, err := newReference(*st, nil, "")
+	assert.Error(t, err)
+}
+
 func TestStorageReferenceTransport(t *testing.T) {
 	newStore(t)
 	ref, err := Transport.ParseReference("busybox")

storage/storage_transport.go

@@ -216,12 +216,12 @@ func (s storageTransport) ParseStoreReference(store storage.Store, ref string) (
 			// so refuse it.
 			return nil, errors.Errorf("invalid reference with digest @%s without a repository name", sum)
 		}
-		if 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")
-		}
 	}
 
-	result := newReference(storageTransport{store: store, defaultUIDMap: s.defaultUIDMap, defaultGIDMap: s.defaultGIDMap}, named, id)
+	result, err := newReference(storageTransport{store: store, defaultUIDMap: s.defaultUIDMap, defaultGIDMap: s.defaultGIDMap}, named, id)
+	if err != nil {
+		return nil, err
+	}
 	logrus.Debugf("parsed reference into %q", result.StringWithinTransport())
 	return result, nil
 }