Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

push workflow not triggered in v3. #134

Open
2 tasks done
JamesKyburz opened this issue Dec 20, 2021 · 19 comments
Open
2 tasks done

push workflow not triggered in v3. #134

JamesKyburz opened this issue Dec 20, 2021 · 19 comments
Labels
wontfix This will not be worked on

Comments

@JamesKyburz
Copy link
Contributor

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the regression has not already been reported

Last working version

2.x

Stopped working in version

3.x

Node.js version

12.x

Operating system

Linux

Operating system version (i.e. 20.04, 11.3, 10)

20.04

💥 Regression Report

Now that v3 directly merges the pull request in the workflow without using an external app no push workflow is triggered.

In v2 once a dependabot pull request got merged any workflows listening on the push event were triggered.

Steps to Reproduce

automerge.yml

name: automerge

on:
  pull_request:
    branches:
      - main

jobs:
  automerge:
    runs-on: ubuntu-latest
    permissions:
      pull-requests: write
      contents: write
    if: ${{ github.actor == 'dependabot[bot]' }}
    steps:
      - uses: fastify/github-action-merge-dependabot@v3.0.2
        with:
          github-token: ${{ secrets.GITHUB_TOKEN }}

ci.yml

name: ci

on:
  push:
    branches:
      - main

jobs:
  ci:
    runs-on: ubuntu-latest
    steps:
      - run: |
          echo hello

Expected Behavior

ci.yml to be called.

I am unsure how/if this can be fixed as dependabot workflows have no access to other secrets other than the default GITHUB_TOKEN.

@simoneb
Copy link
Collaborator

simoneb commented Dec 20, 2021

Thanks for reporting this. I'm afraid that we simply did not consider this when moving away from the backing web app, which we thought of as a temporary workaround for limitations in github permissions.

I think that the behavior you're seeing is another known limitation in github, which prevents workflows to trigger other workflows to avoid infinite recursion.

This is a somewhat important limit that we should probably point out in the docs, but I don't see us going back to the backing web app at this stage.

@simoneb
Copy link
Collaborator

simoneb commented Dec 20, 2021

including @Eomm in this conversation

@Eomm
Copy link
Member

Eomm commented Dec 20, 2021

As workaround, could you try to use a custom developer token instead of the default GITHUB_TOKEN?

@simoneb
Copy link
Collaborator

simoneb commented Dec 20, 2021

together with @Eomm's suggestion above, and regarding your comment:

I am unsure how/if this can be fixed as dependabot workflows have no access to other secrets other than the default GITHUB_TOKEN.

GitHub now supports dependabot secrets. So storing a PAT in secrets AND dependabot secrets would probably make this work.

It's not a full solution, but it may be the best we can do for now, apart from staying on v2.

@mcollina
Copy link
Member

mcollina commented Dec 20, 2021

Now that v3 directly merges the pull request in the workflow without using an external app no push workflow is triggered.

This seems a limitation/bug in Github Actions. New commits are pushed to main so I would have expected the workflow to run. Maybe we should open an issue there?

@JamesKyburz
Copy link
Contributor Author

I will check if a PAT works using a Dependabot secret, if so maybe we could just update the README to reflect the v3 changes.

@JamesKyburz
Copy link
Contributor Author

It does work with a dependabot specific repository secret PAT :) Also if this is used then you don't need to specify extra permissions because the rights given by the PAT.

@simoneb
Copy link
Collaborator

simoneb commented Dec 20, 2021

Thanks for trying this out @JamesKyburz . The main issues with PATs are the lack of permission granularity and that they are bound to a user, which make them inappropriate for repositories with multiple contributors. I would personally not be comfortable using them in a repository with multiple contributors. On the other hand, I don't think there are any other solutions which would cause the merged PRs to trigger other workflows.

@JamesKyburz
Copy link
Contributor Author

@simoneb I agree, although I can't think of any other solution either. I am also yet to find a way to automatically rotate PAT secrets too ☹️

@Eomm
Copy link
Member

Eomm commented Jan 2, 2022

After reading the GH docs it seems not doable by setting some config.

Maybe we could add an extra step to trigger a workflow by calling the /repos/{owner}/{repo}/actions/workflows/{workflow_id}/dispatches API to trigger a workflow manually

@simoneb
Copy link
Collaborator

simoneb commented Jan 3, 2022

We would need to configure which workflow(s) to trigger. I am not a fan of the idea but not against it either. For most practical purposes this unintended behavior we have is surely not nice but not an extremely big deal anyway in my opinion (especially because we don't really have many options outside of going back to the backing github app).

The reason why it's not a very big deal in my opinion is because usually we would configure whatever workflow we run when something is merged to the master/main branch to run both on push and pull_request events, meaning that when you're in a dependabot PR, you're already running those same workflows for both the code that's in the PR and the code that's the result of merging the PR's branch into master/main, meaning that another workflow(s) run when the PR is actually merge is largely redundant.

Now, I appreciate this is not certainly covering all scenarios (e.g. you're triggering a CD workflow when merging to master/main), but lacking an obvious solution I guess we have to accept that we have this limitation, unless we decide that we don't want to have it, in which case the most obvious option is to go back to the github app.

@tinohager
Copy link

Hi, I just stumbled across this problem too and from my point of view I would have no problem specifying the workflow I want to call. Is there an implementation date for the function?

@tinohager
Copy link

I have found a solution for my workflow issue, my pull request workflow has this name Check Pull Request

Add this to the workflow to be executed after the pull request workflow

on:
  workflow_run:
    workflows: ["Check Pull Request"]
    types:
      - completed

@Eomm
Copy link
Member

Eomm commented Jan 24, 2022

Hi, I just stumbled across this problem too and from my point of view I would have no problem specifying the workflow I want to call. Is there an implementation date for the function?

There is nobody working on it. Would you like to send a PR?

I have found a solution for my workflow issue

do you mean something like the following?

name: ci

on:
  push:
    branches:
      - main
  workflow_run:
    workflows: ["automerge"]
    types:
      - completed

jobs:
  ci:
    runs-on: ubuntu-latest
    steps:
      - run: |
          echo hello

It seems a good solution to me: we could document it!

@simoneb
Copy link
Collaborator

simoneb commented Jan 25, 2022

My only doubt here is that we have normally used exactly the same workflow for CI containing both our CI steps and the step which invokes the automerge action. This solution implies that the two would have to live in different workflow files. Let's say you have a ci and an automerge workflows. What triggers what?

pull_request -> ci
ci -> automerge
automerge -> ci

This goes into an infinite loop, doesn't it?

@tinohager
Copy link

I have dependabot active, this create a new pull request on package update, a workflow for pull requeust validate the pull request and start automerge after successful build and trigger the primary workflow to create the new docker container.

@simoneb
Copy link
Collaborator

simoneb commented Jan 25, 2022

can you describe how you set it up in more detail?

@wbaldoumas
Copy link

Just adding here that I am also running into this issue and the discussed workarounds aren't something I'd like to put in place. Is the best course of action to drop down to v2.x.x?

@simoneb
Copy link
Collaborator

simoneb commented Mar 20, 2022

@wbaldoumas no plans to revert to the approach adopted in v2 at the current time. We understand the compromise but we chose to avoid using a backing API due to the maintenance overhead. We've been using v3 for several months now and while the lack of automatic triggers when PRs are merged is an inconvenience, we have not incurred into any major issues.

I'll leave this issue open because it's legit, but we don't plan to address it at the current time.

@simoneb simoneb added the wontfix This will not be worked on label Mar 20, 2022
edgarvonk added a commit to infonl/webdav-servlet that referenced this issue Feb 12, 2024
edgarvonk added a commit to infonl/dimpact-zaakafhandelcomponent that referenced this issue Feb 12, 2024
edgarvonk added a commit to infonl/dimpact-zaakafhandelcomponent that referenced this issue Feb 13, 2024
Did not add auto-merge however because that unfortunately does not
trigger our push workflow. See
fastify/github-action-merge-dependabot#134

Solves PZ-1420
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

6 participants