Support accessing remote registries via ssh (#589)

* Support accessing remote registries via ssh
* Factored out a function that tells whether a registry path is local
* Make sure the URL is used, not self.source which could be a local path

The URL has to be given as "ssh://[user@]host.xz[:port]/path/to/repo.git"
rather than the shorter version "[user@]host.xz:path/to/repo.git"

* Make self.source include the subdir from the start. Allows implementing iter_modules in the base class
* The check already happens in _update_cache()
* Moved is_path_local to shpc.utils
* Added a safeguard to prevent cloning multiple times
* clone() is actually only supported by VersionControl
* No need to yield self.source in iter_modules since it's constant and accessible from outside (and not all callers want it !)
* It's more practical to yield the the registry object (provider) rather than using the "source" path (which is undefined for remote registries anyway)
* Optimised the "update all" mode by directly using Result objects from the registries. Otherwise, it wastes time finding modules that we know are there
* Clones only ever exist within a function
* Optimised iter_modules method for remote registries (using the cache)
* Moved back iter_modules to Filesystem since VersionControl has its own, optimised, version
* Stopped using self.source in VersionControl, to avoid confusion with Filesystem
* url, not source, is to be used for remote registries
* Cannot do these heuristics as we need to report unexisting local paths
* str.split can limit the number of splits
* The main Registry object, not the settings, should decide whether there is a local registry or not
* To avoid duplicating the code that assesses whether a path or local or not, check which sub-class of Provider is used
* The parent class shouldn't know that much about the subclasses
* Restored back the automatic addition of https://
* Restructured to avoid an unnecessary else
* shpc convention: no else when the then ends with a return
* Unnecessary due to operator precedence rule
* Added a cache in `library_url`
* Fixed the implementation of the cache in VersionControl.exists
* exists has its own implementation in VersionControl, so this implementation is in fact specific to Filesystem
* iter_registry is basically iter_modules with an extra filter
* Yield relative paths rather than full paths since *all* consumers need relative paths
* Proper method to cleanup a clone
* Removed a cleanup() call that was expected to do nothing
* Increased the symmetry to simplify the maintainability
* NotImplementedError is more useful than pass
* The tuplized version is not the preference here
* Easier to understand
* Made the clone return a Filesystem object independent from VersionControl
* Extra comment
* Back to a subclass of VersionControl for each forge
* Pre-parse the URL
* VersionControl should not be directly used
* Renamed the variable for clarity
* Removing yaml because it's the only file we have for a container
* Defensive programming: local could still be None
* bugfix: iter_modules needs to yield paths to container.yaml
* Moved the cleanup call up to _sync()
* bugfix: iter_modules now returns path to the container.yaml
* Need to check here too that the clone still exists
* Also need to reset self._clone if the directory is gone
* More checks on local and remote
* The temp directory may have been deleted in the meantime
* It makes more sense to cleanup tmplocal than self, and it works because self expects the cleanup may happen
* Moved this to the parent class
* Another implementation that doesn't make it too obvious the base-class knows about GitHub and GitLab
* Silly typo: self._clone is a Filesystem object, not a string
* No colon
* You shall use American spelling
* Added a test to showcase ssh

* Revert "bugfix: iter_modules needs to yield paths to container.yaml"

This reverts commit f069f48.

* Revert "bugfix: iter_modules now returns path to the container.yaml"

This reverts commit c5b4cb9.

Author: muffato

shpc/main/client.py

@@ -124,12 +124,16 @@ def update(self, name=None, dryrun=False, filters=None):
         """
         # No name provided == "update all"
         if name:
-            modules = [name]
+            # find the module in the registries. _load_container
+            # calls `container.ContainerConfig(result)` like below
+            configs = [self._load_container(name)]
         else:
-            modules = [x[1] for x in list(self.registry.iter_modules())]
-
-        for module_name in modules:
-            config = self._load_container(module_name)
+            # directly iterate over the content of the registry
+            configs = []
+            for result in self.registry.iter_registry():
+                configs.append(container.ContainerConfig(result))
+        # do the update
+        for config in configs:
             config.update(dryrun=dryrun, filters=filters)
 
     def test(

shpc/main/modules/base.py

@@ -172,7 +172,12 @@ def add(self, image, module_name=None, **kwargs):
         """
         Add a container to the registry to enable install.
         """
-        self.settings.ensure_filesystem_registry()
+        local_registry = self.registry.filesystem_registry
+
+        if not local_registry:
+            logger.exit(
+                "This command is only supported for a filesystem registry! Add one or use --registry."
+            )
 
         # Docker module name is always the same namespace as the image
         if image.startswith("docker"):
@@ -185,7 +190,7 @@ def add(self, image, module_name=None, **kwargs):
 
         # Assume adding to default registry
         dest = os.path.join(
-            self.settings.filesystem_registry,
+            local_registry.source,
             module_name.split(":")[0],
             "container.yaml",
         )
@@ -235,10 +240,9 @@ def docgen(self, module_name, registry=None, out=None, branch="main"):
         aliases = config.get_aliases()
         template = self.template.load("docs.md")
         registry = registry or defaults.github_url
-        github_url = "%s/blob/%s/%s/container.yaml" % (registry, branch, module_name)
-        raw_github_url = shpc.main.registry.get_module_config_url(
-            registry, module_name, branch
-        )
+        remote = self.registry.get_registry(registry, tag=branch)
+        github_url = remote.get_container_url(module_name)
+        raw_github_url = remote.get_raw_container_url(module_name)
 
         # Currently one doc is rendered for all containers
         result = template.render(
@@ -306,10 +310,9 @@ def _get_module_lookup(self, base, filename, pattern=None):
         A shared function to get a lookup of installed modules or registry entries
         """
         modules = {}
-        for fullpath in utils.recursive_find(base, pattern):
-            if fullpath.endswith(filename):
-                module_name, version = os.path.dirname(fullpath).rsplit(os.sep, 1)
-                module_name = module_name.replace(base, "").strip(os.sep)
+        for relpath in utils.recursive_find(base, pattern):
+            if relpath.endswith(filename):
+                module_name, version = os.path.dirname(relpath).rsplit(os.sep, 1)
                 if module_name not in modules:
                     modules[module_name] = set()
                 modules[module_name].add(version)

shpc/main/registry/__init__.py

@@ -14,7 +14,7 @@
 from shpc.main.settings import SettingsBase
 
 from .filesystem import Filesystem, FilesystemResult
-from .remote import GitHub, GitLab, get_module_config_url
+from .remote import GitHub, GitLab
 
 
 def update_container_module(module, from_path, existing_path):
@@ -23,13 +23,12 @@ def update_container_module(module, from_path, existing_path):
     """
     if not os.path.exists(existing_path):
         shpc.utils.mkdir_p(existing_path)
-    for filename in shpc.utils.recursive_find(from_path):
-        relative_path = filename.replace(from_path, "").strip("/")
+    for relative_path in shpc.utils.recursive_find(from_path):
         to_path = os.path.join(existing_path, relative_path)
         if os.path.exists(to_path):
             shutil.rmtree(to_path)
         shpc.utils.mkdir_p(os.path.dirname(to_path))
-        shutil.copy2(filename, to_path)
+        shutil.copy2(os.path.join(from_path, relative_path), to_path)
 
 
 class Registry:
@@ -44,21 +43,29 @@ def __init__(self, settings=None):
         # and they must exist.
         self.registries = [self.get_registry(r) for r in self.settings.registry]
 
+    @property
+    def filesystem_registry(self):
+        """
+        Return the first found filesystem registry.
+        """
+        for registry in self.registries:
+            if isinstance(registry, Filesystem):
+                return registry
+
     def exists(self, name):
         """
-        Determine if a module name *exists* in any local registry, return path
+        Determine if a module name *exists* in any registry, return the first one
         """
         for reg in self.registries:
             if reg.exists(name):
-                return os.path.join(reg.source, name)
+                return reg
 
     def iter_registry(self, filter_string=None):
         """
         Iterate over all known registries defined in settings.
         """
         for reg in self.registries:
-            for entry in reg.iter_registry(filter_string=filter_string):
-                yield entry
+            yield from reg.iter_registry(filter_string=filter_string)
 
     def find(self, name, path=None):
         """
@@ -80,19 +87,19 @@ def iter_modules(self):
         """
         Iterate over modules found across the registry
         """
-        for reg in self.registries:
-            for registry, module in reg.iter_modules():
+        for registry in self.registries:
+            for module in registry.iter_modules():
                 yield registry, module
 
-    def get_registry(self, source):
+    def get_registry(self, source, **kwargs):
         """
         A registry is a local or remote registry.
 
         We can upgrade from, or otherwise list
         """
         for Registry in PROVIDERS:
             if Registry.matches(source):
-                return Registry(source)
+                return Registry(source, **kwargs)
         raise ValueError("No matching registry provider for %s" % source)
 
     def sync(
@@ -128,20 +135,10 @@ def _sync(
         local=None,
         sync_registry=None,
     ):
-        # Registry to sync from
-        sync_registry = sync_registry or self.settings.sync_registry
-
         # Create a remote registry with settings preference
-        Remote = GitHub if "github.com" in sync_registry else GitLab
-        remote = Remote(sync_registry, tag=tag)
-        local = self.get_registry(local or self.settings.filesystem_registry)
-
-        # We sync to our first registry - if not filesystem, no go
-        if not local.is_filesystem_registry:
-            logger.exit(
-                "sync is only supported for a remote to a filesystem registry: %s"
-                % sync_registry.source
-            )
+        remote = self.get_registry(
+            sync_registry or self.settings.sync_registry, tag=tag
+        )
 
         # Upgrade the current registry from the remote
         self.sync_from_remote(
@@ -152,6 +149,8 @@ def _sync(
             add_new=add_new,
             local=local,
         )
+
+        #  Cleanup the remote once we've done the sync
         remote.cleanup()
 
     def sync_from_remote(
@@ -163,26 +162,41 @@ def sync_from_remote(
         If the registry module is not installed, we install to the first
         filesystem registry found in the list.
         """
-        updates = False
 
+        ## First get a valid local Registry
         # A local (string) path provided
-        if local and isinstance(local, str) and os.path.exists(local):
+        if local and isinstance(local, str):
+            if not os.path.exists(local):
+                logger.exit("The path %s doesn't exist." % local)
             local = Filesystem(local)
 
         # No local registry provided, use default
         if not local:
-            local = Filesystem(self.settings.filesystem_registry)
+            local = self.filesystem_registry
+            # We sync to our first registry - if not filesystem, no go
+            if not local:
+                logger.exit("No local registry to sync to. Check the shpc settings.")
+
+        if not isinstance(local, Filesystem):
+            logger.exit(
+                "Can only synchronize to a local file system, not to %s." % local
+            )
 
-        tmpdir = remote.source
-        if tmpdir.startswith("http") or not os.path.exists(tmpdir):
-            tmpdir = remote.clone()
+        ## Then a valid remote Registry
+        if not remote:
+            logger.exit("No remote provided. Cannot sync.")
+
+        if not isinstance(remote, Filesystem):
+            # Instantiate a local registry, which will have to be cleaned up
+            remote = remote.clone()
 
         # These are modules to update
-        for regpath, module in remote.iter_modules():
+        updates = False
+        for module in remote.iter_modules():
             if name and module != name:
                 continue
 
-            from_path = os.path.join(regpath, module)
+            from_path = os.path.join(remote.source, module)
             existing_path = local.exists(module)
 
             # If we have an existing module and we want to replace all files

shpc/main/registry/filesystem.py

@@ -75,20 +75,31 @@ def override_exists(self, tag):
 
 
 class Filesystem(Provider):
-    def __init__(self, *args, **kwargs):
-        super().__init__(*args, **kwargs)
-        self.source = os.path.abspath(self.source)
+    def __init__(self, source):
+        if not self.matches(source):
+            raise ValueError(
+                "Filesystem registry source must exist on the filesystem. Got %s"
+                % source
+            )
+        self.source = os.path.abspath(source)
 
     @classmethod
     def matches(cls, source):
         return os.path.exists(source) or source == "."
 
+    def exists(self, name):
+        return os.path.exists(os.path.join(self.source, name))
+
     def iter_modules(self):
+        """
+        yield module names
+        """
+        # Find modules based on container.yaml
         for filename in shpc.utils.recursive_find(self.source, "container.yaml"):
-            module = os.path.dirname(filename).replace(self.source, "").strip(os.sep)
+            module = os.path.dirname(filename)
             if not module:
                 continue
-            yield self.source, module
+            yield module
 
     def find(self, name):
         """
@@ -110,14 +121,9 @@ def iter_registry(self, filter_string=None):
         """
         Iterate over content in filesystem registry.
         """
-        for filename in shpc.utils.recursive_find(self.source):
-            if not filename.endswith("container.yaml"):
-                continue
-            module_name = (
-                os.path.dirname(filename).replace(self.source, "").strip(os.sep)
-            )
-
+        for module_name in self.iter_modules():
             # If the user has provided a filter, honor it
             if filter_string and not re.search(filter_string, module_name):
                 continue
+            filename = os.path.join(self.source, module_name)
             yield FilesystemResult(module_name, filename)

shpc/main/registry/provider.py

@@ -5,6 +5,8 @@
 
 import os
 
+import shpc.utils
+
 
 class Result:
     @property
@@ -32,36 +34,40 @@ class Provider:
     A general provider should retrieve and provide registry files.
     """
 
-    def __init__(self, source, *args, **kwargs):
-        if not (source.startswith("https://") or os.path.exists(source)):
-            raise ValueError(
-                "Registry source must exist on the filesystem or be given as https://."
-            )
-        self.source = source
-
-    def exists(self, name):
-        return os.path.exists(os.path.join(self.source, name))
-
-    @property
-    def is_filesystem_registry(self):
-        return not self.source.startswith("http") and os.path.exists(self.source)
-
-    @property
-    def name(self):
-        return self.__class__.__name__.lower()
-
     @classmethod
-    def matches(cls, source_url: str):
-        pass
+    def matches(cls, source):
+        """
+        Returns true if this class understands the source
+        """
+        raise NotImplementedError
 
     def find(self, name):
-        pass
+        """
+        Returns a Result object if the module can be found in the registry
+        """
+        raise NotImplementedError
+
+    def exists(self, name):
+        """
+        Returns true if the module can be found in the registry
+        """
+        raise NotImplementedError
 
     def cleanup(self):
-        pass
+        """
+        Cleanup the registry
+        """
+        raise NotImplementedError
 
-    def iter_registry(self):
-        pass
+    def iter_registry(self, filter_string=None):
+        """
+        Iterates over the modules of this registry (that match the filte, if
+        provided) as Result instances
+        """
+        raise NotImplementedError
 
     def iter_modules(self):
-        pass
+        """
+        Iterates over the module names of this registry
+        """
+        raise NotImplementedError