Add rubocop cop to prevent model usage in migration specs

Migration specs roll back the schema before running, so model classes
may reference columns that don't exist yet. Using raw Sequel operations
ensures specs are independent of the current model code, as documented
in spec/migrations/Readme.md.

- Add Migration/NoModelInSpecs cop that flags any method call on a
  *Model constant receiver in spec/migrations/
- Replace model .make calls with raw db[:table].insert in four
  migration specs: sidecars, revision_sidecars, sidecar_process_types,
  and revision_process_commands

Author: philippthun

.rubocop_cc.yml

@@ -11,6 +11,7 @@ plugins:
 require:
   - ./spec/linters/migration/add_constraint_name.rb
   - ./spec/linters/migration/include_string_size.rb
+  - ./spec/linters/migration/no_model_in_specs.rb
   - ./spec/linters/migration/require_primary_key.rb
   - ./spec/linters/migration/too_many_migration_runs.rb
   - ./spec/linters/match_requires_with_includes.rb
@@ -67,6 +68,12 @@ Migration/IncludeStringSize: # Exclude for old Migrations
   Exclude:
     - !ruby/regexp /db/migrations/201([0-9]).+\.rb$/
     - !ruby/regexp /db/migrations/202([0-2]).+\.rb$/
+Migration/NoModelInSpecs:
+  Include:
+    - spec/migrations/**/*
+  Exclude:
+    # Uses custom tmp_migrations_dir so schema is never rolled back
+    - spec/migrations/20190712210940_backfill_status_for_deployments_spec.rb
 Migration/RequirePrimaryKey: # Exclude for old Migrations
   Include:
     - db/migrations/**/*

spec/linters/migration/no_model_in_specs.rb

@@ -0,0 +1,25 @@
+module RuboCop
+  module Cop
+    module Migration
+      class NoModelInSpecs < RuboCop::Cop::Base
+        MSG = 'Do not use model classes in migration specs. ' \
+              'Use raw Sequel operations (e.g. db[:table].insert) instead. ' \
+              'See spec/migrations/Readme.md for details.'
+
+        def on_send(node)
+          add_offense(node) if model_receiver?(node.receiver)
+        end
+
+        private
+
+        def model_receiver?(receiver)
+          return false unless receiver
+          return false unless receiver.const_type?
+
+          name = receiver.const_name.to_s
+          name.end_with?('Model') && name != 'Sequel::Model'
+        end
+      end
+    end
+  end
+end

spec/linters/migration/no_model_in_specs_spec.rb

@@ -0,0 +1,74 @@
+require 'spec_helper'
+require 'rubocop'
+require 'rubocop/rspec/cop_helper'
+require 'rubocop/config'
+require 'linters/migration/no_model_in_specs'
+
+RSpec.describe RuboCop::Cop::Migration::NoModelInSpecs do
+  include CopHelper
+
+  subject(:cop) { described_class.new(RuboCop::Config.new({})) }
+
+  let(:message) do
+    'Do not use model classes in migration specs. ' \
+      'Use raw Sequel operations (e.g. db[:table].insert) instead. ' \
+      'See spec/migrations/Readme.md for details.'
+  end
+
+  it 'registers an offense for Model.make' do
+    result = inspect_source(<<~RUBY)
+      VCAP::CloudController::AppModel.make
+    RUBY
+
+    expect(result.size).to eq(1)
+    expect(result.map(&:message)).to eq([message])
+  end
+
+  it 'registers an offense for Model.make!' do
+    result = inspect_source(<<~RUBY)
+      VCAP::CloudController::AppModel.make!
+    RUBY
+
+    expect(result.size).to eq(1)
+  end
+
+  it 'registers an offense for Model.create' do
+    result = inspect_source(<<~RUBY)
+      VCAP::CloudController::DeploymentModel.create(guid: 'test')
+    RUBY
+
+    expect(result.size).to eq(1)
+  end
+
+  it 'registers an offense for Model.where' do
+    result = inspect_source(<<~RUBY)
+      VCAP::CloudController::AppModel.where(guid: 'test').first
+    RUBY
+
+    expect(result.size).to eq(1)
+  end
+
+  it 'does not register an offense for raw Sequel inserts' do
+    result = inspect_source(<<~RUBY)
+      db[:apps].insert(guid: 'test', name: 'test-app')
+    RUBY
+
+    expect(result.size).to eq(0)
+  end
+
+  it 'does not register an offense for Sequel::Model' do
+    result = inspect_source(<<~RUBY)
+      Sequel::Model.db
+    RUBY
+
+    expect(result.size).to eq(0)
+  end
+
+  it 'does not register an offense for non-Model constants' do
+    result = inspect_source(<<~RUBY)
+      SecureRandom.uuid
+    RUBY
+
+    expect(result.size).to eq(0)
+  end
+end

spec/migrations/20251030100000_add_unique_constraint_to_sidecar_process_types_spec.rb

@@ -6,22 +6,27 @@
     let(:migration_filename) { '20251030100000_add_unique_constraint_to_sidecar_process_types.rb' }
   end
 
-  let!(:app) { VCAP::CloudController::AppModel.make }
-  let!(:sidecar) { VCAP::CloudController::SidecarModel.make(app:) }
-  let!(:revision) { VCAP::CloudController::RevisionModel.make(app:) }
-  let!(:revision_sidecar) { VCAP::CloudController::RevisionSidecarModel.make(revision:) }
+  let(:app_guid) { SecureRandom.uuid }
+  let(:sidecar_guid) { SecureRandom.uuid }
+  let(:revision_guid) { SecureRandom.uuid }
+  let(:revision_sidecar_guid) { SecureRandom.uuid }
 
   it 'removes duplicates, adds unique constraints, and is reversible' do
+    db[:apps].insert(guid: app_guid, name: 'test-app')
+    db[:sidecars].insert(guid: sidecar_guid, name: 'test-sidecar', command: 'command', app_guid: app_guid)
+    db[:revisions].insert(guid: revision_guid, app_guid: app_guid)
+    db[:revision_sidecars].insert(guid: revision_sidecar_guid, name: 'test-sidecar', command: 'command', revision_guid: revision_guid)
+
     # =========================================================================================
     # SETUP: Create duplicate entries for both tables to test the de-duplication logic.
     # =========================================================================================
-    db[:sidecar_process_types].insert(sidecar_guid: sidecar.guid, type: 'web', app_guid: app.guid, guid: SecureRandom.uuid)
-    db[:sidecar_process_types].insert(sidecar_guid: sidecar.guid, type: 'web', app_guid: app.guid, guid: SecureRandom.uuid)
-    expect(db[:sidecar_process_types].where(sidecar_guid: sidecar.guid, type: 'web').count).to eq(2)
+    db[:sidecar_process_types].insert(sidecar_guid: sidecar_guid, type: 'web', app_guid: app_guid, guid: SecureRandom.uuid)
+    db[:sidecar_process_types].insert(sidecar_guid: sidecar_guid, type: 'web', app_guid: app_guid, guid: SecureRandom.uuid)
+    expect(db[:sidecar_process_types].where(sidecar_guid: sidecar_guid, type: 'web').count).to eq(2)
 
-    db[:revision_sidecar_process_types].insert(revision_sidecar_guid: revision_sidecar.guid, type: 'worker', guid: SecureRandom.uuid)
-    db[:revision_sidecar_process_types].insert(revision_sidecar_guid: revision_sidecar.guid, type: 'worker', guid: SecureRandom.uuid)
-    expect(db[:revision_sidecar_process_types].where(revision_sidecar_guid: revision_sidecar.guid, type: 'worker').count).to eq(2)
+    db[:revision_sidecar_process_types].insert(revision_sidecar_guid: revision_sidecar_guid, type: 'worker', guid: SecureRandom.uuid)
+    db[:revision_sidecar_process_types].insert(revision_sidecar_guid: revision_sidecar_guid, type: 'worker', guid: SecureRandom.uuid)
+    expect(db[:revision_sidecar_process_types].where(revision_sidecar_guid: revision_sidecar_guid, type: 'worker').count).to eq(2)
 
     # =========================================================================================
     # UP MIGRATION: Run the migration to apply the unique constraints.
@@ -31,13 +36,13 @@
     # =========================================================================================
     # ASSERT UP MIGRATION: Verify that duplicates are removed and constraints are enforced.
     # =========================================================================================
-    expect(db[:sidecar_process_types].where(sidecar_guid: sidecar.guid, type: 'web').count).to eq(1)
+    expect(db[:sidecar_process_types].where(sidecar_guid: sidecar_guid, type: 'web').count).to eq(1)
     expect(db.indexes(:sidecar_process_types)).to include(:sidecar_process_types_sidecar_guid_type_index)
-    expect { db[:sidecar_process_types].insert(sidecar_guid: sidecar.guid, type: 'web', app_guid: app.guid, guid: SecureRandom.uuid) }.to raise_error(Sequel::UniqueConstraintViolation)
+    expect { db[:sidecar_process_types].insert(sidecar_guid: sidecar_guid, type: 'web', app_guid: app_guid, guid: SecureRandom.uuid) }.to raise_error(Sequel::UniqueConstraintViolation)
 
-    expect(db[:revision_sidecar_process_types].where(revision_sidecar_guid: revision_sidecar.guid, type: 'worker').count).to eq(1)
+    expect(db[:revision_sidecar_process_types].where(revision_sidecar_guid: revision_sidecar_guid, type: 'worker').count).to eq(1)
     expect(db.indexes(:revision_sidecar_process_types)).to include(:revision_sidecar_process_types_revision_sidecar_guid_type_index)
-    expect { db[:revision_sidecar_process_types].insert(revision_sidecar_guid: revision_sidecar.guid, type: 'worker', guid: SecureRandom.uuid) }.to raise_error(Sequel::UniqueConstraintViolation)
+    expect { db[:revision_sidecar_process_types].insert(revision_sidecar_guid: revision_sidecar_guid, type: 'worker', guid: SecureRandom.uuid) }.to raise_error(Sequel::UniqueConstraintViolation)
 
     # =========================================================================================
     # TEST IDEMPOTENCY: Running the migration again should not cause any errors.
@@ -53,9 +58,9 @@
     # ASSERT DOWN MIGRATION: Verify that constraints are removed and duplicates can be re-inserted.
     # =========================================================================================
     expect(db.indexes(:sidecar_process_types)).not_to include(:sidecar_process_types_sidecar_guid_type_index)
-    expect { db[:sidecar_process_types].insert(sidecar_guid: sidecar.guid, type: 'web', app_guid: app.guid, guid: SecureRandom.uuid) }.not_to raise_error
+    expect { db[:sidecar_process_types].insert(sidecar_guid: sidecar_guid, type: 'web', app_guid: app_guid, guid: SecureRandom.uuid) }.not_to raise_error
 
     expect(db.indexes(:revision_sidecar_process_types)).not_to include(:revision_sidecar_process_types_revision_sidecar_guid_type_index)
-    expect { db[:revision_sidecar_process_types].insert(revision_sidecar_guid: revision_sidecar.guid, type: 'worker', guid: SecureRandom.uuid) }.not_to raise_error
+    expect { db[:revision_sidecar_process_types].insert(revision_sidecar_guid: revision_sidecar_guid, type: 'worker', guid: SecureRandom.uuid) }.not_to raise_error
   end
 end

spec/migrations/20260320141005_add_unique_constraint_to_revision_sidecars_spec.rb

@@ -5,22 +5,25 @@
     let(:migration_filename) { '20260320141005_add_unique_constraint_to_revision_sidecars.rb' }
   end
 
-  let!(:app) { VCAP::CloudController::AppModel.make }
-  let!(:revision) { VCAP::CloudController::RevisionModel.make(:app) }
+  let(:app_guid) { SecureRandom.uuid }
+  let(:revision_guid) { SecureRandom.uuid }
 
   it 'remove duplicates, add constraint and revert migration' do
+    db[:apps].insert(guid: app_guid, name: 'test-app')
+    db[:revisions].insert(guid: revision_guid, app_guid: app_guid)
+
     # create duplicate entries
-    db[:revision_sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', revision_guid: revision.guid)
-    db[:revision_sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', revision_guid: revision.guid)
-    expect(db[:revision_sidecars].where(name: 'app', revision_guid: revision.guid).count).to eq(2)
+    db[:revision_sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', revision_guid: revision_guid)
+    db[:revision_sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', revision_guid: revision_guid)
+    expect(db[:revision_sidecars].where(name: 'app', revision_guid: revision_guid).count).to eq(2)
 
     # run the migration
     Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true)
 
     # verify duplicates are removed and constraint is enforced
-    expect(db[:revision_sidecars].where(name: 'app', revision_guid: revision.guid).count).to eq(1)
+    expect(db[:revision_sidecars].where(name: 'app', revision_guid: revision_guid).count).to eq(1)
     expect(db.indexes(:revision_sidecars)).to include(:revision_sidecars_revision_guid_name_index)
-    expect { db[:revision_sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', revision_guid: revision.guid) }.to raise_error(Sequel::UniqueConstraintViolation)
+    expect { db[:revision_sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', revision_guid: revision_guid) }.to raise_error(Sequel::UniqueConstraintViolation)
 
     # running the migration again should not cause any errors
     expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) }.not_to raise_error
@@ -30,6 +33,6 @@
 
     # verify constraint is removed and duplicates can be re-inserted
     expect(db.indexes(:revision_sidecars)).not_to include(:revision_sidecars_revision_guid_name_index)
-    expect { db[:revision_sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', revision_guid: revision.guid) }.not_to raise_error
+    expect { db[:revision_sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', revision_guid: revision_guid) }.not_to raise_error
   end
 end

spec/migrations/20260323092954_add_unique_constraint_to_sidecars_spec.rb

@@ -5,21 +5,23 @@
     let(:migration_filename) { '20260323092954_add_unique_constraint_to_sidecars.rb' }
   end
 
-  let!(:app) { VCAP::CloudController::AppModel.make }
+  let(:app_guid) { SecureRandom.uuid }
 
   it 'remove duplicates, add constraint and revert migration' do
+    db[:apps].insert(guid: app_guid, name: 'test-app')
+
     # create duplicate entries
-    db[:sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', app_guid: app.guid)
-    db[:sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', app_guid: app.guid)
-    expect(db[:sidecars].where(name: 'app', app_guid: app.guid).count).to eq(2)
+    db[:sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', app_guid: app_guid)
+    db[:sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', app_guid: app_guid)
+    expect(db[:sidecars].where(name: 'app', app_guid: app_guid).count).to eq(2)
 
     # run the migration
     Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true)
 
     # verify duplicates are removed and constraint is enforced
-    expect(db[:sidecars].where(name: 'app', app_guid: app.guid).count).to eq(1)
+    expect(db[:sidecars].where(name: 'app', app_guid: app_guid).count).to eq(1)
     expect(db.indexes(:sidecars)).to include(:sidecars_app_guid_name_index)
-    expect { db[:sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', app_guid: app.guid) }.to raise_error(Sequel::UniqueConstraintViolation)
+    expect { db[:sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', app_guid: app_guid) }.to raise_error(Sequel::UniqueConstraintViolation)
 
     # running the migration again should not cause any errors
     expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) }.not_to raise_error
@@ -29,6 +31,6 @@
 
     # verify constraint is removed and duplicates can be re-inserted
     expect(db.indexes(:sidecars)).not_to include(:sidecars_app_guid_name_index)
-    expect { db[:sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', app_guid: app.guid) }.not_to raise_error
+    expect { db[:sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', app_guid: app_guid) }.not_to raise_error
   end
 end

spec/migrations/20260323144429_add_unique_constraint_to_revision_process_commands_spec.rb

@@ -6,22 +6,26 @@
     let(:migration_filename) { '20260323144429_add_unique_constraint_to_revision_process_commands.rb' }
   end
 
-  let!(:revision) { VCAP::CloudController::RevisionModel.make }
+  let(:app_guid) { SecureRandom.uuid }
+  let(:revision_guid) { SecureRandom.uuid }
 
   it 'removes duplicates, adds constraint and reverts migration' do
+    db[:apps].insert(guid: app_guid, name: 'test-app')
+    db[:revisions].insert(guid: revision_guid, app_guid: app_guid)
+
     # create duplicate entries
-    db[:revision_process_commands].insert(guid: SecureRandom.uuid, revision_guid: revision.guid, process_type: 'worker')
-    db[:revision_process_commands].insert(guid: SecureRandom.uuid, revision_guid: revision.guid, process_type: 'worker')
-    expect(db[:revision_process_commands].where(revision_guid: revision.guid, process_type: 'worker').count).to eq(2)
+    db[:revision_process_commands].insert(guid: SecureRandom.uuid, revision_guid: revision_guid, process_type: 'worker')
+    db[:revision_process_commands].insert(guid: SecureRandom.uuid, revision_guid: revision_guid, process_type: 'worker')
+    expect(db[:revision_process_commands].where(revision_guid: revision_guid, process_type: 'worker').count).to eq(2)
 
     # run the migration
     Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true)
 
     # verify duplicates are removed and constraint is enforced
-    expect(db[:revision_process_commands].where(revision_guid: revision.guid, process_type: 'worker').count).to eq(1)
+    expect(db[:revision_process_commands].where(revision_guid: revision_guid, process_type: 'worker').count).to eq(1)
     expect(db.indexes(:revision_process_commands)).to include(:revision_process_commands_revision_guid_process_type_index)
     expect do
-      db[:revision_process_commands].insert(guid: SecureRandom.uuid, revision_guid: revision.guid, process_type: 'worker')
+      db[:revision_process_commands].insert(guid: SecureRandom.uuid, revision_guid: revision_guid, process_type: 'worker')
     end.to raise_error(Sequel::UniqueConstraintViolation)
 
     # running the migration again should not cause any errors
@@ -33,7 +37,7 @@
     # verify constraint is removed and duplicates can be re-inserted
     expect(db.indexes(:revision_process_commands)).not_to include(:revision_process_commands_revision_guid_process_type_index)
     expect do
-      db[:revision_process_commands].insert(guid: SecureRandom.uuid, revision_guid: revision.guid, process_type: 'worker')
+      db[:revision_process_commands].insert(guid: SecureRandom.uuid, revision_guid: revision_guid, process_type: 'worker')
     end.not_to raise_error
   end
 end