aurora-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jsir...@apache.org
Subject aurora git commit: Simplify `Credentials`; kill `ZooKeeperClient` dep.
Date Wed, 13 Apr 2016 22:11:42 GMT
Repository: aurora
Updated Branches:
  refs/heads/master ea7648702 -> 8d0473cb5


Simplify `Credentials`; kill `ZooKeeperClient` dep.

The Curator discovery code will need to be configured from the same
command line flags and code as the commons discovery code.  This
simplifies Credentials to be a simple struct and adapts from the
`Credentials.NONE` null-object to use of `Optional` in consumers.

Bugs closed: AURORA-1468

Reviewed at https://reviews.apache.org/r/46133/


Project: http://git-wip-us.apache.org/repos/asf/aurora/repo
Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/8d0473cb
Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/8d0473cb
Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/8d0473cb

Branch: refs/heads/master
Commit: 8d0473cb52296902b7ca73ddf9664c91647aa482
Parents: ea76487
Author: John Sirois <jsirois@apache.org>
Authored: Wed Apr 13 16:11:38 2016 -0600
Committer: John Sirois <john.sirois@gmail.com>
Committed: Wed Apr 13 16:11:38 2016 -0600

----------------------------------------------------------------------
 .../aurora/common/zookeeper/Credentials.java    |  90 ++++++++++++
 .../common/zookeeper/ZooKeeperClient.java       | 147 ++-----------------
 .../testing/BaseZooKeeperClientTest.java        |  15 +-
 .../aurora/common/zookeeper/GroupTest.java      |   3 +-
 .../common/zookeeper/ZooKeeperClientTest.java   |  37 +----
 .../discovery/ServiceDiscoveryModule.java       |  13 +-
 .../log/mesos/MesosLogStreamModule.java         |  30 ++--
 .../guice/client/ZooKeeperClientModule.java     |  21 ++-
 .../client/flagged/FlaggedClientConfig.java     |  20 +--
 .../aurora/scheduler/app/SchedulerIT.java       |   6 +-
 10 files changed, 172 insertions(+), 210 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/aurora/blob/8d0473cb/commons/src/main/java/org/apache/aurora/common/zookeeper/Credentials.java
----------------------------------------------------------------------
diff --git a/commons/src/main/java/org/apache/aurora/common/zookeeper/Credentials.java b/commons/src/main/java/org/apache/aurora/common/zookeeper/Credentials.java
new file mode 100644
index 0000000..18319a3
--- /dev/null
+++ b/commons/src/main/java/org/apache/aurora/common/zookeeper/Credentials.java
@@ -0,0 +1,90 @@
+/**
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.aurora.common.zookeeper;
+
+import com.google.common.base.Objects;
+import com.google.common.base.Preconditions;
+
+import org.apache.aurora.common.base.MorePreconditions;
+import org.apache.commons.lang.builder.EqualsBuilder;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Encapsulates a user's ZooKeeper credentials.
+ */
+public final class Credentials {
+
+  /**
+   * Creates a set of credentials for the ZooKeeper digest authentication mechanism.
+   *
+   * @param username the username to authenticate with
+   * @param password the password to authenticate with
+   * @return a set of credentials that can be used to authenticate the zoo keeper client
+   */
+  public static Credentials digestCredentials(String username, String password) {
+    MorePreconditions.checkNotBlank(username);
+    Preconditions.checkNotNull(password);
+
+    // TODO(John Sirois): DigestAuthenticationProvider is broken - uses platform default
charset
+    // (on server) and so we just have to hope here that clients are deployed in compatible
jvms.
+    // Consider writing and installing a version of DigestAuthenticationProvider that controls
its
+    // Charset explicitly.
+    return new Credentials("digest", (username + ":" + password).getBytes());
+  }
+
+  private final String scheme;
+  private final byte[] authToken;
+
+  public Credentials(String scheme, byte[] authToken) {
+    this.scheme = MorePreconditions.checkNotBlank(scheme);
+    this.authToken = requireNonNull(authToken);
+  }
+
+  /**
+   * Returns the authentication scheme these credentials are for.
+   *
+   * @return the scheme these credentials are for.
+   */
+  public String scheme() {
+    return scheme;
+  }
+
+  /**
+   * Returns the authentication token.
+   *
+   * @return the authentication token.
+   */
+  public byte[] authToken() {
+    return authToken;
+  }
+
+  @Override
+  public boolean equals(Object o) {
+    if (!(o instanceof Credentials)) {
+      return false;
+    }
+
+    Credentials other = (Credentials) o;
+    return new EqualsBuilder()
+        .append(scheme, other.scheme())
+        .append(authToken, other.authToken())
+        .isEquals();
+  }
+
+  @Override
+  public int hashCode() {
+    return Objects.hashCode(scheme, authToken);
+  }
+}

http://git-wip-us.apache.org/repos/asf/aurora/blob/8d0473cb/commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperClient.java
----------------------------------------------------------------------
diff --git a/commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperClient.java
b/commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperClient.java
index 41ae035..ce243fb 100644
--- a/commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperClient.java
+++ b/commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperClient.java
@@ -23,23 +23,17 @@ import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 
-import javax.annotation.Nullable;
-
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Joiner;
-import com.google.common.base.Objects;
 import com.google.common.base.Optional;
 import com.google.common.base.Preconditions;
-import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 
 import org.apache.aurora.common.base.Command;
-import org.apache.aurora.common.base.MorePreconditions;
 import org.apache.aurora.common.net.InetSocketAddressHelper;
 import org.apache.aurora.common.quantity.Amount;
 import org.apache.aurora.common.quantity.Time;
-import org.apache.commons.lang.builder.EqualsBuilder;
 import org.apache.zookeeper.KeeperException;
 import org.apache.zookeeper.KeeperException.SessionExpiredException;
 import org.apache.zookeeper.WatchedEvent;
@@ -60,119 +54,11 @@ public class ZooKeeperClient {
    * Indicates an error connecting to a zookeeper cluster.
    */
   public class ZooKeeperConnectionException extends Exception {
-    public ZooKeeperConnectionException(String message, Throwable cause) {
+    ZooKeeperConnectionException(String message, Throwable cause) {
       super(message, cause);
     }
   }
 
-  /**
-   * Encapsulates a user's credentials and has the ability to authenticate them through a
-   * {@link ZooKeeper} client.
-   */
-  public interface Credentials {
-
-    /**
-     * A set of {@code Credentials} that performs no authentication.
-     */
-    Credentials NONE = new Credentials() {
-      @Override public void authenticate(ZooKeeper zooKeeper) {
-        // noop
-      }
-
-      @Override public String scheme() {
-        return null;
-      }
-
-      @Override public byte[] authToken() {
-        return null;
-      }
-    };
-
-    /**
-     * Authenticates these credentials against the given {@code ZooKeeper} client.
-     *
-     * @param zooKeeper the client to authenticate
-     */
-    void authenticate(ZooKeeper zooKeeper);
-
-    /**
-     * Returns the authentication scheme these credentials are for.
-     *
-     * @return the scheme these credentials are for or {@code null} if no authentication
is
-     *     intended.
-     */
-    @Nullable
-    String scheme();
-
-    /**
-     * Returns the authentication token.
-     *
-     * @return the authentication token or {@code null} if no authentication is intended.
-     */
-    @Nullable
-    byte[] authToken();
-  }
-
-  /**
-   * Creates a set of credentials for the zoo keeper digest authentication mechanism.
-   *
-   * @param username the username to authenticate with
-   * @param password the password to authenticate with
-   * @return a set of credentials that can be used to authenticate the zoo keeper client
-   */
-  public static Credentials digestCredentials(String username, String password) {
-    MorePreconditions.checkNotBlank(username);
-    Preconditions.checkNotNull(password);
-
-    // TODO(John Sirois): DigestAuthenticationProvider is broken - uses platform default
charset
-    // (on server) and so we just have to hope here that clients are deployed in compatible
jvms.
-    // Consider writing and installing a version of DigestAuthenticationProvider that controls
its
-    // Charset explicitly.
-    return credentials("digest", (username + ":" + password).getBytes());
-  }
-
-  /**
-   * Creates a set of credentials for the given authentication {@code scheme}.
-   *
-   * @param scheme the scheme to authenticate with
-   * @param authToken the authentication token
-   * @return a set of credentials that can be used to authenticate the zoo keeper client
-   */
-  public static Credentials credentials(final String scheme, final byte[] authToken) {
-    MorePreconditions.checkNotBlank(scheme);
-    Preconditions.checkNotNull(authToken);
-
-    return new Credentials() {
-      @Override public void authenticate(ZooKeeper zooKeeper) {
-        zooKeeper.addAuthInfo(scheme, authToken);
-      }
-
-      @Override public String scheme() {
-        return scheme;
-      }
-
-      @Override public byte[] authToken() {
-        return authToken;
-      }
-
-      @Override public boolean equals(Object o) {
-        if (!(o instanceof Credentials)) {
-          return false;
-        }
-
-        Credentials other = (Credentials) o;
-        return new EqualsBuilder()
-            .append(scheme, other.scheme())
-            .append(authToken, other.authToken())
-            .isEquals();
-      }
-
-      @Override public int hashCode() {
-        return Objects.hashCode(scheme, authToken);
-      }
-    };
-  }
-
   private final class SessionState {
     private final long sessionId;
     private final byte[] sessionPasswd;
@@ -188,7 +74,7 @@ public class ZooKeeperClient {
   private static final Amount<Long,Time> WAIT_FOREVER = Amount.of(0L, Time.MILLISECONDS);
 
   private final int sessionTimeoutMs;
-  private final Credentials credentials;
+  private final Optional<Credentials> credentials;
   private final String zooKeeperServers;
   // GuardedBy "this", but still volatile for tests, where we want to be able to see writes
   // made from within long synchronized blocks.
@@ -225,7 +111,7 @@ public class ZooKeeperClient {
    */
   public ZooKeeperClient(Amount<Integer, Time> sessionTimeout,
       Iterable<InetSocketAddress> zooKeeperServers) {
-    this(sessionTimeout, Credentials.NONE, Optional.<String> absent(), zooKeeperServers);
+    this(sessionTimeout, Optional.absent(), Optional.absent(), zooKeeperServers);
   }
 
   /**
@@ -240,7 +126,10 @@ public class ZooKeeperClient {
    */
   public ZooKeeperClient(Amount<Integer, Time> sessionTimeout, Credentials credentials,
       InetSocketAddress zooKeeperServer, InetSocketAddress... zooKeeperServers) {
-    this(sessionTimeout, credentials, Optional.<String> absent(), combine(zooKeeperServer,
zooKeeperServers));
+    this(sessionTimeout,
+        Optional.of(credentials),
+        Optional.absent(),
+        combine(zooKeeperServer, zooKeeperServers));
   }
 
   /**
@@ -254,7 +143,10 @@ public class ZooKeeperClient {
    */
   public ZooKeeperClient(Amount<Integer, Time> sessionTimeout, Credentials credentials,
       Iterable<InetSocketAddress> zooKeeperServers) {
-        this(sessionTimeout, credentials, Optional.<String> absent(), zooKeeperServers);
+        this(sessionTimeout,
+            Optional.of(credentials),
+            Optional.absent(),
+            zooKeeperServers);
       }
 
   /**
@@ -267,7 +159,7 @@ public class ZooKeeperClient {
    * @param chrootPath an optional chroot path
    * @param zooKeeperServers the set of servers forming the ZK cluster
    */
-  public ZooKeeperClient(Amount<Integer, Time> sessionTimeout, Credentials credentials,
+  public ZooKeeperClient(Amount<Integer, Time> sessionTimeout, Optional<Credentials>
credentials,
       Optional<String> chrootPath, Iterable<InetSocketAddress> zooKeeperServers)
{
     this.sessionTimeoutMs = Preconditions.checkNotNull(sessionTimeout).as(Time.MILLISECONDS);
     this.credentials = Preconditions.checkNotNull(credentials);
@@ -303,16 +195,6 @@ public class ZooKeeperClient {
   }
 
   /**
-   * Returns true if this client has non-empty credentials set.  For example, returns {@code
false}
-   * if this client was constructed with {@link Credentials#NONE}.
-   *
-   * @return {@code true} if this client is configured with non-empty credentials.
-   */
-  public boolean hasCredentials() {
-    return !Strings.isNullOrEmpty(credentials.scheme()) && (credentials.authToken()
!= null);
-  }
-
-  /**
    * Returns the current active ZK connection or establishes a new one if none has yet been
    * established or a previous connection was disconnected or had its session time out. 
This method
    * will attempt to re-use sessions when possible.  Equivalent to:
@@ -395,7 +277,10 @@ public class ZooKeeperClient {
           throw ex;
         }
       }
-      credentials.authenticate(zooKeeper);
+      if (credentials.isPresent()) {
+        Credentials zkCredentials = credentials.get();
+        zooKeeper.addAuthInfo(zkCredentials.scheme(), zkCredentials.authToken());
+      }
 
       sessionState = new SessionState(zooKeeper.getSessionId(), zooKeeper.getSessionPasswd());
     }

http://git-wip-us.apache.org/repos/asf/aurora/blob/8d0473cb/commons/src/main/java/org/apache/aurora/common/zookeeper/testing/BaseZooKeeperClientTest.java
----------------------------------------------------------------------
diff --git a/commons/src/main/java/org/apache/aurora/common/zookeeper/testing/BaseZooKeeperClientTest.java
b/commons/src/main/java/org/apache/aurora/common/zookeeper/testing/BaseZooKeeperClientTest.java
index b9eaedb..ba09279 100644
--- a/commons/src/main/java/org/apache/aurora/common/zookeeper/testing/BaseZooKeeperClientTest.java
+++ b/commons/src/main/java/org/apache/aurora/common/zookeeper/testing/BaseZooKeeperClientTest.java
@@ -22,6 +22,7 @@ import com.google.common.collect.ImmutableList;
 
 import org.apache.aurora.common.quantity.Amount;
 import org.apache.aurora.common.quantity.Time;
+import org.apache.aurora.common.zookeeper.Credentials;
 import org.apache.aurora.common.zookeeper.ZooKeeperClient;
 
 /**
@@ -89,15 +90,15 @@ public abstract class BaseZooKeeperClientTest extends BaseZooKeeperTest
{
    * with the default session timeout.
    */
   protected final ZooKeeperClient createZkClient() {
-    return createZkClient(ZooKeeperClient.Credentials.NONE);
+    return createZkClient(defaultSessionTimeout, Optional.absent(), Optional.absent());
   }
 
   /**
    * Returns a new authenticated zookeeper client connected to the in-process zookeeper server
with
    * the default session timeout.
    */
-  protected final ZooKeeperClient createZkClient(ZooKeeperClient.Credentials credentials)
{
-    return createZkClient(defaultSessionTimeout, credentials, Optional.absent());
+  protected final ZooKeeperClient createZkClient(Credentials credentials) {
+    return createZkClient(defaultSessionTimeout, Optional.of(credentials), Optional.absent());
   }
 
   /**
@@ -106,7 +107,7 @@ public abstract class BaseZooKeeperClientTest extends BaseZooKeeperTest
{
    * with the given {@code username} and {@code password}.
    */
   protected final ZooKeeperClient createZkClient(String username, String password) {
-    return createZkClient(ZooKeeperClient.digestCredentials(username, password));
+    return createZkClient(Credentials.digestCredentials(username, password));
   }
 
   /**
@@ -114,7 +115,7 @@ public abstract class BaseZooKeeperClientTest extends BaseZooKeeperTest
{
    * with a custom {@code sessionTimeout}.
    */
   protected final ZooKeeperClient createZkClient(Amount<Integer, Time> sessionTimeout)
{
-    return createZkClient(sessionTimeout, ZooKeeperClient.Credentials.NONE, Optional.absent());
+    return createZkClient(sessionTimeout, Optional.absent(), Optional.absent());
   }
 
   /**
@@ -122,13 +123,13 @@ public abstract class BaseZooKeeperClientTest extends BaseZooKeeperTest
{
    * the default session timeout and the custom chroot path.
    */
   protected final ZooKeeperClient createZkClient(String chrootPath) {
-    return createZkClient(defaultSessionTimeout, ZooKeeperClient.Credentials.NONE,
+    return createZkClient(defaultSessionTimeout, Optional.absent(),
         Optional.of(chrootPath));
   }
 
   private ZooKeeperClient createZkClient(
       Amount<Integer, Time> sessionTimeout,
-      ZooKeeperClient.Credentials credentials,
+      Optional<Credentials> credentials,
       Optional<String> chrootPath) {
 
     ZooKeeperClient client = new ZooKeeperClient(sessionTimeout, credentials, chrootPath,

http://git-wip-us.apache.org/repos/asf/aurora/blob/8d0473cb/commons/src/test/java/org/apache/aurora/common/zookeeper/GroupTest.java
----------------------------------------------------------------------
diff --git a/commons/src/test/java/org/apache/aurora/common/zookeeper/GroupTest.java b/commons/src/test/java/org/apache/aurora/common/zookeeper/GroupTest.java
index 9127b6e..97a42d1 100644
--- a/commons/src/test/java/org/apache/aurora/common/zookeeper/GroupTest.java
+++ b/commons/src/test/java/org/apache/aurora/common/zookeeper/GroupTest.java
@@ -29,7 +29,6 @@ import org.apache.aurora.common.zookeeper.Group.GroupChangeListener;
 import org.apache.aurora.common.zookeeper.Group.JoinException;
 import org.apache.aurora.common.zookeeper.Group.Membership;
 import org.apache.aurora.common.zookeeper.Group.NodeScheme;
-import org.apache.aurora.common.zookeeper.ZooKeeperClient.Credentials;
 import org.apache.aurora.common.zookeeper.testing.BaseZooKeeperClientTest;
 import org.apache.zookeeper.ZooDefs.Ids;
 import org.junit.Before;
@@ -250,7 +249,7 @@ public class GroupTest extends BaseZooKeeperClientTest {
     String memberId = securedMembership.join().getMemberId();
 
     Group unauthenticatedObserver =
-        new Group(createZkClient(Credentials.NONE),
+        new Group(createZkClient(),
             Ids.READ_ACL_UNSAFE,
             "/secured/group/membership");
     RecordingListener unauthenticatedListener = new RecordingListener();

http://git-wip-us.apache.org/repos/asf/aurora/blob/8d0473cb/commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperClientTest.java
----------------------------------------------------------------------
diff --git a/commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperClientTest.java
b/commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperClientTest.java
index 537d41e..5eee235 100644
--- a/commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperClientTest.java
+++ b/commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperClientTest.java
@@ -20,7 +20,6 @@ import java.util.concurrent.atomic.AtomicReference;
 
 import org.apache.aurora.common.quantity.Amount;
 import org.apache.aurora.common.quantity.Time;
-import org.apache.aurora.common.zookeeper.ZooKeeperClient.Credentials;
 import org.apache.aurora.common.zookeeper.ZooKeeperClient.ZooKeeperConnectionException;
 import org.apache.aurora.common.zookeeper.testing.BaseZooKeeperClientTest;
 import org.apache.zookeeper.CreateMode;
@@ -31,7 +30,6 @@ import org.junit.Test;
 
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
@@ -163,7 +161,7 @@ public class ZooKeeperClientTest extends BaseZooKeeperClientTest {
         authenticatedClient.get().create(path, "42".getBytes(),
             ZooKeeperUtils.EVERYONE_READ_CREATOR_ALL, CreateMode.PERSISTENT));
 
-    ZooKeeperClient unauthenticatedClient = createZkClient(Credentials.NONE);
+    ZooKeeperClient unauthenticatedClient = createZkClient();
     assertEquals("42", getData(unauthenticatedClient, path));
     try {
       setData(unauthenticatedClient, path, "37");
@@ -187,39 +185,6 @@ public class ZooKeeperClientTest extends BaseZooKeeperClientTest {
   }
 
   @Test
-  public void testHasCredentials() {
-    assertFalse(createZkClient().hasCredentials());
-    assertFalse(createZkClient(Credentials.NONE).hasCredentials());
-    assertFalse(createZkClient(new Credentials() {
-      @Override
-      public void authenticate(ZooKeeper zooKeeper) {
-        // noop
-      }
-      @Override public String scheme() {
-        return "";
-      }
-      @Override public byte[] authToken() {
-        return new byte[0];
-      }
-    }).hasCredentials());
-
-    assertTrue(createZkClient("creator", "creator").hasCredentials());
-    assertTrue(createZkClient(new Credentials() {
-      @Override public void authenticate(ZooKeeper zooKeeper) {
-        // noop
-      }
-      @Override public String scheme() {
-        return "custom";
-      }
-      @Override public byte[] authToken() {
-        // a zero-length token should be ok - ZooKeeper says nothing about the validity of
token
-        // data a scheme can accept.
-        return new byte[0];
-      }
-    }).hasCredentials());
-  }
-
-  @Test
   public void testChrootPath() throws Exception {
     ZooKeeperClient rootClient = createZkClient();
     String rootPath = "/test";

http://git-wip-us.apache.org/repos/asf/aurora/blob/8d0473cb/src/main/java/org/apache/aurora/scheduler/discovery/ServiceDiscoveryModule.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/discovery/ServiceDiscoveryModule.java
b/src/main/java/org/apache/aurora/scheduler/discovery/ServiceDiscoveryModule.java
index c14162f..fa605cc 100644
--- a/src/main/java/org/apache/aurora/scheduler/discovery/ServiceDiscoveryModule.java
+++ b/src/main/java/org/apache/aurora/scheduler/discovery/ServiceDiscoveryModule.java
@@ -17,17 +17,18 @@ import java.util.List;
 
 import javax.inject.Singleton;
 
+import com.google.common.base.Optional;
 import com.google.inject.Exposed;
 import com.google.inject.PrivateModule;
 import com.google.inject.Provides;
 
 import org.apache.aurora.common.net.pool.DynamicHostSet;
 import org.apache.aurora.common.thrift.ServiceInstance;
+import org.apache.aurora.common.zookeeper.Credentials;
 import org.apache.aurora.common.zookeeper.ServerSetImpl;
 import org.apache.aurora.common.zookeeper.SingletonService;
 import org.apache.aurora.common.zookeeper.SingletonServiceImpl;
 import org.apache.aurora.common.zookeeper.ZooKeeperClient;
-import org.apache.aurora.common.zookeeper.ZooKeeperClient.Credentials;
 import org.apache.aurora.common.zookeeper.ZooKeeperUtils;
 import org.apache.aurora.scheduler.app.ServiceGroupMonitor;
 import org.apache.zookeeper.data.ACL;
@@ -44,9 +45,9 @@ public class ServiceDiscoveryModule extends PrivateModule {
   private static final Logger LOG = LoggerFactory.getLogger(ServiceDiscoveryModule.class);
 
   private final String serverSetPath;
-  private final Credentials zkCredentials;
+  private final Optional<Credentials> zkCredentials;
 
-  public ServiceDiscoveryModule(String serverSetPath, Credentials zkCredentials) {
+  public ServiceDiscoveryModule(String serverSetPath, Optional<Credentials> zkCredentials)
{
     this.serverSetPath = requireNonNull(serverSetPath);
     this.zkCredentials = requireNonNull(zkCredentials);
   }
@@ -62,11 +63,11 @@ public class ServiceDiscoveryModule extends PrivateModule {
   @Provides
   @Singleton
   List<ACL> provideAcls() {
-    if (zkCredentials == Credentials.NONE) {
+    if (zkCredentials.isPresent()) {
+      return ZooKeeperUtils.EVERYONE_READ_CREATOR_ALL;
+    } else {
       LOG.warn("Running without ZooKeeper digest credentials. ZooKeeper ACLs are disabled.");
       return ZooKeeperUtils.OPEN_ACL_UNSAFE;
-    } else {
-      return ZooKeeperUtils.EVERYONE_READ_CREATOR_ALL;
     }
   }
 

http://git-wip-us.apache.org/repos/asf/aurora/blob/8d0473cb/src/main/java/org/apache/aurora/scheduler/log/mesos/MesosLogStreamModule.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/log/mesos/MesosLogStreamModule.java
b/src/main/java/org/apache/aurora/scheduler/log/mesos/MesosLogStreamModule.java
index 5daafa9..b20ea5d 100644
--- a/src/main/java/org/apache/aurora/scheduler/log/mesos/MesosLogStreamModule.java
+++ b/src/main/java/org/apache/aurora/scheduler/log/mesos/MesosLogStreamModule.java
@@ -33,6 +33,7 @@ import org.apache.aurora.common.args.CmdLine;
 import org.apache.aurora.common.net.InetSocketAddressHelper;
 import org.apache.aurora.common.quantity.Amount;
 import org.apache.aurora.common.quantity.Time;
+import org.apache.aurora.common.zookeeper.Credentials;
 import org.apache.aurora.gen.storage.LogEntry;
 import org.apache.aurora.scheduler.log.mesos.LogInterface.ReaderInterface;
 import org.apache.aurora.scheduler.log.mesos.LogInterface.WriterInterface;
@@ -142,15 +143,26 @@ public class MesosLogStreamModule extends PrivateModule {
     String zkConnectString = Joiner.on(',').join(
         Iterables.transform(zkClientConfig.servers, InetSocketAddressHelper::toString));
 
-    return new Log(
-        QUORUM_SIZE.get(),
-        logPath.getAbsolutePath(),
-        zkConnectString,
-        zkClientConfig.sessionTimeout.getValue(),
-        zkClientConfig.sessionTimeout.getUnit().getTimeUnit(),
-        zkLogGroupPath,
-        zkClientConfig.credentials.scheme(),
-        zkClientConfig.credentials.authToken());
+    if (zkClientConfig.credentials.isPresent()) {
+      Credentials zkCredentials = zkClientConfig.credentials.get();
+      return new Log(
+          QUORUM_SIZE.get(),
+          logPath.getAbsolutePath(),
+          zkConnectString,
+          zkClientConfig.sessionTimeout.getValue(),
+          zkClientConfig.sessionTimeout.getUnit().getTimeUnit(),
+          zkLogGroupPath,
+          zkCredentials.scheme(),
+          zkCredentials.authToken());
+    } else {
+      return new Log(
+          QUORUM_SIZE.get(),
+          logPath.getAbsolutePath(),
+          zkConnectString,
+          zkClientConfig.sessionTimeout.getValue(),
+          zkClientConfig.sessionTimeout.getUnit().getTimeUnit(),
+          zkLogGroupPath);
+    }
   }
 
   @Provides

http://git-wip-us.apache.org/repos/asf/aurora/blob/8d0473cb/src/main/java/org/apache/aurora/scheduler/zookeeper/guice/client/ZooKeeperClientModule.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/zookeeper/guice/client/ZooKeeperClientModule.java
b/src/main/java/org/apache/aurora/scheduler/zookeeper/guice/client/ZooKeeperClientModule.java
index 4239a86..cab3691 100644
--- a/src/main/java/org/apache/aurora/scheduler/zookeeper/guice/client/ZooKeeperClientModule.java
+++ b/src/main/java/org/apache/aurora/scheduler/zookeeper/guice/client/ZooKeeperClientModule.java
@@ -19,6 +19,7 @@ import java.net.InetSocketAddress;
 
 import com.google.common.base.Optional;
 import com.google.common.base.Throwables;
+import com.google.common.collect.ImmutableList;
 import com.google.common.io.Files;
 import com.google.common.util.concurrent.AbstractIdleService;
 import com.google.inject.AbstractModule;
@@ -32,8 +33,8 @@ import org.apache.aurora.common.application.ShutdownRegistry;
 import org.apache.aurora.common.inject.Bindings.KeyFactory;
 import org.apache.aurora.common.quantity.Amount;
 import org.apache.aurora.common.quantity.Time;
+import org.apache.aurora.common.zookeeper.Credentials;
 import org.apache.aurora.common.zookeeper.ZooKeeperClient;
-import org.apache.aurora.common.zookeeper.ZooKeeperClient.Credentials;
 import org.apache.aurora.common.zookeeper.ZooKeeperUtils;
 import org.apache.aurora.common.zookeeper.testing.ZooKeeperTestServer;
 import org.apache.aurora.scheduler.SchedulerServicesModule;
@@ -139,7 +140,8 @@ public class ZooKeeperClientModule extends AbstractModule {
       return new ZooKeeperClient(
           config.sessionTimeout,
           config.credentials,
-          InetSocketAddress.createUnresolved("localhost", testServer.getPort()));
+          Optional.absent(), // chrootPath
+          ImmutableList.of(InetSocketAddress.createUnresolved("localhost", testServer.getPort())));
     }
   }
 
@@ -153,7 +155,7 @@ public class ZooKeeperClientModule extends AbstractModule {
     public final boolean inProcess;
     public final Amount<Integer, Time> sessionTimeout;
     public final Optional<String> chrootPath;
-    public final Credentials credentials;
+    public final Optional<Credentials> credentials;
 
     /**
      * Creates a new client configuration.
@@ -169,7 +171,7 @@ public class ZooKeeperClientModule extends AbstractModule {
         Optional<String> chrootPath,
         boolean inProcess,
         Amount<Integer, Time> sessionTimeout,
-        Credentials credentials) {
+        Optional<Credentials> credentials) {
 
       this.servers = servers;
       this.chrootPath = chrootPath;
@@ -187,10 +189,10 @@ public class ZooKeeperClientModule extends AbstractModule {
     public static ClientConfig create(Iterable<InetSocketAddress> servers) {
       return new ClientConfig(
           servers,
-          Optional.<String> absent(),
+          Optional.absent(), // chrootPath
           false,
           ZooKeeperUtils.DEFAULT_ZK_SESSION_TIMEOUT,
-          Credentials.NONE);
+          Optional.absent()); // credentials
     }
 
     /**
@@ -201,7 +203,12 @@ public class ZooKeeperClientModule extends AbstractModule {
      * @return A modified clone of this configuration.
      */
     public ClientConfig withCredentials(Credentials newCredentials) {
-      return new ClientConfig(servers, chrootPath, inProcess, sessionTimeout, newCredentials);
+      return new ClientConfig(
+          servers,
+          chrootPath,
+          inProcess,
+          sessionTimeout,
+          Optional.of(newCredentials));
     }
   }
 }

http://git-wip-us.apache.org/repos/asf/aurora/blob/8d0473cb/src/main/java/org/apache/aurora/scheduler/zookeeper/guice/client/flagged/FlaggedClientConfig.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/zookeeper/guice/client/flagged/FlaggedClientConfig.java
b/src/main/java/org/apache/aurora/scheduler/zookeeper/guice/client/flagged/FlaggedClientConfig.java
index 5b59d55..92f3673 100644
--- a/src/main/java/org/apache/aurora/scheduler/zookeeper/guice/client/flagged/FlaggedClientConfig.java
+++ b/src/main/java/org/apache/aurora/scheduler/zookeeper/guice/client/flagged/FlaggedClientConfig.java
@@ -16,6 +16,8 @@ package org.apache.aurora.scheduler.zookeeper.guice.client.flagged;
 import java.net.InetSocketAddress;
 import java.util.List;
 
+import javax.annotation.Nullable;
+
 import com.google.common.base.Optional;
 import com.google.common.base.Splitter;
 import com.google.common.collect.ImmutableList;
@@ -25,8 +27,7 @@ import org.apache.aurora.common.args.CmdLine;
 import org.apache.aurora.common.args.constraints.NotEmpty;
 import org.apache.aurora.common.quantity.Amount;
 import org.apache.aurora.common.quantity.Time;
-import org.apache.aurora.common.zookeeper.ZooKeeperClient;
-import org.apache.aurora.common.zookeeper.ZooKeeperClient.Credentials;
+import org.apache.aurora.common.zookeeper.Credentials;
 import org.apache.aurora.common.zookeeper.ZooKeeperUtils;
 import org.apache.aurora.scheduler.zookeeper.guice.client.ZooKeeperClientModule.ClientConfig;
 
@@ -52,7 +53,7 @@ public final class FlaggedClientConfig {
 
   @CmdLine(name = "zk_digest_credentials",
            help = "user:password to use when authenticating with ZooKeeper.")
-  private static final Arg<String> DIGEST_CREDENTIALS = Arg.create();
+  private static final Arg<String> DIGEST_CREDENTIALS = Arg.create(null);
 
   private FlaggedClientConfig() {
     // Utility class.
@@ -69,18 +70,19 @@ public final class FlaggedClientConfig {
         Optional.fromNullable(CHROOT_PATH.get()),
         IN_PROCESS.get(),
         SESSION_TIMEOUT.get(),
-        DIGEST_CREDENTIALS.hasAppliedValue()
-            ? getCredentials(DIGEST_CREDENTIALS.get())
-            : Credentials.NONE
-    );
+        getCredentials(DIGEST_CREDENTIALS.get()));
   }
 
-  private static Credentials getCredentials(String userAndPass) {
+  private static Optional<Credentials> getCredentials(@Nullable String userAndPass)
{
+    if (userAndPass == null) {
+      return Optional.absent();
+    }
+
     List<String> parts = ImmutableList.copyOf(Splitter.on(":").split(userAndPass));
     if (parts.size() != 2) {
       throw new IllegalArgumentException(
           "zk_digest_credentials must be formatted as user:pass");
     }
-    return ZooKeeperClient.digestCredentials(parts.get(0), parts.get(1));
+    return Optional.of(Credentials.digestCredentials(parts.get(0), parts.get(1)));
   }
 }

http://git-wip-us.apache.org/repos/asf/aurora/blob/8d0473cb/src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java b/src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java
index 89b2813..88d3e3e 100644
--- a/src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java
+++ b/src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java
@@ -44,9 +44,9 @@ import org.apache.aurora.common.application.Lifecycle;
 import org.apache.aurora.common.quantity.Amount;
 import org.apache.aurora.common.quantity.Data;
 import org.apache.aurora.common.stats.Stats;
+import org.apache.aurora.common.zookeeper.Credentials;
 import org.apache.aurora.common.zookeeper.ServerSetImpl;
 import org.apache.aurora.common.zookeeper.ZooKeeperClient;
-import org.apache.aurora.common.zookeeper.ZooKeeperClient.Credentials;
 import org.apache.aurora.common.zookeeper.testing.BaseZooKeeperClientTest;
 import org.apache.aurora.gen.ScheduleStatus;
 import org.apache.aurora.gen.ScheduledTask;
@@ -193,7 +193,7 @@ public class SchedulerIT extends BaseZooKeeperClientTest {
                     .setStatsUrlPrefix(STATS_URL_PREFIX)));
       }
     };
-    Credentials credentials = ZooKeeperClient.digestCredentials("mesos", "mesos");
+    Credentials credentials = Credentials.digestCredentials("mesos", "mesos");
     ClientConfig zkClientConfig = ClientConfig
         .create(ImmutableList.of(InetSocketAddress.createUnresolved("localhost", getPort())))
         .withCredentials(credentials);
@@ -204,7 +204,7 @@ public class SchedulerIT extends BaseZooKeeperClientTest {
             .add(new TierModule(TaskTestUtil.DEV_TIER_CONFIG))
             .add(new LogStorageModule())
             .add(new ZooKeeperClientModule(zkClientConfig))
-            .add(new ServiceDiscoveryModule(SERVERSET_PATH, credentials))
+            .add(new ServiceDiscoveryModule(SERVERSET_PATH, Optional.of(credentials)))
             .add(testModule)
             .build()
     );


Mime
View raw message