Multiple service bindings (#4586)

* Introduce max_service_credential_bindings_per_app_service_instance

Config property allows more than one binding per app and service
instance (default remains 1).

* Introduce strategy parameter for binding creation

Add optional strategy parameter (single | multiple) to the binding
creation API. Allow multiple bindings when strategy = "multiple" and
max_service_credential_bindings_per_app_service_instance is greater than
1.

Author: philippthun

app/actions/service_credential_binding_app_create.rb

@@ -48,7 +48,7 @@ def precursor(service_instance, message:, app: nil, volume_mount_services_enable
               num_valid_bindings += 1
             end
 
-            validate_number_of_bindings!(num_valid_bindings)
+            validate_number_of_bindings!(num_valid_bindings, strategy: message.strategy)
             validate_app_guid_name_uniqueness!(app.guid, message.name, service_instance.guid)
 
             new_binding.save_with_attributes_and_new_operation(
@@ -82,8 +82,12 @@ def validate_binding!(binding, desired_binding_name:)
         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
+      def validate_number_of_bindings!(number_of_bindings, strategy:)
+        if strategy == 'multiple'
+          too_many_bindings! if number_of_bindings >= max_bindings_per_app_service_instance
+        elsif number_of_bindings >= 1
+          already_bound!
+        end
       end
 
       def validate_app_guid_name_uniqueness!(target_app_guid, desired_binding_name, target_service_instance_guid)
@@ -109,12 +113,7 @@ def event_repository
       end
 
       def max_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.
+        VCAP::CloudController::Config.config.get(:max_service_credential_bindings_per_app_service_instance)
       end
 
       def app_is_required!

app/messages/service_credential_app_binding_create_message.rb

@@ -1,5 +1,10 @@
 module VCAP::CloudController
   class ServiceCredentialAppBindingCreateMessage < ServiceCredentialBindingCreateMessage
+    validates :strategy, allow_blank: false, allow_nil: true, inclusion: {
+      in: %w[single multiple],
+      message: "must be 'single' or 'multiple'"
+    }
+
     def relationships_message
       @relationships_message ||= Relationships.new(relationships&.deep_symbolize_keys)
     end

app/messages/service_credential_binding_create_message.rb

@@ -1,7 +1,7 @@
 require 'messages/metadata_base_message'
 module VCAP::CloudController
   class ServiceCredentialBindingCreateMessage < MetadataBaseMessage
-    register_allowed_keys %i[type name relationships parameters]
+    register_allowed_keys %i[type strategy name relationships parameters]
     validates_with NoAdditionalKeysValidator, RelationshipValidator
     validates :parameters, hash: true, allow_nil: true
     validates :type, allow_blank: false, inclusion: {

config/cloud_controller.yml

@@ -322,6 +322,7 @@ directories:
 stacks_file: config/stacks.yml
 newrelic_enabled: false
 
+max_service_credential_bindings_per_app_service_instance: 1
 max_annotations_per_resource: 200
 max_labels_per_resource: 50
 max_migration_duration_in_minutes: 45

docs/v3/source/includes/resources/service_credential_bindings/_create.md.erb

@@ -116,6 +116,7 @@ Name | Type | Description
 Name | Type | Description |
 ---- | ---- | ----------- |
 **relationships.app** | [_to-one relationship_](#to-one-relationships) | The app to be bound. Required when type is `app`
+**strategy** | string | Strategy for creating the service credential binding. Valid values are `single` (default) and `multiple` (experimental). Only valid when type is `app`.
 **parameters** | _object_ | A JSON object that is passed to the service broker
 **metadata.labels** | [_label object_](#labels) | Labels applied to the service credential binding
 **metadata.annotations**  | [_annotation object_](#annotations) | Annotations applied to the service credential binding

docs/v3/source/includes/resources/service_credential_bindings/_header.md

@@ -4,9 +4,8 @@ Service credential bindings are used to make the details of the connection to a
 
 Service credential bindings can be of type `app` or `key`.
 
-A service credential binding is of type `app` when it is a binding between a [service instance](#service-instances) and an [application](#apps)
+A service credential binding is of type `app` when it is a binding between a [service instance](#service-instances) and an [application](#apps).
 Not all services support this binding, as some services deliver value to users directly without integration with an application. 
 Field `broker_catalog.features.bindable` from [service plan](#the-service-plan-object) of the service instance can be used to determine if it is bindable.
 
 A service credential binding is of type `key` when it only retrieves the details of the service instance and makes them available to the developer. 
-

lib/cloud_controller/config_schemas/api_schema.rb

@@ -410,6 +410,7 @@ 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

@@ -221,6 +221,7 @@ class WorkerSchema < VCAP::Config
 
           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,
           custom_metric_tag_prefix_list: Array,

spec/unit/actions/service_credential_binding_app_create_spec.rb

@@ -190,13 +190,11 @@ module V3
           end
 
           context 'when multiple bindings are allowed' do
+            let(:message) { VCAP::CloudController::ServiceCredentialAppBindingCreateMessage.new({ name: name, strategy: 'multiple' }) }
             let(:binding_1) { ServiceBinding.make(service_instance:, app:, name:) }
             let(:binding_2) { ServiceBinding.make(service_instance:, app:, name:) }
 
             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'
-
               TestConfig.override(max_service_credential_bindings_per_app_service_instance: 3)
               binding_1.save_with_attributes_and_new_operation({}, { type: 'create', state: 'succeeded' })
               binding_2.save_with_attributes_and_new_operation({}, { type: 'create', state: 'succeeded' })
@@ -279,6 +277,16 @@ module V3
                                    'The app has too many bindings to this service instance (limit: 2). Consider deleting existing/orphaned bindings.')
               end
             end
+
+            context "when the strategy = 'multiple' parameter is omitted" do
+              let(:message) { VCAP::CloudController::ServiceCredentialAppBindingCreateMessage.new({ name: }) }
+
+              it 'raises an error' do
+                expect do
+                  action.precursor(service_instance, app:, message:)
+                end.to raise_error(ServiceCredentialBindingAppCreate::UnprocessableCreate, 'The app is already bound to the service instance')
+              end
+            end
           end
         end
 

spec/unit/lib/cloud_controller/diego/service_binding_files_builder_spec.rb

@@ -18,11 +18,6 @@ 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