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.

@andrewpollock
Copy link
Contributor

Rocky Linux (currently present in OSV.dev) uses RPM and I'm not sure about Mageia, but it's in progress... It might be worthwhile getting this PR merged?

@another-rex
Copy link
Collaborator

I believe we can get rpmdb support with the osv-scalibr integration soon.

@picatz
Copy link

picatz commented Sep 3, 2024

👋 Is there any status update on this PR? Would love to see this feature land.

@another-rex
Copy link
Collaborator

another-rex commented Sep 4, 2024

@picatz Still in progress and is being actively worked on, rough estimate is around the end of October, there should be some significant progress by then. We will be using the rpmdb support from the osv-scalibr project.

@picatz
Copy link

picatz commented Sep 4, 2024

Awesome, I really appreciate the quick update @another-rex!

@jessesmd
Copy link

Please also consider the epoch related to rpm packages as they have a specific format epoch:name-version-release.arch
Thank you!

Copy link

This pull request has not had any activity for 60 days and will be automatically closed in two weeks

@github-actions github-actions bot added the stale The issue or PR is stale and pending automated closure label Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale The issue or PR is stale and pending automated closure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants