From 55c494aea326ac00453c4397139b0471d21c7aef Mon Sep 17 00:00:00 2001 From: ethanholen-hpe Date: Wed, 13 Nov 2024 08:49:46 -0700 Subject: [PATCH] CRAYSAT-1551: Fix sorting of product versions in sat showrev --products --- .gitignore | 1 + CHANGELOG.md | 16 +++-- sat/cli/showrev/products.py | 3 +- sat/loose_version.py | 98 ++++++++++++++++++++++++++++++ sat/util.py | 13 +++- tests/cli/showrev/test_products.py | 28 +++++---- tests/test_loose_version.py | 91 +++++++++++++++++++++++++++ 7 files changed, 230 insertions(+), 20 deletions(-) create mode 100644 sat/loose_version.py create mode 100644 tests/test_loose_version.py diff --git a/.gitignore b/.gitignore index 402c75a2..fe3aba22 100644 --- a/.gitignore +++ b/.gitignore @@ -17,3 +17,4 @@ venv nosetests.xml .vscode/ +.idea diff --git a/CHANGELOG.md b/CHANGELOG.md index 5bca92f7..1ceb3ed2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [3.32.12] - 2024-11-21 + +### Fixed +- Fix sorting by product version in `sat showrev --products` to sort by semantic + version rather than a simple lexicographic order. + ## [3.32.11] - 2024-11-12 ### Fixed @@ -65,7 +71,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [3.32.4] - 2024-09-27 ### Added -- Add jinja rendering of rootfs_provider_passthrough value for the boot_set to create session +- Add jinja rendering of rootfs_provider_passthrough value for the boot_set to create session template with iSCSI values. ## [3.32.3] - 2024-09-27 @@ -101,7 +107,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [3.31.0] - 2024-08-21 -### Fixed +### Fixed - Updating the cray-product-catalog & python-csm-api-client to latest versions. ## [3.30.2] - 2024-08-14 @@ -326,7 +332,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [3.27.11] - 2024-02-28 ### Fixed -- Fixed for `sat showrev` to stop supporting `--release-files` option +- Fixed for `sat showrev` to stop supporting `--release-files` option and log a warning message indicating that this option is no longer supported. ## [3.27.10] - 2024-02-26 @@ -357,7 +363,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [3.27.6] - 2024-02-16 ### Fixed -- Remove unnecessary queries to BOS to get the name of the session template for +- Remove unnecessary queries to BOS to get the name of the session template for every single node component in the output of `sat status`. ## [3.27.5] - 2024-02-07 @@ -371,7 +377,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Security - Update the version of jinja2 from 3.0.3 to 3.1.3 to address CVE-2024-22195 - + ## [3.27.3] - 2024-01-31 ### Fixed diff --git a/sat/cli/showrev/products.py b/sat/cli/showrev/products.py index 96d1bf76..bc1b69a6 100644 --- a/sat/cli/showrev/products.py +++ b/sat/cli/showrev/products.py @@ -27,6 +27,7 @@ import logging from cray_product_catalog.query import ProductCatalog, ProductCatalogError +from sat.loose_version import LooseVersion LOGGER = logging.getLogger(__name__) @@ -66,6 +67,6 @@ def get_product_versions(): for product in product_catalog.products: images = '\n'.join(sorted(image['name'] for image in product.images)) or '-' recipes = '\n'.join(sorted(recipe['name'] for recipe in product.recipes)) or '-' - products.append([product.name, product.version, images, recipes]) + products.append([product.name, LooseVersion(product.version), images, recipes]) return headers, products diff --git a/sat/loose_version.py b/sat/loose_version.py new file mode 100644 index 00000000..f4f57a86 --- /dev/null +++ b/sat/loose_version.py @@ -0,0 +1,98 @@ +# +# MIT License +# +# (C) Copyright 2024 Hewlett Packard Enterprise Development LP +# +# Permission is hereby granted, free of charge, to any person obtaining a +# copy of this software and associated documentation files (the "Software"), +# to deal in the Software without restriction, including without limitation +# the rights to use, copy, modify, merge, publish, distribute, sublicense, +# and/or sell copies of the Software, and to permit persons to whom the +# Software is furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included +# in all copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL +# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR +# OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, +# ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR +# OTHER DEALINGS IN THE SOFTWARE. +# +""" +Class for representing a semver version +""" +from semver import Version + + +class LooseVersion: + """A LooseVersion representing a version that may be semver or may not. + + This class is used to compare versions that may or may not comply with the + semver format. If the version does not comply with the semver format, then + the MAX_VERSION is used as the comparison version, which results in the + version being considered greater than any other version. + + Args: + version_str (str): The string representation of the version. + + Attributes: + version_str (str): The string representation of the version. + comparison_version (semver.version.Version): The semver.version.Version + object of either the version_str or the MAX_VERSION if semver fails to + parse the version + """ + + MAX_VERSION = "99999999999.99999999999.99999999999" + + def __init__(self, version_str): + """Creates a new LooseVersion object from the given product_version string. + + Args: + version_str (str): The string representation of the version. + """ + self.version_str = version_str + self.comparison_version = self.parse_version(version_str) + + def __str__(self): + return f'{self.version_str}' + + def __repr__(self): + return f"{self.__class__.__name__}('{self.version_str}')" + + def __lt__(self, other): + return self.comparison_version < other.comparison_version + + def __le__(self, other): + return self.comparison_version <= other.comparison_version + + def __eq__(self, other): + return (isinstance(self, type(other)) and + self.comparison_version == other.comparison_version) + + def __gt__(self, other): + return self.comparison_version > other.comparison_version + + def __ge__(self, other): + return self.comparison_version >= other.comparison_version + + def parse_version(self, version_str): + """Parse the version string into a semver.version.Version object if possible. + + Args: + version_str (str): The string representation of the version. + + Returns: + semver.version.Version: The semver.version.Version object of either + the version_str or the MAX_VERSION if semver fails to parse the + version + """ + + try: + parsed_version = Version.parse(version_str) + except ValueError: + parsed_version = Version.parse(self.MAX_VERSION) + + return parsed_version diff --git a/sat/util.py b/sat/util.py index 80ecb9f1..408d3de1 100644 --- a/sat/util.py +++ b/sat/util.py @@ -1,7 +1,7 @@ # # MIT License # -# (C) Copyright 2019-2023 Hewlett Packard Enterprise Development LP +# (C) Copyright 2019-2024 Hewlett Packard Enterprise Development LP # # Permission is hereby granted, free of charge, to any person obtaining a # copy of this software and associated documentation files (the "Software"), @@ -37,6 +37,7 @@ import re import time + # Logic borrowed from imps to get the most efficient YAML available try: from yaml import CSafeDumper as SafeDumper @@ -49,6 +50,7 @@ from prettytable import PrettyTable from sat.xname import XName +from sat.loose_version import LooseVersion from sat.config import get_config_value, read_config_value_file @@ -348,8 +350,15 @@ def _xname_representer(dumper, xname): ) +def _looseversion_representer(dumper, looseversion): + return dumper.represent_scalar( + BaseResolver.DEFAULT_SCALAR_TAG, looseversion.version_str + ) + + SATDumper.add_representer(OrderedDict, _ordered_dict_representer) SATDumper.add_representer(XName, _xname_representer) +SATDumper.add_representer(LooseVersion, _looseversion_representer) # A function to dump YAML to be used by all SAT code. @@ -369,6 +378,8 @@ def default(self, o): """ if isinstance(o, XName): return str(o) + if isinstance(o, LooseVersion): + return o.version_str return JSONEncoder.default(self, o) diff --git a/tests/cli/showrev/test_products.py b/tests/cli/showrev/test_products.py index b95d70df..3fbefc5a 100644 --- a/tests/cli/showrev/test_products.py +++ b/tests/cli/showrev/test_products.py @@ -33,6 +33,7 @@ from cray_product_catalog.query import InstalledProductVersion, ProductCatalogError from sat.cli.showrev.products import get_product_versions +from sat.loose_version import LooseVersion from tests.test_util import ExtendedTestCase SAMPLES_DIR = os.path.join(os.path.dirname(__file__), 'samples') @@ -90,9 +91,9 @@ def tearDown(self): def test_get_product_versions(self): """Test a basic invocation of get_product_versions.""" expected_fields = [ - ['cos', '1.4.0', COS_IMAGE_NAME, COS_RECIPE_NAME], - ['uan', '2.0.0', UAN_IMAGE_NAME, UAN_RECIPE_NAME], - ['pbs', '0.1.0', '-', '-'] + ['cos', LooseVersion('1.4.0'), COS_IMAGE_NAME, COS_RECIPE_NAME], + ['uan', LooseVersion('2.0.0'), UAN_IMAGE_NAME, UAN_RECIPE_NAME], + ['pbs', LooseVersion('0.1.0'), '-', '-'] ] actual_headers, actual_fields = get_product_versions() self.mock_product_catalog_cls.assert_called_once_with() @@ -108,10 +109,10 @@ def test_get_product_versions_multiple_versions(self): self.mock_product_catalog.products.insert(2, new_uan_product) expected_fields = [ - ['cos', '1.4.0', COS_IMAGE_NAME, COS_RECIPE_NAME], - ['uan', '2.0.0', UAN_IMAGE_NAME, UAN_RECIPE_NAME], - ['uan', '2.0.1', new_uan_image_name, new_uan_recipe_name], - ['pbs', '0.1.0', '-', '-'] + ['cos', LooseVersion('1.4.0'), COS_IMAGE_NAME, COS_RECIPE_NAME], + ['uan', LooseVersion('2.0.0'), UAN_IMAGE_NAME, UAN_RECIPE_NAME], + ['uan', LooseVersion('2.0.1'), new_uan_image_name, new_uan_recipe_name], + ['pbs', LooseVersion('0.1.0'), '-', '-'] ] actual_headers, actual_fields = get_product_versions() self.mock_product_catalog_cls.assert_called_once_with() @@ -125,9 +126,10 @@ def test_get_product_versions_multiple_images_recipes(self): self.mock_uan_product.recipes.append(get_fake_ims_data(other_uan_recipe)) expected_fields = [ - ['cos', '1.4.0', COS_IMAGE_NAME, COS_RECIPE_NAME], - ['uan', '2.0.0', f'{other_uan_image}\n{UAN_IMAGE_NAME}', f'{other_uan_recipe}\n{UAN_RECIPE_NAME}'], - ['pbs', '0.1.0', '-', '-'] + ['cos', LooseVersion('1.4.0'), COS_IMAGE_NAME, COS_RECIPE_NAME], + ['uan', LooseVersion('2.0.0'), f'{other_uan_image}\n{UAN_IMAGE_NAME}', + f'{other_uan_recipe}\n{UAN_RECIPE_NAME}'], + ['pbs', LooseVersion('0.1.0'), '-', '-'] ] actual_headers, actual_fields = get_product_versions() self.mock_product_catalog_cls.assert_called_once_with() @@ -150,9 +152,9 @@ def test_get_product_versions_active_version(self): self.mock_uan_product.supports_active = True self.mock_uan_product.active = False expected_fields = [ - ['cos', '1.4.0', COS_IMAGE_NAME, COS_RECIPE_NAME], - ['uan', '2.0.0', UAN_IMAGE_NAME, UAN_RECIPE_NAME], - ['pbs', '0.1.0', '-', '-'] + ['cos', LooseVersion('1.4.0'), COS_IMAGE_NAME, COS_RECIPE_NAME], + ['uan', LooseVersion('2.0.0'), UAN_IMAGE_NAME, UAN_RECIPE_NAME], + ['pbs', LooseVersion('0.1.0'), '-', '-'] ] actual_headers, actual_fields = get_product_versions() self.mock_product_catalog_cls.assert_called_once_with() diff --git a/tests/test_loose_version.py b/tests/test_loose_version.py new file mode 100644 index 00000000..db42ebd6 --- /dev/null +++ b/tests/test_loose_version.py @@ -0,0 +1,91 @@ +# +# MIT License +# +# (C) Copyright 2024 Hewlett Packard Enterprise Development LP +# +# Permission is hereby granted, free of charge, to any person obtaining a +# copy of this software and associated documentation files (the "Software"), +# to deal in the Software without restriction, including without limitation +# the rights to use, copy, modify, merge, publish, distribute, sublicense, +# and/or sell copies of the Software, and to permit persons to whom the +# Software is furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included +# in all copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL +# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR +# OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, +# ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR +# OTHER DEALINGS IN THE SOFTWARE. +# +""" +Tests for the LooseVersion utility class. +""" + +import unittest + +from sat.loose_version import LooseVersion + + +class TestLooseVersion(unittest.TestCase): + """Tests for the XName class""" + + def setUp(self): + self.examples = [ + "1.0.0", + "1.0.0", + "1.0.2", + "2.1.3", + "20240129105721.3d243a4b5120", + "1.9.16-20240522115029.beaa1ce7a544", + "1.9.18-20240625143747.59e8b16343aa", + "2.1.1-64-cos-3.0-aarch64", + "23.10.30-20231027", + "2.2.0-57-cos-3.0-x86-64" + ] + + def test_version_str(self): + """Test version_str property stores unmodified version string""" + for version_str in self.examples: + self.assertEqual(LooseVersion(version_str).version_str, version_str) + + def test_bad_version_str(self): + """Test that bad version string evaluates to max value""" + bad_version_str = "1" + good_version_str = "2.0.0" + self.assertLess(LooseVersion(good_version_str), LooseVersion(bad_version_str)) + + def test_eq(self): + """Test __eq__ for LooseVersion.""" + for version_str in self.examples: + self.assertEqual(LooseVersion(version_str), LooseVersion(version_str)) + + def test_lt(self): + """Test __lt__ for LooseVersion.""" + self.assertLess(LooseVersion('1.0.0'), LooseVersion('1.0.1')) + + def test_le(self): + """Test __le__ for LooseVersion.""" + self.assertLessEqual(LooseVersion('1.0.0'), LooseVersion('1.0.1')) + self.assertLessEqual(LooseVersion('1.0.1'), LooseVersion('1.0.1')) + + def test_gt(self): + """Test __gt__ for LooseVersion.""" + self.assertGreater(LooseVersion('1.0.1'), LooseVersion('1.0.0')) + + def test_ge(self): + """Test __ge__ for LooseVersion.""" + self.assertGreaterEqual(LooseVersion('1.0.0'), LooseVersion('1.0.0')) + self.assertGreaterEqual(LooseVersion('1.0.1'), LooseVersion('1.0.0')) + + def test_repr(self): + """Test __repr__ for LooseVersion.""" + for version_str in self.examples: + self.assertEqual(repr(LooseVersion(version_str)), "LooseVersion('{}')".format(version_str)) + + def test_str(self): + for version_str in self.examples: + self.assertEqual(str(LooseVersion(version_str)), version_str)