diff --git a/storage/storage_reference.go b/storage/storage_reference.go index b5db19c6..68984d95 100644 --- a/storage/storage_reference.go +++ b/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 diff --git a/storage/storage_reference_test.go b/storage/storage_reference_test.go index b0745a9c..dcf85b37 100644 --- a/storage/storage_reference_test.go +++ b/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") diff --git a/storage/storage_transport.go b/storage/storage_transport.go index 84b4f1f4..4713f99f 100644 --- a/storage/storage_transport.go +++ b/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 }