diff --git a/lib/charms/operator_libs_linux/v0/apt.py b/lib/charms/operator_libs_linux/v0/apt.py index 4d25805..f36cc09 100644 --- a/lib/charms/operator_libs_linux/v0/apt.py +++ b/lib/charms/operator_libs_linux/v0/apt.py @@ -924,6 +924,35 @@ class InvalidSourceError(Error): """Exceptions for invalid source entries.""" +class MissingRequiredKeyError(InvalidSourceError): + """Missing a required value in a source file.""" + def __init__( + self, message: str = "", *, file: str, line: Optional[int], key: str + ) -> None: + super().__init__(message, file, line, key) + self.file = file + self.line = line + self.key = key + + +class BadValueError(InvalidSourceError): + """Bad value for an entry in a source file.""" + def __init__( + self, + message: str = "", + *, + file: str, + line: Optional[int], + key: str, + value: str, + ) -> None: + super().__init__(message, file, line, key, value) + self.file = file + self.line = line + self.key = key + self.value = value + + class GPGKeyError(Error): """Exceptions for GPG keys.""" @@ -1243,6 +1272,7 @@ class RepositoryMapping(Mapping[str, DebianRepository]): _sources_subdir = "sources.list.d" _default_list_name = "sources.list" _default_sources_name = "ubuntu.sources" + _last_errors: Iterable[Error] = () def __init__(self): self._repository_map: Dict[str, DebianRepository] = {} @@ -1316,6 +1346,7 @@ def load_deb822(self, filename: str) -> None: self._repository_map[identifier] = repo if errors: + self._last_errors = errors logger.debug( "the following %d error(s) were encountered when reading deb822 sources:\n%s", len(errors), @@ -1657,16 +1688,12 @@ def _deb822_options_to_repos( elif enabled_field == "no": enabled = False else: - raise InvalidSourceError( - ( - "Bad value '{value}' for entry 'Enabled' (line {enabled_line})" - " in file {file}. If 'Enabled' is present it must be one of" - " yes or no (if absent it defaults to yes)." - ).format( - value=enabled_field, - enabled_line=line_numbers.get("Enabled"), - file=filename, - ) + raise BadValueError( + "Must be one of yes or no (default: yes).", + file=filename, + line=line_numbers.get("Enabled"), + key="Enabled", + value=enabled_field, ) # Signed-By gpg_key_file = options.pop("Signed-By", "") @@ -1682,44 +1709,49 @@ def _deb822_options_to_repos( suites = options.pop("Suites").split() except KeyError as e: [key] = e.args - raise InvalidSourceError( - "Missing required entry '{key}' for entry starting on line {line} in {file}.".format( - key=key, - line=min(line_numbers.values()) if line_numbers else None, - file=filename, - ) + raise MissingRequiredKeyError( + key=key, + line=min(line_numbers.values()) if line_numbers else None, + file=filename, ) # Components + # suite can specify an exact path, in which case the components must be omitted and suite must end with a slash (/). + # If suite does not specify an exact path, at least one component must be present. + # https://manpages.ubuntu.com/manpages/noble/man5/sources.list.5.html components: List[str] if len(suites) == 1 and suites[0].endswith("/"): if "Components" in options: - raise InvalidSourceError( - ( - "Since 'Suites' (line {suites_line}) specifies" - " a path relative to 'URIs' (line {uris_line})," - " 'Components' (line {components_line}) must be ommitted" - " (in file {file})." - ).format( - suites_line=line_numbers.get("Suites"), - uris_line=line_numbers.get("URIs"), - components_line=line_numbers.get("Components"), - file=filename, - ) + msg = ( + "Since 'Suites' (line {suites_line}) specifies" + " a path relative to 'URIs' (line {uris_line})," + " 'Components' must be ommitted." + ).format( + suites_line=line_numbers.get("Suites"), + uris_line=line_numbers.get("URIs"), + ) + raise BadValueError( + msg, + file=filename, + line=line_numbers.get("Components"), + key="Components", + value=options["Components"], ) components = [] else: if "Components" not in options: - raise InvalidSourceError( - ( - "Since 'Suites' (line {suites_line}) does not specify" - " a path relative to 'URIs' (line {uris_line})," - " 'Components' must be present in this paragraph" - " (in file {file})." - ).format( - suites_line=line_numbers.get("Suites"), - uris_line=line_numbers.get("URIs"), - file=filename, - ) + msg = ( + "Since 'Suites' (line {suites_line}) does not specify" + " a path relative to 'URIs' (line {uris_line})," + " 'Components' must be present in this paragraph." + ).format( + suites_line=line_numbers.get("Suites"), + uris_line=line_numbers.get("URIs"), + ) + raise MissingRequiredKeyError( + msg, + file=filename, + line=min(line_numbers.values()) if line_numbers else None, + key="Components", ) components = options.pop("Components").split() repos = tuple( diff --git a/tests/unit/data/fake-apt-dirs/noble-with-comments-etc/sources.list b/tests/unit/data/fake-apt-dirs/noble-with-comments-etc/sources.list deleted file mode 100644 index eb39b94..0000000 --- a/tests/unit/data/fake-apt-dirs/noble-with-comments-etc/sources.list +++ /dev/null @@ -1 +0,0 @@ -# Ubuntu sources have moved to /etc/apt/sources.list.d/ubuntu.sources diff --git a/tests/unit/data/fake-apt-dirs/noble-with-comments-etc/sources.list.d/ubuntu.sources b/tests/unit/data/fake-apt-dirs/noble-with-comments-etc/sources.list.d/ubuntu.sources index eff1961..87a867e 100644 --- a/tests/unit/data/fake-apt-dirs/noble-with-comments-etc/sources.list.d/ubuntu.sources +++ b/tests/unit/data/fake-apt-dirs/noble-with-comments-etc/sources.list.d/ubuntu.sources @@ -30,4 +30,16 @@ Components: main # this is an error, can't have components with an exact path Types: deb URIs: http://nz.archive.ubuntu.com/ubuntu/ Suites: noble noble-updates noble-backports -# this is an error, must have at least one component if suites isn't an exact path \ No newline at end of file +# this is an error, must have at least one component if suites isn't an exact path + +Types: deb +URIs: http://nz.archive.ubuntu.com/ubuntu/ +Suites: noble noble-updates noble-backports +Components: main restricted universe multiverse +Enabled: no + +Types: deb +URIs: http://nz.archive.ubuntu.com/ubuntu/ +Suites: noble noble-updates noble-backports +Components: main restricted universe multiverse +Enabled: bad \ No newline at end of file diff --git a/tests/unit/test_deb822.py b/tests/unit/test_deb822.py index 688634f..3ee1633 100644 --- a/tests/unit/test_deb822.py +++ b/tests/unit/test_deb822.py @@ -242,32 +242,48 @@ def test_load_deb822_ubuntu_sources(self): ] def test_load_deb822_w_comments(self): - def isnt_file(f: str) -> bool: - return False - - def iglob_nothing(s: str) -> typing.Iterable[str]: - return [] - - with patch("os.path.isfile", new=isnt_file): - with patch("glob.iglob", new=iglob_nothing): - repository_mapping = apt.RepositoryMapping() - assert not repository_mapping._repository_map - - with patch( - "builtins.open", new_callable=mock_open, read_data=ubuntu_sources_deb822_with_comments + with patch.object( + apt.RepositoryMapping, + "_apt_dir", + str(FAKE_APT_DIRS / "noble-with-comments-etc"), ): - with patch.object(apt.logger, "debug") as debug: - repository_mapping.load_deb822("FILENAME") + repository_mapping = apt.RepositoryMapping() + # TODO: split cases into separate files and test load_deb822 instead + # this will make things a lot more understandable and maintainable + assert sorted(repository_mapping._repository_map.keys()) == [ + "deb-http://archive.ubuntu.com/ubuntu/-noble", + "deb-http://archive.ubuntu.com/ubuntu/-noble-backports", + "deb-http://archive.ubuntu.com/ubuntu/-noble-updates", + "deb-http://nz.archive.ubuntu.com/ubuntu/-an/exact/path/", "deb-http://nz.archive.ubuntu.com/ubuntu/-noble", "deb-http://nz.archive.ubuntu.com/ubuntu/-noble-backports", "deb-http://nz.archive.ubuntu.com/ubuntu/-noble-updates", + "deb-src-http://archive.ubuntu.com/ubuntu/-noble", + "deb-src-http://archive.ubuntu.com/ubuntu/-noble-backports", + "deb-src-http://archive.ubuntu.com/ubuntu/-noble-updates", + "deb-src-http://nz.archive.ubuntu.com/ubuntu/-noble", + "deb-src-http://nz.archive.ubuntu.com/ubuntu/-noble-backports", + "deb-src-http://nz.archive.ubuntu.com/ubuntu/-noble-updates", ] - debug.assert_called_once_with( - ANY, - 1, # number of errors - "Missing required entry 'Types' for entry starting on line 12 in FILENAME.", - ) + errors = tuple(repository_mapping._last_errors) + assert len(errors) == 4 + ( + missing_types, + components_not_ommitted, + components_not_present, + bad_enabled_value, + ) = errors + assert isinstance(missing_types, apt.MissingRequiredKeyError) + assert missing_types.key == "Types" + assert isinstance(components_not_ommitted, apt.BadValueError) + assert components_not_ommitted.key == "Components" + assert components_not_ommitted.value == "main" + assert isinstance(components_not_present, apt.MissingRequiredKeyError) + assert components_not_present.key == "Components" + assert isinstance(bad_enabled_value, apt.BadValueError) + assert bad_enabled_value.key == "Enabled" + assert bad_enabled_value.value == "bad" def test_init_with_deb822(self): """Mock file opening to initialise a RepositoryMapping from deb822 and one-line-style. @@ -307,15 +323,12 @@ def test_init_with_deb822(self): ) def test_disable_with_deb822(self): - def isnt_file(f: str) -> bool: - return False - - def iglob_nothing(s: str) -> typing.Iterable[str]: - return [] - - with patch("os.path.isfile", new=isnt_file): - with patch("glob.iglob", new=iglob_nothing): - repository_mapping = apt.RepositoryMapping() + with patch.object( + apt.RepositoryMapping, + "_apt_dir", + str(FAKE_APT_DIRS / "empty"), + ): + repository_mapping = apt.RepositoryMapping() repo = apt.DebianRepository( enabled=True, repotype="deb",