From 14917c0975ed5975041f7a24a5c21d54c505a15a Mon Sep 17 00:00:00 2001 From: Faizan Qazi Date: Wed, 17 Apr 2024 15:14:54 +0000 Subject: [PATCH] catalog: add descriptor repair to remove missing roles Previously, we had a bug that could lead to descriptors having privileages to roles that no longer exist. This could lead to certain commands like SHOW GRANTS breaking. To address this, this patch will add descirptor repair logic to automatically clean up oprhaned privileges. Fixes: #122552 Release note (bug fix): Add automated clean up / validation for dropped roles inside descriptors. --- pkg/sql/catalog/BUILD.bazel | 1 + .../catalog/dbdesc/database_desc_builder.go | 19 +++++++++ pkg/sql/catalog/dbdesc/database_test.go | 13 ++++++- pkg/sql/catalog/descriptor.go | 5 +++ pkg/sql/catalog/funcdesc/func_desc_builder.go | 19 +++++++++ pkg/sql/catalog/funcdesc/func_desc_test.go | 19 +++++---- .../catalog/post_deserialization_changes.go | 4 ++ pkg/sql/catalog/schemadesc/BUILD.bazel | 1 + .../catalog/schemadesc/schema_desc_builder.go | 19 +++++++++ .../catalog/schemadesc/schema_desc_test.go | 39 +++++++++++++++++++ pkg/sql/catalog/tabledesc/BUILD.bazel | 1 + .../catalog/tabledesc/table_desc_builder.go | 21 ++++++++++ pkg/sql/catalog/tabledesc/table_desc_test.go | 19 ++++++++- pkg/sql/catalog/typedesc/BUILD.bazel | 1 + pkg/sql/catalog/typedesc/type_desc_builder.go | 21 ++++++++++ pkg/sql/catalog/typedesc/type_desc_test.go | 14 ++++++- pkg/sql/catalog/validate.go | 20 ++++++++++ pkg/sql/crdb_internal.go | 23 +++++++++++ pkg/sql/evalcatalog/BUILD.bazel | 1 + pkg/sql/evalcatalog/pg_updatable.go | 5 +++ .../testdata/logic_test/crdb_internal_catalog | 2 +- pkg/sql/sem/builtins/builtins.go | 39 ++++++++++++++++++- pkg/sql/sem/builtins/fixed_oids.go | 2 +- pkg/sql/sem/eval/deps.go | 2 + 24 files changed, 294 insertions(+), 16 deletions(-) diff --git a/pkg/sql/catalog/BUILD.bazel b/pkg/sql/catalog/BUILD.bazel index 7d21d60b46df..cdee3c3216a0 100644 --- a/pkg/sql/catalog/BUILD.bazel +++ b/pkg/sql/catalog/BUILD.bazel @@ -27,6 +27,7 @@ go_library( "//pkg/keys", "//pkg/kv", "//pkg/roachpb", + "//pkg/security/username", "//pkg/server/telemetry", "//pkg/sql/catalog/catenumpb", "//pkg/sql/catalog/catpb", diff --git a/pkg/sql/catalog/dbdesc/database_desc_builder.go b/pkg/sql/catalog/dbdesc/database_desc_builder.go index 96a4fca51907..8ff64312cebf 100644 --- a/pkg/sql/catalog/dbdesc/database_desc_builder.go +++ b/pkg/sql/catalog/dbdesc/database_desc_builder.go @@ -183,6 +183,25 @@ func (ddb *databaseDescriptorBuilder) StripDanglingBackReferences( return nil } +// StripNonExistentRoles implements the catalog.DescriptorBuilder +// interface. +func (ddb *databaseDescriptorBuilder) StripNonExistentRoles( + roleExists func(role username.SQLUsername) bool, +) error { + newPrivs := make([]catpb.UserPrivileges, 0, len(ddb.maybeModified.Privileges.Users)) + for _, priv := range ddb.maybeModified.Privileges.Users { + exists := roleExists(priv.UserProto.Decode()) + if exists { + newPrivs = append(newPrivs, priv) + } + } + if len(newPrivs) != len(ddb.maybeModified.Privileges.Users) { + ddb.maybeModified.Privileges.Users = newPrivs + ddb.changes.Add(catalog.StrippedNonExistentRoles) + } + return nil +} + // SetRawBytesInStorage implements the catalog.DescriptorBuilder interface. func (ddb *databaseDescriptorBuilder) SetRawBytesInStorage(rawBytes []byte) { ddb.rawBytesInStorage = append([]byte(nil), rawBytes...) // deep-copy diff --git a/pkg/sql/catalog/dbdesc/database_test.go b/pkg/sql/catalog/dbdesc/database_test.go index 19677aeaf5e2..bd962c6c895b 100644 --- a/pkg/sql/catalog/dbdesc/database_test.go +++ b/pkg/sql/catalog/dbdesc/database_test.go @@ -389,13 +389,18 @@ func TestMaybeConvertIncompatibleDBPrivilegesToDefaultPrivileges(t *testing.T) { } } -func TestStripDanglingBackReferences(t *testing.T) { +func TestStripDanglingBackReferencesAndRoles(t *testing.T) { type testCase struct { name string input, expectedOutput descpb.DatabaseDescriptor validIDs catalog.DescriptorIDSet } + badPrivilege := catpb.NewBaseDatabasePrivilegeDescriptor(username.RootUserName()) + goodPrivilege := catpb.NewBaseDatabasePrivilegeDescriptor(username.RootUserName()) + badPrivilege.Users = append(badPrivilege.Users, catpb.UserPrivileges{ + UserProto: username.TestUserName().EncodeProto(), + }) testData := []testCase{ { name: "schema", @@ -406,6 +411,7 @@ func TestStripDanglingBackReferences(t *testing.T) { "bar": {ID: 101}, "baz": {ID: 12345}, }, + Privileges: badPrivilege, }, expectedOutput: descpb.DatabaseDescriptor{ Name: "foo", @@ -413,6 +419,7 @@ func TestStripDanglingBackReferences(t *testing.T) { Schemas: map[string]descpb.DatabaseDescriptor_SchemaInfo{ "bar": {ID: 101}, }, + Privileges: goodPrivilege, }, validIDs: catalog.MakeDescriptorIDSet(100, 101), }, @@ -427,8 +434,12 @@ func TestStripDanglingBackReferences(t *testing.T) { require.NoError(t, b.StripDanglingBackReferences(test.validIDs.Contains, func(id jobspb.JobID) bool { return false })) + require.NoError(t, b.StripNonExistentRoles(func(role username.SQLUsername) bool { + return role.IsAdminRole() || role.IsPublicRole() || role.IsRootUser() + })) desc := b.BuildCreatedMutableDatabase() require.True(t, desc.GetPostDeserializationChanges().Contains(catalog.StrippedDanglingBackReferences)) + require.True(t, desc.GetPostDeserializationChanges().Contains(catalog.StrippedNonExistentRoles)) require.Equal(t, out.BuildCreatedMutableDatabase().DatabaseDesc(), desc.DatabaseDesc()) }) } diff --git a/pkg/sql/catalog/descriptor.go b/pkg/sql/catalog/descriptor.go index 26acd3568db0..b7f881148fe8 100644 --- a/pkg/sql/catalog/descriptor.go +++ b/pkg/sql/catalog/descriptor.go @@ -17,6 +17,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/jobs/jobspb" "github.com/cockroachdb/cockroach/pkg/keys" "github.com/cockroachdb/cockroach/pkg/roachpb" + "github.com/cockroachdb/cockroach/pkg/security/username" "github.com/cockroachdb/cockroach/pkg/sql/catalog/catenumpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" @@ -108,6 +109,10 @@ type DescriptorBuilder interface { nonTerminalJobIDMightExist func(id jobspb.JobID) bool, ) error + // StripNonExistentRoles removes any privileges granted to roles that + // don't exist. + StripNonExistentRoles(roleExists func(role username.SQLUsername) bool) error + // SetRawBytesInStorage sets `rawBytesInStorage` field by deep-copying `rawBytes`. SetRawBytesInStorage(rawBytes []byte) diff --git a/pkg/sql/catalog/funcdesc/func_desc_builder.go b/pkg/sql/catalog/funcdesc/func_desc_builder.go index fb76c80a8c99..368f8abf5b23 100644 --- a/pkg/sql/catalog/funcdesc/func_desc_builder.go +++ b/pkg/sql/catalog/funcdesc/func_desc_builder.go @@ -151,6 +151,25 @@ func (fdb *functionDescriptorBuilder) StripDanglingBackReferences( return nil } +// StripNonExistentRoles implements the catalog.DescriptorBuilder +// interface. +func (fdb *functionDescriptorBuilder) StripNonExistentRoles( + roleExists func(role username.SQLUsername) bool, +) error { + newPrivs := make([]catpb.UserPrivileges, 0, len(fdb.maybeModified.Privileges.Users)) + for _, priv := range fdb.maybeModified.Privileges.Users { + exists := roleExists(priv.UserProto.Decode()) + if exists { + newPrivs = append(newPrivs, priv) + } + } + if len(newPrivs) != len(fdb.maybeModified.Privileges.Users) { + fdb.maybeModified.Privileges.Users = newPrivs + fdb.changes.Add(catalog.StrippedNonExistentRoles) + } + return nil +} + // SetRawBytesInStorage implements the catalog.DescriptorBuilder interface. func (fdb *functionDescriptorBuilder) SetRawBytesInStorage(rawBytes []byte) { fdb.rawBytesInStorage = append([]byte(nil), rawBytes...) // deep-copy diff --git a/pkg/sql/catalog/funcdesc/func_desc_test.go b/pkg/sql/catalog/funcdesc/func_desc_test.go index 7e736b12c952..e3b11ff5ef44 100644 --- a/pkg/sql/catalog/funcdesc/func_desc_test.go +++ b/pkg/sql/catalog/funcdesc/func_desc_test.go @@ -804,13 +804,18 @@ func TestToOverload(t *testing.T) { } } -func TestStripDanglingBackReferences(t *testing.T) { +func TestStripDanglingBackReferencesAndRoles(t *testing.T) { type testCase struct { name string input, expectedOutput descpb.FunctionDescriptor validIDs catalog.DescriptorIDSet } + badPrivilege := catpb.NewBaseDatabasePrivilegeDescriptor(username.RootUserName()) + goodPrivilege := catpb.NewBaseDatabasePrivilegeDescriptor(username.RootUserName()) + badPrivilege.Users = append(badPrivilege.Users, catpb.UserPrivileges{ + UserProto: username.TestUserName().EncodeProto(), + }) testData := []testCase{ { name: "depended on by", @@ -827,9 +832,7 @@ func TestStripDanglingBackReferences(t *testing.T) { ColumnIDs: []descpb.ColumnID{1}, }, }, - Privileges: &catpb.PrivilegeDescriptor{ - Version: catpb.Version23_2, - }, + Privileges: badPrivilege, }, expectedOutput: descpb.FunctionDescriptor{ Name: "foo", @@ -840,9 +843,7 @@ func TestStripDanglingBackReferences(t *testing.T) { ColumnIDs: []descpb.ColumnID{1}, }, }, - Privileges: &catpb.PrivilegeDescriptor{ - Version: catpb.Version23_2, - }, + Privileges: goodPrivilege, }, validIDs: catalog.MakeDescriptorIDSet(104, 105), }, @@ -857,8 +858,12 @@ func TestStripDanglingBackReferences(t *testing.T) { require.NoError(t, b.StripDanglingBackReferences(test.validIDs.Contains, func(id jobspb.JobID) bool { return false })) + require.NoError(t, b.StripNonExistentRoles(func(role username.SQLUsername) bool { + return role.IsAdminRole() || role.IsPublicRole() || role.IsRootUser() + })) desc := b.BuildCreatedMutableFunction() require.True(t, desc.GetPostDeserializationChanges().Contains(catalog.StrippedDanglingBackReferences)) + require.True(t, desc.GetPostDeserializationChanges().Contains(catalog.StrippedNonExistentRoles)) require.Equal(t, out.BuildCreatedMutableFunction().FuncDesc(), desc.FuncDesc()) }) } diff --git a/pkg/sql/catalog/post_deserialization_changes.go b/pkg/sql/catalog/post_deserialization_changes.go index 1c6b94fc57f6..13790a078fe9 100644 --- a/pkg/sql/catalog/post_deserialization_changes.go +++ b/pkg/sql/catalog/post_deserialization_changes.go @@ -140,4 +140,8 @@ const ( // FixedIncorrectForeignKeyOrigins indicates that foreign key origin / // reference IDs that should point to the current descriptor were fixed. FixedIncorrectForeignKeyOrigins + + // StrippedNonExistentRoles indicates that at least one role identified did + // not exist. + StrippedNonExistentRoles ) diff --git a/pkg/sql/catalog/schemadesc/BUILD.bazel b/pkg/sql/catalog/schemadesc/BUILD.bazel index cabcf4c93c4c..1b76cfc9783f 100644 --- a/pkg/sql/catalog/schemadesc/BUILD.bazel +++ b/pkg/sql/catalog/schemadesc/BUILD.bazel @@ -16,6 +16,7 @@ go_library( "//pkg/clusterversion", "//pkg/jobs/jobspb", "//pkg/keys", + "//pkg/security/username", "//pkg/sql/catalog", "//pkg/sql/catalog/catpb", "//pkg/sql/catalog/catprivilege", diff --git a/pkg/sql/catalog/schemadesc/schema_desc_builder.go b/pkg/sql/catalog/schemadesc/schema_desc_builder.go index 7cb61af31c1a..cbb7572b92c3 100644 --- a/pkg/sql/catalog/schemadesc/schema_desc_builder.go +++ b/pkg/sql/catalog/schemadesc/schema_desc_builder.go @@ -13,7 +13,9 @@ package schemadesc import ( "github.com/cockroachdb/cockroach/pkg/clusterversion" "github.com/cockroachdb/cockroach/pkg/jobs/jobspb" + "github.com/cockroachdb/cockroach/pkg/security/username" "github.com/cockroachdb/cockroach/pkg/sql/catalog" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog/catprivilege" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" "github.com/cockroachdb/cockroach/pkg/sql/privilege" @@ -138,6 +140,23 @@ func (sdb *schemaDescriptorBuilder) StripDanglingBackReferences( return nil } +func (sdb *schemaDescriptorBuilder) StripNonExistentRoles( + roleExists func(role username.SQLUsername) bool, +) error { + newPrivs := make([]catpb.UserPrivileges, 0, len(sdb.maybeModified.Privileges.Users)) + for _, priv := range sdb.maybeModified.Privileges.Users { + exists := roleExists(priv.UserProto.Decode()) + if exists { + newPrivs = append(newPrivs, priv) + } + } + if len(newPrivs) != len(sdb.maybeModified.Privileges.Users) { + sdb.maybeModified.Privileges.Users = newPrivs + sdb.changes.Add(catalog.StrippedNonExistentRoles) + } + return nil +} + // SetRawBytesInStorage implements the catalog.DescriptorBuilder interface. func (sdb *schemaDescriptorBuilder) SetRawBytesInStorage(rawBytes []byte) { sdb.rawBytesInStorage = append([]byte(nil), rawBytes...) // deep-copy diff --git a/pkg/sql/catalog/schemadesc/schema_desc_test.go b/pkg/sql/catalog/schemadesc/schema_desc_test.go index 916168239a47..6b3e405cfe30 100644 --- a/pkg/sql/catalog/schemadesc/schema_desc_test.go +++ b/pkg/sql/catalog/schemadesc/schema_desc_test.go @@ -243,3 +243,42 @@ func TestValidateCrossSchemaReferences(t *testing.T) { } } } + +func TestStripNonExistentRoles(t *testing.T) { + badPrivilege := catpb.NewBaseDatabasePrivilegeDescriptor(username.RootUserName()) + goodPrivilege := catpb.NewBaseDatabasePrivilegeDescriptor(username.RootUserName()) + badPrivilege.Users = append(badPrivilege.Users, catpb.UserPrivileges{ + UserProto: username.TestUserName().EncodeProto(), + }) + tests := []struct { + desc descpb.SchemaDescriptor + expDesc descpb.SchemaDescriptor + }{ + { // 0 + desc: descpb.SchemaDescriptor{ + ID: 52, + ParentID: 51, + Name: "schema1", + Privileges: badPrivilege, + }, + expDesc: descpb.SchemaDescriptor{ + ID: 52, + ParentID: 51, + Name: "schema1", + Privileges: goodPrivilege, + }, + }, + } + for _, test := range tests { + b := schemadesc.NewBuilder(&test.desc) + require.NoError(t, b.RunPostDeserializationChanges()) + out := schemadesc.NewBuilder(&test.expDesc) + require.NoError(t, out.RunPostDeserializationChanges()) + require.NoError(t, b.StripNonExistentRoles(func(role username.SQLUsername) bool { + return role.IsAdminRole() || role.IsPublicRole() || role.IsRootUser() + })) + desc := b.BuildCreatedMutableSchema() + require.True(t, desc.GetPostDeserializationChanges().Contains(catalog.StrippedNonExistentRoles)) + require.Equal(t, out.BuildCreatedMutableSchema().SchemaDesc(), desc.SchemaDesc()) + } +} diff --git a/pkg/sql/catalog/tabledesc/BUILD.bazel b/pkg/sql/catalog/tabledesc/BUILD.bazel index 8737e92b3f28..97b312099031 100644 --- a/pkg/sql/catalog/tabledesc/BUILD.bazel +++ b/pkg/sql/catalog/tabledesc/BUILD.bazel @@ -25,6 +25,7 @@ go_library( "//pkg/jobs/jobspb", "//pkg/keys", "//pkg/roachpb", + "//pkg/security/username", "//pkg/settings", "//pkg/sql/catalog", "//pkg/sql/catalog/catenumpb", diff --git a/pkg/sql/catalog/tabledesc/table_desc_builder.go b/pkg/sql/catalog/tabledesc/table_desc_builder.go index 3893e9a01d7c..dc8f119c6ef6 100644 --- a/pkg/sql/catalog/tabledesc/table_desc_builder.go +++ b/pkg/sql/catalog/tabledesc/table_desc_builder.go @@ -14,8 +14,10 @@ import ( "github.com/cockroachdb/cockroach/pkg/clusterversion" "github.com/cockroachdb/cockroach/pkg/jobs/jobspb" "github.com/cockroachdb/cockroach/pkg/keys" + "github.com/cockroachdb/cockroach/pkg/security/username" "github.com/cockroachdb/cockroach/pkg/sql/catalog" "github.com/cockroachdb/cockroach/pkg/sql/catalog/catenumpb" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog/catprivilege" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog/seqexpr" @@ -264,6 +266,25 @@ func (tdb *tableDescriptorBuilder) StripDanglingBackReferences( return nil } +// StripNonExistentRoles implements the catalog.DescriptorBuilder +// interface. +func (tdb *tableDescriptorBuilder) StripNonExistentRoles( + roleExists func(role username.SQLUsername) bool, +) error { + newPrivs := make([]catpb.UserPrivileges, 0, len(tdb.maybeModified.Privileges.Users)) + for _, priv := range tdb.maybeModified.Privileges.Users { + exists := roleExists(priv.UserProto.Decode()) + if exists { + newPrivs = append(newPrivs, priv) + } + } + if len(newPrivs) != len(tdb.maybeModified.Privileges.Users) { + tdb.maybeModified.Privileges.Users = newPrivs + tdb.changes.Add(catalog.StrippedNonExistentRoles) + } + return nil +} + // SetRawBytesInStorage implements the catalog.DescriptorBuilder interface. func (tdb *tableDescriptorBuilder) SetRawBytesInStorage(rawBytes []byte) { tdb.rawBytesInStorage = append([]byte(nil), rawBytes...) // deep-copy diff --git a/pkg/sql/catalog/tabledesc/table_desc_test.go b/pkg/sql/catalog/tabledesc/table_desc_test.go index 8d2923a8d90c..e60dad92e581 100644 --- a/pkg/sql/catalog/tabledesc/table_desc_test.go +++ b/pkg/sql/catalog/tabledesc/table_desc_test.go @@ -14,7 +14,9 @@ import ( "testing" "github.com/cockroachdb/cockroach/pkg/jobs/jobspb" + "github.com/cockroachdb/cockroach/pkg/security/username" "github.com/cockroachdb/cockroach/pkg/sql/catalog" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" "github.com/stretchr/testify/require" ) @@ -58,7 +60,7 @@ func (desc *Mutable) TestingSetClusterVersion(d descpb.TableDescriptor) { desc.original = makeImmutable(&d) } -func TestStripDanglingBackReferences(t *testing.T) { +func TestStripDanglingBackReferencesAndRoles(t *testing.T) { type testCase struct { name string input, expectedOutput descpb.TableDescriptor @@ -66,6 +68,11 @@ func TestStripDanglingBackReferences(t *testing.T) { validJobIDs map[jobspb.JobID]struct{} } + badPrivilege := catpb.NewBaseDatabasePrivilegeDescriptor(username.RootUserName()) + goodPrivilege := catpb.NewBaseDatabasePrivilegeDescriptor(username.RootUserName()) + badPrivilege.Users = append(badPrivilege.Users, catpb.UserPrivileges{ + UserProto: username.TestUserName().EncodeProto(), + }) testData := []testCase{ { name: "descriptor IDs", @@ -86,6 +93,7 @@ func TestStripDanglingBackReferences(t *testing.T) { {OriginTableID: 105, ReferencedTableID: 104}, }, ReplacementOf: descpb.TableDescriptor_Replacement{ID: 12345}, + Privileges: badPrivilege, }, expectedOutput: descpb.TableDescriptor{ Name: "foo", @@ -97,6 +105,7 @@ func TestStripDanglingBackReferences(t *testing.T) { InboundFKs: []descpb.ForeignKeyConstraint{ {OriginTableID: 105}, }, + Privileges: goodPrivilege, }, validDescIDs: catalog.MakeDescriptorIDSet(100, 101, 104, 105), validJobIDs: map[jobspb.JobID]struct{}{}, @@ -115,7 +124,8 @@ func TestStripDanglingBackReferences(t *testing.T) { {MutationID: 1}, {MutationID: 2}, }, - DropJobID: 1, + DropJobID: 1, + Privileges: badPrivilege, }, expectedOutput: descpb.TableDescriptor{ Name: "foo", @@ -127,6 +137,7 @@ func TestStripDanglingBackReferences(t *testing.T) { {MutationID: 1}, {MutationID: 2}, }, + Privileges: goodPrivilege, }, validDescIDs: catalog.MakeDescriptorIDSet(100, 101, 104, 105), validJobIDs: map[jobspb.JobID]struct{}{111222333444: {}}, @@ -143,8 +154,12 @@ func TestStripDanglingBackReferences(t *testing.T) { _, ok := test.validJobIDs[id] return ok })) + require.NoError(t, b.StripNonExistentRoles(func(role username.SQLUsername) bool { + return role.IsAdminRole() || role.IsPublicRole() || role.IsRootUser() + })) desc := b.BuildCreatedMutableTable() require.True(t, desc.GetPostDeserializationChanges().Contains(catalog.StrippedDanglingBackReferences)) + require.True(t, desc.GetPostDeserializationChanges().Contains(catalog.StrippedNonExistentRoles)) require.Equal(t, out.BuildCreatedMutableTable().TableDesc(), desc.TableDesc()) }) } diff --git a/pkg/sql/catalog/typedesc/BUILD.bazel b/pkg/sql/catalog/typedesc/BUILD.bazel index c2406e574081..3d85336b3dd8 100644 --- a/pkg/sql/catalog/typedesc/BUILD.bazel +++ b/pkg/sql/catalog/typedesc/BUILD.bazel @@ -15,6 +15,7 @@ go_library( "//pkg/clusterversion", "//pkg/jobs/jobspb", "//pkg/keys", + "//pkg/security/username", "//pkg/sql/catalog", "//pkg/sql/catalog/catpb", "//pkg/sql/catalog/catprivilege", diff --git a/pkg/sql/catalog/typedesc/type_desc_builder.go b/pkg/sql/catalog/typedesc/type_desc_builder.go index 04567f76b851..a7ac0af10e6f 100644 --- a/pkg/sql/catalog/typedesc/type_desc_builder.go +++ b/pkg/sql/catalog/typedesc/type_desc_builder.go @@ -13,7 +13,9 @@ package typedesc import ( "github.com/cockroachdb/cockroach/pkg/clusterversion" "github.com/cockroachdb/cockroach/pkg/jobs/jobspb" + "github.com/cockroachdb/cockroach/pkg/security/username" "github.com/cockroachdb/cockroach/pkg/sql/catalog" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog/catprivilege" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" "github.com/cockroachdb/cockroach/pkg/sql/privilege" @@ -149,6 +151,25 @@ func (tdb *typeDescriptorBuilder) StripDanglingBackReferences( return nil } +// StripNonExistentRoles implements the catalog.DescriptorBuilder +// interface. +func (tdb *typeDescriptorBuilder) StripNonExistentRoles( + roleExists func(role username.SQLUsername) bool, +) error { + newPrivs := make([]catpb.UserPrivileges, 0, len(tdb.maybeModified.Privileges.Users)) + for _, priv := range tdb.maybeModified.Privileges.Users { + exists := roleExists(priv.UserProto.Decode()) + if exists { + newPrivs = append(newPrivs, priv) + } + } + if len(newPrivs) != len(tdb.maybeModified.Privileges.Users) { + tdb.maybeModified.Privileges.Users = newPrivs + tdb.changes.Add(catalog.StrippedNonExistentRoles) + } + return nil +} + // SetRawBytesInStorage implements the catalog.DescriptorBuilder interface. func (tdb *typeDescriptorBuilder) SetRawBytesInStorage(rawBytes []byte) { tdb.rawBytesInStorage = append([]byte(nil), rawBytes...) // deep-copy diff --git a/pkg/sql/catalog/typedesc/type_desc_test.go b/pkg/sql/catalog/typedesc/type_desc_test.go index 4b08b2019d5f..6832a4f52fcc 100644 --- a/pkg/sql/catalog/typedesc/type_desc_test.go +++ b/pkg/sql/catalog/typedesc/type_desc_test.go @@ -848,13 +848,17 @@ func TestValidateTypeDesc(t *testing.T) { } } -func TestStripDanglingBackReferences(t *testing.T) { +func TestStripDanglingBackReferencesAndRoles(t *testing.T) { type testCase struct { name string input, expectedOutput descpb.TypeDescriptor validIDs catalog.DescriptorIDSet } - + badPrivilege := catpb.NewBaseDatabasePrivilegeDescriptor(username.RootUserName()) + goodPrivilege := catpb.NewBaseDatabasePrivilegeDescriptor(username.RootUserName()) + badPrivilege.Users = append(badPrivilege.Users, catpb.UserPrivileges{ + UserProto: username.TestUserName().EncodeProto(), + }) testData := []testCase{ { name: "referencing descriptor IDs", @@ -866,6 +870,7 @@ func TestStripDanglingBackReferences(t *testing.T) { ArrayTypeID: 105, Kind: descpb.TypeDescriptor_TABLE_IMPLICIT_RECORD_TYPE, ReferencingDescriptorIDs: []descpb.ID{12345, 105, 5678}, + Privileges: badPrivilege, }, expectedOutput: descpb.TypeDescriptor{ Name: "foo", @@ -875,6 +880,7 @@ func TestStripDanglingBackReferences(t *testing.T) { ArrayTypeID: 105, Kind: descpb.TypeDescriptor_TABLE_IMPLICIT_RECORD_TYPE, ReferencingDescriptorIDs: []descpb.ID{105}, + Privileges: goodPrivilege, }, validIDs: catalog.MakeDescriptorIDSet(100, 101, 104, 105), }, @@ -889,8 +895,12 @@ func TestStripDanglingBackReferences(t *testing.T) { require.NoError(t, b.StripDanglingBackReferences(test.validIDs.Contains, func(id jobspb.JobID) bool { return false })) + require.NoError(t, b.StripNonExistentRoles(func(role username.SQLUsername) bool { + return role.IsAdminRole() || role.IsPublicRole() || role.IsRootUser() + })) desc := b.BuildCreatedMutableType() require.True(t, desc.GetPostDeserializationChanges().Contains(catalog.StrippedDanglingBackReferences)) + require.True(t, desc.GetPostDeserializationChanges().Contains(catalog.StrippedNonExistentRoles)) require.Equal(t, out.BuildCreatedMutableType().TypeDesc(), desc.TypeDesc()) }) } diff --git a/pkg/sql/catalog/validate.go b/pkg/sql/catalog/validate.go index 81e9caa543b3..7f7b1e784211 100644 --- a/pkg/sql/catalog/validate.go +++ b/pkg/sql/catalog/validate.go @@ -14,6 +14,7 @@ import ( "strings" "github.com/cockroachdb/cockroach/pkg/clusterversion" + "github.com/cockroachdb/cockroach/pkg/security/username" "github.com/cockroachdb/cockroach/pkg/server/telemetry" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" "github.com/cockroachdb/errors" @@ -182,3 +183,22 @@ func ValidateOutboundTypeRefBackReference(selfID descpb.ID, typ TypeDescriptor) return errors.AssertionFailedf("depends-on type %q (%d) has no corresponding referencing-descriptor back references", typ.GetName(), typ.GetID()) } + +// ValidateRolesInDescriptor validates roles within a descriptor. +func ValidateRolesInDescriptor( + descriptor Descriptor, RoleExists func(username username.SQLUsername) (bool, error), +) error { + for _, priv := range descriptor.GetPrivileges().Users { + exists, err := RoleExists(priv.User()) + if err != nil { + return err + } + if !exists { + return errors.AssertionFailedf("descriptor %q (%d) has privilege on a role %q that doesn't exist", + descriptor.GetName(), + descriptor.GetID(), + priv.User()) + } + } + return nil +} diff --git a/pkg/sql/crdb_internal.go b/pkg/sql/crdb_internal.go index cb0d7bfcbe0d..7c502258f5ed 100644 --- a/pkg/sql/crdb_internal.go +++ b/pkg/sql/crdb_internal.go @@ -6151,6 +6151,22 @@ CREATE TABLE crdb_internal.invalid_objects ( for _, validationError := range ve { doError(validationError) } + doError(catalog.ValidateRolesInDescriptor(descriptor, func(username username.SQLUsername) (bool, error) { + if username.IsRootUser() || + username.IsAdminRole() || + username.IsNodeUser() || + username.IsPublicRole() { + return true, nil + } + err := p.CheckRoleExists(ctx, username) + if err != nil && pgerror.GetPGCode(err) == pgcode.UndefinedObject { + err = nil + return false, err + } else if err != nil { + return false, err + } + return true, nil + })) jobs.ValidateJobReferencesInDescriptor(descriptor, jmg, doError) return err } @@ -6597,6 +6613,13 @@ CREATE VIEW crdb_internal.kv_repairable_catalog_corruptions ( system.jobs WHERE status NOT IN ('failed', 'succeeded', 'canceled', 'revert-failed') + ), + ( SELECT + array_agg(username) as username_array FROM + (SELECT username + FROM system.users UNION + SELECT 'public' as username UNION + SELECT 'node' as username) ) ) AS repaired_descriptor diff --git a/pkg/sql/evalcatalog/BUILD.bazel b/pkg/sql/evalcatalog/BUILD.bazel index 778a6ec6e0f0..6ed0bf5443d1 100644 --- a/pkg/sql/evalcatalog/BUILD.bazel +++ b/pkg/sql/evalcatalog/BUILD.bazel @@ -16,6 +16,7 @@ go_library( "//pkg/jobs/jobspb", "//pkg/keys", "//pkg/kv", + "//pkg/security/username", "//pkg/sql/catalog", "//pkg/sql/catalog/descbuilder", "//pkg/sql/catalog/descpb", diff --git a/pkg/sql/evalcatalog/pg_updatable.go b/pkg/sql/evalcatalog/pg_updatable.go index a232674fa08a..091f83253697 100644 --- a/pkg/sql/evalcatalog/pg_updatable.go +++ b/pkg/sql/evalcatalog/pg_updatable.go @@ -14,6 +14,7 @@ import ( "context" "github.com/cockroachdb/cockroach/pkg/jobs/jobspb" + "github.com/cockroachdb/cockroach/pkg/security/username" "github.com/cockroachdb/cockroach/pkg/sql/catalog" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descbuilder" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" @@ -64,6 +65,7 @@ func (b *Builtins) RepairedDescriptor( encodedDescriptor []byte, descIDMightExist func(id descpb.ID) bool, nonTerminalJobIDMightExist func(id jobspb.JobID) bool, + roleExists func(username username.SQLUsername) bool, ) ([]byte, error) { // Use the largest-possible timestamp as a sentinel value when decoding the // descriptor bytes. This will be used to set the modification time field in @@ -82,6 +84,9 @@ func (b *Builtins) RepairedDescriptor( if err := db.StripDanglingBackReferences(descIDMightExist, nonTerminalJobIDMightExist); err != nil { return nil, err } + if err := db.StripNonExistentRoles(roleExists); err != nil { + return nil, err + } mut := db.BuildCreatedMutable() // Strip the sentinel value from the descriptor. if mvccTimestampSentinel.Equal(mut.GetModificationTime()) { diff --git a/pkg/sql/logictest/testdata/logic_test/crdb_internal_catalog b/pkg/sql/logictest/testdata/logic_test/crdb_internal_catalog index 316170def7f5..184dd89ddc5c 100644 --- a/pkg/sql/logictest/testdata/logic_test/crdb_internal_catalog +++ b/pkg/sql/logictest/testdata/logic_test/crdb_internal_catalog @@ -398,7 +398,7 @@ SELECT id, strip_volatile(descriptor) FROM crdb_internal.kv_catalog_descriptor O 4294967195 {"table": {"columns": [{"id": 1, "name": "grantee", "type": {"family": "StringFamily", "oid": 25}}, {"id": 2, "name": "role_name", "type": {"family": "StringFamily", "oid": 25}}, {"id": 3, "name": "is_grantable", "type": {"family": "StringFamily", "oid": 25}}], "formatVersion": 3, "id": 4294967195, "name": "administrable_role_authorizations", "nextColumnId": 4, "nextConstraintId": 2, "nextIndexId": 2, "nextMutationId": 1, "primaryIndex": {"constraintId": 1, "foreignKey": {}, "geoConfig": {}, "id": 1, "interleave": {}, "partitioning": {}, "sharded": {}}, "privileges": {"ownerProto": "node", "users": [{"privileges": "32", "userProto": "public"}], "version": 3}, "replacementOf": {"time": {}}, "unexposedParentSchemaId": 4294967196, "version": "1"}} 4294967196 {"schema": {"defaultPrivileges": {"type": "SCHEMA"}, "id": 4294967196, "name": "information_schema", "privileges": {"ownerProto": "node", "users": [{"privileges": "512", "userProto": "public"}], "version": 3}, "version": "1"}} 4294967197 {"table": {"columns": [{"id": 1, "name": "id", "type": {"family": "UuidFamily", "oid": 2950}}, {"id": 2, "name": "ts", "type": {"family": "DecimalFamily", "oid": 1700}}, {"id": 3, "name": "meta_type", "type": {"family": "StringFamily", "oid": 25}}, {"id": 4, "name": "meta", "nullable": true, "type": {"family": "BytesFamily", "oid": 17}}, {"id": 5, "name": "num_spans", "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 6, "name": "spans", "type": {"family": "BytesFamily", "oid": 17}}, {"id": 7, "name": "verified", "type": {"oid": 16}}, {"id": 8, "name": "target", "nullable": true, "type": {"family": "BytesFamily", "oid": 17}}, {"id": 9, "name": "decoded_meta", "nullable": true, "type": {"family": "JsonFamily", "oid": 3802}}, {"id": 10, "name": "decoded_target", "nullable": true, "type": {"family": "JsonFamily", "oid": 3802}}, {"id": 11, "name": "internal_meta", "nullable": true, "type": {"family": "JsonFamily", "oid": 3802}}, {"id": 12, "name": "num_ranges", "nullable": true, "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 13, "name": "last_updated", "nullable": true, "type": {"family": "DecimalFamily", "oid": 1700}}], "formatVersion": 3, "id": 4294967197, "name": "kv_protected_ts_records", "nextColumnId": 14, "nextConstraintId": 2, "nextIndexId": 2, "nextMutationId": 1, "primaryIndex": {"constraintId": 1, "foreignKey": {}, "geoConfig": {}, "id": 1, "interleave": {}, "partitioning": {}, "sharded": {}}, "privileges": {"ownerProto": "node", "users": [{"privileges": "32", "userProto": "public"}], "version": 3}, "replacementOf": {"time": {}}, "unexposedParentSchemaId": 4294967295, "version": "1"}} -4294967198 {"table": {"columns": [{"id": 1, "name": "parent_id", "nullable": true, "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 2, "name": "parent_schema_id", "nullable": true, "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 3, "name": "name", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}, {"id": 4, "name": "id", "nullable": true, "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 5, "name": "corruption", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}], "formatVersion": 3, "id": 4294967198, "name": "kv_repairable_catalog_corruptions", "nextColumnId": 6, "nextConstraintId": 1, "nextMutationId": 1, "primaryIndex": {"foreignKey": {}, "geoConfig": {}, "interleave": {}, "partitioning": {}, "sharded": {}}, "privileges": {"ownerProto": "node", "users": [{"privileges": "32", "userProto": "public"}], "version": 3}, "replacementOf": {"time": {}}, "unexposedParentSchemaId": 4294967295, "version": "1", "viewQuery": "WITH data AS (SELECT ns.\"parentID\" AS parent_id, ns.\"parentSchemaID\" AS parent_schema_id, ns.name, COALESCE(ns.id, d.id) AS id, d.descriptor, crdb_internal.descriptor_with_post_deserialization_changes(d.descriptor) AS updated_descriptor, crdb_internal.repaired_descriptor(d.descriptor, (SELECT array_agg(id) AS desc_id_array FROM system.descriptor), (SELECT array_agg(id) AS job_id_array FROM system.jobs WHERE status NOT IN ('failed', 'succeeded', 'canceled', 'revert-failed'))) AS repaired_descriptor FROM system.namespace AS ns FULL JOIN system.descriptor AS d ON ns.id = d.id), diag AS (SELECT *, CASE WHEN (descriptor IS NULL) AND (id != 29) THEN 'namespace' WHEN updated_descriptor != repaired_descriptor THEN 'descriptor' ELSE NULL END AS corruption FROM data) SELECT parent_id, parent_schema_id, name, id, corruption FROM diag WHERE corruption IS NOT NULL ORDER BY parent_id, parent_schema_id, name, id"}} +4294967198 {"table": {"columns": [{"id": 1, "name": "parent_id", "nullable": true, "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 2, "name": "parent_schema_id", "nullable": true, "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 3, "name": "name", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}, {"id": 4, "name": "id", "nullable": true, "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 5, "name": "corruption", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}], "formatVersion": 3, "id": 4294967198, "name": "kv_repairable_catalog_corruptions", "nextColumnId": 6, "nextConstraintId": 1, "nextMutationId": 1, "primaryIndex": {"foreignKey": {}, "geoConfig": {}, "interleave": {}, "partitioning": {}, "sharded": {}}, "privileges": {"ownerProto": "node", "users": [{"privileges": "32", "userProto": "public"}], "version": 3}, "replacementOf": {"time": {}}, "unexposedParentSchemaId": 4294967295, "version": "1", "viewQuery": "WITH data AS (SELECT ns.\"parentID\" AS parent_id, ns.\"parentSchemaID\" AS parent_schema_id, ns.name, COALESCE(ns.id, d.id) AS id, d.descriptor, crdb_internal.descriptor_with_post_deserialization_changes(d.descriptor) AS updated_descriptor, crdb_internal.repaired_descriptor(d.descriptor, (SELECT array_agg(id) AS desc_id_array FROM system.descriptor), (SELECT array_agg(id) AS job_id_array FROM system.jobs WHERE status NOT IN ('failed', 'succeeded', 'canceled', 'revert-failed')), (SELECT array_agg(username) AS username_array FROM (SELECT username FROM system.users UNION SELECT 'public' AS username UNION SELECT 'node' AS username))) AS repaired_descriptor FROM system.namespace AS ns FULL JOIN system.descriptor AS d ON ns.id = d.id), diag AS (SELECT *, CASE WHEN (descriptor IS NULL) AND (id != 29) THEN 'namespace' WHEN updated_descriptor != repaired_descriptor THEN 'descriptor' ELSE NULL END AS corruption FROM data) SELECT parent_id, parent_schema_id, name, id, corruption FROM diag WHERE corruption IS NOT NULL ORDER BY parent_id, parent_schema_id, name, id"}} 4294967199 {"table": {"columns": [{"id": 1, "name": "range_id", "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 2, "name": "tenant_id", "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 3, "name": "store_id", "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 4, "name": "priority", "type": {"family": "StringFamily", "oid": 25}}, {"id": 5, "name": "log_term", "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 6, "name": "log_index", "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 7, "name": "tokens", "type": {"family": "IntFamily", "oid": 20, "width": 64}}], "formatVersion": 3, "id": 4294967199, "indexes": [{"foreignKey": {}, "geoConfig": {}, "id": 2, "interleave": {}, "keyColumnDirections": ["ASC"], "keyColumnIds": [1], "keyColumnNames": ["range_id"], "name": "kv_flow_token_deductions_range_id_idx", "partitioning": {}, "sharded": {}, "storeColumnIds": [2, 3, 4, 5, 6, 7], "storeColumnNames": ["tenant_id", "store_id", "priority", "log_term", "log_index", "tokens"], "version": 3}], "name": "kv_flow_token_deductions", "nextColumnId": 8, "nextConstraintId": 2, "nextIndexId": 3, "nextMutationId": 1, "primaryIndex": {"constraintId": 1, "foreignKey": {}, "geoConfig": {}, "id": 1, "interleave": {}, "partitioning": {}, "sharded": {}}, "privileges": {"ownerProto": "node", "users": [{"privileges": "32", "userProto": "public"}], "version": 3}, "replacementOf": {"time": {}}, "unexposedParentSchemaId": 4294967295, "version": "1"}} 4294967200 {"table": {"columns": [{"id": 1, "name": "range_id", "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 2, "name": "tenant_id", "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 3, "name": "store_id", "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 4, "name": "total_tracked_tokens", "type": {"family": "IntFamily", "oid": 20, "width": 64}}], "formatVersion": 3, "id": 4294967200, "indexes": [{"foreignKey": {}, "geoConfig": {}, "id": 2, "interleave": {}, "keyColumnDirections": ["ASC"], "keyColumnIds": [1], "keyColumnNames": ["range_id"], "name": "kv_flow_control_handles_range_id_idx", "partitioning": {}, "sharded": {}, "storeColumnIds": [2, 3, 4], "storeColumnNames": ["tenant_id", "store_id", "total_tracked_tokens"], "version": 3}], "name": "kv_flow_control_handles", "nextColumnId": 5, "nextConstraintId": 2, "nextIndexId": 3, "nextMutationId": 1, "primaryIndex": {"constraintId": 1, "foreignKey": {}, "geoConfig": {}, "id": 1, "interleave": {}, "partitioning": {}, "sharded": {}}, "privileges": {"ownerProto": "node", "users": [{"privileges": "32", "userProto": "public"}], "version": 3}, "replacementOf": {"time": {}}, "unexposedParentSchemaId": 4294967295, "version": "1"}} 4294967201 {"table": {"columns": [{"id": 1, "name": "tenant_id", "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 2, "name": "store_id", "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 3, "name": "available_regular_tokens", "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 4, "name": "available_elastic_tokens", "type": {"family": "IntFamily", "oid": 20, "width": 64}}], "formatVersion": 3, "id": 4294967201, "name": "kv_flow_controller", "nextColumnId": 5, "nextConstraintId": 2, "nextIndexId": 2, "nextMutationId": 1, "primaryIndex": {"constraintId": 1, "foreignKey": {}, "geoConfig": {}, "id": 1, "interleave": {}, "partitioning": {}, "sharded": {}}, "privileges": {"ownerProto": "node", "users": [{"privileges": "32", "userProto": "public"}], "version": 3}, "replacementOf": {"time": {}}, "unexposedParentSchemaId": 4294967295, "version": "1"}} diff --git a/pkg/sql/sem/builtins/builtins.go b/pkg/sql/sem/builtins/builtins.go index cef9b8a5a87a..3cd6043e5932 100644 --- a/pkg/sql/sem/builtins/builtins.go +++ b/pkg/sql/sem/builtins/builtins.go @@ -43,6 +43,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverbase" "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/security/password" + "github.com/cockroachdb/cockroach/pkg/security/username" "github.com/cockroachdb/cockroach/pkg/server/telemetry" "github.com/cockroachdb/cockroach/pkg/settings" "github.com/cockroachdb/cockroach/pkg/settings/cluster" @@ -5277,8 +5278,11 @@ DO NOT USE -- USE 'CREATE VIRTUAL CLUSTER' INSTEAD`, jobIDAlwaysValid := func(id jobspb.JobID) bool { return true } + roleAlwaysValid := func(username username.SQLUsername) bool { + return true + } ret, err := evalCtx.CatalogBuiltins.RepairedDescriptor( - ctx, []byte(s), descIDAlwaysValid, jobIDAlwaysValid, + ctx, []byte(s), descIDAlwaysValid, jobIDAlwaysValid, roleAlwaysValid, ) if err != nil { return nil, err @@ -5300,6 +5304,7 @@ DO NOT USE -- USE 'CREATE VIRTUAL CLUSTER' INSTEAD`, {Name: "descriptor", Typ: types.Bytes}, {Name: "valid_descriptor_ids", Typ: types.IntArray}, {Name: "valid_job_ids", Typ: types.IntArray}, + {Name: "valid_roles", Typ: types.StringArray}, }, ReturnType: tree.FixedReturnType(types.Bytes), Fn: func(ctx context.Context, evalCtx *eval.Context, args tree.Datums) (tree.Datum, error) { @@ -5345,8 +5350,31 @@ DO NOT USE -- USE 'CREATE VIRTUAL CLUSTER' INSTEAD`, return found } } + + roleMightExist := func(username username.SQLUsername) bool { + return true + } + if args[3] != tree.DNull { + roles, ok := tree.AsDArray(args[3]) + if !ok { + return nil, errors.Newf("expected array value, got %T", args[3]) + } + roleMap := make(map[username.SQLUsername]struct{}) + for _, roleDatum := range (*roles).Array { + role := tree.MustBeDString(roleDatum) + roleName, err := username.MakeSQLUsernameFromUserInput(string(role), username.PurposeValidation) + if err != nil { + return nil, err + } + roleMap[roleName] = struct{}{} + } + roleMightExist = func(username username.SQLUsername) bool { + _, ok := roleMap[username] + return ok + } + } ret, err := evalCtx.CatalogBuiltins.RepairedDescriptor( - ctx, []byte(s), descIDMightExist, nonTerminalJobIDMightExist, + ctx, []byte(s), descIDMightExist, nonTerminalJobIDMightExist, roleMightExist, ) if err != nil { return nil, err @@ -5390,6 +5418,13 @@ SELECT system.jobs WHERE status NOT IN ('failed', 'succeeded', 'canceled', 'revert-failed') + ), + ( SELECT + array_agg(username) as username_array FROM + (SELECT username + FROM system.users UNION + SELECT 'public' as username UNION + SELECT 'node' as username) ) ), true diff --git a/pkg/sql/sem/builtins/fixed_oids.go b/pkg/sql/sem/builtins/fixed_oids.go index ed586432d37a..04059e7cb409 100644 --- a/pkg/sql/sem/builtins/fixed_oids.go +++ b/pkg/sql/sem/builtins/fixed_oids.go @@ -2422,7 +2422,7 @@ var builtinOidsArray = []string{ 2449: `st_asmvtgeom(geometry: geometry, bbox: box2d, extent: int, buffer: int) -> geometry`, 2450: `st_asmvtgeom(geometry: geometry, bbox: box2d, extent: int) -> geometry`, 2451: `st_asmvtgeom(geometry: geometry, bbox: box2d) -> geometry`, - 2452: `crdb_internal.repaired_descriptor(descriptor: bytes, valid_descriptor_ids: int[], valid_job_ids: int[]) -> bytes`, + 2452: `crdb_internal.repaired_descriptor(descriptor: bytes, valid_descriptor_ids: int[], valid_job_ids: int[], valid_roles: string[]) -> bytes`, 2453: `crdb_internal.reset_activity_tables() -> bool`, 2454: `crdb_internal.sstable_metrics(node_id: int, store_id: int, start_key: bytes, end_key: bytes) -> tuple{int AS node_id, int AS store_id, int AS level, int AS file_num, int AS approximate_span_bytes, jsonb AS metrics}`, 2455: `crdb_internal.repair_catalog_corruption(descriptor_id: int, corruption: string) -> bool`, diff --git a/pkg/sql/sem/eval/deps.go b/pkg/sql/sem/eval/deps.go index ac7b3f45b625..085124aa30e8 100644 --- a/pkg/sql/sem/eval/deps.go +++ b/pkg/sql/sem/eval/deps.go @@ -128,12 +128,14 @@ type CatalogBuiltins interface { // puts it into a catalog.DescriptorBuilder, // calls RunPostDeserializationChanges, // calls StripDanglingBackReferences, + // calls StripNonExistentRoles, // and re-encodes it. RepairedDescriptor( ctx context.Context, encodedDescriptor []byte, descIDMightExist func(id descpb.ID) bool, nonTerminalJobIDMightExist func(id jobspb.JobID) bool, + roleExists func(username username.SQLUsername) bool, ) ([]byte, error) }