feat: allow to skip draft prs unless /ok-to-test is added

docs/config/plugins/github-com-jenkins-x-lighthouse-pkg-plugins.md

@@ -210,6 +210,7 @@ Trigger specifies a configuration for a single trigger.<br /><br />The configura
 | `only_org_members` | bool | No | OnlyOrgMembers requires PRs and/or /ok-to-test comments to come from org members.<br />By default, trigger also include repo collaborators. |
 | `ignore_ok_to_test` | bool | No | IgnoreOkToTest makes trigger ignore /ok-to-test comments.<br />This is a security mitigation to only allow testing from trusted users. |
 | `elide_skipped_contexts` | bool | No | ElideSkippedContexts makes trigger not post "Skipped" contexts for jobs<br />that could run but do not run. |
+| `skip_draft_pr` | bool | No | SkipDraftPR when enabled, skips triggering pipelines for draft PRs<br />unless /ok-to-test is added. |
 
 ## Welcome
 

docs/install_lighthouse_with_tekton.md

@@ -534,6 +534,7 @@ Now we need to add the sample project to the Lighthouse configuration and setup
       - $bot_user/$sample_repo_name
       ignore_ok_to_test: false
       elide_skipped_contexts: false
+      skip_draft_pr: false
     plugins:
       $bot_user/$config_repo_name:
       - config-updater

docs/plugins/Plugins config.md

@@ -215,6 +215,7 @@ Trigger specifies a configuration for a single trigger.<br /><br />The configura
 | OnlyOrgMembers | `only_org_members` | bool | No | OnlyOrgMembers requires PRs and/or /ok-to-test comments to come from org members.<br />By default, trigger also include repo collaborators. |
 | IgnoreOkToTest | `ignore_ok_to_test` | bool | No | IgnoreOkToTest makes trigger ignore /ok-to-test comments.<br />This is a security mitigation to only allow testing from trusted users. |
 | ElideSkippedContexts | `elide_skipped_contexts` | bool | No | ElideSkippedContexts makes trigger not post "Skipped" contexts for jobs<br />that could run but do not run. |
+| SkipDraftPR | `skip_draft_pr` | bool | No | SkipDraftPR when enabled, skips triggering pipelines for draft PRs<br />unless /ok-to-test is added. |
 
 ## Welcome
 

pkg/plugins/config.go

@@ -309,6 +309,8 @@ type Trigger struct {
 	// ElideSkippedContexts makes trigger not post "Skipped" contexts for jobs
 	// that could run but do not run.
 	ElideSkippedContexts bool `json:"elide_skipped_contexts,omitempty"`
+	// SkipDraftPR when enabled, skips triggering pipelines for draft PRs, unless /ok-to-test is added.
+	SkipDraftPR bool `json:"skip_draft_pr,omitempty"`
 }
 
 // Milestone contains the configuration options for the milestone and

pkg/plugins/trigger/generic-comment.go

@@ -59,7 +59,7 @@ func handleGenericComment(c Client, trigger *plugins.Trigger, gc scmprovider.Gen
 	var l []*scm.Label
 	if !trusted {
 		// Skip untrusted PRs.
-		l, trusted, err = TrustedPullRequest(c.SCMProviderClient, trigger, gc.IssueAuthor.Login, org, repo, number, nil)
+		l, trusted, err = TrustedOrDraftPullRequest(c.SCMProviderClient, trigger, gc.IssueAuthor.Login, org, repo, number, pr.Draft, nil)
 		if err != nil {
 			return err
 		}
@@ -71,7 +71,7 @@ func handleGenericComment(c Client, trigger *plugins.Trigger, gc scmprovider.Gen
 	}
 
 	// At this point we can trust the PR, so we eventually update labels.
-	// Ensure we have labels before test, because TrustedPullRequest() won't be called
+	// Ensure we have labels before test, because TrustedOrDraftPullRequest() won't be called
 	// when commentAuthor is trusted.
 	if l == nil {
 		l, err = c.SCMProviderClient.GetIssueLabels(org, repo, number, gc.IsPR)

pkg/plugins/trigger/pull-request.go

@@ -18,6 +18,8 @@ package trigger
 
 import (
 	"fmt"
+	"net/url"
+
 	"github.com/jenkins-x/go-scm/scm"
 	"github.com/jenkins-x/lighthouse/pkg/config"
 	"github.com/jenkins-x/lighthouse/pkg/config/job"
@@ -28,7 +30,6 @@ import (
 	"github.com/jenkins-x/lighthouse/pkg/scmprovider"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
-	"net/url"
 )
 
 func handlePR(c Client, trigger *plugins.Trigger, pr scm.PullRequestHook) error {
@@ -55,44 +56,36 @@ func handlePR(c Client, trigger *plugins.Trigger, pr scm.PullRequestHook) error
 			}
 			return nil
 		}
+		if trigger.SkipDraftPR && pr.PullRequest.Draft {
+			c.Logger.Infof("Skipping build for draft PR %d unless /ok-to-test is added.", num)
+			if err = welcomeMsgForDraftPR(c.SCMProviderClient, trigger, pr.PullRequest); err != nil {
+				return fmt.Errorf("could not welcome for draft pr %q: %v", author, err)
+			}
+			return nil
+		}
 
 		if err = infoMsg(c, pr.PullRequest); err != nil {
 			return err
 		}
 		c.Logger.Infof("Author %q is a member, Starting all jobs for new PR.", author)
 		return buildAll(c, &pr.PullRequest, pr.GUID, trigger.ElideSkippedContexts)
 	case scm.ActionReopen:
-		// When a PR is reopened, check that the user is in the org or that an org
-		// member had said "/ok-to-test" before building, resulting in label ok-to-test.
-		l, trusted, err := TrustedPullRequest(c.SCMProviderClient, trigger, author, org, repo, num, nil)
-		if err != nil {
-			return fmt.Errorf("could not validate PR: %s", err)
-		} else if trusted {
-			// Eventually remove need-ok-to-test
-			// Does not work for TrustedUser() == true since labels are not fetched in this case
-			if scmprovider.HasLabel(labels.NeedsOkToTest, l) {
-				if err := c.SCMProviderClient.RemoveLabel(org, repo, num, labels.NeedsOkToTest, true); err != nil {
-					return err
-				}
-			}
-			c.Logger.Info("Starting all jobs for updated PR.")
-			return buildAll(c, &pr.PullRequest, pr.GUID, trigger.ElideSkippedContexts)
-		}
+		return buildAllIfTrustedOrDraft(c, trigger, pr)
 	case scm.ActionEdited, scm.ActionUpdate:
 		// if someone changes the base of their PR, we will get this
 		// event and the changes field will list that the base SHA and
 		// ref changes so we can detect such a case and retrigger tests
 		changes := pr.Changes
 		if changes.Base.Ref.From != "" || changes.Base.Sha.From != "" {
 			// the base of the PR changed and we need to re-test it
-			return buildAllIfTrusted(c, trigger, pr)
+			return buildAllIfTrustedOrDraft(c, trigger, pr)
 		}
-	case scm.ActionSync:
-		return buildAllIfTrusted(c, trigger, pr)
+	case scm.ActionSync, scm.ActionReadyForReview, scm.ActionConvertedToDraft:
+		return buildAllIfTrustedOrDraft(c, trigger, pr)
 	case scm.ActionLabel:
 		// When a PR is LGTMd, if it is untrusted then build it once.
 		if pr.Label.Name == labels.LGTM {
-			_, trusted, err := TrustedPullRequest(c.SCMProviderClient, trigger, author, org, repo, num, nil)
+			_, trusted, err := TrustedOrDraftPullRequest(c.SCMProviderClient, trigger, author, org, repo, num, pr.PullRequest.Draft, nil)
 			if err != nil {
 				return fmt.Errorf("could not validate PR: %s", err)
 			} else if !trusted {
@@ -115,14 +108,12 @@ func orgRepoAuthor(pr scm.PullRequest) (string, string, login) {
 	return org, repo, login(author)
 }
 
-func buildAllIfTrusted(c Client, trigger *plugins.Trigger, pr scm.PullRequestHook) error {
-	// When a PR is updated, check that the user is in the org or that an org
-	// member has said "/ok-to-test" before building. There's no need to ask
-	// for "/ok-to-test" because we do that once when the PR is created.
+func buildAllIfTrustedOrDraft(c Client, trigger *plugins.Trigger, pr scm.PullRequestHook) error {
+	// When a PR is updated, check if the user is trusted or if the PR is a draft.
 	org, repo, a := orgRepoAuthor(pr.PullRequest)
 	author := string(a)
 	num := pr.PullRequest.Number
-	l, trusted, err := TrustedPullRequest(c.SCMProviderClient, trigger, author, org, repo, num, nil)
+	l, trusted, err := TrustedOrDraftPullRequest(c.SCMProviderClient, trigger, author, org, repo, num, pr.PullRequest.Draft, nil)
 	if err != nil {
 		return fmt.Errorf("could not validate PR: %s", err)
 	} else if trusted {
@@ -133,6 +124,13 @@ func buildAllIfTrusted(c Client, trigger *plugins.Trigger, pr scm.PullRequestHoo
 				return err
 			}
 		}
+
+		// Skip pipelines trigger if SkipDraftPR enabled and PR is a draft without OkToTest label
+		if trigger.SkipDraftPR && pr.PullRequest.Draft && !scmprovider.HasLabel(labels.OkToTest, l) {
+			c.Logger.Info("Skipping pipelines trigger for draft PR.")
+			return nil
+		}
+
 		c.Logger.Info("Starting all jobs for updated PR.")
 		return buildAll(c, &pr.PullRequest, pr.GUID, trigger.ElideSkippedContexts)
 	}
@@ -242,23 +240,49 @@ I understand the commands that are listed [here](https://jenkins-x.io/v3/develop
 	return nil
 }
 
-// TrustedPullRequest returns whether or not the given PR should be tested.
-// It first checks if the author is in the org, then looks for "ok-to-test" label.
-func TrustedPullRequest(spc scmProviderClient, trigger *plugins.Trigger, author, org, repo string, num int, l []*scm.Label) ([]*scm.Label, bool, error) {
-	// First check if the author is a member of the org.
-	if orgMember, err := TrustedUser(spc, trigger, author, org, repo); err != nil {
-		return l, false, fmt.Errorf("error checking %s for trust: %v", author, err)
-	} else if orgMember {
-		return l, true, nil
+func welcomeMsgForDraftPR(spc scmProviderClient, trigger *plugins.Trigger, pr scm.PullRequest) error {
+	org, repo, author := orgRepoAuthor(pr)
+
+	comment := fmt.Sprintf("Hi @%s. This is a draft PR. Thanks for your contribution!\n", author)
+
+	var errors []error
+
+	if !trigger.IgnoreOkToTest {
+		comment += "To trigger tests on this draft PR, please mark it as ready for review by removing the draft status. Collaborators can also trigger tests on draft PRs using /ok-to-test."
+
+		if err := spc.AddLabel(org, repo, pr.Number, labels.NeedsOkToTest, true); err != nil {
+			errors = append(errors, err)
+		}
+	}
+
+	if err := spc.CreateComment(org, repo, pr.Number, true, comment); err != nil {
+		errors = append(errors, err)
+	}
+
+	if len(errors) > 0 {
+		return errorutil.NewAggregate(errors...)
 	}
-	// Then check if PR has ok-to-test label
+	return nil
+}
+
+// TrustedOrDraftPullRequest returns whether or not the given PR should be tested.
+// It first checks if the author is in the org, then looks for "ok-to-test" label.
+func TrustedOrDraftPullRequest(spc scmProviderClient, trigger *plugins.Trigger, author, org, repo string, num int, isDraft bool, l []*scm.Label) ([]*scm.Label, bool, error) {
+	// First get PR labels
 	if l == nil {
 		var err error
 		l, err = spc.GetIssueLabels(org, repo, num, true)
 		if err != nil {
 			return l, false, fmt.Errorf("error getting issue labels: %v", err)
 		}
 	}
+	// Then check if the author is a member of the org and if trigger.SkipDraftPR disabled or PR not in draft
+	if orgMember, err := TrustedUser(spc, trigger, author, org, repo); err != nil {
+		return l, false, fmt.Errorf("error checking %s for trust: %v", author, err)
+	} else if orgMember && (!trigger.SkipDraftPR || !isDraft) {
+		return l, true, nil
+	}
+	// Then check if PR has ok-to-test label (untrusted user or trigger.SkipDraftPR enabled && draft PR)
 	return l, scmprovider.HasLabel(labels.OkToTest, l), nil
 }