Skip to content

Commit

Permalink
Fix config sync after freezing namespaces
Browse files Browse the repository at this point in the history
This was accidentally broken by #9627 because during config sync, a config
validation happens that uses `--define System.ZonesStageVarDir=...` which fails
on the now frozen namespace.

This commit changes this to use `Internal.ZonesStageVarDir` instead. After all,
this is used for internal functionality, users should not directly interact
with this flag.

Additionally, it no longer freezes the `Internal` namespace which actually
allows using `Internal.ZonesStageVarDir` in the first place. This also fixes
`--define Internal.Debug*` which was also broken by said PR. Freezing of the
`Internal` namespace is not necessary for performance reasons as it's not
searched implicitly (for example when accessing `globals.x`) and should users
actually interact with it, they should know by that name that they are on their
own.
  • Loading branch information
julianbrost committed Feb 1, 2023
1 parent d2fb8a9 commit fd1aa73
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 9 deletions.
6 changes: 2 additions & 4 deletions lib/base/scriptframe.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ using namespace icinga;

boost::thread_specific_ptr<std::stack<ScriptFrame *> > ScriptFrame::m_ScriptFrames;

static Namespace::Ptr l_SystemNS, l_StatsNS, l_InternalNS;
static Namespace::Ptr l_SystemNS, l_StatsNS;

/* Ensure that this gets called with highest priority
* and wins against other static initializers in lib/icinga, etc.
Expand All @@ -36,14 +36,12 @@ INITIALIZE_ONCE_WITH_PRIORITY([]() {
l_StatsNS = new Namespace(true);
globalNS->Set("StatsFunctions", l_StatsNS, true);

l_InternalNS = new Namespace(true);
globalNS->Set("Internal", l_InternalNS, true);
globalNS->Set("Internal", new Namespace(true), true);
}, InitializePriority::CreateNamespaces);

INITIALIZE_ONCE_WITH_PRIORITY([]() {
l_SystemNS->Freeze();
l_StatsNS->Freeze();
l_InternalNS->Freeze();
}, InitializePriority::FreezeNamespaces);

ScriptFrame::ScriptFrame(bool allocLocals)
Expand Down
9 changes: 6 additions & 3 deletions lib/cli/daemonutility.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,9 @@ bool DaemonUtility::ValidateConfigFiles(const std::vector<std::string>& configs,
Namespace::Ptr systemNS = ScriptGlobal::Get("System");
VERIFY(systemNS);

Namespace::Ptr internalNS = ScriptGlobal::Get("Internal");
VERIFY(internalNS);

if (!objectsFile.IsEmpty())
ConfigCompilerContext::GetInstance()->OpenObjectsFile(objectsFile);

Expand All @@ -134,7 +137,7 @@ bool DaemonUtility::ValidateConfigFiles(const std::vector<std::string>& configs,
success = true;

/* Only load zone directory if we're not in staging validation. */
if (!systemNS->Contains("ZonesStageVarDir")) {
if (!internalNS->Contains("ZonesStageVarDir")) {
String zonesEtcDir = Configuration::ZonesDir;
if (!zonesEtcDir.IsEmpty() && Utility::PathExists(zonesEtcDir)) {
std::set<String> zoneEtcDirs;
Expand Down Expand Up @@ -177,8 +180,8 @@ bool DaemonUtility::ValidateConfigFiles(const std::vector<std::string>& configs,
String zonesVarDir = Configuration::DataDir + "/api/zones";

/* Cluster config sync stage validation needs this. */
if (systemNS->Contains("ZonesStageVarDir")) {
zonesVarDir = systemNS->Get("ZonesStageVarDir");
if (internalNS->Contains("ZonesStageVarDir")) {
zonesVarDir = internalNS->Get("ZonesStageVarDir");

Log(LogNotice, "DaemonUtility")
<< "Overriding zones var directory with '" << zonesVarDir << "' for cluster config sync staging.";
Expand Down
4 changes: 2 additions & 2 deletions lib/remote/apilistener-filesync.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,7 @@ void ApiListener::HandleConfigUpdate(const MessageOrigin::Ptr& origin, const Dic
}

/**
* Spawns a new validation process with 'System.ZonesStageVarDir' set to override the config validation zone dirs with
* Spawns a new validation process with 'Internal.ZonesStageVarDir' set to override the config validation zone dirs with
* our current stage. Then waits for the validation result and if it was successful, the configuration is copied from
* stage to production and a restart is triggered. On validation failure, there is no restart and this is logged.
*
Expand Down Expand Up @@ -584,7 +584,7 @@ void ApiListener::TryActivateZonesStage(const std::vector<String>& relativePaths

// Set the ZonesStageDir. This creates our own local chroot without any additional automated zone includes.
args->Add("--define");
args->Add("System.ZonesStageVarDir=" + GetApiZonesStageDir());
args->Add("Internal.ZonesStageVarDir=" + GetApiZonesStageDir());

Process::Ptr process = new Process(Process::PrepareCommand(args));
process->SetTimeout(Application::GetReloadTimeout());
Expand Down

0 comments on commit fd1aa73

Please sign in to comment.