From 10555f155c745617becfd10bea2e9d4f679b013f Mon Sep 17 00:00:00 2001 From: Jesse Hallam Date: Mon, 13 Jul 2026 05:49:33 -0300 Subject: [PATCH] Converge generated-file CI checks on a single `make generated` target (#37451) * Converge generated-file CI checks on a single make generated target The server-ci.yml workflow had many separate "run a make target, then fail on any git diff" jobs, but make generated only covered a few of them, so the target and CI drifted apart. Expand make generated to regenerate every committed asset, adding gen-serialized, migrations-extract, build-templates, mmctl-docs, and modules-tidy, and collapse the per-asset check jobs into a single check-generated job. Split the backport migration guard into its own check-backport-migrations job and make target, renaming the script to match. * git status --porcelain * simplify permissions block given defaults --- .github/workflows/server-ci.yml | 212 ++++-------------- server/Makefile | 8 +- ...hanges.sh => check_backport_migrations.sh} | 0 3 files changed, 46 insertions(+), 174 deletions(-) rename server/scripts/{check_migration_changes.sh => check_backport_migrations.sh} (100%) diff --git a/.github/workflows/server-ci.yml b/.github/workflows/server-ci.yml index e4b194b8184..e6435f014c2 100644 --- a/.github/workflows/server-ci.yml +++ b/.github/workflows/server-ci.yml @@ -64,165 +64,12 @@ jobs: uses: ./.github/actions/setup-buildenv with: go-version: ${{ steps.calculate.outputs.GO_VERSION }} - check-mocks: - name: Check mocks + check-generated: + name: Check generated files needs: go runs-on: ubuntu-22.04 - steps: - - name: Checkout mattermost project - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - with: - persist-credentials: false - - uses: ./.github/actions/run-in-buildenv - with: - run: make mocks - - name: Check mocks - run: | - if [ -n "$(git status --porcelain)" ]; then - echo "Please update the mocks using 'make mocks'" - git diff - exit 1 - fi - check-go-mod-tidy: - name: Check go mod tidy - needs: go - runs-on: ubuntu-22.04 - steps: - - name: Checkout mattermost project - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - with: - persist-credentials: false - - uses: ./.github/actions/run-in-buildenv - with: - run: make modules-tidy - - name: Check modules - run: | - if [ -n "$(git status --porcelain)" ]; then - echo "Please tidy up the Go modules using make modules-tidy" - git diff - exit 1 - fi - check-style: - name: check-style - needs: go - runs-on: ubuntu-22.04 - steps: - - name: Checkout mattermost project - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - with: - persist-credentials: false - - uses: ./.github/actions/run-in-buildenv - with: - run: make check-style - check-gen-serialized: - name: Check serialization methods for hot structs - needs: go - runs-on: ubuntu-22.04 - steps: - - name: Checkout mattermost project - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - with: - persist-credentials: false - - uses: ./.github/actions/run-in-buildenv - with: - run: make gen-serialized - - name: Check serialized - run: | - if [ -n "$(git status --porcelain)" ]; then - echo "Please update the serialized files using 'make gen-serialized'" - git diff - exit 1 - fi - check-mattermost-vet-api: - name: Vet API - needs: go - runs-on: ubuntu-22.04 - steps: - - name: Checkout mattermost project - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - with: - persist-credentials: false - - uses: ./.github/actions/run-in-buildenv - with: - run: make vet-api - check-migrations: - name: Check migration files - needs: go - runs-on: ubuntu-22.04 - steps: - - name: Checkout mattermost project - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - with: - persist-credentials: false - - uses: ./.github/actions/run-in-buildenv - with: - run: make migrations-extract - - name: Check migration files - run: | - if [ -n "$(git status --porcelain)" ]; then - echo "Please update the migrations using make migrations-extract" - git diff - exit 1 - fi - - name: Check for renumbered or renamed migrations - # Only backports (PRs targeting a release branch) need this guard: the - # migrations they add must keep the exact version+name they have on - # master. New migrations on master-targeted PRs are normal and skipped. - if: startsWith(github.base_ref, 'release-') - uses: ./.github/actions/run-in-buildenv - with: - run: | - git fetch --no-tags --depth=1 origin master "${{ github.base_ref }}" - export MM_MIGRATION_CHECK_BASE_REF="origin/${{ github.base_ref }}" - export MM_MIGRATION_CHECK_CANONICAL_REF="origin/master" - make check-migration-changes - check-email-templates: - name: Generate email templates - needs: go - runs-on: ubuntu-22.04 - steps: - - name: Checkout mattermost project - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - with: - persist-credentials: false - - uses: ./.github/actions/run-in-buildenv - with: - run: | - npm install -g mjml@4.9.0 - make build-templates - - name: Check generated email templates - run: | - if [ -n "$(git status --porcelain)" ]; then - echo "Please update the email templates using 'make build-templates'" - git diff - exit 1 - fi - check-store-layers: - name: Check store layers - needs: go - runs-on: ubuntu-22.04 - steps: - - name: Checkout mattermost project - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - with: - persist-credentials: false - - uses: ./.github/actions/run-in-buildenv - with: - run: make store-layers - - name: Check generated code - run: | - if [ -n "$(git status --porcelain)" ]; then - echo "Please update the store layers using make store-layers" - git diff - exit 1 - fi - check-default-roles-permissions: - name: Check default roles permissions - needs: go - runs-on: ubuntu-22.04 - permissions: - contents: read services: + # make default-roles-permissions snapshots a live database. postgres: image: postgres:14 env: @@ -246,33 +93,58 @@ jobs: run: | export IS_CI=true export TEST_DATABASE_POSTGRESQL_DSN="postgres://mmuser:mostest@localhost:5432/mattermost_test?sslmode=disable&connect_timeout=10" - make default-roles-permissions - - name: Check generated code + make generated + - name: Check generated files run: | if [ -n "$(git status --porcelain)" ]; then - echo "Please update the default roles permissions using make default-roles-permissions" + echo "Generated files are out of date. Please run 'make generated' and commit the result." + git status --porcelain git diff exit 1 fi - check-mmctl-docs: - name: Check mmctl docs + check-backport-migrations: + name: Check backport migrations + needs: go + # Only backports (release-* base) must keep a migration's version+name; new master migrations are expected. + if: startsWith(github.base_ref, 'release-') + runs-on: ubuntu-22.04 + steps: + - name: Checkout mattermost project + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + persist-credentials: false + - name: Check for renumbered or renamed migrations + uses: ./.github/actions/run-in-buildenv + with: + run: | + git fetch --no-tags --depth=1 origin master "${{ github.base_ref }}" + export MM_MIGRATION_CHECK_BASE_REF="origin/${{ github.base_ref }}" + export MM_MIGRATION_CHECK_CANONICAL_REF="origin/master" + make check-backport-migrations + check-style: + name: check-style needs: go runs-on: ubuntu-22.04 steps: - - name: Checkout mattermost-server + - name: Checkout mattermost project uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: persist-credentials: false - uses: ./.github/actions/run-in-buildenv with: - run: make mmctl-docs - - name: Check docs - run: | - if [ -n "$(git status --porcelain)" ]; then - echo "Please update the mmctl docs using make mmctl-docs" - git diff - exit 1 - fi + run: make check-style + check-mattermost-vet-api: + name: Vet API + needs: go + runs-on: ubuntu-22.04 + steps: + - name: Checkout mattermost project + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + persist-credentials: false + - uses: ./.github/actions/run-in-buildenv + with: + run: make vet-api # NOTE: Postgres with binary parameters has been moved to server-ci-weekly.yml # (runs Monday 1am EST / 5am UTC). Low regression risk doesn't justify # consuming 8-core runners on every push. diff --git a/server/Makefile b/server/Makefile index 4e4e3a486e9..9f6723bcb69 100644 --- a/server/Makefile +++ b/server/Makefile @@ -443,7 +443,7 @@ mocks: store-mocks filestore-mocks ldap-mocks plugin-mocks einterfaces-mocks sea layers: store-layers pluginapi .PHONY: generated -generated: mocks layers default-roles-permissions +generated: mocks layers gen-serialized migrations-extract build-templates mmctl-docs modules-tidy default-roles-permissions ## Regenerate every committed generated asset checked by the check-generated CI job. .PHONY: check-prereqs-enterprise check-prereqs-enterprise: setup-go-work ## Checks prerequisite software status for enterprise. @@ -1029,10 +1029,10 @@ migrations-extract: @echo "# Autogenerated file to synchronize migrations sequence in the PR workflow, please do not edit." > channels/db/migrations/migrations.list find channels/db/migrations -maxdepth 2 -mindepth 2 | sort >> channels/db/migrations/migrations.list -.PHONY: check-migration-changes -check-migration-changes: ## Fails if a Postgres migration added on top of the base branch was renumbered or renamed relative to master. +.PHONY: check-backport-migrations +check-backport-migrations: ## Fails if a Postgres migration added on top of the base branch was renumbered or renamed relative to master. @echo "Checking for renumbered or renamed migrations" - ./scripts/check_migration_changes.sh + ./scripts/check_backport_migrations.sh .PHONY: test-local-filestore test-local-filestore: setup-go-work # Run tests for local filestore diff --git a/server/scripts/check_migration_changes.sh b/server/scripts/check_backport_migrations.sh similarity index 100% rename from server/scripts/check_migration_changes.sh rename to server/scripts/check_backport_migrations.sh