sql: fix Usage privilege on Tables/DBs after upgrading from 20.1

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

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{}

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;

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
 }
 

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
+
+	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
+	}
+
+	modified := 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