Added config and roundtripper to allow us to handle Azure Devops Retr…

…y-After header. Also added flag to increase timeouts for azdo client. Need both to deal with Azdo throttling

server/events/vcs/azuredevops_client.go

@@ -6,6 +6,7 @@ import (
 	"net/http"
 	"net/url"
 	"path/filepath"
+	"strconv"
 	"strings"
 	"time"
 
@@ -22,14 +23,53 @@ type AzureDevopsClient struct {
 	UserName string
 }
 
+type ThrottleRoundtripper struct {
+	// If azure devops is throttling, this is how long to wait before the next request
+	RetryAfter         time.Duration
+	ParentRoundtripper http.RoundTripper
+}
+
+func NewThrottleRoundtripper(rt http.RoundTripper) http.RoundTripper {
+	return &ThrottleRoundtripper{
+		ParentRoundtripper: rt,
+	}
+}
+
+// RoundTrip follows http.Transport interface and will automatically sleep for retryAfter seconds
+// This allows us to honor the Retry-After header from Azdo
+func (rt *ThrottleRoundtripper) RoundTrip(req *http.Request) (resp *http.Response, err error) {
+	time.Sleep(rt.RetryAfter)
+	rt.RetryAfter = 0
+
+	resp, err = rt.ParentRoundtripper.RoundTrip(req)
+	if err != nil {
+		return
+	}
+
+	retryAfterVal := resp.Header.Get("retry-after")
+	if retryAfterVal != "" {
+		retryAfterSec, err := strconv.Atoi(retryAfterVal)
+		if err == nil {
+			rt.RetryAfter = time.Duration(retryAfterSec) * time.Second
+		}
+	}
+
+	return
+}
+
 // NewAzureDevopsClient returns a valid Azure DevOps client.
-func NewAzureDevopsClient(hostname string, userName string, token string) (*AzureDevopsClient, error) {
+func NewAzureDevopsClient(hostname string, userName string, token string, timeout time.Duration, honorRetryAfterHeader bool) (*AzureDevopsClient, error) {
 	tp := azuredevops.BasicAuthTransport{
 		Username: "",
 		Password: strings.TrimSpace(token),
 	}
 	httpClient := tp.Client()
-	httpClient.Timeout = time.Second * 10
+	httpClient.Timeout = timeout
+
+	if honorRetryAfterHeader {
+		httpClient.Transport = NewThrottleRoundtripper(httpClient.Transport)
+	}
+
 	var adClient, err = azuredevops.NewClient(httpClient)
 	if err != nil {
 		return nil, err

server/events/vcs/azuredevops_client_test.go

@@ -9,6 +9,7 @@ import (
 	"net/url"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/mcdafydd/go-azuredevops/azuredevops"
 	"github.com/runatlantis/atlantis/server/events/models"
@@ -19,28 +20,39 @@ import (
 
 func TestAzureDevopsClient_MergePull(t *testing.T) {
 	cases := []struct {
-		description string
-		response    string
-		code        int
-		expErr      string
+		description  string
+		response     string
+		code         int
+		expErr       string
+		throttleTime time.Duration
 	}{
 		{
 			"success",
 			adMergeSuccess,
 			200,
 			"",
+			0 * time.Second,
 		},
 		{
 			"405",
 			`{"message":"405 Method Not Allowed"}`,
 			405,
 			"405 {message: 405 Method Not Allowed}",
+			0 * time.Second,
 		},
 		{
 			"406",
 			`{"message":"406 Branch cannot be merged"}`,
 			406,
 			"406 {message: 406 Branch cannot be merged}",
+			0 * time.Second,
+		},
+		{
+			"success",
+			adMergeSuccess,
+			200,
+			"",
+			1 * time.Second,
 		},
 	}
 
@@ -81,8 +93,21 @@ func TestAzureDevopsClient_MergePull(t *testing.T) {
 
 	for _, c := range cases {
 		t.Run(c.description, func(t *testing.T) {
+			// Use lastquery to keep track of the last request that hit our server
+			var lastQuery time.Time = time.Time{}
+
 			testServer := httptest.NewTLSServer(
 				http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+					currentTime := time.Now()
+					if !currentTime.After(lastQuery.Add(c.throttleTime)) {
+						t.Errorf("Did not respect retry-after header from azdo")
+						http.Error(w, "throttle not honored", http.StatusBadRequest)
+						return
+					}
+					lastQuery = currentTime
+					// We always set the retry-after
+					w.Header().Set("retry-after", fmt.Sprint(int64(c.throttleTime/time.Second)))
+
 					switch r.RequestURI {
 					// The first request should hit this URL.
 					case "/owner/project/_apis/git/repositories/repo/pullrequests/22?api-version=5.1-preview.1":
@@ -99,7 +124,7 @@ func TestAzureDevopsClient_MergePull(t *testing.T) {
 
 			testServerURL, err := url.Parse(testServer.URL)
 			Ok(t, err)
-			client, err := vcs.NewAzureDevopsClient(testServerURL.Host, "user", "token")
+			client, err := vcs.NewAzureDevopsClient(testServerURL.Host, "user", "token", 5*time.Second, true)
 			client.Client.VsaexBaseURL = *testServerURL
 			Ok(t, err)
 			defer disableSSLVerification()()
@@ -213,7 +238,7 @@ func TestAzureDevopsClient_UpdateStatus(t *testing.T) {
 
 			testServerURL, err := url.Parse(testServer.URL)
 			Ok(t, err)
-			client, err := vcs.NewAzureDevopsClient(testServerURL.Host, "user", "token")
+			client, err := vcs.NewAzureDevopsClient(testServerURL.Host, "user", "token", 1*time.Second, true)
 			Ok(t, err)
 			defer disableSSLVerification()()
 
@@ -276,7 +301,7 @@ func TestAzureDevopsClient_GetModifiedFiles(t *testing.T) {
 
 	testServerURL, err := url.Parse(testServer.URL)
 	Ok(t, err)
-	client, err := vcs.NewAzureDevopsClient(testServerURL.Host, "user", "token")
+	client, err := vcs.NewAzureDevopsClient(testServerURL.Host, "user", "token", 1*time.Second, true)
 	Ok(t, err)
 	defer disableSSLVerification()()
 
@@ -396,7 +421,7 @@ func TestAzureDevopsClient_PullIsMergeable(t *testing.T) {
 			testServerURL, err := url.Parse(testServer.URL)
 			Ok(t, err)
 
-			client, err := vcs.NewAzureDevopsClient(testServerURL.Host, "user", "token")
+			client, err := vcs.NewAzureDevopsClient(testServerURL.Host, "user", "token", 1*time.Second, true)
 			Ok(t, err)
 
 			defer disableSSLVerification()()
@@ -490,7 +515,7 @@ func TestAzureDevopsClient_PullIsApproved(t *testing.T) {
 			testServerURL, err := url.Parse(testServer.URL)
 			Ok(t, err)
 
-			client, err := vcs.NewAzureDevopsClient(testServerURL.Host, "user", "token")
+			client, err := vcs.NewAzureDevopsClient(testServerURL.Host, "user", "token", 1*time.Second, true)
 			Ok(t, err)
 
 			defer disableSSLVerification()()
@@ -535,7 +560,7 @@ func TestAzureDevopsClient_GetPullRequest(t *testing.T) {
 			}))
 		testServerURL, err := url.Parse(testServer.URL)
 		Ok(t, err)
-		client, err := vcs.NewAzureDevopsClient(testServerURL.Host, "user", "token")
+		client, err := vcs.NewAzureDevopsClient(testServerURL.Host, "user", "token", 1*time.Second, true)
 		Ok(t, err)
 		defer disableSSLVerification()()
 
@@ -555,7 +580,7 @@ func TestAzureDevopsClient_GetPullRequest(t *testing.T) {
 }
 
 func TestAzureDevopsClient_MarkdownPullLink(t *testing.T) {
-	client, err := vcs.NewAzureDevopsClient("hostname", "user", "token")
+	client, err := vcs.NewAzureDevopsClient("hostname", "user", "token", 1*time.Second, true)
 	Ok(t, err)
 	pull := models.PullRequest{Num: 1}
 	s, _ := client.MarkdownPullLink(pull)

server/server.go

@@ -200,9 +200,13 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) {
 		}
 	}
 	if userConfig.AzureDevopsUser != "" {
+		azureDevopsTimeout := 10 * time.Second
+		if userConfig.AzureDevopsTimeoutSecondss != 0 {
+			azureDevopsTimeout = time.Duration(userConfig.AzureDevopsTimeoutSecondss) * time.Second
+		}
 		supportedVCSHosts = append(supportedVCSHosts, models.AzureDevops)
 		var err error
-		azuredevopsClient, err = vcs.NewAzureDevopsClient("dev.azure.com", userConfig.AzureDevopsUser, userConfig.AzureDevopsToken)
+		azuredevopsClient, err = vcs.NewAzureDevopsClient("dev.azure.com", userConfig.AzureDevopsUser, userConfig.AzureDevopsToken, azureDevopsTimeout, userConfig.AzureDevopsHonorRetryAfter)
 		if err != nil {
 			return nil, err
 		}

server/user_config.go

@@ -13,6 +13,8 @@ type UserConfig struct {
 	AtlantisURL                string `mapstructure:"atlantis-url"`
 	Automerge                  bool   `mapstructure:"automerge"`
 	AutoplanFileList           string `mapstructure:"autoplan-file-list"`
+	AzureDevopsHonorRetryAfter bool   `mapstructure:"azuredevops-honor-retry-after"`
+	AzureDevopsTimeoutSecondss int    `mapstructure:"azuredevops-timeout-seconds"`
 	AzureDevopsToken           string `mapstructure:"azuredevops-token"`
 	AzureDevopsUser            string `mapstructure:"azuredevops-user"`
 	AzureDevopsWebhookPassword string `mapstructure:"azuredevops-webhook-password"`