Skip to content

Commit

Permalink
Return error when proxy cache get too many request error(429)
Browse files Browse the repository at this point in the history
  Add 429 too many request error in http error
  Fixes #18707

Signed-off-by: stonezdj <stonezdj@gmail.com>
  • Loading branch information
stonezdj committed May 29, 2023
1 parent 4f3393e commit 8224aaf
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 1 deletion.
3 changes: 3 additions & 0 deletions src/controller/proxy/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,9 @@ func (c *controller) UseLocalManifest(ctx context.Context, art lib.ArtifactInfo,
remoteRepo := getRemoteRepo(art)
exist, desc, err := remote.ManifestExist(remoteRepo, getReference(art)) // HEAD
if err != nil {
if errors.IsRateLimitError(err) && a != nil { // if rate limit, use local if it exists, otherwise return error
return true, nil, nil
}
return false, nil, err
}
if !exist || desc == nil {
Expand Down
24 changes: 24 additions & 0 deletions src/controller/proxy/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,30 @@ func (p *proxyControllerTestSuite) TestUseLocalManifest_False() {
p.Assert().False(result)
}

func (p *proxyControllerTestSuite) TestUseLocalManifest_429() {
ctx := context.Background()
dig := "sha256:1a9ec845ee94c202b2d5da74a24f0ed2058318bfa9879fa541efaecba272e86b"
desc := &distribution.Descriptor{Digest: digest.Digest(dig)}
art := lib.ArtifactInfo{Repository: "library/hello-world", Digest: dig}
p.remote.On("ManifestExist", mock.Anything, mock.Anything).Return(false, desc, errors.New("too many requests").WithCode(errors.RateLimitCode))
p.local.On("GetManifest", mock.Anything, mock.Anything).Return(nil, nil)
_, _, err := p.ctr.UseLocalManifest(ctx, art, p.remote)
p.Assert().NotNil(err)
errors.IsRateLimitError(err)
}

func (p *proxyControllerTestSuite) TestUseLocalManifest_429ToLocal() {
ctx := context.Background()
dig := "sha256:1a9ec845ee94c202b2d5da74a24f0ed2058318bfa9879fa541efaecba272e86b"
desc := &distribution.Descriptor{Digest: digest.Digest(dig)}
art := lib.ArtifactInfo{Repository: "library/hello-world", Digest: dig}
p.remote.On("ManifestExist", mock.Anything, mock.Anything).Return(false, desc, errors.New("too many requests").WithCode(errors.RateLimitCode))
p.local.On("GetManifest", mock.Anything, mock.Anything).Return(&artifact.Artifact{}, nil)
result, _, err := p.ctr.UseLocalManifest(ctx, art, p.remote)
p.Assert().Nil(err)
p.Assert().True(result)
}

func (p *proxyControllerTestSuite) TestUseLocalManifestWithTag_False() {
ctx := context.Background()
art := lib.ArtifactInfo{Repository: "library/hello-world", Tag: "latest"}
Expand Down
7 changes: 7 additions & 0 deletions src/lib/errors/const.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ const (
ForbiddenCode = "FORBIDDEN"
// MethodNotAllowedCode ...
MethodNotAllowedCode = "METHOD_NOT_ALLOWED"
// RateLimitCode
RateLimitCode = "TOO_MANY_REQUEST"
// PreconditionCode ...
PreconditionCode = "PRECONDITION"
// GeneralCode ...
Expand Down Expand Up @@ -105,3 +107,8 @@ func IsConflictErr(err error) bool {
func IsChallengesUnsupportedErr(err error) bool {
return IsErr(err, ChallengesUnsupportedCode)
}

// IsRateLimitError checks whether the err chains contains rate limit error
func IsRateLimitError(err error) bool {
return IsErr(err, RateLimitCode)
}
1 change: 1 addition & 0 deletions src/lib/http/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ var (
errors.MethodNotAllowedCode: http.StatusMethodNotAllowed,
errors.DENIED: http.StatusForbidden,
errors.NotFoundCode: http.StatusNotFound,
errors.RateLimitCode: http.StatusTooManyRequests,
errors.ConflictCode: http.StatusConflict,
errors.PreconditionCode: http.StatusPreconditionFailed,
errors.ViolateForeignKeyConstraintCode: http.StatusPreconditionFailed,
Expand Down
2 changes: 2 additions & 0 deletions src/pkg/registry/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -678,6 +678,8 @@ func (c *client) do(req *http.Request) (*http.Response, error) {
code = errors.ForbiddenCode
case http.StatusNotFound:
code = errors.NotFoundCode
case http.StatusTooManyRequests:
code = errors.RateLimitCode
}
return nil, errors.New(nil).WithCode(code).
WithMessage(message)
Expand Down
8 changes: 7 additions & 1 deletion src/server/middleware/repoproxy/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ const (
ensureTagMaxRetry = 60
)

var tooManyRequestsError = errors.New("too many requests to upstream registry").WithCode(errors.RateLimitCode)

// BlobGetMiddleware handle get blob request
func BlobGetMiddleware() func(http.Handler) http.Handler {
return middleware.New(func(w http.ResponseWriter, r *http.Request, next http.Handler) {
Expand Down Expand Up @@ -112,6 +114,10 @@ func ManifestMiddleware() func(http.Handler) http.Handler {
httpLib.SendError(w, err)
return
}
if errors.IsRateLimitError(err) {
httpLib.SendError(w, tooManyRequestsError)
return
}
log.Errorf("failed to proxy manifest, fallback to local, request uri: %v, error: %v", r.RequestURI, err)
next.ServeHTTP(w, r)
}
Expand Down Expand Up @@ -202,7 +208,7 @@ func handleManifest(w http.ResponseWriter, r *http.Request, next http.Handler) e
err = proxyManifestGet(ctx, w, proxyCtl, p, art, remote)
}
if err != nil {
if errors.IsNotFoundErr(err) {
if errors.IsNotFoundErr(err) || errors.IsRateLimitError(err) {
return err
}
log.Warningf("Proxy to remote failed, fallback to local repo, error: %v", err)
Expand Down

0 comments on commit 8224aaf

Please sign in to comment.