Steal config files on 'job --pass-config' (#1361)

CHANGELOG.D/1361.feature

@@ -0,0 +1 @@
+Steal config files on `neuro job --pass-config`.

docs/functions_reference.rst

@@ -78,6 +78,14 @@ Config Factory
    configuration directory (``~/.nmrc`` by default). The default value can be overridden
    by ``NEUROMATION_CONFIG`` environment variable.
 
+   .. attribute:: path
+
+      Revealed path to the configuration directory, expanded as described above.
+
+      Read-only :class:`pathlib.Path` property.
+
+      .. versionadded:: 20.2.25
+
    .. comethod:: get(*, timeout: aiohttp.ClientTimeout = DEFAULT_TIMEOUT) -> Client
 
       Read configuration previously created by *login methods* and return a client

neuromation/api/__init__.py

@@ -24,7 +24,6 @@
     CONFIG_ENV_NAME,
     DEFAULT_API_URL,
     DEFAULT_CONFIG_PATH,
-    TRUSTED_CONFIG_PATH,
     Factory,
 )
 from .core import (

neuromation/api/config_factory.py

@@ -40,7 +40,6 @@
 WIN32 = sys.platform == "win32"
 DEFAULT_CONFIG_PATH = "~/.neuro"
 CONFIG_ENV_NAME = "NEUROMATION_CONFIG"
-TRUSTED_CONFIG_PATH = "NEUROMATION_TRUSTED_CONFIG_PATH"
 DEFAULT_API_URL = URL("https://staging.neu.ro/api/v1")
 
 
@@ -81,6 +80,10 @@ def __init__(
             self._trace_configs += trace_configs
         self._trace_id = trace_id
 
+    @property
+    def path(self) -> Path:
+        return self._path
+
     async def get(self, *, timeout: aiohttp.ClientTimeout = DEFAULT_TIMEOUT) -> Client:
         saved_config = config = self._read()
         session = await _make_session(timeout, self._trace_configs)
@@ -220,8 +223,7 @@ def _read(self) -> _Config:
                 "Please logout and login again."
             )
 
-        trusted_env = WIN32 or bool(os.environ.get(TRUSTED_CONFIG_PATH))
-        if not trusted_env:
+        if not WIN32:
             stat_dir = self._path.stat()
             if stat_dir.st_mode & 0o777 != 0o700:
                 raise ConfigError(

neuromation/cli/job.py

@@ -17,8 +17,6 @@
 from yarl import URL
 
 from neuromation.api import (
-    CONFIG_ENV_NAME,
-    TRUSTED_CONFIG_PATH,
     AuthorizationError,
     Client,
     Container,
@@ -55,6 +53,7 @@
     JOB_NAME,
     LOCAL_REMOTE_PORT,
     MEGABYTE,
+    NEURO_STEAL_CONFIG,
     AsyncExitStack,
     ImageType,
     alias,
@@ -956,13 +955,11 @@ async def run_job(
     volumes = await _build_volumes(root, volume, env_dict)
 
     if pass_config:
-        if CONFIG_ENV_NAME in env_dict:
-            raise ValueError(
-                f"{CONFIG_ENV_NAME} is already set to {env_dict[CONFIG_ENV_NAME]}"
-            )
+        env_name = NEURO_STEAL_CONFIG
+        if env_name in env_dict:
+            raise ValueError(f"{env_name} is already set to {env_dict[env_name]}")
         env_var, secret_volume = await upload_and_map_config(root)
-        env_dict[CONFIG_ENV_NAME] = env_var
-        env_dict[TRUSTED_CONFIG_PATH] = "1"
+        env_dict[NEURO_STEAL_CONFIG] = env_var
         volumes.add(secret_volume)
 
     if volumes:
@@ -1093,16 +1090,20 @@ async def upload_and_map_config(root: Root) -> Tuple[str, Volume]:
 
     # store the Neuro CLI config on the storage under some random path
     nmrc_path = URL(root.config_path.expanduser().resolve().as_uri())
-    random_nmrc_filename = f"{uuid.uuid4()}-nmrc"
-    storage_nmrc_folder = URL(f"storage://{root.client.username}/nmrc/")
+    random_nmrc_filename = f"{uuid.uuid4()}-cfg"
+    storage_nmrc_folder = URL(f"storage://{root.client.username}/.neuro/")
     storage_nmrc_path = storage_nmrc_folder / random_nmrc_filename
-    local_nmrc_folder = f"{STORAGE_MOUNTPOINT}/nmrc/"
+    local_nmrc_folder = f"{STORAGE_MOUNTPOINT}/.neuro/"
     local_nmrc_path = f"{local_nmrc_folder}{random_nmrc_filename}"
     if not root.quiet:
         click.echo(f"Temporary config file created on storage: {storage_nmrc_path}.")
         click.echo(f"Inside container it will be available at: {local_nmrc_path}.")
     await root.client.storage.mkdir(storage_nmrc_folder, parents=True, exist_ok=True)
-    await root.client.storage.upload_dir(nmrc_path, storage_nmrc_path)
+
+    async def skip_tmp(fname: str) -> bool:
+        return not fname.endswith(("-shm", "-wal", "-journal"))
+
+    await root.client.storage.upload_dir(nmrc_path, storage_nmrc_path, filter=skip_tmp)
     # specify a container volume and mount the storage path
     # into specific container path
     return (

neuromation/cli/main.py

@@ -40,6 +40,7 @@
     option,
     pager_maybe,
     print_help,
+    steal_config_maybe,
 )
 
 
@@ -367,6 +368,7 @@ def cli(
             option = "--hide-token" if hide_token else "--no-hide-token"
             raise click.UsageError(f"{option} requires --trace")
         hide_token_bool = hide_token
+    steal_config_maybe(Path(neuromation_config))
     root = Root(
         verbosity=verbosity,
         color=real_color,

neuromation/cli/utils.py

@@ -4,6 +4,8 @@
 import inspect
 import itertools
 import logging
+import os
+import pathlib
 import re
 import shlex
 import shutil
@@ -69,6 +71,8 @@
 JOB_NAME_PATTERN = "^[a-z](?:-?[a-z0-9])*$"
 JOB_NAME_REGEX = re.compile(JOB_NAME_PATTERN)
 
+NEURO_STEAL_CONFIG = "NEURO_STEAL_CONFIG"
+
 
 def warn_if_has_newer_version(
     version: _PyPIVersion,
@@ -741,3 +745,17 @@ def pager_maybe(
         click.echo_via_pager(
             itertools.chain(["\n".join(handled)], (f"\n{line}" for line in lines_it))
         )
+
+
+def steal_config_maybe(dst_path: pathlib.Path) -> None:
+    if NEURO_STEAL_CONFIG in os.environ:
+        src = pathlib.Path(os.environ[NEURO_STEAL_CONFIG])
+        dst = Factory(dst_path).path
+        dst.mkdir(mode=0o700)
+        for f in src.iterdir():
+            target = dst / f.name
+            shutil.copy(f, target)
+            # forbid access to other users
+            os.chmod(target, 0o600)
+            f.unlink()
+        src.rmdir()

tests/api/test_config_factory.py

@@ -14,7 +14,7 @@
 
 import neuromation
 import neuromation.api.config_factory
-from neuromation.api import TRUSTED_CONFIG_PATH, Cluster, ConfigError, Factory
+from neuromation.api import Cluster, ConfigError, Factory
 from neuromation.api.config import (
     _AuthConfig,
     _AuthToken,
@@ -238,26 +238,6 @@ async def test_file_permissions(self, config_dir: Path) -> None:
         with pytest.raises(ConfigError, match=r"permission"):
             await Factory().get()
 
-    @pytest.mark.skipif(
-        sys.platform == "win32",
-        reason="Windows does not supports UNIX-like permissions",
-    )
-    async def test_file_permissions_suppress_security_check(
-        self,
-        tmpdir: Path,
-        token: str,
-        auth_config: _AuthConfig,
-        cluster_config: Cluster,
-        monkeypatch: Any,
-    ) -> None:
-        monkeypatch.setenv(TRUSTED_CONFIG_PATH, "1")
-        config_path = Path(tmpdir) / "test.nmrc"
-        _create_config(config_path, token, auth_config, cluster_config)
-        (config_path / "db").chmod(0o644)
-        client = await Factory(config_path).get()
-        await client.close()
-        assert client
-
     async def test_silent_update(
         self, config_dir: Path, mock_for_login: _TestServer
     ) -> None:

tests/cli/conftest.py

@@ -91,15 +91,19 @@ def _run_cli(arguments: List[str]) -> SysCapWithCode:
 
         code = EX_OK
         try:
-            main(
-                [
-                    "--show-traceback",
-                    "--disable-pypi-version-check",
-                    "--color=no",
-                    f"--neuromation-config={nmrc_path}",
-                ]
-                + arguments
-            )
+            default_args = [
+                "--show-traceback",
+                "--disable-pypi-version-check",
+                "--color=no",
+            ]
+            if "--neuromation-config" not in arguments:
+                for arg in arguments:
+                    if arg.startswith("--neuromation-config="):
+                        break
+                else:
+                    default_args.append(f"--neuromation-config={nmrc_path}")
+
+            main(default_args + arguments)
         except SystemExit as e:
             code = e.code
             pass

tests/cli/test_main.py

@@ -0,0 +1,30 @@
+import shutil
+from pathlib import Path
+from typing import Any, Callable, List
+
+from neuromation.cli.utils import NEURO_STEAL_CONFIG
+
+from .conftest import SysCapWithCode
+
+
+_RunCli = Callable[[List[str]], SysCapWithCode]
+
+
+def test_steal_config(
+    nmrc_path: Path, run_cli: _RunCli, tmp_path: Path, monkeypatch: Any
+) -> None:
+    folder = shutil.move(nmrc_path, tmp_path / "orig-cfg")
+    monkeypatch.setenv(NEURO_STEAL_CONFIG, str(folder))
+    ret = run_cli(["--neuromation-config", str(tmp_path / ".neuro"), "config", "show"])
+    assert ret.code == 0, ret
+
+
+def test_steal_config_dont_override_existing(
+    nmrc_path: Path, run_cli: _RunCli, tmp_path: Path, monkeypatch: Any
+) -> None:
+    folder = shutil.copytree(nmrc_path, tmp_path / "orig-cfg")
+    monkeypatch.setenv(NEURO_STEAL_CONFIG, str(folder))
+    ret = run_cli(["--neuromation-config", str(nmrc_path), "config", "show"])
+    # FileExistsError
+    assert ret.code == 74, ret
+    assert "FileExistsError" in ret.err

tests/e2e/test_e2e_jobs.py

@@ -638,7 +638,7 @@ def test_pass_config(helper: Helper) -> None:
             "--no-wait-start",
             "--pass-config",
             UBUNTU_IMAGE_NAME,
-            'bash -c "sleep 15 && test -f $(NEUROMATION_CONFIG)/db"',
+            'bash -c "sleep 15 && test -f $(NEURO_STEAL_CONFIG)/db"',
         ]
     )
     job_id = captured.out