From 1a22418f9f1573ca8521831d52c8e0562cb3ef8f Mon Sep 17 00:00:00 2001 From: Ahmet Alp Balkan Date: Tue, 3 Mar 2015 18:40:16 -0800 Subject: [PATCH] pkg/archive: adjust chmod bits on windows This change modifies the chmod bits of build context archives built on windows to preserve the execute bit and remove the r/w bits from grp/others. Also adjusted integ-cli tests to verify permissions based on the platform the tests are running. Fixes #11047. Signed-off-by: Ahmet Alp Balkan --- integration-cli/docker_cli_build_test.go | 24 ++++++++++++------------ integration-cli/test_vars_unix.go | 2 ++ integration-cli/test_vars_windows.go | 3 +++ pkg/archive/archive.go | 3 +++ pkg/archive/archive_unix.go | 8 ++++++++ pkg/archive/archive_unix_test.go | 18 ++++++++++++++++++ pkg/archive/archive_windows.go | 12 ++++++++++++ pkg/archive/archive_windows_test.go | 18 ++++++++++++++++++ 8 files changed, 76 insertions(+), 12 deletions(-) diff --git a/integration-cli/docker_cli_build_test.go b/integration-cli/docker_cli_build_test.go index 00183c067177d..cd653ddcef03b 100644 --- a/integration-cli/docker_cli_build_test.go +++ b/integration-cli/docker_cli_build_test.go @@ -690,15 +690,15 @@ func TestBuildSixtySteps(t *testing.T) { func TestBuildAddSingleFileToRoot(t *testing.T) { name := "testaddimg" defer deleteImages(name) - ctx, err := fakeContext(`FROM busybox + ctx, err := fakeContext(fmt.Sprintf(`FROM busybox RUN echo 'dockerio:x:1001:1001::/bin:/bin/false' >> /etc/passwd RUN echo 'dockerio:x:1001:' >> /etc/group RUN touch /exists RUN chown dockerio.dockerio /exists ADD test_file / RUN [ $(ls -l /test_file | awk '{print $3":"$4}') = 'root:root' ] -RUN [ $(ls -l /test_file | awk '{print $1}') = '-rw-r--r--' ] -RUN [ $(ls -l /exists | awk '{print $3":"$4}') = 'dockerio:dockerio' ]`, +RUN [ $(ls -l /test_file | awk '{print $1}') = '%s' ] +RUN [ $(ls -l /exists | awk '{print $3":"$4}') = 'dockerio:dockerio' ]`, expectedFileChmod), map[string]string{ "test_file": "test1", }) @@ -1263,7 +1263,7 @@ RUN [ $(ls -l /exists/test_file | awk '{print $3":"$4}') = 'root:root' ]`, func TestBuildAddWholeDirToRoot(t *testing.T) { name := "testaddwholedirtoroot" defer deleteImages(name) - ctx, err := fakeContext(`FROM busybox + ctx, err := fakeContext(fmt.Sprintf(`FROM busybox RUN echo 'dockerio:x:1001:1001::/bin:/bin/false' >> /etc/passwd RUN echo 'dockerio:x:1001:' >> /etc/group RUN touch /exists @@ -1272,8 +1272,8 @@ ADD test_dir /test_dir RUN [ $(ls -l / | grep test_dir | awk '{print $3":"$4}') = 'root:root' ] RUN [ $(ls -l / | grep test_dir | awk '{print $1}') = 'drwxr-xr-x' ] RUN [ $(ls -l /test_dir/test_file | awk '{print $3":"$4}') = 'root:root' ] -RUN [ $(ls -l /test_dir/test_file | awk '{print $1}') = '-rw-r--r--' ] -RUN [ $(ls -l /exists | awk '{print $3":"$4}') = 'dockerio:dockerio' ]`, +RUN [ $(ls -l /test_dir/test_file | awk '{print $1}') = '%s' ] +RUN [ $(ls -l /exists | awk '{print $3":"$4}') = 'dockerio:dockerio' ]`, expectedFileChmod), map[string]string{ "test_dir/test_file": "test1", }) @@ -1336,15 +1336,15 @@ RUN [ $(ls -l /usr/bin/suidbin | awk '{print $1}') = '-rwsr-xr-x' ]`, func TestBuildCopySingleFileToRoot(t *testing.T) { name := "testcopysinglefiletoroot" defer deleteImages(name) - ctx, err := fakeContext(`FROM busybox + ctx, err := fakeContext(fmt.Sprintf(`FROM busybox RUN echo 'dockerio:x:1001:1001::/bin:/bin/false' >> /etc/passwd RUN echo 'dockerio:x:1001:' >> /etc/group RUN touch /exists RUN chown dockerio.dockerio /exists COPY test_file / RUN [ $(ls -l /test_file | awk '{print $3":"$4}') = 'root:root' ] -RUN [ $(ls -l /test_file | awk '{print $1}') = '-rw-r--r--' ] -RUN [ $(ls -l /exists | awk '{print $3":"$4}') = 'dockerio:dockerio' ]`, +RUN [ $(ls -l /test_file | awk '{print $1}') = '%s' ] +RUN [ $(ls -l /exists | awk '{print $3":"$4}') = 'dockerio:dockerio' ]`, expectedFileChmod), map[string]string{ "test_file": "test1", }) @@ -1496,7 +1496,7 @@ RUN [ $(ls -l /exists/test_file | awk '{print $3":"$4}') = 'root:root' ]`, func TestBuildCopyWholeDirToRoot(t *testing.T) { name := "testcopywholedirtoroot" defer deleteImages(name) - ctx, err := fakeContext(`FROM busybox + ctx, err := fakeContext(fmt.Sprintf(`FROM busybox RUN echo 'dockerio:x:1001:1001::/bin:/bin/false' >> /etc/passwd RUN echo 'dockerio:x:1001:' >> /etc/group RUN touch /exists @@ -1505,8 +1505,8 @@ COPY test_dir /test_dir RUN [ $(ls -l / | grep test_dir | awk '{print $3":"$4}') = 'root:root' ] RUN [ $(ls -l / | grep test_dir | awk '{print $1}') = 'drwxr-xr-x' ] RUN [ $(ls -l /test_dir/test_file | awk '{print $3":"$4}') = 'root:root' ] -RUN [ $(ls -l /test_dir/test_file | awk '{print $1}') = '-rw-r--r--' ] -RUN [ $(ls -l /exists | awk '{print $3":"$4}') = 'dockerio:dockerio' ]`, +RUN [ $(ls -l /test_dir/test_file | awk '{print $1}') = '%s' ] +RUN [ $(ls -l /exists | awk '{print $3":"$4}') = 'dockerio:dockerio' ]`, expectedFileChmod), map[string]string{ "test_dir/test_file": "test1", }) diff --git a/integration-cli/test_vars_unix.go b/integration-cli/test_vars_unix.go index 988d3c4721c42..1ab8a5ca48700 100644 --- a/integration-cli/test_vars_unix.go +++ b/integration-cli/test_vars_unix.go @@ -5,4 +5,6 @@ package main const ( // identifies if test suite is running on a unix platform isUnixCli = true + + expectedFileChmod = "-rw-r--r--" ) diff --git a/integration-cli/test_vars_windows.go b/integration-cli/test_vars_windows.go index f9ad163981fc2..3cad4bceefb39 100644 --- a/integration-cli/test_vars_windows.go +++ b/integration-cli/test_vars_windows.go @@ -5,4 +5,7 @@ package main const ( // identifies if test suite is running on a unix platform isUnixCli = false + + // this is the expected file permission set on windows: gh#11047 + expectedFileChmod = "-rwx------" ) diff --git a/pkg/archive/archive.go b/pkg/archive/archive.go index bce66a505afdc..bfa6e18462f6f 100644 --- a/pkg/archive/archive.go +++ b/pkg/archive/archive.go @@ -204,6 +204,7 @@ func (ta *tarAppender) addTarFile(path, name string) error { if err != nil { return err } + hdr.Mode = int64(chmodTarEntry(os.FileMode(hdr.Mode))) name, err = canonicalTarName(name, fi.IsDir()) if err != nil { @@ -696,6 +697,8 @@ func (archiver *Archiver) CopyFileWithTar(src, dst string) (err error) { return err } hdr.Name = filepath.Base(dst) + hdr.Mode = int64(chmodTarEntry(os.FileMode(hdr.Mode))) + tw := tar.NewWriter(w) defer tw.Close() if err := tw.WriteHeader(hdr); err != nil { diff --git a/pkg/archive/archive_unix.go b/pkg/archive/archive_unix.go index 8c7079f85a505..cbce65e31d485 100644 --- a/pkg/archive/archive_unix.go +++ b/pkg/archive/archive_unix.go @@ -4,6 +4,7 @@ package archive import ( "errors" + "os" "syscall" "github.com/docker/docker/vendor/src/code.google.com/p/go/src/pkg/archive/tar" @@ -16,6 +17,13 @@ func CanonicalTarNameForPath(p string) (string, error) { return p, nil // already unix-style } +// chmodTarEntry is used to adjust the file permissions used in tar header based +// on the platform the archival is done. + +func chmodTarEntry(perm os.FileMode) os.FileMode { + return perm // noop for unix as golang APIs provide perm bits correctly +} + func setHeaderForSpecialDevice(hdr *tar.Header, ta *tarAppender, name string, stat interface{}) (nlink uint32, inode uint64, err error) { s, ok := stat.(*syscall.Stat_t) diff --git a/pkg/archive/archive_unix_test.go b/pkg/archive/archive_unix_test.go index 52f28e20f062b..18f45c480f10b 100644 --- a/pkg/archive/archive_unix_test.go +++ b/pkg/archive/archive_unix_test.go @@ -3,6 +3,7 @@ package archive import ( + "os" "testing" ) @@ -40,3 +41,20 @@ func TestCanonicalTarName(t *testing.T) { } } } + +func TestChmodTarEntry(t *testing.T) { + cases := []struct { + in, expected os.FileMode + }{ + {0000, 0000}, + {0777, 0777}, + {0644, 0644}, + {0755, 0755}, + {0444, 0444}, + } + for _, v := range cases { + if out := chmodTarEntry(v.in); out != v.expected { + t.Fatalf("wrong chmod. expected:%v got:%v", v.expected, out) + } + } +} diff --git a/pkg/archive/archive_windows.go b/pkg/archive/archive_windows.go index b95aa178d4808..2904b38319c60 100644 --- a/pkg/archive/archive_windows.go +++ b/pkg/archive/archive_windows.go @@ -4,6 +4,7 @@ package archive import ( "fmt" + "os" "strings" "github.com/docker/docker/vendor/src/code.google.com/p/go/src/pkg/archive/tar" @@ -23,6 +24,17 @@ func CanonicalTarNameForPath(p string) (string, error) { return strings.Replace(p, "\\", "/", -1), nil } +// chmodTarEntry is used to adjust the file permissions used in tar header based +// on the platform the archival is done. +func chmodTarEntry(perm os.FileMode) os.FileMode { + // Clear r/w on grp/others: no precise equivalen of group/others on NTFS. + perm &= 0711 + // Add the x bit: make everything +x from windows + perm |= 0100 + + return perm +} + func setHeaderForSpecialDevice(hdr *tar.Header, ta *tarAppender, name string, stat interface{}) (nlink uint32, inode uint64, err error) { // do nothing. no notion of Rdev, Inode, Nlink in stat on Windows return diff --git a/pkg/archive/archive_windows_test.go b/pkg/archive/archive_windows_test.go index 2b7993c293f9e..821a76a8623d0 100644 --- a/pkg/archive/archive_windows_test.go +++ b/pkg/archive/archive_windows_test.go @@ -3,6 +3,7 @@ package archive import ( + "os" "testing" ) @@ -46,3 +47,20 @@ func TestCanonicalTarName(t *testing.T) { } } } + +func TestChmodTarEntry(t *testing.T) { + cases := []struct { + in, expected os.FileMode + }{ + {0000, 0100}, + {0777, 0711}, + {0644, 0700}, + {0755, 0711}, + {0444, 0500}, + } + for _, v := range cases { + if out := chmodTarEntry(v.in); out != v.expected { + t.Fatalf("wrong chmod. expected:%v got:%v", v.expected, out) + } + } +}