diff --git a/keystoneauth1/discover.py b/keystoneauth1/discover.py index fb0c5aac..d0b0f81a 100644 --- a/keystoneauth1/discover.py +++ b/keystoneauth1/discover.py @@ -276,42 +276,34 @@ def version_to_string(version): def _version_between(min_version, max_version, candidate): """Determine whether a candidate version is within a specified range. - :param min_version: Normalized lower bound. May be None. May be - (LATEST, LATEST). - :param max_version: Normalized upper bound. May be None. May be - (LATEST, LATEST). - :param candidate: Normalized candidate version to test. May not be None. + :param min_version: The minimum version that is acceptable. + None/empty indicates no lower bound. + :param max_version: The maximum version that is acceptable. + None/empty indicates no upper bound. + :param candidate: Candidate version to test. May not be None/empty. :return: True if candidate is between min_version and max_version; False otherwise. - :raises ValueError: If candidate is None or the input is not properly - normalized. + :raises ValueError: If candidate is None. + :raises TypeError: If any input cannot be normalized. """ - def is_normalized(ver): - return normalize_version_number(ver) == ver + if not candidate: + raise ValueError("candidate is required.") + candidate = normalize_version_number(candidate) - # A version can't be between a range that doesn't exist - if not min_version and not max_version: + # Normalize up front to validate any malformed inputs + if min_version: + min_version = normalize_version_number(min_version) + if max_version: + max_version = normalize_version_number(max_version) + + # If the candidate is less than the min_version, it's not a match. + # No min_version means no lower bound. + if min_version and candidate < min_version: return False - if candidate is None: - raise ValueError("candidate cannot be None.") - - if min_version is not None and not is_normalized(min_version): - raise ValueError("min_version is not normalized.") - if max_version is not None and not is_normalized(max_version): - raise ValueError("max_version is not normalized.") - if not is_normalized(candidate): - raise ValueError("candidate is not normalized.") - # This is only possible if args weren't run through _normalize_version_args - if max_version is None and min_version is not None: - raise ValueError("Can't use None as an upper bound.") - - # If the candidate is less than the min_version, it's - # not a match. None works here. - if min_version is not None and candidate < min_version: - return False - - if max_version is not None and candidate > max_version: + # If the candidate is higher than the max_version, it's not a match. + # No max_version means no upper bound. + if max_version and candidate > max_version: return False return True @@ -633,7 +625,9 @@ class Discover(object): return data if _latest_soft_match(min_version, data['version']): return data - if _version_between(min_version, max_version, data['version']): + # Only validate version bounds if versions were specified + if min_version and max_version and _version_between( + min_version, max_version, data['version']): return data # If there is no version requested and we could not find a matching @@ -1019,8 +1013,10 @@ class EndpointData(object): except TypeError: pass else: - is_between = _version_between(min_version, max_version, - url_version) + # `is_between` means version bounds were specified *and* the URL + # version is between them. + is_between = min_version and max_version and _version_between( + min_version, max_version, url_version) exact_match = (is_between and max_version and max_version[0] == url_version[0]) high_match = (is_between and max_version and diff --git a/keystoneauth1/tests/unit/test_discovery.py b/keystoneauth1/tests/unit/test_discovery.py index ec5a15c4..95e50f0b 100644 --- a/keystoneauth1/tests/unit/test_discovery.py +++ b/keystoneauth1/tests/unit/test_discovery.py @@ -386,30 +386,61 @@ class DiscoverUtils(utils.TestCase): def bad(minver, maxver, cand): self.assertFalse(discover._version_between(minver, maxver, cand)) - def exc(minver, maxver, cand): - self.assertRaises(ValueError, + def exc(excls, minver, maxver, cand): + self.assertRaises(excls, discover._version_between, minver, maxver, cand) + # candidate required + exc(ValueError, (1, 0), (1, 0), None) + exc(ValueError, 'v1.0', '1.0', '') + # malformed candidate + exc(TypeError, None, None, 'bogus') + exc(TypeError, None, None, (1, 'two')) + # malformed min_version + exc(TypeError, 'bogus', None, (1, 0)) + exc(TypeError, (1, 'two'), None, (1, 0)) + # malformed max_version + exc(TypeError, None, 'bogus', (1, 0)) + exc(TypeError, None, (1, 'two'), (1, 0)) + + # fail on minimum + bad((2, 4), None, (1, 55)) + bad('v2.4', '', '2.3') + bad('latest', None, (2, 3000)) + bad((2, discover.LATEST), '', 'v2.3000') + bad((2, 3000), '', (1, discover.LATEST)) + bad('latest', None, 'v1000.latest') + # fail on maximum + bad(None, (2, 4), (2, 5)) + bad('', 'v2.4', '2.5') + bad(None, (2, discover.LATEST), (3, 0)) + bad('', '2000.latest', 'latest') + + # candidate matches a bound good((1, 0), (1, 0), (1, 0)) + good('1.0', '2.9', '1.0') + good('v1.0', 'v2.9', 'v2.9') + # properly in between good((1, 0), (1, 10), (1, 2)) - good(None, (1, 10), (1, 2)) - good((1, 20), (2, 0), (1, 21)) - good((1, 0), (2, discover.LATEST), (1, 21)) - good((1, 0), (2, discover.LATEST), (1, discover.LATEST)) - good((1, 50), (2, discover.LATEST), (2, discover.LATEST)) - - bad((discover.LATEST, discover.LATEST), - (discover.LATEST, discover.LATEST), (1, 0)) - bad(None, None, (1, 0)) - bad((1, 50), (2, discover.LATEST), (3, 0)) - bad((1, 50), (2, discover.LATEST), (3, discover.LATEST)) - bad((1, 50), (2, 5), (2, discover.LATEST)) - - exc((1, 0), (1, 0), None) - exc('v1.0', (1, 0), (1, 0)) - exc((1, 0), 'v1.0', (1, 0)) - exc((1, 0), (1, 0), 'v1.0') - exc((1, 0), None, (1, 0)) + good('1', '2', '1.2') + # no lower bound + good(None, (2, 5), (2, 3)) + # no upper bound + good('2.5', '', '2.6') + # no bounds at all + good('', '', 'v1') + good(None, None, (999, 999)) + good(None, None, 'latest') + # Various good 'latest' scenarios + good((discover.LATEST, discover.LATEST), + (discover.LATEST, discover.LATEST), + (discover.LATEST, discover.LATEST)) + good((discover.LATEST, discover.LATEST), None, + (discover.LATEST, discover.LATEST)) + good('', 'latest', 'latest') + good('2.latest', '3.latest', '3.0') + good('2.latest', None, (55, 66)) + good(None, '3.latest', '3.9999') class VersionDataTests(utils.TestCase):