Hi all, Thanks for writing this up David. I agree with Dmitri and JB.
Fail-fast checks on create and update are worth having. The checks are just property lookups, and a CDI bean per provider keeps PolarisAdminService free of BigLake details. Calling the beans there is fine. BigLake should stay ICEBERG_REST. HIVE and BIGQUERY have their own connection types because the remote API is different. One thing to fix before we require header.x-goog-user-project: the factory never sends it. Iceberg's RESTSessionCatalog does .withHeaders(RESTUtil.configHeaders(config)). IcebergRESTFederatedCatalogFactory builds the same client and skips that call, so the key can be on the property map and still never leave as a header. The CLI already stores it with --property, which lands on catalog properties, and the factory merges those in. I would fix the factory first, on its own, with a test. IcebergRestConnectionConfigInfoDpo already has an allowlist for this one header. I would leave that list as it is. Whether BigLake requires the header belongs in the bean. For the bean, match https://biglake.googleapis.com/iceberg/v1/restcatalog exactly, ignoring a trailing slash. Any other GCP Iceberg REST catalog should pass through as it does today. updateCatalog changes properties and storage config and leaves the connection as it was stored, so the bean needs the stored connection plus the properties and storage about to be saved. The header check has to accept catalog properties too, since that is where the CLI puts it. Remote catalog name, the quota-project header, and, when credential vending is on, a GCS location and a service account. Simple presence checks are enough. Python in its own PR, as Dmitri said. Once the factory forwards the header, the CLI can keep storing it where it does today. wdyt? Regards, Prithvi S On Sat, Sep 26, 2026 at 11:10 AM Jean-Baptiste Onofré <[email protected]> wrote: > Hi guys, > > Thanks for bringing this on the dev mailing list David! > > I agree with Dmitri's points. Here's my perspective on the questions: > > 1. Validation & abstraction: I think fail-fast is definitely good for > user experience, but keeping PolarisAdminService lean and > vendor-neutral is paramount. Dmitri's suggestion to use a > CDI-discovered SPI (e.g. ConnectionConfigValidator or > CatalogConfigValidator) is the right architecture, similar to what we > do for Authorizer or S3 vending mechanisms (ok we talk about SPI again > in Polaris but that's appropriate here :)). > 2. Ownership, dispatch, and validation scope: since catalog creation > and updates are infrequent and validation logic is in-memory, running > matching validators during create/update won't impact performance. > However, we should be cautious about over-validation: > a. We should validate structural prerequisites (e.g. ensuring > required credential-vending fields exist when credential vending is > enabled for instance) and obvious misconfigurations. > b. We should avoid overly rigid allowlists or fragile regexs on > URIs/headers. If a validator rejects unknown headers or URL formats, > any minor update could break users until a new Polaris release is > available. > 3. Generic ICEBERG_REST vs dedicated connection type: I'm strongly in > favor of keeping this under ICEBERG_REST. Introducing a dedicated > ConnectionType for each provider REST catalog would lead to enum > proliferation and defeat the purpose of having a standard ICEBERG_REST > federation layer. Dedicated connection types (like HIVE or BIGQUERY) > should be reserved for distrinct protocols or SDKs, not variations of > standard Iceberg REST endpoints. > 4. Transport & outbound headers (prerequisites): as noted during the > initial PR review, before enforcing configuration properties like > header.x-goog-user-project, we need to make sure > IcebergRESTFederatedCatalogFactory actually propagates those headers > property to the outbound HTTP client. > > I propose the following next steps: > 1. We introduce a clean CDI-based validator SPI (with BigLake > implemented as one bean) as proposed by Dmitri. > 2. We ensure outbound custom header propagation works property in > IcebergRESTFederatedCatalogFactory. > 3. Follow Dmitri's advice on keeping the Python CLI changes in a dedicated > PR. > > WDYT? > > Regards > JB > > On Sat, Sep 26, 2026 at 2:03 AM Dmitri Bourlatchkov <[email protected]> > wrote: > > > > Hi David, > > > > Thanks for stating this thread! > > > > > 1. Is provider-specific configuration validation desirable for generic > > > `ICEBERG_REST` federation? > > > > From POV, having validation can help users detect mistakes early. It is > > nice to have. > > > > In this particular case, the validation is based purely on pattern > matching > > against existing catalog properties. It does not involve I/O or external > > dependencies. > > > > I believe this kind of validation should be ok to keep in the main > codebase > > (runtime/service). I wonder what other people think about this. > > > > Side note: Please consider making Python changes in separate PRs as they > > usually need different reviewers ;) > > > > > 3. What ownership and dispatch boundary would keep the shared admin > > > workflow provider-neutral and avoid running provider-specific logic for > > > every catalog operation? > > > > Regarding abstraction, I would suggest using CDI to find multiple > > implementations of a (new) common validation interface. Admin Service > code > > will stay lean and only call the validation interface. That call will be > > dispatched to every matching CDI bean, each running logic specific to a > use > > case (e.g. BigLake). > > > > Since validation code is involved only on updates (unfrequently), and > > valation logic likely runs in-memory (fast), I do not think we need to > > worry about calling every validators on every change. > > > > The PR [5513] shows some nice (recent) examples of leveraging CDI for > > pluggable components. > > > > [5513] https://github.com/apache/polaris/pull/5513 > > > > Cheers, > > Dmitri. > > > > On Wed, Sep 23, 2026 at 12:26 AM David Chaava via dev < > > [email protected]> wrote: > > > > > Hi Polaris community, > > > > > > > > > We would like guidance on whether Polaris should support > provider-specific > > > configuration validation for external Iceberg REST catalogs, and, if > so, > > > what the appropriate extension model should be. > > > > > > > > > Context: > > > - Polaris supports the generic `ICEBERG_REST` connection type. > > > - The initial BigLake implementation added provider-specific > validation in > > > the shared `PolarisAdminService` create/update path. > > > - Review feedback on PR #5196 raised concerns about putting > > > provider-specific logic in the main admin workflow, introducing a > validator > > > without a shared abstraction, and coupling provider configuration > rules to > > > Polaris releases. > > > > > > > > > We have closed that implementation and would like to agree on the > design > > > before proposing a replacement. > > > > > > > > > Questions: > > > 1. Is provider-specific configuration validation desirable for generic > > > `ICEBERG_REST` federation? > > > 2. If so, should Polaris first define a shared abstraction or extension > > > model for such validation? > > > 3. What ownership and dispatch boundary would keep the shared admin > > > workflow provider-neutral and avoid running provider-specific logic for > > > every catalog operation? > > > 4. Under what conditions, if any, would a separate connection type be > > > preferable to extending the generic `ICEBERG_REST` flow? > > > > > > > > > Links: > > > - Tracking issue: https://github.com/apache/polaris/issues/5195 > > > - Closed initial PR: https://github.com/apache/polaris/pull/5196 > > > - Architecture feedback: > > > https://github.com/apache/polaris/pull/5196#discussion_r3846267047 > > > > > > > > > Thanks, > > > David > > > >
