diff --git a/pkg/services/sqlstore/migrations/user_mig.go b/pkg/services/sqlstore/migrations/user_mig.go index d2c9d319c95..68fe8ba9af4 100644 --- a/pkg/services/sqlstore/migrations/user_mig.go +++ b/pkg/services/sqlstore/migrations/user_mig.go @@ -160,8 +160,7 @@ func addUserMigrations(mg *Migrator) { mg.AddMigration(usermig.AllowSameLoginCrossOrgs, &usermig.ServiceAccountsSameLoginCrossOrgs{}) // Before it was fixed, the previous migration introduced the org_id again in logins that already had it. // This migration removes the duplicate org_id from the login. - // TODO(aarongodin): this migration was causing instances to fail to start. It must be corrected before being added - // mg.AddMigration(usermig.DedupOrgInLogin, &usermig.ServiceAccountsDeduplicateOrgInLogin{}) + mg.AddMigration(usermig.DedupOrgInLogin, &usermig.ServiceAccountsDeduplicateOrgInLogin{}) // Users login and email should be in lower case mg.AddMigration(usermig.LowerCaseUserLoginAndEmail, &usermig.UsersLowerCaseLoginAndEmail{}) diff --git a/pkg/services/sqlstore/migrations/usermig/service_account_multiple_org_login_migrator.go b/pkg/services/sqlstore/migrations/usermig/service_account_multiple_org_login_migrator.go index 54b71250f1b..05436292267 100644 --- a/pkg/services/sqlstore/migrations/usermig/service_account_multiple_org_login_migrator.go +++ b/pkg/services/sqlstore/migrations/usermig/service_account_multiple_org_login_migrator.go @@ -94,27 +94,39 @@ func (p *ServiceAccountsDeduplicateOrgInLogin) Exec(sess *xorm.Session, mg *migr switch dialect.DriverName() { case migrator.Postgres: _, err = sess.Exec(` - UPDATE "user" + UPDATE "user" AS u SET login = 'sa-' || org_id::text || SUBSTRING(login FROM LENGTH('sa-' || org_id::text || '-' || org_id::text)+1) WHERE login IS NOT NULL AND is_service_account = true - AND login LIKE 'sa-' || org_id::text || '-' || org_id::text || '-%'; + AND login LIKE 'sa-' || org_id::text || '-' || org_id::text || '-%' + AND NOT EXISTS ( + SELECT 1 + FROM "user" AS u2 + WHERE u2.login = 'sa-' || u.org_id::text || SUBSTRING(u.login FROM LENGTH('sa-' || u.org_id::text || '-' || u.org_id::text)+1) + );; `) case migrator.MySQL: _, err = sess.Exec(` - UPDATE user - SET login = CONCAT('sa-', org_id, SUBSTRING(login, LENGTH(CONCAT('sa-', org_id, '-', org_id))+1)) - WHERE login IS NOT NULL - AND is_service_account = 1 - AND login LIKE CONCAT('sa-', org_id, '-', org_id, '-%'); + UPDATE user AS u + LEFT JOIN user AS u2 ON u2.login = CONCAT('sa-', u.org_id, SUBSTRING(u.login, LENGTH(CONCAT('sa-', u.org_id, '-', u.org_id))+1)) + SET u.login = CONCAT('sa-', u.org_id, SUBSTRING(u.login, LENGTH(CONCAT('sa-', u.org_id, '-', u.org_id))+1)) + WHERE u.login IS NOT NULL + AND u.is_service_account = 1 + AND u.login LIKE CONCAT('sa-', u.org_id, '-', u.org_id, '-%') + AND u2.login IS NULL; `) case migrator.SQLite: _, err = sess.Exec(` - UPDATE ` + dialect.Quote("user") + ` - SET login = 'sa-' || CAST(org_id AS TEXT) || SUBSTRING(login, LENGTH('sa-'||CAST(org_id AS TEXT)||'-'||CAST(org_id AS TEXT))+1) - WHERE login IS NOT NULL - AND is_service_account = 1 - AND login LIKE 'sa-'||CAST(org_id AS TEXT)||'-'||CAST(org_id AS TEXT)||'-%'; + UPDATE ` + dialect.Quote("user") + ` AS u + SET login = 'sa-' || CAST(u.org_id AS TEXT) || SUBSTRING(u.login, LENGTH('sa-'||CAST(u.org_id AS TEXT)||'-'||CAST(u.org_id AS TEXT))+1) + WHERE u.login IS NOT NULL + AND u.is_service_account = 1 + AND u.login LIKE 'sa-'||CAST(u.org_id AS TEXT)||'-'||CAST(u.org_id AS TEXT)||'-%' + AND NOT EXISTS ( + SELECT 1 + FROM ` + dialect.Quote("user") + `AS u2 + WHERE u2.login = 'sa-' || CAST(u.org_id AS TEXT) || SUBSTRING(u.login, LENGTH('sa-'||CAST(u.org_id AS TEXT)||'-'||CAST(u.org_id AS TEXT))+1) + );; `) default: return fmt.Errorf("dialect not supported: %s", dialect) diff --git a/pkg/services/sqlstore/migrations/usermig/test/service_account_test.go b/pkg/services/sqlstore/migrations/usermig/test/service_account_test.go index 7ef8b475855..7b289e451bc 100644 --- a/pkg/services/sqlstore/migrations/usermig/test/service_account_test.go +++ b/pkg/services/sqlstore/migrations/usermig/test/service_account_test.go @@ -373,6 +373,43 @@ func TestIntegrationServiceAccountDedupOrgMigration(t *testing.T) { }, }, }, + { + desc: "handle conflicts", + serviceAccounts: []*user.User{ + { + ID: 5, + UID: "u5", + Name: "sa-2-conflict", + Login: "sa-2-conflict", + Email: "sa-2-conflict@example.org", + OrgID: 2, + Created: now, + Updated: now, + IsServiceAccount: true, + }, + { + ID: 6, + UID: "u6", + Name: "sa-2b-conflict", + Login: "sa-2-2-conflict", + Email: "sa-2b-conflict@example.org", + OrgID: 2, + Created: now, + Updated: now, + IsServiceAccount: true, + }, + }, + wantServiceAccounts: []*user.User{ + { + ID: 5, + Login: "sa-2-conflict", + }, + { + ID: 6, + Login: "sa-2-2-conflict", + }, + }, + }, } for _, tc := range testCases {