Fix CLI code and tests for user with org (#2748)

* Disks, buckets and secrets are now created with the current organization instead of no organization if `--org` is not explicitly specified. (#2756)
* Fix formatting URIs in short form for users with organization. (#2749)
* Fix E2E tests for users with organization. (#2747)

CHANGELOG.D/2749.bugfix

@@ -0,0 +1 @@
+Fix formatting URIs in short form for users with organization.

CHANGELOG.D/2756.bugfix

@@ -0,0 +1 @@
+Disks, buckets and secrets are now created with the current organization instead of no organization if `--org` is not explicitly specified.

neuro-cli/src/neuro_cli/blob_storage.py

@@ -42,7 +42,7 @@
     uri_formatter,
 )
 from neuro_cli.parse_utils import parse_timedelta
-from neuro_cli.utils import format_size
+from neuro_cli.utils import format_size, parse_org_name
 
 from .const import EX_OSFILE
 from .formatters.blob_storage import (
@@ -144,10 +144,11 @@ async def mkbucket(
     """
     Create a new bucket.
     """
+    org_name = parse_org_name(org, root)
     bucket = await root.client.buckets.create(
         name=name,
         cluster_name=cluster,
-        org_name=org,
+        org_name=org_name,
     )
     bucket_fmtr = BucketFormatter(
         str, datetime_formatter=get_datetime_formatter(root.iso_datetime_format)
@@ -273,6 +274,7 @@ async def importbucket(
     """
     Import an existing bucket.
     """
+    org_name = parse_org_name(org, root)
     provider_type = Bucket.Provider(provider)
     credentials = {}
     if provider_type == Bucket.Provider.AWS:
@@ -317,7 +319,7 @@ async def importbucket(
         credentials=credentials,
         name=name,
         cluster_name=cluster,
-        org_name=org,
+        org_name=org_name,
     )
     bucket_fmtr = BucketFormatter(
         str, datetime_formatter=get_datetime_formatter(root.iso_datetime_format)

neuro-cli/src/neuro_cli/disks.py

@@ -10,7 +10,7 @@
     UnionType,
 )
 from neuro_cli.formatters.utils import get_datetime_formatter
-from neuro_cli.utils import resolve_disk
+from neuro_cli.utils import parse_org_name, resolve_disk
 
 from .formatters.disks import (
     BaseDisksFormatter,
@@ -139,13 +139,14 @@ async def create(
     disk_timeout_unused = None
     if timeout_unused_seconds:
         disk_timeout_unused = timedelta(seconds=timeout_unused_seconds)
+    org_name = parse_org_name(org, root)
 
     disk = await root.client.disks.create(
         parse_memory(storage),
         timeout_unused=disk_timeout_unused,
         name=name,
         cluster_name=cluster,
-        org_name=org,
+        org_name=org_name,
     )
     disk_fmtr = DiskFormatter(
         str, datetime_formatter=get_datetime_formatter(root.iso_datetime_format)

neuro-cli/src/neuro_cli/job.py

@@ -36,7 +36,7 @@
     uri_formatter,
 )
 from neuro_cli.parse_utils import parse_sort_keys
-from neuro_cli.utils import resolve_disk
+from neuro_cli.utils import parse_org_name, resolve_disk
 
 from .ael import process_attach, process_exec, process_logs
 from .click_types import (
@@ -1078,9 +1078,7 @@ async def run(
         http_port = http
     cmd = _fix_cmd("neuro run", "IMAGE -- CMD...", cmd)
     cluster_name = cluster or root.client.cluster_name
-    org_name = org or root.client.config.org_name
-    if org == "NO_ORG":
-        org_name = None
+    org_name = parse_org_name(org, root)
     cluster_config = root.client.config.clusters[cluster_name]
     if not preset:
         preset = next(iter(cluster_config.presets.keys()))

neuro-cli/src/neuro_cli/secrets.py

@@ -10,7 +10,7 @@
 from neuro_cli.formatters.utils import URIFormatter, uri_formatter
 
 from .root import Root
-from .utils import argument, command, group, option
+from .utils import argument, command, group, option, parse_org_name
 
 
 @group()
@@ -83,8 +83,9 @@ async def add(
       neuro secret add KEY_NAME VALUE
       neuro secret add KEY_NAME @path/to/file.txt
     """
+    org_name = parse_org_name(org, root)
     await root.client.secrets.add(
-        key, read_data(value), cluster_name=cluster, org_name=org
+        key, read_data(value), cluster_name=cluster, org_name=org_name
     )
 
 
@@ -105,7 +106,8 @@ async def rm(root: Root, key: str, cluster: Optional[str], org: Optional[str]) -
     Remove secret KEY.
     """
 
-    await root.client.secrets.rm(key, cluster_name=cluster, org_name=org)
+    org_name = parse_org_name(org, root)
+    await root.client.secrets.rm(key, cluster_name=cluster, org_name=org_name)
     if root.verbosity > 0:
         root.print(f"Secret with key '{key}' was successfully removed")
 

neuro-cli/src/neuro_cli/utils.py

@@ -613,6 +613,10 @@ def parse_permission_action(action: str) -> Action:
         )
 
 
+def parse_org_name(org: Optional[str], root: Root) -> Optional[str]:
+    return None if org == "NO_ORG" else (org or root.client.config.org_name)
+
+
 def format_size(value: Optional[float]) -> str:
     if value is None:
         return ""

neuro-cli/tests/e2e/conftest.py

@@ -137,6 +137,19 @@ def cluster_name(self) -> str:
         config = self.get_config()
         return config.cluster_name
 
+    @cached_property
+    def org_name(self) -> Optional[str]:
+        config = self.get_config()
+        return config.org_name
+
+    @cached_property
+    def cluster_uri_base(self) -> str:
+        result = self.cluster_name
+        if self.org_name is not None:
+            result += "/" + self.org_name
+        result += "/" + self.username
+        return result
+
     @cached_property
     def token(self) -> str:
         config = self.get_config()
@@ -158,7 +171,7 @@ def tmpstorage(self) -> URL:
 
     def make_uri(self, path: str, *, fromhome: bool = False) -> URL:
         if fromhome:
-            return URL(f"storage://{self.cluster_name}/{self.username}/{path}")
+            return URL(f"storage://{self.cluster_uri_base}/{path}")
         else:
             return self.tmpstorage / path
 
@@ -398,12 +411,15 @@ async def check_job_output(
                 async for chunk in it:
                     if not chunk:
                         break
-                    chunks.append(chunk.decode())
-                    if re.search(expected, "".join(chunks), flags):
+                    chunks.append(chunk)
+                    if re.search(expected, b"".join(chunks).decode(), flags):
                         return
                     if time() - started_at > JOB_OUTPUT_TIMEOUT:
                         break
 
+        print(f"Output of job {job_id}:")
+        for chunk in chunks:
+            print(f"  {chunk!r}")
         raise AssertionError(
             f"Output of job {job_id} does not satisfy to expected regexp: {expected}"
         )
@@ -813,6 +829,7 @@ def _get_nmrc_path(tmp_path: Any, require_admin: bool) -> Optional[Path]:
 @pytest.fixture
 def helper(tmp_path: Path, nmrc_path: Path) -> Iterator[Helper]:
     ret = Helper(nmrc_path=nmrc_path, tmp_path=tmp_path)
+    ret.cluster_uri_base  # get and cache properties outside of the event loop
     yield ret
     with suppress(Exception):
         # ignore exceptions in helper closing

neuro-cli/tests/e2e/test_e2e_images.py

@@ -65,12 +65,12 @@ def test_images_complete_lifecycle(
     # stderr has "Used image ..." lines
     # assert not captured.err
 
-    image_full_str = f"image://{helper.cluster_name}/{helper.username}/{image}"
+    image_full_str = f"image://{helper.cluster_uri_base}/{image}"
     assert captured.out.endswith(image_full_str)
     image_url = URL(image_full_str)
 
     # Check if image available on registry
-    image_full_str = f"image://{helper.cluster_name}/{helper.username}/{image}"
+    image_full_str = f"image://{helper.cluster_uri_base}/{image}"
     image_short_str = f"image:{image}"
     assert captured.out.endswith(image_full_str)
 
@@ -126,7 +126,7 @@ def test_image_tags(helper: Helper, image: str, tag: str) -> None:
     # push image
     captured = helper.run_cli(["image", "push", image])
 
-    image_full_str = f"image://{helper.cluster_name}/{helper.username}/{image}"
+    image_full_str = f"image://{helper.cluster_uri_base}/{image}"
     assert captured.out.endswith(image_full_str)
 
     image_full_str_no_tag = image_full_str.replace(f":{tag}", "")
@@ -207,11 +207,8 @@ async def test_images_push_with_specified_name(
     captured = helper.run_cli(["image", "push", image, f"image:{pushed_no_tag}:{tag}"])
     # stderr has "Used image ..." lines
     # assert not captured.err
+    image_pushed_full_str = f"image://{helper.cluster_uri_base}/{pushed_no_tag}:{tag}"
     async with helper.client() as client:
-        image_pushed_full_str = (
-            f"image://{client.config.cluster_name}/"
-            f"{client.config.username}/{pushed_no_tag}:{tag}"
-        )
         assert captured.out.endswith(image_pushed_full_str)
 
     # Check if image available on registry
@@ -251,9 +248,11 @@ def test_docker_helper(
 ) -> None:
     monkeypatch.setenv(CONFIG_ENV_NAME, str(nmrc_path or DEFAULT_CONFIG_PATH))
     helper.run_cli(["config", "docker"])
-    registry = helper.registry_url.host
-    username = helper.username
-    full_tag = f"{registry}/{username}/{image}"
+    full_tag = helper.registry_url.host
+    assert full_tag
+    if helper.org_name:
+        full_tag += f"/{helper.org_name}"
+    full_tag += f"/{helper.username}/{image}"
     tag_cmd = f"docker tag {image} {full_tag}"
     result = subprocess.run(tag_cmd, capture_output=True, shell=True)
     assert (
@@ -265,7 +264,7 @@ def test_docker_helper(
         not result.returncode
     ), f"Command {push_cmd} failed: {result.stdout!r} {result.stderr!r} "
     # Run image and check output
-    image_url = f"image://{helper.cluster_name}/{username}/{image}"
+    image_url = f"image://{helper.cluster_uri_base}/{image}"
     job_id = helper.run_job_and_wait_state(
         image_url, "", wait_state=JobStatus.SUCCEEDED, stop_state=JobStatus.FAILED
     )

neuro-cli/tests/e2e/test_e2e_jobs.py

@@ -526,12 +526,12 @@ def test_e2e_ssh_exec_dead_job(helper: Helper) -> None:
 def test_job_save(helper: Helper, docker: aiodocker.Docker) -> None:
     job_name = f"test-job-save-{uuid4().hex[:6]}"
     image = f"{make_image_name()}:{job_name}"
-    image_neuro_name = f"image://{helper.cluster_name}/{helper.username}/{image}"
+    image_neuro_name = f"image://{helper.cluster_uri_base}/{image}"
     command = "sh -c 'echo -n 123 > /test; sleep 10m'"
     job_id_1 = helper.run_job_and_wait_state(
         ALPINE_IMAGE_NAME, command=command, wait_state=JobStatus.RUNNING
     )
-    img_uri = f"image://{helper.cluster_name}/{helper.username}/{image}"
+    img_uri = f"image://{helper.cluster_uri_base}/{image}"
     captured = helper.run_cli(["job", "save", job_id_1, image_neuro_name])
     out = captured.out
     assert f"Saving job '{job_id_1}' to image '{img_uri}'..." in out
@@ -816,7 +816,7 @@ def test_job_run_share(helper: Helper, fakebrowser: Any) -> None:
     )
     job_id = captured.out.strip()
 
-    uri = f"job://{helper.cluster_name}/{helper.username}/{job_id}"
+    uri = f"job://{helper.cluster_uri_base}/{job_id}"
     captured = helper.run_cli(["acl", "list", "--full-uri", "--shared", uri])
     result = [line.split() for line in captured.out.splitlines()]
     assert [uri, "write", another_test_user] in result

neuro-cli/tests/e2e/test_e2e_share.py

@@ -16,7 +16,7 @@ def revoke(helper: Helper, uri: str, username: str) -> None:
 
 @pytest.mark.e2e
 def test_grant_complete_lifecycle(request: Any, helper: Helper) -> None:
-    uri = f"storage://{helper.cluster_name}/{helper.username}/{uuid4()}"
+    uri = f"storage://{helper.cluster_uri_base}/{uuid4()}"
     uri2 = f"{uri}/{uuid4()}"
 
     another_test_user = "test2"
@@ -37,7 +37,7 @@ def test_grant_complete_lifecycle(request: Any, helper: Helper) -> None:
     assert captured.err == ""
     result = [line.split() for line in captured.out.splitlines()]
     assert [
-        f"storage://{helper.cluster_name}/{helper.username}",
+        f"storage://{helper.cluster_uri_base}",
         "manage",
     ] in result or [f"storage://{helper.cluster_name}", "manage"] in result
     assert [f"user://{helper.username}", "read"] in result
@@ -46,7 +46,7 @@ def test_grant_complete_lifecycle(request: Any, helper: Helper) -> None:
     assert captured.err == ""
     result = [line.split() for line in captured.out.splitlines()]
     assert [
-        f"storage://{helper.cluster_name}/{helper.username}",
+        f"storage://{helper.cluster_uri_base}",
         "manage",
     ] in result or [f"storage://{helper.cluster_name}", "manage"] in result
     for line in result:
@@ -95,7 +95,7 @@ def test_grant_complete_lifecycle(request: Any, helper: Helper) -> None:
 
 @pytest.mark.e2e
 def test_revoke_no_effect(helper: Helper) -> None:
-    uri = f"storage://{helper.cluster_name}/{helper.username}/{uuid4()}"
+    uri = f"storage://{helper.cluster_uri_base}/{uuid4()}"
     with pytest.raises(subprocess.CalledProcessError) as cm:
         helper.run_cli(["-v", "acl", "revoke", uri, "public"])
     assert cm.value.returncode == 127
@@ -107,7 +107,7 @@ def test_revoke_no_effect(helper: Helper) -> None:
 def test_grant_image_no_tag(request: Any, helper: Helper) -> None:
     rel_path = str(uuid4())
     rel_uri = f"image:{rel_path}"
-    uri = f"image://{helper.cluster_name}/{helper.username}/{rel_path}"
+    uri = f"image://{helper.cluster_uri_base}/{rel_path}"
     another_test_user = "test2"
 
     request.addfinalizer(lambda: revoke(helper, rel_uri, another_test_user))
@@ -125,7 +125,7 @@ def test_grant_image_no_tag(request: Any, helper: Helper) -> None:
 
 @pytest.mark.e2e
 def test_grant_image_with_tag_fails(request: Any, helper: Helper) -> None:
-    uri = f"image://{helper.cluster_name}/{helper.username}/{uuid4()}:latest"
+    uri = f"image://{helper.cluster_uri_base}/{uuid4()}:latest"
     another_test_user = "test2"
     with pytest.raises(subprocess.CalledProcessError) as cm:
         request.addfinalizer(lambda: revoke(helper, uri, another_test_user))
@@ -161,7 +161,7 @@ def test_add_grant_remove_role(request: Any, helper: Helper) -> None:
         assert captured.err == ""
         assert captured.out == ""
 
-        uri = f"storage://{helper.cluster_name}/{helper.username}/{uuid4()}"
+        uri = f"storage://{helper.cluster_uri_base}/{uuid4()}"
         captured = helper.run_cli(["acl", "grant", uri, role_name, "read"])
         assert captured.err == ""
         assert captured.out == ""

neuro-cli/tests/e2e/test_e2e_storage.py

@@ -513,10 +513,7 @@ def test_e2e_glob(tmp_path: Path, helper: Helper) -> None:
 
     # Test subcommand "glob"
     captured = helper.run_cli(["storage", "glob", helper.tmpstorage / "**"])
-    prefix = (
-        f"storage://{helper.cluster_name}/{helper.username}/"
-        f"{helper.tmpstorage.path}"
-    )
+    prefix = f"storage://{helper.cluster_uri_base}/{helper.tmpstorage.path}"
     assert sorted(captured.out.splitlines()) == [
         prefix.rstrip("/"),
         prefix + "/folder",

neuro-sdk/src/neuro_sdk/_parser.py

@@ -322,13 +322,18 @@ def normalize_uri(
     def _short(self, uri: URL) -> URL:
         ret = uri
         if uri.scheme != "file":
-            if ret.host == self._config.cluster_name and ret.parts[:2] == (
-                "/",
-                self._config.username,
-            ):
-                ret = URL.build(
-                    scheme=ret.scheme, host="", path="/".join(ret.parts[2:])
-                )
+            if ret.host == self._config.cluster_name:
+                prefix: Tuple[str, ...]
+                if self._config.org_name is None:
+                    prefix = ("/", self._config.username)
+                else:
+                    prefix = ("/", self._config.org_name, self._config.username)
+                if ret.parts[: len(prefix)] == prefix:
+                    ret = URL.build(
+                        scheme=ret.scheme,
+                        host="",
+                        path="/".join(ret.parts[len(prefix) :]),
+                    )
         else:
             # file scheme doesn't support relative URLs.
             pass