Deduplicate and stabilize fragile filesystem transactions

by using AtomicFile so they ensure all or nothing of a file gets replaced.

lib/base/scriptglobal.cpp

@@ -1,5 +1,6 @@
 /* Icinga 2 | (c) 2012 Icinga GmbH | GPLv2+ */
 
+#include "base/atomic-file.hpp"
 #include "base/scriptglobal.hpp"
 #include "base/singleton.hpp"
 #include "base/logger.hpp"
@@ -83,12 +84,7 @@ void ScriptGlobal::WriteToFile(const String& filename)
 	Log(LogInformation, "ScriptGlobal")
 		<< "Dumping variables to file '" << filename << "'";
 
-	std::fstream fp;
+	AtomicFile fp (filename, 0600);
-	String tempFilename = Utility::CreateTempFile(filename + ".XXXXXX", 0600, fp);
-
-	if (!fp)
-		BOOST_THROW_EXCEPTION(std::runtime_error("Could not open '" + tempFilename + "' file"));
-
 	StdioStream::Ptr sfp = new StdioStream(&fp, false);
 
 	ObjectLock olock(m_Globals);
@@ -109,9 +105,6 @@ void ScriptGlobal::WriteToFile(const String& filename)
 	}
 
 	sfp->Close();
-
+	fp.Commit();
-	fp.close();
-
-	Utility::RenameFile(tempFilename, filename);
 }
 

lib/base/utility.cpp

@@ -1,5 +1,6 @@
 /* Icinga 2 | (c) 2012 Icinga GmbH | GPLv2+ */
 
+#include "base/atomic-file.hpp"
 #include "base/utility.hpp"
 #include "base/convert.hpp"
 #include "base/application.hpp"
@@ -1443,16 +1444,7 @@ Value Utility::LoadJsonFile(const String& path)
 
 void Utility::SaveJsonFile(const String& path, int mode, const Value& value)
 {
-	namespace fs = boost::filesystem;
+	AtomicFile::Write(path, mode, JsonEncode(value));
-
-	std::fstream fp;
-	String tempFilename = Utility::CreateTempFile(path + ".XXXXXX", mode, fp);
-
-	fp.exceptions(std::ofstream::failbit | std::ofstream::badbit);
-	fp << JsonEncode(value);
-	fp.close();
-
-	RenameFile(tempFilename, path);
 }
 
 static void HexEncode(char ch, std::ostream& os)

lib/cli/apisetuputility.cpp

@@ -5,6 +5,7 @@
 #include "cli/featureutility.hpp"
 #include "remote/apilistener.hpp"
 #include "remote/pkiutility.hpp"
+#include "base/atomic-file.hpp"
 #include "base/logger.hpp"
 #include "base/console.hpp"
 #include "base/application.hpp"
@@ -160,8 +161,7 @@ bool ApiSetupUtility::SetupMasterApiUser()
 
 	NodeUtility::CreateBackupFile(apiUsersPath);
 
-	std::fstream fp;
+	AtomicFile fp (apiUsersPath, 0644);
-	String tempFilename = Utility::CreateTempFile(apiUsersPath + ".XXXXXX", 0644, fp);
 
 	fp << "/**\n"
 		<< " * The ApiUser objects are used for authentication against the API.\n"
@@ -173,9 +173,7 @@ bool ApiSetupUtility::SetupMasterApiUser()
 		<< "  permissions = [ \"*\" ]\n"
 		<< "}\n";
 
-	fp.close();
+	fp.Commit();
-
-	Utility::RenameFile(tempFilename, apiUsersPath);
 
 	return true;
 }

lib/cli/nodesetupcommand.cpp

@@ -6,6 +6,7 @@
 #include "cli/apisetuputility.hpp"
 #include "remote/apilistener.hpp"
 #include "remote/pkiutility.hpp"
+#include "base/atomic-file.hpp"
 #include "base/logger.hpp"
 #include "base/console.hpp"
 #include "base/application.hpp"
@@ -172,8 +173,7 @@ int NodeSetupCommand::SetupMaster(const boost::program_options::variables_map& v
 	String apipath = FeatureUtility::GetFeaturesAvailablePath() + "/api.conf";
 	NodeUtility::CreateBackupFile(apipath);
 
-	std::fstream fp;
+	AtomicFile fp (apipath, 0644);
-	String tempApiPath = Utility::CreateTempFile(apipath + ".XXXXXX", 0644, fp);
 
 	fp << "/**\n"
 		<< " * The API listener is used for distributed monitoring setups.\n"
@@ -205,9 +205,7 @@ int NodeSetupCommand::SetupMaster(const boost::program_options::variables_map& v
 		<< "  ticket_salt = TicketSalt\n"
 		<< "}\n";
 
-	fp.close();
+	fp.Commit();
-
-	Utility::RenameFile(tempApiPath, apipath);
 
 	/* update constants.conf with NodeName = CN + TicketSalt = random value */
 	if (endpointName != Utility::GetFQDN()) {
@@ -447,8 +445,7 @@ int NodeSetupCommand::SetupNode(const boost::program_options::variables_map& vm,
 	String apipath = FeatureUtility::GetFeaturesAvailablePath() + "/api.conf";
 	NodeUtility::CreateBackupFile(apipath);
 
-	std::fstream fp;
+	AtomicFile fp (apipath, 0644);
-	String tempApiPath = Utility::CreateTempFile(apipath + ".XXXXXX", 0644, fp);
 
 	fp << "/**\n"
 		<< " * The API listener is used for distributed monitoring setups.\n"
@@ -479,9 +476,7 @@ int NodeSetupCommand::SetupNode(const boost::program_options::variables_map& vm,
 	fp << "\n"
 		<< "}\n";
 
-	fp.close();
+	fp.Commit();
-
-	Utility::RenameFile(tempApiPath, apipath);
 
 	/* Generate zones configuration. */
 	Log(LogInformation, "cli", "Generating zone and object configuration.");
@@ -530,20 +525,14 @@ int NodeSetupCommand::SetupNode(const boost::program_options::variables_map& vm,
 	if (!ticket.IsEmpty()) {
 		String ticketPath = ApiListener::GetCertsDir() + "/ticket";
 
-		String tempTicketPath = Utility::CreateTempFile(ticketPath + ".XXXXXX", 0600, fp);
+		AtomicFile::Write(ticketPath, 0600, ticket);
 
-		if (!Utility::SetFileOwnership(tempTicketPath, user, group)) {
+		if (!Utility::SetFileOwnership(ticketPath, user, group)) {
 			Log(LogWarning, "cli")
 				<< "Cannot set ownership for user '" << user
 				<< "' group '" << group
-				<< "' on file '" << tempTicketPath << "'. Verify it yourself!";
+				<< "' on file '" << ticketPath << "'. Verify it yourself!";
 		}
-
-		fp << ticket;
-
-		fp.close();
-
-		Utility::RenameFile(tempTicketPath, ticketPath);
 	}
 
 	/* If no parent connection was made, the user must supply the ca.crt before restarting Icinga 2.*/

lib/cli/nodeutility.cpp

@@ -3,6 +3,7 @@
 #include "cli/nodeutility.hpp"
 #include "cli/clicommand.hpp"
 #include "cli/variableutility.hpp"
+#include "base/atomic-file.hpp"
 #include "base/logger.hpp"
 #include "base/application.hpp"
 #include "base/tlsutility.hpp"
@@ -165,8 +166,7 @@ bool NodeUtility::WriteNodeConfigObjects(const String& filename, const Array::Pt
 			<< "Cannot set ownership for user '" << user << "' group '" << group << "' on path '" << path << "'. Verify it yourself!";
 	}
 
-	std::fstream fp;
+	AtomicFile fp (filename, 0644);
-	String tempFilename = Utility::CreateTempFile(filename + ".XXXXXX", 0644, fp);
 
 	fp << "/*\n";
 	fp << " * Generated by Icinga 2 node setup commands\n";
@@ -179,9 +179,7 @@ bool NodeUtility::WriteNodeConfigObjects(const String& filename, const Array::Pt
 	}
 
 	fp << std::endl;
-	fp.close();
+	fp.Commit();
-
-	Utility::RenameFile(tempFilename, filename);
 
 	return true;
 }
@@ -299,8 +297,7 @@ bool NodeUtility::UpdateConfiguration(const String& value, bool include, bool re
 	NodeUtility::CreateBackupFile(configurationFile);
 
 	std::ifstream ifp(configurationFile.CStr());
-	std::fstream ofp;
+	AtomicFile ofp (configurationFile, 0644);
-	String tempFile = Utility::CreateTempFile(configurationFile + ".XXXXXX", 0644, ofp);
 
 	String affectedInclude = value;
 
@@ -349,9 +346,7 @@ bool NodeUtility::UpdateConfiguration(const String& value, bool include, bool re
 	}
 
 	ifp.close();
-	ofp.close();
+	ofp.Commit();
-
-	Utility::RenameFile(tempFile, configurationFile);
 
 	return (found || include);
 }
@@ -366,8 +361,7 @@ void NodeUtility::UpdateConstant(const String& name, const String& value)
 	NodeUtility::CreateBackupFile(constantsConfPath);
 
 	std::ifstream ifp(constantsConfPath.CStr());
-	std::fstream ofp;
+	AtomicFile ofp (constantsConfPath, 0644);
-	String tempFile = Utility::CreateTempFile(constantsConfPath + ".XXXXXX", 0644, ofp);
 
 	bool found = false;
 
@@ -384,7 +378,5 @@ void NodeUtility::UpdateConstant(const String& name, const String& value)
 		ofp << "const " + name + " = \"" + value + "\"\n";
 
 	ifp.close();
-	ofp.close();
+	ofp.Commit();
-
-	Utility::RenameFile(tempFile, constantsConfPath);
 }

lib/cli/nodewizardcommand.cpp

@@ -6,6 +6,7 @@
 #include "cli/apisetuputility.hpp"
 #include "remote/apilistener.hpp"
 #include "remote/pkiutility.hpp"
+#include "base/atomic-file.hpp"
 #include "base/logger.hpp"
 #include "base/console.hpp"
 #include "base/application.hpp"
@@ -451,8 +452,7 @@ wizard_ticket:
 	String apiConfPath = FeatureUtility::GetFeaturesAvailablePath() + "/api.conf";
 	NodeUtility::CreateBackupFile(apiConfPath);
 
-	std::fstream fp;
+	AtomicFile fp (apiConfPath, 0644);
-	String tempApiConfPath = Utility::CreateTempFile(apiConfPath + ".XXXXXX", 0644, fp);
 
 	fp << "/**\n"
 		<< " * The API listener is used for distributed monitoring setups.\n"
@@ -468,9 +468,7 @@ wizard_ticket:
 
 	fp << "}\n";
 
-	fp.close();
+	fp.Commit();
-
-	Utility::RenameFile(tempApiConfPath, apiConfPath);
 
 	/* Zones configuration. */
 	Log(LogInformation, "cli", "Generating local zones.conf.");
@@ -556,20 +554,14 @@ wizard_global_zone_loop_start:
 	if (!ticket.IsEmpty()) {
 		String ticketPath = ApiListener::GetCertsDir() + "/ticket";
 
-		String tempTicketPath = Utility::CreateTempFile(ticketPath + ".XXXXXX", 0600, fp);
+		AtomicFile::Write(ticketPath, 0600, ticket);
 
-		if (!Utility::SetFileOwnership(tempTicketPath, user, group)) {
+		if (!Utility::SetFileOwnership(ticketPath, user, group)) {
 			Log(LogWarning, "cli")
 				<< "Cannot set ownership for user '" << user
 				<< "' group '" << group
-				<< "' on file '" << tempTicketPath << "'. Verify it yourself!";
+				<< "' on file '" << ticketPath << "'. Verify it yourself!";
 		}
-
-		fp << ticket;
-
-		fp.close();
-
-		Utility::RenameFile(tempTicketPath, ticketPath);
 	}
 
 	/* If no parent connection was made, the user must supply the ca.crt before restarting Icinga 2.*/
@@ -745,8 +737,7 @@ wizard_global_zone_loop_start:
 	String apiConfPath = FeatureUtility::GetFeaturesAvailablePath() + "/api.conf";
 	NodeUtility::CreateBackupFile(apiConfPath);
 
-	std::fstream fp;
+	AtomicFile fp (apiConfPath, 0644);
-	String tempApiConfPath = Utility::CreateTempFile(apiConfPath + ".XXXXXX", 0644, fp);
 
 	fp << "/**\n"
 		<< " * The API listener is used for distributed monitoring setups.\n"
@@ -762,9 +753,7 @@ wizard_global_zone_loop_start:
 		<< "  ticket_salt = TicketSalt\n"
 		<< "}\n";
 
-	fp.close();
+	fp.Commit();
-
-	Utility::RenameFile(tempApiConfPath, apiConfPath);
 
 	/* update constants.conf with NodeName = CN + TicketSalt = random value */
 	if (cn != Utility::GetFQDN()) {

lib/compat/statusdatawriter.cpp

@@ -13,6 +13,7 @@
 #include "icinga/compatutility.hpp"
 #include "icinga/pluginutility.hpp"
 #include "icinga/dependency.hpp"
+#include "base/atomic-file.hpp"
 #include "base/configtype.hpp"
 #include "base/objectlock.hpp"
 #include "base/json.hpp"
@@ -544,8 +545,7 @@ void StatusDataWriter::UpdateObjectsCache()
 	/* Use the compat path here from the .ti generated class. */
 	String objectsPath = GetObjectsPath();
 
-	std::fstream objectfp;
+	AtomicFile objectfp (objectsPath, 0644);
-	String tempObjectsPath = Utility::CreateTempFile(objectsPath + ".XXXXXX", 0644, objectfp);
 
 	objectfp << std::fixed;
 
@@ -760,9 +760,7 @@ void StatusDataWriter::UpdateObjectsCache()
 		}
 	}
 
-	objectfp.close();
+	objectfp.Commit();
-
-	Utility::RenameFile(tempObjectsPath, objectsPath);
 }
 
 /**
@@ -779,8 +777,7 @@ void StatusDataWriter::StatusTimerHandler()
 
 	String statusPath = GetStatusPath();
 
-	std::fstream statusfp;
+	AtomicFile statusfp (statusPath, 0644);
-	String tempStatusPath = Utility::CreateTempFile(statusPath + ".XXXXXX", 0644, statusfp);
 
 	statusfp << std::fixed;
 
@@ -833,9 +830,7 @@ void StatusDataWriter::StatusTimerHandler()
 		}
 	}
 
-	statusfp.close();
+	statusfp.Commit();
-
-	Utility::RenameFile(tempStatusPath, statusPath);
 
 	Log(LogNotice, "StatusDataWriter")
 		<< "Writing status.dat file took " << Utility::FormatDuration(Utility::GetTime() - start);

lib/config/configcompilercontext.cpp

@@ -17,20 +17,12 @@ ConfigCompilerContext *ConfigCompilerContext::GetInstance()
 
 void ConfigCompilerContext::OpenObjectsFile(const String& filename)
 {
-	m_ObjectsPath = filename;
-
-	auto *fp = new std::fstream();
 	try {
-		m_ObjectsTempFile = Utility::CreateTempFile(filename + ".XXXXXX", 0600, *fp);
+		m_ObjectsFP = std::make_unique<AtomicFile>(filename, 0600);
 	} catch (const std::exception& ex) {
 		Log(LogCritical, "cli", "Could not create temporary objects file: " + DiagnosticInformation(ex, false));
 		Application::Exit(1);
 	}
-
-	if (!*fp)
-		BOOST_THROW_EXCEPTION(std::runtime_error("Could not open '" + m_ObjectsTempFile + "' file"));
-
-	m_ObjectsFP = fp;
 }
 
 void ConfigCompilerContext::WriteObject(const Dictionary::Ptr& object)
@@ -51,14 +43,7 @@ void ConfigCompilerContext::CancelObjectsFile()
 	if (!m_ObjectsFP)
 		return;
 
-	delete m_ObjectsFP;
+	m_ObjectsFP.reset(nullptr);
-	m_ObjectsFP = nullptr;
-
-#ifdef _WIN32
-	_unlink(m_ObjectsTempFile.CStr());
-#else /* _WIN32 */
-	unlink(m_ObjectsTempFile.CStr());
-#endif /* _WIN32 */
 }
 
 void ConfigCompilerContext::FinishObjectsFile()
@@ -66,9 +51,7 @@ void ConfigCompilerContext::FinishObjectsFile()
 	if (!m_ObjectsFP)
 		return;
 
-	delete m_ObjectsFP;
+	m_ObjectsFP->Commit();
-	m_ObjectsFP = nullptr;
+	m_ObjectsFP.reset(nullptr);
-
-	Utility::RenameFile(m_ObjectsTempFile, m_ObjectsPath);
 }
 

lib/config/configcompilercontext.hpp

@@ -4,8 +4,10 @@
 #define CONFIGCOMPILERCONTEXT_H
 
 #include "config/i2-config.hpp"
+#include "base/atomic-file.hpp"
 #include "base/dictionary.hpp"
 #include <fstream>
+#include <memory>
 #include <mutex>
 
 namespace icinga
@@ -24,15 +26,13 @@ public:
 
 	inline bool IsOpen() const noexcept
 	{
-		return m_ObjectsFP;
+		return (bool)m_ObjectsFP;
 	}
 
 	static ConfigCompilerContext *GetInstance();
 
 private:
-	String m_ObjectsPath;
+	std::unique_ptr<AtomicFile> m_ObjectsFP;
-	String m_ObjectsTempFile;
-	std::fstream *m_ObjectsFP{nullptr};
 
 	mutable std::mutex m_Mutex;
 };

lib/icinga/icingaapplication.cpp

@@ -5,6 +5,7 @@
 #include "icinga/cib.hpp"
 #include "icinga/macroprocessor.hpp"
 #include "config/configcompiler.hpp"
+#include "base/atomic-file.hpp"
 #include "base/configwriter.hpp"
 #include "base/configtype.hpp"
 #include "base/exception.hpp"
@@ -125,7 +126,7 @@ void IcingaApplication::OnShutdown()
 	DumpProgramState();
 }
 
-static void PersistModAttrHelper(std::fstream& fp, ConfigObject::Ptr& previousObject, const ConfigObject::Ptr& object, const String& attr, const Value& value)
+static void PersistModAttrHelper(AtomicFile& fp, ConfigObject::Ptr& previousObject, const ConfigObject::Ptr& object, const String& attr, const Value& value)
 {
 	if (object != previousObject) {
 		if (previousObject) {
@@ -174,9 +175,7 @@ void IcingaApplication::DumpModifiedAttributes()
 		Log(LogWarning, "IcingaApplication") << DiagnosticInformation(ex);
 	}
 
-	std::fstream fp;
+	AtomicFile fp (path, 0644);
-	String tempFilename = Utility::CreateTempFile(path + ".tmp.XXXXXX", 0644, fp);
-	fp.exceptions(std::ofstream::failbit | std::ofstream::badbit);
 
 	ConfigObject::Ptr previousObject;
 	ConfigObject::DumpModifiedAttributes([&fp, &previousObject](const ConfigObject::Ptr& object, const String& attr, const Value& value) {
@@ -189,9 +188,7 @@ void IcingaApplication::DumpModifiedAttributes()
 		ConfigWriter::EmitRaw(fp, "\n}\n");
 	}
 
-	fp.close();
+	fp.Commit();
-
-	Utility::RenameFile(tempFilename, path);
 }
 
 IcingaApplication::Ptr IcingaApplication::GetInstance()

lib/remote/apilistener.cpp

@@ -8,6 +8,7 @@
 #include "remote/apifunction.hpp"
 #include "remote/configpackageutility.hpp"
 #include "remote/configobjectutility.hpp"
+#include "base/atomic-file.hpp"
 #include "base/convert.hpp"
 #include "base/defer.hpp"
 #include "base/io-engine.hpp"
@@ -324,14 +325,7 @@ void ApiListener::RenewOwnCert()
 		return;
 	}
 
-	std::fstream certfp;
+	AtomicFile::Write(certPath, 0644, CertificateToString(cert));
-	auto tempCertPath (Utility::CreateTempFile(certPath + ".XXXXXX", 0644, certfp));
-
-	certfp.exceptions(std::ofstream::failbit | std::ofstream::badbit);
-	certfp << CertificateToString(cert);
-	certfp.close();
-
-	Utility::RenameFile(tempCertPath, certPath);
 	UpdateSSLContext();
 }
 

lib/remote/jsonrpcconnection-pki.cpp

@@ -4,6 +4,7 @@
 #include "remote/apilistener.hpp"
 #include "remote/apifunction.hpp"
 #include "remote/jsonrpc.hpp"
+#include "base/atomic-file.hpp"
 #include "base/configtype.hpp"
 #include "base/objectlock.hpp"
 #include "base/utility.hpp"
@@ -368,12 +369,7 @@ Value UpdateCertificateHandler(const MessageOrigin::Ptr& origin, const Dictionar
 	Log(LogInformation, "JsonRpcConnection")
 		<< "Updating CA certificate in '" << caPath << "'.";
 
-	std::fstream cafp;
+	AtomicFile::Write(caPath, 0644, ca);
-	String tempCaPath = Utility::CreateTempFile(caPath + ".XXXXXX", 0644, cafp);
-	cafp << ca;
-	cafp.close();
-
-	Utility::RenameFile(tempCaPath, caPath);
 
 	/* Update signed certificate. */
 	String certPath = listener->GetDefaultCertPath();
@@ -381,12 +377,7 @@ Value UpdateCertificateHandler(const MessageOrigin::Ptr& origin, const Dictionar
 	Log(LogInformation, "JsonRpcConnection")
 		<< "Updating client certificate for CN '" << cn << "' in '" << certPath << "'.";
 
-	std::fstream certfp;
+	AtomicFile::Write(certPath, 0644, cert);
-	String tempCertPath = Utility::CreateTempFile(certPath + ".XXXXXX", 0644, certfp);
-	certfp << cert;
-	certfp.close();
-
-	Utility::RenameFile(tempCertPath, certPath);
 
 	/* Remove ticket for successful signing request. */
 	String ticketPath = ApiListener::GetCertsDir() + "/ticket";