From 429db383774f208a18092dcc1544adb8daef0fba Mon Sep 17 00:00:00 2001 From: Danny Chiao Date: Thu, 17 Feb 2022 22:02:00 -0500 Subject: [PATCH 1/7] Enabling password authentication and SSL in Redis clusters for the Java feature server Signed-off-by: Danny Chiao --- .gitignore | 6 ++++ .../serving/config/ApplicationProperties.java | 29 +++++++++++++++++-- .../redis/retriever/RedisClusterClient.java | 10 ++++++- .../retriever/RedisClusterStoreConfig.java | 15 +++++++++- 4 files changed, 56 insertions(+), 4 deletions(-) diff --git a/.gitignore b/.gitignore index 90b1a66a8c4..e4becb3c311 100644 --- a/.gitignore +++ b/.gitignore @@ -191,3 +191,9 @@ sdk/go/protos/ #benchmarks .benchmarks + +# Examples registry +**/registry.db +**/*.aof +**/*.rdb +**/nodes.conf \ No newline at end of file diff --git a/java/serving/src/main/java/feast/serving/config/ApplicationProperties.java b/java/serving/src/main/java/feast/serving/config/ApplicationProperties.java index 791c871e59b..a39b94417e7 100644 --- a/java/serving/src/main/java/feast/serving/config/ApplicationProperties.java +++ b/java/serving/src/main/java/feast/serving/config/ApplicationProperties.java @@ -27,6 +27,8 @@ import feast.storage.connectors.redis.retriever.RedisClusterStoreConfig; import feast.storage.connectors.redis.retriever.RedisStoreConfig; import io.lettuce.core.ReadFrom; +import org.slf4j.Logger; + import java.time.Duration; import java.util.*; import javax.annotation.PostConstruct; @@ -36,6 +38,9 @@ /** Feast Serving properties. */ public class ApplicationProperties { + private static final Logger log = + org.slf4j.LoggerFactory.getLogger(ApplicationProperties.class); + public static class FeastProperties { /* Feast Serving build version */ @NotBlank private String version = "unknown"; @@ -246,10 +251,30 @@ public void setType(String type) { * @return Returns the store specific configuration */ public RedisClusterStoreConfig getRedisClusterConfig() { + String read_from = ReadFrom.UPSTREAM.toString(); + if (!this.config.containsKey("read_from") || this.config.get("read_from") == null) { + log.info("'read_from' not defined in Redis cluster config, so setting to UPSTREAM"); + } else { + read_from = this.config.get("read_from"); + } + + if (!this.config.containsKey("timeout") || this.config.get("timeout") == null) { + throw new RuntimeException("Redis cluster config does not have 'timeout' specified"); + } + + boolean ssl = false; + if (!this.config.containsKey("ssl") || this.config.get("ssl") == null) { + log.info("'ssl' not defined in Redis cluster config, so setting to false"); + } else { + ssl = Boolean.parseBoolean(this.config.getOrDefault("ssl", "false")); + } + Duration timeout = Duration.parse(this.config.get("timeout")); return new RedisClusterStoreConfig( this.config.get("connection_string"), - ReadFrom.valueOf(this.config.get("read_from")), - Duration.parse(this.config.get("timeout"))); + ReadFrom.valueOf(read_from), + timeout, + ssl, + this.config.getOrDefault("password", "")); } public RedisStoreConfig getRedisConfig() { diff --git a/java/storage/connectors/redis/src/main/java/feast/storage/connectors/redis/retriever/RedisClusterClient.java b/java/storage/connectors/redis/src/main/java/feast/storage/connectors/redis/retriever/RedisClusterClient.java index 6574962bbb5..bffc3aa6f57 100644 --- a/java/storage/connectors/redis/src/main/java/feast/storage/connectors/redis/retriever/RedisClusterClient.java +++ b/java/storage/connectors/redis/src/main/java/feast/storage/connectors/redis/retriever/RedisClusterClient.java @@ -71,7 +71,15 @@ public static RedisClientAdapter create(RedisClusterStoreConfig config) { .map( hostPort -> { String[] hostPortSplit = hostPort.trim().split(":"); - return RedisURI.create(hostPortSplit[0], Integer.parseInt(hostPortSplit[1])); + RedisURI redisURI = + RedisURI.create(hostPortSplit[0], Integer.parseInt(hostPortSplit[1])); + if (!config.getPassword().isEmpty()) { + redisURI.setPassword(config.getPassword()); + } + if (config.getSsl()) { + redisURI.setSsl(true); + } + return redisURI; }) .collect(Collectors.toList()); diff --git a/java/storage/connectors/redis/src/main/java/feast/storage/connectors/redis/retriever/RedisClusterStoreConfig.java b/java/storage/connectors/redis/src/main/java/feast/storage/connectors/redis/retriever/RedisClusterStoreConfig.java index c179ffe9645..271b07759c0 100644 --- a/java/storage/connectors/redis/src/main/java/feast/storage/connectors/redis/retriever/RedisClusterStoreConfig.java +++ b/java/storage/connectors/redis/src/main/java/feast/storage/connectors/redis/retriever/RedisClusterStoreConfig.java @@ -23,11 +23,16 @@ public class RedisClusterStoreConfig { private final String connectionString; private final ReadFrom readFrom; private final Duration timeout; + private final Boolean ssl; + private final String password; - public RedisClusterStoreConfig(String connectionString, ReadFrom readFrom, Duration timeout) { + public RedisClusterStoreConfig( + String connectionString, ReadFrom readFrom, Duration timeout, Boolean ssl, String password) { this.connectionString = connectionString; this.readFrom = readFrom; this.timeout = timeout; + this.ssl = ssl; + this.password = password; } public String getConnectionString() { @@ -41,4 +46,12 @@ public ReadFrom getReadFrom() { public Duration getTimeout() { return this.timeout; } + + public Boolean getSsl() { + return ssl; + } + + public String getPassword() { + return password; + } } From f1506460c11b137340496fd4815464b209d3953f Mon Sep 17 00:00:00 2001 From: Danny Chiao Date: Thu, 17 Feb 2022 22:06:33 -0500 Subject: [PATCH 2/7] lint Signed-off-by: Danny Chiao --- .../feast/serving/config/ApplicationProperties.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/java/serving/src/main/java/feast/serving/config/ApplicationProperties.java b/java/serving/src/main/java/feast/serving/config/ApplicationProperties.java index a39b94417e7..b2a010d65fc 100644 --- a/java/serving/src/main/java/feast/serving/config/ApplicationProperties.java +++ b/java/serving/src/main/java/feast/serving/config/ApplicationProperties.java @@ -27,19 +27,17 @@ import feast.storage.connectors.redis.retriever.RedisClusterStoreConfig; import feast.storage.connectors.redis.retriever.RedisStoreConfig; import io.lettuce.core.ReadFrom; -import org.slf4j.Logger; - import java.time.Duration; import java.util.*; import javax.annotation.PostConstruct; import javax.validation.*; import javax.validation.constraints.NotBlank; import javax.validation.constraints.NotNull; +import org.slf4j.Logger; /** Feast Serving properties. */ public class ApplicationProperties { - private static final Logger log = - org.slf4j.LoggerFactory.getLogger(ApplicationProperties.class); + private static final Logger log = org.slf4j.LoggerFactory.getLogger(ApplicationProperties.class); public static class FeastProperties { /* Feast Serving build version */ @@ -272,8 +270,8 @@ public RedisClusterStoreConfig getRedisClusterConfig() { return new RedisClusterStoreConfig( this.config.get("connection_string"), ReadFrom.valueOf(read_from), - timeout, - ssl, + timeout, + ssl, this.config.getOrDefault("password", "")); } From 08dde62f8d4b51b2828932d1c238090d34fd84bd Mon Sep 17 00:00:00 2001 From: Danny Chiao Date: Thu, 17 Feb 2022 22:12:47 -0500 Subject: [PATCH 3/7] add test Signed-off-by: Danny Chiao --- java/serving/src/test/java/feast/serving/it/TestUtils.java | 3 ++- .../test/resources/docker-compose/docker-compose-redis-it.yml | 1 + .../test/resources/docker-compose/feast10/feature_store.yaml | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/java/serving/src/test/java/feast/serving/it/TestUtils.java b/java/serving/src/test/java/feast/serving/it/TestUtils.java index 867fa4afb06..9bca14db4e6 100644 --- a/java/serving/src/test/java/feast/serving/it/TestUtils.java +++ b/java/serving/src/test/java/feast/serving/it/TestUtils.java @@ -83,7 +83,8 @@ public static ApplicationProperties.FeastProperties createBasicFeastProperties( new ApplicationProperties.Store( "online", "REDIS", - ImmutableMap.of("host", redisHost, "port", redisPort.toString())))); + ImmutableMap.of( + "host", redisHost, "port", redisPort.toString(), "password", "testpw")))); return feastProperties; } diff --git a/java/serving/src/test/resources/docker-compose/docker-compose-redis-it.yml b/java/serving/src/test/resources/docker-compose/docker-compose-redis-it.yml index 13835e07d41..1dee243cb80 100644 --- a/java/serving/src/test/resources/docker-compose/docker-compose-redis-it.yml +++ b/java/serving/src/test/resources/docker-compose/docker-compose-redis-it.yml @@ -3,6 +3,7 @@ version: '3' services: redis: image: redis:6.2 + command: redis-server --requirepass testpw ports: - "6379:6379" feast: diff --git a/java/serving/src/test/resources/docker-compose/feast10/feature_store.yaml b/java/serving/src/test/resources/docker-compose/feast10/feature_store.yaml index 7554725004f..2e6625c025b 100644 --- a/java/serving/src/test/resources/docker-compose/feast10/feature_store.yaml +++ b/java/serving/src/test/resources/docker-compose/feast10/feature_store.yaml @@ -3,7 +3,7 @@ registry: registry.db provider: local online_store: type: redis - connection_string: "redis:6379" + connection_string: "redis:6379,password=testpw" offline_store: {} flags: alpha_features: true From 01afe5b062092b6d750feef621f2ffae9245948a Mon Sep 17 00:00:00 2001 From: Danny Chiao Date: Fri, 18 Feb 2022 16:59:38 -0500 Subject: [PATCH 4/7] Address comments Signed-off-by: Danny Chiao --- .../main/java/feast/serving/config/ApplicationProperties.java | 3 ++- .../storage/connectors/redis/retriever/RedisClusterClient.java | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/java/serving/src/main/java/feast/serving/config/ApplicationProperties.java b/java/serving/src/main/java/feast/serving/config/ApplicationProperties.java index b2a010d65fc..6c5499c5218 100644 --- a/java/serving/src/main/java/feast/serving/config/ApplicationProperties.java +++ b/java/serving/src/main/java/feast/serving/config/ApplicationProperties.java @@ -257,7 +257,8 @@ public RedisClusterStoreConfig getRedisClusterConfig() { } if (!this.config.containsKey("timeout") || this.config.get("timeout") == null) { - throw new RuntimeException("Redis cluster config does not have 'timeout' specified"); + throw new IllegalArgumentException( + "Redis cluster config does not have 'timeout' specified"); } boolean ssl = false; diff --git a/java/storage/connectors/redis/src/main/java/feast/storage/connectors/redis/retriever/RedisClusterClient.java b/java/storage/connectors/redis/src/main/java/feast/storage/connectors/redis/retriever/RedisClusterClient.java index bffc3aa6f57..d527f245aef 100644 --- a/java/storage/connectors/redis/src/main/java/feast/storage/connectors/redis/retriever/RedisClusterClient.java +++ b/java/storage/connectors/redis/src/main/java/feast/storage/connectors/redis/retriever/RedisClusterClient.java @@ -79,6 +79,7 @@ public static RedisClientAdapter create(RedisClusterStoreConfig config) { if (config.getSsl()) { redisURI.setSsl(true); } + redisURI.setTimeout(config.getTimeout()); return redisURI; }) .collect(Collectors.toList()); From 9465882f37c619d0804a91d61745f28d34896f80 Mon Sep 17 00:00:00 2001 From: Danny Chiao Date: Fri, 18 Feb 2022 17:00:47 -0500 Subject: [PATCH 5/7] Address comments Signed-off-by: Danny Chiao --- .../main/java/feast/serving/config/ApplicationProperties.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/serving/src/main/java/feast/serving/config/ApplicationProperties.java b/java/serving/src/main/java/feast/serving/config/ApplicationProperties.java index 6c5499c5218..7fb5d32f794 100644 --- a/java/serving/src/main/java/feast/serving/config/ApplicationProperties.java +++ b/java/serving/src/main/java/feast/serving/config/ApplicationProperties.java @@ -265,7 +265,7 @@ public RedisClusterStoreConfig getRedisClusterConfig() { if (!this.config.containsKey("ssl") || this.config.get("ssl") == null) { log.info("'ssl' not defined in Redis cluster config, so setting to false"); } else { - ssl = Boolean.parseBoolean(this.config.getOrDefault("ssl", "false")); + ssl = Boolean.parseBoolean(this.config.get("ssl")); } Duration timeout = Duration.parse(this.config.get("timeout")); return new RedisClusterStoreConfig( From 9b0f4701777d779bec8de6c6d7c7b6e0393577f9 Mon Sep 17 00:00:00 2001 From: Danny Chiao Date: Wed, 23 Feb 2022 10:19:13 -0500 Subject: [PATCH 6/7] lint Signed-off-by: Danny Chiao --- .../java/feast/serving/config/ApplicationProperties.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/java/serving/src/main/java/feast/serving/config/ApplicationProperties.java b/java/serving/src/main/java/feast/serving/config/ApplicationProperties.java index 7fb5d32f794..dbd4249121b 100644 --- a/java/serving/src/main/java/feast/serving/config/ApplicationProperties.java +++ b/java/serving/src/main/java/feast/serving/config/ApplicationProperties.java @@ -249,9 +249,10 @@ public void setType(String type) { * @return Returns the store specific configuration */ public RedisClusterStoreConfig getRedisClusterConfig() { - String read_from = ReadFrom.UPSTREAM.toString(); + String read_from; if (!this.config.containsKey("read_from") || this.config.get("read_from") == null) { log.info("'read_from' not defined in Redis cluster config, so setting to UPSTREAM"); + read_from = ReadFrom.UPSTREAM.toString(); } else { read_from = this.config.get("read_from"); } @@ -261,9 +262,10 @@ public RedisClusterStoreConfig getRedisClusterConfig() { "Redis cluster config does not have 'timeout' specified"); } - boolean ssl = false; + Boolean ssl = null; if (!this.config.containsKey("ssl") || this.config.get("ssl") == null) { log.info("'ssl' not defined in Redis cluster config, so setting to false"); + ssl = false; } else { ssl = Boolean.parseBoolean(this.config.get("ssl")); } From 83182f1312a0da78531293de8835df7afaa137f6 Mon Sep 17 00:00:00 2001 From: Danny Chiao Date: Wed, 23 Feb 2022 10:19:52 -0500 Subject: [PATCH 7/7] lint Signed-off-by: Danny Chiao --- .../main/java/feast/serving/config/ApplicationProperties.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/serving/src/main/java/feast/serving/config/ApplicationProperties.java b/java/serving/src/main/java/feast/serving/config/ApplicationProperties.java index dbd4249121b..268592d20ac 100644 --- a/java/serving/src/main/java/feast/serving/config/ApplicationProperties.java +++ b/java/serving/src/main/java/feast/serving/config/ApplicationProperties.java @@ -252,7 +252,7 @@ public RedisClusterStoreConfig getRedisClusterConfig() { String read_from; if (!this.config.containsKey("read_from") || this.config.get("read_from") == null) { log.info("'read_from' not defined in Redis cluster config, so setting to UPSTREAM"); - read_from = ReadFrom.UPSTREAM.toString(); + read_from = ReadFrom.UPSTREAM.toString(); } else { read_from = this.config.get("read_from"); }