[ARVADOS] updated: d2469485358aebd3dd55727f272efd50a2e92a24

git at public.curoverse.com git at public.curoverse.com
Mon Aug 18 13:13:49 EDT 2014


Summary of changes:
 services/keep/src/keep/api_client.go      |  84 ++++++++++++++++++++-----
 services/keep/src/keep/api_client_test.go | 100 ++++++++++++++++++++++++------
 services/keep/src/keep/keep.go            |  10 ++-
 3 files changed, 156 insertions(+), 38 deletions(-)

       via  d2469485358aebd3dd55727f272efd50a2e92a24 (commit)
      from  db783c021cbad2dd4ab4a4db2783000430e292b2 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit d2469485358aebd3dd55727f272efd50a2e92a24
Author: Tim Pierce <twp at curoverse.com>
Date:   Mon Aug 18 13:08:59 2014 -0400

    2769: cache API tokens.
    
    The api_client wrapper functions cache user information retrieved from
    the API server (the User and ApiClientAuthorization records).
    
    TestTokenCache is a unit test that exercises the cache to demonstrate
    that when a token is found in the cache but not in the API server, the
    cached value is returned.
    
    The token_cache_ttl command-line flag sets the TTL for cached
    tokens. The default is one hour.
    
    Refs #2769.

diff --git a/services/keep/src/keep/api_client.go b/services/keep/src/keep/api_client.go
index 90b947a..5e9671a 100644
--- a/services/keep/src/keep/api_client.go
+++ b/services/keep/src/keep/api_client.go
@@ -6,9 +6,18 @@ import (
 	"log"
 	"net/http"
 	"os"
-	_ "time"
+	"time"
 )
 
+// tokenInfo records information we get from an API token
+type tokenInfo struct {
+	GeneratedAt            time.Time
+	User                   sdk.Dict
+	ApiClientAuthorization sdk.Dict
+}
+
+var tokenCache map[string]*tokenInfo
+
 // MakeApiClient returns a sdk.ArvadosClient suitable for connecting to the
 // requested API server.
 //
@@ -31,32 +40,73 @@ func MakeApiClient(api_token string) sdk.ArvadosClient {
 // "true").
 //
 func IsAdmin(api_token string) bool {
-	var api_client = MakeApiClient(api_token)
-	// Ask the API server whether this user is an admin.
-	var userinfo sdk.Dict
-	if err := api_client.List("users/current", nil, &userinfo); err != nil {
-		log.Printf("IsAdmin: %s\n", err)
+	if tok := lookupToken(api_token); tok == nil {
 		return false
+	} else {
+		return tok.User["is_admin"].(bool)
 	}
-	return userinfo["is_admin"].(bool)
 }
 
 // HasUnlimitedScope returns true if the scopes attached to this
 // token's ApiClientAuthorization record include "all".
 func HasUnlimitedScope(api_token string) bool {
-	var api_client = MakeApiClient(api_token)
-	var auth sdk.Dict
-	req := "api_client_authorizations/" + api_token
-	if err := api_client.List(req, nil, &auth); err != nil {
-		log.Printf("HasUnlimitedScope: %s\n", err)
+	if tok := lookupToken(api_token); tok == nil {
 		return false
+	} else {
+		if tok.ApiClientAuthorization["scopes"] == nil {
+			return false
+		}
+		var scopes []interface{} = tok.ApiClientAuthorization["scopes"].([]interface{})
+		for _, s := range scopes {
+			if s.(string) == "all" {
+				return true
+			}
+		}
 	}
+	return false
+}
 
-	var scopes []interface{} = auth["scopes"].([]interface{})
-	for _, s := range scopes {
-		if s.(string) == "all" {
-			return true
+// lookupToken fetches information about an API token (the User and
+// ApiClientAuthorization methods belonging to it). It will use a
+// cached value, refreshing the cache if the token is not found or has
+// expired.
+//
+func lookupToken(api_token string) *tokenInfo {
+	var refresh bool = true
+	var ti *tokenInfo
+	var ok bool
+
+	if tokenCache == nil {
+		tokenCache = make(map[string]*tokenInfo)
+	}
+	if ti, ok = tokenCache[api_token]; ok {
+		token_age := time.Now().Sub(ti.GeneratedAt)
+		if token_age < time.Duration(token_cache_ttl)*time.Second {
+			refresh = false
 		}
 	}
-	return false
+	if refresh {
+		ti = fetchTokenInfo(api_token)
+		tokenCache[api_token] = ti
+	}
+	return ti
+}
+
+// fetchTokenInfo issues API calls for the User and
+// ApiClientAuthorization record associated with a token. It returns a
+// tokenInfo with the fetched data.  If any data could
+func fetchTokenInfo(api_token string) *tokenInfo {
+	var ti tokenInfo
+	var api_client = MakeApiClient(api_token)
+	if err := api_client.List("users/current", nil, &ti.User); err != nil {
+		log.Printf("fetchTokenInfo: %s\n", err)
+		return nil
+	}
+	req := "api_client_authorizations/" + api_token
+	if err := api_client.List(req, nil, &ti.ApiClientAuthorization); err != nil {
+		log.Printf("fetchTokenInfo: %s\n", err)
+		return nil
+	}
+	ti.GeneratedAt = time.Now()
+	return &ti
 }
diff --git a/services/keep/src/keep/api_client_test.go b/services/keep/src/keep/api_client_test.go
index 4c4fe8a..728b977 100644
--- a/services/keep/src/keep/api_client_test.go
+++ b/services/keep/src/keep/api_client_test.go
@@ -32,12 +32,14 @@ var api_token = map[string]string{
 // and URI path, then the response from the server will
 // use the corresponding HTTP status and response body.
 //
-var apiserver_responses = []struct {
+type APIRoute struct {
 	token    string
 	path     string
 	status   int
 	response string
-}{
+}
+
+var DefaultFakeAPIRoutes = []APIRoute{
 	// /users/current requests
 	// admin_* tokens return {"is_admin":true}
 	// user_* tokens return {"is_admin":false}
@@ -95,7 +97,7 @@ var apiserver_responses = []struct {
 	},
 	{
 		token:    api_token["admin_noscope"],
-		path:     "/arvados/v1/api_client_authorizations/" + api_token["admin_badscope"],
+		path:     "/arvados/v1/api_client_authorizations/" + api_token["admin_noscope"],
 		status:   http.StatusOK,
 		response: `{"uuid":"admin_noscope"}`,
 	},
@@ -119,23 +121,8 @@ var apiserver_responses = []struct {
 	},
 }
 
-// FakeAPIServer is the http.HandlerFunc implementing the test API
-// server.
-//
-var FakeAPIServer = http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) {
-	tok := GetApiToken(req)
-	for _, test := range apiserver_responses {
-		if test.token == tok && test.path == req.URL.Path {
-			resp.WriteHeader(test.status)
-			resp.Write([]byte(test.response))
-			return
-		}
-	}
-	http.Error(resp, "Internal server error", http.StatusInternalServerError)
-})
-
 func TestIsAdmin(t *testing.T) {
-	ts := httptest.NewUnstartedServer(FakeAPIServer)
+	ts := MakeFakeAPIServer(DefaultFakeAPIRoutes)
 	ts.StartTLS()
 	defer ts.Close()
 
@@ -161,7 +148,7 @@ func TestIsAdmin(t *testing.T) {
 }
 
 func TestHasUnlimitedScope(t *testing.T) {
-	ts := httptest.NewUnstartedServer(FakeAPIServer)
+	ts := MakeFakeAPIServer(DefaultFakeAPIRoutes)
 	ts.StartTLS()
 	defer ts.Close()
 
@@ -185,3 +172,76 @@ func TestHasUnlimitedScope(t *testing.T) {
 		}
 	}
 }
+
+// TestTokenCache exercises the API token cache.  It issues a token
+// lookup to fill the cache, then tests that a second lookup retrieves
+// the data from cache.
+//
+func TestTokenCache(t *testing.T) {
+	cache_test_token := "cache_test_token"
+	// Default token_cache_ttl = 1 hour (plenty long for unit test)
+	token_cache_ttl = 3600
+	testroutes := append(DefaultFakeAPIRoutes,
+		APIRoute{
+			token:    cache_test_token,
+			path:     "/arvados/v1/users/current",
+			status:   http.StatusOK,
+			response: `{"is_admin":true}`,
+		},
+		APIRoute{
+			token:    cache_test_token,
+			path:     "/arvados/v1/api_client_authorizations/" + cache_test_token,
+			status:   http.StatusOK,
+			response: `{"uuid":"cache_test_token","scopes":["all"]}`,
+		})
+
+	ts := MakeFakeAPIServer(testroutes)
+	ts.StartTLS()
+	defer ts.Close()
+
+	os.Setenv("ARVADOS_API_HOST", ts.Listener.Addr().String())
+	os.Setenv("ARVADOS_API_HOST_INSECURE", "true")
+
+	// Fill the cache.
+	// Use a token belonging to an admin to ensure that
+	// IsAdmin(cache_test_token) will return true.
+	//
+	uncached_result := IsAdmin(cache_test_token)
+	if uncached_result != true {
+		t.Errorf("%s: expected %v\nreceived %v",
+			cache_test_token, true, uncached_result)
+	}
+
+	// Restart the server, but without the cache_test_token.  When
+	// IsAdmin is called, it should return the cached value even
+	// though the API server no longer knows about cache_test_token.
+	ts.Close()
+
+	ts = MakeFakeAPIServer(DefaultFakeAPIRoutes)
+	ts.StartTLS()
+
+	os.Setenv("ARVADOS_API_HOST", ts.Listener.Addr().String())
+	os.Setenv("ARVADOS_API_HOST_INSECURE", "true")
+
+	cached_result := IsAdmin(cache_test_token)
+	if cached_result != true {
+		t.Errorf("%s: expected %v\nreceived %v",
+			cache_test_token, true, cached_result)
+	}
+}
+
+func MakeFakeAPIServer(routes []APIRoute) *httptest.Server {
+	var apihandler = http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) {
+		tok := GetApiToken(req)
+		for _, route := range routes {
+			if route.token == tok && route.path == req.URL.Path {
+				resp.WriteHeader(route.status)
+				resp.Write([]byte(route.response))
+				return
+			}
+		}
+		http.Error(resp, "Internal server error", http.StatusInternalServerError)
+	})
+
+	return httptest.NewUnstartedServer(apihandler)
+}
diff --git a/services/keep/src/keep/keep.go b/services/keep/src/keep/keep.go
index c34c268..4f8eb8f 100644
--- a/services/keep/src/keep/keep.go
+++ b/services/keep/src/keep/keep.go
@@ -53,6 +53,10 @@ var permission_ttl time.Duration
 // Initialized by the --data-manager-token-file flag.
 var data_manager_token string
 
+// token_cache_ttl is the number of seconds for which to honor the API
+// token cache.  Tokens older than this should be considered stale and discarded.
+var token_cache_ttl int
+
 // ==========
 // Error types.
 //
@@ -158,12 +162,16 @@ func main() {
 			"e.g. -volumes=/var/keep1,/var/keep2. If empty or not "+
 			"supplied, Keep will scan mounted filesystems for volumes "+
 			"with a /keep top-level directory.")
-
 	flag.StringVar(
 		&pidfile,
 		"pid",
 		"",
 		"Path to write pid file")
+	flag.IntVar(
+		&token_cache_ttl,
+		"token-cache-ttl",
+		3600,
+		"TTL (in seconds) for cached tokens looked up from the API server.")
 
 	flag.Parse()
 

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list