From ca17d7e2c193a2f8220b4d83453b73a07dee7517 Mon Sep 17 00:00:00 2001 From: turegjorup Date: Wed, 29 Jul 2026 21:02:23 +0200 Subject: [PATCH 1/2] test: run migrations against a populated database in CI The existing Doctrine jobs migrate an empty database, so a migration that cannot cope with existing rows passes them unnoticed. Build the database as it looks before the pull request, fixtures included, then apply the pull request's migrations on top. Fixtures created a single user, and one row cannot collide with itself, so add a few more. --- .github/workflows/doctrine.yaml | 51 +++++++++++++++++++++++++++++++++ fixtures/user.yaml | 9 ++++++ 2 files changed, 60 insertions(+) diff --git a/.github/workflows/doctrine.yaml b/.github/workflows/doctrine.yaml index 38276a6..8eb28dd 100644 --- a/.github/workflows/doctrine.yaml +++ b/.github/workflows/doctrine.yaml @@ -62,3 +62,54 @@ jobs: - name: Load fixtures run: | docker compose run --rm phpfpm composer fixtures + + # The jobs above migrate an empty database. A deployment migrates a database + # that already holds rows, so a migration that cannot cope with existing + # data passes those jobs unnoticed. This job builds the database as it looks + # before the pull request, fixtures included, and then applies the pull + # request's migrations on top of it. + migrate-populated-database: + name: Run migrations on a populated database + runs-on: ubuntu-latest + # This workflow also runs on pushes to main and develop, where there is + # no pull request to read a base branch from: the checkout steps below + # would get an empty revision and fail. A push has no base to compare + # against, so skip the job rather than guess one. + if: github.event_name == 'pull_request' + + steps: + - uses: actions/checkout@v6 + with: + fetch-depth: 0 + + - name: Create docker network + run: | + docker network create frontend + + - name: Check out the base branch + run: | + git checkout ${{ github.event.pull_request.base.sha }} + + - name: Run Composer Install + run: | + docker compose run --rm phpfpm composer install --no-interaction + + - name: Run Doctrine Migrations + run: | + docker compose run --rm phpfpm bin/console doctrine:migrations:migrate --no-interaction + + - name: Load fixtures + run: | + docker compose run --rm phpfpm composer fixtures + + - name: Check out the pull request + run: | + git checkout ${{ github.event.pull_request.head.sha }} + + - name: Run Composer Install + run: | + docker compose run --rm phpfpm composer install --no-interaction + + - name: Run Doctrine Migrations on the populated database + run: | + docker compose run --rm phpfpm bin/console doctrine:migrations:migrate --no-interaction diff --git a/fixtures/user.yaml b/fixtures/user.yaml index 1260509..76710ec 100644 --- a/fixtures/user.yaml +++ b/fixtures/user.yaml @@ -3,3 +3,12 @@ App\Entity\User: __construct: ["Admin", "admin@example.com", [ROLE_ADMIN]] user_user: __construct: ["User", "user@example.com", [ROLE_USER]] + # A few more users so migrations are exercised against a table holding more + # than one row. A single row cannot collide with itself, which hides + # migrations adding a unique column to an existing table. + user_1: + __construct: ["User 1", "user1@example.com", [ROLE_USER]] + user_2: + __construct: ["User 2", "user2@example.com", [ROLE_USER]] + user_3: + __construct: ["User 3", "user3@example.com", [ROLE_USER]] From 554ddd0539a886fd5f2817eb42df44a7a5a301b1 Mon Sep 17 00:00:00 2001 From: turegjorup Date: Wed, 29 Jul 2026 21:02:23 +0200 Subject: [PATCH 2/2] fix: let user API key migration run on populated databases Version20260702123347 added user.api_key as NOT NULL and then put a unique index on it, so every existing user got the same empty string and the index collided on the second row. Any database holding more than one user failed to migrate. Generate a key for each existing user before creating the index, matching App\Entity\User::__construct, which gives every user created since #88 a key of its own. The keys come from PHP rather than SQL because they are credentials and SQL's RAND() and UUID() are not cryptographically secure. --- CHANGELOG.md | 5 +++++ migrations/Version20260702123347.php | 12 +++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a90e2c0..0beda8d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +- [#90](https://github.com/itk-dev/devops_itksites/pull/90) + - Fixed user API key migration failing on databases with more than one user + - Generated an API key for existing users, as users created since already get + - Added users to the fixtures and a CI job running migrations on a populated + database - [#89](https://github.com/itk-dev/devops_itksites/pull/89) Added `--rm` to `docker compose run` in prod deployment - [#88](https://github.com/itk-dev/devops_itksites/pull/88) diff --git a/migrations/Version20260702123347.php b/migrations/Version20260702123347.php index 5552bbd..7139f4a 100644 --- a/migrations/Version20260702123347.php +++ b/migrations/Version20260702123347.php @@ -19,8 +19,18 @@ public function getDescription(): string public function up(Schema $schema): void { - // this up() migration is auto-generated, please modify it to your needs + // Adding the column gives every existing user the same empty string, + // which collides on the unique index. Add random keys to allow unique index. + // + // The keys are generated in PHP because they are credentials and have + // to come from a cryptographically secure source. SQL's RAND() and + // UUID() are not one. $this->addSql('ALTER TABLE user ADD api_key VARCHAR(255) NOT NULL'); + + foreach ($this->connection->fetchFirstColumn('SELECT email FROM user') as $email) { + $this->addSql('UPDATE user SET api_key = ? WHERE email = ?', [sha1(\random_bytes(40)), $email]); + } + $this->addSql('CREATE UNIQUE INDEX UNIQ_8D93D649C912ED9D ON user (api_key)'); }