From 7052e379e4389ece3ebd321ce88b68a05b8fd2b8 Mon Sep 17 00:00:00 2001 From: Monis Khan Date: Mon, 16 Mar 2026 16:08:34 -0400 Subject: [PATCH] KEP-5284: optimize constrained impersonation cache key construction This change reduces allocations by: - Replacing cryptobyte.Builder with manual length-prefix encoding into []byte - Replacing fmt.Sprintf with hex.Encode/hex.AppendEncode - Using unsafe.String for safe []byte to string conversions Signed-off-by: Monis Khan --- .../endpoints/filters/impersonation/cache.go | 68 +++++++++++-------- .../constrained_impersonation_test.go | 8 +++ 2 files changed, 48 insertions(+), 28 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/impersonation/cache.go b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/impersonation/cache.go index cf73107896a..c0ef42c583a 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/impersonation/cache.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/impersonation/cache.go @@ -18,11 +18,12 @@ package impersonation import ( "crypto/sha256" + "encoding/binary" + "encoding/hex" "fmt" "hash/fnv" "time" - - "golang.org/x/crypto/cryptobyte" + "unsafe" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" @@ -60,21 +61,19 @@ func modeIndexCacheKey(attributes authorizer.Attributes) string { key := attributes.GetUser().GetName() // hash the name so our cache size is predicable regardless of the size of usernames // collisions do not matter for this logic as it simply changes the ordering of the modes used - hash := fnvSum128a([]byte(key)) - return fmt.Sprintf("%x", hash) -} - -func fnvSum128a(data []byte) []byte { h := fnv.New128a() - h.Write(data) + h.Write([]byte(key)) var sum [16]byte - return h.Sum(sum[:0]) + h.Sum(sum[:0]) + buf := make([]byte, 32) // 16 bytes of hash -> 32 hex chars + hex.Encode(buf, sum[:]) + return toString(buf) // only safe because buf is heap allocated via make } func newModeIndexCache() *modeIndexCache { return &modeIndexCache{ - // each entry is roughly ~24 bytes (16 bytes for the hashed key, 8 bytes for value) - // thus at even 10k entries, we should use less than 1 MB memory + // each entry is roughly ~100 bytes (hashed string key + int value + LRU list/map overhead) + // thus at even 10k entries, we should use about 1 MB memory // this hardcoded size allows us to remember many users without leaking memory cache: lru.New(10_000), } @@ -246,7 +245,7 @@ func buildKey(wantedUser *user.DefaultInfo, attributes authorizer.Attributes) (s } }) - return b.build() + return b.build(), nil } func addUser(b *cacheKeyBuilder, u user.Info) { @@ -265,20 +264,21 @@ func addUser(b *cacheKeyBuilder, u user.Info) { }) } -// cacheKeyBuilder adds syntactic sugar on top of cryptobyte.Builder to make it easier to use for complex inputs. +// cacheKeyBuilder builds a binary key from structured inputs using uint32 length-prefixed encoding, then hashes +// it with SHA-256. All methods operate on a single shared buffer to avoid closure and child-builder allocations. type cacheKeyBuilder struct { namespace string // in the programming sense, not the Kubernetes concept - builder *cryptobyte.Builder + builder []byte } func newCacheKeyBuilder(namespace string) *cacheKeyBuilder { // start with a reasonable size to avoid too many allocations - return &cacheKeyBuilder{namespace: namespace, builder: cryptobyte.NewBuilder(make([]byte, 0, 384))} + return &cacheKeyBuilder{namespace: namespace, builder: make([]byte, 0, 384)} } func (c *cacheKeyBuilder) addString(value string) *cacheKeyBuilder { c.addLengthPrefixed(func(c *cacheKeyBuilder) { - c.builder.AddBytes([]byte(value)) + c.builder = append(c.builder, value...) }) return c } @@ -297,24 +297,36 @@ func (c *cacheKeyBuilder) addBool(value bool) *cacheKeyBuilder { if value { b = 1 } - c.builder.AddUint8(b) + c.builder = append(c.builder, b) return c } type builderContinuation func(child *cacheKeyBuilder) func (c *cacheKeyBuilder) addLengthPrefixed(f builderContinuation) { - c.builder.AddUint32LengthPrefixed(func(b *cryptobyte.Builder) { - c := &cacheKeyBuilder{namespace: c.namespace, builder: b} - f(c) - }) + offset := len(c.builder) + c.builder = append(c.builder, 0, 0, 0, 0) // placeholder for uint32 length + f(c) + binary.BigEndian.PutUint32(c.builder[offset:], uint32(len(c.builder)-offset-4)) } -func (c *cacheKeyBuilder) build() (string, error) { - key, err := c.builder.Bytes() - if err != nil { - return "", err - } - hash := sha256.Sum256(key) // reduce the size of the cache key to keep the overall cache size small - return fmt.Sprintf("%x/%s", hash[:], c.namespace), nil +func (c *cacheKeyBuilder) build() string { + hashed := sha256.Sum256(c.builder) // reduce the size of the cache key to keep the overall cache size small + // sha256 = 32 bytes -> 64 hex chars + "/" + namespace + buf := make([]byte, 0, 64+1+len(c.namespace)) + buf = hex.AppendEncode(buf, hashed[:]) + buf = append(buf, '/') + buf = append(buf, c.namespace...) + return toString(buf) // only safe because buf is heap allocated via make +} + +// toString performs unholy acts to avoid allocations +func toString(b []byte) string { + // unsafe.SliceData relies on cap whereas we want to rely on len + if len(b) == 0 { + return "" + } + // Copied from go 1.20.1 strings.Builder.String + // https://github.com/golang/go/blob/202a1a57064127c3f19d96df57b9f9586145e21c/src/strings/builder.go#L48 + return unsafe.String(unsafe.SliceData(b), len(b)) } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/impersonation/constrained_impersonation_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/impersonation/constrained_impersonation_test.go index f5eeec20da8..80e771daefe 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/impersonation/constrained_impersonation_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/impersonation/constrained_impersonation_test.go @@ -21,6 +21,7 @@ import ( "crypto/sha256" "encoding/json" "fmt" + "hash/fnv" "io" "net/http" "net/http/httptest" @@ -470,6 +471,13 @@ func usernameHash(username string) string { return fmt.Sprintf("%x", hash) } +func fnvSum128a(data []byte) []byte { + h := fnv.New128a() + h.Write(data) + var sum [16]byte + return h.Sum(sum[:0]) +} + func (c *constrainedImpersonationTest) checkCacheEntry(cache *impersonationCache, wantedUser *user.DefaultInfo, attributes authorizer.Attributes, rawKey string, expectedUser *user.DefaultInfo, expectedVerb string) { rr := require.New(c.t) keyString, err := buildKey(wantedUser, attributes)