From c8b03943cd5b604eaf89e2ec62e171c08220a325 Mon Sep 17 00:00:00 2001
From: zhouyifan279 <zhouyifan279@gmail.com>
Date: Mon, 7 Nov 2022 22:16:03 +0800
Subject: [PATCH] [KYUUBI #3744][Bug] InvalidACL appears in engine after
 upgrade to Kyuubi 1.6.0

Fix #3744

- [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible

- [ ] Add screenshots for manual tests if appropriate

- [x] [Run test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request

Closes #3771 from zhouyifan279/3744.

Closes #3744

58764296 [zhouyifan279] [KYUUBI #3744][Bug] InvalidACL appears in engine after upgrade to Kyuubi 1.6.0
554e8898 [zhouyifan279] [KYUUBI #3744][Bug] InvalidACL appears in engine after upgrade to Kyuubi 1.6.0
a6bfa3d4 [zhouyifan279] [KYUUBI #3744][Bug] InvalidACL appears in engine after upgrade to Kyuubi 1.6.0
c90470f4 [zhouyifan279] [KYUUBI #3744][Bug] InvalidACL appears in engine after upgrade to Kyuubi 1.6.0
fe55f4ad [zhouyifan279] [KYUUBI #3744][Bug] InvalidACL appears in engine after upgrade to Kyuubi 1.6.0
e262872a [zhouyifan279] [KYUUBI #3744][Bug] InvalidACL appears in engine after upgrade to Kyuubi 1.6.0
ed5e8bda [zhouyifan279] [KYUUBI #3744][Bug] InvalidACL appears in engine after upgrade to Kyuubi 1.6.0

Authored-by: zhouyifan279 <zhouyifan279@gmail.com>
Signed-off-by: Cheng Pan <chengpan@apache.org>
---
 docs/deployment/index.rst                     |  3 +-
 docs/deployment/migration-guide.md            | 30 +++++++++++++++++++
 .../kyuubi/ha/HighAvailabilityConf.scala      |  4 ++-
 .../spark/SparkProcessBuilderSuite.scala      |  2 +-
 4 files changed, 36 insertions(+), 3 deletions(-)
 create mode 100644 docs/deployment/migration-guide.md

diff --git a/docs/deployment/index.rst b/docs/deployment/index.rst
index e68268088d6..ffc8c167b4e 100644
--- a/docs/deployment/index.rst
+++ b/docs/deployment/index.rst
@@ -29,6 +29,7 @@ Basics
     kyuubi_on_kubernetes
     hive_metastore
     high_availability_guide
+    migration-guide
 
 Configurations
 --------------
@@ -50,4 +51,4 @@ Engines
     engine_on_kubernetes
     engine_share_level
     engine_lifecycle
-    spark/index
\ No newline at end of file
+    spark/index
diff --git a/docs/deployment/migration-guide.md b/docs/deployment/migration-guide.md
new file mode 100644
index 00000000000..067932cc071
--- /dev/null
+++ b/docs/deployment/migration-guide.md
@@ -0,0 +1,30 @@
+<!--
+ - 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.
+ -->
+
+
+# Kyuubi Migration Guide
+
+## Upgrading from Kyuubi 1.6.0 to 1.6.1
+* Since Kyuubi 1.6.1, `kyuubi.ha.zookeeper.engine.auth.type` does not fallback to `kyuubi.ha.zookeeper.auth.type`.  
+  When Kyuubi engine does Kerberos authentication with Zookeeper, user needs to explicitly set `kyuubi.ha.zookeeper.engine.auth.type` to `KERBEROS`.
+
+## Upgrading from Kyuubi 1.5 to 1.6
+* Kyuubi engine gets Zookeeper principal & keytab from `kyuubi.ha.zookeeper.auth.principal` & `kyuubi.ha.zookeeper.auth.keytab`.    
+  `kyuubi.ha.zookeeper.auth.principal` & `kyuubi.ha.zookeeper.auth.keytab` fallback to `kyuubi.kinit.principal` & `kyuubi.kinit.keytab` when not set.    
+  Since Kyuubi 1.6, `kyuubi.kinit.principal` & `kyuubi.kinit.keytab` are filtered out from Kyuubi engine's conf for better security.  
+  When Kyuubi engine does Kerberos authentication with Zookeeper, user needs to explicitly set `kyuubi.ha.zookeeper.auth.principal` & `kyuubi.ha.zookeeper.auth.keytab`.
+
diff --git a/kyuubi-ha/src/main/scala/org/apache/kyuubi/ha/HighAvailabilityConf.scala b/kyuubi-ha/src/main/scala/org/apache/kyuubi/ha/HighAvailabilityConf.scala
index baa741abd3f..d33dccf982f 100644
--- a/kyuubi-ha/src/main/scala/org/apache/kyuubi/ha/HighAvailabilityConf.scala
+++ b/kyuubi-ha/src/main/scala/org/apache/kyuubi/ha/HighAvailabilityConf.scala
@@ -88,7 +88,9 @@ object HighAvailabilityConf {
       .doc("The type of zookeeper authentication for engine, all candidates are " +
         s"${AuthTypes.values.mkString("<ul><li>", "</li><li> ", "</li></ul>")}")
       .version("1.3.2")
-      .fallbackConf(HA_ZK_AUTH_TYPE)
+      .stringConf
+      .checkValues(AuthTypes.values.map(_.toString))
+      .createWithDefault(AuthTypes.NONE.toString)
 
   val HA_ZK_AUTH_PRINCIPAL: ConfigEntry[Option[String]] =
     buildConf("kyuubi.ha.zookeeper.auth.principal")
diff --git a/kyuubi-server/src/test/scala/org/apache/kyuubi/engine/spark/SparkProcessBuilderSuite.scala b/kyuubi-server/src/test/scala/org/apache/kyuubi/engine/spark/SparkProcessBuilderSuite.scala
index a4f33dde114..42c94e939a0 100644
--- a/kyuubi-server/src/test/scala/org/apache/kyuubi/engine/spark/SparkProcessBuilderSuite.scala
+++ b/kyuubi-server/src/test/scala/org/apache/kyuubi/engine/spark/SparkProcessBuilderSuite.scala
@@ -252,7 +252,7 @@ class SparkProcessBuilderSuite extends KerberizedTestHelper with MockitoSugar {
 
   test("zookeeper kerberos authentication") {
     val conf = KyuubiConf()
-    conf.set(HighAvailabilityConf.HA_ZK_AUTH_TYPE.key, AuthTypes.KERBEROS.toString)
+    conf.set(HighAvailabilityConf.HA_ZK_ENGINE_AUTH_TYPE.key, AuthTypes.KERBEROS.toString)
     conf.set(HighAvailabilityConf.HA_ZK_AUTH_KEYTAB.key, testKeytab)
     conf.set(HighAvailabilityConf.HA_ZK_AUTH_PRINCIPAL.key, testPrincipal)