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

2018-03-08 Thread Todd Lipcon (Code Review) 
Todd Lipcon has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9050 )

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

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

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
Reviewed-on: http://gerrit.cloudera.org:8080/9050
Reviewed-by: Hao Hao 
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin 
---
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, 796 insertions(+), 102 deletions(-)

Approvals:
  Hao Hao: Looks good to me, but someone else must approve
  Kudu Jenkins: Verified
  Alexey Serbin: Looks good to me, approved

--
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: merged
Gerrit-Change-Id: I514253e0a7f067dbc8ffe4eaf5a7a2c32900b539
Gerrit-Change-Number: 9050
Gerrit-PatchSet: 6
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

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

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
..


Patch Set 5: Code-Review+2


--
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: 5
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: Thu, 08 Mar 2018 07:16:35 +
Gerrit-HasComments: No

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

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

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


Patch Set 5: Code-Review+1

(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@140
PS3, Line 140:  * the ticket cache to obtain a new ticket with a later 
expiration time. So, if
> I don't think so because it is more like us trying to prevent a weird edge
Ok, makes sense to me.



--
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: 5
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: Thu, 08 Mar 2018 03:02:59 +
Gerrit-HasComments: Yes

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

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
..


Patch Set 4:

(11 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@140
PS3, Line 140:  * the ticket cache to obtain a new ticket with a later 
expiration time. So, if
> Do we need to mention that Kudu will refuse the re-login attempt if the pri
I don't think so because it is more like us trying to prevent a weird edge case 
we expect to never happen. I think providing too much detail in this context 
may just confuse the reader.


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@95
PS3, Line 95: static
> 'static' is redundant for inner enums.
Done


http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java@107
PS3, Line 107: .
> Nit: add ')'
Done


http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java@130
PS3, Line 130:* @param subject JAAS Subject that the client's credentials 
are stored in
> Nit: remove the extra @param.
Done


http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java@171
PS3, Line 171:   public void refreshSubject() {
> Nit: Maybe add a brief comment in front of the function since it is public.
Done


http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java@191
PS3, Line 191:
> Extra line.
actually I like this extra line because the above comment refers to the rest of 
the function. If I remove this line it makes it look like it's referring to the 
if statement below.


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

http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java@55
PS3, Line 55: static
> Redundant.
Done


http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java@118
PS3, Line 118: miniCluster.killMasters();
> I do not quite follow why do we have to kill the masters. Does it mean as l
yep, because the connection has already been established, keeping it alive 
means you never need to re-authenticate. I'll clarify that point.


http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java@273
PS3, Line 273:   public void testRenewAndReacquireKeberosCredentials() throws 
Exception {
> Nit: addd a comment here?
Done


http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java@355
PS3, Line 355: Closeable c
> Not used?
For a "try-with-resources" block I think it's required to name the resource 
variable even if it's never used: 
https://stackoverflow.com/questions/16588843/why-does-try-with-resource-require-a-local-variable


http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java@386
PS3, Line 386: Closeable c
> Same here.
See above



--
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: 4
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: Thu, 08 Mar 2018 02:44:54 +
Gerrit-HasComments: Yes

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

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

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


Patch Set 4:

(11 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@140
PS3, Line 140:  * the ticket cache to obtain a new ticket with a later 
expiration time. So, if
Do we need to mention that Kudu will refuse the re-login attempt if the 
principal switched?


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@95
PS3, Line 95: static
'static' is redundant for inner enums.


http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java@107
PS3, Line 107: .
Nit: add ')'


http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java@130
PS3, Line 130:* @param subject JAAS Subject that the client's credentials 
are stored in
Nit: remove the extra @param.


http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java@171
PS3, Line 171:   public void refreshSubject() {
Nit: Maybe add a brief comment in front of the function since it is public.


http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java@191
PS3, Line 191:
Extra line.


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

http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java@55
PS3, Line 55: static
Redundant.


http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java@118
PS3, Line 118: miniCluster.killMasters();
I do not quite follow why do we have to kill the masters. Does it mean as long 
as the client has an existing connection, even though the credentials expired 
the client is still able to work fine?


http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java@273
PS3, Line 273:   public void testRenewAndReacquireKeberosCredentials() throws 
Exception {
Nit: addd a comment here?


http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java@355
PS3, Line 355: Closeable c
Not used?


http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java@386
PS3, Line 386: Closeable c
Same here.



--
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: 4
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 21:05:04 +
Gerrit-HasComments: Yes

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

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
..


Patch Set 3:

(8 comments)

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
Done


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 t
Done


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
Done


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
Done


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
Done


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
Done


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?
Done


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
good catch, put the multiplication on the wrong side!



--
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:51:33 +
Gerrit-HasComments: Yes

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

2018-03-07 Thread Todd Lipcon (Code Review) 
Hello Alexey Serbin, Dan Burkert, Jean-Daniel Cryans, Kudu Jenkins, Adar Dembo, 
Hao Hao, Anonymous Coward #380,

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

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

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

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

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

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, 786 insertions(+), 97 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/50/9050/4
--
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: 4
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

[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