You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "joyhaldar (via GitHub)" <gi...@apache.org> on 2026/03/24 15:53:52 UTC

[PR] Add BigQuery Metastore federation support [polaris]

joyhaldar opened a new pull request, #4050:
URL: https://github.com/apache/polaris/pull/4050

   Adds federation support for BigQuery Metastore catalogs, enabling Polaris to serve as a unified REST catalog interface for Iceberg tables managed in BigQuery Metastore.
   
   ### Changes
   - `BigQueryMetastoreFederatedCatalogFactory` - factory for creating BigQuery Metastore catalog handles
   - `BigQueryMetastoreConnectionConfigInfoDpo` - connection configuration with all BigQueryProperties
   - `BIGQUERY` connection type in `ConnectionType`
   - OpenAPI schema for `BigQueryMetastoreConnectionConfigInfo`
   - Support for service account impersonation via Iceberg's BigQueryProperties
   
   ## Checklist
   - [x] ๐Ÿ›ก๏ธ Don't disclose security issues! (contact security@apache.org)
   - [x] ๐Ÿ”— Clearly explained why the changes are needed
   - [ ] ๐Ÿงช Added/updated tests with good coverage, or manually tested (and explained how)
   - [x] ๐Ÿ’ก Added comments for complex logic
   - [ ] ๐Ÿงพ Updated `CHANGELOG.md` (if needed)
   - [ ] ๐Ÿ“š Updated documentation in `site/content/in-dev/unreleased` (if needed)


-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r2985527287


##########
runtime/server/build.gradle.kts:
##########
@@ -43,6 +43,10 @@ dependencies {
     runtimeOnly(project(":polaris-extensions-federation-hive"))
   }
 
+  if ((project.findProperty("NonRESTCatalogs") as String?)?.contains("BIGQUERY") == true) {
+    runtimeOnly(project(":polaris-extensions-federation-bigquery"))

Review Comment:
   Should I remove the conditional statement and include it by default in the standard distribution?



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3019156955


##########
spec/polaris-management-service.yml:
##########
@@ -965,6 +967,44 @@ components:
         warehouse:
           type: string
           description: The warehouse location for the hive catalog.
+    
+    BigQueryMetastoreConnectionConfigInfo:
+      type: object
+      description: Configuration necessary for connecting to a BigQuery Metastore Catalog
+      allOf:
+        - $ref: '#/components/schemas/ConnectionConfigInfo'
+      properties:
+        warehouse:
+          type: string
+          description: The GCS bucket path for the catalog (e.g., gs://bucket-name)
+        gcpProjectId:
+          type: string
+          description: The GCP project ID
+        gcpLocation:
+          type: string
+          description: The GCP location (default = us)
+        listAllTables:
+          type: boolean
+          description: Whether to list all tables (default = true)
+        impersonateServiceAccount:
+          type: string
+          description: Service account email to impersonate
+        impersonateLifetimeSeconds:
+          type: integer
+          description: Token lifetime in seconds for impersonation (default = 3600)
+        impersonateScopes:
+          type: array
+          items:
+            type: string
+          description: OAuth scopes for impersonation (default = cloud-platform)
+        impersonateDelegates:
+          type: array
+          items:
+            type: string
+          description: Delegation chain for impersonation

Review Comment:
   How often does the BQMS config surface change? For example, when new fields are introduced or existing fields are updated. Locking every field into the spec creates a maintenance burden, since each upstream change requires a Polaris spec PR, client regeneration, and a new release.
   
   Instead, we could consider using a bag of open properties to keep the model more flexible.
   ```
           properties:
             type: object
             additionalProperties:
               type: string
   ```



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3034184491


##########
extensions/federation/bigquery/src/main/java/org/apache/polaris/extensions/federation/bigquery/BigQueryMetastoreFederatedCatalogFactory.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.extensions.federation.bigquery;
+
+import io.smallrye.common.annotation.Identifier;
+import jakarta.enterprise.context.ApplicationScoped;
+import java.util.Map;
+import org.apache.iceberg.catalog.Catalog;
+import org.apache.iceberg.gcp.bigquery.BigQueryMetastoreCatalog;
+import org.apache.iceberg.rest.RESTUtil;
+import org.apache.polaris.core.catalog.ExternalCatalogFactory;
+import org.apache.polaris.core.catalog.GenericTableCatalog;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.AuthenticationType;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.connection.bigquery.BigQueryMetastoreConnectionConfigInfoDpo;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+
+/**
+ * Factory class for creating a BigQuery Metastore catalog handle based on connection configuration.
+ */
+@ApplicationScoped
+@Identifier(ConnectionType.BIGQUERY_FACTORY_IDENTIFIER)
+public class BigQueryMetastoreFederatedCatalogFactory implements ExternalCatalogFactory {
+
+  @Override
+  public Catalog createCatalog(
+      ConnectionConfigInfoDpo connectionConfigInfoDpo,
+      PolarisCredentialManager polarisCredentialManager,
+      Map<String, String> catalogProperties) {
+
+    // Currently, Polaris supports BigQuery Metastore federation only via IMPLICIT authentication.
+    // Hence, prior to initializing the configuration, ensure that the catalog uses
+    // IMPLICIT authentication.
+    AuthenticationParametersDpo authenticationParametersDpo =
+        connectionConfigInfoDpo.getAuthenticationParameters();
+    if (authenticationParametersDpo != null

Review Comment:
   I believe declaring the `IMPLICIT` authentication explicitly ๐Ÿ˜‰ is actually good. It increases clarity about the intended catalog behaviour. It is also more robust in case of future API and backend code changes.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r2984649679


##########
runtime/server/build.gradle.kts:
##########
@@ -43,6 +43,10 @@ dependencies {
     runtimeOnly(project(":polaris-extensions-federation-hive"))
   }
 
+  if ((project.findProperty("NonRESTCatalogs") as String?)?.contains("BIGQUERY") == true) {
+    runtimeOnly(project(":polaris-extensions-federation-bigquery"))

Review Comment:
   From my POV it seems reasonable to include it by default in Apache Polaris.
   
   Downstream builds can make their own decisions since the impl. is well-isolated in terms of build modules.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3084903677


##########
runtime/server/build.gradle.kts:
##########
@@ -43,6 +43,10 @@ dependencies {
     runtimeOnly(project(":polaris-extensions-federation-hive"))
   }
 
+  if ((project.findProperty("NonRESTCatalogs") as String?)?.contains("BIGQUERY") == true) {
+    runtimeOnly(project(":polaris-extensions-federation-bigquery"))

Review Comment:
   I can help on the LICENSE/NOTICE. I'm fine to do it in a follow up 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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain closed pull request #4050: Add BigQuery Metastore federation support
URL: https://github.com/apache/polaris/pull/4050


-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3025896082


##########
polaris-core/src/main/java/org/apache/polaris/core/connection/bigquery/BigQueryMetastoreConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,230 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.core.connection.bigquery;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.MoreObjects;
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+import org.apache.polaris.core.credentials.connection.ConnectionCredentials;
+import org.apache.polaris.core.identity.dpo.ServiceIdentityInfoDpo;
+import org.apache.polaris.core.identity.provider.ServiceIdentityProvider;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo} defined in the API
+ * model.
+ */
+public class BigQueryMetastoreConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+  public static final String DEFAULT_URI = "https://bigquery.googleapis.com";
+
+  // BigQuery Metastore catalog property keys
+  private static final String GCP_BIGQUERY_PROJECT_ID = "gcp.bigquery.project-id";
+  private static final String GCP_BIGQUERY_LOCATION = "gcp.bigquery.location";
+  private static final String GCP_BIGQUERY_LIST_ALL_TABLES = "gcp.bigquery.list-all-tables";
+  private static final String IMPERSONATE_SERVICE_ACCOUNT =
+      "gcp.bigquery.impersonate.service-account";
+  private static final String IMPERSONATE_LIFETIME_SECONDS =
+      "gcp.bigquery.impersonate.lifetime-seconds";
+  private static final String IMPERSONATE_SCOPES = "gcp.bigquery.impersonate.scopes";
+  private static final String IMPERSONATE_DELEGATES = "gcp.bigquery.impersonate.delegates";

Review Comment:
   The property keys in Iceberg's [BigQueryProperties](https://github.com/apache/iceberg/blob/main/bigquery/src/main/java/org/apache/iceberg/gcp/bigquery/BigQueryProperties.java) are package-private, so I had to duplicate them here. I can add a comment referencing the Iceberg source. Let me know what you think.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3034154418


##########
polaris-core/src/main/java/org/apache/polaris/core/connection/bigquery/BigQueryMetastoreConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,230 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.core.connection.bigquery;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.MoreObjects;
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+import org.apache.polaris.core.credentials.connection.ConnectionCredentials;
+import org.apache.polaris.core.identity.dpo.ServiceIdentityInfoDpo;
+import org.apache.polaris.core.identity.provider.ServiceIdentityProvider;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo} defined in the API
+ * model.
+ */
+public class BigQueryMetastoreConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {

Review Comment:
   No worries. Let's keep existing pattern.



##########
extensions/federation/bigquery/src/main/java/org/apache/polaris/extensions/federation/bigquery/BigQueryMetastoreFederatedCatalogFactory.java:
##########
@@ -65,6 +65,8 @@ public Catalog createCatalog(
             catalogProperties != null ? catalogProperties : Map.of(),
             connectionConfigInfoDpo.asIcebergCatalogProperties(polarisCredentialManager));
 
+    // Credentials are resolved via Google Application Default Credentials (ADC).
+    // GCS storage operations use the same ADC credentials.

Review Comment:
   The second statement is not 100% accurate. In principle a user can override GCS storage credentials:
   
   https://github.com/apache/polaris/blob/a6ca7cb2a27eb685522c8dc09d15217683714fde/runtime/service/src/main/java/org/apache/polaris/service/storage/StorageConfiguration.java#L151
   
   I'd propose adding `... by default`.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on PR #4050:
URL: https://github.com/apache/polaris/pull/4050#issuecomment-4183745192

   > I have two major questions/"concerns":
   > 
   > 1. As we introduce new dependencies (for BigQuery and transitive), the LICENSE/NOTICE should be updated according to the second point.
   > 2. The question is: do we ship bigquery metastore federation by default in the server ? If yes, the LICENSE/NOTICE of the server should be updated. If no, no need to update it.
   
   Made it optional for now to unblock CI. Will follow up on LICENSE/NOTICE updates separately if we decide to ship it by default.


-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3087637094


##########
polaris-core/src/main/java/org/apache/polaris/core/connection/bigquery/BigQueryMetastoreConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.core.connection.bigquery;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.MoreObjects;
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+import org.apache.polaris.core.credentials.connection.ConnectionCredentials;
+import org.apache.polaris.core.identity.dpo.ServiceIdentityInfoDpo;
+import org.apache.polaris.core.identity.provider.ServiceIdentityProvider;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo} defined in the API
+ * model.
+ */
+public class BigQueryMetastoreConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+  public static final String DEFAULT_URI = "https://bigquery.googleapis.com";
+
+  // BigQuery Metastore catalog property keys
+  private static final String GCP_BIGQUERY_PROJECT_ID = "gcp.bigquery.project-id";
+
+  private final String warehouse;
+  private final String gcpProjectId;
+
+  public BigQueryMetastoreConnectionConfigInfoDpo(
+      @JsonProperty(value = "uri", required = true) @Nonnull String uri,
+      @JsonProperty(value = "authenticationParameters", required = false) @Nullable
+          AuthenticationParametersDpo authenticationParameters,
+      @JsonProperty(value = "warehouse", required = true) @Nonnull String warehouse,
+      @JsonProperty(value = "gcpProjectId", required = true) @Nonnull String gcpProjectId,
+      @JsonProperty(value = "properties", required = false) @Nullable
+          Map<String, String> properties,
+      @JsonProperty(value = "serviceIdentity", required = false) @Nullable
+          ServiceIdentityInfoDpo serviceIdentity) {
+    super(
+        ConnectionType.BIGQUERY.getCode(),
+        uri,
+        authenticationParameters,
+        serviceIdentity,
+        properties);
+    this.warehouse = warehouse;
+    this.gcpProjectId = gcpProjectId;
+  }
+
+  public String getWarehouse() {
+    return warehouse;
+  }
+
+  public String getGcpProjectId() {
+    return gcpProjectId;
+  }
+
+  @Override
+  public String toString() {
+    return MoreObjects.toStringHelper(this)
+        .add("connectionTypeCode", getConnectionTypeCode())
+        .add("uri", getUri())
+        .add("warehouse", warehouse)
+        .add("gcpProjectId", gcpProjectId)
+        .add(
+            "authenticationParameters",
+            getAuthenticationParameters() != null
+                ? getAuthenticationParameters().toString()
+                : "null")
+        .toString();
+  }
+
+  @Override
+  public @Nonnull Map<String, String> asIcebergCatalogProperties(
+      PolarisCredentialManager polarisCredentialManager) {
+    HashMap<String, String> properties = new HashMap<>();
+
+    // Required properties for BigQueryMetastoreCatalog
+    properties.put(GCP_BIGQUERY_PROJECT_ID, gcpProjectId);
+    properties.put(CatalogProperties.WAREHOUSE_LOCATION, warehouse);

Review Comment:
   @dimas-b proposal works for me, as soon as clear documented. That said, I'm also fine to keep them as it's "bigquery" specific.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "nandorKollar (via GitHub)" <gi...@apache.org>.
nandorKollar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3091851946


##########
extensions/federation/bigquery/src/main/java/org/apache/polaris/extensions/federation/bigquery/BigQueryMetastoreFederatedCatalogFactory.java:
##########
@@ -0,0 +1,89 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.extensions.federation.bigquery;
+
+import io.smallrye.common.annotation.Identifier;
+import jakarta.enterprise.context.ApplicationScoped;
+import java.util.Map;
+import org.apache.iceberg.catalog.Catalog;
+import org.apache.iceberg.gcp.bigquery.BigQueryMetastoreCatalog;
+import org.apache.iceberg.rest.RESTUtil;
+import org.apache.polaris.core.catalog.FederatedCatalogFactory;
+import org.apache.polaris.core.catalog.GenericTableCatalog;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.AuthenticationType;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+
+/**
+ * Factory class for creating a BigQuery Metastore catalog handle based on connection configuration.
+ */
+@ApplicationScoped
+@Identifier(ConnectionType.BIGQUERY_FACTORY_IDENTIFIER)
+public class BigQueryMetastoreFederatedCatalogFactory implements FederatedCatalogFactory {
+
+  @Override
+  public Catalog createCatalog(
+      ConnectionConfigInfoDpo connectionConfigInfoDpo,
+      PolarisCredentialManager polarisCredentialManager,
+      Map<String, String> catalogProperties) {
+
+    // Currently, Polaris supports BigQuery Metastore federation only via IMPLICIT authentication.
+    // Hence, prior to initializing the configuration, ensure that the catalog uses
+    // IMPLICIT authentication.
+    AuthenticationParametersDpo authenticationParametersDpo =
+        connectionConfigInfoDpo.getAuthenticationParameters();
+    if (authenticationParametersDpo.getAuthenticationTypeCode()
+        != AuthenticationType.IMPLICIT.getCode()) {
+      throw new IllegalStateException(
+          "BigQuery Metastore federation only supports IMPLICIT authentication.");
+    }
+
+    Map<String, String> properties = connectionConfigInfoDpo.getProperties();
+    String warehouse = properties.get("warehouse");
+    if (warehouse == null || warehouse.isEmpty()) {
+      throw new IllegalArgumentException("warehouse is required for BigQuery Metastore federation");
+    }
+    if (properties.get("gcp.bigquery.project-id") == null) {

Review Comment:
   Is empty string accepted for `gcp.bigquery.project-id`?



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "nandorKollar (via GitHub)" <gi...@apache.org>.
nandorKollar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3091869581


##########
polaris-core/src/main/java/org/apache/polaris/core/connection/bigquery/BigQueryMetastoreConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,123 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.core.connection.bigquery;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.MoreObjects;
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+import org.apache.polaris.core.credentials.connection.ConnectionCredentials;
+import org.apache.polaris.core.identity.dpo.ServiceIdentityInfoDpo;
+import org.apache.polaris.core.identity.provider.ServiceIdentityProvider;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo} defined in the API
+ * model.
+ */
+public class BigQueryMetastoreConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+  public static final String DEFAULT_URI = "https://bigquery.googleapis.com";

Review Comment:
   nit: I think we can keep this private.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3078827107


##########
spec/polaris-management-service.yml:
##########
@@ -965,6 +967,27 @@ components:
         warehouse:
           type: string
           description: The warehouse location for the hive catalog.
+    
+    BigQueryMetastoreConnectionConfigInfo:
+      type: object
+      description: Configuration necessary for connecting to a BigQuery Metastore Catalog
+      allOf:
+        - $ref: '#/components/schemas/ConnectionConfigInfo'
+      properties:
+        warehouse:
+          type: string
+          description: The warehouse location for the BigQuery Metastore Catalog
+        gcpProjectId:
+          type: string
+          description: The GCP project ID
+        properties:
+          type: object
+          additionalProperties:
+            type: string
+            description: Additional catalog properties

Review Comment:
   I have moved `properties` to parent `ConnectionConfigInfo` so all federation types can use it.
   



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r2982783639


##########
runtime/server/build.gradle.kts:
##########
@@ -43,6 +43,10 @@ dependencies {
     runtimeOnly(project(":polaris-extensions-federation-hive"))
   }
 
+  if ((project.findProperty("NonRESTCatalogs") as String?)?.contains("BIGQUERY") == true) {
+    runtimeOnly(project(":polaris-extensions-federation-bigquery"))

Review Comment:
   This means `BIGQUERY` will NOT be included into the standard Apache Polaris distributions... Is that intended?



##########
extensions/federation/bigquery/src/main/java/org/apache/polaris/extensions/federation/bigquery/BigQueryMetastoreFederatedCatalogFactory.java:
##########
@@ -0,0 +1,79 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.extensions.federation.bigquery;
+
+import io.smallrye.common.annotation.Identifier;
+import jakarta.enterprise.context.ApplicationScoped;
+import java.util.Map;
+import org.apache.iceberg.catalog.Catalog;
+import org.apache.iceberg.gcp.bigquery.BigQueryMetastoreCatalog;
+import org.apache.iceberg.rest.RESTUtil;
+import org.apache.polaris.core.catalog.ExternalCatalogFactory;
+import org.apache.polaris.core.catalog.GenericTableCatalog;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.AuthenticationType;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.connection.bigquery.BigQueryMetastoreConnectionConfigInfoDpo;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+
+/** Factory class for creating a BigQuery Metastore catalog handle based on connection configuration. */
+@ApplicationScoped
+@Identifier(ConnectionType.BIGQUERY_FACTORY_IDENTIFIER)
+public class BigQueryMetastoreFederatedCatalogFactory implements ExternalCatalogFactory {
+
+  @Override
+  public Catalog createCatalog(
+      ConnectionConfigInfoDpo connectionConfigInfoDpo,
+      PolarisCredentialManager polarisCredentialManager,
+      Map<String, String> catalogProperties) {
+
+    // Currently, Polaris supports BigQuery Metastore federation only via IMPLICIT authentication.
+    // Hence, prior to initializing the configuration, ensure that the catalog uses
+    // IMPLICIT authentication.
+    AuthenticationParametersDpo authenticationParametersDpo =
+        connectionConfigInfoDpo.getAuthenticationParameters();
+    if (authenticationParametersDpo.getAuthenticationTypeCode()
+        != AuthenticationType.IMPLICIT.getCode()) {
+      throw new IllegalStateException(
+          "BigQuery Metastore federation only supports IMPLICIT authentication.");
+    }
+
+    String warehouse =
+        ((BigQueryMetastoreConnectionConfigInfoDpo) connectionConfigInfoDpo).getWarehouse();
+    Map<String, String> mergedProperties =
+        RESTUtil.merge(
+            catalogProperties != null ? catalogProperties : Map.of(),
+            connectionConfigInfoDpo.asIcebergCatalogProperties(polarisCredentialManager));
+
+    BigQueryMetastoreCatalog bigQueryMetastoreCatalog = new BigQueryMetastoreCatalog();
+    bigQueryMetastoreCatalog.initialize(warehouse, mergedProperties);

Review Comment:
   How / where do it get BigQuery connection credentials?



##########
spec/polaris-management-service.yml:
##########
@@ -933,6 +934,7 @@ components:
           ICEBERG_REST: "#/components/schemas/IcebergRestConnectionConfigInfo"
           HADOOP: "#/components/schemas/HadoopConnectionConfigInfo"
           HIVE: "#/components/schemas/HiveConnectionConfigInfo"
+          BIGQUERY: "#/components/schemas/BigQueryMetastoreConnectionConfigInfo"

Review Comment:
   Please open a `dev` ML discussion for this (it's customary for all REST API changes).



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3041749275


##########
spec/polaris-management-service.yml:
##########
@@ -965,6 +967,27 @@ components:
         warehouse:
           type: string
           description: The warehouse location for the hive catalog.
+    
+    BigQueryMetastoreConnectionConfigInfo:
+      type: object
+      description: Configuration necessary for connecting to a BigQuery Metastore Catalog
+      allOf:
+        - $ref: '#/components/schemas/ConnectionConfigInfo'
+      properties:
+        warehouse:
+          type: string
+          description: The warehouse location for the BigQuery Metastore Catalog
+        gcpProjectId:
+          type: string
+          description: The GCP project ID
+        properties:
+          type: object
+          additionalProperties:
+            type: string
+            description: Additional catalog properties

Review Comment:
   Yes, the more I think about it the more I like the idea of supporting general `properties` inside `ConnectionConfigInfo`. That REST API change should be able to support both  BigQuery Catalog federation and #3729 (CC: @PhillHenry).



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3041785575


##########
spec/polaris-management-service.yml:
##########
@@ -965,6 +967,27 @@ components:
         warehouse:
           type: string
           description: The warehouse location for the hive catalog.
+    
+    BigQueryMetastoreConnectionConfigInfo:
+      type: object
+      description: Configuration necessary for connecting to a BigQuery Metastore Catalog
+      allOf:
+        - $ref: '#/components/schemas/ConnectionConfigInfo'
+      properties:
+        warehouse:
+          type: string
+          description: The warehouse location for the BigQuery Metastore Catalog
+        gcpProjectId:
+          type: string
+          description: The GCP project ID
+        properties:
+          type: object
+          additionalProperties:
+            type: string
+            description: Additional catalog properties

Review Comment:
   Here is another use case that justifies it. A client for the remote catalog may support more properties than those defined in the spec. Without a freeform `properties` field, we would need to keep updating the spec to accommodate properties already supported by existing clients (for example `HMS` or `BQMS`), which is both unnecessary and heavy. With properties, users can simply update their federated catalog in the metastore instead.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3087582707


##########
polaris-core/src/main/java/org/apache/polaris/core/connection/bigquery/BigQueryMetastoreConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.core.connection.bigquery;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.MoreObjects;
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+import org.apache.polaris.core.credentials.connection.ConnectionCredentials;
+import org.apache.polaris.core.identity.dpo.ServiceIdentityInfoDpo;
+import org.apache.polaris.core.identity.provider.ServiceIdentityProvider;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo} defined in the API
+ * model.
+ */
+public class BigQueryMetastoreConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+  public static final String DEFAULT_URI = "https://bigquery.googleapis.com";
+
+  // BigQuery Metastore catalog property keys
+  private static final String GCP_BIGQUERY_PROJECT_ID = "gcp.bigquery.project-id";
+
+  private final String warehouse;
+  private final String gcpProjectId;
+
+  public BigQueryMetastoreConnectionConfigInfoDpo(
+      @JsonProperty(value = "uri", required = true) @Nonnull String uri,
+      @JsonProperty(value = "authenticationParameters", required = false) @Nullable
+          AuthenticationParametersDpo authenticationParameters,
+      @JsonProperty(value = "warehouse", required = true) @Nonnull String warehouse,
+      @JsonProperty(value = "gcpProjectId", required = true) @Nonnull String gcpProjectId,
+      @JsonProperty(value = "properties", required = false) @Nullable
+          Map<String, String> properties,
+      @JsonProperty(value = "serviceIdentity", required = false) @Nullable
+          ServiceIdentityInfoDpo serviceIdentity) {
+    super(
+        ConnectionType.BIGQUERY.getCode(),
+        uri,
+        authenticationParameters,
+        serviceIdentity,
+        properties);
+    this.warehouse = warehouse;
+    this.gcpProjectId = gcpProjectId;
+  }
+
+  public String getWarehouse() {
+    return warehouse;
+  }
+
+  public String getGcpProjectId() {
+    return gcpProjectId;
+  }
+
+  @Override
+  public String toString() {
+    return MoreObjects.toStringHelper(this)
+        .add("connectionTypeCode", getConnectionTypeCode())
+        .add("uri", getUri())
+        .add("warehouse", warehouse)
+        .add("gcpProjectId", gcpProjectId)
+        .add(
+            "authenticationParameters",
+            getAuthenticationParameters() != null
+                ? getAuthenticationParameters().toString()
+                : "null")
+        .toString();
+  }
+
+  @Override
+  public @Nonnull Map<String, String> asIcebergCatalogProperties(
+      PolarisCredentialManager polarisCredentialManager) {
+    HashMap<String, String> properties = new HashMap<>();
+
+    // Required properties for BigQueryMetastoreCatalog
+    properties.put(GCP_BIGQUERY_PROJECT_ID, gcpProjectId);
+    properties.put(CatalogProperties.WAREHOUSE_LOCATION, warehouse);

Review Comment:
   Can I do this in a follow-up PR?
   
   I'm trying to figure out the right pattern here, [Hadoop](https://github.com/apache/polaris/blob/main/extensions/federation/hadoop/src/main/java/org/apache/polaris/extensions/federation/hadoop/HadoopFederatedCatalogFactory.java) and [Hive](https://github.com/apache/polaris/blob/main/extensions/federation/hive/src/main/java/org/apache/polaris/extensions/federation/hive/HiveFederatedCatalogFactory.java) have `warehouse` as an explicit field, but `BigQueryMetastoreCatalog` needs both `warehouse` and `projectId`. Should I:
   
   1. Keep `warehouse` explicit (consistent with `Hadoop`/`Hive`), move `projectId` to properties.
   2. Move both to properties (fully generic, but diverges from `Hadoop`/`Hive` pattern)
   3. Something else?
   
   Would appreciate your guidance.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "nandorKollar (via GitHub)" <gi...@apache.org>.
nandorKollar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3092960299


##########
polaris-core/src/main/java/org/apache/polaris/core/connection/bigquery/BigQueryMetastoreConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,123 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.core.connection.bigquery;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.MoreObjects;
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+import org.apache.polaris.core.credentials.connection.ConnectionCredentials;
+import org.apache.polaris.core.identity.dpo.ServiceIdentityInfoDpo;
+import org.apache.polaris.core.identity.provider.ServiceIdentityProvider;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo} defined in the API
+ * model.
+ */
+public class BigQueryMetastoreConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+  public static final String DEFAULT_URI = "https://bigquery.googleapis.com";

Review Comment:
   Oh okay, I missed that it is used there. Makes sense, thanks!



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on PR #4050:
URL: https://github.com/apache/polaris/pull/4050#issuecomment-4275792391

   > LGTM ๐Ÿ‘ Thanks for bearing with me ๐Ÿ™‚
   
   No, thank you for taking the time to thoroughly review my PR and giving me so much constructive feedback!


-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3015927535


##########
extensions/federation/bigquery/src/main/java/org/apache/polaris/extensions/federation/bigquery/BigQueryMetastoreFederatedCatalogFactory.java:
##########
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.extensions.federation.bigquery;
+
+import io.smallrye.common.annotation.Identifier;
+import jakarta.enterprise.context.ApplicationScoped;
+import java.util.Map;
+import org.apache.iceberg.catalog.Catalog;
+import org.apache.iceberg.gcp.bigquery.BigQueryMetastoreCatalog;
+import org.apache.iceberg.rest.RESTUtil;
+import org.apache.polaris.core.catalog.ExternalCatalogFactory;
+import org.apache.polaris.core.catalog.GenericTableCatalog;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.AuthenticationType;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.connection.bigquery.BigQueryMetastoreConnectionConfigInfoDpo;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+
+/**
+ * Factory class for creating a BigQuery Metastore catalog handle based on connection configuration.
+ */
+@ApplicationScoped
+@Identifier(ConnectionType.BIGQUERY_FACTORY_IDENTIFIER)
+public class BigQueryMetastoreFederatedCatalogFactory implements ExternalCatalogFactory {
+
+  @Override
+  public Catalog createCatalog(
+      ConnectionConfigInfoDpo connectionConfigInfoDpo,
+      PolarisCredentialManager polarisCredentialManager,
+      Map<String, String> catalogProperties) {
+
+    // Currently, Polaris supports BigQuery Metastore federation only via IMPLICIT authentication.
+    // Hence, prior to initializing the configuration, ensure that the catalog uses
+    // IMPLICIT authentication.
+    AuthenticationParametersDpo authenticationParametersDpo =
+        connectionConfigInfoDpo.getAuthenticationParameters();
+    if (authenticationParametersDpo.getAuthenticationTypeCode()

Review Comment:
   Done. Added null check here. If `authenticationParameters` is null, BigQuery uses ADC by default.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3017795685


##########
polaris-core/src/main/java/org/apache/polaris/core/connection/bigquery/BigQueryMetastoreConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,230 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.core.connection.bigquery;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.MoreObjects;
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+import org.apache.polaris.core.credentials.connection.ConnectionCredentials;
+import org.apache.polaris.core.identity.dpo.ServiceIdentityInfoDpo;
+import org.apache.polaris.core.identity.provider.ServiceIdentityProvider;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo} defined in the API
+ * model.
+ */
+public class BigQueryMetastoreConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {

Review Comment:
   Just wondering: Could this class be `@PolarisImmutable`? I guess it might save a good chunk of boiler-plate code ๐Ÿค” 



##########
extensions/federation/bigquery/src/main/java/org/apache/polaris/extensions/federation/bigquery/BigQueryMetastoreFederatedCatalogFactory.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.extensions.federation.bigquery;
+
+import io.smallrye.common.annotation.Identifier;
+import jakarta.enterprise.context.ApplicationScoped;
+import java.util.Map;
+import org.apache.iceberg.catalog.Catalog;
+import org.apache.iceberg.gcp.bigquery.BigQueryMetastoreCatalog;
+import org.apache.iceberg.rest.RESTUtil;
+import org.apache.polaris.core.catalog.ExternalCatalogFactory;
+import org.apache.polaris.core.catalog.GenericTableCatalog;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.AuthenticationType;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.connection.bigquery.BigQueryMetastoreConnectionConfigInfoDpo;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+
+/**
+ * Factory class for creating a BigQuery Metastore catalog handle based on connection configuration.
+ */
+@ApplicationScoped
+@Identifier(ConnectionType.BIGQUERY_FACTORY_IDENTIFIER)
+public class BigQueryMetastoreFederatedCatalogFactory implements ExternalCatalogFactory {
+
+  @Override
+  public Catalog createCatalog(
+      ConnectionConfigInfoDpo connectionConfigInfoDpo,
+      PolarisCredentialManager polarisCredentialManager,
+      Map<String, String> catalogProperties) {
+
+    // Currently, Polaris supports BigQuery Metastore federation only via IMPLICIT authentication.
+    // Hence, prior to initializing the configuration, ensure that the catalog uses
+    // IMPLICIT authentication.
+    AuthenticationParametersDpo authenticationParametersDpo =
+        connectionConfigInfoDpo.getAuthenticationParameters();
+    if (authenticationParametersDpo != null

Review Comment:
   What's the use case for permitting federated catalogs without an `AuthenticationParametersDpo` object?



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3019594370


##########
extensions/federation/bigquery/src/main/java/org/apache/polaris/extensions/federation/bigquery/BigQueryMetastoreFederatedCatalogFactory.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.extensions.federation.bigquery;
+
+import io.smallrye.common.annotation.Identifier;
+import jakarta.enterprise.context.ApplicationScoped;
+import java.util.Map;
+import org.apache.iceberg.catalog.Catalog;
+import org.apache.iceberg.gcp.bigquery.BigQueryMetastoreCatalog;
+import org.apache.iceberg.rest.RESTUtil;
+import org.apache.polaris.core.catalog.ExternalCatalogFactory;
+import org.apache.polaris.core.catalog.GenericTableCatalog;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.AuthenticationType;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.connection.bigquery.BigQueryMetastoreConnectionConfigInfoDpo;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+
+/**
+ * Factory class for creating a BigQuery Metastore catalog handle based on connection configuration.
+ */
+@ApplicationScoped
+@Identifier(ConnectionType.BIGQUERY_FACTORY_IDENTIFIER)
+public class BigQueryMetastoreFederatedCatalogFactory implements ExternalCatalogFactory {
+
+  @Override
+  public Catalog createCatalog(
+      ConnectionConfigInfoDpo connectionConfigInfoDpo,
+      PolarisCredentialManager polarisCredentialManager,
+      Map<String, String> catalogProperties) {
+
+    // Currently, Polaris supports BigQuery Metastore federation only via IMPLICIT authentication.
+    // Hence, prior to initializing the configuration, ensure that the catalog uses
+    // IMPLICIT authentication.
+    AuthenticationParametersDpo authenticationParametersDpo =
+        connectionConfigInfoDpo.getAuthenticationParameters();
+    if (authenticationParametersDpo != null

Review Comment:
   Same concern here. Should we fail fast when `authenticationParametersDpo` is null?



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3030637914


##########
extensions/federation/bigquery/src/main/java/org/apache/polaris/extensions/federation/bigquery/BigQueryMetastoreFederatedCatalogFactory.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.extensions.federation.bigquery;
+
+import io.smallrye.common.annotation.Identifier;
+import jakarta.enterprise.context.ApplicationScoped;
+import java.util.Map;
+import org.apache.iceberg.catalog.Catalog;
+import org.apache.iceberg.gcp.bigquery.BigQueryMetastoreCatalog;
+import org.apache.iceberg.rest.RESTUtil;
+import org.apache.polaris.core.catalog.ExternalCatalogFactory;
+import org.apache.polaris.core.catalog.GenericTableCatalog;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.AuthenticationType;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.connection.bigquery.BigQueryMetastoreConnectionConfigInfoDpo;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+
+/**
+ * Factory class for creating a BigQuery Metastore catalog handle based on connection configuration.
+ */
+@ApplicationScoped
+@Identifier(ConnectionType.BIGQUERY_FACTORY_IDENTIFIER)
+public class BigQueryMetastoreFederatedCatalogFactory implements ExternalCatalogFactory {
+
+  @Override
+  public Catalog createCatalog(
+      ConnectionConfigInfoDpo connectionConfigInfoDpo,
+      PolarisCredentialManager polarisCredentialManager,
+      Map<String, String> catalogProperties) {
+
+    // Currently, Polaris supports BigQuery Metastore federation only via IMPLICIT authentication.
+    // Hence, prior to initializing the configuration, ensure that the catalog uses
+    // IMPLICIT authentication.
+    AuthenticationParametersDpo authenticationParametersDpo =
+        connectionConfigInfoDpo.getAuthenticationParameters();
+    if (authenticationParametersDpo != null

Review Comment:
   I think we will need an `AuthenticationType` regardless of how BigQuery picks up credentials. In this case, it should be `IMPLICIT`(I agreed with line 58 here). However, from a syntax perspective, we may not want users to specify it explicitly. We could default it when the `AuthenticationType` is missing. That said, I would not recommend handling this defaulting logic here. The defaulting to `IMPLICIT` should happen elsewhere. WDYT?



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r2985519365


##########
spec/polaris-management-service.yml:
##########
@@ -933,6 +934,7 @@ components:
           ICEBERG_REST: "#/components/schemas/IcebergRestConnectionConfigInfo"
           HADOOP: "#/components/schemas/HadoopConnectionConfigInfo"
           HIVE: "#/components/schemas/HiveConnectionConfigInfo"
+          BIGQUERY: "#/components/schemas/BigQueryMetastoreConnectionConfigInfo"

Review Comment:
   I have an existing dev ML thread for this. Replied with the PR link: https://lists.apache.org/thread/n3hh5s1zxn6yn7cbowfgf8p029z6m7g1
   
   Would you like me to start a new discussion?



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r2992352934


##########
runtime/server/build.gradle.kts:
##########
@@ -43,6 +43,10 @@ dependencies {
     runtimeOnly(project(":polaris-extensions-federation-hive"))
   }
 
+  if ((project.findProperty("NonRESTCatalogs") as String?)?.contains("BIGQUERY") == true) {
+    runtimeOnly(project(":polaris-extensions-federation-bigquery"))

Review Comment:
   Done.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3033215658


##########
extensions/federation/bigquery/src/main/java/org/apache/polaris/extensions/federation/bigquery/BigQueryMetastoreFederatedCatalogFactory.java:
##########
@@ -0,0 +1,79 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.extensions.federation.bigquery;
+
+import io.smallrye.common.annotation.Identifier;
+import jakarta.enterprise.context.ApplicationScoped;
+import java.util.Map;
+import org.apache.iceberg.catalog.Catalog;
+import org.apache.iceberg.gcp.bigquery.BigQueryMetastoreCatalog;
+import org.apache.iceberg.rest.RESTUtil;
+import org.apache.polaris.core.catalog.ExternalCatalogFactory;
+import org.apache.polaris.core.catalog.GenericTableCatalog;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.AuthenticationType;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.connection.bigquery.BigQueryMetastoreConnectionConfigInfoDpo;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+
+/** Factory class for creating a BigQuery Metastore catalog handle based on connection configuration. */
+@ApplicationScoped
+@Identifier(ConnectionType.BIGQUERY_FACTORY_IDENTIFIER)
+public class BigQueryMetastoreFederatedCatalogFactory implements ExternalCatalogFactory {
+
+  @Override
+  public Catalog createCatalog(
+      ConnectionConfigInfoDpo connectionConfigInfoDpo,
+      PolarisCredentialManager polarisCredentialManager,
+      Map<String, String> catalogProperties) {
+
+    // Currently, Polaris supports BigQuery Metastore federation only via IMPLICIT authentication.
+    // Hence, prior to initializing the configuration, ensure that the catalog uses
+    // IMPLICIT authentication.
+    AuthenticationParametersDpo authenticationParametersDpo =
+        connectionConfigInfoDpo.getAuthenticationParameters();
+    if (authenticationParametersDpo.getAuthenticationTypeCode()
+        != AuthenticationType.IMPLICIT.getCode()) {
+      throw new IllegalStateException(
+          "BigQuery Metastore federation only supports IMPLICIT authentication.");
+    }
+
+    String warehouse =
+        ((BigQueryMetastoreConnectionConfigInfoDpo) connectionConfigInfoDpo).getWarehouse();
+    Map<String, String> mergedProperties =
+        RESTUtil.merge(
+            catalogProperties != null ? catalogProperties : Map.of(),
+            connectionConfigInfoDpo.asIcebergCatalogProperties(polarisCredentialManager));
+
+    BigQueryMetastoreCatalog bigQueryMetastoreCatalog = new BigQueryMetastoreCatalog();
+    bigQueryMetastoreCatalog.initialize(warehouse, mergedProperties);

Review Comment:
   Done. Added a comment explaining the ADC credential behavior in [this commit](https://github.com/apache/polaris/pull/4050/changes/cef1b0fd0976fc313118df5d96e1df87c37e89b3).



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3034300574


##########
extensions/federation/bigquery/src/main/java/org/apache/polaris/extensions/federation/bigquery/BigQueryMetastoreFederatedCatalogFactory.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.extensions.federation.bigquery;
+
+import io.smallrye.common.annotation.Identifier;
+import jakarta.enterprise.context.ApplicationScoped;
+import java.util.Map;
+import org.apache.iceberg.catalog.Catalog;
+import org.apache.iceberg.gcp.bigquery.BigQueryMetastoreCatalog;
+import org.apache.iceberg.rest.RESTUtil;
+import org.apache.polaris.core.catalog.ExternalCatalogFactory;
+import org.apache.polaris.core.catalog.GenericTableCatalog;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.AuthenticationType;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.connection.bigquery.BigQueryMetastoreConnectionConfigInfoDpo;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+
+/**
+ * Factory class for creating a BigQuery Metastore catalog handle based on connection configuration.
+ */
+@ApplicationScoped
+@Identifier(ConnectionType.BIGQUERY_FACTORY_IDENTIFIER)
+public class BigQueryMetastoreFederatedCatalogFactory implements ExternalCatalogFactory {
+
+  @Override
+  public Catalog createCatalog(
+      ConnectionConfigInfoDpo connectionConfigInfoDpo,
+      PolarisCredentialManager polarisCredentialManager,
+      Map<String, String> catalogProperties) {
+
+    // Currently, Polaris supports BigQuery Metastore federation only via IMPLICIT authentication.
+    // Hence, prior to initializing the configuration, ensure that the catalog uses
+    // IMPLICIT authentication.
+    AuthenticationParametersDpo authenticationParametersDpo =
+        connectionConfigInfoDpo.getAuthenticationParameters();
+    if (authenticationParametersDpo != null

Review Comment:
   Agreed, the point is even we want a default value for better UX. This is not the place to handle the defaulting logic.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on PR #4050:
URL: https://github.com/apache/polaris/pull/4050#issuecomment-4122887163

   > Thanks for your contribution, @joyhaldar ! The PR LGTM ๐Ÿ‘ Still, I believe we need to open a `dev` discussion for it before merging as it includes REST API changes.
   
   Thank you for your review @dimas-b. I have an existing dev ML thread for this. Replied with the PR link: https://lists.apache.org/thread/n3hh5s1zxn6yn7cbowfgf8p029z6m7g1
   
   Would you like me to start a new discussion?


-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on PR #4050:
URL: https://github.com/apache/polaris/pull/4050#issuecomment-4124166219

   
   CI failure is unrelated to this PR, MongoDB testcontainer timeout if I understand correctly


-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain merged PR #4050:
URL: https://github.com/apache/polaris/pull/4050


-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3089414051


##########
extensions/federation/bigquery/src/main/java/org/apache/polaris/extensions/federation/bigquery/BigQueryMetastoreFederatedCatalogFactory.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.extensions.federation.bigquery;
+
+import io.smallrye.common.annotation.Identifier;
+import jakarta.enterprise.context.ApplicationScoped;
+import java.util.Map;
+import org.apache.iceberg.catalog.Catalog;
+import org.apache.iceberg.gcp.bigquery.BigQueryMetastoreCatalog;
+import org.apache.iceberg.rest.RESTUtil;
+import org.apache.polaris.core.catalog.ExternalCatalogFactory;
+import org.apache.polaris.core.catalog.GenericTableCatalog;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.AuthenticationType;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.connection.bigquery.BigQueryMetastoreConnectionConfigInfoDpo;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+
+/**
+ * Factory class for creating a BigQuery Metastore catalog handle based on connection configuration.
+ */
+@ApplicationScoped
+@Identifier(ConnectionType.BIGQUERY_FACTORY_IDENTIFIER)
+public class BigQueryMetastoreFederatedCatalogFactory implements ExternalCatalogFactory {
+
+  @Override
+  public Catalog createCatalog(
+      ConnectionConfigInfoDpo connectionConfigInfoDpo,
+      PolarisCredentialManager polarisCredentialManager,
+      Map<String, String> catalogProperties) {
+
+    // Currently, Polaris supports BigQuery Metastore federation only via IMPLICIT authentication.
+    // Hence, prior to initializing the configuration, ensure that the catalog uses
+    // IMPLICIT authentication.
+    AuthenticationParametersDpo authenticationParametersDpo =
+        connectionConfigInfoDpo.getAuthenticationParameters();
+    if (authenticationParametersDpo != null

Review Comment:
   @joyhaldar, sorry to miss this one, I think we will actually need this to be resolved. I'd suggest to add it to the method `fromConnectionConfigInfoModelWithSecrets()`, something like this
   ```
     case BIGQUERY:
       BigQueryMetastoreConnectionConfigInfo bigqueryConfigModel =
           (BigQueryMetastoreConnectionConfigInfo) connectionConfigurationModel;
   
       // Default to IMPLICIT authentication if not provided
       authenticationParameters =
           bigqueryConfigModel.getAuthenticationParameters() != null
               ? AuthenticationParametersDpo.fromAuthenticationParametersModelWithSecrets(
                     bigqueryConfigModel.getAuthenticationParameters(), secretReferences)
               : new ImplicitAuthenticationParametersDpo();
   ```



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3025792008


##########
extensions/federation/bigquery/src/main/java/org/apache/polaris/extensions/federation/bigquery/BigQueryMetastoreFederatedCatalogFactory.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.extensions.federation.bigquery;
+
+import io.smallrye.common.annotation.Identifier;
+import jakarta.enterprise.context.ApplicationScoped;
+import java.util.Map;
+import org.apache.iceberg.catalog.Catalog;
+import org.apache.iceberg.gcp.bigquery.BigQueryMetastoreCatalog;
+import org.apache.iceberg.rest.RESTUtil;
+import org.apache.polaris.core.catalog.ExternalCatalogFactory;
+import org.apache.polaris.core.catalog.GenericTableCatalog;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.AuthenticationType;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.connection.bigquery.BigQueryMetastoreConnectionConfigInfoDpo;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+
+/**
+ * Factory class for creating a BigQuery Metastore catalog handle based on connection configuration.
+ */
+@ApplicationScoped
+@Identifier(ConnectionType.BIGQUERY_FACTORY_IDENTIFIER)
+public class BigQueryMetastoreFederatedCatalogFactory implements ExternalCatalogFactory {
+
+  @Override
+  public Catalog createCatalog(
+      ConnectionConfigInfoDpo connectionConfigInfoDpo,
+      PolarisCredentialManager polarisCredentialManager,
+      Map<String, String> catalogProperties) {
+
+    // Currently, Polaris supports BigQuery Metastore federation only via IMPLICIT authentication.
+    // Hence, prior to initializing the configuration, ensure that the catalog uses
+    // IMPLICIT authentication.
+    AuthenticationParametersDpo authenticationParametersDpo =
+        connectionConfigInfoDpo.getAuthenticationParameters();
+    if (authenticationParametersDpo != null

Review Comment:
   BigQuery always uses [Application Default Credentials](https://docs.cloud.google.com/docs/authentication/application-default-credentials) or the [GOOGLE_APPLICATION_CREDENTIALS](https://docs.cloud.google.com/docs/authentication/application-default-credentials#GAC) environment variable to automatically pick up the user's credentials or a JSON file pointing to their Service Account. Making `authenticationParameters` required would just force users to type IMPLICIT for no reason IMO, but please let me know what you think, or please let me know if you think I am incorrect.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3029516972


##########
spec/polaris-management-service.yml:
##########
@@ -965,6 +967,44 @@ components:
         warehouse:
           type: string
           description: The warehouse location for the hive catalog.
+    
+    BigQueryMetastoreConnectionConfigInfo:
+      type: object
+      description: Configuration necessary for connecting to a BigQuery Metastore Catalog
+      allOf:
+        - $ref: '#/components/schemas/ConnectionConfigInfo'
+      properties:
+        warehouse:
+          type: string
+          description: The GCS bucket path for the catalog (e.g., gs://bucket-name)
+        gcpProjectId:
+          type: string
+          description: The GCP project ID
+        gcpLocation:
+          type: string
+          description: The GCP location (default = us)
+        listAllTables:
+          type: boolean
+          description: Whether to list all tables (default = true)
+        impersonateServiceAccount:
+          type: string
+          description: Service account email to impersonate
+        impersonateLifetimeSeconds:
+          type: integer
+          description: Token lifetime in seconds for impersonation (default = 3600)
+        impersonateScopes:
+          type: array
+          items:
+            type: string
+          description: OAuth scopes for impersonation (default = cloud-platform)
+        impersonateDelegates:
+          type: array
+          items:
+            type: string
+          description: Delegation chain for impersonation

Review Comment:
   Done. Refactored to use a properties bag. Only `warehouse` and `gcpProjectId` remain as required fields, everything else goes into the map. Thank you for your help.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3087270417


##########
polaris-core/src/main/java/org/apache/polaris/core/connection/bigquery/BigQueryMetastoreConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.core.connection.bigquery;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.MoreObjects;
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+import org.apache.polaris.core.credentials.connection.ConnectionCredentials;
+import org.apache.polaris.core.identity.dpo.ServiceIdentityInfoDpo;
+import org.apache.polaris.core.identity.provider.ServiceIdentityProvider;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo} defined in the API
+ * model.
+ */
+public class BigQueryMetastoreConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+  public static final String DEFAULT_URI = "https://bigquery.googleapis.com";
+
+  // BigQuery Metastore catalog property keys
+  private static final String GCP_BIGQUERY_PROJECT_ID = "gcp.bigquery.project-id";
+
+  private final String warehouse;
+  private final String gcpProjectId;
+
+  public BigQueryMetastoreConnectionConfigInfoDpo(
+      @JsonProperty(value = "uri", required = true) @Nonnull String uri,
+      @JsonProperty(value = "authenticationParameters", required = false) @Nullable
+          AuthenticationParametersDpo authenticationParameters,
+      @JsonProperty(value = "warehouse", required = true) @Nonnull String warehouse,
+      @JsonProperty(value = "gcpProjectId", required = true) @Nonnull String gcpProjectId,
+      @JsonProperty(value = "properties", required = false) @Nullable
+          Map<String, String> properties,
+      @JsonProperty(value = "serviceIdentity", required = false) @Nullable
+          ServiceIdentityInfoDpo serviceIdentity) {
+    super(
+        ConnectionType.BIGQUERY.getCode(),
+        uri,
+        authenticationParameters,
+        serviceIdentity,
+        properties);
+    this.warehouse = warehouse;
+    this.gcpProjectId = gcpProjectId;
+  }
+
+  public String getWarehouse() {
+    return warehouse;
+  }
+
+  public String getGcpProjectId() {
+    return gcpProjectId;
+  }
+
+  @Override
+  public String toString() {
+    return MoreObjects.toStringHelper(this)
+        .add("connectionTypeCode", getConnectionTypeCode())
+        .add("uri", getUri())
+        .add("warehouse", warehouse)
+        .add("gcpProjectId", gcpProjectId)
+        .add(
+            "authenticationParameters",
+            getAuthenticationParameters() != null
+                ? getAuthenticationParameters().toString()
+                : "null")
+        .toString();
+  }
+
+  @Override
+  public @Nonnull Map<String, String> asIcebergCatalogProperties(
+      PolarisCredentialManager polarisCredentialManager) {
+    HashMap<String, String> properties = new HashMap<>();
+
+    // Required properties for BigQueryMetastoreCatalog
+    properties.put(GCP_BIGQUERY_PROJECT_ID, gcpProjectId);
+    properties.put(CatalogProperties.WAREHOUSE_LOCATION, warehouse);

Review Comment:
   From my POV this creates a skew in the Polaris API, which becomes dependent on particular Iceberg java classes.
   
   Conceptually, this class merely passes some user-defined properties to the Iceberg catalog impl. This is fine. However, do we have to elevate a sub-set of those properties to explicit Polaris OpenAPI properties?.. I'm not sure.
   
   I believe keeping the Polaris API generic and uniform for all federated catalog properties might be preferable.
   
   Implementations (both Iceberg java classes and Polaris classes) can and should perform config validation as appropriate, of course.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r2989721049


##########
runtime/server/build.gradle.kts:
##########
@@ -43,6 +43,10 @@ dependencies {
     runtimeOnly(project(":polaris-extensions-federation-hive"))
   }
 
+  if ((project.findProperty("NonRESTCatalogs") as String?)?.contains("BIGQUERY") == true) {
+    runtimeOnly(project(":polaris-extensions-federation-bigquery"))

Review Comment:
   Yes, please go with an unconditional dependency.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3015286140


##########
extensions/federation/bigquery/src/main/java/org/apache/polaris/extensions/federation/bigquery/BigQueryMetastoreFederatedCatalogFactory.java:
##########
@@ -0,0 +1,79 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.extensions.federation.bigquery;
+
+import io.smallrye.common.annotation.Identifier;
+import jakarta.enterprise.context.ApplicationScoped;
+import java.util.Map;
+import org.apache.iceberg.catalog.Catalog;
+import org.apache.iceberg.gcp.bigquery.BigQueryMetastoreCatalog;
+import org.apache.iceberg.rest.RESTUtil;
+import org.apache.polaris.core.catalog.ExternalCatalogFactory;
+import org.apache.polaris.core.catalog.GenericTableCatalog;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.AuthenticationType;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.connection.bigquery.BigQueryMetastoreConnectionConfigInfoDpo;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+
+/** Factory class for creating a BigQuery Metastore catalog handle based on connection configuration. */
+@ApplicationScoped
+@Identifier(ConnectionType.BIGQUERY_FACTORY_IDENTIFIER)
+public class BigQueryMetastoreFederatedCatalogFactory implements ExternalCatalogFactory {
+
+  @Override
+  public Catalog createCatalog(
+      ConnectionConfigInfoDpo connectionConfigInfoDpo,
+      PolarisCredentialManager polarisCredentialManager,
+      Map<String, String> catalogProperties) {
+
+    // Currently, Polaris supports BigQuery Metastore federation only via IMPLICIT authentication.
+    // Hence, prior to initializing the configuration, ensure that the catalog uses
+    // IMPLICIT authentication.
+    AuthenticationParametersDpo authenticationParametersDpo =
+        connectionConfigInfoDpo.getAuthenticationParameters();
+    if (authenticationParametersDpo.getAuthenticationTypeCode()
+        != AuthenticationType.IMPLICIT.getCode()) {
+      throw new IllegalStateException(
+          "BigQuery Metastore federation only supports IMPLICIT authentication.");
+    }
+
+    String warehouse =
+        ((BigQueryMetastoreConnectionConfigInfoDpo) connectionConfigInfoDpo).getWarehouse();
+    Map<String, String> mergedProperties =
+        RESTUtil.merge(
+            catalogProperties != null ? catalogProperties : Map.of(),
+            connectionConfigInfoDpo.asIcebergCatalogProperties(polarisCredentialManager));
+
+    BigQueryMetastoreCatalog bigQueryMetastoreCatalog = new BigQueryMetastoreCatalog();
+    bigQueryMetastoreCatalog.initialize(warehouse, mergedProperties);

Review Comment:
   Yes, that's my understanding also. Both BigQuery Metastore and GCS storage will use the same ADC credentials, or impersonated SA if configured. Thank you for clarifying the full picture. Sorry for the late reply.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on PR #4050:
URL: https://github.com/apache/polaris/pull/4050#issuecomment-4318006685

   > Thanks for continuous working on it, @joyhaldar! I think we are getting closer. Left some comments. Could you fix the CI failures as well?
   > 
   > A few followup items(not a blocker):
   > 
   > 1. Document this new feature
   > 2. CLI changes to support BQMS.
   
   Sorry for the delay, I will follow-up on the remaining work very soon.


-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3087678254


##########
polaris-core/src/main/java/org/apache/polaris/core/connection/bigquery/BigQueryMetastoreConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.core.connection.bigquery;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.MoreObjects;
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+import org.apache.polaris.core.credentials.connection.ConnectionCredentials;
+import org.apache.polaris.core.identity.dpo.ServiceIdentityInfoDpo;
+import org.apache.polaris.core.identity.provider.ServiceIdentityProvider;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo} defined in the API
+ * model.
+ */
+public class BigQueryMetastoreConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+  public static final String DEFAULT_URI = "https://bigquery.googleapis.com";
+
+  // BigQuery Metastore catalog property keys
+  private static final String GCP_BIGQUERY_PROJECT_ID = "gcp.bigquery.project-id";
+
+  private final String warehouse;
+  private final String gcpProjectId;
+
+  public BigQueryMetastoreConnectionConfigInfoDpo(
+      @JsonProperty(value = "uri", required = true) @Nonnull String uri,
+      @JsonProperty(value = "authenticationParameters", required = false) @Nullable
+          AuthenticationParametersDpo authenticationParameters,
+      @JsonProperty(value = "warehouse", required = true) @Nonnull String warehouse,
+      @JsonProperty(value = "gcpProjectId", required = true) @Nonnull String gcpProjectId,
+      @JsonProperty(value = "properties", required = false) @Nullable
+          Map<String, String> properties,
+      @JsonProperty(value = "serviceIdentity", required = false) @Nullable
+          ServiceIdentityInfoDpo serviceIdentity) {
+    super(
+        ConnectionType.BIGQUERY.getCode(),
+        uri,
+        authenticationParameters,
+        serviceIdentity,
+        properties);
+    this.warehouse = warehouse;
+    this.gcpProjectId = gcpProjectId;
+  }
+
+  public String getWarehouse() {
+    return warehouse;
+  }
+
+  public String getGcpProjectId() {
+    return gcpProjectId;
+  }
+
+  @Override
+  public String toString() {
+    return MoreObjects.toStringHelper(this)
+        .add("connectionTypeCode", getConnectionTypeCode())
+        .add("uri", getUri())
+        .add("warehouse", warehouse)
+        .add("gcpProjectId", gcpProjectId)
+        .add(
+            "authenticationParameters",
+            getAuthenticationParameters() != null
+                ? getAuthenticationParameters().toString()
+                : "null")
+        .toString();
+  }
+
+  @Override
+  public @Nonnull Map<String, String> asIcebergCatalogProperties(
+      PolarisCredentialManager polarisCredentialManager) {
+    HashMap<String, String> properties = new HashMap<>();
+
+    // Required properties for BigQueryMetastoreCatalog
+    properties.put(GCP_BIGQUERY_PROJECT_ID, gcpProjectId);
+    properties.put(CatalogProperties.WAREHOUSE_LOCATION, warehouse);

Review Comment:
   Re: Hadoop and Hive: from my POV the same argument applies to them, but I missed out on the review of that code when I was introduced :sweat_smile: 



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3084642075


##########
polaris-core/src/main/java/org/apache/polaris/core/connection/bigquery/BigQueryMetastoreConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.core.connection.bigquery;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.MoreObjects;
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+import org.apache.polaris.core.credentials.connection.ConnectionCredentials;
+import org.apache.polaris.core.identity.dpo.ServiceIdentityInfoDpo;
+import org.apache.polaris.core.identity.provider.ServiceIdentityProvider;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo} defined in the API
+ * model.
+ */
+public class BigQueryMetastoreConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+  public static final String DEFAULT_URI = "https://bigquery.googleapis.com";
+
+  // BigQuery Metastore catalog property keys
+  private static final String GCP_BIGQUERY_PROJECT_ID = "gcp.bigquery.project-id";
+
+  private final String warehouse;
+  private final String gcpProjectId;
+
+  public BigQueryMetastoreConnectionConfigInfoDpo(
+      @JsonProperty(value = "uri", required = true) @Nonnull String uri,
+      @JsonProperty(value = "authenticationParameters", required = false) @Nullable
+          AuthenticationParametersDpo authenticationParameters,
+      @JsonProperty(value = "warehouse", required = true) @Nonnull String warehouse,
+      @JsonProperty(value = "gcpProjectId", required = true) @Nonnull String gcpProjectId,
+      @JsonProperty(value = "properties", required = false) @Nullable
+          Map<String, String> properties,
+      @JsonProperty(value = "serviceIdentity", required = false) @Nullable
+          ServiceIdentityInfoDpo serviceIdentity) {
+    super(
+        ConnectionType.BIGQUERY.getCode(),
+        uri,
+        authenticationParameters,
+        serviceIdentity,
+        properties);
+    this.warehouse = warehouse;
+    this.gcpProjectId = gcpProjectId;
+  }
+
+  public String getWarehouse() {
+    return warehouse;
+  }
+
+  public String getGcpProjectId() {
+    return gcpProjectId;
+  }
+
+  @Override
+  public String toString() {
+    return MoreObjects.toStringHelper(this)
+        .add("connectionTypeCode", getConnectionTypeCode())
+        .add("uri", getUri())
+        .add("warehouse", warehouse)
+        .add("gcpProjectId", gcpProjectId)
+        .add(
+            "authenticationParameters",
+            getAuthenticationParameters() != null
+                ? getAuthenticationParameters().toString()
+                : "null")
+        .toString();
+  }
+
+  @Override
+  public @Nonnull Map<String, String> asIcebergCatalogProperties(
+      PolarisCredentialManager polarisCredentialManager) {
+    HashMap<String, String> properties = new HashMap<>();
+
+    // Required properties for BigQueryMetastoreCatalog
+    properties.put(GCP_BIGQUERY_PROJECT_ID, gcpProjectId);
+    properties.put(CatalogProperties.WAREHOUSE_LOCATION, warehouse);

Review Comment:
   These are required fields in the spec. `projectId` is validated as required in [BigQueryProperties.java](https://github.com/apache/iceberg/blob/9a939d68358de9dac2c6ba9b236b675ebe477490/bigquery/src/main/java/org/apache/iceberg/gcp/bigquery/BigQueryProperties.java#L104-L106), and `warehouse` is validated in [BigQueryMetastoreCatalog.java](https://github.com/apache/iceberg/blob/9a939d68358de9dac2c6ba9b236b675ebe477490/bigquery/src/main/java/org/apache/iceberg/gcp/bigquery/BigQueryMetastoreCatalog.java#L310-L314). The properties bag is for optional settings like `gcp.bigquery.location`. But let me know if this is not appropriate, would love your opinion.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3087623134


##########
polaris-core/src/main/java/org/apache/polaris/core/connection/bigquery/BigQueryMetastoreConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.core.connection.bigquery;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.MoreObjects;
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+import org.apache.polaris.core.credentials.connection.ConnectionCredentials;
+import org.apache.polaris.core.identity.dpo.ServiceIdentityInfoDpo;
+import org.apache.polaris.core.identity.provider.ServiceIdentityProvider;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo} defined in the API
+ * model.
+ */
+public class BigQueryMetastoreConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+  public static final String DEFAULT_URI = "https://bigquery.googleapis.com";
+
+  // BigQuery Metastore catalog property keys
+  private static final String GCP_BIGQUERY_PROJECT_ID = "gcp.bigquery.project-id";
+
+  private final String warehouse;
+  private final String gcpProjectId;
+
+  public BigQueryMetastoreConnectionConfigInfoDpo(
+      @JsonProperty(value = "uri", required = true) @Nonnull String uri,
+      @JsonProperty(value = "authenticationParameters", required = false) @Nullable
+          AuthenticationParametersDpo authenticationParameters,
+      @JsonProperty(value = "warehouse", required = true) @Nonnull String warehouse,
+      @JsonProperty(value = "gcpProjectId", required = true) @Nonnull String gcpProjectId,
+      @JsonProperty(value = "properties", required = false) @Nullable
+          Map<String, String> properties,
+      @JsonProperty(value = "serviceIdentity", required = false) @Nullable
+          ServiceIdentityInfoDpo serviceIdentity) {
+    super(
+        ConnectionType.BIGQUERY.getCode(),
+        uri,
+        authenticationParameters,
+        serviceIdentity,
+        properties);
+    this.warehouse = warehouse;
+    this.gcpProjectId = gcpProjectId;
+  }
+
+  public String getWarehouse() {
+    return warehouse;
+  }
+
+  public String getGcpProjectId() {
+    return gcpProjectId;
+  }
+
+  @Override
+  public String toString() {
+    return MoreObjects.toStringHelper(this)
+        .add("connectionTypeCode", getConnectionTypeCode())
+        .add("uri", getUri())
+        .add("warehouse", warehouse)
+        .add("gcpProjectId", gcpProjectId)
+        .add(
+            "authenticationParameters",
+            getAuthenticationParameters() != null
+                ? getAuthenticationParameters().toString()
+                : "null")
+        .toString();
+  }
+
+  @Override
+  public @Nonnull Map<String, String> asIcebergCatalogProperties(
+      PolarisCredentialManager polarisCredentialManager) {
+    HashMap<String, String> properties = new HashMap<>();
+
+    // Required properties for BigQueryMetastoreCatalog
+    properties.put(GCP_BIGQUERY_PROJECT_ID, gcpProjectId);
+    properties.put(CatalogProperties.WAREHOUSE_LOCATION, warehouse);

Review Comment:
   Retracting API changes is not ideal (in a follow-up). 
   
   I'd propose to remove `gcpProjectId` and `warehouse` from OpenAPI _explicit_ properties and process corresponding Iceberg properties from the generic `properties` bag. Also, add description to OpenAPI YAML to list required properties.
   
   I wonder what other reviewers think about this.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r2995732243


##########
extensions/federation/bigquery/src/main/java/org/apache/polaris/extensions/federation/bigquery/BigQueryMetastoreFederatedCatalogFactory.java:
##########
@@ -0,0 +1,79 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.extensions.federation.bigquery;
+
+import io.smallrye.common.annotation.Identifier;
+import jakarta.enterprise.context.ApplicationScoped;
+import java.util.Map;
+import org.apache.iceberg.catalog.Catalog;
+import org.apache.iceberg.gcp.bigquery.BigQueryMetastoreCatalog;
+import org.apache.iceberg.rest.RESTUtil;
+import org.apache.polaris.core.catalog.ExternalCatalogFactory;
+import org.apache.polaris.core.catalog.GenericTableCatalog;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.AuthenticationType;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.connection.bigquery.BigQueryMetastoreConnectionConfigInfoDpo;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+
+/** Factory class for creating a BigQuery Metastore catalog handle based on connection configuration. */
+@ApplicationScoped
+@Identifier(ConnectionType.BIGQUERY_FACTORY_IDENTIFIER)
+public class BigQueryMetastoreFederatedCatalogFactory implements ExternalCatalogFactory {
+
+  @Override
+  public Catalog createCatalog(
+      ConnectionConfigInfoDpo connectionConfigInfoDpo,
+      PolarisCredentialManager polarisCredentialManager,
+      Map<String, String> catalogProperties) {
+
+    // Currently, Polaris supports BigQuery Metastore federation only via IMPLICIT authentication.
+    // Hence, prior to initializing the configuration, ensure that the catalog uses
+    // IMPLICIT authentication.
+    AuthenticationParametersDpo authenticationParametersDpo =
+        connectionConfigInfoDpo.getAuthenticationParameters();
+    if (authenticationParametersDpo.getAuthenticationTypeCode()
+        != AuthenticationType.IMPLICIT.getCode()) {
+      throw new IllegalStateException(
+          "BigQuery Metastore federation only supports IMPLICIT authentication.");
+    }
+
+    String warehouse =
+        ((BigQueryMetastoreConnectionConfigInfoDpo) connectionConfigInfoDpo).getWarehouse();
+    Map<String, String> mergedProperties =
+        RESTUtil.merge(
+            catalogProperties != null ? catalogProperties : Map.of(),
+            connectionConfigInfoDpo.asIcebergCatalogProperties(polarisCredentialManager));
+
+    BigQueryMetastoreCatalog bigQueryMetastoreCatalog = new BigQueryMetastoreCatalog();
+    bigQueryMetastoreCatalog.initialize(warehouse, mergedProperties);

Review Comment:
   So, if storage is also in GCP, the BigQuary connection and storage connections will have to use the same credentials, effectively (in current Polaris code).
   
   It's not a big deal ATM, I think... just trying to understand the full picture :)



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r2992573311


##########
extensions/federation/bigquery/src/main/java/org/apache/polaris/extensions/federation/bigquery/BigQueryMetastoreFederatedCatalogFactory.java:
##########
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.extensions.federation.bigquery;
+
+import io.smallrye.common.annotation.Identifier;
+import jakarta.enterprise.context.ApplicationScoped;
+import java.util.Map;
+import org.apache.iceberg.catalog.Catalog;
+import org.apache.iceberg.gcp.bigquery.BigQueryMetastoreCatalog;
+import org.apache.iceberg.rest.RESTUtil;
+import org.apache.polaris.core.catalog.ExternalCatalogFactory;
+import org.apache.polaris.core.catalog.GenericTableCatalog;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.AuthenticationType;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.connection.bigquery.BigQueryMetastoreConnectionConfigInfoDpo;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+
+/**
+ * Factory class for creating a BigQuery Metastore catalog handle based on connection configuration.
+ */
+@ApplicationScoped
+@Identifier(ConnectionType.BIGQUERY_FACTORY_IDENTIFIER)
+public class BigQueryMetastoreFederatedCatalogFactory implements ExternalCatalogFactory {
+
+  @Override
+  public Catalog createCatalog(
+      ConnectionConfigInfoDpo connectionConfigInfoDpo,
+      PolarisCredentialManager polarisCredentialManager,
+      Map<String, String> catalogProperties) {
+
+    // Currently, Polaris supports BigQuery Metastore federation only via IMPLICIT authentication.
+    // Hence, prior to initializing the configuration, ensure that the catalog uses
+    // IMPLICIT authentication.
+    AuthenticationParametersDpo authenticationParametersDpo =
+        connectionConfigInfoDpo.getAuthenticationParameters();
+    if (authenticationParametersDpo.getAuthenticationTypeCode()

Review Comment:
   Thank you for your review JB.
   
   I copied this pattern from [HiveFederatedCatalogFactory](https://github.com/apache/polaris/blob/49766cca72e6a21ce21cc002a571d485a4586f44/extensions/federation/hive/src/main/java/org/apache/polaris/extensions/federation/hive/HiveFederatedCatalogFactory.java#L49) and [HadoopFederatedCatalogFactory](https://github.com/apache/polaris/blob/49766cca72e6a21ce21cc002a571d485a4586f44/extensions/federation/hadoop/src/main/java/org/apache/polaris/extensions/federation/hadoop/HadoopFederatedCatalogFactory.java#L50) which also don't have null checks. Should I add the null check here and open a separate PR to fix Hive and Hadoop as well? Or keep it consistent with existing code for now?



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r2985519365


##########
spec/polaris-management-service.yml:
##########
@@ -933,6 +934,7 @@ components:
           ICEBERG_REST: "#/components/schemas/IcebergRestConnectionConfigInfo"
           HADOOP: "#/components/schemas/HadoopConnectionConfigInfo"
           HIVE: "#/components/schemas/HiveConnectionConfigInfo"
+          BIGQUERY: "#/components/schemas/BigQueryMetastoreConnectionConfigInfo"

Review Comment:
   Thank you for your review Dmitri.
   
   I have an existing dev ML thread for this. Replied with the PR link: https://lists.apache.org/thread/n3hh5s1zxn6yn7cbowfgf8p029z6m7g1
   
   Would you like me to start a new discussion?



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3092806711


##########
polaris-core/src/main/java/org/apache/polaris/core/connection/hadoop/HadoopConnectionConfigInfoDpo.java:
##########
@@ -51,7 +51,8 @@ public HadoopConnectionConfigInfoDpo(
       @JsonProperty(value = "serviceIdentity", required = false) @Nullable
           ServiceIdentityInfoDpo serviceIdentityInfo,
       @JsonProperty(value = "warehouse", required = false) @Nullable String remoteCatalogName) {
-    super(ConnectionType.HADOOP.getCode(), uri, authenticationParameters, serviceIdentityInfo);
+    super(
+        ConnectionType.HADOOP.getCode(), uri, authenticationParameters, serviceIdentityInfo, null);

Review Comment:
   I have added the overloaded constructor and updated the subclasses.
   
   



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3103937235


##########
extensions/federation/bigquery/src/main/java/org/apache/polaris/extensions/federation/bigquery/BigQueryMetastoreFederatedCatalogFactory.java:
##########
@@ -0,0 +1,90 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.extensions.federation.bigquery;
+
+import io.smallrye.common.annotation.Identifier;
+import jakarta.enterprise.context.ApplicationScoped;
+import java.util.Map;
+import org.apache.iceberg.catalog.Catalog;
+import org.apache.iceberg.gcp.bigquery.BigQueryMetastoreCatalog;
+import org.apache.iceberg.rest.RESTUtil;
+import org.apache.polaris.core.catalog.FederatedCatalogFactory;
+import org.apache.polaris.core.catalog.GenericTableCatalog;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.AuthenticationType;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+
+/**
+ * Factory class for creating a BigQuery Metastore catalog handle based on connection configuration.
+ */
+@ApplicationScoped
+@Identifier(ConnectionType.BIGQUERY_FACTORY_IDENTIFIER)
+public class BigQueryMetastoreFederatedCatalogFactory implements FederatedCatalogFactory {
+
+  @Override
+  public Catalog createCatalog(
+      ConnectionConfigInfoDpo connectionConfigInfoDpo,
+      PolarisCredentialManager polarisCredentialManager,
+      Map<String, String> catalogProperties) {
+
+    // Currently, Polaris supports BigQuery Metastore federation only via IMPLICIT authentication.
+    // Hence, prior to initializing the configuration, ensure that the catalog uses
+    // IMPLICIT authentication.
+    AuthenticationParametersDpo authenticationParametersDpo =
+        connectionConfigInfoDpo.getAuthenticationParameters();
+    if (authenticationParametersDpo.getAuthenticationTypeCode()
+        != AuthenticationType.IMPLICIT.getCode()) {
+      throw new IllegalStateException(
+          "BigQuery Metastore federation only supports IMPLICIT authentication.");
+    }
+
+    Map<String, String> properties = connectionConfigInfoDpo.getProperties();
+    String warehouse = properties.get("warehouse");
+    if (warehouse == null || warehouse.isEmpty()) {
+      throw new IllegalArgumentException("warehouse is required for BigQuery Metastore federation");
+    }
+    String projectId = properties.get("gcp.bigquery.project-id");

Review Comment:
   nit: make a constant of refer to a constant from Iceberg jars?



##########
spec/polaris-management-service.yml:
##########
@@ -965,6 +972,16 @@ components:
         warehouse:
           type: string
           description: The warehouse location for the hive catalog.
+    
+    BigQueryMetastoreConnectionConfigInfo:
+      type: object
+      description: |
+        Configuration necessary for connecting to a BigQuery Metastore Catalog.

Review Comment:
   nit: maybe make a note that `properties` are effectively Iceberg's BigQuery catalog properties (for clarity)?



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3025855618


##########
polaris-core/src/main/java/org/apache/polaris/core/connection/bigquery/BigQueryMetastoreConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,230 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.core.connection.bigquery;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.MoreObjects;
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+import org.apache.polaris.core.credentials.connection.ConnectionCredentials;
+import org.apache.polaris.core.identity.dpo.ServiceIdentityInfoDpo;
+import org.apache.polaris.core.identity.provider.ServiceIdentityProvider;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo} defined in the API
+ * model.
+ */
+public class BigQueryMetastoreConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {

Review Comment:
   The existing `ConnectionConfigInfoDpo` classes (Hadoop, Hive) don't use `@PolarisImmutable`. I can try to refactor and use it if you think it's worthwhile. Would appreciate your inputs before I proceed.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "XJDKC (via GitHub)" <gi...@apache.org>.
XJDKC commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3019866472


##########
polaris-core/src/main/java/org/apache/polaris/core/connection/bigquery/BigQueryMetastoreConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,230 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.core.connection.bigquery;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.MoreObjects;
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+import org.apache.polaris.core.credentials.connection.ConnectionCredentials;
+import org.apache.polaris.core.identity.dpo.ServiceIdentityInfoDpo;
+import org.apache.polaris.core.identity.provider.ServiceIdentityProvider;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo} defined in the API
+ * model.
+ */
+public class BigQueryMetastoreConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+  public static final String DEFAULT_URI = "https://bigquery.googleapis.com";
+
+  // BigQuery Metastore catalog property keys
+  private static final String GCP_BIGQUERY_PROJECT_ID = "gcp.bigquery.project-id";
+  private static final String GCP_BIGQUERY_LOCATION = "gcp.bigquery.location";
+  private static final String GCP_BIGQUERY_LIST_ALL_TABLES = "gcp.bigquery.list-all-tables";
+  private static final String IMPERSONATE_SERVICE_ACCOUNT =
+      "gcp.bigquery.impersonate.service-account";
+  private static final String IMPERSONATE_LIFETIME_SECONDS =
+      "gcp.bigquery.impersonate.lifetime-seconds";
+  private static final String IMPERSONATE_SCOPES = "gcp.bigquery.impersonate.scopes";
+  private static final String IMPERSONATE_DELEGATES = "gcp.bigquery.impersonate.delegates";

Review Comment:
   For these property keys, should we just use the iceberg sdk's properties?
   
   https://github.com/apache/iceberg/blob/main/bigquery/src/main/java/org/apache/iceberg/gcp/bigquery/BigQueryProperties.java



##########
polaris-core/src/main/java/org/apache/polaris/core/connection/bigquery/BigQueryMetastoreConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,230 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.core.connection.bigquery;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.MoreObjects;
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+import org.apache.polaris.core.credentials.connection.ConnectionCredentials;
+import org.apache.polaris.core.identity.dpo.ServiceIdentityInfoDpo;
+import org.apache.polaris.core.identity.provider.ServiceIdentityProvider;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo} defined in the API
+ * model.
+ */
+public class BigQueryMetastoreConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+  public static final String DEFAULT_URI = "https://bigquery.googleapis.com";
+
+  // BigQuery Metastore catalog property keys
+  private static final String GCP_BIGQUERY_PROJECT_ID = "gcp.bigquery.project-id";
+  private static final String GCP_BIGQUERY_LOCATION = "gcp.bigquery.location";
+  private static final String GCP_BIGQUERY_LIST_ALL_TABLES = "gcp.bigquery.list-all-tables";
+  private static final String IMPERSONATE_SERVICE_ACCOUNT =
+      "gcp.bigquery.impersonate.service-account";
+  private static final String IMPERSONATE_LIFETIME_SECONDS =
+      "gcp.bigquery.impersonate.lifetime-seconds";
+  private static final String IMPERSONATE_SCOPES = "gcp.bigquery.impersonate.scopes";
+  private static final String IMPERSONATE_DELEGATES = "gcp.bigquery.impersonate.delegates";
+
+  private final String warehouse;
+  private final String gcpProjectId;
+  private final String gcpLocation;
+  private final Boolean listAllTables;
+  private final String impersonateServiceAccount;
+  private final Integer impersonateLifetimeSeconds;
+  private final List<String> impersonateScopes;
+  private final List<String> impersonateDelegates;

Review Comment:
   These are authentication related properties, wandering if we need to create a new authentication for GCP?
   ```
     private final String impersonateServiceAccount;
     private final Integer impersonateLifetimeSeconds;
     private final List<String> impersonateScopes;
     private final List<String> impersonateDelegates;
   ```



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r2995757639


##########
spec/polaris-management-service.yml:
##########
@@ -933,6 +934,7 @@ components:
           ICEBERG_REST: "#/components/schemas/IcebergRestConnectionConfigInfo"
           HADOOP: "#/components/schemas/HadoopConnectionConfigInfo"
           HIVE: "#/components/schemas/HiveConnectionConfigInfo"
+          BIGQUERY: "#/components/schemas/BigQueryMetastoreConnectionConfigInfo"

Review Comment:
   I opened https://lists.apache.org/thread/m05xm7szd7znrm9yos1rnld5ljx04y0h specifically for community awareness of REST API changes.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3034162329


##########
runtime/server/build.gradle.kts:
##########
@@ -43,6 +43,10 @@ dependencies {
     runtimeOnly(project(":polaris-extensions-federation-hive"))
   }
 
+  if ((project.findProperty("NonRESTCatalogs") as String?)?.contains("BIGQUERY") == true) {
+    runtimeOnly(project(":polaris-extensions-federation-bigquery"))

Review Comment:
   Sounds good. Resolving this comment thread.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r2995862060


##########
polaris-core/src/main/java/org/apache/polaris/core/connection/bigquery/BigQueryMetastoreConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,228 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.core.connection.bigquery;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.MoreObjects;
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+import org.apache.polaris.core.credentials.connection.ConnectionCredentials;
+import org.apache.polaris.core.identity.dpo.ServiceIdentityInfoDpo;
+import org.apache.polaris.core.identity.provider.ServiceIdentityProvider;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo} defined in the API
+ * model.
+ */
+public class BigQueryMetastoreConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+  public static final String DEFAULT_URI = "https://bigquery.googleapis.com";
+
+  // BigQuery Metastore catalog property keys
+  private static final String GCP_BIGQUERY_PROJECT_ID = "gcp.bigquery.project-id";
+  private static final String GCP_BIGQUERY_LOCATION = "gcp.bigquery.location";
+  private static final String GCP_BIGQUERY_LIST_ALL_TABLES = "gcp.bigquery.list-all-tables";
+  private static final String IMPERSONATE_SERVICE_ACCOUNT =
+      "gcp.bigquery.impersonate.service-account";
+  private static final String IMPERSONATE_LIFETIME_SECONDS =
+      "gcp.bigquery.impersonate.lifetime-seconds";
+  private static final String IMPERSONATE_SCOPES = "gcp.bigquery.impersonate.scopes";
+  private static final String IMPERSONATE_DELEGATES = "gcp.bigquery.impersonate.delegates";
+
+  private final String warehouse;
+  private final String gcpProjectId;
+  private final String gcpLocation;
+  private final Boolean listAllTables;
+  private final String impersonateServiceAccount;
+  private final Integer impersonateLifetimeSeconds;
+  private final List<String> impersonateScopes;
+  private final List<String> impersonateDelegates;
+
+  public BigQueryMetastoreConnectionConfigInfoDpo(
+      @JsonProperty(value = "uri", required = true) @Nonnull String uri,
+      @JsonProperty(value = "authenticationParameters", required = false) @Nullable
+          AuthenticationParametersDpo authenticationParameters,
+      @JsonProperty(value = "warehouse", required = true) @Nonnull String warehouse,
+      @JsonProperty(value = "gcpProjectId", required = true) @Nonnull String gcpProjectId,
+      @JsonProperty(value = "gcpLocation", required = false) @Nullable String gcpLocation,
+      @JsonProperty(value = "listAllTables", required = false) @Nullable Boolean listAllTables,
+      @JsonProperty(value = "impersonateServiceAccount", required = false) @Nullable
+          String impersonateServiceAccount,
+      @JsonProperty(value = "impersonateLifetimeSeconds", required = false) @Nullable
+          Integer impersonateLifetimeSeconds,
+      @JsonProperty(value = "impersonateScopes", required = false) @Nullable
+          List<String> impersonateScopes,
+      @JsonProperty(value = "impersonateDelegates", required = false) @Nullable
+          List<String> impersonateDelegates,
+      @JsonProperty(value = "serviceIdentity", required = false) @Nullable
+          ServiceIdentityInfoDpo serviceIdentity) {
+    super(ConnectionType.BIGQUERY.getCode(), uri, authenticationParameters, serviceIdentity);
+    this.warehouse = warehouse;
+    this.gcpProjectId = gcpProjectId;
+    this.gcpLocation = gcpLocation;
+    this.listAllTables = listAllTables;
+    this.impersonateServiceAccount = impersonateServiceAccount;
+    this.impersonateLifetimeSeconds = impersonateLifetimeSeconds;
+    this.impersonateScopes = impersonateScopes;
+    this.impersonateDelegates = impersonateDelegates;
+  }
+
+  public String getWarehouse() {
+    return warehouse;
+  }
+
+  public String getGcpProjectId() {
+    return gcpProjectId;
+  }
+
+  public String getGcpLocation() {
+    return gcpLocation;
+  }
+
+  public Boolean getListAllTables() {
+    return listAllTables;
+  }
+
+  public String getImpersonateServiceAccount() {
+    return impersonateServiceAccount;
+  }
+
+  public Integer getImpersonateLifetimeSeconds() {
+    return impersonateLifetimeSeconds;
+  }
+
+  public List<String> getImpersonateScopes() {
+    return impersonateScopes;
+  }
+
+  public List<String> getImpersonateDelegates() {
+    return impersonateDelegates;
+  }
+
+  @Override
+  public String toString() {
+    return MoreObjects.toStringHelper(this)
+        .add("connectionTypeCode", getConnectionTypeCode())
+        .add("uri", getUri())
+        .add("warehouse", warehouse)
+        .add("gcpProjectId", gcpProjectId)
+        .add("gcpLocation", gcpLocation)
+        .add("listAllTables", listAllTables)
+        .add("authenticationParameters", getAuthenticationParameters().toString())
+        .toString();
+  }
+
+  @Override
+  public @Nonnull Map<String, String> asIcebergCatalogProperties(
+      PolarisCredentialManager polarisCredentialManager) {
+    HashMap<String, String> properties = new HashMap<>();
+
+    // Required properties for BigQueryMetastoreCatalog
+    properties.put(GCP_BIGQUERY_PROJECT_ID, gcpProjectId);
+    properties.put(CatalogProperties.WAREHOUSE_LOCATION, warehouse);
+
+    // Optional properties
+    if (gcpLocation != null) {
+      properties.put(GCP_BIGQUERY_LOCATION, gcpLocation);
+    }
+    if (listAllTables != null) {
+      properties.put(GCP_BIGQUERY_LIST_ALL_TABLES, String.valueOf(listAllTables));
+    }
+
+    // Optional impersonation properties
+    if (impersonateServiceAccount != null) {
+      properties.put(IMPERSONATE_SERVICE_ACCOUNT, impersonateServiceAccount);
+    }
+    if (impersonateLifetimeSeconds != null) {
+      properties.put(IMPERSONATE_LIFETIME_SECONDS, String.valueOf(impersonateLifetimeSeconds));
+    }
+    if (impersonateScopes != null && !impersonateScopes.isEmpty()) {
+      properties.put(IMPERSONATE_SCOPES, String.join(",", impersonateScopes));
+    }
+    if (impersonateDelegates != null && !impersonateDelegates.isEmpty()) {
+      properties.put(IMPERSONATE_DELEGATES, String.join(",", impersonateDelegates));
+    }
+
+    if (getAuthenticationParameters() != null) {
+      // Add authentication-specific metadata (non-credential properties)
+      properties.putAll(
+          getAuthenticationParameters().asIcebergCatalogProperties(polarisCredentialManager));
+      // Add connection credentials from Polaris credential manager
+      ConnectionCredentials connectionCredentials =
+          polarisCredentialManager.getConnectionCredentials(this);
+      properties.putAll(connectionCredentials.credentials());
+    }
+    return properties;
+  }
+
+  @Override
+  public ConnectionConfigInfoDpo withServiceIdentity(
+      @Nonnull ServiceIdentityInfoDpo serviceIdentityInfo) {
+    return new BigQueryMetastoreConnectionConfigInfoDpo(
+        getUri(),
+        getAuthenticationParameters(),
+        warehouse,
+        gcpProjectId,
+        gcpLocation,
+        listAllTables,
+        impersonateServiceAccount,
+        impersonateLifetimeSeconds,
+        impersonateScopes,
+        impersonateDelegates,
+        serviceIdentityInfo);
+  }
+
+  @Override
+  public ConnectionConfigInfo asConnectionConfigInfoModel(

Review Comment:
   Do we have a test to validate no data is lost on round-trips through these different config objects?



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3078821789


##########
extensions/federation/bigquery/src/main/java/org/apache/polaris/extensions/federation/bigquery/BigQueryMetastoreFederatedCatalogFactory.java:
##########
@@ -65,6 +65,8 @@ public Catalog createCatalog(
             catalogProperties != null ? catalogProperties : Map.of(),
             connectionConfigInfoDpo.asIcebergCatalogProperties(polarisCredentialManager));
 
+    // Credentials are resolved via Google Application Default Credentials (ADC).
+    // GCS storage operations use the same ADC credentials.

Review Comment:
   Updated the comment to say `by default`.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on PR #4050:
URL: https://github.com/apache/polaris/pull/4050#issuecomment-4161986291

   > Iโ€™m aligned with the direction here overall. I think there are two high-priority issues worth tightening before this moves forward:
   > 
   > 1. The current BIGQUERY auth contract does not look aligned yet. IIUC, the API/model path still allows `authenticationParameters` to be omitted, while the runtime only supports `IMPLICIT` auth and later dereferences that field as if it were always present. That feels like more than a local null-safety issue, the boundary contract still looks looser than the implementation.
   > 2. Since this adds a new federation connector across OpenAPI, model conversion, and runtime wiring, I think we should have at least one round-trip/ wiring test for `BIGQUERY` before the contract settles.
   > 
   > 2 cents: this PR also seems like a useful point to clarify whether federation here is aiming for a common Polaris contract over native Iceberg catalogs, or for backend-specific adapters behind one REST surface. I think that answer affects how strict/explicit the connector contract should be.
   
   Thank you for the detailed review. I have tried to address both points:
   1. Added null checks in the factory and `asConnectionConfigInfoModel` to handle omitted `authenticationParameters`, which defaults to ADC.
   2. Added `testBigQueryMetastoreConnectionConfig` in `ConnectionConfigInfoDpoTest` for round-trip validation.
   
   Please let me know if you'd like any changes.


-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3014891716


##########
polaris-core/src/main/java/org/apache/polaris/core/connection/bigquery/BigQueryMetastoreConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,228 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.core.connection.bigquery;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.MoreObjects;
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+import org.apache.polaris.core.credentials.connection.ConnectionCredentials;
+import org.apache.polaris.core.identity.dpo.ServiceIdentityInfoDpo;
+import org.apache.polaris.core.identity.provider.ServiceIdentityProvider;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo} defined in the API
+ * model.
+ */
+public class BigQueryMetastoreConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+  public static final String DEFAULT_URI = "https://bigquery.googleapis.com";
+
+  // BigQuery Metastore catalog property keys
+  private static final String GCP_BIGQUERY_PROJECT_ID = "gcp.bigquery.project-id";
+  private static final String GCP_BIGQUERY_LOCATION = "gcp.bigquery.location";
+  private static final String GCP_BIGQUERY_LIST_ALL_TABLES = "gcp.bigquery.list-all-tables";
+  private static final String IMPERSONATE_SERVICE_ACCOUNT =
+      "gcp.bigquery.impersonate.service-account";
+  private static final String IMPERSONATE_LIFETIME_SECONDS =
+      "gcp.bigquery.impersonate.lifetime-seconds";
+  private static final String IMPERSONATE_SCOPES = "gcp.bigquery.impersonate.scopes";
+  private static final String IMPERSONATE_DELEGATES = "gcp.bigquery.impersonate.delegates";
+
+  private final String warehouse;
+  private final String gcpProjectId;
+  private final String gcpLocation;
+  private final Boolean listAllTables;
+  private final String impersonateServiceAccount;
+  private final Integer impersonateLifetimeSeconds;
+  private final List<String> impersonateScopes;
+  private final List<String> impersonateDelegates;
+
+  public BigQueryMetastoreConnectionConfigInfoDpo(
+      @JsonProperty(value = "uri", required = true) @Nonnull String uri,
+      @JsonProperty(value = "authenticationParameters", required = false) @Nullable
+          AuthenticationParametersDpo authenticationParameters,
+      @JsonProperty(value = "warehouse", required = true) @Nonnull String warehouse,
+      @JsonProperty(value = "gcpProjectId", required = true) @Nonnull String gcpProjectId,
+      @JsonProperty(value = "gcpLocation", required = false) @Nullable String gcpLocation,
+      @JsonProperty(value = "listAllTables", required = false) @Nullable Boolean listAllTables,
+      @JsonProperty(value = "impersonateServiceAccount", required = false) @Nullable
+          String impersonateServiceAccount,
+      @JsonProperty(value = "impersonateLifetimeSeconds", required = false) @Nullable
+          Integer impersonateLifetimeSeconds,
+      @JsonProperty(value = "impersonateScopes", required = false) @Nullable
+          List<String> impersonateScopes,
+      @JsonProperty(value = "impersonateDelegates", required = false) @Nullable
+          List<String> impersonateDelegates,
+      @JsonProperty(value = "serviceIdentity", required = false) @Nullable
+          ServiceIdentityInfoDpo serviceIdentity) {
+    super(ConnectionType.BIGQUERY.getCode(), uri, authenticationParameters, serviceIdentity);
+    this.warehouse = warehouse;
+    this.gcpProjectId = gcpProjectId;
+    this.gcpLocation = gcpLocation;
+    this.listAllTables = listAllTables;
+    this.impersonateServiceAccount = impersonateServiceAccount;
+    this.impersonateLifetimeSeconds = impersonateLifetimeSeconds;
+    this.impersonateScopes = impersonateScopes;
+    this.impersonateDelegates = impersonateDelegates;
+  }
+
+  public String getWarehouse() {
+    return warehouse;
+  }
+
+  public String getGcpProjectId() {
+    return gcpProjectId;
+  }
+
+  public String getGcpLocation() {
+    return gcpLocation;
+  }
+
+  public Boolean getListAllTables() {
+    return listAllTables;
+  }
+
+  public String getImpersonateServiceAccount() {
+    return impersonateServiceAccount;
+  }
+
+  public Integer getImpersonateLifetimeSeconds() {
+    return impersonateLifetimeSeconds;
+  }
+
+  public List<String> getImpersonateScopes() {
+    return impersonateScopes;
+  }
+
+  public List<String> getImpersonateDelegates() {
+    return impersonateDelegates;
+  }
+
+  @Override
+  public String toString() {
+    return MoreObjects.toStringHelper(this)
+        .add("connectionTypeCode", getConnectionTypeCode())
+        .add("uri", getUri())
+        .add("warehouse", warehouse)
+        .add("gcpProjectId", gcpProjectId)
+        .add("gcpLocation", gcpLocation)
+        .add("listAllTables", listAllTables)
+        .add("authenticationParameters", getAuthenticationParameters().toString())
+        .toString();
+  }
+
+  @Override
+  public @Nonnull Map<String, String> asIcebergCatalogProperties(
+      PolarisCredentialManager polarisCredentialManager) {
+    HashMap<String, String> properties = new HashMap<>();
+
+    // Required properties for BigQueryMetastoreCatalog
+    properties.put(GCP_BIGQUERY_PROJECT_ID, gcpProjectId);
+    properties.put(CatalogProperties.WAREHOUSE_LOCATION, warehouse);
+
+    // Optional properties
+    if (gcpLocation != null) {
+      properties.put(GCP_BIGQUERY_LOCATION, gcpLocation);
+    }
+    if (listAllTables != null) {
+      properties.put(GCP_BIGQUERY_LIST_ALL_TABLES, String.valueOf(listAllTables));
+    }
+
+    // Optional impersonation properties
+    if (impersonateServiceAccount != null) {
+      properties.put(IMPERSONATE_SERVICE_ACCOUNT, impersonateServiceAccount);
+    }
+    if (impersonateLifetimeSeconds != null) {
+      properties.put(IMPERSONATE_LIFETIME_SECONDS, String.valueOf(impersonateLifetimeSeconds));
+    }
+    if (impersonateScopes != null && !impersonateScopes.isEmpty()) {
+      properties.put(IMPERSONATE_SCOPES, String.join(",", impersonateScopes));
+    }
+    if (impersonateDelegates != null && !impersonateDelegates.isEmpty()) {
+      properties.put(IMPERSONATE_DELEGATES, String.join(",", impersonateDelegates));
+    }
+
+    if (getAuthenticationParameters() != null) {
+      // Add authentication-specific metadata (non-credential properties)
+      properties.putAll(
+          getAuthenticationParameters().asIcebergCatalogProperties(polarisCredentialManager));
+      // Add connection credentials from Polaris credential manager
+      ConnectionCredentials connectionCredentials =
+          polarisCredentialManager.getConnectionCredentials(this);
+      properties.putAll(connectionCredentials.credentials());
+    }
+    return properties;
+  }
+
+  @Override
+  public ConnectionConfigInfoDpo withServiceIdentity(
+      @Nonnull ServiceIdentityInfoDpo serviceIdentityInfo) {
+    return new BigQueryMetastoreConnectionConfigInfoDpo(
+        getUri(),
+        getAuthenticationParameters(),
+        warehouse,
+        gcpProjectId,
+        gcpLocation,
+        listAllTables,
+        impersonateServiceAccount,
+        impersonateLifetimeSeconds,
+        impersonateScopes,
+        impersonateDelegates,
+        serviceIdentityInfo);
+  }
+
+  @Override
+  public ConnectionConfigInfo asConnectionConfigInfoModel(

Review Comment:
   Sorry about that. I have added `testBigQueryMetastoreConnectionConfig` in `ConnectionConfigInfoDpoTest`.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3025936742


##########
spec/polaris-management-service.yml:
##########
@@ -965,6 +967,44 @@ components:
         warehouse:
           type: string
           description: The warehouse location for the hive catalog.
+    
+    BigQueryMetastoreConnectionConfigInfo:
+      type: object
+      description: Configuration necessary for connecting to a BigQuery Metastore Catalog
+      allOf:
+        - $ref: '#/components/schemas/ConnectionConfigInfo'
+      properties:
+        warehouse:
+          type: string
+          description: The GCS bucket path for the catalog (e.g., gs://bucket-name)
+        gcpProjectId:
+          type: string
+          description: The GCP project ID
+        gcpLocation:
+          type: string
+          description: The GCP location (default = us)
+        listAllTables:
+          type: boolean
+          description: Whether to list all tables (default = true)
+        impersonateServiceAccount:
+          type: string
+          description: Service account email to impersonate
+        impersonateLifetimeSeconds:
+          type: integer
+          description: Token lifetime in seconds for impersonation (default = 3600)
+        impersonateScopes:
+          type: array
+          items:
+            type: string
+          description: OAuth scopes for impersonation (default = cloud-platform)
+        impersonateDelegates:
+          type: array
+          items:
+            type: string
+          description: Delegation chain for impersonation

Review Comment:
   You are right, I will work on addressing this.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3019726630


##########
extensions/federation/bigquery/src/main/java/org/apache/polaris/extensions/federation/bigquery/BigQueryMetastoreFederatedCatalogFactory.java:
##########
@@ -0,0 +1,79 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.extensions.federation.bigquery;
+
+import io.smallrye.common.annotation.Identifier;
+import jakarta.enterprise.context.ApplicationScoped;
+import java.util.Map;
+import org.apache.iceberg.catalog.Catalog;
+import org.apache.iceberg.gcp.bigquery.BigQueryMetastoreCatalog;
+import org.apache.iceberg.rest.RESTUtil;
+import org.apache.polaris.core.catalog.ExternalCatalogFactory;
+import org.apache.polaris.core.catalog.GenericTableCatalog;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.AuthenticationType;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.connection.bigquery.BigQueryMetastoreConnectionConfigInfoDpo;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+
+/** Factory class for creating a BigQuery Metastore catalog handle based on connection configuration. */
+@ApplicationScoped
+@Identifier(ConnectionType.BIGQUERY_FACTORY_IDENTIFIER)
+public class BigQueryMetastoreFederatedCatalogFactory implements ExternalCatalogFactory {
+
+  @Override
+  public Catalog createCatalog(
+      ConnectionConfigInfoDpo connectionConfigInfoDpo,
+      PolarisCredentialManager polarisCredentialManager,
+      Map<String, String> catalogProperties) {
+
+    // Currently, Polaris supports BigQuery Metastore federation only via IMPLICIT authentication.
+    // Hence, prior to initializing the configuration, ensure that the catalog uses
+    // IMPLICIT authentication.
+    AuthenticationParametersDpo authenticationParametersDpo =
+        connectionConfigInfoDpo.getAuthenticationParameters();
+    if (authenticationParametersDpo.getAuthenticationTypeCode()
+        != AuthenticationType.IMPLICIT.getCode()) {
+      throw new IllegalStateException(
+          "BigQuery Metastore federation only supports IMPLICIT authentication.");
+    }
+
+    String warehouse =
+        ((BigQueryMetastoreConnectionConfigInfoDpo) connectionConfigInfoDpo).getWarehouse();
+    Map<String, String> mergedProperties =
+        RESTUtil.merge(
+            catalogProperties != null ? catalogProperties : Map.of(),
+            connectionConfigInfoDpo.asIcebergCatalogProperties(polarisCredentialManager));
+
+    BigQueryMetastoreCatalog bigQueryMetastoreCatalog = new BigQueryMetastoreCatalog();
+    bigQueryMetastoreCatalog.initialize(warehouse, mergedProperties);

Review Comment:
   Thanks for the explanation, @joyhaldar. Can we comment out this "hidden" behavior somewhere?



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3033111375


##########
runtime/server/build.gradle.kts:
##########
@@ -43,6 +43,10 @@ dependencies {
     runtimeOnly(project(":polaris-extensions-federation-hive"))
   }
 
+  if ((project.findProperty("NonRESTCatalogs") as String?)?.contains("BIGQUERY") == true) {
+    runtimeOnly(project(":polaris-extensions-federation-bigquery"))

Review Comment:
   Reverted to optional for now. Making it unconditional was breaking CI due to LICENSE/NOTICE requirements for the new BigQuery dependencies. I can make it unconditional once the license entries are sorted out, wanted to unblock the PR first.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "flyingImer (via GitHub)" <gi...@apache.org>.
flyingImer commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r2996622689


##########
spec/polaris-management-service.yml:
##########
@@ -965,6 +967,44 @@ components:
         warehouse:
           type: string
           description: The warehouse location for the hive catalog.
+    
+    BigQueryMetastoreConnectionConfigInfo:
+      type: object
+      description: Configuration necessary for connecting to a BigQuery Metastore Catalog
+      allOf:
+        - $ref: '#/components/schemas/ConnectionConfigInfo'
+      properties:
+        warehouse:
+          type: string
+          description: The GCS bucket path for the catalog (e.g., gs://bucket-name)
+        gcpProjectId:
+          type: string
+          description: The GCP project ID
+        gcpLocation:
+          type: string
+          description: The GCP location (default = us)
+        listAllTables:
+          type: boolean
+          description: Whether to list all tables (default = true)
+        impersonateServiceAccount:
+          type: string
+          description: Service account email to impersonate
+        impersonateLifetimeSeconds:
+          type: integer
+          description: Token lifetime in seconds for impersonation (default = 3600)
+        impersonateScopes:
+          type: array
+          items:
+            type: string
+          description: OAuth scopes for impersonation (default = cloud-platform)
+        impersonateDelegates:
+          type: array
+          items:
+            type: string
+          description: Delegation chain for impersonation
+      required:

Review Comment:
   IIUC, `BIGQUERY` can still omit `authenticationParameters` at the API boundary, but the current implementation only supports `IMPLICIT` auth and later code paths assume the field is present. Would it make sense to tighten the BIGQUERY contract here instead, so the boundary reflects what the runtime actually supports today?



##########
extensions/federation/bigquery/src/main/java/org/apache/polaris/extensions/federation/bigquery/BigQueryMetastoreFederatedCatalogFactory.java:
##########
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.extensions.federation.bigquery;
+
+import io.smallrye.common.annotation.Identifier;
+import jakarta.enterprise.context.ApplicationScoped;
+import java.util.Map;
+import org.apache.iceberg.catalog.Catalog;
+import org.apache.iceberg.gcp.bigquery.BigQueryMetastoreCatalog;
+import org.apache.iceberg.rest.RESTUtil;
+import org.apache.polaris.core.catalog.ExternalCatalogFactory;
+import org.apache.polaris.core.catalog.GenericTableCatalog;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.AuthenticationType;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.connection.bigquery.BigQueryMetastoreConnectionConfigInfoDpo;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+
+/**
+ * Factory class for creating a BigQuery Metastore catalog handle based on connection configuration.
+ */
+@ApplicationScoped
+@Identifier(ConnectionType.BIGQUERY_FACTORY_IDENTIFIER)
+public class BigQueryMetastoreFederatedCatalogFactory implements ExternalCatalogFactory {
+
+  @Override
+  public Catalog createCatalog(
+      ConnectionConfigInfoDpo connectionConfigInfoDpo,
+      PolarisCredentialManager polarisCredentialManager,
+      Map<String, String> catalogProperties) {
+
+    // Currently, Polaris supports BigQuery Metastore federation only via IMPLICIT authentication.
+    // Hence, prior to initializing the configuration, ensure that the catalog uses
+    // IMPLICIT authentication.
+    AuthenticationParametersDpo authenticationParametersDpo =
+        connectionConfigInfoDpo.getAuthenticationParameters();
+    if (authenticationParametersDpo.getAuthenticationTypeCode()

Review Comment:
   IIUC, This path seems to rely on `authenticationParameters` being present, while the public BIGQUERY contract still appears to allow it to be omitted. I wonder if that mismatch should be resolved at validation / schema time rather than here in the runtime path.



##########
polaris-core/src/main/java/org/apache/polaris/core/connection/bigquery/BigQueryMetastoreConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,228 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.core.connection.bigquery;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.MoreObjects;
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+import org.apache.polaris.core.credentials.connection.ConnectionCredentials;
+import org.apache.polaris.core.identity.dpo.ServiceIdentityInfoDpo;
+import org.apache.polaris.core.identity.provider.ServiceIdentityProvider;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo} defined in the API
+ * model.
+ */
+public class BigQueryMetastoreConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+  public static final String DEFAULT_URI = "https://bigquery.googleapis.com";
+
+  // BigQuery Metastore catalog property keys
+  private static final String GCP_BIGQUERY_PROJECT_ID = "gcp.bigquery.project-id";
+  private static final String GCP_BIGQUERY_LOCATION = "gcp.bigquery.location";
+  private static final String GCP_BIGQUERY_LIST_ALL_TABLES = "gcp.bigquery.list-all-tables";
+  private static final String IMPERSONATE_SERVICE_ACCOUNT =
+      "gcp.bigquery.impersonate.service-account";
+  private static final String IMPERSONATE_LIFETIME_SECONDS =
+      "gcp.bigquery.impersonate.lifetime-seconds";
+  private static final String IMPERSONATE_SCOPES = "gcp.bigquery.impersonate.scopes";
+  private static final String IMPERSONATE_DELEGATES = "gcp.bigquery.impersonate.delegates";
+
+  private final String warehouse;
+  private final String gcpProjectId;
+  private final String gcpLocation;
+  private final Boolean listAllTables;
+  private final String impersonateServiceAccount;
+  private final Integer impersonateLifetimeSeconds;
+  private final List<String> impersonateScopes;
+  private final List<String> impersonateDelegates;
+
+  public BigQueryMetastoreConnectionConfigInfoDpo(
+      @JsonProperty(value = "uri", required = true) @Nonnull String uri,
+      @JsonProperty(value = "authenticationParameters", required = false) @Nullable
+          AuthenticationParametersDpo authenticationParameters,
+      @JsonProperty(value = "warehouse", required = true) @Nonnull String warehouse,
+      @JsonProperty(value = "gcpProjectId", required = true) @Nonnull String gcpProjectId,
+      @JsonProperty(value = "gcpLocation", required = false) @Nullable String gcpLocation,
+      @JsonProperty(value = "listAllTables", required = false) @Nullable Boolean listAllTables,
+      @JsonProperty(value = "impersonateServiceAccount", required = false) @Nullable
+          String impersonateServiceAccount,
+      @JsonProperty(value = "impersonateLifetimeSeconds", required = false) @Nullable
+          Integer impersonateLifetimeSeconds,
+      @JsonProperty(value = "impersonateScopes", required = false) @Nullable
+          List<String> impersonateScopes,
+      @JsonProperty(value = "impersonateDelegates", required = false) @Nullable
+          List<String> impersonateDelegates,
+      @JsonProperty(value = "serviceIdentity", required = false) @Nullable
+          ServiceIdentityInfoDpo serviceIdentity) {
+    super(ConnectionType.BIGQUERY.getCode(), uri, authenticationParameters, serviceIdentity);
+    this.warehouse = warehouse;
+    this.gcpProjectId = gcpProjectId;
+    this.gcpLocation = gcpLocation;
+    this.listAllTables = listAllTables;
+    this.impersonateServiceAccount = impersonateServiceAccount;
+    this.impersonateLifetimeSeconds = impersonateLifetimeSeconds;
+    this.impersonateScopes = impersonateScopes;
+    this.impersonateDelegates = impersonateDelegates;
+  }
+
+  public String getWarehouse() {
+    return warehouse;
+  }
+
+  public String getGcpProjectId() {
+    return gcpProjectId;
+  }
+
+  public String getGcpLocation() {
+    return gcpLocation;
+  }
+
+  public Boolean getListAllTables() {
+    return listAllTables;
+  }
+
+  public String getImpersonateServiceAccount() {
+    return impersonateServiceAccount;
+  }
+
+  public Integer getImpersonateLifetimeSeconds() {
+    return impersonateLifetimeSeconds;
+  }
+
+  public List<String> getImpersonateScopes() {
+    return impersonateScopes;
+  }
+
+  public List<String> getImpersonateDelegates() {
+    return impersonateDelegates;
+  }
+
+  @Override
+  public String toString() {
+    return MoreObjects.toStringHelper(this)
+        .add("connectionTypeCode", getConnectionTypeCode())
+        .add("uri", getUri())
+        .add("warehouse", warehouse)
+        .add("gcpProjectId", gcpProjectId)
+        .add("gcpLocation", gcpLocation)
+        .add("listAllTables", listAllTables)
+        .add("authenticationParameters", getAuthenticationParameters().toString())
+        .toString();
+  }
+
+  @Override
+  public @Nonnull Map<String, String> asIcebergCatalogProperties(
+      PolarisCredentialManager polarisCredentialManager) {
+    HashMap<String, String> properties = new HashMap<>();
+
+    // Required properties for BigQueryMetastoreCatalog
+    properties.put(GCP_BIGQUERY_PROJECT_ID, gcpProjectId);
+    properties.put(CatalogProperties.WAREHOUSE_LOCATION, warehouse);
+
+    // Optional properties
+    if (gcpLocation != null) {
+      properties.put(GCP_BIGQUERY_LOCATION, gcpLocation);
+    }
+    if (listAllTables != null) {
+      properties.put(GCP_BIGQUERY_LIST_ALL_TABLES, String.valueOf(listAllTables));
+    }
+
+    // Optional impersonation properties
+    if (impersonateServiceAccount != null) {
+      properties.put(IMPERSONATE_SERVICE_ACCOUNT, impersonateServiceAccount);
+    }
+    if (impersonateLifetimeSeconds != null) {
+      properties.put(IMPERSONATE_LIFETIME_SECONDS, String.valueOf(impersonateLifetimeSeconds));
+    }
+    if (impersonateScopes != null && !impersonateScopes.isEmpty()) {
+      properties.put(IMPERSONATE_SCOPES, String.join(",", impersonateScopes));
+    }
+    if (impersonateDelegates != null && !impersonateDelegates.isEmpty()) {
+      properties.put(IMPERSONATE_DELEGATES, String.join(",", impersonateDelegates));
+    }
+
+    if (getAuthenticationParameters() != null) {
+      // Add authentication-specific metadata (non-credential properties)
+      properties.putAll(
+          getAuthenticationParameters().asIcebergCatalogProperties(polarisCredentialManager));
+      // Add connection credentials from Polaris credential manager
+      ConnectionCredentials connectionCredentials =
+          polarisCredentialManager.getConnectionCredentials(this);
+      properties.putAll(connectionCredentials.credentials());
+    }
+    return properties;
+  }
+
+  @Override
+  public ConnectionConfigInfoDpo withServiceIdentity(
+      @Nonnull ServiceIdentityInfoDpo serviceIdentityInfo) {
+    return new BigQueryMetastoreConnectionConfigInfoDpo(
+        getUri(),
+        getAuthenticationParameters(),
+        warehouse,
+        gcpProjectId,
+        gcpLocation,
+        listAllTables,
+        impersonateServiceAccount,
+        impersonateLifetimeSeconds,
+        impersonateScopes,
+        impersonateDelegates,
+        serviceIdentityInfo);
+  }
+
+  @Override
+  public ConnectionConfigInfo asConnectionConfigInfoModel(
+      ServiceIdentityProvider serviceIdentityProvider) {
+    return BigQueryMetastoreConnectionConfigInfo.builder()
+        .setConnectionType(ConnectionConfigInfo.ConnectionTypeEnum.BIGQUERY)
+        .setUri(getUri())
+        .setWarehouse(warehouse)
+        .setGcpProjectId(gcpProjectId)
+        .setGcpLocation(gcpLocation)
+        .setListAllTables(listAllTables)
+        .setImpersonateServiceAccount(impersonateServiceAccount)
+        .setImpersonateLifetimeSeconds(impersonateLifetimeSeconds)
+        .setImpersonateScopes(impersonateScopes)
+        .setImpersonateDelegates(impersonateDelegates)
+        .setAuthenticationParameters(

Review Comment:
   Related contract gap: the readback/model-conversion path also seems to assume `authenticationParameters` is non-null here. So even if the create/update boundary accepts a BIGQUERY payload without an auth block, we can still fail later when converting the stored config back to the API model.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r2992625114


##########
polaris-core/src/main/java/org/apache/polaris/core/connection/bigquery/BigQueryMetastoreConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,230 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.core.connection.bigquery;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.MoreObjects;
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+import org.apache.polaris.core.credentials.connection.ConnectionCredentials;
+import org.apache.polaris.core.identity.dpo.ServiceIdentityInfoDpo;
+import org.apache.polaris.core.identity.provider.ServiceIdentityProvider;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo} defined in the API
+ * model.
+ */
+public class BigQueryMetastoreConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+  // BigQuery Metastore catalog property keys
+  private static final String GCP_BIGQUERY_PROJECT_ID = "gcp.bigquery.project-id";
+  private static final String GCP_BIGQUERY_LOCATION = "gcp.bigquery.location";
+  private static final String GCP_BIGQUERY_LIST_ALL_TABLES = "gcp.bigquery.list-all-tables";
+  private static final String IMPERSONATE_SERVICE_ACCOUNT =
+      "gcp.bigquery.impersonate.service-account";
+  private static final String IMPERSONATE_LIFETIME_SECONDS =
+      "gcp.bigquery.impersonate.lifetime-seconds";
+  private static final String IMPERSONATE_SCOPES = "gcp.bigquery.impersonate.scopes";
+  private static final String IMPERSONATE_DELEGATES = "gcp.bigquery.impersonate.delegates";
+
+  private final String warehouse;
+  private final String gcpProjectId;
+  private final String gcpLocation;
+  private final Boolean listAllTables;
+  private final String impersonateServiceAccount;
+  private final Integer impersonateLifetimeSeconds;
+  private final List<String> impersonateScopes;
+  private final List<String> impersonateDelegates;
+
+  public BigQueryMetastoreConnectionConfigInfoDpo(
+      @JsonProperty(value = "uri", required = true) @Nonnull String uri,
+      @JsonProperty(value = "authenticationParameters", required = false) @Nullable
+          AuthenticationParametersDpo authenticationParameters,
+      @JsonProperty(value = "warehouse", required = true) @Nonnull String warehouse,
+      @JsonProperty(value = "gcpProjectId", required = true) @Nonnull String gcpProjectId,
+      @JsonProperty(value = "gcpLocation", required = false) @Nullable String gcpLocation,
+      @JsonProperty(value = "listAllTables", required = false) @Nullable Boolean listAllTables,
+      @JsonProperty(value = "impersonateServiceAccount", required = false) @Nullable
+          String impersonateServiceAccount,
+      @JsonProperty(value = "impersonateLifetimeSeconds", required = false) @Nullable
+          Integer impersonateLifetimeSeconds,
+      @JsonProperty(value = "impersonateScopes", required = false) @Nullable
+          List<String> impersonateScopes,
+      @JsonProperty(value = "impersonateDelegates", required = false) @Nullable
+          List<String> impersonateDelegates,
+      @JsonProperty(value = "serviceIdentity", required = false) @Nullable
+          ServiceIdentityInfoDpo serviceIdentity) {
+    super(ConnectionType.BIGQUERY.getCode(), uri, authenticationParameters, serviceIdentity);
+    this.warehouse = warehouse;
+    this.gcpProjectId = gcpProjectId;
+    this.gcpLocation = gcpLocation;
+    this.listAllTables = listAllTables;
+    this.impersonateServiceAccount = impersonateServiceAccount;
+    this.impersonateLifetimeSeconds = impersonateLifetimeSeconds;
+    this.impersonateScopes = impersonateScopes;
+    this.impersonateDelegates = impersonateDelegates;
+  }
+
+  public String getWarehouse() {
+    return warehouse;
+  }
+
+  public String getGcpProjectId() {
+    return gcpProjectId;
+  }
+
+  public String getGcpLocation() {
+    return gcpLocation;
+  }
+
+  public Boolean getListAllTables() {
+    return listAllTables;
+  }
+
+  public String getImpersonateServiceAccount() {
+    return impersonateServiceAccount;
+  }
+
+  public Integer getImpersonateLifetimeSeconds() {
+    return impersonateLifetimeSeconds;
+  }
+
+  public List<String> getImpersonateScopes() {
+    return impersonateScopes;
+  }
+
+  public List<String> getImpersonateDelegates() {
+    return impersonateDelegates;
+  }
+
+  @Override
+  public String toString() {
+    return MoreObjects.toStringHelper(this)
+        .add("connectionTypeCode", getConnectionTypeCode())
+        .add("uri", getUri())
+        .add("warehouse", warehouse)
+        .add("gcpProjectId", gcpProjectId)
+        .add("gcpLocation", gcpLocation)
+        .add("listAllTables", listAllTables)
+        .add("impersonateServiceAccount", impersonateServiceAccount)

Review Comment:
   Thank you for your review JB.
   
   The `impersonateServiceAccount` is just a service account email (e.g., `sa@project.iam.gserviceaccount.com`), not a secret. I think it could be helpful for someone to confirm if impersonation is configured correctly. But I could be wrong. Please let me know what you'd like.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on PR #4050:
URL: https://github.com/apache/polaris/pull/4050#issuecomment-4282728643

   Merge is blocked by Github CI. Re-triggered. I will merge it today if there is no more feedback.


-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3087678254


##########
polaris-core/src/main/java/org/apache/polaris/core/connection/bigquery/BigQueryMetastoreConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.core.connection.bigquery;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.MoreObjects;
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+import org.apache.polaris.core.credentials.connection.ConnectionCredentials;
+import org.apache.polaris.core.identity.dpo.ServiceIdentityInfoDpo;
+import org.apache.polaris.core.identity.provider.ServiceIdentityProvider;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo} defined in the API
+ * model.
+ */
+public class BigQueryMetastoreConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+  public static final String DEFAULT_URI = "https://bigquery.googleapis.com";
+
+  // BigQuery Metastore catalog property keys
+  private static final String GCP_BIGQUERY_PROJECT_ID = "gcp.bigquery.project-id";
+
+  private final String warehouse;
+  private final String gcpProjectId;
+
+  public BigQueryMetastoreConnectionConfigInfoDpo(
+      @JsonProperty(value = "uri", required = true) @Nonnull String uri,
+      @JsonProperty(value = "authenticationParameters", required = false) @Nullable
+          AuthenticationParametersDpo authenticationParameters,
+      @JsonProperty(value = "warehouse", required = true) @Nonnull String warehouse,
+      @JsonProperty(value = "gcpProjectId", required = true) @Nonnull String gcpProjectId,
+      @JsonProperty(value = "properties", required = false) @Nullable
+          Map<String, String> properties,
+      @JsonProperty(value = "serviceIdentity", required = false) @Nullable
+          ServiceIdentityInfoDpo serviceIdentity) {
+    super(
+        ConnectionType.BIGQUERY.getCode(),
+        uri,
+        authenticationParameters,
+        serviceIdentity,
+        properties);
+    this.warehouse = warehouse;
+    this.gcpProjectId = gcpProjectId;
+  }
+
+  public String getWarehouse() {
+    return warehouse;
+  }
+
+  public String getGcpProjectId() {
+    return gcpProjectId;
+  }
+
+  @Override
+  public String toString() {
+    return MoreObjects.toStringHelper(this)
+        .add("connectionTypeCode", getConnectionTypeCode())
+        .add("uri", getUri())
+        .add("warehouse", warehouse)
+        .add("gcpProjectId", gcpProjectId)
+        .add(
+            "authenticationParameters",
+            getAuthenticationParameters() != null
+                ? getAuthenticationParameters().toString()
+                : "null")
+        .toString();
+  }
+
+  @Override
+  public @Nonnull Map<String, String> asIcebergCatalogProperties(
+      PolarisCredentialManager polarisCredentialManager) {
+    HashMap<String, String> properties = new HashMap<>();
+
+    // Required properties for BigQueryMetastoreCatalog
+    properties.put(GCP_BIGQUERY_PROJECT_ID, gcpProjectId);
+    properties.put(CatalogProperties.WAREHOUSE_LOCATION, warehouse);

Review Comment:
   Re: Hadoop and Hive: from my POV the same argument applies to them, but I missed out on the review of that code when it was introduced :sweat_smile: 
   
   This is why my request in this thread is non-blocking.
   
   However, I do not really see a reason to promote an old pattern if we're in agreement about the rationale for generic properties in this case (hence my request for more reviewer opinions).



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3087678254


##########
polaris-core/src/main/java/org/apache/polaris/core/connection/bigquery/BigQueryMetastoreConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.core.connection.bigquery;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.MoreObjects;
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+import org.apache.polaris.core.credentials.connection.ConnectionCredentials;
+import org.apache.polaris.core.identity.dpo.ServiceIdentityInfoDpo;
+import org.apache.polaris.core.identity.provider.ServiceIdentityProvider;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo} defined in the API
+ * model.
+ */
+public class BigQueryMetastoreConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+  public static final String DEFAULT_URI = "https://bigquery.googleapis.com";
+
+  // BigQuery Metastore catalog property keys
+  private static final String GCP_BIGQUERY_PROJECT_ID = "gcp.bigquery.project-id";
+
+  private final String warehouse;
+  private final String gcpProjectId;
+
+  public BigQueryMetastoreConnectionConfigInfoDpo(
+      @JsonProperty(value = "uri", required = true) @Nonnull String uri,
+      @JsonProperty(value = "authenticationParameters", required = false) @Nullable
+          AuthenticationParametersDpo authenticationParameters,
+      @JsonProperty(value = "warehouse", required = true) @Nonnull String warehouse,
+      @JsonProperty(value = "gcpProjectId", required = true) @Nonnull String gcpProjectId,
+      @JsonProperty(value = "properties", required = false) @Nullable
+          Map<String, String> properties,
+      @JsonProperty(value = "serviceIdentity", required = false) @Nullable
+          ServiceIdentityInfoDpo serviceIdentity) {
+    super(
+        ConnectionType.BIGQUERY.getCode(),
+        uri,
+        authenticationParameters,
+        serviceIdentity,
+        properties);
+    this.warehouse = warehouse;
+    this.gcpProjectId = gcpProjectId;
+  }
+
+  public String getWarehouse() {
+    return warehouse;
+  }
+
+  public String getGcpProjectId() {
+    return gcpProjectId;
+  }
+
+  @Override
+  public String toString() {
+    return MoreObjects.toStringHelper(this)
+        .add("connectionTypeCode", getConnectionTypeCode())
+        .add("uri", getUri())
+        .add("warehouse", warehouse)
+        .add("gcpProjectId", gcpProjectId)
+        .add(
+            "authenticationParameters",
+            getAuthenticationParameters() != null
+                ? getAuthenticationParameters().toString()
+                : "null")
+        .toString();
+  }
+
+  @Override
+  public @Nonnull Map<String, String> asIcebergCatalogProperties(
+      PolarisCredentialManager polarisCredentialManager) {
+    HashMap<String, String> properties = new HashMap<>();
+
+    // Required properties for BigQueryMetastoreCatalog
+    properties.put(GCP_BIGQUERY_PROJECT_ID, gcpProjectId);
+    properties.put(CatalogProperties.WAREHOUSE_LOCATION, warehouse);

Review Comment:
   Re: Hadoop and Hive: from my POV the same argument applies to them, but I missed out on the review of that code when it was introduced :sweat_smile: 
   
   This is why my request in this thread is non-blocking.
   
   However, I do not really see a reason to promote an old pattern if we're in agreement about the rationale for generic properties in this case (hence my request for more review opinions).



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3029539760


##########
polaris-core/src/main/java/org/apache/polaris/core/connection/bigquery/BigQueryMetastoreConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,230 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.core.connection.bigquery;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.MoreObjects;
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+import org.apache.polaris.core.credentials.connection.ConnectionCredentials;
+import org.apache.polaris.core.identity.dpo.ServiceIdentityInfoDpo;
+import org.apache.polaris.core.identity.provider.ServiceIdentityProvider;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo} defined in the API
+ * model.
+ */
+public class BigQueryMetastoreConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+  public static final String DEFAULT_URI = "https://bigquery.googleapis.com";
+
+  // BigQuery Metastore catalog property keys
+  private static final String GCP_BIGQUERY_PROJECT_ID = "gcp.bigquery.project-id";
+  private static final String GCP_BIGQUERY_LOCATION = "gcp.bigquery.location";
+  private static final String GCP_BIGQUERY_LIST_ALL_TABLES = "gcp.bigquery.list-all-tables";
+  private static final String IMPERSONATE_SERVICE_ACCOUNT =
+      "gcp.bigquery.impersonate.service-account";
+  private static final String IMPERSONATE_LIFETIME_SECONDS =
+      "gcp.bigquery.impersonate.lifetime-seconds";
+  private static final String IMPERSONATE_SCOPES = "gcp.bigquery.impersonate.scopes";
+  private static final String IMPERSONATE_DELEGATES = "gcp.bigquery.impersonate.delegates";
+
+  private final String warehouse;
+  private final String gcpProjectId;
+  private final String gcpLocation;
+  private final Boolean listAllTables;
+  private final String impersonateServiceAccount;
+  private final Integer impersonateLifetimeSeconds;
+  private final List<String> impersonateScopes;
+  private final List<String> impersonateDelegates;
+
+  public BigQueryMetastoreConnectionConfigInfoDpo(
+      @JsonProperty(value = "uri", required = true) @Nonnull String uri,
+      @JsonProperty(value = "authenticationParameters", required = false) @Nullable
+          AuthenticationParametersDpo authenticationParameters,
+      @JsonProperty(value = "warehouse", required = true) @Nonnull String warehouse,
+      @JsonProperty(value = "gcpProjectId", required = true) @Nonnull String gcpProjectId,
+      @JsonProperty(value = "gcpLocation", required = false) @Nullable String gcpLocation,
+      @JsonProperty(value = "listAllTables", required = false) @Nullable Boolean listAllTables,
+      @JsonProperty(value = "impersonateServiceAccount", required = false) @Nullable
+          String impersonateServiceAccount,
+      @JsonProperty(value = "impersonateLifetimeSeconds", required = false) @Nullable
+          Integer impersonateLifetimeSeconds,
+      @JsonProperty(value = "impersonateScopes", required = false) @Nullable
+          List<String> impersonateScopes,
+      @JsonProperty(value = "impersonateDelegates", required = false) @Nullable
+          List<String> impersonateDelegates,
+      @JsonProperty(value = "serviceIdentity", required = false) @Nullable
+          ServiceIdentityInfoDpo serviceIdentity) {
+    super(ConnectionType.BIGQUERY.getCode(), uri, authenticationParameters, serviceIdentity);
+    this.warehouse = warehouse;
+    this.gcpProjectId = gcpProjectId;
+    this.gcpLocation = gcpLocation;
+    this.listAllTables = listAllTables;
+    this.impersonateServiceAccount = impersonateServiceAccount;
+    this.impersonateLifetimeSeconds = impersonateLifetimeSeconds;
+    this.impersonateScopes = impersonateScopes;
+    this.impersonateDelegates = impersonateDelegates;
+  }
+
+  public String getWarehouse() {
+    return warehouse;
+  }
+
+  public String getGcpProjectId() {
+    return gcpProjectId;
+  }
+
+  public String getGcpLocation() {
+    return gcpLocation;
+  }
+
+  public Boolean getListAllTables() {
+    return listAllTables;
+  }
+
+  public String getImpersonateServiceAccount() {
+    return impersonateServiceAccount;
+  }
+
+  public Integer getImpersonateLifetimeSeconds() {
+    return impersonateLifetimeSeconds;
+  }
+
+  public List<String> getImpersonateScopes() {
+    return impersonateScopes;
+  }
+
+  public List<String> getImpersonateDelegates() {
+    return impersonateDelegates;
+  }
+
+  @Override
+  public String toString() {
+    return MoreObjects.toStringHelper(this)
+        .add("connectionTypeCode", getConnectionTypeCode())
+        .add("uri", getUri())
+        .add("warehouse", warehouse)
+        .add("gcpProjectId", gcpProjectId)
+        .add("gcpLocation", gcpLocation)
+        .add("listAllTables", listAllTables)
+        .add("authenticationParameters", getAuthenticationParameters())

Review Comment:
   Done. Added the null check as suggested. Thank you again JB!



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3029534236


##########
polaris-core/src/main/java/org/apache/polaris/core/connection/bigquery/BigQueryMetastoreConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,230 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.core.connection.bigquery;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.MoreObjects;
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+import org.apache.polaris.core.credentials.connection.ConnectionCredentials;
+import org.apache.polaris.core.identity.dpo.ServiceIdentityInfoDpo;
+import org.apache.polaris.core.identity.provider.ServiceIdentityProvider;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo} defined in the API
+ * model.
+ */
+public class BigQueryMetastoreConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+  public static final String DEFAULT_URI = "https://bigquery.googleapis.com";
+
+  // BigQuery Metastore catalog property keys
+  private static final String GCP_BIGQUERY_PROJECT_ID = "gcp.bigquery.project-id";
+  private static final String GCP_BIGQUERY_LOCATION = "gcp.bigquery.location";
+  private static final String GCP_BIGQUERY_LIST_ALL_TABLES = "gcp.bigquery.list-all-tables";
+  private static final String IMPERSONATE_SERVICE_ACCOUNT =
+      "gcp.bigquery.impersonate.service-account";
+  private static final String IMPERSONATE_LIFETIME_SECONDS =
+      "gcp.bigquery.impersonate.lifetime-seconds";
+  private static final String IMPERSONATE_SCOPES = "gcp.bigquery.impersonate.scopes";
+  private static final String IMPERSONATE_DELEGATES = "gcp.bigquery.impersonate.delegates";
+
+  private final String warehouse;
+  private final String gcpProjectId;
+  private final String gcpLocation;
+  private final Boolean listAllTables;
+  private final String impersonateServiceAccount;
+  private final Integer impersonateLifetimeSeconds;
+  private final List<String> impersonateScopes;
+  private final List<String> impersonateDelegates;

Review Comment:
   Thank you for the review. These fields are now part of the [properties bag](https://github.com/apache/polaris/pull/4050#discussion_r3029516972).



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "nandorKollar (via GitHub)" <gi...@apache.org>.
nandorKollar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3091778864


##########
polaris-core/src/main/java/org/apache/polaris/core/connection/hadoop/HadoopConnectionConfigInfoDpo.java:
##########
@@ -51,7 +51,8 @@ public HadoopConnectionConfigInfoDpo(
       @JsonProperty(value = "serviceIdentity", required = false) @Nullable
           ServiceIdentityInfoDpo serviceIdentityInfo,
       @JsonProperty(value = "warehouse", required = false) @Nullable String remoteCatalogName) {
-    super(ConnectionType.HADOOP.getCode(), uri, authenticationParameters, serviceIdentityInfo);
+    super(
+        ConnectionType.HADOOP.getCode(), uri, authenticationParameters, serviceIdentityInfo, null);

Review Comment:
   nit: I think we can spare adding these ugly 'null' parameters in the super calls. We can overload the public constructor of `ConnectionConfigInfoDpo` like this:
   
   ```
     public ConnectionConfigInfoDpo(
             @JsonProperty(value = "connectionTypeCode", required = true) int connectionTypeCode,
             @JsonProperty(value = "uri", required = true) @Nonnull String uri,
             @JsonProperty(value = "authenticationParameters", required = true) @Nullable
             AuthenticationParametersDpo authenticationParameters,
             @JsonProperty(value = "serviceIdentity", required = false) @Nullable
             ServiceIdentityInfoDpo serviceIdentity,
             @JsonProperty(value = "properties", required = false) @Nullable
             Map<String, String> properties) {
       this(connectionTypeCode, uri, authenticationParameters, serviceIdentity, true, properties);
     }
   
     public ConnectionConfigInfoDpo(
         @JsonProperty(value = "connectionTypeCode", required = true) int connectionTypeCode,
         @JsonProperty(value = "uri", required = true) @Nonnull String uri,
         @JsonProperty(value = "authenticationParameters", required = true) @Nullable
             AuthenticationParametersDpo authenticationParameters,
         @JsonProperty(value = "serviceIdentity", required = false) @Nullable
             ServiceIdentityInfoDpo serviceIdentity) {
       this(connectionTypeCode, uri, authenticationParameters, serviceIdentity, true, null);
     }
   ```



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3092819165


##########
polaris-core/src/main/java/org/apache/polaris/core/connection/bigquery/BigQueryMetastoreConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,123 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.core.connection.bigquery;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.MoreObjects;
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+import org.apache.polaris.core.credentials.connection.ConnectionCredentials;
+import org.apache.polaris.core.identity.dpo.ServiceIdentityInfoDpo;
+import org.apache.polaris.core.identity.provider.ServiceIdentityProvider;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo} defined in the API
+ * model.
+ */
+public class BigQueryMetastoreConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+  public static final String DEFAULT_URI = "https://bigquery.googleapis.com";

Review Comment:
   Actually it's public because it's used in `ConnectionConfigInfoDpo.fromConnectionConfigInfoModelWithSecrets()` for URI defaulting. I kept it in the BQMS DPO since it's specific to BQMS. But let me know if you disagree, would appreciate your inputs.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r2995781653


##########
extensions/federation/bigquery/src/main/java/org/apache/polaris/extensions/federation/bigquery/BigQueryMetastoreFederatedCatalogFactory.java:
##########
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.extensions.federation.bigquery;
+
+import io.smallrye.common.annotation.Identifier;
+import jakarta.enterprise.context.ApplicationScoped;
+import java.util.Map;
+import org.apache.iceberg.catalog.Catalog;
+import org.apache.iceberg.gcp.bigquery.BigQueryMetastoreCatalog;
+import org.apache.iceberg.rest.RESTUtil;
+import org.apache.polaris.core.catalog.ExternalCatalogFactory;
+import org.apache.polaris.core.catalog.GenericTableCatalog;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.AuthenticationType;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.connection.bigquery.BigQueryMetastoreConnectionConfigInfoDpo;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+
+/**
+ * Factory class for creating a BigQuery Metastore catalog handle based on connection configuration.
+ */
+@ApplicationScoped
+@Identifier(ConnectionType.BIGQUERY_FACTORY_IDENTIFIER)
+public class BigQueryMetastoreFederatedCatalogFactory implements ExternalCatalogFactory {
+
+  @Override
+  public Catalog createCatalog(
+      ConnectionConfigInfoDpo connectionConfigInfoDpo,
+      PolarisCredentialManager polarisCredentialManager,
+      Map<String, String> catalogProperties) {
+
+    // Currently, Polaris supports BigQuery Metastore federation only via IMPLICIT authentication.
+    // Hence, prior to initializing the configuration, ensure that the catalog uses
+    // IMPLICIT authentication.
+    AuthenticationParametersDpo authenticationParametersDpo =
+        connectionConfigInfoDpo.getAuthenticationParameters();
+    if (authenticationParametersDpo.getAuthenticationTypeCode()

Review Comment:
   Note: IIRC, Hive federation is not even bundled by default: https://github.com/apache/polaris/blob/b67e1a594f740c580fc95b9d7a6cb78810bf6aeb/runtime/server/build.gradle.kts#L42-L43



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on PR #4050:
URL: https://github.com/apache/polaris/pull/4050#issuecomment-4283048960

   Thanks  @joyhaldar for the change! It's very useful feature. Thanks everyone for the review. 
   
   > I believe there is an option to re-run failed CI jobs without closing/reopening the PR ๐Ÿค”
   
   @dimas-b, got it. Mind sharing the option?
   


-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3020458241


##########
polaris-core/src/main/java/org/apache/polaris/core/connection/bigquery/BigQueryMetastoreConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,230 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.core.connection.bigquery;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.MoreObjects;
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+import org.apache.polaris.core.credentials.connection.ConnectionCredentials;
+import org.apache.polaris.core.identity.dpo.ServiceIdentityInfoDpo;
+import org.apache.polaris.core.identity.provider.ServiceIdentityProvider;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo} defined in the API
+ * model.
+ */
+public class BigQueryMetastoreConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+  public static final String DEFAULT_URI = "https://bigquery.googleapis.com";
+
+  // BigQuery Metastore catalog property keys
+  private static final String GCP_BIGQUERY_PROJECT_ID = "gcp.bigquery.project-id";
+  private static final String GCP_BIGQUERY_LOCATION = "gcp.bigquery.location";
+  private static final String GCP_BIGQUERY_LIST_ALL_TABLES = "gcp.bigquery.list-all-tables";
+  private static final String IMPERSONATE_SERVICE_ACCOUNT =
+      "gcp.bigquery.impersonate.service-account";
+  private static final String IMPERSONATE_LIFETIME_SECONDS =
+      "gcp.bigquery.impersonate.lifetime-seconds";
+  private static final String IMPERSONATE_SCOPES = "gcp.bigquery.impersonate.scopes";
+  private static final String IMPERSONATE_DELEGATES = "gcp.bigquery.impersonate.delegates";
+
+  private final String warehouse;
+  private final String gcpProjectId;
+  private final String gcpLocation;
+  private final Boolean listAllTables;
+  private final String impersonateServiceAccount;
+  private final Integer impersonateLifetimeSeconds;
+  private final List<String> impersonateScopes;
+  private final List<String> impersonateDelegates;
+
+  public BigQueryMetastoreConnectionConfigInfoDpo(
+      @JsonProperty(value = "uri", required = true) @Nonnull String uri,
+      @JsonProperty(value = "authenticationParameters", required = false) @Nullable
+          AuthenticationParametersDpo authenticationParameters,
+      @JsonProperty(value = "warehouse", required = true) @Nonnull String warehouse,
+      @JsonProperty(value = "gcpProjectId", required = true) @Nonnull String gcpProjectId,
+      @JsonProperty(value = "gcpLocation", required = false) @Nullable String gcpLocation,
+      @JsonProperty(value = "listAllTables", required = false) @Nullable Boolean listAllTables,
+      @JsonProperty(value = "impersonateServiceAccount", required = false) @Nullable
+          String impersonateServiceAccount,
+      @JsonProperty(value = "impersonateLifetimeSeconds", required = false) @Nullable
+          Integer impersonateLifetimeSeconds,
+      @JsonProperty(value = "impersonateScopes", required = false) @Nullable
+          List<String> impersonateScopes,
+      @JsonProperty(value = "impersonateDelegates", required = false) @Nullable
+          List<String> impersonateDelegates,
+      @JsonProperty(value = "serviceIdentity", required = false) @Nullable
+          ServiceIdentityInfoDpo serviceIdentity) {
+    super(ConnectionType.BIGQUERY.getCode(), uri, authenticationParameters, serviceIdentity);
+    this.warehouse = warehouse;
+    this.gcpProjectId = gcpProjectId;
+    this.gcpLocation = gcpLocation;
+    this.listAllTables = listAllTables;
+    this.impersonateServiceAccount = impersonateServiceAccount;
+    this.impersonateLifetimeSeconds = impersonateLifetimeSeconds;
+    this.impersonateScopes = impersonateScopes;
+    this.impersonateDelegates = impersonateDelegates;
+  }
+
+  public String getWarehouse() {
+    return warehouse;
+  }
+
+  public String getGcpProjectId() {
+    return gcpProjectId;
+  }
+
+  public String getGcpLocation() {
+    return gcpLocation;
+  }
+
+  public Boolean getListAllTables() {
+    return listAllTables;
+  }
+
+  public String getImpersonateServiceAccount() {
+    return impersonateServiceAccount;
+  }
+
+  public Integer getImpersonateLifetimeSeconds() {
+    return impersonateLifetimeSeconds;
+  }
+
+  public List<String> getImpersonateScopes() {
+    return impersonateScopes;
+  }
+
+  public List<String> getImpersonateDelegates() {
+    return impersonateDelegates;
+  }
+
+  @Override
+  public String toString() {
+    return MoreObjects.toStringHelper(this)
+        .add("connectionTypeCode", getConnectionTypeCode())
+        .add("uri", getUri())
+        .add("warehouse", warehouse)
+        .add("gcpProjectId", gcpProjectId)
+        .add("gcpLocation", gcpLocation)
+        .add("listAllTables", listAllTables)
+        .add("authenticationParameters", getAuthenticationParameters())

Review Comment:
   I know I had a comment in this `toString()` method, and you addressed it. Thanks a lot.
   
   Sorry to be a pain, but I have a follow up question ๐Ÿ˜„ 
   
   Here we directly call `getAuthenticationParameters()`. `authenticationParameters` is `@Nullable`.
   It's similar to `HiveConnectionConfigInfoDpo` which has the same nullable pattern.
   If other DPOs call `toString()` on the auth params, this could throw NPE.
   
   The existing Hadoop DPO has `authenticationParameters` as `@Nonnull` and calls `toString()` explicitly. 
   
   Maybe we should do something similar here to prevent NPE ?
   
   We can do something simple:
   
   ```suggestion
     .add("authenticationParameters",
         getAuthenticationParameters() != null ? getAuthenticationParameters().toString() : "null")  
   ```



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "nandorKollar (via GitHub)" <gi...@apache.org>.
nandorKollar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r2984436088


##########
runtime/server/build.gradle.kts:
##########
@@ -43,6 +43,10 @@ dependencies {
     runtimeOnly(project(":polaris-extensions-federation-hive"))
   }
 
+  if ((project.findProperty("NonRESTCatalogs") as String?)?.contains("BIGQUERY") == true) {
+    runtimeOnly(project(":polaris-extensions-federation-bigquery"))

Review Comment:
   I think - unlike with Hadoop and Hive catalog - this doesn't bring in any new direct dependency, which we have concern with. We should be good to include this extension in the standard distribution no?



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3040722087


##########
spec/polaris-management-service.yml:
##########
@@ -965,6 +967,27 @@ components:
         warehouse:
           type: string
           description: The warehouse location for the hive catalog.
+    
+    BigQueryMetastoreConnectionConfigInfo:
+      type: object
+      description: Configuration necessary for connecting to a BigQuery Metastore Catalog
+      allOf:
+        - $ref: '#/components/schemas/ConnectionConfigInfo'
+      properties:
+        warehouse:
+          type: string
+          description: The warehouse location for the BigQuery Metastore Catalog
+        gcpProjectId:
+          type: string
+          description: The GCP project ID
+        properties:
+          type: object
+          additionalProperties:
+            type: string
+            description: Additional catalog properties

Review Comment:
   Thanks for adding this. Thinking more broadly, this `properties` would be useful for other federated catalog type like HMS. I'd proposed to move it up to its parent `ConnectionConfigInfo`, so that all subtypes can benefit from it. WDYT? cc @XJDKC @HonahX @singhpk234 @dimas-b 



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r2982796252


##########
extensions/federation/bigquery/src/main/java/org/apache/polaris/extensions/federation/bigquery/BigQueryMetastoreFederatedCatalogFactory.java:
##########
@@ -0,0 +1,79 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.extensions.federation.bigquery;
+
+import io.smallrye.common.annotation.Identifier;
+import jakarta.enterprise.context.ApplicationScoped;
+import java.util.Map;
+import org.apache.iceberg.catalog.Catalog;
+import org.apache.iceberg.gcp.bigquery.BigQueryMetastoreCatalog;
+import org.apache.iceberg.rest.RESTUtil;
+import org.apache.polaris.core.catalog.ExternalCatalogFactory;
+import org.apache.polaris.core.catalog.GenericTableCatalog;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.AuthenticationType;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.connection.bigquery.BigQueryMetastoreConnectionConfigInfoDpo;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+
+/** Factory class for creating a BigQuery Metastore catalog handle based on connection configuration. */
+@ApplicationScoped
+@Identifier(ConnectionType.BIGQUERY_FACTORY_IDENTIFIER)
+public class BigQueryMetastoreFederatedCatalogFactory implements ExternalCatalogFactory {
+
+  @Override
+  public Catalog createCatalog(
+      ConnectionConfigInfoDpo connectionConfigInfoDpo,
+      PolarisCredentialManager polarisCredentialManager,
+      Map<String, String> catalogProperties) {
+
+    // Currently, Polaris supports BigQuery Metastore federation only via IMPLICIT authentication.
+    // Hence, prior to initializing the configuration, ensure that the catalog uses
+    // IMPLICIT authentication.
+    AuthenticationParametersDpo authenticationParametersDpo =
+        connectionConfigInfoDpo.getAuthenticationParameters();
+    if (authenticationParametersDpo.getAuthenticationTypeCode()
+        != AuthenticationType.IMPLICIT.getCode()) {
+      throw new IllegalStateException(
+          "BigQuery Metastore federation only supports IMPLICIT authentication.");
+    }
+
+    String warehouse =
+        ((BigQueryMetastoreConnectionConfigInfoDpo) connectionConfigInfoDpo).getWarehouse();
+    Map<String, String> mergedProperties =
+        RESTUtil.merge(
+            catalogProperties != null ? catalogProperties : Map.of(),
+            connectionConfigInfoDpo.asIcebergCatalogProperties(polarisCredentialManager));
+
+    BigQueryMetastoreCatalog bigQueryMetastoreCatalog = new BigQueryMetastoreCatalog();
+    bigQueryMetastoreCatalog.initialize(warehouse, mergedProperties);

Review Comment:
   How / where does it get BigQuery connection credentials?



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r2985546207


##########
extensions/federation/bigquery/src/main/java/org/apache/polaris/extensions/federation/bigquery/BigQueryMetastoreFederatedCatalogFactory.java:
##########
@@ -0,0 +1,79 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.extensions.federation.bigquery;
+
+import io.smallrye.common.annotation.Identifier;
+import jakarta.enterprise.context.ApplicationScoped;
+import java.util.Map;
+import org.apache.iceberg.catalog.Catalog;
+import org.apache.iceberg.gcp.bigquery.BigQueryMetastoreCatalog;
+import org.apache.iceberg.rest.RESTUtil;
+import org.apache.polaris.core.catalog.ExternalCatalogFactory;
+import org.apache.polaris.core.catalog.GenericTableCatalog;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.AuthenticationType;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.connection.bigquery.BigQueryMetastoreConnectionConfigInfoDpo;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+
+/** Factory class for creating a BigQuery Metastore catalog handle based on connection configuration. */
+@ApplicationScoped
+@Identifier(ConnectionType.BIGQUERY_FACTORY_IDENTIFIER)
+public class BigQueryMetastoreFederatedCatalogFactory implements ExternalCatalogFactory {
+
+  @Override
+  public Catalog createCatalog(
+      ConnectionConfigInfoDpo connectionConfigInfoDpo,
+      PolarisCredentialManager polarisCredentialManager,
+      Map<String, String> catalogProperties) {
+
+    // Currently, Polaris supports BigQuery Metastore federation only via IMPLICIT authentication.
+    // Hence, prior to initializing the configuration, ensure that the catalog uses
+    // IMPLICIT authentication.
+    AuthenticationParametersDpo authenticationParametersDpo =
+        connectionConfigInfoDpo.getAuthenticationParameters();
+    if (authenticationParametersDpo.getAuthenticationTypeCode()
+        != AuthenticationType.IMPLICIT.getCode()) {
+      throw new IllegalStateException(
+          "BigQuery Metastore federation only supports IMPLICIT authentication.");
+    }
+
+    String warehouse =
+        ((BigQueryMetastoreConnectionConfigInfoDpo) connectionConfigInfoDpo).getWarehouse();
+    Map<String, String> mergedProperties =
+        RESTUtil.merge(
+            catalogProperties != null ? catalogProperties : Map.of(),
+            connectionConfigInfoDpo.asIcebergCatalogProperties(polarisCredentialManager));
+
+    BigQueryMetastoreCatalog bigQueryMetastoreCatalog = new BigQueryMetastoreCatalog();
+    bigQueryMetastoreCatalog.initialize(warehouse, mergedProperties);

Review Comment:
   Thank you for your review Dmitri.
   
   BigQuery Metastore uses Application Default Credentials via Iceberg's [BigQueryProperties](https://github.com/apache/iceberg/blob/main/bigquery/src/main/java/org/apache/iceberg/gcp/bigquery/BigQueryProperties.java#L146-L175).
   
   Credentials are resolved automatically from the environment, `GOOGLE_APPLICATION_CREDENTIALS`, `gcloud auth`, or attached Service Account.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3015293979


##########
spec/polaris-management-service.yml:
##########
@@ -965,6 +967,44 @@ components:
         warehouse:
           type: string
           description: The warehouse location for the hive catalog.
+    
+    BigQueryMetastoreConnectionConfigInfo:
+      type: object
+      description: Configuration necessary for connecting to a BigQuery Metastore Catalog
+      allOf:
+        - $ref: '#/components/schemas/ConnectionConfigInfo'
+      properties:
+        warehouse:
+          type: string
+          description: The GCS bucket path for the catalog (e.g., gs://bucket-name)
+        gcpProjectId:
+          type: string
+          description: The GCP project ID
+        gcpLocation:
+          type: string
+          description: The GCP location (default = us)
+        listAllTables:
+          type: boolean
+          description: Whether to list all tables (default = true)
+        impersonateServiceAccount:
+          type: string
+          description: Service account email to impersonate
+        impersonateLifetimeSeconds:
+          type: integer
+          description: Token lifetime in seconds for impersonation (default = 3600)
+        impersonateScopes:
+          type: array
+          items:
+            type: string
+          description: OAuth scopes for impersonation (default = cloud-platform)
+        impersonateDelegates:
+          type: array
+          items:
+            type: string
+          description: Delegation chain for impersonation
+      required:

Review Comment:
   Thank you for the feedback. I have added null checks in both `BigQueryMetastoreFederatedCatalogFactory` and `asConnectionConfigInfoModel`. If `authenticationParameters` is null, BigQuery uses ADC by default. Please let me know if you'd like me to change the approach. Sorry for the late reply.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3087270417


##########
polaris-core/src/main/java/org/apache/polaris/core/connection/bigquery/BigQueryMetastoreConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.core.connection.bigquery;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.MoreObjects;
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+import org.apache.polaris.core.credentials.connection.ConnectionCredentials;
+import org.apache.polaris.core.identity.dpo.ServiceIdentityInfoDpo;
+import org.apache.polaris.core.identity.provider.ServiceIdentityProvider;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo} defined in the API
+ * model.
+ */
+public class BigQueryMetastoreConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+  public static final String DEFAULT_URI = "https://bigquery.googleapis.com";
+
+  // BigQuery Metastore catalog property keys
+  private static final String GCP_BIGQUERY_PROJECT_ID = "gcp.bigquery.project-id";
+
+  private final String warehouse;
+  private final String gcpProjectId;
+
+  public BigQueryMetastoreConnectionConfigInfoDpo(
+      @JsonProperty(value = "uri", required = true) @Nonnull String uri,
+      @JsonProperty(value = "authenticationParameters", required = false) @Nullable
+          AuthenticationParametersDpo authenticationParameters,
+      @JsonProperty(value = "warehouse", required = true) @Nonnull String warehouse,
+      @JsonProperty(value = "gcpProjectId", required = true) @Nonnull String gcpProjectId,
+      @JsonProperty(value = "properties", required = false) @Nullable
+          Map<String, String> properties,
+      @JsonProperty(value = "serviceIdentity", required = false) @Nullable
+          ServiceIdentityInfoDpo serviceIdentity) {
+    super(
+        ConnectionType.BIGQUERY.getCode(),
+        uri,
+        authenticationParameters,
+        serviceIdentity,
+        properties);
+    this.warehouse = warehouse;
+    this.gcpProjectId = gcpProjectId;
+  }
+
+  public String getWarehouse() {
+    return warehouse;
+  }
+
+  public String getGcpProjectId() {
+    return gcpProjectId;
+  }
+
+  @Override
+  public String toString() {
+    return MoreObjects.toStringHelper(this)
+        .add("connectionTypeCode", getConnectionTypeCode())
+        .add("uri", getUri())
+        .add("warehouse", warehouse)
+        .add("gcpProjectId", gcpProjectId)
+        .add(
+            "authenticationParameters",
+            getAuthenticationParameters() != null
+                ? getAuthenticationParameters().toString()
+                : "null")
+        .toString();
+  }
+
+  @Override
+  public @Nonnull Map<String, String> asIcebergCatalogProperties(
+      PolarisCredentialManager polarisCredentialManager) {
+    HashMap<String, String> properties = new HashMap<>();
+
+    // Required properties for BigQueryMetastoreCatalog
+    properties.put(GCP_BIGQUERY_PROJECT_ID, gcpProjectId);
+    properties.put(CatalogProperties.WAREHOUSE_LOCATION, warehouse);

Review Comment:
   From my POV this creates a skew in the Polaris API, which becomes dependent on particular Iceberg java classes.
   
   Conceptually, this class merely passes some user-defined properties to the Iceberg catalog impl. This is fine. However, do we have to elevate a sub-set of those properties to explicit Polaris OpenAPI properties?.. I'm not sure.
   
   I believe keeping the Polaris API generic and uniform for all federated catalog properties might be preferable.
   
   Implementations (both Iceberg java class and Polaris classes) can and should perform config validation as appropriate, of course.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3090924588


##########
extensions/federation/bigquery/src/main/java/org/apache/polaris/extensions/federation/bigquery/BigQueryMetastoreFederatedCatalogFactory.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.extensions.federation.bigquery;
+
+import io.smallrye.common.annotation.Identifier;
+import jakarta.enterprise.context.ApplicationScoped;
+import java.util.Map;
+import org.apache.iceberg.catalog.Catalog;
+import org.apache.iceberg.gcp.bigquery.BigQueryMetastoreCatalog;
+import org.apache.iceberg.rest.RESTUtil;
+import org.apache.polaris.core.catalog.ExternalCatalogFactory;
+import org.apache.polaris.core.catalog.GenericTableCatalog;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.AuthenticationType;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.connection.bigquery.BigQueryMetastoreConnectionConfigInfoDpo;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+
+/**
+ * Factory class for creating a BigQuery Metastore catalog handle based on connection configuration.
+ */
+@ApplicationScoped
+@Identifier(ConnectionType.BIGQUERY_FACTORY_IDENTIFIER)
+public class BigQueryMetastoreFederatedCatalogFactory implements ExternalCatalogFactory {
+
+  @Override
+  public Catalog createCatalog(
+      ConnectionConfigInfoDpo connectionConfigInfoDpo,
+      PolarisCredentialManager polarisCredentialManager,
+      Map<String, String> catalogProperties) {
+
+    // Currently, Polaris supports BigQuery Metastore federation only via IMPLICIT authentication.
+    // Hence, prior to initializing the configuration, ensure that the catalog uses
+    // IMPLICIT authentication.
+    AuthenticationParametersDpo authenticationParametersDpo =
+        connectionConfigInfoDpo.getAuthenticationParameters();
+    if (authenticationParametersDpo != null

Review Comment:
   No, I am sorry, I missed it.
   
   I tried adding the defaulting in `fromConnectionConfigInfoModelWithSecrets()` but it doesn't work, `PolarisServiceImpl.validateAuthenticationParameters()` runs before conversion and throws NPE when `authenticationParameters` is null IIUC.
   
   Fixing that would mean changing `PolarisServiceImpl too`, which seems out of scope for this PR.
   For now I'm going with @dimas-b's suggestion, users must pass `authenticationParameters: {authenticationType: IMPLICIT}` explicitly ([commit](https://github.com/apache/polaris/pull/4050/changes/3856a7d5a0d5f3ccd8406037614013615c5a67ce)).
   
   Let me know if my understanding is correct. Would appreciate your guidance.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r2992444216


##########
extensions/federation/bigquery/src/main/java/org/apache/polaris/extensions/federation/bigquery/BigQueryMetastoreFederatedCatalogFactory.java:
##########
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.extensions.federation.bigquery;
+
+import io.smallrye.common.annotation.Identifier;
+import jakarta.enterprise.context.ApplicationScoped;
+import java.util.Map;
+import org.apache.iceberg.catalog.Catalog;
+import org.apache.iceberg.gcp.bigquery.BigQueryMetastoreCatalog;
+import org.apache.iceberg.rest.RESTUtil;
+import org.apache.polaris.core.catalog.ExternalCatalogFactory;
+import org.apache.polaris.core.catalog.GenericTableCatalog;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.AuthenticationType;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.connection.bigquery.BigQueryMetastoreConnectionConfigInfoDpo;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+
+/**
+ * Factory class for creating a BigQuery Metastore catalog handle based on connection configuration.
+ */
+@ApplicationScoped
+@Identifier(ConnectionType.BIGQUERY_FACTORY_IDENTIFIER)
+public class BigQueryMetastoreFederatedCatalogFactory implements ExternalCatalogFactory {
+
+  @Override
+  public Catalog createCatalog(
+      ConnectionConfigInfoDpo connectionConfigInfoDpo,
+      PolarisCredentialManager polarisCredentialManager,
+      Map<String, String> catalogProperties) {
+
+    // Currently, Polaris supports BigQuery Metastore federation only via IMPLICIT authentication.
+    // Hence, prior to initializing the configuration, ensure that the catalog uses
+    // IMPLICIT authentication.
+    AuthenticationParametersDpo authenticationParametersDpo =
+        connectionConfigInfoDpo.getAuthenticationParameters();
+    if (authenticationParametersDpo.getAuthenticationTypeCode()

Review Comment:
   Should we include a `null` check here ?
   
   If `connectionConfigInfoDpo.getAuthenticationParameters()` returns `null`, then `authenticationParametersDpo.getAuthenticationTypeCode()` will throw NPE.
   
   Since the constructor marks `authenticationParameters` as `required = false` I guess it can happen.



##########
polaris-core/src/main/java/org/apache/polaris/core/connection/bigquery/BigQueryMetastoreConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,230 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.core.connection.bigquery;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.MoreObjects;
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+import org.apache.polaris.core.credentials.connection.ConnectionCredentials;
+import org.apache.polaris.core.identity.dpo.ServiceIdentityInfoDpo;
+import org.apache.polaris.core.identity.provider.ServiceIdentityProvider;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo} defined in the API
+ * model.
+ */
+public class BigQueryMetastoreConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+  // BigQuery Metastore catalog property keys
+  private static final String GCP_BIGQUERY_PROJECT_ID = "gcp.bigquery.project-id";
+  private static final String GCP_BIGQUERY_LOCATION = "gcp.bigquery.location";
+  private static final String GCP_BIGQUERY_LIST_ALL_TABLES = "gcp.bigquery.list-all-tables";
+  private static final String IMPERSONATE_SERVICE_ACCOUNT =
+      "gcp.bigquery.impersonate.service-account";
+  private static final String IMPERSONATE_LIFETIME_SECONDS =
+      "gcp.bigquery.impersonate.lifetime-seconds";
+  private static final String IMPERSONATE_SCOPES = "gcp.bigquery.impersonate.scopes";
+  private static final String IMPERSONATE_DELEGATES = "gcp.bigquery.impersonate.delegates";
+
+  private final String warehouse;
+  private final String gcpProjectId;
+  private final String gcpLocation;
+  private final Boolean listAllTables;
+  private final String impersonateServiceAccount;
+  private final Integer impersonateLifetimeSeconds;
+  private final List<String> impersonateScopes;
+  private final List<String> impersonateDelegates;
+
+  public BigQueryMetastoreConnectionConfigInfoDpo(
+      @JsonProperty(value = "uri", required = true) @Nonnull String uri,
+      @JsonProperty(value = "authenticationParameters", required = false) @Nullable
+          AuthenticationParametersDpo authenticationParameters,
+      @JsonProperty(value = "warehouse", required = true) @Nonnull String warehouse,
+      @JsonProperty(value = "gcpProjectId", required = true) @Nonnull String gcpProjectId,
+      @JsonProperty(value = "gcpLocation", required = false) @Nullable String gcpLocation,
+      @JsonProperty(value = "listAllTables", required = false) @Nullable Boolean listAllTables,
+      @JsonProperty(value = "impersonateServiceAccount", required = false) @Nullable
+          String impersonateServiceAccount,
+      @JsonProperty(value = "impersonateLifetimeSeconds", required = false) @Nullable
+          Integer impersonateLifetimeSeconds,
+      @JsonProperty(value = "impersonateScopes", required = false) @Nullable
+          List<String> impersonateScopes,
+      @JsonProperty(value = "impersonateDelegates", required = false) @Nullable
+          List<String> impersonateDelegates,
+      @JsonProperty(value = "serviceIdentity", required = false) @Nullable
+          ServiceIdentityInfoDpo serviceIdentity) {
+    super(ConnectionType.BIGQUERY.getCode(), uri, authenticationParameters, serviceIdentity);
+    this.warehouse = warehouse;
+    this.gcpProjectId = gcpProjectId;
+    this.gcpLocation = gcpLocation;
+    this.listAllTables = listAllTables;
+    this.impersonateServiceAccount = impersonateServiceAccount;
+    this.impersonateLifetimeSeconds = impersonateLifetimeSeconds;
+    this.impersonateScopes = impersonateScopes;
+    this.impersonateDelegates = impersonateDelegates;
+  }
+
+  public String getWarehouse() {
+    return warehouse;
+  }
+
+  public String getGcpProjectId() {
+    return gcpProjectId;
+  }
+
+  public String getGcpLocation() {
+    return gcpLocation;
+  }
+
+  public Boolean getListAllTables() {
+    return listAllTables;
+  }
+
+  public String getImpersonateServiceAccount() {
+    return impersonateServiceAccount;
+  }
+
+  public Integer getImpersonateLifetimeSeconds() {
+    return impersonateLifetimeSeconds;
+  }
+
+  public List<String> getImpersonateScopes() {
+    return impersonateScopes;
+  }
+
+  public List<String> getImpersonateDelegates() {
+    return impersonateDelegates;
+  }
+
+  @Override
+  public String toString() {
+    return MoreObjects.toStringHelper(this)
+        .add("connectionTypeCode", getConnectionTypeCode())
+        .add("uri", getUri())
+        .add("warehouse", warehouse)
+        .add("gcpProjectId", gcpProjectId)
+        .add("gcpLocation", gcpLocation)
+        .add("listAllTables", listAllTables)
+        .add("impersonateServiceAccount", impersonateServiceAccount)

Review Comment:
   Is it ok to include `impersonateServiceAccount` for `toString()` ?
   It could include sensitive data I guess, so it could potentially land in log messages.
   Maybe we should remove the account details or offuscate ?



##########
polaris-core/src/main/java/org/apache/polaris/core/connection/ConnectionConfigInfoDpo.java:
##########
@@ -210,6 +213,30 @@ public static ConnectionConfigInfoDpo fromConnectionConfigInfoModelWithSecrets(
                 hiveConfigModel.getWarehouse(),
                 null /*Service Identity Info*/);
         break;
+      case BIGQUERY:
+        BigQueryMetastoreConnectionConfigInfo bigqueryConfigModel =
+            (BigQueryMetastoreConnectionConfigInfo) connectionConfigurationModel;
+        authenticationParameters =
+            AuthenticationParametersDpo.fromAuthenticationParametersModelWithSecrets(
+                bigqueryConfigModel.getAuthenticationParameters(), secretReferences);
+        String bigqueryUri =
+            bigqueryConfigModel.getUri() != null

Review Comment:
   nit: If URI is `null`, we default to `https://bigquery.googleapis.com`. Maybe this default should live in DPO for visibility (or maybe OpenAPI as you started a discussion to update it ๐Ÿ˜„ ).



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r2992615984


##########
polaris-core/src/main/java/org/apache/polaris/core/connection/ConnectionConfigInfoDpo.java:
##########
@@ -210,6 +213,30 @@ public static ConnectionConfigInfoDpo fromConnectionConfigInfoModelWithSecrets(
                 hiveConfigModel.getWarehouse(),
                 null /*Service Identity Info*/);
         break;
+      case BIGQUERY:
+        BigQueryMetastoreConnectionConfigInfo bigqueryConfigModel =
+            (BigQueryMetastoreConnectionConfigInfo) connectionConfigurationModel;
+        authenticationParameters =
+            AuthenticationParametersDpo.fromAuthenticationParametersModelWithSecrets(
+                bigqueryConfigModel.getAuthenticationParameters(), secretReferences);
+        String bigqueryUri =
+            bigqueryConfigModel.getUri() != null

Review Comment:
   Done. I have moved the default URI to a public constant in `BigQueryMetastoreConnectionConfigInfoDpo`.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r2992625114


##########
polaris-core/src/main/java/org/apache/polaris/core/connection/bigquery/BigQueryMetastoreConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,230 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.core.connection.bigquery;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.MoreObjects;
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+import org.apache.polaris.core.credentials.connection.ConnectionCredentials;
+import org.apache.polaris.core.identity.dpo.ServiceIdentityInfoDpo;
+import org.apache.polaris.core.identity.provider.ServiceIdentityProvider;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo} defined in the API
+ * model.
+ */
+public class BigQueryMetastoreConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+  // BigQuery Metastore catalog property keys
+  private static final String GCP_BIGQUERY_PROJECT_ID = "gcp.bigquery.project-id";
+  private static final String GCP_BIGQUERY_LOCATION = "gcp.bigquery.location";
+  private static final String GCP_BIGQUERY_LIST_ALL_TABLES = "gcp.bigquery.list-all-tables";
+  private static final String IMPERSONATE_SERVICE_ACCOUNT =
+      "gcp.bigquery.impersonate.service-account";
+  private static final String IMPERSONATE_LIFETIME_SECONDS =
+      "gcp.bigquery.impersonate.lifetime-seconds";
+  private static final String IMPERSONATE_SCOPES = "gcp.bigquery.impersonate.scopes";
+  private static final String IMPERSONATE_DELEGATES = "gcp.bigquery.impersonate.delegates";
+
+  private final String warehouse;
+  private final String gcpProjectId;
+  private final String gcpLocation;
+  private final Boolean listAllTables;
+  private final String impersonateServiceAccount;
+  private final Integer impersonateLifetimeSeconds;
+  private final List<String> impersonateScopes;
+  private final List<String> impersonateDelegates;
+
+  public BigQueryMetastoreConnectionConfigInfoDpo(
+      @JsonProperty(value = "uri", required = true) @Nonnull String uri,
+      @JsonProperty(value = "authenticationParameters", required = false) @Nullable
+          AuthenticationParametersDpo authenticationParameters,
+      @JsonProperty(value = "warehouse", required = true) @Nonnull String warehouse,
+      @JsonProperty(value = "gcpProjectId", required = true) @Nonnull String gcpProjectId,
+      @JsonProperty(value = "gcpLocation", required = false) @Nullable String gcpLocation,
+      @JsonProperty(value = "listAllTables", required = false) @Nullable Boolean listAllTables,
+      @JsonProperty(value = "impersonateServiceAccount", required = false) @Nullable
+          String impersonateServiceAccount,
+      @JsonProperty(value = "impersonateLifetimeSeconds", required = false) @Nullable
+          Integer impersonateLifetimeSeconds,
+      @JsonProperty(value = "impersonateScopes", required = false) @Nullable
+          List<String> impersonateScopes,
+      @JsonProperty(value = "impersonateDelegates", required = false) @Nullable
+          List<String> impersonateDelegates,
+      @JsonProperty(value = "serviceIdentity", required = false) @Nullable
+          ServiceIdentityInfoDpo serviceIdentity) {
+    super(ConnectionType.BIGQUERY.getCode(), uri, authenticationParameters, serviceIdentity);
+    this.warehouse = warehouse;
+    this.gcpProjectId = gcpProjectId;
+    this.gcpLocation = gcpLocation;
+    this.listAllTables = listAllTables;
+    this.impersonateServiceAccount = impersonateServiceAccount;
+    this.impersonateLifetimeSeconds = impersonateLifetimeSeconds;
+    this.impersonateScopes = impersonateScopes;
+    this.impersonateDelegates = impersonateDelegates;
+  }
+
+  public String getWarehouse() {
+    return warehouse;
+  }
+
+  public String getGcpProjectId() {
+    return gcpProjectId;
+  }
+
+  public String getGcpLocation() {
+    return gcpLocation;
+  }
+
+  public Boolean getListAllTables() {
+    return listAllTables;
+  }
+
+  public String getImpersonateServiceAccount() {
+    return impersonateServiceAccount;
+  }
+
+  public Integer getImpersonateLifetimeSeconds() {
+    return impersonateLifetimeSeconds;
+  }
+
+  public List<String> getImpersonateScopes() {
+    return impersonateScopes;
+  }
+
+  public List<String> getImpersonateDelegates() {
+    return impersonateDelegates;
+  }
+
+  @Override
+  public String toString() {
+    return MoreObjects.toStringHelper(this)
+        .add("connectionTypeCode", getConnectionTypeCode())
+        .add("uri", getUri())
+        .add("warehouse", warehouse)
+        .add("gcpProjectId", gcpProjectId)
+        .add("gcpLocation", gcpLocation)
+        .add("listAllTables", listAllTables)
+        .add("impersonateServiceAccount", impersonateServiceAccount)

Review Comment:
   Thank you for your review JB.
   
   Done. I have removed **all impersonation fields** from `toString()`.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r2995732243


##########
extensions/federation/bigquery/src/main/java/org/apache/polaris/extensions/federation/bigquery/BigQueryMetastoreFederatedCatalogFactory.java:
##########
@@ -0,0 +1,79 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.extensions.federation.bigquery;
+
+import io.smallrye.common.annotation.Identifier;
+import jakarta.enterprise.context.ApplicationScoped;
+import java.util.Map;
+import org.apache.iceberg.catalog.Catalog;
+import org.apache.iceberg.gcp.bigquery.BigQueryMetastoreCatalog;
+import org.apache.iceberg.rest.RESTUtil;
+import org.apache.polaris.core.catalog.ExternalCatalogFactory;
+import org.apache.polaris.core.catalog.GenericTableCatalog;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.AuthenticationType;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.connection.bigquery.BigQueryMetastoreConnectionConfigInfoDpo;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+
+/** Factory class for creating a BigQuery Metastore catalog handle based on connection configuration. */
+@ApplicationScoped
+@Identifier(ConnectionType.BIGQUERY_FACTORY_IDENTIFIER)
+public class BigQueryMetastoreFederatedCatalogFactory implements ExternalCatalogFactory {
+
+  @Override
+  public Catalog createCatalog(
+      ConnectionConfigInfoDpo connectionConfigInfoDpo,
+      PolarisCredentialManager polarisCredentialManager,
+      Map<String, String> catalogProperties) {
+
+    // Currently, Polaris supports BigQuery Metastore federation only via IMPLICIT authentication.
+    // Hence, prior to initializing the configuration, ensure that the catalog uses
+    // IMPLICIT authentication.
+    AuthenticationParametersDpo authenticationParametersDpo =
+        connectionConfigInfoDpo.getAuthenticationParameters();
+    if (authenticationParametersDpo.getAuthenticationTypeCode()
+        != AuthenticationType.IMPLICIT.getCode()) {
+      throw new IllegalStateException(
+          "BigQuery Metastore federation only supports IMPLICIT authentication.");
+    }
+
+    String warehouse =
+        ((BigQueryMetastoreConnectionConfigInfoDpo) connectionConfigInfoDpo).getWarehouse();
+    Map<String, String> mergedProperties =
+        RESTUtil.merge(
+            catalogProperties != null ? catalogProperties : Map.of(),
+            connectionConfigInfoDpo.asIcebergCatalogProperties(polarisCredentialManager));
+
+    BigQueryMetastoreCatalog bigQueryMetastoreCatalog = new BigQueryMetastoreCatalog();
+    bigQueryMetastoreCatalog.initialize(warehouse, mergedProperties);

Review Comment:
   So, if storage is also in GCP, the BigQuery connection and storage connections will have to use the same credentials, effectively (in current Polaris code).
   
   It's not a big deal ATM, I think... just trying to understand the full picture :)



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r2995769841


##########
extensions/federation/bigquery/src/main/java/org/apache/polaris/extensions/federation/bigquery/BigQueryMetastoreFederatedCatalogFactory.java:
##########
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.extensions.federation.bigquery;
+
+import io.smallrye.common.annotation.Identifier;
+import jakarta.enterprise.context.ApplicationScoped;
+import java.util.Map;
+import org.apache.iceberg.catalog.Catalog;
+import org.apache.iceberg.gcp.bigquery.BigQueryMetastoreCatalog;
+import org.apache.iceberg.rest.RESTUtil;
+import org.apache.polaris.core.catalog.ExternalCatalogFactory;
+import org.apache.polaris.core.catalog.GenericTableCatalog;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.AuthenticationType;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.connection.bigquery.BigQueryMetastoreConnectionConfigInfoDpo;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+
+/**
+ * Factory class for creating a BigQuery Metastore catalog handle based on connection configuration.
+ */
+@ApplicationScoped
+@Identifier(ConnectionType.BIGQUERY_FACTORY_IDENTIFIER)
+public class BigQueryMetastoreFederatedCatalogFactory implements ExternalCatalogFactory {
+
+  @Override
+  public Catalog createCatalog(
+      ConnectionConfigInfoDpo connectionConfigInfoDpo,
+      PolarisCredentialManager polarisCredentialManager,
+      Map<String, String> catalogProperties) {
+
+    // Currently, Polaris supports BigQuery Metastore federation only via IMPLICIT authentication.
+    // Hence, prior to initializing the configuration, ensure that the catalog uses
+    // IMPLICIT authentication.
+    AuthenticationParametersDpo authenticationParametersDpo =
+        connectionConfigInfoDpo.getAuthenticationParameters();
+    if (authenticationParametersDpo.getAuthenticationTypeCode()

Review Comment:
   I'd say let's make new code robust immediately and fix old code later.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3015286140


##########
extensions/federation/bigquery/src/main/java/org/apache/polaris/extensions/federation/bigquery/BigQueryMetastoreFederatedCatalogFactory.java:
##########
@@ -0,0 +1,79 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.extensions.federation.bigquery;
+
+import io.smallrye.common.annotation.Identifier;
+import jakarta.enterprise.context.ApplicationScoped;
+import java.util.Map;
+import org.apache.iceberg.catalog.Catalog;
+import org.apache.iceberg.gcp.bigquery.BigQueryMetastoreCatalog;
+import org.apache.iceberg.rest.RESTUtil;
+import org.apache.polaris.core.catalog.ExternalCatalogFactory;
+import org.apache.polaris.core.catalog.GenericTableCatalog;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.AuthenticationType;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.connection.bigquery.BigQueryMetastoreConnectionConfigInfoDpo;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+
+/** Factory class for creating a BigQuery Metastore catalog handle based on connection configuration. */
+@ApplicationScoped
+@Identifier(ConnectionType.BIGQUERY_FACTORY_IDENTIFIER)
+public class BigQueryMetastoreFederatedCatalogFactory implements ExternalCatalogFactory {
+
+  @Override
+  public Catalog createCatalog(
+      ConnectionConfigInfoDpo connectionConfigInfoDpo,
+      PolarisCredentialManager polarisCredentialManager,
+      Map<String, String> catalogProperties) {
+
+    // Currently, Polaris supports BigQuery Metastore federation only via IMPLICIT authentication.
+    // Hence, prior to initializing the configuration, ensure that the catalog uses
+    // IMPLICIT authentication.
+    AuthenticationParametersDpo authenticationParametersDpo =
+        connectionConfigInfoDpo.getAuthenticationParameters();
+    if (authenticationParametersDpo.getAuthenticationTypeCode()
+        != AuthenticationType.IMPLICIT.getCode()) {
+      throw new IllegalStateException(
+          "BigQuery Metastore federation only supports IMPLICIT authentication.");
+    }
+
+    String warehouse =
+        ((BigQueryMetastoreConnectionConfigInfoDpo) connectionConfigInfoDpo).getWarehouse();
+    Map<String, String> mergedProperties =
+        RESTUtil.merge(
+            catalogProperties != null ? catalogProperties : Map.of(),
+            connectionConfigInfoDpo.asIcebergCatalogProperties(polarisCredentialManager));
+
+    BigQueryMetastoreCatalog bigQueryMetastoreCatalog = new BigQueryMetastoreCatalog();
+    bigQueryMetastoreCatalog.initialize(warehouse, mergedProperties);

Review Comment:
   Yes, that's my understanding also. Both BigQuery Metastore and GCS storage will use the same ADC credentials (or impersonated SA if configured). Thank you for clarifying the full picture. Sorry for the late reply.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3014902671


##########
polaris-core/src/main/java/org/apache/polaris/core/connection/bigquery/BigQueryMetastoreConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,228 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.core.connection.bigquery;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.MoreObjects;
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+import org.apache.polaris.core.credentials.connection.ConnectionCredentials;
+import org.apache.polaris.core.identity.dpo.ServiceIdentityInfoDpo;
+import org.apache.polaris.core.identity.provider.ServiceIdentityProvider;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo} defined in the API
+ * model.
+ */
+public class BigQueryMetastoreConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+  public static final String DEFAULT_URI = "https://bigquery.googleapis.com";
+
+  // BigQuery Metastore catalog property keys
+  private static final String GCP_BIGQUERY_PROJECT_ID = "gcp.bigquery.project-id";
+  private static final String GCP_BIGQUERY_LOCATION = "gcp.bigquery.location";
+  private static final String GCP_BIGQUERY_LIST_ALL_TABLES = "gcp.bigquery.list-all-tables";
+  private static final String IMPERSONATE_SERVICE_ACCOUNT =
+      "gcp.bigquery.impersonate.service-account";
+  private static final String IMPERSONATE_LIFETIME_SECONDS =
+      "gcp.bigquery.impersonate.lifetime-seconds";
+  private static final String IMPERSONATE_SCOPES = "gcp.bigquery.impersonate.scopes";
+  private static final String IMPERSONATE_DELEGATES = "gcp.bigquery.impersonate.delegates";
+
+  private final String warehouse;
+  private final String gcpProjectId;
+  private final String gcpLocation;
+  private final Boolean listAllTables;
+  private final String impersonateServiceAccount;
+  private final Integer impersonateLifetimeSeconds;
+  private final List<String> impersonateScopes;
+  private final List<String> impersonateDelegates;
+
+  public BigQueryMetastoreConnectionConfigInfoDpo(
+      @JsonProperty(value = "uri", required = true) @Nonnull String uri,
+      @JsonProperty(value = "authenticationParameters", required = false) @Nullable
+          AuthenticationParametersDpo authenticationParameters,
+      @JsonProperty(value = "warehouse", required = true) @Nonnull String warehouse,
+      @JsonProperty(value = "gcpProjectId", required = true) @Nonnull String gcpProjectId,
+      @JsonProperty(value = "gcpLocation", required = false) @Nullable String gcpLocation,
+      @JsonProperty(value = "listAllTables", required = false) @Nullable Boolean listAllTables,
+      @JsonProperty(value = "impersonateServiceAccount", required = false) @Nullable
+          String impersonateServiceAccount,
+      @JsonProperty(value = "impersonateLifetimeSeconds", required = false) @Nullable
+          Integer impersonateLifetimeSeconds,
+      @JsonProperty(value = "impersonateScopes", required = false) @Nullable
+          List<String> impersonateScopes,
+      @JsonProperty(value = "impersonateDelegates", required = false) @Nullable
+          List<String> impersonateDelegates,
+      @JsonProperty(value = "serviceIdentity", required = false) @Nullable
+          ServiceIdentityInfoDpo serviceIdentity) {
+    super(ConnectionType.BIGQUERY.getCode(), uri, authenticationParameters, serviceIdentity);
+    this.warehouse = warehouse;
+    this.gcpProjectId = gcpProjectId;
+    this.gcpLocation = gcpLocation;
+    this.listAllTables = listAllTables;
+    this.impersonateServiceAccount = impersonateServiceAccount;
+    this.impersonateLifetimeSeconds = impersonateLifetimeSeconds;
+    this.impersonateScopes = impersonateScopes;
+    this.impersonateDelegates = impersonateDelegates;
+  }
+
+  public String getWarehouse() {
+    return warehouse;
+  }
+
+  public String getGcpProjectId() {
+    return gcpProjectId;
+  }
+
+  public String getGcpLocation() {
+    return gcpLocation;
+  }
+
+  public Boolean getListAllTables() {
+    return listAllTables;
+  }
+
+  public String getImpersonateServiceAccount() {
+    return impersonateServiceAccount;
+  }
+
+  public Integer getImpersonateLifetimeSeconds() {
+    return impersonateLifetimeSeconds;
+  }
+
+  public List<String> getImpersonateScopes() {
+    return impersonateScopes;
+  }
+
+  public List<String> getImpersonateDelegates() {
+    return impersonateDelegates;
+  }
+
+  @Override
+  public String toString() {
+    return MoreObjects.toStringHelper(this)
+        .add("connectionTypeCode", getConnectionTypeCode())
+        .add("uri", getUri())
+        .add("warehouse", warehouse)
+        .add("gcpProjectId", gcpProjectId)
+        .add("gcpLocation", gcpLocation)
+        .add("listAllTables", listAllTables)
+        .add("authenticationParameters", getAuthenticationParameters().toString())
+        .toString();
+  }
+
+  @Override
+  public @Nonnull Map<String, String> asIcebergCatalogProperties(
+      PolarisCredentialManager polarisCredentialManager) {
+    HashMap<String, String> properties = new HashMap<>();
+
+    // Required properties for BigQueryMetastoreCatalog
+    properties.put(GCP_BIGQUERY_PROJECT_ID, gcpProjectId);
+    properties.put(CatalogProperties.WAREHOUSE_LOCATION, warehouse);
+
+    // Optional properties
+    if (gcpLocation != null) {
+      properties.put(GCP_BIGQUERY_LOCATION, gcpLocation);
+    }
+    if (listAllTables != null) {
+      properties.put(GCP_BIGQUERY_LIST_ALL_TABLES, String.valueOf(listAllTables));
+    }
+
+    // Optional impersonation properties
+    if (impersonateServiceAccount != null) {
+      properties.put(IMPERSONATE_SERVICE_ACCOUNT, impersonateServiceAccount);
+    }
+    if (impersonateLifetimeSeconds != null) {
+      properties.put(IMPERSONATE_LIFETIME_SECONDS, String.valueOf(impersonateLifetimeSeconds));
+    }
+    if (impersonateScopes != null && !impersonateScopes.isEmpty()) {
+      properties.put(IMPERSONATE_SCOPES, String.join(",", impersonateScopes));
+    }
+    if (impersonateDelegates != null && !impersonateDelegates.isEmpty()) {
+      properties.put(IMPERSONATE_DELEGATES, String.join(",", impersonateDelegates));
+    }
+
+    if (getAuthenticationParameters() != null) {
+      // Add authentication-specific metadata (non-credential properties)
+      properties.putAll(
+          getAuthenticationParameters().asIcebergCatalogProperties(polarisCredentialManager));
+      // Add connection credentials from Polaris credential manager
+      ConnectionCredentials connectionCredentials =
+          polarisCredentialManager.getConnectionCredentials(this);
+      properties.putAll(connectionCredentials.credentials());
+    }
+    return properties;
+  }
+
+  @Override
+  public ConnectionConfigInfoDpo withServiceIdentity(
+      @Nonnull ServiceIdentityInfoDpo serviceIdentityInfo) {
+    return new BigQueryMetastoreConnectionConfigInfoDpo(
+        getUri(),
+        getAuthenticationParameters(),
+        warehouse,
+        gcpProjectId,
+        gcpLocation,
+        listAllTables,
+        impersonateServiceAccount,
+        impersonateLifetimeSeconds,
+        impersonateScopes,
+        impersonateDelegates,
+        serviceIdentityInfo);
+  }
+
+  @Override
+  public ConnectionConfigInfo asConnectionConfigInfoModel(
+      ServiceIdentityProvider serviceIdentityProvider) {
+    return BigQueryMetastoreConnectionConfigInfo.builder()
+        .setConnectionType(ConnectionConfigInfo.ConnectionTypeEnum.BIGQUERY)
+        .setUri(getUri())
+        .setWarehouse(warehouse)
+        .setGcpProjectId(gcpProjectId)
+        .setGcpLocation(gcpLocation)
+        .setListAllTables(listAllTables)
+        .setImpersonateServiceAccount(impersonateServiceAccount)
+        .setImpersonateLifetimeSeconds(impersonateLifetimeSeconds)
+        .setImpersonateScopes(impersonateScopes)
+        .setImpersonateDelegates(impersonateDelegates)
+        .setAuthenticationParameters(

Review Comment:
   Done. I have added null check in `asConnectionConfigInfoModel`.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3014902671


##########
polaris-core/src/main/java/org/apache/polaris/core/connection/bigquery/BigQueryMetastoreConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,228 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.core.connection.bigquery;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.MoreObjects;
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+import org.apache.polaris.core.credentials.connection.ConnectionCredentials;
+import org.apache.polaris.core.identity.dpo.ServiceIdentityInfoDpo;
+import org.apache.polaris.core.identity.provider.ServiceIdentityProvider;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo} defined in the API
+ * model.
+ */
+public class BigQueryMetastoreConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+  public static final String DEFAULT_URI = "https://bigquery.googleapis.com";
+
+  // BigQuery Metastore catalog property keys
+  private static final String GCP_BIGQUERY_PROJECT_ID = "gcp.bigquery.project-id";
+  private static final String GCP_BIGQUERY_LOCATION = "gcp.bigquery.location";
+  private static final String GCP_BIGQUERY_LIST_ALL_TABLES = "gcp.bigquery.list-all-tables";
+  private static final String IMPERSONATE_SERVICE_ACCOUNT =
+      "gcp.bigquery.impersonate.service-account";
+  private static final String IMPERSONATE_LIFETIME_SECONDS =
+      "gcp.bigquery.impersonate.lifetime-seconds";
+  private static final String IMPERSONATE_SCOPES = "gcp.bigquery.impersonate.scopes";
+  private static final String IMPERSONATE_DELEGATES = "gcp.bigquery.impersonate.delegates";
+
+  private final String warehouse;
+  private final String gcpProjectId;
+  private final String gcpLocation;
+  private final Boolean listAllTables;
+  private final String impersonateServiceAccount;
+  private final Integer impersonateLifetimeSeconds;
+  private final List<String> impersonateScopes;
+  private final List<String> impersonateDelegates;
+
+  public BigQueryMetastoreConnectionConfigInfoDpo(
+      @JsonProperty(value = "uri", required = true) @Nonnull String uri,
+      @JsonProperty(value = "authenticationParameters", required = false) @Nullable
+          AuthenticationParametersDpo authenticationParameters,
+      @JsonProperty(value = "warehouse", required = true) @Nonnull String warehouse,
+      @JsonProperty(value = "gcpProjectId", required = true) @Nonnull String gcpProjectId,
+      @JsonProperty(value = "gcpLocation", required = false) @Nullable String gcpLocation,
+      @JsonProperty(value = "listAllTables", required = false) @Nullable Boolean listAllTables,
+      @JsonProperty(value = "impersonateServiceAccount", required = false) @Nullable
+          String impersonateServiceAccount,
+      @JsonProperty(value = "impersonateLifetimeSeconds", required = false) @Nullable
+          Integer impersonateLifetimeSeconds,
+      @JsonProperty(value = "impersonateScopes", required = false) @Nullable
+          List<String> impersonateScopes,
+      @JsonProperty(value = "impersonateDelegates", required = false) @Nullable
+          List<String> impersonateDelegates,
+      @JsonProperty(value = "serviceIdentity", required = false) @Nullable
+          ServiceIdentityInfoDpo serviceIdentity) {
+    super(ConnectionType.BIGQUERY.getCode(), uri, authenticationParameters, serviceIdentity);
+    this.warehouse = warehouse;
+    this.gcpProjectId = gcpProjectId;
+    this.gcpLocation = gcpLocation;
+    this.listAllTables = listAllTables;
+    this.impersonateServiceAccount = impersonateServiceAccount;
+    this.impersonateLifetimeSeconds = impersonateLifetimeSeconds;
+    this.impersonateScopes = impersonateScopes;
+    this.impersonateDelegates = impersonateDelegates;
+  }
+
+  public String getWarehouse() {
+    return warehouse;
+  }
+
+  public String getGcpProjectId() {
+    return gcpProjectId;
+  }
+
+  public String getGcpLocation() {
+    return gcpLocation;
+  }
+
+  public Boolean getListAllTables() {
+    return listAllTables;
+  }
+
+  public String getImpersonateServiceAccount() {
+    return impersonateServiceAccount;
+  }
+
+  public Integer getImpersonateLifetimeSeconds() {
+    return impersonateLifetimeSeconds;
+  }
+
+  public List<String> getImpersonateScopes() {
+    return impersonateScopes;
+  }
+
+  public List<String> getImpersonateDelegates() {
+    return impersonateDelegates;
+  }
+
+  @Override
+  public String toString() {
+    return MoreObjects.toStringHelper(this)
+        .add("connectionTypeCode", getConnectionTypeCode())
+        .add("uri", getUri())
+        .add("warehouse", warehouse)
+        .add("gcpProjectId", gcpProjectId)
+        .add("gcpLocation", gcpLocation)
+        .add("listAllTables", listAllTables)
+        .add("authenticationParameters", getAuthenticationParameters().toString())
+        .toString();
+  }
+
+  @Override
+  public @Nonnull Map<String, String> asIcebergCatalogProperties(
+      PolarisCredentialManager polarisCredentialManager) {
+    HashMap<String, String> properties = new HashMap<>();
+
+    // Required properties for BigQueryMetastoreCatalog
+    properties.put(GCP_BIGQUERY_PROJECT_ID, gcpProjectId);
+    properties.put(CatalogProperties.WAREHOUSE_LOCATION, warehouse);
+
+    // Optional properties
+    if (gcpLocation != null) {
+      properties.put(GCP_BIGQUERY_LOCATION, gcpLocation);
+    }
+    if (listAllTables != null) {
+      properties.put(GCP_BIGQUERY_LIST_ALL_TABLES, String.valueOf(listAllTables));
+    }
+
+    // Optional impersonation properties
+    if (impersonateServiceAccount != null) {
+      properties.put(IMPERSONATE_SERVICE_ACCOUNT, impersonateServiceAccount);
+    }
+    if (impersonateLifetimeSeconds != null) {
+      properties.put(IMPERSONATE_LIFETIME_SECONDS, String.valueOf(impersonateLifetimeSeconds));
+    }
+    if (impersonateScopes != null && !impersonateScopes.isEmpty()) {
+      properties.put(IMPERSONATE_SCOPES, String.join(",", impersonateScopes));
+    }
+    if (impersonateDelegates != null && !impersonateDelegates.isEmpty()) {
+      properties.put(IMPERSONATE_DELEGATES, String.join(",", impersonateDelegates));
+    }
+
+    if (getAuthenticationParameters() != null) {
+      // Add authentication-specific metadata (non-credential properties)
+      properties.putAll(
+          getAuthenticationParameters().asIcebergCatalogProperties(polarisCredentialManager));
+      // Add connection credentials from Polaris credential manager
+      ConnectionCredentials connectionCredentials =
+          polarisCredentialManager.getConnectionCredentials(this);
+      properties.putAll(connectionCredentials.credentials());
+    }
+    return properties;
+  }
+
+  @Override
+  public ConnectionConfigInfoDpo withServiceIdentity(
+      @Nonnull ServiceIdentityInfoDpo serviceIdentityInfo) {
+    return new BigQueryMetastoreConnectionConfigInfoDpo(
+        getUri(),
+        getAuthenticationParameters(),
+        warehouse,
+        gcpProjectId,
+        gcpLocation,
+        listAllTables,
+        impersonateServiceAccount,
+        impersonateLifetimeSeconds,
+        impersonateScopes,
+        impersonateDelegates,
+        serviceIdentityInfo);
+  }
+
+  @Override
+  public ConnectionConfigInfo asConnectionConfigInfoModel(
+      ServiceIdentityProvider serviceIdentityProvider) {
+    return BigQueryMetastoreConnectionConfigInfo.builder()
+        .setConnectionType(ConnectionConfigInfo.ConnectionTypeEnum.BIGQUERY)
+        .setUri(getUri())
+        .setWarehouse(warehouse)
+        .setGcpProjectId(gcpProjectId)
+        .setGcpLocation(gcpLocation)
+        .setListAllTables(listAllTables)
+        .setImpersonateServiceAccount(impersonateServiceAccount)
+        .setImpersonateLifetimeSeconds(impersonateLifetimeSeconds)
+        .setImpersonateScopes(impersonateScopes)
+        .setImpersonateDelegates(impersonateDelegates)
+        .setAuthenticationParameters(

Review Comment:
   I have added null check in `asConnectionConfigInfoModel`.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3092809811


##########
extensions/federation/bigquery/src/main/java/org/apache/polaris/extensions/federation/bigquery/BigQueryMetastoreFederatedCatalogFactory.java:
##########
@@ -0,0 +1,89 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.extensions.federation.bigquery;
+
+import io.smallrye.common.annotation.Identifier;
+import jakarta.enterprise.context.ApplicationScoped;
+import java.util.Map;
+import org.apache.iceberg.catalog.Catalog;
+import org.apache.iceberg.gcp.bigquery.BigQueryMetastoreCatalog;
+import org.apache.iceberg.rest.RESTUtil;
+import org.apache.polaris.core.catalog.FederatedCatalogFactory;
+import org.apache.polaris.core.catalog.GenericTableCatalog;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.AuthenticationType;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+
+/**
+ * Factory class for creating a BigQuery Metastore catalog handle based on connection configuration.
+ */
+@ApplicationScoped
+@Identifier(ConnectionType.BIGQUERY_FACTORY_IDENTIFIER)
+public class BigQueryMetastoreFederatedCatalogFactory implements FederatedCatalogFactory {
+
+  @Override
+  public Catalog createCatalog(
+      ConnectionConfigInfoDpo connectionConfigInfoDpo,
+      PolarisCredentialManager polarisCredentialManager,
+      Map<String, String> catalogProperties) {
+
+    // Currently, Polaris supports BigQuery Metastore federation only via IMPLICIT authentication.
+    // Hence, prior to initializing the configuration, ensure that the catalog uses
+    // IMPLICIT authentication.
+    AuthenticationParametersDpo authenticationParametersDpo =
+        connectionConfigInfoDpo.getAuthenticationParameters();
+    if (authenticationParametersDpo.getAuthenticationTypeCode()
+        != AuthenticationType.IMPLICIT.getCode()) {
+      throw new IllegalStateException(
+          "BigQuery Metastore federation only supports IMPLICIT authentication.");
+    }
+
+    Map<String, String> properties = connectionConfigInfoDpo.getProperties();
+    String warehouse = properties.get("warehouse");
+    if (warehouse == null || warehouse.isEmpty()) {
+      throw new IllegalArgumentException("warehouse is required for BigQuery Metastore federation");
+    }
+    if (properties.get("gcp.bigquery.project-id") == null) {

Review Comment:
   I have added empty string validation for `gcp.bigquery.project-id`.
   
   



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3087623134


##########
polaris-core/src/main/java/org/apache/polaris/core/connection/bigquery/BigQueryMetastoreConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.core.connection.bigquery;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.MoreObjects;
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+import org.apache.polaris.core.credentials.connection.ConnectionCredentials;
+import org.apache.polaris.core.identity.dpo.ServiceIdentityInfoDpo;
+import org.apache.polaris.core.identity.provider.ServiceIdentityProvider;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo} defined in the API
+ * model.
+ */
+public class BigQueryMetastoreConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+  public static final String DEFAULT_URI = "https://bigquery.googleapis.com";
+
+  // BigQuery Metastore catalog property keys
+  private static final String GCP_BIGQUERY_PROJECT_ID = "gcp.bigquery.project-id";
+
+  private final String warehouse;
+  private final String gcpProjectId;
+
+  public BigQueryMetastoreConnectionConfigInfoDpo(
+      @JsonProperty(value = "uri", required = true) @Nonnull String uri,
+      @JsonProperty(value = "authenticationParameters", required = false) @Nullable
+          AuthenticationParametersDpo authenticationParameters,
+      @JsonProperty(value = "warehouse", required = true) @Nonnull String warehouse,
+      @JsonProperty(value = "gcpProjectId", required = true) @Nonnull String gcpProjectId,
+      @JsonProperty(value = "properties", required = false) @Nullable
+          Map<String, String> properties,
+      @JsonProperty(value = "serviceIdentity", required = false) @Nullable
+          ServiceIdentityInfoDpo serviceIdentity) {
+    super(
+        ConnectionType.BIGQUERY.getCode(),
+        uri,
+        authenticationParameters,
+        serviceIdentity,
+        properties);
+    this.warehouse = warehouse;
+    this.gcpProjectId = gcpProjectId;
+  }
+
+  public String getWarehouse() {
+    return warehouse;
+  }
+
+  public String getGcpProjectId() {
+    return gcpProjectId;
+  }
+
+  @Override
+  public String toString() {
+    return MoreObjects.toStringHelper(this)
+        .add("connectionTypeCode", getConnectionTypeCode())
+        .add("uri", getUri())
+        .add("warehouse", warehouse)
+        .add("gcpProjectId", gcpProjectId)
+        .add(
+            "authenticationParameters",
+            getAuthenticationParameters() != null
+                ? getAuthenticationParameters().toString()
+                : "null")
+        .toString();
+  }
+
+  @Override
+  public @Nonnull Map<String, String> asIcebergCatalogProperties(
+      PolarisCredentialManager polarisCredentialManager) {
+    HashMap<String, String> properties = new HashMap<>();
+
+    // Required properties for BigQueryMetastoreCatalog
+    properties.put(GCP_BIGQUERY_PROJECT_ID, gcpProjectId);
+    properties.put(CatalogProperties.WAREHOUSE_LOCATION, warehouse);

Review Comment:
   Retracting API changes is not ideal (in a follow-up). 
   
   I'd propose to remove `gcpProjectId` and `warehouse` from OpenAPI _explicit_ properties and process corresponding Iceberg properties from the generic `properties` bag. Also, add description to OpenAPI YAML to list required properties.
   
   My rationale is that it is fine to accept pass-through properties destined to become Iceberg (federated) catalog properties. Yet, specific processing / validation is a feature of the implementation and can change in new Polaris versions without requiring API changes.
   
   I wonder what other reviewers think about this.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "joyhaldar (via GitHub)" <gi...@apache.org>.
joyhaldar commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3090685696


##########
polaris-core/src/main/java/org/apache/polaris/core/connection/bigquery/BigQueryMetastoreConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.core.connection.bigquery;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.MoreObjects;
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+import org.apache.polaris.core.credentials.connection.ConnectionCredentials;
+import org.apache.polaris.core.identity.dpo.ServiceIdentityInfoDpo;
+import org.apache.polaris.core.identity.provider.ServiceIdentityProvider;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo} defined in the API
+ * model.
+ */
+public class BigQueryMetastoreConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+  public static final String DEFAULT_URI = "https://bigquery.googleapis.com";
+
+  // BigQuery Metastore catalog property keys
+  private static final String GCP_BIGQUERY_PROJECT_ID = "gcp.bigquery.project-id";
+
+  private final String warehouse;
+  private final String gcpProjectId;
+
+  public BigQueryMetastoreConnectionConfigInfoDpo(
+      @JsonProperty(value = "uri", required = true) @Nonnull String uri,
+      @JsonProperty(value = "authenticationParameters", required = false) @Nullable
+          AuthenticationParametersDpo authenticationParameters,
+      @JsonProperty(value = "warehouse", required = true) @Nonnull String warehouse,
+      @JsonProperty(value = "gcpProjectId", required = true) @Nonnull String gcpProjectId,
+      @JsonProperty(value = "properties", required = false) @Nullable
+          Map<String, String> properties,
+      @JsonProperty(value = "serviceIdentity", required = false) @Nullable
+          ServiceIdentityInfoDpo serviceIdentity) {
+    super(
+        ConnectionType.BIGQUERY.getCode(),
+        uri,
+        authenticationParameters,
+        serviceIdentity,
+        properties);
+    this.warehouse = warehouse;
+    this.gcpProjectId = gcpProjectId;
+  }
+
+  public String getWarehouse() {
+    return warehouse;
+  }
+
+  public String getGcpProjectId() {
+    return gcpProjectId;
+  }
+
+  @Override
+  public String toString() {
+    return MoreObjects.toStringHelper(this)
+        .add("connectionTypeCode", getConnectionTypeCode())
+        .add("uri", getUri())
+        .add("warehouse", warehouse)
+        .add("gcpProjectId", gcpProjectId)
+        .add(
+            "authenticationParameters",
+            getAuthenticationParameters() != null
+                ? getAuthenticationParameters().toString()
+                : "null")
+        .toString();
+  }
+
+  @Override
+  public @Nonnull Map<String, String> asIcebergCatalogProperties(
+      PolarisCredentialManager polarisCredentialManager) {
+    HashMap<String, String> properties = new HashMap<>();
+
+    // Required properties for BigQueryMetastoreCatalog
+    properties.put(GCP_BIGQUERY_PROJECT_ID, gcpProjectId);
+    properties.put(CatalogProperties.WAREHOUSE_LOCATION, warehouse);

Review Comment:
   I have moved `warehouse` and `gcpProjectId` to the properties map with validation in the factory and documented required properties in the spec in [this commit](https://github.com/apache/polaris/pull/4050/changes/33ae93a630e15736b29c989cc3a2b98a418ff712).



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3087623134


##########
polaris-core/src/main/java/org/apache/polaris/core/connection/bigquery/BigQueryMetastoreConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.core.connection.bigquery;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.MoreObjects;
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+import org.apache.polaris.core.credentials.connection.ConnectionCredentials;
+import org.apache.polaris.core.identity.dpo.ServiceIdentityInfoDpo;
+import org.apache.polaris.core.identity.provider.ServiceIdentityProvider;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo} defined in the API
+ * model.
+ */
+public class BigQueryMetastoreConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+  public static final String DEFAULT_URI = "https://bigquery.googleapis.com";
+
+  // BigQuery Metastore catalog property keys
+  private static final String GCP_BIGQUERY_PROJECT_ID = "gcp.bigquery.project-id";
+
+  private final String warehouse;
+  private final String gcpProjectId;
+
+  public BigQueryMetastoreConnectionConfigInfoDpo(
+      @JsonProperty(value = "uri", required = true) @Nonnull String uri,
+      @JsonProperty(value = "authenticationParameters", required = false) @Nullable
+          AuthenticationParametersDpo authenticationParameters,
+      @JsonProperty(value = "warehouse", required = true) @Nonnull String warehouse,
+      @JsonProperty(value = "gcpProjectId", required = true) @Nonnull String gcpProjectId,
+      @JsonProperty(value = "properties", required = false) @Nullable
+          Map<String, String> properties,
+      @JsonProperty(value = "serviceIdentity", required = false) @Nullable
+          ServiceIdentityInfoDpo serviceIdentity) {
+    super(
+        ConnectionType.BIGQUERY.getCode(),
+        uri,
+        authenticationParameters,
+        serviceIdentity,
+        properties);
+    this.warehouse = warehouse;
+    this.gcpProjectId = gcpProjectId;
+  }
+
+  public String getWarehouse() {
+    return warehouse;
+  }
+
+  public String getGcpProjectId() {
+    return gcpProjectId;
+  }
+
+  @Override
+  public String toString() {
+    return MoreObjects.toStringHelper(this)
+        .add("connectionTypeCode", getConnectionTypeCode())
+        .add("uri", getUri())
+        .add("warehouse", warehouse)
+        .add("gcpProjectId", gcpProjectId)
+        .add(
+            "authenticationParameters",
+            getAuthenticationParameters() != null
+                ? getAuthenticationParameters().toString()
+                : "null")
+        .toString();
+  }
+
+  @Override
+  public @Nonnull Map<String, String> asIcebergCatalogProperties(
+      PolarisCredentialManager polarisCredentialManager) {
+    HashMap<String, String> properties = new HashMap<>();
+
+    // Required properties for BigQueryMetastoreCatalog
+    properties.put(GCP_BIGQUERY_PROJECT_ID, gcpProjectId);
+    properties.put(CatalogProperties.WAREHOUSE_LOCATION, warehouse);

Review Comment:
   Retracting API changes is not ideal (in a follow-up). 
   
   I'd propose to remove `gcpProjectId` and `warehouse` from OpenAPI _explicit_ properties and process corresponding Iceberg properties from the generic `properties` bag. Also, add description to OpenAPI YAML to list required properties.
   
   My rationale is that it is fine to accept pass-through properties destined to become Iceberg (federated) catalog properties. Yet, specific processing / validate is a feature of the implementation and can change in new Polaris versions without requiring API changes.
   
   I wonder what other reviewers think about this.



-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on PR #4050:
URL: https://github.com/apache/polaris/pull/4050#issuecomment-4282947092

   I believe there is an option to re-run failed CI jobs without closing/reopening the PR :thinking: 
   
   +1 to merging today.


-- 
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: [PR] Add BigQuery Metastore federation support [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4050:
URL: https://github.com/apache/polaris/pull/4050#discussion_r3081047722


##########
polaris-core/src/main/java/org/apache/polaris/core/connection/bigquery/BigQueryMetastoreConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.core.connection.bigquery;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.MoreObjects;
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.connection.AuthenticationParametersDpo;
+import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
+import org.apache.polaris.core.connection.ConnectionType;
+import org.apache.polaris.core.credentials.PolarisCredentialManager;
+import org.apache.polaris.core.credentials.connection.ConnectionCredentials;
+import org.apache.polaris.core.identity.dpo.ServiceIdentityInfoDpo;
+import org.apache.polaris.core.identity.provider.ServiceIdentityProvider;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.BigQueryMetastoreConnectionConfigInfo} defined in the API
+ * model.
+ */
+public class BigQueryMetastoreConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+  public static final String DEFAULT_URI = "https://bigquery.googleapis.com";
+
+  // BigQuery Metastore catalog property keys
+  private static final String GCP_BIGQUERY_PROJECT_ID = "gcp.bigquery.project-id";
+
+  private final String warehouse;
+  private final String gcpProjectId;
+
+  public BigQueryMetastoreConnectionConfigInfoDpo(
+      @JsonProperty(value = "uri", required = true) @Nonnull String uri,
+      @JsonProperty(value = "authenticationParameters", required = false) @Nullable
+          AuthenticationParametersDpo authenticationParameters,
+      @JsonProperty(value = "warehouse", required = true) @Nonnull String warehouse,
+      @JsonProperty(value = "gcpProjectId", required = true) @Nonnull String gcpProjectId,
+      @JsonProperty(value = "properties", required = false) @Nullable
+          Map<String, String> properties,
+      @JsonProperty(value = "serviceIdentity", required = false) @Nullable
+          ServiceIdentityInfoDpo serviceIdentity) {
+    super(
+        ConnectionType.BIGQUERY.getCode(),
+        uri,
+        authenticationParameters,
+        serviceIdentity,
+        properties);
+    this.warehouse = warehouse;
+    this.gcpProjectId = gcpProjectId;
+  }
+
+  public String getWarehouse() {
+    return warehouse;
+  }
+
+  public String getGcpProjectId() {
+    return gcpProjectId;
+  }
+
+  @Override
+  public String toString() {
+    return MoreObjects.toStringHelper(this)
+        .add("connectionTypeCode", getConnectionTypeCode())
+        .add("uri", getUri())
+        .add("warehouse", warehouse)
+        .add("gcpProjectId", gcpProjectId)
+        .add(
+            "authenticationParameters",
+            getAuthenticationParameters() != null
+                ? getAuthenticationParameters().toString()
+                : "null")
+        .toString();
+  }
+
+  @Override
+  public @Nonnull Map<String, String> asIcebergCatalogProperties(
+      PolarisCredentialManager polarisCredentialManager) {
+    HashMap<String, String> properties = new HashMap<>();
+
+    // Required properties for BigQueryMetastoreCatalog
+    properties.put(GCP_BIGQUERY_PROJECT_ID, gcpProjectId);
+    properties.put(CatalogProperties.WAREHOUSE_LOCATION, warehouse);

Review Comment:
   Why not use the generic `getProperties()` map (line 108) for these parameter too?



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