XJDKC commented on code in PR #724:
URL: https://github.com/apache/polaris/pull/724#discussion_r1915318145
##########
service/common/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java:
##########
@@ -1627,6 +1627,12 @@ private FileIO refreshIOWithCredentials(
// the credentials should always override table-level properties, since
// storage configuration will be found at whatever entity defines it
tableProperties.putAll(credentialsMap);
+
+ // Populate the internal properties to FileIO in case the FileIO
implementation needs them
+ Map<String, String> internalProperties =
+
storageInfoEntity.map(PolarisEntity::getInternalPropertiesAsMap).orElse(Map.of());
Review Comment:
Thank you for sharing your concerns! Here’s some clarification and context
regarding this change from my POV:
1. **Distinction Between `properties` and `internalProperties`**:
* `properties` is the public property map that will be exposed to the
REST Client.
* Reference:
[IcebergCatalogAdapter::getConfig](https://github.com/apache/polaris/blob/5320af566a8829e39a55b59d6d309e7c326ad5ae/service/common/src/main/java/org/apache/polaris/service/catalog/IcebergCatalogAdapter.java#L607-L650)
* `internalProperties` is strictly internal to Polaris and is never
propagated to the REST Client. This ensures that sensitive or
implementation-specific details remain isolated.
* For example, the storage configuration map is serialized as a JSON
string and stored in internalProperties.
2. **Mitigation Against "Bad Things"**:
* The info in the `internalProperties` is not logged or exposed
externally, as it is handled within Polaris’s controlled environment.
* Right now `internalProperties` don't contain secrets but some
sensitive data used by Polaris.
Since this PR only propagates internalProperties to Polaris’s FileIO (and
not to the REST Client), it enables Polaris’s FileIO to implement custom logic
without impacting client-facing behavior. So I think it should be fine.
cc: @snazy @dimas-b @collado-mike @eric-maynard @dennishuo
--
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]
