diff --git a/api/bolt/migrator/migrate_ce.go b/api/bolt/migrator/migrate_ce.go index 7f2b70586..26f751ae1 100644 --- a/api/bolt/migrator/migrate_ce.go +++ b/api/bolt/migrator/migrate_ce.go @@ -301,7 +301,7 @@ func (m *Migrator) Migrate() error { } } - // Portainer 2.9.1 + // Portainer 2.9.1, 2.9.2 if m.currentDBVersion < 33 { err := m.migrateDBVersionToDB33() if err != nil { @@ -316,6 +316,13 @@ func (m *Migrator) Migrate() error { } } + // Portainer 2.9.3 (yep out of order, but 2.10 is EE only) + if m.currentDBVersion < 35 { + if err := m.migrateDBVersionToDB35(); err != nil { + return migrationError(err, "migrateDBVersionToDB35") + } + } + err = m.versionService.StoreDBVersion(portainer.DBVersion) if err != nil { return migrationError(err, "StoreDBVersion") diff --git a/api/bolt/migrator/migrate_dbversion31.go b/api/bolt/migrator/migrate_dbversion31.go index 5993f0513..af80ce25f 100644 --- a/api/bolt/migrator/migrate_dbversion31.go +++ b/api/bolt/migrator/migrate_dbversion31.go @@ -100,6 +100,32 @@ func (m *Migrator) updateDockerhubToDB32() error { RegistryAccesses: portainer.RegistryAccesses{}, } + // The following code will make this function idempotent. + // i.e. if run again, it will not change the data. It will ensure that + // we only have one migrated registry entry. Duplicates will be removed + // if they exist and which has been happening due to earlier migration bugs + migrated := false + registries, _ := m.registryService.Registries() + for _, r := range registries { + if r.Type == registry.Type && + r.Name == registry.Name && + r.URL == registry.URL && + r.Authentication == registry.Authentication { + + if !migrated { + // keep this one entry + migrated = true + } else { + // delete subsequent duplicates + m.registryService.DeleteRegistry(portainer.RegistryID(r.ID)) + } + } + } + + if migrated { + return nil + } + endpoints, err := m.endpointService.Endpoints() if err != nil { return err diff --git a/api/bolt/migrator/migrate_dbversion34.go b/api/bolt/migrator/migrate_dbversion34.go new file mode 100644 index 000000000..78a785051 --- /dev/null +++ b/api/bolt/migrator/migrate_dbversion34.go @@ -0,0 +1,11 @@ +package migrator + +func (m *Migrator) migrateDBVersionToDB35() error { + // These should have been migrated already, but due to an earlier bug and a bunch of duplicates, + // calling it again will now fix the issue as the function has been repaired. + err := m.updateDockerhubToDB32() + if err != nil { + return err + } + return nil +} diff --git a/api/bolt/migrator/migrate_dbversion34_test.go b/api/bolt/migrator/migrate_dbversion34_test.go new file mode 100644 index 000000000..59819ee42 --- /dev/null +++ b/api/bolt/migrator/migrate_dbversion34_test.go @@ -0,0 +1,108 @@ +package migrator + +import ( + "os" + "path" + "testing" + "time" + + "github.com/boltdb/bolt" + portainer "github.com/portainer/portainer/api" + "github.com/portainer/portainer/api/bolt/dockerhub" + "github.com/portainer/portainer/api/bolt/endpoint" + "github.com/portainer/portainer/api/bolt/internal" + "github.com/portainer/portainer/api/bolt/registry" + "github.com/stretchr/testify/assert" +) + +const ( + db35TestFile = "portainer-mig-35.db" + username = "portainer" + password = "password" +) + +func setupDB35Test(t *testing.T) *Migrator { + is := assert.New(t) + dbConn, err := bolt.Open(path.Join(t.TempDir(), db35TestFile), 0600, &bolt.Options{Timeout: 1 * time.Second}) + is.NoError(err, "failed to init testing DB connection") + + // Create an old style dockerhub authenticated account + dockerhubService, err := dockerhub.NewService(&internal.DbConnection{DB: dbConn}) + is.NoError(err, "failed to init testing registry service") + err = dockerhubService.UpdateDockerHub(&portainer.DockerHub{true, username, password}) + is.NoError(err, "failed to create dockerhub account") + + registryService, err := registry.NewService(&internal.DbConnection{DB: dbConn}) + is.NoError(err, "failed to init testing registry service") + + endpointService, err := endpoint.NewService(&internal.DbConnection{DB: dbConn}) + is.NoError(err, "failed to init endpoint service") + + m := &Migrator{ + db: dbConn, + dockerhubService: dockerhubService, + registryService: registryService, + endpointService: endpointService, + } + + return m +} + +// TestUpdateDockerhubToDB32 tests a normal upgrade +func TestUpdateDockerhubToDB32(t *testing.T) { + is := assert.New(t) + m := setupDB35Test(t) + defer m.db.Close() + defer os.Remove(db35TestFile) + + if err := m.updateDockerhubToDB32(); err != nil { + t.Errorf("failed to update settings: %v", err) + } + + // Verify we have a single registry were created + registries, err := m.registryService.Registries() + is.NoError(err, "failed to read registries from the RegistryService") + is.Equal(len(registries), 1, "only one migrated registry expected") +} + +// TestUpdateDockerhubToDB32_with_duplicate_migrations tests an upgrade where in earlier versions a broken migration +// created a large number of duplicate "dockerhub migrated" registry entries. +func TestUpdateDockerhubToDB32_with_duplicate_migrations(t *testing.T) { + is := assert.New(t) + m := setupDB35Test(t) + defer m.db.Close() + defer os.Remove(db35TestFile) + + // Create lots of duplicate entries... + registry := &portainer.Registry{ + Type: portainer.DockerHubRegistry, + Name: "Dockerhub (authenticated - migrated)", + URL: "docker.io", + Authentication: true, + Username: "portainer", + Password: "password", + RegistryAccesses: portainer.RegistryAccesses{}, + } + + for i := 1; i < 150; i++ { + err := m.registryService.CreateRegistry(registry) + assert.NoError(t, err, "create registry failed") + } + + // Verify they were created + registries, err := m.registryService.Registries() + is.NoError(err, "failed to read registries from the RegistryService") + is.Condition(func() bool { + return len(registries) > 1 + }, "expected multiple duplicate registry entries") + + // Now run the migrator + if err := m.updateDockerhubToDB32(); err != nil { + t.Errorf("failed to update settings: %v", err) + } + + // Verify we have a single registry were created + registries, err = m.registryService.Registries() + is.NoError(err, "failed to read registries from the RegistryService") + is.Equal(len(registries), 1, "only one migrated registry expected") +} diff --git a/api/portainer.go b/api/portainer.go index 7c0b908c1..b37e8ab7d 100644 --- a/api/portainer.go +++ b/api/portainer.go @@ -1478,7 +1478,7 @@ const ( // APIVersion is the version number of the Portainer API APIVersion = "2.9.3" // DBVersion is the version number of the Portainer database - DBVersion = 33 + DBVersion = 35 // ComposeSyntaxMaxVersion is a maximum supported version of the docker compose syntax ComposeSyntaxMaxVersion = "3.9" // AssetsServerURL represents the URL of the Portainer asset server