[kudu-CR] KUDU-2264. java: automatically attempt to re-acquire Kerberos credentials before expiration

2018-03-07 Thread Dan Burkert (Code Review) 
Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9050 )

Change subject: KUDU-2264. java: automatically attempt to re-acquire Kerberos 
credentials before expiration
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java
File java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java:

http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java@86
PS3, Line 86:   private final Object subjectLock = new Object();
> Subject could have been passed in by the caller, though, in which case we'd
Good point.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I514253e0a7f067dbc8ffe4eaf5a7a2c32900b539
Gerrit-Change-Number: 9050
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Anonymous Coward #380
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 07 Mar 2018 20:12:07 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2264. java: automatically attempt to re-acquire Kerberos credentials before expiration

2018-03-07 Thread Todd Lipcon (Code Review) 
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9050 )

Change subject: KUDU-2264. java: automatically attempt to re-acquire Kerberos 
credentials before expiration
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java
File java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java:

http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java@86
PS3, Line 86:   private final Object subjectLock = new Object();
> I don't know how you feel about this style, but since 'subject' is private,
Subject could have been passed in by the caller, though, in which case we'd be 
using an external object as an internal lock which is a no-no IMO.


http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java
File java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java:

http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java@196
PS3, Line 196: principal.getRealm() + "@" + principal.getRealm())
> Do we always have the service and the client in the same domain?  Maybe, it
Even in a cross-realm situation, you end up with a service ticket to your local 
realm's KDC. This is called from the findTgt function above which loops over 
all your tickets.

For example, I just logged into a cluster which has cross-realm trust from our 
corporate active directory to a cluster-local KDC, kinitted to active 
directory, and then connected to a kerberos-authenticated service on the 
cluster. kinit shows:


[todd@xxx ~]$ klist
Ticket cache: FILE:/tmp/krb5cc_2009
Default principal: todd@CLOUDERA.LOCAL

Valid starting ExpiresService principal
03/07/18 19:56:23  03/08/18 05:56:25  krbtgt/CLOUDERA.LOCAL@CLOUDERA.LOCAL
renew until 03/14/18 19:56:23
03/07/18 19:56:27  03/08/18 05:56:25  krbtgt/PROD.EDH@CLOUDERA.LOCAL
renew until 03/14/18 19:56:23
03/07/18 19:56:27  03/08/18 05:56:25  impala/xxx.cloudera@prod.edh
renew until 03/12/18 19:56:27

In this case, it's the krbtgt/CLOUDERA.LOCAL@CLOUDERA.LOCAL ticket that we're 
looking for (ie the TGT associated with the primary realm you authenticated 
to). I'll see if I can add some commentary.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I514253e0a7f067dbc8ffe4eaf5a7a2c32900b539
Gerrit-Change-Number: 9050
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Anonymous Coward #380
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 07 Mar 2018 20:07:00 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2264. java: automatically attempt to re-acquire Kerberos credentials before expiration

2018-03-07 Thread Alexey Serbin (Code Review) 
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9050 )

Change subject: KUDU-2264. java: automatically attempt to re-acquire Kerberos 
credentials before expiration
..


Patch Set 3:

(3 comments)

I just did a first quick pass.  Overall looks good, I'm thinking about 
re-visiting this patch once more time tonight.

http://gerrit.cloudera.org:8080/#/c/9050/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/9050/3//COMMIT_MSG@7
PS3, Line 7: java: automatically attempt to re-acquire Kerberos credentials 
before expiration
nit: this looks too long; maybe

java: automatically re-acquire Kerberos credentials


http://gerrit.cloudera.org:8080/#/c/9050/3//COMMIT_MSG@12
PS3, Line 12: It also adds a long javadoc for AsyncKuduClient indicating the 
proper
: way to authenticate to a secured Kudu cluster for various 
scenarios,
: since this is a common question we've gotten.
it's a nice addition


http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java
File java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java:

http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java@196
PS3, Line 196: principal.getRealm() + "@" + principal.getRealm())
Do we always have the service and the client in the same domain?  Maybe, it's 
better to use some pattern matching here instead?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I514253e0a7f067dbc8ffe4eaf5a7a2c32900b539
Gerrit-Change-Number: 9050
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Anonymous Coward #380
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 07 Mar 2018 19:57:30 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2264. java: automatically attempt to re-acquire Kerberos credentials before expiration

2018-03-07 Thread Dan Burkert (Code Review) 
Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9050 )

Change subject: KUDU-2264. java: automatically attempt to re-acquire Kerberos 
credentials before expiration
..


Patch Set 3:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java:

http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@113
PS3, Line 113: associate them with a {@link javax.security.auth.Subject}
 :  * instance, and associate them with the current thread of 
execution
it will be a bit wordier, but I think you should avoid pronouns (them) in this 
sentence, it's not 100% clear what they refer to.


http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@180
PS3, Line 180: This
Specify that this is a function of UserGroupInformation


http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@185
PS3, Line 185:  The Kudu client emits
 :  * DEB
wrapping


http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@221
PS3, Line 221: {@link InterfaceAudience.Private
Are we giving up on ever changing an Unstable interface?  Might be good to add 
a link to InterfaceStability.


http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
File java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java:

http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java@423
PS3, Line 423: if (s == null ||
 : 
s.getPrivateCredentials(KerberosTicket.class).isEmpty()) {
this can be one line now, which will read better


http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java
File java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java:

http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java@86
PS3, Line 86:   private final Object subjectLock = new Object();
I don't know how you feel about this style, but since 'subject' is private, you 
could just use it directly as the lock.


http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java
File java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java:

http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java@97
PS3, Line 97: options.put("debug", 
Boolean.toString(Boolean.getBoolean("kudu.jaas.debug")));
Is this trick worth adding to the debug section of your new doc?


http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java@170
PS3, Line 170:   return millisUntilEnd * 1000 < 
REFRESH_BEFORE_EXPIRATION_SECS;
Shouldn't this be multiplying the expiration_secs by 1000 to make the units 
line up?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I514253e0a7f067dbc8ffe4eaf5a7a2c32900b539
Gerrit-Change-Number: 9050
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Anonymous Coward #380
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 07 Mar 2018 19:21:59 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2264. java: automatically attempt to re-acquire Kerberos credentials before expiration

2018-03-07 Thread Anonymous Coward (Code Review) 
Anonymous Coward #380 has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9050 )

Change subject: KUDU-2264. java: automatically attempt to re-acquire Kerberos 
credentials before expiration
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java:

http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@143
PS3, Line 143:  * example by re-running 'kinit' once each key.
Probably a typo: "by re-running 'kinit' once each key" should probably read: 
"by re-running 'kinit' once each day" s/key/day



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I514253e0a7f067dbc8ffe4eaf5a7a2c32900b539
Gerrit-Change-Number: 9050
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Anonymous Coward #380
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 07 Mar 2018 19:20:43 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2264. java: automatically attempt to re-acquire Kerberos credentials before expiration

2018-03-05 Thread Todd Lipcon (Code Review) 
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9050 )

Change subject: KUDU-2264. java: automatically attempt to re-acquire Kerberos 
credentials before expiration
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java:

http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@1502
PS3, Line 1502:   LOG.info("ConnectToCluster got authn token!");
> Did you mean to remove this or make it DEBUG?
oops, yea, this was just me debugging a unit test, will remove.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I514253e0a7f067dbc8ffe4eaf5a7a2c32900b539
Gerrit-Change-Number: 9050
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 05 Mar 2018 19:49:36 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2264. java: automatically attempt to re-acquire Kerberos credentials before expiration

2018-03-05 Thread Jean-Daniel Cryans (Code Review) 
Jean-Daniel Cryans has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9050 )

Change subject: KUDU-2264. java: automatically attempt to re-acquire Kerberos 
credentials before expiration
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java:

http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@1502
PS3, Line 1502:   LOG.info("ConnectToCluster got authn token!");
Did you mean to remove this or make it DEBUG?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I514253e0a7f067dbc8ffe4eaf5a7a2c32900b539
Gerrit-Change-Number: 9050
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 05 Mar 2018 18:52:29 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2264. java: automatically attempt to re-acquire Kerberos credentials before expiration

2018-03-04 Thread Todd Lipcon (Code Review) 
Hello Alexey Serbin, Dan Burkert, Kudu Jenkins, Adar Dembo, Hao Hao,

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

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

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

Change subject: KUDU-2264. java: automatically attempt to re-acquire Kerberos 
credentials before expiration
..

KUDU-2264. java: automatically attempt to re-acquire Kerberos credentials 
before expiration

This fixes the Java client to notice when the Kerberos credentials it
has are about to expire, and initiates a re-login when necessary.

It also adds a long javadoc for AsyncKuduClient indicating the proper
way to authenticate to a secured Kudu cluster for various scenarios,
since this is a common question we've gotten.

Change-Id: I514253e0a7f067dbc8ffe4eaf5a7a2c32900b539
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M 
java/kudu-client/src/main/java/org/apache/kudu/client/AuthnTokenReacquirer.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java
M java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java
M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java
M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiator.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool.proto
M src/kudu/tools/tool_action_test.cc
14 files changed, 756 insertions(+), 97 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I514253e0a7f067dbc8ffe4eaf5a7a2c32900b539
Gerrit-Change-Number: 9050
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon