Revert "Revert "Revert "Revert "[core] Support encrypted redis connec…

…tion." (#28991)""" (#29109) Reverts #29078 There are two issues: tsan fail and mac build fail. For TSAN, if we build redis + openssl with tsan the redis will segfault. So we exclude the redis target from the tsan build. For mac failure, the root cause is that in the CI somehow the toolchain picked in rules_foreign_cc is wrong. Failed to find any useful solution within a day. So just use the old build for mac and pick in the runtime. Since redis is only used for testing, win/mac/linux both support rediss.
ray-project · Oct 7, 2022 · 2b5f041 · 2b5f041
1 parent 366f2a9
commit 2b5f041
Show file tree

Hide file tree

Showing 15 changed files with 318 additions and 54 deletions.
diff --git a/.bazelrc b/.bazelrc
@@ -89,9 +89,9 @@ build:llvm --action_env=BAZEL_LINKOPTS=-lm:-pthread
 build:llvm --define force_libcpp=enabled
 
 # Thread sanitizer configuration:
+build:tsan --per_file_copt="-bazel-ray/external/com_github_antirez_redis/.*$@-fsanitize=thread"
+build:tsan --per_file_copt="-bazel-ray/external/com_github_antirez_redis/.*$@-DTHREAD_SANITIZER"
 build:tsan --strip=never
-build:tsan --copt -fsanitize=thread
-build:tsan --copt -DTHREAD_SANITIZER
 build:tsan --copt -O2
 build:tsan --copt -g
 build:tsan --copt -fno-omit-frame-pointer

diff --git a/BUILD.bazel b/BUILD.bazel
@@ -2708,6 +2708,7 @@ alias(
     name = "redis-server",
     actual = select({
         "@bazel_tools//src/conditions:windows": "@com_github_tporadowski_redis_bin//:redis-server.exe",
+        "@bazel_tools//src/conditions:darwin": "@com_github_antirez_redis//:redis-server-mac",
         "//conditions:default": "@com_github_antirez_redis//:redis-server",
     }),
     visibility = ["//visibility:public"],
@@ -2717,6 +2718,7 @@ alias(
     name = "redis-cli",
     actual = select({
         "@bazel_tools//src/conditions:windows": "@com_github_tporadowski_redis_bin//:redis-cli.exe",
+        "@bazel_tools//src/conditions:darwin": "@com_github_antirez_redis//:redis-cli-mac",
         "//conditions:default": "@com_github_antirez_redis//:redis-cli",
     }),
     visibility = ["//visibility:public"],

diff --git a/bazel/BUILD.hiredis b/bazel/BUILD.hiredis
@@ -1,6 +1,8 @@
-COPTS = [] + select({
+COPTS = ["-DUSE_SSL=1"] + select({
     "@bazel_tools//src/conditions:windows": [
         "-D_CRT_DECLARE_NONSTDC_NAMES=0",  # don't define off_t, to avoid conflicts
+        "-DWIN32",
+        "-DWIN32_LEAN_AND_MEAN"
     ],
     "//conditions:default": [
     ],
@@ -32,7 +34,6 @@ cc_library(
         ],
         exclude =
         [
-            "ssl.c",
             "test.c",
         ],
     ),
@@ -44,6 +45,8 @@ cc_library(
     include_prefix = "hiredis",
     deps = [
         ":_hiredis",
+        "@boringssl//:ssl",
+        "@boringssl//:crypto"
     ],
     visibility = ["//visibility:public"],
 )
diff --git a/bazel/BUILD.redis b/bazel/BUILD.redis
@@ -1,3 +1,5 @@
+load("@rules_foreign_cc//foreign_cc:defs.bzl", "make")
+
 exports_files(
     [
         "redis-server.exe",
@@ -6,13 +8,67 @@ exports_files(
     visibility = ["//visibility:public"],
 )
 
+filegroup(
+    name = "all_srcs",
+    srcs = glob(
+        include = ["**"],
+        exclude = ["*.bazel"],
+    ),
+)
+
+
+make(
+    name = "redis",
+    args = [
+        "BUILD_TLS=yes",
+        "-s",
+    ],
+    copts = [
+        "-DLUA_USE_MKSTEMP",
+        "-Wno-pragmas",
+        "-Wno-empty-body",
+        "-fPIC",
+    ],
+    visibility = ["//visibility:public"],
+    lib_source = ":all_srcs",
+    deps = [
+        "@openssl//:openssl",
+    ],
+    out_binaries = [
+        "redis-server",
+        "redis-cli"
+    ]
+)
+
 genrule(
     name = "bin",
-    srcs = glob(["**"]),
+    srcs = [":redis"],
     outs = [
         "redis-server",
         "redis-cli",
     ],
+    cmd = """
+    mkdir tmp-redis-bin
+    cp -rf $(locations :redis) ./tmp-redis-bin/
+    cp tmp-redis-bin/redis-server $(location redis-server)
+    chmod +x $(location redis-server)
+    cp tmp-redis-bin/redis-cli $(location redis-cli)
+    chmod +x $(location redis-cli)
+    rm -rf tmp-redis-bin
+    """,
+    visibility = ["//visibility:public"],
+    tags = ["local"],
+)
+
+# This is a build for mac without openssl.
+# rules_foreign_cc somehow pick the wrong toolchain in CI.
+genrule(
+    name = "bin-mac",
+    srcs = glob(["**"]),
+    outs = [
+        "redis-server-mac",
+        "redis-cli-mac",
+    ],
     cmd = """
         unset CC LDFLAGS CXX CXXFLAGS
         tmpdir="redis.tmp"
@@ -21,47 +77,13 @@ genrule(
         chmod +x "$${tmpdir}"/deps/jemalloc/configure
         parallel="$$(getconf _NPROCESSORS_ONLN || echo 1)"
         make -s -C "$${tmpdir}" -j"$${parallel}" V=0 CFLAGS="$${CFLAGS-} -DLUA_USE_MKSTEMP -Wno-pragmas -Wno-empty-body"
-        mv "$${tmpdir}"/src/redis-server $(location redis-server)
-        chmod +x $(location redis-server)
-        mv "$${tmpdir}"/src/redis-cli $(location redis-cli)
-        chmod +x $(location redis-cli)
+        mv "$${tmpdir}"/src/redis-server $(location redis-server-mac)
+        chmod +x $(location redis-server-mac)
+        mv "$${tmpdir}"/src/redis-cli $(location redis-cli-mac)
+        chmod +x $(location redis-cli-mac)
         rm -r -f -- "$${tmpdir}"
     """,
     visibility = ["//visibility:public"],
     tags = ["local"],
 )
 
-# This library is for internal hiredis use, because hiredis assumes a
-# different include prefix for itself than external libraries do.
-cc_library(
-    name = "_hiredis",
-    hdrs = [
-        "deps/hiredis/dict.c",
-        "deps/hiredis/dict.h",
-        "deps/hiredis/fmacros.h",
-    ],
-    strip_include_prefix = "deps/hiredis",
-)
-
-cc_library(
-    name = "hiredis",
-    srcs = glob(
-        [
-            "deps/hiredis/*.c",
-            "deps/hiredis/*.h",
-        ],
-        exclude =
-        [
-            "deps/hiredis/test.c",
-        ],
-    ),
-    hdrs = glob([
-        "deps/hiredis/*.h",
-        "deps/hiredis/adapters/*.h",
-    ]),
-    strip_include_prefix = "deps",
-    deps = [
-        ":_hiredis",
-    ],
-    visibility = ["//visibility:public"],
-)
diff --git a/bazel/ray_deps_build_all.bzl b/bazel/ray_deps_build_all.bzl
@@ -6,6 +6,9 @@ load("@com_github_grpc_grpc//third_party/py:python_configure.bzl", "python_confi
 load("@com_github_grpc_grpc//bazel:grpc_deps.bzl", "grpc_deps")
 load("@rules_proto_grpc//:repositories.bzl", "rules_proto_grpc_toolchains")
 load("@com_github_johnynek_bazel_jar_jar//:jar_jar.bzl", "jar_jar_repositories")
+load("@rules_foreign_cc//foreign_cc:repositories.bzl", "rules_foreign_cc_dependencies")
+load("@rules_foreign_cc_thirdparty//openssl:openssl_setup.bzl", "openssl_setup")
+
 
 
 def ray_deps_build_all():
@@ -17,3 +20,5 @@ def ray_deps_build_all():
   grpc_deps()
   rules_proto_grpc_toolchains()
   jar_jar_repositories()
+  rules_foreign_cc_dependencies()
+  openssl_setup()
diff --git a/bazel/ray_deps_setup.bzl b/bazel/ray_deps_setup.bzl
@@ -98,11 +98,13 @@ def ray_deps_setup():
     auto_http_archive(
         name = "com_github_antirez_redis",
         build_file = "@com_github_ray_project_ray//bazel:BUILD.redis",
+        patch_args = ["-p1"],
         url = "https://github.com/redis/redis/archive/refs/tags/7.0.5.tar.gz",
         sha256 = "40827fcaf188456ad9b3be8e27a4f403c43672b6bb6201192dc15756af6f1eae",
         patches = [
             "@com_github_ray_project_ray//thirdparty/patches:redis-quiet.patch",
         ],
+        workspace_file_content = 'workspace(name = "com_github_antirez_redis")'
     )
 
     auto_http_archive(
@@ -243,6 +245,36 @@ def ray_deps_setup():
             "@com_github_ray_project_ray//thirdparty/patches:grpc-python.patch",
         ],
     )
+
+    http_archive(
+        name = "openssl",
+        strip_prefix = "openssl-1.1.1f",
+        sha256 = "186c6bfe6ecfba7a5b48c47f8a1673d0f3b0e5ba2e25602dd23b629975da3f35",
+        urls = [
+            "https://www.openssl.org/source/openssl-1.1.1f.tar.gz",
+        ],
+        build_file = "@rules_foreign_cc_thirdparty//openssl:BUILD.openssl.bazel",
+    )
+
+    http_archive(
+        name = "rules_foreign_cc",
+        sha256 = "2a4d07cd64b0719b39a7c12218a3e507672b82a97b98c6a89d38565894cf7c51",
+        strip_prefix = "rules_foreign_cc-0.9.0",
+        url = "https://github.com/bazelbuild/rules_foreign_cc/archive/refs/tags/0.9.0.tar.gz",
+    )
+
+    git_repository(
+        name = "rules_perl",
+        remote = "https://github.com/bazelbuild/rules_perl.git",
+        commit = "022b8daf2bb4836ac7a50e4a1d8ea056a3e1e403",
+    )
+
+    http_archive(
+        name = "rules_foreign_cc_thirdparty",
+        sha256 = "2a4d07cd64b0719b39a7c12218a3e507672b82a97b98c6a89d38565894cf7c51",
+        strip_prefix = "rules_foreign_cc-0.9.0/examples/third_party",
+        url = "https://github.com/bazelbuild/rules_foreign_cc/archive/refs/tags/0.9.0.tar.gz",
+    )
 
     http_archive(
         # This rule is used by @com_github_grpc_grpc, and using a GitHub mirror

diff --git a/python/ray/_private/services.py b/python/ray/_private/services.py
@@ -25,7 +25,7 @@
 import ray
 import ray._private.ray_constants as ray_constants
 from ray._private.gcs_utils import GcsClient
-from ray._raylet import GcsClientOptions
+from ray._raylet import GcsClientOptions, Config
 from ray.core.generated.common_pb2 import Language
 
 resource = None
@@ -1107,11 +1107,36 @@ def _start_redis_instance(
         if " " in password:
             raise ValueError("Spaces not permitted in redis password.")
         command += ["--requirepass", password]
-    command += ["--port", str(port), "--loglevel", "warning"]
+
+    if not Config.REDIS_ENABLE_SSL():
+        command += ["--port", str(port), "--loglevel", "warning"]
+    else:
+        import socket
+
+        with socket.socket() as s:
+            s.bind(("", 0))
+            free_port = s.getsockname()[1]
+        command += [
+            "--tls-port",
+            str(port),
+            "--loglevel",
+            "warning",
+            "--port",
+            str(free_port),
+        ]
+
     if listen_to_localhost_only:
         command += ["--bind", "127.0.0.1"]
     pidfile = os.path.join(session_dir_path, "redis-" + uuid.uuid4().hex + ".pid")
     command += ["--pidfile", pidfile]
+    if Config.REDIS_ENABLE_SSL():
+        if Config.REDIS_CA_CERT():
+            command += ["--tls-ca-cert-file", Config.REDIS_CA_CERT()]
+        if Config.REDIS_CLIENT_CERT():
+            command += ["--tls-cert-file", Config.REDIS_CLIENT_CERT()]
+        if Config.REDIS_CLIENT_KEY():
+            command += ["--tls-key-file", Config.REDIS_CLIENT_KEY()]
+        command += ["--tls-replication", "yes"]
     if sys.platform != "win32":
         command += ["--save", "", "--appendonly", "no"]
     process_info = start_ray_process(

diff --git a/python/ray/includes/ray_config.pxd b/python/ray/includes/ray_config.pxd
@@ -70,3 +70,15 @@ cdef extern from "ray/common/ray_config.h" nogil:
         c_bool start_python_importer_thread() const
 
         c_bool use_ray_syncer() const
+
+        c_bool REDIS_ENABLE_SSL() const
+
+        c_string REDIS_CA_CERT() const
+
+        c_string REDIS_CA_PATH() const
+
+        c_string REDIS_CLIENT_CERT() const
+
+        c_string REDIS_CLIENT_KEY() const
+
+        c_string REDIS_SERVER_NAME() const
diff --git a/python/ray/includes/ray_config.pxi b/python/ray/includes/ray_config.pxi
@@ -116,3 +116,27 @@ cdef class Config:
     @staticmethod
     def use_ray_syncer():
         return RayConfig.instance().use_ray_syncer()
+
+    @staticmethod
+    def REDIS_ENABLE_SSL():
+        return RayConfig.instance().REDIS_ENABLE_SSL()
+
+    @staticmethod
+    def REDIS_CA_CERT():
+        return RayConfig.instance().REDIS_CA_CERT()
+
+    @staticmethod
+    def REDIS_CA_PATH():
+        return RayConfig.instance().REDIS_CA_PATH()
+
+    @staticmethod
+    def REDIS_CLIENT_CERT():
+        return RayConfig.instance().REDIS_CLIENT_CERT()
+
+    @staticmethod
+    def REDIS_CLIENT_KEY():
+        return RayConfig.instance().REDIS_CLIENT_KEY()
+
+    @staticmethod
+    def REDIS_SERVER_NAME():
+        return RayConfig.instance().REDIS_SERVER_NAME()
diff --git a/python/ray/tests/BUILD b/python/ray/tests/BUILD
@@ -187,6 +187,7 @@ py_test_module_list(
     "test_raylet_output.py",
     "test_scheduling_performance.py",
     "test_get_or_create_actor.py",
+    "test_redis_tls.py",
   ],
   size = "small",
   tags = ["exclusive", "small_size_python_tests", "team:core"],