-
Notifications
You must be signed in to change notification settings - Fork 2.7k
feat: add Bottlerocket OS package analyzer #8653
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
Conversation
Hi @0intro Trivy repository has been growing bigger and bigger lately. Can you create a new Discussion with a proposal to add |
Thanks. I've opened #8661. |
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.
Thanks for your great contribution! Since I have already heard about some needs on Bottlerocket, I think we want to move forward with this PR.
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 shrink the test data? Our policy for unit tests is not to include data that doesn't change the code path, so I think it's fine to keep it simple with just 2–3 cases, including the normal case and edge cases (if any).
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.
Thanks, I've shrunk the test data down.
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.
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
pkg/fanal/analyzer/pkg/bottlerocket/bottlerocket.go:72
- Consider wrapping the JSON unmarshal error with additional context to improve debugging clarity, such as including the source file or input length.
err = json.Unmarshal(b, &applicationInventory)
f84d18a
to
3dfed29
Compare
Can you please take a look at the test failure? |
3dfed29
to
959aa16
Compare
I've just fixed the linter issue. |
You can run |
959aa16
to
e4808c8
Compare
Surprisingly, the first pass of |
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 looks great! I left small comments.
b, err := io.ReadAll(r) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
var applicationInventory ApplicationInventory | ||
err = json.Unmarshal(b, &applicationInventory) | ||
if err != nil { | ||
return nil, err | ||
} |
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.
Is there any reason all the file content needs to be loaded?
b, err := io.ReadAll(r) | |
if err != nil { | |
return nil, err | |
} | |
var applicationInventory ApplicationInventory | |
err = json.Unmarshal(b, &applicationInventory) | |
if err != nil { | |
return nil, err | |
} | |
var applicationInventory ApplicationInventory | |
if err := json.NewDecoder(r).Decode(&applicationInventory); err != nil { | |
return nil, err | |
} |
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.
Done.
} | ||
|
||
func (a bottlerocketPkgAnalyzer) Type() analyzer.Type { | ||
return analyzer.TypeBottlerocket |
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.
nit: Other package analyzers use the name of the package manager, like rpm and apk. Since Bottlerocket is also an OS name, I would rename it to bottlerocket-package
, bottlerocket-pkg
or things like that so it will not get confused with Bottlerocket OS analyzer.
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.
This is a good point, but I haven't been able to find a good name. What do you think about bottlerocket-inventory
? The file is usually called either application inventory or software inventory in the Bottlerocket documentation.
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.
bottlerocket-inventory
sounds good to me.
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.
Done.
e4808c8
to
eeb2241
Compare
eeb2241
to
16218d2
Compare
I've renamed the |
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.
@DmitriyLewen Can you also take a look?
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 see a few things:
- I think Trivy shows warning (something like
WARN Unsupported os family="bottlerocket"
). This might confuse users. Let's create a new ospkg driver. This driver will only show the Info log that Trivy does not support vulnerability detection for bottlerocket packages (theDetect
function will simply return nil).
@knqyf263 wdyt? - I think we need to update the
purl
package.
We can add the bottlerocket purl type to Trivy (until package-url/purl-spec#454 is merged). You also need to update other functions (purlType
,LangType
, etc.)
or we can use generic type - Add Bottlerocket page in docs
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.
let's use same name as in package (bottlerocket_inventory.go) (with _
)
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.
Done.
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.
If I'm not mistaken, it's still mixed.
pkg/fanal/analyzer/pkg/bottlerocket_inventory/bottlerocket-inventory_test.go
should be pkg/fanal/analyzer/pkg/bottlerocket-inventory/bottlerocket-inventory_test.go
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.
Sorry, it should be fine now.
I thought we could document Trivy supports Bottlerocket only for SBOM, but UX would be better if we show the log message like you suggested. |
Thanks for pointing this out, I forgot to write about it in the review: |
16218d2
to
e6cfae8
Compare
@0intro do you have time to update this PR as per notes from #8653 (review)? |
Yes, I'll take a look. |
e6cfae8
to
11e5765
Compare
I've added a stub |
83f5a11
to
5cc52ee
Compare
I picked up some images from https://gallery.ecr.aws/bottlerocket/, but their OS was Amazon Linux. How can I test this PR with actual images? |
To deploy Bottlerocket on AWS, I've created an EC2 instance using |
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 had trouble enabling SSM properly when launching a Bottlerocket AMI on EC2, so I wasn’t able to access the shell. I was finally able to view Bottlerocket’s root filesystem by launching it as an instance within an EKS cluster. If there’s an easier way to do this, I’d really appreciate it if you could let me know—it would make future testing much smoother.
Sorry for the delay in commenting; it took some time to get the setup working correctly.
"aarch64-bottlerocket-linux-gnu/sys-root/usr/lib/os-release", | ||
"x86_64-bottlerocket-linux-gnu/sys-root/usr/lib/os-release", |
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.
Have you ever seen a case where /usr/lib/os-release
doesn't exist and only ${arch}-bottlerocket-linux-gnu/sys-root/usr/lib/os-release
exists? In my environment, /usr/lib/os-release
is present. If /usr/lib/os-release
is always present, we don't need to check ${arch}-bottlerocket-linux-gnu/sys-root/usr/lib/os-release
.
[root@admin]# ls -alh /.bottlerocket/rootfs/usr/lib/os-release
-rw-r--r--. 1 root root 455 Apr 23 00:29 /.bottlerocket/rootfs/usr/lib/os-release
[root@admin]# cat /.bottlerocket/rootfs/usr/lib/os-release
NAME=Bottlerocket
ID=bottlerocket
VERSION="1.37.0 (aws-k8s-1.30)"
PRETTY_NAME="Bottlerocket OS 1.37.0 (aws-k8s-1.30)"
VARIANT_ID=aws-k8s-1.30
VERSION_ID=1.37.0
BUILD_ID=e5290cd7
VENDOR_NAME="Bottlerocket"
HOME_URL="https://github.com/bottlerocket-os/bottlerocket"
SUPPORT_URL="https://github.com/bottlerocket-os/bottlerocket/discussions"
BUG_REPORT_URL="https://github.com/bottlerocket-os/bottlerocket/issues"
DOCUMENTATION_URL="https://bottlerocket.dev"
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.
Yes, ${arch}-bottlerocket-linux-gnu/sys-root/usr/lib/os-release
is mounted to /usr/lib/os-release
at run time. When Bottlerocket is running, you can access to /usr/lib/os-release
, but when mounting the Bottlerocket partition, you only have access to ${arch}-bottlerocket-linux-gnu/sys-root/usr/lib/os-release
. I'm using Trivy in this later case.
pkg := types.Package{ | ||
Arch: app.Architecture, | ||
Name: app.Name, | ||
Version: app.Version, | ||
} |
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 also need epoch. Otherwise, it leads to false detection.
{
"Name": "xfscli",
"Publisher": "bottlerocket-core-kit",
"Version": "0.0",
"Release": "1.1745352352.c92146c5.br1",
"Epoch": "1",
"InstalledTime": "2025-04-23T00:26:38Z",
"ApplicationType": "Unspecified",
"Architecture": "x86_64",
"Url": "https://github.com/bottlerocket-os/bottlerocket",
"Summary": "XFS progs cli"
},
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've just added handling of epoch.
5cc52ee
to
da46f44
Compare
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.
@DmitriyLewen Can you also take a look?
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
left small comments about comments, tests and purl package
@0intro and can you fix linter errors (mage lint:fix
and mage lint:run
commands help you)?
pkg/fanal/analyzer/pkg/bottlerocket_inventory/bottlerocket_inventory.go
Outdated
Show resolved
Hide resolved
return &Scanner{} | ||
} | ||
|
||
// Detect vulnerabilities in package using Bottlerocket scanner |
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.
This is incorrect, because we don't detect vulns for Bottlerocket pkgs.
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.
Done.
} | ||
|
||
// Detect vulnerabilities in package using Bottlerocket scanner | ||
func (s *Scanner) Detect(ctx context.Context, osVer string, repo *ftypes.Repository, pkgs []ftypes.Package) ([]types.DetectedVulnerability, error) { |
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.
Let's add Info log that Trivy currently doesn't support vulnerability detection for Bottlerocket
packages.
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.
Done.
pkg/purl/purl.go
Outdated
// Temporary type before being added in github.com/package-url/packageurl-go | ||
packageurlTypeBottlerocket = "bottlerocket" |
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.
// Temporary type before being added in github.com/package-url/packageurl-go | |
packageurlTypeBottlerocket = "bottlerocket" | |
// Temporary type before being added in github.com/package-url/packageurl-go | |
// cf. https://github.com/package-url/purl-spec/issues/454 | |
packageurlTypeBottlerocket = "bottlerocket" |
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.
Done.
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 you add small test case for bottlerrocker os-release file?
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.
Done.
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 wrote 2 tests for your changes:
diff --git a/pkg/purl/purl_test.go b/pkg/purl/purl_test.go
index 5607551da..ae00df05d 100644
--- a/pkg/purl/purl_test.go
+++ b/pkg/purl/purl_test.go
@@ -440,6 +440,38 @@ func TestNewPackageURL(t *testing.T) {
},
},
},
+ {
+ name: "bottlerocket package",
+ typ: ftypes.Bottlerocket,
+ metadata: types.Metadata{
+ OS: &ftypes.OS{
+ Family: ftypes.Bottlerocket,
+ Name: "v1.39.0",
+ },
+ },
+ pkg: ftypes.Package{
+ ID: "[email protected]",
+ Name: "glibc",
+ Version: "2.40",
+ Epoch: 1,
+ Arch: "x86_64",
+ },
+ want: &purl.PackageURL{
+ Type: "bottlerocket",
+ Name: "glibc",
+ Version: "2.40",
+ Qualifiers: packageurl.Qualifiers{
+ {
+ Key: "epoch",
+ Value: "1",
+ },
+ {
+ Key: "distro",
+ Value: "bottlerocket-v1.39.0",
+ },
+ },
+ },
+ },
}
for _, tc := range testCases {
@@ -710,6 +742,47 @@ func TestPackageURL_Package(t *testing.T) {
},
},
},
+ {
+ name: "bottlerocker with epoch",
+ pkgURL: &purl.PackageURL{
+ Type: "bottlerocket",
+ Name: "glibc",
+ Version: "2.40",
+ Qualifiers: packageurl.Qualifiers{
+ {
+ Key: "epoch",
+ Value: "1",
+ },
+ {
+ Key: "distro",
+ Value: "bottlerocket-v1.39.0",
+ },
+ },
+ },
+ wantPkg: &ftypes.Package{
+ ID: "[email protected]",
+ Name: "glibc",
+ Version: "2.40",
+ Epoch: 1,
+ Identifier: ftypes.PkgIdentifier{
+ PURL: &packageurl.PackageURL{
+ Type: "bottlerocket",
+ Name: "glibc",
+ Version: "2.40",
+ Qualifiers: packageurl.Qualifiers{
+ {
+ Key: "epoch",
+ Value: "1",
+ },
+ {
+ Key: "distro",
+ Value: "bottlerocket-v1.39.0",
+ },
+ },
+ },
+ },
+ },
+ },
{
name: "wrong epoch",
pkgURL: &purl.PackageURL{
Can you add these tests and update purl
package (add epoch)?
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.
Done.
@@ -80,6 +83,10 @@ func New(t ftypes.TargetType, metadata types.Metadata, pkg ftypes.Package) (*Pac | |||
if metadata.OS != nil { | |||
namespace = string(metadata.OS.Family) | |||
} | |||
case packageurlTypeBottlerocket: |
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.
Add bottlerocker into purlType(t)
function:
diff --git a/pkg/purl/purl.go b/pkg/purl/purl.go
index c0287cca6..7c59ff4f4 100644
--- a/pkg/purl/purl.go
+++ b/pkg/purl/purl.go
@@ -7,7 +7,7 @@ import (
cn "github.com/google/go-containerregistry/pkg/name"
version "github.com/knqyf263/go-rpm-version"
- packageurl "github.com/package-url/packageurl-go"
+ "github.com/package-url/packageurl-go"
"github.com/samber/lo"
"golang.org/x/xerrors"
@@ -489,6 +489,8 @@ func purlType(t ftypes.TargetType) string {
ftypes.OpenSUSELeap, ftypes.OpenSUSETumbleweed, ftypes.SLES, ftypes.SLEMicro, ftypes.Photon,
ftypes.Azure, ftypes.CBLMariner:
return packageurl.TypeRPM
+ case ftypes.Bottlerocket:
+ return packageurlTypeBottlerocket
case TypeOCI:
return packageurl.TypeOCI
case ftypes.Julia:
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.
Done.
da46f44
to
2197941
Compare
Thanks. I think I've done all the required changes. |
2197941
to
ff5f568
Compare
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.
@0intro Thanks for your work!
LGTM.
I left 1 comment.
And you need to update docs (OS page):
https://trivy.dev/latest/docs/coverage/os/
PS use mage docs:serve
to see your changes
PS2 fix linter errors please
dc913f1
to
e4fd63a
Compare
I've added the documentation. |
@0intro Great! |
This change adds the Bottlerocket OS package analyzer. This analyzer parses the package information provided in the application-inventory.json file, as specified on: https://bottlerocket.dev/en/os/1.37.x/concepts/variants/#software-inventory This change also defines the Bottlerocket OS family.
e4fd63a
to
f9345fb
Compare
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.
Description
This change adds the Bottlerocket OS package analyzer.
This analyzer parses the package information provided in the
application-inventory.json
file, as specified on:https://bottlerocket.dev/en/os/1.37.x/concepts/variants/#software-inventory
This change also defines the Bottlerocket OS family.
Checklist