From ae97d516b8f54abc0c9c50f58c8e26e719b0611d Mon Sep 17 00:00:00 2001 From: Andrew Mayorov Date: Wed, 7 Jun 2023 23:25:30 +0300 Subject: [PATCH 1/3] fix(ft): add `enable` flag to every backend And tolerate multiple configured backends, as long as only one of them is enabled. --- apps/emqx_ft/README.md | 3 ++ apps/emqx_ft/src/emqx_ft_schema.erl | 43 ++++++++++++++++--- apps/emqx_ft/src/emqx_ft_storage.erl | 4 +- apps/emqx_ft/src/emqx_ft_storage_exporter.erl | 6 +-- .../src/emqx_ft_storage_exporter_fs.erl | 1 + apps/emqx_ft/src/emqx_ft_storage_fs.erl | 1 + apps/emqx_ft/test/emqx_ft_assembler_SUITE.erl | 1 + apps/emqx_ft/test/emqx_ft_conf_SUITE.erl | 9 +++- .../test/emqx_ft_storage_fs_gc_SUITE.erl | 3 +- apps/emqx_ft/test/emqx_ft_test_helpers.erl | 9 +++- rel/i18n/emqx_ft_schema.hocon | 3 ++ 11 files changed, 68 insertions(+), 15 deletions(-) diff --git a/apps/emqx_ft/README.md b/apps/emqx_ft/README.md index 019c63dea..eaf47d6df 100644 --- a/apps/emqx_ft/README.md +++ b/apps/emqx_ft/README.md @@ -29,6 +29,7 @@ file_transfer { enable = true storage { local { + enable = true exporter { local { root = "/var/lib/emqx/transfers" } } @@ -50,7 +51,9 @@ file_transfer { enable = true storage { local { + enable = true exporter { + enable = true s3 { host = "s3.us-east-1.amazonaws.com" port = "443" diff --git a/apps/emqx_ft/src/emqx_ft_schema.erl b/apps/emqx_ft/src/emqx_ft_schema.erl index c1ee41d0d..d7df09a54 100644 --- a/apps/emqx_ft/src/emqx_ft_schema.erl +++ b/apps/emqx_ft/src/emqx_ft_schema.erl @@ -26,6 +26,7 @@ -export([schema/1]). -export([translate/1]). +-export([backend/1]). -type json_value() :: null @@ -142,7 +143,7 @@ fields(local_storage) -> } } )} - ]; + ] ++ common_backend_fields(); fields(local_storage_segments) -> [ {root, @@ -190,9 +191,9 @@ fields(local_storage_exporter) -> required => false } )} - ]; + ] ++ common_backend_fields(); fields(s3_exporter) -> - emqx_s3_schema:fields(s3); + emqx_s3_schema:fields(s3) ++ common_backend_fields(); fields(local_storage_segments_gc) -> [ {interval, @@ -229,6 +230,18 @@ fields(local_storage_segments_gc) -> )} ]. +common_backend_fields() -> + [ + {enable, + mk( + boolean(), #{ + desc => ?DESC("backend_enable"), + required => false, + default => true + } + )} + ]. + desc(file_transfer) -> "File transfer settings"; desc(local_storage) -> @@ -275,11 +288,19 @@ validator(filename) -> ]; validator(backend) -> fun(Config) -> - case maps:keys(Config) of - [_Type] -> + Enabled = maps:filter( + fun(_, Backend) -> + maps:get(enable, Backend, false) or maps:get(<<"enable">>, Backend, false) + end, + Config + ), + case maps:to_list(Enabled) of + [{_Type, _BackendConfig}] -> ok; _Conflicts = [_ | _] -> - {error, multiple_conflicting_backends} + {error, multiple_enabled_backends}; + _None = [] -> + {error, no_enabled_backend} end end. @@ -319,3 +340,13 @@ translate(Conf) -> ?MODULE, #{atom_to_binary(Root) => Conf}, #{atom_key => true}, [Root] ) ). + +-spec backend(emqx_config:config()) -> + {_Type :: atom(), emqx_config:config()}. +backend(Config) -> + catch maps:foreach(fun emit_enabled/2, Config). + +-spec emit_enabled(atom(), emqx_config:config()) -> + no_return(). +emit_enabled(Type, BConf = #{enable := Enabled}) -> + Enabled andalso throw({Type, BConf}). diff --git a/apps/emqx_ft/src/emqx_ft_storage.erl b/apps/emqx_ft/src/emqx_ft_storage.erl index 007b47db9..e2980c920 100644 --- a/apps/emqx_ft/src/emqx_ft_storage.erl +++ b/apps/emqx_ft/src/emqx_ft_storage.erl @@ -182,8 +182,8 @@ on_backend_update(BackendOld, BackendNew) when %%-------------------------------------------------------------------- -spec backend(config()) -> backend(). -backend(#{local := Storage}) -> - {local, Storage}. +backend(Config) -> + emqx_ft_schema:backend(Config). on_storage_start({Type, Storage}) -> (mod(Type)):start(Storage). diff --git a/apps/emqx_ft/src/emqx_ft_storage_exporter.erl b/apps/emqx_ft/src/emqx_ft_storage_exporter.erl index 6f2b9bea1..4c9cac67a 100644 --- a/apps/emqx_ft/src/emqx_ft_storage_exporter.erl +++ b/apps/emqx_ft/src/emqx_ft_storage_exporter.erl @@ -175,10 +175,10 @@ stop({ExporterMod, ExporterOpts}) -> %%------------------------------------------------------------------------------ exporter(Storage) -> - case maps:get(exporter, Storage) of - #{local := Options} -> + case emqx_ft_schema:backend(maps:get(exporter, Storage)) of + {local, Options} -> {emqx_ft_storage_exporter_fs, Options}; - #{s3 := Options} -> + {s3, Options} -> {emqx_ft_storage_exporter_s3, Options} end. diff --git a/apps/emqx_ft/src/emqx_ft_storage_exporter_fs.erl b/apps/emqx_ft/src/emqx_ft_storage_exporter_fs.erl index e211cb421..589949bda 100644 --- a/apps/emqx_ft/src/emqx_ft_storage_exporter_fs.erl +++ b/apps/emqx_ft/src/emqx_ft_storage_exporter_fs.erl @@ -46,6 +46,7 @@ -export_type([options/0]). -type options() :: #{ + enable := true, root => file:name(), _ => _ }. diff --git a/apps/emqx_ft/src/emqx_ft_storage_fs.erl b/apps/emqx_ft/src/emqx_ft_storage_fs.erl index e84d35328..85aa08405 100644 --- a/apps/emqx_ft/src/emqx_ft_storage_fs.erl +++ b/apps/emqx_ft/src/emqx_ft_storage_fs.erl @@ -102,6 +102,7 @@ -type storage() :: #{ type := 'local', + enable := true, segments := segments(), exporter := emqx_ft_storage_exporter:exporter() }. diff --git a/apps/emqx_ft/test/emqx_ft_assembler_SUITE.erl b/apps/emqx_ft/test/emqx_ft_assembler_SUITE.erl index 1dcc8a79d..50d8579fc 100644 --- a/apps/emqx_ft/test/emqx_ft_assembler_SUITE.erl +++ b/apps/emqx_ft/test/emqx_ft_assembler_SUITE.erl @@ -256,6 +256,7 @@ storage(Config) -> }, <<"exporter">> => #{ <<"local">> => #{ + <<"enable">> => true, <<"root">> => ?config(exports_root, Config) } } diff --git a/apps/emqx_ft/test/emqx_ft_conf_SUITE.erl b/apps/emqx_ft/test/emqx_ft_conf_SUITE.erl index f235b5ebb..0f949855f 100644 --- a/apps/emqx_ft/test/emqx_ft_conf_SUITE.erl +++ b/apps/emqx_ft/test/emqx_ft_conf_SUITE.erl @@ -77,6 +77,7 @@ t_update_config(_Config) -> }, <<"exporter">> => #{ <<"local">> => #{ + <<"enable">> => true, <<"root">> => <<"/tmp/exports">> } } @@ -104,7 +105,10 @@ t_disable_restore_config(Config) -> {ok, _}, emqx_conf:update( [file_transfer], - #{<<"enable">> => true, <<"storage">> => #{<<"local">> => #{}}}, + #{ + <<"enable">> => true, + <<"storage">> => #{<<"local">> => #{}} + }, #{} ) ), @@ -207,6 +211,7 @@ t_switch_exporter(_Config) -> [file_transfer, storage, local, exporter], #{ <<"s3">> => #{ + <<"enable">> => true, <<"bucket">> => <<"emqx">>, <<"host">> => <<"https://localhost">>, <<"port">> => 9000, @@ -234,7 +239,7 @@ t_switch_exporter(_Config) -> {ok, _}, emqx_conf:update( [file_transfer, storage, local, exporter], - #{<<"local">> => #{}}, + #{<<"local">> => #{<<"enable">> => true}}, #{} ) ), diff --git a/apps/emqx_ft/test/emqx_ft_storage_fs_gc_SUITE.erl b/apps/emqx_ft/test/emqx_ft_storage_fs_gc_SUITE.erl index 12f91c808..e008bb2b1 100644 --- a/apps/emqx_ft/test/emqx_ft_storage_fs_gc_SUITE.erl +++ b/apps/emqx_ft/test/emqx_ft_storage_fs_gc_SUITE.erl @@ -45,9 +45,10 @@ init_per_testcase(TC, Config) -> <<"enable">> => true, <<"storage">> => #{ <<"local">> => #{ + <<"enable">> => true, <<"segments">> => #{<<"root">> => SegmentsRoot}, <<"exporter">> => #{ - <<"local">> => #{<<"root">> => ExportsRoot} + <<"local">> => #{<<"enable">> => true, <<"root">> => ExportsRoot} } } } diff --git a/apps/emqx_ft/test/emqx_ft_test_helpers.erl b/apps/emqx_ft/test/emqx_ft_test_helpers.erl index 2eb6d84db..a041dcd50 100644 --- a/apps/emqx_ft/test/emqx_ft_test_helpers.erl +++ b/apps/emqx_ft/test/emqx_ft_test_helpers.erl @@ -55,17 +55,24 @@ local_storage(Config) -> local_storage(Config, Opts) -> #{ <<"local">> => #{ + <<"enable">> => true, <<"segments">> => #{<<"root">> => root(Config, node(), [segments])}, <<"exporter">> => exporter(Config, Opts) } }. exporter(Config, #{exporter := local}) -> - #{<<"local">> => #{<<"root">> => root(Config, node(), [exports])}}; + #{ + <<"local">> => #{ + <<"enable">> => true, + <<"root">> => root(Config, node(), [exports]) + } + }; exporter(_Config, #{exporter := s3, bucket_name := BucketName}) -> BaseConfig = emqx_s3_test_helpers:base_raw_config(tcp), #{ <<"s3">> => BaseConfig#{ + <<"enable">> => true, <<"bucket">> => list_to_binary(BucketName), <<"host">> => ?S3_HOST, <<"port">> => ?S3_PORT diff --git a/rel/i18n/emqx_ft_schema.hocon b/rel/i18n/emqx_ft_schema.hocon index bafda331a..aabdd2032 100644 --- a/rel/i18n/emqx_ft_schema.hocon +++ b/rel/i18n/emqx_ft_schema.hocon @@ -18,6 +18,9 @@ store_segment_timeout.desc: """Timeout for storing a file segment.
After reaching the timeout, message with the segment will be acked with an error""" +backend_enable.desc: +"""Whether to enable this backend.""" + storage_backend.desc: """Storage settings for file transfer.""" From e3c16be3c4b3bc2691802212f2c5736d981f5bb4 Mon Sep 17 00:00:00 2001 From: Andrew Mayorov Date: Thu, 8 Jun 2023 08:13:42 +0300 Subject: [PATCH 2/3] fix(ftschema): ensure `translate` follow `emqx_config` behavior So that atom keys won't unexpectedly get to converters / validators. --- apps/emqx_ft/src/emqx_ft_schema.erl | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/apps/emqx_ft/src/emqx_ft_schema.erl b/apps/emqx_ft/src/emqx_ft_schema.erl index d7df09a54..6ee7c5e10 100644 --- a/apps/emqx_ft/src/emqx_ft_schema.erl +++ b/apps/emqx_ft/src/emqx_ft_schema.erl @@ -288,12 +288,7 @@ validator(filename) -> ]; validator(backend) -> fun(Config) -> - Enabled = maps:filter( - fun(_, Backend) -> - maps:get(enable, Backend, false) or maps:get(<<"enable">>, Backend, false) - end, - Config - ), + Enabled = maps:filter(fun(_, #{<<"enable">> := E}) -> E end, Config), case maps:to_list(Enabled) of [{_Type, _BackendConfig}] -> ok; @@ -334,12 +329,9 @@ ref(Ref) -> translate(Conf) -> [Root] = roots(), - maps:get( - Root, - hocon_tconf:check_plain( - ?MODULE, #{atom_to_binary(Root) => Conf}, #{atom_key => true}, [Root] - ) - ). + RootRaw = atom_to_binary(Root), + ConfChecked = hocon_tconf:check_plain(?MODULE, #{RootRaw => Conf}, #{}, [Root]), + emqx_utils_maps:unsafe_atom_key_map(maps:get(RootRaw, ConfChecked)). -spec backend(emqx_config:config()) -> {_Type :: atom(), emqx_config:config()}. From ce8cc05cc886b37fc9f781aa8999b0482ac888d7 Mon Sep 17 00:00:00 2001 From: Andrew Mayorov Date: Thu, 8 Jun 2023 14:47:10 +0300 Subject: [PATCH 3/3] chore(ftschema): mention `translate` is for tests purpose only --- apps/emqx_ft/src/emqx_ft_schema.erl | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/apps/emqx_ft/src/emqx_ft_schema.erl b/apps/emqx_ft/src/emqx_ft_schema.erl index 6ee7c5e10..90b80ea3d 100644 --- a/apps/emqx_ft/src/emqx_ft_schema.erl +++ b/apps/emqx_ft/src/emqx_ft_schema.erl @@ -25,9 +25,12 @@ -export([schema/1]). --export([translate/1]). +%% Utilities -export([backend/1]). +%% Test-only helpers +-export([translate/1]). + -type json_value() :: null | boolean() @@ -327,11 +330,7 @@ converter(unicode_string) -> ref(Ref) -> ref(?MODULE, Ref). -translate(Conf) -> - [Root] = roots(), - RootRaw = atom_to_binary(Root), - ConfChecked = hocon_tconf:check_plain(?MODULE, #{RootRaw => Conf}, #{}, [Root]), - emqx_utils_maps:unsafe_atom_key_map(maps:get(RootRaw, ConfChecked)). +%% Utilities -spec backend(emqx_config:config()) -> {_Type :: atom(), emqx_config:config()}. @@ -342,3 +341,13 @@ backend(Config) -> no_return(). emit_enabled(Type, BConf = #{enable := Enabled}) -> Enabled andalso throw({Type, BConf}). + +%% Test-only helpers + +-spec translate(emqx_config:raw_config()) -> + emqx_config:config(). +translate(Conf) -> + [Root] = roots(), + RootRaw = atom_to_binary(Root), + ConfChecked = hocon_tconf:check_plain(?MODULE, #{RootRaw => Conf}, #{}, [Root]), + emqx_utils_maps:unsafe_atom_key_map(maps:get(RootRaw, ConfChecked)).