Ensure keys used in storage and cacher start with resourcePrefix

This commit is contained in:
Marek Siarkowicz
2025-09-11 13:42:19 +02:00
parent f9a55d78c7
commit 6d5aece4d8
6 changed files with 81 additions and 16 deletions

View File

@@ -21,6 +21,7 @@ import (
"fmt"
"net/http"
"reflect"
"strings"
"sync"
"time"
@@ -373,8 +374,18 @@ func NewCacherFromConfig(config Config) (*Cacher, error) {
config.Clock = clock.RealClock{}
}
objType := reflect.TypeOf(obj)
resourcePrefix := config.ResourcePrefix
if resourcePrefix == "" {
return nil, fmt.Errorf("resourcePrefix cannot be empty")
}
if resourcePrefix == "/" {
return nil, fmt.Errorf("resourcePrefix cannot be /")
}
if !strings.HasPrefix(resourcePrefix, "/") {
return nil, fmt.Errorf("resourcePrefix needs to start from /")
}
cacher := &Cacher{
resourcePrefix: config.ResourcePrefix,
resourcePrefix: resourcePrefix,
ready: newReady(config.Clock),
storage: config.Storage,
objectType: objType,
@@ -425,8 +436,8 @@ func NewCacherFromConfig(config Config) (*Cacher, error) {
watchCache := newWatchCache(
config.KeyFunc, cacher.processEvent, config.GetAttrsFunc, config.Versioner, config.Indexers,
config.Clock, eventFreshDuration, config.GroupResource, progressRequester, config.Storage.GetCurrentResourceVersion)
listerWatcher := NewListerWatcher(config.Storage, config.ResourcePrefix, config.NewListFunc, contextMetadata)
reflectorName := "storage/cacher.go:" + config.ResourcePrefix
listerWatcher := NewListerWatcher(config.Storage, resourcePrefix, config.NewListFunc, contextMetadata)
reflectorName := "storage/cacher.go:" + resourcePrefix
reflector := cache.NewNamedReflector(reflectorName, listerWatcher, obj, watchCache, 0)
// Configure reflector's pager to for an appropriate pagination chunk size for fetching data from
@@ -1169,7 +1180,7 @@ func (c *Cacher) Stop() {
}
func (c *Cacher) prepareKey(key string, recursive bool) (string, error) {
return storage.PrepareKey(key, recursive)
return storage.PrepareKey(c.resourcePrefix, key, recursive)
}
func forgetWatcher(c *Cacher, w *cacheWatcher, index int, scope namespacedName, triggerValue string, triggerSupported bool) func(bool) {

View File

@@ -70,7 +70,7 @@ func newEtcdTestStorage(t testing.TB, prefix string) (*etcd3testing.EtcdTestServ
newPod,
newPodList,
prefix,
"/pods",
"/pods/",
schema.GroupResource{Resource: "pods"},
identity.NewEncryptCheckTransformer(),
etcd3.NewDefaultLeaseManagerConfig(),

View File

@@ -157,6 +157,12 @@ func New(c *kubernetes.Client, compactor Compactor, codec runtime.Codec, newFunc
if resourcePrefix == "" {
return nil, fmt.Errorf("resourcePrefix cannot be empty")
}
if resourcePrefix == "/" {
return nil, fmt.Errorf("resourcePrefix cannot be /")
}
if !strings.HasPrefix(resourcePrefix, "/") {
return nil, fmt.Errorf("resourcePrefix needs to start from /")
}
listErrAggrFactory := defaultListErrorAggregatorFactory
if utilfeature.DefaultFeatureGate.Enabled(features.AllowUnsafeMalformedObjectDeletion) {
@@ -1105,7 +1111,7 @@ func (s *store) validateMinimumResourceVersion(minimumResourceVersion string, ac
}
func (s *store) prepareKey(key string, recursive bool) (string, error) {
key, err := storage.PrepareKey(key, recursive)
key, err := storage.PrepareKey(s.resourcePrefix, key, recursive)
if err != nil {
return "", err
}

View File

@@ -662,16 +662,30 @@ func testSetup(t testing.TB, opts ...setupOption) (context.Context, *store, *kub
func TestPrepareKey(t *testing.T) {
validKeys := []string{
"/foo/bar/baz/a.b.c/",
"/foo",
"foo/bar/baz",
"/foo/bar..baz/",
"/foo/bar..",
"foo",
"foo/bar",
"/foo/bar/",
"/pods/foo/bar/baz/a.b.c/",
"/pods/foo",
"/pods/foo/bar/baz",
"/pods/foo/bar..baz/",
"/pods/foo/bar..",
"/pods/foo",
"/pods/foo/bar",
"/pods/foo/bar/",
"/pods/",
}
invalidKeys := []string{
"/pods",
"/pods/foo/bar/../a.b.c/",
"/pods/..",
"/pods/../",
"/pods/foo/bar/..",
"/pods/../foo/bar",
"/pods/../foo",
"/pods/foo/bar/../",
"/pods/.",
"/pods/./",
"/pods/foo/.",
"/pods/./bar",
"/pods/foo/./bar/",
"/foo/bar/../a.b.c/",
"..",
"/..",

View File

@@ -392,7 +392,7 @@ type Stats struct {
EstimatedAverageObjectSizeBytes int64
}
func PrepareKey(key string, recursive bool) (string, error) {
func PrepareKey(resourcePrefix, key string, recursive bool) (string, error) {
if key == ".." ||
strings.HasPrefix(key, "../") ||
strings.HasSuffix(key, "/..") ||
@@ -415,5 +415,8 @@ func PrepareKey(key string, recursive bool) (string, error) {
if recursive && !strings.HasSuffix(key, "/") {
key += "/"
}
if !strings.HasPrefix(key, resourcePrefix) {
return "", fmt.Errorf("invalid key: %q lacks resource prefix: %q", key, resourcePrefix)
}
return key, nil
}

View File

@@ -3785,6 +3785,10 @@ func RunTestKeySchema(ctx context.Context, t *testing.T, store storage.Interface
require.ErrorContains(t, store.Create(ctx, "/", createObj, createOut, 0), "empty key")
require.ErrorContains(t, store.Create(ctx, ".", createObj, createOut, 0), "invalid key")
require.ErrorContains(t, store.Create(ctx, "..", createObj, createOut, 0), "invalid key")
require.ErrorContains(t, store.Create(ctx, "pods", createObj, createOut, 0), "lacks resource prefix")
require.ErrorContains(t, store.Create(ctx, "/pods", createObj, createOut, 0), "lacks resource prefix")
require.ErrorContains(t, store.Create(ctx, "/pods.apps", createObj, createOut, 0), "lacks resource prefix")
require.ErrorContains(t, store.Create(ctx, "/foo/", createObj, createOut, 0), "lacks resource prefix")
require.NoError(t, store.Create(ctx, "/pods/", createObj, createOut, 0))
require.NoError(t, store.Create(ctx, "/pods/name", createObj, createOut, 0))
require.NoError(t, store.Create(ctx, "/pods/namespace", createObj, createOut, 0))
@@ -3792,10 +3796,15 @@ func RunTestKeySchema(ctx context.Context, t *testing.T, store storage.Interface
listOut := &example.PodList{}
recursiveListOpts := storage.ListOptions{Predicate: storage.Everything, Recursive: true}
nonRecursiveListOpts := storage.ListOptions{Predicate: storage.Everything, Recursive: false}
require.ErrorContains(t, store.GetList(ctx, "", recursiveListOpts, listOut), "empty key")
require.ErrorContains(t, store.GetList(ctx, "/", recursiveListOpts, listOut), "empty key")
require.ErrorContains(t, store.GetList(ctx, ".", recursiveListOpts, listOut), "invalid key")
require.ErrorContains(t, store.GetList(ctx, "..", recursiveListOpts, listOut), "invalid key")
require.ErrorContains(t, store.GetList(ctx, "pods", recursiveListOpts, listOut), "lacks resource prefix")
require.ErrorContains(t, store.GetList(ctx, "/pods.apps", recursiveListOpts, listOut), "lacks resource prefix")
require.ErrorContains(t, store.GetList(ctx, "/foo/", recursiveListOpts, listOut), "lacks resource prefix")
require.ErrorContains(t, store.GetList(ctx, "/pods", nonRecursiveListOpts, listOut), "lacks resource prefix")
require.NoError(t, store.GetList(ctx, "/pods", recursiveListOpts, listOut))
require.NoError(t, store.GetList(ctx, "/pods/", recursiveListOpts, listOut))
require.NoError(t, store.GetList(ctx, "/pods/namespace", recursiveListOpts, listOut))
@@ -3807,6 +3816,10 @@ func RunTestKeySchema(ctx context.Context, t *testing.T, store storage.Interface
require.ErrorContains(t, store.Get(ctx, "/", getOpts, getOut), "empty key")
require.ErrorContains(t, store.Get(ctx, ".", getOpts, getOut), "invalid key")
require.ErrorContains(t, store.Get(ctx, "..", getOpts, getOut), "invalid key")
require.ErrorContains(t, store.Get(ctx, "pods", getOpts, getOut), "lacks resource prefix")
require.ErrorContains(t, store.Get(ctx, "/pods", getOpts, getOut), "lacks resource prefix")
require.ErrorContains(t, store.Get(ctx, "/pods.apps", getOpts, getOut), "lacks resource prefix")
require.ErrorContains(t, store.Get(ctx, "/foo/", getOpts, getOut), "lacks resource prefix")
require.NoError(t, store.Get(ctx, "/pods/", getOpts, getOut))
require.NoError(t, store.Get(ctx, "/pods/namespace", getOpts, getOut))
require.NoError(t, store.Get(ctx, "/pods/namespace/name", getOpts, getOut))
@@ -3819,7 +3832,17 @@ func RunTestKeySchema(ctx context.Context, t *testing.T, store storage.Interface
require.ErrorContains(t, err, "invalid key")
_, err = store.Watch(ctx, "..", recursiveListOpts)
require.ErrorContains(t, err, "invalid key")
w, err := store.Watch(ctx, "/pods/", recursiveListOpts)
_, err = store.Watch(ctx, "pods", recursiveListOpts)
require.ErrorContains(t, err, "lacks resource prefix")
_, err = store.Watch(ctx, "/pods.apps", recursiveListOpts)
require.ErrorContains(t, err, "lacks resource prefix")
_, err = store.Watch(ctx, "/foo/", recursiveListOpts)
require.ErrorContains(t, err, "lacks resource prefix")
_, err = store.Watch(ctx, "/pods", nonRecursiveListOpts)
require.ErrorContains(t, err, "lacks resource prefix")
w, err := store.Watch(ctx, "/pods", recursiveListOpts)
require.NoError(t, err)
w.Stop()
require.NoError(t, err)
w.Stop()
w, err = store.Watch(ctx, "/pods/namespace", recursiveListOpts)
@@ -3838,6 +3861,10 @@ func RunTestKeySchema(ctx context.Context, t *testing.T, store storage.Interface
require.ErrorContains(t, store.GuaranteedUpdate(ctx, "/", updateOut, false, nil, updateFunc, nil), "empty key")
require.ErrorContains(t, store.GuaranteedUpdate(ctx, ".", updateOut, false, nil, updateFunc, nil), "invalid key")
require.ErrorContains(t, store.GuaranteedUpdate(ctx, "..", updateOut, false, nil, updateFunc, nil), "invalid key")
require.ErrorContains(t, store.GuaranteedUpdate(ctx, "pods", updateOut, false, nil, updateFunc, nil), "lacks resource prefix")
require.ErrorContains(t, store.GuaranteedUpdate(ctx, "/pods", updateOut, false, nil, updateFunc, nil), "lacks resource prefix")
require.ErrorContains(t, store.GuaranteedUpdate(ctx, "/pods.apps", updateOut, false, nil, updateFunc, nil), "lacks resource prefix")
require.ErrorContains(t, store.GuaranteedUpdate(ctx, "/foo/", updateOut, false, nil, updateFunc, nil), "lacks resource prefix")
require.NoError(t, store.GuaranteedUpdate(ctx, "/pods/", updateOut, false, nil, updateFunc, nil))
require.NoError(t, store.GuaranteedUpdate(ctx, "/pods/namespace", updateOut, false, nil, updateFunc, nil))
require.NoError(t, store.GuaranteedUpdate(ctx, "/pods/namespace/name", updateOut, false, nil, updateFunc, nil))
@@ -3851,6 +3878,10 @@ func RunTestKeySchema(ctx context.Context, t *testing.T, store storage.Interface
require.ErrorContains(t, store.Delete(ctx, "/", deleteOut, nil, deleteFunc, nil, deleteOpts), "empty key")
require.ErrorContains(t, store.Delete(ctx, ".", deleteOut, nil, deleteFunc, nil, deleteOpts), "invalid key")
require.ErrorContains(t, store.Delete(ctx, "..", deleteOut, nil, deleteFunc, nil, deleteOpts), "invalid key")
require.ErrorContains(t, store.Delete(ctx, "pods", deleteOut, nil, deleteFunc, nil, deleteOpts), "lacks resource prefix")
require.ErrorContains(t, store.Delete(ctx, "/pods", deleteOut, nil, deleteFunc, nil, deleteOpts), "lacks resource prefix")
require.ErrorContains(t, store.Delete(ctx, "/pods.apps", deleteOut, nil, deleteFunc, nil, deleteOpts), "lacks resource prefix")
require.ErrorContains(t, store.Delete(ctx, "/foo", deleteOut, nil, deleteFunc, nil, deleteOpts), "lacks resource prefix")
require.NoError(t, store.Delete(ctx, "/pods/", deleteOut, nil, deleteFunc, nil, deleteOpts))
require.NoError(t, store.Delete(ctx, "/pods/namespace", deleteOut, nil, deleteFunc, nil, deleteOpts))
require.NoError(t, store.Delete(ctx, "/pods/namespace/name", deleteOut, nil, deleteFunc, nil, deleteOpts))