-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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(mariner): Add support for Azure Linux #7186
Conversation
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
@tofay left a couple of comments.
case "azurelinux": | ||
family = types.Azure |
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 check etc/mariner-release
file for mariner
images:
trivy/pkg/fanal/analyzer/os/mariner/mariner.go
Lines 29 to 37 in b76a725
func (a marinerOSAnalyzer) Analyze(_ context.Context, input analyzer.AnalysisInput) (*analyzer.AnalysisResult, error) { | |
foundOS, err := a.parseRelease(input.Content) | |
if err != nil { | |
return nil, xerrors.Errorf("release parse error: %w", err) | |
} | |
return &analyzer.AnalysisResult{ | |
OS: foundOS, | |
}, nil | |
} |
Does azure
contain etc/azure-release
file?
if yes - i think we can update logic of this 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.
Some Distroless images (e.g., Alpine) include only /etc/os-release, not /etc/xxx-version or /etc/xxx-release. So, I think it's better to use /etc/os-release now. I mean we can change mariner.
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.
hm... i didn't know about this.
then, i think we need to move logic for mariner-release
to 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.
Sorry I was confused. Distroless didn't even contain/etc/os-release.
, and then included /etc/os-release
and /etc/alpine-release
.
#1975
Here is another discussion about /etc/os-release
.
#3485
So, it's anyway good to get a version from /etc/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.
Both Mariner and Azure Linux include an /etc/os-release file. Mariner has a /etc/mariner-release and Azure has /etc/azurelinux-release file.
I've removed the mariner-release parsing and updated Mariner to use /etc/os-release parsing. This means that the build ID is removed from the distro in package URLs:
before that change > ExternalRef: PACKAGE-MANAGER purl pkg:rpm/cbl-mariner/[email protected]?arch=x86_64&distro=cbl-mariner-2.0.20240123
after that change > ExternalRef: PACKAGE-MANAGER purl pkg:rpm/cbl-mariner/[email protected]?arch=x86_64&distro=cbl-mariner-2.0
The PURL spec doesn't say what the distro qualifier should be for RPMs. It seems unnecessary to include the build ID since the package can be identified via the other fields.
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 don't need the assembly ID to detect vulnerabilities.
Looks like we don't need it.
@knqyf263 do you have any objections?
@tofay Can you add tests for detection of 1.0
, 2.0
and 3.0
in release_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.
Looks like we don't need it.
+1
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.
@tofay Can you add tests for detection of 1.0, 2.0 and 3.0 in release_test.go?
Done
Co-authored-by: DmitriyLewen <[email protected]>
go.mod
Outdated
@@ -393,3 +393,5 @@ require ( | |||
sigs.k8s.io/kustomize/kyaml v0.14.3-0.20230601165947-6ce0bf390ce3 // indirect | |||
sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect | |||
) | |||
|
|||
replace github.com/aquasecurity/trivy-db => github.com/tofay/trivy-db v0.0.0-20240717131741-056f5431b796 |
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.
Merged aquasecurity/trivy-db#409
@tofay Is there any public image we can access for testing? I tried |
Did you specify the tag 3.0? The following should be publicly available for testing:
|
I played around with Mariner 1.0, 2.0 and Azure 3.0 images and found no problems. |
Oh, I missed it. I could pull it. Thanks! |
go.sum
Outdated
github.com/tofay/trivy-db v0.0.0-20240717131741-056f5431b796 h1:3hwn7MNI8lYWxUd8lN7sPGCzBeahvX5i/WahXxEV/R8= | ||
github.com/tofay/trivy-db v0.0.0-20240717131741-056f5431b796/go.mod h1:0T6oy2t1Iedt+yi3Ml5cpOYp5FZT4MI1/mx+3p+PIs8= |
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.
go mod tidy
should remove this
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.
@tofay thanks for your work!
LGTM
Is this still relevant? |
Unsure if the jobs are stuck or if that outage is causing large delays. |
GitHub is back. I triggered tests. |
Co-authored-by: DmitriyLewen <[email protected]> Co-authored-by: DmitriyLewen <[email protected]>
Description
Add support for Azure Linux.
Raised this to test the changes in aquasecurity/trivy-db#409
Related issues
Related PRs
Checklist