From 0974196a0490f71442ba2a4335b32161c4483fef Mon Sep 17 00:00:00 2001 From: Alex Blease Date: Wed, 18 Oct 2017 10:45:12 +0100 Subject: [PATCH 01/14] Add tests for ServiceInstance.user_visibility_filter [#152035378] Signed-off-by: Jen Spinney --- .../models/services/service_instance_spec.rb | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/spec/unit/models/services/service_instance_spec.rb b/spec/unit/models/services/service_instance_spec.rb index 671bcc5b92f..36d37512c4f 100644 --- a/spec/unit/models/services/service_instance_spec.rb +++ b/spec/unit/models/services/service_instance_spec.rb @@ -326,5 +326,48 @@ 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 + end end end From d92ffaeca6c9ad8b19399d9991c9c68440b7c678 Mon Sep 17 00:00:00 2001 From: Jen Spinney Date: Wed, 18 Oct 2017 10:48:15 +0100 Subject: [PATCH 02/14] Look at shared spaces to determine user visibility of service instances [#152035378] Signed-off-by: Alex Blease --- app/models/services/service_instance.rb | 1 + .../models/services/service_instance_spec.rb | 24 +++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/app/models/services/service_instance.rb b/app/models/services/service_instance.rb index b2aea99e2af..62fdf11e168 100644 --- a/app/models/services/service_instance.rb +++ b/app/models/services/service_instance.rb @@ -68,6 +68,7 @@ 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], ]) end diff --git a/spec/unit/models/services/service_instance_spec.rb b/spec/unit/models/services/service_instance_spec.rb index 36d37512c4f..abd3347eb92 100644 --- a/spec/unit/models/services/service_instance_spec.rb +++ b/spec/unit/models/services/service_instance_spec.rb @@ -368,6 +368,30 @@ module VCAP::CloudController 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_space_auditor) { make_auditor_for_space(target_space) } + + 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 auditor in the target space' do + it 'the service instance is not visible' do + filter = ServiceInstance.user_visibility_filter(target_space_auditor) + expect(ServiceInstance.filter(filter).all.length).to eq(0) + end + end + end end end end From ec69e2758f19bcb2d65451c94e4dcfe2b868820d Mon Sep 17 00:00:00 2001 From: Alex Blease Date: Wed, 18 Oct 2017 14:53:59 +0100 Subject: [PATCH 03/14] Allow target space devs to view shared service instances /v2/spaces/:guid/services endpoint now includes service instances that have been shared into the given space. [#152035378] Signed-off-by: Jen Spinney --- app/access/service_instance_access.rb | 7 ++++ app/controllers/runtime/spaces_controller.rb | 16 ++++----- app/models/services/service_instance.rb | 2 +- .../runtime/spaces_controller_spec.rb | 33 +++++++++++++++++++ 4 files changed, 48 insertions(+), 10 deletions(-) diff --git a/app/access/service_instance_access.rb b/app/access/service_instance_access.rb index cd8ab240003..07d65219bed 100644 --- a/app/access/service_instance_access.rb +++ b/app/access/service_instance_access.rb @@ -1,6 +1,7 @@ module VCAP::CloudController class ServiceInstanceAccess < BaseAccess def create?(service_instance, params=nil) + return false unless service_instance.space return true if admin_user? FeatureFlag.raise_unless_enabled!(:service_instance_creation) return false if service_instance.in_suspended_org? @@ -8,6 +9,7 @@ def create?(service_instance, params=nil) end def read_for_update?(service_instance, params=nil) + return false unless service_instance.space return true if admin_user? return false if service_instance.in_suspended_org? service_instance.space.has_developer?(context.user) @@ -18,12 +20,14 @@ def update?(service_instance, params=nil) end def delete?(service_instance) + return false unless service_instance.space return true if admin_user? return false if service_instance.in_suspended_org? service_instance.space.has_developer?(context.user) end def manage_permissions?(service_instance) + return false unless service_instance.space return true if admin_user? service_instance.space.has_developer?(context.user) end @@ -33,6 +37,7 @@ def manage_permissions_with_token?(service_instance) end def read_permissions?(service_instance) + return false unless service_instance.space return true if admin_user? || admin_read_only_user? service_instance.space.has_member?(context.user) || service_instance.space.organization.managers.include?(context.user) end @@ -42,6 +47,7 @@ def read_permissions_with_token?(service_instance) end def read_env?(service_instance) + return false unless service_instance.space return true if admin_user? || admin_read_only_user? service_instance.space.has_developer?(context.user) end @@ -64,6 +70,7 @@ def allowed?(service_instance) end def purge?(service_instance) + return false unless service_instance.space admin_user? || (service_instance.space.has_developer?(context.user) && service_instance.service_broker.private?) 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/models/services/service_instance.rb b/app/models/services/service_instance.rb index 62fdf11e168..6f8c65613f0 100644 --- a/app/models/services/service_instance.rb +++ b/app/models/services/service_instance.rb @@ -135,7 +135,7 @@ def credentials_with_serialization alias_method_chain :credentials, 'serialization' def in_suspended_org? - space.in_suspended_org? + space && space.in_suspended_org? end def after_create diff --git a/spec/unit/controllers/runtime/spaces_controller_spec.rb b/spec/unit/controllers/runtime/spaces_controller_spec.rb index 7477158ad49..e407cf5af60 100644 --- a/spec/unit/controllers/runtime/spaces_controller_spec.rb +++ b/spec/unit/controllers/runtime/spaces_controller_spec.rb @@ -240,6 +240,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) } From a1449c43cfb6962b38fcad7fcfd0fcb8849d13ea Mon Sep 17 00:00:00 2001 From: Jen Spinney Date: Thu, 19 Oct 2017 16:24:24 +0100 Subject: [PATCH 04/14] Read permissions on service instance should be the same as read * The only visible change is that the global auditor role now receives "true" for read when calling /v2/service_instances/:guid/permissions. This tells the service broker that the user is allowed to view the service instance dashboard. They already have read access to the service instance, so we don't anticipate that this is a problem. [#152035378] Signed-off-by: Derik Evangelista --- app/access/service_instance_access.rb | 4 +--- .../controllers/services/service_instances_controller_spec.rb | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/app/access/service_instance_access.rb b/app/access/service_instance_access.rb index 07d65219bed..562919072cc 100644 --- a/app/access/service_instance_access.rb +++ b/app/access/service_instance_access.rb @@ -37,9 +37,7 @@ def manage_permissions_with_token?(service_instance) end def read_permissions?(service_instance) - return false unless service_instance.space - return true if admin_user? || admin_read_only_user? - service_instance.space.has_member?(context.user) || service_instance.space.organization.managers.include?(context.user) + read?(service_instance) end def read_permissions_with_token?(service_instance) diff --git a/spec/unit/controllers/services/service_instances_controller_spec.rb b/spec/unit/controllers/services/service_instances_controller_spec.rb index 6f6607fa9e2..157eb768ec9 100644 --- a/spec/unit/controllers/services/service_instances_controller_spec.rb +++ b/spec/unit/controllers/services/service_instances_controller_spec.rb @@ -3404,7 +3404,7 @@ def verify_forbidden(user) 'org_manager' => { manage: false, read: true }, 'admin' => { manage: true, read: true }, 'admin_read_only' => { manage: false, read: true }, - 'global_auditor' => { manage: false, read: false }, + 'global_auditor' => { manage: false, read: true }, }.each do |role, expected_return_values| context "as an #{role}" do before do From 03e1f83d78a1179181116aa37b11d61d88665733 Mon Sep 17 00:00:00 2001 From: Derik Evangelista Date: Fri, 20 Oct 2017 10:41:36 +0100 Subject: [PATCH 05/14] Allow target space auditors, managers and org managers to view shared spaces * Also cleaned up some of the null checking in service_instance_acess and added tests for access permissions on shared service instances. [#152035378] Signed-off-by: Jen Spinney --- app/access/service_instance_access.rb | 18 ++---- app/models/services/service_instance.rb | 3 + .../access/service_instance_access_spec.rb | 57 +++++++++++++++++++ .../models/services/service_instance_spec.rb | 41 ++++++++++++- 4 files changed, 106 insertions(+), 13 deletions(-) diff --git a/app/access/service_instance_access.rb b/app/access/service_instance_access.rb index 562919072cc..085de94693a 100644 --- a/app/access/service_instance_access.rb +++ b/app/access/service_instance_access.rb @@ -1,18 +1,16 @@ module VCAP::CloudController class ServiceInstanceAccess < BaseAccess def create?(service_instance, params=nil) - return false unless service_instance.space 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 && service_instance.space.has_developer?(context.user) && allowed?(service_instance) end def read_for_update?(service_instance, params=nil) - return false unless service_instance.space return true if admin_user? return false if service_instance.in_suspended_org? - service_instance.space.has_developer?(context.user) + service_instance.space && service_instance.space.has_developer?(context.user) end def update?(service_instance, params=nil) @@ -20,16 +18,14 @@ def update?(service_instance, params=nil) end def delete?(service_instance) - return false unless service_instance.space return true if admin_user? return false if service_instance.in_suspended_org? - service_instance.space.has_developer?(context.user) + service_instance.space && service_instance.space.has_developer?(context.user) end def manage_permissions?(service_instance) - return false unless service_instance.space return true if admin_user? - service_instance.space.has_developer?(context.user) + service_instance.space && service_instance.space.has_developer?(context.user) end def manage_permissions_with_token?(service_instance) @@ -45,9 +41,8 @@ def read_permissions_with_token?(service_instance) end def read_env?(service_instance) - return false unless service_instance.space return true if admin_user? || admin_read_only_user? - service_instance.space.has_developer?(context.user) + service_instance.space && service_instance.space.has_developer?(context.user) end def read_env_with_token?(service_instance) @@ -68,8 +63,7 @@ def allowed?(service_instance) end def purge?(service_instance) - return false unless service_instance.space - admin_user? || (service_instance.space.has_developer?(context.user) && service_instance.service_broker.private?) + admin_user? || (service_instance.space && service_instance.space.has_developer?(context.user) && service_instance.service_broker.private?) end def purge_with_token?(instance) diff --git a/app/models/services/service_instance.rb b/app/models/services/service_instance.rb index 6f8c65613f0..cb3d9d4ec56 100644 --- a/app/models/services/service_instance.rb +++ b/app/models/services/service_instance.rb @@ -69,6 +69,9 @@ def self.user_visibility_filter(user) [: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 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/models/services/service_instance_spec.rb b/spec/unit/models/services/service_instance_spec.rb index abd3347eb92..29b5cf92b7e 100644 --- a/spec/unit/models/services/service_instance_spec.rb +++ b/spec/unit/models/services/service_instance_spec.rb @@ -298,6 +298,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 @@ -372,7 +380,10 @@ module VCAP::CloudController 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) @@ -385,9 +396,37 @@ module VCAP::CloudController 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 not visible' 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 From 8e0cfde32a9bfb5c1ded9ec7551e90f5a34285e4 Mon Sep 17 00:00:00 2001 From: Denise Yu Date: Wed, 1 Nov 2017 11:18:07 +0000 Subject: [PATCH 06/14] Use &. operator instead of service_instance && service_instance.space [#152035378] Signed-off-by: Jen Spinney --- app/access/service_instance_access.rb | 12 ++++++------ app/models/services/service_instance.rb | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/access/service_instance_access.rb b/app/access/service_instance_access.rb index 085de94693a..65179d795b8 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 && 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 && 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 && 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 && service_instance.space.has_developer?(context.user) + service_instance.space&.has_developer?(context.user) end def manage_permissions_with_token?(service_instance) @@ -42,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 && service_instance.space.has_developer?(context.user) + service_instance.space&.has_developer?(context.user) end def read_env_with_token?(service_instance) @@ -63,7 +63,7 @@ def allowed?(service_instance) end def purge?(service_instance) - admin_user? || (service_instance.space && 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/models/services/service_instance.rb b/app/models/services/service_instance.rb index cb3d9d4ec56..b17fbf5078a 100644 --- a/app/models/services/service_instance.rb +++ b/app/models/services/service_instance.rb @@ -138,7 +138,7 @@ def credentials_with_serialization alias_method_chain :credentials, 'serialization' def in_suspended_org? - space && space.in_suspended_org? + space&.in_suspended_org? end def after_create From a72880e90d583d56c5eac897a11f6610e932fd30 Mon Sep 17 00:00:00 2001 From: Denise Yu Date: Tue, 7 Nov 2017 11:21:55 +0000 Subject: [PATCH 07/14] Remove global auditors from read_permissions access [#152631507] Signed-off-by: Denise Yu --- app/access/service_instance_access.rb | 2 +- .../controllers/services/service_instances_controller_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/access/service_instance_access.rb b/app/access/service_instance_access.rb index 65179d795b8..dce57e8faa5 100644 --- a/app/access/service_instance_access.rb +++ b/app/access/service_instance_access.rb @@ -33,7 +33,7 @@ def manage_permissions_with_token?(service_instance) end def read_permissions?(service_instance) - read?(service_instance) + admin_user? || admin_read_only_user? || object_is_visible_to_user?(service_instance, context.user) end def read_permissions_with_token?(service_instance) diff --git a/spec/unit/controllers/services/service_instances_controller_spec.rb b/spec/unit/controllers/services/service_instances_controller_spec.rb index 157eb768ec9..6f6607fa9e2 100644 --- a/spec/unit/controllers/services/service_instances_controller_spec.rb +++ b/spec/unit/controllers/services/service_instances_controller_spec.rb @@ -3404,7 +3404,7 @@ def verify_forbidden(user) 'org_manager' => { manage: false, read: true }, 'admin' => { manage: true, read: true }, 'admin_read_only' => { manage: false, read: true }, - 'global_auditor' => { manage: false, read: true }, + 'global_auditor' => { manage: false, read: false }, }.each do |role, expected_return_values| context "as an #{role}" do before do From 26d1e13b34f5a934aa53d4f91d63870ec8d52e59 Mon Sep 17 00:00:00 2001 From: Denise Yu Date: Tue, 24 Oct 2017 17:02:23 +0100 Subject: [PATCH 08/14] Implement /v3/service_instances endpoint * List all service instances that a user has permission to see [#152073155] Signed-off-by: Jen Spinney --- .../v3/service_instances_controller.rb | 19 +++ app/fetchers/service_instance_list_fetcher.rb | 25 ++++ .../service_instances_list_message.rb | 32 +++++ app/presenters/v3/paginated_list_presenter.rb | 4 +- .../v3/service_instance_presenter.rb | 24 ++++ config/routes.rb | 1 + spec/request/service_instances_spec.rb | 112 +++++++++++++++--- .../v3/service_instance_controller_spec.rb | 43 +++++++ .../service_instances_list_message_spec.rb | 60 ++++++++++ .../v3/service_instance_presenter_spec.rb | 20 ++++ .../service_instance_list_fetcher_spec.rb | 74 ++++++++++++ 11 files changed, 396 insertions(+), 18 deletions(-) create mode 100644 app/fetchers/service_instance_list_fetcher.rb create mode 100644 app/messages/service_instances/service_instances_list_message.rb create mode 100644 app/presenters/v3/service_instance_presenter.rb create mode 100644 spec/unit/messages/service_instances_list_message_spec.rb create mode 100644 spec/unit/presenters/v3/service_instance_presenter_spec.rb create mode 100644 spec/unit/queries/service_instance_list_fetcher_spec.rb diff --git a/app/controllers/v3/service_instances_controller.rb b/app/controllers/v3/service_instances_controller.rb index 3f1141b7ae4..1aef6c27de5 100644 --- a/app/controllers/v3/service_instances_controller.rb +++ b/app/controllers/v3/service_instances_controller.rb @@ -1,11 +1,30 @@ 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) diff --git a/app/fetchers/service_instance_list_fetcher.rb b/app/fetchers/service_instance_list_fetcher.rb new file mode 100644 index 00000000000..84fc0e5e022 --- /dev/null +++ b/app/fetchers/service_instance_list_fetcher.rb @@ -0,0 +1,25 @@ +module VCAP::CloudController + class ServiceInstanceListFetcher + def fetch(message:, space_guids:) + dataset = ServiceInstance.select_all(ServiceInstance.table_name). + join(Space.table_name, id: :space_id, guid: space_guids) + + 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/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/spec/request/service_instances_spec.rb b/spec/request/service_instances_spec.rb index c9e0a68bb80..9279b5d22c5 100644 --- a/spec/request/service_instances_spec.rb +++ b/spec/request/service_instances_spec.rb @@ -2,11 +2,89 @@ 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' => 'mysql', + 'created_at' => iso8601, + 'updated_at' => iso8601, + }, + { + 'guid' => service_instance1.guid, + 'name' => 'rabbitmq', + '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' => 'redis', + 'created_at' => iso8601, + 'updated_at' => iso8601, + } + ] + } + ) + end + end describe 'POST /v3/service_instances/:guid/relationships/shared_spaces' do before do @@ -20,7 +98,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 +108,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,11 +122,11 @@ 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 @@ -68,12 +146,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 +161,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/unit/controllers/v3/service_instance_controller_spec.rb b/spec/unit/controllers/v3/service_instance_controller_spec.rb index 16dad33ef32..e1a1b7d7e62 100644 --- a/spec/unit/controllers/v3/service_instance_controller_spec.rb +++ b/spec/unit/controllers/v3/service_instance_controller_spec.rb @@ -2,6 +2,49 @@ 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, name: 'awesome service') } + + describe '#index' do + 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 ? ['awesome service'] : [] + + 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 } 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/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_instance_list_fetcher_spec.rb b/spec/unit/queries/service_instance_list_fetcher_spec.rb new file mode 100644 index 00000000000..92accad7245 --- /dev/null +++ b/spec/unit/queries/service_instance_list_fetcher_spec.rb @@ -0,0 +1,74 @@ +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', 'redis'] } } + + 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, service_instance_2]) + 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 + end + end +end From 021bf4e7e6c19bda419a2524c8ee3a6d9668f24d Mon Sep 17 00:00:00 2001 From: Denise Yu Date: Fri, 27 Oct 2017 10:37:11 +0100 Subject: [PATCH 09/14] Add docs for list V3 service instances [#152073155] Signed-off-by: Alex Blease --- .../api_resources/_service_instances.erb | 25 ++++++++++++++ .../service_instances/_list.md.erb | 34 +++++++++++++++++++ docs/v3/source/index.md | 1 + 3 files changed, 60 insertions(+) create mode 100644 docs/v3/source/includes/experimental_resources/service_instances/_list.md.erb 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..6a10dd62241 --- /dev/null +++ b/docs/v3/source/includes/experimental_resources/service_instances/_list.md.erb @@ -0,0 +1,34 @@ +### 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. + +#### 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 From 74bbcc12f810b259a7a6304cf10de1abcd334709 Mon Sep 17 00:00:00 2001 From: Sam Gunaratne Date: Mon, 30 Oct 2017 16:46:45 +0000 Subject: [PATCH 10/14] GET /v3/service_instances includes shared instances This change allows developers who have been granted access to a service instance through service instance sharing, to query that service instance via /v3/service_instances. [#152344116] Signed-off-by: Alex Blease --- app/fetchers/service_instance_list_fetcher.rb | 9 ++- .../service_instances/_list.md.erb | 2 +- spec/request/service_instances_spec.rb | 44 +++++++++++- .../v3/service_instance_controller_spec.rb | 68 ++++++++++++++++++- .../service_instance_list_fetcher_spec.rb | 33 ++++++++- 5 files changed, 146 insertions(+), 10 deletions(-) diff --git a/app/fetchers/service_instance_list_fetcher.rb b/app/fetchers/service_instance_list_fetcher.rb index 84fc0e5e022..01212a3f3b1 100644 --- a/app/fetchers/service_instance_list_fetcher.rb +++ b/app/fetchers/service_instance_list_fetcher.rb @@ -1,8 +1,13 @@ module VCAP::CloudController class ServiceInstanceListFetcher def fetch(message:, space_guids:) - dataset = ServiceInstance.select_all(ServiceInstance.table_name). - join(Space.table_name, id: :space_id, guid: 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 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 index 6a10dd62241..585f9aa095c 100644 --- a/docs/v3/source/includes/experimental_resources/service_instances/_list.md.erb +++ b/docs/v3/source/includes/experimental_resources/service_instances/_list.md.erb @@ -20,7 +20,7 @@ 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. +This endpoint retrieves the service instances the user has access to. This includes access granted by service instance sharing. #### Definition `GET /v3/service_instances` diff --git a/spec/request/service_instances_spec.rb b/spec/request/service_instances_spec.rb index 9279b5d22c5..b0376067818 100644 --- a/spec/request/service_instances_spec.rb +++ b/spec/request/service_instances_spec.rb @@ -38,13 +38,13 @@ 'resources' => [ { 'guid' => service_instance3.guid, - 'name' => 'mysql', + 'name' => service_instance3.name, 'created_at' => iso8601, 'updated_at' => iso8601, }, { 'guid' => service_instance1.guid, - 'name' => 'rabbitmq', + 'name' => service_instance1.name, 'created_at' => iso8601, 'updated_at' => iso8601, } @@ -76,7 +76,7 @@ 'resources' => [ { 'guid' => service_instance2.guid, - 'name' => 'redis', + 'name' => service_instance2.name, 'created_at' => iso8601, 'updated_at' => iso8601, } @@ -84,6 +84,44 @@ } ) 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 diff --git a/spec/unit/controllers/v3/service_instance_controller_spec.rb b/spec/unit/controllers/v3/service_instance_controller_spec.rb index e1a1b7d7e62..b2025bc5e7f 100644 --- a/spec/unit/controllers/v3/service_instance_controller_spec.rb +++ b/spec/unit/controllers/v3/service_instance_controller_spec.rb @@ -3,9 +3,44 @@ 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, name: 'awesome service') } + 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, @@ -24,7 +59,36 @@ 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 ? ['awesome service'] : [] + 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 diff --git a/spec/unit/queries/service_instance_list_fetcher_spec.rb b/spec/unit/queries/service_instance_list_fetcher_spec.rb index 92accad7245..fa07743ce8d 100644 --- a/spec/unit/queries/service_instance_list_fetcher_spec.rb +++ b/spec/unit/queries/service_instance_list_fetcher_spec.rb @@ -52,11 +52,11 @@ module VCAP::CloudController context 'filter' do context 'by service instance name' do - let(:filters) { { names: ['rabbitmq', 'redis'] } } + 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, service_instance_2]) + expect(results).to match_array([service_instance_1]) end end @@ -69,6 +69,35 @@ module VCAP::CloudController 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 From d4711428fa77e22b79f4abb08a29e557a97a809d Mon Sep 17 00:00:00 2001 From: Denise Yu Date: Wed, 1 Nov 2017 16:43:44 +0000 Subject: [PATCH 11/14] Document that /v3/service_instances only returns managed instances [#152344116] Signed-off-by: Jen Spinney --- .../experimental_resources/service_instances/_list.md.erb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 index 585f9aa095c..de938427740 100644 --- a/docs/v3/source/includes/experimental_resources/service_instances/_list.md.erb +++ b/docs/v3/source/includes/experimental_resources/service_instances/_list.md.erb @@ -20,7 +20,9 @@ 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. This includes access granted by service instance sharing. +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` From 4dab92bce965faae121c7b06c026f49839443212 Mon Sep 17 00:00:00 2001 From: Jen Spinney Date: Wed, 8 Nov 2017 11:07:57 +0000 Subject: [PATCH 12/14] Service instances have /v2/service_instances/:guid/shared_from endpoint * All service_instances have shared_from_url * shared_from endpoint returns space name and org name of service instance, if that service instance has been shared * If not shared, this endpoint returns 204 No Content [#150973038] Signed-off-by: Denise Yu --- .../services/service_instances_controller.rb | 18 +++ app/models/services/service_instance.rb | 4 + .../v2/service_instance_presenter.rb | 1 + .../service_instance_shared_from_presenter.rb | 14 ++ docs/v2/index.html | 3 + ...binding_a_service_instance_to_a_route.html | 3 +- .../creating_a_service_instance.html | 19 ++- .../delete_a_service_instance.html | 19 ++- .../list_all_service_instances.html | 19 ++- ...etrieve_a_particular_service_instance.html | 19 ++- ...where_service_instance_is_shared_from.html | 152 ++++++++++++++++++ .../update_a_service_instance.html | 19 ++- ...ervice_instances_for_the_service_plan.html | 3 +- ...t_all_service_instances_for_the_space.html | 3 +- spec/request/v2/service_bindings_spec.rb | 3 +- spec/request/v2/service_instances_spec.rb | 63 +++++++- spec/request/v2/spaces_spec.rb | 58 ++++++- .../runtime/spaces_controller_spec.rb | 8 + .../service_instances_controller_spec.rb | 107 ++++++++++++ .../models/services/service_instance_spec.rb | 18 +++ .../v2/service_instance_presenter_spec.rb | 61 ++++--- ...ice_instance_shared_from_presenter_spec.rb | 18 +++ 22 files changed, 595 insertions(+), 37 deletions(-) create mode 100644 app/presenters/v2/service_instance_shared_from_presenter.rb create mode 100644 docs/v2/service_instances/retrieve_info_where_service_instance_is_shared_from.html create mode 100644 spec/unit/presenters/v2/service_instance_shared_from_presenter_spec.rb diff --git a/app/controllers/services/service_instances_controller.rb b/app/controllers/services/service_instances_controller.rb index 44cc9ebf2ea..bda6c82755b 100644 --- a/app/controllers/services/service_instances_controller.rb +++ b/app/controllers/services/service_instances_controller.rb @@ -5,6 +5,7 @@ require 'controllers/services/lifecycle/service_instance_deprovisioner' require 'controllers/services/lifecycle/service_instance_purger' require 'fetchers/service_instance_fetcher' +require 'presenters/v2/service_instance_shared_from_presenter' module VCAP::CloudController class ServiceInstancesController < RestController::ModelController @@ -205,6 +206,23 @@ 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 + def self.url_for_guid(guid) object = ServiceInstance.where(guid: guid).first diff --git a/app/models/services/service_instance.rb b/app/models/services/service_instance.rb index b17fbf5078a..02159e1d975 100644 --- a/app/models/services/service_instance.rb +++ b/app/models/services/service_instance.rb @@ -175,6 +175,10 @@ 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..82a678adc70 100644 --- a/app/presenters/v2/service_instance_presenter.rb +++ b/app/presenters/v2/service_instance_presenter.rb @@ -28,6 +28,7 @@ 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" 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/docs/v2/index.html b/docs/v2/index.html index b2e77eb7adc..cc8cb8c269f 100644 --- a/docs/v2/index.html +++ b/docs/v2/index.html @@ -875,6 +875,9 @@

  • Retrieving permissions on a Service Instance
  • +
  • + Retrieve Information Regarding Where a Service Instance is Shared From (Experimental) +
  • Unbinding a service instance from a route
  • diff --git a/docs/v2/service_instances/binding_a_service_instance_to_a_route.html b/docs/v2/service_instances/binding_a_service_instance_to_a_route.html index 9ef1f2357c0..09718949180 100644 --- a/docs/v2/service_instances/binding_a_service_instance_to_a_route.html +++ b/docs/v2/service_instances/binding_a_service_instance_to_a_route.html @@ -164,7 +164,8 @@

    Body

    "service_plan_url": "/v2/service_plans/4cfefd27-3eb7-4f55-806e-1db649038ad2", "service_bindings_url": "/v2/service_instances/6da8d173-b409-4094-949f-3c1cc8a68503/service_bindings", "service_keys_url": "/v2/service_instances/6da8d173-b409-4094-949f-3c1cc8a68503/service_keys", - "routes_url": "/v2/service_instances/6da8d173-b409-4094-949f-3c1cc8a68503/routes" + "routes_url": "/v2/service_instances/6da8d173-b409-4094-949f-3c1cc8a68503/routes", + "shared_from_url": "/v2/service_instances/6da8d173-b409-4094-949f-3c1cc8a68503/shared_from" } } diff --git a/docs/v2/service_instances/creating_a_service_instance.html b/docs/v2/service_instances/creating_a_service_instance.html index ba70c81d34a..0fba375a0cc 100644 --- a/docs/v2/service_instances/creating_a_service_instance.html +++ b/docs/v2/service_instances/creating_a_service_instance.html @@ -557,6 +557,22 @@

    Body

    + + + shared_from_url + + + Source information for a shared service instance. Users of a shared service instance can find out its space and org information. + + +
      +
    + + +
      +
    + + tags @@ -608,7 +624,8 @@

    Body

    "service_plan_url": "/v2/service_plans/fe173a83-df28-4891-8d91-46334e04600d", "service_bindings_url": "/v2/service_instances/cc3b67fa-cda6-4df7-ba47-eb5f2a123992/service_bindings", "service_keys_url": "/v2/service_instances/cc3b67fa-cda6-4df7-ba47-eb5f2a123992/service_keys", - "routes_url": "/v2/service_instances/cc3b67fa-cda6-4df7-ba47-eb5f2a123992/routes" + "routes_url": "/v2/service_instances/cc3b67fa-cda6-4df7-ba47-eb5f2a123992/routes", + "shared_from_url": "/v2/service_instances/0d632575-bb06-4ea5-bb19-a451a9644d92/shared_from" } } diff --git a/docs/v2/service_instances/delete_a_service_instance.html b/docs/v2/service_instances/delete_a_service_instance.html index d97550d3529..69f9b183e8c 100644 --- a/docs/v2/service_instances/delete_a_service_instance.html +++ b/docs/v2/service_instances/delete_a_service_instance.html @@ -461,6 +461,22 @@

    Body

    + + + shared_from_url + + + Source information for a shared service instance. Users of a shared service instance can find out its space and org information. + + +
      +
    + + +
      +
    + + tags @@ -512,7 +528,8 @@

    Body

    "service_plan_url": "/v2/service_plans/8ea19d29-2e20-469e-8b91-917a6410e2f2", "service_bindings_url": "/v2/service_instances/1aaeb02d-16c3-4405-bc41-80e83d196dff/service_bindings", "service_keys_url": "/v2/service_instances/1aaeb02d-16c3-4405-bc41-80e83d196dff/service_keys", - "routes_url": "/v2/service_instances/1aaeb02d-16c3-4405-bc41-80e83d196dff/routes" + "routes_url": "/v2/service_instances/1aaeb02d-16c3-4405-bc41-80e83d196dff/routes", + "shared_from_url": "/v2/service_instances/6da8d173-b409-4094-949f-3c1cc8a68503/shared_from" } } diff --git a/docs/v2/service_instances/list_all_service_instances.html b/docs/v2/service_instances/list_all_service_instances.html index 9b24866534e..8947de4b9c3 100644 --- a/docs/v2/service_instances/list_all_service_instances.html +++ b/docs/v2/service_instances/list_all_service_instances.html @@ -512,6 +512,22 @@

    Body

    + + + shared_from_url + + + Source information for a shared service instance. Users of a shared service instance can find out its space and org information. + + +
      +
    + + +
      +
    + + tags @@ -571,7 +587,8 @@

    Body

    "service_plan_url": "/v2/service_plans/05a372c6-6dc2-4f7f-8f65-a90ebe5fa6e2", "service_bindings_url": "/v2/service_instances/215b97be-ec77-4224-9c38-c4f2d86b56c1/service_bindings", "service_keys_url": "/v2/service_instances/215b97be-ec77-4224-9c38-c4f2d86b56c1/service_keys", - "routes_url": "/v2/service_instances/215b97be-ec77-4224-9c38-c4f2d86b56c1/routes" + "routes_url": "/v2/service_instances/215b97be-ec77-4224-9c38-c4f2d86b56c1/routes", + "shared_from_url": "/v2/service_instances/0d632575-bb06-4ea5-bb19-a451a9644d92/shared_from" } } ] diff --git a/docs/v2/service_instances/retrieve_a_particular_service_instance.html b/docs/v2/service_instances/retrieve_a_particular_service_instance.html index 3b81aee4e6a..b197d20c32c 100644 --- a/docs/v2/service_instances/retrieve_a_particular_service_instance.html +++ b/docs/v2/service_instances/retrieve_a_particular_service_instance.html @@ -368,6 +368,22 @@

    Body

    + + + shared_from_url + + + Source information for a shared service instance. Users of a shared service instance can find out its space and org information. + + +
      +
    + + +
      +
    + + tags @@ -421,7 +437,8 @@

    Body

    "service_plan_url": "/v2/service_plans/779d2df0-9cdd-48e8-9781-ea05301cedb1", "service_bindings_url": "/v2/service_instances/0d632575-bb06-4ea5-bb19-a451a9644d92/service_bindings", "service_keys_url": "/v2/service_instances/0d632575-bb06-4ea5-bb19-a451a9644d92/service_keys", - "routes_url": "/v2/service_instances/0d632575-bb06-4ea5-bb19-a451a9644d92/routes" + "routes_url": "/v2/service_instances/0d632575-bb06-4ea5-bb19-a451a9644d92/routes", + "shared_from_url": "/v2/service_instances/0d632575-bb06-4ea5-bb19-a451a9644d92/shared_from" } } diff --git a/docs/v2/service_instances/retrieve_info_where_service_instance_is_shared_from.html b/docs/v2/service_instances/retrieve_info_where_service_instance_is_shared_from.html new file mode 100644 index 00000000000..eadbbdcf0e2 --- /dev/null +++ b/docs/v2/service_instances/retrieve_info_where_service_instance_is_shared_from.html @@ -0,0 +1,152 @@ + + + + Service Instances API + + + + + + +
    +

    Service Instances API

    + +
    +

    Retrieve Information Regarding Where a Service Instance is Shared From (Experimental)

    +

    GET /v2/service_instances/:guid/shared_from

    + +

    Request

    +

    Route

    +
    GET /v2/service_instances/0d632575-bb06-4ea5-bb19-a451a9644d92/shared_from
    + +

    Headers

    +
    Authorization: bearer eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJ1c2VyX2lkIjoidWFhLWlkLTE1MCIsImVtYWlsIjoiZW1haWwtMTA1QHNvbWVkb21haW4uY29tIiwic2NvcGUiOlsiY2xvdWRfY29udHJvbGxlci5hZG1pbiJdLCJhdWQiOlsiY2xvdWRfY29udHJvbGxlciJdLCJleHAiOjE0NjYwMDg4ODl9.eyvSz10WWSQt0pFotoQI2Lm5XdH22-50O_R8rxy7n4k
    +Host: example.org
    +Cookie: 
    + +

    cURL

    +
    curl "https://api.[your-domain.com]/v2/service_instances/0d632575-bb06-4ea5-bb19-a451a9644d92/shared_from" -X GET \
    +	-H "Authorization: bearer eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJ1c2VyX2lkIjoidWFhLWlkLTE1MCIsImVtYWlsIjoiZW1haWwtMTA1QHNvbWVkb21haW4uY29tIiwic2NvcGUiOlsiY2xvdWRfY29udHJvbGxlci5hZG1pbiJdLCJhdWQiOlsiY2xvdWRfY29udHJvbGxlciJdLCJleHAiOjE0NjYwMDg4ODl9.eyvSz10WWSQt0pFotoQI2Lm5XdH22-50O_R8rxy7n4k" \
    +	-H "Host: example.org" \
    +	-H "Cookie: "
    + +

    Response

    + +

    Status

    +
    200 OK
    + +

    Body

    + + + + + + + + + + + + + + + + + + + + + + + +
    NameDescriptionValid ValuesExample Values
    + space_name + + The space name that this service instance belongs to. + +
      +
    +
    +
      +
    +
    + organization_name + + The organization name that this service instance belongs to. + +
      +
    +
    +
      +
    +
    + +
    {
    +    "space_name": "space-name",
    +    "organization_name": "org-name"
    +}
    +
    + +

    Headers

    +
    Content-Type: application/json;charset=utf-8
    +X-VCAP-Request-ID: febb14ab-a7bc-492d-b017-3db59d83967d
    +Content-Length: 253
    +X-Content-Type-Options: nosniff
    +
    +
    + + diff --git a/docs/v2/service_instances/update_a_service_instance.html b/docs/v2/service_instances/update_a_service_instance.html index 8c5bee3b126..9392093c80c 100644 --- a/docs/v2/service_instances/update_a_service_instance.html +++ b/docs/v2/service_instances/update_a_service_instance.html @@ -499,6 +499,22 @@

    Body

    + + + shared_from_url + + + Source information for a shared service instance. Users of a shared service instance can find out its space and org information. + + +
      +
    + + +
      +
    + + tags @@ -549,7 +565,8 @@

    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" } } 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..30c860e00bf 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,8 @@

    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" } } ] 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..5c4fda1066f 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,8 @@

    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" } } ] diff --git a/spec/request/v2/service_bindings_spec.rb b/spec/request/v2/service_bindings_spec.rb index 11aea28425a..0f45371e09a 100644 --- a/spec/request/v2/service_bindings_spec.rb +++ b/spec/request/v2/service_bindings_spec.rb @@ -197,7 +197,8 @@ '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" } } } diff --git a/spec/request/v2/service_instances_spec.rb b/spec/request/v2/service_instances_spec.rb index 8708ea9e454..97cffdf5878 100644 --- a/spec/request/v2/service_instances_spec.rb +++ b/spec/request/v2/service_instances_spec.rb @@ -54,7 +54,8 @@ '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" } } ) @@ -96,7 +97,8 @@ '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" } } ) @@ -137,7 +139,8 @@ '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" } } ) @@ -145,4 +148,58 @@ 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 end diff --git a/spec/request/v2/spaces_spec.rb b/spec/request/v2/spaces_spec.rb index bdd08d3d8ed..204896abd6e 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,62 @@ 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" + } + }] + }) + 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/unit/controllers/runtime/spaces_controller_spec.rb b/spec/unit/controllers/runtime/spaces_controller_spec.rb index e407cf5af60..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') diff --git a/spec/unit/controllers/services/service_instances_controller_spec.rb b/spec/unit/controllers/services/service_instances_controller_spec.rb index 6f6607fa9e2..c69b134972b 100644 --- a/spec/unit/controllers/services/service_instances_controller_spec.rb +++ b/spec/unit/controllers/services/service_instances_controller_spec.rb @@ -3496,6 +3496,113 @@ 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/service_keys' do let(:space) { Space.make } let(:manager) { make_manager_for_space(space) } diff --git a/spec/unit/models/services/service_instance_spec.rb b/spec/unit/models/services/service_instance_spec.rb index 29b5cf92b7e..52bb0ff5662 100644 --- a/spec/unit/models/services/service_instance_spec.rb +++ b/spec/unit/models/services/service_instance_spec.rb @@ -432,5 +432,23 @@ module VCAP::CloudController 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/presenters/v2/service_instance_presenter_spec.rb b/spec/unit/presenters/v2/service_instance_presenter_spec.rb index 804e909e196..638abbd6065 100644 --- a/spec/unit/presenters/v2/service_instance_presenter_spec.rb +++ b/spec/unit/presenters/v2/service_instance_presenter_spec.rb @@ -11,35 +11,48 @@ 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", + } + ) + 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 From 59de5046b566561676a7e910eeee9a1eb1323260 Mon Sep 17 00:00:00 2001 From: Denise Yu Date: Wed, 8 Nov 2017 11:55:09 +0000 Subject: [PATCH 13/14] Service instances have /v2/service_instances/:guid/shared_to endpoint * All service_instance responses now return a shared_to_url * /v2/service_instances/:guid/shared_to endpoint returns a list of space and org name pairs indicating where the instance has been shared to [#151432798] Signed-off-by: Jen Spinney --- .../services/service_instances_controller.rb | 41 ++++ .../v2/service_instance_presenter.rb | 1 + .../service_instance_shared_to_presenter.rb | 13 + docs/v2/index.html | 3 + ...binding_a_service_instance_to_a_route.html | 3 +- .../creating_a_service_instance.html | 19 +- .../delete_a_service_instance.html | 19 +- .../list_all_service_instances.html | 19 +- ...etrieve_a_particular_service_instance.html | 19 +- ...o_where_service_instance_is_shared_to.html | 226 ++++++++++++++++++ .../update_a_service_instance.html | 19 +- ...ervice_instances_for_the_service_plan.html | 3 +- ...t_all_service_instances_for_the_space.html | 3 +- spec/request/v2/service_bindings_spec.rb | 3 +- spec/request/v2/service_instances_spec.rb | 46 +++- spec/request/v2/spaces_spec.rb | 3 +- .../service_instances_controller_spec.rb | 113 +++++++++ .../v2/service_instance_presenter_spec.rb | 1 + ...rvice_instance_shared_to_presenter_spec.rb | 18 ++ 19 files changed, 559 insertions(+), 13 deletions(-) create mode 100644 app/presenters/v2/service_instance_shared_to_presenter.rb create mode 100644 docs/v2/service_instances/retrieve_info_where_service_instance_is_shared_to.html create mode 100644 spec/unit/presenters/v2/service_instance_shared_to_presenter_spec.rb diff --git a/app/controllers/services/service_instances_controller.rb b/app/controllers/services/service_instances_controller.rb index bda6c82755b..97e2c9294dd 100644 --- a/app/controllers/services/service_instances_controller.rb +++ b/app/controllers/services/service_instances_controller.rb @@ -5,6 +5,7 @@ require 'controllers/services/lifecycle/service_instance_deprovisioner' require 'controllers/services/lifecycle/service_instance_purger' require 'fetchers/service_instance_fetcher' +require 'presenters/v2/service_instance_shared_to_presenter' require 'presenters/v2/service_instance_shared_from_presenter' module VCAP::CloudController @@ -223,6 +224,29 @@ def shared_from_information(guid) 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.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 @@ -321,6 +345,23 @@ def unbind_route(route_guid, instance_guid) private + class ServiceInstanceSharedToSerializer + def serialize(controller, space, opts, orphans=nil) + CloudController::Presenters::V2::ServiceInstanceSharedToPresenter.new.to_hash(space) + end + end + + def create_paginated_collection_renderer + VCAP::CloudController::RestController::PaginatedCollectionRenderer.new( + VCAP::CloudController::RestController::SecureEagerLoader.new, + ServiceInstanceSharedToSerializer.new, + { + 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 diff --git a/app/presenters/v2/service_instance_presenter.rb b/app/presenters/v2/service_instance_presenter.rb index 82a678adc70..8f688d6a781 100644 --- a/app/presenters/v2/service_instance_presenter.rb +++ b/app/presenters/v2/service_instance_presenter.rb @@ -29,6 +29,7 @@ def entity_hash(controller, obj, opts, depth, parents, orphans=nil) 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_to_presenter.rb b/app/presenters/v2/service_instance_shared_to_presenter.rb new file mode 100644 index 00000000000..70f927420c7 --- /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) + super(space) + end + end + end + end +end diff --git a/docs/v2/index.html b/docs/v2/index.html index cc8cb8c269f..6896378863e 100644 --- a/docs/v2/index.html +++ b/docs/v2/index.html @@ -878,6 +878,9 @@

  • Retrieve Information Regarding Where a Service Instance is Shared From (Experimental)
  • +
  • + Retrieve Information Regarding Where a Service Instance is Shared To (Experimental) +
  • Unbinding a service instance from a route
  • diff --git a/docs/v2/service_instances/binding_a_service_instance_to_a_route.html b/docs/v2/service_instances/binding_a_service_instance_to_a_route.html index 09718949180..1912be9e411 100644 --- a/docs/v2/service_instances/binding_a_service_instance_to_a_route.html +++ b/docs/v2/service_instances/binding_a_service_instance_to_a_route.html @@ -165,7 +165,8 @@

    Body

    "service_bindings_url": "/v2/service_instances/6da8d173-b409-4094-949f-3c1cc8a68503/service_bindings", "service_keys_url": "/v2/service_instances/6da8d173-b409-4094-949f-3c1cc8a68503/service_keys", "routes_url": "/v2/service_instances/6da8d173-b409-4094-949f-3c1cc8a68503/routes", - "shared_from_url": "/v2/service_instances/6da8d173-b409-4094-949f-3c1cc8a68503/shared_from" + "shared_from_url": "/v2/service_instances/6da8d173-b409-4094-949f-3c1cc8a68503/shared_from", + "shared_to_url": "/v2/service_instances/6da8d173-b409-4094-949f-3c1cc8a68503/shared_to" } } diff --git a/docs/v2/service_instances/creating_a_service_instance.html b/docs/v2/service_instances/creating_a_service_instance.html index 0fba375a0cc..2e6c57b78a7 100644 --- a/docs/v2/service_instances/creating_a_service_instance.html +++ b/docs/v2/service_instances/creating_a_service_instance.html @@ -573,6 +573,22 @@

    Body

    + + + shared_to_url + + + Information about where the service instance instance has been shared to. + + +
      +
    + + +
      +
    + + tags @@ -625,7 +641,8 @@

    Body

    "service_bindings_url": "/v2/service_instances/cc3b67fa-cda6-4df7-ba47-eb5f2a123992/service_bindings", "service_keys_url": "/v2/service_instances/cc3b67fa-cda6-4df7-ba47-eb5f2a123992/service_keys", "routes_url": "/v2/service_instances/cc3b67fa-cda6-4df7-ba47-eb5f2a123992/routes", - "shared_from_url": "/v2/service_instances/0d632575-bb06-4ea5-bb19-a451a9644d92/shared_from" + "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_instances/delete_a_service_instance.html b/docs/v2/service_instances/delete_a_service_instance.html index 69f9b183e8c..077ad3f0a72 100644 --- a/docs/v2/service_instances/delete_a_service_instance.html +++ b/docs/v2/service_instances/delete_a_service_instance.html @@ -477,6 +477,22 @@

    Body

    + + + shared_to_url + + + Information about where the service instance instance has been shared to. + + +
      +
    + + +
      +
    + + tags @@ -529,7 +545,8 @@

    Body

    "service_bindings_url": "/v2/service_instances/1aaeb02d-16c3-4405-bc41-80e83d196dff/service_bindings", "service_keys_url": "/v2/service_instances/1aaeb02d-16c3-4405-bc41-80e83d196dff/service_keys", "routes_url": "/v2/service_instances/1aaeb02d-16c3-4405-bc41-80e83d196dff/routes", - "shared_from_url": "/v2/service_instances/6da8d173-b409-4094-949f-3c1cc8a68503/shared_from" + "shared_from_url": "/v2/service_instances/6da8d173-b409-4094-949f-3c1cc8a68503/shared_from", + "shared_to_url": "/v2/service_instances/6da8d173-b409-4094-949f-3c1cc8a68503/shared_to" } } diff --git a/docs/v2/service_instances/list_all_service_instances.html b/docs/v2/service_instances/list_all_service_instances.html index 8947de4b9c3..16be8a03de0 100644 --- a/docs/v2/service_instances/list_all_service_instances.html +++ b/docs/v2/service_instances/list_all_service_instances.html @@ -528,6 +528,22 @@

    Body

    + + + shared_to_url + + + Information about where the service instance instance has been shared to. + + +
      +
    + + +
      +
    + + tags @@ -588,7 +604,8 @@

    Body

    "service_bindings_url": "/v2/service_instances/215b97be-ec77-4224-9c38-c4f2d86b56c1/service_bindings", "service_keys_url": "/v2/service_instances/215b97be-ec77-4224-9c38-c4f2d86b56c1/service_keys", "routes_url": "/v2/service_instances/215b97be-ec77-4224-9c38-c4f2d86b56c1/routes", - "shared_from_url": "/v2/service_instances/0d632575-bb06-4ea5-bb19-a451a9644d92/shared_from" + "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_instances/retrieve_a_particular_service_instance.html b/docs/v2/service_instances/retrieve_a_particular_service_instance.html index b197d20c32c..22431727eed 100644 --- a/docs/v2/service_instances/retrieve_a_particular_service_instance.html +++ b/docs/v2/service_instances/retrieve_a_particular_service_instance.html @@ -384,6 +384,22 @@

    Body

    + + + shared_to_url + + + Information about where the service instance instance has been shared to. + + +
      +
    + + +
      +
    + + tags @@ -438,7 +454,8 @@

    Body

    "service_bindings_url": "/v2/service_instances/0d632575-bb06-4ea5-bb19-a451a9644d92/service_bindings", "service_keys_url": "/v2/service_instances/0d632575-bb06-4ea5-bb19-a451a9644d92/service_keys", "routes_url": "/v2/service_instances/0d632575-bb06-4ea5-bb19-a451a9644d92/routes", - "shared_from_url": "/v2/service_instances/0d632575-bb06-4ea5-bb19-a451a9644d92/shared_from" + "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_instances/retrieve_info_where_service_instance_is_shared_to.html b/docs/v2/service_instances/retrieve_info_where_service_instance_is_shared_to.html new file mode 100644 index 00000000000..d3686b1ae0b --- /dev/null +++ b/docs/v2/service_instances/retrieve_info_where_service_instance_is_shared_to.html @@ -0,0 +1,226 @@ + + + + Service Instances API + + + + + + +
    +

    Service Instances API

    + +
    +

    Retrieve Information Regarding Where a Service Instance is Shared To (Experimental)

    +

    GET /v2/service_instances/:guid/shared_to

    + +

    Request

    +

    Route

    +
    GET /v2/service_instances/0d632575-bb06-4ea5-bb19-a451a9644d92/shared_to
    + +

    Parameters

    + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
    NameDescriptionValid ValuesExample Values
    + page + + Page of results to fetch + +
      +
    +
    +
      +
    +
    + results-per-page + + Number of results per page + +
      +
    +
    +
      +
    +
    + order-direction + + Order of the results: asc (default) or desc + +
      +
    +
    +
      +
    +
    + +

    Headers

    +
    Authorization: bearer eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJ1c2VyX2lkIjoidWFhLWlkLTE1MCIsImVtYWlsIjoiZW1haWwtMTA1QHNvbWVkb21haW4uY29tIiwic2NvcGUiOlsiY2xvdWRfY29udHJvbGxlci5hZG1pbiJdLCJhdWQiOlsiY2xvdWRfY29udHJvbGxlciJdLCJleHAiOjE0NjYwMDg4ODl9.eyvSz10WWSQt0pFotoQI2Lm5XdH22-50O_R8rxy7n4k
    +Host: example.org
    +Cookie: 
    + +

    cURL

    +
    curl "https://api.[your-domain.com]/v2/service_instances/0d632575-bb06-4ea5-bb19-a451a9644d92/shared_to" -X GET \
    +	-H "Authorization: bearer eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJ1c2VyX2lkIjoidWFhLWlkLTE1MCIsImVtYWlsIjoiZW1haWwtMTA1QHNvbWVkb21haW4uY29tIiwic2NvcGUiOlsiY2xvdWRfY29udHJvbGxlci5hZG1pbiJdLCJhdWQiOlsiY2xvdWRfY29udHJvbGxlciJdLCJleHAiOjE0NjYwMDg4ODl9.eyvSz10WWSQt0pFotoQI2Lm5XdH22-50O_R8rxy7n4k" \
    +	-H "Host: example.org" \
    +	-H "Cookie: "
    + +

    Response

    + +

    Status

    +
    200 OK
    + +

    Body

    + + + + + + + + + + + + + + + + + + + + + + + +
    NameDescriptionValid ValuesExample Values
    + space_name + + The name of the space being shared to. + +
      +
    +
    +
      +
    +
    + organization_name + + The name of the organization of the space the service instance has been shared to. + +
      +
    +
    +
      +
    +
    + +
    {
    +   "total_results": 2,
    +   "total_pages": 1,
    +   "prev_url": null,
    +   "next_url": null,
    +   "resources": [
    +      {
    +          "space_name": "target-space-1",
    +          "organization_name": "org-name"
    +      },
    +      {
    +          "space_name": "target-space-2",
    +          "organization_name": "org-name"
    +      }
    +   ]
    +}
    + +

    Headers

    +
    Content-Type: application/json;charset=utf-8
    +X-VCAP-Request-ID: febb14ab-a7bc-492d-b017-3db59d83967d
    +Content-Length: 378
    +X-Content-Type-Options: nosniff
    + +
    +
    + + diff --git a/docs/v2/service_instances/update_a_service_instance.html b/docs/v2/service_instances/update_a_service_instance.html index 9392093c80c..c1703cd1835 100644 --- a/docs/v2/service_instances/update_a_service_instance.html +++ b/docs/v2/service_instances/update_a_service_instance.html @@ -515,6 +515,22 @@

    Body

    + + + shared_to_url + + + Information about where the service instance instance has been shared to. + + +
      +
    + + +
      +
    + + tags @@ -566,7 +582,8 @@

    Body

    "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", - "shared_from_url": "/v2/service_instances/0d632575-bb06-4ea5-bb19-a451a9644d92/shared_from" + "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 30c860e00bf..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 @@ -304,7 +304,8 @@

    Body

    "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", - "shared_from_url": "/v2/service_instances/9547e9ed-e460-4abe-bda3-7070b9835917/shared_from" + "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 5c4fda1066f..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 @@ -303,7 +303,8 @@

    Body

    "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", - "shared_from_url": "/v2/service_instances/9547e9ed-e460-4abe-bda3-7070b9835917/shared_from" + "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/spec/request/v2/service_bindings_spec.rb b/spec/request/v2/service_bindings_spec.rb index 0f45371e09a..ce50ba51bb5 100644 --- a/spec/request/v2/service_bindings_spec.rb +++ b/spec/request/v2/service_bindings_spec.rb @@ -198,7 +198,8 @@ '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", - 'shared_from_url' => "/v2/service_instances/#{service_instance.guid}/shared_from" + '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 97cffdf5878..733d1e93582 100644 --- a/spec/request/v2/service_instances_spec.rb +++ b/spec/request/v2/service_instances_spec.rb @@ -55,7 +55,8 @@ '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", - 'shared_from_url' => "/v2/service_instances/#{service_instance.guid}/shared_from" + 'shared_from_url' => "/v2/service_instances/#{service_instance.guid}/shared_from", + 'shared_to_url' => "/v2/service_instances/#{service_instance.guid}/shared_to", } } ) @@ -98,7 +99,8 @@ '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", - 'shared_from_url' => "/v2/service_instances/#{service_instance.guid}/shared_from" + 'shared_from_url' => "/v2/service_instances/#{service_instance.guid}/shared_from", + 'shared_to_url' => "/v2/service_instances/#{service_instance.guid}/shared_to", } } ) @@ -140,7 +142,8 @@ '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", - 'shared_from_url' => "/v2/service_instances/#{service_instance.guid}/shared_from" + 'shared_from_url' => "/v2/service_instances/#{service_instance.guid}/shared_from", + 'shared_to_url' => "/v2/service_instances/#{service_instance.guid}/shared_to", } } ) @@ -202,4 +205,41 @@ 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 and org' 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 + }, + { + 'space_name' => space2.name, + 'organization_name' => space2.organization.name + } + ] + } + ) + end + end end diff --git a/spec/request/v2/spaces_spec.rb b/spec/request/v2/spaces_spec.rb index 204896abd6e..f163cf306fa 100644 --- a/spec/request/v2/spaces_spec.rb +++ b/spec/request/v2/spaces_spec.rb @@ -219,7 +219,8 @@ '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_from_url' => "/v2/service_instances/#{shared_service_instance.guid}/shared_from", + 'shared_to_url' => "/v2/service_instances/#{shared_service_instance.guid}/shared_to", } }] }) diff --git a/spec/unit/controllers/services/service_instances_controller_spec.rb b/spec/unit/controllers/services/service_instances_controller_spec.rb index c69b134972b..2ae20afde85 100644 --- a/spec/unit/controllers/services/service_instances_controller_spec.rb +++ b/spec/unit/controllers/services/service_instances_controller_spec.rb @@ -3603,6 +3603,119 @@ def verify_forbidden(user) 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']) + expect(space2_resource.keys).to match_array(['space_name', 'organization_name']) + + 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) + 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) } diff --git a/spec/unit/presenters/v2/service_instance_presenter_spec.rb b/spec/unit/presenters/v2/service_instance_presenter_spec.rb index 638abbd6065..811a2d87a33 100644 --- a/spec/unit/presenters/v2/service_instance_presenter_spec.rb +++ b/spec/unit/presenters/v2/service_instance_presenter_spec.rb @@ -35,6 +35,7 @@ module CloudController::Presenters::V2 '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 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..a765cc7b26f --- /dev/null +++ b/spec/unit/presenters/v2/service_instance_shared_to_presenter_spec.rb @@ -0,0 +1,18 @@ +require 'spec_helper' + +module CloudController::Presenters::V2 + RSpec.describe ServiceInstanceSharedToPresenter do + describe '#to_hash' do + it 'returns the space and org name' do + space = VCAP::CloudController::Space.make + presenter = ServiceInstanceSharedToPresenter.new + expect(presenter.to_hash(space)).to eq( + { + 'space_name' => space.name, + 'organization_name' => space.organization.name, + } + ) + end + end + end +end From fe81c83404fa13954e4f13daf6a0c0884fa555f0 Mon Sep 17 00:00:00 2001 From: Sam Gunaratne Date: Wed, 8 Nov 2017 13:56:03 +0000 Subject: [PATCH 14/14] Add bound_app_count to /v2/service_instances/:guid/shared_to response * The /v2/service_instance/:guid/shared_to response body now includes the 'bound_app_count' key, which shows the number of apps bound to the service instance in each of the spaces that the instance has been shared to. [#151432730] Signed-off-by: Jen Spinney --- .../services/service_instances_controller.rb | 14 ++++-- app/fetchers/service_binding_list_fetcher.rb | 7 +++ .../service_instance_shared_to_presenter.rb | 4 +- ...o_where_service_instance_is_shared_to.html | 22 ++++++++- spec/request/v2/service_instances_spec.rb | 8 ++-- .../service_instances_controller_spec.rb | 31 +++++++++++- ...rvice_instance_shared_to_presenter_spec.rb | 5 +- .../service_binding_list_fetcher_spec.rb | 48 +++++++++++++++++++ 8 files changed, 124 insertions(+), 15 deletions(-) diff --git a/app/controllers/services/service_instances_controller.rb b/app/controllers/services/service_instances_controller.rb index 97e2c9294dd..40786a6e6fe 100644 --- a/app/controllers/services/service_instances_controller.rb +++ b/app/controllers/services/service_instances_controller.rb @@ -5,6 +5,7 @@ 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' @@ -232,7 +233,7 @@ def enumerate_shared_to_information(guid) associated_controller = VCAP::CloudController::SpacesController associated_path = "#{self.class.url_for_guid(guid)}/shared_to" - create_paginated_collection_renderer.render_json( + create_paginated_collection_renderer(service_instance).render_json( associated_controller, service_instance.shared_spaces_dataset, associated_path, @@ -346,15 +347,20 @@ 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) - CloudController::Presenters::V2::ServiceInstanceSharedToPresenter.new.to_hash(space) + 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 + def create_paginated_collection_renderer(service_instance) VCAP::CloudController::RestController::PaginatedCollectionRenderer.new( VCAP::CloudController::RestController::SecureEagerLoader.new, - ServiceInstanceSharedToSerializer.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), 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/presenters/v2/service_instance_shared_to_presenter.rb b/app/presenters/v2/service_instance_shared_to_presenter.rb index 70f927420c7..0f3229579bf 100644 --- a/app/presenters/v2/service_instance_shared_to_presenter.rb +++ b/app/presenters/v2/service_instance_shared_to_presenter.rb @@ -4,8 +4,8 @@ module CloudController module Presenters module V2 class ServiceInstanceSharedToPresenter < ServiceInstanceSharedFromPresenter - def to_hash(space) - super(space) + def to_hash(space, bound_app_count) + super(space).merge({ 'bound_app_count' => bound_app_count }) end end end diff --git a/docs/v2/service_instances/retrieve_info_where_service_instance_is_shared_to.html b/docs/v2/service_instances/retrieve_info_where_service_instance_is_shared_to.html index d3686b1ae0b..4dfd7a1d103 100644 --- a/docs/v2/service_instances/retrieve_info_where_service_instance_is_shared_to.html +++ b/docs/v2/service_instances/retrieve_info_where_service_instance_is_shared_to.html @@ -194,6 +194,22 @@

    Body

    + + + bound_app_count + + + The number of applications bound to the service instance in the space. + + +
      +
    + + +
      +
    + + @@ -205,11 +221,13 @@

    Body

    "resources": [ { "space_name": "target-space-1", - "organization_name": "org-name" + "organization_name": "org-name", + "bound_app_count": 3 }, { "space_name": "target-space-2", - "organization_name": "org-name" + "organization_name": "org-name", + "bound_app_count": 0 } ] } diff --git a/spec/request/v2/service_instances_spec.rb b/spec/request/v2/service_instances_spec.rb index 733d1e93582..2446c5b6414 100644 --- a/spec/request/v2/service_instances_spec.rb +++ b/spec/request/v2/service_instances_spec.rb @@ -216,7 +216,7 @@ service_instance.add_shared_space(space2) end - it 'returns data about the source space and org' do + 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) @@ -231,11 +231,13 @@ 'resources' => [ { 'space_name' => space1.name, - 'organization_name' => space1.organization.name + 'organization_name' => space1.organization.name, + 'bound_app_count' => 0 }, { 'space_name' => space2.name, - 'organization_name' => space2.organization.name + 'organization_name' => space2.organization.name, + 'bound_app_count' => 0 } ] } diff --git a/spec/unit/controllers/services/service_instances_controller_spec.rb b/spec/unit/controllers/services/service_instances_controller_spec.rb index 2ae20afde85..93f3a78af3e 100644 --- a/spec/unit/controllers/services/service_instances_controller_spec.rb +++ b/spec/unit/controllers/services/service_instances_controller_spec.rb @@ -3636,14 +3636,41 @@ def verify_forbidden(user) 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']) - expect(space2_resource.keys).to match_array(['space_name', 'organization_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 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 index a765cc7b26f..8119cfd3c69 100644 --- a/spec/unit/presenters/v2/service_instance_shared_to_presenter_spec.rb +++ b/spec/unit/presenters/v2/service_instance_shared_to_presenter_spec.rb @@ -3,13 +3,14 @@ module CloudController::Presenters::V2 RSpec.describe ServiceInstanceSharedToPresenter do describe '#to_hash' do - it 'returns the space and org name' 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)).to eq( + expect(presenter.to_hash(space, 42)).to eq( { 'space_name' => space.name, 'organization_name' => space.organization.name, + 'bound_app_count' => 42 } ) 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