Skip to content

Commit

Permalink
fix(schema): validate cluster strategy and node name
Browse files Browse the repository at this point in the history
If cluster strategy is configured as `dns`, the node name
must be IP address
  • Loading branch information
zmstone committed Feb 20, 2024
1 parent 6f6dbc2 commit 35662c2
Show file tree
Hide file tree
Showing 3 changed files with 129 additions and 2 deletions.
59 changes: 57 additions & 2 deletions apps/emqx_conf/src/emqx_conf_schema.erl
Expand Up @@ -42,6 +42,8 @@
%% internal exports for `emqx_enterprise_schema' only.
-export([ensure_unicode_path/2, convert_rotation/2, log_handler_common_confs/2]).

-define(DEFAULT_NODE_NAME, <<"emqx@127.0.0.1">>).

%% Static apps which merge their configs into the merged emqx.conf
%% The list can not be made a dynamic read at run-time as it is used
%% by nodetool to generate app.<time>.config before EMQX is started
Expand Down Expand Up @@ -123,7 +125,8 @@ roots() ->
lists:flatmap(fun roots/1, common_apps()).

validations() ->
hocon_schema:validations(emqx_schema) ++
[{check_node_name_and_discovery_strategy, fun validate_cluster_strategy/1}] ++
hocon_schema:validations(emqx_schema) ++
lists:flatmap(fun hocon_schema:validations/1, common_apps()).

common_apps() ->
Expand Down Expand Up @@ -359,7 +362,7 @@ fields("node") ->
sc(
string(),
#{
default => <<"emqx@127.0.0.1">>,
default => ?DEFAULT_NODE_NAME,
'readOnly' => true,
importance => ?IMPORTANCE_HIGH,
desc => ?DESC(node_name)
Expand Down Expand Up @@ -1436,3 +1439,55 @@ ensure_unicode_path(Path, Opts) ->

log_level() ->
hoconsc:enum([debug, info, notice, warning, error, critical, alert, emergency, all]).

validate_cluster_strategy(#{<<"node">> := _, <<"cluster">> := _} = Conf) ->
Name = hocon_maps:get("node.name", Conf),
[_Prefix, Host] = re:split(Name, "@", [{return, list}, unicode]),
Strategy = hocon_maps:get("cluster.discovery_strategy", Conf),
Type = hocon_maps:get("cluster.dns.record_type", Conf),
validate_dns_cluster_strategy(Strategy, Type, Host);
validate_cluster_strategy(_) ->
true.

validate_dns_cluster_strategy(dns, srv, _Host) ->
ok;
validate_dns_cluster_strategy(dns, Type, Host) ->
case is_ip_addr(unicode:characters_to_list(Host), Type) of
true ->
ok;
false ->
throw(#{
explain =>
"Node name must be of name@IP format "
"for DNS cluster discovery strategy with " ++ atom_to_list(Type) ++
"record type.",
domain => unicode:characters_to_list(Host)
})
end;
validate_dns_cluster_strategy(_Other, _Type, _Name) ->
true.

is_ip_addr(Host, Type) ->
case inet:parse_address(Host) of
{ok, Ip} ->
AddrType = address_type(Ip),
case
(AddrType =:= ipv4 andalso Type =:= a) orelse
(AddrType =:= ipv6 andalso Type =:= aaaa)
of
true ->
true;
false ->
throw(#{
explain => "Node name address " ++ atom_to_list(AddrType) ++
" is incompatible with DNS record type " ++ atom_to_list(Type),
record_type => Type,
address_type => address_type(Ip)
})
end;
_ ->
false
end.

address_type(IP) when tuple_size(IP) =:= 4 -> ipv4;
address_type(IP) when tuple_size(IP) =:= 8 -> ipv6.
69 changes: 69 additions & 0 deletions apps/emqx_conf/test/emqx_conf_schema_tests.erl
Expand Up @@ -619,3 +619,72 @@ load_and_check_test_() ->
end)
end}
].

%% erlfmt-ignore
dns_record_conf(NodeName, DnsRecordType) ->
"
node {
name = \"" ++ NodeName ++ "\"
data_dir = \"data\"
cookie = cookie
max_ports = 2048
process_limit = 10240
}
cluster {
name = emqxcl
discovery_strategy = dns
dns.record_type = " ++ atom_to_list(DnsRecordType) ++"
}
".

a_record_with_non_ip_node_name_test_() ->
Test = fun(DnsRecordType) ->
{ok, ConfMap} = hocon:binary(dns_record_conf("emqx@local.host", DnsRecordType), #{
format => map
}),
?assertThrow(
{emqx_conf_schema, [
#{
reason := integrity_validation_failure,
result := #{domain := "local.host"},
kind := validation_error,
validation_name := check_node_name_and_discovery_strategy
}
]},
hocon_tconf:check_plain(emqx_conf_schema, ConfMap, #{required => false}, [node, cluster])
)
end,
[
{"a record", fun() -> Test(a) end},
{"aaaa record", fun() -> Test(aaaa) end}
].

dns_record_type_incompatiblie_with_node_host_ip_format_test_() ->
Test = fun(Ip, DnsRecordType) ->
{ok, ConfMap} = hocon:binary(dns_record_conf("emqx@" ++ Ip, DnsRecordType), #{format => map}),
?assertThrow(
{emqx_conf_schema, [
#{
reason := integrity_validation_failure,
result := #{
record_type := DnsRecordType,
address_type := _
},
kind := validation_error,
validation_name := check_node_name_and_discovery_strategy
}
]},
hocon_tconf:check_plain(emqx_conf_schema, ConfMap, #{required => false}, [node, cluster])
)
end,
[
{"ipv4 address", fun() -> Test("::1", a) end},
{"ipv6 address", fun() -> Test("127.0.0.1", aaaa) end}
].

dns_srv_record_is_ok_test() ->
{ok, ConfMap} = hocon:binary(dns_record_conf("emqx@local.host", srv), #{format => map}),
?assertMatch(
Value when is_map(Value),
hocon_tconf:check_plain(emqx_conf_schema, ConfMap, #{required => false}, [node, cluster])
).
3 changes: 3 additions & 0 deletions changes/ce/fix-12541.en.md
@@ -0,0 +1,3 @@
Added a config validation to check if `node.name` is compatible with `cluster.discover_strategy`.

For `dns` strategy with `a` or `aaaa` record types, all nodes must use (static) IP address as host name.

0 comments on commit 35662c2

Please sign in to comment.