From 2375ae2812319adc902f917cbdc51032b3290c54 Mon Sep 17 00:00:00 2001 From: Azeem Shaikh Date: Tue, 16 Nov 2021 22:04:37 -0500 Subject: [PATCH] Add a OssFuzzRepoClient (#1280) Co-authored-by: Azeem Shaikh --- checker/check_request.go | 11 ++++---- checks/cii_best_practices.go | 4 +++ checks/fuzzing.go | 32 +++--------------------- clients/githubrepo/client.go | 15 +++++++++++ cmd/root.go | 7 +++++- cmd/serve.go | 8 +++++- cron/worker/main.go | 12 ++++++--- docs/checks/internal/validate/main.go | 2 +- e2e/fuzzing_test.go | 36 ++++++++++++++++++--------- pkg/scorecard.go | 17 +++++++------ 10 files changed, 85 insertions(+), 59 deletions(-) diff --git a/checker/check_request.go b/checker/check_request.go index 3639714bc07..1c048f112d6 100644 --- a/checker/check_request.go +++ b/checker/check_request.go @@ -22,9 +22,10 @@ import ( // CheckRequest struct encapsulates all data to be passed into a CheckFn. type CheckRequest struct { - Ctx context.Context - RepoClient clients.RepoClient - CIIClient clients.CIIBestPracticesClient - Dlogger DetailLogger - Repo clients.Repo + Ctx context.Context + RepoClient clients.RepoClient + CIIClient clients.CIIBestPracticesClient + OssFuzzRepo clients.RepoClient + Dlogger DetailLogger + Repo clients.Repo } diff --git a/checks/cii_best_practices.go b/checks/cii_best_practices.go index 1f1809250df..27bdbc03264 100644 --- a/checks/cii_best_practices.go +++ b/checks/cii_best_practices.go @@ -37,6 +37,10 @@ func init() { // CIIBestPractices runs CII-Best-Practices check. func CIIBestPractices(c *checker.CheckRequest) checker.CheckResult { + if c.CIIClient == nil { + return checker.CreateInconclusiveResult(CheckCIIBestPractices, "CII client is nil") + } + // TODO: not supported for local clients. badgeLevel, err := c.CIIClient.GetBadgeLevel(c.Ctx, c.Repo.URI()) if err == nil { diff --git a/checks/fuzzing.go b/checks/fuzzing.go index d2479f20279..7aa49c7d687 100644 --- a/checks/fuzzing.go +++ b/checks/fuzzing.go @@ -16,28 +16,16 @@ package checks import ( "fmt" - "sync" - - "go.uber.org/zap" "github.com/ossf/scorecard/v3/checker" "github.com/ossf/scorecard/v3/checks/fileparser" "github.com/ossf/scorecard/v3/clients" - "github.com/ossf/scorecard/v3/clients/githubrepo" sce "github.com/ossf/scorecard/v3/errors" ) // CheckFuzzing is the registered name for Fuzzing. const CheckFuzzing = "Fuzzing" -var ( - ossFuzzRepo clients.Repo - ossFuzzRepoClient clients.RepoClient - errOssFuzzRepo error - logger *zap.Logger - once sync.Once -) - //nolint:gochecknoinits func init() { registerCheck(CheckFuzzing, Fuzzing) @@ -57,33 +45,19 @@ func checkCFLite(c *checker.CheckRequest) (bool, error) { } func checkOSSFuzz(c *checker.CheckRequest) (bool, error) { - once.Do(func() { - logger, errOssFuzzRepo = githubrepo.NewLogger(zap.InfoLevel) - if errOssFuzzRepo != nil { - return - } - ossFuzzRepo, errOssFuzzRepo = githubrepo.MakeGithubRepo("google/oss-fuzz") - if errOssFuzzRepo != nil { - return - } - ossFuzzRepoClient = githubrepo.CreateGithubRepoClient(c.Ctx, logger) - errOssFuzzRepo = ossFuzzRepoClient.InitRepo(ossFuzzRepo) - }) - if errOssFuzzRepo != nil { - e := sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("InitRepo: %v", errOssFuzzRepo)) - return false, e + if c.OssFuzzRepo == nil { + return false, nil } req := clients.SearchRequest{ Query: c.RepoClient.URI(), Filename: "project.yaml", } - result, err := ossFuzzRepoClient.Search(req) + result, err := c.OssFuzzRepo.Search(req) if err != nil { e := sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("Client.Search.Code: %v", err)) return false, e } - return result.Hits > 0, nil } diff --git a/clients/githubrepo/client.go b/clients/githubrepo/client.go index d1b8f974a5a..afc1751f033 100644 --- a/clients/githubrepo/client.go +++ b/clients/githubrepo/client.go @@ -230,3 +230,18 @@ func NewLogger(logLevel zapcore.Level) (*zap.Logger, error) { } return logger, nil } + +// CreateOssFuzzRepoClient returns a RepoClient implementation +// intialized to `google/oss-fuzz` GitHub repository. +func CreateOssFuzzRepoClient(ctx context.Context, logger *zap.Logger) (clients.RepoClient, error) { + ossFuzzRepo, err := MakeGithubRepo("google/oss-fuzz") + if err != nil { + return nil, fmt.Errorf("error during githubrepo.MakeGithubRepo: %w", err) + } + + ossFuzzRepoClient := CreateGithubRepoClient(ctx, logger) + if err := ossFuzzRepoClient.InitRepo(ossFuzzRepo); err != nil { + return nil, fmt.Errorf("error during InitRepo: %w", err) + } + return ossFuzzRepoClient, nil +} diff --git a/cmd/root.go b/cmd/root.go index ae35db80a4d..5f8977890a2 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -316,6 +316,11 @@ var rootCmd = &cobra.Command{ defer repoClient.Close() ciiClient := clients.DefaultCIIBestPracticesClient() + ossFuzzRepoClient, err := githubrepo.CreateOssFuzzRepoClient(ctx, logger) + if err != nil { + log.Fatal(err) + } + defer ossFuzzRepoClient.Close() // Read docs. checkDocs, err := docs.Read() @@ -338,7 +343,7 @@ var rootCmd = &cobra.Command{ fmt.Fprintf(os.Stderr, "Starting [%s]\n", checkName) } } - repoResult, err := pkg.RunScorecards(ctx, repoURI, enabledChecks, repoClient, ciiClient) + repoResult, err := pkg.RunScorecards(ctx, repoURI, enabledChecks, repoClient, ossFuzzRepoClient, ciiClient) if err != nil { log.Fatal(err) } diff --git a/cmd/serve.go b/cmd/serve.go index 53e3c18d2d2..1330c64e629 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -65,8 +65,14 @@ var serveCmd = &cobra.Command{ } ctx := r.Context() repoClient := githubrepo.CreateGithubRepoClient(ctx, logger) + ossFuzzRepoClient, err := githubrepo.CreateOssFuzzRepoClient(ctx, logger) + if err != nil { + sugar.Error(err) + rw.WriteHeader(http.StatusInternalServerError) + } + defer ossFuzzRepoClient.Close() ciiClient := clients.DefaultCIIBestPracticesClient() - repoResult, err := pkg.RunScorecards(ctx, repo, checks.AllChecks, repoClient, ciiClient) + repoResult, err := pkg.RunScorecards(ctx, repo, checks.AllChecks, repoClient, ossFuzzRepoClient, ciiClient) if err != nil { sugar.Error(err) rw.WriteHeader(http.StatusInternalServerError) diff --git a/cron/worker/main.go b/cron/worker/main.go index 6171abf62a4..ef5bb49fc71 100644 --- a/cron/worker/main.go +++ b/cron/worker/main.go @@ -51,7 +51,8 @@ var ignoreRuntimeErrors = flag.Bool("ignoreRuntimeErrors", false, "if set to tru func processRequest(ctx context.Context, batchRequest *data.ScorecardBatchRequest, checksToRun checker.CheckNameToFnMap, bucketURL, bucketURL2 string, checkDocs docs.Doc, - repoClient clients.RepoClient, ciiClient clients.CIIBestPracticesClient, logger *zap.Logger) error { + repoClient clients.RepoClient, ossFuzzRepoClient clients.RepoClient, + ciiClient clients.CIIBestPracticesClient, logger *zap.Logger) error { filename := data.GetBlobFilename( fmt.Sprintf("shard-%07d", batchRequest.GetShardNum()), batchRequest.GetJobTime().AsTime()) @@ -82,7 +83,7 @@ func processRequest(ctx context.Context, continue } repo.AppendMetadata(repo.Metadata()...) - result, err := pkg.RunScorecards(ctx, repo, checksToRun, repoClient, ciiClient) + result, err := pkg.RunScorecards(ctx, repo, checksToRun, repoClient, ossFuzzRepoClient, ciiClient) if errors.Is(err, sce.ErrRepoUnreachable) { // Not accessible repo - continue. continue @@ -178,6 +179,11 @@ func main() { } repoClient := githubrepo.CreateGithubRepoClient(ctx, logger) ciiClient := clients.DefaultCIIBestPracticesClient() + ossFuzzRepoClient, err := githubrepo.CreateOssFuzzRepoClient(ctx, logger) + if err != nil { + panic(err) + } + defer ossFuzzRepoClient.Close() exporter, err := startMetricsExporter() if err != nil { @@ -210,7 +216,7 @@ func main() { } if err := processRequest(ctx, req, checksToRun, bucketURL, bucketURL2, checkDocs, - repoClient, ciiClient, logger); err != nil { + repoClient, ossFuzzRepoClient, ciiClient, logger); err != nil { logger.Warn(fmt.Sprintf("error processing request: %v", err)) // Nack the message so that another worker can retry. subscriber.Nack() diff --git a/docs/checks/internal/validate/main.go b/docs/checks/internal/validate/main.go index 7c369969598..31d2fe5a55e 100644 --- a/docs/checks/internal/validate/main.go +++ b/docs/checks/internal/validate/main.go @@ -136,7 +136,7 @@ func contains(l []string, elt string) bool { func supportedInterfacesFromImplementation(checkName string, checkFiles map[string]string) ([]string, error) { // Special case. No APIs are used, // but we need the repo name for an online database lookup. - if checkName == "CII-Best-Practices" { + if checkName == checks.CheckCIIBestPractices || checkName == checks.CheckFuzzing { return []string{"GitHub"}, nil } diff --git a/e2e/fuzzing_test.go b/e2e/fuzzing_test.go index 110ccc8b6cf..d2b0ff119ad 100644 --- a/e2e/fuzzing_test.go +++ b/e2e/fuzzing_test.go @@ -35,11 +35,14 @@ var _ = Describe("E2E TEST:"+checks.CheckFuzzing, func() { repoClient := githubrepo.CreateGithubRepoClient(context.Background(), logger) err = repoClient.InitRepo(repo) Expect(err).Should(BeNil()) + ossFuzzRepoClient, err := githubrepo.CreateOssFuzzRepoClient(context.Background(), logger) + Expect(err).Should(BeNil()) req := checker.CheckRequest{ - Ctx: context.Background(), - RepoClient: repoClient, - Repo: repo, - Dlogger: &dl, + Ctx: context.Background(), + RepoClient: repoClient, + OssFuzzRepo: ossFuzzRepoClient, + Repo: repo, + Dlogger: &dl, } expected := scut.TestReturn{ Error: nil, @@ -51,6 +54,7 @@ var _ = Describe("E2E TEST:"+checks.CheckFuzzing, func() { result := checks.Fuzzing(&req) Expect(scut.ValidateTestReturn(nil, "use fuzzing", &expected, &result, &dl)).Should(BeTrue()) Expect(repoClient.Close()).Should(BeNil()) + Expect(ossFuzzRepoClient.Close()).Should(BeNil()) }) It("Should return use of ClusterFuzzLite", func() { dl := scut.TestDetailLogger{} @@ -59,11 +63,14 @@ var _ = Describe("E2E TEST:"+checks.CheckFuzzing, func() { repoClient := githubrepo.CreateGithubRepoClient(context.Background(), logger) err = repoClient.InitRepo(repo) Expect(err).Should(BeNil()) + ossFuzzRepoClient, err := githubrepo.CreateOssFuzzRepoClient(context.Background(), logger) + Expect(err).Should(BeNil()) req := checker.CheckRequest{ - Ctx: context.Background(), - RepoClient: repoClient, - Repo: repo, - Dlogger: &dl, + Ctx: context.Background(), + RepoClient: repoClient, + OssFuzzRepo: ossFuzzRepoClient, + Repo: repo, + Dlogger: &dl, } expected := scut.TestReturn{ Error: nil, @@ -75,6 +82,7 @@ var _ = Describe("E2E TEST:"+checks.CheckFuzzing, func() { result := checks.Fuzzing(&req) Expect(scut.ValidateTestReturn(nil, "use fuzzing", &expected, &result, &dl)).Should(BeTrue()) Expect(repoClient.Close()).Should(BeNil()) + Expect(ossFuzzRepoClient.Close()).Should(BeNil()) }) It("Should return no fuzzing", func() { dl := scut.TestDetailLogger{} @@ -83,11 +91,14 @@ var _ = Describe("E2E TEST:"+checks.CheckFuzzing, func() { repoClient := githubrepo.CreateGithubRepoClient(context.Background(), logger) err = repoClient.InitRepo(repo) Expect(err).Should(BeNil()) + ossFuzzRepoClient, err := githubrepo.CreateOssFuzzRepoClient(context.Background(), logger) + Expect(err).Should(BeNil()) req := checker.CheckRequest{ - Ctx: context.Background(), - RepoClient: repoClient, - Repo: repo, - Dlogger: &dl, + Ctx: context.Background(), + RepoClient: repoClient, + OssFuzzRepo: ossFuzzRepoClient, + Repo: repo, + Dlogger: &dl, } expected := scut.TestReturn{ Error: nil, @@ -99,6 +110,7 @@ var _ = Describe("E2E TEST:"+checks.CheckFuzzing, func() { result := checks.Fuzzing(&req) Expect(scut.ValidateTestReturn(nil, "no fuzzing", &expected, &result, &dl)).Should(BeTrue()) Expect(repoClient.Close()).Should(BeNil()) + Expect(ossFuzzRepoClient.Close()).Should(BeNil()) }) }) }) diff --git a/pkg/scorecard.go b/pkg/scorecard.go index 70eb32fd936..c8e53e48964 100644 --- a/pkg/scorecard.go +++ b/pkg/scorecard.go @@ -29,13 +29,14 @@ import ( func runEnabledChecks(ctx context.Context, repo clients.Repo, checksToRun checker.CheckNameToFnMap, - repoClient clients.RepoClient, ciiClient clients.CIIBestPracticesClient, + repoClient clients.RepoClient, ossFuzzRepoClient clients.RepoClient, ciiClient clients.CIIBestPracticesClient, resultsCh chan checker.CheckResult) { request := checker.CheckRequest{ - Ctx: ctx, - RepoClient: repoClient, - CIIClient: ciiClient, - Repo: repo, + Ctx: ctx, + RepoClient: repoClient, + OssFuzzRepo: ossFuzzRepoClient, + CIIClient: ciiClient, + Repo: repo, } wg := sync.WaitGroup{} for checkName, checkFn := range checksToRun { @@ -73,7 +74,9 @@ func getRepoCommitHash(r clients.RepoClient) (string, error) { func RunScorecards(ctx context.Context, repo clients.Repo, checksToRun checker.CheckNameToFnMap, - repoClient clients.RepoClient, ciiClient clients.CIIBestPracticesClient) (ScorecardResult, error) { + repoClient clients.RepoClient, + ossFuzzRepoClient clients.RepoClient, + ciiClient clients.CIIBestPracticesClient) (ScorecardResult, error) { if err := repoClient.InitRepo(repo); err != nil { // No need to call sce.WithMessage() since InitRepo will do that for us. //nolint:wrapcheck @@ -98,7 +101,7 @@ func RunScorecards(ctx context.Context, Date: time.Now(), } resultsCh := make(chan checker.CheckResult) - go runEnabledChecks(ctx, repo, checksToRun, repoClient, ciiClient, resultsCh) + go runEnabledChecks(ctx, repo, checksToRun, repoClient, ossFuzzRepoClient, ciiClient, resultsCh) for result := range resultsCh { ret.Checks = append(ret.Checks, result) }