diff --git a/staging/operator-registry/pkg/lib/registry/registry.go b/staging/operator-registry/pkg/lib/registry/registry.go index 4232e07b0d..74800e7283 100644 --- a/staging/operator-registry/pkg/lib/registry/registry.go +++ b/staging/operator-registry/pkg/lib/registry/registry.go @@ -212,7 +212,7 @@ func (r RegistryUpdater) DeleteFromRegistry(request DeleteFromRegistryRequest) e } defer db.Close() - dbLoader, err := sqlite.NewSQLLiteLoader(db) + dbLoader, err := sqlite.NewDeprecationAwareLoader(db) if err != nil { return err } diff --git a/staging/operator-registry/pkg/sqlite/load.go b/staging/operator-registry/pkg/sqlite/load.go index cfb3b7015a..65185ee656 100644 --- a/staging/operator-registry/pkg/sqlite/load.go +++ b/staging/operator-registry/pkg/sqlite/load.go @@ -28,7 +28,7 @@ type MigratableLoader interface { var _ MigratableLoader = &sqlLoader{} -func NewSQLLiteLoader(db *sql.DB, opts ...DbOption) (MigratableLoader, error) { +func newSQLLoader(db *sql.DB, opts ...DbOption) (*sqlLoader, error) { options := defaultDBOptions() for _, o := range opts { o(options) @@ -46,6 +46,10 @@ func NewSQLLiteLoader(db *sql.DB, opts ...DbOption) (MigratableLoader, error) { return &sqlLoader{db: db, migrator: migrator, enableAlpha: options.EnableAlpha}, nil } +func NewSQLLiteLoader(db *sql.DB, opts ...DbOption) (MigratableLoader, error) { + return newSQLLoader(db, opts...) +} + func (s *sqlLoader) Migrate(ctx context.Context) error { if s.migrator == nil { return fmt.Errorf("no migrator configured") @@ -1462,6 +1466,13 @@ func (s *sqlLoader) DeprecateBundle(path string) error { return err } + // Clean up the deprecated table by dropping all truncated bundles + // (see pkg/sqlite/migrations/013_rm_truncated_deprecations.go for more details) + _, err = tx.Exec(`DELETE FROM deprecated WHERE deprecated.operatorbundle_name NOT IN (SELECT DISTINCT deprecated.operatorbundle_name FROM (deprecated INNER JOIN channel_entry ON deprecated.operatorbundle_name = channel_entry.operatorbundle_name))`) + if err != nil { + return err + } + return tx.Commit() } @@ -1536,3 +1547,46 @@ func (s *sqlLoader) deprecated(tx *sql.Tx, name string) (bool, error) { // Ignore any deprecated bundles return err == nil, err } + +// DeprecationAwareLoader understands how bundle deprecations are handled in SQLite and decorates +// the sqlLoader with proxy methods that handle deprecation related table housekeeping. +type DeprecationAwareLoader struct { + *sqlLoader +} + +// NewDeprecationAwareLoader returns a new DeprecationAwareLoader. +func NewDeprecationAwareLoader(db *sql.DB, opts ...DbOption) (*DeprecationAwareLoader, error) { + loader, err := newSQLLoader(db, opts...) + if err != nil { + return nil, err + } + + return &DeprecationAwareLoader{sqlLoader: loader}, nil +} + +func (d *DeprecationAwareLoader) clearLastDeprecatedInPackage(pkg string) error { + tx, err := d.db.Begin() + if err != nil { + return err + } + defer func() { + tx.Rollback() + }() + + // The last deprecated bundles for a package will still have "tombstone" records in channel_entry (among other tables). + // Use that info to relate the package to a set of rows in the deprecated table. + _, err = tx.Exec(`DELETE FROM deprecated WHERE deprecated.operatorbundle_name IN (SELECT DISTINCT deprecated.operatorbundle_name FROM (deprecated INNER JOIN channel_entry ON deprecated.operatorbundle_name = channel_entry.operatorbundle_name) WHERE channel_entry.package_name = ?)`, pkg) + if err != nil { + return err + } + + return tx.Commit() +} + +func (d *DeprecationAwareLoader) RemovePackage(pkg string) error { + if err := d.clearLastDeprecatedInPackage(pkg); err != nil { + return err + } + + return d.sqlLoader.RemovePackage(pkg) +} diff --git a/staging/operator-registry/pkg/sqlite/load_test.go b/staging/operator-registry/pkg/sqlite/load_test.go index 9cde831aea..52c4a93d0a 100644 --- a/staging/operator-registry/pkg/sqlite/load_test.go +++ b/staging/operator-registry/pkg/sqlite/load_test.go @@ -264,6 +264,218 @@ func TestRMBundle(t *testing.T) { require.NoError(t, loader.rmBundle(tx, "non-existent")) } +func TestDeprecationAwareLoader(t *testing.T) { + withBundleImage := func(image string, bundle *registry.Bundle) *registry.Bundle { + bundle.BundleImage = image + return bundle + } + type fields struct { + bundles []*registry.Bundle + pkgs []registry.PackageManifest + deprecatedPaths []string + } + type args struct { + pkg string + } + type expected struct { + err error + deprecated map[string]struct{} + } + tests := []struct { + description string + fields fields + args args + expected expected + }{ + { + description: "NoDeprecation", + fields: fields{ + bundles: []*registry.Bundle{ + withBundleImage("quay.io/my/bundle-a", newBundle(t, "csv-a", "pkg-0", []string{"stable"}, newUnstructuredCSV(t, "csv-a", ""))), + }, + pkgs: []registry.PackageManifest{ + { + PackageName: "pkg-0", + Channels: []registry.PackageChannel{ + { + Name: "stable", + CurrentCSVName: "csv-a", + }, + }, + DefaultChannelName: "stable", + }, + }, + deprecatedPaths: []string{}, + }, + args: args{ + pkg: "pkg-0", + }, + expected: expected{ + err: nil, + deprecated: map[string]struct{}{}, + }, + }, + { + description: "RemovePackage/DropsDeprecated", + fields: fields{ + bundles: []*registry.Bundle{ + withBundleImage("quay.io/my/bundle-a", newBundle(t, "csv-a", "pkg-0", []string{"stable"}, newUnstructuredCSV(t, "csv-a", ""))), + withBundleImage("quay.io/my/bundle-aa", newBundle(t, "csv-aa", "pkg-0", []string{"stable"}, newUnstructuredCSV(t, "csv-aa", "csv-a"))), + }, + pkgs: []registry.PackageManifest{ + { + PackageName: "pkg-0", + Channels: []registry.PackageChannel{ + { + Name: "stable", + CurrentCSVName: "csv-aa", + }, + }, + DefaultChannelName: "stable", + }, + }, + deprecatedPaths: []string{ + "quay.io/my/bundle-a", + }, + }, + args: args{ + pkg: "pkg-0", + }, + expected: expected{ + err: nil, + deprecated: map[string]struct{}{}, + }, + }, + { + description: "RemovePackage/IgnoresOtherPackages", + fields: fields{ + bundles: []*registry.Bundle{ + withBundleImage("quay.io/my/bundle-a", newBundle(t, "csv-a", "pkg-0", []string{"stable"}, newUnstructuredCSV(t, "csv-a", ""))), + withBundleImage("quay.io/my/bundle-aa", newBundle(t, "csv-aa", "pkg-0", []string{"stable"}, newUnstructuredCSV(t, "csv-aa", "csv-a"))), + withBundleImage("quay.io/my/bundle-b", newBundle(t, "csv-b", "pkg-1", []string{"stable"}, newUnstructuredCSV(t, "csv-b", ""))), + withBundleImage("quay.io/my/bundle-bb", newBundle(t, "csv-bb", "pkg-1", []string{"stable"}, newUnstructuredCSV(t, "csv-bb", "csv-b"))), + }, + pkgs: []registry.PackageManifest{ + { + PackageName: "pkg-0", + Channels: []registry.PackageChannel{ + { + Name: "stable", + CurrentCSVName: "csv-aa", + }, + }, + DefaultChannelName: "stable", + }, + { + PackageName: "pkg-1", + Channels: []registry.PackageChannel{ + { + Name: "stable", + CurrentCSVName: "csv-bb", + }, + }, + DefaultChannelName: "stable", + }, + }, + deprecatedPaths: []string{ + "quay.io/my/bundle-a", + "quay.io/my/bundle-b", + }, + }, + args: args{ + pkg: "pkg-0", // Should result in a alone being dropped from the deprecated table + }, + expected: expected{ + err: nil, + deprecated: map[string]struct{}{ + "csv-b": struct{}{}, + }, + }, + }, + { + description: "DeprecateTruncate/DropsTruncated", + fields: fields{ + bundles: []*registry.Bundle{ + withBundleImage("quay.io/my/bundle-a", newBundle(t, "csv-a", "pkg-0", []string{"stable"}, newUnstructuredCSV(t, "csv-a", ""))), + withBundleImage("quay.io/my/bundle-aa", newBundle(t, "csv-aa", "pkg-0", []string{"stable"}, newUnstructuredCSV(t, "csv-aa", "csv-a"))), + withBundleImage("quay.io/my/bundle-aaa", newBundle(t, "csv-aaa", "pkg-0", []string{"stable"}, newUnstructuredCSV(t, "csv-aaa", "csv-aa"))), + }, + pkgs: []registry.PackageManifest{ + { + PackageName: "pkg-0", + Channels: []registry.PackageChannel{ + { + Name: "stable", + CurrentCSVName: "csv-aaa", + }, + }, + DefaultChannelName: "stable", + }, + }, + deprecatedPaths: []string{ + "quay.io/my/bundle-a", + "quay.io/my/bundle-aa", // Should truncate a, dropping it from the deprecated table + }, + }, + expected: expected{ + err: nil, + deprecated: map[string]struct{}{ + "csv-aa": struct{}{}, // csv-b remains in the deprecated table since it has been truncated and hasn't been removed + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.description, func(t *testing.T) { + db, cleanup := CreateTestDb(t) + defer cleanup() + store, err := NewDeprecationAwareLoader(db) + require.NoError(t, err) + err = store.Migrate(context.TODO()) + require.NoError(t, err) + + for _, bundle := range tt.fields.bundles { + require.NoError(t, store.AddOperatorBundle(bundle)) + } + + for _, pkg := range tt.fields.pkgs { + require.NoError(t, store.AddPackageChannels(pkg)) + } + + for _, deprecatedPath := range tt.fields.deprecatedPaths { + require.NoError(t, store.DeprecateBundle(deprecatedPath)) + } + + if tt.args.pkg != "" { + err = store.RemovePackage(tt.args.pkg) + if tt.expected.err != nil { + require.Error(t, err) + } else { + require.NoError(t, err) + } + } + + tx, err := db.Begin() + require.NoError(t, err) + + rows, err := tx.Query(`SELECT operatorbundle_name FROM deprecated`) + require.NoError(t, err) + require.NotNil(t, rows) + + var bundleName string + for rows.Next() { + require.NoError(t, rows.Scan(&bundleName)) + _, ok := tt.expected.deprecated[bundleName] + require.True(t, ok, "bundle shouldn't be in the deprecated table: %s", bundleName) + delete(tt.expected.deprecated, bundleName) + } + + require.Len(t, tt.expected.deprecated, 0, "not all expected bundles exist in deprecated table: %v", tt.expected.deprecated) + }) + } +} + func TestGetTailFromBundle(t *testing.T) { type fields struct { bundles []*registry.Bundle diff --git a/staging/operator-registry/pkg/sqlite/migrations/013_rm_truncated_deprecations.go b/staging/operator-registry/pkg/sqlite/migrations/013_rm_truncated_deprecations.go new file mode 100644 index 0000000000..33ac71b765 --- /dev/null +++ b/staging/operator-registry/pkg/sqlite/migrations/013_rm_truncated_deprecations.go @@ -0,0 +1,31 @@ +package migrations + +import ( + "context" + "database/sql" +) + +const RmTruncatedDeprecationsMigrationKey = 13 + +// Register this migration +func init() { + registerMigration(RmTruncatedDeprecationsMigrationKey, rmTruncatedDeprecationsMigration) +} + +var rmTruncatedDeprecationsMigration = &Migration{ + Id: RmTruncatedDeprecationsMigrationKey, + Up: func(ctx context.Context, tx *sql.Tx) error { + + // Delete deprecation history for all bundles that no longer exist in the channel_entries table + // These bundles have been truncated by more recent deprecations and would only confuse future operations on an index; + // e.g. adding a previously truncated bundle to a package removed via `opm index|registry rm` would lead to that bundle + // being deprecated + _, err := tx.ExecContext(ctx, `DELETE FROM deprecated WHERE deprecated.operatorbundle_name NOT IN (SELECT DISTINCT deprecated.operatorbundle_name FROM (deprecated INNER JOIN channel_entry ON deprecated.operatorbundle_name = channel_entry.operatorbundle_name))`) + + return err + }, + Down: func(ctx context.Context, tx *sql.Tx) error { + // No-op + return nil + }, +} diff --git a/staging/operator-registry/pkg/sqlite/migrations/013_rm_truncated_deprecations_test.go b/staging/operator-registry/pkg/sqlite/migrations/013_rm_truncated_deprecations_test.go new file mode 100644 index 0000000000..8977d73cab --- /dev/null +++ b/staging/operator-registry/pkg/sqlite/migrations/013_rm_truncated_deprecations_test.go @@ -0,0 +1,49 @@ +package migrations_test + +import ( + "context" + "database/sql" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/operator-framework/operator-registry/pkg/sqlite/migrations" +) + +func TestRmTruncatedDeprecations(t *testing.T) { + db, migrator, cleanup := CreateTestDbAt(t, migrations.RmTruncatedDeprecationsMigrationKey-1) + defer cleanup() + + // Insert fixtures to satisfy foreign key constraints + insertBundle := "INSERT INTO operatorbundle(name, version, bundlepath, csv) VALUES (?, ?, ?, ?)" + insertChannel := "INSERT INTO channel(name, package_name, head_operatorbundle_name) VALUES (?, ?, ?)" + insertChannelEntry := "INSERT INTO channel_entry(entry_id, channel_name, package_name, operatorbundle_name) VALUES (?, ?, ?, ?)" + insertDeprecated := "INSERT INTO deprecated(operatorbundle_name) VALUES (?)" + + // Add a deprecated bundle + _, err := db.Exec(insertBundle, "operator.v1.0.0", "1.0.0", "quay.io/operator:v1.0.0", "operator.v1.0.0's csv") + require.NoError(t, err) + _, err = db.Exec(insertChannel, "stable", "apple", "operator.v1.0.0") + require.NoError(t, err) + _, err = db.Exec(insertChannelEntry, 0, "stable", "apple", "operator.v1.0.0") + require.NoError(t, err) + _, err = db.Exec(insertDeprecated, "operator.v1.0.0") + require.NoError(t, err) + + // Add a truncated bundle; i.e. doesn't exist in the channel_entry table + _, err = db.Exec(insertDeprecated, "operator.v1.0.0-pre") + + // This migration should delete all bundles that are not referenced by the channel_entry table + require.NoError(t, migrator.Up(context.Background(), migrations.Only(migrations.RmTruncatedDeprecationsMigrationKey))) + + deprecated, err := db.Query("SELECT * FROM deprecated") + require.NoError(t, err) + defer deprecated.Close() + + require.True(t, deprecated.Next(), "failed to detect deprecated bundle") + var name sql.NullString + require.NoError(t, deprecated.Scan(&name)) + require.True(t, name.Valid) + require.Equal(t, "operator.v1.0.0", name.String) + require.False(t, deprecated.Next(), "incorrect number of deprecated bundles") +} diff --git a/vendor/github.com/operator-framework/operator-registry/pkg/lib/registry/registry.go b/vendor/github.com/operator-framework/operator-registry/pkg/lib/registry/registry.go index 4232e07b0d..74800e7283 100644 --- a/vendor/github.com/operator-framework/operator-registry/pkg/lib/registry/registry.go +++ b/vendor/github.com/operator-framework/operator-registry/pkg/lib/registry/registry.go @@ -212,7 +212,7 @@ func (r RegistryUpdater) DeleteFromRegistry(request DeleteFromRegistryRequest) e } defer db.Close() - dbLoader, err := sqlite.NewSQLLiteLoader(db) + dbLoader, err := sqlite.NewDeprecationAwareLoader(db) if err != nil { return err } diff --git a/vendor/github.com/operator-framework/operator-registry/pkg/sqlite/load.go b/vendor/github.com/operator-framework/operator-registry/pkg/sqlite/load.go index cfb3b7015a..65185ee656 100644 --- a/vendor/github.com/operator-framework/operator-registry/pkg/sqlite/load.go +++ b/vendor/github.com/operator-framework/operator-registry/pkg/sqlite/load.go @@ -28,7 +28,7 @@ type MigratableLoader interface { var _ MigratableLoader = &sqlLoader{} -func NewSQLLiteLoader(db *sql.DB, opts ...DbOption) (MigratableLoader, error) { +func newSQLLoader(db *sql.DB, opts ...DbOption) (*sqlLoader, error) { options := defaultDBOptions() for _, o := range opts { o(options) @@ -46,6 +46,10 @@ func NewSQLLiteLoader(db *sql.DB, opts ...DbOption) (MigratableLoader, error) { return &sqlLoader{db: db, migrator: migrator, enableAlpha: options.EnableAlpha}, nil } +func NewSQLLiteLoader(db *sql.DB, opts ...DbOption) (MigratableLoader, error) { + return newSQLLoader(db, opts...) +} + func (s *sqlLoader) Migrate(ctx context.Context) error { if s.migrator == nil { return fmt.Errorf("no migrator configured") @@ -1462,6 +1466,13 @@ func (s *sqlLoader) DeprecateBundle(path string) error { return err } + // Clean up the deprecated table by dropping all truncated bundles + // (see pkg/sqlite/migrations/013_rm_truncated_deprecations.go for more details) + _, err = tx.Exec(`DELETE FROM deprecated WHERE deprecated.operatorbundle_name NOT IN (SELECT DISTINCT deprecated.operatorbundle_name FROM (deprecated INNER JOIN channel_entry ON deprecated.operatorbundle_name = channel_entry.operatorbundle_name))`) + if err != nil { + return err + } + return tx.Commit() } @@ -1536,3 +1547,46 @@ func (s *sqlLoader) deprecated(tx *sql.Tx, name string) (bool, error) { // Ignore any deprecated bundles return err == nil, err } + +// DeprecationAwareLoader understands how bundle deprecations are handled in SQLite and decorates +// the sqlLoader with proxy methods that handle deprecation related table housekeeping. +type DeprecationAwareLoader struct { + *sqlLoader +} + +// NewDeprecationAwareLoader returns a new DeprecationAwareLoader. +func NewDeprecationAwareLoader(db *sql.DB, opts ...DbOption) (*DeprecationAwareLoader, error) { + loader, err := newSQLLoader(db, opts...) + if err != nil { + return nil, err + } + + return &DeprecationAwareLoader{sqlLoader: loader}, nil +} + +func (d *DeprecationAwareLoader) clearLastDeprecatedInPackage(pkg string) error { + tx, err := d.db.Begin() + if err != nil { + return err + } + defer func() { + tx.Rollback() + }() + + // The last deprecated bundles for a package will still have "tombstone" records in channel_entry (among other tables). + // Use that info to relate the package to a set of rows in the deprecated table. + _, err = tx.Exec(`DELETE FROM deprecated WHERE deprecated.operatorbundle_name IN (SELECT DISTINCT deprecated.operatorbundle_name FROM (deprecated INNER JOIN channel_entry ON deprecated.operatorbundle_name = channel_entry.operatorbundle_name) WHERE channel_entry.package_name = ?)`, pkg) + if err != nil { + return err + } + + return tx.Commit() +} + +func (d *DeprecationAwareLoader) RemovePackage(pkg string) error { + if err := d.clearLastDeprecatedInPackage(pkg); err != nil { + return err + } + + return d.sqlLoader.RemovePackage(pkg) +} diff --git a/vendor/github.com/operator-framework/operator-registry/pkg/sqlite/migrations/013_rm_truncated_deprecations.go b/vendor/github.com/operator-framework/operator-registry/pkg/sqlite/migrations/013_rm_truncated_deprecations.go new file mode 100644 index 0000000000..33ac71b765 --- /dev/null +++ b/vendor/github.com/operator-framework/operator-registry/pkg/sqlite/migrations/013_rm_truncated_deprecations.go @@ -0,0 +1,31 @@ +package migrations + +import ( + "context" + "database/sql" +) + +const RmTruncatedDeprecationsMigrationKey = 13 + +// Register this migration +func init() { + registerMigration(RmTruncatedDeprecationsMigrationKey, rmTruncatedDeprecationsMigration) +} + +var rmTruncatedDeprecationsMigration = &Migration{ + Id: RmTruncatedDeprecationsMigrationKey, + Up: func(ctx context.Context, tx *sql.Tx) error { + + // Delete deprecation history for all bundles that no longer exist in the channel_entries table + // These bundles have been truncated by more recent deprecations and would only confuse future operations on an index; + // e.g. adding a previously truncated bundle to a package removed via `opm index|registry rm` would lead to that bundle + // being deprecated + _, err := tx.ExecContext(ctx, `DELETE FROM deprecated WHERE deprecated.operatorbundle_name NOT IN (SELECT DISTINCT deprecated.operatorbundle_name FROM (deprecated INNER JOIN channel_entry ON deprecated.operatorbundle_name = channel_entry.operatorbundle_name))`) + + return err + }, + Down: func(ctx context.Context, tx *sql.Tx) error { + // No-op + return nil + }, +}