Merge pull request #130688 from serathius/watchcache-validate

Use ValidateListOptions in watch cache

Kubernetes-commit: abaa73bb76855de032920831112409920ceda408

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)

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)
+		}
+	}
+}