Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 27 additions & 2 deletions ntfy/events/imagecomplete_ntfy.event.php
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,33 @@ public function __construct()
*/
public function onEvent($event, $data)
{
self::$message = 'This host has finished imaging.';
self::$shortdesc = 'Imaging Complete';
// One listener, two names: HOST_IMAGE_COMPLETE is a deploy finishing
// and HOST_IMAGEUP_COMPLETE is a capture. Core never fired the capture
// name at all until fogproject#1202, so both outcomes arrived as a
// deploy and this message could not tell them apart.
//
// Composed here rather than left as a bare literal for the parent to
// translate, because the image name has to be substituted OUTSIDE _().
// The parent's _() then finds no entry and passes the finished string
// through, which is what we want. Read defensively: this plugin has to
// keep working against a server that has not taken #1202 yet.
$image = (string) ($data['ImageName'] ?? '');
if ('' === $image) {
$image = _('an unnamed image');
}
if ('HOST_IMAGEUP_COMPLETE' === $event) {
self::$shortdesc = _('Capture Complete');
self::$message = sprintf(
_('This host has finished capturing image %s.'),
$image
);
} else {
self::$shortdesc = _('Deploy Complete');
self::$message = sprintf(
_('This host has finished deploying image %s.'),
$image
);
}
parent::onEvent($event, $data);
}
}
21 changes: 19 additions & 2 deletions ntfy/events/imagefail_ntfy.event.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,25 @@ public function __construct()
*/
public function onEvent($event, $data)
{
self::$message = 'This host has failed to image';
self::$shortdesc = 'Failed';
// HOST_IMAGE_FAIL had no core caller at all until fogproject#1202, so
// this listener has never run on any server. The reason is the part an
// admin can act on, so say it rather than "failed to image". Read
// defensively: this plugin has to keep working against a server that
// has not taken #1202 yet.
$image = (string) ($data['ImageName'] ?? '');
if ('' === $image) {
$image = _('an unnamed image');
}
$reason = (string) ($data['Reason'] ?? '');
if ('' === $reason) {
$reason = _('no reason was reported');
}
self::$shortdesc = _('Imaging Failed');
self::$message = sprintf(
_('This host failed imaging %1$s: %2$s'),
$image,
$reason
);
parent::onEvent($event, $data);
}
}
29 changes: 27 additions & 2 deletions pushbullet/events/imagecomplete_pushbullet.event.php
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,33 @@ public function __construct()
*/
public function onEvent($event, $data)
{
self::$message = 'This host has finished imaging.';
self::$shortdesc = 'Imaging Complete';
// One listener, two names: HOST_IMAGE_COMPLETE is a deploy finishing
// and HOST_IMAGEUP_COMPLETE is a capture. Core never fired the capture
// name at all until fogproject#1202, so both outcomes arrived as a
// deploy and this message could not tell them apart.
//
// Composed here rather than left as a bare literal for the parent to
// translate, because the image name has to be substituted OUTSIDE _().
// The parent's _() then finds no entry and passes the finished string
// through, which is what we want. Read defensively: this plugin has to
// keep working against a server that has not taken #1202 yet.
$image = (string) ($data['ImageName'] ?? '');
if ('' === $image) {
$image = _('an unnamed image');
}
if ('HOST_IMAGEUP_COMPLETE' === $event) {
self::$shortdesc = _('Capture Complete');
self::$message = sprintf(
_('This host has finished capturing image %s.'),
$image
);
} else {
self::$shortdesc = _('Deploy Complete');
self::$message = sprintf(
_('This host has finished deploying image %s.'),
$image
);
}
parent::onEvent($event, $data);
}
}
21 changes: 19 additions & 2 deletions pushbullet/events/imagefail_pushbullet.event.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,25 @@ public function __construct()
*/
public function onEvent($event, $data)
{
self::$message = 'This host has failed to image';
self::$shortdesc = 'Failed';
// HOST_IMAGE_FAIL had no core caller at all until fogproject#1202, so
// this listener has never run on any server. The reason is the part an
// admin can act on, so say it rather than "failed to image". Read
// defensively: this plugin has to keep working against a server that
// has not taken #1202 yet.
$image = (string) ($data['ImageName'] ?? '');
if ('' === $image) {
$image = _('an unnamed image');
}
$reason = (string) ($data['Reason'] ?? '');
if ('' === $reason) {
$reason = _('no reason was reported');
}
self::$shortdesc = _('Imaging Failed');
self::$message = sprintf(
_('This host failed imaging %1$s: %2$s'),
$image,
$reason
);
parent::onEvent($event, $data);
}
}
23 changes: 20 additions & 3 deletions slack/events/imagecomplete_slack.event.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,22 +47,39 @@ public function __construct()
/**
* Perform action
*
* One listener, two names: HOST_IMAGE_COMPLETE is a deploy finishing and
* HOST_IMAGEUP_COMPLETE is a capture. Core never fired the capture name
* at all until fogproject#1202, so both outcomes arrived as a deploy and
* the message could not tell them apart. The payload now also carries the
* image, so the notification can say which one.
*
* Every added key is read defensively: this plugin has to keep working
* against a server that has not taken that change yet.
*
* @param string $event the event to enact
* @param mixed $data the data
*
* @return void
*/
public function onEvent($event, $data)
{
$image = (string) ($data['ImageName'] ?? '');
if ('' === $image) {
$image = _('an unnamed image');
}
$format = (
'HOST_IMAGEUP_COMPLETE' === $event ?
_('Host %1$s finished capturing image %2$s.') :
_('Host %1$s finished deploying image %2$s.')
);
$Slacks = Route::getList('slack');
foreach ($Slacks as $Slack) {
$args = [
'channel' => $Slack->name,
'text' => sprintf(
'%s: %s %s.',
_('Host'),
$format,
$data['HostName'],
_('completed imaging')
$image
)
];
self::getClass('Slack', $Slack->id)->call('chat.postMessage', $args);
Expand Down
21 changes: 18 additions & 3 deletions slack/events/imagefail_slack.event.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,22 +44,37 @@ public function __construct()
/**
* Perform action
*
* HOST_IMAGE_FAIL had no core caller at all until fogproject#1202, so this
* listener has never run on any server. The payload carries the image and
* the reason FOG rejected the task, which is the part an admin can act on.
*
* Every added key is read defensively: this plugin has to keep working
* against a server that has not taken that change yet.
*
* @param string $event the event to enact
* @param mixed $data the data
*
* @return void
*/
public function onEvent($event, $data)
{
$image = (string) ($data['ImageName'] ?? '');
if ('' === $image) {
$image = _('an unnamed image');
}
$reason = (string) ($data['Reason'] ?? '');
if ('' === $reason) {
$reason = _('no reason was reported');
}
$Slacks = Route::getList('slack');
foreach ($Slacks as $Slack) {
$args = [
'channel' => $Slack->name,
'text' => sprintf(
'%s: %s %s.',
_('Host'),
_('Host %1$s failed imaging %2$s: %3$s'),
$data['HostName'],
_('imaging failed')
$image,
$reason
)
];
self::getClass('Slack', $Slack->id)->call('chat.postMessage', $args);
Expand Down
148 changes: 148 additions & 0 deletions tests/imaging-notification-detail.test.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
<?php
/**
* The imaging notifications must survive a server that has not been updated.
*
* fogproject#1202 gave the imaging events a real payload -- ImageName, Reason,
* and a distinct HOST_IMAGEUP_COMPLETE for a capture -- and these six
* listeners now use it. A plugin release is not tied to a FOG release, so
* every one of them has to keep working against a server whose core still
* sends nothing but HostName. Read a missing key directly and the notification
* becomes a PHP warning in the middle of an event that only fires when
* something has already gone wrong.
*
* Also pinned here because this repository has no gettext gate of its own
* (fogproject's tests/gettext-literal-msgid.test.php only scans fogproject):
* a msgid must not mix literal text with a variable. `_("Image $name")`
* extracts nothing and never translates, silently and permanently.
*
* Source-level. These classes extend the plugin's own Event base, which
* extends FOG's, so none of them can be loaded without a booted FOG, a
* session and a database -- the same reason group-tab-permissions.test.php
* inspects rather than instantiates.
*
* Usage: php tests/imaging-notification-detail.test.php
* Exit status 0 = pass, 1 = fail.
*/

$root = dirname(__DIR__);

$complete = [];
$fail = [];
foreach (['slack', 'ntfy', 'pushbullet'] as $plugin) {
$complete[$plugin] = sprintf(
'%s/%s/events/imagecomplete_%s.event.php',
$root,
$plugin,
$plugin
);
$fail[$plugin] = sprintf(
'%s/%s/events/imagefail_%s.event.php',
$root,
$plugin,
$plugin
);
}

$failures = [];
$checks = 0;

/**
* Records one assertion.
*
* @param bool $ok Whether it held.
* @param string $msg What is wrong if it did not.
*
* @return void
*/
$check = function ($ok, $msg) use (&$failures, &$checks) {
++$checks;
if (!$ok) {
$failures[] = $msg;
}
};

foreach (array_merge(array_values($complete), array_values($fail)) as $file) {
$src = file_get_contents($file);
$where = basename($file);

// The compatibility contract with an un-upgraded server.
$check(
false !== strpos($src, "\$data['ImageName'] ?? ''"),
"$where reads ImageName without a default, so it warns against a"
. ' server that has not taken fogproject#1202'
);

// HostName is the one key that has always been there. Losing it would
// break the notification on every server, not just an old one.
$check(
false !== strpos($src, "\$data['HostName']")
|| false !== strpos($src, 'parent::onEvent'),
"$where no longer uses HostName, the only key core has always sent"
);

// The gettext rule: literal inside _(), variable substituted outside.
//
// Only a DOUBLE-quoted literal can interpolate, and only that is a defect
// -- a `$` inside a single-quoted msgid is a positional format specifier
// like %1$s, which is exactly the shape a translator needs to reorder the
// sentence.
$check(
!preg_match('#_\(\s*"[^"]*\$#', $src),
"$where interpolates a variable inside a double-quoted _(), which"
. ' extracts no msgid and so never translates'
);
$check(
!preg_match('#_\(\s*[\'"][^\'"]*[\'"]\s*\.#', $src),
"$where concatenates onto a literal inside _(), same defect as"
. ' interpolating into one'
);
$check(
!preg_match('#_\(\s*sprintf#', $src),
"$where wraps _() around sprintf() rather than the other way round,"
. ' so xgettext extracts the format string only by accident'
);
}

foreach ($complete as $plugin => $file) {
$src = file_get_contents($file);
$where = basename($file);

// The whole point of #1202's core half: one listener, two names.
$check(
false !== strpos($src, "'HOST_IMAGEUP_COMPLETE' === \$event"),
"$where does not distinguish a capture from a deploy, so the name"
. ' core went to the trouble of firing is wasted'
);
$check(
false !== strpos($src, "'HOST_IMAGEUP_COMPLETE'")
&& false !== strpos($src, "'HOST_IMAGE_COMPLETE'"),
"$where no longer registers both completion names"
);
}

foreach ($fail as $plugin => $file) {
$src = file_get_contents($file);
$where = basename($file);

// The reason is the only part of a failure an admin can act on.
$check(
false !== strpos($src, "\$data['Reason'] ?? ''"),
"$where does not report why imaging failed, or reads Reason without a"
. ' default'
);
$check(
false !== strpos($src, "'HOST_IMAGE_FAIL'"),
"$where no longer registers HOST_IMAGE_FAIL"
);
}

if (count($failures) > 0) {
fwrite(STDERR, 'FAIL: ' . count($failures) . " problem(s):\n");
foreach ($failures as $f) {
fwrite(STDERR, " - $f\n");
}
exit(1);
}

printf("ok: %d checks across 6 imaging notification listeners\n", $checks);
exit(0);
Loading