Fixes #39208 - Cache global registration script in smart-proxy

GET /register returns the same shell script for all hosts sharing the
same registration parameters (org, location, hostgroup, activation keys).
Under bulk registration — 100+ hosts hitting the same capsule simultaneously
— this endpoint is called once per host, each time proxying to Foreman and
waiting for the ERB template to render (~103ms in profiling).

Cache the rendered script in memory (5-minute TTL) keyed on a canonical
form of the request query string. The cache key is computed by parsing the
query string, sorting parameters alphabetically, and rebuilding — so
requests that differ only in parameter order share the same cache entry
and the same Foreman response, regardless of how the client ordered them.

The implementation uses three clearly separated layers:

  get '/'             — handles errors only; delegates to registration_script
  registration_script — owns the cache key and the business logic (what
                        to cache and what to do on a miss); raises
                        ScriptFetchError on non-200 so errors never reach
                        the cache write
  cache(key, &block)  — owns the mechanism: per-key double-checked locking
                        via a block abstraction that keeps the caller free
                        of locking concerns

Per-key locking allows concurrent requests for genuinely different keys
(e.g. different activation keys) to fetch from Foreman in parallel, while
serialising only threads competing for the same key. The per-key Mutex is
evicted from KEY_MUTEXES immediately after caching — once a key is hot,
all future requests take the lock-free fast path and KEY_MUTEXES is empty
under steady state.

Only HTTP 200 responses are cached. Non-200 responses raise ScriptFetchError
out of the cache block, which is rescued in get '/' and rendered via
handle_response without poisoning the cache.

Both KEY_MUTEXES and SCRIPT_CACHE use Concurrent::Map (already a
smart-proxy dependency) for lock-free, thread-safe access on all Ruby
VMs without relying on MRI's GIL.

Tests added:
- Cache hit: Foreman called once for repeated identical requests
- Per-key isolation: different parameter sets cached independently
- Parameter order independence: requests differing only in param order
  share the same cache entry
- TTL expiry: expired entries are not served; Foreman is re-called
- Mutex eviction: KEY_MUTEXES is empty after a successful cache write
- Error non-caching: Foreman is called on every request when it errors
- setup clears both SCRIPT_CACHE and KEY_MUTEXES between tests

Author: pablomh

modules/registration/registration_api.rb

@@ -1,9 +1,50 @@
 require 'registration/proxy_request'
 
 class Proxy::Registration::Api < ::Sinatra::Base
+  # Cache for the global registration script (GET /register).
+  #
+  # The script is identical for all hosts sharing the same registration
+  # parameters (org, location, hostgroup, activation keys), making it safe
+  # to serve from an in-memory cache during concurrent bulk registration.
+  #
+  # Per-key double-checked locking prevents thundering herd while allowing
+  # genuinely independent cache keys (e.g. different activation keys) to
+  # fetch from Foreman in parallel.
+  #
+  # Only HTTP 200 responses are cached — errors are raised out of the cache
+  # block so they never poison the cache. The per-key mutex is evicted
+  # immediately after caching so KEY_MUTEXES stays bounded to keys currently
+  # being fetched for the first time (zero under steady state).
+  REGISTRATION_SCRIPT_CACHE_TTL = 5 * 60 # seconds
+  KEY_MUTEXES                   = Concurrent::Map.new
+  SCRIPT_CACHE                  = Concurrent::Map.new
+
+  class ScriptFetchError < StandardError
+    attr_reader :response
+    def initialize(response)
+      super()
+      @response = response
+    end
+  end
+
+  class << self
+    def registration_script_cache
+      SCRIPT_CACHE
+    end
+
+    def key_mutex(cache_key)
+      KEY_MUTEXES.compute_if_absent(cache_key) { Mutex.new }
+    end
+
+    def evict_key_mutex(cache_key)
+      KEY_MUTEXES.delete(cache_key)
+    end
+  end
+
   get '/' do
-    response = Proxy::Registration::ProxyRequest.new.global_register(request)
-    handle_response(response)
+    registration_script
+  rescue ScriptFetchError => e
+    handle_response(e.response)
   rescue StandardError => e
     logger.exception "Error when rendering Global Registration Template", e
     render_error(default_error_msg)
@@ -19,11 +60,41 @@ class Proxy::Registration::Api < ::Sinatra::Base
 
   private
 
+  def registration_script
+    cache_key = Rack::Utils.build_query(
+      Rack::Utils.parse_nested_query(request.query_string).sort_by { |k, _| k }
+    )
+    cache(cache_key) do
+      response = Proxy::Registration::ProxyRequest.new.global_register(request)
+      raise ScriptFetchError.new(response) unless response.code == '200'
+      response.body
+    end
+  end
+
+  def cache(key, &block)
+    value = read_registration_cache(key)
+    return value if value
+
+    self.class.key_mutex(key).synchronize do
+      value = read_registration_cache(key)
+      return value if value
+
+      result = block.call
+      self.class.registration_script_cache[key] = { body: result, at: Time.now }
+      self.class.evict_key_mutex(key)
+      result
+    end
+  end
+
+  def read_registration_cache(cache_key)
+    entry = self.class.registration_script_cache[cache_key]
+    entry[:body] if entry && (Time.now - entry[:at]) < REGISTRATION_SCRIPT_CACHE_TTL
+  end
+
   def handle_response(response)
-    if response.code.start_with? '2'
+    if response.code.start_with?('2')
       response.body
     else
-      # Return error message only if it is not HTML.
       message = response["content-type"].include?('text/plain') ? response.body : default_error_msg
       render_error(message, code: response.code)
     end

test/registration/registration_api_test.rb

@@ -11,6 +11,9 @@ def app
   def setup
     @foreman_url = 'http://foreman.example.com'
     Proxy::SETTINGS.stubs(:foreman_url).returns(@foreman_url)
+    # Clear class-level state between tests to prevent cross-test contamination
+    Proxy::Registration::Api.registration_script_cache.clear
+    Proxy::Registration::Api::KEY_MUTEXES.clear
   end
 
   def test_global_register_template
@@ -102,6 +105,86 @@ def test_global_500
     assert_match("echo \"Internal Server Error\"\nexit 1\n", last_response.body)
   end
 
+  def test_global_register_caches_response
+    stub = stub_request(:get, "#{@foreman_url}/register").to_return(body: 'template')
+
+    2.times do
+      get '/'
+      assert last_response.ok?
+      assert_match('template', last_response.body)
+    end
+
+    assert_requested stub, times: 1
+  end
+
+  def test_global_register_cache_key_is_parameter_order_independent
+    # Cache key is normalised (params sorted alphabetically), so both orderings
+    # produce activation_keys=rhel9&owner=Default_Organization and share one entry.
+    stub_request(:get, "#{@foreman_url}/register?activation_keys=rhel9&owner=Default_Organization")
+      .to_return(body: 'template')
+
+    get '/', { owner: 'Default_Organization', activation_keys: 'rhel9' }
+    assert last_response.ok?
+
+    # Different parameter order — must hit cache, not Foreman again
+    get '/', { activation_keys: 'rhel9', owner: 'Default_Organization' }
+    assert last_response.ok?
+
+    assert_requested :get, "#{@foreman_url}/register?activation_keys=rhel9&owner=Default_Organization", times: 1
+  end
+
+  def test_global_register_caches_per_key
+    stub_a = stub_request(:get, "#{@foreman_url}/register?key=a").to_return(body: 'template_a')
+    stub_b = stub_request(:get, "#{@foreman_url}/register?key=b").to_return(body: 'template_b')
+
+    get '/', { key: 'a' }
+    assert_match('template_a', last_response.body)
+    get '/', { key: 'b' }
+    assert_match('template_b', last_response.body)
+    # second requests — must be served from cache
+    get '/', { key: 'a' }
+    assert_match('template_a', last_response.body)
+    get '/', { key: 'b' }
+    assert_match('template_b', last_response.body)
+
+    assert_requested stub_a, times: 1
+    assert_requested stub_b, times: 1
+  end
+
+  def test_global_register_evicts_mutex_after_caching
+    stub_request(:get, "#{@foreman_url}/register").to_return(body: 'template')
+
+    get '/'
+    assert last_response.ok?
+    assert_empty Proxy::Registration::Api::KEY_MUTEXES
+  end
+
+  def test_global_register_cache_entry_expires_after_ttl
+    Proxy::Registration::Api.registration_script_cache[''] = {
+      body: 'stale-template',
+      at: Time.now - Proxy::Registration::Api::REGISTRATION_SCRIPT_CACHE_TTL - 1,
+    }
+    stub = stub_request(:get, "#{@foreman_url}/register").to_return(body: 'fresh-template')
+
+    get '/'
+    assert last_response.ok?
+    assert_match('fresh-template', last_response.body)
+    assert_requested stub, times: 1
+  end
+
+  def test_global_register_does_not_cache_errors
+    stub = stub_request(:get, "#{@foreman_url}/register").to_return(
+      body: 'error', status: 500, headers: { "Content-Type" => 'text/plain' }
+    )
+
+    2.times do
+      get '/'
+      assert last_response.server_error?
+    end
+
+    assert_requested stub, times: 2
+  end
+
   def test_host_500
     Rack::NullLogger.any_instance.stubs(:exception)
     stub_request(:post, "#{@foreman_url}/register").to_timeout