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 > > >
