Skip to content

Commit

Permalink
use a type to protect migration interactions
Browse files Browse the repository at this point in the history 
Signed-off-by: Jordan Keister <jordan@nimblewidget.com>
  • Loading branch information
@grokspawn
grokspawn committed Aug 7, 2024
1 parent 9e68d1c commit 6aed213
Show file tree
Hide file tree
Showing 12 changed files with 164 additions and 111 deletions.
7 changes: 4 additions & 3 deletions alpha/action/migrate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down
63 changes: 30 additions & 33 deletions alpha/action/migrations/000_bundle_object_to_csv_metadata.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
64 changes: 44 additions & 20 deletions alpha/action/migrations/migrations.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,33 @@ 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
name string
help string
fn func(*declcfg.DeclarativeConfig) error
}

func (s simpleMigration) Token() MigrationToken {
return s.token
}

func (s simpleMigration) Name() string {
Expand All @@ -41,22 +54,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)
Expand All @@ -65,28 +89,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()
Expand Down
20 changes: 7 additions & 13 deletions alpha/action/render.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ type Render struct {
Refs []string
Registry image.Registry
AllowedRefMask RefType
MigrationLevel string
ImageRefTemplate *template.Template
Migrations *migrations.Migrations

skipSqliteDeprecationLog bool
}
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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 {
Expand Down
51 changes: 27 additions & 24 deletions alpha/action/render_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"k8s.io/apimachinery/pkg/util/yaml"

"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/alpha/property"
"github.com/operator-framework/operator-registry/pkg/containertools"
Expand All @@ -29,9 +30,6 @@ import (
"github.com/operator-framework/operator-registry/pkg/sqlite"
)

// aligns with supported migrations in alpha.action.migrations
const migrationLevelCSVMetadata = "bundle-object-to-csv-metadata"

func TestRender(t *testing.T) {
type spec struct {
name string
Expand Down Expand Up @@ -75,13 +73,18 @@ func TestRender(t *testing.T) {
image.SimpleReference("test.registry/foo-operator/foo-bundle:v0.2.0"): "testdata/foo-bundle-v0.2.0",
}
assert.NoError(t, generateSqliteFile(dbFile, imageMap))
allMigrations, err := migrations.NewMigrations(migrations.AllMigrations)
require.NoError(t, err)
noMigrations, err := migrations.NewMigrations(migrations.NoMigrations)
require.NoError(t, err)

specs := []spec{
{
name: "Success/SqliteIndexImage",
render: action.Render{
Refs: []string{"test.registry/foo-operator/foo-index-sqlite:v0.2.0"},
Registry: reg,
Refs: []string{"test.registry/foo-operator/foo-index-sqlite:v0.2.0"},
Registry: reg,
Migrations: noMigrations,
},
expectCfg: &declcfg.DeclarativeConfig{
Packages: []declcfg.Package{
Expand Down Expand Up @@ -172,9 +175,9 @@ func TestRender(t *testing.T) {
{
name: "Success/SqliteIndexImageCSVMigration",
render: action.Render{
Refs: []string{"test.registry/foo-operator/foo-index-sqlite:v0.2.0"},
Registry: reg,
MigrationLevel: migrationLevelCSVMetadata,
Refs: []string{"test.registry/foo-operator/foo-index-sqlite:v0.2.0"},
Registry: reg,
Migrations: allMigrations,
},
expectCfg: &declcfg.DeclarativeConfig{
Packages: []declcfg.Package{
Expand Down Expand Up @@ -355,9 +358,9 @@ func TestRender(t *testing.T) {
{
name: "Success/SqliteFileMigration",
render: action.Render{
Refs: []string{dbFile},
Registry: reg,
MigrationLevel: migrationLevelCSVMetadata,
Refs: []string{dbFile},
Registry: reg,
Migrations: allMigrations,
},
expectCfg: &declcfg.DeclarativeConfig{
Packages: []declcfg.Package{
Expand Down Expand Up @@ -642,9 +645,9 @@ func TestRender(t *testing.T) {
{
name: "Success/DeclcfgImageMigrate",
render: action.Render{
Refs: []string{"test.registry/foo-operator/foo-index-declcfg:v0.2.0"},
MigrationLevel: migrationLevelCSVMetadata,
Registry: reg,
Refs: []string{"test.registry/foo-operator/foo-index-declcfg:v0.2.0"},
Registry: reg,
Migrations: allMigrations,
},
expectCfg: &declcfg.DeclarativeConfig{
Packages: []declcfg.Package{
Expand Down Expand Up @@ -739,9 +742,9 @@ func TestRender(t *testing.T) {
{
name: "Success/DeclcfgDirectoryMigrate",
render: action.Render{
Refs: []string{"testdata/foo-index-v0.2.0-declcfg"},
MigrationLevel: migrationLevelCSVMetadata,
Registry: reg,
Refs: []string{"testdata/foo-index-v0.2.0-declcfg"},
Registry: reg,
Migrations: allMigrations,
},
expectCfg: &declcfg.DeclarativeConfig{
Packages: []declcfg.Package{
Expand Down Expand Up @@ -886,9 +889,9 @@ func TestRender(t *testing.T) {
{
name: "Success/BundleImageMigration",
render: action.Render{
Refs: []string{"test.registry/foo-operator/foo-bundle:v0.2.0"},
Registry: reg,
MigrationLevel: migrationLevelCSVMetadata,
Refs: []string{"test.registry/foo-operator/foo-bundle:v0.2.0"},
Registry: reg,
Migrations: allMigrations,
},
expectCfg: &declcfg.DeclarativeConfig{
Bundles: []declcfg.Bundle{
Expand Down Expand Up @@ -981,9 +984,9 @@ func TestRender(t *testing.T) {
{
name: "Success/BundleImageWithNoCSVRelatedImagesMigration",
render: action.Render{
Refs: []string{"test.registry/foo-operator/foo-bundle-no-csv-related-images:v0.2.0"},
Registry: reg,
MigrationLevel: migrationLevelCSVMetadata,
Refs: []string{"test.registry/foo-operator/foo-bundle-no-csv-related-images:v0.2.0"},
Registry: reg,
Migrations: allMigrations,
},
expectCfg: &declcfg.DeclarativeConfig{
Bundles: []declcfg.Bundle{
Expand Down Expand Up @@ -1077,7 +1080,7 @@ func TestRender(t *testing.T) {
Refs: []string{"testdata/foo-bundle-v0.2.0"},
ImageRefTemplate: template.Must(template.New("imageRef").Parse("test.registry/{{.Package}}-operator/{{.Package}}:v{{.Version}}")),
Registry: reg,
MigrationLevel: migrationLevelCSVMetadata,
Migrations: allMigrations,
},
expectCfg: &declcfg.DeclarativeConfig{
Bundles: []declcfg.Bundle{
Expand Down
Loading

0 comments on commit 6aed213

Please sign in to comment.