Merge pull request #250 from JonathonReinhart/248-restore-named-vols

Add explicit support for mounting named volumes

docs/configuration.rst

@@ -147,25 +147,40 @@ style <https://yaml.org/spec/1.2/spec.html#id2788097>`_:
 
 The optional ``volumes`` node *(added in v2.9.0)* allows additional
 `bind-mounts <https://docs.docker.com/storage/bind-mounts/>`_ to be specified.
+As of v2.13.0, `named volumes <https://docs.docker.com/storage/volumes/>`_
+are also supported.
+
 ``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:
+In the simple form, the value is a string, which can be:
+
+* An absolute or relative path which results in a bind-mount.
+* A Docker volume name.
 
 .. code-block:: yaml
+   :caption: Example of simple-form volumes
 
     volumes:
-      /var/lib/foo: /host/foo
-      /var/lib/bar: ./bar
+      /var/lib/foo: /host/foo  # bind-mount: absolute path
+      /var/lib/bar: ./bar      # bind-mount: path relative to .scuba.yml dir
+      /var/log: persist-logs   # named volume
+
+In the complex form, the value is a mapping with the following supported keys:
+
+* ``hostpath``: An absolute or relative path specifying a host bind-mount.
+* ``name``: The name of a named Docker volume.
+* ``options``: A comma-separated list of volume options.
 
-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:
+``hostpath`` and ``name`` are mutually-exclusive and one must be specified.
 
 .. code-block:: yaml
+   :caption: Example of complex-form volumes
 
     volumes:
       /var/lib/foo:
-        hostpath: /host/foo
+        hostpath: /host/foo   # bind-mount
         options: ro,cached
+      /var/log:
+        name: persist-logs    # named volume
 
 The paths (host or container) used in volume mappings can contain environment
 variables **which are expanded in the host environment**. For example, this
@@ -182,18 +197,32 @@ configuration error.
 
 Volume container paths must be absolute.
 
-Volume host paths can be absolute or relative. If a relative path is used, it
-is interpreted as relative to the directory in which ``.scuba.yml`` is found.
-To avoid ambiguity, relative paths must start with ``./`` or ``../``.
+Bind-mount host paths can be absolute or relative. If a relative path is used,
+it is interpreted as relative to the directory in which ``.scuba.yml`` is
+found.  To avoid ambiguity with a named volume, relative paths must start with
+``./`` or ``../``.
 
-Volume host directories which do not already exist are created as the current
-user before creating the container.
+Bind-mount host directories which do not already exist are created as the
+current user before creating the container.
 
 .. note::
    Because variable expansion is now applied to all volume paths, if one
    desires to use a literal ``$`` character in a path, it must be written as
    ``$$``.
 
+.. note::
+   Docker named volumes are created with ``drwxr-xr-x`` (0755) permissions.
+   If scuba is not run with ``--root``, the scuba user will be unable to write
+   to this directory. As a workaround, one can use a :ref:`root hook
+   <conf_hooks>` to change permissions on the directory.
+
+   .. code-block:: yaml
+
+      volumes:
+        /foo: foo-volume
+      hooks:
+        root: chmod 777 /foo
+
 
 .. _conf_aliases:
 

scuba/config.py

@@ -11,14 +11,16 @@
 import yaml.nodes
 
 from .constants import DEFAULT_SHELL, SCUBA_YML
-from .utils import expand_env_vars, parse_env_var
+from . import utils
 from .dockerutil import make_vol_opt
 
 CfgNode = Any
 CfgData = Dict[str, CfgNode]
 Environment = Dict[str, str]
 _T = TypeVar("_T")
 
+VOLUME_NAME_PATTERN = re.compile(r"^[a-zA-Z0-9][a-zA-Z0-9_.-]+$")
+
 
 class ConfigError(Exception):
     pass
@@ -190,6 +192,32 @@ def find_config() -> Tuple[Path, Path, ScubaConfig]:
             )
 
 
+def _expand_env_vars(in_str: str) -> str:
+    """Wraps utils.expand_env_vars() to convert errors
+
+    Args:
+      in_str: Input string.
+
+    Returns:
+      The input string with environment variables expanded.
+
+    Raises:
+      ConfigError: If a referenced environment variable is not set.
+      ConfigError: An environment variable reference could not be parsed.
+    """
+    try:
+        return utils.expand_env_vars(in_str)
+    except KeyError as err:
+        # pylint: disable=raise-missing-from
+        raise ConfigError(
+            f"Unset environment variable {err.args[0]!r} used in {in_str!r}"
+        )
+    except ValueError as ve:
+        raise ConfigError(
+            f"Unable to expand string '{in_str}' due to parsing errors"
+        ) from ve
+
+
 def _process_script_node(node: CfgNode, name: str) -> List[str]:
     """Process a script-type node
 
@@ -236,7 +264,7 @@ def _process_environment(node: CfgNode, name: str) -> Environment:
             result[k] = str(v)
     elif isinstance(node, list):
         for e in node:
-            k, v = parse_env_var(e)
+            k, v = utils.parse_env_var(e)
             result[k] = v
     else:
         raise ConfigError(
@@ -332,18 +360,16 @@ def _get_volumes(
 
     vols = {}
     for cpath_str, v in voldata.items():
-        cpath = _expand_path(cpath_str)  # no base_dir; container path must be absolute.
+        cpath_str = _expand_env_vars(cpath_str)
+        cpath = _absoluteify_path(cpath_str)  # container path must be absolute.
         vols[cpath] = ScubaVolume.from_dict(cpath, v, scuba_root)
     return vols
 
 
-def _expand_path(in_str: str, base_dir: Optional[Path] = None) -> Path:
-    """Expand variables in a path string and make it absolute.
-
-    Environment variable references (e.g. $FOO and ${FOO}) are expanded using
-    the host environment.
+def _absoluteify_path(in_str: str, base_dir: Optional[Path] = None) -> Path:
+    """Take a path string and make it absolute.
 
-    After environment variable expansion, absolute paths are returned as-is.
+    Absolute paths are returned as-is.
     Relative paths must start with ./ or ../ and are joined to base_dir, if
     provided.
 
@@ -356,26 +382,13 @@ def _expand_path(in_str: str, base_dir: Optional[Path] = None) -> Path:
 
     Raises:
       ValueError: If base_dir is provided but not absolute.
-      ConfigError: If a referenced environment variable is not set.
-      ConfigError: An environment variable reference could not be parsed.
       ConfigError: A relative path does not start with "./" or "../".
       ConfigError: A relative path is given when base_dir is not provided.
     """
     if base_dir is not None and not base_dir.is_absolute():
         raise ValueError(f"base_dir is not absolute: {base_dir}")
 
-    try:
-        path_str = expand_env_vars(in_str)
-    except KeyError as ke:
-        # pylint: disable=raise-missing-from
-        raise ConfigError(
-            f"Unset environment variable {ke.args[0]!r} used in {in_str!r}"
-        )
-    except ValueError as ve:
-        raise ConfigError(
-            f"Unable to expand string '{in_str}' due to parsing errors"
-        ) from ve
-
+    path_str = _expand_env_vars(in_str)
     path = Path(path_str)
 
     if not path.is_absolute():
@@ -399,9 +412,14 @@ def _expand_path(in_str: str, base_dir: Optional[Path] = None) -> Path:
 @dataclasses.dataclass(frozen=True)
 class ScubaVolume:
     container_path: Path
-    host_path: Path  # TODO: Optional for anonymous volume
+    host_path: Optional[Path] = None
+    volume_name: Optional[str] = None
     options: List[str] = dataclasses.field(default_factory=list)
 
+    def __post_init__(self) -> None:
+        if sum(bool(x) for x in (self.host_path, self.volume_name)) != 1:
+            raise ValueError("Exactly one of host_path, volume_name must be set")
+
     @classmethod
     def from_dict(
         cls, cpath: Path, node: CfgNode, scuba_root: Optional[Path]
@@ -412,32 +430,69 @@ def from_dict(
 
         # Simple form:
         # volumes:
-        #   /foo: /host/foo
+        #   /foo: foo-volume  # volume name
+        #   /bar: /host/bar   # absolute path
+        #   /snap: ./snap     # relative path
         if isinstance(node, str):
+            node = _expand_env_vars(node)
+
+            # Absolute or relative path
+            valid_prefixes = ("/", "./", "../")
+            if any(node.startswith(pfx) for pfx in valid_prefixes):
+                return cls(
+                    container_path=cpath,
+                    host_path=_absoluteify_path(node, scuba_root),
+                )
+
+            # Volume name
+            if not VOLUME_NAME_PATTERN.match(node):
+                raise ConfigError(f"Invalid volume name: {node!r}")
             return cls(
                 container_path=cpath,
-                host_path=_expand_path(node, scuba_root),
+                volume_name=node,
             )
 
         # Complex form
         # volumes:
         #   /foo:
         #     hostpath: /host/foo
         #     options: ro,z
+        #   /bar:
+        #     name: bar-volume
         if isinstance(node, dict):
             hpath = node.get("hostpath")
-            if hpath is None:
-                raise ConfigError(f"Volume {cpath} must have a 'hostpath' subkey")
-            return cls(
-                container_path=cpath,
-                host_path=_expand_path(hpath, scuba_root),
-                options=_get_delimited_str_list(node, "options", ","),
-            )
+            name = node.get("name")
+            options = _get_delimited_str_list(node, "options", ",")
+
+            if sum(bool(x) for x in (hpath, name)) != 1:
+                raise ConfigError(
+                    f"Volume {cpath} must have exactly one of"
+                    " 'hostpath' or 'name' subkey"
+                )
+
+            if hpath is not None:
+                hpath = _expand_env_vars(hpath)
+                return cls(
+                    container_path=cpath,
+                    host_path=_absoluteify_path(hpath, scuba_root),
+                    options=options,
+                )
+
+            if name is not None:
+                return cls(
+                    container_path=cpath,
+                    volume_name=_expand_env_vars(name),
+                    options=options,
+                )
 
         raise ConfigError(f"{cpath}: must be string or dict")
 
     def get_vol_opt(self) -> str:
-        return make_vol_opt(self.host_path, self.container_path, self.options)
+        if self.host_path:
+            return make_vol_opt(self.host_path, self.container_path, self.options)
+        if self.volume_name:
+            return make_vol_opt(self.volume_name, self.container_path, self.options)
+        raise Exception("host_path or volume_name must be set")
 
 
 @dataclasses.dataclass(frozen=True)

scuba/dockerutil.py

@@ -132,17 +132,22 @@ def get_image_entrypoint(image: str) -> Optional[Sequence[str]]:
 
 
 def make_vol_opt(
-    hostdir: Path,
+    hostdir_or_volname: Union[Path, str],
     contdir: Path,
     options: Optional[Sequence[str]] = None,
 ) -> str:
     """Generate a docker volume option"""
-    if not hostdir.is_absolute():
-        raise ValueError(f"hostdir not absolute: {hostdir}")
+    if isinstance(hostdir_or_volname, Path):
+        hostdir: Path = hostdir_or_volname
+        if not hostdir.is_absolute():
+            # NOTE: As of Docker Engine version 23, you can use relative paths
+            # on the host. But we have no minimum Docker version, so we don't
+            # rely on this.
+            raise ValueError(f"hostdir not absolute: {hostdir}")
     if not contdir.is_absolute():
         raise ValueError(f"contdir not absolute: {contdir}")
 
-    vol = f"--volume={hostdir}:{contdir}"
+    vol = f"--volume={hostdir_or_volname}:{contdir}"
     if options:
         assert not isinstance(options, str)
         vol += ":" + ",".join(options)

scuba/scuba.py

@@ -165,7 +165,7 @@ def try_create_volumes(self) -> None:
             return
 
         for vol in self.context.volumes.values():
-            if vol.host_path.exists():
+            if vol.host_path is None or vol.host_path.exists():
                 continue
 
             try: