wip

Author: philippthun

app/actions/service_credential_binding_app_create.rb

@@ -112,13 +112,7 @@ def event_repository
       end
 
       def max_bindings_per_app_service_instance
-        # TODO: revert !!!
-        2
-        # 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!

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

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

@@ -43,6 +43,8 @@ module V3
 
       describe '#precursor' do
         RSpec.shared_examples 'the credential binding precursor' do
+          before { TestConfig.override(max_service_credential_bindings_per_app_service_instance: 1) }
+
           it 'returns a service credential binding precursor' do
             binding = action.precursor(service_instance, app:, message:)
 
@@ -69,9 +71,6 @@ module V3
           context 'when a binding already exists' do
             let!(:binding) { ServiceBinding.make(service_instance:, app:) }
 
-            # 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
                 expect { action.precursor(service_instance, app:, message:) }.to raise_error(
@@ -161,9 +160,6 @@ module V3
             end
             let(:service_instance2) { ManagedServiceInstance.make(**si_details) }
 
-            # 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
               expect do
@@ -188,9 +184,6 @@ module V3
               )
             end
 
-            # 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
               expect do
@@ -209,8 +202,6 @@ module V3
             let(:binding_1) { ServiceBinding.make(service_instance: service_instance, app: app, name: nil) }
 
             before do
-              # 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
 
@@ -231,9 +222,6 @@ module V3
           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) }
-
             it 'allows only one binding when two creates run in parallel' do
               # This test simulates a race condition for concurrent binding creation using a spy on `service_instance`.
               # We mock that a second binding is created after the first one acquires a lock and expect an `UnprocessableCreate` error.
@@ -251,11 +239,10 @@ module V3
           end
 
           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'
+            let(:binding_1) { ServiceBinding.make(service_instance:, app:, name:) }
 
-              binding_1 = ServiceBinding.make(service_instance:, app:, name:)
+            before do
+              TestConfig.override(max_service_credential_bindings_per_app_service_instance: 3)
               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' })
@@ -276,8 +263,6 @@ module V3
             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' })
 
@@ -304,8 +289,6 @@ module V3
             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' })
 

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

@@ -18,12 +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
-      # TODO: include !!!
-      # 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