[redis proxy]: redis unsupported command message compatibility

Signed-off-by: duanhongyi <duanhongyi@doopai.com>

docs/root/intro/arch_overview/other_protocols/redis.rst

@@ -302,7 +302,7 @@ Envoy can also generate its own errors in response to the client.
   the connection."
   invalid request, "Command was rejected by the first stage of the command splitter due to
   datatype or length."
-  unsupported command, "The command was not recognized by Envoy and therefore cannot be serviced
+  ERR unknown command, "The command was not recognized by Envoy and therefore cannot be serviced
   because it cannot be hashed to a backend server."
   finished with n errors, "Fragmented commands which sum the response (e.g. DEL) will return the
   total number of errors received if any were received."

source/extensions/filters/network/common/redis/supported_commands.h

@@ -122,6 +122,14 @@ struct SupportedCommands {
   static bool isReadCommand(const std::string& command) {
     return !writeCommands().contains(command);
   }
+
+  static bool isSupportCommand(const std::string& command) {
+    return (simpleCommands().contains(command) || evalCommands().contains(command) ||
+            hashMultipleSumResultCommands().contains(command) ||
+            transactionCommands().contains(command) || auth() == command || echo() == command ||
+            mget() == command || mset() == command || keys() == command || ping() == command ||
+            time() == command || quit() == command || select() == command);
+  }
 };
 
 } // namespace Redis

source/extensions/filters/network/redis_proxy/command_splitter_impl.cc

@@ -745,6 +745,15 @@ SplitRequestPtr InstanceImpl::makeRequest(Common::Redis::RespValuePtr&& request,
   }
 
   std::string command_name = absl::AsciiStrToLower(request->asArray()[0].asString());
+  // Compatible with redis behavior, if there is an unsupported command, return immediately,
+  // this action must be performed before verifying auth, some redis clients rely on this behavior.
+  if (!Common::Redis::SupportedCommands::isSupportCommand(command_name)) {
+    stats_.unsupported_command_.inc();
+    callbacks.onResponse(Common::Redis::Utility::makeError(fmt::format(
+        "ERR unknown command '{}', with args beginning with: {}", request->asArray()[0].asString(),
+        request->asArray().size() > 1 ? request->asArray()[1].asString() : "")));
+    return nullptr;
+  }
 
   if (command_name == Common::Redis::SupportedCommands::auth()) {
     if (request->asArray().size() < 2) {
@@ -836,12 +845,7 @@ SplitRequestPtr InstanceImpl::makeRequest(Common::Redis::RespValuePtr&& request,
 
   // Get the handler for the downstream request
   auto handler = handler_lookup_table_.find(command_name.c_str());
-  if (handler == nullptr) {
-    stats_.unsupported_command_.inc();
-    callbacks.onResponse(Common::Redis::Utility::makeError(
-        fmt::format("unsupported command '{}'", request->asArray()[0].asString())));
-    return nullptr;
-  }
+  ASSERT(handler != nullptr);
 
   // If we are within a transaction, forward all requests to the transaction handler (i.e. handler
   // of "multi" command).

source/extensions/filters/network/redis_proxy/conn_pool_impl.cc

@@ -69,8 +69,6 @@ void InstanceImpl::init() {
   });
 }
 
-//
-//
 uint16_t InstanceImpl::shardSize() { return tls_->getTyped<ThreadLocalPool>().shardSize(); }
 
 // This method is always called from a InstanceSharedPtr we don't have to worry about tls_->getTyped

test/extensions/filters/network/redis_proxy/command_splitter_impl_test.cc

@@ -173,7 +173,7 @@ TEST_F(RedisCommandSplitterImplTest, InvalidRequestArrayNotStrings) {
 TEST_F(RedisCommandSplitterImplTest, UnsupportedCommand) {
   Common::Redis::RespValue response;
   response.type(Common::Redis::RespType::Error);
-  response.asString() = "unsupported command 'newcommand'";
+  response.asString() = "ERR unknown command 'newcommand', with args beginning with: hello";
   EXPECT_CALL(callbacks_, connectionAllowed()).WillOnce(Return(true));
   EXPECT_CALL(callbacks_, onResponse_(PointeesEq(&response)));
   Common::Redis::RespValuePtr request{new Common::Redis::RespValue()};