lasdf1234 commented on PR #11294:
URL: https://github.com/apache/gravitino/pull/11294#issuecomment-4609661990

   > I think there is still a REST proxy issue here. 
`filterCredentialProperties()` now keeps `client.refresh-credentials-endpoint`, 
`adls.refresh-credentials-endpoint`, and 
`gcs.oauth2.refresh-credentials-endpoint`, and the REST catalog path copies the 
filtered `FileIO.properties()` into the downstream `LoadTableResponse.config()` 
in create/stage-create/load-table.
   > 
   > Iceberg clients consume these keys directly from 
`LoadTableResponse.config()`: `RESTSessionCatalog.tableFileIO()` merges 
`response.config()` into the FileIO initialization properties, and the 
S3/GCS/ADLS vended credential providers use those refresh endpoints for later 
refreshes. For a proxied REST catalog, forwarding the upstream endpoint 
unchanged can therefore send clients to the wrong place: a relative upstream 
path will be resolved against the Gravitino catalog URI, while an absolute 
upstream URL lets clients bypass Gravitino and call the upstream `/credentials` 
endpoint directly, which can break auth/topology assumptions.
   > 
   > I think Gravitino should either strip upstream refresh endpoint properties 
from the proxied load-table config, or rewrite them to the current Gravitino 
endpoint (`v1/<catalog>/namespaces/.../tables/.../credentials`) instead of 
forwarding the upstream value unchanged.
   
   Got it. The code has been modified as per your instructions. Now it only 
transmits the "upstream refresh endpoint properties".


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