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

Expose signing algorithm and per-file metadata #19

Merged
merged 3 commits into from
May 29, 2022

Conversation

tofay
Copy link
Contributor

@tofay tofay commented May 20, 2022

Description of changes:
This change updates the PackageInfo struct to expose the algorithm used to sign the RPM, and per file information from the RPM.

Note I've removed the BaseNames, DirIndexes and DirNames fields from PackageInfo as they are superseded by the File information (I think their only use is to derive file information). Let me know if I should add those back in for back-compatibility.

I can also split this PR into separate signing algorithm/file info PRs if you prefer.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice. I expect @wagoodman's confirmation is also required? (This contribution is a rebasing of their commits in https://github.com/anchore/go-rpmdb, which is a fork used in https://github.com/anchore/syft. I'm a syft and trivy user that is trying to remove the need for that fork to resolve anchore/syft#469 😄 )

@wagoodman
Copy link
Contributor

@tofay no permission needed, but thank you for asking -- we've been wanting to get our fork synced upstream (and deleted) this is a huge help 🙏 !

Copy link
Owner

@knqyf263 knqyf263 left a comment

Choose a reason for hiding this comment

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

The original idea of this library is to mimic the behavior of RPM as much as possible. This is because it makes it easier to keep up with upstream changes. Also, this library is basically for general usage. That is why I kept DirIndexes, etc and return installed files via function since RPMTAG_INSTFILENAMES is a virtual tag, not an actual field. From that perspective, I prefer having file attributes in PackageInfo as is and returning FileInfo through Files() or something like that. What do you think?

It is not so strong opinion. If you think my idea has a big penalty of performance or readability, I'm ok with the current implementation.

pkg/rpmdb_test.go Outdated Show resolved Hide resolved
pkg/rpmtags.go Outdated Show resolved Hide resolved
and store data used to determine this in
PackageInfo struct as is
@tofay
Copy link
Contributor Author

tofay commented May 26, 2022

I'm happy to with your suggestion (I think it makes it more readable), so I've gone with that.

Do we need to expose separate functions for getting FileInfos and just file paths?
I thought not (but don't feel strongly about it), and updated InstalledFiles to return FileInfos. Is that ok?

@tofay tofay requested a review from knqyf263 May 26, 2022 11:12
@knqyf263
Copy link
Owner

knqyf263 commented May 26, 2022

I thought not (but don't feel strongly about it), and updated InstalledFiles to return FileInfos. Is that ok?

Can we rename InstalledFiles to InstalledFileNames and keep it so that we can comply with RPMTAG_INSTFILENAMES? Then, InstalledFiles can return FileInfo.

@tofay
Copy link
Contributor Author

tofay commented May 26, 2022

Thanks for the feedback - I've now done that so there is both an InstalledFiles and an InstalledFileNames

@tofay tofay mentioned this pull request May 26, 2022
@knqyf263 knqyf263 merged commit 730f487 into knqyf263:master May 29, 2022
@knqyf263
Copy link
Owner

Thank you so much!!

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.

Support newer versions of 'rpm' that use Sqlite for the db instead of BerkeleyDB
3 participants