From 1dd8409691810152d548c4e11f125b4bcf24f448 Mon Sep 17 00:00:00 2001 From: Michael Friedrich Date: Thu, 5 Apr 2018 17:17:06 +0200 Subject: [PATCH 01/15] Check for verbose error handling in SendJsonError() --- lib/remote/httputility.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/remote/httputility.cpp b/lib/remote/httputility.cpp index 7b0fa8e3c..2788a03b2 100644 --- a/lib/remote/httputility.cpp +++ b/lib/remote/httputility.cpp @@ -89,11 +89,18 @@ void HttpUtility::SendJsonError(HttpResponse& response, const Dictionary::Ptr& p response.SetStatus(code, HttpUtility::GetErrorNameByCode(code)); result->Set("error", code); + bool verbose = false; + + if (params) + verbose = HttpUtility::GetLastParameter(params, "verbose"); + if (!info.IsEmpty()) result->Set("status", info); - if (!diagnosticInformation.IsEmpty()) - result->Set("diagnostic information", diagnosticInformation); + if (verbose) { + if (!diagnosticInformation.IsEmpty()) + result->Set("diagnostic_information", diagnosticInformation); + } HttpUtility::SendJsonBody(response, params, result); } From 75c5e6f6b0ac13e51a726d0a0a0a4f32f96d523a Mon Sep 17 00:00:00 2001 From: Michael Friedrich Date: Thu, 5 Apr 2018 17:17:30 +0200 Subject: [PATCH 02/15] Enhance error handling in config stages handler --- lib/remote/configstageshandler.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/remote/configstageshandler.cpp b/lib/remote/configstageshandler.cpp index c564a8a77..32d56e39e 100644 --- a/lib/remote/configstageshandler.cpp +++ b/lib/remote/configstageshandler.cpp @@ -59,10 +59,10 @@ void ConfigStagesHandler::HandleGet(const ApiUser::Ptr& user, HttpRequest& reque String stageName = HttpUtility::GetLastParameter(params, "stage"); if (!ConfigPackageUtility::ValidateName(packageName)) - return HttpUtility::SendJsonError(response, params, 400, "Invalid package name."); + return HttpUtility::SendJsonError(response, params, 400, "Invalid package name '" + packageName + "'."); if (!ConfigPackageUtility::ValidateName(stageName)) - return HttpUtility::SendJsonError(response, params, 400, "Invalid stage name."); + return HttpUtility::SendJsonError(response, params, 400, "Invalid stage name '" + stageName + "'."); ArrayData results; @@ -95,7 +95,7 @@ void ConfigStagesHandler::HandlePost(const ApiUser::Ptr& user, HttpRequest& requ String packageName = HttpUtility::GetLastParameter(params, "package"); if (!ConfigPackageUtility::ValidateName(packageName)) - return HttpUtility::SendJsonError(response, params, 400, "Invalid package name."); + return HttpUtility::SendJsonError(response, params, 400, "Invalid package name '" + packageName + "'."); bool reload = true; if (params->Contains("reload")) @@ -116,8 +116,8 @@ void ConfigStagesHandler::HandlePost(const ApiUser::Ptr& user, HttpRequest& requ ConfigPackageUtility::AsyncTryActivateStage(packageName, stageName, reload); } catch (const std::exception& ex) { return HttpUtility::SendJsonError(response, params, 500, - "Stage creation failed.", - HttpUtility::GetLastParameter(params, "verboseErrors") ? DiagnosticInformation(ex) : ""); + "Stage creation failed for '" + stageName + "'.", + DiagnosticInformation(ex, false)); } @@ -153,17 +153,17 @@ void ConfigStagesHandler::HandleDelete(const ApiUser::Ptr& user, HttpRequest& re String stageName = HttpUtility::GetLastParameter(params, "stage"); if (!ConfigPackageUtility::ValidateName(packageName)) - return HttpUtility::SendJsonError(response, params, 400, "Invalid package name."); + return HttpUtility::SendJsonError(response, params, 400, "Invalid package name '" + packageName + "'."); if (!ConfigPackageUtility::ValidateName(stageName)) - return HttpUtility::SendJsonError(response, params, 400, "Invalid stage name."); + return HttpUtility::SendJsonError(response, params, 400, "Invalid stage name '" + stageName + "'."); try { ConfigPackageUtility::DeleteStage(packageName, stageName); } catch (const std::exception& ex) { return HttpUtility::SendJsonError(response, params, 500, - "Failed to delete stage.", - HttpUtility::GetLastParameter(params, "verboseErrors") ? DiagnosticInformation(ex) : ""); + "Failed to delete stage '" + stageName + "' in package '" + packageName + "'.", + DiagnosticInformation(ex, false)); } Dictionary::Ptr result1 = new Dictionary({ From 7f015c0d2fefe1d1600201fd06a7b4228ecfe6b9 Mon Sep 17 00:00:00 2001 From: Michael Friedrich Date: Thu, 5 Apr 2018 17:21:14 +0200 Subject: [PATCH 03/15] Enhance error handling in config packages handler --- lib/remote/configpackageshandler.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/remote/configpackageshandler.cpp b/lib/remote/configpackageshandler.cpp index e3e54237b..7ff7567ae 100644 --- a/lib/remote/configpackageshandler.cpp +++ b/lib/remote/configpackageshandler.cpp @@ -54,7 +54,7 @@ void ConfigPackagesHandler::HandleGet(const ApiUser::Ptr& user, HttpRequest& req packages = ConfigPackageUtility::GetPackages(); } catch (const std::exception& ex) { HttpUtility::SendJsonError(response, params, 500, "Could not retrieve packages.", - HttpUtility::GetLastParameter(params, "verboseErrors") ? DiagnosticInformation(ex) : ""); + DiagnosticInformation(ex, false)); return; } @@ -89,7 +89,7 @@ void ConfigPackagesHandler::HandlePost(const ApiUser::Ptr& user, HttpRequest& re String packageName = HttpUtility::GetLastParameter(params, "package"); if (!ConfigPackageUtility::ValidateName(packageName)) { - HttpUtility::SendJsonError(response, params, 400, "Invalid package name."); + HttpUtility::SendJsonError(response, params, 400, "Invalid package name '" + packageName + "'."); return; } @@ -97,8 +97,8 @@ void ConfigPackagesHandler::HandlePost(const ApiUser::Ptr& user, HttpRequest& re boost::mutex::scoped_lock lock(ConfigPackageUtility::GetStaticMutex()); ConfigPackageUtility::CreatePackage(packageName); } catch (const std::exception& ex) { - HttpUtility::SendJsonError(response, params, 500, "Could not create package.", - HttpUtility::GetLastParameter(params, "verboseErrors") ? DiagnosticInformation(ex) : ""); + HttpUtility::SendJsonError(response, params, 500, "Could not create package '" + packageName + "'.", + DiagnosticInformation(ex, false)); return; } @@ -125,7 +125,7 @@ void ConfigPackagesHandler::HandleDelete(const ApiUser::Ptr& user, HttpRequest& String packageName = HttpUtility::GetLastParameter(params, "package"); if (!ConfigPackageUtility::ValidateName(packageName)) { - HttpUtility::SendJsonError(response, params, 400, "Invalid package name."); + HttpUtility::SendJsonError(response, params, 400, "Invalid package name '" + packageName + "'."); return; } From de2d18d85d6187047bbee305b2f6cf99541c1f8e Mon Sep 17 00:00:00 2001 From: Michael Friedrich Date: Thu, 5 Apr 2018 17:22:56 +0200 Subject: [PATCH 04/15] Enhance error handling in type query handler --- lib/remote/typequeryhandler.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/remote/typequeryhandler.cpp b/lib/remote/typequeryhandler.cpp index e3f0c7257..fe4703f60 100644 --- a/lib/remote/typequeryhandler.cpp +++ b/lib/remote/typequeryhandler.cpp @@ -91,7 +91,7 @@ bool TypeQueryHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& requ } catch (const std::exception& ex) { HttpUtility::SendJsonError(response, params, 404, "No objects found.", - HttpUtility::GetLastParameter(params, "verboseErrors") ? DiagnosticInformation(ex) : ""); + DiagnosticInformation(ex)); return true; } From 36cdf8a0d2ffa03e2c770f2b94b9932f6d41bed1 Mon Sep 17 00:00:00 2001 From: Michael Friedrich Date: Fri, 6 Apr 2018 09:53:54 +0200 Subject: [PATCH 05/15] More refactoring of config packages errors --- lib/remote/configpackageshandler.cpp | 31 +++++++++++++--------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/lib/remote/configpackageshandler.cpp b/lib/remote/configpackageshandler.cpp index 7ff7567ae..2ecf78d4c 100644 --- a/lib/remote/configpackageshandler.cpp +++ b/lib/remote/configpackageshandler.cpp @@ -54,7 +54,7 @@ void ConfigPackagesHandler::HandleGet(const ApiUser::Ptr& user, HttpRequest& req packages = ConfigPackageUtility::GetPackages(); } catch (const std::exception& ex) { HttpUtility::SendJsonError(response, params, 500, "Could not retrieve packages.", - DiagnosticInformation(ex, false)); + DiagnosticInformation(ex)); return; } @@ -98,12 +98,13 @@ void ConfigPackagesHandler::HandlePost(const ApiUser::Ptr& user, HttpRequest& re ConfigPackageUtility::CreatePackage(packageName); } catch (const std::exception& ex) { HttpUtility::SendJsonError(response, params, 500, "Could not create package '" + packageName + "'.", - DiagnosticInformation(ex, false)); + DiagnosticInformation(ex)); return; } Dictionary::Ptr result1 = new Dictionary({ { "code", 200 }, + { "package", packageName }, { "status", "Created package." } }); @@ -129,27 +130,23 @@ void ConfigPackagesHandler::HandleDelete(const ApiUser::Ptr& user, HttpRequest& return; } - int code = 200; - String status = "Deleted package."; - DictionaryData result1; - try { ConfigPackageUtility::DeletePackage(packageName); } catch (const std::exception& ex) { - code = 500; - status = "Failed to delete package."; - if (HttpUtility::GetLastParameter(params, "verboseErrors")) - result1.emplace_back("diagnostic information", DiagnosticInformation(ex)); + HttpUtility::SendJsonError(response, params, 500, "Failed to delete package '" + packageName + "'.", + DiagnosticInformation(ex)); } - result1.emplace_back("package", packageName); - result1.emplace_back("code", code); - result1.emplace_back("status", status); - - Dictionary::Ptr result = new Dictionary({ - { "results", new Array({ new Dictionary(std::move(result1)) }) } + Dictionary::Ptr result1 = new Dictionary({ + { "code", 200 }, + { "package", packageName }, + { "status", "Created package." } }); - response.SetStatus(code, (code == 200) ? "OK" : "Internal Server Error"); + Dictionary::Ptr result = new Dictionary({ + { "results", new Array({ result1 }) } + }); + + response.SetStatus(200, "OK"); HttpUtility::SendJsonBody(response, params, result); } From 4bf731fc16b55a1a4b7d76474906a905866c618a Mon Sep 17 00:00:00 2001 From: Michael Friedrich Date: Fri, 6 Apr 2018 10:13:08 +0200 Subject: [PATCH 06/15] More config stages refactoring --- lib/remote/configstageshandler.cpp | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/lib/remote/configstageshandler.cpp b/lib/remote/configstageshandler.cpp index 32d56e39e..673e134a9 100644 --- a/lib/remote/configstageshandler.cpp +++ b/lib/remote/configstageshandler.cpp @@ -98,6 +98,7 @@ void ConfigStagesHandler::HandlePost(const ApiUser::Ptr& user, HttpRequest& requ return HttpUtility::SendJsonError(response, params, 400, "Invalid package name '" + packageName + "'."); bool reload = true; + if (params->Contains("reload")) reload = HttpUtility::GetLastParameter(params, "reload"); @@ -116,13 +117,17 @@ void ConfigStagesHandler::HandlePost(const ApiUser::Ptr& user, HttpRequest& requ ConfigPackageUtility::AsyncTryActivateStage(packageName, stageName, reload); } catch (const std::exception& ex) { return HttpUtility::SendJsonError(response, params, 500, - "Stage creation failed for '" + stageName + "'.", - DiagnosticInformation(ex, false)); + "Stage creation failed.", + DiagnosticInformation(ex)); } String responseStatus = "Created stage. "; - responseStatus += (reload ? " Icinga2 will reload." : " Icinga2 reload skipped."); + + if (reload) + responseStatus += "Reload triggered."; + else + responseStatus += "Reload skipped."; Dictionary::Ptr result1 = new Dictionary({ { "package", packageName }, @@ -163,11 +168,13 @@ void ConfigStagesHandler::HandleDelete(const ApiUser::Ptr& user, HttpRequest& re } catch (const std::exception& ex) { return HttpUtility::SendJsonError(response, params, 500, "Failed to delete stage '" + stageName + "' in package '" + packageName + "'.", - DiagnosticInformation(ex, false)); + DiagnosticInformation(ex)); } Dictionary::Ptr result1 = new Dictionary({ { "code", 200 }, + { "package", packageName }, + { "stage", stageName }, { "status", "Stage deleted." } }); From a00197e919040b8a3e86cbaa93409fa0648e19ee Mon Sep 17 00:00:00 2001 From: Michael Friedrich Date: Fri, 6 Apr 2018 10:30:27 +0200 Subject: [PATCH 07/15] Refactor actions error messages --- lib/remote/actionshandler.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/lib/remote/actionshandler.cpp b/lib/remote/actionshandler.cpp index a8b31fc10..a8a87f5d9 100644 --- a/lib/remote/actionshandler.cpp +++ b/lib/remote/actionshandler.cpp @@ -62,7 +62,7 @@ bool ActionsHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& reques } catch (const std::exception& ex) { HttpUtility::SendJsonError(response, params, 404, "No objects found.", - HttpUtility::GetLastParameter(params, "verboseErrors") ? DiagnosticInformation(ex) : ""); + DiagnosticInformation(ex)); return true; } } else { @@ -75,6 +75,11 @@ bool ActionsHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& reques Log(LogNotice, "ApiActionHandler") << "Running action " << actionName; + bool verbose = false; + + if (params) + verbose = HttpUtility::GetLastParameter(params, "verbose"); + for (const ConfigObject::Ptr& obj : objs) { try { results.emplace_back(action->Invoke(obj, params)); @@ -84,8 +89,9 @@ bool ActionsHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& reques { "status", "Action execution failed: '" + DiagnosticInformation(ex, false) + "'." } }); - if (HttpUtility::GetLastParameter(params, "verboseErrors")) - fail->Set("diagnostic information", DiagnosticInformation(ex)); + /* Exception for actions. Normally we would handle this inside SendJsonError(). */ + if (verbose) + fail->Set("diagnostic_information", DiagnosticInformation(ex)); results.emplace_back(std::move(fail)); } From c4a6ab02115223f8a6238adb3833fe3de3269ea9 Mon Sep 17 00:00:00 2001 From: Michael Friedrich Date: Fri, 6 Apr 2018 12:26:49 +0200 Subject: [PATCH 08/15] Add diagnostic_information as verbose error to config object handlers --- lib/icinga/comment.cpp | 4 +-- lib/icinga/downtime.cpp | 4 +-- lib/remote/apilistener-configsync.cpp | 11 ++++---- lib/remote/configobjectutility.cpp | 39 ++++++++++++++++++--------- lib/remote/configobjectutility.hpp | 8 +++--- lib/remote/createobjecthandler.cpp | 20 ++++++++++++-- lib/remote/deleteobjecthandler.cpp | 15 ++++++++--- lib/remote/modifyobjecthandler.cpp | 10 ++++++- 8 files changed, 78 insertions(+), 33 deletions(-) diff --git a/lib/icinga/comment.cpp b/lib/icinga/comment.cpp index c23e14619..9931b42bd 100644 --- a/lib/icinga/comment.cpp +++ b/lib/icinga/comment.cpp @@ -182,7 +182,7 @@ String Comment::AddComment(const Checkable::Ptr& checkable, CommentType entryTyp Array::Ptr errors = new Array(); - if (!ConfigObjectUtility::CreateObject(Comment::TypeInstance, fullName, config, errors)) { + if (!ConfigObjectUtility::CreateObject(Comment::TypeInstance, fullName, config, errors, nullptr)) { ObjectLock olock(errors); for (const String& error : errors) { Log(LogCritical, "Comment", error); @@ -214,7 +214,7 @@ void Comment::RemoveComment(const String& id, const MessageOrigin::Ptr& origin) Array::Ptr errors = new Array(); - if (!ConfigObjectUtility::DeleteObject(comment, false, errors)) { + if (!ConfigObjectUtility::DeleteObject(comment, false, errors, nullptr)) { ObjectLock olock(errors); for (const String& error : errors) { Log(LogCritical, "Comment", error); diff --git a/lib/icinga/downtime.cpp b/lib/icinga/downtime.cpp index a52358359..73e41c2a8 100644 --- a/lib/icinga/downtime.cpp +++ b/lib/icinga/downtime.cpp @@ -256,7 +256,7 @@ String Downtime::AddDowntime(const Checkable::Ptr& checkable, const String& auth Array::Ptr errors = new Array(); - if (!ConfigObjectUtility::CreateObject(Downtime::TypeInstance, fullName, config, errors)) { + if (!ConfigObjectUtility::CreateObject(Downtime::TypeInstance, fullName, config, errors, nullptr)) { ObjectLock olock(errors); for (const String& error : errors) { Log(LogCritical, "Downtime", error); @@ -309,7 +309,7 @@ void Downtime::RemoveDowntime(const String& id, bool cancelled, bool expired, co Array::Ptr errors = new Array(); - if (!ConfigObjectUtility::DeleteObject(downtime, false, errors)) { + if (!ConfigObjectUtility::DeleteObject(downtime, false, errors, nullptr)) { ObjectLock olock(errors); for (const String& error : errors) { Log(LogCritical, "Downtime", error); diff --git a/lib/remote/apilistener-configsync.cpp b/lib/remote/apilistener-configsync.cpp index 067e2819c..12c91065e 100644 --- a/lib/remote/apilistener-configsync.cpp +++ b/lib/remote/apilistener-configsync.cpp @@ -116,15 +116,14 @@ Value ApiListener::ConfigUpdateObjectAPIHandler(const MessageOrigin::Ptr& origin /* object does not exist, create it through the API */ Array::Ptr errors = new Array(); - if (!ConfigObjectUtility::CreateObject(ptype, - objName, config, errors)) { + if (!ConfigObjectUtility::CreateObject(ptype, objName, config, errors, nullptr)) { Log(LogCritical, "ApiListener") << "Could not create object '" << objName << "':"; - ObjectLock olock(errors); + ObjectLock olock(errors); for (const String& error : errors) { - Log(LogCritical, "ApiListener", error); - } + Log(LogCritical, "ApiListener", error); + } return Empty; } @@ -256,7 +255,7 @@ Value ApiListener::ConfigDeleteObjectAPIHandler(const MessageOrigin::Ptr& origin Array::Ptr errors = new Array(); - if (!ConfigObjectUtility::DeleteObject(object, true, errors)) { + if (!ConfigObjectUtility::DeleteObject(object, true, errors, nullptr)) { Log(LogCritical, "ApiListener", "Could not delete object:"); ObjectLock olock(errors); diff --git a/lib/remote/configobjectutility.cpp b/lib/remote/configobjectutility.cpp index 38fda3824..3ad88c675 100644 --- a/lib/remote/configobjectutility.cpp +++ b/lib/remote/configobjectutility.cpp @@ -98,7 +98,7 @@ 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 String& config, const Array::Ptr& errors, const Array::Ptr& diagnosticInformation) { { boost::mutex::scoped_lock lock(ConfigPackageUtility::GetStaticMutex()); @@ -114,7 +114,7 @@ bool ConfigObjectUtility::CreateObject(const Type::Ptr& type, const String& full Utility::MkDirP(Utility::DirName(path), 0700); if (Utility::PathExists(path)) { - errors->Add("Configuration file '" + path + "' already exists."); + errors->Add("Cannot create object '" + fullName + "'. Configuration file '" + path + "' already exists."); return false; } @@ -144,7 +144,10 @@ bool ConfigObjectUtility::CreateObject(const Type::Ptr& type, const String& full } for (const boost::exception_ptr& ex : upq.GetExceptions()) { - errors->Add(DiagnosticInformation(ex)); + errors->Add(DiagnosticInformation(ex, false)); + + if (diagnosticInformation) + diagnosticInformation->Add(DiagnosticInformation(ex)); } } @@ -161,7 +164,10 @@ bool ConfigObjectUtility::CreateObject(const Type::Ptr& type, const String& full } if (errors) - errors->Add(DiagnosticInformation(ex)); + errors->Add(DiagnosticInformation(ex, false)); + + if (diagnosticInformation) + diagnosticInformation->Add(DiagnosticInformation(ex)); return false; } @@ -169,17 +175,21 @@ bool ConfigObjectUtility::CreateObject(const Type::Ptr& type, const String& full return true; } -bool ConfigObjectUtility::DeleteObjectHelper(const ConfigObject::Ptr& object, bool cascade, const Array::Ptr& errors) +bool ConfigObjectUtility::DeleteObjectHelper(const ConfigObject::Ptr& object, bool cascade, + const Array::Ptr& errors, const Array::Ptr& diagnosticInformation) { std::vector parents = DependencyGraph::GetParents(object); Type::Ptr type = object->GetReflectionType(); + String name = object->GetName(); + if (!parents.empty() && !cascade) { - if (errors) - errors->Add("Object '" + object->GetName() + "' of type '" + type->GetName() + + if (errors) { + errors->Add("Object '" + name + "' of type '" + type->GetName() + "' cannot be deleted because other objects depend on it. " "Use cascading delete to delete it anyway."); + } return false; } @@ -190,10 +200,10 @@ bool ConfigObjectUtility::DeleteObjectHelper(const ConfigObject::Ptr& object, bo if (!parentObj) continue; - DeleteObjectHelper(parentObj, cascade, errors); + DeleteObjectHelper(parentObj, cascade, errors, diagnosticInformation); } - ConfigItem::Ptr item = ConfigItem::GetByTypeAndName(type, object->GetName()); + ConfigItem::Ptr item = ConfigItem::GetByTypeAndName(type, name); try { /* mark this object for cluster delete event */ @@ -208,12 +218,15 @@ bool ConfigObjectUtility::DeleteObjectHelper(const ConfigObject::Ptr& object, bo } catch (const std::exception& ex) { if (errors) - errors->Add(DiagnosticInformation(ex)); + errors->Add(DiagnosticInformation(ex, false)); + + if (diagnosticInformation) + diagnosticInformation->Add(DiagnosticInformation(ex)); return false; } - String path = GetObjectConfigPath(object->GetReflectionType(), object->GetName()); + String path = GetObjectConfigPath(object->GetReflectionType(), name); if (Utility::PathExists(path)) { if (unlink(path.CStr()) < 0 && errno != ENOENT) { @@ -227,7 +240,7 @@ bool ConfigObjectUtility::DeleteObjectHelper(const ConfigObject::Ptr& object, bo return true; } -bool ConfigObjectUtility::DeleteObject(const ConfigObject::Ptr& object, bool cascade, const Array::Ptr& errors) +bool ConfigObjectUtility::DeleteObject(const ConfigObject::Ptr& object, bool cascade, const Array::Ptr& errors, const Array::Ptr& diagnosticInformation) { if (object->GetPackage() != "_api") { if (errors) @@ -236,5 +249,5 @@ bool ConfigObjectUtility::DeleteObject(const ConfigObject::Ptr& object, bool cas return false; } - return DeleteObjectHelper(object, cascade, errors); + return DeleteObjectHelper(object, cascade, errors, diagnosticInformation); } diff --git a/lib/remote/configobjectutility.hpp b/lib/remote/configobjectutility.hpp index ea42ff3f2..472af3c83 100644 --- a/lib/remote/configobjectutility.hpp +++ b/lib/remote/configobjectutility.hpp @@ -45,13 +45,15 @@ public: 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 String& config, const Array::Ptr& errors, const Array::Ptr& diagnosticInformation); - static bool DeleteObject(const ConfigObject::Ptr& object, bool cascade, const Array::Ptr& errors); + static bool DeleteObject(const ConfigObject::Ptr& object, bool cascade, const Array::Ptr& errors, + const Array::Ptr& diagnosticInformation); private: static String EscapeName(const String& name); - static bool DeleteObjectHelper(const ConfigObject::Ptr& object, bool cascade, const Array::Ptr& errors); + static bool DeleteObjectHelper(const ConfigObject::Ptr& object, bool cascade, const Array::Ptr& errors, + const Array::Ptr& diagnosticInformation); }; } diff --git a/lib/remote/createobjecthandler.cpp b/lib/remote/createobjecthandler.cpp index ff90317af..e3646b3f3 100644 --- a/lib/remote/createobjecthandler.cpp +++ b/lib/remote/createobjecthandler.cpp @@ -74,6 +74,7 @@ bool CreateObjectHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& r Dictionary::Ptr result1 = new Dictionary(); String status; Array::Ptr errors = new Array(); + Array::Ptr diagnosticInformation = new Array(); bool ignoreOnError = false; @@ -86,10 +87,22 @@ bool CreateObjectHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& r String config; + bool verbose = false; + + if (params) + verbose = HttpUtility::GetLastParameter(params, "verbose"); + + /* Object creation can cause multiple errors and optionally diagnostic information. + * We can't use SendJsonError() here. + */ try { config = ConfigObjectUtility::CreateObjectConfig(type, name, ignoreOnError, templates, attrs); } catch (const std::exception& ex) { - errors->Add(DiagnosticInformation(ex)); + errors->Add(DiagnosticInformation(ex, false)); + diagnosticInformation->Add(DiagnosticInformation(ex)); + + if (verbose) + result1->Set("diagnostic_information", diagnosticInformation); result1->Set("errors", errors); result1->Set("code", 500); @@ -101,11 +114,14 @@ bool CreateObjectHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& r return true; } - if (!ConfigObjectUtility::CreateObject(type, name, config, errors)) { + if (!ConfigObjectUtility::CreateObject(type, name, config, errors, diagnosticInformation)) { result1->Set("errors", errors); result1->Set("code", 500); result1->Set("status", "Object could not be created."); + if (verbose) + result1->Set("diagnostic_information", diagnosticInformation); + response.SetStatus(500, "Object could not be created"); HttpUtility::SendJsonBody(response, params, result); diff --git a/lib/remote/deleteobjecthandler.cpp b/lib/remote/deleteobjecthandler.cpp index 027f62fed..0d4053df3 100644 --- a/lib/remote/deleteobjecthandler.cpp +++ b/lib/remote/deleteobjecthandler.cpp @@ -65,11 +65,12 @@ bool DeleteObjectHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& r } catch (const std::exception& ex) { HttpUtility::SendJsonError(response, params, 404, "No objects found.", - HttpUtility::GetLastParameter(params, "verboseErrors") ? DiagnosticInformation(ex) : ""); + DiagnosticInformation(ex)); return true; } bool cascade = HttpUtility::GetLastParameter(params, "cascade"); + bool verbose = HttpUtility::GetLastParameter(params, "verbose"); ArrayData results; @@ -79,8 +80,9 @@ bool DeleteObjectHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& r int code; String status; Array::Ptr errors = new Array(); + Array::Ptr diagnosticInformation = new Array(); - if (!ConfigObjectUtility::DeleteObject(obj, cascade, errors)) { + if (!ConfigObjectUtility::DeleteObject(obj, cascade, errors, diagnosticInformation)) { code = 500; status = "Object could not be deleted."; success = false; @@ -89,13 +91,18 @@ bool DeleteObjectHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& r status = "Object was deleted."; } - results.push_back(new Dictionary({ + Dictionary::Ptr result = new Dictionary({ { "type", type->GetName() }, { "name", obj->GetName() }, { "code", code }, { "status", status }, { "errors", errors } - })); + }); + + if (verbose) + result->Set("diagnostic_information", diagnosticInformation); + + results.push_back(result); } Dictionary::Ptr result = new Dictionary({ diff --git a/lib/remote/modifyobjecthandler.cpp b/lib/remote/modifyobjecthandler.cpp index 893360671..603207ebb 100644 --- a/lib/remote/modifyobjecthandler.cpp +++ b/lib/remote/modifyobjecthandler.cpp @@ -77,6 +77,11 @@ bool ModifyObjectHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& r Dictionary::Ptr attrs = attrsVal; + bool verbose = false; + + if (params) + verbose = HttpUtility::GetLastParameter(params, "verbose"); + ArrayData results; for (const ConfigObject::Ptr& obj : objs) { @@ -100,7 +105,10 @@ bool ModifyObjectHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& r 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)); + result1->Set("status", "Attribute '" + key + "' could not be set: " + DiagnosticInformation(ex, false)); + + if (verbose) + result1->Set("diagnostic_information", DiagnosticInformation(ex)); } results.push_back(std::move(result1)); From 3e83e94c15dbd2cab77115668cb511c31ab7329e Mon Sep 17 00:00:00 2001 From: Michael Friedrich Date: Fri, 6 Apr 2018 12:47:20 +0200 Subject: [PATCH 09/15] Fix object and status query verbose errors --- lib/remote/objectqueryhandler.cpp | 8 ++++---- lib/remote/statushandler.cpp | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/remote/objectqueryhandler.cpp b/lib/remote/objectqueryhandler.cpp index d85df022c..490b50dad 100644 --- a/lib/remote/objectqueryhandler.cpp +++ b/lib/remote/objectqueryhandler.cpp @@ -129,7 +129,7 @@ bool ObjectQueryHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& re uattrs = params->Get("attrs"); } catch (const std::exception&) { HttpUtility::SendJsonError(response, params, 400, - "Invalid type for 'attrs' attribute specified. Array type is required.", Empty); + "Invalid type for 'attrs' attribute specified. Array type is required."); return true; } @@ -137,7 +137,7 @@ bool ObjectQueryHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& re ujoins = params->Get("joins"); } catch (const std::exception&) { HttpUtility::SendJsonError(response, params, 400, - "Invalid type for 'joins' attribute specified. Array type is required.", Empty); + "Invalid type for 'joins' attribute specified. Array type is required."); return true; } @@ -145,7 +145,7 @@ bool ObjectQueryHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& re umetas = params->Get("meta"); } catch (const std::exception&) { HttpUtility::SendJsonError(response, params, 400, - "Invalid type for 'meta' attribute specified. Array type is required.", Empty); + "Invalid type for 'meta' attribute specified. Array type is required."); return true; } @@ -166,7 +166,7 @@ bool ObjectQueryHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& re } catch (const std::exception& ex) { HttpUtility::SendJsonError(response, params, 404, "No objects found.", - HttpUtility::GetLastParameter(params, "verboseErrors") ? DiagnosticInformation(ex) : ""); + DiagnosticInformation(ex)); return true; } diff --git a/lib/remote/statushandler.cpp b/lib/remote/statushandler.cpp index e2c85c2b1..b2a9fca10 100644 --- a/lib/remote/statushandler.cpp +++ b/lib/remote/statushandler.cpp @@ -104,7 +104,7 @@ bool StatusHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& request } catch (const std::exception& ex) { HttpUtility::SendJsonError(response, params, 404, "No objects found.", - HttpUtility::GetLastParameter(params, "verboseErrors") ? DiagnosticInformation(ex) : ""); + DiagnosticInformation(ex)); return true; } From 6b2decb44bfe794cc6da875c3648eb75ec21c33d Mon Sep 17 00:00:00 2001 From: Michael Friedrich Date: Fri, 6 Apr 2018 12:50:06 +0200 Subject: [PATCH 10/15] Fix verbose error handling in variable and template query handlers --- lib/remote/templatequeryhandler.cpp | 2 +- lib/remote/variablequeryhandler.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/remote/templatequeryhandler.cpp b/lib/remote/templatequeryhandler.cpp index 60584c0e7..153c887e5 100644 --- a/lib/remote/templatequeryhandler.cpp +++ b/lib/remote/templatequeryhandler.cpp @@ -127,7 +127,7 @@ bool TemplateQueryHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& } catch (const std::exception& ex) { HttpUtility::SendJsonError(response, params, 404, "No templates found.", - HttpUtility::GetLastParameter(params, "verboseErrors") ? DiagnosticInformation(ex) : ""); + DiagnosticInformation(ex)); return true; } diff --git a/lib/remote/variablequeryhandler.cpp b/lib/remote/variablequeryhandler.cpp index 7effbfb9c..4a36fc8ce 100644 --- a/lib/remote/variablequeryhandler.cpp +++ b/lib/remote/variablequeryhandler.cpp @@ -97,7 +97,7 @@ bool VariableQueryHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& } catch (const std::exception& ex) { HttpUtility::SendJsonError(response, params, 404, "No variables found.", - HttpUtility::GetLastParameter(params, "verboseErrors") ? DiagnosticInformation(ex) : ""); + DiagnosticInformation(ex)); return true; } From 17846e04d8ba6f35448aef3469f509b9be17e9c9 Mon Sep 17 00:00:00 2001 From: Michael Friedrich Date: Fri, 6 Apr 2018 12:52:17 +0200 Subject: [PATCH 11/15] Fix verbose error message in modify object handler --- lib/remote/modifyobjecthandler.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/remote/modifyobjecthandler.cpp b/lib/remote/modifyobjecthandler.cpp index 603207ebb..c08d1dee2 100644 --- a/lib/remote/modifyobjecthandler.cpp +++ b/lib/remote/modifyobjecthandler.cpp @@ -63,7 +63,7 @@ bool ModifyObjectHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& r } catch (const std::exception& ex) { HttpUtility::SendJsonError(response, params, 404, "No objects found.", - HttpUtility::GetLastParameter(params, "verboseErrors") ? DiagnosticInformation(ex) : ""); + DiagnosticInformation(ex)); return true; } @@ -71,7 +71,7 @@ bool ModifyObjectHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& r if (attrsVal.GetReflectionType() != Dictionary::TypeInstance) { HttpUtility::SendJsonError(response, params, 400, - "Invalid type for 'attrs' attribute specified. Dictionary type is required.", Empty); + "Invalid type for 'attrs' attribute specified. Dictionary type is required."); return true; } From 1b31ec8378208cc11b12e6975b843a2e06a0728a Mon Sep 17 00:00:00 2001 From: Michael Friedrich Date: Fri, 6 Apr 2018 12:55:03 +0200 Subject: [PATCH 12/15] Fix verbose errors in config files handler --- lib/remote/configfileshandler.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/remote/configfileshandler.cpp b/lib/remote/configfileshandler.cpp index 77acb96b6..49703ae00 100644 --- a/lib/remote/configfileshandler.cpp +++ b/lib/remote/configfileshandler.cpp @@ -91,7 +91,7 @@ bool ConfigFilesHandler::HandleRequest(const ApiUser::Ptr& user, HttpRequest& re response.WriteBody(content.CStr(), content.GetLength()); } catch (const std::exception& ex) { HttpUtility::SendJsonError(response, params, 500, "Could not read file.", - HttpUtility::GetLastParameter(params, "verboseErrors") ? DiagnosticInformation(ex) : ""); + DiagnosticInformation(ex)); } return true; From 5e8321f3a4c8d0a4f7ded3094ef4276761ca2807 Mon Sep 17 00:00:00 2001 From: Michael Friedrich Date: Fri, 6 Apr 2018 13:03:08 +0200 Subject: [PATCH 13/15] Add verbose parameter to the docs --- doc/12-icinga2-api.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/12-icinga2-api.md b/doc/12-icinga2-api.md index e88ca6226..48d96e699 100644 --- a/doc/12-icinga2-api.md +++ b/doc/12-icinga2-api.md @@ -124,7 +124,8 @@ The API will return standard [HTTP statuses](https://www.ietf.org/rfc/rfc2616.tx including error codes. When an error occurs, the response body will contain additional information -about the problem and its source. +about the problem and its source. Set `verbose` to true to retrieve more +insights into what may be causing the error. A status code between 200 and 299 generally means that the request was successful. From b53685db594035ea23408946ce4623c5591a364e Mon Sep 17 00:00:00 2001 From: Michael Friedrich Date: Thu, 12 Apr 2018 19:24:08 +0200 Subject: [PATCH 14/15] Fix error handling on config package delete --- lib/remote/configpackageshandler.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/remote/configpackageshandler.cpp b/lib/remote/configpackageshandler.cpp index 2ecf78d4c..8ad25d0cb 100644 --- a/lib/remote/configpackageshandler.cpp +++ b/lib/remote/configpackageshandler.cpp @@ -135,12 +135,13 @@ void ConfigPackagesHandler::HandleDelete(const ApiUser::Ptr& user, HttpRequest& } catch (const std::exception& ex) { HttpUtility::SendJsonError(response, params, 500, "Failed to delete package '" + packageName + "'.", DiagnosticInformation(ex)); + return; } Dictionary::Ptr result1 = new Dictionary({ { "code", 200 }, { "package", packageName }, - { "status", "Created package." } + { "status", "Deleted package." } }); Dictionary::Ptr result = new Dictionary({ From c58caf35507737d2bcc6201097666861852299d6 Mon Sep 17 00:00:00 2001 From: Michael Friedrich Date: Thu, 12 Apr 2018 19:44:58 +0200 Subject: [PATCH 15/15] Add global API parameters to the docs; Add API troubleshooting section --- doc/12-icinga2-api.md | 23 ++++++++++++++++++++++- doc/15-troubleshooting.md | 27 +++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/doc/12-icinga2-api.md b/doc/12-icinga2-api.md index 48d96e699..c05522999 100644 --- a/doc/12-icinga2-api.md +++ b/doc/12-icinga2-api.md @@ -103,7 +103,7 @@ The output will be sent back as a JSON object: > **Tip** > -> You can use the `pretty` parameter to beautify the JSON response with Icinga v2.9+. +> You can use the [pretty](12-icinga2-api.md#icinga2-api-parameters-global) parameter to beautify the JSON response with Icinga v2.9+. You can also use [jq](https://stedolan.github.io/jq/) or `python -m json.tool` in combination with curl on the CLI. @@ -272,6 +272,27 @@ Here are the exact same query parameters as a JSON object: The [match function](18-library-reference.md#global-functions-match) is available as global function in Icinga 2. +### Global Parameters + +Name | Description +----------------|-------------------- +pretty | Pretty-print the JSON response. +verbose | Add verbose debug information inside the `diagnostic_information` key into the response if available. This helps with troubleshooting failing requests. + +Example as URL parameter: + +``` +/v1/objects/hosts?pretty=1 +``` + +Example as JSON object: + +``` +{ "pretty": true } +``` + +Both parameters have been added in Icinga 2 v2.9. + ### Request Method Override `GET` requests do not allow you to send a request body. In case you cannot pass everything as URL diff --git a/doc/15-troubleshooting.md b/doc/15-troubleshooting.md index c0fad94f6..8e1fc63ff 100644 --- a/doc/15-troubleshooting.md +++ b/doc/15-troubleshooting.md @@ -702,6 +702,33 @@ Look into the log and check whether the feature logs anything specific for this grep GraphiteWriter /var/log/icinga2/icinga2.log ``` +## REST API Troubleshooting + +In order to analyse errors on API requests, you can explicitly enable the [verbose parameter](12-icinga2-api.md#icinga2-api-parameters-global). + +``` +$ curl -k -s -u root:icinga -H 'Accept: application/json' -X DELETE 'https://localhost:5665/v1/objects/hosts/example-cmdb?pretty=1&verbose=1' +{ + "diagnostic_information": "Error: Object does not exist.\n\n ....", + "error": 404.0, + "status": "No objects found." +} +``` + +## REST API Troubleshooting: No Objects Found + +Please note that the `404` status with no objects being found can also originate +from missing or too strict object permissions for the authenticated user. + +This is a security feature to disable object name guessing. If this would not be the +case, restricted users would be able to get a list of names of your objects just by +trying every character combination. + +In order to analyse and fix the problem, please check the following: + +- use an administrative account with full permissions to check whether the objects are actually there. +- verify the permissions on the affected ApiUser object and fix them. + ## Certificate Troubleshooting