From 8a422eda949d672e8816a4b51dcfed635d387ba9 Mon Sep 17 00:00:00 2001
From: johha <johannes.haass@sap.com>
Date: Mon, 18 Aug 2025 15:59:47 +0200
Subject: [PATCH 1/7] Allow multiple service bindings

Allows operators to set the number of allowed service bindings via parameter `max_service_credential_bindings_per_app_service_instance`.
This will be set to 1 in capi-release to not change the current behaviour.
If set to >1 developers can create multiple bindings for the same app and service instance.
This is useful e.g. for rotating the binding credentials without pushing the app again. Restart/restage is sufficient to put the latest binding into the app.

Further details can be found in RFC-0040:
https://github.com/cloudfoundry/community/blob/main/toc/rfc/rfc-0040-service-binding-rotation.md
---
 .../service_credential_binding_app_create.rb  |  86 ++++++++++----
 .../services/service_instances_controller.rb  |   2 +-
 app/models/services/service_binding.rb        |  17 ---
 app/models/services/service_instance.rb       |   2 +-
 .../system_env_presenter.rb                   |   9 +-
 config/cloud_controller.yml                   |   1 +
 ...1071027_allow_multiple_service_bindings.rb |  33 ++++++
 .../config_schemas/api_schema.rb              |   2 +
 .../config_schemas/worker_schema.rb           |   1 +
 .../diego/service_binding_files_builder.rb    |  10 +-
 ...27_allow_multiple_service_bindings_spec.rb |  55 +++++++++
 ...vice_credential_binding_app_create_spec.rb | 109 ++++++++++++++++--
 .../service_binding_files_builder_spec.rb     |  43 ++++++-
 .../system_env_presenter_spec.rb              |  10 ++
 14 files changed, 326 insertions(+), 54 deletions(-)
 create mode 100644 db/migrations/20250731071027_allow_multiple_service_bindings.rb
 create mode 100644 spec/migrations/20250731071027_allow_multiple_service_bindings_spec.rb

diff --git a/app/actions/service_credential_binding_app_create.rb b/app/actions/service_credential_binding_app_create.rb
index 5ac22684606..8e58906065e 100644
--- a/app/actions/service_credential_binding_app_create.rb
+++ b/app/actions/service_credential_binding_app_create.rb
@@ -19,10 +19,8 @@ def initialize(user_audit_info, audit_hash, manifest_triggered: false)
 
       def precursor(service_instance, message:, app: nil, volume_mount_services_enabled: false)
         validate_service_instance!(app, service_instance, volume_mount_services_enabled)
-        binding = ServiceBinding.first(service_instance:, app:)
-        validate_binding!(binding)
 
-        binding_details = {
+        new_binding_details = {
           service_instance: service_instance,
           name: message.name,
           app: app,
@@ -30,26 +28,35 @@ def precursor(service_instance, message:, app: nil, volume_mount_services_enable
           credentials: {}
         }
 
-        ServiceBinding.new.tap do |b|
+        ServiceBinding.new.tap do |new_binding|
           ServiceBinding.db.transaction do
-            if binding
-              binding.destroy
-              VCAP::Services::ServiceBrokers::V2::OrphanMitigator.new.cleanup_failed_bind(binding)
+            validate_app_guid_name_uniqueness!(app.guid, message.name, service_instance.guid) # if max_bindings_per_app_service_instance == 1
+
+            num_valid_bindings = 0
+            existing_bindings = ServiceBinding.where(service_instance:, app:)
+            existing_bindings.each do |binding|
+              binding.lock!
+
+              if binding.create_failed?
+                binding.destroy
+                VCAP::Services::ServiceBrokers::V2::OrphanMitigator.new.cleanup_failed_bind(binding)
+                next
+              end
+
+              validate_binding!(binding, desired_binding_name: message.name)
+              num_valid_bindings += 1
             end
-            b.save_with_attributes_and_new_operation(
-              binding_details,
+
+            validate_number_of_bindings!(num_valid_bindings)
+
+            new_binding.save_with_attributes_and_new_operation(
+              new_binding_details,
               CREATE_INITIAL_OPERATION
             )
-            MetadataUpdate.update(b, message)
+            MetadataUpdate.update(new_binding, message)
           end
         end
       rescue Sequel::ValidationFailed => e
-        if e.message.include?('app_guid and name unique')
-          raise UnprocessableCreate.new("The binding name is invalid. App binding names must be unique. The app already has a binding with name '#{message.name}'.")
-        elsif e.message.include?('service_instance_guid and app_guid unique')
-          raise UnprocessableCreate.new('The app is already bound to the service instance.')
-        end
-
         raise UnprocessableCreate.new(e.full_message)
       end
 
@@ -66,11 +73,25 @@ def validate_service_instance!(app, service_instance, volume_mount_services_enab
         operation_in_progress! if service_instance.operation_in_progress?
       end
 
-      def validate_binding!(binding)
-        return unless binding
+      def validate_binding!(binding, desired_binding_name:)
+        already_bound! if (max_bindings_per_app_service_instance == 1) && (binding.create_succeeded? || binding.create_in_progress?)
+        binding_in_progress!(binding.guid) if binding.create_in_progress?
+        incomplete_deletion! if binding.delete_in_progress? || binding.delete_failed?
+
+        name_cannot_be_changed! if binding.name != desired_binding_name
+      end
+
+      def validate_number_of_bindings!(number_of_bindings)
+        too_many_bindings! if number_of_bindings >= max_bindings_per_app_service_instance
+      end
+
+      def validate_app_guid_name_uniqueness!(target_app_guid, desired_binding_name, target_service_instance_guid)
+        return if desired_binding_name.nil?
+
+        dataset = ServiceBinding.where(app_guid: target_app_guid, name: desired_binding_name)
 
-        already_bound! if binding.create_succeeded? || binding.create_in_progress?
-        incomplete_deletion! if binding.delete_failed? || binding.delete_in_progress?
+        name_uniqueness_violation!(desired_binding_name) if max_bindings_per_app_service_instance == 1 && dataset.first
+        name_uniqueness_violation!(desired_binding_name) if dataset.exclude(service_instance_guid: target_service_instance_guid).first
       end
 
       def permitted_binding_attributes
@@ -87,6 +108,10 @@ def event_repository
         )
       end
 
+      def max_bindings_per_app_service_instance
+        VCAP::CloudController::Config.config.get(:max_service_credential_bindings_per_app_service_instance)
+      end
+
       def app_is_required!
         raise UnprocessableCreate.new('No app was specified')
       end
@@ -95,6 +120,27 @@ def not_supported!
         raise Unimplemented.new('Cannot create credential bindings for managed service instances')
       end
 
+      def binding_in_progress!(binding_guid)
+        raise UnprocessableCreate.new("There is already a binding in progress for this service instance and app (binding guid: #{binding_guid})")
+      end
+
+      def too_many_bindings!
+        raise UnprocessableCreate.new(
+          "The app has too many bindings to this service instance (limit: #{max_bindings_per_app_service_instance}). Consider deleting existing/orphaned bindings."
+        )
+      end
+
+      def name_cannot_be_changed!
+        raise UnprocessableCreate.new('The binding name cannot be changed for the same app and service instance')
+      end
+
+      def name_uniqueness_violation!(name)
+        msg = 'The binding name is invalid. Binding names must be unique for a given service instance and app.'
+        msg += " The app already has a binding with name '#{name}'." unless name.nil? || name.empty?
+
+        raise UnprocessableCreate.new(msg)
+      end
+
       def already_bound!
         raise UnprocessableCreate.new('The app is already bound to the service instance')
       end
diff --git a/app/controllers/services/service_instances_controller.rb b/app/controllers/services/service_instances_controller.rb
index 283bf771543..e2da22ec1a0 100644
--- a/app/controllers/services/service_instances_controller.rb
+++ b/app/controllers/services/service_instances_controller.rb
@@ -415,7 +415,7 @@ def initialize(service_instance)
       end
 
       def serialize(_controller, space, _opts, _orphans=nil)
-        bound_app_count = ServiceBindingListFetcher.fetch_service_instance_bindings_in_space(@service_instance.guid, space.guid).count
+        bound_app_count = ServiceBindingListFetcher.fetch_service_instance_bindings_in_space(@service_instance.guid, space.guid).select(:app_guid).distinct.count
         CloudController::Presenters::V2::ServiceInstanceSharedToPresenter.new.to_hash(space, bound_app_count)
       end
     end
diff --git a/app/models/services/service_binding.rb b/app/models/services/service_binding.rb
index e0d7f38fe32..1124ceac94b 100644
--- a/app/models/services/service_binding.rb
+++ b/app/models/services/service_binding.rb
@@ -35,28 +35,11 @@ class ServiceBinding < Sequel::Model
 
     delegate :service, :service_plan, to: :service_instance
 
-    def around_save
-      yield
-    rescue Sequel::UniqueConstraintViolation => e
-      if e.message.include?('unique_service_binding_app_guid_name')
-        errors.add(%i[app_guid name], :unique)
-        raise validation_failed_error
-      elsif e.message.include?('unique_service_binding_service_instance_guid_app_guid')
-        errors.add(%i[service_instance_guid app_guid], :unique)
-        raise validation_failed_error
-      else
-        raise e
-      end
-    end
-
     def validate
       validates_presence :app
       validates_presence :service_instance
       validates_presence :type
 
-      validates_unique %i[app_guid service_instance_guid], message: Sequel.lit('The app is already bound to the service.')
-      validates_unique %i[app_guid name], message: Sequel.lit("The binding name is invalid. App binding names must be unique. The app already has a binding with name '#{name}'.")
-
       validate_space_match
       validate_cannot_change_binding
 
diff --git a/app/models/services/service_instance.rb b/app/models/services/service_instance.rb
index c3444708161..6bb76d633e2 100644
--- a/app/models/services/service_instance.rb
+++ b/app/models/services/service_instance.rb
@@ -148,7 +148,7 @@ def as_summary_json
       {
         'guid' => guid,
         'name' => name,
-        'bound_app_count' => service_bindings_dataset.count,
+        'bound_app_count' => service_bindings_dataset.select(:app_guid).distinct.count,
         'type' => type
       }
     end
diff --git a/app/presenters/system_environment/system_env_presenter.rb b/app/presenters/system_environment/system_env_presenter.rb
index b3f27e9fd66..749f4071129 100644
--- a/app/presenters/system_environment/system_env_presenter.rb
+++ b/app/presenters/system_environment/system_env_presenter.rb
@@ -22,12 +22,19 @@ def vcap_services
   private
 
   def service_binding_env_variables
+    latest_bindings = @service_bindings.
+                      select(&:create_succeeded?).
+                      group_by(&:service_instance_guid).
+                      values.
+                      map { |list| list.max_by(&:created_at) }
+
     services_hash = {}
-    @service_bindings.select(&:create_succeeded?).each do |service_binding|
+    latest_bindings.each do |service_binding|
       service_name = service_binding_label(service_binding)
       services_hash[service_name.to_sym] ||= []
       services_hash[service_name.to_sym] << service_binding_env_values(service_binding)
     end
+
     services_hash
   end
 
diff --git a/config/cloud_controller.yml b/config/cloud_controller.yml
index d82de9db8e6..ead6eab2a95 100644
--- a/config/cloud_controller.yml
+++ b/config/cloud_controller.yml
@@ -392,6 +392,7 @@ default_app_lifecycle: buildpack
 custom_metric_tag_prefix_list: ["metric.tag.cloudfoundry.org"]
 
 max_manifest_service_binding_poll_duration_in_seconds: 60
+max_service_credential_bindings_per_app_service_instance: 5
 update_metric_tags_on_rename: true
 
 app_log_revision: true
diff --git a/db/migrations/20250731071027_allow_multiple_service_bindings.rb b/db/migrations/20250731071027_allow_multiple_service_bindings.rb
new file mode 100644
index 00000000000..2ed4848bf1f
--- /dev/null
+++ b/db/migrations/20250731071027_allow_multiple_service_bindings.rb
@@ -0,0 +1,33 @@
+Sequel.migration do
+  no_transaction # adding an index concurrently cannot be done within a transaction
+
+  up do
+    transaction do
+      alter_table :service_bindings do
+        drop_constraint :unique_service_binding_service_instance_guid_app_guid if @db.indexes(:service_bindings).include?(:unique_service_binding_service_instance_guid_app_guid)
+
+        drop_constraint :unique_service_binding_app_guid_name if @db.indexes(:service_bindings).include?(:unique_service_binding_app_guid_name)
+      end
+    end
+
+    VCAP::Migration.with_concurrent_timeout(self) do
+      add_index :service_bindings, %i[app_guid service_instance_guid], name: :service_bindings_app_guid_service_instance_guid_index, if_not_exists: true, concurrently: true
+    end
+  end
+
+  down do
+    transaction do
+      alter_table :service_bindings do
+        unless @db.indexes(:service_bindings).include?(:unique_service_binding_service_instance_guid_app_guid)
+          add_unique_constraint %i[service_instance_guid app_guid],
+                                name: :unique_service_binding_service_instance_guid_app_guid
+        end
+        add_unique_constraint %i[app_guid name], name: :unique_service_binding_app_guid_name unless @db.indexes(:service_bindings).include?(:unique_service_binding_app_guid_name)
+      end
+    end
+
+    VCAP::Migration.with_concurrent_timeout(self) do
+      drop_index :service_bindings, %i[app_guid service_instance_guid], name: :service_bindings_app_guid_service_instance_guid_index, if_exists: true, concurrently: true
+    end
+  end
+end
diff --git a/lib/cloud_controller/config_schemas/api_schema.rb b/lib/cloud_controller/config_schemas/api_schema.rb
index 8eb4be21525..6d7c2cacc4d 100644
--- a/lib/cloud_controller/config_schemas/api_schema.rb
+++ b/lib/cloud_controller/config_schemas/api_schema.rb
@@ -409,6 +409,8 @@ class ApiSchema < VCAP::Config
             optional(:query_raise_on_mismatch) => bool
           },
 
+          max_service_credential_bindings_per_app_service_instance: Integer,
+
           max_labels_per_resource: Integer,
           max_annotations_per_resource: Integer,
 
diff --git a/lib/cloud_controller/config_schemas/worker_schema.rb b/lib/cloud_controller/config_schemas/worker_schema.rb
index 86053bbff69..04463d133bb 100644
--- a/lib/cloud_controller/config_schemas/worker_schema.rb
+++ b/lib/cloud_controller/config_schemas/worker_schema.rb
@@ -219,6 +219,7 @@ class WorkerSchema < VCAP::Config
           route_services_enabled: bool,
 
           max_manifest_service_binding_poll_duration_in_seconds: Integer,
+          max_service_credential_bindings_per_app_service_instance: Integer,
 
           max_labels_per_resource: Integer,
           max_annotations_per_resource: Integer,
diff --git a/lib/cloud_controller/diego/service_binding_files_builder.rb b/lib/cloud_controller/diego/service_binding_files_builder.rb
index 20ba186f4ef..fb25a604cd7 100644
--- a/lib/cloud_controller/diego/service_binding_files_builder.rb
+++ b/lib/cloud_controller/diego/service_binding_files_builder.rb
@@ -27,6 +27,7 @@ def build
 
       private
 
+      # rubocop:disable Metrics/CyclomaticComplexity
       def build_service_binding_k8s
         return nil unless @service_binding_k8s_enabled
 
@@ -34,7 +35,13 @@ def build_service_binding_k8s
         names = Set.new # to check for duplicate binding names
         total_bytesize = 0 # to check the total bytesize
 
-        @service_bindings.select(&:create_succeeded?).each do |service_binding|
+        latest_bindings = @service_bindings.
+                          select(&:create_succeeded?).
+                          group_by(&:service_instance_guid).
+                          values.
+                          map { |list| list.max_by(&:created_at) }
+
+        latest_bindings.each do |service_binding|
           sb_hash = ServiceBindingPresenter.new(service_binding, include_instance: true).to_hash
           name = sb_hash[:name]
           raise IncompatibleBindings.new("Invalid binding name: '#{name}'. Name must match #{binding_naming_convention.inspect}") unless valid_name?(name)
@@ -52,6 +59,7 @@ def build_service_binding_k8s
           total_bytesize += add_file(service_binding_files, name, 'type', label)
           total_bytesize += add_file(service_binding_files, name, 'provider', label)
         end
+        # rubocop:enable Metrics/CyclomaticComplexity
 
         raise IncompatibleBindings.new("Bindings exceed the maximum allowed bytesize of #{MAX_ALLOWED_BYTESIZE}: #{total_bytesize}") if total_bytesize > MAX_ALLOWED_BYTESIZE
 
diff --git a/spec/migrations/20250731071027_allow_multiple_service_bindings_spec.rb b/spec/migrations/20250731071027_allow_multiple_service_bindings_spec.rb
new file mode 100644
index 00000000000..288dad45201
--- /dev/null
+++ b/spec/migrations/20250731071027_allow_multiple_service_bindings_spec.rb
@@ -0,0 +1,55 @@
+require 'spec_helper'
+require 'migrations/helpers/migration_shared_context'
+
+RSpec.describe 'migration to allow multiple service bindings', isolation: :truncation, type: :migration do
+  include_context 'migration' do
+    let(:migration_filename) { '20250731071027_allow_multiple_service_bindings.rb' }
+  end
+
+  describe 'service_bindings table' do
+    context 'up migration' do
+      it 'is in the correct state before migration' do
+        expect(db.indexes(:service_bindings)).to include(:unique_service_binding_service_instance_guid_app_guid)
+        expect(db.indexes(:service_bindings)).to include(:unique_service_binding_app_guid_name)
+        expect(db.indexes(:service_bindings)).not_to include(:service_bindings_app_guid_service_instance_guid_index)
+      end
+
+      it 'migrates successfully' do
+        expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) }.not_to raise_error
+        expect(db.indexes(:service_bindings)).not_to include(:unique_service_binding_app_guid_name)
+        expect(db.indexes(:service_bindings)).not_to include(:unique_service_binding_service_instance_guid_app_guid)
+        expect(db.indexes(:service_bindings)).to include(:service_bindings_app_guid_service_instance_guid_index)
+      end
+
+      it 'does not fail if indexes/constraints are already in desired state' do
+        db.alter_table :service_bindings do
+          drop_constraint :unique_service_binding_service_instance_guid_app_guid
+          drop_constraint :unique_service_binding_app_guid_name
+        end
+        db.add_index :service_bindings, %i[app_guid service_instance_guid], name: :service_bindings_app_guid_service_instance_guid_index, if_not_exists: true, concurrently: true
+
+        expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) }.not_to raise_error
+      end
+    end
+
+    context 'down migration' do
+      it 'rolls back successfully' do
+        expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index - 1, allow_missing_migration_files: true) }.not_to raise_error
+        expect(db.indexes(:service_bindings)).to include(:unique_service_binding_service_instance_guid_app_guid)
+        expect(db.indexes(:service_bindings)).to include(:unique_service_binding_app_guid_name)
+        expect(db.indexes(:service_bindings)).not_to include(:service_bindings_app_guid_service_instance_guid_index)
+      end
+
+      it 'does not fail if indexes/constraints are already in desired state' do
+        expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) }.not_to raise_error
+        db.alter_table :service_bindings do
+          add_unique_constraint %i[service_instance_guid app_guid], name: :unique_service_binding_service_instance_guid_app_guid
+          add_unique_constraint %i[app_guid name], name: :unique_service_binding_app_guid_name
+        end
+        db.drop_index :service_bindings, %i[app_guid service_instance_guid], name: :service_bindings_app_guid_service_instance_guid_index, if_exists: true, concurrently: true
+
+        expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index - 1, allow_missing_migration_files: true) }.not_to raise_error
+      end
+    end
+  end
+end
diff --git a/spec/unit/actions/service_credential_binding_app_create_spec.rb b/spec/unit/actions/service_credential_binding_app_create_spec.rb
index 4baf85ba3dc..5b3ecc97eed 100644
--- a/spec/unit/actions/service_credential_binding_app_create_spec.rb
+++ b/spec/unit/actions/service_credential_binding_app_create_spec.rb
@@ -69,6 +69,8 @@ module V3
           context 'when a binding already exists' do
             let!(:binding) { ServiceBinding.make(service_instance:, app:) }
 
+            before { TestConfig.override(max_service_credential_bindings_per_app_service_instance: 1) }
+
             context 'when no last binding operation exists' do
               it 'raises an error' do
                 expect { action.precursor(service_instance, app:, message:) }.to raise_error(
@@ -158,22 +160,19 @@ module V3
             end
             let(:service_instance2) { ManagedServiceInstance.make(**si_details) }
 
+            before { TestConfig.override(max_service_credential_bindings_per_app_service_instance: 1) }
+
             it 'raises an error when the binding name already exists' do
               # First request, should succeed
               expect do
                 action.precursor(service_instance, app:, message:)
               end.not_to raise_error
 
-              # Mock the validation for the second request to simulate the race condition and trigger a
-              # unique constraint violation on app_guid + name
-              allow_any_instance_of(ServiceBinding).to receive(:validate).and_return(true)
-              allow(ServiceBinding).to receive(:first).with(service_instance: service_instance2, app: app).and_return(nil)
-
               # Second request, should fail with correct error
               expect do
                 action.precursor(service_instance2, app:, message:)
               end.to raise_error(ServiceBindingCreate::UnprocessableCreate,
-                                 "The binding name is invalid. App binding names must be unique. The app already has a binding with name 'foo'.")
+                                 "The binding name is invalid. Binding names must be unique for a given service instance and app. The app already has a binding with name 'foo'.")
             end
           end
 
@@ -187,6 +186,8 @@ module V3
               )
             end
 
+            before { TestConfig.override(max_service_credential_bindings_per_app_service_instance: 1) }
+
             it 'raises an error when the app is already bound to the service instance' do
               # First request, should succeed
               expect do
@@ -201,7 +202,21 @@ module V3
               # Second request, should fail with correct error
               expect do
                 action.precursor(service_instance, app: app, message: message2)
-              end.to raise_error(ServiceBindingCreate::UnprocessableCreate, 'The app is already bound to the service instance.')
+              end.to raise_error(ServiceBindingCreate::UnprocessableCreate, 'The app is already bound to the service instance')
+            end
+          end
+
+          context 'app guid name uniqueness validation without name' do
+            let(:name) { nil }
+            let(:binding_1) { ServiceBinding.make(service_instance: service_instance, app: app, name: nil) }
+
+            before do
+              TestConfig.override(max_service_credential_bindings_per_app_service_instance: 1)
+              binding_1.save_with_attributes_and_new_operation({}, { type: 'create', state: 'succeeded' })
+            end
+
+            it 'does not raise an error' do
+              expect { action.precursor(other_service_instance, app:, message:) }.not_to raise_error
             end
           end
 
@@ -213,6 +228,81 @@ module V3
               'The service instance and the app are in different spaces'
             )
           end
+
+          context 'when multiple bindings are allowed' do
+            let!(:binding_1) { ServiceBinding.make(service_instance:, app:, name:) }
+            let!(:binding_2) { ServiceBinding.make(service_instance:, app:, name:) }
+
+            before do
+              binding_1.save_with_attributes_and_new_operation({}, { type: 'create', state: 'succeeded' })
+              binding_2.save_with_attributes_and_new_operation({}, { type: 'create', state: 'succeeded' })
+            end
+
+            it 'creates multiple bindings for the same app and service instance' do
+              expect { action.precursor(service_instance, app:, message:) }.not_to raise_error
+              expect(ServiceBinding.where(app:, service_instance:).count).to eq(3)
+            end
+
+            it 'raises an error if there are too many bindings' do
+              TestConfig.override(max_service_credential_bindings_per_app_service_instance: 2)
+
+              expect { action.precursor(service_instance, app:, message:) }.to raise_error(
+                ServiceCredentialBindingAppCreate::UnprocessableCreate,
+                'The app has too many bindings to this service instance (limit: 2). Consider deleting existing/orphaned bindings.'
+              )
+            end
+
+            it 'raises an error if one of the bindings is in a failed state' do
+              TestConfig.override(max_service_credential_bindings_per_app_service_instance: 2)
+
+              ServiceBinding.make(service_instance:, app:, name:).save_with_attributes_and_new_operation({}, { type: 'create', state: 'succeeded' })
+              ServiceBinding.make(service_instance:, app:, name:).save_with_attributes_and_new_operation({}, { type: 'delete', state: 'failed' })
+
+              expect do
+                action.precursor(service_instance, app:, message:)
+              end.to raise_error(ServiceCredentialBindingAppCreate::UnprocessableCreate, 'The binding is getting deleted or its deletion failed')
+            end
+
+            it 'raises an error if the binding name changes' do
+              new_message = VCAP::CloudController::ServiceCredentialAppBindingCreateMessage.new({ name: 'new_name' })
+              expect { action.precursor(service_instance, app: app, message: new_message) }.to raise_error(
+                ServiceCredentialBindingAppCreate::UnprocessableCreate,
+                'The binding name cannot be changed for the same app and service instance'
+              )
+            end
+
+            it 'raises an error if a previous binding is in progress' do
+              binding_1.save_with_attributes_and_new_operation({}, { type: 'create', state: 'in progress' })
+
+              expect { action.precursor(service_instance, app:, message:) }.to raise_error(
+                ServiceCredentialBindingAppCreate::UnprocessableCreate,
+                "There is already a binding in progress for this service instance and app (binding guid: #{binding_1.guid})"
+              )
+            end
+
+            it 'raises an error if an existing binding has a different name' do
+              TestConfig.override(max_service_credential_bindings_per_app_service_instance: 4)
+
+              ServiceBinding.make(service_instance: service_instance, app: app, name: 'other-name').
+                save_with_attributes_and_new_operation({}, { type: 'create', state: 'succeeded' })
+
+              expect do
+                action.precursor(service_instance, app:, message:) # uses 'foo'
+              end.to raise_error(
+                ServiceCredentialBindingAppCreate::UnprocessableCreate,
+                'The binding name cannot be changed for the same app and service instance'
+              )
+            end
+
+            it 'raises an error if a binding for the same app but a different service instance exists with the same name' do
+              expect do
+                action.precursor(other_service_instance, app:, message:)
+              end.to raise_error(
+                ServiceCredentialBindingAppCreate::UnprocessableCreate,
+                "The binding name is invalid. Binding names must be unique for a given service instance and app. The app already has a binding with name 'foo'."
+              )
+            end
+          end
         end
 
         context 'user-provided service instance' do
@@ -224,7 +314,9 @@ module V3
               syslog_drain_url: 'https://drain.syslog.example.com/runlolarun'
             }
           end
+
           let(:service_instance) { UserProvidedServiceInstance.make(**si_details) }
+          let(:other_service_instance) { UserProvidedServiceInstance.make(**si_details, name: 'other_instance_name') }
 
           it_behaves_like 'the credential binding precursor'
         end
@@ -237,6 +329,7 @@ module V3
           end
 
           let(:service_instance) { ManagedServiceInstance.make(**si_details) }
+          let(:other_service_instance) { ManagedServiceInstance.make(**si_details, name: 'other_service_instance') }
 
           context 'validations' do
             context 'when plan is not bindable' do
@@ -321,6 +414,8 @@ module V3
                 end
               end
 
+              let(:other_service_instance) { ManagedServiceInstance.make(space: space, name: 'other_service_instance') }
+
               it_behaves_like 'the credential binding precursor'
             end
           end
diff --git a/spec/unit/lib/cloud_controller/diego/service_binding_files_builder_spec.rb b/spec/unit/lib/cloud_controller/diego/service_binding_files_builder_spec.rb
index fe716bd526f..72a3810b1fd 100644
--- a/spec/unit/lib/cloud_controller/diego/service_binding_files_builder_spec.rb
+++ b/spec/unit/lib/cloud_controller/diego/service_binding_files_builder_spec.rb
@@ -16,6 +16,29 @@ module VCAP::CloudController::Diego
       expect(service_binding_files.find { |f| f.path == "#{directory}/name" }).to have_attributes(content: name || 'binding-name')
       expect(service_binding_files.find { |f| f.path == "#{directory}/binding-name" }).to have_attributes(content: 'binding-name') if name.nil?
     end
+
+    context 'when there are multiple bindings with the same name for the same app and service instance' do
+      let(:newer_binding_created_at) { Time.now.utc - 2.minutes }
+
+      let!(:newer_binding) do
+        # Create binding eagerly to ensure it exists when ServiceBindingFilesBuilder queries app.service_bindings
+        VCAP::CloudController::ServiceBinding.make(
+          name: binding_name,
+          app: app,
+          credentials: credentials,
+          service_instance: instance,
+          syslog_drain_url: syslog_drain_url,
+          volume_mounts: volume_mounts,
+          created_at: newer_binding_created_at
+        )
+      end
+
+      it 'uses the most recent binding' do
+        expect(service_binding_files.find { |f| f.path == "#{directory}/binding-guid" }).to have_attributes(content: newer_binding.guid)
+        expect(service_binding_files.find { |f| f.path == "#{directory}/name" }).to have_attributes(content: name || 'binding-name')
+        expect(service_binding_files.find { |f| f.path == "#{directory}/binding-name" }).to have_attributes(content: 'binding-name') if name.nil?
+      end
+    end
   end
 
   RSpec.shared_examples 'mapping of instance metadata' do |instance_name|
@@ -67,10 +90,13 @@ module VCAP::CloudController::Diego
   end
 
   RSpec.describe ServiceBindingFilesBuilder do
+    let(:space) { VCAP::CloudController::Space.make }
+    let(:app) { VCAP::CloudController::AppModel.make(space:) }
     let(:service) { VCAP::CloudController::Service.make(label: 'service-name', tags: %w[a-service-tag another-service-tag]) }
     let(:plan) { VCAP::CloudController::ServicePlan.make(name: 'plan-name', service: service) }
-    let(:instance) { VCAP::CloudController::ManagedServiceInstance.make(name: 'instance-name', tags: %w[an-instance-tag another-instance-tag], service_plan: plan) }
+    let(:instance) { VCAP::CloudController::ManagedServiceInstance.make(name: 'instance-name', space: space, tags: %w[an-instance-tag another-instance-tag], service_plan: plan) }
     let(:binding_name) { 'binding-name' }
+    let(:binding_created_at) { Time.now.utc - 3.minutes }
     let(:credentials) do
       {
         string: 'a string',
@@ -84,20 +110,25 @@ module VCAP::CloudController::Diego
     end
     let(:syslog_drain_url) { nil }
     let(:volume_mounts) { nil }
-    let(:binding) do
+    let!(:binding) do
+      # Create binding eagerly to ensure it exists when ServiceBindingFilesBuilder queries app.service_bindings
       VCAP::CloudController::ServiceBinding.make(
         name: binding_name,
+        app: app,
         credentials: credentials,
         service_instance: instance,
         syslog_drain_url: syslog_drain_url,
-        volume_mounts: volume_mounts
+        volume_mounts: volume_mounts,
+        created_at: binding_created_at
       )
     end
-    let(:app) { binding.app }
+
     let(:directory) { 'binding-name' }
 
     describe '#build' do
-      subject(:build) { ServiceBindingFilesBuilder.build(app) }
+      subject(:build) do
+        ServiceBindingFilesBuilder.build(app)
+      end
 
       context 'when service-binding-k8s feature is enabled' do
         before do
@@ -197,7 +228,7 @@ module VCAP::CloudController::Diego
           end
 
           context 'when the instance is user-provided' do
-            let(:instance) { VCAP::CloudController::UserProvidedServiceInstance.make(name: 'upsi', tags: %w[an-upsi-tag another-upsi-tag]) }
+            let(:instance) { VCAP::CloudController::UserProvidedServiceInstance.make(name: 'upsi', space: space, tags: %w[an-upsi-tag another-upsi-tag]) }
 
             include_examples 'mapping of type and provider', 'user-provided'
             include_examples 'mapping of binding metadata'
diff --git a/spec/unit/presenters/system_environment/system_env_presenter_spec.rb b/spec/unit/presenters/system_environment/system_env_presenter_spec.rb
index 7b734293a8e..677c6e923a5 100644
--- a/spec/unit/presenters/system_environment/system_env_presenter_spec.rb
+++ b/spec/unit/presenters/system_environment/system_env_presenter_spec.rb
@@ -130,6 +130,16 @@ module VCAP::CloudController
           end
         end
 
+        context 'when there are multiple service bindings for the same service instance' do
+          it 'includes only the latest binding' do
+            newer_binding = ServiceBinding.make(app: app, service_instance: service_instance, syslog_drain_url: 'logs.go-here.com', created_at: Time.now.utc + 10.seconds)
+
+            bindings = system_env_presenter.system_env[:VCAP_SERVICES][service.label.to_sym]
+            expect(bindings).to have(1).items
+            expect(bindings.first.to_hash[:binding_guid]).to eq(newer_binding.guid)
+          end
+        end
+
         describe 'volume mounts' do
           context 'when the service binding has volume mounts' do
             let!(:service_binding) do

From ee0025f17eaaf2bfa1c12baf90c5b01bd4af0a5f Mon Sep 17 00:00:00 2001
From: johha <johannes.haass@sap.com>
Date: Mon, 18 Aug 2025 16:46:09 +0200
Subject: [PATCH 2/7] Allow only one binding for v2 API

---
 app/actions/v2/services/service_binding_create.rb | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/app/actions/v2/services/service_binding_create.rb b/app/actions/v2/services/service_binding_create.rb
index d2871b58276..cdb06c2807d 100644
--- a/app/actions/v2/services/service_binding_create.rb
+++ b/app/actions/v2/services/service_binding_create.rb
@@ -29,6 +29,7 @@ def create(app, service_instance, message, volume_mount_services_enabled, accept
       raise SpaceMismatch unless bindable_in_space?(service_instance, app.space)
 
       raise_if_instance_locked(service_instance)
+      raise_if_binding_already_exists(service_instance, app)
 
       binding = ServiceBinding.new(
         service_instance: service_instance,
@@ -82,6 +83,12 @@ def bindable_in_space?(service_instance, app_space)
       service_instance.space == app_space || service_instance.shared_spaces.include?(app_space)
     end
 
+    def raise_if_binding_already_exists(service_instance, app)
+      return unless ServiceBinding.where(service_instance:, app:).any?
+
+      raise CloudController::Errors::ApiError.new_from_details('ServiceBindingAppServiceTaken', 'The app is already bound to the service.')
+    end
+
     def logger
       @logger ||= Steno.logger('cc.action.service_binding_create')
     end

From 2210e7bf79f7e979920676a6915a8072aa4f9ab0 Mon Sep 17 00:00:00 2001
From: johha <johannes.haass@sap.com>
Date: Mon, 18 Aug 2025 18:03:52 +0200
Subject: [PATCH 3/7] fix test issues

---
 ...1071027_allow_multiple_service_bindings.rb | 66 ++++++++++++++-----
 ...27_allow_multiple_service_bindings_spec.rb | 20 +++++-
 .../service_credential_bindings_spec.rb       | 26 +++++---
 .../runtime/apps_controller_spec.rb           | 12 ----
 .../models/services/service_binding_spec.rb   | 14 ----
 5 files changed, 83 insertions(+), 55 deletions(-)

diff --git a/db/migrations/20250731071027_allow_multiple_service_bindings.rb b/db/migrations/20250731071027_allow_multiple_service_bindings.rb
index 2ed4848bf1f..438d2e90ab9 100644
--- a/db/migrations/20250731071027_allow_multiple_service_bindings.rb
+++ b/db/migrations/20250731071027_allow_multiple_service_bindings.rb
@@ -2,32 +2,66 @@
   no_transaction # adding an index concurrently cannot be done within a transaction
 
   up do
-    transaction do
-      alter_table :service_bindings do
-        drop_constraint :unique_service_binding_service_instance_guid_app_guid if @db.indexes(:service_bindings).include?(:unique_service_binding_service_instance_guid_app_guid)
-
-        drop_constraint :unique_service_binding_app_guid_name if @db.indexes(:service_bindings).include?(:unique_service_binding_app_guid_name)
+    if database_type == :mysql && server_version < 80_000 # MySQL versions < 8 cannot drop unique constraints directly
+      alter_table(:service_bindings) do
+        # rubocop:disable Sequel/ConcurrentIndex
+        if @db.indexes(:service_bindings).key?(:unique_service_binding_service_instance_guid_app_guid)
+          drop_index %i[service_instance_guid app_guid],
+                     name: :unique_service_binding_service_instance_guid_app_guid
+        end
+        drop_index %i[app_guid name], name: :unique_service_binding_app_guid_name if @db.indexes(:service_bindings).key?(:unique_service_binding_app_guid_name)
+        # rubocop:enable Sequel/ConcurrentIndex
+      end
+    else
+      alter_table(:service_bindings) do
+        drop_constraint(:unique_service_binding_service_instance_guid_app_guid) if @db.indexes(:service_bindings).key?(:unique_service_binding_service_instance_guid_app_guid)
+        drop_constraint(:unique_service_binding_app_guid_name) if @db.indexes(:service_bindings).key?(:unique_service_binding_app_guid_name)
       end
     end
 
-    VCAP::Migration.with_concurrent_timeout(self) do
-      add_index :service_bindings, %i[app_guid service_instance_guid], name: :service_bindings_app_guid_service_instance_guid_index, if_not_exists: true, concurrently: true
+    if database_type == :postgres
+      VCAP::Migration.with_concurrent_timeout(self) do
+        add_index :service_bindings, %i[app_guid service_instance_guid], name: :service_bindings_app_guid_service_instance_guid_index, concurrently: true, if_not_exists: true
+        add_index :service_bindings, %i[app_guid name], name: :service_bindings_app_guid_name_index, concurrently: true, if_not_exists: true
+      end
+    elsif database_type == :mysql
+      alter_table(:service_bindings) do
+        # rubocop:disable Sequel/ConcurrentIndex
+        unless @db.indexes(:service_bindings).key?(:service_bindings_app_guid_service_instance_guid_index)
+          add_index %i[app_guid service_instance_guid],
+                    name: :service_bindings_app_guid_service_instance_guid_index
+        end
+        add_index %i[app_guid name], name: :service_bindings_app_guid_name_index unless @db.indexes(:service_bindings).key?(:service_bindings_app_guid_name_index)
+        # rubocop:enable Sequel/ConcurrentIndex
+      end
     end
   end
 
   down do
-    transaction do
-      alter_table :service_bindings do
-        unless @db.indexes(:service_bindings).include?(:unique_service_binding_service_instance_guid_app_guid)
-          add_unique_constraint %i[service_instance_guid app_guid],
-                                name: :unique_service_binding_service_instance_guid_app_guid
-        end
-        add_unique_constraint %i[app_guid name], name: :unique_service_binding_app_guid_name unless @db.indexes(:service_bindings).include?(:unique_service_binding_app_guid_name)
+    alter_table(:service_bindings) do
+      if @db.indexes(:service_bindings)[:unique_service_binding_service_instance_guid_app_guid].blank?
+        add_unique_constraint %i[service_instance_guid app_guid],
+                              name: :unique_service_binding_service_instance_guid_app_guid
       end
     end
+    alter_table(:service_bindings) do
+      add_unique_constraint %i[app_guid name], name: :unique_service_binding_app_guid_name if @db.indexes(:service_bindings)[:unique_service_binding_app_guid_name].blank?
+    end
 
-    VCAP::Migration.with_concurrent_timeout(self) do
-      drop_index :service_bindings, %i[app_guid service_instance_guid], name: :service_bindings_app_guid_service_instance_guid_index, if_exists: true, concurrently: true
+    if database_type == :postgres
+      VCAP::Migration.with_concurrent_timeout(self) do
+        drop_index :service_bindings, %i[app_guid service_instance_guid], name: :service_bindings_app_guid_service_instance_guid_index, concurrently: true, if_exists: true
+        drop_index :service_bindings, %i[app_guid name], name: :service_bindings_app_guid_name_index, concurrently: true, if_exists: true
+      end
+    elsif database_type == :mysql
+      alter_table(:service_bindings) do
+        # rubocop:disable Sequel/ConcurrentIndex
+        if @db.indexes(:service_bindings).key?(:service_bindings_app_guid_service_instance_guid_index)
+          drop_index %i[app_guid service_instance_guid], name: :service_bindings_app_guid_service_instance_guid_index
+        end
+        drop_index %i[app_guid name], name: :service_bindings_app_guid_name_index if @db.indexes(:service_bindings).key?(:service_bindings_app_guid_name_index)
+        # rubocop:enable Sequel/ConcurrentIndex
+      end
     end
   end
 end
diff --git a/spec/migrations/20250731071027_allow_multiple_service_bindings_spec.rb b/spec/migrations/20250731071027_allow_multiple_service_bindings_spec.rb
index 288dad45201..be2374d40d7 100644
--- a/spec/migrations/20250731071027_allow_multiple_service_bindings_spec.rb
+++ b/spec/migrations/20250731071027_allow_multiple_service_bindings_spec.rb
@@ -12,6 +12,7 @@
         expect(db.indexes(:service_bindings)).to include(:unique_service_binding_service_instance_guid_app_guid)
         expect(db.indexes(:service_bindings)).to include(:unique_service_binding_app_guid_name)
         expect(db.indexes(:service_bindings)).not_to include(:service_bindings_app_guid_service_instance_guid_index)
+        expect(db.indexes(:service_bindings)).not_to include(:service_bindings_app_guid_name_index)
       end
 
       it 'migrates successfully' do
@@ -19,6 +20,7 @@
         expect(db.indexes(:service_bindings)).not_to include(:unique_service_binding_app_guid_name)
         expect(db.indexes(:service_bindings)).not_to include(:unique_service_binding_service_instance_guid_app_guid)
         expect(db.indexes(:service_bindings)).to include(:service_bindings_app_guid_service_instance_guid_index)
+        expect(db.indexes(:service_bindings)).to include(:service_bindings_app_guid_name_index)
       end
 
       it 'does not fail if indexes/constraints are already in desired state' do
@@ -26,8 +28,13 @@
           drop_constraint :unique_service_binding_service_instance_guid_app_guid
           drop_constraint :unique_service_binding_app_guid_name
         end
-        db.add_index :service_bindings, %i[app_guid service_instance_guid], name: :service_bindings_app_guid_service_instance_guid_index, if_not_exists: true, concurrently: true
-
+        if db.database_type == :postgres
+          db.add_index :service_bindings, %i[app_guid service_instance_guid], name: :service_bindings_app_guid_service_instance_guid_index, if_not_exists: true, concurrently: true
+          db.add_index :service_bindings, %i[app_guid name], name: :service_bindings_app_guid_name_index, if_not_exists: true, concurrently: true
+        else
+          db.add_index :service_bindings, %i[app_guid service_instance_guid], name: :service_bindings_app_guid_service_instance_guid_index, if_not_exists: true
+          db.add_index :service_bindings, %i[app_guid name], name: :service_bindings_app_guid_name_index, if_not_exists: true
+        end
         expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) }.not_to raise_error
       end
     end
@@ -38,6 +45,7 @@
         expect(db.indexes(:service_bindings)).to include(:unique_service_binding_service_instance_guid_app_guid)
         expect(db.indexes(:service_bindings)).to include(:unique_service_binding_app_guid_name)
         expect(db.indexes(:service_bindings)).not_to include(:service_bindings_app_guid_service_instance_guid_index)
+        expect(db.indexes(:service_bindings)).not_to include(:service_bindings_app_guid_name_index)
       end
 
       it 'does not fail if indexes/constraints are already in desired state' do
@@ -46,7 +54,13 @@
           add_unique_constraint %i[service_instance_guid app_guid], name: :unique_service_binding_service_instance_guid_app_guid
           add_unique_constraint %i[app_guid name], name: :unique_service_binding_app_guid_name
         end
-        db.drop_index :service_bindings, %i[app_guid service_instance_guid], name: :service_bindings_app_guid_service_instance_guid_index, if_exists: true, concurrently: true
+        if db.database_type == :postgres
+          db.drop_index :service_bindings, %i[app_guid service_instance_guid], name: :service_bindings_app_guid_service_instance_guid_index, if_exists: true, concurrently: true
+          db.drop_index :service_bindings, %i[app_guid name], name: :service_bindings_app_guid_name_index, if_exists: true, concurrently: true
+        else
+          db.drop_index :service_bindings, %i[app_guid service_instance_guid], name: :service_bindings_app_guid_service_instance_guid_index, if_exists: true
+          db.drop_index :service_bindings, %i[app_guid name], name: :service_bindings_app_guid_name_index, if_exists: true
+        end
 
         expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index - 1, allow_missing_migration_files: true) }.not_to raise_error
       end
diff --git a/spec/request/service_credential_bindings_spec.rb b/spec/request/service_credential_bindings_spec.rb
index ae5431b7dce..d4bd4af192c 100644
--- a/spec/request/service_credential_bindings_spec.rb
+++ b/spec/request/service_credential_bindings_spec.rb
@@ -1274,18 +1274,24 @@ def check_filtered_bindings(*bindings)
                                                                }))
         end
 
-        it 'returns 422 when the binding already exists' do
-          api_call.call admin_headers
-          expect(last_response.status).to eq(201).or eq(202)
+        context 'when only one binding per app and service instance is allowed' do
+          before do
+            TestConfig.override(max_service_credential_bindings_per_app_service_instance: 1)
 
-          api_call.call admin_headers
+            it 'returns 422 when the binding already exists' do
+              api_call.call admin_headers
+              expect(last_response.status).to eq(201).or eq(202)
 
-          expect(last_response).to have_status_code(422)
-          expect(parsed_response['errors']).to include(include({
-                                                                 'detail' => include('The app is already bound to the service instance'),
-                                                                 'title' => 'CF-UnprocessableEntity',
-                                                                 'code' => 10_008
-                                                               }))
+              api_call.call admin_headers
+
+              expect(last_response).to have_status_code(422)
+              expect(parsed_response['errors']).to include(include({
+                                                                     'detail' => include('The app is already bound to the service instance'),
+                                                                     'title' => 'CF-UnprocessableEntity',
+                                                                     'code' => 10_008
+                                                                   }))
+            end
+          end
         end
 
         context 'when the service instance does not exist' do
diff --git a/spec/unit/controllers/runtime/apps_controller_spec.rb b/spec/unit/controllers/runtime/apps_controller_spec.rb
index 18ee3d9d58c..46f3bb1200e 100644
--- a/spec/unit/controllers/runtime/apps_controller_spec.rb
+++ b/spec/unit/controllers/runtime/apps_controller_spec.rb
@@ -2496,18 +2496,6 @@ def delete_app
           ServiceBinding.make(service_instance: si2, app: process2.app, name: 'free')
         end
 
-        it 'binding si2 to process1 with a name in use by process1 is not ok' do
-          expect do
-            ServiceBinding.make(service_instance: si2, app: process1.app, name: 'out')
-          end.to raise_error(Sequel::ValidationFailed, /App binding names must be unique\./)
-        end
-
-        it 'binding si1 to process1 with a new name is not ok' do
-          expect do
-            ServiceBinding.make(service_instance: si1, app: process1.app, name: 'gravy')
-          end.to raise_error(Sequel::ValidationFailed, 'The app is already bound to the service.')
-        end
-
         it 'binding si2 to process1 with a name in use by process2 is ok' do
           ServiceBinding.make(service_instance: si2, app: process1.app, name: 'free')
           get "/v2/apps/#{process1.app.guid}/service_bindings?results-per-page=2&page=1&q=name:free"
diff --git a/spec/unit/models/services/service_binding_spec.rb b/spec/unit/models/services/service_binding_spec.rb
index 52959e54fbf..fde838b2224 100644
--- a/spec/unit/models/services/service_binding_spec.rb
+++ b/spec/unit/models/services/service_binding_spec.rb
@@ -25,7 +25,6 @@ module VCAP::CloudController
       it { is_expected.to validate_presence :app }
       it { is_expected.to validate_presence :service_instance }
       it { is_expected.to validate_db_presence :credentials }
-      it { is_expected.to validate_uniqueness %i[app_guid service_instance_guid], message: 'The app is already bound to the service.' }
       it { is_expected.to validate_presence [:type] }
 
       it 'validates max length of name' do
@@ -80,19 +79,6 @@ module VCAP::CloudController
         let(:app) { AppModel.make }
         let(:service_instance) { ServiceInstance.make(space: app.space) }
 
-        context 'and the name is not null' do
-          let(:existing_binding) do
-            ServiceBinding.make(app: app, name: 'some-name', service_instance: service_instance, type: 'app')
-          end
-
-          it 'adds a uniqueness error' do
-            other_service_instance = ServiceInstance.make(space: existing_binding.space)
-            conflict = ServiceBinding.new(app: existing_binding.app, name: existing_binding.name, service_instance: other_service_instance, type: 'app')
-            expect(conflict.valid?).to be(false)
-            expect(conflict.errors.full_messages).to eq(['The binding name is invalid. App binding names must be unique. The app already has a binding with name \'some-name\'.'])
-          end
-        end
-
         context 'and the name is null' do
           let(:existing_binding) do
             ServiceBinding.make(app: app, name: nil, service_instance: service_instance, type: 'app')

From d35a87605124598d6e7b8e09efb84661ae4c2a7a Mon Sep 17 00:00:00 2001
From: johha <johannes.haass@sap.com>
Date: Thu, 21 Aug 2025 12:38:15 +0200
Subject: [PATCH 4/7] Remove db migration and config parameter

To ensure users cannot run create multiple service bindings during the
bosh deployment (when old and new code is running) we need to first
introduce the new validation logic so that we can safely remove the
unique constraints in the next capi release.
---
 .../service_credential_binding_app_create.rb  |  7 +-
 ...1071027_allow_multiple_service_bindings.rb | 67 ------------------
 .../config_schemas/api_schema.rb              |  2 -
 .../config_schemas/worker_schema.rb           |  1 -
 ...27_allow_multiple_service_bindings_spec.rb | 69 -------------------
 .../service_credential_bindings_spec.rb       |  2 -
 ...vice_credential_binding_app_create_spec.rb | 18 +++--
 7 files changed, 17 insertions(+), 149 deletions(-)
 delete mode 100644 db/migrations/20250731071027_allow_multiple_service_bindings.rb
 delete mode 100644 spec/migrations/20250731071027_allow_multiple_service_bindings_spec.rb

diff --git a/app/actions/service_credential_binding_app_create.rb b/app/actions/service_credential_binding_app_create.rb
index 8e58906065e..2789b337845 100644
--- a/app/actions/service_credential_binding_app_create.rb
+++ b/app/actions/service_credential_binding_app_create.rb
@@ -109,7 +109,12 @@ def event_repository
       end
 
       def max_bindings_per_app_service_instance
-        VCAP::CloudController::Config.config.get(:max_service_credential_bindings_per_app_service_instance)
+        1
+        # NOTE: This is hard-coded to 1 for now to preserve the old uniqueness behavior.
+        # TODO: Once the DB migration that drops the unique constraints for service bindings has been released,
+        #       this should be switched to read from config:
+        #       VCAP::CloudController::Config.config.get(:max_service_credential_bindings_per_app_service_instance)
+        # TODO: Also remove skips in related specs.
       end
 
       def app_is_required!
diff --git a/db/migrations/20250731071027_allow_multiple_service_bindings.rb b/db/migrations/20250731071027_allow_multiple_service_bindings.rb
deleted file mode 100644
index 438d2e90ab9..00000000000
--- a/db/migrations/20250731071027_allow_multiple_service_bindings.rb
+++ /dev/null
@@ -1,67 +0,0 @@
-Sequel.migration do
-  no_transaction # adding an index concurrently cannot be done within a transaction
-
-  up do
-    if database_type == :mysql && server_version < 80_000 # MySQL versions < 8 cannot drop unique constraints directly
-      alter_table(:service_bindings) do
-        # rubocop:disable Sequel/ConcurrentIndex
-        if @db.indexes(:service_bindings).key?(:unique_service_binding_service_instance_guid_app_guid)
-          drop_index %i[service_instance_guid app_guid],
-                     name: :unique_service_binding_service_instance_guid_app_guid
-        end
-        drop_index %i[app_guid name], name: :unique_service_binding_app_guid_name if @db.indexes(:service_bindings).key?(:unique_service_binding_app_guid_name)
-        # rubocop:enable Sequel/ConcurrentIndex
-      end
-    else
-      alter_table(:service_bindings) do
-        drop_constraint(:unique_service_binding_service_instance_guid_app_guid) if @db.indexes(:service_bindings).key?(:unique_service_binding_service_instance_guid_app_guid)
-        drop_constraint(:unique_service_binding_app_guid_name) if @db.indexes(:service_bindings).key?(:unique_service_binding_app_guid_name)
-      end
-    end
-
-    if database_type == :postgres
-      VCAP::Migration.with_concurrent_timeout(self) do
-        add_index :service_bindings, %i[app_guid service_instance_guid], name: :service_bindings_app_guid_service_instance_guid_index, concurrently: true, if_not_exists: true
-        add_index :service_bindings, %i[app_guid name], name: :service_bindings_app_guid_name_index, concurrently: true, if_not_exists: true
-      end
-    elsif database_type == :mysql
-      alter_table(:service_bindings) do
-        # rubocop:disable Sequel/ConcurrentIndex
-        unless @db.indexes(:service_bindings).key?(:service_bindings_app_guid_service_instance_guid_index)
-          add_index %i[app_guid service_instance_guid],
-                    name: :service_bindings_app_guid_service_instance_guid_index
-        end
-        add_index %i[app_guid name], name: :service_bindings_app_guid_name_index unless @db.indexes(:service_bindings).key?(:service_bindings_app_guid_name_index)
-        # rubocop:enable Sequel/ConcurrentIndex
-      end
-    end
-  end
-
-  down do
-    alter_table(:service_bindings) do
-      if @db.indexes(:service_bindings)[:unique_service_binding_service_instance_guid_app_guid].blank?
-        add_unique_constraint %i[service_instance_guid app_guid],
-                              name: :unique_service_binding_service_instance_guid_app_guid
-      end
-    end
-    alter_table(:service_bindings) do
-      add_unique_constraint %i[app_guid name], name: :unique_service_binding_app_guid_name if @db.indexes(:service_bindings)[:unique_service_binding_app_guid_name].blank?
-    end
-
-    if database_type == :postgres
-      VCAP::Migration.with_concurrent_timeout(self) do
-        drop_index :service_bindings, %i[app_guid service_instance_guid], name: :service_bindings_app_guid_service_instance_guid_index, concurrently: true, if_exists: true
-        drop_index :service_bindings, %i[app_guid name], name: :service_bindings_app_guid_name_index, concurrently: true, if_exists: true
-      end
-    elsif database_type == :mysql
-      alter_table(:service_bindings) do
-        # rubocop:disable Sequel/ConcurrentIndex
-        if @db.indexes(:service_bindings).key?(:service_bindings_app_guid_service_instance_guid_index)
-          drop_index %i[app_guid service_instance_guid], name: :service_bindings_app_guid_service_instance_guid_index
-        end
-        drop_index %i[app_guid name], name: :service_bindings_app_guid_name_index if @db.indexes(:service_bindings).key?(:service_bindings_app_guid_name_index)
-        # rubocop:enable Sequel/ConcurrentIndex
-      end
-    end
-  end
-end
diff --git a/lib/cloud_controller/config_schemas/api_schema.rb b/lib/cloud_controller/config_schemas/api_schema.rb
index 6d7c2cacc4d..8eb4be21525 100644
--- a/lib/cloud_controller/config_schemas/api_schema.rb
+++ b/lib/cloud_controller/config_schemas/api_schema.rb
@@ -409,8 +409,6 @@ class ApiSchema < VCAP::Config
             optional(:query_raise_on_mismatch) => bool
           },
 
-          max_service_credential_bindings_per_app_service_instance: Integer,
-
           max_labels_per_resource: Integer,
           max_annotations_per_resource: Integer,
 
diff --git a/lib/cloud_controller/config_schemas/worker_schema.rb b/lib/cloud_controller/config_schemas/worker_schema.rb
index 04463d133bb..86053bbff69 100644
--- a/lib/cloud_controller/config_schemas/worker_schema.rb
+++ b/lib/cloud_controller/config_schemas/worker_schema.rb
@@ -219,7 +219,6 @@ class WorkerSchema < VCAP::Config
           route_services_enabled: bool,
 
           max_manifest_service_binding_poll_duration_in_seconds: Integer,
-          max_service_credential_bindings_per_app_service_instance: Integer,
 
           max_labels_per_resource: Integer,
           max_annotations_per_resource: Integer,
diff --git a/spec/migrations/20250731071027_allow_multiple_service_bindings_spec.rb b/spec/migrations/20250731071027_allow_multiple_service_bindings_spec.rb
deleted file mode 100644
index be2374d40d7..00000000000
--- a/spec/migrations/20250731071027_allow_multiple_service_bindings_spec.rb
+++ /dev/null
@@ -1,69 +0,0 @@
-require 'spec_helper'
-require 'migrations/helpers/migration_shared_context'
-
-RSpec.describe 'migration to allow multiple service bindings', isolation: :truncation, type: :migration do
-  include_context 'migration' do
-    let(:migration_filename) { '20250731071027_allow_multiple_service_bindings.rb' }
-  end
-
-  describe 'service_bindings table' do
-    context 'up migration' do
-      it 'is in the correct state before migration' do
-        expect(db.indexes(:service_bindings)).to include(:unique_service_binding_service_instance_guid_app_guid)
-        expect(db.indexes(:service_bindings)).to include(:unique_service_binding_app_guid_name)
-        expect(db.indexes(:service_bindings)).not_to include(:service_bindings_app_guid_service_instance_guid_index)
-        expect(db.indexes(:service_bindings)).not_to include(:service_bindings_app_guid_name_index)
-      end
-
-      it 'migrates successfully' do
-        expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) }.not_to raise_error
-        expect(db.indexes(:service_bindings)).not_to include(:unique_service_binding_app_guid_name)
-        expect(db.indexes(:service_bindings)).not_to include(:unique_service_binding_service_instance_guid_app_guid)
-        expect(db.indexes(:service_bindings)).to include(:service_bindings_app_guid_service_instance_guid_index)
-        expect(db.indexes(:service_bindings)).to include(:service_bindings_app_guid_name_index)
-      end
-
-      it 'does not fail if indexes/constraints are already in desired state' do
-        db.alter_table :service_bindings do
-          drop_constraint :unique_service_binding_service_instance_guid_app_guid
-          drop_constraint :unique_service_binding_app_guid_name
-        end
-        if db.database_type == :postgres
-          db.add_index :service_bindings, %i[app_guid service_instance_guid], name: :service_bindings_app_guid_service_instance_guid_index, if_not_exists: true, concurrently: true
-          db.add_index :service_bindings, %i[app_guid name], name: :service_bindings_app_guid_name_index, if_not_exists: true, concurrently: true
-        else
-          db.add_index :service_bindings, %i[app_guid service_instance_guid], name: :service_bindings_app_guid_service_instance_guid_index, if_not_exists: true
-          db.add_index :service_bindings, %i[app_guid name], name: :service_bindings_app_guid_name_index, if_not_exists: true
-        end
-        expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) }.not_to raise_error
-      end
-    end
-
-    context 'down migration' do
-      it 'rolls back successfully' do
-        expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index - 1, allow_missing_migration_files: true) }.not_to raise_error
-        expect(db.indexes(:service_bindings)).to include(:unique_service_binding_service_instance_guid_app_guid)
-        expect(db.indexes(:service_bindings)).to include(:unique_service_binding_app_guid_name)
-        expect(db.indexes(:service_bindings)).not_to include(:service_bindings_app_guid_service_instance_guid_index)
-        expect(db.indexes(:service_bindings)).not_to include(:service_bindings_app_guid_name_index)
-      end
-
-      it 'does not fail if indexes/constraints are already in desired state' do
-        expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) }.not_to raise_error
-        db.alter_table :service_bindings do
-          add_unique_constraint %i[service_instance_guid app_guid], name: :unique_service_binding_service_instance_guid_app_guid
-          add_unique_constraint %i[app_guid name], name: :unique_service_binding_app_guid_name
-        end
-        if db.database_type == :postgres
-          db.drop_index :service_bindings, %i[app_guid service_instance_guid], name: :service_bindings_app_guid_service_instance_guid_index, if_exists: true, concurrently: true
-          db.drop_index :service_bindings, %i[app_guid name], name: :service_bindings_app_guid_name_index, if_exists: true, concurrently: true
-        else
-          db.drop_index :service_bindings, %i[app_guid service_instance_guid], name: :service_bindings_app_guid_service_instance_guid_index, if_exists: true
-          db.drop_index :service_bindings, %i[app_guid name], name: :service_bindings_app_guid_name_index, if_exists: true
-        end
-
-        expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index - 1, allow_missing_migration_files: true) }.not_to raise_error
-      end
-    end
-  end
-end
diff --git a/spec/request/service_credential_bindings_spec.rb b/spec/request/service_credential_bindings_spec.rb
index d4bd4af192c..c96d0e5cf36 100644
--- a/spec/request/service_credential_bindings_spec.rb
+++ b/spec/request/service_credential_bindings_spec.rb
@@ -1276,8 +1276,6 @@ def check_filtered_bindings(*bindings)
 
         context 'when only one binding per app and service instance is allowed' do
           before do
-            TestConfig.override(max_service_credential_bindings_per_app_service_instance: 1)
-
             it 'returns 422 when the binding already exists' do
               api_call.call admin_headers
               expect(last_response.status).to eq(201).or eq(202)
diff --git a/spec/unit/actions/service_credential_binding_app_create_spec.rb b/spec/unit/actions/service_credential_binding_app_create_spec.rb
index 5b3ecc97eed..55ea6e6f6cc 100644
--- a/spec/unit/actions/service_credential_binding_app_create_spec.rb
+++ b/spec/unit/actions/service_credential_binding_app_create_spec.rb
@@ -69,7 +69,8 @@ module V3
           context 'when a binding already exists' do
             let!(:binding) { ServiceBinding.make(service_instance:, app:) }
 
-            before { TestConfig.override(max_service_credential_bindings_per_app_service_instance: 1) }
+            # TODO: Once the unique constraints to allow multiple bindings are removed, this needs to be set to 1
+            # before { TestConfig.override(max_service_credential_bindings_per_app_service_instance: 1) }
 
             context 'when no last binding operation exists' do
               it 'raises an error' do
@@ -160,7 +161,8 @@ module V3
             end
             let(:service_instance2) { ManagedServiceInstance.make(**si_details) }
 
-            before { TestConfig.override(max_service_credential_bindings_per_app_service_instance: 1) }
+            # TODO: Once the unique constraints to allow multiple bindings are removed, this needs to be set to 1
+            # before { TestConfig.override(max_service_credential_bindings_per_app_service_instance: 1) }
 
             it 'raises an error when the binding name already exists' do
               # First request, should succeed
@@ -186,7 +188,8 @@ module V3
               )
             end
 
-            before { TestConfig.override(max_service_credential_bindings_per_app_service_instance: 1) }
+            # TODO: Once the unique constraints to allow multiple bindings are removed, this needs to be set to 1
+            # before { TestConfig.override(max_service_credential_bindings_per_app_service_instance: 1) }
 
             it 'raises an error when the app is already bound to the service instance' do
               # First request, should succeed
@@ -211,7 +214,8 @@ module V3
             let(:binding_1) { ServiceBinding.make(service_instance: service_instance, app: app, name: nil) }
 
             before do
-              TestConfig.override(max_service_credential_bindings_per_app_service_instance: 1)
+              # TODO: Once the unique constraints to allow multiple bindings are removed, this needs to be set to 1
+              # TestConfig.override(max_service_credential_bindings_per_app_service_instance: 1)
               binding_1.save_with_attributes_and_new_operation({}, { type: 'create', state: 'succeeded' })
             end
 
@@ -230,10 +234,10 @@ module V3
           end
 
           context 'when multiple bindings are allowed' do
-            let!(:binding_1) { ServiceBinding.make(service_instance:, app:, name:) }
-            let!(:binding_2) { ServiceBinding.make(service_instance:, app:, name:) }
-
             before do
+              skip 'this test can be enabled when the service bindings unique constraints are removed and max_bindings_per_app_service_instance can be configured'
+              binding_1 = ServiceBinding.make(service_instance:, app:, name:)
+              binding_2 = ServiceBinding.make(service_instance:, app:, name:)
               binding_1.save_with_attributes_and_new_operation({}, { type: 'create', state: 'succeeded' })
               binding_2.save_with_attributes_and_new_operation({}, { type: 'create', state: 'succeeded' })
             end

From 40fbbe2bd5c6f1187c71c7dceeca5d7c0bf0cdb1 Mon Sep 17 00:00:00 2001
From: johha <johannes.haass@sap.com>
Date: Thu, 21 Aug 2025 14:28:13 +0200
Subject: [PATCH 5/7] skip tests which require unique constraint removal

---
 .../actions/service_credential_binding_app_create_spec.rb    | 2 ++
 .../diego/service_binding_files_builder_spec.rb              | 5 +++++
 .../system_environment/system_env_presenter_spec.rb          | 5 +++++
 3 files changed, 12 insertions(+)

diff --git a/spec/unit/actions/service_credential_binding_app_create_spec.rb b/spec/unit/actions/service_credential_binding_app_create_spec.rb
index 55ea6e6f6cc..900b248a139 100644
--- a/spec/unit/actions/service_credential_binding_app_create_spec.rb
+++ b/spec/unit/actions/service_credential_binding_app_create_spec.rb
@@ -235,7 +235,9 @@ module V3
 
           context 'when multiple bindings are allowed' do
             before do
+              # TODO: Remove skip when the service bindings unique constraints are removed
               skip 'this test can be enabled when the service bindings unique constraints are removed and max_bindings_per_app_service_instance can be configured'
+
               binding_1 = ServiceBinding.make(service_instance:, app:, name:)
               binding_2 = ServiceBinding.make(service_instance:, app:, name:)
               binding_1.save_with_attributes_and_new_operation({}, { type: 'create', state: 'succeeded' })
diff --git a/spec/unit/lib/cloud_controller/diego/service_binding_files_builder_spec.rb b/spec/unit/lib/cloud_controller/diego/service_binding_files_builder_spec.rb
index 72a3810b1fd..edbbe652e39 100644
--- a/spec/unit/lib/cloud_controller/diego/service_binding_files_builder_spec.rb
+++ b/spec/unit/lib/cloud_controller/diego/service_binding_files_builder_spec.rb
@@ -18,6 +18,11 @@ module VCAP::CloudController::Diego
     end
 
     context 'when there are multiple bindings with the same name for the same app and service instance' do
+      before do
+        # TODO: Remove skip when the service bindings unique constraints are removed
+        skip 'this test can be enabled when the service bindings unique constraints are removed and max_bindings_per_app_service_instance can be configured'
+      end
+
       let(:newer_binding_created_at) { Time.now.utc - 2.minutes }
 
       let!(:newer_binding) do
diff --git a/spec/unit/presenters/system_environment/system_env_presenter_spec.rb b/spec/unit/presenters/system_environment/system_env_presenter_spec.rb
index 677c6e923a5..dfd53ca417f 100644
--- a/spec/unit/presenters/system_environment/system_env_presenter_spec.rb
+++ b/spec/unit/presenters/system_environment/system_env_presenter_spec.rb
@@ -131,6 +131,11 @@ module VCAP::CloudController
         end
 
         context 'when there are multiple service bindings for the same service instance' do
+          before do
+            # TODO: Remove skip when the service bindings unique constraints are removed
+            skip 'this test can be enabled when the service bindings unique constraints are removed and max_bindings_per_app_service_instance can be configured'
+          end
+
           it 'includes only the latest binding' do
             newer_binding = ServiceBinding.make(app: app, service_instance: service_instance, syslog_drain_url: 'logs.go-here.com', created_at: Time.now.utc + 10.seconds)
 

From 86adfc7a98706bae38ed8f899fbd33338146bc28 Mon Sep 17 00:00:00 2001
From: johha <johannes.haass@sap.com>
Date: Thu, 21 Aug 2025 16:24:21 +0200
Subject: [PATCH 6/7] remove config parameter

---
 config/cloud_controller.yml | 1 -
 1 file changed, 1 deletion(-)

diff --git a/config/cloud_controller.yml b/config/cloud_controller.yml
index ead6eab2a95..d82de9db8e6 100644
--- a/config/cloud_controller.yml
+++ b/config/cloud_controller.yml
@@ -392,7 +392,6 @@ default_app_lifecycle: buildpack
 custom_metric_tag_prefix_list: ["metric.tag.cloudfoundry.org"]
 
 max_manifest_service_binding_poll_duration_in_seconds: 60
-max_service_credential_bindings_per_app_service_instance: 5
 update_metric_tags_on_rename: true
 
 app_log_revision: true

From 2a1f574b1f20612f397e79cd141262935b078a23 Mon Sep 17 00:00:00 2001
From: johha <johannes.haass@sap.com>
Date: Wed, 3 Sep 2025 17:34:46 +0200
Subject: [PATCH 7/7] Lock service instance to prevent concurrent binding
 creation

---
 .../service_credential_binding_app_create.rb  |  5 +++-
 ...vice_credential_binding_app_create_spec.rb | 28 +++++++++++++++----
 2 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/app/actions/service_credential_binding_app_create.rb b/app/actions/service_credential_binding_app_create.rb
index 2789b337845..d876b0a8fc9 100644
--- a/app/actions/service_credential_binding_app_create.rb
+++ b/app/actions/service_credential_binding_app_create.rb
@@ -30,7 +30,10 @@ def precursor(service_instance, message:, app: nil, volume_mount_services_enable
 
         ServiceBinding.new.tap do |new_binding|
           ServiceBinding.db.transaction do
-            validate_app_guid_name_uniqueness!(app.guid, message.name, service_instance.guid) # if max_bindings_per_app_service_instance == 1
+            # Lock the service instance to prevent multiple bindings being created when not allowed
+            service_instance.lock! if max_bindings_per_app_service_instance == 1
+
+            validate_app_guid_name_uniqueness!(app.guid, message.name, service_instance.guid)
 
             num_valid_bindings = 0
             existing_bindings = ServiceBinding.where(service_instance:, app:)
diff --git a/spec/unit/actions/service_credential_binding_app_create_spec.rb b/spec/unit/actions/service_credential_binding_app_create_spec.rb
index 900b248a139..4d8def3b848 100644
--- a/spec/unit/actions/service_credential_binding_app_create_spec.rb
+++ b/spec/unit/actions/service_credential_binding_app_create_spec.rb
@@ -197,11 +197,6 @@ module V3
                 action.precursor(service_instance, app:, message:)
               end.not_to raise_error
 
-              # Mock the validation for the second request to simulate the race condition and trigger a
-              # unique constraint violation on service_instance_guid + app_guid
-              allow_any_instance_of(ServiceBinding).to receive(:validate).and_return(true)
-              allow(ServiceBinding).to receive(:first).with(service_instance:, app:).and_return(nil)
-
               # Second request, should fail with correct error
               expect do
                 action.precursor(service_instance, app: app, message: message2)
@@ -233,6 +228,29 @@ module V3
             )
           end
 
+          context 'concurrent credential binding creation' do
+            let(:name) { nil }
+
+            # TODO: Once the unique constraints to allow multiple bindings are removed, this needs to be set to 1
+            # before { TestConfig.override(max_service_credential_bindings_per_app_service_instance: 1) }
+
+            def attempt_precursor
+              action.precursor(service_instance, app:, message:)
+              :ok
+            rescue StandardError => e
+              e
+            end
+
+            it 'allows only one binding when two creates run in parallel' do
+              results = [Thread.new { attempt_precursor }, Thread.new { attempt_precursor }].map(&:value)
+
+              expect(ServiceBinding.where(app:, service_instance:).count).to eq(1)
+              expect(results.count(:ok)).to eq(1)
+              expect(results.count { |r| r.is_a?(VCAP::CloudController::V3::ServiceBindingCreate::UnprocessableCreate) }).to eq(1)
+              expect(results.grep(Exception).map(&:message)).to include('The app is already bound to the service instance')
+            end
+          end
+
           context 'when multiple bindings are allowed' do
             before do
               # TODO: Remove skip when the service bindings unique constraints are removed