From db59dab4193322daf0c8d81b382efc52c8073a32 Mon Sep 17 00:00:00 2001 From: Spencer Schrock Date: Tue, 2 Jul 2024 10:48:22 -0700 Subject: [PATCH 1/4] add WithLogLevel option Signed-off-by: Spencer Schrock --- pkg/scorecard.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/pkg/scorecard.go b/pkg/scorecard.go index bf13c246228..5b949622824 100644 --- a/pkg/scorecard.go +++ b/pkg/scorecard.go @@ -301,14 +301,22 @@ type runConfig struct { ciiClient clients.CIIBestPracticesClient projectClient packageclient.ProjectPackageClient ossfuzzClient clients.RepoClient - checks []string commit string + logLevel sclog.Level + checks []string probes []string commitDepth int } type Option func(*runConfig) error +func WithLogLevel(level sclog.Level) Option { + return func(c *runConfig) error { + c.logLevel = level + return nil + } +} + func WithCommitDepth(depth int) Option { return func(c *runConfig) error { c.commitDepth = depth @@ -366,16 +374,16 @@ func WithOpenSSFBestPraticesClient(client clients.CIIBestPracticesClient) Option } func Run(ctx context.Context, repo clients.Repo, opts ...Option) (ScorecardResult, error) { - // TODO logger - logger := sclog.NewLogger(sclog.InfoLevel) c := runConfig{ - commit: clients.HeadSHA, + commit: clients.HeadSHA, + logLevel: sclog.DefaultLevel, } for _, option := range opts { if err := option(&c); err != nil { return ScorecardResult{}, err } } + logger := sclog.NewLogger(c.logLevel) if c.ciiClient == nil { c.ciiClient = clients.DefaultCIIBestPracticesClient() } From 196e6a5bddc99c9f9796febf4c77ab1d0616fd44 Mon Sep 17 00:00:00 2001 From: Spencer Schrock Date: Tue, 2 Jul 2024 11:14:49 -0700 Subject: [PATCH 2/4] migrate scorecard CLI to new Run entrypoint Signed-off-by: Spencer Schrock --- cmd/root.go | 67 ++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 46 insertions(+), 21 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index 8b01f216274..064db51478a 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -17,6 +17,7 @@ package cmd import ( "context" + "errors" "fmt" "os" "sort" @@ -27,6 +28,9 @@ import ( "github.com/ossf/scorecard/v5/checker" "github.com/ossf/scorecard/v5/clients" + "github.com/ossf/scorecard/v5/clients/githubrepo" + "github.com/ossf/scorecard/v5/clients/gitlabrepo" + "github.com/ossf/scorecard/v5/clients/localdir" pmc "github.com/ossf/scorecard/v5/cmd/internal/packagemanager" docs "github.com/ossf/scorecard/v5/docs/checks" sce "github.com/ossf/scorecard/v5/errors" @@ -92,16 +96,23 @@ func rootCmd(o *options.Options) error { } ctx := context.Background() - logger := sclog.NewLogger(sclog.ParseLevel(o.LogLevel)) - repoURI, repoClient, ossFuzzRepoClient, ciiClient, vulnsClient, projectClient, err := checker.GetClients( - ctx, o.Repo, o.Local, logger) // MODIFIED - if err != nil { - return fmt.Errorf("GetClients: %w", err) + opts := []pkg.Option{ + pkg.WithLogLevel(sclog.ParseLevel(o.LogLevel)), + pkg.WithCommitSHA(o.Commit), + pkg.WithCommitDepth(o.CommitDepth), } - defer repoClient.Close() - if ossFuzzRepoClient != nil { - defer ossFuzzRepoClient.Close() + var repo clients.Repo + if o.Local != "" { + repo, err = localdir.MakeLocalDirRepo(o.Local) + if err != nil { + return fmt.Errorf("making local dir: %w", err) + } + } else { + repo, err = makeRepo(o.Repo) + if err != nil { + return fmt.Errorf("making remote repo: %w", err) + } } // Read docs. @@ -117,10 +128,17 @@ func rootCmd(o *options.Options) error { if !strings.EqualFold(o.Commit, clients.HeadSHA) { requiredRequestTypes = append(requiredRequestTypes, checker.CommitBased) } + // this call to policy is different from the one in pkg.Run + // this one is concerned with a policy file, while the pkg.Run call is + // more concerned with the supported request types enabledChecks, err := policy.GetEnabled(pol, o.Checks(), requiredRequestTypes) if err != nil { return fmt.Errorf("GetEnabled: %w", err) } + checks := make([]string, 0, len(enabledChecks)) + for c := range enabledChecks { + checks = append(checks, c) + } enabledProbes := o.Probes() if o.Format == options.FormatDefault { @@ -130,20 +148,12 @@ func rootCmd(o *options.Options) error { printCheckStart(enabledChecks) } } - - repoResult, err = pkg.ExperimentalRunProbes( - ctx, - repoURI, - o.Commit, - o.CommitDepth, - enabledChecks, - enabledProbes, - repoClient, - ossFuzzRepoClient, - ciiClient, - vulnsClient, - projectClient, + opts = append(opts, + pkg.WithProbes(enabledProbes), + pkg.WithChecks(checks), ) + + repoResult, err = pkg.Run(ctx, repo, opts...) if err != nil { return fmt.Errorf("RunScorecard: %w", err) } @@ -206,3 +216,18 @@ func printCheckResults(enabledChecks checker.CheckNameToFnMap) { } fmt.Fprintln(os.Stderr, "\nRESULTS\n-------") } + +// makeRepo helps turn a URI into the appropriate clients.Repo. +// currently this is a decision between GitHub and GitLab, +// but may expand in the future. +func makeRepo(uri string) (clients.Repo, error) { + var repo clients.Repo + var errGitHub, errGitLab error + if repo, errGitHub = githubrepo.MakeGithubRepo(uri); errGitHub != nil { + repo, errGitLab = gitlabrepo.MakeGitlabRepo(uri) + if errGitLab != nil { + return nil, fmt.Errorf("unable to parse as github or gitlab: %w", errors.Join(errGitHub, errGitLab)) + } + } + return repo, nil +} From a64fec86547d4c58f5d09f03863e1cc630d18189 Mon Sep 17 00:00:00 2001 From: Spencer Schrock Date: Tue, 2 Jul 2024 11:37:47 -0700 Subject: [PATCH 3/4] delete ExperimentalRunProbes Switch the test to using the new Run function Signed-off-by: Spencer Schrock --- pkg/scorecard.go | 27 --------------------------- pkg/scorecard_test.go | 22 +++++++++------------- 2 files changed, 9 insertions(+), 40 deletions(-) diff --git a/pkg/scorecard.go b/pkg/scorecard.go index 5b949622824..7506817926b 100644 --- a/pkg/scorecard.go +++ b/pkg/scorecard.go @@ -268,33 +268,6 @@ func RunScorecard(ctx context.Context, ) } -// ExperimentalRunProbes is experimental. Do not depend on it, it may be removed at any point. -func ExperimentalRunProbes(ctx context.Context, - repo clients.Repo, - commitSHA string, - commitDepth int, - checksToRun checker.CheckNameToFnMap, - probesToRun []string, - repoClient clients.RepoClient, - ossFuzzRepoClient clients.RepoClient, - ciiClient clients.CIIBestPracticesClient, - vulnsClient clients.VulnerabilitiesClient, - projectClient packageclient.ProjectPackageClient, -) (ScorecardResult, error) { - return runScorecard(ctx, - repo, - commitSHA, - commitDepth, - checksToRun, - probesToRun, - repoClient, - ossFuzzRepoClient, - ciiClient, - vulnsClient, - projectClient, - ) -} - type runConfig struct { client clients.RepoClient vulnClient clients.VulnerabilitiesClient diff --git a/pkg/scorecard_test.go b/pkg/scorecard_test.go index 333e09613a1..e07d52dcb7c 100644 --- a/pkg/scorecard_test.go +++ b/pkg/scorecard_test.go @@ -202,7 +202,7 @@ func TestRunScorecard(t *testing.T) { } } -func TestExperimentalRunProbes(t *testing.T) { +func TestRun_WithProbes(t *testing.T) { t.Parallel() // These values depend on the environment, // so don't encode particular expectations @@ -283,7 +283,7 @@ func TestExperimentalRunProbes(t *testing.T) { repo.EXPECT().Host().Return("github.com").AnyTimes() mockRepoClient.EXPECT().InitRepo(repo, tt.args.commitSHA, 0).Return(nil) - + mockRepoClient.EXPECT().URI().Return(repo.URI()).AnyTimes() mockRepoClient.EXPECT().Close().DoAndReturn(func() error { return nil }) @@ -320,17 +320,13 @@ func TestExperimentalRunProbes(t *testing.T) { mockRepoClient.EXPECT().ListProgrammingLanguages().Return(progLanguages, nil).AnyTimes() mockRepoClient.EXPECT().GetDefaultBranchName().Return("main", nil).AnyTimes() - got, err := ExperimentalRunProbes(context.Background(), - repo, - tt.args.commitSHA, - 0, - nil, - tt.args.probes, - mockRepoClient, - nil, - nil, - nil, - nil, + mockOSSFuzzClient := mockrepo.NewMockRepoClient(ctrl) + mockOSSFuzzClient.EXPECT().Search(gomock.Any()).Return(clients.SearchResponse{}, nil).AnyTimes() + got, err := Run(context.Background(), repo, + WithRepoClient(mockRepoClient), + WithOSSFuzzClient(mockOSSFuzzClient), + WithCommitSHA(tt.args.commitSHA), + WithProbes(tt.args.probes), ) if (err != nil) != tt.wantErr { t.Errorf("RunScorecard() error = %v, wantErr %v", err, tt.wantErr) From 90f5581cb826ced428f6df7382977087cc36a85c Mon Sep 17 00:00:00 2001 From: Spencer Schrock Date: Mon, 8 Jul 2024 09:34:01 -0700 Subject: [PATCH 4/4] don't store opt slice, just call with args Signed-off-by: Spencer Schrock --- cmd/root.go | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index 064db51478a..3ff9a6fee93 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -96,11 +96,6 @@ func rootCmd(o *options.Options) error { } ctx := context.Background() - opts := []pkg.Option{ - pkg.WithLogLevel(sclog.ParseLevel(o.LogLevel)), - pkg.WithCommitSHA(o.Commit), - pkg.WithCommitDepth(o.CommitDepth), - } var repo clients.Repo if o.Local != "" { @@ -148,12 +143,14 @@ func rootCmd(o *options.Options) error { printCheckStart(enabledChecks) } } - opts = append(opts, + + repoResult, err = pkg.Run(ctx, repo, + pkg.WithLogLevel(sclog.ParseLevel(o.LogLevel)), + pkg.WithCommitSHA(o.Commit), + pkg.WithCommitDepth(o.CommitDepth), pkg.WithProbes(enabledProbes), pkg.WithChecks(checks), ) - - repoResult, err = pkg.Run(ctx, repo, opts...) if err != nil { return fmt.Errorf("RunScorecard: %w", err) }