Add CRD reporting to OperatorPolicy

Also adjusts CSV reporting to handle when the CSV exists but the
subscription does not, by using a label that OLM adds to these objects.

The authorino operator was chosen because it has more than one CRD, but
not too many to make a concise test.

Signed-off-by: Justin Kulikauskas <jkulikau@redhat.com>

controllers/operatorpolicy_controller.go

@@ -65,6 +65,11 @@ var (
 		Version: "v1alpha1",
 		Kind:    "ClusterServiceVersion",
 	}
+	customResourceDefinitionGVK = schema.GroupVersionKind{
+		Group:   "apiextensions.k8s.io",
+		Version: "v1",
+		Kind:    "CustomResourceDefinition",
+	}
 	deploymentGVK = schema.GroupVersionKind{
 		Group:   "apps",
 		Version: "v1",
@@ -251,6 +256,16 @@ func (r *OperatorPolicyReconciler) handleResources(ctx context.Context, policy *
 		return earlyComplianceEvents, condChanged, err
 	}
 
+	earlyConds, changed, err = r.handleCRDs(ctx, policy, subscription)
+	earlyComplianceEvents = append(earlyComplianceEvents, earlyConds...)
+	condChanged = condChanged || changed
+
+	if err != nil {
+		OpLog.Error(err, "Error handling CustomResourceDefinitions")
+
+		return earlyComplianceEvents, condChanged, err
+	}
+
 	changed, err = r.handleDeployment(ctx, policy, csv)
 	condChanged = condChanged || changed
 
@@ -940,37 +955,38 @@ func (r *OperatorPolicyReconciler) handleCSV(
 	}
 
 	watcher := opPolIdentifier(policy.Namespace, policy.Name)
+	selector := subLabelSelector(sub)
 
-	// case where subscription status has not been populated yet
-	if sub.Status.InstalledCSV == "" {
-		return nil, updateStatus(policy, noCSVCond, noExistingCSVObj), nil
+	csvList, err := r.DynamicWatcher.List(watcher, clusterServiceVersionGVK, sub.Namespace, selector)
+	if err != nil {
+		return nil, false, fmt.Errorf("error listing CSVs: %w", err)
 	}
 
-	// Get the CSV related to the object
-	foundCSV, err := r.DynamicWatcher.Get(watcher, clusterServiceVersionGVK, sub.Namespace,
-		sub.Status.InstalledCSV)
-	if err != nil {
-		return nil, false, err
+	var foundCSV *operatorv1alpha1.ClusterServiceVersion
+
+	for _, csv := range csvList {
+		if csv.GetName() == sub.Status.InstalledCSV {
+			matchedCSV := operatorv1alpha1.ClusterServiceVersion{}
+
+			err = runtime.DefaultUnstructuredConverter.FromUnstructured(csv.UnstructuredContent(), &matchedCSV)
+			if err != nil {
+				return nil, false, err
+			}
+
+			foundCSV = &matchedCSV
+		}
 	}
 
+
 	// CSV has not yet been created by OLM
 	if foundCSV == nil {
 		changed := updateStatus(policy,
 			missingWantedCond("ClusterServiceVersion"), missingCSVObj(sub.Name, sub.Namespace))
 
-		return nil, changed, nil
-	}
-
-	// Check CSV most recent condition
-	unstructured := foundCSV.UnstructuredContent()
-	var csv operatorv1alpha1.ClusterServiceVersion
-
-	err = runtime.DefaultUnstructuredConverter.FromUnstructured(unstructured, &csv)
-	if err != nil {
-		return nil, false, err
+		return foundCSV, changed, nil
 	}
 
-	return &csv, updateStatus(policy, buildCSVCond(&csv), existingCSVObj(&csv)), nil
+	return foundCSV, updateStatus(policy, buildCSVCond(foundCSV), existingCSVObj(foundCSV)), nil
 }
 
 func (r *OperatorPolicyReconciler) handleDeployment(
@@ -1029,6 +1045,36 @@ func (r *OperatorPolicyReconciler) handleDeployment(
 	return updateStatus(policy, buildDeploymentCond(depNum > 0, unavailableDeployments), relatedObjects...), nil
 }
 
+func (r *OperatorPolicyReconciler) handleCRDs(
+	_ context.Context,
+	policy *policyv1beta1.OperatorPolicy,
+	sub *operatorv1alpha1.Subscription,
+) ([]metav1.Condition, bool, error) {
+	if sub == nil {
+		return nil, updateStatus(policy, noCRDCond, noExistingCRDObj), nil
+	}
+
+	watcher := opPolIdentifier(policy.Namespace, policy.Name)
+	selector := subLabelSelector(sub)
+
+	crdList, err := r.DynamicWatcher.List(watcher, customResourceDefinitionGVK, sub.Namespace, selector)
+	if err != nil {
+		return nil, false, fmt.Errorf("error listing CRDs: %w", err)
+	}
+
+	if len(crdList) == 0 {
+		return nil, updateStatus(policy, noCRDCond, noExistingCRDObj), nil
+	}
+
+	relatedCRDs := make([]policyv1.RelatedObject, len(crdList))
+
+	for i := range crdList {
+		relatedCRDs[i] = matchedObj(&crdList[i])
+	}
+
+	return nil, updateStatus(policy, crdFoundCond, relatedCRDs...), nil
+}
+
 func (r *OperatorPolicyReconciler) handleCatalogSource(
 	policy *policyv1beta1.OperatorPolicy,
 	subscription *operatorv1alpha1.Subscription,
@@ -1136,3 +1182,20 @@ func (r *OperatorPolicyReconciler) mergeObjects(
 
 	return updateNeeded, false, nil
 }
+
+// subLabelSelector returns a selector that matches a label that OLM adds to resources
+// that are related to a Subscription. It can be used to find those resources even
+// after the Subscription or CSV is deleted.
+func subLabelSelector(sub *operatorv1alpha1.Subscription) labels.Selector {
+	sel, err := metav1.LabelSelectorAsSelector(&metav1.LabelSelector{
+		MatchExpressions: []metav1.LabelSelectorRequirement{{
+			Key:      fmt.Sprintf("operators.coreos.com/%v.%v", sub.Name, sub.Namespace),
+			Operator: metav1.LabelSelectorOpExists,
+		}},
+	})
+	if err != nil {
+		panic(err)
+	}
+
+	return sel
+}

controllers/operatorpolicy_status.go

@@ -233,6 +233,18 @@ func calculateComplianceCondition(policy *policyv1beta1.OperatorPolicy) metav1.C
 		}
 	}
 
+	idx, cond = policy.Status.GetCondition(crdConditionType)
+	if idx == -1 {
+		messages = append(messages, "the status of the CustomResourceDefinitions is unknown")
+		foundNonCompliant = true
+	} else {
+		messages = append(messages, cond.Message)
+
+		if cond.Status != metav1.ConditionTrue {
+			foundNonCompliant = true
+		}
+	}
+
 	idx, cond = policy.Status.GetCondition(deploymentConditionType)
 	if idx == -1 {
 		messages = append(messages, "the status of the Deployments are unknown")
@@ -354,6 +366,7 @@ const (
 	opGroupConditionType     = "OperatorGroupCompliant"
 	subConditionType         = "SubscriptionCompliant"
 	csvConditionType         = "ClusterServiceVersionCompliant"
+	crdConditionType         = "CustomResourceDefinitionCompliant"
 	deploymentConditionType  = "DeploymentCompliant"
 	catalogSrcConditionType  = "CatalogSourcesUnhealthy"
 	installPlanConditionType = "InstallPlanCompliant"
@@ -369,6 +382,8 @@ func condType(kind string) string {
 		return installPlanConditionType
 	case "ClusterServiceVersion":
 		return csvConditionType
+	case "CustomResourceDefinition":
+		return crdConditionType
 	case "Deployment":
 		return deploymentConditionType
 	case "CatalogSource":
@@ -618,6 +633,22 @@ var noCSVCond = metav1.Condition{
 	Message: "A relevant installed ClusterServiceVersion could not be found",
 }
 
+// noCRDCond is a Compliant condition for when no CRDs are found
+var noCRDCond = metav1.Condition{
+	Type:    crdConditionType,
+	Status:  metav1.ConditionTrue,
+	Reason:  "RelevantCRDNotFound",
+	Message: "No CRDs were found for the operator",
+}
+
+// crdFoundCond is a Compliant condition for when CRDs are found
+var crdFoundCond = metav1.Condition{
+	Type:    crdConditionType,
+	Status:  metav1.ConditionTrue,
+	Reason:  "RelevantCRDFound",
+	Message: "There are CRDs present for the operator",
+}
+
 // buildDeploymentCond creates a Condition for deployments. If any are not at their
 // minimum availability, the condition will be NonCompliant, and the message will
 // list the unavailable deployments.
@@ -889,6 +920,21 @@ var noExistingCSVObj = policyv1.RelatedObject{
 	Reason:    "No relevant ClusterServiceVersion found",
 }
 
+// noExistingCRDObj is a Compliant RelatedObject for CustomResourceDefinitions,
+// with Reason 'No relevant CustomResourceDefinitions found'. It is considered
+// compliant because not all operators will have CRDs.
+var noExistingCRDObj = policyv1.RelatedObject{
+	Object: policyv1.ObjectResource{
+		Kind:       customResourceDefinitionGVK.Kind,
+		APIVersion: customResourceDefinitionGVK.GroupVersion().String(),
+		Metadata: policyv1.ObjectMetadata{
+			Name: "-",
+		},
+	},
+	Compliant: string(policyv1.Compliant),
+	Reason:    "No relevant CustomResourceDefinitions found",
+}
+
 // missingDeploymentObj returns a NonCompliant RelatedObject for a Deployment,
 // with Reason 'Resource not found but should exist'
 func missingDeploymentObj(name string, namespace string) policyv1.RelatedObject {

test/e2e/case38_install_operator_test.go

@@ -43,11 +43,11 @@ var _ = Describe("Test installing an operator from OperatorPolicy", Ordered, fun
 	) {
 		var debugMessage string
 
-		defer func() {
+		DeferCleanup(func() {
 			if CurrentSpecReport().Failed() {
 				GinkgoWriter.Println(debugMessage)
 			}
-		}()
+		})
 
 		checkFunc := func(g Gomega) {
 			GinkgoHelper()
@@ -1200,6 +1200,103 @@ var _ = Describe("Test installing an operator from OperatorPolicy", Ordered, fun
 			)
 		})
 	})
+	Describe("Testing CustomResourceDefinition reporting", Ordered, func() {
+		const (
+			opPolYAML = "../resources/case38_operator_install/operator-policy-authorino.yaml"
+			opPolName = "oppol-authorino"
+		)
+		BeforeAll(func() {
+			utils.Kubectl("delete", "crd", "--selector=olm.managed=true")
+			utils.Kubectl("create", "ns", opPolTestNS)
+			DeferCleanup(func() {
+				utils.Kubectl("delete", "ns", opPolTestNS)
+			})
+
+			createObjWithParent(parentPolicyYAML, parentPolicyName,
+				opPolYAML, opPolTestNS, gvrPolicy, gvrOperatorPolicy)
+		})
+
+		It("Should initially not report on CRDs because they won't exist yet", func(ctx SpecContext) {
+			check(
+				opPolName,
+				false,
+				[]policyv1.RelatedObject{{
+					Object: policyv1.ObjectResource{
+						Kind:       "CustomResourceDefinition",
+						APIVersion: "apiextensions.k8s.io/v1",
+						Metadata: policyv1.ObjectMetadata{
+							Name: "-",
+						},
+					},
+					Compliant: "Compliant",
+					Reason:    "No relevant CustomResourceDefinitions found",
+				}},
+				metav1.Condition{
+					Type:    "CustomResourceDefinitionCompliant",
+					Status:  metav1.ConditionTrue,
+					Reason:  "RelevantCRDNotFound",
+					Message: "No CRDs were found for the operator",
+				},
+				"No CRDs were found for the operator",
+			)
+		})
+
+		It("Should generate conditions and relatedobjects of CRD", func(ctx SpecContext) {
+			utils.Kubectl("patch", "operatorpolicy", opPolName, "-n", opPolTestNS, "--type=json", "-p",
+				`[{"op": "replace", "path": "/spec/remediationAction", "value": "enforce"}]`)
+			By("Waiting for a CRD to appear, which should indicate the operator is installing")
+			Eventually(func(ctx SpecContext) *unstructured.Unstructured {
+				crd, _ := clientManagedDynamic.Resource(gvrCRD).Get(ctx,
+					"authconfigs.authorino.kuadrant.io", metav1.GetOptions{})
+
+				return crd
+			}, olmWaitTimeout, 5, ctx).ShouldNot(BeNil())
+
+			By("Waiting for the policy to become compliant, indicating the operator is installed")
+			Eventually(func(g Gomega) string {
+				pol := utils.GetWithTimeout(clientManagedDynamic, gvrOperatorPolicy, opPolName,
+					opPolTestNS, true, eventuallyTimeout)
+				compliance, found, err := unstructured.NestedString(pol.Object, "status", "compliant")
+				g.Expect(err).NotTo(HaveOccurred())
+				g.Expect(found).To(BeTrue())
+
+				return compliance
+			}, olmWaitTimeout, 5, ctx).Should(Equal("Compliant"))
+
+			check(
+				opPolName,
+				false,
+				[]policyv1.RelatedObject{{
+					Object: policyv1.ObjectResource{
+						Kind:       "CustomResourceDefinition",
+						APIVersion: "apiextensions.k8s.io/v1",
+						Metadata: policyv1.ObjectMetadata{
+							Name: "authconfigs.authorino.kuadrant.io",
+						},
+					},
+					Compliant: "Compliant",
+					Reason:    "Resource found as expected",
+				}, {
+					Object: policyv1.ObjectResource{
+						Kind:       "CustomResourceDefinition",
+						APIVersion: "apiextensions.k8s.io/v1",
+						Metadata: policyv1.ObjectMetadata{
+							Name: "authorinos.operator.authorino.kuadrant.io",
+						},
+					},
+					Compliant: "Compliant",
+					Reason:    "Resource found as expected",
+				}},
+				metav1.Condition{
+					Type:    "CustomResourceDefinitionCompliant",
+					Status:  metav1.ConditionTrue,
+					Reason:  "RelevantCRDFound",
+					Message: "There are CRDs present for the operator",
+				},
+				"There are CRDs present for the operator",
+			)
+		})
+	})
 	Describe("Testing OperatorPolicy validation messages", Ordered, func() {
 		const (
 			opPolYAML = "../resources/case38_operator_install/operator-policy-validity-test.yaml"

test/resources/case38_operator_install/operator-policy-authorino.yaml

@@ -0,0 +1,23 @@
+apiVersion: policy.open-cluster-management.io/v1beta1
+kind: OperatorPolicy
+metadata:
+  name: oppol-authorino
+  annotations:
+    policy.open-cluster-management.io/parent-policy-compliance-db-id: "124"
+    policy.open-cluster-management.io/policy-compliance-db-id: "64"
+  ownerReferences:
+  - apiVersion: policy.open-cluster-management.io/v1
+    kind: Policy
+    name: parent-policy
+    uid: 12345678-90ab-cdef-1234-567890abcdef # must be replaced before creation
+spec:
+  remediationAction: inform
+  severity: medium
+  complianceType: musthave
+  subscription:
+    channel: stable
+    name: authorino-operator
+    namespace: operator-policy-testns
+    installPlanApproval: Automatic
+    source: operatorhubio-catalog
+    sourceNamespace: olm