-
Notifications
You must be signed in to change notification settings - Fork 604
🌱 Set OSV User-Agent for scorecard cli and cron workers. #4883
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
base: main
Are you sure you want to change the base?
Conversation
|
This pull request has been marked stale because it has been open for 10 days with no activity |
|
You can track the upstream issue here at google/osv-scanner#2420 We'll need to wait for a new osv-scanner release before we can set it. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4883 +/- ##
==========================================
+ Coverage 66.80% 69.68% +2.88%
==========================================
Files 230 251 +21
Lines 16602 15660 -942
==========================================
- Hits 11091 10913 -178
+ Misses 4808 3873 -935
- Partials 703 874 +171 🚀 New features to boost your workflow:
|
|
@Ly-Joey Can you take a quick look to confirm this is correct? |
|
|
+1 to this. The current strategy was based on the old package global. We might need a new flag in our osv-client representing the source, that way when we initialize the client we can pass in a string to use for the user agent. maybe something like this? And then tweaking DefaultVulnerabilitiesClient to use NewOSVClient for backwards compatibility. (and we can get rid of type OSVConfig struct {
ExperimentalLocal bool
UserAgent string
}
func NewOSVClient(config *OSVConfig) VulnerabilitiesClient {
if config == nil { // some defaults }
// store as needed
} |
|
Will look into it. |
|
@spencerschrock Have made the appropriate changes as per the reviews. |
spencerschrock
left a comment
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, just two small things!
cron/internal/worker/main.go
Outdated
| clients.NewOSVClient(&osvConfig) | ||
| flag.Parse() |
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.
this client made with the config needs to be used by the code, I think you'll want to store it here:
scorecard/cron/internal/worker/main.go
Line 132 in ade5c77
| sw.vulnsClient = clients.DefaultVulnerabilitiesClient() |
main.go
Outdated
| info := version.GetVersionInfo() | ||
| actions := osvscanner.ExperimentalScannerActions{} | ||
| config := clients.OSVConfig{} | ||
| actions.RequestUserAgent = fmt.Sprintf("scorecard-cli/%s", info.GitVersion) | ||
| config.UserAgent = actions.RequestUserAgent | ||
| clients.NewOSVClient(&config) | ||
| opts := options.New() |
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.
Lets move this all to cmd/root.go, a little above this block:
Lines 173 to 179 in ade5c77
| opts := []scorecard.Option{ | |
| scorecard.WithLogLevel(sclog.ParseLevel(o.LogLevel)), | |
| scorecard.WithCommitSHA(o.Commit), | |
| scorecard.WithCommitDepth(o.CommitDepth), | |
| scorecard.WithProbes(enabledProbes), | |
| scorecard.WithChecks(checks), | |
| } |
And then you'll need to use the client you created with NewOSVClient by creating a new opt:
scorecard.WithVulnerabilitiesClient(clients.NewOSVClient(&config)),Signed-off-by: kash2104 <kparikh1104@gmail.com>
Signed-off-by: kash2104 <kparikh1104@gmail.com>
|
@spencerschrock Have done the appropriate changes. |
What kind of change does this PR introduce?
This PR introduces improvement to OSV API request by configuring versioned User-Agent for Scorecard.
What is the current behavior?
Currently API requests to
osv.devare made without specifying a unique user-agent.What is the new behavior (if this is a feature change)?**
Now a distinct, versioned user agent is set for the OSV API request:
scorecard/{version}for CLIscorecard-cron/{version}for cron workersUses
GetId()andGetAliases()inclients/osv.goas per the latest updates in the osv scanner package.Tests for the changes have been added (for bug fixes/features)
Which issue(s) this PR fixes
Fixes #4029
Special notes for your reviewer
osv-scanner/1.9.2has been installed.Does this PR introduce a user-facing change?
No
For user-facing changes, please add a concise, human-readable release note to
the
release-note(In particular, describe what changes users might need to make in their
application as a result of this pull request.)