Repository: cekit/cekit

Issue:
CEKit 4.6.0 and packaging upgrade incompatibility
@spolti has noticed that the packaging library changes has caused further issues. 

This can be seen since 4.6.0 (4.5.0 doesn't exhibit these) produces a trace like:

```
  File "/tmp/cekit/lib/python3.11/site-packages/cekit/cli.py", line 292, in build_osbs
    run_build(ctx, "osbs")
  File "/tmp/cekit/lib/python3.11/site-packages/cekit/cli.py", line 434, in run_build
    run_command(ctx, builder_impl)
  File "/tmp/cekit/lib/python3.11/site-packages/cekit/cli.py", line 398, in run_command
    Cekit(params).run(clazz)
  File "/tmp/cekit/lib/python3.11/site-packages/cekit/cli.py", line 500, in run
    command.execute()
  File "/tmp/cekit/lib/python3.11/site-packages/cekit/builder.py", line 61, in execute
    self.before_generate()
  File "/tmp/cekit/lib/python3.11/site-packages/cekit/builder.py", line 118, in before_generate
    self.generator.init()
  File "/tmp/cekit/lib/python3.11/site-packages/cekit/generator/osbs.py", line 47, in init
    super(OSBSGenerator, self).init()
  File "/tmp/cekit/lib/python3.11/site-packages/cekit/generator/base.py", line 177, in init
    self.build_module_registry()
  File "/tmp/cekit/lib/python3.11/site-packages/cekit/generator/base.py", line 272, in build_module_registry
    self.load_repository(os.path.join(base_dir, repo.target))
  File "/tmp/cekit/lib/python3.11/site-packages/cekit/generator/base.py", line 291, in load_repository
    self._module_registry.add_module(module)
  File "/tmp/cekit/lib/python3.11/site-packages/cekit/generator/base.py", line 734, in add_module
    default_version: Version = parse_version(self._defaults.get(module.name))
                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/cekit/lib/python3.11/site-packages/packaging/version.py", line 52, in parse
    return Version(version)
           ^^^^^^^^^^^^^^^^
  File "/tmp/cekit/lib/python3.11/site-packages/packaging/version.py", line 198, in __init__
    raise InvalidVersion(f"Invalid version: '{version}'")
packaging.version.InvalidVersion: Invalid version: '3.6scl'
```

The problem is that there is a line
```
default_version: Version = parse_version(self._defaults.get(module.name))
```
but under the updated packaging the LegacyVersion support has been removed. This means the fallback fails, and an error is thrown. While in 4.6.0 we attempted to work around this by providing our own LegacyVersion this line was missed.


Discussion:

Patch:
diff --git a/cekit/generator/base.py b/cekit/generator/base.py
index 5b75fd5..211695a 100644
--- a/cekit/generator/base.py
+++ b/cekit/generator/base.py
@@ -10,7 +10,7 @@ from typing import TYPE_CHECKING, Any, Dict, List, Optional
 from urllib.parse import urlparse
 
 from jinja2 import Environment, FileSystemLoader
-from packaging.version import InvalidVersion, Version
+from packaging.version import InvalidVersion, Version, _BaseVersion
 from packaging.version import parse as parse_version
 
 from cekit.cekit_types import PathType
@@ -27,6 +27,7 @@ from cekit.descriptor import (
 )
 from cekit.errors import CekitError
 from cekit.generator import legacy_version
+from cekit.generator.legacy_version import LegacyVersion
 from cekit.template_helper import TemplateHelper
 from cekit.tools import DependencyDefinition, Map, download_file, load_descriptor
 from cekit.version import __version__ as cekit_version
@@ -729,27 +730,31 @@ class ModuleRegistry(object):
                 )
             )
 
-        default_version: Version = parse_version(self._defaults.get(module.name))
-        try:
-            current_version: Version = parse_version(version)
-            # This if block is only for Python3.6 compatibility which still might return
-            # a LegacyVersion and not throw an exception. LegacyVersion does not have
-            # that field so can be used to differentiate.
-            if not hasattr(current_version, "major"):
-                raise InvalidVersion
-            # If current module version is newer, we need to make it the new default
-            if current_version > default_version:
-                self._defaults[module.name] = version
-        except InvalidVersion:
-            LOGGER.error(
-                f"Module's '{module.name}' version '{version}' does not follow PEP 440 versioning scheme "
-                "(https://www.python.org/dev/peps/pep-0440) which should be followed in modules."
-            )
+        current_version: _BaseVersion = internal_parse_version(version, module.name)
+        default_version: _BaseVersion = internal_parse_version(
+            self._defaults.get(module.name), module.name
+        )
 
-            if legacy_version.parse(version) > legacy_version.parse(
-                self._defaults.get(module.name)
-            ):
-                self._defaults[module.name] = version
+        # If current module version is newer, we need to make it the new default
+        if current_version > default_version:
+            self._defaults[module.name] = version
 
         # Finally add the module to registry
         modules[version] = module
+
+
+def internal_parse_version(version: str, module_name: str) -> _BaseVersion:
+    try:
+        result: Version = parse_version(version)
+        # This if block is only for Python3.6 compatibility which still might return
+        # a LegacyVersion and not throw an exception. LegacyVersion does not have
+        # that field so can be used to differentiate.
+        if not hasattr(result, "major"):
+            raise InvalidVersion
+    except InvalidVersion:
+        LOGGER.error(
+            f"Module's '{module_name}' version '{version}' does not follow PEP 440 versioning "
+            "scheme (https://www.python.org/dev/peps/pep-0440) which should be followed in modules."
+        )
+        result: LegacyVersion = legacy_version.parse(version)
+    return result


Test patch:
diff --git a/tests/test_dockerfile.py b/tests/test_dockerfile.py
index a93b09f..1f8e0d9 100644
--- a/tests/test_dockerfile.py
+++ b/tests/test_dockerfile.py
@@ -705,7 +705,6 @@ def test_overrides_applied_to_all_multi_stage_images(tmpdir):
     regex_dockerfile(target, "^###### END image 'targetimage:SNAPSHOT'$")
 
 
-# @pytest.mark.parametrize("manager", ["yum", "dnf", "microdnf"])
 def test_package_removal_and_install_and_reinstall(tmpdir):
     target = str(tmpdir.mkdir("target"))
 
diff --git a/tests/test_validate.py b/tests/test_validate.py
index 218fbae..e47135a 100644
--- a/tests/test_validate.py
+++ b/tests/test_validate.py
@@ -1,3 +1,4 @@
+import logging
 import os
 import platform
 import shutil
@@ -11,6 +12,7 @@ from click.testing import CliRunner
 from cekit.cli import cli
 from cekit.config import Config
 from cekit.descriptor import Repository
+from cekit.generator import base
 from cekit.tools import Chdir
 
 odcs_fake_resp = b"""Result:
@@ -1666,6 +1668,17 @@ def test_run_descriptor_stdin(tmpdir):
     assert check_dockerfile(image_dir, "USER 4321", "Containerfile")
 
 
+@pytest.mark.parametrize(
+    "value", ["EAP7.3.0-kie", "python3", "osbs", "16.0-openjdk11-kie"]
+)
+def test_parse_version(caplog, value):
+    caplog.set_level(logging.DEBUG, logger="cekit")
+
+    base.internal_parse_version(value, "foo")
+
+    assert f"version '{value}' does not follow PEP 440" in caplog.text
+
+
 def run_cekit(cwd, parameters=None, message=None, env=None, input=None):
     if parameters is None:
         parameters = ["build", "--dry-run", "--container-file", "Dockerfile", "podman"]