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 moby#11047. Signed-off-by: Ahmet Alp Balkan <[email protected]>
sjhx · Mar 4, 2015 · 1a22418 · 1a22418
1 parent 5929f40
commit 1a22418
Show file tree

Hide file tree

Showing 8 changed files with 76 additions and 12 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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)
+		}
+	}
+}