Browse Source

chore: apply review suggestions

Zhongwen Deng 2 years ago
parent
commit
fdf9b2a383

+ 18 - 10
apps/emqx_authn/src/simple_authn/emqx_authn_http.erl

@@ -264,10 +264,9 @@ transform_header_name(Headers) ->
     ).
 
 check_ssl_opts(Conf) ->
-    case get_conf_val("url", Conf) of
-        undefined ->
-            ok;
-        Url ->
+    case is_backend_http(Conf) of
+        true ->
+            Url = get_conf_val("url", Conf),
             {BaseUrl, _Path, _Query} = parse_url(Url),
             case BaseUrl of
                 <<"https://", _/binary>> ->
@@ -279,14 +278,15 @@ check_ssl_opts(Conf) ->
                     end;
                 <<"http://", _/binary>> ->
                     ok
-            end
+            end;
+        false ->
+            ok
     end.
 
 check_headers(Conf) ->
-    case get_conf_val("headers", Conf) of
-        undefined ->
-            ok;
-        Headers ->
+    case is_backend_http(Conf) of
+        true ->
+            Headers = get_conf_val("headers", Conf),
             case to_bin(get_conf_val("method", Conf)) of
                 <<"post">> ->
                     ok;
@@ -295,7 +295,15 @@ check_headers(Conf) ->
                         false -> ok;
                         true -> <<"HTTP GET requests cannot include content-type header.">>
                     end
-            end
+            end;
+        false ->
+            ok
+    end.
+
+is_backend_http(Conf) ->
+    case get_conf_val("backend", Conf) of
+        http -> true;
+        _ -> false
     end.
 
 parse_url(Url) ->

+ 1 - 1
apps/emqx_authz/src/emqx_authz.app.src

@@ -1,7 +1,7 @@
 %% -*- mode: erlang -*-
 {application, emqx_authz, [
     {description, "An OTP application"},
-    {vsn, "0.1.17"},
+    {vsn, "0.1.18"},
     {registered, []},
     {mod, {emqx_authz_app, []}},
     {applications, [

+ 80 - 104
apps/emqx_conf/test/emqx_conf_schema_tests.erl

@@ -5,27 +5,28 @@
 -module(emqx_conf_schema_tests).
 
 -include_lib("eunit/include/eunit.hrl").
+
+%% erlfmt-ignore
 -define(BASE_CONF,
-    ""
-    "\n"
-    "     node {\n"
-    "        name = \"emqx1@127.0.0.1\"\n"
-    "        cookie = \"emqxsecretcookie\"\n"
-    "        data_dir = \"data\"\n"
-    "     }\n"
-    "     cluster {\n"
-    "        name = emqxcl\n"
-    "        discovery_strategy = static\n"
-    "        static.seeds = ~p\n"
-    "        core_nodes = ~p\n"
-    "     }\n"
-    ""
-).
+    """
+             node {
+                name = \"emqx1@127.0.0.1\"
+                cookie = \"emqxsecretcookie\"
+                data_dir = \"data\"
+             }
+             cluster {
+                name = emqxcl
+                discovery_strategy = static
+                static.seeds = ~p
+                core_nodes = ~p
+             }
+    """).
+
 array_nodes_test() ->
     ExpectNodes = ['emqx1@127.0.0.1', 'emqx2@127.0.0.1'],
     lists:foreach(
         fun(Nodes) ->
-            ConfFile = iolist_to_binary(io_lib:format(?BASE_CONF, [Nodes, Nodes])),
+            ConfFile = to_bin(?BASE_CONF, [Nodes, Nodes]),
             {ok, Conf} = hocon:binary(ConfFile, #{format => richmap}),
             ConfList = hocon_tconf:generate(emqx_conf_schema, Conf),
             ClusterDiscovery = proplists:get_value(
@@ -46,100 +47,72 @@ array_nodes_test() ->
     ),
     ok.
 
+%% erlfmt-ignore
+-define(BASE_AUTHN_ARRAY,
+    """
+        authentication = [
+          {backend = \"http\"
+          body {password = \"${password}\", username = \"${username}\"}
+          connect_timeout = \"15s\"
+          enable_pipelining = 100
+          headers {\"content-type\" = \"application/json\"}
+          mechanism = \"password_based\"
+          method = \"~p\"
+          pool_size = 8
+          request_timeout = \"5s\"
+          ssl {enable = ~p, verify = \"verify_peer\"}
+          url = \"~ts\"
+        }
+        ]
+    """
+).
+
+-define(ERROR(Reason),
+    {emqx_conf_schema, [
+        #{
+            kind := validation_error,
+            reason := integrity_validation_failure,
+            result := _,
+            validation_name := Reason
+        }
+    ]}
+).
+
 authn_validations_test() ->
-    BaseConf = iolist_to_binary(io_lib:format(?BASE_CONF, ["emqx1@127.0.0.1", "emqx1@127.0.0.1"])),
-    DisableSSLWithHttps =
-        ""
-        "\n"
-        "authentication = [\n"
-        "{backend = \"http\"\n"
-        "body {password = \"${password}\", username = \"${username}\"}\n"
-        "connect_timeout = \"15s\"\n"
-        "enable_pipelining = 100\n"
-        "headers {\"content-type\" = \"application/json\"}\n"
-        "mechanism = \"password_based\"\n"
-        "method = \"post\"\n"
-        "pool_size = 8\n"
-        "request_timeout = \"5s\"\n"
-        "ssl {enable = false, verify = \"verify_peer\"}\n"
-        "url = \"https://127.0.0.1:8080\"\n"
-        "}\n"
-        "]\n"
-        "",
-    Conf = <<BaseConf/binary, (list_to_binary(DisableSSLWithHttps))/binary>>,
-    {ok, ConfMap} = hocon:binary(Conf, #{format => richmap}),
+    BaseConf = to_bin(?BASE_CONF, ["emqx1@127.0.0.1", "emqx1@127.0.0.1"]),
+
+    OKHttps = to_bin(?BASE_AUTHN_ARRAY, [post, true, <<"https://127.0.0.1:8080">>]),
+    Conf0 = <<BaseConf/binary, OKHttps/binary>>,
+    {ok, ConfMap0} = hocon:binary(Conf0, #{format => richmap}),
+    ?assert(is_list(hocon_tconf:generate(emqx_conf_schema, ConfMap0))),
+
+    OKHttp = to_bin(?BASE_AUTHN_ARRAY, [post, false, <<"http://127.0.0.1:8080">>]),
+    Conf1 = <<BaseConf/binary, OKHttp/binary>>,
+    {ok, ConfMap1} = hocon:binary(Conf1, #{format => richmap}),
+    ?assert(is_list(hocon_tconf:generate(emqx_conf_schema, ConfMap1))),
+
+    DisableSSLWithHttps = to_bin(?BASE_AUTHN_ARRAY, [post, false, <<"https://127.0.0.1:8080">>]),
+    Conf2 = <<BaseConf/binary, DisableSSLWithHttps/binary>>,
+    {ok, ConfMap2} = hocon:binary(Conf2, #{format => richmap}),
     ?assertThrow(
-        {emqx_conf_schema, [
-            #{
-                kind := validation_error,
-                reason := integrity_validation_failure,
-                result := _,
-                validation_name := check_http_ssl_opts
-            }
-        ]},
-        hocon_tconf:generate(emqx_conf_schema, ConfMap)
+        ?ERROR(check_http_ssl_opts),
+        hocon_tconf:generate(emqx_conf_schema, ConfMap2)
     ),
-    BadHeader =
-        ""
-        "\n"
-        "authentication = [\n"
-        "{backend = \"http\"\n"
-        "body {password = \"${password}\", username = \"${username}\"}\n"
-        "connect_timeout = \"15s\"\n"
-        "enable_pipelining = 100\n"
-        "headers {\"content-type\" = \"application/json\"}\n"
-        "mechanism = \"password_based\"\n"
-        "method = \"get\"\n"
-        "pool_size = 8\n"
-        "request_timeout = \"5s\"\n"
-        "ssl {enable = false, verify = \"verify_peer\"}\n"
-        "url = \"http://127.0.0.1:8080\"\n"
-        "}\n"
-        "]\n"
-        "",
-    Conf1 = <<BaseConf/binary, (list_to_binary(BadHeader))/binary>>,
-    {ok, ConfMap1} = hocon:binary(Conf1, #{format => richmap}),
+
+    BadHeader = to_bin(?BASE_AUTHN_ARRAY, [get, true, <<"https://127.0.0.1:8080">>]),
+    Conf3 = <<BaseConf/binary, BadHeader/binary>>,
+    {ok, ConfMap3} = hocon:binary(Conf3, #{format => richmap}),
     ?assertThrow(
-        {emqx_conf_schema, [
-            #{
-                kind := validation_error,
-                reason := integrity_validation_failure,
-                result := _,
-                validation_name := check_http_headers
-            }
-        ]},
-        hocon_tconf:generate(emqx_conf_schema, ConfMap1)
+        ?ERROR(check_http_headers),
+        hocon_tconf:generate(emqx_conf_schema, ConfMap3)
     ),
-    BadHeader2 =
-        ""
-        "\n"
-        "authentication = \n"
-        "{backend = \"http\"\n"
-        "body {password = \"${password}\", username = \"${username}\"}\n"
-        "connect_timeout = \"15s\"\n"
-        "enable_pipelining = 100\n"
-        "headers {\"content-type\" = \"application/json\"}\n"
-        "mechanism = \"password_based\"\n"
-        "method = \"get\"\n"
-        "pool_size = 8\n"
-        "request_timeout = \"5s\"\n"
-        "ssl {enable = false, verify = \"verify_peer\"}\n"
-        "url = \"http://127.0.0.1:8080\"\n"
-        "}\n"
-        "\n"
-        "",
-    Conf2 = <<BaseConf/binary, (list_to_binary(BadHeader2))/binary>>,
-    {ok, ConfMap2} = hocon:binary(Conf2, #{format => richmap}),
+
+    BadHeaderWithTuple = binary:replace(BadHeader, [<<"[">>, <<"]">>], <<"">>, [global]),
+    Conf4 = <<BaseConf/binary, BadHeaderWithTuple/binary>>,
+    {ok, ConfMap4} = hocon:binary(Conf4, #{format => richmap}),
     ?assertThrow(
-        {emqx_conf_schema, [
-            #{
-                kind := validation_error,
-                reason := integrity_validation_failure,
-                result := _,
-                validation_name := check_http_headers
-            }
-        ]},
-        hocon_tconf:generate(emqx_conf_schema, ConfMap2)
+        ?ERROR(check_http_headers),
+        hocon_tconf:generate(emqx_conf_schema, ConfMap4)
     ),
     ok.
 
@@ -163,3 +136,6 @@ doc_gen_test() ->
             ok
         end
     }.
+
+to_bin(Format, Args) ->
+    iolist_to_binary(io_lib:format(Format, Args)).

+ 1 - 1
apps/emqx_connector/src/emqx_connector.app.src

@@ -1,7 +1,7 @@
 %% -*- mode: erlang -*-
 {application, emqx_connector, [
     {description, "EMQX Data Integration Connectors"},
-    {vsn, "0.1.20"},
+    {vsn, "0.1.21"},
     {registered, []},
     {mod, {emqx_connector_app, []}},
     {applications, [

+ 0 - 2
apps/emqx_prometheus/TODO

@@ -1,2 +0,0 @@
-1. Add more VM Metrics
-2. Add more emqx Metrics