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

tjgurwara99 · 2022-12-14T14:11:51Z

A new CodeQL CLI gets released (roughly) every 2 weeks¹, which requires some minor updates to the extension. At the time of writing, whoever first determines that a new version of CLI has been released manually updates the version number in some places within this repository. This PR aims to reduce the manual step of updating occurrences of the old CLI version from select files so that they remain up to date for our integration tests. Note that the removal of support of CLI version is a scenario where this newly added script might cause some issues and I'm open to gathering ideas on how to tackle the time when we have to drop support for certain versions.

NOTE for maintainers: This script also requires a newly generated PAT for it to create a PULL request to update the workflow file. Therefore the Bump CLI version workflow is expected to break until a new secret WORKFLOW_TOKEN is added to the repository which would have as its value a PAT with the workflow permission.

Checklist

CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
Issues have been created for any UI or other user-facing changes made by this pull request.

In general, there's a minor version bump every quarter, which requires an extra step: remove old unsupported version of the CLI from our testing matrix. (We stop testing once we no longer support a version of GHES that shipped with that CLI. That means we only need to support 4 minor versions back, since that's how many GHES releases we support.) ↩

…ent version

aeisenberg

Also, just realized there is a problem if this workflow runs multiple times. If there is an open PR generated by this workflow, and the workflow runs again, trying to create a new PR, it will fail. I have an example of how we handle this in other repos.

.github/workflows/bump-cli.yml

scripts/replace-cli-version.sh

.github/workflows/bump-cli.yml

aeisenberg · 2022-12-14T17:55:22Z

Here is an action from a private repo that you (@tjgurwara99) should have access to. We use this to create a PR for a particular branch and close any other PRs for that branch if they are open. It might be overkill for this case, but you can still use this or copy pieces of this for this PR. https://github.com/github/semmle-code/blob/main/.github/actions/create-unique-pr/action.yml

adityasharad · 2022-12-14T18:39:43Z

High-level comment: I wonder if keeping a separate JSON file with the list of CLI versions would help. The various tests could load that file to determine what CLI versions to test against. Then your PR would only need to modify that file (and you can skip creating the PR if there is no diff). This saves you the logic of having to sed-replace the old and new versions within various workflows and test scripts, and allows us to update the entire supported version range, rather than just the last two versions.

scripts/replace-cli-version.sh

.github/workflows/bump-cli.yml

.github/actions/create-pr/action.yml

.github/workflows/main.yml

.github/workflows/bump-cli.yml

supported_cli_versions.json

.github/workflows/bump-cli.yml

aeisenberg · 2022-12-16T22:33:36Z

.github/workflows/bump-cli.yml

+      - name: Commit, Push and Open a PR
+        uses: ./.github/actions/create-pr
+        with:
+          token: ${{ secrets.GITHUB_TOKEN }}


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

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.

aeisenberg · 2022-12-16T22:34:57Z

.github/actions/create-pr/action.yml

+        COMMIT_MESSAGE: ${{ inputs.commit-message }}
+        HEAD_BRANCH: ${{ inputs.head-branch }}
+        BASE_BRANCH: ${{ inputs.base-branch }}
+        GH_TOKEN: ${{ inputs.token }}


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

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...

You need it for gh pr create.

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 😓

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).

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

aeisenberg

We need to handle the case where the minor version has changed. Short term, it's ok just to fail this workflow, but it would be nice to handle it gracefully.

aeisenberg · 2023-01-10T20:51:20Z

.github/workflows/bump-cli.yml

+  pull_request:
+    branches: [main]
+    paths:
+      - .github/actions/create-pr/action.yml


Add this file as well.

Suggested change

- .github/actions/create-pr/action.yml

- .github/actions/create-pr/action.yml

- .github/workflows/bump-cli.yml

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

aeisenberg · 2023-01-10T20:54:12Z

scripts/replace-cli-version.sh

+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/src/vscode-tests/ensureCli.ts


Fixing this can be done at a later time.

aeisenberg · 2023-01-10T21:04:31Z

scripts/replace-cli-version.sh

+LATEST_VERSION=$(echo $VERSIONS | awk '{ print $1 }')
+PREVIOUS_VERSION=$(echo $VERSIONS | awk '{ print $2 }')
+


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

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" ]

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).)

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.

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! 🤔

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 😉)

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.

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

aeisenberg

Also, please keep track of the follow-up changes for this PR. Nice work.

aeisenberg · 2023-01-12T19:20:19Z

.github/workflows/bump-cli.yml

+  pull_request:
+    branches: [main]
+    paths:
+      - .github/actions/create-pr/action.yml


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

tjgurwara99 · 2023-01-13T14:18:40Z

I don't know why the workflow Build Extension / Test is failing - anyone has any insights on this?

shati-patel · 2023-01-13T15:35:54Z

I don't know why the workflow Build Extension / Test is failing - anyone has any insights on this?

Tempted to say it's just flaky 😅 Have you tried re-running?

supported_cli_versions.json

aeisenberg

Great!

Once this is merged, can you do a trial of the new workflow to see how it goes? I'm not sure if you have any examples elsewhere.

tjgurwara99 · 2023-01-13T19:07:45Z

Once this is merged, can you do a trial of the new workflow to see how it goes?

Sure, if by trial you mean sending a workflow dispatch 😄

I'm not sure if you have any examples elsewhere.

The example run was in my fork here. I'm fairly confident that it would work once the workflow token changes from read permission to write permission but I can test it properly in this repo once it's merged. EDIT: I'm gonna eat my words - it turned out that the file was moved and my fork wasn't synced so it started failing without me noticing once I synced it 😓

Also, I don't have the permission to merge this PR, could you merge it please?

aeisenberg · 2023-01-13T21:00:45Z

Sure. I can merge. The latest test failures seem unrelated, but concerning.

aeisenberg · 2023-01-13T22:20:59Z

Hmmm...they seem like flakes.

workflow to automatically bump cli version if different from the curr…

3f39af2

…ent version

tjgurwara99 marked this pull request as ready for review December 14, 2022 15:14

tjgurwara99 requested a review from a team as a code owner December 14, 2022 15:14

aeisenberg reviewed Dec 14, 2022

View reviewed changes

tjgurwara99 marked this pull request as draft December 14, 2022 20:07

tjgurwara99 force-pushed the main branch 11 times, most recently from 84918c8 to 8460034 Compare December 14, 2022 20:45

aeisenberg reviewed Dec 14, 2022

View reviewed changes

scripts/replace-cli-version.sh Show resolved Hide resolved

aeisenberg reviewed Dec 14, 2022

View reviewed changes

.github/workflows/bump-cli.yml Show resolved Hide resolved

.github/actions/create-pr/action.yml Outdated Show resolved Hide resolved

tjgurwara99 force-pushed the main branch 6 times, most recently from b79d21a to 8e532e0 Compare December 14, 2022 22:23

aeisenberg reviewed Dec 14, 2022

View reviewed changes

.github/workflows/main.yml Outdated Show resolved Hide resolved

.github/workflows/bump-cli.yml Outdated Show resolved Hide resolved

aeisenberg reviewed Dec 14, 2022

View reviewed changes

.github/workflows/bump-cli.yml Outdated Show resolved Hide resolved

tjgurwara99 force-pushed the main branch 2 times, most recently from 2ad5584 to e296dba Compare December 14, 2022 22:49

Merge branch 'github:main' into main

f7d4891

tjgurwara99 requested a review from adityasharad December 16, 2022 17:24

Merge branch 'github:main' into main

09d744a

aeisenberg reviewed Dec 16, 2022

View reviewed changes

supported_cli_versions.json Show resolved Hide resolved

aeisenberg reviewed Dec 16, 2022

View reviewed changes

.github/workflows/bump-cli.yml Outdated Show resolved Hide resolved

.github/workflows/bump-cli.yml Outdated Show resolved Hide resolved

tjgurwara99 added 3 commits December 16, 2022 20:28

change the order of supported versions

c89d267

Merge branch 'main' of github.com:tjgurwara99/vscode-codeql

f2076df

using secrets instead of actions context

5a42b8b

tjgurwara99 requested review from aeisenberg and koesie10 December 16, 2022 21:28

aeisenberg reviewed Dec 16, 2022

View reviewed changes

tjgurwara99 requested a review from aeisenberg January 9, 2023 11:05

aeisenberg reviewed Jan 10, 2023

View reviewed changes

aeisenberg reviewed Jan 12, 2023

View reviewed changes

tjgurwara99 and others added 2 commits January 13, 2023 12:58

added the bump-cli.yml file to trigger the workflow when changed

91dc39b

Merge branch 'github:main' into main

7f05af9

shati-patel reviewed Jan 13, 2023

View reviewed changes

supported_cli_versions.json Outdated Show resolved Hide resolved

tjgurwara99 added 2 commits January 13, 2023 15:59

fix: replaced with newer version since the PR was open

c72734e

Merge branch 'main' of github.com:tjgurwara99/vscode-codeql

ef14446

aeisenberg approved these changes Jan 13, 2023

View reviewed changes

fix: src dir to test dir

365519a

tjgurwara99 requested a review from aeisenberg January 13, 2023 19:18

aeisenberg enabled auto-merge January 13, 2023 21:01

aeisenberg merged commit ecbb0fb into github:main Jan 13, 2023

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

		LATEST_VERSION=$(echo $VERSIONS \| awk '{ print $1 }')
		PREVIOUS_VERSION=$(echo $VERSIONS \| awk '{ print $2 }')

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

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

Uh oh!

Conversation

tjgurwara99 commented Dec 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Footnotes

Uh oh!

aeisenberg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aeisenberg commented Dec 14, 2022

Uh oh!

adityasharad commented Dec 14, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tjgurwara99 Dec 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aeisenberg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shati-patel Jan 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aeisenberg left a comment

Choose a reason for hiding this comment

Uh oh!

tjgurwara99 commented Dec 14, 2022 •

edited

Loading

tjgurwara99 Dec 19, 2022 •

edited

Loading

shati-patel Jan 12, 2023 •

edited

Loading

tjgurwara99 commented Jan 13, 2023 •

edited

Loading