From 9b9c426da37604351298aff6147f673f8b8179f8 Mon Sep 17 00:00:00 2001 From: Dale Haiducek <19750917+dhaiducek@users.noreply.github.com> Date: Thu, 6 Apr 2023 16:47:52 -0400 Subject: [PATCH] Update trusted author handling - Separate member and collaborator configurations - Separate trusted apps from the member and collaborator handling Signed-off-by: Dale Haiducek <19750917+dhaiducek@users.noreply.github.com> --- pkg/plugins/config.go | 7 +- pkg/plugins/dco/dco.go | 36 +++++-- pkg/plugins/dco/dco_test.go | 113 +++++++++++++++++++++- pkg/plugins/plugin-config-documented.yaml | 7 +- 4 files changed, 148 insertions(+), 15 deletions(-) diff --git a/pkg/plugins/config.go b/pkg/plugins/config.go index 873fe5c011..e8c54c3c55 100644 --- a/pkg/plugins/config.go +++ b/pkg/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"` diff --git a/pkg/plugins/dco/dco.go b/pkg/plugins/dco/dco.go index d8780e4929..011b1a843f 100644 --- a/pkg/plugins/dco/dco.go +++ b/pkg/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 diff --git a/pkg/plugins/dco/dco_test.go b/pkg/plugins/dco/dco_test.go index 342fbc4878..26284044d9 100644 --- a/pkg/plugins/dco/dco_test.go +++ b/pkg/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
\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
\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
+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). +
+`, + }, + { + 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 + +
+ 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).
`, @@ -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, }, } diff --git a/pkg/plugins/plugin-config-documented.yaml b/pkg/plugins/plugin-config-documented.yaml index cb61c9eb21..f2c688c94d 100644 --- a/pkg/plugins/plugin-config-documented.yaml +++ b/pkg/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.