Sentinels: Accept options to connect to master

[jlledom]

I tried to use SentinelClient on my project and found I couldn't connect to Sentinels + TLS or ACL. The reason is there no way to provide connection options for the master, only for the sentinels. So sentinels can be protected with TLS and ACL and async-redis will connect to them, but after resolving master, SentinelClient will only connect to master if it is not protected by TLS nor ACL.

I wrote this script to test the changes:

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  #gem 'async-redis'
  gem 'async-redis', git: 'https://github.com/jlledom/async-redis', branch: 'jlledom-sentinels-tls-acl'
end

def main
  Async do
    sentinels = [{ host: 'localhost', port: 56380 },
                 { host: 'localhost', port: 56381 },
                 { host: 'localhost', port: 56382 }]

    ssl_params = {
      ca_file: '/path/to/ca-root-cert.pem',
    }
    ssl_context = OpenSSL::SSL::SSLContext.new.tap do |context|
      context.set_params(ssl_params)
    end

    database = '6'
    credentials = %w[username password]
    endpoint_options = {database:, credentials:, ssl_context: }.compact

    sentinel_credentials = %w[sentinel_username sentinel_password]
    master_name = 'redis-master'
    role = :master

    endpoints = sentinels.map do |sentinel|
      scheme = ssl_context ? 'rediss' : 'redis'
      host = sentinel[:host]
      port = sentinel[:port]
      sentinel_uri = URI::Generic.build(scheme:, host:, port:)
      Async::Redis::Endpoint.new(sentinel_uri, nil, credentials: sentinel_credentials, ssl_context:)
    end
    client = Async::Redis::SentinelClient.new(endpoints, master_name:, master_options: endpoint_options, role:)

    while true
      begin
        sleep 1
        result = client.ping 'test'
        puts result
      rescue StandardError => e
        puts e.message
        retry
      end
    end
  end
end

main

Types of Changes

• Bug fix.

Contribution

• I added tests for my changes.
• I tested my changes locally.
• I agree to the Developer's Certificate of Origin 1.1.

I didn't add tests because those must involve much more than just writing the tests. It would require creating a new docker compose file, new certificates, config files, etc.

[jlledom]

I removed the @protocol instance variable because, AFAIK, it is only used to connect to the master, and at this point we still don't know which one will be.

[jlledom]

I don't kinda like this, any ideas?

[jlledom]

I'm not sure what is this for. I hope I'm not breaking anything.

[jlledom]

Here we use the endpoint protocol to connect to the endpoint, I think this is more correct and removes the need for the instance variable.

[Copilot]

Pull Request Overview

This PR extends SentinelClient to accept custom connection options (e.g., TLS, ACL credentials) for the resolved master/slave endpoints.

• Updated initializer to accept master_options and infer secure scheme.
• Switched endpoint creation in resolve_master/resolve_slave to Endpoint.for with provided options.
• Removed the old protocol attribute and default tags in favor of per-endpoint protocol handling.

Comments suppressed due to low confidence (2)

lib/async/redis/sentinel_client.rb:24

• Add a note in the class or initializer docs explaining how master_options interact with TLS/ACL and the :role setting, so users understand its intended use.

			# @property master_options [Hash] Connection options for master.

lib/async/redis/sentinel_client.rb:24

• There are no tests covering the new master_options functionality (e.g., TLS or credential use when connecting to the master). Consider adding unit or integration tests to validate this behavior.

			# @property master_options [Hash] Connection options for master.

[ioquatix]

I will try to review this next week.

[ioquatix]

Will merge in #65.

[ioquatix]

Let me know how you get on with the latest release.