client-go/util/consistencydetector: improve validation of list parameters (RV, ListOptions)
This commit is contained in:
		@@ -41,6 +41,10 @@ type ListFunc[T runtime.Object] func(ctx context.Context, options metav1.ListOpt
 | 
				
			|||||||
// we cannot manipulate the environmental variable because
 | 
					// we cannot manipulate the environmental variable because
 | 
				
			||||||
// it will affect other tests in this package.
 | 
					// 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]) {
 | 
					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)
 | 
						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.ResourceVersion = lastSyncedResourceVersion
 | 
				
			||||||
	listOptions.ResourceVersionMatch = metav1.ResourceVersionMatchExact
 | 
						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
 | 
					type byUID []metav1.Object
 | 
				
			||||||
 | 
					
 | 
				
			||||||
func (a byUID) Len() int           { return len(a) }
 | 
					func (a byUID) Len() int           { return len(a) }
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -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",
 | 
								name: "data consistency panics when data is inconsistent",
 | 
				
			||||||
			listResponse: &v1.PodList{
 | 
								listResponse: &v1.PodList{
 | 
				
			||||||
@@ -99,6 +115,9 @@ func TestDataConsistencyChecker(t *testing.T) {
 | 
				
			|||||||
	for _, scenario := range scenarios {
 | 
						for _, scenario := range scenarios {
 | 
				
			||||||
		t.Run(scenario.name, func(t *testing.T) {
 | 
							t.Run(scenario.name, func(t *testing.T) {
 | 
				
			||||||
			ctx := context.TODO()
 | 
								ctx := context.TODO()
 | 
				
			||||||
 | 
								if scenario.listResponse == nil {
 | 
				
			||||||
 | 
									scenario.listResponse = &v1.PodList{}
 | 
				
			||||||
 | 
								}
 | 
				
			||||||
			fakeLister := &listWrapper{response: scenario.listResponse}
 | 
								fakeLister := &listWrapper{response: scenario.listResponse}
 | 
				
			||||||
			retrievedItemsFunc := func() []*v1.Pod {
 | 
								retrievedItemsFunc := func() []*v1.Pod {
 | 
				
			||||||
				return scenario.retrievedItems
 | 
									return scenario.retrievedItems
 | 
				
			||||||
 
 | 
				
			|||||||
		Reference in New Issue
	
	Block a user