diff --git a/api/v1/configurationpolicy_types.go b/api/v1/configurationpolicy_types.go index e8a21494..eda2b97c 100644 --- a/api/v1/configurationpolicy_types.go +++ b/api/v1/configurationpolicy_types.go @@ -12,6 +12,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" ) // A custom type is required since there is no way to have a kubebuilder marker @@ -315,6 +316,22 @@ type ObjectResource struct { Metadata ObjectMetadata `json:"metadata,omitempty"` } +func ObjectResourceFromObj(obj client.Object) ObjectResource { + name := obj.GetName() + if name == "" { + name = "*" + } + + return ObjectResource{ + Kind: obj.GetObjectKind().GroupVersionKind().Kind, + APIVersion: obj.GetObjectKind().GroupVersionKind().GroupVersion().String(), + Metadata: ObjectMetadata{ + Name: name, + Namespace: obj.GetNamespace(), + }, + } +} + // ObjectMetadata contains the resource metadata for an object being processed by the policy type ObjectMetadata struct { // Name of the referent. More info: diff --git a/api/v1beta1/operatorpolicy_types.go b/api/v1beta1/operatorpolicy_types.go index 41aa47dd..262ccde1 100644 --- a/api/v1beta1/operatorpolicy_types.go +++ b/api/v1beta1/operatorpolicy_types.go @@ -101,6 +101,31 @@ type OperatorPolicyStatus struct { RelatedObjects []policyv1.RelatedObject `json:"relatedObjects"` } +func (status OperatorPolicyStatus) RelatedObjsOfKind(kind string) map[int]policyv1.RelatedObject { + objs := make(map[int]policyv1.RelatedObject) + + for i, related := range status.RelatedObjects { + if related.Object.Kind == kind { + objs[i] = related + } + } + + return objs +} + +// Searches the conditions of the policy, and returns the index and condition matching the +// given condition Type. It will return -1 as the index if no condition of the specified +// Type is found. +func (status OperatorPolicyStatus) GetCondition(condType string) (int, metav1.Condition) { + for i, cond := range status.Conditions { + if cond.Type == condType { + return i, cond + } + } + + return -1, metav1.Condition{} +} + //+kubebuilder:object:root=true //+kubebuilder:subresource:status diff --git a/controllers/operatorpolicy_controller.go b/controllers/operatorpolicy_controller.go index 92808672..d1e01341 100644 --- a/controllers/operatorpolicy_controller.go +++ b/controllers/operatorpolicy_controller.go @@ -5,17 +5,18 @@ package controllers import ( "context" + "encoding/json" "fmt" + "reflect" operatorv1 "github.com/operator-framework/api/pkg/operators/v1" operatorv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1" depclient "github.com/stolostron/kubernetes-dependency-watches/client" - "k8s.io/apimachinery/pkg/api/errors" + k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/types" - "k8s.io/apimachinery/pkg/util/json" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" @@ -24,7 +25,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/controller-runtime/pkg/source" - policyv1 "open-cluster-management.io/config-policy-controller/api/v1" policyv1beta1 "open-cluster-management.io/config-policy-controller/api/v1beta1" ) @@ -37,12 +37,11 @@ var ( operatorGroupGVK = schema.GroupVersionKind{Group: "operators.coreos.com", Version: "v1", Kind: "OperatorGroup"} ) -var OpLog = ctrl.Log.WithName(OperatorControllerName) - // OperatorPolicyReconciler reconciles a OperatorPolicy object type OperatorPolicyReconciler struct { client.Client DynamicWatcher depclient.DynamicWatcher + InstanceName string } // SetupWithManager sets up the controller with the Manager and will reconcile when the dynamic watcher @@ -76,21 +75,15 @@ var _ reconcile.Reconciler = &OperatorPolicyReconciler{} // For more details, check Reconcile and its Result here: // - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.14.4/pkg/reconcile func (r *OperatorPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + OpLog := ctrl.LoggerFrom(ctx) policy := &policyv1beta1.OperatorPolicy{} - - watcher := depclient.ObjectIdentifier{ - Group: operatorv1.GroupVersion.Group, - Version: operatorv1.GroupVersion.Version, - Kind: "OperatorPolicy", - Namespace: req.Namespace, - Name: req.Name, - } + watcher := opPolIdentifier(req.Namespace, req.Name) // Get the applied OperatorPolicy err := r.Get(ctx, req.NamespacedName, policy) if err != nil { - if errors.IsNotFound(err) { - OpLog.Info("Operator policy could not be found", "name", req.Name, "namespace", req.Namespace) + if k8serrors.IsNotFound(err) { + OpLog.Info("Operator policy could not be found") err = r.DynamicWatcher.RemoveWatcher(watcher) if err != nil { @@ -108,8 +101,7 @@ func (r *OperatorPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reque // Start query batch for caching and watching related objects err = r.DynamicWatcher.StartQueryBatch(watcher) if err != nil { - OpLog.Error(err, "Could not start query batch for the watcher", "watcher", watcher.Name, - "watcherKind", watcher.Kind) + OpLog.Error(err, "Could not start query batch for the watcher") return reconcile.Result{}, err } @@ -117,232 +109,155 @@ func (r *OperatorPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reque defer func() { err := r.DynamicWatcher.EndQueryBatch(watcher) if err != nil { - OpLog.Error(err, "Could not end query batch for the watcher", "watcher", watcher.Name, - "watcherKind", watcher.Kind) + OpLog.Error(err, "Could not end query batch for the watcher") } }() // handle the policy - OpLog.Info("Reconciling OperatorPolicy", "policy", policy.Name) + OpLog.Info("Reconciling OperatorPolicy") - // (temporary) - sub := make(map[string]interface{}) - - if err := json.Unmarshal(policy.Spec.Subscription.Raw, &sub); err != nil { - return reconcile.Result{}, fmt.Errorf("error unmarshalling subscription: %w", err) - } - - // Check if specified Subscription exists - cachedSubscription, err := r.DynamicWatcher.Get(watcher, subscriptionGVK, sub["namespace"].(string), - sub["name"].(string)) - if err != nil { - OpLog.Error(err, "Could not get subscription", "kind", subscriptionGVK.Kind, - "name", sub["name"].(string), - "namespace", sub["namespace"].(string)) + if err := r.handleOpGroup(ctx, policy); err != nil { + OpLog.Error(err, "Error handling OperatorGroup") return reconcile.Result{}, err } - subExists := cachedSubscription != nil - - // Check if any OperatorGroups exist in the namespace - ogNamespace := sub["namespace"].(string) - - if policy.Spec.OperatorGroup != nil { - // (temporary) - og := make(map[string]interface{}) - - if err := json.Unmarshal(policy.Spec.OperatorGroup.Raw, &og); err != nil { - return reconcile.Result{}, fmt.Errorf("error unmarshalling operatorgroup: %w", err) - } - - ogNamespace = og["namespace"].(string) - } - - cachedOperatorGroups, err := r.DynamicWatcher.List(watcher, operatorGroupGVK, ogNamespace, nil) - if err != nil { - OpLog.Error(err, "Could not list operator group", "kind", operatorGroupGVK.Kind, - "namespace", ogNamespace) + if err := r.handleSubscription(ctx, policy); err != nil { + OpLog.Error(err, "Error handling Subscription") return reconcile.Result{}, err } - ogExists := len(cachedOperatorGroups) != 0 + // FUTURE: more resource checks - // Exists indicates if a Subscription or Operatorgroup need to be created - exists := subExists && ogExists - shouldExist := policy.Spec.ComplianceType.IsMustHave() - - // Case 1: policy has just been applied and related resources have yet to be created - if !exists && shouldExist { - OpLog.Info("The object does not exist but should exist") - - return r.createPolicyResources(ctx, policy, cachedSubscription, cachedOperatorGroups) - } + return reconcile.Result{}, nil +} - // Case 2: Resources exist, but should not exist (i.e. mustnothave or deletion) - if exists && !shouldExist { - // Future implementation: clean up resources and delete watches if mustnothave, otherwise inform - OpLog.Info("The object exists but should not exist") - } +func (r *OperatorPolicyReconciler) handleOpGroup(ctx context.Context, policy *policyv1beta1.OperatorPolicy) error { + watcher := opPolIdentifier(policy.Namespace, policy.Name) - // Case 3: Resources do not exist, and should not exist - if !exists && !shouldExist { - // Future implementation: Possibly emit a success event - OpLog.Info("The object does not exist and is compliant with the mustnothave compliance type") + desiredOpGroup, err := buildOperatorGroup(policy) + if err != nil { + return fmt.Errorf("error building operator group: %w", err) } - // Case 4: Resources exist, and should exist (i.e. update) - if exists && shouldExist { - // Future implementation: Verify the specs of the object matches the one on the cluster - OpLog.Info("The object already exists. Checking fields to verify matching specs") + foundOpGroups, err := r.DynamicWatcher.List( + watcher, operatorGroupGVK, desiredOpGroup.Namespace, labels.Everything()) + if err != nil { + return fmt.Errorf("error listing OperatorGroups: %w", err) } - return reconcile.Result{}, nil -} + switch len(foundOpGroups) { + case 0: + // Missing OperatorGroup: report NonCompliance + err := r.updateStatus(ctx, policy, missingWantedCond("OperatorGroup"), missingWantedObj(desiredOpGroup)) + if err != nil { + return fmt.Errorf("error updating the status for a missing OperatorGroup: %w", err) + } -// createPolicyResources encapsulates the logic for creating resources specified within an operator policy. -// This should normally happen when the policy is initially applied to the cluster. Creates an OperatorGroup -// if specified, otherwise defaults to using an allnamespaces OperatorGroup. -func (r *OperatorPolicyReconciler) createPolicyResources( - ctx context.Context, - policy *policyv1beta1.OperatorPolicy, - cachedSubscription *unstructured.Unstructured, - cachedOGList []unstructured.Unstructured, -) (ctrl.Result, error) { - // Create og, then trigger reconcile - if len(cachedOGList) == 0 { if policy.Spec.RemediationAction.IsEnforce() { - ogSpec, err := buildOperatorGroup(policy) + err = r.Create(ctx, desiredOpGroup) if err != nil { - return reconcile.Result{}, err + return fmt.Errorf("error creating the OperatorGroup: %w", err) } - err = r.Create(ctx, ogSpec) - if err != nil { - OpLog.Error(err, "Error while creating OperatorGroup") - r.setCompliance(ctx, policy, policyv1.NonCompliant) + desiredOpGroup.SetGroupVersionKind(operatorGroupGVK) // Create stripped this information - return reconcile.Result{}, err + // Now the OperatorGroup should match, so report Compliance + err = r.updateStatus(ctx, policy, createdCond("OperatorGroup"), createdObj(desiredOpGroup)) + if err != nil { + return fmt.Errorf("error updating the status for a created OperatorGroup: %w", err) } - - // Created successfully, requeue result - r.setCompliance(ctx, policy, policyv1.NonCompliant) - - return reconcile.Result{Requeue: true}, err - } else if policy.Spec.OperatorGroup != nil { - // If inform mode, keep going and return at the end without requeue. Before - // continuing, should check if required og spec is created to set compliance state - - // Set to non compliant because operatorgroup does not exist on cluster, but - // should still go to check subscription - r.setCompliance(ctx, policy, policyv1.NonCompliant) } - } - - // Create new subscription - if cachedSubscription == nil { - if policy.Spec.RemediationAction.IsEnforce() { - subscriptionSpec, err := buildSubscription(policy) - if err != nil { - return reconcile.Result{}, err + case 1: + opGroup := foundOpGroups[0] + + if policy.Spec.OperatorGroup != nil { + emptyNameMatch := desiredOpGroup.Name == "" && opGroup.GetGenerateName() == desiredOpGroup.GenerateName + + if opGroup.GetName() == desiredOpGroup.Name || emptyNameMatch { + // check whether the specs match + desiredUnstruct, err := runtime.DefaultUnstructuredConverter.ToUnstructured(desiredOpGroup) + if err != nil { + return fmt.Errorf("error converting desired OperatorGroup to an Unstructured: %w", err) + } + + if reflect.DeepEqual(desiredUnstruct["spec"], opGroup.Object["spec"]) { + // Everything relevant matches! This is a "happy path". + // Only update the condition if it reflects a NonCompliant state because emitting a + // Compliant -> Compliant 'change' is noisy and would hide *why* this part of the + // policy is currently compliant (eg that the policy previously updated the object). + idx, existingCond := policy.Status.GetCondition(opGroupConditionType) + + if idx == -1 || existingCond.Status == metav1.ConditionFalse { + err := r.updateStatus(ctx, policy, matchesCond("OperatorGroup"), matchedObj(&opGroup)) + if err != nil { + return fmt.Errorf("error updating the status for an OperatorGroup that matches: %w", err) + } + } + } else { + // The names match, but the specs don't: report NonCompliance + err := r.updateStatus(ctx, policy, mismatchCond("OperatorGroup"), mismatchedObj(&opGroup)) + if err != nil { + return fmt.Errorf("error updating the status for an OperatorGroup that does not match: %w", err) + } + + if policy.Spec.RemediationAction.IsEnforce() { + desiredOpGroup.ResourceVersion = opGroup.GetResourceVersion() + + err := r.Update(ctx, desiredOpGroup) + if err != nil { + return fmt.Errorf("error updating the OperatorGroup: %w", err) + } + desiredOpGroup.SetGroupVersionKind(operatorGroupGVK) // Update stripped this information + + // It was updated and should match now, so report Compliance + err = r.updateStatus(ctx, policy, updatedCond("OperatorGroup"), updatedObj(desiredOpGroup)) + if err != nil { + return fmt.Errorf("error updating the status after updating the OperatorGroup: %w", err) + } + } + } + } else { + // There is an OperatorGroup in the namespace that does not match the name of what is in the policy. + // Just creating a new one would cause the "TooManyOperatorGroups" failure. + // So, just report a NonCompliant status. + missing := missingWantedObj(desiredOpGroup) + badExisting := mismatchedObj(&opGroup) + + err := r.updateStatus(ctx, policy, mismatchCond("OperatorGroup"), missing, badExisting) + if err != nil { + return fmt.Errorf("error updating the status for an OperatorGroup with the wrong name: %w", err) + } } + } else { + // There is one operator group in the namespace, but the policy doesn't specify what it should look like. - err = r.Create(ctx, subscriptionSpec) - if err != nil { - OpLog.Error(err, "Could not handle missing musthave object") - r.setCompliance(ctx, policy, policyv1.NonCompliant) - - return reconcile.Result{}, err - } + // FUTURE: check if the one operator group is compatible with the desired subscription. - // Future work: Check availability/status of all resources - // managed by the OperatorPolicy before setting compliance state - r.setCompliance(ctx, policy, policyv1.Compliant) + // For an initial implementation, assume if an OperatorGroup already exists, then it's a good one. + // Only update the condition if it reflects a NonCompliant state, to prevent repeats. + idx, existingCond := policy.Status.GetCondition(opGroupConditionType) - return reconcile.Result{}, nil + if idx == -1 || existingCond.Status == metav1.ConditionFalse { + err := r.updateStatus(ctx, policy, matchesCond("OperatorGroup"), matchedObj(&opGroup)) + if err != nil { + return fmt.Errorf("error updating the status for an OperatorGroup that matches: %w", err) + } + } + } + default: + // This situation will always lead to a "TooManyOperatorGroups" failure on the CSV. + // Consider improving this in the future: perhaps this could suggest one of the OperatorGroups to keep. + err := r.updateStatus(ctx, policy, opGroupTooManyCond, opGroupTooManyObjs(foundOpGroups)...) + if err != nil { + return fmt.Errorf("error updating the status when there are multiple OperatorGroups: %w", err) } - // inform - r.setCompliance(ctx, policy, policyv1.NonCompliant) - } - - // Will only reach this if in inform mode - return reconcile.Result{}, nil -} - -// updatePolicyStatus updates the status of the operatorPolicy. -// In the future, a condition should be added as well, and this should generate events. -func (r *OperatorPolicyReconciler) updatePolicyStatus( - ctx context.Context, - policy *policyv1beta1.OperatorPolicy, -) error { - updatedStatus := policy.Status - - err := r.Get(ctx, types.NamespacedName{Namespace: policy.Namespace, Name: policy.Name}, policy) - if err != nil { - OpLog.Info(fmt.Sprintf("Failed to refresh policy; using previously fetched version: %s", err)) - } else { - policy.Status = updatedStatus - } - - err = r.Status().Update(ctx, policy) - if err != nil { - OpLog.Info(fmt.Sprintf("Failed to update policy status: %s", err)) - - return err } return nil } -// buildSubscription bootstraps the subscription spec defined in the operator policy -// with the apiversion and kind in preparation for resource creation -func buildSubscription( - policy *policyv1beta1.OperatorPolicy, -) (*operatorv1alpha1.Subscription, error) { - subscription := new(operatorv1alpha1.Subscription) - - sub := make(map[string]interface{}) - - err := json.Unmarshal(policy.Spec.Subscription.Raw, &sub) - if err != nil { - return nil, fmt.Errorf("error unmarshalling subscription: %w", err) - } - - ns, ok := sub["namespace"].(string) - if !ok { - return nil, fmt.Errorf("namespace is required in spec.subscription") - } - - spec := new(operatorv1alpha1.SubscriptionSpec) - - err = json.Unmarshal(policy.Spec.Subscription.Raw, spec) - if err != nil { - return nil, fmt.Errorf("error unmarshalling subscription: %w", err) - } - - subscription.SetGroupVersionKind(subscriptionGVK) - subscription.ObjectMeta.Name = spec.Package - subscription.ObjectMeta.Namespace = ns - subscription.Spec = spec - - return subscription, nil -} - -// Sets the compliance of the policy -func (r *OperatorPolicyReconciler) setCompliance( - ctx context.Context, - policy *policyv1beta1.OperatorPolicy, - compliance policyv1.ComplianceState, -) { - policy.Status.ComplianceState = compliance - - err := r.updatePolicyStatus(ctx, policy) - if err != nil { - OpLog.Error(err, "error while updating policy status") - } -} - // buildOperatorGroup bootstraps the OperatorGroup spec defined in the operator policy // with the apiversion and kind in preparation for resource creation func buildOperatorGroup( @@ -406,3 +321,125 @@ func buildOperatorGroup( return operatorGroup, nil } + +func (r *OperatorPolicyReconciler) handleSubscription(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) + } + + foundSub, err := r.DynamicWatcher.Get(watcher, subscriptionGVK, desiredSub.Namespace, desiredSub.Name) + if err != nil { + return fmt.Errorf("error listing OperatorGroups: %w", err) + } + + if foundSub == nil { + // Missing Subscription: report NonCompliance + err := r.updateStatus(ctx, policy, missingWantedCond("Subscription"), missingWantedObj(desiredSub)) + if err != nil { + return fmt.Errorf("error updating status for a missing Subscription: %w", err) + } + + if policy.Spec.RemediationAction.IsEnforce() { + err := r.Create(ctx, desiredSub) + if err != nil { + return fmt.Errorf("error creating the Subscription: %w", err) + } + + desiredSub.SetGroupVersionKind(subscriptionGVK) // Create stripped this information + + // Now it should match, so report Compliance + err = r.updateStatus(ctx, policy, createdCond("Subscription"), createdObj(desiredSub)) + if err != nil { + return fmt.Errorf("error updating the status for a created Subscription: %w", err) + } + } + } else { + // Subscription found; check if specs match + desiredUnstruct, err := runtime.DefaultUnstructuredConverter.ToUnstructured(desiredSub) + if err != nil { + return fmt.Errorf("error converting desired Subscription to an Unstructured: %w", err) + } + + if reflect.DeepEqual(desiredUnstruct["spec"], foundSub.Object["spec"]) { + // FUTURE: Check more details about the *status* of the Subscription + // For now, (conditionally) mark it as compliant + idx, existingCond := policy.Status.GetCondition(subConditionType) + + if idx == -1 || existingCond.Status == metav1.ConditionFalse { + 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) + } + } + } else { + err := r.updateStatus(ctx, policy, mismatchCond("Subscription"), mismatchedObj(foundSub)) + if err != nil { + return fmt.Errorf("error updating status for a mismatched Subscription: %w", err) + } + + if policy.Spec.RemediationAction.IsEnforce() { + desiredSub.ResourceVersion = foundSub.GetResourceVersion() + + err := r.Update(ctx, desiredSub) + if err != nil { + return fmt.Errorf("error updating the Subscription: %w", err) + } + desiredSub.SetGroupVersionKind(subscriptionGVK) // Update stripped this information + + err = r.updateStatus(ctx, policy, updatedCond("Subscription"), updatedObj(desiredSub)) + if err != nil { + return fmt.Errorf("error updating status after updating the Subscription: %w", err) + } + } + } + } + + return nil +} + +// buildSubscription bootstraps the subscription spec defined in the operator policy +// with the apiversion and kind in preparation for resource creation +func buildSubscription( + policy *policyv1beta1.OperatorPolicy, +) (*operatorv1alpha1.Subscription, error) { + subscription := new(operatorv1alpha1.Subscription) + + sub := make(map[string]interface{}) + + err := json.Unmarshal(policy.Spec.Subscription.Raw, &sub) + if err != nil { + return nil, fmt.Errorf("error unmarshalling subscription: %w", err) + } + + ns, ok := sub["namespace"].(string) + if !ok { + return nil, fmt.Errorf("namespace is required in spec.subscription") + } + + spec := new(operatorv1alpha1.SubscriptionSpec) + + err = json.Unmarshal(policy.Spec.Subscription.Raw, spec) + if err != nil { + return nil, fmt.Errorf("error unmarshalling subscription: %w", err) + } + + subscription.SetGroupVersionKind(subscriptionGVK) + subscription.ObjectMeta.Name = spec.Package + subscription.ObjectMeta.Namespace = ns + subscription.Spec = spec + + return subscription, nil +} + +func opPolIdentifier(namespace, name string) depclient.ObjectIdentifier { + return depclient.ObjectIdentifier{ + Group: policyv1beta1.GroupVersion.Group, + Version: policyv1beta1.GroupVersion.Version, + Kind: "OperatorPolicy", + Namespace: namespace, + Name: name, + } +} diff --git a/controllers/operatorpolicy_status.go b/controllers/operatorpolicy_status.go new file mode 100644 index 00000000..49302b90 --- /dev/null +++ b/controllers/operatorpolicy_status.go @@ -0,0 +1,429 @@ +package controllers + +import ( + "context" + "fmt" + "sort" + "strings" + "time" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "sigs.k8s.io/controller-runtime/pkg/client" + + policyv1 "open-cluster-management.io/config-policy-controller/api/v1" + policyv1beta1 "open-cluster-management.io/config-policy-controller/api/v1beta1" +) + +// updateStatus takes one condition to update, and related objects for that condition. The related +// objects given will replace all existing relatedObjects with the same gvk. If a condition is +// changed, the compliance will be recalculated and a compliance event will be emitted. The +// condition and related objects can match what is already in the status - in that case, no API +// calls are made. The `lastTransitionTime` on a condition is not considered when checking if the +// condition has changed. If not provided, the `lastTransitionTime` will use "now". It also handles +// preserving the `CreatedByPolicy` property on relatedObjects. +// +// This function assumes that all given related objects are of the same kind. +// +// Note that only changing the related objects will not emit a new compliance event, but will update +// the status. +func (r *OperatorPolicyReconciler) updateStatus( + ctx context.Context, + policy *policyv1beta1.OperatorPolicy, + updatedCondition metav1.Condition, + updatedRelatedObjs ...policyv1.RelatedObject, +) error { + condChanged := false + + if updatedCondition.LastTransitionTime.IsZero() { + updatedCondition.LastTransitionTime = metav1.Now() + } + + condIdx, existingCondition := policy.Status.GetCondition(updatedCondition.Type) + if condIdx == -1 { + condChanged = true + + // Just append, the conditions will be sorted later. + policy.Status.Conditions = append(policy.Status.Conditions, updatedCondition) + } else if conditionChanged(updatedCondition, existingCondition) { + condChanged = true + + policy.Status.Conditions[condIdx] = updatedCondition + } + + if condChanged { + updatedComplianceCondition := calculateComplianceCondition(policy) + + compCondIdx, _ := policy.Status.GetCondition(updatedComplianceCondition.Type) + if compCondIdx == -1 { + policy.Status.Conditions = append(policy.Status.Conditions, updatedComplianceCondition) + } else { + policy.Status.Conditions[compCondIdx] = updatedComplianceCondition + } + + // Sort the conditions based on their type. + sort.SliceStable(policy.Status.Conditions, func(i, j int) bool { + return policy.Status.Conditions[i].Type < policy.Status.Conditions[j].Type + }) + + if updatedComplianceCondition.Status == metav1.ConditionTrue { + policy.Status.ComplianceState = policyv1.Compliant + } else { + policy.Status.ComplianceState = policyv1.NonCompliant + } + + err := r.emitComplianceEvent(ctx, policy, updatedComplianceCondition) + if err != nil { + return err + } + } + + relObjsChanged := false + + prevRelObjs := policy.Status.RelatedObjsOfKind(updatedRelatedObjs[0].Object.Kind) + if len(prevRelObjs) == len(updatedRelatedObjs) { + for _, prevObj := range prevRelObjs { + nameFound := false + + for i, updatedObj := range updatedRelatedObjs { + if prevObj.Object.Metadata.Name == updatedObj.Object.Metadata.Name { + nameFound = true + + if updatedObj.Properties != nil && prevObj.Properties != nil { + if updatedObj.Properties.UID != prevObj.Properties.UID { + relObjsChanged = true + } else if prevObj.Properties.CreatedByPolicy != nil { + // There is an assumption here that it will never need to transition to false. + updatedRelatedObjs[i].Properties.CreatedByPolicy = prevObj.Properties.CreatedByPolicy + } + } + + if prevObj.Compliant != updatedObj.Compliant { + relObjsChanged = true + } else if prevObj.Reason != updatedObj.Reason { + relObjsChanged = true + } + } + } + + if !nameFound { + relObjsChanged = true + } + } + } else { + relObjsChanged = true + } + + if relObjsChanged { + // start with the related objects which do not match the currently considered kind + newRelObjs := make([]policyv1.RelatedObject, 0) + + for idx, relObj := range policy.Status.RelatedObjects { + if _, matchedIdx := prevRelObjs[idx]; !matchedIdx { + newRelObjs = append(newRelObjs, relObj) + } + } + + // add the new related objects + newRelObjs = append(newRelObjs, updatedRelatedObjs...) + + // sort the related objects by kind and name + sort.SliceStable(newRelObjs, func(i, j int) bool { + if newRelObjs[i].Object.Kind != newRelObjs[j].Object.Kind { + return newRelObjs[i].Object.Kind < newRelObjs[j].Object.Kind + } + + return newRelObjs[i].Object.Metadata.Name < newRelObjs[j].Object.Metadata.Name + }) + + policy.Status.RelatedObjects = newRelObjs + } + + if condChanged || relObjsChanged { + return r.Status().Update(ctx, policy) + } + + return nil +} + +func conditionChanged(updatedCondition, existingCondition metav1.Condition) bool { + if updatedCondition.Message != existingCondition.Message { + return true + } + + if updatedCondition.Reason != existingCondition.Reason { + return true + } + + if updatedCondition.Status != existingCondition.Status { + return true + } + + return false +} + +// The Compliance condition is calculated by going through the known conditions in a consistent +// order, checking if there are any reasons the policy should be NonCompliant, and accumulating +// the reasons into one string to reflect the whole status. +func calculateComplianceCondition(policy *policyv1beta1.OperatorPolicy) metav1.Condition { + foundNonCompliant := false + messages := make([]string, 0) + + idx, cond := policy.Status.GetCondition(opGroupConditionType) + if idx == -1 { + messages = append(messages, "the status of the OperatorGroup is unknown") + foundNonCompliant = true + } else { + messages = append(messages, cond.Message) + + if cond.Status != metav1.ConditionTrue { + foundNonCompliant = true + } + } + + idx, cond = policy.Status.GetCondition(subConditionType) + if idx == -1 { + messages = append(messages, "the status of the Subscription is unknown") + foundNonCompliant = true + } else { + messages = append(messages, cond.Message) + + if cond.Status != metav1.ConditionTrue { + foundNonCompliant = true + } + } + + // FUTURE: check additional conditions + + if foundNonCompliant { + return metav1.Condition{ + Type: compliantConditionType, + Status: metav1.ConditionFalse, + LastTransitionTime: metav1.Now(), + Reason: "NonCompliant", + Message: "NonCompliant; " + strings.Join(messages, ", "), + } + } + + return metav1.Condition{ + Type: compliantConditionType, + Status: metav1.ConditionTrue, + LastTransitionTime: metav1.Now(), + Reason: "Compliant", + Message: "Compliant; " + strings.Join(messages, ", "), + } +} + +func (r *OperatorPolicyReconciler) emitComplianceEvent( + ctx context.Context, + policy *policyv1beta1.OperatorPolicy, + complianceCondition metav1.Condition, +) error { + if len(policy.OwnerReferences) == 0 { + return nil // there is nothing to do, since no owner is set + } + + ownerRef := policy.OwnerReferences[0] + now := time.Now() + event := &corev1.Event{ + ObjectMeta: metav1.ObjectMeta{ + // This event name matches the convention of recorders from client-go + Name: fmt.Sprintf("%v.%x", ownerRef.Name, now.UnixNano()), + Namespace: policy.Namespace, + }, + InvolvedObject: corev1.ObjectReference{ + Kind: ownerRef.Kind, + Namespace: policy.Namespace, // k8s ensures owners are always in the same namespace + Name: ownerRef.Name, + UID: ownerRef.UID, + APIVersion: ownerRef.APIVersion, + }, + Reason: fmt.Sprintf(eventFmtStr, policy.Namespace, policy.Name), + Message: complianceCondition.Message, + Source: corev1.EventSource{ + Component: ControllerName, + Host: r.InstanceName, + }, + FirstTimestamp: metav1.NewTime(now), + LastTimestamp: metav1.NewTime(now), + Count: 1, + Type: "Normal", + Action: "ComplianceStateUpdate", + Related: &corev1.ObjectReference{ + Kind: policy.Kind, + Namespace: policy.Namespace, + Name: policy.Name, + UID: policy.UID, + APIVersion: policy.APIVersion, + }, + ReportingController: ControllerName, + ReportingInstance: r.InstanceName, + } + + if policy.Status.ComplianceState != policyv1.Compliant { + event.Type = "Warning" + } + + return r.Create(ctx, event) +} + +const ( + compliantConditionType = "Compliant" + opGroupConditionType = "OperatorGroupCompliant" + subConditionType = "SubscriptionCompliant" +) + +func condType(kind string) string { + switch kind { + case "OperatorGroup": + return opGroupConditionType + case "Subscription": + return subConditionType + default: + panic("Unknown condition type for kind " + kind) + } +} + +// missingWantedCond returns a NonCompliant condition, with a Reason like '____Missing' +// and a Message like 'the ____ required by the policy was not found' +func missingWantedCond(kind string) metav1.Condition { + return metav1.Condition{ + Type: condType(kind), + Status: metav1.ConditionFalse, + Reason: kind + "Missing", + Message: "the " + kind + " required by the policy was not found", + } +} + +// createdCond returns a Compliant condition, with a Reason like'____Created', +// and a Message like 'the ____ required by the policy was created' +func createdCond(kind string) metav1.Condition { + return metav1.Condition{ + Type: condType(kind), + Status: metav1.ConditionTrue, + Reason: kind + "Created", + Message: "the " + kind + " required by the policy was created", + } +} + +// matchesCond returns a Compliant condition, with a Reason like'____Matches', +// and a Message like 'the ____ matches what is required by the policy' +func matchesCond(kind string) metav1.Condition { + return metav1.Condition{ + Type: condType(kind), + Status: metav1.ConditionTrue, + Reason: kind + "Matches", + Message: "the " + kind + " matches what is required by the policy", + } +} + +// mismatchCond returns a NonCompliant condition with a Reason like '____Mismatch', +// and a Message like 'the ____ found on the cluster does not match the policy' +func mismatchCond(kind string) metav1.Condition { + return metav1.Condition{ + Type: condType(kind), + Status: metav1.ConditionFalse, + Reason: kind + "Mismatch", + Message: "the " + kind + " found on the cluster does not match the policy", + } +} + +// updatedCond returns a Compliant condition, with a Reason like'____Updated', +// and a Message like 'the ____ was updated to match the policy' +func updatedCond(kind string) metav1.Condition { + return metav1.Condition{ + Type: condType(kind), + Status: metav1.ConditionTrue, + Reason: kind + "Updated", + Message: "the " + kind + " was updated to match the policy", + } +} + +// opGroupTooManyCond is a NonCompliant condition with a Reason like 'TooManyOperatorGroups', +// and a Message like 'there is more than one OperatorGroup in the namespace' +var opGroupTooManyCond = metav1.Condition{ + Type: opGroupConditionType, + Status: metav1.ConditionFalse, + Reason: "TooManyOperatorGroups", + Message: "there is more than one OperatorGroup in the namespace", +} + +// missingWantedObj returns a NonCompliant RelatedObject with reason = 'Resource not found but should exist' +func missingWantedObj(obj client.Object) policyv1.RelatedObject { + return policyv1.RelatedObject{ + Object: policyv1.ObjectResourceFromObj(obj), + Compliant: string(policyv1.NonCompliant), + Reason: reasonWantFoundDNE, + } +} + +// createdObj returns a Compliant RelatedObject with reason = 'K8s creation success' +func createdObj(obj client.Object) policyv1.RelatedObject { + created := true + + return policyv1.RelatedObject{ + Object: policyv1.ObjectResourceFromObj(obj), + Compliant: string(policyv1.Compliant), + Reason: reasonWantFoundCreated, + Properties: &policyv1.ObjectProperties{ + CreatedByPolicy: &created, + UID: string(obj.GetUID()), + }, + } +} + +// matchedObj returns a Compliant RelatedObject with reason = 'Resource found as expected' +func matchedObj(obj client.Object) policyv1.RelatedObject { + return policyv1.RelatedObject{ + Object: policyv1.ObjectResourceFromObj(obj), + Compliant: string(policyv1.Compliant), + Reason: reasonWantFoundExists, + Properties: &policyv1.ObjectProperties{ + UID: string(obj.GetUID()), + }, + } +} + +// mismatchedObj returns a NonCompliant RelatedObject with reason = 'Resource found but does not match' +func mismatchedObj(obj client.Object) policyv1.RelatedObject { + return policyv1.RelatedObject{ + Object: policyv1.ObjectResourceFromObj(obj), + Compliant: string(policyv1.NonCompliant), + Reason: reasonWantFoundNoMatch, + Properties: &policyv1.ObjectProperties{ + UID: string(obj.GetUID()), + }, + } +} + +// updatedObj returns a Compliant RelatedObject with reason = 'K8s update success' +func updatedObj(obj client.Object) policyv1.RelatedObject { + return policyv1.RelatedObject{ + Object: policyv1.ObjectResourceFromObj(obj), + Compliant: string(policyv1.Compliant), + Reason: reasonUpdateSuccess, + Properties: &policyv1.ObjectProperties{ + UID: string(obj.GetUID()), + }, + } +} + +// opGroupTooManyObjs returns a list of NonCompliant RelatedObjects, each with +// reason = 'There is more than one OperatorGroup in this namespace' +func opGroupTooManyObjs(opGroups []unstructured.Unstructured) []policyv1.RelatedObject { + objs := make([]policyv1.RelatedObject, len(opGroups)) + + for i, opGroup := range opGroups { + objs[i] = policyv1.RelatedObject{ + Object: policyv1.ObjectResourceFromObj(&opGroup), + Compliant: string(policyv1.NonCompliant), + Reason: "There is more than one OperatorGroup in this namespace", + Properties: &policyv1.ObjectProperties{ + UID: string(opGroup.GetUID()), + }, + } + } + + return objs +} diff --git a/main.go b/main.go index 91fde207..92a5d1a2 100644 --- a/main.go +++ b/main.go @@ -434,6 +434,7 @@ func main() { OpReconciler := controllers.OperatorPolicyReconciler{ Client: mgr.GetClient(), DynamicWatcher: watcher, + InstanceName: instanceName, } if err = OpReconciler.SetupWithManager(mgr, depEvents); err != nil { diff --git a/test/e2e/case38_install_operator_test.go b/test/e2e/case38_install_operator_test.go index c3c057be..81d3a86d 100644 --- a/test/e2e/case38_install_operator_test.go +++ b/test/e2e/case38_install_operator_test.go @@ -1,124 +1,508 @@ package e2e import ( + "encoding/json" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + policyv1 "open-cluster-management.io/config-policy-controller/api/v1" + policyv1beta1 "open-cluster-management.io/config-policy-controller/api/v1beta1" "open-cluster-management.io/config-policy-controller/test/utils" ) var _ = Describe("Test installing an operator from OperatorPolicy", Ordered, func() { const ( - case38BasePath = "../resources/case38_operator_install/" - case38OpPolicy = case38BasePath + "case38_OpPlc.yaml" - case38OpPolicyDefaultOg = case38BasePath + "case38_OpPlc_default_og.yaml" - case38OpPolicyName = "test-operator-policy" - case38OpPolicyDefaultOgName = "test-operator-policy-no-og" - case38SubscriptionName = "project-quay" - case38DeploymentName = "quay-operator-tng" - case38OpPolicyNS = "op-1" - case38OpPolicyDefaultOgNS = "op-2" - case38OgName = "og-single" - case38DefaultOgName = case38SubscriptionName + "-default-og" + opPolTestNS = "operator-policy-testns" + parentPolicyYAML = "../resources/case38_operator_install/parent-policy.yaml" + parentPolicyName = "parent-policy" + opPolTimeout = 10 ) - Describe("The operator is being installed for the first time", Ordered, func() { - Context("When there is a OperatorGroup spec specified", Ordered, func() { - BeforeAll(func() { - // Create ns op-1 - utils.Kubectl("create", "ns", case38OpPolicyNS) - // Apply a policy in ns op-1 - utils.Kubectl("apply", "-f", case38OpPolicy, "-n", case38OpPolicyNS) - // Check if applied properly - op := utils.GetWithTimeout(clientManagedDynamic, gvrOperatorPolicy, case38OpPolicyName, - case38OpPolicyNS, true, defaultTimeoutSeconds) - Expect(op).NotTo(BeNil()) - }) + check := func( + polName string, + wantNonCompliant bool, + expectedRelatedObjs []policyv1.RelatedObject, + expectedCondition metav1.Condition, + expectedEventMsgSnippet string, + ) { + Eventually(func(g Gomega) { + unstructPolicy := utils.GetWithTimeout(clientManagedDynamic, gvrOperatorPolicy, polName, + opPolTestNS, true, opPolTimeout) - It("Should create the OperatorGroup", func() { - og := utils.GetWithTimeout(clientManagedDynamic, gvrOperatorGroup, - case38OgName, case38OpPolicyNS, true, defaultTimeoutSeconds) + policyJSON, err := json.MarshalIndent(unstructPolicy.Object, "", " ") + g.Expect(err).NotTo(HaveOccurred()) - Expect(og).NotTo(BeNil()) - }) + policy := policyv1beta1.OperatorPolicy{} + err = json.Unmarshal(policyJSON, &policy) + g.Expect(err).NotTo(HaveOccurred()) - It("Should create the Subscription from the spec", func() { - sub := utils.GetWithTimeout(clientManagedDynamic, gvrSubscription, - case38SubscriptionName, case38OpPolicyNS, true, defaultTimeoutSeconds) + if wantNonCompliant { + g.Expect(policy.Status.ComplianceState).To(Equal(policyv1.NonCompliant)) + } - Expect(sub).NotTo(BeNil()) - }) + matchingRelated := policy.Status.RelatedObjsOfKind(expectedRelatedObjs[0].Object.Kind) + g.Expect(matchingRelated).To(HaveLen(len(expectedRelatedObjs))) - It("Should become Compliant", func() { - OpPlc := utils.GetWithTimeout(clientManagedDynamic, gvrOperatorPolicy, - case38OpPolicyName, case38OpPolicyNS, true, defaultTimeoutSeconds) + for _, expectedRelObj := range expectedRelatedObjs { + foundMatchingName := false + unnamed := expectedRelObj.Object.Metadata.Name == "" - Expect(utils.GetComplianceState(OpPlc)).To(Equal("Compliant")) - }) + for _, actualRelObj := range matchingRelated { + if unnamed || actualRelObj.Object.Metadata.Name == expectedRelObj.Object.Metadata.Name { + foundMatchingName = true + g.Expect(actualRelObj.Compliant).To(Equal(expectedRelObj.Compliant)) + g.Expect(actualRelObj.Reason).To(Equal(expectedRelObj.Reason)) + } + } - It("Should have installed the operator", func() { - deployment := utils.GetWithTimeout(clientManagedDynamic, gvrDeployment, - case38DeploymentName, case38OpPolicyNS, true, defaultTimeoutSeconds*2) + g.Expect(foundMatchingName).To(BeTrue()) + } - Expect(deployment).NotTo(BeNil()) - }) + idx, actualCondition := policy.Status.GetCondition(expectedCondition.Type) + g.Expect(idx).NotTo(Equal(-1)) + g.Expect(actualCondition.Status).To(Equal(expectedCondition.Status)) + g.Expect(actualCondition.Reason).To(Equal(expectedCondition.Reason)) + g.Expect(actualCondition.Message).To(Equal(expectedCondition.Message)) + }, opPolTimeout, 1).Should(Succeed()) - AfterAll(func() { - // Delete related resources created during test - utils.Kubectl("delete", "operatorpolicy", case38OpPolicyName, "-n", case38OpPolicyNS) - utils.Kubectl("delete", "subscription", case38SubscriptionName, "-n", case38OpPolicyNS) - utils.Kubectl("delete", "operatorgroup", case38OgName, "-n", case38OpPolicyNS) - utils.Kubectl("delete", "ns", case38OpPolicyNS) + Eventually(func() []v1.Event { + return utils.GetMatchingEvents(clientManaged, opPolTestNS, + parentPolicyName, "", expectedEventMsgSnippet, opPolTimeout) + }, opPolTimeout, 1).ShouldNot(BeEmpty()) + } + + Describe("Testing OperatorGroup behavior when it is not specified in the policy", Ordered, func() { + const ( + opPolYAML = "../resources/case38_operator_install/operator-policy-no-group.yaml" + opPolName = "oppol-no-group" + extraOpGroupYAML = "../resources/case38_operator_install/extra-operator-group.yaml" + extraOpGroupName = "extra-operator-group" + ) + BeforeAll(func() { + utils.Kubectl("create", "ns", opPolTestNS) + DeferCleanup(func() { + utils.Kubectl("delete", "ns", opPolTestNS) }) + + createObjWithParent(parentPolicyYAML, parentPolicyName, + opPolYAML, opPolTestNS, gvrPolicy, gvrOperatorPolicy) }) - Context("When there is no OperatorGroup spec specified", Ordered, func() { - BeforeAll(func() { - // Create ns op-2 - utils.Kubectl("create", "ns", case38OpPolicyDefaultOgNS) - // Apply a policy in ns op-2 - utils.Kubectl("apply", "-f", case38OpPolicyDefaultOg, "-n", case38OpPolicyDefaultOgNS) - // Check if applied properly - op := utils.GetWithTimeout(clientManagedDynamic, gvrOperatorPolicy, case38OpPolicyDefaultOgName, - case38OpPolicyDefaultOgNS, true, defaultTimeoutSeconds) - Expect(op).NotTo(BeNil()) + It("Should initially be NonCompliant", func() { + check( + opPolName, + true, + []policyv1.RelatedObject{{ + Object: policyv1.ObjectResource{ + Kind: "OperatorGroup", + APIVersion: "operators.coreos.com/v1", + Metadata: policyv1.ObjectMetadata{ + Name: "*", + Namespace: opPolTestNS, + }, + }, + Compliant: "NonCompliant", + Reason: "Resource not found but should exist", + }}, + metav1.Condition{ + Type: "OperatorGroupCompliant", + Status: metav1.ConditionFalse, + Reason: "OperatorGroupMissing", + Message: "the OperatorGroup required by the policy was not found", + }, + "the OperatorGroup required by the policy was not found", + ) + }) + It("Should create the OperatorGroup when it is enforced", func() { + utils.Kubectl("patch", "operatorpolicy", opPolName, "-n", opPolTestNS, "--type=json", "-p", + `[{"op": "replace", "path": "/spec/remediationAction", "value": "enforce"}]`) + check( + opPolName, + false, + []policyv1.RelatedObject{{ + Object: policyv1.ObjectResource{ + Kind: "OperatorGroup", + APIVersion: "operators.coreos.com/v1", + }, + Compliant: "Compliant", + Reason: "K8s creation success", + }}, + metav1.Condition{ + Type: "OperatorGroupCompliant", + Status: metav1.ConditionTrue, + Reason: "OperatorGroupCreated", + Message: "the OperatorGroup required by the policy was created", + }, + "the OperatorGroup required by the policy was created", + ) + }) + It("Should become NonCompliant when an extra OperatorGroup is added", func() { + utils.Kubectl("apply", "-f", extraOpGroupYAML, "-n", opPolTestNS) + check( + opPolName, + true, + []policyv1.RelatedObject{{ + Object: policyv1.ObjectResource{ + Kind: "OperatorGroup", + APIVersion: "operators.coreos.com/v1", + }, + Compliant: "NonCompliant", + Reason: "There is more than one OperatorGroup in this namespace", + }, { + Object: policyv1.ObjectResource{ + Kind: "OperatorGroup", + APIVersion: "operators.coreos.com/v1", + Metadata: policyv1.ObjectMetadata{ + Name: extraOpGroupName, + Namespace: opPolTestNS, + }, + }, + Compliant: "NonCompliant", + Reason: "There is more than one OperatorGroup in this namespace", + }}, + metav1.Condition{ + Type: "OperatorGroupCompliant", + Status: metav1.ConditionFalse, + Reason: "TooManyOperatorGroups", + Message: "there is more than one OperatorGroup in the namespace", + }, + "there is more than one OperatorGroup in the namespace", + ) + }) + It("Should match the OperatorGroup when the extra one is deleted", func() { + utils.Kubectl("delete", "operatorgroup", extraOpGroupName, "-n", opPolTestNS) + check( + opPolName, + false, + []policyv1.RelatedObject{{ + Object: policyv1.ObjectResource{ + Kind: "OperatorGroup", + APIVersion: "operators.coreos.com/v1", + }, + Compliant: "Compliant", + Reason: "Resource found as expected", + }}, + metav1.Condition{ + Type: "OperatorGroupCompliant", + Status: metav1.ConditionTrue, + Reason: "OperatorGroupMatches", + Message: "the OperatorGroup matches what is required by the policy", + }, + "the OperatorGroup matches what is required by the policy", + ) + }) + }) + Describe("Testing OperatorGroup behavior when it is specified in the policy", Ordered, func() { + const ( + opPolYAML = "../resources/case38_operator_install/operator-policy-with-group.yaml" + opPolName = "oppol-with-group" + incorrectOpGroupYAML = "../resources/case38_operator_install/incorrect-operator-group.yaml" + incorrectOpGroupName = "incorrect-operator-group" + scopedOpGroupYAML = "../resources/case38_operator_install/scoped-operator-group.yaml" + scopedOpGroupName = "scoped-operator-group" + ) + + BeforeAll(func() { + utils.Kubectl("create", "ns", opPolTestNS) + DeferCleanup(func() { + utils.Kubectl("delete", "ns", opPolTestNS) }) - It("Should create a default OperatorGroup", func() { - og := utils.GetWithTimeout(clientManagedDynamic, gvrOperatorGroup, - case38DefaultOgName, case38OpPolicyDefaultOgNS, true, defaultTimeoutSeconds) + utils.Kubectl("apply", "-f", incorrectOpGroupYAML, "-n", opPolTestNS) - Expect(og).NotTo(BeNil()) - }) + createObjWithParent(parentPolicyYAML, parentPolicyName, + opPolYAML, opPolTestNS, gvrPolicy, gvrOperatorPolicy) + }) - It("Should create the Subscription from the spec", func() { - sub := utils.GetWithTimeout(clientManagedDynamic, gvrSubscription, - case38SubscriptionName, case38OpPolicyDefaultOgNS, true, defaultTimeoutSeconds) + It("Should initially be NonCompliant", func() { + check( + opPolName, + true, + []policyv1.RelatedObject{{ + Object: policyv1.ObjectResource{ + Kind: "OperatorGroup", + APIVersion: "operators.coreos.com/v1", + Metadata: policyv1.ObjectMetadata{ + Name: incorrectOpGroupName, + Namespace: opPolTestNS, + }, + }, + Compliant: "NonCompliant", + Reason: "Resource found but does not match", + }, { + Object: policyv1.ObjectResource{ + Kind: "OperatorGroup", + APIVersion: "operators.coreos.com/v1", + Metadata: policyv1.ObjectMetadata{ + Name: scopedOpGroupName, + Namespace: opPolTestNS, + }, + }, + Compliant: "NonCompliant", + Reason: "Resource not found but should exist", + }}, + metav1.Condition{ + Type: "OperatorGroupCompliant", + Status: metav1.ConditionFalse, + Reason: "OperatorGroupMismatch", + Message: "the OperatorGroup found on the cluster does not match the policy", + }, + "the OperatorGroup found on the cluster does not match the policy", + ) + }) + It("Should match when the OperatorGroup is manually corrected", func() { + utils.Kubectl("delete", "operatorgroup", incorrectOpGroupName, "-n", opPolTestNS) + utils.Kubectl("apply", "-f", scopedOpGroupYAML, "-n", opPolTestNS) + check( + opPolName, + false, + []policyv1.RelatedObject{{ + Object: policyv1.ObjectResource{ + Kind: "OperatorGroup", + APIVersion: "operators.coreos.com/v1", + Metadata: policyv1.ObjectMetadata{ + Name: scopedOpGroupName, + Namespace: opPolTestNS, + }, + }, + Compliant: "Compliant", + Reason: "Resource found as expected", + }}, + metav1.Condition{ + Type: "OperatorGroupCompliant", + Status: metav1.ConditionTrue, + Reason: "OperatorGroupMatches", + Message: "the OperatorGroup matches what is required by the policy", + }, + "the OperatorGroup matches what is required by the policy", + ) + }) + It("Should report a mismatch when the OperatorGroup is manually edited", func() { + utils.Kubectl("patch", "operatorgroup", scopedOpGroupName, "-n", opPolTestNS, "--type=json", "-p", + `[{"op": "replace", "path": "/spec/targetNamespaces", "value": []}]`) + check( + opPolName, + true, + []policyv1.RelatedObject{{ + Object: policyv1.ObjectResource{ + Kind: "OperatorGroup", + APIVersion: "operators.coreos.com/v1", + Metadata: policyv1.ObjectMetadata{ + Name: scopedOpGroupName, + Namespace: opPolTestNS, + }, + }, + Compliant: "NonCompliant", + Reason: "Resource found but does not match", + }}, + metav1.Condition{ + Type: "OperatorGroupCompliant", + Status: metav1.ConditionFalse, + Reason: "OperatorGroupMismatch", + Message: "the OperatorGroup found on the cluster does not match the policy", + }, + "the OperatorGroup found on the cluster does not match the policy", + ) + }) + It("Should update the OperatorGroup when it is changed to enforce", func() { + utils.Kubectl("patch", "operatorpolicy", opPolName, "-n", opPolTestNS, "--type=json", "-p", + `[{"op": "replace", "path": "/spec/remediationAction", "value": "enforce"}]`) + check( + opPolName, + false, + []policyv1.RelatedObject{{ + Object: policyv1.ObjectResource{ + Kind: "OperatorGroup", + APIVersion: "operators.coreos.com/v1", + Metadata: policyv1.ObjectMetadata{ + Name: scopedOpGroupName, + Namespace: opPolTestNS, + }, + }, + Compliant: "Compliant", + Reason: "K8s update success", + }}, + metav1.Condition{ + Type: "OperatorGroupCompliant", + Status: metav1.ConditionTrue, + Reason: "OperatorGroupUpdated", + Message: "the OperatorGroup was updated to match the policy", + }, + "the OperatorGroup was updated to match the policy", + ) + }) + }) + Describe("Testing Subscription behavior for musthave mode while enforcing", Ordered, func() { + const ( + opPolYAML = "../resources/case38_operator_install/operator-policy-no-group.yaml" + opPolName = "oppol-no-group" + subName = "project-quay" + ) - Expect(sub).NotTo(BeNil()) + BeforeAll(func() { + utils.Kubectl("create", "ns", opPolTestNS) + DeferCleanup(func() { + utils.Kubectl("delete", "ns", opPolTestNS) }) - It("Should become Compliant", func() { - OpPlc := utils.GetWithTimeout(clientManagedDynamic, gvrOperatorPolicy, - case38OpPolicyDefaultOgName, case38OpPolicyDefaultOgNS, true, defaultTimeoutSeconds) + createObjWithParent(parentPolicyYAML, parentPolicyName, + opPolYAML, opPolTestNS, gvrPolicy, gvrOperatorPolicy) + }) - Expect(utils.GetComplianceState(OpPlc)).To(Equal("Compliant")) + It("Should initially be NonCompliant", func() { + check( + opPolName, + true, + []policyv1.RelatedObject{{ + Object: policyv1.ObjectResource{ + Kind: "Subscription", + APIVersion: "operators.coreos.com/v1alpha1", + Metadata: policyv1.ObjectMetadata{ + Name: subName, + Namespace: opPolTestNS, + }, + }, + Compliant: "NonCompliant", + Reason: "Resource not found but should exist", + }}, + metav1.Condition{ + Type: "SubscriptionCompliant", + Status: metav1.ConditionFalse, + Reason: "SubscriptionMissing", + Message: "the Subscription required by the policy was not found", + }, + "the Subscription required by the policy was not found", + ) + }) + It("Should create the Subscription when enforced", func() { + utils.Kubectl("patch", "operatorpolicy", opPolName, "-n", opPolTestNS, "--type=json", "-p", + `[{"op": "replace", "path": "/spec/remediationAction", "value": "enforce"}]`) + check( + opPolName, + false, + []policyv1.RelatedObject{{ + Object: policyv1.ObjectResource{ + Kind: "Subscription", + APIVersion: "operators.coreos.com/v1alpha1", + Metadata: policyv1.ObjectMetadata{ + Name: subName, + Namespace: opPolTestNS, + }, + }, + Compliant: "Compliant", + Reason: "K8s creation success", + }}, + metav1.Condition{ + Type: "SubscriptionCompliant", + Status: metav1.ConditionTrue, + Reason: "SubscriptionCreated", + Message: "the Subscription required by the policy was created", + }, + "the Subscription required by the policy was created", + ) + }) + It("Should apply an update to the Subscription", func() { + utils.Kubectl("patch", "operatorpolicy", opPolName, "-n", opPolTestNS, "--type=json", "-p", + `[{"op": "replace", "path": "/spec/subscription/sourceNamespace", "value": "fake"}]`) + check( + opPolName, + false, + []policyv1.RelatedObject{{ + Object: policyv1.ObjectResource{ + Kind: "Subscription", + APIVersion: "operators.coreos.com/v1alpha1", + Metadata: policyv1.ObjectMetadata{ + Name: subName, + Namespace: opPolTestNS, + }, + }, + Compliant: "Compliant", + Reason: "K8s update success", + }}, + metav1.Condition{ + Type: "SubscriptionCompliant", + Status: metav1.ConditionTrue, + Reason: "SubscriptionUpdated", + Message: "the Subscription was updated to match the policy", + }, + "the Subscription was updated to match the policy", + ) + }) + }) + Describe("Testing Subscription behavior for musthave mode while informing", Ordered, func() { + const ( + opPolYAML = "../resources/case38_operator_install/operator-policy-no-group.yaml" + opPolName = "oppol-no-group" + subName = "project-quay" + subYAML = "../resources/case38_operator_install/subscription.yaml" + ) + + BeforeAll(func() { + utils.Kubectl("create", "ns", opPolTestNS) + DeferCleanup(func() { + utils.Kubectl("delete", "ns", opPolTestNS) }) - It("Should have installed the operator", func() { - deployment := utils.GetWithTimeout(clientManagedDynamic, gvrDeployment, - case38DeploymentName, case38OpPolicyDefaultOgNS, true, defaultTimeoutSeconds*2) + utils.Kubectl("apply", "-f", subYAML, "-n", opPolTestNS) - Expect(deployment).NotTo(BeNil()) - }) + createObjWithParent(parentPolicyYAML, parentPolicyName, + opPolYAML, opPolTestNS, gvrPolicy, gvrOperatorPolicy) + }) - AfterAll(func() { - // Delete related resources created during test - utils.Kubectl("delete", "operatorpolicy", case38OpPolicyDefaultOgName, "-n", case38OpPolicyDefaultOgNS) - utils.Kubectl("delete", "subscription", case38SubscriptionName, "-n", case38OpPolicyDefaultOgNS) - utils.Kubectl("delete", "operatorgroup", case38DefaultOgName, "-n", case38OpPolicyDefaultOgNS) - utils.Kubectl("delete", "ns", case38OpPolicyDefaultOgNS) - }) + It("Should initially notice the matching Subscription", func() { + check( + opPolName, + false, + []policyv1.RelatedObject{{ + Object: policyv1.ObjectResource{ + Kind: "Subscription", + APIVersion: "operators.coreos.com/v1alpha1", + Metadata: policyv1.ObjectMetadata{ + Name: subName, + Namespace: opPolTestNS, + }, + }, + Compliant: "Compliant", + Reason: "Resource found as expected", + }}, + metav1.Condition{ + Type: "SubscriptionCompliant", + Status: metav1.ConditionTrue, + Reason: "SubscriptionMatches", + Message: "the Subscription matches what is required by the policy", + }, + "the Subscription matches what is required by the policy", + ) + }) + It("Should notice the mismatch when the spec is changed in the policy", func() { + utils.Kubectl("patch", "operatorpolicy", opPolName, "-n", opPolTestNS, "--type=json", "-p", + `[{"op": "replace", "path": "/spec/subscription/sourceNamespace", "value": "fake"}]`) + check( + opPolName, + true, + []policyv1.RelatedObject{{ + Object: policyv1.ObjectResource{ + Kind: "Subscription", + APIVersion: "operators.coreos.com/v1alpha1", + Metadata: policyv1.ObjectMetadata{ + Name: subName, + Namespace: opPolTestNS, + }, + }, + Compliant: "NonCompliant", + Reason: "Resource found but does not match", + }}, + metav1.Condition{ + Type: "SubscriptionCompliant", + Status: metav1.ConditionFalse, + Reason: "SubscriptionMismatch", + Message: "the Subscription found on the cluster does not match the policy", + }, + "the Subscription found on the cluster does not match the policy", + ) }) }) }) diff --git a/test/resources/case38_operator_install/extra-operator-group.yaml b/test/resources/case38_operator_install/extra-operator-group.yaml new file mode 100644 index 00000000..d069efb7 --- /dev/null +++ b/test/resources/case38_operator_install/extra-operator-group.yaml @@ -0,0 +1,6 @@ +apiVersion: operators.coreos.com/v1 +kind: OperatorGroup +metadata: + name: extra-operator-group +spec: + targetNamespaces: [] diff --git a/test/resources/case38_operator_install/incorrect-operator-group.yaml b/test/resources/case38_operator_install/incorrect-operator-group.yaml new file mode 100644 index 00000000..7ead7176 --- /dev/null +++ b/test/resources/case38_operator_install/incorrect-operator-group.yaml @@ -0,0 +1,8 @@ +apiVersion: operators.coreos.com/v1 +kind: OperatorGroup +metadata: + name: incorrect-operator-group +spec: + targetNamespaces: + - "the-limit-does-not-exist" + - "bad-namespace" diff --git a/test/resources/case38_operator_install/operator-policy-no-group.yaml b/test/resources/case38_operator_install/operator-policy-no-group.yaml new file mode 100644 index 00000000..86ceaa44 --- /dev/null +++ b/test/resources/case38_operator_install/operator-policy-no-group.yaml @@ -0,0 +1,21 @@ +apiVersion: policy.open-cluster-management.io/v1beta1 +kind: OperatorPolicy +metadata: + name: oppol-no-group + ownerReferences: + - apiVersion: policy.open-cluster-management.io/v1 + kind: Policy + name: parent-policy + uid: 12345678-90ab-cdef-1234-567890abcdef # must be replaced before creation +spec: + remediationAction: inform + severity: medium + complianceType: musthave + subscription: + channel: stable-3.8 + name: project-quay + namespace: operator-policy-testns + installPlanApproval: Automatic + source: operatorhubio-catalog + sourceNamespace: olm + startingCSV: quay-operator.v3.8.1 diff --git a/test/resources/case38_operator_install/operator-policy-with-group.yaml b/test/resources/case38_operator_install/operator-policy-with-group.yaml new file mode 100644 index 00000000..4e24d7f7 --- /dev/null +++ b/test/resources/case38_operator_install/operator-policy-with-group.yaml @@ -0,0 +1,26 @@ +apiVersion: policy.open-cluster-management.io/v1beta1 +kind: OperatorPolicy +metadata: + name: oppol-with-group + ownerReferences: + - apiVersion: policy.open-cluster-management.io/v1 + kind: Policy + name: parent-policy + uid: 12345678-90ab-cdef-1234-567890abcdef # must be replaced before creation +spec: + remediationAction: inform + severity: medium + complianceType: musthave + operatorGroup: # optional + name: scoped-operator-group + namespace: operator-policy-testns + targetNamespaces: + - operator-policy-testns + subscription: + channel: stable-3.8 + name: project-quay + namespace: operator-policy-testns + installPlanApproval: Automatic + source: operatorhubio-catalog + sourceNamespace: olm + startingCSV: quay-operator.v3.8.1 diff --git a/test/resources/case38_operator_install/parent-policy.yaml b/test/resources/case38_operator_install/parent-policy.yaml new file mode 100644 index 00000000..993675de --- /dev/null +++ b/test/resources/case38_operator_install/parent-policy.yaml @@ -0,0 +1,8 @@ +apiVersion: policy.open-cluster-management.io/v1 +kind: Policy +metadata: + name: parent-policy +spec: + remediationAction: inform + disabled: false + policy-templates: [] diff --git a/test/resources/case38_operator_install/scoped-operator-group.yaml b/test/resources/case38_operator_install/scoped-operator-group.yaml new file mode 100644 index 00000000..7249ee27 --- /dev/null +++ b/test/resources/case38_operator_install/scoped-operator-group.yaml @@ -0,0 +1,7 @@ +apiVersion: operators.coreos.com/v1 +kind: OperatorGroup +metadata: + name: scoped-operator-group +spec: + targetNamespaces: + - "operator-policy-testns" diff --git a/test/resources/case38_operator_install/subscription.yaml b/test/resources/case38_operator_install/subscription.yaml new file mode 100644 index 00000000..c4592a1d --- /dev/null +++ b/test/resources/case38_operator_install/subscription.yaml @@ -0,0 +1,11 @@ +apiVersion: operators.coreos.com/v1alpha1 +kind: Subscription +metadata: + name: project-quay +spec: + channel: stable-3.8 + name: project-quay + installPlanApproval: Automatic + source: operatorhubio-catalog + sourceNamespace: olm + startingCSV: quay-operator.v3.8.1