Bug 1849164: don't store full manifests in installplan status (for bundle images)

pkg/controller/bundle/bundle_unpacker.go

@@ -34,6 +34,27 @@ func (b *BundleUnpackResult) Bundle() *api.Bundle {
 	return b.bundle
 }
 
+func (b *BundleUnpackResult) Name() string {
+	return b.name
+}
+
+// SetCondition replaces the existing BundleLookupCondition of the same type, or adds it if it was not found.
+func (b *BundleUnpackResult) SetCondition(cond operatorsv1alpha1.BundleLookupCondition) operatorsv1alpha1.BundleLookupCondition {
+	for i, existing := range b.Conditions {
+		if existing.Type != cond.Type {
+			continue
+		}
+		if existing.Status == cond.Status && existing.Reason == cond.Reason {
+			cond.LastTransitionTime = existing.LastTransitionTime
+		}
+		b.Conditions[i] = cond
+		return cond
+	}
+	b.Conditions = append(b.Conditions, cond)
+
+	return cond
+}
+
 var catalogSourceGVK = operatorsv1alpha1.SchemeGroupVersion.WithKind(operatorsv1alpha1.CatalogSourceKind)
 
 func newBundleUnpackResult(lookup *operatorsv1alpha1.BundleLookup) *BundleUnpackResult {
@@ -251,16 +272,26 @@ const (
 	CatalogSourceMissingMessage = "referenced catalogsource not found"
 	JobIncompleteReason         = "JobIncomplete"
 	JobIncompleteMessage        = "unpack job not completed"
+	JobNotStartedReason         = "JobNotStarted"
+	JobNotStartedMessage        = "unpack job not yet started"
+	NotUnpackedReason           = "BundleNotUnpacked"
+	NotUnpackedMessage          = "bundle contents have not yet been persisted to installplan status"
 )
 
 func (c *ConfigMapUnpacker) UnpackBundle(lookup *operatorsv1alpha1.BundleLookup) (result *BundleUnpackResult, err error) {
 	result = newBundleUnpackResult(lookup)
+
+	// if pending condition is missing, bundle has already been unpacked
 	cond := result.GetCondition(operatorsv1alpha1.BundleLookupPending)
+	if cond.Status == corev1.ConditionUnknown {
+		return result, nil
+	}
+
 	now := c.now()
 
 	var cs *operatorsv1alpha1.CatalogSource
 	if cs, err = c.csLister.CatalogSources(result.CatalogSourceRef.Namespace).Get(result.CatalogSourceRef.Name); err != nil {
-		if apierrors.IsNotFound(err) && cond.Status != corev1.ConditionTrue && cond.Reason != CatalogSourceMissingReason {
+		if apierrors.IsNotFound(err) && cond.Reason != CatalogSourceMissingReason {
 			cond.Status = corev1.ConditionTrue
 			cond.Reason = CatalogSourceMissingReason
 			cond.Message = CatalogSourceMissingMessage
@@ -304,7 +335,7 @@ func (c *ConfigMapUnpacker) UnpackBundle(lookup *operatorsv1alpha1.BundleLookup)
 		return
 	}
 
-	if !jobConditionTrue(job, batchv1.JobComplete) && cond.Status != corev1.ConditionTrue && cond.Reason != JobIncompleteReason {
+	if !jobConditionTrue(job, batchv1.JobComplete) && (cond.Status != corev1.ConditionTrue || cond.Reason != JobIncompleteReason) {
 		cond.Status = corev1.ConditionTrue
 		cond.Reason = JobIncompleteReason
 		cond.Message = JobIncompleteMessage
@@ -319,6 +350,10 @@ func (c *ConfigMapUnpacker) UnpackBundle(lookup *operatorsv1alpha1.BundleLookup)
 		return
 	}
 
+	if result.Bundle() == nil || len(result.Bundle().GetObject()) == 0 {
+		return
+	}
+
 	// A successful load should remove the pending condition
 	result.RemoveCondition(operatorsv1alpha1.BundleLookupPending)
 

pkg/controller/bundle/bundle_unpacker_test.go

@@ -71,6 +71,14 @@ func TestConfigMapUnpacker(t *testing.T) {
 						Namespace: "ns-a",
 						Name:      "src-a",
 					},
+					Conditions: []operatorsv1alpha1.BundleLookupCondition{
+						{
+							Type:    operatorsv1alpha1.BundleLookupPending,
+							Status:  corev1.ConditionTrue,
+							Reason:  JobNotStartedReason,
+							Message: JobNotStartedMessage,
+						},
+					},
 				},
 			},
 			expected: expected{
@@ -116,6 +124,14 @@ func TestConfigMapUnpacker(t *testing.T) {
 						Namespace: "ns-a",
 						Name:      "src-a",
 					},
+					Conditions: []operatorsv1alpha1.BundleLookupCondition{
+						{
+							Type:    operatorsv1alpha1.BundleLookupPending,
+							Status:  corev1.ConditionTrue,
+							Reason:  JobNotStartedReason,
+							Message: JobNotStartedMessage,
+						},
+					},
 				},
 			},
 			expected: expected{

pkg/controller/operators/catalog/manifests.go

@@ -0,0 +1,109 @@
+package catalog
+
+import (
+	"encoding/json"
+	"fmt"
+
+	"github.com/operator-framework/api/pkg/operators/v1alpha1"
+	"github.com/operator-framework/operator-registry/pkg/configmap"
+	errorwrap "github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
+	v1 "k8s.io/client-go/listers/core/v1"
+
+	"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver"
+)
+
+// ManifestResolver can dereference a manifest for a step. Steps may embed manifests directly or reference content
+// in configmaps
+type ManifestResolver interface {
+	ManifestForStep(step *v1alpha1.Step) (string, error)
+}
+
+// manifestResolver caches manifest from unpacked bundles (via configmaps)
+type manifestResolver struct {
+	configMapLister v1.ConfigMapLister
+	unpackedSteps   map[string][]v1alpha1.StepResource
+	namespace       string
+	logger          logrus.FieldLogger
+}
+
+func newManifestResolver(namespace string, configMapLister v1.ConfigMapLister, logger logrus.FieldLogger) *manifestResolver {
+	return &manifestResolver{
+		namespace:       namespace,
+		configMapLister: configMapLister,
+		unpackedSteps:   map[string][]v1alpha1.StepResource{},
+		logger:          logger,
+	}
+}
+
+// ManifestForStep always returns the manifest that should be applied to the cluster for a given step
+// the manifest field in the installplan status can contain a reference to a configmap instead
+func (r *manifestResolver) ManifestForStep(step *v1alpha1.Step) (string, error) {
+	manifest := step.Resource.Manifest
+	ref := refForStep(step, r.logger)
+	if ref == nil {
+		return manifest, nil
+	}
+
+	log := r.logger.WithFields(logrus.Fields{"resolving": step.Resolving, "step": step.Resource.Name})
+	log.WithField("ref", ref).Debug("step is a reference to configmap")
+
+	usteps, err := r.unpackedStepsForBundle(step.Resolving, ref)
+	if err != nil  {
+		return "", err
+	}
+
+	log.Debugf("checking cache for unpacked step")
+	// need to find the real manifest from the unpacked steps
+	for _, u := range usteps {
+		if u.Name == step.Resource.Name &&
+			u.Kind == step.Resource.Kind &&
+			u.Version == step.Resource.Version &&
+			u.Group == step.Resource.Group {
+			manifest = u.Manifest
+			log.WithField("manifest", manifest).Debug("step replaced with unpacked value")
+			break
+		}
+	}
+	if manifest == step.Resource.Manifest {
+		return "", fmt.Errorf("couldn't find unpacked step for %v", step)
+	}
+	return manifest, nil
+}
+
+func (r *manifestResolver) unpackedStepsForBundle(bundleName string, ref *UnpackedBundleReference) ([]v1alpha1.StepResource, error) {
+	usteps, ok := r.unpackedSteps[bundleName]
+	if ok {
+		return usteps, nil
+	}
+	cm, err := r.configMapLister.ConfigMaps(ref.Namespace).Get(ref.Name)
+	if err != nil {
+		return nil, errorwrap.Wrapf(err, "error finding unpacked bundle configmap for ref %v", *ref)
+	}
+	loader := configmap.NewBundleLoader()
+	bundle, err := loader.Load(cm)
+	if err != nil {
+		return nil, errorwrap.Wrapf(err, "error loading unpacked bundle configmap for ref %v", *ref)
+	}
+	steps, err := resolver.NewStepResourceFromBundle(bundle, r.namespace, ref.Replaces, ref.CatalogSourceName, ref.CatalogSourceNamespace)
+	if err != nil {
+		return nil, errorwrap.Wrapf(err, "error calculating steps for ref %v", *ref)
+	}
+	r.unpackedSteps[bundleName] = steps
+	return steps, nil
+}
+
+func refForStep(step *v1alpha1.Step, log logrus.FieldLogger) *UnpackedBundleReference {
+	log = log.WithFields(logrus.Fields{"resolving": step.Resolving, "step": step.Resource.Name})
+	var ref UnpackedBundleReference
+	if err := json.Unmarshal([]byte(step.Resource.Manifest), &ref); err != nil {
+		log.Debug("step is not a reference to an unpacked bundle (this is not an error if the step is a manifest)")
+		return nil
+	}
+	log = log.WithField("ref", ref)
+	if ref.Kind != "ConfigMap" || ref.Name == "" || ref.Namespace == "" || ref.CatalogSourceName == "" || ref.CatalogSourceNamespace == "" {
+		log.Debug("step is not a reference to an unpacked bundle (this is not an error if the step is a manifest)")
+		return nil
+	}
+	return &ref
+}