Add policy checks to Azure DevOps

go.mod

@@ -32,7 +32,7 @@ require (
 	github.com/imdario/mergo v0.3.5 // indirect
 	github.com/inconshreveable/mousetrap v1.0.0 // indirect
 	github.com/magiconair/properties v1.7.3 // indirect
-	github.com/mcdafydd/go-azuredevops v0.10.2
+	github.com/mcdafydd/go-azuredevops v0.11.0
 	github.com/microcosm-cc/bluemonday v1.0.1
 	github.com/mitchellh/colorstring v0.0.0-20150917214807-8631ce90f286
 	github.com/mitchellh/go-homedir v1.0.0
@@ -44,7 +44,7 @@ require (
 	github.com/openzipkin/zipkin-go v0.1.3 // indirect
 	github.com/pelletier/go-buffruneio v0.2.0 // indirect
 	github.com/pelletier/go-toml v1.0.0 // indirect
-	github.com/petergtz/pegomock v2.5.0+incompatible
+	github.com/petergtz/pegomock v2.7.0+incompatible
 	github.com/pkg/errors v0.8.0
 	github.com/prometheus/client_golang v0.9.3-0.20190127221311-3c4408c8b829 // indirect
 	github.com/spf13/afero v0.0.0-20170901052352-ee1bd8ee15a1 // indirect
@@ -58,7 +58,7 @@ require (
 	github.com/urfave/negroni v0.2.0
 	github.com/xanzy/go-gitlab v0.22.2-0.20191127083556-16a492660b8c
 	golang.org/x/build v0.0.0-20190111050920-041ab4dc3f9d // indirect
-	golang.org/x/crypto v0.0.0-20190605123033-f99c8df09eb5
+	golang.org/x/crypto v0.0.0-20200403201458-baeed622b8d8
 	golang.org/x/net v0.0.0-20191126235420-ef20fe5d7933 // indirect
 	golang.org/x/oauth2 v0.0.0-20191122200657-5d9234df094c // indirect
 	google.golang.org/appengine v1.6.5 // indirect

go.sum

@@ -183,6 +183,7 @@ github.com/kr/pty v1.1.3/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
 github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE=
 github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
 github.com/kylelemons/godebug v0.0.0-20170820004349-d65d576e9348/go.mod h1:B69LEHPfb2qLo0BaaOLcbitczOKLWTsrBG9LczfCD4k=
+github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc=
 github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw=
 github.com/magiconair/properties v1.7.3 h1:6AOjgCKyZFMG/1yfReDPDz3CJZPxnYk7DGmj2HtyF24=
 github.com/magiconair/properties v1.7.3/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ=
@@ -200,6 +201,8 @@ github.com/mcdafydd/go-azuredevops v0.10.0 h1:gfwMWZ3Gn14qTDbx1U2Ut2YNzdu64DqPQ5
 github.com/mcdafydd/go-azuredevops v0.10.0/go.mod h1:/NYbgJ/1+9+SmG5CjETCoWm+FlLNcRwdiw1/AGW9zm0=
 github.com/mcdafydd/go-azuredevops v0.10.2 h1:cVAxfGqSUK7i4ZRc7s+EpeWSOrDgkBM4SzTRI/IUfoE=
 github.com/mcdafydd/go-azuredevops v0.10.2/go.mod h1:/NYbgJ/1+9+SmG5CjETCoWm+FlLNcRwdiw1/AGW9zm0=
+github.com/mcdafydd/go-azuredevops v0.11.0 h1:DHUctw4lNpPfExpwAxDsdat+n4IitH3vYLAxSGs9y0E=
+github.com/mcdafydd/go-azuredevops v0.11.0/go.mod h1:B4UDyn7WEj1/97f45j3VnzEfkWKe05+/dCcAPdOET4A=
 github.com/microcosm-cc/bluemonday v1.0.1 h1:SIYunPjnlXcW+gVfvm0IlSeR5U3WZUOLfVmqg85Go44=
 github.com/microcosm-cc/bluemonday v1.0.1/go.mod h1:hsXNsILzKxV+sX77C5b8FSuKF00vh2OMYv+xgHpAMF4=
 github.com/mitchellh/colorstring v0.0.0-20150917214807-8631ce90f286 h1:KHyL+3mQOF9sPfs26lsefckcFNDcIZtiACQiECzIUkw=
@@ -240,6 +243,8 @@ github.com/pelletier/go-toml v1.0.0 h1:QFDlmAXZrfPXEF6c9+15fMqhQIS3O0pxszhnk936v
 github.com/pelletier/go-toml v1.0.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic=
 github.com/petergtz/pegomock v2.5.0+incompatible h1:NgwX1/qc+tsl7I45OkDxYZ1mIonYWbOESnpZcd20sR0=
 github.com/petergtz/pegomock v2.5.0+incompatible/go.mod h1:nuBLWZpVyv/fLo56qTwt/AUau7jgouO1h7bEvZCq82o=
+github.com/petergtz/pegomock v2.7.0+incompatible h1:42rJ5wIOBAg9OGdkLaPW9PlF/RtqDc5aGl6PcTCXl3o=
+github.com/petergtz/pegomock v2.7.0+incompatible/go.mod h1:nuBLWZpVyv/fLo56qTwt/AUau7jgouO1h7bEvZCq82o=
 github.com/pkg/errors v0.8.0 h1:WdK/asTD0HN+q6hsWO3/vpuAkAr+tw6aNJNDFFf0+qw=
 github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
@@ -255,6 +260,7 @@ github.com/prometheus/procfs v0.0.0-20180725123919-05ee40e3a273/go.mod h1:c3At6R
 github.com/prometheus/procfs v0.0.0-20181005140218-185b4288413d/go.mod h1:c3At6R/oaqEKCNdg8wHV1ftS6bRYblBhIjjI8uT2IGk=
 github.com/prometheus/procfs v0.0.0-20190117184657-bf6a532e95b1/go.mod h1:c3At6R/oaqEKCNdg8wHV1ftS6bRYblBhIjjI8uT2IGk=
 github.com/russross/blackfriday v1.5.2/go.mod h1:JO/DiYxRf+HjHt06OyowR9PTA263kcR/rfWxYHBV53g=
+github.com/sergi/go-diff v1.0.0 h1:Kpca3qRNrduNnOQeazBd0ysaKrUJiIuISHxogkT9RPQ=
 github.com/sergi/go-diff v1.0.0/go.mod h1:0CfEIISq7TuYL3j771MWULgwwjU+GofnZX9QAmXWZgo=
 github.com/shurcooL/component v0.0.0-20170202220835-f88ec8f54cc4/go.mod h1:XhFIlyj5a1fBNx5aJTbKoIq0mNaPvOagO+HjB3EtxrY=
 github.com/shurcooL/events v0.0.0-20181021180414-410e4ca65f48/go.mod h1:5u70Mqkb5O5cxEA8nxTsgrgLehJeAw6Oc4Ab1c/P1HM=
@@ -303,6 +309,7 @@ github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+
 github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
 github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q=
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
+github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk=
 github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
 github.com/tarm/serial v0.0.0-20180830185346-98f6abe2eb07/go.mod h1:kDXzergiv9cbyO7IOYJZWg1U88JhDg3PB6klq9Hg2pA=
 github.com/ulikunitz/xz v0.5.5/go.mod h1:2bypXElzHzzJZwzH67Y6wb67pO62Rzfn7BSiF4ABRW8=
@@ -338,6 +345,8 @@ golang.org/x/crypto v0.0.0-20190426145343-a29dc8fdc734 h1:p/H982KKEjUnLJkM3tt/Le
 golang.org/x/crypto v0.0.0-20190426145343-a29dc8fdc734/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
 golang.org/x/crypto v0.0.0-20190605123033-f99c8df09eb5 h1:58fnuSXlxZmFdJyvtTFVmVhcMLU6v5fEb/ok4wyqtNU=
 golang.org/x/crypto v0.0.0-20190605123033-f99c8df09eb5/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
+golang.org/x/crypto v0.0.0-20200403201458-baeed622b8d8 h1:fpnn/HnJONpIu6hkXi1u/7rR0NzilgWr4T0JmWkEitk=
+golang.org/x/crypto v0.0.0-20200403201458-baeed622b8d8/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
 golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
 golang.org/x/exp v0.0.0-20190510132918-efd6b22b2522/go.mod h1:ZjyILWgesfNpC6sMxTJOJm9Kp84zZh5NQWvqDGG3Qr8=
 golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js=

server/events/vcs/azuredevops_client.go

@@ -6,6 +6,7 @@ import (
 	"net/http"
 	"net/url"
 	"path/filepath"
+	"strconv"
 	"strings"
 	"time"
 
@@ -124,43 +125,82 @@ func (g *AzureDevopsClient) CreateComment(repo models.Repo, pullNum int, comment
 	return nil
 }
 
-// PullIsApproved returns true if the merge request was approved.
-// https://docs.microsoft.com/en-us/azure/devops/repos/git/branch-policies?view=azure-devops#require-a-minimum-number-of-reviewers
+// PullIsApproved returns true if the merge request was approved by another reviewer.
 func (g *AzureDevopsClient) PullIsApproved(repo models.Repo, pull models.PullRequest) (bool, error) {
+	owner, project, repoName := SplitAzureDevopsRepoFullName(repo.FullName)
+
 	opts := azuredevops.PullRequestGetOptions{
 		IncludeWorkItemRefs: true,
 	}
-	owner, project, repoName := SplitAzureDevopsRepoFullName(repo.FullName)
 	adPull, _, err := g.Client.PullRequests.GetWithRepo(g.ctx, owner, project, repoName, pull.Num, &opts)
 	if err != nil {
-		return false, errors.Wrap(err, "getting pull request")
+		return false, fmt.Errorf("get pull request: %w", err)
 	}
+
 	for _, review := range adPull.Reviewers {
 		if review == nil {
 			continue
 		}
-		if review.GetVote() == azuredevops.VoteApproved {
+
+		if review.IdentityRef.GetUniqueName() == adPull.GetCreatedBy().GetUniqueName() {
+			continue
+		}
+
+		if review.GetVote() == azuredevops.VoteApproved || review.GetVote() == azuredevops.VoteApprovedWithSuggestions {
 			return true, nil
 		}
 	}
+
 	return false, nil
 }
 
 // PullIsMergeable returns true if the merge request can be merged.
 func (g *AzureDevopsClient) PullIsMergeable(repo models.Repo, pull models.PullRequest) (bool, error) {
-	opts := azuredevops.PullRequestGetOptions{
-		IncludeWorkItemRefs: true,
-	}
 	owner, project, repoName := SplitAzureDevopsRepoFullName(repo.FullName)
+
+	opts := azuredevops.PullRequestGetOptions{IncludeWorkItemRefs: true}
 	adPull, _, err := g.Client.PullRequests.GetWithRepo(g.ctx, owner, project, repoName, pull.Num, &opts)
 	if err != nil {
-		return false, errors.Wrap(err, "getting pull request")
+		return false, fmt.Errorf("get pull request: %w", err)
 	}
-	if *adPull.MergeStatus != azuredevops.MergeConflicts.String() &&
-		*adPull.MergeStatus != azuredevops.MergeRejectedByPolicy.String() {
-		return true, nil
+
+	if *adPull.MergeStatus != azuredevops.MergeSucceeded.String() {
+		return false, nil
 	}
-	return false, nil
+
+	if *adPull.IsDraft {
+		return false, nil
+	}
+
+	if *adPull.Status != "active" {
+		return false, nil
+	}
+
+	projectID := *adPull.Repository.Project.ID
+	artifactID := g.Client.PolicyEvaluations.GetPullRequestArtifactID(projectID, strconv.Itoa(pull.Num))
+	policyEvaluations, _, err := g.Client.PolicyEvaluations.List(g.ctx, owner, project, artifactID, &azuredevops.PolicyEvaluationsListOptions{})
+	if err != nil {
+		return false, fmt.Errorf("list policy evaluations: %w", err)
+	}
+
+	for _, policyEvaluation := range policyEvaluations {
+		if *policyEvaluation.Configuration.IsEnabled == false || *policyEvaluation.Configuration.IsDeleted {
+			continue
+		}
+
+		// Ignore the Atlantis status, even if its set as a blocker.
+		// This status should not be considered when evaluating if the pull request can be applied.
+		settings := (policyEvaluation.Configuration.Settings).(map[string]interface{})
+		if settings["statusName"] == "atlantis/apply" {
+			continue
+		}
+
+		if *policyEvaluation.Configuration.IsBlocking && *policyEvaluation.Status != "approved" {
+			return false, nil
+		}
+	}
+
+	return true, nil
 }
 
 // GetPullRequest returns the pull request.

server/events/vcs/azuredevops_client_test.go

@@ -278,64 +278,73 @@ func TestAzureDevopsClient_GetModifiedFiles(t *testing.T) {
 
 func TestAzureDevopsClient_PullIsMergeable(t *testing.T) {
 	cases := []struct {
-		state        string
+		testName     string
+		mergeStatus  string
+		policyStatus string
 		expMergeable bool
 	}{
 		{
+			"merge conflicts",
 			azuredevops.MergeConflicts.String(),
+			"approved",
 			false,
 		},
 		{
-			azuredevops.MergeRejectedByPolicy.String(),
+			"rejected policy status",
+			azuredevops.MergeSucceeded.String(),
+			"rejected",
 			false,
 		},
 		{
-			azuredevops.MergeFailure.String(),
-			true,
-		},
-		{
-			azuredevops.MergeNotSet.String(),
-			true,
-		},
-		{
-			azuredevops.MergeQueued.String(),
+			"rejected policy status on disabled policy",
+			azuredevops.MergeSucceeded.String(),
+			"rejected",
 			true,
 		},
 		{
+			"merge succeeded",
 			azuredevops.MergeSucceeded.String(),
+			"approved",
 			true,
 		},
 	}
 
-	// Use a real Azure DevOps json response and edit the mergeable_state field.
-	jsBytes, err := ioutil.ReadFile("fixtures/azuredevops-pr.json")
+	jsonPullRequestBytes, err := ioutil.ReadFile("fixtures/azuredevops-pr.json")
 	Ok(t, err)
-	json := string(jsBytes)
+
+	jsonPolicyEvaluationBytes, err := ioutil.ReadFile("fixtures/azuredevops-policyevaluations.json")
+	Ok(t, err)
+
+	pullRequestBody := string(jsonPullRequestBytes)
+	policyEvaluationsBody := string(jsonPolicyEvaluationBytes)
 
 	for _, c := range cases {
-		t.Run(c.state, func(t *testing.T) {
-			response := strings.Replace(json,
-				`"mergeStatus": "NotSet"`,
-				fmt.Sprintf(`"mergeStatus": "%s"`, c.state),
-				1,
-			)
+		t.Run(c.testName, func(t *testing.T) {
+			pullRequestResponse := strings.Replace(pullRequestBody, `"mergeStatus": "notSet"`, fmt.Sprintf(`"mergeStatus": "%s"`, c.mergeStatus), 1)
+			policyEvaluationsResponse := strings.Replace(policyEvaluationsBody, `"status": "approved"`, fmt.Sprintf(`"status": "%s"`, c.policyStatus), 1)
 
 			testServer := httptest.NewTLSServer(
 				http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 					switch r.RequestURI {
 					case "/owner/project/_apis/git/repositories/repo/pullrequests/1?api-version=5.1-preview.1&includeWorkItemRefs=true":
-						w.Write([]byte(response)) // nolint: errcheck
+						w.Write([]byte(pullRequestResponse)) // nolint: errcheck
+						return
+					case "/owner/project/_apis/policy/evaluations?api-version=5.1-preview&artifactId=vstfs%3A%2F%2F%2FCodeReview%2FCodeReviewId%2F33333333-3333-3333-333333333333%2F1":
+						w.Write([]byte(policyEvaluationsResponse)) // nolint: errcheck
 						return
 					default:
 						t.Errorf("got unexpected request at %q", r.RequestURI)
 						http.Error(w, "not found", http.StatusNotFound)
 						return
 					}
 				}))
+
 			testServerURL, err := url.Parse(testServer.URL)
 			Ok(t, err)
+
 			client, err := vcs.NewAzureDevopsClient(testServerURL.Host, "token")
 			Ok(t, err)
+
 			defer disableSSLVerification()()
 
 			actMergeable, err := client.PullIsMergeable(models.Repo{
@@ -359,49 +368,57 @@ func TestAzureDevopsClient_PullIsMergeable(t *testing.T) {
 
 func TestAzureDevopsClient_PullIsApproved(t *testing.T) {
 	cases := []struct {
-		testName    string
-		vote        int
-		expApproved bool
+		testName           string
+		reviewerUniqueName string
+		reviewerVote       int
+		expApproved        bool
 	}{
 		{
 			"approved",
+			"[email protected]",
 			azuredevops.VoteApproved,
 			true,
 		},
 		{
 			"approved with suggestions",
+			"[email protected]",
 			azuredevops.VoteApprovedWithSuggestions,
-			false,
+			true,
 		},
 		{
 			"no vote",
+			"[email protected]",
 			azuredevops.VoteNone,
 			false,
 		},
 		{
 			"vote waiting for author",
+			"[email protected]",
 			azuredevops.VoteWaitingForAuthor,
 			false,
 		},
 		{
 			"vote rejected",
+			"[email protected]",
 			azuredevops.VoteRejected,
 			false,
 		},
+		{
+			"approved only by author",
+			"[email protected]",
+			azuredevops.VoteApproved,
+			false,
+		},
 	}
 
-	// Use a real Azure DevOps json response and edit the mergeable_state field.
 	jsBytes, err := ioutil.ReadFile("fixtures/azuredevops-pr.json")
 	Ok(t, err)
-	json := string(jsBytes)
 
+	json := string(jsBytes)
 	for _, c := range cases {
 		t.Run(c.testName, func(t *testing.T) {
-			response := strings.Replace(json,
-				`"vote": 0,`,
-				fmt.Sprintf(`"vote": %d,`, c.vote),
-				1,
-			)
+			response := strings.Replace(json, `"vote": 0,`, fmt.Sprintf(`"vote": %d,`, c.reviewerVote), 1)
+			response = strings.Replace(response, "[email protected]", c.reviewerUniqueName, 1)
 
 			testServer := httptest.NewTLSServer(
 				http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -415,10 +432,13 @@ func TestAzureDevopsClient_PullIsApproved(t *testing.T) {
 						return
 					}
 				}))
+
 			testServerURL, err := url.Parse(testServer.URL)
 			Ok(t, err)
+
 			client, err := vcs.NewAzureDevopsClient(testServerURL.Host, "token")
 			Ok(t, err)
+
 			defer disableSSLVerification()()
 
 			actApproved, err := client.PullIsApproved(models.Repo{

server/events/vcs/fixtures/azuredevops-policyevaluations.json

@@ -0,0 +1,16 @@
+{
+    "value": [
+        {
+            "configuration": {
+                "isDeleted": false,
+                "isEnabled": true,
+                "isBlocking": true,
+                "settings": {
+                    "statusName": "pending"
+                }
+            },
+            "status": "approved"
+        }
+    ],
+    "count": 1
+}