remove webhook cache implementation and replace with the token cache

The striped cache used by the token cache is slightly more sophisticated
however the simple cache provides about the same exact behavior. I used
the striped cache rather than the simple cache because:

* It has been used without issue as the primary token cache.
* It preforms better under load.
* It is already exposed in the public API of the token cache package.

pkg/kubeapiserver/authenticator/config.go

@@ -179,7 +179,7 @@ func (config AuthenticatorConfig) New() (authenticator.Request, *spec.SecurityDe
 		tokenAuth := tokenunion.New(tokenAuthenticators...)
 		// Optionally cache authentication results
 		if config.TokenSuccessCacheTTL > 0 || config.TokenFailureCacheTTL > 0 {
-			tokenAuth = tokencache.New(tokenAuth, config.TokenSuccessCacheTTL, config.TokenFailureCacheTTL)
+			tokenAuth = tokencache.New(tokenAuth, true, config.TokenSuccessCacheTTL, config.TokenFailureCacheTTL)
 		}
 		authenticators = append(authenticators, bearertoken.New(tokenAuth), websocket.NewProtocolAuthenticator(tokenAuth))
 		securityDefinitions["BearerToken"] = &spec.SecurityScheme{
@@ -317,10 +317,10 @@ func newAuthenticatorFromClientCAFile(clientCAFile string) (authenticator.Reques
 }
 
 func newWebhookTokenAuthenticator(webhookConfigFile string, ttl time.Duration) (authenticator.Token, error) {
-	webhookTokenAuthenticator, err := webhook.New(webhookConfigFile, ttl)
+	webhookTokenAuthenticator, err := webhook.New(webhookConfigFile)
 	if err != nil {
 		return nil, err
 	}
 
-	return webhookTokenAuthenticator, nil
+	return tokencache.New(webhookTokenAuthenticator, false, ttl, ttl), nil
 }

staging/src/k8s.io/apiextensions-apiserver/Godeps/Godeps.json

@@ -1354,6 +1354,10 @@
 			"ImportPath": "k8s.io/apiserver/pkg/authentication/serviceaccount",
 			"Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
 		},
+		{
+			"ImportPath": "k8s.io/apiserver/pkg/authentication/token/cache",
+			"Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
+		},
 		{
 			"ImportPath": "k8s.io/apiserver/pkg/authentication/token/tokenfile",
 			"Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"

staging/src/k8s.io/apiserver/pkg/authentication/authenticatorfactory/BUILD

@@ -23,6 +23,7 @@ go_library(
         "//staging/src/k8s.io/apiserver/pkg/authentication/request/union:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/authentication/request/websocket:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/authentication/request/x509:go_default_library",
+        "//staging/src/k8s.io/apiserver/pkg/authentication/token/cache:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/authentication/token/tokenfile:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/authentication/user:go_default_library",
         "//staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/webhook:go_default_library",

staging/src/k8s.io/apiserver/pkg/authentication/authenticatorfactory/delegating.go

@@ -31,6 +31,7 @@ import (
 	unionauth "k8s.io/apiserver/pkg/authentication/request/union"
 	"k8s.io/apiserver/pkg/authentication/request/websocket"
 	"k8s.io/apiserver/pkg/authentication/request/x509"
+	"k8s.io/apiserver/pkg/authentication/token/cache"
 	webhooktoken "k8s.io/apiserver/plugin/pkg/authenticator/token/webhook"
 	authenticationclient "k8s.io/client-go/kubernetes/typed/authentication/v1beta1"
 	"k8s.io/client-go/util/cert"
@@ -85,11 +86,12 @@ func (c DelegatingAuthenticatorConfig) New() (authenticator.Request, *spec.Secur
 	}
 
 	if c.TokenAccessReviewClient != nil {
-		tokenAuth, err := webhooktoken.NewFromInterface(c.TokenAccessReviewClient, c.CacheTTL)
+		tokenAuth, err := webhooktoken.NewFromInterface(c.TokenAccessReviewClient)
 		if err != nil {
 			return nil, nil, err
 		}
-		authenticators = append(authenticators, bearertoken.New(tokenAuth), websocket.NewProtocolAuthenticator(tokenAuth))
+		cachingTokenAuth := cache.New(tokenAuth, false, c.CacheTTL, c.CacheTTL)
+		authenticators = append(authenticators, bearertoken.New(cachingTokenAuth), websocket.NewProtocolAuthenticator(cachingTokenAuth))
 
 		securityDefinitions["BearerToken"] = &spec.SecurityScheme{
 			SecuritySchemeProps: spec.SecuritySchemeProps{

staging/src/k8s.io/apiserver/pkg/authentication/token/cache/cached_token_authenticator.go

@@ -36,6 +36,7 @@ type cacheRecord struct {
 type cachedTokenAuthenticator struct {
 	authenticator authenticator.Token
 
+	cacheErrs  bool
 	successTTL time.Duration
 	failureTTL time.Duration
 
@@ -52,13 +53,14 @@ type cache interface {
 }
 
 // New returns a token authenticator that caches the results of the specified authenticator. A ttl of 0 bypasses the cache.
-func New(authenticator authenticator.Token, successTTL, failureTTL time.Duration) authenticator.Token {
+func New(authenticator authenticator.Token, cacheErrs bool, successTTL, failureTTL time.Duration) authenticator.Token {
-	return newWithClock(authenticator, successTTL, failureTTL, utilclock.RealClock{})
+	return newWithClock(authenticator, cacheErrs, successTTL, failureTTL, utilclock.RealClock{})
 }
 
-func newWithClock(authenticator authenticator.Token, successTTL, failureTTL time.Duration, clock utilclock.Clock) authenticator.Token {
+func newWithClock(authenticator authenticator.Token, cacheErrs bool, successTTL, failureTTL time.Duration, clock utilclock.Clock) authenticator.Token {
 	return &cachedTokenAuthenticator{
 		authenticator: authenticator,
+		cacheErrs:     cacheErrs,
 		successTTL:    successTTL,
 		failureTTL:    failureTTL,
 		cache:         newStripedCache(32, fnvHashFunc, func() cache { return newSimpleCache(128, clock) }),
@@ -75,6 +77,9 @@ func (a *cachedTokenAuthenticator) AuthenticateToken(ctx context.Context, token
 	}
 
 	resp, ok, err := a.authenticator.AuthenticateToken(ctx, token)
+	if !a.cacheErrs && err != nil {
+		return resp, ok, err
+	}
 
 	switch {
 	case ok && a.successTTL > 0:

staging/src/k8s.io/apiserver/pkg/authentication/token/cache/cached_token_authenticator_test.go

@@ -42,7 +42,7 @@ func TestCachedTokenAuthenticator(t *testing.T) {
 	})
 	fakeClock := utilclock.NewFakeClock(time.Now())
 
-	a := newWithClock(fakeAuth, time.Minute, 0, fakeClock)
+	a := newWithClock(fakeAuth, true, time.Minute, 0, fakeClock)
 
 	calledWithToken, resultUsers, resultOk, resultErr = []string{}, nil, false, nil
 	a.AuthenticateToken(context.Background(), "bad1")
@@ -114,7 +114,7 @@ func TestCachedTokenAuthenticatorWithAudiences(t *testing.T) {
 	})
 	fakeClock := utilclock.NewFakeClock(time.Now())
 
-	a := newWithClock(fakeAuth, time.Minute, 0, fakeClock)
+	a := newWithClock(fakeAuth, true, time.Minute, 0, fakeClock)
 
 	resultUsers["audAusertoken1"] = &user.DefaultInfo{Name: "user1"}
 	resultUsers["audBusertoken1"] = &user.DefaultInfo{Name: "user1-different"}

staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/webhook/BUILD

@@ -16,6 +16,8 @@ go_test(
     deps = [
         "//staging/src/k8s.io/api/authentication/v1beta1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
+        "//staging/src/k8s.io/apiserver/pkg/authentication/authenticator:go_default_library",
+        "//staging/src/k8s.io/apiserver/pkg/authentication/token/cache:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/authentication/user:go_default_library",
         "//staging/src/k8s.io/client-go/tools/clientcmd/api/v1:go_default_library",
     ],
@@ -30,7 +32,6 @@ go_library(
         "//staging/src/k8s.io/api/authentication/v1beta1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
-        "//staging/src/k8s.io/apimachinery/pkg/util/cache:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/authentication/authenticator:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/authentication/user:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/util/webhook:go_default_library",

staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/webhook/webhook.go

@@ -26,7 +26,6 @@ import (
 	authentication "k8s.io/api/authentication/v1beta1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
-	"k8s.io/apimachinery/pkg/util/cache"
 	"k8s.io/apiserver/pkg/authentication/authenticator"
 	"k8s.io/apiserver/pkg/authentication/user"
 	"k8s.io/apiserver/pkg/util/webhook"
@@ -45,28 +44,29 @@ var _ authenticator.Token = (*WebhookTokenAuthenticator)(nil)
 
 type WebhookTokenAuthenticator struct {
 	tokenReview    authenticationclient.TokenReviewInterface
-	responseCache  *cache.LRUExpireCache
-	ttl            time.Duration
 	initialBackoff time.Duration
 }
 
-// NewFromInterface creates a webhook authenticator using the given tokenReview client
+// NewFromInterface creates a webhook authenticator using the given tokenReview
-func NewFromInterface(tokenReview authenticationclient.TokenReviewInterface, ttl time.Duration) (*WebhookTokenAuthenticator, error) {
+// client. It is recommend to wrap this authenticator with the token cache
-	return newWithBackoff(tokenReview, ttl, retryBackoff)
+// authenticator implemented in
+// k8s.io/apiserver/pkg/authentication/token/cache.
+func NewFromInterface(tokenReview authenticationclient.TokenReviewInterface) (*WebhookTokenAuthenticator, error) {
+	return newWithBackoff(tokenReview, retryBackoff)
 }
 
 // New creates a new WebhookTokenAuthenticator from the provided kubeconfig file.
-func New(kubeConfigFile string, ttl time.Duration) (*WebhookTokenAuthenticator, error) {
+func New(kubeConfigFile string) (*WebhookTokenAuthenticator, error) {
 	tokenReview, err := tokenReviewInterfaceFromKubeconfig(kubeConfigFile)
 	if err != nil {
 		return nil, err
 	}
-	return newWithBackoff(tokenReview, ttl, retryBackoff)
+	return newWithBackoff(tokenReview, retryBackoff)
 }
 
 // newWithBackoff allows tests to skip the sleep.
-func newWithBackoff(tokenReview authenticationclient.TokenReviewInterface, ttl, initialBackoff time.Duration) (*WebhookTokenAuthenticator, error) {
+func newWithBackoff(tokenReview authenticationclient.TokenReviewInterface, initialBackoff time.Duration) (*WebhookTokenAuthenticator, error) {
-	return &WebhookTokenAuthenticator{tokenReview, cache.NewLRUExpireCache(1024), ttl, initialBackoff}, nil
+	return &WebhookTokenAuthenticator{tokenReview, initialBackoff}, nil
 }
 
 // AuthenticateToken implements the authenticator.Token interface.
@@ -74,25 +74,20 @@ func (w *WebhookTokenAuthenticator) AuthenticateToken(ctx context.Context, token
 	r := &authentication.TokenReview{
 		Spec: authentication.TokenReviewSpec{Token: token},
 	}
-	if entry, ok := w.responseCache.Get(r.Spec); ok {
+	var (
-		r.Status = entry.(authentication.TokenReviewStatus)
+		result *authentication.TokenReview
-	} else {
+		err    error
-		var (
+	)
-			result *authentication.TokenReview
+	webhook.WithExponentialBackoff(w.initialBackoff, func() error {
-			err    error
+		result, err = w.tokenReview.Create(r)
-		)
+		return err
-		webhook.WithExponentialBackoff(w.initialBackoff, func() error {
+	})
-			result, err = w.tokenReview.Create(r)
+	if err != nil {
-			return err
+		// An error here indicates bad configuration or an outage. Log for debugging.
-		})
+		glog.Errorf("Failed to make webhook authenticator request: %v", err)
-		if err != nil {
+		return nil, false, err
-			// An error here indicates bad configuration or an outage. Log for debugging.
-			glog.Errorf("Failed to make webhook authenticator request: %v", err)
-			return nil, false, err
-		}
-		r.Status = result.Status
-		w.responseCache.Add(r.Spec, result.Status, w.ttl)
 	}
+	r.Status = result.Status
 	if !r.Status.Authenticated {
 		return nil, false, nil
 	}

staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/webhook/webhook_test.go

@@ -33,6 +33,8 @@ import (
 
 	"k8s.io/api/authentication/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apiserver/pkg/authentication/authenticator"
+	"k8s.io/apiserver/pkg/authentication/token/cache"
 	"k8s.io/apiserver/pkg/authentication/user"
 	"k8s.io/client-go/tools/clientcmd/api/v1"
 )
@@ -166,7 +168,7 @@ func (m *mockService) HTTPStatusCode() int { return m.statusCode }
 
 // newTokenAuthenticator creates a temporary kubeconfig file from the provided
 // arguments and attempts to load a new WebhookTokenAuthenticator from it.
-func newTokenAuthenticator(serverURL string, clientCert, clientKey, ca []byte, cacheTime time.Duration) (*WebhookTokenAuthenticator, error) {
+func newTokenAuthenticator(serverURL string, clientCert, clientKey, ca []byte, cacheTime time.Duration) (authenticator.Token, error) {
 	tempfile, err := ioutil.TempFile("", "")
 	if err != nil {
 		return nil, err
@@ -194,7 +196,12 @@ func newTokenAuthenticator(serverURL string, clientCert, clientKey, ca []byte, c
 		return nil, err
 	}
 
-	return newWithBackoff(c, cacheTime, 0)
+	authn, err := newWithBackoff(c, 0)
+	if err != nil {
+		return nil, err
+	}
+
+	return cache.New(authn, false, cacheTime, cacheTime), nil
 }
 
 func TestTLSConfig(t *testing.T) {
@@ -549,15 +556,12 @@ func TestWebhookCacheAndRetry(t *testing.T) {
 			_, ok, err := wh.AuthenticateToken(context.Background(), testcase.token)
 			hasError := err != nil
 			if hasError != testcase.expectError {
-				t.Log(testcase.description)
 				t.Errorf("Webhook returned HTTP %d, expected error=%v, but got error %v", testcase.code, testcase.expectError, err)
 			}
 			if serv.called != testcase.expectCalls {
-				t.Log(testcase.description)
 				t.Errorf("Expected %d calls, got %d", testcase.expectCalls, serv.called)
 			}
 			if ok != testcase.expectOk {
-				t.Log(testcase.description)
 				t.Errorf("Expected ok=%v, got %v", testcase.expectOk, ok)
 			}
 		})

staging/src/k8s.io/kube-aggregator/Godeps/Godeps.json

@@ -1006,6 +1006,10 @@
 			"ImportPath": "k8s.io/apiserver/pkg/authentication/serviceaccount",
 			"Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
 		},
+		{
+			"ImportPath": "k8s.io/apiserver/pkg/authentication/token/cache",
+			"Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
+		},
 		{
 			"ImportPath": "k8s.io/apiserver/pkg/authentication/token/tokenfile",
 			"Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"

staging/src/k8s.io/sample-apiserver/Godeps/Godeps.json

@@ -970,6 +970,10 @@
 			"ImportPath": "k8s.io/apiserver/pkg/authentication/serviceaccount",
 			"Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
 		},
+		{
+			"ImportPath": "k8s.io/apiserver/pkg/authentication/token/cache",
+			"Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
+		},
 		{
 			"ImportPath": "k8s.io/apiserver/pkg/authentication/token/tokenfile",
 			"Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"

test/integration/auth/BUILD

@@ -63,6 +63,7 @@ go_test(
         "//staging/src/k8s.io/apiserver/pkg/authentication/group:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/authentication/request/bearertoken:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/authentication/serviceaccount:go_default_library",
+        "//staging/src/k8s.io/apiserver/pkg/authentication/token/cache:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/authentication/token/tokenfile:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/authentication/user:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/authorization/authorizer:go_default_library",

test/integration/auth/auth_test.go

@@ -40,6 +40,7 @@ import (
 	"k8s.io/apiserver/pkg/authentication/group"
 	"k8s.io/apiserver/pkg/authentication/request/bearertoken"
 	"k8s.io/apiserver/pkg/authentication/serviceaccount"
+	"k8s.io/apiserver/pkg/authentication/token/cache"
 	"k8s.io/apiserver/pkg/authentication/user"
 	"k8s.io/apiserver/pkg/authorization/authorizer"
 	"k8s.io/apiserver/pkg/authorization/authorizerfactory"
@@ -84,11 +85,11 @@ func getTestWebhookTokenAuth(serverURL string) (authenticator.Request, error) {
 	if err := json.NewEncoder(kubecfgFile).Encode(config); err != nil {
 		return nil, err
 	}
-	webhookTokenAuth, err := webhook.New(kubecfgFile.Name(), 2*time.Minute)
+	webhookTokenAuth, err := webhook.New(kubecfgFile.Name())
 	if err != nil {
 		return nil, err
 	}
-	return bearertoken.New(webhookTokenAuth), nil
+	return bearertoken.New(cache.New(webhookTokenAuth, false, 2*time.Minute, 2*time.Minute)), nil
 }
 
 func path(resource, namespace, name string) string {