Hi David Thanks for the update! We will take a look on the PR.
Regards JB On Thu, Oct 15, 2026 at 5:01 PM David Chaava via dev <[email protected]> wrote: > Hi Dmitri, JB, and Prithvi, > > We have opened a draft PR implementing the validator approach discussed > here: > > https://github.com/apache/polaris/pull/5788 > > It introduces a CDI-discovered CatalogConfigValidator SPI with a BigLake > implementation, keeping PolarisAdminService provider-neutral. Validation > runs on creation and update using the effective connection, catalog > properties, and storage configuration. > > The BigLake checks apply only to the canonical endpoint, treating a > trailing slash as equivalent, and cover the required configuration > discussed in this thread. The PR includes validator and management-service > tests. > > Outbound header propagation is being addressed separately in #5613 > <https://github.com/apache/polaris/pull/5613>, which is still open. We > plan > to merge the validator after that fix. > > We would appreciate it if you could review this PR so we can move it > forward. > > Thanks, > David > > On Wed, Sep 30, 2026 at 11:06 AM David Chaava <[email protected]> > wrote: > > > 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 > >>> > > > >>> > >> >
