Bug 1678572 - A github-release shouldn't kick off releases if the bot…

[JohanLorenzo]

… made it

Full context at: https://bugzilla.mozilla.org/show_bug.cgi?id=1678572#c0

Kudos to @camd for the idea 👍 (I wanted to add you as a reviewer but the Github UI prevents me from doing so)

[JohanLorenzo]

You might think this code could be simplified into:

(tasks_for == "github-release" && releaseAction == "published" && (event.sender.login != "mozilla-release-automation-bot") && (event.sender.login != "mozilla-release-automation-bot-staging"))

To be honest, I wish it could be the case. Sadly, we hit that class of bugs many times. The full if statement is evaluated before checking the truthiness of each operand. This means, Taskcluster will try to evaluate event.sender.login and will raise an error because that object doesn't exist there. Thus, this .taskcluter.yml file will be broken for any case that doesn't define event.sender.login. This is why you see variables like releaseAction defined this way, just above:

fenix/.taskcluster.yml

Lines 98 to 101 in 4e598d1

	releaseAction:
	$if: 'tasks_for == "github-release"'
	then: ${event.action}
	else: 'UNDEFINED'

It's a way to ensure the if statement remains "evaluable" under any circumstances.

Back to ownerEmail. It's entirely defined there:

fenix/.taskcluster.yml

Lines 16 to 30 in 4e598d1

	ownerEmail:
	$if: 'tasks_for in ["cron", "action"]'
	then: '${tasks_for}@noreply.mozilla.org'
	else:
	$if: 'event.sender.login == "bors[bot]"'
	then: 'skaspari+mozlando@mozilla.com' # It must match what's in bors.toml
	else:
	$if: 'tasks_for == "github-push"'
	then: '${event.pusher.email}'
	else:
	$if: 'tasks_for == "github-pull-request"'
	then: '${event.pull_request.user.login}@users.noreply.github.com'
	else:
	$if: 'tasks_for == "github-release"'
	then: '${event.sender.login}@users.noreply.github.com'

I could define a new variable called ownerLogin, but this actually increase the complexity of .taskcluster.yml. In fact the $let statement[1] doesn't enable us to define a variable that depends on another, in the same statement. We have to embed the second statement in the in section of the first one. Meaning, if I wanted to factorize $event.sender.login, I would have to shift almost every line of .taskcluster.yml to get variables under the right scope. I don't think it's worth the effort. I believe this one-liner is much easier and the logic is straightforward to understand.

[1] https://json-e.js.org/#Language/-let-

[JohanLorenzo]

I confirm we needed this patch. Shipit just scheduled Fenix 84b2 and this task got created https://firefox-ci-tc.services.mozilla.com/tasks/groups/KvLDY6lmQj2YKZjdXxDVpA. I manually canceled it.