Merge pull request #7920 from unclejack/fix_layer_permissions

ensure correct permissions are set for directories

archive/changes.go

@@ -136,6 +136,7 @@ type FileInfo struct {
 	stat       syscall.Stat_t
 	children   map[string]*FileInfo
 	capability []byte
+	added      bool
 }
 
 func (root *FileInfo) LookUp(path string) *FileInfo {
@@ -169,6 +170,9 @@ func (info *FileInfo) isDir() bool {
 }
 
 func (info *FileInfo) addChanges(oldInfo *FileInfo, changes *[]Change) {
+
+	sizeAtEntry := len(*changes)
+
 	if oldInfo == nil {
 		// add
 		change := Change{
@@ -176,6 +180,7 @@ func (info *FileInfo) addChanges(oldInfo *FileInfo, changes *[]Change) {
 			Kind: ChangeAdd,
 		}
 		*changes = append(*changes, change)
+		info.added = true
 	}
 
 	// We make a copy so we can modify it to detect additions
@@ -213,6 +218,7 @@ func (info *FileInfo) addChanges(oldInfo *FileInfo, changes *[]Change) {
 					Kind: ChangeModify,
 				}
 				*changes = append(*changes, change)
+				newChild.added = true
 			}
 
 			// Remove from copy so we can detect deletions
@@ -230,6 +236,19 @@ func (info *FileInfo) addChanges(oldInfo *FileInfo, changes *[]Change) {
 		*changes = append(*changes, change)
 	}
 
+	// If there were changes inside this directory, we need to add it, even if the directory
+	// itself wasn't changed. This is needed to properly save and restore filesystem permissions.
+	if len(*changes) > sizeAtEntry && info.isDir() && !info.added && info.path() != "/" {
+		change := Change{
+			Path: info.path(),
+			Kind: ChangeModify,
+		}
+		// Let's insert the directory entry before the recently added entries located inside this dir
+		*changes = append(*changes, change) // just to resize the slice, will be overwritten
+		copy((*changes)[sizeAtEntry+1:], (*changes)[sizeAtEntry:])
+		(*changes)[sizeAtEntry] = change
+	}
+
 }
 
 func (info *FileInfo) Changes(oldInfo *FileInfo) []Change {

integration-cli/docker_cli_save_load_test.go

@@ -2,8 +2,11 @@ package main
 
 import (
 	"fmt"
+	"io/ioutil"
 	"os"
 	"os/exec"
+	"path/filepath"
+	"reflect"
 	"testing"
 )
 
@@ -191,3 +194,66 @@ func TestSaveMultipleNames(t *testing.T) {
 
 	logDone("save - save by multiple names")
 }
+
+// Issue #6722 #5892 ensure directories are included in changes
+func TestSaveDirectoryPermissions(t *testing.T) {
+	layerEntries := []string{"opt/", "opt/a/", "opt/a/b/", "opt/a/b/c"}
+
+	name := "save-directory-permissions"
+	tmpDir, err := ioutil.TempDir("", "save-layers-with-directories")
+	extractionDirectory := filepath.Join(tmpDir, "image-extraction-dir")
+	os.Mkdir(extractionDirectory, 0777)
+
+	if err != nil {
+		t.Errorf("failed to create temporary directory: %s", err)
+	}
+	defer os.RemoveAll(tmpDir)
+	defer deleteImages(name)
+	_, err = buildImage(name,
+		`FROM busybox
+	RUN adduser -D user && mkdir -p /opt/a/b && chown -R user:user /opt/a
+	RUN touch /opt/a/b/c && chown user:user /opt/a/b/c`,
+		true)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	saveCmdFinal := fmt.Sprintf("%s save %s | tar -xf - -C %s", dockerBinary, name, extractionDirectory)
+	saveCmd := exec.Command("bash", "-c", saveCmdFinal)
+	out, _, err := runCommandWithOutput(saveCmd)
+	if err != nil {
+		t.Errorf("failed to save and extract image: %s", out)
+	}
+
+	dirs, err := ioutil.ReadDir(extractionDirectory)
+	if err != nil {
+		t.Errorf("failed to get a listing of the layer directories: %s", err)
+	}
+
+	found := false
+	for _, entry := range dirs {
+		if entry.IsDir() {
+			layerPath := filepath.Join(extractionDirectory, entry.Name(), "layer.tar")
+
+			f, err := os.Open(layerPath)
+			if err != nil {
+				t.Fatalf("failed to open %s: %s", layerPath, err)
+			}
+
+			entries, err := ListTar(f)
+			if err != nil {
+				t.Fatalf("encountered error while listing tar entries: %s", err)
+			}
+
+			if reflect.DeepEqual(entries, layerEntries) {
+				found = true
+			}
+		}
+	}
+
+	if !found {
+		t.Fatalf("failed to find the layer with the right content listing")
+	}
+
+	logDone("save - ensure directories exist in exported layers")
+}

integration-cli/utils.go

@@ -12,6 +12,8 @@ import (
 	"syscall"
 	"testing"
 	"time"
+
+	"github.com/docker/docker/vendor/src/code.google.com/p/go/src/pkg/archive/tar"
 )
 
 func getExitCode(err error) (int, error) {
@@ -193,3 +195,20 @@ func compareDirectoryEntries(e1 []os.FileInfo, e2 []os.FileInfo) error {
 	}
 	return nil
 }
+
+func ListTar(f io.Reader) ([]string, error) {
+	tr := tar.NewReader(f)
+	var entries []string
+
+	for {
+		th, err := tr.Next()
+		if err == io.EOF {
+			// end of tar archive
+			return entries, nil
+		}
+		if err != nil {
+			return entries, err
+		}
+		entries = append(entries, th.Name)
+	}
+}