From 7ece6b35f067bb895d9178f3ff04584ae576010e Mon Sep 17 00:00:00 2001
From: "grafana-delivery-bot[bot]"
 <132647405+grafana-delivery-bot[bot]@users.noreply.github.com>
Date: Thu, 10 Oct 2024 11:35:41 +0200
Subject: [PATCH] [v11.1.x] Fix: Ensure SA migrations doesn't hit migrated SAs
 if rerun (#94398)

Fix: Ensure SA migrations doesn't hit migrated SAs if rerun (#94347)

* ensure mig doesn't hit migrated SAs if rerun

* Fix small issue with the test

---------

Co-authored-by: gamab <gabriel.mabille@grafana.com>
(cherry picked from commit 945dd052b1765532c64d7c81df2035d9307eaf88)

Co-authored-by: Jo <joao.guerreiro@grafana.com>
---
 ...ice_account_multiple_org_login_migrator.go | 58 +++++++++++--------
 .../usermig/test/service_account_test.go      | 40 +++++++++++++
 2 files changed, 74 insertions(+), 24 deletions(-)

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 1d60a4f7f28..d51cfacd5a6 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
@@ -35,32 +35,42 @@ func (p *ServiceAccountsSameLoginCrossOrgs) Exec(sess *xorm.Session, mg *migrato
 	var err error
 	switch p.dialect.DriverName() {
 	case migrator.Postgres:
-		_, err = p.sess.Exec(`UPDATE "user"
-		SET login = 'sa-' || org_id::text || '-' || 
-		CASE 
-		  WHEN login LIKE 'sa-%' THEN SUBSTRING(login FROM 4) 
-		  ELSE login 
-		END 
-		WHERE login IS NOT NULL AND is_service_account = true;`,
-		)
+		_, err = p.sess.Exec(`
+            UPDATE "user"
+            SET login = 'sa-' || org_id::text || '-' || 
+                CASE 
+                    WHEN login LIKE 'sa-%' THEN SUBSTRING(login FROM 4) 
+                    ELSE login 
+                END 
+            WHERE login IS NOT NULL 
+              AND is_service_account = true
+              AND login NOT LIKE 'sa-' || org_id::text || '-%';
+        `)
 	case migrator.MySQL:
-		_, err = p.sess.Exec(`UPDATE user
-		SET login = CONCAT('sa-', CAST(org_id AS CHAR), '-',
-		CASE
-		  WHEN login LIKE 'sa-%' THEN SUBSTRING(login, 4)
-		  ELSE login
-		END)
-		WHERE login IS NOT NULL AND is_service_account = 1;`,
-		)
+		_, err = p.sess.Exec(`
+            UPDATE user
+            SET login = CONCAT('sa-', CAST(org_id AS CHAR), '-',
+                CASE
+                    WHEN login LIKE 'sa-%' THEN SUBSTRING(login, 4)
+                    ELSE login
+                END
+            )
+            WHERE login IS NOT NULL 
+              AND is_service_account = 1
+              AND login NOT LIKE CONCAT('sa-', org_id, '-%');
+        `)
 	case migrator.SQLite:
-		_, err = p.sess.Exec(`Update ` + p.dialect.Quote("user") + `
-		SET login = 'sa-' || CAST(org_id AS TEXT) || '-' ||
-		CASE
-			WHEN SUBSTR(login, 1, 3) = 'sa-' THEN SUBSTR(login, 4)
-			ELSE login
-		END
-		WHERE login IS NOT NULL AND is_service_account = 1;`,
-		)
+		_, err = p.sess.Exec(`
+            UPDATE ` + p.dialect.Quote("user") + `
+            SET login = 'sa-' || CAST(org_id AS TEXT) || '-' ||
+                CASE
+                    WHEN SUBSTR(login, 1, 3) = 'sa-' THEN SUBSTR(login, 4)
+                    ELSE login
+                END
+            WHERE login IS NOT NULL 
+              AND is_service_account = 1
+              AND login NOT LIKE 'sa-' || CAST(org_id AS TEXT) || '-%';
+        `)
 	default:
 		return fmt.Errorf("dialect not supported: %s", p.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 043c9852245..17330453cce 100644
--- a/pkg/services/sqlstore/migrations/usermig/test/service_account_test.go
+++ b/pkg/services/sqlstore/migrations/usermig/test/service_account_test.go
@@ -15,6 +15,9 @@ import (
 )
 
 func TestIntegrationServiceAccountMigration(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test in short mode")
+	}
 	// Run initial migration to have a working DB
 	x := setupTestDB(t)
 
@@ -211,6 +214,43 @@ func TestIntegrationServiceAccountMigration(t *testing.T) {
 				},
 			},
 		},
+		{
+			desc: "avoid reapply of migration",
+			serviceAccounts: []*user.User{
+				{
+					ID:               11,
+					UID:              "u11",
+					Name:             "sa-1-extsvc-bug",
+					Login:            "sa-1-extsvc-bug",
+					Email:            "sa-1-extsvc-bug@org.com",
+					OrgID:            1,
+					Created:          now,
+					Updated:          now,
+					IsServiceAccount: true,
+				},
+				{
+					ID:               12,
+					UID:              "u12",
+					Name:             "sa-2-extsvc-bug2",
+					Login:            "sa-2-extsvc-bug2",
+					Email:            "sa-2-extsvc-bug2@org.com",
+					OrgID:            2,
+					Created:          now,
+					Updated:          now,
+					IsServiceAccount: true,
+				},
+			},
+			wantServiceAccounts: []*user.User{
+				{
+					ID:    11,
+					Login: "sa-1-extsvc-bug",
+				},
+				{
+					ID:    12,
+					Login: "sa-2-extsvc-bug2",
+				},
+			},
+		},
 	}
 
 	for _, tc := range testCases {