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

[Todd Lipcon (Code Review)]

Todd Lipcon has removed Anonymous Coward #314 from this change.  ( 
http://gerrit.cloudera.org:8080/9050 )

Change subject: WIP: KUDU-2264. Automatically attempt to re-acquire Kerberos 
credentials before expiration
..


Removed reviewer null.
--
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: deleteReviewer
Gerrit-Change-Id: I514253e0a7f067dbc8ffe4eaf5a7a2c32900b539
Gerrit-Change-Number: 9050
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

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

[Todd Lipcon (Code Review)]

Hello Alexey Serbin, Dan Burkert, Kudu Jenkins, Anonymous Coward #314, Adar 
Dembo,

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

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

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

Change subject: WIP: KUDU-2264. Automatically attempt to re-acquire Kerberos 
credentials before expiration
..

WIP: KUDU-2264. Automatically attempt to re-acquire Kerberos credentials before 
expiration

WIP because it needs a bit more testing and a lot of documentation of
the new behavior and best practices

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/KuduException.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/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
11 files changed, 560 insertions(+), 42 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/50/9050/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: newpatchset
Gerrit-Change-Id: I514253e0a7f067dbc8ffe4eaf5a7a2c32900b539
Gerrit-Change-Number: 9050
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Anonymous Coward #314
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

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

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9050 )

Change subject: WIP: KUDU-2264. Automatically attempt to re-acquire Kerberos 
credentials before expiration
..


Patch Set 1:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/9050/1/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/1/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java@90
PS1, Line 90:   private Object subjectLock = new Object();
> add 'final'
Done


http://gerrit.cloudera.org:8080/#/c/9050/1/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/1/java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java@78
PS1, Line 78: logins
> 'a new subject is logged in'
Done


http://gerrit.cloudera.org:8080/#/c/9050/1/java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java@186
PS1, Line 186: if (principal.getName().equals("krbtgt/" + 
principal.getRealm() +
> simplify this to
Done


http://gerrit.cloudera.org:8080/#/c/9050/1/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/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java@107
PS1, Line 107:   Thread.sleep(5000);
> Could you add an assert which guarantees the if branch is actually taken? R
changed the loop time to ensure that it always runs for twice the renewable 
lifetime, rather than 15 loops


http://gerrit.cloudera.org:8080/#/c/9050/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java@128
PS1, Line 128:   // TODO(todd) add test which externally provides a subject and 
ensures that it doesn't
> I guess one downside of removing the Java MiniKdc is that this becomes much
wasn't that bad. stay tuned.


http://gerrit.cloudera.org:8080/#/c/9050/1/src/kudu/tools/tool.proto
File src/kudu/tools/tool.proto:

http://gerrit.cloudera.org:8080/#/c/9050/1/src/kudu/tools/tool.proto@157
PS1, Line 157:   // is kinitted by default when the cluster starts.
> Instead of documenting this, consider adding it as the protobuf field defau
Done


http://gerrit.cloudera.org:8080/#/c/9050/1/src/kudu/tools/tool_action_test.cc
File src/kudu/tools/tool_action_test.cc:

http://gerrit.cloudera.org:8080/#/c/9050/1/src/kudu/tools/tool_action_test.cc@283
PS1, Line 283: case ControlShellRequestPB::kKinit:
> Just passing through, but could you add a test case for this in kudu-tool-t
Done


http://gerrit.cloudera.org:8080/#/c/9050/1/src/kudu/tools/tool_action_test.cc@288
PS1, Line 288:   string username = req.kinit().has_username() ? 
req.kinit().username() : "test-admin";
> This dance becomes unnecessary with the default defined in the .proto.
Done



--
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: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 23:42:18 +
Gerrit-HasComments: Yes

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

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9050 )

Change subject: WIP: KUDU-2264. Automatically attempt to re-acquire Kerberos 
credentials before expiration
..


Patch Set 1:

https://stackoverflow.com/questions/34616676/should-i-call-ugi-checktgtandreloginfromkeytab-before-every-action-on-hadoop
 is a nice read btw


--
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: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 20:16:33 +
Gerrit-HasComments: No

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

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9050 )

Change subject: WIP: KUDU-2264. Automatically attempt to re-acquire Kerberos 
credentials before expiration
..


Patch Set 1:

I'm not certain it avoids the race. UGI's "relogin" code would end up logging 
out the current Subject (which would be used by Kudu) and then logging it back 
in. We'd have to have our own code which does a synchronized 
(UserGroupInformation.class) { ... } or something in order to avoid potentially 
accessing the Subject while it's in the process of re-login.

Alternatively we could add some code that attempts to retry authentication 
failures if we previously had kerberos tickets and then notice that they 
disappeared?


--
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: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 20:09:45 +
Gerrit-HasComments: No

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

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9050 )

Change subject: WIP: KUDU-2264. Automatically attempt to re-acquire Kerberos 
credentials before expiration
..


Patch Set 1:

Would there be any advantage to that?  I think given that the implementation 
here is pretty straightforward and that it avoids the UGI re-acquisition race 
we discussed offline that this solution is cleaner and better.


--
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: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 19:48:42 +
Gerrit-HasComments: No

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

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9050 )

Change subject: WIP: KUDU-2264. Automatically attempt to re-acquire Kerberos 
credentials before expiration
..


Patch Set 1:

Thanks for the review Dan. Before I go finish up the patch, wanted to make sure 
you think the overall policy is a reasonable one for the client. Should we try 
to do something to automatically hook into hadoop's UGI class if we find such a 
class lying around by reflection?


--
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: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 19:42:02 +
Gerrit-HasComments: No

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

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9050 )

Change subject: WIP: KUDU-2264. Automatically attempt to re-acquire Kerberos 
credentials before expiration
..


Patch Set 1:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/9050/1/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/1/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java@90
PS1, Line 90:   private Object subjectLock = new Object();
add 'final'


http://gerrit.cloudera.org:8080/#/c/9050/1/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/1/java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java@78
PS1, Line 78: logins
'a new subject is logged in'


http://gerrit.cloudera.org:8080/#/c/9050/1/java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java@186
PS1, Line 186: if (principal.getName().equals("krbtgt/" + 
principal.getRealm() +
simplify this to

  return principal.getName().equals("krbtgt/" + principal.getRealm() + "@" + 
principal.getRealm())


http://gerrit.cloudera.org:8080/#/c/9050/1/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/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java@107
PS1, Line 107:   Thread.sleep(5000);
Could you add an assert which guarantees the if branch is actually taken? Right 
now the test is a bit brittle to the TICKED_LIFETIME_SECS value changing in the 
other file, since the number of loops and sleep time is constant.


http://gerrit.cloudera.org:8080/#/c/9050/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java@128
PS1, Line 128:   // TODO(todd) add test which externally provides a subject and 
ensures that it doesn't
I guess one downside of removing the Java MiniKdc is that this becomes much 
harder.


http://gerrit.cloudera.org:8080/#/c/9050/1/src/kudu/tools/tool.proto
File src/kudu/tools/tool.proto:

http://gerrit.cloudera.org:8080/#/c/9050/1/src/kudu/tools/tool.proto@157
PS1, Line 157:   // is kinitted by default when the cluster starts.
Instead of documenting this, consider adding it as the protobuf field default:

  optional string usernam = 1 [default = "test-admin"];


http://gerrit.cloudera.org:8080/#/c/9050/1/src/kudu/tools/tool_action_test.cc
File src/kudu/tools/tool_action_test.cc:

http://gerrit.cloudera.org:8080/#/c/9050/1/src/kudu/tools/tool_action_test.cc@288
PS1, Line 288:   string username = req.kinit().has_username() ? 
req.kinit().username() : "test-admin";
This dance becomes unnecessary with the default defined in the .proto.



--
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: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 18 Jan 2018 19:06:49 +
Gerrit-HasComments: Yes

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

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9050 )

Change subject: WIP: KUDU-2264. Automatically attempt to re-acquire Kerberos 
credentials before expiration
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9050/1/src/kudu/tools/tool_action_test.cc
File src/kudu/tools/tool_action_test.cc:

http://gerrit.cloudera.org:8080/#/c/9050/1/src/kudu/tools/tool_action_test.cc@283
PS1, Line 283: case ControlShellRequestPB::kKinit:
Just passing through, but could you add a test case for this in kudu-tool-test? 
There's a control shell test you can piggy-back on.



--
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: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 18 Jan 2018 02:38:08 +
Gerrit-HasComments: Yes

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

[Todd Lipcon (Code Review)]

Hello Dan Burkert,

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

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

to review the following change.


Change subject: WIP: KUDU-2264. Automatically attempt to re-acquire Kerberos 
credentials before expiration
..

WIP: KUDU-2264. Automatically attempt to re-acquire Kerberos credentials before 
expiration

WIP because it needs a bit more testing and a lot of documentation of
the new behavior and best practices

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/KuduException.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/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/tool.proto
M src/kudu/tools/tool_action_test.cc
10 files changed, 305 insertions(+), 28 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/50/9050/1
--
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: newchange
Gerrit-Change-Id: I514253e0a7f067dbc8ffe4eaf5a7a2c32900b539
Gerrit-Change-Number: 9050
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert