Skip to content

Commit

Permalink
upgrades: remove upgrade granting CREATELOGIN role opt
Browse files Browse the repository at this point in the history
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
  • Loading branch information
andreimatei committed Nov 28, 2022
1 parent 1a6e9f8 commit 56d404e
Showing 1 changed file with 0 additions and 14 deletions.
14 changes: 0 additions & 14 deletions pkg/upgrade/upgrades/permanent_upgrades.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down

0 comments on commit 56d404e

Please sign in to comment.