From 9fc58193cd70fbff317824af9b143ae8ca6f5282 Mon Sep 17 00:00:00 2001 From: richardjcai Date: Tue, 11 May 2021 11:15:36 -0400 Subject: [PATCH 1/2] sql: fix Usage privilege on Tables/DBs after upgrading from 20.1 In 20.2, I made a mistake when introducing USAGE privilege by adding it before ZONECONFIG privilege in the bitfield. Due to this, when updating from 20.1 to 20.2, ZONECONFIG privilege will be interpretted as USAGE. Fortunately, ZONECONFIG was only a valid privilege on tables and databases while USAGE is invalid on both those objects. Due to this, whenever we encounter USAGE on a privilege descriptor versioned 20.1 (InitialVersion) we can fix this by removing the bit for USAGE and adding the bit for ZONECONFIG on the privilege descriptor. Release note: None --- .../catalog/dbdesc/database_desc_builder.go | 1 + pkg/sql/catalog/descpb/privilege.go | 41 ++++ pkg/sql/catalog/descpb/privilege_test.go | 225 ++++++++++++++++++ .../catalog/tabledesc/table_desc_builder.go | 4 + 4 files changed, 271 insertions(+) diff --git a/pkg/sql/catalog/dbdesc/database_desc_builder.go b/pkg/sql/catalog/dbdesc/database_desc_builder.go index af087e51c90a..840e37c5c534 100644 --- a/pkg/sql/catalog/dbdesc/database_desc_builder.go +++ b/pkg/sql/catalog/dbdesc/database_desc_builder.go @@ -59,6 +59,7 @@ func (ddb *databaseDescriptorBuilder) RunPostDeserializationChanges( // run again and mixed-version clusters always write "good" descriptors. ddb.maybeModified = protoutil.Clone(ddb.original).(*descpb.DatabaseDescriptor) descpb.MaybeFixPrivileges(ddb.maybeModified.ID, &ddb.maybeModified.Privileges) + descpb.MaybeFixUsagePrivForTablesAndDBs(&ddb.maybeModified.Privileges) return nil } diff --git a/pkg/sql/catalog/descpb/privilege.go b/pkg/sql/catalog/descpb/privilege.go index ae835d49c71c..1b741e7802c4 100644 --- a/pkg/sql/catalog/descpb/privilege.go +++ b/pkg/sql/catalog/descpb/privilege.go @@ -208,6 +208,47 @@ func (p *PrivilegeDescriptor) Revoke( } } +// MaybeFixUsagePrivForTablesAndDBs fixes cases where privilege descriptors +// with ZONECONFIG were corrupted after upgrading from 20.1 to 20.2. +// USAGE was mistakenly added in the privilege bitfield above ZONECONFIG +// causing privilege descriptors with ZONECONFIG in 20.1 to have USAGE privilege +// instead of ZONECONFIG. +// Fortunately ZONECONFIG was only valid on TABLES/DB while USAGE is not valid +// on either so we know if the descriptor was corrupted. +func MaybeFixUsagePrivForTablesAndDBs(ptr **PrivilegeDescriptor) bool { + if *ptr == nil { + *ptr = &PrivilegeDescriptor{} + } + p := *ptr + modified := false + + if p.Version > InitialVersion { + // InitialVersion is for descriptors that were created in versions 20.1 and + // earlier. If the privilege descriptor was created after 20.1, then we + // do not have to fix it. Furthermore privilege descriptor versions are + // currently never updated so we're guaranteed to only have this issue + // on privilege descriptors that are on "InitialVersion". + return false + } + + for i := range p.Users { + // Users is a slice of values, we need pointers to make them mutable. + userPrivileges := &p.Users[i] + // Tables and Database should not have USAGE privilege in 20.2 onwards. + // The only reason they would have USAGE privilege is because they had + // ZoneConfig in 20.1 and upgrading to 20.2 where USAGE was added + // in the privilege bitfield where ZONECONFIG previously was. + if privilege.USAGE.Mask()&userPrivileges.Privileges != 0 { + // Remove USAGE privilege and add ZONECONFIG. The privilege was + // originally ZONECONFIG in 20.1 but got changed to USAGE in 20.2 + // due to changing the bitfield values. + userPrivileges.Privileges = (userPrivileges.Privileges - privilege.USAGE.Mask()) | privilege.ZONECONFIG.Mask() + modified = true + } + } + return modified +} + // MaybeFixPrivileges fixes the privilege descriptor if needed, including: // * adding default privileges for the "admin" role // * fixing default privileges for the "root" user diff --git a/pkg/sql/catalog/descpb/privilege_test.go b/pkg/sql/catalog/descpb/privilege_test.go index e5192c7d3f48..c89e81cb8c9c 100644 --- a/pkg/sql/catalog/descpb/privilege_test.go +++ b/pkg/sql/catalog/descpb/privilege_test.go @@ -651,3 +651,228 @@ func TestValidateOwnership(t *testing.T) { t.Fatal(err) } } + +// TestMaybeFixUsageAndZoneConfigPrivilege checks that Table and DB descriptors +// on created on v20.1 or prior (PrivilegeDescVersion InitialVersion) with +// USAGE privilege have its privilege correctly updated. +// The bit representing USAGE privilege in 20.2 for Tables/DBs should actually +// be ZONECONFIG privilege and should be updated. +func TestMaybeFixUsageAndZoneConfigPrivilege(t *testing.T) { + + fooUser := security.MakeSQLUsernameFromPreNormalizedString("foo") + barUser := security.MakeSQLUsernameFromPreNormalizedString("bar") + bazUser := security.MakeSQLUsernameFromPreNormalizedString("baz") + + type userPrivileges map[security.SQLUsername]privilege.List + + testCases := []struct { + input userPrivileges + modified bool + output userPrivileges + objectType privilege.ObjectType + privDescVersion PrivilegeDescVersion + description string + isValid bool + }{ + // Cases for Tables and Databases. + { + userPrivileges{ + fooUser: privilege.List{privilege.USAGE}, + }, + true, + userPrivileges{ + fooUser: privilege.List{privilege.ZONECONFIG}, + }, + privilege.Table, + InitialVersion, + "A privilege descriptor from a table created in v20.1 or prior " + + "(InitialVersion) with USAGE should have the privilege converted to ZONECONFIG.", + true, + }, + { + userPrivileges{ + fooUser: privilege.List{privilege.USAGE}, + }, + true, + userPrivileges{ + fooUser: privilege.List{privilege.ZONECONFIG}, + }, + privilege.Database, + InitialVersion, + "A privilege descriptor from a database created in v20.1 or prior " + + "(InitialVersion) with USAGE should have the privilege converted to ZONECONFIG.", + true, + }, + { + userPrivileges{ + fooUser: privilege.List{privilege.USAGE}, + }, + false, + userPrivileges{ + fooUser: privilege.List{privilege.USAGE}, + }, + privilege.Table, + OwnerVersion, + "A privilege descriptor from a table created in v20.2 onwards " + + "(OwnerVersion) should not be modified.", + false, + }, + { + userPrivileges{ + fooUser: privilege.List{privilege.USAGE}, + }, + false, + userPrivileges{ + fooUser: privilege.List{privilege.USAGE}, + }, + privilege.Database, + OwnerVersion, + "A privilege descriptor from a Database created in v20.2 onwards " + + "(OwnerVersion) should not be modified.", + false, + }, + { + userPrivileges{ + fooUser: privilege.List{privilege.ZONECONFIG}, + }, + false, + userPrivileges{ + fooUser: privilege.List{privilege.ZONECONFIG}, + }, + privilege.Table, + OwnerVersion, + "A privilege descriptor from a table created in v20.2 onwards " + + "(OwnerVersion) should not be modified.", + true, + }, + { + userPrivileges{ + fooUser: privilege.List{privilege.ZONECONFIG}, + }, + false, + userPrivileges{ + fooUser: privilege.List{privilege.ZONECONFIG}, + }, + privilege.Database, + OwnerVersion, + "A privilege descriptor from a Database created in v20.2 onwards " + + "(OwnerVersion) should not be modified.", + true, + }, + // Fix privileges for multiple users. + { + userPrivileges{ + fooUser: privilege.List{privilege.USAGE}, + barUser: privilege.List{privilege.USAGE, privilege.CREATE, privilege.SELECT}, + }, + true, + userPrivileges{ + fooUser: privilege.List{privilege.ZONECONFIG}, + barUser: privilege.List{privilege.ZONECONFIG, privilege.CREATE, privilege.SELECT}, + }, + privilege.Table, + InitialVersion, + "A privilege descriptor from a table created in v20.1 or prior " + + "(InitialVersion) with USAGE should have the privilege converted to ZONECONFIG.", + true, + }, + { + userPrivileges{ + fooUser: privilege.List{privilege.USAGE}, + barUser: privilege.List{privilege.USAGE, privilege.CREATE, privilege.SELECT}, + }, + true, + userPrivileges{ + fooUser: privilege.List{privilege.ZONECONFIG}, + barUser: privilege.List{privilege.ZONECONFIG, privilege.CREATE, privilege.SELECT}, + }, + privilege.Database, + InitialVersion, + "A privilege descriptor from a table created in v20.1 or prior " + + "(InitialVersion) with USAGE should have the privilege converted to ZONECONFIG.", + true, + }, + { + userPrivileges{ + fooUser: privilege.List{privilege.USAGE}, + barUser: privilege.List{privilege.USAGE, privilege.CREATE, privilege.GRANT}, + }, + true, + userPrivileges{ + fooUser: privilege.List{privilege.ZONECONFIG}, + barUser: privilege.List{privilege.ZONECONFIG, privilege.CREATE, privilege.GRANT}, + }, + privilege.Database, + InitialVersion, + "A privilege descriptor from a database created in v20.1 or prior " + + "(InitialVersion) with USAGE should have the privilege converted to ZONECONFIG.", + true, + }, + { + userPrivileges{ + fooUser: privilege.List{privilege.USAGE}, + barUser: privilege.List{privilege.USAGE, privilege.CREATE, privilege.SELECT}, + bazUser: privilege.List{privilege.ALL, privilege.USAGE}, + }, + true, + userPrivileges{ + fooUser: privilege.List{privilege.ZONECONFIG}, + barUser: privilege.List{privilege.ZONECONFIG, privilege.CREATE, privilege.SELECT}, + bazUser: privilege.List{privilege.ALL}, + }, + privilege.Database, + InitialVersion, + "A privilege descriptor from a table created in v20.1 or prior " + + "(InitialVersion) with USAGE should have the privilege converted to ZONECONFIG.", + true, + }, + // Test case where the privilege descriptor has ZONECONFIG and USAGE. + { + userPrivileges{ + fooUser: privilege.List{privilege.USAGE, privilege.ZONECONFIG}, + }, + true, + userPrivileges{ + fooUser: privilege.List{privilege.ZONECONFIG}, + }, + privilege.Table, + InitialVersion, + "If the descriptor has USAGE and ZONECONFIG, it should become just " + + "ZONECONFIG", + true, + }, + } + + for num, tc := range testCases { + desc := &PrivilegeDescriptor{Version: tc.privDescVersion} + for u, p := range tc.input { + desc.Grant(u, p) + } + modified := MaybeFixUsagePrivForTablesAndDBs(&desc) + + if tc.modified != modified { + t.Errorf("expected modifed to be %v, was %v", tc.modified, modified) + } + + for u, p := range tc.output { + outputUser, ok := desc.findUser(u) + if !ok { + t.Errorf("#%d: expected user %s in output, but not found (%v)\n%s", + num, u, desc.Users, tc.description, + ) + } + if a, e := privilege.ListFromBitField(outputUser.Privileges, privilege.Any), p; a.ToBitField() != e.ToBitField() { + t.Errorf("#%d: user %s: expected privileges %v, got %v\n%s", + num, u, e, a, tc.description, + ) + } + + err := privilege.ValidatePrivileges(p, tc.objectType) + if tc.isValid && err != nil { + t.Errorf("%s\n%s", err.Error(), tc.description) + } + } + + } + +} diff --git a/pkg/sql/catalog/tabledesc/table_desc_builder.go b/pkg/sql/catalog/tabledesc/table_desc_builder.go index 91844111e4c0..91d017bc1a27 100644 --- a/pkg/sql/catalog/tabledesc/table_desc_builder.go +++ b/pkg/sql/catalog/tabledesc/table_desc_builder.go @@ -190,6 +190,10 @@ func maybeFillInDescriptor( // run again and mixed-version clusters always write "good" descriptors. changes.FixedPrivileges = descpb.MaybeFixPrivileges(desc.ID, &desc.Privileges) + fixedUsagePrivilege := descpb.MaybeFixUsagePrivForTablesAndDBs(&desc.Privileges) + + changes.FixedPrivileges = changes.FixedPrivileges || fixedUsagePrivilege + if dg != nil { changes.UpgradedForeignKeyRepresentation, err = maybeUpgradeForeignKeyRepresentation( ctx, dg, skipFKsWithNoMatchingTable /* skipFKsWithNoMatchingTable*/, desc) From 1e42b0defe5ceab2041199004e917fe2f9d0989e Mon Sep 17 00:00:00 2001 From: richardjcai Date: Wed, 12 May 2021 17:02:20 -0400 Subject: [PATCH 2/2] restore: fix ZONECONFIG privilege when restoring from 20.1 version or prior. This is needed in addition to the previous commit to fix ZONECONFIG privilege when restoring from a version from 20.1 or prior. Release note: None --- pkg/ccl/backupccl/restore_job.go | 4 ++ .../backupccl/restore_old_versions_test.go | 52 ++++++++++++++++++ .../create_with_privileges.sql | 20 +++++++ .../privileges/v20.1.6/657900117481816066.sst | Bin 0 -> 1118 bytes .../privileges/v20.1.6/657900117481848836.sst | Bin 0 -> 938 bytes .../privileges/v20.1.6/657900117481881604.sst | Bin 0 -> 952 bytes .../privileges/v20.1.6/657900117481914372.sst | Bin 0 -> 1063 bytes .../privileges/v20.1.6/657900117482045442.sst | Bin 0 -> 968 bytes .../privileges/v20.1.6/657900117482078210.sst | Bin 0 -> 2475 bytes .../privileges/v20.1.6/657900117485420546.sst | Bin 0 -> 1138 bytes .../privileges/v20.1.6/657900117485879300.sst | Bin 0 -> 1356 bytes .../privileges/v20.1.6/657900117486469124.sst | Bin 0 -> 938 bytes .../privileges/v20.1.6/657900117486829572.sst | Bin 0 -> 1062 bytes .../privileges/v20.1.6/BACKUP | Bin 0 -> 6218 bytes pkg/sql/catalog/descpb/privilege.go | 2 +- pkg/sql/catalog/descpb/privilege_test.go | 13 +++++ 16 files changed, 90 insertions(+), 1 deletion(-) create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/create_with_privileges.sql create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/privileges/v20.1.6/657900117481816066.sst create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/privileges/v20.1.6/657900117481848836.sst create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/privileges/v20.1.6/657900117481881604.sst create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/privileges/v20.1.6/657900117481914372.sst create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/privileges/v20.1.6/657900117482045442.sst create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/privileges/v20.1.6/657900117482078210.sst create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/privileges/v20.1.6/657900117485420546.sst create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/privileges/v20.1.6/657900117485879300.sst create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/privileges/v20.1.6/657900117486469124.sst create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/privileges/v20.1.6/657900117486829572.sst create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/privileges/v20.1.6/BACKUP diff --git a/pkg/ccl/backupccl/restore_job.go b/pkg/ccl/backupccl/restore_job.go index da7097138370..8c9e6c999ef6 100644 --- a/pkg/ccl/backupccl/restore_job.go +++ b/pkg/ccl/backupccl/restore_job.go @@ -329,6 +329,8 @@ func WriteDescriptors( desc.GetID(), desc) } } + privilegeDesc := desc.GetPrivileges() + descpb.MaybeFixUsagePrivForTablesAndDBs(&privilegeDesc) wroteDBs[desc.GetID()] = desc if err := descsCol.WriteDescToBatch( ctx, false /* kvTrace */, desc.(catalog.MutableDescriptor), b, @@ -380,6 +382,8 @@ func WriteDescriptors( table.GetID(), table) } } + privilegeDesc := table.GetPrivileges() + descpb.MaybeFixUsagePrivForTablesAndDBs(&privilegeDesc) // If the table descriptor is being written to a multi-region database and // the table does not have a locality config setup, set one up here. The // table's locality config will be set to the default locality - REGIONAL diff --git a/pkg/ccl/backupccl/restore_old_versions_test.go b/pkg/ccl/backupccl/restore_old_versions_test.go index e3e51bf80f89..431c7bd51dbc 100644 --- a/pkg/ccl/backupccl/restore_old_versions_test.go +++ b/pkg/ccl/backupccl/restore_old_versions_test.go @@ -57,6 +57,7 @@ func TestRestoreOldVersions(t *testing.T) { fkRevDirs = testdataBase + "/fk-rev-history" clusterDirs = testdataBase + "/cluster" exceptionalDirs = testdataBase + "/exceptional" + privilegeDirs = testdataBase + "/privileges" ) t.Run("table-restore", func(t *testing.T) { @@ -150,6 +151,17 @@ ORDER BY object_type, object_name`, [][]string{ }) }) }) + + t.Run("zoneconfig_privilege_restore", func(t *testing.T) { + dirs, err := ioutil.ReadDir(privilegeDirs) + require.NoError(t, err) + for _, dir := range dirs { + require.True(t, dir.IsDir()) + exportDir, err := filepath.Abs(filepath.Join(privilegeDirs, dir.Name())) + require.NoError(t, err) + t.Run(dir.Name(), restoreV201ZoneconfigPrivilegeTest(exportDir)) + } + }) } func restoreOldVersionTest(exportDir string) func(t *testing.T) { @@ -182,6 +194,46 @@ func restoreOldVersionTest(exportDir string) func(t *testing.T) { } } +// restoreV201ZoneconfigPrivilegeTest checks that privilege descriptors with +// ZONECONFIG from tables and databases are correctly restored. +// The ZONECONFIG bit was overwritten to be USAGE in 20.2 onwards. +// We only need to test restoring with full cluster backup / restore as +// it is the only form of restore that restores privileges. +func restoreV201ZoneconfigPrivilegeTest(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{}) + defer cleanup() + err := os.Symlink(exportDir, filepath.Join(tmpDir, "foo")) + require.NoError(t, err) + sqlDB.Exec(t, `RESTORE FROM $1`, LocalFoo) + testDBGrants := [][]string{ + {"test", "admin", "ALL"}, + {"test", "root", "ALL"}, + {"test", "testuser", "ZONECONFIG"}, + } + sqlDB.CheckQueryResults(t, `show grants on database test`, testDBGrants) + + testTableGrants := [][]string{ + {"test", "public", "test_table", "admin", "ALL"}, + {"test", "public", "test_table", "root", "ALL"}, + {"test", "public", "test_table", "testuser", "ZONECONFIG"}, + } + sqlDB.CheckQueryResults(t, `show grants on test.test_table`, testTableGrants) + + testTable2Grants := [][]string{ + {"test", "public", "test_table2", "admin", "ALL"}, + {"test", "public", "test_table2", "root", "ALL"}, + {"test", "public", "test_table2", "testuser", "ALL"}, + } + sqlDB.CheckQueryResults(t, `show grants on test.test_table2`, testTable2Grants) + } +} + func restoreOldVersionFKRevTest(exportDir string) func(t *testing.T) { return func(t *testing.T) { params := base.TestServerArgs{} diff --git a/pkg/ccl/backupccl/testdata/restore_old_versions/create_with_privileges.sql b/pkg/ccl/backupccl/testdata/restore_old_versions/create_with_privileges.sql new file mode 100644 index 000000000000..da39f0dede03 --- /dev/null +++ b/pkg/ccl/backupccl/testdata/restore_old_versions/create_with_privileges.sql @@ -0,0 +1,20 @@ +-- The below SQL is used to create the data that is then exported with BACKUP. +-- This should be run on a v20.1 cluster, the ZONECONFIG bit is incorrectly +-- updated to be interpreted as USAGE in versions of 20.2 and onward. +-- The restore test should ensure that ZONECONFIG stays as ZONECONFIG. + +CREATE DATABASE test; + +SET database = test; + +CREATE USER testuser; + +CREATE TABLE test_table(); + +CREATE TABLE test_table2(); + +GRANT ZONECONFIG ON DATABASE test TO testuser; + +GRANT ZONECONFIG ON test_table TO testuser; + +GRANT ALL ON test_table2 TO testuser; diff --git a/pkg/ccl/backupccl/testdata/restore_old_versions/privileges/v20.1.6/657900117481816066.sst b/pkg/ccl/backupccl/testdata/restore_old_versions/privileges/v20.1.6/657900117481816066.sst new file mode 100644 index 0000000000000000000000000000000000000000..a9878518f0971a27bb468a705dc2c8109048088d GIT binary patch literal 1118 zcmaJ=O>7%Q6rNdo>s@D^I!+v+NUa=`LrKz60)|+G^gu{Rkt$Fd1Qn{b>)jV;)!Er) zW*plEwW)+4P!JLapaS)bxN_lu=m7++)B``awyHRAXfLRj9;_-UGwY;qfp^;X=DqKI z?|W}A8CTKM=4T(2M9cR90tgTu>7D*!=hxo+k#DYU+vD}c`V$Cmq0w`vu3zh1JZ58r zOruu)<;ZqsxN82^%PiAx%}HnRk4dv+&Nu9%gdUMiZ0vhOSvP84rlxpCok`Oxbh6<&TeGc;j-nZ47i zUpTs=NIZAv(pUECh&%V=`-c- z^QbF?bd?-Hnlx@`AKqlKR!iGhF1rC-&toXh8o+(wQdE>5#GO7Op}?AQ9oYpO^T?BA zw+LBf++TrKHx#5LfNKh9FpD*_1B0V@!=o*=DM13DiZC!g*4J(g;s|&XNSpxoSRl}F zCZ}X)ElLx~t^Js_B9hLS?pdf_Q?;Z@WFs#f0JX=>GLFSfVI_JH67 zQYB6Zj`SxW@mqT0(*Mx|j5kTk1u-Yjc;=h=zOhMX^83-e(b^F@&y!RrLO+qEcROEC zPd{$gDH+<;YWG9MAIregXD{xN#>6q1`#m(Wc7FFWJNZJUgV-bybC#uia1J&R2sL&{ zr)qS(Z!s&9cwiB?kTAz12~4O22Fw*1c-ZAxni<#?1GX!q10z&#b$NmI_cW&NL-AKQ zn4OHZ%u~g7c`9VVMB)dmHG<1&#tnDc#MsPe9`A**9>Q;i1MvD%Hh)ja&xjj<9M72q|W1}JxEYjfyytu-$ z+=B5VkXDuY!LGHI=mD2GF3Pe5B+d&e)(IpeXwei*517utaFtlHP^qEnDwi^ICYYfr zPgzN^T)U;s8U^Ky06fl6tm(lDVnV4t!2&H4u&!M6P@=`!=sBPohR2A`6i;E)qA~Z# z_jU!tmmtAk+c#UN?R}lAWGj--jm{p7uO656#m$GM&@X-Wf28aLxGLT+=kOwQnvIbN z-kbEgmw)5OjS406mGR@wup3T;^j@D`B+Y$0{(Lgk-nin+{$tx`?~gk9lV5)TC@m*s literal 0 HcmV?d00001 diff --git a/pkg/ccl/backupccl/testdata/restore_old_versions/privileges/v20.1.6/657900117481881604.sst b/pkg/ccl/backupccl/testdata/restore_old_versions/privileges/v20.1.6/657900117481881604.sst new file mode 100644 index 0000000000000000000000000000000000000000..fa68149d7e22c7fb9e1fd1748b2eb73772a54dac GIT binary patch literal 952 zcmah|J8u**5VkLm+k0@yN*I-J^W(!CAIkNa@zayd1+@I_=d~qVK+drLT3^&BR4%+j^Iu^Psz#c>VBa3!Bgnx@%w5QZwX&Np4hMRmi7TeIG}wY=64a0p`k&h6Ee)wPDP z4innh`qEm%O&T&D+c3&37#}$@mW4j-vbg*c2kEC|KBZK}8mR9?EKdLhCCsR$#PB27 zDpfEDuuc{?r9r{<0D5kINkwRk4V@Z5+CoL%hZjnj|?J1zzDE7n_YIFFIijG^9Aq<4$3o=#= z7z%1Q0o?|oQqWW~gQiD{PIcK4BDFf0z_OE&e8hb123Lz1q}3cup*n$GO&eMc6LR&D zD^P-gvH7C=63rF|*TJcwY3$I6qzMdKlZ<`T!69o8U^K9E`yP-OgZAapfR_Wd0 z(Pmzs-Mp6z{ls_wN6K#CRzbf~LZoK$n b>@w;3*=O}*ys>rFNB(0w7ssc~&er$eT+}W- literal 0 HcmV?d00001 diff --git a/pkg/ccl/backupccl/testdata/restore_old_versions/privileges/v20.1.6/657900117481914372.sst b/pkg/ccl/backupccl/testdata/restore_old_versions/privileges/v20.1.6/657900117481914372.sst new file mode 100644 index 0000000000000000000000000000000000000000..f521799b4946c241d424eb7e7f4339d4e5ad6978 GIT binary patch literal 1063 zcmaJ=J8u**5Vr4f-0tSMyzUT02?Ugqm;g~Iq7=Ll6>p>fS$ylsWr^2bY+vq<0+48U z7En+j8YHMlL4^c}iUuJe3K}TjFVGI-(jy-uACG>zvUE2HDuwIU_6?ztop&nbd{9|+#LyZC5(@?`V%jOVnS;GJRWKK~ z$Xv_~s94m;2M2I-UfR?JX#W-VdJB={jYN<+n#8#R8b>UWQQq9qxmqB zzhkB1Dg|1{QXCaDP1^%r7|PUGUpFll*$iv3(bHpFC#FU?ECQe2zHP_kj;RrCY)`1S zr^ly8EYpz1kqK*UE3S)N(1wLBY_nqVCl11~WG=-oVFlDpA|_4%d47&jLy3+rz!U6u zW8h${E|#W7-g*yORscbHsI>{L96;(^)|?OLg>-5;8!UD$f^vLv*7Le?N4Tsj3d*?E z=}0(;x|v7*YAjVk4OuQiUKMwqz^NN+;ugh0K;LMnW=<*}H38Dq;{u#i0FC54JUR7_&VglONdv z$(dc&?4>&zRg25}VAW7GvfYWG2`rBY`u4inrcLZBaB$c9)G;gWF4++5SdAZ2GTr_A zYF}30-n=Ih`iPtUkCZcBDW_;N!{J1@oht>)DmZuC37`D|XAeJniTYR8JGJ919Lw)5 f{!v)T8f06V?}{6r;I%IFe>?g8@{Q5P{*S)^u>4H@ literal 0 HcmV?d00001 diff --git a/pkg/ccl/backupccl/testdata/restore_old_versions/privileges/v20.1.6/657900117482045442.sst b/pkg/ccl/backupccl/testdata/restore_old_versions/privileges/v20.1.6/657900117482045442.sst new file mode 100644 index 0000000000000000000000000000000000000000..7b2f468a506c9e11d3a45ebe199ccdb5472bd6e1 GIT binary patch literal 968 zcmah|zi$&U6t>f*xuhZeMT=A+$WSq$)`TCTLzMwa+64)eM(u#Alke^|vFfw0w)3k( zh!MmBLSkkG2H0Uhh?T#f8_Z0VSUP}xNq`06>E82u-}k-m`C2d0#q6ucvowmCKzMe& zUc1q#)$iP{A^Zp}JlK8l^~3v(JVprN%<`LO-y4Oo3t1(l*(RuX^r@4dA(i9LA2$oO z0;Of&E^Sv)A@dr|{2UxqAKjrJmHSW8q!+3vAV!mz_EHLmlku1s@u8)0CBuDfs{(F6 zNI*42#L$2PVIqPaG5rJ_?BX;|6zs7c*<;**Vxn1DoWtFgw59vd|10fHXG6)`u^@Xi zX1q^W%|Wri@%k zzK9i2$3zT^0eN{GQA3Glhp>~M#K6H=TkMn?dFMRn+4%+KzShRKZUAX=8Mqh@gmh-P zvzQz2fO6wxY~)Sifo~}&DwJ?b^fLpyFLK#7Xyb_K2`p9s55@c0Ic)+D&yh6Sj(lp$RVf{coV^ vm!o%Rc5L*x6U%1R*zxp9)-A{VKdgT5;L()nV7GHG>mOX-O?5s>P!*{B%xTQ4MQN)l!O3nftjYxOv|`*k}?>a?#V#= zPdnXN-R!=7@9lfP_x3%#_V`Bd*qT@S@7Xd+YDyLX*asY2TF-sdxxK*#felWrKW$_# zq!-%sNFacah{V)`(|_t`1}}X5meo-~Cq=>{Z%9gz4Qo=nB%>hG6G=@~jnvzdJ(JPs z6v6gh+!|ZBV9^Saf_5m7IYlKfJ~^eoA5@2d^HTh~jd2M8^enUg`IN16fdH(JJX3S6 zCt;X5H?7=D8MZe{cgtyR7=RzcVPj*gpo%j{QAx_h2W8DUZm9q)6EfT^0pcd<89za9J} z&e6MpcKdiiMyzgNpL9bC=&Xj?G^B52+f-8n=VO^rm4UOadF)ejxji)o9d zc1WTfrZX6v%2eP4P01So3v%a16zSOr@@tUwWyhccU!3rL5~SO@Dp;5UjzM@Df=LK&K@zU7ybVDQ z0d^7KM_AeQLwAl^CI`m|aEt(_h)2^cG{j-gTvOBRt@3%<7GIUGzQtEx)57}mJX0&# z+UCXvc3qLXm3_M2SI0Is`RW^2wXnXr=0=}yW&NrrStnP@R<*E=wQS3p7N56)_0>0c z+3dVRr=FclQ`4`X{PN0s7*5wC8Cw8XI{~TEkIzBK{B_w2aAp9s4S>~WJbMYIKR@TQ zhBM?p_t#e}=*DSwm+bY1kp1e1m$&|L^#n7_&;Ld31d%mzXzYiiqjx=Bcy@vSiSFwSf{@snfUy)rXS+#Dy!EV>zg6X-SL zR1ewnO_X4IM&CVb`RY8DD!dUT>}Nmy{kE&#C~#zKsf`lvj1!mp!J2W`Zi4w<%JN9V zIJIl~3vYQ=H?XAbp(-X{JM^=GWPfqAe<%@KNxi=PglmE@KG}T@zr@jjKC9!UcABfM zs(EU46T7_1S5;lr;vIcnWPOcnwxWtiRs+0TmY-jcFYtlQk#KgyDf{1k_2z}M2YRWy ze;>2h7+0LT=;Qjj<`F9m5^3tl(GxS%Ut&PhlUF?jsLa32Da!^dl}^0hpGmww0O=(k z;Kc_6xz0q6CNo~5?C$hk29cb*o1h1YnbNq2^NG^9JjYCu_2Dc_vQB+oMkeBPt%NJ= z@7zfH&Ax@z&f$}{y|?d8e9;8g9)UC!R13I*LasECYIiv~y!NCgxty+od}mR9fs-vL zEpvIwoX*Ug0^TWj+IW|%v@L)FsHDJC%DbGN;-X@=Qw)g3&JrgoEfPy|ZNWf*^>Brp z(-kjEN~DwJL!!rB7FScVvf8R48Sy$=F;8u4lL8Vi=LwR6pILKiXNkw}apy&p&5GKg zy{>xy=$TuWU*7P{{++F5i%I6d z8?EAwR&wU5bSbMDhjxfsnhEN5wA$9x0ShYVf!XmD_JtR2S@hWPrrE+I{E-z zIMzx6KR>xsVD9W4{>L_fdLw$!V&eo7jP?u3r~7Xo*z?s^0i>-GNK4bsVA>Ku%>>E+ zX9_{G;>EPn{hP+>lSb=4iZ%#0e!e&8DI%Ht?%ZGR2$HvcDg6Q4Y7S`ro;d%?4*(sv z56u5|c7UVIgF~?Ry~;Bno(gE9z!^Fh;yWE%{4w8b1SKgCIt$3c)h4i6$6Nx=%*~vD zkr%mGHRU8lM4g<`6-K7obUO^=5-fFcZIWyt4R;!6dJ^o|sLF`jjJp1VOxY~~RgQ!d zu8j{#au+9wriE6TN)Cm2jW<*+fG^WjKCsarP#@43>l|F*bxBoP6h0j8O2SoJ4I6l` zcVJu84!8VEBbDTs=)nw{D1j zlZ?3}<==Fqm_0)@S!@i{5N{XF^SSE<_uuc?%A7BAh z7R{z>+A@n#3hsZge86aEd;n|b6h4IRUqR3Jns*yr98LupnaBD%9|pHtjVPi|kef6$ zT=U?lHYVS-WO)qeX7l#{!qQ4HgvNKo>@Y#NNiZ}qi-NhmvcN>%d>>%d+KK0Y`44C| pzr*-GY!7zucg&3CA=mkt;hPr|Gu~!WP164!Pfz!3&x$;GUuYaf7@xW3yxGe(X_IVPt}$LEN*}zTvANVVB#3{iP#;Q7E23qc-TiX6&Ft(t zGn>0>QS_l;8=)$wh*oQ*t)L=QUj)TI)CWP|d=Xzn6hsi8q@YEe*-Kkr^n2RxoA2-M z_kH_G>0Q*=cKXDKgjSpighwEkX$FlBC_Rnvb7=hO!>{~scFV60MyOmm^ThV?^VOy^ zbMm7-2Vd~-MxHZsV&jYRZ-0Ila_bYhYJ?!PWPI9NBu}km-lBWTeej*~;^QwbU5;*f z9(mQ;N&Hafmml4|Xt*{3jCK$?$Y1s5TyOv6$K|=PZu8SiyR+ zJVB0pxYcXA&;CAjq}&`ketT=*Z)ct6*zoE1>MxC8G0*`j5IJ?>Ta#$^_^pS^bM4pP zI&*BtMO$$VzrU|N`{f>F#ms^5_rb-_P<6wbX!GBFL07E3ctn-ku53YrB$QD=wW0~@ zUI(E>Vo@ljWi&M)({M!@uYj8m8ZZSB)hysZV-Z1@>RtvmZEX|IBrL~Wx*T&2G8V0! zwNcz@r^a;!djB$$P317dd=omZjzvs z;qYO>vaB~sd_T>!(N)Lvl6L$c)@q?KJu_eDunK%(*PXj(chA?AVM{}+x3F`*ZYK4k zIMja7)1a*7sM3*d&BiMhH|!wnl#z9DTr7dvN>s-QAmWrFrWsZ7QMihoK@1$U!NrWJ z5Sb*KPG>%sp&ka+GM!-$OAi^|)2$3P2fNTLZ zcOaRF6dK)7F|doA^Gr)Jj76T%V#b1Zh209aTOWfeyTjPTw4h;fLIFQA0V$8nJrzv% zG4d+w`(V(JEHv4vUUuYaf7~i=|yuHntrb!xN{C8rwfb@pTCD-H#c`z|a8)8wL8rxFN+1>9h+sw|c zGqac5Ad)AsNEL-fP#?tNqfijB;-fE06%`~t6bS|KQN)5i6pS-_$<-H~hvVj(Z+_qJ z`~ALIjuLC(1uv$s(h@9{BuKWECEt51b7i z5NAEVLTr&Q=AYq#8#($~eielDSYvqiuf5B;@2}nbta}KFo(HP}91ijTb?0{9nUxOe z_~1uhS%uNc^aQEY%hlpUwGPF}(v)4S68l840?ss4%GElVo*w>D!+vn%%3fN*rpMNR zDdHjb1Yqhpzyx%NM_1qKl-Pdvn{BHw=Wtpq6&IFQN|VvX;?k1~-l@{*=DD(r?egL! zGC%k9xwYl3Q}d<9*^|r7^CzFboH%`?6I?6Bf6zws$_<@6uQgJ*t4jJ?VG$Zk4|7 zT>JIsFE3aHtKT#S`_QnFH`-EM|MZueN(x{5lelzs?2q3JEB&T%bicifAJUz}8}Zv? zMvv)m*Tzy{pR^K-wNfe5V``HWHAcI*vRJ4ro2oz9CIVb6iA@1}%!L+~aR?fAqz{E0 z>Rt=iJ*uowt(Qgy(B^ueQa2#}SJ>?9b^;pt4A+V8(HMKKW|*~a z!&okJ9|xgzIm1X8#sg-p7KD1%#HQjBY}RZ~irM1$DI6X@(jdFsT}OkPNZ35|HR zG?Q1I)?Ck#H9M9dbYx3Nx2D4?%VYQAz;u_cPRZsFhvYF}>G@!o*_2Bp!NS{u-E_7G z0R^daQEdvte8hy75`Ym}6GGOM9Do}%u=O5ZVu4mmuaQjGfzK(mGkZ{5xCGU(dQq#5 zh$*m!TsN}!qk#KF;=|!EkD;~&T3t`zNKK%oVDxq&&FtacA=D%^Qcel%0#$^8IcgeR z!|X*q@CM)@1WuSMjDd7U$<885L&?n}h($h5dQ5y5D3?L%g>er=ckT#McN$Psn2^YC zs0#sg)q_u*ZfkTbb1)A|4Ne?|jv4XcpoDdFLD!ZsR--^cABzjCO5C|1V(zL<)t4lk zKRjAY?#IWUN`!t+SO1Tci)Jpx$!5Ysi|`%FS*f-Pdb^)IjpXOq-(76ON+-R;*$qorH_0G>Rx6951J literal 0 HcmV?d00001 diff --git a/pkg/ccl/backupccl/testdata/restore_old_versions/privileges/v20.1.6/657900117486469124.sst b/pkg/ccl/backupccl/testdata/restore_old_versions/privileges/v20.1.6/657900117486469124.sst new file mode 100644 index 0000000000000000000000000000000000000000..780aa8a82a06032ec05cfbc2c40deb027b089262 GIT binary patch literal 938 zcmah|O>Yx15Vf1O*`y);qD86@L?Qt#}z?Q52#vs*VELh*IsO=$@YM_ zfk5Jb5FF`GK;pOb1c&~Q9$>slS}uq=dB!tu=Do2=d*a*sIW97(37xaXctYQi#n-!E zPEX(N)F>I)jhU^i5{_ly=Iu|vNPX;>%>Eu2Sv|k|k)M1fQ&DO~!n|V{@1KK>MN*F( z(jK=aJ#d(nC_J!)I}ptCL;;gpK%e;{hXA`YLvsszvd{LU@?fRztSrsb-oC-q11SCq zM>7+tQQb_lJ)TKbFe$=-)p~R}%em#=*c6*t!_)mZHKSOgI%&ZWTOfOEPcbm(1RySG-4nb|+ySsLOZKLHpVnV0bT-j(L(^$yV$4TMAg~*ZfA`W59 zlKLVJIg(k6%%Y^gea2sY3eLob@51KmncQg71!0Ff!s9luX%m#DEB_iR{1q6IqGKoD0t z)^%XR2&6eq!(dmNi}aAIE-uP30Se~@of-jxL`|B3?Eo`5SgwU5^OY*9uDeR*-bT|@ zcQaN}EZ6R6qe?-0D*=zw6l*%LjF?cWPq4r!0nV3;9!RuM9X_4`1_U1#o`{d^zYc?nh literal 0 HcmV?d00001 diff --git a/pkg/ccl/backupccl/testdata/restore_old_versions/privileges/v20.1.6/657900117486829572.sst b/pkg/ccl/backupccl/testdata/restore_old_versions/privileges/v20.1.6/657900117486829572.sst new file mode 100644 index 0000000000000000000000000000000000000000..cad6a9ec293b0ee3c844512a2e0296a7c111f94d GIT binary patch literal 1062 zcmaJ=&5IOA6t9|(O!sU%&bqE2>qOQdG6yYncXbcTf|$ezhDD5EAV^cw{dTsox~i?J z*_{!QIpmNvL_A1HFcBn>97GU2coCvEFN(RTAz=Oh5poeCsIPi_r89w z`cdI?va#{q@;B#~$%b<$zyIs!=1rdxpPq&6r%tin@AnUE!dJiWQOUdbd%0OWO1>n8 z(Z=%J6sdoGTVE<3nj)m^PnEX(fKc$qiRoJt_*?WYHijoljUVSGN$}*}j@pt<|E%bRyJ2pV-y^TM-#Nws)^ z48I;2x#sZvkUZbI?*SR|A|1DwH7w=bx8oGNM6Nnydr7P4ltETY;2+lF24ZG;O8`q` z3|(e>8Mw4#b((3IPr7VA5f*eJ+k3`F=+c5h>M7{`2lndaNQq7=**s4Z(PK&MI;>Q^ z^=Za6w@OD?Oe-EOgppdsg31RPj7gOILh>x@jd)?GGK;=h!d}!;*h;LK-L-dWW?aA` zh}p@B>HX6)~N>qnGb+v7e;Z+dej?Guc zQQU*7Bje*Oaa619d3Q}#8Z)n;||NhCd~O1qJ?Tb0_7)V9(r!47Sb z`rEUzb6D1D&9*tLIaXU{HkTpGesi9G!K^l>aN90C+vQ?@Nj>+E$->xakEMP+zgDM}i=X-;!Lyt*P)&d)agSnJr-V(*PQ!&8!Il#j-@EYPQR2YJ!uXm+xjUZ9 z-?_}*wm9MAn+()=Sba5VqJTce_7)o-?N9vljg;bXTGsyTP)TTIM0z3 zO9Pcr4NuLf2t2#Zo@3>7yleQnXZIeTu|BP85#Ddi$;xE!v8CsmcdY&TqQC0PA?ucI zChvMTE#|hfuNQticFN)Ww)$kRbK|d7eQPYm_h-&{xevliF^1!*nHzy;b=Vw^Or6k) zMv1%squRB#U){2e}xo>99_&Py17c^Pfu`dr0`{nYrJL-8N^) zwYAc$_!0lCb4`C52ovL{wmf&|Pc7fSI@f*A-pTfrH^o)g9-aHHee|&VW}O8rGIN4lYu_sAlP6_7=fU~Hw&UBUaRX_~7(SINX3k!$Qd`%0@$!SBwyM3#vZ= zou~SvpYD%AFvcZ$OWkE%`JsR)nh-Krp%`8B$C2ANgoQB(NuyQgiF0=7scWpz*b0bl z@wAnIn+ZtCqTlcKmdVDLD5@ApUN(3KLc@IC6A)FdI!AUE?Cs%Y|8125?-vCWxbc)h z58MwaHFEoSr7FwSxw*MhZ@6i~ty8bNX$lJUG}ueMb82;^NRyp12IWG5p=e@=7sUsv zet(=7ro-V%T(VAu2B)ffWgsH!j;DNMNeOvoq`FySV`L|U*`0J`O@(>OeZ zLpc;uN{f32_SVWt zFyu6vwc;@x_R)$*m}gMPg}b09W#k|U_mhw;mr7(~Vif0=uB%9TLcr^u7Z9C3smd)_ zu0}3ERYmsm{(#(NrdVOD%O`d#O9ms3_-c>D3u=ypdsT5cdUYy&QkhSbW!ez^o)pCV zkLpSj3I!U7sMVrJcHbI?8(X9)*-jC$}K5m)jYO@e7!uDXTJDh26M zD=DKZ#sn2(f^>$mvjs3=ECQy3QIo-9Xa#t;QuloVzJs1nkL2S0ZpkYflcS<(2+BmX z>~Z_6CtW6!hr;;amfJ5|chex{K_LEspmYoI6hS8%lTgR8F>rFsoGv7C4aKyW3S3&wwcr7i*TQR})IX(Eev} zrAl=7^4a8-)QGeYU+BSBlwovKB?y$pH5JBdk4cF5pbvWTh&)5Ur%;dOk}4`huU|GA zyNV9K((Wj)cjtx_^+Qrgx#;pEj?@6@yg_+e0wv0-5+9vcQTXVM4 zDMb-;2ojZW^>QVWSVBqk9{2?>9~v=AKp0aQDDk*mG^#~ijTNIpN+BO9Dls9IQ1Mn( z(sDM;VK&qfs~jU!gI1$Hi);mkZWZf~aCjWoQ(CIj>h+K)r~Te!hy0=)?lP@Anli;{ z;jOdM;*v$vLIfyY>6om{`kg&Z>?@n5kZ-KCwq zk`5V*p1QBdh_Dg7e+V06G-MR3zdc;kY!EsVudr>by}KV+y3tVotB|p z`!+7jxA=d_%4b_HY>#9mmrO!GndE^&dgtc@USBgjpKaQ8e!}^S2R@hrik}9MDwC`k z*368YBx*j#W+2(l^yOI2I)=>}V`WX&oLtVH%d%#Rm1j%rr98*wl)6Nhh<*Uf;W^fx zot1541(%S`I#@9$OK_z3Ds#C^_KeI7mh0_tdjkthe1%}QYqwtsShmlU?b%txJ=th zfUO4fLk){&`d*!|dF>IpdDYIev0K3iDvQ3m=7rDCuR2=ai=muOKH!&}PBQ@Maa6;l f(@SIMzdm-V0atd~(a!bv{>IMbXWIS+wgd1VYf6T< literal 0 HcmV?d00001 diff --git a/pkg/sql/catalog/descpb/privilege.go b/pkg/sql/catalog/descpb/privilege.go index 1b741e7802c4..c1a3b5c6f96c 100644 --- a/pkg/sql/catalog/descpb/privilege.go +++ b/pkg/sql/catalog/descpb/privilege.go @@ -220,7 +220,6 @@ func MaybeFixUsagePrivForTablesAndDBs(ptr **PrivilegeDescriptor) bool { *ptr = &PrivilegeDescriptor{} } p := *ptr - modified := false if p.Version > InitialVersion { // InitialVersion is for descriptors that were created in versions 20.1 and @@ -231,6 +230,7 @@ func MaybeFixUsagePrivForTablesAndDBs(ptr **PrivilegeDescriptor) bool { return false } + modified := false for i := range p.Users { // Users is a slice of values, we need pointers to make them mutable. userPrivileges := &p.Users[i] diff --git a/pkg/sql/catalog/descpb/privilege_test.go b/pkg/sql/catalog/descpb/privilege_test.go index c89e81cb8c9c..c69f4123aace 100644 --- a/pkg/sql/catalog/descpb/privilege_test.go +++ b/pkg/sql/catalog/descpb/privilege_test.go @@ -703,6 +703,19 @@ func TestMaybeFixUsageAndZoneConfigPrivilege(t *testing.T) { "(InitialVersion) with USAGE should have the privilege converted to ZONECONFIG.", true, }, + { + userPrivileges{ + fooUser: privilege.List{privilege.ALL}, + }, + false, + userPrivileges{ + fooUser: privilege.List{privilege.ALL}, + }, + privilege.Table, + InitialVersion, + "ALL should stay as ALL", + true, + }, { userPrivileges{ fooUser: privilege.List{privilege.USAGE},