Skip to content

Commit 0ed865f

Browse files
committed
[KYUUBI #2084][FOLLOWUP] Support arbitrary parameters for KyuubiConf
### _Why are the changes needed?_ In this PR, we narrow the scope of 8f15622 to support arbitrary parameters which configured in kyuubi-defaults.conf only. We shall avoid propagating all `sys.props` of Kyuubi server to engine side, which is a dangerous behavior ### _How was this patch tested?_ - [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible - [ ] Add screenshots for manual tests if appropriate - [ ] [Run test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request Closes #2283 from yaooqinn/2084. Closes #2084 434db31 [Kent Yao] fix flink bdd983d [Kent Yao] [KYUUBI #2084][FOLLOWUP] Support arbitrary parameters for KyuubiConf 78bada0 [Kent Yao] [KYUUBI #2084][FOLLOWUP] Support arbitrary parameters for KyuubiConf 55e6fdf [Kent Yao] [KYUUBI #2084][FOLLOWUP] Support arbitrary parameters for KyuubiConf b2239bc [Kent Yao] [KYUUBI #2084][FOLLOWUP] Support arbitrary parameters for KyuubiConf Authored-by: Kent Yao <yao@apache.org> Signed-off-by: Kent Yao <yao@apache.org>
1 parent 19f1d41 commit 0ed865f

File tree

5 files changed

+20
-42
lines changed

5 files changed

+20
-42
lines changed

docs/deployment/settings.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,6 @@ Key | Default | Meaning | Type | Since
313313

314314
Key | Default | Meaning | Type | Since
315315
--- | --- | --- | --- | ---
316-
<code>kyuubi.server.conf.ignore.prefix.list</code>|<div style='width: 65pt;word-wrap: break-word;white-space: normal'>java,sun,os,jdk</div>|<div style='width: 170pt;word-wrap: break-word;white-space: normal'>A comma separated list of ignored keys prefix. If kyuubi conf contains any of them, the key and the corresponding value will be removed silently during server setup. Note that this rule is for server-side protection defined via administrators to prevent some essential configs from tampering but will not forbid users to set dynamic configurations via SET syntax.</div>|<div style='width: 30pt'>seq</div>|<div style='width: 20pt'>1.6.0</div>
317316
<code>kyuubi.server.name</code>|<div style='width: 65pt;word-wrap: break-word;white-space: normal'>&lt;undefined&gt;</div>|<div style='width: 170pt;word-wrap: break-word;white-space: normal'>The name of Kyuubi Server.</div>|<div style='width: 30pt'>string</div>|<div style='width: 20pt'>1.5.0</div>
318317

319318

externals/kyuubi-flink-sql-engine/src/main/scala/org/apache/kyuubi/engine/flink/FlinkSQLEngine.scala

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,8 @@ import org.apache.flink.table.client.SqlClientException
3131
import org.apache.flink.table.client.gateway.context.DefaultContext
3232
import org.apache.flink.util.JarUtils
3333

34-
import org.apache.kyuubi.{KyuubiSQLException, Logging}
35-
import org.apache.kyuubi.Utils.{addShutdownHook, FLINK_ENGINE_SHUTDOWN_PRIORITY}
36-
import org.apache.kyuubi.Utils.currentUser
34+
import org.apache.kyuubi.{KyuubiSQLException, Logging, Utils}
35+
import org.apache.kyuubi.Utils.{addShutdownHook, currentUser, FLINK_ENGINE_SHUTDOWN_PRIORITY}
3736
import org.apache.kyuubi.config.KyuubiConf
3837
import org.apache.kyuubi.engine.flink.FlinkSQLEngine.{countDownLatch, currentEngine}
3938
import org.apache.kyuubi.service.Serverable
@@ -76,8 +75,10 @@ object FlinkSQLEngine extends Logging {
7675
try {
7776
val flinkConfDir = CliFrontend.getConfigurationDirectoryFromEnv
7877
val flinkConf = GlobalConfiguration.loadConfiguration(flinkConfDir)
79-
val flinkConfFromKyuubi = kyuubiConf.getAllWithPrefix("flink", "")
80-
flinkConf.addAll(Configuration.fromMap(flinkConfFromKyuubi.asJava))
78+
val flinkConfFromSys =
79+
Utils.getSystemProperties.filterKeys(_.startsWith("flink."))
80+
.map { case (k, v) => (k.stripPrefix("flink."), v) }
81+
flinkConf.addAll(Configuration.fromMap(flinkConfFromSys.asJava))
8182

8283
val executionTarget = flinkConf.getString(DeploymentOptions.TARGET)
8384
// set cluster name for per-job and application mode

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

Lines changed: 6 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -25,27 +25,21 @@ import java.util.regex.Pattern
2525
import scala.collection.JavaConverters._
2626

2727
import org.apache.kyuubi.{Logging, Utils}
28+
import org.apache.kyuubi.config.KyuubiConf._
2829
import org.apache.kyuubi.engine.{EngineType, ShareLevel}
2930
import org.apache.kyuubi.service.authentication.{AuthTypes, SaslQOP}
3031

3132
case class KyuubiConf(loadSysDefault: Boolean = true) extends Logging {
32-
import KyuubiConf._
3333

3434
private val settings = new ConcurrentHashMap[String, String]()
3535
private lazy val reader: ConfigProvider = new ConfigProvider(settings)
36-
37-
if (loadSysDefault) {
38-
loadFromMap()
36+
private def loadFromMap(props: Map[String, String]): Unit = {
37+
settings.putAll(props.asJava)
3938
}
4039

41-
private def loadFromMap(props: Map[String, String] = Utils.getSystemProperties): KyuubiConf = {
42-
43-
val ignorePrefixList = props.get(SERVER_CONF_IGNORE_PREFIX_LIST.key)
44-
.map(_.split(",").map(_.trim).toSeq).getOrElse(get(SERVER_CONF_IGNORE_PREFIX_LIST))
45-
46-
props.withFilter { case (key, _) => !ignorePrefixList.contains(key.split("\\.").apply(0).trim) }
47-
.foreach { case (key, value) => set(key, value) }
48-
this
40+
if (loadSysDefault) {
41+
val fromSysDefaults = Utils.getSystemProperties.filterKeys(_.startsWith("kyuubi."))
42+
loadFromMap(fromSysDefaults)
4943
}
5044

5145
def loadFileDefaults(): KyuubiConf = {
@@ -1316,19 +1310,6 @@ object KyuubiConf {
13161310
.stringConf
13171311
.createOptional
13181312

1319-
val SERVER_CONF_IGNORE_PREFIX_LIST: ConfigEntry[Seq[String]] =
1320-
buildConf("kyuubi.server.conf.ignore.prefix.list")
1321-
.doc("A comma separated list of ignored keys prefix. If kyuubi conf contains any of" +
1322-
" them, the key and the corresponding value will be removed silently during server" +
1323-
" setup." +
1324-
" Note that this rule is for server-side protection defined via administrators to" +
1325-
" prevent some essential configs from tampering but will not forbid users to set dynamic" +
1326-
" configurations via SET syntax.")
1327-
.version("1.6.0")
1328-
.stringConf
1329-
.toSequence()
1330-
.createWithDefault(Seq("java", "sun", "os", "jdk"))
1331-
13321313
val ENGINE_SPARK_SHOW_PROGRESS: ConfigEntry[Boolean] =
13331314
buildConf("kyuubi.session.engine.spark.showProgress")
13341315
.doc("When true, show the progress bar in the spark engine log.")

kyuubi-common/src/test/resources/kyuubi-defaults.conf

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,5 @@ ___userb___.spark.user.test b
2424

2525
___userc___.spark.user.test c
2626

27-
jdk.vendor.version=Zulu 8.60.0.21-CA-linux64
28-
sun.os.patch.level=unknown
29-
os.name=Linux
30-
java.vm.name=OpenJDK 64-Bit Server VM
27+
abc=xyz
28+
xyz=abc

kyuubi-common/src/test/scala/org/apache/kyuubi/config/KyuubiConfSuite.scala

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -94,13 +94,12 @@ class KyuubiConfSuite extends KyuubiFunSuite {
9494
assert(conf.getUserDefaults("userc").getOption("spark.user.test").get === "c")
9595
}
9696

97-
test("Ignore configurations with a specific prefix") {
98-
val conf = KyuubiConf().loadFileDefaults()
99-
assert(conf.getOption("jdk.vendor.version").isEmpty)
100-
assert(conf.getOption("sun.os.patch.level").isEmpty)
101-
assert(conf.getOption("os.name").isEmpty)
102-
assert(conf.getOption("java.vm.name").isEmpty)
103-
97+
test("support arbitrary config from kyuubi-defaults") {
98+
val conf = KyuubiConf()
99+
assert(conf.getOption("user.name").isEmpty)
100+
conf.loadFileDefaults()
101+
assert(conf.getOption("abc").get === "xyz")
102+
assert(conf.getOption("xyz").get === "abc")
104103
}
105104

106105
test("time config test") {

0 commit comments

Comments
 (0)