fix(olm): requeue as needed upon permission changes

The fix here is a combination of resetting the phase on CSV that has
been set to a "no retry" state based on previous permission failures
along with requeuing. The requeuing notifies OLM when RBAC additions are
made as well as changes to the service account when specified on an
operator group.

Author: Jeff Peeler

pkg/api/apis/operators/v1alpha1/clusterserviceversion_types.go

@@ -231,7 +231,7 @@ const (
 	CSVReasonRequirementsMet                             ConditionReason = "AllRequirementsMet"
 	CSVReasonOwnerConflict                               ConditionReason = "OwnerConflict"
 	CSVReasonComponentFailed                             ConditionReason = "InstallComponentFailed"
-	CSVReasonComponentFailedNoRetry						 ConditionReason = "InstallComponentFailedNoRetry"
+	CSVReasonComponentFailedNoRetry                      ConditionReason = "InstallComponentFailedNoRetry"
 	CSVReasonInvalidStrategy                             ConditionReason = "InvalidInstallStrategy"
 	CSVReasonWaiting                                     ConditionReason = "InstallWaiting"
 	CSVReasonInstallSuccessful                           ConditionReason = "InstallSucceeded"
@@ -252,6 +252,7 @@ const (
 	CSVReasonTooManyOperatorGroups                       ConditionReason = "TooManyOperatorGroups"
 	CSVReasonInterOperatorGroupOwnerConflict             ConditionReason = "InterOperatorGroupOwnerConflict"
 	CSVReasonCannotModifyStaticOperatorGroupProvidedAPIs ConditionReason = "CannotModifyStaticOperatorGroupProvidedAPIs"
+	CSVReasonDetectedClusterChange                       ConditionReason = "DetectedClusterChange"
 )
 
 // Conditions appear in the status as a record of state transitions on the ClusterServiceVersion

pkg/controller/operators/catalog/installplan_sync.go

@@ -7,11 +7,10 @@ import (
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
-	"k8s.io/apimachinery/pkg/runtime"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 
+	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/scoped"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/operators/v1alpha1"
-	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/ownerutil"
 )
 
 // When a user adds permission to a ServiceAccount by creating or updating
@@ -26,7 +25,7 @@ func (o *Operator) triggerInstallPlanRetry(obj interface{}) (syncError error) {
 		return
 	}
 
-	related, _ := isObjectRBACRelated(obj)
+	related, _ := scoped.IsObjectRBACRelated(obj)
 	if !related {
 		return
 	}
@@ -75,26 +74,3 @@ func (o *Operator) triggerInstallPlanRetry(obj interface{}) (syncError error) {
 	syncError = utilerrors.NewAggregate(errs)
 	return
 }
-
-func isObjectRBACRelated(obj interface{}) (related bool, object runtime.Object) {
-	object, ok := obj.(runtime.Object)
-	if !ok {
-		return
-	}
-
-	if err := ownerutil.InferGroupVersionKind(object); err != nil {
-		return
-	}
-
-	kind := object.GetObjectKind().GroupVersionKind().Kind
-	switch kind {
-	case roleKind:
-		fallthrough
-	case roleBindingKind:
-		fallthrough
-	case serviceAccountKind:
-		related = true
-	}
-
-	return
-}

pkg/controller/operators/olm/operator.go

@@ -679,6 +679,29 @@ func (a *Operator) syncObject(obj interface{}) (syncError error) {
 		a.requeueCSVsByLabelSet(logger, labelSets...)
 	}
 
+	// Requeue CSVs that have the reason of `CSVReasonComponentFailedNoRetry` in the case of an RBAC change
+	var errs []error
+	related, _ := scoped.IsObjectRBACRelated(metaObj)
+	if related {
+		csvList := a.csvSet(metaObj.GetNamespace(), v1alpha1.CSVPhaseFailed)
+		for _, csv := range csvList {
+			if csv.Status.Reason != v1alpha1.CSVReasonComponentFailedNoRetry {
+				continue
+			}
+			csv.SetPhase(v1alpha1.CSVPhasePending, v1alpha1.CSVReasonDetectedClusterChange, "Cluster resources changed state", a.now())
+			_, err := a.client.OperatorsV1alpha1().ClusterServiceVersions(csv.GetNamespace()).UpdateStatus(csv)
+			if err != nil {
+				errs = append(errs, err)
+				continue
+			}
+			if err := a.csvQueueSet.Requeue(csv.GetNamespace(), csv.GetName()); err != nil {
+				errs = append(errs, err)
+			}
+			logger.Debug("Requeuing CSV due to detected RBAC change")
+		}
+	}
+
+	syncError = utilerrors.NewAggregate(errs)
 	return nil
 }
 
@@ -1096,6 +1119,12 @@ func (a *Operator) operatorGroupForCSV(csv *v1alpha1.ClusterServiceVersion, logg
 			if targetNamespaceList, err := a.getOperatorGroupTargets(operatorGroup); err == nil && len(targetNamespaceList) == 0 {
 				csv.SetPhaseWithEventIfChanged(v1alpha1.CSVPhaseFailed, v1alpha1.CSVReasonNoTargetNamespaces, "no targetNamespaces are matched operatorgroups namespace selection", now, a.recorder)
 			}
+			logger.Debug("CSV not in operatorgroup, requeuing operator group")
+			// this requeue helps when an operator group has not annotated a CSV due to a permissions error
+			// but the permissions issue has now been resolved
+			if err := a.ogQueueSet.Requeue(operatorGroup.GetNamespace(), operatorGroup.GetName()); err != nil {
+				return nil, err
+			}
 			return nil, nil
 		}
 		logger.Info("csv in operatorgroup")
@@ -1120,7 +1149,7 @@ func (a *Operator) transitionCSVState(in v1alpha1.ClusterServiceVersion) (out *v
 	})
 
 	if in.Status.Reason == v1alpha1.CSVReasonComponentFailedNoRetry {
-		// will change phase out of failed-no-retry in the event of an intentional requeue
+		// will change phase out of failed in the event of an intentional requeue
 		logger.Debugf("skipping sync for CSV in failed-no-retry state")
 		return
 	}

pkg/controller/operators/olm/operatorgroup.go

@@ -53,11 +53,34 @@ func (a *Operator) syncOperatorGroups(obj interface{}) error {
 		"namespace":     op.GetNamespace(),
 	})
 
+	previousRef := op.Status.ServiceAccountRef.DeepCopy()
 	op, err := a.serviceAccountSyncer.SyncOperatorGroup(op)
 	if err != nil {
 		logger.Errorf("error updating service account - %v", err)
 		return err
 	}
+	if op.Status.ServiceAccountRef != previousRef {
+		crdList, err := a.lister.OperatorsV1alpha1().ClusterServiceVersionLister().List(labels.Everything())
+		if err != nil {
+			return err
+		}
+		for _, csv := range crdList {
+			if group, ok := csv.GetAnnotations()[v1.OperatorGroupAnnotationKey]; !ok || group != op.GetName() {
+				continue
+			}
+			if csv.Status.Reason == v1alpha1.CSVReasonComponentFailedNoRetry {
+				csv.SetPhase(v1alpha1.CSVPhasePending, v1alpha1.CSVReasonDetectedClusterChange, "Cluster resources changed state", a.now())
+				_, err := a.client.OperatorsV1alpha1().ClusterServiceVersions(csv.GetNamespace()).UpdateStatus(csv)
+				if err != nil {
+					return err
+				}
+				if err := a.csvQueueSet.Requeue(csv.GetNamespace(), csv.GetName()); err != nil {
+					return err
+				}
+				logger.Debug("Requeuing CSV due to detected service account change")
+			}
+		}
+	}
 
 	targetNamespaces, err := a.updateNamespaceList(op)
 	if err != nil {

pkg/lib/scoped/util.go

@@ -2,6 +2,15 @@ package scoped
 
 import (
 	v1 "k8s.io/api/core/v1"
+	rbacv1 "k8s.io/api/rbac/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+
+	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/ownerutil"
+)
+
+const (
+	roleKind        = "Role"
+	roleBindingKind = "RoleBinding"
 )
 
 // IsServiceAccountToken returns true if the secret is a valid api token for the service account
@@ -24,3 +33,26 @@ func IsServiceAccountToken(secret *v1.Secret, sa *v1.ServiceAccount) bool {
 
 	return true
 }
+
+func IsObjectRBACRelated(obj interface{}) (related bool, object runtime.Object) {
+	object, ok := obj.(runtime.Object)
+	if !ok {
+		return
+	}
+
+	if err := ownerutil.InferGroupVersionKind(object); err != nil {
+		return
+	}
+
+	kind := object.GetObjectKind().GroupVersionKind().Kind
+	switch kind {
+	case roleKind:
+		fallthrough
+	case roleBindingKind:
+		fallthrough
+	case rbacv1.ServiceAccountKind:
+		related = true
+	}
+
+	return
+}