Fix FK violations in unique constraint migrations for security_groups and buildpacks

db/migrations/20260318083940_add_unique_constraint_to_buildpacks.rb

@@ -6,13 +6,14 @@
       duplicates = self[:buildpacks].select(:name, :stack, :lifecycle).group(:name, :stack, :lifecycle).having { count(1) > 1 }
 
       duplicates.each do |dup|
-        ids_to_remove = self[:buildpacks].
-                        where(name: dup[:name], stack: dup[:stack], lifecycle: dup[:lifecycle]).
-                        select(:id).
-                        order(:id).
-                        offset(1).
-                        map(:id)
+        rows_to_remove = self[:buildpacks].where(name: dup[:name], stack: dup[:stack], lifecycle: dup[:lifecycle]).
+                         select(:id, :guid).order(:id).offset(1).all
 
+        guids_to_remove = rows_to_remove.map { |r| r[:guid] }
+        ids_to_remove   = rows_to_remove.map { |r| r[:id] }
+
+        self[:buildpack_annotations].where(resource_guid: guids_to_remove).delete
+        self[:buildpack_labels].where(resource_guid: guids_to_remove).delete
         self[:buildpacks].where(id: ids_to_remove).delete
       end
     end

db/migrations/20260323130619_add_unique_constraint_to_security_groups.rb

@@ -14,6 +14,8 @@
                         order(:id).
                         offset(1).
                         map(:id)
+        self[:security_groups_spaces].where(security_group_id: ids_to_remove).delete
+        self[:staging_security_groups_spaces].where(staging_security_group_id: ids_to_remove).delete
         self[:security_groups].where(id: ids_to_remove).delete
       end
     end

spec/migrations/20260318083940_add_unique_constraint_to_buildpacks_spec.rb

@@ -11,8 +11,11 @@
       # Drop old unique index so we can insert duplicates
       db.alter_table(:buildpacks) { drop_index %i[name stack], name: :unique_name_and_stack }
 
-      db[:buildpacks].insert(guid: SecureRandom.uuid, name: 'ruby', stack: 'cflinuxfs3', lifecycle: 'buildpack', position: 1)
-      db[:buildpacks].insert(guid: SecureRandom.uuid, name: 'ruby', stack: 'cflinuxfs3', lifecycle: 'buildpack', position: 2)
+      surviving_guid = SecureRandom.uuid
+      duplicate_guid = SecureRandom.uuid
+
+      db[:buildpacks].insert(guid: surviving_guid, name: 'ruby', stack: 'cflinuxfs3', lifecycle: 'buildpack', position: 1)
+      db[:buildpacks].insert(guid: duplicate_guid, name: 'ruby', stack: 'cflinuxfs3', lifecycle: 'buildpack', position: 2)
       db[:buildpacks].insert(guid: SecureRandom.uuid, name: 'ruby', stack: 'cflinuxfs3', lifecycle: 'cnb', position: 3)
       db[:buildpacks].insert(guid: SecureRandom.uuid, name: 'ruby', stack: 'cflinuxfs4', lifecycle: 'buildpack', position: 4)
       db[:buildpacks].insert(guid: SecureRandom.uuid, name: 'go', stack: 'cflinuxfs3', lifecycle: 'buildpack', position: 5)
@@ -22,24 +25,37 @@
       expect(db[:buildpacks].where(name: 'ruby', stack: 'cflinuxfs3', lifecycle: 'buildpack').count).to eq(2)
       expect(db[:buildpacks].where(name: 'go', stack: 'cflinuxfs3', lifecycle: 'buildpack').count).to eq(3)
 
-      # === UP MIGRATION ===
-      expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) }.not_to raise_error
+      # add annotations and labels referencing the duplicate buildpack
+      db[:buildpack_annotations].insert(guid: SecureRandom.uuid, resource_guid: surviving_guid, key_name: 'env', value: 'prod')
+      db[:buildpack_annotations].insert(guid: SecureRandom.uuid, resource_guid: duplicate_guid, key_name: 'env', value: 'staging')
+      db[:buildpack_labels].insert(guid: SecureRandom.uuid, resource_guid: surviving_guid, key_name: 'team', value: 'a')
+      db[:buildpack_labels].insert(guid: SecureRandom.uuid, resource_guid: duplicate_guid, key_name: 'team', value: 'b')
+
+      # run the migration
+      Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true)
 
       # Verify duplicates are removed, keeping one per (name, stack, lifecycle)
       expect(db[:buildpacks].where(name: 'ruby', stack: 'cflinuxfs3', lifecycle: 'buildpack').count).to eq(1)
       expect(db[:buildpacks].where(name: 'ruby', stack: 'cflinuxfs3', lifecycle: 'cnb').count).to eq(1)
       expect(db[:buildpacks].where(name: 'ruby', stack: 'cflinuxfs4', lifecycle: 'buildpack').count).to eq(1)
       expect(db[:buildpacks].where(name: 'go', stack: 'cflinuxfs3', lifecycle: 'buildpack').count).to eq(1)
 
+      # verify annotations and labels for the duplicate are removed, surviving buildpack intact
+      expect(db[:buildpack_annotations].where(resource_guid: duplicate_guid).count).to eq(0)
+      expect(db[:buildpack_labels].where(resource_guid: duplicate_guid).count).to eq(0)
+      expect(db[:buildpack_annotations].where(resource_guid: surviving_guid).count).to eq(1)
+      expect(db[:buildpack_labels].where(resource_guid: surviving_guid).count).to eq(1)
+
       # Verify old index is dropped and new index is added
       expect(db.indexes(:buildpacks)).not_to include(:unique_name_and_stack)
       expect(db.indexes(:buildpacks)).to include(:buildpacks_name_stack_lifecycle_index)
 
       # Test up migration idempotency
       expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) }.not_to raise_error
 
-      # === DOWN MIGRATION ===
       # First remove test data that would conflict with the old (name, stack) unique index
+      db[:buildpack_annotations].delete
+      db[:buildpack_labels].delete
       db[:buildpacks].delete
 
       expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index - 1, allow_missing_migration_files: true) }.not_to raise_error

spec/migrations/20260323130619_add_unique_constraint_to_security_groups_spec.rb

@@ -5,17 +5,32 @@
     let(:migration_filename) { '20260323130619_add_unique_constraint_to_security_groups.rb' }
   end
 
-  it 'remove duplicates, add constraint and revert migration' do
-    # create duplicate entries
-    db[:security_groups].insert(guid: SecureRandom.uuid, name: 'sec1')
-    db[:security_groups].insert(guid: SecureRandom.uuid, name: 'sec1')
+  it 'removes duplicates, adds constraint and reverts migration' do
+    space = VCAP::CloudController::Space.make
+
+    # create duplicate entries with join table references
+    surviving_id = db[:security_groups].insert(guid: SecureRandom.uuid, name: 'sec1')
+    duplicate_id = db[:security_groups].insert(guid: SecureRandom.uuid, name: 'sec1')
     expect(db[:security_groups].where(name: 'sec1').count).to eq(2)
 
+    # add security_groups_spaces and staging_security_groups_spaces referencing the duplicate security_groups
+    db[:security_groups_spaces].insert(security_group_id: surviving_id, space_id: space.id)
+    db[:security_groups_spaces].insert(security_group_id: duplicate_id, space_id: space.id)
+    db[:staging_security_groups_spaces].insert(staging_security_group_id: surviving_id, staging_space_id: space.id)
+    db[:staging_security_groups_spaces].insert(staging_security_group_id: duplicate_id, staging_space_id: space.id)
+
     # 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
+    # verify duplicates and their join table references are removed, surviving group intact
     expect(db[:security_groups].where(name: 'sec1').count).to eq(1)
+    expect(db[:security_groups].where(name: 'sec1').first[:id]).to eq(surviving_id)
+    expect(db[:security_groups_spaces].where(security_group_id: duplicate_id).count).to eq(0)
+    expect(db[:staging_security_groups_spaces].where(staging_security_group_id: duplicate_id).count).to eq(0)
+    expect(db[:security_groups_spaces].where(security_group_id: surviving_id, space_id: space.id).count).to eq(1)
+    expect(db[:staging_security_groups_spaces].where(staging_security_group_id: surviving_id, staging_space_id: space.id).count).to eq(1)
+
+    # verify constraint is enforced
     expect(db.indexes(:security_groups)).to include(:security_groups_name_index)
     expect { db[:security_groups].insert(guid: SecureRandom.uuid, name: 'sec1') }.to raise_error(Sequel::UniqueConstraintViolation)