Merge pull request #107559 from liggitt/invalid-selectors

Handle invalid selectors properly

pkg/controller/deployment/util/deployment_util.go

@@ -900,7 +900,8 @@ func GetDeploymentsForReplicaSet(deploymentLister appslisters.DeploymentLister,
 	for _, d := range dList {
 		selector, err := metav1.LabelSelectorAsSelector(d.Spec.Selector)
 		if err != nil {
-			return nil, fmt.Errorf("invalid label selector: %v", err)
+			// This object has an invalid selector, it does not match the replicaset
+			continue
 		}
 		// If a deployment with a nil or empty selector creeps in, it should match nothing, not everything.
 		if selector.Empty() || !selector.Matches(labels.Set(rs.Labels)) {

pkg/controller/replicaset/replica_set.go

@@ -668,7 +668,7 @@ func (rsc *ReplicaSetController) syncReplicaSet(ctx context.Context, key string)
 	rsNeedsSync := rsc.expectations.SatisfiedExpectations(key)
 	selector, err := metav1.LabelSelectorAsSelector(rs.Spec.Selector)
 	if err != nil {
-		utilruntime.HandleError(fmt.Errorf("error converting pod selector to selector: %v", err))
+		utilruntime.HandleError(fmt.Errorf("error converting pod selector to selector for rs %v/%v: %v", namespace, name, err))
 		return nil
 	}
 
@@ -774,7 +774,8 @@ func (rsc *ReplicaSetController) getIndirectlyRelatedPods(rs *apps.ReplicaSet) (
 	for _, relatedRS := range rsc.getReplicaSetsWithSameController(rs) {
 		selector, err := metav1.LabelSelectorAsSelector(relatedRS.Spec.Selector)
 		if err != nil {
-			return nil, err
+			// This object has an invalid selector, it does not match any pods
+			continue
 		}
 		pods, err := rsc.podLister.Pods(relatedRS.Namespace).List(selector)
 		if err != nil {

pkg/controller/volume/persistentvolume/util/util.go

@@ -197,7 +197,6 @@ func FindMatchingVolume(
 	if claim.Spec.Selector != nil {
 		internalSelector, err := metav1.LabelSelectorAsSelector(claim.Spec.Selector)
 		if err != nil {
-			// should be unreachable code due to validation
 			return nil, fmt.Errorf("error creating internal label selector for claim: %v: %v", claimToClaimKey(claim), err)
 		}
 		selector = internalSelector

pkg/printers/internalversion/printers.go

@@ -1996,14 +1996,16 @@ func printDeployment(obj *apps.Deployment, options printers.GenerateOptions) ([]
 	age := translateTimestampSince(obj.CreationTimestamp)
 	containers := obj.Spec.Template.Spec.Containers
 	selector, err := metav1.LabelSelectorAsSelector(obj.Spec.Selector)
+	selectorString := ""
 	if err != nil {
-		// this shouldn't happen if LabelSelector passed validation
+		selectorString = "<invalid>"
-		return nil, err
+	} else {
+		selectorString = selector.String()
 	}
 	row.Cells = append(row.Cells, obj.Name, fmt.Sprintf("%d/%d", int64(readyReplicas), int64(desiredReplicas)), int64(updatedReplicas), int64(availableReplicas), age)
 	if options.Wide {
 		containers, images := layoutContainerCells(containers)
-		row.Cells = append(row.Cells, containers, images, selector.String())
+		row.Cells = append(row.Cells, containers, images, selectorString)
 	}
 	return []metav1.TableRow{row}, nil
 }

pkg/registry/core/pod/storage/eviction.go

@@ -385,6 +385,7 @@ func (r *EvictionREST) getPodDisruptionBudgets(ctx context.Context, pod *api.Pod
 		}
 		selector, err := metav1.LabelSelectorAsSelector(pdb.Spec.Selector)
 		if err != nil {
+			// This object has an invalid selector, it does not match the pod
 			continue
 		}
 		// If a PDB with a nil or empty selector creeps in, it should match nothing, not everything.

pkg/scheduler/framework/fake/listers.go

@@ -125,7 +125,8 @@ func (f ReplicaSetLister) GetPodReplicaSets(pod *v1.Pod) (rss []*appsv1.ReplicaS
 		}
 		selector, err = metav1.LabelSelectorAsSelector(rs.Spec.Selector)
 		if err != nil {
-			return
+			// This object has an invalid selector, it does not match the pod
+			continue
 		}
 
 		if selector.Matches(labels.Set(pod.Labels)) {
@@ -164,7 +165,8 @@ func (f StatefulSetLister) GetPodStatefulSets(pod *v1.Pod) (sss []*appsv1.Statef
 		}
 		selector, err = metav1.LabelSelectorAsSelector(ss.Spec.Selector)
 		if err != nil {
-			return
+			// This object has an invalid selector, it does not match the pod
+			continue
 		}
 		if selector.Matches(labels.Set(pod.Labels)) {
 			sss = append(sss, ss)

pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go

@@ -277,6 +277,7 @@ func filterPodsWithPDBViolation(podInfos []*framework.PodInfo, pdbs []*policy.Po
 				}
 				selector, err := metav1.LabelSelectorAsSelector(pdb.Spec.Selector)
 				if err != nil {
+					// This object has an invalid selector, it does not match the pod
 					continue
 				}
 				// A PDB with a nil or empty selector matches nothing.

pkg/scheduler/framework/plugins/volumebinding/binder.go

@@ -990,7 +990,6 @@ func (b *volumeBinder) nodeHasAccess(node *v1.Node, capacity *storagev1beta1.CSI
 	// Only matching by label is supported.
 	selector, err := metav1.LabelSelectorAsSelector(capacity.NodeTopology)
 	if err != nil {
-		// This should never happen because NodeTopology must be valid.
 		klog.ErrorS(err, "Unexpected error converting to a label selector", "nodeTopology", capacity.NodeTopology)
 		return false
 	}

staging/src/k8s.io/client-go/listers/apps/v1/daemonset_expansion.go

@@ -60,8 +60,8 @@ func (s *daemonSetLister) GetPodDaemonSets(pod *v1.Pod) ([]*apps.DaemonSet, erro
 		}
 		selector, err = metav1.LabelSelectorAsSelector(daemonSet.Spec.Selector)
 		if err != nil {
-			// this should not happen if the DaemonSet passed validation
+			// This object has an invalid selector, it does not match the pod
-			return nil, err
+			continue
 		}
 
 		// If a daemonSet with a nil or empty selector creeps in, it should match nothing, not everything.
@@ -96,7 +96,8 @@ func (s *daemonSetLister) GetHistoryDaemonSets(history *apps.ControllerRevision)
 	for _, ds := range list {
 		selector, err := metav1.LabelSelectorAsSelector(ds.Spec.Selector)
 		if err != nil {
-			return nil, fmt.Errorf("invalid label selector: %v", err)
+			// This object has an invalid selector, it does not match the history
+			continue
 		}
 		// If a DaemonSet with a nil or empty selector creeps in, it should match nothing, not everything.
 		if selector.Empty() || !selector.Matches(labels.Set(history.Labels)) {

staging/src/k8s.io/client-go/listers/apps/v1/replicaset_expansion.go

@@ -55,7 +55,8 @@ func (s *replicaSetLister) GetPodReplicaSets(pod *v1.Pod) ([]*apps.ReplicaSet, e
 		}
 		selector, err := metav1.LabelSelectorAsSelector(rs.Spec.Selector)
 		if err != nil {
-			return nil, fmt.Errorf("invalid selector: %v", err)
+			// This object has an invalid selector, it does not match the pod
+			continue
 		}
 
 		// If a ReplicaSet with a nil or empty selector creeps in, it should match nothing, not everything.

staging/src/k8s.io/client-go/listers/apps/v1/statefulset_expansion.go

@@ -59,7 +59,8 @@ func (s *statefulSetLister) GetPodStatefulSets(pod *v1.Pod) ([]*apps.StatefulSet
 		}
 		selector, err = metav1.LabelSelectorAsSelector(ps.Spec.Selector)
 		if err != nil {
-			return nil, fmt.Errorf("invalid selector: %v", err)
+			// This object has an invalid selector, it does not match the pod
+			continue
 		}
 
 		// If a StatefulSet with a nil or empty selector creeps in, it should match nothing, not everything.

staging/src/k8s.io/client-go/listers/apps/v1beta1/statefulset_expansion.go

@@ -59,7 +59,8 @@ func (s *statefulSetLister) GetPodStatefulSets(pod *v1.Pod) ([]*apps.StatefulSet
 		}
 		selector, err = metav1.LabelSelectorAsSelector(ps.Spec.Selector)
 		if err != nil {
-			return nil, fmt.Errorf("invalid selector: %v", err)
+			// This object has an invalid selector, it does not match the pod
+			continue
 		}
 
 		// If a StatefulSet with a nil or empty selector creeps in, it should match nothing, not everything.

staging/src/k8s.io/client-go/listers/apps/v1beta2/daemonset_expansion.go

@@ -60,8 +60,8 @@ func (s *daemonSetLister) GetPodDaemonSets(pod *v1.Pod) ([]*apps.DaemonSet, erro
 		}
 		selector, err = metav1.LabelSelectorAsSelector(daemonSet.Spec.Selector)
 		if err != nil {
-			// this should not happen if the DaemonSet passed validation
+			// This object has an invalid selector, it does not match the pod
-			return nil, err
+			continue
 		}
 
 		// If a daemonSet with a nil or empty selector creeps in, it should match nothing, not everything.
@@ -96,7 +96,8 @@ func (s *daemonSetLister) GetHistoryDaemonSets(history *apps.ControllerRevision)
 	for _, ds := range list {
 		selector, err := metav1.LabelSelectorAsSelector(ds.Spec.Selector)
 		if err != nil {
-			return nil, fmt.Errorf("invalid label selector: %v", err)
+			// This object has an invalid selector, it does not match the history object
+			continue
 		}
 		// If a DaemonSet with a nil or empty selector creeps in, it should match nothing, not everything.
 		if selector.Empty() || !selector.Matches(labels.Set(history.Labels)) {

staging/src/k8s.io/client-go/listers/apps/v1beta2/replicaset_expansion.go

@@ -55,7 +55,8 @@ func (s *replicaSetLister) GetPodReplicaSets(pod *v1.Pod) ([]*apps.ReplicaSet, e
 		}
 		selector, err := metav1.LabelSelectorAsSelector(rs.Spec.Selector)
 		if err != nil {
-			return nil, fmt.Errorf("invalid selector: %v", err)
+			// This object has an invalid selector, it does not match the pod
+			continue
 		}
 
 		// If a ReplicaSet with a nil or empty selector creeps in, it should match nothing, not everything.

staging/src/k8s.io/client-go/listers/apps/v1beta2/statefulset_expansion.go

@@ -59,7 +59,8 @@ func (s *statefulSetLister) GetPodStatefulSets(pod *v1.Pod) ([]*apps.StatefulSet
 		}
 		selector, err = metav1.LabelSelectorAsSelector(ps.Spec.Selector)
 		if err != nil {
-			return nil, fmt.Errorf("invalid selector: %v", err)
+			// This object has an invalid selector, it does not match the pod
+			continue
 		}
 
 		// If a StatefulSet with a nil or empty selector creeps in, it should match nothing, not everything.

staging/src/k8s.io/client-go/listers/batch/v1/job_expansion.go

@@ -51,7 +51,11 @@ func (l *jobLister) GetPodJobs(pod *v1.Pod) (jobs []batch.Job, err error) {
 		return
 	}
 	for _, job := range list {
-		selector, _ := metav1.LabelSelectorAsSelector(job.Spec.Selector)
+		selector, err := metav1.LabelSelectorAsSelector(job.Spec.Selector)
+		if err != nil {
+			// This object has an invalid selector, it does not match the pod
+			continue
+		}
 		if !selector.Matches(labels.Set(pod.Labels)) {
 			continue
 		}

staging/src/k8s.io/client-go/listers/extensions/v1beta1/daemonset_expansion.go

@@ -61,8 +61,8 @@ func (s *daemonSetLister) GetPodDaemonSets(pod *v1.Pod) ([]*v1beta1.DaemonSet, e
 		}
 		selector, err = metav1.LabelSelectorAsSelector(daemonSet.Spec.Selector)
 		if err != nil {
-			// this should not happen if the DaemonSet passed validation
+			// This object has an invalid selector, it does not match the pod
-			return nil, err
+			continue
 		}
 
 		// If a daemonSet with a nil or empty selector creeps in, it should match nothing, not everything.
@@ -97,7 +97,8 @@ func (s *daemonSetLister) GetHistoryDaemonSets(history *apps.ControllerRevision)
 	for _, ds := range list {
 		selector, err := metav1.LabelSelectorAsSelector(ds.Spec.Selector)
 		if err != nil {
-			return nil, fmt.Errorf("invalid label selector: %v", err)
+			// This object has an invalid selector, it does not match the history object
+			continue
 		}
 		// If a DaemonSet with a nil or empty selector creeps in, it should match nothing, not everything.
 		if selector.Empty() || !selector.Matches(labels.Set(history.Labels)) {

staging/src/k8s.io/client-go/listers/extensions/v1beta1/replicaset_expansion.go

@@ -55,7 +55,8 @@ func (s *replicaSetLister) GetPodReplicaSets(pod *v1.Pod) ([]*extensions.Replica
 		}
 		selector, err := metav1.LabelSelectorAsSelector(rs.Spec.Selector)
 		if err != nil {
-			return nil, fmt.Errorf("invalid selector: %v", err)
+			// This object has an invalid selector, it does not match the pod
+			continue
 		}
 
 		// If a ReplicaSet with a nil or empty selector creeps in, it should match nothing, not everything.

staging/src/k8s.io/client-go/listers/policy/v1/poddisruptionbudget_expansion.go

@@ -23,7 +23,6 @@ import (
 	policy "k8s.io/api/policy/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
-	"k8s.io/klog/v2"
 )
 
 // PodDisruptionBudgetListerExpansion allows custom methods to be added to
@@ -50,7 +49,7 @@ func (s *podDisruptionBudgetLister) GetPodPodDisruptionBudgets(pod *v1.Pod) ([]*
 		pdb := list[i]
 		selector, err = metav1.LabelSelectorAsSelector(pdb.Spec.Selector)
 		if err != nil {
-			klog.Warningf("invalid selector: %v", err)
+			// This object has an invalid selector, it does not match the pod
 			continue
 		}
 

staging/src/k8s.io/client-go/listers/policy/v1beta1/poddisruptionbudget_expansion.go

@@ -23,7 +23,6 @@ import (
 	policy "k8s.io/api/policy/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
-	"k8s.io/klog/v2"
 )
 
 // PodDisruptionBudgetListerExpansion allows custom methods to be added to
@@ -50,8 +49,7 @@ func (s *podDisruptionBudgetLister) GetPodPodDisruptionBudgets(pod *v1.Pod) ([]*
 		pdb := list[i]
 		selector, err = metav1.LabelSelectorAsSelector(pdb.Spec.Selector)
 		if err != nil {
-			klog.Warningf("invalid selector: %v", err)
+			// This object has an invalid selector, it does not match the pod
-			// TODO(mml): add an event to the PDB
 			continue
 		}