From 51de6b6e5d9b025561b15c30cbb498bf31101427 Mon Sep 17 00:00:00 2001 From: Azeem Shaikh Date: Fri, 12 Nov 2021 17:16:22 -0500 Subject: [PATCH] Check for issue activity in Maintained (#1251) Co-authored-by: Azeem Shaikh --- Makefile | 13 ++++++- checks/maintained.go | 36 ++++++++++++------- clients/githubrepo/branches.go | 19 ---------- clients/githubrepo/client.go | 5 +++ clients/githubrepo/copy.go | 50 +++++++++++++++++++++++++++ clients/githubrepo/graphql.go | 30 ++++++++++++++++ clients/issue.go | 23 ++++++++++++ clients/localdir/client.go | 6 ++++ clients/mockrepo/client.go | 17 ++++++++- clients/repo_client.go | 1 + docs/checks/internal/validate/main.go | 1 + e2e/maintained_test.go | 2 -- tools/go.mod | 1 + tools/go.sum | 1 + tools/tools.go | 4 ++- 15 files changed, 172 insertions(+), 37 deletions(-) create mode 100644 clients/githubrepo/copy.go create mode 100644 clients/issue.go diff --git a/Makefile b/Makefile index 86f82db5..28a206f1 100644 --- a/Makefile +++ b/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 diff --git a/checks/maintained.go b/checks/maintained.go index 130c4e19..773bb29c 100644 --- a/checks/maintained.go +++ b/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) } diff --git a/clients/githubrepo/branches.go b/clients/githubrepo/branches.go index 3595d0a4..f28c3e71 100644 --- a/clients/githubrepo/branches.go +++ b/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 { diff --git a/clients/githubrepo/client.go b/clients/githubrepo/client.go index 02217cef..d1b8f974 100644 --- a/clients/githubrepo/client.go +++ b/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() diff --git a/clients/githubrepo/copy.go b/clients/githubrepo/copy.go new file mode 100644 index 00000000..b1da421d --- /dev/null +++ b/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) +} diff --git a/clients/githubrepo/graphql.go b/clients/githubrepo/graphql.go index 1c262ee5..94760efa 100644 --- a/clients/githubrepo/graphql.go +++ b/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 +} diff --git a/clients/issue.go b/clients/issue.go new file mode 100644 index 00000000..ca55231f --- /dev/null +++ b/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 +} diff --git a/clients/localdir/client.go b/clients/localdir/client.go index 5e91b6ab..47c0a2c9 100644 --- a/clients/localdir/client.go +++ b/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) diff --git a/clients/mockrepo/client.go b/clients/mockrepo/client.go index f867cfdf..93856f87 100644 --- a/clients/mockrepo/client.go +++ b/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") diff --git a/clients/repo_client.go b/clients/repo_client.go index 8432cb60..97dc59b0 100644 --- a/clients/repo_client.go +++ b/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) diff --git a/docs/checks/internal/validate/main.go b/docs/checks/internal/validate/main.go index fd72f85a..7c369969 100644 --- a/docs/checks/internal/validate/main.go +++ b/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"}, diff --git a/e2e/maintained_test.go b/e2e/maintained_test.go index 9a39ee32..449b98af 100644 --- a/e2e/maintained_test.go +++ b/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()) }) diff --git a/tools/go.mod b/tools/go.mod index 7077423d..c23d2759 100644 --- a/tools/go.mod +++ b/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 diff --git a/tools/go.sum b/tools/go.sum index 00eb8d1a..7c1e6724 100644 --- a/tools/go.sum +++ b/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= diff --git a/tools/tools.go b/tools/tools.go index ce01fc02..fe7d4714 100644 --- a/tools/tools.go +++ b/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" )