[kudu-CR] KUDU-1999: Spark connector should login with Kerberos credentials on driver

[Dan Burkert (Code Review)]

Dan Burkert has submitted this change and it was merged.

Change subject: KUDU-1999: Spark connector should login with Kerberos 
credentials on driver
..


KUDU-1999: Spark connector should login with Kerberos credentials on driver

Tested on a secure cluster using the Spark ITBLL job:

spark2-submit \
--deploy-mode=cluster \
--master=yarn \
--principal=dan \
--keytab dan.keytab \
--class org.apache.kudu.spark.tools.IntegrationTestBigLinkedList \
kudu-spark-tools-1.4.0-SNAPSHOT.jar generate \
--master-addrs=kudu-spark-secure-1.gce.cloudera.com

Without some very major changes to our test infrastructure it's not
possible to test this code in unit tests, since it relies on a secure
Yarn cluster being available.

note: long-running jobs will continue to fail, since credentials are
still not refreshed.

Change-Id: If87a470c1cf99ea52668f22b72f1f7331877ec63
Reviewed-on: http://gerrit.cloudera.org:8080/6822
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M docs/developing.adoc
M 
java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/IntegrationTestBigLinkedList.scala
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala
5 files changed, 80 insertions(+), 16 deletions(-)

Approvals:
  Todd Lipcon: Looks good to me, approved
  Kudu Jenkins: Verified



-- 
To view, visit http://gerrit.cloudera.org:8080/6822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: If87a470c1cf99ea52668f22b72f1f7331877ec63
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-1999: Spark connector should login with Kerberos credentials on driver

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: KUDU-1999: Spark connector should login with Kerberos 
credentials on driver
..


Patch Set 3: Code-Review+2

-- 
To view, visit http://gerrit.cloudera.org:8080/6822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If87a470c1cf99ea52668f22b72f1f7331877ec63
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-1999: Spark connector should login with Kerberos credentials on driver

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: KUDU-1999: Spark connector should login with Kerberos 
credentials on driver
..


Patch Set 3:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/6822/1/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
File 
java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala:

Line 49:   import kudu.KuduContext._
> is there any way to magically get a SparkContext from a singleton or someth
I've added a deprecated constructor which should retain compatibility with 
existing applications.


PS1, Line 59: AuthenticationCrede
> maybe rename to loginUser or getSubject or something, since many of the ret
Done


Line 62:   }
> is principal always required? or is there some 'default' principal that wou
Yes, the Spark delegation token logic doesn't kick in unless the principal is 
set.


PS1, Line 65:  val c = KuduConnection.getAsyncClient(kuduMaster)
: if (authnCredentials != null) {
:  
> I'm wondering if this is always a good idea. If the user specifies a keytab
Done


PS1, Line 295: Array(new 
AppConfigurationEntry("com.sun.security.auth.module.Krb5LoginModule",
 : 
AppConfigurationEntry.LoginModuleControlFlag.REQUIRED,
 :  
> is this Scala's equivalent of a static member?
Yes.


-- 
To view, visit http://gerrit.cloudera.org:8080/6822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If87a470c1cf99ea52668f22b72f1f7331877ec63
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1999: Spark connector should login with Kerberos credentials on driver

[Dan Burkert (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/6822

to look at the new patch set (#3).

Change subject: KUDU-1999: Spark connector should login with Kerberos 
credentials on driver
..

KUDU-1999: Spark connector should login with Kerberos credentials on driver

Tested on a secure cluster using the Spark ITBLL job:

spark2-submit \
--deploy-mode=cluster \
--master=yarn \
--principal=dan \
--keytab dan.keytab \
--class org.apache.kudu.spark.tools.IntegrationTestBigLinkedList \
kudu-spark-tools-1.4.0-SNAPSHOT.jar generate \
--master-addrs=kudu-spark-secure-1.gce.cloudera.com

Without some very major changes to our test infrastructure it's not
possible to test this code in unit tests, since it relies on a secure
Yarn cluster being available.

note: long-running jobs will continue to fail, since credentials are
still not refreshed.

Change-Id: If87a470c1cf99ea52668f22b72f1f7331877ec63
---
M docs/developing.adoc
M 
java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/IntegrationTestBigLinkedList.scala
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala
5 files changed, 80 insertions(+), 16 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/22/6822/3
-- 
To view, visit http://gerrit.cloudera.org:8080/6822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If87a470c1cf99ea52668f22b72f1f7331877ec63
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-1999: Spark connector should login with Kerberos credentials on driver

[Dan Burkert (Code Review)]

Hello Jean-Daniel Cryans, Todd Lipcon, Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/6822

to look at the new patch set (#2).

Change subject: KUDU-1999: Spark connector should login with Kerberos 
credentials on driver
..

KUDU-1999: Spark connector should login with Kerberos credentials on driver

Tested on a secure cluster, however I don't think an automated unit test
can be written for this functionality.

note: long-running jobs will still fail, since the credentials aren't
refreshed.

Change-Id: If87a470c1cf99ea52668f22b72f1f7331877ec63
---
M 
java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/IntegrationTestBigLinkedList.scala
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala
4 files changed, 74 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/22/6822/2
-- 
To view, visit http://gerrit.cloudera.org:8080/6822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If87a470c1cf99ea52668f22b72f1f7331877ec63
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-1999: Spark connector should login with Kerberos credentials on driver

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: KUDU-1999: Spark connector should login with Kerberos 
credentials on driver
..


Patch Set 1:

(5 comments)

Can you add some color to the commit message on testing? Clearly our unit test 
environment isn't well equipped to test this right now, but would be good to 
leave some notes about how to test it

http://gerrit.cloudera.org:8080/#/c/6822/1/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
File 
java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala:

Line 49: class KuduContext(kuduMaster: String, @transient sc: SparkContext) 
extends Serializable {
is there any way to magically get a SparkContext from a singleton or something?

If not, we'll need to document this in release notes and also update 
developing.adoc, where we show an example of creating a KuduContext by hand.

Perhaps a compatibility path where we can handle a null spark context is in 
order? Even though we say 'Unstable' above, we did go and document it, so I'd 
feel bad breaking it.


PS1, Line 59: loginUserFromKeytab
maybe rename to loginUser or getSubject or something, since many of the return 
paths don't login from keytab?


Line 62: val principal = 
sc.getConf.getOption("spark.yarn.principal").getOrElse(return subject)
is principal always required? or is there some 'default' principal that would 
be applied?


PS1, Line 65:  if (subject != null && 
!subject.getPrincipals(classOf[KerberosPrincipal]).isEmpty) {
:   return subject
: }
I'm wondering if this is always a good idea. If the user specifies a keytab, 
maybe we should always respect it? What if the one that is already in the 
Subject is an unrelated principal (eg from the wrong realm, etc)?


PS1, Line 295: private object KuduContext {
 :   val Log = LoggerFactory.getLogger(classOf[KuduContext])
 : }
is this Scala's equivalent of a static member?


-- 
To view, visit http://gerrit.cloudera.org:8080/6822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If87a470c1cf99ea52668f22b72f1f7331877ec63
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1999: Spark connector should login with Kerberos credentials on driver

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: KUDU-1999: Spark connector should login with Kerberos 
credentials on driver
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6822/1/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
File 
java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala:

Line 80:   "debug" -> sc.getConf.getBoolean("kudu.jaas.debug", 
defaultValue = false).toString
> Apparently Spark filters out non-spark configurations, so this is not setta
Removed, since I think this can be done globally via a JVM option anyway.


-- 
To view, visit http://gerrit.cloudera.org:8080/6822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If87a470c1cf99ea52668f22b72f1f7331877ec63
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1999: Spark connector should login with Kerberos credentials on driver

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: KUDU-1999: Spark connector should login with Kerberos 
credentials on driver
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6822/1/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
File 
java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala:

Line 80:   "debug" -> sc.getConf.getBoolean("kudu.jaas.debug", 
defaultValue = false).toString
Apparently Spark filters out non-spark configurations, so this is not settable. 
 Should probably be switched to an environment variable.


-- 
To view, visit http://gerrit.cloudera.org:8080/6822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If87a470c1cf99ea52668f22b72f1f7331877ec63
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1999: Spark connector should login with Kerberos credentials on driver

[Dan Burkert (Code Review)]

Hello Jean-Daniel Cryans, Todd Lipcon,

I'd like you to do a code review.  Please visit

http://gerrit.cloudera.org:8080/6822

to review the following change.

Change subject: KUDU-1999: Spark connector should login with Kerberos 
credentials on driver
..

KUDU-1999: Spark connector should login with Kerberos credentials on driver

note: long-running jobs will still fail, since the credentials aren't
refreshed.

Change-Id: If87a470c1cf99ea52668f22b72f1f7331877ec63
---
M 
java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/IntegrationTestBigLinkedList.scala
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala
4 files changed, 64 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/22/6822/1
-- 
To view, visit http://gerrit.cloudera.org:8080/6822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: If87a470c1cf99ea52668f22b72f1f7331877ec63
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Todd Lipcon