Mount with same container path and different mode should override

Signed-off-by: Joffrey F <joffrey@docker.com>

2 files changed, 60 insertions, 13 deletions

diff --git a/compose/config/config.py b/compose/config/config.py
index f16dd01b..948e2376 100644
--- a/compose/config/config.py
+++ b/compose/config/config.py
@@ -1137,24 +1137,30 @@ def resolve_volume_paths(working_dir, service_dict):
 
 
 def resolve_volume_path(working_dir, volume):
+    mount_params = None
     if isinstance(volume, dict):
-        host_path = volume.get('source')
         container_path = volume.get('target')
+        host_path = volume.get('source')
+        mode = None
         if host_path:
             if volume.get('read_only'):
-                container_path += ':ro'
+                mode = 'ro'
             if volume.get('volume', {}).get('nocopy'):
-                container_path += ':nocopy'
+                mode = 'nocopy'
+        mount_params = (host_path, mode)
     else:
-        container_path, host_path = split_path_mapping(volume)
+        container_path, mount_params = split_path_mapping(volume)
 
-    if host_path is not None:
+    if mount_params is not None:
+        host_path, mode = mount_params
+        if host_path is None:
+            return container_path
         if host_path.startswith('.'):
             host_path = expand_path(working_dir, host_path)
         host_path = os.path.expanduser(host_path)
-        return u"{}:{}".format(host_path, container_path)
-    else:
-        return container_path
+        return u"{}:{}{}".format(host_path, container_path, (':' + mode if mode else ''))
+
+    return container_path
 
 
 def normalize_build(service_dict, working_dir, environment):
@@ -1234,7 +1240,12 @@ def split_path_mapping(volume_path):
 
     if ':' in volume_config:
         (host, container) = volume_config.split(':', 1)
-        return (container, drive + host)
+        container_drive, container_path = splitdrive(container)
+        mode = None
+        if ':' in container_path:
+            container_path, mode = container_path.rsplit(':', 1)
+
+        return (container_drive + container_path, (drive + host, mode))
     else:
         return (volume_path, None)
 
@@ -1246,7 +1257,11 @@ def join_path_mapping(pair):
     elif host is None:
         return container
     else:
-        return ":".join((host, container))
+        host, mode = host
+        result = ":".join((host, container))
+        if mode:
+            result += ":" + mode
+        return result
 
 
 def expand_path(working_dir, path):
diff --git a/tests/unit/config/config_test.py b/tests/unit/config/config_test.py
index de9a6130..c5e40130 100644
--- a/tests/unit/config/config_test.py
+++ b/tests/unit/config/config_test.py
@@ -1101,6 +1101,38 @@ class ConfigTest(unittest.TestCase):
             ['/anonymous', '/c:/b:rw', 'vol:/x:ro']
         )
 
+    @mock.patch.dict(os.environ)
+    def test_volume_mode_override(self):
+        os.environ['COMPOSE_CONVERT_WINDOWS_PATHS'] = 'true'
+        base_file = config.ConfigFile(
+            'base.yaml',
+            {
+                'version': '2.3',
+                'services': {
+                    'web': {
+                        'image': 'example/web',
+                        'volumes': ['/c:/b:rw']
+                    }
+                },
+            }
+        )
+
+        override_file = config.ConfigFile(
+            'override.yaml',
+            {
+                'version': '2.3',
+                'services': {
+                    'web': {
+                        'volumes': ['/c:/b:ro']
+                    }
+                }
+            }
+        )
+        details = config.ConfigDetails('.', [base_file, override_file])
+        service_dicts = config.load(details).services
+        svc_volumes = list(map(lambda v: v.repr(), service_dicts[0]['volumes']))
+        assert svc_volumes == ['/c:/b:ro']
+
     def test_undeclared_volume_v2(self):
         base_file = config.ConfigFile(
             'base.yaml',
@@ -4018,7 +4050,7 @@ class VolumePathTest(unittest.TestCase):
     def test_split_path_mapping_with_windows_path(self):
         host_path = "c:\\Users\\msamblanet\\Documents\\anvil\\connect\\config"
         windows_volume_path = host_path + ":/opt/connect/config:ro"
-        expected_mapping = ("/opt/connect/config:ro", host_path)
+        expected_mapping = ("/opt/connect/config", (host_path, 'ro'))
 
         mapping = config.split_path_mapping(windows_volume_path)
         assert mapping == expected_mapping
@@ -4026,7 +4058,7 @@ class VolumePathTest(unittest.TestCase):
     def test_split_path_mapping_with_windows_path_in_container(self):
         host_path = 'c:\\Users\\remilia\\data'
         container_path = 'c:\\scarletdevil\\data'
-        expected_mapping = (container_path, host_path)
+        expected_mapping = (container_path, (host_path, None))
 
         mapping = config.split_path_mapping('{0}:{1}'.format(host_path, container_path))
         assert mapping == expected_mapping
@@ -4034,7 +4066,7 @@ class VolumePathTest(unittest.TestCase):
     def test_split_path_mapping_with_root_mount(self):
         host_path = '/'
         container_path = '/var/hostroot'
-        expected_mapping = (container_path, host_path)
+        expected_mapping = (container_path, (host_path, None))
         mapping = config.split_path_mapping('{0}:{1}'.format(host_path, container_path))
         assert mapping == expected_mapping