diff --git a/app/actions/service_credential_binding_app_create.rb b/app/actions/service_credential_binding_app_create.rb index c91a39f042e..35fe7509ced 100644 --- a/app/actions/service_credential_binding_app_create.rb +++ b/app/actions/service_credential_binding_app_create.rb @@ -45,30 +45,6 @@ def precursor(service_instance, app: nil, name: nil, volume_mount_services_enabl private - def complete_binding_and_save(binding, binding_details, last_operation) - binding.save_with_attributes_and_new_operation( - binding_details.symbolize_keys.slice(*PERMITTED_BINDING_ATTRIBUTES), - { - type: 'create', - state: last_operation[:state], - description: last_operation[:description] - } - ) - event_repository.record_create(binding, @user_audit_info, @audit_hash, manifest_triggered: false) - end - - def save_incomplete_binding(binding, broker_operation) - binding.save_with_attributes_and_new_operation( - {}, - { - type: 'create', - state: 'in progress', - broker_provided_operation: broker_operation - } - ) - event_repository.record_start_create(binding, @user_audit_info, @audit_hash, manifest_triggered: false) - end - def validate!(service_instance, app, volume_mount_services_enabled) app_is_required! unless app.present? space_mismatch! unless all_space_guids(service_instance).include? app.space.guid @@ -81,6 +57,10 @@ def validate!(service_instance, app, volume_mount_services_enabled) end end + def permitted_binding_attributes + PERMITTED_BINDING_ATTRIBUTES + end + def all_space_guids(service_instance) (service_instance.shared_spaces + [service_instance.space]).map(&:guid) end diff --git a/app/actions/service_credential_binding_key_create.rb b/app/actions/service_credential_binding_key_create.rb index 178e3c22286..180d5831a89 100644 --- a/app/actions/service_credential_binding_key_create.rb +++ b/app/actions/service_credential_binding_key_create.rb @@ -2,12 +2,20 @@ module VCAP::CloudController module V3 - class ServiceCredentialBindingKeyCreate + class ServiceCredentialBindingKeyCreate < V3::ServiceBindingCreate include ServiceCredentialBindingCreateMixin class UnprocessableCreate < StandardError end + def initialize(user_audit_info, audit_hash) + super() + @user_audit_info = user_audit_info + @audit_hash = audit_hash + end + + PERMITTED_BINDING_ATTRIBUTES = [:credentials].freeze + def precursor(service_instance, name) validate!(service_instance) @@ -17,8 +25,13 @@ def precursor(service_instance, name) credentials: {} } - ServiceKey.db.transaction do - ServiceKey.create(**binding_details) + ServiceKey.new(**binding_details).tap do |b| + b.save_with_new_operation( + { + type: 'create', + state: 'in progress', + } + ) end rescue Sequel::ValidationFailed => e key_validation_error!( @@ -40,6 +53,15 @@ def validate!(service_instance) end end + def permitted_binding_attributes + PERMITTED_BINDING_ATTRIBUTES + end + + def event_repository + @event_repository ||= Repositories::ServiceGenericBindingEventRepository.new( + Repositories::ServiceGenericBindingEventRepository::SERVICE_KEY_CREDENTIAL_BINDING) + end + def key_not_supported_for_user_provided_service! raise UnprocessableCreate.new("Service credential bindings of type 'key' are not supported for user-provided service instances.") end diff --git a/app/actions/service_route_binding_create.rb b/app/actions/service_route_binding_create.rb index 1f17e3f6533..60c2545bdad 100644 --- a/app/actions/service_route_binding_create.rb +++ b/app/actions/service_route_binding_create.rb @@ -11,6 +11,8 @@ def initialize(user_audit_info, audit_hash) @audit_hash = audit_hash end + PERMITTED_BINDING_ATTRIBUTES = [:route_service_url].freeze + def precursor(service_instance, route, message:) validate!(service_instance, route) @@ -51,44 +53,12 @@ def validate!(service_instance, route) operation_in_progress! if service_instance.operation_in_progress? end - def complete_binding_and_save(binding, binding_details, last_operation) - binding.save_with_attributes_and_new_operation( - { - route_service_url: binding_details[:route_service_url] - }, - { - type: 'create', - state: last_operation[:state], - description: last_operation[:description], - } - ) - - binding.notify_diego - - event_repository.record_create( - binding, - @user_audit_info, - @audit_hash, - manifest_triggered: false - ) + def permitted_binding_attributes + PERMITTED_BINDING_ATTRIBUTES end - def save_incomplete_binding(precursor, operation) - precursor.save_with_attributes_and_new_operation( - {}, - { - type: 'create', - state: 'in progress', - broker_provided_operation: operation - } - ) - - event_repository.record_start_create( - precursor, - @user_audit_info, - @audit_hash, - manifest_triggered: false - ) + def post_bind_action(binding) + binding.notify_diego end def operation_in_progress! diff --git a/app/actions/v3/service_binding_create.rb b/app/actions/v3/service_binding_create.rb index f92d6d0d7ce..ca2701a3659 100644 --- a/app/actions/v3/service_binding_create.rb +++ b/app/actions/v3/service_binding_create.rb @@ -64,6 +64,25 @@ def save_failed_state(binding, e) ) end + def complete_binding_and_save(binding, binding_details, last_operation) + binding.save_with_attributes_and_new_operation( + binding_details.symbolize_keys.slice(*permitted_binding_attributes), + { + type: 'create', + state: last_operation[:state], + description: last_operation[:description], + } + ) + + post_bind_action(binding) + + event_repository.record_create( + binding, + @user_audit_info, + @audit_hash + ) + end + def save_last_operation(binding, details) binding.save_with_attributes_and_new_operation( {}, @@ -76,6 +95,20 @@ def save_last_operation(binding, details) ) end + def save_incomplete_binding(binding, broker_operation) + binding.save_with_attributes_and_new_operation( + {}, + { + type: 'create', + state: 'in progress', + broker_provided_operation: broker_operation + } + ) + event_repository.record_start_create(binding, @user_audit_info, @audit_hash) + end + + def post_bind_action(binding); end + def bindings_retrievable?(binding) binding.service_instance.service.bindings_retrievable end diff --git a/app/controllers/v3/service_credential_bindings_controller.rb b/app/controllers/v3/service_credential_bindings_controller.rb index 4677e0a431a..ed10b719368 100644 --- a/app/controllers/v3/service_credential_bindings_controller.rb +++ b/app/controllers/v3/service_credential_bindings_controller.rb @@ -120,13 +120,11 @@ def parameters private def create_key_binding(message, service_instance) - V3::ServiceCredentialBindingKeyCreate.new.precursor( - service_instance, - message.name - ) + action = V3::ServiceCredentialBindingKeyCreate.new(user_audit_info, message.audit_hash) + binding = action.precursor(service_instance, message.name) - head :not_implemented - return + pollable_job_guid = enqueue_bind_job(:key, binding.guid, message) + head :accepted, 'Location' => url_builder.build_url(path: "/v3/jobs/#{pollable_job_guid}") end def build_create_message(params) @@ -161,7 +159,7 @@ def create_app_binding(message, service_instance, app) case service_instance when ManagedServiceInstance - pollable_job_guid = enqueue_bind_job(binding.guid, message) + pollable_job_guid = enqueue_bind_job(:credential, binding.guid, message) head :accepted, 'Location' => url_builder.build_url(path: "/v3/jobs/#{pollable_job_guid}") when UserProvidedServiceInstance action.bind(binding) @@ -169,9 +167,9 @@ def create_app_binding(message, service_instance, app) end end - def enqueue_bind_job(binding_guid, message) + def enqueue_bind_job(type, binding_guid, message) bind_job = VCAP::CloudController::V3::CreateBindingAsyncJob.new( - :credential, + type, binding_guid, user_audit_info: user_audit_info, audit_hash: message.audit_hash, diff --git a/app/jobs/v3/create_service_binding_job_factory.rb b/app/jobs/v3/create_service_binding_job_factory.rb index 035ae984a4c..4539d899511 100644 --- a/app/jobs/v3/create_service_binding_job_factory.rb +++ b/app/jobs/v3/create_service_binding_job_factory.rb @@ -1,5 +1,9 @@ require 'jobs/v3/create_service_route_binding_job_actor' require 'jobs/v3/create_service_credential_binding_job_actor' +require 'jobs/v3/create_service_key_binding_job_actor' +require 'actions/service_credential_binding_app_create' +require 'actions/service_credential_binding_key_create' +require 'actions/service_route_binding_create' module VCAP::CloudController module V3 @@ -13,6 +17,8 @@ def self.for(type) CreateServiceRouteBindingJobActor.new when :credential CreateServiceCredentialBindingJobActor.new + when :key + CreateServiceKeyBindingJobActor.new else raise InvalidType end @@ -24,6 +30,8 @@ def self.action(type, user_audit_info, audit_hash) V3::ServiceRouteBindingCreate.new(user_audit_info, audit_hash) when :credential V3::ServiceCredentialBindingAppCreate.new(user_audit_info, audit_hash) + when :key + V3::ServiceCredentialBindingKeyCreate.new(user_audit_info, audit_hash) else raise InvalidType end diff --git a/app/jobs/v3/create_service_credential_binding_job_actor.rb b/app/jobs/v3/create_service_credential_binding_job_actor.rb index f130fce1d22..268d34a4191 100644 --- a/app/jobs/v3/create_service_credential_binding_job_actor.rb +++ b/app/jobs/v3/create_service_credential_binding_job_actor.rb @@ -1,7 +1,3 @@ -require 'jobs/reoccurring_job' -require 'actions/service_credential_binding_app_create' -require 'cloud_controller/errors/api_error' - module VCAP::CloudController module V3 class CreateServiceCredentialBindingJobActor diff --git a/app/jobs/v3/create_service_key_binding_job_actor.rb b/app/jobs/v3/create_service_key_binding_job_actor.rb new file mode 100644 index 00000000000..b4b70eda5a3 --- /dev/null +++ b/app/jobs/v3/create_service_key_binding_job_actor.rb @@ -0,0 +1,17 @@ +module VCAP::CloudController + module V3 + class CreateServiceKeyBindingJobActor + def display_name + 'service_keys.create' + end + + def resource_type + 'service_credential_binding' + end + + def get_resource(resource_id) + ServiceKey.first(guid: resource_id) + end + end + end +end diff --git a/app/models.rb b/app/models.rb index 9498523d24b..7f7a7cc1fae 100644 --- a/app/models.rb +++ b/app/models.rb @@ -132,6 +132,7 @@ require 'models/services/service_plan_label_model' require 'models/services/service_usage_event' require 'models/services/service_key' +require 'models/services/service_key_operation' require 'models/services/service_credential_binding_view' require 'models/request_count' diff --git a/app/models/runtime/pollable_job_model.rb b/app/models/runtime/pollable_job_model.rb index 6caf5ba597b..a22c670eb73 100644 --- a/app/models/runtime/pollable_job_model.rb +++ b/app/models/runtime/pollable_job_model.rb @@ -27,7 +27,7 @@ def resource_exists? when 'service_route_binding' RouteBinding when 'service_credential_binding' - ServiceBinding + ServiceCredentialBinding::View else Sequel::Model(ActiveSupport::Inflector.pluralize(resource_type).to_sym) end diff --git a/app/models/services/service_binding.rb b/app/models/services/service_binding.rb index 0f7333c9e2c..556c8216eb8 100644 --- a/app/models/services/service_binding.rb +++ b/app/models/services/service_binding.rb @@ -133,15 +133,11 @@ def is_created? end def terminal_state? - !service_binding_operation || (['succeeded', 'failed'].include? service_binding_operation.state) + !service_binding_operation || (%w(succeeded failed).include? service_binding_operation.state) end def operation_in_progress? - if service_binding_operation && service_binding_operation.state == 'in progress' - return true - end - - false + !!service_binding_operation && service_binding_operation.state == 'in progress' end def save_with_attributes_and_new_operation(attributes, operation) diff --git a/app/models/services/service_key.rb b/app/models/services/service_key.rb index d542a3dc68a..a0502fa112b 100644 --- a/app/models/services/service_key.rb +++ b/app/models/services/service_key.rb @@ -4,6 +4,8 @@ class InvalidAppAndServiceRelation < StandardError; end many_to_one :service_instance + one_to_one :service_key_operation + export_attributes :name, :service_instance_guid, :credentials import_attributes :name, :service_instance_guid, :credentials @@ -79,8 +81,44 @@ def logger @logger ||= Steno.logger('cc.models.service_key') end + def last_operation + service_key_operation + end + + def terminal_state? + !service_key_operation || (%w(succeeded failed).include? service_key_operation.state) + end + + # TODO: can these methods be a mixin somehow def operation_in_progress? - false + !!service_key_operation && service_key_operation.state == 'in progress' + end + + def required_parameters + nil + end + + def save_with_attributes_and_new_operation(attributes, operation) + save_with_new_operation(operation, attributes: attributes) + self + end + + def save_with_new_operation(last_operation, attributes: {}) + ServiceKey.db.transaction do + self.lock! + set(attributes.except(:parameters, :route_services_url, :endpoints)) + save_changes + + if self.last_operation + self.last_operation.destroy + end + + # it is important to create the service key operation with the service key + # instead of doing self.service_key_operation = x + # because mysql will deadlock when requests happen concurrently otherwise. + ServiceKeyOperation.create(last_operation.merge(service_key_id: self.id)) + self.service_key_operation(reload: true) + end end end end diff --git a/app/models/services/service_key_operation.rb b/app/models/services/service_key_operation.rb new file mode 100644 index 00000000000..516949c2271 --- /dev/null +++ b/app/models/services/service_key_operation.rb @@ -0,0 +1,10 @@ +module VCAP::CloudController + class ServiceKeyOperation < Sequel::Model + export_attributes :state, :description, :type, :updated_at, :created_at + + def update_attributes(attrs) + self.set attrs + self.save + end + end +end diff --git a/db/migrations/20201124153320_create_service_key_operations.rb b/db/migrations/20201124153320_create_service_key_operations.rb new file mode 100644 index 00000000000..23016cc59e7 --- /dev/null +++ b/db/migrations/20201124153320_create_service_key_operations.rb @@ -0,0 +1,17 @@ +Sequel.migration do + change do + create_table :service_key_operations do + VCAP::Migration.timestamps(self, :service_key_operations) + + Integer :service_key_id + String :state, size: 255, null: false + String :type, size: 255, null: false + String :description, size: 10000 + String :broker_provided_operation, size: 10000 + + index :service_key_id, name: :svc_key_id_index, unique: true + primary_key :id, name: :id + foreign_key [:service_key_id], :service_keys, name: :fk_svc_key_op_svc_key_id, on_delete: :cascade + end + end +end diff --git a/lib/services/service_brokers/v2/client.rb b/lib/services/service_brokers/v2/client.rb index 6c53ed8078f..8e8b6f0865e 100644 --- a/lib/services/service_brokers/v2/client.rb +++ b/lib/services/service_brokers/v2/client.rb @@ -94,21 +94,22 @@ def create_service_key(key, arbitrary_parameters: {}) end def bind(binding, arbitrary_parameters: {}, accepts_incomplete: false) - path = service_binding_resource_path(binding.guid, binding.service_instance.guid, accepts_incomplete: accepts_incomplete) - body = { + path = service_binding_resource_path(binding.guid, binding.service_instance.guid, accepts_incomplete: accepts_incomplete) + key_required_parameters = { credential_client_id: @config.get(:cc_service_key_client_name) } if binding.is_a?(VCAP::CloudController::ServiceKey) + body = { service_id: binding.service.broker_provided_id, plan_id: binding.service_plan.broker_provided_id, app_guid: binding.try(:app_guid), - bind_resource: binding.required_parameters, + bind_resource: binding.required_parameters || key_required_parameters, context: context_hash(binding.service_instance) } - body = body.reject { |_, v| v.nil? } + body = body.reject { |_, v| v.nil? } body[:parameters] = arbitrary_parameters if arbitrary_parameters.present? begin response = @http_client.put(path, body) rescue Errors::HttpClientTimeout => e - @orphan_mitigator.cleanup_failed_bind(@attrs, binding) + cleanup_failed_bind(@attrs, binding) raise e end @@ -140,12 +141,20 @@ def bind(binding, arbitrary_parameters: {}, accepts_incomplete: false) Errors::ServiceBrokerInvalidSyslogDrainUrl, Errors::ServiceBrokerResponseMalformed => e unless e.instance_of?(Errors::ServiceBrokerResponseMalformed) && e.status == 200 - @orphan_mitigator.cleanup_failed_bind(@attrs, binding) + cleanup_failed_bind(@attrs, binding) end raise e end + def cleanup_failed_bind(attrs, binding) + if binding.is_a?(VCAP::CloudController::ServiceKey) + @orphan_mitigator.cleanup_failed_key(attrs, binding) + else + @orphan_mitigator.cleanup_failed_bind(attrs, binding) + end + end + def unbind(service_binding, user_guid=nil, accepts_incomplete=false) path = service_binding_resource_path(service_binding.guid, service_binding.service_instance.guid, accepts_incomplete: accepts_incomplete) diff --git a/spec/request/service_credential_bindings_spec.rb b/spec/request/service_credential_bindings_spec.rb index 08d52ac3b9f..9a25ce3b180 100644 --- a/spec/request/service_credential_bindings_spec.rb +++ b/spec/request/service_credential_bindings_spec.rb @@ -841,10 +841,11 @@ def check_filtered_bindings(*bindings) context 'creating a credential binding to an app' do let(:app_to_bind_to) { VCAP::CloudController::AppModel.make(space: space) } let(:app_guid) { app_to_bind_to.guid } + let(:binding_name) { 'some-name' } let(:create_body) { { type: 'app', - name: 'some-name', + name: binding_name, relationships: { service_instance: { data: { guid: service_instance_guid } }, app: { data: { guid: app_guid } } @@ -1153,333 +1154,19 @@ def check_filtered_bindings(*bindings) end describe 'a successful creation' do - it 'creates a credential binding in the database' do - api_call.call(admin_headers) - - expect(binding.service_instance).to eq(service_instance) - expect(binding.app).to eq(app_to_bind_to) - expect(binding.last_operation.state).to eq('in progress') - expect(binding.last_operation.type).to eq('create') - end + let(:syslog_drain_url) { 'http://syslog.example.com/wow' } + let(:route_service_url) { 'http://route.example.com/wow' } - it 'responds with a job resource' do - api_call.call(admin_headers) - - expect(last_response).to have_status_code(202) - expect(last_response.headers['Location']).to end_with("/v3/jobs/#{job.guid}") - - expect(job.state).to eq(VCAP::CloudController::PollableJobModel::PROCESSING_STATE) - expect(job.operation).to eq('service_bindings.create') - expect(job.resource_guid).to eq(binding.guid) - expect(job.resource_type).to eq('service_credential_binding') - - get "/v3/jobs/#{job.guid}", nil, admin_headers - expect(last_response).to have_status_code(200) - expect(parsed_response['guid']).to eq(job.guid) - binding_link = parsed_response.dig('links', 'service_credential_binding', 'href') - expect(binding_link).to end_with("/v3/service_credential_bindings/#{binding.guid}") - end - end - - describe 'the pollable job' do - let(:credentials) { { 'password' => 'special sauce' } } - let(:broker_base_url) { service_instance.service_broker.broker_url } - let(:broker_bind_url) { "#{broker_base_url}/v2/service_instances/#{service_instance.guid}/service_bindings/#{binding.guid}" } - let(:broker_status_code) { 201 } - let(:broker_response) { { credentials: credentials } } - let(:client_body) do - { - context: { - platform: 'cloudfoundry', - organization_guid: org.guid, - organization_name: org.name, - space_guid: space.guid, - space_name: space.name, - }, - app_guid: app_to_bind_to.guid, - service_id: service_instance.service_plan.service.unique_id, - plan_id: service_instance.service_plan.unique_id, - bind_resource: { - app_guid: app_to_bind_to.guid, - space_guid: service_instance.space.guid - }, - } - end - - before do - api_call.call(admin_headers) - expect(last_response).to have_status_code(202) - - stub_request(:put, broker_bind_url). - with(query: { accepts_incomplete: true }). - to_return(status: broker_status_code, body: broker_response.to_json, headers: {}) - end - - it 'sends a bind request with the right arguments to the service broker' do - execute_all_jobs(expected_successes: 1, expected_failures: 0) - - expect( - a_request(:put, broker_bind_url). - with( - body: client_body, - query: { accepts_incomplete: true } - ) - ).to have_been_made.once - end - - context 'parameters are specified' do - let(:request_extra) { { parameters: { foo: 'bar' } } } - - it 'sends the parameters to the broker' do - execute_all_jobs(expected_successes: 1, expected_failures: 0) - - expect( - a_request(:put, broker_bind_url). - with( - body: client_body.deep_merge(request_extra), - query: { accepts_incomplete: true } - ) - ).to have_been_made.once - end - end - - context 'when the bind completes synchronously' do - it 'updates the the binding' do - execute_all_jobs(expected_successes: 1, expected_failures: 0) - - binding.reload - expect(binding.credentials).to eq(credentials) - expect(binding.last_operation.type).to eq('create') - expect(binding.last_operation.state).to eq('succeeded') - end - - it 'completes the job' do - execute_all_jobs(expected_successes: 1, expected_failures: 0) - - expect(job.state).to eq(VCAP::CloudController::PollableJobModel::COMPLETE_STATE) - end - - it 'logs an audit event' do - execute_all_jobs(expected_successes: 1, expected_failures: 0) - - event = VCAP::CloudController::Event.find(type: 'audit.service_binding.create') - expect(event).to be - expect(event.actee).to eq(binding.guid) - expect(event.actee_name).to eq(binding.name) - expect(event.data).to include({ - 'request' => create_body.with_indifferent_access - }) - end - end - - context 'when the broker fails to bind' do - let(:broker_status_code) { 422 } - let(:broker_response) { { error: 'RequiresApp' } } - - it 'updates the the binding with a failure' do - execute_all_jobs(expected_successes: 0, expected_failures: 1) - - binding.reload - expect(binding.last_operation.type).to eq('create') - expect(binding.last_operation.state).to eq('failed') - end - - it 'fails the job' do - execute_all_jobs(expected_successes: 0, expected_failures: 1) - - expect(job.state).to eq(VCAP::CloudController::PollableJobModel::FAILED_STATE) - end - end - - context 'when the binding completes asynchronously' do - let(:broker_status_code) { 202 } - let(:operation) { Sham.guid } - let(:broker_response) { { operation: operation } } - let(:broker_binding_last_operation_url) { "#{broker_base_url}/v2/service_instances/#{service_instance.guid}/service_bindings/#{binding.guid}/last_operation" } - let(:last_operation_status_code) { 200 } - let(:description) { Sham.description } - let(:state) { 'in progress' } - let(:last_operation_body) do - { - description: description, - state: state, - } - end - - before do - stub_request(:get, broker_binding_last_operation_url). - with(query: hash_including({ - operation: operation - })). - to_return(status: last_operation_status_code, body: last_operation_body.to_json, headers: {}) - end - - it 'polls the last operation endpoint' do - execute_all_jobs(expected_successes: 1, expected_failures: 0) - - expect( - a_request(:get, broker_binding_last_operation_url). - with(query: { - operation: operation, - service_id: service_instance.service_plan.service.unique_id, - plan_id: service_instance.service_plan.unique_id, - }) - ).to have_been_made.once - end - - it 'updates the binding and job' do - execute_all_jobs(expected_successes: 1, expected_failures: 0) - - expect(binding.last_operation.type).to eq('create') - expect(binding.last_operation.state).to eq(state) - expect(binding.last_operation.description).to eq(description) - - expect(job.state).to eq(VCAP::CloudController::PollableJobModel::POLLING_STATE) - end - - it 'logs an audit event' do - execute_all_jobs(expected_successes: 1, expected_failures: 0) - - event = VCAP::CloudController::Event.find(type: 'audit.service_binding.start_create') - expect(event).to be - expect(event.actee).to eq(binding.guid) - expect(event.data).to include({ - 'request' => create_body.with_indifferent_access - }) - end - - it 'enqueues the next fetch last operation job' do - execute_all_jobs(expected_successes: 1, expected_failures: 0) - expect(Delayed::Job.count).to eq(1) - end - - it 'keeps track of the broker operation' do - execute_all_jobs(expected_successes: 1, expected_failures: 0) - expect(Delayed::Job.count).to eq(1) - - Timecop.travel(Time.now + 1.minute) - execute_all_jobs(expected_successes: 1, expected_failures: 0) - - expect( - a_request(:get, broker_binding_last_operation_url). - with(query: { - operation: operation, - service_id: service_instance.service_plan.service.unique_id, - plan_id: service_instance.service_plan.unique_id, - }) - ).to have_been_made.twice - end - - context 'last operation response is 200 OK and indicates success' do - let(:state) { 'succeeded' } - let(:fetch_binding_status_code) { 200 } - let(:syslog_drain_url) { 'http://syslog.example.com/wow' } - let(:route_service_url) { 'http://route.example.com/wow' } - let(:credentials) { { password: 'foo' } } - let(:parameters) { { foo: 'bar', another_foo: 'another_bar' } } - - let(:fetch_binding_body) do - { - syslog_drain_url: syslog_drain_url, - credentials: credentials, - parameters: parameters, - service_id: 'extra-field-service_id-should-ignore', - name: 'extra-field-name-should-not-update', - } - end - - before do - stub_request(:get, broker_bind_url). - to_return(status: fetch_binding_status_code, body: fetch_binding_body.to_json, headers: {}) - end - - it 'fetches the binding' do - execute_all_jobs(expected_successes: 1, expected_failures: 0) - - expect( - a_request(:get, broker_bind_url) - ).to have_been_made.once - end - - it 'updates the binding and job' do - execute_all_jobs(expected_successes: 1, expected_failures: 0) - - expect(binding.last_operation.type).to eq('create') - expect(binding.last_operation.state).to eq(state) - expect(binding.last_operation.description).to eq(description) - - expect(job.state).to eq(VCAP::CloudController::PollableJobModel::COMPLETE_STATE) - end - - it 'updates the binding details with the fetch binding response ignoring extra fields' do - execute_all_jobs(expected_successes: 1, expected_failures: 0) - - expect(binding.reload.syslog_drain_url).to eq(syslog_drain_url) - expect(binding.credentials).to eq(credentials.with_indifferent_access) - expect(binding.name).to eq('some-name') - end - - context 'fetching binding fails ' do - let(:fetch_binding_status_code) { 404 } - let(:fetch_binding_body) {} - - it 'fails the job' do - execute_all_jobs(expected_successes: 0, expected_failures: 1) - - expect(binding.last_operation.type).to eq('create') - expect(binding.last_operation.state).to eq('failed') - expect(binding.last_operation.description).to include('The service broker rejected the request. Status Code: 404 Not Found') - - expect(job.state).to eq(VCAP::CloudController::PollableJobModel::FAILED_STATE) - expect(job.cf_api_error).not_to be_nil - error = YAML.safe_load(job.cf_api_error) - expect(error['errors'].first).to include({ - 'code' => 10009, - 'title' => 'CF-UnableToPerform', - 'detail' => 'bind could not be completed: The service broker rejected the request. Status Code: 404 Not Found, Body: null', - }) - end - end - end - - it_behaves_like 'binding last operation response handling', 'create' - - context 'binding not retrievable' do - let(:offering) { VCAP::CloudController::Service.make(bindings_retrievable: false) } - - it 'fails the job with an appropriate error' do - execute_all_jobs(expected_successes: 0, expected_failures: 1) - - expect(binding.last_operation.type).to eq('create') - expect(binding.last_operation.state).to eq('failed') - expect(binding.last_operation.description).to eq('The broker responded asynchronously but does not support fetching binding data') - - expect(job.state).to eq(VCAP::CloudController::PollableJobModel::FAILED_STATE) - expect(job.cf_api_error).not_to be_nil - error = YAML.safe_load(job.cf_api_error) - expect(error['errors'].first).to include({ - 'code' => 90001, - 'title' => 'CF-ServiceBindingInvalid', - 'detail' => 'The service binding is invalid: The broker responded asynchronously but does not support fetching binding data', - }) - end - end - end - - context 'orphan mitigation' do - it_behaves_like 'create binding orphan mitigation' do - let(:bind_url) { broker_bind_url } - let(:plan_id) { plan.unique_id } - let(:offering_id) { offering.unique_id } - end - end + it_behaves_like 'service credential binding create endpoint', VCAP::CloudController::ServiceBinding, true, 'service_binding', 'service_bindings' end end end context 'creating a credential binding as a key' do - let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space: space, **service_instance_details) } - let(:binding_name) { 'some-key-name' } + let(:offering) { VCAP::CloudController::Service.make(bindings_retrievable: true) } + let(:plan) { VCAP::CloudController::ServicePlan.make(service: offering) } + let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space: space, service_plan: plan) } + let(:binding_name) { Sham.name } let(:create_body) { { type: 'key', @@ -1495,8 +1182,8 @@ def check_filtered_bindings(*bindings) it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS do let(:expected_codes_and_responses) do Hash.new(code: 403).tap do |h| - h['space_developer'] = { code: 501 } - h['admin'] = { code: 501 } + h['space_developer'] = { code: 202 } + h['admin'] = { code: 202 } h['org_billing_manager'] = { code: 422 } h['org_auditor'] = { code: 422 } h['no_role'] = { code: 422 } @@ -1508,7 +1195,9 @@ def check_filtered_bindings(*bindings) context 'users in the space where the SI has been shared to' do let(:orginal_org) { VCAP::CloudController::Organization.make } let(:original_space) { VCAP::CloudController::Space.make(organization: orginal_org) } - let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space: original_space) } + let(:offering) { VCAP::CloudController::Service.make(bindings_retrievable: true) } + let(:plan) { VCAP::CloudController::ServicePlan.make(service: offering) } + let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space: original_space, service_plan: plan) } before do service_instance.add_shared_space(space) @@ -1517,7 +1206,7 @@ def check_filtered_bindings(*bindings) it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS do let(:expected_codes_and_responses) do Hash.new(code: 403).tap do |h| - h['admin'] = { code: 501 } + h['admin'] = { code: 202 } h['org_billing_manager'] = { code: 422 } h['org_auditor'] = { code: 422 } h['no_role'] = { code: 422 } @@ -1708,9 +1397,8 @@ def check_filtered_bindings(*bindings) end end - it 'should return 501' do - api_call.call admin_headers - expect(last_response).to have_status_code(501) + context 'request is valid' do + it_behaves_like 'service credential binding create endpoint', VCAP::CloudController::ServiceKey, false, 'service_key', 'service_keys' end end end diff --git a/spec/support/fakes/blueprints.rb b/spec/support/fakes/blueprints.rb index 17855924560..fbe20ece80b 100644 --- a/spec/support/fakes/blueprints.rb +++ b/spec/support/fakes/blueprints.rb @@ -380,6 +380,13 @@ module VCAP::CloudController updated_at { Time.now.utc } end + ServiceKeyOperation.blueprint do + type { 'create' } + state { 'succeeded' } + description { 'description goes here' } + updated_at { Time.now.utc } + end + Stack.blueprint do name { Sham.name } description { Sham.description } diff --git a/spec/support/shared_examples/models/operations.rb b/spec/support/shared_examples/models/operations.rb new file mode 100644 index 00000000000..add7751e1c0 --- /dev/null +++ b/spec/support/shared_examples/models/operations.rb @@ -0,0 +1,43 @@ +RSpec.shared_examples 'operation' do + let(:updated_at_time) { Time.utc(2018, 5, 2, 3, 30, 0) } + let(:created_at_time) { Time.utc(2018, 5, 2, 3, 30, 0) } + let(:operation_attributes) do + { + state: 'in progress', + description: '10%', + type: 'create', + } + end + + let(:operation) { described_class.make(operation_attributes) } + + before do + operation.this.update(updated_at: updated_at_time, created_at: created_at_time) + operation.reload + end + + describe '#to_hash' do + it 'includes the state, type, description, created_at and updated_at' do + expect(operation.to_hash).to include({ + 'state' => 'in progress', + 'type' => 'create', + 'description' => '10%', + }) + + expect(operation.to_hash['updated_at']).to eq(updated_at_time) + expect(operation.to_hash['created_at']).to eq(created_at_time) + end + end + + describe 'updating attributes' do + it 'updates the attributes of the operation' do + new_attributes = { + state: 'finished', + description: '100%' + } + operation.update_attributes(new_attributes) + expect(operation.state).to eq 'finished' + expect(operation.description).to eq '100%' + end + end +end diff --git a/spec/support/shared_examples/request/bindings_create.rb b/spec/support/shared_examples/request/bindings_create.rb new file mode 100644 index 00000000000..1590cc436ad --- /dev/null +++ b/spec/support/shared_examples/request/bindings_create.rb @@ -0,0 +1,363 @@ +RSpec.shared_examples 'service credential binding create endpoint' do |klass, check_app, audit_event_type, display_name| + describe 'managed instance' do + let(:api_call) { ->(user_headers) { post '/v3/service_credential_bindings', create_body.to_json, user_headers } } + let(:job) { VCAP::CloudController::PollableJobModel.last } + let(:binding) { klass.last } + + describe 'a successful creation' do + before do + api_call.call(admin_headers) + end + + it 'creates a credential binding in the database' do + expect(binding.app).to eq(app_to_bind_to) if check_app + expect(binding.service_instance).to eq(service_instance) + expect(binding.last_operation.state).to eq('in progress') + expect(binding.last_operation.type).to eq('create') + end + + it 'responds with a job resource' do + expect(last_response).to have_status_code(202) + expect(last_response.headers['Location']).to end_with("/v3/jobs/#{job.guid}") + + expect(job.state).to eq(VCAP::CloudController::PollableJobModel::PROCESSING_STATE) + expect(job.operation).to eq("#{display_name}.create") + expect(job.resource_guid).to eq(binding.guid) + expect(job.resource_type).to eq('service_credential_binding') + + get "/v3/jobs/#{job.guid}", nil, admin_headers + expect(last_response).to have_status_code(200) + expect(parsed_response['guid']).to eq(job.guid) + binding_link = parsed_response.dig('links', 'service_credential_binding', 'href') + expect(binding_link).to end_with("/v3/service_credential_bindings/#{binding.guid}") + end + end + + describe 'the pollable job' do + let(:credentials) { { 'password' => 'special sauce' } } + let(:broker_base_url) { service_instance.service_broker.broker_url } + let(:broker_bind_url) { "#{broker_base_url}/v2/service_instances/#{service_instance.guid}/service_bindings/#{binding.guid}" } + let(:broker_status_code) { 201 } + let(:broker_response) { { credentials: credentials } } + let(:app_binding_attributes) { + if check_app + { + app_guid: app_to_bind_to.guid, + bind_resource: { + app_guid: app_to_bind_to.guid, + space_guid: service_instance.space.guid + } + } + else + {} + end + } + let(:client_body) do + { + context: { + platform: 'cloudfoundry', + organization_guid: org.guid, + organization_name: org.name, + space_guid: space.guid, + space_name: space.name, + }, + service_id: service_instance.service_plan.service.unique_id, + plan_id: service_instance.service_plan.unique_id, + bind_resource: { + credential_client_id: 'cc_service_key_client', + }, + }.merge(app_binding_attributes) + end + + before do + api_call.call(admin_headers) + expect(last_response).to have_status_code(202) + + stub_request(:put, broker_bind_url). + with(query: { accepts_incomplete: true }). + to_return(status: broker_status_code, body: broker_response.to_json, headers: {}) + end + + it 'sends a bind request with the right arguments to the service broker' do + execute_all_jobs(expected_successes: 1, expected_failures: 0) + + expect( + a_request(:put, broker_bind_url). + with( + body: client_body, + query: { accepts_incomplete: true } + ) + ).to have_been_made.once + end + + context 'parameters are specified' do + let(:request_extra) { { parameters: { foo: 'bar' } } } + + it 'sends the parameters to the broker' do + execute_all_jobs(expected_successes: 1, expected_failures: 0) + + expect( + a_request(:put, broker_bind_url). + with( + body: client_body.deep_merge(request_extra), + query: { accepts_incomplete: true } + ) + ).to have_been_made.once + end + end + + context 'when the bind completes synchronously' do + it 'updates the the binding' do + execute_all_jobs(expected_successes: 1, expected_failures: 0) + + binding.reload + expect(binding.credentials).to eq(credentials) + expect(binding.last_operation.type).to eq('create') + expect(binding.last_operation.state).to eq('succeeded') + end + + it 'completes the job' do + execute_all_jobs(expected_successes: 1, expected_failures: 0) + + expect(job.state).to eq(VCAP::CloudController::PollableJobModel::COMPLETE_STATE) + end + + it 'logs an audit event' do + execute_all_jobs(expected_successes: 1, expected_failures: 0) + + event = VCAP::CloudController::Event.find(type: "audit.#{audit_event_type}.create") + expect(event).to be + expect(event.actee).to eq(binding.guid) + expect(event.actee_name).to eq(binding.name) + expect(event.data).to include({ + 'request' => create_body.with_indifferent_access + }) + end + end + + context 'when the broker fails to bind' do + let(:broker_status_code) { 422 } + let(:broker_response) { { error: 'RequiresApp' } } + + it 'updates the the binding with a failure' do + execute_all_jobs(expected_successes: 0, expected_failures: 1) + + binding.reload + expect(binding.last_operation.type).to eq('create') + expect(binding.last_operation.state).to eq('failed') + end + + it 'fails the job' do + execute_all_jobs(expected_successes: 0, expected_failures: 1) + + expect(job.state).to eq(VCAP::CloudController::PollableJobModel::FAILED_STATE) + end + end + + context 'when the binding completes asynchronously' do + let(:broker_status_code) { 202 } + let(:operation) { Sham.guid } + let(:broker_response) { { operation: operation } } + let(:broker_binding_last_operation_url) { "#{broker_base_url}/v2/service_instances/#{service_instance.guid}/service_bindings/#{binding.guid}/last_operation" } + let(:last_operation_status_code) { 200 } + let(:description) { Sham.description } + let(:state) { 'in progress' } + let(:last_operation_body) do + { + description: description, + state: state, + } + end + + before do + stub_request(:get, broker_binding_last_operation_url). + with(query: hash_including({ + operation: operation + })). + to_return(status: last_operation_status_code, body: last_operation_body.to_json, headers: {}) + end + + it 'polls the last operation endpoint' do + execute_all_jobs(expected_successes: 1, expected_failures: 0) + + expect( + a_request(:get, broker_binding_last_operation_url). + with(query: { + operation: operation, + service_id: service_instance.service_plan.service.unique_id, + plan_id: service_instance.service_plan.unique_id, + }) + ).to have_been_made.once + end + + it 'updates the binding and job' do + execute_all_jobs(expected_successes: 1, expected_failures: 0) + + expect(binding.last_operation.type).to eq('create') + expect(binding.last_operation.state).to eq(state) + expect(binding.last_operation.description).to eq(description) + + expect(job.state).to eq(VCAP::CloudController::PollableJobModel::POLLING_STATE) + end + + it 'logs an audit event' do + execute_all_jobs(expected_successes: 1, expected_failures: 0) + + event = VCAP::CloudController::Event.find(type: "audit.#{audit_event_type}.start_create") + expect(event).to be + expect(event.actee).to eq(binding.guid) + expect(event.data).to include({ + 'request' => create_body.with_indifferent_access + }) + end + + it 'enqueues the next fetch last operation job' do + execute_all_jobs(expected_successes: 1, expected_failures: 0) + expect(Delayed::Job.count).to eq(1) + end + + it 'keeps track of the broker operation' do + execute_all_jobs(expected_successes: 1, expected_failures: 0) + expect(Delayed::Job.count).to eq(1) + + Timecop.travel(Time.now + 1.minute) + execute_all_jobs(expected_successes: 1, expected_failures: 0) + + expect( + a_request(:get, broker_binding_last_operation_url). + with(query: { + operation: operation, + service_id: service_instance.service_plan.service.unique_id, + plan_id: service_instance.service_plan.unique_id, + }) + ).to have_been_made.twice + end + + context 'last operation response is 200 OK and indicates success' do + let(:state) { 'succeeded' } + let(:fetch_binding_status_code) { 200 } + let(:credentials) { { password: 'foo' } } + let(:parameters) { { foo: 'bar', another_foo: 'another_bar' } } + + let(:app_binding_attributes) { + if check_app + { + syslog_drain_url: syslog_drain_url, + } + else + {} + end + } + + let(:fetch_binding_body) do + { + credentials: credentials, + parameters: parameters, + service_id: 'extra-field-service_id-should-ignore', + name: 'extra-field-name-should-not-update', + }.merge(app_binding_attributes) + end + + before do + stub_request(:get, broker_bind_url). + to_return(status: fetch_binding_status_code, body: fetch_binding_body.to_json, headers: {}) + end + + it 'fetches the binding' do + execute_all_jobs(expected_successes: 1, expected_failures: 0) + + expect( + a_request(:get, broker_bind_url) + ).to have_been_made.once + end + + it 'updates the binding and job' do + execute_all_jobs(expected_successes: 1, expected_failures: 0) + + expect(binding.last_operation.type).to eq('create') + expect(binding.last_operation.state).to eq(state) + expect(binding.last_operation.description).to eq(description) + + expect(job.state).to eq(VCAP::CloudController::PollableJobModel::COMPLETE_STATE) + end + + it 'updates the binding details with the fetch binding response ignoring extra fields' do + execute_all_jobs(expected_successes: 1, expected_failures: 0) + + expect(binding.reload.credentials).to eq(credentials.with_indifferent_access) + expect(binding.syslog_drain_url).to eq(syslog_drain_url) if check_app + expect(binding.name).to eq(binding_name) + end + + it 'logs an audit event' do + execute_all_jobs(expected_successes: 1, expected_failures: 0) + + event = VCAP::CloudController::Event.find(type: "audit.#{audit_event_type}.create") + expect(event).to be + expect(event.actee).to eq(binding.guid) + expect(event.data).to include({ + 'request' => create_body.with_indifferent_access + }) + end + + context 'fetching binding fails ' do + let(:fetch_binding_status_code) { 404 } + let(:fetch_binding_body) {} + + it 'fails the job' do + execute_all_jobs(expected_successes: 0, expected_failures: 1) + + expect(binding.last_operation.type).to eq('create') + expect(binding.last_operation.state).to eq('failed') + expect(binding.last_operation.description).to include('The service broker rejected the request. Status Code: 404 Not Found') + + expect(job.state).to eq(VCAP::CloudController::PollableJobModel::FAILED_STATE) + expect(job.cf_api_error).not_to be_nil + error = YAML.safe_load(job.cf_api_error) + expect(error['errors'].first).to include({ + 'code' => 10009, + 'title' => 'CF-UnableToPerform', + 'detail' => 'bind could not be completed: The service broker rejected the request. Status Code: 404 Not Found, Body: null', + }) + end + end + end + + it_behaves_like 'binding last operation response handling', 'create' + + context 'binding not retrievable' do + let(:offering) { VCAP::CloudController::Service.make(bindings_retrievable: false) } + + it 'fails the job with an appropriate error' do + execute_all_jobs(expected_successes: 0, expected_failures: 1) + + expect(binding.last_operation.type).to eq('create') + expect(binding.last_operation.state).to eq('failed') + expect(binding.last_operation.description).to eq('The broker responded asynchronously but does not support fetching binding data') + + expect(job.state).to eq(VCAP::CloudController::PollableJobModel::FAILED_STATE) + expect(job.cf_api_error).not_to be_nil + error = YAML.safe_load(job.cf_api_error) + # TODO: check error message + expect(error['errors'].first).to include({ + 'code' => 90001, + 'title' => 'CF-ServiceBindingInvalid', + 'detail' => 'The service binding is invalid: The broker responded asynchronously but does not support fetching binding data', + }) + end + end + end + + if check_app + # temporarily only check for app bindings as it is not finished for key bindings + context 'orphan mitigation' do + it_behaves_like 'create binding orphan mitigation' do + let(:bind_url) { broker_bind_url } + let(:plan_id) { plan.unique_id } + let(:offering_id) { offering.unique_id } + end + end + end + end + end +end diff --git a/spec/support/shared_examples/request/bindings_orphan_mitigation.rb b/spec/support/shared_examples/request/bindings_orphan_mitigation.rb index 28343ed3f14..2992c906477 100644 --- a/spec/support/shared_examples/request/bindings_orphan_mitigation.rb +++ b/spec/support/shared_examples/request/bindings_orphan_mitigation.rb @@ -12,8 +12,7 @@ }).to_return(status: 200, body: {}.to_json) stub_request(:put, bind_url). - with(query: { accepts_incomplete: true }, - body: client_body).to_return(status: broker_bind_status_code, body: bind_response_body) + with(query: { accepts_incomplete: true }).to_return(status: broker_bind_status_code, body: bind_response_body) end after do @@ -248,8 +247,7 @@ stub_request(:put, bind_url). with(query: { accepts_incomplete: true, - }, - body: client_body).to_timeout + }).to_timeout end it 'does orphan mitigation and fails the job' do diff --git a/spec/support/shared_examples/v3_service_binding_create.rb b/spec/support/shared_examples/v3_service_binding_create.rb index be0ee364b61..cf9a689e594 100644 --- a/spec/support/shared_examples/v3_service_binding_create.rb +++ b/spec/support/shared_examples/v3_service_binding_create.rb @@ -251,3 +251,125 @@ class BadError < StandardError; end end end end + +RSpec.shared_examples 'polling service credential binding creation' do + describe '#poll' do + let(:credentials) { { 'password' => 'rennt', 'username' => 'lola' } } + let(:fetch_binding_response) { { credentials: credentials, syslog_drain_url: syslog_drain_url, volume_mounts: volume_mounts, name: 'updated-name' } } + + it_behaves_like 'polling service binding creation' + + describe 'credential bindings specific behaviour' do + let(:service_offering) { VCAP::CloudController::Service.make(bindings_retrievable: true, requires: ['route_forwarding']) } + let(:service_plan) { VCAP::CloudController::ServicePlan.make(service: service_offering) } + let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space: space, service_plan: service_plan) } + let(:broker_provided_operation) { Sham.guid } + let(:bind_response) { { async: true, operation: broker_provided_operation } } + let(:description) { Sham.description } + let(:state) { 'in progress' } + let(:fetch_last_operation_response) do + { + last_operation: { + state: state, + description: description, + }, + } + end + let(:broker_client) do + instance_double( + VCAP::Services::ServiceBrokers::V2::Client, + { + bind: bind_response, + fetch_and_handle_service_binding_last_operation: fetch_last_operation_response, + fetch_service_binding: fetch_binding_response, + } + ) + end + + before do + allow(VCAP::Services::ServiceBrokers::V2::Client).to receive(:new).and_return(broker_client) + + action.bind(binding, accepts_incomplete: true) + end + + context 'response says complete' do + let(:description) { Sham.description } + let(:state) { 'succeeded' } + + it 'fetches the service binding and updates only the credentials, volume_mounts and syslog_drain_url' do + action.poll(binding) + + expect(broker_client).to have_received(:fetch_service_binding).with(binding) + + binding.reload + expect(binding.credentials).to eq(credentials) + expect(binding.syslog_drain_url).to eq(syslog_drain_url) if binding.respond_to?(:syslog_drain_url) + expect(binding.volume_mounts).to eq(volume_mounts) if binding.respond_to?(:volume_mounts) + expect(binding.name).to eq(original_name) + end + + it 'creates an audit event' do + action.poll(binding) + + expect(binding_event_repo).to have_received(:record_create).with( + binding, + user_audit_info, + audit_hash + ) + end + end + + context 'response says in progress' do + it 'does not create an audit event' do + action.poll(binding) + + expect(binding_event_repo).not_to have_received(:record_create) + end + end + + context 'response says failed' do + let(:state) { 'failed' } + it 'does not create an audit event' do + expect { action.poll(binding) }.to raise_error(VCAP::CloudController::V3::LastOperationFailedState) + + expect(binding_event_repo).not_to have_received(:record_create) + end + end + end + end +end + +RSpec.shared_examples 'the sync credential binding' do |klass, extra_checks| + it 'creates and returns the credential binding' do + action.bind(precursor) + + precursor.reload + expect(precursor).to eq(klass.where(guid: precursor.guid).first) + expect(precursor.credentials).to eq(details[:credentials]) + expect(precursor.syslog_drain_url).to eq(details[:syslog_drain_url]) if extra_checks + expect(precursor.last_operation.type).to eq('create') + expect(precursor.last_operation.state).to eq('succeeded') + end + + it 'creates an audit event' do + action.bind(precursor) + expect(binding_event_repo).to have_received(:record_create).with( + precursor, + user_audit_info, + audit_hash + ) + end + + context 'when saving to the db fails' do + it 'fails the binding operation' do + allow(precursor).to receive(:save_with_attributes_and_new_operation).once.and_raise(Sequel::ValidationFailed, 'Meh') + allow(precursor).to receive(:save_with_attributes_and_new_operation). + with(anything, { type: 'create', state: 'failed', description: 'Meh' }).and_call_original + expect { action.bind(precursor) }.to raise_error(Sequel::ValidationFailed, 'Meh') + precursor.reload + expect(precursor.last_operation.type).to eq('create') + expect(precursor.last_operation.state).to eq('failed') + expect(precursor.last_operation.description).to eq('Meh') + end + end +end 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 08d020f8de4..5a91d73a4ea 100644 --- a/spec/unit/actions/service_credential_binding_app_create_spec.rb +++ b/spec/unit/actions/service_credential_binding_app_create_spec.rb @@ -85,79 +85,80 @@ module V3 let(:service_instance) { ManagedServiceInstance.make(**details) } - context 'when plan is not bindable' do - before do - service_instance.service_plan.update(bindable: false) - end + context 'validations' do + context 'when plan is not bindable' do + before do + service_instance.service_plan.update(bindable: false) + end - it 'raises an error' do - expect { action.precursor(service_instance, app: app) }.to raise_error( - ServiceCredentialBindingAppCreate::UnprocessableCreate, - 'Service plan does not allow bindings' - ) + it 'raises an error' do + expect { action.precursor(service_instance, app: app) }.to raise_error( + ServiceCredentialBindingAppCreate::UnprocessableCreate, + 'Service plan does not allow bindings' + ) + end end - end - context 'when plan is not available' do - before do - service_instance.service_plan.update(active: false) - end + context 'when plan is not available' do + before do + service_instance.service_plan.update(active: false) + end - it 'raises an error' do - expect { action.precursor(service_instance, app: app) }.to raise_error( - ServiceCredentialBindingAppCreate::UnprocessableCreate, - 'Service plan is not available' - ) + it 'raises an error' do + expect { action.precursor(service_instance, app: app) }.to raise_error( + ServiceCredentialBindingAppCreate::UnprocessableCreate, + 'Service plan is not available' + ) + end end - end - context 'when the service is a volume service and service volume mounting is disabled' do - let(:service_instance) { ManagedServiceInstance.make(:volume_mount, **details) } + context 'when the service is a volume service and service volume mounting is disabled' do + let(:service_instance) { ManagedServiceInstance.make(:volume_mount, **details) } - it 'raises an error' do - expect { - action.precursor(service_instance, app: app, volume_mount_services_enabled: false) - }.to raise_error( - ServiceCredentialBindingAppCreate::UnprocessableCreate, - 'Support for volume mount services is disabled' - ) + it 'raises an error' do + expect { + action.precursor(service_instance, app: app, volume_mount_services_enabled: false) + }.to raise_error( + ServiceCredentialBindingAppCreate::UnprocessableCreate, + 'Support for volume mount services is disabled' + ) + end end - end - context 'when there is an operation in progress for the service instance' do - it 'raises an error' do - service_instance.save_with_new_operation({}, { type: 'tacos', state: 'in progress' }) + context 'when there is an operation in progress for the service instance' do + it 'raises an error' do + service_instance.save_with_new_operation({}, { type: 'tacos', state: 'in progress' }) - expect { - action.precursor(service_instance, app: app, volume_mount_services_enabled: false) - }.to raise_error( - ServiceCredentialBindingAppCreate::UnprocessableCreate, - 'There is an operation in progress for the service instance' - ) + expect { + action.precursor(service_instance, app: app, volume_mount_services_enabled: false) + }.to raise_error( + ServiceCredentialBindingAppCreate::UnprocessableCreate, + 'There is an operation in progress for the service instance' + ) + end end - end - context 'when the service is a volume service and service volume mounting is enabled' do - let(:service_instance) { ManagedServiceInstance.make(:volume_mount, **details) } + context 'when the service is a volume service and service volume mounting is enabled' do + let(:service_instance) { ManagedServiceInstance.make(:volume_mount, **details) } - it 'does not raise an error' do - expect { - action.precursor(service_instance, app: app, volume_mount_services_enabled: true) - }.not_to raise_error + it 'does not raise an error' do + expect { + action.precursor(service_instance, app: app, volume_mount_services_enabled: true) + }.not_to raise_error + end end - end - context 'when binding from an app in a shared space' do - let(:other_space) { Space.make } - let(:service_instance) do - ManagedServiceInstance.make(space: other_space).tap do |si| - si.add_shared_space(space) + context 'when binding from an app in a shared space' do + let(:other_space) { Space.make } + let(:service_instance) do + ManagedServiceInstance.make(space: other_space).tap do |si| + si.add_shared_space(space) + end end - end - it_behaves_like 'the credential binding precursor' + it_behaves_like 'the credential binding precursor' + end end - context 'when successful' do it_behaves_like 'the credential binding precursor' end @@ -177,42 +178,6 @@ module V3 it_behaves_like 'service binding creation', ServiceBinding describe 'app specific behaviour' do - RSpec.shared_examples 'the sync credential binding' do - it 'creates and returns the credential binding' do - action.bind(precursor) - - precursor.reload - expect(precursor).to eq(ServiceBinding.where(guid: precursor.guid).first) - expect(precursor.credentials).to eq(details[:credentials]) - expect(precursor.syslog_drain_url).to eq(details[:syslog_drain_url]) - expect(precursor.last_operation.type).to eq('create') - expect(precursor.last_operation.state).to eq('succeeded') - end - - it 'creates an audit event' do - action.bind(precursor) - expect(binding_event_repo).to have_received(:record_create).with( - precursor, - user_audit_info, - audit_hash, - manifest_triggered: false, - ) - end - - context 'when saving to the db fails' do - it 'fails the binding operation' do - allow(precursor).to receive(:save_with_attributes_and_new_operation).once.and_raise(Sequel::ValidationFailed, 'Meh') - allow(precursor).to receive(:save_with_attributes_and_new_operation). - with(anything, { type: 'create', state: 'failed', description: 'Meh' }).and_call_original - expect { action.bind(precursor) }.to raise_error(Sequel::ValidationFailed, 'Meh') - precursor.reload - expect(precursor.last_operation.type).to eq('create') - expect(precursor.last_operation.state).to eq('failed') - expect(precursor.last_operation.description).to eq('Meh') - end - end - end - context 'managed service instance' do let(:service_offering) { Service.make(bindings_retrievable: true) } let(:service_plan) { ServicePlan.make(service: service_offering) } @@ -223,7 +188,7 @@ module V3 allow(VCAP::Services::ServiceBrokers::V2::Client).to receive(:new).and_return(broker_client) end - it_behaves_like 'the sync credential binding' + it_behaves_like 'the sync credential binding', ServiceBinding, true context 'asynchronous binding' do let(:broker_provided_operation) { Sham.guid } @@ -235,8 +200,7 @@ module V3 expect(binding_event_repo).to have_received(:record_start_create).with( precursor, user_audit_info, - audit_hash, - manifest_triggered: false, + audit_hash ) end end @@ -252,110 +216,30 @@ module V3 } let(:service_instance) { UserProvidedServiceInstance.make(**details) } - it_behaves_like 'the sync credential binding' + it_behaves_like 'the sync credential binding', ServiceBinding, true end end end describe '#poll' do - let(:binding) { action.precursor(service_instance, app: app, name: 'original-name') } - let(:credentials) { { 'password' => 'rennt', 'username' => 'lola' } } + let(:original_name) { 'original-name' } + let(:binding) { action.precursor(service_instance, app: app, name: original_name) } let(:volume_mounts) { [{ - 'driver' => 'cephdriver', - 'container_dir' => '/data/images', - 'mode' => 'r', - 'device_type' => 'shared', - 'device' => { - 'volume_id' => 'bc2c1eab-05b9-482d-b0cf-750ee07de311', - 'mount_config' => { - 'key' => 'value' - } - } - }] + 'driver' => 'cephdriver', + 'container_dir' => '/data/images', + 'mode' => 'r', + 'device_type' => 'shared', + 'device' => { + 'volume_id' => 'bc2c1eab-05b9-482d-b0cf-750ee07de311', + 'mount_config' => { + 'key' => 'value' + } + } + }] } let(:syslog_drain_url) { 'https://drain.syslog.example.com/runlolarun' } - let(:fetch_binding_response) { { credentials: credentials, syslog_drain_url: syslog_drain_url, volume_mounts: volume_mounts, name: 'updated-name' } } - - it_behaves_like 'polling service binding creation' - - describe 'app specific behaviour' do - let(:service_offering) { Service.make(bindings_retrievable: true, requires: ['route_forwarding']) } - let(:service_plan) { ServicePlan.make(service: service_offering) } - let(:service_instance) { ManagedServiceInstance.make(space: space, service_plan: service_plan) } - let(:broker_provided_operation) { Sham.guid } - let(:bind_response) { { async: true, operation: broker_provided_operation } } - let(:description) { Sham.description } - let(:state) { 'in progress' } - let(:fetch_last_operation_response) do - { - last_operation: { - state: state, - description: description, - }, - } - end - let(:broker_client) do - instance_double( - VCAP::Services::ServiceBrokers::V2::Client, - { - bind: bind_response, - fetch_and_handle_service_binding_last_operation: fetch_last_operation_response, - fetch_service_binding: fetch_binding_response, - } - ) - end - - before do - allow(VCAP::Services::ServiceBrokers::V2::Client).to receive(:new).and_return(broker_client) - - action.bind(binding, accepts_incomplete: true) - end - - context 'response says complete' do - let(:description) { Sham.description } - let(:state) { 'succeeded' } - it 'fetches the service binding and updates only the credentials, volume_mounts and syslog_drain_url' do - action.poll(binding) - - expect(broker_client).to have_received(:fetch_service_binding).with(binding) - - binding.reload - expect(binding.credentials).to eq(credentials) - expect(binding.syslog_drain_url).to eq(syslog_drain_url) - expect(binding.volume_mounts).to eq(volume_mounts) - expect(binding.name).to eq('original-name') - end - - it 'creates an audit event' do - action.poll(binding) - - expect(binding_event_repo).to have_received(:record_create).with( - binding, - user_audit_info, - audit_hash, - manifest_triggered: false, - ) - end - end - - context 'response says in progress' do - it 'does not create an audit event' do - action.poll(binding) - - expect(binding_event_repo).not_to have_received(:record_create) - end - end - - context 'response says failed' do - let(:state) { 'failed' } - it 'does not create an audit event' do - expect { action.poll(binding) }.to raise_error(VCAP::CloudController::V3::LastOperationFailedState) - - expect(binding_event_repo).not_to have_received(:record_create) - end - end - end + it_behaves_like 'polling service credential binding creation' end end end diff --git a/spec/unit/actions/service_credential_binding_key_create_spec.rb b/spec/unit/actions/service_credential_binding_key_create_spec.rb index 863fa403cf6..e4a62ba9546 100644 --- a/spec/unit/actions/service_credential_binding_key_create_spec.rb +++ b/spec/unit/actions/service_credential_binding_key_create_spec.rb @@ -5,12 +5,21 @@ module VCAP::CloudController module V3 RSpec.describe ServiceCredentialBindingKeyCreate do - subject(:action) { described_class.new } + subject(:action) { described_class.new(user_audit_info, audit_hash) } + let(:audit_hash) { { some_info: 'some_value' } } + let(:user_audit_info) { UserAuditInfo.new(user_email: 'run@lola.run', user_guid: '100_000') } let(:org) { Organization.make } let(:space) { Space.make(organization: org) } let(:binding_details) {} let(:name) { 'test-key' } + let(:binding_event_repo) { instance_double(Repositories::ServiceGenericBindingEventRepository) } + + before do + allow(Repositories::ServiceGenericBindingEventRepository).to receive(:new).with('service_key').and_return(binding_event_repo) + allow(binding_event_repo).to receive(:record_create) + allow(binding_event_repo).to receive(:record_start_create) + end describe '#precursor' do RSpec.shared_examples 'the credential binding precursor' do @@ -47,53 +56,55 @@ module V3 context 'managed service instance' do let(:service_instance) { ManagedServiceInstance.make(space: space) } - context 'when plan is not bindable' do - before do - service_instance.service_plan.update(bindable: false) + context 'validations' do + context 'when plan is not bindable' do + before do + service_instance.service_plan.update(bindable: false) + end + + it 'raises an error' do + expect { action.precursor(service_instance, name) }.to raise_error( + ServiceCredentialBindingKeyCreate::UnprocessableCreate, + 'Service plan does not allow bindings.' + ) + end end - it 'raises an error' do - expect { action.precursor(service_instance, name) }.to raise_error( - ServiceCredentialBindingKeyCreate::UnprocessableCreate, - 'Service plan does not allow bindings.' - ) + context 'when plan is not available' do + before do + service_instance.service_plan.update(active: false) + end + + it 'raises an error' do + expect { action.precursor(service_instance, name) }.to raise_error( + ServiceCredentialBindingKeyCreate::UnprocessableCreate, + 'Service plan is not available.' + ) + end end - end - - context 'when plan is not available' do - before do - service_instance.service_plan.update(active: false) - end - - it 'raises an error' do - expect { action.precursor(service_instance, name) }.to raise_error( - ServiceCredentialBindingKeyCreate::UnprocessableCreate, - 'Service plan is not available.' - ) - end - end - - context 'when there is an operation in progress for the service instance' do - it 'raises an error' do - service_instance.save_with_new_operation({}, { type: 'tacos', state: 'in progress' }) - expect { - action.precursor(service_instance, name) - }.to raise_error( - ServiceCredentialBindingKeyCreate::UnprocessableCreate, - 'There is an operation in progress for the service instance.' - ) + context 'when there is an operation in progress for the service instance' do + it 'raises an error' do + service_instance.save_with_new_operation({}, { type: 'tacos', state: 'in progress' }) + + expect { + action.precursor(service_instance, name) + }.to raise_error( + ServiceCredentialBindingKeyCreate::UnprocessableCreate, + 'There is an operation in progress for the service instance.' + ) + end end - end - context 'when the name is taken' do - let(:existing_service_key) { ServiceKey.make(service_instance: service_instance) } + context 'when the name is taken' do + let(:existing_service_key) { ServiceKey.make(service_instance: service_instance) } - it 'raises an error' do - expect { action.precursor(service_instance, existing_service_key.name) }.to raise_error( - ServiceCredentialBindingKeyCreate::UnprocessableCreate, - "The binding name is invalid. Key binding names must be unique. The service instance already has a key binding with name '#{existing_service_key.name}'." - ) + it 'raises an error' do + expect { action.precursor(service_instance, existing_service_key.name) }.to raise_error( + ServiceCredentialBindingKeyCreate::UnprocessableCreate, + "The binding name is invalid. Key binding names must be unique. The service instance already has a key binding with name '#{existing_service_key.name}'." + ) + end end end @@ -136,6 +147,59 @@ module V3 end end end + + context '#bind' do + let(:app) { nil } + let(:precursor) { action.precursor(service_instance, name) } + let(:specific_fields) { {} } + let(:details) { + { + credentials: { 'password' => 'rennt', 'username' => 'lola' } + }.merge(specific_fields) + } + let(:bind_response) { { binding: details } } + + it_behaves_like 'service binding creation', ServiceKey + + describe 'key specific behaviour' do + context 'managed service instance' do + let(:service_offering) { Service.make(bindings_retrievable: true) } + let(:service_plan) { ServicePlan.make(service: service_offering) } + let(:service_instance) { ManagedServiceInstance.make(space: space, service_plan: service_plan) } + let(:broker_client) { instance_double(VCAP::Services::ServiceBrokers::V2::Client, bind: bind_response) } + + before do + allow(VCAP::Services::ServiceBrokers::V2::Client).to receive(:new).and_return(broker_client) + end + + it_behaves_like 'the sync credential binding', ServiceKey + + context 'asynchronous binding' do + let(:broker_provided_operation) { Sham.guid } + let(:bind_async_response) { { async: true, operation: broker_provided_operation } } + let(:broker_client) { instance_double(VCAP::Services::ServiceBrokers::V2::Client, bind: bind_async_response) } + + it 'should log audit start_create' do + action.bind(precursor) + expect(binding_event_repo).to have_received(:record_start_create).with( + precursor, + user_audit_info, + audit_hash + ) + end + end + end + end + end + + describe '#poll' do + let(:original_name) { 'original-name' } + let(:binding) { action.precursor(service_instance, original_name) } + let(:volume_mounts) { nil } + let(:syslog_drain_url) { nil } + + it_behaves_like 'polling service credential binding creation' + end end end end diff --git a/spec/unit/actions/service_route_binding_create_spec.rb b/spec/unit/actions/service_route_binding_create_spec.rb index 5be329dcd0d..afa1059a797 100644 --- a/spec/unit/actions/service_route_binding_create_spec.rb +++ b/spec/unit/actions/service_route_binding_create_spec.rb @@ -202,8 +202,7 @@ module V3 expect(binding_event_repo).to have_received(:record_create).with( precursor, user_audit_info, - audit_hash, - manifest_triggered: false, + audit_hash ) end @@ -249,8 +248,7 @@ module V3 expect(binding_event_repo).to have_received(:record_start_create).with( precursor, user_audit_info, - audit_hash, - manifest_triggered: false, + audit_hash ) end end @@ -325,8 +323,7 @@ module V3 expect(binding_event_repo).to have_received(:record_create).with( binding, user_audit_info, - audit_hash, - manifest_triggered: false, + audit_hash ) end diff --git a/spec/unit/jobs/v3/create_binding_async_job_spec.rb b/spec/unit/jobs/v3/create_binding_async_job_spec.rb index 934079301ba..59fa9fccce9 100644 --- a/spec/unit/jobs/v3/create_binding_async_job_spec.rb +++ b/spec/unit/jobs/v3/create_binding_async_job_spec.rb @@ -42,6 +42,24 @@ module V3 it_behaves_like 'create binding job', :credential end + context 'key bindings' do + let(:binding) do + ServiceKey.new.save_with_attributes_and_new_operation( + { + service_instance: service_instance, + name: 'key-name', + credentials: {}, + }, + { + type: 'create', + state: 'in progress' + }, + ) + end + + it_behaves_like 'create binding job', :key + end + let(:subject) do described_class.new( :any, diff --git a/spec/unit/jobs/v3/create_service_binding_job_factory_spec.rb b/spec/unit/jobs/v3/create_service_binding_job_factory_spec.rb index 80bc0e946d1..459472facfe 100644 --- a/spec/unit/jobs/v3/create_service_binding_job_factory_spec.rb +++ b/spec/unit/jobs/v3/create_service_binding_job_factory_spec.rb @@ -4,7 +4,7 @@ module VCAP::CloudController module V3 RSpec.describe CreateServiceBindingFactory do - let(:factory) { CreateServiceBindingFactory.new } + let(:factory) { subject } describe '#for' do it 'should return route job actor when type is route' do @@ -17,8 +17,13 @@ module V3 expect(actor).to be_an_instance_of(CreateServiceCredentialBindingJobActor) end + it 'should return key job actor when type is key' do + actor = CreateServiceBindingFactory.for(:key) + expect(actor).to be_an_instance_of(CreateServiceKeyBindingJobActor) + end + it 'raise for unknown types' do - expect { CreateServiceBindingFactory.for(:key) }.to raise_error(CreateServiceBindingFactory::InvalidType) + expect { CreateServiceBindingFactory.for(:random) }.to raise_error(CreateServiceBindingFactory::InvalidType) end end @@ -33,8 +38,13 @@ module V3 expect(actor).to be_an_instance_of(ServiceCredentialBindingAppCreate) end + it 'should return credential binding action when type is key' do + actor = CreateServiceBindingFactory.action(:key, {}, {}) + expect(actor).to be_an_instance_of(ServiceCredentialBindingKeyCreate) + end + it 'raise for unknown types' do - expect { CreateServiceBindingFactory.action(:key, {}, {}) }.to raise_error(CreateServiceBindingFactory::InvalidType) + expect { CreateServiceBindingFactory.action(:random, {}, {}) }.to raise_error(CreateServiceBindingFactory::InvalidType) end end end diff --git a/spec/unit/jobs/v3/create_service_credential_binding_job_actor_spec.rb b/spec/unit/jobs/v3/create_service_credential_binding_job_actor_spec.rb index 04d3a2c8c3e..78a005d3943 100644 --- a/spec/unit/jobs/v3/create_service_credential_binding_job_actor_spec.rb +++ b/spec/unit/jobs/v3/create_service_credential_binding_job_actor_spec.rb @@ -1,14 +1,9 @@ require 'db_spec_helper' -require 'support/shared_examples/jobs/delayed_job' require 'jobs/v3/create_service_credential_binding_job_actor' module VCAP::CloudController module V3 RSpec.describe CreateServiceCredentialBindingJobActor do - let(:subject) do - described_class.new - end - describe '#display_name' do it 'returns "service_bindings.create"' do expect(subject.display_name).to eq('service_bindings.create') diff --git a/spec/unit/jobs/v3/create_service_key_binding_job_actor_spec.rb b/spec/unit/jobs/v3/create_service_key_binding_job_actor_spec.rb new file mode 100644 index 00000000000..b4b7805f033 --- /dev/null +++ b/spec/unit/jobs/v3/create_service_key_binding_job_actor_spec.rb @@ -0,0 +1,34 @@ +require 'db_spec_helper' +require 'jobs/v3/create_service_key_binding_job_actor' + +module VCAP::CloudController + module V3 + RSpec.describe CreateServiceKeyBindingJobActor do + describe '#display_name' do + it 'returns "service_keys.create"' do + expect(subject.display_name).to eq('service_keys.create') + end + end + + describe '#resource_type' do + it 'returns "service_credential_binding"' do + expect(subject.resource_type).to eq('service_credential_binding') + end + end + + describe '#get_resource' do + let(:binding) do + ServiceKey.make + end + + it 'returns the resource when it exists' do + expect(subject.get_resource(binding.guid)).to eq(binding) + end + + it 'returns nil when resource not found' do + expect(subject.get_resource('fake_guid')).to be_nil + end + end + end + end +end diff --git a/spec/unit/lib/services/service_brokers/v2/client_spec.rb b/spec/unit/lib/services/service_brokers/v2/client_spec.rb index 2a88fd5e3b9..477dddd371a 100644 --- a/spec/unit/lib/services/service_brokers/v2/client_spec.rb +++ b/spec/unit/lib/services/service_brokers/v2/client_spec.rb @@ -1221,6 +1221,28 @@ module VCAP::Services::ServiceBrokers::V2 end end + context 'when the binding is of type key' do + let(:binding) { VCAP::CloudController::ServiceKey.make } + + it 'sends the right bind_resource' do + client.bind(binding) + + expect(http_client).to have_received(:put). + with(anything, + hash_including({ bind_resource: { credential_client_id: 'cc_service_key_client' } }) + ) + end + + it 'does not send the app_guid in the request' do + client.bind(binding) + + expect(http_client).to have_received(:put). + with(anything, + hash_excluding(:app_guid) + ) + end + end + context 'with a syslog drain url' do before do instance.service_plan.service.update_from_hash(requires: ['syslog_drain']) @@ -1337,6 +1359,19 @@ module VCAP::Services::ServiceBrokers::V2 expect(orphan_mitigator).to have_received(:cleanup_failed_bind). with(client_attrs, binding) end + + context 'binding of type key' do + let(:binding) { VCAP::CloudController::ServiceKey.make } + + it 'propagates the error and cleans up the failed binding' do + expect { + client.bind(binding) + }.to raise_error(Errors::HttpClientTimeout) + + expect(orphan_mitigator).to have_received(:cleanup_failed_key). + with(client_attrs, binding) + end + end end end diff --git a/spec/unit/models/runtime/pollable_job_model_spec.rb b/spec/unit/models/runtime/pollable_job_model_spec.rb index fc3453b37c9..fa03f9f4292 100644 --- a/spec/unit/models/runtime/pollable_job_model_spec.rb +++ b/spec/unit/models/runtime/pollable_job_model_spec.rb @@ -85,24 +85,30 @@ module VCAP::CloudController expect(job.resource_exists?).to be(false) end - it 'returns true if the resource exists' do + it 'returns true if the route binding resource exists' do route_binding = RouteBinding.make job = PollableJobModel.make(resource_type: 'service_route_binding', resource_guid: route_binding.guid) expect(job.resource_exists?).to be(true) end - it 'returns false if the resource does NOT exist' do + it 'returns false if the route binding resource does NOT exist' do job = PollableJobModel.make(resource_type: 'service_route_binding', resource_guid: 'not-a-real-guid') expect(job.resource_exists?).to be(false) end - it 'returns true if the resource exists' do + it 'returns true if the service binding resource exists' do binding = ServiceBinding.make job = PollableJobModel.make(resource_type: 'service_credential_binding', resource_guid: binding.guid) expect(job.resource_exists?).to be(true) end - it 'returns false if the resource does NOT exist' do + it 'returns true if the service key resource exists' do + binding = ServiceKey.make + job = PollableJobModel.make(resource_type: 'service_credential_binding', resource_guid: binding.guid) + expect(job.resource_exists?).to be(true) + end + + it 'returns false if the service credential binding resource does NOT exist' do job = PollableJobModel.make(resource_type: 'service_credential_binding', resource_guid: 'not-a-real-guid') expect(job.resource_exists?).to be(false) end diff --git a/spec/unit/models/services/service_binding_operation_spec.rb b/spec/unit/models/services/service_binding_operation_spec.rb index d4a540bf8b8..02728f837dd 100644 --- a/spec/unit/models/services/service_binding_operation_spec.rb +++ b/spec/unit/models/services/service_binding_operation_spec.rb @@ -1,47 +1,8 @@ require 'spec_helper' +require 'support/shared_examples/models/operations' module VCAP::CloudController RSpec.describe ServiceBindingOperation, type: :model do - let(:updated_at_time) { Time.utc(2018, 5, 2, 3, 30, 0) } - let(:created_at_time) { Time.utc(2018, 5, 2, 3, 30, 0) } - let(:operation_attributes) do - { - state: 'in progress', - description: '10%', - type: 'create', - } - end - - let(:operation) { ServiceBindingOperation.make(operation_attributes) } - - before do - operation.this.update(updated_at: updated_at_time, created_at: created_at_time) - operation.reload - end - - describe '#to_hash' do - it 'includes the state, type, description, created_at and updated_at' do - expect(operation.to_hash).to include({ - 'state' => 'in progress', - 'type' => 'create', - 'description' => '10%', - }) - - expect(operation.to_hash['updated_at']).to eq(updated_at_time) - expect(operation.to_hash['created_at']).to eq(created_at_time) - end - end - - describe 'updating attributes' do - it 'updates the attributes of the service instance operation' do - new_attributes = { - state: 'finished', - description: '100%' - } - operation.update_attributes(new_attributes) - expect(operation.state).to eq 'finished' - expect(operation.description).to eq '100%' - end - end + it_behaves_like 'operation' end end diff --git a/spec/unit/models/services/service_key_operation_spec.rb b/spec/unit/models/services/service_key_operation_spec.rb new file mode 100644 index 00000000000..9dfe5cb9bdc --- /dev/null +++ b/spec/unit/models/services/service_key_operation_spec.rb @@ -0,0 +1,8 @@ +require 'spec_helper' +require 'support/shared_examples/models/operations' + +module VCAP::CloudController + RSpec.describe ServiceKeyOperation, type: :model do + it_behaves_like 'operation' + end +end diff --git a/spec/unit/models/services/service_key_spec.rb b/spec/unit/models/services/service_key_spec.rb index 2c6fa7a65e4..ec4f82e44d8 100644 --- a/spec/unit/models/services/service_key_spec.rb +++ b/spec/unit/models/services/service_key_spec.rb @@ -155,5 +155,178 @@ def new_model expect(visible_to_other_user.all).to be_empty end end + + describe '#save_with_new_operation' do + let(:service_instance) { ServiceInstance.make } + let(:binding) { + ServiceKey.new( + service_instance: service_instance, + credentials: {}, + name: 'foo', + ) + } + + it 'creates a new last_operation object and associates it with the binding' do + last_operation = { + state: 'in progress', + type: 'create', + description: '10%' + } + binding.save_with_new_operation(last_operation) + + expect(binding.last_operation.state).to eq 'in progress' + expect(binding.last_operation.description).to eq '10%' + expect(binding.last_operation.type).to eq 'create' + expect(ServiceKey.where(guid: binding.guid).count).to eq(1) + end + + context 'when saving the binding operation fails' do + before do + allow(ServiceKeyOperation).to receive(:create).and_raise(Sequel::DatabaseError, 'failed to create new-binding operation') + end + + it 'should rollback the binding' do + expect { binding.save_with_new_operation({ state: 'will fail' }) }.to raise_error(Sequel::DatabaseError) + expect(ServiceKey.where(guid: binding.guid).count).to eq(0) + end + end + + context 'when called twice' do + it 'does saves the second operation' do + binding.save_with_new_operation({ state: 'in progress', type: 'create', description: 'description' }) + binding.save_with_new_operation({ state: 'in progress', type: 'delete' }) + + expect(binding.last_operation.state).to eq 'in progress' + expect(binding.last_operation.type).to eq 'delete' + expect(binding.last_operation.description).to eq nil + expect(ServiceKey.where(guid: binding.guid).count).to eq(1) + expect(ServiceKeyOperation.where(service_key_id: binding.id).count).to eq(1) + end + end + + context 'when attributes are passed in' do + let(:credentials) { { password: 'rice' } } + let(:attributes) { + { + name: 'gohan', + credentials: credentials, + } + } + let(:last_operation) { { + state: 'in progress', + type: 'create', + description: '10%' + } + } + + it 'updates the attributes' do + binding.save_with_new_operation(last_operation, attributes: attributes) + binding.reload + expect(binding.last_operation.state).to eq 'in progress' + expect(binding.last_operation.description).to eq '10%' + expect(binding.last_operation.type).to eq 'create' + expect(binding.name).to eq 'gohan' + expect(binding.credentials).to eq(credentials.with_indifferent_access) + expect(ServiceKey.where(guid: binding.guid).count).to eq(1) + end + + it 'only saves permitted attributes' do + expect { + binding.save_with_new_operation(last_operation, attributes: attributes.merge( + parameters: { + foo: 'bar', + ding: 'dong' + }, + endpoints: [{ host: 'mysqlhost', ports: ['3306'] }], + )) + }.not_to raise_error + end + end + end + + describe '#terminal_state?' do + let(:service_binding) { ServiceKey.make } + let(:operation) { ServiceKeyOperation.make(state: state) } + + before do + service_binding.service_key_operation = operation + end + + context 'when state is succeeded' do + let(:state) { 'succeeded' } + + it 'returns true' do + expect(service_binding.terminal_state?).to be true + end + end + + context 'when state is failed' do + let(:state) { 'failed' } + + it 'returns true when state is `failed`' do + expect(service_binding.terminal_state?).to be true + end + end + + context 'when state is something else' do + let(:state) { 'in progress' } + + it 'returns false' do + expect(service_binding.terminal_state?).to be false + end + end + + context 'when binding operation is missing' do + let(:operation) { nil } + + it 'returns true' do + expect(service_binding.terminal_state?).to be true + end + end + end + + describe 'operation_in_progress?' do + let(:service_instance) { ManagedServiceInstance.make } + let(:service_key) { ServiceKey.make(service_instance: service_instance) } + + context 'when the service key has been created synchronously' do + it 'returns false' do + expect(service_key.operation_in_progress?).to be false + end + end + + context 'when the service key is being created asynchronously' do + let(:state) {} + let(:operation) { ServiceKeyOperation.make(state: state) } + + before do + service_key.service_key_operation = operation + end + + context 'and the operation is in progress' do + let(:state) { 'in progress' } + + it 'returns true' do + expect(service_key.operation_in_progress?).to be true + end + end + + context 'and the operation has failed' do + let(:state) { 'failed' } + + it 'returns false' do + expect(service_key.operation_in_progress?).to be false + end + end + + context 'and the operation has succeeded' do + let(:state) { 'succeeded' } + + it 'returns false' do + expect(service_key.operation_in_progress?).to be false + end + end + end + end end end