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 <t...@apache.org> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Dan Burkert <d...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Comment-Date: Thu, 18 Jan 2018 23:42:18 +0000 Gerrit-HasComments: Yes
