From d822e8afe37aacaf44a10156f753a0c750aa3cd3 Mon Sep 17 00:00:00 2001 From: alyssawilk Date: Thu, 5 Dec 2024 11:07:26 -0500 Subject: [PATCH] watched: removing exceptions (#37461) Risk Level: low Testing: updated tests Docs Changes: n/a Release Notes: n/a https://github.com/envoyproxy/envoy-mobile/issues/176 Signed-off-by: Alyssa Wilk --- source/common/config/watched_directory.cc | 19 +++++++++++++++---- source/common/config/watched_directory.h | 10 +++++++--- source/common/secret/sds_api.cc | 11 +++++++---- .../filesystem_subscription_impl.cc | 5 +++-- test/common/config/watched_directory_test.cc | 4 ++-- tools/code_format/config.yaml | 1 - 6 files changed, 34 insertions(+), 16 deletions(-) diff --git a/source/common/config/watched_directory.cc b/source/common/config/watched_directory.cc index 1340d5e789da..bb2483b3b058 100644 --- a/source/common/config/watched_directory.cc +++ b/source/common/config/watched_directory.cc @@ -3,12 +3,23 @@ namespace Envoy { namespace Config { +absl::StatusOr> +WatchedDirectory::create(const envoy::config::core::v3::WatchedDirectory& config, + Event::Dispatcher& dispatcher) { + absl::Status creation_status = absl::OkStatus(); + auto ret = + std::unique_ptr(new WatchedDirectory(config, dispatcher, creation_status)); + RETURN_IF_NOT_OK_REF(creation_status); + return ret; +} + WatchedDirectory::WatchedDirectory(const envoy::config::core::v3::WatchedDirectory& config, - Event::Dispatcher& dispatcher) { + Event::Dispatcher& dispatcher, absl::Status& creation_status) { watcher_ = dispatcher.createFilesystemWatcher(); - THROW_IF_NOT_OK(watcher_->addWatch(absl::StrCat(config.path(), "/"), - Filesystem::Watcher::Events::MovedTo, - [this](uint32_t) { return cb_(); })); + SET_AND_RETURN_IF_NOT_OK(watcher_->addWatch(absl::StrCat(config.path(), "/"), + Filesystem::Watcher::Events::MovedTo, + [this](uint32_t) { return cb_(); }), + creation_status); } } // namespace Config diff --git a/source/common/config/watched_directory.h b/source/common/config/watched_directory.h index 4b9a4239bf5f..b465426b2f56 100644 --- a/source/common/config/watched_directory.h +++ b/source/common/config/watched_directory.h @@ -10,13 +10,17 @@ namespace Config { // Implement the common functionality of envoy::config::core::v3::WatchedDirectory. class WatchedDirectory { public: - using Callback = std::function; + static absl::StatusOr> + create(const envoy::config::core::v3::WatchedDirectory& config, Event::Dispatcher& dispatcher); - WatchedDirectory(const envoy::config::core::v3::WatchedDirectory& config, - Event::Dispatcher& dispatcher); + using Callback = std::function; void setCallback(Callback cb) { cb_ = cb; } +protected: + WatchedDirectory(const envoy::config::core::v3::WatchedDirectory& config, + Event::Dispatcher& dispatcher, absl::Status& creation_status); + private: std::unique_ptr watcher_; Callback cb_; diff --git a/source/common/secret/sds_api.cc b/source/common/secret/sds_api.cc index af57547af7c2..6d1d053ecc8a 100644 --- a/source/common/secret/sds_api.cc +++ b/source/common/secret/sds_api.cc @@ -270,8 +270,9 @@ void TlsCertificateSdsApi::setSecret( secret.tls_certificate()); resolved_tls_certificate_secrets_ = nullptr; if (secret.tls_certificate().has_watched_directory()) { - watched_directory_ = std::make_unique( - secret.tls_certificate().watched_directory(), dispatcher_); + watched_directory_ = THROW_OR_RETURN_VALUE( + Config::WatchedDirectory::create(secret.tls_certificate().watched_directory(), dispatcher_), + std::unique_ptr); } else { watched_directory_.reset(); } @@ -324,8 +325,10 @@ void CertificateValidationContextSdsApi::setSecret( secret.validation_context()); resolved_certificate_validation_context_secrets_ = nullptr; if (secret.validation_context().has_watched_directory()) { - watched_directory_ = std::make_unique( - secret.validation_context().watched_directory(), dispatcher_); + watched_directory_ = + THROW_OR_RETURN_VALUE(Config::WatchedDirectory::create( + secret.validation_context().watched_directory(), dispatcher_), + std::unique_ptr); } else { watched_directory_.reset(); } diff --git a/source/extensions/config_subscription/filesystem/filesystem_subscription_impl.cc b/source/extensions/config_subscription/filesystem/filesystem_subscription_impl.cc index 76a34fa6bd5b..576608292bdc 100644 --- a/source/extensions/config_subscription/filesystem/filesystem_subscription_impl.cc +++ b/source/extensions/config_subscription/filesystem/filesystem_subscription_impl.cc @@ -35,8 +35,9 @@ FilesystemSubscriptionImpl::FilesystemSubscriptionImpl( return absl::OkStatus(); })); } else { - directory_watcher_ = - std::make_unique(path_config_source.watched_directory(), dispatcher); + directory_watcher_ = THROW_OR_RETURN_VALUE( + WatchedDirectory::create(path_config_source.watched_directory(), dispatcher), + std::unique_ptr); directory_watcher_->setCallback([this]() { if (started_) { refresh(); diff --git a/test/common/config/watched_directory_test.cc b/test/common/config/watched_directory_test.cc index e5a4a3026210..1b61d9bfae08 100644 --- a/test/common/config/watched_directory_test.cc +++ b/test/common/config/watched_directory_test.cc @@ -23,9 +23,9 @@ TEST(WatchedDirectory, All) { Filesystem::Watcher::OnChangedCb cb; EXPECT_CALL(*watcher, addWatch("foo/bar/", Filesystem::Watcher::Events::MovedTo, _)) .WillOnce(DoAll(SaveArg<2>(&cb), Return(absl::OkStatus()))); - WatchedDirectory wd(config, dispatcher); + auto wd = *WatchedDirectory::create(config, dispatcher); bool called = false; - wd.setCallback([&called] { + wd->setCallback([&called] { called = true; return absl::OkStatus(); }); diff --git a/tools/code_format/config.yaml b/tools/code_format/config.yaml index 34104c1f4c96..4d42fdb2fc99 100644 --- a/tools/code_format/config.yaml +++ b/tools/code_format/config.yaml @@ -140,7 +140,6 @@ paths: - source/common/grpc/async_client_impl.cc - source/common/grpc/google_grpc_creds_impl.cc - source/common/local_reply/local_reply.cc - - source/common/config/watched_directory.cc - source/server/drain_manager_impl.cc - source/common/router/rds_impl.cc - source/server/hot_restarting_parent.cc