Skip to content

Commit 03879f0

Browse files
kathapkathap
committed
restructuring
1 parent e9634a8 commit 03879f0

File tree

2 files changed

+81
-76
lines changed

2 files changed

+81
-76
lines changed

‎middleware/service_broker_rate_limiter.rb‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,14 +113,14 @@ def call(env)
113113

114114
def apply_rate_limiting?(env)
115115
request = ActionDispatch::Request.new(env)
116-
!admin? && rate_limit_method?(request)
116+
!admin? && is_rate_limited_service_request?(request)
117117
end
118118

119119
def admin?
120120
VCAP::CloudController::SecurityContext.admin? || VCAP::CloudController::SecurityContext.admin_read_only?
121121
end
122122

123-
def rate_limit_method?(request)
123+
def is_rate_limited_service_request?(request)
124124
RATE_LIMITED_ENDPOINTS.any? do |endpoint|
125125
endpoint.endpoint_pattern.match?(request.fullpath) && endpoint.request_methods.include?(request.method)
126126
end

‎spec/unit/middleware/service_broker_rate_limiter_spec.rb‎

Lines changed: 79 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,85 @@ module Middleware
6161
expect(status).to eq(200)
6262
end
6363

64+
describe 'service endpoints' do
65+
shared_examples 'rate-limited service endpoint' do
66+
before do
67+
allow(app).to receive(:call) do
68+
sleep(1)
69+
[200, {}, 'a body']
70+
end
71+
end
72+
73+
it 'does not allow more than the max number of concurrent requests' do
74+
threads = 2.times.map { Thread.new { middleware.call(env) } }
75+
statuses = threads.map(&:join).map(&:value).map(&:first)
76+
77+
expect(statuses).to include(200)
78+
expect(statuses).to include(429)
79+
expect(app).to have_received(:call).once
80+
expect(logger).to have_received(:info).with("Service broker concurrent rate limit exceeded for user '#{user_guid}'")
81+
end
82+
end
83+
84+
context 'v2 rate limited service endpoints' do
85+
v2_rate_limited_service_endpoints = %w[/v2/service_instances /v2/service_bindings /v2/service_keys]
86+
87+
methods = %w[PUT POST DELETE]
88+
89+
v2_rate_limited_service_endpoints.each do |endpoint|
90+
methods.each do |method|
91+
context "#{endpoint} #{method} - rate-limited" do
92+
let(:path) { endpoint }
93+
let(:request_method) { method }
94+
let(:fake_request) { instance_double(ActionDispatch::Request, fullpath: endpoint, method: method) }
95+
96+
include_examples 'rate-limited service endpoint'
97+
end
98+
end
99+
end
100+
end
101+
102+
context 'v3 rate limited GET service endpoints' do
103+
v3_rate_limited_service_endpoints = %w[/v3/service_instances/:guid/parameters /v3/service_credential_bindings/:guid/parameters
104+
/v3/service_route_bindings/:guid/parameters]
105+
106+
v3_rate_limited_service_endpoints.each do |endpoint|
107+
context "#{endpoint} GET - rate-limited" do
108+
let(:path) { endpoint.gsub(':guid', instance.guid.to_s) }
109+
let(:request_method) { 'GET' }
110+
let(:fake_request) { instance_double(ActionDispatch::Request, fullpath: path, method: request_method) }
111+
112+
include_examples 'rate-limited service endpoint'
113+
end
114+
end
115+
end
116+
117+
context 'v3 non rate limited service endpoints' do
118+
v3_not_rate_limited_service_endpoints = %w[/v3/service_instances /v3/service_credential_bindings /v3/service_route_bindings]
119+
120+
methods = %w[POST PATCH DELETE]
121+
122+
v3_not_rate_limited_service_endpoints.each do |endpoint|
123+
methods.each do |method|
124+
context "#{endpoint} #{method} - non-rate-limited" do
125+
let(:path) { endpoint }
126+
let(:request_method) { method }
127+
let(:fake_request) { instance_double(ActionDispatch::Request, fullpath: endpoint, method: method) }
128+
129+
it 'allows concurrent requests without limits' do
130+
threads = 2.times.map { Thread.new { middleware.call(env) } }
131+
statuses = threads.map(&:join).map(&:value).map(&:first)
132+
133+
expect(statuses).to all(eq(200))
134+
expect(app).to have_received(:call).twice
135+
expect(logger).not_to have_received(:info)
136+
end
137+
end
138+
end
139+
end
140+
end
141+
end
142+
64143
describe 'errors' do
65144
let(:max_concurrent_requests) { 0 }
66145

@@ -109,80 +188,6 @@ module Middleware
109188
end
110189
end
111190
end
112-
113-
shared_examples 'rate-limited endpoint' do
114-
it 'does not allow more than the max number of concurrent requests' do
115-
allow(app).to receive(:call) do
116-
sleep(1)
117-
[200, {}, 'a body']
118-
end
119-
120-
threads = 2.times.map { Thread.new { middleware.call(env) } }
121-
statuses = threads.map(&:join).map(&:value).map(&:first)
122-
123-
expect(statuses).to include(200)
124-
expect(statuses).to include(429)
125-
expect(app).to have_received(:call).once
126-
expect(logger).to have_received(:info).with("Service broker concurrent rate limit exceeded for user '#{user_guid}'")
127-
end
128-
end
129-
130-
v2_rate_limited_service_endpoints = %w[/v2/service_instances /v2/service_bindings /v2/service_keys]
131-
132-
methods = %w[PUT POST DELETE]
133-
134-
v2_rate_limited_service_endpoints.each do |endpoint|
135-
methods.each do |method|
136-
context "#{endpoint} #{method} endpoint" do
137-
let(:path) { endpoint }
138-
let(:request_method) { method }
139-
let(:fake_request) { instance_double(ActionDispatch::Request, fullpath: endpoint, method: method) }
140-
141-
include_examples 'rate-limited endpoint'
142-
end
143-
end
144-
end
145-
146-
v3_rate_limited_service_endpoints = %w[/v3/service_instances/:guid/parameters /v3/service_credential_bindings/:guid/parameters /v3/service_route_bindings/:guid/parameters]
147-
148-
v3_rate_limited_service_endpoints.each do |endpoint|
149-
context "#{endpoint} GET" do
150-
let(:path) { endpoint.gsub(':guid', instance.guid.to_s) }
151-
let(:request_method) { 'GET' }
152-
let(:fake_request) { instance_double(ActionDispatch::Request, fullpath: path, method: request_method) }
153-
154-
include_examples 'rate-limited endpoint'
155-
end
156-
end
157-
end
158-
159-
describe 'excluded requests' do
160-
shared_examples 'non-rate-limited endpoint' do
161-
it 'allows concurrent requests without limits' do
162-
threads = 2.times.map { Thread.new { middleware.call(env) } }
163-
statuses = threads.map(&:join).map(&:value).map(&:first)
164-
165-
expect(statuses).to all(eq(200))
166-
expect(app).to have_received(:call).twice
167-
expect(logger).not_to have_received(:info)
168-
end
169-
end
170-
171-
v3_not_rate_limited_service_endpoints = %w[/v3/service_instances /v3/service_credential_bindings /v3/service_route_bindings]
172-
173-
methods = %w[POST PATCH DELETE]
174-
175-
v3_not_rate_limited_service_endpoints.each do |endpoint|
176-
methods.each do |method|
177-
context "#{endpoint} #{method} - non-rate-limited" do
178-
let(:path) { endpoint }
179-
let(:request_method) { method }
180-
let(:fake_request) { instance_double(ActionDispatch::Request, fullpath: endpoint, method: method) }
181-
182-
include_examples 'non-rate-limited endpoint'
183-
end
184-
end
185-
end
186191
end
187192

188193
describe 'skipped requests' do

0 commit comments

Comments
 (0)