Skip to content
This repository has been archived by the owner on Dec 13, 2023. It is now read-only.

Adding Redis db index and username properties #3847

Merged
merged 1 commit into from
Nov 25, 2023
Merged

Adding Redis db index and username properties #3847

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions redis-persistence/src/main/java/com/netflix/conductor/redis/config/RedisProperties.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@ public RedisProperties(ConductorProperties conductorProperties) {

private int numTestsPerEvictionRun = 3;

private int database = 0;

private String username = null;

public int getNumTestsPerEvictionRun() {
return numTestsPerEvictionRun;
}
Expand Down Expand Up @@ -283,4 +287,20 @@ public RetryPolicyFactory getConnectionRetryPolicy() {
return () -> new RetryNTimes(maxRetryAttempts, false);
}
}

public int getDatabase() {
return database;
}

public void setDatabase(int database) {
this.database = database;
}

public String getUsername() {
return username;
}

public void setUsername(String username) {
this.username = username;
}
}
21 changes: 19 additions & 2 deletions ...sistence/src/main/java/com/netflix/conductor/redis/config/RedisSentinelConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,16 +63,33 @@ protected JedisCommands createJedisCommands(
}
// We use the password of the first sentinel host as password and sentinelPassword
String password = getPassword(hostSupplier.getHosts());
if (password != null) {
if (properties.getUsername() != null && password != null) {
Copy link

Choose a reason for hiding this comment

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

Shall we add null + empty check both? I was also thinking if we should also throw error if password is null/empty, basically make password a mandatory field regardless and rather do that check right after line 65. I can't think of a reason why someone would run Redis without password (and if we must support it, one can look at anonymous flag)... Lets see what community says about this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The password is optional for various persistence layers that conductor supports, and is the reason why it is obtained from the Host properties. Redis is one of the persistence options and even with Redis it doesn't mandate the use of password. Null check is what existed earlier we can add empty check in addition if needed.

return new JedisSentinel(
new JedisSentinelPool(
properties.getClusterName(),
sentinels,
genericObjectPoolConfig,
Protocol.DEFAULT_TIMEOUT,
Protocol.DEFAULT_TIMEOUT,
properties.getUsername(),
password,
Protocol.DEFAULT_DATABASE,
properties.getDatabase(),
null,
Protocol.DEFAULT_TIMEOUT,
Protocol.DEFAULT_TIMEOUT,
properties.getUsername(),
password,
null));
} else if (password != null) {
return new JedisSentinel(
new JedisSentinelPool(
properties.getClusterName(),
sentinels,
genericObjectPoolConfig,
Protocol.DEFAULT_TIMEOUT,
Protocol.DEFAULT_TIMEOUT,
password,
properties.getDatabase(),
null,
Protocol.DEFAULT_TIMEOUT,
Protocol.DEFAULT_TIMEOUT,
Expand Down
19 changes: 15 additions & 4 deletions ...stence/src/main/java/com/netflix/conductor/redis/config/RedisStandaloneConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,18 +45,29 @@ protected JedisCommands createJedisCommands(
config.setMaxTotal(properties.getMaxConnectionsPerHost());
log.info("Starting conductor server using redis_standalone.");
Host host = hostSupplier.getHosts().get(0);
return new JedisStandalone(getJedisPool(config, host));
return new JedisStandalone(getJedisPool(config, host, properties));
}

private JedisPool getJedisPool(JedisPoolConfig config, Host host) {
if (host.getPassword() != null) {
private JedisPool getJedisPool(JedisPoolConfig config, Host host, RedisProperties properties) {
if (properties.getUsername() != null && host.getPassword() != null) {
log.info("Connecting to Redis Standalone with AUTH");
return new JedisPool(
config,
host.getHostName(),
host.getPort(),
Protocol.DEFAULT_TIMEOUT,
host.getPassword());
properties.getUsername(),
host.getPassword(),
properties.getDatabase());
} else if (host.getPassword() != null) {
log.info("Connecting to Redis Standalone with AUTH");
return new JedisPool(
config,
host.getHostName(),
host.getPort(),
Protocol.DEFAULT_TIMEOUT,
host.getPassword(),
properties.getDatabase());
} else {
return new JedisPool(config, host.getHostName(), host.getPort());
}
Expand Down
Loading