-
Notifications
You must be signed in to change notification settings - Fork 506
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: adding SBOM to package #2810
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2810 +/- ##
==========================================
+ Coverage 81.11% 83.14% +2.03%
==========================================
Files 648 651 +3
Lines 10168 10254 +86
Branches 1369 1382 +13
==========================================
+ Hits 8248 8526 +278
+ Misses 1558 1376 -182
+ Partials 362 352 -10
Flags with carried forward coverage won't be shown. Click here to find out more. see 18 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure whether we should be removing the markdown generator or using the output somewhere, but I think this may need some changes. I'd probably default to just taking the lines out of the .yml file but I'll let @anthonyharrison review this PR first in case I'm missing something.
.github/workflows/sbom.yml
Outdated
pip install . --upgrade --upgrade-strategy=eager | ||
- name: Generate SBOM for cve-bin-tool | ||
run: | | ||
sbom4python --module cve-bin-tool --output cve-bin-tool-py${{ matrix.python }}.spdx | ||
sbom4python --module cve-bin-tool --sbom cyclonedx --format json --output cve-bin-tool-py${{ matrix.python }}.json | ||
- name: Generate Markdown for SBOM | ||
run: | | ||
sbom2doc --input-file cve-bin-tool-py${{ matrix.python }}.spdx --format markdown --output-file cve-bin-tool-py${{ matrix.python }}.html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This appears to generate a markdown version of the sbom but then it doesn't look like you use it anywhere. What was the goal here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@terriko The idea I had was to create a release note which included the SBOM information. I suggested that we use my sbom2doc utility as it produces a markdown version of the SBOM and the idea (which I probably didn't share with @Rexbeast2) was to use this file as part of a bigger document. Does this seem like a resonable goal @terriko ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good long-term, but if that's how it's going to be used, let's add it into a docs build step rather than updating it every week alongside the SBOM. So we can take it out of here and build it into a different PR when we're actually prepared to use it. They can be generated at any time since we'll have the weekly sboms anyhow.
.github/workflows/sbom.yml
Outdated
pip install . --upgrade --upgrade-strategy=eager | ||
- name: Generate SBOM for cve-bin-tool | ||
run: | | ||
sbom4python --module cve-bin-tool --output cve-bin-tool-py${{ matrix.python }}.spdx | ||
sbom4python --module cve-bin-tool --sbom cyclonedx --format json --output cve-bin-tool-py${{ matrix.python }}.json | ||
- name: Generate Markdown for SBOM | ||
run: | | ||
sbom2doc --input-file cve-bin-tool-py${{ matrix.python }}.spdx --format markdown --output-file cve-bin-tool-py${{ matrix.python }}.html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good long-term, but if that's how it's going to be used, let's add it into a docs build step rather than updating it every week alongside the SBOM. So we can take it out of here and build it into a different PR when we're actually prepared to use it. They can be generated at any time since we'll have the weekly sboms anyhow.
@terriko if there are any more changes you want. let me know. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've changed the sbom suggestions so you should be able to just click and remove those sections and then this should be ready to merge. We'll probably move the sbom generation into the docs/ build files in a separate PR; I'll open an issue about it.
.github/workflows/testing.yml
Outdated
@@ -37,10 +37,14 @@ jobs: | |||
python -m pip install --upgrade setuptools | |||
python -m pip install --upgrade wheel | |||
python -m pip install --upgrade -r doc/requirements.txt | |||
python -m pip install --upgrade sbom2doc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
python -m pip install --upgrade sbom2doc |
Removing this for now until we're ready to use the generated sbom docs.
.github/workflows/testing.yml
Outdated
- name: Generate Markdown for SBOM | ||
run: | | ||
sbom2doc --input-file cve-bin-tool-py${{ matrix.python }}.spdx --format markdown --output-file cve-bin-tool-py${{ matrix.python }}.html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- name: Generate Markdown for SBOM | |
run: | | |
sbom2doc --input-file cve-bin-tool-py${{ matrix.python }}.spdx --format markdown --output-file cve-bin-tool-py${{ matrix.python }}.html |
As elsewhere, removing the sbom->doc translation for now until we're ready to use the results in our docs somewhere. I think this will go into the docs build script rather than in testing.yml in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I think we're ready to get this merged. Thank you!
fixes #2687