Added support for configurable socket address in filesystem for hot restart (envoyproxy#12712)

Fixes envoyproxy#2908

Signed-off-by: Manish Kumar <[email protected]>

Author: Manish

api/envoy/admin/v3/server_info.proto

@@ -54,7 +54,7 @@ message ServerInfo {
   CommandLineOptions command_line_options = 6;
 }
 
-// [#next-free-field: 35]
+// [#next-free-field: 37]
 message CommandLineOptions {
   option (udpa.annotations.versioning).previous_message_type =
       "envoy.admin.v2alpha.CommandLineOptions";
@@ -179,4 +179,10 @@ message CommandLineOptions {
 
   // See :option:`--enable-fine-grain-logging` for details.
   bool enable_fine_grain_logging = 34;
+
+  // See :option:`--socket-path` for details.
+  string socket_path = 35;
+
+  // See :option:`--socket-mode` for details.
+  uint32 socket_mode = 36;
 }

api/envoy/admin/v4alpha/server_info.proto

@@ -192,6 +192,21 @@ following are the command line options that Envoy supports.
   more. The administration interface usage is similar. Please see `Administration interface 
   <https://www.envoyproxy.io/docs/envoy/latest/operations/admin>`_ for more detail.
 
+.. option:: --socket-path <path string>
+
+  *(optional)* The output file path to the socket address for :ref:`hot restart <arch_overview_hot_restart>`.
+  Default to "@envoy_domain_socket" which will be created in the abstract namespace. Suffix _{role}_{id}
+  is appended to provide name. All envoy processes wanting to participate in hot-restart together must
+  use the same value for this option.
+
+  **NOTE**: The path started with "@" will be created in the abstract namespace.
+
+.. option:: --socket-mode <string>
+
+  *(optional)* The socket file permission for :ref:`hot restart <arch_overview_hot_restart>`.
+  This must be a valid octal file permission, such as 644. The default value is 600.
+  This flag may not be used when :option:`--socket-path` is start with "@" or not set.
+
 .. option:: --hot-restart-version
 
   *(optional)* Outputs an opaque hot restart compatibility version for the binary. This can be

docs/root/operations/cli.rst

@@ -79,6 +79,7 @@ New Features
 * grpc-json: support specifying `response_body` field in for `google.api.HttpBody` message.
 * hds: added :ref:`cluster_endpoints_health <envoy_v3_api_field_service.health.v3.EndpointHealthResponse.cluster_endpoints_health>` to HDS responses, keeping endpoints in the same groupings as they were configured in the HDS specifier by cluster and locality instead of as a flat list.
 * hds: added :ref:`transport_socket_matches <envoy_v3_api_field_service.health.v3.ClusterHealthCheck.transport_socket_matches>` to HDS cluster health check specifier, so the existing match filter :ref:`transport_socket_match_criteria <envoy_v3_api_field_config.core.v3.HealthCheck.transport_socket_match_criteria>` in the repeated field :ref:`health_checks <envoy_v3_api_field_service.health.v3.ClusterHealthCheck.health_checks>` has context to match against. This unblocks support for health checks over HTTPS and HTTP/2.
+* hot restart: added :option:`--socket-path` and :option:`--socket-mode` to configure UDS path in the filesystem and set permission to it.
 * http: added support for :ref:`%DOWNSTREAM_PEER_FINGERPRINT_1% <config_http_conn_man_headers_custom_request_headers>` as custom header.
 * http: added :ref:`allow_chunked_length <envoy_v3_api_field_config.core.v3.Http1ProtocolOptions.allow_chunked_length>` configuration option for HTTP/1 codec to allow processing requests/responses with both Content-Length and Transfer-Encoding: chunked headers. If such message is served and option is enabled - per RFC Content-Length is ignored and removed.
 * http: introduced new HTTP/1 and HTTP/2 codec implementations that will remove the use of exceptions for control flow due to high risk factors and instead use error statuses. The old behavior is used by default, but the new codecs can be enabled for testing by setting the runtime feature `envoy.reloadable_features.new_codec_behavior` to true. The new codecs will be in development for one month, and then enabled by default while the old codecs are deprecated.

docs/root/version_history/current.rst

@@ -254,6 +254,16 @@ class Options {
    * @return CommandLineOptionsPtr the protobuf representation of the options.
    */
   virtual CommandLineOptionsPtr toCommandLineOptions() const PURE;
+
+  /**
+   * @return the path of socket file.
+   */
+  virtual const std::string& socketPath() const PURE;
+
+  /**
+   * @return the mode of socket file.
+   */
+  virtual mode_t socketMode() const PURE;
 };
 
 } // namespace Server

generated_api_shadow/envoy/admin/v3/server_info.proto

@@ -120,7 +120,8 @@ void MainCommonBase::configureHotRestarter(Random::RandomGenerator& random_gener
         base_id = static_cast<uint32_t>(random_generator.random()) & 0x0FFFFFFF;
 
         try {
-          restarter = std::make_unique<Server::HotRestartImpl>(base_id, 0);
+          restarter = std::make_unique<Server::HotRestartImpl>(base_id, 0, options_.socketPath(),
+                                                               options_.socketMode());
         } catch (Server::HotRestartDomainSocketInUseException& ex) {
           // No luck, try again.
           ENVOY_LOG_MISC(debug, "dynamic base id: {}", ex.what());
@@ -133,7 +134,8 @@ void MainCommonBase::configureHotRestarter(Random::RandomGenerator& random_gener
 
       restarter_.swap(restarter);
     } else {
-      restarter_ = std::make_unique<Server::HotRestartImpl>(base_id, options_.restartEpoch());
+      restarter_ = std::make_unique<Server::HotRestartImpl>(
+          base_id, options_.restartEpoch(), options_.socketPath(), options_.socketMode());
     }
 
     // Write the base-id to the requested path whether we selected it

generated_api_shadow/envoy/admin/v4alpha/server_info.proto

@@ -95,10 +95,11 @@ void initializeMutex(pthread_mutex_t& mutex) {
 // performed the multiplication in OptionsImpl which produced incorrect server info output.
 // TODO(zuercher): ideally, the base_id would be separated from the restart_epoch in
 // the socket names to entirely prevent collisions between consecutive base ids.
-HotRestartImpl::HotRestartImpl(uint32_t base_id, uint32_t restart_epoch)
+HotRestartImpl::HotRestartImpl(uint32_t base_id, uint32_t restart_epoch,
+                               const std::string& socket_path, mode_t socket_mode)
     : base_id_(base_id), scaled_base_id_(base_id * 10),
-      as_child_(HotRestartingChild(scaled_base_id_, restart_epoch)),
-      as_parent_(HotRestartingParent(scaled_base_id_, restart_epoch)),
+      as_child_(HotRestartingChild(scaled_base_id_, restart_epoch, socket_path, socket_mode)),
+      as_parent_(HotRestartingParent(scaled_base_id_, restart_epoch, socket_path, socket_mode)),
       shmem_(attachSharedMemory(scaled_base_id_, restart_epoch)), log_lock_(shmem_->log_lock_),
       access_log_lock_(shmem_->access_log_lock_) {
   // If our parent ever goes away just terminate us so that we don't have to rely on ops/launching

include/envoy/server/options.h

@@ -98,7 +98,8 @@ class ProcessSharedMutex : public Thread::BasicLockable {
  */
 class HotRestartImpl : public HotRestart {
 public:
-  HotRestartImpl(uint32_t base_id, uint32_t restart_epoch);
+  HotRestartImpl(uint32_t base_id, uint32_t restart_epoch, const std::string& socket_path,
+                 mode_t socket_mode);
 
   // Server::HotRestart
   void drainParentListeners() override;

source/exe/main_common.cc

@@ -2,6 +2,7 @@
 
 #include "common/api/os_sys_calls_impl.h"
 #include "common/common/utility.h"
+#include "common/network/address_impl.h"
 #include "common/stats/utility.h"
 
 namespace Envoy {
@@ -24,28 +25,30 @@ void HotRestartingBase::initDomainSocketAddress(sockaddr_un* address) {
   address->sun_family = AF_UNIX;
 }
 
-sockaddr_un HotRestartingBase::createDomainSocketAddress(uint64_t id, const std::string& role) {
+sockaddr_un HotRestartingBase::createDomainSocketAddress(uint64_t id, const std::string& role,
+                                                         const std::string& socket_path,
+                                                         mode_t socket_mode) {
   // Right now we only allow a maximum of 3 concurrent envoy processes to be running. When the third
   // starts up it will kill the oldest parent.
   static constexpr uint64_t MaxConcurrentProcesses = 3;
   id = id % MaxConcurrentProcesses;
-
-  // This creates an anonymous domain socket name (where the first byte of the name of \0).
   sockaddr_un address;
   initDomainSocketAddress(&address);
-  StringUtil::strlcpy(&address.sun_path[1],
-                      fmt::format("envoy_domain_socket_{}_{}", role, base_id_ + id).c_str(),
-                      sizeof(address.sun_path) - 1);
-  address.sun_path[0] = 0;
+  Network::Address::PipeInstance addr(fmt::format(socket_path + "_{}_{}", role, base_id_ + id),
+                                      socket_mode, nullptr);
+  memcpy(&address, addr.sockAddr(), addr.sockAddrLen());
+  fchmod(my_domain_socket_, socket_mode);
   return address;
 }
 
-void HotRestartingBase::bindDomainSocket(uint64_t id, const std::string& role) {
+void HotRestartingBase::bindDomainSocket(uint64_t id, const std::string& role,
+                                         const std::string& socket_path, mode_t socket_mode) {
   Api::OsSysCalls& os_sys_calls = Api::OsSysCallsSingleton::get();
   // This actually creates the socket and binds it. We use the socket in datagram mode so we can
   // easily read single messages.
   my_domain_socket_ = socket(AF_UNIX, SOCK_DGRAM | SOCK_NONBLOCK, 0);
-  sockaddr_un address = createDomainSocketAddress(id, role);
+  sockaddr_un address = createDomainSocketAddress(id, role, socket_path, socket_mode);
+  unlink(address.sun_path);
   Api::SysCallIntResult result =
       os_sys_calls.bind(my_domain_socket_, reinterpret_cast<sockaddr*>(&address), sizeof(address));
   if (result.rc_ != 0) {

source/server/hot_restart_impl.cc

@@ -1,7 +1,9 @@
 #pragma once
 
 #include <fcntl.h>
+#include <sys/stat.h>
 #include <sys/un.h>
+#include <unistd.h>
 
 #include <array>
 #include <atomic>
@@ -28,8 +30,10 @@ class HotRestartingBase {
   ~HotRestartingBase();
 
   void initDomainSocketAddress(sockaddr_un* address);
-  sockaddr_un createDomainSocketAddress(uint64_t id, const std::string& role);
-  void bindDomainSocket(uint64_t id, const std::string& role);
+  sockaddr_un createDomainSocketAddress(uint64_t id, const std::string& role,
+                                        const std::string& socket_path, mode_t socket_mode);
+  void bindDomainSocket(uint64_t id, const std::string& role, const std::string& socket_path,
+                        mode_t socket_mode);
   int myDomainSocket() const { return my_domain_socket_; }
 
   // Protocol description:

source/server/hot_restart_impl.h

@@ -7,13 +7,15 @@ namespace Server {
 
 using HotRestartMessage = envoy::HotRestartMessage;
 
-HotRestartingChild::HotRestartingChild(int base_id, int restart_epoch)
+HotRestartingChild::HotRestartingChild(int base_id, int restart_epoch,
+                                       const std::string& socket_path, mode_t socket_mode)
     : HotRestartingBase(base_id), restart_epoch_(restart_epoch) {
   initDomainSocketAddress(&parent_address_);
   if (restart_epoch_ != 0) {
-    parent_address_ = createDomainSocketAddress(restart_epoch_ + -1, "parent");
+    parent_address_ =
+        createDomainSocketAddress(restart_epoch_ + -1, "parent", socket_path, socket_mode);
   }
-  bindDomainSocket(restart_epoch_, "child");
+  bindDomainSocket(restart_epoch_, "child", socket_path, socket_mode);
 }
 
 int HotRestartingChild::duplicateParentListenSocket(const std::string& address) {

source/server/hot_restarting_base.cc

@@ -12,7 +12,8 @@ namespace Server {
  */
 class HotRestartingChild : HotRestartingBase, Logger::Loggable<Logger::Id::main> {
 public:
-  HotRestartingChild(int base_id, int restart_epoch);
+  HotRestartingChild(int base_id, int restart_epoch, const std::string& socket_path,
+                     mode_t socket_mode);
 
   int duplicateParentListenSocket(const std::string& address);
   std::unique_ptr<envoy::HotRestartMessage> getParentStats();

source/server/hot_restarting_base.h

@@ -15,10 +15,11 @@ namespace Server {
 
 using HotRestartMessage = envoy::HotRestartMessage;
 
-HotRestartingParent::HotRestartingParent(int base_id, int restart_epoch)
+HotRestartingParent::HotRestartingParent(int base_id, int restart_epoch,
+                                         const std::string& socket_path, mode_t socket_mode)
     : HotRestartingBase(base_id), restart_epoch_(restart_epoch) {
-  child_address_ = createDomainSocketAddress(restart_epoch_ + 1, "child");
-  bindDomainSocket(restart_epoch_, "parent");
+  child_address_ = createDomainSocketAddress(restart_epoch_ + 1, "child", socket_path, socket_mode);
+  bindDomainSocket(restart_epoch_, "parent", socket_path, socket_mode);
 }
 
 void HotRestartingParent::initialize(Event::Dispatcher& dispatcher, Server::Instance& server) {

source/server/hot_restarting_child.cc

@@ -14,7 +14,8 @@ namespace Server {
  */
 class HotRestartingParent : HotRestartingBase, Logger::Loggable<Logger::Id::main> {
 public:
-  HotRestartingParent(int base_id, int restart_epoch);
+  HotRestartingParent(int base_id, int restart_epoch, const std::string& socket_path,
+                      mode_t socket_mode);
   void initialize(Event::Dispatcher& dispatcher, Server::Instance& server);
   void shutdown();
 

source/server/hot_restarting_child.h

@@ -160,6 +160,12 @@ OptionsImpl::OptionsImpl(std::vector<std::string> args,
                                                   "Comma-separated list of extensions to disable",
                                                   false, "", "string", cmd);
 
+  TCLAP::ValueArg<std::string> socket_path("", "socket-path", "Path to hot restart socket file",
+                                           false, "@envoy_domain_socket", "string", cmd);
+
+  TCLAP::ValueArg<std::string> socket_mode("", "socket-mode", "Socket file permission", false,
+                                           "600", "string", cmd);
+
   cmd.setExceptionHandling(false);
   try {
     cmd.parse(args);
@@ -263,6 +269,18 @@ OptionsImpl::OptionsImpl(std::vector<std::string> args,
   file_flush_interval_msec_ = std::chrono::milliseconds(file_flush_interval_msec.getValue());
   drain_time_ = std::chrono::seconds(drain_time_s.getValue());
   parent_shutdown_time_ = std::chrono::seconds(parent_shutdown_time_s.getValue());
+  socket_path_ = socket_path.getValue();
+
+  if (socket_path_.at(0) == '@') {
+    socket_mode_ = 0;
+  } else {
+    uint64_t socket_mode_helper;
+    if (!StringUtil::atoull(socket_mode.getValue().c_str(), socket_mode_helper, 8)) {
+      throw MalformedArgvException(
+          fmt::format("error: invalid socket-mode '{}'", socket_mode.getValue()));
+    }
+    socket_mode_ = socket_mode_helper;
+  }
 
   if (drain_strategy.getValue() == "immediate") {
     drain_strategy_ = Server::DrainStrategy::Immediate;
@@ -393,6 +411,8 @@ Server::CommandLineOptionsPtr OptionsImpl::toCommandLineOptions() const {
   for (const auto& e : disabledExtensions()) {
     command_line_options->add_disabled_extensions(e);
   }
+  command_line_options->set_socket_path(socketPath());
+  command_line_options->set_socket_mode(socketMode());
   return command_line_options;
 }
 
@@ -406,7 +426,8 @@ OptionsImpl::OptionsImpl(const std::string& service_cluster, const std::string&
       service_zone_(service_zone), file_flush_interval_msec_(10000), drain_time_(600),
       parent_shutdown_time_(900), drain_strategy_(Server::DrainStrategy::Gradual),
       mode_(Server::Mode::Serve), hot_restart_disabled_(false), signal_handling_enabled_(true),
-      mutex_tracing_enabled_(false), cpuset_threads_(false), fake_symbol_table_enabled_(false) {}
+      mutex_tracing_enabled_(false), cpuset_threads_(false), fake_symbol_table_enabled_(false),
+      socket_path_("@envoy_domain_socket"), socket_mode_(0) {}
 
 void OptionsImpl::disableExtensions(const std::vector<std::string>& names) {
   for (const auto& name : names) {

source/server/hot_restarting_parent.cc

@@ -104,6 +104,10 @@ class OptionsImpl : public Server::Options, protected Logger::Loggable<Logger::I
     fake_symbol_table_enabled_ = fake_symbol_table_enabled;
   }
 
+  void setSocketPath(const std::string& socket_path) { socket_path_ = socket_path; }
+
+  void setSocketMode(mode_t socket_mode) { socket_mode_ = socket_mode; }
+
   // Server::Options
   uint64_t baseId() const override { return base_id_; }
   bool useDynamicBaseId() const override { return use_dynamic_base_id_; }
@@ -154,6 +158,8 @@ class OptionsImpl : public Server::Options, protected Logger::Loggable<Logger::I
     return disabled_extensions_;
   }
   uint32_t count() const;
+  const std::string& socketPath() const override { return socket_path_; }
+  mode_t socketMode() const override { return socket_mode_; }
 
   /**
    * disableExtensions parses the given set of extension names of
@@ -206,6 +212,8 @@ class OptionsImpl : public Server::Options, protected Logger::Loggable<Logger::I
   // Initialization added here to avoid integration_admin_test failure caused by uninitialized
   // enable_fine_grain_logging_.
   bool enable_fine_grain_logging_ = false;
+  std::string socket_path_;
+  mode_t socket_mode_;
 };
 
 /**