Remove provides+required GVK and modify/remove associated tests (#408)

* Remove provides+required GVK and modify/remove associated tests Signed-off-by: dtfranz <[email protected]> * Add missing tests for bundle properties Signed-off-by: dtfranz <[email protected]> --------- Signed-off-by: dtfranz <[email protected]>
operator-framework · Sep 14, 2023 · ca95325 · ca95325
1 parent dd0dfaf
commit ca95325
Show file tree

Hide file tree

Showing 8 changed files with 219 additions and 275 deletions.
diff --git a/internal/catalogmetadata/filter/bundle_predicates.go b/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

diff --git a/internal/catalogmetadata/filter/bundle_predicates_test.go b/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"}}

diff --git a/internal/catalogmetadata/types.go b/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
 }
@@ -23,20 +29,9 @@ type Channel struct {
 	declcfg.Channel
 }
 
-type GVK property.GVK
-
-func (g GVK) String() string {
-	return fmt.Sprintf(`group:"%s" version:"%s" kind:"%s"`, g.Group, g.Version, g.Kind)
-}
-
-type GVKRequired property.GVKRequired
-
-func (g GVKRequired) String() string {
-	return fmt.Sprintf(`group:"%s" version:"%s" kind:"%s"`, g.Group, g.Version, g.Kind)
-}
-
-func (g GVKRequired) AsGVK() GVK {
-	return GVK(g)
+type PackageRequired struct {
+	property.PackageRequired
+	SemverRange *bsemver.Range `json:"-"`
 }
 
 type Bundle struct {
@@ -46,11 +41,11 @@ type Bundle struct {
 
 	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 +55,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 +90,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
 }