From c7618255c39298faeec831fca11c3b5abae1d01e Mon Sep 17 00:00:00 2001 From: Jordan Keister Date: Tue, 23 Jul 2024 14:13:05 -0500 Subject: [PATCH] schema migration optional; default to olm.bundle.object Signed-off-by: Jordan Keister --- .../000_bundle_object_to_csv_metadata.go | 47 ----------------- alpha/action/migrate.go | 6 +-- .../000_bundle_object_to_csv_metadata.go | 50 +++++++++++++++++++ .../{internal => }/migrations/migrations.go | 32 +++++++----- alpha/action/render.go | 30 ++++------- alpha/action/render_test.go | 12 ++--- alpha/template/basic/basic.go | 6 +-- alpha/template/semver/semver.go | 2 +- alpha/template/semver/types.go | 6 +-- cmd/opm/alpha/render-graph/cmd.go | 2 +- cmd/opm/alpha/template/basic.go | 3 +- cmd/opm/alpha/template/semver.go | 5 +- cmd/opm/migrate/cmd.go | 7 ++- cmd/opm/render/cmd.go | 9 ++-- 14 files changed, 111 insertions(+), 106 deletions(-) delete mode 100644 alpha/action/internal/migrations/000_bundle_object_to_csv_metadata.go create mode 100644 alpha/action/migrations/000_bundle_object_to_csv_metadata.go rename alpha/action/{internal => }/migrations/migrations.go (65%) diff --git a/alpha/action/internal/migrations/000_bundle_object_to_csv_metadata.go b/alpha/action/internal/migrations/000_bundle_object_to_csv_metadata.go deleted file mode 100644 index cc9d4f35d..000000000 --- a/alpha/action/internal/migrations/000_bundle_object_to_csv_metadata.go +++ /dev/null @@ -1,47 +0,0 @@ -package migrations - -import ( - "encoding/json" - - "github.com/operator-framework/api/pkg/operators/v1alpha1" - - "github.com/operator-framework/operator-registry/alpha/declcfg" - "github.com/operator-framework/operator-registry/alpha/property" -) - -var BundleObjectToCSVMetadata = NewMigration("bundle-object-to-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)) - return nil - } - - for bi := range cfg.Bundles { - if err := convertBundleObjectToCSVMetadata(&cfg.Bundles[bi]); err != nil { - return err - } - } - return nil -}) diff --git a/alpha/action/migrate.go b/alpha/action/migrate.go index 78c5f41a8..962cd6489 100644 --- a/alpha/action/migrate.go +++ b/alpha/action/migrate.go @@ -12,7 +12,7 @@ import ( type Migrate struct { CatalogRef string OutputDir string - Stages int + Level string WriteFunc declcfg.WriteFunc FileExt string @@ -29,8 +29,8 @@ func (m Migrate) Run(ctx context.Context) error { } r := Render{ - Refs: []string{m.CatalogRef}, - MigrateStages: m.Stages, + Refs: []string{m.CatalogRef}, + MigrationLevel: m.Level, // Only allow catalogs to be migrated. AllowedRefMask: RefSqliteImage | RefSqliteFile | RefDCImage | RefDCDir, diff --git a/alpha/action/migrations/000_bundle_object_to_csv_metadata.go b/alpha/action/migrations/000_bundle_object_to_csv_metadata.go new file mode 100644 index 000000000..2da207b28 --- /dev/null +++ b/alpha/action/migrations/000_bundle_object_to_csv_metadata.go @@ -0,0 +1,50 @@ +package migrations + +import ( + "encoding/json" + + "github.com/operator-framework/api/pkg/operators/v1alpha1" + + "github.com/operator-framework/operator-registry/alpha/declcfg" + "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)) + return nil + } + + for bi := range cfg.Bundles { + if err := convertBundleObjectToCSVMetadata(&cfg.Bundles[bi]); err != nil { + return err + } + } + return nil + }) diff --git a/alpha/action/internal/migrations/migrations.go b/alpha/action/migrations/migrations.go similarity index 65% rename from alpha/action/internal/migrations/migrations.go rename to alpha/action/migrations/migrations.go index d52b76f6b..a66408fd3 100644 --- a/alpha/action/internal/migrations/migrations.go +++ b/alpha/action/migrations/migrations.go @@ -10,15 +10,17 @@ import ( type Migration interface { Name() string + Help() string Migrate(*declcfg.DeclarativeConfig) error } -func NewMigration(name string, fn func(config *declcfg.DeclarativeConfig) error) Migration { - return &simpleMigration{name: name, fn: fn} +func newMigration(name string, help string, fn func(config *declcfg.DeclarativeConfig) error) Migration { + return &simpleMigration{name: name, help: help, fn: fn} } type simpleMigration struct { name string + help string fn func(*declcfg.DeclarativeConfig) error } @@ -30,10 +32,16 @@ func (s simpleMigration) Migrate(config *declcfg.DeclarativeConfig) error { return s.fn(config) } +func (s simpleMigration) Help() string { + return s.help +} + type Migrations struct { - migrations []Migration + Migrations []Migration } +// allMigrations represents the migration catalog +// the order of these migrations is important var allMigrations = []Migration{ BundleObjectToCSVMetadata, } @@ -41,7 +49,7 @@ var allMigrations = []Migration{ func NewMigrations(level string) (*Migrations, error) { migrations := slices.Clone(allMigrations) if level == "" { - return &Migrations{migrations: migrations}, nil + return &Migrations{Migrations: migrations}, nil } found := false @@ -56,26 +64,24 @@ func NewMigrations(level string) (*Migrations, error) { if !found { return nil, fmt.Errorf("unknown migration level %q", level) } - return &Migrations{migrations: keep}, nil + return &Migrations{Migrations: keep}, nil } -func (m *Migrations) HelpText() string { +func HelpText() string { var help strings.Builder - help.WriteString("-- Migrations --\n") - help.WriteString(" To run a migration, use the --level flag with the migration name.\n") help.WriteString(" The migrator will run all migrations up to and including the selected level.\n\n") - help.WriteString(" Available migration levels:\n") - if len(m.migrations) == 0 { + help.WriteString(" Available migrators:\n") + if len(allMigrations) == 0 { help.WriteString(" (no migrations available in this version)\n") } - for i, migration := range m.migrations { - help.WriteString(fmt.Sprintf(" - %s\n", i+1, migration.Name())) + for _, migration := range allMigrations { + help.WriteString(fmt.Sprintf(" - %s\n", migration.Name())) } return help.String() } func (m *Migrations) Migrate(config *declcfg.DeclarativeConfig) error { - for _, migration := range m.migrations { + for _, migration := range m.Migrations { if err := migration.Migrate(config); err != nil { return err } diff --git a/alpha/action/render.go b/alpha/action/render.go index a94bbeea0..320835314 100644 --- a/alpha/action/render.go +++ b/alpha/action/render.go @@ -19,6 +19,7 @@ import ( "github.com/operator-framework/api/pkg/operators/v1alpha1" + "github.com/operator-framework/operator-registry/alpha/action/migrations" "github.com/operator-framework/operator-registry/alpha/declcfg" "github.com/operator-framework/operator-registry/alpha/property" "github.com/operator-framework/operator-registry/pkg/containertools" @@ -55,7 +56,7 @@ type Render struct { Refs []string Registry image.Registry AllowedRefMask RefType - MigrateStages int + MigrationLevel string ImageRefTemplate *template.Template skipSqliteDeprecationLog bool @@ -89,7 +90,7 @@ func (r Render) Run(ctx context.Context) (*declcfg.DeclarativeConfig, error) { }) } - if err := migrate(cfg, r.MigrateStages); err != nil { + if err := migrate(cfg, r.MigrationLevel); err != nil { return nil, fmt.Errorf("migrate: %v", err) } @@ -415,27 +416,14 @@ func moveBundleObjectsToEndOfPropertySlices(cfg *declcfg.DeclarativeConfig) { } } -func migrate(cfg *declcfg.DeclarativeConfig, migrateStages int) error { - // Do not delete or change the order of the below migrations. - // The --migrate-stage flag is an API that depends on the presence - // and order of these migrations. - migrations := []func(*declcfg.DeclarativeConfig) error{ - convertObjectsToCSVMetadata, - } - - if migrateStages > len(migrations) { - return fmt.Errorf("number of requested migration stages to run (%d) exceeds number of available migrations (%d)", migrateStages, len(migrations)) +func migrate(cfg *declcfg.DeclarativeConfig, migrateLevel string) error { + mobj, err := migrations.NewMigrations(migrateLevel) + if err != nil { + return err } - // migrateStages <0 means all migrations - // migrateStages 0 means no migrations - // migrateStages 1 means only the first migration - // etc... - for i, m := range migrations { - if i == migrateStages { - break - } - if err := m(cfg); err != nil { + for _, m := range (*mobj).Migrations { + if err := m.Migrate(cfg); err != nil { return err } } diff --git a/alpha/action/render_test.go b/alpha/action/render_test.go index b1ee6840f..46b28bd40 100644 --- a/alpha/action/render_test.go +++ b/alpha/action/render_test.go @@ -453,9 +453,9 @@ func TestRender(t *testing.T) { { name: "Success/DeclcfgImageMigrate", render: action.Render{ - Refs: []string{"test.registry/foo-operator/foo-index-declcfg:v0.2.0"}, - Migrate: true, - Registry: reg, + Refs: []string{"test.registry/foo-operator/foo-index-declcfg:v0.2.0"}, + MigrationLevel: "bundle-object-to-csv-metadata", + Registry: reg, }, expectCfg: &declcfg.DeclarativeConfig{ Packages: []declcfg.Package{ @@ -550,9 +550,9 @@ func TestRender(t *testing.T) { { name: "Success/DeclcfgDirectoryMigrate", render: action.Render{ - Refs: []string{"testdata/foo-index-v0.2.0-declcfg"}, - Migrate: true, - Registry: reg, + Refs: []string{"testdata/foo-index-v0.2.0-declcfg"}, + MigrationLevel: "bundle-object-to-csv-metadata", + Registry: reg, }, expectCfg: &declcfg.DeclarativeConfig{ Packages: []declcfg.Package{ diff --git a/alpha/template/basic/basic.go b/alpha/template/basic/basic.go index 90a114062..766d32ac3 100644 --- a/alpha/template/basic/basic.go +++ b/alpha/template/basic/basic.go @@ -16,8 +16,8 @@ import ( const schema string = "olm.template.basic" type Template struct { - Registry image.Registry - MigrateStages int + Registry image.Registry + MigrateLevel string } type BasicTemplate struct { @@ -60,7 +60,7 @@ func (t Template) Render(ctx context.Context, reader io.Reader) (*declcfg.Declar r := action.Render{ Registry: t.Registry, AllowedRefMask: action.RefBundleImage, - MigrateStages: t.MigrateStages, + MigrationLevel: t.MigrateLevel, } for _, b := range cfg.Bundles { diff --git a/alpha/template/semver/semver.go b/alpha/template/semver/semver.go index c153bdba8..aecbfa094 100644 --- a/alpha/template/semver/semver.go +++ b/alpha/template/semver/semver.go @@ -35,7 +35,7 @@ func (t Template) Render(ctx context.Context) (*declcfg.DeclarativeConfig, error AllowedRefMask: action.RefBundleImage, Refs: []string{b}, Registry: t.Registry, - MigrateStages: t.MigrateStages, + MigrationLevel: t.MigrateLevel, } c, err := r.Run(ctx) if err != nil { diff --git a/alpha/template/semver/types.go b/alpha/template/semver/types.go index 0fe26fb1b..5c616a5c6 100644 --- a/alpha/template/semver/types.go +++ b/alpha/template/semver/types.go @@ -10,9 +10,9 @@ import ( // data passed into this module externally type Template struct { - Data io.Reader - Registry image.Registry - MigrateStages int + Data io.Reader + Registry image.Registry + MigrateLevel string } // IO structs -- BEGIN diff --git a/cmd/opm/alpha/render-graph/cmd.go b/cmd/opm/alpha/render-graph/cmd.go index da534f851..1c33c4b20 100644 --- a/cmd/opm/alpha/render-graph/cmd.go +++ b/cmd/opm/alpha/render-graph/cmd.go @@ -63,7 +63,7 @@ $ opm alpha render-graph quay.io/operatorhubio/catalog:latest | \ render.Registry = registry // Run all migrations - render.MigrateStages = -1 + render.MigrationLevel = "" cfg, err := render.Run(cmd.Context()) if err != nil { diff --git a/cmd/opm/alpha/template/basic.go b/cmd/opm/alpha/template/basic.go index bf13d4138..9e99bd2c9 100644 --- a/cmd/opm/alpha/template/basic.go +++ b/cmd/opm/alpha/template/basic.go @@ -8,6 +8,7 @@ import ( "github.com/sirupsen/logrus" "github.com/spf13/cobra" + "github.com/operator-framework/operator-registry/alpha/action/migrations" "github.com/operator-framework/operator-registry/alpha/declcfg" "github.com/operator-framework/operator-registry/alpha/template/basic" "github.com/operator-framework/operator-registry/cmd/opm/internal/util" @@ -74,7 +75,7 @@ When FILE is '-' or not provided, the template is read from standard input`, }, } - cmd.Flags().IntVar(&template.MigrateStages, "migrate-stages", 0, "Number of migration stages to run; use -1 for all available stages") + cmd.Flags().StringVar(&template.MigrateLevel, "migrate-level", "", "Name of the last migration to run (default: none)\n"+migrations.HelpText()) return cmd } diff --git a/cmd/opm/alpha/template/semver.go b/cmd/opm/alpha/template/semver.go index c25de901c..7caf93036 100644 --- a/cmd/opm/alpha/template/semver.go +++ b/cmd/opm/alpha/template/semver.go @@ -9,6 +9,7 @@ import ( "github.com/sirupsen/logrus" "github.com/spf13/cobra" + "github.com/operator-framework/operator-registry/alpha/action/migrations" "github.com/operator-framework/operator-registry/alpha/declcfg" "github.com/operator-framework/operator-registry/alpha/template/semver" "github.com/operator-framework/operator-registry/cmd/opm/internal/util" @@ -16,7 +17,7 @@ import ( func newSemverTemplateCmd() *cobra.Command { var ( - migrateStages int + migrateLevel string ) cmd := &cobra.Command{ @@ -86,7 +87,7 @@ When FILE is '-' or not provided, the template is read from standard input`, }, } - cmd.Flags().IntVar(&migrateStages, "migrate-stages", 0, "Number of migration stages to run; use -1 for all available stages") + cmd.Flags().StringVar(&migrateLevel, "migrate-level", "", "Name of the last migration to run (default: none)\n"+migrations.HelpText()) return cmd } diff --git a/cmd/opm/migrate/cmd.go b/cmd/opm/migrate/cmd.go index e88123723..95c665611 100644 --- a/cmd/opm/migrate/cmd.go +++ b/cmd/opm/migrate/cmd.go @@ -7,6 +7,7 @@ import ( "github.com/spf13/cobra" "github.com/operator-framework/operator-registry/alpha/action" + "github.com/operator-framework/operator-registry/alpha/action/migrations" "github.com/operator-framework/operator-registry/alpha/declcfg" "github.com/operator-framework/operator-registry/pkg/sqlite" ) @@ -51,6 +52,10 @@ parsers that assume that a file contains exactly one valid JSON object. }, } cmd.Flags().StringVarP(&output, "output", "o", "json", "Output format (json|yaml)") - cmd.Flags().IntVar(&migrate.Stages, "stages", -1, "Number of migration stages to run; use -1 for all available stages") + cmd.Flags().StringVar(&migrate.Level, "migrate-level", "", "Name of the last migration to run (default: none)\n"+migrations.HelpText()) + _ = cmd.RegisterFlagCompletionFunc("migrate-level", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { + return []string{migrations.HelpText()}, cobra.ShellCompDirectiveDefault + }) + return cmd } diff --git a/cmd/opm/render/cmd.go b/cmd/opm/render/cmd.go index b178496fd..915f8a304 100644 --- a/cmd/opm/render/cmd.go +++ b/cmd/opm/render/cmd.go @@ -10,6 +10,7 @@ import ( "github.com/spf13/cobra" "github.com/operator-framework/operator-registry/alpha/action" + "github.com/operator-framework/operator-registry/alpha/action/migrations" "github.com/operator-framework/operator-registry/alpha/declcfg" "github.com/operator-framework/operator-registry/cmd/opm/internal/util" "github.com/operator-framework/operator-registry/pkg/sqlite" @@ -67,7 +68,7 @@ database files. if deprecatedMigrateFlag { // If the deprecated --migrate flag is set, run all migration stages. - render.MigrateStages = -1 + render.MigrationLevel = "" } cfg, err := render.Run(cmd.Context()) @@ -82,10 +83,10 @@ database files. } cmd.Flags().StringVarP(&output, "output", "o", "json", "Output format of the streamed file-based catalog objects (json|yaml)") - cmd.Flags().IntVar(&render.MigrateStages, "migrate-stages", 0, "Number of migration stages to run; use -1 for all available stages") + cmd.Flags().StringVar(&render.MigrationLevel, "migrate-level", "", "Name of the last migration to run (default: none)\n"+migrations.HelpText()) cmd.Flags().BoolVar(&deprecatedMigrateFlag, "migrate", false, "Perform migrations on the rendered FBC") - cmd.Flags().MarkDeprecated("migrate", "use --migrate-stages instead") - cmd.MarkFlagsMutuallyExclusive("migrate", "migrate-stages") + cmd.Flags().MarkDeprecated("migrate", "use --migrate-level instead") + cmd.MarkFlagsMutuallyExclusive("migrate", "migrate-level") // Alpha flags cmd.Flags().StringVar(&imageRefTemplate, "alpha-image-ref-template", "", "When bundle image reference information is unavailable, populate it with this template")