kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From granthe...@apache.org
Subject kudu git commit: KUDU-2379. java: exportAuthenticationCredentials must connect to the cluster
Date Wed, 28 Mar 2018 03:13:00 GMT
Repository: kudu
Updated Branches:
  refs/heads/branch-1.7.x 66b827240 -> 608b5a3d3


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 <hao.hao@cloudera.com>
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert <danburkert@apache.org>
(cherry picked from commit aa94d16fda6cd6f3f413decf46bf9353680480c7)
Reviewed-on: http://gerrit.cloudera.org:8080/9832
Reviewed-by: Grant Henke <granthenke@apache.org>
Tested-by: Grant Henke <granthenke@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/608b5a3d
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/608b5a3d
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/608b5a3d

Branch: refs/heads/branch-1.7.x
Commit: 608b5a3d3eb20b9787d213c2ef7e801cd8fa0144
Parents: 66b8272
Author: Todd Lipcon <todd@apache.org>
Authored: Mon Mar 26 20:18:29 2018 -0700
Committer: Grant Henke <granthenke@apache.org>
Committed: Wed Mar 28 03:11:40 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/608b5a3d/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/608b5a3d/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/608b5a3d/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.
    */


Mime
View raw message