Implement OperatorPolicy health checks for CSV

Following the 'handle*' function pattern that is
used by the subscription and operatorgroup checks,
similar functionality was added to handle OLM CSVs.
Modifications were made to the original subscription
handle function in order to return the unstructured
subscription object to make checking CSVs easier.

Ref: https://issues.redhat.com/browse/ACM-9284

Signed-off-by: Jeffrey Luo <jeluo@redhat.com>

controllers/operatorpolicy_controller.go

@@ -36,8 +36,18 @@ const (
 )
 
 var (
-	subscriptionGVK  = schema.GroupVersionKind{Group: "operators.coreos.com", Version: "v1alpha1", Kind: "Subscription"}
-	operatorGroupGVK = schema.GroupVersionKind{Group: "operators.coreos.com", Version: "v1", Kind: "OperatorGroup"}
+	subscriptionGVK = schema.GroupVersionKind{
+		Group:   "operators.coreos.com",
+		Version: "v1alpha1", Kind: "Subscription",
+	}
+	operatorGroupGVK = schema.GroupVersionKind{
+		Group:   "operators.coreos.com",
+		Version: "v1", Kind: "OperatorGroup",
+	}
+	clusterServiceVersionGVK = schema.GroupVersionKind{
+		Group:   "operators.coreos.com",
+		Version: "v1alpha1", Kind: "ClusterServiceVersion",
+	}
 )
 
 // OperatorPolicyReconciler reconciles a OperatorPolicy object
@@ -125,13 +135,19 @@ func (r *OperatorPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 		return reconcile.Result{}, err
 	}
 
-	if err := r.handleSubscription(ctx, policy); err != nil {
+	subscription, err := r.handleSubscription(ctx, policy)
+	if err != nil {
 		OpLog.Error(err, "Error handling Subscription")
 
 		return reconcile.Result{}, err
 	}
 
-	// FUTURE: more resource checks
+	_, err = r.handleCSV(ctx, policy, subscription)
+	if err != nil {
+		OpLog.Error(err, "Error handling CSVs")
+
+		return reconcile.Result{}, err
+	}
 
 	return reconcile.Result{}, nil
 }
@@ -358,98 +374,116 @@ func buildOperatorGroup(
 	return operatorGroup, nil
 }
 
-func (r *OperatorPolicyReconciler) handleSubscription(ctx context.Context, policy *policyv1beta1.OperatorPolicy) error {
+func (r *OperatorPolicyReconciler) handleSubscription(ctx context.Context,
+	policy *policyv1beta1.OperatorPolicy,
+) (*unstructured.Unstructured, error) {
 	watcher := opPolIdentifier(policy.Namespace, policy.Name)
 
 	desiredSub, err := buildSubscription(policy)
 	if err != nil {
-		return fmt.Errorf("error building subscription: %w", err)
+		return nil, fmt.Errorf("error building subscription: %w", err)
 	}
 
 	foundSub, err := r.DynamicWatcher.Get(watcher, subscriptionGVK, desiredSub.Namespace, desiredSub.Name)
 	if err != nil {
-		return fmt.Errorf("error getting the Subscription: %w", err)
+		return nil, fmt.Errorf("error getting the Subscription: %w", err)
 	}
 
 	if foundSub == nil {
 		// Missing Subscription: report NonCompliance
 		err := r.updateStatus(ctx, policy, missingWantedCond("Subscription"), missingWantedObj(desiredSub))
 		if err != nil {
-			return fmt.Errorf("error updating status for a missing Subscription: %w", err)
+			return nil, fmt.Errorf("error updating status for a missing Subscription: %w", err)
 		}
 
 		if policy.Spec.RemediationAction.IsEnforce() {
 			err := r.Create(ctx, desiredSub)
 			if err != nil {
-				return fmt.Errorf("error creating the Subscription: %w", err)
+				return nil, fmt.Errorf("error creating the Subscription: %w", err)
 			}
 
 			desiredSub.SetGroupVersionKind(subscriptionGVK) // Create stripped this information
 
 			// Now it should match, so report Compliance
 			err = r.updateStatus(ctx, policy, createdCond("Subscription"), createdObj(desiredSub))
 			if err != nil {
-				return fmt.Errorf("error updating the status for a created Subscription: %w", err)
+				return nil, fmt.Errorf("error updating the status for a created Subscription: %w", err)
 			}
 		}
 
-		return nil
+		if foundSub == nil {
+			return nil, nil
+		}
+
+		return foundSub.DeepCopy(), nil
 	}
 
 	// Subscription found; check if specs match
 	desiredUnstruct, err := runtime.DefaultUnstructuredConverter.ToUnstructured(desiredSub)
 	if err != nil {
-		return fmt.Errorf("error converting desired Subscription to an Unstructured: %w", err)
+		return nil, fmt.Errorf("error converting desired Subscription to an Unstructured: %w", err)
 	}
 
 	merged := foundSub.DeepCopy() // Copy it so that the value in the cache is not changed
 
 	updateNeeded, skipUpdate, err := r.mergeObjects(ctx, desiredUnstruct, merged, string(policy.Spec.ComplianceType))
 	if err != nil {
-		return fmt.Errorf("error checking if the Subscription needs an update: %w", err)
+		return nil, fmt.Errorf("error checking if the Subscription needs an update: %w", err)
 	}
 
 	if !updateNeeded {
 		// FUTURE: Check more details about the *status* of the Subscription
 		// For now, just mark it as compliant
 		err := r.updateStatus(ctx, policy, matchesCond("Subscription"), matchedObj(foundSub))
 		if err != nil {
-			return fmt.Errorf("error updating the status for an OperatorGroup that matches: %w", err)
+			return nil, fmt.Errorf("error updating the status for an OperatorGroup that matches: %w", err)
 		}
 
-		return nil
+		if foundSub == nil {
+			return nil, nil
+		}
+
+		return foundSub.DeepCopy(), nil
 	}
 
 	// Specs don't match.
 	if policy.Spec.RemediationAction.IsEnforce() && skipUpdate {
 		err = r.updateStatus(ctx, policy, mismatchCondUnfixable("Subscription"), mismatchedObj(foundSub))
 		if err != nil {
-			return fmt.Errorf("error updating status for a mismatched Subscription that can't be enforced: %w", err)
+			return nil, fmt.Errorf("error updating status for unenforceable mismatched Subscription: %w", err)
+		}
+
+		if foundSub == nil {
+			return nil, nil
 		}
 
-		return nil
+		return foundSub.DeepCopy(), nil
 	}
 
 	err = r.updateStatus(ctx, policy, mismatchCond("Subscription"), mismatchedObj(foundSub))
 	if err != nil {
-		return fmt.Errorf("error updating status for a mismatched Subscription: %w", err)
+		return nil, fmt.Errorf("error updating status for a mismatched Subscription: %w", err)
 	}
 
 	if policy.Spec.RemediationAction.IsEnforce() {
 		err := r.Update(ctx, merged)
 		if err != nil {
-			return fmt.Errorf("error updating the Subscription: %w", err)
+			return nil, fmt.Errorf("error updating the Subscription: %w", err)
 		}
 
 		desiredSub.SetGroupVersionKind(subscriptionGVK) // Update stripped this information
 
 		err = r.updateStatus(ctx, policy, updatedCond("Subscription"), updatedObj(desiredSub))
 		if err != nil {
-			return fmt.Errorf("error updating status after updating the Subscription: %w", err)
+			return nil, fmt.Errorf("error updating status after updating the Subscription: %w", err)
 		}
 	}
 
-	return nil
+	if foundSub == nil {
+		return nil, nil
+	}
+
+	return foundSub.DeepCopy(), nil
 }
 
 // buildSubscription bootstraps the subscription spec defined in the operator policy
@@ -486,6 +520,59 @@ func buildSubscription(
 	return subscription, nil
 }
 
+func (r *OperatorPolicyReconciler) handleCSV(ctx context.Context,
+	policy *policyv1beta1.OperatorPolicy,
+	subscription *unstructured.Unstructured,
+) (*operatorv1alpha1.ClusterServiceVersion, error) {
+	// case where subscription is nil
+	if subscription == nil {
+		return nil, nil
+	}
+
+	watcher := opPolIdentifier(policy.Namespace, policy.Name)
+
+	unstructured := subscription.UnstructuredContent()
+	var sub operatorv1alpha1.Subscription
+
+	err := runtime.DefaultUnstructuredConverter.FromUnstructured(unstructured, &sub)
+	if err != nil {
+		return nil, err
+	}
+
+	// Get the CSV related to the object
+	foundCSV, err := r.DynamicWatcher.Get(watcher, clusterServiceVersionGVK, sub.Namespace,
+		sub.Status.CurrentCSV)
+	if err != nil {
+		return nil, err
+	}
+
+	// CSV has not yet been created by OLM
+	if foundCSV == nil {
+		err := r.updateStatus(ctx, policy, missingWantedCond("ClusterServiceVersion"), missingCSVObj(&sub))
+		if err != nil {
+			return nil, fmt.Errorf("error updating the status for a missing ClusterServiceVersion: %w", err)
+		}
+
+		return nil, err
+	}
+
+	// Check CSV most recent condition
+	unstructured = foundCSV.UnstructuredContent()
+	var csv operatorv1alpha1.ClusterServiceVersion
+	err = runtime.DefaultUnstructuredConverter.FromUnstructured(unstructured, &csv)
+
+	if err != nil {
+		return nil, err
+	}
+
+	err = r.updateStatus(ctx, policy, buildCSVCond(&csv), existingCSVObj(&csv))
+	if err != nil {
+		return &csv, fmt.Errorf("error updating the status for an existing ClusterServiceVersion: %w", err)
+	}
+
+	return &csv, nil
+}
+
 func opPolIdentifier(namespace, name string) depclient.ObjectIdentifier {
 	return depclient.ObjectIdentifier{
 		Group:     policyv1beta1.GroupVersion.Group,

controllers/operatorpolicy_status.go

@@ -7,6 +7,7 @@ import (
 	"strings"
 	"time"
 
+	operatorv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -199,6 +200,18 @@ func calculateComplianceCondition(policy *policyv1beta1.OperatorPolicy) metav1.C
 		}
 	}
 
+	idx, cond = policy.Status.GetCondition(csvConditionType)
+	if idx == -1 {
+		messages = append(messages, "the status of the ClusterServiceVersion is unknown")
+		foundNonCompliant = true
+	} else {
+		messages = append(messages, cond.Message)
+
+		if cond.Status != metav1.ConditionTrue {
+			foundNonCompliant = true
+		}
+	}
+
 	// FUTURE: check additional conditions
 
 	if foundNonCompliant {
@@ -277,6 +290,7 @@ const (
 	compliantConditionType = "Compliant"
 	opGroupConditionType   = "OperatorGroupCompliant"
 	subConditionType       = "SubscriptionCompliant"
+	csvConditionType       = "CSVCompliant"
 )
 
 func condType(kind string) string {
@@ -285,6 +299,8 @@ func condType(kind string) string {
 		return opGroupConditionType
 	case "Subscription":
 		return subConditionType
+	case "ClusterServiceVersion":
+		return csvConditionType
 	default:
 		panic("Unknown condition type for kind " + kind)
 	}
@@ -356,6 +372,21 @@ func updatedCond(kind string) metav1.Condition {
 	}
 }
 
+// buildCSVCond takes a csv and returns a shortened version of its most recent Condition
+func buildCSVCond(csv *operatorv1alpha1.ClusterServiceVersion) metav1.Condition {
+	status := metav1.ConditionFalse
+	if csv.Status.Phase == operatorv1alpha1.CSVPhaseSucceeded {
+		status = metav1.ConditionTrue
+	}
+
+	return metav1.Condition{
+		Type:    condType(csv.Kind),
+		Status:  status,
+		Reason:  string(csv.Status.Reason) + "Phase" + string(csv.Status.Phase),
+		Message: "ClusterServiceVersion - " + csv.Status.Message,
+	}
+}
+
 var opGroupPreexistingCond = metav1.Condition{
 	Type:   opGroupConditionType,
 	Status: metav1.ConditionTrue,
@@ -440,7 +471,7 @@ func opGroupTooManyObjs(opGroups []unstructured.Unstructured) []policyv1.Related
 
 	for i, opGroup := range opGroups {
 		objs[i] = policyv1.RelatedObject{
-			Object:    policyv1.ObjectResourceFromObj(&opGroups[i]),
+			Object:    policyv1.ObjectResourceFromObj(&opGroup),
 			Compliant: string(policyv1.NonCompliant),
 			Reason:    "There is more than one OperatorGroup in this namespace",
 			Properties: &policyv1.ObjectProperties{
@@ -451,3 +482,41 @@ func opGroupTooManyObjs(opGroups []unstructured.Unstructured) []policyv1.Related
 
 	return objs
 }
+
+func missingCSVObj(sub *operatorv1alpha1.Subscription) policyv1.RelatedObject {
+	return policyv1.RelatedObject{
+		Object: policyv1.ObjectResource{
+			Kind:       clusterServiceVersionGVK.Kind,
+			APIVersion: clusterServiceVersionGVK.GroupVersion().String(),
+			Metadata: policyv1.ObjectMetadata{
+				Name:      sub.Status.CurrentCSV,
+				Namespace: sub.GetNamespace(),
+			},
+		},
+		Compliant: string(policyv1.NonCompliant),
+		Reason:    reasonWantFoundDNE,
+	}
+}
+
+func existingCSVObj(csv *operatorv1alpha1.ClusterServiceVersion) policyv1.RelatedObject {
+	compliance := policyv1.NonCompliant
+	if csv.Status.Phase == operatorv1alpha1.CSVPhaseSucceeded {
+		compliance = policyv1.Compliant
+	}
+
+	return policyv1.RelatedObject{
+		Object: policyv1.ObjectResource{
+			Kind:       clusterServiceVersionGVK.Kind,
+			APIVersion: clusterServiceVersionGVK.GroupVersion().String(),
+			Metadata: policyv1.ObjectMetadata{
+				Name:      csv.Name,
+				Namespace: csv.GetNamespace(),
+			},
+		},
+		Compliant: string(compliance),
+		Reason:    string(csv.Status.Reason) + "Phase" + string(csv.Status.Phase),
+		Properties: &policyv1.ObjectProperties{
+			UID: string(csv.GetUID()),
+		},
+	}
+}