Update trusted author handling

- Separate member and collaborator configurations
- Separate trusted apps from the member and collaborator handling

Signed-off-by: Dale Haiducek <[email protected]>

Author: dhaiducek

prow/plugins/config.go

@@ -694,16 +694,15 @@ func (w Welcome) getRepos() []string {
 
 // Dco is config for the DCO (https://developercertificate.org/) checker plugin.
 type Dco struct {
-	// SkipDCOCheckForMembers is used to skip DCO check for trusted org members
+	// SkipDCOCheckForMembers is used to skip DCO check for org members.
 	SkipDCOCheckForMembers bool `json:"skip_dco_check_for_members,omitempty"`
 	// TrustedApps defines list of apps which commits will not be checked for DCO singoff.
 	// The list should contain usernames of each GitHub App without [bot] suffix.
-	// By default, this option is ignored.
 	TrustedApps []string `json:"trusted_apps,omitempty"`
 	// TrustedOrg is the org whose members' commits will not be checked for DCO signoff
-	// if the skip DCO option is enabled. The default is the PR's org.
+	// if the SkipDCOCheckForMembers option is enabled. The default is the PR's org.
 	TrustedOrg string `json:"trusted_org,omitempty"`
-	// SkipDCOCheckForCollaborators is used to skip DCO check for trusted org members
+	// SkipDCOCheckForCollaborators is used to skip DCO check for repository collaborators.
 	SkipDCOCheckForCollaborators bool `json:"skip_dco_check_for_collaborators,omitempty"`
 	// ContributingRepo is used to point users to a different repo containing CONTRIBUTING.md
 	ContributingRepo string `json:"contributing_repo,omitempty"`

prow/plugins/dco/dco.go

@@ -79,6 +79,7 @@ func helpProvider(config *plugins.Configuration, enabledRepos []config.OrgRepo)
 		Dco: map[string]*plugins.Dco{
 			"org/repo": {
 				SkipDCOCheckForMembers:       true,
+				TrustedApps:                  []string{"trusted-app"},
 				TrustedOrg:                   "org",
 				SkipDCOCheckForCollaborators: true,
 				ContributingRepo:             "other-org/other-repo",
@@ -124,14 +125,37 @@ type commentPruner interface {
 }
 
 // filterTrustedUsers checks whether the commits are from a trusted user and returns those that are not
-func filterTrustedUsers(gc gitHubClient, l *logrus.Entry, skipDCOCheckForCollaborators bool, trustedApps []string, trustedOrg, org, repo string, allCommits []github.RepositoryCommit) ([]github.RepositoryCommit, error) {
+func filterTrustedUsers(gc gitHubClient, l *logrus.Entry, config plugins.Dco, org, repo string, allCommits []github.RepositoryCommit) ([]github.RepositoryCommit, error) {
 	untrustedCommits := make([]github.RepositoryCommit, 0, len(allCommits))
 
+	var trustedResponse trigger.TrustedUserResponse
+
 	for _, commit := range allCommits {
-		trustedResponse, err := trigger.TrustedUser(gc, !skipDCOCheckForCollaborators, trustedApps, trustedOrg, commit.Author.Login, org, repo)
-		if err != nil {
-			return nil, fmt.Errorf("Error checking is member trusted: %w", err)
+		trustedResponse.IsTrusted = false
+
+		// Handle TrustedApps separately (since Trigger checks this last and doesn't have member/collaborator configurable handling)
+		for _, trustedApp := range config.TrustedApps {
+			if tUser := strings.TrimSuffix(commit.Author.Login, "[bot]"); tUser == trustedApp {
+				trustedResponse.IsTrusted = true
+				break
+			}
 		}
+
+		if !trustedResponse.IsTrusted && (config.SkipDCOCheckForMembers || config.SkipDCOCheckForCollaborators) {
+			trustedOrg := config.TrustedOrg
+
+			// If SkipDCOCheckforMembers is disabled, make sure the trusted org is empty
+			if !config.SkipDCOCheckForMembers {
+				trustedOrg = ""
+			}
+
+			var err error
+			trustedResponse, err = trigger.TrustedUser(gc, !config.SkipDCOCheckForCollaborators, config.TrustedApps, trustedOrg, commit.Author.Login, org, repo)
+			if err != nil {
+				return nil, fmt.Errorf("Error checking is member trusted: %w", err)
+			}
+		}
+
 		if !trustedResponse.IsTrusted {
 			l.Debugf("Member %s is not trusted", commit.Author.Login)
 			untrustedCommits = append(untrustedCommits, commit)
@@ -296,8 +320,8 @@ func handle(config plugins.Dco, gc gitHubClient, cp commentPruner, log *logrus.E
 		return err
 	}
 
-	if config.SkipDCOCheckForMembers || config.SkipDCOCheckForCollaborators {
-		commitsMissingDCO, err = filterTrustedUsers(gc, l, config.SkipDCOCheckForCollaborators, config.TrustedApps, config.TrustedOrg, org, repo, commitsMissingDCO)
+	if config.SkipDCOCheckForMembers || config.SkipDCOCheckForCollaborators || len(config.TrustedApps) > 0 {
+		commitsMissingDCO, err = filterTrustedUsers(gc, l, config, org, repo, commitsMissingDCO)
 		if err != nil {
 			l.WithError(err).Infof("Error running trusted org member check against commits in PR")
 			return err

prow/plugins/dco/dco_test.go

@@ -255,6 +255,39 @@ Instructions for interacting with me using PR comments are available [here](http
 
 			addedLabel:     fmt.Sprintf("/#3:%s", dcoYesLabel),
 			expectedStatus: github.StatusSuccess,
+		}, {
+			name: "should fail if an user is member of the trusted org but skip member check is false (commit non-signed)",
+			config: plugins.Dco{
+				SkipDCOCheckForMembers: false,
+				TrustedOrg:             "kubernetes",
+			},
+			pullRequestEvent: github.PullRequestEvent{
+				Action:      github.PullRequestActionOpened,
+				PullRequest: github.PullRequest{Number: 3, Head: github.PullRequestBranch{SHA: "sha"}},
+			},
+			commits: []github.RepositoryCommit{
+				{
+					SHA:    "sha",
+					Commit: github.GitCommit{Message: "not signed off"},
+					Author: github.User{
+						Login: "test",
+					},
+				},
+			},
+			issueState: "open",
+			hasDCONo:   true,
+			hasDCOYes:  false,
+
+			addedLabel:     fmt.Sprintf("/#3:%s", dcoNoLabel),
+			expectedStatus: github.StatusFailure,
+			addedComment: "/#3:Thanks for your pull request. Before we can look at it, you'll need to add a 'DCO signoff' to your commits.\n\n" +
+				":memo: **Please follow instructions in the [contributing guide](https://github.com///blob/master/CONTRIBUTING.md) to update your " +
+				"commits with the DCO**\n\nFull details of the Developer Certificate of Origin can be found at " +
+				"[developercertificate.org](https://developercertificate.org/).\n\n**The list of commits missing DCO signoff**:\n\n" +
+				"- [sha](https://github.com///commits/sha) not signed off\n\n<details>\n\nInstructions for interacting with me using PR comments are " +
+				"available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related " +
+				"to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) " +
+				"repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).\n</details>\n",
 		},
 		{
 			name: "should add label and update status context if an user is member of the trusted org (one commit signed, one non-signed)",
@@ -415,6 +448,55 @@ Full details of the Developer Certificate of Origin can be found at [developerce
 
 <details>
 
+Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
+</details>
+`,
+		},
+		{
+			name: "should fail dco check as one unsigned commit is not from the trusted app",
+			config: plugins.Dco{
+				SkipDCOCheckForMembers: false,
+				TrustedOrg:             "kubernetes",
+				TrustedApps:            []string{"my-trusted-app"},
+			},
+			pullRequestEvent: github.PullRequestEvent{
+				Action:      github.PullRequestActionOpened,
+				PullRequest: github.PullRequest{Number: 3, Head: github.PullRequestBranch{SHA: "sha"}},
+			},
+			commits: []github.RepositoryCommit{
+				{
+					SHA:    "sha",
+					Commit: github.GitCommit{Message: "not signed off"},
+					Author: github.User{
+						Login: "test",
+					},
+				},
+				{
+					SHA:    "sha2",
+					Commit: github.GitCommit{Message: "not signed off"},
+					Author: github.User{
+						Login: "my-trusted-app",
+					},
+				},
+			},
+			issueState: "open",
+			hasDCONo:   false,
+			hasDCOYes:  false,
+
+			addedLabel:     fmt.Sprintf("/#3:%s", dcoNoLabel),
+			expectedStatus: github.StatusFailure,
+			addedComment: `/#3:Thanks for your pull request. Before we can look at it, you'll need to add a 'DCO signoff' to your commits.
+
+:memo: **Please follow instructions in the [contributing guide](https://github.com///blob/master/CONTRIBUTING.md) to update your commits with the DCO**
+
+Full details of the Developer Certificate of Origin can be found at [developercertificate.org](https://developercertificate.org/).
+
+**The list of commits missing DCO signoff**:
+
+- [sha](https://github.com///commits/sha) not signed off
+
+<details>
+
 Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
 </details>
 `,
@@ -463,7 +545,7 @@ Instructions for interacting with me using PR comments are available [here](http
 		{
 			name: "should skip dco check as commit is from a collaborator",
 			config: plugins.Dco{
-				SkipDCOCheckForMembers:       true,
+				SkipDCOCheckForMembers:       false,
 				SkipDCOCheckForCollaborators: true,
 				TrustedOrg:                   "kubernetes",
 			},
@@ -771,6 +853,35 @@ Instructions for interacting with me using PR comments are available [here](http
 			},
 			issueState: "open",
 
+			expectedStatus: github.StatusSuccess,
+		},
+		{
+			name: "should succeed from a trusted app",
+			config: plugins.Dco{
+				SkipDCOCheckForMembers: false,
+				TrustedApps:            []string{"my-trusted-app"},
+			},
+			commentEvent: github.GenericCommentEvent{
+				IssueState: "open",
+				Action:     github.GenericCommentActionCreated,
+				Body:       "/check-dco",
+				IsPR:       true,
+				Number:     3,
+			},
+			pullRequests: map[int]*github.PullRequest{
+				3: {Number: 3, Head: github.PullRequestBranch{SHA: "sha"}},
+			},
+			commits: []github.RepositoryCommit{
+				{
+					SHA:    "sha",
+					Commit: github.GitCommit{Message: "not a sign off"},
+					Author: github.User{
+						Login: "my-trusted-app",
+					},
+				},
+			},
+			issueState: "open",
+
 			expectedStatus: github.StatusSuccess,
 		},
 	}

prow/plugins/plugin-config-documented.yaml

@@ -369,17 +369,16 @@ dco:
         contributing_path: ' '
         # ContributingRepo is used to point users to a different repo containing CONTRIBUTING.md
         contributing_repo: ' '
-        # SkipDCOCheckForCollaborators is used to skip DCO check for trusted org members
+        # SkipDCOCheckForCollaborators is used to skip DCO check for repository collaborators.
         skip_dco_check_for_collaborators: true
-        # SkipDCOCheckForMembers is used to skip DCO check for trusted org members
+        # SkipDCOCheckForMembers is used to skip DCO check for org members.
         skip_dco_check_for_members: true
         # TrustedApps defines list of apps which commits will not be checked for DCO singoff.
         # The list should contain usernames of each GitHub App without [bot] suffix.
-        # By default, this option is ignored.
         trusted_apps:
             - ""
         # TrustedOrg is the org whose members' commits will not be checked for DCO signoff
-        # if the skip DCO option is enabled. The default is the PR's org.
+        # if the SkipDCOCheckForMembers option is enabled. The default is the PR's org.
         trusted_org: ' '
 # ExternalPlugins is a map of repositories (eg "k/k") to lists of
 # external plugins.