Add support to explicitly add priority for the handshakers using enums (#30971)

Currently the order of the handshakers is controlled by a single bool(at_start). This doesn't allow for more complex use cases where the handshaker has to be done before tcp connect handshaker for example.

By explicitly adding enums that specify the priority, we allow for a cleaner abstraction for registering handshakers.

src/core/lib/security/transport/security_handshaker.cc

@@ -617,6 +617,9 @@ class ClientSecurityHandshakerFactory : public HandshakerFactory {
                                           handshake_mgr);
     }
   }
+  HandshakerPriority Priority() override {
+    return HandshakerPriority::kSecurityHandshakers;
+  }
   ~ClientSecurityHandshakerFactory() override = default;
 };
 
@@ -631,6 +634,9 @@ class ServerSecurityHandshakerFactory : public HandshakerFactory {
                                           handshake_mgr);
     }
   }
+  HandshakerPriority Priority() override {
+    return HandshakerPriority::kSecurityHandshakers;
+  }
   ~ServerSecurityHandshakerFactory() override = default;
 };
 
@@ -654,11 +660,9 @@ RefCountedPtr<Handshaker> SecurityHandshakerCreate(
 
 void SecurityRegisterHandshakerFactories(CoreConfiguration::Builder* builder) {
   builder->handshaker_registry()->RegisterHandshakerFactory(
-      false /* at_start */, HANDSHAKER_CLIENT,
-      std::make_unique<ClientSecurityHandshakerFactory>());
+      HANDSHAKER_CLIENT, std::make_unique<ClientSecurityHandshakerFactory>());
   builder->handshaker_registry()->RegisterHandshakerFactory(
-      false /* at_start */, HANDSHAKER_SERVER,
-      std::make_unique<ServerSecurityHandshakerFactory>());
+      HANDSHAKER_SERVER, std::make_unique<ServerSecurityHandshakerFactory>());
 }
 
 }  // namespace grpc_core

src/core/lib/transport/handshaker_factory.h

@@ -37,9 +37,35 @@ class HandshakeManager;
 
 class HandshakerFactory {
  public:
+  // Enum representing the priority of the handshakers.
+  // The order of the handshakers is decided by the priority.
+  // For example kPreTCPConnect handshakers are called before kTCPConnect and so
+  // on.
+  enum class HandshakerPriority : int {
+    // Handshakers that should be called before a TCP connect. Applicable mainly
+    // for Client handshakers.
+    kPreTCPConnectHandshakers,
+    // Handshakers responsible for the actual TCP connect establishment.
+    // Applicable mainly for Client handshakers.
+    kTCPConnectHandshakers,
+    // Handshakers responsible for the actual HTTP connect established.
+    // Applicable
+    // mainly for Client handshakers.
+    kHTTPConnectHandshakers,
+    // Handshakers that should be called before security handshakes but after
+    // connect establishment. Applicable mainly for Server handshakers
+    // currently.
+    kReadAheadSecurityHandshakers,
+    // Handshakers that are responsible for post connect security handshakes.
+    // Applicable for both Client and Server handshakers.
+    kSecurityHandshakers,
+  };
+
   virtual void AddHandshakers(const ChannelArgs& args,
                               grpc_pollset_set* interested_parties,
                               HandshakeManager* handshake_mgr) = 0;
+  // Return the priority associated with the handshaker.
+  virtual HandshakerPriority Priority() = 0;
   virtual ~HandshakerFactory() = default;
 };
 

src/core/lib/transport/handshaker_registry.cc

@@ -28,10 +28,16 @@
 namespace grpc_core {
 
 void HandshakerRegistry::Builder::RegisterHandshakerFactory(
-    bool at_start, HandshakerType handshaker_type,
+    HandshakerType handshaker_type,
     std::unique_ptr<HandshakerFactory> factory) {
   auto& vec = factories_[handshaker_type];
-  auto where = at_start ? vec.begin() : vec.end();
+  auto where = vec.empty() ? vec.begin() : vec.end();
+  for (auto iter = vec.begin(); iter != vec.end(); ++iter) {
+    if (factory->Priority() < iter->get()->Priority()) {
+      where = iter;
+      break;
+    }
+  }
   vec.insert(where, std::move(factory));
 }
 

src/core/lib/transport/handshaker_registry.h

@@ -41,10 +41,9 @@ class HandshakerRegistry {
   class Builder {
    public:
     /// Registers a new handshaker factory.  Takes ownership.
-    /// If \a at_start is true, the new handshaker will be at the beginning of
-    /// the list.  Otherwise, it will be added to the end.
-    void RegisterHandshakerFactory(bool at_start,
-                                   HandshakerType handshaker_type,
+    /// The priority of the handshaker will be used to order the handshakers
+    /// in the list.
+    void RegisterHandshakerFactory(HandshakerType handshaker_type,
                                    std::unique_ptr<HandshakerFactory> factory);
 
     HandshakerRegistry Build();

src/core/lib/transport/http_connect_handshaker.cc

@@ -385,6 +385,9 @@ class HttpConnectHandshakerFactory : public HandshakerFactory {
                       HandshakeManager* handshake_mgr) override {
     handshake_mgr->Add(MakeRefCounted<HttpConnectHandshaker>());
   }
+  HandshakerPriority Priority() override {
+    return HandshakerPriority::kHTTPConnectHandshakers;
+  }
   ~HttpConnectHandshakerFactory() override = default;
 };
 
@@ -392,8 +395,7 @@ class HttpConnectHandshakerFactory : public HandshakerFactory {
 
 void RegisterHttpConnectHandshaker(CoreConfiguration::Builder* builder) {
   builder->handshaker_registry()->RegisterHandshakerFactory(
-      true /* at_start */, HANDSHAKER_CLIENT,
-      std::make_unique<HttpConnectHandshakerFactory>());
+      HANDSHAKER_CLIENT, std::make_unique<HttpConnectHandshakerFactory>());
 }
 
 }  // namespace grpc_core

src/core/lib/transport/tcp_connect_handshaker.cc

@@ -232,6 +232,9 @@ class TCPConnectHandshakerFactory : public HandshakerFactory {
     handshake_mgr->Add(
         MakeRefCounted<TCPConnectHandshaker>(interested_parties));
   }
+  HandshakerPriority Priority() override {
+    return HandshakerPriority::kTCPConnectHandshakers;
+  }
   ~TCPConnectHandshakerFactory() override = default;
 };
 
@@ -239,8 +242,7 @@ class TCPConnectHandshakerFactory : public HandshakerFactory {
 
 void RegisterTCPConnectHandshaker(CoreConfiguration::Builder* builder) {
   builder->handshaker_registry()->RegisterHandshakerFactory(
-      true /* at_start */, HANDSHAKER_CLIENT,
-      std::make_unique<TCPConnectHandshakerFactory>());
+      HANDSHAKER_CLIENT, std::make_unique<TCPConnectHandshakerFactory>());
 }
 
 }  // namespace grpc_core

test/core/handshake/readahead_handshaker_server_ssl.cc

@@ -67,6 +67,9 @@ class ReadAheadHandshakerFactory : public HandshakerFactory {
                       HandshakeManager* handshake_mgr) override {
     handshake_mgr->Add(MakeRefCounted<ReadAheadHandshaker>());
   }
+  HandshakerPriority Priority() override {
+    return HandshakerPriority::kReadAheadSecurityHandshakers;
+  }
   ~ReadAheadHandshakerFactory() override = default;
 };
 
@@ -77,7 +80,7 @@ TEST(HandshakeServerWithReadaheadHandshakerTest, MainTest) {
       [](grpc_core::CoreConfiguration::Builder* builder) {
         BuildCoreConfiguration(builder);
         builder->handshaker_registry()->RegisterHandshakerFactory(
-            true /* at_start */, grpc_core::HANDSHAKER_SERVER,
+            grpc_core::HANDSHAKER_SERVER,
             std::make_unique<grpc_core::ReadAheadHandshakerFactory>());
       });