From 539d48d438c0bc389a43b6490d5f899f297d20b1 Mon Sep 17 00:00:00 2001 From: alexzorin Date: Mon, 12 Jun 2023 23:56:53 +1000 Subject: [PATCH 1/3] letstest: replace buster with bullseye (#9718) --- letstest/targets/targets.yaml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/letstest/targets/targets.yaml b/letstest/targets/targets.yaml index 24c711d79..502205b8b 100644 --- a/letstest/targets/targets.yaml +++ b/letstest/targets/targets.yaml @@ -18,13 +18,13 @@ targets: user: ubuntu #----------------------------------------------------------------------------- # Debian - - ami: ami-01db78123b2b99496 - name: debian10 + - ami: ami-0fec2c2e2017f4e7b + name: debian11 type: ubuntu virt: hvm user: admin - - ami: ami-0fec2c2e2017f4e7b - name: debian11 + - ami: ami-02ff60b9d37a0a0be + name: debian12 type: ubuntu virt: hvm user: admin From b1e5efac3cbc787234f9a52d2cd91802195e6ac1 Mon Sep 17 00:00:00 2001 From: alexzorin Date: Sat, 17 Jun 2023 01:26:15 +1000 Subject: [PATCH 2/3] disco: print the name of the plugin if it fails to load (#9719) --- certbot/certbot/_internal/plugins/disco.py | 10 ++++++++-- certbot/certbot/_internal/tests/plugins/disco_test.py | 11 +++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/certbot/certbot/_internal/plugins/disco.py b/certbot/certbot/_internal/plugins/disco.py index 5e767ae6d..2ee07f083 100644 --- a/certbot/certbot/_internal/plugins/disco.py +++ b/certbot/certbot/_internal/plugins/disco.py @@ -189,8 +189,14 @@ class PluginsRegistry(Mapping): pkg_resources.iter_entry_points( constants.OLD_SETUPTOOLS_PLUGINS_ENTRY_POINT),) for entry_point in entry_points: - cls._load_entry_point(entry_point, plugins) - + try: + cls._load_entry_point(entry_point, plugins) + except Exception as e: + raise errors.PluginError( + f"The '{entry_point.module_name}' plugin errored while loading: {e}. " + "You may need to remove or update this plugin. The Certbot log will " + "contain the full error details and this should be reported to the " + "plugin developer.") from e return cls(plugins) @classmethod diff --git a/certbot/certbot/_internal/tests/plugins/disco_test.py b/certbot/certbot/_internal/tests/plugins/disco_test.py index 0a0fd9826..97b3f2176 100644 --- a/certbot/certbot/_internal/tests/plugins/disco_test.py +++ b/certbot/certbot/_internal/tests/plugins/disco_test.py @@ -194,6 +194,17 @@ class PluginsRegistryTest(unittest.TestCase): assert plugins["ep1"].entry_point is self.ep1 assert "p1:ep1" not in plugins + def test_find_all_error_message(self): + from certbot._internal.plugins.disco import PluginsRegistry + with mock.patch("certbot._internal.plugins.disco.pkg_resources") as mock_pkg: + EP_SA.load = None # This triggers a TypeError when the entrypoint loads + mock_pkg.iter_entry_points.side_effect = [ + iter([EP_SA]), iter([EP_WR, self.ep1]) + ] + with self.assertRaises(errors.PluginError) as cm: + PluginsRegistry.find_all() + assert "standalone' plugin errored" in str(cm.exception) + def test_getitem(self): assert self.plugin_ep == self.reg["mock"] From 4fc4d536c1acc65646189c5b5aaf02aaa88c3f4a Mon Sep 17 00:00:00 2001 From: Leon G Date: Wed, 21 Jun 2023 16:57:50 +0200 Subject: [PATCH 3/3] Add LooseVersion class with risky comparison, deprecate parse_loose_version (#9646) * Replace parse_loose_version with LooseVersion * Fix LooseVersion docstring * Strengthen LooseVersion comparison * Update changelog --- AUTHORS.md | 1 + certbot/CHANGELOG.md | 2 +- certbot/certbot/_internal/tests/util_test.py | 47 +++++++++++ certbot/certbot/util.py | 89 +++++++++++++++++--- 4 files changed, 125 insertions(+), 14 deletions(-) diff --git a/AUTHORS.md b/AUTHORS.md index 539eed4a4..88060f42a 100644 --- a/AUTHORS.md +++ b/AUTHORS.md @@ -154,6 +154,7 @@ Authors * [LeCoyote](https://github.com/LeCoyote) * [Lee Watson](https://github.com/TheReverend403) * [Leo Famulari](https://github.com/lfam) +* [Leon G](https://github.com/LeonGr) * [lf](https://github.com/lf-) * [Liam Marshall](https://github.com/liamim) * [Lior Sabag](https://github.com/liorsbg) diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index 6a0aa553f..1e4cea6ce 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -6,7 +6,7 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). ### Added -* +* Add `certbot.util.LooseVersion` class. See [GH #9489](https://github.com/certbot/certbot/issues/9489). ### Changed diff --git a/certbot/certbot/_internal/tests/util_test.py b/certbot/certbot/_internal/tests/util_test.py index b4256176e..7ab451a39 100644 --- a/certbot/certbot/_internal/tests/util_test.py +++ b/certbot/certbot/_internal/tests/util_test.py @@ -625,6 +625,53 @@ class AtexitRegisterTest(unittest.TestCase): atexit_func(*args[1:], **kwargs) +class LooseVersionTest(unittest.TestCase): + """Test for certbot.util.LooseVersion. + + These tests are based on the original tests for + distutils.version.LooseVersion at + https://github.com/python/cpython/blob/v3.10.0/Lib/distutils/tests/test_version.py#L58-L81. + + """ + + @classmethod + def _call(cls, *args, **kwargs): + from certbot.util import LooseVersion + return LooseVersion(*args, **kwargs) + + def test_less_than(self): + comparisons = (('1.5.1', '1.5.2b2'), + ('3.4j', '1996.07.12'), + ('2g6', '11g'), + ('0.960923', '2.2beta29'), + ('1.13++', '5.5.kw'), + ('2.0', '2.0.1'), + ('a', 'b')) + for v1, v2 in comparisons: + assert self._call(v1).try_risky_comparison(self._call(v2)) == -1 + + def test_equal(self): + comparisons = (('8.02', '8.02'), + ('1a', '1a'), + ('2', '2.0.0'), + ('2.0', '2.0.0')) + for v1, v2 in comparisons: + assert self._call(v1).try_risky_comparison(self._call(v2)) == 0 + + def test_greater_than(self): + comparisons = (('161', '3.10a'), + ('3.2.pl0', '3.1.1.6')) + for v1, v2 in comparisons: + assert self._call(v1).try_risky_comparison(self._call(v2)) == 1 + + def test_incomparible(self): + comparisons = (('bookworm/sid', '9'), + ('1a', '1.0')) + for v1, v2 in comparisons: + with pytest.raises(ValueError): + assert self._call(v1).try_risky_comparison(self._call(v2)) + + class ParseLooseVersionTest(unittest.TestCase): """Test for certbot.util.parse_loose_version. diff --git a/certbot/certbot/util.py b/certbot/certbot/util.py index a477e972d..f75a989e1 100644 --- a/certbot/certbot/util.py +++ b/certbot/certbot/util.py @@ -2,6 +2,7 @@ import argparse import atexit import errno +import itertools import logging import platform import re @@ -48,6 +49,79 @@ class CSR(NamedTuple): form: str +class LooseVersion: + """A version with loose rules, i.e. any given string is a valid version number. + + but regular comparison is not supported. Instead, the `try_risky_comparison` method is + provided, which may return an error if two LooseVersions are 'incomparible'. + For example when integer and string version components are present in the same position. + + Differences with old distutils.version.LooseVersion: + (https://github.com/python/cpython/blob/v3.10.0/Lib/distutils/version.py#L269) + Most version comparisons should give the same result. However, if a version has multiple + trailing zeroes, not all of them are used in the comparison. This ensure that, for example, + "2.0" and "2.0.0" are equal. + """ + + def __init__(self, version_string: str) -> None: + """Parses a version string into its components. + + :param str version_string: version string + """ + components: List[Union[int, str]] + components = [x for x in _VERSION_COMPONENT_RE.split(version_string) + if x and x != '.'] + for i, obj in enumerate(components): + try: + components[i] = int(obj) + except ValueError: + pass + + self.version_components = components + + def try_risky_comparison(self, other: 'LooseVersion') -> int: + """Compares the LooseVersion to another value. + + If the other value is another LooseVersion, the version components are compared. Otherwise, + an exception is raised. + + Comparison is performed element-wise. If the version components being compared are of + different types, the two versions are considered incomparible. Otherwise, if either of the + components is not equal to the other, less or greater is returned based on the comparison's + result. In case the two versions are of different lengths, some elements in the longer + version have not yet been compared. If these are all equal to zero, the two versions are + equal. Otherwise, the longer version is greater. + + If the two versions are incomparible, an exception is raised. Otherwise, the returned + integer indicates the result of the comparison. If self == other, 0 is returned. + If self > other, 1 is returned. If self < other -1 is returned. + + Examples: + Equality: + - LooseVersion('1.0').try_risky_comparison(LooseVersion('1.0')) -> 0 + - LooseVersion('2.0.0a').try_risky_comparison(LooseVersion('2.0.0a')) -> 0 + Inequality: + - LooseVersion('2.0.0').try_risky_comparison(LooseVersion('1.0')) -> 1 + - LooseVersion('1.0.1').try_risky_comparison(LooseVersion('2.0a')) -> -1 + Incomparability: + - LooseVersion('1a').try_risky_comparison(LooseVersion('1.0')) -> ValueError + """ + try: + for self_vc, other_vc in itertools.zip_longest(self.version_components, + other.version_components, + fillvalue=0): + # ensure mypy ignores types here and catch any TypeErrors + if self_vc < other_vc: # type: ignore + return -1 + elif self_vc > other_vc: # type: ignore + return 1 + return 0 + except TypeError: + raise ValueError("Cannot meaningfully compare LooseVersion {} with LooseVersion {} " + "due to comparison of version components with different types." + .format(self.version_components, other.version_components)) + + # ANSI SGR escape codes # Formats text as bold or with increased intensity ANSI_SGR_BOLD = '\033[1m' @@ -640,28 +714,17 @@ def atexit_register(func: Callable, *args: Any, **kwargs: Any) -> None: def parse_loose_version(version_string: str) -> List[Union[int, str]]: """Parses a version string into its components. - This code and the returned tuple is based on the now deprecated distutils.version.LooseVersion class from the Python standard library. Two LooseVersion classes and two lists as returned by this function should compare in the same way. See https://github.com/python/cpython/blob/v3.10.0/Lib/distutils/version.py#L205-L347. - :param str version_string: version string - :returns: list of parsed version string components :rtype: list - """ - components: List[Union[int, str]] - components = [x for x in _VERSION_COMPONENT_RE.split(version_string) - if x and x != '.'] - for i, obj in enumerate(components): - try: - components[i] = int(obj) - except ValueError: - pass - return components + loose_version = LooseVersion(version_string) + return loose_version.version_components def _atexit_call(func: Callable, *args: Any, **kwargs: Any) -> None: