From b6107dca86c9755599071e0ac817dd4e8a7251d6 Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Tue, 18 Sep 2018 16:49:08 -0700 Subject: [PATCH 1/2] Add import integration test Move tar creation test utilities to separate package Test all supported formats for import Signed-off-by: Derek McGowan --- archive/tar_test.go | 258 ++++++---------------------------- archive/tartest/tar.go | 210 ++++++++++++++++++++++++++++ images/archive/importer.go | 10 +- import_test.go | 276 +++++++++++++++++++++++++++++++++++++ 4 files changed, 535 insertions(+), 219 deletions(-) create mode 100644 archive/tartest/tar.go diff --git a/archive/tar_test.go b/archive/tar_test.go index 7486d3793..631a1dfc3 100644 --- a/archive/tar_test.go +++ b/archive/tar_test.go @@ -32,6 +32,7 @@ import ( _ "crypto/sha256" + "github.com/containerd/containerd/archive/tartest" "github.com/containerd/continuity/fs" "github.com/containerd/continuity/fs/fstest" "github.com/pkg/errors" @@ -183,7 +184,7 @@ func TestSymlinks(t *testing.T) { } func TestBreakouts(t *testing.T) { - tc := TarContext{}.WithUIDGID(os.Getuid(), os.Getgid()).WithModTime(time.Now().UTC()) + tc := tartest.TarContext{}.WithUIDGID(os.Getuid(), os.Getgid()).WithModTime(time.Now().UTC()) expected := "unbroken" unbrokenCheck := func(root string) error { b, err := ioutil.ReadFile(filepath.Join(root, "etc", "unbroken")) @@ -313,14 +314,14 @@ func TestBreakouts(t *testing.T) { breakouts := []struct { name string - w WriterToTar + w tartest.WriterToTar apply fstest.Applier validator func(string) error err error }{ { name: "SymlinkAbsolute", - w: TarAll( + w: tartest.TarAll( tc.Dir("etc", 0755), tc.Symlink("/etc", "localetc"), tc.File("/localetc/unbroken", []byte(expected), 0644), @@ -329,7 +330,7 @@ func TestBreakouts(t *testing.T) { }, { name: "SymlinkUpAndOut", - w: TarAll( + w: tartest.TarAll( tc.Dir("etc", 0755), tc.Dir("dummy", 0755), tc.Symlink("/dummy/../etc", "localetc"), @@ -339,7 +340,7 @@ func TestBreakouts(t *testing.T) { }, { name: "SymlinkMultipleAbsolute", - w: TarAll( + w: tartest.TarAll( tc.Dir("etc", 0755), tc.Dir("dummy", 0755), tc.Symlink("/etc", "/dummy/etc"), @@ -350,7 +351,7 @@ func TestBreakouts(t *testing.T) { }, { name: "SymlinkMultipleRelative", - w: TarAll( + w: tartest.TarAll( tc.Dir("etc", 0755), tc.Dir("dummy", 0755), tc.Symlink("/etc", "/dummy/etc"), @@ -361,7 +362,7 @@ func TestBreakouts(t *testing.T) { }, { name: "SymlinkEmptyFile", - w: TarAll( + w: tartest.TarAll( tc.Dir("etc", 0755), tc.File("etc/emptied", []byte("notempty"), 0644), tc.Symlink("/etc", "localetc"), @@ -380,7 +381,7 @@ func TestBreakouts(t *testing.T) { }, { name: "HardlinkRelative", - w: TarAll( + w: tartest.TarAll( tc.Dir("etc", 0770), tc.File("/etc/passwd", []byte("inside"), 0644), tc.Dir("breakouts", 0755), @@ -391,7 +392,7 @@ func TestBreakouts(t *testing.T) { }, { name: "HardlinkDownAndOut", - w: TarAll( + w: tartest.TarAll( tc.Dir("etc", 0770), tc.File("/etc/passwd", []byte("inside"), 0644), tc.Dir("breakouts", 0755), @@ -403,7 +404,7 @@ func TestBreakouts(t *testing.T) { }, { name: "HardlinkAbsolute", - w: TarAll( + w: tartest.TarAll( tc.Dir("etc", 0770), tc.File("/etc/passwd", []byte("inside"), 0644), tc.Symlink("/etc", "localetc"), @@ -413,7 +414,7 @@ func TestBreakouts(t *testing.T) { }, { name: "HardlinkRelativeLong", - w: TarAll( + w: tartest.TarAll( tc.Dir("etc", 0770), tc.File("/etc/passwd", []byte("inside"), 0644), tc.Symlink("../../../../../../../etc", "localetc"), @@ -423,7 +424,7 @@ func TestBreakouts(t *testing.T) { }, { name: "HardlinkRelativeUpAndOut", - w: TarAll( + w: tartest.TarAll( tc.Dir("etc", 0770), tc.File("/etc/passwd", []byte("inside"), 0644), tc.Symlink("upandout/../../../etc", "localetc"), @@ -433,7 +434,7 @@ func TestBreakouts(t *testing.T) { }, { name: "HardlinkDirectRelative", - w: TarAll( + w: tartest.TarAll( tc.Dir("etc", 0770), tc.File("/etc/passwd", []byte("inside"), 0644), tc.Link("../../../../../etc/passwd", "localpasswd"), @@ -442,7 +443,7 @@ func TestBreakouts(t *testing.T) { }, { name: "HardlinkDirectAbsolute", - w: TarAll( + w: tartest.TarAll( tc.Dir("etc", 0770), tc.File("/etc/passwd", []byte("inside"), 0644), tc.Link("/etc/passwd", "localpasswd"), @@ -451,7 +452,7 @@ func TestBreakouts(t *testing.T) { }, { name: "HardlinkSymlinkBeforeCreateTarget", - w: TarAll( + w: tartest.TarAll( tc.Dir("etc", 0770), tc.Symlink("/etc/passwd", "localpasswd"), tc.Link("localpasswd", "localpasswd-dup"), @@ -461,7 +462,7 @@ func TestBreakouts(t *testing.T) { }, { name: "HardlinkSymlinkRelative", - w: TarAll( + w: tartest.TarAll( tc.Dir("etc", 0770), tc.File("/etc/passwd", []byte("inside"), 0644), tc.Symlink("../../../../../etc/passwd", "passwdlink"), @@ -474,7 +475,7 @@ func TestBreakouts(t *testing.T) { }, { name: "HardlinkSymlinkAbsolute", - w: TarAll( + w: tartest.TarAll( tc.Dir("etc", 0770), tc.File("/etc/passwd", []byte("inside"), 0644), tc.Symlink("/etc/passwd", "passwdlink"), @@ -487,7 +488,7 @@ func TestBreakouts(t *testing.T) { }, { name: "SymlinkParentDirectory", - w: TarAll( + w: tartest.TarAll( tc.Dir("etc", 0770), tc.File("/etc/passwd", []byte("inside"), 0644), tc.Symlink("/etc/", ".."), @@ -497,7 +498,7 @@ func TestBreakouts(t *testing.T) { }, { name: "SymlinkEmptyFilename", - w: TarAll( + w: tartest.TarAll( tc.Dir("etc", 0770), tc.File("/etc/passwd", []byte("inside"), 0644), tc.Symlink("/etc/", ""), @@ -507,7 +508,7 @@ func TestBreakouts(t *testing.T) { }, { name: "SymlinkParentRelative", - w: TarAll( + w: tartest.TarAll( tc.Dir("etc", 0770), tc.File("/etc/passwd", []byte("inside"), 0644), tc.Symlink("/etc/", "localetc/sub/.."), @@ -517,7 +518,7 @@ func TestBreakouts(t *testing.T) { }, { name: "SymlinkSlashEnded", - w: TarAll( + w: tartest.TarAll( tc.Dir("etc", 0770), tc.File("/etc/passwd", []byte("inside"), 0644), tc.Dir("localetc/", 0770), @@ -532,7 +533,7 @@ func TestBreakouts(t *testing.T) { fstest.CreateFile("/etc/passwd", []byte("inside"), 0644), fstest.CreateDir("/localetc/", 0755), ), - w: TarAll( + w: tartest.TarAll( tc.Symlink("/etc", "localetc"), tc.Link("/etc/passwd", "/localetc/localpasswd"), ), @@ -545,7 +546,7 @@ func TestBreakouts(t *testing.T) { fstest.CreateFile("/etc/passwd", []byte("inside"), 0644), fstest.CreateDir("/localetc/", 0755), ), - w: TarAll( + w: tartest.TarAll( tc.Symlink("../../etc", "localetc"), tc.Link("/etc/passwd", "/localetc/localpasswd"), ), @@ -558,7 +559,7 @@ func TestBreakouts(t *testing.T) { fstest.CreateFile("/etc/passwd", []byte("inside"), 0644), fstest.Symlink("/etc", "localetc"), ), - w: TarAll( + w: tartest.TarAll( tc.Dir("/localetc/", 0755), tc.Link("/etc/passwd", "/localetc/localpasswd"), ), @@ -572,7 +573,7 @@ func TestBreakouts(t *testing.T) { fstest.Symlink("etc", "localetc"), fstest.Link("/etc/passwd", "/localetc/localpasswd"), ), - w: TarAll( + w: tartest.TarAll( tc.Dir("/localetc/", 0755), tc.File("/localetc/localpasswd", []byte("different"), 0644), ), @@ -584,7 +585,7 @@ func TestBreakouts(t *testing.T) { fstest.CreateDir("/etc/", 0755), fstest.CreateFile("/etc/passwd", []byte("inside"), 0644), ), - w: TarAll( + w: tartest.TarAll( tc.File(".wh...", []byte{}, 0644), // Should wipe out whole directory ), err: errInvalidArchive, @@ -595,7 +596,7 @@ func TestBreakouts(t *testing.T) { fstest.CreateDir("/etc/", 0755), fstest.CreateFile("/etc/passwd", []byte("inside"), 0644), ), - w: TarAll( + w: tartest.TarAll( tc.File("etc/.wh...", []byte{}, 0644), ), err: errInvalidArchive, @@ -606,7 +607,7 @@ func TestBreakouts(t *testing.T) { fstest.CreateDir("/etc/", 0755), fstest.CreateFile("/etc/passwd", []byte("inside"), 0644), ), - w: TarAll( + w: tartest.TarAll( tc.File(".wh..", []byte{}, 0644), ), err: errInvalidArchive, @@ -617,7 +618,7 @@ func TestBreakouts(t *testing.T) { fstest.CreateDir("/etc/", 0755), fstest.CreateFile("/etc/passwd", []byte("inside"), 0644), ), - w: TarAll( + w: tartest.TarAll( tc.File("etc/.wh..", []byte{}, 0644), // Should wipe out whole directory ), err: errInvalidArchive, @@ -629,7 +630,7 @@ func TestBreakouts(t *testing.T) { fstest.CreateFile("/etc/passwd", []byte("all users"), 0644), fstest.Symlink("/etc", "localetc"), ), - w: TarAll( + w: tartest.TarAll( tc.File(".wh.localetc", []byte{}, 0644), // Should wipe out whole directory ), validator: all( @@ -647,7 +648,7 @@ func TestBreakouts(t *testing.T) { fstest.CreateFile("/etc/whitedout", []byte("ahhhh whiteout"), 0644), fstest.Symlink("/etc", "localetc"), ), - w: TarAll( + w: tartest.TarAll( tc.File("localetc/.wh.whitedout", []byte{}, 0644), ), validator: all( @@ -663,7 +664,7 @@ func TestBreakouts(t *testing.T) { fstest.CreateFile("/etc/whitedout", []byte("ahhhh whiteout"), 0644), fstest.Symlink("/etc", "localetc"), ), - w: TarAll( + w: tartest.TarAll( tc.File(".wh.etc/somefile", []byte("non-empty"), 0644), ), validator: all( @@ -678,7 +679,7 @@ func TestBreakouts(t *testing.T) { fstest.CreateFile("/etc/passwd", []byte("all users"), 0644), fstest.Symlink("/dne", "localetc"), ), - w: TarAll( + w: tartest.TarAll( tc.File("localetc/.wh.etc", []byte{}, 0644), ), // no-op, remove does not @@ -691,7 +692,7 @@ func TestBreakouts(t *testing.T) { fstest.CreateFile("/etc/passwd", []byte("all users"), 0644), fstest.Symlink("/dne", "localetc"), ), - w: TarAll( + w: tartest.TarAll( tc.File("dne/../.wh.etc", []byte{}, 0644), ), // resolution ends up just removing etc @@ -709,7 +710,7 @@ func TestDiffApply(t *testing.T) { } func TestApplyTar(t *testing.T) { - tc := TarContext{}.WithUIDGID(os.Getuid(), os.Getgid()).WithModTime(time.Now().UTC()) + tc := tartest.TarContext{}.WithUIDGID(os.Getuid(), os.Getgid()).WithModTime(time.Now().UTC()) directoriesExist := func(dirs ...string) func(string) error { return func(root string) error { for _, d := range dirs { @@ -727,7 +728,7 @@ func TestApplyTar(t *testing.T) { tests := []struct { name string - w WriterToTar + w tartest.WriterToTar apply fstest.Applier validator func(string) error err error @@ -737,7 +738,7 @@ func TestApplyTar(t *testing.T) { apply: fstest.Apply( fstest.CreateDir("/etc/", 0755), ), - w: TarAll( + w: tartest.TarAll( tc.Dir("/etc/subdir", 0755), tc.Dir("/etc/subdir2/", 0755), tc.Dir("/etc/subdir2/more", 0755), @@ -866,7 +867,7 @@ func testDiffApply(appliers ...fstest.Applier) error { return fstest.CheckDirectoryEqual(td, dest) } -func makeWriterToTarTest(wt WriterToTar, a fstest.Applier, validate func(string) error, applyErr error) func(*testing.T) { +func makeWriterToTarTest(wt tartest.WriterToTar, a fstest.Applier, validate func(string) error, applyErr error) func(*testing.T) { return func(t *testing.T) { td, err := ioutil.TempDir("", "test-writer-to-tar-") if err != nil { @@ -880,7 +881,7 @@ func makeWriterToTarTest(wt WriterToTar, a fstest.Applier, validate func(string) } } - tr := TarFromWriterTo(wt) + tr := tartest.TarFromWriterTo(wt) if _, err := Apply(context.Background(), td, tr); err != nil { if applyErr == nil { @@ -1278,182 +1279,3 @@ func requireTar(t *testing.T) { t.Skipf("%s not found, skipping", tarCmd) } } - -// WriterToTar is an type which writes to a tar writer -type WriterToTar interface { - WriteTo(*tar.Writer) error -} - -type writerToFn func(*tar.Writer) error - -func (w writerToFn) WriteTo(tw *tar.Writer) error { - return w(tw) -} - -// TarAll creates a WriterToTar which calls all the provided writers -// in the order in which they are provided. -func TarAll(wt ...WriterToTar) WriterToTar { - return writerToFn(func(tw *tar.Writer) error { - for _, w := range wt { - if err := w.WriteTo(tw); err != nil { - return err - } - } - return nil - }) -} - -// TarFromWriterTo is used to create a tar stream from a tar record -// creator. This can be used to manifacture more specific tar records -// which allow testing specific tar cases which may be encountered -// by the untar process. -func TarFromWriterTo(wt WriterToTar) io.ReadCloser { - r, w := io.Pipe() - go func() { - tw := tar.NewWriter(w) - if err := wt.WriteTo(tw); err != nil { - w.CloseWithError(err) - return - } - w.CloseWithError(tw.Close()) - }() - - return r -} - -// TarContext is used to create tar records -type TarContext struct { - UID int - GID int - - // ModTime sets the modtimes for all files, if nil the current time - // is used for each file when it was written - ModTime *time.Time - - Xattrs map[string]string -} - -func (tc TarContext) newHeader(mode os.FileMode, name, link string, size int64) *tar.Header { - ti := tarInfo{ - name: name, - mode: mode, - size: size, - modt: tc.ModTime, - hdr: &tar.Header{ - Uid: tc.UID, - Gid: tc.GID, - Xattrs: tc.Xattrs, - }, - } - - if mode&os.ModeSymlink == 0 && link != "" { - ti.hdr.Typeflag = tar.TypeLink - ti.hdr.Linkname = link - } - - hdr, err := tar.FileInfoHeader(ti, link) - if err != nil { - // Only returns an error on bad input mode - panic(err) - } - - return hdr -} - -type tarInfo struct { - name string - mode os.FileMode - size int64 - modt *time.Time - hdr *tar.Header -} - -func (ti tarInfo) Name() string { - return ti.name -} - -func (ti tarInfo) Size() int64 { - return ti.size -} -func (ti tarInfo) Mode() os.FileMode { - return ti.mode -} - -func (ti tarInfo) ModTime() time.Time { - if ti.modt != nil { - return *ti.modt - } - return time.Now().UTC() -} - -func (ti tarInfo) IsDir() bool { - return (ti.mode & os.ModeDir) != 0 -} -func (ti tarInfo) Sys() interface{} { - return ti.hdr -} - -func (tc TarContext) WithUIDGID(uid, gid int) TarContext { - ntc := tc - ntc.UID = uid - ntc.GID = gid - return ntc -} - -func (tc TarContext) WithModTime(modtime time.Time) TarContext { - ntc := tc - ntc.ModTime = &modtime - return ntc -} - -// WithXattrs adds these xattrs to all files, merges with any -// previously added xattrs -func (tc TarContext) WithXattrs(xattrs map[string]string) TarContext { - ntc := tc - if ntc.Xattrs == nil { - ntc.Xattrs = map[string]string{} - } - for k, v := range xattrs { - ntc.Xattrs[k] = v - } - return ntc -} - -func (tc TarContext) File(name string, content []byte, perm os.FileMode) WriterToTar { - return writerToFn(func(tw *tar.Writer) error { - return writeHeaderAndContent(tw, tc.newHeader(perm, name, "", int64(len(content))), content) - }) -} - -func (tc TarContext) Dir(name string, perm os.FileMode) WriterToTar { - return writerToFn(func(tw *tar.Writer) error { - return writeHeaderAndContent(tw, tc.newHeader(perm|os.ModeDir, name, "", 0), nil) - }) -} - -func (tc TarContext) Symlink(oldname, newname string) WriterToTar { - return writerToFn(func(tw *tar.Writer) error { - return writeHeaderAndContent(tw, tc.newHeader(0777|os.ModeSymlink, newname, oldname, 0), nil) - }) -} - -func (tc TarContext) Link(oldname, newname string) WriterToTar { - return writerToFn(func(tw *tar.Writer) error { - return writeHeaderAndContent(tw, tc.newHeader(0777, newname, oldname, 0), nil) - }) -} - -func writeHeaderAndContent(tw *tar.Writer, h *tar.Header, b []byte) error { - if h.Size != int64(len(b)) { - return errors.New("bad content length") - } - if err := tw.WriteHeader(h); err != nil { - return err - } - if len(b) > 0 { - if _, err := tw.Write(b); err != nil { - return err - } - } - return nil -} diff --git a/archive/tartest/tar.go b/archive/tartest/tar.go new file mode 100644 index 000000000..36d899808 --- /dev/null +++ b/archive/tartest/tar.go @@ -0,0 +1,210 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package tartest + +import ( + "archive/tar" + "errors" + "io" + "os" + "time" +) + +// WriterToTar is an type which writes to a tar writer +type WriterToTar interface { + WriteTo(*tar.Writer) error +} + +type writerToFn func(*tar.Writer) error + +func (w writerToFn) WriteTo(tw *tar.Writer) error { + return w(tw) +} + +// TarAll creates a WriterToTar which calls all the provided writers +// in the order in which they are provided. +func TarAll(wt ...WriterToTar) WriterToTar { + return writerToFn(func(tw *tar.Writer) error { + for _, w := range wt { + if err := w.WriteTo(tw); err != nil { + return err + } + } + return nil + }) +} + +// TarFromWriterTo is used to create a tar stream from a tar record +// creator. This can be used to manifacture more specific tar records +// which allow testing specific tar cases which may be encountered +// by the untar process. +func TarFromWriterTo(wt WriterToTar) io.ReadCloser { + r, w := io.Pipe() + go func() { + tw := tar.NewWriter(w) + if err := wt.WriteTo(tw); err != nil { + w.CloseWithError(err) + return + } + w.CloseWithError(tw.Close()) + }() + + return r +} + +// TarContext is used to create tar records +type TarContext struct { + UID int + GID int + + // ModTime sets the modtimes for all files, if nil the current time + // is used for each file when it was written + ModTime *time.Time + + Xattrs map[string]string +} + +func (tc TarContext) newHeader(mode os.FileMode, name, link string, size int64) *tar.Header { + ti := tarInfo{ + name: name, + mode: mode, + size: size, + modt: tc.ModTime, + hdr: &tar.Header{ + Uid: tc.UID, + Gid: tc.GID, + Xattrs: tc.Xattrs, + }, + } + + if mode&os.ModeSymlink == 0 && link != "" { + ti.hdr.Typeflag = tar.TypeLink + ti.hdr.Linkname = link + } + + hdr, err := tar.FileInfoHeader(ti, link) + if err != nil { + // Only returns an error on bad input mode + panic(err) + } + + return hdr +} + +type tarInfo struct { + name string + mode os.FileMode + size int64 + modt *time.Time + hdr *tar.Header +} + +func (ti tarInfo) Name() string { + return ti.name +} + +func (ti tarInfo) Size() int64 { + return ti.size +} +func (ti tarInfo) Mode() os.FileMode { + return ti.mode +} + +func (ti tarInfo) ModTime() time.Time { + if ti.modt != nil { + return *ti.modt + } + return time.Now().UTC() +} + +func (ti tarInfo) IsDir() bool { + return (ti.mode & os.ModeDir) != 0 +} +func (ti tarInfo) Sys() interface{} { + return ti.hdr +} + +// WithUIDGID sets the UID and GID for tar entries +func (tc TarContext) WithUIDGID(uid, gid int) TarContext { + ntc := tc + ntc.UID = uid + ntc.GID = gid + return ntc +} + +// WithModTime sets the ModTime for tar entries +func (tc TarContext) WithModTime(modtime time.Time) TarContext { + ntc := tc + ntc.ModTime = &modtime + return ntc +} + +// WithXattrs adds these xattrs to all files, merges with any +// previously added xattrs +func (tc TarContext) WithXattrs(xattrs map[string]string) TarContext { + ntc := tc + if ntc.Xattrs == nil { + ntc.Xattrs = map[string]string{} + } + for k, v := range xattrs { + ntc.Xattrs[k] = v + } + return ntc +} + +// File returns a regular file tar entry using the provided bytes +func (tc TarContext) File(name string, content []byte, perm os.FileMode) WriterToTar { + return writerToFn(func(tw *tar.Writer) error { + return writeHeaderAndContent(tw, tc.newHeader(perm, name, "", int64(len(content))), content) + }) +} + +// Dir returns a directory tar entry +func (tc TarContext) Dir(name string, perm os.FileMode) WriterToTar { + return writerToFn(func(tw *tar.Writer) error { + return writeHeaderAndContent(tw, tc.newHeader(perm|os.ModeDir, name, "", 0), nil) + }) +} + +// Symlink returns a symlink tar entry +func (tc TarContext) Symlink(oldname, newname string) WriterToTar { + return writerToFn(func(tw *tar.Writer) error { + return writeHeaderAndContent(tw, tc.newHeader(0777|os.ModeSymlink, newname, oldname, 0), nil) + }) +} + +// Link returns a hard link tar entry +func (tc TarContext) Link(oldname, newname string) WriterToTar { + return writerToFn(func(tw *tar.Writer) error { + return writeHeaderAndContent(tw, tc.newHeader(0777, newname, oldname, 0), nil) + }) +} + +func writeHeaderAndContent(tw *tar.Writer, h *tar.Header, b []byte) error { + if h.Size != int64(len(b)) { + return errors.New("bad content length") + } + if err := tw.WriteHeader(h); err != nil { + return err + } + if len(b) > 0 { + if _, err := tw.Write(b); err != nil { + return err + } + } + return nil +} diff --git a/images/archive/importer.go b/images/archive/importer.go index 59514c66a..c1522df46 100644 --- a/images/archive/importer.go +++ b/images/archive/importer.go @@ -115,6 +115,10 @@ func ImportIndex(ctx context.Context, store content.Store, reader io.Reader) (oc return idx, nil } + if mfsts == nil { + return ocispec.Descriptor{}, errors.Errorf("unrecognized image format") + } + for name, linkname := range symlinks { desc, ok := blobs[linkname] if !ok { @@ -123,7 +127,11 @@ func ImportIndex(ctx context.Context, store content.Store, reader io.Reader) (oc blobs[name] = desc } - var idx ocispec.Index + idx := ocispec.Index{ + Versioned: specs.Versioned{ + SchemaVersion: 2, + }, + } for _, mfst := range mfsts { config, ok := blobs[mfst.Config] if !ok { diff --git a/import_test.go b/import_test.go index 817c7b296..0afd75521 100644 --- a/import_test.go +++ b/import_test.go @@ -17,11 +17,22 @@ package containerd import ( + "context" + "encoding/json" + "io" + + "io/ioutil" + "math/rand" "runtime" "testing" + "github.com/containerd/containerd/archive/tartest" + "github.com/containerd/containerd/images" "github.com/containerd/containerd/images/archive" "github.com/containerd/containerd/images/oci" + digest "github.com/opencontainers/go-digest" + specs "github.com/opencontainers/image-spec/specs-go" + ocispec "github.com/opencontainers/image-spec/specs-go/v1" ) // TestOCIExportAndImport exports testImage as a tar stream, @@ -65,3 +76,268 @@ func TestOCIExportAndImport(t *testing.T) { } } } + +func TestImport(t *testing.T) { + ctx, cancel := testContext() + defer cancel() + + client, err := New(address) + if err != nil { + t.Fatal(err) + } + defer client.Close() + + tc := tartest.TarContext{} + + b1, d1 := createContent(256, 1) + empty := []byte("{}") + version := []byte("1.0") + + c1, d2 := createConfig() + + m1, d3 := createManifest(c1, [][]byte{b1}) + + provider := client.ContentStore() + + checkManifest := func(ctx context.Context, t *testing.T, d ocispec.Descriptor) { + m, err := images.Manifest(ctx, provider, d, nil) + if err != nil { + t.Fatalf("unable to read target blob: %+v", err) + } + + if m.Config.Digest != d2 { + t.Fatalf("unexpected digest hash %s, expected %s", m.Config.Digest, d2) + } + + if len(m.Layers) != 1 { + t.Fatalf("expected 1 layer, has %d", len(m.Layers)) + } + + if m.Layers[0].Digest != d1 { + t.Fatalf("unexpected layer hash %s, expected %s", m.Layers[0].Digest, d1) + } + } + + for _, tc := range []struct { + Name string + Writer tartest.WriterToTar + Check func(*testing.T, []images.Image) + Opts []ImportOpt + }{ + { + Name: "DockerV2.0", + Writer: tartest.TarAll( + tc.Dir("bd765cd43e95212f7aa2cab51d0a", 0755), + tc.File("bd765cd43e95212f7aa2cab51d0a/json", empty, 0644), + tc.File("bd765cd43e95212f7aa2cab51d0a/layer.tar", b1, 0644), + tc.File("bd765cd43e95212f7aa2cab51d0a/VERSION", version, 0644), + tc.File("repositories", []byte(`{"any":{"1":"bd765cd43e95212f7aa2cab51d0a"}}`), 0644), + ), + }, + { + Name: "DockerV2.1", + Writer: tartest.TarAll( + tc.Dir("bd765cd43e95212f7aa2cab51d0a", 0755), + tc.File("bd765cd43e95212f7aa2cab51d0a/json", empty, 0644), + tc.File("bd765cd43e95212f7aa2cab51d0a/layer.tar", b1, 0644), + tc.File("bd765cd43e95212f7aa2cab51d0a/VERSION", version, 0644), + tc.File("e95212f7aa2cab51d0abd765cd43.json", c1, 0644), + tc.File("manifest.json", []byte(`[{"Config":"e95212f7aa2cab51d0abd765cd43.json","RepoTags":["test-import:notlatest", "another/repo:tag"],"Layers":["bd765cd43e95212f7aa2cab51d0a/layer.tar"]}]`), 0644), + ), + Check: func(t *testing.T, imgs []images.Image) { + if len(imgs) == 0 { + t.Fatalf("no images") + } + + names := []string{ + "docker.io/library/test-import:notlatest", + "docker.io/another/repo:tag", + } + + checkImages(t, imgs[0].Target.Digest, imgs, names...) + checkManifest(ctx, t, imgs[0].Target) + }, + }, + { + Name: "OCI-BadFormat", + Writer: tartest.TarAll( + tc.File("oci-layout", []byte(`{"imageLayoutVersion":"2.0.0"}`), 0644), + ), + }, + { + Name: "OCI", + Writer: tartest.TarAll( + tc.Dir("blobs", 0755), + tc.Dir("blobs/sha256", 0755), + tc.File("blobs/sha256/"+d1.Encoded(), b1, 0644), + tc.File("blobs/sha256/"+d2.Encoded(), c1, 0644), + tc.File("blobs/sha256/"+d3.Encoded(), m1, 0644), + tc.File("index.json", createIndex(m1, "latest", "docker.io/lib/img:ok"), 0644), + tc.File("oci-layout", []byte(`{"imageLayoutVersion":"1.0.0"}`), 0644), + ), + Check: func(t *testing.T, imgs []images.Image) { + names := []string{ + "latest", + "docker.io/lib/img:ok", + } + + checkImages(t, d3, imgs, names...) + checkManifest(ctx, t, imgs[0].Target) + }, + }, + { + Name: "OCIPrefixName", + Writer: tartest.TarAll( + tc.Dir("blobs", 0755), + tc.Dir("blobs/sha256", 0755), + tc.File("blobs/sha256/"+d1.Encoded(), b1, 0644), + tc.File("blobs/sha256/"+d2.Encoded(), c1, 0644), + tc.File("blobs/sha256/"+d3.Encoded(), m1, 0644), + tc.File("index.json", createIndex(m1, "latest", "docker.io/lib/img:ok"), 0644), + tc.File("oci-layout", []byte(`{"imageLayoutVersion":"1.0.0"}`), 0644), + ), + Check: func(t *testing.T, imgs []images.Image) { + names := []string{ + "localhost:5000/myimage:latest", + "docker.io/lib/img:ok", + } + + checkImages(t, d3, imgs, names...) + checkManifest(ctx, t, imgs[0].Target) + }, + Opts: []ImportOpt{ + WithImageRefTranslator(archive.AddRefPrefix("localhost:5000/myimage")), + }, + }, + { + Name: "OCIPrefixName", + Writer: tartest.TarAll( + tc.Dir("blobs", 0755), + tc.Dir("blobs/sha256", 0755), + tc.File("blobs/sha256/"+d1.Encoded(), b1, 0644), + tc.File("blobs/sha256/"+d2.Encoded(), c1, 0644), + tc.File("blobs/sha256/"+d3.Encoded(), m1, 0644), + tc.File("index.json", createIndex(m1, "latest", "localhost:5000/myimage:old", "docker.io/lib/img:ok"), 0644), + tc.File("oci-layout", []byte(`{"imageLayoutVersion":"1.0.0"}`), 0644), + ), + Check: func(t *testing.T, imgs []images.Image) { + names := []string{ + "localhost:5000/myimage:latest", + "localhost:5000/myimage:old", + } + + checkImages(t, d3, imgs, names...) + checkManifest(ctx, t, imgs[0].Target) + }, + Opts: []ImportOpt{ + WithImageRefTranslator(archive.FilterRefPrefix("localhost:5000/myimage")), + }, + }, + } { + t.Run(tc.Name, func(t *testing.T) { + images, err := client.Import(ctx, tartest.TarFromWriterTo(tc.Writer), tc.Opts...) + if err != nil { + if tc.Check != nil { + t.Errorf("unexpected import error: %+v", err) + } + return + } else if tc.Check == nil { + t.Fatalf("expected error on import") + } + + tc.Check(t, images) + }) + } +} + +func checkImages(t *testing.T, target digest.Digest, actual []images.Image, names ...string) { + if len(names) != len(actual) { + t.Fatalf("expected %d images, got %d", len(names), len(actual)) + } + + for i, n := range names { + if actual[i].Target.Digest != target { + t.Fatalf("image(%d) unexpected target %s, expected %s", i, actual[i].Target.Digest, target) + } + if actual[i].Name != n { + t.Fatalf("image(%d) unexpected name %q, expected %q", i, actual[i].Name, n) + } + + if actual[i].Target.MediaType != ocispec.MediaTypeImageManifest { + t.Fatalf("image(%d) unexpected media type: %s", i, actual[i].Target.MediaType) + } + } + +} + +func createContent(size int64, seed int64) ([]byte, digest.Digest) { + b, err := ioutil.ReadAll(io.LimitReader(rand.New(rand.NewSource(seed)), size)) + if err != nil { + panic(err) + } + return b, digest.FromBytes(b) +} + +func createConfig() ([]byte, digest.Digest) { + image := ocispec.Image{ + OS: "any", + Architecture: "any", + Author: "test", + } + b, _ := json.Marshal(image) + + return b, digest.FromBytes(b) +} + +func createManifest(config []byte, layers [][]byte) ([]byte, digest.Digest) { + manifest := ocispec.Manifest{ + Versioned: specs.Versioned{ + SchemaVersion: 2, + }, + Config: ocispec.Descriptor{ + MediaType: ocispec.MediaTypeImageConfig, + Digest: digest.FromBytes(config), + Size: int64(len(config)), + }, + } + for _, l := range layers { + manifest.Layers = append(manifest.Layers, ocispec.Descriptor{ + MediaType: ocispec.MediaTypeImageLayer, + Digest: digest.FromBytes(l), + Size: int64(len(l)), + }) + } + + b, _ := json.Marshal(manifest) + + return b, digest.FromBytes(b) +} + +func createIndex(manifest []byte, tags ...string) []byte { + idx := ocispec.Index{ + Versioned: specs.Versioned{ + SchemaVersion: 2, + }, + } + d := ocispec.Descriptor{ + MediaType: ocispec.MediaTypeImageManifest, + Digest: digest.FromBytes(manifest), + Size: int64(len(manifest)), + } + + if len(tags) == 0 { + idx.Manifests = append(idx.Manifests, d) + } else { + for _, t := range tags { + dt := d + dt.Annotations = map[string]string{ + ocispec.AnnotationRefName: t, + } + idx.Manifests = append(idx.Manifests, dt) + } + } + + b, _ := json.Marshal(idx) + + return b +} From 440c7ed249d255d7426e9ac8b017463ec346d3ca Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Tue, 2 Oct 2018 14:16:57 -0700 Subject: [PATCH 2/2] Fix commit already exists not leasing Signed-off-by: Derek McGowan --- metadata/content.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/metadata/content.go b/metadata/content.go index ac4440060..f51b0aadd 100644 --- a/metadata/content.go +++ b/metadata/content.go @@ -553,7 +553,9 @@ func (nw *namespacedWriter) Commit(ctx context.Context, size int64, expected dig nw.l.RLock() defer nw.l.RUnlock() - return update(ctx, nw.db, func(tx *bolt.Tx) error { + var innerErr error + + if err := update(ctx, nw.db, func(tx *bolt.Tx) error { bkt := getIngestsBucket(tx, nw.namespace) if bkt != nil { if err := bkt.DeleteBucket([]byte(nw.ref)); err != nil && err != bolt.ErrBucketNotFound { @@ -562,13 +564,20 @@ func (nw *namespacedWriter) Commit(ctx context.Context, size int64, expected dig } dgst, err := nw.commit(ctx, tx, size, expected, opts...) if err != nil { - return err + if !errdefs.IsAlreadyExists(err) { + return err + } + innerErr = err } if err := removeIngestLease(ctx, tx, nw.ref); err != nil { return err } return addContentLease(ctx, tx, dgst) - }) + }); err != nil { + return err + } + + return innerErr } func (nw *namespacedWriter) commit(ctx context.Context, tx *bolt.Tx, size int64, expected digest.Digest, opts ...content.Opt) (digest.Digest, error) { @@ -611,7 +620,7 @@ func (nw *namespacedWriter) commit(ctx context.Context, tx *bolt.Tx, size int64, bkt, err := createBlobBucket(tx, nw.namespace, actual) if err != nil { if err == bolt.ErrBucketExists { - return "", errors.Wrapf(errdefs.ErrAlreadyExists, "content %v", actual) + return actual, errors.Wrapf(errdefs.ErrAlreadyExists, "content %v", actual) } return "", err }