add filter tests

Signed-off-by: Joe Lanford <joe.lanford@gmail.com>

alpha/declcfg/filter/config/v1alpha1/config.go

@@ -3,6 +3,8 @@ package v1alpha1
 import (
 	"errors"
 	"fmt"
+	"io"
+	"io/ioutil"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/yaml"
@@ -41,7 +43,11 @@ type Channel struct {
 	VersionRange string `json:"versionRange,omitempty"`
 }
 
-func LoadFilterConfiguration(data []byte) (*FilterConfiguration, error) {
+func LoadFilterConfiguration(r io.Reader) (*FilterConfiguration, error) {
+	data, err := ioutil.ReadAll(r)
+	if err != nil {
+		return nil, err
+	}
 	cfg := &FilterConfiguration{}
 	if err := yaml.Unmarshal(data, cfg); err != nil {
 		return nil, err

alpha/declcfg/filter/config/v1alpha1/config_test.go

@@ -0,0 +1,137 @@
+package v1alpha1
+
+import (
+	"bytes"
+	"embed"
+	"errors"
+	"io"
+	"testing"
+	"testing/iotest"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestLoadFilterConfiguration(t *testing.T) {
+	tests := []struct {
+		name      string
+		openFile  func() (io.Reader, error)
+		assertion func(*testing.T, *FilterConfiguration, error)
+	}{
+		{
+			name:     "ReadFailure",
+			openFile: func() (io.Reader, error) { return iotest.ErrReader(errors.New("read failure")), nil },
+			assertion: func(t *testing.T, cfg *FilterConfiguration, err error) {
+				assert.Nil(t, cfg)
+				require.Error(t, err)
+				assert.ErrorContains(t, err, "read failure")
+			},
+		},
+		{
+			name:     "ParseFailure",
+			openFile: func() (io.Reader, error) { return bytes.NewReader([]byte(`{`)), nil },
+			assertion: func(t *testing.T, cfg *FilterConfiguration, err error) {
+				assert.Nil(t, cfg)
+				require.Error(t, err)
+				assert.ErrorContains(t, err, "yaml: line 1: did not find expected node content")
+			},
+		},
+		{
+			name:     "WrongAPIVersion",
+			openFile: func() (io.Reader, error) { return testdataFS.Open("testdata/invalid_apiversion.yaml") },
+			assertion: func(t *testing.T, cfg *FilterConfiguration, err error) {
+				assert.Nil(t, cfg)
+				require.Error(t, err)
+				assert.ErrorContains(t, err, "unexpected API version")
+			},
+		},
+		{
+			name:     "WrongKind",
+			openFile: func() (io.Reader, error) { return testdataFS.Open("testdata/invalid_kind.yaml") },
+			assertion: func(t *testing.T, cfg *FilterConfiguration, err error) {
+				assert.Nil(t, cfg)
+				require.Error(t, err)
+				assert.ErrorContains(t, err, "unexpected kind")
+			},
+		},
+		{
+			name:     "NoPackages",
+			openFile: func() (io.Reader, error) { return testdataFS.Open("testdata/invalid_nopackages.yaml") },
+			assertion: func(t *testing.T, cfg *FilterConfiguration, err error) {
+				assert.Nil(t, cfg)
+				require.Error(t, err)
+				assert.ErrorContains(t, err, "at least one package must be specified")
+			},
+		},
+		{
+			name:     "MissingPackageName",
+			openFile: func() (io.Reader, error) { return testdataFS.Open("testdata/invalid_missingpackagename.yaml") },
+			assertion: func(t *testing.T, cfg *FilterConfiguration, err error) {
+				assert.Nil(t, cfg)
+				require.Error(t, err)
+				assert.ErrorContains(t, err, `package "" at index [1] is invalid: name must be specified`)
+			},
+		},
+		{
+			name:     "MissingChannelName",
+			openFile: func() (io.Reader, error) { return testdataFS.Open("testdata/invalid_missingchannelname.yaml") },
+			assertion: func(t *testing.T, cfg *FilterConfiguration, err error) {
+				assert.Nil(t, cfg)
+				require.Error(t, err)
+				assert.ErrorContains(t, err, `package "bar" at index [1] is invalid: channel "" at index [1] is invalid: name must be specified`)
+			},
+		},
+		{
+			name:     "MultipleErrors",
+			openFile: func() (io.Reader, error) { return testdataFS.Open("testdata/invalid_multipleerrors.yaml") },
+			assertion: func(t *testing.T, cfg *FilterConfiguration, err error) {
+				assert.Nil(t, cfg)
+				require.Error(t, err)
+				assert.ErrorContains(t, err, `unexpected API version`)
+				assert.ErrorContains(t, err, "unexpected kind")
+				assert.ErrorContains(t, err, `package "" at index [2] is invalid: name must be specified`)
+				assert.ErrorContains(t, err, `package "" at index [2] is invalid: channel "" at index [0] is invalid: name must be specified`)
+				assert.ErrorContains(t, err, `package "" at index [2] is invalid: channel "" at index [1] is invalid: name must be specified`)
+				assert.ErrorContains(t, err, `package "" at index [3] is invalid: name must be specified`)
+				assert.ErrorContains(t, err, `package "" at index [3] is invalid: channel "" at index [0] is invalid: name must be specified`)
+				assert.ErrorContains(t, err, `package "" at index [3] is invalid: channel "" at index [1] is invalid: name must be specified`)
+				assert.ErrorContains(t, err, `package "baz" at index [4] is invalid: channel "" at index [0] is invalid: name must be specified`)
+			},
+		},
+		{
+			name:     "Valid",
+			openFile: func() (io.Reader, error) { return testdataFS.Open("testdata/valid.yaml") },
+			assertion: func(t *testing.T, cfg *FilterConfiguration, err error) {
+				require.NoError(t, err)
+				require.NotNil(t, cfg)
+				assert.Equal(t, "olm.operatorframework.io/v1alpha1", cfg.APIVersion)
+				assert.Equal(t, "FilterConfiguration", cfg.Kind)
+				assert.Len(t, cfg.Packages, 2)
+				assert.Equal(t, "foo", cfg.Packages[0].Name)
+				assert.Len(t, cfg.Packages[0].Channels, 0)
+				assert.Equal(t, "", cfg.Packages[0].DefaultChannel)
+				assert.Equal(t, "bar", cfg.Packages[1].Name)
+				assert.Len(t, cfg.Packages[1].Channels, 2)
+				assert.Equal(t, "bar-channel1", cfg.Packages[1].DefaultChannel)
+				assert.Equal(t, "bar-channel1", cfg.Packages[1].Channels[0].Name)
+				assert.Equal(t, ">=1.0.0 <2.0.0", cfg.Packages[1].Channels[0].VersionRange)
+				assert.Equal(t, "bar-channel2", cfg.Packages[1].Channels[1].Name)
+				assert.Equal(t, ">=2.0.0 <3.0.0", cfg.Packages[1].Channels[1].VersionRange)
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			f, err := tt.openFile()
+			if err != nil {
+				tt.assertion(t, nil, err)
+				return
+			}
+			cfg, err := LoadFilterConfiguration(f)
+			tt.assertion(t, cfg, err)
+		})
+	}
+}
+
+//go:embed testdata/*
+var testdataFS embed.FS

alpha/declcfg/filter/config/v1alpha1/filter.go

@@ -23,7 +23,7 @@ type filterOptions struct {
 
 type FilterOption func(*filterOptions)
 
-type filterer struct {
+type filter struct {
 	pkgConfigs map[string]Package
 	keeps      map[string]sets.Set[string]
 	opts       filterOptions
@@ -41,7 +41,7 @@ func nullLogger() *logrus.Entry {
 	return logrus.NewEntry(l)
 }
 
-func NewFilterer(config *FilterConfiguration, filterOpts ...FilterOption) declcfg.CatalogFilter {
+func NewFilter(config *FilterConfiguration, filterOpts ...FilterOption) declcfg.CatalogFilter {
 	opts := filterOptions{
 		Log: nullLogger(),
 	}
@@ -58,14 +58,14 @@ func NewFilterer(config *FilterConfiguration, filterOpts ...FilterOption) declcf
 		}
 		keeps[pkg.Name] = channels
 	}
-	return &filterer{
+	return &filter{
 		pkgConfigs: pkgConfigs,
 		keeps:      keeps,
 		opts:       opts,
 	}
 }
 
-func (f *filterer) FilterCatalog(_ context.Context, fbc *declcfg.DeclarativeConfig) (*declcfg.DeclarativeConfig, error) {
+func (f *filter) FilterCatalog(_ context.Context, fbc *declcfg.DeclarativeConfig) (*declcfg.DeclarativeConfig, error) {
 	m, err := declcfg.ConvertToModel(*fbc)
 	if err != nil {
 		return nil, err
@@ -89,7 +89,7 @@ func (f *filterer) FilterCatalog(_ context.Context, fbc *declcfg.DeclarativeConf
 	return &filtered, nil
 }
 
-func (f *filterer) KeepMeta(meta *declcfg.Meta) bool {
+func (f *filter) KeepMeta(meta *declcfg.Meta) bool {
 	if len(f.keeps) == 0 {
 		return true
 	}
@@ -103,15 +103,15 @@ func (f *filterer) KeepMeta(meta *declcfg.Meta) bool {
 	return ok
 }
 
-func (f *filterer) filterPackages(m model.Model) {
+func (f *filter) filterPackages(m model.Model) {
 	for _, pkg := range m {
 		if _, ok := f.keeps[pkg.Name]; !ok {
 			delete(m, pkg.Name)
 		}
 	}
 }
 
-func (f *filterer) filterChannels(pkg *model.Package) error {
+func (f *filter) filterChannels(pkg *model.Package) error {
 	// if no channels are set, then no channel filtering is needed
 	pkgConfig, ok := f.pkgConfigs[pkg.Name]
 	if !ok || len(pkgConfig.Channels) == 0 {
@@ -180,7 +180,7 @@ func setDefaultChannel(pkg *model.Package, pkgConfig Package) error {
 //     be included in the filtered channel.
 //
 // If the entire channel is filtered out, we will emit an error.
-func (f *filterer) filterBundles(ch *model.Channel, versionRange string) error {
+func (f *filter) filterBundles(ch *model.Channel, versionRange string) error {
 	if versionRange == "" {
 		return nil
 	}

alpha/declcfg/filter/config/v1alpha1/testdata/invalid_apiversion.yaml

@@ -0,0 +1,10 @@
+apiVersion: olmfiltering.operatorframework.io/v1alpha1
+kind: FilterConfiguration
+packages:
+  - name: "foo"
+  - name: "bar"
+    channels:
+      - name: "bar-channel1"
+        versionRange: ">=1.0.0 <2.0.0"
+      - name: "bar-channel2"
+        versionRange: ">=2.0.0 <3.0.0"

alpha/declcfg/filter/config/v1alpha1/testdata/invalid_kind.yaml

@@ -0,0 +1,10 @@
+apiVersion: olm.operatorframework.io/v1alpha1
+kind: CatalogFilterConfiguration
+packages:
+  - name: "foo"
+  - name: "bar"
+    channels:
+      - name: "bar-channel1"
+        versionRange: ">=1.0.0 <2.0.0"
+      - name: "bar-channel2"
+        versionRange: ">=2.0.0 <3.0.0"

alpha/declcfg/filter/config/v1alpha1/testdata/invalid_missingchannelname.yaml

@@ -0,0 +1,10 @@
+apiVersion: olm.operatorframework.io/v1alpha1
+kind: FilterConfiguration
+packages:
+  - name: "foo"
+  - name: "bar"
+    channels:
+      - name: "bar-channel1"
+        versionRange: ">=1.0.0 <2.0.0"
+      - name: ""
+        versionRange: ">=2.0.0 <3.0.0"

alpha/declcfg/filter/config/v1alpha1/testdata/invalid_missingpackagename.yaml

@@ -0,0 +1,5 @@
+apiVersion: olm.operatorframework.io/v1alpha1
+kind: FilterConfiguration
+packages:
+  - name: "foo"
+  - name: ""

alpha/declcfg/filter/config/v1alpha1/testdata/invalid_multipleerrors.yaml

@@ -0,0 +1,24 @@
+apiVersion: olmfiltering.operatorframework.io/v1alpha1
+kind: CatalogFilterConfiguration
+packages:
+  - name: "foo"
+  - name: "bar"
+    channels:
+      - name: "bar-channel1"
+        versionRange: ">=1.0.0 <2.0.0"
+      - name: "bar-channel2"
+        versionRange: ">=2.0.0 <3.0.0"
+  - name: ""
+    channels:
+      - name: ""
+      - name: ""
+  - name: ""
+    channels:
+      - name: ""
+      - name: ""
+  - name: "baz"
+    channels:
+      - name: ""
+        versionRange: ">=1.0.0 <2.0.0"
+      - name: "baz-channel2"
+        versionRange: ">=2.0.0 <3.0.0"

alpha/declcfg/filter/config/v1alpha1/testdata/invalid_nopackages.yaml

@@ -0,0 +1,2 @@
+apiVersion: olm.operatorframework.io/v1alpha1
+kind: FilterConfiguration

alpha/declcfg/filter/config/v1alpha1/testdata/valid.yaml

@@ -0,0 +1,11 @@
+apiVersion: olm.operatorframework.io/v1alpha1
+kind: FilterConfiguration
+packages:
+  - name: "foo"
+  - name: "bar"
+    defaultChannel: "bar-channel1"
+    channels:
+      - name: "bar-channel1"
+        versionRange: ">=1.0.0 <2.0.0"
+      - name: "bar-channel2"
+        versionRange: ">=2.0.0 <3.0.0"

alpha/declcfg/filter/filter.go

@@ -9,8 +9,12 @@ import (
 	"github.com/operator-framework/operator-registry/alpha/declcfg"
 )
 
+// KeepAppMetas is a MetaFilter that unconditionally keeps every meta object. This is a useful default for filters that
+// don't need to filter meta objects.
 var KeepAllMetas = declcfg.MetaFilter(declcfg.MetaFilterFunc(func(meta *declcfg.Meta) bool { return true }))
 
+// NewPackageFilter returns a CatalogFilter that keeps package, channel, bundle, deprecation, and other
+// meta objects for the given package names. All objects for all other packages are removed.
 func NewPackageFilter(keepPackages ...string) declcfg.CatalogFilter {
 	return &packageFilter{keepPackages: sets.New[string](keepPackages...)}
 }
@@ -20,19 +24,14 @@ type packageFilter struct {
 }
 
 func (f *packageFilter) FilterCatalog(_ context.Context, fbc *declcfg.DeclarativeConfig) (*declcfg.DeclarativeConfig, error) {
-	slices.DeleteFunc(fbc.Packages, func(pkg declcfg.Package) bool {
-		return !f.keepPackages.Has(pkg.Name)
-	})
-	slices.DeleteFunc(fbc.Channels, func(channel declcfg.Channel) bool {
-		return !f.keepPackages.Has(channel.Package)
-	})
-	slices.DeleteFunc(fbc.Bundles, func(bundle declcfg.Bundle) bool {
-		return !f.keepPackages.Has(bundle.Package)
-	})
-	slices.DeleteFunc(fbc.Deprecations, func(deprecation declcfg.Deprecation) bool { return !f.keepPackages.Has(deprecation.Package) })
-	slices.DeleteFunc(fbc.Others, func(other declcfg.Meta) bool {
-		return !f.keepPackages.Has(other.Package)
-	})
+	if fbc == nil {
+		return nil, nil
+	}
+	fbc.Packages = slices.DeleteFunc(fbc.Packages, func(pkg declcfg.Package) bool { return !f.keepPackages.Has(pkg.Name) })
+	fbc.Channels = slices.DeleteFunc(fbc.Channels, func(channel declcfg.Channel) bool { return !f.keepPackages.Has(channel.Package) })
+	fbc.Bundles = slices.DeleteFunc(fbc.Bundles, func(bundle declcfg.Bundle) bool { return !f.keepPackages.Has(bundle.Package) })
+	fbc.Deprecations = slices.DeleteFunc(fbc.Deprecations, func(deprecation declcfg.Deprecation) bool { return !f.keepPackages.Has(deprecation.Package) })
+	fbc.Others = slices.DeleteFunc(fbc.Others, func(other declcfg.Meta) bool { return !f.keepPackages.Has(other.Package) })
 	return fbc, nil
 }