You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "eric-maynard (via GitHub)" <gi...@apache.org> on 2025/04/25 20:43:23 UTC
[PR] Add polaris-wrapper config; implement polaris-wrapper.mode=hadoop [polaris]
eric-maynard opened a new pull request, #1466:
URL: https://github.com/apache/polaris/pull/1466
Polaris recently added support for catalog federation in #1305, which allows a Polaris instance to federate requests against a given catalog to another Polaris instance. This is useful for deployments with multiple Polaris instances where one primary instance should defer to another for requests against a given catalog:
<img width="350" alt="Screenshot 2025-04-25 at 1 39 19 PM" src="https://github.com/user-attachments/assets/9dac7cd7-2108-495b-ac1d-cc5f2b3eaba1" />
However, many deployments also have _other_ catalogs in use. For example, some legacy data may be stored using the file-based Hadoop catalog, one part of the organization may be using a vendor's Iceberg catalog, and there could be a team that's been using a Hive catalog. In order for Polaris to federate to these various catalogs, we can deploy Polaris in _wrapper mode_ against these catalogs:
<img width="350" alt="Screenshot 2025-04-25 at 1 42 59 PM" src="https://github.com/user-attachments/assets/78204d0d-3b5d-4cee-a899-75d8dd90d9f0" />
This PR introduces the configurations related to this wrapper mode, and implements a simple wrapper for HadoopCatalog.
--
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] Implement federation to HadoopCatalog [polaris]
Posted by "eric-maynard (via GitHub)" <gi...@apache.org>.
eric-maynard merged PR #1466:
URL: https://github.com/apache/polaris/pull/1466
--
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] Implement federation to HadoopCatalog [polaris]
Posted by "singhpk234 (via GitHub)" <gi...@apache.org>.
singhpk234 commented on code in PR #1466:
URL: https://github.com/apache/polaris/pull/1466#discussion_r2072515021
##########
polaris-core/src/main/java/org/apache/polaris/core/connection/hadoop/HadoopConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,88 @@
+/*
+ * 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.hadoop;
+
+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 org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.HadoopConnectionConfigInfo;
+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.secrets.UserSecretsManager;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.HadoopConnectionConfigInfo} defined in the API model.
+ */
+public class HadoopConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+ private final String warehouse;
+
+ public HadoopConnectionConfigInfoDpo(
+ @JsonProperty(value = "uri", required = true) @Nonnull String uri,
+ @JsonProperty(value = "authenticationParameters", required = true) @Nonnull
Review Comment:
Should we restrict HadoopCatalog backed by cloud-providers only (like S3) and not HDFS clusters for phase 1, as opening up HDFS will add more cases like HDFS backed by kreberos, its not just hdfs-site.xml that Configuration contains, thoughts ?
--
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] Implement federation to HadoopCatalog [polaris]
Posted by "adnanhemani (via GitHub)" <gi...@apache.org>.
adnanhemani commented on code in PR #1466:
URL: https://github.com/apache/polaris/pull/1466#discussion_r2067662627
##########
integration-tests/src/main/java/org/apache/polaris/service/it/env/ManagementApi.java:
##########
@@ -173,6 +173,9 @@ public void createCatalog(String principalRoleName, Catalog catalog) {
public void createCatalog(Catalog catalog) {
try (Response response = request("v1/catalogs").post(Entity.json(catalog))) {
+ if (response.getStatus() == 500) {
Review Comment:
nit: Might it be more helpful to log the response itself?
--
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] Implement federation to HadoopCatalog [polaris]
Posted by "eric-maynard (via GitHub)" <gi...@apache.org>.
eric-maynard commented on code in PR #1466:
URL: https://github.com/apache/polaris/pull/1466#discussion_r2073971189
##########
polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java:
##########
@@ -234,4 +235,11 @@ public static void enforceFeatureEnabledOrThrow(
.description("If true, the policy-store endpoints are enabled")
.defaultValue(true)
.buildFeatureConfiguration();
+
+ public static final FeatureConfiguration<List<String>> SUPPORTED_CATALOG_CONNECTION_TYPES =
+ PolarisConfiguration.<List<String>>builder()
+ .key("SUPPORTED_CATALOG_CONNECTION_TYPES")
+ .description("The list of supported catalog connection types for federation")
+ .defaultValue(List.of(ConnectionType.ICEBERG_REST.name(), ConnectionType.HADOOP.name()))
Review Comment:
Maybe one way we can stay consistent with this precedent but also be really safe-by-default (beyond what we do for storage types even) is to add a second flag gating off support for federation altogether... something like `CATALOG_FEDERATION_ENABLED`?
--
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] Implement federation to HadoopCatalog [polaris]
Posted by "dennishuo (via GitHub)" <gi...@apache.org>.
dennishuo commented on code in PR #1466:
URL: https://github.com/apache/polaris/pull/1466#discussion_r2065422240
##########
spec/polaris-management-service.yml:
##########
@@ -862,6 +862,7 @@ components:
type: string
enum:
- ICEBERG_REST
+ - HADOOP
Review Comment:
Since the `HadoopCatalog` is highly unsafe if the set of people who have `CATALOG_CREATE` privileges isn't exactly equal to the set of people with superuser privileges to run the entire Catalog service itself, before we introduce this one even as an option we should probably add a `ALLOWED_CATALOG_CONNECTION_TYPES` or `SUPPORTED_CATALOG_CONNECTION_TYPES` analogous to `SUPPORTED_CATALOG_STORAGE_TYPES`.
##########
polaris-core/src/main/java/org/apache/polaris/core/connection/hadoop/HadoopConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,88 @@
+/*
+ * 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.hadoop;
+
+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 org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.HadoopConnectionConfigInfo;
+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.secrets.UserSecretsManager;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.HadoopConnectionConfigInfo} defined in the API model.
+ */
+public class HadoopConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+ private final String warehouse;
+
+ public HadoopConnectionConfigInfoDpo(
+ @JsonProperty(value = "uri", required = true) @Nonnull String uri,
+ @JsonProperty(value = "authenticationParameters", required = true) @Nonnull
Review Comment:
If we extend or refactor `HadoopCatalog` then the auth could be more properly injectable; unfortunately right now it appears to just hard-code using `HadoopFileIO` as its `fileIOImpl` and also independently instantiates a `org.apache.hadoop.fs.FileSystem`, which relies on the `org.apache.hadoop.conf.Configuration` which appears to be null in this PR since we call the no-arg constructor of `HadoopCatalog` instead of the 2-arg constructor that takes a `Configuration`.
From what I can tell this means it'll just fallback to Hadoop's "default" Configuration loading where it would normally load things from e.g. `hdfs-site.xml`.
If we're going to really support running this federation mode properly, we should at least add a TODO to expand a HadoopCatalog-specific AuthenticationParameters subclass. One way would be to express similar delegation settings as `StorageConfigInfo`, or alternatively, HadoopCatalog is a special-case where the real storage credential *is* the catalog credential, so maybe we could just have something that "points" at the Catalog's StorageConfigInfo as the effective AuthenticationParameters.
--
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] Implement federation to HadoopCatalog [polaris]
Posted by "eric-maynard (via GitHub)" <gi...@apache.org>.
eric-maynard commented on code in PR #1466:
URL: https://github.com/apache/polaris/pull/1466#discussion_r2065140736
##########
polaris-core/src/main/java/org/apache/polaris/core/connection/hadoop/HadoopConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,88 @@
+/*
+ * 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.hadoop;
+
+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 org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.HadoopConnectionConfigInfo;
+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.secrets.UserSecretsManager;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.HadoopConnectionConfigInfo} defined in the API model.
+ */
+public class HadoopConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+ private final String warehouse;
+
+ public HadoopConnectionConfigInfoDpo(
+ @JsonProperty(value = "uri", required = true) @Nonnull String uri,
+ @JsonProperty(value = "authenticationParameters", required = true) @Nonnull
Review Comment:
We essentially just forward whatever properties into the catalog itself, which I think functions as the auth mechanism
--
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] Implement federation to HadoopCatalog [polaris]
Posted by "eric-maynard (via GitHub)" <gi...@apache.org>.
eric-maynard commented on code in PR #1466:
URL: https://github.com/apache/polaris/pull/1466#discussion_r2065139598
##########
polaris-core/src/main/java/org/apache/polaris/core/connection/iceberg/IcebergRestConnectionConfigInfoDpo.java:
##########
Review Comment:
My concern with that alternative is that delegation to e.g. Unity Catalog will also be over REST. So it's more like iceberg-rest, I called it `iceberg` but `rest` is not ideal either
--
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] Implement federation to HadoopCatalog [polaris]
Posted by "eric-maynard (via GitHub)" <gi...@apache.org>.
eric-maynard commented on code in PR #1466:
URL: https://github.com/apache/polaris/pull/1466#discussion_r2075920779
##########
polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java:
##########
@@ -234,4 +235,11 @@ public static void enforceFeatureEnabledOrThrow(
.description("If true, the policy-store endpoints are enabled")
.defaultValue(true)
.buildFeatureConfiguration();
+
+ public static final FeatureConfiguration<List<String>> SUPPORTED_CATALOG_CONNECTION_TYPES =
+ PolarisConfiguration.<List<String>>builder()
+ .key("SUPPORTED_CATALOG_CONNECTION_TYPES")
+ .description("The list of supported catalog connection types for federation")
+ .defaultValue(List.of(ConnectionType.ICEBERG_REST.name(), ConnectionType.HADOOP.name()))
Review Comment:
Doesn't storage type FILE also risk the security of the server? It seems like almost the same situation there.
--
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] Implement federation to HadoopCatalog [polaris]
Posted by "eric-maynard (via GitHub)" <gi...@apache.org>.
eric-maynard commented on code in PR #1466:
URL: https://github.com/apache/polaris/pull/1466#discussion_r2075920779
##########
polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java:
##########
@@ -234,4 +235,11 @@ public static void enforceFeatureEnabledOrThrow(
.description("If true, the policy-store endpoints are enabled")
.defaultValue(true)
.buildFeatureConfiguration();
+
+ public static final FeatureConfiguration<List<String>> SUPPORTED_CATALOG_CONNECTION_TYPES =
+ PolarisConfiguration.<List<String>>builder()
+ .key("SUPPORTED_CATALOG_CONNECTION_TYPES")
+ .description("The list of supported catalog connection types for federation")
+ .defaultValue(List.of(ConnectionType.ICEBERG_REST.name(), ConnectionType.HADOOP.name()))
Review Comment:
Doesn't storage type FILE also risk the security of the server?
--
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] Implement federation to HadoopCatalog [polaris]
Posted by "dennishuo (via GitHub)" <gi...@apache.org>.
dennishuo commented on code in PR #1466:
URL: https://github.com/apache/polaris/pull/1466#discussion_r2073870563
##########
polaris-core/src/main/java/org/apache/polaris/core/connection/hadoop/HadoopConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,88 @@
+/*
+ * 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.hadoop;
+
+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 org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.HadoopConnectionConfigInfo;
+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.secrets.UserSecretsManager;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.HadoopConnectionConfigInfo} defined in the API model.
+ */
+public class HadoopConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+ private final String warehouse;
+
+ public HadoopConnectionConfigInfoDpo(
+ @JsonProperty(value = "uri", required = true) @Nonnull String uri,
+ @JsonProperty(value = "authenticationParameters", required = true) @Nonnull
Review Comment:
Right, identity-passthrough/delegation is important long-term, but has a lot of complicated prerequisites and other dependencies on how we express federated identities and user-isolated caching of metadata or storage credentials, so in the meantime we should just clearly document what the "powerful connectionconfig" model implies.
##########
polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java:
##########
@@ -234,4 +235,11 @@ public static void enforceFeatureEnabledOrThrow(
.description("If true, the policy-store endpoints are enabled")
.defaultValue(true)
.buildFeatureConfiguration();
+
+ public static final FeatureConfiguration<List<String>> SUPPORTED_CATALOG_CONNECTION_TYPES =
+ PolarisConfiguration.<List<String>>builder()
+ .key("SUPPORTED_CATALOG_CONNECTION_TYPES")
+ .description("The list of supported catalog connection types for federation")
+ .defaultValue(List.of(ConnectionType.ICEBERG_REST.name(), ConnectionType.HADOOP.name()))
Review Comment:
Is it possible to make this default an empty list, so that whoever wants to enable the federation feature has to explicitly opt-in to the connection types they want?
And/or set it to an empty list in application.properties. Or if empty list doesn't work for some reason, it seems ICEBERG_REST-only is a reasonable initial default since it's the minimal-dependency one long-term with a more well-defined security boundary.
--
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] Implement federation to HadoopCatalog [polaris]
Posted by "eric-maynard (via GitHub)" <gi...@apache.org>.
eric-maynard commented on code in PR #1466:
URL: https://github.com/apache/polaris/pull/1466#discussion_r2065756665
##########
polaris-core/src/main/java/org/apache/polaris/core/connection/hadoop/HadoopConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,88 @@
+/*
+ * 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.hadoop;
+
+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 org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.HadoopConnectionConfigInfo;
+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.secrets.UserSecretsManager;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.HadoopConnectionConfigInfo} defined in the API model.
+ */
+public class HadoopConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+ private final String warehouse;
+
+ public HadoopConnectionConfigInfoDpo(
+ @JsonProperty(value = "uri", required = true) @Nonnull String uri,
+ @JsonProperty(value = "authenticationParameters", required = true) @Nonnull
Review Comment:
I actually think supporting HadoopCatalog as a sort of "dumb" federation mode that just uses the properties you give it (and loads from `hdfs-site.xml` by default) is fine... if we wanted to add another module that brings in a `POLARIS_HADOOP` mode or something in the future that could be nice, 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
Re: [PR] Implement federation to HadoopCatalog [polaris]
Posted by "eric-maynard (via GitHub)" <gi...@apache.org>.
eric-maynard commented on code in PR #1466:
URL: https://github.com/apache/polaris/pull/1466#discussion_r2065758136
##########
polaris-core/src/main/java/org/apache/polaris/core/connection/hadoop/HadoopConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,88 @@
+/*
+ * 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.hadoop;
+
+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 org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.HadoopConnectionConfigInfo;
+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.secrets.UserSecretsManager;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.HadoopConnectionConfigInfo} defined in the API model.
+ */
+public class HadoopConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+ private final String warehouse;
+
+ public HadoopConnectionConfigInfoDpo(
+ @JsonProperty(value = "uri", required = true) @Nonnull String uri,
+ @JsonProperty(value = "authenticationParameters", required = true) @Nonnull
Review Comment:
> it appears to just hard-code using HadoopFileIO
I think the properties we pass into `initialize` should take effect right? That would be the fallback if `io-impl` is not specified.
--
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] Implement federation to HadoopCatalog [polaris]
Posted by "eric-maynard (via GitHub)" <gi...@apache.org>.
eric-maynard commented on code in PR #1466:
URL: https://github.com/apache/polaris/pull/1466#discussion_r2065758423
##########
spec/polaris-management-service.yml:
##########
@@ -862,6 +862,7 @@ components:
type: string
enum:
- ICEBERG_REST
+ - HADOOP
Review Comment:
Great idea, added
--
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] Implement federation to HadoopCatalog [polaris]
Posted by "eric-maynard (via GitHub)" <gi...@apache.org>.
eric-maynard commented on code in PR #1466:
URL: https://github.com/apache/polaris/pull/1466#discussion_r2065142880
##########
polaris-core/src/main/java/org/apache/polaris/core/connection/ConnectionType.java:
##########
@@ -32,6 +32,7 @@
public enum ConnectionType {
NULL_TYPE(0),
ICEBERG_REST(1),
+ HADOOP(2),
Review Comment:
This is for Hadoop's `IcebergCatalog`, but good point. For UC, we might have a separate connection for UC's IRC endpoints compared to the non-IRC ones and they might be used for different table types
--
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] Implement federation to HadoopCatalog [polaris]
Posted by "eric-maynard (via GitHub)" <gi...@apache.org>.
eric-maynard commented on code in PR #1466:
URL: https://github.com/apache/polaris/pull/1466#discussion_r2065756665
##########
polaris-core/src/main/java/org/apache/polaris/core/connection/hadoop/HadoopConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,88 @@
+/*
+ * 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.hadoop;
+
+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 org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.HadoopConnectionConfigInfo;
+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.secrets.UserSecretsManager;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.HadoopConnectionConfigInfo} defined in the API model.
+ */
+public class HadoopConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+ private final String warehouse;
+
+ public HadoopConnectionConfigInfoDpo(
+ @JsonProperty(value = "uri", required = true) @Nonnull String uri,
+ @JsonProperty(value = "authenticationParameters", required = true) @Nonnull
Review Comment:
I actually think supporting HadoopCatalog as a sort of "dumb" federation mode that just uses the properties you give it (and loads from `hdfs-site.xml` by default) is fine... if we wanted to add another module that brings in a `POLARIS_HADOOP` mode or something in the future that could be nice, too.
In practice, the way you would probably deploy this is by creating a second Polaris instance dedicated to wrapping the Hadoop catalog, where you can specify env variables and a `hdfs-site.xml`
--
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] Implement federation to HadoopCatalog [polaris]
Posted by "eric-maynard (via GitHub)" <gi...@apache.org>.
eric-maynard commented on code in PR #1466:
URL: https://github.com/apache/polaris/pull/1466#discussion_r2072522362
##########
polaris-core/src/main/java/org/apache/polaris/core/connection/hadoop/HadoopConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,88 @@
+/*
+ * 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.hadoop;
+
+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 org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.HadoopConnectionConfigInfo;
+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.secrets.UserSecretsManager;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.HadoopConnectionConfigInfo} defined in the API model.
+ */
+public class HadoopConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+ private final String warehouse;
+
+ public HadoopConnectionConfigInfoDpo(
+ @JsonProperty(value = "uri", required = true) @Nonnull String uri,
+ @JsonProperty(value = "authenticationParameters", required = true) @Nonnull
Review Comment:
> Polaris should IMHO need to some down scoping or never the less make a way to provide the federated catalog the requester's identity
The same would be true for even the simplest case -- Polaris -> Polaris federation. This isn't unique to federation to Hadoop. It is, however, a very hard problem. What if the two systems don't have the same concept of identity, or they don't trust the same IdP?
There are cases where identity federation may unlock new solutions for us, where the two catalogs can federate to the same trusted IdP. But there are also going to be cases where this is not so.
The way Polaris -> IRC federation currently works, and the way that I think federation will have to continue working for the time being, is that Polaris simply communicates with the remote catalog via the connection config. This can be set on the catalog level today, but in the future we will be able to set this on the namespace or the table level, similar to a storage config.
--
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] Implement federation to HadoopCatalog [polaris]
Posted by "eric-maynard (via GitHub)" <gi...@apache.org>.
eric-maynard commented on code in PR #1466:
URL: https://github.com/apache/polaris/pull/1466#discussion_r2072522362
##########
polaris-core/src/main/java/org/apache/polaris/core/connection/hadoop/HadoopConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,88 @@
+/*
+ * 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.hadoop;
+
+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 org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.HadoopConnectionConfigInfo;
+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.secrets.UserSecretsManager;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.HadoopConnectionConfigInfo} defined in the API model.
+ */
+public class HadoopConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+ private final String warehouse;
+
+ public HadoopConnectionConfigInfoDpo(
+ @JsonProperty(value = "uri", required = true) @Nonnull String uri,
+ @JsonProperty(value = "authenticationParameters", required = true) @Nonnull
Review Comment:
> Polaris should IMHO need to some down scoping or never the less make a way to provide the federated catalog the requester's identity
I agree with you that this should be a goal. I would point out however that the same would be true for even the simplest case -- Polaris -> Polaris federation. This isn't unique to federation to Hadoop. It is, however, a very hard problem. What if the two systems don't have the same concept of identity, or they don't trust the same IdP?
There are cases where identity federation may unlock new solutions for us, where the two catalogs can federate to the same trusted IdP. But there are also going to be cases where this is not so.
The way Polaris -> IRC federation currently works, and the way that I think federation will have to continue working for the time being, is that Polaris simply communicates with the remote catalog via the connection config. This can be set on the catalog level today, but in the future we will be able to set this on the namespace or the table level, similar to a storage config.
--
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] Implement federation to HadoopCatalog [polaris]
Posted by "XJDKC (via GitHub)" <gi...@apache.org>.
XJDKC commented on code in PR #1466:
URL: https://github.com/apache/polaris/pull/1466#discussion_r2064829470
##########
polaris-core/src/main/java/org/apache/polaris/core/connection/iceberg/IcebergRestConnectionConfigInfoDpo.java:
##########
Review Comment:
Should we name the package as `rest`? Also, `IcebergCatalogProperties` is also implemented by `HadoopConnectionConfigInfoDpo`, it's used by any Iceberg related connection configs, maybe we can leave it in the current package (`iceberg`).
##########
polaris-core/src/main/java/org/apache/polaris/core/connection/ConnectionType.java:
##########
@@ -32,6 +32,7 @@
public enum ConnectionType {
NULL_TYPE(0),
ICEBERG_REST(1),
+ HADOOP(2),
Review Comment:
Do we plan to federate to `Hadoop` through a non-Iceberg `Catalog`?
Just curious how generic tables would work with Catalog Federation in that case.
##########
polaris-core/src/main/java/org/apache/polaris/core/connection/hadoop/HadoopConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,88 @@
+/*
+ * 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.hadoop;
+
+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 org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.HadoopConnectionConfigInfo;
+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.secrets.UserSecretsManager;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.HadoopConnectionConfigInfo} defined in the API model.
+ */
+public class HadoopConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+ private final String warehouse;
+
+ public HadoopConnectionConfigInfoDpo(
+ @JsonProperty(value = "uri", required = true) @Nonnull String uri,
+ @JsonProperty(value = "authenticationParameters", required = true) @Nonnull
Review Comment:
QQ: What type of authentication method does `HadoopCatalog` typically use? I tried looking for some docs but couldn't find much.
Should we add a no-op authentication method?
##########
spec/polaris-management-service.yml:
##########
@@ -887,6 +889,16 @@ components:
this is specified as the 'warehouse' when multiple logical catalogs are served under the same base
uri, and often translates into a 'prefix' added to all REST resource paths
+ HadoopConnectionConfigInfo:
+ type: object
+ description: Configuration necessary for connecting to a Hadoop Catalog
+ allOf:
+ - $ref: '#/components/schemas/ConnectionConfigInfo'
+ properties:
+ warehouse:
Review Comment:
Just want to flag that, for `IcebergRestConnectionConfigInfo`, we use `remoteCatalogName`, and it's passed as the warehouse property when initializing the catalog client. Using `warehouse` make more sense for `HadoopConnectionConfigInfo`.
--
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] Implement federation to HadoopCatalog [polaris]
Posted by "eric-maynard (via GitHub)" <gi...@apache.org>.
eric-maynard commented on code in PR #1466:
URL: https://github.com/apache/polaris/pull/1466#discussion_r2065758136
##########
polaris-core/src/main/java/org/apache/polaris/core/connection/hadoop/HadoopConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,88 @@
+/*
+ * 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.hadoop;
+
+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 org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.HadoopConnectionConfigInfo;
+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.secrets.UserSecretsManager;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.HadoopConnectionConfigInfo} defined in the API model.
+ */
+public class HadoopConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+ private final String warehouse;
+
+ public HadoopConnectionConfigInfoDpo(
+ @JsonProperty(value = "uri", required = true) @Nonnull String uri,
+ @JsonProperty(value = "authenticationParameters", required = true) @Nonnull
Review Comment:
> it appears to just hard-code using HadoopFileIO
I think the properties we pass into `initialize` should take effect right? That would be the fallback.
##########
polaris-core/src/main/java/org/apache/polaris/core/connection/hadoop/HadoopConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,88 @@
+/*
+ * 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.hadoop;
+
+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 org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.HadoopConnectionConfigInfo;
+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.secrets.UserSecretsManager;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.HadoopConnectionConfigInfo} defined in the API model.
+ */
+public class HadoopConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+ private final String warehouse;
+
+ public HadoopConnectionConfigInfoDpo(
+ @JsonProperty(value = "uri", required = true) @Nonnull String uri,
+ @JsonProperty(value = "authenticationParameters", required = true) @Nonnull
Review Comment:
> it appears to just hard-code using HadoopFileIO
I think the properties we pass into `initialize` should take effect right? That would be the fallback.
--
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] Implement federation to HadoopCatalog [polaris]
Posted by "eric-maynard (via GitHub)" <gi...@apache.org>.
eric-maynard commented on code in PR #1466:
URL: https://github.com/apache/polaris/pull/1466#discussion_r2072515956
##########
polaris-core/src/main/java/org/apache/polaris/core/connection/hadoop/HadoopConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,88 @@
+/*
+ * 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.hadoop;
+
+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 org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.HadoopConnectionConfigInfo;
+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.secrets.UserSecretsManager;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.HadoopConnectionConfigInfo} defined in the API model.
+ */
+public class HadoopConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+ private final String warehouse;
+
+ public HadoopConnectionConfigInfoDpo(
+ @JsonProperty(value = "uri", required = true) @Nonnull String uri,
+ @JsonProperty(value = "authenticationParameters", required = true) @Nonnull
Review Comment:
I think the concept of federation means that Polaris itself is completely agnostic to the configuration of the remote catalog. Otherwise, when we delegate to IRCs, should we give admins some kind of knobs about which IRCs we can and cannot delegate to? Should we enforce certain settings for those IRCs?
In my view, it makes sense to just create a HadoopCatalog and pass along whatever properties the user provides when the user has configured Polaris to delegate to a Hadoop catalog.
--
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] Implement federation to HadoopCatalog [polaris]
Posted by "eric-maynard (via GitHub)" <gi...@apache.org>.
eric-maynard commented on code in PR #1466:
URL: https://github.com/apache/polaris/pull/1466#discussion_r2072522362
##########
polaris-core/src/main/java/org/apache/polaris/core/connection/hadoop/HadoopConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,88 @@
+/*
+ * 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.hadoop;
+
+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 org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.HadoopConnectionConfigInfo;
+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.secrets.UserSecretsManager;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.HadoopConnectionConfigInfo} defined in the API model.
+ */
+public class HadoopConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+ private final String warehouse;
+
+ public HadoopConnectionConfigInfoDpo(
+ @JsonProperty(value = "uri", required = true) @Nonnull String uri,
+ @JsonProperty(value = "authenticationParameters", required = true) @Nonnull
Review Comment:
> Polaris should IMHO need to some down scoping or never the less make a way to provide the federated catalog the requester's identity
I agree with you that this should be a goal. I would point out however that the same would be true for even the simplest case -- Polaris -> Polaris federation. This problem isn't unique to federation to Hadoop. It is, however, a very hard problem. What if the two systems don't have the same concept of identity, or they don't trust the same IdP?
There are cases where identity federation may unlock new solutions for us, where the two catalogs can federate to the same trusted IdP. But there are also going to be cases where this is not so.
The way Polaris -> IRC federation currently works, and the way that I think federation will have to continue working for the time being, is that Polaris simply communicates with the remote catalog via the connection config. This can be set on the catalog level today, but in the future we will be able to set this on the namespace or the table level, similar to a storage config.
--
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] Implement federation to HadoopCatalog [polaris]
Posted by "eric-maynard (via GitHub)" <gi...@apache.org>.
eric-maynard commented on code in PR #1466:
URL: https://github.com/apache/polaris/pull/1466#discussion_r2073959622
##########
polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java:
##########
@@ -234,4 +235,11 @@ public static void enforceFeatureEnabledOrThrow(
.description("If true, the policy-store endpoints are enabled")
.defaultValue(true)
.buildFeatureConfiguration();
+
+ public static final FeatureConfiguration<List<String>> SUPPORTED_CATALOG_CONNECTION_TYPES =
+ PolarisConfiguration.<List<String>>builder()
+ .key("SUPPORTED_CATALOG_CONNECTION_TYPES")
+ .description("The list of supported catalog connection types for federation")
+ .defaultValue(List.of(ConnectionType.ICEBERG_REST.name(), ConnectionType.HADOOP.name()))
Review Comment:
It's possible and I agree with the sentiment, but I would point to the default inclusion of FILE as a supported storage type as an important precedent here. Following this, it seems like what we should do is call out `SUPPORTED_CATALOG_CONNECTION_TYPES` in `configuring-polaris-for-production`. 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] Implement federation to HadoopCatalog [polaris]
Posted by "dennishuo (via GitHub)" <gi...@apache.org>.
dennishuo commented on code in PR #1466:
URL: https://github.com/apache/polaris/pull/1466#discussion_r2074686001
##########
polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java:
##########
@@ -234,4 +235,11 @@ public static void enforceFeatureEnabledOrThrow(
.description("If true, the policy-store endpoints are enabled")
.defaultValue(true)
.buildFeatureConfiguration();
+
+ public static final FeatureConfiguration<List<String>> SUPPORTED_CATALOG_CONNECTION_TYPES =
+ PolarisConfiguration.<List<String>>builder()
+ .key("SUPPORTED_CATALOG_CONNECTION_TYPES")
+ .description("The list of supported catalog connection types for federation")
+ .defaultValue(List.of(ConnectionType.ICEBERG_REST.name(), ConnectionType.HADOOP.name()))
Review Comment:
Yes, we already have a feature gate for this: ["ENABLE_CATALOG_FEDERATION"](https://github.com/apache/polaris/blob/40aea3ac39ac485a76f3aaa930694eeab8ec14d8/quarkus/defaults/src/main/resources/application.properties#L113)
However, at the moment enabling that only risks "broken experimental functionality" but not the security of the server, so the main concern would be if any service providers happen to already be setting `ENABLE_CATALOG_FEDERATION=true` but aren't paying attention to the addition of new federation types.
Perhaps we can at least include the new `SUPPORTED_CATALOG_CONNECTION_TYPES` in application.properties to explicitly list the two types, and maybe with a commented-out version that only includes ICEBERG_REST as well, with some comments about how the reader should choose the configuration. Or make the default one only `[ICEBERG_REST]` with a commented-out version that includes both `[ICEBERG_REST, HADOOP]`.
--
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] Implement federation to HadoopCatalog [polaris]
Posted by "singhpk234 (via GitHub)" <gi...@apache.org>.
singhpk234 commented on code in PR #1466:
URL: https://github.com/apache/polaris/pull/1466#discussion_r2072519291
##########
polaris-core/src/main/java/org/apache/polaris/core/connection/hadoop/HadoopConnectionConfigInfoDpo.java:
##########
@@ -0,0 +1,88 @@
+/*
+ * 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.hadoop;
+
+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 org.apache.iceberg.CatalogProperties;
+import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
+import org.apache.polaris.core.admin.model.HadoopConnectionConfigInfo;
+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.secrets.UserSecretsManager;
+
+/**
+ * The internal persistence-object counterpart to {@link
+ * org.apache.polaris.core.admin.model.HadoopConnectionConfigInfo} defined in the API model.
+ */
+public class HadoopConnectionConfigInfoDpo extends ConnectionConfigInfoDpo {
+
+ private final String warehouse;
+
+ public HadoopConnectionConfigInfoDpo(
+ @JsonProperty(value = "uri", required = true) @Nonnull String uri,
+ @JsonProperty(value = "authenticationParameters", required = true) @Nonnull
Review Comment:
> In my view, it makes sense to just create a HadoopCatalog and pass along whatever properties the user provides when the user has configured Polaris to delegate to a Hadoop catalog.
I agree in principal for this, I was mostly coming from introducing / opening up for some use-case that polaris doesn't support in the moment itself, hence was my consideration, but totally fine if we move ahead.
> I think the concept of federation means that Polaris itself is completely agnostic to the configuration of the remote catalog.
I am not sure if I fully agree with this though, my reasoning is following because Polaris can't just be a pass through, the identity that the federated catalog sees is much more powerful so Polaris should IMHO need to some down scoping or never the less make a way to provide the federated catalog the requester's identity, which in Kreberos world is way different and have to go hand in had with downstream catalog so some co-ordination is required. mostly talking of this : https://github.com/apache/polaris/pull/1026#discussion_r1964494532
May be I am making it more complicated, than its supposed to be at-least this is chain of thoughts that come to my mind, on thinking 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] Implement federation to HadoopCatalog [polaris]
Posted by "eric-maynard (via GitHub)" <gi...@apache.org>.
eric-maynard commented on code in PR #1466:
URL: https://github.com/apache/polaris/pull/1466#discussion_r2075934454
##########
polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java:
##########
@@ -234,4 +235,11 @@ public static void enforceFeatureEnabledOrThrow(
.description("If true, the policy-store endpoints are enabled")
.defaultValue(true)
.buildFeatureConfiguration();
+
+ public static final FeatureConfiguration<List<String>> SUPPORTED_CATALOG_CONNECTION_TYPES =
+ PolarisConfiguration.<List<String>>builder()
+ .key("SUPPORTED_CATALOG_CONNECTION_TYPES")
+ .description("The list of supported catalog connection types for federation")
+ .defaultValue(List.of(ConnectionType.ICEBERG_REST.name(), ConnectionType.HADOOP.name()))
Review Comment:
For now I'll update both the FeatureConfiguration default and the application.properties entry to only include ICEBERG_REST; when we add other federation types we can make a case-by-case evaluation of whether we want to enable those types 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