feat: add Bottlerocket OS package analyzer#8653
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. |
knqyf263
left a comment
There was a problem hiding this comment.
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.
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.
Thanks, I've shrunk the test data down.
There was a problem hiding this comment.
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 |
knqyf263
left a comment
There was a problem hiding this comment.
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.
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 | |
| } |
| } | ||
|
|
||
| func (a bottlerocketPkgAnalyzer) Type() analyzer.Type { | ||
| return analyzer.TypeBottlerocket |
There was a problem hiding this comment.
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.
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.
bottlerocket-inventory sounds good to me.
e4808c8 to
eeb2241
Compare
eeb2241 to
16218d2
Compare
|
I've renamed the |
knqyf263
left a comment
There was a problem hiding this comment.
LGTM.
@DmitriyLewen Can you also take a look?
There was a problem hiding this comment.
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 (theDetectfunction will simply return nil).
@knqyf263 wdyt? - I think we need to update the
purlpackage.
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.
let's use same name as in package (bottlerocket_inventory.go) (with _)
There was a problem hiding this comment.
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.
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 |
knqyf263
left a comment
There was a problem hiding this comment.
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.
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.
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.
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.
I've just added handling of epoch.
5cc52ee to
da46f44
Compare
knqyf263
left a comment
There was a problem hiding this comment.
@DmitriyLewen Can you also take a look?
There was a problem hiding this comment.
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.
This is incorrect, because we don't detect vulns for Bottlerocket pkgs.
| } | ||
|
|
||
| // 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.
Let's add Info log that Trivy currently doesn't support vulnerability detection for Bottlerocket packages.
| // Temporary type before being added in github.com/package-url/packageurl-go | ||
| packageurlTypeBottlerocket = "bottlerocket" |
There was a problem hiding this comment.
| // 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.
Can you add small test case for bottlerrocker os-release file?
There was a problem hiding this comment.
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: "glibc@2.40",
+ 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: "glibc@2.40",
+ 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)?
| @@ -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.
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:
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.
@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
Description
This change adds the Bottlerocket OS package analyzer.
This analyzer parses the package information provided in the
application-inventory.jsonfile, as specified on:https://bottlerocket.dev/en/os/1.37.x/concepts/variants/#software-inventory
This change also defines the Bottlerocket OS family.
Checklist