1

I'm making some CICD and I'm using github labels to demonstrate intent to deploy into production.

So in my github actions, I have some boolean that checks if there to be a label 'deploy-prd'. There's a check for pull request review with status approved.

But... if a user commits some code after it has been approved, then the approval is still valid in the eyes of the cicd.

When the user adds the label 'deploy-prd' and the cicd runs, it just sees that there is an existing approval and that there is a 'deploy-prd' tag and deploys the newly committed and unapproved code to prod. Is there a way to compare the latest commit to the timestamp of the approval? Or is there another logic I should follow?

The current soln is making deploy-prd only accessible by admins.. which is meh at best.

Also: Would a synchronize trigger to remove existing approvals be a good idea in terms of performance, execution limits, and future technical debt? Or would this cause more headache down the line?

Below are some key excerpts from the cicd workflows.

Pull Request Trigger label flow

name: 'Label Trigger'

on:
  pull_request:
    types: [labeled]
jobs:
  gcp-pull-request-ci:
    if: github.event.review.state == 'approved'
    uses: ${{github.repository}}/github-actions/.github/workflows/gcp-label-ci.yaml@master
    with:
      repository: ${{ github.repository }}
      deploy-prod: ${{ contains( github.event.pull_request.labels.*.name, 'deploy-prd') }}
      pull-request-number: ${{ github.event.pull_request.number }}

Reusable workflow job logic

  check-for-deploy-prd:
    needs: deploy-stg
    if: inputs.deploy-prod
    outputs:
      data: ${{ steps.get_approved_prs.outputs.data }}
    runs-on: ubuntu-latest
    defaults:
      run:
        shell: bash

    steps:
    # Query approval status 
    - uses: octokit/request-action@v2.x
      id: get_approved_prs
      with:
        route: GET /repos/${{inputs.repository}}/pulls/${{inputs.pull-request-number}}/reviews
      env:
        GITHUB_TOKEN: ${{ secrets.token }}
  deploy-prod:
    needs: check-for-deploy-prd
    if: contains(fromJSON(needs.check-for-deploy-prd.outputs.data).*.state,'APPROVED')
torek
  • 448,244
  • 59
  • 642
  • 775
PaulyP
  • 165
  • 10
  • My suggestion is that your first workflow could also check and remove the label `deploy-prd` if the PR isn't approved anymore. In that case, you should also guarantee the second workflow (the reusable workflow for deploy) isn't started before the label check (maybe using a workflow_run trigger?) – GuiFalourd May 16 '22 at 19:51
  • Why not just make "deploy into production" run off a certain branch, such as main or whatever, and then let the PR system handle the rest? PR's should be invalidated and require re-approval if someone pushes more commits to them after they've been approved. Basically, you don't base your deployment pipeline on **intent** to deploy, but instead "merged into main". – Lasse V. Karlsen May 16 '22 at 19:51
  • As far as i've witnessed, the approvals remain after a commit happens and I still get the push to prd behavior when adding a label after a new commit. I wanted to avoid making branch specific flows but that might be fruitful if necessary. @LasseV.Karlsen – PaulyP May 16 '22 at 19:55
  • Wondering how to check if those commits aren't approved anymore. When I looked around, I found that the only way to query a timestamp is on the sync event. @GuiFalourd – PaulyP May 16 '22 at 19:55
  • In my experience with github, and understand that this is something that others set up, if I push another commit after a reviewer have approved the PR, the PR gets invalidated and require another approval before it can be merged. So I assume this is a feature of Github. I do not, however, know how this is set up, only that this is how it seems to work in my organization. – Lasse V. Karlsen May 16 '22 at 19:58
  • Cool. I appreciate the input and I'll do some more testing and think on it. I like the branch specific flow idea ... if no good answers come up I'll update with what we decide to go with. @LasseV.Karlsen – PaulyP May 16 '22 at 20:01
  • It sounds like you want exactly what @LasseV.Karlsen is talking about. In which case this may be a dup of [this question](https://stackoverflow.com/q/40505904/184546). – TTT May 16 '22 at 21:21
  • Unfortunately this violates one of our core principles which is that we do not want to merge anything to master branch unless it is deployed to prod. Kind of like a chicken and egg problem, but my leadership is urging me to go with the way we have it currently. I can see the benefits of the other way, though. I think I'm gonna make a trigger to run on pull request sync to clear any approved reviews – PaulyP May 16 '22 at 22:56
  • Note that PRs themselves (and all the approval stuff surrounding them) are GIt*Hub* features, not generic Git (as that doesn't have PRs in the first place). What GitHub offer here, I can't say with precision as I never got that far into administering a group GitHub site. But if an approval saves a *commit hash ID* (and is linked to that hash ID) it would automatically be revoked on a rebase or additional push since now the tip commit of the PR would have a different hash ID. That seems like the most robust way to go. – torek May 17 '22 at 04:03
  • The option where you have an approval set a flag, but have a `git push` operation *clear* that flag, seems likely to be a source of races: e.g., A makes PR; B approves PR as it is *while* A is adding a commit; A pushes a commit; A now sees the approval and thinks it refers to the new commit as well; A merges before the flag gets cleared. The method using hash IDs *can't* have races. – torek May 17 '22 at 04:05
  • If you deploy to prod from a branch (even when it has been approved), you run the risk of there being changes in the base branch that are no in the branch you are deploying. There could even by merge conflicts. By merging the PR to the base branch and then deploying to prod, you easily avoid this. (I know you can write workflows to do the merge, detect conflicts and all that but why take that burden on your team when GitHub has a tested, working method already.) – Lee Meador Jul 08 '22 at 16:28

1 Answers1

1

There is a branch rule you can create that will stale approvals when a new commit is made.

How to un-approve github review after new commits automatically

PaulyP
  • 165
  • 10