Better support for .dockerignore

- Support all basic pattern forms: file, directory, *, ?, !

- Fix handling of wildcard patterns and subdirectories - `*/a.py` should
  match `foo/a.py`, but not `foo/bar/a.py`

- Fix handling of directory patterns with a trailing slash - make sure
  they're handled equivalently to those without one

- Fix handling of custom Dockerfiles - make sure they go in the tarball
Signed-off-by: Aanand Prasad <aanand.prasad@gmail.com>

docker/client.py

@@ -95,13 +95,7 @@ class Client(clientbase.ClientBase):
             if os.path.exists(dockerignore):
                 with open(dockerignore, 'r') as f:
                     exclude = list(filter(bool, f.read().splitlines()))
-                    # These are handled by the docker daemon and should not be
-                    # excluded on the client
-                    if 'Dockerfile' in exclude:
-                        exclude.remove('Dockerfile')
-                    if '.dockerignore' in exclude:
-                        exclude.remove(".dockerignore")
-            context = utils.tar(path, exclude=exclude)
+            context = utils.tar(path, exclude=exclude, dockerfile=dockerfile)
 
         if utils.compare_version('1.8', self._version) >= 0:
             stream = True

docker/utils/__init__.py

 from .utils import (
     compare_version, convert_port_bindings, convert_volume_binds,
-    mkbuildcontext, tar, parse_repository_tag, parse_host,
+    mkbuildcontext, tar, exclude_paths, parse_repository_tag, parse_host,
     kwargs_from_env, convert_filters, create_host_config,
     create_container_config, parse_bytes, ping_registry, parse_env_file
 ) # flake8: noqa

docker/utils/utils.py

@@ -66,39 +66,82 @@ def mkbuildcontext(dockerfile):
     return f
 
 
-def fnmatch_any(relpath, patterns):
-    return any([fnmatch(relpath, pattern) for pattern in patterns])
-
-
-def tar(path, exclude=None):
+def tar(path, exclude=None, dockerfile=None):
     f = tempfile.NamedTemporaryFile()
     t = tarfile.open(mode='w', fileobj=f)
-    for dirpath, dirnames, filenames in os.walk(path):
-        relpath = os.path.relpath(dirpath, path)
-        if relpath == '.':
-            relpath = ''
-        if exclude is None:
-            fnames = filenames
-        else:
-            dirnames[:] = [d for d in dirnames
-                           if not fnmatch_any(os.path.join(relpath, d),
-                                              exclude)]
-            fnames = [name for name in filenames
-                      if not fnmatch_any(os.path.join(relpath, name),
-                                         exclude)]
-        dirnames.sort()
-        for name in sorted(fnames):
-            arcname = os.path.join(relpath, name)
-            t.add(os.path.join(path, arcname), arcname=arcname)
-        for name in dirnames:
-            arcname = os.path.join(relpath, name)
-            t.add(os.path.join(path, arcname),
-                  arcname=arcname, recursive=False)
+
+    root = os.path.abspath(path)
+    exclude = exclude or []
+
+    for path in sorted(exclude_paths(root, exclude, dockerfile=dockerfile)):
+        t.add(os.path.join(root, path), arcname=path, recursive=False)
+
     t.close()
     f.seek(0)
     return f
 
 
+def exclude_paths(root, patterns, dockerfile=None):
+    """
+    Given a root directory path and a list of .dockerignore patterns, return
+    an iterator of all paths (both regular files and directories) in the root
+    directory that do *not* match any of the patterns.
+
+    All paths returned are relative to the root.
+    """
+    if dockerfile is None:
+        dockerfile = 'Dockerfile'
+
+    exceptions = [p for p in patterns if p.startswith('!')]
+
+    include_patterns = [p[1:] for p in exceptions]
+    include_patterns += [dockerfile, '.dockerignore']
+
+    exclude_patterns = list(set(patterns) - set(exceptions))
+
+    all_paths = get_paths(root)
+
+    # Remove all paths that are matched by any exclusion pattern
+    paths = [
+        p for p in all_paths
+        if not any(match_path(p, pattern) for pattern in exclude_patterns)
+    ]
+
+    # Add back the set of paths that are matched by any inclusion pattern.
+    # Include parent dirs - if we add back 'foo/bar', add 'foo' as well
+    for p in all_paths:
+        if any(match_path(p, pattern) for pattern in include_patterns):
+            components = p.split('/')
+            paths += [
+                '/'.join(components[:end])
+                for end in range(1, len(components)+1)
+            ]
+
+    return set(paths)
+
+
+def get_paths(root):
+    paths = []
+
+    for parent, dirs, files in os.walk(root, followlinks=False):
+        parent = os.path.relpath(parent, root)
+        if parent == '.':
+            parent = ''
+        for path in dirs:
+            paths.append(os.path.join(parent, path))
+        for path in files:
+            paths.append(os.path.join(parent, path))
+
+    return paths
+
+
+def match_path(path, pattern):
+    pattern = pattern.rstrip('/')
+    pattern_components = pattern.split('/')
+    path_components = path.split('/')[:len(pattern_components)]
+    return fnmatch('/'.join(path_components), pattern)
+
+
 def compare_version(v1, v2):
     """Compare docker versions
 

tests/helpers.py

+import os
+import os.path
+import tempfile
+
+
+def make_tree(dirs, files):
+    base = tempfile.mkdtemp()
+
+    for path in dirs:
+        os.makedirs(os.path.join(base, path))
+
+    for path in files:
+        with open(os.path.join(base, path), 'w') as f:
+            f.write("content")
+
+    return base

tests/test.py

@@ -35,6 +35,7 @@ import six
 
 from . import base
 from . import fake_api
+from .helpers import make_tree
 
 import pytest
 
@@ -2054,26 +2055,50 @@ class DockerClientTest(Cleanup, base.BaseTestCase):
         self.assertEqual(cfg.get('auth'), None)
 
     def test_tar_with_excludes(self):
-        base = tempfile.mkdtemp()
+        dirs = [
+            'foo',
+            'foo/bar',
+            'bar',
+        ]
+
+        files = [
+            'Dockerfile',
+            'Dockerfile.alt',
+            '.dockerignore',
+            'a.py',
+            'a.go',
+            'b.py',
+            'cde.py',
+            'foo/a.py',
+            'foo/b.py',
+            'foo/bar/a.py',
+            'bar/a.py',
+        ]
+
+        exclude = [
+            '*.py',
+            '!b.py',
+            '!a.go',
+            'foo',
+            'Dockerfile*',
+            '.dockerignore',
+        ]
+
+        expected_names = set([
+            'Dockerfile',
+            '.dockerignore',
+            'a.go',
+            'b.py',
+            'bar',
+            'bar/a.py',
+        ])
+
+        base = make_tree(dirs, files)
         self.addCleanup(shutil.rmtree, base)
-        for d in ['test/foo', 'bar']:
-            os.makedirs(os.path.join(base, d))
-            for f in ['a.txt', 'b.py', 'other.png']:
-                with open(os.path.join(base, d, f), 'w') as f:
-                    f.write("content")
-
-        for exclude, names in (
-                (['*.py'], ['bar', 'bar/a.txt', 'bar/other.png',
-                            'test', 'test/foo', 'test/foo/a.txt',
-                            'test/foo/other.png']),
-                (['*.png', 'bar'], ['test', 'test/foo', 'test/foo/a.txt',
-                                    'test/foo/b.py']),
-                (['test/foo', 'a.txt'], ['bar', 'bar/a.txt', 'bar/b.py',
-                                         'bar/other.png', 'test']),
-        ):
-            with docker.utils.tar(base, exclude=exclude) as archive:
-                tar = tarfile.open(fileobj=archive)
-                self.assertEqual(sorted(tar.getnames()), names)
+
+        with docker.utils.tar(base, exclude=exclude) as archive:
+            tar = tarfile.open(fileobj=archive)
+            assert sorted(tar.getnames()) == sorted(expected_names)
 
     def test_tar_with_empty_directory(self):
         base = tempfile.mkdtemp()

tests/utils_test.py

 import os
 import os.path
+import shutil
 import tempfile
 
 from docker.client import Client
@@ -7,12 +8,14 @@ from docker.constants import DEFAULT_DOCKER_API_VERSION
 from docker.errors import DockerException
 from docker.utils import (
     parse_repository_tag, parse_host, convert_filters, kwargs_from_env,
-    create_host_config, Ulimit, LogConfig, parse_bytes, parse_env_file
+    create_host_config, Ulimit, LogConfig, parse_bytes, parse_env_file,
+    exclude_paths,
 )
 from docker.utils.ports import build_port_bindings, split_port
 from docker.auth import resolve_repository_name, resolve_authconfig
 
 from . import base
+from .helpers import make_tree
 
 import pytest
 
@@ -472,3 +475,141 @@ class UtilsTest(base.BaseTestCase):
             ["127.0.0.1:1000:1000", "127.0.0.1:2000:2000"])
         self.assertEqual(port_bindings["1000"], [("127.0.0.1", "1000")])
         self.assertEqual(port_bindings["2000"], [("127.0.0.1", "2000")])
+
+
+class ExcludePathsTest(base.BaseTestCase):
+    dirs = [
+        'foo',
+        'foo/bar',
+        'bar',
+    ]
+
+    files = [
+        'Dockerfile',
+        'Dockerfile.alt',
+        '.dockerignore',
+        'a.py',
+        'a.go',
+        'b.py',
+        'cde.py',
+        'foo/a.py',
+        'foo/b.py',
+        'foo/bar/a.py',
+        'bar/a.py',
+    ]
+
+    all_paths = set(dirs + files)
+
+    def setUp(self):
+        self.base = make_tree(self.dirs, self.files)
+
+    def tearDown(self):
+        shutil.rmtree(self.base)
+
+    def exclude(self, patterns, dockerfile=None):
+        return set(exclude_paths(self.base, patterns, dockerfile=dockerfile))
+
+    def test_no_excludes(self):
+        assert self.exclude(['']) == self.all_paths
+
+    def test_no_dupes(self):
+        paths = exclude_paths(self.base, ['!a.py'])
+        assert sorted(paths) == sorted(set(paths))
+
+    def test_wildcard_exclude(self):
+        assert self.exclude(['*']) == set(['Dockerfile', '.dockerignore'])
+
+    def test_exclude_dockerfile_dockerignore(self):
+        """
+        Even if the .dockerignore file explicitly says to exclude
+        Dockerfile and/or .dockerignore, don't exclude them from
+        the actual tar file.
+        """
+        assert self.exclude(['Dockerfile', '.dockerignore']) == self.all_paths
+
+    def test_exclude_custom_dockerfile(self):
+        """
+        If we're using a custom Dockerfile, make sure that's not
+        excluded.
+        """
+        assert self.exclude(['*'], dockerfile='Dockerfile.alt') == \
+            set(['Dockerfile.alt', '.dockerignore'])
+
+    def test_single_filename(self):
+        assert self.exclude(['a.py']) == self.all_paths - set(['a.py'])
+
+    # As odd as it sounds, a filename pattern with a trailing slash on the
+    # end *will* result in that file being excluded.
+    def test_single_filename_trailing_slash(self):
+        assert self.exclude(['a.py/']) == self.all_paths - set(['a.py'])
+
+    def test_wildcard_filename_start(self):
+        assert self.exclude(['*.py']) == self.all_paths - set([
+            'a.py', 'b.py', 'cde.py',
+        ])
+
+    def test_wildcard_with_exception(self):
+        assert self.exclude(['*.py', '!b.py']) == self.all_paths - set([
+            'a.py', 'cde.py',
+        ])
+
+    def test_wildcard_with_wildcard_exception(self):
+        assert self.exclude(['*.*', '!*.go']) == self.all_paths - set([
+            'a.py', 'b.py', 'cde.py', 'Dockerfile.alt',
+        ])
+
+    def test_wildcard_filename_end(self):
+        assert self.exclude(['a.*']) == self.all_paths - set(['a.py', 'a.go'])
+
+    def test_question_mark(self):
+        assert self.exclude(['?.py']) == self.all_paths - set(['a.py', 'b.py'])
+
+    def test_single_subdir_single_filename(self):
+        assert self.exclude(['foo/a.py']) == self.all_paths - set(['foo/a.py'])
+
+    def test_single_subdir_wildcard_filename(self):
+        assert self.exclude(['foo/*.py']) == self.all_paths - set([
+            'foo/a.py', 'foo/b.py',
+        ])
+
+    def test_wildcard_subdir_single_filename(self):
+        assert self.exclude(['*/a.py']) == self.all_paths - set([
+            'foo/a.py', 'bar/a.py',
+        ])
+
+    def test_wildcard_subdir_wildcard_filename(self):
+        assert self.exclude(['*/*.py']) == self.all_paths - set([
+            'foo/a.py', 'foo/b.py', 'bar/a.py',
+        ])
+
+    def test_directory(self):
+        assert self.exclude(['foo']) == self.all_paths - set([
+            'foo', 'foo/a.py', 'foo/b.py',
+            'foo/bar', 'foo/bar/a.py',
+        ])
+
+    def test_directory_with_trailing_slash(self):
+        assert self.exclude(['foo']) == self.all_paths - set([
+            'foo', 'foo/a.py', 'foo/b.py',
+            'foo/bar', 'foo/bar/a.py',
+        ])
+
+    def test_directory_with_single_exception(self):
+        assert self.exclude(['foo', '!foo/bar/a.py']) == self.all_paths - set([
+            'foo/a.py', 'foo/b.py',
+        ])
+
+    def test_directory_with_subdir_exception(self):
+        assert self.exclude(['foo', '!foo/bar']) == self.all_paths - set([
+            'foo/a.py', 'foo/b.py',
+        ])
+
+    def test_directory_with_wildcard_exception(self):
+        assert self.exclude(['foo', '!foo/*.py']) == self.all_paths - set([
+            'foo/bar', 'foo/bar/a.py',
+        ])
+
+    def test_subdirectory(self):
+        assert self.exclude(['foo/bar']) == self.all_paths - set([
+            'foo/bar', 'foo/bar/a.py',
+        ])