ci/ipsec: Fix downgrade version retrieval

[ upstream commit 6fee46f ] [ backporter's note: - e2e upgrade test doesn't exist in this branch. Removed it. - Minor conflict in tests-clustermesh-upgrade.yaml ++<<<<<<< HEAD + if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then + SHA="${{ inputs.SHA }}" + else + SHA="${{ github.sha }}" + fi ++======= + CILIUM_DOWNGRADE_VERSION=$(contrib/scripts/print-downgrade-version.sh stable) + echo "downgrade_version=${CILIUM_DOWNGRADE_VERSION}" >> $GITHUB_OUTPUT ++>>>>>>> 8c3b175f5d (ci/ipsec: Fix downgrade version retrieval) ] Figuring out the right "previous patch release version number" to downgrade to in print-downgrade-version.sh turns out to be more complex than expected [0][1][2][3]. This commit is an attempt to 1) fix issues with the current script and 2) overall make the script clearer, so we can avoid repeating these mistakes. As for the fixes, there are two things that are not correct with the current version. First, we're trying to validate the existence of the tag to downgrade to, in case the script runs on top of a release preparation commit for which file VERSION has been updated to a value that does not yet contains a corresponding tag. This part of the script is actually OK, but not the way we call it in the IPsec workflow: we use "fetch-tags: true" but "fetch-depth: 0" (the default), and the two are not compatible, a shallow clone results in no tags being fetched. To address this, we retrieve the tag differently: instead of relying on "fetch-tags" from the workflow, we call "git fetch" from the script itself, provided the preconditions are met (we only run it from a Git repository, if the "origin" remote is defined). If the tag exists, either locally or remotely, then we can use it. Otherwise, the script considers that it runs from a release preparation Pull Request, and decrements the patch release number. The second issue is that we would return no value from the script if the patch release is zero. This is to avoid any attempt to find a previous patch release when working on a development branch. However, this logics is incorrect (it comes from a previous version of the script where we would always decrement the patch number). After the first release of a new minor version, it's fine to have a patch number at 0. What we should check instead is whether the version ends with "-dev". This commit brings additional changes for clarity: more comments, and a better separation between the "get latest patch release" and "get previous stable branch" cases, moving the relevant code to independent functions, plus better argument handling. We also edit the IPsec workflow to add some logs about the version retrieved. The logs should also display the script's error messages, if any, that are printed to stderr. Sample output from the script: VERSION Tag exists Prevous minor Previous patch release 1.14.3 Y v1.13 v1.14.3 1.14.1 Y v1.13 v1.14.1 1.14.0 Y v1.13 v1.14.0 1.14.1-dev N v1.13 <error> 1.15.0-dev N v1.14 <error> 1.13.90 N v1.12 v1.13.89 <- decremented 2.0.0 N <error> <error> 2.0.1 N <error> v2.0.0 <- decremented 2.1.1 N v2.0 v2.1.0 <- decremented [0] 56dfec2 ("contrib/scripts: Support patch releases in print-downgrade-version.sh") [1] 4d7902f ("contrib/scripts: Remove special handling for patch release number 90") [2] 5581963 ("ci/ipsec: Fix version retrieval for downgrades to closest patch release") [3] 3803f53 ("ci/ipsec: Fix downgrade version for release preparation commits") Fixes: 3803f53 ("ci/ipsec: Fix downgrade version for release preparation commits") Signed-off-by: Quentin Monnet <[email protected]> Signed-off-by: Yutaro Hayakawa <[email protected]>
DataDog · Mar 1, 2024 · ee4d3cb · ee4d3cb
1 parent 51602e1
commit ee4d3cb
Show file tree

Hide file tree

Showing 3 changed files with 190 additions and 36 deletions.
diff --git a/.github/workflows/tests-clustermesh-upgrade.yaml b/.github/workflows/tests-clustermesh-upgrade.yaml
@@ -112,7 +112,7 @@ jobs:
             SHA="${{ github.sha }}"
           fi
 
-          CILIUM_DOWNGRADE_VERSION=$(contrib/scripts/print-downgrade-version.sh)
+          CILIUM_DOWNGRADE_VERSION=$(contrib/scripts/print-downgrade-version.sh stable)
 
           CILIUM_IMAGE_SETTINGS=" \
             --chart-directory=./untrusted/cilium-newest/install/kubernetes/cilium \

diff --git a/.github/workflows/tests-ipsec-upgrade.yaml b/.github/workflows/tests-ipsec-upgrade.yaml
@@ -149,13 +149,14 @@ jobs:
         uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
         with:
           ref: ${{ inputs.context-ref || github.sha }}
-          # We need to be able to check the existence of the tag we want to
-          # downgrade to, in case we're on a release preparation commit and the
-          # value in VERSION does not correspond to an existing tag yet. In
-          # that case, print-downgrade-version.sh needs to adjust the patch
-          # release number to downgrade to.
-          fetch-tags: true
-          persist-credentials: false
+          # We keep the credentials here, to make sure we're able to run
+          # "git fetch" in print-downgrade-version.sh in a few steps below.
+          # We'll call it again to remove the credentials before pulling the
+          # untrusted branch from the PR. We remain in a trusted context while
+          # credentials persist.
+          # This remains faster than downloading the full project history to
+          # make tags available to print-downgrade-version.sh.
+          persist-credentials: true
 
       - name: Set Environment Variables
         uses: ./.github/actions/set-env-variables
@@ -171,7 +172,7 @@ jobs:
 
           echo sha=${SHA} >> $GITHUB_OUTPUT
           if [ "${{ matrix.mode }}" = "minor" ]; then
-            CILIUM_DOWNGRADE_VERSION=$(contrib/scripts/print-downgrade-version.sh)
+            CILIUM_DOWNGRADE_VERSION=$(contrib/scripts/print-downgrade-version.sh stable)
             IMAGE_TAG=${CILIUM_DOWNGRADE_VERSION}
           else
             # Upgrade from / downgrade to patch release.
@@ -184,9 +185,37 @@ jobs:
             # "-ci" suffix
             IMAGE_TAG=''
           fi
+          echo "CILIUM_DOWNGRADE_VERSION: ${CILIUM_DOWNGRADE_VERSION}"
+          echo "IMAGE_TAG: ${IMAGE_TAG}"
+          if [ -z "${CILIUM_DOWNGRADE_VERSION}" ]; then
+            echo "::notice::No CILIUM_DOWNGRADE_VERSION returned; skipping remaining steps"
+          fi
           echo downgrade_version=${CILIUM_DOWNGRADE_VERSION} >> $GITHUB_OUTPUT
           echo image_tag=${IMAGE_TAG} >> $GITHUB_OUTPUT
 
+      - name: Call actions/checkout again to remove credentials
+        uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
+        with:
+          ref: ${{ inputs.context-ref || github.sha }}
+          persist-credentials: false
+
+      - name: Check we effectively removed Git credentials
+        shell: bash
+        run: |
+          # For private repositories requiring authentication, check that we
+          # can no longer fetch from the repository.
+          if ! curl -sL \
+            -H "Accept: application/vnd.github+json" \
+            -H "X-GitHub-Api-Version: 2022-11-28" \
+            "https://api.github.com/repos/${{ github.repository }}" | \
+            jq --exit-status '.private == false'; then
+            echo 'Checking whether "git fetch" succeeds'
+            if git fetch origin HEAD; then
+              echo "::error::Git credentials not removed, aborting now."
+              false
+            fi
+          fi
+
       - name: Derive stable Cilium installation config
         id: cilium-stable-config
         if: ${{ steps.vars.outputs.downgrade_version != '' }}

diff --git a/contrib/scripts/print-downgrade-version.sh b/contrib/scripts/print-downgrade-version.sh
@@ -2,54 +2,179 @@
 #
 # A utility script to print the branch name of the previous stable or patch
 # release.
+#
+# The script returns the estimated branch or tag name for the version to
+# downgrade to. If it cannot determine this value, it returns nothing (and
+# prints a message to stderr). It belongs to the calling workflow to determine
+# whether an empty value should lead to an error or to skipping parts of a CI
+# job.
+#
+# To some extent, this script is taylored for Cilium's CI workflows, and is
+# probably not something you want to run for other purposes.
+#
+# Usage:
+#
+#   $ print-downgrade-version.sh <stable|patch>
+#
+# With "stable", the script prints the branch name of the previous stable
+# branch. With "patch", it attempts to find out the tag of the latest patch
+# release for the current branch.
+#
+# The version for the previous branch is computed by decrementing the minor
+# version number for the current branch, based on the value in VERSION.
+#
+# The version for the latest patch release is computed as follows:
+#
+# - Error out on the development branch (if the VERSION ends with "-dev").
+# - If the value in VERSION corresponds to an existing tag, return this value.
+# - If the value in VERSION does not correspond to an existing tag, assume we
+#   are on a release preparation Pull Request and attempt to compute the
+#   previous patch release by decrementing the patch release version.
+#
+# Environment variables:
+#
+# - VERSION: The version supposed to be in the VERSION file at the root of the
+#   repository. For testing purposes.
+# - BRANCH_SUFFIX: A suffix to append to the generated branch name, when
+#   downgrading to the lower stable branch.
+# - TAG_SUFFIX: A suffix to append to the generated tag name, when downgrading
+#   to the latest patch release, provided the script does not simply reuse the
+#   value in VERSION.
 
 set -o errexit
 set -o nounset
 
-SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
-VERSION=${VERSION-"$(cat "$SCRIPT_DIR/../../VERSION")"}
-if [[ $VERSION =~ ([0-9^]+)\.([0-9^]+)\.([0-9^]+).* ]] ; then
-    major=${BASH_REMATCH[1]}
-    minor=${BASH_REMATCH[2]}
-    patch=${BASH_REMATCH[3]}
+usage() {
+    >&2 echo "Usage: $0 <stable|patch>"
+    exit 1
+}
+
+if [[ "$#" -ne 1 ]]; then
+    usage
+fi
+case "${1}" in
+    stable|patch)
+        ;;
+    *)
+        usage
+        ;;
+esac
+
+SCRIPT_DIR="$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )"
+VERSION="${VERSION-"$(cat "${SCRIPT_DIR}/../../VERSION")"}"
+if [[ "${VERSION}" =~ ([0-9^]+)\.([0-9^]+)\.([0-9^]+).* ]] ; then
+    MAJOR="${BASH_REMATCH[1]}"
+    MINOR="${BASH_REMATCH[2]}"
+    PATCH="${BASH_REMATCH[3]}"
 else
-  >&2 echo "ERROR: failed to parse version '$VERSION'"
+  >&2 echo "ERROR: failed to parse version '${VERSION}'"
   exit 1
 fi
 
-if [[ ${1-} == "patch" ]] ; then
-    # If user passed "patch" as first argument, print the latest patch version
+tag_exists() {
+    local tag="refs/tags/${1}"
+
+    # Check if the tag is already present locally.
+    if git rev-parse --verify --end-of-options "${tag}" &> /dev/null; then
+        return 0
+    fi
+
+    # If not, try to fetch it from origin.
+    #
+    # Note: Stuff we tried before in the past (for the patch release case),
+    # instead of calling "git fetch" in this script:
+    #
+    # - Downloading the tags directly in the CI workflow YAML file, by passing
+    #   "fetch-tags: true" to the checkout Action. This does not work with
+    #   shallow clones, only the tags pointing to objects present in the clone
+    #   are fetched.
+    # - Setting "fetch-depth: 2" in the workflow to fetch two commits: the
+    #   latest commit on top of a commit that squashes all the rest of the
+    #   history. Then we can check whether the latest commit updates VERSION,
+    #   and assume we're on a release preparation PR in that case. This does
+    #   not work well, however, if the release manager pushes additional fixes
+    #   on top of the prep commit.
+    >&2 echo "INFO: tag '${tag}' not present locally, trying to fetch it from origin"
+    git fetch --depth=1 origin "+${tag}:${tag}" &> /dev/null || true
+
+    # Retry after the fetch attempt.
+    if git rev-parse --verify --end-of-options "${tag}" &> /dev/null; then
+        return 0
+    fi
 
-    if [[ "${patch}" -le "0" ]] ; then
-        >&2 echo "ERROR: failed to deduce patch release previous to version '$VERSION'"
+    return 1
+}
+
+print_prev_patch() {
+    local version="${1}" major="${2}" minor="${3}" patch="${4}"
+    local tag
+
+    # If we're on the development branch, there is no previous patch release to
+    # downgrade to. Calling workflow should typically skip the job.
+    if [[ "${version}" =~ -dev$ ]] ; then
+        >&2 echo "ERROR: no previous patch release for development version '${version}'"
         exit 1
     fi
 
-    tag="v${major}.${minor}.${patch}${TAG_SUFFIX:-}"
+    # In most cases, the previous patch release is in fact the same as
+    # indicated in $version and we just need to return it.
+    # Note that we still prefix it with "v" to match the tag format.
+    # Also note that in this case, we assume that VERSION already contains any
+    # TAG_SUFFIX that would be required, and we do not append the content from
+    # the environment variable.
+    local tag="v${VERSION}"
 
-    # Hack: When working on a patch release preparation commit, file VERSION
+    # Hack: When working on a patch release preparation PR, file VERSION
     # contains the new value for the release that is yet to be tagged and
     # published. So if the tag does not exist, we want to downgrade to the
-    # previous patch release.
+    # previous patch release, by computing the (assumed) previous tag value.
     #
-    # Only run this step if we're in a Git repository, and we have tag v1.0.0
-    # in the repo (otherwise, we're likely on a shallow clone, with no tags
-    # fetched).
+    # Only run this step if we're in a Git repository with a remote named
+    # "origin".
     if git rev-parse --is-inside-work-tree &> /dev/null && \
-        git rev-parse --verify --end-of-options v1.0.0 &> /dev/null && \
-        ! git rev-parse --verify --end-of-options "${tag}" &> /dev/null; then
-        >&2 echo "INFO: tag ${tag} not found, decrementing patch release number"
-        patch=$((patch - 1))
-        tag="v${major}.${minor}.${patch}${TAG_SUFFIX:-}"
+        git remote | grep -q '^origin$' ; then
+
+        if ! tag_exists "${tag}"; then
+            # If the patch version is 0, we cannot decrement it further.
+            if [[ "${patch}" -le "0" ]] ; then
+                >&2 echo "ERROR: failed to deduce patch release previous to version '${version}' (cannot decrement patch version)"
+                exit 1
+            fi
+
+            >&2 echo "INFO: tag '${tag}' not found, assuming we're on a release preparation Pull Request: decrementing patch release number"
+            tag="v${major}.${minor}.$((patch - 1))${TAG_SUFFIX:-}"
+
+            # Note: Based on the current usage of this script in workflows, we
+            # assume that the version computed by decrementing the patch number
+            # exists, and we don't check for its existence here. If it does not
+            # exist at this stage, the calling workflow will try to downgrade
+            # to an inexistent version and fail loudly, which is the desired
+            # behaviour.
+        fi
     fi
+
     echo "${tag}"
-else
+}
+
+print_prev_branch() {
+    local version="${1}" major="${2}" minor="${3}"
+
+    # If the minor version is 0, we cannot decrement it further.
     if [[ "${minor}" == "0" ]] ; then
-        >&2 echo "ERROR: failed to deduce release previous to version '$VERSION'"
+        >&2 echo "ERROR: failed to deduce release previous to version '${version}' (cannot decrement minor version number)"
         exit 1
     fi
-    # Else print the previous stable version by decrementing the minor version
-    # and trimming the patch version.
-    ((minor--))
+
+    # Print the previous stable version by decrementing the minor version and
+    # trimming the patch version.
+    minor=$((minor - 1))
     echo "v${major}.${minor}${BRANCH_SUFFIX:-}"
+}
+
+# If user passed "patch" as first argument, print the latest patch version.
+# Otherwise, print the latest stable version.
+if [[ ${1} == "patch" ]] ; then
+    print_prev_patch "${VERSION}" "${MAJOR}" "${MINOR}" "${PATCH}"
+else
+    print_prev_branch "${VERSION}" "${MAJOR}" "${MINOR}"
 fi