From 9baa3c4341e6ebed0b03230e7a20906d462783d8 Mon Sep 17 00:00:00 2001 From: Sukhwinder Dhillon Date: Fri, 17 Nov 2023 12:31:20 +0100 Subject: [PATCH] Remove redudant class `MonitorBackendMonitoring` and adjuct code accordigly - Use class `Monitoring` instead - Remove not in use methods from `MonitorBackend` interface and from classes that implements this interface - Add param types and return type hint to methods --- application/controllers/HostController.php | 19 +-- application/controllers/ServiceController.php | 2 +- library/Director/Backend/MonitorBackend.php | 18 +-- .../Backend/MonitorBackendIcingadb.php | 75 +-------- .../Backend/MonitorBackendMonitoring.php | 149 ------------------ .../DirectorObject/Lookup/ServiceFinder.php | 2 +- .../MonitoringModule/Monitoring.php | 61 +++---- .../ProvidedHook/Monitoring/HostActions.php | 2 +- .../Monitoring/ServiceActions.php | 2 +- .../Web/Controller/ActionController.php | 4 +- 10 files changed, 49 insertions(+), 285 deletions(-) delete mode 100644 library/Director/Backend/MonitorBackendMonitoring.php diff --git a/application/controllers/HostController.php b/application/controllers/HostController.php index a23da138..cce491e2 100644 --- a/application/controllers/HostController.php +++ b/application/controllers/HostController.php @@ -34,7 +34,9 @@ class HostController extends ObjectController $host = $this->getHostObject(); $auth = $this->Auth(); $backend = $this->backend(); - if ($this->isServiceAction() && $backend->authCanEditService($auth, $host, $this->getParam('service'))) { + if ($this->isServiceAction() + && $backend->canModifyService($host->getObjectName(), $this->getParam('service')) + ) { return; } if ($auth->hasPermission(Permission::MONITORING_SERVICES_RO) && $this->isServicesReadOnlyAction()) { @@ -43,7 +45,7 @@ class HostController extends ObjectController if ($auth->hasPermission(Permission::HOSTS)) { // faster return; } - if ($backend->authCanEditHost($host)) { + if ($backend->canModifyHost($host->getObjectName())) { return; } $this->assertPermission(Permission::HOSTS); // complain about default hosts permission @@ -570,14 +572,13 @@ class HostController extends ObjectController && $host->isObject() && $backend->hasHost($host->getObjectName()) ) { - $this->actions()->add($backend->getHostLink( - $this->translate('Show'), - $host->getObjectName(), + $this->actions()->add(Link::create($this->translate('Show'), + $backend->getHostUrl($host->getObjectName()), + null, [ - 'class' => 'icon-globe critical', - 'data-base-target' => '_next' - ] - )); + 'class' => 'icon-globe critical', + 'data-base-target' => '_next' + ])); // Intentionally placed here, show it only for deployed Hosts $this->addOptionalInspectLink(); diff --git a/application/controllers/ServiceController.php b/application/controllers/ServiceController.php index 5d2f3d1f..c09b0937 100644 --- a/application/controllers/ServiceController.php +++ b/application/controllers/ServiceController.php @@ -30,7 +30,7 @@ class ServiceController extends ObjectController protected function checkDirectorPermissions() { if ($this->hasPermission(Permission::MONITORING_SERVICES)) { - if ($this->backend()->authCanEditService($this->Auth(), $this->getParam('host'), $this->getParam('name'))) { + if ($this->backend()->canModifyService($this->getParam('host'), $this->getParam('name'))) { return; } } diff --git a/library/Director/Backend/MonitorBackend.php b/library/Director/Backend/MonitorBackend.php index da41fd2f..5950d2c0 100644 --- a/library/Director/Backend/MonitorBackend.php +++ b/library/Director/Backend/MonitorBackend.php @@ -2,25 +2,19 @@ namespace Icinga\Module\Director\Backend; -use Icinga\Data\Filter\Filter; +use Icinga\Web\Url; interface MonitorBackend { - public function isAvailable(); + public function isAvailable(): bool; - public function hasHost($hostname); + public function hasHost(string $hostname): bool; - public function hasHostWithExtraFilter($hostname, Filter $filter); - - public function hasService($hostname, $service); - - public function hasServiceWithExtraFilter($hostname, $service, Filter $filter); - - public function getHostLink($title, $hostname, array $attributes = null); - - public function getHostState($hostname); + public function hasService(string $hostname, string $service): bool; public function canModifyHost(string $hostName): bool; public function canModifyService(string $hostName, string $serviceName): bool; + + public function getHostUrl(string $hostname): Url; } diff --git a/library/Director/Backend/MonitorBackendIcingadb.php b/library/Director/Backend/MonitorBackendIcingadb.php index a1cea3e0..2305f696 100644 --- a/library/Director/Backend/MonitorBackendIcingadb.php +++ b/library/Director/Backend/MonitorBackendIcingadb.php @@ -5,11 +5,13 @@ namespace Icinga\Module\Director\Backend; use gipfl\IcingaWeb2\Link; use Icinga\Application\Icinga; use Icinga\Data\Filter\Filter as DataFilter; +use Icinga\Module\Director\Objects\IcingaHost; use Icinga\Module\Icingadb\Common\Auth; use Icinga\Module\Icingadb\Common\Database; use Icinga\Module\Icingadb\Model\Host; use Icinga\Module\Icingadb\Model\Service; use Icinga\Module\Icingadb\Redis\VolatileStateResults; +use Icinga\Web\Url; use ipl\Stdlib\Filter; class MonitorBackendIcingadb implements MonitorBackend @@ -17,14 +19,14 @@ class MonitorBackendIcingadb implements MonitorBackend use Database; use Auth; - public function isAvailable() + public function isAvailable(): bool { $app = Icinga::app(); $modules = $app->getModuleManager(); return $modules->hasLoaded('icingadb'); } - public function hasHost($hostname) + public function hasHost($hostname): bool { $query = Host::on($this->getDb()); $query->filter(Filter::equal('host.name', $hostname)); @@ -37,13 +39,7 @@ class MonitorBackendIcingadb implements MonitorBackend return ($host !== null); } - public function hasHostWithExtraFilter($hostname, DataFilter $filter) - { - // TODO - return false; - } - - public function hasService($hostname, $service) + public function hasService($hostname, $service): bool { $query = Service::on($this->getDb()); $query @@ -60,66 +56,9 @@ class MonitorBackendIcingadb implements MonitorBackend return ($service !== null); } - public function hasServiceWithExtraFilter($hostname, $service, DataFilter $filter) + public function getHostUrl(string $hostname): Url { - // TODO - return false; - } - - public function getHostLink($title, $hostname, array $attributes = null) - { - return Link::create( - $title, - 'icingadb/host', - ['name' => $hostname], - $attributes - ); - } - - public function getHostState($hostname) - { - $hostStates = [ - '0' => 'up', - '1' => 'down', - '2' => 'unreachable', - '99' => 'pending', - ]; - - $query = Host::on($this->getDb())->with(['state']); - $query - ->setResultSetClass(VolatileStateResults::class) - ->filter(Filter::equal('host.name', $hostname)); - - $this->applyRestrictions($query); - - /** @var Host $host */ - $host = $query->first(); - - $result = (object) [ - 'hostname' => $hostname, - 'state' => '99', - 'problem' => '0', - 'acknowledged' => '0', - 'in_downtime' => '0', - 'output' => null, - ]; - - if ($host !== null) { - // TODO: implement this for icingadb (function is unused atm) - /** - $query = $this->backend->select()->from('hostStatus', [ - 'hostname' => 'host_name', - 'state' => 'host_state', - 'problem' => 'host_problem', - 'acknowledged' => 'host_acknowledged', - 'in_downtime' => 'host_in_downtime', - 'output' => 'host_output', - ])->where('host_name', $hostname); - */ - } - - $result->state = $hostStates[$result->state]; - return $result; + return Url::fromPath('icingadb/host', ['name' => $hostname]); } public function canModifyHost(string $hostName): bool diff --git a/library/Director/Backend/MonitorBackendMonitoring.php b/library/Director/Backend/MonitorBackendMonitoring.php deleted file mode 100644 index 0f2fdf71..00000000 --- a/library/Director/Backend/MonitorBackendMonitoring.php +++ /dev/null @@ -1,149 +0,0 @@ -getModuleManager(); - if (!$modules->hasLoaded('monitoring') && $app->isCli()) { - $app->getModuleManager()->loadEnabledModules(); - } - - if ($modules->hasLoaded('monitoring')) { - $this->backend = MonitoringBackend::instance(); - } - } - - public function isAvailable() - { - return $this->backend !== null; - } - - public function hasHost($hostname) - { - if ($this->backend === null) { - return false; - } - - return $this->backend->select()->from('hostStatus', [ - 'hostname' => 'host_name', - ])->where('host_name', $hostname)->fetchOne() === $hostname; - } - - public function hasHostWithExtraFilter($hostname, Filter $filter) - { - if ($this->backend === null) { - return false; - } - - return $this->backend->select()->from('hostStatus', [ - 'hostname' => 'host_name', - ])->where('host_name', $hostname)->applyFilter($filter)->fetchOne() === $hostname; - } - - public function hasService($hostname, $service) - { - if ($this->backend === null) { - return false; - } - - return (array) $this->prepareServiceKeyColumnQuery($hostname, $service)->fetchRow() === [ - 'hostname' => $hostname, - 'service' => $service, - ]; - } - - public function hasServiceWithExtraFilter($hostname, $service, Filter $filter) - { - if ($this->backend === null) { - return false; - } - - return (array) $this - ->prepareServiceKeyColumnQuery($hostname, $service) - ->applyFilter($filter) - ->fetchRow() === [ - 'hostname' => $hostname, - 'service' => $service, - ]; - } - - public function getHostLink($title, $hostname, array $attributes = null) - { - return Link::create( - $title, - 'monitoring/host/show', - ['host' => $hostname], - $attributes - ); - } - - public function getHostState($hostname) - { - $hostStates = [ - '0' => 'up', - '1' => 'down', - '2' => 'unreachable', - '99' => 'pending', - ]; - - $query = $this->backend->select()->from('hostStatus', [ - 'hostname' => 'host_name', - 'state' => 'host_state', - 'problem' => 'host_problem', - 'acknowledged' => 'host_acknowledged', - 'in_downtime' => 'host_in_downtime', - 'output' => 'host_output', - ])->where('host_name', $hostname); - - $res = $query->fetchRow(); - if ($res === false) { - $res = (object) [ - 'hostname' => $hostname, - 'state' => '99', - 'problem' => '0', - 'acknowledged' => '0', - 'in_downtime' => '0', - 'output' => null, - ]; - } - - $res->state = $hostStates[$res->state]; - - return $res; - } - - protected function prepareServiceKeyColumnQuery($hostname, $service) - { - return $this->backend - ->select() - ->from('serviceStatus', [ - 'hostname' => 'host_name', - 'service' => 'service_description', - ]) - ->where('host_name', $hostname) - ->where('service_description', $service); - } - - public function canModifyHost(string $hostName): bool - { - // TODO: Implement canModifyHost() method. - return false; - } - - public function canModifyService(string $hostName, string $serviceName): bool - { - // TODO: Implement canModifyService() method. - return false; - } -} diff --git a/library/Director/DirectorObject/Lookup/ServiceFinder.php b/library/Director/DirectorObject/Lookup/ServiceFinder.php index 583b4d35..fba6b65e 100644 --- a/library/Director/DirectorObject/Lookup/ServiceFinder.php +++ b/library/Director/DirectorObject/Lookup/ServiceFinder.php @@ -68,7 +68,7 @@ class ServiceFinder } if ($this->auth->hasPermission(Permission::MONITORING_HOSTS)) { if ($info = $this::find($this->host, $serviceName)) { - if ((new Monitoring($this->auth))->canModifyServiceByName($this->host->getObjectName(), $serviceName)) { + if ((new Monitoring($this->auth))->canModifyService($this->host->getObjectName(), $serviceName)) { return $info->getUrl(); } } diff --git a/library/Director/Integration/MonitoringModule/Monitoring.php b/library/Director/Integration/MonitoringModule/Monitoring.php index 3a9cb296..2cda38b1 100644 --- a/library/Director/Integration/MonitoringModule/Monitoring.php +++ b/library/Director/Integration/MonitoringModule/Monitoring.php @@ -10,10 +10,11 @@ use Icinga\Exception\ConfigurationError; use Icinga\Module\Director\Auth\MonitoringRestriction; use Icinga\Module\Director\Auth\Permission; use Icinga\Module\Director\Auth\Restriction; -use Icinga\Module\Director\Objects\IcingaHost; +use Icinga\Module\Director\Backend\MonitorBackend; use Icinga\Module\Monitoring\Backend\MonitoringBackend; +use Icinga\Web\Url; -class Monitoring +class Monitoring implements MonitorBackend { /** @var ?MonitoringBackend */ protected $backend; @@ -32,9 +33,14 @@ class Monitoring return $this->backend !== null; } - public function hasHost(IcingaHost $host): bool + public function getHostUrl(string $hostname): Url { - return $this->hasHostByName($host->getObjectName()); + return Url::fromPath('monitoring/host/show', ['host' => $hostname]); + } + + public function hasHost($hostname): bool + { + return $this->hasHostByName($hostname); } public function hasHostByName($hostname): bool @@ -50,6 +56,12 @@ class Monitoring } } + + public function hasService($hostname, $service): bool + { + return $this->hasServiceByName($hostname, $service); + } + public function hasServiceByName($hostname, $service): bool { if (! $this->isAvailable()) { @@ -63,9 +75,9 @@ class Monitoring } } - public function canModifyService(IcingaHost $host, $service): bool + public function canModifyService(string $hostName, string $serviceName): bool { - return $this->canModifyServiceByName($host->getObjectName(), $service); + return $this->canModifyServiceByName($hostName, $serviceName); } public function canModifyServiceByName($hostname, $service): bool @@ -88,9 +100,9 @@ class Monitoring return false; } - public function canModifyHost(IcingaHost $host): bool + public function canModifyHost(string $hostName): bool { - return $this->canModifyHostByName($host->getObjectName()); + return $this->canModifyHostByName($hostName); } public function canModifyHostByName($hostname): bool @@ -132,39 +144,6 @@ class Monitoring } } - public function getHostState($hostname) - { - $hostStates = [ - '0' => 'up', - '1' => 'down', - '2' => 'unreachable', - '99' => 'pending', - ]; - - $query = $this->selectHostStatus($hostname, [ - 'hostname' => 'host_name', - 'state' => 'host_state', - 'problem' => 'host_problem', - 'acknowledged' => 'host_acknowledged', - 'in_downtime' => 'host_in_downtime', - ])->where('host_name', $hostname); - - $res = $query->fetchRow(); - if ($res === false) { - $res = (object) [ - 'hostname' => $hostname, - 'state' => '99', - 'problem' => '0', - 'acknowledged' => '0', - 'in_downtime' => '0', - ]; - } - - $res->state = $hostStates[$res->state]; - - return $res; - } - protected function selectHost($hostname) { return $this->selectHostStatus($hostname, [ diff --git a/library/Director/ProvidedHook/Monitoring/HostActions.php b/library/Director/ProvidedHook/Monitoring/HostActions.php index d1379c8a..2d0469de 100644 --- a/library/Director/ProvidedHook/Monitoring/HostActions.php +++ b/library/Director/ProvidedHook/Monitoring/HostActions.php @@ -45,7 +45,7 @@ class HostActions extends HostActionsHook $allowEdit = true; } if (Util::hasPermission(Permission::MONITORING_HOSTS)) { - if ((new Monitoring(Auth::getInstance()))->canModifyHostByName($hostname)) { + if ((new Monitoring(Auth::getInstance()))->canModifyHost($hostname)) { $allowEdit = IcingaHost::exists($hostname, $db); } } diff --git a/library/Director/ProvidedHook/Monitoring/ServiceActions.php b/library/Director/ProvidedHook/Monitoring/ServiceActions.php index 5796fde0..834b1664 100644 --- a/library/Director/ProvidedHook/Monitoring/ServiceActions.php +++ b/library/Director/ProvidedHook/Monitoring/ServiceActions.php @@ -56,7 +56,7 @@ class ServiceActions extends ServiceActionsHook if (Util::hasPermission(Permission::HOSTS)) { $title = mt('director', 'Modify'); } elseif (Util::hasPermission(Permission::MONITORING_SERVICES)) { - if ((new Monitoring(Auth::getInstance()))->canModifyServiceByName($hostname, $serviceName)) { + if ((new Monitoring(Auth::getInstance()))->canModifyService($hostname, $serviceName)) { $title = mt('director', 'Modify'); } } elseif (Util::hasPermission(Permission::MONITORING_SERVICES_RO)) { diff --git a/library/Director/Web/Controller/ActionController.php b/library/Director/Web/Controller/ActionController.php index b58a955f..b1aadb96 100644 --- a/library/Director/Web/Controller/ActionController.php +++ b/library/Director/Web/Controller/ActionController.php @@ -11,7 +11,7 @@ use Icinga\Exception\NotFoundError; use Icinga\Exception\ProgrammingError; use Icinga\Module\Director\Backend\MonitorBackend; use Icinga\Module\Director\Backend\MonitorBackendIcingadb; -use Icinga\Module\Director\Backend\MonitorBackendMonitoring; +use Icinga\Module\Director\Integration\MonitoringModule\Monitoring; use Icinga\Module\Director\Web\Controller\Extension\CoreApi; use Icinga\Module\Director\Web\Controller\Extension\DirectorDb; use Icinga\Module\Director\Web\Controller\Extension\RestApi; @@ -252,7 +252,7 @@ abstract class ActionController extends Controller implements ControlsAndContent if (Module::exists('icingadb')) { $this->backend = new MonitorBackendIcingadb(); } else { - $this->backend = new MonitorBackendMonitoring(); + $this->backend = new Monitoring($this->Auth()); } }