Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add policy checks to Azure DevOps #984

Merged
merged 14 commits into from
Apr 16, 2020
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,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.1
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
Expand All @@ -53,7 +53,7 @@ require (
github.com/urfave/cli v1.20.0
github.com/urfave/negroni v0.2.0
github.com/xanzy/go-gitlab v0.22.2-0.20191127083556-16a492660b8c
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
Expand Down
6 changes: 6 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,10 @@ github.com/mattn/go-isatty v0.0.4/go.mod h1:M+lRXTBqGeGNdLjl/ufCoiOlB5xdOkqRJdNx
github.com/mattn/go-runewidth v0.0.4/go.mod h1:LwmH8dsx7+W8Uxz3IHJYH5QSwggIsqBzpuz5H//U1FU=
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/mcdafydd/go-azuredevops v0.11.1 h1:NO4wlkyFpdxqZZzNzn5m3fJc4box0jnkC8LBhAaPXeA=
github.com/mcdafydd/go-azuredevops v0.11.1/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=
Expand Down Expand Up @@ -225,6 +229,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=
Expand Down
61 changes: 50 additions & 11 deletions server/events/vcs/azuredevops_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,43 +124,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")
}

for _, review := range adPull.Reviewers {
if review == nil {
continue
}
if review.GetVote() == azuredevops.VoteApproved {

if review.IdentityRef.GetUniqueName() == adPull.GetCreatedBy().GetUniqueName() {
jpreese marked this conversation as resolved.
Show resolved Hide resolved
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")
}
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 != azuredevops.PullActive.String() {
return false, nil
}
jpreese marked this conversation as resolved.
Show resolved Hide resolved

projectID := *adPull.Repository.Project.ID
artifactID := g.Client.PolicyEvaluations.GetPullRequestArtifactID(projectID, pull.Num)
policyEvaluations, _, err := g.Client.PolicyEvaluations.List(g.ctx, owner, project, artifactID, &azuredevops.PolicyEvaluationsListOptions{})
if err != nil {
return false, errors.Wrap(err, "getting policy evaluations")
}

for _, policyEvaluation := range policyEvaluations {
if !*policyEvaluation.Configuration.IsEnabled || *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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🍻

settings := (policyEvaluation.Configuration.Settings).(map[string]interface{})
if status, ok := settings["statusName"]; ok && status == "atlantis/apply" {
continue
}

if *policyEvaluation.Configuration.IsBlocking && *policyEvaluation.Status != azuredevops.PolicyEvaluationApproved {
return false, nil
}
}

return true, nil
}

// GetPullRequest returns the pull request.
Expand Down
84 changes: 49 additions & 35 deletions server/events/vcs/azuredevops_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -278,64 +278,67 @@ 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(),
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{
Expand All @@ -359,49 +362,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",
"atlantis.reviewer@example.com",
azuredevops.VoteApproved,
true,
},
{
"approved with suggestions",
"atlantis.reviewer@example.com",
azuredevops.VoteApprovedWithSuggestions,
false,
true,
jpreese marked this conversation as resolved.
Show resolved Hide resolved
},
{
"no vote",
"atlantis.reviewer@example.com",
azuredevops.VoteNone,
false,
},
{
"vote waiting for author",
"atlantis.reviewer@example.com",
azuredevops.VoteWaitingForAuthor,
false,
},
{
"vote rejected",
"atlantis.reviewer@example.com",
azuredevops.VoteRejected,
false,
},
{
"approved only by author",
"atlantis.author@example.com",
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, "atlantis.reviewer@example.com", c.reviewerUniqueName, 1)

testServer := httptest.NewTLSServer(
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
Expand All @@ -415,10 +426,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{
Expand Down
16 changes: 16 additions & 0 deletions server/events/vcs/fixtures/azuredevops-policyevaluations.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
"value": [
{
"configuration": {
"isDeleted": false,
"isEnabled": true,
"isBlocking": true,
"settings": {
"statusName": "pending"
}
},
"status": "approved"
}
],
"count": 1
}
Loading