diff --git a/pkg/controller/operators/olm/csv_cleanup.go b/pkg/controller/operators/olm/csv_cleanup.go new file mode 100644 index 0000000000..6edb9b367a --- /dev/null +++ b/pkg/controller/operators/olm/csv_cleanup.go @@ -0,0 +1,380 @@ +package olm + +import ( + "encoding/json" + "fmt" + "slices" + "time" + + "github.com/sirupsen/logrus" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + utilerrors "k8s.io/apimachinery/pkg/util/errors" + + "github.com/operator-framework/api/pkg/operators/v1alpha1" + + "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/ownerutil" +) + +// A CSV that fails because of its OperatorGroup loses what OLM created for it: +// the Deployments and webhook configurations of the installer, and the RBAC +// steps of its InstallPlan. A step whose object the cleanup found, verified +// as owned by the CSV and deleted is marked NotPresent in the InstallPlan +// status while the plan stays Complete. When the CSV is Pending again, a +// plan with NotPresent steps goes back to Installing and the catalog-operator +// creates those objects again. The InstallPlan status is the only record of +// the cleanup, and only the controllers write it. +// +// InstallPlan execution is at least once: a catalog-operator sync that works +// from a stale Installing copy of a plan that is already Complete creates the +// objects again. The cleanup therefore deletes on every sync of the failed +// CSV, marks or not, and never skips a step because it is already marked. + +// reinstalledStepKinds are the InstallPlan steps the cleanup deletes and the +// recovery creates again. ServiceAccounts are kept: the OperatorGroup scoped +// ServiceAccount can be one of them and the catalog-operator needs it to +// execute the plan. +var reinstalledStepKinds = []string{"Role", "RoleBinding", "ClusterRole", "ClusterRoleBinding"} + +// installPlanRecheckDelay is how long cleanupFailedCSV waits before it looks +// at an InstallPlan that is still running. +const installPlanRecheckDelay = 10 * time.Second + +// cleanupFailedCSV removes what OLM created for a CSV that failed because of +// its OperatorGroup. Deployments and webhook configurations go always. The +// objects of the InstallPlan steps go when the plan is Complete and its +// manifests can be applied again, otherwise nothing could put them back. +// Errors are returned so the sync is retried. +func (a *Operator) cleanupFailedCSV(logger *logrus.Entry, csv *v1alpha1.ClusterServiceVersion) error { + var errs []error + if err := a.cleanupCSVDeployments(logger, csv); err != nil { + errs = append(errs, err) + } + if err := a.deleteOwnedWebhooks(logger, csv); err != nil { + errs = append(errs, err) + } + + plan, err := a.installPlanFor(csv) + switch { + case err != nil: + errs = append(errs, err) + case plan == nil: + logger.Debug("no InstallPlan for the CSV, keeping its RBAC") + case plan.Status.Phase == v1alpha1.InstallPlanPhaseComplete: + if err := a.deletePlanSteps(logger, csv, plan); err != nil { + errs = append(errs, err) + } + case plan.Status.Phase == v1alpha1.InstallPlanPhaseInstalling: + // The InstallPlan is still creating resources. Look again once it is done. + logger.WithField("installplan", plan.GetName()).Debug("InstallPlan of the CSV still running, checking again later") + if err := a.csvQueueSet.RequeueAfter(csv.GetNamespace(), csv.GetName(), installPlanRecheckDelay); err != nil { + logger.WithError(err).Warn("unable to requeue") + } + default: + // Failed, RequiresApproval or not started: nothing will create the RBAC again. + logger.WithFields(logrus.Fields{"installplan": plan.GetName(), "planPhase": plan.Status.Phase}).Debug("InstallPlan of the CSV cannot run, keeping its RBAC") + } + return utilerrors.NewAggregate(errs) +} + +// installPlanFor returns the newest InstallPlan that installed the CSV, or +// nil when there is none. A plan installed the CSV when its status, which +// only the catalog-operator writes, holds the step that created the CSV. +func (a *Operator) installPlanFor(csv *v1alpha1.ClusterServiceVersion) (*v1alpha1.InstallPlan, error) { + plans, err := a.client.OperatorsV1alpha1().InstallPlans(csv.GetNamespace()).List(a.ctx, metav1.ListOptions{}) + if err != nil { + return nil, err + } + var newest *v1alpha1.InstallPlan + for i := range plans.Items { + plan := &plans.Items[i] + if !planInstalledCSV(plan, csv) { + continue + } + if newest == nil || newest.CreationTimestamp.Before(&plan.CreationTimestamp) { + newest = plan + } + } + return newest, nil +} + +func planInstalledCSV(plan *v1alpha1.InstallPlan, csv *v1alpha1.ClusterServiceVersion) bool { + for _, step := range plan.Status.Plan { + if step.Resolving == csv.GetName() && step.Resource.Kind == v1alpha1.ClusterServiceVersionKind { + return true + } + } + return false +} + +// csvSteps returns the RBAC steps of the plan that belong to the CSV. +func csvSteps(plan *v1alpha1.InstallPlan, csv *v1alpha1.ClusterServiceVersion) []*v1alpha1.Step { + var steps []*v1alpha1.Step + for _, step := range plan.Status.Plan { + if step.Resolving == csv.GetName() && slices.Contains(reinstalledStepKinds, step.Resource.Kind) { + steps = append(steps, step) + } + } + return steps +} + +// deletePlanSteps deletes the objects of the CSV's RBAC steps that exist and +// that the CSV owns, and marks those steps NotPresent. The marks are written +// before the objects are deleted: a failed delete after the mark is harmless, +// the reopened plan finds the object and marks it Present again, while a +// delete without a mark would leave nothing to recover from. An object that +// is already gone, or that another owner holds, is left alone and its step +// keeps its status. The plan stays Complete. Nothing is deleted when a step +// manifest of the CSV cannot be applied again. +func (a *Operator) deletePlanSteps(logger *logrus.Entry, csv *v1alpha1.ClusterServiceVersion, plan *v1alpha1.InstallPlan) error { + logger = logger.WithField("installplan", plan.GetName()) + + // ExecutePlan resolves the manifests of every step of the plan, also the + // steps of other CSVs and the steps that are already Present. + resolvable, err := a.stepManifestsResolvable(plan.Status.Plan) + if err != nil { + return err + } + if !resolvable { + logger.Debug("InstallPlan manifests cannot be applied again, keeping the RBAC of the CSV") + return nil + } + + type victim struct { + step *v1alpha1.Step + uid types.UID + } + var victims []victim + var errs []error + changed := false + for _, step := range csvSteps(plan, csv) { + object, err := a.getStepObject(plan.GetNamespace(), step) + if apierrors.IsNotFound(err) { + continue + } + if err != nil { + errs = append(errs, err) + continue + } + if !ownedByCSV(object, csv) { + logger.WithFields(logrus.Fields{"kind": step.Resource.Kind, "name": step.Resource.Name}).Debug("InstallPlan step object is not owned by this CSV, keeping it") + // Someone else holds that name now. Drop a mark from an earlier + // cleanup so the reopened plan does not overwrite their object. + if step.Status == v1alpha1.StepStatusNotPresent { + step.Status = v1alpha1.StepStatusPresent + changed = true + } + continue + } + victims = append(victims, victim{step: step, uid: object.GetUID()}) + if step.Status != v1alpha1.StepStatusNotPresent { + step.Status = v1alpha1.StepStatusNotPresent + changed = true + } + } + if changed { + if _, err := a.client.OperatorsV1alpha1().InstallPlans(plan.GetNamespace()).UpdateStatus(a.ctx, plan, metav1.UpdateOptions{}); err != nil { + errs = append(errs, err) + return utilerrors.NewAggregate(errs) + } + } + unconfirmed := false + for _, v := range victims { + stepLogger := logger.WithFields(logrus.Fields{"kind": v.step.Resource.Kind, "name": v.step.Resource.Name}) + deleted, err := a.deleteStepObject(plan.GetNamespace(), v.step, v.uid) + if err != nil { + stepLogger.WithError(err).Warn("error cleaning up InstallPlan step object") + errs = append(errs, err) + continue + } + if !deleted { + // Someone else deleted the object between the read and the delete. + // That was their decision, drop the mark so the plan does not undo it. + stepLogger.Info("InstallPlan step object was deleted by someone else, not marked for reinstall") + v.step.Status = v1alpha1.StepStatusPresent + unconfirmed = true + continue + } + stepLogger.Info("cleaned up InstallPlan step object") + } + if unconfirmed { + if _, err := a.client.OperatorsV1alpha1().InstallPlans(plan.GetNamespace()).UpdateStatus(a.ctx, plan, metav1.UpdateOptions{}); err != nil { + errs = append(errs, err) + } + } + return utilerrors.NewAggregate(errs) +} + +// ownedByCSV reports whether the object carries the CSV owner labels or an +// owner reference to the CSV. +func ownedByCSV(object metav1.Object, csv *v1alpha1.ClusterServiceVersion) bool { + labels := object.GetLabels() + if labels[ownerutil.OwnerKey] == csv.GetName() { + namespace, ok := labels[ownerutil.OwnerNamespaceKey] + if !ok || namespace == csv.GetNamespace() { + return true + } + } + return ownerutil.IsOwnedBy(object, csv) +} + +// getStepObject reads the object of an RBAC step. +func (a *Operator) getStepObject(namespace string, step *v1alpha1.Step) (metav1.Object, error) { + kube := a.opClient.KubernetesInterface().RbacV1() + name := step.Resource.Name + options := metav1.GetOptions{} + switch step.Resource.Kind { + case "Role": + return kube.Roles(namespace).Get(a.ctx, name, options) + case "RoleBinding": + return kube.RoleBindings(namespace).Get(a.ctx, name, options) + case "ClusterRole": + return kube.ClusterRoles().Get(a.ctx, name, options) + case "ClusterRoleBinding": + return kube.ClusterRoleBindings().Get(a.ctx, name, options) + } + return nil, fmt.Errorf("step kind %s is not deleted on failure", step.Resource.Kind) +} + +// unpackedBundleReference is the manifest of a step whose real manifest +// lives in the ConfigMap of an unpacked bundle. It mirrors the catalog +// package type of the same name. +type unpackedBundleReference struct { + Kind string `json:"kind"` + Name string `json:"name"` + Namespace string `json:"namespace"` + CatalogSourceName string `json:"catalogSourceName"` + CatalogSourceNamespace string `json:"catalogSourceNamespace"` +} + +// stepManifestsResolvable reports whether the catalog-operator can apply +// every step again: the manifest is inline, or it refers to an unpacked +// bundle ConfigMap that still exists. +func (a *Operator) stepManifestsResolvable(steps []*v1alpha1.Step) (bool, error) { + for _, step := range steps { + if step.Resource.Manifest == "" { + return false, nil + } + var ref unpackedBundleReference + if err := json.Unmarshal([]byte(step.Resource.Manifest), &ref); err != nil { + continue + } + if ref.Kind != "ConfigMap" || ref.Name == "" || ref.Namespace == "" || ref.CatalogSourceName == "" || ref.CatalogSourceNamespace == "" { + continue + } + if _, err := a.opClient.KubernetesInterface().CoreV1().ConfigMaps(ref.Namespace).Get(a.ctx, ref.Name, metav1.GetOptions{}); err != nil { + if apierrors.IsNotFound(err) { + return false, nil + } + return false, err + } + } + return true, nil +} + +// deleteStepObject deletes the object of an RBAC step, and only the object +// with the given UID. It reports whether this call deleted the object: a +// missing object is not an error but was not deleted by us. A UID conflict +// means the object was replaced between the read and the delete, the error +// is returned so the next sync reads the replacement and fixes the mark. +func (a *Operator) deleteStepObject(namespace string, step *v1alpha1.Step, uid types.UID) (bool, error) { + kube := a.opClient.KubernetesInterface().RbacV1() + name := step.Resource.Name + options := metav1.DeleteOptions{Preconditions: &metav1.Preconditions{UID: &uid}} + var err error + switch step.Resource.Kind { + case "Role": + err = kube.Roles(namespace).Delete(a.ctx, name, options) + case "RoleBinding": + err = kube.RoleBindings(namespace).Delete(a.ctx, name, options) + case "ClusterRole": + err = kube.ClusterRoles().Delete(a.ctx, name, options) + case "ClusterRoleBinding": + err = kube.ClusterRoleBindings().Delete(a.ctx, name, options) + default: + return false, fmt.Errorf("step kind %s is not deleted on failure", step.Resource.Kind) + } + if apierrors.IsNotFound(err) { + return false, nil + } + return err == nil, err +} + +// deleteOwnedWebhooks deletes the webhook configurations the installer +// created for the CSV. A webhook that points at a deleted Deployment would +// block the API calls it covers. The installer creates them again when the +// CSV is installed again. +func (a *Operator) deleteOwnedWebhooks(logger *logrus.Entry, csv *v1alpha1.ClusterServiceVersion) error { + admission := a.opClient.KubernetesInterface().AdmissionregistrationV1() + listOptions := metav1.ListOptions{LabelSelector: ownerutil.CSVOwnerSelector(csv).String()} + var errs []error + + validating, err := admission.ValidatingWebhookConfigurations().List(a.ctx, listOptions) + if err != nil { + errs = append(errs, err) + } else { + for _, webhook := range validating.Items { + uid := webhook.GetUID() + if err := admission.ValidatingWebhookConfigurations().Delete(a.ctx, webhook.GetName(), metav1.DeleteOptions{Preconditions: &metav1.Preconditions{UID: &uid}}); err != nil && !apierrors.IsNotFound(err) { + errs = append(errs, err) + continue + } + logger.WithField("validatingwebhookconfiguration", webhook.GetName()).Info("cleaned up CSV webhook") + } + } + mutating, err := admission.MutatingWebhookConfigurations().List(a.ctx, listOptions) + if err != nil { + errs = append(errs, err) + } else { + for _, webhook := range mutating.Items { + uid := webhook.GetUID() + if err := admission.MutatingWebhookConfigurations().Delete(a.ctx, webhook.GetName(), metav1.DeleteOptions{Preconditions: &metav1.Preconditions{UID: &uid}}); err != nil && !apierrors.IsNotFound(err) { + errs = append(errs, err) + continue + } + logger.WithField("mutatingwebhookconfiguration", webhook.GetName()).Info("cleaned up CSV webhook") + } + } + return utilerrors.NewAggregate(errs) +} + +// reinstallPlanSteps makes the catalog-operator create again what +// deletePlanSteps removed. When the CSV's Complete InstallPlan has steps that +// are NotPresent, the plan goes back to Installing with a fresh start time. +// The catalog-operator executes those steps with its usual logic and client +// and the plan returns to Complete. A plan without NotPresent steps is left +// alone, so objects that someone else removed are not created again. The +// check is a pure read of the marks and runs on every sync of a Pending CSV, +// whether or not its requirements are met. +func (a *Operator) reinstallPlanSteps(logger *logrus.Entry, csv *v1alpha1.ClusterServiceVersion) error { + plan, err := a.installPlanFor(csv) + if err != nil || plan == nil || plan.Status.Phase != v1alpha1.InstallPlanPhaseComplete { + return err + } + pending := false + for _, step := range csvSteps(plan, csv) { + if step.Status == v1alpha1.StepStatusNotPresent { + pending = true + break + } + } + if !pending { + return nil + } + + plan.Status.Phase = v1alpha1.InstallPlanPhaseInstalling + plan.Status.StartTime = nil + plan.Status.Message = "reinstalling resources removed while the ClusterServiceVersion was failed" + plan.Status.SetCondition(v1alpha1.InstallPlanCondition{ + Type: v1alpha1.InstallPlanInstalled, + Status: corev1.ConditionUnknown, + Reason: v1alpha1.InstallPlanReasonPlanUnknown, + Message: plan.Status.Message, + LastTransitionTime: a.now(), + }) + logger.WithField("installplan", plan.GetName()).Info("reinstalling InstallPlan steps for recovered CSV") + _, err = a.client.OperatorsV1alpha1().InstallPlans(plan.GetNamespace()).UpdateStatus(a.ctx, plan, metav1.UpdateOptions{}) + return err +} diff --git a/pkg/controller/operators/olm/csv_cleanup_test.go b/pkg/controller/operators/olm/csv_cleanup_test.go new file mode 100644 index 0000000000..57a8184258 --- /dev/null +++ b/pkg/controller/operators/olm/csv_cleanup_test.go @@ -0,0 +1,1226 @@ +package olm + +import ( + "context" + "errors" + "slices" + "testing" + "time" + + "github.com/sirupsen/logrus" + logrustest "github.com/sirupsen/logrus/hooks/test" + "github.com/stretchr/testify/require" + admissionregistrationv1 "k8s.io/api/admissionregistration/v1" + corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + k8sfake "k8s.io/client-go/kubernetes/fake" + clienttesting "k8s.io/client-go/testing" + + apiregistrationv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1" + + operatorsv1 "github.com/operator-framework/api/pkg/operators/v1" + "github.com/operator-framework/api/pkg/operators/v1alpha1" + + "github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/clientset/versioned/fake" + "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install" + "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/ownerutil" + "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/queueinformer" +) + +// The fixtures model one operator installed by an InstallPlan: the CSV, its +// Deployment, its ServiceAccount, and the RBAC the InstallPlan created. The +// plan status lists every object as a Present step with an inline manifest. + +const ( + cleanupNamespace = "ns" + cleanupCSVName = "csv1" + cleanupCSVUID = types.UID("csv1-uid") + cleanupDeployment = "a1" + cleanupSA = "sa" + cleanupPlanName = "install-1" +) + +var cleanupRBACNames = map[string]string{ + "Role": "csv1-role", + "RoleBinding": "csv1-rolebinding", + "ClusterRole": "csv1-clusterrole", + "ClusterRoleBinding": "csv1-clusterrolebinding", +} + +// cleanupStepKinds are the step kinds the cleanup deletes. ServiceAccounts +// are plan steps too but are never deleted. +var cleanupStepKinds = []string{"Role", "RoleBinding", "ClusterRole", "ClusterRoleBinding"} + +func cleanupOperatorGroup(static bool) *operatorsv1.OperatorGroup { + return &operatorsv1.OperatorGroup{ + TypeMeta: metav1.TypeMeta{Kind: "OperatorGroup", APIVersion: operatorsv1.SchemeGroupVersion.String()}, + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + Namespace: cleanupNamespace, + // Already annotated, otherwise the first sync only annotates the group and returns. + Annotations: map[string]string{operatorsv1.OperatorGroupProvidedAPIsAnnotationKey: "c1.v1.g1"}, + }, + Spec: operatorsv1.OperatorGroupSpec{StaticProvidedAPIs: static}, + Status: operatorsv1.OperatorGroupStatus{Namespaces: []string{cleanupNamespace}}, + } +} + +func cleanupAnnotations() map[string]string { + return map[string]string{ + operatorsv1.OperatorGroupTargetsAnnotationKey: cleanupNamespace, + operatorsv1.OperatorGroupNamespaceAnnotationKey: cleanupNamespace, + operatorsv1.OperatorGroupAnnotationKey: "default", + } +} + +var unsupportedInstallModes = []v1alpha1.InstallMode{{Type: v1alpha1.InstallModeTypeSingleNamespace, Supported: false}} + +// cleanupCSV needs a Role for its ServiceAccount, so the requirements stay +// unmet while the RBAC the InstallPlan created is missing. +func cleanupCSV(phase v1alpha1.ClusterServiceVersionPhase, reason v1alpha1.ConditionReason) *v1alpha1.ClusterServiceVersion { + permissions := []v1alpha1.StrategyDeploymentPermissions{{ + ServiceAccountName: cleanupSA, + Rules: []rbacv1.PolicyRule{{Verbs: []string{"get"}, APIGroups: []string{""}, Resources: []string{"configmaps"}}}, + }} + out := withConditionReason(csvWithAnnotations(csv(cleanupCSVName, + cleanupNamespace, + "0.0.0", + "", + installStrategy(cleanupDeployment, permissions, nil), + []*apiextensionsv1.CustomResourceDefinition{crd("c1", "v1", "g1")}, + []*apiextensionsv1.CustomResourceDefinition{}, + phase, + ), cleanupAnnotations()), reason) + out.UID = cleanupCSVUID + return out +} + +// failedUnsupportedCSV is a Succeeded CSV that fails on the next sync with +// UnsupportedOperatorGroup. +func failedUnsupportedCSV() *v1alpha1.ClusterServiceVersion { + return withInstallModes(cleanupCSV(v1alpha1.CSVPhaseSucceeded, v1alpha1.CSVReasonInstallSuccessful), unsupportedInstallModes) +} + +func ownedDeployment(csv *v1alpha1.ClusterServiceVersion) runtime.Object { + d := deployment(cleanupDeployment, cleanupNamespace, cleanupSA, cleanupAnnotations()) + ownerutil.AddNonBlockingOwner(d, csv) + return d +} + +// ownedRBAC returns the ServiceAccount, Role, RoleBinding, ClusterRole and +// ClusterRoleBinding the InstallPlan created for the CSV. +func ownedRBAC(csv *v1alpha1.ClusterServiceVersion) []runtime.Object { + l := ownerutil.OwnerLabel(csv, v1alpha1.ClusterServiceVersionKind) + l[install.OLMManagedLabelKey] = install.OLMManagedLabelValue + namespaced := func(name string) metav1.ObjectMeta { + return metav1.ObjectMeta{Name: name, Namespace: csv.GetNamespace(), Labels: l} + } + clusterScoped := func(name string) metav1.ObjectMeta { + return metav1.ObjectMeta{Name: name, Labels: l} + } + return []runtime.Object{ + serviceAccount(cleanupSA, cleanupNamespace), + &rbacv1.Role{ObjectMeta: namespaced(cleanupRBACNames["Role"])}, + &rbacv1.RoleBinding{ObjectMeta: namespaced(cleanupRBACNames["RoleBinding"])}, + &rbacv1.ClusterRole{ObjectMeta: clusterScoped(cleanupRBACNames["ClusterRole"])}, + &rbacv1.ClusterRoleBinding{ObjectMeta: clusterScoped(cleanupRBACNames["ClusterRoleBinding"])}, + } +} + +func cleanupK8sObjs(csv *v1alpha1.ClusterServiceVersion) []runtime.Object { + return append([]runtime.Object{ownedDeployment(csv)}, ownedRBAC(csv)...) +} + +// completedInstallPlan returns the Complete InstallPlan that installed the +// CSV. Every step is Present with an inline manifest. +func completedInstallPlan() *v1alpha1.InstallPlan { + return installPlanInPhase(v1alpha1.InstallPlanPhaseComplete) +} + +func installPlanInPhase(phase v1alpha1.InstallPlanPhase) *v1alpha1.InstallPlan { + start := metav1.Now() + step := func(kind, name string) *v1alpha1.Step { + return &v1alpha1.Step{ + Resolving: cleanupCSVName, + Status: v1alpha1.StepStatusPresent, + Resource: v1alpha1.StepResource{Kind: kind, Name: name, Manifest: `{"kind":"` + kind + `"}`}, + } + } + return &v1alpha1.InstallPlan{ + TypeMeta: metav1.TypeMeta{Kind: v1alpha1.InstallPlanKind, APIVersion: v1alpha1.SchemeGroupVersion.String()}, + ObjectMeta: metav1.ObjectMeta{Name: cleanupPlanName, Namespace: cleanupNamespace}, + Spec: v1alpha1.InstallPlanSpec{ + ClusterServiceVersionNames: []string{cleanupCSVName}, + Approval: v1alpha1.ApprovalAutomatic, + Approved: true, + }, + Status: v1alpha1.InstallPlanStatus{ + Phase: phase, + StartTime: &start, + Plan: []*v1alpha1.Step{ + step(v1alpha1.ClusterServiceVersionKind, cleanupCSVName), + step("CustomResourceDefinition", "c1"), + step("ServiceAccount", cleanupSA), + step("Role", cleanupRBACNames["Role"]), + step("RoleBinding", cleanupRBACNames["RoleBinding"]), + step("ClusterRole", cleanupRBACNames["ClusterRole"]), + step("ClusterRoleBinding", cleanupRBACNames["ClusterRoleBinding"]), + }, + }, + } +} + +// cleanedUpInstallPlan is the plan as deletePlanSteps leaves it: Complete, +// the RBAC steps NotPresent. +func cleanedUpInstallPlan() *v1alpha1.InstallPlan { + plan := completedInstallPlan() + for _, step := range plan.Status.Plan { + if slices.Contains(cleanupStepKinds, step.Resource.Kind) { + step.Status = v1alpha1.StepStatusNotPresent + } + } + return plan +} + +func stepStatuses(plan *v1alpha1.InstallPlan) map[string]v1alpha1.StepStatus { + statuses := map[string]v1alpha1.StepStatus{} + for _, step := range plan.Status.Plan { + statuses[step.Resource.Kind] = step.Status + } + return statuses +} + +func stepObjectExists(t *testing.T, op *Operator, kind string) bool { + t.Helper() + var err error + switch kind { + case "ServiceAccount": + _, err = op.opClient.GetServiceAccount(cleanupNamespace, cleanupSA) + case "Role": + _, err = op.opClient.GetRole(cleanupNamespace, cleanupRBACNames[kind]) + case "RoleBinding": + _, err = op.opClient.GetRoleBinding(cleanupNamespace, cleanupRBACNames[kind]) + case "ClusterRole": + _, err = op.opClient.GetClusterRole(cleanupRBACNames[kind]) + case "ClusterRoleBinding": + _, err = op.opClient.GetClusterRoleBinding(cleanupRBACNames[kind]) + } + if err == nil { + return true + } + require.True(t, apierrors.IsNotFound(err), "%s: %v", kind, err) + return false +} + +func deploymentExists(t *testing.T, op *Operator) bool { + t.Helper() + _, err := op.opClient.GetDeployment(cleanupNamespace, cleanupDeployment) + if err == nil { + return true + } + require.True(t, apierrors.IsNotFound(err), "deployment: %v", err) + return false +} + +// requireStepObjects asserts for every RBAC step kind whether its object +// exists. The ServiceAccount must exist in every case. +func requireStepObjects(t *testing.T, op *Operator, exist bool) { + t.Helper() + for _, kind := range cleanupStepKinds { + require.Equal(t, exist, stepObjectExists(t, op, kind), "%s exists", kind) + } + require.True(t, stepObjectExists(t, op, "ServiceAccount"), "ServiceAccount must never be deleted") +} + +// requirePlan asserts the phase of the stored plan and the status of its +// RBAC steps. The ServiceAccount, CSV and CRD steps must stay Present. +func requirePlan(t *testing.T, ctx context.Context, op *Operator, phase v1alpha1.InstallPlanPhase, steps v1alpha1.StepStatus) *v1alpha1.InstallPlan { + t.Helper() + plan, err := op.client.OperatorsV1alpha1().InstallPlans(cleanupNamespace).Get(ctx, cleanupPlanName, metav1.GetOptions{}) + require.NoError(t, err) + require.Equal(t, phase, plan.Status.Phase) + statuses := stepStatuses(plan) + for _, kind := range cleanupStepKinds { + require.Equal(t, steps, statuses[kind], "step %s", kind) + } + require.Equal(t, v1alpha1.StepStatusPresent, statuses[v1alpha1.ClusterServiceVersionKind], "CSV step") + require.Equal(t, v1alpha1.StepStatusPresent, statuses["CustomResourceDefinition"], "CRD step") + require.Equal(t, v1alpha1.StepStatusPresent, statuses["ServiceAccount"], "ServiceAccount step") + return plan +} + +func getCSV(t *testing.T, ctx context.Context, op *Operator) *v1alpha1.ClusterServiceVersion { + t.Helper() + got, err := op.client.OperatorsV1alpha1().ClusterServiceVersions(cleanupNamespace).Get(ctx, cleanupCSVName, metav1.GetOptions{}) + require.NoError(t, err) + return got +} + +func newCleanupOperator(t *testing.T, ctx context.Context, csv *v1alpha1.ClusterServiceVersion, clientObjs, k8sObjs []runtime.Object, reconciler APIIntersectionReconciler) *Operator { + t.Helper() + op, err := NewFakeOperator( + ctx, + withNamespaces(cleanupNamespace, "kube-system"), + withClientObjs(append(clientObjs, csv)...), + withK8sObjs(k8sObjs...), + withExtObjs(crd("c1", "v1", "g1")), + withPartialMetadata(&metav1.PartialObjectMetadata{ObjectMeta: csv.ObjectMeta}), + withOperatorNamespace(cleanupNamespace), + withAPIReconciler(reconciler), + ) + require.NoError(t, err) + return op +} + +// failingReactor makes every call with the verb and resource fail. +func failingReactor(msg string) clienttesting.ReactionFunc { + return func(clienttesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New(msg) + } +} + +func operatorsFake(op *Operator) *fake.ReactionForwardingClientsetDecorator { + return op.client.(*fake.ReactionForwardingClientsetDecorator) +} + +func kubeFake(op *Operator) *k8sfake.Clientset { + return op.opClient.KubernetesInterface().(*k8sfake.Clientset) +} + +// recover syncs a Failed CSV once so it goes back to Pending, waits for the +// informer cache, and syncs the Pending CSV once more. The second sync is +// where the InstallPlan is reopened. It returns the error of the second sync. +func recover(t *testing.T, ctx context.Context, op *Operator, csv *v1alpha1.ClusterServiceVersion) error { + t.Helper() + require.NoError(t, op.syncClusterServiceVersion(csv)) + require.Eventually(t, func() bool { + got, err := op.lister.OperatorsV1alpha1().ClusterServiceVersionLister().ClusterServiceVersions(cleanupNamespace).Get(cleanupCSVName) + return err == nil && got.Status.Phase == v1alpha1.CSVPhasePending + }, 5*time.Second, 10*time.Millisecond, "CSV did not reach Pending") + return op.syncClusterServiceVersion(getCSV(t, ctx, op)) +} + +// TestFailedCSVCleanup: a Succeeded operator whose CSV fails because of its +// OperatorGroup loses its Deployment. It loses the objects of its InstallPlan +// steps when the plan is Complete, and the plan records that as NotPresent +// steps. Otherwise the objects stay and the plan is untouched. +func TestFailedCSVCleanup(t *testing.T) { + tests := []struct { + name string + reason v1alpha1.ConditionReason + installModes []v1alpha1.InstallMode + static bool + reconciler APIIntersectionReconciler + installPlan *v1alpha1.InstallPlan + wantDeleted bool + }{ + { + name: "UnsupportedOperatorGroup", + reason: v1alpha1.CSVReasonUnsupportedOperatorGroup, + installModes: unsupportedInstallModes, + installPlan: completedInstallPlan(), + wantDeleted: true, + }, + { + name: "UnsupportedOperatorGroup/NoInstallPlan", + reason: v1alpha1.CSVReasonUnsupportedOperatorGroup, + installModes: unsupportedInstallModes, + installPlan: nil, + wantDeleted: false, + }, + { + name: "UnsupportedOperatorGroup/InstallPlanStillInstalling", + reason: v1alpha1.CSVReasonUnsupportedOperatorGroup, + installModes: unsupportedInstallModes, + installPlan: installPlanInPhase(v1alpha1.InstallPlanPhaseInstalling), + wantDeleted: false, + }, + { + name: "UnsupportedOperatorGroup/InstallPlanFailed", + reason: v1alpha1.CSVReasonUnsupportedOperatorGroup, + installModes: unsupportedInstallModes, + installPlan: installPlanInPhase(v1alpha1.InstallPlanPhaseFailed), + wantDeleted: false, + }, + { + name: "UnsupportedOperatorGroup/InstallPlanRequiresApproval", + reason: v1alpha1.CSVReasonUnsupportedOperatorGroup, + installModes: unsupportedInstallModes, + installPlan: installPlanInPhase(v1alpha1.InstallPlanPhaseRequiresApproval), + wantDeleted: false, + }, + { + name: "InterOperatorGroupOwnerConflict", + reason: v1alpha1.CSVReasonInterOperatorGroupOwnerConflict, + reconciler: buildFakeAPIIntersectionReconcilerThatReturns(APIConflict), + installPlan: completedInstallPlan(), + wantDeleted: true, + }, + { + name: "CannotModifyStaticOperatorGroupProvidedAPIs", + reason: v1alpha1.CSVReasonCannotModifyStaticOperatorGroupProvidedAPIs, + static: true, + reconciler: buildFakeAPIIntersectionReconcilerThatReturns(AddAPIs), + installPlan: completedInstallPlan(), + wantDeleted: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + csv := cleanupCSV(v1alpha1.CSVPhaseSucceeded, v1alpha1.CSVReasonInstallSuccessful) + if tt.installModes != nil { + csv = withInstallModes(csv, tt.installModes) + } + clientObjs := []runtime.Object{cleanupOperatorGroup(tt.static)} + if tt.installPlan != nil { + clientObjs = append(clientObjs, tt.installPlan) + } + op := newCleanupOperator(t, ctx, csv, clientObjs, cleanupK8sObjs(csv), tt.reconciler) + + require.NoError(t, op.syncClusterServiceVersion(csv)) + + got := getCSV(t, ctx, op) + require.Equal(t, v1alpha1.CSVPhaseFailed, got.Status.Phase) + require.Equal(t, tt.reason, got.Status.Reason) + require.False(t, deploymentExists(t, op), "deployment exists") + requireStepObjects(t, op, !tt.wantDeleted) + if tt.installPlan != nil { + steps := v1alpha1.StepStatusPresent + if tt.wantDeleted { + steps = v1alpha1.StepStatusNotPresent + } + requirePlan(t, ctx, op, tt.installPlan.Status.Phase, steps) + } + }) + } +} + +// TestFailedCSVCleanupThenRecovery: the whole cycle on one operator. Fail, +// clean up, fix the install modes, recover, and the plan is reopened for the +// steps the cleanup removed. +func TestFailedCSVCleanupThenRecovery(t *testing.T) { + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + csv := failedUnsupportedCSV() + op := newCleanupOperator(t, ctx, csv, + []runtime.Object{cleanupOperatorGroup(false), completedInstallPlan()}, + cleanupK8sObjs(csv), nil) + + require.NoError(t, op.syncClusterServiceVersion(csv)) + requireStepObjects(t, op, false) + requirePlan(t, ctx, op, v1alpha1.InstallPlanPhaseComplete, v1alpha1.StepStatusNotPresent) + + // The user fixes the install modes. + failed := getCSV(t, ctx, op) + failed.Spec.InstallModes = cleanupCSV(v1alpha1.CSVPhaseFailed, "").Spec.InstallModes + fixed, err := op.client.OperatorsV1alpha1().ClusterServiceVersions(cleanupNamespace).Update(ctx, failed, metav1.UpdateOptions{}) + require.NoError(t, err) + require.Eventually(t, func() bool { + got, err := op.lister.OperatorsV1alpha1().ClusterServiceVersionLister().ClusterServiceVersions(cleanupNamespace).Get(cleanupCSVName) + return err == nil && len(got.Spec.InstallModes) == len(fixed.Spec.InstallModes) + }, 5*time.Second, 10*time.Millisecond, "informer did not see the fixed install modes") + + require.ErrorIs(t, recover(t, ctx, op, fixed), ErrRequirementsNotMet) + plan := requirePlan(t, ctx, op, v1alpha1.InstallPlanPhaseInstalling, v1alpha1.StepStatusNotPresent) + require.Nil(t, plan.Status.StartTime, "startTime must be cleared so the install timeout starts again") + require.Equal(t, corev1.ConditionUnknown, plan.Status.GetCondition(v1alpha1.InstallPlanInstalled).Status) +} + +// TestFailedCSVCleanupIgnoresForgedInstallPlan: a newer InstallPlan that +// only names the CSV in its spec, without the status the catalog-operator +// writes, does not count. The real plan is used. +func TestFailedCSVCleanupIgnoresForgedInstallPlan(t *testing.T) { + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + forged := &v1alpha1.InstallPlan{ + ObjectMeta: metav1.ObjectMeta{Name: "install-forged", Namespace: cleanupNamespace, CreationTimestamp: metav1.NewTime(time.Now().Add(time.Hour))}, + Spec: v1alpha1.InstallPlanSpec{ClusterServiceVersionNames: []string{cleanupCSVName}}, + } + csv := failedUnsupportedCSV() + op := newCleanupOperator(t, ctx, csv, + []runtime.Object{cleanupOperatorGroup(false), completedInstallPlan(), forged}, + cleanupK8sObjs(csv), nil) + + require.NoError(t, op.syncClusterServiceVersion(csv)) + + requireStepObjects(t, op, false) + requirePlan(t, ctx, op, v1alpha1.InstallPlanPhaseComplete, v1alpha1.StepStatusNotPresent) +} + +// TestFailedCSVCleanupIgnoresOtherInstallPlans: the plan of another operator +// in the namespace does not count as the plan of this CSV. +func TestFailedCSVCleanupIgnoresOtherInstallPlans(t *testing.T) { + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + other := completedInstallPlan() + other.Name = "install-other" + other.Spec.ClusterServiceVersionNames = []string{"other-csv"} + for _, step := range other.Status.Plan { + step.Resolving = "other-csv" + } + csv := failedUnsupportedCSV() + op := newCleanupOperator(t, ctx, csv, + []runtime.Object{cleanupOperatorGroup(false), other}, + cleanupK8sObjs(csv), nil) + + require.NoError(t, op.syncClusterServiceVersion(csv)) + + require.False(t, deploymentExists(t, op), "deployment exists") + requireStepObjects(t, op, true) +} + +// TestFailedCSVCleanupUsesNewestInstallPlan: with two plans for the CSV the +// newer one decides. The newer one is Complete, so the objects are deleted. +func TestFailedCSVCleanupUsesNewestInstallPlan(t *testing.T) { + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + older := installPlanInPhase(v1alpha1.InstallPlanPhaseInstalling) + older.Name = "install-0" + older.CreationTimestamp = metav1.NewTime(time.Now().Add(-time.Hour)) + newer := completedInstallPlan() + newer.CreationTimestamp = metav1.NewTime(time.Now()) + + csv := failedUnsupportedCSV() + op := newCleanupOperator(t, ctx, csv, + []runtime.Object{cleanupOperatorGroup(false), older, newer}, + cleanupK8sObjs(csv), nil) + + require.NoError(t, op.syncClusterServiceVersion(csv)) + + requireStepObjects(t, op, false) +} + +// TestFailedCSVCleanupManifestReferences: steps of a bundle-image install +// refer to the ConfigMap of the unpacked bundle. The objects are deleted only +// while that ConfigMap exists, because the catalog-operator needs it to +// create them again. +func TestFailedCSVCleanupManifestReferences(t *testing.T) { + ref := `{"kind":"ConfigMap","name":"bundle-cm","namespace":"ns","catalogSourceName":"cat","catalogSourceNamespace":"ns"}` + tests := []struct { + name string + configMap bool + wantDeleted bool + }{ + {"ConfigMapMissing", false, false}, + {"ConfigMapPresent", true, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + plan := completedInstallPlan() + for _, step := range plan.Status.Plan { + step.Resource.Manifest = ref + } + csv := failedUnsupportedCSV() + k8sObjs := cleanupK8sObjs(csv) + if tt.configMap { + k8sObjs = append(k8sObjs, &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "bundle-cm", Namespace: cleanupNamespace}}) + } + op := newCleanupOperator(t, ctx, csv, []runtime.Object{cleanupOperatorGroup(false), plan}, k8sObjs, nil) + + require.NoError(t, op.syncClusterServiceVersion(csv)) + + requireStepObjects(t, op, !tt.wantDeleted) + }) + } +} + +// TestFailedCSVCleanupEmptyManifest: a step without a manifest cannot be +// applied again, nothing is deleted. The step belongs to another CSV of the +// same plan: ExecutePlan resolves every step of the plan. +func TestFailedCSVCleanupEmptyManifest(t *testing.T) { + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + plan := completedInstallPlan() + plan.Status.Plan = append(plan.Status.Plan, &v1alpha1.Step{Resolving: "other-csv", Status: v1alpha1.StepStatusPresent, Resource: v1alpha1.StepResource{Kind: "ClusterServiceVersion", Name: "other-csv"}}) + csv := failedUnsupportedCSV() + op := newCleanupOperator(t, ctx, csv, []runtime.Object{cleanupOperatorGroup(false), plan}, cleanupK8sObjs(csv), nil) + + require.NoError(t, op.syncClusterServiceVersion(csv)) + + requireStepObjects(t, op, true) +} + +// TestFailedCSVCleanupDeletesWebhooks: webhook configurations of the CSV are +// deleted with the Deployment, so no webhook points at a dead backend. +func TestFailedCSVCleanupDeletesWebhooks(t *testing.T) { + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + csv := failedUnsupportedCSV() + labels := ownerutil.OwnerLabel(csv, v1alpha1.ClusterServiceVersionKind) + webhooks := []runtime.Object{ + &admissionregistrationv1.ValidatingWebhookConfiguration{ObjectMeta: metav1.ObjectMeta{Name: "csv1-validating", Labels: labels}}, + &admissionregistrationv1.MutatingWebhookConfiguration{ObjectMeta: metav1.ObjectMeta{Name: "csv1-mutating", Labels: labels}}, + &admissionregistrationv1.ValidatingWebhookConfiguration{ObjectMeta: metav1.ObjectMeta{Name: "other-validating"}}, + } + op := newCleanupOperator(t, ctx, csv, []runtime.Object{cleanupOperatorGroup(false)}, append(cleanupK8sObjs(csv), webhooks...), nil) + + require.NoError(t, op.syncClusterServiceVersion(csv)) + + admission := op.opClient.KubernetesInterface().AdmissionregistrationV1() + _, err := admission.ValidatingWebhookConfigurations().Get(ctx, "csv1-validating", metav1.GetOptions{}) + require.True(t, apierrors.IsNotFound(err), "validating webhook of the CSV: %v", err) + _, err = admission.MutatingWebhookConfigurations().Get(ctx, "csv1-mutating", metav1.GetOptions{}) + require.True(t, apierrors.IsNotFound(err), "mutating webhook of the CSV: %v", err) + _, err = admission.ValidatingWebhookConfigurations().Get(ctx, "other-validating", metav1.GetOptions{}) + require.NoError(t, err, "webhook of another owner must stay") +} + +// TestFailedCSVCleanupKeepsForeignDeployment: a Deployment with the name +// from the install strategy that another CSV owns is kept. +func TestFailedCSVCleanupKeepsForeignDeployment(t *testing.T) { + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + csv := failedUnsupportedCSV() + other := cleanupCSV(v1alpha1.CSVPhaseSucceeded, v1alpha1.CSVReasonInstallSuccessful) + other.Name, other.UID = "other-csv", "other-uid" + op := newCleanupOperator(t, ctx, csv, + []runtime.Object{cleanupOperatorGroup(false), completedInstallPlan()}, + append([]runtime.Object{ownedDeployment(other)}, ownedRBAC(csv)...), nil) + + require.NoError(t, op.syncClusterServiceVersion(csv)) + + require.True(t, deploymentExists(t, op), "foreign deployment must stay") + requireStepObjects(t, op, false) +} + +// TestFailedCSVCleanupErrors: an API error during the cleanup is returned so +// the sync is retried, and the parts that did not fail still happen. +func TestFailedCSVCleanupErrors(t *testing.T) { + tests := []struct { + name string + breakOperators func(*fake.ReactionForwardingClientsetDecorator) + breakKube func(*k8sfake.Clientset) + wantErr string + wantDeployment bool + skipDeployment bool + wantObjects map[string]bool + wantSteps map[string]v1alpha1.StepStatus + }{ + { + name: "InstallPlanListFails", + breakOperators: func(c *fake.ReactionForwardingClientsetDecorator) { + c.PrependReactor("list", "installplans", failingReactor("list installplans")) + }, + wantErr: "list installplans", + wantObjects: map[string]bool{"ServiceAccount": true, "Role": true, "RoleBinding": true, "ClusterRole": true, "ClusterRoleBinding": true}, + }, + { + name: "RoleDeleteFails", + breakKube: func(c *k8sfake.Clientset) { + c.PrependReactor("delete", "roles", failingReactor("delete role")) + }, + wantErr: "delete role", + wantObjects: map[string]bool{"ServiceAccount": true, "Role": true, "RoleBinding": false, "ClusterRole": false, "ClusterRoleBinding": false}, + wantSteps: map[string]v1alpha1.StepStatus{"Role": v1alpha1.StepStatusNotPresent, "RoleBinding": v1alpha1.StepStatusNotPresent}, + }, + { + name: "RoleGetFails", + breakKube: func(c *k8sfake.Clientset) { + c.PrependReactor("get", "roles", failingReactor("get role")) + }, + wantErr: "get role", + wantObjects: map[string]bool{"ServiceAccount": true, "RoleBinding": false, "ClusterRole": false, "ClusterRoleBinding": false}, + wantSteps: map[string]v1alpha1.StepStatus{"Role": v1alpha1.StepStatusPresent, "RoleBinding": v1alpha1.StepStatusNotPresent}, + }, + { + name: "DeploymentDeleteFails", + breakKube: func(c *k8sfake.Clientset) { + c.PrependReactor("delete", "deployments", failingReactor("delete deployment")) + }, + wantErr: "delete deployment", + wantDeployment: true, + wantObjects: map[string]bool{"ServiceAccount": true, "Role": false, "RoleBinding": false, "ClusterRole": false, "ClusterRoleBinding": false}, + }, + { + name: "DeploymentGetFails", + breakKube: func(c *k8sfake.Clientset) { + c.PrependReactor("get", "deployments", failingReactor("get deployment")) + }, + wantErr: "get deployment", + skipDeployment: true, + wantObjects: map[string]bool{"ServiceAccount": true, "Role": false, "RoleBinding": false, "ClusterRole": false, "ClusterRoleBinding": false}, + }, + { + name: "WebhookListFails", + breakKube: func(c *k8sfake.Clientset) { + c.PrependReactor("list", "validatingwebhookconfigurations", failingReactor("list webhooks")) + }, + wantErr: "list webhooks", + wantObjects: map[string]bool{"ServiceAccount": true, "Role": false, "RoleBinding": false, "ClusterRole": false, "ClusterRoleBinding": false}, + }, + { + name: "InstallPlanUpdateStatusFails/NothingDeleted", + breakOperators: func(c *fake.ReactionForwardingClientsetDecorator) { + c.PrependReactor("update", "installplans", failingReactor("update installplans")) + }, + wantErr: "update installplans", + wantObjects: map[string]bool{"ServiceAccount": true, "Role": true, "RoleBinding": true, "ClusterRole": true, "ClusterRoleBinding": true}, + wantSteps: map[string]v1alpha1.StepStatus{"Role": v1alpha1.StepStatusPresent}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + csv := failedUnsupportedCSV() + op := newCleanupOperator(t, ctx, csv, + []runtime.Object{cleanupOperatorGroup(false), completedInstallPlan()}, + cleanupK8sObjs(csv), nil) + if tt.breakOperators != nil { + tt.breakOperators(operatorsFake(op)) + } + if tt.breakKube != nil { + tt.breakKube(kubeFake(op)) + } + + require.EqualError(t, op.syncClusterServiceVersion(csv), tt.wantErr) + + require.Equal(t, v1alpha1.CSVReasonUnsupportedOperatorGroup, getCSV(t, ctx, op).Status.Reason) + if !tt.skipDeployment { + require.Equal(t, tt.wantDeployment, deploymentExists(t, op), "deployment exists") + } + for kind, want := range tt.wantObjects { + require.Equal(t, want, stepObjectExists(t, op, kind), "%s exists", kind) + } + if tt.wantSteps != nil { + plan, err := op.client.OperatorsV1alpha1().InstallPlans(cleanupNamespace).Get(ctx, cleanupPlanName, metav1.GetOptions{}) + require.NoError(t, err) + statuses := stepStatuses(plan) + for kind, want := range tt.wantSteps { + require.Equal(t, want, statuses[kind], "step %s", kind) + } + } + }) + } +} + +// TestFailedCSVCleanupRequeueFailure: when the CSV cannot be requeued while +// its InstallPlan still runs, the cleanup logs and returns. An empty queue +// set makes RequeueAfter fail. +func TestFailedCSVCleanupRequeueFailure(t *testing.T) { + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + csv := failedUnsupportedCSV() + op := newCleanupOperator(t, ctx, csv, []runtime.Object{installPlanInPhase(v1alpha1.InstallPlanPhaseInstalling)}, nil, nil) + op.csvQueueSet = queueinformer.NewEmptyResourceQueueSet() + + logger, hook := logrustest.NewNullLogger() + require.NoError(t, op.cleanupFailedCSV(logrus.NewEntry(logger), csv)) + + messages := []string{} + for _, entry := range hook.AllEntries() { + messages = append(messages, entry.Message) + } + require.Contains(t, messages, "unable to requeue") +} + +// TestPendingCSVReopensCleanedUpPlan: a Pending CSV with unmet requirements +// whose InstallPlan records NotPresent steps gets its plan reopened. +func TestPendingCSVReopensCleanedUpPlan(t *testing.T) { + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + csv := cleanupCSV(v1alpha1.CSVPhasePending, v1alpha1.CSVReasonRequirementsUnknown) + op := newCleanupOperator(t, ctx, csv, []runtime.Object{cleanupOperatorGroup(false), cleanedUpInstallPlan()}, nil, nil) + + require.ErrorIs(t, op.syncClusterServiceVersion(csv), ErrRequirementsNotMet) + + plan := requirePlan(t, ctx, op, v1alpha1.InstallPlanPhaseInstalling, v1alpha1.StepStatusNotPresent) + require.Nil(t, plan.Status.StartTime) +} + +// TestPendingCSVLeavesIntactPlanAlone: a Pending CSV whose RBAC is missing +// but whose InstallPlan has no NotPresent steps (someone else removed the +// RBAC, for example an administrator who revoked a binding) does not get +// its plan reopened. OLM only recreates what it removed itself. +func TestPendingCSVLeavesIntactPlanAlone(t *testing.T) { + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + csv := cleanupCSV(v1alpha1.CSVPhasePending, v1alpha1.CSVReasonRequirementsUnknown) + op := newCleanupOperator(t, ctx, csv, []runtime.Object{cleanupOperatorGroup(false), completedInstallPlan()}, nil, nil) + operatorsFake(op).PrependReactor("update", "installplans", failingReactor("must not write the InstallPlan")) + + require.ErrorIs(t, op.syncClusterServiceVersion(csv), ErrRequirementsNotMet) + + requirePlan(t, ctx, op, v1alpha1.InstallPlanPhaseComplete, v1alpha1.StepStatusPresent) +} + +// TestFailedCSVRecoveryForEachReason: whatever reason the CSV failed with +// last, once it is back in Pending a cleaned-up plan is reopened. The reason +// can change while Failed, for example from UnsupportedOperatorGroup to +// NoOperatorGroup when the user replaces the OperatorGroup. +func TestFailedCSVRecoveryForEachReason(t *testing.T) { + tests := []struct { + name string + reason v1alpha1.ConditionReason + static bool + }{ + {"UnsupportedOperatorGroup", v1alpha1.CSVReasonUnsupportedOperatorGroup, false}, + {"InterOperatorGroupOwnerConflict", v1alpha1.CSVReasonInterOperatorGroupOwnerConflict, false}, + {"CannotModifyStaticOperatorGroupProvidedAPIs", v1alpha1.CSVReasonCannotModifyStaticOperatorGroupProvidedAPIs, true}, + {"NoOperatorGroup", v1alpha1.CSVReasonNoOperatorGroup, false}, + {"TooManyOperatorGroups", v1alpha1.CSVReasonTooManyOperatorGroups, false}, + {"NoTargetNamespaces", v1alpha1.CSVReasonNoTargetNamespaces, false}, + {"InvalidInstallModes", v1alpha1.CSVReasonInvalidInstallModes, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + csv := cleanupCSV(v1alpha1.CSVPhaseFailed, tt.reason) + op := newCleanupOperator(t, ctx, csv, + []runtime.Object{cleanupOperatorGroup(tt.static), cleanedUpInstallPlan()}, + nil, + buildFakeAPIIntersectionReconcilerThatReturns(NoAPIConflict)) + + require.ErrorIs(t, recover(t, ctx, op, csv), ErrRequirementsNotMet) + + requirePlan(t, ctx, op, v1alpha1.InstallPlanPhaseInstalling, v1alpha1.StepStatusNotPresent) + }) + } +} + +// TestFailedCSVRecoveryWithoutInstallPlan: a CSV that no InstallPlan created +// recovers as before, nothing to reinstall. +func TestFailedCSVRecoveryWithoutInstallPlan(t *testing.T) { + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + csv := cleanupCSV(v1alpha1.CSVPhaseFailed, v1alpha1.CSVReasonUnsupportedOperatorGroup) + op := newCleanupOperator(t, ctx, csv, []runtime.Object{cleanupOperatorGroup(false)}, nil, nil) + + require.ErrorIs(t, recover(t, ctx, op, csv), ErrRequirementsNotMet) + require.Equal(t, v1alpha1.CSVPhasePending, getCSV(t, ctx, op).Status.Phase) +} + +// TestPendingCSVReopenErrors: when the InstallPlan cannot be read or written +// the sync returns the error and the plan is left as it was, so the reopen +// is retried. +func TestPendingCSVReopenErrors(t *testing.T) { + tests := []struct { + name string + verb string + wantErr string + }{ + {"InstallPlanListFails", "list", "list installplans"}, + {"InstallPlanUpdateStatusFails", "update", "update installplans"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + csv := cleanupCSV(v1alpha1.CSVPhasePending, v1alpha1.CSVReasonRequirementsUnknown) + op := newCleanupOperator(t, ctx, csv, []runtime.Object{cleanupOperatorGroup(false), cleanedUpInstallPlan()}, nil, nil) + operatorsFake(op).PrependReactor(tt.verb, "installplans", failingReactor(tt.wantErr)) + + require.EqualError(t, op.syncClusterServiceVersion(csv), tt.wantErr) + + requirePlan(t, ctx, op, v1alpha1.InstallPlanPhaseComplete, v1alpha1.StepStatusNotPresent) + }) + } +} + +// TestCleanupCSVDeploymentsInvalidStrategy: a CSV whose install strategy +// cannot be parsed is skipped, nothing is deleted. +func TestCleanupCSVDeploymentsInvalidStrategy(t *testing.T) { + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + csv := failedUnsupportedCSV() + csv.Spec.InstallStrategy.StrategyName = "not-a-strategy" + op := newCleanupOperator(t, ctx, csv, []runtime.Object{cleanupOperatorGroup(false)}, cleanupK8sObjs(csv), nil) + + require.NoError(t, op.cleanupCSVDeployments(logrus.NewEntry(logrus.New()), csv)) + require.True(t, deploymentExists(t, op), "deployment exists") +} + +// otherStrategy is an install strategy the resolver can parse but that is +// not a deployment strategy. +type otherStrategy struct { + install.StrategyResolverInterface +} + +func (otherStrategy) GetStrategyName() string { return "other" } + +func (o otherStrategy) UnmarshalStrategy(v1alpha1.NamedInstallStrategy) (install.Strategy, error) { + return o, nil +} + +// TestCleanupCSVDeploymentsNonDeploymentStrategy: a parsed strategy that is +// not a deployment strategy is skipped, nothing is deleted. +func TestCleanupCSVDeploymentsNonDeploymentStrategy(t *testing.T) { + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + csv := failedUnsupportedCSV() + op := newCleanupOperator(t, ctx, csv, []runtime.Object{cleanupOperatorGroup(false)}, cleanupK8sObjs(csv), nil) + op.resolver = otherStrategy{StrategyResolverInterface: op.resolver} + + require.NoError(t, op.cleanupCSVDeployments(logrus.NewEntry(logrus.New()), csv)) + require.True(t, deploymentExists(t, op), "deployment exists") +} + +// TestDeleteStepObjectUnknownKind: a step of a kind the cleanup does not +// handle is an error, not a silent skip. +func TestDeleteStepObjectUnknownKind(t *testing.T) { + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + csv := failedUnsupportedCSV() + op := newCleanupOperator(t, ctx, csv, nil, nil, nil) + + step := &v1alpha1.Step{Resource: v1alpha1.StepResource{Kind: "Secret", Name: "s"}} + _, err := op.deleteStepObject(cleanupNamespace, step, "uid") + require.EqualError(t, err, "step kind Secret is not deleted on failure") + _, err = op.getStepObject(cleanupNamespace, step) + require.EqualError(t, err, "step kind Secret is not deleted on failure") +} + +// TestPendingCSVReopensPlanEvenWhenRequirementsAreMet: the reopen is a pure +// check of the marks. A CSV whose requirements happen to be met (here it +// declares no permissions) still gets its plan reopened and moves on. +func TestPendingCSVReopensPlanEvenWhenRequirementsAreMet(t *testing.T) { + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + csv := cleanupCSV(v1alpha1.CSVPhasePending, v1alpha1.CSVReasonRequirementsUnknown) + csv.Spec.InstallStrategy = installStrategy(cleanupDeployment, nil, nil) + op := newCleanupOperator(t, ctx, csv, []runtime.Object{cleanupOperatorGroup(false), cleanedUpInstallPlan()}, nil, nil) + + require.NoError(t, op.syncClusterServiceVersion(csv)) + + requirePlan(t, ctx, op, v1alpha1.InstallPlanPhaseInstalling, v1alpha1.StepStatusNotPresent) + require.Equal(t, v1alpha1.CSVPhaseInstallReady, getCSV(t, ctx, op).Status.Phase) +} + +// TestFailedCSVCleanupAlreadyCleanedPlan: a second sync of a Failed CSV +// whose plan already records the cleanup writes nothing. +func TestFailedCSVCleanupAlreadyCleanedPlan(t *testing.T) { + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + csv := failedUnsupportedCSV() + op := newCleanupOperator(t, ctx, csv, []runtime.Object{cleanupOperatorGroup(false), cleanedUpInstallPlan()}, nil, nil) + operatorsFake(op).PrependReactor("update", "installplans", failingReactor("must not write the InstallPlan")) + + require.NoError(t, op.syncClusterServiceVersion(csv)) + requirePlan(t, ctx, op, v1alpha1.InstallPlanPhaseComplete, v1alpha1.StepStatusNotPresent) +} + +// TestFailedCSVCleanupMissingObjects: objects that are already gone were +// removed by someone else, for example an administrator who revoked a +// binding. Their steps stay Present, so recovery does not bring them back. +func TestFailedCSVCleanupMissingObjects(t *testing.T) { + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + csv := failedUnsupportedCSV() + op := newCleanupOperator(t, ctx, csv, []runtime.Object{cleanupOperatorGroup(false), completedInstallPlan()}, []runtime.Object{serviceAccount(cleanupSA, cleanupNamespace)}, nil) + operatorsFake(op).PrependReactor("update", "installplans", failingReactor("must not write the InstallPlan")) + + require.NoError(t, op.syncClusterServiceVersion(csv)) + requirePlan(t, ctx, op, v1alpha1.InstallPlanPhaseComplete, v1alpha1.StepStatusPresent) +} + +// TestFailedCSVCleanupRecreatedObjects: InstallPlan execution is at least +// once. When a stale catalog-operator sync creates the objects again after +// the cleanup marked their steps, the next sync deletes them again without +// another status write. +func TestFailedCSVCleanupRecreatedObjects(t *testing.T) { + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + csv := failedUnsupportedCSV() + op := newCleanupOperator(t, ctx, csv, []runtime.Object{cleanupOperatorGroup(false), cleanedUpInstallPlan()}, cleanupK8sObjs(csv), nil) + operatorsFake(op).PrependReactor("update", "installplans", failingReactor("must not write the InstallPlan")) + + require.NoError(t, op.syncClusterServiceVersion(csv)) + requireStepObjects(t, op, false) + requirePlan(t, ctx, op, v1alpha1.InstallPlanPhaseComplete, v1alpha1.StepStatusNotPresent) +} + +// TestFailedCSVCleanupForeignStepObject: an object at a step name that +// another owner holds is kept and its step stays Present. +func TestFailedCSVCleanupForeignStepObject(t *testing.T) { + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + csv := failedUnsupportedCSV() + objs := cleanupK8sObjs(csv) + for _, o := range objs { + if role, ok := o.(*rbacv1.Role); ok { + role.Labels = map[string]string{ownerutil.OwnerKey: "other-csv", ownerutil.OwnerNamespaceKey: cleanupNamespace, ownerutil.OwnerKind: v1alpha1.ClusterServiceVersionKind} + } + } + op := newCleanupOperator(t, ctx, csv, []runtime.Object{cleanupOperatorGroup(false), completedInstallPlan()}, objs, nil) + + require.NoError(t, op.syncClusterServiceVersion(csv)) + + require.True(t, stepObjectExists(t, op, "Role"), "foreign Role must stay") + for _, kind := range []string{"RoleBinding", "ClusterRole", "ClusterRoleBinding"} { + require.False(t, stepObjectExists(t, op, kind), "%s exists", kind) + } + plan, err := op.client.OperatorsV1alpha1().InstallPlans(cleanupNamespace).Get(ctx, cleanupPlanName, metav1.GetOptions{}) + require.NoError(t, err) + statuses := stepStatuses(plan) + require.Equal(t, v1alpha1.StepStatusPresent, statuses["Role"]) + require.Equal(t, v1alpha1.StepStatusNotPresent, statuses["RoleBinding"]) +} + +// TestFailedCSVCleanupKeepsDeploymentOfAPIServer: a CSV that owns an +// APIService or a conversion webhook keeps its Deployment, the API would +// break without a backend. The RBAC is still cleaned up. +func TestFailedCSVCleanupKeepsDeploymentOfAPIServer(t *testing.T) { + tests := []struct { + name string + set func(*v1alpha1.ClusterServiceVersion) + }{ + {"OwnedAPIService", func(c *v1alpha1.ClusterServiceVersion) { + c.Spec.APIServiceDefinitions.Owned = []v1alpha1.APIServiceDescription{{Name: "v1.api.example.com", Group: "api.example.com", Version: "v1", Kind: "Thing", DeploymentName: cleanupDeployment}} + }}, + {"ConversionWebhook", func(c *v1alpha1.ClusterServiceVersion) { + c.Spec.WebhookDefinitions = []v1alpha1.WebhookDescription{{GenerateName: "conv", Type: v1alpha1.ConversionWebhook, DeploymentName: cleanupDeployment}} + }}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + csv := failedUnsupportedCSV() + tt.set(csv) + op := newCleanupOperator(t, ctx, csv, []runtime.Object{cleanupOperatorGroup(false), completedInstallPlan()}, cleanupK8sObjs(csv), nil) + + require.NoError(t, op.syncClusterServiceVersion(csv)) + + require.True(t, deploymentExists(t, op), "deployment of an API server must stay") + requireStepObjects(t, op, false) + }) + } +} + +// TestFailedCSVCleanupDeleteConflict: a delete that fails its UID +// precondition means the object was replaced in between. The error is +// returned so the next sync reads the replacement. +func TestFailedCSVCleanupDeleteConflict(t *testing.T) { + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + csv := failedUnsupportedCSV() + op := newCleanupOperator(t, ctx, csv, []runtime.Object{cleanupOperatorGroup(false), completedInstallPlan()}, cleanupK8sObjs(csv), nil) + kubeFake(op).PrependReactor("delete", "roles", func(clienttesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewConflict(rbacv1.Resource("roles"), cleanupRBACNames["Role"], errors.New("uid changed")) + }) + + require.Error(t, op.syncClusterServiceVersion(csv)) + require.True(t, stepObjectExists(t, op, "Role")) +} + +// TestFailedCSVCleanupForeignObjectClearsMark: a step marked NotPresent by +// an earlier cleanup whose name now belongs to another owner gets its mark +// removed, so the reopened plan does not overwrite the other owner's object. +func TestFailedCSVCleanupForeignObjectClearsMark(t *testing.T) { + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + csv := failedUnsupportedCSV() + foreign := &rbacv1.Role{ObjectMeta: metav1.ObjectMeta{Name: cleanupRBACNames["Role"], Namespace: cleanupNamespace, Labels: map[string]string{ownerutil.OwnerKey: "other-csv", ownerutil.OwnerNamespaceKey: cleanupNamespace}}} + op := newCleanupOperator(t, ctx, csv, []runtime.Object{cleanupOperatorGroup(false), cleanedUpInstallPlan()}, []runtime.Object{serviceAccount(cleanupSA, cleanupNamespace), foreign}, nil) + + require.NoError(t, op.syncClusterServiceVersion(csv)) + + require.True(t, stepObjectExists(t, op, "Role"), "foreign Role must stay") + plan, err := op.client.OperatorsV1alpha1().InstallPlans(cleanupNamespace).Get(ctx, cleanupPlanName, metav1.GetOptions{}) + require.NoError(t, err) + statuses := stepStatuses(plan) + require.Equal(t, v1alpha1.StepStatusPresent, statuses["Role"], "mark cleared for the foreign object") + require.Equal(t, v1alpha1.StepStatusNotPresent, statuses["RoleBinding"], "other marks kept") +} + +// TestFailedCSVReopensPlanFromFailedBranch: a Failed CSV whose reason is not +// an OperatorGroup reason can go to Succeeded without passing Pending. The +// cleaned-up plan is reopened from the Failed branch as well. +func TestFailedCSVReopensPlanFromFailedBranch(t *testing.T) { + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + csv := cleanupCSV(v1alpha1.CSVPhaseFailed, v1alpha1.CSVReasonComponentFailed) + op := newCleanupOperator(t, ctx, csv, []runtime.Object{cleanupOperatorGroup(false), cleanedUpInstallPlan()}, []runtime.Object{serviceAccount(cleanupSA, cleanupNamespace)}, nil) + + require.NoError(t, op.syncClusterServiceVersion(csv)) + + requirePlan(t, ctx, op, v1alpha1.InstallPlanPhaseInstalling, v1alpha1.StepStatusNotPresent) +} + +// TestFailedCSVCleanupKeepsDeploymentOfLiveAPIService: an APIService that +// carries the CSV owner labels keeps the Deployment even when the CSV spec +// no longer declares it. +func TestFailedCSVCleanupKeepsDeploymentOfLiveAPIService(t *testing.T) { + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + csv := failedUnsupportedCSV() + apiService := &apiregistrationv1.APIService{ObjectMeta: metav1.ObjectMeta{Name: "v1.api.example.com", Labels: ownerutil.OwnerLabel(csv, v1alpha1.ClusterServiceVersionKind)}} + op, err := NewFakeOperator(ctx, + withNamespaces(cleanupNamespace, "kube-system"), + withClientObjs(cleanupOperatorGroup(false), completedInstallPlan(), csv), + withK8sObjs(cleanupK8sObjs(csv)...), + withExtObjs(crd("c1", "v1", "g1")), + withRegObjs(apiService), + withPartialMetadata(&metav1.PartialObjectMetadata{ObjectMeta: csv.ObjectMeta}), + withOperatorNamespace(cleanupNamespace)) + require.NoError(t, err) + + require.NoError(t, op.syncClusterServiceVersion(csv)) + + require.True(t, deploymentExists(t, op), "deployment of a live API server must stay") + requireStepObjects(t, op, false) +} + +// TestFailedCSVCleanupManifestForms: an inline manifest in any form can be +// applied again, an empty manifest cannot. +func TestFailedCSVCleanupManifestForms(t *testing.T) { + tests := []struct { + name string + manifest string + wantDeleted bool + }{ + {"Empty", "", false}, + {"NotJSON", "apiVersion: rbac.authorization.k8s.io/v1", true}, + {"JSONNotAReference", `{"kind":"ConfigMap","name":"x"}`, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + plan := completedInstallPlan() + plan.Status.Plan[3].Resource.Manifest = tt.manifest + csv := failedUnsupportedCSV() + op := newCleanupOperator(t, ctx, csv, []runtime.Object{cleanupOperatorGroup(false), plan}, cleanupK8sObjs(csv), nil) + + require.NoError(t, op.syncClusterServiceVersion(csv)) + requireStepObjects(t, op, !tt.wantDeleted) + }) + } +} + +// TestFailedCSVCleanupConfigMapError: an error other than NotFound while +// checking the unpacked bundle ConfigMap is returned, nothing is deleted. +func TestFailedCSVCleanupConfigMapError(t *testing.T) { + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + plan := completedInstallPlan() + for _, step := range plan.Status.Plan { + step.Resource.Manifest = `{"kind":"ConfigMap","name":"bundle-cm","namespace":"ns","catalogSourceName":"cat","catalogSourceNamespace":"ns"}` + } + csv := failedUnsupportedCSV() + op := newCleanupOperator(t, ctx, csv, []runtime.Object{cleanupOperatorGroup(false), plan}, cleanupK8sObjs(csv), nil) + kubeFake(op).PrependReactor("get", "configmaps", failingReactor("get configmap")) + + require.EqualError(t, op.syncClusterServiceVersion(csv), "get configmap") + requireStepObjects(t, op, true) +} + +// TestFailedCSVCleanupWebhookErrors: webhook API errors are returned, the +// rest of the cleanup still happens. +func TestFailedCSVCleanupWebhookErrors(t *testing.T) { + tests := []struct { + name string + verb string + resource string + }{ + {"ValidatingDeleteFails", "delete", "validatingwebhookconfigurations"}, + {"MutatingListFails", "list", "mutatingwebhookconfigurations"}, + {"MutatingDeleteFails", "delete", "mutatingwebhookconfigurations"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + csv := failedUnsupportedCSV() + labels := ownerutil.OwnerLabel(csv, v1alpha1.ClusterServiceVersionKind) + webhooks := []runtime.Object{ + &admissionregistrationv1.ValidatingWebhookConfiguration{ObjectMeta: metav1.ObjectMeta{Name: "csv1-validating", Labels: labels}}, + &admissionregistrationv1.MutatingWebhookConfiguration{ObjectMeta: metav1.ObjectMeta{Name: "csv1-mutating", Labels: labels}}, + } + op := newCleanupOperator(t, ctx, csv, []runtime.Object{cleanupOperatorGroup(false), completedInstallPlan()}, append(cleanupK8sObjs(csv), webhooks...), nil) + kubeFake(op).PrependReactor(tt.verb, tt.resource, failingReactor(tt.name)) + + require.EqualError(t, op.syncClusterServiceVersion(csv), tt.name) + require.False(t, deploymentExists(t, op), "deployment exists") + requireStepObjects(t, op, false) + }) + } +} + +// TestFailedCSVCleanupObjectGoneBeforeDelete: an object that someone else +// deleted between the read and the delete was their decision. The mark is +// taken back so the reopened plan does not create it again. The other +// steps keep their marks. +func TestFailedCSVCleanupObjectGoneBeforeDelete(t *testing.T) { + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + csv := failedUnsupportedCSV() + op := newCleanupOperator(t, ctx, csv, []runtime.Object{cleanupOperatorGroup(false), completedInstallPlan()}, cleanupK8sObjs(csv), nil) + kubeFake(op).PrependReactor("delete", "roles", func(clienttesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewNotFound(rbacv1.Resource("roles"), cleanupRBACNames["Role"]) + }) + + require.NoError(t, op.syncClusterServiceVersion(csv)) + + plan, err := op.client.OperatorsV1alpha1().InstallPlans(cleanupNamespace).Get(ctx, cleanupPlanName, metav1.GetOptions{}) + require.NoError(t, err) + statuses := stepStatuses(plan) + require.Equal(t, v1alpha1.StepStatusPresent, statuses["Role"], "mark taken back") + require.Equal(t, v1alpha1.StepStatusNotPresent, statuses["RoleBinding"], "other marks kept") +} + +// TestFailedCSVCleanupObjectGoneBeforeDeleteStatusWriteFails: taking a +// mark back is a status write too, its failure is returned for retry. +func TestFailedCSVCleanupObjectGoneBeforeDeleteStatusWriteFails(t *testing.T) { + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + csv := failedUnsupportedCSV() + op := newCleanupOperator(t, ctx, csv, []runtime.Object{cleanupOperatorGroup(false), completedInstallPlan()}, cleanupK8sObjs(csv), nil) + kubeFake(op).PrependReactor("delete", "roles", func(clienttesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewNotFound(rbacv1.Resource("roles"), cleanupRBACNames["Role"]) + }) + updates := 0 + operatorsFake(op).PrependReactor("update", "installplans", func(clienttesting.Action) (bool, runtime.Object, error) { + updates++ + if updates == 2 { + return true, nil, errors.New("second status write fails") + } + return false, nil, nil + }) + + require.EqualError(t, op.syncClusterServiceVersion(csv), "second status write fails") +} diff --git a/pkg/controller/operators/olm/operator.go b/pkg/controller/operators/olm/operator.go index 7507b97aaf..fea06c2c5e 100644 --- a/pkg/controller/operators/olm/operator.go +++ b/pkg/controller/operators/olm/operator.go @@ -1036,8 +1036,9 @@ func (a *Operator) syncObject(obj interface{}) (syncError error) { // Objects that can't have ownerrefs (cluster -> namespace, cross-namespace) // are handled by finalizer - // Requeue all owner CSVs - if ownerutil.IsOwnedByKind(metaObj, v1alpha1.ClusterServiceVersionKind) { + // Requeue all owner CSVs. Cluster-scoped objects carry owner labels + // instead of owner references. + if _, _, labeled := ownerutil.GetOwnerByKindLabel(metaObj, v1alpha1.ClusterServiceVersionKind); labeled || ownerutil.IsOwnedByKind(metaObj, v1alpha1.ClusterServiceVersionKind) { logger.Debug("requeueing owner csvs") a.requeueOwnerCSVs(metaObj) } @@ -2140,6 +2141,7 @@ func (a *Operator) transitionCSVState(in v1alpha1.ClusterServiceVersion) (out *v if err := modeSet.Supports(out.GetNamespace(), namespaces); err != nil { logger.WithField("reason", err.Error()).Info("installmodeset does not support operatorgroups namespace selection") out.SetPhaseWithEventIfChanged(v1alpha1.CSVPhaseFailed, v1alpha1.CSVReasonUnsupportedOperatorGroup, err.Error(), now, a.recorder) + syncError = a.cleanupFailedCSV(logger, out) return } } else { @@ -2171,16 +2173,16 @@ func (a *Operator) transitionCSVState(in v1alpha1.ClusterServiceVersion) (out *v if out.Status.Reason != v1alpha1.CSVReasonInterOperatorGroupOwnerConflict { logger.WithField("apis", providedAPIs).Warn("cannot modify provided apis of static provided api operatorgroup") out.SetPhaseWithEvent(v1alpha1.CSVPhaseFailed, v1alpha1.CSVReasonCannotModifyStaticOperatorGroupProvidedAPIs, "static provided api operatorgroup cannot be modified by these apis", now, a.recorder) - a.cleanupCSVDeployments(logger, out) } + syncError = a.cleanupFailedCSV(logger, out) return case result == APIConflict: // Transition the CSV to FAILED with status reason "InterOperatorGroupOwnerConflict" if out.Status.Reason != v1alpha1.CSVReasonInterOperatorGroupOwnerConflict { logger.WithField("apis", providedAPIs).Warn("intersecting operatorgroups provide the same apis") out.SetPhaseWithEvent(v1alpha1.CSVPhaseFailed, v1alpha1.CSVReasonInterOperatorGroupOwnerConflict, "intersecting operatorgroups provide the same apis", now, a.recorder) - a.cleanupCSVDeployments(logger, out) } + syncError = a.cleanupFailedCSV(logger, out) return case result == AddAPIs: // Add the CSV's provided APIs to its OperatorGroup's annotation @@ -2238,6 +2240,12 @@ func (a *Operator) transitionCSVState(in v1alpha1.ClusterServiceVersion) (out *v return } } + // The RBAC that cleanupFailedCSV removed while the CSV was Failed is + // created again by the InstallPlan. A plan without marks is left alone. + if syncError = a.reinstallPlanSteps(logger, out); syncError != nil { + return + } + met, statuses, err := a.requirementAndPermissionStatus(out) if err != nil { // TODO: account for Bad Rule as well @@ -2475,6 +2483,12 @@ func (a *Operator) transitionCSVState(in v1alpha1.ClusterServiceVersion) (out *v return } + // A Failed CSV can go to Succeeded from here without passing Pending, + // so a cleaned-up plan is reopened here as well. + if syncError = a.reinstallPlanSteps(logger, out); syncError != nil { + return + } + // Check if requirements exist met, statuses, err := a.requirementAndPermissionStatus(out) if err != nil && out.Status.Reason != v1alpha1.CSVReasonInvalidStrategy { @@ -2940,29 +2954,77 @@ func (a *Operator) requeueOwnerCSVs(ownee metav1.Object) { } } -func (a *Operator) cleanupCSVDeployments(logger *logrus.Entry, csv *v1alpha1.ClusterServiceVersion) { +// cleanupCSVDeployments deletes the Deployments of the CSV install strategy +// that the CSV owns. A Deployment of the same name owned by another CSV is +// kept. A missing Deployment is not an error. +func (a *Operator) cleanupCSVDeployments(logger *logrus.Entry, csv *v1alpha1.ClusterServiceVersion) error { // Extract the InstallStrategy for the deployment strategy, err := a.resolver.UnmarshalStrategy(csv.Spec.InstallStrategy) if err != nil { logger.Warn("could not parse install strategy while cleaning up CSV deployment") - return + return nil } // Assume the strategy is for a deployment strategyDetailsDeployment, ok := strategy.(*v1alpha1.StrategyDetailsDeployment) if !ok { logger.Warnf("could not cast install strategy as type %T", strategyDetailsDeployment) - return + return nil + } + + // An owned APIService or a CRD conversion webhook keeps pointing at the + // Deployment. Without a backend every call to those APIs fails, which is + // worse than a running operator with a Failed CSV. + if a.servesAPIs(csv) { + logger.Debug("CSV serves APIServices or conversion webhooks, keeping its deployments") + return nil } // Delete deployments + var errs []error for _, spec := range strategyDetailsDeployment.DeploymentSpecs { logger := logger.WithField("deployment", spec.Name) + deployment, err := a.opClient.GetDeployment(csv.GetNamespace(), spec.Name) + if apierrors.IsNotFound(err) { + continue + } + if err != nil { + errs = append(errs, err) + continue + } + if !ownerutil.IsOwnedBy(deployment, csv) { + logger.Debug("deployment is not owned by this CSV, keeping it") + continue + } logger.Debug("cleaning up CSV deployment") - if err := a.opClient.DeleteDeployment(csv.GetNamespace(), spec.Name, &metav1.DeleteOptions{}); err != nil { + uid := deployment.GetUID() + options := &metav1.DeleteOptions{Preconditions: &metav1.Preconditions{UID: &uid}} + if err := a.opClient.DeleteDeployment(csv.GetNamespace(), spec.Name, options); err != nil && !apierrors.IsNotFound(err) { logger.WithField("err", err).Warn("error cleaning up CSV deployment") + errs = append(errs, err) } } + return utilerrors.NewAggregate(errs) +} + +// servesAPIs reports whether the CSV owns an APIService or a CRD conversion +// webhook, both of which route API requests to its Deployment. The live +// APIServices count as well as the spec, the spec can be edited after the +// install. +func (a *Operator) servesAPIs(csv *v1alpha1.ClusterServiceVersion) bool { + if len(csv.Spec.APIServiceDefinitions.Owned) > 0 { + return true + } + for _, webhook := range csv.Spec.WebhookDefinitions { + if webhook.Type == v1alpha1.ConversionWebhook { + return true + } + } + apiServices, err := a.lister.APIRegistrationV1().APIServiceLister().List(ownerutil.CSVOwnerSelector(csv)) + if err != nil { + return true + } + return len(apiServices) > 0 } // ensureLabels merges a label set with a CSV's labels and attempts to update the CSV if the merged set differs from the CSV's original labels. diff --git a/test/e2e/operator_groups_e2e_test.go b/test/e2e/operator_groups_e2e_test.go index c848bcb808..72492739f6 100644 --- a/test/e2e/operator_groups_e2e_test.go +++ b/test/e2e/operator_groups_e2e_test.go @@ -854,6 +854,189 @@ var _ = Describe("Operator Group", Label("OperatorGroup"), func() { csvA, err = fetchCSV(crc, nsA, csvA.GetName(), csvSucceededChecker) require.NoError(GinkgoT(), err) }) + It("cleans up and reinstalls the InstallPlan RBAC of a CSV that fails on its OperatorGroup", func() { + By(`Create a catalog with an operator that supports only AllNamespaces and needs RBAC`) + By(`Update the OperatorGroup to target only its own namespace`) + By(`Subscribe and wait for the CSV to fail with reason "UnsupportedOperatorGroup"`) + By(`Wait for the InstallPlan to complete`) + By(`Ensure the Roles and ClusterRoles the InstallPlan created are removed`) + By(`Update the OperatorGroup to select all namespaces`) + By(`Ensure the InstallPlan creates the RBAC again and the CSV succeeds`) + By(`Break the OperatorGroup of the running operator, the Deployment and the plan RBAC go away`) + By(`Fix the OperatorGroup again, the operator comes back`) + + ns := generatedNamespace.GetName() + pkg := genName("pkg-") + csvName := pkg + "-stable" + stableChannel := "stable" + saName := genName("sa-") + + permissions := []v1alpha1.StrategyDeploymentPermissions{{ + ServiceAccountName: saName, + Rules: []rbacv1.PolicyRule{{ + Verbs: []string{"get", "list"}, + APIGroups: []string{""}, + Resources: []string{"configmaps"}, + }}, + }} + clusterPermissions := []v1alpha1.StrategyDeploymentPermissions{{ + ServiceAccountName: saName, + Rules: []rbacv1.PolicyRule{{ + Verbs: []string{"get", "list"}, + APIGroups: []string{""}, + Resources: []string{"namespaces"}, + }}, + }} + strategy := newNginxInstallStrategy(csvName, permissions, clusterPermissions) + crd := newCRD(genName(pkg)) + csv := newCSV(csvName, ns, "", semver.MustParse("0.1.0"), []apiextensionsv1.CustomResourceDefinition{crd}, nil, &strategy) + csv.Spec.InstallModes = []v1alpha1.InstallMode{ + {Type: v1alpha1.InstallModeTypeOwnNamespace, Supported: false}, + {Type: v1alpha1.InstallModeTypeSingleNamespace, Supported: false}, + {Type: v1alpha1.InstallModeTypeMultiNamespace, Supported: false}, + {Type: v1alpha1.InstallModeTypeAllNamespaces, Supported: true}, + } + + By(`Create a catalog with an operator that supports only AllNamespaces and needs RBAC`) + manifests := []registry.PackageManifest{{ + PackageName: pkg, + Channels: []registry.PackageChannel{{Name: stableChannel, CurrentCSVName: csvName}}, + DefaultChannelName: stableChannel, + }} + catalog := genName("catalog-") + _, cleanupCatalogSource := createInternalCatalogSource(c, crc, catalog, ns, manifests, []apiextensionsv1.CustomResourceDefinition{crd}, []v1alpha1.ClusterServiceVersion{csv}) + defer cleanupCatalogSource() + _, err := fetchCatalogSourceOnStatus(crc, catalog, ns, catalogSourceRegistryPodSynced()) + require.NoError(GinkgoT(), err) + + // The generated namespace already has an OperatorGroup. A second one + // would stop the catalog-operator from resolving, so retarget that one. + groupName := fmt.Sprintf("%s-operatorgroup", ns) + setTargetNamespaces := func(targets []string) error { + return retry.RetryOnConflict(retry.DefaultBackoff, func() error { + current, err := crc.OperatorsV1().OperatorGroups(ns).Get(context.TODO(), groupName, metav1.GetOptions{}) + if err != nil { + return err + } + current.Spec.TargetNamespaces = targets + _, err = crc.OperatorsV1().OperatorGroups(ns).Update(context.TODO(), current, metav1.UpdateOptions{}) + return err + }) + } + + By(`Update the OperatorGroup to target only its own namespace`) + require.NoError(GinkgoT(), setTargetNamespaces([]string{ns})) + + By(`Subscribe and wait for the CSV to fail with reason "UnsupportedOperatorGroup"`) + subName := genName("sub-") + cleanupSub := createSubscriptionForCatalog(crc, ns, subName, catalog, pkg, stableChannel, csvName, v1alpha1.ApprovalAutomatic) + defer cleanupSub() + sub, err := fetchSubscription(crc, ns, subName, subscriptionHasInstallPlanChecker()) + require.NoError(GinkgoT(), err) + failedWithUnsupportedOperatorGroup := func(csv *v1alpha1.ClusterServiceVersion) bool { + return csvFailedChecker(csv) && csv.Status.Reason == v1alpha1.CSVReasonUnsupportedOperatorGroup + } + _, err = fetchCSV(crc, ns, csvName, failedWithUnsupportedOperatorGroup) + require.NoError(GinkgoT(), err) + + By(`Wait for the InstallPlan to complete`) + planName := sub.Status.InstallPlanRef.Name + _, err = fetchInstallPlan(GinkgoT(), crc, planName, ns, buildInstallPlanPhaseCheckFunc(v1alpha1.InstallPlanPhaseComplete)) + require.NoError(GinkgoT(), err) + + ownedRBAC := func() (map[string]int, error) { + selector := labels.SelectorFromSet(map[string]string{ + ownerutil.OwnerKey: csvName, + ownerutil.OwnerNamespaceKey: ns, + ownerutil.OwnerKind: v1alpha1.ClusterServiceVersionKind, + }) + options := metav1.ListOptions{LabelSelector: selector.String()} + counts := map[string]int{} + roles, err := c.KubernetesInterface().RbacV1().Roles(ns).List(context.TODO(), options) + if err != nil { + return nil, err + } + counts["Role"] = len(roles.Items) + roleBindings, err := c.KubernetesInterface().RbacV1().RoleBindings(ns).List(context.TODO(), options) + if err != nil { + return nil, err + } + counts["RoleBinding"] = len(roleBindings.Items) + clusterRoles, err := c.KubernetesInterface().RbacV1().ClusterRoles().List(context.TODO(), options) + if err != nil { + return nil, err + } + counts["ClusterRole"] = len(clusterRoles.Items) + clusterRoleBindings, err := c.KubernetesInterface().RbacV1().ClusterRoleBindings().List(context.TODO(), options) + if err != nil { + return nil, err + } + counts["ClusterRoleBinding"] = len(clusterRoleBindings.Items) + return counts, nil + } + none := map[string]int{"Role": 0, "RoleBinding": 0, "ClusterRole": 0, "ClusterRoleBinding": 0} + + By(`Ensure the Roles and ClusterRoles the InstallPlan created are removed`) + Eventually(ownedRBAC, pollDuration, pollInterval).Should(Equal(none)) + + By(`Update the OperatorGroup to select all namespaces`) + require.NoError(GinkgoT(), setTargetNamespaces(nil)) + + By(`Ensure the InstallPlan creates the RBAC again and the CSV succeeds`) + _, err = fetchCSV(crc, ns, csvName, csvSucceededChecker) + require.NoError(GinkgoT(), err) + plan, err := fetchInstallPlan(GinkgoT(), crc, planName, ns, buildInstallPlanPhaseCheckFunc(v1alpha1.InstallPlanPhaseComplete)) + require.NoError(GinkgoT(), err) + // A re-executed step reports Created, or Present when the + // catalog-operator synced the plan a second time. + for _, step := range plan.Status.Plan { + switch step.Resource.Kind { + case "Role", "RoleBinding", "ClusterRole", "ClusterRoleBinding": + require.Contains(GinkgoT(), []v1alpha1.StepStatus{v1alpha1.StepStatusCreated, v1alpha1.StepStatusPresent}, step.Status, "step %s/%s was not executed again", step.Resource.Kind, step.Resource.Name) + } + } + counts, err := ownedRBAC() + require.NoError(GinkgoT(), err) + for kind, count := range counts { + require.GreaterOrEqual(GinkgoT(), count, 1, "%s owned by the CSV", kind) + } + + By(`Break the OperatorGroup of the running operator, the Deployment and the plan RBAC go away`) + require.NoError(GinkgoT(), setTargetNamespaces([]string{ns})) + _, err = fetchCSV(crc, ns, csvName, failedWithUnsupportedOperatorGroup) + require.NoError(GinkgoT(), err) + Eventually(func() (bool, error) { + _, err := c.GetDeployment(ns, csvName) + if apierrors.IsNotFound(err) { + return true, nil + } + return false, err + }, pollDuration, pollInterval).Should(BeTrue(), "deployment of the failed operator must be deleted") + Eventually(func() (int, error) { + plan, err := crc.OperatorsV1alpha1().InstallPlans(ns).Get(context.TODO(), planName, metav1.GetOptions{}) + if err != nil { + return -1, err + } + notPresent := 0 + for _, step := range plan.Status.Plan { + switch step.Resource.Kind { + case "Role", "RoleBinding", "ClusterRole", "ClusterRoleBinding": + if step.Status == v1alpha1.StepStatusNotPresent { + notPresent++ + } + } + } + return notPresent, nil + }, pollDuration, pollInterval).Should(BeNumerically(">=", 4), "plan RBAC steps marked NotPresent") + + By(`Fix the OperatorGroup again, the operator comes back`) + require.NoError(GinkgoT(), setTargetNamespaces(nil)) + _, err = fetchCSV(crc, ns, csvName, csvSucceededChecker) + require.NoError(GinkgoT(), err) + _, err = fetchInstallPlan(GinkgoT(), crc, planName, ns, buildInstallPlanPhaseCheckFunc(v1alpha1.InstallPlanPhaseComplete)) + require.NoError(GinkgoT(), err) + }) + It("[FLAKE] intersection", func() { By(`Generate namespaceA`)