Add v0 of RepoClient interface (#587)

Co-authored-by: Azeem Shaikh <azeems@google.com>

checker/check_request.go

@@ -20,6 +20,8 @@ import (
 
 	"github.com/google/go-github/v32/github"
 	"github.com/shurcooL/githubv4"
+
+	"github.com/ossf/scorecard/clients"
 )
 
 type CheckRequest struct {
@@ -27,6 +29,7 @@ type CheckRequest struct {
 	Client      *github.Client
 	GraphClient *githubv4.Client
 	HTTPClient  *http.Client
+	RepoClient  clients.RepoClient
 	Logf        func(s string, f ...interface{})
 	Owner, Repo string
 }

checks/checkforcontent.go

@@ -16,11 +16,11 @@ package checks
 
 import (
 	"archive/tar"
+	"bytes"
 	"compress/gzip"
 	"errors"
 	"fmt"
 	"io"
-	"net/http"
 	"path"
 	"strings"
 
@@ -81,20 +81,8 @@ func extractFullpath(fn string) (string, bool) {
 	return fullpath, true
 }
 
-// Using the http.get instead of the lib httpClient because
-// the default checker.HTTPClient caches everything in the memory and it causes oom.
-func getHTTPResponse(url string) (*http.Response, error) {
-	//https://securego.io/docs/rules/g107.html
-	//nolint
-	resp, err := http.Get(url)
-	if err != nil {
-		return nil, fmt.Errorf("get request failed: %w", err)
-	}
-	return resp, nil
-}
-
-func getTarReader(resp *http.Response) (*tar.Reader, error) {
-	gz, err := gzip.NewReader(resp.Body)
+func getTarReader(in io.Reader) (*tar.Reader, error) {
+	gz, err := gzip.NewReader(in)
 	if err != nil {
 		return nil, fmt.Errorf("gzip reader failed: %w", err)
 	}
@@ -115,20 +103,7 @@ func CheckFilesContent(checkName, shellPathFnPattern string,
 	onFileContent func(path string, content []byte,
 		Logf func(s string, f ...interface{})) (bool, error),
 ) checker.CheckResult {
-	r, _, err := c.Client.Repositories.Get(c.Ctx, c.Owner, c.Repo)
-	if err != nil {
-		return checker.MakeRetryResult(checkName, err)
-	}
-	url := r.GetArchiveURL()
-	url = strings.Replace(url, "{archive_format}", "tarball/", 1)
-	url = strings.Replace(url, "{/ref}", r.GetDefaultBranch(), 1)
-
-	resp, err := getHTTPResponse(url)
-	if err != nil {
-		return checker.MakeRetryResult(checkName, err)
-	}
-
-	tr, err := getTarReader(resp)
+	tr, err := getTarReader(bytes.NewReader(c.RepoClient.GetRepoArchive()))
 	if err != nil {
 		return checker.MakeRetryResult(checkName, err)
 	}

checks/checkforfile.go

@@ -16,10 +16,10 @@ package checks
 
 import (
 	"archive/tar"
+	"bytes"
 	"compress/gzip"
 	"errors"
 	"io"
-	"net/http"
 	"strings"
 
 	"github.com/ossf/scorecard/checker"
@@ -29,26 +29,7 @@ import (
 // for the occurrence.
 func CheckIfFileExists(checkName string, c *checker.CheckRequest, onFile func(name string,
 	Logf func(s string, f ...interface{})) (bool, error)) checker.CheckResult {
-	r, _, err := c.Client.Repositories.Get(c.Ctx, c.Owner, c.Repo)
-	if err != nil {
-		return checker.MakeRetryResult(checkName, err)
-	}
-	url := r.GetArchiveURL()
-	url = strings.Replace(url, "{archive_format}", "tarball/", 1)
-	url = strings.Replace(url, "{/ref}", r.GetDefaultBranch(), 1)
-
-	// Using the http.get instead of the lib httpClient because
-	// the default checker.HTTPClient caches everything in the memory and it causes oom.
-
-	//https://securego.io/docs/rules/g107.html
-	//nolint
-	resp, err := http.Get(url)
-	if err != nil {
-		return checker.MakeRetryResult(checkName, err)
-	}
-	defer resp.Body.Close()
-
-	gz, err := gzip.NewReader(resp.Body)
+	gz, err := gzip.NewReader(bytes.NewReader(c.RepoClient.GetRepoArchive()))
 	if err != nil {
 		return checker.MakeRetryResult(checkName, err)
 	}

clients/githubrepo/client.go

@@ -0,0 +1,78 @@
+// Copyright 2021 Security Scorecard Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package githubrepo
+
+import (
+	"context"
+	"fmt"
+	"io/ioutil"
+	"net/http"
+	"strings"
+
+	"github.com/google/go-github/v32/github"
+
+	"github.com/ossf/scorecard/clients"
+)
+
+type Client struct {
+	repo        *github.Repository
+	repoClient  *github.Client
+	ctx         context.Context
+	owner       string
+	repoName    string
+	archiveData []byte
+}
+
+func (client *Client) InitRepo(owner, repoName string) error {
+	client.owner = owner
+	client.repoName = repoName
+	repo, _, err := client.repoClient.Repositories.Get(client.ctx, client.owner, client.repoName)
+	if err != nil {
+		return fmt.Errorf("error during Repositories.Get: %w", err)
+	}
+	client.repo = repo
+
+	url := client.repo.GetArchiveURL()
+	url = strings.Replace(url, "{archive_format}", "tarball/", 1)
+	url = strings.Replace(url, "{/ref}", client.repo.GetDefaultBranch(), 1)
+
+	req, err := http.NewRequestWithContext(client.ctx, http.MethodGet, url, nil)
+	if err != nil {
+		return fmt.Errorf("error during http.NewRequestWithContext: %w", err)
+	}
+	resp, err := http.DefaultClient.Do(req)
+	if err != nil {
+		return fmt.Errorf("%w", err)
+	}
+	defer resp.Body.Close()
+
+	respData, err := ioutil.ReadAll(resp.Body)
+	if err != nil {
+		return fmt.Errorf("%w", err)
+	}
+	client.archiveData = respData
+	return nil
+}
+
+func (client *Client) GetRepoArchive() []byte {
+	return client.archiveData
+}
+
+func CreateGithubRepoClient(ctx context.Context, client *github.Client) clients.RepoClient {
+	return &Client{
+		ctx:        ctx,
+		repoClient: client,
+	}
+}

clients/repo_client.go

@@ -0,0 +1,20 @@
+// Copyright 2021 Security Scorecard Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package clients
+
+type RepoClient interface {
+	InitRepo(owner, repo string) error
+	GetRepoArchive() []byte
+}

cmd/root.go

@@ -34,6 +34,7 @@ import (
 
 	"github.com/ossf/scorecard/checker"
 	"github.com/ossf/scorecard/checks"
+	"github.com/ossf/scorecard/clients/githubrepo"
 	"github.com/ossf/scorecard/pkg"
 	"github.com/ossf/scorecard/repos"
 	"github.com/ossf/scorecard/roundtripper"
@@ -133,7 +134,8 @@ or ./scorecard --{npm,pypi,rubgems}=<package_name> [--checks=check1,...] [--show
 		}
 		githubClient := github.NewClient(httpClient)
 		graphClient := githubv4.NewClient(httpClient)
-		repoResult := pkg.RunScorecards(ctx, repo, enabledChecks, httpClient, githubClient, graphClient)
+		repoClient := githubrepo.CreateGithubRepoClient(ctx, githubClient)
+		repoResult := pkg.RunScorecards(ctx, repo, enabledChecks, repoClient, httpClient, githubClient, graphClient)
 		repoResult.Metadata = append(repoResult.Metadata, metaData...)
 
 		// Sort them by name

cmd/serve.go

@@ -28,6 +28,7 @@ import (
 	"go.uber.org/zap"
 
 	"github.com/ossf/scorecard/checks"
+	"github.com/ossf/scorecard/clients/githubrepo"
 	"github.com/ossf/scorecard/pkg"
 	"github.com/ossf/scorecard/repos"
 	"github.com/ossf/scorecard/roundtripper"
@@ -76,7 +77,8 @@ var serveCmd = &cobra.Command{
 			}
 			githubClient := github.NewClient(httpClient)
 			graphClient := githubv4.NewClient(httpClient)
-			repoResult := pkg.RunScorecards(ctx, repo, checks.AllChecks, httpClient, githubClient, graphClient)
+			repoClient := githubrepo.CreateGithubRepoClient(ctx, githubClient)
+			repoResult := pkg.RunScorecards(ctx, repo, checks.AllChecks, repoClient, httpClient, githubClient, graphClient)
 
 			if r.Header.Get("Content-Type") == "application/json" {
 				if err := repoResult.AsJSON(showDetails, rw); err != nil {

cron/worker/main.go

@@ -32,6 +32,8 @@ import (
 
 	"github.com/ossf/scorecard/checker"
 	"github.com/ossf/scorecard/checks"
+	"github.com/ossf/scorecard/clients"
+	"github.com/ossf/scorecard/clients/githubrepo"
 	"github.com/ossf/scorecard/cron/config"
 	"github.com/ossf/scorecard/cron/data"
 	"github.com/ossf/scorecard/cron/monitoring"
@@ -44,6 +46,7 @@ import (
 
 func processRequest(ctx context.Context,
 	batchRequest *data.ScorecardBatchRequest, checksToRun checker.CheckNameToFnMap, bucketURL string,
+	repoClient clients.RepoClient,
 	httpClient *http.Client, githubClient *github.Client, graphClient *githubv4.Client) error {
 	filename := data.GetBlobFilename(
 		fmt.Sprintf("shard-%05d", batchRequest.GetShardNum()),
@@ -75,7 +78,7 @@ func processRequest(ctx context.Context,
 	// TODO: run Scorecard for each repo in a separate thread.
 	for _, repoURL := range repoURLs {
 		log.Printf("Running Scorecard for repo: %s", repoURL.URL())
-		result := pkg.RunScorecards(ctx, repoURL, checksToRun, httpClient, githubClient, graphClient)
+		result := pkg.RunScorecards(ctx, repoURL, checksToRun, repoClient, httpClient, githubClient, graphClient)
 		result.Date = batchRequest.GetJobTime().AsTime().Format("2006-01-02")
 		err := result.AsJSON(true /*showDetails*/, &buffer)
 		if err != nil {
@@ -91,7 +94,9 @@ func processRequest(ctx context.Context,
 }
 
 func createNetClients(ctx context.Context) (
-	httpClient *http.Client, githubClient *github.Client, graphClient *githubv4.Client, logger *zap.Logger) {
+	repoClient clients.RepoClient,
+	httpClient *http.Client,
+	githubClient *github.Client, graphClient *githubv4.Client, logger *zap.Logger) {
 	cfg := zap.NewProductionConfig()
 	cfg.Level.SetLevel(zap.InfoLevel)
 	logger, err := cfg.Build()
@@ -106,6 +111,7 @@ func createNetClients(ctx context.Context) (
 	}
 	githubClient = github.NewClient(httpClient)
 	graphClient = githubv4.NewClient(httpClient)
+	repoClient = githubrepo.CreateGithubRepoClient(ctx, githubClient)
 	return
 }
 
@@ -155,7 +161,7 @@ func main() {
 		panic(fmt.Errorf("env_vars %s must be set", roundtripper.BucketURL))
 	}
 
-	httpClient, githubClient, graphClient, logger := createNetClients(ctx)
+	repoClient, httpClient, githubClient, graphClient, logger := createNetClients(ctx)
 
 	exporter, err := startMetricsExporter()
 	if err != nil {
@@ -185,7 +191,8 @@ func main() {
 			log.Print("subscription returned nil message during Receive, exiting")
 			break
 		}
-		if err := processRequest(ctx, req, checksToRun, bucketURL, httpClient, githubClient, graphClient); err != nil {
+		if err := processRequest(ctx, req, checksToRun, bucketURL,
+			repoClient, httpClient, githubClient, graphClient); err != nil {
 			// Nack the message so that another worker can retry.
 			subscriber.Nack()
 		}

pkg/scorecard.go

@@ -27,6 +27,7 @@ import (
 	"go.opencensus.io/tag"
 
 	"github.com/ossf/scorecard/checker"
+	"github.com/ossf/scorecard/clients"
 	"github.com/ossf/scorecard/repos"
 	"github.com/ossf/scorecard/stats"
 )
@@ -37,12 +38,13 @@ func logStats(ctx context.Context, startTime time.Time) {
 }
 
 func runEnabledChecks(ctx context.Context,
-	repo repos.RepoURL, checksToRun checker.CheckNameToFnMap,
+	repo repos.RepoURL, checksToRun checker.CheckNameToFnMap, repoClient clients.RepoClient,
 	httpClient *http.Client, githubClient *github.Client, graphClient *githubv4.Client,
 	resultsCh chan checker.CheckResult) {
 	request := checker.CheckRequest{
 		Ctx:         ctx,
 		Client:      githubClient,
+		RepoClient:  repoClient,
 		HTTPClient:  httpClient,
 		Owner:       repo.Owner,
 		Repo:        repo.Repo,
@@ -70,6 +72,7 @@ func runEnabledChecks(ctx context.Context,
 func RunScorecards(ctx context.Context,
 	repo repos.RepoURL,
 	checksToRun checker.CheckNameToFnMap,
+	repoClient clients.RepoClient,
 	httpClient *http.Client,
 	githubClient *github.Client,
 	graphClient *githubv4.Client) repos.RepoResult {
@@ -79,12 +82,15 @@ func RunScorecards(ctx context.Context,
 	}
 	defer logStats(ctx, time.Now())
 
+	if err := repoClient.InitRepo(repo.Owner, repo.Repo); err != nil {
+		log.Panicf("error during InitRepo: %v", err)
+	}
 	ret := repos.RepoResult{
 		Repo: repo.URL(),
 		Date: time.Now().Format("2006-01-02"),
 	}
 	resultsCh := make(chan checker.CheckResult)
-	go runEnabledChecks(ctx, repo, checksToRun,
+	go runEnabledChecks(ctx, repo, checksToRun, repoClient,
 		httpClient, githubClient, graphClient,
 		resultsCh)
 	for result := range resultsCh {