feat: support multiple DB repositories for vulnerability and Java DB #7605

nikpivkin · 2024-09-26T11:36:57Z

Description

Usage:

./trivy image alpine:3.19 --db-repository "ghcr.io/aquasecurity/trivy-db:2","public.ecr.aws/aquasecurity/trivy-db"
./trivy image alpine:3.19 --db-repository ghcr.io/aquasecurity/trivy-db:2 --db-repository public.ecr.aws/aquasecurity/trivy-db

Example:

Downloading an artifact from another repository when receiving error code 429:

./trivy image alpine:3.19 --db-repository "ghcr.io/aquasecurity/trivy-db:2","public.ecr.aws/aquasecurity/trivy-db"
2024-10-01T14:00:39+06:00       INFO    [db] Need to update DB
2024-10-01T14:00:39+06:00       INFO    Downloading vulnerability DB...
2024-10-01T14:00:39+06:00       INFO    Downloading  artifact...        repo="ghcr.io/aquasecurity/trivy-db:2"
2024-10-01T14:00:41+06:00       ERROR   Failed to download artifact     repo="ghcr.io/aquasecurity/trivy-db:2" err="oci download error: failed to fetch the layer: GET https://ghcr.io/v2/aquasecurity/trivy-db/blobs/sha256:23d1b901e7534020d5ac5f238b090ad66dcb78afef7301ed7d8ebe6b974ab5f1: TOOMANYREQUESTS: retry-after: 1.254886ms, allowed: 44000/minute"
2024-10-01T14:00:41+06:00       INFO    Trying to download artifact from other repository...
2024-10-01T14:00:41+06:00       INFO    Downloading  artifact...        repo="public.ecr.aws/aquasecurity/trivy-db:2"
53.85 MiB / 53.85 MiB [--------------------------------------------------------------------------------------] 100.00% 1.45 MiB p/s 37s
2024-10-01T14:01:22+06:00       INFO    Artifact successfully downloaded        repo="public.ecr.aws/aquasecurity/trivy-db:2"

Related issues

Close feat: support multiple DB repositories for vulnerability and Java DB #7603

Related PRs

ci: publish trivy-java-db to ECR Public trivy-java-db#37

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).

simar7 · 2024-09-27T01:01:36Z

pkg/db/db.go

+			}
+			continue
+		}
+		return nil


Should we also have a debug log to say where the DB was downloaded from?

It's logged https://github.com/aquasecurity/trivy/pull/7605/files#diff-5da3684806835246d262230050593f460b12b6c0e3966df174e6061be0e9e575R235

I meant to say log the success of the DB. Something along the lines of "Successfully download from ... " as a debug print. But it's just a nit.

Got it now, I added d75a584

pkg/commands/operation/operation.go

DmitriyLewen · 2024-09-27T11:18:19Z

pkg/db/db.go

+	if c.artifact != nil {
+		return c.artifact.Download(ctx, dst, downloadOpt)
+	}


Do we need this?
We do same in initOCIArtifact:

trivy/pkg/db/db.go

Lines 202 to 204 in 95df470

if c.artifact != nil {

return c.artifact, nil

}

+

trivy/pkg/db/db.go

Lines 237 to 239 in 95df470

if err := a.Download(ctx, dst, downloadOpt); err != nil {

log.Error("Failed to download DB", log.String("repo", repo.String()), log.Err(err))

if i < len(c.dbRepositories)-1 {

If we already have one artifact initialized with a repository via WithOCIArtifact, there is no point in looping through repositories.

Maybe I didn't make it clear.

You have duplicate code.
Can we keep c.artifact != nil check and a.Download function in just one place?

trivy/pkg/db/db.go

Lines 226 to 228 in 95df470

if c.artifact != nil {

return c.artifact.Download(ctx, dst, downloadOpt)

}

trivy/pkg/db/db.go

Lines 202 to 204 in 95df470

if c.artifact != nil {

return c.artifact, nil

}

and

trivy/pkg/db/db.go

Line 227 in 95df470

return c.artifact.Download(ctx, dst, downloadOpt)

trivy/pkg/db/db.go

Line 237 in 95df470

if err := a.Download(ctx, dst, downloadOpt); err != nil {

@DmitriyLewen If the artifact already exists (manually created), then we don't need to create it here and we can just download it instead of trying to load it from possible repositories.

@DmitriyLewen Done b08004a

pkg/flag/db_flags.go

Signed-off-by: nikpivkin <nikita.pivkin@smartforce.io>

simar7

lgtm, just left one nit comment.

Signed-off-by: nikpivkin <nikita.pivkin@smartforce.io>

knqyf263 · 2024-09-29T11:03:38Z

I'll review it tomorrow.

Signed-off-by: nikpivkin <nikita.pivkin@smartforce.io>

DmitriyLewen

LGTM

knqyf263 · 2024-09-30T10:07:23Z

pkg/db/db.go

+
+	for i, art := range arts {
+		log.Info("Downloading vulnerability DB...", log.String("repo", art.Repository()))
+		if err := art.Download(ctx, dst, oci.DownloadOption{MediaType: dbMediaType}); err != nil {


We should try the next repository only when the error is 429 or 5xx, like this. Can we extract status code by using transport.Error with errors.As? Temporary may help.

Agreed, I think we can move this error handling when downloading an artifact.

knqyf263 · 2024-09-30T10:30:37Z

pkg/db/db.go

+	for _, repo := range c.dbRepositories {
+		a, err := c.initOCIArtifact(repo, opt)
+		if err != nil {
+			return nil, err
+		}
+		artifacts = append(artifacts, a)
+	}
+	return artifacts, nil


All OCI artifacts are always initialized now, even if the primary registry works correctly. It means we waste HTTP calls. What if delaying initialization when needed? Please let me know if I'm missing something.

diff --git a/pkg/db/db.go b/pkg/db/db.go index 46c2a0584..716dd5d71 100644 --- a/pkg/db/db.go +++ b/pkg/db/db.go @@ -42,7 +42,7 @@ var ( ) type options struct { - artifact *oci.Artifact + artifact *oci.Artifact // For testing purpose only dbRepositories []name.Reference } @@ -199,6 +199,10 @@ func (c *Client) updateDownloadedAt(ctx context.Context, dbDir string) error { } func (c *Client) initOCIArtifact(repository name.Reference, opt types.RegistryOptions) (*oci.Artifact, error) { + if c.artifact != nil { + return c.artifact, nil // For unit tests + } + art, err := oci.NewArtifact(repository.String(), c.quiet, opt) // TODO: NewArtifact never returns an error if err != nil { @@ -218,30 +222,12 @@ func (c *Client) initOCIArtifact(repository name.Reference, opt types.RegistryOp return art, nil } -func (c *Client) initArtifacts(opt types.RegistryOptions) ([]*oci.Artifact, error) { - if c.artifact != nil { - return []*oci.Artifact{c.artifact}, nil - } - - artifacts := make([]*oci.Artifact, 0, len(c.dbRepositories)) - - for _, repo := range c.dbRepositories { - a, err := c.initOCIArtifact(repo, opt) +func (c *Client) downloadDB(ctx context.Context, opt types.RegistryOptions, dst string) error { + for i, repo := range c.dbRepositories { + art, err := c.initOCIArtifact(repo, opt) if err != nil { - return nil, err + return xerrors.Errorf("failed to initialize OCI artifact: %w", err) } - artifacts = append(artifacts, a) - } - return artifacts, nil -} - -func (c *Client) downloadDB(ctx context.Context, opt types.RegistryOptions, dst string) error { - arts, err := c.initArtifacts(opt) - if err != nil { - return err - } - - for i, art := range arts { log.Info("Downloading vulnerability DB...", log.String("repo", art.Repository())) if err := art.Download(ctx, dst, oci.DownloadOption{MediaType: dbMediaType}); err != nil { log.Error("Failed to download DB", log.String("repo", art.Repository()), log.Err(err))

Client.artifact is for testing purposes only. We don't need to expand it to a slice. It means it's okay to return the same instance, but we can change it if we really need it.

@knqyf263 Artifact initialization does not cause http requests and never returns an error. https://github.com/aquasecurity/trivy/blob/main/pkg/oci/artifact.go#L60-L70 If necessary, I can do some refactoring in this PR.

Hmm. We forgot to fix the error handling. I'll open a PR now.

artifact can be passed through the WithOCIArtifact option, which is public. Can anyone use it?

Opened #7615

Merged. #7615

artifact can be passed through the WithOCIArtifact option, which is public. Can anyone use it?

@knqyf263

@nikpivkin Yes, anybody can use it, and it's actually used in several places.

trivy/pkg/db/db_test.go

Line 175 in fc6b3a7

client := db.NewClient(dbDir, true, db.WithOCIArtifact(art))

trivy/pkg/rpc/server/listen_test.go

Line 97 in 4c6e8ca

client := db.NewClient(dbDir, true, db.WithOCIArtifact(art))

knqyf263 · 2024-09-30T11:30:05Z

pkg/flag/db_flags.go

 		Name:       "db-repository",
 		ConfigName: "db.repository",
-		Default:    db.DefaultRepository,
-		Usage:      "OCI repository to retrieve trivy-db from",
+		Default:    []string{db.DefaultGHCRRepository, db.DefaultECRRepository},


We're unsure how the free tier in ECR Public works, so we should probably avoid adding ECR for now. Instead, we'll document how to use ECR Public in another PR.

Signed-off-by: nikpivkin <nikita.pivkin@smartforce.io>

pkg/module/command.go

pkg/oci/artifact.go

Signed-off-by: nikpivkin <nikita.pivkin@smartforce.io>

knqyf263

I want to improve logging a bit. I'll open another PR soon.

beltran-rubo · 2024-10-07T11:47:00Z

pkg/flag/db_flags_test.go

+			fields: fields{
+				SkipDBUpdate:     true,
+				DownloadDBOnly:   false,
+				DBRepository:     []string{"ghcr.io/aquasecurity/trivy-db:2", "gallery.ecr.aws/aquasecurity/trivy-db:2"},


The 2nd default repository is wrong, it should be "public.ecr.aws/aquasecurity/trivy-db:2"

stewartcampbell · 2024-10-08T08:10:40Z

If I add a typo to the first URL, it fails to try the second. Is that expected?

trivy --version
Version: 0.56.1

trivy image alpine:3.19 --db-repository "ghcrx.io/aquasecuridty/trivy-db:2","public.ecr.aws/aquasecurity/trivy-db:2" --debug
2024-10-08T09:07:29+01:00       DEBUG   No plugins loaded
2024-10-08T09:07:29+01:00       DEBUG   Default config file "file_path=trivy.yaml" not found, using built in values
2024-10-08T09:07:29+01:00       DEBUG   Cache dir       dir="/home/stewart/.cache/trivy"
2024-10-08T09:07:29+01:00       DEBUG   Cache dir       dir="/home/stewart/.cache/trivy"
2024-10-08T09:07:30+01:00       DEBUG   Parsed severities       severities=[UNKNOWN LOW MEDIUM HIGH CRITICAL]
2024-10-08T09:07:30+01:00       DEBUG   Ignore statuses statuses=[]
2024-10-08T09:07:30+01:00       DEBUG   [vulndb] There is no valid metadata file        err="unable to open a file: open /home/stewart/.cache/trivy/db/metadata.json: no such file or directory"
2024-10-08T09:07:30+01:00       INFO    [vulndb] Need to update DB
2024-10-08T09:07:30+01:00       DEBUG   [vulndb] No metadata file
2024-10-08T09:07:30+01:00       INFO    [vulndb] Downloading vulnerability DB...
2024-10-08T09:07:30+01:00       INFO    [vulndb] Downloading artifact...        repo="ghcrx.io/aquasecuridty/trivy-db:2"
2024-10-08T09:07:30+01:00       FATAL   Fatal error
  - init error:
    github.com/aquasecurity/trivy/pkg/commands/artifact.Run
        /home/runner/work/trivy/trivy/pkg/commands/artifact/run.go:367
  - DB error:
    github.com/aquasecurity/trivy/pkg/commands/artifact.NewRunner
        /home/runner/work/trivy/trivy/pkg/commands/artifact/run.go:119
  - failed to download vulnerability DB:
    github.com/aquasecurity/trivy/pkg/commands/operation.DownloadDB
        /home/runner/work/trivy/trivy/pkg/commands/operation/operation.go:40
  - OCI artifact error:
    github.com/aquasecurity/trivy/pkg/db.(*Client).Download
        /home/runner/work/trivy/trivy/pkg/db/db.go:158
  - failed to download vulnerability DB:
    github.com/aquasecurity/trivy/pkg/db.(*Client).downloadDB
        /home/runner/work/trivy/trivy/pkg/db/db.go:207
  - failed to download artifact from ghcrx.io/aquasecuridty/trivy-db:2:
    github.com/aquasecurity/trivy/pkg/oci.Artifacts.Download
        /home/runner/work/trivy/trivy/pkg/oci/artifact.go:236
  - OCI repository error:
    github.com/aquasecurity/trivy/pkg/oci.(*Artifact).populate
        /home/runner/work/trivy/trivy/pkg/oci/artifact.go:96
  - 1 error occurred:
        * Get "https://ghcrx.io/v2/": dial tcp: lookup ghcrx.io on 10.255.255.254:53: no such host

trivy image alpine:3.19 --db-repository ghcrx.io/aquasecurity/trivy-db:2 --db-repository public.ecr.aws/aquasecurity/trivy-db:2 --debug
2024-10-08T09:10:02+01:00       DEBUG   No plugins loaded
2024-10-08T09:10:02+01:00       DEBUG   Default config file "file_path=trivy.yaml" not found, using built in values
2024-10-08T09:10:02+01:00       DEBUG   Cache dir       dir="/home/stewart/.cache/trivy"
2024-10-08T09:10:02+01:00       DEBUG   Cache dir       dir="/home/stewart/.cache/trivy"
2024-10-08T09:10:02+01:00       DEBUG   Parsed severities       severities=[UNKNOWN LOW MEDIUM HIGH CRITICAL]
2024-10-08T09:10:02+01:00       DEBUG   Ignore statuses statuses=[]
2024-10-08T09:10:02+01:00       DEBUG   [vulndb] There is no valid metadata file        err="unable to open a file: open /home/stewart/.cache/trivy/db/metadata.json: no such file or directory"
2024-10-08T09:10:02+01:00       INFO    [vulndb] Need to update DB
2024-10-08T09:10:02+01:00       DEBUG   [vulndb] No metadata file
2024-10-08T09:10:02+01:00       INFO    [vulndb] Downloading vulnerability DB...
2024-10-08T09:10:02+01:00       INFO    [vulndb] Downloading artifact...        repo="ghcrx.io/aquasecurity/trivy-db:2"
2024-10-08T09:10:02+01:00       FATAL   Fatal error
  - init error:
    github.com/aquasecurity/trivy/pkg/commands/artifact.Run
        /home/runner/work/trivy/trivy/pkg/commands/artifact/run.go:367
  - DB error:
    github.com/aquasecurity/trivy/pkg/commands/artifact.NewRunner
        /home/runner/work/trivy/trivy/pkg/commands/artifact/run.go:119
  - failed to download vulnerability DB:
    github.com/aquasecurity/trivy/pkg/commands/operation.DownloadDB
        /home/runner/work/trivy/trivy/pkg/commands/operation/operation.go:40
  - OCI artifact error:
    github.com/aquasecurity/trivy/pkg/db.(*Client).Download
        /home/runner/work/trivy/trivy/pkg/db/db.go:158
  - failed to download vulnerability DB:
    github.com/aquasecurity/trivy/pkg/db.(*Client).downloadDB
        /home/runner/work/trivy/trivy/pkg/db/db.go:207
  - failed to download artifact from ghcrx.io/aquasecurity/trivy-db:2:
    github.com/aquasecurity/trivy/pkg/oci.Artifacts.Download
        /home/runner/work/trivy/trivy/pkg/oci/artifact.go:236
  - OCI repository error:
    github.com/aquasecurity/trivy/pkg/oci.(*Artifact).populate
        /home/runner/work/trivy/trivy/pkg/oci/artifact.go:96
  - 1 error occurred:
        * Get "https://ghcrx.io/v2/": dial tcp: lookup ghcrx.io on 10.255.255.254:53: no such host

DmitriyLewen · 2024-10-08T08:52:16Z

Hello @stewartcampbell
This is expected behavior.
We only skip the repository for 429 and 5xx errors - #7605 (comment)

stewartcampbell · 2024-10-08T08:58:58Z

Thanks @DmitriyLewen

simar7 reviewed Sep 27, 2024

View reviewed changes

nikpivkin force-pushed the multi-repo branch from ca4c363 to 95df470 Compare September 27, 2024 10:21

nikpivkin marked this pull request as ready for review September 27, 2024 10:54

nikpivkin requested review from knqyf263 and DmitriyLewen as code owners September 27, 2024 10:54

DmitriyLewen reviewed Sep 27, 2024

View reviewed changes

nikpivkin added 3 commits September 27, 2024 18:19

feat: support multiple DB repositories for vulnerability and Java DB

7b19d5d

Signed-off-by: nikpivkin <nikita.pivkin@smartforce.io>

chore: remove unnecessary logging

aea7eac

Signed-off-by: nikpivkin <nikita.pivkin@smartforce.io>

cli: update description of db flags

50ad0d6

Signed-off-by: nikpivkin <nikita.pivkin@smartforce.io>

nikpivkin force-pushed the multi-repo branch from c9aff65 to 50ad0d6 Compare September 27, 2024 12:19

simar7 self-requested a review September 28, 2024 05:04

simar7 approved these changes Sep 28, 2024

View reviewed changes

chore: add successful download logs

d75a584

Signed-off-by: nikpivkin <nikita.pivkin@smartforce.io>

knqyf263 mentioned this pull request Sep 30, 2024

release: v0.56.0 [main] #7447

Merged

refactor: remove duplicate artifact downloading

b08004a

Signed-off-by: nikpivkin <nikita.pivkin@smartforce.io>

DmitriyLewen approved these changes Sep 30, 2024

View reviewed changes

knqyf263 reviewed Sep 30, 2024

View reviewed changes

nikpivkin added 3 commits September 30, 2024 17:54

Merge remote-tracking branch 'origin' into multi-repo

d66b1a3

refactor: update download policy

73c4012

Signed-off-by: nikpivkin <nikita.pivkin@smartforce.io>

cli: remove default ECR repository

9fe203f

Signed-off-by: nikpivkin <nikita.pivkin@smartforce.io>

nikpivkin force-pushed the multi-repo branch from d19ea06 to ae50c82 Compare October 1, 2024 06:01

simplify download artifact

179e628

Signed-off-by: nikpivkin <nikita.pivkin@smartforce.io>

nikpivkin force-pushed the multi-repo branch from ae50c82 to 179e628 Compare October 1, 2024 06:13

nikpivkin requested a review from knqyf263 October 1, 2024 06:30

knqyf263 reviewed Oct 1, 2024

View reviewed changes

pkg/module/command.go Outdated Show resolved Hide resolved

pkg/oci/artifact.go Outdated Show resolved Hide resolved

refactor: use Artifacts to align with Artifact

8a960b3

Signed-off-by: nikpivkin <nikita.pivkin@smartforce.io>

knqyf263 approved these changes Oct 1, 2024

View reviewed changes

knqyf263 added this pull request to the merge queue Oct 1, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 1, 2024

knqyf263 added this pull request to the merge queue Oct 1, 2024

Merged via the queue into aquasecurity:main with commit 3562529 Oct 1, 2024
17 checks passed

beltran-rubo reviewed Oct 7, 2024

View reviewed changes

nikpivkin deleted the multi-repo branch October 8, 2024 05:59

shino mentioned this pull request Oct 8, 2024

chore(deps): bump github.com/aquasecurity/trivy from 0.55.2 to 0.56.1 future-architect/vuls#2044

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support multiple DB repositories for vulnerability and Java DB #7605

feat: support multiple DB repositories for vulnerability and Java DB #7605

nikpivkin commented Sep 26, 2024 •

edited

Loading

simar7 Sep 27, 2024

nikpivkin Sep 27, 2024

simar7 Sep 28, 2024

nikpivkin Sep 28, 2024

DmitriyLewen Sep 27, 2024

nikpivkin Sep 27, 2024

DmitriyLewen Sep 30, 2024

nikpivkin Sep 30, 2024

nikpivkin Sep 30, 2024

simar7 left a comment

knqyf263 commented Sep 29, 2024

DmitriyLewen left a comment

knqyf263 Sep 30, 2024

nikpivkin Sep 30, 2024

knqyf263 Sep 30, 2024

knqyf263 Sep 30, 2024

nikpivkin Sep 30, 2024

knqyf263 Sep 30, 2024

nikpivkin Sep 30, 2024

knqyf263 Sep 30, 2024

knqyf263 Sep 30, 2024

nikpivkin Sep 30, 2024

knqyf263 Oct 1, 2024 •

edited

Loading

knqyf263 Sep 30, 2024

knqyf263 left a comment

beltran-rubo Oct 7, 2024

stewartcampbell commented Oct 8, 2024 •

edited

Loading

DmitriyLewen commented Oct 8, 2024

stewartcampbell commented Oct 8, 2024

	if err := a.Download(ctx, dst, downloadOpt); err != nil {
	log.Error("Failed to download DB", log.String("repo", repo.String()), log.Err(err))
	if i < len(c.dbRepositories)-1 {

	if c.artifact != nil {
	return c.artifact.Download(ctx, dst, downloadOpt)
	}

feat: support multiple DB repositories for vulnerability and Java DB #7605

feat: support multiple DB repositories for vulnerability and Java DB #7605

Conversation

nikpivkin commented Sep 26, 2024 • edited Loading

Description

Usage:

Related issues

Related PRs

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simar7 left a comment

Choose a reason for hiding this comment

knqyf263 commented Sep 29, 2024

DmitriyLewen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

knqyf263 Oct 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

knqyf263 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stewartcampbell commented Oct 8, 2024 • edited Loading

DmitriyLewen commented Oct 8, 2024

stewartcampbell commented Oct 8, 2024

nikpivkin commented Sep 26, 2024 •

edited

Loading

knqyf263 Oct 1, 2024 •

edited

Loading

stewartcampbell commented Oct 8, 2024 •

edited

Loading