[#3337] improvement(hadoop-catalog): Support user impersonation for Hadoop catalog. by yuqi1129 · Pull Request #3352 · apache/gravitino

yuqi1129 · 2024-05-11T13:59:57Z

What changes were proposed in this pull request?

Add user impersonation for the Hadoop catalog.

Why are the changes needed?

We need authentication for the encrypted HDFS cluster.

Fix: #3337

Does this PR introduce any user-facing change?

N/A.

How was this patch tested?

UT(TO add).

...ommon/src/main/java/com/datastrato/gravitino/hadoop/authentication/AuthenticationConfig.java

jerryshao · 2024-05-20T06:51:16Z

Is it ready for review?

yuqi1129 · 2024-05-20T06:54:36Z

Is it ready for review?

I'm afraid we need to add some tests using the HDFS cluster, not just a mini cluster here. If this does not matter, I think it's ready for review.

jerryshao · 2024-05-20T07:06:24Z

We could have a separate PR for e2e test, using mock test here to cover the logic should be enough.

catalogs/hadoop-compatible-common/build.gradle.kts

yuqi1129 · 2024-05-21T09:44:55Z

The following point needs clarification:

After introducing a common module that handles Kerberos authentication, the Hive catalog will depend on hadoop3(see below), is it acceptable? I think it's a bit risky to use a different release version for the Hive catalog.

If the first option is not desirable, I believe we may not be able to share code between modules that use Kerberos authentication, can we accept duplicated code of Kerberos code logic here?

@jerryshao
What's your opinion? I'm looking forward to your kind reply, thank you.

jerryshao · 2024-05-21T10:53:00Z

I would choose the option 2 as the bottom line. If we have a better solution to avoid code duplication while not changing Hadoop version, that would be better.

yuqi1129 · 2024-05-21T14:03:14Z

@qqqttt123 @jerryshao
The code has been updated, please help to review it if you have time, thanks.

yuqi1129 · 2024-05-23T11:38:29Z

@jerryshao
Please help to review this pr, The Hadoop e2e test depends on this PR.

…_3337

yuqi1129 · 2024-05-29T02:21:43Z

All resolved, please help to double-check if it's okay now.
@qqqttt123

...og-hadoop/src/main/java/com/datastrato/gravitino/catalog/hadoop/kerberos/KerberosClient.java

.../catalog-hadoop/src/main/java/com/datastrato/gravitino/catalog/hadoop/HadoopProxyPlugin.java

roryqi · 2024-05-29T06:59:23Z

You need to notice this apache/uniffle#824

roryqi · 2024-05-29T07:06:01Z

What configurations do you need if you support IAM? Could you configure it with kerberos at the same time?
I also have concern the support of fileset Kerberos. We can't use keytabs of different KDC in the same catalog.

yuqi1129 · 2024-05-29T07:25:24Z

What configurations do you need if you support IAM? Could you configure it with kerberos at the same time? I also have concern the support of fileset Kerberos. We can't use keytabs of different KDC in the same catalog.

I would prefer to use another PR solution to support multiple KDCs and multiple tables in the Hadoop catalogs, I think the Hive catalog also needs to handle this issue.

I may take some time to think about it and complete it. Currently, we don't have enough time to handle it in this version.

roryqi · 2024-05-29T07:29:25Z

What configurations do you need if you support IAM? Could you configure it with kerberos at the same time? I also have concern the support of fileset Kerberos. We can't use keytabs of different KDC in the same catalog.

I would prefer to use another PR solution to support multiple KDCs and multiple key tables in the Hadoop catalogs, I think the Hive catalog also needs to handle this issue.

I may take some time to think about it and complete it. Currently, we don't have enough time to handle it in this version.

Hive catalog doesn't need to handle this.

yuqi1129 · 2024-05-29T08:36:31Z

What configurations do you need if you support IAM? Could you configure it with kerberos at the same time? I also have concern the support of fileset Kerberos. We can't use keytabs of different KDC in the same catalog.

I would prefer to use another PR solution to support multiple KDCs and multiple key tables in the Hadoop catalogs, I think the Hive catalog also needs to handle this issue.
I may take some time to think about it and complete it. Currently, we don't have enough time to handle it in this version.

Hive catalog doesn't need to handle this.

I have added the following keys and other according changes:

public static final String AUTH_TYPE_KEY = "hadoop.authentication.type";
public static final String ENABLE_AUTH_KEY = "hadoop.authentication.enable";

roryqi · 2024-05-29T09:15:27Z

...oop/src/main/java/com/datastrato/gravitino/catalog/hadoop/kerberos/AuthenticationConfig.java

+          .put(
+              AUTH_TYPE_KEY,
+              PropertyEntry.stringImmutablePropertyEntry(
+                  AUTH_TYPE_KEY, "The uri of key tab for the catalog", false, null, false, false))


The comment isn't right.

roryqi · 2024-05-29T09:18:43Z

...oop/src/main/java/com/datastrato/gravitino/catalog/hadoop/kerberos/AuthenticationConfig.java

+import java.util.Map;
+
+public class AuthenticationConfig extends Config {
+  public static final String AUTH_TYPE_KEY = "hadoop.authentication.type";


Maybe we should use authentication.type.

This is the configuration of Hadoop catalog, why do we need to remove hadoop here?

Hadoop catalog properties don't start with hadoop. You can refer to location. hadoop is rebudant information.

Let me check it.

roryqi · 2024-05-29T09:19:59Z

...og-hadoop/src/main/java/com/datastrato/gravitino/catalog/hadoop/kerberos/KerberosConfig.java

+import org.apache.commons.lang3.StringUtils;
+
+public class KerberosConfig extends AuthenticationConfig {
+  public static final String KEY_TAB_URI_KEY = "kerberos.keytab-uri";


authentication.kerberos.keytab-uri?

This makes sense.

roryqi · 2024-05-29T09:39:46Z

LGTM, @jerryshao Could you check whether the config options are ok?

… for Hadoop catalog. (apache#3352) ### What changes were proposed in this pull request? Add user impersonation for the Hadoop catalog. ### Why are the changes needed? We need authentication for the encrypted HDFS cluster. Fix: apache#3337 ### Does this PR introduce _any_ user-facing change? N/A. ### How was this patch tested? UT(TO add).

yuqi1129 marked this pull request as draft May 11, 2024 14:00

yuqi1129 marked this pull request as ready for review May 15, 2024 12:20

qqqttt123 reviewed May 15, 2024

View reviewed changes

...ommon/src/main/java/com/datastrato/gravitino/hadoop/authentication/AuthenticationConfig.java Outdated Show resolved Hide resolved

yuqi1129 self-assigned this May 20, 2024

jerryshao reviewed May 20, 2024

View reviewed changes

catalogs/hadoop-compatible-common/build.gradle.kts Outdated Show resolved Hide resolved

jerryshao reviewed May 20, 2024

View reviewed changes

catalogs/hadoop-compatible-common/build.gradle.kts Outdated Show resolved Hide resolved

Refresh code.

1c4cc87

yuqi1129 force-pushed the issue_3337 branch from 5e5f607 to 1c4cc87 Compare May 21, 2024 13:30

yuqi1129 added 2 commits May 23, 2024 19:36

Merge main and resolve conflicts.

33eccc9

Merge branch 'main' of github.com:datastrato/gravitino into issue_3337

01a0ea6

yuqi1129 added 3 commits May 23, 2024 21:21

fix

f6a1754

fix

6693d3d

Merge branch 'main' into issue_3337

a234732

yuqi1129 closed this May 23, 2024

yuqi1129 reopened this May 23, 2024

yuqi1129 added the debug action label May 24, 2024

yuqi1129 closed this May 24, 2024

yuqi1129 reopened this May 24, 2024

yuqi1129 added 2 commits May 24, 2024 10:02

fix

b3cfc91

Merge branch 'issue_3337' of github.com:yuqi1129/gravitino into issue…

9a1da22

…_3337

qqqttt123 removed the debug action label May 24, 2024

qqqttt123 closed this May 24, 2024

qqqttt123 reopened this May 24, 2024

yuqi1129 added 2 commits May 29, 2024 10:14

Fix

819b270

Merge branch 'main' of github.com:datastrato/gravitino into issue_3337

7bb136a

roryqi reviewed May 29, 2024

View reviewed changes

...og-hadoop/src/main/java/com/datastrato/gravitino/catalog/hadoop/kerberos/KerberosClient.java Outdated Show resolved Hide resolved

Fix comment again.

9d974a2

yuqi1129 closed this May 29, 2024

yuqi1129 reopened this May 29, 2024

roryqi reviewed May 29, 2024

View reviewed changes

.../catalog-hadoop/src/main/java/com/datastrato/gravitino/catalog/hadoop/HadoopProxyPlugin.java Outdated Show resolved Hide resolved

fix

25a1d03

Fix

33415c8

Fix CI

93c1be1

roryqi reviewed May 29, 2024

View reviewed changes

Fix mistake about description of AUTH_TYPE_KEY

180c206

roryqi reviewed May 29, 2024

View reviewed changes

fix comments

87574a3

qqqttt123 approved these changes May 29, 2024

View reviewed changes

yuqi1129 closed this May 29, 2024

yuqi1129 reopened this May 29, 2024

Merge branch 'main' into issue_3337

617b485

yuqi1129 merged commit 783839f into apache:main May 29, 2024

yuqi1129 deleted the issue_3337 branch May 29, 2024 13:39

yuqi1129 mentioned this pull request May 30, 2024

[Improvement] Add document of Hadoop catalog user authentication #3658

Closed

Conversation

yuqi1129 commented May 11, 2024

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

Uh oh!

jerryshao commented May 20, 2024

Uh oh!

yuqi1129 commented May 20, 2024

Uh oh!

jerryshao commented May 20, 2024

Uh oh!

Uh oh!

Uh oh!

yuqi1129 commented May 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jerryshao commented May 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yuqi1129 commented May 21, 2024

Uh oh!

yuqi1129 commented May 23, 2024

Uh oh!

yuqi1129 commented May 29, 2024

Uh oh!

Uh oh!

Uh oh!

roryqi commented May 29, 2024

Uh oh!

roryqi commented May 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yuqi1129 commented May 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

roryqi commented May 29, 2024 • edited by yuqi1129 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yuqi1129 commented May 29, 2024

Uh oh!

roryqi May 29, 2024

Choose a reason for hiding this comment

Uh oh!

roryqi May 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yuqi1129 May 29, 2024

Choose a reason for hiding this comment

Uh oh!

roryqi May 29, 2024

Choose a reason for hiding this comment

Uh oh!

yuqi1129 May 29, 2024

Choose a reason for hiding this comment

Uh oh!

roryqi May 29, 2024

Choose a reason for hiding this comment

Uh oh!

yuqi1129 May 29, 2024

Choose a reason for hiding this comment

Uh oh!

roryqi commented May 29, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

yuqi1129 commented May 21, 2024 •

edited

Loading

jerryshao commented May 21, 2024 •

edited

Loading

roryqi commented May 29, 2024 •

edited

Loading

yuqi1129 commented May 29, 2024 •

edited

Loading

roryqi commented May 29, 2024 •

edited by yuqi1129

Loading

roryqi May 29, 2024 •

edited

Loading