From e6f6530973d30058719175b955f2387790ab3ff5 Mon Sep 17 00:00:00 2001 From: Gernot Hillier Date: Tue, 14 Jan 2025 19:44:18 +0100 Subject: [PATCH 1/2] fix: avoid `bom map` losing releases due to inconsistent SW360 data In case of paring errors in the version number, an empty ComparableVersion object was created, leading to an exception in release sorting in map_bom_item_no_cache(). This was caught by the caller map_bom_to_releases(), but lead to this component being missing completely from the output BOM. This fixes the obvious part of #117. However, I still think that the catch-all exception handling in map_bom_to_releases() should be re-considered. --- ChangeLog.md | 1 + capycli/bom/map_bom.py | 16 ++++++++++---- capycli/common/comparable_version.py | 1 + tests/test_bom_map2.py | 32 ++++++++++++++++++++++++++++ 4 files changed, 46 insertions(+), 4 deletions(-) diff --git a/ChangeLog.md b/ChangeLog.md index edf0234..cdb7773 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -18,6 +18,7 @@ to SPECIFIC of to the value given by `-pms`. Now **existing** Project Mainline States are kept. * `project create` has a new parameter `--copy_from` which allows to first create a copy of the given project and then update the releases based on the contents of the given SBOM. +* fix for `bom map` loosing SBOM items when it tries to map to invalid SW360 releases ## 2.6.0 diff --git a/capycli/bom/map_bom.py b/capycli/bom/map_bom.py index a1f7c29..ed9cfff 100644 --- a/capycli/bom/map_bom.py +++ b/capycli/bom/map_bom.py @@ -375,9 +375,13 @@ def map_bom_item_no_cache(self, component: Component) -> MapResult: # Sorted alternatives in descending version order # Please note: the release list sometimes contain just the href but no version - rel_list = sorted(rel_list, - key=lambda x: "version" in x and ComparableVersion( - x.get("version", "")), reverse=True) # type: ignore + try: + rel_list = sorted(rel_list, + key=lambda x: "version" in x and ComparableVersion( + x.get("version", "")), reverse=True) # type: ignore + except ValueError: + pass # we can live with an unsorted list + for relref in rel_list: href = relref["_links"]["self"]["href"] real_release = self.client.get_release_by_url(href) @@ -779,7 +783,11 @@ def create_updated_bom(self, old_bom: Bom, result: List[MapResult]) -> Bom: newbom.components.add(newitem) # Sorted alternatives in descending version order - item.releases = sorted(item.releases, key=lambda x: ComparableVersion(x['Version']), reverse=True) + try: + item.releases = sorted(item.releases, key=lambda x: ComparableVersion(x['Version']), reverse=True) + except ValueError: + pass # we can live with an unsorted list + for match_item in item.releases: if self.is_good_match(match_item["MapResult"]): newitem = self.update_bom_item(item.component, match_item) diff --git a/capycli/common/comparable_version.py b/capycli/common/comparable_version.py index 80c4f1e..4fbda58 100644 --- a/capycli/common/comparable_version.py +++ b/capycli/common/comparable_version.py @@ -31,6 +31,7 @@ def __init__(self, version: str) -> None: self.parts = self.parse(version) except Exception: LOG.warning("Unable to parse version %s", version) + raise # pass on to caller as object is useless without self.parts @staticmethod def parse(version: str) -> List[Tuple[bool, int | str]]: diff --git a/tests/test_bom_map2.py b/tests/test_bom_map2.py index bedf9db..6bf7b57 100644 --- a/tests/test_bom_map2.py +++ b/tests/test_bom_map2.py @@ -191,6 +191,38 @@ def test_map_bom_item_nocache_mixed_match(self) -> None: assert res.result == MapResult.FULL_MATCH_BY_NAME_AND_VERSION assert len(res.releases) == 1 + @responses.activate + def test_map_bom_item_nocache_invalid_version(self) -> None: + bomitem = Component( + name="mail", + version="1.4") + component_matches = {"_embedded": {"sw360:components": [ + {"name": "mail", + "_links": {"self": {"href": SW360_BASE_URL + 'components/b001'}}}]}} + component_data1 = {"_embedded": {"sw360:releases": [ + {"version": "1.4", + "_links": {"self": {"href": SW360_BASE_URL + 'releases/1111'}}}, + {"version": "1.0._ME-2", + "_links": {"self": {"href": SW360_BASE_URL + 'releases/1112'}}}]}} + release_data1 = {"name": "mail", "version": "1.4", "_links": { + "self": {"href": SW360_BASE_URL + 'releases/1111'}, + "sw360:component": {"href": SW360_BASE_URL + "components/b001"}}} + release_data2 = {"name": "Mail", "version": "1.0._ME-2", "_links": { + "self": {"href": SW360_BASE_URL + 'releases/1112'}, + "sw360:component": {"href": SW360_BASE_URL + "components/b002"}}} + responses.add(responses.GET, SW360_BASE_URL + 'components?name=mail', + json=component_matches) + responses.add(responses.GET, SW360_BASE_URL + 'components/b001', + json=component_data1) + responses.add(responses.GET, SW360_BASE_URL + 'releases/1111', + json=release_data1) + responses.add(responses.GET, SW360_BASE_URL + 'releases/1112', + json=release_data2) + + res = self.app.map_bom_item_no_cache(bomitem) + assert res.result == MapResult.FULL_MATCH_BY_NAME_AND_VERSION + assert len(res.releases) == 1 + # ----------------- map_bom_item_no_cache purl cases -------------------- @responses.activate From ad73a3f5073995c28d696ba396bfe845401681ce Mon Sep 17 00:00:00 2001 From: Gernot Hillier Date: Tue, 14 Jan 2025 19:50:26 +0100 Subject: [PATCH 2/2] tests(test_bom_map2): fix TODO comment The was a reference to our old, internal issue tracker, so I migrated this issue to Github #118 and updated the reference. --- tests/test_bom_map2.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_bom_map2.py b/tests/test_bom_map2.py index 6bf7b57..219cade 100644 --- a/tests/test_bom_map2.py +++ b/tests/test_bom_map2.py @@ -178,7 +178,7 @@ def test_map_bom_item_nocache_mixed_match(self) -> None: res = self.app.map_bom_item_no_cache(bomitem) assert res.result == MapResult.FULL_MATCH_BY_NAME_AND_VERSION - # TODO see #25: assert len(res.releases) == 1 + # TODO see #118: assert len(res.releases) == 1 component_matches = {"_embedded": {"sw360:components": [ {"name": "Mail",