diff --git a/controllers/operatorpolicy_controller.go b/controllers/operatorpolicy_controller.go index 9a66f190..45bbd2c7 100644 --- a/controllers/operatorpolicy_controller.go +++ b/controllers/operatorpolicy_controller.go @@ -35,6 +35,7 @@ import ( const ( OperatorControllerName string = "operator-policy-controller" + CatalogSourceReady string = "READY" ) var ( @@ -58,6 +59,11 @@ var ( Version: "v1", Kind: "Deployment", } + catalogSrcGVK = schema.GroupVersionKind{ + Group: "operators.coreos.com", + Version: "v1alpha1", + Kind: "CatalogSource", + } ) // OperatorPolicyReconciler reconciles a OperatorPolicy object @@ -165,9 +171,83 @@ func (r *OperatorPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reque return reconcile.Result{}, err } + // handle catalogsource + if err := r.handleCatalogSource(ctx, policy, subscription); 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, + subscription *operatorv1alpha1.Subscription, +) error { + watcher := opPolIdentifier(policy.Namespace, policy.Name) + + var catalogName string + var catalogNS string + + if subscription == nil { + sub, err := buildSubscription(policy) + if err != nil { + return fmt.Errorf("error building Subscription: %w", err) + } + + catalogName = sub.Spec.CatalogSource + catalogNS = sub.Spec.CatalogSourceNamespace + } else { + catalogName = subscription.Spec.CatalogSource + catalogNS = subscription.Spec.CatalogSourceNamespace + } + + // Check if CatalogSource exists + foundCatalogSrc, err := r.DynamicWatcher.Get(watcher, catalogSrcGVK, + catalogNS, catalogName) + if err != nil { + return fmt.Errorf("error getting CatalogSource: %w", err) + } + + isMissing := foundCatalogSrc == nil + isUnhealthy := isMissing + + if !isMissing { + // CatalogSource is found, initiate health check + catalogSrcUnstruct := foundCatalogSrc.DeepCopy() + catalogSrc := new(operatorv1alpha1.CatalogSource) + + err := runtime.DefaultUnstructuredConverter. + FromUnstructured(catalogSrcUnstruct.Object, catalogSrc) + if err != nil { + return fmt.Errorf("error converting the retrieved CatalogSource to the Go type: %w", err) + } + + if catalogSrc.Status.GRPCConnectionState == nil { + // Unknown State + err := r.updateStatus(ctx, policy, catalogSourceUnknownCond, catalogSrcUnknownObj(catalogName, catalogNS)) + if err != nil { + return fmt.Errorf("error retrieving the status for a CatalogSource: %w", err) + } + + return nil + } + + CatalogSrcState := catalogSrc.Status.GRPCConnectionState.LastObservedState + isUnhealthy = (CatalogSrcState != CatalogSourceReady) + } + + err = r.updateStatus(ctx, policy, catalogSourceFindCond(isUnhealthy, isMissing), + catalogSourceObj(catalogName, catalogNS, isUnhealthy, isMissing)) + if err != nil { + return fmt.Errorf("error updating the status for a CatalogSource: %w", err) + } + + return nil +} + func (r *OperatorPolicyReconciler) handleOpGroup(ctx context.Context, policy *policyv1beta1.OperatorPolicy) error { watcher := opPolIdentifier(policy.Namespace, policy.Name) @@ -453,7 +533,7 @@ func (r *OperatorPolicyReconciler) handleSubscription( // For now, just mark it as compliant err := r.updateStatus(ctx, policy, matchesCond("Subscription"), matchedObj(foundSub)) if err != nil { - return nil, fmt.Errorf("error updating the status for an OperatorGroup that matches: %w", err) + return nil, fmt.Errorf("error updating the status for a Subscription that matches: %w", err) } return mergedSub, nil diff --git a/controllers/operatorpolicy_status.go b/controllers/operatorpolicy_status.go index 4afc25b8..03fada5e 100644 --- a/controllers/operatorpolicy_status.go +++ b/controllers/operatorpolicy_status.go @@ -226,6 +226,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{ @@ -305,6 +316,7 @@ const ( subConditionType = "SubscriptionCompliant" csvConditionType = "ClusterServiceVersionCompliant" deploymentConditionType = "DeploymentCompliant" + catalogSrcConditionType = "CatalogSourcesUnhealthy" ) func condType(kind string) string { @@ -468,6 +480,88 @@ var noDeploymentsCond = metav1.Condition{ Message: "The ClusterServiceVersion is missing, thus meaning there are no relevant deployments", } +// catalogSourceFindCond is a conditionally compliant condition with reason +// based on the `isUnhealthy` and `isMissing` parameters +func catalogSourceFindCond(isUnhealthy bool, isMissing bool) metav1.Condition { + status := metav1.ConditionFalse + reason := "CatalogSourcesFound" + message := "CatalogSource was found" + + if isUnhealthy { + status = metav1.ConditionTrue + reason = "CatalogSourcesFoundUnhealthy" + message = "CatalogSource was found but is unhealthy" + } + + if isMissing { + status = metav1.ConditionTrue + reason = "CatalogSourcesNotFound" + message = "CatalogSource was not found" + } + + return metav1.Condition{ + Type: "CatalogSourcesUnhealthy", + Status: status, + Reason: reason, + Message: message, + } +} + +// catalogSourceUnknownCond is a NonCompliant condition +var catalogSourceUnknownCond = metav1.Condition{ + Type: "CatalogSourcesUnknownState", + Status: metav1.ConditionTrue, + Reason: "LastObservedUnknown", + Message: "Could not determine last observed state of CatalogSource", +} + +// catalogSourceObj returns a conditionally compliant RelatedObject with reason based on the +// `isUnhealthy` and `isMissing` parameters +func catalogSourceObj(catalogName string, catalogNS string, isUnhealthy bool, isMissing bool) policyv1.RelatedObject { + compliance := string(policyv1.Compliant) + reason := reasonWantFoundExists + + if isUnhealthy { + compliance = string(policyv1.NonCompliant) + reason = reasonWantFoundExists + " but is unhealthy" + } + + if isMissing { + compliance = string(policyv1.NonCompliant) + reason = reasonWantFoundDNE + } + + return policyv1.RelatedObject{ + Object: policyv1.ObjectResource{ + Kind: catalogSrcGVK.Kind, + APIVersion: catalogSrcGVK.GroupVersion().String(), + Metadata: policyv1.ObjectMetadata{ + Name: catalogName, + Namespace: catalogNS, + }, + }, + Compliant: compliance, + Reason: reason, + } +} + +// catalogSrcUnknownObj returns a NonCompliant RelatedObject with +// reason = 'Resource found but current state is unknown' +func catalogSrcUnknownObj(catalogName string, catalogNS string) policyv1.RelatedObject { + return policyv1.RelatedObject{ + Object: policyv1.ObjectResource{ + Kind: catalogSrcGVK.Kind, + APIVersion: catalogSrcGVK.GroupVersion().String(), + Metadata: policyv1.ObjectMetadata{ + Name: catalogName, + Namespace: catalogNS, + }, + }, + Compliant: string(policyv1.NonCompliant), + Reason: "Resource found but current state is unknown", + } +} + // 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 0406679d..277d6b39 100644 --- a/test/e2e/case38_install_operator_test.go +++ b/test/e2e/case38_install_operator_test.go @@ -496,7 +496,6 @@ var _ = Describe("Test installing an operator from OperatorPolicy", Ordered, fun createObjWithParent(parentPolicyYAML, parentPolicyName, opPolYAML, opPolTestNS, gvrPolicy, gvrOperatorPolicy) }) - It("Should initially notice the matching Subscription", func() { check( opPolName, @@ -550,7 +549,6 @@ var _ = Describe("Test installing an operator from OperatorPolicy", Ordered, fun ) }) }) - Describe("Test health checks on OLM resources after OperatorPolicy operator installation", Ordered, func() { const ( opPolYAML = "../resources/case38_operator_install/operator-policy-no-group-enforce.yaml" @@ -610,7 +608,6 @@ var _ = Describe("Test installing an operator from OperatorPolicy", Ordered, fun ) }) }) - Describe("Test health checks on OLM resources on OperatorPolicy with failed CSV", Ordered, func() { const ( opPolYAML = "../resources/case38_operator_install/operator-policy-no-group-csv-fail.yaml" @@ -672,4 +669,146 @@ var _ = Describe("Test installing an operator from OperatorPolicy", Ordered, fun ) }) }) + Describe("Test status reporting for CatalogSource", Ordered, func() { + const ( + OpPlcYAML = "../resources/case38_operator_install/operator-policy-with-group.yaml" + OpPlcName = "oppol-with-group" + subName = "project-quay" + catSrcName = "operatorhubio-catalog" + ) + + BeforeAll(func() { + By("Applying creating a ns and the test policy") + utils.Kubectl("create", "ns", opPolTestNS) + DeferCleanup(func() { + utils.Kubectl("patch", "catalogsource", catSrcName, "-n", "olm", "--type=json", "-p", + `[{"op": "replace", "path": "/spec/image", "value": "quay.io/operatorhubio/catalog:latest"}]`) + utils.Kubectl("delete", "ns", opPolTestNS) + }) + + createObjWithParent(parentPolicyYAML, parentPolicyName, + OpPlcYAML, opPolTestNS, gvrPolicy, gvrOperatorPolicy) + }) + + It("Should initially show the CatalogSource is compliant", func() { + By("Checking the condition fields") + check( + OpPlcName, + false, + []policyv1.RelatedObject{{ + Object: policyv1.ObjectResource{ + Kind: "CatalogSource", + APIVersion: "operators.coreos.com/v1alpha1", + Metadata: policyv1.ObjectMetadata{ + Name: catSrcName, + 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 the condition fields") + check( + OpPlcName, + false, + []policyv1.RelatedObject{{ + Object: policyv1.ObjectResource{ + Kind: "CatalogSource", + APIVersion: "operators.coreos.com/v1alpha1", + Metadata: policyv1.ObjectMetadata{ + Name: catSrcName, + 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 become NonCompliant when CatalogSource DNE", 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: metav1.ConditionTrue, + Reason: "CatalogSourcesNotFound", + Message: "CatalogSource was not found", + }, + "CatalogSource was not found", + ) + }) + It("Should remain NonCompliant when CatalogSource fails", func() { + By("Patching the policy to point to an existing CatalogSource") + utils.Kubectl("patch", "operatorpolicy", OpPlcName, "-n", opPolTestNS, "--type=json", "-p", + `[{"op": "replace", "path": "/spec/subscription/source", "value": "operatorhubio-catalog"}]`) + + By("Patching the CatalogSource to reference a broken image link") + utils.Kubectl("patch", "catalogsource", catSrcName, "-n", "olm", "--type=json", "-p", + `[{"op": "replace", "path": "/spec/image", "value": "quay.io/operatorhubio/fakecatalog:latest"}]`) + + 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: catSrcName, + Namespace: opPolTestNS, + }, + }, + Compliant: "NonCompliant", + Reason: "Resource found as expected but is unhealthy", + }}, + metav1.Condition{ + Type: "CatalogSourcesUnhealthy", + Status: metav1.ConditionTrue, + Reason: "CatalogSourcesFoundUnhealthy", + Message: "CatalogSource was found but is unhealthy", + }, + "CatalogSource was found but is unhealthy", + ) + }) + }) }) diff --git a/test/resources/case38_operator_install/case38_OpPlc.yaml b/test/resources/case38_operator_install/case38_OpPlc.yaml deleted file mode 100644 index 4a643b12..00000000 --- a/test/resources/case38_operator_install/case38_OpPlc.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: op-1 - target: - namespaces: - - op-1 - subscription: - channel: stable-3.8 - name: project-quay - namespace: op-1 - 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 \ No newline at end of file diff --git a/test/resources/case38_operator_install/case38_OpPlc_default_og.yaml b/test/resources/case38_operator_install/case38_OpPlc_default_og.yaml deleted file mode 100644 index cd57496d..00000000 --- a/test/resources/case38_operator_install/case38_OpPlc_default_og.yaml +++ /dev/null @@ -1,28 +0,0 @@ -apiVersion: policy.open-cluster-management.io/v1beta1 -kind: OperatorPolicy -metadata: - name: test-operator-policy-no-og -spec: - remediationAction: enforce # or inform - severity: medium - complianceType: musthave # or mustnothave - subscription: - channel: stable-3.8 - name: project-quay - namespace: op-2 - 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 \ No newline at end of file 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