Repository: kudu Updated Branches: refs/heads/master fc833ca72 -> aa94d16fd
KUDU-2379. java: exportAuthenticationCredentials must connect to the cluster In the case that exportAuthenticationCredentials is called before the client has connected to the cluster, it needs to trigger a connection in order to obtain credentials. KUDU-2259 broke this when it changed exportAuthenticationCredentials() to return a non-null credential in this case. The fix just tracks whether the client has successfully connected to a cluster, and if it has not, makes sure that it has done so before generating credentials. Tested manually using spark2-submit against a secure cluster. Prior to this fix, it did not succeed. A new unit test is included as well. Change-Id: Icd1b69abadb4b9c97f6c8f7ed38897c33ca7ff72 Reviewed-on: http://gerrit.cloudera.org:8080/9814 Reviewed-by: Hao Hao <[email protected]> Tested-by: Kudu Jenkins Reviewed-by: Dan Burkert <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/kudu/repo Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/aa94d16f Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/aa94d16f Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/aa94d16f Branch: refs/heads/master Commit: aa94d16fda6cd6f3f413decf46bf9353680480c7 Parents: fc833ca Author: Todd Lipcon <[email protected]> Authored: Mon Mar 26 20:18:29 2018 -0700 Committer: Todd Lipcon <[email protected]> Committed: Tue Mar 27 23:16:48 2018 +0000 ---------------------------------------------------------------------- .../org/apache/kudu/client/AsyncKuduClient.java | 17 ++++++++++++++--- .../org/apache/kudu/client/SecurityContext.java | 9 ++++++++- .../java/org/apache/kudu/client/TestSecurity.java | 16 ++++++++++++++++ 3 files changed, 38 insertions(+), 4 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/aa94d16f/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java ---------------------------------------------------------------------- diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java b/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java index 9c52748..1bf2184 100644 --- a/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java +++ b/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java @@ -307,6 +307,14 @@ public class AsyncKuduClient implements AutoCloseable { private long lastPropagatedTimestamp = NO_TIMESTAMP; /** + * Set to true once we have connected to a master at least once. + * + * This determines whether exportAuthenticationCredentials() needs to + * proactively connect to the cluster to obtain a token. + */ + private volatile boolean hasConnectedToMaster = false; + + /** * Semaphore used to rate-limit master lookups * Once we have more than this number of concurrent master lookups, we'll * start to throttle ourselves slightly. @@ -792,9 +800,11 @@ public class AsyncKuduClient implements AutoCloseable { */ @InterfaceStability.Unstable public Deferred<byte[]> exportAuthenticationCredentials() { - byte[] authnData = securityContext.exportAuthenticationCredentials(); - if (authnData != null) { - return Deferred.fromResult(authnData); + // If we've already connected to the master, use the authentication + // credentials that we received when we connected. + if (hasConnectedToMaster) { + return Deferred.fromResult( + securityContext.exportAuthenticationCredentials()); } // We have no authn data -- connect to the master, which will fetch // new info. @@ -1521,6 +1531,7 @@ public class AsyncKuduClient implements AutoCloseable { e.getMessage()); } } + hasConnectedToMaster = true; // Translate the located master into a TableLocations // since the rest of our locations caching code expects this type. http://git-wip-us.apache.org/repos/asf/kudu/blob/aa94d16f/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java ---------------------------------------------------------------------- diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java b/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java index 3183b07..d921fc5 100644 --- a/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java +++ b/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java @@ -310,7 +310,14 @@ class SecurityContext { checkUserMatches(authnToken, pb.getAuthnToken()); } - authnToken = pb.getAuthnToken(); + LOG.debug("Importing authentication credentials with {} authn token, " + + "{} cert(s), and realUser={}", + pb.hasAuthnToken() ? "one" : "no", + pb.getCaCertDersCount(), + pb.hasRealUser() ? pb.getRealUser() : "<none>"); + if (pb.hasAuthnToken()) { + authnToken = pb.getAuthnToken(); + } trustCertificates(pb.getCaCertDersList()); if (pb.hasRealUser()) { http://git-wip-us.apache.org/repos/asf/kudu/blob/aa94d16f/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java ---------------------------------------------------------------------- diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java b/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java index 2af042d..762d12c 100644 --- a/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java +++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java @@ -166,6 +166,22 @@ public class TestSecurity { } /** + * Regression test for KUDU-2379: if the first usage of a client + * is to export credentials, that should trigger a connection to the + * cluster rather than returning empty credentials. + */ + @Test(timeout=60000) + public void testExportCredentialsBeforeAnyOtherAccess() throws IOException { + startCluster(ImmutableSet.<Option>of()); + try (KuduClient c = createClient()) { + AuthenticationCredentialsPB pb = AuthenticationCredentialsPB.parseFrom( + c.exportAuthenticationCredentials()); + Assert.assertTrue(pb.hasAuthnToken()); + Assert.assertTrue(pb.getCaCertDersCount() > 0); + } + } + + /** * Test that if, for some reason, the client has a token but no CA certs, it * will emit an appropriate error message in the exception. */
