pkg/{minio,server/middleware}: use project for concurrent req limiter

this changes the concurrent requests limiter to use public project
ID if the authservice record contains it.

Updates #525

Change-Id: I67ae2ad52f74db0a1446cc3dd3f7f81024c1a114

pkg/minio/api-router.go

@@ -22,8 +22,8 @@ func RegisterAPIRouter(router *mux.Router, layer *gw.MultiTenancyLayer, domainNa
 		CacheAPI:  func() cmd.CacheObjectLayer { return nil },
 	}, corsAllowedOrigins}
 
-	// limit the conccurrency of uploads and downloads per macaroon head
-	limit := middleware.NewMacaroonLimiter(concurrentAllowed,
+	// limit the conccurrency of uploads and downloads
+	limit := middleware.NewConcurrentRequestsLimiter(concurrentAllowed,
 		func(w http.ResponseWriter, r *http.Request) {
 			err := cmd.APIError{
 				Code:           "SlowDown",                 // necessary to return a RetryAfter header

pkg/server/middleware/limiter.go

@@ -10,23 +10,30 @@ import (
 	"storj.io/common/grant"
 )
 
-// NewMacaroonLimiter constructs a Limiter that limits based on macaroon credentials.
+// NewConcurrentRequestsLimiter constructs a Limiter that limits using a key from credentials.
 // It relies on the AccessKey middleware being run to append credentials to the request context.
-func NewMacaroonLimiter(allowed uint, limitFunc func(w http.ResponseWriter, r *http.Request)) *Limiter {
-	return NewLimiter(allowed, getRequestMacaroonHead, limitFunc)
+func NewConcurrentRequestsLimiter(allowed uint, limitFunc func(w http.ResponseWriter, r *http.Request)) *Limiter {
+	return NewLimiter(allowed, getLimitKey, limitFunc)
 }
 
-// getRequestMacaroonHead gets the macaroon head corresponding to the current request.
-// Macaroon head is the best available criteria for associating a request to a user.
-func getRequestMacaroonHead(r *http.Request) (ip string, err error) {
+// getLimitKey retrieves a key used to identify the user for limiting requests.
+func getLimitKey(r *http.Request) (identifier string, err error) {
 	credentials := GetAccess(r.Context())
 	if credentials == nil || credentials.AccessGrant == "" {
 		return "", ParseV4CredentialError.New("missing access grant")
 	}
+	if credentials.PublicProjectID != "" {
+		return credentials.PublicProjectID, nil
+	}
+
+	// fallback to macaroon head if the credentials don't contain public project ID. This is likely because the
+	// authservice record is old and hasn't been backfilled with the ID yet, or the ID couldn't be resolved due
+	// to a connectivity issue between authservice and the satellite.
 	access, err := grant.ParseAccess(credentials.AccessGrant)
 	if err != nil {
 		return "", err
 	}
+
 	return string(access.APIKey.Head()), err
 }
 
@@ -40,7 +47,7 @@ type Limiter struct {
 	m      sync.RWMutex
 }
 
-// NewLimiter constructs a concurrency Limiter.  Error and Limit functions are user defined
+// NewLimiter constructs a concurrency Limiter. Error and Limit functions are user defined
 // in part because referencing the "minio" package here would cause an import loop.
 func NewLimiter(allowed uint, keyFunc func(*http.Request) (string, error), limitFunc func(w http.ResponseWriter, r *http.Request)) *Limiter {
 	return &Limiter{

pkg/server/middleware/limiter_test.go

@@ -17,16 +17,17 @@ import (
 	"storj.io/common/grant"
 	"storj.io/common/macaroon"
 	"storj.io/common/testcontext"
+	"storj.io/common/testrand"
 	"storj.io/edge/pkg/authclient"
 )
 
 const maxConncurrent = 3
 
-func TestNewMacaroonLimiter(t *testing.T) {
+func TestNewConcurrentRequestsLimiter(t *testing.T) {
 	var next = make(chan struct{})
-	var done = make(chan struct{}, maxConncurrent*2*3)
+	var done = make(chan struct{}, maxConncurrent*2*4)
 	var allTests = make(chan struct{})
-	rateLimiter := NewMacaroonLimiter(maxConncurrent,
+	rateLimiter := NewConcurrentRequestsLimiter(maxConncurrent,
 		func(w http.ResponseWriter, r *http.Request) {
 			next <- struct{}{} // create in-order results
 			http.Error(w, "", http.StatusTooManyRequests)
@@ -45,27 +46,31 @@ func TestNewMacaroonLimiter(t *testing.T) {
 	}))
 
 	// expect burst number of successes
-	creds1 := getCredentials(t)
-	testWithMacaroon(ctx, t, creds1, handler, next, done)
-	// expect similar results for a different apiKey / macaroon
-	creds2 := getCredentials(t)
-	testWithMacaroon(ctx, t, creds2, handler, next, done)
-	// expect similar results for a different apiKey / macaroon
-	creds3 := getCredentials(t)
-	testWithMacaroon(ctx, t, creds3, handler, next, done)
+	creds1 := getCredentials(t, true)
+	testWithCredentials(ctx, t, creds1, handler, next, done)
+	// expect similar results for a different project
+	creds2 := getCredentials(t, true)
+	testWithCredentials(ctx, t, creds2, handler, next, done)
+
+	creds3 := getCredentials(t, false)
+	testWithCredentials(ctx, t, creds3, handler, next, done)
+	// expect similar results for a different macaroon head
+	creds4 := getCredentials(t, false)
+	testWithCredentials(ctx, t, creds4, handler, next, done)
 	close(allTests)
 
 	// wait for previous responses to arrive so we can retest cred1
-	for x := 0; x < maxConncurrent*2*3; x++ {
+	for x := 0; x < maxConncurrent*2*4; x++ {
 		<-done
 	}
-	// expect burst number of successes with cred1 again
+	// expect burst number of successes again
 	allTests = make(chan struct{})
-	testWithMacaroon(ctx, t, creds1, handler, next, done)
+	testWithCredentials(ctx, t, creds1, handler, next, done)
+	testWithCredentials(ctx, t, creds3, handler, next, done)
 	close(allTests)
 }
 
-func testWithMacaroon(ctx *testcontext.Context, t *testing.T, creds *Credentials, handler http.Handler, next, done chan struct{}) {
+func testWithCredentials(ctx *testcontext.Context, t *testing.T, creds *Credentials, handler http.Handler, next, done chan struct{}) {
 	// expect maxConncurrent HTTP 200s, then maxConncurrent HTTP 429s
 	for x := 0; x < maxConncurrent*2; x++ {
 		localX := x
@@ -92,7 +97,7 @@ func doRequest(ctx context.Context, t *testing.T, creds *Credentials, handler ht
 	return rr.Code
 }
 
-func getCredentials(t *testing.T) *Credentials {
+func getCredentials(t *testing.T, includePublicProjectID bool) *Credentials {
 	apiKey, err := macaroon.NewAPIKey([]byte("secret"))
 	require.NoError(t, err)
 	ag := grant.Access{
@@ -102,7 +107,7 @@ func getCredentials(t *testing.T) *Credentials {
 	}
 	grant, err := ag.Serialize()
 	require.NoError(t, err)
-	return &Credentials{
+	credentials := Credentials{
 		AccessKey: "accessKey",
 		AuthServiceResponse: authclient.AuthServiceResponse{
 			AccessGrant: grant,
@@ -111,6 +116,10 @@ func getCredentials(t *testing.T) *Credentials {
 		},
 		Error: nil,
 	}
+	if includePublicProjectID {
+		credentials.PublicProjectID = testrand.UUID().String()
+	}
+	return &credentials
 }
 
 func simpleKeyFunc(r *http.Request) (string, error) {