Skip to content
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(metadata): add vulnerability-database-updated-at in metadata #12

Merged
merged 1 commit into from
Nov 12, 2019

Conversation

heww
Copy link
Collaborator

@heww heww commented Nov 11, 2019

returns harbor.scanner-adapter/vulnerability-database-updated-at in metadata when config clair database url for the scanner

Signed-off-by: He Weiwei hweiwei@vmware.com

@codecov-io
Copy link

codecov-io commented Nov 11, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@b2b178f). Click here to learn what that means.
The diff coverage is 17.39%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master      #12   +/-   ##
=========================================
  Coverage          ?   51.75%           
=========================================
  Files             ?       13           
  Lines             ?      541           
  Branches          ?        0           
=========================================
  Hits              ?      280           
  Misses            ?      244           
  Partials          ?       17
Impacted Files Coverage Δ
pkg/etc/config.go 44.44% <0%> (ø)
pkg/clair/client.go 0% <0%> (ø)
pkg/http/api/v1/handler.go 92.59% <72.72%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b2b178f...85ce953. Read the comment docs.

Copy link
Collaborator

@steven-zou steven-zou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do changes

@@ -33,6 +33,7 @@ Configuration of the adapter is done via environment variables at startup.
| `SCANNER_API_SERVER_WRITE_TIMEOUT` | `15s` | The maximum duration before timing out writes of the response. |
| `SCANNER_API_SERVER_IDLE_TIMEOUT` | `60s` | The maximum amount of time to wait for the next request when keep-alives are enabled. |
| `SCANNER_CLAIR_URL` | `http://harbor-harbor-clair:6060` | Clair URL |
| `SCANNER_CLAIR_DATABASE_URL` | | The Clair database URL, it is used to fetch vulnerability database updated time of the Clair. Its format is `postgresql://user:password@host/db?sslmode=disable` |
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

| | two columns?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are three columns in the table, the middle one is the default value of the config, for SCANNER_CLAIR_DATABASE_URL is empty string which means not return the vulnerability database updated time in the metadata of the scanner.

endpointURL string
// need to customize the logger to write output to job log.
client *http.Client
}

// NewClient constructs a new client for Clair REST API pointing to the specified endpoint URL.
func NewClient(tlsConfig etc.TLSConfig, cfg etc.ClairConfig) Client {
var db *sql.DB
if cfg.DatabaseURL != "" {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should return an error here as CVE timestamp is an optional feature. Logging it is ok.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this feature enabled only clair database url configed, return error is ok when users enable this feature but adapter can not connect the db

"net/http"
"net/http/httptest"
"strings"
"testing"
"time"

"github.com/goharbor/harbor-scanner-clair/pkg/harbor"
"github.com/goharbor/harbor-scanner-clair/pkg/job"

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra line

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@heww heww force-pushed the vulnerability-database-updated-at branch from 03cdeab to e12025a Compare November 11, 2019 10:46
danielpacak
danielpacak previously approved these changes Nov 11, 2019
Copy link
Contributor

@danielpacak danielpacak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀

steven-zou
steven-zou previously approved these changes Nov 12, 2019
Copy link
Collaborator

@steven-zou steven-zou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Signed-off-by: He Weiwei <hweiwei@vmware.com>
@heww heww dismissed stale reviews from steven-zou and danielpacak via 85ce953 November 12, 2019 05:23
@heww heww force-pushed the vulnerability-database-updated-at branch from e12025a to 85ce953 Compare November 12, 2019 05:23
Copy link
Collaborator

@steven-zou steven-zou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@steven-zou
Copy link
Collaborator

as we both approved this PR, let's merge it.

@steven-zou steven-zou merged commit a52017b into goharbor:master Nov 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants