From 1aec6c84bef999f99964303eac099151b4449a64 Mon Sep 17 00:00:00 2001 From: Callahan Date: Thu, 3 Nov 2022 13:42:04 +0100 Subject: [PATCH 1/5] snap config: do not crash when config cannot be retrieved (#3971) Signed-off-by: Callahan Kovacs --- snapcraft/snap_config.py | 13 +++++-- snapcraft_legacy/internal/common.py | 13 +++++-- tests/legacy/unit/test_common.py | 53 +++++++++++++++++++++++++++++ tests/unit/test_snap_config.py | 19 ++++++++--- 4 files changed, 90 insertions(+), 8 deletions(-) diff --git a/snapcraft/snap_config.py b/snapcraft/snap_config.py index 85bc0a220c..8e24b893de 100644 --- a/snapcraft/snap_config.py +++ b/snapcraft/snap_config.py @@ -19,7 +19,7 @@ import pydantic from craft_cli import emit -from snaphelpers import SnapConfigOptions +from snaphelpers import SnapConfigOptions, SnapCtlError from snapcraft.utils import is_snapcraft_running_from_snap @@ -75,7 +75,16 @@ def get_snap_config() -> Optional[SnapConfig]: ) return None - snap_config = SnapConfigOptions(keys=["provider"]) + try: + snap_config = SnapConfigOptions(keys=["provider"]) + except (AttributeError, SnapCtlError) as error: + # snaphelpers raises an error (either AttributeError or SnapCtlError) when + # it fails to get the snap config. this can occur when running inside a + # docker or podman container where snapd is not available + emit.debug("Could not retrieve the snap config. Is snapd running?") + emit.trace(f"snaphelpers error: {error!r}") + return None + snap_config.fetch() emit.debug(f"Retrieved snap config: {snap_config.as_dict()}") diff --git a/snapcraft_legacy/internal/common.py b/snapcraft_legacy/internal/common.py index ad6b09499b..ef4a356754 100644 --- a/snapcraft_legacy/internal/common.py +++ b/snapcraft_legacy/internal/common.py @@ -30,7 +30,7 @@ from pathlib import Path from typing import Callable, Dict, List, Optional, Union -from snaphelpers import SnapConfigOptions +from snaphelpers import SnapConfigOptions, SnapCtlError from snapcraft_legacy.internal import errors @@ -384,7 +384,16 @@ def get_snap_config() -> Optional[Dict[str, str]]: ) return None - snap_config = SnapConfigOptions(keys=["provider"]) + try: + snap_config = SnapConfigOptions(keys=["provider"]) + except (AttributeError, SnapCtlError) as error: + # snaphelpers raises an error (either AttributeError or SnapCtlError) when + # it fails to get the snap config. this can occur when running inside a + # docker or podman container where snapd is not available + logger.debug("Could not retrieve the snap config. Is snapd running?") + logger.debug("snaphelpers error: {%r}", error) + return None + snap_config.fetch() logger.debug("Retrieved snap config: %s", snap_config.as_dict()) diff --git a/tests/legacy/unit/test_common.py b/tests/legacy/unit/test_common.py index 0ecd312254..f2fe24b04b 100644 --- a/tests/legacy/unit/test_common.py +++ b/tests/legacy/unit/test_common.py @@ -15,14 +15,29 @@ # along with this program. If not, see . import os +from unittest.mock import MagicMock, patch import pytest +from snaphelpers import SnapCtlError from testtools.matchers import Equals from snapcraft_legacy.internal import common, errors from tests.legacy import unit +@pytest.fixture +def mock_config(): + with patch( + "snapcraft_legacy.internal.common.SnapConfigOptions", autospec=True + ) as mock_snap_config: + yield mock_snap_config + + +@pytest.fixture() +def mock_is_snap(mocker): + yield mocker.patch("snapcraft_legacy.internal.common.is_snap", return_value=True) + + class CommonTestCase(unit.TestCase): def test_set_plugindir(self): plugindir = os.path.join(self.path, "testplugin") @@ -198,3 +213,41 @@ def test_version_missing_and_not_allowed_is_error(): # fact that version is not allowed. with pytest.raises(KeyError): common.format_snap_name(dict(name="name")) + + +@pytest.mark.parametrize("provider", ["lxd", "multipass"]) +def test_get_snap_config(mock_config, mock_is_snap, provider): + """Verify getting a valid snap config.""" + + def fake_as_dict(): + return {"provider": provider} + + mock_config.return_value.as_dict.side_effect = fake_as_dict + + assert common.get_snap_config() == {"provider": provider} + + +def test_get_snap_config_empty(mock_config, mock_is_snap): + """Verify getting an empty config returns a default SnapConfig.""" + + def fake_as_dict(): + return {} + + mock_config.return_value.as_dict.side_effect = fake_as_dict + + assert common.get_snap_config() == {} + + +def test_get_snap_config_not_from_snap(mock_is_snap, mocker): + """Verify None is returned when snapcraft is not running from a snap.""" + mock_is_snap.return_value = False + + assert common.get_snap_config() is None + + +@pytest.mark.parametrize("error", [AttributeError, SnapCtlError(process=MagicMock())]) +def test_get_snap_config_handle_error(error, mock_config, mock_is_snap, mocker): + """An error when retrieving the snap config should return None.""" + mock_config.side_effect = error + + assert common.get_snap_config() is None diff --git a/tests/unit/test_snap_config.py b/tests/unit/test_snap_config.py index 2dce0451af..422b10be1e 100644 --- a/tests/unit/test_snap_config.py +++ b/tests/unit/test_snap_config.py @@ -15,9 +15,10 @@ # along with this program. If not, see . """Unit tests for SnapConfig class.""" -from unittest.mock import patch +from unittest.mock import MagicMock, patch import pytest +from snaphelpers import SnapCtlError from snapcraft.snap_config import SnapConfig, get_snap_config @@ -32,7 +33,7 @@ def mock_config(): @pytest.fixture() def mock_is_running_from_snap(mocker): - mocker.patch( + yield mocker.patch( "snapcraft.snap_config.is_snapcraft_running_from_snap", return_value=True ) @@ -102,8 +103,18 @@ def fake_as_dict(): assert config == SnapConfig() -def test_get_snap_config_not_from_snap(mocker): +def test_get_snap_config_not_from_snap(mocker, mock_is_running_from_snap): """Verify None is returned when snapcraft is not running from a snap.""" - mocker.patch("snapcraft.utils.is_snapcraft_running_from_snap", return_value=False) + mock_is_running_from_snap.return_value = False + + assert get_snap_config() is None + + +@pytest.mark.parametrize("error", [AttributeError, SnapCtlError(process=MagicMock())]) +def test_get_snap_config_handle_error( + error, mock_config, mock_is_running_from_snap, mocker +): + """An error when retrieving the snap config should return None.""" + mock_config.side_effect = error assert get_snap_config() is None From 3a1ed84d57c317e70dc1728b840c2c64010340c1 Mon Sep 17 00:00:00 2001 From: Callahan Date: Fri, 4 Nov 2022 13:47:59 +0100 Subject: [PATCH 2/5] snap config: catch more errors when config cannot be retrieved (#3974) Signed-off-by: Callahan Kovacs --- snapcraft/snap_config.py | 5 +++-- snapcraft_legacy/internal/common.py | 5 +++-- tests/legacy/unit/test_common.py | 12 ++++++++++-- tests/unit/test_snap_config.py | 18 ++++++++++++++---- 4 files changed, 30 insertions(+), 10 deletions(-) diff --git a/snapcraft/snap_config.py b/snapcraft/snap_config.py index 8e24b893de..288e1b5d35 100644 --- a/snapcraft/snap_config.py +++ b/snapcraft/snap_config.py @@ -77,6 +77,9 @@ def get_snap_config() -> Optional[SnapConfig]: try: snap_config = SnapConfigOptions(keys=["provider"]) + # even if the initialization of SnapConfigOptions succeeds, `fetch()` may + # raise the same errors since it makes calls to snapd + snap_config.fetch() except (AttributeError, SnapCtlError) as error: # snaphelpers raises an error (either AttributeError or SnapCtlError) when # it fails to get the snap config. this can occur when running inside a @@ -85,8 +88,6 @@ def get_snap_config() -> Optional[SnapConfig]: emit.trace(f"snaphelpers error: {error!r}") return None - snap_config.fetch() - emit.debug(f"Retrieved snap config: {snap_config.as_dict()}") return SnapConfig.unmarshal(snap_config.as_dict()) diff --git a/snapcraft_legacy/internal/common.py b/snapcraft_legacy/internal/common.py index ef4a356754..6017b40505 100644 --- a/snapcraft_legacy/internal/common.py +++ b/snapcraft_legacy/internal/common.py @@ -386,6 +386,9 @@ def get_snap_config() -> Optional[Dict[str, str]]: try: snap_config = SnapConfigOptions(keys=["provider"]) + # even if the initialization of SnapConfigOptions succeeds, `fetch()` may + # raise the same errors since it makes calls to snapd + snap_config.fetch() except (AttributeError, SnapCtlError) as error: # snaphelpers raises an error (either AttributeError or SnapCtlError) when # it fails to get the snap config. this can occur when running inside a @@ -394,7 +397,5 @@ def get_snap_config() -> Optional[Dict[str, str]]: logger.debug("snaphelpers error: {%r}", error) return None - snap_config.fetch() - logger.debug("Retrieved snap config: %s", snap_config.as_dict()) return snap_config.as_dict() diff --git a/tests/legacy/unit/test_common.py b/tests/legacy/unit/test_common.py index f2fe24b04b..64a91c9786 100644 --- a/tests/legacy/unit/test_common.py +++ b/tests/legacy/unit/test_common.py @@ -246,8 +246,16 @@ def test_get_snap_config_not_from_snap(mock_is_snap, mocker): @pytest.mark.parametrize("error", [AttributeError, SnapCtlError(process=MagicMock())]) -def test_get_snap_config_handle_error(error, mock_config, mock_is_snap, mocker): - """An error when retrieving the snap config should return None.""" +def test_get_snap_config_handle_init_error(error, mock_config, mock_is_snap): + """An error when initializing the snap config object should return None.""" mock_config.side_effect = error assert common.get_snap_config() is None + + +@pytest.mark.parametrize("error", [AttributeError, SnapCtlError(process=MagicMock())]) +def test_get_snap_config_handle_fetch_error(error, mock_config, mock_is_snap): + """An error when fetching the snap config should return None.""" + mock_config.return_value.fetch.side_effect = error + + assert common.get_snap_config() is None diff --git a/tests/unit/test_snap_config.py b/tests/unit/test_snap_config.py index 422b10be1e..4dd9c54970 100644 --- a/tests/unit/test_snap_config.py +++ b/tests/unit/test_snap_config.py @@ -103,7 +103,7 @@ def fake_as_dict(): assert config == SnapConfig() -def test_get_snap_config_not_from_snap(mocker, mock_is_running_from_snap): +def test_get_snap_config_not_from_snap(mock_is_running_from_snap): """Verify None is returned when snapcraft is not running from a snap.""" mock_is_running_from_snap.return_value = False @@ -111,10 +111,20 @@ def test_get_snap_config_not_from_snap(mocker, mock_is_running_from_snap): @pytest.mark.parametrize("error", [AttributeError, SnapCtlError(process=MagicMock())]) -def test_get_snap_config_handle_error( - error, mock_config, mock_is_running_from_snap, mocker +def test_get_snap_config_handle_init_error( + error, mock_config, mock_is_running_from_snap ): - """An error when retrieving the snap config should return None.""" + """An error when initializing the snap config object should return None.""" mock_config.side_effect = error assert get_snap_config() is None + + +@pytest.mark.parametrize("error", [AttributeError, SnapCtlError(process=MagicMock())]) +def test_get_snap_config_handle_fetch_error( + error, mock_config, mock_is_running_from_snap +): + """An error when fetching the snap config should return None.""" + mock_config.return_value.fetch.side_effect = error + + assert get_snap_config() is None From 92a9614f5a8018108f98642e3068c38d8b7990fa Mon Sep 17 00:00:00 2001 From: Alex Lewontin Date: Wed, 9 Nov 2022 11:53:16 -0500 Subject: [PATCH 3/5] schema,tests: add support for new system usernames (#3964) Signed-off-by: Alex Lewontin Co-authored-by: Callahan Kovacs --- schema/snapcraft.json | 2 +- tests/legacy/unit/project/test_schema.py | 20 ++++++++++++++++++-- tests/unit/meta/test_snap_yaml.py | 6 ++++++ tests/unit/test_projects.py | 4 ++++ 4 files changed, 29 insertions(+), 3 deletions(-) diff --git a/schema/snapcraft.json b/schema/snapcraft.json index 806177dc15..c9c61369e4 100644 --- a/schema/snapcraft.json +++ b/schema/snapcraft.json @@ -523,7 +523,7 @@ "additionalProperties": false, "validation-failure": "{!r} is not a valid system-username.", "patternProperties": { - "^snap_(daemon|microk8s)$": { + "^snap_(daemon|microk8s|aziotedge|aziotdu)$": { "oneOf": [ { "$ref": "#/definitions/system-username-scope" diff --git a/tests/legacy/unit/project/test_schema.py b/tests/legacy/unit/project/test_schema.py index 3b5593b791..d84205e50f 100644 --- a/tests/legacy/unit/project/test_schema.py +++ b/tests/legacy/unit/project/test_schema.py @@ -1225,13 +1225,29 @@ def test_invalid_command_chain(data, command_chain): assert expected_message in str(error.value) -@pytest.mark.parametrize("username", ["snap_daemon", "snap_microk8s"]) +@pytest.mark.parametrize( + "username", + [ + "snap_daemon", + "snap_microk8s", + "snap_aziotedge", + "snap_aziotdu", + ], +) def test_yaml_valid_system_usernames_long(data, username): data["system-usernames"] = {username: {"scope": "shared"}} Validator(data).validate() -@pytest.mark.parametrize("username", ["snap_daemon", "snap_microk8s"]) +@pytest.mark.parametrize( + "username", + [ + "snap_daemon", + "snap_microk8s", + "snap_aziotedge", + "snap_aziotdu", + ], +) def test_yaml_valid_system_usernames_short(data, username): data["system-usernames"] = {username: "shared"} Validator(data).validate() diff --git a/tests/unit/meta/test_snap_yaml.py b/tests/unit/meta/test_snap_yaml.py index 13c1566c56..d0b29ceeb1 100644 --- a/tests/unit/meta/test_snap_yaml.py +++ b/tests/unit/meta/test_snap_yaml.py @@ -199,6 +199,9 @@ def complex_project(): snap_daemon: scope: shared snap_microk8s: shared + snap_aziotedge: shared + snap_aziotdu: + scope: shared layout: /usr/share/libdrm: @@ -340,6 +343,9 @@ def test_complex_snap_yaml(complex_project, new_dir): snap_daemon: scope: shared snap_microk8s: shared + snap_aziotedge: shared + snap_aziotdu: + scope: shared provenance: test-provenance-1 """ ) diff --git a/tests/unit/test_projects.py b/tests/unit/test_projects.py index 742938ac66..fadc38e17b 100644 --- a/tests/unit/test_projects.py +++ b/tests/unit/test_projects.py @@ -1025,8 +1025,12 @@ def test_app_sockets_valid_socket_mode(self, socket_mode, socket_yaml_data): [ {"snap_daemon": {"scope": "shared"}}, {"snap_microk8s": {"scope": "shared"}}, + {"snap_aziotedge": {"scope": "shared"}}, + {"snap_aziotdu": {"scope": "shared"}}, {"snap_daemon": "shared"}, {"snap_microk8s": "shared"}, + {"snap_aziotedge": "shared"}, + {"snap_aziotdu": "shared"}, ], ) def test_project_system_usernames_valid(self, system_username, project_yaml_data): From 010fd708d65d753ed76824d8aade0f7689300baf Mon Sep 17 00:00:00 2001 From: Callahan Date: Wed, 9 Nov 2022 13:34:40 -0600 Subject: [PATCH 4/5] cli: set default verbosity level with environment variable (#3958) The default verbosity level can be set with the environment variable SNAPCRAFT_VERBOSITY_LEVEL, where valid values are 'quiet', 'brief', 'verbose', 'debug', and 'trace'. Signed-off-by: Callahan Kovacs --- snapcraft/cli.py | 14 ++++++++++++ snapcraft/utils.py | 12 +++++++++-- tests/unit/cli/test_verbosity.py | 37 +++++++++++++++++++++++++++++++- tests/unit/test_utils.py | 15 +++++++++++++ 4 files changed, 75 insertions(+), 3 deletions(-) diff --git a/snapcraft/cli.py b/snapcraft/cli.py index d31a071891..e204b97813 100644 --- a/snapcraft/cli.py +++ b/snapcraft/cli.py @@ -148,6 +148,20 @@ def get_verbosity() -> EmitterMode: if utils.strtobool(os.getenv("SNAPCRAFT_ENABLE_DEVELOPER_DEBUG", "n").strip()): verbosity = EmitterMode.DEBUG + # if defined, use environmental variable SNAPCRAFT_VERBOSITY_LEVEL + verbosity_env = os.getenv("SNAPCRAFT_VERBOSITY_LEVEL") + if verbosity_env: + try: + verbosity = EmitterMode[verbosity_env.strip().upper()] + except KeyError: + values = utils.humanize_list( + [e.name.lower() for e in EmitterMode], "and", sort=False + ) + raise ArgumentParsingError( + f"cannot parse verbosity level {verbosity_env!r} from environment " + f"variable SNAPCRAFT_VERBOSITY_LEVEL (valid values are {values})" + ) from KeyError + return verbosity diff --git a/snapcraft/utils.py b/snapcraft/utils.py index 57fd66140b..6a1bc5c7ef 100644 --- a/snapcraft/utils.py +++ b/snapcraft/utils.py @@ -290,7 +290,10 @@ def prompt(prompt_text: str, *, hide: bool = False) -> str: def humanize_list( - items: Iterable[str], conjunction: str, item_format: str = "{!r}" + items: Iterable[str], + conjunction: str, + item_format: str = "{!r}", + sort: bool = True, ) -> str: """Format a list into a human-readable string. @@ -298,11 +301,16 @@ def humanize_list( :param conjunction: the conjunction used to join the final element to the rest of the list (e.g. 'and'). :param item_format: format string to use per item. + :param sort: if true, sort the list. """ if not items: return "" - quoted_items = [item_format.format(item) for item in sorted(items)] + quoted_items = [item_format.format(item) for item in items] + + if sort: + quoted_items = sorted(quoted_items) + if len(quoted_items) == 1: return quoted_items[0] diff --git a/tests/unit/cli/test_verbosity.py b/tests/unit/cli/test_verbosity.py index 40ff1f6780..7e108ebbda 100644 --- a/tests/unit/cli/test_verbosity.py +++ b/tests/unit/cli/test_verbosity.py @@ -15,12 +15,14 @@ # along with this program. If not, see . import pytest +from craft_cli import ArgumentParsingError from snapcraft import cli @pytest.mark.parametrize("value", ["yes", "YES", "1", "y", "Y"]) def test_developer_debug_in_env_sets_debug(monkeypatch, value): + """DEVELOPER_DEBUG sets verbosity level to 'debug'.""" monkeypatch.setenv("SNAPCRAFT_ENABLE_DEVELOPER_DEBUG", value) assert cli.get_verbosity() == cli.EmitterMode.DEBUG @@ -28,6 +30,7 @@ def test_developer_debug_in_env_sets_debug(monkeypatch, value): @pytest.mark.parametrize("value", ["no", "NO", "0", "n", "N"]) def test_developer_debug_in_env_sets_brief(monkeypatch, value): + """Default verbosity is used when DEVELOPER_DEBUG=0.""" monkeypatch.setenv("SNAPCRAFT_ENABLE_DEVELOPER_DEBUG", value) monkeypatch.setattr("sys.stdin.isatty", lambda: True) @@ -36,6 +39,7 @@ def test_developer_debug_in_env_sets_brief(monkeypatch, value): @pytest.mark.parametrize("value", ["foo", "BAR", "2"]) def test_parse_issue_in_developer_debug_in_env_sets_brief(monkeypatch, value): + """Invalid values for DEVELOPER_DEBUG are silently ignored.""" monkeypatch.setenv("SNAPCRAFT_ENABLE_DEVELOPER_DEBUG", value) monkeypatch.setattr("sys.stdin.isatty", lambda: True) @@ -43,6 +47,7 @@ def test_parse_issue_in_developer_debug_in_env_sets_brief(monkeypatch, value): def test_no_env_returns_brief(monkeypatch): + """Default verbosity is used when there is no value for DEVELOPER_DEBUG.""" monkeypatch.delenv("SNAPCRAFT_ENABLE_DEVELOPER_DEBUG", False) monkeypatch.setattr("sys.stdin.isatty", lambda: True) @@ -50,6 +55,8 @@ def test_no_env_returns_brief(monkeypatch): def test_sdtin_no_tty_returns_verbose(monkeypatch): + """Default verbosity for environments with a closed stdin is used when there is no + value for DEVELOPER_DEBUG.""" monkeypatch.delenv("SNAPCRAFT_ENABLE_DEVELOPER_DEBUG", False) monkeypatch.setattr("sys.stdin.isatty", lambda: False) @@ -57,7 +64,35 @@ def test_sdtin_no_tty_returns_verbose(monkeypatch): def test_developer_debug_and_sdtin_no_tty_returns_debug(monkeypatch): - monkeypatch.setenv("SNAPCRAFT_ENABLE_DEVELOPER_DEBUG", True) + """DEVELOPER_DEBUG sets verbosity in an environment with a closed stdin.""" + monkeypatch.setenv("SNAPCRAFT_ENABLE_DEVELOPER_DEBUG", "y") monkeypatch.setattr("sys.stdin.isatty", lambda: False) assert cli.get_verbosity() == cli.EmitterMode.DEBUG + + +@pytest.mark.parametrize("developer_debug", ["n", "y"]) +@pytest.mark.parametrize("isatty", [False, True]) +@pytest.mark.parametrize("verbosity", ["quiet", "brief", "verbose", "debug", "trace"]) +def test_env_var(monkeypatch, developer_debug, isatty, verbosity): + """Environment variable sets verbosity, even when DEVELOPER_DEBUG is defined or + stdin is closed.""" + monkeypatch.setenv("SNAPCRAFT_VERBOSITY_LEVEL", verbosity) + monkeypatch.setenv("SNAPCRAFT_ENABLE_DEVELOPER_DEBUG", developer_debug) + monkeypatch.setattr("sys.stdin.isatty", lambda: isatty) + + assert cli.get_verbosity() == cli.EmitterMode[verbosity.upper()] + + +def test_env_var_invalid(monkeypatch): + """Error is raised when environmental variable is invalid.""" + monkeypatch.setenv("SNAPCRAFT_VERBOSITY_LEVEL", "invalid") + + with pytest.raises(ArgumentParsingError) as raised: + cli.get_verbosity() + + assert str(raised.value) == ( + "cannot parse verbosity level 'invalid' from environment " + "variable SNAPCRAFT_VERBOSITY_LEVEL (valid values are 'quiet', 'brief', " + "'verbose', 'debug', and 'trace')" + ) diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index fcf63cb669..f82fae2c43 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -281,6 +281,21 @@ def test_humanize_list(items, conjunction, expected): assert utils.humanize_list(items, conjunction) == expected +def test_humanize_list_sorted(): + """Verify `sort` parameter.""" + input_list = ["z", "a", "m test", "1"] + + # unsorted list is in the same order as the original list + expected_list_unsorted = "'z', 'a', 'm test', and '1'" + + # sorted list is sorted alphanumerically + expected_list_sorted = "'1', 'a', 'm test', and 'z'" + + assert utils.humanize_list(input_list, "and") == expected_list_sorted + assert utils.humanize_list(input_list, "and", sort=True) == expected_list_sorted + assert utils.humanize_list(input_list, "and", sort=False) == expected_list_unsorted + + ################# # Library Paths # ################# From 3410375734c50731e491ca81eb496d5b13462c46 Mon Sep 17 00:00:00 2001 From: Callahan Date: Tue, 15 Nov 2022 17:58:51 -0600 Subject: [PATCH 5/5] repo: raise error when populating the apt cache directory fails (#3983) If the contents of /etc/apt cannot be copied to the cache directory, snapcraft will produce a helpful error. This can occur on a system where Ubuntu Pro is enabled, snapcraft is run in destructive mode, and snapcraft is not being run with sudo nor as root. Signed-off-by: Callahan Kovacs --- snapcraft_legacy/internal/repo/apt_cache.py | 17 +++++- snapcraft_legacy/internal/repo/errors.py | 23 +++++++- tests/legacy/unit/repo/test_apt_cache.py | 62 +++++++++++++++++++++ tests/legacy/unit/repo/test_errors.py | 30 ++++++++++ 4 files changed, 130 insertions(+), 2 deletions(-) diff --git a/snapcraft_legacy/internal/repo/apt_cache.py b/snapcraft_legacy/internal/repo/apt_cache.py index 11be8a58f5..f562034352 100644 --- a/snapcraft_legacy/internal/repo/apt_cache.py +++ b/snapcraft_legacy/internal/repo/apt_cache.py @@ -110,6 +110,7 @@ def _populate_stage_cache_dir(self) -> None: return # Copy apt configuration from host. + etc_apt_path = Path("/etc/apt") cache_etc_apt_path = Path(self.stage_cache, "etc", "apt") # Delete potentially outdated cache configuration. @@ -120,7 +121,21 @@ def _populate_stage_cache_dir(self) -> None: # Copy current cache configuration. cache_etc_apt_path.parent.mkdir(parents=True, exist_ok=True) - shutil.copytree("/etc/apt", cache_etc_apt_path) + + # systems with ubuntu pro have an auth file inside the /etc/apt directory. + # this auth file is readable only by root, so the copytree call below may + # fail when attempting to copy this file into the cache directory + try: + shutil.copytree(etc_apt_path, cache_etc_apt_path) + except shutil.Error as error: + # copytree is a multi-file operation, so it generates a list of exceptions + # each exception in the list is a 3-element tuple: (source, dest, reason) + raise errors.PopulateCacheDirError(error.args[0]) from error + except PermissionError as error: + # catch the PermissionError raised when `/etc/apt` is unreadable + raise errors.PopulateCacheDirError( + [(etc_apt_path, cache_etc_apt_path, error)] + ) from error # Specify default arch (if specified). if self.stage_cache_arch is not None: diff --git a/snapcraft_legacy/internal/repo/errors.py b/snapcraft_legacy/internal/repo/errors.py index b06d3943ce..19985097d0 100644 --- a/snapcraft_legacy/internal/repo/errors.py +++ b/snapcraft_legacy/internal/repo/errors.py @@ -15,7 +15,7 @@ # along with this program. If not, see . from pathlib import Path -from typing import List, Optional, Sequence +from typing import List, Optional, Sequence, Tuple from snapcraft_legacy import formatting_utils from snapcraft_legacy.internal import errors @@ -57,6 +57,27 @@ def __init__(self, errors: str) -> None: super().__init__(errors=errors) +class PopulateCacheDirError(SnapcraftException): + def __init__(self, copy_errors: List[Tuple]) -> None: + """:param copy_errors: A list of tuples containing the copy errors, where each + tuple is ordered as (source, destination, reason).""" + self.copy_errors = copy_errors + + def get_brief(self) -> str: + return "Could not populate apt cache directory." + + def get_details(self) -> str: + """Build a readable list of errors.""" + details = "" + for error in self.copy_errors: + source, dest, reason = error + details += f"Unable to copy {source} to {dest}: {reason}\n" + return details + + def get_resolution(self) -> str: + return "Verify user has read access to contents of /etc/apt." + + class FileProviderNotFound(RepoError): fmt = "{file_path} is not provided by any package." diff --git a/tests/legacy/unit/repo/test_apt_cache.py b/tests/legacy/unit/repo/test_apt_cache.py index 676154fa02..b2230c2bd1 100644 --- a/tests/legacy/unit/repo/test_apt_cache.py +++ b/tests/legacy/unit/repo/test_apt_cache.py @@ -15,14 +15,17 @@ # along with this program. If not, see . import os +import shutil import unittest from pathlib import Path from unittest.mock import call import fixtures +import pytest from testtools.matchers import Equals from snapcraft_legacy.internal.repo.apt_cache import AptCache +from snapcraft_legacy.internal.repo.errors import PopulateCacheDirError from tests.legacy import unit @@ -191,3 +194,62 @@ def test_host_get_installed_version(self): self.assertThat( apt_cache.get_installed_version("fake-news-bears"), Equals(None) ) + + +def test_populate_stage_cache_dir_shutil_error(mocker, tmp_path): + """Raise an error when the apt cache directory cannot be populated.""" + mock_copytree = mocker.patch( + "snapcraft_legacy.internal.repo.apt_cache.shutil.copytree", + side_effect=shutil.Error( + [ + ( + "/etc/apt/source-file-1", + "/root/.cache/dest-file-1", + "[Errno 13] Permission denied: '/etc/apt/source-file-1'", + ), + ( + "/etc/apt/source-file-2", + "/root/.cache/dest-file-2", + "[Errno 13] Permission denied: '/etc/apt/source-file-2'", + ), + ] + ), + ) + + with pytest.raises(PopulateCacheDirError) as raised: + with AptCache() as apt_cache: + # set stage_cache directory so method does not return early + apt_cache.stage_cache = tmp_path + apt_cache._populate_stage_cache_dir() + + assert mock_copytree.mock_calls == [call(Path("/etc/apt"), tmp_path / "etc/apt")] + + # verify the data inside the shutil error was passed to PopulateCacheDirError + assert raised.value.get_details() == ( + "Unable to copy /etc/apt/source-file-1 to /root/.cache/dest-file-1: " + "[Errno 13] Permission denied: '/etc/apt/source-file-1'\n" + "Unable to copy /etc/apt/source-file-2 to /root/.cache/dest-file-2: " + "[Errno 13] Permission denied: '/etc/apt/source-file-2'\n" + ) + + +def test_populate_stage_cache_dir_permission_error(mocker, tmp_path): + """Raise an error when the apt cache directory cannot be populated.""" + mock_copytree = mocker.patch( + "snapcraft_legacy.internal.repo.apt_cache.shutil.copytree", + side_effect=PermissionError("[Errno 13] Permission denied: '/etc/apt"), + ) + + with pytest.raises(PopulateCacheDirError) as raised: + with AptCache() as apt_cache: + # set stage_cache directory so method does not return early + apt_cache.stage_cache = tmp_path + apt_cache._populate_stage_cache_dir() + + assert mock_copytree.mock_calls == [call(Path("/etc/apt"), tmp_path / "etc/apt")] + + # verify the data inside the permission error was passed to PopulateCacheDirError + assert raised.value.get_details() == ( + f"Unable to copy {Path('/etc/apt')} to {tmp_path / 'etc/apt'}: " + "[Errno 13] Permission denied: '/etc/apt\n" + ) diff --git a/tests/legacy/unit/repo/test_errors.py b/tests/legacy/unit/repo/test_errors.py index dc336d66eb..6b088a88b3 100644 --- a/tests/legacy/unit/repo/test_errors.py +++ b/tests/legacy/unit/repo/test_errors.py @@ -181,3 +181,33 @@ def test_multiple_packages_not_found_error(): assert exception.get_details() is None assert exception.get_docs_url() is None assert exception.get_reportable() is False + + +def test_snapcraft_exception_handling(): + exception = errors.PopulateCacheDirError( + [ + ( + "/etc/apt/source-file-1", + "/root/.cache/dest-file-1", + "[Errno 13] Permission denied: '/etc/apt/source-file-1'", + ), + ( + "/etc/apt/source-file-2", + "/root/.cache/dest-file-2", + "[Errno 13] Permission denied: '/etc/apt/source-file-2'", + ), + ] + ) + + assert exception.get_brief() == "Could not populate apt cache directory." + assert exception.get_resolution() == ( + "Verify user has read access to contents of /etc/apt." + ) + assert exception.get_details() == ( + "Unable to copy /etc/apt/source-file-1 to /root/.cache/dest-file-1: " + "[Errno 13] Permission denied: '/etc/apt/source-file-1'\n" + "Unable to copy /etc/apt/source-file-2 to /root/.cache/dest-file-2: " + "[Errno 13] Permission denied: '/etc/apt/source-file-2'\n" + ) + assert exception.get_docs_url() is None + assert exception.get_reportable() is False