config: moving grpc subscriptions to extensions (envoyproxy#26742)

Risk Level: low Testing: n/a Docs Changes: inline Release Notes: n/a Signed-off-by: Alyssa Wilk <[email protected]>
pchaseh · May 2, 2023 · 522f055 · 522f055
1 parent 580f910
commit 522f055
Show file tree

Hide file tree

Showing 30 changed files with 500 additions and 221 deletions.
diff --git a/CODEOWNERS b/CODEOWNERS
@@ -325,6 +325,7 @@ extensions/filters/http/oauth2 @derekargueta @snowp
 /*/source/extensions/listener_managers/listener_manager @UNOWNED @UNOWNED
 /*/source/extensions/listener_managers/validation_listener_manager @UNOWNED @UNOWNED
 /*/source/extensions/config_subscription/ @UNOWNED @UNOWNED
+/*/source/extensions/config_subscription/grpc @adisuissa @UNOWNED
 
 # URL Pattern Match and Rewrite Library
 /*/extensions/path/uri_template_lib @alyssawilk @yanjunxiang-google

diff --git a/api/envoy/config/core/v3/config_source.proto b/api/envoy/config/core/v3/config_source.proto
@@ -281,15 +281,3 @@ message ExtensionConfigSource {
   // if they do not match any type URL in the set.
   repeated string type_urls = 4 [(validate.rules).repeated = {min_items: 1}];
 }
-
-// A placeholder proto for the registration for the REST subscription implementation.
-message RestSubscription {
-}
-
-// A placeholder proto for the registration for the filesystem subscription implementation.
-message FilesystemSubscription {
-}
-
-// A placeholder proto for the registration for the filesystem collection subscription implementation.
-message FilesystemCollectionSubscription {
-}
diff --git a/changelogs/current.yaml b/changelogs/current.yaml
@@ -6,6 +6,10 @@ behavior_changes:
   change: |
     Moved the subset and maglev LB code into extensions. If you use these load balancers and override
     extensions_build_config.bzl you will need to include them explicitly.
+- area: build
+  change: |
+    Moved xDS code extensions. If you use the xDS and override extensions_build_config.bzl you will
+    need to include the new config_subscriptions explicitly.
 
 minor_behavior_changes:
 # *Changes that may cause incompatibilities for some users, but should not for most*

diff --git a/docs/root/api-docs/xds_protocol.rst b/docs/root/api-docs/xds_protocol.rst
@@ -88,6 +88,9 @@ occurs.
 
 .. _xds_protocol_streaming_grpc_subscriptions:
 
+.. _extension_envoy.config_subscription.grpc:
+.. _extension_envoy.config_subscription.aggregated_grpc_collection:
+
 Streaming gRPC subscriptions
 ----------------------------
 
@@ -771,6 +774,9 @@ This feature is gated by the *xds.config.supports-resource-in-sotw* client featu
 
 Aggregated Discovery Service
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+.. _extension_envoy.config_subscription.ads:
+.. _extension_envoy.config_subscription.ads_collection:
+
 
 It's challenging to provide the above guarantees on sequencing to avoid
 traffic drop when management servers are distributed. ADS allow a single
@@ -792,6 +798,8 @@ An example minimal ``bootstrap.yaml`` fragment for ADS configuration is:
 .. literalinclude:: ../_include/ads.yaml
 
 .. _xds_protocol_delta:
+.. _extension_envoy.config_subscription.delta_grpc:
+.. _extension_envoy.config_subscription.aggregated_delta_grpc_collection:
 
 Incremental xDS
 ~~~~~~~~~~~~~~~

diff --git a/envoy/config/subscription_factory.h b/envoy/config/subscription_factory.h
@@ -11,7 +11,14 @@
 #include "xds/core/v3/resource_locator.pb.h"
 
 namespace Envoy {
+
+namespace Server {
+class Instance;
+} // namespace Server
+
 namespace Config {
+class XdsResourcesDelegate;
+class XdsConfigTracker;
 
 class SubscriptionFactory {
 public:
@@ -71,17 +78,30 @@ class SubscriptionFactory {
 };
 
 // A factory class that individual config subscriptions can subclass to be factory-created.
-// TODO(alyssawilk) rename once https://github.com/envoyproxy/envoy/pull/26468 lands
-class ConfigSubscriptionFactory : public Config::TypedFactory {
+class ConfigSubscriptionFactory : public Config::UntypedFactory {
 public:
+  struct SubscriptionData {
+    const LocalInfo::LocalInfo& local_info_;
+    Event::Dispatcher& dispatcher_;
+    Upstream::ClusterManager& cm_;
+    ProtobufMessage::ValidationVisitor& validation_visitor_;
+    Api::Api& api_;
+    const Server::Instance& server_;
+    OptRef<XdsResourcesDelegate> xds_resources_delegate_;
+    OptRef<XdsConfigTracker> xds_config_tracker_;
+
+    const envoy::config::core::v3::ConfigSource& config_;
+    absl::string_view type_url_;
+    Stats::Scope& scope_;
+    SubscriptionCallbacks& callbacks_;
+    OpaqueResourceDecoderSharedPtr resource_decoder_;
+    const SubscriptionOptions& options_;
+    OptRef<const xds::core::v3::ResourceLocator> collection_locator_;
+    SubscriptionStats stats_;
+  };
+
   std::string category() const override { return "envoy.config_subscription"; }
-  virtual SubscriptionPtr create(const LocalInfo::LocalInfo& local_info,
-                                 Upstream::ClusterManager& cm, Event::Dispatcher& dispatcher,
-                                 Api::Api& api, const envoy::config::core::v3::ConfigSource& config,
-                                 absl::string_view type_url, SubscriptionCallbacks& callbacks,
-                                 OpaqueResourceDecoderSharedPtr resource_decoder,
-                                 SubscriptionStats stats,
-                                 ProtobufMessage::ValidationVisitor& validation_visitor) PURE;
+  virtual SubscriptionPtr create(SubscriptionData& data) PURE;
 };
 
 } // namespace Config

diff --git a/mobile/test/common/integration/xds_integration_test.cc b/mobile/test/common/integration/xds_integration_test.cc
@@ -4,6 +4,8 @@
 #include "envoy/config/cluster/v3/cluster.pb.h"
 
 #include "source/common/grpc/google_grpc_creds_impl.h"
+#include "source/extensions/config_subscription/grpc/grpc_collection_subscription_factory.h"
+#include "source/extensions/config_subscription/grpc/grpc_subscription_factory.h"
 
 #include "test/common/grpc/grpc_client_integration.h"
 #include "test/common/integration/base_client_integration_test.h"
@@ -20,6 +22,13 @@ using ::testing::AssertionSuccess;
 
 XdsIntegrationTest::XdsIntegrationTest() : BaseClientIntegrationTest(ipVersion()) {
   Grpc::forceRegisterDefaultGoogleGrpcCredentialsFactory();
+  Config::forceRegisterAdsConfigSubscriptionFactory();
+  Config::forceRegisterGrpcConfigSubscriptionFactory();
+  Config::forceRegisterDeltaGrpcConfigSubscriptionFactory();
+  Config::forceRegisterDeltaGrpcCollectionConfigSubscriptionFactory();
+  Config::forceRegisterAggregatedGrpcCollectionConfigSubscriptionFactory();
+  Config::forceRegisterAdsCollectionConfigSubscriptionFactory();
+
   override_builder_config_ = false;
   expect_dns_ = false; // doesn't use DFP.
   create_xds_upstream_ = true;

diff --git a/source/common/config/BUILD b/source/common/config/BUILD
@@ -161,19 +161,6 @@ envoy_cc_library(
     ],
 )
 
-envoy_cc_library(
-    name = "grpc_subscription_lib",
-    srcs = ["grpc_subscription_impl.cc"],
-    hdrs = ["grpc_subscription_impl.h"],
-    deps = [
-        ":grpc_mux_lib",
-        ":xds_resource_lib",
-        "//envoy/config:subscription_interface",
-        "//envoy/event:dispatcher_interface",
-        "//envoy/grpc:async_client_interface",
-    ],
-)
-
 envoy_cc_library(
     name = "new_grpc_mux_lib",
     srcs = ["new_grpc_mux_impl.cc"],
@@ -280,7 +267,6 @@ envoy_cc_library(
     hdrs = ["subscription_factory_impl.h"],
     deps = [
         ":custom_config_validators_lib",
-        ":grpc_subscription_lib",
         ":new_grpc_mux_lib",
         ":type_to_endpoint_lib",
         ":utility_lib",