Browse Source

Maintenance: Improve Redis setup

Unifies Redis client configuration and allows TLS connections.
Tobias Schäfer 3 weeks ago
parent
commit
eb35506517

+ 19 - 0
.dev/rubocop/cop/zammad/forbid_redis_client.rb

@@ -0,0 +1,19 @@
+# Copyright (C) 2012-2025 Zammad Foundation, https://zammad-foundation.org/
+
+module RuboCop
+  module Cop
+    module Zammad
+      class ForbidRedisClient < Base
+        MSG = 'Do not use Redis.new directly. Use Zammad::Service::Redis.new instead.'.freeze
+
+        def_node_matcher :redis_client_usage?, <<~PATTERN
+          (send (const nil? :Redis) :new)
+        PATTERN
+
+        def on_send(node)
+          add_offense(node, message: MSG) if redis_client_usage?(node)
+        end
+      end
+    end
+  end
+end

+ 1 - 0
.dev/rubocop/rubocop_zammad.rb

@@ -12,6 +12,7 @@ require_relative 'cop/zammad/faker_unique'
 require_relative 'cop/zammad/forbid_def_send'
 require_relative 'cop/zammad/forbid_def_send'
 require_relative 'cop/zammad/forbid_default_scope'
 require_relative 'cop/zammad/forbid_default_scope'
 require_relative 'cop/zammad/forbid_rand'
 require_relative 'cop/zammad/forbid_rand'
+require_relative 'cop/zammad/forbid_redis_client'
 require_relative 'cop/zammad/forbid_translatable_marker'
 require_relative 'cop/zammad/forbid_translatable_marker'
 require_relative 'cop/zammad/migration_scheduler_last_run'
 require_relative 'cop/zammad/migration_scheduler_last_run'
 require_relative 'cop/zammad/no_to_sym_on_string'
 require_relative 'cop/zammad/no_to_sym_on_string'

+ 1 - 2
app/models/system_report/plugin/redis.rb

@@ -4,8 +4,7 @@ class SystemReport::Plugin::Redis < SystemReport::Plugin
   DESCRIPTION = __('Redis version').freeze
   DESCRIPTION = __('Redis version').freeze
 
 
   def fetch
   def fetch
-    redis_url = ENV['REDIS_URL'].presence || 'redis://localhost:6379'
-    ::Redis.new(driver: :hiredis, url: redis_url).info
+    Zammad::Service::Redis.new.info
   rescue
   rescue
     nil
     nil
   end
   end

+ 12 - 14
app/services/service/execute_locked_block.rb

@@ -1,44 +1,42 @@
 # Copyright (C) 2012-2025 Zammad Foundation, https://zammad-foundation.org/
 # Copyright (C) 2012-2025 Zammad Foundation, https://zammad-foundation.org/
 
 
 class Service::ExecuteLockedBlock < Service::Base
 class Service::ExecuteLockedBlock < Service::Base
-  REDIS_URL = ENV['REDIS_URL'].presence || 'redis://localhost:6379'
 
 
   attr_reader :resource, :ttl, :redis_url
   attr_reader :resource, :ttl, :redis_url
 
 
-  def self.locked?(resource, redis_url: REDIS_URL)
-    dlm = Redlock::Client.new(redis_url)
+  def self.locked?(resource)
+    dlm = Redlock::Client.new
     dlm.locked?(resource)
     dlm.locked?(resource)
   end
   end
 
 
-  def self.locked!(resource, redis_url: REDIS_URL)
-    raise(ExecuteLockedBlockError) if locked?(resource, redis_url: redis_url)
+  def self.locked!(resource)
+    raise(ExecuteLockedBlockError) if locked?(resource)
   end
   end
 
 
-  def self.lock(resource, ttl, redis_url: REDIS_URL)
-    dlm = Redlock::Client.new(redis_url)
+  def self.lock(resource, ttl)
+    dlm = Redlock::Client.new
     dlm.lock(resource, ttl)
     dlm.lock(resource, ttl)
   end
   end
 
 
-  def self.unlock(lock_info, redis_url: REDIS_URL)
-    dlm = Redlock::Client.new(redis_url)
+  def self.unlock(lock_info)
+    dlm = Redlock::Client.new
     dlm.unlock(lock_info)
     dlm.unlock(lock_info)
   end
   end
 
 
-  def self.extend(lock_info, redis_url: REDIS_URL)
-    dlm = Redlock::Client.new(redis_url)
+  def self.extend(lock_info)
+    dlm = Redlock::Client.new
     dlm.lock(nil, nil, extend: lock_info)
     dlm.lock(nil, nil, extend: lock_info)
   end
   end
 
 
-  def initialize(resource, ttl, redis_url: REDIS_URL)
+  def initialize(resource, ttl)
     super()
     super()
 
 
     @resource = resource
     @resource = resource
     @ttl = ttl
     @ttl = ttl
-    @redis_url = redis_url
   end
   end
 
 
   def execute(&)
   def execute(&)
-    dlm = Redlock::Client.new(redis_url)
+    dlm = Redlock::Client.new
     dlm.lock(resource, ttl, &)
     dlm.lock(resource, ttl, &)
   end
   end
 
 

+ 5 - 2
config/initializers/zzz_action_cable_preferences.rb

@@ -1,17 +1,20 @@
 # Copyright (C) 2012-2025 Zammad Foundation, https://zammad-foundation.org/
 # Copyright (C) 2012-2025 Zammad Foundation, https://zammad-foundation.org/
 
 
+require_relative '../../lib/zammad/service/redis'
+
 # If REDIS_URL is not set, fall back to default port / localhost, to ease configuration
 # If REDIS_URL is not set, fall back to default port / localhost, to ease configuration
 #   for simple installations.
 #   for simple installations.
 redis_url = ENV['REDIS_URL'].presence || 'redis://localhost:6379'
 redis_url = ENV['REDIS_URL'].presence || 'redis://localhost:6379'
+driver = redis_url.start_with?('rediss://') ? :ruby : :hiredis
 
 
 Rails.application.config.action_cable.cable = {
 Rails.application.config.action_cable.cable = {
   adapter:        :redis,
   adapter:        :redis,
-  driver:         :hiredis,
+  driver:         driver,
   url:            redis_url,
   url:            redis_url,
   channel_prefix: "zammad_#{Rails.env}",
   channel_prefix: "zammad_#{Rails.env}",
 }
 }
 begin
 begin
-  Redis.new(driver: :hiredis, url: redis_url).ping
+  Zammad::Service::Redis.new.ping
   Rails.logger.info { "ActionCable is using the redis instance at #{redis_url}." }
   Rails.logger.info { "ActionCable is using the redis instance at #{redis_url}." }
 rescue Redis::CannotConnectError => e
 rescue Redis::CannotConnectError => e
   warn "There was an error trying to connect to Redis via #{redis_url}."
   warn "There was an error trying to connect to Redis via #{redis_url}."

+ 1 - 1
lib/app_version.rb

@@ -99,7 +99,7 @@ returns
   private_class_method :restart_required?
   private_class_method :restart_required?
 
 
   def self.redis
   def self.redis
-    self._redis ||= ::Redis.new(driver: :hiredis, url: ENV['REDIS_URL'].presence || 'redis://localhost:6379')
+    self._redis ||= Zammad::Service::Redis.new
   end
   end
   private_class_method :redis
   private_class_method :redis
 end
 end

+ 2 - 2
lib/redlock/client.rb

@@ -10,9 +10,9 @@
 #       [0] https://github.com/leandromoreira/redlock-rb
 #       [0] https://github.com/leandromoreira/redlock-rb
 module Redlock
 module Redlock
   class Client
   class Client
-    def initialize(server)
+    def initialize
       @id = SecureRandom.uuid
       @id = SecureRandom.uuid
-      @redis = Redis.new(driver: :hiredis, url: server)
+      @redis = Zammad::Service::Redis.new
     end
     end
 
 
     def lock(resource, ttl, options = {}, &block)
     def lock(resource, ttl, options = {}, &block)

+ 1 - 1
lib/sessions/store/redis.rb

@@ -7,7 +7,7 @@ class Sessions::Store::Redis
   NODES_KEY = 'nodes'.freeze
   NODES_KEY = 'nodes'.freeze
 
 
   def initialize
   def initialize
-    @redis = Redis.new(driver: :hiredis)
+    @redis = Zammad::Service::Redis.new
   end
   end
 
 
   def create(client_id, data)
   def create(client_id, data)

+ 14 - 0
lib/zammad/service/redis.rb

@@ -0,0 +1,14 @@
+# Copyright (C) 2012-2025 Zammad Foundation, https://zammad-foundation.org/
+
+module Zammad
+  module Service
+    class Redis < ::Redis
+      def initialize
+        url = ENV['REDIS_URL'].presence || 'redis://localhost:6379'
+        driver = url.start_with?('rediss://') ? :ruby : :hiredis
+
+        super(url: url, driver: driver)
+      end
+    end
+  end
+end

+ 16 - 0
spec/rubocop/cop/zammad/forbid_redis_client_spec.rb

@@ -0,0 +1,16 @@
+# Copyright (C) 2012-2025 Zammad Foundation, https://zammad-foundation.org/
+
+require 'rails_helper'
+require_relative '../../../../.dev/rubocop/cop/zammad/forbid_redis_client'
+
+RSpec.describe RuboCop::Cop::Zammad::ForbidRedisClient, type: :rubocop do
+  it 'accepts Zammad::Service::Redis.new' do
+    expect_no_offenses('Zammad::Service::Redis.new')
+  end
+
+  it 'rejects Redis.new' do
+    result = inspect_source('Redis.new')
+
+    expect(result.first.cop_name).to eq('Zammad/ForbidRedisClient')
+  end
+end

Some files were not shown because too many files changed in this diff