Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 8 additions & 9 deletions app/actions/service_credential_binding_app_create.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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)
Expand All @@ -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!
Expand Down
5 changes: 5 additions & 0 deletions app/messages/service_credential_app_binding_create_message.rb
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion app/messages/service_credential_binding_create_message.rb
Original file line number Diff line number Diff line change
@@ -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: {
Expand Down
1 change: 1 addition & 0 deletions config/cloud_controller.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.

1 change: 1 addition & 0 deletions lib/cloud_controller/config_schemas/api_schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,

Expand Down
1 change: 1 addition & 0 deletions lib/cloud_controller/config_schemas/worker_schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
14 changes: 11 additions & 3 deletions spec/unit/actions/service_credential_binding_app_create_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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' })
Expand Down Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ module VCAP::CloudController
let(:params) do
{
type: 'app',
strategy: 'single',
name: 'some-name',
parameters: {
some_param: 'very important',
Expand All @@ -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')
Expand Down Expand Up @@ -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] = ''
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ module VCAP::CloudController
let(:params) do
{
type: 'app',
strategy: 'single',
name: 'some-name',
parameters: {
some_param: 'very important',
Expand All @@ -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' })
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down