github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-08-15 23:03:40 +00:00
Code Issues Projects Releases Wiki Activity

Don't cache rediculous subject access reviews

Browse Source
This commit is contained in:
Tim Allclair 2018-10-26 13:18:06 -07:00
parent cddb4a327a
commit ea1b4eb239
2 changed files with 79 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook.go
View File

@ -39,7 +39,11 @@ var (
groupVersions = []schema.GroupVersion{authorization.SchemeGroupVersion}
)
const retryBackoff = 500 * time.Millisecond
const (
retryBackoff = 500 * time.Millisecond
// The maximum key length for unauthorized request keys to qualify for caching.
maxUnauthorizedCachedKeySize = 10000
)
// Ensure Webhook implements the authorizer.Authorizer interface.
var _ authorizer.Authorizer = (*WebhookAuthorizer)(nil)
@ -196,7 +200,9 @@ func (w *WebhookAuthorizer) Authorize(attr authorizer.Attributes) (decision auth
if r.Status.Allowed {
w.responseCache.Add(string(key), r.Status, w.authorizedTTL)
} else {
w.responseCache.Add(string(key), r.Status, w.unauthorizedTTL)
if len(key) <= maxUnauthorizedCachedKeySize {
w.responseCache.Add(string(key), r.Status, w.unauthorizedTTL)
}
}
}
switch {

117
staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook_test.go
View File

@ -28,6 +28,7 @@ import (
"os"
"path/filepath"
"reflect"
"strings"
"testing"
"text/template"
"time"
@ -542,41 +543,6 @@ func TestWebhook(t *testing.T) {
}
}
type webhookCacheTestCase struct {
attr authorizer.AttributesRecord
allow bool
statusCode int
expectedErr bool
expectedAuthorized bool
expectedCalls int
}
func testWebhookCacheCases(t *testing.T, serv *mockService, wh *WebhookAuthorizer, tests []webhookCacheTestCase) {
for i, test := range tests {
serv.called = 0
serv.allow = test.allow
serv.statusCode = test.statusCode
authorized, _, err := wh.Authorize(test.attr)
if test.expectedErr && err == nil {
t.Errorf("%d: Expected error", i)
continue
} else if !test.expectedErr && err != nil {
t.Errorf("%d: unexpected error: %v", i, err)
continue
}
if test.expectedAuthorized != (authorized == authorizer.DecisionAllow) {
t.Errorf("%d: expected authorized=%v, got %v", i, test.expectedAuthorized, authorized)
}
if test.expectedCalls != serv.called {
t.Errorf("%d: expected %d calls, got %d", i, test.expectedCalls, serv.called)
}
}
}
// TestWebhookCache verifies that error responses from the server are not
// cached, but successful responses are.
func TestWebhookCache(t *testing.T) {
@ -595,27 +561,86 @@ func TestWebhookCache(t *testing.T) {
aliceAttr := authorizer.AttributesRecord{User: &user.DefaultInfo{Name: "alice"}}
bobAttr := authorizer.AttributesRecord{User: &user.DefaultInfo{Name: "bob"}}
aliceRidiculousAttr := authorizer.AttributesRecord{
User: &user.DefaultInfo{Name: "alice"},
ResourceRequest: true,
Verb: strings.Repeat("v", 2000),
APIGroup: strings.Repeat("g", 2000),
APIVersion: strings.Repeat("a", 2000),
Resource: strings.Repeat("r", 2000),
Name: strings.Repeat("n", 2000),
}
bobRidiculousAttr := authorizer.AttributesRecord{
User: &user.DefaultInfo{Name: "bob"},
ResourceRequest: true,
Verb: strings.Repeat("v", 2000),
APIGroup: strings.Repeat("g", 2000),
APIVersion: strings.Repeat("a", 2000),
Resource: strings.Repeat("r", 2000),
Name: strings.Repeat("n", 2000),
}
type webhookCacheTestCase struct {
name string
attr authorizer.AttributesRecord
allow bool
statusCode int
expectedErr bool
expectedAuthorized bool
expectedCalls int
}
tests := []webhookCacheTestCase{
// server error and 429's retry
{attr: aliceAttr, allow: false, statusCode: 500, expectedErr: true, expectedAuthorized: false, expectedCalls: 5},
{attr: aliceAttr, allow: false, statusCode: 429, expectedErr: true, expectedAuthorized: false, expectedCalls: 5},
{name: "server errors retry", attr: aliceAttr, allow: false, statusCode: 500, expectedErr: true, expectedAuthorized: false, expectedCalls: 5},
{name: "429s retry", attr: aliceAttr, allow: false, statusCode: 429, expectedErr: true, expectedAuthorized: false, expectedCalls: 5},
// regular errors return errors but do not retry
{attr: aliceAttr, allow: false, statusCode: 404, expectedErr: true, expectedAuthorized: false, expectedCalls: 1},
{attr: aliceAttr, allow: false, statusCode: 403, expectedErr: true, expectedAuthorized: false, expectedCalls: 1},
{attr: aliceAttr, allow: false, statusCode: 401, expectedErr: true, expectedAuthorized: false, expectedCalls: 1},
{name: "404 doesnt retry", attr: aliceAttr, allow: false, statusCode: 404, expectedErr: true, expectedAuthorized: false, expectedCalls: 1},
{name: "403 doesnt retry", attr: aliceAttr, allow: false, statusCode: 403, expectedErr: true, expectedAuthorized: false, expectedCalls: 1},
{name: "401 doesnt retry", attr: aliceAttr, allow: false, statusCode: 401, expectedErr: true, expectedAuthorized: false, expectedCalls: 1},
// successful responses are cached
{attr: aliceAttr, allow: true, statusCode: 200, expectedErr: false, expectedAuthorized: true, expectedCalls: 1},
{name: "alice successful request", attr: aliceAttr, allow: true, statusCode: 200, expectedErr: false, expectedAuthorized: true, expectedCalls: 1},
// later requests within the cache window don't hit the backend
{attr: aliceAttr, allow: false, statusCode: 500, expectedErr: false, expectedAuthorized: true, expectedCalls: 0},
{name: "alice cached request", attr: aliceAttr, allow: false, statusCode: 500, expectedErr: false, expectedAuthorized: true, expectedCalls: 0},
// a request with different attributes doesn't hit the cache
{attr: bobAttr, allow: false, statusCode: 500, expectedErr: true, expectedAuthorized: false, expectedCalls: 5},
{name: "bob failed request", attr: bobAttr, allow: false, statusCode: 500, expectedErr: true, expectedAuthorized: false, expectedCalls: 5},
// successful response for other attributes is cached
{attr: bobAttr, allow: true, statusCode: 200, expectedErr: false, expectedAuthorized: true, expectedCalls: 1},
{name: "bob unauthorized request", attr: bobAttr, allow: false, statusCode: 200, expectedErr: false, expectedAuthorized: false, expectedCalls: 1},
// later requests within the cache window don't hit the backend
{attr: bobAttr, allow: false, statusCode: 500, expectedErr: false, expectedAuthorized: true, expectedCalls: 0},
{name: "bob unauthorized cached request", attr: bobAttr, allow: false, statusCode: 500, expectedErr: false, expectedAuthorized: false, expectedCalls: 0},
// ridiculous unauthorized requests are not cached.
{name: "ridiculous unauthorized request", attr: bobRidiculousAttr, allow: false, statusCode: 200, expectedErr: false, expectedAuthorized: false, expectedCalls: 1},
// later ridiculous requests within the cache window still hit the backend
{name: "ridiculous unauthorized request again", attr: bobRidiculousAttr, allow: false, statusCode: 200, expectedErr: false, expectedAuthorized: false, expectedCalls: 1},
// ridiculous authorized requests are still cached.
{name: "ridiculous authorized request", attr: aliceRidiculousAttr, allow: true, statusCode: 200, expectedErr: false, expectedAuthorized: true, expectedCalls: 1},
// later ridiculous requests within the cache window don't hit the backend
{name: "ridiculous authorized request again", attr: aliceRidiculousAttr, allow: false, statusCode: 500, expectedErr: false, expectedAuthorized: true, expectedCalls: 0},
}
testWebhookCacheCases(t, serv, wh, tests)
for i, test := range tests {
t.Run(test.name, func(t *testing.T) {
serv.called = 0
serv.allow = test.allow
serv.statusCode = test.statusCode
authorized, _, err := wh.Authorize(test.attr)
if test.expectedErr && err == nil {
t.Fatalf("%d: Expected error", i)
} else if !test.expectedErr && err != nil {
t.Fatalf("%d: unexpected error: %v", i, err)
}
if test.expectedAuthorized != (authorized == authorizer.DecisionAllow) {
t.Errorf("%d: expected authorized=%v, got %v", i, test.expectedAuthorized, authorized)
}
if test.expectedCalls != serv.called {
t.Errorf("%d: expected %d calls, got %d", i, test.expectedCalls, serv.called)
}
})
}
}