fix(vuln): report architecture for `apk` packages #4247

AliDatadog · 2023-05-09T13:42:44Z

Description

The architecture field was always an empty string for apk packages. This PR sets the field correctly.

Checklist

I've read the guidelines for contributing to this repository.
I've followed the conventions in the PR title.
I've added tests that prove my fix is effective or that my feature works.
I've updated the documentation with the relevant information (if needed).
I've added usage information (if the PR introduces new options)
I've included a "before" and "after" example to the description (if the PR is a user interface change).

Co-Authored-by: Sylvain Baubeau <lebauce@gmail.com>

knqyf263

Thanks for your contribution. I left a question.

knqyf263 · 2023-05-09T13:50:46Z

pkg/fanal/handler/dpkg/dpkg_test.go

What is this file for?

Thanks for sharing. I remember removing this file manually but apparently I didn't push the change.

Co-Authored-by: Sylvain Baubeau <lebauce@gmail.com>

AliDatadog · 2023-05-10T08:13:09Z

Do you have a command to generate the files under integration/testdata/*.json.golden ?

knqyf263 · 2023-05-11T10:36:59Z

Yes, you can use -update.

go test -tags=integration ./pkg/fanal/test/integration/... -update

AliDatadog · 2023-05-11T15:51:46Z

I think there might be an issue on main branch unrelated to my changes.
As you can see, the integration tests do not pass because of missing Digest for layers in alpine-310.json.golden which is unrelated to my changes.

I ran the command go test -tags=integration ./pkg/fanal/test/integration/... -update on main. It generated a golden file without this field, making the test fail on main as well.

The integration test does not pass since I merged my branch with main. I suspect the issue was introduced in #4168

knqyf263

@DmitriyLewen Can you take a look?

knqyf263 · 2023-05-14T12:15:47Z

pkg/fanal/test/integration/testdata/goldens/packages/alpine-310.json.golden

    "DependsOn": [
      "busybox@1.30.1-r2",
      "musl@1.1.22-r3"
    ],
    "Layer": {
-      "Digest": "sha256:9d48c3bd43c520dc2784e868a780e976b207cbf493eaff8c6596eb871cbd9609",


It is weird. It is gone.

Looks like it is old issue, but all contributors manually updated golden files.
I continue to investigate issue.

I created #4379 to fix this problem.

#4379 got merged.
@AliDatadog Could you merge the main branch and recreate the golden files again?

Thanks it seems to work now 👍

AliDatadog · 2023-05-17T13:45:51Z

It is unclear why the integration tests failed here.

DmitriyLewen · 2023-05-18T03:53:57Z

Hello @AliDatadog
Looks like problem with test-repo.json.golden file.
This file is empty.

How did you update golden files?
mage test:updateGolden command works correctly with this file on my local PC.

AliDatadog · 2023-05-19T10:43:35Z

Hi @DmitriyLewen, I tried to copy the test-repo.json.golden file from main and run mage test:updateGolden, it failed and deleted the file. I think the error is expected:

2023/05/19 12:38:13 Container is ready id: 33fef91ff319 image: registry:2
--- FAIL: TestTLSRegistry (2.64s)
    --- FAIL: TestTLSRegistry/happy_path (0.53s)
        registry_test.go:187: 
                Error Trace:    /Users/user/trivy/pkg/fanal/test/integration/registry_test.go:187
                Error:          Not equal: 
                                expected: false
                                actual  : true
                Test:           TestTLSRegistry/happy_path
                Messages:       2 errors occurred:
                                        * GET https://localhost:55689/v2/ghcr.io/aquasecurity/trivy-test-images/manifests/alpine-310: MANIFEST_UNKNOWN: manifest unknown; map[Tag:alpine-310]
                                        * GET https://localhost:55689/v2/ghcr.io/aquasecurity/trivy-test-images/manifests/alpine-310: UNAUTHORIZED: authentication required; [map[Action:pull Class: Name:ghcr.io/aquasecurity/trivy-test-images Type:repository]]
                            
    --- FAIL: TestTLSRegistry/happy_path_with_docker_login (0.27s)
        registry_test.go:179: 
                Error Trace:    /Users/user/trivy/pkg/fanal/test/integration/registry_test.go:179
                Error:          Received unexpected error:
                                exit status 1
                Test:           TestTLSRegistry/happy_path_with_docker_login
FAIL
FAIL    github.com/aquasecurity/trivy/pkg/fanal/test/integration        26.593s
FAIL
Error: running "go test -tags=integration ./integration/... ./pkg/fanal/test/integration/... -update" failed with exit code 1

I am running the tests with a Mac M1. Perhaps I need to install some more dependencies.

DmitriyLewen · 2023-05-22T04:26:03Z

hello @AliDatadog
Do you have an error for this test only?
mage test:integration command works correctly on your PC?

test-repo.json.golden file is only used here.

DmitriyLewen

LGTM

knqyf263 · 2023-05-22T06:51:03Z

@AliDatadog Are you still facing any issues?

AliDatadog · 2023-05-22T11:51:49Z

Indeed, this test is the only one for which I have an error and test-repo.json.golden gets deleted by mage: test:updateGolden.
I think this PR is ready to be merged now @knqyf263

knqyf263

Thanks for your efforts.

Co-authored-by: Sylvain Baubeau <lebauce@gmail.com>

add arch to apk package

45ff7cf

Co-Authored-by: Sylvain Baubeau <lebauce@gmail.com>

AliDatadog requested a review from knqyf263 as a code owner May 9, 2023 13:42

knqyf263 reviewed May 9, 2023

View reviewed changes

AliDatadog force-pushed the ali/add-arch-apk branch from 36e938c to 45ff7cf Compare May 9, 2023 14:05

AliDatadog marked this pull request as draft May 9, 2023 15:58

fix unit tests

827fdd6

Co-Authored-by: Sylvain Baubeau <lebauce@gmail.com>

update golden files Co-Authored-by: Sylvain Baubeau <lebauce@gmail.com>

1be7a1a

AliDatadog added 3 commits May 11, 2023 12:45

fix merge

c369be9

update fixtures again

6caea6a

Merge branch 'main' into ali/add-arch-apk

b4cc5f0

AliDatadog requested a review from knqyf263 May 11, 2023 18:29

AliDatadog marked this pull request as ready for review May 11, 2023 18:29

knqyf263 reviewed May 14, 2023

View reviewed changes

AliDatadog added 4 commits May 16, 2023 09:57

Merge branch 'main' into ali/add-arch-apk

14ebc70

merge with main and regenerate golden files

398053e

update golden files manually

ac614e8

now regenerate golden files again

a3e9786

AliDatadog requested a review from knqyf263 May 16, 2023 16:24

AliDatadog added 2 commits May 17, 2023 11:26

regenerate golden files

987d828

yet another golden files update

53ecf62

add manually test-repo.json.golden

f7ba876

AliDatadog requested a review from DmitriyLewen May 19, 2023 14:40

DmitriyLewen approved these changes May 22, 2023

View reviewed changes

knqyf263 approved these changes May 22, 2023

View reviewed changes

knqyf263 merged commit 63cfb27 into aquasecurity:main May 22, 2023
9 checks passed

knqyf263 pushed a commit to knqyf263/trivy that referenced this pull request May 23, 2023

fix(vuln): report architecture for apk packages (aquasecurity#4247)

a3202e0

Co-authored-by: Sylvain Baubeau <lebauce@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(vuln): report architecture for `apk` packages #4247

fix(vuln): report architecture for `apk` packages #4247

AliDatadog commented May 9, 2023

knqyf263 left a comment

knqyf263 May 9, 2023

AliDatadog May 9, 2023

AliDatadog commented May 10, 2023

knqyf263 commented May 11, 2023

AliDatadog commented May 11, 2023 •

edited

knqyf263 left a comment

knqyf263 May 14, 2023

DmitriyLewen May 15, 2023

DmitriyLewen May 15, 2023

knqyf263 May 15, 2023

AliDatadog May 16, 2023

AliDatadog commented May 17, 2023

DmitriyLewen commented May 18, 2023

AliDatadog commented May 19, 2023

DmitriyLewen commented May 22, 2023 •

edited

DmitriyLewen left a comment

knqyf263 commented May 22, 2023

AliDatadog commented May 22, 2023

knqyf263 left a comment

fix(vuln): report architecture for apk packages #4247

fix(vuln): report architecture for apk packages #4247

Conversation

AliDatadog commented May 9, 2023

Description

Checklist

knqyf263 left a comment

Choose a reason for hiding this comment

knqyf263 May 9, 2023

Choose a reason for hiding this comment

AliDatadog May 9, 2023

Choose a reason for hiding this comment

AliDatadog commented May 10, 2023

knqyf263 commented May 11, 2023

AliDatadog commented May 11, 2023 • edited

knqyf263 left a comment

Choose a reason for hiding this comment

knqyf263 May 14, 2023

Choose a reason for hiding this comment

DmitriyLewen May 15, 2023

Choose a reason for hiding this comment

DmitriyLewen May 15, 2023

Choose a reason for hiding this comment

knqyf263 May 15, 2023

Choose a reason for hiding this comment

AliDatadog May 16, 2023

Choose a reason for hiding this comment

AliDatadog commented May 17, 2023

DmitriyLewen commented May 18, 2023

AliDatadog commented May 19, 2023

DmitriyLewen commented May 22, 2023 • edited

DmitriyLewen left a comment

Choose a reason for hiding this comment

knqyf263 commented May 22, 2023

AliDatadog commented May 22, 2023

knqyf263 left a comment

Choose a reason for hiding this comment

fix(vuln): report architecture for `apk` packages #4247

fix(vuln): report architecture for `apk` packages #4247

AliDatadog commented May 11, 2023 •

edited

DmitriyLewen commented May 22, 2023 •

edited