Fix .dockerignore when ignoring unreadable dirs

The initial `ValidateContextDirectory` implementation fails loudly when a file
lacks read permissions in the current context. However that situation is valid
if the file is included in the `.dockerignore` patterns.

Docker-DCO-1.1-Signed-off-by: Bruno Renié <brutasse@gmail.com> (github: brutasse)

api/client/commands.go

@@ -163,30 +163,27 @@ func (cli *DockerCli) CmdBuild(args ...string) error {
 		if _, err = os.Stat(filename); os.IsNotExist(err) {
 			return fmt.Errorf("no Dockerfile found in %s", cmd.Arg(0))
 		}
-		if err = utils.ValidateContextDirectory(root); err != nil {
+		var excludes []string
+		ignore, err := ioutil.ReadFile(path.Join(root, ".dockerignore"))
+		if err != nil && !os.IsNotExist(err) {
+			return fmt.Errorf("Error reading .dockerignore: '%s'", err)
+		}
+		for _, pattern := range strings.Split(string(ignore), "\n") {
+			ok, err := filepath.Match(pattern, "Dockerfile")
+			if err != nil {
+				return fmt.Errorf("Bad .dockerignore pattern: '%s', error: %s", pattern, err)
+			}
+			if ok {
+				return fmt.Errorf("Dockerfile was excluded by .dockerignore pattern '%s'", pattern)
+			}
+			excludes = append(excludes, pattern)
+		}
+		if err = utils.ValidateContextDirectory(root, excludes); err != nil {
 			return fmt.Errorf("Error checking context is accessible: '%s'. Please check permissions and try again.", err)
 		}
 		options := &archive.TarOptions{
 			Compression: archive.Uncompressed,
-		}
-		if ignore, err := ioutil.ReadFile(path.Join(root, ".dockerignore")); err != nil && !os.IsNotExist(err) {
-			return fmt.Errorf("Error reading .dockerignore: '%s'", err)
-		} else if err == nil {
-			for _, pattern := range strings.Split(string(ignore), "\n") {
-				if pattern == "" {
-					continue
-				}
-
-				ok, err := filepath.Match(pattern, "Dockerfile")
-				if err != nil {
-					utils.Errorf("Bad .dockerignore pattern: '%s', error: %s", pattern, err)
-					continue
-				}
-				if ok {
-					return fmt.Errorf("Dockerfile was excluded by .dockerignore pattern '%s'", pattern)
-				}
-				options.Excludes = append(options.Excludes, pattern)
-			}
+			Excludes:    excludes,
 		}
 		context, err = archive.TarWithOptions(root, options)
 		if err != nil {

archive/archive.go

@@ -349,23 +349,16 @@ func TarWithOptions(srcPath string, options *TarOptions) (io.ReadCloser, error)
 					return nil
 				}
 
-				for _, exclude := range options.Excludes {
-					matched, err := filepath.Match(exclude, relFilePath)
-					if err != nil {
-						utils.Errorf("Error matching: %s (pattern: %s)", relFilePath, exclude)
-						return err
-					}
-					if matched {
-						if filepath.Clean(relFilePath) == "." {
-							utils.Errorf("Can't exclude whole path, excluding pattern: %s", exclude)
-							continue
-						}
-						utils.Debugf("Skipping excluded path: %s", relFilePath)
-						if f.IsDir() {
-							return filepath.SkipDir
-						}
-						return nil
+				skip, err := utils.Matches(relFilePath, options.Excludes)
+				if err != nil {
+					utils.Debugf("Error matching %s\n", relFilePath, err)
+					return nil
+				}
+				if skip {
+					if f.IsDir() {
+						return filepath.SkipDir
 					}
+					return nil
 				}
 
 				if err := addTarFile(filePath, relFilePath, tw, twBuf); err != nil {

integration-cli/build_tests/TestBuildWithInaccessibleFilesInContext/ignoredinaccessible/.dockerignore

@@ -0,0 +1 @@
+directoryWeCantStat

integration-cli/build_tests/TestBuildWithInaccessibleFilesInContext/ignoredinaccessible/Dockerfile

@@ -0,0 +1,2 @@
+FROM busybox
+ADD . /foo/

integration-cli/build_tests/TestBuildWithInaccessibleFilesInContext/ignoredinaccessible/directoryWeCantStat/bar

@@ -0,0 +1 @@
+foo

integration-cli/docker_cli_build_test.go

@@ -474,10 +474,33 @@ func TestBuildWithInaccessibleFilesInContext(t *testing.T) {
 
 		deleteImages("testlinksok")
 
+	}
+	{
+		// This is used to ensure we don't try to add inaccessible files when they are ignored by a .dockerignore pattern
+		pathToInaccessibleDirectoryBuildDirectory := filepath.Join(buildDirectory, "ignoredinaccessible")
+		pathToDirectoryWithoutReadAccess := filepath.Join(pathToInaccessibleDirectoryBuildDirectory, "directoryWeCantStat")
+		pathToFileInDirectoryWithoutReadAccess := filepath.Join(pathToDirectoryWithoutReadAccess, "bar")
+		err := os.Chown(pathToDirectoryWithoutReadAccess, 0, 0)
+		errorOut(err, t, fmt.Sprintf("failed to chown directory to root: %s", err))
+		err = os.Chmod(pathToDirectoryWithoutReadAccess, 0444)
+		errorOut(err, t, fmt.Sprintf("failed to chmod directory to 755: %s", err))
+		err = os.Chmod(pathToFileInDirectoryWithoutReadAccess, 0700)
+		errorOut(err, t, fmt.Sprintf("failed to chmod file to 444: %s", err))
+
+		buildCommandStatement := fmt.Sprintf("%s build -t ignoredinaccessible .", dockerBinary)
+		buildCmd := exec.Command("su", "unprivilegeduser", "-c", buildCommandStatement)
+		buildCmd.Dir = pathToInaccessibleDirectoryBuildDirectory
+		out, exitCode, err := runCommandWithOutput(buildCmd)
+		if err != nil || exitCode != 0 {
+			t.Fatalf("build should have worked: %s %s", err, out)
+		}
+		deleteImages("ignoredinaccessible")
+
 	}
 	deleteImages("inaccessiblefiles")
 	logDone("build - ADD from context with inaccessible files must fail")
 	logDone("build - ADD from context with accessible links must work")
+	logDone("build - ADD from context with ignored inaccessible files must work")
 }
 
 func TestBuildForceRm(t *testing.T) {

utils/utils.go

@@ -684,16 +684,27 @@ func TreeSize(dir string) (size int64, err error) {
 // ValidateContextDirectory checks if all the contents of the directory
 // can be read and returns an error if some files can't be read
 // symlinks which point to non-existing files don't trigger an error
-func ValidateContextDirectory(srcPath string) error {
+func ValidateContextDirectory(srcPath string, excludes []string) error {
 	var finalError error
 
 	filepath.Walk(filepath.Join(srcPath, "."), func(filePath string, f os.FileInfo, err error) error {
 		// skip this directory/file if it's not in the path, it won't get added to the context
-		_, err = filepath.Rel(srcPath, filePath)
+		relFilePath, err := filepath.Rel(srcPath, filePath)
 		if err != nil && os.IsPermission(err) {
 			return nil
 		}
 
+		skip, err := Matches(relFilePath, excludes)
+		if err != nil {
+			finalError = err
+		}
+		if skip {
+			if f.IsDir() {
+				return filepath.SkipDir
+			}
+			return nil
+		}
+
 		if _, err := os.Stat(filePath); err != nil && os.IsPermission(err) {
 			finalError = fmt.Errorf("can't stat '%s'", filePath)
 			return err
@@ -726,3 +737,23 @@ func StringsContainsNoCase(slice []string, s string) bool {
 	}
 	return false
 }
+
+// Matches returns true if relFilePath matches any of the patterns
+func Matches(relFilePath string, patterns []string) (bool, error) {
+	for _, exclude := range patterns {
+		matched, err := filepath.Match(exclude, relFilePath)
+		if err != nil {
+			Errorf("Error matching: %s (pattern: %s)", relFilePath, exclude)
+			return false, err
+		}
+		if matched {
+			if filepath.Clean(relFilePath) == "." {
+				Errorf("Can't exclude whole path, excluding pattern: %s", exclude)
+				continue
+			}
+			Debugf("Skipping excluded path: %s", relFilePath)
+			return true, nil
+		}
+	}
+	return false, nil
+}