Skip to content

Commit

Permalink
[APPAI-1776] Fixed sentry error due to invalid version (#737)
Browse files Browse the repository at this point in the history
* [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 7fb71d1.

* 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
  • Loading branch information
dgpatelgit authored Mar 22, 2021
1 parent f4edec1 commit c07d525
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 116 deletions.
35 changes: 15 additions & 20 deletions bayesian/utility/v2/ca_response_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down
42 changes: 0 additions & 42 deletions bayesian/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions requirements.in
Original file line number Diff line number Diff line change
Expand Up @@ -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
10 changes: 6 additions & 4 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
48 changes: 0 additions & 48 deletions tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) == '[email protected]'
Expand Down
4 changes: 2 additions & 2 deletions tests/utility/v2/test_ca_response_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down

0 comments on commit c07d525

Please sign in to comment.