Merge #12307

12307: Retry 403 rate limit errors from github r=Frassle a=Frassle

<!--- 
Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation.
-->

# Description

<!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. -->

Adding a small retry loop to the github downloader to retry 403 errors _iff_ they're due to rate limits. We can't just retry all 403 errors because that code could be returned for other errors as well.

Fixes #11013

## Checklist

<!--- Please provide details if the checkbox below is to be left unchecked. -->
- [x] I have added tests that prove my fix is effective or that my feature works
<!--- 
User-facing changes require a CHANGELOG entry.
-->
- [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change
<!--
If the change(s) in this PR is a modification of an existing call to the Pulumi Service,
then the service should honor older versions of the CLI where this change would not exist.
You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add
it to the service.
-->
- [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Service API version
  <!-- `@Pulumi` employees: If yes, you must submit corresponding changes in the service repo. -->


Co-authored-by: Fraser Waters <fraser@pulumi.com>

changelog/pending/20230228--cli-plugin--retry-403-rate-limit-errors-from-github-when-downloading-plugins.yaml

@@ -0,0 +1,4 @@
+changes:
+- type: fix
+  scope: cli/plugin
+  description: Retry 403 rate limit errors from GitHub when downloading plugins.

sdk/go/common/workspace/plugins.go

@@ -33,6 +33,7 @@ import (
 	"regexp"
 	"runtime"
 	"sort"
+	"strconv"
 	"strings"
 	"time"
 
@@ -237,7 +238,11 @@ func newGitlabSource(url *url.URL, name string, kind PluginKind) (*gitlabSource,
 }
 
 func (source *gitlabSource) newHTTPRequest(url, accept string) (*http.Request, error) {
-	req, err := buildHTTPRequest(url, fmt.Sprintf("Bearer %s", source.token))
+	var token string
+	if source.token != "" {
+		token = fmt.Sprintf("Bearer %s", source.token)
+	}
+	req, err := buildHTTPRequest(url, token)
 	if err != nil {
 		return nil, err
 	}
@@ -353,14 +358,54 @@ func newGithubSource(url *url.URL, name string, kind PluginKind) (*githubSource,
 }
 
 func (source *githubSource) newHTTPRequest(url, accept string) (*http.Request, error) {
-	req, err := buildHTTPRequest(url, fmt.Sprintf("token %s", source.token))
+	var token string
+	if source.token != "" {
+		token = fmt.Sprintf("token %s", source.token)
+	}
+	req, err := buildHTTPRequest(url, token)
 	if err != nil {
 		return nil, err
 	}
 	req.Header.Set("Accept", accept)
 	return req, nil
 }
 
+func (source *githubSource) retryHTTP(
+	getHTTPResponse func(*http.Request) (io.ReadCloser, int64, error),
+	req *http.Request) (io.ReadCloser, int64, error) {
+
+	for attempt := 0; ; attempt++ {
+		resp, length, err := getHTTPResponse(req)
+		if err == nil {
+			return resp, length, nil
+		}
+
+		// Only retry a few times.
+		const maxAttempts = 5
+		if attempt >= maxAttempts {
+			return nil, -1, err
+		}
+
+		// We'll retry only 403 rate limit errors.
+		var downErr *downloadError
+		if !errors.As(err, &downErr) || downErr.code != 403 {
+			return nil, -1, err
+		}
+
+		// This is a rate limiting error only if x-ratelimit-remaining is 0.
+		// https://docs.github.com/en/rest/overview/resources-in-the-rest-api?apiVersion=2022-11-28#exceeding-the-rate-limit
+		if downErr.header.Get("x-ratelimit-remaining") != "0" {
+			return nil, -1, err
+		}
+
+		delay := 80 * time.Millisecond // sensible default if we can't parse the header
+		if reset, err := strconv.ParseInt(downErr.header.Get("x-ratelimit-reset"), 10, 64); err == nil {
+			delay = time.Until(time.Unix(reset, 0).UTC())
+		}
+		time.Sleep(delay)
+	}
+}
+
 func (source *githubSource) GetLatestVersion(
 	getHTTPResponse func(*http.Request) (io.ReadCloser, int64, error)) (*semver.Version, error) {
 	releaseURL := fmt.Sprintf(
@@ -371,7 +416,7 @@ func (source *githubSource) GetLatestVersion(
 	if err != nil {
 		return nil, err
 	}
-	resp, length, err := getHTTPResponse(req)
+	resp, length, err := source.retryHTTP(getHTTPResponse, req)
 	if err != nil {
 		return nil, err
 	}
@@ -406,7 +451,7 @@ func (source *githubSource) Download(
 	if err != nil {
 		return nil, -1, err
 	}
-	resp, length, err := getHTTPResponse(req)
+	resp, length, err := source.retryHTTP(getHTTPResponse, req)
 	if err != nil {
 		return nil, -1, err
 	}
@@ -440,7 +485,7 @@ func (source *githubSource) Download(
 	if err != nil {
 		return nil, -1, err
 	}
-	return getHTTPResponse(req)
+	return source.retryHTTP(getHTTPResponse, req)
 }
 
 // httpSource can download a plugin from a given http url, it doesn't support GetLatestVersion
@@ -901,26 +946,24 @@ func getHTTPResponse(req *http.Request) (io.ReadCloser, int64, error) {
 	logging.V(11).Infof("plugin install response headers: %v", resp.Header)
 
 	if resp.StatusCode < 200 || resp.StatusCode > 299 {
-		return nil, -1, newDownloadError(resp.StatusCode, req.URL)
+		contract.IgnoreClose(resp.Body)
+		return nil, -1, newDownloadError(resp.StatusCode, req.URL, resp.Header)
 	}
 
 	return resp.Body, resp.ContentLength, nil
 }
 
 // downloadError is an error that happened during the HTTP download of a plugin.
 type downloadError struct {
-	msg  string
-	code int
+	msg    string
+	code   int
+	header http.Header
 }
 
 func (e *downloadError) Error() string {
 	return e.msg
 }
 
-func (e *downloadError) Code() int {
-	return e.code
-}
-
 // Create a new downloadError with a message that indicates GITHUB_TOKEN should be set.
 func newGithubPrivateRepoError(statusCode int, url *url.URL) error {
 	return &downloadError{
@@ -934,13 +977,14 @@ func newGithubPrivateRepoError(statusCode int, url *url.URL) error {
 }
 
 // Create a new downloadError.
-func newDownloadError(statusCode int, url *url.URL) error {
+func newDownloadError(statusCode int, url *url.URL, header http.Header) error {
 	if url.Host == "api.github.com" && statusCode == 404 {
 		return newGithubPrivateRepoError(statusCode, url)
 	}
 	return &downloadError{
-		code: statusCode,
-		msg:  fmt.Sprintf("%d HTTP error fetching plugin from %s", statusCode, url),
+		code:   statusCode,
+		msg:    fmt.Sprintf("%d HTTP error fetching plugin from %s", statusCode, url),
+		header: header,
 	}
 }
 
@@ -1078,7 +1122,7 @@ func DownloadToFile(
 			}
 
 			// Don't retry, since the request was processed and rejected.
-			if err, ok := readErr.(*downloadError); ok && (err.Code() == 404 || err.Code() == 403) {
+			if err, ok := readErr.(*downloadError); ok && (err.code == 404 || err.code == 403) {
 				return "", readErr
 			}
 

sdk/go/common/workspace/plugins_test.go

@@ -573,6 +573,62 @@ func TestPluginDownload(t *testing.T) {
 		assert.Equal(t, int(l), len(readBytes))
 		assert.Equal(t, expectedBytes, readBytes)
 	})
+	t.Run("GitHub Releases with 403 retry", func(t *testing.T) {
+		t.Setenv("GITHUB_TOKEN", "")
+		version := semver.MustParse("1.23.0")
+		spec := PluginSpec{
+			PluginDownloadURL: "github://api.github.org/someorg",
+			Name:              "someprovider",
+			Version:           &version,
+			Kind:              PluginKind("resource"),
+		}
+		source, err := spec.GetSource()
+		require.NoError(t, err)
+		// Fail the first request, succeed the second
+		failCounter := 0
+		getHTTPResponse := func(req *http.Request) (io.ReadCloser, int64, error) {
+			if req.URL.String() == "https://api.github.org/repos/someorg/pulumi-someprovider/releases/tags/v1.23.0" {
+				if failCounter == 0 {
+					failCounter++
+					return nil, 0, newDownloadError(403, req.URL, http.Header{"X-Ratelimit-Remaining": []string{"0"}})
+				}
+				failCounter = 0
+				assert.Equal(t, "", req.Header.Get("Authorization"))
+				assert.Equal(t, "application/json", req.Header.Get("Accept"))
+				// Minimal JSON from the releases API to get the test to pass
+				return newMockReadCloserString(`{
+					"assets": [
+					  {
+						"url": "https://api.github.com/repos/someorg/pulumi-someprovider/releases/assets/654321",
+						"name": "pulumi-someprovider_1.23.0_checksums.txt"
+					  },
+					  {
+						"url": "https://api.github.com/repos/someorg/pulumi-someprovider/releases/assets/123456",
+						"name": "pulumi-resource-someprovider-v1.23.0-windows-amd64.tar.gz"
+					  }
+					]
+				  }
+				`)
+			}
+
+			if failCounter == 0 {
+				failCounter++
+				return nil, 0, newDownloadError(403, req.URL, http.Header{"X-Ratelimit-Remaining": []string{"0"}})
+			}
+			failCounter = 0
+
+			assert.Equal(t, "https://api.github.com/repos/someorg/pulumi-someprovider/releases/assets/123456", req.URL.String())
+			assert.Equal(t, "", req.Header.Get("Authorization"))
+			assert.Equal(t, "application/octet-stream", req.Header.Get("Accept"))
+			return newMockReadCloser(expectedBytes)
+		}
+		r, l, err := source.Download(*spec.Version, "windows", "amd64", getHTTPResponse)
+		require.NoError(t, err)
+		readBytes, err := io.ReadAll(r)
+		require.NoError(t, err)
+		assert.Equal(t, int(l), len(readBytes))
+		assert.Equal(t, expectedBytes, readBytes)
+	})
 }
 
 //nolint:paralleltest // mutates environment variables