From 62f869df8e97916b8ec4e03776dfc8768fb3e5e4 Mon Sep 17 00:00:00 2001 From: Eneko Fernandez Date: Tue, 8 Aug 2023 12:16:24 +0100 Subject: [PATCH 1/8] initial writting for the issue added --- doc/issues-3907.md | 63 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 doc/issues-3907.md diff --git a/doc/issues-3907.md b/doc/issues-3907.md new file mode 100644 index 0000000000..afd6f1f889 --- /dev/null +++ b/doc/issues-3907.md @@ -0,0 +1,63 @@ +# Improving OSS Release to avoid out of order issues + +https://github.com/weaveworks/weave-gitops/issues/3907 + +## Identified out of order paths + +1. Release human merged release pr without CI job finish. +2. Release job steps ordering https://github.com/weaveworks/weave-gitops/blob/main/.github/workflows/release.yaml#L104-L107 + +## Release human merged release pr without CI job finish + +We should be looking to avoid by design ending up in a human merging it. The events happening during the release process: + +1. new release branch +2. releaser approves it +3. job releases starts +4. job release ends and branch is merged +5. release completed + +What we want to avoid is the release by mistake to do anything after 2) with the following alternatives: + +1. no human intervention has effect or not possible after 2 +2. no human intervention required during the release process + +### no human intervention has effect or not possible after 2 + +We could achieve that by extending current `branch merge protection` to block on a new `release` status check: +- if not release branch does nothing -> noop +- if release branch gets updated by the release job -> update happens after release + +Therefore, an attempt to merge a release would be blocked until the release does not happen + + +#### PoC + +Acceptance for this solution + +```gherkin + +Feature: + Scenario: can build non-release branches without changing the flow nor overhead + Given a non release pr + When build + Then CI workflow passes + And Release process build passes + Then I could merge + + Scenario: can build release branches with guardrails + Given a release pr + When build + Then CI workflow passes + And cannot merge it + When human approves it + Then release process starts + And cannot merge it + When release process ends + Then can merge it +``` +**Scenario A: non release PR** + + +**Scenario B: release PR** + From b19734f41969bc08c9b051236ae1709c7e3d25fe Mon Sep 17 00:00:00 2001 From: Eneko Fernandez Date: Tue, 8 Aug 2023 12:31:19 +0100 Subject: [PATCH 2/8] do nothing for non-release branches --- .github/workflows/is3907-release.yaml | 23 +++++++++++++++++++++++ doc/issues-3907.md | 5 ++++- 2 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/is3907-release.yaml diff --git a/.github/workflows/is3907-release.yaml b/.github/workflows/is3907-release.yaml new file mode 100644 index 0000000000..02808107e5 --- /dev/null +++ b/.github/workflows/is3907-release.yaml @@ -0,0 +1,23 @@ +name: is3907-release + +on: + pull_request_review: + types: + - submitted + +jobs: + release-not-required: + if: !startsWith(github.event.pull_request.head.ref, 'releases/') + runs-on: ubuntu-latest + outputs: + version: ${{ steps.release-version.outputs.version }} + steps: + - name: Checkout + uses: actions/checkout@v3 + with: + ref: ${{ github.event.pull_request.head.sha }} + - name: Find release version + id: release-version + run: | + version=$(echo ${{ github.event.pull_request.head.ref }} | cut -d'/' -f2) + echo "::set-output name=version::$version" \ No newline at end of file diff --git a/doc/issues-3907.md b/doc/issues-3907.md index afd6f1f889..e20a6ed485 100644 --- a/doc/issues-3907.md +++ b/doc/issues-3907.md @@ -56,7 +56,10 @@ Feature: When release process ends Then can merge it ``` -**Scenario A: non release PR** +**Scenario: can build non-release branches without changing the flow nor overhead** + + + **Scenario B: release PR** From af16b3f08a0aafbd648d154d562b71f7a8dbf839 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eneko=20Fern=C3=A1ndez?= <12957664+enekofb@users.noreply.github.com> Date: Tue, 8 Aug 2023 12:42:02 +0100 Subject: [PATCH 3/8] Update is3907-release.yaml --- .github/workflows/is3907-release.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/is3907-release.yaml b/.github/workflows/is3907-release.yaml index 02808107e5..bec12ec473 100644 --- a/.github/workflows/is3907-release.yaml +++ b/.github/workflows/is3907-release.yaml @@ -7,7 +7,7 @@ on: jobs: release-not-required: - if: !startsWith(github.event.pull_request.head.ref, 'releases/') + if: ${{ !startsWith(github.event.pull_request.head.ref, 'releases/') }} runs-on: ubuntu-latest outputs: version: ${{ steps.release-version.outputs.version }} @@ -20,4 +20,4 @@ jobs: id: release-version run: | version=$(echo ${{ github.event.pull_request.head.ref }} | cut -d'/' -f2) - echo "::set-output name=version::$version" \ No newline at end of file + echo "::set-output name=version::$version" From 45115c2ef4d84d0152e3dc100c96c6791ddcb23f Mon Sep 17 00:00:00 2001 From: Eneko Fernandez Date: Tue, 8 Aug 2023 12:43:32 +0100 Subject: [PATCH 4/8] rename --- .github/workflows/is3907-release.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/is3907-release.yaml b/.github/workflows/is3907-release.yaml index bec12ec473..208c878373 100644 --- a/.github/workflows/is3907-release.yaml +++ b/.github/workflows/is3907-release.yaml @@ -1,4 +1,4 @@ -name: is3907-release +name: is3907release on: pull_request_review: From d6f1098d1ee1c4b4e9d7f80edbb9bd20e9d4d07c Mon Sep 17 00:00:00 2001 From: Eneko Fernandez Date: Tue, 8 Aug 2023 12:47:16 +0100 Subject: [PATCH 5/8] just silly ci workflow --- .github/workflows/pr.yaml | 257 +------------------------------------- 1 file changed, 1 insertion(+), 256 deletions(-) diff --git a/.github/workflows/pr.yaml b/.github/workflows/pr.yaml index 6eafcc5f94..9d92f2eb3a 100644 --- a/.github/workflows/pr.yaml +++ b/.github/workflows/pr.yaml @@ -13,61 +13,6 @@ env: name: PR CI Workflow jobs: - ci-js: - name: CI Test JS - runs-on: ubuntu-latest - strategy: - matrix: - node-version: [16.X] - steps: - - uses: actions/checkout@v3 - - name: Node modules cache - uses: actions/cache@v2 - env: - cache-name: cache-node-modules - with: - # npm cache files are stored in `~/.npm` on Linux/macOS - path: ~/.npm - key: ${{ runner.os }}-build-${{ env.cache-name }}-${{ hashFiles('**/package-lock.json') }} - restore-keys: | - ${{ runner.os }}-build-${{ env.cache-name }}- - ${{ runner.os }}-build- - ${{ runner.os }}- - - name: Use Node.js ${{ matrix.node-version }} - uses: actions/setup-node@v3 - with: - node-version: ${{ matrix.node-version }} - - run: make node_modules - - name: Check that package.json & package-lock.json were updated in commit - run: | - echo "Using node.js "$(node --version) - echo "Using NPM "$(npm --version) - git diff --no-ext-diff --exit-code - - run: make ui-audit - - run: make ui - - run: make ui-lint - - run: make ui-prettify-check - - run: make ui-test - - run: make ui-lib - - ci-go: - name: CI Test Go - runs-on: ubuntu-latest - strategy: - matrix: - go-version: [1.20.X] - steps: - - uses: actions/checkout@v3 - - name: Setup Go - uses: actions/setup-go@v4 - with: - go-version: ${{ matrix.go-version }} - - name: Setup Flux CLI - uses: fluxcd/flux2/action@main - with: - token: ${{ secrets.GITHUB_TOKEN }} - - run: make unit-tests - # - run: make lib-test ci-static: name: CI Check Static Checks @@ -82,204 +27,4 @@ jobs: uses: actions/setup-go@v4 with: go-version: ${{ matrix.go-version }} - - run: make check-format - - run: make lint - - run: go mod tidy - - name: Check that go mod tidy has been run - run: git diff --no-ext-diff --exit-code - - run: make proto - - name: Check that make proto has been run - run: git diff --no-ext-diff --exit-code - - run: make fakes - - name: Check that make fakes has been run - run: git diff --no-ext-diff --exit-code - - ci-generate-tag: - name: CI Generate Image Tag - runs-on: ubuntu-latest - outputs: - tag: ${{ steps.generate-tag.outputs.tag }} - steps: - - id: generate-tag - run: echo "::set-output name=tag::$(date -u +%s)-${{ github.sha }}" - - ci-build-gitops-image: - name: CI Build Gitops Docker Image - runs-on: ubuntu-latest - needs: [ci-generate-tag] - strategy: - matrix: - docker-image: - - gitops - - gitops-server - steps: - - uses: actions/checkout@v3 - - uses: docker/setup-buildx-action@v2 - - name: Set build-time flags - run: | - echo "LDFLAGS=$(make echo-ldflags)" >> $GITHUB_ENV - echo "FLUX_VERSION=$(make echo-flux-version)" >> $GITHUB_ENV - - name: Build and export - uses: docker/build-push-action@v3 - with: - tags: "${{ env.CI_CONTAINER_REPOSITORY }}/${{ matrix.docker-image }}:${{ needs.ci-generate-tag.outputs.tag }}" - outputs: type=docker,dest=/tmp/${{ matrix.docker-image }}.tar - file: ${{ matrix.docker-image }}.dockerfile - build-args: | - FLUX_VERSION=${{ env.FLUX_VERSION }} - LDFLAGS=${{ env.LDFLAGS }} - GIT_COMMIT=${{ github.sha }} - - name: Load docker image - run: docker load --input /tmp/${{ matrix.docker-image }}.tar - - name: Cache docker image - uses: actions/upload-artifact@v3 - with: - name: ${{ matrix.docker-image }} - path: /tmp/${{ matrix.docker-image }}.tar - - ci-upload-images: - name: CI Upload Images - runs-on: ubuntu-latest - # Make sure we only upload images if tests etc have passed - needs: [ci-go, ci-static, ci-js, ci-build-gitops-image, ci-generate-tag] - permissions: - contents: 'read' - id-token: 'write' - if: github.event_name == 'push' - strategy: - matrix: - docker-image: - - gitops - - gitops-server - steps: - - uses: docker/setup-buildx-action@v2 - - uses: google-github-actions/setup-gcloud@v1 - - name: Download cached docker image - uses: actions/download-artifact@v3 - with: - name: ${{ matrix.docker-image }} - path: /tmp - - name: Authenticate to Google Cloud - id: gcloud-auth - uses: google-github-actions/auth@v1 - with: - service_account: ${{ secrets.service_account }} - workload_identity_provider: ${{ secrets.workload_identity_provider }} - - name: Login to gcloud for docker - run: gcloud --quiet auth configure-docker ${{ env.CI_CONTAINER_REGISTRY }} - - name: Push images to gcloud - run: | - docker load --input /tmp/${{ matrix.docker-image }}.tar - docker push "${{ env.CI_CONTAINER_REPOSITORY }}/${{ matrix.docker-image }}:${{ needs.ci-generate-tag.outputs.tag }}" - - ci-upload-binary: - name: Upload Binary - runs-on: ${{matrix.os}} - needs: [ci-go, ci-static, ci-js, ci-build-gitops-image] - strategy: - matrix: - os: [ubuntu-latest, macOS-latest] - if: github.event_name == 'push' - steps: - - name: Install Go - uses: actions/setup-go@v4 - with: - go-version: 1.20.X - - name: Checkout code - uses: actions/checkout@v3 - - name: Clean - run: make clean - - id: gitsha - run: | - gitsha=$(git rev-parse --short ${{ github.sha }}) - echo "::set-output name=sha::$gitsha" - - name: build - run: | - make gitops - - name: publish to s3 - uses: aws-actions/configure-aws-credentials@v2 - with: - aws-access-key-id: ${{ secrets.AWS_ACCESS_KEY_ID }} - aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY }} - aws-region: us-east-2 - - run: | - aws s3 cp bin/gitops s3://weave-gitops/gitops-${{matrix.os}}-${{steps.gitsha.outputs.sha}} - aws s3 cp s3://weave-gitops/gitops-${{matrix.os}}-${{steps.gitsha.outputs.sha}} s3://weave-gitops/gitops-${{matrix.os}} - - ci-publish-js-lib: - name: Publish js library - runs-on: ubuntu-latest - needs: [ci-js] - permissions: - packages: write - outputs: - js-version: ${{ steps.package-version.outputs.js-version }} - steps: - - name: Checkout - uses: actions/checkout@v3 - with: - # avoid the merge commit that on.pull_request creates - # fallback to github.sha if not present (e.g. on.push(main)) - # https://github.com/actions/checkout#checkout-pull-request-head-commit-instead-of-merge-commit - # We want the correct sha so we can tag the npm package correctly - ref: ${{ github.event.pull_request.head.sha || github.sha }} - fetch-depth: 0 - - uses: actions/setup-node@v3 - with: - node-version: "16.X" - registry-url: "https://npm.pkg.github.com" - scope: "@weaveworks" - - run: npm install - - run: make ui-lib - - name: Update package version - id: package-version - run: | - GITOPS_VERSION=$(git describe) - echo "::set-output name=js-version::$GITOPS_VERSION" - jq '.version = "'$GITOPS_VERSION'" | .name = "@weaveworks/weave-gitops-main"' < dist/package.json > dist/package-new.json - mv dist/package-new.json dist/package.json - cp .npmrc dist - - run: cd dist && npm publish - env: - NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - - # We only push images on merge so create a passing check if everything finished - finish-ci-pr: - name: PR CI Pipeline - runs-on: ubuntu-latest - needs: - - ci-go - - ci-static - - ci-js - - ci-build-gitops-image - if: github.event_name != 'push' - steps: - - run: echo "All done" - - finish-ci-merge: - # must match https://github.com/weaveworks/corp/blob/master/github-repo-weave-gitops.tf - name: PR CI Pipeline - runs-on: ubuntu-latest - needs: - - ci-upload-images - - ci-upload-binary - - ci-publish-js-lib - steps: - - run: echo "All done" - - notify-failure: - name: Notify Slack on Failure - runs-on: ubuntu-latest - needs: - - finish-ci-merge - if: ${{ failure() }} - steps: - - name: Send alert - if: github.event_name == 'push' - uses: actions-ecosystem/action-slack-notifier@fc778468d09c43a6f4d1b8cccaca59766656996a - with: - slack_token: ${{ secrets.SLACK_TOKEN_BLUETONIUM }} - message: ":sad-parrot: The from ${{ github.actor }} is failing on main. and weep." - channel: team-denim - color: red - verbose: false + - run: make check-format \ No newline at end of file From 91a547ab952d105363950e42385ca35d94c38949 Mon Sep 17 00:00:00 2001 From: Eneko Fernandez Date: Tue, 8 Aug 2023 12:47:47 +0100 Subject: [PATCH 6/8] removed fossa --- .github/workflows/scan.yaml | 29 ----------------------------- 1 file changed, 29 deletions(-) delete mode 100644 .github/workflows/scan.yaml diff --git a/.github/workflows/scan.yaml b/.github/workflows/scan.yaml deleted file mode 100644 index 36a15f0963..0000000000 --- a/.github/workflows/scan.yaml +++ /dev/null @@ -1,29 +0,0 @@ -name: Code Scan - -on: - push: - branches: - - main - - v2 - pull_request: - branches: - - main - - v2 - workflow_dispatch: - -jobs: - fossa: - name: FOSSA - runs-on: ubuntu-latest - steps: - - name: Checkout - uses: actions/checkout@v3 - - name: Install Go - uses: actions/setup-go@v4 - with: - go-version: 1.20.X - - name: Run FOSSA scan and upload build data - uses: fossa-contrib/fossa-action@v2 - with: - fossa-api-key: 93622b4d45d39a92872a9593c815d7f3 - github-token: ${{ github.token }} From eeb904865bbba2f4d1b8d54ce3fa57540a0dfef1 Mon Sep 17 00:00:00 2001 From: Eneko Fernandez Date: Tue, 8 Aug 2023 12:54:34 +0100 Subject: [PATCH 7/8] update flow --- .github/workflows/pr.yaml | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/.github/workflows/pr.yaml b/.github/workflows/pr.yaml index 9d92f2eb3a..681bd07347 100644 --- a/.github/workflows/pr.yaml +++ b/.github/workflows/pr.yaml @@ -27,4 +27,13 @@ jobs: uses: actions/setup-go@v4 with: go-version: ${{ matrix.go-version }} - - run: make check-format \ No newline at end of file + - run: make check-format + + finish-ci-pr: + name: PR CI Pipeline + runs-on: ubuntu-latest + needs: + - ci-static + if: github.event_name != 'push' + steps: + - run: echo "All done" \ No newline at end of file From c61ec1c0e554fea42b5a380b79794b510c1714cd Mon Sep 17 00:00:00 2001 From: Eneko Fernandez Date: Tue, 8 Aug 2023 17:54:49 +0100 Subject: [PATCH 8/8] updating flow to include release path --- .github/workflows/is3907-release.yaml | 38 ++++++++++++++++++++++++--- doc/issues-3907.md | 27 +++++++++++-------- 2 files changed, 51 insertions(+), 14 deletions(-) diff --git a/.github/workflows/is3907-release.yaml b/.github/workflows/is3907-release.yaml index 208c878373..c113141a28 100644 --- a/.github/workflows/is3907-release.yaml +++ b/.github/workflows/is3907-release.yaml @@ -1,9 +1,13 @@ name: is3907release on: - pull_request_review: - types: - - submitted + push: + branches: + - main + pull_request: + branches: + - main + workflow_dispatch: jobs: release-not-required: @@ -21,3 +25,31 @@ jobs: run: | version=$(echo ${{ github.event.pull_request.head.ref }} | cut -d'/' -f2) echo "::set-output name=version::$version" + + release: + if: ${{ startsWith(github.event.pull_request.head.ref, 'releases/') }} + runs-on: ubuntu-latest + outputs: + version: ${{ steps.release-version.outputs.version }} + steps: + - name: Checkout + uses: actions/checkout@v3 + with: + ref: ${{ github.event.pull_request.head.sha }} + - name: Find release version + id: release-version + run: | + version=$(echo ${{ github.event.pull_request.head.ref }} | cut -d'/' -f2) + echo "::set-output name=version::$version" + - name: Wait on release status check + run: | + curl --request POST \ + --url https://api.github.com/repos/${{ github.repository }}/statuses/${{ github.event.pull_request.head.sha }} \ + --header 'authorization: Bearer ${{ secrets.WEAVE_GITOPS_BOT_ACCESS_TOKEN }}' \ + --header 'content-type: application/json' \ + --data '{ + "state":"success", + "target_url":"https://example.com/build/status", + "description":"The build succeeded!", + "context":"continuous-integration/jenkins" + }' diff --git a/doc/issues-3907.md b/doc/issues-3907.md index e20a6ed485..d970b82d3a 100644 --- a/doc/issues-3907.md +++ b/doc/issues-3907.md @@ -36,27 +36,32 @@ Therefore, an attempt to merge a release would be blocked until the release does Acceptance for this solution ```gherkin - -Feature: - Scenario: can build non-release branches without changing the flow nor overhead +Feature: can build non-release branches without changing the flow nor overhead + Scenario: current flow Given a non release pr When build Then CI workflow passes - And Release process build passes - Then I could merge + And I could merge +``` - Scenario: can build release branches with guardrails + +```gherkin +Feature: can build release branches with guardrails + + Scenario: add a check to the PR that only gets passed when release goes on Given a release pr + And CI build check preventing merging + And Release check preventing merging When build - Then CI workflow passes - And cannot merge it + Then CI workflow passes so CI build check passes + And cannot merge it cause release hasnt happened When human approves it Then release process starts - And cannot merge it - When release process ends + And cannot merge it cause release hasnt happened + When release process ends so release check passes Then can merge it ``` -**Scenario: can build non-release branches without changing the flow nor overhead** +