From 56d404efde5f5bb922d8773b318ad0fbd7959c7b Mon Sep 17 00:00:00 2001 From: Andrei Matei Date: Mon, 28 Nov 2022 13:09:39 -0500 Subject: [PATCH] upgrades: remove upgrade granting CREATELOGIN role opt This patch removes a permanent upgrade that was granting the CREATELOGIN role option to users that had the CREATEROLE option. This upgrade was introduced in 20.2, meant to grant the then-new CREATELOGIN option to users created in 20.1 and prior that had the CREATEROLE option (CREATELOGIN was being split from CREATEROLE). This code stayed around as a startupmigration since it was introduced, even though it didn't make sense for it to stay around after 20.2. Technically, I think this startup migration should have had the flag `IncludedInBootstrap=v20.2`, since we don't want it to run for clusters created at or after 20.2; this migration is not idempotent in the general sense, since it potentially picks up new, unintended users when it runs. Since 22.2, this migration would fail to run on anything but an empty system.role_options table because it would attempt to put NULLs into a non-nullable column. This was all benign since the startupmigrations had protection in place, preventing them for running a 2nd time after a completed attempt. So, for upgraded cluster, the migration only when the first 20.2 node came up, and for new clusters it would be a no-op since the system.role_options table starts empty. This migration became problematic in #91627 when I've turned the startupmigrations into upgrades. These upgrades run once when upgrading to 23.1; I'm relying on their idempotence. Fixes #92230 Fixes #92371 Fixes #92569 Release note: None --- pkg/upgrade/upgrades/permanent_upgrades.go | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/pkg/upgrade/upgrades/permanent_upgrades.go b/pkg/upgrade/upgrades/permanent_upgrades.go index 856d2a215bc7..33f6f64e6790 100644 --- a/pkg/upgrade/upgrades/permanent_upgrades.go +++ b/pkg/upgrade/upgrades/permanent_upgrades.go @@ -57,20 +57,6 @@ func addRootUser( ` _, err = deps.InternalExecutor.Exec( ctx, "addRootToAdminRole", nil /* txn */, upsertMembership, username.AdminRole, username.RootUser) - if err != nil { - return err - } - - // Add the CREATELOGIN option to roles that already have CREATEROLE. - const upsertCreateRoleStmt = ` - UPSERT INTO system.role_options (username, option, value) - SELECT username, 'CREATELOGIN', NULL - FROM system.role_options - WHERE option = 'CREATEROLE' - ` - _, err = deps.InternalExecutor.Exec( - ctx, "add CREATELOGIN where a role already has CREATEROLE", nil, /* txn */ - upsertCreateRoleStmt) return err }