Update SignedReleases to use RepoClient API (#844)

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

Makefile

@@ -1,7 +1,7 @@
 SHELL := /bin/bash
 GOPATH := $(go env GOPATH)
 GINKGO := ginkgo
-GIT_HASH := $(git rev-parse HEAD)
+GIT_HASH := $(shell git rev-parse HEAD)
 GOLANGGCI_LINT := golangci-lint
 PROTOC_GEN_GO := protoc-gen-go
 PROTOC := $(shell which protoc)

checks/signed_releases.go

@@ -18,8 +18,6 @@ import (
 	"fmt"
 	"strings"
 
-	"github.com/google/go-github/v32/github"
-
 	"github.com/ossf/scorecard/v2/checker"
 	sce "github.com/ossf/scorecard/v2/errors"
 )
@@ -30,39 +28,34 @@ const (
 	releaseLookBack     = 5
 )
 
+var artifactExtensions = []string{".asc", ".minisig", ".sig"}
+
 //nolint:gochecknoinits
 func init() {
 	registerCheck(CheckSignedReleases, SignedReleases)
 }
 
 // SignedReleases runs Signed-Releases check.
 func SignedReleases(c *checker.CheckRequest) checker.CheckResult {
-	releases, _, err := c.Client.Repositories.ListReleases(c.Ctx, c.Owner, c.Repo, &github.ListOptions{})
+	releases, err := c.RepoClient.ListReleases()
 	if err != nil {
 		e := sce.Create(sce.ErrScorecardInternal, fmt.Sprintf("Client.Repositories.ListReleases: %v", err))
 		return checker.CreateRuntimeErrorResult(CheckSignedReleases, e)
 	}
 
-	artifactExtensions := []string{".asc", ".minisig", ".sig"}
-
 	totalReleases := 0
 	totalSigned := 0
 	for _, r := range releases {
-		assets, _, err := c.Client.Repositories.ListReleaseAssets(c.Ctx, c.Owner, c.Repo, r.GetID(), &github.ListOptions{})
-		if err != nil {
-			e := sce.Create(sce.ErrScorecardInternal, fmt.Sprintf("Client.Repositories.ListReleaseAssets: %v", err))
-			return checker.CreateRuntimeErrorResult(CheckSignedReleases, e)
-		}
-		if len(assets) == 0 {
+		if len(r.Assets) == 0 {
 			continue
 		}
-		c.Dlogger.Debug("GitHub release found: %s", r.GetTagName())
+		c.Dlogger.Debug("GitHub release found: %s", r.TagName)
 		totalReleases++
 		signed := false
-		for _, asset := range assets {
+		for _, asset := range r.Assets {
 			for _, suffix := range artifactExtensions {
-				if strings.HasSuffix(asset.GetName(), suffix) {
-					c.Dlogger.Info("signed release artifact: %s, url: %s", asset.GetName(), asset.GetURL())
+				if strings.HasSuffix(asset.Name, suffix) {
+					c.Dlogger.Info("signed release artifact: %s, url: %s", asset.Name, asset.URL)
 					signed = true
 					break
 				}
@@ -73,7 +66,7 @@ func SignedReleases(c *checker.CheckRequest) checker.CheckResult {
 			}
 		}
 		if !signed {
-			c.Dlogger.Warn("release artifact %s not signed", r.GetTagName())
+			c.Dlogger.Warn("release artifact %s not signed", r.TagName)
 		}
 		if totalReleases >= releaseLookBack {
 			break

clients/githubrepo/client.go

@@ -77,6 +77,11 @@ func (client *Client) ListCommits() ([]clients.Commit, error) {
 	return client.graphClient.getCommits()
 }
 
+// ListReleases implements RepoClient.ListReleases.
+func (client *Client) ListReleases() ([]clients.Release, error) {
+	return client.graphClient.getReleases()
+}
+
 // IsArchived implements RepoClient.IsArchived.
 func (client *Client) IsArchived() (bool, error) {
 	return client.graphClient.isArchived()

clients/githubrepo/graphql.go

@@ -25,10 +25,12 @@ import (
 )
 
 const (
-	pullRequestsToAnalyze = 30
-	reviewsToAnalyze      = 30
-	labelsToAnalyze       = 30
-	commitsToAnalyze      = 30
+	pullRequestsToAnalyze  = 30
+	reviewsToAnalyze       = 30
+	labelsToAnalyze        = 30
+	commitsToAnalyze       = 30
+	releasesToAnalyze      = 30
+	releaseAssetsToAnalyze = 30
 )
 
 // nolint: govet
@@ -77,6 +79,17 @@ type graphqlData struct {
 				} `graphql:"latestReviews(last: $reviewsToAnalyze)"`
 			}
 		} `graphql:"pullRequests(last: $pullRequestsToAnalyze, states: MERGED)"`
+		Releases struct {
+			Nodes []struct {
+				TagName       githubv4.String
+				ReleaseAssets struct {
+					Nodes []struct {
+						Name githubv4.String
+						URL  githubv4.String
+					}
+				} `graphql:"releaseAssets(last: $releaseAssetsToAnalyze)"`
+			}
+		} `graphql:"releases(first: $releasesToAnalyze, orderBy:{field: CREATED_AT, direction:DESC})"`
 	} `graphql:"repository(owner: $owner, name: $name)"`
 }
 
@@ -85,26 +98,30 @@ type graphqlHandler struct {
 	data             *graphqlData
 	prs              []clients.PullRequest
 	commits          []clients.Commit
+	releases         []clients.Release
 	defaultBranchRef clients.BranchRef
 	archived         bool
 }
 
 func (handler *graphqlHandler) init(ctx context.Context, owner, repo string) error {
 	vars := map[string]interface{}{
-		"owner":                 githubv4.String(owner),
-		"name":                  githubv4.String(repo),
-		"pullRequestsToAnalyze": githubv4.Int(pullRequestsToAnalyze),
-		"reviewsToAnalyze":      githubv4.Int(reviewsToAnalyze),
-		"labelsToAnalyze":       githubv4.Int(labelsToAnalyze),
-		"commitsToAnalyze":      githubv4.Int(commitsToAnalyze),
+		"owner":                  githubv4.String(owner),
+		"name":                   githubv4.String(repo),
+		"pullRequestsToAnalyze":  githubv4.Int(pullRequestsToAnalyze),
+		"reviewsToAnalyze":       githubv4.Int(reviewsToAnalyze),
+		"labelsToAnalyze":        githubv4.Int(labelsToAnalyze),
+		"commitsToAnalyze":       githubv4.Int(commitsToAnalyze),
+		"releasesToAnalyze":      githubv4.Int(releasesToAnalyze),
+		"releaseAssetsToAnalyze": githubv4.Int(releaseAssetsToAnalyze),
 	}
 	handler.data = new(graphqlData)
 	if err := handler.client.Query(ctx, handler.data, vars); err != nil {
 		// nolint: wrapcheck
 		return sce.Create(sce.ErrScorecardInternal, fmt.Sprintf("githubv4.Query: %v", err))
 	}
 	handler.archived = bool(handler.data.Repository.IsArchived)
-	handler.prs = pullRequestFrom(handler.data)
+	handler.prs = pullRequestsFrom(handler.data)
+	handler.releases = releasesFrom(handler.data)
 	handler.defaultBranchRef = defaultBranchRefFrom(handler.data)
 	handler.commits = commitsFrom(handler.data)
 	return nil
@@ -122,11 +139,15 @@ func (handler *graphqlHandler) getCommits() ([]clients.Commit, error) {
 	return handler.commits, nil
 }
 
+func (handler *graphqlHandler) getReleases() ([]clients.Release, error) {
+	return handler.releases, nil
+}
+
 func (handler *graphqlHandler) isArchived() (bool, error) {
 	return handler.archived, nil
 }
 
-func pullRequestFrom(data *graphqlData) []clients.PullRequest {
+func pullRequestsFrom(data *graphqlData) []clients.PullRequest {
 	ret := make([]clients.PullRequest, len(data.Repository.PullRequests.Nodes))
 	for i, pr := range data.Repository.PullRequests.Nodes {
 		toAppend := clients.PullRequest{
@@ -152,6 +173,24 @@ func pullRequestFrom(data *graphqlData) []clients.PullRequest {
 	return ret
 }
 
+func releasesFrom(data *graphqlData) []clients.Release {
+	// nolint: prealloc // https://github.com/golang/go/wiki/CodeReviewComments#declaring-empty-slices
+	var releases []clients.Release
+	for _, r := range data.Repository.Releases.Nodes {
+		release := clients.Release{
+			TagName: string(r.TagName),
+		}
+		for _, a := range r.ReleaseAssets.Nodes {
+			release.Assets = append(release.Assets, clients.ReleaseAsset{
+				Name: string(a.Name),
+				URL:  string(a.URL),
+			})
+		}
+		releases = append(releases, release)
+	}
+	return releases
+}
+
 func defaultBranchRefFrom(data *graphqlData) clients.BranchRef {
 	return clients.BranchRef{
 		Name: string(data.Repository.DefaultBranchRef.Name),

clients/release.go

@@ -0,0 +1,27 @@
+// 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
+
+// Release represents a release version of a package/repo.
+type Release struct {
+	TagName string
+	Assets  []ReleaseAsset
+}
+
+// ReleaseAsset is part of the Release bundle.
+type ReleaseAsset struct {
+	Name string
+	URL  string
+}

clients/repo_client.go

@@ -51,5 +51,6 @@ type RepoClient interface {
 	ListMergedPRs() ([]PullRequest, error)
 	GetDefaultBranch() (BranchRef, error)
 	ListCommits() ([]Commit, error)
+	ListReleases() ([]Release, error)
 	Close() error
 }

e2e/signedreleases_test.go

@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+// nolint: dupl
 package e2e
 
 import (
@@ -22,18 +23,22 @@ import (
 
 	"github.com/ossf/scorecard/v2/checker"
 	"github.com/ossf/scorecard/v2/checks"
+	"github.com/ossf/scorecard/v2/clients/githubrepo"
 	scut "github.com/ossf/scorecard/v2/utests"
 )
 
-var _ = Describe("E2E TEST:Signedreleases", func() {
+var _ = Describe("E2E TEST:"+checks.CheckSignedReleases, func() {
 	Context("E2E TEST:Validating signed releases", func() {
 		It("Should return valid signed releases", func() {
 			dl := scut.TestDetailLogger{}
+			repoClient := githubrepo.CreateGithubRepoClient(context.Background(), ghClient, graphClient)
+			err := repoClient.InitRepo("ossf-tests", "scorecard-check-signed-releases-e2e")
+			Expect(err).Should(BeNil())
 			req := checker.CheckRequest{
 				Ctx:         context.Background(),
 				Client:      ghClient,
 				HTTPClient:  httpClient,
-				RepoClient:  nil,
+				RepoClient:  repoClient,
 				Owner:       "ossf-tests",
 				Repo:        "scorecard-check-signed-releases-e2e",
 				GraphClient: graphClient,