diff --git a/app/access/service_instance_access.rb b/app/access/service_instance_access.rb
index cd8ab240003..dce57e8faa5 100644
--- a/app/access/service_instance_access.rb
+++ b/app/access/service_instance_access.rb
@@ -4,13 +4,13 @@ def create?(service_instance, params=nil)
return true if admin_user?
FeatureFlag.raise_unless_enabled!(:service_instance_creation)
return false if service_instance.in_suspended_org?
- service_instance.space.has_developer?(context.user) && allowed?(service_instance)
+ service_instance.space&.has_developer?(context.user) && allowed?(service_instance)
end
def read_for_update?(service_instance, params=nil)
return true if admin_user?
return false if service_instance.in_suspended_org?
- service_instance.space.has_developer?(context.user)
+ service_instance.space&.has_developer?(context.user)
end
def update?(service_instance, params=nil)
@@ -20,12 +20,12 @@ def update?(service_instance, params=nil)
def delete?(service_instance)
return true if admin_user?
return false if service_instance.in_suspended_org?
- service_instance.space.has_developer?(context.user)
+ service_instance.space&.has_developer?(context.user)
end
def manage_permissions?(service_instance)
return true if admin_user?
- service_instance.space.has_developer?(context.user)
+ service_instance.space&.has_developer?(context.user)
end
def manage_permissions_with_token?(service_instance)
@@ -33,8 +33,7 @@ def manage_permissions_with_token?(service_instance)
end
def read_permissions?(service_instance)
- return true if admin_user? || admin_read_only_user?
- service_instance.space.has_member?(context.user) || service_instance.space.organization.managers.include?(context.user)
+ admin_user? || admin_read_only_user? || object_is_visible_to_user?(service_instance, context.user)
end
def read_permissions_with_token?(service_instance)
@@ -43,7 +42,7 @@ def read_permissions_with_token?(service_instance)
def read_env?(service_instance)
return true if admin_user? || admin_read_only_user?
- service_instance.space.has_developer?(context.user)
+ service_instance.space&.has_developer?(context.user)
end
def read_env_with_token?(service_instance)
@@ -64,7 +63,7 @@ def allowed?(service_instance)
end
def purge?(service_instance)
- admin_user? || (service_instance.space.has_developer?(context.user) && service_instance.service_broker.private?)
+ admin_user? || (service_instance.space&.has_developer?(context.user) && service_instance.service_broker.private?)
end
def purge_with_token?(instance)
diff --git a/app/actions/service_instance_share.rb b/app/actions/service_instance_share.rb
index 7defe5c2bb6..d3634f1afa6 100644
--- a/app/actions/service_instance_share.rb
+++ b/app/actions/service_instance_share.rb
@@ -3,6 +3,12 @@
module VCAP::CloudController
class ServiceInstanceShare
def create(service_instance, target_spaces, user_audit_info)
+ if service_instance.managed_instance?
+ unless service_instance.shareable?
+ raise CloudController::Errors::ApiError.new_from_details('ServiceShareIsDisabled', service_instance.service.label)
+ end
+ end
+
ServiceInstance.db.transaction do
target_spaces.each do |space|
service_instance.add_shared_space(space)
@@ -10,7 +16,8 @@ def create(service_instance, target_spaces, user_audit_info)
end
Repositories::ServiceInstanceShareEventRepository.record_share_event(
- service_instance, target_spaces.map(&:guid), user_audit_info)
+ service_instance, target_spaces.map(&:guid), user_audit_info
+ )
service_instance
end
end
diff --git a/app/controllers/runtime/spaces_controller.rb b/app/controllers/runtime/spaces_controller.rb
index 7132a96abb6..d7e8780036c 100644
--- a/app/controllers/runtime/spaces_controller.rb
+++ b/app/controllers/runtime/spaces_controller.rb
@@ -138,20 +138,18 @@ def enumerate_services(guid)
def enumerate_service_instances(guid)
space = find_guid_and_validate_access(:read, guid)
- if params['return_user_provided_service_instances'] == 'true'
- model_class = ServiceInstance
- relation_name = :service_instances
- else
- model_class = ManagedServiceInstance
- relation_name = :managed_service_instances
- end
+ model_class = params['return_user_provided_service_instances'] == 'true' ? ServiceInstance : ManagedServiceInstance
service_instances = Query.filtered_dataset_from_query_params(
model_class,
- space.user_visible_relationship_dataset(relation_name, @access_context.user, @access_context.admin_override),
+ model_class.user_visible(@access_context.user, @access_context.admin_override),
ServiceInstancesController.query_parameters,
@opts)
- service_instances.filter(space: space)
+
+ service_instances = service_instances.filter(Sequel.or([
+ [:space, space],
+ [:shared_spaces, space]
+ ]))
collection_renderer.render_json(
ServiceInstancesController,
diff --git a/app/controllers/services/service_instances_controller.rb b/app/controllers/services/service_instances_controller.rb
index 44cc9ebf2ea..c3dd190c79b 100644
--- a/app/controllers/services/service_instances_controller.rb
+++ b/app/controllers/services/service_instances_controller.rb
@@ -5,6 +5,9 @@
require 'controllers/services/lifecycle/service_instance_deprovisioner'
require 'controllers/services/lifecycle/service_instance_purger'
require 'fetchers/service_instance_fetcher'
+require 'fetchers/service_binding_list_fetcher'
+require 'presenters/v2/service_instance_shared_to_presenter'
+require 'presenters/v2/service_instance_shared_from_presenter'
module VCAP::CloudController
class ServiceInstancesController < RestController::ModelController
@@ -31,14 +34,13 @@ class ServiceInstancesController < RestController::ModelController
define_routes
def self.translate_validation_exception(e, attributes)
- space_and_name_errors = e.errors.on([:space_id, :name]).to_a
quota_errors = e.errors.on(:quota).to_a
service_plan_errors = e.errors.on(:service_plan).to_a
service_instance_errors = e.errors.on(:service_instance).to_a
service_instance_name_errors = e.errors.on(:name).to_a
service_instance_tags_errors = e.errors.on(:tags).to_a
- if space_and_name_errors.include?(:unique)
+ if service_instance_name_errors.include?(:unique)
return CloudController::Errors::ApiError.new_from_details('ServiceInstanceNameTaken', attributes['name'])
elsif quota_errors.include?(:service_instance_space_quota_exceeded)
return CloudController::Errors::ApiError.new_from_details('ServiceInstanceSpaceQuotaExceeded')
@@ -157,11 +159,16 @@ def delete(guid)
end
validate_access(:delete, service_instance)
- has_assocations = has_routes?(service_instance) ||
- has_bindings?(service_instance) ||
- has_keys?(service_instance)
- association_not_empty! if has_assocations && !recursive_delete?
+ unless recursive_delete?
+ service_is_shared!(service_instance.name) if has_shares?(service_instance)
+
+ has_associations = has_routes?(service_instance) ||
+ has_bindings?(service_instance) ||
+ has_keys?(service_instance)
+
+ association_not_empty! if has_associations
+ end
deprovisioner = ServiceInstanceDeprovisioner.new(@services_event_repository, self, logger)
delete_job = deprovisioner.deprovision_service_instance(service_instance, accepts_incomplete, async)
@@ -205,6 +212,46 @@ def permissions(guid)
end
end
+ get '/v2/service_instances/:guid/shared_from', :shared_from_information
+ def shared_from_information(guid)
+ service_instance = find_guid_and_validate_access(:read, guid, ManagedServiceInstance)
+
+ if service_instance.shared?
+ [HTTP::OK, {}, JSON.generate(CloudController::Presenters::V2::ServiceInstanceSharedFromPresenter.new.to_hash(service_instance.space))]
+ else
+ [HTTP::NO_CONTENT, {}, '']
+ end
+ rescue CloudController::Errors::ApiError => e
+ if e.name == 'NotAuthorized'
+ HTTP::NOT_FOUND
+ else
+ raise e
+ end
+ end
+
+ get '/v2/service_instances/:guid/shared_to', :enumerate_shared_to_information
+ def enumerate_shared_to_information(guid)
+ service_instance = find_guid_and_validate_access(:read, guid, ManagedServiceInstance)
+ validate_access(:read, service_instance.space)
+
+ associated_controller = VCAP::CloudController::SpacesController
+ associated_path = "#{self.class.url_for_guid(guid)}/shared_to"
+
+ create_paginated_collection_renderer(service_instance).render_json(
+ associated_controller,
+ service_instance.shared_spaces_dataset,
+ associated_path,
+ @opts,
+ {},
+ )
+ rescue CloudController::Errors::ApiError => e
+ if e.name == 'NotAuthorized'
+ HTTP::NOT_FOUND
+ else
+ raise e
+ end
+ end
+
def self.url_for_guid(guid)
object = ServiceInstance.where(guid: guid).first
@@ -303,6 +350,28 @@ def unbind_route(route_guid, instance_guid)
private
+ class ServiceInstanceSharedToSerializer
+ def initialize(service_instance)
+ @service_instance = service_instance
+ end
+
+ def serialize(controller, space, opts, orphans=nil)
+ bound_app_count = ServiceBindingListFetcher.fetch_service_instance_bindings_in_space(@service_instance.guid, space.guid).count
+ CloudController::Presenters::V2::ServiceInstanceSharedToPresenter.new.to_hash(space, bound_app_count)
+ end
+ end
+
+ def create_paginated_collection_renderer(service_instance)
+ VCAP::CloudController::RestController::PaginatedCollectionRenderer.new(
+ VCAP::CloudController::RestController::SecureEagerLoader.new,
+ ServiceInstanceSharedToSerializer.new(service_instance),
+ {
+ max_results_per_page: config.get(:renderer, :max_results_per_page),
+ default_results_per_page: config.get(:renderer, :default_results_per_page),
+ max_inline_relations_depth: config.get(:renderer, :max_inline_relations_depth),
+ })
+ end
+
def route_services_enabled?
@config.get(:route_services_enabled)
end
@@ -397,6 +466,10 @@ def association_not_empty!
raise CloudController::Errors::ApiError.new_from_details('AssociationNotEmpty', associations, :service_instances)
end
+ def service_is_shared!(name)
+ raise CloudController::Errors::ApiError.new_from_details('ServiceInstanceDeletionSharesExists', name)
+ end
+
def space_change_not_allowed!
raise CloudController::Errors::ApiError.new_from_details('ServiceInstanceSpaceChangeNotAllowed')
end
@@ -429,6 +502,10 @@ def has_keys?(service_instance)
!service_instance.service_keys.empty?
end
+ def has_shares?(service_instance)
+ !service_instance.shared_spaces.empty?
+ end
+
def space_change_requested?(requested_space_guid, current_space)
requested_space_guid && requested_space_guid != current_space.guid
end
diff --git a/app/controllers/services/user_provided_service_instances_controller.rb b/app/controllers/services/user_provided_service_instances_controller.rb
index ad831bb98de..08b99d87fd6 100644
--- a/app/controllers/services/user_provided_service_instances_controller.rb
+++ b/app/controllers/services/user_provided_service_instances_controller.rb
@@ -29,11 +29,11 @@ def inject_dependencies(dependencies)
end
def self.translate_validation_exception(e, attributes)
- space_and_name_errors = e.errors.on([:space_id, :name])
+ name_errors = e.errors.on(:name)
service_instance_errors = e.errors.on(:service_instance)
service_instance_name_errors = e.errors.on(:name).to_a
- if space_and_name_errors&.include?(:unique)
+ if name_errors&.include?(:unique)
CloudController::Errors::ApiError.new_from_details('ServiceInstanceNameTaken', attributes['name'])
elsif service_instance_errors&.include?(:space_mismatch)
CloudController::Errors::ApiError.new_from_details('ServiceInstanceRouteBindingSpaceMismatch')
diff --git a/app/controllers/v3/service_instances_controller.rb b/app/controllers/v3/service_instances_controller.rb
index 3f1141b7ae4..525dc08a654 100644
--- a/app/controllers/v3/service_instances_controller.rb
+++ b/app/controllers/v3/service_instances_controller.rb
@@ -1,17 +1,36 @@
require 'messages/to_many_relationship_message'
+require 'messages/service_instances/service_instances_list_message'
require 'presenters/v3/relationship_presenter'
require 'presenters/v3/to_many_relationship_presenter'
+require 'presenters/v3/paginated_list_presenter'
require 'actions/service_instance_share'
require 'actions/service_instance_unshare'
+require 'fetchers/service_instance_list_fetcher'
class ServiceInstancesV3Controller < ApplicationController
+ def index
+ message = ServiceInstancesListMessage.from_params(query_params)
+ invalid_param!(message.errors.full_messages) unless message.valid?
+
+ dataset = if can_read_globally?
+ ServiceInstanceListFetcher.new.fetch_all(message: message)
+ else
+ ServiceInstanceListFetcher.new.fetch(message: message, space_guids: readable_space_guids)
+ end
+
+ render status: :ok, json: Presenters::V3::PaginatedListPresenter.new(
+ dataset: dataset,
+ path: '/v3/service_instances',
+ message: message)
+ end
+
def share_service_instance
FeatureFlag.raise_unless_enabled!(:service_instance_sharing)
service_instance = ServiceInstance.first(guid: params[:service_instance_guid])
- resource_not_found!(:service_instance) unless service_instance && can_read_space?(service_instance.space)
+ resource_not_found!(:service_instance) unless service_instance && can_read_service_instance?(service_instance)
unauthorized! unless can_write_space?(service_instance.space)
message = VCAP::CloudController::ToManyRelationshipMessage.create_from_http_request(params[:body])
@@ -20,6 +39,7 @@ def share_service_instance
spaces = Space.where(guid: message.guids)
check_spaces_exist_and_are_readable!(message.guids, spaces)
check_spaces_are_writeable!(spaces)
+ ensure_not_sharing_to_self!(service_instance.space, spaces)
share = ServiceInstanceShare.new
share.create(service_instance, spaces, user_audit_info)
@@ -33,7 +53,7 @@ def unshare_service_instance
service_instance = ServiceInstance.first(guid: params[:service_instance_guid])
- resource_not_found!(:service_instance) unless service_instance && can_read_space?(service_instance.space)
+ resource_not_found!(:service_instance) unless service_instance && can_read_service_instance?(service_instance)
unauthorized! unless can_write_space?(service_instance.space)
space_guid = params[:space_guid]
@@ -51,12 +71,16 @@ def unshare_service_instance
private
+ def ensure_not_sharing_to_self!(service_instance_space, target_spaces)
+ unprocessable!('Service instances cannot be shared into the space where they were created') if target_spaces.include?(service_instance_space)
+ end
+
def check_spaces_are_writeable!(spaces)
unwriteable_spaces = spaces.reject do |space|
can_write?(space.guid)
end
- unauthorized! unless unwriteable_spaces.empty?
+ unauthorized! if unwriteable_spaces.any?
end
def check_spaces_exist_and_are_readable!(request_guids, found_spaces)
@@ -74,8 +98,16 @@ def check_spaces_exist_and_are_readable!(request_guids, found_spaces)
end
end
+ def can_read_service_instance?(service_instance)
+ readable_spaces = service_instance.shared_spaces + [service_instance.space]
+
+ readable_spaces.any? do |space|
+ can_read?(space.guid, space.organization_guid)
+ end
+ end
+
def can_read_space?(space)
- can_read?(space.guid, space.organization_guid)
+ can_read?(space.guid, space.organization.guid)
end
def can_write_space?(space)
diff --git a/app/fetchers/service_binding_list_fetcher.rb b/app/fetchers/service_binding_list_fetcher.rb
index 2939ccd5eac..fd49728ba31 100644
--- a/app/fetchers/service_binding_list_fetcher.rb
+++ b/app/fetchers/service_binding_list_fetcher.rb
@@ -16,6 +16,13 @@ def fetch_all
filter(dataset)
end
+ def self.fetch_service_instance_bindings_in_space(service_instance_guid, space_guid)
+ ServiceBinding.select_all(ServiceBinding.table_name).
+ join(:apps, guid: :app_guid).
+ where(apps__space_guid: space_guid).
+ where(service_bindings__service_instance_guid: service_instance_guid)
+ end
+
private
def filter(dataset)
diff --git a/app/fetchers/service_instance_list_fetcher.rb b/app/fetchers/service_instance_list_fetcher.rb
new file mode 100644
index 00000000000..01212a3f3b1
--- /dev/null
+++ b/app/fetchers/service_instance_list_fetcher.rb
@@ -0,0 +1,30 @@
+module VCAP::CloudController
+ class ServiceInstanceListFetcher
+ def fetch(message:, space_guids:)
+ source_space_instance_dataset = ServiceInstance.select_all(ServiceInstance.table_name).
+ join(Space.table_name, id: :space_id, guid: space_guids)
+
+ shared_instance_dataset = ServiceInstance.select_all(ServiceInstance.table_name).
+ join(:service_instance_shares, service_instance_guid: :guid, target_space_guid: space_guids)
+
+ dataset = source_space_instance_dataset.union(shared_instance_dataset, alias: :service_instances)
+
+ filter(dataset, message)
+ end
+
+ def fetch_all(message:)
+ dataset = ServiceInstance.dataset
+ filter(dataset, message)
+ end
+
+ private
+
+ def filter(dataset, message)
+ if message.requested?(:names)
+ dataset = dataset.where(service_instances__name: message.names)
+ end
+
+ dataset
+ end
+ end
+end
diff --git a/app/messages/service_instances/service_instances_list_message.rb b/app/messages/service_instances/service_instances_list_message.rb
new file mode 100644
index 00000000000..f368e921fe4
--- /dev/null
+++ b/app/messages/service_instances/service_instances_list_message.rb
@@ -0,0 +1,32 @@
+require 'messages/list_message'
+
+module VCAP::CloudController
+ class ServiceInstancesListMessage < ListMessage
+ ALLOWED_KEYS = [:page, :per_page, :order_by, :names].freeze
+
+ attr_accessor(*ALLOWED_KEYS)
+
+ validates_with NoAdditionalParamsValidator
+ validates :names, array: true, allow_nil: true
+
+ def initialize(params={})
+ super(params.symbolize_keys)
+ end
+
+ def self.from_params(params)
+ opts = params.dup
+ to_array! opts, 'names'
+ new(opts.symbolize_keys)
+ end
+
+ def valid_order_by_values
+ super << :name
+ end
+
+ private
+
+ def allowed_keys
+ ALLOWED_KEYS
+ end
+ end
+end
diff --git a/app/models/services/managed_service_instance.rb b/app/models/services/managed_service_instance.rb
index 6c27b9626e6..f28c8a35938 100644
--- a/app/models/services/managed_service_instance.rb
+++ b/app/models/services/managed_service_instance.rb
@@ -103,6 +103,10 @@ def route_service?
service.route_service?
end
+ def shareable?
+ service.shareable?
+ end
+
def volume_service?
service.volume_service?
end
diff --git a/app/models/services/service.rb b/app/models/services/service.rb
index 5737aa973f0..f9e3fda9d52 100644
--- a/app/models/services/service.rb
+++ b/app/models/services/service.rb
@@ -129,6 +129,14 @@ def route_service?
requires.include?('route_forwarding')
end
+ def shareable?
+ return false if extra.nil?
+ metadata = JSON.parse(extra)
+ metadata && metadata['shareable']
+ rescue JSON::ParserError
+ return false
+ end
+
def volume_service?
requires.include?('volume_mount')
end
diff --git a/app/models/services/service_instance.rb b/app/models/services/service_instance.rb
index b2aea99e2af..58c9449b507 100644
--- a/app/models/services/service_instance.rb
+++ b/app/models/services/service_instance.rb
@@ -68,6 +68,10 @@ def self.user_visibility_filter(user)
[:space, user.spaces_dataset],
[:space, user.audited_spaces_dataset],
[:space, user.managed_spaces_dataset],
+ [:shared_spaces, user.spaces_dataset],
+ [:shared_spaces, user.managed_spaces_dataset],
+ [:shared_spaces, user.audited_spaces_dataset],
+ [:shared_spaces, managed_organizations_spaces_dataset(user.managed_organizations_dataset)],
])
end
@@ -83,14 +87,27 @@ def managed_instance?
!user_provided_instance?
end
+ def name_clashes
+ proc do |_, instance|
+ next if instance.space_id.nil? || instance.name.nil?
+
+ clashes_with_shared_instance_names =
+ ServiceInstance.select_all(ServiceInstance.table_name).
+ join(:service_instance_shares, service_instance_guid: :guid, target_space_guid: instance.space_guid).
+ where(name: instance.name)
+
+ clashes_with_instance_names =
+ ServiceInstance.select_all(ServiceInstance.table_name).
+ where(space_id: instance.space_id, name: instance.name)
+
+ clashes_with_shared_instance_names.union(clashes_with_instance_names)
+ end
+ end
+
def validate
validates_presence :name
validates_presence :space
- validates_unique [:space_id, :name], where: (proc do |_, obj, arr|
- vals = arr.map { |x| obj.send(x) }
- next if vals.any?(&:nil?)
- ServiceInstance.where(arr.zip(vals))
- end)
+ validates_unique :name, where: name_clashes
validates_max_length 50, :name
validates_max_length 10_000, :syslog_drain_url, allow_nil: true
end
@@ -134,7 +151,7 @@ def credentials_with_serialization
alias_method_chain :credentials, 'serialization'
def in_suspended_org?
- space.in_suspended_org?
+ space&.in_suspended_org?
end
def after_create
@@ -167,10 +184,18 @@ def route_service?
false
end
+ def shareable?
+ false
+ end
+
def volume_service?
false
end
+ def shared?
+ shared_spaces.any?
+ end
+
def self.managed_organizations_spaces_dataset(managed_organizations_dataset)
VCAP::CloudController::Space.dataset.filter({ organization_id: managed_organizations_dataset.select(:organization_id) })
end
diff --git a/app/presenters/v2/service_instance_presenter.rb b/app/presenters/v2/service_instance_presenter.rb
index a020fdae8f2..8f688d6a781 100644
--- a/app/presenters/v2/service_instance_presenter.rb
+++ b/app/presenters/v2/service_instance_presenter.rb
@@ -28,6 +28,8 @@ def entity_hash(controller, obj, opts, depth, parents, orphans=nil)
obj_hash['service_plan_guid'] = service_plan.guid
obj_hash['service_guid'] = service_plan.service.guid
rel_hash['service_url'] = "/v2/services/#{service_plan.service.guid}"
+ rel_hash['shared_from_url'] = "/v2/service_instances/#{obj.guid}/shared_from"
+ rel_hash['shared_to_url'] = "/v2/service_instances/#{obj.guid}/shared_to"
end
obj_hash.merge!(rel_hash)
diff --git a/app/presenters/v2/service_instance_shared_from_presenter.rb b/app/presenters/v2/service_instance_shared_from_presenter.rb
new file mode 100644
index 00000000000..405ac03c3bd
--- /dev/null
+++ b/app/presenters/v2/service_instance_shared_from_presenter.rb
@@ -0,0 +1,14 @@
+module CloudController
+ module Presenters
+ module V2
+ class ServiceInstanceSharedFromPresenter
+ def to_hash(space)
+ {
+ 'space_name' => space.name,
+ 'organization_name' => space.organization.name
+ }
+ end
+ end
+ end
+ end
+end
diff --git a/app/presenters/v2/service_instance_shared_to_presenter.rb b/app/presenters/v2/service_instance_shared_to_presenter.rb
new file mode 100644
index 00000000000..0f3229579bf
--- /dev/null
+++ b/app/presenters/v2/service_instance_shared_to_presenter.rb
@@ -0,0 +1,13 @@
+require 'presenters/v2/service_instance_shared_from_presenter'
+
+module CloudController
+ module Presenters
+ module V2
+ class ServiceInstanceSharedToPresenter < ServiceInstanceSharedFromPresenter
+ def to_hash(space, bound_app_count)
+ super(space).merge({ 'bound_app_count' => bound_app_count })
+ end
+ end
+ end
+ end
+end
diff --git a/app/presenters/v3/paginated_list_presenter.rb b/app/presenters/v3/paginated_list_presenter.rb
index 48ad1f07a3f..238e18a7729 100644
--- a/app/presenters/v3/paginated_list_presenter.rb
+++ b/app/presenters/v3/paginated_list_presenter.rb
@@ -6,6 +6,7 @@
require 'presenters/v3/process_presenter'
require 'presenters/v3/route_mapping_presenter'
require 'presenters/v3/service_binding_presenter'
+require 'presenters/v3/service_instance_presenter'
require 'presenters/v3/task_presenter'
require 'presenters/v3/organization_presenter'
require 'presenters/v3/space_presenter'
@@ -24,7 +25,8 @@ class PaginatedListPresenter
'PackageModel' => VCAP::CloudController::Presenters::V3::PackagePresenter,
'RouteMappingModel' => VCAP::CloudController::Presenters::V3::RouteMappingPresenter,
'ServiceBinding' => VCAP::CloudController::Presenters::V3::ServiceBindingPresenter,
- 'TaskModel' => VCAP::CloudController::Presenters::V3::TaskPresenter,
+ 'ManagedServiceInstance' => VCAP::CloudController::Presenters::V3::ServiceInstancePresenter,
+ 'TaskModel' => VCAP::CloudController::Presenters::V3::TaskPresenter,
}.freeze
def initialize(dataset:, path:, message: nil, show_secrets: false)
diff --git a/app/presenters/v3/service_instance_presenter.rb b/app/presenters/v3/service_instance_presenter.rb
new file mode 100644
index 00000000000..537d1969e8a
--- /dev/null
+++ b/app/presenters/v3/service_instance_presenter.rb
@@ -0,0 +1,24 @@
+require 'presenters/v3/base_presenter'
+
+module VCAP::CloudController
+ module Presenters
+ module V3
+ class ServiceInstancePresenter < BasePresenter
+ def to_hash
+ {
+ guid: service_instance.guid,
+ created_at: service_instance.created_at,
+ updated_at: service_instance.updated_at,
+ name: service_instance.name
+ }
+ end
+
+ private
+
+ def service_instance
+ @resource
+ end
+ end
+ end
+ end
+end
diff --git a/config/routes.rb b/config/routes.rb
index 729606e0a8a..34661d0e55b 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -119,6 +119,7 @@
get '/apps/:app_guid/tasks', to: 'tasks#index'
# service_instances
+ get '/service_instances', to: 'service_instances_v3#index'
post '/service_instances/:service_instance_guid/relationships/shared_spaces', to: 'service_instances_v3#share_service_instance'
delete '/service_instances/:service_instance_guid/relationships/shared_spaces/:space_guid', to: 'service_instances_v3#unshare_service_instance'
end
diff --git a/docs/v2/index.html b/docs/v2/index.html
index b2e77eb7adc..6896378863e 100644
--- a/docs/v2/index.html
+++ b/docs/v2/index.html
@@ -875,6 +875,12 @@
tags
@@ -549,7 +581,9 @@ Body
"service_plan_url": "/v2/service_plans/4ec73bf4-9f3a-44c7-bbac-61ee9cb5a511",
"service_bindings_url": "/v2/service_instances/a34f1423-4b84-4727-ab49-3f1522c4cb16/service_bindings",
"service_keys_url": "/v2/service_instances/a34f1423-4b84-4727-ab49-3f1522c4cb16/service_keys",
- "routes_url": "/v2/service_instances/a34f1423-4b84-4727-ab49-3f1522c4cb16/routes"
+ "routes_url": "/v2/service_instances/a34f1423-4b84-4727-ab49-3f1522c4cb16/routes",
+ "shared_from_url": "/v2/service_instances/0d632575-bb06-4ea5-bb19-a451a9644d92/shared_from",
+ "shared_to_url": "/v2/service_instances/0d632575-bb06-4ea5-bb19-a451a9644d92/shared_to"
}
}
diff --git a/docs/v2/service_plans/list_all_service_instances_for_the_service_plan.html b/docs/v2/service_plans/list_all_service_instances_for_the_service_plan.html
index 5a7aab99fdb..770bcaf779f 100644
--- a/docs/v2/service_plans/list_all_service_instances_for_the_service_plan.html
+++ b/docs/v2/service_plans/list_all_service_instances_for_the_service_plan.html
@@ -303,7 +303,9 @@ Body
"service_plan_url": "/v2/service_plans/85615ea0-9d23-4de8-aabd-89bffcce39d5",
"service_bindings_url": "/v2/service_instances/0fac6687-69fd-4567-afb0-dd39503523ff/service_bindings",
"service_keys_url": "/v2/service_instances/0fac6687-69fd-4567-afb0-dd39503523ff/service_keys",
- "routes_url": "/v2/service_instances/0fac6687-69fd-4567-afb0-dd39503523ff/routes"
+ "routes_url": "/v2/service_instances/0fac6687-69fd-4567-afb0-dd39503523ff/routes",
+ "shared_from_url": "/v2/service_instances/9547e9ed-e460-4abe-bda3-7070b9835917/shared_from",
+ "shared_to_url": "/v2/service_instances/9547e9ed-e460-4abe-bda3-7070b9835917/shared_to"
}
}
]
diff --git a/docs/v2/spaces/list_all_service_instances_for_the_space.html b/docs/v2/spaces/list_all_service_instances_for_the_space.html
index 19145598b74..e3cd4cae7f9 100644
--- a/docs/v2/spaces/list_all_service_instances_for_the_space.html
+++ b/docs/v2/spaces/list_all_service_instances_for_the_space.html
@@ -302,7 +302,9 @@ Body
"service_plan_url": "/v2/service_plans/fcf57f7f-3c51-49b2-b252-dc24e0f7dcab",
"service_bindings_url": "/v2/service_instances/9547e9ed-e460-4abe-bda3-7070b9835917/service_bindings",
"service_keys_url": "/v2/service_instances/9547e9ed-e460-4abe-bda3-7070b9835917/service_keys",
- "routes_url": "/v2/service_instances/9547e9ed-e460-4abe-bda3-7070b9835917/routes"
+ "routes_url": "/v2/service_instances/9547e9ed-e460-4abe-bda3-7070b9835917/routes",
+ "shared_from_url": "/v2/service_instances/9547e9ed-e460-4abe-bda3-7070b9835917/shared_from",
+ "shared_to_url": "/v2/service_instances/9547e9ed-e460-4abe-bda3-7070b9835917/shared_to"
}
}
]
diff --git a/docs/v3/source/includes/api_resources/_service_instances.erb b/docs/v3/source/includes/api_resources/_service_instances.erb
index 3868b3df80f..a9a15f49183 100644
--- a/docs/v3/source/includes/api_resources/_service_instances.erb
+++ b/docs/v3/source/includes/api_resources/_service_instances.erb
@@ -18,3 +18,28 @@
}
}
<% end %>
+
+<% content_for :paginated_list_of_service_instances do %>
+{
+ "pagination": {
+ "total_results": 1,
+ "total_pages": 1,
+ "first": {
+ "href": "https://api.example.org/v3/service_instances?page=1&per_page=50"
+ },
+ "last": {
+ "href": "https://api.example.org/v3/service_instances?page=1&per_page=50"
+ },
+ "next": null,
+ "previous": null
+ },
+ "resources": [
+ {
+ "guid": "d4c91047-7b29-4fda-b7f9-04033e5c9c9f",
+ "created_at": "2017-02-02T00:14:30Z",
+ "updated_at": "2017-02-02T00:14:30Z",
+ "name": "my_service_instance"
+ }
+ ]
+}
+<% end %>
diff --git a/docs/v3/source/includes/experimental_resources/service_instances/_list.md.erb b/docs/v3/source/includes/experimental_resources/service_instances/_list.md.erb
new file mode 100644
index 00000000000..de938427740
--- /dev/null
+++ b/docs/v3/source/includes/experimental_resources/service_instances/_list.md.erb
@@ -0,0 +1,36 @@
+### List service instances
+
+```
+Example Request
+```
+
+```shell
+curl "https://api.example.org/v3/service_instances" \
+ -X GET \
+ -H "Authorization: bearer [token]"
+```
+
+```
+Example Response
+```
+
+```http
+HTTP/1.1 200 OK
+Content-Type: application/json
+
+<%= yield_content :paginated_list_of_service_instances, '/v3/service_instances' %>
+```
+This endpoint retrieves the service instances the user has access to. At the moment, this endpoint only returns managed service instances. This may change in the future.
+
+This includes access granted by service instance sharing.
+
+#### Definition
+`GET /v3/service_instances`
+
+#### Query Parameters
+
+Name | Type | Description
+---- | ---- | ------------
+**name** | _list of strings_ | Comma-delimited list of service instance names to filter by.
+**page** | _integer_ | Page to display. Valid values are integers >= 1.
+**per_page** | _integer_ | Number of results per page. Valid values are 1 through 5000.
diff --git a/docs/v3/source/index.md b/docs/v3/source/index.md
index 75e2112c34b..b035577261e 100644
--- a/docs/v3/source/index.md
+++ b/docs/v3/source/index.md
@@ -137,6 +137,7 @@ includes:
- experimental_resources/service_bindings/delete
- experimental_resources/service_bindings/list
- experimental_resources/service_instances/header
+ - experimental_resources/service_instances/list
- experimental_resources/service_instances/share_to_space
- experimental_resources/service_instances/unshare_from_space
search: true
diff --git a/spec/request/service_instances_spec.rb b/spec/request/service_instances_spec.rb
index c9e0a68bb80..5d98657fb46 100644
--- a/spec/request/service_instances_spec.rb
+++ b/spec/request/service_instances_spec.rb
@@ -2,11 +2,127 @@
RSpec.describe 'Service Instances' do
let(:user_email) { 'user@email.example.com' }
- let(:user_name) { 'sharer_username' }
+ let(:user_name) { 'username' }
let(:user) { VCAP::CloudController::User.make }
+ let(:user_header) { headers_for(user) }
let(:admin_header) { admin_headers_for(user, email: user_email, user_name: user_name) }
+ let(:space) { VCAP::CloudController::Space.make }
let(:target_space) { VCAP::CloudController::Space.make }
- let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make }
+ let!(:service_instance1) { VCAP::CloudController::ManagedServiceInstance.make(space: space, name: 'rabbitmq') }
+ let!(:service_instance2) { VCAP::CloudController::ManagedServiceInstance.make(space: space, name: 'redis') }
+ let!(:service_instance3) { VCAP::CloudController::ManagedServiceInstance.make(space: space, name: 'mysql') }
+
+ describe 'GET /v3/service_instances' do
+ it 'returns a paginated list of service instances the user has access to' do
+ set_current_user_as_role(role: 'space_developer', org: space.organization, space: space, user: user)
+ get '/v3/service_instances?per_page=2&order_by=name', nil, user_header
+ expect(last_response.status).to eq(200)
+
+ parsed_response = MultiJson.load(last_response.body)
+ expect(parsed_response).to be_a_response_like(
+ {
+ 'pagination' => {
+ 'total_results' => 3,
+ 'total_pages' => 2,
+ 'first' => {
+ 'href' => "#{link_prefix}/v3/service_instances?order_by=name&page=1&per_page=2"
+ },
+ 'last' => {
+ 'href' => "#{link_prefix}/v3/service_instances?order_by=name&page=2&per_page=2"
+ },
+ 'next' => {
+ 'href' => "#{link_prefix}/v3/service_instances?order_by=name&page=2&per_page=2"
+ },
+ 'previous' => nil
+ },
+ 'resources' => [
+ {
+ 'guid' => service_instance3.guid,
+ 'name' => service_instance3.name,
+ 'created_at' => iso8601,
+ 'updated_at' => iso8601,
+ },
+ {
+ 'guid' => service_instance1.guid,
+ 'name' => service_instance1.name,
+ 'created_at' => iso8601,
+ 'updated_at' => iso8601,
+ }
+ ]
+ }
+ )
+ end
+
+ it 'returns a paginated list of service instances filtered by name' do
+ set_current_user_as_role(role: 'space_developer', org: space.organization, space: space, user: user)
+ get '/v3/service_instances?per_page=2&names=redis', nil, user_header
+ expect(last_response.status).to eq(200)
+
+ parsed_response = MultiJson.load(last_response.body)
+ expect(parsed_response).to be_a_response_like(
+ {
+ 'pagination' => {
+ 'total_results' => 1,
+ 'total_pages' => 1,
+ 'first' => {
+ 'href' => "#{link_prefix}/v3/service_instances?names=redis&page=1&per_page=2"
+ },
+ 'last' => {
+ 'href' => "#{link_prefix}/v3/service_instances?names=redis&page=1&per_page=2"
+ },
+ 'next' => nil,
+ 'previous' => nil
+ },
+ 'resources' => [
+ {
+ 'guid' => service_instance2.guid,
+ 'name' => service_instance2.name,
+ 'created_at' => iso8601,
+ 'updated_at' => iso8601,
+ }
+ ]
+ }
+ )
+ end
+
+ context 'when a user has access to a shared service instance' do
+ before do
+ service_instance1.add_shared_space(target_space)
+ end
+
+ it 'returns a paginated list of service instances the user has access to' do
+ set_current_user_as_role(role: 'space_developer', org: target_space.organization, space: target_space, user: user)
+ get '/v3/service_instances?per_page=2&order_by=name', nil, user_header
+ expect(last_response.status).to eq(200)
+
+ parsed_response = MultiJson.load(last_response.body)
+ expect(parsed_response).to be_a_response_like(
+ {
+ 'pagination' => {
+ 'total_results' => 1,
+ 'total_pages' => 1,
+ 'first' => {
+ 'href' => "#{link_prefix}/v3/service_instances?order_by=name&page=1&per_page=2"
+ },
+ 'last' => {
+ 'href' => "#{link_prefix}/v3/service_instances?order_by=name&page=1&per_page=2"
+ },
+ 'next' => nil,
+ 'previous' => nil
+ },
+ 'resources' => [
+ {
+ 'guid' => service_instance1.guid,
+ 'name' => service_instance1.name,
+ 'created_at' => iso8601,
+ 'updated_at' => iso8601,
+ }
+ ]
+ }
+ )
+ end
+ end
+ end
describe 'POST /v3/service_instances/:guid/relationships/shared_spaces' do
before do
@@ -20,7 +136,7 @@
]
}
- post "/v3/service_instances/#{service_instance.guid}/relationships/shared_spaces", share_request.to_json, admin_header
+ post "/v3/service_instances/#{service_instance1.guid}/relationships/shared_spaces", share_request.to_json, admin_header
parsed_response = MultiJson.load(last_response.body)
expect(last_response.status).to eq(200)
@@ -30,8 +146,8 @@
{ 'guid' => target_space.guid }
],
'links' => {
- 'self' => { 'href' => "#{link_prefix}/v3/service_instances/#{service_instance.guid}/relationships/shared_spaces" },
- 'related' => { 'href' => "#{link_prefix}/v3/service_instances/#{service_instance.guid}/shared_spaces" },
+ 'self' => { 'href' => "#{link_prefix}/v3/service_instances/#{service_instance1.guid}/relationships/shared_spaces" },
+ 'related' => { 'href' => "#{link_prefix}/v3/service_instances/#{service_instance1.guid}/shared_spaces" },
}
}
@@ -44,14 +160,36 @@
actor_type: 'user',
actor_name: user_email,
actor_username: user_name,
- actee: service_instance.guid,
+ actee: service_instance1.guid,
actee_type: 'service_instance',
- actee_name: service_instance.name,
- space_guid: service_instance.space.guid,
- organization_guid: service_instance.space.organization.guid
+ actee_name: service_instance1.name,
+ space_guid: space.guid,
+ organization_guid: space.organization.guid
})
expect(event.metadata['target_space_guids']).to eq([target_space.guid])
end
+
+ context 'when the service offering has shareable false' do
+ before do
+ service_instance1.service.extra = { shareable: false }.to_json
+ service_instance1.service.save
+ end
+
+ it 'fails to share' do
+ share_request = {
+ 'data' => [
+ { 'guid' => target_space.guid }
+ ]
+ }
+
+ post "/v3/service_instances/#{service_instance1.guid}/relationships/shared_spaces", share_request.to_json, admin_header
+
+ expect(last_response.status).to eq(400)
+ parsed_response = MultiJson.load(last_response.body)
+ expect(parsed_response['errors'].first['code']).to eq(390003)
+ expect(parsed_response['errors'].first['title']).to eq('CF-ServiceShareIsDisabled')
+ end
+ end
end
describe 'DELETE /v3/service_instances/:guid/relationships/shared_spaces/:space-guid' do
@@ -68,12 +206,12 @@
]
}
- post "/v3/service_instances/#{service_instance.guid}/relationships/shared_spaces", share_request.to_json, admin_header
+ post "/v3/service_instances/#{service_instance1.guid}/relationships/shared_spaces", share_request.to_json, admin_header
expect(last_response.status).to eq(200)
end
it 'unshares the service instance from the target space' do
- delete "/v3/service_instances/#{service_instance.guid}/relationships/shared_spaces/#{target_space.guid}", nil, admin_header
+ delete "/v3/service_instances/#{service_instance1.guid}/relationships/shared_spaces/#{target_space.guid}", nil, admin_header
expect(last_response.status).to eq(204)
event = VCAP::CloudController::Event.last
@@ -83,23 +221,23 @@
actor_type: 'user',
actor_name: user_email,
actor_username: user_name,
- actee: service_instance.guid,
+ actee: service_instance1.guid,
actee_type: 'service_instance',
- actee_name: service_instance.name,
- space_guid: service_instance.space.guid,
- organization_guid: service_instance.space.organization.guid
+ actee_name: service_instance1.name,
+ space_guid: space.guid,
+ organization_guid: space.organization.guid
})
expect(event.metadata['target_space_guid']).to eq(target_space.guid)
end
it 'deletes associated bindings in target space when service instance is unshared' do
process = VCAP::CloudController::ProcessModelFactory.make(diego: false, space: target_space)
- service_binding = VCAP::CloudController::ServiceBinding.make(service_instance: service_instance, app: process.app, credentials: { secret: 'key' })
+ service_binding = VCAP::CloudController::ServiceBinding.make(service_instance: service_instance1, app: process.app, credentials: { secret: 'key' })
get "/v2/service_bindings/#{service_binding.guid}", nil, admin_header
expect(last_response.status).to eq(200)
- delete "/v3/service_instances/#{service_instance.guid}/relationships/shared_spaces/#{target_space.guid}", nil, admin_header
+ delete "/v3/service_instances/#{service_instance1.guid}/relationships/shared_spaces/#{target_space.guid}", nil, admin_header
expect(last_response.status).to eq(204)
get "/v2/service_bindings/#{service_binding.guid}", nil, admin_header
diff --git a/spec/request/v2/service_bindings_spec.rb b/spec/request/v2/service_bindings_spec.rb
index 11aea28425a..ce50ba51bb5 100644
--- a/spec/request/v2/service_bindings_spec.rb
+++ b/spec/request/v2/service_bindings_spec.rb
@@ -197,7 +197,9 @@
'service_plan_url' => "/v2/service_plans/#{service_instance.service_plan.guid}",
'service_bindings_url' => "/v2/service_instances/#{service_instance.guid}/service_bindings",
'service_keys_url' => "/v2/service_instances/#{service_instance.guid}/service_keys",
- 'routes_url' => "/v2/service_instances/#{service_instance.guid}/routes"
+ 'routes_url' => "/v2/service_instances/#{service_instance.guid}/routes",
+ 'shared_from_url' => "/v2/service_instances/#{service_instance.guid}/shared_from",
+ 'shared_to_url' => "/v2/service_instances/#{service_instance.guid}/shared_to",
}
}
}
diff --git a/spec/request/v2/service_instances_spec.rb b/spec/request/v2/service_instances_spec.rb
index 8708ea9e454..cb4ba70c566 100644
--- a/spec/request/v2/service_instances_spec.rb
+++ b/spec/request/v2/service_instances_spec.rb
@@ -54,7 +54,9 @@
'service_plan_url' => "/v2/service_plans/#{service_plan.guid}",
'service_bindings_url' => "/v2/service_instances/#{service_instance.guid}/service_bindings",
'service_keys_url' => "/v2/service_instances/#{service_instance.guid}/service_keys",
- 'routes_url' => "/v2/service_instances/#{service_instance.guid}/routes"
+ 'routes_url' => "/v2/service_instances/#{service_instance.guid}/routes",
+ 'shared_from_url' => "/v2/service_instances/#{service_instance.guid}/shared_from",
+ 'shared_to_url' => "/v2/service_instances/#{service_instance.guid}/shared_to",
}
}
)
@@ -96,7 +98,9 @@
'service_url' => "/v2/services/#{service_instance.service.guid}",
'service_bindings_url' => "/v2/service_instances/#{service_instance.guid}/service_bindings",
'service_keys_url' => "/v2/service_instances/#{service_instance.guid}/service_keys",
- 'routes_url' => "/v2/service_instances/#{service_instance.guid}/routes"
+ 'routes_url' => "/v2/service_instances/#{service_instance.guid}/routes",
+ 'shared_from_url' => "/v2/service_instances/#{service_instance.guid}/shared_from",
+ 'shared_to_url' => "/v2/service_instances/#{service_instance.guid}/shared_to",
}
}
)
@@ -137,7 +141,9 @@
'service_url' => "/v2/services/#{service_instance.service.guid}",
'service_bindings_url' => "/v2/service_instances/#{service_instance.guid}/service_bindings",
'service_keys_url' => "/v2/service_instances/#{service_instance.guid}/service_keys",
- 'routes_url' => "/v2/service_instances/#{service_instance.guid}/routes"
+ 'routes_url' => "/v2/service_instances/#{service_instance.guid}/routes",
+ 'shared_from_url' => "/v2/service_instances/#{service_instance.guid}/shared_from",
+ 'shared_to_url' => "/v2/service_instances/#{service_instance.guid}/shared_to",
}
}
)
@@ -145,4 +151,126 @@
end
end
end
+
+ describe 'GET /v2/service_instances/:service_instance_guid/shared_from' do
+ let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space: space) }
+
+ before do
+ service_instance.add_shared_space(VCAP::CloudController::Space.make)
+ end
+
+ it 'returns data about the source space and org' do
+ get "v2/service_instances/#{service_instance.guid}/shared_from", nil, admin_headers
+
+ expect(last_response.status).to eq(200), last_response.body
+
+ parsed_response = MultiJson.load(last_response.body)
+ expect(parsed_response).to be_a_response_like({
+ 'space_name' => space.name,
+ 'organization_name' => space.organization.name
+ })
+ end
+
+ context 'when the user is a member of the space where a service instance has been shared to' do
+ let(:other_space) { VCAP::CloudController::Space.make }
+ let(:other_user) { make_developer_for_space(other_space) }
+ let(:req_body) do
+ {
+ data: [
+ { guid: other_space.guid }
+ ]
+ }.to_json
+ end
+
+ before do
+ VCAP::CloudController::FeatureFlag.make(name: 'service_instance_sharing', enabled: true, error_message: nil)
+
+ other_space.organization.add_user(user)
+ other_space.add_developer(user)
+
+ post "v3/service_instances/#{service_instance.guid}/relationships/shared_spaces", req_body, headers_for(user)
+ expect(last_response.status).to eq(200)
+ end
+
+ it 'returns data about the source space and org' do
+ get "v2/service_instances/#{service_instance.guid}/shared_from", nil, headers_for(other_user)
+
+ expect(last_response.status).to eq(200)
+
+ parsed_response = MultiJson.load(last_response.body)
+ expect(parsed_response).to be_a_response_like({
+ 'space_name' => space.name,
+ 'organization_name' => space.organization.name
+ })
+ end
+ end
+ end
+
+ describe 'GET /v2/service_instances/:service_instance_guid/shared_to' do
+ let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space: space) }
+ let(:space1) { VCAP::CloudController::Space.make }
+ let(:space2) { VCAP::CloudController::Space.make }
+
+ before do
+ service_instance.add_shared_space(space1)
+ service_instance.add_shared_space(space2)
+ end
+
+ it 'returns data about the source space, org, and bound_app_count' do
+ get "v2/service_instances/#{service_instance.guid}/shared_to", nil, admin_headers
+
+ expect(last_response.status).to eq(200)
+
+ parsed_response = MultiJson.load(last_response.body)
+ expect(parsed_response).to be_a_response_like(
+ {
+ 'total_results' => 2,
+ 'total_pages' => 1,
+ 'prev_url' => nil,
+ 'next_url' => nil,
+ 'resources' => [
+ {
+ 'space_name' => space1.name,
+ 'organization_name' => space1.organization.name,
+ 'bound_app_count' => 0
+ },
+ {
+ 'space_name' => space2.name,
+ 'organization_name' => space2.organization.name,
+ 'bound_app_count' => 0
+ }
+ ]
+ }
+ )
+ end
+ end
+
+ describe 'DELETE /v2/service_instance/:guid' do
+ let(:originating_space) { VCAP::CloudController::Space.make }
+ let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space: originating_space) }
+
+ context 'when the service instance has been shared' do
+ before do
+ allow(VCAP::Services::ServiceBrokers::V2::Client).to receive(:new) do |*args, **kwargs, &block|
+ FakeServiceBrokerV2Client.new(*args, **kwargs, &block)
+ end
+
+ set_current_user_as_admin
+ service_instance.add_shared_space(space)
+ end
+
+ it 'fails with an appropriate response' do
+ delete "v2/service_instances/#{service_instance.guid}", nil, admin_headers
+
+ expect(last_response.status).to eq(400)
+
+ parsed_response = MultiJson.load(last_response.body)
+ expect(parsed_response['description']).to eq 'Service instances must be unshared before they can be deleted. ' \
+ "Unsharing #{service_instance.name} will automatically delete any bindings " \
+ 'that have been made to applications in other spaces.'
+ expect(parsed_response['error_code']).to eq 'CF-ServiceInstanceDeletionSharesExists'
+ expect(parsed_response['code']).to eq 390002
+ end
+ end
+ end
end
diff --git a/spec/request/v2/spaces_spec.rb b/spec/request/v2/spaces_spec.rb
index bdd08d3d8ed..f163cf306fa 100644
--- a/spec/request/v2/spaces_spec.rb
+++ b/spec/request/v2/spaces_spec.rb
@@ -132,7 +132,7 @@
space.add_developer(user)
end
- it 'lists the isolation segment for SpaceDvelopers' do
+ it 'lists the isolation segment for SpaceDevelopers' do
get "/v2/spaces/#{space.guid}", {}, headers_for(user)
expect(last_response.status).to eq(200)
@@ -170,6 +170,63 @@
end
end
+ describe 'GET /v2/spaces/:guid/service_instances' do
+ let(:originating_space) { VCAP::CloudController::Space.make }
+ let(:shared_service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space: originating_space) }
+ let(:space) { VCAP::CloudController::Space.make }
+
+ before do
+ originating_space.organization.add_user(user)
+ originating_space.add_developer(user)
+ space.organization.add_user(user)
+ space.add_developer(user)
+
+ shared_service_instance.add_shared_space(space)
+ end
+
+ it 'shows the shared service instances associated with the space' do
+ get "/v2/spaces/#{space.guid}/service_instances", {}, headers_for(user)
+
+ expect(last_response.status).to eq(200)
+ parsed_response = MultiJson.load(last_response.body)
+
+ expect(parsed_response).to be_a_response_like({
+ 'total_results' => 1,
+ 'total_pages' => 1,
+ 'prev_url' => nil,
+ 'next_url' => nil,
+ 'resources' => [{
+ 'metadata' => {
+ 'guid' => shared_service_instance.guid,
+ 'url' => "/v2/service_instances/#{shared_service_instance.guid}",
+ 'created_at' => iso8601,
+ 'updated_at' => iso8601,
+ },
+ 'entity' => {
+ 'name' => shared_service_instance.name,
+ 'credentials' => shared_service_instance.credentials,
+ 'service_plan_guid' => shared_service_instance.service_plan_guid,
+ 'space_guid' => originating_space.guid,
+ 'gateway_data' => nil,
+ 'dashboard_url' => nil,
+ 'type' => 'managed_service_instance',
+ 'last_operation' => nil,
+ 'tags' => [],
+ 'service_guid' => shared_service_instance.service_plan.service_guid,
+ 'space_url' => "/v2/spaces/#{originating_space.guid}",
+ 'service_plan_url' => "/v2/service_plans/#{shared_service_instance.service_plan_guid}",
+ 'service_bindings_url' => "/v2/service_instances/#{shared_service_instance.guid}/service_bindings",
+ 'service_keys_url' => "/v2/service_instances/#{shared_service_instance.guid}/service_keys",
+ 'routes_url' => "/v2/service_instances/#{shared_service_instance.guid}/routes",
+ 'service_url' => "/v2/services/#{shared_service_instance.service_plan.service_guid}",
+ 'shared_from_url' => "/v2/service_instances/#{shared_service_instance.guid}/shared_from",
+ 'shared_to_url' => "/v2/service_instances/#{shared_service_instance.guid}/shared_to",
+ }
+ }]
+ })
+ end
+ end
+
describe 'DELETE /v2/spaces/:guid/unmapped_routes' do
let(:space) { VCAP::CloudController::Space.make(organization: org) }
let(:process) { VCAP::CloudController::ProcessModelFactory.make(state: 'STARTED') }
diff --git a/spec/support/fakes/blueprints.rb b/spec/support/fakes/blueprints.rb
index 403c2ed6a90..78d2c49052d 100644
--- a/spec/support/fakes/blueprints.rb
+++ b/spec/support/fakes/blueprints.rb
@@ -205,6 +205,7 @@ module VCAP::CloudController
active { true }
service_broker { ServiceBroker.make }
description { Sham.description } # remove hack
+ extra { '{"shareable": true}' }
end
Service.blueprint(:routing) do
diff --git a/spec/support/fakes/fake_service_broker_v2_client.rb b/spec/support/fakes/fake_service_broker_v2_client.rb
index 45ccacbdf04..2e23581aa54 100644
--- a/spec/support/fakes/fake_service_broker_v2_client.rb
+++ b/spec/support/fakes/fake_service_broker_v2_client.rb
@@ -46,6 +46,16 @@ def provision(_instance, arbitrary_parameters: {}, accepts_incomplete: false)
}
end
+ def deprovision(_instance, arbitrary_parameters: {}, accepts_incomplete: false)
+ {
+ last_operation: {
+ type: 'delete',
+ description: '',
+ state: 'succeeded'
+ }
+ }
+ end
+
def bind(_binding, _arbitrary_parameters)
{
credentials: credentials,
diff --git a/spec/support/matchers/sequel_validations.rb b/spec/support/matchers/sequel_validations.rb
index 7e40682eb99..c4f397a2e07 100644
--- a/spec/support/matchers/sequel_validations.rb
+++ b/spec/support/matchers/sequel_validations.rb
@@ -48,7 +48,7 @@
duplicate_object[attr] = source_obj[attr]
end
unless duplicate_object.valid?
- errors_key = attributes.length > 1 ? attributes : attributes.first
+ errors_key = options[:error_key] || (attributes.length > 1 ? attributes : attributes.first)
errors = duplicate_object.errors.on(errors_key)
expected_error = options[:message] || :unique
errors && errors.include?(expected_error)
diff --git a/spec/unit/access/service_instance_access_spec.rb b/spec/unit/access/service_instance_access_spec.rb
index 1361e7236d4..7ed67887254 100644
--- a/spec/unit/access/service_instance_access_spec.rb
+++ b/spec/unit/access/service_instance_access_spec.rb
@@ -149,6 +149,63 @@ module VCAP::CloudController
end
end
+ context 'space developer in a space that the service instance has been shared into' do
+ before do
+ org.add_user(user)
+ target_space = VCAP::CloudController::Space.make(organization: org)
+ target_space.add_developer(user)
+ service_instance.add_shared_space(target_space)
+ end
+
+ context 'when the space of the service instance is visible' do
+ it_behaves_like :read_only_access do
+ let(:object) { service_instance }
+ end
+
+ it 'does NOT allow the user to have manage permissions of the service instance' do
+ expect(subject).to_not allow_op_on_object(:manage_permissions, service_instance)
+ end
+
+ it 'allows the user to have read permissions of the service instance' do
+ expect(subject).to allow_op_on_object(:read_permissions, service_instance)
+ end
+
+ it 'does NOT allow the user to read default credentials of the service instance' do
+ expect(subject).not_to allow_op_on_object(:read_env, service_instance)
+ end
+
+ it 'returns false for purge' do
+ expect(subject).not_to allow_op_on_object(:purge, service_instance)
+ end
+ end
+
+ context 'when the space of the service instance is not visible' do
+ before do
+ service_instance.space = nil
+ end
+
+ it_behaves_like :read_only_access do
+ let(:object) { service_instance }
+ end
+
+ it 'does NOT allow the user to have manage permissions of the service instance' do
+ expect(subject).to_not allow_op_on_object(:manage_permissions, service_instance)
+ end
+
+ it 'allows the user to have read permissions of the service instance' do
+ expect(subject).to allow_op_on_object(:read_permissions, service_instance)
+ end
+
+ it 'does NOT allow the user to read default credentials of the service instance' do
+ expect(subject).not_to allow_op_on_object(:read_env, service_instance)
+ end
+
+ it 'returns false for purge' do
+ expect(subject).not_to allow_op_on_object(:purge, service_instance)
+ end
+ end
+ end
+
context 'organization manager (defensive)' do
before { org.add_manager(user) }
diff --git a/spec/unit/actions/service_instance_share_spec.rb b/spec/unit/actions/service_instance_share_spec.rb
index 12f938d4c17..b63b54293b9 100644
--- a/spec/unit/actions/service_instance_share_spec.rb
+++ b/spec/unit/actions/service_instance_share_spec.rb
@@ -29,6 +29,79 @@ module VCAP::CloudController
expect(Repositories::ServiceInstanceShareEventRepository).to have_received(:record_share_event).with(
service_instance, [target_space1.guid, target_space2.guid], user_audit_info)
end
+
+ context 'when a share already exists' do
+ before do
+ service_instance.add_shared_space(target_space1)
+ end
+
+ it 'is idempotent' do
+ shared_instance = service_instance_share.create(service_instance, [target_space1], user_audit_info)
+ expect(shared_instance.shared_spaces.length).to eq 1
+ end
+ end
+
+ context 'when sharing one space from the list of spaces fails' do
+ before do
+ allow(service_instance).to receive(:add_shared_space).with(target_space1).and_call_original
+ allow(service_instance).to receive(:add_shared_space).with(target_space2).and_raise('db failure')
+ end
+
+ it 'does not share with any spaces' do
+ expect {
+ service_instance_share.create(service_instance, [target_space1, target_space2], user_audit_info)
+ }.to raise_error('db failure')
+
+ instance = ServiceInstance.find(guid: service_instance.guid)
+
+ expect(instance.shared_spaces.length).to eq 0
+ end
+
+ it 'does not audit any share events' do
+ expect(Repositories::ServiceInstanceShareEventRepository).to_not receive(:record_share_event)
+
+ expect {
+ service_instance_share.create(service_instance, [target_space1, target_space2], user_audit_info)
+ }.to raise_error('db failure')
+ end
+ end
+
+ context 'when source space is included in list of target spaces' do
+ before do
+ allow(service_instance).to receive(:add_shared_space).with(target_space1).and_call_original
+ allow(service_instance).to receive(:add_shared_space).with(service_instance.space).and_raise('db failure')
+ end
+
+ it 'does not share with any spaces' do
+ expect {
+ service_instance_share.create(service_instance, [target_space1, service_instance.space], user_audit_info)
+ }.to raise_error('db failure')
+
+ instance = ServiceInstance.find(guid: service_instance.guid)
+
+ expect(instance.shared_spaces.length).to eq 0
+ end
+
+ it 'does not audit any share events' do
+ expect(Repositories::ServiceInstanceShareEventRepository).to_not receive(:record_share_event)
+
+ expect {
+ service_instance_share.create(service_instance, [target_space1, service_instance.space], user_audit_info)
+ }.to raise_error('db failure')
+ end
+ end
+
+ context 'when the service does is not shareable' do
+ before do
+ allow(service_instance).to receive(:shareable?).and_return(false)
+ end
+
+ it 'raises an api error' do
+ expect {
+ service_instance_share.create(service_instance, [target_space1, target_space2], user_audit_info)
+ }.to raise_error(CloudController::Errors::ApiError, /The #{service_instance.service.label} service does not support service instance sharing./)
+ end
+ end
end
end
end
diff --git a/spec/unit/controllers/runtime/spaces_controller_spec.rb b/spec/unit/controllers/runtime/spaces_controller_spec.rb
index 7477158ad49..de2de367020 100644
--- a/spec/unit/controllers/runtime/spaces_controller_spec.rb
+++ b/spec/unit/controllers/runtime/spaces_controller_spec.rb
@@ -223,6 +223,14 @@ def decoded_guids
before { set_current_user(developer) }
+ it 'returns the shared from url' do
+ space_instance = ManagedServiceInstance.make(space: space)
+
+ get "/v2/spaces/#{space.guid}/service_instances"
+ service_instance_response = decoded_response.fetch('resources').first
+ expect(service_instance_response.fetch('entity').fetch('shared_from_url')).to eq("/v2/service_instances/#{space_instance.guid}/shared_from")
+ end
+
context 'when filtering results' do
it 'returns only matching results' do
user_provided_service_instance_1 = UserProvidedServiceInstance.make(space: space, name: 'provided service 1')
@@ -240,6 +248,39 @@ def decoded_guids
end
end
+ describe 'shared service instances' do
+ context 'when a service instance has been shared from another space' do
+ let(:shared_service_instance) { ManagedServiceInstance.make(space: Space.make) }
+
+ before do
+ shared_service_instance.add_shared_space(space)
+ end
+
+ it 'returns the shared service instance' do
+ get "v2/spaces/#{space.guid}/service_instances"
+
+ guids = decoded_response.fetch('resources').map { |service| service.fetch('metadata').fetch('guid') }
+ expect(guids).to include(shared_service_instance.guid)
+ end
+ end
+
+ context 'when a service instance has been shared between two spaces that are not the queried space' do
+ let(:other_space) { make_space_for_user(developer) }
+ let(:irrelevant_shared_service_instance) { ManagedServiceInstance.make(space: Space.make) }
+
+ before do
+ irrelevant_shared_service_instance.add_shared_space(other_space)
+ end
+
+ it 'does not return the irrelevant shared service instance' do
+ get "v2/spaces/#{space.guid}/service_instances"
+
+ guids = decoded_response.fetch('resources').map { |service| service.fetch('metadata').fetch('guid') }
+ expect(guids).not_to include(irrelevant_shared_service_instance.guid)
+ end
+ end
+ end
+
context 'when there are provided service instances' do
let!(:user_provided_service_instance) { UserProvidedServiceInstance.make(space: space) }
let!(:managed_service_instance) { ManagedServiceInstance.make(space: space) }
diff --git a/spec/unit/controllers/services/service_instances_controller_spec.rb b/spec/unit/controllers/services/service_instances_controller_spec.rb
index 6f6607fa9e2..020eee3c66a 100644
--- a/spec/unit/controllers/services/service_instances_controller_spec.rb
+++ b/spec/unit/controllers/services/service_instances_controller_spec.rb
@@ -885,6 +885,53 @@ def stub_delete_and_return(status, body)
expect(last_response.status).to eq(400)
expect(decoded_response['code']).to eq(60002)
end
+
+ context 'when a service instance share exists between spaces' do
+ let(:source_space) { Space.make(organization: space.organization) }
+ before do
+ source_space.add_developer(developer)
+
+ service_instance = create_managed_service_instance(accepts_incomplete: 'false', space: source_space)
+ service_instance.add_shared_space(space)
+ expect(last_response.status).to eq(201)
+ end
+
+ it 'does not allow a managed service instance with same name as a shared service instance' do
+ create_managed_service_instance
+ expect(last_response.status).to eq(400)
+ expect(decoded_response['code']).to eq(60002)
+ end
+
+ it 'does not allow a user provided service instance with same name as a shared service instance' do
+ create_user_provided_service_instance
+ expect(last_response.status).to eq(400)
+ expect(decoded_response['code']).to eq(60002)
+ end
+
+ context 'when an unshared instance exists in the source space' do
+ before do
+ create_managed_service_instance(accepts_incomplete: 'false', space: source_space, name: 'bar')
+ expect(last_response.status).to eq(201)
+ end
+
+ it 'allows an instance of the same name to be created in the shared to space' do
+ create_managed_service_instance(accepts_incomplete: 'false', space: space, name: 'bar')
+ expect(last_response.status).to eq(201)
+ end
+ end
+
+ context 'when an unshared instance exists in the shared to space' do
+ before do
+ create_managed_service_instance(accepts_incomplete: 'false', space: space, name: 'bar')
+ expect(last_response.status).to eq(201)
+ end
+
+ it 'allows an instance of the same name to be created in the source space' do
+ create_managed_service_instance(accepts_incomplete: 'false', space: source_space, name: 'bar')
+ expect(last_response.status).to eq(201)
+ end
+ end
+ end
end
context 'when the service_plan does not exist' do
@@ -1542,6 +1589,43 @@ def stub_delete_and_return(status, body)
end
end
+ context 'when the service instance is shared' do
+ let(:service_instance) { ManagedServiceInstance.make }
+ let(:shared_to_space) { Space.make }
+ let(:body) do
+ {
+ tags: []
+ }.to_json
+ end
+
+ before do
+ service_instance.add_shared_space(shared_to_space)
+ end
+
+ context 'and a developer in the originating space tries to update the instance' do
+ it 'updates successfully' do
+ put "/v2/service_instances/#{service_instance.guid}", body
+ expect(last_response).to have_status_code 201
+ end
+ end
+
+ context 'and a developer in the shared to space tries to update the instance' do
+ let(:shared_to_user) { make_developer_for_space(shared_to_space) }
+
+ before do
+ set_current_user(shared_to_user)
+ end
+
+ it 'should give the user an error' do
+ put "/v2/service_instances/#{service_instance.guid}", body
+
+ expect(last_response).to have_status_code 403
+ expect(last_response.body).to include 'CF-NotAuthorized'
+ expect(last_response.body).to include 'You are not authorized to perform the requested action'
+ end
+ end
+ end
+
describe 'error cases' do
context 'when the service instance does not exist' do
it 'returns a ServiceInstanceNotFound error' do
@@ -2315,6 +2399,84 @@ def stub_delete_and_return(status, body)
end
end
+ context 'when the service instance has been shared' do
+ let(:originating_space) { Space.make }
+ let!(:service_instance) { ManagedServiceInstance.make(space: originating_space) }
+
+ before do
+ service_instance.add_shared_space(space)
+ end
+
+ context 'as a SpaceDeveloper in source and target space' do
+ it 'should give the user an error' do
+ delete "/v2/service_instances/#{service_instance.guid}"
+
+ expect(last_response).to have_status_code 400
+ expect(last_response.body).to include 'ServiceInstanceDeletionSharesExists'
+ expect(last_response.body).to include(
+ 'Service instances must be unshared before they can be deleted. ' \
+ "Unsharing #{service_instance.name} will automatically delete any bindings " \
+ 'that have been made to applications in other spaces.')
+ end
+
+ it 'associated shares are not deleted' do
+ delete "/v2/service_instances/#{service_instance.guid}"
+
+ expect(ServiceInstance.find(guid: service_instance.guid)).to be
+ expect(ServiceInstance.find(guid: service_instance.guid).shared_spaces.length).to eq(1)
+ end
+
+ context 'and there are bindings to the shared instance' do
+ before do
+ ServiceBinding.make(
+ app: AppModel.make(space: space),
+ service_instance: service_instance
+ )
+ end
+
+ it 'should give the user an error' do
+ delete "/v2/service_instances/#{service_instance.guid}"
+
+ expect(last_response).to have_status_code 400
+ expect(last_response.body).to include 'ServiceInstanceDeletionSharesExists'
+ expect(last_response.body).to include(
+ 'Service instances must be unshared before they can be deleted. ' \
+ "Unsharing #{service_instance.name} will automatically delete any bindings " \
+ 'that have been made to applications in other spaces.')
+ end
+ end
+
+ context 'and recursive=true' do
+ it 'deletes the associated shares' do
+ expect {
+ delete "/v2/service_instances/#{service_instance.guid}?recursive=true"
+ }.to change(ServiceInstance.join(:service_instance_shares, service_instance_guid: :service_instances__guid), :count).by(-1)
+
+ expect(last_response.status).to eq(204)
+ expect(ServiceInstance.find(guid: service_instance.guid)).to be_nil
+ end
+ end
+ end
+
+ context 'as a SpaceDeveloper in target space' do
+ let(:target_space) { Space.make }
+ let(:target_space_dev) { make_developer_for_space(target_space) }
+
+ before do
+ service_instance.add_shared_space(target_space)
+ set_current_user(target_space_dev)
+ end
+
+ it 'should give the user an error' do
+ delete "/v2/service_instances/#{service_instance.guid}"
+
+ expect(last_response).to have_status_code 403
+ expect(last_response.body).to include 'CF-NotAuthorized'
+ expect(last_response.body).to include 'You are not authorized to perform the requested action'
+ end
+ end
+ end
+
context 'with ?accepts_incomplete=true' do
before do
stub_deprovision(service_instance, body: body, status: status, accepts_incomplete: true)
@@ -3496,6 +3658,253 @@ def verify_forbidden(user)
end
end
+ describe 'GET /v2/service_instances/:service_instance_guid/shared_from' do
+ let(:org) { Organization.make }
+ let(:space) { Space.make(organization: org) }
+ let(:instance) { ManagedServiceInstance.make(space: space) }
+
+ context 'when the service instance is not shared' do
+ it 'returns no content' do
+ set_current_user_as_admin
+ get "/v2/service_instances/#{instance.guid}/shared_from"
+ expect(last_response.status).to eql(204)
+ expect(JSON.parse(last_response.body)).to be nil
+ end
+ end
+
+ context 'when the service instance is shared' do
+ let(:other_org) { Organization.make }
+ let(:other_space) { Space.make(organization: other_org) }
+
+ before do
+ instance.add_shared_space(other_space)
+ end
+
+ it 'returns the correct body' do
+ set_current_user_as_admin
+ get "/v2/service_instances/#{instance.guid}/shared_from"
+ expect(last_response.status).to eql(200), last_response.body
+ parsed_response = JSON.parse(last_response.body)
+ expect(parsed_response['space_name']).to eq(space.name)
+ expect(parsed_response['organization_name']).to eq(space.organization.name)
+ expect(parsed_response.keys).to match_array(['space_name', 'organization_name'])
+ end
+
+ describe 'permissions' do
+ let(:user) { User.make }
+
+ context 'when the user is a member of the org/space this instance exists in' do
+ {
+ 'admin' => 200,
+ 'space_developer' => 200,
+ 'admin_read_only' => 200,
+ 'global_auditor' => 200,
+ 'space_manager' => 200,
+ 'space_auditor' => 200,
+ 'org_manager' => 200,
+ 'org_auditor' => 404,
+ 'org_billing_manager' => 404,
+ }.each do |role, expected_status|
+ context "as an #{role}" do
+ before do
+ set_current_user_as_role(
+ role: role,
+ org: org,
+ space: space,
+ user: user,
+ scopes: ['cloud_controller.read']
+ )
+ end
+
+ it "has a #{expected_status} http status code" do
+ get "/v2/service_instances/#{instance.guid}/shared_from"
+ expect(last_response.status).to eq(expected_status), "Expected #{expected_status}, got: #{last_response.status}, role: #{role}"
+ end
+ end
+ end
+ end
+
+ context 'when the user is a member of the org/space where the service instance was shared to' do
+ {
+ 'space_developer' => 200,
+ 'space_manager' => 200,
+ 'space_auditor' => 200,
+ 'org_manager' => 200,
+ 'org_auditor' => 404,
+ 'org_billing_manager' => 404,
+ }.each do |role, expected_status|
+ context "as an #{role}" do
+ before do
+ set_current_user_as_role(
+ role: role,
+ org: other_org,
+ space: other_space,
+ user: user,
+ scopes: ['cloud_controller.read']
+ )
+ end
+
+ it "has a #{expected_status} http status code" do
+ get "/v2/service_instances/#{instance.guid}/shared_from"
+ expect(last_response.status).to eq(expected_status), "Expected #{expected_status}, got: #{last_response.status}, role: #{role}"
+ end
+ end
+ end
+ end
+
+ context 'when the user is NOT a member of the space this instance exists in' do
+ let(:instance) { ManagedServiceInstance.make }
+
+ it 'returns a JSON payload indicating the user does not have permission to manage this instance' do
+ set_current_user(user)
+ get "/v2/service_instances/#{instance.guid}/shared_from"
+ expect(last_response.status).to eql(404)
+ end
+ end
+ end
+ end
+ end
+
+ describe 'GET /v2/service_instances/:service_instance_guid/shared_to' do
+ let(:org) { Organization.make }
+ let(:space) { Space.make(organization: org) }
+ let(:instance) { ManagedServiceInstance.make(space: space) }
+
+ it 'returns the correct body' do
+ set_current_user_as_admin
+ get "/v2/service_instances/#{instance.guid}/shared_to"
+ expect(last_response.status).to eql(200)
+ expect(JSON.parse(last_response.body)['resources']).to eq([])
+ end
+
+ context 'when the service instance is shared into multiple spaces' do
+ let(:space1) { Space.make }
+ let(:space2) { Space.make }
+
+ before do
+ FeatureFlag.make(name: 'service_instance_sharing', enabled: true, error_message: nil)
+ instance.add_shared_space(space1)
+ instance.add_shared_space(space2)
+ end
+
+ it 'returns the correct body' do
+ set_current_user_as_admin
+ get "/v2/service_instances/#{instance.guid}/shared_to"
+ decoded_response = JSON.parse(last_response.body)
+ expect(last_response.status).to eql(200), last_response.body
+ expect(decoded_response.fetch('total_results')).to eq(2)
+ resources = decoded_response.fetch('resources')
+
+ space1_resource = resources.find { |resource| resource['space_name'] == space1.name }
+ space2_resource = resources.find { |resource| resource['space_name'] == space2.name }
+
+ expect(space1_resource.keys).to match_array(['space_name', 'organization_name', 'bound_app_count'])
+ expect(space2_resource.keys).to match_array(['space_name', 'organization_name', 'bound_app_count'])
+
+ expect(space1_resource.fetch('space_name')).to eq(space1.name)
+ expect(space2_resource.fetch('space_name')).to eq(space2.name)
+
+ expect(space1_resource.fetch('organization_name')).to eq(space1.organization.name)
+ expect(space2_resource.fetch('organization_name')).to eq(space2.organization.name)
+
+ expect(space1_resource.fetch('bound_app_count')).to eq(0)
+ expect(space2_resource.fetch('bound_app_count')).to eq(0)
+ end
+
+ context 'when there are apps bound to the shared service instance' do
+ before do
+ ServiceBinding.make(service_instance: instance, app: AppModel.make(space: space1))
+ ServiceBinding.make(service_instance: instance, app: AppModel.make(space: space1))
+ ServiceBinding.make(service_instance: ServiceInstance.make(space: space1), app: AppModel.make(space: space1))
+
+ ServiceBinding.make(service_instance: instance, app: AppModel.make(space: space2))
+ end
+
+ it 'returns the correct bound_app_count' do
+ set_current_user_as_admin
+ get "/v2/service_instances/#{instance.guid}/shared_to"
+ decoded_response = JSON.parse(last_response.body)
+ expect(last_response.status).to eql(200), last_response.body
+ resources = decoded_response.fetch('resources')
+
+ space1_resource = resources.find { |resource| resource['space_name'] == space1.name }
+ space2_resource = resources.find { |resource| resource['space_name'] == space2.name }
+
+ expect(space1_resource.fetch('bound_app_count')).to eq(2)
+ expect(space2_resource.fetch('bound_app_count')).to eq(1)
+ end
+ end
+ end
+
+ describe 'permissions' do
+ let(:user) { User.make }
+
+ context 'when the user is a member of the org/space this instance exists in' do
+ {
+ 'admin' => 200,
+ 'space_developer' => 200,
+ 'admin_read_only' => 200,
+ 'global_auditor' => 200,
+ 'space_manager' => 200,
+ 'space_auditor' => 200,
+ 'org_manager' => 200,
+ 'org_auditor' => 404,
+ 'org_billing_manager' => 404,
+ }.each do |role, expected_status|
+ context "as an #{role}" do
+ before do
+ set_current_user_as_role(
+ role: role,
+ org: org,
+ space: space,
+ user: user,
+ )
+ end
+
+ it "has a #{expected_status} http status code" do
+ get "/v2/service_instances/#{instance.guid}/shared_to"
+ expect(last_response.status).to eq(expected_status), "Expected #{expected_status}, got: #{last_response.status}, role: #{role}"
+ end
+ end
+ end
+ end
+
+ context 'when the user is a member of the org/space where the service instance was shared to' do
+ let(:other_org) { Organization.make }
+ let(:other_space) { Space.make(organization: other_org) }
+
+ before do
+ instance.add_shared_space(other_space)
+ end
+
+ {
+ 'space_developer' => 404,
+ 'space_manager' => 404,
+ 'space_auditor' => 404,
+ 'org_manager' => 404,
+ 'org_auditor' => 404,
+ 'org_billing_manager' => 404,
+ }.each do |role, expected_status|
+ context "as an #{role}" do
+ before do
+ set_current_user_as_role(
+ role: role,
+ org: other_org,
+ space: other_space,
+ user: user,
+ )
+ end
+
+ it "has a #{expected_status} http status code" do
+ get "/v2/service_instances/#{instance.guid}/shared_to"
+ expect(last_response.status).to eq(expected_status), "Expected #{expected_status}, got: #{last_response.status}, role: #{role}"
+ end
+ end
+ end
+ end
+ end
+ end
+
describe 'GET /v2/service_instances/:service_instance_guid/service_keys' do
let(:space) { Space.make }
let(:manager) { make_manager_for_space(space) }
@@ -3523,6 +3932,16 @@ def verify_forbidden(user)
it 'returns the forbidden code for auditors' do
verify_forbidden auditor
end
+
+ context 'when user is a developer in space to which the instance was shared' do
+ before do
+ instance.add_shared_space(space)
+ end
+
+ it 'returns the forbidden code' do
+ verify_forbidden developer
+ end
+ end
end
context 'when the user is a member of the space this instance exists in' do
@@ -3756,7 +4175,6 @@ def verify_forbidden(user)
let(:errors) { instance_double(Sequel::Model::Errors) }
let(:attributes) { {} }
- let(:space_and_name_errors) { nil }
let(:quota_errors) { nil }
let(:service_plan_errors) { nil }
let(:service_instance_name_errors) { nil }
@@ -3766,7 +4184,6 @@ def verify_forbidden(user)
before do
allow(e).to receive(:errors).and_return(errors)
- allow(errors).to receive(:on).with([:space_id, :name]).and_return(space_and_name_errors)
allow(errors).to receive(:on).with(:quota).and_return(quota_errors)
allow(errors).to receive(:on).with(:service_plan).and_return(service_plan_errors)
allow(errors).to receive(:on).with(:name).and_return(service_instance_name_errors)
@@ -3781,7 +4198,6 @@ def verify_forbidden(user)
end
context "when errors are included but aren't supported validation exceptions" do
- let(:space_and_name_errors) { [:stuff] }
let(:quota_errors) { [:stuff] }
let(:service_plan_errors) { [:stuff] }
let(:service_instance_name_errors) { [:stuff] }
@@ -3795,7 +4211,7 @@ def verify_forbidden(user)
context 'when there is a service instance name taken error' do
let(:attributes) { { 'name' => 'test name' } }
- let(:space_and_name_errors) { [:unique] }
+ let(:service_instance_name_errors) { [:unique] }
it 'returns a ServiceInstanceNameTaken error' do
expect(VCAP::CloudController::ServiceInstancesController.translate_validation_exception(e, attributes).name).to eq('ServiceInstanceNameTaken')
@@ -3864,10 +4280,12 @@ def create_managed_service_instance(user_opts={})
arbitrary_params = user_opts.delete(:parameters)
accepts_incomplete = user_opts.delete(:accepts_incomplete) { |_| 'true' }
tags = user_opts.delete(:tags)
+ service_instance_space = user_opts.delete(:space) || space
+ service_instance_name = user_opts.delete(:name) || 'foo'
body = {
- name: 'foo',
- space_guid: space.guid,
+ name: service_instance_name,
+ space_guid: service_instance_space.guid,
service_plan_guid: plan.guid,
}
body[:parameters] = arbitrary_params if arbitrary_params
diff --git a/spec/unit/controllers/services/service_keys_controller_spec.rb b/spec/unit/controllers/services/service_keys_controller_spec.rb
index d5ef8692e94..d14a0a0b8ee 100644
--- a/spec/unit/controllers/services/service_keys_controller_spec.rb
+++ b/spec/unit/controllers/services/service_keys_controller_spec.rb
@@ -387,6 +387,30 @@ def bind_url_regex(opts={})
expect(a_request(:put, url_regex).with(body: hash_including(expected_body))).to have_been_made
end
end
+
+ context 'when the service instance has been shared' do
+ let(:other_space) { Space.make }
+
+ before do
+ instance.add_shared_space(other_space)
+ end
+
+ context 'when the user is a space developer in the service instance space' do
+ it 'returns successfully' do
+ post '/v2/service_keys', req
+ expect(last_response).to have_status_code(201)
+ end
+ end
+
+ context 'when the user does not have access to the service instance space' do
+ let(:developer) { make_developer_for_space(other_space) }
+
+ it 'returns a 403' do
+ post '/v2/service_keys', req
+ expect(last_response).to have_status_code(403)
+ end
+ end
+ end
end
context 'for a user-provided service instance' do
@@ -456,8 +480,8 @@ def verify_not_found_response(service_key_guid)
end
context 'Not authorized to perform get operation' do
- let(:manager) { make_manager_for_space(service_key.service_instance.space) }
- let(:auditor) { make_auditor_for_space(service_key.service_instance.space) }
+ let(:manager) { make_manager_for_space(instance.space) }
+ let(:auditor) { make_auditor_for_space(instance.space) }
it 'SpaceManager role can not get a service key' do
set_current_user(manager)
@@ -470,6 +494,20 @@ def verify_not_found_response(service_key_guid)
get "/v2/service_keys/#{service_key.guid}"
verify_not_found_response(service_key.guid)
end
+
+ context 'when the user is a developer in a space to which the service instance is shared' do
+ let(:other_space) { Space.make }
+ let(:developer) { make_developer_for_space(other_space) }
+
+ before do
+ instance.add_shared_space(other_space)
+ end
+
+ it 'is reports the key as not found' do
+ get "/v2/service_keys/#{service_key.guid}"
+ verify_not_found_response(service_key.guid)
+ end
+ end
end
context 'when the key is a CredHub reference' do
@@ -529,10 +567,11 @@ def verify_not_found_response(service_key_guid)
describe 'DELETE', '/v2/service_keys/:service_key_guid' do
let(:service_key) { ServiceKey.make }
- let(:developer) { make_developer_for_space(service_key.service_instance.space) }
+ let(:instance) { service_key.service_instance }
+ let(:developer) { make_developer_for_space(instance.space) }
before do
- stub_requests(service_key.service_instance.service.service_broker)
+ stub_requests(instance.service.service_broker)
set_current_user(developer, email: 'example@example.com')
end
@@ -543,8 +582,8 @@ def verify_not_found_response(service_key_guid)
end
context 'Not authorized to perform delete operation' do
- let(:manager) { make_manager_for_space(service_key.service_instance.space) }
- let(:auditor) { make_auditor_for_space(service_key.service_instance.space) }
+ let(:manager) { make_manager_for_space(instance.space) }
+ let(:auditor) { make_auditor_for_space(instance.space) }
it 'SpaceManager role can not delete a service key' do
set_current_user(manager)
@@ -557,6 +596,20 @@ def verify_not_found_response(service_key_guid)
delete "/v2/service_keys/#{service_key.guid}"
verify_not_found_response(service_key.guid)
end
+
+ context 'when the user is a developer in a space to which the service instance is shared' do
+ let(:other_space) { Space.make }
+ let(:developer) { make_developer_for_space(other_space) }
+
+ before do
+ instance.add_shared_space(other_space)
+ end
+
+ it 'is reports the key as not found' do
+ delete "/v2/service_keys/#{service_key.guid}"
+ verify_not_found_response(service_key.guid)
+ end
+ end
end
it 'returns ServiceKeyNotFound error if there is no such key' do
diff --git a/spec/unit/controllers/v3/service_instance_controller_spec.rb b/spec/unit/controllers/v3/service_instance_controller_spec.rb
index 16dad33ef32..8e170faf627 100644
--- a/spec/unit/controllers/v3/service_instance_controller_spec.rb
+++ b/spec/unit/controllers/v3/service_instance_controller_spec.rb
@@ -2,9 +2,116 @@
RSpec.describe ServiceInstancesV3Controller, type: :controller do
let(:user) { set_current_user(VCAP::CloudController::User.make) }
+ let(:space) { VCAP::CloudController::Space.make }
+ let!(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space: space) }
+
+ describe '#index' do
+ context 'when there are multiple service instances' do
+ let!(:service_instance2) { VCAP::CloudController::ManagedServiceInstance.make }
+ let!(:service_instance3) { VCAP::CloudController::ManagedServiceInstance.make }
+
+ context 'as an admin' do
+ before do
+ set_current_user_as_admin
+ end
+
+ it 'returns all service instances' do
+ get :index
+ expect(response.status).to eq(200), response.body
+ expect(parsed_body['resources'].length).to eq 3
+
+ response_names = parsed_body['resources'].map { |resource| resource['name'] }
+ expect(response_names).to include(service_instance.name, service_instance2.name, service_instance3.name)
+ end
+ end
+
+ context 'as a user who only has limited access' do
+ before do
+ set_current_user_as_role(role: 'space_developer', org: space.organization, space: space, user: user)
+ end
+
+ it 'returns a subset of service instances' do
+ get :index
+ expect(response.status).to eq(200), response.body
+ expect(parsed_body['resources'].length).to eq 1
+
+ response_names = parsed_body['resources'].map { |resource| resource['name'] }
+ expect(response_names).to include(service_instance.name)
+ end
+ end
+ end
+
+ describe 'permissions by role' do
+ role_to_expected_http_response = {
+ 'admin' => true,
+ 'admin_read_only' => true,
+ 'global_auditor' => true,
+ 'org_manager' => true,
+ 'org_auditor' => false,
+ 'org_billing_manager' => false,
+ 'space_manager' => true,
+ 'space_auditor' => true,
+ 'space_developer' => true,
+ }.freeze
+
+ role_to_expected_http_response.each do |role, can_see_service_instance|
+ context "as an #{role}" do
+ it "#{can_see_service_instance ? 'can' : 'cannot'} see the service instance" do
+ set_current_user_as_role(role: role, org: space.organization, space: space, user: user)
+
+ expected_service_instance_names = can_see_service_instance ? [service_instance.name] : []
+
+ get :index
+ expect(response.status).to eq(200), response.body
+ expect(parsed_body['resources'].map { |h| h['name'] }).to match_array(expected_service_instance_names)
+ end
+ end
+ end
+ end
+
+ describe 'permissions by role for shared services' do
+ let(:target_space) { VCAP::CloudController::Space.make }
+ before do
+ service_instance.add_shared_space(target_space)
+ end
+ role_to_expected_http_response = {
+ 'org_manager' => true,
+ 'org_auditor' => false,
+ 'org_billing_manager' => false,
+ 'space_manager' => true,
+ 'space_auditor' => true,
+ 'space_developer' => true,
+ }.freeze
+
+ role_to_expected_http_response.each do |role, can_see_service_instance|
+ context "as an #{role}" do
+ it "#{can_see_service_instance ? 'can' : 'cannot'} see the service instance" do
+ set_current_user_as_role(role: role, org: target_space.organization, space: target_space, user: user)
+
+ expected_service_instance_names = can_see_service_instance ? [service_instance.name] : []
+
+ get :index
+ expect(response.status).to eq(200), response.body
+ expect(parsed_body['resources'].map { |h| h['name'] }).to match_array(expected_service_instance_names)
+ end
+ end
+ end
+ end
+
+ context 'when a non-supported value is specified' do
+ it 'a bad query parameter error is returned' do
+ set_current_user_as_admin
+ get :index, { order_by: 'banana' }
+
+ expect(response.status).to eq(400)
+ expect(response.body).to include 'BadQueryParameter'
+ expect(response.body).to include("Order by can only be: 'created_at', 'updated_at', 'name'")
+ end
+ end
+ end
describe '#share_service_instance' do
- let(:service_instance) { VCAP::CloudController::ServiceInstance.make }
+ let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make }
let(:target_space) { VCAP::CloudController::Space.make }
let(:target_space2) { VCAP::CloudController::Space.make }
let(:service_instance_sharing_enabled) { true }
@@ -23,28 +130,39 @@
VCAP::CloudController::FeatureFlag.make(name: 'service_instance_sharing', enabled: service_instance_sharing_enabled, error_message: nil)
end
- it 'shares the service instance to the target space' do
- post :share_service_instance, service_instance_guid: service_instance.guid, body: req_body
+ it 'calls the service instance share action' do
+ action = instance_double(VCAP::CloudController::ServiceInstanceShare)
+ allow(VCAP::CloudController::ServiceInstanceShare).to receive(:new).and_return(action)
- expect(response.status).to eq 200
- expect(parsed_body['data'][0]['guid']).to eq(target_space.guid)
- expect(service_instance.shared_spaces).to contain_exactly(target_space)
+ expect(action).to receive(:create).with(service_instance, [target_space], an_instance_of(VCAP::CloudController::UserAuditInfo))
+
+ post :share_service_instance, service_instance_guid: service_instance.guid, body: req_body
end
it 'shares the service instance to multiple target spaces' do
+ action = instance_double(VCAP::CloudController::ServiceInstanceShare)
+ allow(VCAP::CloudController::ServiceInstanceShare).to receive(:new).and_return(action)
+ expect(action).to receive(:create).with(service_instance, a_collection_containing_exactly(target_space, target_space2), an_instance_of(VCAP::CloudController::UserAuditInfo))
+
req_body[:data] << { guid: target_space2.guid }
post :share_service_instance, service_instance_guid: service_instance.guid, body: req_body
-
expect(response.status).to eq 200
+ end
+
+ context 'when the service instance share action errors' do
+ before do
+ action = instance_double(VCAP::CloudController::ServiceInstanceShare)
+ allow(VCAP::CloudController::ServiceInstanceShare).to receive(:new).and_return(action)
- target_space_guids = []
- parsed_body['data'].each do |item|
- target_space_guids << item['guid']
+ expect(action).to receive(:create).and_raise('boom')
end
- expect(target_space_guids).to contain_exactly(target_space.guid, target_space2.guid)
- expect(service_instance.shared_spaces).to contain_exactly(target_space, target_space2)
+ it 'returns the error to the user' do
+ expect {
+ post :share_service_instance, service_instance_guid: service_instance.guid, body: req_body
+ }.to raise_error('boom')
+ end
end
context 'when the service_instance_sharing feature flag is disabled' do
@@ -99,16 +217,20 @@
end
end
- context 'when the service instance has already been shared with the specified space' do
+ context 'when the source space is contained in the list of target spaces' do
before do
- post :share_service_instance, service_instance_guid: service_instance.guid, body: req_body
+ req_body[:data] = [
+ { guid: service_instance.space.guid },
+ { guid: target_space.guid }
+ ]
end
- it 'returns a 200 and leaves the existing share intact' do
+ it 'does not share into any spaces and returns an error message' do
post :share_service_instance, service_instance_guid: service_instance.guid, body: req_body
- expect(response.status).to eq 200
- expect(service_instance.shared_spaces).to include(target_space)
+ expect(response.status).to eq 422
+ expect(response.body).to include('Service instances cannot be shared into the space where they were created')
+ expect(service_instance.shared_spaces).to be_empty
end
end
@@ -125,6 +247,25 @@
end
end
+ context 'when the user has access to the service instance through a share' do
+ before do
+ service_instance.add_shared_space(target_space)
+ set_current_user_as_role(role: 'space_developer', org: target_space.organization, space: target_space, user: user)
+
+ outer_space = VCAP::CloudController::Space.make
+ req_body[:data] = [{ guid: outer_space.guid }]
+ end
+
+ after do
+ service_instance.remove_shared_space(target_space)
+ end
+
+ it 'cannot share the service instance into another space' do
+ post :share_service_instance, service_instance_guid: service_instance.guid, body: req_body
+ expect(response.status).to eq 403
+ end
+ end
+
describe 'permissions by role' do
context 'when the user is a space developer in the source space' do
before do
@@ -272,6 +413,18 @@
end
end
+ context 'when the user has access to the service instance through a share' do
+ before do
+ service_instance.add_shared_space(target_space)
+ set_current_user_as_role(role: 'space_developer', org: target_space.organization, space: target_space, user: user)
+ end
+
+ it 'cannot unshare the service instance from another space' do
+ delete :unshare_service_instance, service_instance_guid: service_instance.guid, space_guid: target_space.guid
+ expect(response.status).to eq 403
+ end
+ end
+
describe 'permissions by role' do
role_to_expected_http_response = {
'admin' => 204,
diff --git a/spec/unit/messages/service_instances_list_message_spec.rb b/spec/unit/messages/service_instances_list_message_spec.rb
new file mode 100644
index 00000000000..1d935b1d602
--- /dev/null
+++ b/spec/unit/messages/service_instances_list_message_spec.rb
@@ -0,0 +1,60 @@
+require 'spec_helper'
+require 'messages/service_instances/service_instances_list_message'
+
+module VCAP::CloudController
+ RSpec.describe ServiceInstancesListMessage do
+ describe '.from_params' do
+ let(:params) do
+ {
+ 'page' => 1,
+ 'per_page' => 5,
+ 'order_by' => 'name',
+ 'names' => 'rabbitmq, redis,mysql'
+ }
+ end
+
+ it 'returns the correct ServiceInstancesListMessage' do
+ message = ServiceInstancesListMessage.from_params(params)
+
+ expect(message).to be_a(ServiceInstancesListMessage)
+ expect(message.page).to eq(1)
+ expect(message.per_page).to eq(5)
+ expect(message.order_by).to eq('name')
+ expect(message.names).to match_array(['mysql', 'rabbitmq', 'redis'])
+ end
+
+ it 'converts requested keys to symbols' do
+ message = ServiceInstancesListMessage.from_params(params)
+
+ expect(message.requested?(:page)).to be_truthy
+ expect(message.requested?(:per_page)).to be_truthy
+ expect(message.requested?(:order_by)).to be_truthy
+ expect(message.requested?(:names)).to be_truthy
+ end
+ end
+
+ describe 'fields' do
+ it 'accepts a set of fields' do
+ message = ServiceInstancesListMessage.new({
+ page: 1,
+ per_page: 5,
+ order_by: 'created_at',
+ names: ['rabbitmq', 'redis']
+ })
+ expect(message).to be_valid
+ end
+
+ it 'accepts an empty set' do
+ message = ServiceInstancesListMessage.new
+ expect(message).to be_valid
+ end
+
+ it 'does not accept a field not in this set' do
+ message = ServiceInstancesListMessage.new({ foobar: 'pants' })
+
+ expect(message).not_to be_valid
+ expect(message.errors[:base]).to include("Unknown query parameter(s): 'foobar'")
+ end
+ end
+ end
+end
diff --git a/spec/unit/models/services/managed_service_instance_spec.rb b/spec/unit/models/services/managed_service_instance_spec.rb
index 56d9ea23ce9..cffdec7fec9 100644
--- a/spec/unit/models/services/managed_service_instance_spec.rb
+++ b/spec/unit/models/services/managed_service_instance_spec.rb
@@ -32,7 +32,7 @@ module VCAP::CloudController
it { is_expected.to validate_presence :name }
it { is_expected.to validate_presence :service_plan }
it { is_expected.to validate_presence :space }
- it { is_expected.to validate_uniqueness [:space_id, :name] }
+ it { is_expected.to validate_uniqueness :space_id, :name, { error_key: :name } }
it { is_expected.to strip_whitespace :name }
let(:max_tags) { ['a' * 1024, 'b' * 1024] }
@@ -258,6 +258,32 @@ module VCAP::CloudController
end
end
+ describe '#shareable?' do
+ let(:service) { Service.make }
+ let(:service_instance) { ManagedServiceInstance.make }
+
+ before do
+ allow(service).to receive(:shareable?).and_return(is_shareable)
+ allow(service_instance).to receive(:service).and_return(service)
+ end
+
+ context 'when the service instance is not a shareable' do
+ let(:is_shareable) { false }
+
+ it 'returns false' do
+ expect(service_instance).to_not be_shareable
+ end
+ end
+
+ context 'when the service instance is shareable' do
+ let(:is_shareable) { true }
+
+ it 'returns true' do
+ expect(service_instance).to be_shareable
+ end
+ end
+ end
+
describe '#as_summary_json' do
let(:service) { Service.make(label: 'YourSQL', guid: '9876XZ') }
let(:service_plan) { ServicePlan.make(name: 'Gold Plan', guid: '12763abc', service: service) }
diff --git a/spec/unit/models/services/service_instance_spec.rb b/spec/unit/models/services/service_instance_spec.rb
index 671bcc5b92f..5ba7dcd4ead 100644
--- a/spec/unit/models/services/service_instance_spec.rb
+++ b/spec/unit/models/services/service_instance_spec.rb
@@ -85,7 +85,7 @@ module VCAP::CloudController
expect {
service_instance_foo.set(name: 'bar')
service_instance_foo.save_changes
- }.to raise_error(Sequel::ValidationFailed, /space_id and name unique/)
+ }.to raise_error(Sequel::ValidationFailed, /name unique/)
end
end
end
@@ -97,13 +97,13 @@ module VCAP::CloudController
it 'raises an exception when creating another UserProvidedServiceInstance' do
expect {
UserProvidedServiceInstance.create(service_instance_attrs)
- }.to raise_error(Sequel::ValidationFailed, /space_id and name unique/)
+ }.to raise_error(Sequel::ValidationFailed, /name unique/)
end
it 'raises an exception when creating a ManagedServiceInstance' do
expect {
ManagedServiceInstance.create(service_instance_attrs)
- }.to raise_error(Sequel::ValidationFailed, /space_id and name unique/)
+ }.to raise_error(Sequel::ValidationFailed, /name unique/)
end
end
@@ -116,13 +116,37 @@ module VCAP::CloudController
it 'raises an exception when creating another ManagedServiceInstance' do
expect {
ManagedServiceInstance.create(service_instance_attrs)
- }.to raise_error(Sequel::ValidationFailed, /space_id and name unique/)
+ }.to raise_error(Sequel::ValidationFailed, /name unique/)
end
it 'raises an exception when creating a UserProvidedServiceInstance' do
expect {
UserProvidedServiceInstance.create(service_instance_attrs)
- }.to raise_error(Sequel::ValidationFailed, /space_id and name unique/)
+ }.to raise_error(Sequel::ValidationFailed, /name unique/)
+ end
+ end
+
+ describe 'when a ManagedServiceInstance has been shared' do
+ let(:space) { Space.make }
+ let(:originating_space) { Space.make }
+ let(:service_instance) {
+ ManagedServiceInstance.make(name: 'shared-service', space: originating_space)
+ }
+
+ before do
+ service_instance.add_shared_space(space)
+ end
+
+ it 'raises an exception when creating another ManagedServiceInstance' do
+ expect {
+ ManagedServiceInstance.make(name: 'shared-service', space: space)
+ }.to raise_error(Sequel::ValidationFailed, /name unique/)
+ end
+
+ it 'raises an exception when creating another UserProvidedServiceInstance' do
+ expect {
+ UserProvidedServiceInstance.make(name: 'shared-service', space: space)
+ }.to raise_error(Sequel::ValidationFailed, /name unique/)
end
end
end
@@ -265,6 +289,12 @@ module VCAP::CloudController
it { is_expected.to be_bindable }
end
+ describe '#shareable?' do
+ it 'returns false' do
+ expect(service_instance.shareable?).to be_falsey
+ end
+ end
+
describe '#as_summary_json' do
it 'contains name, guid, and binding count' do
instance = VCAP::CloudController::ServiceInstance.make(
@@ -298,6 +328,14 @@ module VCAP::CloudController
expect(service_instance).not_to be_in_suspended_org
end
end
+
+ context 'when the service instance space is not visible' do
+ let(:space) { nil }
+
+ it 'is false' do
+ expect(service_instance).not_to be_in_suspended_org
+ end
+ end
end
describe '#to_hash' do
@@ -326,5 +364,121 @@ module VCAP::CloudController
expect(service_instance.to_hash(opts)['credentials']).to eq({ redacted_message: '[PRIVATE DATA HIDDEN]' })
end
end
+
+ describe '#user_visibility_filter' do
+ let(:developer) { make_developer_for_space(service_instance.space) }
+ let(:auditor) { make_auditor_for_space(service_instance.space) }
+ let(:user) { make_user_for_space(service_instance.space) }
+ let(:org_manager) { make_manager_for_org(service_instance.space.organization) }
+ let(:space_manager) { make_manager_for_space(service_instance.space) }
+
+ context 'when a user is an org manager where the instance was created' do
+ it 'the service instance is visible' do
+ filter = ServiceInstance.user_visibility_filter(org_manager)
+ expect(ServiceInstance.filter(filter).all.length).to eq(1)
+ end
+ end
+
+ context 'when a user is a space developer in the space the instance was created' do
+ it 'the service instance is visible' do
+ filter = ServiceInstance.user_visibility_filter(developer)
+ expect(ServiceInstance.filter(filter).all.length).to eq(1)
+ end
+ end
+
+ context 'when a user is a space auditor in the space the instance was created' do
+ it 'the service instance is visible' do
+ filter = ServiceInstance.user_visibility_filter(auditor)
+ expect(ServiceInstance.filter(filter).all.length).to eq(1)
+ end
+ end
+
+ context 'when a user is a space manager in the space the instance was created' do
+ it 'the service instance is visible' do
+ filter = ServiceInstance.user_visibility_filter(space_manager)
+ expect(ServiceInstance.filter(filter).all.length).to eq(1)
+ end
+ end
+
+ context 'when a user does not have access to the originating space' do
+ it 'the service instance is not visible' do
+ filter = ServiceInstance.user_visibility_filter(user)
+ expect(ServiceInstance.filter(filter).all.length).to eq(0)
+ end
+ end
+
+ context 'when the service instance is shared' do
+ let(:target_space) { VCAP::CloudController::Space.make }
+ let(:target_space_dev) { make_developer_for_space(target_space) }
+ let(:target_org_user) { make_user_for_org(target_space.organization) }
+ let(:target_space_auditor) { make_auditor_for_space(target_space) }
+ let(:target_space_manager) { make_manager_for_space(target_space) }
+ let(:target_space_org_manager) { make_manager_for_org(target_space.organization) }
+
+ before do
+ service_instance.add_shared_space(target_space)
+ end
+
+ context 'when a user is a space developer in the target space' do
+ it 'the service instance is visible' do
+ filter = ServiceInstance.user_visibility_filter(target_space_dev)
+ expect(ServiceInstance.filter(filter).all.length).to eq(1)
+ end
+ end
+
+ context 'when a user is a space developer in the source space' do
+ it 'the service instance is visible' do
+ filter = ServiceInstance.user_visibility_filter(developer)
+ expect(ServiceInstance.filter(filter).all.length).to eq(1)
+ end
+ end
+
+ context 'when a user is a space auditor in the target space' do
+ it 'the service instance is visible' do
+ filter = ServiceInstance.user_visibility_filter(target_space_auditor)
+ expect(ServiceInstance.filter(filter).all.length).to eq(1)
+ end
+ end
+
+ context 'when a user is a space manager in the target space' do
+ it 'the service instance is visible' do
+ filter = ServiceInstance.user_visibility_filter(target_space_manager)
+ expect(ServiceInstance.filter(filter).all.length).to eq(1)
+ end
+ end
+
+ context 'when a user is a org manager in the target space' do
+ it 'the service instance is visible' do
+ filter = ServiceInstance.user_visibility_filter(target_space_org_manager)
+ expect(ServiceInstance.filter(filter).all.length).to eq(1)
+ end
+ end
+
+ context 'when a user does not have access to the target space' do
+ it 'the service instance is not visible' do
+ filter = ServiceInstance.user_visibility_filter(target_org_user)
+ expect(ServiceInstance.filter(filter).all.length).to eq(0)
+ end
+ end
+ end
+ end
+
+ describe '#shared?' do
+ context 'when the service instance has shared spaces' do
+ before do
+ service_instance.add_shared_space(Space.make)
+ end
+
+ it 'returns true' do
+ expect(service_instance.shared?).to be true
+ end
+ end
+
+ context 'when the service instance does not have shared spaces' do
+ it 'returns false' do
+ expect(service_instance.shared?).to be false
+ end
+ end
+ end
end
end
diff --git a/spec/unit/models/services/service_spec.rb b/spec/unit/models/services/service_spec.rb
index 6755b052670..c2a7f037cf3 100644
--- a/spec/unit/models/services/service_spec.rb
+++ b/spec/unit/models/services/service_spec.rb
@@ -397,6 +397,48 @@ def records(user)
end
end
+ describe '#shareable?' do
+ context 'when the service metadata include shareable true' do
+ let(:service) { Service.make(extra: '{"shareable":true}') }
+
+ it 'returns true' do
+ expect(service).to be_shareable
+ end
+ end
+
+ context 'when the service metadata include shareable false' do
+ let(:service) { Service.make(extra: '{"shareable":false}') }
+
+ it 'returns false' do
+ expect(service).to_not be_shareable
+ end
+ end
+
+ context 'when the service does not include the shareable field in metadata' do
+ let(:service) { Service.make(extra: '{"other-key": "value"}') }
+
+ it 'returns false' do
+ expect(service).to_not be_shareable
+ end
+ end
+
+ context 'when the service metadata is nil' do
+ let(:service) { Service.make(extra: nil) }
+
+ it 'returns false' do
+ expect(service).to_not be_shareable
+ end
+ end
+
+ context 'when extra contains malformed json' do
+ let(:service) { Service.make(extra: '{"not-json"}') }
+
+ it 'returns false' do
+ expect(service).to_not be_shareable
+ end
+ end
+ end
+
describe '#client' do
let(:service) { Service.make(service_broker: ServiceBroker.make) }
diff --git a/spec/unit/presenters/v2/service_instance_presenter_spec.rb b/spec/unit/presenters/v2/service_instance_presenter_spec.rb
index 804e909e196..811a2d87a33 100644
--- a/spec/unit/presenters/v2/service_instance_presenter_spec.rb
+++ b/spec/unit/presenters/v2/service_instance_presenter_spec.rb
@@ -11,35 +11,49 @@ module CloudController::Presenters::V2
let(:relations_hash) { { 'relationship_url' => 'http://relationship.example.com' } }
subject { ServiceInstancePresenter.new }
- describe '#entity_hash' do
- before do
- set_current_user_as_admin
- end
+ before do
+ set_current_user_as_admin
+ allow(RelationsPresenter).to receive(:new).and_return(relations_presenter)
+ end
- let(:service_instance) do
- VCAP::CloudController::ServiceInstance.make(
- name: 'things',
- )
- end
- let(:service_plan) { VCAP::CloudController::ServicePlan.make }
+ describe 'ManagedServiceInstance' do
+ describe '#entity_hash' do
+ let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make }
+ let(:service_plan) { VCAP::CloudController::ServicePlan.make }
- before do
- service_instance.service_plan_id = service_plan.id
- service_instance.save
+ before do
+ service_instance.service_plan_id = service_plan.id
+ service_instance.save
+ end
- allow(RelationsPresenter).to receive(:new).and_return(relations_presenter)
+ it 'returns the service instance entity' do
+ expect(subject.entity_hash(controller, service_instance, opts, depth, parents, orphans)).to eq(
+ {
+ 'name' => service_instance.name,
+ 'service_plan_guid' => service_plan.guid,
+ 'service_guid' => service_plan.service.guid,
+ 'relationship_url' => 'http://relationship.example.com',
+ 'service_url' => "/v2/services/#{service_plan.service.guid}",
+ 'shared_from_url' => "/v2/service_instances/#{service_instance.guid}/shared_from",
+ 'shared_to_url' => "/v2/service_instances/#{service_instance.guid}/shared_to",
+ }
+ )
+ end
end
+ end
+
+ describe 'UserProvidedServiceInstance' do
+ describe '#entity_hash' do
+ let(:service_instance) { VCAP::CloudController::UserProvidedServiceInstance.make }
- it 'returns the service instance entity' do
- expect(subject.entity_hash(controller, service_instance, opts, depth, parents, orphans)).to eq(
- {
- 'name' => service_instance.name,
- 'service_plan_guid' => service_plan.guid,
- 'service_guid' => service_plan.service.guid,
- 'relationship_url' => 'http://relationship.example.com',
- 'service_url' => "/v2/services/#{service_plan.service.guid}"
- }
- )
+ it 'returns the service instance entity' do
+ expect(subject.entity_hash(controller, service_instance, opts, depth, parents, orphans)).to eq(
+ {
+ 'name' => service_instance.name,
+ 'relationship_url' => 'http://relationship.example.com',
+ }
+ )
+ end
end
end
end
diff --git a/spec/unit/presenters/v2/service_instance_shared_from_presenter_spec.rb b/spec/unit/presenters/v2/service_instance_shared_from_presenter_spec.rb
new file mode 100644
index 00000000000..8c18d57602e
--- /dev/null
+++ b/spec/unit/presenters/v2/service_instance_shared_from_presenter_spec.rb
@@ -0,0 +1,18 @@
+require 'spec_helper'
+
+module CloudController::Presenters::V2
+ RSpec.describe ServiceInstanceSharedFromPresenter do
+ describe '#to_hash' do
+ it 'returns the space and org name' do
+ space = VCAP::CloudController::Space.make
+ presenter = ServiceInstanceSharedFromPresenter.new
+ expect(presenter.to_hash(space)).to eq(
+ {
+ 'space_name' => space.name,
+ 'organization_name' => space.organization.name,
+ }
+ )
+ end
+ end
+ end
+end
diff --git a/spec/unit/presenters/v2/service_instance_shared_to_presenter_spec.rb b/spec/unit/presenters/v2/service_instance_shared_to_presenter_spec.rb
new file mode 100644
index 00000000000..8119cfd3c69
--- /dev/null
+++ b/spec/unit/presenters/v2/service_instance_shared_to_presenter_spec.rb
@@ -0,0 +1,19 @@
+require 'spec_helper'
+
+module CloudController::Presenters::V2
+ RSpec.describe ServiceInstanceSharedToPresenter do
+ describe '#to_hash' do
+ it 'returns the space name, org name, and bound app count' do
+ space = VCAP::CloudController::Space.make
+ presenter = ServiceInstanceSharedToPresenter.new
+ expect(presenter.to_hash(space, 42)).to eq(
+ {
+ 'space_name' => space.name,
+ 'organization_name' => space.organization.name,
+ 'bound_app_count' => 42
+ }
+ )
+ end
+ end
+ end
+end
diff --git a/spec/unit/presenters/v3/service_instance_presenter_spec.rb b/spec/unit/presenters/v3/service_instance_presenter_spec.rb
new file mode 100644
index 00000000000..58c595f90a4
--- /dev/null
+++ b/spec/unit/presenters/v3/service_instance_presenter_spec.rb
@@ -0,0 +1,20 @@
+require 'spec_helper'
+require 'presenters/v3/service_instance_presenter'
+
+module VCAP::CloudController::Presenters::V3
+ RSpec.describe ServiceInstancePresenter do
+ let(:presenter) { ServiceInstancePresenter.new(service_instance) }
+ let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(name: 'denise-db') }
+
+ describe '#to_hash' do
+ let(:result) { presenter.to_hash }
+
+ it 'presents the model as a hash' do
+ expect(result[:guid]).to eq(service_instance.guid)
+ expect(result[:created_at]).to eq(service_instance.created_at)
+ expect(result[:updated_at]).to eq(service_instance.updated_at)
+ expect(result[:name]).to eq('denise-db')
+ end
+ end
+ end
+end
diff --git a/spec/unit/queries/service_binding_list_fetcher_spec.rb b/spec/unit/queries/service_binding_list_fetcher_spec.rb
index 1e6e825dfaf..3e45c6eb02a 100644
--- a/spec/unit/queries/service_binding_list_fetcher_spec.rb
+++ b/spec/unit/queries/service_binding_list_fetcher_spec.rb
@@ -90,5 +90,53 @@ module VCAP::CloudController
end
end
end
+
+ describe '#fetch_service_instance_bindings_in_space' do
+ let(:space) { Space.make }
+ let(:service_instance) { ServiceInstance.make(space: space) }
+
+ it 'returns a Sequel::Dataset' do
+ results = ServiceBindingListFetcher.fetch_service_instance_bindings_in_space(service_instance.guid, space.guid)
+ expect(results).to be_a(Sequel::Dataset)
+ end
+
+ context 'when there are no bindings' do
+ it 'returns an empty dataset' do
+ results = ServiceBindingListFetcher.fetch_service_instance_bindings_in_space(service_instance.guid, space.guid)
+ expect(results.count).to eql(0)
+ end
+ end
+
+ context 'when a binding exists in a space' do
+ let!(:service_binding) { ServiceBinding.make(app: AppModel.make(space: space), service_instance: service_instance) }
+ let!(:other_service_binding) { ServiceBinding.make }
+
+ it 'returns the binding for the correct space' do
+ results = ServiceBindingListFetcher.fetch_service_instance_bindings_in_space(service_instance.guid, space.guid)
+ expect(results.count).to eql(1)
+ end
+ end
+
+ context 'when multiple bindings exist in a space' do
+ let!(:service_binding1) { ServiceBinding.make(app: AppModel.make(space: space), service_instance: service_instance) }
+ let!(:service_binding2) { ServiceBinding.make(app: AppModel.make(space: space), service_instance: service_instance) }
+ let!(:other_service_binding) { ServiceBinding.make }
+
+ it 'returns the bindings for the correct space' do
+ results = ServiceBindingListFetcher.fetch_service_instance_bindings_in_space(service_instance.guid, space.guid)
+ expect(results.count).to eql(2)
+ end
+ end
+
+ context 'when multiple service instances exist' do
+ let!(:service_binding) { ServiceBinding.make(app: AppModel.make(space: space), service_instance: service_instance) }
+ let!(:other_service_binding) { ServiceBinding.make(service_instance: ServiceInstance.make(space: space)) }
+
+ it 'returns the binding for the correct service instance' do
+ results = ServiceBindingListFetcher.fetch_service_instance_bindings_in_space(service_instance.guid, space.guid)
+ expect(results.count).to eql(1)
+ end
+ end
+ end
end
end
diff --git a/spec/unit/queries/service_instance_list_fetcher_spec.rb b/spec/unit/queries/service_instance_list_fetcher_spec.rb
new file mode 100644
index 00000000000..fa07743ce8d
--- /dev/null
+++ b/spec/unit/queries/service_instance_list_fetcher_spec.rb
@@ -0,0 +1,103 @@
+require 'spec_helper'
+require 'fetchers/service_instance_list_fetcher'
+require 'messages/service_instances/service_instances_list_message'
+
+module VCAP::CloudController
+ RSpec.describe ServiceInstanceListFetcher do
+ let(:filters) { {} }
+ let(:message) { ServiceInstancesListMessage.new(filters) }
+ let(:fetcher) { ServiceInstanceListFetcher.new }
+
+ describe '#fetch_all' do
+ let!(:service_instance_1) { ManagedServiceInstance.make(name: 'rabbitmq') }
+ let!(:service_instance_2) { ManagedServiceInstance.make(name: 'redis') }
+
+ it 'returns a Sequel::Dataset' do
+ results = fetcher.fetch_all(message: message)
+ expect(results).to be_a(Sequel::Dataset)
+ end
+
+ it 'includes all the V3 Service Instances' do
+ results = fetcher.fetch_all(message: message).all
+ expect(results.length).to eq 2
+ expect(results).to include(service_instance_1, service_instance_2)
+ end
+
+ context 'filter' do
+ context 'by service instance name' do
+ let(:filters) { { names: ['rabbitmq'] } }
+
+ it 'only returns matching service instances' do
+ results = fetcher.fetch_all(message: message).all
+ expect(results).to match_array([service_instance_1])
+ expect(results).not_to include(service_instance_2)
+ end
+ end
+ end
+ end
+
+ describe '#fetch' do
+ let!(:service_instance_1) { ManagedServiceInstance.make(name: 'rabbitmq', space: space_1) }
+ let!(:service_instance_2) { ManagedServiceInstance.make(name: 'redis', space: space_1) }
+ let!(:service_instance_3) { ManagedServiceInstance.make(name: 'mysql', space: space_2) }
+
+ let(:space_1) { Space.make }
+ let(:space_2) { Space.make }
+
+ it 'returns all of the service instances in the specified space' do
+ results = fetcher.fetch(message: message, space_guids: [space_1.guid]).all
+
+ expect(results).to match_array([service_instance_1, service_instance_2])
+ end
+
+ context 'filter' do
+ context 'by service instance name' do
+ let(:filters) { { names: ['rabbitmq'] } }
+
+ it 'only returns matching service instances' do
+ results = fetcher.fetch(message: message, space_guids: [space_1.guid]).all
+ expect(results).to match_array([service_instance_1])
+ end
+ end
+
+ context 'by non-existent service instance name' do
+ let(:filters) { { names: ['made-up-name'] } }
+
+ it 'returns no matching service instances' do
+ results = fetcher.fetch(message: message, space_guids: [space_1.guid]).all
+ expect(results).to be_empty
+ end
+ end
+ end
+
+ context 'when service instances are shared' do
+ let(:shared_to_space) { Space.make }
+
+ before do
+ service_instance_2.add_shared_space(shared_to_space)
+ service_instance_1.add_shared_space(shared_to_space)
+ end
+
+ it 'returns all of the service instances shared into the specified space' do
+ results = fetcher.fetch(message: message, space_guids: [shared_to_space.guid]).all
+ expect(results).to match_array([service_instance_1, service_instance_2])
+ end
+ end
+
+ context 'when a space contains both shared and non-shared service instances' do
+ let(:shared_to_space) { Space.make }
+ let!(:service_instance_4) { ManagedServiceInstance.make(space: shared_to_space) }
+
+ before do
+ service_instance_2.add_shared_space(shared_to_space)
+ service_instance_1.add_shared_space(shared_to_space)
+ end
+
+ it 'returns all of the service instances shared into the specified space' do
+ results = fetcher.fetch(message: message, space_guids: [shared_to_space.guid]).all
+ expect(results).to match_array([service_instance_1, service_instance_2, service_instance_4])
+ end
+ end
+ end
+ end
+end
diff --git a/vendor/errors/v2.yml b/vendor/errors/v2.yml
index 70d20c87ee8..818bf9bf196 100644
--- a/vendor/errors/v2.yml
+++ b/vendor/errors/v2.yml
@@ -1108,3 +1108,13 @@
name: ServiceInstanceUnshareFailed
http_code: 502
message: "Unshare of service instance failed because one or more bindings could not be deleted.\n\n%s"
+
+390002:
+ name: ServiceInstanceDeletionSharesExists
+ http_code: 400
+ message: "Service instances must be unshared before they can be deleted. Unsharing %s will automatically delete any bindings that have been made to applications in other spaces."
+
+390003:
+ name: ServiceShareIsDisabled
+ http_code: 400
+ message: "The %s service does not support service instance sharing."
|