You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "youngrae81 (via GitHub)" <gi...@apache.org> on 2026/10/02 08:30:05 UTC

[I] Apply `connectionConfigInfo.properties` when initializing HADOOP and HIVE federated catalogs [polaris]

youngrae81 opened a new issue, #5676:
URL: https://github.com/apache/polaris/issues/5676

   ### Is your feature request related to a problem? Please describe.
   
   Since #5644, `connectionConfigInfo.properties` is persisted and returned for `HADOOP` and `HIVE` connection configurations. It is still not used anywhere: `HadoopConnectionConfigInfoDpo.asIcebergCatalogProperties` and `HiveConnectionConfigInfoDpo.asIcebergCatalogProperties` build the Iceberg catalog properties from `uri`, `warehouse`, the authentication parameters and the vended credentials only, and never read `getProperties()`. So an operator can set the map, see it echoed back by `GET /catalogs/{name}`, and observe no effect on the federated catalog.
   
   The other connection types already apply the map, but with different policies:
   
   | Connection type | What `properties` does at catalog initialization |
   |---|---|
   | `ICEBERG_REST` | Allow-list: only `header.x-goog-user-project` is forwarded (`IcebergRestConnectionConfigInfoDpo.ALLOWED_PROPERTIES`) |
   | `BIGQUERY` | Everything is merged (`putAll(getProperties())`), and the docs describe the map as the place for BigQuery-specific settings |
   | `HADOOP`, `HIVE` | Ignored |
   
   The spec only says "Additional connection properties", so there is no documented reason for the difference. 
   #5644 deliberately left this out of scope so it could be discussed separately (agreed in review).
   
   Why it matters: the Hive federation docs note that S3-compatible warehouses may need non-secret client settings such as an endpoint or path-style access. Today the only way to pass those is the catalog-level `properties` map, which is returned to every catalog client through `/v1/config`.
   `connectionConfigInfo.properties` is visible only through the management API, so it is the natural place for server-side client settings that should not be advertised to clients.
   
   ### Describe the solution you'd like
   
   Forward `connectionConfigInfo.properties` into the Iceberg catalog properties for `HADOOP` and `HIVE`, following the `ICEBERG_REST` precedent of an explicit allow-list rather than merging everything:
   
   - Hive: Iceberg `HiveCatalog` properties such as `clients` and `list-all-tables`.
   - Hadoop: Iceberg `HadoopCatalog` properties such as `suppress-permission-error` and `lock-impl`.
   - Non-secret FileIO client settings (`s3.endpoint`, `s3.path-style-access`, `s3.region`) for both, since those are the settings the docs say S3-compatible stores need.
   
   The exact list is the thing to agree on here. Keys that select implementations (`io-impl`, `catalog-impl`) and anything that carries credentials should stay out, consistent with the existing stance that caller-supplied FileIO keys are not trusted.
   
   Connection-level values should take precedence over catalog-level `properties` (the factories already merge in that order with `RESTUtil.merge`), matching what the Iceberg REST federation docs state.
   
   If a shared allow-list filter makes sense, it could live in `ConnectionConfigInfoDpo` so each type only declares its list. I am happy to send a PR once the direction and the list have a +1. 
   
   ### Describe alternatives you've considered
   
   1. Merge everything, like `BIGQUERY`. Simplest and most flexible, but it lets a catalog admin inject arbitrary FileIO and implementation keys into the server-side Hive/Hadoop clients, which is the pattern Polaris avoids elsewhere.
   2. Keep the map as stored metadata only and document that `HADOOP`/`HIVE` ignore it. Removes the confusion without new behavior, but leaves no admin-only place for non-secret client settings.
   
   ### Additional context
   
   - #5644 (persist and return the map; propagation explicitly deferred in review).
   - `ICEBERG_REST` allow-list: `IcebergRestConnectionConfigInfoDpo`; `BIGQUERY` merge-all: `BigQueryMetastoreConnectionConfigInfoDpo`.
   - Factories: `HiveFederatedCatalogFactory`, `HadoopFederatedCatalogFactory` (both merge catalog properties with `asIcebergCatalogProperties` and call `initialize`).
   - Related: #5613 (open) fixes `header.*` forwarding on the REST client after initialization; different type, same "connection properties reach the client" theme.


-- 
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: issues-unsubscribe@polaris.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Apply `connectionConfigInfo.properties` when initializing HADOOP and HIVE federated catalogs [polaris]

Posted by "youngrae81 (via GitHub)" <gi...@apache.org>.
youngrae81 commented on issue #5676:
URL: https://github.com/apache/polaris/issues/5676#issuecomment-6121563362

   Related data point: in #5613, @flyrain suggested that client-side settings for a federated catalog belong in `ConnectionConfigInfo` rather than catalog-level `properties`, since catalog properties follow the catalog when the connection type changes. That is the same reasoning behind option 1 here (connection-level map, explicit allow-list), applied to HADOOP and HIVE.
   
   @flyrain @dimas-b @jbonofre: does the allow-list direction look right to you? #5680 has a concrete list to react to (Hive: `clients`, `list all-tables`; Hadoop: `suppress-permission-error`; both: `s3.endpoint`, `s3.path-style-access`, `client.region`).
   If you would rather see a different shape, for example widening it per type or a dedicated field in the spec, I am happy to rework the PR.


-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Apply `connectionConfigInfo.properties` when initializing HADOOP and HIVE federated catalogs [polaris]

Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain closed issue #5676: Apply `connectionConfigInfo.properties` when initializing HADOOP and HIVE federated catalogs
URL: https://github.com/apache/polaris/issues/5676


-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Apply `connectionConfigInfo.properties` when initializing HADOOP and HIVE federated catalogs [polaris]

Posted by "youngrae81 (via GitHub)" <gi...@apache.org>.
youngrae81 commented on issue #5676:
URL: https://github.com/apache/polaris/issues/5676#issuecomment-5951710118

   Opened #5680 with a concrete proposal so the allow-list can be discussed against actual code rather than in the abstract. 
   One change from the list sketched above: `lock-impl` is dropped, since `HadoopCatalog` loads it reflectively and that makes it an implementation-selecting key like `io-impl`. 
   The PR also documents that the `s3.*` / `client.region` keys only take effect when `io-impl` is set to `S3FileIO` at the catalog level. Happy to adjust the list either way. 


-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org