Allow 'setup-workflows' to create a PR instead of pushing to main branch

[f1ames]

As mentioned in #6 (comment):

We may try to fiddle with bot/token permissions to see if it's possible to workaround.

At the moment, bot has Write access in ckeditor4 repostiory. To be able to push despite branch protection rules, it would need to have Admin access which I think is rather a bad idea... It should have as little permissions as possible. So the other solution is to go with:

If not, other solution may be to adjust the workflow (via config flag, see #4) so it creates a PR instead of pushing directly to main branch.

Let's try the later approach and see how it works.

So I went with configuration option which allows to alter the workflow to create PR instead of pushing directly to main branch. Since it doesn't make sense to keep configuration in GH secrets I have introduced optional JSON config file (#4).

The above needs proper explanation so I took this occasion to improve README file and describe all common workflows we have now.

The default flow and the one wit PR were tested in separate test repos:

• https://github.com/ckeditor/workflows-test-1
• https://github.com/ckeditor/workflows-test-2

Closes #6. Closes #4.

[f1ames]

After this PR is merged we need manually update setup-workflows.yml in ckeditor4 repository.

[jacekbogdanski]

Looks good, some minor changes required.

[jacekbogdanski]

Maybe we could load JSON file into the env variable as a separate, initial step, so the config is available everywhere? I was thinking about some separate step like:

- name: Load settings
  run: |
	echo "SETTINGS={}" >> $GITHUB_ENV
	if [[ -f "./.github/workflows-config.json" ]]; then
		echo "SETTINGS=${{ jq './.github/workflows-config.json }}" >> $GITHUB_ENV
	fi

and then use it everywhere as e.g. jq -n .setupWorkflows.pushAsPullRequest (should return "null" if nothing found). WDYT?

Also, maybe there is some better way to keep settings somehow easily accessible? Overall it should work for our case now, but any other setting will require duplicated logic for loading config which may be a bit of pain.

If we go with some good approach here, we will be able to just copy it in any action using config file.

[f1ames]

Makes perfect sense. The best approach would be probably extracting config reading logic to separate action (so it can be called with two lines) or adding as external script to be reused.

However, as this is quite simple code I'm for adding it as initial step for now to each workflow which needs it (using copy/paste) and then we can see how problematic it is. I will also add short README section on this.

[jacekbogdanski]

Maybe we could add information when cron runs the action? (02:00 UTC).

We could also add this information to other actions.

[jacekbogdanski]

I'm wondering if it shouldn't be a part of new settings json file? We could default it into github.ref if not set (depending on the action). The good part is that we will be able to configure the branch per action if needed, and setting env variables is less friendly than config.

[f1ames]

I have updated https://github.com/ckeditor/workflows-test-1 and https://github.com/ckeditor/workflows-test-2 withe latest changes.

[f1ames]

I see this line

pushAsPullRequest=$(echo ${{ env.SETTINGS }} | jq ".pushAsPullRequest")

 doesn't work as it should (see here) resulting in

pushAsPullRequest=$(echo {} | jq ".pushAsPullRequest")

so this still needs to be fixed.

[f1ames]

I had some hard time providing proper loading for config files but it seems to be working now. Also, it seems adding manual dispatch for some jobs could be useful (and also helpful for testing). And so:

update-deps

1. Tested with no config - https://github.com/ckeditor/workflows-test-1/runs/1839790535?check_suite_focus=true#step:3:11
2. Tested with empty config {} - https://github.com/ckeditor/workflows-test-1/runs/1839793100?check_suite_focus=true#step:3:11
3. Tested with sample config {"branchMain": "major"} - https://github.com/ckeditor/workflows-test-1/runs/1839558113?check_suite_focus=true#step:3:11

All steps passes until NPM update since there is no package.json in this repo. But you can see based on branch names and steps that config was read and processed correctly.

setup-worfklows

1. Tested with sample config {"pushAsPullRequest": true} - https://github.com/ckeditor/workflows-test-1/runs/1839877207?check_suite_focus=true and PR was created (Update 'setup-workflows' workflow workflows-test-1#1)
2. Tested with sample config but read from config file - https://github.com/ckeditor/workflows-test-2/runs/1840137819?check_suite_focus=true#step:4:11 and PR created (Update 'setup-workflows' workflow workflows-test-2#2)
3. Tested with direct branch push with review protection branch rules (same as ckeditor4 repo) - https://github.com/ckeditor/workflows-test-2/runs/1840157154?check_suite_focus=true#step:11:12

---

So it seems everything works fine but testing it is kind of cumbersome.

[f1ames]

As for testing workflows locally there is already mentioned act and some other approaches like creating and testing CI-agnostic parts (e.g. https://stackoverflow.com/a/59988803/646871)

[f1ames]

Ready for another review.

[f1ames]

However, I found that setup-workflows.yml fails when try to make PR while another PR exists. It uses the same branch name as the source branch in PR.   

Maybe we could force push for such a branch? It will be eventually updated to the newest changes, and we will stay with a single PR?

I added checking if the PR branch already exists - in such cases, workflow will be simply canceled.

I'm thinking if there is a way to make an initial workflow setup, that will correspond to this repo? I mean besides manually creating an initial workflow?

We were thinking about it on a very beginning but since it is "setup and forget' and requires minimal amount of work (copy-paste setup-workflows.yml), there is no sense in automating this.

[f1ames]

@sculpt0r ready for another review.

[f1ames]

@sculpt0r ready for another review.

[sculpt0r]

I added checking if the PR branch already exists - in such cases, workflow will be simply canceled.

Also sounds like a good idea :)

Now setup-workflows.yml stuck at Process config in my test repo

However, I can't figure out where is the misspell or syntax error... 🤔 Could you take a look there?

[f1ames]

Now setup-workflows.yml stuck at Process config in my test repo

However, I can't figure out where is the misspell or syntax error... Could you take a look there?

I used elif without any condition 🤦 Replaced with else, see a2e1abc.

@sculpt0r taken over this review.

[f1ames]

@sculpt0r ready for another review.

[sculpt0r]

#12 (comment) Seems to work right now :)
Also, the entire action looks canceled which is good: https://github.com/ckeditor/workflow-tests-PR-6/actions
However, it looks like steps after Cancel build if update branch already existwas executed anyway and crash on Push changes again: https://github.com/ckeditor/workflow-tests-PR-6/runs/1960342487?check_suite_focus=true
Another run looks good: https://github.com/ckeditor/workflow-tests-PR-6/runs/1960334325?check_suite_focus=true

You use cancel build action. The description says The cancellation might take a few seconds to process. So it might be that few other actions will be invoked before it actually works? 🤔 So it looks like there might be a situation that some action will be invoked even if you cancel the entire run.

Could you take another look into the 'canceling' method and try to improve it?

[f1ames]

You use cancel build action. The description says The cancellation might take a few seconds to process. So it might be that few other actions will be invoked before it actually works? So it looks like there might be a situation that some action will be invoked even if you cancel the entire run.

Could you take another look into the 'canceling' method and try to improve it?

Ok, it seems like that's the case. This cancel action is kind of a workaround, since it's not that straightforward to cancel action from within itself:

• Step can be canceled by exiting the process but next steps will be processed normally (no matter the exit code).
• All further steps can be conditional, like if: env.SHOULD_CANCEL == 1 but then it bloats the code.

As for the solution to the cancel action delay, we can add wait action after - if the job needs to be cancel, the cancel step is executed and then wait step. Otherwise, those to steps will be simply skipped.

[f1ames]

As for the solution to the cancel action delay, we can add wait action after - if the job needs to be cancel, the cancel step is executed and then wait step. Otherwise, those to steps will be simply skipped.

As mentioned above, I've added wait step - c93ee5b.

[f1ames]

@sculpt0r ready for another review 🤞

[sculpt0r]

@sculpt0r ready for another review

LGTM 🎉 

A few runs and it ends with proper cancelation. Of course, it elongates waiting time before any results - but since it's automatic - it's fine.