From 095cc77c0b8409d5a5d2f428fe2c2779a3206e18 Mon Sep 17 00:00:00 2001 From: Justin Kulikauskas Date: Thu, 25 Jan 2024 11:29:40 -0500 Subject: [PATCH] Handle situations when operators were preinstalled The general plan is to continue adding `handle*` functions for the other resources that an OperatorPolicy needs to examine. Each handle function will update the policy's status (with conditions and relatedObjects), and possibly emit compliance events. This may cause more compliance events "than usual" compared to other controllers, but I think the separation of concerns will help each function be more maintainable. My hope is that some of the `*Cond` and `*Obj` functions in the status section can be reused in the future handlers. There was already overlap between the Subscription and OperatorGroup, so this seemed reasonable. Refs: - https://issues.redhat.com/browse/ACM-9283 Signed-off-by: Justin Kulikauskas --- api/v1/configurationpolicy_types.go | 17 + api/v1beta1/operatorpolicy_types.go | 25 + controllers/operatorpolicy_controller.go | 451 +++++++------- controllers/operatorpolicy_status.go | 429 ++++++++++++++ main.go | 1 + test/e2e/case38_install_operator_test.go | 552 +++++++++++++++--- .../extra-operator-group.yaml | 6 + .../incorrect-operator-group.yaml | 8 + .../operator-policy-no-group.yaml | 21 + .../operator-policy-with-group.yaml | 26 + .../parent-policy.yaml | 8 + .../scoped-operator-group.yaml | 7 + .../case38_operator_install/subscription.yaml | 11 + 13 files changed, 1271 insertions(+), 291 deletions(-) create mode 100644 controllers/operatorpolicy_status.go create mode 100644 test/resources/case38_operator_install/extra-operator-group.yaml create mode 100644 test/resources/case38_operator_install/incorrect-operator-group.yaml create mode 100644 test/resources/case38_operator_install/operator-policy-no-group.yaml create mode 100644 test/resources/case38_operator_install/operator-policy-with-group.yaml create mode 100644 test/resources/case38_operator_install/parent-policy.yaml create mode 100644 test/resources/case38_operator_install/scoped-operator-group.yaml create mode 100644 test/resources/case38_operator_install/subscription.yaml 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