use a type to protect migration interactions

Signed-off-by: Jordan Keister <jordan@nimblewidget.com>

alpha/action/migrate.go

@@ -5,14 +5,15 @@ import (
 	"fmt"
 	"os"
 
+	"github.com/operator-framework/operator-registry/alpha/action/migrations"
 	"github.com/operator-framework/operator-registry/alpha/declcfg"
 	"github.com/operator-framework/operator-registry/pkg/image"
 )
 
 type Migrate struct {
 	CatalogRef string
 	OutputDir  string
-	Level      string
+	Migrations *migrations.Migrations
 
 	WriteFunc declcfg.WriteFunc
 	FileExt   string
@@ -29,8 +30,8 @@ func (m Migrate) Run(ctx context.Context) error {
 	}
 
 	r := Render{
-		Refs:           []string{m.CatalogRef},
-		MigrationLevel: m.Level,
+		Refs:       []string{m.CatalogRef},
+		Migrations: m.Migrations,
 
 		// Only allow catalogs to be migrated.
 		AllowedRefMask: RefSqliteImage | RefSqliteFile | RefDCImage | RefDCDir,

alpha/action/migrations/000_bundle_object_to_csv_metadata.go

@@ -9,42 +9,39 @@ import (
 	"github.com/operator-framework/operator-registry/alpha/property"
 )
 
-var bundleObjectToCSVMetadata = newMigration(
-	"bundle-object-to-csv-metadata",
-	"migrates bundles' `olm.bundle.object` to `olm.csv.metadata`",
-	func(cfg *declcfg.DeclarativeConfig) error {
-		convertBundleObjectToCSVMetadata := func(b *declcfg.Bundle) error {
-			if b.Image == "" || b.CsvJSON == "" {
-				return nil
-			}
-
-			var csv v1alpha1.ClusterServiceVersion
-			if err := json.Unmarshal([]byte(b.CsvJSON), &csv); err != nil {
-				return err
-			}
-
-			props := b.Properties[:0]
-			for _, p := range b.Properties {
-				switch p.Type {
-				case property.TypeBundleObject:
-					// Get rid of the bundle objects
-				case property.TypeCSVMetadata:
-					// If this bundle already has a CSV metadata
-					// property, we won't mutate the bundle at all.
-					return nil
-				default:
-					// Keep all of the other properties
-					props = append(props, p)
-				}
-			}
-			b.Properties = append(props, property.MustBuildCSVMetadata(csv))
+func bundleObjectToCSVMetadata(cfg *declcfg.DeclarativeConfig) error {
+	convertBundleObjectToCSVMetadata := func(b *declcfg.Bundle) error {
+		if b.Image == "" || b.CsvJSON == "" {
 			return nil
 		}
 
-		for bi := range cfg.Bundles {
-			if err := convertBundleObjectToCSVMetadata(&cfg.Bundles[bi]); err != nil {
-				return err
+		var csv v1alpha1.ClusterServiceVersion
+		if err := json.Unmarshal([]byte(b.CsvJSON), &csv); err != nil {
+			return err
+		}
+
+		props := b.Properties[:0]
+		for _, p := range b.Properties {
+			switch p.Type {
+			case property.TypeBundleObject:
+				// Get rid of the bundle objects
+			case property.TypeCSVMetadata:
+				// If this bundle already has a CSV metadata
+				// property, we won't mutate the bundle at all.
+				return nil
+			default:
+				// Keep all of the other properties
+				props = append(props, p)
 			}
 		}
+		b.Properties = append(props, property.MustBuildCSVMetadata(csv))
 		return nil
-	})
+	}
+
+	for bi := range cfg.Bundles {
+		if err := convertBundleObjectToCSVMetadata(&cfg.Bundles[bi]); err != nil {
+			return err
+		}
+	}
+	return nil
+}

alpha/action/migrations/migrations.go

@@ -9,24 +9,32 @@ import (
 	"github.com/operator-framework/operator-registry/alpha/declcfg"
 )
 
+type MigrationToken string
+
+const (
+	invalidMigration string = ""
+	NoMigrations     string = "none"
+	AllMigrations    string = "all"
+)
+
 type Migration interface {
-	Name() string
+	Token() MigrationToken
 	Help() string
 	Migrate(*declcfg.DeclarativeConfig) error
 }
 
-func newMigration(name string, help string, fn func(config *declcfg.DeclarativeConfig) error) Migration {
-	return &simpleMigration{name: name, help: help, fn: fn}
+func newMigration(token string, help string, fn func(config *declcfg.DeclarativeConfig) error) Migration {
+	return &simpleMigration{token: MigrationToken(token), help: help, fn: fn}
 }
 
 type simpleMigration struct {
-	name string
-	help string
-	fn   func(*declcfg.DeclarativeConfig) error
+	token MigrationToken
+	help  string
+	fn    func(*declcfg.DeclarativeConfig) error
 }
 
-func (s simpleMigration) Name() string {
-	return s.name
+func (s simpleMigration) Token() MigrationToken {
+	return s.token
 }
 
 func (s simpleMigration) Migrate(config *declcfg.DeclarativeConfig) error {
@@ -41,22 +49,33 @@ type Migrations struct {
 	Migrations []Migration
 }
 
-func GetLastMigrationName() string {
-	if len(allMigrations) == 0 {
-		return ""
+func validateName(name string) (MigrationToken, error) {
+	if name == AllMigrations {
+		return MigrationToken(AllMigrations), nil
 	}
-	return allMigrations[len(allMigrations)-1].Name()
+	for _, migration := range allMigrations {
+		if migration.Token() == MigrationToken(name) {
+			return migration.Token(), nil
+		}
+	}
+	return MigrationToken(invalidMigration), fmt.Errorf("unknown migration level %q", name)
 }
 
 // allMigrations represents the migration catalog
 // the order of these migrations is important
 var allMigrations = []Migration{
-	bundleObjectToCSVMetadata,
+	newMigration(NoMigrations, "do nothing", func(_ *declcfg.DeclarativeConfig) error { return nil }),
+	newMigration("bundle-object-to-csv-metadata", `migrates bundles' "olm.bundle.object" to "olm.csv.metadata"`, bundleObjectToCSVMetadata),
 }
 
-func NewMigrations(level string) (*Migrations, error) {
-	if level == "" {
-		return &Migrations{}, nil
+func NewMigrations(name string) (*Migrations, error) {
+	if name == AllMigrations {
+		return &Migrations{Migrations: slices.Clone(allMigrations)}, nil
+	}
+
+	token, err := validateName(name)
+	if err != nil {
+		return nil, err
 	}
 
 	migrations := slices.Clone(allMigrations)
@@ -65,28 +84,28 @@ func NewMigrations(level string) (*Migrations, error) {
 	keep := migrations[:0]
 	for _, migration := range migrations {
 		keep = append(keep, migration)
-		if migration.Name() == level {
+		if migration.Token() == token {
 			found = true
 			break
 		}
 	}
 	if !found {
-		return nil, fmt.Errorf("unknown migration level %q", level)
+		return nil, fmt.Errorf("unknown migration level %q", name)
 	}
 	return &Migrations{Migrations: keep}, nil
 }
 
 func HelpText() string {
 	var help strings.Builder
-	help.WriteString("  The migrator will run all migrations up to and including the selected level.\n\n")
-	help.WriteString("  Available migrators:\n")
+	help.WriteString("\nThe migrator will run all migrations up to and including the selected level.\n\n")
+	help.WriteString("Available migrators:\n")
 	if len(allMigrations) == 0 {
 		help.WriteString("   (no migrations available in this version)\n")
 	}
 
-	tabber := tabwriter.NewWriter(&help, 20, 30, 1, '\t', tabwriter.AlignRight)
+	tabber := tabwriter.NewWriter(&help, 0, 0, 1, ' ', 0)
 	for _, migration := range allMigrations {
-		fmt.Fprintf(tabber, "    - %s\t%s\n", migration.Name(), migration.Help())
+		fmt.Fprintf(tabber, "  - %s\t: %s\n", migration.Token(), migration.Help())
 	}
 	tabber.Flush()
 	return help.String()

alpha/action/migrations/migrations_test.go

@@ -0,0 +1,132 @@
+package migrations
+
+import (
+	"bytes"
+	"encoding/json"
+	"fmt"
+	"io"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/operator-framework/api/pkg/operators/v1alpha1"
+	"github.com/operator-framework/operator-registry/alpha/declcfg"
+	"github.com/operator-framework/operator-registry/alpha/property"
+	"github.com/stretchr/testify/require"
+)
+
+func TestMigrations(t *testing.T) {
+	noneMigration, err := NewMigrations(NoMigrations)
+	require.NoError(t, err)
+	csvMigration, err := NewMigrations("bundle-object-to-csv-metadata")
+	require.NoError(t, err)
+	allMigrations, err := NewMigrations(AllMigrations)
+	require.NoError(t, err)
+
+	var evaluators map[MigrationToken]func(*declcfg.DeclarativeConfig) error = map[MigrationToken]func(*declcfg.DeclarativeConfig) error{
+		MigrationToken(NoMigrations): func(d *declcfg.DeclarativeConfig) error {
+			if diff := cmp.Diff(*d, unmigratedCatalogFBC); diff != "" {
+				return fmt.Errorf("'none' migrator is not expected to change the config\n%s", diff)
+			}
+			return nil
+		},
+		MigrationToken("bundle-object-to-csv-metadata"): func(d *declcfg.DeclarativeConfig) error {
+			if diff := cmp.Diff(*d, csvMetadataCatalogFBC); diff == "" {
+				return fmt.Errorf("unexpected result of migration\n%s", diff)
+			}
+			return nil
+		},
+	}
+
+	tests := []struct {
+		name      string
+		migrators *Migrations
+	}{
+		{
+			name:      "NoMigrations",
+			migrators: noneMigration,
+		},
+		{
+			name:      "BundleObjectToCSVMetadata",
+			migrators: csvMigration,
+		},
+		{
+			name:      "MigrationSequence",
+			migrators: allMigrations,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			var config declcfg.DeclarativeConfig = unmigratedCatalogFBC
+
+			for _, m := range test.migrators.Migrations {
+				err := m.Migrate(&config)
+				require.NoError(t, err)
+				err = evaluators[m.Token()](&config)
+				require.NoError(t, err)
+			}
+		})
+	}
+}
+
+func mustBuildCSVMetadata(r io.Reader) property.Property {
+	var csv v1alpha1.ClusterServiceVersion
+	if err := json.NewDecoder(r).Decode(&csv); err != nil {
+		panic(err)
+	}
+	return property.MustBuildCSVMetadata(csv)
+}
+
+var fooRawCsv = []byte(`{"apiVersion": "operators.coreos.com/v1alpha1", "kind": "ClusterServiceVersion", "metadata": {"name": "foo.v0.1.0"}, "spec": {"displayName": "Foo Operator", "customresourcedefinitions": {"owned": [{"group": "test.foo", "version": "v1", "kind": "Foo", "name": "foos.test.foo"}]}, "version": "0.1.0", "relatedImages": [{"name": "operator", "image": "test.registry/foo-operator/foo:v0.1.0"}]}}`)
+
+var fooRawCrd = []byte(`---
+apiVersion: apiextensions.k8s.io/v1
+kind: CustomResourceDefinition
+metadata:
+  name: foos.test.foo
+spec:
+  group: test.foo
+  names:
+    kind: Foo
+    plural: foos
+  versions:
+    - name: v1`,
+)
+
+var unmigratedCatalogFBC = declcfg.DeclarativeConfig{
+	Bundles: []declcfg.Bundle{
+		{
+			Schema:  "olm.bundle",
+			Name:    "foo.v0.1.0",
+			Package: "foo",
+			Properties: []property.Property{
+				property.MustBuildGVK("test.foo", "v1", "Foo"),
+				property.MustBuildGVKRequired("test.bar", "v1alpha1", "Bar"),
+				property.MustBuildPackage("foo", "0.1.0"),
+				property.MustBuildPackageRequired("bar", "<0.1.0"),
+				property.MustBuildBundleObject(fooRawCrd),
+				property.MustBuildBundleObject(fooRawCsv),
+			},
+			Objects: []string{string(fooRawCsv), string(fooRawCrd)},
+			CsvJSON: string(fooRawCsv),
+		},
+	},
+}
+var csvMetadataCatalogFBC = declcfg.DeclarativeConfig{
+	Bundles: []declcfg.Bundle{
+		{
+			Schema:  "olm.bundle",
+			Name:    "foo.v0.2.0",
+			Package: "foo",
+			Properties: []property.Property{
+				property.MustBuildGVK("test.foo", "v1", "Foo"),
+				property.MustBuildGVKRequired("test.bar", "v1alpha1", "Bar"),
+				property.MustBuildPackage("foo", "0.1.0"),
+				property.MustBuildPackageRequired("bar", "<0.1.0"),
+				mustBuildCSVMetadata(bytes.NewReader(fooRawCsv)),
+			},
+			Objects: []string{string(fooRawCsv), string(fooRawCrd)},
+			CsvJSON: string(fooRawCsv),
+		},
+	},
+}

alpha/action/render.go

@@ -54,8 +54,8 @@ type Render struct {
 	Refs             []string
 	Registry         image.Registry
 	AllowedRefMask   RefType
-	MigrationLevel   string
 	ImageRefTemplate *template.Template
+	Migrations       *migrations.Migrations
 
 	skipSqliteDeprecationLog bool
 }
@@ -88,7 +88,7 @@ func (r Render) Run(ctx context.Context) (*declcfg.DeclarativeConfig, error) {
 			})
 		}
 
-		if err := migrate(cfg, r.MigrationLevel); err != nil {
+		if err := r.migrate(cfg); err != nil {
 			return nil, fmt.Errorf("migrate: %v", err)
 		}
 
@@ -414,18 +414,12 @@ func moveBundleObjectsToEndOfPropertySlices(cfg *declcfg.DeclarativeConfig) {
 	}
 }
 
-func migrate(cfg *declcfg.DeclarativeConfig, migrateLevel string) error {
-	mobj, err := migrations.NewMigrations(migrateLevel)
-	if err != nil {
-		return err
-	}
-
-	err = mobj.Migrate(cfg)
-	if err != nil {
-		return err
+func (r Render) migrate(cfg *declcfg.DeclarativeConfig) error {
+	// If there are no migrations, do nothing.
+	if r.Migrations == nil {
+		return nil
 	}
-
-	return nil
+	return r.Migrations.Migrate(cfg)
 }
 
 func combineConfigs(cfgs []declcfg.DeclarativeConfig) *declcfg.DeclarativeConfig {