Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add gitlab importer #662

Merged
merged 4 commits into from
May 21, 2022
Merged

Add gitlab importer #662

merged 4 commits into from
May 21, 2022

Conversation

TG1999
Copy link
Contributor

@TG1999 TG1999 commented Mar 31, 2022

No description provided.

@TG1999 TG1999 force-pushed the new_importer/gitlab branch from 94cda22 to 809ef6d Compare March 31, 2022 18:49
@TG1999 TG1999 changed the title New importer/gitlab Add gitlab importer Mar 31, 2022
pombredanne
pombredanne requested changes Apr 5, 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.

This looks good overall... we are just missing tests?

@TG1999
Copy link
Contributor Author

TG1999 commented Apr 5, 2022

@pombredanne some work related to Nuget VersionRange is pending at univers side, once that is done I will add tests and the improver for this importer as well.

@TG1999 TG1999 force-pushed the new_importer/gitlab branch from 809ef6d to 8b3d8e1 Compare April 27, 2022 10:43
@TG1999 TG1999 force-pushed the new_importer/gitlab branch from bb4df98 to 5041b3f Compare May 5, 2022 13:52
@TG1999 TG1999 linked an issue May 5, 2022 that may be closed by this pull request
Add support for new GitLab community advisories #438
Closed
@TG1999 TG1999 force-pushed the new_importer/gitlab branch 12 times, most recently from 0ce7f2f to 5ae6aeb Compare May 12, 2022 11:18
@TG1999 TG1999 changed the title Add gitlab importer Add gitlab importer and use NginxVersion instead of SemverVersion in Nginx importer and improver May 12, 2022
@TG1999 TG1999 force-pushed the new_importer/gitlab branch from 5ae6aeb to 9548943 Compare May 12, 2022 14:11
@TG1999 TG1999 changed the title Add gitlab importer and use NginxVersion instead of SemverVersion in Nginx importer and improver Add gitlab importer May 13, 2022
@TG1999 TG1999 force-pushed the new_importer/gitlab branch from 2b393d8 to 8cc0045 Compare May 13, 2022 15:37
pombredanne
pombredanne requested changes May 13, 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! Some partial and early feedback for your review.

vulnerabilities/helpers.py Outdated Show resolved Hide resolved
vulnerabilities/helpers.py Outdated Show resolved Hide resolved
vulnerabilities/helpers.py Outdated Show resolved Hide resolved
vulnerabilities/importers/github.py Outdated Show resolved Hide resolved
vulnerabilities/importers/github.py Outdated Show resolved Hide resolved
vulnerabilities/importers/gitlab.py Show resolved Hide resolved
vulnerabilities/importers/gitlab.py Outdated Show resolved Hide resolved
vulnerabilities/importers/gitlab.py Outdated Show resolved Hide resolved
vulnerabilities/importers/gitlab.py Outdated Show resolved Hide resolved
vulnerabilities/importers/gitlab.py Outdated Show resolved Hide resolved
@TG1999 TG1999 force-pushed the new_importer/gitlab branch 6 times, most recently from d9380ca to 372a8cc Compare May 17, 2022 19:35
pombredanne
pombredanne requested changes May 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.

Thanks!
I have a few review comments for your consideration.

vulnerabilities/importers/gitlab.py Outdated Show resolved Hide resolved
vulnerabilities/importers/gitlab.py Outdated Show resolved Hide resolved
vulnerabilities/importers/gitlab.py Outdated Show resolved Hide resolved
vulnerabilities/importers/gitlab.py Show resolved Hide resolved
vulnerabilities/importers/gitlab.py Outdated Show resolved Hide resolved
vulnerabilities/importers/gitlab.py Outdated Show resolved Hide resolved
vulnerabilities/importers/gitlab.py Outdated Show resolved Hide resolved
vulnerabilities/importers/gitlab.py Outdated Show resolved Hide resolved
vulnerabilities/importers/gitlab.py Show resolved Hide resolved
vulnerabilities/package_managers.py Show resolved Hide resolved
@TG1999 TG1999 mentioned this pull request May 18, 2022
Open
@TG1999 TG1999 force-pushed the new_importer/gitlab branch 2 times, most recently from 5faee3f to 33d9f18 Compare May 18, 2022 17:06
@TG1999 TG1999 mentioned this pull request May 20, 2022
Open
pombredanne
pombredanne reviewed May 20, 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.

Just a tiny nit left wrt. purl for npm and golang.
You should consider the namespace there IMHO when doing API lookups. Both are using / as a separator. Then please go ahead and merge!
🙇

TG1999 added 2 commits May 21, 2022 15:09
@TG1999
And gitlab importer
c32dde1 
And use NginxVersion instead of SemverVersion in Nginx importer and improver

Signed-off-by: Tushar Goel <[email protected]>
@TG1999
Address review comments
5127d0b 
Signed-off-by: Tushar Goel <[email protected]>
@TG1999 TG1999 force-pushed the new_importer/gitlab branch from fb4ed56 to 260d49b Compare May 21, 2022 10:14
@TG1999 TG1999 linked an issue May 21, 2022 that may be closed by this pull request
Errors when running the vulnerabilities.importers.github.GitHubAPIImporter #673
Closed
@TG1999 TG1999 force-pushed the new_importer/gitlab branch from 260d49b to f366a01 Compare May 21, 2022 10:29
@TG1999 TG1999 force-pushed the new_importer/gitlab branch from f366a01 to 80f883a Compare May 21, 2022 10:38
@TG1999
Reference: aboutcode-org#673
b7ae8dc 
Return PackageURL for composer in github importer if namespace is not present

Signed-off-by: Tushar Goel <[email protected]>
@TG1999 TG1999 force-pushed the new_importer/gitlab branch from 80f883a to b7ae8dc Compare May 21, 2022 10:39
@TG1999
Copy link
Contributor Author

TG1999 commented May 21, 2022

While I was at it I also solved a minor wart in github importer in this commit b7ae8dc, solving #673

@TG1999 TG1999 merged commit 6ec2e9e into aboutcode-org:main May 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pombredanne pombredanne pombredanne approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@TG1999 @pombredanne