Charmhub/address charmhub limitiations (#24)

* Remove dependancies of unzip and juju from host machine

* match url/request/unzip patthrn on each charm*_downloader

* No longer need juju in lxd, determined there was no need for security.priv in lxd, and made 'bundles' a required argument

* improve API used to fetch charmhub info

* Fetch resource associated with the bundle if specified, and fetch resources from charmhub

* respect bundle defined app channels for charmstore

shrinkwrap.py

@@ -2,6 +2,8 @@
 
 import argparse
 import datetime
+from collections import namedtuple
+from collections.abc import Sequence
 from contextlib import contextmanager
 from io import BytesIO
 import os
@@ -84,7 +86,17 @@ def to_args(target: Path, channel: Optional[str] = None, arch: Optional[str] = N
         return r_args, target
 
 
-class BundleDownloader(Downloader):
+class StoreDownloader(Downloader):
+    CS_URL = "https://api.jujucharms.com/charmstore/v5"
+    CH_URL = "https://api.charmhub.io/v2"
+
+    def _charmhub_info(self, name, **query):
+        url = f"{self.CH_URL}/charms/info/{name}"
+        resp = requests.get(url, params=query)
+        return resp.json()
+
+
+class BundleDownloader(StoreDownloader):
     def __init__(self, root, args):
         """
         @param root: PathLike[str]
@@ -151,44 +163,26 @@ def _downloader(self, name, path, channel):
         if ch:
             return self._charmhub_downloader(name, target, channel=channel)
         else:
-            return self._charmstore_downloader(name, target)
-
-    @staticmethod
-    def _charmhub_refresh(name, channel, architecture):
-        channel = channel or "stable"
-        architecture = architecture or "amd64"
-        data = {
-            "context": [],
-            "actions": [
-                {
-                    "name": name,
-                    "base": {"name": "ubuntu", "architecture": architecture, "channel": channel},
-                    "action": "install",
-                    "instance-key": "shrinkwrap",
-                }
-            ],
-        }
-        resp = requests.post("https://api.charmhub.io/v2/charms/refresh", json=data)
-        return resp.json()
+            return self._charmstore_downloader(name, target, channel=channel)
 
-    def _charmhub_downloader(self, name, target, channel=None, arch=None):
-        with status(f'Downloading "{name}" from charm hub'):
-            _, rsc_target = self.to_args(target, channel, arch)
-            refreshed = self._charmhub_refresh(name, channel, arch)
-            url = refreshed["results"][0]["charm"]["download"]["url"]
+    def _charmhub_downloader(self, name, target, channel=None):
+        with status(f'Downloading "{name} {channel}" from charm hub'):
+            _, rsc_target = self.to_args(target, channel)
+            charm_info = self._charmhub_info(name, channel=channel, fields="default-release.revision.download.url")
+            url = charm_info["default-release"]["revision"]["download"]["url"]
             resp = requests.get(url)
             return zipfile.ZipFile(BytesIO(resp.content)).extractall(rsc_target)
 
-    def _charmstore_downloader(self, name, target):
-        with status(f'Downloading "{name}" from charm store'):
-            _, rsc_target = self.to_args(target)
-            url = f"https://api.jujucharms.com/charmstore/v5/{name}/archive"
-            resp = requests.get(url)
+    def _charmstore_downloader(self, name, target, channel=None):
+        with status(f'Downloading "{name} {channel}" from charm store'):
+            _, rsc_target = self.to_args(target, channel=channel)
+            url = f"{self.CS_URL}/{name}/archive"
+            resp = requests.get(url, params={"channel": channel})
             return zipfile.ZipFile(BytesIO(resp.content)).extractall(rsc_target)
 
 
 class OverlayDownloader(Downloader):
-    URL = "https://api.github.com/repos/charmed-kubernetes/bundle/contents/overlays"
+    GH_URL = "https://api.github.com/repos/charmed-kubernetes/bundle/contents/overlays"
 
     def __init__(self, bundles_path):
         """
@@ -201,7 +195,7 @@ def __init__(self, bundles_path):
     def list(self):
         if self._list_cache:
             return self._list_cache
-        resp = requests.get(self.URL, headers={"Accept": "application/vnd.github.v3+json"})
+        resp = requests.get(self.GH_URL, headers={"Accept": "application/vnd.github.v3+json"})
         self._list_cache = {obj.get("name"): obj.get("download_url") for obj in resp.json()}
         return self._list_cache
 
@@ -332,9 +326,38 @@ def download(self):
                 check_call(shlx(f"mv {tgz} {snap_target}"))
 
 
-class ResourceDownloader(Downloader):
-    URL = "https://api.jujucharms.com/charmstore/v5"
+class Resource(namedtuple("Resource", "name, type, path, revision, url_format")):
+    @classmethod
+    def from_charmstore(cls, baseurl, json):
+        if isinstance(json, Sequence):
+            return [cls.from_charmstore(baseurl, _) for _ in json]
+        return cls(
+            json["Name"],
+            json["Type"],
+            json["Path"],
+            json["Revision"],
+            f"{baseurl}/resource/{json['Name']}/{{revision}}",
+        )
+
+    @classmethod
+    def from_charmhub(cls, json):
+        if isinstance(json, Sequence):
+            return [cls.from_charmhub(_) for _ in json]
+        url_format = remove_suffix(json["download"]["url"], f"_{json['revision']}")
+        return cls(
+            json["name"],
+            json["type"],
+            json["filename"],
+            json["revision"],
+            f"{url_format}_{{revision}}",
+        )
+
+    @property
+    def url(self):
+        return self.url_format.format(revision=self.revision)
+
 
+class ResourceDownloader(StoreDownloader):
     def __init__(self, root):
         """
         @param root: PathLike[str]
@@ -344,38 +367,36 @@ def __init__(self, root):
     def list(self, charm, channel):
         ch = charm.startswith("ch:") or not charm.startswith("cs:")
         if ch:
-            raise NotImplementedError("Fetching of resources from Charmhub not supported.")
+            resp = self._charmhub_info(charm, channel=channel, fields="default-release.resources")
+            resources = Resource.from_charmhub(resp["default-release"]["resources"])
         else:
+            name = remove_prefix(charm, "cs:")
             resp = requests.get(
-                f"{self.URL}/{remove_prefix(charm, 'cs:')}/meta/resources",
+                f"{self.CS_URL}/{name}/meta/resources",
                 params={"channel": channel},
             )
-        return resp.json()
+            resources = Resource.from_charmstore(f"{self.CS_URL}/{name}", resp.json())
+        return resources
 
-    def mark_download(self, app, charm, name, revision, filename) -> Path:
-        resource_key = app, charm, name, revision, filename
+    def mark_download(self, app, charm, resource) -> Path:
+        resource_key = app, charm, resource
         if resource_key not in self._downloaded:
-            self._downloaded[resource_key] = self.path / app / name / filename
+            self._downloaded[resource_key] = self.path / app / resource.name / resource.path
         return self._downloaded[resource_key]
 
     def download(self):
         print("Resources")
-        for (app, charm, name, revision, filename), target in self._downloaded.items():
+        for (app, charm, resource), target in self._downloaded.items():
             if target.exists():
-                print(f"    Downloaded resource {name} - {revision} exists")
+                print(f"    Downloaded resource {resource.name} - {resource.revision} exists")
                 continue
 
             target.parent.mkdir(parents=True, exist_ok=True)
-            ch = charm.startswith("ch:") or not charm.startswith("cs:")
-            if ch:
-                raise NotImplementedError("Charmhub doesn't support fetching resources")
-            else:
-                url = f"{self.URL}/{remove_prefix(charm, 'cs:')}/resource/{name}/{revision}"
-                with status(f"Downloading resource {name} from charm store revision {revision}"):
-                    check_call(shlx(f"wget --quiet {url} -O {target}"))
+            with status(f"Downloading {charm} resource {resource.name} @ revision {resource.revision}"):
+                check_call(shlx(f"wget --quiet {resource.url} -O {target}"))
 
 
-def charm_channel(app, charm_path) -> str:
+def charm_snap_channel(app, charm_path) -> str:
     # Try to get channel from config
     with (charm_path / "config.yaml").open() as stream:
         config = yaml.safe_load(stream)
@@ -406,22 +427,23 @@ def download(args, root):
     for app_name, app in charms.applications.items():
         charm = charms.app_download(app_name, app)
 
-        app_channel = charm_channel(app, root / "charms" / app_name)
+        snap_channel = charm_snap_channel(app, root / "charms" / app_name)
+        charm_channel = app.get("channel")
         if charm in ["kubernetes-control-plane", "kubernetes-master"]:
-            k8s_cp_channel = app_channel
+            k8s_cp_channel = snap_channel
 
         # Download each resource or snap.
-        for resource in resources.list(charm, args.channel):
+        for resource in resources.list(charm, charm_channel):
             # Create the filename from the snap Name and Path extension. Use this instead of just Path because
             # multiple resources can have the same names for Paths.
-            path = resource["Path"]
-            name = resource["Name"]
+            path = resource.path
+            name = resource.name
 
             # If it's a snap, download it from the store.
             if path.endswith(".snap"):
                 # If the current resource is the core snap, ignore channel
                 # and instead always download from stable.
-                snap_channel = app_channel if name != "core" else "stable"
+                snap_channel = snap_channel if name != "core" else "stable"
 
                 # Path without .snap extension is currently a match for the name in the snap store. This may not always
                 # be the case.
@@ -436,9 +458,11 @@ def download(args, root):
                     snap_resource.parent.mkdir(parents=True, exist_ok=True)
                     check_call(shlx(f"ln -r -s {snaps.empty_snap} {snap_resource}"))
             else:
-                # This isn't a snap, do it the easy way.
-                revision = resource["Revision"]
-                resource["filepath"] = resources.mark_download(app_name, charm, name, revision, path)
+                # This isn't a snap, pull the resource from the appropriate store
+                # use the bundle provided resource revision if available
+                resource_rev = app["resources"].get(resource.name) or resource.revision
+                resource = Resource(resource.name, resource.type, resource.path, resource_rev, resource.url_format)
+                resources.mark_download(app_name, charm, resource)
 
     base_snaps = ["core18", "core20", "lxd", "snapd"]
     for snap in base_snaps:

tests/unit/test_bundle_downloader.py

@@ -27,37 +27,36 @@ def mock_cs_downloader():
 
 
 @mock.patch("shrinkwrap.requests.get")
-@mock.patch("shrinkwrap.requests.post")
 @mock.patch("shrinkwrap.zipfile.ZipFile")
-def test_charmhub_downloader(mock_zipfile, mock_post, mock_get, tmpdir):
+def test_charmhub_downloader(mock_zipfile, mock_get, tmpdir):
     args = mock.MagicMock()
     args.bundle = "ch:kubernetes-unit-test"
     args.channel = None
     args.overlay = []
 
+    def mock_get_response(url, **_kwargs):
+        response = mock.MagicMock()
+        if "info" in url:
+            response.json.return_value = {"default-release": {"revision": {"download": {"url": bundle_mock_url}}}}
+        elif bundle_mock_url == url:
+            response.content = b"bytes-values"
+        return response
+
     bundle_mock_url = mock.MagicMock()
-    mock_post.return_value.json.return_value = {"results": [{"charm": {"download": {"url": bundle_mock_url}}}]}
     mock_downloaded = mock_zipfile.return_value.extractall.return_value
-    mock_get.return_value.content = b"bytes-values"
+    mock_get.side_effect = mock_get_response
 
     downloader = BundleDownloader(tmpdir, args)
     result = downloader.bundle_download()
     assert result is mock_downloaded
-    mock_post.assert_called_once_with(
-        "https://api.charmhub.io/v2/charms/refresh",
-        json={
-            "context": [],
-            "actions": [
-                {
-                    "name": "kubernetes-unit-test",
-                    "base": {"name": "ubuntu", "architecture": "amd64", "channel": "stable"},
-                    "action": "install",
-                    "instance-key": "shrinkwrap",
-                }
-            ],
-        },
-    )
-    mock_get.assert_called_once_with(bundle_mock_url)
+    expected_gets = [
+        mock.call(
+            "https://api.charmhub.io/v2/charms/info/kubernetes-unit-test",
+            params=dict(channel=args.channel, fields="default-release.revision.download.url"),
+        ),
+        mock.call(bundle_mock_url),
+    ]
+    mock_get.assert_has_calls(expected_gets)
     mock_zipfile.assert_called_once()
     assert isinstance(mock_zipfile.call_args.args[0], BytesIO)
 
@@ -75,7 +74,9 @@ def test_charmstore_downloader(mock_zipfile, mock_get, tmpdir):
     downloader = BundleDownloader(tmpdir, args)
     result = downloader.bundle_download()
     assert result is mock_downloaded
-    mock_get.assert_called_once_with("https://api.jujucharms.com/charmstore/v5/kubernetes-unit-test/archive")
+    mock_get.assert_called_once_with(
+        "https://api.jujucharms.com/charmstore/v5/kubernetes-unit-test/archive", params={"channel": args.channel}
+    )
     mock_zipfile.assert_called_once()
     assert isinstance(mock_zipfile.call_args.args[0], BytesIO)
 
@@ -95,13 +96,13 @@ def test_bundle_downloader(tmpdir, mock_ch_downloader, mock_cs_downloader):
         == "cs:~containers/containerd-160"
     )
     mock_ch_downloader.assert_called_once_with("etcd", charms_path / "etcd", channel="latest/edge")
-    mock_cs_downloader.assert_called_once_with("~containers/containerd-160", charms_path / "containerd")
+    mock_cs_downloader.assert_called_once_with("~containers/containerd-160", charms_path / "containerd", channel=None)
 
     mock_ch_downloader.reset_mock()
     mock_cs_downloader.reset_mock()
     downloader.bundle_download()
     mock_ch_downloader.assert_not_called()
-    mock_cs_downloader.assert_called_once_with("kubernetes-unit-test", downloader.bundle_path)
+    mock_cs_downloader.assert_called_once_with("kubernetes-unit-test", downloader.bundle_path, channel=args.channel)
 
 
 def test_bundle_downloader_properties(tmpdir, test_bundle, test_overlay, mock_overlay_list):

tests/unit/test_download_all.py

@@ -2,7 +2,7 @@
 
 import yaml
 
-from shrinkwrap import download, BundleDownloader
+from shrinkwrap import download, BundleDownloader, Resource
 
 import mock
 
@@ -32,39 +32,24 @@ def test_download_method(resource_list, resource_dl, snap_dl, app_dl, tmpdir, te
         (root / "charms" / app_name).mkdir(parents=True)
         (root / "charms" / app_name / "config.yaml").write_text(fp.read())
 
-    app_dl.return_value = "cs:~containers/etcd"  # name of the charm, not the app
+    app_dl.return_value = "etcd"  # name of the charm, not the app
     resource_list.return_value = [
-        {
-            "Name": "core",
-            "Type": "file",
-            "Path": "core.snap",
-            "Description": "Snap package of core",
-            "Revision": 0,
-            "Size": 0,
-        },
-        {
-            "Name": "etcd",
-            "Type": "file",
-            "Path": "etcd.snap",
-            "Description": "Snap package of etcd",
-            "Revision": 3,
-            "Size": 0,
-        },
-        {
-            "Name": "snapshot",
-            "Type": "file",
-            "Path": "snapshot.tar.gz",
-            "Description": "Tarball snapshot of an etcd clusters data.",
-            "Revision": 0,
-            "Size": 124,
-        },
+        Resource("core", "file", "core.snap", 0, "https://api.jujucharms.com/charmstore/v5/etcd/resource/core/0"),
+        Resource("etcd", "file", "etcd.snap", 3, "https://api.jujucharms.com/charmstore/v5/etcd/resource/etcd/3"),
+        Resource(
+            "snapshot",
+            "file",
+            "snapshot.tar.gz",
+            0,
+            "https://api.jujucharms.com/charmstore/v5/etcd/resource/snapshot/0",
+        ),
     ]
 
     charms = download(args, root)
     assert isinstance(charms, BundleDownloader)
 
     app_dl.assert_called_once_with(app_name, charms.applications[app_name])
-    resource_list.assert_called_once_with(app_dl.return_value, args.channel)
+    resource_list.assert_called_once_with(app_dl.return_value, "latest/edge")
     snap_dl.assert_called_once()
     resource_dl.assert_called_once()
     assert (Path(tmpdir) / "resources" / "etcd" / "etcd" / "etcd.snap").is_symlink()