From c89ec0b1f7a9897170bef95dfb998f6cadf4394b Mon Sep 17 00:00:00 2001 From: Thales Macedo Garitezi Date: Thu, 23 Nov 2023 10:25:36 -0300 Subject: [PATCH] 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. --- apps/emqx_bridge/src/emqx_bridge.erl | 1 - apps/emqx_bridge/src/emqx_bridge_api.erl | 18 +---------- apps/emqx_bridge/src/emqx_bridge_v2.erl | 30 +++++++++---------- apps/emqx_bridge/test/emqx_bridge_testlib.erl | 16 ++++++++++ .../test/emqx_bridge_v2_testlib.erl | 1 + .../emqx_bridge_azure_event_hub_v2_SUITE.erl | 9 ++++++ .../emqx_bridge_v2_kafka_producer_SUITE.erl | 14 +++++++++ 7 files changed, 56 insertions(+), 33 deletions(-) diff --git a/apps/emqx_bridge/src/emqx_bridge.erl b/apps/emqx_bridge/src/emqx_bridge.erl index 64bec3a4e..569c1e75a 100644 --- a/apps/emqx_bridge/src/emqx_bridge.erl +++ b/apps/emqx_bridge/src/emqx_bridge.erl @@ -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), diff --git a/apps/emqx_bridge/src/emqx_bridge_api.erl b/apps/emqx_bridge/src/emqx_bridge_api.erl index d263817bf..188f26ab5 100644 --- a/apps/emqx_bridge/src/emqx_bridge_api.erl +++ b/apps/emqx_bridge/src/emqx_bridge_api.erl @@ -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. diff --git a/apps/emqx_bridge/src/emqx_bridge_v2.erl b/apps/emqx_bridge/src/emqx_bridge_v2.erl index 706849965..d9ca1acce 100644 --- a/apps/emqx_bridge/src/emqx_bridge_v2.erl +++ b/apps/emqx_bridge/src/emqx_bridge_v2.erl @@ -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>>, diff --git a/apps/emqx_bridge/test/emqx_bridge_testlib.erl b/apps/emqx_bridge/test/emqx_bridge_testlib.erl index df404d9b0..f486e5d64 100644 --- a/apps/emqx_bridge/test/emqx_bridge_testlib.erl +++ b/apps/emqx_bridge/test/emqx_bridge_testlib.erl @@ -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 = #{}). diff --git a/apps/emqx_bridge/test/emqx_bridge_v2_testlib.erl b/apps/emqx_bridge/test/emqx_bridge_v2_testlib.erl index 6c48f5663..5cb9b043f 100644 --- a/apps/emqx_bridge/test/emqx_bridge_v2_testlib.erl +++ b/apps/emqx_bridge/test/emqx_bridge_v2_testlib.erl @@ -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), diff --git a/apps/emqx_bridge_azure_event_hub/test/emqx_bridge_azure_event_hub_v2_SUITE.erl b/apps/emqx_bridge_azure_event_hub/test/emqx_bridge_azure_event_hub_v2_SUITE.erl index 4d441ea0b..9661004d0 100644 --- a/apps/emqx_bridge_azure_event_hub/test/emqx_bridge_azure_event_hub_v2_SUITE.erl +++ b/apps/emqx_bridge_azure_event_hub/test/emqx_bridge_azure_event_hub_v2_SUITE.erl @@ -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. diff --git a/apps/emqx_bridge_kafka/test/emqx_bridge_v2_kafka_producer_SUITE.erl b/apps/emqx_bridge_kafka/test/emqx_bridge_v2_kafka_producer_SUITE.erl index 8ce3b7f6b..2ad0504b4 100644 --- a/apps/emqx_bridge_kafka/test/emqx_bridge_v2_kafka_producer_SUITE.erl +++ b/apps/emqx_bridge_kafka/test/emqx_bridge_v2_kafka_producer_SUITE.erl @@ -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.