Coalesce directory management to Envoy::TestEnvironment

include/envoy/filesystem/filesystem.h

@@ -102,6 +102,13 @@ class Instance {
    */
   virtual std::string fileReadToEnd(const std::string& path) PURE;
 
+  /**
+   * @path full file path on input which is truncated to the path
+   * @name the resulting file name component from the input path
+   */
+  virtual std::pair<absl::string_view, absl::string_view>
+  splitPathFromFilename(absl::string_view path) PURE;
+
   /**
    * Determine if the path is on a list of paths Envoy will refuse to access. This
    * is a basic sanity check for users, blacklisting some clearly bad paths. Paths
@@ -130,10 +137,6 @@ struct DirectoryEntry {
   }
 };
 
-/**
- * Abstraction for listing a directory.
- * TODO(sesmith177): replace with std::filesystem::directory_iterator once we move to C++17
- */
 class DirectoryIteratorImpl;
 class DirectoryIterator {
 public:

include/envoy/filesystem/watcher.h

@@ -8,6 +8,8 @@
 #include "envoy/common/platform.h"
 #include "envoy/common/pure.h"
 
+#include "absl/strings/string_view.h"
+
 namespace Envoy {
 namespace Filesystem {
 
@@ -31,7 +33,7 @@ class Watcher {
    * @param events supplies the events to watch.
    * @param cb supplies the callback to invoke when a change occurs.
    */
-  virtual void addWatch(const std::string& path, uint32_t events, OnChangedCb cb) PURE;
+  virtual void addWatch(absl::string_view path, uint32_t events, OnChangedCb cb) PURE;
 };
 
 using WatcherPtr = std::unique_ptr<Watcher>;

source/common/event/dispatcher_impl.cc

@@ -133,7 +133,7 @@ FileEventPtr DispatcherImpl::createFileEvent(int fd, FileReadyCb cb, FileTrigger
 
 Filesystem::WatcherPtr DispatcherImpl::createFilesystemWatcher() {
   ASSERT(isThreadSafe());
-  return Filesystem::WatcherPtr{new Filesystem::WatcherImpl(*this)};
+  return Filesystem::WatcherPtr{new Filesystem::WatcherImpl(*this, api_)};
 }
 
 Network::ListenerPtr DispatcherImpl::createListener(Network::SocketSharedPtr&& socket,

source/common/filesystem/BUILD

@@ -79,6 +79,9 @@ envoy_cc_library(
         "//bazel:apple": [
             "kqueue/watcher_impl.cc",
         ],
+        "//bazel:windows_x86_64": [
+            "win32/watcher_impl.cc",
+        ],
         "//conditions:default": [
             "inotify/watcher_impl.cc",
         ],
@@ -87,6 +90,9 @@ envoy_cc_library(
         "//bazel:apple": [
             "kqueue/watcher_impl.h",
         ],
+        "//bazel:windows_x86_64": [
+            "win32/watcher_impl.h",
+        ],
         "//conditions:default": [
             "inotify/watcher_impl.h",
         ],
@@ -96,13 +102,21 @@ envoy_cc_library(
     ],
     strip_include_prefix = select({
         "//bazel:apple": "kqueue",
+        "//bazel:windows_x86_64": "win32",
         "//conditions:default": "inotify",
     }),
     deps = [
+        "//include/envoy/api:api_interface",
         "//include/envoy/event:dispatcher_interface",
         "//source/common/common:assert_lib",
         "//source/common/common:linked_object",
         "//source/common/common:minimal_logger_lib",
         "//source/common/common:utility_lib",
-    ],
+    ] + select({
+        "//bazel:windows_x86_64": [
+            "//source/common/api:os_sys_calls_lib",
+            "//source/common/common:thread_lib",
+        ],
+        "//conditions:default": [],
+    }),
 )

source/common/filesystem/directory.h

@@ -2,6 +2,8 @@
 
 #include <string>
 
+#include "envoy/filesystem/filesystem.h"
+
 #include "common/filesystem/directory_iterator_impl.h"
 
 namespace Envoy {

source/common/filesystem/inotify/watcher_impl.cc

@@ -3,6 +3,7 @@
 #include <cstdint>
 #include <string>
 
+#include "envoy/api/api.h"
 #include "envoy/common/exception.h"
 #include "envoy/event/dispatcher.h"
 #include "envoy/event/file_event.h"
@@ -15,8 +16,8 @@
 namespace Envoy {
 namespace Filesystem {
 
-WatcherImpl::WatcherImpl(Event::Dispatcher& dispatcher)
-    : inotify_fd_(inotify_init1(IN_NONBLOCK)),
+WatcherImpl::WatcherImpl(Event::Dispatcher& dispatcher, Api::Api& api)
+    : api_(api), inotify_fd_(inotify_init1(IN_NONBLOCK)),
       inotify_event_(dispatcher.createFileEvent(
           inotify_fd_,
           [this](uint32_t events) -> void {
@@ -30,19 +31,15 @@ WatcherImpl::WatcherImpl(Event::Dispatcher& dispatcher)
 
 WatcherImpl::~WatcherImpl() { close(inotify_fd_); }
 
-void WatcherImpl::addWatch(const std::string& path, uint32_t events, OnChangedCb callback) {
+void WatcherImpl::addWatch(absl::string_view path, uint32_t events, OnChangedCb callback) {
   // Because of general inotify pain, we always watch the directory that the file lives in,
   // and then synthetically raise per file events.
-  size_t last_slash = path.rfind('/');
-  if (last_slash == std::string::npos) {
-    throw EnvoyException(absl::StrCat("invalid watch path ", path));
-  }
-
-  std::string directory = last_slash != 0 ? path.substr(0, last_slash) : "/";
-  std::string file = StringUtil::subspan(path, last_slash + 1, path.size());
+  auto result = api_.fileSystem().splitPathFromFilename(path);
+  auto directory = result.first;
+  auto file = std::string(result.second);
 
   const uint32_t watch_mask = IN_MODIFY | IN_MOVED_TO;
-  int watch_fd = inotify_add_watch(inotify_fd_, directory.c_str(), watch_mask);
+  int watch_fd = inotify_add_watch(inotify_fd_, std::string(directory).c_str(), watch_mask);
   if (watch_fd == -1) {
     throw EnvoyException(
         fmt::format("unable to add filesystem watch for file {}: {}", path, strerror(errno)));

source/common/filesystem/inotify/watcher_impl.h

@@ -5,6 +5,7 @@
 #include <string>
 #include <unordered_map>
 
+#include "envoy/api/api.h"
 #include "envoy/event/dispatcher.h"
 #include "envoy/filesystem/watcher.h"
 
@@ -20,11 +21,11 @@ namespace Filesystem {
  */
 class WatcherImpl : public Watcher, Logger::Loggable<Logger::Id::file> {
 public:
-  WatcherImpl(Event::Dispatcher& dispatcher);
+  WatcherImpl(Event::Dispatcher& dispatcher, Api::Api& api);
   ~WatcherImpl() override;
 
   // Filesystem::Watcher
-  void addWatch(const std::string& path, uint32_t events, OnChangedCb cb) override;
+  void addWatch(absl::string_view path, uint32_t events, OnChangedCb cb) override;
 
 private:
   struct FileWatch {
@@ -39,6 +40,7 @@ class WatcherImpl : public Watcher, Logger::Loggable<Logger::Id::file> {
 
   void onInotifyEvent();
 
+  Api::Api& api_;
   int inotify_fd_;
   Event::FileEventPtr inotify_event_;
   std::unordered_map<int, DirectoryWatch> callback_map_;

source/common/filesystem/kqueue/watcher_impl.cc

@@ -16,15 +16,16 @@
 namespace Envoy {
 namespace Filesystem {
 
-WatcherImpl::WatcherImpl(Event::Dispatcher& dispatcher)
-    : queue_(kqueue()), kqueue_event_(dispatcher.createFileEvent(
-                            queue_,
-                            [this](uint32_t events) -> void {
-                              if (events & Event::FileReadyType::Read) {
-                                onKqueueEvent();
-                              }
-                            },
-                            Event::FileTriggerType::Edge, Event::FileReadyType::Read)) {}
+WatcherImpl::WatcherImpl(Event::Dispatcher& dispatcher, Api::Api& api)
+    : api_(api), queue_(kqueue()),
+    : kqueue_event_(dispatcher.createFileEvent(
+          queue_,
+          [this](uint32_t events) -> void {
+            if (events & Event::FileReadyType::Read) {
+              onKqueueEvent();
+            }
+          },
+          Event::FileTriggerType::Edge, Event::FileReadyType::Read)) {}
 
 WatcherImpl::~WatcherImpl() {
   close(queue_);
@@ -38,7 +39,7 @@ void WatcherImpl::addWatch(const std::string& path, uint32_t events, Watcher::On
   }
 }
 
-WatcherImpl::FileWatchPtr WatcherImpl::addWatch(const std::string& path, uint32_t events,
+WatcherImpl::FileWatchPtr WatcherImpl::addWatch(absl::string_view path, uint32_t events,
                                                 Watcher::OnChangedCb cb, bool path_must_exist) {
   bool watching_dir = false;
   int watch_fd = open(path.c_str(), O_SYMLINK);
@@ -47,13 +48,8 @@ WatcherImpl::FileWatchPtr WatcherImpl::addWatch(const std::string& path, uint32_
       return nullptr;
     }
 
-    size_t last_slash = path.rfind('/');
-    if (last_slash == std::string::npos) {
-      return nullptr;
-    }
-
-    std::string directory = path.substr(0, last_slash);
-    watch_fd = open(directory.c_str(), 0);
+    auto directory = api_fileSystem().splitPathFromFilename(path).first;
+    watch_fd = open(std::string(directory).c_str(), 0);
     if (watch_fd == -1) {
       return nullptr;
     }
@@ -63,7 +59,7 @@ WatcherImpl::FileWatchPtr WatcherImpl::addWatch(const std::string& path, uint32_
 
   FileWatchPtr watch(new FileWatch());
   watch->fd_ = watch_fd;
-  watch->file_ = path;
+  watch->file_ = std::string(path);
   watch->events_ = events;
   watch->callback_ = cb;
   watch->watching_dir_ = watching_dir;

source/common/filesystem/kqueue/watcher_impl.h

@@ -4,6 +4,7 @@
 #include <list>
 #include <string>
 
+#include "envoy/api/api.h"
 #include "envoy/event/dispatcher.h"
 #include "envoy/filesystem/watcher.h"
 
@@ -24,7 +25,7 @@ class WatcherImpl : public Watcher, Logger::Loggable<Logger::Id::file> {
   ~WatcherImpl();
 
   // Filesystem::Watcher
-  void addWatch(const std::string& path, uint32_t events, OnChangedCb cb) override;
+  void addWatch(absl::string_view path, uint32_t events, OnChangedCb cb) override;
 
 private:
   struct FileWatch : LinkedObject<FileWatch> {
@@ -40,10 +41,11 @@ class WatcherImpl : public Watcher, Logger::Loggable<Logger::Id::file> {
   using FileWatchPtr = std::shared_ptr<FileWatch>;
 
   void onKqueueEvent();
-  FileWatchPtr addWatch(const std::string& path, uint32_t events, Watcher::OnChangedCb cb,
+  FileWatchPtr addWatch(absl::string_view path, uint32_t events, Watcher::OnChangedCb cb,
                         bool pathMustExist);
   void removeWatch(FileWatchPtr& watch);
 
+  Api::Api& api_;
   int queue_;
   std::unordered_map<int, FileWatchPtr> watches_;
   Event::FileEventPtr kqueue_event_;

source/common/filesystem/posix/directory_iterator_impl.cc

@@ -55,7 +55,11 @@ FileType DirectoryIteratorImpl::fileType(const std::string& full_path) const {
 
   const Api::SysCallIntResult result = os_sys_calls_.stat(full_path.c_str(), &stat_buf);
   if (result.rc_ != 0) {
-    throw EnvoyException(fmt::format("unable to stat file: '{}'", full_path));
+    if (errno == ENOENT)
+      // Special case, a symlink is broken, target couldn't be stat'ed
+      if (::lstat(full_path.c_str(), &stat_buf) == 0 && S_ISLNK(stat_buf.st_mode))
+        return FileType::Regular;
+    throw EnvoyException(fmt::format("unable to stat file: '{}' ({})", full_path, errno));
   }
 
   if (S_ISDIR(stat_buf.st_mode)) {

source/common/filesystem/posix/filesystem_impl.cc

@@ -108,6 +108,19 @@ std::string InstanceImplPosix::fileReadToEnd(const std::string& path) {
   return file_string.str();
 }
 
+std::pair<absl::string_view, absl::string_view>
+InstanceImplPosix::splitPathFromFilename(absl::string_view path) {
+  size_t last_slash = path.rfind('/');
+  if (last_slash == std::string::npos) {
+    throw EnvoyException(fmt::format("invalid file path {}", path));
+  }
+  absl::string_view name = path.substr(last_slash + 1);
+  // truncate all trailing slashes, except root slash
+  if (last_slash == 0)
+    ++last_slash;
+  return std::make_pair(path.substr(0, last_slash), name);
+}
+
 bool InstanceImplPosix::illegalPath(const std::string& path) {
   // Special case, allow /dev/fd/* access here so that config can be passed in a
   // file descriptor from a bootstrap script via exec. The reason we do this
@@ -141,7 +154,6 @@ bool InstanceImplPosix::illegalPath(const std::string& path) {
 }
 
 Api::SysCallStringResult InstanceImplPosix::canonicalPath(const std::string& path) {
-  // TODO(htuch): When we are using C++17, switch to std::filesystem::canonical.
   char* resolved_path = ::realpath(path.c_str(), nullptr);
   if (resolved_path == nullptr) {
     return {std::string(), errno};

source/common/filesystem/posix/filesystem_impl.h

@@ -39,6 +39,8 @@ class InstanceImplPosix : public Instance {
   bool directoryExists(const std::string& path) override;
   ssize_t fileSize(const std::string& path) override;
   std::string fileReadToEnd(const std::string& path) override;
+  std::pair<absl::string_view, absl::string_view>
+  splitPathFromFilename(absl::string_view path) override;
   bool illegalPath(const std::string& path) override;
 
 private: