Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

catalog: add descriptor repair to remove missing roles #122557

Merged
merged 1 commit into from
Apr 19, 2024
Merged

catalog: add descriptor repair to remove missing roles #122557

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions pkg/sql/catalog/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
19 changes: 19 additions & 0 deletions pkg/sql/catalog/dbdesc/database_desc_builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 12 additions & 1 deletion pkg/sql/catalog/dbdesc/database_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -406,13 +411,15 @@ func TestStripDanglingBackReferences(t *testing.T) {
"bar": {ID: 101},
"baz": {ID: 12345},
},
Privileges: badPrivilege,
},
expectedOutput: descpb.DatabaseDescriptor{
Name: "foo",
ID: 100,
Schemas: map[string]descpb.DatabaseDescriptor_SchemaInfo{
"bar": {ID: 101},
},
Privileges: goodPrivilege,
},
validIDs: catalog.MakeDescriptorIDSet(100, 101),
},
Expand All @@ -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())
})
}
Expand Down
5 changes: 5 additions & 0 deletions pkg/sql/catalog/descriptor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)

Expand Down
19 changes: 19 additions & 0 deletions pkg/sql/catalog/funcdesc/func_desc_builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 12 additions & 7 deletions pkg/sql/catalog/funcdesc/func_desc_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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),
},
Expand All @@ -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())
})
}
Expand Down
4 changes: 4 additions & 0 deletions pkg/sql/catalog/post_deserialization_changes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
1 change: 1 addition & 0 deletions pkg/sql/catalog/schemadesc/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
19 changes: 19 additions & 0 deletions pkg/sql/catalog/schemadesc/schema_desc_builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
39 changes: 39 additions & 0 deletions pkg/sql/catalog/schemadesc/schema_desc_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
}
1 change: 1 addition & 0 deletions pkg/sql/catalog/tabledesc/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
21 changes: 21 additions & 0 deletions pkg/sql/catalog/tabledesc/table_desc_builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
19 changes: 17 additions & 2 deletions pkg/sql/catalog/tabledesc/table_desc_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -58,14 +60,19 @@ 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
validDescIDs catalog.DescriptorIDSet
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",
Expand All @@ -85,6 +92,7 @@ func TestStripDanglingBackReferences(t *testing.T) {
{OriginTableID: 105},
},
ReplacementOf: descpb.TableDescriptor_Replacement{ID: 12345},
Privileges: badPrivilege,
},
expectedOutput: descpb.TableDescriptor{
Name: "foo",
Expand All @@ -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{}{},
Expand All @@ -114,7 +123,8 @@ func TestStripDanglingBackReferences(t *testing.T) {
{MutationID: 1},
{MutationID: 2},
},
DropJobID: 1,
DropJobID: 1,
Privileges: badPrivilege,
},
expectedOutput: descpb.TableDescriptor{
Name: "foo",
Expand All @@ -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: {}},
Expand All @@ -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())
})
}
Expand Down
Loading
Loading