Skip to content

Refactor Gitimporter using fetchcode#817

Merged
pombredanne merged 1 commit intoaboutcode-org:mainfrom
ziadhany:git-fetchcode
Oct 18, 2022
Merged

Refactor Gitimporter using fetchcode#817
pombredanne merged 1 commit intoaboutcode-org:mainfrom
ziadhany:git-fetchcode

Conversation

@ziadhany
Copy link
Collaborator

@ziadhany ziadhany commented Aug 1, 2022

Reference: #806
Signed-off-by: ziad ziadhany2016@gmail.com

@ziadhany ziadhany mentioned this pull request Sep 5, 2022
Closed
@TG1999
Copy link
Contributor

TG1999 commented Sep 22, 2022

@ziadhany , please add logs for gitlab importer.

TG1999
TG1999 requested changes Sep 22, 2022
View reviewed changes
@ziadhany ziadhany mentioned this pull request Sep 22, 2022
Open
9 tasks
@ziadhany
Copy link
Collaborator Author

ziadhany commented Sep 24, 2022

@ziadhany , please add logs for gitlab importer.

Importing data using vulnerabilities.importers.gitlab.GitLabGitImporter
Unknow package type 'conan'
Unknow package type 'conan'
...
Unknow package type 'conan'
Unknow package type 'conan'
Unknow package type 'go'
....
Unknow package type 'go'
Unknow package type 'go'
....
parse_yaml_file: affected_range is not parsable: '(,)' type:'nuget' error: InvalidVersion("'None' is not a valid <class 'univers.versions.NugetVersion'>")
 Traceback (most recent call last):
  File "/home/ziad/PycharmProjects/vulnerablecode/vulnerabilities/importers/gitlab.py", line 223, in parse_gitlab_advisory
    affected_version_range = vrc.from_native(affected_range)
  File "/home/ziad/PycharmProjects/vulnerablecode/venv/lib/python3.10/site-packages/univers/version_range.py", line 616, in from_native
    VersionConstraint(comparator="=", version=cls.version_class(str(lower_bound)))
  File "<attrs generated init univers.versions.NugetVersion>", line 7, in __init__
    self.__attrs_post_init__()
  File "/home/ziad/PycharmProjects/vulnerablecode/venv/lib/python3.10/site-packages/univers/versions.py", line 87, in __attrs_post_init__
    raise InvalidVersion(f"{self.string!r} is not a valid {self.__class__!r}")
univers.versions.InvalidVersion: 'None' is not a valid <class 'univers.versions.NugetVersion'>

parse_yaml_file: affected_range is not parsable: '[4.0.0],(,3.4.0),[4.0.0,4.1.2)' type:'nuget' error: VersionRangeParseError('Ranges overlap: [4.0.0],(,3.4.0),[4.0.0,4.1.2)')
 Traceback (most recent call last):
  File "/home/ziad/PycharmProjects/vulnerablecode/vulnerabilities/importers/gitlab.py", line 223, in parse_gitlab_advisory
    affected_version_range = vrc.from_native(affected_range)
  File "/home/ziad/PycharmProjects/vulnerablecode/venv/lib/python3.10/site-packages/univers/version_range.py", line 605, in from_native
    restrictions = maven.VersionRange(string).restrictions
  File "/home/ziad/PycharmProjects/vulnerablecode/venv/lib/python3.10/site-packages/univers/maven.py", line 226, in __init__
    raise VersionRangeParseError("Ranges overlap: %s" % spec)
univers.maven.VersionRangeParseError: Ranges overlap: [4.0.0],(,3.4.0),[4.0.0,4.1.2)

parse_yaml_file: affected_range is not parsable: '[2.2],[2.1]' type:'nuget' error: VersionRangeParseError('Ranges overlap: [2.2],[2.1]')
 Traceback (most recent call last):
  File "/home/ziad/PycharmProjects/vulnerablecode/vulnerabilities/importers/gitlab.py", line 223, in parse_gitlab_advisory
    affected_version_range = vrc.from_native(affected_range)
  File "/home/ziad/PycharmProjects/vulnerablecode/venv/lib/python3.10/site-packages/univers/version_range.py", line 605, in from_native
    restrictions = maven.VersionRange(string).restrictions
  File "/home/ziad/PycharmProjects/vulnerablecode/venv/lib/python3.10/site-packages/univers/maven.py", line 226, in __init__
    raise VersionRangeParseError("Ranges overlap: %s" % spec)
univers.maven.VersionRangeParseError: Ranges overlap: [2.2],[2.1]
.....

Successfully imported data using vulnerabilities.importers.gitlab.GitLabGitImporter

TG1999
TG1999 reviewed Sep 27, 2022
View reviewed changes
vulnerabilities/importers/gitlab.py Outdated


class GitLabAPIImporter(Importer):
class GitLabGitImporter(GitImporter):
Copy link
Contributor

@TG1999 TG1999 Sep 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ziadhany this will change the qualified name for importer and the databases will still have the old qualified name stored in them, which will now not be recogonised by improver, either write a migration to support this change or do not change the name of importer.

TG1999
TG1999 approved these changes Oct 10, 2022
View reviewed changes
Copy link
Contributor

@TG1999 TG1999 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks

@TG1999
Copy link
Contributor

TG1999 commented Oct 10, 2022

Please rebase with the latest main

@TG1999 TG1999 requested a review from pombredanne October 10, 2022 14:03
pombredanne
pombredanne reviewed Oct 10, 2022
View reviewed changes
Copy link
Member

@pombredanne pombredanne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!
I posted a few comments inline to ensure we do drop tests. Beside these, this looks good.

vulnerabilities/tests/test_data_source.py
TEST_DATA = os.path.join(BASE_DIR, "test_data/")


def load_oval_data():
Copy link
Member

@pombredanne pombredanne Oct 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO this would be best moved to a new test_oval.py

vulnerabilities/tests/test_data_source.py
assert os.path.join(self.repodir, "crates/hyper/RUSTSEC-2020-0008.toml") in updated_files


class TestOvalImporter(TestCase):
Copy link
Member

@pombredanne pombredanne Oct 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO this would be best moved to a new test_oval.py and not just deleted

Copy link
Member

@pombredanne pombredanne Oct 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot approve this if we just delete all these tests above.

Copy link
Contributor

@TG1999 TG1999 Oct 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have secured oval importer tests in the file itself https://github.com/nexB/vulnerablecode/pull/817/files#diff-eb1cffdadef3eacc470900468af6cce0bdd6ae476bc9ae94ce512863c060f850R40, and just deleted the GitImporter tests that are not related with the latest importers.

vulnerabilities/tests/test_data_source.py Outdated
def test_clone_valid(self):
with patch.object(GitImporter, "__init__", return_value=None):
c = GitImporter(None)
c.repo_url = "git+https://github.com/nexB/fetchcode"
Copy link
Member

@pombredanne pombredanne Oct 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this do a live clone over the network?

Copy link
Collaborator Author

@ziadhany ziadhany Oct 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, it clones the repository
( I just mock a constructor and then make the Git importer run, so I can test the result )

@TG1999 TG1999 force-pushed the git-fetchcode branch 5 times, most recently from e4ad48a to b8c4b6f Compare October 18, 2022 11:29
@ziadhany @TG1999
Refactor Gitimporter using fetchcode
3632832 
Co-authored-by: Tushar Goel <tushar.goel.dav@gmail.com>

Signed-off-by: ziadhany <ziadhany2016@gmail.com>
pombredanne
pombredanne approved these changes Oct 18, 2022
View reviewed changes
Copy link
Member

@pombredanne pombredanne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

@pombredanne pombredanne merged commit 8e8624b into aboutcode-org:main Oct 18, 2022
@ziadhany ziadhany deleted the git-fetchcode branch February 13, 2024 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TG1999 TG1999 TG1999 approved these changes

@pombredanne pombredanne pombredanne approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ziadhany @TG1999 @pombredanne