Skip to content

Workflow to automatically bump cli version if different from the current version #1871

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

Merged
merged 14 commits into from
Jan 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 82 additions & 0 deletions .github/actions/create-pr/action.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
name: Create a PR if one doesn't exists
description: >
Creates a commit with the current changes to the repo, and opens a PR for that commit. If
any PR with the same title exists, then this action is marked as succeeded.
inputs:
commit-message:
description: >
The message for the commit to be created.
required: true

title:
description: >
The title of the PR. If empty, the title and body will be determined from the commit message.
default: ''
required: false

body:
description: >
The body (description) of the PR. The `title` input must be specified in order for this input to be used.
default: ''
required: false

head-branch:
description: >
The name of the branch to hold the new commit. If an existing open PR with the same head
branch exists, the new branch will be force-pushed to that PR instead of creating a new PR.
required: true

base-branch:
description: >
The base branch to target with the new PR.
required: true

token:
description: |
The GitHub token to use. It must have enough privileges to
make API calls to create and close pull requests.
required: true

runs:
using: composite
steps:
- name: Update git config
shell: bash
run: |
git config --global user.email "github-actions@github.com"
git config --global user.name "github-actions[bot]"
- name: Commit, Push and Open PR
shell: bash
env:
COMMIT_MESSAGE: ${{ inputs.commit-message }}
HEAD_BRANCH: ${{ inputs.head-branch }}
BASE_BRANCH: ${{ inputs.base-branch }}
GH_TOKEN: ${{ inputs.token }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this needs to change to GITHUB_TOKEN so that the git commands can work.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, the checkout step has already taken care of the ambiguity. Both GH_TOKEN and GITHUB_TOKEN work 😄 - I also think there might actually be no need for this since the GH_TOKEN is not being used in this action at all...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need it for gh pr create.

Copy link
Member Author

@tjgurwara99 tjgurwara99 Dec 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but this is after the checkout step which sets up the whole gh cli stuff right? We're calling it as a composite step in another action, and this itself is not a standalone action I believe 😅 Unless I'm mistaken 😓

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. I believe the checkout step sets up Git, and uses the token information to set up Git remotes and credentials, but does not do gh auth for the CLI. I believe the CLI will still be expecting a token in GH/GITHUB_TOKEN and will attempt interactive auth if that's missing (which you don't want in CI).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, I'll keep it as it is then 😄

TITLE: ${{ inputs.title }}
BODY: ${{ inputs.body }}
run: |
set -exu
if ! [[ $(git diff --stat) != '' ]]; then
exit 0 # exit early
fi
# stage changes in the working tree
git add .
git commit -m "$COMMIT_MESSAGE"
git checkout -b "$HEAD_BRANCH"
# CAUTION: gits history changes with the following
git push --force origin "$HEAD_BRANCH"
PR_JSON=$(gh pr list --state open --json number --head "$HEAD_BRANCH")
if [[ $? -ne 0 ]]; then
echo "Failed to fetch existing PRs."
exit 1
fi
PR_NUMBERS=$(echo $PR_JSON | jq '. | length')
if [[ $PR_NUMBERS -ne 0 ]]; then
echo "Found existing open PR: $PR_NUMBERS"
exit 0
fi
gh pr create --head "$HEAD_BRANCH" --base "$BASE_BRANCH" --title "$TITLE" --body "$BODY" --assignee ${{ github.actor }}
if [[ $? -ne 0 ]]; then
echo "Failed to create new PR."
exit 1
fi
39 changes: 39 additions & 0 deletions .github/workflows/bump-cli.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
name: Bump CLI version
on:
workflow_dispatch:
pull_request:
branches: [main]
paths:
- .github/actions/create-pr/action.yml
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add this file as well.

Suggested change
- .github/actions/create-pr/action.yml
- .github/actions/create-pr/action.yml
- .github/workflows/bump-cli.yml

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is the last change that needs to be done before we can merge.

- .github/workflows/bump-cli.yml
schedule:
- cron: 0 0 */14 * * # run every 14 days

permissions:
contents: write
pull-requests: write

jobs:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a permissions block to this job or workflow, with contents: write and pull-requests: write permissions. That should be enough to allow the workflow to create a PR when run from the parent repo. It will still not allow the workflow to create a PR when run from a fork, which I think is preferable.

With this change, I think we can avoid changing the setting to grant read+write permissions to workflows on PRs from forks.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh ok. I did try that but it failed, I thought maybe it still needed that permission - didn't know it was because it was from the fork, it got rejected. That's great, thanks!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It still won't work on your fork, which is ok. The write permission in the workflow gets downgraded to read on forks, thanks to the repo setting you noticed. It's safer to have read-only permissions on forks than trying to reason about whether we are granting the correct set of permissions to contributor PRs from forks.

If you're making automation changes in future it may be easier to use a branch on the parent repo (otherwise forks are totally fine).

build:
name: Build
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v3
with:
fetch-depth: 1
- name: Bump CLI
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
run: |
scripts/replace-cli-version.sh
- name: Commit, Push and Open a PR
uses: ./.github/actions/create-pr
with:
token: ${{ secrets.GITHUB_TOKEN }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the token doesn't have permission to push. I would think that contents: write should be enough.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the right permission, but it won't work while this PR runs from a fork (by design). All write permissions get downgraded to read on forks.

base-branch: main
head-branch: github-action/bump-cli
commit-message: Bump CLI version from ${{ env.PREVIOUS_VERSION }} to ${{ env.LATEST_VERSION }} for integration tests
title: Bump CLI Version to ${{ env.LATEST_VERSION }} for integration tests
body: >
Bumps CLI version from ${{ env.PREVIOUS_VERSION }} to ${{ env.LATEST_VERSION }}
15 changes: 13 additions & 2 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -183,14 +183,25 @@ jobs:
run: |
npm run integration

set-matrix:
name: Set Matrix for cli-test
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v3
- name: Set the variables
id: set-variables
run: echo "cli-versions=$(cat ./supported_cli_versions.json | jq -rc)" >> $GITHUB_OUTPUT
outputs:
cli-versions: ${{ steps.set-variables.outputs.cli-versions }}
cli-test:
name: CLI Test
runs-on: ${{ matrix.os }}
needs: [find-nightly]
needs: [find-nightly, set-matrix]
strategy:
matrix:
os: [ubuntu-latest, windows-latest]
version: ['v2.7.6', 'v2.8.5', 'v2.9.4', 'v2.10.5', 'v2.11.6', 'nightly']
version: ${{ fromJson(needs.set-matrix.outputs.cli-versions) }}
fail-fast: false
env:
CLI_VERSION: ${{ matrix.version }}
Expand Down
13 changes: 13 additions & 0 deletions scripts/replace-cli-version.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#!/bin/bash

VERSIONS=$(gh api -H "Accept: application/vnd.github+json" /repos/github/codeql-cli-binaries/releases | jq -r '.[].tag_name' | head -2)

# we are exporting these variables so that we can access these variables in the workflow
LATEST_VERSION=$(echo $VERSIONS | awk '{ print $1 }')
PREVIOUS_VERSION=$(echo $VERSIONS | awk '{ print $2 }')

Comment on lines +6 to +8
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we do a minor version bump, we need to add the new version instead of replacing it.

Doing something like this can check if the minor version has changed:

LATEST_MINOR_VERSION="$(echo $LATEST_VERSION | awk 'BEGIN{FS=".";}{print $1$2}')"
PREVIOUS_MINOR_VERSION="$(echo $PREVIOUS_VERSION | awk 'BEGIN{FS=".";}{print $1$2}')"

if [ $LATEST_MINOR_VERSION != $PREVIOUS_MINOR_VERSION ];
then
   # handle case where minor version is changed
else
  # handle case where patch version is changed
fi

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the list of versions in supported_cli_versions.json is expanding every time we have a minor version bump? For example, if we have the following in the file

[
  "v2.11.5",
  "v2.7.6",
  "v2.8.5",
  "v2.9.4",
  "v2.10.5",
  "nightly"
]

and a version v2.11.6 has been released. Do we want to add that like the following?

[
  "v2.11.6", // this is the new version that got added
  "v2.11.5",
  "v2.7.6",
  "v2.8.5",
  "v2.9.4",
  "v2.10.5",
  "nightly"
]

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

v2.11.5 -> v2.11.6 would be a patch version bump, so in that case we want to replace!

The minor version bump would be v2.12.0 in this case, so we want to add it:

[
  "v2.12.0", // this is the new minor version that got added
  "v2.11.5",
  "v2.7.6",
  "v2.8.5",
  "v2.9.4",
  "v2.10.5",
  "nightly"
]

(Or even, we could add the new minor version and remove the oldest one (i.e. v.2.7.6).)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we can actually remove 2.7.6. The removal depends on the support GHES versions and GHES 3.3 is still supported. GHES 3.3 recommends CodeQL 2.7.6, so we can't remove that version yet. In general, I don't think we can remove a version automatically because it's not strictly aligned with the CodeQL CLI releases.

Copy link
Contributor

@shati-patel shati-patel Jan 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GHES 3.3 is only supported for another 6 days (2023-01-18), so it kind of aligns with the CodeQL CLI (give or take 1 week 😄). But we don't have to automate the removal (or at least not in this PR) if we want to keep that separate/manual! 🤔

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lol, I always get confused with semver 😂 For some reason I thought number after the second dot was minor release 😅

I think the removal of a version should be done through a separate workflow/action - from what I can tell, there doesn't seem to be an automated way of telling the which version should be removed. I also think that given a manual trigger with the some inputs (like for example in the image below) and manually adding the version to remove would be better unless we want to completely automate it which may not be as simple or quick 😄 Let me know though whichever way is preferred and I can think on what I can do. PS: I think this issue was for the patch release being released every two weeks or so, so this might benefit from a separate issue which I can take (this PR has been open for a while now - I want to close it 😂 of course if it meets the acceptable standards 😉)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good points @tjgurwara99. I expect that this workflow will always be triggered manually since the timing of when to run this workflow depends on a bunch of human decisions.

I'm personally fine with focusing on patch releases for now and working on a new PR to add minor version bumps. I think we can avoid creating a second workflow for minor version bumps and handle that with a new set of inputs.

And removing releases when a GHES version drops out of support is probably something that should be done manually since it also means we can remove certain feature flags that only pertain to the older version.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added the issue and assigned it to myself 😄 will get to it whenever I get time

echo "LATEST_VERSION=$LATEST_VERSION" >> $GITHUB_ENV
echo "PREVIOUS_VERSION=$PREVIOUS_VERSION" >> $GITHUB_ENV

sed -i "s/$PREVIOUS_VERSION/$LATEST_VERSION/g" supported_cli_versions.json
sed -i "s/$PREVIOUS_VERSION/$LATEST_VERSION/g" extensions/ql-vscode/test/vscode-tests/ensureCli.ts
8 changes: 8 additions & 0 deletions supported_cli_versions.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[
"v2.11.6",
"v2.7.6",
"v2.8.5",
"v2.9.4",
"v2.10.5",
"nightly"
]