From 6a5816347ed8950815fd58aa81b6c2a1e60a172e Mon Sep 17 00:00:00 2001 From: Spencer Schrock Date: Mon, 8 Jul 2024 10:46:32 -0700 Subject: [PATCH] :seedling: Migrate other RunScorecard callers (#4208) * convert attestor Signed-off-by: Spencer Schrock * convert serve command Signed-off-by: Spencer Schrock * add WithLogLevel option Signed-off-by: Spencer Schrock * change e2e result test Signed-off-by: Spencer Schrock * change unit test Signed-off-by: Spencer Schrock --------- Signed-off-by: Spencer Schrock --- attestor/command/check.go | 44 ++++++++++--------------------------- cmd/serve.go | 15 +++++-------- e2e/attestor_policy_test.go | 34 +++++++++------------------- pkg/scorecard.go | 16 ++++++++++---- pkg/scorecard_e2e_test.go | 14 +++--------- pkg/scorecard_test.go | 8 ++++--- 6 files changed, 47 insertions(+), 84 deletions(-) diff --git a/attestor/command/check.go b/attestor/command/check.go index 9b715f14..ac5a8787 100644 --- a/attestor/command/check.go +++ b/attestor/command/check.go @@ -21,7 +21,6 @@ import ( "github.com/ossf/scorecard/v5/attestor/policy" "github.com/ossf/scorecard/v5/checker" - "github.com/ossf/scorecard/v5/checks" sclog "github.com/ossf/scorecard/v5/log" "github.com/ossf/scorecard/v5/pkg" ) @@ -77,7 +76,7 @@ func RunCheckWithParams(repoURL, commitSHA, policyPath string) (policy.PolicyRes } } - repo, repoClient, ossFuzzRepoClient, ciiClient, vulnsClient, projectClient, err := checker.GetClients( + repo, repoClient, ossFuzzRepoClient, ciiClient, vulnsClient, _, err := checker.GetClients( ctx, repoURL, "", logger) if err != nil { return policy.Fail, fmt.Errorf("couldn't set up clients: %w", err) @@ -85,39 +84,20 @@ func RunCheckWithParams(repoURL, commitSHA, policyPath string) (policy.PolicyRes requiredChecks := attestationPolicy.GetRequiredChecksForPolicy() - enabledChecks := map[string]checker.Check{ - checks.CheckBinaryArtifacts: { - Fn: checks.BinaryArtifacts, - }, - checks.CheckVulnerabilities: { - Fn: checks.Vulnerabilities, - }, - checks.CheckCodeReview: { - Fn: checks.CodeReview, - }, - checks.CheckPinnedDependencies: { - Fn: checks.PinningDependencies, - }, - } - - // Filter out checks that won't be needed for policy-evaluation time - for name := range enabledChecks { - if _, isRequired := requiredChecks[name]; !isRequired { - delete(enabledChecks, name) + var enabledChecks []string + for check, required := range requiredChecks { + if required { + enabledChecks = append(enabledChecks, check) } } - repoResult, err := pkg.RunScorecard( - ctx, - repo, - commitSHA, - 0, - enabledChecks, - repoClient, - ossFuzzRepoClient, - ciiClient, - vulnsClient, - projectClient, + repoResult, err := pkg.Run(ctx, repo, + pkg.WithCommitSHA(commitSHA), + pkg.WithChecks(enabledChecks), + pkg.WithRepoClient(repoClient), + pkg.WithOSSFuzzClient(ossFuzzRepoClient), + pkg.WithOpenSSFBestPraticesClient(ciiClient), + pkg.WithVulnerabilitiesClient(vulnsClient), ) if err != nil { return policy.Fail, fmt.Errorf("RunScorecard: %w", err) diff --git a/cmd/serve.go b/cmd/serve.go index 9830fd46..d7a19c3e 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -23,11 +23,8 @@ import ( "github.com/spf13/cobra" - "github.com/ossf/scorecard/v5/checks" - "github.com/ossf/scorecard/v5/clients" "github.com/ossf/scorecard/v5/clients/githubrepo" "github.com/ossf/scorecard/v5/clients/ossfuzz" - "github.com/ossf/scorecard/v5/internal/packageclient" "github.com/ossf/scorecard/v5/log" "github.com/ossf/scorecard/v5/options" "github.com/ossf/scorecard/v5/pkg" @@ -63,18 +60,16 @@ func serveCmd(o *options.Options) *cobra.Command { ctx := r.Context() repoClient := githubrepo.CreateGithubRepoClient(ctx, logger) ossFuzzRepoClient, err := ossfuzz.CreateOSSFuzzClientEager(ossfuzz.StatusURL) - vulnsClient := clients.DefaultVulnerabilitiesClient() if err != nil { logger.Error(err, "initializing clients") rw.WriteHeader(http.StatusInternalServerError) } defer ossFuzzRepoClient.Close() - ciiClient := clients.DefaultCIIBestPracticesClient() - projectClient := packageclient.CreateDepsDevClient() - checksToRun := checks.GetAll() - repoResult, err := pkg.RunScorecard( - ctx, repo, clients.HeadSHA /*commitSHA*/, o.CommitDepth, checksToRun, repoClient, - ossFuzzRepoClient, ciiClient, vulnsClient, projectClient) + repoResult, err := pkg.Run(ctx, repo, + pkg.WithCommitDepth(o.CommitDepth), + pkg.WithRepoClient(repoClient), + pkg.WithOSSFuzzClient(ossFuzzRepoClient), + ) if err != nil { logger.Error(err, "running enabled scorecard checks on repo") rw.WriteHeader(http.StatusInternalServerError) diff --git a/e2e/attestor_policy_test.go b/e2e/attestor_policy_test.go index c3c0f2b1..04602e6a 100644 --- a/e2e/attestor_policy_test.go +++ b/e2e/attestor_policy_test.go @@ -26,10 +26,8 @@ import ( "github.com/ossf/scorecard/v5/attestor/command" "github.com/ossf/scorecard/v5/attestor/policy" - "github.com/ossf/scorecard/v5/checker" - "github.com/ossf/scorecard/v5/checks" - "github.com/ossf/scorecard/v5/clients" - sclog "github.com/ossf/scorecard/v5/log" + "github.com/ossf/scorecard/v5/clients/githubrepo" + "github.com/ossf/scorecard/v5/internal/checknames" "github.com/ossf/scorecard/v5/pkg" ) @@ -226,27 +224,15 @@ var _ = Describe("E2E TEST PAT: scorecard-attestor policy", func() { func getScorecardResult(repoURL string) (pkg.ScorecardResult, error) { ctx := context.Background() - logger := sclog.NewLogger(sclog.DefaultLevel) - - enabledChecks := map[string]checker.Check{ - checks.CheckBinaryArtifacts: { - Fn: checks.BinaryArtifacts, - }, - checks.CheckVulnerabilities: { - Fn: checks.Vulnerabilities, - }, - checks.CheckCodeReview: { - Fn: checks.CodeReview, - }, - checks.CheckPinnedDependencies: { - Fn: checks.PinningDependencies, - }, + enabledChecks := []string{ + checknames.BinaryArtifacts, + checknames.Vulnerabilities, + checknames.CodeReview, + checknames.PinnedDependencies, } - repo, repoClient, ossFuzzRepoClient, ciiClient, vulnsClient, projectClient, err := checker.GetClients( - ctx, repoURL, "", logger) + repo, err := githubrepo.MakeGithubRepo(repoURL) if err != nil { - return pkg.ScorecardResult{}, fmt.Errorf("couldn't set up clients: %w", err) + return pkg.ScorecardResult{}, fmt.Errorf("couldn't set up repo: %w", err) } - - return pkg.RunScorecard(ctx, repo, clients.HeadSHA, 0, enabledChecks, repoClient, ossFuzzRepoClient, ciiClient, vulnsClient, projectClient) + return pkg.Run(ctx, repo, pkg.WithChecks(enabledChecks)) } diff --git a/pkg/scorecard.go b/pkg/scorecard.go index bf13c246..5b949622 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() } diff --git a/pkg/scorecard_e2e_test.go b/pkg/scorecard_e2e_test.go index f2357dc9..5f0cb855 100644 --- a/pkg/scorecard_e2e_test.go +++ b/pkg/scorecard_e2e_test.go @@ -25,8 +25,6 @@ import ( . "github.com/onsi/gomega" "github.com/ossf/scorecard/v5/checker" - "github.com/ossf/scorecard/v5/checks" - "github.com/ossf/scorecard/v5/clients" "github.com/ossf/scorecard/v5/clients/githubrepo" sclog "github.com/ossf/scorecard/v5/log" ) @@ -104,24 +102,18 @@ var _ = Describe("E2E TEST: RunScorecard with re-used repoClient", func() { return } ctx := context.Background() - allChecks := checks.GetAll() - isolatedLogger := sclog.NewLogger(sclog.DebugLevel) lastRepo := repos[len(repos)-1] - repo, rc, ofrc, cc, vc, dc, err := checker.GetClients(ctx, lastRepo, "", isolatedLogger) + repo, err := githubrepo.MakeGithubRepo(lastRepo) Expect(err).Should(BeNil()) - isolatedResult, err := RunScorecard(ctx, repo, clients.HeadSHA, 0, allChecks, rc, ofrc, cc, vc, dc) - Expect(err).Should(BeNil()) - - logger := sclog.NewLogger(sclog.DebugLevel) - _, rc2, ofrc2, cc2, vc2, dc2, err := checker.GetClients(ctx, repos[0], "", logger) + isolatedResult, err := Run(ctx, repo, WithLogLevel(sclog.DebugLevel)) Expect(err).Should(BeNil()) var sharedResult ScorecardResult for i := range repos { repo, err = githubrepo.MakeGithubRepo(repos[i]) Expect(err).Should(BeNil()) - sharedResult, err = RunScorecard(ctx, repo, clients.HeadSHA, 0, allChecks, rc2, ofrc2, cc2, vc2, dc2) + sharedResult, err = Run(ctx, repo, WithLogLevel(sclog.DebugLevel)) Expect(err).Should(BeNil()) } diff --git a/pkg/scorecard_test.go b/pkg/scorecard_test.go index 333e0961..c597656d 100644 --- a/pkg/scorecard_test.go +++ b/pkg/scorecard_test.go @@ -128,7 +128,7 @@ func Test_getRepoCommitHashLocal(t *testing.T) { } } -func TestRunScorecard(t *testing.T) { +func TestRun(t *testing.T) { t.Parallel() type args struct { uri string @@ -188,8 +188,10 @@ func TestRunScorecard(t *testing.T) { }, }, nil }) - defer ctrl.Finish() - got, err := RunScorecard(context.Background(), repo, tt.args.commitSHA, 0, nil, mockRepoClient, nil, nil, nil, nil) + got, err := Run(context.Background(), repo, + WithCommitSHA(tt.args.commitSHA), + WithRepoClient(mockRepoClient), + ) if (err != nil) != tt.wantErr { t.Errorf("RunScorecard() error = %v, wantErr %v", err, tt.wantErr) return