mirror of
https://github.com/Cacti/plugin_thold.git
synced 2026-10-23 03:27:05 +09:00
fix incorrect maint include (#858)
* fix include * return back deleted file * Fix maint include guard: test file_exists on path, not require_once return file_exists() was wrapping require_once(), so the first require ran unconditionally and its integer return (1) was passed to file_exists(), which fell through to require the non-existent includes/functions.php and fatalled. Test the path strings directly. * Cover maint include fallback and error branches The maint-include lookup in thold_check_threshold() and thold_update_host_status() gained an includes/functions.php fallback and an error-log branch, but the suite only exercised the plugin-root path. Add MaintIncludeResolutionTest driving all three outcomes for both functions, with cacti-root fixtures for the includes/ layout and the missing-plugin case, so patch coverage of the changed lines reaches 100%. --------- Co-authored-by: TheWitness <thewitness@cacti.net>
This commit is contained in:
1 parent
0929150560
commit
dd2abc92cd
10 files changed
+179
-4
No files matched your search
@@ -3414,7 +3414,13 @@ function thold_check_threshold(&$thold_data) {
|
||||
|
||||
// don't alert for this host if it's selected for maintenance
|
||||
if (api_plugin_is_enabled('maint') || in_array('maint', $plugins, true)) {
|
||||
include_once($config['base_path'] . '/plugins/maint/functions.php');
|
||||
if (file_exists($config['base_path'] . '/plugins/maint/functions.php')) {
|
||||
require_once($config['base_path'] . '/plugins/maint/functions.php');
|
||||
} else if (file_exists($config['base_path'] . '/plugins/maint/includes/functions.php')) {
|
||||
require_once($config['base_path'] . '/plugins/maint/includes/functions.php');
|
||||
} else {
|
||||
cacti_log('ERROR: cannot include maint functions', false, 'THOLD');
|
||||
}
|
||||
}
|
||||
|
||||
if (api_plugin_is_enabled('maint') && plugin_maint_check_cacti_host($thold_data['host_id'])) {
|
||||
|
||||
@@ -465,7 +465,13 @@ function thold_update_host_status() {
|
||||
include_once($config['library_path'] . '/snmp.php');
|
||||
|
||||
if (api_plugin_is_enabled('maint')) {
|
||||
include_once($config['base_path'] . '/plugins/maint/functions.php');
|
||||
if (file_exists($config['base_path'] . '/plugins/maint/functions.php')) {
|
||||
require_once($config['base_path'] . '/plugins/maint/functions.php');
|
||||
} else if (file_exists($config['base_path'] . '/plugins/maint/includes/functions.php')) {
|
||||
require_once($config['base_path'] . '/plugins/maint/includes/functions.php');
|
||||
} else {
|
||||
cacti_log('ERROR: cannot include maint functions', false, 'THOLD');
|
||||
}
|
||||
}
|
||||
|
||||
$ping_failure_count = read_config_option('ping_failure_count');
|
||||
|
||||
@@ -0,0 +1,3 @@
|
||||
<?php
|
||||
// Empty maint plugin fixture using the includes/ layout: the function it would
|
||||
// provide is stubbed, so this file only needs to exist at this path.
|
||||
@@ -0,0 +1,2 @@
|
||||
/* Fixture stylesheet so thold_page_head()'s file_exists() theme-css branch is
|
||||
reachable under the 'modern' theme returned by the test bootstrap. */
|
||||
@@ -0,0 +1,3 @@
|
||||
<?php
|
||||
// Load the plugin arrays through the same path shape as a Cacti installation.
|
||||
require dirname(__DIR__, 6) . '/includes/arrays.php';
|
||||
@@ -0,0 +1,3 @@
|
||||
# Cacti root fixture with no maint plugin installed, so neither
|
||||
# plugins/maint/functions.php nor plugins/maint/includes/functions.php exists.
|
||||
# This drives the "cannot include maint functions" error branch.
|
||||
@@ -0,0 +1,2 @@
|
||||
/* Fixture stylesheet so thold_page_head()'s file_exists() theme-css branch is
|
||||
reachable under the 'modern' theme returned by the test bootstrap. */
|
||||
@@ -0,0 +1,3 @@
|
||||
<?php
|
||||
// Load the plugin arrays through the same path shape as a Cacti installation.
|
||||
require dirname(__DIR__, 6) . '/includes/arrays.php';
|
||||
@@ -252,9 +252,13 @@ final class ThresholdScenario {
|
||||
/**
|
||||
* Put the device into a maintenance window.
|
||||
*
|
||||
* @param string|null $cactiRoot Cacti root fixture to resolve the maint
|
||||
* include against; defaults to the layout that ships functions.php at the
|
||||
* plugin root.
|
||||
*
|
||||
* @return self
|
||||
*/
|
||||
public function inMaintenance() {
|
||||
public function inMaintenance($cactiRoot = null) {
|
||||
// Asked more than once per poll, so a queued value would run out.
|
||||
CactiStubs::willAlwaysReturn('api_plugin_is_enabled', true);
|
||||
CactiStubs::willAlwaysReturn('plugin_maint_check_cacti_host', true);
|
||||
@@ -264,7 +268,7 @@ final class ThresholdScenario {
|
||||
* base_path at a tracked, repository-contained Cacti root fixture so a
|
||||
* unit test never writes into the caller's Cacti checkout.
|
||||
*/
|
||||
$GLOBALS['config']['base_path'] = dirname(__DIR__) . '/Fixtures/cacti-root';
|
||||
$GLOBALS['config']['base_path'] = $cactiRoot ?? dirname(__DIR__) . '/Fixtures/cacti-root';
|
||||
|
||||
return $this;
|
||||
}
|
||||
|
||||
@@ -0,0 +1,143 @@
|
||||
<?php
|
||||
/*
|
||||
+-------------------------------------------------------------------------+
|
||||
| Copyright (C) 2004-2026 The Cacti Group |
|
||||
| |
|
||||
| This program is free software; you can redistribute it and/or |
|
||||
| modify it under the terms of the GNU General Public License |
|
||||
| as published by the Free Software Foundation; either version 2 |
|
||||
| of the License, or (at your option) any later version. |
|
||||
+-------------------------------------------------------------------------+
|
||||
| Cacti: The Complete RRDtool-based Graphing Solution |
|
||||
+-------------------------------------------------------------------------+
|
||||
| http://www.cacti.net/ |
|
||||
+-------------------------------------------------------------------------+
|
||||
*/
|
||||
|
||||
/**
|
||||
* Both maintenance-aware entry points resolve the maint plugin the same way:
|
||||
* the functions.php that ships at the plugin root, the includes/functions.php
|
||||
* of the newer layout, or an error log when neither is present. These tests
|
||||
* drive all three outcomes for thold_check_threshold() and
|
||||
* thold_update_host_status() so a future change to the lookup is caught.
|
||||
*/
|
||||
final class MaintIncludeResolutionTest extends TestCase {
|
||||
/**
|
||||
* @return void
|
||||
*/
|
||||
public static function setUpBeforeClass(): void {
|
||||
self::loadPluginSource('includes/functions.php');
|
||||
self::loadPluginSource('includes/polling.php');
|
||||
self::loadPluginSourceAlways('includes/arrays.php');
|
||||
self::loadPluginConstants();
|
||||
}
|
||||
|
||||
/**
|
||||
* @param array<string, mixed> $overrides
|
||||
*
|
||||
* @return ThresholdScenario
|
||||
*/
|
||||
private function baseline(array $overrides = []) {
|
||||
return ThresholdScenario::threshold($overrides + [
|
||||
'thold_type' => 1,
|
||||
'bl_type' => 0,
|
||||
'bl_pct_up' => 10,
|
||||
'bl_pct_down' => 10,
|
||||
'bl_ref_time_range' => 3600,
|
||||
'bl_fail_trigger' => 1,
|
||||
])
|
||||
->alertRecipient('ops@example.org')
|
||||
->referenceStatistics(100, 100, 100, 100);
|
||||
}
|
||||
|
||||
/**
|
||||
* @param string $name
|
||||
*
|
||||
* @return string
|
||||
*/
|
||||
private function fixtureRoot($name) {
|
||||
return dirname(__DIR__) . '/Fixtures/' . $name;
|
||||
}
|
||||
|
||||
/**
|
||||
* The newer maint layout keeps functions.php under includes/; a device in a
|
||||
* maintenance window still resolves it and suppresses the notification.
|
||||
*
|
||||
* @return void
|
||||
*/
|
||||
public function testCheckThresholdResolvesMaintFromIncludesLayout(): void {
|
||||
$outcome = $this->baseline(['lastread' => 500])
|
||||
->inMaintenance($this->fixtureRoot('cacti-root-maint-includes'))
|
||||
->poll();
|
||||
|
||||
$this->assertSame(0, $outcome->mailCount());
|
||||
}
|
||||
|
||||
/**
|
||||
* With neither maint layout present the include is skipped and the failure
|
||||
* is logged rather than fatal.
|
||||
*
|
||||
* @return void
|
||||
*/
|
||||
public function testCheckThresholdLogsWhenMaintFunctionsMissing(): void {
|
||||
$this->baseline(['lastread' => 500])
|
||||
->inMaintenance($this->fixtureRoot('cacti-root-no-maint'))
|
||||
->poll();
|
||||
|
||||
$this->assertContains('ERROR: cannot include maint functions', CactiStubs::$log);
|
||||
}
|
||||
|
||||
/**
|
||||
* Drive thold_update_host_status() far enough to run the maint include
|
||||
* block. Downstream work needs a live Cacti, so engine messages are
|
||||
* swallowed and any throw past the block is ignored.
|
||||
*
|
||||
* @param string $cactiRoot
|
||||
*
|
||||
* @return void
|
||||
*/
|
||||
private function runHostStatus($cactiRoot) {
|
||||
CactiStubs::willAlwaysReturn('api_plugin_is_enabled', true);
|
||||
CactiStubs::$configOptions['alert_deadnotify'] = 'on';
|
||||
$GLOBALS['config']['base_path'] = $cactiRoot;
|
||||
|
||||
set_error_handler(static fn (): bool => true);
|
||||
ob_start();
|
||||
|
||||
try {
|
||||
thold_update_host_status();
|
||||
} catch (\Throwable $e) {
|
||||
// Reaching the maint include block is the point.
|
||||
} finally {
|
||||
ob_end_clean();
|
||||
restore_error_handler();
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @return void
|
||||
*/
|
||||
public function testHostStatusResolvesMaintFromPluginRoot(): void {
|
||||
$this->runHostStatus($this->fixtureRoot('cacti-root'));
|
||||
|
||||
$this->assertNotContains('ERROR: cannot include maint functions', CactiStubs::$log);
|
||||
}
|
||||
|
||||
/**
|
||||
* @return void
|
||||
*/
|
||||
public function testHostStatusResolvesMaintFromIncludesLayout(): void {
|
||||
$this->runHostStatus($this->fixtureRoot('cacti-root-maint-includes'));
|
||||
|
||||
$this->assertNotContains('ERROR: cannot include maint functions', CactiStubs::$log);
|
||||
}
|
||||
|
||||
/**
|
||||
* @return void
|
||||
*/
|
||||
public function testHostStatusLogsWhenMaintFunctionsMissing(): void {
|
||||
$this->runHostStatus($this->fixtureRoot('cacti-root-no-maint'));
|
||||
|
||||
$this->assertContains('ERROR: cannot include maint functions', CactiStubs::$log);
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user