Hi Michael, Thanks!
Did you forget to push? The PR still shows old data in GH. Cheers, Dmitri. On Wed, Mar 4, 2026 at 12:52 PM Michael Collado <[email protected]> wrote: > Thanks Dmitri. I did update the PR with a changelog update. > > Re: the other privileges, I'm not really sure what the intention was in > adding them. Perhaps @dennis-huo can chime in? > > Mike > > On Mon, Mar 2, 2026 at 9:48 AM Dmitri Bourlatchkov <[email protected]> > wrote: > > > Hi Michael, > > > > Making catalog-level grant/revote permissions symmetrical makes sense to > > me. > > > > Re: PR 3906, the change looks good to me. I believe it still deserves > > mention in the CHANGELOG. > > > > I have a couple of side question, though: while browsing related code, I > > found that the following existing operations are not used in Apache > > Polaris: > > * REVOKE_PRINCIPAL_GRANT_FROM_PRINCIPAL_ROLE > > * REVOKE_PRINCIPAL_ROLE_GRANT_FROM_PRINCIPAL_ROLE > > > > The operations also reference the > PRINCIPAL_ROLE_MANAGE_GRANTS_FOR_GRANTEE > > permission, but due to lack of their usage, reasoning about this change > is > > a bit complicated in the current codebase. > > > > Would it make sense to remove the unused operations (in a separate PR, of > > course)? > > > > The only remaining usage of PRINCIPAL_ROLE_MANAGE_GRANTS_FOR_GRANTEE > > appears to be in REVOKE_ROOT_GRANT_FROM_PRINCIPAL_ROLE, which is invoked > by > > PolarisAdminService.revokePrivilegeOnRootContainerFromPrincipalRole()... > > yet it does not appear to be called anywhere. WDYT? > > > > Is the REVOKE_ROOT_GRANT_FROM_PRINCIPAL_ROLE permission relevant anymore? > > > > Thanks, > > Dmitri. > > > > On Mon, Mar 2, 2026 at 12:19 PM Michael Collado <[email protected]> > > wrote: > > > > > Hey folks > > > > > > I wanted to raise awareness of a small change in the privilege model in > > > #3906. Currently, the catalog_admin role in a given catalog has > > privileges > > > to grant a Catalog Role to any Principal Role. However, the > catalog_admin > > > role by itself is not enough to revoke that Catalog Role. Instead, the > > > privilege model requires the user has both the catalog_admin role and > > also > > > the privilege to manage grants for principal roles (typically, the > > > service_admin). In effect, this means that the service_admin role has > to > > > have catalog_admin privileges on every catalog or catalog roles can't > be > > > revoked once they were granted. > > > > > > The change in my PR removes the requirement to manage grants on > > > the principale role so that the grant and revoke actions are > symmetrical > > > and require the same privilege - CATALOG_MANAGE_ACCESS on the target > > > catalog. > > > > > > Unless there are objections, I'd like to merge this PR in the next > couple > > > of days. Please let me know if there are any concerns. > > > > > > https://github.com/apache/polaris/pull/3906 > > > > > > Mike > > > > > >
