sql: fix excess privileges being created from default privileges. #72323
Conversation
|
This change is |
| @@ -148,28 +148,35 @@ func (d *immutable) CreatePrivilegesFromDefaultPrivileges( | |||
| // If default privileges are not defined for the creator role, we handle | |||
| // it as the default case where the user has all privileges. | |||
| role := descpb.DefaultPrivilegesRole{Role: user} | |||
| if _, found := d.GetDefaultPrivilegesForRole(role); !found { | |||
| if defaultPrivilegesForRole, found := d.GetDefaultPrivilegesForRole(role); !found { | |||
There was a problem hiding this comment.
does the comment above need adjusting?
There was a problem hiding this comment.
No this is still accurate but it might be more fitting to put this into the if !found clause? Thoughts?
There was a problem hiding this comment.
yeah i'm in favor of putting it inside of if !found and then one more comment inside of the else
RichardJCai
force-pushed
the
fix_creating_excess_privileges_from_default_privileges_11022021
branch
from
5d1a574
to
110f641
Compare
Release note (bug fix): Previously, when creating an object default privileges from users that were not the user creating the object would be added to the privileges of the object. This fix ensures only the relevant default privileges are applied.
RichardJCai
force-pushed
the
fix_creating_excess_privileges_from_default_privileges_11022021
branch
from
110f641
to
aeb60d0
Compare
There was a problem hiding this comment.
Part of me wonders if the thing we were missing here was table-driven unit testing of the functions in catprivilege.
Reviewed 1 of 2 files at r1.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @ajwerner, @rafiss, and @RichardJCai)
|
TFTR! |
Yeah we're definitely missing some unit tests here. I'll add some in, this isn't good |
|
Build failed (retrying...): |
|
Build failed (retrying...): |
|
Build failed: |
|
Looks like the test failure is a flake. |
|
Build failed (retrying...): |
|
Build failed (retrying...): |
|
Build succeeded: |
Release note (bug fix): Previously, when creating an object
default privileges from users that were not the user creating
the object would be added to the privileges of the object.
This fix ensures only the relevant default privileges are applied.
Resolves #72322