[GitHub] [spark] AngersZhuuuu commented on a change in pull request #26594: [SPARK-29957][TEST] Bump MiniKdc to 3.2.0
AngersZh commented on a change in pull request #26594: [SPARK-29957][TEST]
Bump MiniKdc to 3.2.0
URL: https://github.com/apache/spark/pull/26594#discussion_r351723504
##
File path:
external/kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaTestUtils.scala
##
@@ -136,6 +137,20 @@ class KafkaTestUtils(
kdcConf.setProperty(MiniKdc.DEBUG, "true")
kdc = new MiniKdc(kdcConf, kdcDir)
kdc.start()
+val krb5Conf = Source.fromFile(kdc.getKrb5conf, "UTF-8").getLines()
+val rewriteKrb5Conf = krb5Conf.map(s => if (s.contains("libdefaults")) {
+ s + "\n" +
Review comment:
> `File.separator`?
System.lineSeparator
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] AngersZhuuuu commented on a change in pull request #26594: [SPARK-29957][TEST] Bump MiniKdc to 3.2.0
AngersZh commented on a change in pull request #26594: [SPARK-29957][TEST]
Bump MiniKdc to 3.2.0
URL: https://github.com/apache/spark/pull/26594#discussion_r351664368
##
File path:
external/kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaTestUtils.scala
##
@@ -136,6 +137,20 @@ class KafkaTestUtils(
kdcConf.setProperty(MiniKdc.DEBUG, "true")
kdc = new MiniKdc(kdcConf, kdcDir)
kdc.start()
+val krb5Conf = Source.fromFile(kdc.getKrb5conf, "UTF-8").getLines()
+val rewriteKrb5Conf = krb5Conf.map(s => if (s.contains("libdefaults")) {
+ s + "\n" +
+"default_tkt_enctypes=aes128-cts-hmac-sha1-96\n" +
+"default_tgs_enctypes=aes128-cts-hmac-sha1-96 "
+} else {
+ s
+})
+kdc.getKrb5conf.delete()
+val writer = new PrintWriter(kdc.getKrb5conf)
+// scalastyle:off
Review comment:
> ```
> // scalastyle:off println
> ...
> // scalastyle:on println
> ```
>
> ?
Miss it , will add.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] AngersZhuuuu commented on a change in pull request #26594: [SPARK-29957][TEST] Bump MiniKdc to 3.2.0
AngersZh commented on a change in pull request #26594: [SPARK-29957][TEST]
Bump MiniKdc to 3.2.0
URL: https://github.com/apache/spark/pull/26594#discussion_r351661826
##
File path:
external/kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaTestUtils.scala
##
@@ -136,6 +137,20 @@ class KafkaTestUtils(
kdcConf.setProperty(MiniKdc.DEBUG, "true")
kdc = new MiniKdc(kdcConf, kdcDir)
kdc.start()
+val krb5Conf = Source.fromFile(kdc.getKrb5conf, "UTF-8").getLines()
+val rewriteKrb5Conf = krb5Conf.map(s => if (s.contains("libdefaults")) {
Review comment:
> It's stable but MiniKDC can create a `krb5.conf` file which doesn't
contain `libdefault`. Such case the code does something different.
e, Stable in `MiniKdc`
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] AngersZhuuuu commented on a change in pull request #26594: [SPARK-29957][TEST] Bump MiniKdc to 3.2.0
AngersZh commented on a change in pull request #26594: [SPARK-29957][TEST] Bump MiniKdc to 3.2.0 URL: https://github.com/apache/spark/pull/26594#discussion_r350729070 ## File path: external/kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaTestUtils.scala ## @@ -136,6 +137,20 @@ class KafkaTestUtils( kdcConf.setProperty(MiniKdc.DEBUG, "true") kdc = new MiniKdc(kdcConf, kdcDir) kdc.start() +val krb5Conf = Source.fromFile(kdc.getKrb5conf, "UTF-8").getLines() Review comment: > Maybe we can extract this functionality into a function and this way simple tests can be added. Here I want to show add `default_tkt_enctypes` and `default_tgs_enctypes` is useful and it can solve this problem. For me, I prefer to build spark's MiniKDC like Kafka and make `krb5.conf` customize. Hadoop's MiniKDC is not so good. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] AngersZhuuuu commented on a change in pull request #26594: [SPARK-29957][TEST] Bump MiniKdc to 3.2.0
AngersZh commented on a change in pull request #26594: [SPARK-29957][TEST]
Bump MiniKdc to 3.2.0
URL: https://github.com/apache/spark/pull/26594#discussion_r350726037
##
File path:
external/kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaTestUtils.scala
##
@@ -136,6 +137,20 @@ class KafkaTestUtils(
kdcConf.setProperty(MiniKdc.DEBUG, "true")
kdc = new MiniKdc(kdcConf, kdcDir)
kdc.start()
+val krb5Conf = Source.fromFile(kdc.getKrb5conf, "UTF-8").getLines()
+val rewriteKrb5Conf = krb5Conf.map(s => if (s.contains("libdefaults")) {
Review comment:
> If `libdefaults` doesn't exist then no need to add `default_tkt_enctypes`,
etc..? (tests would show the intention)
Emmm in krb5.conf, `libdefault` is stable.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] AngersZhuuuu commented on a change in pull request #26594: [SPARK-29957][TEST] Bump MiniKdc to 3.2.0
AngersZh commented on a change in pull request #26594: [SPARK-29957][TEST] Bump MiniKdc to 3.2.0 URL: https://github.com/apache/spark/pull/26594#discussion_r350725716 ## File path: external/kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaTestUtils.scala ## @@ -171,6 +186,7 @@ class KafkaTestUtils( | useKeyTab=true | storeKey=true | useTicketCache=false + | refreshKrb5Config=true Review comment: > `refreshKrb5Config` is safe to add. Yea, it's necessary in this case. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] AngersZhuuuu commented on a change in pull request #26594: [SPARK-29957][TEST] Bump MiniKdc to 3.2.0
AngersZh commented on a change in pull request #26594: [SPARK-29957][TEST]
Bump MiniKdc to 3.2.0
URL: https://github.com/apache/spark/pull/26594#discussion_r350567964
##
File path:
external/kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaTestUtils.scala
##
@@ -136,6 +137,20 @@ class KafkaTestUtils(
kdcConf.setProperty(MiniKdc.DEBUG, "true")
kdc = new MiniKdc(kdcConf, kdcDir)
kdc.start()
+val krb5Conf = Source.fromFile(kdc.getKrb5conf, "UTF-8").getLines()
+val rewriteKrb5Conf = krb5Conf.map(s => if (s.contains("libdefaults")) {
+ s + "\n" +
+"default_tkt_enctypes=aes128-cts-hmac-sha1-96\n" +
+"default_tgs_enctypes=aes128-cts-hmac-sha1-96 "
Review comment:
Check Kafka's `MiniKdc`, it rewrite from hadoop-2.7's MiniKDC, and make
`krb5.conf` customize.
Add spark's own MiniKDC like Kafka is better. This is also beneficial to
future work about kerberos test case.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] AngersZhuuuu commented on a change in pull request #26594: [SPARK-29957][TEST] Bump MiniKdc to 3.2.0
AngersZh commented on a change in pull request #26594: [SPARK-29957][TEST]
Bump MiniKdc to 3.2.0
URL: https://github.com/apache/spark/pull/26594#discussion_r350549930
##
File path: pom.xml
##
@@ -1024,7 +1025,7 @@
org.apache.hadoop
hadoop-minikdc
-${hadoop.version}
+${minikdc.version}
Review comment:
@srowen @wangyum @dongjoon-hyun
I checked the kerberos code that when client call a request
`KrbTgsReq/KrbTktReq `, it will first check if there are config
`default_tkt_enctypes` and `default_tgs_enctypes`in `krb5.conf`, if not, then
use `jdk buildin enctypes`.
And i have checked the build in ecntypes:
```
jdk8
static {
DEBUG = Krb5.DEBUG;
initStatic();
BUILTIN_ETYPES = new int[]{18, 17, 16, 23, 1, 3};
BUILTIN_ETYPES_NOAES256 = new int[]{17, 16, 23, 1, 3};
}
jdk11
static {
DEBUG = Krb5.DEBUG;
initStatic();
BUILTIN_ETYPES = new int[]{18, 17, 20, 19, 16, 23, 1, 3};
BUILTIN_ETYPES_NOAES256 = new int[]{17, 19, 16, 23, 1, 3};
}
```
In server side , they both use `aes128-cts-hmac-sha1-96` as first choice.
But MiniKdc don't have API for us to config these. So rewrite and refresh
it.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] AngersZhuuuu commented on a change in pull request #26594: [SPARK-29957][TEST] Bump MiniKdc to 3.2.0
AngersZh commented on a change in pull request #26594: [SPARK-29957][TEST] Bump MiniKdc to 3.2.0 URL: https://github.com/apache/spark/pull/26594#discussion_r349898383 ## File path: pom.xml ## @@ -123,6 +123,7 @@ 1.7.16 1.2.17 2.7.4 +3.2.0 Review comment: > BTW, `miniKdc` -> `minikdc`. As you see, Spark-introduced properties are are camelcase like `codahale` and `htmlunit`. If needed, we may use `-`, but I prefer `minikdc` here in this case. Thank you for your careful explanation. Updated. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
