Skip to content

Commit

Permalink
feat: assess commits on PR for Author and Verification
Browse files Browse the repository at this point in the history
Signed-off-by: Paul Horton <[email protected]>
  • Loading branch information
madpah committed Sep 17, 2024
1 parent 9e5eeb9 commit 3fe9654
Show file tree
Hide file tree
Showing 2 changed files with 121 additions and 10 deletions.
81 changes: 80 additions & 1 deletion github/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,11 +248,34 @@ func EvaluatePullRequest(logger *zap.Logger, postgres db.IClaDB, evalInfo *types
// The following loop will change a loop as a result
var usersNeedingToSignCLA []types.UserSignature
var usersSigned []types.UserSignature
var commitsMissingAuthor []github.RepositoryCommit
var commitsMissingVerification []github.RepositoryCommit

for _, v := range commits {
// It is important to use GetAuthor() instead of v.Commit.GetCommitter() because the committer can be the GH webflow user, whereas the author is
// the canonical author of the commit
author := *v.GetAuthor()
author := v.GetAuthor()
commitFailedChecks := false

if author == nil {
commitsMissingAuthor = append(commitsMissingAuthor, *v)
commitFailedChecks = true
}

commitVerified := false
commitVerification := v.Commit.GetVerification()
if commitVerification != nil {
commitVerified = *v.Commit.Verification.Verified
}
if commitVerified == false {

Check failure on line 270 in github/github.go

View workflow job for this annotation

GitHub Actions / Code Quality

S1002: should omit comparison to bool constant, can be simplified to `!commitVerified` (gosimple)
commitsMissingVerification = append(commitsMissingVerification, *v)
commitFailedChecks = true
logger.Debug("Commit failed verification check", zap.Any("Commit", v))
}

if commitFailedChecks {
continue
}

// if author is a collaborator, that author need not sign the cla.
var isCollaborator bool
Expand Down Expand Up @@ -293,6 +316,59 @@ func EvaluatePullRequest(logger *zap.Logger, postgres db.IClaDB, evalInfo *types
}
}

logger.Info(
fmt.Sprintf("Commits Reviewed for PR #%d", evalInfo.PRNumber),
zap.Int("Missing Author", len(commitsMissingAuthor)),
zap.Int("Missing Verification", len(commitsMissingVerification)),
)

if len(commitsMissingAuthor) > 0 || len(commitsMissingVerification) > 0 {
if len(commitsMissingAuthor) > 0 {
err := createRepoLabel(logger, client.Issues, evalInfo.RepoOwner, evalInfo.RepoName, labelNameCommitsNoAuthor, "B60205", "Commits are missing author information - this must be resolved", evalInfo.PRNumber)
if err != nil {
return err
}
}

if len(commitsMissingVerification) > 0 {
err := createRepoLabel(
logger,
client.Issues,
evalInfo.RepoOwner,
evalInfo.RepoName,
labelNameCommitsMissingVerification,
"B60205",
"Some commits are not signed - this must be resolved",
evalInfo.PRNumber,
)
if err != nil {
return err
}
}

message := `Thanks for the contribution. Unfortunately some of your commits don't meet our standards. All commits must be signed and have author information set.
The commits to review are:
%s`
commitsMessage := ""
for _, c := range commitsMissingAuthor {
commitsMessage += commitsMessage + fmt.Sprintf(`- <a href="%s">%s</a> - missing author :cop:`, *c.HTMLURL, *c.SHA)
}
for _, c := range commitsMissingVerification {
commitsMessage += commitsMessage + fmt.Sprintf(`- <a href="%s">%s</a> - unsigned commit :key:`, *c.HTMLURL, *c.SHA)
}
logger.Debug("Adding Comment to Issue", zap.Int("Issue #", int(evalInfo.PRNumber)), zap.String("Comment", fmt.Sprintf(message, commitsMessage)))
_, err = addCommentToIssueIfNotExists(
client.Issues, evalInfo.RepoOwner, evalInfo.RepoName, int(evalInfo.PRNumber),
fmt.Sprintf(message, commitsMessage))
if err != nil {
return err
}

return nil
}

if len(usersNeedingToSignCLA) > 0 {
err := createRepoLabel(logger, client.Issues, evalInfo.RepoOwner, evalInfo.RepoName, labelNameCLANotSigned, "ff3333", "The CLA needs to be signed", evalInfo.PRNumber)
if err != nil {
Expand Down Expand Up @@ -374,6 +450,8 @@ func createRepoStatus(repositoryService RepositoriesService, owner, repo, sha, s

const labelNameCLANotSigned string = ":monocle_face: cla not signed"
const labelNameCLASigned string = ":heart_eyes: cla signed"
const labelNameCommitsNoAuthor string = ":unamused: commits missing author"
const labelNameCommitsMissingVerification string = ":anguished: commits missing verification"

func createRepoLabel(logger *zap.Logger,
issuesService IssuesService,
Expand Down Expand Up @@ -435,6 +513,7 @@ func addCommentToIssueIfNotExists(issuesService IssuesService, owner, repo strin
}

if !alreadyCommented {

prComment := &github.IssueComment{}
prComment.Body = &message

Expand Down
50 changes: 41 additions & 9 deletions github/github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -289,10 +289,12 @@ func TestHandlePullRequestIsCollaboratorError(t *testing.T) {
GHImpl = origGithubImpl
}()
mockAuthorLogin := "myAuthorLogin"
mockRepositoryCommits := []*github.RepositoryCommit{{Author: &github.User{Login: &mockAuthorLogin}}}
mockRepositoryCommits := getMockRepositoryCommitsSigned(mockAuthorLogin)
forcedError := fmt.Errorf("forced IsCollaborator error")
GHImpl = &GHInterfaceMock{
PullRequestsMock: PullRequestsMock{mockRepositoryCommits: mockRepositoryCommits},
PullRequestsMock: PullRequestsMock{
mockRepositoryCommits: mockRepositoryCommits,
},
RepositoriesMock: RepositoriesMock{
isCollaboratorErr: forcedError,
},
Expand Down Expand Up @@ -325,9 +327,11 @@ func TestHandlePullRequestIsCollaboratorTrueCollaborator(t *testing.T) {
GHImpl = origGithubImpl
}()
mockAuthorLogin := "myAuthorLogin"
mockRepositoryCommits := []*github.RepositoryCommit{{Author: &github.User{Login: &mockAuthorLogin}}}
mockRepositoryCommits := getMockRepositoryCommitsSigned(mockAuthorLogin)
GHImpl = &GHInterfaceMock{
PullRequestsMock: PullRequestsMock{mockRepositoryCommits: mockRepositoryCommits},
PullRequestsMock: PullRequestsMock{
mockRepositoryCommits: mockRepositoryCommits,
},
RepositoriesMock: RepositoriesMock{
isCollaboratorResult: true,
},
Expand Down Expand Up @@ -370,7 +374,7 @@ func TestHandlePullRequestCreateLabelError(t *testing.T) {
GHImpl = origGithubImpl
}()
mockAuthorLogin := "myAuthorLogin"
mockRepositoryCommits := []*github.RepositoryCommit{{Author: &github.User{Login: &mockAuthorLogin}}}
mockRepositoryCommits := getMockRepositoryCommitsSigned(mockAuthorLogin)
forcedError := fmt.Errorf("forced CreateLabel error")
GHImpl = &GHInterfaceMock{
PullRequestsMock: PullRequestsMock{mockRepositoryCommits: mockRepositoryCommits},
Expand Down Expand Up @@ -407,7 +411,7 @@ func TestHandlePullRequestAddLabelsToIssueError(t *testing.T) {
GHImpl = origGithubImpl
}()
mockAuthorLogin := "myAuthorLogin"
mockRepositoryCommits := []*github.RepositoryCommit{{Author: &github.User{Login: &mockAuthorLogin}}}
mockRepositoryCommits := getMockRepositoryCommitsSigned(mockAuthorLogin)
forcedError := fmt.Errorf("forced AddLabelsToIssue error")
GHImpl = &GHInterfaceMock{
PullRequestsMock: PullRequestsMock{mockRepositoryCommits: mockRepositoryCommits},
Expand Down Expand Up @@ -457,7 +461,7 @@ func TestHandlePullRequestGetAppError(t *testing.T) {
GHImpl = origGithubImpl
}()
mockAuthorLogin := "myAuthorLogin"
mockRepositoryCommits := []*github.RepositoryCommit{{Author: &github.User{Login: &mockAuthorLogin}}}
mockRepositoryCommits := getMockRepositoryCommitsSigned(mockAuthorLogin)
GHImpl = &GHInterfaceMock{
PullRequestsMock: PullRequestsMock{mockRepositoryCommits: mockRepositoryCommits},
IssuesMock: IssuesMock{
Expand Down Expand Up @@ -578,13 +582,23 @@ func TestHandlePullRequestListCommits(t *testing.T) {
Login: github.String(login),
Email: github.String("[email protected]"),
},
Commit: &github.Commit{
Verification: &github.SignatureVerification{
Verified: github.Bool(true),
},
},
SHA: github.String("johnSHA"),
},
{
Author: &github.User{
Login: github.String(login2),
Email: github.String("[email protected]"),
},
Commit: &github.Commit{
Verification: &github.SignatureVerification{
Verified: github.Bool(true),
},
},
SHA: github.String("doeSHA"),
},
}
Expand Down Expand Up @@ -646,7 +660,8 @@ func TestHandlePullRequestListCommitsNoAuthor(t *testing.T) {
// Date: github.Timestamp.Local(),
},
},
SHA: github.String("johnSHA"),
SHA: github.String("johnSHA"),
HTMLURL: github.String("https://github.com"),
},
}
GHImpl = &GHInterfaceMock{
Expand Down Expand Up @@ -712,7 +727,8 @@ func TestHandlePullRequestListCommitsUnsignedCommit(t *testing.T) {
Payload: nil,
},
},
SHA: github.String("johnSHA"),
SHA: github.String("johnSHA"),
HTMLURL: github.String("https://github.com"),
},
}
GHImpl = &GHInterfaceMock{
Expand Down Expand Up @@ -905,3 +921,19 @@ func TestReviewPriorPRs(t *testing.T) {

assert.NoError(t, ReviewPriorPRs(logger, mockDB, &user))
}

func getMockRepositoryCommitsSigned(mockAuthorLogin string) []*github.RepositoryCommit {
mockRepositoryCommits := []*github.RepositoryCommit{
{
Author: &github.User{
Login: &mockAuthorLogin,
},
Commit: &github.Commit{
Verification: &github.SignatureVerification{
Verified: github.Bool(true),
},
},
},
}
return mockRepositoryCommits
}

0 comments on commit 3fe9654

Please sign in to comment.