dennishuo commented on code in PR #3927:
URL: https://github.com/apache/polaris/pull/3927#discussion_r2899017079


##########
spec/polaris-management-service.yml:
##########
@@ -1582,6 +1582,7 @@ components:
         - CATALOG_MANAGE_ACCESS
         - CATALOG_MANAGE_CONTENT
         - CATALOG_MANAGE_METADATA
+        - CATALOG_READ_DATA

Review Comment:
   If we do make this `NAMESPACE_READ_DATA`, we can still allow it to be set at 
either this `CatalogPrivilege` level *or* at the `NamespacePrivilege` level, 
which I think could actually be in keeping with the intended use case, where 
you may want to grant "read all" semantics at some entire namespace subtree 
even if not granting at the entire catalog level.



##########
polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrivilege.java:
##########
@@ -256,6 +256,16 @@ public enum PolarisPrivilege {
       PolarisEntityType.TABLE_LIKE,
       List.of(PolarisEntitySubType.ICEBERG_TABLE, 
PolarisEntitySubType.GENERIC_TABLE),
       PolarisEntityType.CATALOG_ROLE),
+  /**
+   * Grants read-only access to all data and navigational metadata within a 
specific catalog.
+   * Intended for data analyst principals who need to read tables and views 
across an entire catalog
+   * without any write or administrative capabilities.
+   *
+   * <p>This privilege subsumes: {@link #NAMESPACE_LIST}, {@link 
#NAMESPACE_READ_PROPERTIES},
+   * {@link #TABLE_LIST}, {@link #TABLE_READ_PROPERTIES}, {@link 
#TABLE_READ_DATA}, {@link
+   * #VIEW_LIST}, and {@link #VIEW_READ_PROPERTIES}.
+   */
+  CATALOG_READ_DATA(103, PolarisEntityType.CATALOG),

Review Comment:
   I agree with the scoping of this privilege to be those 7 privileges. Given 
that though, we might want to consider whether to call this privilege 
`NAMESPACE_READ_DATA` instead, since it doesn't contain any catalog-entity 
privileges like `CATALOG_READ_PROPERTIES`.
   
   This may also highlight something we can discuss more in the dev list, which 
is the intentionally separate meanings of the privilege-inheritance from the 
*securable hierarchy* vs the logical *privilege hierarchy* defined here. As in, 
despite a privilege being applicable to a leaf entity or middle-layer entity 
(e.g. TABLE_READ_PROPERTIES, NAMESPACE_READ_PROPERTIES), and child-entity 
privilege types are also allowed to be granted *on* the securable parent (e.g. 
`GRANT TABLE_READ_PROPERTIES ON CATALOG c1 TO CATALOG_ROLE cr1`), and whether 
it's a source of confusion.



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

Reply via email to