Add test for zookeeper auth config parsing
We have now split parsing of the curator config from actually building
the curator object.
When calling builder.build() the Curator framework waits for it to
connect to a zookeeper instance, which we don't have in this instance.
In order to be able to test the parsed configuration without the need to
start a Zookeeper instance, the refactoring in ZookeeperConfig.java was
required.
Change-Id: I0695678348e64b7fc7390d4eb5bbcec19f290475
diff --git a/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZkInit.java b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZkInit.java
index 71762c7..c62bbf5 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZkInit.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZkInit.java
@@ -72,7 +72,7 @@
injector.injectMembers(this);
try {
- try (CuratorFramework curator = new ZookeeperConfig(config).buildCurator()) {
+ try (CuratorFramework curator = new ZookeeperConfig(config).startCurator()) {
zkMigrations.migrate(injector, curator, versionManager.read());
}
} catch (StorageException e) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZkValidationModule.java b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZkValidationModule.java
index cf97adf..f847a95 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZkValidationModule.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZkValidationModule.java
@@ -39,7 +39,7 @@
DynamicItem.bind(binder(), GlobalRefDatabase.class)
.to(ZkSharedRefDatabase.class)
.in(Scopes.SINGLETON);
- bind(CuratorFramework.class).toInstance(cfg.buildCurator());
+ bind(CuratorFramework.class).toInstance(cfg.startCurator());
bind(ZkConnectionConfig.class)
.toInstance(
new ZkConnectionConfig(cfg.buildCasRetryPolicy(), cfg.getZkInterProcessLockTimeOut()));
diff --git a/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZookeeperConfig.java b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZookeeperConfig.java
index b6abb40..2f367c9 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZookeeperConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZookeeperConfig.java
@@ -191,7 +191,27 @@
connectionString);
}
- public CuratorFramework buildCurator() {
+ protected CuratorFrameworkFactory.Builder parseCuratorConfig() {
+ CuratorFrameworkFactory.Builder builder =
+ CuratorFrameworkFactory.builder()
+ .connectString(connectionString)
+ .sessionTimeoutMs(sessionTimeoutMs)
+ .connectionTimeoutMs(connectionTimeoutMs)
+ .retryPolicy(
+ new BoundedExponentialBackoffRetry(baseSleepTimeMs, maxSleepTimeMs, maxRetries))
+ .namespace(root);
+ if (zkUsername.isPresent() != zkPassword.isPresent()) {
+ throw new IllegalArgumentException(
+ "Only one between password or username for Zookeeper was set, please set both to successfully authenticate");
+ } else {
+ zkUsername
+ .flatMap(usr -> zkPassword.map(pwd -> usr + ":" + pwd))
+ .ifPresent(auth -> configureAuth(builder, auth));
+ }
+ return builder;
+ }
+
+ public CuratorFramework startCurator() {
if (isSSLEnabled) {
System.setProperty(
@@ -209,23 +229,7 @@
}
if (build == null) {
- CuratorFrameworkFactory.Builder builder =
- CuratorFrameworkFactory.builder()
- .connectString(connectionString)
- .sessionTimeoutMs(sessionTimeoutMs)
- .connectionTimeoutMs(connectionTimeoutMs)
- .retryPolicy(
- new BoundedExponentialBackoffRetry(baseSleepTimeMs, maxSleepTimeMs, maxRetries))
- .namespace(root);
- if (zkUsername.isPresent() != zkPassword.isPresent()) {
- throw new IllegalArgumentException(
- "Only one between password or username for Zookeeper was set, please set both to successfully authenticate");
- } else {
- zkUsername
- .flatMap(usr -> zkPassword.map(pwd -> usr + ":" + pwd))
- .ifPresent(auth -> configureAuth(builder, auth));
- }
- this.build = builder.build();
+ this.build = parseCuratorConfig().build();
this.build.start();
}
return this.build;
diff --git a/src/test/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZookeeperACLParsingTest.java b/src/test/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZookeeperACLParsingTest.java
new file mode 100644
index 0000000..b43c2c0
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZookeeperACLParsingTest.java
@@ -0,0 +1,108 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// 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 com.googlesource.gerrit.plugins.validation.dfsrefdb.zookeeper;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import com.google.gerrit.server.config.PluginConfigFactory;
+import java.nio.charset.StandardCharsets;
+import org.apache.curator.framework.CuratorFrameworkFactory;
+import org.eclipse.jgit.lib.Config;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TestName;
+
+public class ZookeeperACLParsingTest {
+ @Rule public TestName nameRule = new TestName();
+
+ String PLUGIN_NAME = "zookeeper";
+
+ @Test
+ public void shouldNotSetUpAuthIfNotProvided() {
+
+ PluginConfigFactory cfgFactory = mock(PluginConfigFactory.class);
+ when(cfgFactory.getGlobalPluginConfig(PLUGIN_NAME)).thenReturn(new Config());
+
+ ZookeeperConfig configuration = new ZookeeperConfig(cfgFactory, PLUGIN_NAME);
+
+ CuratorFrameworkFactory.Builder builder = configuration.parseCuratorConfig();
+
+ assertThat(builder.getAuthInfos()).isNull();
+ }
+
+ @Test
+ public void shouldCorrectlyParseAuthConfigIfProvided() {
+ Config pluginConfig = new Config();
+ pluginConfig.setString(
+ "ref-database", ZookeeperConfig.SUBSECTION, ZookeeperConfig.KEY_USERNAME, "globalrefdb");
+ pluginConfig.setString(
+ "ref-database",
+ ZookeeperConfig.SUBSECTION,
+ ZookeeperConfig.KEY_PASSWORD,
+ "globalrefdb-secret");
+
+ PluginConfigFactory cfgFactory = mock(PluginConfigFactory.class);
+ when(cfgFactory.getGlobalPluginConfig(PLUGIN_NAME)).thenReturn(pluginConfig);
+
+ ZookeeperConfig configuration = new ZookeeperConfig(cfgFactory, PLUGIN_NAME);
+
+ CuratorFrameworkFactory.Builder builder = configuration.parseCuratorConfig();
+
+ String authInfo = new String(builder.getAuthInfos().get(0).getAuth(), StandardCharsets.UTF_8);
+
+ assertThat(authInfo).isEqualTo("globalrefdb:globalrefdb-secret");
+ }
+
+ @Test
+ public void shouldNotParseAuthConfigIfUsernameIsNotProvided() {
+ Config pluginConfig = new Config();
+ pluginConfig.setString(
+ "ref-database", ZookeeperConfig.SUBSECTION, ZookeeperConfig.KEY_PASSWORD, "pwd");
+
+ PluginConfigFactory cfgFactory = mock(PluginConfigFactory.class);
+ when(cfgFactory.getGlobalPluginConfig(PLUGIN_NAME)).thenReturn(pluginConfig);
+
+ ZookeeperConfig configuration = new ZookeeperConfig(cfgFactory, PLUGIN_NAME);
+
+ Exception e =
+ assertThrows(IllegalArgumentException.class, () -> configuration.parseCuratorConfig());
+ assertTrue(
+ e.getMessage()
+ .contains(
+ "Only one between password or username for Zookeeper was set, please set both to successfully authenticate"));
+ }
+
+ @Test
+ public void shouldNotParseAuthConfigIfPasswordIsNotProvided() {
+ Config pluginConfig = new Config();
+ pluginConfig.setString(
+ "ref-database", ZookeeperConfig.SUBSECTION, ZookeeperConfig.KEY_USERNAME, "usr");
+
+ PluginConfigFactory cfgFactory = mock(PluginConfigFactory.class);
+ when(cfgFactory.getGlobalPluginConfig(PLUGIN_NAME)).thenReturn(pluginConfig);
+
+ ZookeeperConfig configuration = new ZookeeperConfig(cfgFactory, PLUGIN_NAME);
+
+ Exception e =
+ assertThrows(IllegalArgumentException.class, () -> configuration.parseCuratorConfig());
+ assertTrue(
+ e.getMessage()
+ .contains(
+ "Only one between password or username for Zookeeper was set, please set both to successfully authenticate"));
+ }
+}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZookeeperTestContainerSupport.java b/src/test/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZookeeperTestContainerSupport.java
index 366a83f..b2ac599 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZookeeperTestContainerSupport.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZookeeperTestContainerSupport.java
@@ -71,7 +71,7 @@
when(cfgFactory.getGlobalPluginConfig("zookeeper")).thenReturn(sharedRefDbConfig);
configuration = new ZookeeperConfig(cfgFactory, "zookeeper");
- this.curator = configuration.buildCurator();
+ this.curator = configuration.startCurator();
}
public void cleanup() {
