mchades commented on code in PR #6923:
URL: https://github.com/apache/gravitino/pull/6923#discussion_r2062800804
##########
catalogs/catalog-lakehouse-hudi/src/main/java/org/apache/gravitino/catalog/lakehouse/hudi/backend/hms/kerberos/KerberosClient.java:
##########
Review Comment:
As of now, there are a lot of duplicate code in the Kerberos authentication
for Hadoop catalog, Paimon catalog, and Iceberg catalog.
I think we can refactor it after this PR is merged by adding a Kerberos
authentication framework for code reuse and facilitating the extension of
Kerberos support to other catalogs in the future. WDYT? @yuqi1129 @jerryshao
##########
catalogs/catalog-lakehouse-hudi/src/main/java/org/apache/gravitino/catalog/lakehouse/hudi/backend/hms/kerberos/AuthenticationConfig.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.gravitino.catalog.lakehouse.hudi.backend.hms.kerberos;
+
+import org.apache.gravitino.config.ConfigBuilder;
+import org.apache.gravitino.config.ConfigConstants;
+import org.apache.gravitino.config.ConfigEntry;
+
+public class AuthenticationConfig extends org.apache.gravitino.Config {
+
+ // The key for the authentication type, currently we support Kerberos and
simple
+ public static final String AUTH_TYPE_KEY = "authentication.type";
+
+ public static final String IMPERSONATION_ENABLE_KEY =
"authentication.impersonation-enable";
+
+ enum AuthenticationType {
+ SIMPLE,
+ KERBEROS;
+
+ public static AuthenticationType fromString(String type) {
+ return AuthenticationType.valueOf(type.toUpperCase());
+ }
+ }
+
+ public static final boolean KERBEROS_DEFAULT_IMPERSONATION_ENABLE = false;
+
+ public AuthenticationConfig(java.util.Map<String, String> properties) {
+ super(false);
+ loadFromMap(properties, k -> true);
+ }
+
+ public static final ConfigEntry<String> AUTH_TYPE_ENTRY =
+ new ConfigBuilder(AUTH_TYPE_KEY)
+ .doc(
+ "The type of authentication for Hadoop catalog, currently we
only support simple and Kerberos")
+ .version(ConfigConstants.VERSION_0_10_0)
+ .stringConf()
+ .createWithDefault("simple");
+
+ public static final ConfigEntry<Boolean> ENABLE_IMPERSONATION_ENTRY =
+ new ConfigBuilder(IMPERSONATION_ENABLE_KEY)
+ .doc("Whether to enable impersonation for the Hadoop catalog")
Review Comment:
"Hadoop catalog"?
##########
catalogs/catalog-lakehouse-hudi/src/main/java/org/apache/gravitino/catalog/lakehouse/hudi/backend/hms/kerberos/AuthenticationConfig.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.gravitino.catalog.lakehouse.hudi.backend.hms.kerberos;
+
+import org.apache.gravitino.config.ConfigBuilder;
+import org.apache.gravitino.config.ConfigConstants;
+import org.apache.gravitino.config.ConfigEntry;
+
+public class AuthenticationConfig extends org.apache.gravitino.Config {
+
+ // The key for the authentication type, currently we support Kerberos and
simple
+ public static final String AUTH_TYPE_KEY = "authentication.type";
+
+ public static final String IMPERSONATION_ENABLE_KEY =
"authentication.impersonation-enable";
+
+ enum AuthenticationType {
+ SIMPLE,
+ KERBEROS;
+
+ public static AuthenticationType fromString(String type) {
+ return AuthenticationType.valueOf(type.toUpperCase());
+ }
+ }
+
+ public static final boolean KERBEROS_DEFAULT_IMPERSONATION_ENABLE = false;
+
+ public AuthenticationConfig(java.util.Map<String, String> properties) {
+ super(false);
+ loadFromMap(properties, k -> true);
+ }
+
+ public static final ConfigEntry<String> AUTH_TYPE_ENTRY =
+ new ConfigBuilder(AUTH_TYPE_KEY)
+ .doc(
+ "The type of authentication for Hadoop catalog, currently we
only support simple and Kerberos")
+ .version(ConfigConstants.VERSION_0_10_0)
+ .stringConf()
+ .createWithDefault("simple");
+
+ public static final ConfigEntry<Boolean> ENABLE_IMPERSONATION_ENTRY =
+ new ConfigBuilder(IMPERSONATION_ENABLE_KEY)
+ .doc("Whether to enable impersonation for the Hadoop catalog")
+ .version(ConfigConstants.VERSION_0_10_0)
+ .booleanConf()
+ .createWithDefault(KERBEROS_DEFAULT_IMPERSONATION_ENABLE);
+
+ public String getAuthType() {
+ return get(AUTH_TYPE_ENTRY);
+ }
+
+ public boolean isImpersonationEnabled() {
+ return get(ENABLE_IMPERSONATION_ENTRY);
+ }
+
+ public boolean isSimpleAuth() {
Review Comment:
I don't see the usage of the method; should it be removed?
##########
docs/lakehouse-hudi-catalog.md:
##########
@@ -39,6 +39,21 @@ Tested and verified with Apache Hudi `0.15.0`.
| `client.pool-cache.eviction-interval-ms` | For HMS backend. The cache pool
eviction interval.
| 300000 | No |
0.7.0-incubating |
| `gravitino.bypass.` | Property name with this prefix
passed down to the underlying backend client for use. Such as
`gravitino.bypass.hive.metastore.failure.retries = 3` indicate 3 times of
retries upon failure of Thrift metastore calls for HMS backend. | (none)
| No | 0.7.0-incubating |
+#### Catalog backend security
+
+Users can use the following properties to configure the security of the
catalog backend if needed. For example, if you are using a Kerberos Hive
catalog backend, you must set `authentication.type` to `Kerberos` and provide
`authentication.kerberos.principal` and `authentication.kerberos.keytab-uri`.
+
+| Property name | Description
| Default value |
Required | Since Version
|
+|----------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------|---------------|-------------------------------------------------------------|-------------------|
+| `authentication.type` | The type of
authentication for hudi catalog backend. This configuration only applicable for
for hms backend, and only supports `kerberos`, `simple` currently. | `simple`
| No |
0.10.0-incubating |
+| `authentication.impersonation-enable` | Whether to enable
impersonation for the hudi catalog
| `false` |
No | 0.10.0-incubating
|
+| `authentication.kerberos.principal` | The principal of the
Kerberos authentication
| (none) |
required if the value of `authentication.type` is kerberos. | 0.10.0-incubating
|
+| `authentication.kerberos.keytab-uri` | The URI of The keytab
for the Kerberos authentication.
| (none) |
required if the value of `authentication.type` is kerberos. | 0.10.0-incubating
|
+| `authentication.kerberos.check-interval-sec` | The check interval of
Kerberos credential for hudi catalog.
| 60 | No
| 0.10.0-incubating |
+| `authentication.kerberos.keytab-fetch-timeout-sec` | The fetch timeout of
retrieving Kerberos keytab from `authentication.kerberos.keytab-uri`.
| 60 | No
| 0.10.0-incubating |
+Property name with this prefix passed down to the underlying backend client
for use. Such as
`gravitino.bypass.hive.metastore.kerberos.principal=XXXX`、`gravitino.bypass.hadoop.security.authentication=kerberos`、`gravitino.bypass.hive.metastore.sasl.enabled=ture`
And so on.
Review Comment:
Please add a blank line above.
##########
catalogs/catalog-lakehouse-hudi/src/main/java/org/apache/gravitino/catalog/lakehouse/hudi/backend/hms/kerberos/AuthenticationConfig.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.gravitino.catalog.lakehouse.hudi.backend.hms.kerberos;
+
+import org.apache.gravitino.config.ConfigBuilder;
+import org.apache.gravitino.config.ConfigConstants;
+import org.apache.gravitino.config.ConfigEntry;
+
+public class AuthenticationConfig extends org.apache.gravitino.Config {
+
+ // The key for the authentication type, currently we support Kerberos and
simple
+ public static final String AUTH_TYPE_KEY = "authentication.type";
+
+ public static final String IMPERSONATION_ENABLE_KEY =
"authentication.impersonation-enable";
+
+ enum AuthenticationType {
+ SIMPLE,
+ KERBEROS;
+
+ public static AuthenticationType fromString(String type) {
+ return AuthenticationType.valueOf(type.toUpperCase());
+ }
+ }
+
+ public static final boolean KERBEROS_DEFAULT_IMPERSONATION_ENABLE = false;
+
+ public AuthenticationConfig(java.util.Map<String, String> properties) {
+ super(false);
+ loadFromMap(properties, k -> true);
+ }
+
+ public static final ConfigEntry<String> AUTH_TYPE_ENTRY =
+ new ConfigBuilder(AUTH_TYPE_KEY)
+ .doc(
+ "The type of authentication for Hadoop catalog, currently we
only support simple and Kerberos")
Review Comment:
"Hadoop catalog"?
##########
catalogs/catalog-lakehouse-hudi/src/main/java/org/apache/gravitino/catalog/lakehouse/hudi/backend/hms/kerberos/AuthenticationConfig.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.gravitino.catalog.lakehouse.hudi.backend.hms.kerberos;
+
+import org.apache.gravitino.config.ConfigBuilder;
+import org.apache.gravitino.config.ConfigConstants;
+import org.apache.gravitino.config.ConfigEntry;
+
+public class AuthenticationConfig extends org.apache.gravitino.Config {
+
+ // The key for the authentication type, currently we support Kerberos and
simple
+ public static final String AUTH_TYPE_KEY = "authentication.type";
+
+ public static final String IMPERSONATION_ENABLE_KEY =
"authentication.impersonation-enable";
+
+ enum AuthenticationType {
+ SIMPLE,
+ KERBEROS;
+
+ public static AuthenticationType fromString(String type) {
+ return AuthenticationType.valueOf(type.toUpperCase());
+ }
+ }
+
+ public static final boolean KERBEROS_DEFAULT_IMPERSONATION_ENABLE = false;
+
+ public AuthenticationConfig(java.util.Map<String, String> properties) {
+ super(false);
+ loadFromMap(properties, k -> true);
+ }
+
+ public static final ConfigEntry<String> AUTH_TYPE_ENTRY =
+ new ConfigBuilder(AUTH_TYPE_KEY)
+ .doc(
+ "The type of authentication for Hadoop catalog, currently we
only support simple and Kerberos")
+ .version(ConfigConstants.VERSION_0_10_0)
+ .stringConf()
+ .createWithDefault("simple");
+
+ public static final ConfigEntry<Boolean> ENABLE_IMPERSONATION_ENTRY =
+ new ConfigBuilder(IMPERSONATION_ENABLE_KEY)
+ .doc("Whether to enable impersonation for the Hadoop catalog")
+ .version(ConfigConstants.VERSION_0_10_0)
+ .booleanConf()
+ .createWithDefault(KERBEROS_DEFAULT_IMPERSONATION_ENABLE);
+
+ public String getAuthType() {
+ return get(AUTH_TYPE_ENTRY);
+ }
+
+ public boolean isImpersonationEnabled() {
Review Comment:
I don't see the usage of the method; should it be removed?
##########
catalogs/catalog-lakehouse-hudi/src/main/java/org/apache/gravitino/catalog/lakehouse/hudi/backend/hms/kerberos/AuthenticationConfig.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.gravitino.catalog.lakehouse.hudi.backend.hms.kerberos;
+
+import org.apache.gravitino.config.ConfigBuilder;
+import org.apache.gravitino.config.ConfigConstants;
+import org.apache.gravitino.config.ConfigEntry;
+
+public class AuthenticationConfig extends org.apache.gravitino.Config {
+
+ // The key for the authentication type, currently we support Kerberos and
simple
+ public static final String AUTH_TYPE_KEY = "authentication.type";
+
+ public static final String IMPERSONATION_ENABLE_KEY =
"authentication.impersonation-enable";
+
+ enum AuthenticationType {
+ SIMPLE,
+ KERBEROS;
+
+ public static AuthenticationType fromString(String type) {
Review Comment:
I don't see the usage of the method; should it be removed?
##########
docs/lakehouse-hudi-catalog.md:
##########
@@ -39,6 +39,21 @@ Tested and verified with Apache Hudi `0.15.0`.
| `client.pool-cache.eviction-interval-ms` | For HMS backend. The cache pool
eviction interval.
| 300000 | No |
0.7.0-incubating |
| `gravitino.bypass.` | Property name with this prefix
passed down to the underlying backend client for use. Such as
`gravitino.bypass.hive.metastore.failure.retries = 3` indicate 3 times of
retries upon failure of Thrift metastore calls for HMS backend. | (none)
| No | 0.7.0-incubating |
+#### Catalog backend security
+
+Users can use the following properties to configure the security of the
catalog backend if needed. For example, if you are using a Kerberos Hive
catalog backend, you must set `authentication.type` to `Kerberos` and provide
`authentication.kerberos.principal` and `authentication.kerberos.keytab-uri`.
+
+| Property name | Description
| Default value |
Required | Since Version
|
+|----------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------|---------------|-------------------------------------------------------------|-------------------|
+| `authentication.type` | The type of
authentication for hudi catalog backend. This configuration only applicable for
for hms backend, and only supports `kerberos`, `simple` currently. | `simple`
| No |
0.10.0-incubating |
+| `authentication.impersonation-enable` | Whether to enable
impersonation for the hudi catalog
| `false` |
No | 0.10.0-incubating
|
+| `authentication.kerberos.principal` | The principal of the
Kerberos authentication
| (none) |
required if the value of `authentication.type` is kerberos. | 0.10.0-incubating
|
+| `authentication.kerberos.keytab-uri` | The URI of The keytab
for the Kerberos authentication.
| (none) |
required if the value of `authentication.type` is kerberos. | 0.10.0-incubating
|
+| `authentication.kerberos.check-interval-sec` | The check interval of
Kerberos credential for hudi catalog.
| 60 | No
| 0.10.0-incubating |
+| `authentication.kerberos.keytab-fetch-timeout-sec` | The fetch timeout of
retrieving Kerberos keytab from `authentication.kerberos.keytab-uri`.
| 60 | No
| 0.10.0-incubating |
+Property name with this prefix passed down to the underlying backend client
for use. Such as
`gravitino.bypass.hive.metastore.kerberos.principal=XXXX`、`gravitino.bypass.hadoop.security.authentication=kerberos`、`gravitino.bypass.hive.metastore.sasl.enabled=ture`
And so on.
Review Comment:
You should separate the prefixes of these properties and describe them in
the table accordingly.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
