diff --git a/.github/workflows/run_static_checks.yaml b/.github/workflows/run_static_checks.yaml new file mode 100644 index 000000000..b30097823 --- /dev/null +++ b/.github/workflows/run_static_checks.yaml @@ -0,0 +1,23 @@ +name: Run static checks + +concurrency: + group: static-check-${{ github.event_name }}-${{ github.ref }} + cancel-in-progress: true + +on: + push: + tags: + - v* + - e* + pull_request: + +jobs: + run_static_analysis: + runs-on: self-hosted + container: "ghcr.io/emqx/emqx-builder/5.0-3:24.1.5-3-alpine3.14" + steps: + - uses: actions/checkout@v2 + - name: xref + run: make xref + - name: dialyzer + run: make dialyzer diff --git a/.github/workflows/run_test_cases.yaml b/.github/workflows/run_test_cases.yaml index bb0fb1c82..67e3c922c 100644 --- a/.github/workflows/run_test_cases.yaml +++ b/.github/workflows/run_test_cases.yaml @@ -12,22 +12,6 @@ on: pull_request: jobs: - run_static_analysis: - strategy: - matrix: - emqx_builder: - - 5.0-3:24.1.5-3 # run dialyzer on latest OTP - - runs-on: ubuntu-20.04 - container: "ghcr.io/emqx/emqx-builder/${{ matrix.emqx_builder }}-ubuntu20.04" - - steps: - - uses: actions/checkout@v2 - - name: xref - run: make xref - - name: dialyzer - run: make dialyzer - run_proper_test: strategy: matrix: diff --git a/apps/emqx_management/src/emqx_mgmt_api_app.erl b/apps/emqx_management/src/emqx_mgmt_api_app.erl index 489d679be..4fa9ce469 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_app.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_app.erl @@ -139,13 +139,7 @@ api_key(post, #{body := App}) -> <<"desc">> := Desc0, <<"enable">> := Enable } = App, - %% undefined is never expired - ExpiredAt0 = maps:get(<<"expired_at">>, App, <<"undefined">>), - ExpiredAt = - case ExpiredAt0 of - <<"undefined">> -> undefined; - _ -> ExpiredAt0 - end, + ExpiredAt = ensure_expired_at(App), Desc = unicode:characters_to_binary(Desc0, unicode), case emqx_mgmt_auth:create(Name, Enable, ExpiredAt, Desc) of {ok, NewApp} -> {200, format(NewApp)}; @@ -164,7 +158,7 @@ api_key_by_name(delete, #{bindings := #{name := Name}}) -> end; api_key_by_name(put, #{bindings := #{name := Name}, body := Body}) -> Enable = maps:get(<<"enable">>, Body, undefined), - ExpiredAt = maps:get(<<"expired_at">>, Body, undefined), + ExpiredAt = ensure_expired_at(Body), Desc = maps:get(<<"desc">>, Body, undefined), case emqx_mgmt_auth:update(Name, Enable, ExpiredAt, Desc) of {ok, App} -> {200, format(App)}; @@ -181,3 +175,6 @@ format(App = #{expired_at := ExpiredAt0, created_at := CreateAt}) -> expired_at => ExpiredAt, created_at => list_to_binary(calendar:system_time_to_rfc3339(CreateAt)) }. + +ensure_expired_at(#{<<"expired_at">> := ExpiredAt})when is_integer(ExpiredAt) -> ExpiredAt; +ensure_expired_at(_) -> undefined. diff --git a/apps/emqx_management/src/emqx_mgmt_api_trace.erl b/apps/emqx_management/src/emqx_mgmt_api_trace.erl index 296cecea2..d92fd6ad8 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_trace.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_trace.erl @@ -304,13 +304,16 @@ download_trace_log(get, #{bindings := #{name := Name}}) -> Zips = group_trace_file(ZipDir, TraceLog, TraceFiles), ZipFileName = ZipDir ++ binary_to_list(Name) ++ ".zip", {ok, ZipFile} = zip:zip(ZipFileName, Zips, [{cwd, ZipDir}]), - emqx_trace:delete_files_after_send(ZipFileName, Zips), + %% emqx_trace:delete_files_after_send(ZipFileName, Zips), + %% TODO use file replace file_binary.(delete file after send is not ready now). + {ok, Binary} = file:read_file(ZipFile), + ZipName = filename:basename(ZipFile), + _ = file:delete(ZipFile), Headers = #{ <<"content-type">> => <<"application/x-zip">>, - <<"content-disposition">> => - iolist_to_binary("attachment; filename=" ++ filename:basename(ZipFile)) + <<"content-disposition">> => iolist_to_binary("attachment; filename=" ++ ZipName) }, - {200, Headers, {file, ZipFile}}; + {200, Headers, {file_binary, ZipName, Binary}}; {error, not_found} -> ?NOT_FOUND(Name) end. diff --git a/apps/emqx_management/src/emqx_mgmt_auth.erl b/apps/emqx_management/src/emqx_mgmt_auth.erl index 512ec6b0f..55201dc86 100644 --- a/apps/emqx_management/src/emqx_mgmt_auth.erl +++ b/apps/emqx_management/src/emqx_mgmt_auth.erl @@ -68,11 +68,11 @@ update(Name, Enable, ExpiredAt, Desc) -> Fun = fun() -> case mnesia:read(?APP, Name, write) of [] -> mnesia:abort(not_found); - [App0 = #?APP{enable = Enable0, expired_at = ExpiredAt0, desc = Desc0}] -> + [App0 = #?APP{enable = Enable0, desc = Desc0}] -> App = App0#?APP{ + expired_at = ExpiredAt, enable = ensure_not_undefined(Enable, Enable0), - expired_at = ensure_not_undefined(ExpiredAt, ExpiredAt0), desc = ensure_not_undefined(Desc, Desc0) }, ok = mnesia:write(App), diff --git a/apps/emqx_management/test/emqx_mgmt_auth_api_SUITE.erl b/apps/emqx_management/test/emqx_mgmt_auth_api_SUITE.erl index 73d4ad566..031e1622f 100644 --- a/apps/emqx_management/test/emqx_mgmt_auth_api_SUITE.erl +++ b/apps/emqx_management/test/emqx_mgmt_auth_api_SUITE.erl @@ -96,6 +96,13 @@ t_update(_Config) -> ?assertEqual(calendar:rfc3339_to_system_time(binary_to_list(ExpiredAt)), calendar:rfc3339_to_system_time(binary_to_list(maps:get(<<"expired_at">>, Update1))) ), + Unexpired1 = maps:without([expired_at], Change), + {ok, Update2} = update_app(Name, Unexpired1), + ?assertEqual(<<"undefined">>, maps:get(<<"expired_at">>, Update2)), + Unexpired2 = Change#{expired_at => <<"undefined">>}, + {ok, Update3} = update_app(Name, Unexpired2), + ?assertEqual(<<"undefined">>, maps:get(<<"expired_at">>, Update3)), + ?assertEqual({error, {"HTTP/1.1", 404, "Not Found"}}, update_app(<<"Not-Exist">>, Change)), ok. @@ -137,6 +144,10 @@ t_authorize(_Config) -> }, ?assertMatch({ok, #{<<"api_key">> := _, <<"enable">> := true}}, update_app(Name, Expired)), ?assertEqual(Unauthorized, emqx_mgmt_api_test_util:request_api(get, BanPath, BasicHeader)), + UnExpired = #{expired_at => undefined}, + ?assertMatch({ok, #{<<"api_key">> := _, <<"expired_at">> := <<"undefined">>}}, + update_app(Name, UnExpired)), + {ok, _Status1} = emqx_mgmt_api_test_util:request_api(get, BanPath, BasicHeader), ok. t_create_unexpired_app(_Config) -> diff --git a/apps/emqx_statsd/src/emqx_statsd_api.erl b/apps/emqx_statsd/src/emqx_statsd_api.erl index d545003b0..ab9416266 100644 --- a/apps/emqx_statsd/src/emqx_statsd_api.erl +++ b/apps/emqx_statsd/src/emqx_statsd_api.erl @@ -59,10 +59,12 @@ statsd(put, #{body := Body}) -> Body, #{rawconf_with_defaults => true, override_to => cluster}) of {ok, #{raw_config := NewConfig, config := Config}} -> - _ = emqx_statsd_sup:stop_child(?APP), + ok = emqx_statsd_sup:ensure_child_stopped(?APP), case maps:get(<<"enable">>, Body) of - true -> emqx_statsd_sup:start_child(?APP, maps:get(config, Config)); - false -> ok + true -> + ok = emqx_statsd_sup:ensure_child_started(?APP, maps:get(config, Config)); + false -> + ok end, {200, NewConfig}; {error, Reason} -> diff --git a/apps/emqx_statsd/src/emqx_statsd_app.erl b/apps/emqx_statsd/src/emqx_statsd_app.erl index 820a86c04..c0a17fc5b 100644 --- a/apps/emqx_statsd/src/emqx_statsd_app.erl +++ b/apps/emqx_statsd/src/emqx_statsd_app.erl @@ -34,7 +34,7 @@ stop(_) -> maybe_enable_statsd() -> case emqx_conf:get([statsd, enable], false) of true -> - emqx_statsd_sup:start_child(?APP, emqx_conf:get([statsd], #{})); + emqx_statsd_sup:ensure_child_started(?APP, emqx_conf:get([statsd], #{})); false -> ok end. diff --git a/apps/emqx_statsd/src/emqx_statsd_sup.erl b/apps/emqx_statsd/src/emqx_statsd_sup.erl index 02b50e3ca..983cf97d4 100644 --- a/apps/emqx_statsd/src/emqx_statsd_sup.erl +++ b/apps/emqx_statsd/src/emqx_statsd_sup.erl @@ -8,9 +8,9 @@ -behaviour(supervisor). -export([ start_link/0 - , start_child/1 - , start_child/2 - , stop_child/1 + , ensure_child_started/1 + , ensure_child_started/2 + , ensure_child_stopped/1 ]). -export([init/1]). @@ -26,19 +26,24 @@ start_link() -> supervisor:start_link({local, ?MODULE}, ?MODULE, []). --spec start_child(supervisor:child_spec()) -> ok. -start_child(ChildSpec) when is_map(ChildSpec) -> +-spec ensure_child_started(supervisor:child_spec()) -> ok. +ensure_child_started(ChildSpec) when is_map(ChildSpec) -> assert_started(supervisor:start_child(?MODULE, ChildSpec)). --spec start_child(atom(), map()) -> ok. -start_child(Mod, Opts) when is_atom(Mod) andalso is_map(Opts) -> +-spec ensure_child_started(atom(), map()) -> ok. +ensure_child_started(Mod, Opts) when is_atom(Mod) andalso is_map(Opts) -> assert_started(supervisor:start_child(?MODULE, ?CHILD(Mod, Opts))). --spec(stop_child(any()) -> ok | {error, term()}). -stop_child(ChildId) -> +%% @doc Stop the child worker process. +-spec ensure_child_stopped(any()) -> ok. +ensure_child_stopped(ChildId) -> case supervisor:terminate_child(?MODULE, ChildId) of - ok -> supervisor:delete_child(?MODULE, ChildId); - Error -> Error + ok -> + %% with terminate_child/2 returned 'ok', it's not possible + %% for supervisor:delete_child/2 to return {error, Reason} + ok = supervisor:delete_child(?MODULE, ChildId); + {error, not_found} -> + ok end. init([]) -> @@ -50,5 +55,5 @@ init([]) -> assert_started({ok, _Pid}) -> ok; assert_started({ok, _Pid, _Info}) -> ok; -assert_started({error, {already_tarted, _Pid}}) -> ok; +assert_started({error, {already_started, _Pid}}) -> ok; assert_started({error, Reason}) -> erlang:error(Reason).