From 7dfbb222413ec4ca7adbfe6e3bfc5ba7ddb2c26b Mon Sep 17 00:00:00 2001 From: Yiran Wang Date: Tue, 10 Mar 2020 22:37:21 -0700 Subject: [PATCH 1/3] Make ADD untar tar files --- lib/builder/step/add_copy_step.go | 10 ++++-- lib/builder/step/add_copy_step_test.go | 8 ++--- lib/builder/step/add_step.go | 3 +- lib/builder/step/copy_step.go | 3 +- lib/snapshot/copy_op.go | 44 ++++++++++++++++++++++---- lib/snapshot/copy_op_test.go | 20 ++++++------ lib/snapshot/mem_fs_test.go | 16 +++++----- 7 files changed, 70 insertions(+), 34 deletions(-) diff --git a/lib/builder/step/add_copy_step.go b/lib/builder/step/add_copy_step.go index 14dbdeba..39fe1cf1 100644 --- a/lib/builder/step/add_copy_step.go +++ b/lib/builder/step/add_copy_step.go @@ -58,12 +58,13 @@ type addCopyStep struct { toPath string chown string preserveOwner bool + isAdd bool } // newAddCopyStep returns a BuildStep from given arguments. func newAddCopyStep( directive Directive, args, chown, fromStage string, - fromPaths []string, toPath string, commit, preserveOwner bool) (*addCopyStep, error) { + fromPaths []string, toPath string, commit, preserveOwner, isAdd bool) (*addCopyStep, error) { toPath = strings.Trim(toPath, "\"'") for i := range fromPaths { @@ -80,6 +81,7 @@ func newAddCopyStep( toPath: toPath, chown: chown, preserveOwner: preserveOwner, + isAdd: isAdd, }, nil } @@ -136,14 +138,16 @@ func (s *addCopyStep) Execute(ctx *context.BuildContext, modifyFS bool) (err err internal := s.fromStage != "" blacklist := append(pathutils.DefaultBlacklist, ctx.ImageStore.RootDir) - copyOp, err := snapshot.NewCopyOperation( - relPaths, sourceRoot, s.workingDir, s.toPath, s.chown, blacklist, internal, s.preserveOwner) + copyOp, err := snapshot.NewCopyOperation(relPaths, sourceRoot, s.workingDir, s.toPath, s.chown, + s.preserveOwner, internal, blacklist, s.isAdd) if err != nil { return fmt.Errorf("invalid copy operation: %s", err) } ctx.CopyOps = append(ctx.CopyOps, copyOp) if modifyFS { + // Note: We still want to keep this copyop in the list, just in case there + // is no RUN afterwards, and layer needs to be created from copyops. return copyOp.Execute() } return nil diff --git a/lib/builder/step/add_copy_step_test.go b/lib/builder/step/add_copy_step_test.go index b7de81cf..8cdec0cb 100644 --- a/lib/builder/step/add_copy_step_test.go +++ b/lib/builder/step/add_copy_step_test.go @@ -24,21 +24,21 @@ func TestContextDirs(t *testing.T) { require := require.New(t) srcs := []string{} - ac, err := newAddCopyStep(Copy, "", "", "", srcs, "", false, false) + ac, err := newAddCopyStep(Copy, "", "", "", srcs, "", false, false, false) require.NoError(err) stage, paths := ac.ContextDirs() require.Equal("", stage) require.Len(paths, 0) srcs = []string{"src"} - ac, err = newAddCopyStep(Copy, "", "", "", srcs, "", false, false) + ac, err = newAddCopyStep(Copy, "", "", "", srcs, "", false, false, false) require.NoError(err) stage, paths = ac.ContextDirs() require.Equal("", stage) require.Len(paths, 0) srcs = []string{"src"} - ac, err = newAddCopyStep(Copy, "", "", "stage", srcs, "", false, false) + ac, err = newAddCopyStep(Copy, "", "", "stage", srcs, "", false, false, false) require.NoError(err) stage, paths = ac.ContextDirs() require.Equal("stage", stage) @@ -48,7 +48,7 @@ func TestContextDirs(t *testing.T) { func TestTrimmingPaths(t *testing.T) { require := require.New(t) - ac, err := newAddCopyStep(Copy, "", "", "", []string{"\"/from/path\""}, "\"/to/path\"", false, false) + ac, err := newAddCopyStep(Copy, "", "", "", []string{"\"/from/path\""}, "\"/to/path\"", false, false, false) require.NoError(err) require.Equal("/from/path", ac.fromPaths[0]) diff --git a/lib/builder/step/add_step.go b/lib/builder/step/add_step.go index d4fc8f5b..5eb4333e 100644 --- a/lib/builder/step/add_step.go +++ b/lib/builder/step/add_step.go @@ -23,7 +23,8 @@ type AddStep struct { // NewAddStep creates a new AddStep func NewAddStep(args, chown string, fromPaths []string, toPath string, commit, preserverOwner bool) (*AddStep, error) { - s, err := newAddCopyStep(Add, args, chown, "", fromPaths, toPath, commit, preserverOwner) + s, err := newAddCopyStep( + Add, args, chown, "", fromPaths, toPath, commit, preserverOwner, true) if err != nil { return nil, fmt.Errorf("new add/copy step: %s", err) } diff --git a/lib/builder/step/copy_step.go b/lib/builder/step/copy_step.go index 44738638..3343b7cd 100644 --- a/lib/builder/step/copy_step.go +++ b/lib/builder/step/copy_step.go @@ -26,7 +26,8 @@ func NewCopyStep( args, chown, fromStage string, fromPaths []string, toPath string, commit, preserveOwner bool, ) (*CopyStep, error) { - s, err := newAddCopyStep(Copy, args, chown, fromStage, fromPaths, toPath, commit, preserveOwner) + s, err := newAddCopyStep( + Copy, args, chown, fromStage, fromPaths, toPath, commit, preserveOwner, false) if err != nil { return nil, fmt.Errorf("new add/copy step: %s", err) } diff --git a/lib/snapshot/copy_op.go b/lib/snapshot/copy_op.go index 06aef3f3..2b3dc2b5 100644 --- a/lib/snapshot/copy_op.go +++ b/lib/snapshot/copy_op.go @@ -22,10 +22,10 @@ import ( "github.com/uber/makisu/lib/fileio" "github.com/uber/makisu/lib/pathutils" + "github.com/uber/makisu/lib/tario" "github.com/uber/makisu/lib/utils" ) -// CopyOperation defines a copy operation that occurred to generate a layer from. type CopyOperation struct { srcRoot string srcs []string @@ -34,16 +34,20 @@ type CopyOperation struct { gid int preserveOwner bool - blacklist []string // Indicates if the copy op is used for copying from previous stages. - internal bool + // Blacklist is ignored in that case. + internal bool + blacklist []string + + // Set to true if the copy op was created by ADD step. + // Some functionalities are only available to ADD. + fromAdd bool } // NewCopyOperation initializes and validates a CopyOperation. Use "internal" to // specify if the copy op is used for copying from previous stages. -func NewCopyOperation( - srcs []string, srcRoot, workDir, dst, chown string, - blacklist []string, internal, preserveOwner bool) (*CopyOperation, error) { +func NewCopyOperation(srcs []string, srcRoot, workDir, dst, chown string, + preserveOwner, internal bool, blacklist []string, fromAdd bool) (*CopyOperation, error) { if err := checkCopyParams(srcs, workDir, dst); err != nil { return nil, fmt.Errorf("check copy param: %s", err) @@ -70,6 +74,7 @@ func NewCopyOperation( preserveOwner: preserveOwner, blacklist: blacklist, internal: internal, + fromAdd: fromAdd, }, nil } @@ -86,6 +91,32 @@ func (c *CopyOperation) Execute() error { if err != nil { return fmt.Errorf("lstat %s: %s", src, err) } + + if strings.HasSuffix(src, ".tar.gz") && c.fromAdd { + // Special feature for ADD - Extract tar.gz into dst directory. + // If dst is an existing directory, untar. + // If dst doesn't exist, create it with root. + // If dst exists and is not a directory, fail. + reader, err := os.Open(src) + if err != nil { + return fmt.Errorf("open tar file: %s", err) + } + defer reader.Close() + + dstFi, err := os.Stat(c.dst) + if os.IsNotExist(err) { + if err := os.MkdirAll(c.dst, 0755); err != nil { + return fmt.Errorf("create untar directory: %s", err) + } + } + if !dstFi.IsDir() { + return fmt.Errorf("target untar path exists and is not directory: %s", c.dst) + } + if err := tario.Untar(reader, c.dst); err != nil { + return fmt.Errorf("untar tar: %s", err) + } + } + var copier fileio.Copier if c.internal { copier = fileio.NewInternalCopier() @@ -115,7 +146,6 @@ func (c *CopyOperation) Execute() error { return fmt.Errorf("copy file %s to dir %s: %s", src, targetFilePath, err) } } - } else { // File to file if c.preserveOwner { diff --git a/lib/snapshot/copy_op_test.go b/lib/snapshot/copy_op_test.go index e82d1dcc..aac2f921 100644 --- a/lib/snapshot/copy_op_test.go +++ b/lib/snapshot/copy_op_test.go @@ -45,28 +45,28 @@ func TestNewCopyOperation(t *testing.T) { workDir := "" dst := "/test2/test.txt" _, err = NewCopyOperation( - srcs, srcRoot, workDir, dst, validChown, pathutils.DefaultBlacklist, false, false) + srcs, srcRoot, workDir, dst, validChown, false, false, pathutils.DefaultBlacklist, false) require.Error(err) srcs = []string{"file", "dir/"} workDir = "" dst = "/target/test" _, err = NewCopyOperation( - srcs, srcRoot, workDir, dst, validChown, pathutils.DefaultBlacklist, false, false) + srcs, srcRoot, workDir, dst, validChown, false, false, pathutils.DefaultBlacklist, false) require.Error(err) srcs = []string{"file", "dir/"} workDir = "" dst = "target/test" _, err = NewCopyOperation( - srcs, srcRoot, workDir, dst, validChown, pathutils.DefaultBlacklist, false, false) + srcs, srcRoot, workDir, dst, validChown, false, false, pathutils.DefaultBlacklist, false) require.Error(err) srcs = []string{"file", "dir/"} workDir = "wrk/" dst = "target/test/" _, err = NewCopyOperation( - srcs, srcRoot, workDir, dst, validChown, pathutils.DefaultBlacklist, false, false) + srcs, srcRoot, workDir, dst, validChown, false, false, pathutils.DefaultBlacklist, false) require.Error(err) } @@ -87,7 +87,7 @@ func TestExecuteCopyOperation(t *testing.T) { srcRoot := tmpRoot1 dst := filepath.Join(tmpRoot2, "test2/test.txt") c, err := NewCopyOperation( - srcs, srcRoot, "", dst, validChown, pathutils.DefaultBlacklist, false, true) + srcs, srcRoot, "", dst, validChown, true, false, pathutils.DefaultBlacklist, false) require.NoError(err) require.NoError(c.Execute()) b, err := ioutil.ReadFile(dst) @@ -107,7 +107,7 @@ func TestExecuteCopyOperation(t *testing.T) { workDir := tmpRoot2 dst := "test2/test.txt" c, err := NewCopyOperation( - srcs, srcRoot, workDir, dst, validChown, pathutils.DefaultBlacklist, false, false) + srcs, srcRoot, workDir, dst, validChown, false, false, pathutils.DefaultBlacklist, false) require.NoError(err) require.NoError(c.Execute()) b, err := ioutil.ReadFile(filepath.Join(tmpRoot2, dst)) @@ -129,7 +129,7 @@ func TestExecuteCopyOperation(t *testing.T) { workDir := tmpRoot2 dst := "test2/" c, err := NewCopyOperation( - srcs, srcRoot, workDir, dst, validChown, pathutils.DefaultBlacklist, false, false) + srcs, srcRoot, workDir, dst, validChown, false, false, pathutils.DefaultBlacklist, false) require.NoError(err) require.NoError(c.Execute()) b, err := ioutil.ReadFile(filepath.Join(tmpRoot2, dst, "test.txt")) @@ -154,7 +154,7 @@ func TestExecuteCopyOperation(t *testing.T) { workDir := filepath.Join(tmpRoot2, "test2") dst := "." c, err := NewCopyOperation( - srcs, srcRoot, workDir, dst, validChown, pathutils.DefaultBlacklist, false, true) + srcs, srcRoot, workDir, dst, validChown, true, false, pathutils.DefaultBlacklist, false) require.NoError(err) require.NoError(c.Execute()) b, err := ioutil.ReadFile(filepath.Join(tmpRoot2, "test2", "test.txt")) @@ -181,7 +181,7 @@ func TestExecuteCopyOperation(t *testing.T) { workDir := tmpRoot2 dst := "test2/" c, err := NewCopyOperation( - srcs, srcRoot, workDir, dst, validChown, pathutils.DefaultBlacklist, false, false) + srcs, srcRoot, workDir, dst, validChown, false, false, pathutils.DefaultBlacklist, false) require.NoError(err) require.NoError(c.Execute()) b, err := ioutil.ReadFile(filepath.Join(tmpRoot2, dst, "test.txt")) @@ -207,7 +207,7 @@ func TestExecuteCopyOperation(t *testing.T) { workDir := tmpRoot2 dst := "test2/" c, err := NewCopyOperation( - srcs, srcRoot, workDir, dst, validChown, pathutils.DefaultBlacklist, false, false) + srcs, srcRoot, workDir, dst, validChown, false, false, pathutils.DefaultBlacklist, false) require.NoError(err) require.NoError(c.Execute()) b, err := ioutil.ReadFile(filepath.Join(tmpRoot2, dst, "test.txt")) diff --git a/lib/snapshot/mem_fs_test.go b/lib/snapshot/mem_fs_test.go index 42d84a26..5cceae81 100644 --- a/lib/snapshot/mem_fs_test.go +++ b/lib/snapshot/mem_fs_test.go @@ -710,7 +710,7 @@ func TestCreateLayerByCopy(t *testing.T) { workDir := "" dst := "/test2/test.txt" c, err := NewCopyOperation( - srcs, srcRoot, workDir, dst, validChown, pathutils.DefaultBlacklist, false, false) + srcs, srcRoot, workDir, dst, validChown, false, false, pathutils.DefaultBlacklist, false) require.NoError(err) err = fs.addToLayer(newMemLayer(), c) require.NoError(err) @@ -750,7 +750,7 @@ func TestCreateLayerByCopy(t *testing.T) { workDir := "" dst := "/dst/" c, err := NewCopyOperation( - srcs, srcRoot, workDir, dst, validChown, pathutils.DefaultBlacklist, false, false) + srcs, srcRoot, workDir, dst, validChown, false, false, pathutils.DefaultBlacklist, false) require.NoError(err) err = fs.addToLayer(newMemLayer(), c) require.NoError(err) @@ -792,7 +792,7 @@ func TestCreateLayerByCopy(t *testing.T) { workDir := "" dst := "/dst/" c, err := NewCopyOperation( - srcs, srcRoot, workDir, dst, validChown, pathutils.DefaultBlacklist, false, false) + srcs, srcRoot, workDir, dst, validChown, false, false, pathutils.DefaultBlacklist, false) require.NoError(err) err = fs.addToLayer(newMemLayer(), c) require.NoError(err) @@ -839,7 +839,7 @@ func TestCreateLayerByCopy(t *testing.T) { workDir := "" dst := "/dst/" c, err := NewCopyOperation( - srcs, srcRoot, workDir, dst, validChown, pathutils.DefaultBlacklist, false, false) + srcs, srcRoot, workDir, dst, validChown, false, false, pathutils.DefaultBlacklist, false) require.NoError(err) err = fs.addToLayer(newMemLayer(), c) require.NoError(err) @@ -887,7 +887,7 @@ func TestCreateLayerByCopy(t *testing.T) { workDir := "" dst := "/dst/" c, err := NewCopyOperation( - srcs, srcRoot, workDir, dst, validChown, pathutils.DefaultBlacklist, false, false) + srcs, srcRoot, workDir, dst, validChown, false, false, pathutils.DefaultBlacklist, false) require.NoError(err) err = fs.addToLayer(newMemLayer(), c) require.NoError(err) @@ -945,7 +945,7 @@ func TestCreateLayerByCopy(t *testing.T) { workDir := "" dst := "/dst/" c, err := NewCopyOperation( - srcs, srcRoot, workDir, dst, validChown, pathutils.DefaultBlacklist, false, false) + srcs, srcRoot, workDir, dst, validChown, false, false, pathutils.DefaultBlacklist, false) require.NoError(err) err = fs.addToLayer(newMemLayer(), c) require.NoError(err) @@ -1003,7 +1003,7 @@ func TestCreateLayerByCopy(t *testing.T) { workDir := "/wrk" dst := "dst/" c, err := NewCopyOperation( - srcs, srcRoot, workDir, dst, validChown, pathutils.DefaultBlacklist, false, false) + srcs, srcRoot, workDir, dst, validChown, false, false, pathutils.DefaultBlacklist, false) require.NoError(err) err = fs.addToLayer(newMemLayer(), c) require.NoError(err) @@ -1159,7 +1159,7 @@ func TestAddLayersEqual(t *testing.T) { workDir := "/wrk" dst := "dst/" c, err := NewCopyOperation( - srcs, srcRoot, workDir, dst, validChown, pathutils.DefaultBlacklist, false, false) + srcs, srcRoot, workDir, dst, validChown, false, false, pathutils.DefaultBlacklist, false) require.NoError(err) err = fs1.AddLayerByCopyOps([]*CopyOperation{c}, w1) require.NoError(err) From 4ef420c1bfde4a1a27dd8b97d3d2fea815794863 Mon Sep 17 00:00:00 2001 From: Yiran Wang Date: Wed, 11 Mar 2020 07:10:40 -0700 Subject: [PATCH 2/3] Refactor tests --- lib/snapshot/copy_op_test.go | 61 +++++++++++++++++++++++++----------- 1 file changed, 42 insertions(+), 19 deletions(-) diff --git a/lib/snapshot/copy_op_test.go b/lib/snapshot/copy_op_test.go index aac2f921..fac71cee 100644 --- a/lib/snapshot/copy_op_test.go +++ b/lib/snapshot/copy_op_test.go @@ -71,16 +71,16 @@ func TestNewCopyOperation(t *testing.T) { } func TestExecuteCopyOperation(t *testing.T) { - tmpRoot1, err := ioutil.TempDir("/tmp", "makisu-test") - require.NoError(t, err) - defer os.RemoveAll(tmpRoot1) - tmpRoot2, err := ioutil.TempDir("/tmp", "makisu-test") - require.NoError(t, err) - defer os.RemoveAll(tmpRoot2) - t.Run("absolute file to absolute file", func(t *testing.T) { require := require.New(t) + tmpRoot1, err := ioutil.TempDir("/tmp", "makisu-test") + require.NoError(err) + defer os.RemoveAll(tmpRoot1) + tmpRoot2, err := ioutil.TempDir("/tmp", "makisu-test") + require.NoError(err) + defer os.RemoveAll(tmpRoot2) + srcs := []string{"/test.txt"} require.NoError(ioutil.WriteFile(filepath.Join(tmpRoot1, "/test.txt"), _hello, os.ModePerm)) require.NoError(os.Chown(filepath.Join(tmpRoot1, "/test.txt"), testutil.CurrUID(), testutil.CurrGID())) @@ -94,12 +94,17 @@ func TestExecuteCopyOperation(t *testing.T) { require.NoError(err) require.Equal(_hello, b) }) - removeAllChildren(tmpRoot1, nil) - removeAllChildren(tmpRoot2, nil) t.Run("absolute file to relative file", func(t *testing.T) { require := require.New(t) + tmpRoot1, err := ioutil.TempDir("/tmp", "makisu-test") + require.NoError(err) + defer os.RemoveAll(tmpRoot1) + tmpRoot2, err := ioutil.TempDir("/tmp", "makisu-test") + require.NoError(err) + defer os.RemoveAll(tmpRoot2) + srcs := []string{"/test.txt"} require.NoError(ioutil.WriteFile(filepath.Join(tmpRoot1, "test.txt"), _hello, os.ModePerm)) require.NoError(os.Chown(filepath.Join(tmpRoot1, "/test.txt"), testutil.CurrUID(), testutil.CurrGID())) @@ -114,12 +119,17 @@ func TestExecuteCopyOperation(t *testing.T) { require.NoError(err) require.Equal(_hello, b) }) - removeAllChildren(tmpRoot1, nil) - removeAllChildren(tmpRoot2, nil) t.Run("absolute files to absolute dir", func(t *testing.T) { require := require.New(t) + tmpRoot1, err := ioutil.TempDir("/tmp", "makisu-test") + require.NoError(err) + defer os.RemoveAll(tmpRoot1) + tmpRoot2, err := ioutil.TempDir("/tmp", "makisu-test") + require.NoError(err) + defer os.RemoveAll(tmpRoot2) + srcs := []string{"/test.txt", "/test2.txt"} require.NoError(ioutil.WriteFile(filepath.Join(tmpRoot1, "test.txt"), _hello, os.ModePerm)) require.NoError(os.Chown(filepath.Join(tmpRoot1, "test.txt"), testutil.CurrUID(), testutil.CurrGID())) @@ -139,12 +149,17 @@ func TestExecuteCopyOperation(t *testing.T) { require.NoError(err) require.Equal(_hello2, b) }) - removeAllChildren(tmpRoot1, nil) - removeAllChildren(tmpRoot2, nil) t.Run("absolute files to relative dir", func(t *testing.T) { require := require.New(t) + tmpRoot1, err := ioutil.TempDir("/tmp", "makisu-test") + require.NoError(err) + defer os.RemoveAll(tmpRoot1) + tmpRoot2, err := ioutil.TempDir("/tmp", "makisu-test") + require.NoError(err) + defer os.RemoveAll(tmpRoot2) + srcs := []string{"/test.txt", "/test2.txt"} require.NoError(ioutil.WriteFile(filepath.Join(tmpRoot1, "test.txt"), _hello, os.ModePerm)) require.NoError(os.Chown(filepath.Join(tmpRoot1, "test.txt"), testutil.CurrUID(), testutil.CurrGID())) @@ -164,12 +179,17 @@ func TestExecuteCopyOperation(t *testing.T) { require.NoError(err) require.Equal(_hello2, b) }) - removeAllChildren(tmpRoot1, nil) - removeAllChildren(tmpRoot2, nil) t.Run("absolute dirs to relative dir", func(t *testing.T) { require := require.New(t) + tmpRoot1, err := ioutil.TempDir("/tmp", "makisu-test") + require.NoError(err) + defer os.RemoveAll(tmpRoot1) + tmpRoot2, err := ioutil.TempDir("/tmp", "makisu-test") + require.NoError(err) + defer os.RemoveAll(tmpRoot2) + srcs := []string{"/test/", "/test2/"} require.NoError(os.MkdirAll(filepath.Join(tmpRoot1, "test"), os.ModePerm)) require.NoError(os.MkdirAll(filepath.Join(tmpRoot1, "test2"), os.ModePerm)) @@ -191,12 +211,17 @@ func TestExecuteCopyOperation(t *testing.T) { require.NoError(err) require.Equal(_hello2, b) }) - removeAllChildren(tmpRoot1, nil) - removeAllChildren(tmpRoot2, nil) t.Run("absolute dir and file to relative dir", func(t *testing.T) { require := require.New(t) + tmpRoot1, err := ioutil.TempDir("/tmp", "makisu-test") + require.NoError(err) + defer os.RemoveAll(tmpRoot1) + tmpRoot2, err := ioutil.TempDir("/tmp", "makisu-test") + require.NoError(err) + defer os.RemoveAll(tmpRoot2) + srcs := []string{"/test/", "/test2.txt"} require.NoError(os.MkdirAll(filepath.Join(tmpRoot1, "test"), os.ModePerm)) require.NoError(ioutil.WriteFile(filepath.Join(tmpRoot1, "test", "test.txt"), _hello, os.ModePerm)) @@ -217,6 +242,4 @@ func TestExecuteCopyOperation(t *testing.T) { require.NoError(err) require.Equal(_hello2, b) }) - removeAllChildren(tmpRoot1, nil) - removeAllChildren(tmpRoot2, nil) } From 1ff7da88d954bea123e877dcf84480c9a3766e4d Mon Sep 17 00:00:00 2001 From: Yiran Wang Date: Wed, 11 Mar 2020 13:35:01 -0700 Subject: [PATCH 3/3] Add test --- lib/snapshot/copy_op.go | 18 +++++++-------- lib/snapshot/copy_op_test.go | 42 ++++++++++++++++++++++++++++++++++ lib/snapshot/testutils_test.go | 6 ++++- 3 files changed, 56 insertions(+), 10 deletions(-) diff --git a/lib/snapshot/copy_op.go b/lib/snapshot/copy_op.go index 2b3dc2b5..52caa972 100644 --- a/lib/snapshot/copy_op.go +++ b/lib/snapshot/copy_op.go @@ -103,18 +103,18 @@ func (c *CopyOperation) Execute() error { } defer reader.Close() - dstFi, err := os.Stat(c.dst) - if os.IsNotExist(err) { - if err := os.MkdirAll(c.dst, 0755); err != nil { - return fmt.Errorf("create untar directory: %s", err) - } + if err := os.MkdirAll(c.dst, 0755); err != nil { + return fmt.Errorf("create/validate untar dst: %s", err) } - if !dstFi.IsDir() { - return fmt.Errorf("target untar path exists and is not directory: %s", c.dst) + + gzipReader, err := tario.NewGzipReader(reader) + if err != nil { + return fmt.Errorf("unzip gz %s: %s", src, err) } - if err := tario.Untar(reader, c.dst); err != nil { - return fmt.Errorf("untar tar: %s", err) + if err := tario.Untar(gzipReader, c.dst); err != nil { + return fmt.Errorf("untar %s to dst %s: %s", src, c.dst, err) } + return nil } var copier fileio.Copier diff --git a/lib/snapshot/copy_op_test.go b/lib/snapshot/copy_op_test.go index fac71cee..791e0a16 100644 --- a/lib/snapshot/copy_op_test.go +++ b/lib/snapshot/copy_op_test.go @@ -15,13 +15,16 @@ package snapshot import ( + "archive/tar" "fmt" "io/ioutil" "os" + "path" "path/filepath" "testing" "github.com/uber/makisu/lib/pathutils" + "github.com/uber/makisu/lib/tario" "github.com/uber/makisu/lib/utils/testutil" "github.com/stretchr/testify/require" @@ -242,4 +245,43 @@ func TestExecuteCopyOperation(t *testing.T) { require.NoError(err) require.Equal(_hello2, b) }) + + t.Run("untar gz if created from add step", func(t *testing.T) { + require := require.New(t) + + tmpDir, err := ioutil.TempDir("/tmp", "makisu-test") + require.NoError(err) + defer os.RemoveAll(tmpDir) + srcRoot, err := ioutil.TempDir("/tmp", "makisu-test") + require.NoError(err) + defer os.RemoveAll(srcRoot) + workDir, err := ioutil.TempDir("/tmp", "makisu-test") + require.NoError(err) + defer os.RemoveAll(workDir) + dst := "test_dst/" // dst is not absolute and will be placed under workdir. + + require.NoError(os.MkdirAll(filepath.Join(tmpDir, "test"), os.ModePerm)) + require.NoError(ioutil.WriteFile(filepath.Join(tmpDir, "test", "test.txt"), _hello, os.ModePerm)) + require.NoError(os.Chown(filepath.Join(tmpDir, "test", "test.txt"), testutil.CurrUID(), testutil.CurrGID())) + + tempGzipTar, err := os.Create(path.Join(srcRoot, "test_add.tar.gz")) + require.NoError(err) + gzipper, err := tario.NewGzipWriter(tempGzipTar) + require.NoError(err) + tarWriter := tar.NewWriter(gzipper) + require.NoError(writeTarHelper(tmpDir, tarWriter)) + tarWriter.Close() + gzipper.Close() + tempGzipTar.Close() + + srcs := []string{"test_add.tar.gz"} + c, err := NewCopyOperation( + srcs, srcRoot, workDir, dst, validChown, false, false, pathutils.DefaultBlacklist, true) + require.NoError(err) + require.NoError(c.Execute()) + + b, err := ioutil.ReadFile(filepath.Join(workDir, dst, "test", "test.txt")) + require.NoError(err) + require.Equal(_hello, b) + }) } diff --git a/lib/snapshot/testutils_test.go b/lib/snapshot/testutils_test.go index cdf65062..49ce7d33 100644 --- a/lib/snapshot/testutils_test.go +++ b/lib/snapshot/testutils_test.go @@ -237,7 +237,7 @@ func findNode(fs *MemFS, p string, followSymlink bool, depth int) (*memFSNode, e return nil, os.ErrNotExist } -func writeTarHelper(m *memFile, srcRoot string, w *tar.Writer) error { +func writeTarHelper(srcRoot string, w *tar.Writer) error { return filepath.Walk(srcRoot, func(p string, fi os.FileInfo, err error) error { if err != nil { return err @@ -250,6 +250,10 @@ func writeTarHelper(m *memFile, srcRoot string, w *tar.Writer) error { return err } + if h.Name, err = filepath.Rel(srcRoot, p); err != nil { + return err + } + return tario.WriteEntry(w, p, h) }) }