diff --git a/pkg/sql/catalog/BUILD.bazel b/pkg/sql/catalog/BUILD.bazel index 5c6eb02f4c27..105b137fcf20 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 042f9f531ab5..18f9dba7cce4 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 276ca64ace9e..16d00eb0a6ae 100644 --- a/pkg/sql/catalog/funcdesc/func_desc_test.go +++ b/pkg/sql/catalog/funcdesc/func_desc_test.go @@ -814,13 +814,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", @@ -837,9 +842,7 @@ func TestStripDanglingBackReferences(t *testing.T) { ColumnIDs: []descpb.ColumnID{1}, }, }, - Privileges: &catpb.PrivilegeDescriptor{ - Version: catpb.Version23_2, - }, + Privileges: badPrivilege, }, expectedOutput: descpb.FunctionDescriptor{ Name: "foo", @@ -850,9 +853,7 @@ func TestStripDanglingBackReferences(t *testing.T) { ColumnIDs: []descpb.ColumnID{1}, }, }, - Privileges: &catpb.PrivilegeDescriptor{ - Version: catpb.Version23_2, - }, + Privileges: goodPrivilege, }, validIDs: catalog.MakeDescriptorIDSet(104, 105), }, @@ -867,8 +868,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 909cde2efe81..3d65086e0b9e 100644 --- a/pkg/sql/catalog/post_deserialization_changes.go +++ b/pkg/sql/catalog/post_deserialization_changes.go @@ -132,4 +132,8 @@ const ( // GrantExecuteOnFunctionToPublicRole indicates that EXECUTE was granted // to the public role for a function. GrantExecuteOnFunctionToPublicRole + + // 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 5955bfcec8c1..fc1500576249 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 be197b510e64..a1da6105dc27 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 02070fc8aa35..fbeb64cd5b09 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 1d601a4ac88f..7bb60915eceb 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", @@ -85,6 +92,7 @@ func TestStripDanglingBackReferences(t *testing.T) { {OriginTableID: 105}, }, ReplacementOf: descpb.TableDescriptor_Replacement{ID: 12345}, + Privileges: badPrivilege, }, expectedOutput: descpb.TableDescriptor{ Name: "foo", @@ -96,6 +104,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{}{}, @@ -114,7 +123,8 @@ func TestStripDanglingBackReferences(t *testing.T) { {MutationID: 1}, {MutationID: 2}, }, - DropJobID: 1, + DropJobID: 1, + Privileges: badPrivilege, }, expectedOutput: descpb.TableDescriptor{ Name: "foo", @@ -126,6 +136,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: {}}, @@ -142,8 +153,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 f0b661bd4d32..69804352e277 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 47c168c15308..cca52af05707 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" @@ -196,3 +197,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 0aa5352789db..2ab925c7bf17 100644 --- a/pkg/sql/crdb_internal.go +++ b/pkg/sql/crdb_internal.go @@ -6176,6 +6176,21 @@ 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.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 } @@ -6622,6 +6637,12 @@ 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) ) ) AS repaired_descriptor diff --git a/pkg/sql/evalcatalog/BUILD.bazel b/pkg/sql/evalcatalog/BUILD.bazel index 73ab778139e4..a8604817f49b 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 d9f771bb71f8..adbbee31019b 100644 --- a/pkg/sql/logictest/testdata/logic_test/crdb_internal_catalog +++ b/pkg/sql/logictest/testdata/logic_test/crdb_internal_catalog @@ -403,7 +403,7 @@ SELECT id, strip_volatile(descriptor) FROM crdb_internal.kv_catalog_descriptor O 4294967195 {"table": {"columns": [{"id": 1, "name": "job_id", "nullable": true, "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 2, "name": "start_key", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}, {"id": 3, "name": "end_key", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}, {"id": 4, "name": "resolved", "nullable": true, "type": {"family": "DecimalFamily", "oid": 1700}}, {"id": 5, "name": "resolved_age", "nullable": true, "type": {"family": "IntervalFamily", "intervalDurationField": {}, "oid": 1186}}], "formatVersion": 3, "id": 4294967195, "name": "cluster_replication_spans", "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 spans AS (SELECT j.id AS job_id, jsonb_array_elements(((crdb_internal.pb_to_json('progress', i.value)->'streamIngest')->'checkpoint')->'resolvedSpans') AS s FROM system.jobs AS j LEFT JOIN system.job_info AS i ON (j.id = i.job_id) AND (i.info_key = 'legacy_progress') WHERE j.job_type = 'REPLICATION STREAM INGESTION') SELECT job_id, crdb_internal.pretty_key(decode((s->'span')->>'key', 'base64'), 0) AS start_key, crdb_internal.pretty_key(decode((s->'span')->>'endKey', 'base64'), 0) AS end_key, ((((s->'timestamp')->>'wallTime') || '.') || COALESCE(((s->'timestamp')->'logical'), '0'))::DECIMAL AS resolved, date_trunc('second', ((cluster_logical_timestamp() - ((s->'timestamp')->>'wallTime')::INT8) / 1e9)::INTERVAL) AS resolved_age FROM spans"}} 4294967196 {"table": {"columns": [{"id": 1, "name": "desc_id", "nullable": true, "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 2, "name": "version", "nullable": true, "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 3, "name": "sql_instance_id", "nullable": true, "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 4, "name": "session_id", "type": {"family": "BytesFamily", "oid": 17}}, {"id": 5, "name": "crdb_region", "type": {"family": "BytesFamily", "oid": 17}}], "formatVersion": 3, "id": 4294967196, "name": "kv_session_based_leases", "nextColumnId": 6, "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"}} 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))) 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 90810baa1180..35b121cca92f 100644 --- a/pkg/sql/sem/builtins/builtins.go +++ b/pkg/sql/sem/builtins/builtins.go @@ -41,6 +41,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" @@ -5366,8 +5367,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 @@ -5389,6 +5393,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) { @@ -5434,8 +5439,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 @@ -5479,6 +5507,12 @@ 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) ) ), true diff --git a/pkg/sql/sem/builtins/fixed_oids.go b/pkg/sql/sem/builtins/fixed_oids.go index bbb777b181fe..88eaee930f88 100644 --- a/pkg/sql/sem/builtins/fixed_oids.go +++ b/pkg/sql/sem/builtins/fixed_oids.go @@ -2419,7 +2419,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 9425bd484e5d..6a757eee09d2 100644 --- a/pkg/sql/sem/eval/deps.go +++ b/pkg/sql/sem/eval/deps.go @@ -127,12 +127,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) }