Check for issue activity in Maintained (#1251)

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

Makefile

@@ -6,6 +6,7 @@ GIT_VERSION ?= $(shell git describe --tags --always --dirty)
 SOURCE_DATE_EPOCH=$(shell git log --date=iso8601-strict -1 --pretty=%ct)
 GOLANGGCI_LINT := golangci-lint
 PROTOC_GEN_GO := protoc-gen-go
+MOCKGEN := mockgen
 PROTOC := $(shell which protoc)
 IMAGE_NAME = scorecard
 OUTPUT = output
@@ -96,7 +97,7 @@ tree-status: ## Verify tree is clean and all changes are committed
 build-cron: build-pubsub build-bq-transfer build-github-server build-webhook build-add-script \
 	  build-validate-script build-update-script
 
-build-targets = generate-docs build-proto build-scorecard build-cron ko-build-everything dockerbuild
+build-targets = generate-mocks generate-docs build-proto build-scorecard build-cron ko-build-everything dockerbuild
 .PHONY: build $(build-targets)
 build: ## Build all binaries and images in the repo.
 build: $(build-targets)
@@ -108,6 +109,16 @@ cron/data/request.pb.go: cron/data/request.proto |  $(PROTOC)
 cron/data/metadata.pb.go: cron/data/metadata.proto |  $(PROTOC)
 	protoc --go_out=../../../ cron/data/metadata.proto
 
+generate-mocks: ## Compiles and generates all mocks using mockgen.
+generate-mocks: clients/mockrepo/client.go clients/mockrepo/repo.go
+clients/mockrepo/client.go: clients/repo_client.go
+	# Generating MockRepoClient
+	$(MOCKGEN) -source=clients/repo_client.go -destination clients/mockrepo/client.go -package mockrepo -copyright_file clients/mockrepo/license.txt
+clients/mockrepo/repo.go: clients/repo.go
+	# Generating MockRepoClient
+	$(MOCKGEN) -source=clients/repo.go -destination clients/mockrepo/repo.go -package mockrepo -copyright_file clients/mockrepo/license.txt
+
+
 generate-docs: ## Generates docs
 generate-docs: validate-docs docs/checks.md
 docs/checks.md: docs/checks/internal/checks.yaml docs/checks/internal/*.go docs/checks/internal/generate/*.go

checks/maintained.go

@@ -26,7 +26,7 @@ const (
 	// CheckMaintained is the exported check name for Maintained.
 	CheckMaintained = "Maintained"
 	lookBackDays    = 90
-	commitsPerWeek  = 1
+	activityPerWeek = 1
 	daysInOneWeek   = 7
 )
 
@@ -46,25 +46,35 @@ func IsMaintained(c *checker.CheckRequest) checker.CheckResult {
 		return checker.CreateMinScoreResult(CheckMaintained, "repo is marked as archived")
 	}
 
+	// If not explicitly marked archived, look for activity in past `lookBackDays`.
+	threshold := time.Now().AddDate(0 /*years*/, 0 /*months*/, -1*lookBackDays /*days*/)
+
 	commits, err := c.RepoClient.ListCommits()
 	if err != nil {
 		e := sce.WithMessage(sce.ErrScorecardInternal, err.Error())
 		return checker.CreateRuntimeErrorResult(CheckMaintained, e)
 	}
-
-	tz, err := time.LoadLocation("UTC")
-	if err != nil {
-		e := sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("time.LoadLocation: %v", err))
-		return checker.CreateRuntimeErrorResult(CheckMaintained, e)
-	}
-	threshold := time.Now().In(tz).AddDate(0, 0, -1*lookBackDays)
-	totalCommits := 0
+	commitsWithinThreshold := 0
 	for _, commit := range commits {
 		if commit.CommittedDate.After(threshold) {
-			totalCommits++
+			commitsWithinThreshold++
 		}
 	}
-	return checker.CreateProportionalScoreResult(CheckMaintained,
-		fmt.Sprintf("%d commit(s) found in the last %d days", totalCommits, lookBackDays),
-		totalCommits, commitsPerWeek*lookBackDays/daysInOneWeek)
+
+	issues, err := c.RepoClient.ListIssues()
+	if err != nil {
+		e := sce.WithMessage(sce.ErrScorecardInternal, err.Error())
+		return checker.CreateRuntimeErrorResult(CheckMaintained, e)
+	}
+	issuesUpdatedWithinThreshold := 0
+	for _, issue := range issues {
+		if issue.UpdatedAt.After(threshold) {
+			issuesUpdatedWithinThreshold++
+		}
+	}
+
+	return checker.CreateProportionalScoreResult(CheckMaintained, fmt.Sprintf(
+		"%d commit(s) out of %d and %d issue activity out of %d found in the last %d days",
+		commitsWithinThreshold, len(commits), issuesUpdatedWithinThreshold, len(issues), lookBackDays),
+		commitsWithinThreshold+issuesUpdatedWithinThreshold, activityPerWeek*lookBackDays/daysInOneWeek)
 }

clients/githubrepo/branches.go

@@ -114,25 +114,6 @@ func (handler *branchesHandler) listBranches() ([]*clients.BranchRef, error) {
 	return handler.branches, nil
 }
 
-func copyBoolPtr(src *bool, dest **bool) {
-	if src != nil {
-		*dest = new(bool)
-		**dest = *src
-	}
-}
-
-func copyInt32Ptr(src *int32, dest **int32) {
-	if src != nil {
-		*dest = new(int32)
-		**dest = *src
-	}
-}
-
-func copyStringSlice(src []string, dest *[]string) {
-	*dest = make([]string, len(src))
-	copy(*dest, src)
-}
-
 func getBranchRefFrom(data branch) *clients.BranchRef {
 	branchRef := new(clients.BranchRef)
 	if data.Name != nil {

clients/githubrepo/client.go

@@ -124,6 +124,11 @@ func (client *Client) ListCommits() ([]clients.Commit, error) {
 	return client.graphClient.getCommits()
 }
 
+// ListIssues implements RepoClient.ListIssues.
+func (client *Client) ListIssues() ([]clients.Issue, error) {
+	return client.graphClient.getIssues()
+}
+
 // ListReleases implements RepoClient.ListReleases.
 func (client *Client) ListReleases() ([]clients.Release, error) {
 	return client.releases.getReleases()

clients/githubrepo/copy.go

@@ -0,0 +1,50 @@
+// 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 "time"
+
+func copyBoolPtr(src *bool, dest **bool) {
+	if src != nil {
+		*dest = new(bool)
+		**dest = *src
+	}
+}
+
+func copyStringPtr(src *string, dest **string) {
+	if src != nil {
+		*dest = new(string)
+		**dest = *src
+	}
+}
+
+func copyInt32Ptr(src *int32, dest **int32) {
+	if src != nil {
+		*dest = new(int32)
+		**dest = *src
+	}
+}
+
+func copyTimePtr(src *time.Time, dest **time.Time) {
+	if src != nil {
+		*dest = new(time.Time)
+		**dest = *src
+	}
+}
+
+func copyStringSlice(src []string, dest *[]string) {
+	*dest = make([]string, len(src))
+	copy(*dest, src)
+}

clients/githubrepo/graphql.go

@@ -18,6 +18,7 @@ import (
 	"context"
 	"fmt"
 	"sync"
+	"time"
 
 	"github.com/shurcooL/githubv4"
 
@@ -27,6 +28,7 @@ import (
 
 const (
 	pullRequestsToAnalyze = 30
+	issuesToAnalyze       = 30
 	reviewsToAnalyze      = 30
 	labelsToAnalyze       = 30
 	commitsToAnalyze      = 30
@@ -81,6 +83,13 @@ type graphqlData struct {
 				} `graphql:"reviews(last: $reviewsToAnalyze)"`
 			}
 		} `graphql:"pullRequests(last: $pullRequestsToAnalyze, states: MERGED)"`
+		Issues struct {
+			Nodes []struct {
+				// nolint: revive,stylecheck // naming according to githubv4 convention.
+				Url       *string
+				UpdatedAt *time.Time
+			}
+		} `graphql:"issues(first: $issuesToAnalyze, orderBy:{field:UPDATED_AT, direction:DESC})"`
 	} `graphql:"repository(owner: $owner, name: $name)"`
 }
 
@@ -94,6 +103,7 @@ type graphqlHandler struct {
 	repo     string
 	prs      []clients.PullRequest
 	commits  []clients.Commit
+	issues   []clients.Issue
 	archived bool
 }
 
@@ -112,6 +122,7 @@ func (handler *graphqlHandler) setup() error {
 			"owner":                 githubv4.String(handler.owner),
 			"name":                  githubv4.String(handler.repo),
 			"pullRequestsToAnalyze": githubv4.Int(pullRequestsToAnalyze),
+			"issuesToAnalyze":       githubv4.Int(issuesToAnalyze),
 			"reviewsToAnalyze":      githubv4.Int(reviewsToAnalyze),
 			"labelsToAnalyze":       githubv4.Int(labelsToAnalyze),
 			"commitsToAnalyze":      githubv4.Int(commitsToAnalyze),
@@ -122,6 +133,7 @@ func (handler *graphqlHandler) setup() error {
 		handler.archived = bool(handler.data.Repository.IsArchived)
 		handler.prs = pullRequestsFrom(handler.data)
 		handler.commits = commitsFrom(handler.data)
+		handler.issues = issuesFrom(handler.data)
 	})
 	return handler.errSetup
 }
@@ -140,6 +152,13 @@ func (handler *graphqlHandler) getCommits() ([]clients.Commit, error) {
 	return handler.commits, nil
 }
 
+func (handler *graphqlHandler) getIssues() ([]clients.Issue, error) {
+	if err := handler.setup(); err != nil {
+		return nil, fmt.Errorf("error during graphqlHandler.setup: %w", err)
+	}
+	return handler.issues, nil
+}
+
 func (handler *graphqlHandler) isArchived() (bool, error) {
 	if err := handler.setup(); err != nil {
 		return false, fmt.Errorf("error during graphqlHandler.setup: %w", err)
@@ -193,3 +212,14 @@ func commitsFrom(data *graphqlData) []clients.Commit {
 	}
 	return ret
 }
+
+func issuesFrom(data *graphqlData) []clients.Issue {
+	var ret []clients.Issue
+	for _, issue := range data.Repository.Issues.Nodes {
+		var tmpIssue clients.Issue
+		copyStringPtr(issue.Url, &tmpIssue.URI)
+		copyTimePtr(issue.UpdatedAt, &tmpIssue.UpdatedAt)
+		ret = append(ret, tmpIssue)
+	}
+	return ret
+}

clients/issue.go

@@ -0,0 +1,23 @@
+// 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
+
+import "time"
+
+// Issue represents a thread like GitHub issue comment thread.
+type Issue struct {
+	URI       *string
+	UpdatedAt *time.Time
+}

clients/localdir/client.go

@@ -169,10 +169,16 @@ func (client *localDirClient) GetDefaultBranch() (*clients.BranchRef, error) {
 	return nil, fmt.Errorf("GetDefaultBranch: %w", clients.ErrUnsupportedFeature)
 }
 
+// ListCommits implements RepoClient.ListCommits.
 func (client *localDirClient) ListCommits() ([]clients.Commit, error) {
 	return nil, fmt.Errorf("ListCommits: %w", clients.ErrUnsupportedFeature)
 }
 
+// ListIssues implements RepoClient.ListIssues.
+func (client *localDirClient) ListIssues() ([]clients.Issue, error) {
+	return nil, fmt.Errorf("ListIssues: %w", clients.ErrUnsupportedFeature)
+}
+
 // ListReleases implements RepoClient.ListReleases.
 func (client *localDirClient) ListReleases() ([]clients.Release, error) {
 	return nil, fmt.Errorf("ListReleases: %w", clients.ErrUnsupportedFeature)

clients/mockrepo/client.go

@@ -197,6 +197,21 @@ func (mr *MockRepoClientMockRecorder) ListFiles(predicate interface{}) *gomock.C
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListFiles", reflect.TypeOf((*MockRepoClient)(nil).ListFiles), predicate)
 }
 
+// ListIssues mocks base method.
+func (m *MockRepoClient) ListIssues() ([]clients.Issue, error) {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "ListIssues")
+	ret0, _ := ret[0].([]clients.Issue)
+	ret1, _ := ret[1].(error)
+	return ret0, ret1
+}
+
+// ListIssues indicates an expected call of ListIssues.
+func (mr *MockRepoClientMockRecorder) ListIssues() *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListIssues", reflect.TypeOf((*MockRepoClient)(nil).ListIssues))
+}
+
 // ListMergedPRs mocks base method.
 func (m *MockRepoClient) ListMergedPRs() ([]clients.PullRequest, error) {
 	m.ctrl.T.Helper()
@@ -272,7 +287,7 @@ func (mr *MockRepoClientMockRecorder) Search(request interface{}) *gomock.Call {
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Search", reflect.TypeOf((*MockRepoClient)(nil).Search), request)
 }
 
-// URL mocks base method.
+// URI mocks base method.
 func (m *MockRepoClient) URI() string {
 	m.ctrl.T.Helper()
 	ret := m.ctrl.Call(m, "URI")

clients/repo_client.go

@@ -31,6 +31,7 @@ type RepoClient interface {
 	ListBranches() ([]*BranchRef, error)
 	GetDefaultBranch() (*BranchRef, error)
 	ListCommits() ([]Commit, error)
+	ListIssues() ([]Issue, error)
 	ListReleases() ([]Release, error)
 	ListContributors() ([]Contributor, error)
 	ListSuccessfulWorkflowRuns(filename string) ([]WorkflowRun, error)

docs/checks/internal/validate/main.go

@@ -43,6 +43,7 @@ var (
 		"ListBranches":               {"GitHub"},
 		"GetDefaultBranch":           {"GitHub"},
 		"ListCommits":                {"GitHub"},
+		"ListIssues":                 {"GitHub"},
 		"ListReleases":               {"GitHub"},
 		"ListContributors":           {"GitHub"},
 		"ListSuccessfulWorkflowRuns": {"GitHub"},

e2e/maintained_test.go

@@ -16,7 +16,6 @@ package e2e
 
 import (
 	"context"
-	"fmt"
 
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
@@ -54,7 +53,6 @@ var _ = Describe("E2E TEST:"+checks.CheckMaintained, func() {
 			// Old version.
 			Expect(result.Error).Should(BeNil())
 			Expect(result.Pass).Should(BeTrue())
-			fmt.Printf("%v", result)
 			// New version.
 			Expect(scut.ValidateTestReturn(nil, "active repo", &expected, &result, &dl)).Should(BeTrue())
 		})

tools/go.mod

@@ -3,6 +3,7 @@ module github.com/ossf/scorecard/tools
 go 1.17
 
 require (
+	github.com/golang/mock v1.6.0
 	github.com/golangci/golangci-lint v1.42.1
 	github.com/google/addlicense v1.0.0
 	github.com/google/ko v0.9.3

tools/go.sum

@@ -356,6 +356,7 @@ github.com/golang/mock v1.4.1/go.mod h1:UOMv5ysSaYNkG+OFQykRIcU/QvvxJf3p21QfJ2Bt
 github.com/golang/mock v1.4.3/go.mod h1:UOMv5ysSaYNkG+OFQykRIcU/QvvxJf3p21QfJ2Bt3cw=
 github.com/golang/mock v1.4.4/go.mod h1:l3mdAwkq5BuhzHwde/uurv3sEJeZMXNpwsxVWU71h+4=
 github.com/golang/mock v1.5.0/go.mod h1:CWnOUgYIOo4TcNZ0wHX3YZCqsaM1I1Jvs6v3mP3KVu8=
+github.com/golang/mock v1.6.0 h1:ErTB+efbowRARo13NNdxyJji2egdxLGQhRaY+DUumQc=
 github.com/golang/mock v1.6.0/go.mod h1:p6yTPP+5HYm5mzsMV8JkE6ZKdX+/wYM6Hr+LicevLPs=
 github.com/golang/protobuf v1.1.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
 github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=

tools/tools.go

@@ -1,3 +1,4 @@
+//go:build tools
 // +build tools
 
 // Copyright 2020 Security Scorecard Authors
@@ -17,10 +18,11 @@
 package main
 
 import (
+	_ "github.com/golang/mock/mockgen"
 	_ "github.com/golangci/golangci-lint/cmd/golangci-lint"
 	_ "github.com/google/addlicense"
+	_ "github.com/google/ko"
 	_ "github.com/naveensrinivasan/stunning-tribble"
 	_ "github.com/onsi/ginkgo/ginkgo"
 	_ "google.golang.org/protobuf/cmd/protoc-gen-go"
-	_ "github.com/google/ko"
 )