Mybinder.org-deploy: deploying open PRs to staging only

[minrk]

Now that we are on GitHub actions and we have action-experts like @sgibson91 and @hamelsmu, I wonder if we can feasibly have test deployments, where we deploy an open PR to staging only. I'm not sure what the right mechanism for that would be, but it seems like it might be possible with the controls we have.

The idea would be that a special comment or tag (or some other trigger) could be used on a PR to deploy it to staging without also deploying to production. We would need to still limit the deployment permissions to those who can merge PRs (i.e. not anyone can open a PR and comment on their own PR to deploy to staging).

I'm sure we could do it by merging to a staging branch, but I think it would be cool if we could accomplish it without needing to create that extra branch.

Does this sound doable?

[sgibson91]

I think this would only take a few edits to the cd.yml workflow. If we add the pull_request trigger then we can use conditionals to run the 2 jobs for different triggers. Checkout this action I wrote for the Turing Way, which only tests changed files in a Pull Request and then tests _all_ files on merge to master. (The if statements are the important bit and we'd want to define them at the job level, not the step level.)

One niggly bit is, do we want to re-deploy to staging on merge or if the PR passes is that fine?

[sgibson91]

Another thing to consider is when the PR is opened and what effect this has on the state of staging. I'm one of those people who opens PRs early and keeps pushing to them so I can continually check CI is passing. The GHA config I'm suggesting will re-run tests on every commit to an open PR. So if one commit breaks staging, how can we gracefully recover? We may want to restrict the pull_request trigger to certain events, such as opened only? Or it might just be people like me needing to learn a different workflow 😄 (Most PRs we get, e.g. henchbot, are single commit)

[sgibson91]

We would need to still limit the deployment permissions to those who can merge PRs (i.e. not anyone can open a PR and comment on their own PR to deploy to staging).

This section has just sunk into my post-holiday brain. This will be trickier. I'll mull it over but it _could_ involve checking if github.actor is in a list of allowed mergers.

[hamelsmu]

Another flexible way, rather than maintaining a whitelisted names, is to inspect the payload via the github.event variable. This contains the commiters permission level on the repo. I am doing something similar in fastpages:

   - name: Comment on issue if sufficient access does not exist
      if: |
        (github.event.issue.author_association != 'OWNER') && 
        (github.event.issue.author_association != 'COLLABORATOR') &&
        (github.event.issue.author_association != 'MEMBER')
      uses: actions/[email protected]
      with:
        github-token: ${{secrets.GITHUB_TOKEN}}
        script: |
          var permission_level = process.env.permission_level;
          var url = 'https://help.github.com/en/github/setting-up-and-managing-your-github-user-account/permission-levels-for-a-user-account-repository#collaborator-access-on-a-repository-owned-by-a-user-account'
          var msg = `You must have the [permission level](${url}) of either an **OWNER**, **COLLABORATOR** or **MEMBER** to instantiate an upgrade request.  Your permission level is ${permission_level}`
          github.issues.createComment({
            issue_number: context.issue.number,
            owner: context.repo.owner,
            repo: context.repo.repo,
            body: msg
          })
          github.issues.update({
            issue_number: context.issue.number,
            owner: context.repo.owner,
            repo: context.repo.repo,
            state: 'closed'
          })
          throw msg;
      env:
        permission_level: ${{ github.event.issue.author_association }}

[sgibson91]

@hamelsmu Amazing! I'm still learning about the payload

[hamelsmu]

I'm still learning about it too LOL Actions is a crazy large surface area!

[sgibson91]

Hmmm, seems we have few owners/collaborators/members and most access is granted through the mybinder.org operators team. Is that info available through the payload?

[hamelsmu]

Hmm not exactly sure this answers the question but you could also get
arbitrary org and user data via the GitHub api inside the Actions runtime.
Do you think that might help?

On Tue, Sep 1, 2020 at 8:47 AM Sarah Gibson [e-mail]
wrote:

>
>

Hmmm, seems we have few owners/collaborators/members and most access is
granted through the mybinder.org operators team. Is that info available
through the payload?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/jupyterhub/mybinder.org-deploy/issues/1581#issuecomment-684951660,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AALKJESUNZ7HZXH2JGRCFCDSDUJRDANCNFSM4QRUGUZQ
.

[sgibson91]

I found this action if we want to stick to the "triggered by a comment" idea: https://github.com/Khan/pull-request-comment-trigger

[minrk]

This is great info, thanks folks!

I'm definitely not tied to the comment idea, that's just how some bots I've used work. the main thing is that I think we probably don't want the trigger to be a commit, because I think we probably don't want to do this most of the time, only on semi-rare PRs. Or I guess deploy on commit if a given tag is applied could work. That wouldn't need any fancy permission management, I suppose, only add tag and then deploy to staging on commit while the PR has the given tag. Is there an event for when tags are applied so that an extra commit post-tagging wouldn't be needed?

I was thinking of particular changes like #1582 where I'd like to see the upgrade process roll out on staging before possibly disrupting prod.

[sgibson91]

We could do something like this and have a specific label like test-staging? https://stackoverflow.com/questions/62325286/run-github-actions-when-pull-requests-have-a-specific-label

Edited to add: This may even solve the permissions thing as only those with write/triage/admin access will be able to add the label.

[minrk]

Yeah, that sounds like it might be the easiest approach.

[sgibson91]

I opened PR #1583

[hamelsmu]

Just Incase it’s helpful, I’m an author of this Action
https://github.com/machine-learning-apps/actions-chatops

On Wed, Sep 2, 2020 at 1:12 AM Sarah Gibson [e-mail]
wrote:

>
>

I opened PR #1583
https://github.com/jupyterhub/mybinder.org-deploy/pull/1583

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/jupyterhub/mybinder.org-deploy/issues/1581#issuecomment-685432640,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AALKJETTPY7NGOKO3CP5VQLSDX46PANCNFSM4QRUGUZQ
.