diff --git a/.github/workflows/php-unit-tests.yml b/.github/workflows/php-unit-tests.yml index 89eb1ad4..da6cba4a 100644 --- a/.github/workflows/php-unit-tests.yml +++ b/.github/workflows/php-unit-tests.yml @@ -72,8 +72,19 @@ jobs: # Thold contributes no parallel vendor directory or PHPUnit dependency. - name: Build Cacti test image run: | - docker build --tag cacti-web --file cacti-toolchain/docker/Dockerfile cacti-toolchain/docker - docker build --tag cacti-thold-test --file cacti-toolchain/docker/Dockerfile.test cacti-toolchain + for attempt in 1 2 3; do + if docker build --tag cacti-web --file cacti-toolchain/docker/Dockerfile cacti-toolchain/docker && \ + docker build --tag cacti-thold-test --file cacti-toolchain/docker/Dockerfile.test cacti-toolchain; then + exit 0 + fi + + if [ "$attempt" -lt 3 ]; then + sleep 10 + fi + done + + echo 'Cacti test image build failed after three attempts.' >&2 + exit 1 - name: Lint every PHP source file run: | diff --git a/.github/workflows/plugin-ci-workflow.yml b/.github/workflows/plugin-ci-workflow.yml index e17554e8..73630b54 100644 --- a/.github/workflows/plugin-ci-workflow.yml +++ b/.github/workflows/plugin-ci-workflow.yml @@ -149,7 +149,18 @@ jobs: run: | cd ${{ github.workspace }}/cacti if [ -f composer.json ]; then - sudo composer install --prefer-dist --no-progress + for attempt in 1 2 3; do + if sudo composer install --prefer-dist --no-progress --no-interaction; then + exit 0 + fi + + if [ "$attempt" -lt 3 ]; then + sleep 10 + fi + done + + echo 'Composer install failed after three attempts.' >&2 + exit 1 fi - name: Create Cacti config.php diff --git a/tests/Unit/TholdMailNotificationTest.php b/tests/Unit/TholdMailNotificationTest.php new file mode 100644 index 00000000..8ca4eb5c --- /dev/null +++ b/tests/Unit/TholdMailNotificationTest.php @@ -0,0 +1,193 @@ + $overrides + * + * @return array + */ + private function threshold(array $overrides = []) { + return $overrides + [ + 'id' => 1, + 'name_cache' => 'CPU', + 'data_source_name' => 'traffic_in', + 'lastread' => 95, + 'local_graph_id' => 7, + 'acknowledgment' => '', + 'notes' => '', + 'dnotes' => '', + 'external_id' => '', + 'thold_type' => 0, + 'thold_hi' => 90, + 'thold_low' => 10, + 'thold_fail_trigger' => 3, + 'email_body' => '', + ]; + } + + /** + * @return array + */ + private function device() { + return [ + 'id' => 2, + 'description' => 'core-switch-1', + 'hostname' => '10.0.0.1', + 'location' => 'rack 4', + 'site_id' => 1, + ]; + } + + /** + * @param string $recipients + * @param array $overrides + * @param string $type + * + * @return string + */ + private function deliver($recipients, array $overrides = [], $type = 'alert') { + $thold = $this->threshold($overrides); + $device = $this->device(); + + return thold_mail_notification($recipients, 'bcc@example.org', 'ALERT: CPU', $type, 4, [], $thold, $device); + } + + /** + * @return void + */ + public function testRecipientsReceiveTheNotification(): void { + $this->deliver('ops@example.org'); + + $this->assertCount(1, CactiStubs::$mail); + $this->assertSame('ops@example.org', CactiStubs::$mail[0]['to']); + $this->assertSame('bcc@example.org', CactiStubs::$mail[0]['bcc']); + $this->assertSame('ALERT: CPU', CactiStubs::$mail[0]['subject']); + } + + /** + * @return array + */ + public static function emptyRecipientProvider() { + return [ + 'empty string' => [''], + 'whitespace' => [' '], + ]; + } + + /** + * @dataProvider emptyRecipientProvider + * + * @param string $recipients + * + * @return void + */ + public function testNothingIsSentWithoutRecipients($recipients): void { + $this->assertSame('', $this->deliver($recipients)); + $this->assertSame([], CactiStubs::$mail); + } + + /** + * An acknowledged threshold has already been seen by an operator, so it + * stops mailing even while it keeps breaching. + * + * @return void + */ + public function testAcknowledgedThresholdIsNotMailed(): void { + $this->assertSame('', $this->deliver('ops@example.org', ['acknowledgment' => 'on'])); + $this->assertSame([], CactiStubs::$mail); + } + + /** + * Building the body queries, so an empty recipient list must not pay for a + * message nobody receives. + * + * @return void + */ + public function testNoMessageIsBuiltWhenThereIsNoOneToMail(): void { + $this->deliver(''); + + // Composing a body resolves the device's site; skipping it must not. + $site_lookups = array_filter(CactiStubs::$calls, static function ($call) { + return strpos($call['sql'], 'FROM sites') !== false; + }); + + $this->assertSame([], array_values($site_lookups)); + } + + /** + * @return array + */ + public static function textTypeProvider() { + return [ + 'alert' => ['alert', 'thold_alert_text'], + 'warning' => ['warning', 'thold_warning_text'], + 'restoral' => ['restoral', 'thold_restoral_text'], + ]; + } + + /** + * Each class of notification takes its body from its own setting. + * + * @dataProvider textTypeProvider + * + * @param string $type + * @param string $option + * + * @return void + */ + public function testEachNotificationClassUsesItsOwnBody($type, $option): void { + CactiStubs::$configOptions[$option] = 'body for ' . $type; + + $this->assertSame('body for ' . $type, $this->deliver('ops@example.org', [], $type)); + } + + /** + * @return void + */ + public function testTheSentMessageIsReturnedForReuse(): void { + CactiStubs::$configOptions['thold_alert_text'] = 'the body'; + + $this->assertSame('the body', $this->deliver('ops@example.org')); + } + + /** + * @return void + */ + public function testTheListFormatIsResolvedEvenWhenNothingIsSent(): void { + $this->deliver(''); + + $format_lookups = array_filter(CactiStubs::$calls, static function ($call) { + return strpos($call['sql'], 'format_file') !== false; + }); + + $this->assertNotEmpty($format_lookups); + } +} diff --git a/tests/Unit/ThresholdHiLowCharacterizationTest.php b/tests/Unit/ThresholdHiLowCharacterizationTest.php index 85eb1004..d23613f5 100644 --- a/tests/Unit/ThresholdHiLowCharacterizationTest.php +++ b/tests/Unit/ThresholdHiLowCharacterizationTest.php @@ -136,6 +136,24 @@ public function testReadingBetweenWarningAndAlertBoundsNotifiesTheWarning(): voi $this->assertSame([ST_NOTIFYWA], $outcome->logStatuses()); } + /** + * A threshold already in alert whose reading falls back into the warning + * band is a de-escalation, not a restoral, and gets its own notification. + * + * @return void + */ + public function testFallingFromAlertIntoTheWarningBandNotifiesTheDowngrade(): void { + $outcome = $this->bounded([ + 'lastread' => 85, + 'thold_alert' => STAT_HI, + 'thold_fail_count' => 5, + 'thold_warning_fail_count' => 5, + ])->poll(); + + $this->assertSame([ST_NOTIFYAW], $outcome->logStatuses()); + $this->assertStringStartsWith('ALERT > WARNING', $outcome->subjects()[0]); + } + /** * @return void */ diff --git a/thold_functions.php b/thold_functions.php index f195d302..019da05a 100644 --- a/thold_functions.php +++ b/thold_functions.php @@ -2202,16 +2202,53 @@ function thold_datasource_required($name, $data_source) { } /** - * Gather the settings a threshold evaluation reads, in one place. + * Deliver one notification to one recipient list. * - * Everything here is derived from the threshold row and the Cacti settings; it - * does not decide anything and has no side effects, which is what lets it move - * out of thold_check_threshold() without changing behaviour. + * The eighteen send sites in thold_check_threshold() all resolve the list's + * format file, then mail the list unless it is empty or the threshold has been + * acknowledged. The message body is built inside that guard rather than by the + * caller, because building it queries and an empty recipient list should not + * pay for a message nobody receives. * + * @param string $recipients Comma separated addresses, possibly empty. + * @param string $bcc Comma separated blind addresses. + * @param string $subject Subject line, already composed. + * @param string $text_type alert, warning or restoral. + * @param int $list_id Notification list supplying the format. + * @param array $file_array Graph attachment, or empty for none. * @param array $thold_data Threshold row. + * @param array $h Device row. + * @param int $timespan Graph timespan for the attachment. * - * @return array + * @return string The message that was sent, or '' when nothing was. */ +function thold_mail_notification($recipients, $bcc, $subject, $text_type, $list_id, $file_array, &$thold_data, &$h, $timespan = 7) { + $format_file = thold_get_thold_notification_format_file($thold_data['id'], $list_id); + + if (trim($recipients) == '' || $thold_data['acknowledgment'] != '') { + return ''; + } + + switch ($text_type) { + case 'alert': + $message = get_thold_alert_text($thold_data['data_source_name'], $thold_data, $h, $thold_data['lastread'], $thold_data['local_graph_id']); + + break; + case 'warning': + $message = get_thold_warning_text($thold_data['data_source_name'], $thold_data, $h, $thold_data['lastread'], $thold_data['local_graph_id']); + + break; + default: + $message = get_thold_restoral_text($thold_data['data_source_name'], $thold_data, $h, $thold_data['lastread'], $thold_data['local_graph_id']); + + break; + } + + thold_mail($recipients, $bcc, '', $subject, $message, $file_array, '', $list_id, $h, $format_file, $timespan); + + return $message; +} + function thold_evaluation_context(array $thold_data) { $alert_trigger = read_config_option('alert_trigger'); $httpurl = read_config_option('base_url'); @@ -2565,14 +2602,7 @@ function thold_check_threshold(&$thold_data) { logger($subject, $url, $syslog_priority, $syslog_facility); } - $notify_list_id = $thold_data['notify_warning']; - $format_file = thold_get_thold_notification_format_file($thold_data['id'], $notify_list_id); - - if (trim($warning_emails) != '' && $thold_data['acknowledgment'] == '') { - $message = get_thold_warning_text($thold_data['data_source_name'], $thold_data, $h, $thold_data['lastread'], $thold_data['local_graph_id']); - - thold_mail($warning_emails, $warning_bcc_emails, '', $subject, $message, $file_array, '', $notify_list_id, $h, $format_file, $thold_data['graph_timespan']); - } + $message = thold_mail_notification($warning_emails, $warning_bcc_emails, $subject, 'warning', $thold_data['notify_warning'], $file_array, $thold_data, $h, $thold_data['graph_timespan']); $save = [ 'class' => 'warn', @@ -2627,14 +2657,7 @@ function thold_check_threshold(&$thold_data) { $subject = get_email_subject('ALERT > WARNING', false, $lastread, $ra, $warning_breach_up, $thold_data); if (!$suspend_notify && !$maint_dev) { - $notify_list_id = $thold_data['notify_alert']; - $format_file = thold_get_thold_notification_format_file($thold_data['id'], $notify_list_id); - - if (trim($alert_emails) != '' && $thold_data['acknowledgment'] == '') { - $message = get_thold_warning_text($thold_data['data_source_name'], $thold_data, $h, $thold_data['lastread'], $thold_data['local_graph_id']); - - thold_mail($alert_emails, $alert_bcc_emails, '', $subject, $message, $file_array, '', $notify_list_id, $h, $format_file, $thold_data['graph_timespan']); - } + $message = thold_mail_notification($alert_emails, $alert_bcc_emails, $subject, 'warning', $thold_data['notify_alert'], $file_array, $thold_data, $h, $thold_data['graph_timespan']); if ($notify_different) { $notify_list_id = $thold_data['notify_warning']; @@ -2885,14 +2908,7 @@ function thold_check_threshold(&$thold_data) { logger($subject, $url, $syslog_priority, $syslog_facility); } - $notify_list_id = $thold_data['notify_alert']; - $format_file = thold_get_thold_notification_format_file($thold_data['id'], $notify_list_id); - - if (trim($alert_emails) != '' && $thold_data['acknowledgment'] == '') { - $message = get_thold_restoral_text($thold_data['data_source_name'], $thold_data, $h, $thold_data['lastread'], $thold_data['local_graph_id']); - - thold_mail($alert_emails, $alert_bcc_emails, '', $subject, $message, $file_array, '', $notify_list_id, $h, $format_file, $thold_data['graph_timespan']); - } + $message = thold_mail_notification($alert_emails, $alert_bcc_emails, $subject, 'restoral', $thold_data['notify_alert'], $file_array, $thold_data, $h, $thold_data['graph_timespan']); if ($notify_different) { $notify_list_id = $thold_data['notify_warning']; @@ -3003,14 +3019,7 @@ function thold_check_threshold(&$thold_data) { logger($subject, $url, $syslog_priority, $syslog_facility); } - $notify_list_id = $thold_data['notify_alert']; - $format_file = thold_get_thold_notification_format_file($thold_data['id'], $notify_list_id); - - if (trim($alert_emails) != '' && $thold_data['acknowledgment'] == '') { - $message = get_thold_alert_text($thold_data['data_source_name'], $thold_data, $h, $thold_data['lastread'], $thold_data['local_graph_id']); - - thold_mail($alert_emails, $alert_bcc_emails, '', $subject, $message, $file_array, '', $notify_list_id, $h, $format_file, $thold_data['graph_timespan']); - } + $message = thold_mail_notification($alert_emails, $alert_bcc_emails, $subject, 'alert', $thold_data['notify_alert'], $file_array, $thold_data, $h, $thold_data['graph_timespan']); if ($notify_different) { $notify_list_id = $thold_data['notify_warning']; @@ -3232,14 +3241,7 @@ function thold_check_threshold(&$thold_data) { logger($subject, $url, $syslog_priority, $syslog_facility); } - $notify_list_id = $thold_data['notify_alert']; - $format_file = thold_get_thold_notification_format_file($thold_data['id'], $notify_list_id); - - if (trim($alert_emails) != '' && $thold_data['acknowledgment'] == '') { - $message = get_thold_alert_text($thold_data['data_source_name'], $thold_data, $h, $thold_data['lastread'], $thold_data['local_graph_id']); - - thold_mail($alert_emails, $alert_bcc_emails, '', $subject, $message, $file_array, '', $notify_list_id, $h, $format_file, $thold_data['graph_timespan']); - } + $message = thold_mail_notification($alert_emails, $alert_bcc_emails, $subject, 'alert', $thold_data['notify_alert'], $file_array, $thold_data, $h, $thold_data['graph_timespan']); if ($notify_different) { $notify_list_id = $thold_data['notify_warning'];