Merge branch 'garribas-5183-array-form-of-labels-unmarshall-error'

compose/config/config.py

@@ -707,16 +707,16 @@ def process_service(service_config):
     if 'build' in service_dict:
         if isinstance(service_dict['build'], six.string_types):
             service_dict['build'] = resolve_build_path(working_dir, service_dict['build'])
-        elif isinstance(service_dict['build'], dict) and 'context' in service_dict['build']:
-            path = service_dict['build']['context']
-            service_dict['build']['context'] = resolve_build_path(working_dir, path)
+        elif isinstance(service_dict['build'], dict):
+            if 'context' in service_dict['build']:
+                path = service_dict['build']['context']
+                service_dict['build']['context'] = resolve_build_path(working_dir, path)
+            if 'labels' in service_dict['build']:
+                service_dict['build']['labels'] = parse_labels(service_dict['build']['labels'])
 
     if 'volumes' in service_dict and service_dict.get('volume_driver') is None:
         service_dict['volumes'] = resolve_volume_paths(working_dir, service_dict)
 
-    if 'labels' in service_dict:
-        service_dict['labels'] = parse_labels(service_dict['labels'])
-
     if 'sysctls' in service_dict:
         service_dict['sysctls'] = build_string_dict(parse_sysctls(service_dict['sysctls']))
 
@@ -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):

compose/service.py

@@ -881,9 +881,12 @@ class Service(object):
 
     def get_secret_volumes(self):
         def build_spec(secret):
-            target = '{}/{}'.format(
-                const.SECRETS_PATH,
-                secret['secret'].target or secret['secret'].source)
+            target = secret['secret'].target
+            if target is None:
+                target = '{}/{}'.format(const.SECRETS_PATH, secret['secret'].source)
+            elif not os.path.isabs(target):
+                target = '{}/{}'.format(const.SECRETS_PATH, target)
+
             return VolumeSpec(secret['file'], target, 'ro')
 
         return [build_spec(secret) for secret in self.secrets]

tests/unit/config/config_test.py

@@ -892,7 +892,7 @@ class ConfigTest(unittest.TestCase):
         assert service['build']['args']['opt1'] == '42'
         assert service['build']['args']['opt2'] == 'foobar'
 
-    def test_load_with_build_labels(self):
+    def test_load_build_labels_dict(self):
         service = config.load(
             build_config_details(
                 {
@@ -919,6 +919,28 @@ class ConfigTest(unittest.TestCase):
         assert service['build']['labels']['label1'] == 42
         assert service['build']['labels']['label2'] == 'foobar'
 
+    def test_load_build_labels_list(self):
+        base_file = config.ConfigFile(
+            'base.yml',
+            {
+                'version': '2.3',
+                'services': {
+                    'web': {
+                        'build': {
+                            'context': '.',
+                            'labels': ['foo=bar', 'baz=true', 'foobar=1']
+                        },
+                    },
+                },
+            }
+        )
+
+        details = config.ConfigDetails('.', [base_file])
+        service = config.load(details).services[0]
+        assert service['build']['labels'] == {
+            'foo': 'bar', 'baz': 'true', 'foobar': '1'
+        }
+
     def test_build_args_allow_empty_properties(self):
         service = config.load(
             build_config_details(
@@ -1101,6 +1123,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 +4072,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 +4080,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 +4088,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
 

tests/unit/service_test.py

@@ -9,12 +9,14 @@ from .. import mock
 from .. import unittest
 from compose.config.errors import DependencyError
 from compose.config.types import ServicePort
+from compose.config.types import ServiceSecret
 from compose.config.types import VolumeFromSpec
 from compose.config.types import VolumeSpec
 from compose.const import LABEL_CONFIG_HASH
 from compose.const import LABEL_ONE_OFF
 from compose.const import LABEL_PROJECT
 from compose.const import LABEL_SERVICE
+from compose.const import SECRETS_PATH
 from compose.container import Container
 from compose.project import OneOffFilter
 from compose.service import build_ulimits
@@ -1089,3 +1091,56 @@ class ServiceVolumesTest(unittest.TestCase):
         self.assertEqual(
             self.mock_client.create_host_config.call_args[1]['binds'],
             [volume])
+
+
+class ServiceSecretTest(unittest.TestCase):
+    def setUp(self):
+        self.mock_client = mock.create_autospec(docker.APIClient)
+
+    def test_get_secret_volumes(self):
+        secret1 = {
+            'secret': ServiceSecret.parse({'source': 'secret1', 'target': 'b.txt'}),
+            'file': 'a.txt'
+        }
+        service = Service(
+            'web',
+            client=self.mock_client,
+            image='busybox',
+            secrets=[secret1]
+        )
+        volumes = service.get_secret_volumes()
+
+        assert volumes[0].external == secret1['file']
+        assert volumes[0].internal == '{}/{}'.format(SECRETS_PATH, secret1['secret'].target)
+
+    def test_get_secret_volumes_abspath(self):
+        secret1 = {
+            'secret': ServiceSecret.parse({'source': 'secret1', 'target': '/d.txt'}),
+            'file': 'c.txt'
+        }
+        service = Service(
+            'web',
+            client=self.mock_client,
+            image='busybox',
+            secrets=[secret1]
+        )
+        volumes = service.get_secret_volumes()
+
+        assert volumes[0].external == secret1['file']
+        assert volumes[0].internal == secret1['secret'].target
+
+    def test_get_secret_volumes_no_target(self):
+        secret1 = {
+            'secret': ServiceSecret.parse({'source': 'secret1'}),
+            'file': 'c.txt'
+        }
+        service = Service(
+            'web',
+            client=self.mock_client,
+            image='busybox',
+            secrets=[secret1]
+        )
+        volumes = service.get_secret_volumes()
+
+        assert volumes[0].external == secret1['file']
+        assert volumes[0].internal == '{}/{}'.format(SECRETS_PATH, secret1['secret'].source)