More helpful message for corner case regarding updates to workflow files

[joao-paulo-parity]

It's not documented in the API, but according to https://g.nite07.orgmunity/t/github-action-resource-not-accessible-by-integration/16034/3:

OAuth tokens and GitHub Apps are restricted from editing the main.workflow file. This means it isn’t possible to merge a Pull Request via an Action if it updates the main.workflow file I’m afraid.

We've seen the bot failing with an unhelpful message in that scenario (could be the case in paritytech/cumulus#507 (comment)).

As a workaround: if the PR changes some workflow file, instead say something like:

The bot cannot merge when there are changes to a workflow file due to a Github limitation (link).

Yet another option: create a real Github User for the bot, generate a push token for it and use that token for pushing (instead of the bot's own token like it is right now); that way the merger is no longer considered a Github App and therefore it will not be affected by the aforementioned rule. At the same time, we should perhaps poke Github about supporting our use-case without resorting to this approach.

[joao-paulo-parity]

Follow-up response from Github Support:

Yeah, that's a known limitation of the GitHub Apps. We have an open internal issue tracking this as feedback and I'm happy to add this report as a +1. This is currently not on our roadmap. I'm afraid, I can make any guarantee if or when this limitation will be removed.

It's not even in the roadmap, so who knows when this will be implemented. It seems like we do need to create the bot's user.

[TriplEight]

I think the security causes this corner case so that the actions do not execute absolutely everything they want.

As they don't want to fix this, it makes sense for us to place a workaround. Do we stumble upon this issue often?

[joao-paulo-parity]

I think the security causes this corner case so that the actions do not execute absolutely everything they want.

It's not about executing but rather preventing the merge of "protected files" (e.g. a GitHub workflow) by bots.

Do we stumble upon this issue often?

It always happens when "protected files" (e.g. a GitHub workflow) are changed within a pull request. I don't know how often those files are changed though.

If this protection is desired, processbot should stop the merge and provide a good error message instead of 403 Forbidden. That would not "fix" the scenario but instead provide a better understanding of what the issue is.

If this protection is not desired we should create a GitHub Access Token from a service account e.g. https://github.com/paritytech-ci and use that for merging; since it counts as a user and not a bot, I reckon that would work.

[joao-paulo-parity]

In the meantime GitHub might've added support for this use-case, as per paritytech/cumulus#1436 (comment)

refusing to allow a GitHub App to create or update workflow .github/workflows/release-02_create-draft.yml without workflows permission

So if we add the workflows permission we might overcome this problem. I'll come up with a PR for adding that to the documentation.

[joao-paulo-parity]

So if we add the workflows permission we might overcome this problem.

I've sent GitHub support a ticket for confirming this. I'll update here once there's a response.

[joao-paulo-parity]

Re #302 (comment)

Update: no, it doesn't seem like this ticket can be closed. From Q&A with GitHub support

Q: Does that mean that our app will be able to merge PRs which have changes to workflow files if the Repository permissions > Worflows (Read & write) permission is set up for it, or is the feature still not implemented?

A: Yes, GitHub Apps with Worflow:write permission are able to merge PRs which have changes to workflow files. Just to add more context --The App would only be able to merge PRs when the workflow file content matches files on another branch in the same repository.

The limitation described in the "more context" part means that not all PRs would be covered by this feature, so it's still not working as intended. I think we'll need a PAT (#302 (comment)) at least for merging.