dennishuo commented on code in PR #2223:
URL: https://github.com/apache/polaris/pull/2223#discussion_r2370914277
##########
runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java:
##########
@@ -1686,14 +1691,33 @@ public boolean grantPrivilegeOnNamespaceToRole(
PolarisAuthorizableOperation.ADD_NAMESPACE_GRANT_TO_CATALOG_ROLE;
authorizeGrantOnNamespaceOperationOrThrow(op, catalogName, namespace,
catalogRoleName);
+ CatalogEntity catalogEntity =
+ findCatalogByName(catalogName)
+ .orElseThrow(() -> new NotFoundException("Parent catalog %s not
found", catalogName));
PolarisEntity catalogRoleEntity =
findCatalogRoleByName(catalogName, catalogRoleName)
.orElseThrow(() -> new NotFoundException("CatalogRole %s not
found", catalogRoleName));
PolarisResolvedPathWrapper resolvedPathWrapper =
resolutionManifest.getResolvedPath(namespace);
if (resolvedPathWrapper == null
|| !resolvedPathWrapper.isFullyResolvedNamespace(catalogName,
namespace)) {
- throw new NotFoundException("Namespace %s not found", namespace);
+ boolean rbacForFederatedCatalogsEnabled =
+ getCurrentPolarisContext()
+ .getRealmConfig()
+
.getConfig(FeatureConfiguration.ENABLE_SUB_CATALOG_RBAC_FOR_FEDERATED_CATALOGS);
+ if (resolutionManifest.getIsPassthroughFacade() &&
rbacForFederatedCatalogsEnabled) {
+ resolvedPathWrapper =
+ createSyntheticNamespaceEntities(catalogEntity, namespace,
resolvedPathWrapper);
Review Comment:
Relating to my comment in the other file
https://github.com/apache/polaris/pull/2223#discussion_r2366654356 we probably
don't need to necessarily care about the *synthetic* nature of the entity so
much as its role as a "Passthrough Facade".
@dimas-b I agree we should extract common "backfill/JIT-creation" logic,
possibly as a fast-follow or we can do it inline here if we really want as
well, but it would be "in order to" use that common logic for the implicit
backfill during grants. rather than precluding that backfill.
It would then also be shared for implicit backfill during loadTable (could
be configured as an option, if the serving of the loadTable doesn't *strictly*
require the entity to be persisted).
This design tradeoff of whether to JIT-create automatically vs requiring the
separate API is discussed a bit here:
https://docs.google.com/document/d/1Q6eEytxb0btpOPcL8RtkULskOlYUCo_3FLvFRnHkzBY/edit?tab=t.0#bookmark=id.cf85r4rp8tz
- `Passthrough Facade with JIT-Creation instead of requiring upfront facade
creation
`
I suppose we could ultimately also make it a configurable option if we
expose an actual REST API endpoint for materializing the facade entity, but
historically the requirement of upfront facade creation has demonstrated to be
a barrier to adoption.
I'd say these JIT-created facade entities *are* still first-class entities
as intended, just that they provide the semantic of the Passthrough Facade
rather than a source-of-truth entity. In general a Passthrough Facade would
mean:
1. *Listing* the parent doesn't get to short-circuit with passthrough
facades -- "list" is strictly defined by the remote catalog
2. GET on the entity represented by the facade still requires *some*
readthrough to the remote catalog, but the contents of a facade entity will
*augment* or *optimize* the response. For example:
- Cached metadata representation - a facade *may* have a local copy of
the full TableMetadata JSON along with the etag associated with it from the
remote - the passthrough `loadTable` may use the etag to efficiently determine
that the local copy is already fresh, and thus serve the response directly from
the local copy
- Grants semantics - as this PR adds, the facade entity serves as the
relational entity within the Polaris RBAC grants system
- Credential vending - In upcoming related work outlined in the addendum
https://docs.google.com/document/d/19Vm0Uy-EyEYtgd2-bZpPICODOB3rYQJvHeqL1ODOOLc/edit?tab=t.0
the local TableMetadata of the facade entity allows us to safely reuse
credential-subscoping logic which takes in the logical table data/metadata
read/write paths as part of the subscoping policy
--
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]
