-
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(db): Add dbRepository flag to get advisory database from OCI registry #1873
Conversation
…istry Author: Sashi Kumar<[email protected]>
90b8f47
to
bd243bc
Compare
pkg/db/db.go
Outdated
@@ -132,13 +129,13 @@ func (c *Client) isNewDB(meta metadata.Metadata) bool { | |||
} | |||
|
|||
// Download downloads the DB file | |||
func (c *Client) Download(ctx context.Context, dst string) error { | |||
func (c *Client) Download(ctx context.Context, dst string, dbRepository string) 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.
What if we add WithDBRepository here?
Lines 26 to 46 in bd243bc
type options struct { | |
artifact *oci.Artifact | |
clock clock.Clock | |
} | |
// Option is a functional option | |
type Option func(*options) | |
// WithOCIArtifact takes an OCI artifact | |
func WithOCIArtifact(art *oci.Artifact) Option { | |
return func(opts *options) { | |
opts.artifact = art | |
} | |
} | |
// WithClock takes a clock | |
func WithClock(clock clock.Clock) Option { | |
return func(opts *options) { | |
opts.clock = clock | |
} | |
} |
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.
Good idea! I've updated it 👍
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 the update.
pkg/rpc/server/listen.go
Outdated
@@ -50,7 +51,7 @@ func (s Server) ListenAndServe(serverCache cache.Cache) error { | |||
dbUpdateWg := &sync.WaitGroup{} | |||
|
|||
go func() { | |||
worker := newDBWorker(dbc.NewClient(s.cacheDir, true)) | |||
worker := newDBWorker(dbc.NewClient(s.cacheDir, true, dbc.WithDBRepository(dbRepository))) |
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.
Then, we don't have to pass the default value here.
pkg/db/db_test.go
Outdated
@@ -206,7 +207,7 @@ func TestClient_Download(t *testing.T) { | |||
art, err := oci.NewArtifact("db", mediaType, true, oci.WithImage(img)) | |||
require.NoError(t, err) | |||
|
|||
client := db.NewClient(cacheDir, true, db.WithOCIArtifact(art), db.WithClock(timeDownloadedAt)) | |||
client := db.NewClient(cacheDir, true, db.WithOCIArtifact(art), db.WithClock(timeDownloadedAt), db.WithDBRepository(dbRepository)) |
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.
ditto
pkg/db/db.go
Outdated
@@ -171,7 +178,7 @@ func (c *Client) updateDownloadedAt(dst string) error { | |||
return nil | |||
} | |||
|
|||
func (c *Client) populateOCIArtifact() error { | |||
func (c *Client) populateOCIArtifact(dbRepository string) 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.
We have dbRepository
as a member of Client
. Do we need to pass it as an argument? We can access c.dbRepository
in this method.
pkg/db/db.go
Outdated
@@ -171,7 +178,7 @@ func (c *Client) updateDownloadedAt(dst string) error { | |||
return nil | |||
} | |||
|
|||
func (c *Client) populateOCIArtifact() error { | |||
func (c *Client) populateOCIArtifact(dbRepository string) error { | |||
if c.artifact == nil { | |||
repo := fmt.Sprintf("%s:%d", dbRepository, db.SchemaVersion) |
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.
Like
repo := fmt.Sprintf("%s:%d", dbRepository, db.SchemaVersion) | |
repo := fmt.Sprintf("%s:%d", c.dbRepository, db.SchemaVersion) |
pkg/commands/operation/operation.go
Outdated
@@ -106,6 +106,7 @@ func DownloadDB(appVersion, cacheDir string, quiet, skipUpdate bool) error { | |||
if needsUpdate { | |||
log.Logger.Info("Need to update DB") | |||
log.Logger.Info("Downloading DB...") | |||
log.Logger.Infof("Repository: %s", dbRepository) |
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: I want to put it before Downloading DB...
.
62d86e7
to
1dceb4f
Compare
@knqyf263 Thanks for the great comments, I've addressed them. Could you please take a look again? |
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 great! Thanks for the contribution!
I'll fix the lint issues |
Thanks for helping me to merge this 👍 🙏 |
…istry (#1873) Co-authored-by: knqyf263 <[email protected]>
Description
This MR adds a new option
--db-repository
to download the vulnerability database from an external OCI registry. This will be helpful for GitLab's Cluster Image Scanning to use GitLab's Advisory DB.Related issue in GitLab: #350232
Checklist