Merge pull request #7357 from crosbymichael/docker-ignore

Fix .dockerignore when ignoring unreadable dirs

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
-			return fmt.Errorf("Error checking context is accessible: '%s'. Please check permissions and try again.", err)
+		ignore, err := ioutil.ReadFile(path.Join(root, ".dockerignore"))
-		}
+		if err != nil && !os.IsNotExist(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
 		}
-
+		for _, pattern := range strings.Split(string(ignore), "\n") {
 			ok, err := filepath.Match(pattern, "Dockerfile")
 			if err != nil {
-					utils.Errorf("Bad .dockerignore pattern: '%s', error: %s", pattern, err)
+				return fmt.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 = 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,
+			Excludes:    excludes,
 		}
 		context, err = archive.TarWithOptions(root, options)
 		if err != nil {

archive/archive.go

@@ -349,24 +349,18 @@ func TarWithOptions(srcPath string, options *TarOptions) (io.ReadCloser, error)
 					return nil
 				}
 
-				for _, exclude := range options.Excludes {
+				skip, err := utils.Matches(relFilePath, options.Excludes)
-					matched, err := filepath.Match(exclude, relFilePath)
 				if err != nil {
-						utils.Errorf("Error matching: %s (pattern: %s)", relFilePath, exclude)
+					utils.Debugf("Error matching %s\n", relFilePath, err)
 					return err
 				}
-					if matched {
+
-						if filepath.Clean(relFilePath) == "." {
+				if skip {
-							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
 				}
-				}
 
 				if err := addTarFile(filePath, relFilePath, tw, twBuf); err != nil {
 					utils.Debugf("Can't add file %s to tar: %s\n", srcPath, err)

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
+}