-
Notifications
You must be signed in to change notification settings - Fork 531
feat: added ci script that updates mismatch database #4236
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
Conversation
Signed-off-by: Meet Soni <[email protected]>
Signed-off-by: Meet Soni <[email protected]>
Signed-off-by: Meet Soni <[email protected]>
Signed-off-by: Meet Soni <[email protected]>
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.
See notes below, but basically: this should get integrated with the update-cache.yml github action if you want to have it running right now.
Longer term, though, we should have cve-bin-tool -u now
load the mismatch data so a separate update isn't needed at all.
- name: Update database | ||
run: | | ||
[[ -e cache ]] && mkdir -p .cache && mv cache ~/.cache/cve-bin-tool | ||
python -m cve_bin_tool.cli test/assets/test-kerberos-5-1.15.1.out -u now | ||
cp -r ~/.cache/cve-bin-tool cache | ||
- name: Update mismatch database | ||
run: | | ||
python -m cve_bin_tool.mismatch_loader |
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 is doing a full database update which you probably don't want, and then it's saving the data from that update but not saving the data from the mismatch loader. Here's a quick rework just to highlight what's going on and what should be going on instead:
- name: Update database | |
run: | | |
[[ -e cache ]] && mkdir -p .cache && mv cache ~/.cache/cve-bin-tool | |
python -m cve_bin_tool.cli test/assets/test-kerberos-5-1.15.1.out -u now | |
cp -r ~/.cache/cve-bin-tool cache | |
- name: Update mismatch database | |
run: | | |
python -m cve_bin_tool.mismatch_loader | |
- name: Copy github cache into appropriate directory | |
run: | | |
[[ -e cache ]] && mkdir -p .cache && mv cache ~/.cache/cve-bin-tool | |
- name: Update mismatch database | |
run: | | |
python -m cve_bin_tool.mismatch_loader | |
- name: Save data back to github cache | |
run: | | |
cp -r ~/.cache/cve-bin-tool cache |
That said, don't accept that suggestion. You definitely want to take what's there and integrate the update into the existing cache-update.yml file instead.
The reason is mostly that I want to make sure that only one job is updating the date and data in the github cache because doing anything else is going to obfuscate any uptime issues with NVD and they're already a pain to handle.
Longer-term, though: we shouldn't need to run a separate script to update the mismatch data. All of that should be integrated into what happens when you run cve-bin-tool -u now
so that it happens seamlessly. Probably you want to treat mismatch as a data_source
similar to how purl2cpe is loaded.
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.
Idea of separate script was to ensure we can make a library for mismatch database in which user can add/remove data as a standalone entity. Making it similar to data_source, wouldn't that be attached to cve-bin-tool?
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.
Yeah, I think we'll need to refactor a bit if we want it to work standalone because of the database initialization -- it's not much of a standalone if we have to run a full cve-bin-tool update to make it work. But let's get it working this way first.
I think what we want eventually is the ability to update any single data source separately (we've got an open issue for that) and we can probably solve that problem and this one at the same time.
Signed-off-by: Meet Soni <[email protected]>
Re-running the finicky tests. |
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 looks ready to merge, but I can't merge it until the tests pass. You probably should just make a PR that (temporarily) disables those tests so you don't get blocked. I'm not going to get to it before I have to travel!
Updating your branch for the cranky tests now. |
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.
Finally, no more tests behaving badly. Let's get this merged, and thanks for your patience!
depends on #4223
This script will run the
mismatch_loader
when thedata/
directory is updated.cc @terriko @anthonyharrison