[kudu-CR] WIP: KUDU-2264. Automatically attempt to re-acquire Kerberos credentials before expiration
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 LipconGerrit-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
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 LipconGerrit-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 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 LipconGerrit-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 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 LipconGerrit-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 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 LipconGerrit-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 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 LipconGerrit-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 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 LipconGerrit-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 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 LipconGerrit-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 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 LipconGerrit-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
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 LipconGerrit-Reviewer: Dan Burkert