Merge pull request #8458 from JimMoen/fix-hocon-required-fields

fix hocon required fields
2022-07-13 11:24:17 +08:00 · 2022-07-13 11:24:17 +08:00 · 6b2b0e5e0d
parent 8908e93bc3 24f24361da
commit 6b2b0e5e0d
11 changed files with 73 additions and 59 deletions
--- a/.ci/docker-compose-file/Makefile.local
+++ b/.ci/docker-compose-file/Makefile.local
@ -26,6 +26,8 @@ up:
 		-f .ci/docker-compose-file/docker-compose-pgsql-tls.yaml \
 		-f .ci/docker-compose-file/docker-compose-redis-single-tcp.yaml \
 		-f .ci/docker-compose-file/docker-compose-redis-single-tls.yaml \
+		-f .ci/docker-compose-file/docker-compose-redis-sentinel-tcp.yaml \
+		-f .ci/docker-compose-file/docker-compose-redis-sentinel-tls.yaml \
 		up -d --build

 down:
@ -39,6 +41,8 @@ down:
 		-f .ci/docker-compose-file/docker-compose-pgsql-tls.yaml \
 		-f .ci/docker-compose-file/docker-compose-redis-single-tcp.yaml \
 		-f .ci/docker-compose-file/docker-compose-redis-single-tls.yaml \
+		-f .ci/docker-compose-file/docker-compose-redis-sentinel-tcp.yaml \
+		-f .ci/docker-compose-file/docker-compose-redis-sentinel-tls.yaml \
 		down

 ct:
--- a/.ci/docker-compose-file/docker-compose-redis-sentinel-tcp.yaml
+++ b/.ci/docker-compose-file/docker-compose-redis-sentinel-tcp.yaml
@ -1,8 +1,8 @@
 version: '3.9'

 services:
-  redis_server:
-    container_name: redis
+  redis_sentinel_server:
+    container_name: redis-sentinel
    image: redis:${REDIS_TAG}
    volumes:
      - ./redis/:/data/conf
--- a/.ci/docker-compose-file/docker-compose-redis-sentinel-tls.yaml
+++ b/.ci/docker-compose-file/docker-compose-redis-sentinel-tls.yaml
@ -1,8 +1,8 @@
 version: '3.9'

 services:
-  redis_server:
-    container_name: redis
+  redis_sentinel_server_tls:
+    container_name: redis-sentinel-tls
    image: redis:${REDIS_TAG}
    volumes:
      - ../../apps/emqx/etc/certs/cacert.pem:/etc/certs/ca.crt
--- a/.ci/docker-compose-file/docker-compose-redis-single-tcp.yaml
+++ b/.ci/docker-compose-file/docker-compose-redis-single-tcp.yaml
@ -2,7 +2,7 @@ version: '3.9'

 services:
  redis_server:
-    container_name: redis 
+    container_name: redis
    image: redis:${REDIS_TAG}
    ports:
      - "6379:6379"
--- a/.ci/docker-compose-file/redis/redis.sh
+++ b/.ci/docker-compose-file/redis/redis.sh
@ -16,11 +16,15 @@ case $key in
    shift # past argument
    shift # past value
    ;;
-    -t|--tls-enabled)
+    -t)
    tls="$2"
    shift # past argument
    shift # past value
    ;;
+    --tls-enabled)
+    tls=1
+    shift # past argument
+    ;;
    *)
    shift # past argument
    ;;
--- a/.github/workflows/run_test_cases.yaml
+++ b/.github/workflows/run_test_cases.yaml
@ -118,6 +118,8 @@ jobs:
                -f .ci/docker-compose-file/docker-compose-pgsql-tls.yaml \
                -f .ci/docker-compose-file/docker-compose-redis-single-tcp.yaml \
                -f .ci/docker-compose-file/docker-compose-redis-single-tls.yaml \
+                -f .ci/docker-compose-file/docker-compose-redis-sentinel-tcp.yaml \
+                -f .ci/docker-compose-file/docker-compose-redis-sentinel-tls.yaml \
                -f .ci/docker-compose-file/docker-compose.yaml \
                up -d --build

--- a/CHANGES-5.0.md
+++ b/CHANGES-5.0.md
@ -11,6 +11,8 @@
 * The rule engine's jq function now works even when the path to the EMQX install dir contains spaces [jq#35](https://github.com/emqx/jq/pull/35) [#8455](https://github.com/emqx/emqx/pull/8455)
 * Avoid applying any ACL checks on superusers [#8452](https://github.com/emqx/emqx/pull/8452)
 * Fix statistics related system topic name error
+* Fix AuthN JWKS SSL schema. Using schema in `emqx_schema`. [#8458](https://github.com/emqx/emqx/pull/8458)
+* `sentinel` field should be required when AuthN/AuthZ Redis using sentinel mode. [#8458](https://github.com/emqx/emqx/pull/8458)

 # 5.0.3

--- a/apps/emqx/test/emqx_common_test_helpers.erl
+++ b/apps/emqx/test/emqx_common_test_helpers.erl
@ -44,6 +44,7 @@
    client_ssl_twoway/1,
    ensure_mnesia_stopped/0,
    ensure_quic_listener/2,
+    is_all_tcp_servers_available/1,
    is_tcp_server_available/2,
    is_tcp_server_available/3,
    load_config/2,
@ -432,6 +433,18 @@ load_config(SchemaModule, Config, Opts) ->
 load_config(SchemaModule, Config) ->
    load_config(SchemaModule, Config, #{raw_with_default => false}).

+-spec is_all_tcp_servers_available(Servers) -> Result when
+    Servers :: [{Host, Port}],
+    Host :: inet:socket_address() | inet:hostname(),
+    Port :: inet:port_number(),
+    Result :: boolean().
+is_all_tcp_servers_available(Servers) ->
+    Fun =
+        fun({Host, Port}) ->
+            is_tcp_server_available(Host, Port)
+        end,
+    lists:all(Fun, Servers).
+
 -spec is_tcp_server_available(
    Host :: inet:socket_address() | inet:hostname(),
    Port :: inet:port_number()
--- a/apps/emqx_authn/src/simple_authn/emqx_authn_jwt.erl
+++ b/apps/emqx_authn/src/simple_authn/emqx_authn_jwt.erl
@ -75,26 +75,11 @@ fields('jwks') ->
        {pool_size, fun emqx_connector_schema_lib:pool_size/1},
        {refresh_interval, fun refresh_interval/1},
        {ssl, #{
-            type => hoconsc:union([
-                hoconsc:ref(?MODULE, ssl_enable),
-                hoconsc:ref(?MODULE, ssl_disable)
-            ]),
-            desc => ?DESC(ssl),
+            type => hoconsc:ref(emqx_schema, "ssl_client_opts"),
            default => #{<<"enable">> => false},
-            required => false
+            desc => ?DESC("ssl")
        }}
-    ] ++ common_fields();
-fields(ssl_enable) ->
-    [
-        {enable, #{type => true, desc => ?DESC(enable)}},
-        {cacertfile, fun cacertfile/1},
-        {certfile, fun certfile/1},
-        {keyfile, fun keyfile/1},
-        {verify, fun verify/1},
-        {server_name_indication, fun server_name_indication/1}
-    ];
-fields(ssl_disable) ->
-    [{enable, #{type => false, desc => ?DESC(enable)}}].
+    ] ++ common_fields().

 desc('hmac-based') ->
    ?DESC('hmac-based');
@ -147,27 +132,6 @@ refresh_interval(default) -> 300;
 refresh_interval(validator) -> [fun(I) -> I > 0 end];
 refresh_interval(_) -> undefined.

-cacertfile(type) -> string();
-cacertfile(desc) -> ?DESC(?FUNCTION_NAME);
-cacertfile(_) -> undefined.
-
-certfile(type) -> string();
-certfile(desc) -> ?DESC(?FUNCTION_NAME);
-certfile(_) -> undefined.
-
-keyfile(type) -> string();
-keyfile(desc) -> ?DESC(?FUNCTION_NAME);
-keyfile(_) -> undefined.
-
-verify(type) -> hoconsc:enum([verify_peer, verify_none]);
-verify(desc) -> ?DESC(?FUNCTION_NAME);
-verify(default) -> verify_none;
-verify(_) -> undefined.
-
-server_name_indication(type) -> string();
-server_name_indication(desc) -> ?DESC(?FUNCTION_NAME);
-server_name_indication(_) -> undefined.
-
 verify_claims(type) ->
    list();
 verify_claims(desc) ->
--- a/apps/emqx_connector/src/emqx_connector_redis.erl
+++ b/apps/emqx_connector/src/emqx_connector_redis.erl
@ -90,6 +90,7 @@ fields(sentinel) ->
        }},
        {sentinel, #{
            type => string(),
+            required => true,
            desc => ?DESC("sentinel_desc")
        }}
    ] ++
--- a/apps/emqx_connector/test/emqx_connector_redis_SUITE.erl
+++ b/apps/emqx_connector/test/emqx_connector_redis_SUITE.erl
@ -23,8 +23,10 @@
 -include_lib("emqx/include/emqx.hrl").
 -include_lib("stdlib/include/assert.hrl").

-define(REDIS_HOST, "redis").
-define(REDIS_PORT, 6379).
+-define(REDIS_SINGLE_HOST, "redis").
+-define(REDIS_SINGLE_PORT, 6379).
+-define(REDIS_SENTINEL_HOST, "redis-sentinel").
+-define(REDIS_SENTINEL_PORT, 26379).
 -define(REDIS_RESOURCE_MOD, emqx_connector_redis).

 all() ->
@ -34,7 +36,14 @@ groups() ->
    [].

 init_per_suite(Config) ->
-    case emqx_common_test_helpers:is_tcp_server_available(?REDIS_HOST, ?REDIS_PORT) of
+    case
+        emqx_common_test_helpers:is_all_tcp_servers_available(
+            [
+                {?REDIS_SINGLE_HOST, ?REDIS_SINGLE_PORT},
+                {?REDIS_SENTINEL_HOST, ?REDIS_SENTINEL_PORT}
+            ]
+        )
+    of
        true ->
            ok = emqx_common_test_helpers:start_apps([emqx_conf]),
            ok = emqx_connector_test_helpers:start_apps([emqx_resource, emqx_connector]),
@ -141,20 +150,35 @@ redis_config_cluster() ->
 redis_config_sentinel() ->
    redis_config_base("sentinel", "servers").

+-define(REDIS_CONFIG_BASE(MaybeSentinel),
+    "" ++
+        "\n" ++
+        "    auto_reconnect = true\n" ++
+        "    database = 1\n" ++
+        "    pool_size = 8\n" ++
+        "    redis_type = ~s\n" ++
+        MaybeSentinel ++
+        "    password = public\n" ++
+        "    ~s = \"~s:~b\"\n" ++
+        "    " ++
+        ""
+).
+
 redis_config_base(Type, ServerKey) ->
+    case Type of
+        "sentinel" ->
+            Host = ?REDIS_SENTINEL_HOST,
+            Port = ?REDIS_SENTINEL_PORT,
+            MaybeSentinel = "    sentinel = mymaster\n";
+        _ ->
+            Host = ?REDIS_SINGLE_HOST,
+            Port = ?REDIS_SINGLE_PORT,
+            MaybeSentinel = ""
+    end,
    RawConfig = list_to_binary(
        io_lib:format(
-            ""
-            "\n"
-            "    auto_reconnect = true\n"
-            "    database = 1\n"
-            "    pool_size = 8\n"
-            "    redis_type = ~s\n"
-            "    password = public\n"
-            "    ~s = \"~s:~b\"\n"
-            "    "
-            "",
-            [Type, ServerKey, ?REDIS_HOST, ?REDIS_PORT]
+            ?REDIS_CONFIG_BASE(MaybeSentinel),
+            [Type, ServerKey, Host, Port]
        )
    ),