XJDKC commented on code in PR #701:
URL: https://github.com/apache/polaris/pull/701#discussion_r1911920498
##########
polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisCredentialProperty.java:
##########
@@ -41,7 +41,12 @@ public enum PolarisCredentialProperty {
"the azure storage account host",
"the azure account name + endpoint that will append to the
ADLS_SAS_TOKEN_PREFIX"),
EXPIRATION_TIME(
- Long.class, "expiration-time", "the expiration time for the access
token, in milliseconds");
+ Long.class, "expiration-time", "the expiration time for the access
token, in milliseconds"),
+ ADDITIONAL_STORAGE_CONFIG(
+ String.class,
+ "additional-storage-config",
+ "the additional storage config for the cloud storage"),
Review Comment:
**Context**
`getSubscopedCreds` serves two purposes:
1. For Polaris: Supplies credentials to initialize FileIO, enabling storage
access for Polaris's operations such as `doCommit`, `doRefresh`, and
`registerTable`
*
[BasePolarisCatalog.java#L1607-L1631](https://github.com/apache/polaris/blob/7d53b73ca21c93c474c19722356eaac121051fc1/service/common/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java#L1607-L1631)
2. For REST Client: Vends credentials to the REST Client, enabling it to
access storage.
*
[PolarisCatalogHandlerWrapper#L617-L635](https://github.com/apache/polaris/blob/16f00936f6632e316e5f89c12c91c635ba93ee01/service/common/src/main/java/org/apache/polaris/service/catalog/PolarisCatalogHandlerWrapper.java#L617-L635)
**Our Intention**
The `PolarisMetaStoreManager` we implement may need to dynamically select
AWS S3 or Azure storage endpoints at runtime based on routing requirements
(e.g., proxy, PrivateLink). It interprets these details contextually. This
requires updating the routing logic to ensure Polaris can access the storage by
sending requests to the correct Hostnames/IPs, so we implement our own FileIO.
To achieve this, we need to pass additional information along with
credentials to Polaris. We consider `getSubscopedCreds` a suitable mechanism
for providing these details, **as it effectively informs Polaris how to access
storage. But right now, this api happens to mostly be credentials**.
**Problem**
However, as you mentioned, including additional props in `getSubscopedCreds`
also exposes them to the REST client, which uses the same API for a different
purpose. Since the client (query engine) operates in the user’s environment,
these info is unnecessary and should not be shared with the REST client due to
security and relevance concerns.
**Proposed Solution**
To resolve this, we can filter out the additional storage configuration from
the loadTable response, ensuring that it is only utilized internally by Polaris
and not sent to the REST client.
cc: @dimas-b: Hope this answers your question!
--
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]
