From 69ec76f586df51d472ab3701e91acb3a918bfce7 Mon Sep 17 00:00:00 2001 From: Thales Macedo Garitezi Date: Tue, 2 Aug 2022 17:25:33 -0300 Subject: [PATCH] chore(license): treat license file API as an upload (5.0) Improving the HTTP API for license after PR feedback. --- .../i18n/emqx_license_http_api.conf | 17 ++- lib-ee/emqx_license/src/emqx_license.erl | 18 ++- .../src/emqx_license_http_api.erl | 80 +++++----- .../emqx_license/src/emqx_license_schema.erl | 14 +- .../emqx_license/test/emqx_license_SUITE.erl | 7 +- .../test/emqx_license_http_api_SUITE.erl | 140 ++++++------------ 6 files changed, 131 insertions(+), 145 deletions(-) diff --git a/lib-ee/emqx_license/i18n/emqx_license_http_api.conf b/lib-ee/emqx_license/i18n/emqx_license_http_api.conf index 67ec1381f..59f76b7d6 100644 --- a/lib-ee/emqx_license/i18n/emqx_license_http_api.conf +++ b/lib-ee/emqx_license/i18n/emqx_license_http_api.conf @@ -10,10 +10,21 @@ emqx_license_http_api { } } - desc_license_upload_api { + desc_license_file_api { desc { - en: "Upload a license file or key" - zh: "上传许可证文件或密钥" + en: "Upload a license file" + zh: "上传一个许可证文件" + } + label: { + en: "Update license" + zh: "更新许可证" + } + } + + desc_license_key_api { + desc { + en: "Update a license key" + zh: "更新一个许可证密钥" } label: { en: "Update license" diff --git a/lib-ee/emqx_license/src/emqx_license.erl b/lib-ee/emqx_license/src/emqx_license.erl index 24b2cc709..d37d40dd3 100644 --- a/lib-ee/emqx_license/src/emqx_license.erl +++ b/lib-ee/emqx_license/src/emqx_license.erl @@ -22,6 +22,7 @@ read_license/0, read_license/1, update_file/1, + update_file_contents/1, update_key/1, license_dir/0, save_and_backup_license/1 @@ -70,16 +71,21 @@ relative_license_path() -> update_file(Filename) when is_binary(Filename); is_list(Filename) -> case file:read_file(Filename) of {ok, Contents} -> - Result = emqx_conf:update( - ?CONF_KEY_PATH, - {file, Contents}, - #{rawconf_with_defaults => true, override_to => local} - ), - handle_config_update_result(Result); + update_file_contents(Contents); {error, Error} -> {error, Error} end. +-spec update_file_contents(binary() | string()) -> + {ok, emqx_config:update_result()} | {error, emqx_config:update_error()}. +update_file_contents(Contents) when is_binary(Contents) -> + Result = emqx_conf:update( + ?CONF_KEY_PATH, + {file, Contents}, + #{rawconf_with_defaults => true, override_to => local} + ), + handle_config_update_result(Result). + -spec update_key(binary() | string()) -> {ok, emqx_config:update_result()} | {error, emqx_config:update_error()}. update_key(Value) when is_binary(Value); is_list(Value) -> diff --git a/lib-ee/emqx_license/src/emqx_license_http_api.erl b/lib-ee/emqx_license/src/emqx_license_http_api.erl index e11631004..e758bbf6b 100644 --- a/lib-ee/emqx_license/src/emqx_license_http_api.erl +++ b/lib-ee/emqx_license/src/emqx_license_http_api.erl @@ -18,11 +18,11 @@ -export([ '/license'/2, - '/license/upload'/2 + '/license/key'/2, + '/license/file'/2 ]). -define(BAD_REQUEST, 'BAD_REQUEST'). --define(NOT_FOUND, 'NOT_FOUND'). namespace() -> "license_http_api". @@ -32,7 +32,8 @@ api_spec() -> paths() -> [ "/license", - "/license/upload" + "/license/key", + "/license/file" ]. schema("/license") -> @@ -54,15 +55,36 @@ schema("/license") -> } } }; -schema("/license/upload") -> +schema("/license/file") -> #{ - 'operationId' => '/license/upload', + 'operationId' => '/license/file', post => #{ tags => [<<"license">>], - summary => <<"Upload license">>, - description => ?DESC("desc_license_upload_api"), + summary => <<"Upload license file">>, + description => ?DESC("desc_license_file_api"), + 'requestBody' => emqx_dashboard_swagger:file_schema(filename), + responses => #{ + 200 => emqx_dashboard_swagger:schema_with_examples( + map(), + #{ + sample_license_info => #{ + value => sample_license_info_response() + } + } + ), + 400 => emqx_dashboard_swagger:error_codes([?BAD_REQUEST], <<"Bad license file">>) + } + } + }; +schema("/license/key") -> + #{ + 'operationId' => '/license/key', + post => #{ + tags => [<<"license">>], + summary => <<"Update license key">>, + description => ?DESC("desc_license_key_api"), 'requestBody' => emqx_dashboard_swagger:schema_with_examples( - emqx_license_schema:license_type(), + emqx_license_schema:key_license(), #{ license_key => #{ summary => <<"License key string">>, @@ -71,14 +93,6 @@ schema("/license/upload") -> <<"connection_low_watermark">> => "75%", <<"connection_high_watermark">> => "80%" } - }, - license_file => #{ - summary => <<"Path to a license file">>, - value => #{ - <<"file">> => <<"/path/to/license">>, - <<"connection_low_watermark">> => "75%", - <<"connection_high_watermark">> => "80%" - } } } ), @@ -91,8 +105,7 @@ schema("/license/upload") -> } } ), - 400 => emqx_dashboard_swagger:error_codes([?BAD_REQUEST], <<"Bad license key">>), - 404 => emqx_dashboard_swagger:error_codes([?NOT_FOUND], <<"File not found">>) + 400 => emqx_dashboard_swagger:error_codes([?BAD_REQUEST], <<"Bad license file">>) } } }. @@ -117,37 +130,26 @@ error_msg(Code, Msg) -> License = maps:from_list(emqx_license_checker:dump()), {200, License}. -'/license/upload'(post, #{body := #{<<"file">> := Filepath}}) -> - case emqx_license:update_file(Filepath) of - {error, enoent} -> - ?SLOG(error, #{ - msg => "license_file_not_found", - path => Filepath - }), - {404, error_msg(?NOT_FOUND, <<"File not found">>)}; - {error, Error} when is_atom(Error) -> - ?SLOG(error, #{ - msg => "bad_license_file", - reason => Error, - path => Filepath - }), - {400, error_msg(?BAD_REQUEST, emqx_misc:explain_posix(Error))}; +'/license/file'(post, #{body := #{<<"filename">> := #{type := _} = File}}) -> + [{_Filename, Contents}] = maps:to_list(maps:without([type], File)), + case emqx_license:update_file_contents(Contents) of {error, Error} -> ?SLOG(error, #{ msg => "bad_license_file", - reason => Error, - path => Filepath + reason => Error }), {400, error_msg(?BAD_REQUEST, <<"Bad license file">>)}; {ok, _} -> ?SLOG(info, #{ - msg => "updated_license_file", - path => Filepath + msg => "updated_license_file" }), License = maps:from_list(emqx_license_checker:dump()), {200, License} end; -'/license/upload'(post, #{body := #{<<"key">> := Key}}) -> +'/license/file'(post, _Params) -> + {400, error_msg(?BAD_REQUEST, <<"Invalid request params">>)}. + +'/license/key'(post, #{body := #{<<"key">> := Key}}) -> case emqx_license:update_key(Key) of {error, Error} -> ?SLOG(error, #{ @@ -160,5 +162,5 @@ error_msg(Code, Msg) -> License = maps:from_list(emqx_license_checker:dump()), {200, License} end; -'/license/upload'(post, _Params) -> +'/license/key'(post, _Params) -> {400, error_msg(?BAD_REQUEST, <<"Invalid request params">>)}. diff --git a/lib-ee/emqx_license/src/emqx_license_schema.erl b/lib-ee/emqx_license/src/emqx_license_schema.erl index 88d245eb3..ab0da3b9a 100644 --- a/lib-ee/emqx_license/src/emqx_license_schema.erl +++ b/lib-ee/emqx_license/src/emqx_license_schema.erl @@ -15,7 +15,9 @@ -export([roots/0, fields/1, validations/0, desc/1]). -export([ - license_type/0 + license_type/0, + key_license/0, + file_license/0 ]). roots() -> @@ -99,10 +101,16 @@ validations() -> license_type() -> hoconsc:union([ - hoconsc:ref(?MODULE, key_license), - hoconsc:ref(?MODULE, file_license) + key_license(), + file_license() ]). +key_license() -> + hoconsc:ref(?MODULE, key_license). + +file_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 08b3cb692..ec92f4efe 100644 --- a/lib-ee/emqx_license/test/emqx_license_SUITE.erl +++ b/lib-ee/emqx_license/test/emqx_license_SUITE.erl @@ -16,6 +16,9 @@ all() -> emqx_common_test_helpers:all(?MODULE). init_per_suite(Config) -> + %% hack needed to avoid inter-suite flakiness when using meck... + ok = cover:stop(), + {ok, _} = cover:start(), _ = application:load(emqx_conf), emqx_config:save_schema_mod_and_names(emqx_license_schema), emqx_common_test_helpers:start_apps([emqx_license], fun set_special_configs/1), @@ -141,7 +144,9 @@ setup_test(TestCase, Config) when emqx_config:put([license], LicConfig), RawConfig = #{<<"type">> => file, <<"file">> => LicensePath}, emqx_config:put_raw([<<"license">>], RawConfig), - ok = meck:new(emqx_license, [non_strict, passthrough, no_history, no_link]), + ok = meck:new(emqx_license_parser, [ + non_strict, passthrough, no_history, no_link + ]), meck:expect( emqx_license_parser, parse, diff --git a/lib-ee/emqx_license/test/emqx_license_http_api_SUITE.erl b/lib-ee/emqx_license/test/emqx_license_http_api_SUITE.erl index cb34f8f50..f2feed893 100644 --- a/lib-ee/emqx_license/test/emqx_license_http_api_SUITE.erl +++ b/lib-ee/emqx_license/test/emqx_license_http_api_SUITE.erl @@ -21,23 +21,12 @@ all() -> init_per_suite(Config) -> _ = application:load(emqx_conf), emqx_config:save_schema_mod_and_names(emqx_license_schema), - ok = meck:new(emqx_license_parser, [non_strict, passthrough, no_history, no_link]), - ok = meck:expect( - emqx_license_parser, - parse, - fun(X) -> - emqx_license_parser:parse( - X, - emqx_license_test_lib:public_key_pem() - ) - end - ), emqx_common_test_helpers:start_apps([emqx_license, emqx_dashboard], fun set_special_configs/1), Config. end_per_suite(_) -> - emqx_common_test_helpers:stop_apps([emqx_license, emqx_dashboard]), ok = meck:unload([emqx_license_parser]), + emqx_common_test_helpers:stop_apps([emqx_license, emqx_dashboard]), Config = #{type => file, file => emqx_license_test_lib:default_license()}, emqx_config:put([license], Config), RawConfig = #{<<"type">> => file, <<"file">> => emqx_license_test_lib:default_license()}, @@ -51,7 +40,19 @@ set_special_configs(emqx_license) -> Config = #{type => key, key => LicenseKey}, emqx_config:put([license], Config), RawConfig = #{<<"type">> => key, <<"key">> => LicenseKey}, - emqx_config:put_raw([<<"license">>], RawConfig); + emqx_config:put_raw([<<"license">>], RawConfig), + ok = meck:new(emqx_license_parser, [non_strict, passthrough, no_history, no_link]), + ok = meck:expect( + emqx_license_parser, + parse, + fun(X) -> + emqx_license_parser:parse( + X, + emqx_license_test_lib:public_key_pem() + ) + end + ), + ok; set_special_configs(_) -> ok. @@ -88,6 +89,14 @@ assert_untouched_license() -> get_license() ). +multipart_formdata_request(Uri, File) -> + emqx_dashboard_api_test_helpers:multipart_formdata_request( + Uri, + _Username = <<"license_admin">>, + _Fields = [], + [File] + ). + %%------------------------------------------------------------------------------ %% Testcases %%------------------------------------------------------------------------------ @@ -114,109 +123,54 @@ t_license_info(_Config) -> t_license_upload_file_success(_Config) -> NewKey = emqx_license_test_lib:make_license(#{max_connections => "999"}), - Path = "/tmp/new.lic", - ok = file:write_file(Path, NewKey), - try - Res = request( - post, - uri(["license", "upload"]), - #{file => Path} - ), - ?assertMatch({ok, 200, _}, Res), - {ok, 200, Payload} = Res, - ?assertEqual( - #{ - <<"customer">> => <<"Foo">>, - <<"customer_type">> => 10, - <<"deployment">> => <<"bar-deployment">>, - <<"email">> => <<"contact@foo.com">>, - <<"expiry">> => false, - <<"expiry_at">> => <<"2295-10-27">>, - <<"max_connections">> => 999, - <<"start_at">> => <<"2022-01-11">>, - <<"type">> => <<"trial">> - }, - emqx_json:decode(Payload, [return_maps]) - ), - ?assertMatch( - #{max_connections := 999}, - get_license() - ), - ok - after - ok = file:delete(Path), - ok - end. - -t_license_upload_file_not_found(_Config) -> - Res = request( - post, - uri(["license", "upload"]), - #{file => "/tmp/inexistent.lic"} + Res = multipart_formdata_request( + uri(["license", "file"]), + {filename, "emqx.lic", NewKey} ), - - ?assertMatch({ok, 404, _}, Res), - {ok, 404, Payload} = Res, + ?assertMatch({ok, 200, _}, Res), + {ok, 200, Payload} = Res, ?assertEqual( #{ - <<"code">> => <<"NOT_FOUND">>, - <<"message">> => <<"File not found">> + <<"customer">> => <<"Foo">>, + <<"customer_type">> => 10, + <<"deployment">> => <<"bar-deployment">>, + <<"email">> => <<"contact@foo.com">>, + <<"expiry">> => false, + <<"expiry_at">> => <<"2295-10-27">>, + <<"max_connections">> => 999, + <<"start_at">> => <<"2022-01-11">>, + <<"type">> => <<"trial">> }, emqx_json:decode(Payload, [return_maps]) ), - assert_untouched_license(), + ?assertMatch( + #{max_connections := 999}, + get_license() + ), ok. -t_license_upload_file_reading_error(_Config) -> - %% eisdir - Path = "/tmp/", - Res = request( - post, - uri(["license", "upload"]), - #{file => Path} +t_license_upload_file_bad_license(_Config) -> + Res = multipart_formdata_request( + uri(["license", "file"]), + {filename, "bad.lic", <<"bad key">>} ), ?assertMatch({ok, 400, _}, Res), {ok, 400, Payload} = Res, ?assertEqual( #{ <<"code">> => <<"BAD_REQUEST">>, - <<"message">> => <<"Illegal operation on a directory">> + <<"message">> => <<"Bad license file">> }, emqx_json:decode(Payload, [return_maps]) ), assert_untouched_license(), ok. -t_license_upload_file_bad_license(_Config) -> - Path = "/tmp/bad.lic", - ok = file:write_file(Path, <<"bad key">>), - try - Res = request( - post, - uri(["license", "upload"]), - #{file => Path} - ), - ?assertMatch({ok, 400, _}, Res), - {ok, 400, Payload} = Res, - ?assertEqual( - #{ - <<"code">> => <<"BAD_REQUEST">>, - <<"message">> => <<"Bad license file">> - }, - emqx_json:decode(Payload, [return_maps]) - ), - assert_untouched_license(), - ok - after - ok = file:delete(Path), - ok - end. - t_license_upload_key_success(_Config) -> NewKey = emqx_license_test_lib:make_license(#{max_connections => "999"}), Res = request( post, - uri(["license", "upload"]), + uri(["license", "key"]), #{key => NewKey} ), ?assertMatch({ok, 200, _}, Res), @@ -245,7 +199,7 @@ t_license_upload_key_bad_key(_Config) -> BadKey = <<"bad key">>, Res = request( post, - uri(["license", "upload"]), + uri(["license", "key"]), #{key => BadKey} ), ?assertMatch({ok, 400, _}, Res),