From 0c58d39e76292cf4e291b6905e38074d27adbc3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20=C5=A0tibran=C3=BD?= Date: Wed, 19 Mar 2025 12:34:44 +0100 Subject: [PATCH] Spanner-related fixes (#102376) * Fix UNION syntax in resourcepermissions package. * Fix migrations in usermig package to work with Spanner. * Fix health query. * Use more connections for integration tests. * Add test-go-integration-spanner target to run integration tests against Spanner emulator. * Add test for enterprise. * Don't delete sequence number for migration_log.id column. * Only bump max open connections to 20 for Spanner. Lower integration test timeout. --- Makefile | 7 +++++ pkg/api/health.go | 2 +- .../resourcepermissions/store.go | 2 +- ...ice_account_multiple_org_login_migrator.go | 27 +++++++++++++++++++ .../usermig/user_lowercase_login_and_email.go | 2 +- .../sqlstore/migrator/spanner_dialect.go | 3 +++ pkg/tests/testinfra/testinfra.go | 12 +++++++-- 7 files changed, 50 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index 8b0c6664c18..74fedb47854 100644 --- a/Makefile +++ b/Makefile @@ -311,6 +311,13 @@ test-go-integration-memcached: ## Run integration tests for memcached cache. $(GO) clean -testcache MEMCACHED_HOSTS=localhost:11211 $(GO) test $(GO_RACE_FLAG) $(GO_TEST_FLAGS) -run IntegrationMemcached -covermode=atomic -timeout=2m $(GO_INTEGRATION_TESTS) +.PHONY: test-go-integration-spanner +test-go-integration-spanner: ## Run integration tests for Spanner backend with flags. Uses spanner-emulator on localhost:9010 and localhost:9020. + @if [ "${WIRE_TAGS}" != "enterprise" ]; then echo "Spanner integration test require enterprise setup"; exit 1; fi + @echo "test backend integration spanner tests" + GRAFANA_TEST_DB=spanner SPANNER_DB=emulator \ + $(GO) test $(GO_RACE_FLAG) $(GO_TEST_FLAGS) -p=1 -count=1 -v -run "^TestIntegration" -covermode=atomic -timeout=2m $(GO_INTEGRATION_TESTS) + .PHONY: test-js test-js: ## Run tests for frontend. @echo "test frontend" diff --git a/pkg/api/health.go b/pkg/api/health.go index db80f9cdaec..b564dd6ada2 100644 --- a/pkg/api/health.go +++ b/pkg/api/health.go @@ -15,7 +15,7 @@ func (hs *HTTPServer) databaseHealthy(ctx context.Context) bool { } err := hs.SQLStore.WithDbSession(ctx, func(session *db.Session) error { - _, err := session.Exec("SELECT 1") + _, err := session.Query("SELECT 1") return err }) healthy := err == nil diff --git a/pkg/services/accesscontrol/resourcepermissions/store.go b/pkg/services/accesscontrol/resourcepermissions/store.go index c8d8e8db88e..f7444376ee9 100644 --- a/pkg/services/accesscontrol/resourcepermissions/store.go +++ b/pkg/services/accesscontrol/resourcepermissions/store.go @@ -449,7 +449,7 @@ func (s *store) getResourcePermissions(sess *db.Session, orgID int64, query GetR builtin := builtinSelect + builtinFrom + where args = append(args, args[:initialLength]...) - sql := userQuery + " UNION " + team + " UNION " + builtin + sql := userQuery + " " + s.sql.GetDialect().UnionDistinct() + " " + team + " " + s.sql.GetDialect().UnionDistinct() + " " + builtin queryResults := make([]flatResourcePermission, 0) if err := sess.SQL(sql, args...).Find(&queryResults); err != nil { return nil, err 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 b1ad97f621e..bb2c878e296 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 @@ -72,6 +72,20 @@ func (p *ServiceAccountsSameLoginCrossOrgs) Exec(sess *xorm.Session, mg *migrato AND is_service_account = 1 AND login NOT LIKE 'sa-' || CAST(org_id AS TEXT) || '-%'; `) + case migrator.Spanner: + _, err = p.sess.Exec(` + UPDATE user + SET login = CONCAT('sa-', CAST(org_id AS STRING), '-', + CASE + WHEN login LIKE 'sa-%' THEN SUBSTRING(login, 4) + ELSE login + END + ) + WHERE login IS NOT NULL + AND is_service_account + AND login NOT LIKE CONCAT('sa-', CAST(org_id AS STRING), '-%') + `) + default: return fmt.Errorf("dialect not supported: %s", p.dialect) } @@ -128,6 +142,19 @@ func (p *ServiceAccountsDeduplicateOrgInLogin) Exec(sess *xorm.Session, mg *migr 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) );; `) + case migrator.Spanner: + _, err = sess.Exec(` + UPDATE ` + dialect.Quote("user") + ` AS u + SET login = 'sa-' || CAST(u.org_id AS STRING) || SUBSTRING(u.login, LENGTH('sa-'||CAST(u.org_id AS STRING)||'-'||CAST(u.org_id AS STRING))+1) + WHERE u.login IS NOT NULL + AND u.is_service_account + AND u.login LIKE 'sa-'||CAST(u.org_id AS STRING)||'-'||CAST(u.org_id AS STRING)||'-%' + AND NOT EXISTS ( + SELECT 1 + FROM ` + dialect.Quote("user") + `AS u2 + WHERE u2.login = 'sa-' || CAST(u.org_id AS STRING) || SUBSTRING(u.login, LENGTH('sa-'||CAST(u.org_id AS STRING)||'-'||CAST(u.org_id AS STRING))+1) + );; + `) default: return fmt.Errorf("dialect not supported: %s", dialect) } diff --git a/pkg/services/sqlstore/migrations/usermig/user_lowercase_login_and_email.go b/pkg/services/sqlstore/migrations/usermig/user_lowercase_login_and_email.go index 570b79ba52e..2d85a64a426 100644 --- a/pkg/services/sqlstore/migrations/usermig/user_lowercase_login_and_email.go +++ b/pkg/services/sqlstore/migrations/usermig/user_lowercase_login_and_email.go @@ -30,7 +30,7 @@ func (p *UsersLowerCaseLoginAndEmail) SQL(dialect migrator.Dialect) string { func (p *UsersLowerCaseLoginAndEmail) Exec(sess *xorm.Session, mg *migrator.Migrator) error { // Get all users users := make([]*user.User, 0) - err := sess.Table("user").Find(&users) + err := sess.Table("user").Asc("created").Find(&users) if err != nil { return err } diff --git a/pkg/services/sqlstore/migrator/spanner_dialect.go b/pkg/services/sqlstore/migrator/spanner_dialect.go index 68d1f8fe3a0..c66032367ff 100644 --- a/pkg/services/sqlstore/migrator/spanner_dialect.go +++ b/pkg/services/sqlstore/migrator/spanner_dialect.go @@ -147,6 +147,9 @@ func (s *SpannerDialect) TruncateDBTables(engine *xorm.Engine) error { switch table.Name { case "": continue + case "autoincrement_sequences": + // Don't delete sequence number for migration_log.id column. + statements = append(statements, fmt.Sprintf("DELETE FROM %v WHERE name <> 'migration_log:id'", s.Quote(table.Name))) case "migration_log": continue case "dashboard_acl": diff --git a/pkg/tests/testinfra/testinfra.go b/pkg/tests/testinfra/testinfra.go index d8c412f9618..aaf7a746956 100644 --- a/pkg/tests/testinfra/testinfra.go +++ b/pkg/tests/testinfra/testinfra.go @@ -495,9 +495,17 @@ func CreateGrafDir(t *testing.T, opts GrafanaOpts) (string, string) { require.NoError(t, err) _, err = dbSection.NewKey("query_retries", fmt.Sprintf("%d", queryRetries)) require.NoError(t, err) - _, err = dbSection.NewKey("max_open_conn", "2") + if db.IsTestDBSpanner() { + _, err = dbSection.NewKey("max_open_conn", "20") + } else { + _, err = dbSection.NewKey("max_open_conn", "2") + } require.NoError(t, err) - _, err = dbSection.NewKey("max_idle_conn", "2") + if db.IsTestDBSpanner() { + _, err = dbSection.NewKey("max_idle_conn", "20") + } else { + _, err = dbSection.NewKey("max_idle_conn", "2") + } require.NoError(t, err) cfgPath := filepath.Join(cfgDir, "test.ini")