From 9e7c080b863896ffbe9eff2a7edc63aa72ec30cf Mon Sep 17 00:00:00 2001 From: Marek Siarkowicz Date: Mon, 10 Mar 2025 14:29:24 +0100 Subject: [PATCH] Use ValidateListOptions in watch cache --- .../apiserver/pkg/storage/cacher/delegator.go | 4 ++ .../pkg/storage/cacher/delegator_test.go | 55 +++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/staging/src/k8s.io/apiserver/pkg/storage/cacher/delegator.go b/staging/src/k8s.io/apiserver/pkg/storage/cacher/delegator.go index 50a902fecb7..dd2388bc13f 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/cacher/delegator.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/cacher/delegator.go @@ -176,6 +176,10 @@ func (c *CacheDelegator) Get(ctx context.Context, key string, opts storage.GetOp } func (c *CacheDelegator) GetList(ctx context.Context, key string, opts storage.ListOptions, listObj runtime.Object) error { + _, _, err := storage.ValidateListOptions(c.cacher.resourcePrefix, c.cacher.versioner, opts) + if err != nil { + return err + } shouldDelegate, consistentRead := shouldDelegateList(opts) if shouldDelegate { return c.storage.GetList(ctx, key, opts, listObj) diff --git a/staging/src/k8s.io/apiserver/pkg/storage/cacher/delegator_test.go b/staging/src/k8s.io/apiserver/pkg/storage/cacher/delegator_test.go index f812e169f54..cea88f556b3 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/cacher/delegator_test.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/cacher/delegator_test.go @@ -20,6 +20,8 @@ import ( "context" "testing" + "k8s.io/apimachinery/pkg/apis/meta/internalversion" + "k8s.io/apimachinery/pkg/apis/meta/internalversion/validation" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apiserver/pkg/apis/example" @@ -192,3 +194,56 @@ func TestCalculateDigest(t *testing.T) { }) } } + +func TestValidateUndelegatedListOptions(t *testing.T) { + opts := []storage.ListOptions{} + keyPrefix := "/pods/" + continueOnRV1, err := storage.EncodeContinue("/pods/a", keyPrefix, 1) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + continueOnNegativeRV, err := storage.EncodeContinue("/pods/a", keyPrefix, -1) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + for _, rv := range []string{"", "0", "1"} { + for _, match := range []metav1.ResourceVersionMatch{"", metav1.ResourceVersionMatchExact, metav1.ResourceVersionMatchNotOlderThan} { + for _, c := range []string{"", continueOnRV1, continueOnNegativeRV} { + for _, limit := range []int64{0, 100} { + for _, recursive := range []bool{true, false} { + opt := storage.ListOptions{ + ResourceVersion: rv, + ResourceVersionMatch: match, + Predicate: storage.SelectionPredicate{ + Limit: limit, + Continue: c, + }, + Recursive: recursive, + } + // Skip requests that will not pass apiserver validation + if errs := validation.ValidateListOptions(&internalversion.ListOptions{ + ResourceVersion: opt.ResourceVersion, + ResourceVersionMatch: opt.ResourceVersionMatch, + Limit: opt.Predicate.Limit, + Continue: opt.Predicate.Continue, + }, false); len(errs) != 0 { + continue + } + // Skip requests sent directly to etcd + if delegateList, _ := shouldDelegateList(opt); delegateList { + continue + } + opts = append(opts, opt) + } + + } + } + } + } + for _, opt := range opts { + _, _, err := storage.ValidateListOptions(keyPrefix, storage.APIObjectVersioner{}, opt) + if err != nil { + t.Errorf("Expected List requests %+v to pass validation, got: %v", opt, err) + } + } +}