diff --git a/go.mod b/go.mod index 7a6582158df8..7f094c9e8d3d 100644 --- a/go.mod +++ b/go.mod @@ -42,7 +42,7 @@ require ( github.com/golang-jwt/jwt v3.2.2+incompatible github.com/google/go-cmp v0.6.0 github.com/google/go-containerregistry v0.17.0 - github.com/google/go-github/v42 v42.0.0 + github.com/google/go-github/v57 v57.0.0 github.com/google/uuid v1.4.0 github.com/googleapis/gax-go/v2 v2.12.0 github.com/h2non/filetype v1.1.3 diff --git a/go.sum b/go.sum index 22db1852365d..234405a2d37f 100644 --- a/go.sum +++ b/go.sum @@ -359,10 +359,10 @@ github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/go-containerregistry v0.17.0 h1:5p+zYs/R4VGHkhyvgWurWrpJ2hW4Vv9fQI+GzdcwXLk= github.com/google/go-containerregistry v0.17.0/go.mod h1:u0qB2l7mvtWVR5kNcbFIhFY1hLbf8eeGapA+vbFDCtQ= -github.com/google/go-github/v42 v42.0.0 h1:YNT0FwjPrEysRkLIiKuEfSvBPCGKphW5aS5PxwaoLec= -github.com/google/go-github/v42 v42.0.0/go.mod h1:jgg/jvyI0YlDOM1/ps6XYh04HNQ3vKf0CVko62/EhRg= github.com/google/go-github/v56 v56.0.0 h1:TysL7dMa/r7wsQi44BjqlwaHvwlFlqkK8CtBWCX3gb4= github.com/google/go-github/v56 v56.0.0/go.mod h1:D8cdcX98YWJvi7TLo7zM4/h8ZTx6u6fwGEkCdisopo0= +github.com/google/go-github/v57 v57.0.0 h1:L+Y3UPTY8ALM8x+TV0lg+IEBI+upibemtBD8Q9u7zHs= +github.com/google/go-github/v57 v57.0.0/go.mod h1:s0omdnye0hvK/ecLvpsGfJMiRt85PimQh4oygmLIxHw= github.com/google/go-querystring v0.0.0-20170111101155-53e6ce116135/go.mod h1:odCYkC5MyYFN7vkCjXpyrEuKhc/BUO6wN/zVPAxq5ck= github.com/google/go-querystring v1.1.0 h1:AnCroh3fv4ZBgVIf1Iwtovgjaw/GiKJo8M8yD/fhyJ8= github.com/google/go-querystring v1.1.0/go.mod h1:Kcdr2DB4koayq7X8pmAG4sNG59So17icRSOU623lUBU= diff --git a/pkg/sources/git/git.go b/pkg/sources/git/git.go index f70c7393f9bd..ca4560b580fa 100644 --- a/pkg/sources/git/git.go +++ b/pkg/sources/git/git.go @@ -18,7 +18,7 @@ import ( "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/plumbing" "github.com/go-git/go-git/v5/plumbing/object" - "github.com/google/go-github/v42/github" + "github.com/google/go-github/v57/github" diskbufferreader "github.com/trufflesecurity/disk-buffer-reader" "golang.org/x/oauth2" "golang.org/x/sync/semaphore" diff --git a/pkg/sources/github/github.go b/pkg/sources/github/github.go index 5a52ef0a838d..ceae7f8aba41 100644 --- a/pkg/sources/github/github.go +++ b/pkg/sources/github/github.go @@ -2,6 +2,7 @@ package github import ( "fmt" + "golang.org/x/exp/rand" "net/http" "net/url" "os" @@ -19,7 +20,7 @@ import ( gogit "github.com/go-git/go-git/v5" "github.com/go-logr/logr" "github.com/gobwas/glob" - "github.com/google/go-github/v42/github" + "github.com/google/go-github/v57/github" "golang.org/x/oauth2" "golang.org/x/sync/errgroup" "google.golang.org/protobuf/proto" @@ -824,53 +825,70 @@ func (s *Source) scan(ctx context.Context, installationClient *github.Client, ch return nil } +var ( + rateLimitMu sync.RWMutex + rateLimitResumeTime time.Time +) + // handleRateLimit returns true if a rate limit was handled -// Unauthenticated access to most github endpoints has a rate limit of 60 requests per hour. -// This will likely only be exhausted if many users/orgs are scanned without auth +// +// Unauthenticated users have a rate limit of 60 requests per hour. +// Authenticated users have a rate limit of 5,000 requests per hour, +// however, certain actions are subject to a stricter "secondary" limit. +// https://docs.github.com/en/rest/overview/rate-limits-for-the-rest-api func (s *Source) handleRateLimit(errIn error, res *github.Response) bool { - var ( - knownWait = true - remaining = 0 - retryAfter time.Duration - ) - - // GitHub has both primary (RateLimit) and secondary (AbuseRateLimit) errors. - var rateLimit *github.RateLimitError - var abuseLimit *github.AbuseRateLimitError - if errors.As(errIn, &rateLimit) { - // Do nothing - } else if errors.As(errIn, &abuseLimit) { - retryAfter = abuseLimit.GetRetryAfter() - } else { + if errIn == nil { return false } - githubNumRateLimitEncountered.WithLabelValues(s.name).Inc() - // Parse retry information from response headers, unless a Retry-After value was already provided. - // https://docs.github.com/en/rest/overview/resources-in-the-rest-api#exceeding-the-rate-limit - if retryAfter <= 0 && res != nil { - var err error - remaining, err = strconv.Atoi(res.Header.Get("x-ratelimit-remaining")) - if err != nil { - knownWait = false + rateLimitMu.RLock() + resumeTime := rateLimitResumeTime + rateLimitMu.RUnlock() + + var retryAfter time.Duration + if resumeTime.IsZero() || time.Now().After(resumeTime) { + rateLimitMu.Lock() + + var ( + now = time.Now() + + // GitHub has both primary (RateLimit) and secondary (AbuseRateLimit) errors. + limitType string + rateLimit *github.RateLimitError + abuseLimit *github.AbuseRateLimitError + ) + if errors.As(errIn, &rateLimit) { + limitType = "primary" + rate := rateLimit.Rate + if rate.Remaining == 0 { // TODO: Will we ever receive a |RateLimitError| when remaining > 0? + retryAfter = rate.Reset.Sub(now) + } + } else if errors.As(errIn, &abuseLimit) { + limitType = "secondary" + retryAfter = abuseLimit.GetRetryAfter() + } else { + rateLimitMu.Unlock() + return false } - resetTime, err := strconv.Atoi(res.Header.Get("x-ratelimit-reset")) - if err != nil || resetTime == 0 { - knownWait = false - } else if resetTime > 0 { - retryAfter = time.Duration(int64(resetTime)-time.Now().Unix()) * time.Second + jitter := time.Duration(rand.Intn(10)+1) * time.Second + if retryAfter > 0 { + retryAfter = retryAfter + jitter + rateLimitResumeTime = now.Add(retryAfter) + s.log.V(0).Info(fmt.Sprintf("exceeded %s rate limit", limitType), "resume_time", rateLimitResumeTime.Format(time.RFC3339)) + } else { + retryAfter = (5 * time.Minute) + jitter + rateLimitResumeTime = now.Add(retryAfter) + // TODO: Use exponential backoff instead of static retry time. + s.log.V(0).Error(errIn, "unexpected rate limit error", "resume_time", rateLimitResumeTime.Format(time.RFC3339)) } - } - resumeTime := time.Now().Add(retryAfter).String() - if knownWait && remaining == 0 && retryAfter > 0 { - s.log.V(2).Info("rate limited", "retry_after", retryAfter.String(), "resume_time", resumeTime) + rateLimitMu.Unlock() } else { - // TODO: Use exponential backoff instead of static retry time. - retryAfter = time.Minute * 5 - s.log.V(2).Error(errIn, "unexpected rate limit error", "retry_after", retryAfter.String(), "resume_time", resumeTime) + retryAfter = resumeTime.Sub(time.Now()) } + + githubNumRateLimitEncountered.WithLabelValues(s.name).Inc() time.Sleep(retryAfter) githubSecondsSpentRateLimited.WithLabelValues(s.name).Add(retryAfter.Seconds()) return true diff --git a/pkg/sources/github/github_test.go b/pkg/sources/github/github_test.go index 8b5fd27e1909..fba7ccd5494c 100644 --- a/pkg/sources/github/github_test.go +++ b/pkg/sources/github/github_test.go @@ -17,7 +17,7 @@ import ( "github.com/go-logr/logr" "github.com/google/go-cmp/cmp" - "github.com/google/go-github/v42/github" + "github.com/google/go-github/v57/github" "github.com/stretchr/testify/assert" "golang.org/x/sync/errgroup" "google.golang.org/protobuf/types/known/anypb" diff --git a/pkg/sources/github/repo.go b/pkg/sources/github/repo.go index 836cb1a238ce..b32b56032137 100644 --- a/pkg/sources/github/repo.go +++ b/pkg/sources/github/repo.go @@ -6,7 +6,7 @@ import ( "strings" gogit "github.com/go-git/go-git/v5" - "github.com/google/go-github/v42/github" + "github.com/google/go-github/v57/github" "github.com/trufflesecurity/trufflehog/v3/pkg/context" "github.com/trufflesecurity/trufflehog/v3/pkg/giturl"