From c07d5259bdca1e09fac9f12472fde6c90f029b4a Mon Sep 17 00:00:00 2001 From: dgpatelgit Date: Mon, 22 Mar 2021 15:34:09 +0530 Subject: [PATCH] [APPAI-1776] Fixed sentry error due to invalid version (#737) * [APPAI-1776] Fixed sentry error due to invalid version * Used version comparator utility instead of custom code * Moved version comparator to latest code * Revert "Moved version comparator to latest code" This reverts commit 7fb71d1910285a626a4c9487c50010a53b0616a4. * Removed latest non-cve version loop * Reduced security issues and resolve 1 exploits, add required deps * Added more test cases to improve codecov * Moved conditions outside exception as per review comments * Moved comparison to else block as per review comments * Removed test case as per comments --- bayesian/utility/v2/ca_response_builder.py | 35 ++++++-------- bayesian/utils.py | 42 ----------------- requirements.in | 1 + requirements.txt | 10 ++-- tests/test_utils.py | 48 -------------------- tests/utility/v2/test_ca_response_builder.py | 4 +- 6 files changed, 24 insertions(+), 116 deletions(-) diff --git a/bayesian/utility/v2/ca_response_builder.py b/bayesian/utility/v2/ca_response_builder.py index 68934416..a2b293cf 100644 --- a/bayesian/utility/v2/ca_response_builder.py +++ b/bayesian/utility/v2/ca_response_builder.py @@ -15,12 +15,13 @@ # """Utility function from API v2.""" +# import typing from urllib.parse import quote import logging from bayesian.utility.db_gateway import GraphAnalyses from f8a_utils.user_token_utils import UserStatus -from bayesian.utils import version_info_tuple, convert_version_to_proper_semantic +from f8a_version_comparator.comparable_version import ComparableVersion from typing import Dict, List, Optional from collections import namedtuple from abc import ABC @@ -159,26 +160,20 @@ def get_version_without_cves(self, latest_non_cve_versions): :return: Highest Version out of all latest_non_cve_versions. """ - logger.debug("All Versions with Vulnerabilities.") - input_version_tuple = version_info_tuple( - convert_version_to_proper_semantic(self.version) - ) highest_version = '' - for version in latest_non_cve_versions: - - graph_version_tuple = version_info_tuple( - convert_version_to_proper_semantic(version) - ) - if graph_version_tuple > input_version_tuple: - if not highest_version: - highest_version = version - highest_version_tuple = version_info_tuple( - convert_version_to_proper_semantic(highest_version) - ) - # If version to recommend is closer to what a user is using then, use less than - # If recommendation is to show highest version then, use greater than - if graph_version_tuple > highest_version_tuple: - highest_version = version + try: + input_version_comparable = ComparableVersion(self.version) + + # latest non-cve is list with only one entry. + latest_non_cve_version_comparable = ComparableVersion(latest_non_cve_versions[0]) + except TypeError: + logger.error("Package %s@%s raised a TypeError", self.package, self.version) + else: + if latest_non_cve_version_comparable > input_version_comparable: + highest_version = latest_non_cve_versions[0] + + logger.info("Highest non-cve version for %s@%s is %s", self.package, self.version, + highest_version) return highest_version def has_cves(self): diff --git a/bayesian/utils.py b/bayesian/utils.py index fa03bd18..2aea524f 100644 --- a/bayesian/utils.py +++ b/bayesian/utils.py @@ -195,48 +195,6 @@ def request_timed_out(request): return False -def convert_version_to_proper_semantic(version, package_name=None): - """Perform Semantic versioning. - - : type version: string - : param version: The raw input version that needs to be converted. - : type return: semantic_version.base.Version - : return: The semantic version of raw input version. - """ - conv_version = sv.Version.coerce('0.0.0') - try: - if version in ('', '-1', None): - version = '0.0.0' - """Needed for maven version like 1.5.2.RELEASE to be converted to - 1.5.2 - RELEASE for semantic version to work.""" - version = version.replace('.', '-', 3) - version = version.replace('-', '.', 2) - # Needed to add this so that -RELEASE is account as a Version.build - version = version.replace('-', '+', 3) - conv_version = sv.Version.coerce(version) - except ValueError: - logger.error('Unexpected ValueError for the package %s due to version %s', - package_name, version) - pass - finally: - return conv_version - - -def version_info_tuple(version): - """Return version information in form of (major, minor, patch, build) for a given sem Version. - - : type version: semantic_version.base.Version - : param version: The semantic version whole details are needed. - : return: A tuple in form of Version.(major, minor, patch, build) - """ - if type(version) == sv.base.Version: - return(version.major, - version.minor, - version.patch, - version.build) - return (0, 0, 0, tuple()) - - def is_valid(param): """Return true is the param is not a null value.""" return param is not None diff --git a/requirements.in b/requirements.in index 4fc92d37..3890fb5f 100644 --- a/requirements.in +++ b/requirements.in @@ -27,3 +27,4 @@ psycopg2-binary f8a_worker @ git+https://github.com/fabric8-analytics/fabric8-analytics-worker.git@af1680a#egg=f8a_worker f8a_auth @ git+https://github.com/fabric8-analytics/fabric8-analytics-auth.git@5ff9438#egg=fabric8a_auth f8a_utils @ git+https://github.com/fabric8-analytics/fabric8-analytics-utils.git@839e022#egg=f8a_utils +f8a_version_comparator @ git+https://github.com/fabric8-analytics/fabric8-analytics-version-comparator.git@8a57ac7#egg=f8a_version_comparator diff --git a/requirements.txt b/requirements.txt index dc7037ec..87c9cdf0 100644 --- a/requirements.txt +++ b/requirements.txt @@ -56,7 +56,7 @@ colorama==0.4.4 # via rainbow-logging-handler configobj==5.0.6 # via anymarkup -cryptography==3.3.1 +cryptography==3.4.6 # via # f8a-utils # fabric8a-auth @@ -69,7 +69,9 @@ git+https://github.com/fabric8-analytics/fabric8-analytics-utils.git@839e022#egg # -r requirements.in # f8a-worker git+https://github.com/fabric8-analytics/fabric8-analytics-version-comparator.git@8a57ac7#egg=f8a_version_comparator - # via f8a-utils + # via + # -r requirements.in + # f8a-utils git+https://github.com/fabric8-analytics/fabric8-analytics-worker.git@af1680a#egg=f8a_worker # via -r requirements.in git+https://github.com/fabric8-analytics/fabric8-analytics-auth.git@5ff9438#egg=fabric8a_auth @@ -138,7 +140,7 @@ itsdangerous==1.1.0 # flask # flask-security # flask-wtf -jinja2==2.11.2 +jinja2==2.11.3 # via # flask # flask-babelex @@ -261,7 +263,7 @@ toml==0.9.4 # f8a-worker typing-extensions==3.7.4.3 # via importlib-metadata -urllib3==1.26.3 +urllib3==1.26.4 # via # botocore # requests diff --git a/tests/test_utils.py b/tests/test_utils.py index 91681530..71a989bf 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -2,11 +2,8 @@ import datetime import pytest -import semantic_version as sv from bayesian.utils import ( is_valid, get_user_email, - convert_version_to_proper_semantic as cvs, - version_info_tuple as vt, resolved_files_exist, get_ecosystem_from_manifest, check_for_accepted_ecosystem @@ -212,51 +209,6 @@ def test_github_url_exist_or_not(self): self.__url).code == 200, "Not able to access the url {}".format(self.__url) -def test_semantic_versioning(): - """Check the function cvs().""" - package_name = "test_package" - version = "-1" - assert cvs(version, package_name) == sv.Version("0.0.0") - version = "" - assert cvs(version, package_name) == sv.Version("0.0.0") - version = None - assert cvs(version, package_name) == sv.Version("0.0.0") - version = "1.5.2.RELEASE" - assert cvs(version, package_name) == sv.Version("1.5.2+RELEASE") - version = "1.5-2.RELEASE" - assert cvs(version, package_name) == sv.Version("1.5.2+RELEASE") - version = "2" - assert cvs(version, package_name) == sv.Version("2.0.0") - version = "2.3" - assert cvs(version, package_name) == sv.Version("2.3.0") - version = "2.0.rc1" - assert cvs(version, package_name) == sv.Version("2.0.0+rc1") - - -def test_version_info_tuple(): - """Check the function vt().""" - version_str = "2.0.rc1" - package_name = "test_package" - version_obj = cvs(version_str, package_name) - version_info = vt(version_obj) - assert len(version_info) == 4 - assert version_info[0] == version_obj.major - assert version_info[1] == version_obj.minor - assert version_info[2] == version_obj.patch - assert version_info[3] == version_obj.build - - -def test_version_info_tuple_empty_version_obj(): - """Check the function vt() for empty version object.""" - version_obj = "" - version_info = vt(version_obj) - assert len(version_info) == 4 - assert version_info[0] == 0 - assert version_info[1] == 0 - assert version_info[2] == 0 - assert version_info[3] == tuple() - - def test_get_user_email(): """Check the function get_user_email().""" assert get_user_email(None) == 'bayesian@redhat.com' diff --git a/tests/utility/v2/test_ca_response_builder.py b/tests/utility/v2/test_ca_response_builder.py index ed5ca749..6853bf22 100644 --- a/tests/utility/v2/test_ca_response_builder.py +++ b/tests/utility/v2/test_ca_response_builder.py @@ -283,8 +283,8 @@ def test_get_version_without_cves(self): def test_get_version_without_cves_highest(self): """Test Get highest version without cves.""" response_obj = ComponentAnalysisResponseBuilder(self.eco, self.pkg, self.ver) - version = response_obj.get_version_without_cves(['2', '3']) - self.assertEqual(version, '3') + version = response_obj.get_version_without_cves(['0.6']) + self.assertEqual(version, '') @patch('bayesian.utility.v2.ca_response_builder.' 'ComponentAnalysisResponseBuilder.get_cve_maps')