Skip to content

Commit

Permalink
fix(launchpad): use the project's privacy for repos and recipes (#615)
Browse files Browse the repository at this point in the history
Signed-off-by: Callahan Kovacs <callahan.kovacs@canonical.com>
  • Loading branch information
mr-cal authored and lengau committed Jan 29, 2025
1 parent 537cf7b commit a98336d
Show file tree
Hide file tree
Showing 7 changed files with 203 additions and 21 deletions.
14 changes: 13 additions & 1 deletion craft_application/launchpad/launchpad.py
Original file line number Diff line number Diff line change
Expand Up @@ -239,16 +239,28 @@ def new_repository(
name: str,
owner: str | None = None,
project: str | models.Project | None = None,
information_type: models.InformationType | None = None,
) -> models.GitRepository:
"""Create a new git repository.
:param name: The name of the repository.
:param owner: (Optional) the username of the owner (defaults to oneself).
:param project: (Optional) the project to which the repository will be attached.
Defines the information type of the repository if 'information_type' is not set.
:param information_type: (Optional) The information type of the repository
This overrides the project's information type. Defaults to public.
"""
kwargs: dict[str, Any] = {}
if information_type:
kwargs["information_type"] = information_type
elif isinstance(project, models.Project):
kwargs["information_type"] = project.information_type

if isinstance(project, models.Project):
project = project.name

if owner is None:
owner = self.username

return models.GitRepository.new(self, name, owner, project)
# repos default to public if information_type is not set
return models.GitRepository.new(self, name, owner, project, **kwargs)
2 changes: 1 addition & 1 deletion craft_application/launchpad/models/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ def __setattr__(self, key: str, value: Any) -> None: # noqa: ANN401
return
annotations = util.get_annotations(self.__class__)
if key in annotations:
attr_path = self._attr_map.get(key, default=key)
attr_path = self._attr_map.get(key, key)
util.set_innermost_attr(self._obj, attr_path, value)
elif key in self._attr_map:
util.set_innermost_attr(self._obj, self._attr_map[key], value)
Expand Down
39 changes: 35 additions & 4 deletions craft_application/launchpad/models/recipe.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
import lazr.restfulclient.errors # type: ignore[import-untyped]
from typing_extensions import Any, Self, TypedDict, override

from craft_application.launchpad import errors, util
from craft_application.launchpad import errors, models, util
from craft_application.util import retry

from . import build
Expand Down Expand Up @@ -185,7 +185,8 @@ def new( # noqa: PLR0913
*,
architectures: Collection[str] | None = None,
description: str | None = None,
project: str | None = None,
project: str | models.Project | None = None,
information_type: models.InformationType | None = None,
# Repository. Must be either a git repository or a Bazaar branch but not both.
git_ref: str | None = None,
bzr_branch: str | None = None,
Expand All @@ -207,7 +208,11 @@ def new( # noqa: PLR0913
:param architectures: A collection of architecture names to build the recipe.
If None, detects the architectures from `snapcraft.yaml`
:param description: (Optional) A description of the recipe.
:param project: (Optional) The name of the project to which to attach this recipe.
:param project: (Optional) The project or name of the project to which to
attach this recipe. Defines the information type of the repository if
'information_type' is not set.
:param information_type: (Optional) The information type of the recipe.
This overrides the project's information type. Defaults to public.
:param git_ref: A link to a git repository and branch from which to build.
Mutually exclusive with bzr_branch.
:param bzr_branch: A link to a bazaar branch from which to build.
Expand All @@ -230,8 +235,18 @@ def new( # noqa: PLR0913
kwargs["processors"] = [util.get_processor(arch) for arch in architectures]
if description:
kwargs["description"] = description

# set information type from the arg or the project
if information_type:
kwargs["information_type"] = information_type.value
elif isinstance(project, models.Project):
kwargs["information_type"] = project.information_type.value

if isinstance(project, models.Project):
project = project.name
if project:
kwargs["project"] = f"/{project}"

cls._fill_repo_info(kwargs, git_ref=git_ref, bzr_branch=bzr_branch)
cls._fill_store_info(
kwargs,
Expand Down Expand Up @@ -347,9 +362,10 @@ def new( # noqa: PLR0913
lp: Launchpad,
name: str,
owner: str,
project: str,
project: str | models.Project,
*,
build_path: str | None = None,
information_type: models.InformationType | None = None,
# Automatic build options.
auto_build: bool = False,
auto_build_channels: BuildChannels | None = None,
Expand All @@ -365,8 +381,13 @@ def new( # noqa: PLR0913
:param name: The recipe name
:param owner: The username of the person or team who owns the recipe
:param project: The name of the project to which this recipe should be attached.
:param project: The project or name of the project to which to attach this
recipe. Defines the information type of the repository if 'information_type'
is not set.
:param build_path: (Optional) The path to the directory containing
the project file (if it's not the root directory).
:param information_type: (Optional) The information type of the recipe.
This overrides the project's information type. Defaults to public.
:param git_ref: A link to a git repository and branch from which to build.
Mutually exclusive with bzr_branch.
:param auto_build: Whether to automatically build on pushes to the branch.
Expand All @@ -392,6 +413,16 @@ def new( # noqa: PLR0913
kwargs["auto_build_channels"] = auto_build_channels
if build_path:
kwargs["build_path"] = build_path

# set information type from the arg or the project
if information_type:
kwargs["information_type"] = information_type.value
elif isinstance(project, models.Project):
kwargs["information_type"] = project.information_type.value

if isinstance(project, models.Project):
project = project.name

cls._fill_store_info(
kwargs,
store_name=store_name,
Expand Down
35 changes: 26 additions & 9 deletions craft_application/services/remotebuild.py
Original file line number Diff line number Diff line change
Expand Up @@ -272,9 +272,7 @@ def _ensure_repository(
work_tree = WorkTree(self._app.name, self._name, project_dir)
work_tree.init_repo()
try:
lp_repository = self.lp.new_repository(
self._name, project=self._lp_project.name
)
lp_repository = self.lp.new_repository(self._name, project=self._lp_project)
except launchpadlib.errors.HTTPError:
lp_repository = self.lp.get_repository(
name=self._name, project=self._lp_project.name
Expand All @@ -285,10 +283,7 @@ def _ensure_repository(
expiry=datetime.datetime.now(tz=datetime.timezone.utc)
+ datetime.timedelta(seconds=300),
)
repo_url = parse.urlparse(str(lp_repository.git_https_url))
push_url = repo_url._replace(
netloc=f"{self.lp.lp.me.name}:{token}@{repo_url.netloc}" # pyright: ignore[reportOptionalMemberAccess,reportAttributeAccessIssue,reportUnknownMemberType]
)
push_url = self._get_push_url(lp_repository, token)

try:
local_repository = GitRepo(work_tree.repo_dir)
Expand All @@ -300,6 +295,25 @@ def _ensure_repository(
else:
return work_tree, lp_repository

def _get_push_url(
self, lp_repository: launchpad.models.GitRepository, token: str
) -> urllib.parse.ParseResult:
if self.is_project_private():
# private repositories can only be accessed via ssh
repo_url = parse.urlparse(str(lp_repository.git_ssh_url))
push_url = repo_url._replace(
netloc=f"{self.lp.lp.me.name}@{repo_url.netloc}" # pyright: ignore[reportOptionalMemberAccess,reportAttributeAccessIssue,reportUnknownMemberType]
)
craft_cli.emit.debug(f"Using ssh url for private repository: {push_url}")
else:
repo_url = parse.urlparse(str(lp_repository.git_https_url))
push_url = repo_url._replace(
netloc=f"{self.lp.lp.me.name}:{token}@{repo_url.netloc}" # pyright: ignore[reportOptionalMemberAccess,reportAttributeAccessIssue,reportUnknownMemberType]
)
craft_cli.emit.debug(f"Using https url for public repository: {push_url}")

return push_url

def _get_repository(self) -> launchpad.models.GitRepository:
"""Get an existing repository on Launchpad."""
return self.lp.get_repository(name=self._name, owner=self.lp.username)
Expand All @@ -325,7 +339,10 @@ def _new_recipe(
) -> launchpad.models.Recipe:
"""Create a new recipe for the given repository."""
repository.lp_refresh() # Prevents a race condition on new repositories.
git_ref = parse.urlparse(str(repository.git_https_url)).path + "/+ref/main"

# public repos use https for backward compatibility
url = repository.git_ssh_url if repository.private else repository.git_https_url
git_ref = parse.urlparse(str(url)).path + "/+ref/main"

# if kwargs has a collection of 'architectures', replace 'all' with 'amd64'
# because the amd64 runners are fast to build on
Expand All @@ -339,7 +356,7 @@ def _new_recipe(
name,
self.lp.username,
git_ref=git_ref,
project=self._lp_project.name,
project=self._lp_project,
**kwargs,
)

Expand Down
88 changes: 88 additions & 0 deletions tests/unit/launchpad/test_launchpad.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,14 @@ def flatten_enum(e: type[enum.Enum]) -> list:
return [*e, *(val.name for val in e), *(val.value for val in e)]


@pytest.fixture
def mock_project():
_mock_project = mock.Mock(spec=models.Project)
_mock_project.name = "test-name"
_mock_project.information_type = models.InformationType.PUBLIC
return _mock_project


@pytest.mark.parametrize(
"cache_path",
[
Expand Down Expand Up @@ -184,6 +192,86 @@ def test_get_recipe_snap(fake_launchpad, type_, owner):
)


@pytest.mark.parametrize("information_type", list(models.InformationType))
@pytest.mark.parametrize("project", [None, "test-project", mock_project])
@pytest.mark.parametrize(
("recipe", "recipe_name"),
[
(models.SnapRecipe, "snaps"),
(models.CharmRecipe, "charm_recipes"),
(models.RockRecipe, "rock_recipes"),
],
)
def test_recipe_new_info_type(information_type, project, recipe, recipe_name):
"""Pass the information_type to the recipe."""
mock_launchpad = mock.Mock(spec=launchpad.Launchpad)
mock_launchpad.lp = mock.Mock(spec=launchpadlib.launchpad.Launchpad)
mock_recipe = mock.Mock()

mock_entry = mock.Mock(spec=Entry)
mock_entry.resource_type_link = "http://blah/#snap"
mock_entry.name = "test-snap"

mock_recipe.new = mock.Mock(return_value=mock_entry)
setattr(mock_launchpad.lp, f"{recipe_name}", mock_recipe)

actual_recipe = recipe.new(
mock_launchpad,
"my_recipe",
"test_user",
git_ref="my_ref",
# `information_type` overrides the project's information_type
project=project,
information_type=information_type,
)

assert isinstance(actual_recipe, recipe)
assert (
mock_recipe.new.mock_calls[0].kwargs["information_type"]
== information_type.value
)


@pytest.mark.parametrize("information_type", list(models.InformationType))
@pytest.mark.parametrize(
("recipe", "recipe_name"),
[
(models.SnapRecipe, "snaps"),
(models.CharmRecipe, "charm_recipes"),
(models.RockRecipe, "rock_recipes"),
],
)
def test_recipe_new_info_type_in_project(
mock_project, information_type, recipe, recipe_name
):
"""Use the info type from the project."""
mock_launchpad = mock.Mock(spec=launchpad.Launchpad)
mock_launchpad.lp = mock.Mock(spec=launchpadlib.launchpad.Launchpad)
mock_recipe = mock.Mock()

mock_entry = mock.Mock(spec=Entry)
mock_entry.resource_type_link = "http://blah/#snap"
mock_entry.name = "test-snap"

mock_recipe.new = mock.Mock(return_value=mock_entry)
setattr(mock_launchpad.lp, f"{recipe_name}", mock_recipe)
mock_project.information_type = information_type

actual_recipe = recipe.new(
mock_launchpad,
"my_recipe",
"test_user",
project=mock_project,
git_ref="my_ref",
)

assert isinstance(actual_recipe, recipe)
assert (
mock_recipe.new.mock_calls[0].kwargs["information_type"]
== information_type.value
)


def test_recipe_snap_new_retry(emitter, mocker):
"""Test that a SnapRecipe is retried when it fails to create."""
mocker.patch("time.sleep")
Expand Down
3 changes: 3 additions & 0 deletions tests/unit/services/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ def mock_project_entry():
return get_mock_lazr_entry(
resource_type="project",
name="craft_test_user-craft-remote-build",
information_type=launchpad.models.InformationType.PUBLIC.value,
)


Expand All @@ -65,6 +66,8 @@ def mock_git_repository():
"git_repository",
issueAccessToken=mock.Mock(spec=Callable, return_value="super_secret_token"),
git_https_url="https://git.launchpad.net/",
git_ssh_url="ssh:git.launchpad.net/",
private=False,
)


Expand Down
Loading

0 comments on commit a98336d

Please sign in to comment.