diff --git a/app/actions/service_credential_binding_app_create.rb b/app/actions/service_credential_binding_app_create.rb index 574d041d339..b577b93c78c 100644 --- a/app/actions/service_credential_binding_app_create.rb +++ b/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! diff --git a/app/messages/service_credential_app_binding_create_message.rb b/app/messages/service_credential_app_binding_create_message.rb index 5fafe4f4d63..7129b25a452 100644 --- a/app/messages/service_credential_app_binding_create_message.rb +++ b/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 diff --git a/app/messages/service_credential_binding_create_message.rb b/app/messages/service_credential_binding_create_message.rb index c74bbcf52f7..96f97bd1455 100644 --- a/app/messages/service_credential_binding_create_message.rb +++ b/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: { diff --git a/config/cloud_controller.yml b/config/cloud_controller.yml index c4d48202b93..9df9955070c 100644 --- a/config/cloud_controller.yml +++ b/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 diff --git a/docs/v3/source/includes/resources/service_credential_bindings/_create.md.erb b/docs/v3/source/includes/resources/service_credential_bindings/_create.md.erb index 68fc735edd2..84dfb4f3abb 100644 --- a/docs/v3/source/includes/resources/service_credential_bindings/_create.md.erb +++ b/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 diff --git a/docs/v3/source/includes/resources/service_credential_bindings/_header.md b/docs/v3/source/includes/resources/service_credential_bindings/_header.md index f86b24a3745..e41de91076d 100644 --- a/docs/v3/source/includes/resources/service_credential_bindings/_header.md +++ b/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. - diff --git a/lib/cloud_controller/config_schemas/api_schema.rb b/lib/cloud_controller/config_schemas/api_schema.rb index f68b93d34e2..b1979fe5d0f 100644 --- a/lib/cloud_controller/config_schemas/api_schema.rb +++ b/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, diff --git a/lib/cloud_controller/config_schemas/worker_schema.rb b/lib/cloud_controller/config_schemas/worker_schema.rb index f10347ef961..7e47e22cf16 100644 --- a/lib/cloud_controller/config_schemas/worker_schema.rb +++ b/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, 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 25c1700c1a8..ec6749ea182 100644 --- a/spec/unit/actions/service_credential_binding_app_create_spec.rb +++ b/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 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 196fe570567..6d17c1f95ef 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,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 diff --git a/spec/unit/messages/service_credential_app_binding_create_message_spec.rb b/spec/unit/messages/service_credential_app_binding_create_message_spec.rb index a7eeb70ea3d..51fd4d8ad36 100644 --- a/spec/unit/messages/service_credential_app_binding_create_message_spec.rb +++ b/spec/unit/messages/service_credential_app_binding_create_message_spec.rb @@ -8,6 +8,7 @@ module VCAP::CloudController let(:params) do { type: 'app', + strategy: 'single', name: 'some-name', parameters: { some_param: 'very important', @@ -26,6 +27,7 @@ module VCAP::CloudController it 'builds a valid ServiceCredentialBindingCreateMessage' do expect(message).to be_valid expect(message.type).to eq('app') + expect(message.strategy).to eq('single') expect(message.name).to eq('some-name') expect(message.service_instance_guid).to eq('some-instance-guid') expect(message.app_guid).to eq('some-app-guid') @@ -58,6 +60,20 @@ module VCAP::CloudController end end + context 'strategy' do + it 'accepts single and multiple' do + %w[single multiple].each do |strategy| + params[:strategy] = strategy + expect(subject.new(params)).to be_valid + end + end + + it 'is invalid with any other value' do + params[:strategy] = 'test' + expect(subject.new(params)).not_to be_valid + end + end + context 'name' do it 'accepts empty' do params[:name] = '' diff --git a/spec/unit/messages/service_credential_binding_create_message_spec.rb b/spec/unit/messages/service_credential_binding_create_message_spec.rb index 639a88e8f64..15a6db4afbc 100644 --- a/spec/unit/messages/service_credential_binding_create_message_spec.rb +++ b/spec/unit/messages/service_credential_binding_create_message_spec.rb @@ -8,6 +8,7 @@ module VCAP::CloudController let(:params) do { type: 'app', + strategy: 'single', name: 'some-name', parameters: { some_param: 'very important', @@ -29,6 +30,7 @@ module VCAP::CloudController it 'builds a valid ServiceCredentialBindingCreateMessage' do expect(message).to be_valid expect(message.type).to eq('app') + expect(message.strategy).to eq('single') expect(message.name).to eq('some-name') expect(message.service_instance_guid).to eq('some-instance-guid') expect(message.parameters).to eq({ some_param: 'very important', another_param: 'epa' }) 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 dfd53ca417f..677c6e923a5 100644 --- a/spec/unit/presenters/system_environment/system_env_presenter_spec.rb +++ b/spec/unit/presenters/system_environment/system_env_presenter_spec.rb @@ -131,11 +131,6 @@ 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)