diff --git a/api/v1beta1/operatorpolicy_types.go b/api/v1beta1/operatorpolicy_types.go index 9d16f6e9..853c1ab4 100644 --- a/api/v1beta1/operatorpolicy_types.go +++ b/api/v1beta1/operatorpolicy_types.go @@ -12,19 +12,20 @@ import ( policyv1 "open-cluster-management.io/config-policy-controller/api/v1" ) -// StatusConfigAction : StatusMessageOnly or NonCompliant -// +kubebuilder:validation:Enum=StatusMessageOnly;NonCompliant -type StatusConfigAction string +// ComplianceConfigAction : Compliant or NonCompliant +// +kubebuilder:validation:Enum=Compliant;NonCompliant +type ComplianceConfigAction string // RemovalAction : Keep, Delete, or DeleteIfUnused type RemovalAction string const ( - // StatusMessageOnly is a StatusConfigAction that only shows the status message - StatusMessageOnly StatusConfigAction = "StatusMessageOnly" - // NonCompliant is a StatusConfigAction that shows the status message and sets - // the compliance to NonCompliant - NonCompliant StatusConfigAction = "NonCompliant" + // Compliant is a ComplianceConfigAction that only shows the status message and + // does not affect the overall compliance + Compliant ComplianceConfigAction = "Compliant" + // NonCompliant is a ComplianceConfigAction that shows the status message and sets + // the overall compliance when the condition is met + NonCompliant ComplianceConfigAction = "NonCompliant" ) const ( @@ -98,12 +99,20 @@ func (rb RemovalBehavior) ApplyDefaults() RemovalBehavior { return withDefaults } -// StatusConfig defines how resource statuses affect the OperatorPolicy status and compliance -type StatusConfig struct { - CatalogSourceUnhealthy StatusConfigAction `json:"catalogSourceUnhealthy,omitempty"` - DeploymentsUnavailable StatusConfigAction `json:"deploymentsUnavailable,omitempty"` - UpgradesAvailable StatusConfigAction `json:"upgradesAvailable,omitempty"` - UpgradesProgressing StatusConfigAction `json:"upgradesProgressing,omitempty"` +// ComplianceConfig defines how resource statuses affect the OperatorPolicy compliance +type ComplianceConfig struct { + //+kubebuilder:default=Compliant + // Specifies how the CatalogSourceUnhealthy typed condition should affect + // overall policy compliance. Defaults to 'Compliant' + CatalogSourceUnhealthy ComplianceConfigAction `json:"catalogSourceUnhealthy,omitempty"` + //+kubebuilder:default=NonCompliant + // Specifies how the DeploymentCompliant typed condition should affect + // overall policy compliance. Defaults to 'NonCompliant' + DeploymentsUnavailable ComplianceConfigAction `json:"deploymentsUnavailable,omitempty"` + //+kubebuilder:default=Compliant + // Specifies how the InstallPlanCompliant typed condition should affect + // overall policy compliance. Defaults to 'Compliant' + UpgradesAvailable ComplianceConfigAction `json:"upgradesAvailable,omitempty"` } // OperatorPolicySpec defines the desired state of OperatorPolicy @@ -115,15 +124,15 @@ type OperatorPolicySpec struct { // Include the name, namespace, and any `spec` fields for the OperatorGroup. // For more info, see `kubectl explain operatorgroup.spec` or // https://olm.operatorframework.io/docs/concepts/crds/operatorgroup/ - // +kubebuilder:pruning:PreserveUnknownFields - // +optional + //+kubebuilder:pruning:PreserveUnknownFields + //+optional OperatorGroup *runtime.RawExtension `json:"operatorGroup,omitempty"` // Include the namespace, and any `spec` fields for the Subscription. // For more info, see `kubectl explain subscription.spec` or // https://olm.operatorframework.io/docs/concepts/crds/subscription/ - // +kubebuilder:validation:Required - // +kubebuilder:pruning:PreserveUnknownFields + //+kubebuilder:validation:Required + //+kubebuilder:pruning:PreserveUnknownFields Subscription runtime.RawExtension `json:"subscription"` // Versions is a list of nonempty strings that specifies which installed versions are compliant when @@ -143,6 +152,12 @@ type OperatorPolicySpec struct { // approval is not affected by this setting. This setting has no effect when the policy is in // 'mustnothave' mode. Allowed values are "None" or "Automatic". UpgradeApproval string `json:"upgradeApproval"` + + //+kubebuilder:default={} + // ComplianceConfig defines how resource statuses affect the OperatorPolicy status and compliance. + // When set to Compliant, the condition does not impact the OperatorPolicy compliance. + // When set to NonCompliant, the condition causes the OperatorPolicy to become NonCompliant. + ComplianceConfig ComplianceConfig `json:"complianceConfig,omitempty"` } // OperatorPolicyStatus defines the observed state of OperatorPolicy diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index b0faad9e..76c5d43c 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -13,6 +13,21 @@ import ( "open-cluster-management.io/config-policy-controller/api/v1" ) +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ComplianceConfig) DeepCopyInto(out *ComplianceConfig) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ComplianceConfig. +func (in *ComplianceConfig) DeepCopy() *ComplianceConfig { + if in == nil { + return nil + } + out := new(ComplianceConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *OperatorPolicy) DeepCopyInto(out *OperatorPolicy) { *out = *in @@ -87,6 +102,7 @@ func (in *OperatorPolicySpec) DeepCopyInto(out *OperatorPolicySpec) { copy(*out, *in) } out.RemovalBehavior = in.RemovalBehavior + out.ComplianceConfig = in.ComplianceConfig } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OperatorPolicySpec. @@ -147,18 +163,3 @@ func (in *RemovalBehavior) DeepCopy() *RemovalBehavior { in.DeepCopyInto(out) return out } - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *StatusConfig) DeepCopyInto(out *StatusConfig) { - *out = *in -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new StatusConfig. -func (in *StatusConfig) DeepCopy() *StatusConfig { - if in == nil { - return nil - } - out := new(StatusConfig) - in.DeepCopyInto(out) - return out -} diff --git a/controllers/operatorpolicy_controller.go b/controllers/operatorpolicy_controller.go index bdffa234..58a12c75 100644 --- a/controllers/operatorpolicy_controller.go +++ b/controllers/operatorpolicy_controller.go @@ -1326,6 +1326,7 @@ func (r *OperatorPolicyReconciler) musthaveInstallPlan( ipsRequiringApproval := make([]unstructured.Unstructured, 0) anyInstalling := false currentPlanFailed := false + complianceConfig := policy.Spec.ComplianceConfig.UpgradesAvailable // Construct the relevant relatedObjects, and collect any that might be considered for approval for i, installPlan := range ownedInstallPlans { @@ -1356,7 +1357,8 @@ func (r *OperatorPolicyReconciler) musthaveInstallPlan( } } - relatedInstallPlans = append(relatedInstallPlans, existingInstallPlanObj(&ownedInstallPlans[i], phase)) + relatedInstallPlans = append(relatedInstallPlans, + existingInstallPlanObj(&ownedInstallPlans[i], phase, complianceConfig)) } if currentPlanFailed { @@ -1396,9 +1398,8 @@ func (r *OperatorPolicyReconciler) musthaveInstallPlan( // Only report this status when not approving an InstallPlan, because otherwise it could easily // oscillate between this and another condition. if policy.Spec.RemediationAction.IsInform() || (!initialInstall && !autoUpgrade) { - // FUTURE: check policy.spec.statusConfig.upgradesAvailable to determine `compliant`. - // For now this condition assumes it is set to 'NonCompliant' - return updateStatus(policy, installPlanUpgradeCond(allUpgradeVersions, nil), relatedInstallPlans...), nil + return updateStatus(policy, installPlanUpgradeCond(complianceConfig, allUpgradeVersions, nil), + relatedInstallPlans...), nil } approvedVersion := "" // this will only be accurate when there is only one approvable InstallPlan @@ -1446,8 +1447,11 @@ func (r *OperatorPolicyReconciler) musthaveInstallPlan( } if len(approvableInstallPlans) != 1 { - changed := updateStatus(policy, - installPlanUpgradeCond(allUpgradeVersions, approvableInstallPlans), relatedInstallPlans...) + changed := updateStatus( + policy, + installPlanUpgradeCond(complianceConfig, allUpgradeVersions, approvableInstallPlans), + relatedInstallPlans..., + ) return changed, nil } @@ -1644,6 +1648,7 @@ func (r *OperatorPolicyReconciler) handleDeployment( var relatedObjects []policyv1.RelatedObject var unavailableDeployments []appsv1.Deployment + complianceConfig := policy.Spec.ComplianceConfig.DeploymentsUnavailable depNum := 0 for _, dep := range csv.Spec.InstallStrategy.StrategySpec.DeploymentSpecs { @@ -1680,7 +1685,7 @@ func (r *OperatorPolicyReconciler) handleDeployment( if policy.Spec.ComplianceType.IsMustNotHave() { relatedObjects = append(relatedObjects, foundNotApplicableObj(&dep)) } else { - relatedObjects = append(relatedObjects, existingDeploymentObj(&dep)) + relatedObjects = append(relatedObjects, existingDeploymentObj(&dep, complianceConfig)) } } @@ -1688,7 +1693,8 @@ func (r *OperatorPolicyReconciler) handleDeployment( return updateStatus(policy, notApplicableCond("Deployment"), relatedObjects...), nil } - return updateStatus(policy, buildDeploymentCond(depNum > 0, unavailableDeployments), relatedObjects...), nil + return updateStatus(policy, buildDeploymentCond(complianceConfig, depNum > 0, unavailableDeployments), + relatedObjects...), nil } func (r *OperatorPolicyReconciler) handleCRDs( @@ -1888,8 +1894,9 @@ func (r *OperatorPolicyReconciler) musthaveCatalogSource( isUnhealthy = (CatalogSrcState != CatalogSourceReady) } - changed := updateStatus(policy, catalogSourceFindCond(isUnhealthy, isMissing, catalogName), - catalogSourceObj(catalogName, catalogNS, isUnhealthy, isMissing)) + complianceConfig := policy.Spec.ComplianceConfig.CatalogSourceUnhealthy + changed := updateStatus(policy, catalogSourceFindCond(complianceConfig, isUnhealthy, isMissing, catalogName), + catalogSourceObj(catalogName, catalogNS, isUnhealthy, isMissing, complianceConfig)) return changed, nil } diff --git a/controllers/operatorpolicy_status.go b/controllers/operatorpolicy_status.go index 9ef2f0e5..657eb2f7 100644 --- a/controllers/operatorpolicy_status.go +++ b/controllers/operatorpolicy_status.go @@ -211,8 +211,10 @@ func calculateComplianceCondition(policy *policyv1beta1.OperatorPolicy) metav1.C } idx, cond = policy.Status.GetCondition(installPlanConditionType) + if idx == -1 { messages = append(messages, "the status of the InstallPlan is unknown") + foundNonCompliant = true } else { messages = append(messages, cond.Message) @@ -247,8 +249,10 @@ func calculateComplianceCondition(policy *policyv1beta1.OperatorPolicy) metav1.C } idx, cond = policy.Status.GetCondition(deploymentConditionType) + if idx == -1 { messages = append(messages, "the status of the Deployments are unknown") + foundNonCompliant = true } else { messages = append(messages, cond.Message) @@ -259,8 +263,10 @@ func calculateComplianceCondition(policy *policyv1beta1.OperatorPolicy) metav1.C } 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) @@ -668,13 +674,16 @@ var installPlansNoApprovals = metav1.Condition{ } // installPlanUpgradeCond is a NonCompliant condition with Reason 'InstallPlanRequiresApproval' -// and a message detailing which possible updates are available -func installPlanUpgradeCond(versions []string, approvableIPs []unstructured.Unstructured) metav1.Condition { - // FUTURE: check policy.spec.statusConfig.upgradesAvailable to determine `compliant`. - // For now this condition assumes it is set to 'NonCompliant' +// and a message detailing which possible updates are available. The `complianceConfig` parameter +// determines whether the existence of available upgrades should result in NonCompliance when the +// policy status is updated. +func installPlanUpgradeCond( + complianceConfig policyv1beta1.ComplianceConfigAction, + versions []string, + approvableIPs []unstructured.Unstructured, +) metav1.Condition { cond := metav1.Condition{ Type: installPlanConditionType, - Status: metav1.ConditionFalse, Reason: "InstallPlanRequiresApproval", } @@ -693,6 +702,12 @@ func installPlanUpgradeCond(versions []string, approvableIPs []unstructured.Unst cond.Message += " but multiple of those match the versions specified in the policy" } + if complianceConfig == "Compliant" { + cond.Status = metav1.ConditionTrue + } else { + cond.Status = metav1.ConditionFalse + } + return cond } @@ -748,8 +763,11 @@ var crdFoundCond = metav1.Condition{ // 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. +// list the unavailable deployments. The `complianceConfig` parameter determines +// whether an unavailable deployment will lead to NonCompliance when the policy +// status is updated. func buildDeploymentCond( + complianceConfig policyv1beta1.ComplianceConfigAction, depsExist bool, unavailableDeps []appsv1.Deployment, ) metav1.Condition { @@ -775,6 +793,12 @@ func buildDeploymentCond( message = fmt.Sprintf("the deployments %s do not have their minimum availability", names) } + // If the status changed, that means the condition should be NonCompliant + // Apply override if complianceConfig is set to Compliant + if status == metav1.ConditionFalse && complianceConfig == "Compliant" { + status = metav1.ConditionTrue + } + return metav1.Condition{ Type: condType(deploymentGVK.Kind), Status: status, @@ -793,8 +817,15 @@ var noDeploymentsCond = metav1.Condition{ } // catalogSourceFindCond is a conditionally compliant condition with reason -// based on the `isUnhealthy` and `isMissing` parameters -func catalogSourceFindCond(isUnhealthy bool, isMissing bool, name string) metav1.Condition { +// based on the `isUnhealthy` and `isMissing` parameters. The `complianceConfig` +// parameter determines whether an unhealthy CatalogSource should lead to +// NonCompliance when status is updated. +func catalogSourceFindCond( + complianceConfig policyv1beta1.ComplianceConfigAction, + isUnhealthy bool, + isMissing bool, + name string, +) metav1.Condition { status := metav1.ConditionFalse reason := "CatalogSourcesFound" message := "CatalogSource was found" @@ -811,6 +842,11 @@ func catalogSourceFindCond(isUnhealthy bool, isMissing bool, name string) metav1 message = "CatalogSource '" + name + "' was not found" } + // Only override if condition evaluated to NonCompliant + if status == metav1.ConditionTrue && complianceConfig == "Compliant" { + status = metav1.ConditionFalse + } + return metav1.Condition{ Type: "CatalogSourcesUnhealthy", Status: status, @@ -1044,7 +1080,13 @@ func noInstallPlansObj(namespace string) policyv1.RelatedObject { // existingInstallPlanObj returns a RelatedObject for the InstallPlan, with a reason // like 'The InstallPlan is ____' based on the phase. When the InstallPlan is in phase // 'Complete', the object will be Compliant, otherwise it will be NonCompliant. -func existingInstallPlanObj(ip client.Object, phase string) policyv1.RelatedObject { +// The `complianceConfig` parameter determines whether the existence of available upgrades +// or installing phase should result in NonCompliance when the policy status is updated. +func existingInstallPlanObj( + ip client.Object, + phase string, + complianceConfig policyv1beta1.ComplianceConfigAction, +) policyv1.RelatedObject { relObj := policyv1.RelatedObject{ Object: policyv1.ObjectResourceFromObj(ip), Properties: &policyv1.ObjectProperties{ @@ -1059,10 +1101,20 @@ func existingInstallPlanObj(ip client.Object, phase string) policyv1.RelatedObje } switch phase { + case string(operatorv1alpha1.InstallPlanPhaseInstalling): + // Check policy.spec.statusConfig.upgradesAvailable to determine `compliant`. + if complianceConfig != "Compliant" { + relObj.Compliant = string(policyv1.NonCompliant) + } else { + relObj.Compliant = string(policyv1.Compliant) + } case string(operatorv1alpha1.InstallPlanPhaseRequiresApproval): - // FUTURE: check policy.spec.statusConfig.upgradesAvailable to determine `compliant`. - // For now, assume it is set to 'NonCompliant' - relObj.Compliant = string(policyv1.NonCompliant) + // Check policy.spec.statusConfig.upgradesAvailable to determine `compliant`. + if complianceConfig != "Compliant" { + relObj.Compliant = string(policyv1.NonCompliant) + } else { + relObj.Compliant = string(policyv1.Compliant) + } case string(operatorv1alpha1.InstallPlanPhaseComplete): relObj.Compliant = string(policyv1.Compliant) default: @@ -1154,14 +1206,23 @@ var noExistingCRDObj = policyv1.RelatedObject{ } // 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 { +// be Compliant if there are no unavailable replicas on the deployment. The +// `complianceConfig` parameter determines whether an unavailable deployment +// results in NonCompliance when the policy status is updated. +func existingDeploymentObj( + dep *appsv1.Deployment, + complianceConfig policyv1beta1.ComplianceConfigAction, +) policyv1.RelatedObject { compliance := policyv1.NonCompliant reason := "Deployment Unavailable" if dep.Status.UnavailableReplicas == 0 { compliance = policyv1.Compliant reason = "Deployment Available" + } else if complianceConfig == "Compliant" { + compliance = policyv1.Compliant + // Notify user since complianceConfig was changed from NonCompliant -> Compliant + reason += " (policy compliance is not impacted due to spec.complianceConfig.deploymentsUnavailable)" } return policyv1.RelatedObject{ @@ -1187,20 +1248,34 @@ var noExistingDeploymentObj = policyv1.RelatedObject{ Reason: "No relevant deployments found", } -// 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 { +// catalogSourceObj returns a conditionally compliant RelatedObject with reason +// based on the `isUnhealthy` and `isMissing` parameters. The `complianceConfig` +// parameter determines whether an unhealthy CatalogSource should lead to +// NonCompliance when status is updated. +func catalogSourceObj( + catalogName string, + catalogNS string, + isUnhealthy bool, + isMissing bool, + complianceConfig policyv1beta1.ComplianceConfigAction, +) policyv1.RelatedObject { compliance := string(policyv1.Compliant) reason := reasonWantFoundExists if isUnhealthy { - compliance = string(policyv1.NonCompliant) reason = reasonWantFoundExists + " but is unhealthy" + + if complianceConfig != "Compliant" { + compliance = string(policyv1.NonCompliant) + } } if isMissing { - compliance = string(policyv1.NonCompliant) reason = reasonWantFoundDNE + + if complianceConfig != "Compliant" { + compliance = string(policyv1.NonCompliant) + } } return policyv1.RelatedObject{ diff --git a/controllers/operatorpolicy_status_test.go b/controllers/operatorpolicy_status_test.go new file mode 100644 index 00000000..be87f897 --- /dev/null +++ b/controllers/operatorpolicy_status_test.go @@ -0,0 +1,190 @@ +package controllers + +import ( + "testing" + + operatorv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1" + "github.com/stretchr/testify/assert" + appsv1 "k8s.io/api/apps/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + policyv1beta1 "open-cluster-management.io/config-policy-controller/api/v1beta1" +) + +func TestExistingInstallPlanObj(t *testing.T) { + // Empty InstallPlan + testIP := &operatorv1alpha1.InstallPlan{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{}, + Spec: operatorv1alpha1.InstallPlanSpec{}, + Status: operatorv1alpha1.InstallPlanStatus{}, + } + + // Test available upgrades with Compliant complianceConfig + phase := "RequiresApproval" + complianceConfig := policyv1beta1.ComplianceConfigAction("Compliant") + res := existingInstallPlanObj(testIP, phase, complianceConfig) + assert.Equal(t, "Compliant", res.Compliant) + + // Test available upgrades with NonCompliant complianceConfig + phase = "RequiresApproval" + complianceConfig = "NonCompliant" + res = existingInstallPlanObj(testIP, phase, complianceConfig) + assert.Equal(t, "NonCompliant", res.Compliant) + + // Test installing phase with Compliant complianceConfig + phase = "Installing" + complianceConfig = policyv1beta1.ComplianceConfigAction("Compliant") + res = existingInstallPlanObj(testIP, phase, complianceConfig) + assert.Equal(t, "Compliant", res.Compliant) + + // Test installing phase with NonCompliant complianceConfig + phase = "Installing" + complianceConfig = "NonCompliant" + res = existingInstallPlanObj(testIP, phase, complianceConfig) + assert.Equal(t, "NonCompliant", res.Compliant) +} + +func TestBuildDeploymentCond(t *testing.T) { + complianceConfig := policyv1beta1.Compliant + depsExist := true // if any deployments + unavailableDeps := make([]appsv1.Deployment, 0, 1) // deployments are there, but no available + + // Test Compliant complianceConfig with available deployments + cond := buildDeploymentCond(complianceConfig, depsExist, unavailableDeps) + assert.Equal(t, metav1.ConditionTrue, cond.Status) + + // Test NonCompliant complianceConfig with available deployments + complianceConfig = policyv1beta1.NonCompliant + cond = buildDeploymentCond(complianceConfig, depsExist, unavailableDeps) + assert.Equal(t, metav1.ConditionTrue, cond.Status) + assert.Equal(t, "all operator Deployments have their minimum availability", cond.Message) + + // Test Compliant complianceConfig with unavailable deployments + testDeployment := appsv1.Deployment{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Name: "TestDeployment", + }, + Spec: appsv1.DeploymentSpec{}, + Status: appsv1.DeploymentStatus{ + UnavailableReplicas: 1, + }, + } + complianceConfig = policyv1beta1.Compliant + + unavailableDeps = append(unavailableDeps, testDeployment) + cond = buildDeploymentCond(complianceConfig, depsExist, unavailableDeps) + assert.Equal(t, metav1.ConditionTrue, cond.Status) + + // Test NonCompliance complianceConfig with unavailable deployments + complianceConfig = policyv1beta1.NonCompliant + cond = buildDeploymentCond(complianceConfig, depsExist, unavailableDeps) + assert.Equal(t, metav1.ConditionFalse, cond.Status) + assert.Equal(t, "the deployments TestDeployment do not have their minimum availability", cond.Message) +} + +func TestExistingDeploymentObj(t *testing.T) { + testDeployment := &appsv1.Deployment{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{}, + Spec: appsv1.DeploymentSpec{}, + Status: appsv1.DeploymentStatus{ + UnavailableReplicas: 1, + }, + } + + // Test Compliant complianceConfig with UnavailableReplicas > 0 + complianceConfig := policyv1beta1.ComplianceConfigAction("Compliant") + res := existingDeploymentObj(testDeployment, complianceConfig) + assert.Equal(t, "Compliant", res.Compliant) + assert.Equal(t, "Deployment Unavailable"+ + " (policy compliance is not impacted due to spec.complianceConfig.deploymentsUnavailable)", res.Reason) + + // Test Compliant complianceConfig with UnavailableReplicas = 0 + complianceConfig = "Compliant" + testDeployment.Status.UnavailableReplicas = 0 + res = existingDeploymentObj(testDeployment, complianceConfig) + assert.Equal(t, "Compliant", res.Compliant) + assert.Equal(t, "Deployment Available", res.Reason) + + // Test NonCompliant complianceConfig with UnavailableReplicas > 0 + complianceConfig = "NonCompliant" + testDeployment.Status.UnavailableReplicas = 1 + res = existingDeploymentObj(testDeployment, complianceConfig) + assert.Equal(t, "NonCompliant", res.Compliant) + assert.Equal(t, "Deployment Unavailable", res.Reason) + + // Test NonCompliant complianceConfig with UnavailableReplicas = 0 + complianceConfig = "NonCompliant" + testDeployment.Status.UnavailableReplicas = 0 + res = existingDeploymentObj(testDeployment, complianceConfig) + assert.Equal(t, "Compliant", res.Compliant) + assert.Equal(t, "Deployment Available", res.Reason) +} + +func TestCatalogSourceFindCond(t *testing.T) { + complianceConfig := policyv1beta1.Compliant + isUnhealthy := false + isMissing := false + name := "TestCatalog" + + // Test Compliant complianceConfig with healthy CatalogSource + cond := catalogSourceFindCond(complianceConfig, isUnhealthy, isMissing, name) + assert.Equal(t, metav1.ConditionFalse, cond.Status) + + // Test Compliant complianceConfig with unhealthy CatalogSource + isUnhealthy = true + cond = catalogSourceFindCond(complianceConfig, isUnhealthy, isMissing, name) + assert.Equal(t, metav1.ConditionFalse, cond.Status) + + // Test NonCompliant complianceConfig with healthy CatalogSource + isUnhealthy = false + complianceConfig = policyv1beta1.NonCompliant + cond = catalogSourceFindCond(complianceConfig, isUnhealthy, isMissing, name) + assert.Equal(t, metav1.ConditionFalse, cond.Status) + assert.Equal(t, "CatalogSource was found", cond.Message) + + // Test NonCompliant complianceConfig with unhealthy CatalogSource + isUnhealthy = true + cond = catalogSourceFindCond(complianceConfig, isUnhealthy, isMissing, name) + assert.Equal(t, metav1.ConditionTrue, cond.Status) + assert.Equal(t, "CatalogSource was found but is unhealthy", cond.Message) +} + +func TestCatalogSourceObj(t *testing.T) { + catalogName := "TestCatalog" + catalogNS := "TestNamespace" + + // Test Compliant complianceConfig with healthy CatalogSource + isUnhealthy := false + isMissing := false + complianceConfig := policyv1beta1.ComplianceConfigAction("Compliant") + res := catalogSourceObj(catalogName, catalogNS, isUnhealthy, isMissing, complianceConfig) + assert.Equal(t, "Compliant", res.Compliant) + assert.Equal(t, "Resource found as expected", res.Reason) + + // Test Compliant complianceConfig with unhealthy CatalogSource + isUnhealthy = true + isMissing = false + complianceConfig = "Compliant" + res = catalogSourceObj(catalogName, catalogNS, isUnhealthy, isMissing, complianceConfig) + assert.Equal(t, "Compliant", res.Compliant) + assert.Equal(t, "Resource found as expected but is unhealthy", res.Reason) + + // Test NonCompliant complianceConfig with healthy CatalogSource + isUnhealthy = false + isMissing = false + complianceConfig = "NonCompliant" + res = catalogSourceObj(catalogName, catalogNS, isUnhealthy, isMissing, complianceConfig) + assert.Equal(t, "Compliant", res.Compliant) + assert.Equal(t, "Resource found as expected", res.Reason) + + // Test NonCompliant complianceConfig with unhealthy CatalogSource + isUnhealthy = true + isMissing = false + complianceConfig = "NonCompliant" + res = catalogSourceObj(catalogName, catalogNS, isUnhealthy, isMissing, complianceConfig) + assert.Equal(t, "NonCompliant", res.Compliant) + assert.Equal(t, "Resource found as expected but is unhealthy", res.Reason) +} diff --git a/deploy/crds/kustomize_operatorpolicy/policy.open-cluster-management.io_operatorpolicies.yaml b/deploy/crds/kustomize_operatorpolicy/policy.open-cluster-management.io_operatorpolicies.yaml index a3a4fda7..f8908e70 100644 --- a/deploy/crds/kustomize_operatorpolicy/policy.open-cluster-management.io_operatorpolicies.yaml +++ b/deploy/crds/kustomize_operatorpolicy/policy.open-cluster-management.io_operatorpolicies.yaml @@ -39,6 +39,41 @@ spec: spec: description: OperatorPolicySpec defines the desired state of OperatorPolicy properties: + complianceConfig: + default: {} + description: |- + ComplianceConfig defines how resource statuses affect the OperatorPolicy status and compliance. + When set to Compliant, the condition does not impact the OperatorPolicy compliance. + When set to NonCompliant, the condition causes the OperatorPolicy to become NonCompliant. + properties: + catalogSourceUnhealthy: + default: Compliant + description: |- + Specifies how the CatalogSourceUnhealthy typed condition should affect + overall policy compliance. Defaults to 'Compliant' + enum: + - Compliant + - NonCompliant + type: string + deploymentsUnavailable: + default: NonCompliant + description: |- + Specifies how the DeploymentCompliant typed condition should affect + overall policy compliance. Defaults to 'NonCompliant' + enum: + - Compliant + - NonCompliant + type: string + upgradesAvailable: + default: Compliant + description: |- + Specifies how the InstallPlanCompliant typed condition should affect + overall policy compliance. Defaults to 'Compliant' + enum: + - Compliant + - NonCompliant + type: string + type: object complianceType: description: ComplianceType describes whether we must or must not have a given resource diff --git a/deploy/crds/policy.open-cluster-management.io_operatorpolicies.yaml b/deploy/crds/policy.open-cluster-management.io_operatorpolicies.yaml index c77382e7..be44e52e 100644 --- a/deploy/crds/policy.open-cluster-management.io_operatorpolicies.yaml +++ b/deploy/crds/policy.open-cluster-management.io_operatorpolicies.yaml @@ -41,6 +41,41 @@ spec: spec: description: OperatorPolicySpec defines the desired state of OperatorPolicy properties: + complianceConfig: + default: {} + description: |- + ComplianceConfig defines how resource statuses affect the OperatorPolicy status and compliance. + When set to Compliant, the condition does not impact the OperatorPolicy compliance. + When set to NonCompliant, the condition causes the OperatorPolicy to become NonCompliant. + properties: + catalogSourceUnhealthy: + default: Compliant + description: |- + Specifies how the CatalogSourceUnhealthy typed condition should affect + overall policy compliance. Defaults to 'Compliant' + enum: + - Compliant + - NonCompliant + type: string + deploymentsUnavailable: + default: NonCompliant + description: |- + Specifies how the DeploymentCompliant typed condition should affect + overall policy compliance. Defaults to 'NonCompliant' + enum: + - Compliant + - NonCompliant + type: string + upgradesAvailable: + default: Compliant + description: |- + Specifies how the InstallPlanCompliant typed condition should affect + overall policy compliance. Defaults to 'Compliant' + enum: + - Compliant + - NonCompliant + type: string + type: object complianceType: description: ComplianceType describes whether we must or must not have a given resource diff --git a/test/e2e/case38_install_operator_test.go b/test/e2e/case38_install_operator_test.go index 2503cd2a..6fc6e597 100644 --- a/test/e2e/case38_install_operator_test.go +++ b/test/e2e/case38_install_operator_test.go @@ -1106,7 +1106,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, Label("supports-hosted"), fu "CatalogSource was found", ) }) - It("Should become NonCompliant when CatalogSource DNE", func() { + It("Should report in status when CatalogSource DNE", func() { By("Patching the policy to reference a CatalogSource that DNE to emulate failure") utils.Kubectl("patch", "operatorpolicy", OpPlcName, "-n", testNamespace, "--type=json", "-p", `[{"op": "replace", "path": "/spec/subscription/source", "value": "fakeName"}]`) @@ -1114,7 +1114,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, Label("supports-hosted"), fu By("Checking the conditions and relatedObj in the policy") check( OpPlcName, - true, + false, []policyv1.RelatedObject{{ Object: policyv1.ObjectResource{ Kind: "CatalogSource", @@ -1124,19 +1124,19 @@ var _ = Describe("Testing OperatorPolicy", Ordered, Label("supports-hosted"), fu Namespace: opPolTestNS, }, }, - Compliant: "NonCompliant", + Compliant: "Compliant", Reason: "Resource not found but should exist", }}, metav1.Condition{ Type: "CatalogSourcesUnhealthy", - Status: metav1.ConditionTrue, + Status: metav1.ConditionFalse, Reason: "CatalogSourcesNotFound", Message: "CatalogSource 'fakeName' was not found", }, "CatalogSource 'fakeName' was not found", ) }) - It("Should remain NonCompliant when CatalogSource fails", func() { + It("Should report unhealthy status when CatalogSource fails", func() { By("Patching the policy to point to an existing CatalogSource") utils.Kubectl("patch", "operatorpolicy", OpPlcName, "-n", testNamespace, "--type=json", "-p", `[{"op": "replace", "path": "/spec/subscription/source", "value": "operatorhubio-catalog"}]`) @@ -1145,6 +1145,36 @@ var _ = Describe("Testing OperatorPolicy", Ordered, Label("supports-hosted"), fu KubectlTarget("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, + 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 but is unhealthy", + }}, + metav1.Condition{ + Type: "CatalogSourcesUnhealthy", + Status: metav1.ConditionFalse, + Reason: "CatalogSourcesFoundUnhealthy", + Message: "CatalogSource was found but is unhealthy", + }, + "CatalogSource was found but is unhealthy", + ) + }) + It("Should become NonCompliant when ComplianceConfig is modified", func() { + By("Patching the policy ComplianceConfig to NonCompliant") + utils.Kubectl("patch", "operatorpolicy", OpPlcName, "-n", testNamespace, "--type=json", "-p", + `[{"op": "replace", "path": "/spec/complianceConfig/catalogSourceUnhealthy", "value": "NonCompliant"}]`) + By("Checking the conditions and relatedObj in the policy") check( OpPlcName, @@ -1279,6 +1309,36 @@ var _ = Describe("Testing OperatorPolicy", Ordered, Label("supports-hosted"), fu return len(ipList.Items) }, olmWaitTimeout, 5, ctx).Should(Equal(1)) + check( + opPolName, + false, + []policyv1.RelatedObject{{ + Object: policyv1.ObjectResource{ + Kind: "InstallPlan", + APIVersion: "operators.coreos.com/v1alpha1", + Metadata: policyv1.ObjectMetadata{ + Namespace: opPolTestNS, + }, + }, + Compliant: "Compliant", + Reason: "The InstallPlan is RequiresApproval", + }}, + metav1.Condition{ + Type: "InstallPlanCompliant", + Status: metav1.ConditionTrue, + Reason: "InstallPlanRequiresApproval", + Message: "an InstallPlan to update to [strimzi-cluster-operator.v0.36.0] is available" + + " for approval", + }, + "an InstallPlan to update .* is available for approval", + ) + }) + It("Should become NonCompliant when ComplianceConfig is modified to NonCompliant", func(ctx SpecContext) { + By("Patching the policy ComplianceConfig to Compliant") + utils.Kubectl("patch", "operatorpolicy", opPolName, "-n", testNamespace, "--type=json", "-p", + `[{"op": "replace", "path": "/spec/complianceConfig/upgradesAvailable", "value": "NonCompliant"}]`) + + By("Checking the conditions and relatedObj in the policy") check( opPolName, true, @@ -1302,7 +1362,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, Label("supports-hosted"), fu "an InstallPlan to update .* is available for approval", ) }) - It("Should do the initial install when enforced, and stop at the next version", func(ctx SpecContext) { + It("Should do the upgrade when enforced, and stop at the next version", func(ctx SpecContext) { ipList, err := targetK8sDynamic.Resource(gvrInstallPlan).Namespace(opPolTestNS). List(ctx, metav1.ListOptions{}) Expect(err).NotTo(HaveOccurred()) @@ -2994,6 +3054,34 @@ var _ = Describe("Testing OperatorPolicy", Ordered, Label("supports-hosted"), fu Expect(remBehavior).To(HaveKeyWithValue("customResourceDefinitions", "Keep")) }) }) + Describe("Testing defaulted values of ComplianceConfig in an OperatorPolicy", func() { + const ( + opPolYAML = "../resources/case38_operator_install/operator-policy-no-group.yaml" + opPolName = "oppol-no-group" + ) + + BeforeEach(func() { + preFunc() + + createObjWithParent(parentPolicyYAML, parentPolicyName, + opPolYAML, testNamespace, gvrPolicy, gvrOperatorPolicy) + }) + + It("Should have applied defaults to the ComplianceConfig field", func(ctx SpecContext) { + policy, err := clientManagedDynamic.Resource(gvrOperatorPolicy).Namespace(testNamespace). + Get(ctx, opPolName, metav1.GetOptions{}) + Expect(err).NotTo(HaveOccurred()) + Expect(policy).NotTo(BeNil()) + + complianceConfig, found, err := unstructured.NestedStringMap(policy.Object, "spec", "complianceConfig") + Expect(found).To(BeTrue()) + Expect(err).NotTo(HaveOccurred()) + + Expect(complianceConfig).To(HaveKeyWithValue("catalogSourceUnhealthy", "Compliant")) + Expect(complianceConfig).To(HaveKeyWithValue("deploymentsUnavailable", "NonCompliant")) + Expect(complianceConfig).To(HaveKeyWithValue("upgradesAvailable", "Compliant")) + }) + }) Describe("Testing operator policies that specify the same subscription", Ordered, func() { const ( musthaveYAML = "../resources/case38_operator_install/operator-policy-no-group.yaml"