Merge pull request #9445 from Icinga/9365

Disallow config modifications via API during reload

lib/base/CMakeLists.txt

@@ -65,6 +65,7 @@ set(base_SOURCES
   scriptutils.cpp scriptutils.hpp
   serializer.cpp serializer.hpp
   shared.hpp
+  shared-memory.hpp
   shared-object.hpp
   singleton.hpp
   socket.cpp socket.hpp

lib/base/shared-memory.hpp

@@ -0,0 +1,45 @@
+/* Icinga 2 | (c) 2023 Icinga GmbH | GPLv2+ */
+
+#pragma once
+
+#include <boost/interprocess/anonymous_shared_memory.hpp>
+#include <utility>
+
+namespace icinga
+{
+
+/**
+ * Type-safe memory shared across fork(2).
+ *
+ * @ingroup base
+ */
+template<class T>
+class SharedMemory
+{
+public:
+	template<class... Args>
+	SharedMemory(Args&&... args) : m_Memory(boost::interprocess::anonymous_shared_memory(sizeof(T)))
+	{
+		new(GetAddress()) T(std::forward<Args>(args)...);
+	}
+
+	SharedMemory(const SharedMemory&) = delete;
+	SharedMemory(SharedMemory&&) = delete;
+	SharedMemory& operator=(const SharedMemory&) = delete;
+	SharedMemory& operator=(SharedMemory&&) = delete;
+
+	inline T& Get() const
+	{
+		return *GetAddress();
+	}
+
+private:
+	inline T* GetAddress() const
+	{
+		return (T*)m_Memory.get_address();
+	}
+
+	boost::interprocess::mapped_region m_Memory;
+};
+
+}

lib/cli/daemoncommand.cpp

@@ -3,6 +3,7 @@
 #include "cli/daemoncommand.hpp"
 #include "cli/daemonutility.hpp"
 #include "remote/apilistener.hpp"
+#include "remote/configobjectslock.hpp"
 #include "remote/configobjectutility.hpp"
 #include "config/configcompiler.hpp"
 #include "config/configcompilercontext.hpp"
@@ -801,6 +802,10 @@ int DaemonCommand::Run(const po::variables_map& vm, const std::vector<std::strin
 			sd_notify(0, "RELOADING=1");
 #endif /* HAVE_SYSTEMD */
 
+			// The old process is still active, yet.
+			// Its config changes would not be visible to the new one after config load.
+			ConfigObjectsExclusiveLock lock;
+
 			pid_t nextWorker = StartUnixWorker(configs);
 
 			switch (nextWorker) {

lib/icinga/apiactions.cpp

@@ -12,6 +12,7 @@
 #include "icinga/clusterevents.hpp"
 #include "remote/apiaction.hpp"
 #include "remote/apilistener.hpp"
+#include "remote/configobjectslock.hpp"
 #include "remote/filterutility.hpp"
 #include "remote/pkiutility.hpp"
 #include "remote/httputility.hpp"
@@ -254,6 +255,12 @@ Dictionary::Ptr ApiActions::AcknowledgeProblem(const ConfigObject::Ptr& object,
 		return ApiActions::CreateResult(409, (service ? "Service " : "Host ") + checkable->GetName() + " is already acknowledged.");
 	}
 
+	ConfigObjectsSharedLock lock (std::try_to_lock);
+
+	if (!lock) {
+		return ApiActions::CreateResult(503, "Icinga is reloading.");
+	}
+
 	Comment::AddComment(checkable, CommentAcknowledgement, HttpUtility::GetLastParameter(params, "author"),
 		HttpUtility::GetLastParameter(params, "comment"), persistent, timestamp, sticky == AcknowledgementSticky);
 	checkable->AcknowledgeProblem(HttpUtility::GetLastParameter(params, "author"),
@@ -272,6 +279,12 @@ Dictionary::Ptr ApiActions::RemoveAcknowledgement(const ConfigObject::Ptr& objec
 			"Cannot remove acknowledgement for non-existent checkable object "
 			+ object->GetName() + ".");
 
+	ConfigObjectsSharedLock lock (std::try_to_lock);
+
+	if (!lock) {
+		return ApiActions::CreateResult(503, "Icinga is reloading.");
+	}
+
 	String removedBy (HttpUtility::GetLastParameter(params, "author"));
 
 	checkable->ClearAcknowledgement(removedBy);
@@ -297,6 +310,12 @@ Dictionary::Ptr ApiActions::AddComment(const ConfigObject::Ptr& object,
 		timestamp = HttpUtility::GetLastParameter(params, "expiry");
 	}
 
+	ConfigObjectsSharedLock lock (std::try_to_lock);
+
+	if (!lock) {
+		return ApiActions::CreateResult(503, "Icinga is reloading.");
+	}
+
 	String commentName = Comment::AddComment(checkable, CommentUser,
 		HttpUtility::GetLastParameter(params, "author"),
 		HttpUtility::GetLastParameter(params, "comment"), false, timestamp);
@@ -316,6 +335,12 @@ Dictionary::Ptr ApiActions::AddComment(const ConfigObject::Ptr& object,
 Dictionary::Ptr ApiActions::RemoveComment(const ConfigObject::Ptr& object,
 	const Dictionary::Ptr& params)
 {
+	ConfigObjectsSharedLock lock (std::try_to_lock);
+
+	if (!lock) {
+		return ApiActions::CreateResult(503, "Icinga is reloading.");
+	}
+
 	auto author (HttpUtility::GetLastParameter(params, "author"));
 	Checkable::Ptr checkable = dynamic_pointer_cast<Checkable>(object);
 
@@ -388,6 +413,12 @@ Dictionary::Ptr ApiActions::ScheduleDowntime(const ConfigObject::Ptr& object,
 		}
 	}
 
+	ConfigObjectsSharedLock lock (std::try_to_lock);
+
+	if (!lock) {
+		return ApiActions::CreateResult(503, "Icinga is reloading.");
+	}
+
 	Downtime::Ptr downtime = Downtime::AddDowntime(checkable, author, comment, startTime, endTime,
 		fixed, triggerName, duration);
 	String downtimeName = downtime->GetName();
@@ -500,6 +531,12 @@ Dictionary::Ptr ApiActions::ScheduleDowntime(const ConfigObject::Ptr& object,
 Dictionary::Ptr ApiActions::RemoveDowntime(const ConfigObject::Ptr& object,
 	const Dictionary::Ptr& params)
 {
+	ConfigObjectsSharedLock lock (std::try_to_lock);
+
+	if (!lock) {
+		return ApiActions::CreateResult(503, "Icinga is reloading.");
+	}
+
 	auto author (HttpUtility::GetLastParameter(params, "author"));
 	Checkable::Ptr checkable = dynamic_pointer_cast<Checkable>(object);
 

lib/remote/CMakeLists.txt

@@ -14,6 +14,7 @@ set(remote_SOURCES
   apilistener-authority.cpp
   apiuser.cpp apiuser.hpp apiuser-ti.hpp
   configfileshandler.cpp configfileshandler.hpp
+  configobjectslock.cpp configobjectslock.hpp
   configobjectutility.cpp configobjectutility.hpp
   configpackageshandler.cpp configpackageshandler.hpp
   configpackageutility.cpp configpackageutility.hpp

lib/remote/configobjectslock.cpp

@@ -0,0 +1,24 @@
+/* Icinga 2 | (c) 2022 Icinga GmbH | GPLv2+ */
+
+#ifndef _WIN32
+
+#include "base/shared-memory.hpp"
+#include "remote/configobjectslock.hpp"
+#include <boost/interprocess/sync/lock_options.hpp>
+
+using namespace icinga;
+
+// On *nix one process may write config objects while another is loading the config, so this uses IPC.
+static SharedMemory<boost::interprocess::interprocess_sharable_mutex> l_ConfigObjectsMutex;
+
+ConfigObjectsExclusiveLock::ConfigObjectsExclusiveLock()
+	: m_Lock(l_ConfigObjectsMutex.Get())
+{
+}
+
+ConfigObjectsSharedLock::ConfigObjectsSharedLock(std::try_to_lock_t)
+	: m_Lock(l_ConfigObjectsMutex.Get(), boost::interprocess::try_to_lock)
+{
+}
+
+#endif /* _WIN32 */

lib/remote/configobjectslock.hpp

@@ -0,0 +1,72 @@
+/* Icinga 2 | (c) 2023 Icinga GmbH | GPLv2+ */
+
+#pragma once
+
+#include <mutex>
+
+#ifndef _WIN32
+#include <boost/interprocess/sync/interprocess_sharable_mutex.hpp>
+#include <boost/interprocess/sync/scoped_lock.hpp>
+#include <boost/interprocess/sync/sharable_lock.hpp>
+#endif /* _WIN32 */
+
+namespace icinga
+{
+
+#ifdef _WIN32
+
+class ConfigObjectsSharedLock
+{
+public:
+	inline ConfigObjectsSharedLock(std::try_to_lock_t)
+	{
+	}
+
+	constexpr explicit operator bool() const
+	{
+		return true;
+	}
+};
+
+#else /* _WIN32 */
+
+/**
+ * Waits until all ConfigObjects*Lock-s have vanished. For its lifetime disallows such.
+ * Keep an instance alive during reload to forbid runtime config changes!
+ * This way Icinga reads a consistent config which doesn't suddenly get runtime-changed.
+ *
+ * @ingroup remote
+ */
+class ConfigObjectsExclusiveLock
+{
+public:
+	ConfigObjectsExclusiveLock();
+
+private:
+	boost::interprocess::scoped_lock<boost::interprocess::interprocess_sharable_mutex> m_Lock;
+};
+
+/**
+ * Waits until the only ConfigObjectsExclusiveLock has vanished (if any). For its lifetime disallows such.
+ * Keep an instance alive during runtime config changes to delay a reload (if any)!
+ * This way Icinga reads a consistent config which doesn't suddenly get runtime-changed.
+ *
+ * @ingroup remote
+ */
+class ConfigObjectsSharedLock
+{
+public:
+	ConfigObjectsSharedLock(std::try_to_lock_t);
+
+	inline explicit operator bool() const
+	{
+		return m_Lock.owns();
+	}
+
+private:
+	boost::interprocess::sharable_lock<boost::interprocess::interprocess_sharable_mutex> m_Lock;
+};
+
+#endif /* _WIN32 */
+
+}

lib/remote/createobjecthandler.cpp

@@ -1,6 +1,7 @@
 /* Icinga 2 | (c) 2012 Icinga GmbH | GPLv2+ */
 
 #include "remote/createobjecthandler.hpp"
+#include "remote/configobjectslock.hpp"
 #include "remote/configobjectutility.hpp"
 #include "remote/httputility.hpp"
 #include "remote/jsonrpcconnection.hpp"
@@ -94,6 +95,13 @@ bool CreateObjectHandler::HandleRequest(
 	if (params)
 		verbose = HttpUtility::GetLastParameter(params, "verbose");
 
+	ConfigObjectsSharedLock lock (std::try_to_lock);
+
+	if (!lock) {
+		HttpUtility::SendJsonError(response, params, 503, "Icinga is reloading");
+		return true;
+	}
+
 	/* Object creation can cause multiple errors and optionally diagnostic information.
 	 * We can't use SendJsonError() here.
 	 */

lib/remote/deleteobjecthandler.cpp

@@ -1,6 +1,7 @@
 /* Icinga 2 | (c) 2012 Icinga GmbH | GPLv2+ */
 
 #include "remote/deleteobjecthandler.hpp"
+#include "remote/configobjectslock.hpp"
 #include "remote/configobjectutility.hpp"
 #include "remote/httputility.hpp"
 #include "remote/filterutility.hpp"
@@ -66,6 +67,13 @@ bool DeleteObjectHandler::HandleRequest(
 	bool cascade = HttpUtility::GetLastParameter(params, "cascade");
 	bool verbose = HttpUtility::GetLastParameter(params, "verbose");
 
+	ConfigObjectsSharedLock lock (std::try_to_lock);
+
+	if (!lock) {
+		HttpUtility::SendJsonError(response, params, 503, "Icinga is reloading");
+		return true;
+	}
+
 	ArrayData results;
 
 	bool success = true;

lib/remote/modifyobjecthandler.cpp

@@ -1,6 +1,7 @@
 /* Icinga 2 | (c) 2012 Icinga GmbH | GPLv2+ */
 
 #include "remote/modifyobjecthandler.hpp"
+#include "remote/configobjectslock.hpp"
 #include "remote/httputility.hpp"
 #include "remote/filterutility.hpp"
 #include "remote/apiaction.hpp"
@@ -77,6 +78,13 @@ bool ModifyObjectHandler::HandleRequest(
 	if (params)
 		verbose = HttpUtility::GetLastParameter(params, "verbose");
 
+	ConfigObjectsSharedLock lock (std::try_to_lock);
+
+	if (!lock) {
+		HttpUtility::SendJsonError(response, params, 503, "Icinga is reloading");
+		return true;
+	}
+
 	ArrayData results;
 
 	for (const ConfigObject::Ptr& obj : objs) {