aajisaka commented on code in PR #7349:
URL: https://github.com/apache/kyuubi/pull/7349#discussion_r2922486250
##########
kyuubi-ha/src/main/scala/org/apache/kyuubi/ha/client/zookeeper/ZookeeperClientProvider.scala:
##########
@@ -110,53 +110,56 @@ object ZookeeperClientProvider extends Logging {
*/
@throws[Exception]
def setUpZooKeeperAuth(conf: KyuubiConf): Unit = {
- def setupZkAuth(): Unit = (conf.get(HA_ZK_AUTH_PRINCIPAL),
getKeyTabFile(conf)) match {
- case (Some(principal), Some(keytab)) if
UserGroupInformation.isSecurityEnabled =>
- if (!new File(keytab).exists()) {
- throw new IOException(s"${HA_ZK_AUTH_KEYTAB.key}: $keytab does not
exists")
- }
- System.setProperty("zookeeper.sasl.clientconfig",
"KyuubiZooKeeperClient")
- conf.get(HA_ZK_AUTH_SERVER_PRINCIPAL).foreach { zkServerPrincipal =>
- // ZOOKEEPER-1467 allows configuring SPN in client
- System.setProperty("zookeeper.server.principal", zkServerPrincipal)
- }
- val zkClientPrincipal = KyuubiHadoopUtils.getServerPrincipal(principal)
- val jaasConf = jaasConfigurationCache.computeIfAbsent(
- (principal, keytab),
- _ => {
- // HDFS-16591 makes breaking change on JaasConfiguration
- DynConstructors.builder()
- .impl( // Hadoop 3.3.5 and above
-
"org.apache.hadoop.security.authentication.util.JaasConfiguration",
- classOf[String],
- classOf[String],
- classOf[String])
- .impl( // Hadoop 3.3.4 and previous
- // scalastyle:off
-
"org.apache.hadoop.security.token.delegation.ZKDelegationTokenSecretManager$JaasConfiguration",
- // scalastyle:on
- classOf[String],
- classOf[String],
- classOf[String])
- .build[Configuration]()
- .newInstance("KyuubiZooKeeperClient", zkClientPrincipal, keytab)
- })
- Configuration.setConfiguration(jaasConf)
- case _ =>
- }
+ def setupZkAuth(
+ principalConfKey: ConfigEntry[Option[String]],
+ keytabConfKey: ConfigEntry[Option[String]]): Unit =
+ (conf.get(principalConfKey), getKeyTabFile(conf, keytabConfKey)) match {
+ case (Some(principal), Some(keytab)) if
UserGroupInformation.isSecurityEnabled =>
+ if (!new File(keytab).exists()) {
+ throw new IOException(s"${keytabConfKey.key}: $keytab does not
exists")
+ }
+ System.setProperty("zookeeper.sasl.clientconfig",
"KyuubiZooKeeperClient")
+ conf.get(HA_ZK_AUTH_SERVER_PRINCIPAL).foreach { zkServerPrincipal =>
+ // ZOOKEEPER-1467 allows configuring SPN in client
+ System.setProperty("zookeeper.server.principal", zkServerPrincipal)
+ }
+ val zkClientPrincipal =
KyuubiHadoopUtils.getServerPrincipal(principal)
+ val jaasConf = jaasConfigurationCache.computeIfAbsent(
+ (principal, keytab),
+ _ => {
+ // HDFS-16591 makes breaking change on JaasConfiguration
+ DynConstructors.builder()
+ .impl( // Hadoop 3.3.5 and above
+
"org.apache.hadoop.security.authentication.util.JaasConfiguration",
+ classOf[String],
+ classOf[String],
+ classOf[String])
+ .impl( // Hadoop 3.3.4 and previous
+ // scalastyle:off
+
"org.apache.hadoop.security.token.delegation.ZKDelegationTokenSecretManager$JaasConfiguration",
+ // scalastyle:on
+ classOf[String],
+ classOf[String],
+ classOf[String])
+ .build[Configuration]()
+ .newInstance("KyuubiZooKeeperClient", zkClientPrincipal,
keytab)
+ })
+ Configuration.setConfiguration(jaasConf)
+ case _ =>
+ }
Review Comment:
The indents look incorrect. Would you keep the current indents so that
reviewers can easily review the diff.
##########
docs/configuration/settings.md:
##########
@@ -310,33 +310,35 @@ You can configure the Kyuubi properties in
`$KYUUBI_HOME/conf/kyuubi-defaults.co
### Ha
-| Key |
Default |
Meaning
| Type | Since |
-|------------------------------------------------|----------------------------------------------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------|-------|
-| kyuubi.ha.addresses
|| The connection string for the discovery
ensemble
| string | 1.6.0 |
-| kyuubi.ha.client.class |
org.apache.kyuubi.ha.client.zookeeper.ZookeeperDiscoveryClient | Class name for
service discovery client.<ul> <li>Zookeeper:
org.apache.kyuubi.ha.client.zookeeper.ZookeeperDiscoveryClient</li> <li>Etcd:
org.apache.kyuubi.ha.client.etcd.EtcdDiscoveryClient</li></ul>
| string | 1.6.0 |
-| kyuubi.ha.etcd.lease.timeout | PT10S
| Timeout for etcd keep alive lease. The
kyuubi server will know the unexpected loss of engine after up to this seconds.
| duration | 1.6.0 |
-| kyuubi.ha.etcd.ssl.ca.path | <undefined>
| Where the etcd CA certificate file is
stored.
| string | 1.6.0 |
-| kyuubi.ha.etcd.ssl.client.certificate.path | <undefined>
| Where the etcd SSL certificate file is
stored.
| string | 1.6.0 |
-| kyuubi.ha.etcd.ssl.client.key.path | <undefined>
| Where the etcd SSL key file is stored.
| string | 1.6.0 |
-| kyuubi.ha.etcd.ssl.enabled | false
| When set to true, will build an SSL
secured etcd client.
| boolean | 1.6.0 |
-| kyuubi.ha.namespace | kyuubi
| The root directory for the service to
deploy its instance uri
| string | 1.6.0 |
-| kyuubi.ha.zookeeper.acl.enabled | false
| (deprecated) Set to true if the ZooKeeper
ensemble is kerberized
| boolean | 1.0.0 |
-| kyuubi.ha.zookeeper.auth.digest | <undefined>
| The digest auth string is used for
ZooKeeper authentication, like: username:password.
| string | 1.3.2 |
-| kyuubi.ha.zookeeper.auth.keytab | <undefined>
| Location of the Kyuubi server's keytab
that is used for ZooKeeper authentication.
| string | 1.3.2 |
-| kyuubi.ha.zookeeper.auth.principal | <undefined>
| Kerberos principal name that is used for
ZooKeeper authentication.
| string | 1.3.2 |
-| kyuubi.ha.zookeeper.auth.serverPrincipal | <undefined>
| Kerberos principal name of ZooKeeper
Server. It only takes effect when Zookeeper client's version at least 3.5.7 or
3.6.0 or applies ZOOKEEPER-1467. To use Zookeeper 3.6 client, compile Kyuubi
with `-Pzookeeper-3.6`. | string | 1.8.0 |
-| kyuubi.ha.zookeeper.auth.type | NONE
| The type of ZooKeeper authentication, all
candidates are <ul><li>NONE</li><li> KERBEROS</li><li> DIGEST</li></ul>
| string | 1.3.2 |
-| kyuubi.ha.zookeeper.connection.base.retry.wait | 1000
| Initial amount of time to wait between
retries to the ZooKeeper ensemble
| int | 1.0.0 |
-| kyuubi.ha.zookeeper.connection.max.retries | 3
| Max retry times for connecting to the
ZooKeeper ensemble
| int | 1.0.0 |
-| kyuubi.ha.zookeeper.connection.max.retry.wait | 30000
| Max amount of time to wait between retries
for BOUNDED_EXPONENTIAL_BACKOFF policy can reach, or max time until elapsed for
UNTIL_ELAPSED policy to connect the zookeeper ensemble
| int | 1.0.0 |
-| kyuubi.ha.zookeeper.connection.retry.policy | EXPONENTIAL_BACKOFF
| The retry policy for connecting to the
ZooKeeper ensemble, all candidates are: <ul><li>ONE_TIME</li><li>
N_TIME</li><li> EXPONENTIAL_BACKOFF</li><li>
BOUNDED_EXPONENTIAL_BACKOFF</li><li> UNTIL_ELAPSED</li></ul> | string |
1.0.0 |
-| kyuubi.ha.zookeeper.connection.timeout | 15000
| The timeout(ms) of creating the connection
to the ZooKeeper ensemble
| int | 1.0.0 |
-| kyuubi.ha.zookeeper.engine.auth.type | NONE
| The type of ZooKeeper authentication for
the engine, all candidates are <ul><li>NONE</li><li> KERBEROS</li><li>
DIGEST</li></ul>
| string | 1.3.2 |
-| kyuubi.ha.zookeeper.namespace | kyuubi
| (deprecated) The root directory for the
service to deploy its instance uri
| string | 1.0.0 |
-| kyuubi.ha.zookeeper.node.creation.timeout | PT2M
| Timeout for creating ZooKeeper node
| duration | 1.2.0 |
-| kyuubi.ha.zookeeper.publish.configs | false
| When set to true, publish Kerberos configs
to Zookeeper. Note that the Hive driver needs to be greater than 1.3 or 2.0 or
apply HIVE-11581 patch.
| boolean | 1.4.0 |
-| kyuubi.ha.zookeeper.quorum
|| (deprecated) The connection string for the
ZooKeeper ensemble
| string | 1.0.0 |
-| kyuubi.ha.zookeeper.session.timeout | 60000
| The timeout(ms) of a connected session to
be idled
| int | 1.0.0 |
+| Key |
Default |
Meaning
| Type | Since |
+|------------------------------------------------|----------------------------------------------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------|--------|
+| kyuubi.ha.addresses
|| The connection string for the discovery
ensemble
| string | 1.6.0 |
+| kyuubi.ha.client.class |
org.apache.kyuubi.ha.client.zookeeper.ZookeeperDiscoveryClient | Class name for
service discovery client.<ul> <li>Zookeeper:
org.apache.kyuubi.ha.client.zookeeper.ZookeeperDiscoveryClient</li> <li>Etcd:
org.apache.kyuubi.ha.client.etcd.EtcdDiscoveryClient</li></ul>
| string | 1.6.0 |
+| kyuubi.ha.etcd.lease.timeout | PT10S
| Timeout for etcd keep alive lease. The
kyuubi server will know the unexpected loss of engine after up to this seconds.
| duration | 1.6.0 |
+| kyuubi.ha.etcd.ssl.ca.path | <undefined>
| Where the etcd CA certificate file is
stored.
| string | 1.6.0 |
+| kyuubi.ha.etcd.ssl.client.certificate.path | <undefined>
| Where the etcd SSL certificate file is
stored.
| string | 1.6.0 |
+| kyuubi.ha.etcd.ssl.client.key.path | <undefined>
| Where the etcd SSL key file is stored.
| string | 1.6.0 |
+| kyuubi.ha.etcd.ssl.enabled | false
| When set to true, will build an SSL
secured etcd client.
| boolean | 1.6.0 |
+| kyuubi.ha.namespace | kyuubi
| The root directory for the service to
deploy its instance uri
| string | 1.6.0 |
+| kyuubi.ha.zookeeper.acl.enabled | false
| (deprecated) Set to true if the ZooKeeper
ensemble is kerberized
| boolean | 1.0.0 |
+| kyuubi.ha.zookeeper.auth.digest | <undefined>
| The digest auth string is used for
ZooKeeper authentication, like: username:password.
| string | 1.3.2 |
+| kyuubi.ha.zookeeper.auth.keytab | <undefined>
| Location of the Kyuubi server's keytab
that is used for ZooKeeper authentication.
| string | 1.3.2 |
+| kyuubi.ha.zookeeper.auth.principal | <undefined>
| Kerberos principal name that is used for
ZooKeeper authentication.
| string | 1.3.2 |
+| kyuubi.ha.zookeeper.auth.serverPrincipal | <undefined>
| Kerberos principal name of ZooKeeper
Server. It only takes effect when Zookeeper client's version at least 3.5.7 or
3.6.0 or applies ZOOKEEPER-1467. To use Zookeeper 3.6 client, compile Kyuubi
with `-Pzookeeper-3.6`. | string | 1.8.0 |
+| kyuubi.ha.zookeeper.auth.type | NONE
| The type of ZooKeeper authentication, all
candidates are <ul><li>NONE</li><li> KERBEROS</li><li> DIGEST</li></ul>
| string | 1.3.2 |
+| kyuubi.ha.zookeeper.connection.base.retry.wait | 1000
| Initial amount of time to wait between
retries to the ZooKeeper ensemble
| int | 1.0.0 |
+| kyuubi.ha.zookeeper.connection.max.retries | 3
| Max retry times for connecting to the
ZooKeeper ensemble
| int | 1.0.0 |
+| kyuubi.ha.zookeeper.connection.max.retry.wait | 30000
| Max amount of time to wait between retries
for BOUNDED_EXPONENTIAL_BACKOFF policy can reach, or max time until elapsed for
UNTIL_ELAPSED policy to connect the zookeeper ensemble
| int | 1.0.0 |
+| kyuubi.ha.zookeeper.connection.retry.policy | EXPONENTIAL_BACKOFF
| The retry policy for connecting to the
ZooKeeper ensemble, all candidates are: <ul><li>ONE_TIME</li><li>
N_TIME</li><li> EXPONENTIAL_BACKOFF</li><li>
BOUNDED_EXPONENTIAL_BACKOFF</li><li> UNTIL_ELAPSED</li></ul> | string |
1.0.0 |
+| kyuubi.ha.zookeeper.connection.timeout | 15000
| The timeout(ms) of creating the connection
to the ZooKeeper ensemble
| int | 1.0.0 |
+| kyuubi.ha.zookeeper.engine.auth.principal | <undefined>
| Kerberos principal name that is used for
the engine's ZooKeeper authentication.
| string | 1.12.0 |
+| kyuubi.ha.zookeeper.engine.auth.keytab | <undefined>
| Location of the Kyuubi server’s keytab
that is used for the engine's ZooKeeper authentication.
| string | 1.12.0 |
Review Comment:
If they are unset, what are used by default? Would you document?
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
