feat(digest): allow modern algorithm (#853)

mhenrixon · Jul 25, 2024 · 6d31d93 · 6d31d93
1 parent bdca185
commit 6d31d93
Show file tree

Hide file tree

Showing 69 changed files with 339 additions and 225 deletions.
diff --git a/.github/workflows/rspec.yml b/.github/workflows/rspec.yml
@@ -63,6 +63,7 @@ jobs:
           - sidekiq_7.0
           - sidekiq_7.1
           - sidekiq_7.2
+          - sidekiq_7.3
 
     steps:
       - uses: actions/checkout@v4

diff --git a/.rubocop.yml b/.rubocop.yml
@@ -27,10 +27,24 @@ AllCops:
     - "myapp/**/*"
     - "Sidekiq/**/*"
     - "vendor/**/*"
+Layout/ArgumentAlignment:
+  Enabled: true
+  EnforcedStyle: with_fixed_indentation
 
 Layout/EndAlignment:
   EnforcedStyleAlignWith: variable
 
+Layout/FirstArrayElementIndentation:
+  EnforcedStyle: consistent
+
+Layout/FirstHashElementIndentation:
+  EnforcedStyle: consistent
+
+Layout/HashAlignment:
+  EnforcedColonStyle: key
+  EnforcedHashRocketStyle: key
+  EnforcedLastArgumentHashStyle: always_inspect
+
 Layout/LineContinuationLeadingSpace:
   Enabled: false
 
@@ -115,7 +129,13 @@ RSpec/ExpectActual:
 RSpec/ExpectChange:
   EnforcedStyle: block
 
-RSpec/FilePath:
+RSpec/SpecFilePathFormat:
+  Enabled: true
+  Exclude:
+    - spec/performance/locksmith_spec.rb
+    - spec/performance/lock_digest_spec.rb
+
+RSpec/SpecFilePathSuffix:
   Enabled: true
   Exclude:
     - spec/performance/locksmith_spec.rb

diff --git a/Appraisals b/Appraisals
@@ -15,3 +15,7 @@ end
 appraise "sidekiq-7.2" do
   gem "sidekiq", "~> 7.2.0"
 end
+
+appraise "sidekiq-7.3" do
+  gem "sidekiq", "~> 7.3.0"
+end
diff --git a/README.md b/README.md
@@ -59,6 +59,7 @@ Want to show me some ❤️ for the hard work I do on this gem? You can use the
     - [sidekiq-global_id](#sidekiq-global_id)
     - [sidekiq-status](#sidekiq-status)
   - [Global Configuration](#global-configuration)
+    - [digest_algorithm](#digest_algorithm)
     - [debug_lua](#debug_lua)
     - [lock_timeout](#lock_timeout)
     - [lock_ttl](#lock_ttl)
@@ -734,17 +735,29 @@ Configure SidekiqUniqueJobs in an initializer or the sidekiq initializer on appl
 
 ```ruby
 SidekiqUniqueJobs.configure do |config|
-  config.logger = Sidekiq.logger # default, change at your own discretion
-  config.logger_enabled  = true # default, disable for test environments
-  config.debug_lua       = false # Turn on when debugging
-  config.lock_info       = false # Turn on when debugging
-  config.lock_ttl        = 600   # Expire locks after 10 minutes
-  config.lock_timeout    = nil   # turn off lock timeout
-  config.max_history     = 0     # Turn on when debugging
-  config.reaper          = :ruby # :ruby, :lua or :none/nil
-  config.reaper_count    = 1000  # Stop reaping after this many keys
-  config.reaper_interval = 600   # Reap orphans every 10 minutes
-  config.reaper_timeout  = 150   # Timeout reaper after 2.5 minutes
+  config.logger = Sidekiq.logger    # default, change at your own discretion
+  config.logger_enabled   = true    # default, disable for test environments
+  config.debug_lua        = false   # Turn on when debugging
+  config.lock_info        = false   # Turn on when debugging
+  config.lock_ttl         = 600     # Expire locks after 10 minutes
+  config.lock_timeout     = nil     # turn off lock timeout
+  config.max_history      = 0       # Turn on when debugging
+  config.reaper           = :ruby   # :ruby, :lua or :none/nil
+  config.reaper_count     = 1000    # Stop reaping after this many keys
+  config.reaper_interval  = 600     # Reap orphans every 10 minutes
+  config.reaper_timeout   = 150     # Timeout reaper after 2.5 minutes
+  config.digest_algorithm = :modern # Timeout reaper after 2.5 minutes
+end
+```
+#### digest_algorithm
+
+For backwards compatibility this one is set to `:legacy` by the default. If you happen to run into issues with FIPS being enabled on your redis server you might want to set this to `:modern`.
+
+See: https://github.com/mhenrixon/sidekiq-unique-jobs/issues/848 for explanation
+
+```ruby
+SidekiqUniqueJobs.configure do |config|
+  config.digest_algorithm = :modern # Timeout reaper after 2.5 minutes
 end
 ```
 

diff --git a/gemfiles/sidekiq_7.1.gemfile b/gemfiles/sidekiq_7.1.gemfile
@@ -19,7 +19,7 @@ gem "sinatra"
 gem "timecop"
 gem "toxiproxy"
 gem "yard"
-gem "sidekiq", "~> 7.0.0"
+gem "sidekiq", "~> 7.1.0"
 
 platforms :mri do
   gem "concurrent-ruby-ext"

diff --git a/gemfiles/sidekiq_7.2.gemfile b/gemfiles/sidekiq_7.2.gemfile
@@ -19,7 +19,7 @@ gem "sinatra"
 gem "timecop"
 gem "toxiproxy"
 gem "yard"
-gem "sidekiq", "~> 7.0.0"
+gem "sidekiq", "~> 7.2.0"
 
 platforms :mri do
   gem "concurrent-ruby-ext"

diff --git a/gemfiles/sidekiq_7.3.gemfile b/gemfiles/sidekiq_7.3.gemfile
@@ -0,0 +1,28 @@
+# This file was generated by Appraisal
+
+source "https://rubygems.org"
+
+gem "appraisal"
+gem "faraday-retry"
+gem "gem-release"
+gem "github-markup"
+gem "rack-test"
+gem "rake", "13.0.3"
+gem "reek", ">= 5.3"
+gem "rspec"
+gem "rspec-benchmark"
+gem "rspec-html-matchers"
+gem "rspec-its"
+gem "rubocop-mhenrixon"
+gem "simplecov-sublime", ">= 0.21.2", require: false
+gem "sinatra"
+gem "timecop"
+gem "toxiproxy"
+gem "yard"
+gem "sidekiq", "~> 7.3.0"
+
+platforms :mri do
+  gem "concurrent-ruby-ext"
+end
+
+gemspec path: "../"
diff --git a/lib/sidekiq_unique_jobs/config.rb b/lib/sidekiq_unique_jobs/config.rb
@@ -2,26 +2,29 @@
 
 module SidekiqUniqueJobs
   # ThreadSafe config exists to be able to document the config class without errors
-  ThreadSafeConfig = Concurrent::MutableStruct.new("ThreadSafeConfig",
-                                                   :lock_timeout,
-                                                   :lock_ttl,
-                                                   :enabled,
-                                                   :lock_prefix,
-                                                   :logger,
-                                                   :logger_enabled,
-                                                   :locks,
-                                                   :strategies,
-                                                   :debug_lua,
-                                                   :max_history,
-                                                   :reaper,
-                                                   :reaper_count,
-                                                   :reaper_interval,
-                                                   :reaper_timeout,
-                                                   :reaper_resurrector_interval,
-                                                   :reaper_resurrector_enabled,
-                                                   :lock_info,
-                                                   :raise_on_config_error,
-                                                   :current_redis_version)
+  ThreadSafeConfig = Concurrent::MutableStruct.new(
+    "ThreadSafeConfig",
+    :lock_timeout,
+    :lock_ttl,
+    :enabled,
+    :lock_prefix,
+    :logger,
+    :logger_enabled,
+    :locks,
+    :strategies,
+    :debug_lua,
+    :max_history,
+    :reaper,
+    :reaper_count,
+    :reaper_interval,
+    :reaper_timeout,
+    :reaper_resurrector_interval,
+    :reaper_resurrector_enabled,
+    :lock_info,
+    :raise_on_config_error,
+    :current_redis_version,
+    :digest_algorithm,
+  )
 
   #
   # Shared class for dealing with gem configuration
@@ -118,11 +121,9 @@ class Config < ThreadSafeConfig
     #
     # @return [3600] check if reaper is dead each 3600 seconds
     REAPER_RESURRECTOR_INTERVAL = 3600
-
     #
     # @return [false] enable reaper resurrector
     REAPER_RESURRECTOR_ENABLED = false
-
     #
     # @return [false] while useful it also adds overhead so disable lock_info by default
     USE_LOCK_INFO         = false
@@ -132,6 +133,9 @@ class Config < ThreadSafeConfig
     #
     # @return [0.0.0] default redis version is only to avoid NoMethodError on nil
     REDIS_VERSION         = "0.0.0"
+    #
+    # @return [:legacy] default digest algorithm :modern or :legacy
+    DIGEST_ALGORITHM      = :legacy
 
     #
     # Returns a default configuration
@@ -198,6 +202,7 @@ def self.default # rubocop:disable Metrics/MethodLength
         USE_LOCK_INFO,
         RAISE_ON_CONFIG_ERROR,
         REDIS_VERSION,
+        DIGEST_ALGORITHM,
       )
     end
 
@@ -210,8 +215,8 @@ def self.default # rubocop:disable Metrics/MethodLength
     # @return [<type>] <description>
     #
     def default_lock_ttl=(obj)
-      warn "[DEPRECATION] `#{class_name}##{__method__}` is deprecated." \
-           " Please use `#{class_name}#lock_ttl=` instead."
+      warn "[DEPRECATION] `#{class_name}##{__method__}` is deprecated. " \
+           "Please use `#{class_name}#lock_ttl=` instead."
       self.lock_ttl = obj
     end
 
@@ -224,8 +229,8 @@ def default_lock_ttl=(obj)
     # @return [Integer]
     #
     def default_lock_timeout=(obj)
-      warn "[DEPRECATION] `#{class_name}##{__method__}` is deprecated." \
-           " Please use `#{class_name}#lock_timeout=` instead."
+      warn "[DEPRECATION] `#{class_name}##{__method__}` is deprecated. " \
+           "Please use `#{class_name}#lock_timeout=` instead."
       self.lock_timeout = obj
     end
 
@@ -236,8 +241,8 @@ def default_lock_timeout=(obj)
     # @return [nil, Integer] configured value or nil
     #
     def default_lock_ttl
-      warn "[DEPRECATION] `#{class_name}##{__method__}` is deprecated." \
-           " Please use `#{class_name}#lock_ttl` instead."
+      warn "[DEPRECATION] `#{class_name}##{__method__}` is deprecated. " \
+           "Please use `#{class_name}#lock_ttl` instead."
       lock_ttl
     end
 
@@ -249,8 +254,8 @@ def default_lock_ttl
     # @return [nil, Integer] configured value or nil
     #
     def default_lock_timeout
-      warn "[DEPRECATION] `#{class_name}##{__method__}` is deprecated." \
-           " Please use `#{class_name}#lock_timeout` instead."
+      warn "[DEPRECATION] `#{class_name}##{__method__}` is deprecated. " \
+           "Please use `#{class_name}#lock_timeout` instead."
       lock_timeout
     end
 
@@ -304,6 +309,21 @@ def add_strategy(name, klass)
       self.strategies = new_strategies
     end
 
+    #
+    # Sets digest_algorithm to either :modern or :legacy
+    #
+    # @param [Symbol] value
+    #
+    # @return [Symbol] the new value
+    #
+    def digest_algorithm=(value)
+      unless [:modern, :legacy].include?(value)
+        raise ArgumentError, "Invalid digest algorithm: #{value} (should be :modern or :legacy)"
+      end
+
+      super
+    end
+
     #
     # The current version of redis
     #

diff --git a/lib/sidekiq_unique_jobs/digests.rb b/lib/sidekiq_unique_jobs/digests.rb
@@ -18,7 +18,7 @@ class Digests < Redis::SortedSet
     EMPTY_KEYS_SEGMENT = ["", "", "", ""].freeze
 
     def initialize(digests_key = DIGESTS)
-      super(digests_key)
+      super
     end
 
     #

diff --git a/lib/sidekiq_unique_jobs/exceptions.rb b/lib/sidekiq_unique_jobs/exceptions.rb
@@ -80,8 +80,8 @@ def initialize(options)
 
       super(
         "#{job_class}##{lock_args_method} takes #{num_args} arguments, received #{given.inspect}" \
-        "\n\n" \
-        "   #{source_location.join(':')}"
+        "\n\n   " \
+        "#{source_location.join(':')}"
       )
     end
   end

diff --git a/lib/sidekiq_unique_jobs/lock/base_lock.rb b/lib/sidekiq_unique_jobs/lock/base_lock.rb
@@ -147,7 +147,7 @@ def strategy_for(origin)
           server_strategy
         else
           raise SidekiqUniqueJobs::InvalidArgument,
-                "#origin needs to be either `:server` or `:client`"
+            "#origin needs to be either `:server` or `:client`"
         end
       end
 

diff --git a/lib/sidekiq_unique_jobs/lock/while_executing.rb b/lib/sidekiq_unique_jobs/lock/while_executing.rb
@@ -23,7 +23,7 @@ class WhileExecuting < BaseLock
       # @param [Sidekiq::RedisConnection, ConnectionPool] redis_pool the redis connection
       #
       def initialize(item, callback, redis_pool = nil)
-        super(item, callback, redis_pool)
+        super
         append_unique_key_suffix
       end
 

diff --git a/lib/sidekiq_unique_jobs/lock_args.rb b/lib/sidekiq_unique_jobs/lock_args.rb
@@ -91,9 +91,9 @@ def filter_by_symbol(args)
       job_class.send(lock_args_method, args)
     rescue ArgumentError
       raise SidekiqUniqueJobs::InvalidUniqueArguments,
-            given: args,
-            job_class: job_class,
-            lock_args_method: lock_args_method
+        given: args,
+        job_class: job_class,
+        lock_args_method: lock_args_method
     end
 
     # The method to use for filtering unique arguments

diff --git a/lib/sidekiq_unique_jobs/lock_digest.rb b/lib/sidekiq_unique_jobs/lock_digest.rb
@@ -51,7 +51,12 @@ def lock_digest
     # Creates a namespaced unique digest based on the {#digestable_hash} and the {#lock_prefix}
     # @return [String] a unique digest
     def create_digest
-      digest = OpenSSL::Digest::MD5.hexdigest(dump_json(digestable_hash.sort))
+      digest = if SidekiqUniqueJobs.config.digest_algorithm == :legacy
+        OpenSSL::Digest::MD5.hexdigest(dump_json(digestable_hash.sort))
+      else
+        OpenSSL::Digest.new("SHA3-256", dump_json(digestable_hash.sort)).hexdigest
+      end
+
       "#{lock_prefix}:#{digest}"
     end
 

diff --git a/lib/sidekiq_unique_jobs/locksmith.rb b/lib/sidekiq_unique_jobs/locksmith.rb
@@ -251,10 +251,10 @@ def primed_async(conn, wait = nil, &block) # rubocop:disable Metrics/MethodLengt
       concurrent_timeout = add_drift(timeout)
 
       reflect(:debug, :timeouts, item,
-              timeouts: {
-                brpoplpush_timeout: brpoplpush_timeout,
-                concurrent_timeout: concurrent_timeout,
-              })
+        timeouts: {
+          brpoplpush_timeout: brpoplpush_timeout,
+          concurrent_timeout: concurrent_timeout,
+        })
 
       # NOTE: When debugging, change .value to .value!
       primed_jid = Concurrent::Promises

diff --git a/lib/sidekiq_unique_jobs/logging.rb b/lib/sidekiq_unique_jobs/logging.rb
@@ -223,8 +223,8 @@ def no_sidekiq_context_method
     end
 
     def fake_logger_context(_context)
-      logger.warn "Don't know how to setup the logging context. Please open a feature request:" \
-                  " https://github.com/mhenrixon/sidekiq-unique-jobs/issues/new?template=feature_request.md"
+      logger.warn "Don't know how to setup the logging context. Please open a feature request: " \
+                  "https://github.com/mhenrixon/sidekiq-unique-jobs/issues/new?template=feature_request.md"
 
       yield
     end

diff --git a/lib/sidekiq_unique_jobs/on_conflict.rb b/lib/sidekiq_unique_jobs/on_conflict.rb
@@ -33,8 +33,8 @@ def self.find_strategy(strategy)
 
       strategies.fetch(strategy.to_sym) do
         SidekiqUniqueJobs.logger.warn(
-          "No matching implementation for strategy: #{strategy}, returning OnConflict::NullStrategy." \
-          " Available strategies are (#{strategies.inspect})",
+          "No matching implementation for strategy: #{strategy}, returning OnConflict::NullStrategy. " \
+          "Available strategies are (#{strategies.inspect})",
         )
 
         OnConflict::NullStrategy