Merge pull request #1934 from dotcloud/host-permissions

Only copy files and change permissions with non bindmount

container.go

@@ -671,9 +671,11 @@ func (container *Container) Start(hostConfig *HostConfig) error {
 			continue
 		}
 		var srcPath string
+		var isBindMount bool
 		srcRW := false
 		// If an external bind is defined for this volume, use that as a source
 		if bindMap, exists := binds[volPath]; exists {
+			isBindMount = true
 			srcPath = bindMap.SrcPath
 			if strings.ToLower(bindMap.Mode) == "rw" {
 				srcRW = true
@@ -697,7 +699,9 @@ func (container *Container) Start(hostConfig *HostConfig) error {
 		if err := os.MkdirAll(rootVolPath, 0755); err != nil {
 			return nil
 		}
-		if srcRW {
+
+		// Do not copy or change permissions if we are mounting from the host
+		if srcRW && !isBindMount {
 			volList, err := ioutil.ReadDir(rootVolPath)
 			if err != nil {
 				return err
@@ -708,22 +712,26 @@ func (container *Container) Start(hostConfig *HostConfig) error {
 					return err
 				}
 				if len(srcList) == 0 {
+					// If the source volume is empty copy files from the root into the volume
 					if err := CopyWithTar(rootVolPath, srcPath); err != nil {
 						return err
 					}
-				}
-			}
-			var stat syscall.Stat_t
-			if err := syscall.Stat(rootVolPath, &stat); err != nil {
-				return err
-			}
-			var srcStat syscall.Stat_t
-			if err := syscall.Stat(srcPath, &srcStat); err != nil {
-				return err
-			}
-			if stat.Uid != srcStat.Uid || stat.Gid != srcStat.Gid {
-				if err := os.Chown(srcPath, int(stat.Uid), int(stat.Gid)); err != nil {
-					return err
+
+					var stat syscall.Stat_t
+					if err := syscall.Stat(rootVolPath, &stat); err != nil {
+						return err
+					}
+					var srcStat syscall.Stat_t
+					if err := syscall.Stat(srcPath, &srcStat); err != nil {
+						return err
+					}
+					// Change the source volume's ownership if it differs from the root
+					// files that where just copied
+					if stat.Uid != srcStat.Uid || stat.Gid != srcStat.Gid {
+						if err := os.Chown(srcPath, int(stat.Uid), int(stat.Gid)); err != nil {
+							return err
+						}
+					}
 				}
 			}
 		}

container_test.go

@@ -1202,7 +1202,7 @@ func TestCopyVolumeUidGid(t *testing.T) {
 	defer nuke(r)
 
 	// Add directory not owned by root
-	container1, _, _ := mkContainer(r, []string{"_", "/bin/sh", "-c", "mkdir -p /hello && chown daemon.daemon /hello"}, t)
+	container1, _, _ := mkContainer(r, []string{"_", "/bin/sh", "-c", "mkdir -p /hello && touch /hello/test.txt && chown daemon.daemon /hello"}, t)
 	defer r.Destroy(container1)
 
 	if container1.State.Running {
@@ -1227,18 +1227,10 @@ func TestCopyVolumeUidGid(t *testing.T) {
 	// Test that the uid and gid is copied from the image to the volume
 	tmpDir1 := tempDir(t)
 	defer os.RemoveAll(tmpDir1)
-	stdout1, _ := runContainer(r, []string{"-v", fmt.Sprintf("%s:/hello", tmpDir1), img.ID, "stat", "-c", "%U %G", "/hello"}, t)
+	stdout1, _ := runContainer(r, []string{"-v", "/hello", img.ID, "stat", "-c", "%U %G", "/hello"}, t)
 	if !strings.Contains(stdout1, "daemon daemon") {
 		t.Fatal("Container failed to transfer uid and gid to volume")
 	}
-
-	// Test that the uid and gid is not copied from the image when the volume is read only
-	tmpDir2 := tempDir(t)
-	defer os.RemoveAll(tmpDir1)
-	stdout2, _ := runContainer(r, []string{"-v", fmt.Sprintf("%s:/hello:ro", tmpDir2), img.ID, "stat", "-c", "%U %G", "/hello"}, t)
-	if strings.Contains(stdout2, "daemon daemon") {
-		t.Fatal("Container transfered uid and gid to volume")
-	}
 }
 
 // Test for #1582
@@ -1272,27 +1264,10 @@ func TestCopyVolumeContent(t *testing.T) {
 	// Test that the content is copied from the image to the volume
 	tmpDir1 := tempDir(t)
 	defer os.RemoveAll(tmpDir1)
-	stdout1, _ := runContainer(r, []string{"-v", fmt.Sprintf("%s:/hello", tmpDir1), img.ID, "find", "/hello"}, t)
+	stdout1, _ := runContainer(r, []string{"-v", "/hello", img.ID, "find", "/hello"}, t)
 	if !(strings.Contains(stdout1, "/hello/local/world") && strings.Contains(stdout1, "/hello/local")) {
 		t.Fatal("Container failed to transfer content to volume")
 	}
-
-	// Test that the content is not copied when the volume is readonly
-	tmpDir2 := tempDir(t)
-	defer os.RemoveAll(tmpDir2)
-	stdout2, _ := runContainer(r, []string{"-v", fmt.Sprintf("%s:/hello:ro", tmpDir2), img.ID, "find", "/hello"}, t)
-	if strings.Contains(stdout2, "/hello/local/world") || strings.Contains(stdout2, "/hello/local") {
-		t.Fatal("Container transfered content to readonly volume")
-	}
-
-	// Test that the content is not copied when the volume is non-empty
-	tmpDir3 := tempDir(t)
-	defer os.RemoveAll(tmpDir3)
-	writeFile(path.Join(tmpDir3, "touch-me"), "", t)
-	stdout3, _ := runContainer(r, []string{"-v", fmt.Sprintf("%s:/hello:rw", tmpDir3), img.ID, "find", "/hello"}, t)
-	if strings.Contains(stdout3, "/hello/local/world") || strings.Contains(stdout3, "/hello/local") || !strings.Contains(stdout3, "/hello/touch-me") {
-		t.Fatal("Container transfered content to non-empty volume")
-	}
 }
 
 func TestBindMounts(t *testing.T) {