From 56ace869bdca92e1afed46fe3d5e99fdd99cefdf 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. To check if the Subscription/OperatorGroup on the cluster matches what is desired by the policy, a function `handleKeys` was created that can be used for OperatorPolicies and ConfigurationPolicies. 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/configurationpolicy_controller.go | 132 ++-- controllers/operatorpolicy_controller.go | 515 ++++++++++------ controllers/operatorpolicy_status.go | 453 ++++++++++++++ .../kustomization.yaml | 6 + .../template-label.json | 7 + ...luster-management.io_operatorpolicies.yaml | 2 + main.go | 1 + test/e2e/case38_install_operator_test.go | 564 +++++++++++++++--- .../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 + 17 files changed, 1476 insertions(+), 333 deletions(-) create mode 100644 controllers/operatorpolicy_status.go create mode 100644 deploy/crds/kustomize_operatorpolicy/template-label.json 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 3a8a8870..ada14940 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 @@ -324,6 +325,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/configurationpolicy_controller.go b/controllers/configurationpolicy_controller.go index 56574138..57e4626f 100644 --- a/controllers/configurationpolicy_controller.go +++ b/controllers/configurationpolicy_controller.go @@ -2586,72 +2586,28 @@ func (r *ConfigurationPolicyReconciler) checkAndUpdateResource( res = r.TargetK8sDynamicClient.Resource(obj.gvr) } - updateSucceeded = false // Use a copy since some values can be directly assigned to mergedObj in handleSingleKey. existingObjectCopy := obj.existingObj.DeepCopy() removeFieldsForComparison(existingObjectCopy) - var statusMismatch bool - - for key := range obj.desiredObj.Object { - isStatus := key == "status" - - // use metadatacompliancetype to evaluate metadata if it is set - keyComplianceType := complianceType - if key == "metadata" && mdComplianceType != "" { - keyComplianceType = mdComplianceType - } - - // check key for mismatch - errorMsg, keyUpdateNeeded, mergedObj, skipped := handleSingleKey( - key, obj.desiredObj, existingObjectCopy, keyComplianceType, !r.DryRunSupported, - ) - if errorMsg != "" { - log.Info(errorMsg) - - return true, errorMsg, true, false - } - - if mergedObj == nil && skipped { - continue - } - - // only look at labels and annotations for metadata - configurationPolicies do not update other metadata fields - if key == "metadata" { - // if it's not the right type, the map will be empty - mdMap, _ := mergedObj.(map[string]interface{}) + throwSpecViolation, message, updateNeeded, statusMismatch := handleKeys( + obj.desiredObj, obj.existingObj, existingObjectCopy, complianceType, mdComplianceType, !r.DryRunSupported, + ) + if message != "" { + return true, message, true, false + } - // if either isn't found, they'll just be empty - mergedAnnotations, _, _ := unstructured.NestedStringMap(mdMap, "annotations") - mergedLabels, _, _ := unstructured.NestedStringMap(mdMap, "labels") + if updateNeeded { + mismatchLog := "Detected value mismatch" - obj.existingObj.SetAnnotations(mergedAnnotations) - obj.existingObj.SetLabels(mergedLabels) - } else { - obj.existingObj.UnstructuredContent()[key] = mergedObj + // Add a configuration breadcrumb for users that might be looking in the logs for a diff + if objectT.RecordDiff != policyv1.RecordDiffLog { + mismatchLog += " (Diff disabled. To log the diff, " + + "set 'spec.object-tempates[].recordDiff' to 'Log' for this object-template.)" } - if keyUpdateNeeded { - if isStatus { - throwSpecViolation = true - statusMismatch = true - - log.Info("Ignoring an update to the object status", "key", key) - } else { - updateNeeded = true + log.Info(mismatchLog) - mismatchLog := "Detected value mismatch for object key: " + key - // Add a configuration breadcrumb for users that might be looking in the logs for a diff - if objectT.RecordDiff != policyv1.RecordDiffLog { - mismatchLog += " (Diff disabled. To log the diff, " + - "set 'spec.object-tempates[].recordDiff' to 'Log' for this object-template.)" - } - log.Info(mismatchLog) - } - } - } - - if updateNeeded { // FieldValidation is supported in k8s 1.25 as beta release // so if the version is below 1.25, we need to use client side validation to validate the object if semver.Compare(r.serverVersion, "v1.25.0") < 0 { @@ -2783,6 +2739,68 @@ func (r *ConfigurationPolicyReconciler) checkAndUpdateResource( return throwSpecViolation, "", updateNeeded, updateSucceeded } +// handleKeys goes through all of the fields in the desired object and checks if the existing object +// matches. When a field is a map or slice, the value in the existing object will be updated with +// the result of merging its current value with the desired value. +func handleKeys( + desiredObj unstructured.Unstructured, + existingObj *unstructured.Unstructured, + existingObjectCopy *unstructured.Unstructured, + compType string, + mdCompType string, + zeroValueEqualsNil bool, +) (throwSpecViolation bool, message string, updateNeeded bool, statusMismatch bool) { + for key := range desiredObj.Object { + isStatus := key == "status" + + // use metadatacompliancetype to evaluate metadata if it is set + keyComplianceType := compType + if key == "metadata" && mdCompType != "" { + keyComplianceType = mdCompType + } + + // check key for mismatch + errorMsg, keyUpdateNeeded, mergedObj, skipped := handleSingleKey( + key, desiredObj, existingObjectCopy, keyComplianceType, zeroValueEqualsNil, + ) + if errorMsg != "" { + log.Info(errorMsg) + + return true, errorMsg, true, statusMismatch + } + + if mergedObj == nil && skipped { + continue + } + + // only look at labels and annotations for metadata - configurationPolicies do not update other metadata fields + if key == "metadata" { + // if it's not the right type, the map will be empty + mdMap, _ := mergedObj.(map[string]interface{}) + + // if either isn't found, they'll just be empty + mergedAnnotations, _, _ := unstructured.NestedStringMap(mdMap, "annotations") + mergedLabels, _, _ := unstructured.NestedStringMap(mdMap, "labels") + + existingObj.SetAnnotations(mergedAnnotations) + existingObj.SetLabels(mergedLabels) + } else { + existingObj.UnstructuredContent()[key] = mergedObj + } + + if keyUpdateNeeded { + if isStatus { + throwSpecViolation = true + statusMismatch = true + } else { + updateNeeded = true + } + } + } + + return +} + func removeFieldsForComparison(obj *unstructured.Unstructured) { unstructured.RemoveNestedField(obj.Object, "metadata", "managedFields") unstructured.RemoveNestedField( diff --git a/controllers/operatorpolicy_controller.go b/controllers/operatorpolicy_controller.go index 92808672..b0e6fe74 100644 --- a/controllers/operatorpolicy_controller.go +++ b/controllers/operatorpolicy_controller.go @@ -5,17 +5,21 @@ package controllers import ( "context" + "encoding/json" + "errors" "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" + "k8s.io/apimachinery/pkg/util/validation" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" @@ -24,7 +28,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 +40,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 +78,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 +104,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,230 +112,182 @@ 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) - - // (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) - } + OpLog.Info("Reconciling OperatorPolicy") - // 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 - - // Exists indicates if a Subscription or Operatorgroup need to be created - exists := subExists && ogExists - shouldExist := policy.Spec.ComplianceType.IsMustHave() + // FUTURE: more resource checks - // 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) } + } + case 1: + opGroup := foundOpGroups[0] - // Created successfully, requeue result - r.setCompliance(ctx, policy, policyv1.NonCompliant) + // Check if what's on the cluster matches what the policy wants (whether it's specified or not) - 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 + emptyNameMatch := desiredOpGroup.Name == "" && opGroup.GetGenerateName() == desiredOpGroup.GenerateName - // Set to non compliant because operatorgroup does not exist on cluster, but - // should still go to check subscription - r.setCompliance(ctx, policy, policyv1.NonCompliant) - } - } + if !(opGroup.GetName() == desiredOpGroup.Name || emptyNameMatch) { + if policy.Spec.OperatorGroup == nil { + // The policy doesn't specify what the OperatorGroup should look like, but what is already + // there is not the default one the policy would create. + // FUTURE: check if the one operator group is compatible with the desired subscription. + // For an initial implementation, assume if an OperatorGroup already exists, then it's a good one. + err := r.updateStatus(ctx, policy, opGroupPreexistingCond, matchedObj(&opGroup)) + if err != nil { + return fmt.Errorf("error updating the status for a pre-existing OperatorGroup: %w", err) + } - // Create new subscription - if cachedSubscription == nil { - if policy.Spec.RemediationAction.IsEnforce() { - subscriptionSpec, err := buildSubscription(policy) - if err != nil { - return reconcile.Result{}, err + return nil } - err = r.Create(ctx, subscriptionSpec) - if err != nil { - OpLog.Error(err, "Could not handle missing musthave object") - r.setCompliance(ctx, policy, policyv1.NonCompliant) + // 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) - return reconcile.Result{}, err + 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) } - // Future work: Check availability/status of all resources - // managed by the OperatorPolicy before setting compliance state - r.setCompliance(ctx, policy, policyv1.Compliant) - - return reconcile.Result{}, nil + return nil } - // 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 + // 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) + } - 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 - } + merged := opGroup.DeepCopy() // Copy it so that the value in the cache is not changed - err = r.Status().Update(ctx, policy) - if err != nil { - OpLog.Info(fmt.Sprintf("Failed to update policy status: %s", err)) + updateNeeded, skipUpdate, err := r.mergeObjects( + ctx, desiredUnstruct, merged, string(policy.Spec.ComplianceType), + ) + if err != nil { + return fmt.Errorf("error checking if the OperatorGroup needs an update: %w", err) + } - return err - } + if !updateNeeded { + // Everything relevant matches! + 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) + } - return nil -} + 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) + // Specs don't match. - sub := make(map[string]interface{}) + if policy.Spec.OperatorGroup == nil { + // The policy doesn't specify what the OperatorGroup should look like, but what is already + // there is not the default one the policy would create. + // FUTURE: check if the one operator group is compatible with the desired subscription. + // For an initial implementation, assume if an OperatorGroup already exists, then it's a good one. + err := r.updateStatus(ctx, policy, opGroupPreexistingCond, matchedObj(&opGroup)) + if err != nil { + return fmt.Errorf("error updating the status for a pre-existing OperatorGroup: %w", err) + } - err := json.Unmarshal(policy.Spec.Subscription.Raw, &sub) - if err != nil { - return nil, fmt.Errorf("error unmarshalling subscription: %w", err) - } + return nil + } - ns, ok := sub["namespace"].(string) - if !ok { - return nil, fmt.Errorf("namespace is required in spec.subscription") - } + if policy.Spec.RemediationAction.IsEnforce() && skipUpdate { + err = r.updateStatus(ctx, policy, mismatchCondUnfixable("OperatorGroup"), mismatchedObj(&opGroup)) + if err != nil { + return fmt.Errorf("error updating status for an unenforceable mismatched OperatorGroup: %w", err) + } - spec := new(operatorv1alpha1.SubscriptionSpec) + return nil + } - err = json.Unmarshal(policy.Spec.Subscription.Raw, spec) - if err != nil { - return nil, fmt.Errorf("error unmarshalling subscription: %w", err) - } + // 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) + } - subscription.SetGroupVersionKind(subscriptionGVK) - subscription.ObjectMeta.Name = spec.Package - subscription.ObjectMeta.Namespace = ns - subscription.Spec = spec + if policy.Spec.RemediationAction.IsEnforce() { + desiredOpGroup.ResourceVersion = opGroup.GetResourceVersion() - return subscription, nil -} + err := r.Update(ctx, merged) + if err != nil { + return fmt.Errorf("error updating the OperatorGroup: %w", err) + } -// Sets the compliance of the policy -func (r *OperatorPolicyReconciler) setCompliance( - ctx context.Context, - policy *policyv1beta1.OperatorPolicy, - compliance policyv1.ComplianceState, -) { - policy.Status.ComplianceState = compliance + desiredOpGroup.SetGroupVersionKind(operatorGroupGVK) // Update stripped this information - err := r.updatePolicyStatus(ctx, policy) - if err != nil { - OpLog.Error(err, "error while updating policy status") + // 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) + } + } + 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) + } } + + return nil } // buildOperatorGroup bootstraps the OperatorGroup spec defined in the operator policy @@ -365,6 +312,10 @@ func buildOperatorGroup( return nil, fmt.Errorf("namespace is required in spec.subscription") } + if validationErrs := validation.IsDNS1123Label(subNamespace); len(validationErrs) != 0 { + return nil, fmt.Errorf("the namespace specified in spec.subscription is not a valid namespace identifier") + } + // Create a default OperatorGroup if one wasn't specified in the policy if policy.Spec.OperatorGroup == nil { operatorGroup.ObjectMeta.SetNamespace(subNamespace) @@ -384,7 +335,7 @@ func buildOperatorGroup( // Fallback to the Subscription namespace if the OperatorGroup namespace is not specified in the policy. ogNamespace := subNamespace - if specifiedNS, ok := opGroup["namespace"].(string); ok { + if specifiedNS, ok := opGroup["namespace"].(string); ok || specifiedNS == "" { ogNamespace = specifiedNS } @@ -406,3 +357,187 @@ 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 getting the Subscription: %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) + } + } + + return nil + } + + // 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) + } + + merged := foundSub.DeepCopy() // Copy it so that the value in the cache is not changed + + updateNeeded, skipUpdate, err := r.mergeObjects(ctx, desiredUnstruct, merged, string(policy.Spec.ComplianceType)) + if err != nil { + return fmt.Errorf("error checking if the Subscription needs an update: %w", err) + } + + if !updateNeeded { + // FUTURE: Check more details about the *status* of the Subscription + // For now, just mark it as compliant + 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) + } + + return nil + } + + // Specs don't match. + if policy.Spec.RemediationAction.IsEnforce() && skipUpdate { + err = r.updateStatus(ctx, policy, mismatchCondUnfixable("Subscription"), mismatchedObj(foundSub)) + if err != nil { + return fmt.Errorf("error updating status for a mismatched Subscription that can't be enforced: %w", err) + } + + return nil + } + + 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() { + err := r.Update(ctx, merged) + 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, + } +} + +// mergeObjects takes fields from the desired object and sets/merges them on the +// existing object. It checks and returns whether an update is really necessary +// with a server-side dry-run. +func (r *OperatorPolicyReconciler) mergeObjects( + ctx context.Context, + desired map[string]interface{}, + existing *unstructured.Unstructured, + complianceType string, +) (updateNeeded, updateIsForbidden bool, err error) { + desiredObj := unstructured.Unstructured{Object: desired} + + // Use a copy since some values can be directly assigned to mergedObj in handleSingleKey. + existingObjectCopy := existing.DeepCopy() + removeFieldsForComparison(existingObjectCopy) + + _, errMsg, updateNeeded, _ := handleKeys( + desiredObj, existing, existingObjectCopy, complianceType, "", false, + ) + if errMsg != "" { + return updateNeeded, false, errors.New(errMsg) + } + + if updateNeeded { + err := r.Update(ctx, existing, client.DryRunAll) + if err != nil { + if k8serrors.IsForbidden(err) { + // This indicates the update would make a change, but the change is not allowed, + // for example, the changed field might be immutable. + // The policy should be marked as noncompliant, but an enforcement update would fail. + return true, true, nil + } + + return updateNeeded, false, err + } + + removeFieldsForComparison(existing) + + if reflect.DeepEqual(existing.Object, existingObjectCopy.Object) { + // The dry run indicates that there is not *really* a mismatch. + updateNeeded = false + } + } + + return updateNeeded, false, nil +} diff --git a/controllers/operatorpolicy_status.go b/controllers/operatorpolicy_status.go new file mode 100644 index 00000000..e89372c5 --- /dev/null +++ b/controllers/operatorpolicy_status.go @@ -0,0 +1,453 @@ +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 requires 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 := make(map[int]policyv1.RelatedObject) + if len(updatedRelatedObjs) != 0 { + prevRelObjs = policy.Status.RelatedObjsOfKind(updatedRelatedObjs[0].Object.Kind) + } + + for _, prevObj := range prevRelObjs { + nameFound := false + + for i, updatedObj := range updatedRelatedObjs { + if prevObj.Object.Metadata.Name != updatedObj.Object.Metadata.Name { + continue + } + + 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 || prevObj.Reason != updatedObj.Reason { + relObjsChanged = true + } + } + + if !nameFound { + relObjsChanged = true + } + } + + // Catch the case where there is a new object in updatedRelatedObjs + if len(prevRelObjs) != len(updatedRelatedObjs) { + 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", + } +} + +// mismatchCondUnfixable returns a NonCompliant condition with a Reason like '____Mismatch', +// and a Message like 'the ____ found on the cluster does not match the policy and can't be enforced' +func mismatchCondUnfixable(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 and can't be enforced", + } +} + +// 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", + } +} + +var opGroupPreexistingCond = metav1.Condition{ + Type: opGroupConditionType, + Status: metav1.ConditionTrue, + Reason: "PreexistingOperatorGroupFound", + Message: "the policy does not specify an OperatorGroup but one already exists in the namespace - " + + "assuming that OperatorGroup is correct", +} + +// 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/deploy/crds/kustomize_operatorpolicy/kustomization.yaml b/deploy/crds/kustomize_operatorpolicy/kustomization.yaml index ba26658e..e836fde8 100644 --- a/deploy/crds/kustomize_operatorpolicy/kustomization.yaml +++ b/deploy/crds/kustomize_operatorpolicy/kustomization.yaml @@ -9,3 +9,9 @@ patches: version: v1 kind: CustomResourceDefinition name: operatorpolicies.policy.open-cluster-management.io +- path: template-label.json + target: + group: apiextensions.k8s.io + version: v1 + kind: CustomResourceDefinition + name: operatorpolicies.policy.open-cluster-management.io diff --git a/deploy/crds/kustomize_operatorpolicy/template-label.json b/deploy/crds/kustomize_operatorpolicy/template-label.json new file mode 100644 index 00000000..7d9cc120 --- /dev/null +++ b/deploy/crds/kustomize_operatorpolicy/template-label.json @@ -0,0 +1,7 @@ +[ + { + "op":"add", + "path":"/metadata/labels", + "value": {"policy.open-cluster-management.io/policy-type": "template"} + } +] diff --git a/deploy/crds/policy.open-cluster-management.io_operatorpolicies.yaml b/deploy/crds/policy.open-cluster-management.io_operatorpolicies.yaml index 810d7074..54b4bbf1 100644 --- a/deploy/crds/policy.open-cluster-management.io_operatorpolicies.yaml +++ b/deploy/crds/policy.open-cluster-management.io_operatorpolicies.yaml @@ -6,6 +6,8 @@ metadata: annotations: controller-gen.kubebuilder.io/version: v0.6.1 creationTimestamp: null + labels: + policy.open-cluster-management.io/policy-type: template name: operatorpolicies.policy.open-cluster-management.io spec: group: policy.open-cluster-management.io 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 32f8351b..10b53019 100644 --- a/test/e2e/case38_install_operator_test.go +++ b/test/e2e/case38_install_operator_test.go @@ -1,126 +1,518 @@ package e2e import ( + "encoding/json" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + 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, + ) { + checkFunc := func(g Gomega) { + GinkgoHelper() - It("Should create the OperatorGroup", func() { - og := utils.GetWithTimeout(clientManagedDynamic, gvrOperatorGroup, - case38OgName, case38OpPolicyNS, true, defaultTimeoutSeconds) + unstructPolicy := utils.GetWithTimeout(clientManagedDynamic, gvrOperatorPolicy, polName, + opPolTestNS, true, opPolTimeout) - Expect(og).NotTo(BeNil()) - }) + policyJSON, err := json.MarshalIndent(unstructPolicy.Object, "", " ") + g.Expect(err).NotTo(HaveOccurred()) - It("Should create the Subscription from the spec", func() { - sub := utils.GetWithTimeout(clientManagedDynamic, gvrSubscription, - case38SubscriptionName, case38OpPolicyNS, true, defaultTimeoutSeconds) + GinkgoWriter.Printf("Debug info for failure.\npolicy JSON: %s\nwanted related objects: %+v\n"+ + "wanted condition: %+v\n", string(policyJSON), expectedRelatedObjs, expectedCondition) - Expect(sub).NotTo(BeNil()) - }) + policy := policyv1beta1.OperatorPolicy{} + err = json.Unmarshal(policyJSON, &policy) + g.Expect(err).NotTo(HaveOccurred()) - It("Should become Compliant", func() { - OpPlc := utils.GetWithTimeout(clientManagedDynamic, gvrOperatorPolicy, - case38OpPolicyName, case38OpPolicyNS, true, defaultTimeoutSeconds) + if wantNonCompliant { + g.Expect(policy.Status.ComplianceState).To(Equal(policyv1.NonCompliant)) + } - Expect(utils.GetComplianceState(OpPlc)).To(Equal("Compliant")) - }) + matchingRelated := policy.Status.RelatedObjsOfKind(expectedRelatedObjs[0].Object.Kind) + g.Expect(matchingRelated).To(HaveLen(len(expectedRelatedObjs))) - It("Should have installed the operator", func() { - deployment := utils.GetWithTimeout(clientManagedDynamic, gvrDeployment, - case38DeploymentName, case38OpPolicyNS, true, defaultTimeoutSeconds*2) + for _, expectedRelObj := range expectedRelatedObjs { + foundMatchingName := false + unnamed := expectedRelObj.Object.Metadata.Name == "" - Expect(deployment).NotTo(BeNil()) - }) + 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)) + } + } + + g.Expect(foundMatchingName).To(BeTrue()) + } + + 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)) + + g.Expect(utils.GetMatchingEvents( + clientManaged, opPolTestNS, parentPolicyName, "", expectedEventMsgSnippet, opPolTimeout, + )).NotTo(BeEmpty()) + } - 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) + EventuallyWithOffset(1, checkFunc, opPolTimeout, 3).Should(Succeed()) + ConsistentlyWithOffset(1, checkFunc, 3, 1).Should(Succeed()) + } + + 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) + }) + + 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: "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 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 warn about the OperatorGroup when it doesn't match the default", func() { + utils.Kubectl("patch", "operatorpolicy", opPolName, "-n", opPolTestNS, "--type=json", "-p", + `[{"op": "replace", "path": "/spec/remediationAction", "value": "inform"}]`) + utils.Kubectl("delete", "operatorgroup", "-n", opPolTestNS, "--all") + utils.Kubectl("apply", "-f", extraOpGroupYAML, "-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: "PreexistingOperatorGroupFound", + Message: "the policy does not specify an OperatorGroup but one already exists in the namespace" + + " - assuming that OperatorGroup is correct", + }, + "assuming that OperatorGroup is correct", + ) + }) + }) + 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" + ) - 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()) + 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: "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 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() { - Eventually(func() interface{} { - OpPlc := utils.GetWithTimeout(clientManagedDynamic, gvrOperatorPolicy, - case38OpPolicyDefaultOgName, case38OpPolicyDefaultOgNS, true, defaultTimeoutSeconds) + createObjWithParent(parentPolicyYAML, parentPolicyName, + opPolYAML, opPolTestNS, gvrPolicy, gvrOperatorPolicy) + }) - return utils.GetComplianceState(OpPlc) - }, defaultTimeoutSeconds, 1).Should(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: "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 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: "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 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