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.

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

feat: Add purl support for SBOMs #3373

Merged
merged 5 commits into from
Oct 9, 2023
Merged

Conversation

anthonyharrison
Copy link
Contributor

No description provided.

@terriko
Copy link
Contributor

terriko commented Oct 2, 2023

I'm so excited for this!

Copy link
Contributor

@terriko terriko 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. I think black needs one more re-run, and then I need to go add packageurl-python to our licensing list before I can approve it, but it looks like it's MIT licensed so there shouldn't be any issue.

cve_bin_tool/sbom_manager/__init__.py Outdated Show resolved Hide resolved
@terriko terriko added the dependencies Pull requests that update a dependency file label Oct 3, 2023
@terriko
Copy link
Contributor

terriko commented Oct 3, 2023

Also the test fails other than black should be fixed by #3362 once I get someone to approve that so it can be merged.

@terriko
Copy link
Contributor

terriko commented Oct 3, 2023

Updating branch for the commons-io problem

@codecov-commenter
Copy link

codecov-commenter commented Oct 3, 2023

Codecov Report

Merging #3373 (dd6426d) into main (4233f47) will decrease coverage by 0.90%.
Report is 6 commits behind head on main.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #3373      +/-   ##
==========================================
- Coverage   80.72%   79.83%   -0.90%     
==========================================
  Files         750      750              
  Lines       11534    11557      +23     
  Branches     1560     1568       +8     
==========================================
- Hits         9311     9226      -85     
- Misses       1786     1909     +123     
+ Partials      437      422      -15     
Flag Coverage Δ
longtests 74.72% <100.00%> (-1.12%) ⬇️
win-longtests 77.80% <100.00%> (-0.91%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
cve_bin_tool/sbom_manager/__init__.py 98.82% <100.00%> (+3.66%) ⬆️
test/test_sbom.py 100.00% <ø> (ø)

... and 8 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@@ -100,6 +97,41 @@ def get_vendor(self, product: str) -> list:
vendorlist.append("UNKNOWN")
return vendorlist

def parse_sbom(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we put a docstring here, since I'm starting to fill those out elsewhere? Something like "parse SBOM, using PURL identifiers preferentially if found" maybe?

Copy link
Contributor

@terriko terriko 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 to me. I've put in a request to update our licensing data and I don't anticipate any problems, but since it'll likely take a day before I get an answer, I'm going to be nitpicky and ask you to put a docstring in for parse_sbom()

Copy link
Contributor

@terriko terriko left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

Licensing asked me to file a new ticket (because my other one had a question about release-monitoring.org that will take longer to resolve) so that's been done this morning and hopefully I'll be able to merge this later this week.

@terriko
Copy link
Contributor

terriko commented Oct 9, 2023

Got my approval, so let's get this merged!

@terriko terriko merged commit fa8e6d8 into intel:main Oct 9, 2023
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants