Skip to content

Commit

Permalink
restoreccl: allow restoring system users without user ids
Browse files Browse the repository at this point in the history 
During restore, when users in the backup did not have ids, they'll be given one.

Release note: None
  • Loading branch information
@RichardJCai
RichardJCai committed Aug 1, 2022
1 parent 7e2f424 commit 1687d19
Show file tree
Hide file tree
Showing 34 changed files with 310 additions and 19 deletions.
55 changes: 38 additions & 17 deletions pkg/ccl/backupccl/backup_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1070,17 +1070,14 @@ func TestBackupRestoreSystemTables(t *testing.T) {

// At the time this test was written, these were the only system tables that
// were reasonable for a user to backup and restore into another cluster.
tables := []string{"locations", "role_members", "users", "zones", "role_id_seq"}
tables := []string{"locations", "role_members", "users", "zones"}
tableSpec := "system." + strings.Join(tables, ", system.")

// Take a consistent fingerprint of the original tables.
var backupAsOf string
expectedFingerprints := map[string][][]string{}
err := crdb.ExecuteTx(ctx, conn, nil /* txopts */, func(tx *gosql.Tx) error {
for _, table := range tables {
if table == "role_id_seq" {
continue
}
rows, err := conn.Query("SHOW EXPERIMENTAL_FINGERPRINTS FROM TABLE system." + table)
if err != nil {
return err
Expand All @@ -1106,9 +1103,6 @@ func TestBackupRestoreSystemTables(t *testing.T) {

// Verify the fingerprints match.
for _, table := range tables {
if table == "role_id_seq" {
continue
}
a := sqlDB.QueryStr(t, "SHOW EXPERIMENTAL_FINGERPRINTS FROM TABLE system_new."+table)
if e := expectedFingerprints[table]; !reflect.DeepEqual(e, a) {
t.Fatalf("fingerprints between system.%[1]s and system_new.%[1]s did not match:%s\n",
Expand Down Expand Up @@ -9515,7 +9509,6 @@ func TestExcludeDataFromBackupDoesNotHoldupGC(t *testing.T) {
func TestBackupRestoreSystemUsers(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)
skip.WithIssue(t, 78963)

sqlDB, tempDir, cleanupFn := createEmptyCluster(t, singleNode)
_, sqlDBRestore, cleanupEmptyCluster := backupRestoreTestSetupEmpty(t, singleNode, tempDir, InitManualReplication, base.TestClusterArgs{})
Expand Down Expand Up @@ -9576,18 +9569,17 @@ func TestBackupRestoreSystemUsers(t *testing.T) {
defer cleanupEmptyCluster1()
t.Run("restore-from-backup-with-no-system-role-members", func(t *testing.T) {
sqlDBRestore1.Exec(t, "RESTORE SYSTEM USERS FROM $1", localFoo+"/3")

sqlDBRestore1.CheckQueryResults(t, "SELECT username, \"hashedPassword\", \"isRole\" FROM system.users", [][]string{
{"admin", "", "true"},
{"app", "NULL", "false"},
{"app_role", "NULL", "true"},
{"root", "", "false"},
{"test", "NULL", "false"},
{"test_role", "NULL", "true"},
})
sqlDBRestore1.CheckQueryResults(t, "SELECT \"role\", \"member\", \"isAdmin\" FROM system.role_members", [][]string{
{"admin", "root", "true"},
})
sqlDBRestore1.CheckQueryResults(t, "SELECT username, \"hashedPassword\", \"isRole\", \"user_id\" FROM system.users", [][]string{
{"admin", "", "true", "2"},
{"app", "NULL", "false", "100"},
{"app_role", "NULL", "true", "101"},
{"root", "", "false", "1"},
{"test", "NULL", "false", "102"},
{"test_role", "NULL", "true", "103"},
})
sqlDBRestore1.CheckQueryResults(t, "SHOW USERS", [][]string{
{"admin", "", "{}"},
{"app", "", "{}"},
Expand All @@ -9597,6 +9589,35 @@ func TestBackupRestoreSystemUsers(t *testing.T) {
{"test_role", "", "{}"},
})
})
_, sqlDBRestore2, cleanupEmptyCluster2 := backupRestoreTestSetupEmpty(t, singleNode, tempDir, InitManualReplication, base.TestClusterArgs{})
defer cleanupEmptyCluster2()
t.Run("restore-from-backup-with-existing-user", func(t *testing.T) {
// Create testuser and verify that the system user ids are
// allocated properly in the restore.
sqlDBRestore2.Exec(t, "CREATE USER testuser")
sqlDBRestore2.Exec(t, "RESTORE SYSTEM USERS FROM $1", localFoo+"/3")
sqlDBRestore2.CheckQueryResults(t, "SELECT \"role\", \"member\", \"isAdmin\" FROM system.role_members", [][]string{
{"admin", "root", "true"},
})
sqlDBRestore2.CheckQueryResults(t, "SELECT username, \"hashedPassword\", \"isRole\", \"user_id\" FROM system.users", [][]string{
{"admin", "", "true", "2"},
{"app", "NULL", "false", "101"},
{"app_role", "NULL", "true", "102"},
{"root", "", "false", "1"},
{"test", "NULL", "false", "103"},
{"test_role", "NULL", "true", "104"},
{"testuser", "NULL", "false", "100"},
})
sqlDBRestore2.CheckQueryResults(t, "SHOW USERS", [][]string{
{"admin", "", "{}"},
{"app", "", "{}"},
{"app_role", "", "{}"},
{"root", "", "{admin}"},
{"test", "", "{}"},
{"test_role", "", "{}"},
{"testuser", "", "{}"},
})
})
}

// TestUserfileNormalizationIncrementalShowBackup tests to see that file
Expand Down
48 changes: 48 additions & 0 deletions pkg/ccl/backupccl/full_cluster_backup_restore_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1112,3 +1112,51 @@ DROP DATABASE defaultdb;
{fmt.Sprint(parentID), fmt.Sprint(parentSchemaID), name, fmt.Sprint(ID)},
})
}

func TestFullClusterRestoreWithUserIDs(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)

params := base.TestClusterArgs{
ServerArgs: base.TestServerArgs{
Knobs: base.TestingKnobs{
JobsTestingKnobs: jobs.NewTestingKnobsWithShortIntervals(),
},
},
}
const numAccounts = 10
_, sqlDB, tempDir, cleanupFn := backupRestoreTestSetupWithParams(t, singleNode, numAccounts, InitManualReplication, params)
_, sqlDBRestore, cleanupEmptyCluster := backupRestoreTestSetupEmpty(t, singleNode, tempDir, InitManualReplication, params)
defer cleanupFn()
defer cleanupEmptyCluster()

sqlDB.Exec(t, `CREATE USER test1`)
sqlDB.Exec(t, `CREATE USER test2`)
sqlDB.Exec(t, `BACKUP TO $1`, localFoo)

sqlDB.CheckQueryResults(t, `SELECT * FROM system.users ORDER BY user_id`, [][]string{
{"root", "", "false", "1"},
{"admin", "", "true", "2"},
{"test1", "NULL", "false", "100"},
{"test2", "NULL", "false", "101"},
})
// Ensure that the new backup succeeds.
sqlDBRestore.Exec(t, `RESTORE FROM $1`, localFoo)

sqlDBRestore.CheckQueryResults(t, `SELECT * FROM system.users ORDER BY user_id`, [][]string{
{"root", "", "false", "1"},
{"admin", "", "true", "2"},
{"test1", "NULL", "false", "100"},
{"test2", "NULL", "false", "101"},
})

sqlDBRestore.Exec(t, `CREATE USER test3`)

sqlDBRestore.CheckQueryResults(t, `SELECT * FROM system.users ORDER BY user_id`, [][]string{
{"root", "", "false", "1"},
{"admin", "", "true", "2"},
{"test1", "NULL", "false", "100"},
{"test2", "NULL", "false", "101"},
{"test3", "NULL", "false", "102"},
})
}
17 changes: 16 additions & 1 deletion pkg/ccl/backupccl/restore_job.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1165,6 +1165,7 @@ func remapPublicSchemas(
// if the database does not have a public schema backed by a descriptor
// (meaning they were created before 22.1), we need to create a public
// schema descriptor for it.
// TODO(richardjcai): Are we worried about leaking IDs here?
id, err := descidgen.GenerateUniqueDescID(ctx, p.ExecCfg().DB, p.ExecCfg().Codec)
if err != nil {
return err
Expand Down Expand Up @@ -2394,11 +2395,25 @@ func (r *restoreResumer) restoreSystemUsers(
}

insertUser := `INSERT INTO system.users ("username", "hashedPassword", "isRole") VALUES ($1, $2, $3)`
if r.execCfg.Settings.Version.IsActive(ctx, clusterversion.AddSystemUserIDColumn) {
insertUser = `INSERT INTO system.users ("username", "hashedPassword", "isRole", "user_id") VALUES ($1, $2, $3, $4)`
}
newUsernames := make(map[string]bool)
args := make([]interface{}, 4)
for _, user := range users {
newUsernames[user[0].String()] = true
args[0] = user[0]
args[1] = user[1]
args[2] = user[2]
if r.execCfg.Settings.Version.IsActive(ctx, clusterversion.AddSystemUserIDColumn) {
id, err := descidgen.GenerateUniqueRoleID(ctx, r.execCfg.DB, r.execCfg.Codec)
if err != nil {
return err
}
args[3] = id
}
if _, err = executor.Exec(ctx, "insert-non-existent-users", txn, insertUser,
user[0], user[1], user[2]); err != nil {
args...); err != nil {
return err
}
}
Expand Down
85 changes: 85 additions & 0 deletions pkg/ccl/backupccl/restore_old_versions_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ func TestRestoreOldVersions(t *testing.T) {
privilegeDirs = testdataBase + "/privileges"
multiRegionDirs = testdataBase + "/multi-region"
publicSchemaDirs = testdataBase + "/public-schema-remap"
systemUsersDirs = testdataBase + "/system-users-restore"
)

t.Run("table-restore", func(t *testing.T) {
Expand Down Expand Up @@ -299,6 +300,17 @@ ORDER BY object_type, object_name`, [][]string{
t.Run(dir.Name(), restoreSyntheticPublicSchemaNamespaceEntryCleanupOnFail(exportDir))
}
})

t.Run("system-users-restore", func(t *testing.T) {
dirs, err := ioutil.ReadDir(systemUsersDirs)
require.NoError(t, err)
for _, dir := range dirs {
require.True(t, dir.IsDir())
exportDir, err := filepath.Abs(filepath.Join(systemUsersDirs, dir.Name()))
require.NoError(t, err)
t.Run(dir.Name(), restoreSystemUsersWithoutIDs(exportDir))
}
})
}

func restoreOldVersionTestWithInterleave(exportDir string) func(t *testing.T) {
Expand Down Expand Up @@ -1111,3 +1123,76 @@ func restoreSyntheticPublicSchemaNamespaceEntryCleanupOnFail(exportDir string) f
sqlDB.CheckQueryResults(t, `SELECT id FROM system.namespace WHERE name = 'public' AND id=29 AND "parentID"!=1`, [][]string{})
}
}

func restoreSystemUsersWithoutIDs(exportDir string) func(t *testing.T) {
return func(t *testing.T) {
const numAccounts = 1000
_, _, tmpDir, cleanupFn := backupRestoreTestSetup(t, multiNode, numAccounts, InitManualReplication)
defer cleanupFn()

_, sqlDB, cleanup := backupRestoreTestSetupEmpty(t, singleNode, tmpDir,
InitManualReplication, base.TestClusterArgs{
ServerArgs: base.TestServerArgs{
Knobs: base.TestingKnobs{
JobsTestingKnobs: jobs.NewTestingKnobsWithShortIntervals(),
},
}})
defer cleanup()
err := os.Symlink(exportDir, filepath.Join(tmpDir, "foo"))
require.NoError(t, err)

sqlDB.Exec(t, fmt.Sprintf("RESTORE SYSTEM USERS FROM '%s'", localFoo))

sqlDB.CheckQueryResults(t, `SELECT username, "hashedPassword", "isRole", user_id FROM system.users`, [][]string{
{"admin", "", "true", "2"},
{"root", "", "false", "1"},
{"testrole", "NULL", "true", "100"},
{"testuser", "NULL", "false", "101"},
{"testuser2", "NULL", "false", "102"},
{"testuser3", "NULL", "false", "103"},
})

// Verify that the next user we create uses the next biggest ID.
sqlDB.Exec(t, fmt.Sprintf("CREATE USER testuser4"))

sqlDB.CheckQueryResults(t, `SELECT username, "hashedPassword", "isRole", user_id FROM system.users`, [][]string{
{"admin", "", "true", "2"},
{"root", "", "false", "1"},
{"testrole", "NULL", "true", "100"},
{"testuser", "NULL", "false", "101"},
{"testuser2", "NULL", "false", "102"},
{"testuser3", "NULL", "false", "103"},
{"testuser4", "NULL", "false", "104"},
})

// Drop some users and try restoring again.
sqlDB.Exec(t, fmt.Sprintf("DROP ROLE testrole"))
sqlDB.Exec(t, fmt.Sprintf("DROP ROLE testuser2"))
sqlDB.Exec(t, fmt.Sprintf("DROP ROLE testuser3"))
sqlDB.Exec(t, fmt.Sprintf("DROP ROLE testuser4"))

sqlDB.Exec(t, fmt.Sprintf("RESTORE SYSTEM USERS FROM '%s'", localFoo))

// testrole, testuser2, testuser3 should be reassigned higher ids.
sqlDB.CheckQueryResults(t, `SELECT username, "hashedPassword", "isRole", user_id FROM system.users`, [][]string{
{"admin", "", "true", "2"},
{"root", "", "false", "1"},
{"testrole", "NULL", "true", "105"},
{"testuser", "NULL", "false", "101"},
{"testuser2", "NULL", "false", "106"},
{"testuser3", "NULL", "false", "107"},
})

// Verify that the next user we create uses the next biggest ID.
sqlDB.Exec(t, fmt.Sprintf("CREATE USER testuser4"))
sqlDB.CheckQueryResults(t, `SELECT username, "hashedPassword", "isRole", user_id FROM system.users`, [][]string{
{"admin", "", "true", "2"},
{"root", "", "false", "1"},
{"testrole", "NULL", "true", "105"},
{"testuser", "NULL", "false", "101"},
{"testuser2", "NULL", "false", "106"},
{"testuser3", "NULL", "false", "107"},
{"testuser4", "NULL", "false", "108"},
})
}
}
Loading

0 comments on commit 1687d19

Please sign in to comment.