diff --git a/controllers/configurationpolicy_controller.go b/controllers/configurationpolicy_controller.go index 539ef19c..7226b9bb 100644 --- a/controllers/configurationpolicy_controller.go +++ b/controllers/configurationpolicy_controller.go @@ -68,16 +68,16 @@ var ( ) const ( - reasonWantFoundExists = "Resource found as expected" - reasonWantFoundCreated = "K8s creation success" - reasonUpdateSuccess = "K8s update success" - reasonDeleteSuccess = "K8s deletion success" - reasonWantFoundNoMatch = "Resource found but does not match" - reasonWantFoundDNE = "Resource not found but should exist" - reasonWantNotFoundExists = "Resource found but should not exist" - reasonWantNotFoundDNE = "Resource not found as expected" - reasonCleanupError = "Error cleaning up child objects" - reasonIgnoreNotApplicable = "Resource found but will not be handled in mustnothave mode" + reasonWantFoundExists = "Resource found as expected" + reasonWantFoundCreated = "K8s creation success" + reasonUpdateSuccess = "K8s update success" + reasonDeleteSuccess = "K8s deletion success" + reasonWantFoundNoMatch = "Resource found but does not match" + reasonWantFoundDNE = "Resource not found but should exist" + reasonWantNotFoundExists = "Resource found but should not exist" + reasonWantNotFoundDNE = "Resource not found as expected" + reasonCleanupError = "Error cleaning up child objects" + reasonFoundNotApplicable = "Resource found but will not be handled in mustnothave mode" ) // SetupWithManager sets up the controller with the Manager. diff --git a/controllers/operatorpolicy_controller.go b/controllers/operatorpolicy_controller.go index 720d550b..570e4e3c 100644 --- a/controllers/operatorpolicy_controller.go +++ b/controllers/operatorpolicy_controller.go @@ -1515,14 +1515,14 @@ func (r *OperatorPolicyReconciler) handleDeployment( policy *policyv1beta1.OperatorPolicy, csv *operatorv1alpha1.ClusterServiceVersion, ) (bool, error) { - if policy.Spec.ComplianceType.IsMustNotHave() { - return updateStatus(policy, notApplicableCond("Deployment")), nil - } - // case where csv is nil if csv == nil { // need to report lack of existing Deployments - return updateStatus(policy, noDeploymentsCond, noExistingDeploymentObj), nil + if policy.Spec.ComplianceType.IsMustHave() { + return updateStatus(policy, noDeploymentsCond, noExistingDeploymentObj), nil + } + + return updateStatus(policy, notApplicableCond("Deployment")), nil } OpLog := ctrl.LoggerFrom(ctx) @@ -1542,7 +1542,8 @@ func (r *OperatorPolicyReconciler) handleDeployment( // report missing deployment in relatedObjects list if foundDep == nil { - relatedObjects = append(relatedObjects, missingDeploymentObj(dep.Name, csv.Namespace)) + relatedObjects = append(relatedObjects, + missingObj(dep.Name, csv.Namespace, policy.Spec.ComplianceType, deploymentGVK)) continue } @@ -1564,7 +1565,15 @@ func (r *OperatorPolicyReconciler) handleDeployment( depNum++ - relatedObjects = append(relatedObjects, existingDeploymentObj(&dep)) + if policy.Spec.ComplianceType.IsMustNotHave() { + relatedObjects = append(relatedObjects, foundNotApplicableObj(&dep)) + } else { + relatedObjects = append(relatedObjects, existingDeploymentObj(&dep)) + } + } + + if policy.Spec.ComplianceType.IsMustNotHave() { + return updateStatus(policy, notApplicableCond("Deployment"), relatedObjects...), nil } return updateStatus(policy, buildDeploymentCond(depNum > 0, unavailableDeployments), relatedObjects...), nil @@ -1668,18 +1677,20 @@ func (r *OperatorPolicyReconciler) handleCatalogSource( policy *policyv1beta1.OperatorPolicy, subscription *operatorv1alpha1.Subscription, ) (bool, error) { - if policy.Spec.ComplianceType.IsMustNotHave() { - cond := notApplicableCond("CatalogSource") - cond.Status = metav1.ConditionFalse // CatalogSource condition has the opposite polarity - - return updateStatus(policy, cond), nil - } - watcher := opPolIdentifier(policy.Namespace, policy.Name) if subscription == nil { // Note: existing related objects will not be removed by this status update - return updateStatus(policy, invalidCausingUnknownCond("CatalogSource")), nil + if policy.Spec.ComplianceType.IsMustHave() { + return updateStatus(policy, invalidCausingUnknownCond("CatalogSource")), nil + } + + // CatalogSource may be available + // related objects will remain the same to report last known state + cond := notApplicableCond("CatalogSource") + cond.Status = metav1.ConditionFalse + + return updateStatus(policy, cond), nil } catalogName := subscription.Spec.CatalogSource @@ -1692,22 +1703,64 @@ func (r *OperatorPolicyReconciler) handleCatalogSource( return false, fmt.Errorf("error getting CatalogSource: %w", err) } - isMissing := foundCatalogSrc == nil - isUnhealthy := isMissing + var catalogSrc *operatorv1alpha1.CatalogSource - if !isMissing { + if foundCatalogSrc != nil { // CatalogSource is found, initiate health check - catalogSrc := new(operatorv1alpha1.CatalogSource) + catalogSrc = new(operatorv1alpha1.CatalogSource) err := runtime.DefaultUnstructuredConverter. FromUnstructured(foundCatalogSrc.Object, catalogSrc) if err != nil { return false, fmt.Errorf("error converting the retrieved CatalogSource to the Go type: %w", err) } + } + + if policy.Spec.ComplianceType.IsMustNotHave() { + return r.mustnothaveCatalogSource(policy, catalogSrc, catalogName, catalogNS) + } + + return r.musthaveCatalogSource(policy, catalogSrc, catalogName, catalogNS) +} + +func (r *OperatorPolicyReconciler) mustnothaveCatalogSource( + policy *policyv1beta1.OperatorPolicy, + catalogSrc *operatorv1alpha1.CatalogSource, + catalogName string, + catalogNS string, +) (bool, error) { + var relObj policyv1.RelatedObject + + cond := notApplicableCond("CatalogSource") + cond.Status = metav1.ConditionFalse // CatalogSource condition has the opposite polarity + if catalogSrc == nil { + relObj = missingObj(catalogName, catalogNS, policyv1.MustNotHave, catalogSrcGVK) + } else { + relObj = foundNotApplicableObj(catalogSrc) + } + + return updateStatus(policy, cond, relObj), nil +} + +func (r *OperatorPolicyReconciler) musthaveCatalogSource( + policy *policyv1beta1.OperatorPolicy, + catalogSrc *operatorv1alpha1.CatalogSource, + catalogName string, + catalogNS string, +) (bool, error) { + isMissing := catalogSrc == nil + isUnhealthy := isMissing + + if catalogSrc != nil { + // CatalogSource is found, initiate health check if catalogSrc.Status.GRPCConnectionState == nil { // Unknown State - changed := updateStatus(policy, catalogSourceUnknownCond, catalogSrcUnknownObj(catalogName, catalogNS)) + changed := updateStatus( + policy, + catalogSourceUnknownCond, + catalogSrcUnknownObj(catalogSrc.Name, catalogSrc.Namespace), + ) return changed, nil } diff --git a/controllers/operatorpolicy_status.go b/controllers/operatorpolicy_status.go index f413390e..053c7576 100644 --- a/controllers/operatorpolicy_status.go +++ b/controllers/operatorpolicy_status.go @@ -12,6 +12,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/controller-runtime/pkg/client" policyv1 "open-cluster-management.io/config-policy-controller/api/v1" @@ -856,19 +857,55 @@ func foundNotWantedObj(obj client.Object) policyv1.RelatedObject { } } -// foundNotWantedIpObj returns a Compliant RelatedObject with +// foundNotApplicableObj returns a Compliant RelatedObject with // reason = 'Resource found but will not be handled in mustnothave mode' func foundNotApplicableObj(obj client.Object) policyv1.RelatedObject { return policyv1.RelatedObject{ Object: policyv1.ObjectResourceFromObj(obj), Compliant: string(policyv1.Compliant), - Reason: reasonIgnoreNotApplicable, + Reason: reasonFoundNotApplicable, Properties: &policyv1.ObjectProperties{ UID: string(obj.GetUID()), }, } } +// missingObj returns a conditionally Compliant RelatedObject with +// reason = "Resource not found but should exist"/"Resource not found as expected" +// based on the complianceType specified by the policy +func missingObj( + name string, + namespace string, + complianceType policyv1.ComplianceType, + gvk schema.GroupVersionKind, +) policyv1.RelatedObject { + var compliance policyv1.ComplianceState + var reason string + + if complianceType.IsMustHave() { + compliance = policyv1.NonCompliant + reason = reasonWantFoundDNE + } else { + // Non-Applicables are not handled by controller + // However if the're not found -> report NA or report not found as expected? + compliance = policyv1.Compliant + reason = reasonWantNotFoundDNE + } + + return policyv1.RelatedObject{ + Object: policyv1.ObjectResource{ + Kind: gvk.Kind, + APIVersion: gvk.GroupVersion().String(), + Metadata: policyv1.ObjectMetadata{ + Name: name, + Namespace: namespace, + }, + }, + Compliant: string(compliance), + Reason: reason, + } +} + // createdObj returns a Compliant RelatedObject with reason = 'K8s creation success' func createdObj(obj client.Object) policyv1.RelatedObject { created := true @@ -1116,23 +1153,6 @@ var noExistingCRDObj = policyv1.RelatedObject{ 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 { - return policyv1.RelatedObject{ - Object: policyv1.ObjectResource{ - Kind: deploymentGVK.Kind, - APIVersion: deploymentGVK.GroupVersion().String(), - Metadata: policyv1.ObjectMetadata{ - Name: name, - Namespace: namespace, - }, - }, - Compliant: string(policyv1.NonCompliant), - Reason: reasonWantFoundDNE, - } -} - // existingDeploymentObj returns a RelatedObject for a Deployment, which will // be Compliant if there are no unavailable replicas on the deployment. func existingDeploymentObj(dep *appsv1.Deployment) policyv1.RelatedObject { diff --git a/test/e2e/case38_install_operator_test.go b/test/e2e/case38_install_operator_test.go index 91b9b1a5..bde53355 100644 --- a/test/e2e/case38_install_operator_test.go +++ b/test/e2e/case38_install_operator_test.go @@ -1565,9 +1565,12 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { }) Describe("Testing general OperatorPolicy mustnothave behavior", Ordered, func() { const ( - opPolYAML = "../resources/case38_operator_install/operator-policy-mustnothave.yaml" - opPolName = "oppol-mustnothave" - subName = "project-quay" + opPolYAML = "../resources/case38_operator_install/operator-policy-mustnothave.yaml" + opPolName = "oppol-mustnothave" + subName = "project-quay" + deploymentName = "quay-operator-tng" + catSrcName = "operatorhubio-catalog" + catSrcNS = "olm" ) BeforeAll(func() { @@ -1865,6 +1868,52 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { }, `the CustomResourceDefinition is present`, ) + check( + opPolName, + true, + []policyv1.RelatedObject{{ + Object: policyv1.ObjectResource{ + Kind: "Deployment", + APIVersion: "apps/v1", + Metadata: policyv1.ObjectMetadata{ + Name: deploymentName, + Namespace: opPolTestNS, + }, + }, + Compliant: "Compliant", + Reason: "Resource found but will not be handled in mustnothave mode", + }}, + metav1.Condition{ + Type: "DeploymentCompliant", + Status: metav1.ConditionTrue, + Reason: "DeploymentNotApplicable", + Message: "MustNotHave policies ignore kind Deployment", + }, + `MustNotHave policies ignore kind Deployment`, + ) + check( + opPolName, + false, + []policyv1.RelatedObject{{ + Object: policyv1.ObjectResource{ + Kind: "CatalogSource", + APIVersion: "operators.coreos.com/v1alpha1", + Metadata: policyv1.ObjectMetadata{ + Name: catSrcName, + Namespace: catSrcNS, + }, + }, + Compliant: "Compliant", + Reason: "Resource found but will not be handled in mustnothave mode", + }}, + metav1.Condition{ + Type: "CatalogSourcesUnhealthy", + Status: metav1.ConditionFalse, + Reason: "CatalogSourceNotApplicable", + Message: "MustNotHave policies ignore kind CatalogSource", + }, + `MustNotHave policies ignore kind CatalogSource`, + ) }) // These are the same for inform and enforce, so just write them once @@ -1966,6 +2015,52 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { `{"op": "replace", "path": "/spec/removalBehavior/customResourceDefinitions", "value": "Keep"}]`) By("Checking the OperatorPolicy status") keptChecks() + check( + opPolName, + false, + []policyv1.RelatedObject{{ + Object: policyv1.ObjectResource{ + Kind: "Deployment", + APIVersion: "apps/v1", + Metadata: policyv1.ObjectMetadata{ + Name: deploymentName, + Namespace: opPolTestNS, + }, + }, + Compliant: "Compliant", + Reason: "Resource found but will not be handled in mustnothave mode", + }}, + metav1.Condition{ + Type: "DeploymentCompliant", + Status: metav1.ConditionTrue, + Reason: "DeploymentNotApplicable", + Message: "MustNotHave policies ignore kind Deployment", + }, + `MustNotHave policies ignore kind Deployment`, + ) + check( + opPolName, + false, + []policyv1.RelatedObject{{ + Object: policyv1.ObjectResource{ + Kind: "CatalogSource", + APIVersion: "operators.coreos.com/v1alpha1", + Metadata: policyv1.ObjectMetadata{ + Name: catSrcName, + Namespace: catSrcNS, + }, + }, + Compliant: "Compliant", + Reason: "Resource found but will not be handled in mustnothave mode", + }}, + metav1.Condition{ + Type: "CatalogSourcesUnhealthy", + Status: metav1.ConditionFalse, + Reason: "CatalogSourceNotApplicable", + Message: "MustNotHave policies ignore kind CatalogSource", + }, + `MustNotHave policies ignore kind CatalogSource`, + ) }) It("Should not remove anything when enforced while set to Keep everything", func(ctx SpecContext) { // Enforce the policy @@ -1973,6 +2068,52 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { `[{"op": "replace", "path": "/spec/remediationAction", "value": "enforce"}]`) By("Checking the OperatorPolicy status") keptChecks() + check( + opPolName, + false, + []policyv1.RelatedObject{{ + Object: policyv1.ObjectResource{ + Kind: "Deployment", + APIVersion: "apps/v1", + Metadata: policyv1.ObjectMetadata{ + Name: deploymentName, + Namespace: opPolTestNS, + }, + }, + Compliant: "Compliant", + Reason: "Resource found but will not be handled in mustnothave mode", + }}, + metav1.Condition{ + Type: "DeploymentCompliant", + Status: metav1.ConditionTrue, + Reason: "DeploymentNotApplicable", + Message: "MustNotHave policies ignore kind Deployment", + }, + `MustNotHave policies ignore kind Deployment`, + ) + check( + opPolName, + false, + []policyv1.RelatedObject{{ + Object: policyv1.ObjectResource{ + Kind: "CatalogSource", + APIVersion: "operators.coreos.com/v1alpha1", + Metadata: policyv1.ObjectMetadata{ + Name: catSrcName, + Namespace: catSrcNS, + }, + }, + Compliant: "Compliant", + Reason: "Resource found but will not be handled in mustnothave mode", + }}, + metav1.Condition{ + Type: "CatalogSourcesUnhealthy", + Status: metav1.ConditionFalse, + Reason: "CatalogSourceNotApplicable", + Message: "MustNotHave policies ignore kind CatalogSource", + }, + `MustNotHave policies ignore kind CatalogSource`, + ) By("Checking that certain (named) resources are still there") utils.GetWithTimeout(clientManagedDynamic, gvrClusterServiceVersion, "quay-operator.v3.10.0", @@ -2294,6 +2435,52 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { }, `the CustomResourceDefinition was deleted`, ) + check( + opPolName, + false, + []policyv1.RelatedObject{{ + Object: policyv1.ObjectResource{ + Kind: "Deployment", + APIVersion: "apps/v1", + Metadata: policyv1.ObjectMetadata{ + Name: deploymentName, + Namespace: opPolTestNS, + }, + }, + Compliant: "Compliant", + Reason: "Resource found but will not be handled in mustnothave mode", + }}, + metav1.Condition{ + Type: "DeploymentCompliant", + Status: metav1.ConditionTrue, + Reason: "DeploymentNotApplicable", + Message: "MustNotHave policies ignore kind Deployment", + }, + `MustNotHave policies ignore kind Deployment`, + ) + check( + opPolName, + false, + []policyv1.RelatedObject{{ + Object: policyv1.ObjectResource{ + Kind: "CatalogSource", + APIVersion: "operators.coreos.com/v1alpha1", + Metadata: policyv1.ObjectMetadata{ + Name: catSrcName, + Namespace: catSrcNS, + }, + }, + Compliant: "Compliant", + Reason: "Resource found but will not be handled in mustnothave mode", + }}, + metav1.Condition{ + Type: "CatalogSourcesUnhealthy", + Status: metav1.ConditionFalse, + Reason: "CatalogSourceNotApplicable", + Message: "MustNotHave policies ignore kind CatalogSource", + }, + `MustNotHave policies ignore kind CatalogSource`, + ) // the checks don't verify that the policy is compliant, do that now: pol := utils.GetWithTimeout(clientManagedDynamic, gvrOperatorPolicy, opPolName,