(bugfix): rewrite the contentmanager implementation

to fix bugs associated with insufficient permissions resulting
in halting reconciliation of all ClusterExtension and informer
sync errors not being reported via the ClusterExtension
status conditions.

Signed-off-by: everettraven <[email protected]>

Author: everettraven

api/v1alpha1/clusterextension_types.go

@@ -390,6 +390,7 @@ const (
 	// TODO(user): add more Types, here and into init()
 	TypeInstalled = "Installed"
 	TypeResolved  = "Resolved"
+	TypeHealthy   = "Healthy"
 
 	// TypeDeprecated is a rollup condition that is present when
 	// any of the deprecated conditions are present.
@@ -414,6 +415,8 @@ const (
 
 	ReasonErrorGettingReleaseState = "ErrorGettingReleaseState"
 
+	ReasonUnverifiable = "Unverifiable"
+
 	CRDUpgradeSafetyPolicyEnabled  CRDUpgradeSafetyPolicy = "Enabled"
 	CRDUpgradeSafetyPolicyDisabled CRDUpgradeSafetyPolicy = "Disabled"
 )
@@ -428,6 +431,7 @@ func init() {
 		TypeChannelDeprecated,
 		TypeBundleDeprecated,
 		TypeUnpacked,
+		TypeHealthy,
 	)
 	// TODO(user): add Reasons from above
 	conditionsets.ConditionReasons = append(conditionsets.ConditionReasons,
@@ -441,6 +445,7 @@ func init() {
 		ReasonUnpackSuccess,
 		ReasonUnpackFailed,
 		ReasonErrorGettingReleaseState,
+		ReasonUnverifiable,
 	)
 }
 

cmd/manager/main.go

@@ -249,14 +249,26 @@ func main() {
 		Preflights:         preflights,
 	}
 
+	cm := contentmanager.NewManager(clientRestConfigMapper, mgr.GetConfig(), mgr.GetRESTMapper())
+	contentManagerFinalizerKey := fmt.Sprintf("%s/contentmanager-cleanup", domain)
+	err = clusterExtensionFinalizers.Register(contentManagerFinalizerKey, finalizerFunc(func(ctx context.Context, obj client.Object) (crfinalizer.Result, error) {
+		ext := obj.(*ocv1alpha1.ClusterExtension)
+		err := cm.Delete(ext)
+		return crfinalizer.Result{}, err
+	}))
+	if err != nil {
+		setupLog.Error(err, "unable to register content manager cleanup finalizer")
+		os.Exit(1)
+	}
+
 	if err = (&controllers.ClusterExtensionReconciler{
 		Client:                cl,
 		Resolver:              resolver,
 		Unpacker:              unpacker,
 		Applier:               applier,
 		InstalledBundleGetter: &controllers.DefaultInstalledBundleGetter{ActionClientGetter: acg},
 		Finalizers:            clusterExtensionFinalizers,
-		Watcher:               contentmanager.New(clientRestConfigMapper, mgr.GetConfig(), mgr.GetRESTMapper()),
+		Manager:               cm,
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "ClusterExtension")
 		os.Exit(1)

config/samples/olm_v1alpha1_clusterextension.yaml

@@ -36,10 +36,10 @@ rules:
 # Manage ArgoCD CRDs
 - apiGroups: [apiextensions.k8s.io]
   resources: [customresourcedefinitions]
-  verbs: [create]
+  verbs: [create, list, watch]
 - apiGroups: [apiextensions.k8s.io]
   resources: [customresourcedefinitions]
-  verbs: [get, list, watch, update, patch, delete]
+  verbs: [get, update, patch, delete]
   resourceNames:
   - appprojects.argoproj.io
   - argocds.argoproj.io

internal/applier/helm.go

@@ -1,18 +1,23 @@
 package applier
 
 import (
+	"bytes"
 	"context"
 	"errors"
 	"fmt"
+	"io"
 	"io/fs"
 	"strings"
 
 	"helm.sh/helm/v3/pkg/action"
 	"helm.sh/helm/v3/pkg/chart"
 	"helm.sh/helm/v3/pkg/chartutil"
+	"helm.sh/helm/v3/pkg/postrender"
 	"helm.sh/helm/v3/pkg/release"
 	"helm.sh/helm/v3/pkg/storage/driver"
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	apimachyaml "k8s.io/apimachinery/pkg/util/yaml"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	helmclient "github.com/operator-framework/helm-operator-plugins/pkg/client"
@@ -51,7 +56,7 @@ type Helm struct {
 	Preflights         []Preflight
 }
 
-func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1alpha1.ClusterExtension, labels map[string]string) ([]client.Object, string, error) {
+func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1alpha1.ClusterExtension, objectLabels map[string]string, storageLabels map[string]string) ([]client.Object, string, error) {
 	chrt, err := convert.RegistryV1ToHelmChart(ctx, contentFS, ext.Spec.InstallNamespace, []string{corev1.NamespaceAll})
 	if err != nil {
 		return nil, "", err
@@ -63,7 +68,11 @@ func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1alpha1.Clust
 		return nil, "", err
 	}
 
-	rel, desiredRel, state, err := h.getReleaseState(ac, ext, chrt, values, labels)
+	post := &postrenderer{
+		labels: objectLabels,
+	}
+
+	rel, desiredRel, state, err := h.getReleaseState(ac, ext, chrt, values, post)
 	if err != nil {
 		return nil, "", err
 	}
@@ -94,18 +103,18 @@ func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1alpha1.Clust
 	case StateNeedsInstall:
 		rel, err = ac.Install(ext.GetName(), ext.Spec.InstallNamespace, chrt, values, func(install *action.Install) error {
 			install.CreateNamespace = false
-			install.Labels = labels
+			install.Labels = storageLabels
 			return nil
-		})
+		}, helmclient.AppendInstallPostRenderer(post))
 		if err != nil {
 			return nil, state, err
 		}
 	case StateNeedsUpgrade:
 		rel, err = ac.Upgrade(ext.GetName(), ext.Spec.InstallNamespace, chrt, values, func(upgrade *action.Upgrade) error {
 			upgrade.MaxHistory = maxHelmReleaseHistory
-			upgrade.Labels = labels
+			upgrade.Labels = storageLabels
 			return nil
-		})
+		}, helmclient.AppendUpgradePostRenderer(post))
 		if err != nil {
 			return nil, state, err
 		}
@@ -125,7 +134,7 @@ func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1alpha1.Clust
 	return relObjects, state, nil
 }
 
-func (h *Helm) getReleaseState(cl helmclient.ActionInterface, ext *ocv1alpha1.ClusterExtension, chrt *chart.Chart, values chartutil.Values, labels map[string]string) (*release.Release, *release.Release, string, error) {
+func (h *Helm) getReleaseState(cl helmclient.ActionInterface, ext *ocv1alpha1.ClusterExtension, chrt *chart.Chart, values chartutil.Values, post postrender.PostRenderer) (*release.Release, *release.Release, string, error) {
 	currentRelease, err := cl.Get(ext.GetName())
 	if err != nil && !errors.Is(err, driver.ErrReleaseNotFound) {
 		return nil, nil, StateError, err
@@ -138,9 +147,8 @@ func (h *Helm) getReleaseState(cl helmclient.ActionInterface, ext *ocv1alpha1.Cl
 		desiredRelease, err := cl.Install(ext.GetName(), ext.Spec.InstallNamespace, chrt, values, func(i *action.Install) error {
 			i.DryRun = true
 			i.DryRunOption = "server"
-			i.Labels = labels
 			return nil
-		})
+		}, helmclient.AppendInstallPostRenderer(post))
 		if err != nil {
 			return nil, nil, StateError, err
 		}
@@ -150,9 +158,8 @@ func (h *Helm) getReleaseState(cl helmclient.ActionInterface, ext *ocv1alpha1.Cl
 		upgrade.MaxHistory = maxHelmReleaseHistory
 		upgrade.DryRun = true
 		upgrade.DryRunOption = "server"
-		upgrade.Labels = labels
 		return nil
-	})
+	}, helmclient.AppendUpgradePostRenderer(post))
 	if err != nil {
 		return currentRelease, nil, StateError, err
 	}
@@ -164,3 +171,33 @@ func (h *Helm) getReleaseState(cl helmclient.ActionInterface, ext *ocv1alpha1.Cl
 	}
 	return currentRelease, desiredRelease, relState, nil
 }
+
+type postrenderer struct {
+	labels  map[string]string
+	cascade postrender.PostRenderer
+}
+
+func (p *postrenderer) Run(renderedManifests *bytes.Buffer) (*bytes.Buffer, error) {
+	var buf bytes.Buffer
+	dec := apimachyaml.NewYAMLOrJSONDecoder(renderedManifests, 1024)
+	for {
+		obj := unstructured.Unstructured{}
+		err := dec.Decode(&obj)
+		if errors.Is(err, io.EOF) {
+			break
+		}
+		if err != nil {
+			return nil, err
+		}
+		obj.SetLabels(util.MergeMaps(obj.GetLabels(), p.labels))
+		b, err := obj.MarshalJSON()
+		if err != nil {
+			return nil, err
+		}
+		buf.Write(b)
+	}
+	if p.cascade != nil {
+		return p.cascade.Run(&buf)
+	}
+	return &buf, nil
+}