[KYUUBI #6003] Allow disabling user impersonation on launching engine #6003

pan3793 · 2024-01-22T01:24:48Z

🔍 Description

Issue References 🔗

HiveServer2 has a configuration hive.server2.enable.doAs to control the execution user between the session user and the server user, Kyuubi's CONNECTION and USER share levels always perform like doAs enabled do. In CDH 5/6, this is disabled by default, users who want to migrate from CDH to Kyuubi may encounter permission issues with the current implementation.

Describe Your Solution 🔧

This pull request introduces a new configuration kyuubi.engine.doAs.enabled to allow enable/disable user impersonation on launching engine. For security purpose, it's not allowed to be overridden by session conf.

The change in this PR has certain limitations:

only supports Spark engine
only supports interactive mode, specifically, it does not take effect on Spark batch mode now.

Types of changes 🔖

Bugfix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Test Plan 🧪

The first step is passing all existing UTs when kyuubi.engine.doAs.enabled=true.

Tested on internal Kerberized-environment, when kyuubi.engine.share.level=CONNECTION and kyuubi.engine.doAs.enabled=false, use user 'spark' to launch engine, and the engine submitted without --proxy-user spark, thus engine launched by server user hive, then run select session_user(), current_user() and returns

+-----------------+-----------------+
| session_user()  | current_user()  |
+-----------------+-----------------+
| spark           | hive            |
+-----------------+-----------------+

And I checked the spark.app.name and registered path on Zookeeper also expected.

+-----------------+--------------------------------------------------------------------------+
|       key       |                       value                                              |
+-----------------+--------------------------------------------------------------------------+
| spark.app.name  | kyuubi_USER_SPARK_SQL_spark_default_51a416e5-6023-4bac-a964-cd9605f17c61 |
+-----------------+--------------------------------------------------------------------------+

Checklist 📝

This patch was not authored or co-authored using Generative Tooling

Be nice. Be informative.

pan3793 · 2024-01-22T02:04:03Z

kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala

@@ -2087,20 +2087,33 @@ object KyuubiConf {
      .version("1.5.0")
      .fallbackConf(ENGINE_CONNECTION_URL_USE_HOSTNAME)

+  val ENGINE_DO_AS_ENABLED: ConfigEntry[Boolean] =
+    buildConf("kyuubi.engine.doAs.enabled")


cc @yaooqinn @zhouyifan279 WDYT of this configuration name? My another candidate is kyuubi.engine.impersonation.enabled

+1 for kyuubi.engine.doAs.enabled.

codecov-commenter · 2024-01-22T03:19:31Z

Codecov Report

Attention: 34 lines in your changes are missing coverage. Please review.

Comparison is base (d474768) 61.10% compared to head (add20fd) 61.09%.

❗ Current head add20fd differs from pull request most recent head c4002fe. Consider uploading reports for the commit c4002fe to get more accurate results

Files	Patch %	Lines
...ain/scala/org/apache/kyuubi/engine/EngineRef.scala	34.61%	11 Missing and 6 partials ⚠️
...ache/kyuubi/engine/spark/SparkProcessBuilder.scala	28.57%	5 Missing and 5 partials ⚠️
...apache/kyuubi/engine/chat/ChatProcessBuilder.scala	0.00%	3 Missing ⚠️
...apache/kyuubi/engine/hive/HiveProcessBuilder.scala	60.00%	2 Missing ⚠️
...yuubi/engine/hive/HiveYarnModeProcessBuilder.scala	50.00%	1 Missing ⚠️
.../org/apache/kyuubi/session/KyuubiSessionImpl.scala	75.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #6003      +/-   ##
============================================
- Coverage     61.10%   61.09%   -0.02%     
  Complexity       23       23              
============================================
  Files           623      623              
  Lines         37103    37136      +33     
  Branches       5029     5031       +2     
============================================
+ Hits          22672    22687      +15     
- Misses        11981    12001      +20     
+ Partials       2450     2448       -2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

kyuubi-server/src/main/scala/org/apache/kyuubi/engine/spark/SparkProcessBuilder.scala

pan3793 · 2024-01-22T04:30:30Z

kyuubi-server/src/main/scala/org/apache/kyuubi/engine/EngineRef.scala

@@ -97,6 +98,8 @@ private[kyuubi] class EngineRef(
    case _ => user
  }

+  private[kyuubi] val effectiveUser: String = if (doAsEnabled) appUser else Utils.currentUser


there are three user variables in this class now,

user: session user

appUser: for engine routing

effectiveUser: for engine launching

those name are not much straightforward, maybe I need to rename them to

user => sessionUser

appUser => routingUser

effectiveUser => appUser

thought?

+1. It's better to let appUser be consistent with the user on YARN.

yaooqinn · 2024-01-22T06:09:14Z

kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala

+        s"`kyuubi.engine.share.level`, different users will be used to launch the engine. " +
+        "Otherwise, Kyuubi Server's user will always be used to launch the engine.")
+      .version("1.8.1")
+      .booleanConf


Let's add this to 1.9.0 instead of a bugfix version

zhouyifan279 · 2024-01-22T07:18:56Z

kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala

@@ -2087,20 +2087,33 @@ object KyuubiConf {
      .version("1.5.0")
      .fallbackConf(ENGINE_CONNECTION_URL_USE_HOSTNAME)

+  val ENGINE_DO_AS_ENABLED: ConfigEntry[Boolean] =
+    buildConf("kyuubi.engine.doAs.enabled")


+1 for kyuubi.engine.doAs.enabled.

zhouyifan279 · 2024-01-22T07:36:33Z

kyuubi-server/src/main/scala/org/apache/kyuubi/engine/spark/SparkProcessBuilder.scala

          warn(s"The session proxy user: $proxyUser is not same with " +
            s"spark principal: ${ugi.getShortUserName}, so we can't support use keytab. " +
            s"Fallback to use proxy user.")
          None
+        } else if (!doAsEnabled && ugi.getShortUserName != Utils.currentUser) {
+          warn(s"The server's user: ${Utils.currentUser} is not same with " +
+            s"spark principal: ${ugi.getShortUserName}, skip to use keytab.")


Change skip to use keytab. to Fallback to use server's user. can avoid misunderstanding.

updated. I kept both

kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala

wForget

LGTM

zhouyifan279 · 2024-01-29T12:14:48Z

kyuubi-server/src/main/scala/org/apache/kyuubi/engine/spark/SparkProcessBuilder.scala

          warn(s"The session proxy user: $proxyUser is not same with " +
            s"spark principal: ${ugi.getShortUserName}, so we can't support use keytab. " +
            s"Fallback to use proxy user.")
          None
+        } else if (!doAsEnabled && ugi.getShortUserName != Utils.currentUser) {
+          warn(s"The server's user: ${Utils.currentUser} is not same with " +
+            s"spark principal: ${ugi.getShortUserName}, skip to use keytab. " +


Should be skip using keytab

zhouyifan279 · 2024-01-29T12:20:32Z

LGTM

pan3793 · 2024-01-29T13:31:03Z

Thanks, merged to master

…engine # 🔍 Description ## Issue References 🔗 HiveServer2 has a configuration `hive.server2.enable.doAs` to control the execution user between the session user and the server user, Kyuubi's CONNECTION and USER share levels always perform like doAs enabled do. In CDH 5/6, this is disabled by default, users who want to migrate from CDH to Kyuubi may encounter permission issues with the current implementation. ## Describe Your Solution 🔧 This pull request introduces a new configuration `kyuubi.engine.doAs.enabled` to allow enable/disable user impersonation on launching engine. For security purpose, it's not allowed to be overridden by session conf. The change in this PR has certain limitations: - only supports Spark engine - only supports interactive mode, specifically, it does not take effect on Spark batch mode now. ## Types of changes 🔖 - [ ] Bugfix (non-breaking change which fixes an issue) - [x] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) ## Test Plan 🧪 The first step is passing all existing UTs when `kyuubi.engine.doAs.enabled=true`. Tested on internal Kerberized-environment, when `kyuubi.engine.share.level=CONNECTION` and `kyuubi.engine.doAs.enabled=false`, use user 'spark' to launch engine, and the engine submitted without `--proxy-user spark`, thus engine launched by server user `hive`, then run `select session_user(), current_user()` and returns ``` +-----------------+-----------------+ | session_user() | current_user() | +-----------------+-----------------+ | spark | hive | +-----------------+-----------------+ ``` And I checked the `spark.app.name` and registered path on Zookeeper also expected. ``` +-----------------+--------------------------------------------------------------------------+ | key | value | +-----------------+--------------------------------------------------------------------------+ | spark.app.name | kyuubi_USER_SPARK_SQL_spark_default_51a416e5-6023-4bac-a964-cd9605f17c61 | +-----------------+--------------------------------------------------------------------------+ ``` --- # Checklist 📝 - [x] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html) **Be nice. Be informative.** Closes apache#6003 from pan3793/doas. Closes apache#6003 c4002fe [Cheng Pan] grammar add20fd [Cheng Pan] nit 8711c22 [Cheng Pan] address comment 033a322 [Cheng Pan] 1.9.0 9273b94 [Cheng Pan] fix a1563e1 [Cheng Pan] HadoopCredentialsManager e982e23 [Cheng Pan] Allow disable user impersonation on launching engine Authored-by: Cheng Pan <[email protected]> Signed-off-by: Cheng Pan <[email protected]>

github-actions bot added kind:documentation Documentation is a feature! module:server module:common labels Jan 22, 2024

pan3793 commented Jan 22, 2024

View reviewed changes

pan3793 changed the title ~~Allow disable user impersonation on launching engine~~ Allow disabling user impersonation on launching engine Jan 22, 2024

github-actions bot added module:kubernetes module:integration-tests labels Jan 22, 2024

wForget reviewed Jan 22, 2024

View reviewed changes

kyuubi-server/src/main/scala/org/apache/kyuubi/engine/spark/SparkProcessBuilder.scala Show resolved Hide resolved

pan3793 commented Jan 22, 2024

View reviewed changes

yaooqinn reviewed Jan 22, 2024

View reviewed changes

pan3793 changed the title ~~Allow disabling user impersonation on launching engine~~ [KYUUBI #6003] Allow disabling user impersonation on launching engine Jan 22, 2024

zhouyifan279 reviewed Jan 22, 2024

View reviewed changes

wForget reviewed Jan 25, 2024

View reviewed changes

kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala Outdated Show resolved Hide resolved

pan3793 added 5 commits January 29, 2024 14:24

Allow disable user impersonation on launching engine

e982e23

HadoopCredentialsManager

a1563e1

fix

9273b94

1.9.0

033a322

address comment

8711c22

pan3793 force-pushed the doas branch from d1e48e7 to 8711c22 Compare January 29, 2024 07:28

nit

add20fd

pan3793 requested review from wForget and zhouyifan279 January 29, 2024 09:24

wForget approved these changes Jan 29, 2024

View reviewed changes

pan3793 self-assigned this Jan 29, 2024

pan3793 added this to the v1.9.0 milestone Jan 29, 2024

zhouyifan279 reviewed Jan 29, 2024

View reviewed changes

grammar

c4002fe

pan3793 closed this in 3f993f4 Jan 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[KYUUBI #6003] Allow disabling user impersonation on launching engine #6003

[KYUUBI #6003] Allow disabling user impersonation on launching engine #6003

pan3793 commented Jan 22, 2024 •

edited

Loading

pan3793 Jan 22, 2024

zhouyifan279 Jan 22, 2024

codecov-commenter commented Jan 22, 2024 •

edited

Loading

pan3793 Jan 22, 2024

zhouyifan279 Jan 22, 2024

yaooqinn Jan 22, 2024

pan3793 Jan 22, 2024

zhouyifan279 Jan 22, 2024

zhouyifan279 Jan 22, 2024

pan3793 Jan 29, 2024

wForget left a comment

zhouyifan279 Jan 29, 2024

zhouyifan279 commented Jan 29, 2024

pan3793 commented Jan 29, 2024

[KYUUBI #6003] Allow disabling user impersonation on launching engine #6003

[KYUUBI #6003] Allow disabling user impersonation on launching engine #6003

Conversation

pan3793 commented Jan 22, 2024 • edited Loading

🔍 Description

Issue References 🔗

Describe Your Solution 🔧

Types of changes 🔖

Test Plan 🧪

Checklist 📝

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Jan 22, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wForget left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhouyifan279 commented Jan 29, 2024

pan3793 commented Jan 29, 2024

pan3793 commented Jan 22, 2024 •

edited

Loading

codecov-commenter commented Jan 22, 2024 •

edited

Loading