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

Rpmdb support #255

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Rpmdb support #255

wants to merge 4 commits into from

Conversation

cmaritan
Copy link
Contributor

@cmaritan cmaritan commented Mar 1, 2023

Address #254

Copy link
Collaborator

@another-rex another-rex left a comment

Choose a reason for hiding this comment

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

Thanks! Looks like a reasonable implementation to me, my main concern is pulling in the relatively large number of dependencies dependencies to parse the sqlite database., especially before any support for redhat advisories are included in osv.dev .

Copy link
Collaborator

@G-Rath G-Rath 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 cool! It would be good if you could have a look over the other parsers and consider if there's any situations they account for that should be handled for this parser/"lockfile" too, such as the same packages being listed multiple times

(I'm not super familiar with RedHat stuff, so it might all be being handled by the library)

pkg/lockfile/rpmdb.go Outdated Show resolved Hide resolved
pkg/lockfile/rpmdb_test.go Outdated Show resolved Hide resolved
@cmaritan
Copy link
Contributor Author

cmaritan commented Mar 6, 2023

Thanks! Looks like a reasonable implementation to me, my main concern is pulling in the relatively large number of dependencies dependencies to parse the sqlite database., especially before any support for redhat advisories are included in osv.dev .

Thanks for the feedback, I hadn't considered this point.

If it's a problem, we can surely keep aside this PR and merge it when osv.dev will have redhat advisories support.

@cmaritan
Copy link
Contributor Author

cmaritan commented Mar 6, 2023

This looks cool! It would be good if you could have a look over the other parsers and consider if there's any situations they account for that should be handled for this parser/"lockfile" too, such as the same packages being listed multiple times

(I'm not super familiar with RedHat stuff, so it might all be being handled by the library)

Usually rpmdb are written only by librpm, passing duplicates/invalid values should not be possible.
Generating this test case would require direct insert of records inside sqlite db but maintainers keep its structure undocumented so it's possible to break other things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants