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(remotebuild): use legacy launchpad credentials if they exist #4892

Merged
merged 4 commits into from
Jul 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 5 additions & 37 deletions .github/workflows/spread-scheduled.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -51,44 +51,9 @@ jobs:
run: |
spread google:ubuntu-22.04-64:tests/spread/plugins/${{ matrix.type }}/kernel

remote-build-core24:
runs-on: [spread-installed]
needs: [snap-build]
strategy:
fail-fast: false
matrix:
system:
- ubuntu-24.04-64
steps:
- name: Cleanup job workspace
run: |
rm -rf "${{ github.workspace }}"
mkdir "${{ github.workspace }}"
- name: Checkout snapcraft
uses: actions/checkout@v4
with:
fetch-depth: 0
submodules: true
- name: Download snap artifact
uses: actions/download-artifact@v4
with:
name: snap
path: tests
- name: remote-build test
env:
LAUNCHPAD_TOKEN: "${{ secrets.LAUNCHPAD_TOKEN }}"
run: |
spread google:${{ matrix.system }}:tests/spread/core24/remote-build

remote-build:
runs-on: [spread-installed]
needs: [snap-build]
strategy:
fail-fast: false
matrix:
system:
- ubuntu-22.04-64
- fedora-39-64
steps:
- name: Cleanup job workspace
run: |
Expand All @@ -108,5 +73,8 @@ jobs:
env:
LAUNCHPAD_TOKEN: "${{ secrets.LAUNCHPAD_TOKEN }}"
run: |
spread google:${{ matrix.system }}:tests/spread/general/remote-build
spread google:${{ matrix.system }}:tests/spread/core24/remote-build
spread google:ubuntu-20.04-64:tests/spread/core20/remote-build-legacy \
mr-cal marked this conversation as resolved.
Show resolved Hide resolved
google:ubuntu-22.04-64:tests/spread/core22/remote-build-legacy \
google:ubuntu-22.04-64:tests/spread/core22/remote-build \
google:ubuntu-24.04-64:tests/spread/core24/remote-build \
google:fedora-39-64:tests/spread/core24/remote-build:no_platforms
1 change: 1 addition & 0 deletions docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
"common/craft-parts/explanation/overlay_parameters.rst",
"common/craft-parts/explanation/overlays.rst",
"common/craft-parts/how-to/craftctl.rst",
"common/craft-parts/reference/partition_specific_output_directory_variables.rst",
"common/craft-parts/reference/parts_steps.rst",
"common/craft-parts/reference/step_execution_environment.rst",
"common/craft-parts/reference/step_output_directories.rst",
Expand Down
6 changes: 3 additions & 3 deletions docs/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ chardet==5.2.0
charset-normalizer==3.3.2
click==8.1.7
colorama==0.4.6
craft-application==3.0.0
craft-application==3.1.0
craft-archives==1.1.3
craft-cli==2.5.1
craft-cli==2.6.0
craft-grammar==1.2.0
craft-parts==1.32.0
craft-parts==1.33.0
craft-providers==1.23.1
craft-store==2.6.2
cryptography==42.0.5
Expand Down
6 changes: 3 additions & 3 deletions requirements-devel.txt
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ click==8.1.7
codespell==2.2.6
colorama==0.4.6
coverage==7.4.4
craft-application==3.0.0
craft-application==3.1.0
craft-archives==1.1.3
craft-cli==2.5.1
craft-cli==2.6.0
craft-grammar==1.2.0
craft-parts==1.32.0
craft-parts==1.33.0
craft-providers==1.23.1
craft-store==2.6.2
cryptography==42.0.5
Expand Down
6 changes: 3 additions & 3 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ cffi==1.16.0
chardet==5.2.0
charset-normalizer==3.3.2
click==8.1.7
craft-application==3.0.0
craft-application==3.1.0
craft-archives==1.1.3
craft-cli==2.5.1
craft-cli==2.6.0
craft-grammar==1.2.0
craft-parts==1.32.0
craft-parts==1.33.0
craft-providers==1.23.1
craft-store==2.6.2
cryptography==42.0.5
Expand Down
6 changes: 3 additions & 3 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,11 @@ def recursive_data_files(directory, install_directory):
"attrs",
"catkin-pkg; sys_platform == 'linux'",
"click",
"craft-application>=3.0.0",
"craft-application>=3.1.0",
"craft-archives",
"craft-cli",
"craft-cli>=2.6.0",
"craft-grammar",
"craft-parts>=1.32.0",
"craft-parts>=1.33.0",
"craft-providers",
"craft-store",
"docutils<0.20", # Frozen until we can update sphinx dependencies.
Expand Down
15 changes: 0 additions & 15 deletions snapcraft/commands/remote.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
from pathlib import Path
from typing import Any, cast

import craft_cli
import lazr.restfulclient.errors
from craft_application import errors
from craft_application.application import filter_plan
Expand Down Expand Up @@ -147,20 +146,6 @@ def _validate(self, parsed_args: argparse.Namespace) -> None:
reportable=False,
retcode=77,
)
project = cast(models.Project, self._services.project)
if project.architectures:
for arch in project.architectures:
if (
isinstance(arch, models.Architecture)
and arch.build_for
and "all" in arch.build_for
):
raise craft_cli.CraftError(
message="Remote build does not support architecture 'all'.",
resolution="Reconfigure your snap for architecture-dependent builds.",
reportable=False,
retcode=78, # Configuration error
)

def _run(self, parsed_args: argparse.Namespace, **kwargs: Any) -> int | None:
"""Run the remote-build command.
Expand Down
35 changes: 35 additions & 0 deletions snapcraft/services/remotebuild.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,47 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.
"""Snapcraft Lifecycle Service."""

import pathlib

import craft_cli
import platformdirs
from craft_application import launchpad
from craft_application.services import remotebuild
from typing_extensions import override


class RemoteBuild(remotebuild.RemoteBuildService):
"""Snapcraft remote build service."""

RecipeClass = launchpad.models.SnapRecipe

__credentials_filepath: pathlib.Path | None = None

@property
@override
def credentials_filepath(self) -> pathlib.Path:
"""The filepath to the Launchpad credentials.

The legacy credentials are only loaded when they exist and the new credentials
do not exist. If the legacy credentials are loaded, emit a deprecation warning.
"""
# return early so the deprecation notice is emitted only once
if self.__credentials_filepath:
return self.__credentials_filepath

credentials_filepath = super().credentials_filepath
legacy_credentials_filepath = (
platformdirs.user_data_path("snapcraft") / "provider/launchpad/credentials"
)

if not credentials_filepath.exists() and legacy_credentials_filepath.exists():
craft_cli.emit.progress(
f"Warning: Using launchpad credentials from deprecated location {str(legacy_credentials_filepath)!r}.\n"
f"Credentials should be migrated to {str(credentials_filepath)!r}.",
permanent=True,
)
self.__credentials_filepath = legacy_credentials_filepath
else:
self.__credentials_filepath = credentials_filepath

return self.__credentials_filepath
26 changes: 23 additions & 3 deletions snapcraft_legacy/internal/remote_build/_launchpad.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -*-
#
# Copyright (C) 2019 Canonical Ltd
# Copyright (C) 2019,2024 Canonical Ltd
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License version 3 as
Expand All @@ -16,6 +16,8 @@

import gzip
import logging
import pathlib
import platformdirs
import os
import shutil
import time
Expand Down Expand Up @@ -111,7 +113,6 @@ def __init__(

self._cache_dir = self._create_cache_directory()
self._data_dir = self._create_data_directory()
self._credentials = os.path.join(self._data_dir, "credentials")

self._lp: Launchpad = self.login()
self.user = self._lp.me.name
Expand All @@ -122,6 +123,25 @@ def __init__(
def architectures(self) -> Sequence[str]:
return self._architectures

@property
def _credentials_filepath(self) -> pathlib.Path:
"""The filepath to the Launchpad credentials.

If the credentials file does not exist in the default location but exists in the
legacy location, emit a deprecation warning and return the legacy location.
"""
credentials_filepath = platformdirs.user_data_path("snapcraft") / "launchpad-credentials"
legacy_credentials_filepath = platformdirs.user_data_path("snapcraft") / "provider/launchpad/credentials"

if not credentials_filepath.exists() and legacy_credentials_filepath.exists():
logger.warning(
f"Warning: Using launchpad credentials from deprecated location {str(legacy_credentials_filepath)!r}.\n"
f"Credentials should be migrated to {str(credentials_filepath)!r}."
)
return legacy_credentials_filepath

return credentials_filepath

@architectures.setter
def architectures(self, architectures: Sequence[str]) -> None:
self._lp_processors: Optional[Sequence[str]] = None
Expand Down Expand Up @@ -256,7 +276,7 @@ def login(self) -> Launchpad:
"snapcraft remote-build {}".format(snapcraft_legacy.__version__),
"production",
self._cache_dir,
credentials_file=self._credentials,
credentials_file=self._credentials_filepath,
version="devel",
)
except (ConnectionRefusedError, TimeoutError):
Expand Down
5 changes: 5 additions & 0 deletions spread.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,11 @@ restore-each: |
"$TOOLS_DIR"/restore.sh

suites:
tests/spread/core20/:
summary: core20 specific tests
systems:
- ubuntu-20.04*

tests/spread/core22/:
summary: core22 specific tests
systems:
Expand Down
28 changes: 27 additions & 1 deletion tests/legacy/unit/remote_build/test_launchpad.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -*-
#
# Copyright (C) 2019 Canonical Ltd
# Copyright (C) 2019,2024 Canonical Ltd
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License version 3 as
Expand All @@ -16,6 +16,8 @@

import textwrap
from datetime import datetime, timedelta, timezone
import pathlib
import pytest
from unittest import mock

import fixtures
Expand Down Expand Up @@ -535,3 +537,27 @@ def test_push_source_tree_error(self):
self.assertRaises(
errors.LaunchpadGitPushError, self.lpc.push_source_tree, repo_dir
)


@pytest.mark.parametrize(
("new_exists", "legacy_exists", "expected"),
[
(False, False, pathlib.Path("launchpad-credentials")),
(False, True, pathlib.Path("provider/launchpad/credentials")),
(True, False, pathlib.Path("launchpad-credentials")),
(True, True, pathlib.Path("launchpad-credentials")),
],
)
def test_credentials_filepaths(new_exists, legacy_exists, expected, mocker, new_dir):
"""Load legacy credentials only when they exist and the new ones do not."""
mocker.patch.object(LaunchpadClient, "__init__", lambda x: None)
mocker.patch("platformdirs.user_data_path", return_value=new_dir)
if new_exists:
(new_dir / "launchpad-credentials").touch()
if legacy_exists:
(new_dir / "provider/launchpad/credentials").mkdir(parents=True)
(new_dir / "provider/launchpad/credentials").touch()

credentials_filepath = LaunchpadClient()._credentials_filepath

assert credentials_filepath == new_dir / expected
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
test-snap-all-core20_1.0_all.snap
16 changes: 16 additions & 0 deletions tests/spread/core20/remote-build-legacy/snaps/all/snapcraft.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
name: test-snap-all-core20
base: core20
version: "1.0"
summary: Test snap for remote build
description: Test snap for remote build

grade: stable
confinement: strict

architectures:
- build-on: [amd64]
run-on: [all]

parts:
my-part:
plugin: nil
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
test-snap-architectures-core20_1.0_amd64.snap
test-snap-architectures-core20_1.0_arm64.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
name: test-snap-architectures-core20
base: core20
version: "1.0"
summary: Test snap for remote build
description: Test snap for remote build

grade: stable
confinement: strict

architectures:
- build-on: [amd64]
run-on: [amd64]
- build-on: [arm64]
run-on: [arm64]

parts:
my-part:
plugin: nil
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
test-snap-no-architectures-core20_1.0_amd64.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
name: test-snap-no-architectures-core20
base: core20
version: "1.0"
summary: Test snap for remote build
description: Test snap for remote build

grade: stable
confinement: strict

parts:
my-part:
plugin: nil
Loading
Loading