Merge pull request #795 from alecmerdler/OLM-974

Garbage Collection for OperatorGroup RBAC

Author: openshift-merge-robot

pkg/controller/operators/olm/operator.go

@@ -299,7 +299,7 @@ func NewOperator(logger *logrus.Logger, crClient versioned.Interface, opClient o
 		// Register queue and QueueInformer
 		queueName := fmt.Sprintf("%s/operatorgroups", namespace)
 		operatorGroupQueue := workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), queueName)
-		operatorGroupQueueInformer := queueinformer.NewInformer(operatorGroupQueue, operatorGroupInformer.Informer(), op.syncOperatorGroups, nil, queueName, metrics.NewMetricsNil(), logger)
+		operatorGroupQueueInformer := queueinformer.NewInformer(operatorGroupQueue, operatorGroupInformer.Informer(), op.syncOperatorGroups, &cache.ResourceEventHandlerFuncs{DeleteFunc: op.operatorGroupDeleted}, queueName, metrics.NewMetricsNil(), logger)
 		op.RegisterQueueInformer(operatorGroupQueueInformer)
 		op.ogQueueSet.Set(namespace, operatorGroupQueue)
 	}

pkg/controller/operators/olm/operator_test.go

@@ -3935,6 +3935,10 @@ func TestSyncOperatorGroups(t *testing.T) {
 					withAnnotations(operatorCSVFinal.DeepCopy(), map[string]string{v1.OperatorGroupTargetsAnnotationKey: "", v1.OperatorGroupAnnotationKey: "operator-group-1", v1.OperatorGroupNamespaceAnnotationKey: operatorNamespace}),
 					annotatedGlobalDeployment,
 					&v1.OperatorGroup{
+						TypeMeta: metav1.TypeMeta{
+							Kind:       v1.OperatorGroupKind,
+							APIVersion: strings.Join([]string{v1.GroupName, v1.GroupVersion}, "/"),
+						},
 						ObjectMeta: metav1.ObjectMeta{
 							Name:      "operator-group-1",
 							Namespace: operatorNamespace,

pkg/controller/operators/olm/operatorgroup.go

@@ -117,6 +117,31 @@ func (a *Operator) syncOperatorGroups(obj interface{}) error {
 	return nil
 }
 
+func (a *Operator) operatorGroupDeleted(obj interface{}) {
+	op, ok := obj.(*v1.OperatorGroup)
+	if !ok {
+		a.Log.Debugf("casting OperatorGroup failed, wrong type: %#v\n", obj)
+		return
+	}
+
+	logger := a.Log.WithFields(logrus.Fields{
+		"operatorGroup": op.GetName(),
+		"namespace":     op.GetNamespace(),
+	})
+
+	clusterRoles, err := a.lister.RbacV1().ClusterRoleLister().List(labels.SelectorFromSet(ownerutil.OwnerLabel(op, "OperatorGroup")))
+	if err != nil {
+		logger.WithError(err).Error("failed to list ClusterRoles for garbage collection")
+		return
+	}
+	for _, clusterRole := range clusterRoles {
+		err = a.OpClient.KubernetesInterface().RbacV1().ClusterRoles().Delete(clusterRole.GetName(), &metav1.DeleteOptions{})
+		if err != nil {
+			logger.WithError(err).Error("failed to delete ClusterRole during garbage collection")
+		}
+	}
+}
+
 func (a *Operator) annotateCSVs(group *v1.OperatorGroup, targetNamespaces []string, logger *logrus.Entry) error {
 	updateErrs := []error{}
 	targetNamespaceSet := resolver.NewNamespaceSet(targetNamespaces)
@@ -222,6 +247,10 @@ func (a *Operator) ensureProvidedAPIClusterRole(operatorGroup *v1.OperatorGroup,
 		},
 		Rules: []rbacv1.PolicyRule{{Verbs: verbs, APIGroups: []string{group}, Resources: []string{resource}, ResourceNames: resourceNames}},
 	}
+	err := ownerutil.AddOwnerLabels(clusterRole, operatorGroup)
+	if err != nil {
+		return err
+	}
 	existingCR, err := a.OpClient.KubernetesInterface().RbacV1().ClusterRoles().Create(clusterRole)
 	if k8serrors.IsAlreadyExists(err) {
 		if existingCR != nil && reflect.DeepEqual(existingCR.Labels, clusterRole.Labels) && reflect.DeepEqual(existingCR.Rules, clusterRole.Rules) {
@@ -754,7 +783,11 @@ func (a *Operator) ensureOpGroupClusterRole(op *v1.OperatorGroup, suffix string)
 			},
 		},
 	}
-	_, err := a.OpClient.KubernetesInterface().RbacV1().ClusterRoles().Create(clusterRole)
+	err := ownerutil.AddOwnerLabels(clusterRole, op)
+	if err != nil {
+		return err
+	}
+	_, err = a.OpClient.KubernetesInterface().RbacV1().ClusterRoles().Create(clusterRole)
 	if k8serrors.IsAlreadyExists(err) {
 		return nil
 	} else if err != nil {

test/e2e/operator_groups_e2e_test.go

@@ -21,7 +21,7 @@ import (
 
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/ownerutil"
 
-	"github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/operators/v1"
+	v1 "github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/operators/v1"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/operators/v1alpha1"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry"
@@ -139,11 +139,6 @@ func TestOperatorGroup(t *testing.T) {
 	}
 	_, err = crc.OperatorsV1().OperatorGroups(opGroupNamespace).Create(&operatorGroup)
 	require.NoError(t, err)
-	defer func() {
-		err = crc.OperatorsV1().OperatorGroups(opGroupNamespace).Delete(operatorGroup.Name, &metav1.DeleteOptions{})
-		require.NoError(t, err)
-	}()
-
 	expectedOperatorGroupStatus := v1.OperatorGroupStatus{
 		Namespaces: []string{opGroupNamespace, createdOtherNamespace.GetName()},
 	}
@@ -407,6 +402,36 @@ func TestOperatorGroup(t *testing.T) {
 		return err
 	})
 	require.NoError(t, err)
+
+	err = crc.OperatorsV1().OperatorGroups(opGroupNamespace).Delete(operatorGroup.Name, &metav1.DeleteOptions{})
+	require.NoError(t, err)
+	t.Log("Waiting for OperatorGroup RBAC to be garbage collected")
+	err = wait.Poll(pollInterval, pollDuration, func() (bool, error) {
+		_, err := c.KubernetesInterface().RbacV1().ClusterRoles().Get(operatorGroup.Name+"-admin", metav1.GetOptions{})
+		if err == nil {
+			return false, nil
+		}
+		return true, err
+	})
+	require.True(t, errors.IsNotFound(err))
+
+	err = wait.Poll(pollInterval, pollDuration, func() (bool, error) {
+		_, err := c.KubernetesInterface().RbacV1().ClusterRoles().Get(operatorGroup.Name+"-edit", metav1.GetOptions{})
+		if err == nil {
+			return false, nil
+		}
+		return true, err
+	})
+	require.True(t, errors.IsNotFound(err))
+
+	err = wait.Poll(pollInterval, pollDuration, func() (bool, error) {
+		_, err := c.KubernetesInterface().RbacV1().ClusterRoles().Get(operatorGroup.Name+"-view", metav1.GetOptions{})
+		if err == nil {
+			return false, nil
+		}
+		return true, err
+	})
+	require.True(t, errors.IsNotFound(err))
 }
 
 func createProjectAdmin(t *testing.T, c operatorclient.ClientInterface, namespace string) (string, cleanupFunc) {