From dea34ff68bb24df46167ab23f05847aefdd9133e Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Mon, 17 Aug 2026 06:53:04 -0700 Subject: [PATCH] fix(notify): drain only the notifications this run claimed thold_notify.php stamps its own identifier on the rows it intends to handle, then called thold_notification_execute() with no arguments. That defaults the identifier to zero, which drops the filter, so the drain selected every unprocessed row regardless of who claimed it. poller_thold.php launches this script on every poller cycle without checking whether the previous one has finished, so two overlapping runs both mailed the same notifications. The claim also ran before register_process_start(), so a second instance stamped its identifier over the first instance's rows before discovering it should exit, and it now takes only rows nobody holds. Where neither posix_getpgid() nor posix_kill() is available there is no way to ask whether the recorded process is alive. That case fell through and drained the queue a second time; it now stands down, which is the safe reading of an unanswerable question. Refs #784 Signed-off-by: Thomas Vincent --- .github/workflows/php-unit-tests.yml | 92 ++++ .gitignore | 7 + composer.json | 49 ++ phpunit.xml | 32 ++ tests/Helpers/CactiStubs.php | 217 +++++++++ tests/TestCase.php | 65 +++ tests/Unit/NotificationQueueClaimTest.php | 157 ++++++ tests/bin/patch-coverage.php | 158 ++++++ tests/bootstrap-unit.php | 527 +++++++++++++++++++++ tests/docker/Dockerfile | 29 ++ tests/docker/docker-compose.yml | 15 + tests/fixtures/cacti-lib/snmp.php | 21 + tests/fixtures/cacti-lib/variables.php | 22 + tests/fixtures/optional-core-functions.php | 45 ++ thold_notify.php | 67 ++- 15 files changed, 1480 insertions(+), 23 deletions(-) create mode 100644 .github/workflows/php-unit-tests.yml create mode 100644 composer.json create mode 100644 phpunit.xml create mode 100644 tests/Helpers/CactiStubs.php create mode 100644 tests/TestCase.php create mode 100644 tests/Unit/NotificationQueueClaimTest.php create mode 100644 tests/bin/patch-coverage.php create mode 100644 tests/bootstrap-unit.php create mode 100644 tests/docker/Dockerfile create mode 100644 tests/docker/docker-compose.yml create mode 100644 tests/fixtures/cacti-lib/snmp.php create mode 100644 tests/fixtures/cacti-lib/variables.php create mode 100644 tests/fixtures/optional-core-functions.php diff --git a/.github/workflows/php-unit-tests.yml b/.github/workflows/php-unit-tests.yml new file mode 100644 index 00000000..9ab561f2 --- /dev/null +++ b/.github/workflows/php-unit-tests.yml @@ -0,0 +1,92 @@ +# +-------------------------------------------------------------------------+ +# | 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. | +# | | +# | This program is distributed in the hope that it will be useful, | +# | but WITHOUT ANY WARRANTY; without even the implied warranty of | +# | MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | +# | GNU General Public License for more details. | +# +-------------------------------------------------------------------------+ +# | Cacti: The Complete RRDtool-based Graphing Solution | +# +-------------------------------------------------------------------------+ +# | This code is designed, written, and maintained by the Cacti Group. See | +# | about.php and/or the AUTHORS file for specific developer information. | +# +-------------------------------------------------------------------------+ +# | http://www.cacti.net/ | +# +-------------------------------------------------------------------------+ + + +name: PHP Unit Tests + +on: + push: + branches: + - main + - develop + pull_request: + branches: + - main + - develop + +permissions: + contents: read + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + unit-test: + name: PHPUnit on PHP 8.1 (Docker) + runs-on: ubuntu-latest + + steps: + - name: Checkout Thold Plugin + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + # patch-coverage.php diffs against the base branch. + fetch-depth: 0 + + # The image is the same one developers run locally via + # `composer test:docker`, so a green run here is reproducible off-CI. + - name: Build test image + run: docker build --tag cacti-thold-test:php8.1 --file tests/docker/Dockerfile tests/docker + + - name: Validate composer.json + run: docker run --rm --volume "$PWD":/cacti/plugins/thold cacti-thold-test:php8.1 composer validate --strict --no-check-lock + + - name: Install dependencies + run: docker run --rm --volume "$PWD":/cacti/plugins/thold cacti-thold-test:php8.1 composer install --no-progress --no-ansi + + # Same scripts a developer runs locally, and the same names Cacti core uses. + - name: Lint every PHP source file + run: docker run --rm --volume "$PWD":/cacti/plugins/thold cacti-thold-test:php8.1 composer lint + + - name: Run unit tests with coverage + run: | + docker run --rm --volume "$PWD":/cacti/plugins/thold cacti-thold-test:php8.1 \ + composer test:coverage + + # Whole-file coverage is meaningless here: most of the plugin only runs + # inside a live Cacti. What is enforceable is that a change covers the + # lines it adds. + - name: Enforce coverage of changed lines + if: github.event_name == 'pull_request' + env: + BASE_REF: origin/${{ github.base_ref }} + run: | + docker run --rm --volume "$PWD":/cacti/plugins/thold --env BASE_REF \ + cacti-thold-test:php8.1 \ + sh -c 'git config --global --add safe.directory /cacti/plugins/thold && php tests/bin/patch-coverage.php coverage/clover.xml "$BASE_REF" 100' + + - name: Upload coverage report + if: always() + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + with: + name: coverage + path: coverage/ + if-no-files-found: warn diff --git a/.gitignore b/.gitignore index eb716067..0806dc45 100644 --- a/.gitignore +++ b/.gitignore @@ -20,3 +20,10 @@ # +-------------------------------------------------------------------------+ locales/po/*.mo + +/vendor/ +/composer.lock +/.phpunit.cache/ +/coverage/ +/coverage/ +/.phpunit.result.cache diff --git a/composer.json b/composer.json new file mode 100644 index 00000000..94aead2f --- /dev/null +++ b/composer.json @@ -0,0 +1,49 @@ +{ + "_comment": [ + "+-------------------------------------------------------------------------+", + "| 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. |", + "| |", + "| This program is distributed in the hope that it will be useful, |", + "| but WITHOUT ANY WARRANTY; without even the implied warranty of |", + "| MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |", + "| GNU General Public License for more details. |", + "+-------------------------------------------------------------------------+", + "| Cacti: The Complete RRDtool-based Graphing Solution |", + "+-------------------------------------------------------------------------+", + "| http://www.cacti.net/ |", + "+-------------------------------------------------------------------------+" + ], + "name": "cacti/plugin-thold", + "description": "Thold Plugin for Cacti", + "type": "project", + "license": "GPL-2.0-only", + "require-dev": { + "overtrue/phplint": "^9.6", + "phpunit/phpunit": "^10.5.64" + }, + "scripts": { + "lint": "phplint --no-cache --exclude=vendor ", + "test": "phpunit --display-warnings", + "test:coverage": "phpunit --display-warnings --coverage-clover=coverage/clover.xml", + "test:docker": "docker compose -f tests/docker/docker-compose.yml run --rm phpunit" + }, + "config": { + "sort-packages": true, + "vendor-dir": "vendor", + "platform": { + "php": "8.1.0" + }, + "platform-check": true + }, + "autoload-dev": { + "classmap": [ + "tests/Helpers/", + "tests/TestCase.php" + ] + } +} diff --git a/phpunit.xml b/phpunit.xml new file mode 100644 index 00000000..46a8d9a8 --- /dev/null +++ b/phpunit.xml @@ -0,0 +1,32 @@ + + + + + + + + + ./tests/Unit + + + + + + + thold_functions.php + + + + diff --git a/tests/Helpers/CactiStubs.php b/tests/Helpers/CactiStubs.php new file mode 100644 index 00000000..59d084cf --- /dev/null +++ b/tests/Helpers/CactiStubs.php @@ -0,0 +1,217 @@ +}> + */ + public static $calls = []; + + /** + * Queued return values, keyed by function name. Each call shifts one value + * off the front; an exhausted queue falls back to the type default. + * + * @var array> + */ + public static $returns = []; + + /** + * Return values chosen by a fragment of the SQL, keyed by function name. + * Each entry is [fragment, value]. Consulted before $returns. + * + * @var array> + */ + public static $matchedReturns = []; + + /** + * Values handed back on every call, keyed by function name. Consulted last. + * + * @var array + */ + public static $stickyReturns = []; + + /** + * Values handed back by the get_*_request_var() family, keyed by var name. + * + * @var array + */ + public static $requestVars = []; + + /** + * Values handed back by read_config_option(), keyed by option name. + * + * @var array + */ + public static $configOptions = []; + + /** + * Messages passed to cacti_log(), in order. + * + * @var array + */ + public static $log = []; + + /** + * Mail handed to Cacti's mailer(), in order. + * + * @var array + */ + public static $mail = []; + + /** + * Clear all recorded and programmed state. + * + * @return void + */ + public static function reset() { + self::$calls = []; + self::$returns = []; + self::$matchedReturns = []; + self::$stickyReturns = []; + self::$requestVars = []; + self::$configOptions = []; + self::$log = []; + self::$mail = []; + } + + /** + * Record one Cacti function call. + * + * @param string $fn Cacti function name. + * @param string $sql SQL text, or '' for non-query calls. + * @param array $params Bound parameters, if any. + * + * @return void + */ + public static function record($fn, $sql = '', array $params = []) { + self::$calls[] = ['fn' => $fn, 'sql' => $sql, 'params' => $params]; + } + + /** + * Hand back $value for every call to $fn. + * + * @param string $fn Cacti function name. + * @param mixed $value Value to hand back. + * + * @return void + */ + public static function willAlwaysReturn($fn, $value) { + self::$stickyReturns[$fn] = $value; + } + + /** + * Queue one return value for the next call to $fn. + * + * @param string $fn Cacti function name. + * @param mixed $value Value to hand back. + * + * @return void + */ + public static function willReturn($fn, $value) { + self::$returns[$fn][] = $value; + } + + /** + * Answer any call to $fn whose SQL contains $fragment with $value. + * + * A function such as db_fetch_cell_prepared is called many times with + * different queries in one run, so a positional queue would break as soon + * as the code under test reordered a lookup. Matching on the query keeps + * the fixture readable and stable. + * + * @param string $fn Cacti function name. + * @param string $fragment Distinctive substring of the SQL. + * @param mixed $value Value to hand back, or a callable taking the SQL + * and returning it. + * + * @return void + */ + public static function willReturnFor($fn, $fragment, $value) { + self::$matchedReturns[$fn][] = [$fragment, $value]; + } + + /** + * Take the return value for a call: a SQL match first, then the queue, then + * the type default. + * + * @param string $fn Cacti function name. + * @param mixed $default Fallback when nothing matches. + * @param string $sql SQL the caller passed, for matching. + * + * @return mixed + */ + public static function nextReturn($fn, $default, $sql = '') { + if ($sql !== '' && !empty(self::$matchedReturns[$fn])) { + $flat = preg_replace('/\s+/', ' ', $sql); + + foreach (self::$matchedReturns[$fn] as $entry) { + if (strpos($flat, preg_replace('/\s+/', ' ', $entry[0])) !== false) { + // A callable answers per query, for a lookup whose result + // depends on the values the caller asked about. + return is_callable($entry[1]) ? ($entry[1])($sql) : $entry[1]; + } + } + } + + if (!empty(self::$returns[$fn])) { + return array_shift(self::$returns[$fn]); + } + + if (array_key_exists($fn, self::$stickyReturns)) { + return self::$stickyReturns[$fn]; + } + + return $default; + } + + /** + * All recorded calls to $fn. + * + * @param string $fn Cacti function name. + * + * @return array}> + */ + public static function callsTo($fn) { + return array_values(array_filter(self::$calls, function ($call) use ($fn) { + return $call['fn'] === $fn; + })); + } + + /** + * The recorded call log reduced to function names, in order. Useful for + * asserting transaction sequencing. + * + * @return array + */ + public static function callSequence() { + return array_column(self::$calls, 'fn'); + } +} diff --git a/tests/TestCase.php b/tests/TestCase.php new file mode 100644 index 00000000..67342561 --- /dev/null +++ b/tests/TestCase.php @@ -0,0 +1,65 @@ + + */ + private function queueQueries() { + $queries = []; + + foreach (CactiStubs::$calls as $call) { + if (strpos($call['sql'], 'notification_queue') !== false) { + $queries[] = preg_replace('/\s+/', ' ', $call['sql']); + } + } + + return $queries; + } + + /** + * @return void + */ + public function testADrainWithAnIdentifierOnlyTakesThatProcessesRows(): void { + thold_notification_execute(4242); + + $queries = $this->queueQueries(); + + $this->assertNotEmpty($queries); + + foreach ($queries as $sql) { + $this->assertStringContainsString('process_id = 4242', $sql); + } + } + + /** + * Passing nothing is what thold_notify.php used to do, and it selects + * every unprocessed row regardless of who claimed it. + * + * @return void + */ + public function testADrainWithoutAnIdentifierIsUnscoped(): void { + thold_notification_execute(); + + $queries = $this->queueQueries(); + + $this->assertNotEmpty($queries); + + foreach ($queries as $sql) { + $this->assertStringNotContainsString('process_id =', $sql); + } + } + + /** + * @return void + */ + public function testBothStagesThatReadTheQueueAreScoped(): void { + thold_notification_execute(77); + + $queries = $this->queueQueries(); + + // the non-device stage and the device stage each select from the queue + $this->assertCount(2, $queries); + + foreach ($queries as $sql) { + $this->assertStringContainsString('process_id = 77', $sql); + } + } + + /** + * @return void + */ + public function testTheDrainRespectsARecordLimit(): void { + thold_notification_execute(5, 10); + + $limited = array_filter($this->queueQueries(), static function ($sql) { + return strpos($sql, 'LIMIT 10') !== false; + }); + + $this->assertNotEmpty($limited); + } + + /** + * The collector claims only rows nobody holds, so a second instance + * cannot take rows the first is already working on. + * + * @return void + */ + public function testTheClaimTakesOnlyUnheldRows(): void { + $src = file_get_contents(dirname(__DIR__, 2) . '/thold_notify.php'); + + $this->assertMatchesRegularExpression( + '/SET process_id = \?\s+WHERE event_processed = 0\s+AND process_id = 0/', + $src + ); + } + + /** + * The claim has to follow the registration, or a second instance stamps + * its identifier over the first instance's rows before discovering that + * it should exit. + * + * @return void + */ + public function testTheClaimFollowsTheProcessRegistration(): void { + $src = file_get_contents(dirname(__DIR__, 2) . '/thold_notify.php'); + + $registered = strpos($src, "register_process_start('thold_notify'"); + $claimed = strpos($src, 'SET process_id = ?'); + + $this->assertNotFalse($registered); + $this->assertNotFalse($claimed); + $this->assertLessThan($claimed, $registered); + } + + /** + * Without a way to ask whether the recorded process is still alive, the + * run must stand down rather than proceed beside it. It previously fell + * through and drained the queue a second time. + * + * @return void + */ + public function testAnInstanceThatCannotCheckForAPeerStandsDown(): void { + $src = file_get_contents(dirname(__DIR__, 2) . '/thold_notify.php'); + + $this->assertMatchesRegularExpression('/\$running = true;/', $src); + $this->assertMatchesRegularExpression('/if \(\$running\) \{\s+exit\(1\);/', $src); + } +} diff --git a/tests/bin/patch-coverage.php b/tests/bin/patch-coverage.php new file mode 100644 index 00000000..83f9c73f --- /dev/null +++ b/tests/bin/patch-coverage.php @@ -0,0 +1,158 @@ + [min-percent] + * + * Exits 1 if coverage is below the threshold, 2 on bad input. + */ + +if ($argc < 3) { + fwrite(STDERR, "usage: patch-coverage.php [min-percent]\n"); + + exit(2); +} + +$clover_path = $argv[1]; +$base_ref = $argv[2]; +$minimum = isset($argv[3]) ? (float) $argv[3] : 100.0; + +if (!is_readable($clover_path)) { + fwrite(STDERR, "cannot read coverage report: $clover_path\n"); + + exit(2); +} + +/** + * Line numbers each measured file changed, keyed by repository-relative path. + * + * Only added and modified lines count. Deletions have nothing left to cover, + * and context lines were not part of this change. + * + * Paths stay repository-relative so the report can be produced in a container + * and evaluated on the host, where the absolute paths differ. + * + * @param string $base_ref Git ref to diff against. + * + * @return array> + */ +function changed_lines($base_ref) { + $command = 'git diff --no-ext-diff --unified=0 --no-color --diff-filter=AM ' . escapeshellarg($base_ref) . '...HEAD -- "*.php"'; + $diff = shell_exec($command); + + if ($diff === null) { + fwrite(STDERR, "git diff failed\n"); + + exit(2); + } + + $changed = []; + $file = null; + + foreach (explode("\n", $diff) as $line) { + if (strncmp($line, '+++ b/', 6) === 0) { + $file = substr($line, 6); + $changed[$file] = []; + } elseif (strncmp($line, '@@', 2) === 0 && $file !== null) { + if (preg_match('/\+(\d+)(?:,(\d+))?/', $line, $match)) { + $start = (int) $match[1]; + $count = isset($match[2]) ? (int) $match[2] : 1; + + for ($i = 0; $i < $count; $i++) { + $changed[$file][$start + $i] = true; + } + } + } + } + + return $changed; +} + +$changed = changed_lines($base_ref); +$clover = simplexml_load_file($clover_path); + +if ($clover === false) { + fwrite(STDERR, "cannot parse coverage report: $clover_path\n"); + + exit(2); +} + +$covered = 0; +$total = 0; +$missing = []; + +foreach ($clover->xpath('//file') as $file) { + $path = (string) $file['name']; + $relative = null; + + foreach (array_keys($changed) as $candidate) { + if ($path === $candidate || substr($path, -strlen('/' . $candidate)) === '/' . $candidate) { + $relative = $candidate; + + break; + } + } + + if ($relative === null) { + continue; + } + + foreach ($file->line as $line) { + $number = (int) $line['num']; + + // Only statement lines are measurable; method markers double-count. + if ((string) $line['type'] !== 'stmt' || !isset($changed[$relative][$number])) { + continue; + } + + $total++; + + if ((int) $line['count'] > 0) { + $covered++; + } else { + $missing[] = $relative . ':' . $number; + } + } +} + +if ($total === 0) { + print "Patch coverage: no measured lines changed.\n"; + + exit(0); +} + +$percent = ($covered / $total) * 100; + +printf("Patch coverage: %.2f%% (%d/%d lines)\n", $percent, $covered, $total); + +if ($missing !== []) { + print "Uncovered changed lines:\n " . implode("\n ", $missing) . "\n"; +} + +if ($percent + 0.005 < $minimum) { + printf("FAIL: below the %.2f%% minimum.\n", $minimum); + + exit(1); +} + +exit(0); diff --git a/tests/bootstrap-unit.php b/tests/bootstrap-unit.php new file mode 100644 index 00000000..6ab1ecbe --- /dev/null +++ b/tests/bootstrap-unit.php @@ -0,0 +1,527 @@ + dirname(dirname(dirname(__DIR__))), + 'url_path' => '/cacti/', + 'cacti_version' => '1.2.31', + 'cacti_server_os' => 'unix', +]; + +// thold_expand_string() include_once()s library_path/variables.php at call time. +$GLOBALS['config']['library_path'] = __DIR__ . '/fixtures/cacti-lib'; + +// thold reads and writes this on every RPN evaluation. +$GLOBALS['rpn_error'] = false; + +// Cacti's list of enabled plugins; thold_check_threshold() declares it global. +$GLOBALS['plugins'] = []; + +// Cacti's debug flag, also declared global by thold_check_threshold(). +$GLOBALS['debug'] = false; + +if (!function_exists('db_execute')) { + function db_execute($sql, $log = true, $db_conn = false) { + CactiStubs::record('db_execute', $sql); + + return CactiStubs::nextReturn('db_execute', true, $sql); + } +} + +if (!function_exists('db_execute_prepared')) { + function db_execute_prepared($sql, $params = [], $log = true, $db_conn = false) { + CactiStubs::record('db_execute_prepared', $sql, $params); + + return CactiStubs::nextReturn('db_execute_prepared', true, $sql); + } +} + +if (!function_exists('db_fetch_assoc')) { + function db_fetch_assoc($sql, $log = true, $db_conn = false) { + CactiStubs::record('db_fetch_assoc', $sql); + + return CactiStubs::nextReturn('db_fetch_assoc', [], $sql); + } +} + +if (!function_exists('db_fetch_assoc_prepared')) { + function db_fetch_assoc_prepared($sql, $params = [], $log = true, $db_conn = false) { + CactiStubs::record('db_fetch_assoc_prepared', $sql, $params); + + return CactiStubs::nextReturn('db_fetch_assoc_prepared', [], $sql); + } +} + +if (!function_exists('db_fetch_row')) { + function db_fetch_row($sql, $log = true, $db_conn = false) { + CactiStubs::record('db_fetch_row', $sql); + + return CactiStubs::nextReturn('db_fetch_row', [], $sql); + } +} + +if (!function_exists('db_fetch_row_prepared')) { + function db_fetch_row_prepared($sql, $params = [], $log = true, $db_conn = false) { + CactiStubs::record('db_fetch_row_prepared', $sql, $params); + + return CactiStubs::nextReturn('db_fetch_row_prepared', [], $sql); + } +} + +/* + * Cacti's cell fetchers return false, not '', when the query matches no row. + * The difference matters on PHP 8: false coerces to 0 in arithmetic while '' + * raises a TypeError, so a stub returning '' invents failures that production + * does not have. + */ +if (!function_exists('db_fetch_cell')) { + function db_fetch_cell($sql, $col_name = '', $log = true, $db_conn = false) { + CactiStubs::record('db_fetch_cell', $sql); + + return CactiStubs::nextReturn('db_fetch_cell', false, $sql); + } +} + +if (!function_exists('db_fetch_cell_prepared')) { + function db_fetch_cell_prepared($sql, $params = [], $col_name = '', $log = true, $db_conn = false) { + CactiStubs::record('db_fetch_cell_prepared', $sql, $params); + + return CactiStubs::nextReturn('db_fetch_cell_prepared', false, $sql); + } +} + +if (!function_exists('db_qstr')) { + function db_qstr($string) { + return "'" . str_replace("'", "''", (string) $string) . "'"; + } +} + +if (!function_exists('db_begin_transaction')) { + function db_begin_transaction() { + CactiStubs::record('db_begin_transaction'); + + return CactiStubs::nextReturn('db_begin_transaction', true); + } +} + +if (!function_exists('db_commit_transaction')) { + function db_commit_transaction() { + CactiStubs::record('db_commit_transaction'); + + return CactiStubs::nextReturn('db_commit_transaction', true); + } +} + +if (!function_exists('db_rollback_transaction')) { + function db_rollback_transaction() { + CactiStubs::record('db_rollback_transaction'); + + return CactiStubs::nextReturn('db_rollback_transaction', true); + } +} + +if (!function_exists('html_escape')) { + function html_escape($string) { + return htmlspecialchars((string) $string, ENT_QUOTES, 'UTF-8'); + } +} + +/* + * Mirrors Cacti 1.2 lib/functions.php. KEEP IN SYNC: if core tightens its + * checks, tests here would otherwise keep passing while production diverges. + */ +if (!function_exists('sanitize_unserialize_selected_items')) { + function sanitize_unserialize_selected_items($items) { + if (empty($items)) { + return false; + } + + $data = unserialize($items, ['allowed_classes' => false]); // nosemgrep: php.lang.security.unserialize-use.unserialize-use -- test stub mirroring Cacti core; allowed_classes:false blocks object injection + + if (!is_array($data)) { + return false; + } + + foreach ($data as $value) { + if (!is_numeric($value)) { + return false; + } + } + + return $data; + } +} + +if (!function_exists('read_config_option')) { + function read_config_option($name, $force = false) { + return isset(CactiStubs::$configOptions[$name]) ? CactiStubs::$configOptions[$name] : ''; + } +} + +if (!function_exists('set_config_option')) { + function set_config_option($name, $value) { + CactiStubs::$configOptions[$name] = $value; + } +} + +if (!function_exists('__')) { + function __($text) { + $args = array_slice(func_get_args(), 1); + + // Cacti's __() accepts sprintf arguments after the format string. + return $args === [] ? $text : vsprintf($text, $args); + } +} + +if (!function_exists('__esc')) { + function __esc($text) { + return htmlspecialchars(call_user_func_array('__', func_get_args()), ENT_QUOTES, 'UTF-8'); + } +} + +if (!function_exists('cacti_log')) { + function cacti_log($message, $output = false, $environ = 'CMDPHP', $level = 0) { + CactiStubs::$log[] = $message; + } +} + +if (!function_exists('cacti_sizeof')) { + function cacti_sizeof($array) { + return (is_array($array) || $array instanceof Countable) ? count($array) : 0; + } +} + +if (!function_exists('cacti_count')) { + function cacti_count($array) { + return cacti_sizeof($array); + } +} + +if (!function_exists('get_request_var')) { + function get_request_var($name, $default = '') { + return isset(CactiStubs::$requestVars[$name]) ? CactiStubs::$requestVars[$name] : $default; + } +} + +if (!function_exists('get_nfilter_request_var')) { + function get_nfilter_request_var($name, $default = '') { + return get_request_var($name, $default); + } +} + +if (!function_exists('get_filter_request_var')) { + function get_filter_request_var($name, $filter = FILTER_VALIDATE_INT, $options = []) { + return get_request_var($name); + } +} + +if (!function_exists('isset_request_var')) { + function isset_request_var($name) { + return isset(CactiStubs::$requestVars[$name]); + } +} + +if (!function_exists('cacti_escapeshellarg')) { + function cacti_escapeshellarg($string, $quote = true) { + return escapeshellarg((string) $string); + } +} + +if (!function_exists('api_plugin_hook_function')) { + function api_plugin_hook_function($name, $data = '') { + return $data; + } +} + +if (!function_exists('get_simple_graph_perms')) { + function get_simple_graph_perms($user_id) { + return CactiStubs::nextReturn('get_simple_graph_perms', true); + } +} + +if (!function_exists('get_policies')) { + function get_policies($user_id) { + return CactiStubs::nextReturn('get_policies', []); + } +} + +if (!function_exists('get_policy_where')) { + function get_policy_where($graph_auth_method, $policies, $sql_where) { + CactiStubs::record('get_policy_where', $sql_where); + + return CactiStubs::nextReturn('get_policy_where', $sql_where); + } +} + +if (!function_exists('expand_title')) { + function expand_title($host_id, $snmp_query_id, $snmp_index, $title) { + CactiStubs::record('expand_title', $title); + + return CactiStubs::nextReturn('expand_title', $title); + } +} + +if (!function_exists('get_graph_title')) { + function get_graph_title($local_graph_id) { + return CactiStubs::nextReturn('get_graph_title', 'Traffic - eth0'); + } +} + +if (!function_exists('rrdtool_function_fetch')) { + function rrdtool_function_fetch($local_data_id, $start_time, $end_time, $resolution = 0, $show_unknown = false, $rrdtool_file = null) { + CactiStubs::record('rrdtool_function_fetch', (string) $local_data_id); + + return CactiStubs::nextReturn('rrdtool_function_fetch', []); + } +} + +if (!function_exists('get_data_source_path')) { + function get_data_source_path($local_data_id, $expand_paths = true) { + return '/var/lib/cacti/rra/test_' . (int) $local_data_id . '.rrd'; + } +} + +if (!function_exists('sql_save')) { + function sql_save($array_items, $table_name, $key_cols = 'id', $autoinc = true, $db_conn = false) { + CactiStubs::record('sql_save', $table_name, $array_items); + + return CactiStubs::nextReturn('sql_save', 1); + } +} + +if (!function_exists('db_affected_rows')) { + function db_affected_rows($db_conn = false) { + return CactiStubs::nextReturn('db_affected_rows', 1); + } +} + +if (!function_exists('rrdtool_function_graph')) { + function rrdtool_function_graph($local_graph_id, $rra_id, $graph_data_array, $rrdtool_pipe = false, &$xport_meta = [], $user = 0) { + CactiStubs::record('rrdtool_function_graph', (string) $local_graph_id); + + // A one-pixel PNG stands in for the rendered graph. + return base64_decode('iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAAAAAA6fptVAAAACklEQVR4nGMAAQAABQABDQottAAAAABJRU5ErkJggg==', true); + } +} + +if (!function_exists('get_timespan')) { + function get_timespan(&$timespan, $time, $span, $first_weekdayid) { + $timespan['begin_now'] = $time - 86400; + $timespan['end_now'] = $time; + } +} + +if (!function_exists('read_user_setting')) { + function read_user_setting($config_name, $default = false, $force = false, $user = 0) { + return CactiStubs::nextReturn('read_user_setting', $default); + } +} + +if (!function_exists('get_selected_theme')) { + function get_selected_theme() { + return 'modern'; + } +} + +if (!function_exists('mailer')) { + function mailer($from, $to, $cc = '', $bcc = '', $replyto = '', $subject = '', $body = '', $body_text = '', $attachments = null, $headers = [], $html = true) { + CactiStubs::$mail[] = [ + 'to' => is_array($to) ? implode(',', $to) : (string) $to, + 'bcc' => is_array($bcc) ? implode(',', $bcc) : (string) $bcc, + 'subject' => (string) $subject, + ]; + + return CactiStubs::nextReturn('mailer', ''); + } +} + +if (!function_exists('cacti_snmp_send')) { + function cacti_snmp_send($hostname, $version, $community, $oid, $value, $type = 's') { + CactiStubs::record('cacti_snmp_send', (string) $oid); + + return true; + } +} + +if (!function_exists('debounce_run_notification')) { + function debounce_run_notification($id, $frequency = 60, ...$args) { + CactiStubs::record('debounce_run_notification', (string) $id); + } +} + +if (!function_exists('array_rekey')) { + function array_rekey($array, $key, $key_value) { + $ret_array = []; + + if (is_array($array)) { + foreach ($array as $item) { + $item_key = $item[$key]; + + if (is_array($key_value)) { + foreach ($key_value as $value) { + $ret_array[$item_key][$value] = $item[$value]; + } + } else { + $ret_array[$item_key] = $item[$key_value]; + } + } + } + + return $ret_array; + } +} + +if (!function_exists('clean_up_name')) { + function clean_up_name($string) { + $string = preg_replace('/[\s\.]+/', '_', $string); + $string = preg_replace('/[^a-zA-Z0-9_]+/', '', $string); + + return preg_replace('/_{2,}/', '_', $string); + } +} + +if (!function_exists('plugin_maint_check_cacti_host')) { + function plugin_maint_check_cacti_host($host_id) { + return CactiStubs::nextReturn('plugin_maint_check_cacti_host', false); + } +} + +if (!function_exists('api_plugin_is_enabled')) { + function api_plugin_is_enabled($plugin) { + return CactiStubs::nextReturn('api_plugin_is_enabled', false); + } +} + +if (!function_exists('api_plugin_hook')) { + function api_plugin_hook($name, $data = '') { + CactiStubs::record('api_plugin_hook', $name); + + return $data; + } +} + +if (!function_exists('api_user_realm_auth')) { + function api_user_realm_auth($filename = '') { + return CactiStubs::nextReturn('api_user_realm_auth', true); + } +} + +if (!function_exists('raise_message')) { + function raise_message($message_id, $message = '', $level = 0) { + CactiStubs::record('raise_message', (string) $message_id); + } +} + +if (!function_exists('rrdtool_execute')) { + function rrdtool_execute($command, $log_to_stdout = false, $output_flag = 1, $rrdtool_pipe = false, $logopt = 'WEBLOG') { + CactiStubs::record('rrdtool_execute', $command); + + return CactiStubs::nextReturn('rrdtool_execute', ''); + } +} + +if (!function_exists('rrdtool_function_interface_speed')) { + function rrdtool_function_interface_speed($data_local) { + return CactiStubs::nextReturn('rrdtool_function_interface_speed', 0); + } +} + +if (!function_exists('get_timeinstate')) { + function get_timeinstate($host) { + return CactiStubs::nextReturn('get_timeinstate', '1 day'); + } +} + +if (!function_exists('get_daysfromtime')) { + function get_daysfromtime($timestamp) { + return CactiStubs::nextReturn('get_daysfromtime', '1 day'); + } +} + +if (!function_exists('number_format_i18n')) { + function number_format_i18n($number, $decimals = 0, $baseu = 1000) { + return number_format((float) $number, $decimals < 0 ? 0 : (int) $decimals); + } +} + +if (!defined('FILTER_VALIDATE_IS_REGEX')) { + define('FILTER_VALIDATE_IS_REGEX', 99999); +} + +// Device states, from Cacti include/global_constants.php. +foreach (['HOST_UNKNOWN' => 0, 'HOST_DOWN' => 1, 'HOST_RECOVERING' => 2, 'HOST_UP' => 3, 'HOST_ERROR' => 4] as $name => $value) { + if (!defined($name)) { + define($name, $value); + } +} + +if (!defined('RRDTOOL_OUTPUT_STDOUT')) { + define('RRDTOOL_OUTPUT_STDOUT', 1); +} + +if (!defined('CACTI_DATE_TIME_FORMAT')) { + define('CACTI_DATE_TIME_FORMAT', 'Y-m-d H:i:s'); +} + +if (!defined('CACTI_PATH_BASE')) { + define('CACTI_PATH_BASE', $GLOBALS['config']['base_path']); +} + +/** + * Load a plugin source file at global scope. + * + * Several plugin files (includes/arrays.php in particular) define their data + * as file-scope variables that the rest of the plugin reads as globals, and + * they read $config while doing so. Requiring them from inside a method would + * make both halves of that method-local, so the require happens here and any + * variable the file introduced is published to $GLOBALS. + * + * @param string $path Absolute path to the file. + * + * @return void + */ +function thold_test_load($path) { + global $config; + + $__before = get_defined_vars(); + + require_once $path; + + foreach (get_defined_vars() as $__name => $__value) { + if (!array_key_exists($__name, $__before) && strncmp($__name, '__', 2) !== 0) { + $GLOBALS[$__name] = $__value; + } + } +} diff --git a/tests/docker/Dockerfile b/tests/docker/Dockerfile new file mode 100644 index 00000000..518f7321 --- /dev/null +++ b/tests/docker/Dockerfile @@ -0,0 +1,29 @@ +# Test runner for the Thold plugin. +# +# Pinned to PHP 8.1 because that is the oldest interpreter the CI matrix +# covers; what passes here passes on 8.2-8.4. pcov rather than Xdebug: line +# coverage is the only debug feature the suite needs and pcov is far cheaper. +FROM php:8.1-cli-alpine@sha256:7949370448b0b4d9787776dc5968e0fd8d48763292344b5fbf21539441228a98 + +# git is needed by the changed-line coverage gate, which diffs against the +# base branch. +RUN apk add --no-cache git gmp-dev \ + && docker-php-ext-install gmp \ + && apk add --no-cache --virtual .build-deps $PHPIZE_DEPS \ + && pecl install pcov \ + && docker-php-ext-enable pcov \ + && apk del .build-deps + +COPY --from=composer:2@sha256:4d71c3c2109c61d5415544264b59ad4087e4c5b7244481723664138fd36d5040 /usr/bin/composer /usr/bin/composer + +# The plugin lives where Cacti would put it, because thold_functions.php +# resolves its own includes through $config['base_path'] . '/plugins/thold'. +# No network or database is involved; the Cacti framework functions themselves +# are stubbed in tests/bootstrap.php. +WORKDIR /cacti/plugins/thold + +ENV COMPOSER_ALLOW_SUPERUSER=1 \ + COMPOSER_NO_INTERACTION=1 \ + COMPOSER_CACHE_DIR=/tmp/composer-cache + +CMD ["sh", "-c", "composer install --no-progress --no-ansi && composer test"] diff --git a/tests/docker/docker-compose.yml b/tests/docker/docker-compose.yml new file mode 100644 index 00000000..99d38b47 --- /dev/null +++ b/tests/docker/docker-compose.yml @@ -0,0 +1,15 @@ +# Local mirror of the unit-test CI job. `docker compose -f +# tests/docker/docker-compose.yml run --rm phpunit` runs exactly what CI runs. +services: + phpunit: + build: + context: . + dockerfile: Dockerfile + image: cacti-thold-test:php8.1 + working_dir: /cacti/plugins/thold + volumes: + - ../..:/cacti/plugins/thold + - composer-cache:/tmp/composer-cache + +volumes: + composer-cache: diff --git a/tests/fixtures/cacti-lib/snmp.php b/tests/fixtures/cacti-lib/snmp.php new file mode 100644 index 00000000..9b9341bd --- /dev/null +++ b/tests/fixtures/cacti-lib/snmp.php @@ -0,0 +1,21 @@ + 255) { + $s = substr($s, 0, 255); + } + + $s = str_replace(["\0", '|', '{', '}'], '', $s); + + return 'RLIKE ' . db_qstr($s, $db_conn); + } +} + +if (!function_exists('get_total_row_data')) { + function get_total_row_data($user_id, $sql, $sql_params = [], $class = '', $timeout = 86400) { + CactiStubs::record('get_total_row_data', $sql, $sql_params); + + return CactiStubs::nextReturn('get_total_row_data', 0); + } +} diff --git a/thold_notify.php b/thold_notify.php index b16749c2..90b43951 100644 --- a/thold_notify.php +++ b/thold_notify.php @@ -102,18 +102,14 @@ $start = microtime(true); // This is where we can parallelize -if ($thread === false) { +$collector = ($thread === false); +$pid = 0; +$total_rows = 0; + +if ($collector) { thold_cli_debug('Thold Notification Main Collector Started'); $thread = 1; - $pid = getmypid(); - - db_execute_prepared('UPDATE notification_queue - SET process_id = ? - WHERE event_processed = 0', - [$pid]); - - $total_rows = db_affected_rows(); } else { thold_cli_debug("Thold Notification Child Thread $thread Started"); } @@ -122,30 +118,55 @@ // kill any running services that have run outside of their timeout if (!register_process_start('thold_notify', 'child', $thread, $timeout)) { - $pid = db_fetch_cell_prepared('SELECT pid + $running_pid = db_fetch_cell_prepared('SELECT pid FROM processes WHERE tasktype = "thold_notify" AND taskname = "child" AND taskid = ?', [$thread]); - if ($config['cacti_server_os'] == 'unix') { - if (function_exists('posix_getpgid')) { - $running = posix_getpgid($pid); - } elseif (function_exists('posix_kill')) { - $running = posix_kill($pid, 0); - } + if ($config['cacti_server_os'] == 'unix' && function_exists('posix_getpgid')) { + $running = posix_getpgid($running_pid); + } elseif ($config['cacti_server_os'] == 'unix' && function_exists('posix_kill')) { + $running = posix_kill($running_pid, 0); + } else { + /* + * Without a way to ask whether the recorded process is alive, assume + * it is. Carrying on regardless is what let a second instance run + * beside the first and mail the same queue twice. + */ + $running = true; + } - if ($running) { - exit(1); - } else { - unregister_process('thold_notify', 'child', $thread); - register_process_start('thold_notify', 'child', $thread, $timeout); - } + if ($running) { + exit(1); } + + unregister_process('thold_notify', 'child', $thread); + register_process_start('thold_notify', 'child', $thread, $timeout); +} + +/* + * Claim the queue only once this instance is the registered one, and only the + * rows nobody else holds. Claiming before the registration above meant a + * second instance stamped its own identifier over the first instance's rows + * even in the case where it went on to exit. + */ +if ($collector) { + $pid = getmypid(); + + db_execute_prepared('UPDATE notification_queue + SET process_id = ? + WHERE event_processed = 0 + AND process_id = 0', + [$pid]); + + $total_rows = db_affected_rows(); } -thold_notification_execute(); +// Drain only what was claimed. Passing nothing selected every unprocessed row, +// so two overlapping runs both mailed the same notifications. +thold_notification_execute($pid); $end = microtime(true);