Remove provides+required GVK and modify/remove associated tests

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

internal/catalogmetadata/filter/bundle_predicates.go

@@ -52,22 +52,6 @@ func InChannel(channelName string) Predicate[catalogmetadata.Bundle] {
 	}
 }
 
-func ProvidesGVK(gvk *catalogmetadata.GVK) Predicate[catalogmetadata.Bundle] {
-	return func(bundle *catalogmetadata.Bundle) bool {
-		providedGVKs, err := bundle.ProvidedGVKs()
-		if err != nil {
-			return false
-		}
-		for i := 0; i < len(providedGVKs); i++ {
-			providedGVK := &providedGVKs[i]
-			if providedGVK.String() == gvk.String() {
-				return true
-			}
-		}
-		return false
-	}
-}
-
 func WithBundleImage(bundleImage string) Predicate[catalogmetadata.Bundle] {
 	return func(bundle *catalogmetadata.Bundle) bool {
 		return bundle.Image == bundleImage

internal/catalogmetadata/filter/bundle_predicates_test.go

@@ -114,34 +114,6 @@ func TestInChannel(t *testing.T) {
 	assert.False(t, f(b3))
 }
 
-func TestProvidesGVK(t *testing.T) {
-	b1 := &catalogmetadata.Bundle{Bundle: declcfg.Bundle{
-		Properties: []property.Property{
-			{
-				Type:  property.TypeGVK,
-				Value: json.RawMessage(`[{"group":"foo.io","kind":"Foo","version":"v1"},{"group":"bar.io","kind":"Bar","version":"v1"}]`),
-			},
-		},
-	}}
-	b2 := &catalogmetadata.Bundle{Bundle: declcfg.Bundle{}}
-	f1 := filter.ProvidesGVK(&catalogmetadata.GVK{
-		Group:   "foo.io",
-		Version: "v1",
-		Kind:    "Foo",
-	})
-	f2 := filter.ProvidesGVK(&catalogmetadata.GVK{
-		Group:   "baz.io",
-		Version: "v1alpha1",
-		Kind:    "Baz",
-	})
-	// Filter with Bundle which provides the GVK should return true
-	assert.True(t, f1(b1))
-	// Filter with Bundle which does not provide the GVK should return false
-	assert.False(t, f2(b1))
-	// Filter with Bundle with no GVK should return false
-	assert.False(t, f1(b2))
-}
-
 func TestWithBundleImage(t *testing.T) {
 	b1 := &catalogmetadata.Bundle{Bundle: declcfg.Bundle{Image: "fake-image-uri-1"}}
 	b2 := &catalogmetadata.Bundle{Bundle: declcfg.Bundle{Image: "fake-image-uri-2"}}

internal/catalogmetadata/types.go

@@ -11,6 +11,12 @@ import (
 	"github.com/operator-framework/operator-registry/alpha/property"
 )
 
+const (
+	MediaTypePlain          = "plain+v0"
+	MediaTypeRegistry       = "registry+v1"
+	PropertyBundleMediaType = "olm.bundle.mediatype"
+)
+
 type Schemas interface {
 	Package | Bundle | Channel
 }
@@ -39,18 +45,23 @@ func (g GVKRequired) AsGVK() GVK {
 	return GVK(g)
 }
 
+type PackageRequired struct {
+	property.PackageRequired
+	SemverRange *bsemver.Range `json:"-"`
+}
+
 type Bundle struct {
 	declcfg.Bundle
 	CatalogName string
 	InChannels  []*Channel
 
 	mu sync.RWMutex
 	// these properties are lazy loaded as they are requested
-	propertiesMap map[string]property.Property
-	bundlePackage *property.Package
-	semVersion    *bsemver.Version
-	providedGVKs  []GVK
-	requiredGVKs  []GVKRequired
+	propertiesMap    map[string]property.Property
+	bundlePackage    *property.Package
+	semVersion       *bsemver.Version
+	requiredPackages []PackageRequired
+	mediaType        string
 }
 
 func (b *Bundle) Version() (*bsemver.Version, error) {
@@ -60,18 +71,19 @@ func (b *Bundle) Version() (*bsemver.Version, error) {
 	return b.semVersion, nil
 }
 
-func (b *Bundle) ProvidedGVKs() ([]GVK, error) {
-	if err := b.loadProvidedGVKs(); err != nil {
+func (b *Bundle) RequiredPackages() ([]PackageRequired, error) {
+	if err := b.loadRequiredPackages(); err != nil {
 		return nil, err
 	}
-	return b.providedGVKs, nil
+	return b.requiredPackages, nil
 }
 
-func (b *Bundle) RequiredGVKs() ([]GVKRequired, error) {
-	if err := b.loadRequiredGVKs(); err != nil {
-		return nil, err
+func (b *Bundle) MediaType() (string, error) {
+	if err := b.loadMediaType(); err != nil {
+		return "", err
 	}
-	return b.requiredGVKs, nil
+
+	return b.mediaType, nil
 }
 
 func (b *Bundle) loadPackage() error {
@@ -94,28 +106,40 @@ func (b *Bundle) loadPackage() error {
 	return nil
 }
 
-func (b *Bundle) loadProvidedGVKs() error {
+func (b *Bundle) loadRequiredPackages() error {
 	b.mu.Lock()
 	defer b.mu.Unlock()
-	if b.providedGVKs == nil {
-		providedGVKs, err := loadFromProps[[]GVK](b, property.TypeGVK, false)
+	if b.requiredPackages == nil {
+		requiredPackages, err := loadFromProps[[]PackageRequired](b, property.TypePackageRequired, false)
 		if err != nil {
-			return fmt.Errorf("error determining provided GVKs for bundle %q: %s", b.Name, err)
+			return fmt.Errorf("error determining bundle required packages for bundle %q: %s", b.Name, err)
+		}
+		for _, requiredPackage := range requiredPackages {
+			semverRange, err := bsemver.ParseRange(requiredPackage.VersionRange)
+			if err != nil {
+				return fmt.Errorf(
+					"error parsing bundle required package semver range for bundle %q (required package %q): %s",
+					b.Name,
+					requiredPackage.PackageName,
+					err,
+				)
+			}
+			requiredPackage.SemverRange = &semverRange
 		}
-		b.providedGVKs = providedGVKs
+		b.requiredPackages = requiredPackages
 	}
 	return nil
 }
 
-func (b *Bundle) loadRequiredGVKs() error {
+func (b *Bundle) loadMediaType() error {
 	b.mu.Lock()
 	defer b.mu.Unlock()
-	if b.requiredGVKs == nil {
-		requiredGVKs, err := loadFromProps[[]GVKRequired](b, property.TypeGVKRequired, false)
+	if b.mediaType == "" {
+		mediaType, err := loadFromProps[string](b, PropertyBundleMediaType, false)
 		if err != nil {
-			return fmt.Errorf("error determining required GVKs for bundle %q: %s", b.Name, err)
+			return fmt.Errorf("error determining bundle mediatype for bundle %q: %s", b.Name, err)
 		}
-		b.requiredGVKs = requiredGVKs
+		b.mediaType = mediaType
 	}
 	return nil
 }

internal/catalogmetadata/types_test.go

@@ -71,82 +71,4 @@ func TestBundle(t *testing.T) {
 		assert.EqualError(t, err, "bundle property with type \"olm.package\" not found")
 		assert.Nil(t, ver)
 	})
-
-	t.Run("ProvidedGVKs", func(t *testing.T) {
-		validGVK := &catalogmetadata.Bundle{Bundle: declcfg.Bundle{
-			Name: "fake-bundle.v1",
-			Properties: []property.Property{
-				{
-					Type:  property.TypeGVK,
-					Value: json.RawMessage(`[{"group":"foo.io","kind":"Foo","version":"v1"},{"group":"bar.io","kind":"Bar","version":"v1alpha1"}]`),
-				},
-			},
-		}}
-		invalidGVK := &catalogmetadata.Bundle{Bundle: declcfg.Bundle{
-			Name: "fake-bundle.invalid",
-			Properties: []property.Property{
-				{
-					Type:  property.TypeGVK,
-					Value: json.RawMessage(`badGvkStructure`),
-				},
-			},
-		}}
-		noGVK := &catalogmetadata.Bundle{Bundle: declcfg.Bundle{
-			Name: "fake-bundle.noGVK",
-		}}
-
-		gvk, err := validGVK.ProvidedGVKs()
-		assert.NoError(t, err)
-		assert.Equal(t, []catalogmetadata.GVK{
-			{Group: "foo.io", Kind: "Foo", Version: "v1"},
-			{Group: "bar.io", Kind: "Bar", Version: "v1alpha1"},
-		}, gvk)
-
-		gvk, err = invalidGVK.ProvidedGVKs()
-		assert.EqualError(t, err, "error determining provided GVKs for bundle \"fake-bundle.invalid\": property \"olm.gvk\" with value \"badGvkStructure\" could not be parsed: invalid character 'b' looking for beginning of value")
-		assert.Nil(t, gvk)
-
-		gvk, err = noGVK.ProvidedGVKs()
-		assert.NoError(t, err)
-		assert.Nil(t, gvk)
-	})
-
-	t.Run("RequiredGVKs", func(t *testing.T) {
-		validGVK := &catalogmetadata.Bundle{Bundle: declcfg.Bundle{
-			Name: "fake-bundle.v1",
-			Properties: []property.Property{
-				{
-					Type:  property.TypeGVKRequired,
-					Value: json.RawMessage(`[{"group":"foo.io","kind":"Foo","version":"v1"},{"group":"bar.io","kind":"Bar","version":"v1alpha1"}]`),
-				},
-			},
-		}}
-		invalidGVK := &catalogmetadata.Bundle{Bundle: declcfg.Bundle{
-			Name: "fake-bundle.invalid",
-			Properties: []property.Property{
-				{
-					Type:  property.TypeGVKRequired,
-					Value: json.RawMessage(`badGvkStructure`),
-				},
-			},
-		}}
-		noGVK := &catalogmetadata.Bundle{Bundle: declcfg.Bundle{
-			Name: "fake-bundle.noGVK",
-		}}
-
-		gvk, err := validGVK.RequiredGVKs()
-		assert.NoError(t, err)
-		assert.Equal(t, []catalogmetadata.GVKRequired{
-			{Group: "foo.io", Kind: "Foo", Version: "v1"},
-			{Group: "bar.io", Kind: "Bar", Version: "v1alpha1"},
-		}, gvk)
-
-		gvk, err = invalidGVK.RequiredGVKs()
-		assert.EqualError(t, err, "error determining required GVKs for bundle \"fake-bundle.invalid\": property \"olm.gvk.required\" with value \"badGvkStructure\" could not be parsed: invalid character 'b' looking for beginning of value")
-		assert.Nil(t, gvk)
-
-		gvk, err = noGVK.RequiredGVKs()
-		assert.NoError(t, err)
-		assert.Nil(t, gvk)
-	})
 }

internal/resolution/variablesources/bundles_and_dependencies.go

@@ -107,27 +107,6 @@ func (b *BundlesAndDepsVariableSource) getEntityDependencies(ctx context.Context
 		}
 	}
 
-	// gather required gvk dependencies
-	// todo(perdasilva): disambiguate between not found and actual errors
-	gvkDependencies, _ := bundleEntity.RequiredGVKs()
-	for i := 0; i < len(gvkDependencies); i++ {
-		providedGvk := gvkDependencies[i].AsGVK()
-		gvkDependencyBundles, err := entitySource.Filter(ctx, predicates.ProvidesGVK(&providedGvk))
-		if err != nil {
-			return nil, err
-		}
-		if len(gvkDependencyBundles) == 0 {
-			return nil, fmt.Errorf("could not find gvk dependencies for bundle '%s'", bundleEntity.ID)
-		}
-		for i := 0; i < len(gvkDependencyBundles); i++ {
-			entity := gvkDependencyBundles[i]
-			if _, ok := added[entity.ID]; !ok {
-				dependencies = append(dependencies, olmentity.NewBundleEntity(&entity))
-				added[entity.ID] = struct{}{}
-			}
-		}
-	}
-
 	// sort bundles in version order
 	sort.SliceStable(dependencies, func(i, j int) bool {
 		return entitysort.ByChannelAndVersion(dependencies[i].Entity, dependencies[j].Entity)

internal/resolution/variablesources/bundles_and_dependencies_test.go

@@ -160,28 +160,16 @@ var _ = Describe("BundlesAndDepsVariableSource", func() {
 				bundleVariables = append(bundleVariables, v)
 			}
 		}
-		Expect(bundleVariables).To(WithTransform(CollectBundleVariableIDs, Equal([]string{"bundle-2", "bundle-1", "bundle-15", "bundle-16", "bundle-17", "bundle-9", "bundle-8", "bundle-7", "bundle-5", "bundle-4", "bundle-11", "bundle-10"})))
+		// Note: When accounting for Required GVKs (currently not in use), we would expect additional
+		// dependencies (bundles 7, 8, 9, 10, 11) to appear here due to their GVKs being required by
+		// some of the packages.
+		Expect(bundleVariables).To(WithTransform(CollectBundleVariableIDs, Equal([]string{"bundle-2", "bundle-1", "bundle-15", "bundle-16", "bundle-17", "bundle-5", "bundle-4"})))
 
 		// check dependencies for one of the bundles
 		bundle2 := VariableWithID("bundle-2")(bundleVariables)
+		// Note: As above, bundle-2 has GVK requirements satisfied by bundles 7, 8, and 9, but they
+		// will not appear in this list as we are not currently taking Required GVKs into account
 		Expect(bundle2.Dependencies()).To(WithTransform(CollectDeppyEntities, Equal([]*input.Entity{
-			input.NewEntity("bundle-9", map[string]string{
-				property.TypePackage: `{"packageName": "another-package", "version": "1.0.0"}`,
-				property.TypeChannel: `{"channelName":"stable","priority":0}`,
-				property.TypeGVK:     `[{"group":"foo.io","kind":"Foo","version":"v1"}]`,
-			}),
-			input.NewEntity("bundle-8", map[string]string{
-				property.TypePackage:         `{"packageName": "some-other-package", "version": "1.5.0"}`,
-				property.TypeChannel:         `{"channelName":"stable","priority":0}`,
-				property.TypeGVK:             `[{"group":"foo.io","kind":"Foo","version":"v1"}]`,
-				property.TypeGVKRequired:     `[{"group":"bar.io","kind":"Bar","version":"v1"}]`,
-				property.TypePackageRequired: `[{"packageName": "another-package", "versionRange": "< 2.0.0"}]`,
-			}),
-			input.NewEntity("bundle-7", map[string]string{
-				property.TypePackage: `{"packageName": "some-other-package", "version": "1.0.0"}`,
-				property.TypeChannel: `{"channelName":"stable","priority":0}`,
-				property.TypeGVK:     `[{"group":"foo.io","kind":"Foo","version":"v1"}]`,
-			}),
 			input.NewEntity("bundle-5", map[string]string{
 				property.TypePackage: `{"packageName": "some-package", "version": "1.5.0"}`,
 				property.TypeChannel: `{"channelName":"stable","priority":0}`,

internal/resolution/variablesources/crd_constraints.go

@@ -44,7 +44,6 @@ func (g *CRDUniquenessConstraintsVariableSource) GetVariables(ctx context.Contex
 	//                   not all packages will necessarily export a CRD
 
 	pkgToBundleMap := map[string]map[deppy.Identifier]struct{}{}
-	gvkToBundleMap := map[string]map[deppy.Identifier]struct{}{}
 	for _, variable := range variables {
 		switch v := variable.(type) {
 		case *olmvariables.BundleVariable:
@@ -61,19 +60,6 @@ func (g *CRDUniquenessConstraintsVariableSource) GetVariables(ctx context.Contex
 					pkgToBundleMap[packageName] = map[deppy.Identifier]struct{}{}
 				}
 				pkgToBundleMap[packageName][bundleEntity.ID] = struct{}{}
-
-				// get bundleID gvks and update map
-				exportedGVKs, err := bundleEntity.ProvidedGVKs()
-				if err != nil {
-					return nil, fmt.Errorf("error creating global constraints: %w", err)
-				}
-				for i := 0; i < len(exportedGVKs); i++ {
-					gvk := exportedGVKs[i].String()
-					if _, ok := gvkToBundleMap[gvk]; !ok {
-						gvkToBundleMap[gvk] = map[deppy.Identifier]struct{}{}
-					}
-					gvkToBundleMap[gvk][bundleEntity.ID] = struct{}{}
-				}
 			}
 		}
 	}
@@ -88,14 +74,5 @@ func (g *CRDUniquenessConstraintsVariableSource) GetVariables(ctx context.Contex
 		variables = append(variables, olmvariables.NewBundleUniquenessVariable(varID, bundleIDs...))
 	}
 
-	for gvk, bundleIDMap := range gvkToBundleMap {
-		var bundleIDs []deppy.Identifier
-		for bundleID := range bundleIDMap {
-			bundleIDs = append(bundleIDs, bundleID)
-		}
-		varID := deppy.IdentifierFromString(fmt.Sprintf("%s gvk uniqueness", gvk))
-		variables = append(variables, olmvariables.NewBundleUniquenessVariable(varID, bundleIDs...))
-	}
-
 	return variables, nil
 }

internal/resolution/variablesources/crd_constraints_test.go

@@ -210,26 +210,25 @@ var _ = Describe("CRDUniquenessConstraintsVariableSource", func() {
 		}
 		variables, err := crdConstraintVariableSource.GetVariables(ctx, entitySource)
 		Expect(err).ToNot(HaveOccurred())
-		Expect(variables).To(HaveLen(26))
+		// Note: When accounting for GVK Uniqueness (which we are currently not doing), we
+		// would expect to have 26 variables from the 5 unique GVKs (Bar, Bit, Buz, Fiz, Foo).
+		Expect(variables).To(HaveLen(21))
 		var crdConstraintVariables []*olmvariables.BundleUniquenessVariable
 		for _, variable := range variables {
 			switch v := variable.(type) {
 			case *olmvariables.BundleUniquenessVariable:
 				crdConstraintVariables = append(crdConstraintVariables, v)
 			}
 		}
+		// Note: As above, the 5 GVKs would appear here as GVK uniqueness constraints
+		// if GVK Uniqueness were being accounted for.
 		Expect(crdConstraintVariables).To(WithTransform(CollectGlobalConstraintVariableIDs, ConsistOf([]string{
 			"another-package package uniqueness",
 			"bar-package package uniqueness",
 			"test-package-2 package uniqueness",
 			"test-package package uniqueness",
 			"some-package package uniqueness",
 			"some-other-package package uniqueness",
-			`group:"buz.io" version:"v1" kind:"Buz" gvk uniqueness`,
-			`group:"bit.io" version:"v1" kind:"Bit" gvk uniqueness`,
-			`group:"fiz.io" version:"v1" kind:"Fiz" gvk uniqueness`,
-			`group:"foo.io" version:"v1" kind:"Foo" gvk uniqueness`,
-			`group:"bar.io" version:"v1" kind:"Bar" gvk uniqueness`,
 		})))
 	})