diff --git a/include/envoy/api/io_error.h b/include/envoy/api/io_error.h index 91c0a88e76f7f..fb5dda090efc5 100644 --- a/include/envoy/api/io_error.h +++ b/include/envoy/api/io_error.h @@ -68,6 +68,8 @@ template struct IoCallResult { IoErrorPtr err_; }; +using IoCallBoolResult = IoCallResult; +using IoCallSizeResult = IoCallResult; using IoCallUint64Result = IoCallResult; inline Api::IoCallUint64Result ioCallUint64ResultNoError() { diff --git a/include/envoy/api/os_sys_calls.h b/include/envoy/api/os_sys_calls.h index 07b11a65c4286..2a4161c5d5084 100644 --- a/include/envoy/api/os_sys_calls.h +++ b/include/envoy/api/os_sys_calls.h @@ -1,16 +1,20 @@ #pragma once +#ifndef WIN32 #include #include // for mode_t #include // for sockaddr #include #include // for iovec +#endif + #include #include #include "envoy/api/os_sys_calls_common.h" #include "envoy/common/pure.h" +#include "envoy/common/platform.h" namespace Envoy { namespace Api { diff --git a/include/envoy/common/platform.h b/include/envoy/common/platform.h index e98b9d1365627..12e8f4bca33c2 100644 --- a/include/envoy/common/platform.h +++ b/include/envoy/common/platform.h @@ -2,12 +2,18 @@ // NOLINT(namespace-envoy) #ifdef _MSC_VER -#include +#include #define PACKED_STRUCT(definition, ...) \ __pragma(pack(push, 1)) definition, ##__VA_ARGS__; \ __pragma(pack(pop)) +#ifdef _M_X64 +using ssize_t = int64_t; +#else +#error Envoy is not supported on 32-bit Windows +#endif + #else #define PACKED_STRUCT(definition, ...) definition, ##__VA_ARGS__ __attribute__((packed)) diff --git a/include/envoy/filesystem/BUILD b/include/envoy/filesystem/BUILD index d61b9ed36d0a1..e740d2ee25e40 100644 --- a/include/envoy/filesystem/BUILD +++ b/include/envoy/filesystem/BUILD @@ -12,6 +12,7 @@ envoy_cc_library( name = "filesystem_interface", hdrs = ["filesystem.h"], deps = [ + "//include/envoy/api:io_error_interface", "//include/envoy/api:os_sys_calls_interface", "//include/envoy/event:dispatcher_interface", ], diff --git a/include/envoy/filesystem/filesystem.h b/include/envoy/filesystem/filesystem.h index 194a5a6376600..a866cff3afb79 100644 --- a/include/envoy/filesystem/filesystem.h +++ b/include/envoy/filesystem/filesystem.h @@ -4,7 +4,8 @@ #include #include -#include "envoy/api/os_sys_calls.h" +#include "envoy/api/io_error.h" +#include "envoy/common/platform.h" #include "envoy/common/pure.h" #include "absl/strings/string_view.h" @@ -25,37 +26,31 @@ class File { * * @return bool whether the open succeeded */ - virtual Api::SysCallBoolResult open() PURE; + virtual Api::IoCallBoolResult open() PURE; /** * Write the buffer to the file. The file must be explicitly opened before writing. * * @return ssize_t number of bytes written, or -1 for failure */ - virtual Api::SysCallSizeResult write(absl::string_view buffer) PURE; + virtual Api::IoCallSizeResult write(absl::string_view buffer) PURE; /** * Close the file. * * @return bool whether the close succeeded */ - virtual Api::SysCallBoolResult close() PURE; + virtual Api::IoCallBoolResult close() PURE; /** * @return bool is the file open */ - virtual bool isOpen() PURE; + virtual bool isOpen() const PURE; /** * @return string the file path */ - virtual std::string path() PURE; - - /** - * @return string a human-readable string describing the error code - * TODO(sesmith177) Use the IOError class after #5829 merges - */ - virtual std::string errorToString(int error) PURE; + virtual std::string path() const PURE; }; using FilePtr = std::unique_ptr; @@ -97,12 +92,6 @@ class Instance { */ virtual std::string fileReadToEnd(const std::string& path) PURE; - /** - * @param path some filesystem path. - * @return SysCallStringResult containing the canonical path (see realpath(3)). - */ - virtual Api::SysCallStringResult canonicalPath(const std::string& 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 diff --git a/source/common/access_log/access_log_manager_impl.cc b/source/common/access_log/access_log_manager_impl.cc index 14a27be7b61e5..cbf5f8caa4a6a 100644 --- a/source/common/access_log/access_log_manager_impl.cc +++ b/source/common/access_log/access_log_manager_impl.cc @@ -42,10 +42,10 @@ AccessLogFileImpl::AccessLogFileImpl(Filesystem::FilePtr&& file, Event::Dispatch } void AccessLogFileImpl::open() { - const Api::SysCallBoolResult result = file_->open(); + const Api::IoCallBoolResult result = file_->open(); if (!result.rc_) { - throw EnvoyException(fmt::format("unable to open file '{}': {}", file_->path(), - file_->errorToString(result.errno_))); + throw EnvoyException( + fmt::format("unable to open file '{}': {}", file_->path(), result.err_->getErrorDetails())); } } @@ -68,9 +68,9 @@ AccessLogFileImpl::~AccessLogFileImpl() { doWrite(flush_buffer_); } - const Api::SysCallBoolResult result = file_->close(); + const Api::IoCallBoolResult result = file_->close(); ASSERT(result.rc_, fmt::format("unable to close file '{}': {}", file_->path(), - file_->errorToString(result.errno_))); + result.err_->getErrorDetails())); } } @@ -91,7 +91,7 @@ void AccessLogFileImpl::doWrite(Buffer::Instance& buffer) { Thread::LockGuard lock(file_lock_); for (const Buffer::RawSlice& slice : slices) { absl::string_view data(static_cast(slice.mem_), slice.len_); - const Api::SysCallSizeResult result = file_->write(data); + const Api::IoCallSizeResult result = file_->write(data); ASSERT(result.rc_ == static_cast(slice.len_)); stats_.write_completed_.inc(); } @@ -131,9 +131,9 @@ void AccessLogFileImpl::flushThreadFunc() { try { if (reopen_file_) { reopen_file_ = false; - const Api::SysCallBoolResult result = file_->close(); + const Api::IoCallBoolResult result = file_->close(); ASSERT(result.rc_, fmt::format("unable to close file '{}': {}", file_->path(), - file_->errorToString(result.errno_))); + result.err_->getErrorDetails())); open(); } diff --git a/source/common/api/BUILD b/source/common/api/BUILD index d4dd9c3950223..737ab7d2ada48 100644 --- a/source/common/api/BUILD +++ b/source/common/api/BUILD @@ -16,7 +16,6 @@ envoy_cc_library( "//include/envoy/api:api_interface", "//source/common/common:thread_lib", "//source/common/event:dispatcher_lib", - "//source/common/filesystem:filesystem_lib", ], ) diff --git a/source/common/api/api_impl.cc b/source/common/api/api_impl.cc index 43477603fdd3c..9f997f377a09b 100644 --- a/source/common/api/api_impl.cc +++ b/source/common/api/api_impl.cc @@ -9,8 +9,9 @@ namespace Envoy { namespace Api { -Impl::Impl(Thread::ThreadFactory& thread_factory, Stats::Store&, Event::TimeSystem& time_system) - : thread_factory_(thread_factory), time_system_(time_system) {} +Impl::Impl(Thread::ThreadFactory& thread_factory, Stats::Store&, Event::TimeSystem& time_system, + Filesystem::Instance& file_system) + : thread_factory_(thread_factory), time_system_(time_system), file_system_(file_system) {} Event::DispatcherPtr Impl::allocateDispatcher() { return std::make_unique(*this, time_system_); diff --git a/source/common/api/api_impl.h b/source/common/api/api_impl.h index f2c6e8b671a07..61733b4ce53fe 100644 --- a/source/common/api/api_impl.h +++ b/source/common/api/api_impl.h @@ -8,8 +8,6 @@ #include "envoy/filesystem/filesystem.h" #include "envoy/thread/thread.h" -#include "common/filesystem/filesystem_impl.h" - namespace Envoy { namespace Api { @@ -18,7 +16,8 @@ namespace Api { */ class Impl : public Api { public: - Impl(Thread::ThreadFactory& thread_factory, Stats::Store&, Event::TimeSystem& time_system); + Impl(Thread::ThreadFactory& thread_factory, Stats::Store&, Event::TimeSystem& time_system, + Filesystem::Instance& file_system); // Api::Api Event::DispatcherPtr allocateDispatcher() override; @@ -29,8 +28,8 @@ class Impl : public Api { private: Thread::ThreadFactory& thread_factory_; - Filesystem::InstanceImpl file_system_; Event::TimeSystem& time_system_; + Filesystem::Instance& file_system_; }; } // namespace Api diff --git a/source/common/filesystem/BUILD b/source/common/filesystem/BUILD index 76b74aa9ea9b5..f88f9d6fa2e37 100644 --- a/source/common/filesystem/BUILD +++ b/source/common/filesystem/BUILD @@ -40,10 +40,36 @@ envoy_cc_posix_library( envoy_cc_library( name = "filesystem_lib", - srcs = ["filesystem_impl.cc"], - hdrs = ["filesystem_impl.h"], + deps = envoy_cc_platform_dep("filesystem_impl_lib"), +) + +envoy_cc_posix_library( + name = "filesystem_impl_lib", + srcs = ["posix/filesystem_impl.cc"], + hdrs = ["posix/filesystem_impl.h"], + strip_include_prefix = "posix", + deps = [ + ":file_shared_lib", + ], +) + +envoy_cc_win32_library( + name = "filesystem_impl_lib", + srcs = ["win32/filesystem_impl.cc"], + hdrs = ["win32/filesystem_impl.h"], + strip_include_prefix = "win32", + deps = [ + ":file_shared_lib", + ], +) + +envoy_cc_library( + name = "file_shared_lib", + srcs = ["file_shared_impl.cc"], + hdrs = ["file_shared_impl.h"], deps = [ "//include/envoy/filesystem:filesystem_interface", + "//source/common/common:assert_lib", ], ) diff --git a/source/common/filesystem/file_shared_impl.cc b/source/common/filesystem/file_shared_impl.cc new file mode 100644 index 0000000000000..5e235c534e639 --- /dev/null +++ b/source/common/filesystem/file_shared_impl.cc @@ -0,0 +1,39 @@ +#include "common/filesystem/file_shared_impl.h" + +#include + +namespace Envoy { +namespace Filesystem { + +Api::IoError::IoErrorCode IoFileError::getErrorCode() const { return IoErrorCode::UnknownError; } + +std::string IoFileError::getErrorDetails() const { return ::strerror(errno_); } + +Api::IoCallBoolResult FileSharedImpl::open() { + if (isOpen()) { + return resultSuccess(true); + } + + openFile(); + return fd_ != -1 ? resultSuccess(true) : resultFailure(false, errno); +} + +Api::IoCallSizeResult FileSharedImpl::write(absl::string_view buffer) { + const ssize_t rc = writeFile(buffer); + return rc != -1 ? resultSuccess(rc) : resultFailure(rc, errno); +}; + +Api::IoCallBoolResult FileSharedImpl::close() { + ASSERT(isOpen()); + + bool success = closeFile(); + fd_ = -1; + return success ? resultSuccess(true) : resultFailure(false, errno); +} + +bool FileSharedImpl::isOpen() const { return fd_ != -1; }; + +std::string FileSharedImpl::path() const { return path_; }; + +} // namespace Filesystem +} // namespace Envoy \ No newline at end of file diff --git a/source/common/filesystem/file_shared_impl.h b/source/common/filesystem/file_shared_impl.h new file mode 100644 index 0000000000000..d018ed0314ae9 --- /dev/null +++ b/source/common/filesystem/file_shared_impl.h @@ -0,0 +1,61 @@ +#pragma once + +#include + +#include "envoy/filesystem/filesystem.h" + +#include "common/common/assert.h" + +namespace Envoy { +namespace Filesystem { + +class IoFileError : public Api::IoError { +public: + explicit IoFileError(int sys_errno) : errno_(sys_errno) {} + + ~IoFileError() override {} + + Api::IoError::IoErrorCode getErrorCode() const override; + std::string getErrorDetails() const override; + +private: + const int errno_; +}; + +using IoFileErrorPtr = std::unique_ptr; + +template Api::IoCallResult resultFailure(T result, int sys_errno) { + return {result, IoFileErrorPtr(new IoFileError(sys_errno), [](Api::IoError* err) { + ASSERT(err != nullptr); + delete err; + })}; +} + +template Api::IoCallResult resultSuccess(T result) { + return {result, IoFileErrorPtr(nullptr, [](Api::IoError*) { NOT_REACHED_GCOVR_EXCL_LINE; })}; +} + +class FileSharedImpl : public File { +public: + FileSharedImpl(const std::string& path) : fd_(-1), path_(path) {} + + virtual ~FileSharedImpl() {} + + // Filesystem::File + Api::IoCallBoolResult open() override; + Api::IoCallSizeResult write(absl::string_view buffer) override; + Api::IoCallBoolResult close() override; + bool isOpen() const override; + std::string path() const override; + +protected: + virtual void openFile() PURE; + virtual ssize_t writeFile(absl::string_view buffer) PURE; + virtual bool closeFile() PURE; + + int fd_; + const std::string path_; +}; + +} // namespace Filesystem +} // namespace Envoy \ No newline at end of file diff --git a/source/common/filesystem/filesystem_impl.cc b/source/common/filesystem/posix/filesystem_impl.cc similarity index 65% rename from source/common/filesystem/filesystem_impl.cc rename to source/common/filesystem/posix/filesystem_impl.cc index a940c12f6fae9..214d97aed3240 100644 --- a/source/common/filesystem/filesystem_impl.cc +++ b/source/common/filesystem/posix/filesystem_impl.cc @@ -1,5 +1,3 @@ -#include "common/filesystem/filesystem_impl.h" - #include #include #include @@ -16,69 +14,43 @@ #include "common/common/assert.h" #include "common/common/fmt.h" #include "common/common/logger.h" +#include "common/filesystem/filesystem_impl.h" #include "absl/strings/match.h" namespace Envoy { namespace Filesystem { -FileImpl::FileImpl(const std::string& path) : fd_(-1), path_(path) {} - -FileImpl::~FileImpl() { +FileImplPosix::~FileImplPosix() { if (isOpen()) { - const Api::SysCallBoolResult result = close(); + const Api::IoCallBoolResult result = close(); ASSERT(result.rc_); } } -Api::SysCallBoolResult FileImpl::open() { - if (isOpen()) { - return {true, 0}; - } - +void FileImplPosix::openFile() { const int flags = O_RDWR | O_APPEND | O_CREAT; const int mode = S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH; fd_ = ::open(path_.c_str(), flags, mode); - if (-1 == fd_) { - return {false, errno}; - } - return {true, 0}; -} - -Api::SysCallSizeResult FileImpl::write(absl::string_view buffer) { - const ssize_t rc = ::write(fd_, buffer.data(), buffer.size()); - return {rc, errno}; } -Api::SysCallBoolResult FileImpl::close() { - ASSERT(isOpen()); - - const int rc = ::close(fd_); - if (rc == -1) { - return {false, errno}; - } - - fd_ = -1; - return {true, 0}; +ssize_t FileImplPosix::writeFile(absl::string_view buffer) { + return ::write(fd_, buffer.data(), buffer.size()); } -bool FileImpl::isOpen() { return fd_ != -1; } - -std::string FileImpl::path() { return path_; } +bool FileImplPosix::closeFile() { return ::close(fd_) != -1; } -std::string FileImpl::errorToString(int error) { return ::strerror(error); } - -FilePtr InstanceImpl::createFile(const std::string& path) { - return std::make_unique(path); +FilePtr InstanceImplPosix::createFile(const std::string& path) { + return std::make_unique(path); } -bool InstanceImpl::fileExists(const std::string& path) { +bool InstanceImplPosix::fileExists(const std::string& path) { std::ifstream input_file(path); return input_file.is_open(); } -bool InstanceImpl::directoryExists(const std::string& path) { +bool InstanceImplPosix::directoryExists(const std::string& path) { DIR* const dir = ::opendir(path.c_str()); const bool dir_exists = nullptr != dir; if (dir_exists) { @@ -88,7 +60,7 @@ bool InstanceImpl::directoryExists(const std::string& path) { return dir_exists; } -ssize_t InstanceImpl::fileSize(const std::string& path) { +ssize_t InstanceImplPosix::fileSize(const std::string& path) { struct stat info; if (::stat(path.c_str(), &info) != 0) { return -1; @@ -96,7 +68,7 @@ ssize_t InstanceImpl::fileSize(const std::string& path) { return info.st_size; } -std::string InstanceImpl::fileReadToEnd(const std::string& path) { +std::string InstanceImplPosix::fileReadToEnd(const std::string& path) { if (illegalPath(path)) { throw EnvoyException(fmt::format("Invalid path: {}", path)); } @@ -104,7 +76,7 @@ std::string InstanceImpl::fileReadToEnd(const std::string& path) { std::ios::sync_with_stdio(false); std::ifstream file(path); - if (!file) { + if (file.fail()) { throw EnvoyException(fmt::format("unable to read file: {}", path)); } @@ -114,18 +86,7 @@ std::string InstanceImpl::fileReadToEnd(const std::string& path) { return file_string.str(); } -Api::SysCallStringResult InstanceImpl::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}; - } - std::string resolved_path_string{resolved_path}; - ::free(resolved_path); - return {resolved_path_string, 0}; -} - -bool InstanceImpl::illegalPath(const std::string& path) { +bool InstanceImplPosix::illegalPath(const std::string& path) { const Api::SysCallStringResult canonical_path = canonicalPath(path); if (canonical_path.rc_.empty()) { ENVOY_LOG_MISC(debug, "Unable to determine canonical path for {}: {}", path, @@ -146,5 +107,16 @@ bool InstanceImpl::illegalPath(const std::string& path) { return false; } +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}; + } + std::string resolved_path_string{resolved_path}; + ::free(resolved_path); + return {resolved_path_string, 0}; +} + } // namespace Filesystem } // namespace Envoy diff --git a/source/common/filesystem/filesystem_impl.h b/source/common/filesystem/posix/filesystem_impl.h similarity index 54% rename from source/common/filesystem/filesystem_impl.h rename to source/common/filesystem/posix/filesystem_impl.h index ec6424932323e..8c6279e664999 100644 --- a/source/common/filesystem/filesystem_impl.h +++ b/source/common/filesystem/posix/filesystem_impl.h @@ -3,31 +3,29 @@ #include #include -#include "envoy/filesystem/filesystem.h" +#include "envoy/api/os_sys_calls.h" + +#include "common/filesystem/file_shared_impl.h" namespace Envoy { namespace Filesystem { -class FileImpl : public File { +class FileImplPosix : public FileSharedImpl { public: - FileImpl(const std::string& path); - ~FileImpl(); + FileImplPosix(const std::string& path) : FileSharedImpl(path) {} + ~FileImplPosix(); - // Filesystem::File - Api::SysCallBoolResult open() override; - Api::SysCallSizeResult write(absl::string_view buffer) override; - Api::SysCallBoolResult close() override; - bool isOpen() override; - std::string path() override; - std::string errorToString(int error) override; +protected: + // Filesystem::FileSharedImpl + void openFile() override; + ssize_t writeFile(absl::string_view buffer) override; + bool closeFile() override; private: - int fd_; - const std::string path_; friend class FileSystemImplTest; }; -class InstanceImpl : public Instance { +class InstanceImplPosix : public Instance { public: // Filesystem::Instance FilePtr createFile(const std::string& path) override; @@ -35,9 +33,12 @@ class InstanceImpl : 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; - Api::SysCallStringResult canonicalPath(const std::string& path) override; bool illegalPath(const std::string& path) override; + +private: + Api::SysCallStringResult canonicalPath(const std::string& path); + friend class FileSystemImplTest; }; } // namespace Filesystem -} // namespace Envoy \ No newline at end of file +} // namespace Envoy diff --git a/source/common/filesystem/win32/filesystem_impl.cc b/source/common/filesystem/win32/filesystem_impl.cc new file mode 100644 index 0000000000000..41a15235ef3cd --- /dev/null +++ b/source/common/filesystem/win32/filesystem_impl.cc @@ -0,0 +1,100 @@ +#include +#include +#include +#include + +// uses macros to #define a ton of symbols, two of which (DELETE and GetMessage) +// interfere with our code. DELETE shows up in the base.pb.h header generated from +// api/envoy/api/core/base.proto. Since it's a generated header, we can't #undef DELETE at +// the top of that header to avoid the collision. Similarly, GetMessage shows up in generated +// protobuf code so we can't #undef the symbol there. +#undef DELETE +#undef GetMessage + +#include "common/common/assert.h" +#include "common/filesystem/filesystem_impl.h" + +#include +#include +#include +#include + +#include "envoy/common/exception.h" + +#include "common/common/fmt.h" + +namespace Envoy { +namespace Filesystem { + +FileImplWin32::~FileImplWin32() { + if (isOpen()) { + const Api::IoCallBoolResult result = close(); + ASSERT(result.rc_); + } +} + +void FileImplWin32::openFile() { + const int flags = _O_RDWR | _O_APPEND | _O_CREAT; + const int mode = _S_IREAD | _S_IWRITE; + + fd_ = ::_open(path_.c_str(), flags, mode); +} + +ssize_t FileImplWin32::writeFile(absl::string_view buffer) { + return ::_write(fd_, buffer.data(), buffer.size()); +} + +bool FileImplWin32::closeFile() { return ::_close(fd_) != -1; } + +FilePtr InstanceImplWin32::createFile(const std::string& path) { + return std::make_unique(path); +} + +bool InstanceImplWin32::fileExists(const std::string& path) { + const DWORD attributes = ::GetFileAttributes(path.c_str()); + return attributes != INVALID_FILE_ATTRIBUTES; +} + +bool InstanceImplWin32::directoryExists(const std::string& path) { + const DWORD attributes = ::GetFileAttributes(path.c_str()); + if (attributes == INVALID_FILE_ATTRIBUTES) { + return false; + } + return attributes & FILE_ATTRIBUTE_DIRECTORY; +} + +ssize_t InstanceImplWin32::fileSize(const std::string& path) { + struct _stat info; + if (::_stat(path.c_str(), &info) != 0) { + return -1; + } + return info.st_size; +} + +std::string InstanceImplWin32::fileReadToEnd(const std::string& path) { + if (illegalPath(path)) { + throw EnvoyException(fmt::format("Invalid path: {}", path)); + } + + std::ios::sync_with_stdio(false); + + // On Windows, we need to explicitly set the file mode as binary. Otherwise, + // 0x1a will be treated as EOF + std::ifstream file(path, std::ios_base::binary); + if (file.fail()) { + throw EnvoyException(fmt::format("unable to read file: {}", path)); + } + + std::stringstream file_string; + file_string << file.rdbuf(); + + return file_string.str(); +} + +bool InstanceImplWin32::illegalPath(const std::string& path) { + // Currently, we don't know of any obviously illegal paths on Windows + return false; +} + +} // namespace Filesystem +} // namespace Envoy diff --git a/source/common/filesystem/win32/filesystem_impl.h b/source/common/filesystem/win32/filesystem_impl.h new file mode 100644 index 0000000000000..7c7add205c87d --- /dev/null +++ b/source/common/filesystem/win32/filesystem_impl.h @@ -0,0 +1,38 @@ +#pragma once + +#include +#include + +#include "common/filesystem/file_shared_impl.h" + +namespace Envoy { +namespace Filesystem { + +class FileImplWin32 : public FileSharedImpl { +public: + FileImplWin32(const std::string& path) : FileSharedImpl(path) {} + ~FileImplWin32(); + +protected: + // Filesystem::FileSharedImpl + void openFile() override; + ssize_t writeFile(absl::string_view buffer) override; + bool closeFile() override; + +private: + friend class FileSystemImplTest; +}; + +class InstanceImplWin32 : public Instance { +public: + // Filesystem::Instance + FilePtr createFile(const std::string& path) override; + bool fileExists(const std::string& path) override; + bool directoryExists(const std::string& path) override; + ssize_t fileSize(const std::string& path) override; + std::string fileReadToEnd(const std::string& path) override; + bool illegalPath(const std::string& path) override; +}; + +} // namespace Filesystem +} // namespace Envoy diff --git a/source/exe/BUILD b/source/exe/BUILD index 3ab1d2d4c8fc4..4298fb5e36666 100644 --- a/source/exe/BUILD +++ b/source/exe/BUILD @@ -88,7 +88,10 @@ envoy_cc_posix_library( name = "platform_impl_lib", hdrs = ["posix/platform_impl.h"], strip_include_prefix = "posix", - deps = ["//source/common/common:thread_lib"], + deps = [ + "//source/common/common:thread_lib", + "//source/common/filesystem:filesystem_lib", + ], ) envoy_cc_win32_library( @@ -98,6 +101,7 @@ envoy_cc_win32_library( deps = [ "//source/common/common:assert_lib", "//source/common/common:thread_lib", + "//source/common/filesystem:filesystem_lib", ], ) diff --git a/source/exe/main_common.cc b/source/exe/main_common.cc index 5b7f9e14be832..7071c486fd402 100644 --- a/source/exe/main_common.cc +++ b/source/exe/main_common.cc @@ -45,8 +45,10 @@ Runtime::LoaderPtr ProdComponentFactory::createRuntime(Server::Instance& server, MainCommonBase::MainCommonBase(const OptionsImpl& options, Event::TimeSystem& time_system, TestHooks& test_hooks, Server::ComponentFactory& component_factory, std::unique_ptr&& random_generator, - Thread::ThreadFactory& thread_factory) - : options_(options), component_factory_(component_factory), thread_factory_(thread_factory) { + Thread::ThreadFactory& thread_factory, + Filesystem::Instance& file_system) + : options_(options), component_factory_(component_factory), thread_factory_(thread_factory), + file_system_(file_system) { Thread::ThreadFactorySingleton::set(&thread_factory_); ares_library_init(ARES_LIB_INIT_ALL); Event::Libevent::Global::initialize(); @@ -83,7 +85,8 @@ MainCommonBase::MainCommonBase(const OptionsImpl& options, Event::TimeSystem& ti server_ = std::make_unique( options_, time_system, local_address, test_hooks, *restarter_, *stats_store_, - access_log_lock, component_factory, std::move(random_generator), *tls_, thread_factory); + access_log_lock, component_factory, std::move(random_generator), *tls_, thread_factory_, + file_system_); break; } @@ -115,7 +118,8 @@ bool MainCommonBase::run() { return true; case Server::Mode::Validate: { auto local_address = Network::Utility::getLocalAddress(options_.localAddressIpVersion()); - return Server::validateConfig(options_, local_address, component_factory_, thread_factory_); + return Server::validateConfig(options_, local_address, component_factory_, thread_factory_, + file_system_); } case Server::Mode::InitOnly: PERF_DUMP(); @@ -139,7 +143,8 @@ void MainCommonBase::adminRequest(absl::string_view path_and_query, absl::string MainCommon::MainCommon(int argc, const char* const* argv) : options_(argc, argv, &MainCommon::hotRestartVersion, spdlog::level::info), base_(options_, real_time_system_, default_test_hooks_, prod_component_factory_, - std::make_unique(), platform_impl_.threadFactory()) {} + std::make_unique(), platform_impl_.threadFactory(), + platform_impl_.fileSystem()) {} std::string MainCommon::hotRestartVersion(uint64_t max_num_stats, uint64_t max_stat_name_len, bool hot_restart_enabled) { diff --git a/source/exe/main_common.h b/source/exe/main_common.h index aa1821a57dc40..81b20c272d096 100644 --- a/source/exe/main_common.h +++ b/source/exe/main_common.h @@ -36,7 +36,7 @@ class MainCommonBase { MainCommonBase(const OptionsImpl& options, Event::TimeSystem& time_system, TestHooks& test_hooks, Server::ComponentFactory& component_factory, std::unique_ptr&& random_generator, - Thread::ThreadFactory& thread_factory); + Thread::ThreadFactory& thread_factory, Filesystem::Instance& file_system); ~MainCommonBase(); bool run(); @@ -67,6 +67,7 @@ class MainCommonBase { Server::ComponentFactory& component_factory_; Thread::ThreadFactory& thread_factory_; + Filesystem::Instance& file_system_; std::unique_ptr tls_; std::unique_ptr restarter_; diff --git a/source/exe/posix/platform_impl.h b/source/exe/posix/platform_impl.h index f43f765172a19..45fbd7340779c 100644 --- a/source/exe/posix/platform_impl.h +++ b/source/exe/posix/platform_impl.h @@ -1,16 +1,18 @@ #pragma once -#include "common/common/macros.h" #include "common/common/thread_impl.h" +#include "common/filesystem/filesystem_impl.h" namespace Envoy { class PlatformImpl { public: Thread::ThreadFactory& threadFactory() { return thread_factory_; } + Filesystem::Instance& fileSystem() { return file_system_; } private: Thread::ThreadFactoryImplPosix thread_factory_; + Filesystem::InstanceImplPosix file_system_; }; -} // namespace Envoy \ No newline at end of file +} // namespace Envoy diff --git a/source/exe/win32/platform_impl.h b/source/exe/win32/platform_impl.h index b69b2f5b1be1b..ffb239dd7ebbc 100644 --- a/source/exe/win32/platform_impl.h +++ b/source/exe/win32/platform_impl.h @@ -2,6 +2,7 @@ #include "common/common/assert.h" #include "common/common/thread_impl.h" +#include "common/filesystem/filesystem_impl.h" // clang-format off #include @@ -21,9 +22,11 @@ class PlatformImpl { ~PlatformImpl() { ::WSACleanup(); } Thread::ThreadFactory& threadFactory() { return thread_factory_; } + Filesystem::Instance& fileSystem() { return file_system_; } private: Thread::ThreadFactoryImplWin32 thread_factory_; + Filesystem::InstanceImplWin32 file_system_; }; -} // namespace Envoy \ No newline at end of file +} // namespace Envoy diff --git a/source/extensions/quic_listeners/quiche/platform/quic_test_output_impl.cc b/source/extensions/quic_listeners/quiche/platform/quic_test_output_impl.cc index 2ab6a1fa66615..8746ffc0eecbc 100644 --- a/source/extensions/quic_listeners/quiche/platform/quic_test_output_impl.cc +++ b/source/extensions/quic_listeners/quiche/platform/quic_test_output_impl.cc @@ -36,7 +36,7 @@ void QuicRecordTestOutputToFile(const std::string& filename, QuicStringPiece dat output_dir += '/'; } - Envoy::Filesystem::InstanceImpl file_system; + Envoy::Filesystem::InstanceImplPosix file_system; if (!file_system.directoryExists(output_dir)) { QUIC_LOG(ERROR) << "Directory does not exist while writing test output: " << output_dir; return; diff --git a/source/server/config_validation/api.cc b/source/server/config_validation/api.cc index f03da1c16a1ca..23954bb561b50 100644 --- a/source/server/config_validation/api.cc +++ b/source/server/config_validation/api.cc @@ -8,8 +8,8 @@ namespace Envoy { namespace Api { ValidationImpl::ValidationImpl(Thread::ThreadFactory& thread_factory, Stats::Store& stats_store, - Event::TimeSystem& time_system) - : Impl(thread_factory, stats_store, time_system), time_system_(time_system) {} + Event::TimeSystem& time_system, Filesystem::Instance& file_system) + : Impl(thread_factory, stats_store, time_system, file_system), time_system_(time_system) {} Event::DispatcherPtr ValidationImpl::allocateDispatcher() { return Event::DispatcherPtr{new Event::ValidationDispatcher(*this, time_system_)}; diff --git a/source/server/config_validation/api.h b/source/server/config_validation/api.h index 46161e14aa894..658e671a8d970 100644 --- a/source/server/config_validation/api.h +++ b/source/server/config_validation/api.h @@ -16,7 +16,7 @@ namespace Api { class ValidationImpl : public Impl { public: ValidationImpl(Thread::ThreadFactory& thread_factory, Stats::Store& stats_store, - Event::TimeSystem& time_system); + Event::TimeSystem& time_system, Filesystem::Instance& file_system); Event::DispatcherPtr allocateDispatcher() override; Event::DispatcherPtr allocateDispatcher(Buffer::WatermarkFactoryPtr&& watermark_factory) override; diff --git a/source/server/config_validation/server.cc b/source/server/config_validation/server.cc index 4cd901bc5df91..8c99b19d9b558 100644 --- a/source/server/config_validation/server.cc +++ b/source/server/config_validation/server.cc @@ -18,14 +18,15 @@ namespace Envoy { namespace Server { bool validateConfig(const Options& options, Network::Address::InstanceConstSharedPtr local_address, - ComponentFactory& component_factory, Thread::ThreadFactory& thread_factory) { + ComponentFactory& component_factory, Thread::ThreadFactory& thread_factory, + Filesystem::Instance& file_system) { Thread::MutexBasicLockable access_log_lock; Stats::IsolatedStoreImpl stats_store; try { Event::RealTimeSystem time_system; ValidationInstance server(options, time_system, local_address, stats_store, access_log_lock, - component_factory, thread_factory); + component_factory, thread_factory, file_system); std::cout << "configuration '" << options.configPath() << "' OK" << std::endl; server.shutdown(); return true; @@ -39,9 +40,10 @@ ValidationInstance::ValidationInstance(const Options& options, Event::TimeSystem Stats::IsolatedStoreImpl& store, Thread::BasicLockable& access_log_lock, ComponentFactory& component_factory, - Thread::ThreadFactory& thread_factory) + Thread::ThreadFactory& thread_factory, + Filesystem::Instance& file_system) : options_(options), stats_store_(store), - api_(new Api::ValidationImpl(thread_factory, store, time_system)), + api_(new Api::ValidationImpl(thread_factory, store, time_system, file_system)), dispatcher_(api_->allocateDispatcher()), singleton_manager_(new Singleton::ManagerImpl(api_->threadFactory().currentThreadId())), access_log_manager_(options.fileFlushIntervalMsec(), *api_, *dispatcher_, access_log_lock, diff --git a/source/server/config_validation/server.h b/source/server/config_validation/server.h index 2369cda8777c5..c3db338e9b2f1 100644 --- a/source/server/config_validation/server.h +++ b/source/server/config_validation/server.h @@ -35,7 +35,8 @@ namespace Server { * the config is valid, false if invalid. */ bool validateConfig(const Options& options, Network::Address::InstanceConstSharedPtr local_address, - ComponentFactory& component_factory, Thread::ThreadFactory& thread_factory); + ComponentFactory& component_factory, Thread::ThreadFactory& thread_factory, + Filesystem::Instance& file_system); /** * ValidationInstance does the bulk of the work for config-validation runs of Envoy. It implements @@ -57,7 +58,8 @@ class ValidationInstance : Logger::Loggable, ValidationInstance(const Options& options, Event::TimeSystem& time_system, Network::Address::InstanceConstSharedPtr local_address, Stats::IsolatedStoreImpl& store, Thread::BasicLockable& access_log_lock, - ComponentFactory& component_factory, Thread::ThreadFactory& thread_factory); + ComponentFactory& component_factory, Thread::ThreadFactory& thread_factory, + Filesystem::Instance& file_system); // Server::Instance Admin& admin() override { return admin_; } diff --git a/source/server/server.cc b/source/server/server.cc index 62e9ae99a902f..6899183a29ff5 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -51,11 +51,12 @@ InstanceImpl::InstanceImpl(const Options& options, Event::TimeSystem& time_syste Thread::BasicLockable& access_log_lock, ComponentFactory& component_factory, Runtime::RandomGeneratorPtr&& random_generator, - ThreadLocal::Instance& tls, Thread::ThreadFactory& thread_factory) + ThreadLocal::Instance& tls, Thread::ThreadFactory& thread_factory, + Filesystem::Instance& file_system) : secret_manager_(std::make_unique()), shutdown_(false), options_(options), time_source_(time_system), restarter_(restarter), start_time_(time(nullptr)), original_start_time_(start_time_), stats_store_(store), - thread_local_(tls), api_(new Api::Impl(thread_factory, store, time_system)), + thread_local_(tls), api_(new Api::Impl(thread_factory, store, time_system, file_system)), dispatcher_(api_->allocateDispatcher()), singleton_manager_(new Singleton::ManagerImpl(api_->threadFactory().currentThreadId())), handler_(new ConnectionHandlerImpl(ENVOY_LOGGER(), *dispatcher_)), diff --git a/source/server/server.h b/source/server/server.h index aa026fb65ce13..ced45edf99ee6 100644 --- a/source/server/server.h +++ b/source/server/server.h @@ -146,7 +146,7 @@ class InstanceImpl : Logger::Loggable, public Instance { HotRestart& restarter, Stats::StoreRoot& store, Thread::BasicLockable& access_log_lock, ComponentFactory& component_factory, Runtime::RandomGeneratorPtr&& random_generator, ThreadLocal::Instance& tls, - Thread::ThreadFactory& thread_factory); + Thread::ThreadFactory& thread_factory, Filesystem::Instance& file_system); ~InstanceImpl() override; diff --git a/test/common/access_log/access_log_manager_impl_test.cc b/test/common/access_log/access_log_manager_impl_test.cc index 1a14cddae2a5e..4681e9453a7cc 100644 --- a/test/common/access_log/access_log_manager_impl_test.cc +++ b/test/common/access_log/access_log_manager_impl_test.cc @@ -1,6 +1,7 @@ #include #include "common/access_log/access_log_manager_impl.h" +#include "common/filesystem/file_shared_impl.h" #include "common/stats/isolated_store_impl.h" #include "test/mocks/access_log/mocks.h" @@ -12,6 +13,7 @@ #include "gtest/gtest.h" using testing::_; +using testing::ByMove; using testing::NiceMock; using testing::Return; using testing::ReturnNew; @@ -47,21 +49,21 @@ class AccessLogManagerImplTest : public testing::Test { TEST_F(AccessLogManagerImplTest, BadFile) { EXPECT_CALL(dispatcher_, createTimer_(_)); - EXPECT_CALL(*file_, open_()).WillOnce(Return(Api::SysCallBoolResult{false, 0})); + EXPECT_CALL(*file_, open_()).WillOnce(Return(ByMove(Filesystem::resultFailure(false, 0)))); EXPECT_THROW(access_log_manager_.createAccessLog("foo"), EnvoyException); } TEST_F(AccessLogManagerImplTest, flushToLogFilePeriodically) { NiceMock* timer = new NiceMock(&dispatcher_); - EXPECT_CALL(*file_, open_()).WillOnce(Return(Api::SysCallBoolResult{true, 0})); + EXPECT_CALL(*file_, open_()).WillOnce(Return(ByMove(Filesystem::resultSuccess(true)))); AccessLogFileSharedPtr log_file = access_log_manager_.createAccessLog("foo"); EXPECT_CALL(*timer, enableTimer(timeout_40ms_)); EXPECT_CALL(*file_, write_(_)) - .WillOnce(Invoke([](absl::string_view data) -> Api::SysCallSizeResult { + .WillOnce(Invoke([](absl::string_view data) -> Api::IoCallSizeResult { EXPECT_EQ(0, data.compare("test")); - return {static_cast(data.length()), 0}; + return Filesystem::resultSuccess(static_cast(data.length())); })); log_file->write("test"); @@ -74,9 +76,9 @@ TEST_F(AccessLogManagerImplTest, flushToLogFilePeriodically) { } EXPECT_CALL(*file_, write_(_)) - .WillOnce(Invoke([](absl::string_view data) -> Api::SysCallSizeResult { + .WillOnce(Invoke([](absl::string_view data) -> Api::IoCallSizeResult { EXPECT_EQ(0, data.compare("test2")); - return {static_cast(data.length()), 0}; + return Filesystem::resultSuccess(static_cast(data.length())); })); // make sure timer is re-enabled on callback call @@ -90,13 +92,13 @@ TEST_F(AccessLogManagerImplTest, flushToLogFilePeriodically) { file_->write_event_.wait(file_->write_mutex_); } } - EXPECT_CALL(*file_, close_()).WillOnce(Return(Api::SysCallBoolResult{true, 0})); + EXPECT_CALL(*file_, close_()).WillOnce(Return(ByMove(Filesystem::resultSuccess(true)))); } TEST_F(AccessLogManagerImplTest, flushToLogFileOnDemand) { NiceMock* timer = new NiceMock(&dispatcher_); - EXPECT_CALL(*file_, open_()).WillOnce(Return(Api::SysCallBoolResult{true, 0})); + EXPECT_CALL(*file_, open_()).WillOnce(Return(ByMove(Filesystem::resultSuccess(true)))); AccessLogFileSharedPtr log_file = access_log_manager_.createAccessLog("foo"); EXPECT_CALL(*timer, enableTimer(timeout_40ms_)); @@ -105,8 +107,8 @@ TEST_F(AccessLogManagerImplTest, flushToLogFileOnDemand) { // immediately (race on whether it will or not). So do a write and flush to // get that state out of the way, then test that small writes don't trigger a flush. EXPECT_CALL(*file_, write_(_)) - .WillOnce(Invoke([](absl::string_view data) -> Api::SysCallSizeResult { - return {static_cast(data.length()), 0}; + .WillOnce(Invoke([](absl::string_view data) -> Api::IoCallSizeResult { + return Filesystem::resultSuccess(static_cast(data.length())); })); log_file->write("prime-it"); log_file->flush(); @@ -117,9 +119,9 @@ TEST_F(AccessLogManagerImplTest, flushToLogFileOnDemand) { } EXPECT_CALL(*file_, write_(_)) - .WillOnce(Invoke([](absl::string_view data) -> Api::SysCallSizeResult { + .WillOnce(Invoke([](absl::string_view data) -> Api::IoCallSizeResult { EXPECT_EQ(0, data.compare("test")); - return {static_cast(data.length()), 0}; + return Filesystem::resultSuccess(static_cast(data.length())); })); log_file->write("test"); @@ -137,9 +139,9 @@ TEST_F(AccessLogManagerImplTest, flushToLogFileOnDemand) { } EXPECT_CALL(*file_, write_(_)) - .WillOnce(Invoke([](absl::string_view data) -> Api::SysCallSizeResult { + .WillOnce(Invoke([](absl::string_view data) -> Api::IoCallSizeResult { EXPECT_EQ(0, data.compare("test2")); - return {static_cast(data.length()), 0}; + return Filesystem::resultSuccess(static_cast(data.length())); })); // make sure timer is re-enabled on callback call @@ -154,21 +156,23 @@ TEST_F(AccessLogManagerImplTest, flushToLogFileOnDemand) { file_->write_event_.wait(file_->write_mutex_); } } - EXPECT_CALL(*file_, close_()).WillOnce(Return(Api::SysCallBoolResult{true, 0})); + EXPECT_CALL(*file_, close_()).WillOnce(Return(ByMove(Filesystem::resultSuccess(true)))); } TEST_F(AccessLogManagerImplTest, reopenFile) { NiceMock* timer = new NiceMock(&dispatcher_); Sequence sq; - EXPECT_CALL(*file_, open_()).InSequence(sq).WillOnce(Return(Api::SysCallBoolResult{true, 0})); + EXPECT_CALL(*file_, open_()) + .InSequence(sq) + .WillOnce(Return(ByMove(Filesystem::resultSuccess(true)))); AccessLogFileSharedPtr log_file = access_log_manager_.createAccessLog("foo"); EXPECT_CALL(*file_, write_(_)) .InSequence(sq) - .WillOnce(Invoke([](absl::string_view data) -> Api::SysCallSizeResult { + .WillOnce(Invoke([](absl::string_view data) -> Api::IoCallSizeResult { EXPECT_EQ(0, data.compare("before")); - return {static_cast(data.length()), 0}; + return Filesystem::resultSuccess(static_cast(data.length())); })); log_file->write("before"); @@ -181,17 +185,23 @@ TEST_F(AccessLogManagerImplTest, reopenFile) { } } - EXPECT_CALL(*file_, close_()).InSequence(sq).WillOnce(Return(Api::SysCallBoolResult{true, 0})); - EXPECT_CALL(*file_, open_()).InSequence(sq).WillOnce(Return(Api::SysCallBoolResult{true, 0})); + EXPECT_CALL(*file_, close_()) + .InSequence(sq) + .WillOnce(Return(ByMove(Filesystem::resultSuccess(true)))); + EXPECT_CALL(*file_, open_()) + .InSequence(sq) + .WillOnce(Return(ByMove(Filesystem::resultSuccess(true)))); EXPECT_CALL(*file_, write_(_)) .InSequence(sq) - .WillOnce(Invoke([](absl::string_view data) -> Api::SysCallSizeResult { + .WillOnce(Invoke([](absl::string_view data) -> Api::IoCallSizeResult { EXPECT_EQ(0, data.compare("reopened")); - return {static_cast(data.length()), 0}; + return Filesystem::resultSuccess(static_cast(data.length())); })); - EXPECT_CALL(*file_, close_()).InSequence(sq).WillOnce(Return(Api::SysCallBoolResult{true, 0})); + EXPECT_CALL(*file_, close_()) + .InSequence(sq) + .WillOnce(Return(ByMove(Filesystem::resultSuccess(true)))); log_file->reopen(); log_file->write("reopened"); @@ -209,16 +219,22 @@ TEST_F(AccessLogManagerImplTest, reopenThrows) { NiceMock* timer = new NiceMock(&dispatcher_); EXPECT_CALL(*file_, write_(_)) - .WillRepeatedly(Invoke([](absl::string_view data) -> Api::SysCallSizeResult { - return {static_cast(data.length()), 0}; + .WillRepeatedly(Invoke([](absl::string_view data) -> Api::IoCallSizeResult { + return Filesystem::resultSuccess(static_cast(data.length())); })); Sequence sq; - EXPECT_CALL(*file_, open_()).InSequence(sq).WillOnce(Return(Api::SysCallBoolResult{true, 0})); + EXPECT_CALL(*file_, open_()) + .InSequence(sq) + .WillOnce(Return(ByMove(Filesystem::resultSuccess(true)))); AccessLogFileSharedPtr log_file = access_log_manager_.createAccessLog("foo"); - EXPECT_CALL(*file_, close_()).InSequence(sq).WillOnce(Return(Api::SysCallBoolResult{true, 0})); - EXPECT_CALL(*file_, open_()).InSequence(sq).WillOnce(Return(Api::SysCallBoolResult{false, 0})); + EXPECT_CALL(*file_, close_()) + .InSequence(sq) + .WillOnce(Return(ByMove(Filesystem::resultSuccess(true)))); + EXPECT_CALL(*file_, open_()) + .InSequence(sq) + .WillOnce(Return(ByMove(Filesystem::resultFailure(false, 0)))); log_file->write("test write"); timer->callback_(); @@ -246,13 +262,13 @@ TEST_F(AccessLogManagerImplTest, reopenThrows) { } TEST_F(AccessLogManagerImplTest, bigDataChunkShouldBeFlushedWithoutTimer) { - EXPECT_CALL(*file_, open_()).WillOnce(Return(Api::SysCallBoolResult{true, 0})); + EXPECT_CALL(*file_, open_()).WillOnce(Return(ByMove(Filesystem::resultSuccess(true)))); AccessLogFileSharedPtr log_file = access_log_manager_.createAccessLog("foo"); EXPECT_CALL(*file_, write_(_)) - .WillOnce(Invoke([](absl::string_view data) -> Api::SysCallSizeResult { + .WillOnce(Invoke([](absl::string_view data) -> Api::IoCallSizeResult { EXPECT_EQ(0, data.compare("a")); - return {static_cast(data.length()), 0}; + return Filesystem::resultSuccess(static_cast(data.length())); })); log_file->write("a"); @@ -267,10 +283,10 @@ TEST_F(AccessLogManagerImplTest, bigDataChunkShouldBeFlushedWithoutTimer) { // First write happens without waiting on thread_flush_. Now make a big string and it should be // flushed even when timer is not enabled EXPECT_CALL(*file_, write_(_)) - .WillOnce(Invoke([](absl::string_view data) -> Api::SysCallSizeResult { + .WillOnce(Invoke([](absl::string_view data) -> Api::IoCallSizeResult { std::string expected(1024 * 64 + 1, 'b'); EXPECT_EQ(0, data.compare(expected)); - return {static_cast(data.length()), 0}; + return Filesystem::resultSuccess(static_cast(data.length())); })); std::string big_string(1024 * 64 + 1, 'b'); @@ -282,39 +298,57 @@ TEST_F(AccessLogManagerImplTest, bigDataChunkShouldBeFlushedWithoutTimer) { file_->write_event_.wait(file_->write_mutex_); } } - EXPECT_CALL(*file_, close_()).WillOnce(Return(Api::SysCallBoolResult{true, 0})); + EXPECT_CALL(*file_, close_()).WillOnce(Return(ByMove(Filesystem::resultSuccess(true)))); } TEST_F(AccessLogManagerImplTest, reopenAllFiles) { EXPECT_CALL(dispatcher_, createTimer_(_)).WillRepeatedly(ReturnNew>()); - EXPECT_CALL(*file_, open_()).Times(2).WillRepeatedly(Return(Api::SysCallBoolResult{true, 0})); + Sequence sq; + EXPECT_CALL(*file_, open_()) + .InSequence(sq) + .WillOnce(Return(ByMove(Filesystem::resultSuccess(true)))); AccessLogFileSharedPtr log = access_log_manager_.createAccessLog("foo"); NiceMock* file2 = new NiceMock; EXPECT_CALL(file_system_, createFile("bar")) .WillOnce(Return(ByMove(std::unique_ptr>(file2)))); - EXPECT_CALL(*file2, open_()).Times(2).WillRepeatedly(Return(Api::SysCallBoolResult{true, 0})); + + Sequence sq2; + EXPECT_CALL(*file2, open_()) + .InSequence(sq2) + .WillOnce(Return(ByMove(Filesystem::resultSuccess(true)))); AccessLogFileSharedPtr log2 = access_log_manager_.createAccessLog("bar"); // Make sure that getting the access log with the same name returns the same underlying file. EXPECT_EQ(log, access_log_manager_.createAccessLog("foo")); EXPECT_EQ(log2, access_log_manager_.createAccessLog("bar")); - EXPECT_CALL(*file_, close_()).Times(2).WillRepeatedly(Return(Api::SysCallBoolResult{true, 0})); - EXPECT_CALL(*file2, close_()).Times(2).WillRepeatedly(Return(Api::SysCallBoolResult{true, 0})); - // Test that reopen reopens all of the files EXPECT_CALL(*file_, write_(_)) - .WillRepeatedly(Invoke([](absl::string_view data) -> Api::SysCallSizeResult { - return {static_cast(data.length()), 0}; + .WillRepeatedly(Invoke([](absl::string_view data) -> Api::IoCallSizeResult { + return Filesystem::resultSuccess(static_cast(data.length())); })); EXPECT_CALL(*file2, write_(_)) - .WillRepeatedly(Invoke([](absl::string_view data) -> Api::SysCallSizeResult { - return {static_cast(data.length()), 0}; + .WillRepeatedly(Invoke([](absl::string_view data) -> Api::IoCallSizeResult { + return Filesystem::resultSuccess(static_cast(data.length())); })); + EXPECT_CALL(*file_, close_()) + .InSequence(sq) + .WillOnce(Return(ByMove(Filesystem::resultSuccess(true)))); + EXPECT_CALL(*file2, close_()) + .InSequence(sq2) + .WillOnce(Return(ByMove(Filesystem::resultSuccess(true)))); + + EXPECT_CALL(*file_, open_()) + .InSequence(sq) + .WillOnce(Return(ByMove(Filesystem::resultSuccess(true)))); + EXPECT_CALL(*file2, open_()) + .InSequence(sq2) + .WillOnce(Return(ByMove(Filesystem::resultSuccess(true)))); + access_log_manager_.reopen(); log->write("this is to force reopen"); @@ -333,6 +367,13 @@ TEST_F(AccessLogManagerImplTest, reopenAllFiles) { file2->open_event_.wait(file2->open_mutex_); } } + + EXPECT_CALL(*file_, close_()) + .InSequence(sq) + .WillOnce(Return(ByMove(Filesystem::resultSuccess(true)))); + EXPECT_CALL(*file2, close_()) + .InSequence(sq2) + .WillOnce(Return(ByMove(Filesystem::resultSuccess(true)))); } } // namespace diff --git a/test/common/filesystem/filesystem_impl_test.cc b/test/common/filesystem/filesystem_impl_test.cc index cc7bb2cdff8e5..47a657d220a80 100644 --- a/test/common/filesystem/filesystem_impl_test.cc +++ b/test/common/filesystem/filesystem_impl_test.cc @@ -15,27 +15,54 @@ namespace Filesystem { class FileSystemImplTest : public testing::Test { protected: int getFd(File* file) { - auto file_impl = dynamic_cast(file); +#ifdef WIN32 + auto file_impl = dynamic_cast(file); +#else + auto file_impl = dynamic_cast(file); +#endif RELEASE_ASSERT(file_impl != nullptr, "failed to cast File* to FileImpl*"); return file_impl->fd_; } - - InstanceImpl file_system_; +#ifdef WIN32 + InstanceImplWin32 file_system_; +#else + Api::SysCallStringResult canonicalPath(const std::string& path) { + return file_system_.canonicalPath(path); + } + InstanceImplPosix file_system_; +#endif }; TEST_F(FileSystemImplTest, fileExists) { - EXPECT_TRUE(file_system_.fileExists("/dev/null")); EXPECT_FALSE(file_system_.fileExists("/dev/blahblahblah")); +#ifdef WIN32 + const std::string file_path = TestEnvironment::writeStringToFileForTest("test_envoy", "x"); + EXPECT_TRUE(file_system_.fileExists(file_path)); + EXPECT_TRUE(file_system_.fileExists("c:/windows")); +#else + EXPECT_TRUE(file_system_.fileExists("/dev/null")); + EXPECT_TRUE(file_system_.fileExists("/dev")); +#endif } TEST_F(FileSystemImplTest, directoryExists) { - EXPECT_TRUE(file_system_.directoryExists("/dev")); - EXPECT_FALSE(file_system_.directoryExists("/dev/null")); EXPECT_FALSE(file_system_.directoryExists("/dev/blahblah")); +#ifdef WIN32 + const std::string file_path = TestEnvironment::writeStringToFileForTest("test_envoy", "x"); + EXPECT_FALSE(file_system_.directoryExists(file_path)); + EXPECT_TRUE(file_system_.directoryExists("c:/windows")); +#else + EXPECT_FALSE(file_system_.directoryExists("/dev/null")); + EXPECT_TRUE(file_system_.directoryExists("/dev")); +#endif } TEST_F(FileSystemImplTest, fileSize) { +#ifdef WIN32 + EXPECT_EQ(0, file_system_.fileSize("NUL")); +#else EXPECT_EQ(0, file_system_.fileSize("/dev/null")); +#endif EXPECT_EQ(-1, file_system_.fileSize("/dev/blahblahblah")); const std::string data = "test string\ntest"; const std::string file_path = TestEnvironment::writeStringToFileForTest("test_envoy", data); @@ -78,18 +105,30 @@ TEST_F(FileSystemImplTest, fileReadToEndBlacklisted) { EXPECT_THROW(file_system_.fileReadToEnd("/sys/block/sda/dev"), EnvoyException); } -TEST_F(FileSystemImplTest, CanonicalPathSuccess) { - EXPECT_EQ("/", file_system_.canonicalPath("//").rc_); -} +#ifndef WIN32 +TEST_F(FileSystemImplTest, CanonicalPathSuccess) { EXPECT_EQ("/", canonicalPath("//").rc_); } +#endif +#ifndef WIN32 TEST_F(FileSystemImplTest, CanonicalPathFail) { - const Api::SysCallStringResult result = file_system_.canonicalPath("/_some_non_existent_file"); + const Api::SysCallStringResult result = canonicalPath("/_some_non_existent_file"); EXPECT_TRUE(result.rc_.empty()); EXPECT_STREQ("No such file or directory", ::strerror(result.errno_)); } +#endif TEST_F(FileSystemImplTest, IllegalPath) { EXPECT_FALSE(file_system_.illegalPath("/")); + EXPECT_FALSE(file_system_.illegalPath("//")); +#ifdef WIN32 + EXPECT_FALSE(file_system_.illegalPath("/dev")); + EXPECT_FALSE(file_system_.illegalPath("/dev/")); + EXPECT_FALSE(file_system_.illegalPath("/proc")); + EXPECT_FALSE(file_system_.illegalPath("/proc/")); + EXPECT_FALSE(file_system_.illegalPath("/sys")); + EXPECT_FALSE(file_system_.illegalPath("/sys/")); + EXPECT_FALSE(file_system_.illegalPath("/_some_non_existent_file")); +#else EXPECT_TRUE(file_system_.illegalPath("/dev")); EXPECT_TRUE(file_system_.illegalPath("/dev/")); EXPECT_TRUE(file_system_.illegalPath("/proc")); @@ -97,6 +136,7 @@ TEST_F(FileSystemImplTest, IllegalPath) { EXPECT_TRUE(file_system_.illegalPath("/sys")); EXPECT_TRUE(file_system_.illegalPath("/sys/")); EXPECT_TRUE(file_system_.illegalPath("/_some_non_existent_file")); +#endif } TEST_F(FileSystemImplTest, ConstructedFileNotOpen) { @@ -112,7 +152,7 @@ TEST_F(FileSystemImplTest, Open) { ::unlink(new_file_path.c_str()); FilePtr file = file_system_.createFile(new_file_path); - const Api::SysCallBoolResult result = file->open(); + const Api::IoCallBoolResult result = file->open(); EXPECT_TRUE(result.rc_); EXPECT_TRUE(file->isOpen()); } @@ -124,21 +164,21 @@ TEST_F(FileSystemImplTest, OpenTwice) { FilePtr file = file_system_.createFile(new_file_path); EXPECT_EQ(getFd(file.get()), -1); - Api::SysCallBoolResult result = file->open(); + const Api::IoCallBoolResult result1 = file->open(); const int initial_fd = getFd(file.get()); - EXPECT_TRUE(result.rc_); + EXPECT_TRUE(result1.rc_); EXPECT_TRUE(file->isOpen()); // check that we don't leak a file descriptor - result = file->open(); + const Api::IoCallBoolResult result2 = file->open(); EXPECT_EQ(initial_fd, getFd(file.get())); - EXPECT_TRUE(result.rc_); + EXPECT_TRUE(result2.rc_); EXPECT_TRUE(file->isOpen()); } TEST_F(FileSystemImplTest, OpenBadFilePath) { FilePtr file = file_system_.createFile(""); - const Api::SysCallBoolResult result = file->open(); + const Api::IoCallBoolResult result = file->open(); EXPECT_FALSE(result.rc_); } @@ -148,10 +188,10 @@ TEST_F(FileSystemImplTest, ExistingFile) { { FilePtr file = file_system_.createFile(file_path); - const Api::SysCallBoolResult open_result = file->open(); + const Api::IoCallBoolResult open_result = file->open(); EXPECT_TRUE(open_result.rc_); std::string data(" new data"); - const Api::SysCallSizeResult result = file->write(data); + const Api::IoCallSizeResult result = file->write(data); EXPECT_EQ(data.length(), result.rc_); } @@ -165,10 +205,10 @@ TEST_F(FileSystemImplTest, NonExistingFile) { { FilePtr file = file_system_.createFile(new_file_path); - const Api::SysCallBoolResult open_result = file->open(); + const Api::IoCallBoolResult open_result = file->open(); EXPECT_TRUE(open_result.rc_); std::string data(" new data"); - const Api::SysCallSizeResult result = file->write(data); + const Api::IoCallSizeResult result = file->write(data); EXPECT_EQ(data.length(), result.rc_); } @@ -181,12 +221,12 @@ TEST_F(FileSystemImplTest, Close) { ::unlink(new_file_path.c_str()); FilePtr file = file_system_.createFile(new_file_path); - Api::SysCallBoolResult result = file->open(); - EXPECT_TRUE(result.rc_); + const Api::IoCallBoolResult result1 = file->open(); + EXPECT_TRUE(result1.rc_); EXPECT_TRUE(file->isOpen()); - result = file->close(); - EXPECT_TRUE(result.rc_); + const Api::IoCallBoolResult result2 = file->close(); + EXPECT_TRUE(result2.rc_); EXPECT_FALSE(file->isOpen()); } @@ -195,13 +235,14 @@ TEST_F(FileSystemImplTest, WriteAfterClose) { ::unlink(new_file_path.c_str()); FilePtr file = file_system_.createFile(new_file_path); - Api::SysCallBoolResult bool_result = file->open(); - EXPECT_TRUE(bool_result.rc_); - bool_result = file->close(); - EXPECT_TRUE(bool_result.rc_); - const Api::SysCallSizeResult result = file->write(" new data"); - EXPECT_EQ(-1, result.rc_); - EXPECT_EQ(EBADF, result.errno_); + const Api::IoCallBoolResult bool_result1 = file->open(); + EXPECT_TRUE(bool_result1.rc_); + const Api::IoCallBoolResult bool_result2 = file->close(); + EXPECT_TRUE(bool_result2.rc_); + const Api::IoCallSizeResult size_result = file->write(" new data"); + EXPECT_EQ(-1, size_result.rc_); + EXPECT_EQ(IoFileError::IoErrorCode::UnknownError, size_result.err_->getErrorCode()); + EXPECT_EQ("Bad file descriptor", size_result.err_->getErrorDetails()); } } // namespace Filesystem diff --git a/test/integration/server.cc b/test/integration/server.cc index 8c1f88bd323b0..bf54551b2b34e 100644 --- a/test/integration/server.cc +++ b/test/integration/server.cc @@ -165,7 +165,7 @@ void IntegrationTestServerImpl::createAndRunEnvoyServer( Server::InstanceImpl server(options, time_system, local_address, hooks, restarter, stat_store, access_log_lock, component_factory, std::move(random_generator), tls, - Thread::threadFactoryForTest()); + Thread::threadFactoryForTest(), Filesystem::fileSystemForTest()); // This is technically thread unsafe (assigning to a shared_ptr accessed // across threads), but because we synchronize below through serverReady(), the only // consumer on the main test thread in ~IntegrationTestServerImpl will not race. diff --git a/test/integration/utility.cc b/test/integration/utility.cc index e565594453017..a5cbc2c6c83ed 100644 --- a/test/integration/utility.cc +++ b/test/integration/utility.cc @@ -64,7 +64,8 @@ IntegrationUtil::makeSingleRequest(const Network::Address::InstanceConstSharedPt NiceMock mock_stats_store; Event::GlobalTimeSystem time_system; - Api::Impl api(Thread::threadFactoryForTest(), mock_stats_store, time_system); + Api::Impl api(Thread::threadFactoryForTest(), mock_stats_store, time_system, + Filesystem::fileSystemForTest()); Event::DispatcherPtr dispatcher(api.allocateDispatcher()); std::shared_ptr cluster{new NiceMock()}; Upstream::HostDescriptionConstSharedPtr host_description{ diff --git a/test/mocks/filesystem/mocks.cc b/test/mocks/filesystem/mocks.cc index 5fe62371c2058..5e585b5c8816f 100644 --- a/test/mocks/filesystem/mocks.cc +++ b/test/mocks/filesystem/mocks.cc @@ -1,5 +1,6 @@ #include "test/mocks/filesystem/mocks.h" +#include "common/common/assert.h" #include "common/common/lock_guard.h" namespace Envoy { @@ -8,10 +9,10 @@ namespace Filesystem { MockFile::MockFile() : num_opens_(0), num_writes_(0), is_open_(false) {} MockFile::~MockFile() {} -Api::SysCallBoolResult MockFile::open() { +Api::IoCallBoolResult MockFile::open() { Thread::LockGuard lock(open_mutex_); - const Api::SysCallBoolResult result = open_(); + Api::IoCallBoolResult result = open_(); is_open_ = result.rc_; num_opens_++; open_event_.notifyOne(); @@ -19,21 +20,21 @@ Api::SysCallBoolResult MockFile::open() { return result; } -Api::SysCallSizeResult MockFile::write(absl::string_view buffer) { +Api::IoCallSizeResult MockFile::write(absl::string_view buffer) { Thread::LockGuard lock(write_mutex_); if (!is_open_) { - return {-1, EBADF}; + return {-1, Api::IoErrorPtr(nullptr, [](Api::IoError*) { NOT_REACHED_GCOVR_EXCL_LINE; })}; } - const Api::SysCallSizeResult result = write_(buffer); + Api::IoCallSizeResult result = write_(buffer); num_writes_++; write_event_.notifyOne(); return result; } -Api::SysCallBoolResult MockFile::close() { - const Api::SysCallBoolResult result = close_(); +Api::IoCallBoolResult MockFile::close() { + Api::IoCallBoolResult result = close_(); is_open_ = !result.rc_; return result; diff --git a/test/mocks/filesystem/mocks.h b/test/mocks/filesystem/mocks.h index 821cf2b3c17f8..e1cd62f79b8e1 100644 --- a/test/mocks/filesystem/mocks.h +++ b/test/mocks/filesystem/mocks.h @@ -19,16 +19,15 @@ class MockFile : public File { ~MockFile(); // Filesystem::File - Api::SysCallBoolResult open() override; - Api::SysCallSizeResult write(absl::string_view buffer) override; - Api::SysCallBoolResult close() override; - bool isOpen() override { return is_open_; }; - MOCK_METHOD0(path, std::string()); - MOCK_METHOD1(errorToString, std::string(int)); + Api::IoCallBoolResult open() override; + Api::IoCallSizeResult write(absl::string_view buffer) override; + Api::IoCallBoolResult close() override; + bool isOpen() const override { return is_open_; }; + MOCK_CONST_METHOD0(path, std::string()); - MOCK_METHOD0(open_, Api::SysCallBoolResult()); - MOCK_METHOD1(write_, Api::SysCallSizeResult(absl::string_view buffer)); - MOCK_METHOD0(close_, Api::SysCallBoolResult()); + MOCK_METHOD0(open_, Api::IoCallBoolResult()); + MOCK_METHOD1(write_, Api::IoCallSizeResult(absl::string_view buffer)); + MOCK_METHOD0(close_, Api::IoCallBoolResult()); size_t num_opens_; size_t num_writes_; @@ -52,7 +51,6 @@ class MockInstance : public Instance { MOCK_METHOD1(directoryExists, bool(const std::string&)); MOCK_METHOD1(fileSize, ssize_t(const std::string&)); MOCK_METHOD1(fileReadToEnd, std::string(const std::string&)); - MOCK_METHOD1(canonicalPath, Api::SysCallStringResult(const std::string&)); MOCK_METHOD1(illegalPath, bool(const std::string&)); }; diff --git a/test/server/config_validation/config_fuzz_test.cc b/test/server/config_validation/config_fuzz_test.cc index 65f58fcfe81e0..c12dbef8c4cb9 100644 --- a/test/server/config_validation/config_fuzz_test.cc +++ b/test/server/config_validation/config_fuzz_test.cc @@ -32,7 +32,7 @@ DEFINE_PROTO_FUZZER(const envoy::config::bootstrap::v2::Bootstrap& input) { try { validateConfig(options, Network::Address::InstanceConstSharedPtr(), component_factory, - Thread::threadFactoryForTest()); + Thread::threadFactoryForTest(), Filesystem::fileSystemForTest()); } catch (const EnvoyException& ex) { ENVOY_LOG_MISC(debug, "Controlled EnvoyException exit: {}", ex.what()); } diff --git a/test/server/config_validation/dispatcher_test.cc b/test/server/config_validation/dispatcher_test.cc index e1c78c8b1dec3..c895ec4fa82ce 100644 --- a/test/server/config_validation/dispatcher_test.cc +++ b/test/server/config_validation/dispatcher_test.cc @@ -25,7 +25,8 @@ class ConfigValidation : public testing::TestWithParam(Thread::threadFactoryForTest(), - stats_store_, test_time_.timeSystem()); + stats_store_, test_time_.timeSystem(), + Filesystem::fileSystemForTest()); dispatcher_ = validation_->allocateDispatcher(); } diff --git a/test/server/config_validation/server_test.cc b/test/server/config_validation/server_test.cc index 6c10d1c113489..b6e8c78e4003f 100644 --- a/test/server/config_validation/server_test.cc +++ b/test/server/config_validation/server_test.cc @@ -51,7 +51,8 @@ class ValidationServerTest_1 : public ValidationServerTest { TEST_P(ValidationServerTest, Validate) { EXPECT_TRUE(validateConfig(options_, Network::Address::InstanceConstSharedPtr(), - component_factory_, Thread::threadFactoryForTest())); + component_factory_, Thread::threadFactoryForTest(), + Filesystem::fileSystemForTest())); } // TODO(rlazarus): We'd like use this setup to replace //test/config_test (that is, run it against @@ -68,7 +69,7 @@ INSTANTIATE_TEST_SUITE_P(ValidConfigs, ValidationServerTest, // may not be successful, but there should be no crash. TEST_P(ValidationServerTest_1, RunWithoutCrash) { validateConfig(options_, Network::Address::InstanceConstSharedPtr(), component_factory_, - Thread::threadFactoryForTest()); + Thread::threadFactoryForTest(), Filesystem::fileSystemForTest()); SUCCEED(); } diff --git a/test/server/server_fuzz_test.cc b/test/server/server_fuzz_test.cc index bbb3411c1c640..ff9432bc60ec3 100644 --- a/test/server/server_fuzz_test.cc +++ b/test/server/server_fuzz_test.cc @@ -79,7 +79,7 @@ DEFINE_PROTO_FUZZER(const envoy::config::bootstrap::v2::Bootstrap& input) { options, test_time.timeSystem(), std::make_shared("127.0.0.1"), hooks, restart, stats_store, fakelock, component_factory, std::make_unique(), - thread_local_instance, Thread::threadFactoryForTest()); + thread_local_instance, Thread::threadFactoryForTest(), Filesystem::fileSystemForTest()); } catch (const EnvoyException& ex) { ENVOY_LOG_MISC(debug, "Controlled EnvoyException exit: {}", ex.what()); return; diff --git a/test/server/server_test.cc b/test/server/server_test.cc index d9b8c54284d00..27500a007abe7 100644 --- a/test/server/server_test.cc +++ b/test/server/server_test.cc @@ -131,7 +131,7 @@ class ServerInstanceImplTest : public testing::TestWithParam>(), thread_local_, - Thread::threadFactoryForTest()); + Thread::threadFactoryForTest(), Filesystem::fileSystemForTest()); EXPECT_TRUE(server_->api().fileSystem().fileExists("/dev/null")); } @@ -148,7 +148,7 @@ class ServerInstanceImplTest : public testing::TestWithParam>(), thread_local_, - Thread::threadFactoryForTest()); + Thread::threadFactoryForTest(), Filesystem::fileSystemForTest()); EXPECT_TRUE(server_->api().fileSystem().fileExists("/dev/null")); } @@ -342,7 +342,7 @@ TEST_P(ServerInstanceImplTest, NoOptionsPassed) { Network::Address::InstanceConstSharedPtr(new Network::Address::Ipv4Instance("127.0.0.1")), hooks_, restart_, stats_store_, fakelock_, component_factory_, std::make_unique>(), thread_local_, - Thread::threadFactoryForTest())), + Thread::threadFactoryForTest(), Filesystem::fileSystemForTest())), EnvoyException, "At least one of --config-path and --config-yaml should be non-empty"); } diff --git a/test/test_common/BUILD b/test/test_common/BUILD index b6be6366c7eed..ff4995e825aa9 100644 --- a/test/test_common/BUILD +++ b/test/test_common/BUILD @@ -102,6 +102,7 @@ envoy_cc_test_library( "//source/common/common:utility_lib", "//source/common/config:bootstrap_json_lib", "//source/common/filesystem:directory_lib", + "//source/common/filesystem:filesystem_lib", "//source/common/http:header_map_lib", "//source/common/json:json_loader_lib", "//source/common/network:address_lib", diff --git a/test/test_common/utility.cc b/test/test_common/utility.cc index ef145bda07111..540ead1b7efc1 100644 --- a/test/test_common/utility.cc +++ b/test/test_common/utility.cc @@ -36,6 +36,7 @@ #include "common/network/utility.h" #include "common/stats/stats_options_impl.h" #include "common/filesystem/directory.h" +#include "common/filesystem/filesystem_impl.h" #include "test/test_common/printers.h" #include "test/test_common/test_time.h" @@ -398,6 +399,21 @@ ThreadFactory& threadFactoryForTest() { } // namespace Thread +namespace Filesystem { + +// TODO(sesmith177) Tests should get the Filesystem::Instance from the same location as the main +// code +Instance& fileSystemForTest() { +#ifdef WIN32 + static InstanceImplWin32* file_system = new InstanceImplWin32(); +#else + static InstanceImplPosix* file_system = new InstanceImplPosix(); +#endif + return *file_system; +} + +} // namespace Filesystem + namespace Api { class TestImplProvider { @@ -408,26 +424,34 @@ class TestImplProvider { class TestImpl : public TestImplProvider, public Impl { public: - TestImpl(Thread::ThreadFactory& thread_factory, Stats::Store& stats_store) - : Impl(thread_factory, stats_store, global_time_system_) {} - TestImpl(Thread::ThreadFactory& thread_factory, Event::TimeSystem& time_system) - : Impl(thread_factory, default_stats_store_, time_system) {} - TestImpl(Thread::ThreadFactory& thread_factory) - : Impl(thread_factory, default_stats_store_, global_time_system_) {} + TestImpl(Thread::ThreadFactory& thread_factory, Stats::Store& stats_store, + Filesystem::Instance& file_system) + : Impl(thread_factory, stats_store, global_time_system_, file_system) {} + TestImpl(Thread::ThreadFactory& thread_factory, Event::TimeSystem& time_system, + Filesystem::Instance& file_system) + : Impl(thread_factory, default_stats_store_, time_system, file_system) {} + TestImpl(Thread::ThreadFactory& thread_factory, Filesystem::Instance& file_system) + : Impl(thread_factory, default_stats_store_, global_time_system_, file_system) {} }; -ApiPtr createApiForTest() { return std::make_unique(Thread::threadFactoryForTest()); } +ApiPtr createApiForTest() { + return std::make_unique(Thread::threadFactoryForTest(), + Filesystem::fileSystemForTest()); +} ApiPtr createApiForTest(Stats::Store& stat_store) { - return std::make_unique(Thread::threadFactoryForTest(), stat_store); + return std::make_unique(Thread::threadFactoryForTest(), stat_store, + Filesystem::fileSystemForTest()); } ApiPtr createApiForTest(Event::TimeSystem& time_system) { - return std::make_unique(Thread::threadFactoryForTest(), time_system); + return std::make_unique(Thread::threadFactoryForTest(), time_system, + Filesystem::fileSystemForTest()); } ApiPtr createApiForTest(Stats::Store& stat_store, Event::TimeSystem& time_system) { - return std::make_unique(Thread::threadFactoryForTest(), stat_store, time_system); + return std::make_unique(Thread::threadFactoryForTest(), stat_store, time_system, + Filesystem::fileSystemForTest()); } } // namespace Api diff --git a/test/test_common/utility.h b/test/test_common/utility.h index 0c4a251918878..af3cfc646bbfb 100644 --- a/test/test_common/utility.h +++ b/test/test_common/utility.h @@ -512,6 +512,10 @@ namespace Thread { ThreadFactory& threadFactoryForTest(); } // namespace Thread +namespace Filesystem { +Instance& fileSystemForTest(); +} // namespace Filesystem + namespace Api { ApiPtr createApiForTest(); ApiPtr createApiForTest(Stats::Store& stat_store); diff --git a/tools/bootstrap2pb.cc b/tools/bootstrap2pb.cc index 6ca6eee145d5e..09e0e6f4f55bd 100644 --- a/tools/bootstrap2pb.cc +++ b/tools/bootstrap2pb.cc @@ -30,7 +30,8 @@ int main(int argc, char** argv) { Envoy::PlatformImpl platform_impl_; Envoy::Stats::IsolatedStoreImpl stats_store; Envoy::Event::RealTimeSystem time_system; // NO_CHECK_FORMAT(real_time) - Envoy::Api::Impl api(platform_impl_.threadFactory(), stats_store, time_system); + Envoy::Api::Impl api(platform_impl_.threadFactory(), stats_store, time_system, + platform_impl_.fileSystem()); envoy::config::bootstrap::v2::Bootstrap bootstrap; Envoy::MessageUtil::loadFromFile(argv[1], bootstrap, api); diff --git a/tools/v1_to_bootstrap.cc b/tools/v1_to_bootstrap.cc index 76a2cecaf4e8f..a4d261f5a7009 100644 --- a/tools/v1_to_bootstrap.cc +++ b/tools/v1_to_bootstrap.cc @@ -30,7 +30,8 @@ int main(int argc, char** argv) { Envoy::PlatformImpl platform_impl_; Envoy::Stats::IsolatedStoreImpl stats_store; Envoy::Event::RealTimeSystem time_system; // NO_CHECK_FORMAT(real_time) - Envoy::Api::Impl api(platform_impl_.threadFactory(), stats_store, time_system); + Envoy::Api::Impl api(platform_impl_.threadFactory(), stats_store, time_system, + platform_impl_.fileSystem()); envoy::config::bootstrap::v2::Bootstrap bootstrap; auto config_json = Envoy::Json::Factory::loadFromFile(argv[1], api);