-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
feat: support RPM archives #7628
Conversation
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
@DmitriyLewen Some tests are failing due to 429 from GHCR, but it's ready for review. |
Signed-off-by: knqyf263 <knqyf263@gmail.com>
sourceRpm, err := h.GetString(rpmutils.SOURCERPM) | ||
if a.unexpectedError(err) != nil { | ||
return types.Package{}, xerrors.Errorf("failed to get source rpm: %w", err) | ||
} | ||
srcName, srcVer, srcRel, err := splitFileName(sourceRpm) | ||
if err != nil { | ||
a.logger.Debug("Invalid Source RPM Found", log.String("sourcerpm", sourceRpm)) | ||
} |
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.
can we skip splitFileName
If SOURCERPM
is not found?
(to avoid splitFileName
error)
as for rpm:
trivy/pkg/fanal/analyzer/pkg/rpm/rpm.go
Lines 130 to 136 in 096fcd0
if pkg.SourceRpm != "(none)" && pkg.SourceRpm != "" { | |
// source epoch is not included in SOURCERPM | |
srcName, srcVer, srcRel, err = splitFileName(pkg.SourceRpm) | |
if err != nil { | |
log.DebugContext(ctx, "Invalid Source RPM Found", log.String("sourcerpm", pkg.SourceRpm)) | |
} | |
} |
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.
Fixed in c1fa209
func (a *rpmArchiveAnalyzer) generatePURL(pkg *types.Package) *packageurl.PackageURL { | ||
vendor := strings.ToLower(pkg.Maintainer) | ||
|
||
// TODO: Handle more vendors | ||
var ns string | ||
switch { | ||
case strings.Contains(vendor, "red hat"): | ||
ns = "redhat" | ||
case strings.Contains(vendor, "fedora"): | ||
ns = "fedora" | ||
case strings.Contains(vendor, "opensuse"): | ||
ns = "opensuse" | ||
case strings.Contains(vendor, "suse"): | ||
ns = "suse" | ||
} | ||
return packageurl.NewPackageURL(packageurl.TypeRPM, ns, pkg.Name, utils.FormatVersion(*pkg), nil, "") | ||
} |
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.
IIUC we will overwrite this purl if OS
is detected:
trivy/pkg/fanal/applier/docker.go
Lines 223 to 225 in 096fcd0
if mergedLayer.OS.Family != "" { | |
mergedLayer.Packages[i].Identifier.PURL = newPURL(mergedLayer.OS.Family, types.Metadata{OS: &mergedLayer.OS}, pkg) | |
} |
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.
We need to generate PURL here as OS is not detected when scanning RPM files.
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.
I agree that we need to generate PURL here.
I am worried about case when we scan image that containing *.rpm
file, and OS
and vendor of this file are not equal.
In this case we will use namespace from OS.
(example): fedora
image contains socat-1.7.3.2-2.el7.x86_64.rpm
file (for RedHat).
in analyzer we use redhat
namespace.
But in applier
we will overwrite purl and use fedora
namespace.
I meant that perhaps we need to add purl == nil
check in applier.
UPD:
➜ cat Dockerfile
FROM centos
COPY socat-1.7.3.2-2.el7.x86_64.rpm socat-1.7.3.2-2.el7.x86_64.rpm
➜ TRIVY_EXPERIMENTAL_RPM_ARCHIVE=true ./trivy -q image -f json --list-all-pkgs rpm-archives | grep socat@1.7.3.2
"PURL": "pkg:rpm/centos/socat@1.7.3.2-2.el7?arch=x86_64\u0026distro=centos-8.4.2105",
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.
It makes sense. Added 9c26be0
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
I found another problem:
It works only when
See trivy/pkg/scanner/local/scan.go Lines 165 to 166 in 4926da7
|
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Nice catch. I added a note. |
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.
LGTM
Description
Add support for RPM files. It's disabled unless
TRIVY_EXPERIMENTAL_RPM_ARCHIVE
is set as it's experimental.Details
TODOs
Checklist