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

Author: johha

app/actions/service_credential_binding_app_create.rb

@@ -19,37 +19,44 @@ 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,
           type: 'app',
           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

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

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
 

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

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
 

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

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

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,
 

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,

lib/cloud_controller/diego/service_binding_files_builder.rb

@@ -27,14 +27,21 @@ def build
 
       private
 
+      # rubocop:disable Metrics/CyclomaticComplexity
       def build_service_binding_k8s
         return nil unless @service_binding_k8s_enabled
 
         service_binding_files = {}
         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