Skip to content
This repository was archived by the owner on Jun 2, 2021. It is now read-only.

Commit 90b5158

Browse files
Derik Evangelistabutzopower
andauthored
v3(services): delete user-provided SI (cloudfoundry#1708)
[#171726153](https://www.pivotaltracker.com/story/show/171726153) Co-authored-by: Brian Butz <butzopower@gmail.com> Signed-off-by: George Blue <gblue@pivotal.io>
1 parent 946a581 commit 90b5158

11 files changed

Lines changed: 267 additions & 1 deletion

File tree

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
module VCAP::CloudController
2+
class ServiceInstanceDeleteUserProvided
3+
class AssociationNotEmptyError < StandardError; end
4+
5+
def initialize(event_repo)
6+
@service_event_repository = event_repo
7+
end
8+
9+
def delete(service_instance)
10+
association_not_empty! if service_instance.has_bindings? || service_instance.has_keys?
11+
12+
service_instance.db.transaction do
13+
service_instance.lock!
14+
service_instance.destroy
15+
service_event_repository.record_user_provided_service_instance_event(:delete, service_instance, {})
16+
end
17+
end
18+
19+
private
20+
21+
def association_not_empty!
22+
raise AssociationNotEmptyError.new
23+
end
24+
25+
attr_reader :service_event_repository
26+
end
27+
end

app/controllers/v3/service_instances_controller.rb

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
require 'actions/service_instance_update_managed'
1616
require 'actions/service_instance_update_user_provided'
1717
require 'actions/service_instance_create_user_provided'
18+
require 'actions/service_instance_delete_user_provided'
1819
require 'actions/service_instance_create_managed'
1920
require 'fetchers/service_instance_list_fetcher'
2021
require 'decorators/field_service_instance_space_decorator'
@@ -102,6 +103,24 @@ def update
102103
end
103104
end
104105

106+
def destroy
107+
service_instance = ServiceInstance.first(guid: hashed_params[:guid])
108+
service_instance_not_found! unless service_instance && can_read_service_instance?(service_instance)
109+
110+
unauthorized! unless can_write_space?(service_instance.space)
111+
112+
case service_instance
113+
when UserProvidedServiceInstance
114+
service_event_repository = VCAP::CloudController::Repositories::ServiceEventRepository::WithUserActor.new(user_audit_info)
115+
ServiceInstanceDeleteUserProvided.new(service_event_repository).delete(service_instance)
116+
head :no_content
117+
else
118+
head :not_implemented
119+
end
120+
rescue ServiceInstanceDeleteUserProvided::AssociationNotEmptyError
121+
associations_not_empty!
122+
end
123+
105124
def share_service_instance
106125
FeatureFlag.raise_unless_enabled!(:service_instance_sharing)
107126

@@ -327,4 +346,11 @@ def broker_unavailable!
327346
def invalid_service_plan_relation!
328347
raise CloudController::Errors::ApiError.new_from_details('InvalidRelation', 'service plan relates to a different service offering')
329348
end
349+
350+
def associations_not_empty!
351+
associations = 'service_bindings, service_keys, and routes'
352+
raise CloudController::Errors::ApiError.
353+
new_from_details('AssociationNotEmpty', associations, :service_instances).
354+
with_response_code(422)
355+
end
330356
end

app/models/services/service_instance.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,14 @@ def shared?
209209
shared_spaces.any?
210210
end
211211

212+
def has_bindings?
213+
!service_bindings.empty?
214+
end
215+
216+
def has_keys?
217+
!service_keys.empty?
218+
end
219+
212220
def self.managed_organizations_spaces_dataset(managed_organizations_dataset)
213221
VCAP::CloudController::Space.dataset.filter({ organization_id: managed_organizations_dataset.select(:organization_id) })
214222
end

config/routes.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,7 @@
218218
post '/service_instances', to: 'service_instances_v3#create'
219219
post '/service_instances/:service_instance_guid/relationships/shared_spaces', to: 'service_instances_v3#share_service_instance'
220220
patch '/service_instances/:guid', to: 'service_instances_v3#update'
221+
delete '/service_instances/:guid', to: 'service_instances_v3#destroy'
221222
delete '/service_instances/:service_instance_guid/relationships/shared_spaces/:space_guid', to: 'service_instances_v3#unshare_service_instance'
222223

223224
# space_features
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
### Delete a service instance
2+
3+
```
4+
Example Request
5+
```
6+
7+
```shell
8+
curl "https://api.example.org/v3/service_instances/[guid]" \
9+
-X DELETE \
10+
-H "Authorization: bearer [token]"
11+
```
12+
13+
```
14+
Example Response
15+
```
16+
17+
```http
18+
HTTP/1.1 204 No Content
19+
```
20+
21+
#### Definition
22+
`DELETE /v3/service_instances/:guid`
23+
24+
#### Permitted Roles
25+
|
26+
--- | ---
27+
Admin |
28+
Space Developer |

docs/v3/source/index.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,7 @@ includes:
356356
- experimental_resources/service_instances/get
357357
- experimental_resources/service_instances/credentials
358358
- experimental_resources/service_instances/parameters
359+
- experimental_resources/service_instances/delete
359360
- experimental_resources/sidecars/header
360361
- experimental_resources/sidecars/object
361362
- experimental_resources/sidecars/create_from_app

lib/cloud_controller/errors/api_error.rb

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,13 @@ def name
3333
details.try(:name)
3434
end
3535

36+
def with_response_code(response_code)
37+
@response_code = response_code
38+
self
39+
end
40+
3641
def response_code
37-
details.try(:response_code)
42+
@response_code || details.try(:response_code)
3843
end
3944
end
4045
end

spec/request/service_instances_spec.rb

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2143,6 +2143,66 @@ def check_filtered_instances(*instances)
21432143
end
21442144
end
21452145

2146+
describe 'DELETE /v3/service_instances/:guid' do
2147+
let(:api_call) { lambda { |user_headers| delete "/v3/service_instances/#{instance.guid}", '{}', user_headers } }
2148+
2149+
context 'user provided service instance' do
2150+
let!(:instance) { VCAP::CloudController::UserProvidedServiceInstance.make(space: space) }
2151+
2152+
let(:db_check) {
2153+
lambda do
2154+
get "/v3/service_instances/#{instance.guid}", {}, admin_headers
2155+
expect(last_response.status).to eq(404)
2156+
2157+
expect(VCAP::CloudController::ServiceInstanceLabelModel.where(service_instance: instance).all).to be_empty
2158+
expect(VCAP::CloudController::ServiceInstanceAnnotationModel.where(service_instance: instance).all).to be_empty
2159+
end
2160+
}
2161+
2162+
let(:expected_codes_and_responses) do
2163+
Hash.new(code: 403).tap do |h|
2164+
h['space_developer'] = { code: 204 }
2165+
h['admin'] = { code: 204 }
2166+
h['no_role'] = { code: 404 }
2167+
h['org_auditor'] = { code: 404 }
2168+
h['org_billing_manager'] = { code: 404 }
2169+
end
2170+
end
2171+
2172+
it_behaves_like 'permissions for delete endpoint', ALL_PERMISSIONS
2173+
2174+
context 'when associations are not empty' do
2175+
it 'returns a 422 Unprocessable Entity when there are service bindings' do
2176+
VCAP::CloudController::ServiceBinding.make(service_instance: instance)
2177+
2178+
api_call.call(admin_headers)
2179+
expect(last_response).to have_status_code(422)
2180+
response = parsed_response['errors'].first
2181+
expect(response).to include('title' => 'CF-AssociationNotEmpty')
2182+
expect(response).to include('detail' => 'Please delete the service_bindings, service_keys, and routes associations for your service_instances.')
2183+
end
2184+
2185+
it 'returns a 422 Unprocessable Entity when there are service keys' do
2186+
VCAP::CloudController::ServiceKey.make(service_instance: instance)
2187+
2188+
api_call.call(admin_headers)
2189+
expect(last_response).to have_status_code(422)
2190+
response = parsed_response['errors'].first
2191+
expect(response).to include('title' => 'CF-AssociationNotEmpty')
2192+
expect(response).to include('detail' => 'Please delete the service_bindings, service_keys, and routes associations for your service_instances.')
2193+
end
2194+
end
2195+
end
2196+
2197+
context 'when the service instance does not exist' do
2198+
let(:instance) { Struct.new(:guid).new('some-fake-guid') }
2199+
it 'returns a 404' do
2200+
api_call.call(admin_headers)
2201+
expect(last_response).to have_status_code(404)
2202+
end
2203+
end
2204+
end
2205+
21462206
def create_managed_json(instance, labels: {}, annotations: {}, last_operation: {}, tags: [])
21472207
{
21482208
guid: instance.guid,
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
require 'spec_helper'
2+
require 'actions/service_instance_delete_user_provided'
3+
4+
module VCAP::CloudController
5+
RSpec.describe ServiceInstanceDeleteUserProvided do
6+
describe '#delete' do
7+
subject(:action) { described_class.new(event_repository) }
8+
let(:event_repository) do
9+
dbl = double(Repositories::ServiceEventRepository::WithUserActor)
10+
allow(dbl).to receive(:record_user_provided_service_instance_event)
11+
dbl
12+
end
13+
14+
let!(:service_instance) do
15+
si = VCAP::CloudController::UserProvidedServiceInstance.make(
16+
name: 'foo',
17+
credentials: {
18+
foo: 'bar',
19+
baz: 'qux'
20+
},
21+
syslog_drain_url: 'https://foo.com',
22+
route_service_url: 'https://bar.com',
23+
tags: %w(accounting mongodb)
24+
)
25+
si.label_ids = [
26+
VCAP::CloudController::ServiceInstanceLabelModel.make(key_prefix: 'pre.fix', key_name: 'to_delete', value: 'value'),
27+
VCAP::CloudController::ServiceInstanceLabelModel.make(key_prefix: 'pre.fix', key_name: 'tail', value: 'fluffy')
28+
]
29+
si.annotation_ids = [
30+
VCAP::CloudController::ServiceInstanceAnnotationModel.make(key_prefix: 'pre.fix', key_name: 'to_delete', value: 'value').id,
31+
VCAP::CloudController::ServiceInstanceAnnotationModel.make(key_prefix: 'pre.fix', key_name: 'fox', value: 'bushy').id
32+
]
33+
si
34+
end
35+
36+
it 'deletes the service instance from the database' do
37+
subject.delete(service_instance)
38+
39+
expect {
40+
service_instance.reload
41+
}.to raise_error(Sequel::Error, 'Record not found')
42+
expect(VCAP::CloudController::ServiceInstanceLabelModel.where(service_instance: service_instance)).to be_empty
43+
expect(VCAP::CloudController::ServiceInstanceAnnotationModel.where(service_instance: service_instance)).to be_empty
44+
end
45+
46+
it 'creates an audit event' do
47+
subject.delete(service_instance)
48+
49+
expect(event_repository).
50+
to have_received(:record_user_provided_service_instance_event).
51+
with(:delete, instance_of(UserProvidedServiceInstance), {})
52+
end
53+
54+
context 'when there are associated service bindings' do
55+
before do
56+
VCAP::CloudController::ServiceBinding.make(service_instance: service_instance)
57+
end
58+
59+
it 'does not delete the service instance' do
60+
expect {
61+
subject.delete(service_instance)
62+
}.to raise_error(
63+
ServiceInstanceDeleteUserProvided::AssociationNotEmptyError,
64+
)
65+
end
66+
end
67+
68+
context 'when there are associated service keys' do
69+
before do
70+
VCAP::CloudController::ServiceKey.make(service_instance: service_instance)
71+
end
72+
73+
it 'does not delete the service instance' do
74+
expect {
75+
subject.delete(service_instance)
76+
}.to raise_error(
77+
ServiceInstanceDeleteUserProvided::AssociationNotEmptyError,
78+
)
79+
end
80+
end
81+
end
82+
end
83+
end

spec/unit/lib/cloud_controller/errors/api_error_spec.rb

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,11 @@ def create_details(message)
8686
expect(api_error.response_code).to eq(400)
8787
end
8888

89+
it 'can be set to a different http code' do
90+
api_error.with_response_code(422)
91+
expect(api_error.response_code).to eq(422)
92+
end
93+
8994
it 'exposes the name' do
9095
expect(api_error.name).to eq('ServiceInvalid')
9196
end

0 commit comments

Comments
 (0)