diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 3c9ca55c547..2d2b6257808 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -169,8 +169,8 @@ RSpec/Be: # Offense count: 8 RSpec/BeforeAfterAll: Exclude: - - 'spec/integration/app_log_emitter_spec.rb' - - 'spec/integration/cors_spec.rb' + - 'spec/integration/app_log_emitter_shared_context.rb' + - 'spec/integration/zz_cc_suite_spec.rb' - 'spec/isolated_specs/inline_runner_spec.rb' - 'spec/unit/lib/sequel/extensions/default_order_by_id_spec.rb' - 'spec/unit/lib/vcap/rest_api/event_query_spec.rb' diff --git a/spec/integration/app_log_emitter_spec.rb b/spec/integration/app_log_emitter_shared_context.rb similarity index 65% rename from spec/integration/app_log_emitter_spec.rb rename to spec/integration/app_log_emitter_shared_context.rb index c396e819704..5ba99a0e4db 100644 --- a/spec/integration/app_log_emitter_spec.rb +++ b/spec/integration/app_log_emitter_shared_context.rb @@ -1,7 +1,6 @@ require 'spec_helper' -require 'tempfile' -RSpec.describe 'Cloud controller Loggregator Integration', type: :integration do +RSpec.shared_context 'Cloud controller Loggregator Integration' do before(:all) do @authed_headers = { 'Authorization' => "bearer #{admin_token}", @@ -9,20 +8,7 @@ 'Content-Type' => 'application/json' } - base_cc_config_file = 'config/cloud_controller.yml' - port_8181_overrides = 'spec/fixtures/config/port_8181_config.yml' - config = VCAP::CloudController::YAMLConfig.safe_load_file(base_cc_config_file).deep_merge( - VCAP::CloudController::YAMLConfig.safe_load_file(port_8181_overrides) - ) - config['loggregator'] = { 'endpoint' => 'localhost:12345' } - - @cc_config_file = Tempfile.new('cc_config.yml') - @cc_config_file.write(YAML.dump(config)) - @cc_config_file.close - - start_cc(debug: false, config: @cc_config_file.path) - - @loggregator_server = FakeLoggregatorServer.new(12_345) + @loggregator_server = FakeLoggregatorServer.new(3456) @loggregator_server.start org = org_with_default_quota(@authed_headers) @@ -38,8 +24,6 @@ end after(:all) do - stop_cc - @cc_config_file.unlink @loggregator_server.stop end diff --git a/spec/integration/cors_shared_context.rb b/spec/integration/cors_shared_context.rb new file mode 100644 index 00000000000..d74e582b534 --- /dev/null +++ b/spec/integration/cors_shared_context.rb @@ -0,0 +1,124 @@ +require 'spec_helper' + +RSpec.shared_context 'CORS' do + let(:authed_headers) do + { + 'Authorization' => "bearer #{admin_token}", + 'Accept' => 'application/json', + 'Content-Type' => 'application/json' + } + end + + def make_preflight_request_with_origin(test_path, origin, method=nil, extra_headers={}) + headers = { 'Origin' => origin } + headers['Access-Control-Request-Method'] = method unless method.nil? + headers.merge!(extra_headers) + make_options_request(test_path, headers) + end + + [ + { app: 'v3 rails app', path: '/v3/processes', options_404: true }, + { app: 'v2 sinatra app', path: '/v2/info', options_404: true } + ].each do |suite| + describe suite[:app] do + let(:test_path) { suite[:path] } + + context 'when the Origin header is not present' do + it 'does not return any Access-Control headers and delegates to the initial request' do + response = make_get_request(test_path, authed_headers) + expect(response.code).to eq('200') + expect(response['Access-Control']).to be_nil + expect(response.json_body).to be_a(Hash) + end + end + + context 'when the Origin header is present' do + describe 'a preflight request' do + context 'and the origin is not in the whitelist' do + it 'does not return any Access-Control headers and returns 404' do + response = make_preflight_request_with_origin(test_path, 'http://corblimey.com', 'GET', authed_headers) + expect(response['Access-Control']).to be_nil + expect(response.code).to eq('404') + end + end + + context 'and the origin is a subset of a domain in the whitelist, but does not match' do + it 'does not return any Access-Control headers and returns 404' do + response = make_preflight_request_with_origin(test_path, 'http://talkoncorners.com.extra', 'GET', authed_headers) + expect(response['Access-Control']).to be_nil + expect(response.code).to eq('404') + end + end + + context 'and the origin matches a domain in the whitelist' do + context 'but no Access-Control-Request-Method header is present' do + it 'does not return any Access-Control headers' do + response = make_preflight_request_with_origin(test_path, 'http://wildcarded.inblue.net', nil, authed_headers) + expect(response['Access-Control']).to be_nil + end + end + + context 'and the Access-Control-Request-Method header is present' do + it 'returns correct preflight response headers' do + response = make_preflight_request_with_origin(test_path, 'http://bar.baz.inblue.net', 'PUT', authed_headers) + expect(response.code).to eq('200') + expect(response.body).to eq('') + expect(response['Content-Type']).to eq('text/plain') + expect(response['Vary']).to eq('Origin') + expect(response['Access-Control-Allow-Origin']).to eq('http://bar.baz.inblue.net') + expect(response['Access-Control-Allow-Credentials']).to eq('true') + expect(response['Access-Control-Allow-Methods'].split(',')).to contain_exactly('PUT', 'POST', 'DELETE', 'GET') + expect(response['Access-Control-Max-Age'].to_i).to be > 600 + expect(response['Access-Control-Expose-Headers'].split(',')). + to contain_exactly('x-cf-warnings', 'x-app-staging-log', 'range', 'location', VCAP::Request::HEADER_NAME.downcase) + expect(response['Access-Control-Allow-Headers'].split(',')).to contain_exactly('origin', 'content-type', 'authorization') + end + + context 'when the request asks to allow additional request headers' do + it 'adds them to the Allow-Headers list' do + extra = { 'Access-Control-Request-Headers' => 'foo, bar, baz, Authorization' } + response = make_preflight_request_with_origin(test_path, 'http://bar.baz.inblue.net', 'PUT', authed_headers.merge(extra)) + expect(response['Access-Control-Allow-Headers'].split(',')).to contain_exactly( + 'origin', 'content-type', 'authorization', 'foo', 'bar', 'baz' + ) + end + end + end + end + end + + describe 'a simple request or actual request' do + context 'and the origin is not in the whitelist' do + it 'does not return any Access-Control headers and delegates to the initial request' do + response = make_get_request(test_path, authed_headers.merge('Origin' => 'http://corblimey.com')) + expect(response.code).to eq('200') + expect(response['Access-Control']).to be_nil + expect(response.json_body).to be_a(Hash) + end + end + + context 'and the origin is a subset of a domain in the whitelist, but does not match' do + it 'does not return any Access-Control headers and delegates to the initial request' do + response = make_get_request(test_path, authed_headers.merge('Origin' => 'http://talkoncorners.com.extra')) + expect(response['Access-Control']).to be_nil + expect(response.code).to eq('200') + expect(response.json_body).to be_a(Hash) + end + end + + context 'and the origin matches an entry in the whitelist' do + it 'returns correct CORS response headers and delegates to the initial request' do + response = make_get_request(test_path, authed_headers.merge('Origin' => 'http://foo.inblue.net')) + expect(response.code).to eq('200') + expect(response.json_body).to be_a(Hash) + expect(response['Access-Control-Allow-Origin']).to eq('http://foo.inblue.net') + expect(response['Access-Control-Allow-Credentials']).to eq('true') + expect(response['Access-Control-Expose-Headers'].split(',')). + to contain_exactly('x-cf-warnings', 'x-app-staging-log', 'range', 'location', VCAP::Request::HEADER_NAME.downcase) + end + end + end + end + end + end +end diff --git a/spec/integration/cors_spec.rb b/spec/integration/cors_spec.rb deleted file mode 100644 index 6d70dcd5b40..00000000000 --- a/spec/integration/cors_spec.rb +++ /dev/null @@ -1,401 +0,0 @@ -require 'spec_helper' - -RSpec.describe 'CORS', type: :integration do - before(:all) do - start_cc - end - - after(:all) do - stop_cc - end - - let(:authed_headers) do - { - 'Authorization' => "bearer #{admin_token}", - 'Accept' => 'application/json', - 'Content-Type' => 'application/json' - } - end - - describe 'v3 rails app' do - let(:test_path) { '/v3/processes' } - - context 'when the Origin header is not present' do - it 'does not return any Access-Control headers (the request is not a CORS request)' do - response = make_get_request(test_path, authed_headers) - expect(response.code).to eq('200') - expect(response['Access-Control']).to be_nil - end - - it 'delegates to the initial request' do - response = make_get_request(test_path, authed_headers) - expect(response.code).to eq('200') - expect(response.json_body).to be_a(Hash) - end - end - - context 'when the Origin header is present' do - describe 'a preflight request' do - def make_preflight_request_with_origin(origin, method=nil, extra_headers={}) - headers = {} - headers = headers.merge({ 'Origin' => origin }) - headers = headers.merge({ 'Access-Control-Request-Method' => method }) unless method.nil? - headers = headers.merge(extra_headers) - - make_options_request(test_path, headers) - end - - context 'and the origin is not in the whitelist' do - it 'does not return any Access-Control headers' do - response = make_preflight_request_with_origin 'http://corblimey.com', 'GET', authed_headers - expect(response['Access-Control']).to be_nil - end - - it 'delegates to the initial request ( there is no options method for /v3/processes so we get a 404 )' do - response = make_preflight_request_with_origin 'http://corblimey.com', 'GET', authed_headers - expect(response.code).to eq('404') - end - end - - context 'and the origin is a subset of a domain in the whitelist, but does not match' do - it 'does not return any Access-Control headers' do - response = make_preflight_request_with_origin 'http://talkoncorners.com.extra', 'GET', authed_headers - expect(response['Access-Control']).to be_nil - end - - it 'delegates to the initial request ( there is no options method for /v3/processes so we get a 404 )' do - response = make_preflight_request_with_origin 'http://talkoncorners.com.extra', 'GET', authed_headers - expect(response.code).to eq('404') - end - end - - context 'and the origin matches a domain in the whitelist' do - context 'but no Access-Control-Request-Method header is present' do - it 'does not return any Access-Control headers' do - response = make_preflight_request_with_origin 'http://wildcarded.inblue.net', nil, authed_headers - expect(response['Access-Control']).to be_nil - end - end - - context 'and the Access-Control-Request-Method header is present' do - it 'returns a 200 code and does not process the original request' do - response = make_preflight_request_with_origin 'http://bar.baz.inblue.net', 'PUT', authed_headers - expect(response.code).to eq('200') - expect(response.body).to eq('') - end - - it 'sets the Content-Type: text/plain header' do - response = make_preflight_request_with_origin 'http://bar.baz.inblue.net', 'PUT', authed_headers - expect(response.code).to eq('200') - expect(response.body).to eq('') - expect(response['Content-Type']).to eq('text/plain') - end - - it 'returns a Vary: Origin header to ensure response is not cached for different origins' do - response = make_preflight_request_with_origin 'http://bar.baz.inblue.net', 'PUT', authed_headers - expect(response['Vary']).to eq('Origin') - end - - it 'returns an Access-Control-Allow-Origin header containing the requested origin domain' do - response = make_preflight_request_with_origin 'http://bar.baz.inblue.net', 'PUT', authed_headers - expect(response['Access-Control-Allow-Origin']).to eq('http://bar.baz.inblue.net') - end - - it 'allows credentials to be supplied' do - response = make_preflight_request_with_origin 'http://bar.baz.inblue.net', 'PUT', authed_headers - expect(response['Access-Control-Allow-Credentials']).to eq('true') - end - - it 'returns the valid request methods in the Access-Control-Allow-Methods header' do - response = make_preflight_request_with_origin 'http://bar.baz.inblue.net', 'PUT', authed_headers - expect(response['Access-Control-Allow-Methods'].split(',')).to contain_exactly( - 'PUT', 'POST', 'DELETE', 'GET' - ) - end - - it 'returns a max-age header with a large value (since these headers rarely change' do - response = make_preflight_request_with_origin 'http://bar.baz.inblue.net', 'PUT', authed_headers - expect(response['Access-Control-Max-Age'].to_i).to be > 600 - end - - it 'allows custom headers to be returned' do - response = make_preflight_request_with_origin 'http://bar.baz.inblue.net', 'PUT', authed_headers - expect(response['Access-Control-Expose-Headers'].split(',')). - to contain_exactly('x-cf-warnings', 'x-app-staging-log', 'range', 'location', VCAP::Request::HEADER_NAME.downcase) - end - - it 'allows needed request headers to be included' do - response = make_preflight_request_with_origin 'http://bar.baz.inblue.net', 'PUT', authed_headers - expect(response['Access-Control-Allow-Headers'].split(',')).to contain_exactly( - 'origin', - 'content-type', - 'authorization' - ) - end - - context 'when the request asks to allow additional request headers' do - let(:extra_headers) { { 'Access-Control-Request-Headers' => 'foo, bar, baz, Authorization' } } - - it 'allows that by adding them to the Allow-Headers list' do - response = make_preflight_request_with_origin 'http://bar.baz.inblue.net', 'PUT', authed_headers.merge(extra_headers) - expect(response['Access-Control-Allow-Headers'].split(',')).to contain_exactly( - 'origin', - 'content-type', - 'authorization', - 'foo', 'bar', 'baz' - ) - end - end - end - end - end - - describe 'a simple request or actual request' do - context 'and the origin is not in the whitelist' do - it 'does not return any Access-Control headers' do - response = make_get_request(test_path, authed_headers.merge({ 'Origin' => 'http://corblimey.com' })) - expect(response.code).to eq('200') - expect(response['Access-Control']).to be_nil - end - - it 'delegates to the initial request' do - response = make_get_request(test_path, authed_headers.merge({ 'Origin' => 'http://corblimey.com' })) - expect(response.code).to eq('200') - expect(response.json_body).to be_a(Hash) - end - end - - context 'and the origin is a subset of a domain in the whitelist, but does not match' do - it 'does not return any Access-Control headers' do - response = make_get_request(test_path, authed_headers.merge({ 'Origin' => 'http://talkoncorners.com.extra' })) - expect(response['Access-Control']).to be_nil - end - - it 'delegates to the initial request' do - response = make_get_request(test_path, authed_headers.merge({ 'Origin' => 'http://talkoncorners.com.extra' })) - expect(response.code).to eq('200') - expect(response.json_body).to be_a(Hash) - end - end - - context 'and the origin matches an entry in the whitelist' do - it 'delegates to the initial request' do - response = make_get_request(test_path, authed_headers.merge({ 'Origin' => 'http://foo.inblue.net' })) - expect(response.code).to eq('200') - expect(response.json_body).to be_a(Hash) - end - - it 'returns an Access-Control-Allow-Origin header containing the requested origin domain' do - response = make_get_request(test_path, authed_headers.merge({ 'Origin' => 'http://foo.inblue.net' })) - expect(response.code).to eq('200') - expect(response['Access-Control-Allow-Origin']).to eq('http://foo.inblue.net') - end - - it 'allows credentials to be supplied' do - response = make_get_request(test_path, authed_headers.merge({ 'Origin' => 'http://foo.inblue.net' })) - expect(response.code).to eq('200') - expect(response['Access-Control-Allow-Credentials']).to eq('true') - end - - it 'allows custom headers to be returned' do - response = make_get_request(test_path, authed_headers.merge({ 'Origin' => 'http://foo.inblue.net' })) - expect(response.code).to eq('200') - expect(response['Access-Control-Expose-Headers'].split(',')). - to contain_exactly('x-cf-warnings', 'x-app-staging-log', 'range', 'location', VCAP::Request::HEADER_NAME.downcase) - end - end - end - end - end - - describe 'v2 sinatra app' do - let(:test_path) { '/v2/info' } - - context 'when the Origin header is not present' do - it 'does not return any Access-Control headers (the request is not a CORS request)' do - response = make_get_request(test_path, authed_headers) - expect(response.code).to eq('200') - expect(response['Access-Control']).to be_nil - end - - it 'delegates to the initial request' do - response = make_get_request(test_path, authed_headers) - expect(response.code).to eq('200') - expect(response.json_body).to be_a(Hash) - end - end - - context 'when the Origin header is present' do - describe 'a preflight request' do - def make_preflight_request_with_origin(origin, method=nil, extra_headers={}) - headers = {} - headers = headers.merge({ 'Origin' => origin }) - headers = headers.merge({ 'Access-Control-Request-Method' => method }) unless method.nil? - headers = headers.merge(extra_headers) - - make_options_request(test_path, headers) - end - - context 'and the origin is not in the whitelist' do - it 'does not return any Access-Control headers' do - response = make_preflight_request_with_origin 'http://corblimey.com', 'GET', authed_headers - expect(response['Access-Control']).to be_nil - end - - it 'delegates to the initial request ( there is no options method for /v2/info so we get a 404 )' do - response = make_preflight_request_with_origin 'http://corblimey.com', 'GET', authed_headers - expect(response.code).to eq('404') - end - end - - context 'and the origin is a subset of a domain in the whitelist, but does not match' do - it 'does not return any Access-Control headers' do - response = make_preflight_request_with_origin 'http://talkoncorners.com.extra', 'GET', authed_headers - expect(response['Access-Control']).to be_nil - end - - it 'delegates to the initial request ( there is no options method for /v2/info so we get a 404 )' do - response = make_preflight_request_with_origin 'http://talkoncorners.com.extra', 'GET', authed_headers - expect(response.code).to eq('404') - end - end - - context 'and the origin matches a domain in the whitelist' do - context 'but no Access-Control-Request-Method header is present' do - it 'does not return any Access-Control headers' do - response = make_preflight_request_with_origin 'http://wildcarded.inblue.net', nil, authed_headers - expect(response['Access-Control']).to be_nil - end - end - - context 'and the Access-Control-Request-Method header is present' do - it 'returns a 200 code and does not process the original request' do - response = make_preflight_request_with_origin 'http://bar.baz.inblue.net', 'PUT', authed_headers - expect(response.code).to eq('200') - expect(response.body).to eq('') - end - - it 'sets the Content-Type: text/plain header' do - response = make_preflight_request_with_origin 'http://bar.baz.inblue.net', 'PUT', authed_headers - expect(response.code).to eq('200') - expect(response.body).to eq('') - expect(response['Content-Type']).to eq('text/plain') - end - - it 'returns a Vary: Origin header to ensure response is not cached for different origins' do - response = make_preflight_request_with_origin 'http://bar.baz.inblue.net', 'PUT', authed_headers - expect(response['Vary']).to eq('Origin') - end - - it 'returns an Access-Control-Allow-Origin header containing the requested origin domain' do - response = make_preflight_request_with_origin 'http://bar.baz.inblue.net', 'PUT', authed_headers - expect(response['Access-Control-Allow-Origin']).to eq('http://bar.baz.inblue.net') - end - - it 'allows credentials to be supplied' do - response = make_preflight_request_with_origin 'http://bar.baz.inblue.net', 'PUT', authed_headers - expect(response['Access-Control-Allow-Credentials']).to eq('true') - end - - it 'returns the valid request methods in the Access-Control-Allow-Methods header' do - response = make_preflight_request_with_origin 'http://bar.baz.inblue.net', 'PUT', authed_headers - expect(response['Access-Control-Allow-Methods'].split(',')).to contain_exactly( - 'PUT', 'POST', 'DELETE', 'GET' - ) - end - - it 'returns a max-age header with a large value (since these headers rarely change' do - response = make_preflight_request_with_origin 'http://bar.baz.inblue.net', 'PUT', authed_headers - expect(response['Access-Control-Max-Age'].to_i).to be > 600 - end - - it 'allows custom headers to be returned' do - response = make_preflight_request_with_origin 'http://bar.baz.inblue.net', 'PUT', authed_headers - expect(response['Access-Control-Expose-Headers'].split(',')). - to contain_exactly('x-cf-warnings', 'x-app-staging-log', 'range', 'location', VCAP::Request::HEADER_NAME.downcase) - end - - it 'allows needed request headers to be included' do - response = make_preflight_request_with_origin 'http://bar.baz.inblue.net', 'PUT', authed_headers - expect(response['Access-Control-Allow-Headers'].split(',')).to contain_exactly( - 'origin', - 'content-type', - 'authorization' - ) - end - - context 'when the request asks to allow additional request headers' do - let(:extra_headers) { { 'Access-Control-Request-Headers' => 'foo, bar, baz, Authorization' } } - - it 'allows that by adding them to the Allow-Headers list' do - response = make_preflight_request_with_origin 'http://bar.baz.inblue.net', 'PUT', authed_headers.merge(extra_headers) - expect(response['Access-Control-Allow-Headers'].split(',')).to contain_exactly( - 'origin', - 'content-type', - 'authorization', - 'foo', 'bar', 'baz' - ) - end - end - end - end - end - - describe 'a simple request or actual request' do - context 'and the origin is not in the whitelist' do - it 'does not return any Access-Control headers' do - response = make_get_request(test_path, authed_headers.merge({ 'Origin' => 'http://corblimey.com' })) - expect(response.code).to eq('200') - expect(response['Access-Control']).to be_nil - end - - it 'delegates to the initial request' do - response = make_get_request(test_path, authed_headers.merge({ 'Origin' => 'http://corblimey.com' })) - expect(response.code).to eq('200') - expect(response.json_body).to be_a(Hash) - end - end - - context 'and the origin is a subset of a domain in the whitelist, but does not match' do - it 'does not return any Access-Control headers' do - response = make_get_request(test_path, authed_headers.merge({ 'Origin' => 'http://talkoncorners.com.extra' })) - expect(response['Access-Control']).to be_nil - end - - it 'delegates to the initial request' do - response = make_get_request(test_path, authed_headers.merge({ 'Origin' => 'http://talkoncorners.com.extra' })) - expect(response.code).to eq('200') - expect(response.json_body).to be_a(Hash) - end - end - - context 'and the origin matches an entry in the whitelist' do - it 'delegates to the initial request' do - response = make_get_request(test_path, authed_headers.merge({ 'Origin' => 'http://foo.inblue.net' })) - expect(response.code).to eq('200') - expect(response.json_body).to be_a(Hash) - end - - it 'returns an Access-Control-Allow-Origin header containing the requested origin domain' do - response = make_get_request(test_path, authed_headers.merge({ 'Origin' => 'http://foo.inblue.net' })) - expect(response.code).to eq('200') - expect(response['Access-Control-Allow-Origin']).to eq('http://foo.inblue.net') - end - - it 'allows credentials to be supplied' do - response = make_get_request(test_path, authed_headers.merge({ 'Origin' => 'http://foo.inblue.net' })) - expect(response.code).to eq('200') - expect(response['Access-Control-Allow-Credentials']).to eq('true') - end - - it 'allows custom headers to be returned' do - response = make_get_request(test_path, authed_headers.merge({ 'Origin' => 'http://foo.inblue.net' })) - expect(response.code).to eq('200') - expect(response['Access-Control-Expose-Headers'].split(',')). - to contain_exactly('x-cf-warnings', 'x-app-staging-log', 'range', 'location', VCAP::Request::HEADER_NAME.downcase) - end - end - end - end - end -end diff --git a/spec/integration/zz_cc_suite_spec.rb b/spec/integration/zz_cc_suite_spec.rb new file mode 100644 index 00000000000..c1f78b09ac0 --- /dev/null +++ b/spec/integration/zz_cc_suite_spec.rb @@ -0,0 +1,16 @@ +require 'spec_helper' +require_relative 'app_log_emitter_shared_context' +require_relative 'cors_shared_context' + +RSpec.describe 'Integration suite', type: :integration do + before(:all) do + start_cc + end + + after(:all) do + stop_cc + end + + include_context 'CORS' + include_context 'Cloud controller Loggregator Integration' +end diff --git a/spec/logcache/container_metric_batcher_spec.rb b/spec/logcache/container_metric_batcher_spec.rb index f80e9f36251..eef6a02dbce 100644 --- a/spec/logcache/container_metric_batcher_spec.rb +++ b/spec/logcache/container_metric_batcher_spec.rb @@ -484,6 +484,7 @@ def generate_batch(size, offset: 0, last_timestamp: TimeUtils.to_nanoseconds(Tim describe 'walking the log cache' do let(:lookback_window) { 2.minutes } + let(:process_guid) { SecureRandom.uuid } context 'when log cache never stops returning results' do let(:envelopes_max_limit_first_page) { generate_batch(1000) } diff --git a/spec/unit/actions/services/service_instance_delete_spec.rb b/spec/unit/actions/services/service_instance_delete_spec.rb index d39cbc392ce..c8038926343 100644 --- a/spec/unit/actions/services/service_instance_delete_spec.rb +++ b/spec/unit/actions/services/service_instance_delete_spec.rb @@ -10,117 +10,437 @@ module VCAP::CloudController subject(:service_instance_delete) { ServiceInstanceDelete.new(event_repository:) } describe '#delete' do - let!(:route_service_instance) { create(:managed_service_instance, :routing) } - let!(:managed_service_instance) { create(:managed_service_instance) } - let!(:user_provided_service_instance) { create(:user_provided_service_instance) } + context 'with pre-existing instances and bindings' do + let!(:route_service_instance) { create(:managed_service_instance, :routing) } + let!(:managed_service_instance) { create(:managed_service_instance) } + let!(:user_provided_service_instance) { create(:user_provided_service_instance) } - let!(:service_binding_1) { create(:service_binding, service_instance: managed_service_instance) } - let!(:service_binding_2) { create(:service_binding, service_instance: managed_service_instance) } - let!(:service_binding_3) { create(:service_binding, service_instance: user_provided_service_instance) } - let!(:service_binding_4) { create(:service_binding, service_instance: user_provided_service_instance) } + let!(:service_binding_1) { create(:service_binding, service_instance: managed_service_instance) } + let!(:service_binding_2) { create(:service_binding, service_instance: managed_service_instance) } + let!(:service_binding_3) { create(:service_binding, service_instance: user_provided_service_instance) } + let!(:service_binding_4) { create(:service_binding, service_instance: user_provided_service_instance) } - let!(:route_1) { create(:route, space: route_service_instance.space) } - let!(:route_2) { create(:route, space: route_service_instance.space) } - let!(:route_binding_1) { create(:route_binding, route: route_1, service_instance: route_service_instance) } - let!(:route_binding_2) { create(:route_binding, route: route_2, service_instance: route_service_instance) } + let!(:route_1) { create(:route, space: route_service_instance.space) } + let!(:route_2) { create(:route, space: route_service_instance.space) } + let!(:route_binding_1) { create(:route_binding, route: route_1, service_instance: route_service_instance) } + let!(:route_binding_2) { create(:route_binding, route: route_2, service_instance: route_service_instance) } - let!(:service_key) { create(:service_key, service_instance: managed_service_instance) } + let!(:service_key) { create(:service_key, service_instance: managed_service_instance) } - let(:service_instance_dataset) { ServiceInstance.dataset } + let(:service_instance_dataset) { ServiceInstance.dataset } - before do - [route_service_instance, managed_service_instance].each do |service_instance| - stub_deprovision(service_instance) + before do + [route_service_instance, managed_service_instance].each do |service_instance| + stub_deprovision(service_instance) + end + + stub_unbind(service_binding_1) + stub_unbind(service_binding_2) + stub_unbind(route_binding_1) + stub_unbind(route_binding_2) + stub_unbind(service_key) end - stub_unbind(service_binding_1) - stub_unbind(service_binding_2) - stub_unbind(route_binding_1) - stub_unbind(route_binding_2) - stub_unbind(service_key) - end + it 'deletes all the service_instances and logs events' do + expect(event_repository).to receive(:record_service_instance_event).with(:delete, instance_of(ManagedServiceInstance), {}).twice + expect(event_repository).to receive(:record_user_provided_service_instance_event).with(:delete, instance_of(UserProvidedServiceInstance), {}).once + expect do + service_instance_delete.delete(service_instance_dataset) + end.to change(ServiceInstance, :count).by(-3) + end - it 'deletes all the service_instances and logs events' do - expect(event_repository).to receive(:record_service_instance_event).with(:delete, instance_of(ManagedServiceInstance), {}).twice - expect(event_repository).to receive(:record_user_provided_service_instance_event).with(:delete, instance_of(UserProvidedServiceInstance), {}).once - expect do - service_instance_delete.delete(service_instance_dataset) - end.to change(ServiceInstance, :count).by(-3) - end + it 'deletes all the bindings for all the service instance' do + expect do + service_instance_delete.delete(service_instance_dataset) + end.to change(ServiceBinding, :count).by(-4) + end - it 'deletes all the bindings for all the service instance' do - expect do - service_instance_delete.delete(service_instance_dataset) - end.to change(ServiceBinding, :count).by(-4) - end + it 'deletes all the route bindings for all the service instance' do + expect do + service_instance_delete.delete(service_instance_dataset) + end.to change(RouteBinding, :count).by(-2) + end - it 'deletes all the route bindings for all the service instance' do - expect do - service_instance_delete.delete(service_instance_dataset) - end.to change(RouteBinding, :count).by(-2) - end + it 'deletes associated labels' do + labels = service_instance_dataset.map { |si| create(:service_instance_label_model, resource_guid: si.guid, key_name: 'test', value: 'bommel') } + + expect do + service_instance_delete.delete(service_instance_dataset) + end.to change(ServiceInstanceLabelModel, :count).by(-labels.length) + expect(labels).to be_none(&:exists?) + end - it 'deletes associated labels' do - labels = service_instance_dataset.map { |si| create(:service_instance_label_model, resource_guid: si.guid, key_name: 'test', value: 'bommel') } + it 'deletes associated annotations' do + annotations = service_instance_dataset.map { |si| create(:service_instance_annotation_model, resource_guid: si.guid, key_name: 'test', value: 'bommel') } - expect do - service_instance_delete.delete(service_instance_dataset) - end.to change(ServiceInstanceLabelModel, :count).by(-labels.length) - expect(labels).to be_none(&:exists?) - end + expect do + service_instance_delete.delete(service_instance_dataset) + end.to change(ServiceInstanceAnnotationModel, :count).by(-annotations.length) + expect(annotations).to be_none(&:exists?) + end - it 'deletes associated annotations' do - annotations = service_instance_dataset.map { |si| create(:service_instance_annotation_model, resource_guid: si.guid, key_name: 'test', value: 'bommel') } + it 'deletes user provided service instances' do + user_provided_instance = create(:user_provided_service_instance) + errors, warnings = service_instance_delete.delete(service_instance_dataset) + expect(errors).to be_empty + expect(warnings).to be_empty - expect do - service_instance_delete.delete(service_instance_dataset) - end.to change(ServiceInstanceAnnotationModel, :count).by(-annotations.length) - expect(annotations).to be_none(&:exists?) - end + expect(user_provided_instance).not_to exist + end - it 'deletes user provided service instances' do - user_provided_instance = create(:user_provided_service_instance) - errors, warnings = service_instance_delete.delete(service_instance_dataset) - expect(errors).to be_empty - expect(warnings).to be_empty + it 'deletes service keys associated with the service instance' do + expect do + service_instance_delete.delete(service_instance_dataset) + end.to change(ServiceKey, :count).by(-1) + end - expect(user_provided_instance).not_to exist - end + it 'unshares shared managed service instance and records only one unshare event' do + shared_to_space = create(:space) + managed_service_instance.add_shared_space(shared_to_space) - it 'deletes service keys associated with the service instance' do - expect do - service_instance_delete.delete(service_instance_dataset) - end.to change(ServiceKey, :count).by(-1) - end + expect(managed_service_instance).to receive(:remove_shared_space) + expect(route_service_instance).not_to receive(:remove_shared_space) + expect(Repositories::ServiceInstanceShareEventRepository).to receive(:record_unshare_event).once - it 'unshares shared managed service instance and records only one unshare event' do - shared_to_space = create(:space) - managed_service_instance.add_shared_space(shared_to_space) + service_instance_delete.delete([managed_service_instance, route_service_instance]) + end - expect(managed_service_instance).to receive(:remove_shared_space) - expect(route_service_instance).not_to receive(:remove_shared_space) - expect(Repositories::ServiceInstanceShareEventRepository).to receive(:record_unshare_event).once + it 'deletes the last operation for each managed service instance' do + instance_operation_1 = create(:service_instance_operation, state: 'succeeded') + route_service_instance.service_instance_operation = instance_operation_1 + route_service_instance.save - service_instance_delete.delete([managed_service_instance, route_service_instance]) - end + errors, warnings = service_instance_delete.delete(service_instance_dataset) + expect(errors).to be_empty + expect(warnings).to be_empty - it 'deletes the last operation for each managed service instance' do - instance_operation_1 = create(:service_instance_operation, state: 'succeeded') - route_service_instance.service_instance_operation = instance_operation_1 - route_service_instance.save + expect(route_service_instance).not_to exist + expect(instance_operation_1).not_to exist + end - errors, warnings = service_instance_delete.delete(service_instance_dataset) - expect(errors).to be_empty - expect(warnings).to be_empty + it 'defaults accepts_incomplete to false' do + service_instance_delete.delete([route_service_instance]) + broker_url = deprovision_url(route_service_instance) + expect(a_request(:delete, broker_url)).to have_been_made + end - expect(route_service_instance).not_to exist - expect(instance_operation_1).not_to exist - end + context 'when unbinding a service instance fails' do + before do + stub_unbind(service_binding_1, status: 500) + end + + it 'leaves the service instance unchanged' do + original_attrs = managed_service_instance.as_json + service_instance_delete.delete(service_instance_dataset) + + managed_service_instance.reload + + expect(a_request(:delete, unbind_url(service_binding_1))). + to have_been_made.times(1) + + expect(managed_service_instance.as_json).to eq(original_attrs) + expect(service_binding_1).to exist + end + end + + context 'when deprovisioning a service instance fails' do + before do + stub_deprovision(route_service_instance, status: 500) + end + + it 'marks the service instance as failed' do + service_instance_delete.delete(service_instance_dataset) + route_service_instance.reload + + expect(a_request(:delete, deprovision_url(route_service_instance))). + to have_been_made.times(1) + expect(route_service_instance.last_operation.type).to eq('delete') + expect(route_service_instance.last_operation.state).to eq('failed') + end + end + + context 'when a service instance has an update operation in progress' do + before do + route_service_instance.service_instance_operation = create(:service_instance_operation, + state: 'in progress', + type: 'update') + end + + it 'returns an operation in progress error for route and service bindings' do + errors, warnings = service_instance_delete.delete(service_instance_dataset) + expect(warnings).to be_empty + expect(errors.length).to eq 1 + expect(errors.first.name).to eq 'AsyncServiceInstanceOperationInProgress' + end + + it 'still exists and is in an `in progress` state' do + service_instance_delete.delete(service_instance_dataset) + expect(route_service_instance.last_operation.reload.state).to eq 'in progress' + end + end + + context 'when a service instance has a create operation in progress' do + let(:service_instance) { create(:managed_service_instance) } + + before do + service_instance.service_instance_operation = create(:service_instance_operation, + state: 'in progress', + type: 'create') + end + + context 'when service instance deprovision happen to be synchronous' do + before do + stub_deprovision(service_instance) + end + + it 'deletes the instance and tells broker to deprovision and returns no errors' do + expect(event_repository).to receive(:record_service_instance_event). + with(:delete, instance_of(ManagedServiceInstance), {}).once + errors, warnings = service_instance_delete.delete([service_instance]) + expect(warnings).to be_empty + expect(errors.length).to eq 0 + expect(ServiceInstance.where(id: service_instance.id).count).to eq 0 + broker_url = deprovision_url(service_instance) + expect(a_request(:delete, broker_url)).to have_been_made + end + end + + context 'when service instance deprovision happen to be asynchronous' do + subject(:service_instance_delete) do + ServiceInstanceDelete.new( + accepts_incomplete: true, + event_repository: event_repository + ) + end + + before do + stub_deprovision(service_instance, accepts_incomplete: true, status: 202, body: {}.to_json) + end + + it 'passes the accepts_incomplete flag, updates the instance to be in progress, and enqueues a fetch job' do + service_instance_delete.delete([service_instance]) + broker_url = deprovision_url(service_instance, accepts_incomplete: true) + expect(a_request(:delete, broker_url)).to have_been_made + expect(service_instance.last_operation.state).to eq 'in progress' + expect(service_instance.last_operation.type).to eq 'delete' + + job = Delayed::Job.last + expect(job).to be_a_fully_wrapped_job_of Jobs::Services::ServiceInstanceStateFetch + + inner_job = job.payload_object.handler.handler + expect(inner_job.name).to eq 'service-instance-state-fetch' + expect(inner_job.service_instance_guid).to eq service_instance.guid + expect(inner_job.request_attrs).to eq({}) + expect(inner_job.poll_interval).to eq(60) + end + + it 'does not delete the instance' do + expect do + service_instance_delete.delete([service_instance]) + end.not_to(change(ServiceInstance, :count)) + end + + context 'when there is an error during service instance delete' do + before do + stub_deprovision(service_instance, accepts_incomplete: true, status: 422, body: { error: 'ConcurrencyError' }.to_json) + end + + it 'does not update the operation type and returns errors' do + expect(service_instance.last_operation.type).to eq 'create' + errors, warnings = service_instance_delete.delete([service_instance]) + expect(service_instance.last_operation.type).to eq 'create' + expect(warnings).to be_empty + expect(errors.count).to eq(1) + expect(errors.first.name).to eq 'AsyncServiceInstanceOperationInProgress' + end + end + end + end + + context 'when the broker returns an error for one of the deletions' do + let(:error_status_code) { 500 } + + before do + stub_deprovision(managed_service_instance, status: error_status_code) + end + + it 'does not rollback previous deletions of service instances' do + expect do + service_instance_delete.delete(service_instance_dataset) + end.to change(ServiceInstance, :count).by(-2) + end + + it 'returns errors it has captured' do + errors, warnings = service_instance_delete.delete(service_instance_dataset) + expect(warnings).to be_empty + expect(errors.count).to eq(1) + expect(errors[0]).to be_instance_of(VCAP::Services::ServiceBrokers::V2::Errors::ServiceBrokerBadResponse) + end + + it 'fails the last operation of the service instance' do + service_instance_delete.delete(service_instance_dataset) + expect(managed_service_instance.last_operation.state).to eq('failed') + end + + it 'only records one delete audit event' do + expect(event_repository).to receive(:record_service_instance_event).with(:delete, route_service_instance, {}).once + service_instance_delete.delete(service_instance_dataset) + end + end + + context 'when the broker returns an error for route unbinding' do + before do + stub_unbind(route_binding_2, status: 500) + end + + it 'does not rollback previous deletions of service instances' do + expect do + service_instance_delete.delete(service_instance_dataset) + end.to change(ServiceInstance, :count).by(-2) + end + + it 'propagates service unbind error' do + errors, warnings = service_instance_delete.delete(service_instance_dataset) + expect(warnings).to be_empty + expect(errors).to have(1).item + error = errors.first + expect(error).to be_instance_of(CloudController::Errors::ApiError) + expect(error.message).to match "^Deletion of service instance #{route_service_instance.name} failed because one or more associated resources could not be deleted.\n\n" + expect(error.message).to match 'The service broker returned an invalid response' + end + + it 'does not attempt to delete that service instance' do + service_instance_delete.delete(service_instance_dataset) + expect(route_service_instance).to exist + expect(managed_service_instance).not_to exist + + broker_url_1 = deprovision_url(route_service_instance, accepts_incomplete: nil) + broker_url_2 = deprovision_url(managed_service_instance, accepts_incomplete: nil) + expect(a_request(:delete, broker_url_1)).not_to have_been_made + expect(a_request(:delete, broker_url_2)).to have_been_made + end + end + + context 'when the broker returns an error for unbinding' do + before do + stub_unbind(managed_service_instance.service_bindings.first, status: 500) + end + + it 'does not rollback previous deletions of service instances' do + expect do + service_instance_delete.delete(service_instance_dataset) + end.to change(ServiceInstance, :count).by(-2) + end + + it 'propagates service unbind error' do + errors, warnings = service_instance_delete.delete(service_instance_dataset) + expect(warnings).to be_empty + expect(errors).to have(1).item + error = errors.first + expect(error).to be_instance_of(CloudController::Errors::ApiError) + expect(error.message).to match "^Deletion of service instance #{managed_service_instance.name} failed because one or more associated resources could not be deleted." + expect(error.message).to match 'The service broker returned an invalid response' + end + + it 'does not attempt to delete that service instance' do + service_instance_delete.delete(service_instance_dataset) + expect(route_service_instance).not_to exist + expect(managed_service_instance).to exist + + broker_url_1 = deprovision_url(route_service_instance, accepts_incomplete: nil) + broker_url_2 = deprovision_url(managed_service_instance, accepts_incomplete: nil) + expect(a_request(:delete, broker_url_1)).to have_been_made + expect(a_request(:delete, broker_url_2)).not_to have_been_made + end + + it 'does not attempt to unshare the service instance' do + shared_to_space = create(:space) + managed_service_instance.add_shared_space(shared_to_space) + + expect_any_instance_of(ServiceInstanceUnshare).not_to receive(:unshare) + + service_instance_delete.delete([managed_service_instance]) + end + end + + context 'when the broker returns warnings when unbinding' do + before do + service_binding_deleter = instance_double(ServiceBindingDelete) + allow(service_binding_deleter).to receive(:delete) do |service_bindings| + service_bindings.each(&:destroy) + [[], %w[warning-1 warning-2]] + end + + allow(ServiceBindingDelete).to receive(:new).and_return(service_binding_deleter) + end + + it 'returns the warnings for all service instances' do + errors, warnings = service_instance_delete.delete(service_instance_dataset.limit(2)) + expect(errors).to be_empty + expect(warnings).to match_array(%w[warning-1 warning-2 warning-1 warning-2]) + end + end + + context 'when unsharing fails for a shared service instance' do + before do + shared_to_space = create(:space) + managed_service_instance.add_shared_space(shared_to_space) + + allow(managed_service_instance).to receive(:remove_shared_space).and_raise('Unsharing failed') + end + + it 'does not rollback previous deletions of service instances' do + expect do + service_instance_delete.delete([managed_service_instance, route_service_instance]) + end.to change(ServiceInstance, :count).by(-1) + end + + it 'returns the unbinding error' do + errors, warnings = service_instance_delete.delete([route_service_instance, managed_service_instance]) + expect(warnings).to be_empty + expect(errors.count).to eq(1) + expect(errors[0].message).to match 'Unsharing failed' + end + + it 'does not record an unshare event' do + expect(Repositories::ServiceInstanceShareEventRepository).not_to receive(:record_unshare_event) + + service_instance_delete.delete([route_service_instance, managed_service_instance]) + end + end + + context 'when deletion from the database fails for a service instance' do + before do + allow(managed_service_instance).to receive(:destroy).and_raise('BOOM') + end + + it 'does not rollback previous deletions of service instances' do + expect do + service_instance_delete.delete([route_service_instance, managed_service_instance]) + end.to change(ServiceInstance, :count).by(-1) + end - it 'defaults accepts_incomplete to false' do - service_instance_delete.delete([route_service_instance]) - broker_url = deprovision_url(route_service_instance) - expect(a_request(:delete, broker_url)).to have_been_made + it 'returns errors it has captured' do + errors, warnings = service_instance_delete.delete([route_service_instance, managed_service_instance]) + expect(warnings).to be_empty + expect(errors.count).to eq(1) + expect(errors[0].message).to eq 'BOOM' + end + end + + context 'when deleting already deleted service instance' do + it 'does not throw errors as element is missing anyway' do + expect(ServiceInstance.count).to eq 3 + service_instance_delete.delete([route_service_instance]) + expect(ServiceInstance.count).to eq 2 + errors, warnings = service_instance_delete.delete([route_service_instance]) + expect(warnings).to be_empty + + expect(ServiceInstance.count).to eq 2 + + expect(errors.count).to eq(0) + end + end end context 'when accepts_incomplete is true' do @@ -295,349 +615,6 @@ module VCAP::CloudController service_instance_delete.delete([service_instance]) end end - - context 'when unbinding a service instance fails' do - before do - stub_unbind(service_binding_1, status: 500) - end - - it 'leaves the service instance unchanged' do - original_attrs = managed_service_instance.as_json - service_instance_delete.delete(service_instance_dataset) - - managed_service_instance.reload - - expect(a_request(:delete, unbind_url(service_binding_1))). - to have_been_made.times(1) - - expect(managed_service_instance.as_json).to eq(original_attrs) - expect(service_binding_1).to exist - end - end - - context 'when deprovisioning a service instance fails' do - before do - stub_deprovision(route_service_instance, status: 500) - end - - it 'marks the service instance as failed' do - service_instance_delete.delete(service_instance_dataset) - route_service_instance.reload - - expect(a_request(:delete, deprovision_url(route_service_instance))). - to have_been_made.times(1) - expect(route_service_instance.last_operation.type).to eq('delete') - expect(route_service_instance.last_operation.state).to eq('failed') - end - end - - context 'when a service instance has an update operation in progress' do - before do - route_service_instance.service_instance_operation = create(:service_instance_operation, - state: 'in progress', - type: 'update') - end - - it 'returns an operation in progress error for route and service bindings' do - errors, warnings = service_instance_delete.delete(service_instance_dataset) - expect(warnings).to be_empty - expect(errors.length).to eq 1 - expect(errors.first.name).to eq 'AsyncServiceInstanceOperationInProgress' - end - - it 'still exists and is in an `in progress` state' do - service_instance_delete.delete(service_instance_dataset) - expect(route_service_instance.last_operation.reload.state).to eq 'in progress' - end - end - - context 'when a service instance has a create operation in progress' do - let(:service_instance) { create(:managed_service_instance) } - - before do - service_instance.service_instance_operation = create(:service_instance_operation, - state: 'in progress', - type: 'create') - end - - context 'when service instance deprovision happen to be synchronous' do - before do - stub_deprovision(service_instance) - end - - it 'deletes the instance' do - expect(event_repository).to receive(:record_service_instance_event). - with(:delete, instance_of(ManagedServiceInstance), {}).once - expect do - service_instance_delete.delete([service_instance]) - end.to change(ServiceInstance, :count).by(-1) - end - - it 'tells broker to deprovision the service' do - service_instance_delete.delete([service_instance]) - broker_url = deprovision_url(service_instance) - expect(a_request(:delete, broker_url)).to have_been_made - end - - it 'does not return any errors' do - errors, warnings = service_instance_delete.delete([service_instance]) - expect(warnings).to be_empty - expect(errors.length).to eq 0 - end - end - - context 'when service instance deprovision happen to be asynchronous' do - subject(:service_instance_delete) do - ServiceInstanceDelete.new( - accepts_incomplete: true, - event_repository: event_repository - ) - end - - before do - stub_deprovision(service_instance, accepts_incomplete: true, status: 202, body: {}.to_json) - end - - it 'passes the accepts_incomplete flag to the client deprovision call' do - service_instance_delete.delete([service_instance]) - broker_url = deprovision_url(service_instance, accepts_incomplete: true) - expect(a_request(:delete, broker_url)).to have_been_made - end - - it 'updates the instance to be in progress' do - service_instance_delete.delete([service_instance]) - expect(service_instance.last_operation.state).to eq 'in progress' - end - - it 'updates the instance operation type to be delete' do - service_instance_delete.delete([service_instance]) - expect(service_instance.last_operation.type).to eq 'delete' - end - - it 'enqueues a job to fetch state' do - service_instance_delete.delete([service_instance]) - - job = Delayed::Job.last - expect(job).to be_a_fully_wrapped_job_of Jobs::Services::ServiceInstanceStateFetch - - inner_job = job.payload_object.handler.handler - expect(inner_job.name).to eq 'service-instance-state-fetch' - expect(inner_job.service_instance_guid).to eq service_instance.guid - expect(inner_job.request_attrs).to eq({}) - expect(inner_job.poll_interval).to eq(60) - end - - it 'does not delete the instance' do - expect do - service_instance_delete.delete([service_instance]) - end.not_to(change(ServiceInstance, :count)) - end - - context 'when there is an error during service instance delete' do - before do - stub_deprovision(service_instance, accepts_incomplete: true, status: 422, body: { error: 'ConcurrencyError' }.to_json) - end - - it 'does not update the operation type' do - expect(service_instance.last_operation.type).to eq 'create' - service_instance_delete.delete([service_instance]) - expect(service_instance.last_operation.type).to eq 'create' - end - - it 'returns errors it has captured' do - errors, warnings = service_instance_delete.delete([service_instance]) - expect(warnings).to be_empty - expect(errors.count).to eq(1) - expect(errors.first.name).to eq 'AsyncServiceInstanceOperationInProgress' - end - end - end - end - - context 'when the broker returns an error for one of the deletions' do - let(:error_status_code) { 500 } - - before do - stub_deprovision(managed_service_instance, status: error_status_code) - end - - it 'does not rollback previous deletions of service instances' do - expect do - service_instance_delete.delete(service_instance_dataset) - end.to change(ServiceInstance, :count).by(-2) - end - - it 'returns errors it has captured' do - errors, warnings = service_instance_delete.delete(service_instance_dataset) - expect(warnings).to be_empty - expect(errors.count).to eq(1) - expect(errors[0]).to be_instance_of(VCAP::Services::ServiceBrokers::V2::Errors::ServiceBrokerBadResponse) - end - - it 'fails the last operation of the service instance' do - service_instance_delete.delete(service_instance_dataset) - expect(managed_service_instance.last_operation.state).to eq('failed') - end - - it 'only records one delete audit event' do - expect(event_repository).to receive(:record_service_instance_event).with(:delete, route_service_instance, {}).once - service_instance_delete.delete(service_instance_dataset) - end - end - - context 'when the broker returns an error for route unbinding' do - before do - stub_unbind(route_binding_2, status: 500) - end - - it 'does not rollback previous deletions of service instances' do - expect do - service_instance_delete.delete(service_instance_dataset) - end.to change(ServiceInstance, :count).by(-2) - end - - it 'propagates service unbind error' do - errors, warnings = service_instance_delete.delete(service_instance_dataset) - expect(warnings).to be_empty - expect(errors).to have(1).item - error = errors.first - expect(error).to be_instance_of(CloudController::Errors::ApiError) - expect(error.message).to match "^Deletion of service instance #{route_service_instance.name} failed because one or more associated resources could not be deleted.\n\n" - expect(error.message).to match 'The service broker returned an invalid response' - end - - it 'does not attempt to delete that service instance' do - service_instance_delete.delete(service_instance_dataset) - expect(route_service_instance).to exist - expect(managed_service_instance).not_to exist - - broker_url_1 = deprovision_url(route_service_instance, accepts_incomplete: nil) - broker_url_2 = deprovision_url(managed_service_instance, accepts_incomplete: nil) - expect(a_request(:delete, broker_url_1)).not_to have_been_made - expect(a_request(:delete, broker_url_2)).to have_been_made - end - end - - context 'when the broker returns an error for unbinding' do - before do - stub_unbind(managed_service_instance.service_bindings.first, status: 500) - end - - it 'does not rollback previous deletions of service instances' do - expect do - service_instance_delete.delete(service_instance_dataset) - end.to change(ServiceInstance, :count).by(-2) - end - - it 'propagates service unbind error' do - errors, warnings = service_instance_delete.delete(service_instance_dataset) - expect(warnings).to be_empty - expect(errors).to have(1).item - error = errors.first - expect(error).to be_instance_of(CloudController::Errors::ApiError) - expect(error.message).to match "^Deletion of service instance #{managed_service_instance.name} failed because one or more associated resources could not be deleted.\n\n" - expect(error.message).to match 'The service broker returned an invalid response' - end - - it 'does not attempt to delete that service instance' do - service_instance_delete.delete(service_instance_dataset) - expect(route_service_instance).not_to exist - expect(managed_service_instance).to exist - - broker_url_1 = deprovision_url(route_service_instance, accepts_incomplete: nil) - broker_url_2 = deprovision_url(managed_service_instance, accepts_incomplete: nil) - expect(a_request(:delete, broker_url_1)).to have_been_made - expect(a_request(:delete, broker_url_2)).not_to have_been_made - end - - it 'does not attempt to unshare the service instance' do - shared_to_space = create(:space) - managed_service_instance.add_shared_space(shared_to_space) - - expect_any_instance_of(ServiceInstanceUnshare).not_to receive(:unshare) - - service_instance_delete.delete([managed_service_instance]) - end - end - - context 'when the broker returns warnings when unbinding' do - before do - service_binding_deleter = instance_double(ServiceBindingDelete) - allow(service_binding_deleter).to receive(:delete) do |service_bindings| - service_bindings.each(&:destroy) - [[], %w[warning-1 warning-2]] - end - - allow(ServiceBindingDelete).to receive(:new).and_return(service_binding_deleter) - end - - it 'returns the warnings for all service instances' do - errors, warnings = service_instance_delete.delete(service_instance_dataset.limit(2)) - expect(errors).to be_empty - expect(warnings).to match_array(%w[warning-1 warning-2 warning-1 warning-2]) - end - end - - context 'when unsharing fails for a shared service instance' do - before do - shared_to_space = create(:space) - managed_service_instance.add_shared_space(shared_to_space) - - allow(managed_service_instance).to receive(:remove_shared_space).and_raise('Unsharing failed') - end - - it 'does not rollback previous deletions of service instances' do - expect do - service_instance_delete.delete([managed_service_instance, route_service_instance]) - end.to change(ServiceInstance, :count).by(-1) - end - - it 'returns the unbinding error' do - errors, warnings = service_instance_delete.delete([route_service_instance, managed_service_instance]) - expect(warnings).to be_empty - expect(errors.count).to eq(1) - expect(errors[0].message).to match 'Unsharing failed' - end - - it 'does not record an unshare event' do - expect(Repositories::ServiceInstanceShareEventRepository).not_to receive(:record_unshare_event) - - service_instance_delete.delete([route_service_instance, managed_service_instance]) - end - end - - context 'when deletion from the database fails for a service instance' do - before do - allow(managed_service_instance).to receive(:destroy).and_raise('BOOM') - end - - it 'does not rollback previous deletions of service instances' do - expect do - service_instance_delete.delete([route_service_instance, managed_service_instance]) - end.to change(ServiceInstance, :count).by(-1) - end - - it 'returns errors it has captured' do - errors, warnings = service_instance_delete.delete([route_service_instance, managed_service_instance]) - expect(warnings).to be_empty - expect(errors.count).to eq(1) - expect(errors[0].message).to eq 'BOOM' - end - end - - context 'when deleting already deleted service instance' do - it 'does not throw errors as element is missing anyway' do - expect(ServiceInstance.count).to eq 3 - service_instance_delete.delete([route_service_instance]) - expect(ServiceInstance.count).to eq 2 - errors, warnings = service_instance_delete.delete([route_service_instance]) - expect(warnings).to be_empty - - expect(ServiceInstance.count).to eq 2 - - expect(errors.count).to eq(0) - end - end end describe '#can_return_warnings?' do diff --git a/spec/unit/lib/delayed_job/threaded_worker_spec.rb b/spec/unit/lib/delayed_job/threaded_worker_spec.rb index f425a1b38a3..51f473d0749 100644 --- a/spec/unit/lib/delayed_job/threaded_worker_spec.rb +++ b/spec/unit/lib/delayed_job/threaded_worker_spec.rb @@ -3,7 +3,7 @@ require 'delayed_job/threaded_worker' RSpec.describe Delayed::ThreadedWorker do - let(:options) { { num_threads: 2, sleep_delay: 0.1, grace_period_seconds: 2 } } + let(:options) { { num_threads: 2, sleep_delay: 0.1, grace_period_seconds: 0.5 } } let(:worker) { Delayed::ThreadedWorker.new(options) } let(:worker_name) { 'instance_name' } diff --git a/spec/unit/lib/uaa/uaa_verification_keys_spec.rb b/spec/unit/lib/uaa/uaa_verification_keys_spec.rb index f41208ce99d..e2e6145d36b 100644 --- a/spec/unit/lib/uaa/uaa_verification_keys_spec.rb +++ b/spec/unit/lib/uaa/uaa_verification_keys_spec.rb @@ -7,7 +7,12 @@ module VCAP::CloudController let(:config_hash) { { url: 'http://uaa-url' } } let(:uaa_info) { double(CF::UAA::Info) } - let(:rsa_key) { OpenSSL::PKey::RSA.new(2048) } + + # Generated once at load time to avoid per-example RSA key generation overhead + rsa_key_const = OpenSSL::PKey::RSA.new(2048) + rsa_key2_const = OpenSSL::PKey::RSA.new(2048) + + let(:rsa_key) { rsa_key_const } let(:rsa_pem) { rsa_key.public_key.to_pem } let(:key_hash) { { 'key-name' => { 'value' => rsa_pem } } } let(:my_logger) { double(Steno::Logger) } @@ -52,7 +57,7 @@ module VCAP::CloudController end context 'when key was fetched more than 30 seconds ago' do - let(:rsa_key2) { OpenSSL::PKey::RSA.new(2048) } + let(:rsa_key2) { rsa_key2_const } let(:key_hash2) { { 'key-name' => { 'value' => rsa_key2.public_key.to_pem } } } before { allow(uaa_info).to receive(:validation_keys_hash).and_return(key_hash, key_hash2) } @@ -72,7 +77,7 @@ module VCAP::CloudController end context 'when key was fetched less than 30 seconds ago' do - let(:rsa_key2) { OpenSSL::PKey::RSA.new(2048) } + let(:rsa_key2) { rsa_key2_const } let(:key_hash2) { { 'key-name' => { 'value' => rsa_key2.public_key.to_pem } } } before { allow(uaa_info).to receive(:validation_keys_hash).and_return(key_hash, key_hash2) } diff --git a/spec/unit/messages/validators/label_selector_requirement_validator_spec.rb b/spec/unit/messages/validators/label_selector_requirement_validator_spec.rb index 396973ba2fd..57ccaf10f58 100644 --- a/spec/unit/messages/validators/label_selector_requirement_validator_spec.rb +++ b/spec/unit/messages/validators/label_selector_requirement_validator_spec.rb @@ -3,20 +3,19 @@ module VCAP::CloudController::Validators RSpec.describe 'LabelSelectorRequirementValidator' do - let(:label_selector_class) do - Class.new do - include ActiveModel::Model - include VCAP::CloudController::Validators + label_selector_class = Class.new do + include ActiveModel::Model + include VCAP::CloudController::Validators - attr_accessor :requirements + attr_accessor :requirements - validates_with LabelSelectorRequirementValidator + validates_with LabelSelectorRequirementValidator - def self.model_name - ActiveModel::Name.new(self, nil, 'label selector class') - end + def self.model_name + ActiveModel::Name.new(self, nil, 'label selector class') end end + let(:message) { label_selector_class.new({ requirements: }) } context 'when requirements are empty' do diff --git a/spec/unit/messages/validators/security_group_rule_validator_spec.rb b/spec/unit/messages/validators/security_group_rule_validator_spec.rb index 308f5699d40..8d72f6a6706 100644 --- a/spec/unit/messages/validators/security_group_rule_validator_spec.rb +++ b/spec/unit/messages/validators/security_group_rule_validator_spec.rb @@ -3,18 +3,16 @@ module VCAP::CloudController::Validators RSpec.describe 'SecurityGroupRuleValidator' do - let(:class_with_rules) do - Class.new do - include ActiveModel::Model + class_with_rules = Class.new do + include ActiveModel::Model - validates_with RulesValidator + validates_with RulesValidator - def self.name - 'TestClass' - end - - attr_accessor :rules + def self.name + 'TestClass' end + + attr_accessor :rules end let(:rules) { [] } diff --git a/spec/unit/messages/validators_spec.rb b/spec/unit/messages/validators_spec.rb index 03e31652f02..693f8b43a55 100644 --- a/spec/unit/messages/validators_spec.rb +++ b/spec/unit/messages/validators_spec.rb @@ -10,20 +10,42 @@ require 'pry' module VCAP::CloudController::Validators - RSpec.describe 'Validators' do - let(:fake_class) do - Class.new do - include ActiveModel::Model - include VCAP::CloudController::Validators + FAKE_BASE_CLASS = Class.new do + include ActiveModel::Model + include VCAP::CloudController::Validators - attr_accessor :field + attr_accessor :field - def self.model_name - ActiveModel::Name.new(self, nil, 'fake class') - end - end + def self.model_name + ActiveModel::Name.new(self, nil, 'fake class') end + end + FAKE_ARRAY_CLASS = Class.new(FAKE_BASE_CLASS) { validates :field, array: true } + FAKE_STRING_CLASS = Class.new(FAKE_BASE_CLASS) { validates :field, string: true } + FAKE_BOOLEAN_CLASS = Class.new(FAKE_BASE_CLASS) { validates :field, boolean: true } + FAKE_BOOLEAN_STRING_CLASS = Class.new(FAKE_BASE_CLASS) { validates :field, boolean_string: true } + FAKE_HASH_CLASS = Class.new(FAKE_BASE_CLASS) { validates :field, hash: true } + FAKE_GUID_CLASS = Class.new(FAKE_BASE_CLASS) { validates :field, guid: true } + FAKE_URI_CLASS = Class.new(FAKE_BASE_CLASS) { validates :field, uri: true } + FAKE_ENV_VARS_CLASS = Class.new(FAKE_BASE_CLASS) { validates :field, environment_variables: true } + FAKE_ENV_VARS_STRING_VALUES_CLASS = Class.new(FAKE_BASE_CLASS) { validates :field, environment_variables_string_values: true } + FAKE_FIELDS_CLASS = Class.new(FAKE_BASE_CLASS) { validates :field, fields: { allowed: { 'space.organization' => ['name'] } } } + FAKE_FIELDS_MULTI_KEYS_CLASS = Class.new(FAKE_BASE_CLASS) { validates :field, fields: { allowed: { 'some.resource' => %w[fake-value-1 fake-value-2] } } } + FAKE_FIELDS_MULTI_RESOURCES_CLASS = Class.new(FAKE_BASE_CLASS) do + validates :field, fields: { allowed: { 'a.resource' => ['fake-value'], 'another.resource' => ['another-fake-value'] } } + end + FAKE_HEALTH_CHECK_CLASS = Class.new(FAKE_BASE_CLASS) do + attr_accessor :health_check_type, :health_check_http_endpoint + + validates_with HealthCheckValidator + end + FAKE_TO_ONE_CLASS = Class.new(FAKE_BASE_CLASS) { validates :field, to_one_relationship: true } + FAKE_TO_MANY_CLASS = Class.new(FAKE_BASE_CLASS) { validates :field, to_many_relationship: true } + FAKE_VISIBILITY_CLASS = Class.new(FAKE_BASE_CLASS) { validates :field, org_visibility: true } + FAKE_TIMESTAMP_CLASS = Class.new(FAKE_BASE_CLASS) { validates :field, timestamp: true } + + RSpec.describe 'Validators' do describe 'validator extending StandaloneValidator' do describe '.validate_each' do it 'calls through to the instance method so it can be easily used outside of Active Models' do @@ -41,385 +63,305 @@ def validate_each(record, attr_name, value) end describe 'ArrayValidator' do - let(:array_class) do - Class.new(fake_class) do - validates :field, array: true - end - end - it 'adds an error if the field is not an array' do - fake_class = array_class.new field: 'not array' - expect(fake_class).not_to be_valid - expect(fake_class.errors[:field]).to include 'must be an array' + instance = FAKE_ARRAY_CLASS.new field: 'not array' + expect(instance).not_to be_valid + expect(instance.errors[:field]).to include 'must be an array' end it 'does not add an error if the field is an array' do - fake_class = array_class.new field: %w[an array] - expect(fake_class).to be_valid + instance = FAKE_ARRAY_CLASS.new field: %w[an array] + expect(instance).to be_valid end end describe 'StringValidator' do - let(:string_class) do - Class.new(fake_class) do - validates :field, string: true - end - end - it 'adds an error if the field is not a string' do - fake_class = string_class.new field: {} - expect(fake_class).not_to be_valid - expect(fake_class.errors[:field]).to include 'must be a string' + instance = FAKE_STRING_CLASS.new field: {} + expect(instance).not_to be_valid + expect(instance.errors[:field]).to include 'must be a string' end it 'does not add an error if the field is a string' do - fake_class = string_class.new field: 'hi i am string' - expect(fake_class).to be_valid + instance = FAKE_STRING_CLASS.new field: 'hi i am string' + expect(instance).to be_valid end end describe 'BooleanValidator' do - let(:boolean_class) do - Class.new(fake_class) do - validates :field, boolean: true - end - end - it 'adds an error if the field is not a boolean' do - instance = boolean_class.new field: {} + instance = FAKE_BOOLEAN_CLASS.new field: {} expect(instance).not_to be_valid expect(instance.errors[:field]).to include 'must be a boolean' end it 'does not add an error if the field is a boolean' do - instance = boolean_class.new field: true + instance = FAKE_BOOLEAN_CLASS.new field: true expect(instance).to be_valid - instance = boolean_class.new field: false + instance = FAKE_BOOLEAN_CLASS.new field: false expect(instance).to be_valid end end describe 'BooleanStringValidator' do - let(:boolean_class) do - Class.new(fake_class) do - validates :field, boolean_string: true - end - end - it 'adds an error if the field is not a boolean string' do - instance = boolean_class.new field: 'snarf' + instance = FAKE_BOOLEAN_STRING_CLASS.new field: 'snarf' expect(instance).not_to be_valid expect(instance.errors[:field]).to include "must be 'true' or 'false'" end it 'does not add an error if the field is a boolean string' do - instance = boolean_class.new field: 'true' + instance = FAKE_BOOLEAN_STRING_CLASS.new field: 'true' expect(instance).to be_valid - instance = boolean_class.new field: 'false' + instance = FAKE_BOOLEAN_STRING_CLASS.new field: 'false' expect(instance).to be_valid end end describe 'HashValidator' do - let(:hash_class) do - Class.new(fake_class) do - validates :field, hash: true - end - end - it 'adds an error if the field is not an object' do - fake_class = hash_class.new field: 'not an object' - expect(fake_class).not_to be_valid - expect(fake_class.errors[:field]).to include 'must be an object' + instance = FAKE_HASH_CLASS.new field: 'not an object' + expect(instance).not_to be_valid + expect(instance.errors[:field]).to include 'must be an object' end it 'does not add an error if the field is a hash' do - fake_class = hash_class.new field: { totes: 'hash' } - expect(fake_class).to be_valid + instance = FAKE_HASH_CLASS.new field: { totes: 'hash' } + expect(instance).to be_valid end end describe 'GuidValidator' do - let(:guid_class) do - Class.new(fake_class) do - validates :field, guid: true - end - end - it 'adds an error if the field is not a string' do - fake_class = guid_class.new field: 4 - expect(fake_class).not_to be_valid - expect(fake_class.errors[:field]).to include 'must be a string' + instance = FAKE_GUID_CLASS.new field: 4 + expect(instance).not_to be_valid + expect(instance.errors[:field]).to include 'must be a string' end it 'adds an error if the field is nil' do - fake_class = guid_class.new field: nil - expect(fake_class).not_to be_valid - expect(fake_class.errors[:field]).to include 'must be a string' + instance = FAKE_GUID_CLASS.new field: nil + expect(instance).not_to be_valid + expect(instance.errors[:field]).to include 'must be a string' end it 'adds an error if the field is too long' do - fake_class = guid_class.new field: 'a' * 201 - expect(fake_class).not_to be_valid - expect(fake_class.errors[:field]).to include 'must be between 1 and 200 characters' + instance = FAKE_GUID_CLASS.new field: 'a' * 201 + expect(instance).not_to be_valid + expect(instance.errors[:field]).to include 'must be between 1 and 200 characters' end it 'adds an error if the field is empty' do - fake_class = guid_class.new field: '' - expect(fake_class).not_to be_valid - expect(fake_class.errors[:field]).to include 'must be between 1 and 200 characters' + instance = FAKE_GUID_CLASS.new field: '' + expect(instance).not_to be_valid + expect(instance.errors[:field]).to include 'must be between 1 and 200 characters' end it 'does not add an error if the field is a guid' do - fake_class = guid_class.new field: 'such-a-guid-1234' - expect(fake_class).to be_valid + instance = FAKE_GUID_CLASS.new field: 'such-a-guid-1234' + expect(instance).to be_valid end end describe 'UriValidator' do - let(:uri_class) do - Class.new(fake_class) do - validates :field, uri: true - end - end - it 'adds an error if the field is not a URI' do - fake_class = uri_class.new field: 'not a URI' - expect(fake_class).not_to be_valid - expect(fake_class.errors[:field]).to include 'must be a valid URI' + instance = FAKE_URI_CLASS.new field: 'not a URI' + expect(instance).not_to be_valid + expect(instance.errors[:field]).to include 'must be a valid URI' end it 'does not add an error if the field is a URI' do - fake_class = uri_class.new field: 'http://www.purple.com' - expect(fake_class).to be_valid + instance = FAKE_URI_CLASS.new field: 'http://www.purple.com' + expect(instance).to be_valid end end describe 'EnvironmentVariablesValidator' do - let(:environment_variables_class) do - Class.new(fake_class) do - validates :field, environment_variables: true - end - end - it 'does not add an error if the environment variables are correct' do - fake_class = environment_variables_class.new field: { VARIABLE: 'amazing' } - expect(fake_class).to be_valid + instance = FAKE_ENV_VARS_CLASS.new field: { VARIABLE: 'amazing' } + expect(instance).to be_valid end it 'validates that the input is a hash' do - fake_class = environment_variables_class.new field: 4 - expect(fake_class).not_to be_valid - expect(fake_class.errors[:field]).to include 'must be an object' + instance = FAKE_ENV_VARS_CLASS.new field: 4 + expect(instance).not_to be_valid + expect(instance.errors[:field]).to include 'must be an object' end it 'does not allow variables that start with VCAP_' do - fake_class = environment_variables_class.new field: { VCAP_BANANA: 'woo' } - expect(fake_class).not_to be_valid - expect(fake_class.errors[:field]).to include 'cannot start with VCAP_' + instance = FAKE_ENV_VARS_CLASS.new field: { VCAP_BANANA: 'woo' } + expect(instance).not_to be_valid + expect(instance.errors[:field]).to include 'cannot start with VCAP_' end it 'does not allow variables that start with vcap_' do - fake_class = environment_variables_class.new field: { vcap_donkey: 'hee-haw' } - expect(fake_class).not_to be_valid - expect(fake_class.errors[:field]).to include 'cannot start with VCAP_' + instance = FAKE_ENV_VARS_CLASS.new field: { vcap_donkey: 'hee-haw' } + expect(instance).not_to be_valid + expect(instance.errors[:field]).to include 'cannot start with VCAP_' end it 'does not allow variables that start with VMC_' do - fake_class = environment_variables_class.new field: { VMC_BANANA: 'woo' } - expect(fake_class).not_to be_valid - expect(fake_class.errors[:field]).to include 'cannot start with VMC_' + instance = FAKE_ENV_VARS_CLASS.new field: { VMC_BANANA: 'woo' } + expect(instance).not_to be_valid + expect(instance.errors[:field]).to include 'cannot start with VMC_' end it 'does not allow variables that start with vmc_' do - fake_class = environment_variables_class.new field: { vmc_donkey: 'hee-haw' } - expect(fake_class).not_to be_valid - expect(fake_class.errors[:field]).to include 'cannot start with VMC_' + instance = FAKE_ENV_VARS_CLASS.new field: { vmc_donkey: 'hee-haw' } + expect(instance).not_to be_valid + expect(instance.errors[:field]).to include 'cannot start with VMC_' end it 'does not allow variables that are PORT' do - fake_class = environment_variables_class.new field: { PORT: 'el lunes nos ponemos camisetas naranjas' } - expect(fake_class).not_to be_valid - expect(fake_class.errors[:field]).to include 'cannot set PORT' + instance = FAKE_ENV_VARS_CLASS.new field: { PORT: 'el lunes nos ponemos camisetas naranjas' } + expect(instance).not_to be_valid + expect(instance.errors[:field]).to include 'cannot set PORT' end it 'does not allow variables that are port' do - fake_class = environment_variables_class.new field: { port: 'el lunes nos ponemos camisetas naranjas' } - expect(fake_class).not_to be_valid - expect(fake_class.errors[:field]).to include 'cannot set PORT' + instance = FAKE_ENV_VARS_CLASS.new field: { port: 'el lunes nos ponemos camisetas naranjas' } + expect(instance).not_to be_valid + expect(instance.errors[:field]).to include 'cannot set PORT' end it 'does not allow variables with zero key length' do - fake_class = environment_variables_class.new field: { '': 'el lunes nos ponemos camisetas naranjas' } - expect(fake_class).not_to be_valid - expect(fake_class.errors[:field]).to include 'key must be a minimum length of 1' + instance = FAKE_ENV_VARS_CLASS.new field: { '': 'el lunes nos ponemos camisetas naranjas' } + expect(instance).not_to be_valid + expect(instance.errors[:field]).to include 'key must be a minimum length of 1' end it 'does not allow variables with non-string keys' do - fake_class = environment_variables_class.new field: { 1 => 'el lunes nos ponemos camisetas naranjas' } - expect(fake_class).not_to be_valid - expect(fake_class.errors[:field]).to include 'key must be a string' + instance = FAKE_ENV_VARS_CLASS.new field: { 1 => 'el lunes nos ponemos camisetas naranjas' } + expect(instance).not_to be_valid + expect(instance.errors[:field]).to include 'key must be a string' end end describe 'EnvironmentVariablesStringValuesValidator' do - let(:environment_variables_class) do - Class.new(fake_class) do - validates :field, environment_variables_string_values: true - end - end - it 'does not add an error if the environment variables are correct' do - fake_class = environment_variables_class.new field: { VARIABLE: 'amazing' } - expect(fake_class).to be_valid + instance = FAKE_ENV_VARS_STRING_VALUES_CLASS.new field: { VARIABLE: 'amazing' } + expect(instance).to be_valid end it 'validates that the input is a hash' do - fake_class = environment_variables_class.new field: 4 - expect(fake_class).not_to be_valid - expect(fake_class.errors[:field]).to include 'must be an object' + instance = FAKE_ENV_VARS_STRING_VALUES_CLASS.new field: 4 + expect(instance).not_to be_valid + expect(instance.errors[:field]).to include 'must be an object' end it 'does not allow variables that start with VCAP_' do - fake_class = environment_variables_class.new field: { VCAP_BANANA: 'woo' } - expect(fake_class).not_to be_valid - expect(fake_class.errors[:field]).to include 'cannot start with VCAP_' + instance = FAKE_ENV_VARS_STRING_VALUES_CLASS.new field: { VCAP_BANANA: 'woo' } + expect(instance).not_to be_valid + expect(instance.errors[:field]).to include 'cannot start with VCAP_' end it 'does not allow variables that start with vcap_' do - fake_class = environment_variables_class.new field: { vcap_donkey: 'hee-haw' } - expect(fake_class).not_to be_valid - expect(fake_class.errors[:field]).to include 'cannot start with VCAP_' + instance = FAKE_ENV_VARS_STRING_VALUES_CLASS.new field: { vcap_donkey: 'hee-haw' } + expect(instance).not_to be_valid + expect(instance.errors[:field]).to include 'cannot start with VCAP_' end it 'does not allow variables that start with VMC_' do - fake_class = environment_variables_class.new field: { VMC_BANANA: 'woo' } - expect(fake_class).not_to be_valid - expect(fake_class.errors[:field]).to include 'cannot start with VMC_' + instance = FAKE_ENV_VARS_STRING_VALUES_CLASS.new field: { VMC_BANANA: 'woo' } + expect(instance).not_to be_valid + expect(instance.errors[:field]).to include 'cannot start with VMC_' end it 'does not allow variables that start with vmc_' do - fake_class = environment_variables_class.new field: { vmc_donkey: 'hee-haw' } - expect(fake_class).not_to be_valid - expect(fake_class.errors[:field]).to include 'cannot start with VMC_' + instance = FAKE_ENV_VARS_STRING_VALUES_CLASS.new field: { vmc_donkey: 'hee-haw' } + expect(instance).not_to be_valid + expect(instance.errors[:field]).to include 'cannot start with VMC_' end it 'does not allow variables that are PORT' do - fake_class = environment_variables_class.new field: { PORT: 'el lunes nos ponemos camisetas naranjas' } - expect(fake_class).not_to be_valid - expect(fake_class.errors[:field]).to include 'cannot set PORT' + instance = FAKE_ENV_VARS_STRING_VALUES_CLASS.new field: { PORT: 'el lunes nos ponemos camisetas naranjas' } + expect(instance).not_to be_valid + expect(instance.errors[:field]).to include 'cannot set PORT' end it 'does not allow variables that are port' do - fake_class = environment_variables_class.new field: { port: 'el lunes nos ponemos camisetas naranjas' } - expect(fake_class).not_to be_valid - expect(fake_class.errors[:field]).to include 'cannot set PORT' + instance = FAKE_ENV_VARS_STRING_VALUES_CLASS.new field: { port: 'el lunes nos ponemos camisetas naranjas' } + expect(instance).not_to be_valid + expect(instance.errors[:field]).to include 'cannot set PORT' end it 'does not allow variables with zero key length' do - fake_class = environment_variables_class.new field: { '': 'el lunes nos ponemos camisetas naranjas' } - expect(fake_class).not_to be_valid - expect(fake_class.errors[:field]).to include 'key must be a minimum length of 1' + instance = FAKE_ENV_VARS_STRING_VALUES_CLASS.new field: { '': 'el lunes nos ponemos camisetas naranjas' } + expect(instance).not_to be_valid + expect(instance.errors[:field]).to include 'key must be a minimum length of 1' end it 'does not allow variables with non-string keys' do - fake_class = environment_variables_class.new field: { 1 => 'el lunes nos ponemos camisetas naranjas' } - expect(fake_class).not_to be_valid - expect(fake_class.errors[:field]).to include 'key must be a string' + instance = FAKE_ENV_VARS_STRING_VALUES_CLASS.new field: { 1 => 'el lunes nos ponemos camisetas naranjas' } + expect(instance).not_to be_valid + expect(instance.errors[:field]).to include 'key must be a string' end it 'does not allow variables with array values' do - fake_class = environment_variables_class.new field: { fibonacci: [1, 1, 2, 3, 5, 8] } - expect(fake_class).not_to be_valid - expect(fake_class.errors[:base]).to eq ["Non-string value in environment variable for key 'fibonacci', value '[1,1,2,3,5,8]'"] + instance = FAKE_ENV_VARS_STRING_VALUES_CLASS.new field: { fibonacci: [1, 1, 2, 3, 5, 8] } + expect(instance).not_to be_valid + expect(instance.errors[:base]).to eq ["Non-string value in environment variable for key 'fibonacci', value '[1,1,2,3,5,8]'"] end it 'does not allow variables with object values' do - fake_class = environment_variables_class.new field: { obj: { wow: 'cool' } } - expect(fake_class).not_to be_valid - expect(fake_class.errors[:base]).to eq ["Non-string value in environment variable for key 'obj', value '{\"wow\":\"cool\"}'"] + instance = FAKE_ENV_VARS_STRING_VALUES_CLASS.new field: { obj: { wow: 'cool' } } + expect(instance).not_to be_valid + expect(instance.errors[:base]).to eq ["Non-string value in environment variable for key 'obj', value '{\"wow\":\"cool\"}'"] end end describe 'FieldsValidator' do - let(:fields_class) do - Class.new(fake_class) do - validates :field, fields: { allowed: { 'space.organization' => ['name'] } } - end - end - it 'rejects values that are not hashes' do - fake_class = fields_class.new field: 'foo' - expect(fake_class).not_to be_valid - expect(fake_class.errors[:field]).to include 'must be an object' + instance = FAKE_FIELDS_CLASS.new field: 'foo' + expect(instance).not_to be_valid + expect(instance.errors[:field]).to include 'must be an object' end context 'allowed keys' do - let(:fields_class_multiple_keys) do - Class.new(fake_class) do - validates :field, fields: { allowed: { 'some.resource' => %w[fake-value-1 fake-value-2] } } - end - end - it 'allows a multiple keys to be present' do - fake_class = fields_class_multiple_keys.new field: { 'some.resource': %w[fake-value-2 fake-value-1] } - expect(fake_class).to be_valid + instance = FAKE_FIELDS_MULTI_KEYS_CLASS.new field: { 'some.resource': %w[fake-value-2 fake-value-1] } + expect(instance).to be_valid end it 'allows a subset of keys' do - fake_class = fields_class_multiple_keys.new field: { 'some.resource': %w[fake-value-2] } - expect(fake_class).to be_valid + instance = FAKE_FIELDS_MULTI_KEYS_CLASS.new field: { 'some.resource': %w[fake-value-2] } + expect(instance).to be_valid end it 'reject keys not in the list' do - fake_class = fields_class_multiple_keys.new field: { 'some.resource': %w[fake-value-2 url] } - expect(fake_class).not_to be_valid - expect(fake_class.errors[:field]).to include "valid keys for 'some.resource' are: 'fake-value-1', 'fake-value-2'" + instance = FAKE_FIELDS_MULTI_KEYS_CLASS.new field: { 'some.resource': %w[fake-value-2 url] } + expect(instance).not_to be_valid + expect(instance.errors[:field]).to include "valid keys for 'some.resource' are: 'fake-value-1', 'fake-value-2'" end end context 'allowed resources' do - let(:fields_class_multiple_resources) do - Class.new(fake_class) do - validates :field, fields: { allowed: { 'a.resource' => ['fake-value'], 'another.resource' => ['another-fake-value'] } } - end - end - it 'allows a multiple resources to be present' do - fake_class = fields_class_multiple_resources.new field: { 'a.resource': %w[fake-value], 'another.resource': %w[another-fake-value] } - expect(fake_class).to be_valid + instance = FAKE_FIELDS_MULTI_RESOURCES_CLASS.new field: { 'a.resource': %w[fake-value], 'another.resource': %w[another-fake-value] } + expect(instance).to be_valid end it 'allows a subset of the resources to be present' do - fake_class = fields_class_multiple_resources.new field: { 'another.resource': %w[another-fake-value] } - expect(fake_class).to be_valid + instance = FAKE_FIELDS_MULTI_RESOURCES_CLASS.new field: { 'another.resource': %w[another-fake-value] } + expect(instance).to be_valid end it 'rejects resources not specified' do - fake_class = fields_class_multiple_resources.new field: { 'wrong.resource': %w[another-fake-value] } - expect(fake_class).not_to be_valid - expect(fake_class.errors[:field]).to include "[wrong.resource] valid resources are: 'a.resource', 'another.resource'" + instance = FAKE_FIELDS_MULTI_RESOURCES_CLASS.new field: { 'wrong.resource': %w[another-fake-value] } + expect(instance).not_to be_valid + expect(instance.errors[:field]).to include "[wrong.resource] valid resources are: 'a.resource', 'another.resource'" end end end describe 'HealthCheckValidator' do - let(:health_check_class) do - Class.new(fake_class) do - attr_accessor :health_check_type, :health_check_http_endpoint - - validates_with HealthCheckValidator - end - end - context 'when the healthcheck type is not "http"' do it 'correctly adds the health_check_type validation errors' do - message = health_check_class.new({ - health_check_type: 'not-http', - health_check_http_endpoint: 'a-great-uri' - }) + message = FAKE_HEALTH_CHECK_CLASS.new({ + health_check_type: 'not-http', + health_check_http_endpoint: 'a-great-uri' + }) expect(message).not_to be_valid expect(message.errors_on(:health_check_type)).to include('must be "http" to set a health check HTTP endpoint') @@ -429,7 +371,7 @@ def validate_each(record, attr_name, value) describe 'LifecycleValidator' do let(:lifecycle_class) do - Class.new(fake_class) do + Class.new(FAKE_BASE_CLASS) do attr_accessor :lifecycle validates_with LifecycleValidator @@ -530,21 +472,15 @@ class Relationships < VCAP::CloudController::BaseMessage end describe 'ToOneRelationshipValidator' do - let(:to_one_class) do - Class.new(fake_class) do - validates :field, to_one_relationship: true - end - end - it 'ensures that the data has the correct structure' do - bad_guid_key = to_one_class.new({ field: { data: { not_a_guid: '1234' } } }) - bad_guid_value = to_one_class.new({ field: { data: { guid: { woah: '1234' } } } }) + bad_guid_key = FAKE_TO_ONE_CLASS.new({ field: { data: { not_a_guid: '1234' } } }) + bad_guid_value = FAKE_TO_ONE_CLASS.new({ field: { data: { guid: { woah: '1234' } } } }) - bad_data_key = to_one_class.new({ field: { not_data: '1234' } }) - bad_data_value = to_one_class.new({ field: { data: '1234' } }) - missing_data = to_one_class.new({ field: '1234' }) + bad_data_key = FAKE_TO_ONE_CLASS.new({ field: { not_data: '1234' } }) + bad_data_value = FAKE_TO_ONE_CLASS.new({ field: { data: '1234' } }) + missing_data = FAKE_TO_ONE_CLASS.new({ field: '1234' }) - valid = to_one_class.new(field: { data: { guid: '1234' } }) + valid = FAKE_TO_ONE_CLASS.new(field: { data: { guid: '1234' } }) expect(bad_guid_key).not_to be_valid expect(bad_guid_value).not_to be_valid @@ -555,32 +491,26 @@ class Relationships < VCAP::CloudController::BaseMessage end it 'allows for nil value in data' do - valid = to_one_class.new(field: { data: nil }) + valid = FAKE_TO_ONE_CLASS.new(field: { data: nil }) expect(valid).to be_valid end it 'adds an error if the field is not structured correctly' do - invalid = to_one_class.new({ field: { data: { not_a_guid: 1234 } } }) + invalid = FAKE_TO_ONE_CLASS.new({ field: { data: { not_a_guid: 1234 } } }) expect(invalid).not_to be_valid expect(invalid.errors[:field]).to include 'must be structured like this: "field: {"data": {"guid": "valid-guid"}}"' end end describe 'ToManyRelationshipValidator' do - let(:to_many_class) do - Class.new(fake_class) do - validates :field, to_many_relationship: true - end - end - it 'ensures that the data has the correct structure' do - valid = to_many_class.new({ field: { - data: [{ guid: '1234' }, { guid: '1234' }, { guid: '1234' }, { guid: '1234' }] - } }) - invalid_one = to_many_class.new({ field: { data: { guid: '1234' } } }) - invalid_two = to_many_class.new({ field: { data: [{ guid: 1234 }, { guid: 1234 }] } }) - invalid_three = to_many_class.new({ field: [{ guid: '1234' }, { guid: '1234' }, { guid: '1234' }, { guid: '1234' }] }) + valid = FAKE_TO_MANY_CLASS.new({ field: { + data: [{ guid: '1234' }, { guid: '1234' }, { guid: '1234' }, { guid: '1234' }] + } }) + invalid_one = FAKE_TO_MANY_CLASS.new({ field: { data: { guid: '1234' } } }) + invalid_two = FAKE_TO_MANY_CLASS.new({ field: { data: [{ guid: 1234 }, { guid: 1234 }] } }) + invalid_three = FAKE_TO_MANY_CLASS.new({ field: [{ guid: '1234' }, { guid: '1234' }, { guid: '1234' }, { guid: '1234' }] }) expect(valid).to be_valid expect(invalid_one).not_to be_valid @@ -590,17 +520,11 @@ class Relationships < VCAP::CloudController::BaseMessage end describe 'OrgVisibilityValidator' do - let(:visibility_class) do - Class.new(fake_class) do - validates :field, org_visibility: true - end - end - it 'ensures that it has correct structure' do - valid = visibility_class.new({ field: [{ guid: '1234' }, { guid: '1234' }, { guid: '1234' }, { guid: '1234' }] }) - invalid_one = visibility_class.new({ field: { guid: '1234' } }) - invalid_two = visibility_class.new({ field: [{ guid: 1234 }, { guid: 1234 }] }) - invalid_three = visibility_class.new({ field: ['123'] }) + valid = FAKE_VISIBILITY_CLASS.new({ field: [{ guid: '1234' }, { guid: '1234' }, { guid: '1234' }, { guid: '1234' }] }) + invalid_one = FAKE_VISIBILITY_CLASS.new({ field: { guid: '1234' } }) + invalid_two = FAKE_VISIBILITY_CLASS.new({ field: [{ guid: 1234 }, { guid: 1234 }] }) + invalid_three = FAKE_VISIBILITY_CLASS.new({ field: ['123'] }) expect(valid).to be_valid expect(invalid_one).not_to be_valid @@ -610,84 +534,78 @@ class Relationships < VCAP::CloudController::BaseMessage end describe 'TimestampValidator' do - let(:timestamp_class) do - Class.new(fake_class) do - validates :field, timestamp: true - end - end - it 'requires a hash or an array of timestamps' do - message = timestamp_class.new({ field: 47 }) + message = FAKE_TIMESTAMP_CLASS.new({ field: 47 }) expect(message).not_to be_valid expect(message.errors[:field]).to include('relational operator and timestamp must be specified') end it 'requires a valid relational operator' do - message = timestamp_class.new({ field: { garbage: Time.now.utc.iso8601 } }) + message = FAKE_TIMESTAMP_CLASS.new({ field: { garbage: Time.now.utc.iso8601 } }) expect(message).not_to be_valid expect(message.errors[:field]).to include("Invalid relational operator: 'garbage'") end context 'requires a valid timestamp' do it 'does not accept a malformed timestamp' do - message = timestamp_class.new({ field: [Time.now.utc.iso8601.to_s, 'bogus'] }) + message = FAKE_TIMESTAMP_CLASS.new({ field: [Time.now.utc.iso8601.to_s, 'bogus'] }) expect(message).not_to be_valid expect(message.errors[:field]).to include("has an invalid timestamp format. Timestamps should be formatted as 'YYYY-MM-DDThh:mm:ssZ'") end it 'does not accept garbage' do - message = timestamp_class.new({ field: { gt: 123 } }) + message = FAKE_TIMESTAMP_CLASS.new({ field: { gt: 123 } }) expect(message).not_to be_valid expect(message.errors[:field]).to include("has an invalid timestamp format. Timestamps should be formatted as 'YYYY-MM-DDThh:mm:ssZ'") end it "does not accept fractional seconds even though it's ISO 8601-compliant" do - message = timestamp_class.new({ field: { gt: '2020-06-30T12:34:56.78Z' } }) + message = FAKE_TIMESTAMP_CLASS.new({ field: { gt: '2020-06-30T12:34:56.78Z' } }) expect(message).not_to be_valid expect(message.errors[:field]).to include("has an invalid timestamp format. Timestamps should be formatted as 'YYYY-MM-DDThh:mm:ssZ'") end it "does not accept local time zones even though it's ISO 8601-compliant" do - message = timestamp_class.new({ field: { gt: '2020-06-30T12:34:56.78-0700' } }) + message = FAKE_TIMESTAMP_CLASS.new({ field: { gt: '2020-06-30T12:34:56.78-0700' } }) expect(message).not_to be_valid expect(message.errors[:field]).to include("has an invalid timestamp format. Timestamps should be formatted as 'YYYY-MM-DDThh:mm:ssZ'") end end it 'allows comma-separated timestamps' do - message = timestamp_class.new({ field: [Time.now.utc.iso8601.to_s, Time.now.utc.iso8601.to_s] }) + message = FAKE_TIMESTAMP_CLASS.new({ field: [Time.now.utc.iso8601.to_s, Time.now.utc.iso8601.to_s] }) expect(message).to be_valid end it 'allows the lt operator' do - message = timestamp_class.new({ field: { lt: Time.now.utc.iso8601 } }) + message = FAKE_TIMESTAMP_CLASS.new({ field: { lt: Time.now.utc.iso8601 } }) expect(message).to be_valid end it 'allows the lte operator' do - message = timestamp_class.new({ field: { lte: Time.now.utc.iso8601 } }) + message = FAKE_TIMESTAMP_CLASS.new({ field: { lte: Time.now.utc.iso8601 } }) expect(message).to be_valid end it 'allows the gt operator' do - message = timestamp_class.new({ field: { gt: Time.now.utc.iso8601 } }) + message = FAKE_TIMESTAMP_CLASS.new({ field: { gt: Time.now.utc.iso8601 } }) expect(message).to be_valid end it 'allows the gte operator' do - message = timestamp_class.new({ field: { gte: Time.now.utc.iso8601 } }) + message = FAKE_TIMESTAMP_CLASS.new({ field: { gte: Time.now.utc.iso8601 } }) expect(message).to be_valid end it 'does not allow multiple timestamps with an operator' do - message = timestamp_class.new({ field: { gte: "#{Time.now.utc.iso8601},#{Time.now.utc.iso8601}" } }) + message = FAKE_TIMESTAMP_CLASS.new({ field: { gte: "#{Time.now.utc.iso8601},#{Time.now.utc.iso8601}" } }) expect(message).not_to be_valid expect(message.errors[:field]).to include('only accepts one value when using a relational operator') end context 'when the operator is an equals operator' do it 'allows the equals operator' do - message = timestamp_class.new({ field: [Time.now.utc.iso8601] }) + message = FAKE_TIMESTAMP_CLASS.new({ field: [Time.now.utc.iso8601] }) expect(message).to be_valid end end