Skip to content

Commit

Permalink
Don't write runtime created objects to disk before validating them
Browse files Browse the repository at this point in the history
  • Loading branch information
yhabteab committed Jan 25, 2024
1 parent 01a6c4c commit 23d30d0
Showing 1 changed file with 17 additions and 13 deletions.
30 changes: 17 additions & 13 deletions lib/remote/configobjectutility.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -198,13 +198,16 @@ bool ConfigObjectUtility::CreateObject(const Type::Ptr& type, const String& full
return false;
}

Utility::MkDirP(Utility::DirName(path), 0700);

std::ofstream fp(path.CStr(), std::ofstream::out | std::ostream::trunc);
fp << config;
fp.close();

std::unique_ptr<Expression> expr = ConfigCompiler::CompileFile(path, String(), "_api");
// Given that the internal config::Update cluster events are using this as well to create synced runtime objects,
// we don't want to write the config first to the disk and then load and validate it with CompileFile.
// Otherwise, we are forced to remove the newly created file whenever we can't validate, commit or activate it,
// even if something goes terribly wrong. This approach would also have the downside that two cluster events for
// the same object arriving at the same time from two different endpoints would result in two threads simultaneously
// creating and loading the same configuration file - whereby only one of them surpasses the validation process,
// while the other is facing an object `re-definition` error and tries to remove that configuration file it
// mistakenly thinks it has created. As a consequence, an object successfully created by the former is implicitly
// deleted by the latter thread, causing the objects to mysteriously disappear.
std::unique_ptr<Expression> expr = ConfigCompiler::CompileText(path, config, String(), "_api");

try {
ActivationScope ascope;
Expand All @@ -227,8 +230,6 @@ bool ConfigObjectUtility::CreateObject(const Type::Ptr& type, const String& full
Log(LogNotice, "ConfigObjectUtility")
<< "Failed to commit config item '" << fullName << "'. Aborting and removing config path '" << path << "'.";

Utility::Remove(path);

for (const boost::exception_ptr& ex : upq.GetExceptions()) {
errors->Add(DiagnosticInformation(ex, false));

Expand All @@ -250,8 +251,6 @@ bool ConfigObjectUtility::CreateObject(const Type::Ptr& type, const String& full
Log(LogNotice, "ConfigObjectUtility")
<< "Failed to activate config object '" << fullName << "'. Aborting and removing config path '" << path << "'.";

Utility::Remove(path);

for (const boost::exception_ptr& ex : upq.GetExceptions()) {
errors->Add(DiagnosticInformation(ex, false));

Expand All @@ -275,6 +274,13 @@ bool ConfigObjectUtility::CreateObject(const Type::Ptr& type, const String& full
ConfigObject::Ptr obj = ctype->GetObject(fullName);

if (obj) {
// It's now safe to put the config on the disk as the object has surpassed the compiling/validation processes.
Utility::MkDirP(Utility::DirName(path), 0700);

std::ofstream fp(path.CStr(), std::ofstream::out | std::ostream::trunc);
fp << config;
fp.close();

Log(LogInformation, "ConfigObjectUtility")
<< "Created and activated object '" << fullName << "' of type '" << type->GetName() << "'.";
} else {
Expand All @@ -283,8 +289,6 @@ bool ConfigObjectUtility::CreateObject(const Type::Ptr& type, const String& full
}

} catch (const std::exception& ex) {
Utility::Remove(path);

if (errors)
errors->Add(DiagnosticInformation(ex, false));

Expand Down

0 comments on commit 23d30d0

Please sign in to comment.