markhoerth commented on PR #12940: URL: https://github.com/apache/gravitino/pull/12940#issuecomment-5576140493
@lasdf1234 @roryqi Thanks both. Two things: the release this targets, and the permission model. **Release.** The `branch-1.3` label should come off. I checked the branch and the running 1.3 build. On `origin/branch-1.3` the only references to `/configs/secrets/providers` are in `GravitinoServer.java`, line 97 adding it to `ROOT_MOUNTED_PATHS` and the line 244 comment saying it is open pending #12921. No servlet, resource or handler serves it, and `SecretProvidersConfigServlet` is not on the branch at all. A `curl` against a running 1.3 install returns 404. So the endpoint does not exist in 1.3 and there is no exposure there to fix. We are past feature complete and stabilizing 1.3 Enterprise, and this is a block of new functionality that has not been tested. I have no UI to manage secret providers in 1.3 Enterprise, and this is too big of a change to put on 1.3.1 OSS. It should target 2.0. **Permission model.** Rory is right that service admin is the wrong gate, and the disagreement points past this endpoint. Service admin is already a set of global capabilities: create a metalake, and now read the secret provider registry. Neither is affiliated with a metalake, and neither can be delegated, because the serviceAdmins list is a config file entry rather than a securable object. Adding a third capability there makes that worse instead of answering it. So the 2.0 requirement: a SYSTEM securable object, with the service admin's capabilities expressed as privileges that bind to it, `CREATE_METALAKE` and `VIEW_SECRET_PROVIDERS`. SYSTEM rather than SERVER, because these privileges apply to the deployment and not to one replica. The serviceAdmins list stays as the bootstrap identity and stops being where capabilities accumulate. Every right an administrator holds should be grantable to someone else, which is the model the rest of Gravitino already follows. **The uri stays out.** Two requirements on the registry itself. Provider names must be unique, so the name is always sufficient to identify a provider. And there must never be more than two providers configured: the secret provider for the cloud the deployment is installed on, plus a local one. We do not reach into secret providers on other clouds. With those two rules the same-type case does not arise, and distinguishing providers is what the name is for. The uri is also the field that made this a security issue, being internal infrastructure topology, so it does not belong in the response. Next step: drop the `branch-1.3` label, retarget to 2.0, and open an issue for the SYSTEM securable object so the permission model is settled before this endpoint picks a gate. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
