diff --git a/controllers/operatorpolicy_controller.go b/controllers/operatorpolicy_controller.go index b0e6fe74..80eecada 100644 --- a/controllers/operatorpolicy_controller.go +++ b/controllers/operatorpolicy_controller.go @@ -36,8 +36,21 @@ 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", + } + catalogSrcGVK = schema.GroupVersionKind{ + Group: "operators.coreos.com", + Version: "v1alpha1", + Kind: "CatalogSource", + } ) // OperatorPolicyReconciler reconciles a OperatorPolicy object @@ -133,9 +146,92 @@ func (r *OperatorPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reque // FUTURE: more resource checks + // handle catalogsource + if err := r.handleCatalogSource(ctx, policy); err != nil { + OpLog.Error(err, "Error handling CatalogSource") + + return reconcile.Result{}, err + } + return reconcile.Result{}, nil } +func (r *OperatorPolicyReconciler) handleCatalogSource( + ctx context.Context, + policy *policyv1beta1.OperatorPolicy, +) error { + watcher := opPolIdentifier(policy.Namespace, policy.Name) + + desiredSub, err := buildSubscription(policy) + if err != nil { + return fmt.Errorf("error building Subscription: %w", err) + } + + catalogName := desiredSub.Spec.CatalogSource + catalogNS := desiredSub.Spec.CatalogSourceNamespace + + // Retrieve Subscription + foundSub, err := r.DynamicWatcher.Get(watcher, subscriptionGVK, desiredSub.Namespace, desiredSub.Name) + if err != nil { + return fmt.Errorf("error getting Subscription: %w", err) + } + + if foundSub == nil { + // Subscription not found, check the CatalogSource object + foundCatalogSrc, err := r.DynamicWatcher.Get(watcher, catalogSrcGVK, + desiredSub.Spec.CatalogSourceNamespace, desiredSub.Spec.CatalogSource) + if err != nil { + return fmt.Errorf("error getting CatalogSource: %w", err) + } + + // Conditionally emit health event using the defined status template + isMissing := foundCatalogSrc == nil + + err = r.updateStatus(ctx, policy, catalogSourceFindCond(isMissing), + catalogSourceObj(catalogName, catalogNS, isMissing)) + if err != nil { + return fmt.Errorf("error updating the status for CatalogSource when Subscription DNE on cluster: %w", err) + } + + return nil + } + + unstructured := foundSub.UnstructuredContent() + var subObj operatorv1alpha1.Subscription + + err = runtime.DefaultUnstructuredConverter.FromUnstructured(unstructured, &subObj) + if err != nil { + return fmt.Errorf("failed to convert type from unstructured to Subscription") + } + + // Iterate over all the conditions to determine the state of the CatalogSource + for _, condition := range subObj.Status.Conditions { + if condition.Type == catalogSrcConditionType { + status := metav1.ConditionFalse + if string(condition.Status) == string(metav1.ConditionTrue) { + status = metav1.ConditionTrue + } + + isMissing := string(condition.Status) == string(metav1.ConditionTrue) + + healthCondition := metav1.Condition{ + Type: string(condition.Type), + Status: status, + LastTransitionTime: *condition.LastTransitionTime, + Reason: condition.Reason, + Message: condition.Message, + } + + err := r.updateStatus(ctx, policy, healthCondition, catalogSourceObj(catalogName, catalogNS, isMissing)) + if err != nil { + return fmt.Errorf("error updating the status for a missing CatalogSource: %w", err) + } + } + } + + return nil +} + func (r *OperatorPolicyReconciler) handleOpGroup(ctx context.Context, policy *policyv1beta1.OperatorPolicy) error { watcher := opPolIdentifier(policy.Namespace, policy.Name) @@ -414,7 +510,7 @@ func (r *OperatorPolicyReconciler) handleSubscription(ctx context.Context, polic // 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 fmt.Errorf("error updating the status for a Subscription that matches: %w", err) } return nil diff --git a/controllers/operatorpolicy_status.go b/controllers/operatorpolicy_status.go index d30bd707..dc6037ba 100644 --- a/controllers/operatorpolicy_status.go +++ b/controllers/operatorpolicy_status.go @@ -200,6 +200,17 @@ func calculateComplianceCondition(policy *policyv1beta1.OperatorPolicy) metav1.C } // FUTURE: check additional conditions + idx, cond = policy.Status.GetCondition(catalogSrcConditionType) + if idx == -1 { + messages = append(messages, "the status of the CatalogSource is unknown") + foundNonCompliant = true + } else { + messages = append(messages, cond.Message) + + if cond.Status != metav1.ConditionFalse { + foundNonCompliant = true + } + } if foundNonCompliant { return metav1.Condition{ @@ -274,9 +285,10 @@ func (r *OperatorPolicyReconciler) emitComplianceEvent( } const ( - compliantConditionType = "Compliant" - opGroupConditionType = "OperatorGroupCompliant" - subConditionType = "SubscriptionCompliant" + compliantConditionType = "Compliant" + opGroupConditionType = "OperatorGroupCompliant" + subConditionType = "SubscriptionCompliant" + catalogSrcConditionType = "CatalogSourcesUnhealthy" ) func condType(kind string) string { @@ -373,6 +385,51 @@ var opGroupTooManyCond = metav1.Condition{ Message: "there is more than one OperatorGroup in the namespace", } +// catalogSourceFindCond is a conditionally compliant condition with reason +// based on the `isMissing` parameter +func catalogSourceFindCond(isMissing bool) metav1.Condition { + status := metav1.ConditionFalse + reason := "CatalogSourcesFound" + message := "CatalogSource was found" + + if isMissing { + status = metav1.ConditionTrue + reason = "CatalogSourcesNotFound" + message = "CatalogSource was not found" + } + + return metav1.Condition{ + Type: "CatalogSourcesUnhealthy", + Status: status, + Reason: reason, + Message: message, + } +} + +// catalogSourceObj returns a conditionally compliant RelatedObject with reason based on the `isMissing` parameter +func catalogSourceObj(catalogName string, catalogNS string, isMissing bool) policyv1.RelatedObject { + compliance := string(policyv1.NonCompliant) + reason := reasonWantFoundDNE + + if !isMissing { + compliance = string(policyv1.Compliant) + reason = reasonWantFoundExists + } + + return policyv1.RelatedObject{ + Object: policyv1.ObjectResource{ + Kind: "CatalogSource", + APIVersion: "operators.coreos.com/v1alpha1", + Metadata: policyv1.ObjectMetadata{ + Name: catalogName, + Namespace: catalogNS, + }, + }, + Compliant: compliance, + Reason: reason, + } +} + // missingWantedObj returns a NonCompliant RelatedObject with reason = 'Resource not found but should exist' func missingWantedObj(obj client.Object) policyv1.RelatedObject { return policyv1.RelatedObject{ diff --git a/test/e2e/case38_install_operator_test.go b/test/e2e/case38_install_operator_test.go index 55219c94..270fd51d 100644 --- a/test/e2e/case38_install_operator_test.go +++ b/test/e2e/case38_install_operator_test.go @@ -550,4 +550,111 @@ var _ = Describe("Test installing an operator from OperatorPolicy", Ordered, fun ) }) }) + Describe("Test status reporting for catalogsource", Ordered, func() { + const ( + // opPolTestNS = "operator-policy-testns" + OpPlcYAML = "../resources/case38_operator_install/operator-policy-with-group.yaml" + OpPlcName = "oppol-with-group" + subName = "project-quay" + ) + + BeforeAll(func() { + By("Applying creating a ns and the test policy") + utils.Kubectl("create", "ns", opPolTestNS) + DeferCleanup(func() { + utils.Kubectl("delete", "ns", opPolTestNS) + }) + + createObjWithParent(parentPolicyYAML, parentPolicyName, + OpPlcYAML, opPolTestNS, gvrPolicy, gvrOperatorPolicy) + }) + + It("Should initially be Compliant", func() { + check( + OpPlcName, + false, + []policyv1.RelatedObject{{ + Object: policyv1.ObjectResource{ + Kind: "CatalogSource", + APIVersion: "operators.coreos.com/v1alpha1", + Metadata: policyv1.ObjectMetadata{ + Name: "operatorhubio-catalog", + Namespace: opPolTestNS, + }, + }, + Compliant: "Compliant", + Reason: "Resource found as expected", + }}, + metav1.Condition{ + Type: "CatalogSourcesUnhealthy", + Status: metav1.ConditionFalse, + Reason: "CatalogSourcesFound", + Message: "CatalogSource was found", + }, + "CatalogSource was found", + ) + }) + + It("Should remain compliant when policy is enforced", func() { + By("Enforcing the policy") + utils.Kubectl("patch", "operatorpolicy", OpPlcName, "-n", opPolTestNS, "--type=json", "-p", + `[{"op": "replace", "path": "/spec/remediationAction", "value": "enforce"}]`) + + By("Checking if the condition fields were inherited from the Subscription") + check( + OpPlcName, + false, + []policyv1.RelatedObject{{ + Object: policyv1.ObjectResource{ + Kind: "CatalogSource", + APIVersion: "operators.coreos.com/v1alpha1", + Metadata: policyv1.ObjectMetadata{ + Name: "operatorhubio-catalog", + Namespace: opPolTestNS, + }, + }, + Compliant: "Compliant", + Reason: "Resource found as expected", + }}, + metav1.Condition{ + Type: "CatalogSourcesUnhealthy", + Status: metav1.ConditionFalse, + Reason: "AllCatalogSourcesHealthy", + Message: "all available catalogsources are healthy", + }, + "all available catalogsources are healthy", + ) + }) + + It("Should become NonCompliant when CatalogSource fails", func() { + By("Patching the policy to reference a catalogSource that DNE to emulate failure") + utils.Kubectl("patch", "operatorpolicy", OpPlcName, "-n", opPolTestNS, "--type=json", "-p", + `[{"op": "replace", "path": "/spec/subscription/source", "value": "fakeName"}]`) + + By("Checking the conditions and relatedObj in the policy") + check( + OpPlcName, + true, + []policyv1.RelatedObject{{ + Object: policyv1.ObjectResource{ + Kind: "CatalogSource", + APIVersion: "operators.coreos.com/v1alpha1", + Metadata: policyv1.ObjectMetadata{ + Name: "fakeName", + Namespace: opPolTestNS, + }, + }, + Compliant: "NonCompliant", + Reason: "Resource not found but should exist", + }}, + metav1.Condition{ + Type: "CatalogSourcesUnhealthy", + Status: "True", + Reason: "UnhealthyCatalogSourceFound", + Message: "targeted catalogsource olm/fakeName missing", + }, + "targeted catalogsource olm/fakeName missing", + ) + }) + }) }) diff --git a/test/resources/operatorpolicy.yaml b/test/resources/operatorpolicy.yaml deleted file mode 100644 index eb6209ce..00000000 --- a/test/resources/operatorpolicy.yaml +++ /dev/null @@ -1,34 +0,0 @@ -apiVersion: policy.open-cluster-management.io/v1beta1 -kind: OperatorPolicy -metadata: - name: test-operator-policy -spec: - remediationAction: enforce # or inform - severity: medium - complianceType: musthave # or mustnothave - operatorGroup: # optional - name: og-single - namespace: default - target: - namespaces: - - default - subscription: - channel: stable-3.8 - name: project-quay - namespace: default - installPlanApproval: Automatic # may be overridden to Manual by other settings - source: operatorhubio-catalog - sourceNamespace: olm - startingCSV: quay-operator.v3.8.1 - removalBehavior: - operatorGroups: DeleteIfUnused - subscriptions: Delete - clusterServiceVersions: Delete - installPlans: Keep - customResourceDefinitions: Keep - apiServiceDefinitions: Keep - statusConfig: - catalogSourceUnhealthy: StatusMessageOnly - deploymentsUnavailable: NonCompliant - upgradesAvailable: StatusMessageOnly - upgradesProgressing: NonCompliant