From 691bbf6a2971bd6d57892cbf7efe188f431919a2 Mon Sep 17 00:00:00 2001 From: Leszek Kowalski Date: Sat, 12 Jul 2014 23:28:04 +0100 Subject: [PATCH 1/2] Include directories that contain changes inside them when calculating diff. Fixes #6722 Docker-DCO-1.1-Signed-off-by: Leszek Kowalski (github: ljk) --- archive/changes.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/archive/changes.go b/archive/changes.go index eefc4b67e4..231f3064c1 100644 --- a/archive/changes.go +++ b/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 { From cc5fb986b03ce28075a9f3aee248d401f3be0c96 Mon Sep 17 00:00:00 2001 From: unclejack Date: Sat, 6 Sep 2014 14:49:40 +0300 Subject: [PATCH 2/2] integ-cli: test correct directory permissions Docker-DCO-1.1-Signed-off-by: Cristian Staretu (github: unclejack) --- integration-cli/docker_cli_save_load_test.go | 66 ++++++++++++++++++++ integration-cli/utils.go | 19 ++++++ 2 files changed, 85 insertions(+) diff --git a/integration-cli/docker_cli_save_load_test.go b/integration-cli/docker_cli_save_load_test.go index 6f04ec40f2..46fa2a58e0 100644 --- a/integration-cli/docker_cli_save_load_test.go +++ b/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") +} diff --git a/integration-cli/utils.go b/integration-cli/utils.go index e4a1e95fbe..9d0aceb156 100644 --- a/integration-cli/utils.go +++ b/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) + } +}