Handle invalid label during resource cleanup

Invalid label due to too long CSV name will lead to label selector
malfunction as it returns an empty selector that matches everything.
As a result, some unrelated resources are garbaged-collected when OLM
is doing some resource cleanup during uninstallation. This fix will
use SelectorFromValidatedSet instead of SelectorFromSet to let the
server rejects invalid label sets instead of using am empty Selector.

Signed-off-by: Vu Dinh <[email protected]>

Author: dinhxuanvu

pkg/controller/install/webhook.go

@@ -78,7 +78,7 @@ func (i *StrategyDeploymentInstaller) createOrUpdateWebhook(caPEM []byte, desc v
 func (i *StrategyDeploymentInstaller) createOrUpdateMutatingWebhook(ogNamespacelabelSelector *metav1.LabelSelector, caPEM []byte, desc v1alpha1.WebhookDescription) error {
 	webhookLabels := ownerutil.OwnerLabel(i.owner, i.owner.GetObjectKind().GroupVersionKind().Kind)
 	webhookLabels[WebhookDescKey] = desc.GenerateName
-	webhookSelector := labels.SelectorFromSet(webhookLabels).String()
+	webhookSelector := labels.SelectorFromValidatedSet(webhookLabels).String()
 
 	existingWebhooks, err := i.strategyClient.GetOpClient().KubernetesInterface().AdmissionregistrationV1().MutatingWebhookConfigurations().List(context.TODO(), metav1.ListOptions{LabelSelector: webhookSelector})
 	if err != nil {
@@ -124,7 +124,7 @@ func (i *StrategyDeploymentInstaller) createOrUpdateMutatingWebhook(ogNamespacel
 func (i *StrategyDeploymentInstaller) createOrUpdateValidatingWebhook(ogNamespacelabelSelector *metav1.LabelSelector, caPEM []byte, desc v1alpha1.WebhookDescription) error {
 	webhookLabels := ownerutil.OwnerLabel(i.owner, i.owner.GetObjectKind().GroupVersionKind().Kind)
 	webhookLabels[WebhookDescKey] = desc.GenerateName
-	webhookSelector := labels.SelectorFromSet(webhookLabels).String()
+	webhookSelector := labels.SelectorFromValidatedSet(webhookLabels).String()
 
 	existingWebhooks, err := i.strategyClient.GetOpClient().KubernetesInterface().AdmissionregistrationV1().ValidatingWebhookConfigurations().List(context.TODO(), metav1.ListOptions{LabelSelector: webhookSelector})
 	if err != nil {

pkg/controller/operators/olm/apiservices.go

@@ -264,7 +264,7 @@ func (a *Operator) getCABundle(csv *v1alpha1.ClusterServiceVersion) ([]byte, err
 	for _, desc := range csv.Spec.WebhookDefinitions {
 		webhookLabels := ownerutil.OwnerLabel(csv, v1alpha1.ClusterServiceVersionKind)
 		webhookLabels[install.WebhookDescKey] = desc.GenerateName
-		webhookSelector := labels.SelectorFromSet(webhookLabels).String()
+		webhookSelector := labels.SelectorFromValidatedSet(webhookLabels).String()
 
 		switch desc.Type {
 		case v1alpha1.MutatingAdmissionWebhook:
@@ -493,7 +493,7 @@ func (a *Operator) updateDeploymentSpecsWithApiServiceData(csv *v1alpha1.Cluster
 
 func (a *Operator) cleanUpRemovedWebhooks(csv *v1alpha1.ClusterServiceVersion) error {
 	webhookLabels := ownerutil.OwnerLabel(csv, v1alpha1.ClusterServiceVersionKind)
-	webhookSelector := labels.SelectorFromSet(webhookLabels).String()
+	webhookSelector := labels.SelectorFromValidatedSet(webhookLabels).String()
 
 	csvWebhookGenerateNames := make(map[string]struct{}, len(csv.Spec.WebhookDefinitions))
 	for _, webhook := range csv.Spec.WebhookDefinitions {
@@ -549,7 +549,7 @@ func (a *Operator) areWebhooksAvailable(csv *v1alpha1.ClusterServiceVersion) (bo
 		webhookLabels := ownerutil.OwnerLabel(csv, v1alpha1.ClusterServiceVersionKind)
 		webhookLabels[install.WebhookDescKey] = desc.GenerateName
 		webhookLabels[install.WebhookHashKey] = install.HashWebhookDesc(desc)
-		webhookSelector := labels.SelectorFromSet(webhookLabels).String()
+		webhookSelector := labels.SelectorFromValidatedSet(webhookLabels).String()
 
 		webhookCount := 0
 		switch desc.Type {

pkg/controller/operators/olm/operator.go

@@ -968,7 +968,7 @@ func (a *Operator) handleClusterServiceVersionDeletion(obj interface{}) {
 		}
 	}
 
-	webhookSelector := labels.SelectorFromSet(ownerutil.OwnerLabel(clusterServiceVersion, v1alpha1.ClusterServiceVersionKind)).String()
+	webhookSelector := labels.SelectorFromValidatedSet(ownerutil.OwnerLabel(clusterServiceVersion, v1alpha1.ClusterServiceVersionKind)).String()
 	mWebhooks, err := a.opClient.KubernetesInterface().AdmissionregistrationV1().MutatingWebhookConfigurations().List(context.TODO(), metav1.ListOptions{LabelSelector: webhookSelector})
 	if err != nil {
 		logger.WithError(err).Warn("cannot list MutatingWebhookConfigurations")

pkg/controller/operators/olm/operatorgroup.go

@@ -185,7 +185,7 @@ func (a *Operator) operatorGroupDeleted(obj interface{}) {
 		"namespace":     op.GetNamespace(),
 	})
 
-	clusterRoles, err := a.lister.RbacV1().ClusterRoleLister().List(labels.SelectorFromSet(ownerutil.OwnerLabel(op, "OperatorGroup")))
+	clusterRoles, err := a.lister.RbacV1().ClusterRoleLister().List(labels.SelectorFromValidatedSet(ownerutil.OwnerLabel(op, "OperatorGroup")))
 	if err != nil {
 		logger.WithError(err).Error("failed to list ClusterRoles for garbage collection")
 		return

pkg/controller/registry/reconciler/configmap.go

@@ -208,7 +208,7 @@ func (c *ConfigMapRegistryReconciler) currentRoleBinding(source configMapCatalog
 
 func (c *ConfigMapRegistryReconciler) currentPods(source configMapCatalogSourceDecorator, image string) []*v1.Pod {
 	podName := source.Pod(image).GetName()
-	pods, err := c.Lister.CoreV1().PodLister().Pods(source.GetNamespace()).List(labels.SelectorFromSet(source.Selector()))
+	pods, err := c.Lister.CoreV1().PodLister().Pods(source.GetNamespace()).List(labels.SelectorFromValidatedSet(source.Selector()))
 	if err != nil {
 		logrus.WithField("pod", podName).WithError(err).Debug("couldn't find pod in cache")
 		return nil

pkg/controller/registry/reconciler/configmap_test.go

@@ -459,7 +459,7 @@ func TestConfigMapRegistryReconciler(t *testing.T) {
 			decorated := configMapCatalogSourceDecorator{tt.in.catsrc}
 
 			pod := decorated.Pod(registryImageName)
-			listOptions := metav1.ListOptions{LabelSelector: labels.SelectorFromSet(labels.Set{CatalogSourceLabelKey: tt.in.catsrc.GetName()}).String()}
+			listOptions := metav1.ListOptions{LabelSelector: labels.SelectorFromValidatedSet(labels.Set{CatalogSourceLabelKey: tt.in.catsrc.GetName()}).String()}
 			outPods, err := client.KubernetesInterface().CoreV1().Pods(pod.GetNamespace()).List(context.TODO(), listOptions)
 			require.NoError(t, err)
 			require.Len(t, outPods.Items, 1)

pkg/controller/registry/reconciler/grpc_test.go

@@ -208,7 +208,7 @@ func TestGrpcRegistryReconciler(t *testing.T) {
 			decorated := grpcCatalogSourceDecorator{tt.in.catsrc}
 			pod := decorated.Pod()
 			service := decorated.Service()
-			listOptions := metav1.ListOptions{LabelSelector: labels.SelectorFromSet(labels.Set{CatalogSourceLabelKey: tt.in.catsrc.GetName()}).String()}
+			listOptions := metav1.ListOptions{LabelSelector: labels.SelectorFromValidatedSet(labels.Set{CatalogSourceLabelKey: tt.in.catsrc.GetName()}).String()}
 			outPods, podErr := client.KubernetesInterface().CoreV1().Pods(pod.GetNamespace()).List(context.TODO(), listOptions)
 			outService, serviceErr := client.KubernetesInterface().CoreV1().Services(service.GetNamespace()).Get(context.TODO(), service.GetName(), metav1.GetOptions{})
 

pkg/lib/ownerutil/util.go

@@ -268,7 +268,7 @@ func AdoptableLabels(labels map[string]string, checkName bool, targets ...Owner)
 
 // CSVOwnerSelector returns a label selector to find generated objects owned by owner
 func CSVOwnerSelector(owner *operatorsv1alpha1.ClusterServiceVersion) labels.Selector {
-	return labels.SelectorFromSet(OwnerLabel(owner, operatorsv1alpha1.ClusterServiceVersionKind))
+	return labels.SelectorFromValidatedSet(OwnerLabel(owner, operatorsv1alpha1.ClusterServiceVersionKind))
 }
 
 // AddOwner adds an owner to the ownerref list.

test/e2e/catalog_e2e_test.go

@@ -272,13 +272,13 @@ var _ = Describe("Catalog", func() {
 
 		// Await 1 CatalogSource registry pod matching the updated labels
 		singlePod := podCount(1)
-		selector := labels.SelectorFromSet(map[string]string{"olm.catalogSource": mainCatalogName, "olm.configMapResourceVersion": updatedConfigMap.GetResourceVersion()})
+		selector := labels.SelectorFromValidatedSet(map[string]string{"olm.catalogSource": mainCatalogName, "olm.configMapResourceVersion": updatedConfigMap.GetResourceVersion()})
 		podList, err := awaitPods(GinkgoT(), c, testNamespace, selector.String(), singlePod)
 		require.NoError(GinkgoT(), err)
 		require.Equal(GinkgoT(), 1, len(podList.Items), "expected pod list not of length 1")
 
 		// Await 1 CatalogSource registry pod matching the updated labels
-		selector = labels.SelectorFromSet(map[string]string{"olm.catalogSource": mainCatalogName})
+		selector = labels.SelectorFromValidatedSet(map[string]string{"olm.catalogSource": mainCatalogName})
 		podList, err = awaitPods(GinkgoT(), c, testNamespace, selector.String(), singlePod)
 		require.NoError(GinkgoT(), err)
 		require.Equal(GinkgoT(), 1, len(podList.Items), "expected pod list not of length 1")
@@ -538,7 +538,7 @@ var _ = Describe("Catalog", func() {
 		defer cleanupSource()
 
 		// Wait for a new registry pod to be created
-		selector := labels.SelectorFromSet(map[string]string{"olm.catalogSource": sourceName})
+		selector := labels.SelectorFromValidatedSet(map[string]string{"olm.catalogSource": sourceName})
 		singlePod := podCount(1)
 		registryPods, err := awaitPods(GinkgoT(), c, testNamespace, selector.String(), singlePod)
 		require.NoError(GinkgoT(), err, "error awaiting registry pod")
@@ -602,7 +602,7 @@ var _ = Describe("Catalog", func() {
 
 		// Wait for a new registry pod to be created
 		c := newKubeClient()
-		selector := labels.SelectorFromSet(map[string]string{"olm.catalogSource": source.GetName()})
+		selector := labels.SelectorFromValidatedSet(map[string]string{"olm.catalogSource": source.GetName()})
 		singlePod := podCount(1)
 		registryPods, err := awaitPods(GinkgoT(), c, source.GetNamespace(), selector.String(), singlePod)
 		require.NoError(GinkgoT(), err, "error awaiting registry pod")
@@ -753,7 +753,7 @@ var _ = Describe("Catalog", func() {
 
 		// wait for new catalog source pod to be created
 		// Wait for a new registry pod to be created
-		selector := labels.SelectorFromSet(map[string]string{"olm.catalogSource": source.GetName()})
+		selector := labels.SelectorFromValidatedSet(map[string]string{"olm.catalogSource": source.GetName()})
 		singlePod := podCount(1)
 		registryPods, err := awaitPods(GinkgoT(), c, source.GetNamespace(), selector.String(), singlePod)
 		require.NoError(GinkgoT(), err, "error awaiting registry pod")
@@ -1020,7 +1020,7 @@ var _ = Describe("Catalog", func() {
 		Expect(err).ToNot(HaveOccurred())
 
 		// wait for new catalog source pod to be created and report ready
-		selector := labels.SelectorFromSet(map[string]string{"olm.catalogSource": source.GetName()})
+		selector := labels.SelectorFromValidatedSet(map[string]string{"olm.catalogSource": source.GetName()})
 		singlePod := podCount(1)
 		catalogPods, err := awaitPods(GinkgoT(), c, source.GetNamespace(), selector.String(), singlePod)
 		Expect(err).ToNot(HaveOccurred())
@@ -1043,7 +1043,7 @@ var _ = Describe("Catalog", func() {
 		}).Should(BeTrue())
 
 		// Wait roughly the polling interval for update pod to show up
-		updateSelector := labels.SelectorFromSet(map[string]string{"catalogsource.operators.coreos.com/update": source.GetName()})
+		updateSelector := labels.SelectorFromValidatedSet(map[string]string{"catalogsource.operators.coreos.com/update": source.GetName()})
 		updatePods, err := awaitPodsWithInterval(GinkgoT(), c, source.GetNamespace(), updateSelector.String(), 5*time.Second, 2*time.Minute, singlePod)
 		Expect(err).ToNot(HaveOccurred())
 		Expect(updatePods).ToNot(BeNil())

test/e2e/webhook_e2e_test.go

@@ -900,7 +900,7 @@ var _ = Describe("CSVs with a Webhook", func() {
 })
 
 func getWebhookWithGenerateName(c operatorclient.ClientInterface, generateName string) (*admissionregistrationv1.ValidatingWebhookConfiguration, error) {
-	webhookSelector := labels.SelectorFromSet(map[string]string{install.WebhookDescKey: generateName}).String()
+	webhookSelector := labels.SelectorFromValidatedSet(map[string]string{install.WebhookDescKey: generateName}).String()
 	existingWebhooks, err := c.KubernetesInterface().AdmissionregistrationV1().ValidatingWebhookConfigurations().List(context.TODO(), metav1.ListOptions{LabelSelector: webhookSelector})
 	if err != nil {
 		return nil, err