Add a OssFuzzRepoClient (#1280)

Co-authored-by: Azeem Shaikh <[email protected]>

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
 }

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 {

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
 }
 

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
+}

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)
 		}

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)

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()

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
 	}
 

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())
 		})
 	})
 })

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)
 	}