Add system constraint provider logic to resolver

pkg/controller/registry/resolver/hooks.go

@@ -0,0 +1,9 @@
+package resolver
+
+// stepResolverInitHook provides a way for the downstream
+// to modify the step resolver at creation time.
+// This is a bit of a hack to enable system constraints downstream
+// without affecting the upstream. We may want to clean this up when
+// either we have a more pluggable architecture; or system constraints
+// come to the upstream
+type stepResolverInitHook func(*OperatorStepResolver) error

pkg/controller/registry/resolver/resolver.go

@@ -26,9 +26,10 @@ type OperatorResolver interface {
 }
 
 type SatResolver struct {
-	cache cache.OperatorCacheProvider
-	log   logrus.FieldLogger
-	pc    *predicateConverter
+	cache                     cache.OperatorCacheProvider
+	log                       logrus.FieldLogger
+	pc                        *predicateConverter
+	systemConstraintsProvider solver.ConstraintProvider
 }
 
 func NewDefaultSatResolver(rcp cache.SourceProvider, catsrcLister v1alpha1listers.CatalogSourceLister, logger logrus.FieldLogger) *SatResolver {
@@ -177,6 +178,25 @@ func (r *SatResolver) SolveOperators(namespaces []string, csvs []*v1alpha1.Clust
 	return operators, nil
 }
 
+// newBundleInstallableFromEntry converts an entry into a bundle installable with
+// system constraints applied, if they are defined for the entry
+func (r *SatResolver) newBundleInstallableFromEntry(entry *cache.Entry) (*BundleInstallable, error) {
+	bundleInstalleble, err := NewBundleInstallableFromOperator(entry)
+	if err != nil {
+		return nil, err
+	}
+
+	// apply system constraints if necessary
+	if r.systemConstraintsProvider != nil && !(entry.SourceInfo.Catalog.Virtual()) {
+		systemConstraints, err := r.systemConstraintsProvider.Constraints(entry)
+		if err != nil {
+			return nil, err
+		}
+		bundleInstalleble.constraints = append(bundleInstalleble.constraints, systemConstraints...)
+	}
+	return &bundleInstalleble, nil
+}
+
 func (r *SatResolver) getSubscriptionInstallables(sub *v1alpha1.Subscription, current *cache.Entry, namespacedCache cache.MultiCatalogOperatorFinder, visited map[*cache.Entry]*BundleInstallable) (map[solver.Identifier]solver.Installable, error) {
 	var cachePredicates, channelPredicates []cache.Predicate
 	installables := make(map[solver.Identifier]solver.Installable, 0)
@@ -334,13 +354,13 @@ func (r *SatResolver) getBundleInstallables(preferredNamespace string, bundleSta
 			continue
 		}
 
-		bundleInstallable, err := NewBundleInstallableFromOperator(bundle)
+		bundleInstallable, err := r.newBundleInstallableFromEntry(bundle)
 		if err != nil {
 			errs = append(errs, err)
 			continue
 		}
 
-		visited[bundle] = &bundleInstallable
+		visited[bundle] = bundleInstallable
 
 		dependencyPredicates, err := r.pc.convertDependencyProperties(bundle.Properties)
 		if err != nil {
@@ -389,12 +409,12 @@ func (r *SatResolver) getBundleInstallables(preferredNamespace string, bundleSta
 			// (after sorting) to remove all bundles that
 			// don't satisfy the dependency.
 			for _, b := range cache.Filter(sortedBundles, d) {
-				i, err := NewBundleInstallableFromOperator(b)
+				i, err := r.newBundleInstallableFromEntry(b)
 				if err != nil {
 					errs = append(errs, err)
 					continue
 				}
-				installables[i.Identifier()] = &i
+				installables[i.Identifier()] = i
 				bundleDependencies = append(bundleDependencies, i.Identifier())
 				bundleStack = append(bundleStack, b)
 			}
@@ -404,7 +424,7 @@ func (r *SatResolver) getBundleInstallables(preferredNamespace string, bundleSta
 			))
 		}
 
-		installables[bundleInstallable.Identifier()] = &bundleInstallable
+		installables[bundleInstallable.Identifier()] = bundleInstallable
 	}
 
 	if len(errs) > 0 {

pkg/controller/registry/resolver/resolver_test.go

@@ -66,6 +66,109 @@ func TestSolveOperators(t *testing.T) {
 	require.EqualValues(t, expected, operators)
 }
 
+func TestSolveOperators_WithSystemConstraints(t *testing.T) {
+	const namespace = "test-namespace"
+	catalog := cache.SourceKey{Name: "test-catalog", Namespace: namespace}
+
+	packageASub := newSub(namespace, "packageA", "alpha", catalog)
+	packageDSub := existingSub(namespace, "packageD.v1", "packageD", "alpha", catalog)
+
+	APISet := cache.APISet{opregistry.APIKey{Group: "g", Version: "v", Kind: "k", Plural: "ks"}: struct{}{}}
+
+	// packageA requires an API that can be provided by B or C
+	packageA := genOperator("packageA.v1", "0.0.1", "", "packageA", "alpha", catalog.Name, catalog.Namespace, APISet, nil, nil, "", false)
+	packageB := genOperator("packageB.v1", "1.0.0", "", "packageB", "alpha", catalog.Name, catalog.Namespace, nil, APISet, nil, "", false)
+	packageC := genOperator("packageC.v1", "1.0.0", "", "packageC", "alpha", catalog.Name, catalog.Namespace, nil, APISet, nil, "", false)
+
+	// Existing operators
+	packageD := genOperator("packageD.v1", "1.0.0", "", "packageD", "alpha", catalog.Name, catalog.Namespace, nil, nil, nil, "", false)
+	existingPackageD := existingOperator(namespace, "packageD.v1", "packageD", "alpha", "", nil, nil, nil, nil)
+	existingPackageD.Annotations = map[string]string{"operatorframework.io/properties": `{"properties":[{"type":"olm.package","value":{"packageName":"packageD","version":"1.0.0"}}]}`}
+
+	whiteListConstraintProvider := func(whiteList ...*cache.Entry) solver.ConstraintProviderFunc {
+		return func(entry *cache.Entry) ([]solver.Constraint, error) {
+			for _, whiteListedEntry := range whiteList {
+				if whiteListedEntry.Package() == entry.Package() &&
+					whiteListedEntry.Name == entry.Name &&
+					whiteListedEntry.Version == entry.Version {
+					return nil, nil
+				}
+			}
+			return []solver.Constraint{PrettyConstraint(
+				solver.Prohibited(),
+				fmt.Sprintf("package: %s is not white listed", entry.Package()),
+			)}, nil
+		}
+	}
+
+	testCases := []struct {
+		title                     string
+		systemConstraintsProvider solver.ConstraintProvider
+		expectedOperators         cache.OperatorSet
+		csvs                      []*v1alpha1.ClusterServiceVersion
+		subs                      []*v1alpha1.Subscription
+		snapshotEntries           []*cache.Entry
+		err                       string
+	}{
+		{
+			title:                     "No runtime constraints",
+			snapshotEntries:           []*cache.Entry{packageA, packageB, packageC, packageD},
+			systemConstraintsProvider: nil,
+			expectedOperators:         cache.OperatorSet{"packageA.v1": packageA, "packageB.v1": packageB},
+			csvs:                      nil,
+			subs:                      []*v1alpha1.Subscription{packageASub},
+			err:                       "",
+		},
+		{
+			title:                     "Runtime constraints only accept packages A and C",
+			snapshotEntries:           []*cache.Entry{packageA, packageB, packageC, packageD},
+			systemConstraintsProvider: whiteListConstraintProvider(packageA, packageC),
+			expectedOperators:         cache.OperatorSet{"packageA.v1": packageA, "packageC.v1": packageC},
+			csvs:                      nil,
+			subs:                      []*v1alpha1.Subscription{packageASub},
+			err:                       "",
+		},
+		{
+			title:                     "Existing packages are ignored",
+			snapshotEntries:           []*cache.Entry{packageA, packageB, packageC, packageD},
+			systemConstraintsProvider: whiteListConstraintProvider(packageA, packageC),
+			expectedOperators:         cache.OperatorSet{"packageA.v1": packageA, "packageC.v1": packageC},
+			csvs:                      []*v1alpha1.ClusterServiceVersion{existingPackageD},
+			subs:                      []*v1alpha1.Subscription{packageASub, packageDSub},
+			err:                       "",
+		},
+		{
+			title:                     "Runtime constraints don't allow A",
+			snapshotEntries:           []*cache.Entry{packageA, packageB, packageC, packageD},
+			systemConstraintsProvider: whiteListConstraintProvider(),
+			expectedOperators:         nil,
+			csvs:                      nil,
+			subs:                      []*v1alpha1.Subscription{packageASub},
+			err:                       "packageA is not white listed",
+		},
+	}
+
+	for _, testCase := range testCases {
+		satResolver := SatResolver{
+			cache: cache.New(cache.StaticSourceProvider{
+				catalog: &cache.Snapshot{
+					Entries: testCase.snapshotEntries,
+				},
+			}),
+			log:                       logrus.New(),
+			systemConstraintsProvider: testCase.systemConstraintsProvider,
+		}
+		operators, err := satResolver.SolveOperators([]string{namespace}, testCase.csvs, testCase.subs)
+
+		if testCase.err != "" {
+			require.Containsf(t, err.Error(), testCase.err, "Test %s failed", testCase.title)
+		} else {
+			require.NoErrorf(t, err, "Test %s failed", testCase.title)
+		}
+		require.EqualValuesf(t, testCase.expectedOperators, operators, "Test %s failed", testCase.title)
+	}
+}
+
 func TestDisjointChannelGraph(t *testing.T) {
 	const namespace = "test-namespace"
 	catalog := cache.SourceKey{Name: "test-catalog", Namespace: namespace}

pkg/controller/registry/resolver/solver/constraint_provider.go

@@ -0,0 +1,19 @@
+package solver
+
+import "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/cache"
+
+// ConstraintProvider knows how to provide solver constraints for a given cache entry.
+// For instance, it could be used to surface additional constraints against an entry given some
+// properties it may expose. E.g. olm.maxOpenShiftVersion could be checked against the cluster version
+// and prohibit any entry that doesn't meet the requirement
+type ConstraintProvider interface {
+	// Constraints returns a set of solver constraints for a cache entry.
+	Constraints(e *cache.Entry) ([]Constraint, error)
+}
+
+// ConstraintProviderFunc is a simple implementation of ConstraintProvider
+type ConstraintProviderFunc func(e *cache.Entry) ([]Constraint, error)
+
+func (c ConstraintProviderFunc) Constraints(e *cache.Entry) ([]Constraint, error) {
+	return c(e)
+}

pkg/controller/registry/resolver/step_resolver.go

@@ -26,6 +26,9 @@ const (
 	BundleLookupConditionPacked v1alpha1.BundleLookupConditionType = "BundleLookupNotPersisted"
 )
 
+// init hooks provides the downstream a way to modify the upstream behavior
+var initHooks []stepResolverInitHook
+
 var timeNow = func() metav1.Time { return metav1.NewTime(time.Now().UTC()) }
 
 type StepResolver interface {
@@ -48,7 +51,7 @@ var _ StepResolver = &OperatorStepResolver{}
 
 func NewOperatorStepResolver(lister operatorlister.OperatorLister, client versioned.Interface, kubeclient kubernetes.Interface,
 	globalCatalogNamespace string, provider RegistryClientProvider, log logrus.FieldLogger) *OperatorStepResolver {
-	return &OperatorStepResolver{
+	stepResolver := &OperatorStepResolver{
 		subLister:              lister.OperatorsV1alpha1().SubscriptionLister(),
 		csvLister:              lister.OperatorsV1alpha1().ClusterServiceVersionLister(),
 		ipLister:               lister.OperatorsV1alpha1().InstallPlanLister(),
@@ -58,6 +61,15 @@ func NewOperatorStepResolver(lister operatorlister.OperatorLister, client versio
 		satResolver:            NewDefaultSatResolver(SourceProviderFromRegistryClientProvider(provider, log), lister.OperatorsV1alpha1().CatalogSourceLister(), log),
 		log:                    log,
 	}
+
+	// init hooks can be added to the downstream to
+	// modify resolver behaviour
+	for _, initHook := range initHooks {
+		if err := initHook(stepResolver); err != nil {
+			panic(err)
+		}
+	}
+	return stepResolver
 }
 
 func (r *OperatorStepResolver) Expire(key cache.SourceKey) {

pkg/controller/registry/resolver/step_resolver_test.go

@@ -47,6 +47,33 @@ var (
 	Requires4 = APISet4
 )
 
+func TestInitHooks(t *testing.T) {
+	clientFake := fake.NewSimpleClientset()
+	lister := operatorlister.NewLister()
+	kClientFake := k8sfake.NewSimpleClientset()
+	log := logrus.New()
+
+	// no init hooks
+	resolver := NewOperatorStepResolver(lister, clientFake, kClientFake, "", nil, log)
+	require.NotNil(t, resolver.satResolver)
+
+	// with init hook
+	var testHook stepResolverInitHook = func(resolver *OperatorStepResolver) error {
+		resolver.satResolver = nil
+		return nil
+	}
+
+	// defined in step_resolver.go
+	initHooks = append(initHooks, testHook)
+	defer func() {
+		// reset initHooks
+		initHooks = nil
+	}()
+
+	resolver = NewOperatorStepResolver(lister, clientFake, kClientFake, "", nil, log)
+	require.Nil(t, resolver.satResolver)
+}
+
 func TestResolver(t *testing.T) {
 	const namespace = "catsrc-namespace"
 	catalog := resolvercache.SourceKey{Name: "catsrc", Namespace: namespace}