fix(olm): delete stale resources after op group change

operator-framework · Jan 30, 2019 · 819fb7b · 819fb7b
1 parent a000b6e
commit 819fb7b
Show file tree

Hide file tree

Showing 2 changed files with 73 additions and 13 deletions.
diff --git a/pkg/controller/operators/olm/operator.go b/pkg/controller/operators/olm/operator.go
@@ -424,13 +424,32 @@ func (a *Operator) syncClusterServiceVersion(obj interface{}) (syncError error)
 
 	operatorGroup := a.operatorGroupForActiveCSV(logger, clusterServiceVersion)
 	if operatorGroup != nil {
+		allNamespaces := make([]string, 0)
+		pruneNamespaces := make([]string, 0)
+		targetNamespaces := make([]string, 0)
+		namespaceObjs, err := a.lister.CoreV1().NamespaceLister().List(labels.Everything())
+		if err != nil {
+			return err
+		}
+
+		for _, ns := range namespaceObjs {
+			allNamespaces = append(allNamespaces, ns.GetName())
+		}
+		if len(operatorGroup.Status.Namespaces) == 1 && operatorGroup.Status.Namespaces[0] == corev1.NamespaceAll {
+			targetNamespaces = allNamespaces
+		} else {
+			targetNamespaces = operatorGroup.Status.Namespaces
+			pruneNamespaces = sliceCompare(allNamespaces, targetNamespaces)
+			logger.Debugf("Found namespaces to clean %v", pruneNamespaces)
+		}
+
 		// Check if we need to do any copying / annotation for the operatorgroup
-		if err := a.copyCsvToTargetNamespace(clusterServiceVersion, operatorGroup); err != nil {
+		if err := a.copyCsvToTargetNamespace(clusterServiceVersion, operatorGroup, targetNamespaces, pruneNamespaces); err != nil {
 			logger.WithError(err).Info("couldn't copy CSV to target namespaces")
 		}
 
 		// Ensure operator has access to targetnamespaces
-		if err := a.ensureRBACInTargetNamespace(clusterServiceVersion, operatorGroup); err != nil {
+		if err := a.ensureRBACInTargetNamespace(clusterServiceVersion, operatorGroup, pruneNamespaces); err != nil {
 			logger.WithError(err).Info("couldn't ensure RBAC in target namespaces")
 		}
 

diff --git a/pkg/controller/operators/olm/operatorgroup.go b/pkg/controller/operators/olm/operatorgroup.go
@@ -160,7 +160,7 @@ func (a *Operator) ensureClusterRolesForCSV(csv *v1alpha1.ClusterServiceVersion,
 	return nil
 }
 
-func (a *Operator) ensureRBACInTargetNamespace(csv *v1alpha1.ClusterServiceVersion, operatorGroup *v1alpha2.OperatorGroup) error {
+func (a *Operator) ensureRBACInTargetNamespace(csv *v1alpha1.ClusterServiceVersion, operatorGroup *v1alpha2.OperatorGroup, pruneNamespaces []string) error {
 	opPerms, err := resolver.RBACForClusterServiceVersion(csv)
 	if err != nil {
 		return err
@@ -184,7 +184,7 @@ func (a *Operator) ensureRBACInTargetNamespace(csv *v1alpha1.ClusterServiceVersi
 	// otherwise, create roles/rolebindings for each target namespace
 	for _, ns := range targetNamespaces {
 		for _, p := range opPerms {
-			if err := a.ensureTenantRBAC(operatorGroup.GetNamespace(), ns, csv, *p); err != nil {
+			if err := a.ensureTenantRBAC(operatorGroup.GetNamespace(), ns, csv, *p, pruneNamespaces); err != nil {
 				return err
 			}
 		}
@@ -263,7 +263,7 @@ func (a *Operator) ensureSingletonRBAC(operatorNamespace string, csv *v1alpha1.C
 	return nil
 }
 
-func (a *Operator) ensureTenantRBAC(operatorNamespace, targetNamespace string, csv *v1alpha1.ClusterServiceVersion, permissions resolver.OperatorPermissions) error {
+func (a *Operator) ensureTenantRBAC(operatorNamespace, targetNamespace string, csv *v1alpha1.ClusterServiceVersion, permissions resolver.OperatorPermissions, pruneNamespaces []string) error {
 	ownerSelector := ownerutil.CSVOwnerSelector(csv)
 	ownedRoles, err := a.lister.RbacV1().RoleLister().Roles(operatorNamespace).List(ownerSelector)
 	if err != nil {
@@ -282,6 +282,16 @@ func (a *Operator) ensureTenantRBAC(operatorNamespace, targetNamespace string, c
 				return err
 			}
 		}
+		for _, ns := range pruneNamespaces {
+			_, err := a.lister.RbacV1().RoleLister().Roles(ns).Get(r.GetName())
+			if k8serrors.IsNotFound(err) {
+				continue
+			}
+			a.Log.Debugf("Found role '%v' in namespace %v to delete", r.GetName(), ns)
+			if err := a.OpClient.DeleteRole(ns, r.GetName(), &metav1.DeleteOptions{}); err != nil {
+				return err
+			}
+		}
 		// TODO check rules
 	}
 
@@ -305,22 +315,37 @@ func (a *Operator) ensureTenantRBAC(operatorNamespace, targetNamespace string, c
 			}
 			// TODO check rules
 		}
+		for _, ns := range pruneNamespaces {
+			_, err := a.lister.RbacV1().RoleBindingLister().RoleBindings(ns).Get(r.GetName())
+			if k8serrors.IsNotFound(err) {
+				continue
+			}
+			a.Log.Debugf("Found rolebinding '%v' in namespace %v to delete", r.GetName(), ns)
+			if err := a.OpClient.DeleteRoleBinding(ns, r.GetName(), &metav1.DeleteOptions{}); err != nil {
+				return err
+			}
+		}
 	}
 	return nil
 }
 
-func (a *Operator) copyCsvToTargetNamespace(csv *v1alpha1.ClusterServiceVersion, operatorGroup *v1alpha2.OperatorGroup) error {
-	namespaces := make([]string, 0)
-	if len(operatorGroup.Status.Namespaces) == 1 && operatorGroup.Status.Namespaces[0] == corev1.NamespaceAll {
-		namespaceObjs, err := a.lister.CoreV1().NamespaceLister().List(labels.Everything())
+func (a *Operator) copyCsvToTargetNamespace(csv *v1alpha1.ClusterServiceVersion, operatorGroup *v1alpha2.OperatorGroup, namespaces []string, pruneNamespaces []string) error {
+	// check for stale CSVs from a different previous operator group configuration
+	for _, ns := range pruneNamespaces {
+		// QUESTION: is there a way to query on annotation?
+		fetchedCSVs, err := a.lister.OperatorsV1alpha1().ClusterServiceVersionLister().ClusterServiceVersions(ns).List(labels.Everything())
 		if err != nil {
 			return err
 		}
-		for _, ns := range namespaceObjs {
-			namespaces = append(namespaces, ns.GetName())
+		for _, csv := range fetchedCSVs {
+			if csv.GetAnnotations()[v1alpha2.OperatorGroupAnnotationKey] == operatorGroup.GetName() {
+				a.Log.Debugf("Found CSV '%v' in namespace %v to delete", csv.GetName(), ns)
+				err := a.client.OperatorsV1alpha1().ClusterServiceVersions(ns).Delete(csv.GetName(), &metav1.DeleteOptions{})
+				if err != nil {
+					return err
+				}
+			}
 		}
-	} else {
-		namespaces = operatorGroup.Status.Namespaces
 	}
 
 	logger := a.Log.WithField("operator-ns", operatorGroup.GetNamespace())
@@ -413,6 +438,22 @@ func (a *Operator) copyOperatorGroupAnnotations(obj *metav1.ObjectMeta) map[stri
 	return copiedAnnotations
 }
 
+// returns items in a that are not in b
+// TODO: combine with namespacesChanged?
+func sliceCompare(a, b []string) []string {
+	mb := make(map[string]struct{})
+	for _, x := range b {
+		mb[x] = struct{}{}
+	}
+	ab := []string{}
+	for _, x := range a {
+		if _, ok := mb[x]; !ok {
+			ab = append(ab, x)
+		}
+	}
+	return ab
+}
+
 func namespacesChanged(clusterNamespaces []string, statusNamespaces []string) bool {
 	if len(clusterNamespaces) != len(statusNamespaces) {
 		return true