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 >
