diff --git a/daemon/daemon.go b/daemon/daemon.go index 6a8d18e923..3ef328a866 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -1024,20 +1024,16 @@ func (daemon *Daemon) changes(container *Container) ([]archive.Change, error) { // imageName. If force is true, an existing tag with the same name may be // overwritten. func (daemon *Daemon) TagImage(newTag reference.Named, imageName string, force bool) error { - if _, isDigested := newTag.(reference.Digested); isDigested { - return errors.New("refusing to create a tag with a digest reference") - } - if newTag.Name() == string(digest.Canonical) { - return errors.New("refusing to create an ambiguous tag using digest algorithm as name") - } - - newTag = registry.NormalizeLocalReference(newTag) imageID, err := daemon.GetImageID(imageName) if err != nil { return err } + newTag = registry.NormalizeLocalReference(newTag) + if err := daemon.tagStore.AddTag(newTag, imageID, force); err != nil { + return err + } daemon.EventsService.Log("tag", newTag.String(), "") - return daemon.tagStore.Add(newTag, imageID, force) + return nil } // PullImage initiates a pull operation. image is the repository name to pull, and diff --git a/daemon/daemon_windows.go b/daemon/daemon_windows.go index 53dad672dc..2b887b98a9 100644 --- a/daemon/daemon_windows.go +++ b/daemon/daemon_windows.go @@ -195,7 +195,7 @@ func restoreCustomImage(driver graphdriver.Driver, is image.Store, ls layer.Stor return err } - if err := ts.Add(ref, id, true); err != nil { + if err := ts.AddTag(ref, id, true); err != nil { return err } diff --git a/distribution/pull_v1.go b/distribution/pull_v1.go index d79b517082..5b6a9101b0 100644 --- a/distribution/pull_v1.go +++ b/distribution/pull_v1.go @@ -332,7 +332,7 @@ func (p *v1Puller) pullImage(out io.Writer, v1ID, endpoint string, localNameRef return layersDownloaded, err } - if err := p.config.TagStore.Add(localNameRef, imageID, true); err != nil { + if err := p.config.TagStore.AddTag(localNameRef, imageID, true); err != nil { return layersDownloaded, err } diff --git a/distribution/pull_v2.go b/distribution/pull_v2.go index b27970d2c8..fb25545c11 100644 --- a/distribution/pull_v2.go +++ b/distribution/pull_v2.go @@ -406,7 +406,11 @@ func (p *v2Puller) pullV2Tag(out io.Writer, ref reference.Named) (tagUpdated boo } if tagUpdated { - if err = p.config.TagStore.Add(ref, imageID, true); err != nil { + if canonical, ok := ref.(reference.Canonical); ok { + if err = p.config.TagStore.AddDigest(canonical, imageID, true); err != nil { + return false, err + } + } else if err = p.config.TagStore.AddTag(ref, imageID, true); err != nil { return false, err } } diff --git a/image/tarexport/load.go b/image/tarexport/load.go index 5d5507e0f3..473eb18e56 100644 --- a/image/tarexport/load.go +++ b/image/tarexport/load.go @@ -128,7 +128,7 @@ func (l *tarexporter) setLoadedTag(ref reference.NamedTagged, imgID image.ID, ou fmt.Fprintf(outStream, "The image %s already exists, renaming the old one with ID %s to empty string\n", ref.String(), string(prevID)) // todo: this message is wrong in case of multiple tags } - if err := l.ts.Add(ref, imgID, true); err != nil { + if err := l.ts.AddTag(ref, imgID, true); err != nil { return err } return nil diff --git a/migrate/v1/migratev1.go b/migrate/v1/migratev1.go index 86d99e6d06..14f6ce4ba5 100644 --- a/migrate/v1/migratev1.go +++ b/migrate/v1/migratev1.go @@ -190,7 +190,8 @@ func migrateContainers(root string, ls graphIDMounter, is image.Store, imageMapp } type tagAdder interface { - Add(ref reference.Named, id image.ID, force bool) error + AddTag(ref reference.Named, id image.ID, force bool) error + AddDigest(ref reference.Canonical, id image.ID, force bool) error } func migrateTags(root, driverName string, ts tagAdder, mappings map[string]image.ID) error { @@ -226,20 +227,23 @@ func migrateTags(root, driverName string, ts tagAdder, mappings map[string]image continue } if dgst, err := digest.ParseDigest(tag); err == nil { - ref, err = reference.WithDigest(ref, dgst) + canonical, err := reference.WithDigest(ref, dgst) if err != nil { logrus.Errorf("migrate tags: invalid digest %q, %q", dgst, err) continue } + if err := ts.AddDigest(canonical, strongID, false); err != nil { + logrus.Errorf("can't migrate digest %q for %q, err: %q", ref.String(), strongID, err) + } } else { - ref, err = reference.WithTag(ref, tag) + tagRef, err := reference.WithTag(ref, tag) if err != nil { logrus.Errorf("migrate tags: invalid tag %q, %q", tag, err) continue } - } - if err := ts.Add(ref, strongID, false); err != nil { - logrus.Errorf("can't migrate tag %q for %q, err: %q", ref.String(), strongID, err) + if err := ts.AddTag(tagRef, strongID, false); err != nil { + logrus.Errorf("can't migrate tag %q for %q, err: %q", ref.String(), strongID, err) + } } logrus.Infof("migrated tag %s:%s to point to %s", name, tag, strongID) } diff --git a/migrate/v1/migratev1_test.go b/migrate/v1/migratev1_test.go index 48d2a23411..11f250edcf 100644 --- a/migrate/v1/migratev1_test.go +++ b/migrate/v1/migratev1_test.go @@ -289,13 +289,16 @@ type mockTagAdder struct { refs map[string]string } -func (t *mockTagAdder) Add(ref reference.Named, id image.ID, force bool) error { +func (t *mockTagAdder) AddTag(ref reference.Named, id image.ID, force bool) error { if t.refs == nil { t.refs = make(map[string]string) } t.refs[ref.String()] = id.String() return nil } +func (t *mockTagAdder) AddDigest(ref reference.Canonical, id image.ID, force bool) error { + return t.AddTag(ref, id, force) +} type mockRegistrar struct { layers map[layer.ChainID]*mockLayer diff --git a/tag/store.go b/tag/store.go index 605f3ea8d6..0565cf335b 100644 --- a/tag/store.go +++ b/tag/store.go @@ -9,6 +9,7 @@ import ( "path/filepath" "sync" + "github.com/docker/distribution/digest" "github.com/docker/distribution/reference" "github.com/docker/docker/image" ) @@ -32,7 +33,8 @@ type Association struct { type Store interface { References(id image.ID) []reference.Named ReferencesByName(ref reference.Named) []Association - Add(ref reference.Named, id image.ID, force bool) error + AddTag(ref reference.Named, id image.ID, force bool) error + AddDigest(ref reference.Canonical, id image.ID, force bool) error Delete(ref reference.Named) (bool, error) Get(ref reference.Named) (image.ID, error) } @@ -90,10 +92,24 @@ func NewTagStore(jsonPath string) (Store, error) { return store, nil } -// Add adds a tag or digest to the store. If force is set to true, existing +// Add adds a tag to the store. If force is set to true, existing // references can be overwritten. This only works for tags, not digests. -func (store *store) Add(ref reference.Named, id image.ID, force bool) error { - ref = defaultTagIfNameOnly(ref) +func (store *store) AddTag(ref reference.Named, id image.ID, force bool) error { + if _, isDigested := ref.(reference.Digested); isDigested { + return errors.New("refusing to create a tag with a digest reference") + } + return store.addReference(defaultTagIfNameOnly(ref), id, force) +} + +// Add adds a digest reference to the store. +func (store *store) AddDigest(ref reference.Canonical, id image.ID, force bool) error { + return store.addReference(ref, id, force) +} + +func (store *store) addReference(ref reference.Named, id image.ID, force bool) error { + if ref.Name() == string(digest.Canonical) { + return errors.New("refusing to create an ambiguous tag using digest algorithm as name") + } store.mu.Lock() defer store.mu.Unlock() diff --git a/tag/store_test.go b/tag/store_test.go index 3a61289091..a424325616 100644 --- a/tag/store_test.go +++ b/tag/store_test.go @@ -4,6 +4,7 @@ import ( "bytes" "io/ioutil" "os" + "path/filepath" "sort" "strings" "testing" @@ -79,9 +80,16 @@ func TestSave(t *testing.T) { if err != nil { t.Fatalf("failed to parse reference: %v", err) } - err = store.Add(ref, id, false) - if err != nil { - t.Fatalf("could not add reference %s: %v", refStr, err) + if canonical, ok := ref.(reference.Canonical); ok { + err = store.AddDigest(canonical, id, false) + if err != nil { + t.Fatalf("could not add digest reference %s: %v", refStr, err) + } + } else { + err = store.AddTag(ref, id, false) + if err != nil { + t.Fatalf("could not add reference %s: %v", refStr, err) + } } } @@ -130,7 +138,7 @@ func TestAddDeleteGet(t *testing.T) { if err != nil { t.Fatalf("could not parse reference: %v", err) } - if err = store.Add(nameOnly, testImageID1, false); err != nil { + if err = store.AddTag(nameOnly, testImageID1, false); err != nil { t.Fatalf("error adding to store: %v", err) } @@ -139,7 +147,7 @@ func TestAddDeleteGet(t *testing.T) { if err != nil { t.Fatalf("could not parse reference: %v", err) } - if err = store.Add(ref1, testImageID1, false); err != nil { + if err = store.AddTag(ref1, testImageID1, false); err != nil { t.Fatalf("error adding to store: %v", err) } @@ -147,7 +155,7 @@ func TestAddDeleteGet(t *testing.T) { if err != nil { t.Fatalf("could not parse reference: %v", err) } - if err = store.Add(ref2, testImageID2, false); err != nil { + if err = store.AddTag(ref2, testImageID2, false); err != nil { t.Fatalf("error adding to store: %v", err) } @@ -155,7 +163,7 @@ func TestAddDeleteGet(t *testing.T) { if err != nil { t.Fatalf("could not parse reference: %v", err) } - if err = store.Add(ref3, testImageID1, false); err != nil { + if err = store.AddTag(ref3, testImageID1, false); err != nil { t.Fatalf("error adding to store: %v", err) } @@ -163,7 +171,7 @@ func TestAddDeleteGet(t *testing.T) { if err != nil { t.Fatalf("could not parse reference: %v", err) } - if err = store.Add(ref4, testImageID2, false); err != nil { + if err = store.AddTag(ref4, testImageID2, false); err != nil { t.Fatalf("error adding to store: %v", err) } @@ -171,16 +179,16 @@ func TestAddDeleteGet(t *testing.T) { if err != nil { t.Fatalf("could not parse reference: %v", err) } - if err = store.Add(ref5, testImageID2, false); err != nil { + if err = store.AddDigest(ref5.(reference.Canonical), testImageID2, false); err != nil { t.Fatalf("error adding to store: %v", err) } // Attempt to overwrite with force == false - if err = store.Add(ref4, testImageID3, false); err == nil || !strings.HasPrefix(err.Error(), "Conflict:") { + if err = store.AddTag(ref4, testImageID3, false); err == nil || !strings.HasPrefix(err.Error(), "Conflict:") { t.Fatalf("did not get expected error on overwrite attempt - got %v", err) } // Repeat to overwrite with force == true - if err = store.Add(ref4, testImageID3, true); err != nil { + if err = store.AddTag(ref4, testImageID3, true); err != nil { t.Fatalf("failed to force tag overwrite: %v", err) } @@ -326,3 +334,35 @@ func TestAddDeleteGet(t *testing.T) { t.Fatal("Expected ErrDoesNotExist from Get") } } + +func TestInvalidTags(t *testing.T) { + tmpDir, err := ioutil.TempDir("", "tag-store-test") + defer os.RemoveAll(tmpDir) + + store, err := NewTagStore(filepath.Join(tmpDir, "repositories.json")) + if err != nil { + t.Fatalf("error creating tag store: %v", err) + } + id := image.ID("sha256:470022b8af682154f57a2163d030eb369549549cba00edc69e1b99b46bb924d6") + + // sha256 as repo name + ref, err := reference.ParseNamed("sha256:abc") + if err != nil { + t.Fatal(err) + } + err = store.AddTag(ref, id, true) + if err == nil { + t.Fatalf("expected setting tag %q to fail", ref) + } + + // setting digest as a tag + ref, err = reference.ParseNamed("registry@sha256:367eb40fd0330a7e464777121e39d2f5b3e8e23a1e159342e53ab05c9e4d94e6") + if err != nil { + t.Fatal(err) + } + err = store.AddTag(ref, id, true) + if err == nil { + t.Fatalf("expected setting digest %q to fail", ref) + } + +}