From c75663b197c972ff65999db79c36eae6f622b91d Mon Sep 17 00:00:00 2001 From: jockeych Date: Mon, 6 Jan 2020 13:07:45 -0800 Subject: [PATCH] enable archive flag (#297) * enable archive flag * address comments and add an integration test * address lint issue and check file permission --- .DS_Store | Bin 0 -> 6148 bytes lib/builder/step/add_copy_step.go | 24 +++---- lib/builder/step/add_copy_step_test.go | 8 +-- lib/builder/step/add_step.go | 4 +- lib/builder/step/copy_step.go | 4 +- lib/builder/step/copy_step_test.go | 26 ++++---- lib/builder/step/fixtures.go | 16 ++--- lib/builder/step/fixtures_test.go | 16 ++--- lib/builder/step/step.go | 4 +- lib/fileio/copy.go | 3 + lib/parser/dockerfile/add_copy.go | 48 +++++++++++--- lib/parser/dockerfile/add_test.go | 42 ++++++------ lib/parser/dockerfile/common.go | 10 ++- lib/parser/dockerfile/copy.go | 4 +- lib/parser/dockerfile/copy_test.go | 60 ++++++++++-------- lib/parser/dockerfile/fixtures.go | 2 + lib/parser/dockerfile/healthcheck.go | 8 +-- lib/parser/dockerfile/parse_file_test.go | 2 + lib/snapshot/copy_op.go | 60 ++++++++++++------ lib/snapshot/copy_op_test.go | 20 +++--- lib/snapshot/mem_fs.go | 8 +-- lib/snapshot/mem_fs_test.go | 16 ++--- test/python/test_build.py | 9 +++ .../build-context/copy-archive/Dockerfile | 11 ++++ 24 files changed, 250 insertions(+), 155 deletions(-) create mode 100644 .DS_Store create mode 100644 testdata/build-context/copy-archive/Dockerfile diff --git a/.DS_Store b/.DS_Store new file mode 100644 index 0000000000000000000000000000000000000000..4abe6ef7924e2029f6741da260ee5eac580db3c5 GIT binary patch literal 6148 zcmeHK%TC-d6g}>QA)RjM0+nT!-GJ0}Hz}Q2AeAoQ2RbjRhztQ{s9={(|DnIoKkJ?k zfOD@+WjiU$s;YcP_D!5~uYHakdjh~5-Ynh$I{9ed$gcIl1g*kFBB zH}hHD%sS}QkQ7J?{A&uxyF0`bYyOr9-(T4j-m|=YP}89gFhhk4?=2V9&j>Zb7!Bq) zXU=P!F`~vYj4^MbnAd$VIN;c7TuPsv4Bw*+85Y{BzXx%2=03vZ9MvuJu9%nO3ihlg z&ObcPLwsf41L7MjLKe!r54*+H!pcW&;}eeX5fgmIDJBtGxcTTD z$7CkD_o^=QX>yO-h^)?^bR)+*-q&Ft!ZC>1QYB<|W2HH#0rDkIIPMRe*<*T9Fw)PE zZ4j}`^|Hhj)vNSs#k&hxm)Q44x#9e6!codB2bs1RGiQ#9v)sF`&8??HMs!77v`UqG zh$DvBxOZXh<8sL*IF5SCdK!&Zc>DM*VjBMHj;6xPM-K5#HIcP`9cIL{QVyW zrMsj+QsBQ*z+~go@gXU>v$aD`&f0?I3yYZGTI)50l{=30LXP4+7BxOA... +// ADD/COPY [--archive] ["",... ""] // ADD/COPY [--chown=:] ["",... ""] // ADD/COPY [--chown=:] ... func newAddCopyDirective(base *baseDirective, args []string) (*addCopyDirective, error) { @@ -33,11 +39,34 @@ func newAddCopyDirective(base *baseDirective, args []string) (*addCopyDirective, return nil, base.err(errMissingArgs) } + // Check the flag numbers here since we only allow zero or one flag here. + var chownCount, archiveCount int var chown string - if val, ok, err := parseFlag(args[0], "chown"); err != nil { - return nil, base.err(err) - } else if ok { - chown = val + var preserveOwner bool + for _, arg := range args[:len(args)-1] { + if strings.HasPrefix(arg, "--chown") { + if val, ok, err := parseStringFlag(arg, "chown"); err != nil { + return nil, base.err(err) + } else if ok { + chown = val + chownCount++ + continue + } + } + + if strings.HasPrefix(arg, "--archive") { + if err := parseBoolFlag(arg, "archive"); err == nil { + archiveCount++ + preserveOwner = true + } else { + return nil, fmt.Errorf("archive flag format is wrong") + } + } + } + + if archiveCount+chownCount >= 2 { + return nil, base.err(fmt.Errorf("argument shouldn't contain more than one flag [--chown or --archive]")) + } else if archiveCount+chownCount == 1 { args = args[1:] } @@ -52,6 +81,5 @@ func newAddCopyDirective(base *baseDirective, args []string) (*addCopyDirective, } srcs := parsed[:len(parsed)-1] dst := parsed[len(parsed)-1] - - return &addCopyDirective{base, chown, srcs, dst}, nil + return &addCopyDirective{base, chown, preserveOwner, srcs, dst}, nil } diff --git a/lib/parser/dockerfile/add_test.go b/lib/parser/dockerfile/add_test.go index 37dbc46d..d2d253c3 100644 --- a/lib/parser/dockerfile/add_test.go +++ b/lib/parser/dockerfile/add_test.go @@ -25,26 +25,30 @@ func TestNewAddDirective(t *testing.T) { buildState.stageVars = map[string]string{"prefix": "test_", "suffix": "_test", "comma": ","} tests := []struct { - desc string - succeed bool - input string - srcs []string - dst string - chown string + desc string + succeed bool + input string + srcs []string + dst string + chown string + preserveOwner bool }{ - {"shell single source", true, `add src dst`, []string{"src"}, "dst", ""}, - {"shell multi source", true, `add src1 src2 dst`, []string{"src1", "src2"}, "dst", ""}, - {"shell substitution", true, `add src1 ${prefix}src2 dst$suffix`, []string{"src1", "test_src2"}, "dst_test", ""}, - {"shell substitution bad", false, "add src1 ${prefix", nil, "", ""}, - {"shell chown", true, `add --chown=user:group src dst`, []string{"src"}, "dst", "user:group"}, - {"shell chown bad", false, `add --chown= src dst`, nil, "", ""}, - {"shell chown substitution", true, `add --chown=${prefix}user:group src1 ${prefix}src2 dst$suffix`, []string{"src1", "test_src2"}, "dst_test", "test_user:group"}, - {"json bad", false, `add ["src"]`, nil, "", ""}, - {"json single source", true, `add ["src", "dst"]`, []string{"src"}, "dst", ""}, - {"json multi source", true, `add ["src1", "src2", "dst"]`, []string{"src1", "src2"}, "dst", ""}, - {"json substitution", true, `add ["src1"$comma "src2${suffix}", "${prefix}dst"]`, []string{"src1", "src2_test"}, "test_dst", ""}, - {"json chown", true, `add --chown=user:group ["src", "dst"]`, []string{"src"}, "dst", "user:group"}, - {"json chown substitution", true, `add --chown=${prefix}user:group ["src1"$comma "src2${suffix}", "${prefix}dst"]`, []string{"src1", "src2_test"}, "test_dst", "test_user:group"}, + {"shell single source", true, `add src dst`, []string{"src"}, "dst", "", false}, + {"shell multi source", true, `add src1 src2 dst`, []string{"src1", "src2"}, "dst", "", false}, + {"shell substitution", true, `add src1 ${prefix}src2 dst$suffix`, []string{"src1", "test_src2"}, "dst_test", "", false}, + {"shell substitution bad", false, "add src1 ${prefix", nil, "", "", false}, + {"shell chown", true, `add --chown=user:group src dst`, []string{"src"}, "dst", "user:group", false}, + {"shell chown bad", false, `add --chown= src dst`, nil, "", "", false}, + {"shell chown substitution", true, `add --chown=${prefix}user:group src1 ${prefix}src2 dst$suffix`, []string{"src1", "test_src2"}, "dst_test", "test_user:group", false}, + {"json bad", false, `add ["src"]`, nil, "", "", false}, + {"json single source", true, `add ["src", "dst"]`, []string{"src"}, "dst", "", false}, + {"json multi source", true, `add ["src1", "src2", "dst"]`, []string{"src1", "src2"}, "dst", "", false}, + {"json substitution", true, `add ["src1"$comma "src2${suffix}", "${prefix}dst"]`, []string{"src1", "src2_test"}, "test_dst", "", false}, + {"json chown", true, `add --chown=user:group ["src", "dst"]`, []string{"src"}, "dst", "user:group", false}, + {"json chown substitution", true, `add --chown=${prefix}user:group ["src1"$comma "src2${suffix}", "${prefix}dst"]`, []string{"src1", "src2_test"}, "test_dst", "test_user:group", false}, + {"json multiple flags", false, `add --chown=user:group --archive ["src", "dst"]`, []string{"src"}, "dst", "user:group", true}, + {"json archive", true, `add --archive ["src", "dst"]`, []string{"src"}, "dst", "", true}, + {"json archive bad", false, `add --archive=ss ["src", "dst"]`, []string{"src"}, "dst", "", true}, } for _, test := range tests { diff --git a/lib/parser/dockerfile/common.go b/lib/parser/dockerfile/common.go index 685e0e78..a78359ab 100644 --- a/lib/parser/dockerfile/common.go +++ b/lib/parser/dockerfile/common.go @@ -32,7 +32,15 @@ var ( errUnsupportedDirective = errors.New("Unsupported directive type") ) -func parseFlag(s string, name string) (string, bool, error) { +func parseBoolFlag(s string, name string) error { + flag := "--" + name + if !strings.EqualFold(s, flag) { + return fmt.Errorf("Wrong flag format for %s", flag) + } + return nil +} + +func parseStringFlag(s string, name string) (string, bool, error) { flag := "--" + name + "=" if !strings.HasPrefix(s, flag) { return "", false, nil diff --git a/lib/parser/dockerfile/copy.go b/lib/parser/dockerfile/copy.go index af1c9acf..fd3c4e17 100644 --- a/lib/parser/dockerfile/copy.go +++ b/lib/parser/dockerfile/copy.go @@ -39,13 +39,13 @@ func newCopyDirective(base *baseDirective, state *parsingState) (Directive, erro } var fromStage string - if val, ok, err := parseFlag(args[0], "from"); err != nil { + if val, ok, err := parseStringFlag(args[0], "from"); err != nil { return nil, base.err(err) } else if ok { fromStage = val args = args[1:] } else if len(args) >= 3 { - if val, ok, err := parseFlag(args[1], "from"); err != nil { + if val, ok, err := parseStringFlag(args[1], "from"); err != nil { return nil, base.err(err) } else if ok { fromStage = val diff --git a/lib/parser/dockerfile/copy_test.go b/lib/parser/dockerfile/copy_test.go index c4869332..c17f5406 100644 --- a/lib/parser/dockerfile/copy_test.go +++ b/lib/parser/dockerfile/copy_test.go @@ -25,35 +25,39 @@ func TestNewCopyDirective(t *testing.T) { buildState.stageVars = map[string]string{"prefix": "test_", "suffix": "_test", "comma": ","} tests := []struct { - desc string - succeed bool - input string - srcs []string - dst string - fromStage string - chown string + desc string + succeed bool + input string + srcs []string + dst string + fromStage string + chown string + preserverOwner bool }{ - {"missing args", false, "copy ", nil, "", "", ""}, - {"missing args after replace", false, "copy \\", nil, "", "", ""}, - {"shell single source", true, `copy src dst`, []string{"src"}, "dst", "", ""}, - {"shell multi source", true, `copy src1 src2 dst`, []string{"src1", "src2"}, "dst", "", ""}, - {"shell substitution", true, `copy src1 ${prefix}src2 dst$suffix`, []string{"src1", "test_src2"}, "dst_test", "", ""}, - {"shell substitution bad", false, "copy src1 ${prefix", nil, "", "", ""}, - {"shell from", true, `copy --from=stage src dst`, []string{"src"}, "dst", "stage", ""}, - {"shell from bad", false, `copy --from= src dst`, nil, "", "", ""}, - {"shell chown", true, `copy --chown=user:group src dst`, []string{"src"}, "dst", "", "user:group"}, - {"shell from chown", true, `copy --chown=user:group --from=stage src dst`, []string{"src"}, "dst", "stage", "user:group"}, - {"shell from chown bad", false, `copy --chown=user:group --from= src dst`, nil, "", "", ""}, - {"shell from chown substitution", true, `copy --chown=${prefix}user:group --from=${prefix}stage src1 ${prefix}src2 dst$suffix`, []string{"src1", "test_src2"}, "dst_test", "test_stage", "test_user:group"}, - {"json single source", true, `copy ["src", "dst"]`, []string{"src"}, "dst", "", ""}, - {"json multi source", true, `copy ["src1", "src2", "dst"]`, []string{"src1", "src2"}, "dst", "", ""}, - {"json substitution", true, `copy ["src1"$comma "src2${suffix}", "${prefix}dst"]`, []string{"src1", "src2_test"}, "test_dst", "", ""}, - {"json from", true, `copy --from=stage ["src", "dst"]`, []string{"src"}, "dst", "stage", ""}, - {"json from missing args", false, "copy --from=stage", nil, "", "", ""}, - {"json from substitution", true, `copy --from=${prefix}stage ["src1"$comma "src2${suffix}", "${prefix}dst"]`, []string{"src1", "src2_test"}, "test_dst", "test_stage", ""}, - {"json chown", true, `copy --chown=user:group ["src", "dst"]`, []string{"src"}, "dst", "", "user:group"}, - {"json from chown", true, `copy --chown=user:group --from=stage ["src", "dst"]`, []string{"src"}, "dst", "stage", "user:group"}, - {"json from chown substitution", true, `copy --chown=${prefix}user:group --from=${prefix}stage ["src1"$comma "src2${suffix}", "${prefix}dst"]`, []string{"src1", "src2_test"}, "test_dst", "test_stage", "test_user:group"}, + {"missing args", false, "copy ", nil, "", "", "", false}, + {"missing args after replace", false, "copy \\", nil, "", "", "", false}, + {"shell single source", true, `copy src dst`, []string{"src"}, "dst", "", "", false}, + {"shell multi source", true, `copy src1 src2 dst`, []string{"src1", "src2"}, "dst", "", "", false}, + {"shell substitution", true, `copy src1 ${prefix}src2 dst$suffix`, []string{"src1", "test_src2"}, "dst_test", "", "", false}, + {"shell substitution bad", false, "copy src1 ${prefix", nil, "", "", "", false}, + {"shell from", true, `copy --from=stage src dst`, []string{"src"}, "dst", "stage", "", false}, + {"shell from bad", false, `copy --from= src dst`, nil, "", "", "", false}, + {"shell chown", true, `copy --chown=user:group src dst`, []string{"src"}, "dst", "", "user:group", false}, + {"shell from chown", true, `copy --chown=user:group --from=stage src dst`, []string{"src"}, "dst", "stage", "user:group", false}, + {"shell from chown bad", false, `copy --chown=user:group --from= src dst`, nil, "", "", "", false}, + {"shell from chown substitution", true, `copy --chown=${prefix}user:group --from=${prefix}stage src1 ${prefix}src2 dst$suffix`, []string{"src1", "test_src2"}, "dst_test", "test_stage", "test_user:group", false}, + {"json single source", true, `copy ["src", "dst"]`, []string{"src"}, "dst", "", "", false}, + {"json multi source", true, `copy ["src1", "src2", "dst"]`, []string{"src1", "src2"}, "dst", "", "", false}, + {"json substitution", true, `copy ["src1"$comma "src2${suffix}", "${prefix}dst"]`, []string{"src1", "src2_test"}, "test_dst", "", "", false}, + {"json from", true, `copy --from=stage ["src", "dst"]`, []string{"src"}, "dst", "stage", "", false}, + {"json from missing args", false, "copy --from=stage", nil, "", "", "", false}, + {"json from substitution", true, `copy --from=${prefix}stage ["src1"$comma "src2${suffix}", "${prefix}dst"]`, []string{"src1", "src2_test"}, "test_dst", "test_stage", "", false}, + {"json chown", true, `copy --chown=user:group ["src", "dst"]`, []string{"src"}, "dst", "", "user:group", false}, + {"json from chown", true, `copy --chown=user:group --from=stage ["src", "dst"]`, []string{"src"}, "dst", "stage", "user:group", false}, + {"json from chown substitution", true, `copy --chown=${prefix}user:group --from=${prefix}stage ["src1"$comma "src2${suffix}", "${prefix}dst"]`, []string{"src1", "src2_test"}, "test_dst", "test_stage", "test_user:group", false}, + {"json multiple flags", false, `copy --chown=user:group --archive ["src", "dst"]`, []string{"src"}, "dst", "", "user:group", true}, + {"json archive", true, `copy --archive ["src", "dst"]`, []string{"src"}, "dst", "", "", true}, + {"json archive bad", false, `copy --archive==bad ["src", "dst"]`, []string{"src"}, "dst", "", "", true}, } for _, test := range tests { diff --git a/lib/parser/dockerfile/fixtures.go b/lib/parser/dockerfile/fixtures.go index fb3c2919..8622cdab 100644 --- a/lib/parser/dockerfile/fixtures.go +++ b/lib/parser/dockerfile/fixtures.go @@ -51,6 +51,7 @@ func CopyDirectiveFixture(args, chown, fromStage string, srcs []string, dst stri &addCopyDirective{ &baseDirective{"copy", args, false}, chown, + false, srcs, dst, }, @@ -89,6 +90,7 @@ func AddDirectiveFixture(args, chown string, srcs []string, dst string) *AddDire &addCopyDirective{ &baseDirective{"add", args, false}, chown, + false, srcs, dst, }, diff --git a/lib/parser/dockerfile/healthcheck.go b/lib/parser/dockerfile/healthcheck.go index 04185d55..19476caf 100644 --- a/lib/parser/dockerfile/healthcheck.go +++ b/lib/parser/dockerfile/healthcheck.go @@ -62,7 +62,7 @@ func newHealthcheckDirective(base *baseDirective, state *parsingState) (Directiv var interval, timeout, startPeriod time.Duration var retries int for _, flag := range flags { - if val, ok, err := parseFlag(flag, "interval"); err != nil { + if val, ok, err := parseStringFlag(flag, "interval"); err != nil { return nil, base.err(err) } else if ok { interval, err = time.ParseDuration(val) @@ -72,7 +72,7 @@ func newHealthcheckDirective(base *baseDirective, state *parsingState) (Directiv continue } - if val, ok, err := parseFlag(flag, "timeout"); err != nil { + if val, ok, err := parseStringFlag(flag, "timeout"); err != nil { return nil, base.err(err) } else if ok { timeout, err = time.ParseDuration(val) @@ -82,7 +82,7 @@ func newHealthcheckDirective(base *baseDirective, state *parsingState) (Directiv continue } - if val, ok, err := parseFlag(flag, "start-period"); err != nil { + if val, ok, err := parseStringFlag(flag, "start-period"); err != nil { return nil, base.err(err) } else if ok { startPeriod, err = time.ParseDuration(val) @@ -92,7 +92,7 @@ func newHealthcheckDirective(base *baseDirective, state *parsingState) (Directiv continue } - if val, ok, err := parseFlag(flag, "retries"); err != nil { + if val, ok, err := parseStringFlag(flag, "retries"); err != nil { return nil, base.err(err) } else if ok { retries, err = strconv.Atoi(val) diff --git a/lib/parser/dockerfile/parse_file_test.go b/lib/parser/dockerfile/parse_file_test.go index 09d61b5e..3dcccc03 100644 --- a/lib/parser/dockerfile/parse_file_test.go +++ b/lib/parser/dockerfile/parse_file_test.go @@ -610,6 +610,7 @@ func integration() []*test { &addCopyDirective{ &baseDirective{"copy", "--from=digest --chown=user:group src1 src2 src3 dst/", true}, "user:group", + false, []string{"src1", "src2", "src3"}, "dst/", }, @@ -633,6 +634,7 @@ func integration() []*test { &addCopyDirective{ &baseDirective{"add", `--chown=user:group ["src1", "src2", "src3", "dst/"]`, true}, "user:group", + false, []string{"src1", "src2", "src3"}, "dst/", }, diff --git a/lib/snapshot/copy_op.go b/lib/snapshot/copy_op.go index 26ef53e5..06aef3f3 100644 --- a/lib/snapshot/copy_op.go +++ b/lib/snapshot/copy_op.go @@ -27,11 +27,12 @@ import ( // CopyOperation defines a copy operation that occurred to generate a layer from. type CopyOperation struct { - srcRoot string - srcs []string - dst string - uid int - gid int + srcRoot string + srcs []string + dst string + uid int + gid int + preserveOwner bool blacklist []string // Indicates if the copy op is used for copying from previous stages. @@ -42,7 +43,7 @@ type CopyOperation struct { // specify if the copy op is used for copying from previous stages. func NewCopyOperation( srcs []string, srcRoot, workDir, dst, chown string, - blacklist []string, internal bool) (*CopyOperation, error) { + blacklist []string, internal, preserveOwner bool) (*CopyOperation, error) { if err := checkCopyParams(srcs, workDir, dst); err != nil { return nil, fmt.Errorf("check copy param: %s", err) @@ -61,13 +62,14 @@ func NewCopyOperation( dst = resolveDestination(workDir, dst) return &CopyOperation{ - srcRoot: srcRoot, - srcs: relSources, - dst: dst, - uid: uid, - gid: gid, - blacklist: blacklist, - internal: internal, + srcRoot: srcRoot, + srcs: relSources, + dst: dst, + uid: uid, + gid: gid, + preserveOwner: preserveOwner, + blacklist: blacklist, + internal: internal, }, nil } @@ -92,20 +94,40 @@ func (c *CopyOperation) Execute() error { } if fi.IsDir() { // Dir to dir - if err := copier.CopyDir(src, c.dst, c.uid, c.gid); err != nil { - return fmt.Errorf("copy dir %s to dir %s: %s", src, c.dst, err) + if c.preserveOwner { + if err := copier.CopyDirPreserveOwner(src, c.dst); err != nil { + return fmt.Errorf("copy dir %s to dir %s: %s", src, c.dst, err) + } + } else { + if err := copier.CopyDir(src, c.dst, c.uid, c.gid); err != nil { + return fmt.Errorf("copy dir %s to dir %s: %s", src, c.dst, err) + } } } else if isDirFormat(c.dst) { // File to dir targetFilePath := filepath.Join(c.dst, filepath.Base(src)) - if err := copier.CopyFile(src, targetFilePath, c.uid, c.gid); err != nil { - return fmt.Errorf("copy file %s to dir %s: %s", src, targetFilePath, err) + if c.preserveOwner { + if err := copier.CopyFilePreserveOwner(src, targetFilePath); err != nil { + return fmt.Errorf("copy file %s to dir %s: %s", src, targetFilePath, err) + } + } else { + if err := copier.CopyFile(src, targetFilePath, c.uid, c.gid); err != nil { + return fmt.Errorf("copy file %s to dir %s: %s", src, targetFilePath, err) + } } + } else { // File to file - if err := copier.CopyFile(src, c.dst, c.uid, c.gid); err != nil { - return fmt.Errorf("copy file %s to file %s: %s", src, c.dst, err) + if c.preserveOwner { + if err := copier.CopyFilePreserveOwner(src, c.dst); err != nil { + return fmt.Errorf("copy file %s to dir %s: %s", src, c.dst, err) + } + } else { + if err := copier.CopyFile(src, c.dst, c.uid, c.gid); err != nil { + return fmt.Errorf("copy file %s to file %s: %s", src, c.dst, err) + } } + } } return nil diff --git a/lib/snapshot/copy_op_test.go b/lib/snapshot/copy_op_test.go index 322922a2..e82d1dcc 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) + srcs, srcRoot, workDir, dst, validChown, pathutils.DefaultBlacklist, false, false) require.Error(err) srcs = []string{"file", "dir/"} workDir = "" dst = "/target/test" _, err = NewCopyOperation( - srcs, srcRoot, workDir, dst, validChown, pathutils.DefaultBlacklist, false) + srcs, srcRoot, workDir, dst, validChown, pathutils.DefaultBlacklist, false, false) require.Error(err) srcs = []string{"file", "dir/"} workDir = "" dst = "target/test" _, err = NewCopyOperation( - srcs, srcRoot, workDir, dst, validChown, pathutils.DefaultBlacklist, false) + srcs, srcRoot, workDir, dst, validChown, pathutils.DefaultBlacklist, false, false) require.Error(err) srcs = []string{"file", "dir/"} workDir = "wrk/" dst = "target/test/" _, err = NewCopyOperation( - srcs, srcRoot, workDir, dst, validChown, pathutils.DefaultBlacklist, false) + srcs, srcRoot, workDir, dst, validChown, pathutils.DefaultBlacklist, false, 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) + srcs, srcRoot, "", dst, validChown, pathutils.DefaultBlacklist, false, true) 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) + srcs, srcRoot, workDir, dst, validChown, pathutils.DefaultBlacklist, false, 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) + srcs, srcRoot, workDir, dst, validChown, pathutils.DefaultBlacklist, false, 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) + srcs, srcRoot, workDir, dst, validChown, pathutils.DefaultBlacklist, false, true) 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) + srcs, srcRoot, workDir, dst, validChown, pathutils.DefaultBlacklist, false, 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) + srcs, srcRoot, workDir, dst, validChown, pathutils.DefaultBlacklist, false, 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.go b/lib/snapshot/mem_fs.go index 0bd23cd1..2b595b3c 100644 --- a/lib/snapshot/mem_fs.go +++ b/lib/snapshot/mem_fs.go @@ -24,13 +24,12 @@ import ( "syscall" "time" + "github.com/andres-erbsen/clock" "github.com/uber/makisu/lib/fileio" "github.com/uber/makisu/lib/log" "github.com/uber/makisu/lib/mountutils" "github.com/uber/makisu/lib/pathutils" "github.com/uber/makisu/lib/tario" - - "github.com/andres-erbsen/clock" ) // memFSNode represents one node of the directory tree in the merged fs view. @@ -116,12 +115,13 @@ func (fs *MemFS) Checkpoint(newRoot string, sources []string) error { if err != nil { return fmt.Errorf("stat %s: %s", src, err) } + if sourceInfo.IsDir() { - if err := copier.CopyDir(src, dst, 0, 0); err != nil { + if err := copier.CopyDirPreserveOwner(src, dst); err != nil { return fmt.Errorf("copy dir %s: %s", src, err) } } else { - if err := copier.CopyFile(src, dst, 0, 0); err != nil { + if err := copier.CopyFilePreserveOwner(src, dst); err != nil { return fmt.Errorf("copy file %s: %s", src, err) } } diff --git a/lib/snapshot/mem_fs_test.go b/lib/snapshot/mem_fs_test.go index b285ccd3..42d84a26 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) + srcs, srcRoot, workDir, dst, validChown, pathutils.DefaultBlacklist, false, 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) + srcs, srcRoot, workDir, dst, validChown, pathutils.DefaultBlacklist, false, 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) + srcs, srcRoot, workDir, dst, validChown, pathutils.DefaultBlacklist, false, 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) + srcs, srcRoot, workDir, dst, validChown, pathutils.DefaultBlacklist, false, 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) + srcs, srcRoot, workDir, dst, validChown, pathutils.DefaultBlacklist, false, 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) + srcs, srcRoot, workDir, dst, validChown, pathutils.DefaultBlacklist, false, 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) + srcs, srcRoot, workDir, dst, validChown, pathutils.DefaultBlacklist, false, 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) + srcs, srcRoot, workDir, dst, validChown, pathutils.DefaultBlacklist, false, false) require.NoError(err) err = fs1.AddLayerByCopyOps([]*CopyOperation{c}, w1) require.NoError(err) diff --git a/test/python/test_build.py b/test/python/test_build.py index 2c49e6ff..6fa386d8 100644 --- a/test/python/test_build.py +++ b/test/python/test_build.py @@ -70,6 +70,15 @@ def test_build_copy_add_chown(registry1, storage_dir): code, err = utils.docker_run_image(registry1.addr, new_image) assert code == 0, err +def test_build_copy_archive(registry1, storage_dir): + new_image = new_image_name() + context_dir = os.path.join( + os.getcwd(), 'testdata/build-context/copy-archive') + utils.makisu_build_image( + new_image, context_dir, storage_dir, registry=registry1.addr) + code, err = utils.docker_run_image(registry1.addr, new_image) + assert code == 0, err + def test_build_arg_and_env(registry1, storage_dir): new_image = new_image_name() context_dir = os.path.join( diff --git a/testdata/build-context/copy-archive/Dockerfile b/testdata/build-context/copy-archive/Dockerfile new file mode 100644 index 00000000..27c35b5d --- /dev/null +++ b/testdata/build-context/copy-archive/Dockerfile @@ -0,0 +1,11 @@ +FROM alpine as phaseA +RUN mkdir /hello +RUN mkdir /hello/world +RUN touch /hello/world/a.txt +RUN chmod 775 /hello/world/a.txt +RUN chown daemon:daemon /hello/world + +FROM alpine +RUN mkdir /hello +COPY --from=phaseA --archive /hello /hello +ENTRYPOINT ["/bin/sh", "-c", "test $(stat -c '%U:%G' /hello) = 'root:root' && test $(stat -c '%U:%G' /hello/world) = 'daemon:daemon' && test $(stat -c '%U:%G' /hello/world/a.txt) = 'root:root' && test $(stat -c '%a' /hello/world/a.txt) = 775 "]