From 39f7ff80a25d5134e2671159cbe1ea9352e18796 Mon Sep 17 00:00:00 2001 From: Adrian Clay Lake Date: Fri, 20 Sep 2024 13:40:15 +0200 Subject: [PATCH 01/14] feat: add Ubuntu Pro argument and validation (#438) * feat: add pro argument to lifecycle commands * feat: added testing to pro argument * feat: add ProServices class * refactor: migrated to pro python api * refactor(application.py): migrated validation instructions to run function * refactor: pro services validation * feat: testing support for pro service in existing tests * feat: parameterized testing of Peo services validation * refactor: fixed spelling * refactor: fixed pyright issues, needs future rework * refactor: address tox lint issues * refactor: applied suggestions from PR * fix(craft_application/commands/lifecycle.py): add argparse permanently as it is needed for SUPPRESS option * fix(craft_application/util/pro_services.py): incorrect service name in build_service_scope * feat: added exception for unsupported pro services * fix(craft_application/commands/lifecycle.py): missing lines in pro help msg * feat: add "fips" as --pro argument value * feat: only check pro status on host in managed mode * refactor(craft_application/application.py): update comments * fix: linting issues found in #438 * refactor: switch to pytest.raises + nullcontext instead of try/except * feat: added validation to managed mode * refactor: fixes for linting problems * feat: ignore validation outside of lifecycle commands * refactor: export ProServices, and ValidatorOptions in __init__.py * refactor: noted ubuntu requirement in --pro arg help string * refactor: applied black reformat --- craft_application/application.py | 107 +++++++----- craft_application/commands/lifecycle.py | 49 ++++-- craft_application/errors.py | 84 +++++++++ craft_application/util/__init__.py | 3 + craft_application/util/pro_services.py | 220 ++++++++++++++++++++++++ tests/conftest.py | 39 +++++ tests/unit/commands/test_lifecycle.py | 124 ++++++++++++- tests/unit/test_application.py | 127 +++++++++++++- 8 files changed, 688 insertions(+), 65 deletions(-) create mode 100644 craft_application/util/pro_services.py diff --git a/craft_application/application.py b/craft_application/application.py index 36ab639a6..286bc2896 100644 --- a/craft_application/application.py +++ b/craft_application/application.py @@ -38,6 +38,7 @@ from craft_application import _config, commands, errors, models, util from craft_application.errors import PathInvalidError +from craft_application.util import ValidatorOptions if TYPE_CHECKING: import argparse @@ -560,6 +561,7 @@ def _pre_run(self, dispatcher: craft_cli.Dispatcher) -> None: # Some commands might have a project_dir parameter. Those commands and # only those commands should get a project directory, but only when # not managed. + if self.is_managed(): self.project_dir = pathlib.Path("/root/project") elif project_dir := getattr(args, "project_dir", None): @@ -591,13 +593,21 @@ def get_arg_or_config(self, parsed_args: argparse.Namespace, item: str) -> Any: def _run_inner(self) -> int: """Actual run implementation.""" dispatcher = self._get_dispatcher() - command = cast( - commands.AppCommand, - dispatcher.load_command(self.app_config), - ) - parsed_args = dispatcher.parsed_args() - platform = self.get_arg_or_config(parsed_args, "platform") - build_for = self.get_arg_or_config(parsed_args, "build_for") + craft_cli.emit.debug("Preparing application...") + + return_code = 1 # General error + try: + command = cast( + commands.AppCommand, + dispatcher.load_command(self.app_config), + ) + + platform = getattr(dispatcher.parsed_args(), "platform", None) + build_for = getattr(dispatcher.parsed_args(), "build_for", None) + pro_services = getattr(dispatcher.parsed_args(), "pro", None) + + run_managed = command.run_managed(dispatcher.parsed_args()) + is_managed = self.is_managed() # Some commands (e.g. remote build) can allow multiple platforms # or build-fors, comma-separated. In these cases, we create the @@ -608,46 +618,53 @@ def _run_inner(self) -> int: build_for = build_for.split(",", maxsplit=1)[0] craft_cli.emit.debug(f"Build plan: platform={platform}, build_for={build_for}") - self._pre_run(dispatcher) - - if command.needs_project(parsed_args): - project_service = self.services.get("project") - # This branch always runs, except during testing. - if not project_service.is_configured: - project_service.configure(platform=platform, build_for=build_for) - - managed_mode = command.run_managed(parsed_args) - provider_name = command.provider_name(parsed_args) - self._configure_services(provider_name) - - return_code = 1 # General error - if not managed_mode: - # command runs in the outer instance - craft_cli.emit.debug(f"Running {self.app.name} {command.name} on host") - return_code = dispatcher.run() or os.EX_OK - elif not self.is_managed(): - # command runs in inner instance, but this is the outer instance - self.run_managed(platform, build_for) - return_code = os.EX_OK - else: - # command runs in inner instance - return_code = dispatcher.run() or 0 - - return return_code - - def run(self) -> int: - """Bootstrap and run the application.""" - self._setup_logging() - self._configure_early_services() - self._initialize_craft_parts() - self._load_plugins() - - craft_cli.emit.debug("Preparing application...") + # Check that pro services are correctly configured. A ProServices instance will + # only be available for lifecycle commands, otherwise we default to None + if pro_services is not None: + # Validate requested pro services on the host if we are running in destructive mode... + if not run_managed and not is_managed: + craft_cli.emit.debug( + f"Validating requested Ubuntu Pro status on host: {pro_services}" + ) + pro_services.validate() + # .. or running in managed mode inside a managed instance + elif run_managed and is_managed: + craft_cli.emit.debug( + f"Validating requested Ubuntu Pro status in managed instance: {pro_services}" + ) + pro_services.validate() + # .. or validate pro attachment and service names on the host before starting a managed instance. + elif run_managed and not is_managed: + craft_cli.emit.debug( + f"Validating requested Ubuntu Pro attachment on host: {pro_services}" + ) + pro_services.validate( + options=ValidatorOptions.ATTACHMENT | ValidatorOptions.SUPPORT + ) - debug_mode = self.services.get("config").get("debug") + if run_managed or command.needs_project(dispatcher.parsed_args()): + self.services.project = self.get_project( + platform=platform, build_for=build_for + ) - try: - return_code = self._run_inner() + craft_cli.emit.debug( + f"Build plan: platform={platform}, build_for={build_for}" + ) + self._pre_run(dispatcher) + + self._configure_services(provider_name) + + if not run_managed: + # command runs in the outer instance + craft_cli.emit.debug(f"Running {self.app.name} {command.name} on host") + return_code = dispatcher.run() or os.EX_OK + elif not is_managed: + # command runs in inner instance, but this is the outer instance + self.run_managed(platform, build_for) + return_code = os.EX_OK + else: + # command runs in inner instance + return_code = dispatcher.run() or 0 except craft_cli.ArgumentParsingError as err: print(err, file=sys.stderr) # to stderr, as argparse normally does craft_cli.emit.ended_ok() diff --git a/craft_application/commands/lifecycle.py b/craft_application/commands/lifecycle.py index edc4244a3..2b406151a 100644 --- a/craft_application/commands/lifecycle.py +++ b/craft_application/commands/lifecycle.py @@ -28,6 +28,7 @@ from craft_application import errors, models, util from craft_application.commands import base +from craft_application.util import ProServices _PACKED_FILE_LIST_PATH = ".craft/packed-files" @@ -72,18 +73,42 @@ def _fill_parser(self, parser: argparse.ArgumentParser) -> None: super()._fill_parser(parser) # type: ignore[arg-type] group = parser.add_mutually_exclusive_group() - if self._allow_destructive: - group.add_argument( - "--destructive-mode", - action="store_true", - help="Build in the current host", - ) - if self._show_lxd_arg: - group.add_argument( - "--use-lxd", - action="store_true", - help="Build in a LXD container.", - ) + group.add_argument( + "--destructive-mode", + action="store_true", + help="Build in the current host", + ) + group.add_argument( + "--use-lxd", + action="store_true", + help="Build in a LXD container.", + ) + + supported_pro_services = ", ".join( + [f"'{name}'" for name in ProServices.supported_services] + ) + + parser.add_argument( + "--pro", + type=ProServices.from_csv, + metavar="", + help=( + "Enable Ubuntu Pro services for this command. " + f"Supported values include: {supported_pro_services}. " + "Multiple values can be passed separated by commas. " + "Note: This feature requires an Ubuntu Pro compatible host and build base." + ), + default=ProServices(), + ) + + # TODO: do we need this? + # @override + # def get_managed_cmd(self, parsed_args: argparse.Namespace) -> list[str]: + # cmd = super().get_managed_cmd(parsed_args) + + # cmd.extend(parsed_args.parts) + + # return cmd @override def provider_name(self, parsed_args: argparse.Namespace) -> str | None: diff --git a/craft_application/errors.py b/craft_application/errors.py index e7f9c6b08..e2a8884eb 100644 --- a/craft_application/errors.py +++ b/craft_application/errors.py @@ -384,3 +384,87 @@ class ArtifactCreationError(CraftError): class StateServiceError(CraftError): """Errors related to the state service.""" + + +class UbuntuProError(CraftError): + """Base Exception class for ProServices.""" + + +class UbuntuProApiError(UbuntuProError): + """Base class for exceptions raised during Ubuntu Pro Api calls.""" + + +class InvalidUbuntuProStateError(UbuntuProError): + """Base class for exceptions raised during Ubuntu Pro validation.""" + + # TODO: some of the resolution strings may not sense in a managed + # environment. What is the best way to get the is_managed method here? + + +class UbuntuProClientNotFoundError(UbuntuProApiError): + """Raised when Ubuntu Pro client was not found on the system.""" + + def __init__(self, path: str) -> None: + message = f'The Ubuntu Pro client was not found on the system at "{path}"' + + super().__init__(message=message) + + +class UbuntuProDetachedError(InvalidUbuntuProStateError): + """Raised when Ubuntu Pro is not attached, but Pro services were requested.""" + + def __init__(self) -> None: + message = "Ubuntu Pro is requested, but was found detached." + resolution = 'Attach Ubuntu Pro to continue. See "pro" command for details.' + + super().__init__(message=message, resolution=resolution) + + +class UbuntuProAttachedError(InvalidUbuntuProStateError): + """Raised when Ubuntu Pro is attached, but Pro services were not requested.""" + + def __init__(self) -> None: + message = "Ubuntu Pro is not requested, but was found attached." + resolution = 'Detach Ubuntu Pro to continue. See "pro" command for details.' + + super().__init__(message=message, resolution=resolution) + + +class InvalidUbuntuProServiceError(InvalidUbuntuProStateError): + """Raised when the requested Ubuntu Pro service is not supported or invalid.""" + + # TODO: Should there be separate exceptions for services that not supported vs. invalid? + # if so where is the list of supported service names? + + def __init__(self, invalid_services: set[str]) -> None: + invalid_services_str = "".join(invalid_services) + + message = "Invalid Ubuntu Pro Services were requested." + resolution = ( + "The services listed are either not supported by this application " + "or are invalid Ubuntu Pro Services.\n" + f"Invalid Services: {invalid_services_str}\n" + 'See "--pro" argument details for supported services.' + ) + + super().__init__(message=message, resolution=resolution) + + +class InvalidUbuntuProStatusError(InvalidUbuntuProStateError): + """Raised when the incorrect set of Pro Services are enabled.""" + + def __init__( + self, requested_services: set[str], available_services: set[str] + ) -> None: + enable_services_str = " ".join(requested_services - available_services) + disable_services_str = " ".join(available_services - requested_services) + + message = "Incorrect Ubuntu Pro Services were enabled." + resolution = ( + "Please enable or disable the following services.\n" + f"Enable: {enable_services_str}\n" + f"Disable: {disable_services_str}\n" + 'See "pro" command for details.' + ) + + super().__init__(message=message, resolution=resolution) diff --git a/craft_application/util/__init__.py b/craft_application/util/__init__.py index aade87a8c..de0092167 100644 --- a/craft_application/util/__init__.py +++ b/craft_application/util/__init__.py @@ -43,6 +43,7 @@ from craft_application.util.system import get_parallel_build_count from craft_application.util.yaml import dump_yaml, safe_yaml_load from craft_application.util.cli import format_timestamp +from craft_application.util.pro_services import ProServices, ValidatorOptions __all__ = [ "get_unique_callbacks", @@ -69,4 +70,6 @@ "get_hostname", "is_managed_mode", "format_timestamp", + "ProServices", + "ValidatorOptions", ] diff --git a/craft_application/util/pro_services.py b/craft_application/util/pro_services.py new file mode 100644 index 000000000..70bec90a0 --- /dev/null +++ b/craft_application/util/pro_services.py @@ -0,0 +1,220 @@ +# This file is part of craft_application. +# +# Copyright 2024 Canonical Ltd. +# +# This program is free software: you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License version 3, as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranties of MERCHANTABILITY, +# SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. +# See the GNU Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License along +# with this program. If not, see . +"""Handling of Ubuntu Pro Services.""" +from __future__ import annotations + +import json +import logging +import subprocess as sub +from enum import Flag, auto +from pathlib import Path +from typing import Any + +from craft_application.errors import ( + InvalidUbuntuProServiceError, + InvalidUbuntuProStatusError, + UbuntuProApiError, + UbuntuProAttachedError, + UbuntuProClientNotFoundError, + UbuntuProDetachedError, +) + +logger = logging.getLogger(__name__) + + +# locations to search for pro executable +# TODO: which path will we support in long term? +PRO_CLIENT_PATHS = [ + Path("/usr/bin/ubuntu-advantage"), + Path("/usr/bin/ua"), + Path("/usr/bin/pro"), +] + + +class ValidatorOptions(Flag): + """Options for ProServices.validate method. + + SUPPORT: Check names in ProServices set against supported services. + AVAILABILITY: Check Ubuntu Pro is attached if ProServices set is not empty + ATTACHMENT: Check Ubuntu Pro is attached or detached to match ProServices set. + ENABLEMENT: Check enabled Ubuntu Pro enablement matches ProServices set. + """ + + SUPPORT = auto() + _ATTACHED = auto() + _DETACHED = auto() + # TODO: remove AVAILABILITY if not needed. This flag is useful if we can manually control + # if a managed instance is pro or not. It allows us to check if the host has + # any pro services to support a pro build. In this case, if pro is not requested + # the managed instance would not be attached. + AVAILABILITY = _ATTACHED + ATTACHMENT = _ATTACHED | _DETACHED + ENABLEMENT = auto() + DEFAULT = SUPPORT | ATTACHMENT | ENABLEMENT + + +class ProServices(set[str]): + """Class for managing pro-services within the lifecycle.""" + + # placeholder for empty sets + empty_placeholder = "none" + + supported_services: set[str] = { + "esm-apps", + "esm-infra", + "fips", + "fips-preview", + "fips-updates", + } + + # location of pro client + pro_executable: Path | None = next( + (path for path in PRO_CLIENT_PATHS if path.exists()), None + ) + # locations to check for pro client + + def __str__(self) -> str: + """Convert to string for display to user.""" + return ", ".join(self) if self else self.empty_placeholder + + @classmethod + def from_csv(cls, services: str) -> ProServices: + """Create a new ProServices instance from a csv string.""" + split = [service.strip() for service in services.split(",")] + return cls(split) + + @classmethod + def pro_client_exists(cls) -> bool: + """Check if Ubuntu Pro executable exists or not.""" + return cls.pro_executable is not None and cls.pro_executable.exists() + + @classmethod + def _log_processes(cls, process: sub.CompletedProcess[str]) -> None: + logger.error( + "Ubuntu Pro Client Response: \n" + f"Return Code: {process.returncode}\n" + f"StdOut:\n{process.stdout}\n" + f"StdErr:\n{process.stderr}\n" + ) + + @classmethod + def _pro_api_call(cls, endpoint: str) -> dict[str, Any]: + """Call Ubuntu Pro executable and parse response.""" + if not cls.pro_client_exists(): + raise UbuntuProClientNotFoundError(str(cls.pro_executable)) + + try: + proc = sub.run( + [str(cls.pro_executable), "api", endpoint], + capture_output=True, + text=True, + check=False, + ) + except Exception as exc: + raise UbuntuProApiError( + f'An error occurred while executing "{cls.pro_executable}"' + ) from exc + + if proc.returncode != 0: + cls._log_processes(proc) + raise UbuntuProApiError( + f"The Pro Client returned a non-zero status: {proc.returncode}. " + "See log for more details" + ) + + try: + result = json.loads(proc.stdout) + + except json.decoder.JSONDecodeError: + cls._log_processes(proc) + raise UbuntuProApiError( + "Could not parse JSON response from Ubuntu Pro client. " + "See log for more details" + ) + + if result["result"] != "success": + cls._log_processes(proc) + raise UbuntuProApiError( + "Ubuntu Pro API returned an error response. See log for more details" + ) + + # Ignore typing for this private method. The returned object is variable in type, but types are declared in the API docs: + # https://canonical-ubuntu-pro-client.readthedocs-hosted.com/en/v32/references/api/ + return result # type: ignore [no-any-return] + + @classmethod + def is_pro_attached(cls) -> bool: + """Return True if environment is attached to Ubuntu Pro.""" + response = cls._pro_api_call("u.pro.status.is_attached.v1") + + # Ignore typing here. This field's type is static according to: + # https://canonical-ubuntu-pro-client.readthedocs-hosted.com/en/v32/references/api/#u-pro-status-is-attached-v1 + return response["data"]["attributes"]["is_attached"] # type: ignore [no-any-return] + + @classmethod + def get_pro_services(cls) -> ProServices: + """Return set of enabled Ubuntu Pro services in the environment. + + The returned set only includes services relevant to lifecycle commands. + """ + response = cls._pro_api_call("u.pro.status.enabled_services.v1") + enabled_services = response["data"]["attributes"]["enabled_services"] + + service_names = {service["name"] for service in enabled_services} + + # remove any services that aren't relevant to build services + service_names = service_names.intersection(cls.supported_services) + + return cls(service_names) + + def validate( + self, + options: ValidatorOptions = ValidatorOptions.DEFAULT, + ) -> None: + """Validate the environment against pro services specified in this ProServices instance.""" + # raise exception if any service was requested outside of build_service_scope + if ValidatorOptions.SUPPORT in options and ( + invalid_services := self - self.supported_services + ): + raise InvalidUbuntuProServiceError(invalid_services) + + try: + # first, check Ubuntu Pro status + # Since we extend the set class, cast ourselves to bool to check if we empty. if we are not + # empty, this implies we require pro services. + + if self.is_pro_attached() != bool(self): + if ValidatorOptions._ATTACHED in options and self: # type: ignore [reportPrivateUsage] + # Ubuntu Pro is requested but not attached + raise UbuntuProDetachedError + + if ValidatorOptions._DETACHED in options and not self: # type: ignore [reportPrivateUsage] + # Ubuntu Pro is not requested but attached + raise UbuntuProAttachedError + + # second, check that the set of enabled pro services in the environment matches + # the services specified in this set + if ValidatorOptions.ENABLEMENT in options and ( + (available_services := self.get_pro_services()) != self + ): + raise InvalidUbuntuProStatusError(self, available_services) + + except UbuntuProClientNotFoundError: + + # If The pro client was not found, we may be on a non Ubuntu + # system, but if Pro services were requested, re-raise error + if self and not self.pro_client_exists(): + raise diff --git a/tests/conftest.py b/tests/conftest.py index 12015110f..c8a2a702c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -590,3 +590,42 @@ def repository_with_unannotated_tag( subprocess.run(["git", "tag", test_tag], check=True) repository_with_commit.tag = test_tag return repository_with_commit + + +@pytest.fixture +def mock_pro_api_call(mocker): + mock_responses = { + "u.pro.status.is_attached.v1": { + "data": { + "attributes": {"is_attached": False}, + "result": "success", + } + }, + "u.pro.status.enabled_services.v1": { + "data": {"attributes": {"enabled_services": []}}, + "result": "success", + }, + } + + def set_is_attached(value: bool): # noqa: FBT001 + response = mock_responses["u.pro.status.is_attached.v1"] + response["data"]["attributes"]["is_attached"] = value + + def set_enabled_services(service_names: list[str]): + enabled_services = [ + {"name": name, "variant_enabled": False, "variant_name": None} + for name in service_names + ] + + response = mock_responses["u.pro.status.enabled_services.v1"] + response["data"]["attributes"]["enabled_services"] = enabled_services + + def mock_pro_api_call(endpoint: str): + return mock_responses[endpoint] + + mocker.patch( + "craft_application.util.ProServices._pro_api_call", + new=mock_pro_api_call, + ) + + return set_is_attached, set_enabled_services diff --git a/tests/unit/commands/test_lifecycle.py b/tests/unit/commands/test_lifecycle.py index 2a6fc5782..0a75cdb42 100644 --- a/tests/unit/commands/test_lifecycle.py +++ b/tests/unit/commands/test_lifecycle.py @@ -19,6 +19,7 @@ import pathlib import subprocess from unittest import mock +from contextlib import nullcontext import craft_parts import craft_platforms @@ -44,6 +45,18 @@ from craft_parts import Features pytestmark = [pytest.mark.usefixtures("fake_project_file")] +from craft_application.errors import ( + InvalidUbuntuProServiceError, + InvalidUbuntuProStatusError, + UbuntuProAttachedError, + UbuntuProDetachedError, +) +from craft_application.util import ProServices +from craft_cli import emit +from craft_parts import Features + +# disable black reformat for improve readability on long parameterisations +# fmt: off PARTS_LISTS = [[], ["my-part"], ["my-part", "your-part"]] SHELL_PARAMS = [ @@ -61,6 +74,28 @@ ({"destructive_mode": True, "use_lxd": False}, ["--destructive-mode"]), ({"destructive_mode": False, "use_lxd": True}, ["--use-lxd"]), ] + +# test paring --pro argument with various pro services, and whitespace +PRO_SERVICE_COMMANDS = [ + ({"pro": ProServices()}, []), + ({"pro": ProServices(["fips-updates"])}, ["--pro", "fips-updates"]), + ({"pro": ProServices(["fips-updates", "esm-infra"])}, ["--pro", "fips-updates,esm-infra"]), + ({"pro": ProServices(["fips-updates", "esm-infra"])}, ["--pro", "fips-updates , esm-infra"]), + ({"pro": ProServices(["fips-updates"])}, ["--pro", "fips-updates,fips-updates"]), +] + +PRO_SERVICE_CONFIGS = [ + # is_attached, enabled_services, pro_services_args, expected_exception + (False, [], [], None), + (True, ["esm-apps"], ["esm-apps"], None), + (True, ["esm-apps", "fips-updates"],["esm-apps", "fips-updates"], None), + (True, ["esm-apps"], [], UbuntuProAttachedError), + (False, [], ["esm-apps"], UbuntuProDetachedError), + (True, ["esm-apps", "fips-updates"],["fips-updates"], InvalidUbuntuProStatusError), + (True, ["esm-apps",], ["fips-updates", "fips-updates"], InvalidUbuntuProStatusError), + (True, ["esm-apps"], ["esm-apps", "invalid-service"], InvalidUbuntuProServiceError), +] + STEP_NAMES = [step.name.lower() for step in craft_parts.Step] MANAGED_LIFECYCLE_COMMANDS = ( PullCommand, @@ -73,6 +108,8 @@ ALL_LIFECYCLE_COMMANDS = MANAGED_LIFECYCLE_COMMANDS + UNMANAGED_LIFECYCLE_COMMANDS NON_CLEAN_COMMANDS = (*MANAGED_LIFECYCLE_COMMANDS, PackCommand) +# fmt: on + def get_fake_command_class(parent_cls, managed): """Create a fully described fake command based on a partial class.""" @@ -89,6 +126,32 @@ def run_managed(self, parsed_args: argparse.Namespace) -> bool: return FakeCommand +@pytest.mark.parametrize( + ("is_attached", "enabled_services", "pro_services_args", "expected_exception"), + PRO_SERVICE_CONFIGS, +) +def test_validate_pro_services( + mock_pro_api_call, + is_attached, + enabled_services, + pro_services_args, + expected_exception, +): + # configure api state + set_is_attached, set_enabled_service = mock_pro_api_call + set_is_attached(is_attached) + set_enabled_service(enabled_services) + + exception_context = ( + pytest.raises(expected_exception) if expected_exception else nullcontext() + ) + + with exception_context: + # create and validate pro services + pro_services = ProServices(pro_services_args) + pro_services.validate() + + @pytest.mark.parametrize( ("enable_overlay", "commands"), [ @@ -117,12 +180,15 @@ def test_get_lifecycle_command_group(enable_overlay, commands): Features.reset() +@pytest.mark.parametrize(("pro_service_dict", "pro_service_args"), PRO_SERVICE_COMMANDS) @pytest.mark.parametrize(("build_env_dict", "build_env_args"), BUILD_ENV_COMMANDS) @pytest.mark.parametrize(("debug_dict", "debug_args"), DEBUG_PARAMS) @pytest.mark.parametrize(("shell_dict", "shell_args"), SHELL_PARAMS) def test_lifecycle_command_fill_parser( default_app_metadata, fake_services, + pro_service_dict, + pro_service_args, build_env_dict, build_env_args, debug_dict, @@ -139,11 +205,16 @@ def test_lifecycle_command_fill_parser( **shell_dict, **debug_dict, **build_env_dict, + **pro_service_dict, } command.fill_parser(parser) - args_dict = vars(parser.parse_args([*build_env_args, *debug_args, *shell_args])) + args_dict = vars( + parser.parse_args( + [*pro_service_args, *build_env_args, *debug_args, *shell_args] + ) + ) assert args_dict == expected @@ -291,6 +362,7 @@ def test_run_manager_for_build_plan( mock_run_managed.assert_called_once_with(build, fetch) +@pytest.mark.parametrize(("pro_service_dict", "pro_service_args"), PRO_SERVICE_COMMANDS) @pytest.mark.parametrize(("build_env_dict", "build_env_args"), BUILD_ENV_COMMANDS) @pytest.mark.parametrize(("debug_dict", "debug_args"), DEBUG_PARAMS) @pytest.mark.parametrize(("shell_dict", "shell_args"), SHELL_PARAMS) @@ -298,6 +370,8 @@ def test_run_manager_for_build_plan( def test_step_command_fill_parser( default_app_metadata, fake_services, + pro_service_dict, + pro_service_args, parts_args, build_env_dict, build_env_args, @@ -315,13 +389,16 @@ def test_step_command_fill_parser( **shell_dict, **debug_dict, **build_env_dict, + **pro_service_dict, } command = cls({"app": default_app_metadata, "services": fake_services}) command.fill_parser(parser) args_dict = vars( - parser.parse_args([*build_env_args, *shell_args, *debug_args, *parts_args]) + parser.parse_args( + [*pro_service_args, *build_env_args, *shell_args, *debug_args, *parts_args] + ) ) assert args_dict == expected @@ -460,6 +537,44 @@ def test_clean_run_without_parts( assert mock_services.provider.clean_instances.called == expected_provider +@pytest.mark.parametrize( + ("destructive", "build_env", "parts", "expected_run_managed"), + [ + # destructive mode or CRAFT_BUILD_ENV==host should not run managed + (True, "lxd", [], False), + (True, "host", [], False), + (False, "host", [], False), + (True, "lxd", ["part1"], False), + (True, "host", ["part1"], False), + (False, "host", ["part1"], False), + (True, "lxd", ["part1", "part2"], False), + (True, "host", ["part1", "part2"], False), + (False, "host", ["part1", "part2"], False), + # destructive mode==False and CRAFT_BUILD_ENV!=host: depends on "parts" + # clean specific parts: should run managed + (False, "lxd", ["part1"], True), + (False, "lxd", ["part1", "part2"], True), + # "part-less" clean: shouldn't run managed + (False, "lxd", [], False), + ], +) +def test_clean_run_managed( + app_metadata, + mock_services, + destructive, + build_env, + parts, + expected_run_managed, + monkeypatch, +): + monkeypatch.setenv("CRAFT_BUILD_ENVIRONMENT", build_env) + parsed_args = argparse.Namespace(parts=parts, destructive_mode=destructive) + command = CleanCommand({"app": app_metadata, "services": mock_services}) + + assert command.run_managed(parsed_args) == expected_run_managed + + +@pytest.mark.parametrize(("pro_service_dict", "pro_service_args"), PRO_SERVICE_COMMANDS) @pytest.mark.parametrize(("build_env_dict", "build_env_args"), BUILD_ENV_COMMANDS) @pytest.mark.parametrize(("shell_dict", "shell_args"), SHELL_PARAMS) @pytest.mark.parametrize(("debug_dict", "debug_args"), DEBUG_PARAMS) @@ -467,6 +582,8 @@ def test_clean_run_without_parts( def test_pack_fill_parser( app_metadata, mock_services, + pro_service_dict, + pro_service_args, build_env_dict, build_env_args, shell_dict, @@ -486,6 +603,7 @@ def test_pack_fill_parser( **shell_dict, **debug_dict, **build_env_dict, + **pro_service_dict, } command = PackCommand({"app": app_metadata, "services": mock_services}) @@ -493,7 +611,7 @@ def test_pack_fill_parser( args_dict = vars( parser.parse_args( - [*build_env_args, *shell_args, *debug_args, f"--output={output_arg}"] + [*pro_service_args, *build_env_args, *debug_args, f"--output={output_arg}"] ) ) assert args_dict == expected diff --git a/tests/unit/test_application.py b/tests/unit/test_application.py index e8db292ab..81a269514 100644 --- a/tests/unit/test_application.py +++ b/tests/unit/test_application.py @@ -462,7 +462,15 @@ def test_craft_lib_log_level(app_metadata, fake_services): assert logger.level == logging.DEBUG -def test_gets_project(monkeypatch, fake_project_file, app_metadata, fake_services): +def test_gets_project( + monkeypatch, + tmp_path, + app_metadata, + fake_services, + mock_pro_api_call, # noqa: ARG001 +): + project_file = tmp_path / "testcraft.yaml" + project_file.write_text(BASIC_PROJECT_YAML) monkeypatch.setattr(sys, "argv", ["testcraft", "pull", "--destructive-mode"]) app = FakeApplication(app_metadata, fake_services) @@ -474,7 +482,13 @@ def test_gets_project(monkeypatch, fake_project_file, app_metadata, fake_service def test_fails_without_project( - monkeypatch, capsys, tmp_path, app_metadata, fake_services, app, debug_mode + monkeypatch, + capsys, + tmp_path, + app_metadata, + fake_services, + app, + mock_pro_api_call, # noqa: ARG001 ): # Set up a real project service - the fake one for testing gets a fake project! del app.services._services["project"] @@ -562,7 +576,11 @@ def test_pre_run_project_dir_success_unmanaged(app, fs, project_dir): @pytest.mark.parametrize("project_dir", ["relative/file", "/absolute/file"]) -def test_pre_run_project_dir_not_a_directory(app, fs, project_dir): +def test_pre_run_project_dir_not_a_directory( + app, + fs, + project_dir, +): fs.create_file(project_dir) dispatcher = mock.Mock(spec_set=craft_cli.Dispatcher) dispatcher.parsed_args.return_value.project_dir = project_dir @@ -574,7 +592,14 @@ def test_pre_run_project_dir_not_a_directory(app, fs, project_dir): @pytest.mark.parametrize("load_project", [True, False]) @pytest.mark.parametrize("return_code", [None, 0, 1]) def test_run_success_unmanaged( - monkeypatch, emitter, check, app, fake_project, return_code, load_project + monkeypatch, + emitter, + check, + app, + fake_project, + return_code, + load_project, + mock_pro_api_call, # noqa: ARG001 ): class UnmanagedCommand(commands.AppCommand): name = "pass" @@ -597,9 +622,101 @@ def run(self, parsed_args: argparse.Namespace): emitter.assert_debug("Running testcraft pass on host") +def test_run_success_managed( + monkeypatch, + app, + fake_project, + mocker, + mock_pro_api_call, # noqa: ARG001 +): + mocker.patch.object(app, "get_project", return_value=fake_project) + app.run_managed = mock.Mock() + monkeypatch.setattr(sys, "argv", ["testcraft", "pull"]) + + pytest_check.equal(app.run(), 0) + + app.run_managed.assert_called_once_with(None, None) # --build-for not used + + +def test_run_success_managed_with_arch( + monkeypatch, + app, + fake_project, + mocker, + mock_pro_api_call, # noqa: ARG001 +): + mocker.patch.object(app, "get_project", return_value=fake_project) + app.run_managed = mock.Mock() + arch = get_host_architecture() + monkeypatch.setattr(sys, "argv", ["testcraft", "pull", f"--build-for={arch}"]) + + pytest_check.equal(app.run(), 0) + + app.run_managed.assert_called_once() + + +def test_run_success_managed_with_platform( + monkeypatch, + app, + fake_project, + mocker, + mock_pro_api_call, # noqa: ARG001 +): + mocker.patch.object(app, "get_project", return_value=fake_project) + app.run_managed = mock.Mock() + monkeypatch.setattr(sys, "argv", ["testcraft", "pull", "--platform=foo"]) + + pytest_check.equal(app.run(), 0) + + app.run_managed.assert_called_once_with("foo", None) + + +@pytest.mark.parametrize( + ("params", "expected_call"), + [ + ([], mock.call(None, None)), + (["--platform=s390x"], mock.call("s390x", None)), + ( + ["--platform", get_host_architecture()], + mock.call(get_host_architecture(), None), + ), + ( + ["--build-for", get_host_architecture()], + mock.call(None, get_host_architecture()), + ), + (["--build-for", "s390x"], mock.call(None, "s390x")), + (["--platform", "s390x,riscv64"], mock.call("s390x", None)), + (["--build-for", "s390x,riscv64"], mock.call(None, "s390x")), + ], +) +def test_run_passes_platforms( + monkeypatch, + app, + fake_project, + mocker, + params, + expected_call, + mock_pro_api_call, # noqa: ARG001 +): + mocker.patch.object(app, "get_project", return_value=fake_project) + app.run_managed = mock.Mock(return_value=False) + monkeypatch.setattr(sys, "argv", ["testcraft", "pull", *params]) + + pytest_check.equal(app.run(), 0) + + assert app.run_managed.mock_calls == [expected_call] + + @pytest.mark.parametrize("return_code", [None, 0, 1]) def test_run_success_managed_inside_managed( - monkeypatch, check, app, fake_project, mock_dispatcher, return_code, mocker + monkeypatch, + check, + app, + fake_project, + mock_dispatcher, + return_code, + mocker, + mock_pro_api_call, # noqa: ARG001 ): mocker.patch.object(app, "get_project", return_value=fake_project) mocker.patch.object( From d276de53d618515d0ae050d53fd0d89184749207 Mon Sep 17 00:00:00 2001 From: Adrian Clay Lake Date: Tue, 22 Oct 2024 14:20:09 +0200 Subject: [PATCH 02/14] feat: enable requested ubuntu pro services (#504) * feat: remove version pin from craft-providers for dev * feat: add setup pro services * feat: remove exess logging in paused block * fix: pass pro services instance in run_managed * test: force pro client install * test: remove pro validation * test: disable auto-enable services * test: attach pro subscription * feat: craft_application enables pro services * fix: ignore temporary lint warning * fix: temporarily supressing ruff warnings until pro features are merged into main * feat: pass use_host_sources to lifecycle service * fix(craft_application/application.py): incorrectly checking absence of pro argument --- craft_application/application.py | 88 ++++++++++++++------ craft_application/commands/lifecycle.py | 45 +++++----- craft_application/errors.py | 4 + tests/unit/test_application.py | 104 +++++++++++++++++++++++- 4 files changed, 190 insertions(+), 51 deletions(-) diff --git a/craft_application/application.py b/craft_application/application.py index 286bc2896..863003f02 100644 --- a/craft_application/application.py +++ b/craft_application/application.py @@ -33,12 +33,13 @@ import craft_cli import craft_platforms import craft_providers +import craft_providers.lxd from craft_parts.errors import PartsError from platformdirs import user_cache_path from craft_application import _config, commands, errors, models, util from craft_application.errors import PathInvalidError -from craft_application.util import ValidatorOptions +from craft_application.util import ProServices, ValidatorOptions if TYPE_CHECKING: import argparse @@ -163,6 +164,10 @@ def __init__( # Set a globally usable project directory for the application. # This may be overridden by specific application implementations. self.project_dir = pathlib.Path.cwd() + # Ubuntu ProServices instance containing relevant pro services specified by the user. + # Storage of this instance may change in the future as we migrate Pro operations towards + # an application service. + self._pro_services: ProServices | None = None if self.is_managed(): self._work_dir = pathlib.Path("/root") @@ -345,6 +350,9 @@ def _configure_services(self, provider_name: str | None) -> None: "lifecycle", cache_dir=self.cache_dir, work_dir=self._work_dir, + use_host_sources=bool( + self._pro_services + ), # TODO: should this be passed as a arg instead? ) self.services.update_kwargs( "provider", @@ -426,6 +434,25 @@ def run_managed(self, platform: str | None, build_for: str | None) -> None: clean_existing=self._enable_fetch_service, use_base_instance=not active_fetch_service, ) as instance: + # if pro services are required, ensure the pro client is + # installed, attached and the correct services are enabled + if self._pro_services: + # Suggestion: create a Pro abstract class used to ensure minimum support by instances. + # we can then check for pro support by inheritance. + if not isinstance(instance, craft_providers.lxd.LXDInstance): + raise errors.UbuntuProNotSupportedError( + "Ubuntu Pro builds are only supported on LXC." + ) + + craft_cli.emit.debug( + f"Enabling Ubuntu Pro Services {self._pro_services} on" + ) + # TODO: remove ignores after these methods are merged into main in craft-providers. + # see https://github.com/canonical/craft-providers/pull/664/files + instance.install_pro_client() # type: ignore # noqa: PGH003 + instance.attach_pro_subscription() # type: ignore # noqa: PGH003 + instance.enable_pro_service(self._pro_services) # type: ignore # noqa: PGH003 + if self._enable_fetch_service: fetch_env = self.services.fetch.create_session(instance) env.update(fetch_env) @@ -439,7 +466,6 @@ def run_managed(self, platform: str | None, build_for: str | None) -> None: ) try: with craft_cli.emit.pause(): - # Pyright doesn't fully understand craft_providers's CompletedProcess. instance.execute_run( # pyright: ignore[reportUnknownMemberType,reportUnknownVariableType] cmd, cwd=instance_path, @@ -573,6 +599,34 @@ def _pre_run(self, dispatcher: craft_cli.Dispatcher) -> None: resolution="Ensure the path entered is correct.", ) + @staticmethod + def _check_pro_requirement( + pro_services: ProServices | None, + run_managed: bool, # noqa: FBT001 + is_managed: bool, # noqa: FBT001 + ) -> None: + if pro_services: + # Validate requested pro services on the host if we are running in destructive mode. + if not run_managed and not is_managed: + craft_cli.emit.debug( + f"Validating requested Ubuntu Pro status on host: {pro_services}" + ) + pro_services.validate() + # Validate requested pro services running in managed mode inside a managed instance. + elif run_managed and is_managed: + craft_cli.emit.debug( + f"Validating requested Ubuntu Pro status in managed instance: {pro_services}" + ) + pro_services.validate() + # Validate pro attachment and service names on the host before starting a managed instance. + elif run_managed and not is_managed: + craft_cli.emit.debug( + f"Validating requested Ubuntu Pro attachment on host: {pro_services}" + ) + pro_services.validate( + options=ValidatorOptions.ATTACHMENT | ValidatorOptions.SUPPORT + ) + fetch_service_policy: str | None = getattr(args, "fetch_service_policy", None) if fetch_service_policy: self._enable_fetch_service = True @@ -604,7 +658,6 @@ def _run_inner(self) -> int: platform = getattr(dispatcher.parsed_args(), "platform", None) build_for = getattr(dispatcher.parsed_args(), "build_for", None) - pro_services = getattr(dispatcher.parsed_args(), "pro", None) run_managed = command.run_managed(dispatcher.parsed_args()) is_managed = self.is_managed() @@ -618,29 +671,12 @@ def _run_inner(self) -> int: build_for = build_for.split(",", maxsplit=1)[0] craft_cli.emit.debug(f"Build plan: platform={platform}, build_for={build_for}") - # Check that pro services are correctly configured. A ProServices instance will - # only be available for lifecycle commands, otherwise we default to None - if pro_services is not None: - # Validate requested pro services on the host if we are running in destructive mode... - if not run_managed and not is_managed: - craft_cli.emit.debug( - f"Validating requested Ubuntu Pro status on host: {pro_services}" - ) - pro_services.validate() - # .. or running in managed mode inside a managed instance - elif run_managed and is_managed: - craft_cli.emit.debug( - f"Validating requested Ubuntu Pro status in managed instance: {pro_services}" - ) - pro_services.validate() - # .. or validate pro attachment and service names on the host before starting a managed instance. - elif run_managed and not is_managed: - craft_cli.emit.debug( - f"Validating requested Ubuntu Pro attachment on host: {pro_services}" - ) - pro_services.validate( - options=ValidatorOptions.ATTACHMENT | ValidatorOptions.SUPPORT - ) + # TODO: Move pro operations out to new service for managing Ubuntu Pro + # A ProServices instance will only be available for lifecycle commands, + # which may consume pro packages, + self._pro_services = getattr(dispatcher.parsed_args(), "pro", None) + # Check that pro services are correctly configured if available + self._check_pro_requirement(self._pro_services, run_managed, is_managed) if run_managed or command.needs_project(dispatcher.parsed_args()): self.services.project = self.get_project( diff --git a/craft_application/commands/lifecycle.py b/craft_application/commands/lifecycle.py index 2b406151a..3980c4410 100644 --- a/craft_application/commands/lifecycle.py +++ b/craft_application/commands/lifecycle.py @@ -84,31 +84,13 @@ def _fill_parser(self, parser: argparse.ArgumentParser) -> None: help="Build in a LXD container.", ) - supported_pro_services = ", ".join( - [f"'{name}'" for name in ProServices.supported_services] - ) - - parser.add_argument( - "--pro", - type=ProServices.from_csv, - metavar="", - help=( - "Enable Ubuntu Pro services for this command. " - f"Supported values include: {supported_pro_services}. " - "Multiple values can be passed separated by commas. " - "Note: This feature requires an Ubuntu Pro compatible host and build base." - ), - default=ProServices(), - ) - - # TODO: do we need this? - # @override - # def get_managed_cmd(self, parsed_args: argparse.Namespace) -> list[str]: - # cmd = super().get_managed_cmd(parsed_args) + @override + def get_managed_cmd(self, parsed_args: argparse.Namespace) -> list[str]: + cmd = super().get_managed_cmd(parsed_args) - # cmd.extend(parsed_args.parts) + cmd.extend(parsed_args.parts) - # return cmd + return cmd @override def provider_name(self, parsed_args: argparse.Namespace) -> str | None: @@ -166,6 +148,23 @@ def _fill_parser(self, parser: argparse.ArgumentParser) -> None: help="Shell into the environment after the step has run.", ) + supported_pro_services = ", ".join( + [f"'{name}'" for name in ProServices.supported_services] + ) + + parser.add_argument( + "--pro", + type=ProServices.from_csv, + metavar="", + help=( + "Enable Ubuntu Pro services for this command. " + f"Supported values include: {supported_pro_services}. " + "Multiple values can be passed separated by commas. " + "Note: This feature requires an Ubuntu Pro compatible host and build base." + ), + default=ProServices(), + ) + parser.add_argument( "--debug", action="store_true", diff --git a/craft_application/errors.py b/craft_application/errors.py index e2a8884eb..6f5041671 100644 --- a/craft_application/errors.py +++ b/craft_application/errors.py @@ -401,6 +401,10 @@ class InvalidUbuntuProStateError(UbuntuProError): # environment. What is the best way to get the is_managed method here? +class UbuntuProNotSupportedError(UbuntuProError): + """Raised when Ubuntu Pro client is not supported on the base or build base.""" + + class UbuntuProClientNotFoundError(UbuntuProApiError): """Raised when Ubuntu Pro client was not found on the system.""" diff --git a/tests/unit/test_application.py b/tests/unit/test_application.py index 81a269514..10e4f1446 100644 --- a/tests/unit/test_application.py +++ b/tests/unit/test_application.py @@ -51,8 +51,8 @@ ) from craft_cli import emit from craft_parts.plugins.plugins import PluginType - -from tests.conftest import FakeApplication +from craft_providers import bases, lxd +from overrides import override EMPTY_COMMAND_GROUP = craft_cli.CommandGroup("FakeCommands", []) @@ -297,6 +297,76 @@ def test_run_managed_failure(app, fake_project): assert exc_info.value.brief == "Failed to execute testcraft in instance." +def test_run_managed_configure_pro(mocker, app, fake_project, fake_build_plan): + """Ensure that Pro is installed and configured in a managed instance.""" + mock_provider = mocker.MagicMock(spec_set=services.ProviderService) + app.services.provider = mock_provider + # provide spec to pass type check for pro support + mock_instance = mocker.MagicMock(spec=lxd.LXDInstance) + + # TODO: these methods are currently in review, https://github.com/canonical/craft-providers/pull/664/files + # Remove when craft-providers with pro support in lxd is merged to main. + mock_instance.install_pro_client = mocker.Mock() + mock_instance.attach_pro_subscription = mocker.Mock() + mock_instance.enable_pro_service = mocker.Mock() + + mock_provider.instance.return_value.__enter__.return_value = mock_instance + app.project = fake_project + app._build_plan = fake_build_plan + app._pro_services = util.ProServices(["esm-apps"]) + arch = get_host_architecture() + + app.run_managed(None, arch) + + assert mock_instance.install_pro_client.call_count == 1 + assert mock_instance.attach_pro_subscription.call_count == 1 + assert mock_instance.enable_pro_service.call_count == 1 + + +def test_run_managed_pro_not_supported(mocker, app, fake_project, fake_build_plan): + """Ensure that providers that do not support Ubuntu Pro cause an exception + when pro services are requested.""" + mock_provider = mocker.MagicMock(spec_set=services.ProviderService) + app.services.provider = mock_provider + # Provide an unsupported instance type to raise exception + mock_instance = mocker.MagicMock() + + mock_provider.instance.return_value.__enter__.return_value = mock_instance + app.project = fake_project + app._build_plan = fake_build_plan + app._pro_services = util.ProServices(["esm-apps"]) + arch = get_host_architecture() + + with pytest.raises(errors.UbuntuProNotSupportedError): + app.run_managed(None, arch) + + +def test_run_managed_skip_configure_pro(mocker, app, fake_project, fake_build_plan): + """Ensure that Pro is not installed and configured when it is not required.""" + mock_provider = mocker.MagicMock(spec_set=services.ProviderService) + app.services.provider = mock_provider + # provide spec to pass type check for pro support + mock_instance = mocker.MagicMock(spec=lxd.LXDInstance) + + # TODO: Remove when these mocks when methods are present in main. + # see TODO in test_run_managed_configure_pro for details. + mock_instance.install_pro_client = mocker.Mock() + mock_instance.attach_pro_subscription = mocker.Mock() + mock_instance.enable_pro_service = mocker.Mock() + + mock_provider.instance.return_value.__enter__.return_value = mock_instance + app.project = fake_project + app._build_plan = fake_build_plan + app._pro_services = util.ProServices([]) + arch = get_host_architecture() + + app.run_managed(None, arch) + + assert mock_instance.install_pro_client.call_count == 0 + assert mock_instance.attach_pro_subscription.call_count == 0 + assert mock_instance.enable_pro_service.call_count == 0 + + def test_run_managed_multiple(app, fake_host_architecture): mock_provider = mock.MagicMock(spec_set=services.ProviderService) app.services._services["provider"] = mock_provider @@ -628,6 +698,11 @@ def test_run_success_managed( fake_project, mocker, mock_pro_api_call, # noqa: ARG001 + monkeypatch, + app, + fake_project, + mocker, + mock_pro_api_call, # noqa: ARG001 ): mocker.patch.object(app, "get_project", return_value=fake_project) app.run_managed = mock.Mock() @@ -1243,3 +1318,28 @@ def test_doc_url_in_command_help(monkeypatch, capsys, app): expected = "For more information, check out: www.testcraft.example/docs/3.14159/reference/commands/app-config\n\n" _, err = capsys.readouterr() assert err.endswith(expected) + + +# fmt: off +@pytest.mark.parametrize( + ( "run_managed", "is_managed", "call_count", "validator_options"), + [ + (False, False, 1, None), + (True, True, 1, None), + (True, False, 1, util.ValidatorOptions.ATTACHMENT | util.ValidatorOptions.SUPPORT,), + (False, True, 0, None), + ], +) +# fmt: on +def test_check_pro_requirement( + mocker, app, run_managed, is_managed, call_count, validator_options +): + """Test that _check_pro_requirement validates Pro Services in the correct situations""" + pro_services = mocker.Mock() + app._check_pro_requirement(pro_services, run_managed, is_managed) + + assert pro_services.validate.call_count == call_count + + for call in pro_services.validate.call_args_list: + if validator_options is not None: # skip assert if default value is passed + assert call.kwargs["options"] == validator_options From 0cf818cc019be6441901c06add105d418f5c78ce Mon Sep 17 00:00:00 2001 From: Adrian Clay Lake Date: Thu, 21 Nov 2024 14:15:56 +0100 Subject: [PATCH 03/14] fix(pro): fix _check_pro_requirement function to not ignore empty ProServices set (#563) --- craft_application/application.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/craft_application/application.py b/craft_application/application.py index 863003f02..061344541 100644 --- a/craft_application/application.py +++ b/craft_application/application.py @@ -605,7 +605,7 @@ def _check_pro_requirement( run_managed: bool, # noqa: FBT001 is_managed: bool, # noqa: FBT001 ) -> None: - if pro_services: + if pro_services is not None: # should not be None for all lifecycle commands. # Validate requested pro services on the host if we are running in destructive mode. if not run_managed and not is_managed: craft_cli.emit.debug( From 835869efad007c971629928a09704ee6bbf754cc Mon Sep 17 00:00:00 2001 From: Anas Husseini Date: Thu, 5 Dec 2024 08:59:36 +0200 Subject: [PATCH 04/14] fix lint errors --- tests/unit/commands/test_lifecycle.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/unit/commands/test_lifecycle.py b/tests/unit/commands/test_lifecycle.py index 0a75cdb42..6d113be36 100644 --- a/tests/unit/commands/test_lifecycle.py +++ b/tests/unit/commands/test_lifecycle.py @@ -611,7 +611,13 @@ def test_pack_fill_parser( args_dict = vars( parser.parse_args( - [*pro_service_args, *build_env_args, *debug_args, f"--output={output_arg}"] + [ + *pro_service_args, + *build_env_args, + *shell_args, + *debug_args, + f"--output={output_arg}", + ] ) ) assert args_dict == expected From 5c67d746ddf18c7badfdef48f9be8383775bce8c Mon Sep 17 00:00:00 2001 From: Adrian Clay Lake Date: Wed, 5 Feb 2025 21:16:12 +0100 Subject: [PATCH 05/14] feat: more updates to support Pro Rocks in Rockcraft (#616) - added caching for --pro argument value on lifecycle command - added early abort for commands reentering instances with varying --pro argument - refactor Ubuntu Pro configuration for LXD instance - fixed Ubuntu Pro services validation for standard builds on pro hosts - added Exception resolutions aware of managed state --- .github/workflows/tests.yaml | 143 +++++++++++++ craft_application/application.py | 89 +++++--- craft_application/commands/remote.py | 1 + craft_application/errors.py | 39 ++-- craft_application/secrets.py | 192 ++++++++++++++++++ craft_application/services/service_factory.py | 14 ++ craft_application/util/__init__.py | 4 + craft_application/util/pro_services.py | 60 ++++-- docs/conf.py | 2 +- docs/reference/changelog.rst | 101 --------- pyproject.toml | 5 + tests/conftest.py | 11 +- tests/integration/commands/test_init.py | 1 + tests/integration/conftest.py | 2 + tests/integration/launchpad/conftest.py | 1 + .../launchpad/test_anonymous_access.py | 2 + tests/integration/services/test_fetch.py | 3 + tests/integration/services/test_init.py | 2 +- tests/integration/services/test_lifecycle.py | 1 + .../integration/services/test_remotebuild.py | 1 + .../services/test_service_factory.py | 1 + tests/integration/test_version.py | 3 +- tests/unit/commands/test_base.py | 3 +- tests/unit/commands/test_init.py | 1 + tests/unit/commands/test_lifecycle.py | 7 +- tests/unit/commands/test_other.py | 1 + tests/unit/git/test_git.py | 1 - tests/unit/launchpad/conftest.py | 1 + tests/unit/launchpad/models/test_base.py | 3 +- tests/unit/launchpad/models/test_code.py | 1 + tests/unit/launchpad/test_launchpad.py | 3 +- tests/unit/launchpad/test_util.py | 3 +- tests/unit/models/test_base.py | 3 +- tests/unit/models/test_constraints.py | 3 +- tests/unit/models/test_manifest.py | 3 + tests/unit/models/test_project.py | 1 + tests/unit/remote/test_git.py | 1 + tests/unit/remote/test_utils.py | 1 + tests/unit/remote/test_worktree.py | 1 + tests/unit/services/conftest.py | 1 + tests/unit/services/test_config.py | 5 +- tests/unit/services/test_init.py | 3 +- tests/unit/services/test_lifecycle.py | 7 + tests/unit/services/test_package.py | 1 + tests/unit/services/test_remotebuild.py | 2 +- tests/unit/services/test_repositories.py | 1 + tests/unit/services/test_service_factory.py | 1 - tests/unit/test_application.py | 31 ++- tests/unit/test_application_fetch.py | 124 +++++++++++ tests/unit/test_errors.py | 5 +- tests/unit/test_fetch.py | 3 +- tests/unit/test_grammar.py | 1 + tests/unit/test_secrets.py | 155 ++++++++++++++ tests/unit/util/test_docs.py | 1 + tests/unit/util/test_error_formatting.py | 1 + tests/unit/util/test_logging.py | 3 +- tests/unit/util/test_paths.py | 3 +- tests/unit/util/test_retry.py | 1 + tests/unit/util/test_snap_config.py | 3 +- tests/unit/util/test_string.py | 1 + tests/unit/util/test_system.py | 1 + 61 files changed, 869 insertions(+), 200 deletions(-) create mode 100644 .github/workflows/tests.yaml create mode 100644 craft_application/secrets.py create mode 100644 tests/unit/test_application_fetch.py create mode 100644 tests/unit/test_secrets.py diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml new file mode 100644 index 000000000..0c6ef4e9e --- /dev/null +++ b/.github/workflows/tests.yaml @@ -0,0 +1,143 @@ +name: Tests, linting, etc. +on: + push: + branches: + - "main" + - "feature/*" + - "hotfix/*" + - "release/*" + pull_request: + +jobs: + linters: + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v4 + with: + fetch-depth: 0 + - name: Setup Python + uses: actions/setup-python@v5 + with: + python-version: '3.12' + cache: 'pip' + - name: Configure environment + run: | + echo "::group::Begin snap install" + echo "Installing snaps in the background while running apt and pip..." + sudo snap install --no-wait --classic pyright + sudo snap install --no-wait ruff shellcheck + echo "::endgroup::" + echo "::group::apt-get" + sudo apt update + sudo apt-get install -y libapt-pkg-dev + echo "::endgroup::" + echo "::group::pip install" + python -m pip install tox + echo "::endgroup::" + echo "::group::Create virtual environments for linting processes." + tox run -m lint --notest + echo "::endgroup::" + echo "::group::Wait for snap to complete" + snap watch --last=install + echo "::endgroup::" + - name: Run Linters + run: .tox/.tox/bin/tox run --skip-pkg-install --no-list-dependencies -m lint + unit-tests: + strategy: + fail-fast: false + matrix: + platform: [ubuntu-22.04] + runs-on: ${{ matrix.platform }} + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + - name: Set up Python versions on ${{ matrix.platform }} + uses: actions/setup-python@v5 + with: + python-version: | + 3.10 + 3.12 + cache: 'pip' + - name: Configure environment + run: | + echo "::group::apt-get" + sudo apt update + sudo apt-get install -y libapt-pkg-dev + echo "::endgroup::" + echo "::group::pip install" + python -m pip install tox + echo "::endgroup::" + mkdir -p results + - name: Setup Tox environments + run: tox run -m unit-tests --notest + - name: Unit tests + run: .tox/.tox/bin/tox run --skip-pkg-install --no-list-dependencies --result-json results/tox-${{ matrix.platform }}.json -m unit-tests + env: + PYTEST_ADDOPTS: "--no-header -vv -rN" + - name: Upload code coverage + uses: codecov/codecov-action@v4 + with: + directory: ./results/ + files: coverage*.xml + - name: Upload test results + if: success() || failure() + uses: actions/upload-artifact@v4 + with: + name: unit-test-results-${{ matrix.platform }} + path: results/ + integration-tests: + strategy: + fail-fast: false + matrix: + platform: [ubuntu-22.04] + python: [py310, py312] + runs-on: ${{ matrix.platform }} + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + - name: Set up Python versions on ${{ matrix.platform }} + uses: actions/setup-python@v5 + with: + python-version: | + 3.10 + 3.12 + cache: 'pip' + - name: Setup LXD + uses: canonical/setup-lxd@v0.1.2 + - name: Configure environment + run: | + echo "::group::Begin snap install" + echo "Installing snaps in the background while running apt and pip..." + sudo snap install --no-wait --channel=candidate fetch-service + echo "::endgroup::" + echo "::group::apt-get" + sudo apt update + sudo apt-get install -y libapt-pkg-dev + echo "::endgroup::" + echo "::group::pip install" + python -m pip install tox + echo "::endgroup::" + mkdir -p results + echo "::group::Wait for snap to complete" + snap watch --last=install + echo "::endgroup::" + - name: Setup Tox environments + run: tox run -e integration-${{ matrix.python }} --notest + - name: Integration tests + run: .tox/.tox/bin/tox run --skip-pkg-install --no-list-dependencies --result-json results/tox-${{ matrix.platform }}-${{ matrix.python }}.json -e integration-${{ matrix.python }} + env: + PYTEST_ADDOPTS: "--no-header -vv -rN" + - name: Upload code coverage + uses: codecov/codecov-action@v4 + with: + directory: ./results/ + files: coverage*.xml + - name: Upload test results + if: success() || failure() + uses: actions/upload-artifact@v4 + with: + name: integration-test-results-${{ matrix.platform }}-${{ matrix.python }} + path: results/ diff --git a/craft_application/application.py b/craft_application/application.py index 061344541..7b08fabfd 100644 --- a/craft_application/application.py +++ b/craft_application/application.py @@ -38,7 +38,7 @@ from platformdirs import user_cache_path from craft_application import _config, commands, errors, models, util -from craft_application.errors import PathInvalidError +from craft_application.errors import InvalidUbuntuProStatusError, PathInvalidError from craft_application.util import ProServices, ValidatorOptions if TYPE_CHECKING: @@ -350,9 +350,9 @@ def _configure_services(self, provider_name: str | None) -> None: "lifecycle", cache_dir=self.cache_dir, work_dir=self._work_dir, - use_host_sources=bool( - self._pro_services - ), # TODO: should this be passed as a arg instead? + build_plan=self._build_plan, + partitions=self._partitions, + use_host_sources=bool(self._pro_services), ) self.services.update_kwargs( "provider", @@ -396,6 +396,41 @@ def is_managed(self) -> bool: """Shortcut to tell whether we're running in managed mode.""" return self.services.get_class("provider").is_managed() + def _configure_instance_with_pro(self, instance: craft_providers.Executor) -> None: + """Configure an instance with Ubuntu Pro. Currently we only support LXD instances.""" + # TODO: Remove craft_provider typing ignores after feature/pro-sources # noqa: FIX002 + # has been merged into main. + + # Check if the instance has pro services enabled and if they match the requested services. + # If not, raise an Exception and bail out. + if ( + isinstance(instance, craft_providers.lxd.LXDInstance) + and instance.pro_services is not None # type: ignore # noqa: PGH003 + and instance.pro_services != self._pro_services # type: ignore # noqa: PGH003 + ): + raise InvalidUbuntuProStatusError(self._pro_services, instance.pro_services) # type: ignore # noqa: PGH003 + + # if pro services are required, ensure the pro client is + # installed, attached and the correct services are enabled + if self._pro_services: + # Suggestion: create a Pro abstract class used to ensure minimum support by instances. + # we can then check for pro support by inheritance. + if not isinstance(instance, craft_providers.lxd.LXDInstance): + raise errors.UbuntuProNotSupportedError( + "Ubuntu Pro builds are only supported with LXC backend." + ) + + craft_cli.emit.debug( + f"Enabling Ubuntu Pro Services {self._pro_services}, {set(self._pro_services)}" + ) + instance.install_pro_client() # type: ignore # noqa: PGH003 + instance.attach_pro_subscription() # type: ignore # noqa: PGH003 + instance.enable_pro_service(self._pro_services) # type: ignore # noqa: PGH003 + + # Cache the current pro services, for prior checks in reentrant calls. + if self._pro_services is not None: + instance.pro_services = self._pro_services # type: ignore # noqa: PGH003 + def run_managed(self, platform: str | None, build_for: str | None) -> None: """Run the application in a managed instance.""" build_planner = self.services.get("build_plan") @@ -418,6 +453,15 @@ def run_managed(self, platform: str | None, build_for: str | None) -> None: "CRAFT_VERBOSITY_LEVEL": craft_cli.emit.get_mode().name, } + if self.app.features.build_secrets: + # If using build secrets, put them in the environment of the managed + # instance. + secret_values = cast(secrets.BuildSecrets, self._secrets) + # disable logging CRAFT_SECRETS value passed to the managed instance + craft_cli.emit.set_secrets(list(secret_values.environment.values())) + + env.update(secret_values.environment) + extra_args["env"] = env craft_cli.emit.debug( @@ -434,24 +478,11 @@ def run_managed(self, platform: str | None, build_for: str | None) -> None: clean_existing=self._enable_fetch_service, use_base_instance=not active_fetch_service, ) as instance: - # if pro services are required, ensure the pro client is - # installed, attached and the correct services are enabled - if self._pro_services: - # Suggestion: create a Pro abstract class used to ensure minimum support by instances. - # we can then check for pro support by inheritance. - if not isinstance(instance, craft_providers.lxd.LXDInstance): - raise errors.UbuntuProNotSupportedError( - "Ubuntu Pro builds are only supported on LXC." - ) + if self._enable_fetch_service: + session_env = self.services.fetch.create_session(instance) + env.update(session_env) - craft_cli.emit.debug( - f"Enabling Ubuntu Pro Services {self._pro_services} on" - ) - # TODO: remove ignores after these methods are merged into main in craft-providers. - # see https://github.com/canonical/craft-providers/pull/664/files - instance.install_pro_client() # type: ignore # noqa: PGH003 - instance.attach_pro_subscription() # type: ignore # noqa: PGH003 - instance.enable_pro_service(self._pro_services) # type: ignore # noqa: PGH003 + self._configure_instance_with_pro(instance) if self._enable_fetch_service: fetch_env = self.services.fetch.create_session(instance) @@ -605,6 +636,9 @@ def _check_pro_requirement( run_managed: bool, # noqa: FBT001 is_managed: bool, # noqa: FBT001 ) -> None: + craft_cli.emit.debug( + f"pro_services: {pro_services}, run_managed: {run_managed}, is_managed: {is_managed}" + ) if pro_services is not None: # should not be None for all lifecycle commands. # Validate requested pro services on the host if we are running in destructive mode. if not run_managed and not is_managed: @@ -624,7 +658,7 @@ def _check_pro_requirement( f"Validating requested Ubuntu Pro attachment on host: {pro_services}" ) pro_services.validate( - options=ValidatorOptions.ATTACHMENT | ValidatorOptions.SUPPORT + options=ValidatorOptions.AVAILABILITY | ValidatorOptions.SUPPORT ) fetch_service_policy: str | None = getattr(args, "fetch_service_policy", None) @@ -671,12 +705,11 @@ def _run_inner(self) -> int: build_for = build_for.split(",", maxsplit=1)[0] craft_cli.emit.debug(f"Build plan: platform={platform}, build_for={build_for}") - # TODO: Move pro operations out to new service for managing Ubuntu Pro - # A ProServices instance will only be available for lifecycle commands, - # which may consume pro packages, - self._pro_services = getattr(dispatcher.parsed_args(), "pro", None) - # Check that pro services are correctly configured if available - self._check_pro_requirement(self._pro_services, run_managed, is_managed) + # A ProServices instance will only be available for lifecycle commands, + # which may consume pro packages, + self._pro_services = getattr(dispatcher.parsed_args(), "pro", None) + # Check that pro services are correctly configured if available + self._check_pro_requirement(self._pro_services, managed_mode, self.is_managed()) if run_managed or command.needs_project(dispatcher.parsed_args()): self.services.project = self.get_project( diff --git a/craft_application/commands/remote.py b/craft_application/commands/remote.py index 9780b2207..18761300b 100644 --- a/craft_application/commands/remote.py +++ b/craft_application/commands/remote.py @@ -23,6 +23,7 @@ from craft_cli import emit from overrides import override # pyright: ignore[reportUnknownVariableType] +0 from craft_application import errors from craft_application.commands import ExtensibleCommand from craft_application.launchpad.models import Build, BuildState diff --git a/craft_application/errors.py b/craft_application/errors.py index 6f5041671..b596a9b35 100644 --- a/craft_application/errors.py +++ b/craft_application/errors.py @@ -397,7 +397,7 @@ class UbuntuProApiError(UbuntuProError): class InvalidUbuntuProStateError(UbuntuProError): """Base class for exceptions raised during Ubuntu Pro validation.""" - # TODO: some of the resolution strings may not sense in a managed + # TODO: some of the resolution strings may not sense in a managed # noqa: FIX002 # environment. What is the best way to get the is_managed method here? @@ -437,11 +437,9 @@ def __init__(self) -> None: class InvalidUbuntuProServiceError(InvalidUbuntuProStateError): """Raised when the requested Ubuntu Pro service is not supported or invalid.""" - # TODO: Should there be separate exceptions for services that not supported vs. invalid? - # if so where is the list of supported service names? - - def __init__(self, invalid_services: set[str]) -> None: - invalid_services_str = "".join(invalid_services) + def __init__(self, invalid_services: set[str] | None) -> None: + invalid_services_set = invalid_services or set() + invalid_services_str = "".join(invalid_services_set) message = "Invalid Ubuntu Pro Services were requested." resolution = ( @@ -458,17 +456,28 @@ class InvalidUbuntuProStatusError(InvalidUbuntuProStateError): """Raised when the incorrect set of Pro Services are enabled.""" def __init__( - self, requested_services: set[str], available_services: set[str] + self, + requested_services: set[str] | None, + available_services: set[str] | None, ) -> None: - enable_services_str = " ".join(requested_services - available_services) - disable_services_str = " ".join(available_services - requested_services) + requested_services_set = requested_services or set() + available_services_set = available_services or set() + enable_services_str = str(requested_services_set - available_services_set) + disable_services_str = str(available_services_set - requested_services_set) message = "Incorrect Ubuntu Pro Services were enabled." - resolution = ( - "Please enable or disable the following services.\n" - f"Enable: {enable_services_str}\n" - f"Disable: {disable_services_str}\n" - 'See "pro" command for details.' - ) + + if "container" in os.environ: + resolution = ( + "Please enable or disable the following services.\n" + f"Enable: {enable_services_str}\n" + f"Disable: {disable_services_str}\n" + 'See "pro" command for details.' + ) + else: + app_name = os.environ.get("SNAP_INSTANCE_NAME", "*craft") + resolution = ( + f'Please run "{app_name} clean" to reset Ubuntu Pro Services.\n' + ) super().__init__(message=message, resolution=resolution) diff --git a/craft_application/secrets.py b/craft_application/secrets.py new file mode 100644 index 000000000..c658e42c5 --- /dev/null +++ b/craft_application/secrets.py @@ -0,0 +1,192 @@ +# noqa: A005 (stdlib-module-shadowing) +# This file is part of craft_application. +# +# Copyright 2023 Canonical Ltd. +# +# This program is free software: you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License version 3, as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranties of MERCHANTABILITY, +# SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. +# See the GNU Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License along +# with this program. If not, see . +"""Handling of build-time secrets.""" +from __future__ import annotations + +import base64 +import dataclasses +import json +import os +import re +import subprocess +from collections.abc import Mapping +from typing import Any, cast + +from craft_application import errors + +SECRET_REGEX = re.compile(r"\$\(HOST_SECRET:(?P.*)\)") + + +@dataclasses.dataclass(frozen=True) +class BuildSecrets: + """The data needed to correctly handle build-time secrets in the application.""" + + environment: dict[str, str] + """The encoded information that can be passed to a managed instance's environment.""" + + secret_strings: set[str] + """The actual secret text strings, to be passed to craft_cli.""" + + +def render_secrets(yaml_data: dict[str, Any], *, managed_mode: bool) -> BuildSecrets: + """Render/expand the build secrets in a project's yaml data (in-place). + + This function will process directives of the form $(HOST_SECRET:) in string + values in ``yaml_data``. For each such directive, the part is executed (with + bash) and the resulting output replaces the whole directive. The returned object + contains the result of HOST_SECRET processing (for masking with craft-cli and + forwarding to managed instances). + + Note that only a few fields are currently supported: + + - "source" and "build-environment" for parts. + + Using HOST_SECRET directives in any other field is an error. + + :param yaml_data: The project's loaded data + :param managed_mode: Whether the current application is running in managed mode. + """ + command_cache: dict[str, str] = {} + + if managed_mode: + command_cache = _decode_commands(os.environ) + + # Process the fields where we allow build secrets + parts = yaml_data.get("parts", {}) + for part in parts.values(): + _render_part_secrets(part, command_cache, managed_mode) + + # Now loop over all the data to check for build secrets in disallowed fields + _check_for_secrets(yaml_data) + + return BuildSecrets( + environment=_encode_commands(command_cache), + secret_strings=set(command_cache.values()), + ) + + +def _render_part_secrets( + part_data: dict[str, Any], + command_cache: dict[str, Any], + managed_mode: bool, # noqa: FBT001 (boolean positional arg) +) -> None: + # Render "source" + source = part_data.get("source", "") + if (rendered := _render_secret(source, command_cache, managed_mode)) is not None: + part_data["source"] = rendered + + # Render "build-environment" + build_env = part_data.get("build-environment", []) + # "build-environment" is a list of dicts with a single item each + for single_entry_dict in build_env: + for var_name, var_value in single_entry_dict.items(): + rendered = _render_secret(var_value, command_cache, managed_mode) + if rendered is not None: + single_entry_dict[var_name] = rendered + + +def _render_secret( + yaml_string: str, + command_cache: dict[str, str], + managed_mode: bool, # noqa: FBT001 (boolean positional arg) +) -> str | None: + if match := SECRET_REGEX.search(yaml_string): + command = match.group("command") + host_directive = match.group(0) + + if command in command_cache: + output = command_cache[command] + else: + if managed_mode: + # In managed mode the command *must* be in the cache; this is an error. + raise errors.SecretsManagedError(host_directive) + + try: + output = _run_command(command) + except subprocess.CalledProcessError as err: + raise errors.SecretsCommandError( + host_directive, err.stderr.decode() + ) from err + command_cache[command] = output + + return yaml_string.replace(host_directive, output) + return None + + +def _run_command(command: str) -> str: + bash_command = f"set -euo pipefail; {command}" + return ( + subprocess.check_output(["bash", "-c", bash_command], stderr=subprocess.PIPE) + .decode("utf-8") + .strip() + ) + + +# pyright: reportUnknownVariableType=false,reportUnknownArgumentType=false +def _check_for_secrets(data: Any) -> None: # noqa: ANN401 (using Any on purpose) + if isinstance(data, dict): + for key, value in data.items(): + _check_str(value, field_name=key) + if isinstance(value, list): + for item in value: + _check_str(item, field_name=key) + _check_for_secrets(item) + elif isinstance(value, dict): + _check_for_secrets(value) + + +def _check_str( + value: Any, field_name: str # noqa: ANN401 (using Any on purpose) +) -> None: + if isinstance(value, str) and (match := SECRET_REGEX.search(value)): + raise errors.SecretsFieldError( + host_directive=match.group(), field_name=field_name + ) + + +def _encode_commands(commands: dict[str, str]) -> dict[str, str]: + """Encode a dict of (command, command-output) pairs for env serialization. + + The resulting dict can be passed to the environment of managed instances (via + subprocess.run() or equivalents). + """ + if not commands: + # Empty dict: don't spend time encoding anything. + return {} + + # The current encoding scheme is to dump the entire dict to base64-encoded json + # string, then put this resulting string in a dict under the "CRAFT_SECRETS" key. + as_json = json.dumps(commands) + as_bytes = as_json.encode("utf-8") + as_b64 = base64.b64encode(as_bytes) + as_str = as_b64.decode("ascii") + + return {"CRAFT_SECRETS": as_str} + + +def _decode_commands(environment: Mapping[str, Any]) -> dict[str, str]: + as_str = environment.get("CRAFT_SECRETS") + + if as_str is None: + # Not necessarily an error: it means the project has no secrets. + return {} + + as_b64 = as_str.encode("ascii") + as_bytes = base64.b64decode(as_b64) + as_json = as_bytes.decode("utf-8") + + return cast("dict[str, str]", json.loads(as_json)) diff --git a/craft_application/services/service_factory.py b/craft_application/services/service_factory.py index 5f5c559ad..cbfd544b1 100644 --- a/craft_application/services/service_factory.py +++ b/craft_application/services/service_factory.py @@ -57,6 +57,20 @@ T = TypeVar("T") _ClassName = Annotated[str, annotated_types.Predicate(lambda x: x.endswith("Class"))] +_DEFAULT_SERVICES = { + "config": "ConfigService", + "fetch": "FetchService", + "init": "InitService", + "lifecycle": "LifecycleService", + "provider": "ProviderService", + "remote_build": "RemoteBuildService", + "request": "RequestService", +} +_CAMEL_TO_PYTHON_CASE_REGEX = re.compile(r"(?. """Handling of Ubuntu Pro Services.""" + from __future__ import annotations import json import logging import subprocess as sub from enum import Flag, auto +from io import TextIOWrapper from pathlib import Path from typing import Any +import yaml + from craft_application.errors import ( InvalidUbuntuProServiceError, InvalidUbuntuProStatusError, @@ -35,8 +39,7 @@ logger = logging.getLogger(__name__) -# locations to search for pro executable -# TODO: which path will we support in long term? +# check for pro client in these paths for backwards compatibility. PRO_CLIENT_PATHS = [ Path("/usr/bin/ubuntu-advantage"), Path("/usr/bin/ua"), @@ -54,14 +57,10 @@ class ValidatorOptions(Flag): """ SUPPORT = auto() - _ATTACHED = auto() - _DETACHED = auto() - # TODO: remove AVAILABILITY if not needed. This flag is useful if we can manually control - # if a managed instance is pro or not. It allows us to check if the host has - # any pro services to support a pro build. In this case, if pro is not requested - # the managed instance would not be attached. - AVAILABILITY = _ATTACHED - ATTACHMENT = _ATTACHED | _DETACHED + ATTACHED = auto() + DETACHED = auto() + AVAILABILITY = ATTACHED + ATTACHMENT = ATTACHED | DETACHED ENABLEMENT = auto() DEFAULT = SUPPORT | ATTACHMENT | ENABLEMENT @@ -70,12 +69,14 @@ class ProServices(set[str]): """Class for managing pro-services within the lifecycle.""" # placeholder for empty sets - empty_placeholder = "none" + empty_placeholder = "None" supported_services: set[str] = { "esm-apps", "esm-infra", "fips", + # TODO: fips-preview is not part of the spec, but # noqa: FIX002 + # it should be added. Bring this up at regular sync. "fips-preview", "fips-updates", } @@ -88,7 +89,18 @@ class ProServices(set[str]): def __str__(self) -> str: """Convert to string for display to user.""" - return ", ".join(self) if self else self.empty_placeholder + services = ", ".join(self) if self else self.empty_placeholder + return f"" + + @classmethod + def load_yaml(cls, f: TextIOWrapper) -> ProServices: + """Create a new ProServices instance from a yaml file.""" + serialized_data = yaml.safe_load(f) + return cls(serialized_data) + + def save_yaml(self, f: TextIOWrapper) -> None: + """Save the ProServices instance to a yaml file.""" + yaml.safe_dump(set(self), f) @classmethod def from_csv(cls, services: str) -> ProServices: @@ -196,14 +208,23 @@ def validate( # Since we extend the set class, cast ourselves to bool to check if we empty. if we are not # empty, this implies we require pro services. - if self.is_pro_attached() != bool(self): - if ValidatorOptions._ATTACHED in options and self: # type: ignore [reportPrivateUsage] - # Ubuntu Pro is requested but not attached - raise UbuntuProDetachedError + is_pro_attached = self.is_pro_attached() - if ValidatorOptions._DETACHED in options and not self: # type: ignore [reportPrivateUsage] - # Ubuntu Pro is not requested but attached - raise UbuntuProAttachedError + if ( + ValidatorOptions.ATTACHED in options + and bool(self) + and not is_pro_attached + ): + # Ubuntu Pro is requested but not attached + raise UbuntuProDetachedError + + if ( + ValidatorOptions.DETACHED in options + and not bool(self) + and is_pro_attached + ): + # Ubuntu Pro is not requested but attached + raise UbuntuProAttachedError # second, check that the set of enabled pro services in the environment matches # the services specified in this set @@ -213,7 +234,6 @@ def validate( raise InvalidUbuntuProStatusError(self, available_services) except UbuntuProClientNotFoundError: - # If The pro client was not found, we may be on a non Ubuntu # system, but if Pro services were requested, re-raise error if self and not self.pro_client_exists(): diff --git a/docs/conf.py b/docs/conf.py index b368bd202..c7f5e56bb 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -20,7 +20,7 @@ project = "Craft Application" author = "Canonical" -copyright = "2023-%s, %s" % (datetime.date.today().year, author) +copyright = f"2023-{datetime.date.today().year}, {author}" # noqa: A001 # region Configuration for canonical-sphinx ogp_site_url = "https://canonical-craft-application.readthedocs-hosted.com/" diff --git a/docs/reference/changelog.rst b/docs/reference/changelog.rst index e46dfc9c1..b07cffc8f 100644 --- a/docs/reference/changelog.rst +++ b/docs/reference/changelog.rst @@ -295,107 +295,6 @@ Services deprecated. Use ``update_kwargs`` instead. Testing -======= - -- Add a :doc:`pytest-plugin` with a fixture that enables production mode for the - application if a test requires it. - -Breaking changes -================ - -- The pytest plugin includes an auto-used fixture that puts the app into debug mode - by default for tests. -- Support for secrets has been removed. -- The abstract class ``ProjectService`` has been removed. Services can no longer - designate that they require a project, but should instead use the - :py:meth:`~craft_application.services.project.ProjectService.get()` method of the - ``ProjectService`` to retrieve the project. It will error accordingly. -- The ``BuildPlanner`` pydantic model has been replaced with the - :py:class:`~craft_application.services.services.buildplan.BuildPlanService` -- The internal ``BuildInfo`` model is replaced with - :external+craft-platforms:class:`craft_platforms.BuildInfo` - -For a complete list of commits, check out the `5.0.0`_ release on GitHub. - -4.10.0 (2025-Feb-27) --------------------- - -Application -=========== - -- Add an API for additional snaps to be installed in the managed instance by the - provider service. -- Increase timeout in fetch-service queries. - -For a complete list of commits, check out the `4.10.0`_ release on GitHub. - -4.9.1 (2025-Feb-12) -------------------- - -Application -=========== - -- Load python plugins after the emitter has been initialized so they can be logged. - -For a complete list of commits, check out the `4.9.1`_ release on GitHub. - -4.9.0 (2025-Feb-10) -------------------- - -All bug fixes from the 4.8 and 4.4 series are included in 4.9.0. - -Application -=========== - -- Add a feature to allow `Python plugins - `_ - to extend or modify the behaviour of applications that use craft-application as a - framework. The plugin packages must be installed in the same virtual environment - as the application. - -Remote build -============ - -- Add hooks to further customize functionality -- Add a ``--project`` parameter for user-defined Launchpad projects, including - private projects. -- Add "pending" as a displayed status for in-progress remote builds - -For a complete list of commits, check out the `4.9.0`_ release on GitHub. - -4.4.1 (2025-Feb-05) -------------------- - -Application -=========== - -- Fix an issue with processing fetch-service output. -- The fetch-service integration now assumes that the fetch-service snap is - tracking the ``latest/candidate`` channel. - -Remote build -============ - -- Fix a bug where repositories and recipes for private Launchpad projects - would be public while the build was in progress. - -For a complete list of commits, check out the `4.4.1`_ release on GitHub. - -4.8.3 (2025-Jan-31) -------------------- - -Remote build -============ - -- Fix a bug where repositories and recipes for private Launchpad projects - would be public while the build was in progress. -- Fix a bug where the remote-build command would suggest running an invalid - command. -- Fix a bug where a timeout would cause the remote builder to remove an - ongoing build. - -For a complete list of commits, check out the `4.8.3`_ release on GitHub. - 4.8.2 (2025-Jan-16) ------------------- diff --git a/pyproject.toml b/pyproject.toml index e013daefc..2a530ca5f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -362,6 +362,11 @@ ignore = [ "COM812", # Missing trailing comma - mostly the same, but marginal differences. "ISC001", # Single-line implicit string concatenation. + # Ignored due to conflicts with ruff's formatter: + # https://docs.astral.sh/ruff/formatter/#conflicting-lint-rules + "COM812", # Missing trailing comma - mostly the same, but marginal differences. + "ISC001", # Single-line implicit string concatenation. + # Ignored due to common usage in current code "TRY003", # Avoid specifying long messages outside the exception class ] diff --git a/tests/conftest.py b/tests/conftest.py index c8a2a702c..b09e23526 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -27,7 +27,6 @@ from typing import TYPE_CHECKING, Any, cast from unittest.mock import Mock -import craft_application import craft_parts import craft_platforms import distro @@ -44,6 +43,10 @@ from jinja2 import FileSystemLoader from typing_extensions import override +import craft_application +from craft_application import application, git, launchpad, models, services, util +from craft_application.services import service_factory + if TYPE_CHECKING: # pragma: no cover from collections.abc import Iterator @@ -101,6 +104,12 @@ def fake_platform(request: pytest.FixtureRequest) -> str: return request.param +@pytest.fixture(autouse=True) +def reset_services(): + yield + service_factory.ServiceFactory.reset() + + @pytest.fixture def platform_independent_project(fake_project_file, fake_project_dict): """Turn the fake project into a platform-independent project. diff --git a/tests/integration/commands/test_init.py b/tests/integration/commands/test_init.py index 65f817881..4ef450151 100644 --- a/tests/integration/commands/test_init.py +++ b/tests/integration/commands/test_init.py @@ -22,6 +22,7 @@ import textwrap import pytest + from craft_application.commands import InitCommand # init operates in the current working directory diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index ebeacf7bf..b276e6b47 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -24,6 +24,8 @@ import craft_platforms import pytest +from craft_providers import bases, lxd, multipass + from craft_application import launchpad from craft_application.services import provider, remotebuild from craft_providers import lxd, multipass diff --git a/tests/integration/launchpad/conftest.py b/tests/integration/launchpad/conftest.py index 158238b93..4740ab522 100644 --- a/tests/integration/launchpad/conftest.py +++ b/tests/integration/launchpad/conftest.py @@ -6,6 +6,7 @@ import launchpadlib.uris import platformdirs import pytest + from craft_application.launchpad import Launchpad diff --git a/tests/integration/launchpad/test_anonymous_access.py b/tests/integration/launchpad/test_anonymous_access.py index 0b4260d0a..a16e8c495 100644 --- a/tests/integration/launchpad/test_anonymous_access.py +++ b/tests/integration/launchpad/test_anonymous_access.py @@ -1,5 +1,7 @@ """Tests for anonymous access.""" +import datetime + import pytest import requests from craft_application import launchpad diff --git a/tests/integration/services/test_fetch.py b/tests/integration/services/test_fetch.py index 41e9fa5db..c42664718 100644 --- a/tests/integration/services/test_fetch.py +++ b/tests/integration/services/test_fetch.py @@ -30,6 +30,9 @@ import craft_platforms import craft_providers import pytest +from craft_cli import EmitterMode, emit +from craft_providers import bases + from craft_application import errors, fetch, services, util from craft_application.application import DEFAULT_CLI_LOGGERS from craft_application.services.fetch import ( diff --git a/tests/integration/services/test_init.py b/tests/integration/services/test_init.py index c0a70e0b1..d77483c12 100644 --- a/tests/integration/services/test_init.py +++ b/tests/integration/services/test_init.py @@ -22,10 +22,10 @@ import textwrap import pytest + from craft_application import errors from craft_application.models.project import Project from craft_application.services import InitService - from tests.conftest import RepositoryDefinition # init operates in the current working directory diff --git a/tests/integration/services/test_lifecycle.py b/tests/integration/services/test_lifecycle.py index dc0f7e5d0..aa84b02c3 100644 --- a/tests/integration/services/test_lifecycle.py +++ b/tests/integration/services/test_lifecycle.py @@ -21,6 +21,7 @@ import craft_cli import pytest import pytest_check + from craft_application.services.lifecycle import LifecycleService diff --git a/tests/integration/services/test_remotebuild.py b/tests/integration/services/test_remotebuild.py index 14d83a883..86b66dbac 100644 --- a/tests/integration/services/test_remotebuild.py +++ b/tests/integration/services/test_remotebuild.py @@ -16,6 +16,7 @@ """Tests for the remote build service.""" import pytest + from craft_application import errors diff --git a/tests/integration/services/test_service_factory.py b/tests/integration/services/test_service_factory.py index 23efac7ec..8a6ce973d 100644 --- a/tests/integration/services/test_service_factory.py +++ b/tests/integration/services/test_service_factory.py @@ -16,6 +16,7 @@ """Integration tests for ServiceFactory.""" import pytest + from craft_application import services diff --git a/tests/integration/test_version.py b/tests/integration/test_version.py index ee78b3b7c..071f7209d 100644 --- a/tests/integration/test_version.py +++ b/tests/integration/test_version.py @@ -18,9 +18,10 @@ import re import subprocess -import craft_application import pytest +import craft_application + def _repo_has_version_tag() -> bool: """Returns True if the repo has a git tag usable for versioning.""" diff --git a/tests/unit/commands/test_base.py b/tests/unit/commands/test_base.py index 1de81bed1..e2b83a1ec 100644 --- a/tests/unit/commands/test_base.py +++ b/tests/unit/commands/test_base.py @@ -19,10 +19,11 @@ from unittest import mock import pytest -from craft_application.commands import base from craft_cli import EmitterMode, emit from typing_extensions import override +from craft_application.commands import base + @pytest.fixture def fake_command(app_metadata, fake_services): diff --git a/tests/unit/commands/test_init.py b/tests/unit/commands/test_init.py index 57299e5ee..9139f8882 100644 --- a/tests/unit/commands/test_init.py +++ b/tests/unit/commands/test_init.py @@ -20,6 +20,7 @@ import pathlib import pytest + from craft_application.commands import InitCommand from craft_application.errors import InitError diff --git a/tests/unit/commands/test_lifecycle.py b/tests/unit/commands/test_lifecycle.py index 6d113be36..fdc18d7d0 100644 --- a/tests/unit/commands/test_lifecycle.py +++ b/tests/unit/commands/test_lifecycle.py @@ -24,6 +24,9 @@ import craft_parts import craft_platforms import pytest +from craft_cli import emit +from craft_parts import Features + import pytest_mock from craft_application import errors, models from craft_application.application import AppMetadata @@ -52,8 +55,6 @@ UbuntuProDetachedError, ) from craft_application.util import ProServices -from craft_cli import emit -from craft_parts import Features # disable black reformat for improve readability on long parameterisations # fmt: off @@ -92,7 +93,7 @@ (True, ["esm-apps"], [], UbuntuProAttachedError), (False, [], ["esm-apps"], UbuntuProDetachedError), (True, ["esm-apps", "fips-updates"],["fips-updates"], InvalidUbuntuProStatusError), - (True, ["esm-apps",], ["fips-updates", "fips-updates"], InvalidUbuntuProStatusError), + (True, ["esm-apps"], ["fips-updates", "fips-updates"], InvalidUbuntuProStatusError), (True, ["esm-apps"], ["esm-apps", "invalid-service"], InvalidUbuntuProServiceError), ] diff --git a/tests/unit/commands/test_other.py b/tests/unit/commands/test_other.py index d8a936ff0..6a75aaaff 100644 --- a/tests/unit/commands/test_other.py +++ b/tests/unit/commands/test_other.py @@ -18,6 +18,7 @@ import argparse import pytest + from craft_application.commands import InitCommand from craft_application.commands.other import VersionCommand, get_other_command_group diff --git a/tests/unit/git/test_git.py b/tests/unit/git/test_git.py index f15e403e7..0e16ccd41 100644 --- a/tests/unit/git/test_git.py +++ b/tests/unit/git/test_git.py @@ -46,7 +46,6 @@ RemoteBuildInvalidGitRepoError, check_git_repo_for_remote_build, ) - from tests.conftest import RepositoryDefinition diff --git a/tests/unit/launchpad/conftest.py b/tests/unit/launchpad/conftest.py index d5a3ca606..cece05340 100644 --- a/tests/unit/launchpad/conftest.py +++ b/tests/unit/launchpad/conftest.py @@ -18,6 +18,7 @@ import lazr.restfulclient.resource import pytest + from craft_application.launchpad import Launchpad diff --git a/tests/unit/launchpad/models/test_base.py b/tests/unit/launchpad/models/test_base.py index 8733b04a1..f094ad0e9 100644 --- a/tests/unit/launchpad/models/test_base.py +++ b/tests/unit/launchpad/models/test_base.py @@ -21,10 +21,11 @@ from unittest import mock import pytest -from craft_application.launchpad import Launchpad, LaunchpadObject from lazr.restfulclient.resource import Entry from typing_extensions import Self +from craft_application.launchpad import Launchpad, LaunchpadObject + class Type(enum.Enum): """Resource types for testing.""" diff --git a/tests/unit/launchpad/models/test_code.py b/tests/unit/launchpad/models/test_code.py index d8f887086..c99930543 100644 --- a/tests/unit/launchpad/models/test_code.py +++ b/tests/unit/launchpad/models/test_code.py @@ -16,6 +16,7 @@ """Unit tests for source repositories.""" import pytest + from craft_application.launchpad.models import InformationType, code diff --git a/tests/unit/launchpad/test_launchpad.py b/tests/unit/launchpad/test_launchpad.py index eeb69c0bc..067c94360 100644 --- a/tests/unit/launchpad/test_launchpad.py +++ b/tests/unit/launchpad/test_launchpad.py @@ -29,9 +29,10 @@ import launchpadlib.uris import lazr.restfulclient.errors import pytest +from lazr.restfulclient.resource import Entry + from craft_application import launchpad from craft_application.launchpad import models -from lazr.restfulclient.resource import Entry def flatten_enum(e: type[enum.Enum]) -> list: diff --git a/tests/unit/launchpad/test_util.py b/tests/unit/launchpad/test_util.py index d93c31803..67eece8f8 100644 --- a/tests/unit/launchpad/test_util.py +++ b/tests/unit/launchpad/test_util.py @@ -18,10 +18,11 @@ from unittest import mock import pytest -from craft_application.launchpad import models, util from hypothesis import given, strategies from lazr.restfulclient.resource import Entry +from craft_application.launchpad import models, util + @given( path=strategies.iterables( diff --git a/tests/unit/models/test_base.py b/tests/unit/models/test_base.py index c3b732f91..46cf8390c 100644 --- a/tests/unit/models/test_base.py +++ b/tests/unit/models/test_base.py @@ -19,10 +19,11 @@ import pydantic import pytest -from craft_application import errors, models from hypothesis import given, strategies from overrides import override +from craft_application import errors, models + class MyBaseModel(models.CraftBaseModel): value1: int diff --git a/tests/unit/models/test_constraints.py b/tests/unit/models/test_constraints.py index 130ed6c19..d10cd587f 100644 --- a/tests/unit/models/test_constraints.py +++ b/tests/unit/models/test_constraints.py @@ -21,6 +21,8 @@ import pydantic.errors import pytest +from hypothesis import given, strategies + from craft_application.models import constraints from craft_application.models.constraints import ( LicenseStr, @@ -29,7 +31,6 @@ SpdxLicenseStr, VersionStr, ) -from hypothesis import given, strategies ALPHA_NUMERIC = [*ascii_letters, *digits] LOWER_ALPHA_NUMERIC = [*ascii_lowercase, *digits] diff --git a/tests/unit/models/test_manifest.py b/tests/unit/models/test_manifest.py index b172be2b8..f2b369a02 100644 --- a/tests/unit/models/test_manifest.py +++ b/tests/unit/models/test_manifest.py @@ -18,6 +18,9 @@ import craft_platforms import pytest +from craft_providers import bases +from freezegun import freeze_time + from craft_application import util from craft_application.models.manifest import ( CraftManifest, diff --git a/tests/unit/models/test_project.py b/tests/unit/models/test_project.py index 192acb7f2..dc793f914 100644 --- a/tests/unit/models/test_project.py +++ b/tests/unit/models/test_project.py @@ -24,6 +24,7 @@ import craft_platforms import craft_providers.bases import pytest + from craft_application import util from craft_application.errors import CraftValidationError from craft_application.models import ( diff --git a/tests/unit/remote/test_git.py b/tests/unit/remote/test_git.py index 7bfbc2113..aff14e6ec 100644 --- a/tests/unit/remote/test_git.py +++ b/tests/unit/remote/test_git.py @@ -15,6 +15,7 @@ """Remote-build git tests.""" import pytest + from craft_application.git import GitType from craft_application.remote import errors, git diff --git a/tests/unit/remote/test_utils.py b/tests/unit/remote/test_utils.py index bf4336cce..628432a33 100644 --- a/tests/unit/remote/test_utils.py +++ b/tests/unit/remote/test_utils.py @@ -18,6 +18,7 @@ from pathlib import Path import pytest + from craft_application.remote import ( UnsupportedArchitectureError, get_build_id, diff --git a/tests/unit/remote/test_worktree.py b/tests/unit/remote/test_worktree.py index d646a5bcb..9225190b0 100644 --- a/tests/unit/remote/test_worktree.py +++ b/tests/unit/remote/test_worktree.py @@ -18,6 +18,7 @@ from unittest.mock import call import pytest + from craft_application.git import GitError from craft_application.remote import RemoteBuildGitError, WorkTree diff --git a/tests/unit/services/conftest.py b/tests/unit/services/conftest.py index 6136f1cfb..22b6d03c2 100644 --- a/tests/unit/services/conftest.py +++ b/tests/unit/services/conftest.py @@ -23,6 +23,7 @@ import launchpadlib.launchpad import lazr.restfulclient.resource import pytest + from craft_application import launchpad, services diff --git a/tests/unit/services/test_config.py b/tests/unit/services/test_config.py index 6c6670071..b1c67d30a 100644 --- a/tests/unit/services/test_config.py +++ b/tests/unit/services/test_config.py @@ -23,14 +23,15 @@ from collections.abc import Iterator from unittest import mock -import craft_application import craft_cli import pytest import pytest_subprocess import snaphelpers +from hypothesis import given, strategies + +import craft_application from craft_application import launchpad from craft_application.services import config -from hypothesis import given, strategies CRAFT_APPLICATION_TEST_ENTRY_VALUES = [ *( diff --git a/tests/unit/services/test_init.py b/tests/unit/services/test_init.py index d92853436..fbd67651b 100644 --- a/tests/unit/services/test_init.py +++ b/tests/unit/services/test_init.py @@ -26,10 +26,11 @@ import pytest import pytest_check import pytest_mock +from craft_cli.pytest_plugin import RecordingEmitter + from craft_application import errors, services from craft_application.git import GitRepo, short_commit_sha from craft_application.models.constraints import MESSAGE_INVALID_NAME -from craft_cli.pytest_plugin import RecordingEmitter @pytest.fixture diff --git a/tests/unit/services/test_lifecycle.py b/tests/unit/services/test_lifecycle.py index cb78d88cd..63415c5f5 100644 --- a/tests/unit/services/test_lifecycle.py +++ b/tests/unit/services/test_lifecycle.py @@ -59,6 +59,13 @@ def skip_if_build_plan_empty(build_planner: BuildPlanService): pytest.skip(reason="Empty build plan") +from craft_application import errors, models, util +from craft_application.errors import PartsLifecycleError +from craft_application.models.project import BuildInfo +from craft_application.services import lifecycle +from craft_application.util import repositories + + # region Local fixtures class FakePartsLifecycle(lifecycle.LifecycleService): def _init_lifecycle_manager(self) -> LifecycleManager: diff --git a/tests/unit/services/test_package.py b/tests/unit/services/test_package.py index af65cc7c1..c02fa3adb 100644 --- a/tests/unit/services/test_package.py +++ b/tests/unit/services/test_package.py @@ -21,6 +21,7 @@ from typing import TYPE_CHECKING import pytest + from craft_application import errors, models from craft_application.services import package diff --git a/tests/unit/services/test_remotebuild.py b/tests/unit/services/test_remotebuild.py index c42f25fb5..789bd918d 100644 --- a/tests/unit/services/test_remotebuild.py +++ b/tests/unit/services/test_remotebuild.py @@ -24,12 +24,12 @@ import lazr.restfulclient.resource import platformdirs import pytest + from craft_application import errors, git, launchpad, services from craft_application.remote.errors import ( RemoteBuildGitError, RemoteBuildInvalidGitRepoError, ) - from tests.unit.services.conftest import ( get_mock_callable, ) diff --git a/tests/unit/services/test_repositories.py b/tests/unit/services/test_repositories.py index e8543ea99..87fd602e9 100644 --- a/tests/unit/services/test_repositories.py +++ b/tests/unit/services/test_repositories.py @@ -20,6 +20,7 @@ from pathlib import Path import pytest + from craft_application.util import repositories diff --git a/tests/unit/services/test_service_factory.py b/tests/unit/services/test_service_factory.py index 1414cb809..5efecd874 100644 --- a/tests/unit/services/test_service_factory.py +++ b/tests/unit/services/test_service_factory.py @@ -20,7 +20,6 @@ import pytest import pytest_check -from craft_application import AppMetadata, services from craft_cli import emit if TYPE_CHECKING: diff --git a/tests/unit/test_application.py b/tests/unit/test_application.py index 10e4f1446..4df4c0a98 100644 --- a/tests/unit/test_application.py +++ b/tests/unit/test_application.py @@ -26,8 +26,6 @@ from textwrap import dedent from unittest import mock -import craft_application -import craft_application.errors import craft_cli import craft_cli.pytest_plugin import craft_parts @@ -35,6 +33,13 @@ import craft_providers import pytest import pytest_check +from craft_cli import emit +from craft_parts.plugins.plugins import PluginType +from craft_providers import bases, lxd +from overrides import override + +import craft_application +import craft_application.errors from craft_application import ( application, commands, @@ -304,11 +309,13 @@ def test_run_managed_configure_pro(mocker, app, fake_project, fake_build_plan): # provide spec to pass type check for pro support mock_instance = mocker.MagicMock(spec=lxd.LXDInstance) - # TODO: these methods are currently in review, https://github.com/canonical/craft-providers/pull/664/files + # TODO: these methods are currently in review, https://github.com/canonical/craft-providers/pull/664/files # noqa: FIX002 # Remove when craft-providers with pro support in lxd is merged to main. mock_instance.install_pro_client = mocker.Mock() mock_instance.attach_pro_subscription = mocker.Mock() mock_instance.enable_pro_service = mocker.Mock() + # pretend to be a fresh instance w/o any services installed + mock_instance.pro_services = None mock_provider.instance.return_value.__enter__.return_value = mock_instance app.project = fake_project @@ -348,11 +355,13 @@ def test_run_managed_skip_configure_pro(mocker, app, fake_project, fake_build_pl # provide spec to pass type check for pro support mock_instance = mocker.MagicMock(spec=lxd.LXDInstance) - # TODO: Remove when these mocks when methods are present in main. + # TODO: Remove when these mocks when methods are present in main. # noqa: FIX002 # see TODO in test_run_managed_configure_pro for details. mock_instance.install_pro_client = mocker.Mock() mock_instance.attach_pro_subscription = mocker.Mock() mock_instance.enable_pro_service = mocker.Mock() + # pretend to be a fresh instance w/o any services installed + mock_instance.pro_services = None mock_provider.instance.return_value.__enter__.return_value = mock_instance app.project = fake_project @@ -537,7 +546,7 @@ def test_gets_project( tmp_path, app_metadata, fake_services, - mock_pro_api_call, # noqa: ARG001 + mock_pro_api_call, ): project_file = tmp_path / "testcraft.yaml" project_file.write_text(BASIC_PROJECT_YAML) @@ -669,7 +678,7 @@ def test_run_success_unmanaged( fake_project, return_code, load_project, - mock_pro_api_call, # noqa: ARG001 + mock_pro_api_call, ): class UnmanagedCommand(commands.AppCommand): name = "pass" @@ -718,7 +727,7 @@ def test_run_success_managed_with_arch( app, fake_project, mocker, - mock_pro_api_call, # noqa: ARG001 + mock_pro_api_call, ): mocker.patch.object(app, "get_project", return_value=fake_project) app.run_managed = mock.Mock() @@ -735,7 +744,7 @@ def test_run_success_managed_with_platform( app, fake_project, mocker, - mock_pro_api_call, # noqa: ARG001 + mock_pro_api_call, ): mocker.patch.object(app, "get_project", return_value=fake_project) app.run_managed = mock.Mock() @@ -771,7 +780,7 @@ def test_run_passes_platforms( mocker, params, expected_call, - mock_pro_api_call, # noqa: ARG001 + mock_pro_api_call, ): mocker.patch.object(app, "get_project", return_value=fake_project) app.run_managed = mock.Mock(return_value=False) @@ -791,7 +800,7 @@ def test_run_success_managed_inside_managed( mock_dispatcher, return_code, mocker, - mock_pro_api_call, # noqa: ARG001 + mock_pro_api_call, ): mocker.patch.object(app, "get_project", return_value=fake_project) mocker.patch.object( @@ -1326,7 +1335,7 @@ def test_doc_url_in_command_help(monkeypatch, capsys, app): [ (False, False, 1, None), (True, True, 1, None), - (True, False, 1, util.ValidatorOptions.ATTACHMENT | util.ValidatorOptions.SUPPORT,), + (True, False, 1, util.ValidatorOptions.AVAILABILITY | util.ValidatorOptions.SUPPORT), (False, True, 0, None), ], ) diff --git a/tests/unit/test_application_fetch.py b/tests/unit/test_application_fetch.py new file mode 100644 index 000000000..ecf8faba8 --- /dev/null +++ b/tests/unit/test_application_fetch.py @@ -0,0 +1,124 @@ +# This file is part of craft_application. +# +# Copyright 2024 Canonical Ltd. +# +# This program is free software: you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License version 3, as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranties of MERCHANTABILITY, +# SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. +# See the GNU Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License along +# with this program. If not, see . +"""Unit tests for the interaction between the Application and the FetchService.""" +from typing import Any +from unittest import mock + +import craft_providers +import pytest +from typing_extensions import override + +from craft_application import services + + +class FakeFetchService(services.FetchService): + """Fake FetchService that tracks calls""" + + def __init__(self, *args, fetch_calls: list[str], **kwargs): + super().__init__(*args, **kwargs) + self.calls = fetch_calls + + @override + def setup(self) -> None: + self.calls.append("setup") + + @override + def create_session( + self, + instance: craft_providers.Executor, # (unused-method-argument) + ) -> dict[str, str]: + self.calls.append("create_session") + return {} + + @override + def teardown_session(self) -> dict[str, Any]: + self.calls.append("teardown_session") + return {} + + @override + def shutdown(self, *, force: bool = False) -> None: + self.calls.append(f"shutdown({force})") + + +@pytest.mark.parametrize("fake_build_plan", [2], indirect=True) +@pytest.mark.parametrize( + ("pack_args", "expected_calls", "expected_clean_existing"), + [ + # No --enable-fetch-service: no calls to the FetchService + ( + [], + [], + False, + ), + # --enable-fetch-service: full expected calls to the FetchService + ( + ["--enable-fetch-service", "strict"], + [ + # One call to setup + "setup", + # Two pairs of create/teardown sessions, for two builds + "create_session", + "teardown_session", + "create_session", + "teardown_session", + # One call to shut down (with `force`) + "shutdown(True)", + ], + True, + ), + ], +) +def test_run_managed_fetch_service( + app, + fake_project, + fake_build_plan, + monkeypatch, + pack_args, + expected_calls, + expected_clean_existing, +): + """Test that the application calls the correct FetchService methods.""" + mock_provider = mock.MagicMock(spec_set=services.ProviderService) + app.services.provider = mock_provider + app.set_project(fake_project) + + expected_build_infos = 2 + assert len(fake_build_plan) == expected_build_infos + app._build_plan = fake_build_plan + + fetch_calls: list[str] = [] + app.services.register("fetch", FakeFetchService) + app.services.update_kwargs("fetch", fetch_calls=fetch_calls) + + monkeypatch.setattr("sys.argv", ["testcraft", "pack", *pack_args]) + app.run() + + assert fetch_calls == expected_calls + + # Check that the provider service was correctly instructed to clean, or not + # clean, the existing instance. + + # Filter out the various calls to entering and exiting the instance() + # context manager. + instance_calls = [ + call + for call in mock_provider.instance.mock_calls + if "work_dir" in call.kwargs and "clean_existing" in call.kwargs + ] + + assert len(instance_calls) == len(fake_build_plan) + for call in instance_calls: + assert call.kwargs["clean_existing"] == expected_clean_existing diff --git a/tests/unit/test_errors.py b/tests/unit/test_errors.py index f3f214166..1fd648749 100644 --- a/tests/unit/test_errors.py +++ b/tests/unit/test_errors.py @@ -22,13 +22,14 @@ import pytest import pytest_check import yaml +from pydantic import BaseModel +from typing_extensions import Self + from craft_application.errors import ( CraftValidationError, PartsLifecycleError, YamlError, ) -from pydantic import BaseModel -from typing_extensions import Self @pytest.mark.parametrize( diff --git a/tests/unit/test_fetch.py b/tests/unit/test_fetch.py index b8f5d832a..f37853812 100644 --- a/tests/unit/test_fetch.py +++ b/tests/unit/test_fetch.py @@ -24,10 +24,11 @@ import pytest import responses -from craft_application import errors, fetch from craft_providers.lxd import LXDInstance from responses import matchers +from craft_application import errors, fetch + CONTROL = fetch._DEFAULT_CONFIG.control PROXY = fetch._DEFAULT_CONFIG.proxy AUTH = fetch._DEFAULT_CONFIG.auth diff --git a/tests/unit/test_grammar.py b/tests/unit/test_grammar.py index 07055ac9c..7ffc4f78e 100644 --- a/tests/unit/test_grammar.py +++ b/tests/unit/test_grammar.py @@ -17,6 +17,7 @@ import pydantic import pytest + from craft_application.models.grammar import ( GrammarAwareProject, _GrammarAwarePart, diff --git a/tests/unit/test_secrets.py b/tests/unit/test_secrets.py new file mode 100644 index 000000000..ed71faa16 --- /dev/null +++ b/tests/unit/test_secrets.py @@ -0,0 +1,155 @@ +# This file is part of craft_application. +# +# Copyright 2023 Canonical Ltd. +# +# This program is free software: you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License version 3, as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranties of MERCHANTABILITY, +# SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License along +# with this program. If not, see . +"""Tests for build secrets.""" + +import pytest + +from craft_application import errors, secrets + + +@pytest.fixture +def good_yaml_data(): + p1_data = { + "source": "the source secret is $(HOST_SECRET:echo ${SECRET_1})", + "build-environment": [ + {"VAR1": "the env secret is $(HOST_SECRET:echo ${SECRET_2})"}, + {"VAR2": "some value"}, + ], + } + + return {"parts": {"p1": p1_data}} + + +@pytest.mark.parametrize("managed_mode", [True, False]) +def test_secrets_parts(monkeypatch, good_yaml_data, managed_mode): + commands = { + "echo ${SECRET_1}": "source-secret", + "echo ${SECRET_2}": "env-secret", + } + encoded = secrets._encode_commands(commands) + + if not managed_mode: + # Running on the "host"; the secrets will be obtained by running commands, + # so we set some environment variables that those commands will use. + monkeypatch.setenv("SECRET_1", "source-secret") + monkeypatch.setenv("SECRET_2", "env-secret") + else: + # Managed mode; the secrets must necessarily come from the environment already. + # Here we fake the work that the host application would have done: get the + # secrets by running the commands, and then encode them into the environment + # of the launched (managed) application. + monkeypatch.setenv("CRAFT_SECRETS", encoded["CRAFT_SECRETS"]) + + secret_values = secrets.render_secrets(good_yaml_data, managed_mode=managed_mode) + + assert secret_values.environment == encoded + assert secret_values.secret_strings == {"source-secret", "env-secret"} + + p1_data = good_yaml_data["parts"]["p1"] + assert p1_data["source"] == "the source secret is source-secret" + assert p1_data["build-environment"][0]["VAR1"] == "the env secret is env-secret" + + # Check that the rest of the build-environment is preserved + assert p1_data["build-environment"][1]["VAR2"] == "some value" + + +def test_secrets_command_error(): + yaml_data = {"parts": {"p1": {"source": "$(HOST_SECRET:echo ${I_DONT_EXIST})"}}} + + with pytest.raises(errors.SecretsCommandError) as exc: + secrets.render_secrets(yaml_data, managed_mode=False) + + expected_message = ( + 'Error when processing secret "$(HOST_SECRET:echo ${I_DONT_EXIST})"' + ) + expected_details = "I_DONT_EXIST: unbound variable" + + error = exc.value + assert str(error) == expected_message + assert error.details is not None + assert expected_details in error.details + + +def test_secrets_cache(mocker, monkeypatch): + monkeypatch.setenv("SECRET_1", "secret") + p1_data = { + "source": "the source secret is $(HOST_SECRET:echo ${SECRET_1})", + "build-environment": [ + {"VAR1": "the env secret is $(HOST_SECRET:echo ${SECRET_1})"} + ], + } + yaml_data = {"parts": {"p1": p1_data}} + + spied_run = mocker.spy(secrets, "_run_command") + secrets.render_secrets(yaml_data, managed_mode=False) + + # Even though the HOST_SECRET is used twice, only a single bash call is done because + # the command is the same. + spied_run.assert_called_once_with("echo ${SECRET_1}") + + +_SECRET = "$(HOST_SECRET:echo ${GIT_VERSION})" # (this is not a password) + + +@pytest.mark.parametrize( + ("yaml_data", "field_name"), + [ + # A basic string field + ({"version": f"v{_SECRET}"}, "version"), + # A list item + ({"stage-packages": ["first", "second", _SECRET]}, "stage-packages"), + # A dict value + ({"parts": {"p1": {"source-version": f"v{_SECRET}"}}}, "source-version"), + ], +) +def test_secrets_bad_field(monkeypatch, yaml_data, field_name): + monkeypatch.setenv("GIT_VERSION", "1.0") + + with pytest.raises(errors.SecretsFieldError) as exc: + secrets.render_secrets(yaml_data, managed_mode=False) + + expected_error = f'Build secret "{_SECRET}" is not allowed on field "{field_name}"' + err = exc.value + assert str(err) == expected_error + + +def test_secrets_encode_decode(): + commands = { + "echo $VAR1": "var1-value", + "echo $VAR2": "var2-value", + } + + encoded = secrets._encode_commands(commands) + decoded = secrets._decode_commands(encoded) + + assert decoded == commands + + +def test_secrets_encode_decode_empty(): + commands = {} + + encoded = secrets._encode_commands(commands) + decoded = secrets._decode_commands(encoded) + + assert decoded == encoded == commands + + +def test_secrets_managed_missing_key(good_yaml_data): + with pytest.raises(errors.SecretsManagedError) as exc: + secrets.render_secrets(good_yaml_data, managed_mode=True) + + expected_message = 'Build secret "$(HOST_SECRET:echo ${SECRET_1})" was not found in the managed environment.' + assert str(exc.value) == expected_message diff --git a/tests/unit/util/test_docs.py b/tests/unit/util/test_docs.py index 0c63adc9d..648f74329 100644 --- a/tests/unit/util/test_docs.py +++ b/tests/unit/util/test_docs.py @@ -16,6 +16,7 @@ """Tests for documentation functions.""" import pytest + from craft_application.util import docs diff --git a/tests/unit/util/test_error_formatting.py b/tests/unit/util/test_error_formatting.py index 9989e12cf..cbbfac8c3 100644 --- a/tests/unit/util/test_error_formatting.py +++ b/tests/unit/util/test_error_formatting.py @@ -19,6 +19,7 @@ import pytest import pytest_check + from craft_application.util.error_formatting import ( FieldLocationTuple, format_pydantic_error, diff --git a/tests/unit/util/test_logging.py b/tests/unit/util/test_logging.py index 4e63990fd..d8f791b7a 100644 --- a/tests/unit/util/test_logging.py +++ b/tests/unit/util/test_logging.py @@ -1,9 +1,10 @@ import logging import pytest_check -from craft_application import util from hypothesis import given, strategies +from craft_application import util + @given(names=strategies.lists(strategies.text())) def test_setup_loggers_resulting_level(names): diff --git a/tests/unit/util/test_paths.py b/tests/unit/util/test_paths.py index 0aaf61383..890a9e0e9 100644 --- a/tests/unit/util/test_paths.py +++ b/tests/unit/util/test_paths.py @@ -19,9 +19,10 @@ import shutil import pytest +from hypothesis import given, provisional + from craft_application import util from craft_application.util.paths import get_filename_from_url_path -from hypothesis import given, provisional def test_get_managed_logpath(app_metadata): diff --git a/tests/unit/util/test_retry.py b/tests/unit/util/test_retry.py index 17ede13dd..1739ebf52 100644 --- a/tests/unit/util/test_retry.py +++ b/tests/unit/util/test_retry.py @@ -19,6 +19,7 @@ from unittest.mock import call import pytest + from craft_application.util import retry diff --git a/tests/unit/util/test_snap_config.py b/tests/unit/util/test_snap_config.py index cc5e908d5..34066c14a 100644 --- a/tests/unit/util/test_snap_config.py +++ b/tests/unit/util/test_snap_config.py @@ -19,9 +19,10 @@ from unittest.mock import MagicMock import pytest +from snaphelpers import SnapCtlError + from craft_application.errors import CraftValidationError from craft_application.util import SnapConfig, get_snap_config, is_running_from_snap -from snaphelpers import SnapCtlError @pytest.fixture diff --git a/tests/unit/util/test_string.py b/tests/unit/util/test_string.py index 7253f8ff3..36953a1ea 100644 --- a/tests/unit/util/test_string.py +++ b/tests/unit/util/test_string.py @@ -16,6 +16,7 @@ """Tests for internal str autilities.""" import pytest + from craft_application.util import string diff --git a/tests/unit/util/test_system.py b/tests/unit/util/test_system.py index 5cf1915a2..0c7f507f5 100644 --- a/tests/unit/util/test_system.py +++ b/tests/unit/util/test_system.py @@ -16,6 +16,7 @@ """Unit tests for system util module.""" import pytest + from craft_application import util from craft_application.errors import InvalidParameterError From 83cb80df2e702615e14ed849dd95012a90780290 Mon Sep 17 00:00:00 2001 From: Adrian Clay Lake Date: Mon, 10 Feb 2025 23:04:17 +0100 Subject: [PATCH 06/14] feat: ensure pro validation does not run on devel bases --- craft_application/application.py | 15 ++++++--- craft_application/errors.py | 13 ++++++++ craft_application/util/pro_services.py | 16 ++++++++-- tests/unit/commands/test_lifecycle.py | 43 ++++++++++++++++++++++++-- tests/unit/test_application.py | 20 +++++++----- 5 files changed, 89 insertions(+), 18 deletions(-) diff --git a/craft_application/application.py b/craft_application/application.py index 7b08fabfd..f7ea92256 100644 --- a/craft_application/application.py +++ b/craft_application/application.py @@ -635,6 +635,7 @@ def _check_pro_requirement( pro_services: ProServices | None, run_managed: bool, # noqa: FBT001 is_managed: bool, # noqa: FBT001 + project: models.Project, ) -> None: craft_cli.emit.debug( f"pro_services: {pro_services}, run_managed: {run_managed}, is_managed: {is_managed}" @@ -645,20 +646,22 @@ def _check_pro_requirement( craft_cli.emit.debug( f"Validating requested Ubuntu Pro status on host: {pro_services}" ) - pro_services.validate() + pro_services.validate_project(project) + pro_services.validate_environment() # Validate requested pro services running in managed mode inside a managed instance. elif run_managed and is_managed: craft_cli.emit.debug( f"Validating requested Ubuntu Pro status in managed instance: {pro_services}" ) - pro_services.validate() + pro_services.validate_environment() # Validate pro attachment and service names on the host before starting a managed instance. elif run_managed and not is_managed: craft_cli.emit.debug( f"Validating requested Ubuntu Pro attachment on host: {pro_services}" ) - pro_services.validate( - options=ValidatorOptions.AVAILABILITY | ValidatorOptions.SUPPORT + pro_services.validate_project(project) + pro_services.validate_environment( + options=ValidatorOptions.AVAILABILITY, ) fetch_service_policy: str | None = getattr(args, "fetch_service_policy", None) @@ -709,7 +712,9 @@ def _run_inner(self) -> int: # which may consume pro packages, self._pro_services = getattr(dispatcher.parsed_args(), "pro", None) # Check that pro services are correctly configured if available - self._check_pro_requirement(self._pro_services, managed_mode, self.is_managed()) + self._check_pro_requirement( + self._pro_services, managed_mode, self.is_managed(), self.get_project() + ) if run_managed or command.needs_project(dispatcher.parsed_args()): self.services.project = self.get_project( diff --git a/craft_application/errors.py b/craft_application/errors.py index b596a9b35..f9bbb5792 100644 --- a/craft_application/errors.py +++ b/craft_application/errors.py @@ -452,6 +452,19 @@ def __init__(self, invalid_services: set[str] | None) -> None: super().__init__(message=message, resolution=resolution) +class InvalidUbuntuProBaseError(InvalidUbuntuProStateError): + """Raised when the requested base, (or build_base) do not support Ubuntu Pro Builds.""" + + def __init__(self) -> None: + message = 'Ubuntu Pro builds are not supported on "devel" bases.' + resolution = ( + 'Remove --pro argument or set "base" and/or "build-base" to a LTS ' + "release. Example: ubuntu@24.04" + ) + + super().__init__(message=message, resolution=resolution) + + class InvalidUbuntuProStatusError(InvalidUbuntuProStateError): """Raised when the incorrect set of Pro Services are enabled.""" diff --git a/craft_application/util/pro_services.py b/craft_application/util/pro_services.py index 31c9e88a6..374324da2 100644 --- a/craft_application/util/pro_services.py +++ b/craft_application/util/pro_services.py @@ -27,7 +27,9 @@ import yaml +from craft_application import models from craft_application.errors import ( + InvalidUbuntuProBaseError, InvalidUbuntuProServiceError, InvalidUbuntuProStatusError, UbuntuProApiError, @@ -51,7 +53,7 @@ class ValidatorOptions(Flag): """Options for ProServices.validate method. SUPPORT: Check names in ProServices set against supported services. - AVAILABILITY: Check Ubuntu Pro is attached if ProServices set is not empty + AVAILABILITY: Check Ubuntu Pro is attached if ProServices are valid. ATTACHMENT: Check Ubuntu Pro is attached or detached to match ProServices set. ENABLEMENT: Check enabled Ubuntu Pro enablement matches ProServices set. """ @@ -59,10 +61,10 @@ class ValidatorOptions(Flag): SUPPORT = auto() ATTACHED = auto() DETACHED = auto() - AVAILABILITY = ATTACHED ATTACHMENT = ATTACHED | DETACHED ENABLEMENT = auto() DEFAULT = SUPPORT | ATTACHMENT | ENABLEMENT + AVAILABILITY = ATTACHED | SUPPORT class ProServices(set[str]): @@ -192,7 +194,15 @@ def get_pro_services(cls) -> ProServices: return cls(service_names) - def validate( + def validate_project(self, project: models.Project) -> None: + """Ensure no unsupported interim bases are used in Ubuntu Pro builds.""" + if bool(self) and ( + (project.base is not None and project.base == "devel") + or (project.build_base is not None and project.build_base == "devel") + ): + raise InvalidUbuntuProBaseError + + def validate_environment( self, options: ValidatorOptions = ValidatorOptions.DEFAULT, ) -> None: diff --git a/tests/unit/commands/test_lifecycle.py b/tests/unit/commands/test_lifecycle.py index fdc18d7d0..b0e678946 100644 --- a/tests/unit/commands/test_lifecycle.py +++ b/tests/unit/commands/test_lifecycle.py @@ -49,6 +49,7 @@ pytestmark = [pytest.mark.usefixtures("fake_project_file")] from craft_application.errors import ( + InvalidUbuntuProBaseError, InvalidUbuntuProServiceError, InvalidUbuntuProStatusError, UbuntuProAttachedError, @@ -97,6 +98,18 @@ (True, ["esm-apps"], ["esm-apps", "invalid-service"], InvalidUbuntuProServiceError), ] +PRO_PROJECT_CONFIGS = [ + # base build_base pro_services_args expected_exception + ("ubuntu@20.04", None, "esm-apps", None), + ("ubuntu@20.04", "ubuntu@20.04", "esm-apps", None), + ("devel", None, "esm-apps", InvalidUbuntuProBaseError), + ("ubuntu@20.04", "devel", "esm-apps", InvalidUbuntuProBaseError), + ("devel", "ubuntu@20.04", "esm-apps", InvalidUbuntuProBaseError), + ("devel", None, None, None), + ("ubuntu@20.04", "devel", None, None), + ("devel", "ubuntu@20.04", None, None), +] + STEP_NAMES = [step.name.lower() for step in craft_parts.Step] MANAGED_LIFECYCLE_COMMANDS = ( PullCommand, @@ -131,7 +144,7 @@ def run_managed(self, parsed_args: argparse.Namespace) -> bool: ("is_attached", "enabled_services", "pro_services_args", "expected_exception"), PRO_SERVICE_CONFIGS, ) -def test_validate_pro_services( +def pro_services_validate_environment( mock_pro_api_call, is_attached, enabled_services, @@ -150,7 +163,33 @@ def test_validate_pro_services( with exception_context: # create and validate pro services pro_services = ProServices(pro_services_args) - pro_services.validate() + pro_services.validate_environment() + + +@pytest.mark.parametrize( + ("base", "build_base", "pro_services_args", "expected_exception"), + PRO_PROJECT_CONFIGS, +) +def pro_services_validate_project( + mocker, + base, + build_base, + pro_services_args, + expected_exception, +): + # configure project + project = mocker.Mock() + project.base = base + project.build_base = build_base + + exception_context = ( + pytest.raises(expected_exception) if expected_exception else nullcontext() + ) + + with exception_context: + # create and validate pro services + pro_services = ProServices(pro_services_args) + pro_services.validate_project(project) @pytest.mark.parametrize( diff --git a/tests/unit/test_application.py b/tests/unit/test_application.py index 4df4c0a98..df8d0a3e9 100644 --- a/tests/unit/test_application.py +++ b/tests/unit/test_application.py @@ -1331,23 +1331,27 @@ def test_doc_url_in_command_help(monkeypatch, capsys, app): # fmt: off @pytest.mark.parametrize( - ( "run_managed", "is_managed", "call_count", "validator_options"), + ( "run_managed", "is_managed", "val_env_calls", "val_prj_calls", "validator_options"), [ - (False, False, 1, None), - (True, True, 1, None), - (True, False, 1, util.ValidatorOptions.AVAILABILITY | util.ValidatorOptions.SUPPORT), - (False, True, 0, None), + (False, False, 1, 1, None), + (True, True, 1, 0, None), + (True, False, 1, 1, util.ValidatorOptions.AVAILABILITY), + (False, True, 0, 0, None), ], ) # fmt: on def test_check_pro_requirement( - mocker, app, run_managed, is_managed, call_count, validator_options + mocker, app, run_managed, is_managed, val_env_calls, val_prj_calls, validator_options ): """Test that _check_pro_requirement validates Pro Services in the correct situations""" pro_services = mocker.Mock() - app._check_pro_requirement(pro_services, run_managed, is_managed) + project = mocker.Mock() + project.base = None + project.build_base = None + app._check_pro_requirement(pro_services, run_managed, is_managed, project) - assert pro_services.validate.call_count == call_count + assert pro_services.validate_environment.call_count == val_env_calls + assert pro_services.validate_project.call_count == val_prj_calls for call in pro_services.validate.call_args_list: if validator_options is not None: # skip assert if default value is passed From c8dc70ccd11f62ce58dcf61c077f9e417ea2e4dc Mon Sep 17 00:00:00 2001 From: Adrian Clay Lake Date: Sun, 16 Feb 2025 20:06:57 +0100 Subject: [PATCH 07/14] feat: more descriptive text for InvalidUbuntuProBaseError --- craft_application/errors.py | 9 +++------ craft_application/util/pro_services.py | 11 ++++++----- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/craft_application/errors.py b/craft_application/errors.py index f9bbb5792..3f97640d0 100644 --- a/craft_application/errors.py +++ b/craft_application/errors.py @@ -455,12 +455,9 @@ def __init__(self, invalid_services: set[str] | None) -> None: class InvalidUbuntuProBaseError(InvalidUbuntuProStateError): """Raised when the requested base, (or build_base) do not support Ubuntu Pro Builds.""" - def __init__(self) -> None: - message = 'Ubuntu Pro builds are not supported on "devel" bases.' - resolution = ( - 'Remove --pro argument or set "base" and/or "build-base" to a LTS ' - "release. Example: ubuntu@24.04" - ) + def __init__(self, base_type: str, base_name: str) -> None: + message = f'Ubuntu Pro builds are not supported on "{base_name}" {base_type}.' + resolution = f"Remove --pro argument or set {base_type} to a supported base." super().__init__(message=message, resolution=resolution) diff --git a/craft_application/util/pro_services.py b/craft_application/util/pro_services.py index 374324da2..a6b9413e5 100644 --- a/craft_application/util/pro_services.py +++ b/craft_application/util/pro_services.py @@ -196,11 +196,12 @@ def get_pro_services(cls) -> ProServices: def validate_project(self, project: models.Project) -> None: """Ensure no unsupported interim bases are used in Ubuntu Pro builds.""" - if bool(self) and ( - (project.base is not None and project.base == "devel") - or (project.build_base is not None and project.build_base == "devel") - ): - raise InvalidUbuntuProBaseError + invalid_bases = ["devel", "ubuntu@24.04"] + if bool(self): + if project.base is not None and project.base in invalid_bases: + raise InvalidUbuntuProBaseError("base", project.base) + if project.build_base is not None and project.build_base in invalid_bases: + raise InvalidUbuntuProBaseError("build-base", project.build_base) def validate_environment( self, From 03f99ce1e456575afa4359d3c8e3de751814ecfc Mon Sep 17 00:00:00 2001 From: Adrian Clay Lake Date: Sun, 16 Feb 2025 20:07:44 +0100 Subject: [PATCH 08/14] fix: move project pro validation to project get method --- craft_application/application.py | 7 +----- tests/unit/test_application.py | 39 +++++++++++++++++++++++--------- 2 files changed, 29 insertions(+), 17 deletions(-) diff --git a/craft_application/application.py b/craft_application/application.py index f7ea92256..4255de977 100644 --- a/craft_application/application.py +++ b/craft_application/application.py @@ -635,7 +635,6 @@ def _check_pro_requirement( pro_services: ProServices | None, run_managed: bool, # noqa: FBT001 is_managed: bool, # noqa: FBT001 - project: models.Project, ) -> None: craft_cli.emit.debug( f"pro_services: {pro_services}, run_managed: {run_managed}, is_managed: {is_managed}" @@ -646,7 +645,6 @@ def _check_pro_requirement( craft_cli.emit.debug( f"Validating requested Ubuntu Pro status on host: {pro_services}" ) - pro_services.validate_project(project) pro_services.validate_environment() # Validate requested pro services running in managed mode inside a managed instance. elif run_managed and is_managed: @@ -659,7 +657,6 @@ def _check_pro_requirement( craft_cli.emit.debug( f"Validating requested Ubuntu Pro attachment on host: {pro_services}" ) - pro_services.validate_project(project) pro_services.validate_environment( options=ValidatorOptions.AVAILABILITY, ) @@ -712,9 +709,7 @@ def _run_inner(self) -> int: # which may consume pro packages, self._pro_services = getattr(dispatcher.parsed_args(), "pro", None) # Check that pro services are correctly configured if available - self._check_pro_requirement( - self._pro_services, managed_mode, self.is_managed(), self.get_project() - ) + self._check_pro_requirement(self._pro_services, managed_mode, self.is_managed()) if run_managed or command.needs_project(dispatcher.parsed_args()): self.services.project = self.get_project( diff --git a/tests/unit/test_application.py b/tests/unit/test_application.py index df8d0a3e9..b8096c014 100644 --- a/tests/unit/test_application.py +++ b/tests/unit/test_application.py @@ -1331,28 +1331,45 @@ def test_doc_url_in_command_help(monkeypatch, capsys, app): # fmt: off @pytest.mark.parametrize( - ( "run_managed", "is_managed", "val_env_calls", "val_prj_calls", "validator_options"), + ( "run_managed", "is_managed", "val_env_calls", "validator_options"), [ - (False, False, 1, 1, None), - (True, True, 1, 0, None), - (True, False, 1, 1, util.ValidatorOptions.AVAILABILITY), - (False, True, 0, 0, None), + (False, False, 1, None), + (True, True, 1, None), + (True, False, 1, util.ValidatorOptions.AVAILABILITY), + (False, True, 0, None), ], ) # fmt: on def test_check_pro_requirement( - mocker, app, run_managed, is_managed, val_env_calls, val_prj_calls, validator_options + mocker, app, run_managed, is_managed, val_env_calls, validator_options ): """Test that _check_pro_requirement validates Pro Services in the correct situations""" pro_services = mocker.Mock() - project = mocker.Mock() - project.base = None - project.build_base = None - app._check_pro_requirement(pro_services, run_managed, is_managed, project) + app._check_pro_requirement(pro_services, run_managed, is_managed) assert pro_services.validate_environment.call_count == val_env_calls - assert pro_services.validate_project.call_count == val_prj_calls for call in pro_services.validate.call_args_list: if validator_options is not None: # skip assert if default value is passed assert call.kwargs["options"] == validator_options + + +# fmt: off +@pytest.mark.parametrize( + ( "is_pro", "val_prj_calls"), + [ + (False, 0), + (True, 1), + ], +) +# fmt: on +@pytest.mark.usefixtures("fake_project_file") +def test_validate_project_pro_requirements(mocker, is_pro, val_prj_calls, app_metadata, fake_services): + """Test validate_project is called only when ProServices are available.""" + app = application.Application(app_metadata, fake_services) + + app._pro_services = mocker.MagicMock() + + app._pro_services.__bool__.return_value = is_pro + app.get_project(build_for=get_host_architecture()) + assert app._pro_services.validate_project.call_count == val_prj_calls From d01e79d9b5d9a2b7cd46b1493fb6ac579914bf3a Mon Sep 17 00:00:00 2001 From: Adrian Clay Lake Date: Sun, 16 Feb 2025 20:09:12 +0100 Subject: [PATCH 09/14] fix: remove base used for testing invalid pro bases --- craft_application/util/pro_services.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/craft_application/util/pro_services.py b/craft_application/util/pro_services.py index a6b9413e5..2d04fff39 100644 --- a/craft_application/util/pro_services.py +++ b/craft_application/util/pro_services.py @@ -196,7 +196,7 @@ def get_pro_services(cls) -> ProServices: def validate_project(self, project: models.Project) -> None: """Ensure no unsupported interim bases are used in Ubuntu Pro builds.""" - invalid_bases = ["devel", "ubuntu@24.04"] + invalid_bases = ["devel"] if bool(self): if project.base is not None and project.base in invalid_bases: raise InvalidUbuntuProBaseError("base", project.base) From f1ee96169a0f46c8b33d90bb8bb25d3d655dd226 Mon Sep 17 00:00:00 2001 From: Adrian Clay Lake Date: Sun, 16 Feb 2025 20:18:04 +0100 Subject: [PATCH 10/14] refactor: minor changes for linting --- tests/unit/test_application.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_application.py b/tests/unit/test_application.py index b8096c014..84fca1e3e 100644 --- a/tests/unit/test_application.py +++ b/tests/unit/test_application.py @@ -1370,6 +1370,6 @@ def test_validate_project_pro_requirements(mocker, is_pro, val_prj_calls, app_me app._pro_services = mocker.MagicMock() - app._pro_services.__bool__.return_value = is_pro + app._pro_services.__bool__.return_value = is_pro # type: ignore # noqa: PGH003 app.get_project(build_for=get_host_architecture()) - assert app._pro_services.validate_project.call_count == val_prj_calls + assert app._pro_services.validate_project.call_count == val_prj_calls # type: ignore # noqa: PGH003 From ba7289aeec44bc6f5da5060e4bb5de198a5e412c Mon Sep 17 00:00:00 2001 From: Adrian Clay Lake Date: Wed, 27 Aug 2025 23:35:45 +0200 Subject: [PATCH 11/14] fix: fast-tests are now executing --- craft_application/application.py | 32 ++++++++++++--------- tests/unit/services/test_lifecycle.py | 2 +- tests/unit/services/test_service_factory.py | 1 + tests/unit/test_application.py | 11 +++---- 4 files changed, 24 insertions(+), 22 deletions(-) diff --git a/craft_application/application.py b/craft_application/application.py index 4255de977..5f9d2fda1 100644 --- a/craft_application/application.py +++ b/craft_application/application.py @@ -696,20 +696,24 @@ def _run_inner(self) -> int: run_managed = command.run_managed(dispatcher.parsed_args()) is_managed = self.is_managed() - # Some commands (e.g. remote build) can allow multiple platforms - # or build-fors, comma-separated. In these cases, we create the - # project using the first defined platform. - if platform and "," in platform: - platform = platform.split(",", maxsplit=1)[0] - if build_for and "," in build_for: - build_for = build_for.split(",", maxsplit=1)[0] - craft_cli.emit.debug(f"Build plan: platform={platform}, build_for={build_for}") - - # A ProServices instance will only be available for lifecycle commands, - # which may consume pro packages, - self._pro_services = getattr(dispatcher.parsed_args(), "pro", None) - # Check that pro services are correctly configured if available - self._check_pro_requirement(self._pro_services, managed_mode, self.is_managed()) + # Some commands (e.g. remote build) can allow multiple platforms + # or build-fors, comma-separated. In these cases, we create the + # project using the first defined platform. + if platform and "," in platform: + platform = platform.split(",", maxsplit=1)[0] + if build_for and "," in build_for: + build_for = build_for.split(",", maxsplit=1)[0] + craft_cli.emit.debug( + f"Build plan: platform={platform}, build_for={build_for}" + ) + + # A ProServices instance will only be available for lifecycle commands, + # which may consume pro packages, + self._pro_services = getattr(dispatcher.parsed_args(), "pro", None) + # Check that pro services are correctly configured if available + self._check_pro_requirement( + self._pro_services, run_managed, self.is_managed() + ) if run_managed or command.needs_project(dispatcher.parsed_args()): self.services.project = self.get_project( diff --git a/tests/unit/services/test_lifecycle.py b/tests/unit/services/test_lifecycle.py index 63415c5f5..8ff6e556e 100644 --- a/tests/unit/services/test_lifecycle.py +++ b/tests/unit/services/test_lifecycle.py @@ -61,7 +61,7 @@ def skip_if_build_plan_empty(build_planner: BuildPlanService): from craft_application import errors, models, util from craft_application.errors import PartsLifecycleError -from craft_application.models.project import BuildInfo +import craft_platforms from craft_application.services import lifecycle from craft_application.util import repositories diff --git a/tests/unit/services/test_service_factory.py b/tests/unit/services/test_service_factory.py index 5efecd874..1414cb809 100644 --- a/tests/unit/services/test_service_factory.py +++ b/tests/unit/services/test_service_factory.py @@ -20,6 +20,7 @@ import pytest import pytest_check +from craft_application import AppMetadata, services from craft_cli import emit if TYPE_CHECKING: diff --git a/tests/unit/test_application.py b/tests/unit/test_application.py index 84fca1e3e..c5354bd36 100644 --- a/tests/unit/test_application.py +++ b/tests/unit/test_application.py @@ -53,12 +53,14 @@ ) from craft_application.util import ( get_host_architecture, # pyright: ignore[reportGeneralTypeIssues] + ValidatorOptions, ) from craft_cli import emit from craft_parts.plugins.plugins import PluginType from craft_providers import bases, lxd from overrides import override + EMPTY_COMMAND_GROUP = craft_cli.CommandGroup("FakeCommands", []) @@ -706,12 +708,7 @@ def test_run_success_managed( app, fake_project, mocker, - mock_pro_api_call, # noqa: ARG001 - monkeypatch, - app, - fake_project, - mocker, - mock_pro_api_call, # noqa: ARG001 + mock_pro_api_call, ): mocker.patch.object(app, "get_project", return_value=fake_project) app.run_managed = mock.Mock() @@ -1335,7 +1332,7 @@ def test_doc_url_in_command_help(monkeypatch, capsys, app): [ (False, False, 1, None), (True, True, 1, None), - (True, False, 1, util.ValidatorOptions.AVAILABILITY), + (True, False, 1, ValidatorOptions.AVAILABILITY), (False, True, 0, None), ], ) From 19e9edd6b471ff8597d0ccfa6379aef20c99e5de Mon Sep 17 00:00:00 2001 From: Adrian Clay Lake Date: Thu, 28 Aug 2025 11:47:58 +0200 Subject: [PATCH 12/14] refactor: rework merge from rebase on to head --- .github/workflows/tests.yaml | 143 ------------- craft_application/application.py | 19 +- craft_application/commands/lifecycle.py | 30 ++- craft_application/commands/remote.py | 1 - craft_application/secrets.py | 192 ------------------ craft_application/services/service_factory.py | 14 -- craft_application/util/__init__.py | 2 - docs/conf.py | 2 +- docs/reference/changelog.rst | 101 +++++++++ pyproject.toml | 5 - tests/conftest.py | 5 +- tests/integration/commands/test_init.py | 1 - tests/integration/conftest.py | 2 - tests/integration/launchpad/conftest.py | 1 - .../launchpad/test_anonymous_access.py | 2 - tests/integration/services/test_fetch.py | 3 - tests/integration/services/test_init.py | 2 +- tests/integration/services/test_lifecycle.py | 1 - .../integration/services/test_remotebuild.py | 1 - .../services/test_service_factory.py | 1 - tests/integration/test_version.py | 3 +- tests/unit/commands/test_base.py | 3 +- tests/unit/commands/test_init.py | 1 - tests/unit/commands/test_lifecycle.py | 3 - tests/unit/commands/test_other.py | 1 - tests/unit/git/test_git.py | 1 + tests/unit/launchpad/conftest.py | 1 - tests/unit/launchpad/models/test_base.py | 3 +- tests/unit/launchpad/models/test_code.py | 1 - tests/unit/launchpad/test_launchpad.py | 3 +- tests/unit/launchpad/test_util.py | 3 +- tests/unit/models/test_base.py | 3 +- tests/unit/models/test_constraints.py | 3 +- tests/unit/models/test_manifest.py | 3 - tests/unit/remote/test_git.py | 1 - tests/unit/remote/test_utils.py | 1 - tests/unit/remote/test_worktree.py | 1 - tests/unit/services/conftest.py | 1 - tests/unit/services/test_config.py | 5 +- tests/unit/services/test_init.py | 3 +- tests/unit/services/test_lifecycle.py | 7 - tests/unit/services/test_package.py | 1 - tests/unit/services/test_remotebuild.py | 2 +- tests/unit/services/test_repositories.py | 1 - tests/unit/test_application.py | 10 +- tests/unit/test_application_fetch.py | 124 ----------- tests/unit/test_errors.py | 5 +- tests/unit/test_fetch.py | 3 +- tests/unit/test_grammar.py | 1 - tests/unit/test_secrets.py | 155 -------------- tests/unit/util/test_docs.py | 1 - tests/unit/util/test_error_formatting.py | 1 - tests/unit/util/test_logging.py | 3 +- tests/unit/util/test_paths.py | 3 +- tests/unit/util/test_retry.py | 1 - tests/unit/util/test_snap_config.py | 3 +- tests/unit/util/test_string.py | 1 - tests/unit/util/test_system.py | 1 - 58 files changed, 139 insertions(+), 756 deletions(-) delete mode 100644 .github/workflows/tests.yaml delete mode 100644 craft_application/secrets.py delete mode 100644 tests/unit/test_application_fetch.py delete mode 100644 tests/unit/test_secrets.py diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml deleted file mode 100644 index 0c6ef4e9e..000000000 --- a/.github/workflows/tests.yaml +++ /dev/null @@ -1,143 +0,0 @@ -name: Tests, linting, etc. -on: - push: - branches: - - "main" - - "feature/*" - - "hotfix/*" - - "release/*" - pull_request: - -jobs: - linters: - runs-on: ubuntu-latest - steps: - - name: Checkout - uses: actions/checkout@v4 - with: - fetch-depth: 0 - - name: Setup Python - uses: actions/setup-python@v5 - with: - python-version: '3.12' - cache: 'pip' - - name: Configure environment - run: | - echo "::group::Begin snap install" - echo "Installing snaps in the background while running apt and pip..." - sudo snap install --no-wait --classic pyright - sudo snap install --no-wait ruff shellcheck - echo "::endgroup::" - echo "::group::apt-get" - sudo apt update - sudo apt-get install -y libapt-pkg-dev - echo "::endgroup::" - echo "::group::pip install" - python -m pip install tox - echo "::endgroup::" - echo "::group::Create virtual environments for linting processes." - tox run -m lint --notest - echo "::endgroup::" - echo "::group::Wait for snap to complete" - snap watch --last=install - echo "::endgroup::" - - name: Run Linters - run: .tox/.tox/bin/tox run --skip-pkg-install --no-list-dependencies -m lint - unit-tests: - strategy: - fail-fast: false - matrix: - platform: [ubuntu-22.04] - runs-on: ${{ matrix.platform }} - steps: - - uses: actions/checkout@v4 - with: - fetch-depth: 0 - - name: Set up Python versions on ${{ matrix.platform }} - uses: actions/setup-python@v5 - with: - python-version: | - 3.10 - 3.12 - cache: 'pip' - - name: Configure environment - run: | - echo "::group::apt-get" - sudo apt update - sudo apt-get install -y libapt-pkg-dev - echo "::endgroup::" - echo "::group::pip install" - python -m pip install tox - echo "::endgroup::" - mkdir -p results - - name: Setup Tox environments - run: tox run -m unit-tests --notest - - name: Unit tests - run: .tox/.tox/bin/tox run --skip-pkg-install --no-list-dependencies --result-json results/tox-${{ matrix.platform }}.json -m unit-tests - env: - PYTEST_ADDOPTS: "--no-header -vv -rN" - - name: Upload code coverage - uses: codecov/codecov-action@v4 - with: - directory: ./results/ - files: coverage*.xml - - name: Upload test results - if: success() || failure() - uses: actions/upload-artifact@v4 - with: - name: unit-test-results-${{ matrix.platform }} - path: results/ - integration-tests: - strategy: - fail-fast: false - matrix: - platform: [ubuntu-22.04] - python: [py310, py312] - runs-on: ${{ matrix.platform }} - steps: - - uses: actions/checkout@v4 - with: - fetch-depth: 0 - - name: Set up Python versions on ${{ matrix.platform }} - uses: actions/setup-python@v5 - with: - python-version: | - 3.10 - 3.12 - cache: 'pip' - - name: Setup LXD - uses: canonical/setup-lxd@v0.1.2 - - name: Configure environment - run: | - echo "::group::Begin snap install" - echo "Installing snaps in the background while running apt and pip..." - sudo snap install --no-wait --channel=candidate fetch-service - echo "::endgroup::" - echo "::group::apt-get" - sudo apt update - sudo apt-get install -y libapt-pkg-dev - echo "::endgroup::" - echo "::group::pip install" - python -m pip install tox - echo "::endgroup::" - mkdir -p results - echo "::group::Wait for snap to complete" - snap watch --last=install - echo "::endgroup::" - - name: Setup Tox environments - run: tox run -e integration-${{ matrix.python }} --notest - - name: Integration tests - run: .tox/.tox/bin/tox run --skip-pkg-install --no-list-dependencies --result-json results/tox-${{ matrix.platform }}-${{ matrix.python }}.json -e integration-${{ matrix.python }} - env: - PYTEST_ADDOPTS: "--no-header -vv -rN" - - name: Upload code coverage - uses: codecov/codecov-action@v4 - with: - directory: ./results/ - files: coverage*.xml - - name: Upload test results - if: success() || failure() - uses: actions/upload-artifact@v4 - with: - name: integration-test-results-${{ matrix.platform }}-${{ matrix.python }} - path: results/ diff --git a/craft_application/application.py b/craft_application/application.py index 5f9d2fda1..19ba4ad4b 100644 --- a/craft_application/application.py +++ b/craft_application/application.py @@ -350,8 +350,6 @@ def _configure_services(self, provider_name: str | None) -> None: "lifecycle", cache_dir=self.cache_dir, work_dir=self._work_dir, - build_plan=self._build_plan, - partitions=self._partitions, use_host_sources=bool(self._pro_services), ) self.services.update_kwargs( @@ -453,15 +451,6 @@ def run_managed(self, platform: str | None, build_for: str | None) -> None: "CRAFT_VERBOSITY_LEVEL": craft_cli.emit.get_mode().name, } - if self.app.features.build_secrets: - # If using build secrets, put them in the environment of the managed - # instance. - secret_values = cast(secrets.BuildSecrets, self._secrets) - # disable logging CRAFT_SECRETS value passed to the managed instance - craft_cli.emit.set_secrets(list(secret_values.environment.values())) - - env.update(secret_values.environment) - extra_args["env"] = env craft_cli.emit.debug( @@ -478,12 +467,6 @@ def run_managed(self, platform: str | None, build_for: str | None) -> None: clean_existing=self._enable_fetch_service, use_base_instance=not active_fetch_service, ) as instance: - if self._enable_fetch_service: - session_env = self.services.fetch.create_session(instance) - env.update(session_env) - - self._configure_instance_with_pro(instance) - if self._enable_fetch_service: fetch_env = self.services.fetch.create_session(instance) env.update(fetch_env) @@ -497,6 +480,7 @@ def run_managed(self, platform: str | None, build_for: str | None) -> None: ) try: with craft_cli.emit.pause(): + # Pyright doesn't fully understand craft_providers's CompletedProcess. instance.execute_run( # pyright: ignore[reportUnknownMemberType,reportUnknownVariableType] cmd, cwd=instance_path, @@ -618,7 +602,6 @@ def _pre_run(self, dispatcher: craft_cli.Dispatcher) -> None: # Some commands might have a project_dir parameter. Those commands and # only those commands should get a project directory, but only when # not managed. - if self.is_managed(): self.project_dir = pathlib.Path("/root/project") elif project_dir := getattr(args, "project_dir", None): diff --git a/craft_application/commands/lifecycle.py b/craft_application/commands/lifecycle.py index 3980c4410..997d9c958 100644 --- a/craft_application/commands/lifecycle.py +++ b/craft_application/commands/lifecycle.py @@ -73,24 +73,18 @@ def _fill_parser(self, parser: argparse.ArgumentParser) -> None: super()._fill_parser(parser) # type: ignore[arg-type] group = parser.add_mutually_exclusive_group() - group.add_argument( - "--destructive-mode", - action="store_true", - help="Build in the current host", - ) - group.add_argument( - "--use-lxd", - action="store_true", - help="Build in a LXD container.", - ) - - @override - def get_managed_cmd(self, parsed_args: argparse.Namespace) -> list[str]: - cmd = super().get_managed_cmd(parsed_args) - - cmd.extend(parsed_args.parts) - - return cmd + if self._allow_destructive: + group.add_argument( + "--destructive-mode", + action="store_true", + help="Build in the current host", + ) + if self._show_lxd_arg: + group.add_argument( + "--use-lxd", + action="store_true", + help="Build in a LXD container.", + ) @override def provider_name(self, parsed_args: argparse.Namespace) -> str | None: diff --git a/craft_application/commands/remote.py b/craft_application/commands/remote.py index 18761300b..9780b2207 100644 --- a/craft_application/commands/remote.py +++ b/craft_application/commands/remote.py @@ -23,7 +23,6 @@ from craft_cli import emit from overrides import override # pyright: ignore[reportUnknownVariableType] -0 from craft_application import errors from craft_application.commands import ExtensibleCommand from craft_application.launchpad.models import Build, BuildState diff --git a/craft_application/secrets.py b/craft_application/secrets.py deleted file mode 100644 index c658e42c5..000000000 --- a/craft_application/secrets.py +++ /dev/null @@ -1,192 +0,0 @@ -# noqa: A005 (stdlib-module-shadowing) -# This file is part of craft_application. -# -# Copyright 2023 Canonical Ltd. -# -# This program is free software: you can redistribute it and/or modify it -# under the terms of the GNU Lesser General Public License version 3, as -# published by the Free Software Foundation. -# -# This program is distributed in the hope that it will be useful, but WITHOUT -# ANY WARRANTY; without even the implied warranties of MERCHANTABILITY, -# SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. -# See the GNU Lesser General Public License for more details. -# -# You should have received a copy of the GNU Lesser General Public License along -# with this program. If not, see . -"""Handling of build-time secrets.""" -from __future__ import annotations - -import base64 -import dataclasses -import json -import os -import re -import subprocess -from collections.abc import Mapping -from typing import Any, cast - -from craft_application import errors - -SECRET_REGEX = re.compile(r"\$\(HOST_SECRET:(?P.*)\)") - - -@dataclasses.dataclass(frozen=True) -class BuildSecrets: - """The data needed to correctly handle build-time secrets in the application.""" - - environment: dict[str, str] - """The encoded information that can be passed to a managed instance's environment.""" - - secret_strings: set[str] - """The actual secret text strings, to be passed to craft_cli.""" - - -def render_secrets(yaml_data: dict[str, Any], *, managed_mode: bool) -> BuildSecrets: - """Render/expand the build secrets in a project's yaml data (in-place). - - This function will process directives of the form $(HOST_SECRET:) in string - values in ``yaml_data``. For each such directive, the part is executed (with - bash) and the resulting output replaces the whole directive. The returned object - contains the result of HOST_SECRET processing (for masking with craft-cli and - forwarding to managed instances). - - Note that only a few fields are currently supported: - - - "source" and "build-environment" for parts. - - Using HOST_SECRET directives in any other field is an error. - - :param yaml_data: The project's loaded data - :param managed_mode: Whether the current application is running in managed mode. - """ - command_cache: dict[str, str] = {} - - if managed_mode: - command_cache = _decode_commands(os.environ) - - # Process the fields where we allow build secrets - parts = yaml_data.get("parts", {}) - for part in parts.values(): - _render_part_secrets(part, command_cache, managed_mode) - - # Now loop over all the data to check for build secrets in disallowed fields - _check_for_secrets(yaml_data) - - return BuildSecrets( - environment=_encode_commands(command_cache), - secret_strings=set(command_cache.values()), - ) - - -def _render_part_secrets( - part_data: dict[str, Any], - command_cache: dict[str, Any], - managed_mode: bool, # noqa: FBT001 (boolean positional arg) -) -> None: - # Render "source" - source = part_data.get("source", "") - if (rendered := _render_secret(source, command_cache, managed_mode)) is not None: - part_data["source"] = rendered - - # Render "build-environment" - build_env = part_data.get("build-environment", []) - # "build-environment" is a list of dicts with a single item each - for single_entry_dict in build_env: - for var_name, var_value in single_entry_dict.items(): - rendered = _render_secret(var_value, command_cache, managed_mode) - if rendered is not None: - single_entry_dict[var_name] = rendered - - -def _render_secret( - yaml_string: str, - command_cache: dict[str, str], - managed_mode: bool, # noqa: FBT001 (boolean positional arg) -) -> str | None: - if match := SECRET_REGEX.search(yaml_string): - command = match.group("command") - host_directive = match.group(0) - - if command in command_cache: - output = command_cache[command] - else: - if managed_mode: - # In managed mode the command *must* be in the cache; this is an error. - raise errors.SecretsManagedError(host_directive) - - try: - output = _run_command(command) - except subprocess.CalledProcessError as err: - raise errors.SecretsCommandError( - host_directive, err.stderr.decode() - ) from err - command_cache[command] = output - - return yaml_string.replace(host_directive, output) - return None - - -def _run_command(command: str) -> str: - bash_command = f"set -euo pipefail; {command}" - return ( - subprocess.check_output(["bash", "-c", bash_command], stderr=subprocess.PIPE) - .decode("utf-8") - .strip() - ) - - -# pyright: reportUnknownVariableType=false,reportUnknownArgumentType=false -def _check_for_secrets(data: Any) -> None: # noqa: ANN401 (using Any on purpose) - if isinstance(data, dict): - for key, value in data.items(): - _check_str(value, field_name=key) - if isinstance(value, list): - for item in value: - _check_str(item, field_name=key) - _check_for_secrets(item) - elif isinstance(value, dict): - _check_for_secrets(value) - - -def _check_str( - value: Any, field_name: str # noqa: ANN401 (using Any on purpose) -) -> None: - if isinstance(value, str) and (match := SECRET_REGEX.search(value)): - raise errors.SecretsFieldError( - host_directive=match.group(), field_name=field_name - ) - - -def _encode_commands(commands: dict[str, str]) -> dict[str, str]: - """Encode a dict of (command, command-output) pairs for env serialization. - - The resulting dict can be passed to the environment of managed instances (via - subprocess.run() or equivalents). - """ - if not commands: - # Empty dict: don't spend time encoding anything. - return {} - - # The current encoding scheme is to dump the entire dict to base64-encoded json - # string, then put this resulting string in a dict under the "CRAFT_SECRETS" key. - as_json = json.dumps(commands) - as_bytes = as_json.encode("utf-8") - as_b64 = base64.b64encode(as_bytes) - as_str = as_b64.decode("ascii") - - return {"CRAFT_SECRETS": as_str} - - -def _decode_commands(environment: Mapping[str, Any]) -> dict[str, str]: - as_str = environment.get("CRAFT_SECRETS") - - if as_str is None: - # Not necessarily an error: it means the project has no secrets. - return {} - - as_b64 = as_str.encode("ascii") - as_bytes = base64.b64decode(as_b64) - as_json = as_bytes.decode("utf-8") - - return cast("dict[str, str]", json.loads(as_json)) diff --git a/craft_application/services/service_factory.py b/craft_application/services/service_factory.py index cbfd544b1..5f5c559ad 100644 --- a/craft_application/services/service_factory.py +++ b/craft_application/services/service_factory.py @@ -57,20 +57,6 @@ T = TypeVar("T") _ClassName = Annotated[str, annotated_types.Predicate(lambda x: x.endswith("Class"))] -_DEFAULT_SERVICES = { - "config": "ConfigService", - "fetch": "FetchService", - "init": "InitService", - "lifecycle": "LifecycleService", - "provider": "ProviderService", - "remote_build": "RemoteBuildService", - "request": "RequestService", -} -_CAMEL_TO_PYTHON_CASE_REGEX = re.compile(r"(?`_ + to extend or modify the behaviour of applications that use craft-application as a + framework. The plugin packages must be installed in the same virtual environment + as the application. + +Remote build +============ + +- Add hooks to further customize functionality +- Add a ``--project`` parameter for user-defined Launchpad projects, including + private projects. +- Add "pending" as a displayed status for in-progress remote builds + +For a complete list of commits, check out the `4.9.0`_ release on GitHub. + +4.4.1 (2025-Feb-05) +------------------- + +Application +=========== + +- Fix an issue with processing fetch-service output. +- The fetch-service integration now assumes that the fetch-service snap is + tracking the ``latest/candidate`` channel. + +Remote build +============ + +- Fix a bug where repositories and recipes for private Launchpad projects + would be public while the build was in progress. + +For a complete list of commits, check out the `4.4.1`_ release on GitHub. + +4.8.3 (2025-Jan-31) +------------------- + +Remote build +============ + +- Fix a bug where repositories and recipes for private Launchpad projects + would be public while the build was in progress. +- Fix a bug where the remote-build command would suggest running an invalid + command. +- Fix a bug where a timeout would cause the remote builder to remove an + ongoing build. + +For a complete list of commits, check out the `4.8.3`_ release on GitHub. + 4.8.2 (2025-Jan-16) ------------------- diff --git a/pyproject.toml b/pyproject.toml index 2a530ca5f..400bdf5e8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -357,11 +357,6 @@ ignore = [ "SIM117", # Use a single `with` statement with multiple contexts instead of nested `with` statements # (reason: this creates long lines that get wrapped and reduces readability) - # Ignored due to conflicts with ruff's formatter: - # https://docs.astral.sh/ruff/formatter/#conflicting-lint-rules - "COM812", # Missing trailing comma - mostly the same, but marginal differences. - "ISC001", # Single-line implicit string concatenation. - # Ignored due to conflicts with ruff's formatter: # https://docs.astral.sh/ruff/formatter/#conflicting-lint-rules "COM812", # Missing trailing comma - mostly the same, but marginal differences. diff --git a/tests/conftest.py b/tests/conftest.py index b09e23526..5e28c6f03 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -27,6 +27,7 @@ from typing import TYPE_CHECKING, Any, cast from unittest.mock import Mock +import craft_application import craft_parts import craft_platforms import distro @@ -43,10 +44,6 @@ from jinja2 import FileSystemLoader from typing_extensions import override -import craft_application -from craft_application import application, git, launchpad, models, services, util -from craft_application.services import service_factory - if TYPE_CHECKING: # pragma: no cover from collections.abc import Iterator diff --git a/tests/integration/commands/test_init.py b/tests/integration/commands/test_init.py index 4ef450151..65f817881 100644 --- a/tests/integration/commands/test_init.py +++ b/tests/integration/commands/test_init.py @@ -22,7 +22,6 @@ import textwrap import pytest - from craft_application.commands import InitCommand # init operates in the current working directory diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index b276e6b47..ebeacf7bf 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -24,8 +24,6 @@ import craft_platforms import pytest -from craft_providers import bases, lxd, multipass - from craft_application import launchpad from craft_application.services import provider, remotebuild from craft_providers import lxd, multipass diff --git a/tests/integration/launchpad/conftest.py b/tests/integration/launchpad/conftest.py index 4740ab522..158238b93 100644 --- a/tests/integration/launchpad/conftest.py +++ b/tests/integration/launchpad/conftest.py @@ -6,7 +6,6 @@ import launchpadlib.uris import platformdirs import pytest - from craft_application.launchpad import Launchpad diff --git a/tests/integration/launchpad/test_anonymous_access.py b/tests/integration/launchpad/test_anonymous_access.py index a16e8c495..0b4260d0a 100644 --- a/tests/integration/launchpad/test_anonymous_access.py +++ b/tests/integration/launchpad/test_anonymous_access.py @@ -1,7 +1,5 @@ """Tests for anonymous access.""" -import datetime - import pytest import requests from craft_application import launchpad diff --git a/tests/integration/services/test_fetch.py b/tests/integration/services/test_fetch.py index c42664718..41e9fa5db 100644 --- a/tests/integration/services/test_fetch.py +++ b/tests/integration/services/test_fetch.py @@ -30,9 +30,6 @@ import craft_platforms import craft_providers import pytest -from craft_cli import EmitterMode, emit -from craft_providers import bases - from craft_application import errors, fetch, services, util from craft_application.application import DEFAULT_CLI_LOGGERS from craft_application.services.fetch import ( diff --git a/tests/integration/services/test_init.py b/tests/integration/services/test_init.py index d77483c12..c0a70e0b1 100644 --- a/tests/integration/services/test_init.py +++ b/tests/integration/services/test_init.py @@ -22,10 +22,10 @@ import textwrap import pytest - from craft_application import errors from craft_application.models.project import Project from craft_application.services import InitService + from tests.conftest import RepositoryDefinition # init operates in the current working directory diff --git a/tests/integration/services/test_lifecycle.py b/tests/integration/services/test_lifecycle.py index aa84b02c3..dc0f7e5d0 100644 --- a/tests/integration/services/test_lifecycle.py +++ b/tests/integration/services/test_lifecycle.py @@ -21,7 +21,6 @@ import craft_cli import pytest import pytest_check - from craft_application.services.lifecycle import LifecycleService diff --git a/tests/integration/services/test_remotebuild.py b/tests/integration/services/test_remotebuild.py index 86b66dbac..14d83a883 100644 --- a/tests/integration/services/test_remotebuild.py +++ b/tests/integration/services/test_remotebuild.py @@ -16,7 +16,6 @@ """Tests for the remote build service.""" import pytest - from craft_application import errors diff --git a/tests/integration/services/test_service_factory.py b/tests/integration/services/test_service_factory.py index 8a6ce973d..23efac7ec 100644 --- a/tests/integration/services/test_service_factory.py +++ b/tests/integration/services/test_service_factory.py @@ -16,7 +16,6 @@ """Integration tests for ServiceFactory.""" import pytest - from craft_application import services diff --git a/tests/integration/test_version.py b/tests/integration/test_version.py index 071f7209d..ee78b3b7c 100644 --- a/tests/integration/test_version.py +++ b/tests/integration/test_version.py @@ -18,9 +18,8 @@ import re import subprocess -import pytest - import craft_application +import pytest def _repo_has_version_tag() -> bool: diff --git a/tests/unit/commands/test_base.py b/tests/unit/commands/test_base.py index e2b83a1ec..1de81bed1 100644 --- a/tests/unit/commands/test_base.py +++ b/tests/unit/commands/test_base.py @@ -19,11 +19,10 @@ from unittest import mock import pytest +from craft_application.commands import base from craft_cli import EmitterMode, emit from typing_extensions import override -from craft_application.commands import base - @pytest.fixture def fake_command(app_metadata, fake_services): diff --git a/tests/unit/commands/test_init.py b/tests/unit/commands/test_init.py index 9139f8882..57299e5ee 100644 --- a/tests/unit/commands/test_init.py +++ b/tests/unit/commands/test_init.py @@ -20,7 +20,6 @@ import pathlib import pytest - from craft_application.commands import InitCommand from craft_application.errors import InitError diff --git a/tests/unit/commands/test_lifecycle.py b/tests/unit/commands/test_lifecycle.py index b0e678946..66eb65272 100644 --- a/tests/unit/commands/test_lifecycle.py +++ b/tests/unit/commands/test_lifecycle.py @@ -24,9 +24,6 @@ import craft_parts import craft_platforms import pytest -from craft_cli import emit -from craft_parts import Features - import pytest_mock from craft_application import errors, models from craft_application.application import AppMetadata diff --git a/tests/unit/commands/test_other.py b/tests/unit/commands/test_other.py index 6a75aaaff..d8a936ff0 100644 --- a/tests/unit/commands/test_other.py +++ b/tests/unit/commands/test_other.py @@ -18,7 +18,6 @@ import argparse import pytest - from craft_application.commands import InitCommand from craft_application.commands.other import VersionCommand, get_other_command_group diff --git a/tests/unit/git/test_git.py b/tests/unit/git/test_git.py index 0e16ccd41..f15e403e7 100644 --- a/tests/unit/git/test_git.py +++ b/tests/unit/git/test_git.py @@ -46,6 +46,7 @@ RemoteBuildInvalidGitRepoError, check_git_repo_for_remote_build, ) + from tests.conftest import RepositoryDefinition diff --git a/tests/unit/launchpad/conftest.py b/tests/unit/launchpad/conftest.py index cece05340..d5a3ca606 100644 --- a/tests/unit/launchpad/conftest.py +++ b/tests/unit/launchpad/conftest.py @@ -18,7 +18,6 @@ import lazr.restfulclient.resource import pytest - from craft_application.launchpad import Launchpad diff --git a/tests/unit/launchpad/models/test_base.py b/tests/unit/launchpad/models/test_base.py index f094ad0e9..8733b04a1 100644 --- a/tests/unit/launchpad/models/test_base.py +++ b/tests/unit/launchpad/models/test_base.py @@ -21,11 +21,10 @@ from unittest import mock import pytest +from craft_application.launchpad import Launchpad, LaunchpadObject from lazr.restfulclient.resource import Entry from typing_extensions import Self -from craft_application.launchpad import Launchpad, LaunchpadObject - class Type(enum.Enum): """Resource types for testing.""" diff --git a/tests/unit/launchpad/models/test_code.py b/tests/unit/launchpad/models/test_code.py index c99930543..d8f887086 100644 --- a/tests/unit/launchpad/models/test_code.py +++ b/tests/unit/launchpad/models/test_code.py @@ -16,7 +16,6 @@ """Unit tests for source repositories.""" import pytest - from craft_application.launchpad.models import InformationType, code diff --git a/tests/unit/launchpad/test_launchpad.py b/tests/unit/launchpad/test_launchpad.py index 067c94360..eeb69c0bc 100644 --- a/tests/unit/launchpad/test_launchpad.py +++ b/tests/unit/launchpad/test_launchpad.py @@ -29,10 +29,9 @@ import launchpadlib.uris import lazr.restfulclient.errors import pytest -from lazr.restfulclient.resource import Entry - from craft_application import launchpad from craft_application.launchpad import models +from lazr.restfulclient.resource import Entry def flatten_enum(e: type[enum.Enum]) -> list: diff --git a/tests/unit/launchpad/test_util.py b/tests/unit/launchpad/test_util.py index 67eece8f8..d93c31803 100644 --- a/tests/unit/launchpad/test_util.py +++ b/tests/unit/launchpad/test_util.py @@ -18,11 +18,10 @@ from unittest import mock import pytest +from craft_application.launchpad import models, util from hypothesis import given, strategies from lazr.restfulclient.resource import Entry -from craft_application.launchpad import models, util - @given( path=strategies.iterables( diff --git a/tests/unit/models/test_base.py b/tests/unit/models/test_base.py index 46cf8390c..c3b732f91 100644 --- a/tests/unit/models/test_base.py +++ b/tests/unit/models/test_base.py @@ -19,11 +19,10 @@ import pydantic import pytest +from craft_application import errors, models from hypothesis import given, strategies from overrides import override -from craft_application import errors, models - class MyBaseModel(models.CraftBaseModel): value1: int diff --git a/tests/unit/models/test_constraints.py b/tests/unit/models/test_constraints.py index d10cd587f..130ed6c19 100644 --- a/tests/unit/models/test_constraints.py +++ b/tests/unit/models/test_constraints.py @@ -21,8 +21,6 @@ import pydantic.errors import pytest -from hypothesis import given, strategies - from craft_application.models import constraints from craft_application.models.constraints import ( LicenseStr, @@ -31,6 +29,7 @@ SpdxLicenseStr, VersionStr, ) +from hypothesis import given, strategies ALPHA_NUMERIC = [*ascii_letters, *digits] LOWER_ALPHA_NUMERIC = [*ascii_lowercase, *digits] diff --git a/tests/unit/models/test_manifest.py b/tests/unit/models/test_manifest.py index f2b369a02..b172be2b8 100644 --- a/tests/unit/models/test_manifest.py +++ b/tests/unit/models/test_manifest.py @@ -18,9 +18,6 @@ import craft_platforms import pytest -from craft_providers import bases -from freezegun import freeze_time - from craft_application import util from craft_application.models.manifest import ( CraftManifest, diff --git a/tests/unit/remote/test_git.py b/tests/unit/remote/test_git.py index aff14e6ec..7bfbc2113 100644 --- a/tests/unit/remote/test_git.py +++ b/tests/unit/remote/test_git.py @@ -15,7 +15,6 @@ """Remote-build git tests.""" import pytest - from craft_application.git import GitType from craft_application.remote import errors, git diff --git a/tests/unit/remote/test_utils.py b/tests/unit/remote/test_utils.py index 628432a33..bf4336cce 100644 --- a/tests/unit/remote/test_utils.py +++ b/tests/unit/remote/test_utils.py @@ -18,7 +18,6 @@ from pathlib import Path import pytest - from craft_application.remote import ( UnsupportedArchitectureError, get_build_id, diff --git a/tests/unit/remote/test_worktree.py b/tests/unit/remote/test_worktree.py index 9225190b0..d646a5bcb 100644 --- a/tests/unit/remote/test_worktree.py +++ b/tests/unit/remote/test_worktree.py @@ -18,7 +18,6 @@ from unittest.mock import call import pytest - from craft_application.git import GitError from craft_application.remote import RemoteBuildGitError, WorkTree diff --git a/tests/unit/services/conftest.py b/tests/unit/services/conftest.py index 22b6d03c2..6136f1cfb 100644 --- a/tests/unit/services/conftest.py +++ b/tests/unit/services/conftest.py @@ -23,7 +23,6 @@ import launchpadlib.launchpad import lazr.restfulclient.resource import pytest - from craft_application import launchpad, services diff --git a/tests/unit/services/test_config.py b/tests/unit/services/test_config.py index b1c67d30a..6c6670071 100644 --- a/tests/unit/services/test_config.py +++ b/tests/unit/services/test_config.py @@ -23,15 +23,14 @@ from collections.abc import Iterator from unittest import mock +import craft_application import craft_cli import pytest import pytest_subprocess import snaphelpers -from hypothesis import given, strategies - -import craft_application from craft_application import launchpad from craft_application.services import config +from hypothesis import given, strategies CRAFT_APPLICATION_TEST_ENTRY_VALUES = [ *( diff --git a/tests/unit/services/test_init.py b/tests/unit/services/test_init.py index fbd67651b..d92853436 100644 --- a/tests/unit/services/test_init.py +++ b/tests/unit/services/test_init.py @@ -26,11 +26,10 @@ import pytest import pytest_check import pytest_mock -from craft_cli.pytest_plugin import RecordingEmitter - from craft_application import errors, services from craft_application.git import GitRepo, short_commit_sha from craft_application.models.constraints import MESSAGE_INVALID_NAME +from craft_cli.pytest_plugin import RecordingEmitter @pytest.fixture diff --git a/tests/unit/services/test_lifecycle.py b/tests/unit/services/test_lifecycle.py index 8ff6e556e..cb78d88cd 100644 --- a/tests/unit/services/test_lifecycle.py +++ b/tests/unit/services/test_lifecycle.py @@ -59,13 +59,6 @@ def skip_if_build_plan_empty(build_planner: BuildPlanService): pytest.skip(reason="Empty build plan") -from craft_application import errors, models, util -from craft_application.errors import PartsLifecycleError -import craft_platforms -from craft_application.services import lifecycle -from craft_application.util import repositories - - # region Local fixtures class FakePartsLifecycle(lifecycle.LifecycleService): def _init_lifecycle_manager(self) -> LifecycleManager: diff --git a/tests/unit/services/test_package.py b/tests/unit/services/test_package.py index c02fa3adb..af65cc7c1 100644 --- a/tests/unit/services/test_package.py +++ b/tests/unit/services/test_package.py @@ -21,7 +21,6 @@ from typing import TYPE_CHECKING import pytest - from craft_application import errors, models from craft_application.services import package diff --git a/tests/unit/services/test_remotebuild.py b/tests/unit/services/test_remotebuild.py index 789bd918d..c42f25fb5 100644 --- a/tests/unit/services/test_remotebuild.py +++ b/tests/unit/services/test_remotebuild.py @@ -24,12 +24,12 @@ import lazr.restfulclient.resource import platformdirs import pytest - from craft_application import errors, git, launchpad, services from craft_application.remote.errors import ( RemoteBuildGitError, RemoteBuildInvalidGitRepoError, ) + from tests.unit.services.conftest import ( get_mock_callable, ) diff --git a/tests/unit/services/test_repositories.py b/tests/unit/services/test_repositories.py index 87fd602e9..e8543ea99 100644 --- a/tests/unit/services/test_repositories.py +++ b/tests/unit/services/test_repositories.py @@ -20,7 +20,6 @@ from pathlib import Path import pytest - from craft_application.util import repositories diff --git a/tests/unit/test_application.py b/tests/unit/test_application.py index c5354bd36..7b67e003e 100644 --- a/tests/unit/test_application.py +++ b/tests/unit/test_application.py @@ -26,6 +26,8 @@ from textwrap import dedent from unittest import mock +import craft_application +import craft_application.errors import craft_cli import craft_cli.pytest_plugin import craft_parts @@ -33,13 +35,7 @@ import craft_providers import pytest import pytest_check -from craft_cli import emit -from craft_parts.plugins.plugins import PluginType -from craft_providers import bases, lxd -from overrides import override -import craft_application -import craft_application.errors from craft_application import ( application, commands, @@ -60,6 +56,8 @@ from craft_providers import bases, lxd from overrides import override +from tests.conftest import FakeApplication + EMPTY_COMMAND_GROUP = craft_cli.CommandGroup("FakeCommands", []) diff --git a/tests/unit/test_application_fetch.py b/tests/unit/test_application_fetch.py deleted file mode 100644 index ecf8faba8..000000000 --- a/tests/unit/test_application_fetch.py +++ /dev/null @@ -1,124 +0,0 @@ -# This file is part of craft_application. -# -# Copyright 2024 Canonical Ltd. -# -# This program is free software: you can redistribute it and/or modify it -# under the terms of the GNU Lesser General Public License version 3, as -# published by the Free Software Foundation. -# -# This program is distributed in the hope that it will be useful, but WITHOUT -# ANY WARRANTY; without even the implied warranties of MERCHANTABILITY, -# SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. -# See the GNU Lesser General Public License for more details. -# -# You should have received a copy of the GNU Lesser General Public License along -# with this program. If not, see . -"""Unit tests for the interaction between the Application and the FetchService.""" -from typing import Any -from unittest import mock - -import craft_providers -import pytest -from typing_extensions import override - -from craft_application import services - - -class FakeFetchService(services.FetchService): - """Fake FetchService that tracks calls""" - - def __init__(self, *args, fetch_calls: list[str], **kwargs): - super().__init__(*args, **kwargs) - self.calls = fetch_calls - - @override - def setup(self) -> None: - self.calls.append("setup") - - @override - def create_session( - self, - instance: craft_providers.Executor, # (unused-method-argument) - ) -> dict[str, str]: - self.calls.append("create_session") - return {} - - @override - def teardown_session(self) -> dict[str, Any]: - self.calls.append("teardown_session") - return {} - - @override - def shutdown(self, *, force: bool = False) -> None: - self.calls.append(f"shutdown({force})") - - -@pytest.mark.parametrize("fake_build_plan", [2], indirect=True) -@pytest.mark.parametrize( - ("pack_args", "expected_calls", "expected_clean_existing"), - [ - # No --enable-fetch-service: no calls to the FetchService - ( - [], - [], - False, - ), - # --enable-fetch-service: full expected calls to the FetchService - ( - ["--enable-fetch-service", "strict"], - [ - # One call to setup - "setup", - # Two pairs of create/teardown sessions, for two builds - "create_session", - "teardown_session", - "create_session", - "teardown_session", - # One call to shut down (with `force`) - "shutdown(True)", - ], - True, - ), - ], -) -def test_run_managed_fetch_service( - app, - fake_project, - fake_build_plan, - monkeypatch, - pack_args, - expected_calls, - expected_clean_existing, -): - """Test that the application calls the correct FetchService methods.""" - mock_provider = mock.MagicMock(spec_set=services.ProviderService) - app.services.provider = mock_provider - app.set_project(fake_project) - - expected_build_infos = 2 - assert len(fake_build_plan) == expected_build_infos - app._build_plan = fake_build_plan - - fetch_calls: list[str] = [] - app.services.register("fetch", FakeFetchService) - app.services.update_kwargs("fetch", fetch_calls=fetch_calls) - - monkeypatch.setattr("sys.argv", ["testcraft", "pack", *pack_args]) - app.run() - - assert fetch_calls == expected_calls - - # Check that the provider service was correctly instructed to clean, or not - # clean, the existing instance. - - # Filter out the various calls to entering and exiting the instance() - # context manager. - instance_calls = [ - call - for call in mock_provider.instance.mock_calls - if "work_dir" in call.kwargs and "clean_existing" in call.kwargs - ] - - assert len(instance_calls) == len(fake_build_plan) - for call in instance_calls: - assert call.kwargs["clean_existing"] == expected_clean_existing diff --git a/tests/unit/test_errors.py b/tests/unit/test_errors.py index 1fd648749..f3f214166 100644 --- a/tests/unit/test_errors.py +++ b/tests/unit/test_errors.py @@ -22,14 +22,13 @@ import pytest import pytest_check import yaml -from pydantic import BaseModel -from typing_extensions import Self - from craft_application.errors import ( CraftValidationError, PartsLifecycleError, YamlError, ) +from pydantic import BaseModel +from typing_extensions import Self @pytest.mark.parametrize( diff --git a/tests/unit/test_fetch.py b/tests/unit/test_fetch.py index f37853812..b8f5d832a 100644 --- a/tests/unit/test_fetch.py +++ b/tests/unit/test_fetch.py @@ -24,11 +24,10 @@ import pytest import responses +from craft_application import errors, fetch from craft_providers.lxd import LXDInstance from responses import matchers -from craft_application import errors, fetch - CONTROL = fetch._DEFAULT_CONFIG.control PROXY = fetch._DEFAULT_CONFIG.proxy AUTH = fetch._DEFAULT_CONFIG.auth diff --git a/tests/unit/test_grammar.py b/tests/unit/test_grammar.py index 7ffc4f78e..07055ac9c 100644 --- a/tests/unit/test_grammar.py +++ b/tests/unit/test_grammar.py @@ -17,7 +17,6 @@ import pydantic import pytest - from craft_application.models.grammar import ( GrammarAwareProject, _GrammarAwarePart, diff --git a/tests/unit/test_secrets.py b/tests/unit/test_secrets.py deleted file mode 100644 index ed71faa16..000000000 --- a/tests/unit/test_secrets.py +++ /dev/null @@ -1,155 +0,0 @@ -# This file is part of craft_application. -# -# Copyright 2023 Canonical Ltd. -# -# This program is free software: you can redistribute it and/or modify it -# under the terms of the GNU Lesser General Public License version 3, as -# published by the Free Software Foundation. -# -# This program is distributed in the hope that it will be useful, but WITHOUT -# ANY WARRANTY; without even the implied warranties of MERCHANTABILITY, -# SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. See the GNU -# Lesser General Public License for more details. -# -# You should have received a copy of the GNU Lesser General Public License along -# with this program. If not, see . -"""Tests for build secrets.""" - -import pytest - -from craft_application import errors, secrets - - -@pytest.fixture -def good_yaml_data(): - p1_data = { - "source": "the source secret is $(HOST_SECRET:echo ${SECRET_1})", - "build-environment": [ - {"VAR1": "the env secret is $(HOST_SECRET:echo ${SECRET_2})"}, - {"VAR2": "some value"}, - ], - } - - return {"parts": {"p1": p1_data}} - - -@pytest.mark.parametrize("managed_mode", [True, False]) -def test_secrets_parts(monkeypatch, good_yaml_data, managed_mode): - commands = { - "echo ${SECRET_1}": "source-secret", - "echo ${SECRET_2}": "env-secret", - } - encoded = secrets._encode_commands(commands) - - if not managed_mode: - # Running on the "host"; the secrets will be obtained by running commands, - # so we set some environment variables that those commands will use. - monkeypatch.setenv("SECRET_1", "source-secret") - monkeypatch.setenv("SECRET_2", "env-secret") - else: - # Managed mode; the secrets must necessarily come from the environment already. - # Here we fake the work that the host application would have done: get the - # secrets by running the commands, and then encode them into the environment - # of the launched (managed) application. - monkeypatch.setenv("CRAFT_SECRETS", encoded["CRAFT_SECRETS"]) - - secret_values = secrets.render_secrets(good_yaml_data, managed_mode=managed_mode) - - assert secret_values.environment == encoded - assert secret_values.secret_strings == {"source-secret", "env-secret"} - - p1_data = good_yaml_data["parts"]["p1"] - assert p1_data["source"] == "the source secret is source-secret" - assert p1_data["build-environment"][0]["VAR1"] == "the env secret is env-secret" - - # Check that the rest of the build-environment is preserved - assert p1_data["build-environment"][1]["VAR2"] == "some value" - - -def test_secrets_command_error(): - yaml_data = {"parts": {"p1": {"source": "$(HOST_SECRET:echo ${I_DONT_EXIST})"}}} - - with pytest.raises(errors.SecretsCommandError) as exc: - secrets.render_secrets(yaml_data, managed_mode=False) - - expected_message = ( - 'Error when processing secret "$(HOST_SECRET:echo ${I_DONT_EXIST})"' - ) - expected_details = "I_DONT_EXIST: unbound variable" - - error = exc.value - assert str(error) == expected_message - assert error.details is not None - assert expected_details in error.details - - -def test_secrets_cache(mocker, monkeypatch): - monkeypatch.setenv("SECRET_1", "secret") - p1_data = { - "source": "the source secret is $(HOST_SECRET:echo ${SECRET_1})", - "build-environment": [ - {"VAR1": "the env secret is $(HOST_SECRET:echo ${SECRET_1})"} - ], - } - yaml_data = {"parts": {"p1": p1_data}} - - spied_run = mocker.spy(secrets, "_run_command") - secrets.render_secrets(yaml_data, managed_mode=False) - - # Even though the HOST_SECRET is used twice, only a single bash call is done because - # the command is the same. - spied_run.assert_called_once_with("echo ${SECRET_1}") - - -_SECRET = "$(HOST_SECRET:echo ${GIT_VERSION})" # (this is not a password) - - -@pytest.mark.parametrize( - ("yaml_data", "field_name"), - [ - # A basic string field - ({"version": f"v{_SECRET}"}, "version"), - # A list item - ({"stage-packages": ["first", "second", _SECRET]}, "stage-packages"), - # A dict value - ({"parts": {"p1": {"source-version": f"v{_SECRET}"}}}, "source-version"), - ], -) -def test_secrets_bad_field(monkeypatch, yaml_data, field_name): - monkeypatch.setenv("GIT_VERSION", "1.0") - - with pytest.raises(errors.SecretsFieldError) as exc: - secrets.render_secrets(yaml_data, managed_mode=False) - - expected_error = f'Build secret "{_SECRET}" is not allowed on field "{field_name}"' - err = exc.value - assert str(err) == expected_error - - -def test_secrets_encode_decode(): - commands = { - "echo $VAR1": "var1-value", - "echo $VAR2": "var2-value", - } - - encoded = secrets._encode_commands(commands) - decoded = secrets._decode_commands(encoded) - - assert decoded == commands - - -def test_secrets_encode_decode_empty(): - commands = {} - - encoded = secrets._encode_commands(commands) - decoded = secrets._decode_commands(encoded) - - assert decoded == encoded == commands - - -def test_secrets_managed_missing_key(good_yaml_data): - with pytest.raises(errors.SecretsManagedError) as exc: - secrets.render_secrets(good_yaml_data, managed_mode=True) - - expected_message = 'Build secret "$(HOST_SECRET:echo ${SECRET_1})" was not found in the managed environment.' - assert str(exc.value) == expected_message diff --git a/tests/unit/util/test_docs.py b/tests/unit/util/test_docs.py index 648f74329..0c63adc9d 100644 --- a/tests/unit/util/test_docs.py +++ b/tests/unit/util/test_docs.py @@ -16,7 +16,6 @@ """Tests for documentation functions.""" import pytest - from craft_application.util import docs diff --git a/tests/unit/util/test_error_formatting.py b/tests/unit/util/test_error_formatting.py index cbbfac8c3..9989e12cf 100644 --- a/tests/unit/util/test_error_formatting.py +++ b/tests/unit/util/test_error_formatting.py @@ -19,7 +19,6 @@ import pytest import pytest_check - from craft_application.util.error_formatting import ( FieldLocationTuple, format_pydantic_error, diff --git a/tests/unit/util/test_logging.py b/tests/unit/util/test_logging.py index d8f791b7a..4e63990fd 100644 --- a/tests/unit/util/test_logging.py +++ b/tests/unit/util/test_logging.py @@ -1,9 +1,8 @@ import logging import pytest_check -from hypothesis import given, strategies - from craft_application import util +from hypothesis import given, strategies @given(names=strategies.lists(strategies.text())) diff --git a/tests/unit/util/test_paths.py b/tests/unit/util/test_paths.py index 890a9e0e9..0aaf61383 100644 --- a/tests/unit/util/test_paths.py +++ b/tests/unit/util/test_paths.py @@ -19,10 +19,9 @@ import shutil import pytest -from hypothesis import given, provisional - from craft_application import util from craft_application.util.paths import get_filename_from_url_path +from hypothesis import given, provisional def test_get_managed_logpath(app_metadata): diff --git a/tests/unit/util/test_retry.py b/tests/unit/util/test_retry.py index 1739ebf52..17ede13dd 100644 --- a/tests/unit/util/test_retry.py +++ b/tests/unit/util/test_retry.py @@ -19,7 +19,6 @@ from unittest.mock import call import pytest - from craft_application.util import retry diff --git a/tests/unit/util/test_snap_config.py b/tests/unit/util/test_snap_config.py index 34066c14a..cc5e908d5 100644 --- a/tests/unit/util/test_snap_config.py +++ b/tests/unit/util/test_snap_config.py @@ -19,10 +19,9 @@ from unittest.mock import MagicMock import pytest -from snaphelpers import SnapCtlError - from craft_application.errors import CraftValidationError from craft_application.util import SnapConfig, get_snap_config, is_running_from_snap +from snaphelpers import SnapCtlError @pytest.fixture diff --git a/tests/unit/util/test_string.py b/tests/unit/util/test_string.py index 36953a1ea..7253f8ff3 100644 --- a/tests/unit/util/test_string.py +++ b/tests/unit/util/test_string.py @@ -16,7 +16,6 @@ """Tests for internal str autilities.""" import pytest - from craft_application.util import string diff --git a/tests/unit/util/test_system.py b/tests/unit/util/test_system.py index 0c7f507f5..5cf1915a2 100644 --- a/tests/unit/util/test_system.py +++ b/tests/unit/util/test_system.py @@ -16,7 +16,6 @@ """Unit tests for system util module.""" import pytest - from craft_application import util from craft_application.errors import InvalidParameterError From e3ce4b848792080a128e8fd4405ceb69904d01e9 Mon Sep 17 00:00:00 2001 From: Adrian Clay Lake Date: Thu, 28 Aug 2025 11:51:19 +0200 Subject: [PATCH 13/14] refactor: additional rework merge from rebase on to head --- pyproject.toml | 2 -- 1 file changed, 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 400bdf5e8..fe37ae0c4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -359,8 +359,6 @@ ignore = [ # Ignored due to conflicts with ruff's formatter: # https://docs.astral.sh/ruff/formatter/#conflicting-lint-rules - "COM812", # Missing trailing comma - mostly the same, but marginal differences. - "ISC001", # Single-line implicit string concatenation. # Ignored due to common usage in current code "TRY003", # Avoid specifying long messages outside the exception class From acfe171f8765bc7013730b92129929e399a9b50d Mon Sep 17 00:00:00 2001 From: Adrian Clay Lake Date: Thu, 28 Aug 2025 11:51:48 +0200 Subject: [PATCH 14/14] refactor: additional rework merge from rebase on to head --- pyproject.toml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index fe37ae0c4..e013daefc 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -359,6 +359,8 @@ ignore = [ # Ignored due to conflicts with ruff's formatter: # https://docs.astral.sh/ruff/formatter/#conflicting-lint-rules + "COM812", # Missing trailing comma - mostly the same, but marginal differences. + "ISC001", # Single-line implicit string concatenation. # Ignored due to common usage in current code "TRY003", # Avoid specifying long messages outside the exception class