Skip to content

Commit

Permalink
fix: allow fetch-libs to work without logging in (#1702)
Browse files Browse the repository at this point in the history
Fixes #1681
  • Loading branch information
lengau authored Jun 13, 2024
1 parent bcb1903 commit 1da46c8
Show file tree
Hide file tree
Showing 9 changed files with 151 additions and 59 deletions.
15 changes: 13 additions & 2 deletions charmcraft/services/store.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
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
Expand Down Expand Up @@ -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]]
Expand Down Expand Up @@ -209,7 +220,7 @@ def get_libraries_metadata(self, libraries: Sequence[CharmLib]) -> Sequence[Libr
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]
Expand All @@ -224,6 +235,6 @@ def get_library(
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
)
78 changes: 39 additions & 39 deletions charmcraft/store/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,45 @@ def request_urlpath_json(self, method: str, urlpath: str, *args, **kwargs) -> di
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."""
Expand Down Expand Up @@ -171,42 +210,3 @@ def _storage_push(self, monitor) -> requests.Response:
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
13 changes: 12 additions & 1 deletion tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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

Expand Down
16 changes: 15 additions & 1 deletion tests/integration/services/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,31 @@
#
# 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
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)
Expand Down
10 changes: 7 additions & 3 deletions tests/integration/services/test_package.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#
# For further info, check https://github.com/canonical/charmcraft
"""Tests for package service."""
import contextlib
import datetime
import pathlib

Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand All @@ -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")
Expand Down
44 changes: 44 additions & 0 deletions tests/integration/services/test_store.py
Original file line number Diff line number Diff line change
@@ -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
12 changes: 6 additions & 6 deletions tests/unit/commands/test_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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())
Expand Down
3 changes: 2 additions & 1 deletion tests/unit/services/test_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
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


Expand Down Expand Up @@ -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)
19 changes: 13 additions & 6 deletions tests/unit/store/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
[
Expand All @@ -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,
Expand All @@ -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])

Expand Down Expand Up @@ -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)]
Expand Down

0 comments on commit 1da46c8

Please sign in to comment.