From 80d35feb3315de00ad38d31a6f530112c86402d6 Mon Sep 17 00:00:00 2001 From: Thales Macedo Garitezi Date: Thu, 28 Jul 2022 11:16:33 -0300 Subject: [PATCH] fix(license): change schema to allow updating license to different type Before this change, if a license in `emqx.conf` was of type `key`/`file` and then changed to the opposite type, such change would be saved to `{cluster,local}-overrides.conf`. When the node restarts after that, the configs are merged naively and becomes invalid, as it contains both the `key` and `file` keys, and crashes. --- bin/emqx | 2 +- bin/nodetool | 4 +- lib-ee/emqx_license/etc/emqx_license.conf | 1 + lib-ee/emqx_license/src/emqx_license.erl | 8 +- .../emqx_license/src/emqx_license_schema.erl | 34 +++++++- .../emqx_license/test/emqx_license_SUITE.erl | 82 +++++++++++++++++-- .../test/emqx_license_checker_SUITE.erl | 2 +- .../test/emqx_license_cli_SUITE.erl | 4 +- .../test/emqx_license_installer_SUITE.erl | 2 +- .../test/emqx_license_parser_SUITE.erl | 2 +- .../test/emqx_license_parser_legacy_SUITE.erl | 2 +- .../test/emqx_license_resources_SUITE.erl | 2 +- 12 files changed, 121 insertions(+), 24 deletions(-) diff --git a/bin/emqx b/bin/emqx index cac5cf655..a70c676fd 100755 --- a/bin/emqx +++ b/bin/emqx @@ -416,7 +416,7 @@ call_hocon() { ## and parsing HOCON config + environment variables is a non-trivial task CONF_KEYS=( 'node.data_dir' 'node.name' 'node.cookie' 'node.db_backend' 'cluster.proto_dist' ) if [ "$IS_ENTERPRISE" = 'yes' ]; then - CONF_KEYS+=( 'license.file' 'license.key' ) + CONF_KEYS+=( 'license.type' 'license.file' 'license.key' ) fi if [ "$IS_BOOT_COMMAND" = 'yes' ]; then diff --git a/bin/nodetool b/bin/nodetool index 773f855a0..0711f32d6 100755 --- a/bin/nodetool +++ b/bin/nodetool @@ -25,9 +25,9 @@ main(Args) -> %% forward the call to hocon_cli hocon_cli:main(Rest); ["check_license_key", Key] -> - check_license(#{key => list_to_binary(Key)}); + check_license(#{type => key, key => list_to_binary(Key)}); ["check_license_file", File] -> - check_license(#{file => list_to_binary(File)}); + check_license(#{type => file, file => list_to_binary(File)}); _ -> do(Args) end. diff --git a/lib-ee/emqx_license/etc/emqx_license.conf b/lib-ee/emqx_license/etc/emqx_license.conf index 476444ea0..b5684b740 100644 --- a/lib-ee/emqx_license/etc/emqx_license.conf +++ b/lib-ee/emqx_license/etc/emqx_license.conf @@ -1,4 +1,5 @@ license { + type = key # The default license has 1000 connections limit, it is issued on 20220419 and valid for 5 years (1825 days) key = "MjIwMTExCjAKMTAKRXZhbHVhdGlvbgpjb250YWN0QGVtcXguaW8KZGVmYXVsdAoyMDIyMDQxOQoxODI1CjEwMDAK.MEQCICbgRVijCQov2hrvZXR1mk9Oa+tyV1F5oJ6iOZeSHjnQAiB9dUiVeaZekDOjztk+NCWjhk4PG8tWfw2uFZWruSzD6g==" connection_low_watermark = 75%, diff --git a/lib-ee/emqx_license/src/emqx_license.erl b/lib-ee/emqx_license/src/emqx_license.erl index 787a8b283..e7a0cce48 100644 --- a/lib-ee/emqx_license/src/emqx_license.erl +++ b/lib-ee/emqx_license/src/emqx_license.erl @@ -130,7 +130,7 @@ do_update({file, Filename}, Conf) -> {ok, Content} -> case emqx_license_parser:parse(Content) of {ok, _License} -> - maps:remove(<<"key">>, Conf#{<<"file">> => Filename}); + maps:remove(<<"key">>, Conf#{<<"type">> => file, <<"file">> => Filename}); {error, Reason} -> erlang:throw(Reason) end; @@ -140,7 +140,7 @@ do_update({file, Filename}, Conf) -> do_update({key, Content}, Conf) when is_binary(Content); is_list(Content) -> case emqx_license_parser:parse(Content) of {ok, _License} -> - maps:remove(<<"file">>, Conf#{<<"key">> => Content}); + maps:remove(<<"file">>, Conf#{<<"type">> => key, <<"key">> => Content}); {error, Reason} -> erlang:throw(Reason) end; @@ -151,12 +151,12 @@ do_update(_Other, Conf) -> check_max_clients_exceeded(MaxClients) -> emqx_license_resources:connection_count() > MaxClients * 1.1. -read_license(#{file := Filename}) -> +read_license(#{type := file, file := Filename}) -> case file:read_file(Filename) of {ok, Content} -> emqx_license_parser:parse(Content); {error, _} = Error -> Error end; -read_license(#{key := Content}) -> +read_license(#{type := key, key := Content}) -> emqx_license_parser:parse(Content). handle_config_update_result({error, _} = Error) -> diff --git a/lib-ee/emqx_license/src/emqx_license_schema.erl b/lib-ee/emqx_license/src/emqx_license_schema.erl index d6517ab88..88d245eb3 100644 --- a/lib-ee/emqx_license/src/emqx_license_schema.erl +++ b/lib-ee/emqx_license/src/emqx_license_schema.erl @@ -14,14 +14,15 @@ -export([roots/0, fields/1, validations/0, desc/1]). +-export([ + license_type/0 +]). + roots() -> [ {license, hoconsc:mk( - hoconsc:union([ - hoconsc:ref(?MODULE, key_license), - hoconsc:ref(?MODULE, file_license) - ]), + license_type(), #{ desc => "EMQX Enterprise license.\n" @@ -36,16 +37,35 @@ roots() -> fields(key_license) -> [ + {type, #{ + type => key, + required => true + }}, {key, #{ type => string(), %% so it's not logged sensitive => true, + required => true, desc => "License string" + }}, + {file, #{ + type => string(), + required => false }} | common_fields() ]; fields(file_license) -> [ + {type, #{ + type => file, + required => true + }}, + {key, #{ + type => string(), + %% so it's not logged + sensitive => true, + required => false + }}, {file, #{ type => string(), desc => "Path to the license file" @@ -77,6 +97,12 @@ common_fields() -> validations() -> [{check_license_watermark, fun check_license_watermark/1}]. +license_type() -> + hoconsc:union([ + hoconsc:ref(?MODULE, key_license), + hoconsc:ref(?MODULE, file_license) + ]). + check_license_watermark(Conf) -> case hocon_maps:get("license.connection_low_watermark", Conf) of undefined -> diff --git a/lib-ee/emqx_license/test/emqx_license_SUITE.erl b/lib-ee/emqx_license/test/emqx_license_SUITE.erl index bc73f9071..ba898b360 100644 --- a/lib-ee/emqx_license/test/emqx_license_SUITE.erl +++ b/lib-ee/emqx_license/test/emqx_license_SUITE.erl @@ -28,28 +28,65 @@ end_per_suite(_) -> init_per_testcase(Case, Config) -> {ok, _} = emqx_cluster_rpc:start_link(node(), emqx_cluster_rpc, 1000), set_invalid_license_file(Case), - Config. + Paths = set_override_paths(Case), + Paths ++ Config. -end_per_testcase(Case, _Config) -> +end_per_testcase(Case, Config) -> restore_valid_license_file(Case), + clean_overrides(Case, Config), + ok. + +set_override_paths(TestCase) when + TestCase =:= t_change_from_file_to_key; + TestCase =:= t_change_from_key_to_file +-> + LocalOverridePath = filename:join([ + "/tmp", + "local-" ++ atom_to_list(TestCase) ++ ".conf" + ]), + ClusterOverridePath = filename:join([ + "/tmp", + "local-" ++ atom_to_list(TestCase) ++ ".conf" + ]), + application:set_env(emqx, local_override_conf_file, LocalOverridePath), + application:set_env(emqx, cluster_override_conf_file, ClusterOverridePath), + [ + {local_override_path, LocalOverridePath}, + {cluster_override_path, ClusterOverridePath} + ]; +set_override_paths(_TestCase) -> + []. + +clean_overrides(TestCase, Config) when + TestCase =:= t_change_from_file_to_key; + TestCase =:= t_change_from_key_to_file +-> + LocalOverridePath = ?config(local_override_path, Config), + ClusterOverridePath = ?config(cluster_override_path, Config), + file:delete(LocalOverridePath), + file:delete(ClusterOverridePath), + application:unset_env(emqx, local_override_conf_file), + application:unset_env(emqx, cluster_override_conf_file), + ok; +clean_overrides(_TestCase, _Config) -> ok. set_invalid_license_file(t_read_license_from_invalid_file) -> - Config = #{file => "/invalid/file"}, + Config = #{type => file, file => "/invalid/file"}, emqx_config:put([license], Config); set_invalid_license_file(_) -> ok. restore_valid_license_file(t_read_license_from_invalid_file) -> - Config = #{file => emqx_license_test_lib:default_license()}, + Config = #{type => file, file => emqx_license_test_lib:default_license()}, emqx_config:put([license], Config); restore_valid_license_file(_) -> ok. set_special_configs(emqx_license) -> - Config = #{file => emqx_license_test_lib:default_license()}, + Config = #{type => file, file => emqx_license_test_lib:default_license()}, emqx_config:put([license], Config), - RawConfig = #{<<"file">> => emqx_license_test_lib:default_license()}, + RawConfig = #{<<"type">> => file, <<"file">> => emqx_license_test_lib:default_license()}, emqx_config:put_raw([<<"license">>], RawConfig); set_special_configs(_) -> ok. @@ -183,6 +220,39 @@ t_check_not_loaded(_Config) -> emqx_license:check(#{}, #{}) ). +t_change_from_file_to_key(_Config) -> + %% precondition + ?assertMatch(#{file := _}, emqx_conf:get([license])), + + OldConf = emqx_conf:get_raw([]), + + %% this saves updated config to `{cluster,local}-overrrides.conf' + {ok, LicenseValue} = file:read_file(emqx_license_test_lib:default_license()), + {ok, _NewConf} = emqx_license:update_key(LicenseValue), + + %% assert that `{cluster,local}-overrides.conf' merge correctly + ?assertEqual(ok, emqx_config:init_load(emqx_license_schema, OldConf, #{})), + + ok. + +t_change_from_key_to_file(_Config) -> + Config = #{type => key, key => <<"some key">>}, + emqx_config:put([license], Config), + RawConfig = #{<<"type">> => key, <<"key">> => <<"some key">>}, + emqx_config:put_raw([<<"license">>], RawConfig), + + %% precondition + ?assertMatch(#{type := key, key := _}, emqx_conf:get([license])), + OldConf = emqx_conf:get_raw([]), + + %% this saves updated config to `{cluster,local}-overrrides.conf' + {ok, _NewConf} = emqx_license:update_file(emqx_license_test_lib:default_license()), + + %% assert that `{cluster,local}-overrides.conf' merge correctly + ?assertEqual(ok, emqx_config:init_load(emqx_license_schema, OldConf, #{})), + + ok. + %%------------------------------------------------------------------------------ %% Helpers %%------------------------------------------------------------------------------ diff --git a/lib-ee/emqx_license/test/emqx_license_checker_SUITE.erl b/lib-ee/emqx_license/test/emqx_license_checker_SUITE.erl index 8842db7f9..0e10b684d 100644 --- a/lib-ee/emqx_license/test/emqx_license_checker_SUITE.erl +++ b/lib-ee/emqx_license/test/emqx_license_checker_SUITE.erl @@ -35,7 +35,7 @@ end_per_testcase(_Case, _Config) -> ok. set_special_configs(emqx_license) -> - Config = #{file => emqx_license_test_lib:default_license()}, + Config = #{type => file, file => emqx_license_test_lib:default_license()}, emqx_config:put([license], Config); set_special_configs(_) -> ok. diff --git a/lib-ee/emqx_license/test/emqx_license_cli_SUITE.erl b/lib-ee/emqx_license/test/emqx_license_cli_SUITE.erl index ab7fd2dc8..5cf11adda 100644 --- a/lib-ee/emqx_license/test/emqx_license_cli_SUITE.erl +++ b/lib-ee/emqx_license/test/emqx_license_cli_SUITE.erl @@ -31,9 +31,9 @@ end_per_testcase(_Case, _Config) -> ok. set_special_configs(emqx_license) -> - Config = #{file => emqx_license_test_lib:default_license()}, + Config = #{type => file, file => emqx_license_test_lib:default_license()}, emqx_config:put([license], Config), - RawConfig = #{<<"file">> => emqx_license_test_lib:default_license()}, + RawConfig = #{<<"type">> => file, <<"file">> => emqx_license_test_lib:default_license()}, emqx_config:put_raw([<<"license">>], RawConfig); set_special_configs(_) -> ok. diff --git a/lib-ee/emqx_license/test/emqx_license_installer_SUITE.erl b/lib-ee/emqx_license/test/emqx_license_installer_SUITE.erl index e92c082be..e62c4d814 100644 --- a/lib-ee/emqx_license/test/emqx_license_installer_SUITE.erl +++ b/lib-ee/emqx_license/test/emqx_license_installer_SUITE.erl @@ -31,7 +31,7 @@ end_per_testcase(_Case, _Config) -> ok. set_special_configs(emqx_license) -> - Config = #{file => emqx_license_test_lib:default_license()}, + Config = #{type => file, file => emqx_license_test_lib:default_license()}, emqx_config:put([license], Config); set_special_configs(_) -> ok. diff --git a/lib-ee/emqx_license/test/emqx_license_parser_SUITE.erl b/lib-ee/emqx_license/test/emqx_license_parser_SUITE.erl index 3b4e78a49..e9868cdc1 100644 --- a/lib-ee/emqx_license/test/emqx_license_parser_SUITE.erl +++ b/lib-ee/emqx_license/test/emqx_license_parser_SUITE.erl @@ -30,7 +30,7 @@ end_per_testcase(_Case, _Config) -> ok. set_special_configs(emqx_license) -> - Config = #{file => emqx_license_test_lib:default_license()}, + Config = #{type => file, file => emqx_license_test_lib:default_license()}, emqx_config:put([license], Config); set_special_configs(_) -> ok. diff --git a/lib-ee/emqx_license/test/emqx_license_parser_legacy_SUITE.erl b/lib-ee/emqx_license/test/emqx_license_parser_legacy_SUITE.erl index b1c45ade7..61f3c4cd8 100644 --- a/lib-ee/emqx_license/test/emqx_license_parser_legacy_SUITE.erl +++ b/lib-ee/emqx_license/test/emqx_license_parser_legacy_SUITE.erl @@ -30,7 +30,7 @@ end_per_testcase(_Case, _Config) -> ok. set_special_configs(emqx_license) -> - Config = #{file => emqx_license_test_lib:default_license()}, + Config = #{type => file, file => emqx_license_test_lib:default_license()}, emqx_config:put([license], Config); set_special_configs(_) -> ok. diff --git a/lib-ee/emqx_license/test/emqx_license_resources_SUITE.erl b/lib-ee/emqx_license/test/emqx_license_resources_SUITE.erl index a6411902e..fc4e19e5b 100644 --- a/lib-ee/emqx_license/test/emqx_license_resources_SUITE.erl +++ b/lib-ee/emqx_license/test/emqx_license_resources_SUITE.erl @@ -31,7 +31,7 @@ end_per_testcase(_Case, _Config) -> ok. set_special_configs(emqx_license) -> - Config = #{file => emqx_license_test_lib:default_license()}, + Config = #{type => file, file => emqx_license_test_lib:default_license()}, emqx_config:put([license], Config); set_special_configs(_) -> ok.