Fix status reporting inconsistency in mustnothave mode

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.

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
  }

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 {