Implement ConfigTree in approve plugin

kubernetes · May 4, 2019 · 5785601 · 5785601
1 parent 2228dd1
commit 5785601
Show file tree

Hide file tree

Showing 7 changed files with 297 additions and 151 deletions.
diff --git a/prow/plugins.yaml b/prow/plugins.yaml
@@ -42,57 +42,71 @@ use_deprecated_2018_implicit_self_approve_default_migrate_before_july_2019: true
 use_deprecated_2018_review_acts_as_approve_default_migrate_before_july_2019: true
 
 approve:
-- repos:
-  - kubernetes/cloud-provider-aws
-  - kubernetes/cloud-provider-azure
-  - kubernetes/cluster-registry
-  - kubernetes/contrib
-  - kubernetes/dashboard
-  - kubernetes/dns
-  - kubernetes/enhancements
-  - kubernetes/examples
-  - kubernetes/federation
-  - kubernetes/gengo
-  - kubernetes/ingress-gce
-  - kubernetes/ingress-nginx
-  - kubernetes/klog
-  - kubernetes/kube-deploy
-  - kubernetes/kubeadm
-  - kubernetes/kubectl
-  - kubernetes/kubernetes-template-project
-  - kubernetes/kube-state-metrics
-  - kubernetes/minikube
-  - kubernetes/node-problem-detector
-  - kubernetes/perf-tests
-  - kubernetes/publishing-bot
-  - kubernetes/release
-  - kubernetes/repo-infra
-  - kubernetes/sig-release
-  - kubernetes/steering
-  - kubernetes/utils
-  - kubernetes-incubator/ip-masq-agent
-  require_self_approval: false
-  lgtm_acts_as_approve: true
-- repos:
-  - kubernetes/kops
-  - kubernetes/kubernetes
-  - kubernetes-client
-  - kubernetes-csi
-  - kubernetes-sigs
-  - client-go/unofficial-docs
-  require_self_approval: false
-- repos:
-  - bazelbuild
-  - kubernetes/community
-  - kubernetes/org
-  - kubernetes/test-infra
-  - kubernetes-sigs/kind
-  require_self_approval: false
-  ignore_review_state: false
-- repos:
-  - helm/charts
-  require_self_approval: false
-  lgtm_acts_as_approve: true
+  orgs:
+    helm:
+      repos:
+        charts:
+          lgtm_acts_as_approve: true
+    kubernetes:
+      repos:
+        cloud-provider-aws:
+          lgtm_acts_as_approve: true
+        cloud-provider-azure:
+          lgtm_acts_as_approve: true
+        cluster-registry:
+          lgtm_acts_as_approve: true
+        contrib:
+          lgtm_acts_as_approve: true
+        dashboard:
+          lgtm_acts_as_approve: true
+        dns:
+          lgtm_acts_as_approve: true
+        enhancements:
+          lgtm_acts_as_approve: true
+        examples:
+          lgtm_acts_as_approve: true
+        federation:
+          lgtm_acts_as_approve: true
+        gengo:
+          lgtm_acts_as_approve: true
+        ingress-gce:
+          lgtm_acts_as_approve: true
+        ingress-nginx:
+          lgtm_acts_as_approve: true
+        klog:
+          lgtm_acts_as_approve: true
+        kube-deploy:
+          lgtm_acts_as_approve: true
+        kubeadm:
+          lgtm_acts_as_approve: true
+        kubectl:
+          lgtm_acts_as_approve: true
+        kubernetes-template-project:
+          lgtm_acts_as_approve: true
+        kube-state-metrics:
+          lgtm_acts_as_approve: true
+        minikube:
+          lgtm_acts_as_approve: true
+        node-problem-detector:
+          lgtm_acts_as_approve: true
+        perf-tests:
+          lgtm_acts_as_approve: true
+        publishing-bot:
+          lgtm_acts_as_approve: true
+        release:
+          lgtm_acts_as_approve: true
+        repo-infra:
+          lgtm_acts_as_approve: true
+        sig-release:
+          lgtm_acts_as_approve: true
+        steering:
+          lgtm_acts_as_approve: true
+        utils:
+          lgtm_acts_as_approve: true
+    kubernetes-incubator:
+      repos:
+        ip-masq-agent:
+          lgtm_acts_as_approve: true
 
 # Lower bounds in number of lines changed; XS is assumed to be zero.
 size:

diff --git a/prow/plugins/BUILD.bazel b/prow/plugins/BUILD.bazel
@@ -10,6 +10,7 @@ go_test(
     name = "go_default_test",
     srcs = [
         "config_test.go",
+        "helpers_test.go",
         "plugins_test.go",
         "respond_test.go",
     ],
@@ -25,6 +26,7 @@ go_library(
     name = "go_default_library",
     srcs = [
         "config.go",
+        "helpers.go",
         "plugins.go",
         "respond.go",
     ],

diff --git a/prow/plugins/approve/approve.go b/prow/plugins/approve/approve.go
@@ -113,12 +113,12 @@ func helpProvider(config *plugins.Configuration, enabledRepos []string) (*plugin
 	approveConfig := map[string]string{}
 	for _, repo := range enabledRepos {
 		parts := strings.Split(repo, "/")
-		var opts *plugins.Approve
+		var opts plugins.Approve
 		switch len(parts) {
 		case 1:
-			opts = optionsForRepo(config, repo, "")
+			opts = config.Approve.GetLeafConfig(repo, "", "").(plugins.Approve)
 		case 2:
-			opts = optionsForRepo(config, parts[0], parts[1])
+			opts = config.Approve.GetLeafConfig(parts[0], parts[1], "").(plugins.Approve)
 		default:
 			return nil, fmt.Errorf("invalid repo in enabledRepos: %q", repo)
 		}
@@ -142,17 +142,18 @@ func helpProvider(config *plugins.Configuration, enabledRepos []string) (*plugin
 }
 
 func handleGenericCommentEvent(pc plugins.Agent, ce github.GenericCommentEvent) error {
+	opts := pc.PluginConfig.Approve.GetLeafConfig(ce.Repo.Owner.Login, ce.Repo.Name, "").(plugins.Approve)
 	return handleGenericComment(
 		pc.Logger,
 		pc.GitHubClient,
 		pc.OwnersClient,
 		pc.Config.GitHubOptions,
-		pc.PluginConfig,
+		&opts,
 		&ce,
 	)
 }
 
-func handleGenericComment(log *logrus.Entry, ghc githubClient, oc ownersClient, githubConfig config.GitHubOptions, config *plugins.Configuration, ce *github.GenericCommentEvent) error {
+func handleGenericComment(log *logrus.Entry, ghc githubClient, oc ownersClient, githubConfig config.GitHubOptions, opts *plugins.Approve, ce *github.GenericCommentEvent) error {
 	if ce.Action != github.GenericCommentActionCreated || !ce.IsPR || ce.IssueState == "closed" {
 		return nil
 	}
@@ -162,7 +163,6 @@ func handleGenericComment(log *logrus.Entry, ghc githubClient, oc ownersClient,
 		return err
 	}
 
-	opts := optionsForRepo(config, ce.Repo.Owner.Login, ce.Repo.Name)
 	if !isApprovalCommand(botName, opts.LgtmActsAsApprove, &comment{Body: ce.Body, Author: ce.User.Login}) {
 		return nil
 	}
@@ -199,17 +199,18 @@ func handleGenericComment(log *logrus.Entry, ghc githubClient, oc ownersClient,
 // handleReviewEvent should only handle reviews that have no approval command.
 // Reviews with approval commands will be handled by handleGenericCommentEvent.
 func handleReviewEvent(pc plugins.Agent, re github.ReviewEvent) error {
+	opts := pc.PluginConfig.Approve.GetLeafConfig(re.Repo.Owner.Login, re.Repo.Name, "").(plugins.Approve)
 	return handleReview(
 		pc.Logger,
 		pc.GitHubClient,
 		pc.OwnersClient,
 		pc.Config.GitHubOptions,
-		pc.PluginConfig,
+		&opts,
 		&re,
 	)
 }
 
-func handleReview(log *logrus.Entry, ghc githubClient, oc ownersClient, githubConfig config.GitHubOptions, config *plugins.Configuration, re *github.ReviewEvent) error {
+func handleReview(log *logrus.Entry, ghc githubClient, oc ownersClient, githubConfig config.GitHubOptions, opts *plugins.Approve, re *github.ReviewEvent) error {
 	if re.Action != github.ReviewActionSubmitted && re.Action != github.ReviewActionDismissed {
 		return nil
 	}
@@ -219,8 +220,6 @@ func handleReview(log *logrus.Entry, ghc githubClient, oc ownersClient, githubCo
 		return err
 	}
 
-	opts := optionsForRepo(config, re.Repo.Owner.Login, re.Repo.Name)
-
 	// Check for an approval command is in the body. If one exists, let the
 	// genericCommentEventHandler handle this event. Approval commands override
 	// review state.
@@ -244,7 +243,7 @@ func handleReview(log *logrus.Entry, ghc githubClient, oc ownersClient, githubCo
 		ghc,
 		repo,
 		githubConfig,
-		optionsForRepo(config, re.Repo.Owner.Login, re.Repo.Name),
+		opts,
 		&state{
 			org:       re.Repo.Owner.Login,
 			repo:      re.Repo.Name,
@@ -260,17 +259,18 @@ func handleReview(log *logrus.Entry, ghc githubClient, oc ownersClient, githubCo
 }
 
 func handlePullRequestEvent(pc plugins.Agent, pre github.PullRequestEvent) error {
+	opts := pc.PluginConfig.Approve.GetLeafConfig(pre.Repo.Owner.Login, pre.Repo.Name, "").(plugins.Approve)
 	return handlePullRequest(
 		pc.Logger,
 		pc.GitHubClient,
 		pc.OwnersClient,
 		pc.Config.GitHubOptions,
-		pc.PluginConfig,
+		&opts,
 		&pre,
 	)
 }
 
-func handlePullRequest(log *logrus.Entry, ghc githubClient, oc ownersClient, githubConfig config.GitHubOptions, config *plugins.Configuration, pre *github.PullRequestEvent) error {
+func handlePullRequest(log *logrus.Entry, ghc githubClient, oc ownersClient, githubConfig config.GitHubOptions, opts *plugins.Approve, pre *github.PullRequestEvent) error {
 	if pre.Action != github.PullRequestActionOpened &&
 		pre.Action != github.PullRequestActionReopened &&
 		pre.Action != github.PullRequestActionSynchronize &&
@@ -296,7 +296,7 @@ func handlePullRequest(log *logrus.Entry, ghc githubClient, oc ownersClient, git
 		ghc,
 		repo,
 		githubConfig,
-		optionsForRepo(config, pre.Repo.Owner.Login, pre.Repo.Name),
+		opts,
 		&state{
 			org:       pre.Repo.Owner.Login,
 			repo:      pre.Repo.Name,
@@ -603,50 +603,6 @@ func addApprovers(approversHandler *approvers.Approvers, approveComments []*comm
 	}
 }
 
-// optionsForRepo gets the plugins.Approve struct that is applicable to the indicated repo.
-func optionsForRepo(config *plugins.Configuration, org, repo string) *plugins.Approve {
-	fullName := fmt.Sprintf("%s/%s", org, repo)
-
-	a := func() *plugins.Approve {
-		// First search for repo config
-		for _, c := range config.Approve {
-			if !strInSlice(fullName, c.Repos) {
-				continue
-			}
-			return &c
-		}
-
-		// If you don't find anything, loop again looking for an org config
-		for _, c := range config.Approve {
-			if !strInSlice(org, c.Repos) {
-				continue
-			}
-			return &c
-		}
-
-		// Return an empty config, and use plugin defaults
-		return &plugins.Approve{}
-	}()
-	if a.DeprecatedImplicitSelfApprove == nil && a.RequireSelfApproval == nil && config.UseDeprecatedSelfApprove {
-		no := false
-		a.DeprecatedImplicitSelfApprove = &no
-	}
-	if a.DeprecatedReviewActsAsApprove == nil && a.IgnoreReviewState == nil && config.UseDeprecatedReviewApprove {
-		no := false
-		a.DeprecatedReviewActsAsApprove = &no
-	}
-	return a
-}
-
-func strInSlice(str string, slice []string) bool {
-	for _, elem := range slice {
-		if elem == str {
-			return true
-		}
-	}
-	return false
-}
-
 type comment struct {
 	Body        string
 	Author      string