From e2af4fa93b8ce1d19d34bc0056f7bede617b8da9 Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Mon, 18 Nov 2024 23:32:51 -0800 Subject: [PATCH 1/4] Update tests Signed-off-by: Andrew Carbonetto --- glide-core/redis-rs/redis/src/types.rs | 6 + glide-core/src/client/mod.rs | 67 ++++++--- glide-core/src/protobuf/command_request.proto | 2 +- glide-core/src/socket_listener.rs | 2 +- java/client/build.gradle | 2 +- .../src/main/java/glide/api/BaseClient.java | 26 ++-- .../java/glide/managers/CommandManager.java | 7 +- .../test/java/glide/SharedClientTests.java | 3 +- .../glide/cluster/ClusterClientTests.java | 15 +- .../standalone/StandaloneClientTests.java | 132 ++++++++++++++---- 10 files changed, 195 insertions(+), 67 deletions(-) diff --git a/glide-core/redis-rs/redis/src/types.rs b/glide-core/redis-rs/redis/src/types.rs index 4b6cdbb150..2d8035d697 100644 --- a/glide-core/redis-rs/redis/src/types.rs +++ b/glide-core/redis-rs/redis/src/types.rs @@ -153,6 +153,10 @@ pub enum ErrorKind { /// Not all slots are covered by the cluster NotAllSlotsCovered, + + /// Used when an error occurs on when user perform wrong usage of management operation. + /// E.g. not allowed configuration change. + UserOperationError, } #[derive(PartialEq, Debug)] @@ -900,6 +904,7 @@ impl RedisError { ErrorKind::RESP3NotSupported => "resp3 is not supported by server", ErrorKind::ParseError => "parse error", ErrorKind::NotAllSlotsCovered => "not all slots are covered", + ErrorKind::UserOperationError => "Wrong usage of management operation", } } @@ -1095,6 +1100,7 @@ impl RedisError { ErrorKind::NotAllSlotsCovered => RetryMethod::NoRetry, ErrorKind::FatalReceiveError => RetryMethod::Reconnect, ErrorKind::FatalSendError => RetryMethod::ReconnectAndRetry, + ErrorKind::UserOperationError => RetryMethod::NoRetry, } } } diff --git a/glide-core/src/client/mod.rs b/glide-core/src/client/mod.rs index ffbdc60d4e..33258399ab 100644 --- a/glide-core/src/client/mod.rs +++ b/glide-core/src/client/mod.rs @@ -479,28 +479,63 @@ impl Client { /// Update the password used to authenticate with the servers. /// If None is passed, the password will be removed. - /// If `re_auth` is true, the new password will be used to re-authenticate with all of the nodes. + /// If `immediate_auth` is true, the password will be used to authenticate with the servers immediately using the `AUTH` command. + /// The default behavior is to update the password without authenticating immediately. + /// If the password is empty or None, and `immediate_auth` is true, the password will be updated and an error will be returned. pub async fn update_connection_password( &mut self, password: Option, - re_auth: bool, + immediate_auth: bool, ) -> RedisResult { - if re_auth { - let routing = RoutingInfo::MultiNode(( - MultipleNodeRoutingInfo::AllNodes, - Some(ResponsePolicy::AllSucceeded), - )); - let mut cmd = redis::cmd("AUTH"); - cmd.arg(&password); - self.send_command(&cmd, Some(routing)).await?; + let timeout = self.request_timeout; + // The password update operation is wrapped in a timeout to prevent it from blocking indefinitely. + // If the operation times out, an error is returned. + // Since the password update operation is not a command that go through the regular command pipeline, + // it is not have the regular timeout handling, as such we need to handle it separately. + match tokio::time::timeout(timeout, async { + match self.internal_client { + ClientWrapper::Standalone(ref mut client) => { + client.update_connection_password(password.clone()).await + } + ClientWrapper::Cluster { ref mut client } => { + client.update_connection_password(password.clone()).await + } + } + }) + .await + { + Ok(result) => { + if immediate_auth { + self.send_immediate_auth(password).await + } else { + result + } + } + Err(_elapsed) => Err(RedisError::from(( + ErrorKind::IoError, + "Password update operation timed out, please check the connection", + ))), } + } - match self.internal_client { - ClientWrapper::Standalone(ref mut client) => { - client.update_connection_password(password).await - } - ClientWrapper::Cluster { ref mut client } => { - client.update_connection_password(password).await + async fn send_immediate_auth(&mut self, password: Option) -> RedisResult { + match &password { + Some(pw) if pw.is_empty() => Err(RedisError::from(( + ErrorKind::UserOperationError, + "Empty password provided for authentication", + ))), + None => Err(RedisError::from(( + ErrorKind::UserOperationError, + "No password provided for authentication", + ))), + Some(password) => { + let routing = RoutingInfo::MultiNode(( + MultipleNodeRoutingInfo::AllNodes, + Some(ResponsePolicy::AllSucceeded), + )); + let mut cmd = redis::cmd("AUTH"); + cmd.arg(password); + self.send_command(&cmd, Some(routing)).await } } } diff --git a/glide-core/src/protobuf/command_request.proto b/glide-core/src/protobuf/command_request.proto index e50cdc8b3c..30b33362af 100644 --- a/glide-core/src/protobuf/command_request.proto +++ b/glide-core/src/protobuf/command_request.proto @@ -510,7 +510,7 @@ message ClusterScan { message UpdateConnectionPassword { optional string password = 1; - bool re_auth = 2; + bool immediate_auth = 2; } message CommandRequest { diff --git a/glide-core/src/socket_listener.rs b/glide-core/src/socket_listener.rs index b7f967e0bd..b9db4e6d99 100644 --- a/glide-core/src/socket_listener.rs +++ b/glide-core/src/socket_listener.rs @@ -529,7 +529,7 @@ fn handle_request(request: CommandRequest, mut client: Client, writer: Rc handleLcsIdxResponse(Map response) * that the internal reconnection mechanism can handle reconnection seamlessly, preventing the * loss of in-flight commands. * + * @param immediateAuth A boolean flag. If true, the client will + * authenticate immediately with the new password against all connections, Using AUTH + * command.
+ * If password supplied is an empty string, the client will not perform auth and a warning + * will be returned.
+ * The default is `false`. * @apiNote This method updates the client's internal password configuration and does not perform * password rotation on the server side. * @param password A new password to set. - * @param mode Password update mode, see {@link PasswordUpdateMode}. * @return "OK". * @example * 
{@code
@@ -799,9 +803,9 @@ protected Map handleLcsIdxResponse(Map response)
      * }
*/ public CompletableFuture updateConnectionPassword( - @NonNull String password, @NonNull PasswordUpdateMode mode) { + @NonNull String password, boolean immediateAuth) { return commandManager.submitPasswordUpdate( - Optional.of(password), mode, this::handleStringResponse); + Optional.of(password), immediateAuth, this::handleStringResponse); } /** @@ -815,16 +819,22 @@ public CompletableFuture updateConnectionPassword( * * @apiNote This method updates the client's internal password configuration and does not perform * password rotation on the server side. - * @param mode Password update mode, see {@link PasswordUpdateMode}. + * @param immediateAuth A boolean flag. If true, the client will + * authenticate immediately with the new password against all connections, Using AUTH + * command.
+ * If password supplied is an empty string, the client will not perform auth and a warning + * will be returned.
+ * The default is `false`. * @return "OK". * @example * 
{@code
-     * String response = client.resetConnectionPassword(RE_AUTHENTICATE).get();
+     * String response = client.resetConnectionPassword(true).get();
      * assert response.equals("OK");
      * }
*/ - public CompletableFuture updateConnectionPassword(@NonNull PasswordUpdateMode mode) { - return commandManager.submitPasswordUpdate(Optional.empty(), mode, this::handleStringResponse); + public CompletableFuture updateConnectionPassword(boolean immediateAuth) { + return commandManager.submitPasswordUpdate( + Optional.empty(), immediateAuth, this::handleStringResponse); } @Override diff --git a/java/client/src/main/java/glide/managers/CommandManager.java b/java/client/src/main/java/glide/managers/CommandManager.java index b1422a0eee..d069c6bd72 100644 --- a/java/client/src/main/java/glide/managers/CommandManager.java +++ b/java/client/src/main/java/glide/managers/CommandManager.java @@ -17,7 +17,6 @@ import glide.api.models.GlideString; import glide.api.models.Script; import glide.api.models.Transaction; -import glide.api.models.commands.PasswordUpdateMode; import glide.api.models.commands.scan.ClusterScanCursor; import glide.api.models.commands.scan.ScanOptions; import glide.api.models.configuration.RequestRoutingConfiguration.ByAddressRoute; @@ -224,16 +223,16 @@ public CompletableFuture submitClusterScan( * Submit a password update request to GLIDE core. * * @param password A new password to set or empty value to remove the password. - * @param mode Password update mode. + * @param immediateAuth immediately perform auth. * @param responseHandler A response handler. * @return A request promise. * @param Type of the response. */ public CompletableFuture submitPasswordUpdate( Optional password, - PasswordUpdateMode mode, + boolean immediateAuth, GlideExceptionCheckedFunction responseHandler) { - var builder = UpdateConnectionPassword.newBuilder().setReAuth(mode.getValue()); + var builder = UpdateConnectionPassword.newBuilder().setImmediateAuth(immediateAuth); password.ifPresent(builder::setPassword); var command = CommandRequest.newBuilder().setUpdateConnectionPassword(builder.build()); diff --git a/java/integTest/src/test/java/glide/SharedClientTests.java b/java/integTest/src/test/java/glide/SharedClientTests.java index 0ed48a6077..26a4144e96 100644 --- a/java/integTest/src/test/java/glide/SharedClientTests.java +++ b/java/integTest/src/test/java/glide/SharedClientTests.java @@ -27,11 +27,12 @@ import net.bytebuddy.utility.RandomString; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Timeout; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; -// @Timeout(35) // seconds +@Timeout(35) // seconds public class SharedClientTests { private static GlideClient standaloneClient = null; diff --git a/java/integTest/src/test/java/glide/cluster/ClusterClientTests.java b/java/integTest/src/test/java/glide/cluster/ClusterClientTests.java index e907bee796..68bc0377ab 100644 --- a/java/integTest/src/test/java/glide/cluster/ClusterClientTests.java +++ b/java/integTest/src/test/java/glide/cluster/ClusterClientTests.java @@ -13,9 +13,9 @@ import static org.junit.jupiter.api.Assumptions.assumeTrue; import glide.api.GlideClusterClient; -import glide.api.models.commands.PasswordUpdateMode; import glide.api.models.configuration.ServerCredentials; import glide.api.models.exceptions.ClosingException; +import glide.api.models.exceptions.ConnectionException; import glide.api.models.exceptions.RequestException; import java.util.Map; import java.util.UUID; @@ -24,7 +24,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.EnumSource; +import org.junit.jupiter.params.provider.ValueSource; @Timeout(10) // seconds public class ClusterClientTests { @@ -168,8 +168,8 @@ public void closed_client_throws_ExecutionException_with_ClosingException_as_cau @SneakyThrows @ParameterizedTest - @EnumSource(PasswordUpdateMode.class) - public void password_update(PasswordUpdateMode mode) { + @ValueSource(booleans = {true, false}) + public void password_update(boolean immediateAuth) { GlideClusterClient client = GlideClusterClient.createClient(commonClusterClientConfig().build()).get(); @@ -185,16 +185,15 @@ public void password_update(PasswordUpdateMode mode) { // set the password and forcefully drop connection for the second client assertEquals("OK", client.configSet(Map.of("requirepass", pwd)).get()); - if (mode == PasswordUpdateMode.RE_AUTHENTICATE) - testClient.customCommand(new String[] {"RESET"}, ALL_NODES).get(); + if (immediateAuth) testClient.customCommand(new String[] {"RESET"}, ALL_NODES).get(); else client.customCommand(new String[] {"CLIENT", "KILL", "TYPE", "NORMAL"}, ALL_NODES).get(); // client should reconnect, but will receive NOAUTH error var exception = assertThrows(ExecutionException.class, () -> testClient.get(key).get()); - assertInstanceOf(RequestException.class, exception.getCause()); + assertInstanceOf(ConnectionException.class, exception.getCause()); assertTrue(exception.getMessage().toLowerCase().contains("noauth")); - assertEquals("OK", testClient.updateConnectionPassword(pwd, mode).get()); + assertEquals("OK", testClient.updateConnectionPassword(pwd, immediateAuth).get()); // after setting new password we should be able to work with the server assertEquals("meow meow", testClient.get(key).get()); diff --git a/java/integTest/src/test/java/glide/standalone/StandaloneClientTests.java b/java/integTest/src/test/java/glide/standalone/StandaloneClientTests.java index 6e6c23263f..85b5cdd889 100644 --- a/java/integTest/src/test/java/glide/standalone/StandaloneClientTests.java +++ b/java/integTest/src/test/java/glide/standalone/StandaloneClientTests.java @@ -7,12 +7,12 @@ import static glide.api.BaseClient.OK; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; import glide.api.GlideClient; -import glide.api.models.commands.PasswordUpdateMode; import glide.api.models.configuration.ServerCredentials; import glide.api.models.exceptions.ClosingException; import glide.api.models.exceptions.RequestException; @@ -20,10 +20,11 @@ import java.util.UUID; import java.util.concurrent.ExecutionException; import lombok.SneakyThrows; +import org.apache.commons.lang3.RandomStringUtils; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.EnumSource; +import org.junit.jupiter.params.provider.ValueSource; @Timeout(10) // seconds public class StandaloneClientTests { @@ -167,42 +168,119 @@ public void closed_client_throws_ExecutionException_with_ClosingException_as_cau } @SneakyThrows - @ParameterizedTest - @EnumSource(PasswordUpdateMode.class) - public void password_update(PasswordUpdateMode mode) { - GlideClient client = GlideClient.createClient(commonClientConfig().build()).get(); - var key = UUID.randomUUID().toString(); + @Test + public void test_update_connection_password_auth_non_valid_pass() { + // Test Client fails on call to updateConnectionPassword with invalid parameters + try (var testClient = GlideClient.createClient(commonClientConfig().build()).get()) { + var emptyPasswordException = + assertThrows( + ExecutionException.class, () -> testClient.updateConnectionPassword("", true).get()); + assertInstanceOf(RequestException.class, emptyPasswordException.getCause()); + + var noPasswordException = + assertThrows( + ExecutionException.class, () -> testClient.updateConnectionPassword(true).get()); + assertInstanceOf(RequestException.class, noPasswordException.getCause()); + } + } + + @SneakyThrows + @Test + public void test_update_connection_password_no_server_auth() { var pwd = UUID.randomUUID().toString(); - client.set(key, "meow meow").get(); try (var testClient = GlideClient.createClient(commonClientConfig().build()).get()) { + // validate that we can use the client + assertNotNull(testClient.info().get()); - // validate that we can get the value - assertEquals("meow meow", testClient.get(key).get()); + // Test that immediate re-authentication fails when no server password is set. + var exception = + assertThrows( + ExecutionException.class, () -> testClient.updateConnectionPassword(pwd, true).get()); + assertInstanceOf(RequestException.class, exception.getCause()); + } + } - // set the password and forcefully drop connection for the second client - assertEquals("OK", client.configSet(Map.of("requirepass", pwd)).get()); - client.customCommand(new String[] {"CLIENT", "KILL", "TYPE", "NORMAL"}).get(); + @SneakyThrows + @Test + public void test_update_connection_password_long() { + var pwd = RandomStringUtils.randomAlphabetic(1000); - // client should reconnect, but will receive NOAUTH error - var exception = assertThrows(ExecutionException.class, () -> testClient.get(key).get()); - assertInstanceOf(RequestException.class, exception.getCause()); - assertTrue(exception.getMessage().toLowerCase().contains("noauth")); + try (var testClient = GlideClient.createClient(commonClientConfig().build()).get()) { + // validate that we can use the client + assertNotNull(testClient.info().get()); - assertEquals("OK", testClient.updateConnectionPassword(pwd, mode).get()); + // Test replacing connection password with a long password string. + assertEquals(OK, testClient.updateConnectionPassword(pwd, false).get()); + } + } - // after setting new password we should be able to work with the server - assertEquals("meow meow", testClient.get(key).get()); + @Timeout(50) + @SneakyThrows + @Test + public void test_replace_password_immediateAuth_wrong_password() { + var pwd = UUID.randomUUID().toString(); + var notThePwd = UUID.randomUUID().toString(); + + GlideClient adminClient = GlideClient.createClient(commonClientConfig().build()).get(); + try (var testClient = GlideClient.createClient(commonClientConfig().build()).get()) { + // validate that we can use the client + assertNotNull(testClient.info().get()); - // unset the password and drop connection again - assertEquals("OK", client.configSet(Map.of("requirepass", "")).get()); - client.customCommand(new String[] {"CLIENT", "KILL", "TYPE", "NORMAL"}).get(); + // set the password to something else + adminClient.configSet(Map.of("requirepass", notThePwd)).get(); - // client should reconnect, but since no auth configured, able to get a value - assertEquals("meow meow", testClient.get(key).get()); + // Test that re-authentication fails when using wrong password. + var exception = + assertThrows( + ExecutionException.class, () -> testClient.updateConnectionPassword(pwd, true).get()); + assertInstanceOf(RequestException.class, exception.getCause()); + + // But using something else password returns OK + assertEquals(OK, testClient.updateConnectionPassword(notThePwd, true).get()); + } finally { + adminClient.configSet(Map.of("requirepass", "")).get(); + adminClient.close(); + } + } + + @SneakyThrows + @ParameterizedTest + @ValueSource(booleans = {true, false}) + public void test_update_connection_password_connection_lost_before_password_update( + boolean immediateAuth) { + GlideClient adminClient = GlideClient.createClient(commonClientConfig().build()).get(); + var pwd = UUID.randomUUID().toString(); + + try (var testClient = GlideClient.createClient(commonClientConfig().build()).get()) { + // validate that we can use the client + assertNotNull(testClient.info().get()); + + // set the password and forcefully drop connection for the testClient + assertEquals("OK", adminClient.configSet(Map.of("requirepass", pwd)).get()); + adminClient.customCommand(new String[] {"CLIENT", "KILL", "TYPE", "NORMAL"}).get(); + + /* + * Some explanation for the curious mind: + * Our library is abstracting a connection or connections, with a lot of mechanism around it, making it behave like what we call a "client". + * When using standalone mode, the client is a single connection, so on disconnection the first thing it planned to do is to reconnect. + * + * There's no reason to get other commands and to take care of them since to serve commands we need to be connected. + * + * Hence, the client will try to reconnect and will not listen try to take care of new tasks, but will let them wait in line, + * so the update connection password will not be able to reach the connection and will return an error. + * For future versions, standalone will be considered as a different animal then it is now, since standalone is not necessarily one node. + * It can be replicated and have a lot of nodes, and to be what we like to call "one shard cluster". + * So, in the future, we will have many existing connection and request can be managed also when one connection is locked, + */ + var exception = + assertThrows( + ExecutionException.class, + () -> testClient.updateConnectionPassword(pwd, immediateAuth).get()); + assertInstanceOf(RequestException.class, exception.getCause()); } finally { - client.configSet(Map.of("requirepass", "")).get(); - client.close(); + adminClient.configSet(Map.of("requirepass", "")).get(); + adminClient.close(); } } } From 5120e0156acfee20fbec3b982bbbb5d8ca5944c5 Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Tue, 19 Nov 2024 13:36:18 -0800 Subject: [PATCH 2/4] Add cluster-mode tests Signed-off-by: Andrew Carbonetto --- .../glide/cluster/ClusterClientTests.java | 131 +++++++++++++----- 1 file changed, 99 insertions(+), 32 deletions(-) diff --git a/java/integTest/src/test/java/glide/cluster/ClusterClientTests.java b/java/integTest/src/test/java/glide/cluster/ClusterClientTests.java index 68bc0377ab..1c635bbc64 100644 --- a/java/integTest/src/test/java/glide/cluster/ClusterClientTests.java +++ b/java/integTest/src/test/java/glide/cluster/ClusterClientTests.java @@ -2,16 +2,19 @@ package glide.cluster; import static glide.TestConfiguration.SERVER_VERSION; +import static glide.TestUtilities.commonClientConfig; import static glide.TestUtilities.commonClusterClientConfig; import static glide.TestUtilities.getRandomString; import static glide.api.BaseClient.OK; import static glide.api.models.configuration.RequestRoutingConfiguration.SimpleMultiNodeRoute.ALL_NODES; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; +import glide.api.GlideClient; import glide.api.GlideClusterClient; import glide.api.models.configuration.ServerCredentials; import glide.api.models.exceptions.ClosingException; @@ -20,13 +23,16 @@ import java.util.Map; import java.util.UUID; import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; import lombok.SneakyThrows; +import org.apache.commons.lang3.RandomStringUtils; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; -@Timeout(10) // seconds +@Timeout(30) // seconds public class ClusterClientTests { @SneakyThrows @@ -167,49 +173,110 @@ public void closed_client_throws_ExecutionException_with_ClosingException_as_cau } @SneakyThrows - @ParameterizedTest - @ValueSource(booleans = {true, false}) - public void password_update(boolean immediateAuth) { - GlideClusterClient client = - GlideClusterClient.createClient(commonClusterClientConfig().build()).get(); + @Test + public void test_update_connection_password() { + GlideClusterClient adminClient = GlideClusterClient.createClient(commonClusterClientConfig().build()).get(); + String pwd = UUID.randomUUID().toString(); + + try (GlideClusterClient testClient = GlideClusterClient.createClient(commonClusterClientConfig().build()).get()) { + // validate that we can use the client + assertNotNull(testClient.info().get()); + + // Update password without re-authentication + assertEquals(OK, testClient.updateConnectionPassword(pwd, false).get()); + + // Verify client still works with old auth + assertNotNull(testClient.info().get()); + + // Update server password + // Kill all other clients to force reconnection + assertEquals("OK", adminClient.configSet(Map.of("requirepass", pwd)).get()); + adminClient.customCommand(new String[] {"CLIENT", "KILL", "TYPE", "NORMAL"}).get(); + + Thread.sleep(1000); + + // Verify client auto-reconnects with new password + assertNotNull(testClient.info().get()); + } finally { + adminClient.configSet(Map.of("requirepass", "")).get(); + adminClient.close(); + } + } + + @SneakyThrows + @Test + public void test_update_connection_password_auth_non_valid_pass() { + // Test Client fails on call to updateConnectionPassword with invalid parameters + try (GlideClusterClient testClient = GlideClusterClient.createClient(commonClusterClientConfig().build()).get()) { + var emptyPasswordException = + assertThrows( + ExecutionException.class, () -> testClient.updateConnectionPassword("", true).get()); + assertInstanceOf(RequestException.class, emptyPasswordException.getCause()); + + var noPasswordException = + assertThrows( + ExecutionException.class, () -> testClient.updateConnectionPassword(true).get()); + assertInstanceOf(RequestException.class, noPasswordException.getCause()); + } + } - var key = UUID.randomUUID().toString(); + @SneakyThrows + @Test + public void test_update_connection_password_no_server_auth() { var pwd = UUID.randomUUID().toString(); - client.set(key, "meow meow").get(); - try (var testClient = - GlideClusterClient.createClient(commonClusterClientConfig().build()).get()) { + try (GlideClusterClient testClient = GlideClusterClient.createClient(commonClusterClientConfig().build()).get()) { + // validate that we can use the client + assertNotNull(testClient.info().get()); - // validate that we can get the value - assertEquals("meow meow", testClient.get(key).get()); + // Test that immediate re-authentication fails when no server password is set. + var exception = + assertThrows( + ExecutionException.class, () -> testClient.updateConnectionPassword(pwd, true).get()); + assertInstanceOf(RequestException.class, exception.getCause()); + } + } - // set the password and forcefully drop connection for the second client - assertEquals("OK", client.configSet(Map.of("requirepass", pwd)).get()); - if (immediateAuth) testClient.customCommand(new String[] {"RESET"}, ALL_NODES).get(); - else client.customCommand(new String[] {"CLIENT", "KILL", "TYPE", "NORMAL"}, ALL_NODES).get(); + @SneakyThrows + @Test + public void test_update_connection_password_long() { + var pwd = RandomStringUtils.randomAlphabetic(1000); + + try (GlideClusterClient testClient = GlideClusterClient.createClient(commonClusterClientConfig().build()).get()) { + // validate that we can use the client + assertNotNull(testClient.info().get()); - // client should reconnect, but will receive NOAUTH error - var exception = assertThrows(ExecutionException.class, () -> testClient.get(key).get()); - assertInstanceOf(ConnectionException.class, exception.getCause()); - assertTrue(exception.getMessage().toLowerCase().contains("noauth")); + // Test replacing connection password with a long password string. + assertEquals(OK, testClient.updateConnectionPassword(pwd, false).get()); + } + } - assertEquals("OK", testClient.updateConnectionPassword(pwd, immediateAuth).get()); + @Timeout(50) + @SneakyThrows + @Test + public void test_replace_password_immediateAuth_wrong_password() { + var pwd = UUID.randomUUID().toString(); + var notThePwd = UUID.randomUUID().toString(); - // after setting new password we should be able to work with the server - assertEquals("meow meow", testClient.get(key).get()); + GlideClusterClient adminClient = GlideClusterClient.createClient(commonClusterClientConfig().build()).get(); + try (GlideClusterClient testClient = GlideClusterClient.createClient(commonClusterClientConfig().build()).get()) { + // validate that we can use the client + assertNotNull(testClient.info().get()); - // unset the password and drop connection again - assertEquals("OK", client.configSet(Map.of("requirepass", "")).get()); + // set the password to something else + adminClient.configSet(Map.of("requirepass", notThePwd)).get(); - client.customCommand(new String[] {"CLIENT", "KILL", "TYPE", "NORMAL"}, ALL_NODES).get(); + // Test that re-authentication fails when using wrong password. + var exception = + assertThrows( + ExecutionException.class, () -> testClient.updateConnectionPassword(pwd, true).get()); + assertInstanceOf(RequestException.class, exception.getCause()); - // client should reconnect, but since no auth configured, able to get a value - assertEquals("meow meow", testClient.get(key).get()); - } catch (Exception e) { - e.printStackTrace(); + // But using something else password returns OK + assertEquals(OK, testClient.updateConnectionPassword(notThePwd, true).get()); } finally { - client.configSet(Map.of("requirepass", "")).get(); - client.close(); + adminClient.configSet(Map.of("requirepass", "")).get(); + adminClient.close(); } } } From dbfa0697ed741e591f24f530d920a5beb4c053de Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Tue, 19 Nov 2024 13:36:52 -0800 Subject: [PATCH 3/4] Spotless Signed-off-by: Andrew Carbonetto --- .../glide/cluster/ClusterClientTests.java | 45 +++++++++---------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/java/integTest/src/test/java/glide/cluster/ClusterClientTests.java b/java/integTest/src/test/java/glide/cluster/ClusterClientTests.java index 1c635bbc64..9c0663331f 100644 --- a/java/integTest/src/test/java/glide/cluster/ClusterClientTests.java +++ b/java/integTest/src/test/java/glide/cluster/ClusterClientTests.java @@ -2,11 +2,9 @@ package glide.cluster; import static glide.TestConfiguration.SERVER_VERSION; -import static glide.TestUtilities.commonClientConfig; import static glide.TestUtilities.commonClusterClientConfig; import static glide.TestUtilities.getRandomString; import static glide.api.BaseClient.OK; -import static glide.api.models.configuration.RequestRoutingConfiguration.SimpleMultiNodeRoute.ALL_NODES; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -14,23 +12,17 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; -import glide.api.GlideClient; import glide.api.GlideClusterClient; import glide.api.models.configuration.ServerCredentials; import glide.api.models.exceptions.ClosingException; -import glide.api.models.exceptions.ConnectionException; import glide.api.models.exceptions.RequestException; import java.util.Map; import java.util.UUID; import java.util.concurrent.ExecutionException; -import java.util.concurrent.TimeUnit; import lombok.SneakyThrows; import org.apache.commons.lang3.RandomStringUtils; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.ValueSource; @Timeout(30) // seconds public class ClusterClientTests { @@ -175,10 +167,12 @@ public void closed_client_throws_ExecutionException_with_ClosingException_as_cau @SneakyThrows @Test public void test_update_connection_password() { - GlideClusterClient adminClient = GlideClusterClient.createClient(commonClusterClientConfig().build()).get(); + GlideClusterClient adminClient = + GlideClusterClient.createClient(commonClusterClientConfig().build()).get(); String pwd = UUID.randomUUID().toString(); - try (GlideClusterClient testClient = GlideClusterClient.createClient(commonClusterClientConfig().build()).get()) { + try (GlideClusterClient testClient = + GlideClusterClient.createClient(commonClusterClientConfig().build()).get()) { // validate that we can use the client assertNotNull(testClient.info().get()); @@ -207,15 +201,16 @@ public void test_update_connection_password() { @Test public void test_update_connection_password_auth_non_valid_pass() { // Test Client fails on call to updateConnectionPassword with invalid parameters - try (GlideClusterClient testClient = GlideClusterClient.createClient(commonClusterClientConfig().build()).get()) { + try (GlideClusterClient testClient = + GlideClusterClient.createClient(commonClusterClientConfig().build()).get()) { var emptyPasswordException = - assertThrows( - ExecutionException.class, () -> testClient.updateConnectionPassword("", true).get()); + assertThrows( + ExecutionException.class, () -> testClient.updateConnectionPassword("", true).get()); assertInstanceOf(RequestException.class, emptyPasswordException.getCause()); var noPasswordException = - assertThrows( - ExecutionException.class, () -> testClient.updateConnectionPassword(true).get()); + assertThrows( + ExecutionException.class, () -> testClient.updateConnectionPassword(true).get()); assertInstanceOf(RequestException.class, noPasswordException.getCause()); } } @@ -225,14 +220,15 @@ public void test_update_connection_password_auth_non_valid_pass() { public void test_update_connection_password_no_server_auth() { var pwd = UUID.randomUUID().toString(); - try (GlideClusterClient testClient = GlideClusterClient.createClient(commonClusterClientConfig().build()).get()) { + try (GlideClusterClient testClient = + GlideClusterClient.createClient(commonClusterClientConfig().build()).get()) { // validate that we can use the client assertNotNull(testClient.info().get()); // Test that immediate re-authentication fails when no server password is set. var exception = - assertThrows( - ExecutionException.class, () -> testClient.updateConnectionPassword(pwd, true).get()); + assertThrows( + ExecutionException.class, () -> testClient.updateConnectionPassword(pwd, true).get()); assertInstanceOf(RequestException.class, exception.getCause()); } } @@ -242,7 +238,8 @@ public void test_update_connection_password_no_server_auth() { public void test_update_connection_password_long() { var pwd = RandomStringUtils.randomAlphabetic(1000); - try (GlideClusterClient testClient = GlideClusterClient.createClient(commonClusterClientConfig().build()).get()) { + try (GlideClusterClient testClient = + GlideClusterClient.createClient(commonClusterClientConfig().build()).get()) { // validate that we can use the client assertNotNull(testClient.info().get()); @@ -258,8 +255,10 @@ public void test_replace_password_immediateAuth_wrong_password() { var pwd = UUID.randomUUID().toString(); var notThePwd = UUID.randomUUID().toString(); - GlideClusterClient adminClient = GlideClusterClient.createClient(commonClusterClientConfig().build()).get(); - try (GlideClusterClient testClient = GlideClusterClient.createClient(commonClusterClientConfig().build()).get()) { + GlideClusterClient adminClient = + GlideClusterClient.createClient(commonClusterClientConfig().build()).get(); + try (GlideClusterClient testClient = + GlideClusterClient.createClient(commonClusterClientConfig().build()).get()) { // validate that we can use the client assertNotNull(testClient.info().get()); @@ -268,8 +267,8 @@ public void test_replace_password_immediateAuth_wrong_password() { // Test that re-authentication fails when using wrong password. var exception = - assertThrows( - ExecutionException.class, () -> testClient.updateConnectionPassword(pwd, true).get()); + assertThrows( + ExecutionException.class, () -> testClient.updateConnectionPassword(pwd, true).get()); assertInstanceOf(RequestException.class, exception.getCause()); // But using something else password returns OK From 2a924a894819eafbef9d2dabf1aa65cfc36c9a97 Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Tue, 19 Nov 2024 13:43:05 -0800 Subject: [PATCH 4/4] Fixes for self-review Signed-off-by: Andrew Carbonetto --- java/client/build.gradle | 2 +- .../src/test/java/glide/cluster/ClusterClientTests.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/java/client/build.gradle b/java/client/build.gradle index 9a30713c56..364b09ca1e 100644 --- a/java/client/build.gradle +++ b/java/client/build.gradle @@ -14,7 +14,7 @@ repositories { } dependencies { - implementation group: 'com.google.protobuf', name: 'protobuf-java', version: '4.28.2' + implementation group: 'com.google.protobuf', name: 'protobuf-java', version: '4.27.1' implementation group: 'org.apache.commons', name: 'commons-lang3', version: '3.13.0' implementation group: 'io.netty', name: 'netty-handler', version: '4.1.100.Final' diff --git a/java/integTest/src/test/java/glide/cluster/ClusterClientTests.java b/java/integTest/src/test/java/glide/cluster/ClusterClientTests.java index 9c0663331f..380cd1a2c0 100644 --- a/java/integTest/src/test/java/glide/cluster/ClusterClientTests.java +++ b/java/integTest/src/test/java/glide/cluster/ClusterClientTests.java @@ -24,7 +24,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; -@Timeout(30) // seconds +@Timeout(10) // seconds public class ClusterClientTests { @SneakyThrows