deployer icon indicating copy to clipboard operation
deployer copied to clipboard

{{target}} changed since v7, so notification recipes are not working as intended

Open dorantor opened this issue 1 year ago • 4 comments

Deployer version

vendor/bin/dep -V
Deployer 7.3.1

Deployment OS:

Should be irrelevant. From container with alpine linux To Debian 11 host

Problem example:

Notification from Deployer v6 to Slack/Mattermost:

Deployer BOT [14:46]
APP
Deploy to php022.example.com successful

or

Deployer BOT [14:46]
APP
Deploy to prod successful

That was good and meaningful. But with Deployer v7 it changed radically

Deployer BOT [14:46]
APP
Deploy to HEAD successful

Less helpful and meaningful.

Please, provide a minimal reproducible example of deploy.php

<?php

declare(strict_types=1);

namespace Deployer;

require 'contrib/slack.php';

set('slack_webhook', '<your webhook here>'); 

host('localhost')
    ->set('labels', [
        'env'   => 'stage',
        'fpm'   => 'yes',
    ])
;

desc('Testing command');
task('app:slacktest', function () {
    writeln('Testing slack notifications');
});

after('app:slacktest', 'slack:notify:success');
# expect to get at least env=stage instead of HEAD in notification
vendor/bin/dep app:slacktest env=stage
# expect to get localhost instead of HEAD in notification
vendor/bin/dep app:slacktest localhost

Some findings

With v6 we had

// recipe/common.php
set('target', function () {
    return input()->getArgument('stage') ?: get('hostname');
});

With v7 we got

// recipe/deploy/update_code.php
set('target', function () {
    $target = '';

    $branch = get('branch');
    if (!empty($branch)) {
        $target = $branch;
    }

    // Override target from CLI options.
    if (input()->hasOption('branch') && !empty(input()->getOption('branch'))) {
        $target = input()->getOption('branch');
    }
    if (input()->hasOption('tag') && !empty(input()->getOption('tag'))) {
        $target = input()->getOption('tag');
    }
    if (input()->hasOption('revision') && !empty(input()->getOption('revision'))) {
        $target = input()->getOption('revision');
    }

    if (empty($target)) {
        $target = "HEAD";
    }
    return $target;
});

As we can see {{target}} variable meaning changed completely thus making all notification recipes broken. Or at least they are not working as intended anymore. I tried to get target value from arguments with input()->getArgument('selector'), but this naive approach didn't work.

I think I could do a PR, but not sure how this could be fixed.

Upvote & Fund

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar

dorantor avatar Apr 19 '23 15:04 dorantor

Any updates or suggestions how it could/should be fixed?

dorantor avatar May 19 '23 15:05 dorantor

Broken because target is always HEAD?

antonmedv avatar May 19 '23 19:05 antonmedv

No. Because {{target}} changed meaning. Previously it had what we gave it as a "target" - host, env. Now it's more like "source" to me. In my opinion, HEAD values says "where from" code was taken, while host/env value - "where to" it was sent.

dorantor avatar May 25 '23 00:05 dorantor

Still a relevant issue. While {{target}} is used throughout, it's meaning is now weird in notification helpers (eg. Slack).

  • Do we have a placeholder in place for the value in host()[possibly an array!] / eg. {{current_host}} Even though that was shot down at #2625 - to add to the confusion we also use a different hostname(). We use generic server-addresses, not the application's entry URL, meaning that value wouldn't really make sense for our case either.

xewl avatar Jul 20 '23 17:07 xewl