Skip to content

Commit 27cca4c

Browse files
brutasseMichael Crosby
authored andcommitted
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é <[email protected]> (github: brutasse)
1 parent 54b287b commit 27cca4c

File tree

7 files changed

+86
-38
lines changed

7 files changed

+86
-38
lines changed

api/client/commands.go

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -163,30 +163,27 @@ func (cli *DockerCli) CmdBuild(args ...string) error {
163163
if _, err = os.Stat(filename); os.IsNotExist(err) {
164164
return fmt.Errorf("no Dockerfile found in %s", cmd.Arg(0))
165165
}
166-
if err = utils.ValidateContextDirectory(root); err != nil {
166+
var excludes []string
167+
ignore, err := ioutil.ReadFile(path.Join(root, ".dockerignore"))
168+
if err != nil && !os.IsNotExist(err) {
169+
return fmt.Errorf("Error reading .dockerignore: '%s'", err)
170+
}
171+
for _, pattern := range strings.Split(string(ignore), "\n") {
172+
ok, err := filepath.Match(pattern, "Dockerfile")
173+
if err != nil {
174+
return fmt.Errorf("Bad .dockerignore pattern: '%s', error: %s", pattern, err)
175+
}
176+
if ok {
177+
return fmt.Errorf("Dockerfile was excluded by .dockerignore pattern '%s'", pattern)
178+
}
179+
excludes = append(excludes, pattern)
180+
}
181+
if err = utils.ValidateContextDirectory(root, excludes); err != nil {
167182
return fmt.Errorf("Error checking context is accessible: '%s'. Please check permissions and try again.", err)
168183
}
169184
options := &archive.TarOptions{
170185
Compression: archive.Uncompressed,
171-
}
172-
if ignore, err := ioutil.ReadFile(path.Join(root, ".dockerignore")); err != nil && !os.IsNotExist(err) {
173-
return fmt.Errorf("Error reading .dockerignore: '%s'", err)
174-
} else if err == nil {
175-
for _, pattern := range strings.Split(string(ignore), "\n") {
176-
if pattern == "" {
177-
continue
178-
}
179-
180-
ok, err := filepath.Match(pattern, "Dockerfile")
181-
if err != nil {
182-
utils.Errorf("Bad .dockerignore pattern: '%s', error: %s", pattern, err)
183-
continue
184-
}
185-
if ok {
186-
return fmt.Errorf("Dockerfile was excluded by .dockerignore pattern '%s'", pattern)
187-
}
188-
options.Excludes = append(options.Excludes, pattern)
189-
}
186+
Excludes: excludes,
190187
}
191188
context, err = archive.TarWithOptions(root, options)
192189
if err != nil {

archive/archive.go

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -349,23 +349,16 @@ func TarWithOptions(srcPath string, options *TarOptions) (io.ReadCloser, error)
349349
return nil
350350
}
351351

352-
for _, exclude := range options.Excludes {
353-
matched, err := filepath.Match(exclude, relFilePath)
354-
if err != nil {
355-
utils.Errorf("Error matching: %s (pattern: %s)", relFilePath, exclude)
356-
return err
357-
}
358-
if matched {
359-
if filepath.Clean(relFilePath) == "." {
360-
utils.Errorf("Can't exclude whole path, excluding pattern: %s", exclude)
361-
continue
362-
}
363-
utils.Debugf("Skipping excluded path: %s", relFilePath)
364-
if f.IsDir() {
365-
return filepath.SkipDir
366-
}
367-
return nil
352+
skip, err := utils.Matches(relFilePath, options.Excludes)
353+
if err != nil {
354+
utils.Debugf("Error matching %s\n", relFilePath, err)
355+
return nil
356+
}
357+
if skip {
358+
if f.IsDir() {
359+
return filepath.SkipDir
368360
}
361+
return nil
369362
}
370363

371364
if err := addTarFile(filePath, relFilePath, tw, twBuf); err != nil {
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
directoryWeCantStat
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
FROM busybox
2+
ADD . /foo/
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
foo

integration-cli/docker_cli_build_test.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -474,10 +474,33 @@ func TestBuildWithInaccessibleFilesInContext(t *testing.T) {
474474

475475
deleteImages("testlinksok")
476476

477+
}
478+
{
479+
// This is used to ensure we don't try to add inaccessible files when they are ignored by a .dockerignore pattern
480+
pathToInaccessibleDirectoryBuildDirectory := filepath.Join(buildDirectory, "ignoredinaccessible")
481+
pathToDirectoryWithoutReadAccess := filepath.Join(pathToInaccessibleDirectoryBuildDirectory, "directoryWeCantStat")
482+
pathToFileInDirectoryWithoutReadAccess := filepath.Join(pathToDirectoryWithoutReadAccess, "bar")
483+
err := os.Chown(pathToDirectoryWithoutReadAccess, 0, 0)
484+
errorOut(err, t, fmt.Sprintf("failed to chown directory to root: %s", err))
485+
err = os.Chmod(pathToDirectoryWithoutReadAccess, 0444)
486+
errorOut(err, t, fmt.Sprintf("failed to chmod directory to 755: %s", err))
487+
err = os.Chmod(pathToFileInDirectoryWithoutReadAccess, 0700)
488+
errorOut(err, t, fmt.Sprintf("failed to chmod file to 444: %s", err))
489+
490+
buildCommandStatement := fmt.Sprintf("%s build -t ignoredinaccessible .", dockerBinary)
491+
buildCmd := exec.Command("su", "unprivilegeduser", "-c", buildCommandStatement)
492+
buildCmd.Dir = pathToInaccessibleDirectoryBuildDirectory
493+
out, exitCode, err := runCommandWithOutput(buildCmd)
494+
if err != nil || exitCode != 0 {
495+
t.Fatalf("build should have worked: %s %s", err, out)
496+
}
497+
deleteImages("ignoredinaccessible")
498+
477499
}
478500
deleteImages("inaccessiblefiles")
479501
logDone("build - ADD from context with inaccessible files must fail")
480502
logDone("build - ADD from context with accessible links must work")
503+
logDone("build - ADD from context with ignored inaccessible files must work")
481504
}
482505

483506
func TestBuildForceRm(t *testing.T) {

utils/utils.go

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -684,16 +684,27 @@ func TreeSize(dir string) (size int64, err error) {
684684
// ValidateContextDirectory checks if all the contents of the directory
685685
// can be read and returns an error if some files can't be read
686686
// symlinks which point to non-existing files don't trigger an error
687-
func ValidateContextDirectory(srcPath string) error {
687+
func ValidateContextDirectory(srcPath string, excludes []string) error {
688688
var finalError error
689689

690690
filepath.Walk(filepath.Join(srcPath, "."), func(filePath string, f os.FileInfo, err error) error {
691691
// skip this directory/file if it's not in the path, it won't get added to the context
692-
_, err = filepath.Rel(srcPath, filePath)
692+
relFilePath, err := filepath.Rel(srcPath, filePath)
693693
if err != nil && os.IsPermission(err) {
694694
return nil
695695
}
696696

697+
skip, err := Matches(relFilePath, excludes)
698+
if err != nil {
699+
finalError = err
700+
}
701+
if skip {
702+
if f.IsDir() {
703+
return filepath.SkipDir
704+
}
705+
return nil
706+
}
707+
697708
if _, err := os.Stat(filePath); err != nil && os.IsPermission(err) {
698709
finalError = fmt.Errorf("can't stat '%s'", filePath)
699710
return err
@@ -726,3 +737,23 @@ func StringsContainsNoCase(slice []string, s string) bool {
726737
}
727738
return false
728739
}
740+
741+
// Matches returns true if relFilePath matches any of the patterns
742+
func Matches(relFilePath string, patterns []string) (bool, error) {
743+
for _, exclude := range patterns {
744+
matched, err := filepath.Match(exclude, relFilePath)
745+
if err != nil {
746+
Errorf("Error matching: %s (pattern: %s)", relFilePath, exclude)
747+
return false, err
748+
}
749+
if matched {
750+
if filepath.Clean(relFilePath) == "." {
751+
Errorf("Can't exclude whole path, excluding pattern: %s", exclude)
752+
continue
753+
}
754+
Debugf("Skipping excluded path: %s", relFilePath)
755+
return true, nil
756+
}
757+
}
758+
return false, nil
759+
}

0 commit comments

Comments
 (0)