fix(bridge_api): don't mangle configs, use correct type as argument

Fixes https://emqx.atlassian.net/browse/EMQX-11412

- The wrong type was being used in a list lookup function, resulting in the automatic
transformation being called erroneously and mangling the config.
- There was a left-over workaround still around which could still mangle the config.
This commit is contained in:
Thales Macedo Garitezi 2023-11-23 10:25:36 -03:00
parent 209077593d
commit c89ec0b1f7
7 changed files with 56 additions and 33 deletions

View File

@ -313,7 +313,6 @@ list() ->
BridgeV2Bridges =
emqx_bridge_v2:bridge_v1_list_and_transform(),
BridgeV1Bridges ++ BridgeV2Bridges.
%%BridgeV2Bridges = emqx_bridge_v2:list().
lookup(Id) ->
{Type, Name} = emqx_bridge_resource:parse_bridge_id(Id),

View File

@ -900,7 +900,7 @@ format_resource(
case emqx_bridge_v2:is_bridge_v2_type(Type) of
true ->
%% The defaults are already filled in
downgrade_raw_conf(Type, RawConf);
RawConf;
false ->
fill_defaults(Type, RawConf)
end,
@ -1164,19 +1164,3 @@ upgrade_type(Type) ->
downgrade_type(Type) ->
emqx_bridge_lib:downgrade_type(Type).
%% TODO: move it to callback
downgrade_raw_conf(kafka_producer, RawConf) ->
rename(<<"parameters">>, <<"kafka">>, RawConf);
downgrade_raw_conf(azure_event_hub_producer, RawConf) ->
rename(<<"parameters">>, <<"kafka">>, RawConf);
downgrade_raw_conf(_Type, RawConf) ->
RawConf.
rename(OldKey, NewKey, Map) ->
case maps:find(OldKey, Map) of
{ok, Value} ->
maps:remove(OldKey, maps:put(NewKey, Value, Map));
error ->
Map
end.

View File

@ -1063,17 +1063,17 @@ bridge_v1_list_and_transform() ->
Bridges = list_with_lookup_fun(fun bridge_v1_lookup_and_transform/2),
[B || B <- Bridges, B =/= not_bridge_v1_compatible_error()].
bridge_v1_lookup_and_transform(BridgeV1Type, Name) ->
bridge_v1_lookup_and_transform(ActionType, Name) ->
BridgeV1Type = ?MODULE:bridge_v2_type_to_bridge_v1_type(ActionType),
case ?MODULE:bridge_v1_is_valid(BridgeV1Type, Name) of
true ->
Type = ?MODULE:bridge_v1_type_to_bridge_v2_type(BridgeV1Type),
case lookup(Type, Name) of
case lookup(ActionType, Name) of
{ok, #{raw_config := #{<<"connector">> := ConnectorName}} = BridgeV2} ->
ConnectorType = connector_type(Type),
ConnectorType = connector_type(ActionType),
case emqx_connector:lookup(ConnectorType, ConnectorName) of
{ok, Connector} ->
bridge_v1_lookup_and_transform_helper(
BridgeV1Type, Name, Type, BridgeV2, ConnectorType, Connector
BridgeV1Type, Name, ActionType, BridgeV2, ConnectorType, Connector
);
Error ->
Error
@ -1089,7 +1089,7 @@ not_bridge_v1_compatible_error() ->
{error, not_bridge_v1_compatible}.
bridge_v1_lookup_and_transform_helper(
BridgeV1Type, BridgeName, BridgeV2Type, BridgeV2, ConnectorType, Connector
BridgeV1Type, BridgeName, ActionType, Action, ConnectorType, Connector
) ->
ConnectorRawConfig1 = maps:get(raw_config, Connector),
ConnectorRawConfig2 = fill_defaults(
@ -1098,10 +1098,10 @@ bridge_v1_lookup_and_transform_helper(
<<"connectors">>,
emqx_connector_schema
),
BridgeV2RawConfig1 = maps:get(raw_config, BridgeV2),
BridgeV2RawConfig2 = fill_defaults(
BridgeV2Type,
BridgeV2RawConfig1,
ActionRawConfig1 = maps:get(raw_config, Action),
ActionRawConfig2 = fill_defaults(
ActionType,
ActionRawConfig1,
<<"actions">>,
emqx_bridge_v2_schema
),
@ -1110,7 +1110,7 @@ bridge_v1_lookup_and_transform_helper(
emqx_action_info:has_custom_connector_action_config_to_bridge_v1_config(BridgeV1Type)
of
false ->
BridgeV1Config1 = maps:remove(<<"connector">>, BridgeV2RawConfig2),
BridgeV1Config1 = maps:remove(<<"connector">>, ActionRawConfig2),
%% Move parameters to the top level
ParametersMap = maps:get(<<"parameters">>, BridgeV1Config1, #{}),
BridgeV1Config2 = maps:remove(<<"parameters">>, BridgeV1Config1),
@ -1118,13 +1118,13 @@ bridge_v1_lookup_and_transform_helper(
emqx_utils_maps:deep_merge(ConnectorRawConfig2, BridgeV1Config3);
true ->
emqx_action_info:connector_action_config_to_bridge_v1_config(
BridgeV1Type, ConnectorRawConfig2, BridgeV2RawConfig2
BridgeV1Type, ConnectorRawConfig2, ActionRawConfig2
)
end,
BridgeV1Tmp = maps:put(raw_config, BridgeV1ConfigFinal, BridgeV2),
BridgeV1Tmp = maps:put(raw_config, BridgeV1ConfigFinal, Action),
BridgeV1 = maps:remove(status, BridgeV1Tmp),
BridgeV2Status = maps:get(status, BridgeV2, undefined),
BridgeV2Error = maps:get(error, BridgeV2, undefined),
BridgeV2Status = maps:get(status, Action, undefined),
BridgeV2Error = maps:get(error, Action, undefined),
ResourceData1 = maps:get(resource_data, BridgeV1, #{}),
%% Replace id in resouce data
BridgeV1Id = <<"bridge:", (bin(BridgeV1Type))/binary, ":", (bin(BridgeName))/binary>>,

View File

@ -120,6 +120,22 @@ create_bridge(Config, Overrides) ->
ct:pal("creating bridge with config: ~p", [BridgeConfig]),
emqx_bridge:create(BridgeType, BridgeName, BridgeConfig).
list_bridges_api() ->
Params = [],
Path = emqx_mgmt_api_test_util:api_path(["bridges"]),
AuthHeader = emqx_mgmt_api_test_util:auth_header_(),
Opts = #{return_all => true},
ct:pal("listing bridges (via http)"),
Res =
case emqx_mgmt_api_test_util:request_api(get, Path, "", AuthHeader, Params, Opts) of
{ok, {Status, Headers, Body0}} ->
{ok, {Status, Headers, emqx_utils_json:decode(Body0, [return_maps])}};
Error ->
Error
end,
ct:pal("list bridge result: ~p", [Res]),
Res.
create_bridge_api(Config) ->
create_bridge_api(Config, _Overrides = #{}).

View File

@ -139,6 +139,7 @@ create_bridge(Config, Overrides) ->
ConnectorName = ?config(connector_name, Config),
ConnectorType = ?config(connector_type, Config),
ConnectorConfig = ?config(connector_config, Config),
ct:pal("creating connector with config: ~p", [ConnectorConfig]),
{ok, _} =
emqx_connector:create(ConnectorType, ConnectorName, ConnectorConfig),

View File

@ -368,3 +368,12 @@ t_parameters_key_api_spec(_Config) ->
?assert(is_map_key(<<"parameters">>, ActionProps), #{action_props => ActionProps}),
ok.
t_http_api_get(Config) ->
?assertMatch({ok, _}, emqx_bridge_v2_testlib:create_bridge(Config)),
%% v1 api; no mangling of configs; has `kafka' top level config key
?assertMatch(
{ok, {{_, 200, _}, _, [#{<<"kafka">> := _}]}},
emqx_bridge_testlib:list_bridges_api()
),
ok.

View File

@ -369,3 +369,17 @@ t_parameters_key_api_spec(_Config) ->
?assert(is_map_key(<<"parameters">>, ActionProps), #{action_props => ActionProps}),
ok.
t_http_api_get(_Config) ->
ConnectorName = <<"test_connector">>,
ActionName = <<"test_action">>,
ActionConfig = bridge_v2_config(<<"test_connector">>),
ConnectorConfig = connector_config(),
?assertMatch({ok, _}, create_connector(ConnectorName, ConnectorConfig)),
?assertMatch({ok, _}, create_action(ActionName, ActionConfig)),
%% v1 api; no mangling of configs; has `kafka' top level config key
?assertMatch(
{ok, {{_, 200, _}, _, [#{<<"kafka">> := _}]}},
emqx_bridge_testlib:list_bridges_api()
),
ok.