Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: allow fetch-libs to work without logging in #1702

Merged
merged 9 commits into from
Jun 13, 2024
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(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this feature (anonymous access) something that we should (eventually) upstream in BaseStoreService?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eventually once it's a craft_store feature, yeah.

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
Loading