From 798ea541734ecfc5e1ea2f1dea617f2410bc823b Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Thu, 5 Dec 2019 11:08:16 +0100 Subject: [PATCH 1/3] Refuse acking an already acked checkable --- lib/icinga/apiactions.cpp | 6 ++++++ lib/icinga/clusterevents.cpp | 9 +++++++++ lib/icinga/externalcommandprocessor.cpp | 20 ++++++++++++++++++++ 3 files changed, 35 insertions(+) diff --git a/lib/icinga/apiactions.cpp b/lib/icinga/apiactions.cpp index 7d4f1f5c2..875fc2a3f 100644 --- a/lib/icinga/apiactions.cpp +++ b/lib/icinga/apiactions.cpp @@ -214,6 +214,8 @@ Dictionary::Ptr ApiActions::AcknowledgeProblem(const ConfigObject::Ptr& object, } else timestamp = 0; + ObjectLock oLock (checkable); + Host::Ptr host; Service::Ptr service; tie(host, service) = GetHostService(checkable); @@ -226,6 +228,10 @@ Dictionary::Ptr ApiActions::AcknowledgeProblem(const ConfigObject::Ptr& object, return ApiActions::CreateResult(409, "Service " + checkable->GetName() + " is OK."); } + if (checkable->IsAcknowledged()) { + return ApiActions::CreateResult(409, (service ? "Service " : "Host ") + checkable->GetName() + " is already acknowledged."); + } + Comment::AddComment(checkable, CommentAcknowledgement, HttpUtility::GetLastParameter(params, "author"), HttpUtility::GetLastParameter(params, "comment"), persistent, timestamp); checkable->AcknowledgeProblem(HttpUtility::GetLastParameter(params, "author"), diff --git a/lib/icinga/clusterevents.cpp b/lib/icinga/clusterevents.cpp index 5d8f236ef..8309f2212 100644 --- a/lib/icinga/clusterevents.cpp +++ b/lib/icinga/clusterevents.cpp @@ -535,6 +535,15 @@ Value ClusterEvents::AcknowledgementSetAPIHandler(const MessageOrigin::Ptr& orig return Empty; } + ObjectLock oLock (checkable); + + if (checkable->IsAcknowledged()) { + Log(LogWarning, "ClusterEvents") + << "Discarding 'acknowledgement set' message for checkable '" << checkable->GetName() + << "' from '" << origin->FromClient->GetIdentity() << "': Checkable is already acknowledged."; + return Empty; + } + checkable->AcknowledgeProblem(params->Get("author"), params->Get("comment"), static_cast(static_cast(params->Get("acktype"))), params->Get("notify"), params->Get("persistent"), params->Get("expiry"), origin); diff --git a/lib/icinga/externalcommandprocessor.cpp b/lib/icinga/externalcommandprocessor.cpp index a0720a0be..e974a0175 100644 --- a/lib/icinga/externalcommandprocessor.cpp +++ b/lib/icinga/externalcommandprocessor.cpp @@ -588,6 +588,7 @@ void ExternalCommandProcessor::AcknowledgeSvcProblem(double, const std::vector 0 ? true : false); Service::Ptr service = Service::GetByNamePair(arguments[0], arguments[1]); + ObjectLock oLock (service); if (!service) BOOST_THROW_EXCEPTION(std::invalid_argument("Cannot acknowledge service problem for non-existent service '" + arguments[1] + "' on host '" + arguments[0] + "'")); @@ -595,6 +596,10 @@ void ExternalCommandProcessor::AcknowledgeSvcProblem(double, const std::vectorGetState() == ServiceOK) BOOST_THROW_EXCEPTION(std::invalid_argument("The service '" + arguments[1] + "' is OK.")); + if (service->IsAcknowledged()) { + BOOST_THROW_EXCEPTION(std::invalid_argument("The service '" + arguments[1] + "' is already acknowledged.")); + } + Log(LogNotice, "ExternalCommandProcessor") << "Setting acknowledgement for service '" << service->GetName() << "'" << (notify ? "" : ". Disabled notification"); @@ -610,6 +615,7 @@ void ExternalCommandProcessor::AcknowledgeSvcProblemExpire(double, const std::ve double timestamp = Convert::ToDouble(arguments[5]); Service::Ptr service = Service::GetByNamePair(arguments[0], arguments[1]); + ObjectLock oLock (service); if (!service) BOOST_THROW_EXCEPTION(std::invalid_argument("Cannot acknowledge service problem with expire time for non-existent service '" + arguments[1] + "' on host '" + arguments[0] + "'")); @@ -620,6 +626,10 @@ void ExternalCommandProcessor::AcknowledgeSvcProblemExpire(double, const std::ve if (timestamp != 0 && timestamp <= Utility::GetTime()) BOOST_THROW_EXCEPTION(std::invalid_argument("Acknowledgement expire time must be in the future for service '" + arguments[1] + "' on host '" + arguments[0] + "'")); + if (service->IsAcknowledged()) { + BOOST_THROW_EXCEPTION(std::invalid_argument("The service '" + arguments[1] + "' is already acknowledged.")); + } + Log(LogNotice, "ExternalCommandProcessor") << "Setting timed acknowledgement for service '" << service->GetName() << "'" << (notify ? "" : ". Disabled notification"); @@ -652,6 +662,7 @@ void ExternalCommandProcessor::AcknowledgeHostProblem(double, const std::vector< bool persistent = (Convert::ToLong(arguments[3]) > 0 ? true : false); Host::Ptr host = Host::GetByName(arguments[0]); + ObjectLock oLock (host); if (!host) BOOST_THROW_EXCEPTION(std::invalid_argument("Cannot acknowledge host problem for non-existent host '" + arguments[0] + "'")); @@ -662,6 +673,10 @@ void ExternalCommandProcessor::AcknowledgeHostProblem(double, const std::vector< if (host->GetState() == HostUp) BOOST_THROW_EXCEPTION(std::invalid_argument("The host '" + arguments[0] + "' is OK.")); + if (host->IsAcknowledged()) { + BOOST_THROW_EXCEPTION(std::invalid_argument("The host '" + arguments[1] + "' is already acknowledged.")); + } + Comment::AddComment(host, CommentAcknowledgement, arguments[4], arguments[5], persistent, 0); host->AcknowledgeProblem(arguments[4], arguments[5], sticky ? AcknowledgementSticky : AcknowledgementNormal, notify, persistent); } @@ -674,6 +689,7 @@ void ExternalCommandProcessor::AcknowledgeHostProblemExpire(double, const std::v double timestamp = Convert::ToDouble(arguments[4]); Host::Ptr host = Host::GetByName(arguments[0]); + ObjectLock oLock (host); if (!host) BOOST_THROW_EXCEPTION(std::invalid_argument("Cannot acknowledge host problem with expire time for non-existent host '" + arguments[0] + "'")); @@ -687,6 +703,10 @@ void ExternalCommandProcessor::AcknowledgeHostProblemExpire(double, const std::v if (timestamp != 0 && timestamp <= Utility::GetTime()) BOOST_THROW_EXCEPTION(std::invalid_argument("Acknowledgement expire time must be in the future for host '" + arguments[0] + "'")); + if (host->IsAcknowledged()) { + BOOST_THROW_EXCEPTION(std::invalid_argument("The host '" + arguments[1] + "' is already acknowledged.")); + } + Comment::AddComment(host, CommentAcknowledgement, arguments[5], arguments[6], persistent, timestamp); host->AcknowledgeProblem(arguments[5], arguments[6], sticky ? AcknowledgementSticky : AcknowledgementNormal, notify, persistent, timestamp); } From da6a9174d14c79d43c8f9adaed3898c39f004b0a Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Thu, 5 Dec 2019 11:54:33 +0100 Subject: [PATCH 2/3] Checkable#ClearAcknowledgement(): OnAcknowledgementCleared() at most once --- lib/icinga/checkable.cpp | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/lib/icinga/checkable.cpp b/lib/icinga/checkable.cpp index cf2d7c9d2..e5362bb86 100644 --- a/lib/icinga/checkable.cpp +++ b/lib/icinga/checkable.cpp @@ -138,13 +138,27 @@ void Checkable::AcknowledgeProblem(const String& author, const String& comment, void Checkable::ClearAcknowledgement(const String& removedBy, const MessageOrigin::Ptr& origin) { + ObjectLock oLock (this); + + bool wasAcked; + + if (GetAcknowledgementRaw() == AcknowledgementNone) { + wasAcked = false; + } else { + double expiry = GetAcknowledgementExpiry(); + + wasAcked = expiry == 0 || expiry >= Utility::GetTime(); + } + SetAcknowledgementRaw(AcknowledgementNone); SetAcknowledgementExpiry(0); Log(LogInformation, "Checkable") << "Acknowledgement cleared for checkable '" << GetName() << "'."; - OnAcknowledgementCleared(this, removedBy, origin); + if (wasAcked) { + OnAcknowledgementCleared(this, removedBy, origin); + } } Endpoint::Ptr Checkable::GetCommandEndpoint() const From 69881770d8dbf1a5a368a7581690b5ded3604364 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Mon, 9 Dec 2019 17:54:47 +0100 Subject: [PATCH 3/3] Adjust docs --- doc/16-upgrading-icinga-2.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/doc/16-upgrading-icinga-2.md b/doc/16-upgrading-icinga-2.md index 95e08a2d7..55314d4e5 100644 --- a/doc/16-upgrading-icinga-2.md +++ b/doc/16-upgrading-icinga-2.md @@ -8,6 +8,13 @@ Specific version upgrades are described below. Please note that version updates are incremental. An upgrade from v2.6 to v2.8 requires to follow the instructions for v2.7 too. +## Upgrading to v2.12 + +### Breaking changes + +As of v2.12 our [API](12-icinga2-api.md) URL endpoint [`/v1/actions/acknowledge-problem`](12-icinga2-api.md#icinga2-api-actions-acknowledge-problem) refuses acknowledging an already acknowledged checkable by overwriting the acknowledgement. +To replace an acknowledgement you have to remove the old one before adding the new one. + ## Upgrading to v2.11 ### Bugfixes for 2.11