config: add path_config_source and watched_directory config (envoypro…

…xy#19974) For xDS over the file system, sometimes more control is required over what directory/file is watched for symbolic link swaps. Specifically, in order to deliver xDS over a Kubernetes ConfigMap, this extra configuration is required. Fixes envoyproxy#10979 Signed-off-by: Matt Klein <[email protected]>
hango-io · Feb 17, 2022 · 8670309 · 8670309
1 parent 0ed7da3
commit 8670309
Show file tree

Hide file tree

Showing 40 changed files with 314 additions and 110 deletions.
diff --git a/api/envoy/config/core/v3/config_source.proto b/api/envoy/config/core/v3/config_source.proto
@@ -2,6 +2,7 @@ syntax = "proto3";
 
 package envoy.config.core.v3;
 
+import "envoy/config/core/v3/base.proto";
 import "envoy/config/core/v3/grpc_service.proto";
 
 import "google/protobuf/duration.proto";
@@ -143,13 +144,49 @@ message RateLimitSettings {
   google.protobuf.DoubleValue fill_rate = 2 [(validate.rules).double = {gt: 0.0}];
 }
 
+// Local filesystem path configuration source.
+message PathConfigSource {
+  // Path on the filesystem to source and watch for configuration updates.
+  // When sourcing configuration for a :ref:`secret <envoy_v3_api_msg_extensions.transport_sockets.tls.v3.Secret>`,
+  // the certificate and key files are also watched for updates.
+  //
+  // .. note::
+  //
+  //  The path to the source must exist at config load time.
+  //
+  // .. note::
+  //
+  //   If `watched_directory` is *not* configured, Envoy will watch the file path for *moves.*
+  //   This is because in general only moves are atomic. The same method of swapping files as is
+  //   demonstrated in the :ref:`runtime documentation <config_runtime_symbolic_link_swap>` can be
+  //   used here also. If `watched_directory` is configured, no watch will be placed directly on
+  //   this path. Instead, the configured `watched_directory` will be used to trigger reloads of
+  //   this path. This is required in certain deployment scenarios. See below for more information.
+  string path = 1 [(validate.rules).string = {min_len: 1}];
+
+  // If configured, this directory will be watched for *moves.* When an entry in this directory is
+  // moved to, the `path` will be reloaded. This is required in certain deployment scenarios.
+  //
+  // Specifically, if trying to load an xDS resource using a
+  // `Kubernetes ConfigMap <https://kubernetes.io/docs/concepts/configuration/configmap/>`_, the
+  // following configuration might be used:
+  // 1. Store xds.yaml inside a ConfigMap.
+  // 2. Mount the ConfigMap to `/config_map/xds`
+  // 3. Configure path `/config_map/xds/xds.yaml`
+  // 4. Configure watched directory `/config_map/xds`
+  //
+  // The above configuration will ensure that Envoy watches the owning directory for moves which is
+  // required due to how Kubernetes manages ConfigMap symbolic links during atomic updates.
+  WatchedDirectory watched_directory = 2;
+}
+
 // Configuration for :ref:`listeners <config_listeners>`, :ref:`clusters
 // <config_cluster_manager>`, :ref:`routes
 // <envoy_v3_api_msg_config.route.v3.RouteConfiguration>`, :ref:`endpoints
 // <arch_overview_service_discovery>` etc. may either be sourced from the
 // filesystem or from an xDS API source. Filesystem configs are watched with
 // inotify for updates.
-// [#next-free-field: 8]
+// [#next-free-field: 9]
 message ConfigSource {
   option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.core.ConfigSource";
 
@@ -162,20 +199,11 @@ message ConfigSource {
   oneof config_source_specifier {
     option (validate.required) = true;
 
-    // Path on the filesystem to source and watch for configuration updates.
-    // When sourcing configuration for :ref:`secret <envoy_v3_api_msg_extensions.transport_sockets.tls.v3.Secret>`,
-    // the certificate and key files are also watched for updates.
-    //
-    // .. note::
-    //
-    //  The path to the source must exist at config load time.
-    //
-    // .. note::
-    //
-    //   Envoy will only watch the file path for *moves.* This is because in general only moves
-    //   are atomic. The same method of swapping files as is demonstrated in the
-    //   :ref:`runtime documentation <config_runtime_symbolic_link_swap>` can be used here also.
-    string path = 1;
+    // Deprecated in favor of `path_config_source`. Use that field instead.
+    string path = 1 [deprecated = true, (envoy.annotations.deprecated_at_minor_version) = "3.0"];
+
+    // Local filesystem path configuration source.
+    PathConfigSource path_config_source = 8;
 
     // API configuration source.
     ApiConfigSource api_config_source = 2;

diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst
@@ -53,6 +53,10 @@ Removed Config or Runtime
 New Features
 ------------
 * access_log: make consistent access_log format fields ``%(DOWN|DIRECT_DOWN|UP)STREAM_(LOCAL|REMOTE)_*%`` to provide all combinations of local & remote addresses for upstream & downstream connections.
+* config: added new file based xDS configuration via :ref:`path_config_source <envoy_v3_api_field_config.core.v3.ConfigSource.path_config_source>`.
+  :ref:`watched_directory <envoy_v3_api_field_config.core.v3.PathConfigSource.watched_directory>` can
+  be used to setup an independent watch for when to reload the file path, for example when using
+  Kubernetes ConfigMaps to deliver configuration. See the linked documentation for more information.
 * cors: add dynamic support for headers ``access-control-allow-methods`` and ``access-control-allow-headers`` in cors.
 * http: added random_value_specifier in :ref:`weighted_clusters <envoy_v3_api_field_config.route.v3.RouteAction.weighted_clusters>` to allow random value to be specified from configuration proto.
 * http: added support for :ref:`proxy_status_config <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.proxy_status_config>` for configuring `Proxy-Status <https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-proxy-status-08>`_ HTTP response header fields.
@@ -64,5 +68,7 @@ New Features
 Deprecated
 ----------
 
+* config: deprecated :ref:`path <envoy_v3_api_field_config.core.v3.ConfigSource.path>` in favor of
+  :ref:`path_config_source <envoy_v3_api_field_config.core.v3.ConfigSource.path_config_source>`
 * http: removing support for long-deprecated old style filter names, e.g. envoy.router, envoy.lua.
 * re2: removed undocumented histograms ``re2.program_size`` and ``re2.exceeded_warn_level``.
diff --git a/envoy/config/subscription_factory.h b/envoy/config/subscription_factory.h
@@ -13,6 +13,16 @@ class SubscriptionFactory {
 public:
   virtual ~SubscriptionFactory() = default;
 
+  /**
+   * @return true if a config source comes from the local filesystem.
+   */
+  static bool
+  isPathBasedConfigSource(envoy::config::core::v3::ConfigSource::ConfigSourceSpecifierCase type) {
+    return type == envoy::config::core::v3::ConfigSource::ConfigSourceSpecifierCase::kPath ||
+           type ==
+               envoy::config::core::v3::ConfigSource::ConfigSourceSpecifierCase::kPathConfigSource;
+  }
+
   /**
    * Subscription factory interface.
    *

diff --git a/envoy/router/router.h b/envoy/router/router.h
@@ -383,7 +383,7 @@ class RetryState {
   /**
    * Determine whether a request should be retried based on the response headers.
    * @param response_headers supplies the response headers.
-   * @param original_request supplies the orignal request headers.
+   * @param original_request supplies the original request headers.
    * @param callback supplies the callback that will be invoked when the retry should take place.
    *                 This is used to add timed backoff, etc. The callback will never be called
    *                 inline.
@@ -401,7 +401,7 @@ class RetryState {
    * the information about whether a response is "good" or not is useful, but a retry should
    * not be attempted for other reasons.
    * @param response_headers supplies the response headers.
-   * @param original_request supplies the orignal request headers.
+   * @param original_request supplies the original request headers.
    * @param retry_as_early_data output argument to tell the caller if a retry should be sent as
    *        early data if it is warranted.
    * @return RetryDecision if a retry would be warranted based on the retry policy and if it would

diff --git a/examples/dynamic-config-fs/envoy.yaml b/examples/dynamic-config-fs/envoy.yaml
@@ -4,9 +4,11 @@ node:
 
 dynamic_resources:
   cds_config:
-    path: /var/lib/envoy/cds.yaml
+    path_config_source:
+      path: /var/lib/envoy/cds.yaml
   lds_config:
-    path: /var/lib/envoy/lds.yaml
+    path_config_source:
+      path: /var/lib/envoy/lds.yaml
 
 admin:
   address:

diff --git a/source/common/config/BUILD b/source/common/config/BUILD
@@ -120,6 +120,7 @@ envoy_cc_library(
     hdrs = ["filesystem_subscription_impl.h"],
     deps = [
         ":decoded_resource_lib",
+        ":watched_directory_lib",
         "//envoy/config:subscription_interface",
         "//envoy/event:dispatcher_interface",
         "//envoy/filesystem:filesystem_interface",
@@ -129,6 +130,7 @@ envoy_cc_library(
         "//source/common/protobuf",
         "//source/common/protobuf:message_validator_lib",
         "//source/common/protobuf:utility_lib",
+        "@envoy_api//envoy/config/core/v3:pkg_cc_proto",
         "@envoy_api//envoy/service/discovery/v3:pkg_cc_proto",
     ],
 )

diff --git a/source/common/config/filesystem_subscription_impl.cc b/source/common/config/filesystem_subscription_impl.cc
@@ -12,18 +12,35 @@
 namespace Envoy {
 namespace Config {
 
+envoy::config::core::v3::PathConfigSource makePathConfigSource(const std::string& path) {
+  envoy::config::core::v3::PathConfigSource path_config_source;
+  path_config_source.set_path(path);
+  return path_config_source;
+}
+
 FilesystemSubscriptionImpl::FilesystemSubscriptionImpl(
-    Event::Dispatcher& dispatcher, absl::string_view path, SubscriptionCallbacks& callbacks,
-    OpaqueResourceDecoder& resource_decoder, SubscriptionStats stats,
-    ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api)
-    : path_(path), watcher_(dispatcher.createFilesystemWatcher()), callbacks_(callbacks),
-      resource_decoder_(resource_decoder), stats_(stats), api_(api),
-      validation_visitor_(validation_visitor) {
-  watcher_->addWatch(path_, Filesystem::Watcher::Events::MovedTo, [this](uint32_t) {
-    if (started_) {
-      refresh();
-    }
-  });
+    Event::Dispatcher& dispatcher,
+    const envoy::config::core::v3::PathConfigSource& path_config_source,
+    SubscriptionCallbacks& callbacks, OpaqueResourceDecoder& resource_decoder,
+    SubscriptionStats stats, ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api)
+    : path_(path_config_source.path()), callbacks_(callbacks), resource_decoder_(resource_decoder),
+      stats_(stats), api_(api), validation_visitor_(validation_visitor) {
+  if (!path_config_source.has_watched_directory()) {
+    file_watcher_ = dispatcher.createFilesystemWatcher();
+    file_watcher_->addWatch(path_, Filesystem::Watcher::Events::MovedTo, [this](uint32_t) {
+      if (started_) {
+        refresh();
+      }
+    });
+  } else {
+    directory_watcher_ =
+        std::make_unique<WatchedDirectory>(path_config_source.watched_directory(), dispatcher);
+    directory_watcher_->setCallback([this]() {
+      if (started_) {
+        refresh();
+      }
+    });
+  }
 }
 
 // Config::Subscription
@@ -88,10 +105,11 @@ void FilesystemSubscriptionImpl::refresh() {
 }
 
 FilesystemCollectionSubscriptionImpl::FilesystemCollectionSubscriptionImpl(
-    Event::Dispatcher& dispatcher, absl::string_view path, SubscriptionCallbacks& callbacks,
-    OpaqueResourceDecoder& resource_decoder, SubscriptionStats stats,
-    ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api)
-    : FilesystemSubscriptionImpl(dispatcher, path, callbacks, resource_decoder, stats,
+    Event::Dispatcher& dispatcher,
+    const envoy::config::core::v3::PathConfigSource& path_config_source,
+    SubscriptionCallbacks& callbacks, OpaqueResourceDecoder& resource_decoder,
+    SubscriptionStats stats, ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api)
+    : FilesystemSubscriptionImpl(dispatcher, path_config_source, callbacks, resource_decoder, stats,
                                  validation_visitor, api) {}
 
 std::string

diff --git a/source/common/config/filesystem_subscription_impl.h b/source/common/config/filesystem_subscription_impl.h
@@ -1,16 +1,20 @@
 #pragma once
 
 #include "envoy/api/api.h"
+#include "envoy/config/core/v3/config_source.pb.h"
 #include "envoy/config/subscription.h"
 #include "envoy/event/dispatcher.h"
 #include "envoy/filesystem/filesystem.h"
 #include "envoy/protobuf/message_validator.h"
 
 #include "source/common/common/logger.h"
+#include "source/common/config/watched_directory.h"
 
 namespace Envoy {
 namespace Config {
 
+envoy::config::core::v3::PathConfigSource makePathConfigSource(const std::string& path);
+
 /**
  * Filesystem inotify implementation of the API Subscription interface. This allows the API to be
  * consumed on filesystem changes to files containing the JSON canonical representation of
@@ -19,7 +23,8 @@ namespace Config {
 class FilesystemSubscriptionImpl : public Config::Subscription,
                                    protected Logger::Loggable<Logger::Id::config> {
 public:
-  FilesystemSubscriptionImpl(Event::Dispatcher& dispatcher, absl::string_view path,
+  FilesystemSubscriptionImpl(Event::Dispatcher& dispatcher,
+                             const envoy::config::core::v3::PathConfigSource& path_config_source,
                              SubscriptionCallbacks& callbacks,
                              OpaqueResourceDecoder& resource_decoder, SubscriptionStats stats,
                              ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api);
@@ -40,7 +45,8 @@ class FilesystemSubscriptionImpl : public Config::Subscription,
 
   bool started_{};
   const std::string path_;
-  std::unique_ptr<Filesystem::Watcher> watcher_;
+  std::unique_ptr<Filesystem::Watcher> file_watcher_;
+  WatchedDirectoryPtr directory_watcher_;
   SubscriptionCallbacks& callbacks_;
   OpaqueResourceDecoder& resource_decoder_;
   SubscriptionStats stats_;
@@ -52,12 +58,12 @@ class FilesystemSubscriptionImpl : public Config::Subscription,
 // non-inline collection resources.
 class FilesystemCollectionSubscriptionImpl : public FilesystemSubscriptionImpl {
 public:
-  FilesystemCollectionSubscriptionImpl(Event::Dispatcher& dispatcher, absl::string_view path,
-                                       SubscriptionCallbacks& callbacks,
-                                       OpaqueResourceDecoder& resource_decoder,
-                                       SubscriptionStats stats,
-                                       ProtobufMessage::ValidationVisitor& validation_visitor,
-                                       Api::Api& api);
+  FilesystemCollectionSubscriptionImpl(
+      Event::Dispatcher& dispatcher,
+      const envoy::config::core::v3::PathConfigSource& path_config_source,
+      SubscriptionCallbacks& callbacks, OpaqueResourceDecoder& resource_decoder,
+      SubscriptionStats stats, ProtobufMessage::ValidationVisitor& validation_visitor,
+      Api::Api& api);
 
   std::string refreshInternal(ProtobufTypes::MessagePtr* config_update) override;
 };

diff --git a/source/common/config/subscription_factory_impl.cc b/source/common/config/subscription_factory_impl.cc
@@ -36,7 +36,14 @@ SubscriptionPtr SubscriptionFactoryImpl::subscriptionFromConfigSource(
   case envoy::config::core::v3::ConfigSource::ConfigSourceSpecifierCase::kPath: {
     Utility::checkFilesystemSubscriptionBackingPath(config.path(), api_);
     return std::make_unique<Config::FilesystemSubscriptionImpl>(
-        dispatcher_, config.path(), callbacks, resource_decoder, stats, validation_visitor_, api_);
+        dispatcher_, makePathConfigSource(config.path()), callbacks, resource_decoder, stats,
+        validation_visitor_, api_);
+  }
+  case envoy::config::core::v3::ConfigSource::ConfigSourceSpecifierCase::kPathConfigSource: {
+    Utility::checkFilesystemSubscriptionBackingPath(config.path_config_source().path(), api_);
+    return std::make_unique<Config::FilesystemSubscriptionImpl>(
+        dispatcher_, config.path_config_source(), callbacks, resource_decoder, stats,
+        validation_visitor_, api_);
   }
   case envoy::config::core::v3::ConfigSource::ConfigSourceSpecifierCase::kApiConfigSource: {
     const envoy::config::core::v3::ApiConfigSource& api_config_source = config.api_config_source();
@@ -134,7 +141,8 @@ SubscriptionPtr SubscriptionFactoryImpl::collectionSubscriptionFromUrl(
     const std::string path = Http::Utility::localPathFromFilePath(collection_locator.id());
     Utility::checkFilesystemSubscriptionBackingPath(path, api_);
     return std::make_unique<Config::FilesystemCollectionSubscriptionImpl>(
-        dispatcher_, path, callbacks, resource_decoder, stats, validation_visitor_, api_);
+        dispatcher_, makePathConfigSource(path), callbacks, resource_decoder, stats,
+        validation_visitor_, api_);
   }
   case xds::core::v3::ResourceLocator::XDSTP: {
     if (resource_type != collection_locator.resource_type()) {

diff --git a/source/common/upstream/cluster_manager_impl.cc b/source/common/upstream/cluster_manager_impl.cc
@@ -315,8 +315,8 @@ ClusterManagerImpl::ClusterManagerImpl(
   auto is_primary_cluster = [](const envoy::config::cluster::v3::Cluster& cluster) -> bool {
     return cluster.type() != envoy::config::cluster::v3::Cluster::EDS ||
            (cluster.type() == envoy::config::cluster::v3::Cluster::EDS &&
-            cluster.eds_cluster_config().eds_config().config_source_specifier_case() ==
-                envoy::config::core::v3::ConfigSource::ConfigSourceSpecifierCase::kPath);
+            Config::SubscriptionFactory::isPathBasedConfigSource(
+                cluster.eds_cluster_config().eds_config().config_source_specifier_case()));
   };
   // Build book-keeping for which clusters are primary. This is useful when we
   // invoke loadCluster() below and it needs the complete set of primaries.
@@ -399,8 +399,8 @@ ClusterManagerImpl::ClusterManagerImpl(
   for (const auto& cluster : bootstrap.static_resources().clusters()) {
     // Now load all the secondary clusters.
     if (cluster.type() == envoy::config::cluster::v3::Cluster::EDS &&
-        cluster.eds_cluster_config().eds_config().config_source_specifier_case() !=
-            envoy::config::core::v3::ConfigSource::ConfigSourceSpecifierCase::kPath) {
+        !Config::SubscriptionFactory::isPathBasedConfigSource(
+            cluster.eds_cluster_config().eds_config().config_source_specifier_case())) {
       loadCluster(cluster, MessageUtil::hash(cluster), "", false, active_clusters_);
     }
   }

diff --git a/source/common/upstream/eds.cc b/source/common/upstream/eds.cc
@@ -28,8 +28,8 @@ EdsClusterImpl::EdsClusterImpl(
   Event::Dispatcher& dispatcher = factory_context.mainThreadDispatcher();
   assignment_timeout_ = dispatcher.createTimer([this]() -> void { onAssignmentTimeout(); });
   const auto& eds_config = cluster.eds_cluster_config().eds_config();
-  if (eds_config.config_source_specifier_case() ==
-      envoy::config::core::v3::ConfigSource::ConfigSourceSpecifierCase::kPath) {
+  if (Config::SubscriptionFactory::isPathBasedConfigSource(
+          eds_config.config_source_specifier_case())) {
     initialize_phase_ = InitializePhase::Primary;
   } else {
     initialize_phase_ = InitializePhase::Secondary;

diff --git a/test/common/config/BUILD b/test/common/config/BUILD
@@ -123,6 +123,7 @@ envoy_cc_test(
         "//test/mocks/event:event_mocks",
         "//test/mocks/filesystem:filesystem_mocks",
         "//test/test_common:logging_lib",
+        "@envoy_api//envoy/config/core/v3:pkg_cc_proto",
         "@envoy_api//envoy/config/endpoint/v3:pkg_cc_proto",
         "@envoy_api//envoy/config/listener/v3:pkg_cc_proto",
     ],