Merge pull request kubernetes#125166 from p0lyn0mial/upstream-improve-check-data-consistency

client-go/util/consistencydetector: improve validation of list parameters (RV, ListOptions)

Author: k8s-ci-robot

staging/src/k8s.io/client-go/util/consistencydetector/data_consistency_detector.go

@@ -41,6 +41,10 @@ type ListFunc[T runtime.Object] func(ctx context.Context, options metav1.ListOpt
 // we cannot manipulate the environmental variable because
 // it will affect other tests in this package.
 func CheckDataConsistency[T runtime.Object, U any](ctx context.Context, identity string, lastSyncedResourceVersion string, listFn ListFunc[T], listOptions metav1.ListOptions, retrieveItemsFn RetrieveItemsFunc[U]) {
+	if !canFormAdditionalListCall(lastSyncedResourceVersion, listOptions) {
+		klog.V(4).Infof("data consistency check for %s is enabled but the parameters (RV, ListOptions) doesn't allow for creating a valid LIST request. Skipping the data consistency check.", identity)
+		return
+	}
 	klog.Warningf("data consistency check for %s is enabled, this will result in an additional call to the API server.", identity)
 	listOptions.ResourceVersion = lastSyncedResourceVersion
 	listOptions.ResourceVersionMatch = metav1.ResourceVersionMatchExact
@@ -79,6 +83,26 @@ func CheckDataConsistency[T runtime.Object, U any](ctx context.Context, identity
 	}
 }
 
+// canFormAdditionalListCall ensures that we can form a valid LIST requests
+// for checking data consistency.
+func canFormAdditionalListCall(resourceVersion string, options metav1.ListOptions) bool {
+	// since we are setting ResourceVersionMatch to metav1.ResourceVersionMatchExact
+	// we need to make sure that the continuation hasn't been set
+	// https://github.com/kubernetes/kubernetes/blob/be4afb9ef90b19ccb6f7e595cbdb247e088b2347/staging/src/k8s.io/apimachinery/pkg/apis/meta/internalversion/validation/validation.go#L38
+	if len(options.Continue) > 0 {
+		return false
+	}
+
+	// since we are setting ResourceVersionMatch to metav1.ResourceVersionMatchExact
+	// we need to make sure that the RV is valid because the validation code forbids RV == "0"
+	// https://github.com/kubernetes/kubernetes/blob/be4afb9ef90b19ccb6f7e595cbdb247e088b2347/staging/src/k8s.io/apimachinery/pkg/apis/meta/internalversion/validation/validation.go#L44
+	if resourceVersion == "0" {
+		return false
+	}
+
+	return true
+}
+
 type byUID []metav1.Object
 
 func (a byUID) Len() int           { return len(a) }

staging/src/k8s.io/client-go/util/consistencydetector/data_consistency_detector_test.go

@@ -76,6 +76,22 @@ func TestDataConsistencyChecker(t *testing.T) {
 			},
 		},
 
+		{
+			name:                 "data consistency check won't be performed when Continuation was set",
+			requestOptions:       metav1.ListOptions{Continue: "fake continuation token"},
+			expectedListRequests: 0,
+		},
+
+		{
+			name: "data consistency check won't be performed when ResourceVersion was set to 0",
+			listResponse: &v1.PodList{
+				ListMeta: metav1.ListMeta{ResourceVersion: "0"},
+				Items:    []v1.Pod{*makePod("p1", "1"), *makePod("p2", "2")},
+			},
+			requestOptions:       metav1.ListOptions{},
+			expectedListRequests: 0,
+		},
+
 		{
 			name: "data consistency panics when data is inconsistent",
 			listResponse: &v1.PodList{
@@ -99,6 +115,9 @@ func TestDataConsistencyChecker(t *testing.T) {
 	for _, scenario := range scenarios {
 		t.Run(scenario.name, func(t *testing.T) {
 			ctx := context.TODO()
+			if scenario.listResponse == nil {
+				scenario.listResponse = &v1.PodList{}
+			}
 			fakeLister := &listWrapper{response: scenario.listResponse}
 			retrievedItemsFunc := func() []*v1.Pod {
 				return scenario.retrievedItems