From 5aacae705fc17d8429b2e1e6d146ae9e218f3cee Mon Sep 17 00:00:00 2001 From: Mattia Codato Date: Mon, 15 Feb 2021 14:04:25 +0100 Subject: [PATCH] Add package parameter in object creation API Allow deletion via API of objects belonging to any package refs #8639 --- lib/remote/apilistener-configsync.cpp | 44 +++++++++++---------------- lib/remote/configobjectutility.cpp | 40 +++++++++++------------- lib/remote/configobjectutility.hpp | 9 +++--- lib/remote/createobjecthandler.cpp | 6 +++- lib/remote/deleteobjecthandler.cpp | 11 ++++++- lib/remote/modifyobjecthandler.cpp | 38 ++++++++++++++--------- 6 files changed, 80 insertions(+), 68 deletions(-) diff --git a/lib/remote/apilistener-configsync.cpp b/lib/remote/apilistener-configsync.cpp index 627141166ea..2bcdc92295c 100644 --- a/lib/remote/apilistener-configsync.cpp +++ b/lib/remote/apilistener-configsync.cpp @@ -110,6 +110,10 @@ Value ApiListener::ConfigUpdateObjectAPIHandler(const MessageOrigin::Ptr& origin bool newObject = false; + String package = "_api"; + if (params->Contains("package")) + package = params->Get("package"); + if (!object && !config.IsEmpty()) { newObject = true; @@ -120,7 +124,8 @@ Value ApiListener::ConfigUpdateObjectAPIHandler(const MessageOrigin::Ptr& origin * Create the config object through our internal API. * IMPORTANT: Pass the origin to prevent cluster sync loops. */ - if (!ConfigObjectUtility::CreateObject(ptype, objName, config, errors, nullptr, origin)) { + if (!ConfigObjectUtility::CreateObject(ptype, objName, config, errors, nullptr, origin, + package)) { Log(LogCritical, "ApiListener") << "Could not create object '" << objName << "':"; @@ -266,12 +271,6 @@ Value ApiListener::ConfigDeleteObjectAPIHandler(const MessageOrigin::Ptr& origin return Empty; } - if (object->GetPackage() != "_api") { - Log(LogCritical, "ApiListener") - << "Could not delete object '" << objName << "': Not created by the API."; - return Empty; - } - Log(LogNotice, "ApiListener") << "Processing config delete" << " from '" << identity << "' (endpoint: '" << endpoint->GetName() << "', zone: '" << endpointZone->GetName() << "')" @@ -311,9 +310,6 @@ void ApiListener::UpdateConfigObject(const ConfigObject::Ptr& object, const Mess } } - if (object->GetPackage() != "_api" && object->GetVersion() == 0) - return; - Dictionary::Ptr params = new Dictionary(); Dictionary::Ptr message = new Dictionary({ @@ -325,27 +321,26 @@ void ApiListener::UpdateConfigObject(const ConfigObject::Ptr& object, const Mess params->Set("name", object->GetName()); params->Set("type", object->GetReflectionType()->GetName()); params->Set("version", object->GetVersion()); + params->Set("package", object->GetPackage()); String zoneName = object->GetZoneName(); if (!zoneName.IsEmpty()) params->Set("zone", zoneName); - if (object->GetPackage() == "_api") { - String file; - - try { - file = ConfigObjectUtility::GetObjectConfigPath(object->GetReflectionType(), object->GetName()); - } catch (const std::exception& ex) { - Log(LogNotice, "ApiListener") - << "Cannot sync object '" << object->GetName() << "': " << ex.what(); - return; - } + String file; - std::ifstream fp(file.CStr(), std::ifstream::binary); - if (!fp) - return; + try { + file = ConfigObjectUtility::GetObjectConfigPath(object->GetReflectionType(), object->GetName(), + object->GetPackage()); + } catch (const std::exception& ex) { + Log(LogNotice, "ApiListener") + << "Cannot sync object '" << object->GetName() << "': " << ex.what(); + return; + } + std::ifstream fp(file.CStr(), std::ifstream::binary); + if (fp) { String content((std::istreambuf_iterator(fp)), std::istreambuf_iterator()); params->Set("config", content); } @@ -396,9 +391,6 @@ void ApiListener::UpdateConfigObject(const ConfigObject::Ptr& object, const Mess void ApiListener::DeleteConfigObject(const ConfigObject::Ptr& object, const MessageOrigin::Ptr& origin, const JsonRpcConnection::Ptr& client) { - if (object->GetPackage() != "_api") - return; - /* only send objects to zones which have access to the object */ if (client) { Zone::Ptr target_zone = client->GetEndpoint()->GetZone(); diff --git a/lib/remote/configobjectutility.cpp b/lib/remote/configobjectutility.cpp index 6ba0e0eccf1..677f5215281 100644 --- a/lib/remote/configobjectutility.cpp +++ b/lib/remote/configobjectutility.cpp @@ -18,24 +18,24 @@ using namespace icinga; -String ConfigObjectUtility::GetConfigDir() +String ConfigObjectUtility::GetConfigDir(const String& package) { - String prefix = ConfigPackageUtility::GetPackageDir() + "/_api/"; - String activeStage = ConfigPackageUtility::GetActiveStage("_api"); + String prefix = ConfigPackageUtility::GetPackageDir() + "/" + package + "/"; + String activeStage = ConfigPackageUtility::GetActiveStage(package); if (activeStage.IsEmpty()) - RepairPackage("_api"); + RepairPackage(package); return prefix + activeStage; } -String ConfigObjectUtility::GetObjectConfigPath(const Type::Ptr& type, const String& fullName) +String ConfigObjectUtility::GetObjectConfigPath(const Type::Ptr& type, const String& fullName, const String& package) { String typeDir = type->GetPluralName(); boost::algorithm::to_lower(typeDir); /* This may throw an exception the caller above must handle. */ - String prefix = GetConfigDir(); + String prefix = GetConfigDir(package); auto old (prefix + "/conf.d/" + typeDir + "/" + EscapeName(fullName) + ".conf"); @@ -84,13 +84,10 @@ void ConfigObjectUtility::RepairPackage(const String& package) } } -void ConfigObjectUtility::CreateStorage() +void ConfigObjectUtility::CreateStorage(const String& package) { std::unique_lock lock(ConfigPackageUtility::GetStaticPackageMutex()); - /* For now, we only use _api as our creation target. */ - String package = "_api"; - if (!ConfigPackageUtility::PackageExists(package)) { Log(LogNotice, "ConfigObjectUtility") << "Package " << package << " doesn't exist yet, creating it."; @@ -155,9 +152,10 @@ String ConfigObjectUtility::CreateObjectConfig(const Type::Ptr& type, const Stri } bool ConfigObjectUtility::CreateObject(const Type::Ptr& type, const String& fullName, - const String& config, const Array::Ptr& errors, const Array::Ptr& diagnosticInformation, const Value& cookie) + const String& config, const Array::Ptr& errors, const Array::Ptr& diagnosticInformation, const Value& cookie, + const String& package) { - CreateStorage(); + CreateStorage(package); { auto configType (dynamic_cast(type.get())); @@ -171,7 +169,7 @@ bool ConfigObjectUtility::CreateObject(const Type::Ptr& type, const String& full String path; try { - path = GetObjectConfigPath(type, fullName); + path = GetObjectConfigPath(type, fullName, package); } catch (const std::exception& ex) { errors->Add("Config package broken: " + DiagnosticInformation(ex, false)); return false; @@ -183,7 +181,7 @@ bool ConfigObjectUtility::CreateObject(const Type::Ptr& type, const String& full fp << config; fp.close(); - std::unique_ptr expr = ConfigCompiler::CompileFile(path, String(), "_api"); + std::unique_ptr expr = ConfigCompiler::CompileFile(path, String(), package); try { ActivationScope ascope; @@ -335,12 +333,17 @@ bool ConfigObjectUtility::DeleteObjectHelper(const ConfigObject::Ptr& object, bo String path; try { - path = GetObjectConfigPath(object->GetReflectionType(), name); + path = GetObjectConfigPath(object->GetReflectionType(), name, object->GetPackage()); } catch (const std::exception& ex) { errors->Add("Config package broken: " + DiagnosticInformation(ex, false)); return false; } + /* + * This is currently not throwing any error if the config file doesn't exist. + * This can happen if it is called for an object that was not created by the API. + * https://github.com/Icinga/icinga2/issues/8639 + */ Utility::Remove(path); Log(LogInformation, "ConfigObjectUtility") @@ -352,12 +355,5 @@ bool ConfigObjectUtility::DeleteObjectHelper(const ConfigObject::Ptr& object, bo bool ConfigObjectUtility::DeleteObject(const ConfigObject::Ptr& object, bool cascade, const Array::Ptr& errors, const Array::Ptr& diagnosticInformation, const Value& cookie) { - if (object->GetPackage() != "_api") { - if (errors) - errors->Add("Object cannot be deleted because it was not created using the API."); - - return false; - } - return DeleteObjectHelper(object, cascade, errors, diagnosticInformation, cookie); } diff --git a/lib/remote/configobjectutility.hpp b/lib/remote/configobjectutility.hpp index f383a211b20..499eb0f4d27 100644 --- a/lib/remote/configobjectutility.hpp +++ b/lib/remote/configobjectutility.hpp @@ -21,16 +21,17 @@ class ConfigObjectUtility { public: - static String GetConfigDir(); - static String GetObjectConfigPath(const Type::Ptr& type, const String& fullName); + static String GetConfigDir(const String &package = "_api"); + static String GetObjectConfigPath(const Type::Ptr& type, const String& fullName, const String &package = "_api"); static void RepairPackage(const String& package); - static void CreateStorage(); + static void CreateStorage(const String& package = "_api"); static String CreateObjectConfig(const Type::Ptr& type, const String& fullName, bool ignoreOnError, const Array::Ptr& templates, const Dictionary::Ptr& attrs); static bool CreateObject(const Type::Ptr& type, const String& fullName, - const String& config, const Array::Ptr& errors, const Array::Ptr& diagnosticInformation, const Value& cookie = Empty); + const String& config, const Array::Ptr& errors, const Array::Ptr& diagnosticInformation, const Value& cookie = Empty, + const String& package = "_api"); static bool DeleteObject(const ConfigObject::Ptr& object, bool cascade, const Array::Ptr& errors, const Array::Ptr& diagnosticInformation, const Value& cookie = Empty); diff --git a/lib/remote/createobjecthandler.cpp b/lib/remote/createobjecthandler.cpp index c01b2364156..d85a50bda5d 100644 --- a/lib/remote/createobjecthandler.cpp +++ b/lib/remote/createobjecthandler.cpp @@ -94,6 +94,10 @@ bool CreateObjectHandler::HandleRequest( if (params) verbose = HttpUtility::GetLastParameter(params, "verbose"); + String package = "_api"; + if (params->Contains("package")) + package = HttpUtility::GetLastParameter(params, "package"); + /* Object creation can cause multiple errors and optionally diagnostic information. * We can't use SendJsonError() here. */ @@ -116,7 +120,7 @@ bool CreateObjectHandler::HandleRequest( return true; } - if (!ConfigObjectUtility::CreateObject(type, name, config, errors, diagnosticInformation)) { + if (!ConfigObjectUtility::CreateObject(type, name, config, errors, diagnosticInformation, nullptr, package)) { result1->Set("errors", errors); result1->Set("code", 500); result1->Set("status", "Object could not be created."); diff --git a/lib/remote/deleteobjecthandler.cpp b/lib/remote/deleteobjecthandler.cpp index 2edb0e45519..bdd2daf8c48 100644 --- a/lib/remote/deleteobjecthandler.cpp +++ b/lib/remote/deleteobjecthandler.cpp @@ -65,6 +65,10 @@ bool DeleteObjectHandler::HandleRequest( bool cascade = HttpUtility::GetLastParameter(params, "cascade"); bool verbose = HttpUtility::GetLastParameter(params, "verbose"); + String package = HttpUtility::GetLastParameter(params, "package"); + if (package == Empty) { + package = "_api"; + } ArrayData results; @@ -76,7 +80,12 @@ bool DeleteObjectHandler::HandleRequest( Array::Ptr errors = new Array(); Array::Ptr diagnosticInformation = new Array(); - if (!ConfigObjectUtility::DeleteObject(obj, cascade, errors, diagnosticInformation)) { + if (obj->GetPackage() != package) { + code = 500; + status = "Object could not be deleted because it was create in package '" + obj->GetPackage() + + "' and not in package: '" + package + "'."; + success = false; + } else if (!ConfigObjectUtility::DeleteObject(obj, cascade, errors, diagnosticInformation)) { code = 500; status = "Object could not be deleted."; success = false; diff --git a/lib/remote/modifyobjecthandler.cpp b/lib/remote/modifyobjecthandler.cpp index cc008b90c53..2439658bc65 100644 --- a/lib/remote/modifyobjecthandler.cpp +++ b/lib/remote/modifyobjecthandler.cpp @@ -77,6 +77,10 @@ bool ModifyObjectHandler::HandleRequest( if (params) verbose = HttpUtility::GetLastParameter(params, "verbose"); + String package = "_api"; + if (params->Contains("package")) + package = HttpUtility::GetLastParameter(params, "package"); + ArrayData results; for (const ConfigObject::Ptr& obj : objs) { @@ -87,23 +91,29 @@ bool ModifyObjectHandler::HandleRequest( String key; - try { - if (attrs) { - ObjectLock olock(attrs); - for (const Dictionary::Pair& kv : attrs) { - key = kv.first; - obj->ModifyAttribute(kv.first, kv.second); + if (obj->GetPackage() != package) { + result1->Set("code", 500); + result1->Set("status", "Object cannot be modified because it was create in package '" + + obj->GetPackage() + "' and not in package: '" + package + "'."); + } else { + try { + if (attrs) { + ObjectLock olock(attrs); + for (const Dictionary::Pair &kv : attrs) { + key = kv.first; + obj->ModifyAttribute(kv.first, kv.second); + } } - } - result1->Set("code", 200); - result1->Set("status", "Attributes updated."); - } catch (const std::exception& ex) { - result1->Set("code", 500); - result1->Set("status", "Attribute '" + key + "' could not be set: " + DiagnosticInformation(ex, false)); + result1->Set("code", 200); + result1->Set("status", "Attributes updated."); + } catch (const std::exception &ex) { + result1->Set("code", 500); + result1->Set("status", "Attribute '" + key + "' could not be set: " + DiagnosticInformation(ex, false)); - if (verbose) - result1->Set("diagnostic_information", DiagnosticInformation(ex)); + if (verbose) + result1->Set("diagnostic_information", DiagnosticInformation(ex)); + } } results.push_back(std::move(result1));