Hi Dmitri, JB, and Prithvi, Thank you for the thoughtful feedback. The comments help clarify both the architectural concern with the initial implementation and the configuration gap we intended to address.
Our proposed direction is a CDI-discovered validator SPI with a BigLake-specific implementation, keeping `PolarisAdminService` provider-neutral. The validator would receive the stored connection configuration and the catalog properties and storage configuration being saved, so the same relevant checks can be applied during creation and update. We’ll scope the BigLake validator to the canonical BigLake Iceberg REST endpoint, treating a trailing slash as equivalent. Other GCP-backed Iceberg REST catalogs should continue to pass through unchanged. Within that scope, validation would focus on required configuration: tthe remote catalog name, the `x-goog-user-project` value, and GCS settings when Polaris-managed credential vending requires them. We would avoid broad URL or header allowlists. We agree that outbound custom-header propagation should be addressed separately first, with request-level test coverage in `IcebergRESTFederatedCatalogFactory`. The Python CLI work will also remain in a separate PR. Once the header-propagation change is addressed, we’ll prepare the validator SPI and BigLake implementation around these boundaries. If we’ve misunderstood any part of the suggested scope, please let us know before we proceed. Thanks again, David On Sat, Sep 26, 2026 at 7:53 AM Prithvi S <[email protected]> wrote: > 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 >> > > >> >
