Fix resolution error if inner entry doesn't provide a required API.

For bundle-to-bundle dependencies (as opposed to
subscription-to-bundle "dependencies"), candidates were filtered first
to only those that satisfy the constraint in question, then sorted in
channel order. Since the relative order of entries within a channel is
currently determined by replaces-distance from the channel head,
sorting would fail if an entry in the middle of a channel's replaces
chain did not satisfy the constraint.

For example, assume an operator "bar" has three available versions
within a single channel: bar-1, bar-2, and bar-3. If bar-1 and bar-3
provide the API "Bar", but bar-2 does not, then resolution will fail
for any operator that requires the API "Bar", because bar-1 and bar-3
are not connected by update edges without bar-2.

Now, all (package, channel, catalog) combinations satisfying a
constraint are enumerated and sorted before filtering to only those
entries that satisfy the constraint.

Signed-off-by: Ben Luddy <[email protected]>

Author: benluddy

pkg/controller/registry/resolver/cache.go

@@ -514,6 +514,12 @@ func WithLabel(label string) OperatorPredicate {
 	})
 }
 
+func WithCatalog(key registry.CatalogKey) OperatorPredicate {
+	return OperatorPredicateFunc(func(o *Operator) bool {
+		return key.Equal(o.SourceInfo().Catalog)
+	})
+}
+
 func ProvidingAPI(api opregistry.APIKey) OperatorPredicate {
 	return OperatorPredicateFunc(func(o *Operator) bool {
 		for _, p := range o.Properties() {
@@ -611,6 +617,12 @@ func True() OperatorPredicate {
 	})
 }
 
+func False() OperatorPredicate {
+	return OperatorPredicateFunc(func(*Operator) bool {
+		return false
+	})
+}
+
 func CountingPredicate(p OperatorPredicate, n *int) OperatorPredicate {
 	return OperatorPredicateFunc(func(o *Operator) bool {
 		if p.Test(o) {

pkg/controller/registry/resolver/resolver.go

@@ -339,15 +339,38 @@ func (r *SatResolver) getBundleInstallables(catalog registry.CatalogKey, predica
 		}
 
 		for _, d := range dependencyPredicates {
-			// errors ignored; this will build an empty/unsatisfiable dependency if no candidates are found
-			candidateBundles, _ := AtLeast(1, namespacedCache.FindPreferred(&bundle.sourceInfo.Catalog, d))
-			sortedBundles, err := r.sortBundles(candidateBundles)
+			sourcePredicate := False()
+			// Build a filter matching all (catalog,
+			// package, channel) combinations that contain
+			// at least one candidate bundle, even if only
+			// a subset of those bundles actually satisfy
+			// the dependency.
+			sources := map[OperatorSourceInfo]struct{}{}
+			for _, b := range namespacedCache.Find(d) {
+				si := b.SourceInfo()
+
+				if _, ok := sources[*si]; ok {
+					// Predicate already covers this source.
+					continue
+				}
+				sources[*si] = struct{}{}
+
+				sourcePredicate = Or(sourcePredicate, And(
+					WithPackage(si.Package),
+					WithChannel(si.Channel),
+					WithCatalog(si.Catalog),
+				))
+			}
+			sortedBundles, err := r.sortBundles(namespacedCache.FindPreferred(&bundle.sourceInfo.Catalog, sourcePredicate))
 			if err != nil {
 				errs = append(errs, err)
 				continue
 			}
 			bundleDependencies := make([]solver.Identifier, 0)
-			for _, b := range sortedBundles {
+			// The dependency predicate is applied here
+			// (after sorting) to remove all bundles that
+			// don't satisfy the dependency.
+			for _, b := range Filter(sortedBundles, d) {
 				src := b.SourceInfo()
 				if src == nil {
 					err := fmt.Errorf("unable to resolve the source of bundle %s, invalid cache", bundle.Identifier())
@@ -360,7 +383,6 @@ func (r *SatResolver) getBundleInstallables(catalog registry.CatalogKey, predica
 				bundleDependencies = append(bundleDependencies, i.Identifier())
 				bundleStack = append(bundleStack, b)
 			}
-
 			bundleInstallable.AddDependency(bundleDependencies)
 		}
 
@@ -469,6 +491,20 @@ func (r *SatResolver) newSnapshotForNamespace(namespace string, subs []*v1alpha1
 		op.sourceInfo = &OperatorSourceInfo{
 			Catalog: existingOperatorCatalog,
 		}
+		// Try to determine source package name from properties and add to SourceInfo.
+		for _, p := range op.properties {
+			if p.Type != opregistry.PackageType {
+				continue
+			}
+			var pp opregistry.PackageProperty
+			err := json.Unmarshal([]byte(p.Value), &pp)
+			if err != nil {
+				r.log.Warnf("failed to unmarshal package property of csv %q: %w", csv.Name, err)
+				continue
+			}
+			op.sourceInfo.Package = pp.PackageName
+		}
+
 		standaloneOperators = append(standaloneOperators, op)
 
 		// all standalone operators are mandatory

pkg/controller/registry/resolver/resolver_test.go

@@ -1319,6 +1319,8 @@ func genOperator(name, version, replaces, pkg, channel, catalogName, catalogName
 				Namespace: catalogNamespace,
 			},
 			DefaultChannel: defaultChannel != "" && channel == defaultChannel,
+			Package:        pkg,
+			Channel:        channel,
 		},
 		providedAPIs: providedAPIs,
 		requiredAPIs: requiredAPIs,

pkg/controller/registry/resolver/step_resolver_test.go

@@ -855,7 +855,7 @@ func TestResolver(t *testing.T) {
 			steps, lookups, subs, err := resolver.ResolveSteps(namespace, nil)
 			if tt.out.solverError == nil {
 				if tt.out.errAssert == nil {
-					assert.Nil(t, err)
+					assert.NoError(t, err)
 				} else {
 					tt.out.errAssert(t, err)
 				}

pkg/controller/registry/resolver/util_test.go

@@ -329,7 +329,7 @@ func bundle(name, pkg, channel, replaces string, providedCRDs, requiredCRDs, pro
 		RequiredApis: apiSetToGVK(requiredCRDs, requiredAPIServices),
 		Replaces:     replaces,
 		Dependencies: apiSetToDependencies(requiredCRDs, requiredAPIServices),
-		Properties:   append(apiSetToProperties(providedCRDs, providedAPIServices, false),
+		Properties: append(apiSetToProperties(providedCRDs, providedAPIServices, false),
 			packageNameToProperty(pkg, "0.0.0"),
 		),
 	}

test/e2e/subscription_e2e_test.go

@@ -55,6 +55,76 @@ var _ = Describe("Subscription", func() {
 		TearDown(testNamespace)
 	})
 
+	When("an entry in the middle of a channel does not provide a required GVK", func() {
+		var (
+			teardown func()
+		)
+
+		BeforeEach(func() {
+			teardown = func() {}
+
+			packages := []registry.PackageManifest{
+				{
+					PackageName: "dependency",
+					Channels: []registry.PackageChannel{
+						{Name: "channel-dependency", CurrentCSVName: "csv-dependency-3"},
+					},
+					DefaultChannelName: "channel-dependency",
+				},
+				{
+					PackageName: "root",
+					Channels: []registry.PackageChannel{
+						{Name: "channel-root", CurrentCSVName: "csv-root"},
+					},
+					DefaultChannelName: "channel-root",
+				},
+			}
+
+			crds := []apiextensions.CustomResourceDefinition{newCRD(genName("crd-"))}
+			csvs := []operatorsv1alpha1.ClusterServiceVersion{
+				newCSV("csv-dependency-1", testNamespace, "", semver.MustParse("1.0.0"), crds, nil, nil),
+				newCSV("csv-dependency-2", testNamespace, "csv-dependency-1", semver.MustParse("2.0.0"), nil, nil, nil),
+				newCSV("csv-dependency-3", testNamespace, "csv-dependency-2", semver.MustParse("3.0.0"), crds, nil, nil),
+				newCSV("csv-root", testNamespace, "", semver.MustParse("1.0.0"), nil, crds, nil),
+			}
+
+			_, teardown = createInternalCatalogSource(ctx.Ctx().KubeClient(), ctx.Ctx().OperatorClient(), "test-catalog", testNamespace, packages, crds, csvs)
+
+			createSubscriptionForCatalog(ctx.Ctx().OperatorClient(), testNamespace, "test-subscription", "test-catalog", "root", "channel-root", "", operatorsv1alpha1.ApprovalAutomatic)
+		})
+
+		AfterEach(func() {
+			teardown()
+		})
+
+		It("should create a Subscription for the latest entry providing the required GVK", func() {
+			Eventually(func() ([]operatorsv1alpha1.Subscription, error) {
+				var list operatorsv1alpha1.SubscriptionList
+				if err := ctx.Ctx().Client().List(context.TODO(), &list); err != nil {
+					return nil, err
+				}
+				return list.Items, nil
+			}).Should(ContainElement(WithTransform(
+				func(in operatorsv1alpha1.Subscription) operatorsv1alpha1.SubscriptionSpec {
+					return operatorsv1alpha1.SubscriptionSpec{
+						CatalogSource:          in.Spec.CatalogSource,
+						CatalogSourceNamespace: in.Spec.CatalogSourceNamespace,
+						Package:                in.Spec.Package,
+						Channel:                in.Spec.Channel,
+						StartingCSV:            in.Spec.StartingCSV,
+					}
+				},
+				Equal(operatorsv1alpha1.SubscriptionSpec{
+					CatalogSource:          "test-catalog",
+					CatalogSourceNamespace: testNamespace,
+					Package:                "dependency",
+					Channel:                "channel-dependency",
+					StartingCSV:            "csv-dependency-3",
+				}),
+			)))
+		})
+	})
+
 	//   I. Creating a new subscription
 	//      A. If package is not installed, creating a subscription should install latest version
 	It("creation if not installed", func() {