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.
2022-07-28 11:16:33 -03:00 · 2022-07-28 11:16:33 -03:00 · 80d35feb33
parent 5f49f9058f
commit 80d35feb33
12 changed files with 121 additions and 24 deletions
--- 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
--- 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.
--- 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%,
--- 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) ->
--- 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 ->
--- 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
 %%------------------------------------------------------------------------------
--- 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.
--- 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.
--- 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.
--- 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.
--- 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.
--- 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.