mirror of
https://github.com/Cacti/cacti.git
synced 2026-10-31 09:07:38 +09:00
* fix(boost): harden data handoffs and cache writes Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Define $updates_ok when there is nothing to flush PHPStan level 6 also flags the poller ownership check, which is the POLLER_ID constant case the file already annotates above. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Use a line comment to match the fixer Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Record the boost process columns in the audit schema Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: resolve includes through Cacti path constants Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Fix Boost handoff review concerns Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: seed settings in Boost database fixture Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: refresh SQL interpolation baseline Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * docs: record Boost handoff hardening Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix: stop boost discarding a cycle and wedging a shard Three defects in the handoff hardening. A rejected ownership handoff returned false, which the poller reads as boost having consumed the data, so it skipped the RRD write. The poller_output rows were already deleted by then, so a stale poller_item copy on a remote poller silently lost the whole cycle. Return true instead and let the poller write directly, matching the database failure path. Any non-OK RRDtool response retained the shard, and the retry then replayed rows already written, which RRDtool rejects as an illegal update time. That failed again on every pass, so the archive tables accumulated and boost never drained. Treat an at-or-behind timestamp as applied, since the sample is in the RRD; genuine errors still retain the shard. The two new poller_output_boost_processes columns passed 'null' where db_install_update_table reads 'NULL', so NOT NULL was never emitted and upgraded installs diverged from cacti.sql. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix: spell the NULL column key so upgrades emit NOT NULL db_add_column() reads $column['NULL'] and PHP array keys are case sensitive, so the thirteen definitions in the 1.3.0 upgrade that passed a lowercase 'null' key had the flag discarded and created nullable columns. Fresh installs built from cacti.sql got NOT NULL, upgraded installs did not, and Cacti's own schema audit reports the difference. The same file already spells the key in upper case 144 times, and the wrapper's own docblock example uses the upper case form. Guard the class with a test over every upgrade file rather than only the one that was wrong. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: assert the NULL key plainly toContain() takes needles rather than a failure message. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(boost): close remaining handoff review gaps Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(boost): guard nullable graph output Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(boost): keep in-flight completion rows through the 1.3.0 upgrade The truncate only existed so the run_child UNIQUE key could build over rows that predate run_id; deleting just those rows leaves a running boost its completion records. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * refactor(boost): drop the poller ownership gate Its boost_poller_on_demand() call sits behind the boost_redirect early return that poller.php forces on for every multi-collector install, and its recovery call turns a device deletion on main into a recovery that never brings the collector back online. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(boost): bound atomic cache write allocations Guard upgrade source reads so failures report clearly, write graph cache data in fixed-size chunks, and exercise a multi-chunk payload. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(boost): support Windows cache replacement Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(boost): expand handoff failure coverage Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(boost): support ANSI_QUOTES upgrade checks Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(boost): stop staging after chunk failure Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(recovery): use numeric poller status Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(sql): support ANSI_QUOTES in recovery paths Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(boost): preserve cache on replacement failure Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(boost): retain readable cache permissions Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(boost): repair completion schema at runtime Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(recovery): bound timestamp batch fetches Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(boost): satisfy runtime schema analysis Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(recovery): restore status after transfer failure Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(boost): rely on unit bootstrap helpers Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(boost): refresh generated cache secret Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(boost): normalize cache path separators Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(boost): fail clearly on unreadable fixtures * fix(boost): name ignored duplicate sample keys The staging warning only said rows were dropped, so a retry could not be told from a colliding writer. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Lack of pre-commit hook execution allowed lint failure --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: TheWitness <thewitness@cacti.net>
228 lines
11 KiB
PHP
228 lines
11 KiB
PHP
<?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 |
|
|
+-------------------------------------------------------------------------+
|
|
*/
|
|
|
|
/*
|
|
* Tests for the boost poller correctness fixes:
|
|
*
|
|
* 1. Startup barrier waits for every launched child to register before the
|
|
* parent drains and drops archive tables.
|
|
* 2. The archive-table fallback validates the parent-supplied name and confirms
|
|
* it with a data-plane read instead of the lagging SHOW TABLES metadata.
|
|
* 3. boost_parallel clamping and the log-path allow-list are shared, single
|
|
* sources of truth and the allow-list accepts Windows paths.
|
|
*
|
|
* The pure helpers (clamp, name validation, log-path safety, barrier predicate)
|
|
* are exercised behaviourally. The unit bootstrap provides core helpers before
|
|
* lib/boost.php is loaded. The wiring in poller_boost.php and the concurrency
|
|
* race itself are asserted by reading the source; the full multi-process race
|
|
* needs the integration suite.
|
|
*/
|
|
|
|
require_once CACTI_PATH_LIBRARY . '/boost.php';
|
|
|
|
$boostPollerPath = CACTI_PATH_BASE . '/poller_boost.php';
|
|
$boostLibPath = CACTI_PATH_LIBRARY . '/boost.php';
|
|
|
|
test('boost_clamp_parallel maps misconfigured values to a single child', function () {
|
|
expect(boost_clamp_parallel(''))->toBe(1);
|
|
expect(boost_clamp_parallel(null))->toBe(1);
|
|
expect(boost_clamp_parallel(0))->toBe(1);
|
|
expect(boost_clamp_parallel('0'))->toBe(1);
|
|
expect(boost_clamp_parallel(-4))->toBe(1);
|
|
expect(boost_clamp_parallel('-4'))->toBe(1);
|
|
expect(boost_clamp_parallel('abc'))->toBe(1);
|
|
});
|
|
|
|
test('boost_clamp_parallel preserves valid process counts', function () {
|
|
expect(boost_clamp_parallel('8'))->toBe(8);
|
|
expect(boost_clamp_parallel(8))->toBe(8);
|
|
expect(boost_clamp_parallel(1))->toBe(1);
|
|
expect(boost_clamp_parallel('16'))->toBe(16);
|
|
});
|
|
|
|
test('boost_is_valid_archive_table accepts well-formed names and rejects injection', function () {
|
|
expect(boost_is_valid_archive_table('poller_output_boost_arch_123'))->toBeTrue();
|
|
expect(boost_is_valid_archive_table('poller_output_boost_arch_1746000000'))->toBeTrue();
|
|
|
|
expect(boost_is_valid_archive_table(''))->toBeFalse();
|
|
expect(boost_is_valid_archive_table('poller_output_boost_arch_'))->toBeFalse();
|
|
expect(boost_is_valid_archive_table('poller_output_boost_arch_abc'))->toBeFalse();
|
|
expect(boost_is_valid_archive_table('poller_output_boost_arch_123; DROP TABLE users'))->toBeFalse();
|
|
expect(boost_is_valid_archive_table('poller_output_boost_arch_1 --x'))->toBeFalse();
|
|
expect(boost_is_valid_archive_table("poller_output_boost_arch_1\n--x"))->toBeFalse();
|
|
expect(boost_is_valid_archive_table('../poller_output_boost_arch_1'))->toBeFalse();
|
|
expect(boost_is_valid_archive_table('`poller_output_boost_arch_1`'))->toBeFalse();
|
|
expect(boost_is_valid_archive_table(123))->toBeFalse();
|
|
expect(boost_is_valid_archive_table(null))->toBeFalse();
|
|
});
|
|
|
|
test('boost_log_path_is_safe accepts a unix path and rejects shell metacharacters', function () {
|
|
expect(boost_log_path_is_safe('/var/log/cacti/boost.log'))->toBeTrue();
|
|
expect(boost_log_path_is_safe('boost.log'))->toBeTrue();
|
|
|
|
expect(boost_log_path_is_safe(''))->toBeFalse();
|
|
expect(boost_log_path_is_safe(null))->toBeFalse();
|
|
expect(boost_log_path_is_safe('/var/log/$(touch pwn)'))->toBeFalse();
|
|
expect(boost_log_path_is_safe('/var/log/boost.log; rm -rf /'))->toBeFalse();
|
|
expect(boost_log_path_is_safe('/var/log/`id`.log'))->toBeFalse();
|
|
expect(boost_log_path_is_safe('/var/log/boost.log | cat'))->toBeFalse();
|
|
});
|
|
|
|
test('boost_log_path_is_safe allow-list permits Windows path characters', function () {
|
|
// On non-win32 the plain class is applied, so assert the win32 class directly
|
|
// against the same metacharacter set the helper uses. This is the regex the
|
|
// helper runs under PHP_OS_FAMILY === 'Windows'.
|
|
$winClass = '/[^A-Za-z0-9_.\/\\\\: -]/';
|
|
|
|
expect(preg_match($winClass, 'C:/cacti/log/boost.log'))->toBe(0);
|
|
expect(preg_match($winClass, 'C:\\cacti\\log\\boost.log'))->toBe(0);
|
|
expect(preg_match($winClass, 'C:\\Program Files\\Cacti\\boost.log'))->toBe(0);
|
|
|
|
// Even with the relaxed class, shell metacharacters are still rejected.
|
|
expect(preg_match($winClass, 'C:\\cacti\\$(pwn).log'))->toBe(1);
|
|
expect(preg_match($winClass, 'C:\\cacti\\boost.log;del'))->toBe(1);
|
|
expect(preg_match($winClass, 'C:\\cacti\\`id`.log'))->toBe(1);
|
|
});
|
|
|
|
test('boost_all_children_registered holds until every launched child is accounted for', function () {
|
|
// Four launched, only one registered so far: barrier must not release.
|
|
expect(boost_all_children_registered(4, 1, 0))->toBeFalse();
|
|
expect(boost_all_children_registered(4, 0, 0))->toBeFalse();
|
|
|
|
// The race this closes: a fast child finishes (running drops, completed rises)
|
|
// while siblings are still booting. running + completed still under expected.
|
|
expect(boost_all_children_registered(4, 2, 1))->toBeFalse();
|
|
|
|
// All four present, whether still running, finished, or a mix.
|
|
expect(boost_all_children_registered(4, 4, 0))->toBeTrue();
|
|
expect(boost_all_children_registered(4, 0, 4))->toBeTrue();
|
|
expect(boost_all_children_registered(4, 1, 3))->toBeTrue();
|
|
expect(boost_all_children_registered(1, 1, 0))->toBeTrue();
|
|
});
|
|
|
|
test('archive-table fallback no longer relies on the lagging SHOW TABLES probe', function () use ($boostLibPath) {
|
|
$contents = file_get_contents($boostLibPath);
|
|
|
|
$func_pos = strpos($contents, 'function boost_get_arch_table_names');
|
|
$func_end = strpos($contents, "\nfunction ", $func_pos + 1);
|
|
$func_body = substr($contents, $func_pos, $func_end - $func_pos);
|
|
|
|
// db_table_exists() runs SHOW TABLES, the same metadata query that lags under
|
|
// the replication this fix targets; the fallback must not depend on it.
|
|
expect($func_body)->not->toContain('db_table_exists(');
|
|
|
|
// The fallback must validate the name and confirm it with a data-plane read.
|
|
expect($func_body)->toContain('boost_is_valid_archive_table($latest_table)');
|
|
expect($func_body)->toContain('boost_archive_table_readable($latest_table)');
|
|
expect($func_body)->toContain("Boost ignored an unexpected archive-like table name: ' . \$display_name");
|
|
});
|
|
|
|
test('boost_archive_table_readable probes data, not metadata, and rejects bad names', function () use ($boostLibPath) {
|
|
$contents = file_get_contents($boostLibPath);
|
|
|
|
$func_pos = strpos($contents, 'function boost_archive_table_readable');
|
|
$func_end = strpos($contents, "\nfunction ", $func_pos + 1);
|
|
$func_body = substr($contents, $func_pos, $func_end - $func_pos);
|
|
|
|
// Validate the interpolated name before it reaches SQL.
|
|
expect($func_body)->toContain('boost_is_valid_archive_table($table)');
|
|
|
|
// Read from the table itself (data plane), not SHOW TABLES / information_schema.
|
|
expect($func_body)->toContain('SELECT COUNT(*) FROM `$table`');
|
|
expect($func_body)->not->toContain('SHOW TABLES');
|
|
expect($func_body)->not->toContain('information_schema');
|
|
});
|
|
|
|
test('parent waits for all launched children before draining', function () use ($boostPollerPath) {
|
|
$contents = file_get_contents($boostPollerPath);
|
|
expect($contents)->not->toBeFalse();
|
|
$contents = (string) $contents;
|
|
|
|
// boost_launch_children() returns the launched count and the barrier waits on
|
|
// boost_all_children_registered() rather than releasing on the first signup.
|
|
expect($contents)->toContain('$expected_children = boost_launch_children();');
|
|
expect($contents)->toContain('boost_all_children_registered($expected_children');
|
|
expect($contents)->not->toContain('boost_processes_running() < 1');
|
|
|
|
// A deadline still bounds the wait, with a log line if it expires early.
|
|
expect($contents)->toContain('$startup_deadline');
|
|
expect($contents)->toContain('Boost startup barrier timed out');
|
|
});
|
|
|
|
test('drain loop exits only when every child has recorded completion', function () use ($boostPollerPath) {
|
|
$contents = file_get_contents($boostPollerPath);
|
|
expect($contents)->not->toBeFalse();
|
|
$contents = (string) $contents;
|
|
|
|
// The old loop exited as soon as no child was running. The fix also requires
|
|
// every launched child to have a completion row before draining.
|
|
expect($contents)->toContain('boost_completed_children($run_id) < $expected_children');
|
|
expect($contents)->not->toMatch('/while\s*\(\s*\$running\s*=\s*boost_processes_running\(\)\s*\)/');
|
|
});
|
|
|
|
test('end-of-run cleanup drops only this run\'s archive tables', function () use ($boostPollerPath) {
|
|
$contents = file_get_contents($boostPollerPath);
|
|
|
|
// The unconditional DROP over every poller_output_boost_arch_% table could
|
|
// destroy a newer rotation or an older crashed run still holding rows. The
|
|
// cleanup must iterate the recorded in-scope set instead.
|
|
expect($contents)->toContain('foreach ($boost_run_arch_tables as $table)');
|
|
|
|
// Each dropped name is re-validated before interpolation.
|
|
$drop_pos = strpos($contents, 'foreach ($boost_run_arch_tables as $table)');
|
|
$segment = substr($contents, $drop_pos, 400);
|
|
expect($segment)->toContain('boost_is_valid_archive_table($table)');
|
|
expect($segment)->toContain('DROP TABLE IF EXISTS `$table`');
|
|
});
|
|
|
|
test('both boost_parallel call sites use the shared clamp helper', function () use ($boostPollerPath) {
|
|
$contents = file_get_contents($boostPollerPath);
|
|
|
|
// boost_prepare_process_table() and boost_launch_children() must agree on the
|
|
// process count so the parent spawns exactly what it later waits for.
|
|
expect(substr_count($contents, 'boost_clamp_parallel(read_config_option(\'boost_parallel\'))'))->toBeGreaterThanOrEqual(2);
|
|
|
|
// The old divergent guards must be gone.
|
|
expect($contents)->not->toContain('if (empty($processes)) {');
|
|
});
|
|
|
|
test('boost_launch_children uses the shared log-path safety helper', function () use ($boostPollerPath) {
|
|
$contents = file_get_contents($boostPollerPath);
|
|
|
|
expect($contents)->toContain('boost_log_path_is_safe($boost_log)');
|
|
});
|
|
|
|
test('parent and child repair the completion schema before using it', function () use ($boostPollerPath) {
|
|
$contents = file_get_contents($boostPollerPath);
|
|
|
|
$master_repair = strpos($contents, 'boost_ensure_process_table(true)');
|
|
$truncate = strpos($contents, "db_execute('TRUNCATE TABLE poller_output_boost_processes')");
|
|
$child_start = strpos($contents, "cacti_log('INFO: Boost register child process '");
|
|
$child_repair = strpos($contents, 'boost_ensure_process_table(true)', $master_repair + 1);
|
|
$child_insert = strpos($contents, "db_execute_prepared('INSERT INTO poller_output_boost_processes", $child_start);
|
|
|
|
expect($master_repair)->not->toBeFalse()
|
|
->and($truncate)->not->toBeFalse()
|
|
->and($master_repair)->toBeLessThan($truncate)
|
|
->and($child_repair)->not->toBeFalse()
|
|
->and($child_insert)->not->toBeFalse()
|
|
->and($child_repair)->toBeLessThan($child_insert);
|
|
});
|
|
|
|
test('uses the unit bootstrap cacti_sizeof implementation', function () {
|
|
$contents = file_get_contents(__FILE__);
|
|
|
|
expect($contents)->not->toContain('function cacti_' . 'sizeof(');
|
|
});
|