Merge pull request #125584 from serathius/list-recursive-prefix

Fix TestGetListRecursivePrefix for all types of LIST

staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"net/http"
 	"reflect"
+	"strings"
 	"sync"
 	"time"
 
@@ -818,6 +819,14 @@ func (c *Cacher) GetList(ctx context.Context, key string, opts storage.ListOptio
 			return c.storage.GetList(ctx, key, opts, listObj)
 		}
 	}
+	// For recursive lists, we need to make sure the key ended with "/" so that we only
+	// get children "directories". e.g. if we have key "/a", "/a/b", "/ab", getting keys
+	// with prefix "/a" will return all three, while with prefix "/a/" will return only
+	// "/a/b" which is the correct answer.
+	preparedKey := key
+	if opts.Recursive && !strings.HasSuffix(key, "/") {
+		preparedKey += "/"
+	}
 	requestWatchProgressSupported := etcdfeature.DefaultFeatureSupportChecker.Supports(storage.RequestWatchProgress)
 	if resourceVersion == "" && utilfeature.DefaultFeatureGate.Enabled(features.ConsistentListFromCache) && requestWatchProgressSupported {
 		listRV, err = storage.GetCurrentResourceVersionFromStorage(ctx, c.storage, c.newListFunc, c.resourcePrefix, c.objectType.String())
@@ -856,9 +865,9 @@ func (c *Cacher) GetList(ctx context.Context, key string, opts storage.ListOptio
 	if listVal.Kind() != reflect.Slice {
 		return fmt.Errorf("need a pointer to slice, got %v", listVal.Kind())
 	}
-	filter := filterWithAttrsFunction(key, pred)
+	filter := filterWithAttrsFunction(preparedKey, pred)
 
-	objs, readResourceVersion, indexUsed, err := c.listItems(ctx, listRV, key, pred, recursive)
+	objs, readResourceVersion, indexUsed, err := c.listItems(ctx, listRV, preparedKey, pred, recursive)
 	if err != nil {
 		return err
 	}

staging/src/k8s.io/apiserver/pkg/storage/testing/store_tests.go

@@ -1578,6 +1578,7 @@ func RunTestGetListRecursivePrefix(ctx context.Context, t *testing.T, store stor
 	fooKey, fooObj := testPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "test-ns"}})
 	fooBarKey, fooBarObj := testPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foobar", Namespace: "test-ns"}})
 	_, otherNamespaceObj := testPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "baz", Namespace: "test-ns2"}})
+	lastRev := otherNamespaceObj.ResourceVersion
 
 	tests := []struct {
 		name        string
@@ -1635,19 +1636,47 @@ func RunTestGetListRecursivePrefix(ctx context.Context, t *testing.T, store stor
 		},
 	}
 
-	for _, tt := range tests {
+	listTypes := []struct {
-		tt := tt
+		name            string
-		t.Run(tt.name, func(t *testing.T) {
+		ResourceVersion string
-			out := &example.PodList{}
+		Match           metav1.ResourceVersionMatch
-			storageOpts := storage.ListOptions{
+	}{
-				Recursive: tt.recursive,
+		{
-				Predicate: storage.Everything,
+			name:            "Exact",
+			ResourceVersion: lastRev,
+			Match:           metav1.ResourceVersionMatchExact,
+		},
+		{
+			name:            "Consistent",
+			ResourceVersion: "",
+		},
+		{
+			name:            "NotOlderThan",
+			ResourceVersion: "0",
+			Match:           metav1.ResourceVersionMatchNotOlderThan,
+		},
+	}
+
+	for _, listType := range listTypes {
+		listType := listType
+		t.Run(listType.name, func(t *testing.T) {
+			for _, tt := range tests {
+				tt := tt
+				t.Run(tt.name, func(t *testing.T) {
+					out := &example.PodList{}
+					storageOpts := storage.ListOptions{
+						ResourceVersion:      listType.ResourceVersion,
+						ResourceVersionMatch: listType.Match,
+						Recursive:            tt.recursive,
+						Predicate:            storage.Everything,
+					}
+					err := store.GetList(ctx, tt.key, storageOpts, out)
+					if err != nil {
+						t.Fatalf("GetList failed: %v", err)
+					}
+					expectNoDiff(t, "incorrect list pods", tt.expectedOut, out.Items)
+				})
 			}
-			err := store.GetList(ctx, tt.key, storageOpts, out)
-			if err != nil {
-				t.Fatalf("GetList failed: %v", err)
-			}
-			expectNoDiff(t, "incorrect list pods", tt.expectedOut, out.Items)
 		})
 	}
 }