Adjust rate limits for service-related endpoints.

Most if not all service broker operations for the CF API v3 have been moved into background jobs and are asynchronous operations.
This means that the service broker operation can't block anymore the CF API v3 requests (which returns a 202 and a link to the polling job).
After reviewing /v3/service related endpoints, we adapted the rate
limiting:
- v3 service related endpoints that never interact with SBs synchronously are removed from rate limiting (POST, PATCH, DELETE /v3/[service_instances|service_credential_bindings|service_route_bindings])
- v3 GET methods that interact with SBs synchronously are added to rate
  limiting
  (/v3/(service_instances|service_credential_bindings|service_route_bindings)/.+/parameters)

Author: kathap

middleware/service_broker_rate_limiter.rb

@@ -3,6 +3,14 @@
 
 module CloudFoundry
   module Middleware
+    RateLimitEndpoint = Struct.new(:endpoint_pattern, :methods)
+
+    RATE_LIMITED_ENDPOINTS = [
+      RateLimitEndpoint.new(%r{\A/v2/(service_instances|service_credential_bindings|service_route_bindings)}, %w[PUT POST DELETE PATCH]),
+      RateLimitEndpoint.new(%r{\A/v3/(service_instances|service_credential_bindings|service_route_bindings)/.+/parameters\z}, %w[GET]),
+      RateLimitEndpoint.new(%r{\A/v3/(service_instances|service_credential_bindings|service_route_bindings)}, %w[PUT])
+    ].freeze
+
     class ConcurrentRequestCounter
       def initialize(key_prefix, redis_connection_pool_size: nil)
         @key_prefix = key_prefix
@@ -106,26 +114,17 @@ def call(env)
 
       def apply_rate_limiting?(env)
         request = ActionDispatch::Request.new(env)
-        !admin? && is_service_request?(request) && rate_limit_method?(request)
+        !admin? && rate_limit_method?(request)
       end
 
       def admin?
         VCAP::CloudController::SecurityContext.admin? || VCAP::CloudController::SecurityContext.admin_read_only?
       end
 
-      def is_service_request?(request)
-        [
-          %r{\A/v2/service_instances},
-          %r{\A/v2/service_bindings},
-          %r{\A/v2/service_keys},
-          %r{\A/v3/service_instances},
-          %r{\A/v3/service_credential_bindings},
-          %r{\A/v3/service_route_bindings}
-        ].any? { |re| request.fullpath.match(re) }
-      end
-
       def rate_limit_method?(request)
-        %w[PATCH PUT POST DELETE].include?(request.method)
+        RATE_LIMITED_ENDPOINTS.any? do |endpoint|
+          endpoint.endpoint_pattern.match?(request.fullpath) && endpoint.methods.include?(request.method)
+        end
       end
 
       def suggested_retry_after

spec/unit/middleware/service_broker_rate_limiter_spec.rb

@@ -8,7 +8,7 @@ module Middleware
       let(:path_info) { '/v3/service_instances' }
       let(:user_guid) { SecureRandom.uuid }
       let(:user_env) { { 'cf.user_guid' => user_guid, 'PATH_INFO' => path_info } }
-      let(:fake_request) { instance_double(ActionDispatch::Request, fullpath: '/v3/service_instances', method: 'POST') }
+      let(:fake_request) { instance_double(ActionDispatch::Request, fullpath: '/v3/service_instances', method: 'PUT') }
       let(:max_concurrent_requests) { 1 }
       let(:broker_timeout) { 60 }
       let(:middleware) do
@@ -25,6 +25,8 @@ module Middleware
       end
 
       describe 'included requests' do
+        let(:request_method) { 'PUT' } # Adjust as needed to test different HTTP methods
+
         it 'allows a service broker request within the limit' do
           status, = middleware.call(user_env)
           expect(status).to eq(200)
@@ -117,6 +119,21 @@ module Middleware
         end
       end
 
+      describe 'excluded requests' do
+        let(:request_method) { 'POST' }
+
+        it 'allows requests that are no longer rate limited' do
+          status, = middleware.call(user_env)
+          expect(status).to eq(200)
+
+          status, = middleware.call(user_env)
+          expect(status).to eq(200)
+
+          statuses = 2.times.map { middleware.call(user_env).first }
+          expect(statuses).to match_array([200, 200])
+        end
+      end
+
       describe 'skipped requests' do
         let(:concurrent_request_counter) { double }