Enable manually running workflow actions

deviant77 commented 2 years ago

For testing builds on branches before you commit them to main. If you merge this I can then test and send you a pull request for a working Frigate 0.11.0 beta 2 build.

weltenwort commented 2 years ago

Thanks for this proposal. What keeps you from submitting a PR without this trigger? After approval from a collaborator the workflows should already be able to run.

deviant77 commented 2 years ago

Thanks for this proposal. What keeps you from submitting a PR without this trigger? After approval from a collaborator the workflows should already be able to run.

There are breaking changes in 0.11.0 so I wanted to work in a branch to test (and troubleshoot) changes to the build workflow and then use the docker image to make sure it actually worked before sending you a clean PR. It takes out the guesswork because my skills are much lower than yours. :smile: In this case I had to commit this to the main branch of my fork ("workflow_dispatch:" needs to be in the main branch) and then when I got it working, reset my main to your upstream main, which was less than ideal. :zany_face:

weltenwort commented 2 years ago

Ah, I get it - I appreciate the intent to submit clean PRs :+1: I wonder about the security implications of enabling workflow_dispatch. I tried to read up on it, but couldn't find anything. I assume it's still subject to the same contributor limitations, so it should be ok.

weltenwort commented 2 years ago

I suspect there might be a problem with how I wrote the actions, though. The steps guarded like this would be executed in a workflow_dispatch run, even though they shouldn't:

      - name: Sign the published Docker image
        if: ${{ github.event_name != 'pull_request' }}
        env:
          COSIGN_EXPERIMENTAL: "true"
        # This step uses the identity token to provision an ephemeral certificate
        # against the sigstore community Fulcio instance.
        run: cosign sign ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}@${{ steps.build-and-push.outputs.digest }}

I guess we'll have to turn the guard into an allow-list instead of a deny-list to support it. :thinking:

weltenwort commented 2 years ago

I think it should be as simple as changing the github.event_name comparisons from != 'pull_request' to == 'push'. Would you be willing to include that in this PR?

deviant77 commented 2 years ago

Ah, I get it - I appreciate the intent to submit clean PRs 👍 I wonder about the security implications of enabling workflow_dispatch. I tried to read up on it, but couldn't find anything. I assume it's still subject to the same contributor limitations, so it should be ok.

Here's some GitHub documents:

The blog announcement: https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/
The workflow_dispatch event: https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#workflow_dispatch
Using the "Run workflow" button: https://docs.github.com/en/actions/managing-workflow-runs/manually-running-a-workflow - note the line "_To trigger the workflow_dispatch event, your workflow must be in the default branch._".

The "Run workflow" button only appears in you own Actions. It won't be there for me in your repository, and it won't for you in my fork. I'm not sure there any settings to even change this limitation!

You can see a simplistic practical example here (actually where I got the idea from): https://github.com/deviant77/ipq807x-openwrt-builder The workflow has workflow_dispatch: in it. You have no "Run workflow" button in either my fork, or the upstream (neither do I), but if you fork it you will find the "Run workflow" button under Actions.

I suspect there might be a problem with how I wrote the actions, though. The steps guarded like this would be executed in a workflow_dispatch run, even though they shouldn't:
      - name: Sign the published Docker image
        if: ${{ github.event_name != 'pull_request' }}
        env:
          COSIGN_EXPERIMENTAL: "true"
        # This step uses the identity token to provision an ephemeral certificate
        # against the sigstore community Fulcio instance.
        run: cosign sign ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}@${{ steps.build-and-push.outputs.digest }}
I guess we'll have to turn the guard into an allow-list instead of a deny-list to support it. 🤔

I think your actions are correct. With your current workflow actions and the workflow_dispatch: PR it makes the following possible:

In my fork I create a branch to work on.
I make some commits in my branch.
When ready, I run the workflow manually on my branch.
It either fails and I troubleshoot and make more commits, or it's successful and I have a docker image in my repository which I can test, then if testing is okay...
I create a nice clean tested PR from my branch to your repository.
Your workflow checks my PR but doesn't push or sign (as it does now).
If it's successful (which it should be) you merge the PR to main and the workflow builds and publishes (as it does now)!

I think it should be as simple as changing the github.event_name comparisons from != 'pull_request' to == 'push'. Would you be willing to include that in this PR?

I think != 'pull_request' is okay because you've specified the workflow only triggers on push and pull requests on the "main" branch where you're merging the PRs to.

So in summary I think everything is fine as it is, we just need to enable the "Run workflow" button by merging this PR to main! :smile:

deviant77 commented 2 years ago

I've only just got builds working on my fork! They were working when I added workflow_dispatch to main in my fork, but after you merged this PR and I re-created my fork, the builds were mysteriously failing with a permissions error. I couldn't work it out, and decided to come back to it "later". :zany_face: I just deleted the package (even though it was showing as linked to my fork and had correct permissions), and ran a build manually and it re-created the package and all successful now.

I've just created a branch named "this-is-a-test-branch" and ran a manual build. You can see the resulting package here. The docker image tag is still the name of the branch (in this case frigate-synology-dsm7:this-is-a-test-branch), which I think is good.

weltenwort commented 2 years ago

good to hear, thanks for sharing!

weltenwort / frigate-synology-dsm7

Enable manually running workflow actions #39