Check removed columns against ignore_column directives only

Andrew Dunnrequested to merge
fix/migration-validator-ignore-columns-match into master

What does this MR do and why?

A developer who drops or renames a column must first ship a release in which the model ignores it. Rails caches the column list, so code that is still running during a deploy keeps selecting a column that no longer exists. The db:check-migrations job stops this. It reads each new migration, finds the model for the table, and fails unless the model has an ignore_column for the column.

Today the job only asks whether the column name appears somewhere after an ignore_column in the model file. Once a model ignores any column, a comment, a method body or the end of a longer name is enough. Ignoring :namespace_id lets a drop of :id pass. For app/models/project.rb, 51 of the 81 columns the model does not ignore pass the check.

With this change the job reads the names listed in the ignore_column and ignore_columns directives and checks the dropped column against that list.

flowchart TD
    A[New migration drops or renames a column] --> B[Find the model for the table]
    B --> C[Read the names listed in the model's ignore_column and ignore_columns directives]
    C --> D{Is the dropped column in that list?}
    D -- yes --> E[Pass]
    D -- no --> F[Fail, naming Model: column]
    classDef changed fill:#fff3cd,stroke:#b58900,color:#333
    class C changed

!177474 (merged) added the check with a pattern anchored to the start of the list. !181660 (merged) loosened it so directives that continue over several lines pass. That still holds here: the names are read across line breaks, from :symbol lists, %i[...] and [...] arrays.

What changed

The matching is in IGNORE_DIRECTIVE_REGEX and ignored_columns in scripts/migration_schema_validator.rb.

  • A commented-out ignore_column no longer counts as an ignore.
  • [...] arrays and parenthesized calls now count. Before, they counted only when an earlier directive let the loose match through.
  • A directive that names its columns through a constant, a variable, a bare string or %w[...] is not read, so the job fails with its usual message. No model in app/models or ee/app/models is written that way.

New examples in spec/scripts/migration_schema_validator_spec.rb make three loose matches fail: a later mention of the dropped column (the output names User: foo), a longer name such as :other_foo, and a commented-out directive. Multi-line symbol lists and %i[...] arrays still pass, and [...] arrays and parenthesized calls now pass.

References

How to set up and validate locally

  1. Run the spec file: 
    bundle exec rspec spec/scripts/migration_schema_validator_spec.rb
  2. Commit a migration that drops a column app/models/project.rb does not ignore but mentions later: 
    # db/post_migrate/20261015000001_remove_project_name.rb
class RemoveProjectName < Gitlab::Database::Migration[2.4]
  milestone '19.6'

  def up
    remove_column :projects, :name
  end

  def down
    add_column :projects, :name, :string
  end
end
  3. Run the validator with the skip label set, so it stops after the ignore check: 
    CI_MERGE_REQUEST_LABELS=pipeline::skip-check-migrations CI_MERGE_REQUEST_TARGET_BRANCH_NAME=origin/master scripts/validate_migration_schema
    On master it exits 0 with "Label pipeline::skip-check-migrations is present, skipping schema validation". With this change it exits 1 and prints Project: name.
  4. Add ignore_column :name, remove_with: '19.6', remove_after: '2026-11-20' to app/models/project.rb and run the command again. It exits 0 at the skip-label message.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

An AI coding assistant wrote this change. It ran the spec file, RuboCop on the changed files and a mutation pass over the changed lines on a test VM under Ruby 3.3.12, and the validator commands above against a full checkout of this branch under Ruby 3.4.10. It also ran a script that compares the old and new checks over every model with an ignore_column directive, which gave the 51 of 81 figure.

Merge request reports