cilium: Adjust onConfigUpdate signature

Relates: envoyproxy/envoy#29439 Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras · Jan 29, 2024 · 65a3de6 · 65a3de6
1 parent 7195707
commit 65a3de6
Show file tree

Hide file tree

Showing 6 changed files with 26 additions and 21 deletions.
diff --git a/cilium/host_map.cc b/cilium/host_map.cc
@@ -159,8 +159,9 @@ void PolicyHostMap::startSubscription(Server::Configuration::CommonFactoryContex
   subscription_->start({});
 }
 
-void PolicyHostMap::onConfigUpdate(const std::vector<Envoy::Config::DecodedResourceRef>& resources,
-                                   const std::string& version_info) {
+absl::Status
+PolicyHostMap::onConfigUpdate(const std::vector<Envoy::Config::DecodedResourceRef>& resources,
+                              const std::string& version_info) {
   ENVOY_LOG(debug, "PolicyHostMap::onConfigUpdate({}), {} resources, version: {}", name_,
             resources.size(), version_info);
 
@@ -189,6 +190,7 @@ void PolicyHostMap::onConfigUpdate(const std::vector<Envoy::Config::DecodedResou
     return newmap;
   });
   logmaps("onConfigUpdate");
+  return absl::OkStatus();
 }
 
 void PolicyHostMap::onConfigUpdateFailed(Envoy::Config::ConfigUpdateFailureReason,

diff --git a/cilium/host_map.h b/cilium/host_map.h
@@ -199,15 +199,16 @@ class PolicyHostMap : public Singleton::Instance,
   }
 
   // Config::SubscriptionCallbacks
-  void onConfigUpdate(const std::vector<Envoy::Config::DecodedResourceRef>& resources,
-                      const std::string& version_info) override;
-  void onConfigUpdate(const std::vector<Envoy::Config::DecodedResourceRef>& added_resources,
-                      const Protobuf::RepeatedPtrField<std::string>& removed_resources,
-                      const std::string& system_version_info) override {
+  absl::Status onConfigUpdate(const std::vector<Envoy::Config::DecodedResourceRef>& resources,
+                              const std::string& version_info) override;
+  absl::Status onConfigUpdate(const std::vector<Envoy::Config::DecodedResourceRef>& added_resources,
+                              const Protobuf::RepeatedPtrField<std::string>& removed_resources,
+                              const std::string& system_version_info) override {
     // NOT IMPLEMENTED YET.
     UNREFERENCED_PARAMETER(added_resources);
     UNREFERENCED_PARAMETER(removed_resources);
     UNREFERENCED_PARAMETER(system_version_info);
+    return absl::OkStatus();
   }
   void onConfigUpdateFailed(Envoy::Config::ConfigUpdateFailureReason,
                             const EnvoyException* e) override;

diff --git a/cilium/network_policy.cc b/cilium/network_policy.cc
@@ -1155,9 +1155,9 @@ void ThreadLocalPolicyMap::Update(std::vector<std::shared_ptr<PolicyInstanceImpl
   }
 }
 
-void NetworkPolicyMap::onConfigUpdate(
-    const std::vector<Envoy::Config::DecodedResourceRef>& resources,
-    const std::string& version_info) {
+absl::Status
+NetworkPolicyMap::onConfigUpdate(const std::vector<Envoy::Config::DecodedResourceRef>& resources,
+                                 const std::string& version_info) {
   ENVOY_LOG(debug, "NetworkPolicyMap::onConfigUpdate({}), {} resources, version: {}", name_,
             resources.size(), version_info);
 
@@ -1312,6 +1312,7 @@ void NetworkPolicyMap::onConfigUpdate(
 #pragma clang diagnostic ignored "-Wnull-dereference"
   transport_factory_context_->setInitManager(*static_cast<Init::Manager*>(nullptr));
 #pragma clang diagnostic pop
+  return absl::OkStatus();
 }
 
 void NetworkPolicyMap::onConfigUpdateFailed(Envoy::Config::ConfigUpdateFailureReason,

diff --git a/cilium/network_policy.h b/cilium/network_policy.h
@@ -195,15 +195,16 @@ class NetworkPolicyMap : public Singleton::Instance,
   void runAfterAllThreads(std::function<void()>) const;
 
   // Config::SubscriptionCallbacks
-  void onConfigUpdate(const std::vector<Envoy::Config::DecodedResourceRef>& resources,
-                      const std::string& version_info) override;
-  void onConfigUpdate(const std::vector<Envoy::Config::DecodedResourceRef>& added_resources,
-                      const Protobuf::RepeatedPtrField<std::string>& removed_resources,
-                      const std::string& system_version_info) override {
+  absl::Status onConfigUpdate(const std::vector<Envoy::Config::DecodedResourceRef>& resources,
+                              const std::string& version_info) override;
+  absl::Status onConfigUpdate(const std::vector<Envoy::Config::DecodedResourceRef>& added_resources,
+                              const Protobuf::RepeatedPtrField<std::string>& removed_resources,
+                              const std::string& system_version_info) override {
     // NOT IMPLEMENTED YET.
     UNREFERENCED_PARAMETER(added_resources);
     UNREFERENCED_PARAMETER(removed_resources);
     UNREFERENCED_PARAMETER(system_version_info);
+    return absl::OkStatus();
   }
   void onConfigUpdateFailed(Envoy::Config::ConfigUpdateFailureReason,
                             const EnvoyException* e) override;

diff --git a/tests/cilium_http_integration_test.cc b/tests/cilium_http_integration_test.cc
@@ -373,9 +373,8 @@ class HostMapTest : public CiliumHttpIntegrationTest {
     const auto decoded_resources = Envoy::Config::DecodedResourcesWrapper(
         host_decoder, message.resources(), message.version_info());
 
-    EXPECT_THROW_WITH_MESSAGE(
-        hmap.onConfigUpdate(decoded_resources.refvec_, message.version_info()), EnvoyException,
-        exmsg);
+    EXPECT_EQ(hmap.onConfigUpdate(decoded_resources.refvec_, message.version_info()).message(),
+              exmsg);
     tls.shutdownGlobalThreading();
   }
 };
@@ -411,7 +410,7 @@ TEST_P(HostMapTest, HostMapValid) {
   const auto decoded_resources = Envoy::Config::DecodedResourcesWrapper(
       host_decoder, message.resources(), message.version_info());
 
-  VERBOSE_EXPECT_NO_THROW(hmap->onConfigUpdate(decoded_resources.refvec_, message.version_info()));
+  EXPECT_TRUE(hmap->onConfigUpdate(decoded_resources.refvec_, message.version_info()).ok());
 
   EXPECT_EQ(hmap->resolve(Network::Address::Ipv4Instance("192.168.0.1").ip()), 173);
   EXPECT_EQ(hmap->resolve(Network::Address::Ipv4Instance("192.168.0.0").ip()), 12);

diff --git a/tests/cilium_network_policy_test.cc b/tests/cilium_network_policy_test.cc
@@ -52,7 +52,8 @@ class CiliumNetworkPolicyTest : public ::testing::Test {
     NetworkPolicyDecoder network_policy_decoder;
     const auto decoded_resources = Config::DecodedResourcesWrapper(
         network_policy_decoder, message.resources(), message.version_info());
-    policy_map_->onConfigUpdate(decoded_resources.refvec_, message.version_info());
+    EXPECT_TRUE(
+        policy_map_->onConfigUpdate(decoded_resources.refvec_, message.version_info()).ok());
     return message.version_info();
   }
 
@@ -96,7 +97,7 @@ class CiliumNetworkPolicyTest : public ::testing::Test {
 };
 
 TEST_F(CiliumNetworkPolicyTest, EmptyPolicyUpdate) {
-  EXPECT_NO_THROW(policy_map_->onConfigUpdate({}, "1"));
+  EXPECT_TRUE(policy_map_->onConfigUpdate({}, "1").ok());
   EXPECT_FALSE(Validate("10.1.2.3", "")); // Policy not found
 }