Add RPM image scanner #342
Conversation
k8s-ci-robot
added
kind/feature
k8s-ci-robot
added
the
size/XL
micahhausler
force-pushed
the
rpm-scanner
branch
2 times, most recently
from
86418a7 to
e1d5855
Compare
There was a problem hiding this comment.
That's pretty cool, thank you for the contribution!
@puerco PTAL
/hold
k8s-ci-robot
added
the
do-not-merge/hold
k8s-ci-robot
added
lgtm
micahhausler
force-pushed
the
rpm-scanner
branch
from
e1d5855 to
d4fae86
Compare
k8s-ci-robot
removed
the
lgtm
|
I rebased to include the fix in #343 |
There was a problem hiding this comment.
This is awesome @micahhausler !
The oldest branch I had on my machine was work in progress to implement this. It has finally happened, thank you Micah 🥲
I left a question there, but in it goes! I think it is time to cut a new relase 🚀
| Type: "rpm", | ||
| // Namespace is set later | ||
| MaintainerName: p.Vendor, | ||
| // Most RPM pacakges don't have SPDX-valid license names |
There was a problem hiding this comment.
Curious: Is this true for all distros? Are they using an alternative naming scheme that could be translated later?
There was a problem hiding this comment.
This potentially affects other distros, I haven't looked at APK or DEB for validation. I spent a lot of time digging into this and found that basically the story is not great:
- If you stuff a SPDX-invalid license name into the field, the SPDX doc fails to verify
- Some distros like Fedora are working to migrate license names to SPDX license strings. This effort is not complete yet for Fedora. Fedora has a
license-fedora2spdxpackage (Python scripts/ config) that tries to do convert license name strings, but fails pretty hard in a lot of cases. Ex:BSDwas Fedora-valid, but the tool spits out something like"BSD": "Warning: more options on how to interpret BSD. Possible options: ['BSD-1-Clause', 'BSD-2-Clause-FreeBSD', 'BSD-2-Clause-Views', 'BSD-2-Clause', 'BSD-3-Clause-Modification', 'BSD-3-Clause']which requires more info. - There isn't a documented way to parse the RPM database to produce the equivalent of
rpm -q --licensefiles $PACKAGEand get the list of license files in the image.- Most distros seem to put license files in
/usr/share/licenses/$PACKAGEwith multiple different filenames (Try runningfind /usr/share/licenses -type f -exec basename {} \; | sort |uniq | wc -lin any RPM-based container distro, you'll get >40 different names). Even if you ranrpm -qL $PKG, or just searched in the directory and tried to analyze the license files, you wouldn't know if there's anANDvs anORfor a given package. (ANDtypically indicating valid SPDX license vsandbeing a legacy license name. Tryrpm -qa --queryformat '%{name} %{license}\n' | grep -E ' (and|or) 'to find those legacy ones. )
- Most distros seem to put license files in
We could try calling Reader.LicenseFromLabel() and only including valid ones, but for now I just mirrored what DEB and APK parsers do.
| @@ -56,6 +56,8 @@ func ReadOSPackages(layers []string) ( | |||
| cs = newDebianScanner() | |||
| case OSAlpine, OSWolfi: | |||
| cs = newAlpineScanner() | |||
| case OSAmazonLinux, OSFedora, OSRHEL: | |||
There was a problem hiding this comment.
nit for later: we can also add opensuse ! 🦎
(Not actual chameleon emoji, but close :)
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: micahhausler, puerco, saschagrunert The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/hold cancel |
k8s-ci-robot
removed
the
do-not-merge/hold
What type of PR is this?
/kind feature
What this PR does / why we need it:
Adds support for scanning RPM databases
Which issue(s) this PR fixes:
None
Special notes for your reviewer:
I split layer scanning tests into separate files
Does this PR introduce a user-facing change?