Repository: JonathonReinhart/scuba

Issue:
Mount additional volumes
Would it make sense to allow mounting additional directories through `.scuba.yml`, or is it already possible to do so?

My use case is that I would like to run tools to fetch dependencies (like `yarn`, `maven`, `composer`, ...). In order to access private repositories through SSH, one easy solution would be to bind-mount `$SSH_AUTH_SOCK` into the container.

Also, depending on the tool, it might be valuable to mount cache directories for `yarn` or `maven` into the build container.

Example:

```yaml
image: none

environment:
  SSH_AUTH_SOCK: /tmp/ssh-auth.sock

aliases:
  yarn:
    image: node:12-alpine
    mount:
      - $SSH_AUTH_SOCK:/tmp/ssh-auth.sock
      - $HOME/.cache/yarn:/root/.cache/yarn
```

Does that make sense and if so, could you guide me as to which changes are probably necessary? I am not a Python programmer. In particular, it would be helpful to know how I could run a locally checked-out version of `scuba` from inside my build directory.


Discussion:
github-actions[bot]: ## Unit Test Results
    4 files      4 suites   1m 37s :stopwatch:
145 tests 143 :heavy_check_mark: 2 :zzz: 0 :x:
580 runs  572 :heavy_check_mark: 8 :zzz: 0 :x:

Results for commit cb100607.


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2f8e106..1cc0f40 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,11 @@
 All notable changes to this project will be documented in this file.
 This project adheres to [Semantic Versioning](http://semver.org/).
 
+## [Unreleased]
+### Added
+- Add ability to specify volumes in `.scuba.yml` (#186)
+
+
 ## [2.8.0] - 2021-08-18
 ### Added
 - Add ability to specify additional docker arguments in `.scuba.yml` (#177)
diff --git a/docs/configuration.rst b/docs/configuration.rst
index f660173..e2f44f7 100644
--- a/docs/configuration.rst
+++ b/docs/configuration.rst
@@ -93,6 +93,30 @@ style <https://yaml.org/spec/1.2/spec.html#id2788097>`_:
 
     docker_args: '--privileged -v "/tmp/hello world:/tmp/hello world"'
 
+``volumes``
+-----------
+The optional ``volumes`` node allows additional `volumes
+<https://docs.docker.com/storage/volumes/>`_ or bind-mounts to be specified.
+``volumes`` is a mapping (dictionary) where each key is the container-path.
+In the simple form, the value is a string, the host-path to be bind-mounted:
+
+.. code-block:: yaml
+
+    volumes:
+      /var/lib/foo: /host/foo
+
+In the complex form, the value is a mapping which must contain a ``hostpath``
+subkey. It can also contain an ``options`` subkey with a comma-separated list
+of volume options:
+
+.. code-block:: yaml
+
+    volumes:
+      /var/lib/foo:
+        hostpath: /host/foo
+        options: ro,cached
+
+
 
 .. _conf_aliases:
 
@@ -191,6 +215,23 @@ alias:
         script:
           - ls -l /tmp/
 
+
+Aliases can extend or override the top-level ``volumes``:
+
+.. code-block:: yaml
+
+    volumes:
+      /var/lib/foo: /host/foo
+    aliases:
+      example:
+        volumes:
+          /var/lib/foo: /example/foo
+          /var/lib/bar: /example/bar
+        script:
+          - ls -l /var/lib/foo /var/lib/bar
+
+
+
 ``hooks``
 ---------
 
diff --git a/scuba/config.py b/scuba/config.py
index 0cf0de5..45039d6 100644
--- a/scuba/config.py
+++ b/scuba/config.py
@@ -6,6 +6,7 @@ import shlex
 
 from .constants import *
 from .utils import *
+from .dockerutil import make_vol_opt
 
 class ConfigError(Exception):
     pass
@@ -239,9 +240,78 @@ def _get_docker_args(data):
 
     return args
 
+def _get_typed_val(data, key, type_):
+    v = data.get(key)
+    if v is not None and not isinstance(v, type_):
+        raise ConfigError("'{}' must be a {}, not {}".format(
+                key, type_.__name__, type(v).__name__))
+    return v
+
+def _get_dict(data, key):
+    return _get_typed_val(data, key, dict)
+
+def _get_delimited_str_list(data, key, sep):
+    s = _get_typed_val(data, key, str)
+    return s.split(sep) if s else []
+
+def _get_volumes(data):
+    voldata = _get_dict(data, 'volumes')
+    if voldata is None:
+        return None
+
+    vols = {}
+    for cpath, v in voldata.items():
+        vols[cpath] = ScubaVolume.from_dict(cpath, v)
+    return vols
+
+class ScubaVolume:
+    def __init__(self, container_path, host_path=None, options=None):
+        self.container_path = container_path
+        self.host_path = host_path
+        self.options = options or []
+
+    @classmethod
+    def from_dict(cls, cpath, node):
+        # Treat a null node as an empty dict
+        if node is None:
+            node = {}
+
+        # Simple form:
+        # volumes:
+        #   /foo: /host/foo
+        if isinstance(node, str):
+            return cls(
+                container_path = cpath,
+                host_path = node,
+                )
+
+        # Complex form
+        # volumes:
+        #   /foo:
+        #     hostpath: /host/foo
+        #     options: ro,z
+        if isinstance(node, dict):
+            hpath = node.get('hostpath')
+            if hpath is None:
+                raise ConfigError("Volume {} must have a 'hostpath' subkey".format(cpath))
+            return cls(
+                container_path = cpath,
+                host_path = hpath,
+                options = _get_delimited_str_list(node, 'options', ','),
+                )
+
+        raise ConfigError("{}: must be string or dict".format(cpath))
+
+    def get_vol_opt(self):
+        if not self.host_path:
+            raise NotImplementedError("No anonymous volumes for now")
+        return make_vol_opt(self.host_path, self.container_path, self.options)
+
+
 class ScubaAlias:
     def __init__(self, name, script, image=None, entrypoint=None,
-            environment=None, shell=None, as_root=None, docker_args=None):
+            environment=None, shell=None, as_root=None, docker_args=None,
+            volumes=None):
         self.name = name
         self.script = script
         self.image = image
@@ -250,6 +320,7 @@ class ScubaAlias:
         self.shell = shell
         self.as_root = bool(as_root)
         self.docker_args = docker_args
+        self.volumes = volumes
 
     @classmethod
     def from_dict(cls, name, node):
@@ -267,6 +338,7 @@ class ScubaAlias:
                 shell = node.get('shell'),
                 as_root = node.get('root'),
                 docker_args = _get_docker_args(node),
+                volumes = _get_volumes(node),
                 )
 
         return cls(name=name, script=script)
@@ -274,7 +346,10 @@ class ScubaAlias:
 
 class ScubaConfig:
     def __init__(self, **data):
-        optional_nodes = ('image','aliases','hooks','entrypoint','environment','shell','docker_args')
+        optional_nodes = (
+            'image', 'aliases', 'hooks', 'entrypoint', 'environment', 'shell',
+            'docker_args', 'volumes',
+        )
 
         # Check for unrecognized nodes
         extra = [n for n in data if not n in optional_nodes]
@@ -286,6 +361,7 @@ class ScubaConfig:
         self.shell = data.get('shell', DEFAULT_SHELL)
         self.entrypoint = _get_entrypoint(data)
         self.docker_args = _get_docker_args(data)
+        self.volumes = _get_volumes(data)
         self._load_aliases(data)
         self._load_hooks(data)
         self._load_environment(data)
diff --git a/scuba/scuba.py b/scuba/scuba.py
index 2f642a9..e33489f 100644
--- a/scuba/scuba.py
+++ b/scuba/scuba.py
@@ -129,6 +129,7 @@ class ScubaDive:
         writescl('script', self.context.script)
         writescl('image', self.context.image)
         writelist('docker_args', self.context.docker_args)
+        writelist('volumes', self.context.volumes)
 
         return s.getvalue()
 
@@ -318,6 +319,9 @@ class ScubaDive:
         for hostpath, contpath, options in self.__get_vol_opts():
             args.append(make_vol_opt(hostpath, contpath, options))
 
+        for _, vol in self.context.volumes.items():
+            args.append(vol.get_vol_opt())
+
         if self.workdir:
             args += ['-w', self.workdir]
 
@@ -341,7 +345,7 @@ class ScubaDive:
 
 
 class ScubaContext:
-    def __init__(self, image=None, script=None, entrypoint=None, environment=None, shell=None, docker_args=None):
+    def __init__(self, image=None, script=None, entrypoint=None, environment=None, shell=None, docker_args=None, volumes=None):
         self.image = image
         self.script = script
         self.as_root = False
@@ -349,6 +353,7 @@ class ScubaContext:
         self.environment = environment
         self.shell = shell
         self.docker_args = docker_args
+        self.volumes = volumes or {}
 
     @classmethod
     def process_command(cls, cfg, command, image=None, shell=None):
@@ -369,6 +374,7 @@ class ScubaContext:
                 environment = cfg.environment.copy(),
                 shell = cfg.shell,
                 docker_args = cfg.docker_args,
+                volumes = cfg.volumes,
                 )
 
         if command:
@@ -393,6 +399,9 @@ class ScubaContext:
                 elif alias.docker_args is not None:
                     result.docker_args.extend(alias.docker_args)
 
+                if alias.volumes is not None:
+                    result.volumes.update(alias.volumes)
+
                 # Merge/override the environment
                 if alias.environment:
                     result.environment.update(alias.environment)


Test patch:
diff --git a/tests/test_config.py b/tests/test_config.py
index b59b258..fc93eef 100644
--- a/tests/test_config.py
+++ b/tests/test_config.py
@@ -726,3 +726,160 @@ class TestConfig:
         config = scuba.config.load_config('.scuba.yml')
         assert config.aliases['testalias'].docker_args == ['-v', '/tmp/:/tmp/']
         assert isinstance(config.aliases['testalias'].docker_args, scuba.config.OverrideMixin)
+
+    ############################################################################
+    # volumes
+
+    def test_volumes_not_set(self):
+        '''volumes can be missing'''
+        with open('.scuba.yml', 'w') as f:
+            f.write(r'''
+                image: na
+                ''')
+
+        config = scuba.config.load_config('.scuba.yml')
+        assert config.volumes is None
+
+    def test_volumes_null(self):
+        '''volumes can be set to null'''
+        with open('.scuba.yml', 'w') as f:
+            f.write(r'''
+                image: na
+                volumes:
+                ''')
+
+        config = scuba.config.load_config('.scuba.yml')
+        assert config.volumes == None
+
+    def test_volumes_invalid(self):
+        '''volumes of incorrect type raises ConfigError'''
+        with open('.scuba.yml', 'w') as f:
+            f.write(r'''
+                image: na
+                volumes: 666
+                ''')
+
+        self._invalid_config('must be a dict')
+
+    def test_volumes_invalid_volume_type(self):
+        '''volume of incorrect type (list) raises ConfigError'''
+        with open('.scuba.yml', 'w') as f:
+            f.write(r'''
+                image: na
+                volumes:
+                  /foo:
+                    - a list makes no sense
+                ''')
+
+        self._invalid_config('must be string or dict')
+
+    def test_volumes_null_volume_type(self):
+        '''volume of None type raises ConfigError'''
+        # NOTE: In the future, we might want to support this as a volume
+        #       (non-bindmount, e.g. '-v /somedata'), or as tmpfs
+        with open('.scuba.yml', 'w') as f:
+            f.write(r'''
+                image: na
+                volumes:
+                  /bar:
+                ''')
+
+        self._invalid_config('hostpath')
+
+    def test_volume_as_dict_missing_hostpath(self):
+        '''volume of incorrect type raises ConfigError'''
+        # NOTE: In the future, we might want to support this as a volume
+        #       (non-bindmount, e.g. '-v /somedata'), or as tmpfs
+        with open('.scuba.yml', 'w') as f:
+            f.write(r'''
+                image: na
+                volumes:
+                  /bar:
+                    options: hostpath,is,missing
+                ''')
+
+        self._invalid_config('hostpath')
+
+    def test_volumes_simple_volume(self):
+        '''volumes can be set using the simple form'''
+        with open('.scuba.yml', 'w') as f:
+            f.write(r'''
+                image: na
+                volumes:
+                  /cpath: /hpath
+                ''')
+
+        config = scuba.config.load_config('.scuba.yml')
+        assert len(config.volumes) == 1
+
+        v = config.volumes['/cpath']
+        assert v.container_path == '/cpath'
+        assert v.host_path == '/hpath'
+
+    def test_volumes_complex(self):
+        '''volumes can be set using the complex form'''
+        with open('.scuba.yml', 'w') as f:
+            f.write(r'''
+                image: na
+                volumes:
+                  /foo: /host/foo
+                  /bar:
+                    hostpath: /host/bar
+                  /snap:
+                    hostpath: /host/snap
+                    options: z,ro
+                ''')
+
+        config = scuba.config.load_config('.scuba.yml')
+        vols = config.volumes
+        assert len(vols) == 3
+
+        v = vols['/foo']
+        assert isinstance(v, scuba.config.ScubaVolume)
+        assert v.container_path == '/foo'
+        assert v.host_path == '/host/foo'
+        assert v.options == []
+
+        v = vols['/bar']
+        assert isinstance(v, scuba.config.ScubaVolume)
+        assert v.container_path == '/bar'
+        assert v.host_path == '/host/bar'
+        assert v.options == []
+
+        v = vols['/snap']
+        assert isinstance(v, scuba.config.ScubaVolume)
+        assert v.container_path == '/snap'
+        assert v.host_path == '/host/snap'
+        assert v.options == ['z', 'ro']
+
+    def test_alias_volumes_set(self):
+        '''docker_args can be set via alias'''
+        with open('.scuba.yml', 'w') as f:
+            f.write(r'''
+                image: na
+                aliases:
+                  testalias:
+                    script:
+                      - ugh
+                    volumes:
+                      /foo: /host/foo
+                      /bar:
+                        hostpath: /host/bar
+                        options: z,ro
+                ''')
+
+        config = scuba.config.load_config('.scuba.yml')
+        vols = config.aliases['testalias'].volumes
+        assert len(vols) == 2
+
+        v = vols['/foo']
+        assert isinstance(v, scuba.config.ScubaVolume)
+        assert v.container_path == '/foo'
+        assert v.host_path == '/host/foo'
+        assert v.options == []
+
+        v = vols['/bar']
+        assert isinstance(v, scuba.config.ScubaVolume)
+        assert v.container_path == '/bar'
+        assert v.host_path == '/host/bar'
+        assert v.options == ['z', 'ro']
diff --git a/tests/test_main.py b/tests/test_main.py
index 2c19f58..668ad44 100644
--- a/tests/test_main.py
+++ b/tests/test_main.py
@@ -875,3 +875,75 @@ class TestMain:
                 '''.format(image=DOCKER_IMAGE))
         out, _ = self.run_scuba(['--shell', '/bin/bash', 'shell_check'])
         assert_str_equalish("/bin/bash", out)
+
+    ############################################################################
+    # Volumes
+
+    def test_volumes_basic(self):
+        '''Verify volumes can be added at top-level and alias'''
+
+        # Create some temporary directories with a file in each
+        topdata = Path('./topdata')
+        topdata.mkdir()
+        (topdata / "thing").write_text("from the top\n")
+
+        aliasdata = Path('./aliasdata')
+        aliasdata.mkdir()
+        (aliasdata / "thing").write_text("from the alias\n")
+
+        with open('.scuba.yml', 'w') as f:
+            f.write('''
+                image: {image}
+                volumes:
+                  /topdata: {topdata_dir}
+                aliases:
+                  doit:
+                    volumes:
+                      /aliasdata: {aliasdata_dir}
+                    script: "cat /topdata/thing /aliasdata/thing"
+                '''.format(image = DOCKER_IMAGE,
+                           topdata_dir = topdata.absolute(),
+                           aliasdata_dir = aliasdata.absolute(),
+                           ))
+
+        out, _ = self.run_scuba(['doit'])
+        out = out.splitlines()
+        assert out == ["from the top", "from the alias"]
+
+
+    def test_volumes_alias_override(self):
+        '''Verify volumes can be overridden by an alias'''
+
+        # Create some temporary directories with a file in each
+        topdata = Path('./topdata')
+        topdata.mkdir()
+        (topdata / "thing").write_text("from the top\n")
+
+        aliasdata = Path('./aliasdata')
+        aliasdata.mkdir()
+        (aliasdata / "thing").write_text("from the alias\n")
+
+        with open('.scuba.yml', 'w') as f:
+            f.write('''
+                image: {image}
+                volumes:
+                  /data: {topdata_dir}
+                aliases:
+                  doit:
+                    volumes:
+                      /data: {aliasdata_dir}
+                    script: "cat /data/thing"
+                '''.format(image = DOCKER_IMAGE,
+                           topdata_dir = topdata.absolute(),
+                           aliasdata_dir = aliasdata.absolute(),
+                           ))
+
+        # Run a non-alias command
+        out, _ = self.run_scuba(['cat', '/data/thing'])
+        out = out.splitlines()
+        assert out == ["from the top"]
+
+        # Run the alias
+        out, _ = self.run_scuba(['doit'])
+        out = out.splitlines()
+        assert out == ["from the alias"]
diff --git a/tests/test_scuba.py b/tests/test_scuba.py
index 9a9b748..57351b0 100644
--- a/tests/test_scuba.py
+++ b/tests/test_scuba.py
@@ -298,3 +298,63 @@ class TestScubaContext:
             )
         result = ScubaContext.process_command(cfg, ['apple'])
         assert result.docker_args == ['--privileged']
+
+
+    ############################################################################
+    # volumes
+
+    def test_process_command_alias_extends_volumes(self):
+        '''aliases can extend the volumes'''
+        cfg = ScubaConfig(
+                image = 'default',
+                volumes = {
+                    '/foo': '/host/foo',
+                },
+                aliases = dict(
+                    apple = dict(
+                        script = [
+                            'banana cherry "pie is good"',
+                        ],
+                        volumes = {
+                            '/bar': '/host/bar',
+                        },
+                    ),
+                ),
+            )
+        result = ScubaContext.process_command(cfg, ['apple'])
+        vols = result.volumes
+        assert len(vols) == 2
+
+        v = vols['/foo']
+        assert v.container_path == '/foo'
+        assert v.host_path == '/host/foo'
+
+        v = vols['/bar']
+        assert v.container_path == '/bar'
+        assert v.host_path == '/host/bar'
+
+    def test_process_command_alias_updates_volumes(self):
+        '''aliases can extend the volumes'''
+        cfg = ScubaConfig(
+                image = 'default',
+                volumes = {
+                    '/foo': '/host/foo',
+                },
+                aliases = dict(
+                    apple = dict(
+                        script = [
+                            'banana cherry "pie is good"',
+                        ],
+                        volumes = {
+                            '/foo': '/alternate/foo',
+                        },
+                    ),
+                ),
+            )
+        result = ScubaContext.process_command(cfg, ['apple'])
+        vols = result.volumes
+        assert len(vols) == 1
+
+        v = vols['/foo']
+        assert v.container_path == '/foo'
+        assert v.host_path == '/alternate/foo'