9 Commits
Author SHA1 Message Date
63a53d511b fix: Make boost_redirect the default boost behavior (develop port of #8559) (#8561)
* fix: Make boost_redirect the default boost behavior

Port of the 1.2.x change (issue#8470) to develop. Boost redirection is now
permanent whenever boost is enabled, so the boost_redirect setting is removed
and the collector always stages poller output directly into
poller_output_boost.

- cmd.php: drop the boost_redirect guard; stage whenever boost_rrd_update_enable
  is on.
- include/global_settings.php, settings.php: remove the boost_redirect setting
  and its UI toggles.
- lib/snmpagent.php: always report the redirect object as enabled.
- poller.php, settings.php: stop force-setting boost_redirect for multi-poller
  installs (boost_rrd_update_enable alone now implies redirection).
- lib/boost.php: boost_poller_on_demand() no longer stages inline; cmd.php owns
  staging, so it only reports rows as already staged and never re-inserts.

Tests are updated to match the single, permanent-redirect pipeline: the
hand-off acknowledgement and consolidation contracts point at the surviving
cmd.php/poller_recovery.php call sites, and the PollerOutput probe drops the
now-dead redirect-off staging scenarios.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* test(coverage): cover the permanent Boost redirect in snmpagent

Add a request-coverage probe that exercises the permanent
boostApplRrdUpdateRedirect assignment in snmpagent_global_settings_update().
The statement lives in a MibCache-driven function that cannot run under the
unit/integration coverage driver without a live SNMP agent cache, so the
single changed line is compiled at its original line number and executed
against a minimal MibCache stand-in, with Xdebug coverage remapped to
lib/snmpagent.php. Register the probe so the changed-PHP coverage gate runs
it whenever lib/snmpagent.php changes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* i18n:

* tests: Cover the cmd.php Boost flush guards for the changed-line gate

Making boost_redirect the default removed the boost_redirect half of the three poller flush conditions in cmd.php, leaving bare read_config_option(boost_rrd_update_enable) guards at lines 392, 501, and 537. Those statements live in cmd.php top-level poller loop, so the unit/integration driver includes them in the denominator but never executes them, dropping changed-line coverage to 3 of 6 (50 percent).

Adds a request-coverage probe that compiles each changed guard at its original line number and executes it against minimal read_config_option/boost_flush_output_batch stand-ins, mirroring the existing snmpagent redirect probe. Xdebug hits are remapped to cmd.php, raising changed-line coverage to 6 of 6 (100 percent). Registers the probe in run_request_coverage.sh so it runs whenever cmd.php changes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(boost): keep the boost_redirect compatibility flag enabled

Port the poller.php compatibility assignment from the 1.2.x change
(#8559, commit ba2ab827) that this develop port dropped. Supported
Spine versions still read the hidden boost_redirect flag to stage
poller output into poller_output_boost; without it a compatible
collector never stages, yet boost_poller_on_demand() still reports
the rows as staged and poller_acknowledge_output() deletes them,
losing samples. Set the flag unconditionally before launching
collectors so redirection stays permanent whenever Boost is enabled.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: TheWitness <copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-11-02 17:28:47 +00:00
TheWitnessandCopilot 76a916cf03 fix(boost): never re-stage poller output when boost_redirect is on (#8374)
* fix(boost): never re-stage poller output when boost_redirect is on

#8332 gated the boost_redirect bypass in boost_poller_on_demand() on the
caller not passing $staged. process_poller_output() always passes it, so
the bypass never fired and the main poller re-inserted every row spine or
cmd.php had already written to poller_output_boost, flooding the log with
"Boost staging ignored N of N duplicate sample keys" warnings.

When boost_redirect is enabled the poller now never inserts into
poller_output_boost and reports the rows as staged so they are cleared
from poller_output. poller_recovery.php writes recovered remote rows via
boost_flush_output_batch() directly and is unaffected.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: TheWitness <thewitness@cacti.net>

* chore(changelog): use changelog.d fragment for #8374

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: TheWitness <thewitness@cacti.net>

* fix(boost): log failed poller_output_boost staging as an ERROR

boost_flush_output_batch() returned false silently when the database did
not acknowledge a chunk. cmd.php's boost_redirect staging ignores that
return, so a failed collector-side write left no trace. Log a BOOST ERROR
with the sample count from the helper so every caller's failure is visible.

Adds in-process coverage for the failure log and for boost_poller_on_demand()
never re-staging rows when boost_redirect is on.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: TheWitness <thewitness@cacti.net>

---------

Signed-off-by: TheWitness <thewitness@cacti.net>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-10-14 21:13:43 -04:00
Thomas Vincent a12fe59627 fix(poller): [develop] retain output until bounded storage batches succeed (#8332)
* fix(poller): retain output until bounded storage batches succeed

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test(poller): refresh SQL audit after exact-key acknowledgements

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test(boost): preserve handoff contracts with explicit acknowledgements

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(poller): retain failed timestamp ordering across batches

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(poller): acknowledge complete timestamp groups together

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(poller): retry failed source metadata reads on develop

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(poller): preload standalone script exclusions once per process

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(poller): bound metadata failures and validate acknowledgement queries

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
2026-10-12 15:41:53 -07:00
Thomas Vincent 76579e6292 Fix dsstats partition dispatch and profile retention fallbacks (#8304)
* fix(core): preserve dsstats type and render retention fallbacks

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test(auth): use certificate expiry truth in rendering assertions

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
2026-10-11 21:32:00 -04:00
TheWitness 0415606710 fix: skip replication-ignored files in cache_in_path() to stop poll-cycle log spam (#8127)
* fix: skip replication-ignored files in cache_in_path() to stop poll-cycle log spam

cache_in_path() emitted a 'Detecting Resource Change' log and called update_db_from_path() for tooling artifacts (.gitignore, .mdlrc, tests/, ...) that update_db_from_path() then declines to cache. Because their md5 is never stored, every poll cycle re-detected them as changed, spamming the Cacti log once per minute. Guard cache_in_path() with should_ignore_from_replication() up front so these paths are skipped entirely.

* test: assert cache_in_path() consults the ignore predicate before logging/updating

Regression guard for the per-poll-cycle log spam fix. cache_in_path() runs
change detection before update_db_from_path(), so it must short-circuit on
should_ignore_from_replication() up front. Without that early return an
ignored dotfile (.gitignore, .mdlrc, ...) is logged and re-processed every
cycle because its md5 is never stored and it is forever re-detected as
"changed". The new test pins the guard ahead of the first change-detection
log line in cache_in_path().

* fix: restore literal &lt;?php opening tag in replication exclusion test

The previous commit stored the opening tag as an HTML entity, which would
make the test file fail to parse.
2026-10-01 12:03:52 -05:00
TheWitness ac2445fab5 Exclude 'tests' directories from resource replication (#8118)
* poller: exclude 'tests' directories from resource replication

should_ignore_from_replication() now skips any path with a 'tests' segment, covering Cacti's own tests/ and every plugin's tests/ for both the cache-in (md5sum_path/update_db_from_path) and cache-out (resource_cache_out) directions. update_resource_cache()'s plugin scan also adds 'tests' to its directory exclusions so a plugin's tests/ tree is never cached in the first place. Adds a Pest unit test covering both directions and the plugin-scan exclusion.

* poller: purge stale excluded rows, guard collector dirs, strip install root

Address review feedback on #8118:

- should_ignore_from_replication() now strips CACTI_PATH_BASE from absolute file paths before segment matching, so an installation living under a 'tests' ancestor (e.g. /srv/tests/cacti) no longer rejects ordinary files.

- update_resource_cache() purges rows that are now excluded even when their source file still exists, dropping tests/* rows cached before the exclusion.

- The remote collector skips excluded plugin rows before building directories, so it no longer recreates plugins/foo/tests/ from stale cache rows.

Adds regression tests for the absolute-path handling and both source-level guards.
2026-09-29 23:08:10 -04:00
Thomas Vincent 3f377531b7 fix(poller): stop the registry pid probe narrowing a wide pid to -1 (#7956)
* fix(poller): stop the registry pid probe narrowing a wide pid to -1

processes.pid is int(10) unsigned, so a corrupted row can hold 4294967295.
Through PHP 8.4 posix_kill() narrows that to a 32 bit pid_t of -1 and kill(2)
reads it as every process the caller may signal, so cacti_process_pid_exists()
calls a dead registry row live and the two kill sites that ask it first SIGTERM
the caller's whole session. From PHP 8.5 the same value raises a ValueError and
ends the poller instead. Refusing it ahead of the call covers both.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* hardening: bound every process table PID before signalling it

Twelve call sites read a pid out of a process table and handed it straight to
posix_kill(). processes.pid is int(10) unsigned, so a corrupted row can carry
4294967295, which narrows to a 32 bit pid_t of -1 and signals every process
the caller owns, or from PHP 8.5 raises a ValueError that ends the collector.
They now go through cacti_process_kill(), which refuses init, anything that
cannot name a process, and anything wider than pid_t.

poller_recovery read its pid with a bare posix_kill($pid, 0) and treats false
as "stale, clear the row and run". cacti_process_signalable() keeps exactly
that meaning, including EPERM as stale, and adds the bound. Neither
cacti_process_pid_exists() nor cacti_process_still_running() fits there: both
report a pid live when signalling it is merely forbidden, so a recycled
recovery_pid would have wedged recovery until an operator cleared the row.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test: place process guards in poller domain

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(poller): honor platform PID ranges

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test(poller): cover platform signal semantics

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
2026-09-12 15:24:27 -04:00
Thomas Vincent 49f5212181 fix(tests): restore paths and isolate test doubles (#7764)
* fix(tests): correct path depths after the domain reorganisation

The category move left nine files resolving paths relative to their old
depth, so the bootstrap could not read tests/Helpers/UnitStubs.php and no
test ran at all.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix tests without replacing production functions

* remove unrelated failing integration tests

* remove unrelated broken test workflows

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
2026-08-19 12:28:45 -07:00
Sean Mancini 96033267cb refactor(tests): reorganize unit tests into domain category folders (#7733)
* refactor(tests): reorganize unit tests into domain category folders

- Consolidate per-advisory GHSA regression tests into Security/ subfolders by vuln class
- Group all 186 test files into domain folders: DataCollection/, Ui/, Core/,
  Database/, Installer/, Scripts/, Security/, Plugin/
- Strip GitHub issue/GHSA numbers from filenames; keep IDs in test descriptions
- Fix all __DIR__ relative paths to match new folder nesting depth
- Update CI workflow path triggers and phpunit XML configs for new file locations
- Update .github/copilot-instructions.md with unit test organization guidelines
- Add tests/vendor/ to .gitignore

* add gaurds

* fix up guard for unit tests

* fix redeclaration of cacti_log in tests

* fix updates to unit tests

* fix test patjhs

* fix: update test __DIR__ paths for reorganized test structure

* fix: conditionally define read_user_setting to prevent redeclaration in tests

* fix: conditionally define functions to prevent redeclaration in tests

- Wrap clean_up_lines, raise_message, cacti_debug_backtrace, and get_debug_prefix
  with function_exists() checks to prevent conflicts when test stubs are loaded
  before lib/functions.php in the same PHP process

* fix: add unit test bootstrap to prevent function redeclaration errors

- Create tests/bootstrap-unit.php to load core libraries before tests
- Ensures lib/functions.php loads before test stubs define stub functions
- Test stubs use function_exists() checks, so they detect pre-loaded functions
- Update phpunit.xml to use bootstrap-unit.php for Unit testsuite
- Fixes 'Cannot redeclare read_user_setting()' errors when tests run in same process

* fix: load test stubs before global.php in unit test bootstrap

- Reorder bootstrap to load CactiStubs.php and UnitStubs.php first
- Ensures stub functions like __() are available when global.php loads
- global.php calls get_cacti_version_text() which needs __() for translations
- Fixes 'Call to undefined function __()' error during bootstrap

* fix: remove function conflicts from test stubs

- Remove stub definitions for functions already in lib/functions.php:
  cacti_sizeof, cacti_log, cacti_strtolower, read_config_option,
  read_user_setting, cacti_count
- Keep translation function stubs (__(), __esc()) needed by global.php
- Update bootstrap to load lib/functions.php after stubs
- This prevents function redeclaration errors while maintaining test isolation

* fix: prevent constant and variable redeclaration in bootstrap

- Remove explicit PHP_TESTING and CACTI_TEST_BOOTSTRAP defines from bootstrap
  (CactiStubs.php now handles this with guards)
- Add guards to CactiStubs.php to prevent redefinition of PHP_TESTING
- Only set CACTI_TEST_BOOTSTRAP env vars if not already set
- Fixes PHP warnings about 'Constant X already defined'

* fix: remove redundant lib/functions.php load from bootstrap

include/global.php already loads lib/functions.php at lines 287-288.
The bootstrap was trying to load it again after global.php, but global.php
needs format_cacti_version() to be available on lines 293-294 to define
CACTI_VERSION and CACTI_VERSION_FULL constants.

Solution: Let global.php handle loading lib/functions.php itself, and don't
load it separately in the bootstrap.

* fix: remove more conflicting function stubs from test helpers

- Remove stubs for functions also defined in lib/functions.php:
  raise_message, clean_up_lines, cacti_debug_backtrace, get_debug_prefix
- Remove POLLER_ID constant stub (defined unconditionally by global.php)
- Keep only translation functions and test-specific helpers
- This prevents function/constant redeclaration errors

* fix: minimize test stubs to only translation functions

Remove ALL helper functions and constants from UnitStubs.php that are:
- Defined in lib/functions.php (raise_message, clean_up_lines, etc.)
- Defined in lib/html_utility.php (srv, set_request_var)
- Defined in global_constants.php (POLLER_VERBOSITY_*, MESSAGE_LEVEL_*)
- Defined in global.php (POLLER_ID)

Keep ONLY translation functions (__(), __esc()) that are:
- Called by global.php during initialization
- Not defined elsewhere
- Required for proper bootstrap sequence

This eliminates all constant and function redeclaration errors.

* fix: remove __esc stub - already defined in global_languages.php

Keep only __() translation function stub, which is needed by global.php
and not defined elsewhere in Cacti core.

* fix: handle no-arg case in __() translation stub

The __() function may be called with just a string and no format arguments.
Previously, vsprintf() would fail with 'arguments array must contain 2 items'.
Now, if no args are provided, return the text as-is.
2026-08-16 19:23:31 -07:00