Repository: canonical/charmcraft

Issue:
`fetch-libs` crashes with an internal error
### Bug Description

After logging in and running `fetch-libs`, there is an internal error `charmcraft internal error: StoreServerError("Store operation failed:\n- None: Items need to include 'library_id' or 'package_id'")`

### To Reproduce

1. Create a multipass VM
2. Install `latest/edge`  of charmcraft
3. Include a `charm-libs` entry such as:

```yaml
charm-libs:
  - lib: observability_libs.juju_topology
    version: 0
```

4. run `fetch-libs`

### Environment

arm64 ubuntu jammy, charmcraft on `latest/edge`

### charmcraft.yaml

```shell
# This file configures Charmcraft.
# See https://juju.is/docs/sdk/charmcraft-config for guidance.

name: sample-flask

type: charm

bases:
  - build-on:
    - name: ubuntu
      channel: "22.04"
    run-on:
    - name: ubuntu
      channel: "22.04"

# (Required)
summary: A very short one-line summary of the flask application.

# (Required)
description: |
  A comprehensive overview of your Flask application.

extensions:
  - flask-framework

charm-libs:
  - lib: observability_libs.juju_topology
    version: 0

# Uncomment the integrations used by your application
# requires:
#   mysql:
#     interface: mysql_client
#     limit: 1
#   postgresql:
#     interface: postgresql_client
#     limit: 1
```


### Relevant log output

```shell
cat /home/ubuntu/.local/state/charmcraft/log/charmcraft-20240509-174417.997582.log
2024-05-09 17:44:17.998 Starting charmcraft, version 3.0.0.post39+ge2ad204
2024-05-09 17:44:17.998 Log verbosity level set to BRIEF
2024-05-09 17:44:17.998 Configuring application...
2024-05-09 17:44:17.998 Preparing application...
2024-05-09 17:44:17.998 Build plan: platform=None, build_for=None
2024-05-09 17:44:17.998 Loading project file '/home/ubuntu/src/sample-flask/charm/charmcraft.yaml'
2024-05-09 17:44:18.000 Processing grammar (on arm64 for arm64)
2024-05-09 17:44:18.000 Processing grammar for plugin: charm
2024-05-09 17:44:18.000 Processing grammar for source: .
2024-05-09 17:44:18.000 Running charmcraft fetch-libs on host
2024-05-09 17:44:18.000 Setting up StoreService
2024-05-09 17:44:18.074 Falling back to file based storage
2024-05-09 17:44:18.074 Getting library metadata from charmhub
2024-05-09 17:44:18.074 Retrieving credentials for 'charmcraft' on 'api.charmhub.io' from keyring 'auth FileKeyring'.
2024-05-09 17:44:18.074 HTTP 'POST' for 'https://api.charmhub.io/v1/charm/libraries/bulk' with params None and headers {'Authorization': '<macaroon>', 'User-Agent': 'Charmcraft/3.0.0.post39+ge2ad204 (Linux 5.15.0-106-generic; aarch64; CPython 3.10.12; Ubuntu 22.04)'}
2024-05-09 17:44:18.258 charmcraft internal error: StoreServerError("Store operation failed:\n- None: Items need to include 'library_id' or 'package_id'")
2024-05-09 17:44:18.265 Traceback (most recent call last):
2024-05-09 17:44:18.265   File "/snap/charmcraft/3739/lib/python3.10/site-packages/craft_application/application.py", line 517, in run
2024-05-09 17:44:18.265     return_code = dispatcher.run() or 0
2024-05-09 17:44:18.265   File "/snap/charmcraft/3739/lib/python3.10/site-packages/craft_cli/dispatcher.py", line 487, in run
2024-05-09 17:44:18.265     return self._loaded_command.run(self._parsed_command_args)
2024-05-09 17:44:18.265   File "/snap/charmcraft/3739/lib/python3.10/site-packages/charmcraft/application/commands/store.py", line 1679, in run
2024-05-09 17:44:18.265     libs_metadata = store.get_libraries_metadata_by_name(charm_libs)
2024-05-09 17:44:18.265   File "/snap/charmcraft/3739/lib/python3.10/site-packages/charmcraft/services/store.py", line 220, in get_libraries_metadata_by_name
2024-05-09 17:44:18.265     for lib in self.get_libraries_metadata(libraries)
2024-05-09 17:44:18.265   File "/snap/charmcraft/3739/lib/python3.10/site-packages/charmcraft/services/store.py", line 212, in get_libraries_metadata
2024-05-09 17:44:18.265     return self.client.fetch_libraries_metadata(store_libs)
2024-05-09 17:44:18.265   File "/snap/charmcraft/3739/lib/python3.10/site-packages/charmcraft/store/client.py", line 202, in fetch_libraries_metadata
2024-05-09 17:44:18.265     response = self.request_urlpath_json("POST", "/v1/charm/libraries/bulk", json=libs)
2024-05-09 17:44:18.265   File "/snap/charmcraft/3739/lib/python3.10/site-packages/charmcraft/store/client.py", line 134, in request_urlpath_json
2024-05-09 17:44:18.265     response = super().request(method, self.api_base_url + urlpath, *args, **kwargs)
2024-05-09 17:44:18.265   File "/snap/charmcraft/3739/lib/python3.10/site-packages/craft_store/base_client.py", line 186, in request
2024-05-09 17:44:18.265     return self.http_client.request(
2024-05-09 17:44:18.265   File "/snap/charmcraft/3739/lib/python3.10/site-packages/craft_store/http_client.py", line 166, in request
2024-05-09 17:44:18.265     raise errors.StoreServerError(response)
2024-05-09 17:44:18.265 craft_store.errors.StoreServerError: Store operation failed:
2024-05-09 17:44:18.265 - None: Items need to include 'library_id' or 'package_id'
2024-05-09 17:44:18.265 Full execution log: '/home/ubuntu/.local/state/charmcraft/log/charmcraft-20240509-174417.997582.log'
```


Discussion:

Patch:
diff --git a/charmcraft/services/store.py b/charmcraft/services/store.py
index 785a9da0..546a7996 100644
--- a/charmcraft/services/store.py
+++ b/charmcraft/services/store.py
@@ -22,6 +22,7 @@ from collections.abc import Collection, Mapping, Sequence
 import craft_application
 import craft_store
 from craft_store import models
+from overrides import override
 
 from charmcraft import const, env, errors, store
 from charmcraft.models import CharmLib
@@ -164,6 +165,16 @@ class StoreService(BaseStoreService):
 
     ClientClass = store.Client
     client: store.Client  # pyright: ignore[reportIncompatibleVariableOverride]
+    anonymous_client: store.AnonymousClient
+
+    @override
+    def setup(self) -> None:
+        """Set up the store service."""
+        super().setup()
+        self.anonymous_client = store.AnonymousClient(
+            api_base_url=self._base_url,
+            storage_base_url=self._storage_url,
+        )
 
     def set_resource_revisions_architectures(
         self, name: str, resource_name: str, updates: dict[int, list[str]]
@@ -209,7 +220,7 @@ class StoreService(BaseStoreService):
                 store_lib["patch"] = patch_version
             store_libs.append(store_lib)
 
-        return self.client.fetch_libraries_metadata(store_libs)
+        return self.anonymous_client.fetch_libraries_metadata(store_libs)
 
     def get_libraries_metadata_by_name(
         self, libraries: Sequence[CharmLib]
@@ -224,6 +235,6 @@ class StoreService(BaseStoreService):
         self, charm_name: str, *, library_id: str, api: int | None = None, patch: int | None = None
     ) -> Library:
         """Get a library by charm name and ID from charmhub."""
-        return self.client.get_library(
+        return self.anonymous_client.get_library(
             charm_name=charm_name, library_id=library_id, api=api, patch=patch
         )
diff --git a/charmcraft/store/client.py b/charmcraft/store/client.py
index 9c013260..3bcbce42 100644
--- a/charmcraft/store/client.py
+++ b/charmcraft/store/client.py
@@ -70,6 +70,45 @@ class AnonymousClient:
                 f"Could not retrieve json response ({response.status_code} from request"
             ) from json_error
 
+    def get_library(
+        self, *, charm_name: str, library_id: str, api: int | None = None, patch: int | None = None
+    ) -> Library:
+        """Fetch a library attached to a charm.
+
+        http://api.charmhub.io/docs/libraries.html#fetch_library
+        """
+        params = {}
+        if api is not None:
+            params["api"] = api
+        if patch is not None:
+            params["patch"] = patch
+        return Library.from_dict(
+            self.request_urlpath_json(
+                "GET",
+                f"/v1/charm/libraries/{charm_name}/{library_id}",
+                params=params,
+            )
+        )
+
+    def fetch_libraries_metadata(
+        self, libs: Sequence[LibraryMetadataRequest]
+    ) -> Sequence[Library]:
+        """Fetch the metadata for one or more charm libraries.
+
+        http://api.charmhub.io/docs/libraries.html#fetch_libraries
+        """
+        emit.trace(
+            f"Fetching library metadata from charmhub: {libs}",
+        )
+        response = self.request_urlpath_json("POST", "/v1/charm/libraries/bulk", json=libs)
+        if "libraries" not in response:
+            raise CraftError(
+                "Server returned invalid response while querying libraries", details=str(response)
+            )
+        converted_response = [Library.from_dict(lib) for lib in response["libraries"]]
+        emit.trace(f"Store response: {converted_response}")
+        return converted_response
+
 
 class Client(craft_store.StoreClient):
     """Lightweight layer above StoreClient."""
@@ -171,42 +210,3 @@ class Client(craft_store.StoreClient):
             headers={"Content-Type": monitor.content_type, "Accept": "application/json"},
             data=monitor,
         )
-
-    def get_library(
-        self, *, charm_name: str, library_id: str, api: int | None = None, patch: int | None = None
-    ) -> Library:
-        """Fetch a library attached to a charm.
-
-        http://api.charmhub.io/docs/libraries.html#fetch_library
-        """
-        params = {}
-        if api is not None:
-            params["api"] = api
-        if patch is not None:
-            params["patch"] = patch
-        return Library.from_dict(
-            self.request_urlpath_json(
-                "GET",
-                f"/v1/charm/libraries/{charm_name}/{library_id}",
-                params=params,
-            )
-        )
-
-    def fetch_libraries_metadata(
-        self, libs: Sequence[LibraryMetadataRequest]
-    ) -> Sequence[Library]:
-        """Fetch the metadata for one or more charm libraries.
-
-        http://api.charmhub.io/docs/libraries.html#fetch_libraries
-        """
-        emit.trace(
-            f"Fetching library metadata from charmhub: {libs}",
-        )
-        response = self.request_urlpath_json("POST", "/v1/charm/libraries/bulk", json=libs)
-        if "libraries" not in response:
-            raise CraftError(
-                "Server returned invalid response while querying libraries", details=str(response)
-            )
-        converted_response = [Library.from_dict(lib) for lib in response["libraries"]]
-        emit.trace(f"Store response: {converted_response}")
-        return converted_response
diff --git a/spread.yaml b/spread.yaml
index d9c0b81c..bd5124a4 100644
--- a/spread.yaml
+++ b/spread.yaml
@@ -162,6 +162,8 @@ suites:
     summary: test charm dependencies
     systems:
       - ubuntu-22.04-64
+    kill-timeout: 60m  # Setting up Juju can take a while.
+    priority: 50  # Because setting up Juju takes a while, do these first.
     prepare: |
       snap install juju --channel=3.2/stable
       # Set up Juju controllers for the charms


Test patch:
diff --git a/tests/conftest.py b/tests/conftest.py
index 75cad67e..a875ada9 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -64,9 +64,19 @@ def mock_store_client():
     return client
 
 
+@pytest.fixture()
+def mock_store_anonymous_client() -> mock.Mock:
+    return mock.Mock(spec_set=store.AnonymousClient)
+
+
 @pytest.fixture()
 def service_factory(
-    fs, fake_project_dir, fake_prime_dir, simple_charm, mock_store_client
+    fs,
+    fake_project_dir,
+    fake_prime_dir,
+    simple_charm,
+    mock_store_client,
+    mock_store_anonymous_client,
 ) -> services.CharmcraftServiceFactory:
     factory = services.CharmcraftServiceFactory(app=APP_METADATA)
 
@@ -78,6 +88,7 @@ def service_factory(
     factory.project = simple_charm
 
     factory.store.client = mock_store_client
+    factory.store.anonymous_client = mock_store_anonymous_client
 
     return factory
 
diff --git a/tests/integration/services/conftest.py b/tests/integration/services/conftest.py
index a21a248d..693d797f 100644
--- a/tests/integration/services/conftest.py
+++ b/tests/integration/services/conftest.py
@@ -14,6 +14,10 @@
 #
 # For further info, check https://github.com/canonical/charmcraft
 """Configuration for services integration tests."""
+import contextlib
+import sys
+
+import pyfakefs.fake_filesystem
 import pytest
 
 from charmcraft import services
@@ -21,10 +25,20 @@ from charmcraft.application.main import APP_METADATA, Charmcraft
 
 
 @pytest.fixture()
-def service_factory(fs, fake_path, simple_charm) -> services.CharmcraftServiceFactory:
+def service_factory(
+    fs: pyfakefs.fake_filesystem.FakeFilesystem, fake_path, simple_charm
+) -> services.CharmcraftServiceFactory:
     fake_project_dir = fake_path / "project"
     fake_project_dir.mkdir()
 
+    # Allow access to the real venv library path.
+    # This is necessary because certifi lazy-loads the certificate file.
+    for python_path in sys.path:
+        if not python_path:
+            continue
+        with contextlib.suppress(OSError):
+            fs.add_real_directory(python_path)
+
     factory = services.CharmcraftServiceFactory(app=APP_METADATA)
 
     app = Charmcraft(app=APP_METADATA, services=factory)
diff --git a/tests/integration/services/test_package.py b/tests/integration/services/test_package.py
index 40a7e5ae..9e261e85 100644
--- a/tests/integration/services/test_package.py
+++ b/tests/integration/services/test_package.py
@@ -14,6 +14,7 @@
 #
 # For further info, check https://github.com/canonical/charmcraft
 """Tests for package service."""
+import contextlib
 import datetime
 import pathlib
 
@@ -50,7 +51,8 @@ def package_service(fake_path, service_factory, default_build_plan):
 @freezegun.freeze_time(datetime.datetime(2020, 3, 14, 0, 0, 0, tzinfo=datetime.timezone.utc))
 def test_write_metadata(monkeypatch, fs, package_service, project_path):
     monkeypatch.setattr(charmcraft, "__version__", "3.0-test-version")
-    fs.add_real_directory(project_path)
+    with contextlib.suppress(FileExistsError):
+        fs.add_real_directory(project_path)
     test_prime_dir = pathlib.Path("/prime")
     fs.create_dir(test_prime_dir)
     expected_prime_dir = project_path / "prime"
@@ -79,7 +81,8 @@ def test_overwrite_metadata(monkeypatch, fs, package_service, project_path):
     Regression test for https://github.com/canonical/charmcraft/issues/1654
     """
     monkeypatch.setattr(charmcraft, "__version__", "3.0-test-version")
-    fs.add_real_directory(project_path)
+    with contextlib.suppress(FileExistsError):
+        fs.add_real_directory(project_path)
     test_prime_dir = pathlib.Path("/prime")
     fs.create_dir(test_prime_dir)
     expected_prime_dir = project_path / "prime"
@@ -104,7 +107,8 @@ def test_no_overwrite_reactive_metadata(monkeypatch, fs, package_service):
     """
     monkeypatch.setattr(charmcraft, "__version__", "3.0-test-version")
     project_path = pathlib.Path(__file__).parent / "sample_projects" / "basic-reactive"
-    fs.add_real_directory(project_path)
+    with contextlib.suppress(FileExistsError):
+        fs.add_real_directory(project_path)
     test_prime_dir = pathlib.Path("/prime")
     fs.create_dir(test_prime_dir)
     test_stage_dir = pathlib.Path("/stage")
diff --git a/tests/integration/services/test_store.py b/tests/integration/services/test_store.py
new file mode 100644
index 00000000..05ed870c
--- /dev/null
+++ b/tests/integration/services/test_store.py
@@ -0,0 +1,44 @@
+# Copyright 2024 Canonical Ltd.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+# For further info, check https://github.com/canonical/charmcraft
+"""Integration tests for the store service."""
+
+
+import pytest
+
+from charmcraft import models, services
+
+
+@pytest.fixture()
+def store_service(service_factory):
+    return service_factory.store
+
+
+@pytest.mark.parametrize(
+    "libraries",
+    [
+        [models.CharmLib(lib="observability-libs.cert_handler", version="1")],
+        [models.CharmLib(lib="observability-libs.cert_handler", version="1.8")],
+    ],
+)
+def test_get_libraries(store_service: services.StoreService, libraries):
+    libraries_response = store_service.get_libraries_metadata(libraries)
+    assert len(libraries_response) == len(libraries)
+    for lib in libraries_response:
+        full_lib = store_service.get_library(
+            lib.charm_name, library_id=lib.lib_id, api=lib.api, patch=lib.patch
+        )
+        assert full_lib.content is not None
+        assert full_lib.content_hash == lib.content_hash
diff --git a/tests/spread/smoketests/basic/task.yaml b/tests/spread/smoketests/basic/task.yaml
index c82ebcc9..79ba8ac8 100644
--- a/tests/spread/smoketests/basic/task.yaml
+++ b/tests/spread/smoketests/basic/task.yaml
@@ -1,4 +1,6 @@
 summary: pack a simple charm on various bases
+kill-timeout: 30m  # These sometimes take a while to download bases.
+priority: 50  # Because these can take a while, run them early.
 
 environment:
   BASE_CHANNEL/focal_bases: 20.04
diff --git a/tests/unit/commands/test_store.py b/tests/unit/commands/test_store.py
index 8e9102e5..774b3b37 100644
--- a/tests/unit/commands/test_store.py
+++ b/tests/unit/commands/test_store.py
@@ -173,7 +173,7 @@ def test_fetch_libs_no_charm_libs(
 )
 def test_fetch_libs_missing_from_store(service_factory, libs, expected):
     service_factory.project.charm_libs = libs
-    service_factory.store.client.fetch_libraries_metadata.return_value = []
+    service_factory.store.anonymous_client.fetch_libraries_metadata.return_value = []
     fetch_libs = FetchLibs({"app": APP_METADATA, "services": service_factory})
 
     with pytest.raises(errors.CraftError) as exc_info:
@@ -213,8 +213,8 @@ def test_fetch_libs_missing_from_store(service_factory, libs, expected):
 )
 def test_fetch_libs_no_content(new_path, service_factory, libs, store_libs, dl_lib, expected):
     service_factory.project.charm_libs = libs
-    service_factory.store.client.fetch_libraries_metadata.return_value = store_libs
-    service_factory.store.client.get_library.return_value = dl_lib
+    service_factory.store.anonymous_client.fetch_libraries_metadata.return_value = store_libs
+    service_factory.store.anonymous_client.get_library.return_value = dl_lib
     fetch_libs = FetchLibs({"app": APP_METADATA, "services": service_factory})
 
     with pytest.raises(errors.CraftError, match=expected) as exc_info:
@@ -254,10 +254,10 @@ def test_fetch_libs_no_content(new_path, service_factory, libs, store_libs, dl_l
 )
 def test_fetch_libs_success(
     new_path, emitter, service_factory, libs, store_libs, dl_lib, expected
-):
+) -> None:
     service_factory.project.charm_libs = libs
-    service_factory.store.client.fetch_libraries_metadata.return_value = store_libs
-    service_factory.store.client.get_library.return_value = dl_lib
+    service_factory.store.anonymous_client.fetch_libraries_metadata.return_value = store_libs
+    service_factory.store.anonymous_client.get_library.return_value = dl_lib
     fetch_libs = FetchLibs({"app": APP_METADATA, "services": service_factory})
 
     fetch_libs.run(argparse.Namespace())
diff --git a/tests/unit/services/test_store.py b/tests/unit/services/test_store.py
index 0090a878..540b6629 100644
--- a/tests/unit/services/test_store.py
+++ b/tests/unit/services/test_store.py
@@ -35,6 +35,7 @@ from tests import get_fake_revision
 def store(service_factory) -> services.StoreService:
     store = services.StoreService(app=application.APP_METADATA, services=service_factory)
     store.client = mock.Mock(spec_set=client.Client)
+    store.anonymous_client = mock.Mock(spec_set=client.AnonymousClient)
     return store
 
 
@@ -244,4 +245,4 @@ def test_fetch_libraries_metadata(monkeypatch, store, libs, expected_call):
 
     store.get_libraries_metadata(libs)
 
-    store.client.fetch_libraries_metadata.assert_called_once_with(expected_call)
+    store.anonymous_client.fetch_libraries_metadata.assert_called_once_with(expected_call)
diff --git a/tests/unit/store/test_client.py b/tests/unit/store/test_client.py
index 146a02ed..ccc9df1a 100644
--- a/tests/unit/store/test_client.py
+++ b/tests/unit/store/test_client.py
@@ -27,6 +27,11 @@ def client() -> store.Client:
     return store.Client(api_base_url="http://charmhub.local")
 
 
+@pytest.fixture()
+def anonymous_client() -> store.AnonymousClient:
+    return store.AnonymousClient("http://charmhub.local", "http://storage.charmhub.local")
+
+
 @pytest.mark.parametrize(
     ("charm", "lib_id", "api", "patch", "expected_call"),
     [
@@ -48,7 +53,9 @@ def client() -> store.Client:
         ),
     ],
 )
-def test_get_library_success(monkeypatch, client, charm, lib_id, api, patch, expected_call):
+def test_get_library_success(
+    monkeypatch, anonymous_client, charm, lib_id, api, patch, expected_call
+):
     mock_get_urlpath_json = mock.Mock(
         return_value={
             "charm-name": charm,
@@ -59,9 +66,9 @@ def test_get_library_success(monkeypatch, client, charm, lib_id, api, patch, exp
             "hash": "hashy!",
         }
     )
-    monkeypatch.setattr(client, "request_urlpath_json", mock_get_urlpath_json)
+    monkeypatch.setattr(anonymous_client, "request_urlpath_json", mock_get_urlpath_json)
 
-    client.get_library(charm_name=charm, library_id=lib_id, api=api, patch=patch)
+    anonymous_client.get_library(charm_name=charm, library_id=lib_id, api=api, patch=patch)
 
     mock_get_urlpath_json.assert_has_calls([expected_call])
 
@@ -98,11 +105,11 @@ def test_get_library_success(monkeypatch, client, charm, lib_id, api, patch, exp
         ),
     ],
 )
-def test_fetch_libraries_metadata(monkeypatch, client, libs, json_response, expected):
+def test_fetch_libraries_metadata(monkeypatch, anonymous_client, libs, json_response, expected):
     mock_get_urlpath_json = mock.Mock(return_value=json_response)
-    monkeypatch.setattr(client, "request_urlpath_json", mock_get_urlpath_json)
+    monkeypatch.setattr(anonymous_client, "request_urlpath_json", mock_get_urlpath_json)
 
-    assert client.fetch_libraries_metadata(libs) == expected
+    assert anonymous_client.fetch_libraries_metadata(libs) == expected
 
     mock_get_urlpath_json.assert_has_calls(
         [mock.call("POST", "/v1/charm/libraries/bulk", json=libs)]