Skip to content

Implement ability to connect to Sentinel with TLS #2139

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

Talon876
Copy link
Contributor

This change allows connecting to sentinel via TLS. Additionally, you can specify if you want to connect to the redis master with TLS or not.

When connecting to sentinel with TLS, it will still broadcast the non-TLS ports for redis. For this scenario to work, toHostAndPort was modified to be protected so consumers can sub-class JedisSentinelPool and implement your own port mapping behavior.

@gkorland gkorland requested a review from sazzad16 March 22, 2020 12:38
@@ -28,6 +32,12 @@
protected String sentinelPassword;
protected String sentinelClientName;

protected boolean isRedisSslEnabled;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Master would be better that Redis. E.g. isMasterSslEnabled. Also, apply this change entire PR.

Comment on lines 105 to +111
public JedisSentinelPool(String masterName, Set<String> sentinels,
final GenericObjectPoolConfig poolConfig, final int connectionTimeout, final int soTimeout,
final String password, final int database, final String clientName,
final int sentinelConnectionTimeout, final int sentinelSoTimeout, final String sentinelPassword,
final String sentinelClientName) {
final String sentinelClientName, final boolean isRedisSslEnabled, final boolean isSentinelSslEnabled,
final SSLSocketFactory sslSocketFactory, final SSLParameters sslParameters,
final HostnameVerifier hostnameVerifier) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DON'T change an existing constructor or public method. Create a new one.

}

public MasterListener(String masterName, String host, int port,
long subscribeRetryWaitTimeMillis) {
this(masterName, host, port);
long subscribeRetryWaitTimeMillis) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
long subscribeRetryWaitTimeMillis) {
long subscribeRetryWaitTimeMillis) {

@sazzad16 sazzad16 added this to the 3.6.0 milestone Mar 18, 2021
sazzad16 added a commit that referenced this pull request Mar 18, 2021
Thanks @Talon876 for the test instances in the Makefile (#2139)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants